On Thu, Mar 28, 2024 at 09:02:28AM +0000, Chen, Jiqian wrote:
>
> 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?
Works for me.
> >
> >
>
> --
> Best regards,
> Jiqian Chen.
