diff mbox series

[v4,3/8] hw/arm/virt: Add a machine option to bypass iommu for primary bus

Message ID 1621914605-14724-4-git-send-email-wangxingang5@huawei.com (mailing list archive)
State New, archived
Headers show
Series IOMMU: Add support for IOMMU Bypass Feature | expand

Commit Message

Wang Xingang May 25, 2021, 3:50 a.m. UTC 
From: Xingang Wang <wangxingang5@huawei.com>

This add a bypass_iommu option for arm virt machine,
the option can be used in this manner:
qemu -machine virt,iommu=smmuv3,bypass_iommu=true

Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
---
 hw/arm/virt.c         | 26 ++++++++++++++++++++++++++
 include/hw/arm/virt.h |  1 +
 2 files changed, 27 insertions(+)

Comments

Eric Auger June 2, 2021, 12:25 p.m. UTC | #1 
Hi Xingang,

On 5/25/21 5:50 AM, Wang Xingang wrote:
> From: Xingang Wang <wangxingang5@huawei.com>
>
> This add a bypass_iommu option for arm virt machine,
> the option can be used in this manner:
> qemu -machine virt,iommu=smmuv3,bypass_iommu=true
This still looks confusing to me. On one hand we say that for the virt
machine the iommu is set to smmuv3 and we say bypass_iommu=true on the
virt machine option line
It is not straightforward that the bypass_iommu only relates to devices
plugged onto the "default" root bus.

At least the name of the property should reflect that I think

> Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
> ---
>  hw/arm/virt.c         | 26 ++++++++++++++++++++++++++
>  include/hw/arm/virt.h |  1 +
>  2 files changed, 27 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 840758666d..49d8a801ed 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1364,6 +1364,7 @@ static void create_pcie(VirtMachineState *vms)
>      }
>  
>      pci = PCI_HOST_BRIDGE(dev);
> +    pci->bypass_iommu = vms->bypass_iommu;
>      vms->bus = pci->bus;
>      if (vms->bus) {
>          for (i = 0; i < nb_nics; i++) {
> @@ -2319,6 +2320,21 @@ static void virt_set_iommu(Object *obj, const char *value, Error **errp)
>      }
>  }
>  
> +static bool virt_get_bypass_iommu(Object *obj, Error **errp)
> +{
> +    VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +    return vms->bypass_iommu;
> +}
> +
> +static void virt_set_bypass_iommu(Object *obj, bool value,
> +                                              Error **errp)
> +{
> +    VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +    vms->bypass_iommu = value;
> +}
> +
>  static CpuInstanceProperties
>  virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
>  {
> @@ -2656,6 +2672,13 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>                                            "Set the IOMMU type. "
>                                            "Valid values are none and smmuv3");
>  
> +    object_class_property_add_bool(oc, "bypass_iommu",
> +                                   virt_get_bypass_iommu,
> +                                   virt_set_bypass_iommu);
> +    object_class_property_set_description(oc, "bypass_iommu",
> +                                          "Set on/off to enable/disable "
> +                                          "bypass_iommu for primary bus");
> +
>      object_class_property_add_bool(oc, "ras", virt_get_ras,
>                                     virt_set_ras);
>      object_class_property_set_description(oc, "ras",
> @@ -2723,6 +2746,9 @@ static void virt_instance_init(Object *obj)
>      /* Default disallows iommu instantiation */
>      vms->iommu = VIRT_IOMMU_NONE;
>  
> +    /* The primary bus is attached to iommu by default */
> +    vms->bypass_iommu = false;
I don't fully master the PCI topology but I think you should clarify
which primary bus we talk about. AFAIU PXB's bus also is a primary bus.

Thanks

Eric
> +
>      /* Default disallows RAS instantiation */
>      vms->ras = false;
>  
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 921416f918..82bceadb82 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -147,6 +147,7 @@ struct VirtMachineState {
>      OnOffAuto acpi;
>      VirtGICType gic_version;
>      VirtIOMMUType iommu;
> +    bool bypass_iommu;
>      VirtMSIControllerType msi_controller;
>      uint16_t virtio_iommu_bdf;
>      struct arm_boot_info bootinfo;
Wang Xingang June 3, 2021, 12:47 p.m. UTC | #2 
Hi Eric,

On 2021/6/2 20:25, Eric Auger wrote:
> Hi Xingang,
> 
> On 5/25/21 5:50 AM, Wang Xingang wrote:
>> From: Xingang Wang <wangxingang5@huawei.com>
>>
>> This add a bypass_iommu option for arm virt machine,
>> the option can be used in this manner:
>> qemu -machine virt,iommu=smmuv3,bypass_iommu=true
> This still looks confusing to me. On one hand we say that for the virt
> machine the iommu is set to smmuv3 and we say bypass_iommu=true on the
> virt machine option line
> It is not straightforward that the bypass_iommu only relates to devices
> plugged onto the "default" root bus.
> 
> At least the name of the property should reflect that I think
> 

Thanks, maybe I should replace the name to "default_rootbus_bypass_iommu".
I just considered that the name might be too long,
anyway, explicitness is important, I will fix this.


>> Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
>> ---
>>   hw/arm/virt.c         | 26 ++++++++++++++++++++++++++
>>   include/hw/arm/virt.h |  1 +
>>   2 files changed, 27 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 840758666d..49d8a801ed 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -1364,6 +1364,7 @@ static void create_pcie(VirtMachineState *vms)
>>       }
>>   
>>       pci = PCI_HOST_BRIDGE(dev);
>> +    pci->bypass_iommu = vms->bypass_iommu;
>>       vms->bus = pci->bus;
>>       if (vms->bus) {
>>           for (i = 0; i < nb_nics; i++) {
>> @@ -2319,6 +2320,21 @@ static void virt_set_iommu(Object *obj, const char *value, Error **errp)
>>       }
>>   }
>>   
>> +static bool virt_get_bypass_iommu(Object *obj, Error **errp)
>> +{
>> +    VirtMachineState *vms = VIRT_MACHINE(obj);
>> +
>> +    return vms->bypass_iommu;
>> +}
>> +
>> +static void virt_set_bypass_iommu(Object *obj, bool value,
>> +                                              Error **errp)
>> +{
>> +    VirtMachineState *vms = VIRT_MACHINE(obj);
>> +
>> +    vms->bypass_iommu = value;
>> +}
>> +
>>   static CpuInstanceProperties
>>   virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
>>   {
>> @@ -2656,6 +2672,13 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>>                                             "Set the IOMMU type. "
>>                                             "Valid values are none and smmuv3");
>>   
>> +    object_class_property_add_bool(oc, "bypass_iommu",
>> +                                   virt_get_bypass_iommu,
>> +                                   virt_set_bypass_iommu);
>> +    object_class_property_set_description(oc, "bypass_iommu",
>> +                                          "Set on/off to enable/disable "
>> +                                          "bypass_iommu for primary bus");
>> +
>>       object_class_property_add_bool(oc, "ras", virt_get_ras,
>>                                      virt_set_ras);
>>       object_class_property_set_description(oc, "ras",
>> @@ -2723,6 +2746,9 @@ static void virt_instance_init(Object *obj)
>>       /* Default disallows iommu instantiation */
>>       vms->iommu = VIRT_IOMMU_NONE;
>>   
>> +    /* The primary bus is attached to iommu by default */
>> +    vms->bypass_iommu = false;
> I don't fully master the PCI topology but I think you should clarify
> which primary bus we talk about. AFAIU PXB's bus also is a primary bus.
> 
> Thanks
> 
> Eric

Thanks, I will make this annotation clearer.

>> +
>>       /* Default disallows RAS instantiation */
>>       vms->ras = false;
>>   
>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
>> index 921416f918..82bceadb82 100644
>> --- a/include/hw/arm/virt.h
>> +++ b/include/hw/arm/virt.h
>> @@ -147,6 +147,7 @@ struct VirtMachineState {
>>       OnOffAuto acpi;
>>       VirtGICType gic_version;
>>       VirtIOMMUType iommu;
>> +    bool bypass_iommu;
>>       VirtMSIControllerType msi_controller;
>>       uint16_t virtio_iommu_bdf;
>>       struct arm_boot_info bootinfo;
> 
> .
> 

Xingang

.
diff mbox series

Patch

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 840758666d..49d8a801ed 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1364,6 +1364,7 @@  static void create_pcie(VirtMachineState *vms)
     }
 
     pci = PCI_HOST_BRIDGE(dev);
+    pci->bypass_iommu = vms->bypass_iommu;
     vms->bus = pci->bus;
     if (vms->bus) {
         for (i = 0; i < nb_nics; i++) {
@@ -2319,6 +2320,21 @@  static void virt_set_iommu(Object *obj, const char *value, Error **errp)
     }
 }
 
+static bool virt_get_bypass_iommu(Object *obj, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    return vms->bypass_iommu;
+}
+
+static void virt_set_bypass_iommu(Object *obj, bool value,
+                                              Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    vms->bypass_iommu = value;
+}
+
 static CpuInstanceProperties
 virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
 {
@@ -2656,6 +2672,13 @@  static void virt_machine_class_init(ObjectClass *oc, void *data)
                                           "Set the IOMMU type. "
                                           "Valid values are none and smmuv3");
 
+    object_class_property_add_bool(oc, "bypass_iommu",
+                                   virt_get_bypass_iommu,
+                                   virt_set_bypass_iommu);
+    object_class_property_set_description(oc, "bypass_iommu",
+                                          "Set on/off to enable/disable "
+                                          "bypass_iommu for primary bus");
+
     object_class_property_add_bool(oc, "ras", virt_get_ras,
                                    virt_set_ras);
     object_class_property_set_description(oc, "ras",
@@ -2723,6 +2746,9 @@  static void virt_instance_init(Object *obj)
     /* Default disallows iommu instantiation */
     vms->iommu = VIRT_IOMMU_NONE;
 
+    /* The primary bus is attached to iommu by default */
+    vms->bypass_iommu = false;
+
     /* Default disallows RAS instantiation */
     vms->ras = false;
 
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 921416f918..82bceadb82 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -147,6 +147,7 @@  struct VirtMachineState {
     OnOffAuto acpi;
     VirtGICType gic_version;
     VirtIOMMUType iommu;
+    bool bypass_iommu;
     VirtMSIControllerType msi_controller;
     uint16_t virtio_iommu_bdf;
     struct arm_boot_info bootinfo;