Re: [Xen-devel] [PATCH 16 of 16] libxl: add iommu parameter to qemu-dm

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2011-12-14 at 15:29 +0000, Wei Wang wrote:
> # HG changeset patch
> # User Wei Wang <wei.wang2@xxxxxxx>
> # Date 1323876144 -3600
> # Node ID 24f4a0a23da71c58f457f0bf98aa8238dd45332d
> # Parent  93658ca85035d6a4e56e2e6602c02859974d30a4
> libxl: add iommu parameter to qemu-dm.
> When iomm = 0, virtual iommu device will be disabled.
> 
> Signed-off-by: Wei Wang <wei.wang2@xxxxxxx>
> 
> diff -r 93658ca85035 -r 24f4a0a23da7 tools/libxl/libxl_dm.c
> --- a/tools/libxl/libxl_dm.c  Wed Dec 14 16:22:23 2011 +0100
> +++ b/tools/libxl/libxl_dm.c  Wed Dec 14 16:22:24 2011 +0100
> @@ -194,6 +194,9 @@ static char ** libxl__build_device_model
>          if (info->gfx_passthru) {
>              flexarray_append(dm_args, "-gfx_passthru");
>          }
> +        if (info->iommu) {
> +            flexarray_append(dm_args, "-iommu");
> +        }
>      }
>      if (info->saved_state) {
>          flexarray_vappend(dm_args, "-loadvm", info->saved_state, NULL);
> @@ -404,6 +407,9 @@ static char ** libxl__build_device_model
>          if (info->gfx_passthru) {
>              flexarray_append(dm_args, "-gfx_passthru");
>          }
> +        if (info->iommu) {
> +            flexarray_append(dm_args, "-iommu");
> +        }
>      }
>      if (info->saved_state) {
>          /* This file descriptor is meant to be used by QEMU */
> diff -r 93658ca85035 -r 24f4a0a23da7 tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl     Wed Dec 14 16:22:23 2011 +0100
> +++ b/tools/libxl/libxl_types.idl     Wed Dec 14 16:22:24 2011 +0100
> @@ -254,6 +254,7 @@ The password never expires"""),
>      ("extra",            libxl_string_list, False, "extra parameters pass 
> directly to qemu, NULL terminated"),
>      ("extra_pv",         libxl_string_list, False, "extra parameters pass 
> directly to qemu for PV guest, NULL terminated"),
>      ("extra_hvm",        libxl_string_list, False, "extra parameters pass 
> directly to qemu for HVM guest, NULL terminated"),
> +    ("iommu",            bool,              False, "guest iommu enabled or 
> disabled"),
>      ],
>      comment=
>  """Device Model information.
> diff -r 93658ca85035 -r 24f4a0a23da7 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c        Wed Dec 14 16:22:23 2011 +0100
> +++ b/tools/libxl/xl_cmdimpl.c        Wed Dec 14 16:22:24 2011 +0100
> @@ -386,6 +386,7 @@ static void printf_info(int domid,
>          printf("\t\t\t(spicedisable_ticketing %d)\n",
>                      dm_info->spicedisable_ticketing);
>          printf("\t\t\t(spiceagent_mouse %d)\n", dm_info->spiceagent_mouse);
> +        printf("\t\t\t(iommu %d)\n", dm_info->iommu);
>          printf("\t\t)\n");
>          break;
>      case LIBXL_DOMAIN_TYPE_PV:
> @@ -1217,6 +1218,8 @@ skip_vfb:
>          xlu_cfg_replace_string (config, "soundhw", &dm_info->soundhw, 0);
>          if (!xlu_cfg_get_long (config, "xen_platform_pci", &l, 0))
>              dm_info->xen_platform_pci = l;
> +        if (!xlu_cfg_get_long (config, "iommu", &l, 0))
> +            dm_info->iommu = l;

Didn't you already parse this same key into the build_info? Is there
ever a possibility of the dm_info and build_info versions of this field
differing?

Assuming not I think this setting ought to only live in one place and I
think build_info should be that place rather than the dm info. That
might need some refactoring in libxl to pass the right struct down.

Also you have only CC'd the hypervisor maintainers on this (and the
other?) tool stack patch. Please check MAINTAINERS to see who ought to
be CC'd.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel