Currently the default input range can extend to 64 bits. On x86,
when the virtio-iommu protects vfio devices, the physical iommu
may support only 39 bits. Let's set the default to 39, as done
for the intel-iommu. On ARM we set 48b as a default (matching
SMMUv3 SMMU_IDR5.VAX == 0).
We use hw_compat_8_2 to handle the compatibility for machines
before 9.0 which used to have a virtio-iommu default input range
of 64 bits.
Of course if aw-bits is set from the command line, the default
is overriden.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v1 -> v2:
- set aw-bits to 48b on ARM
- use hw_compat_8_2 to handle the compat for older machines
which used 64b as a default
---
hw/arm/virt.c | 6 ++++++
hw/core/machine.c | 5 ++++-
hw/i386/pc.c | 6 ++++++
hw/virtio/virtio-iommu.c | 2 +-
4 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index e6ead2c5c8..56539f2fc5 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2718,10 +2718,16 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+ uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
+ "aw-bits", NULL);
hwaddr db_start = 0, db_end = 0;
QList *reserved_regions;
char *resv_prop_str;
+ if (!aw_bits) {
+ qdev_prop_set_uint8(dev, "aw-bits", 48);
+ }
+
if (vms->iommu != VIRT_IOMMU_NONE) {
error_setg(errp, "virt machine does not support multiple IOMMUs");
return;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index fb5afdcae4..70ac96954c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -30,9 +30,12 @@
#include "exec/confidential-guest-support.h"
#include "hw/virtio/virtio-pci.h"
#include "hw/virtio/virtio-net.h"
+#include "hw/virtio/virtio-iommu.h"
#include "audio/audio.h"
-GlobalProperty hw_compat_8_2[] = {};
+GlobalProperty hw_compat_8_2[] = {
+ { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" },
+};
const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
GlobalProperty hw_compat_8_1[] = {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 803244e5cc..0e2bcb4840 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1458,6 +1458,8 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+ uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
+ "aw-bits", NULL);
/* Declare the APIC range as the reserved MSI region */
char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
VIRTIO_IOMMU_RESV_MEM_T_MSI);
@@ -1466,6 +1468,10 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
qlist_append_str(reserved_regions, resv_prop_str);
qdev_prop_set_array(dev, "reserved-regions", reserved_regions);
+ if (!aw_bits) {
+ qdev_prop_set_uint8(dev, "aw-bits", 39);
+ }
+
g_free(resv_prop_str);
}
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 7870bdbeee..c468e9b13b 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -1529,7 +1529,7 @@ static Property virtio_iommu_properties[] = {
DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
TYPE_PCI_BUS, PCIBus *),
DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
- DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
+ DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 0),
DEFINE_PROP_END_OF_LIST(),
};
--
2.41.0On 2/1/24 17:32, Eric Auger wrote:
> Currently the default input range can extend to 64 bits. On x86,
> when the virtio-iommu protects vfio devices, the physical iommu
> may support only 39 bits. Let's set the default to 39, as done
> for the intel-iommu. On ARM we set 48b as a default (matching
> SMMUv3 SMMU_IDR5.VAX == 0).
>
> We use hw_compat_8_2 to handle the compatibility for machines
> before 9.0 which used to have a virtio-iommu default input range
> of 64 bits.
>
> Of course if aw-bits is set from the command line, the default
> is overriden.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> v1 -> v2:
> - set aw-bits to 48b on ARM
> - use hw_compat_8_2 to handle the compat for older machines
> which used 64b as a default
> ---
> hw/arm/virt.c | 6 ++++++
> hw/core/machine.c | 5 ++++-
> hw/i386/pc.c | 6 ++++++
> hw/virtio/virtio-iommu.c | 2 +-
> 4 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index e6ead2c5c8..56539f2fc5 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2718,10 +2718,16 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
> virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> + uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
> + "aw-bits", NULL);
object_property_get_uint() should not fail. Please use &error_abort.
> hwaddr db_start = 0, db_end = 0;
> QList *reserved_regions;
> char *resv_prop_str;
>
> + if (!aw_bits) {
> + qdev_prop_set_uint8(dev, "aw-bits", 48);
> + }
> +
> if (vms->iommu != VIRT_IOMMU_NONE) {
> error_setg(errp, "virt machine does not support multiple IOMMUs");
> return;
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index fb5afdcae4..70ac96954c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -30,9 +30,12 @@
> #include "exec/confidential-guest-support.h"
> #include "hw/virtio/virtio-pci.h"
> #include "hw/virtio/virtio-net.h"
> +#include "hw/virtio/virtio-iommu.h"
> #include "audio/audio.h"
>
> -GlobalProperty hw_compat_8_2[] = {};
> +GlobalProperty hw_compat_8_2[] = {
> + { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" },
> +};
> const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
>
> GlobalProperty hw_compat_8_1[] = {
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 803244e5cc..0e2bcb4840 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1458,6 +1458,8 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
> virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> + uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
> + "aw-bits", NULL);
> /* Declare the APIC range as the reserved MSI region */
> char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
> VIRTIO_IOMMU_RESV_MEM_T_MSI);
> @@ -1466,6 +1468,10 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> qlist_append_str(reserved_regions, resv_prop_str);
> qdev_prop_set_array(dev, "reserved-regions", reserved_regions);
>
> + if (!aw_bits) {
> + qdev_prop_set_uint8(dev, "aw-bits", 39);
May be use VTD_HOST_AW_39BIT instead of 39 ? This would make it
easier to find uses of certain defaults values and would clarify
that the default AW of virtio-iommu is set as intel-iommu.
Thanks,
C.
> + }
> +
> g_free(resv_prop_str);
> }
>
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 7870bdbeee..c468e9b13b 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -1529,7 +1529,7 @@ static Property virtio_iommu_properties[] = {
> DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
> TYPE_PCI_BUS, PCIBus *),
> DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
> - DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
> + DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 0),
> DEFINE_PROP_END_OF_LIST(),
> };
>
On 2/5/24 10:33, Cédric Le Goater wrote:
> On 2/1/24 17:32, Eric Auger wrote:
>> Currently the default input range can extend to 64 bits. On x86,
>> when the virtio-iommu protects vfio devices, the physical iommu
>> may support only 39 bits. Let's set the default to 39, as done
>> for the intel-iommu. On ARM we set 48b as a default (matching
>> SMMUv3 SMMU_IDR5.VAX == 0).
>>
>> We use hw_compat_8_2 to handle the compatibility for machines
>> before 9.0 which used to have a virtio-iommu default input range
>> of 64 bits.
>>
>> Of course if aw-bits is set from the command line, the default
>> is overriden.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v1 -> v2:
>> - set aw-bits to 48b on ARM
>> - use hw_compat_8_2 to handle the compat for older machines
>> which used 64b as a default
>> ---
>> hw/arm/virt.c | 6 ++++++
>> hw/core/machine.c | 5 ++++-
>> hw/i386/pc.c | 6 ++++++
>> hw/virtio/virtio-iommu.c | 2 +-
>> 4 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index e6ead2c5c8..56539f2fc5 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -2718,10 +2718,16 @@ static void
>> virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>> virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev),
>> MACHINE(hotplug_dev), errp);
>> } else if (object_dynamic_cast(OBJECT(dev),
>> TYPE_VIRTIO_IOMMU_PCI)) {
>> + uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>> + "aw-bits", NULL);
>
> object_property_get_uint() should not fail. Please use &error_abort.
sure
>
>> hwaddr db_start = 0, db_end = 0;
>> QList *reserved_regions;
>> char *resv_prop_str;
>> + if (!aw_bits) {
>> + qdev_prop_set_uint8(dev, "aw-bits", 48);
>> + }
>> +
>> if (vms->iommu != VIRT_IOMMU_NONE) {
>> error_setg(errp, "virt machine does not support
>> multiple IOMMUs");
>> return;
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index fb5afdcae4..70ac96954c 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -30,9 +30,12 @@
>> #include "exec/confidential-guest-support.h"
>> #include "hw/virtio/virtio-pci.h"
>> #include "hw/virtio/virtio-net.h"
>> +#include "hw/virtio/virtio-iommu.h"
>> #include "audio/audio.h"
>> -GlobalProperty hw_compat_8_2[] = {};
>> +GlobalProperty hw_compat_8_2[] = {
>> + { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" },
>> +};
>> const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
>> GlobalProperty hw_compat_8_1[] = {
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 803244e5cc..0e2bcb4840 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1458,6 +1458,8 @@ static void
>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>> virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev),
>> MACHINE(hotplug_dev), errp);
>> } else if (object_dynamic_cast(OBJECT(dev),
>> TYPE_VIRTIO_IOMMU_PCI)) {
>> + uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>> + "aw-bits", NULL);
>> /* Declare the APIC range as the reserved MSI region */
>> char *resv_prop_str =
>> g_strdup_printf("0xfee00000:0xfeefffff:%d",
>>
>> VIRTIO_IOMMU_RESV_MEM_T_MSI);
>> @@ -1466,6 +1468,10 @@ static void
>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>> qlist_append_str(reserved_regions, resv_prop_str);
>> qdev_prop_set_array(dev, "reserved-regions",
>> reserved_regions);
>> + if (!aw_bits) {
>> + qdev_prop_set_uint8(dev, "aw-bits", 39);
>
> May be use VTD_HOST_AW_39BIT instead of 39 ? This would make it
> easier to find uses of certain defaults values and would clarify
> that the default AW of virtio-iommu is set as intel-iommu.
OK
Thanks!
Eric
>
> Thanks,
>
> C.
>
>
>
>> + }
>> +
>> g_free(resv_prop_str);
>> }
>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>> index 7870bdbeee..c468e9b13b 100644
>> --- a/hw/virtio/virtio-iommu.c
>> +++ b/hw/virtio/virtio-iommu.c
>> @@ -1529,7 +1529,7 @@ static Property virtio_iommu_properties[] = {
>> DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>> TYPE_PCI_BUS, PCIBus *),
>> DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>> - DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>> + DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 0),
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>
Hi Eric,
>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>pc_q35_9.0 and arm virt
>
>Currently the default input range can extend to 64 bits. On x86,
>when the virtio-iommu protects vfio devices, the physical iommu
>may support only 39 bits. Let's set the default to 39, as done
>for the intel-iommu. On ARM we set 48b as a default (matching
>SMMUv3 SMMU_IDR5.VAX == 0).
>
>We use hw_compat_8_2 to handle the compatibility for machines
>before 9.0 which used to have a virtio-iommu default input range
>of 64 bits.
>
>Of course if aw-bits is set from the command line, the default
>is overriden.
>
>Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
>---
>
>v1 -> v2:
>- set aw-bits to 48b on ARM
>- use hw_compat_8_2 to handle the compat for older machines
> which used 64b as a default
>---
> hw/arm/virt.c | 6 ++++++
> hw/core/machine.c | 5 ++++-
> hw/i386/pc.c | 6 ++++++
> hw/virtio/virtio-iommu.c | 2 +-
> 4 files changed, 17 insertions(+), 2 deletions(-)
>
>diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>index e6ead2c5c8..56539f2fc5 100644
>--- a/hw/arm/virt.c
>+++ b/hw/arm/virt.c
>@@ -2718,10 +2718,16 @@ static void
>virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
> virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev),
>errp);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>+ uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>+ "aw-bits", NULL);
> hwaddr db_start = 0, db_end = 0;
> QList *reserved_regions;
> char *resv_prop_str;
>
>+ if (!aw_bits) {
>+ qdev_prop_set_uint8(dev, "aw-bits", 48);
>+ }
>+
> if (vms->iommu != VIRT_IOMMU_NONE) {
> error_setg(errp, "virt machine does not support multiple IOMMUs");
> return;
>diff --git a/hw/core/machine.c b/hw/core/machine.c
>index fb5afdcae4..70ac96954c 100644
>--- a/hw/core/machine.c
>+++ b/hw/core/machine.c
>@@ -30,9 +30,12 @@
> #include "exec/confidential-guest-support.h"
> #include "hw/virtio/virtio-pci.h"
> #include "hw/virtio/virtio-net.h"
>+#include "hw/virtio/virtio-iommu.h"
> #include "audio/audio.h"
>
>-GlobalProperty hw_compat_8_2[] = {};
>+GlobalProperty hw_compat_8_2[] = {
>+ { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" },
>+};
> const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
>
> GlobalProperty hw_compat_8_1[] = {
>diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>index 803244e5cc..0e2bcb4840 100644
>--- a/hw/i386/pc.c
>+++ b/hw/i386/pc.c
>@@ -1458,6 +1458,8 @@ static void
>pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
> virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev),
>errp);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>+ uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>+ "aw-bits", NULL);
> /* Declare the APIC range as the reserved MSI region */
> char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
> VIRTIO_IOMMU_RESV_MEM_T_MSI);
>@@ -1466,6 +1468,10 @@ static void
>pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> qlist_append_str(reserved_regions, resv_prop_str);
> qdev_prop_set_array(dev, "reserved-regions", reserved_regions);
>
>+ if (!aw_bits) {
>+ qdev_prop_set_uint8(dev, "aw-bits", 39);
>+ }
>+
> g_free(resv_prop_str);
> }
>
>diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>index 7870bdbeee..c468e9b13b 100644
>--- a/hw/virtio/virtio-iommu.c
>+++ b/hw/virtio/virtio-iommu.c
>@@ -1529,7 +1529,7 @@ static Property virtio_iommu_properties[] = {
> DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
> TYPE_PCI_BUS, PCIBus *),
> DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>- DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>+ DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 0),
Not clear if virtio-iommu support other archs besides x86 and arm.
It looks on those archs, aw_bits is default 0 on machine 9.0 above
and will fails the check in realize?
Thanks
Zhenzhong
> DEFINE_PROP_END_OF_LIST(),
> };
>
>--
>2.41.0
Hi Zhenzhong,
On 2/2/24 07:51, Duan, Zhenzhong wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> pc_q35_9.0 and arm virt
>>
>> Currently the default input range can extend to 64 bits. On x86,
>> when the virtio-iommu protects vfio devices, the physical iommu
>> may support only 39 bits. Let's set the default to 39, as done
>> for the intel-iommu. On ARM we set 48b as a default (matching
>> SMMUv3 SMMU_IDR5.VAX == 0).
>>
>> We use hw_compat_8_2 to handle the compatibility for machines
>> before 9.0 which used to have a virtio-iommu default input range
>> of 64 bits.
>>
>> Of course if aw-bits is set from the command line, the default
>> is overriden.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v1 -> v2:
>> - set aw-bits to 48b on ARM
>> - use hw_compat_8_2 to handle the compat for older machines
>> which used 64b as a default
>> ---
>> hw/arm/virt.c | 6 ++++++
>> hw/core/machine.c | 5 ++++-
>> hw/i386/pc.c | 6 ++++++
>> hw/virtio/virtio-iommu.c | 2 +-
>> 4 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index e6ead2c5c8..56539f2fc5 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -2718,10 +2718,16 @@ static void
>> virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>> virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev),
>> errp);
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>> + uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>> + "aw-bits", NULL);
>> hwaddr db_start = 0, db_end = 0;
>> QList *reserved_regions;
>> char *resv_prop_str;
>>
>> + if (!aw_bits) {
>> + qdev_prop_set_uint8(dev, "aw-bits", 48);
>> + }
>> +
>> if (vms->iommu != VIRT_IOMMU_NONE) {
>> error_setg(errp, "virt machine does not support multiple IOMMUs");
>> return;
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index fb5afdcae4..70ac96954c 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -30,9 +30,12 @@
>> #include "exec/confidential-guest-support.h"
>> #include "hw/virtio/virtio-pci.h"
>> #include "hw/virtio/virtio-net.h"
>> +#include "hw/virtio/virtio-iommu.h"
>> #include "audio/audio.h"
>>
>> -GlobalProperty hw_compat_8_2[] = {};
>> +GlobalProperty hw_compat_8_2[] = {
>> + { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" },
>> +};
>> const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
>>
>> GlobalProperty hw_compat_8_1[] = {
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 803244e5cc..0e2bcb4840 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1458,6 +1458,8 @@ static void
>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>> virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev),
>> errp);
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>> + uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>> + "aw-bits", NULL);
>> /* Declare the APIC range as the reserved MSI region */
>> char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
>> VIRTIO_IOMMU_RESV_MEM_T_MSI);
>> @@ -1466,6 +1468,10 @@ static void
>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>> qlist_append_str(reserved_regions, resv_prop_str);
>> qdev_prop_set_array(dev, "reserved-regions", reserved_regions);
>>
>> + if (!aw_bits) {
>> + qdev_prop_set_uint8(dev, "aw-bits", 39);
>> + }
>> +
>> g_free(resv_prop_str);
>> }
>>
>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>> index 7870bdbeee..c468e9b13b 100644
>> --- a/hw/virtio/virtio-iommu.c
>> +++ b/hw/virtio/virtio-iommu.c
>> @@ -1529,7 +1529,7 @@ static Property virtio_iommu_properties[] = {
>> DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>> TYPE_PCI_BUS, PCIBus *),
>> DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>> - DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>> + DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 0),
> Not clear if virtio-iommu support other archs besides x86 and arm.
> It looks on those archs, aw_bits is default 0 on machine 9.0 above
> and will fails the check in realize?
At the moment the virtio-iommu only is supported along with q35 and
arm-virt.
Only those machines set the reserved-regions prop and the aw-bits, which
are both requested for a correct behavior.
Thank you for the review!
Eric
>
> Thanks
> Zhenzhong
>
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>> --
>> 2.41.0
>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: Re: [PATCH v2 3/3] hw: Set virtio-iommu aw-bits default value on
>pc_q35_9.0 and arm virt
>
>Hi Zhenzhong,
>
>On 2/2/24 07:51, Duan, Zhenzhong wrote:
>> Hi Eric,
>>
>>> -----Original Message-----
>>> From: Eric Auger <eric.auger@redhat.com>
>>> pc_q35_9.0 and arm virt
>>>
>>> Currently the default input range can extend to 64 bits. On x86,
>>> when the virtio-iommu protects vfio devices, the physical iommu
>>> may support only 39 bits. Let's set the default to 39, as done
>>> for the intel-iommu. On ARM we set 48b as a default (matching
>>> SMMUv3 SMMU_IDR5.VAX == 0).
>>>
>>> We use hw_compat_8_2 to handle the compatibility for machines
>>> before 9.0 which used to have a virtio-iommu default input range
>>> of 64 bits.
>>>
>>> Of course if aw-bits is set from the command line, the default
>>> is overriden.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>
>>> ---
>>>
>>> v1 -> v2:
>>> - set aw-bits to 48b on ARM
>>> - use hw_compat_8_2 to handle the compat for older machines
>>> which used 64b as a default
>>> ---
>>> hw/arm/virt.c | 6 ++++++
>>> hw/core/machine.c | 5 ++++-
>>> hw/i386/pc.c | 6 ++++++
>>> hw/virtio/virtio-iommu.c | 2 +-
>>> 4 files changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index e6ead2c5c8..56539f2fc5 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -2718,10 +2718,16 @@ static void
>>> virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>>> virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev),
>MACHINE(hotplug_dev),
>>> errp);
>>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI))
>{
>>> + uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>>> + "aw-bits", NULL);
>>> hwaddr db_start = 0, db_end = 0;
>>> QList *reserved_regions;
>>> char *resv_prop_str;
>>>
>>> + if (!aw_bits) {
>>> + qdev_prop_set_uint8(dev, "aw-bits", 48);
>>> + }
>>> +
>>> if (vms->iommu != VIRT_IOMMU_NONE) {
>>> error_setg(errp, "virt machine does not support multiple
>IOMMUs");
>>> return;
>>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>>> index fb5afdcae4..70ac96954c 100644
>>> --- a/hw/core/machine.c
>>> +++ b/hw/core/machine.c
>>> @@ -30,9 +30,12 @@
>>> #include "exec/confidential-guest-support.h"
>>> #include "hw/virtio/virtio-pci.h"
>>> #include "hw/virtio/virtio-net.h"
>>> +#include "hw/virtio/virtio-iommu.h"
>>> #include "audio/audio.h"
>>>
>>> -GlobalProperty hw_compat_8_2[] = {};
>>> +GlobalProperty hw_compat_8_2[] = {
>>> + { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" },
>>> +};
>>> const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
>>>
>>> GlobalProperty hw_compat_8_1[] = {
>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>> index 803244e5cc..0e2bcb4840 100644
>>> --- a/hw/i386/pc.c
>>> +++ b/hw/i386/pc.c
>>> @@ -1458,6 +1458,8 @@ static void
>>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>>> virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev),
>MACHINE(hotplug_dev),
>>> errp);
>>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI))
>{
>>> + uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>>> + "aw-bits", NULL);
>>> /* Declare the APIC range as the reserved MSI region */
>>> char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
>>> VIRTIO_IOMMU_RESV_MEM_T_MSI);
>>> @@ -1466,6 +1468,10 @@ static void
>>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>> qlist_append_str(reserved_regions, resv_prop_str);
>>> qdev_prop_set_array(dev, "reserved-regions", reserved_regions);
>>>
>>> + if (!aw_bits) {
>>> + qdev_prop_set_uint8(dev, "aw-bits", 39);
>>> + }
>>> +
>>> g_free(resv_prop_str);
>>> }
>>>
>>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>>> index 7870bdbeee..c468e9b13b 100644
>>> --- a/hw/virtio/virtio-iommu.c
>>> +++ b/hw/virtio/virtio-iommu.c
>>> @@ -1529,7 +1529,7 @@ static Property virtio_iommu_properties[] = {
>>> DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>>> TYPE_PCI_BUS, PCIBus *),
>>> DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>>> - DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>>> + DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 0),
>> Not clear if virtio-iommu support other archs besides x86 and arm.
>> It looks on those archs, aw_bits is default 0 on machine 9.0 above
>> and will fails the check in realize?
>
>At the moment the virtio-iommu only is supported along with q35 and
>arm-virt.
>Only those machines set the reserved-regions prop and the aw-bits, which
>are both requested for a correct behavior.
Clear, thanks Eric.
Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
BRs.
Zhenzhong
>
>Thank you for the review!
>
>Eric
>
>>
>> Thanks
>> Zhenzhong
>>
>>> DEFINE_PROP_END_OF_LIST(),
>>> };
>>>
>>> --
>>> 2.41.0
© 2016 - 2024 Red Hat, Inc.