Re: [Qemu-devel] [V11 3/4] hw/core: provision for overriding emulated IOMMU

[Peter Xu <peterx@redhat.com>]

On Sun, May 22, 2016 at 01:21:53PM +0300, David Kiarie wrote:
[...]
>  static void machine_set_suppress_vmdesc(Object *obj, bool value, Error **errp)
>  {
>      MachineState *ms = MACHINE(obj);
> @@ -473,10 +492,14 @@ static void machine_initfn(Object *obj)
>                                      "Firmware image",
>                                      NULL);
>      object_property_add_bool(obj, "iommu",
> -                             machine_get_iommu,
> -                             machine_set_iommu, NULL);
> +                             machine_get_iommu, machine_set_iommu, NULL);

Shall we avoid this line-merge change?

>      object_property_set_description(obj, "iommu",
> -                                    "Set on/off to enable/disable Intel IOMMU (VT-d)",
> +                                    "Set on to enable IOMMU emulation",
> +                                    NULL);
> +    object_property_add_str(obj, "x-iommu-type",
> +                            NULL, machine_set_iommu_override, NULL);
> +    object_property_set_description(obj, "x-iommu-type",
> +                                    "Set on to override emulated IOMMU to AMD IOMMU",

Here if we can still configure x-iommu-type to "intel", shall we
change it to something like:

"Configure specific type of IOMMU (intel/amd)"

I believe we just forgot to update this desc after we changed
x-iommu-type from boolean to string.

[...]
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 6106520..81217d3 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -38,7 +38,8 @@ 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=on|off controls emulated IOMMU support(default: off)\n"
> +    "                x-iommu-type=amd|intel overrides emulated IOMMU to AMD IOMMU (default: intel)\n"

Similar to above, how about:

"x-iommu-type=intel|amd configure emulated IOMMU type (default: intel)\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"
> @@ -74,7 +75,9 @@ 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.
> +Enables and disables IOMMU emulation. The default is off.
> +@item x-iommu-type=on|off
> +Overrides emulated IOMMU from AMD IOMMU. By default Intel IOMMU is emulated.

Need to update here with latest codes as well:

"""
@item x-iommu-type=intel|amd
Configure IOMMU type (amd/intel).  By default Intel IOMMU is emulated.
"""

>  @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 fb97307..8886abf 100644
> --- a/util/qemu-config.c
> +++ b/util/qemu-config.c
> @@ -213,8 +213,12 @@ 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_BOOL,
                       ^
Meaningless extra space here?

Thanks,

-- peterx