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

[Wei Wang2 <wei.wang2@amd.com>]

On Thursday 15 December 2011 15:30:44 Ian Campbell wrote:
> On Wed, 2011-12-14 at 15:29 +0000, Wei Wang wrote:
> > # HG changeset patch
> > # User Wei Wang <wei.wang2@amd.com>
> > # 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@amd.com>
> >
> > 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.

Yes, I added a new flag in build_info, which will control hvmloader to build 
IVRS table. And I also need a flag for qemu-dm to decide if virtual iommu 
device should be registered or not.  I just saw other parameters like 
gfx-passthu are attached to dm_info, so I do the same thing for iommu. 
Also I cannot make a reference of libxl_domain_build_info in 
libxl__build_device_model_args.

> 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.
Thanks, I cc'd them to Ian Jackson.

> Ian.

Wei