[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
Hi Cornelia, Michael, > From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis- > open.org> On Behalf Of Parav Pandit > Sent: Wednesday, October 18, 2023 4:43 PM > > > 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". > I have addressed the comments of v3 in v4 at [1]. The device requirement is kept as MUST to keep it aligned with the "driver requirements section".' [1] 20231017143135.758523-1-parav@nvidia.com/T/#mc49b6e54186c5f1b1e8c743bc440ca0cc0938212">https://lore.kernel.org/virtio-comment/20231017143135.758523-1-parav@nvidia.com/T/#mc49b6e54186c5f1b1e8c743bc440ca0cc0938212 Please review.
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]