* [PATCH V2] virtio-pci: compat page aligned ATS
@ 2021-04-06 4:03 Jason Wang
2021-04-06 9:39 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 4+ messages in thread
From: Jason Wang @ 2021-04-06 4:03 UTC (permalink / raw)
To: mst, qemu-devel; +Cc: ehabkost, Jason Wang, qemu-stable, peterx, dgilbert
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.
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)
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH V2] virtio-pci: compat page aligned ATS
2021-04-06 4:03 [PATCH V2] virtio-pci: compat page aligned ATS Jason Wang
@ 2021-04-06 9:39 ` Dr. David Alan Gilbert
2021-04-07 8:24 ` Jason Wang
0 siblings, 1 reply; 4+ messages in thread
From: Dr. David Alan Gilbert @ 2021-04-06 9:39 UTC (permalink / raw)
To: Jason Wang; +Cc: ehabkost, mst, qemu-stable, peterx, qemu-devel
* 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.
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH V2] virtio-pci: compat page aligned ATS
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
0 siblings, 1 reply; 4+ messages in thread
From: Jason Wang @ 2021-04-07 8:24 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-devel, qemu-stable, ehabkost, peterx, mst
在 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?
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)
>>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH V2] virtio-pci: compat page aligned ATS
2021-04-07 8:24 ` Jason Wang
@ 2021-04-07 8:50 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 4+ messages in thread
From: Dr. David Alan Gilbert @ 2021-04-07 8:50 UTC (permalink / raw)
To: Jason Wang; +Cc: qemu-devel, qemu-stable, ehabkost, peterx, mst
* 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-04-07 8:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-06 4:03 [PATCH V2] virtio-pci: compat page aligned ATS 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 is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.