Hi Cornelia, Michael,
> From: <-
> open.org> On Behalf Of Parav Pandit
> Sent: Wednesday, October 18, 2023 4:43 PM
>
> > From: Cornelia Huck <>
> > Sent: Wednesday, October 18, 2023 4:34 PM
>
> > On Wed, Oct 18 2023, "Michael S. Tsirkin" <> wrote:
> >
> > > On Wed, Oct 18, 2023 at 12:25:23PM +0200, Cornelia Huck wrote:
> > >> On Tue, Oct 17 2023, Parav Pandit <> wrote:
> > >>
> > >> >> From: Cornelia Huck <>
> > >> >> Sent: Tuesday, October 17, 2023 5:55 PM
> > >> >>
> > >> >> On Mon, Oct 02 2023, Parav Pandit <> 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] /T/#mc49b6e54186c5f1b1e8c743bc440ca0cc0938212">https://lore.kernel.org/virtio-comment/[email protected]/T/#mc49b6e54186c5f1b1e8c743bc440ca0cc0938212
Please review.
