Re: [Qemu-devel] [PATCH v11 14/17] hw/arm/virt: Introduce the iommu option

[Peter Maydell <peter.maydell@linaro.org>]

On 12 April 2018 at 08:38, Eric Auger <eric.auger@redhat.com> wrote:
> ARM virt machine now exposes a new "iommu" option.
> The SMMUv3 IOMMU is instantiated using -machine virt,iommu=smmuv3.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Prem Mallappa <prem.mallappa@broadcom.com>
>
> ---
> v9 -> v10:
> - remove no_iommu
>
> v7 -> v8:
> - Revert to machine option, now dubbed "iommu", preparing for virtio
>   instantiation.
>
> v5 -> v6: machine 2_11
> ---
>  hw/arm/virt.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 1799702..a3398d6 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1560,6 +1560,34 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp)
>      }
>  }
>
> +static char *virt_get_iommu(Object *obj, Error **errp)
> +{
> +    VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +    switch (vms->iommu) {
> +    case VIRT_IOMMU_NONE:
> +        return g_strdup("none");
> +    case VIRT_IOMMU_SMMUV3:
> +        return g_strdup("smmuv3");
> +    default:
> +        g_assert_not_reached();
> +    }
> +}
> +
> +static void virt_set_iommu(Object *obj, const char *value, Error **errp)
> +{
> +    VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +    if (!strcmp(value, "smmuv3")) {
> +        vms->iommu = VIRT_IOMMU_SMMUV3;
> +    } else if (!strcmp(value, "none")) {
> +        vms->iommu = VIRT_IOMMU_NONE;
> +    } else {
> +        error_setg(errp, "Invalid iommu value");
> +        error_append_hint(errp, "Valid values are none, smmuv3.\n");
> +    }
> +}
> +
>  static CpuInstanceProperties
>  virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
>  {
> @@ -1692,6 +1720,14 @@ static void virt_2_12_instance_init(Object *obj)
>                                          NULL);
>      }
>
> +    /* Default disallows iommu instantiation */
> +    vms->iommu = VIRT_IOMMU_NONE;
> +    object_property_add_str(obj, "iommu", virt_get_iommu, virt_set_iommu, NULL);
> +    object_property_set_description(obj, "iommu",
> +                                    "Set the IOMMU model among "
> +                                    "none, smmuv3 (default none)",
> +                                    NULL);

We should phrase this consistently with how we're describing other
options like the GIC version, so
   "Set the IOMMU type. Valid values are none and smmuv3"

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM