Hi Zhenzhong, On 2/2/24 07:51, Duan, Zhenzhong wrote: > Hi Eric, > >> -----Original Message----- >> From: Eric Auger <eric.au...@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.au...@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