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

["Michael S. Tsirkin" <mst@redhat.com>]

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.