On Fri, Mar 29, 2024 at 4:00 PM Chen, Jiqian <jiqian.c...@amd.com> wrote:
>
> On 2024/3/29 15:20, 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?
> Sorry, it's my misunderstanding of this prefix, if No_Soft_Reset doesn't need
> this prefix, I will delete it in next version.
> Does x prefix means compat machinery? Or other meanings?
>
> >
> >>>>>> 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.
> Yes, I just gave an example. It actually this bit is not always set. What's
> your opinion about when to set this bit or which virtio-device should set
> this bit?
If the implementation of Qemu is correct, we should set it unless we
need compatibility.
>
> >
> >>>
> >>> 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?
> I don't know what you mean.
> If the device doesn't need reset, we can config true to set this bit, then on
> the driver side, driver finds this bit is set, then driver will not trigger a
> soft reset.
I mean if the device can suspend without reset, we don't need to
bother the driver to save and load states.
>
> >
> > Btw, no_soft_reset is insufficient for some cases,
> May I know which cases?
>
> > there's a proposal for the virtio-spec. I think we need to wait until it is
> > done.
> Can you share the proposal?
See this
https://lore.kernel.org/all/20240227015345.3614965-1-steve...@chromium.org/T/
Thanks
>
> >
> >> 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.
> >
>
> --
> Best regards,
> Jiqian Chen.
