On 2024/4/7 11:20, Jason Wang wrote:
> On Tue, Apr 2, 2024 at 11:03 AM Chen, Jiqian <jiqian.c...@amd.com> wrote:
>>
>> On 2024/3/29 18:44, Michael S. Tsirkin wrote:
>>> 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?
>> Yes, I looked the detail of the proposal, I also think they are unrelated.
>
> The point is the proposal said
>
> """
> Without a mechanism to
> suspend/resume virtio devices when the driver is suspended/resumed in
> the early phase of suspend/late phase of resume, there is a window where
> interrupts can be lost.
> """
>
> It looks safe to enable it with the suspend bit. Or if you think it's
> wrong, please comment on the virtio spec patch.
If I understand the proposal correctly.
Only need to check the SUSPEND bit when virtio_pci_bus_reset_hold is called.
It seems the proposal won't block this patch to upstream.
In next version, I will add comments to note the SUSPEND bit that need to be
considered once it is accepted.
>
>> I will set the default value of No_Soft_Reset bit to true in next version
>> according to your opinion.
>> About the compatibility of old machine types, which types should I consider?
>> Does the same as x-pcie-pm-init(hw_compat_2_8)?
>> Forgive me for not knowing much about compatibility.
>
> "x" means no compatibility at all, please drop the "x" prefix. And it
Thanks to explain.
So it seems the prefix "x" of "x-pcie-pm-init" is also wrong? Because it
considered the hw_compat_2_8. Also "x-pcie-flr-init".
Back to No_Soft_Reset, do you know which old machines should I consider to
compatible with?
> looks more safe to start as "false" by default.
>
> 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.
>
--
Best regards,
Jiqian Chen.
