Re: [Qemu-devel] [V3 2/4] hw/core: Add AMD IO MMU to machine properties

[Marcel Apfelbaum <marcel@redhat.com>]

On 01/14/2016 10:04 AM, David Kiarie wrote:
> Add IO MMU as a string to machine properties which
> is used to control whether and they type of IO MMU
> to emulate
>
> Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
> ---
>   hw/core/machine.c   | 17 +++++++++--------
>   include/hw/boards.h |  3 ++-
>   qemu-options.hx     |  6 +++---
>   util/qemu-config.c  |  4 ++--
>   4 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index c46ddc7..cb309aa 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -283,18 +283,19 @@ static void machine_set_firmware(Object *obj, const char *value, Error **errp)
>       ms->firmware = g_strdup(value);
>   }
>
> -static bool machine_get_iommu(Object *obj, Error **errp)
> +static char *machine_get_iommu(Object *obj, Error **errp)
>   {
>       MachineState *ms = MACHINE(obj);
>
> -    return ms->iommu;
> +    return g_strdup(ms->iommu);
>   }
>
> -static void machine_set_iommu(Object *obj, bool value, Error **errp)
> +static void machine_set_iommu(Object *obj, const char *value, Error **errp)
>   {
>       MachineState *ms = MACHINE(obj);
>
> -    ms->iommu = value;
> +    g_free(ms->iommu);
> +    ms->iommu = g_strdup(value);

Hi,

The patch is looking good, the only thing I am missing
is dealing with incorrect input. We should accept only
"intel" or "amd".

Thanks,
Marcel

>   }
>
>   static void machine_set_suppress_vmdesc(Object *obj, bool value, Error **errp)
> @@ -454,11 +455,10 @@ static void machine_initfn(Object *obj)
>       object_property_set_description(obj, "firmware",
>                                       "Firmware image",
>                                       NULL);
> -    object_property_add_bool(obj, "iommu",
> -                             machine_get_iommu,
> -                             machine_set_iommu, NULL);
> +    object_property_add_str(obj, "iommu",
> +                            machine_get_iommu, machine_set_iommu, NULL);
>       object_property_set_description(obj, "iommu",
> -                                    "Set on/off to enable/disable Intel IOMMU (VT-d)",
> +                                    "IOMMU list",
>                                       NULL);
>       object_property_add_bool(obj, "suppress-vmdesc",
>                                machine_get_suppress_vmdesc,
> @@ -484,6 +484,7 @@ static void machine_finalize(Object *obj)
>       g_free(ms->dumpdtb);
>       g_free(ms->dt_compatible);
>       g_free(ms->firmware);
> +    g_free(ms->iommu);
>   }
>
>   bool machine_usb(MachineState *machine)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 0f30959..b119245 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -36,6 +36,7 @@ bool machine_usb(MachineState *machine);
>   bool machine_kernel_irqchip_allowed(MachineState *machine);
>   bool machine_kernel_irqchip_required(MachineState *machine);
>   bool machine_kernel_irqchip_split(MachineState *machine);
> +bool machine_amd_iommu(MachineState *machine);
>   int machine_kvm_shadow_mem(MachineState *machine);
>   int machine_phandle_start(MachineState *machine);
>   bool machine_dump_guest_core(MachineState *machine);
> @@ -126,7 +127,7 @@ struct MachineState {
>       bool usb_disabled;
>       bool igd_gfx_passthru;
>       char *firmware;
> -    bool iommu;
> +    char *iommu;
>       bool suppress_vmdesc;
>
>       ram_addr_t ram_size;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 215d00d..ac327c8 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -38,7 +38,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>       "                kvm_shadow_mem=size of KVM shadow MMU\n"
>       "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
>       "                mem-merge=on|off controls memory merge support (default: on)\n"
> -    "                iommu=on|off controls emulated Intel IOMMU (VT-d) support (default=off)\n"
> +    "                iommu=amd|intel enables and selects the emulated IO MMU (default: off)\n"
>       "                igd-passthru=on|off controls IGD GFX passthrough support (default=off)\n"
>       "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
>       "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
> @@ -72,8 +72,8 @@ Include guest memory in a core dump. The default is on.
>   Enables or disables memory merge support. This feature, when supported by
>   the host, de-duplicates identical memory pages among VMs instances
>   (enabled by default).
> -@item iommu=on|off
> -Enables or disables emulated Intel IOMMU (VT-d) support. The default is off.
> +@item iommu=intel|amd
> +Enables and selects the emulated IO MMU. The default is off.
>   @item aes-key-wrap=on|off
>   Enables or disables AES key wrapping support on s390-ccw hosts. This feature
>   controls whether AES wrapping keys will be created to allow
> diff --git a/util/qemu-config.c b/util/qemu-config.c
> index 687fd34..f79b98c 100644
> --- a/util/qemu-config.c
> +++ b/util/qemu-config.c
> @@ -213,8 +213,8 @@ static QemuOptsList machine_opts = {
>               .help = "firmware image",
>           },{
>               .name = "iommu",
> -            .type = QEMU_OPT_BOOL,
> -            .help = "Set on/off to enable/disable Intel IOMMU (VT-d)",
> +            .type =  QEMU_OPT_STRING,
> +            .help = "Enables IO MMU and sets the emulated type",
>           },{
>               .name = "suppress-vmdesc",
>               .type = QEMU_OPT_BOOL,
>