Re: [virtio-comment] Re: [PATCH 0/2] Selective queue enabling

[Eugenio Perez Martin <eperezma@redhat.com>]

On Thu, Jun 15, 2023 at 7:56âAM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>
>
>
> On 6/14/2023 7:56 PM, Parav Pandit wrote:
> >> From: Zhu, Lingshan <lingshan.zhu@intel.com>
> >> Sent: Wednesday, June 14, 2023 2:11 AM
> >> On 6/14/2023 12:32 PM, Parav Pandit wrote:
> >>>> From: Zhu, Lingshan <lingshan.zhu@intel.com>
> >>>> Sent: Wednesday, June 14, 2023 12:27 AM
> >>>>> If a device has assumed that queue_reset must be done after
> >>>>> DRIVER_OK stage because until that point the device is not "live",
> >>>>> Such device needs the
> >>>> fix.
> >>>> I think queue_reset just resets the queue and its fields including
> >>>> the queue_enable. DRIVER_OK is not a must for performing a
> >>>> queue_reset, My understanding is:
> >>>> - before DRIVER_OK, the driver can re-config the queue anytime,
> >>>> doesn't need a reset_queue
> >>> Before DRIVER_OK stage, to reconfigure a queue, driver needs to write
> >> queue_enable = 0.
> >>> This is not allowed in the spec.
> >>> So driver need to use queue_reset to reconfigure.
> >> The spec requires the driver config all other fields before set queue_enable.
> >> I am not sure whether there are real occurring cases that re-config the vq fields
> >> between queue_enable and DRIVER_OK in the initialization.
> >>
> > Probably there is no case today, but may be it can occur where driver tries to configure something and if it fails, it may need to roll back and re-configure queues.
> >
> >> But I agree maybe we should allow set queue_enable = 0.
> >>
> > This functionality of queue_enable = 0, is supported using queue_reset register.
> >
> >> This reset may happen after DRIVER_OK.
> > Yes, the queue_reset after DRIVER_OK is not ambiguous.
> > Queue_reset before DRIVER_OK has the ambiguity, and will clarify that it should be supported by device before DRIVER_OK.
> >
> > For the main motivation of this patch, the _RESET bit seems enough to selectively enable the queue before or after DRIVER_OK.
> I guess the intention may be:
> Step 1: the hypervisor owns all queues, it enables all queues and cvq,
> set queue fields with phony host values.
> Then set DRIVER_OK, config the device and send commands through cvq.
> But don't do link announce. So the device would not consume the
> descriptors and external network will not send
> any data to it.
>

Even without a link announcement, the device can process descriptors.
Some examples are receiving broadcast packets or just sending any
packet (which effectively acts as a link announcement for the network
topology).

Thanks!

> Step 2: Reset the queues, set the fields of the queues with guest
> config, selectively enable the queues,
> so the device works for the guest.
>
> Does this sounds good?
>
>
>