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

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

On Thursday 15 December 2011 17:59:19 Ian Campbell wrote:
> 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@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.
>
> 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.
Ok, that sounds doable. I will fix that in the next version.
Thanks,
Wei

>
> > > 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@lists.xensource.com
> http://lists.xensource.com/xen-devel