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.
