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