| Message ID | 20240201163324.564525-4-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | VIRTIO-IOMMU: Introduce an aw-bits option | expand |
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
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. > 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(), >> }; >> >
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(), };
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(-)