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

[Michael S. Tsirkin]

On Fri, Mar 29, 2024 at 03:20:59PM +0800, Jason Wang wrote:
> 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.

That seems orthogonal or did I miss something?

> > 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.