Re: [RFC QEMU PATCH v8 2/2] virtio-pci: implement No_Soft_Reset bit

[Jason Wang]

On Fri, Mar 29, 2024 at 3:07 PM Chen, Jiqian <Jiqian.Chen@amd.com> wrote:
>
> On 2024/3/28 20:36, Michael S. Tsirkin wrote:
> >>>> +}
> >>>> +
> >>>>  static void virtio_pci_bus_reset_hold(Object *obj)
> >>>>  {
> >>>>      PCIDevice *dev = PCI_DEVICE(obj);
> >>>>      DeviceState *qdev = DEVICE(obj);
> >>>>
> >>>> +    if (virtio_pci_no_soft_reset(dev)) {
> >>>> +        return;
> >>>> +    }
> >>>> +
> >>>>      virtio_pci_reset(qdev);
> >>>>
> >>>>      if (pci_is_express(dev)) {
> >>>> @@ -2484,6 +2511,8 @@ static Property virtio_pci_properties[] = {
> >>>>                      VIRTIO_PCI_FLAG_INIT_LNKCTL_BIT, true),
> >>>>      DEFINE_PROP_BIT("x-pcie-pm-init", VirtIOPCIProxy, flags,
> >>>>                      VIRTIO_PCI_FLAG_INIT_PM_BIT, true),
> >>>> +    DEFINE_PROP_BIT("x-pcie-pm-no-soft-reset", VirtIOPCIProxy, flags,
> >>>> +                    VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT, false),

Why does it come with an x prefix?

> >>>>      DEFINE_PROP_BIT("x-pcie-flr-init", VirtIOPCIProxy, flags,
> >>>>                      VIRTIO_PCI_FLAG_INIT_FLR_BIT, true),
> >>>>      DEFINE_PROP_BIT("aer", VirtIOPCIProxy, flags,
> >>>
> >>> I am a bit confused about this part.
> >>> Do you want to make this software controllable?
> >> Yes, because even the real hardware, this bit is not always set.

We are talking about emulated devices here.

> >
> > So which virtio devices should and which should not set this bit?
> This depends on the scenario the virtio-device is used, if we want to trigger 
> an internal soft reset for the virtio-device during S3, this bit shouldn't be 
> set.

If the device doesn't need reset, why bother the driver for this?

Btw, no_soft_reset is insufficient for some cases, there's a proposal
for the virtio-spec. I think we need to wait until it is done.

> In my use case on my environment, I don't want to reset virtio-gpu during S3,
> because once the display resources are destroyed, there are not enough 
> information to re-create them, so this bit should be set.
> Making this bit software controllable is convenient for users to take their 
> own choices.

Thanks

>
> >
> >>> Or should this be set to true by default and then
> >>> changed to false for old machine types?
> >> How can I do so?
> >> Do you mean set this to true by default, and if old machine types don't 
> >> need this bit, they can pass false config to qemu when running qemu?
> >
> > No, you would use compat machinery. See how is x-pcie-flr-init handled.
> >
> >
>
> --
> Best regards,
> Jiqian Chen.