OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-comment message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage



> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Wednesday, October 18, 2023 4:34 PM
 
> On Wed, Oct 18 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Oct 18, 2023 at 12:25:23PM +0200, Cornelia Huck wrote:
> >> On Tue, Oct 17 2023, Parav Pandit <parav@nvidia.com> wrote:
> >>
> >> >> From: Cornelia Huck <cohuck@redhat.com>
> >> >> Sent: Tuesday, October 17, 2023 5:55 PM
> >> >>
> >> >> On Mon, Oct 02 2023, Parav Pandit <parav@nvidia.com> wrote:
> >> >> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver MUST
> >> >> > +enable the required number of virtqueues before setting the
> DRIVER_OK status bit.
> >> >>
> >> >> What does "required" mean here? It just chooses to enable the
> >> >> queues it wants to use, right?
> >> > Right.
> >> > Required meaning, whatever number of queues that driver choose to
> enable, those must be enabled before driver_ok.
> >> > So it is "required by the driver".
> >> > Would that be ok?
> >>
> >> I'd write it as "the driver MUST enable any virtqueue it plans to use"
> >> or something like that.
> >>
> >> (...)
> >
> > It would have to be SHOULD - we can't add new MUST requirements not
> > contingent on a feature bit, we can give recommendation based on
> > existing installed base.
> 
It is not just installed base.
Spec clearly says that:
"Perform device-specific setup, including discovery of virtqueues for the device, optional per-bus setup,
reading and possibly writing the device's virtio configuration space, and population of virtqueues."

There is no feature bit or any text that says "population of virtqueues" can be done later outside of the device initialization phase.
Only reference is _F_RESET bit which is only for re-enabling. (not enabling).

> Then I wonder whether we need to add any normative statement at all -- prior
> to the introduction of this new feature, the driver had to enable anything it
> wanted to use before DRIVER_OK, and if it does not negotiate the new feature,
> nothing changes. Spelling that out in non-normative sections should be
> enough?
It is already implied in the spec that queues must be enabled before DRIVER_OK.
So what is written in normative is not changing any behavior.
It is only making the implied behavior explicit.

If it says SHOULD, that means, one can enable the queue for the first time after DRIVER_OK stage too.
And that would violate the "Driver Requirements: Device Initialization".



[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]