Re: [RFC QEMU PATCH v7 1/1] virtio-pci: implement No_Soft_Reset bit

[Chen, Jiqian]

On 2024/3/28 16:11, Michael S. Tsirkin wrote:
> 
>> +        VirtIOPCIProxy *proxy = VIRTIO_PCI(dev);
>> +
>>          pcie_cap_deverr_reset(dev);
>>          pcie_cap_lnkctl_reset(dev);
>>  
>> -        pci_set_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL, 0);
>> +        if (proxy->flags & VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET) {
>> +            val |= PCI_PM_CTRL_NO_SOFT_RESET;
>> +        }
>> +        pci_set_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL, val);
> 
> 
> There is no need to do it like this - only state is writeable
> anyway. So simply
>       pci_word_test_and_clear_mask(dev->config + dev->exp.pm_cap + 
> PCI_PM_CTRL, PCI_PM_CTRL_STATE_MASK)
> 
> 
> maybe we should actually check here:
>        if (proxy->flags & VIRTIO_PCI_FLAG_INIT_PM)
> there's a chance commit 27ce0f3afc9 broke things for old machines
> and we never noticed. If so that should be a separate bugfix patch though.
Make sense. It is actually a bug imported by 27ce0f3afc9.
According to your comments, I think here should be a separate patch, like:
   if (pci_is_express(dev)) {
        VirtIOPCIProxy *proxy = VIRTIO_PCI(dev);

        pcie_cap_deverr_reset(dev);
        pcie_cap_lnkctl_reset(dev);

        if (proxy->flags & VIRTIO_PCI_FLAG_INIT_PM) {
            pci_word_test_and_clear_mask(
                dev->config + dev->exp.pm_cap + PCI_PM_CTRL,
                PCI_PM_CTRL_STATE_MASK);
        }
    }
Right?

> 
> 

-- 
Best regards,
Jiqian Chen.