qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-stable@nongnu.org,
	ehabkost@redhat.com, peterx@redhat.com, mst@redhat.com
Subject: Re: [PATCH V2] virtio-pci: compat page aligned ATS
Date: Wed, 7 Apr 2021 09:50:50 +0100	[thread overview]
Message-ID: <YG1yam6UlqWosWvv@work-vm> (raw)
In-Reply-To: <3537aa98-f6fc-bf99-cbc6-3c90f1387dc5@redhat.com>

* Jason Wang (jasowang@redhat.com) wrote:
> 
> 在 2021/4/6 下午5:39, Dr. David Alan Gilbert 写道:
> > * Jason Wang (jasowang@redhat.com) wrote:
> > > Commit 4c70875372b8 ("pci: advertise a page aligned ATS") advertises
> > > the page aligned via ATS capability (RO) to unbrek recent Linux IOMMU
> > > drivers since 5.2. But it forgot the compat the capability which
> > > breaks the migration from old machine type:
> > > 
> > > (qemu) qemu-kvm: get_pci_config_device: Bad config data: i=0x104 read:
> > > 0 device: 20 cmask: ff wmask: 0 w1cmask:0
> > > 
> > > This patch introduces a new parameter "x-ats-page-aligned" for
> > > virtio-pci device and turns it on for machine type which is newer than
> > > 5.1.
> > I don't know the IOMMU code, but from the property/hw_compat I think
> > this is OK.
> > It's a bit tricky; since this was broken in 5.2, changing the
> > hw_compat_5_1 means that there are exisitng 5.2 users who will be broken
> > - howeer, the original 4c70875372b8 patch was sent to qemu-stable
> > which I think means there are other versions that are inconsistent as
> > well.
> 
> 
> Good point. It's not an easy task, we need to choose to break migration or
> virtio with vIOMMU.
> 
> This patch chooes to do the latter, or is there any other better way to
> solve this?

Not that I can think of.

Dave

