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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2011-12-15 at 14:52 +0000, Wei Wang2 wrote:
> 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@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.
> 
> 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. 

gfx-passthru is only in dm_info

> Also I cannot make a reference of libxl_domain_build_info in 
> libxl__build_device_model_args.

You just need to plumb the variable through. This is apparently just the
first time we have need of it.

> > 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
> 



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