qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: "Chen, Jiqian" <Jiqian.Chen@amd.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"Huang, Ray" <Ray.Huang@amd.com>,
	Eduardo Habkost <eduardo@habkost.net>,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Subject: Re: [PATCH v11 1/2] virtio-pci: only reset pm state during resetting
Date: Fri, 21 Jun 2024 05:38:19 -0400	[thread overview]
Message-ID: <20240621052334-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <BL1PR12MB58491ED83770E67AC98633D7E7C92@BL1PR12MB5849.namprd12.prod.outlook.com>

On Fri, Jun 21, 2024 at 09:19:11AM +0000, Chen, Jiqian wrote:
> Hi MST,
> 
> On 2024/6/6 18:22, Jiqian Chen wrote:
> > Fix bug imported by 27ce0f3afc9dd ("fix Power Management Control Register for PCI Express virtio devices"
> > After this change, observe that QEMU may erroneously clear the power status of the device,
> > or may erroneously clear non writable registers, such as NO_SOFT_RESET, etc.
> > 
> > Only state of PM_CTRL is writable.
> > Only when flag VIRTIO_PCI_FLAG_INIT_PM is set, need to reset state.
> > 
> > Fixes: 27ce0f3afc9dd ("fix Power Management Control Register for PCI Express virtio devices"
> > 
> > Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
> > ---
> >  hw/virtio/virtio-pci.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index b1d02f4b3de0..1b63bcb3f15c 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -2300,10 +2300,16 @@ static void virtio_pci_bus_reset_hold(Object *obj, ResetType type)
> >      virtio_pci_reset(qdev);
> >  
> >      if (pci_is_express(dev)) {
> > +        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_INIT_PM) {
> > +            pci_word_test_and_clear_mask(
> > +                dev->config + dev->exp.pm_cap + PCI_PM_CTRL,
> > +                PCI_PM_CTRL_STATE_MASK);
> > +        }
> >      }
> >  }
> >  
> I noticed that you merged this patch into the staging before, but then reverted it. Do you still have any concerns?
> 
> -- 
> Best regards,
> Jiqian Chen.

Sorry I don't remember at this point. Normally it's because of
test failures but I then also notify the authors ... 
I will try to re-merge and see.

-- 
MST



  reply	other threads:[~2024-06-21  9:39 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-06 10:22 [PATCH v11 0/2] S3 support Jiqian Chen
2024-06-06 10:22 ` [PATCH v11 1/2] virtio-pci: only reset pm state during resetting Jiqian Chen
2024-06-21  9:19   ` Chen, Jiqian
2024-06-21  9:38     ` Michael S. Tsirkin [this message]
2024-06-06 10:22 ` [PATCH v11 2/2] virtio-pci: implement No_Soft_Reset bit Jiqian Chen
2024-06-21  9:20   ` Chen, Jiqian
2024-07-02  2:36     ` Chen, Jiqian
2024-07-02 11:07       ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240621052334-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=Jiqian.Chen@amd.com \
    --cc=Ray.Huang@amd.com \
    --cc=eduardo@habkost.net \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).