> Thanks
> 
> 
> > 
> > Dave
> > 
> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > Cc: Peter Xu <peterx@redhat.com>
> > > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > Cc: qemu-stable@nongnu.org
> > > Fixes: 4c70875372b8 ("pci: advertise a page aligned ATS")
> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > ---
> > > Changes since V1:
> > > - switch to use "x-ats-page-aligned" instead
> > > ---
> > >   hw/core/machine.c      |  1 +
> > >   hw/pci/pcie.c          | 10 ++++++----
> > >   hw/virtio/virtio-pci.c |  5 ++++-
> > >   hw/virtio/virtio-pci.h |  5 +++++
> > >   include/hw/pci/pcie.h  |  2 +-
> > >   5 files changed, 17 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > index 9935c6ddd5..a50f2d5f0a 100644
> > > --- a/hw/core/machine.c
> > > +++ b/hw/core/machine.c
> > > @@ -53,6 +53,7 @@ GlobalProperty hw_compat_5_1[] = {
> > >       { "nvme", "use-intel-id", "on"},
> > >       { "pvpanic", "events", "1"}, /* PVPANIC_PANICKED */
> > >       { "pl011", "migrate-clk", "off" },
> > > +    { "virtio-pci", "x-ats-page-aligned", "off"},
> > >   };
> > >   const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1);
> > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > index a733e2fb87..fd0fa157e8 100644
> > > --- a/hw/pci/pcie.c
> > > +++ b/hw/pci/pcie.c
> > > @@ -963,16 +963,18 @@ void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num)
> > >       pci_set_quad(dev->config + offset + pci_dsn_cap, ser_num);
> > >   }
> > > -void pcie_ats_init(PCIDevice *dev, uint16_t offset)
> > > +void pcie_ats_init(PCIDevice *dev, uint16_t offset, bool aligned)
> > >   {
> > >       pcie_add_capability(dev, PCI_EXT_CAP_ID_ATS, 0x1,
> > >                           offset, PCI_EXT_CAP_ATS_SIZEOF);
> > >       dev->exp.ats_cap = offset;
> > > -    /* Invalidate Queue Depth 0, Page Aligned Request 1 */
> > > -    pci_set_word(dev->config + offset + PCI_ATS_CAP,
> > > -                 PCI_ATS_CAP_PAGE_ALIGNED);
> > > +    /* Invalidate Queue Depth 0 */
> > > +    if (aligned) {
> > > +        pci_set_word(dev->config + offset + PCI_ATS_CAP,
> > > +                     PCI_ATS_CAP_PAGE_ALIGNED);
> > > +    }
> > >       /* STU 0, Disabled by default */
> > >       pci_set_word(dev->config + offset + PCI_ATS_CTRL, 0);
> > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > > index 883045a223..2b7142a21e 100644
> > > --- a/hw/virtio/virtio-pci.c
> > > +++ b/hw/virtio/virtio-pci.c
> > > @@ -1848,7 +1848,8 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
> > >           }
> > >           if (proxy->flags & VIRTIO_PCI_FLAG_ATS) {
> > > -            pcie_ats_init(pci_dev, last_pcie_cap_offset);
> > > +            pcie_ats_init(pci_dev, last_pcie_cap_offset,
> > > +                          proxy->flags & VIRTIO_PCI_FLAG_ATS_PAGE_ALIGNED);
> > >               last_pcie_cap_offset += PCI_EXT_CAP_ATS_SIZEOF;
> > >           }
> > > @@ -1925,6 +1926,8 @@ static Property virtio_pci_properties[] = {
> > >                        ignore_backend_features, false),
> > >       DEFINE_PROP_BIT("ats", VirtIOPCIProxy, flags,
> > >                       VIRTIO_PCI_FLAG_ATS_BIT, false),
> > > +    DEFINE_PROP_BIT("x-ats-page-aligned", VirtIOPCIProxy, flags,
> > > +                    VIRTIO_PCI_FLAG_ATS_PAGE_ALIGNED_BIT, true),
> > >       DEFINE_PROP_BIT("x-pcie-deverr-init", VirtIOPCIProxy, flags,
> > >                       VIRTIO_PCI_FLAG_INIT_DEVERR_BIT, true),
> > >       DEFINE_PROP_BIT("x-pcie-lnkctl-init", VirtIOPCIProxy, flags,
> > > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> > > index d7d5d403a9..2446dcd9ae 100644
> > > --- a/hw/virtio/virtio-pci.h
> > > +++ b/hw/virtio/virtio-pci.h
> > > @@ -42,6 +42,7 @@ enum {
> > >       VIRTIO_PCI_FLAG_INIT_PM_BIT,
> > >       VIRTIO_PCI_FLAG_INIT_FLR_BIT,
> > >       VIRTIO_PCI_FLAG_AER_BIT,
> > > +    VIRTIO_PCI_FLAG_ATS_PAGE_ALIGNED_BIT,
> > >   };
> > >   /* Need to activate work-arounds for buggy guests at vmstate load. */
> > > @@ -84,6 +85,10 @@ enum {
> > >   /* Advanced Error Reporting capability */
> > >   #define VIRTIO_PCI_FLAG_AER (1 << VIRTIO_PCI_FLAG_AER_BIT)
> > > +/* Page Aligned Address space Translation Service */
> > > +#define VIRTIO_PCI_FLAG_ATS_PAGE_ALIGNED \
> > > +  (1 << VIRTIO_PCI_FLAG_ATS_PAGE_ALIGNED_BIT)
> > > +
> > >   typedef struct {
> > >       MSIMessage msg;
> > >       int virq;
> > > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> > > index 14c58ebdb6..6063bee0ec 100644
> > > --- a/include/hw/pci/pcie.h
> > > +++ b/include/hw/pci/pcie.h
> > > @@ -137,7 +137,7 @@ void pcie_acs_reset(PCIDevice *dev);
> > >   void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
> > >   void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);
> > > -void pcie_ats_init(PCIDevice *dev, uint16_t offset);
> > > +void pcie_ats_init(PCIDevice *dev, uint16_t offset, bool aligned);
> > >   void pcie_cap_slot_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> > >                                  Error **errp);
> > > -- 
> > > 2.24.3 (Apple Git-128)
> > > 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



      reply	other threads:[~2021-04-07  8:53 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-06  4:03 Jason Wang
2021-04-06  9:39 ` Dr. David Alan Gilbert
2021-04-07  8:24   ` Jason Wang
2021-04-07  8:50     ` Dr. David Alan Gilbert [this message]

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=YG1yam6UlqWosWvv@work-vm \
    --to=dgilbert@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --subject='Re: [PATCH V2] virtio-pci: compat page aligned ATS' \
    /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

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).