From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40488) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaQA3-0002ae-4s for qemu-devel@nongnu.org; Tue, 24 Mar 2015 10:50:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YaQ9z-0007FE-LB for qemu-devel@nongnu.org; Tue, 24 Mar 2015 10:50:31 -0400 Received: from smtp02.citrix.com ([66.165.176.63]:22862) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaQ9z-0007ET-2O for qemu-devel@nongnu.org; Tue, 24 Mar 2015 10:50:27 -0400 Message-ID: <1427208618.21742.421.camel@citrix.com> From: Ian Campbell Date: Tue, 24 Mar 2015 14:50:18 +0000 In-Reply-To: <1427073466-16956-3-git-send-email-tiejun.chen@intel.com> References: <1427073466-16956-1-git-send-email-tiejun.chen@intel.com> <1427073466-16956-3-git-send-email-tiejun.chen@intel.com> Content-Type: text/plain; charset="UTF-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [v3][PATCH 2/2] libxl: introduce gfx_passthru_kind List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tiejun Chen Cc: Ian.Jackson@eu.citrix.com, wei.liu2@citrix.com, qemu-devel@nongnu.org, stefano.stabellini@citrix.com, xen-devel@lists.xen.org On Mon, 2015-03-23 at 09:17 +0800, Tiejun Chen wrote: > Although we already have 'gfx_passthru' in b_info, this doesn' suffice ^t > after we want to handle IGD specifically. Now we define a new field of > type, gfx_passthru_kind, to indicate we're trying to pass IGD. Actually > this means we can benefit this to support other specific devices just > by extending gfx_passthru_kind. And then we can cooperate with > gfx_passthru to address IGD cases as follows: > > gfx_passthru = 0 => sets build_info.u.gfx_passthru to false > gfx_passthru = 1 => sets build_info.u.gfx_passthru to true and > build_info.u.gfx_passthru_kind to DEFAULT > gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false true You had it right in the code. > and build_info.u.gfx_passthru_kind to IGD > > Here if gfx_passthru_kind = DEFAULT, we will call > libxl__is_igd_vga_passthru() to check if we're hitting that table to need > to pass that option to qemu. But if gfx_passthru_kind = "igd" we always > force to pass that. > > And "-gfx_passthru" is just introduced to work for qemu-xen-traditional > so we should get this away from libxl__build_device_model_args_new() in > the case of qemu upstream. > > Signed-off-by: Tiejun Chen > --- > docs/man/xl.cfg.pod.5 | 11 +++++++---- > tools/libxl/libxl.h | 6 ++++++ > tools/libxl/libxl_dm.c | 36 +++++++++++++++++++++++++++++++++--- > tools/libxl/libxl_internal.h | 4 ++++ > tools/libxl/libxl_types.idl | 6 ++++++ > tools/libxl/xl_cmdimpl.c | 14 ++++++++++++-- > 6 files changed, 68 insertions(+), 9 deletions(-) > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 > index 408653f..c29bcbf 100644 > --- a/docs/man/xl.cfg.pod.5 > +++ b/docs/man/xl.cfg.pod.5 > @@ -671,7 +671,7 @@ through to this VM. See L above. > devices passed through to this VM. See L > above. > > -=item B > +=item B I think B is more accurate. > Enable graphics device PCI passthrough. This option makes an assigned > PCI graphics card become primary graphics card in the VM. The QEMU > @@ -699,9 +699,12 @@ working graphics passthrough. See the XenVGAPassthroughTestedAdapters > L wiki page > for currently supported graphics cards for gfx_passthru. > > -gfx_passthru is currently only supported with the qemu-xen-traditional > -device-model. Upstream qemu-xen device-model currently does not have > -support for gfx_passthru. > +gfx_passthru is currently supported both with the qemu-xen-traditional > +device-model and upstream qemu-xen device-model. Note with the > +qemu-xen-traditional device-model this option is just treated as BOOLEAN > +actually, but with upstream qemu-xen device-model this option is extended > +to pass a specific device name to force work. Currently just 'igd' is > +defined to support Intel graphics device. How about: When given as a boolean the B option either disables gfx passthru or enables autodetection. When given as a string the B option describes the type of passthru to enable. Valid options are: =over 4 =item B Disables gfx_passthru. =item B, B Enables gfx_passthru and autodetects the type of device which is being used. =item "igd" Enables gfx_passthru of the Intel Graphics Device. =back Note: When used with the qemu-xen-traditional device model only IGD passthru is supported. (do check my pod syntax, I'm mostly making it up!) The note at the end makes me thing that perhaps something ought to check this constraint in the qemu-xen-traditional case. It might be easiest to do it in libxl__build_device_model_args_old using the libxl__detect_gfx_passthru_kind helper you have added e.g. where libxl__build_device_model_args_old has: if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { flexarray_append(dm_args, "-gfx_passthru"); } would become something like: if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { if (libxl__detect_gfx_passthru_kind(gc, guest_config) != LIBXL_GFX_PASSTHRU_KIND_IGD) { LOG("only IGD gfx_passthru is supported with qemu-xen-traditional"); return NULL; } flexarray_append(dm_args, "-gfx_passthru"); } Or something like that. What do you think? > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index c97c62d..26a01cb 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -3632,6 +3632,10 @@ static inline void libxl__update_config_vtpm(libxl__gc *gc, > */ > void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr, > const libxl_bitmap *sptr); > + > +bool libxl__is_igd_vga_passthru(libxl__gc *gc, > + const libxl_domain_config *d_config); This should be in the previous patch. In fact I think it is and this is a redundant second instance. Ian.