From patchwork Wed Apr 1 01:05:09 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tiejun Chen X-Patchwork-Id: 457055 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id BA50314018C for ; Wed, 1 Apr 2015 12:05:52 +1100 (AEDT) Received: from localhost ([::1]:41114 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yd76M-0007lT-1g for incoming@patchwork.ozlabs.org; Tue, 31 Mar 2015 21:05:50 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40410) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yd75s-0007KK-8Y for qemu-devel@nongnu.org; Tue, 31 Mar 2015 21:05:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yd75o-0005le-5O for qemu-devel@nongnu.org; Tue, 31 Mar 2015 21:05:20 -0400 Received: from mga14.intel.com ([192.55.52.115]:40884) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yd75n-0005g9-QW for qemu-devel@nongnu.org; Tue, 31 Mar 2015 21:05:16 -0400 Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga103.fm.intel.com with ESMTP; 31 Mar 2015 18:05:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,503,1422950400"; d="scan'208";a="549249619" Received: from tiejunch-mobl.ccr.corp.intel.com (HELO [10.238.130.124]) ([10.238.130.124]) by orsmga003.jf.intel.com with ESMTP; 31 Mar 2015 18:05:10 -0700 Message-ID: <551B4445.9070007@intel.com> Date: Wed, 01 Apr 2015 09:05:09 +0800 From: "Chen, Tiejun" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Ian Campbell References: <1427073466-16956-1-git-send-email-tiejun.chen@intel.com> <1427073466-16956-3-git-send-email-tiejun.chen@intel.com> <1427208618.21742.421.camel@citrix.com> <55120B1C.5080004@intel.com> <1427279543.10784.53.camel@citrix.com> <551358A7.2090607@intel.com> <1427364418.10784.122.camel@citrix.com> <5514B281.1050301@intel.com> <1427450046.13935.90.camel@citrix.com> <5518A6B8.2050407@intel.com> <1427707150.13935.235.camel@citrix.com> In-Reply-To: <1427707150.13935.235.camel@citrix.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 192.55.52.115 Cc: Ian.Jackson@eu.citrix.com, wei.liu2@citrix.com, qemu-devel@nongnu.org, stefano.stabellini@citrix.com, xen-devel@lists.xen.org Subject: Re: [Qemu-devel] [v3][PATCH 2/2] libxl: introduce gfx_passthru_kind X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 2015/3/30 17:19, Ian Campbell wrote: > On Mon, 2015-03-30 at 09:28 +0800, Chen, Tiejun wrote: >> Sounds it should be a legacy fix to qemu-xen-tranditional :) So lets do >> it now, >> >> @@ -326,6 +326,10 @@ static char ** >> libxl__build_device_model_args_old(libxl__gc *gc, >> } >> if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { >> flexarray_append(dm_args, "-gfx_passthru"); >> + if (b_info->u.hvm.gfx_passthru_kind > >> + LIBXL_GFX_PASSTHRU_KIND_IGD) >> + LOG(ERROR, "unsupported device type for >> \"gfx_passthru\".\n"); >> + return NULL; > > I'd rather not encode any ordering constraints if we don't have to. I > think this is preferable: > > if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { > switch (b_info->u.hvm.gfx_passthru_kind) { > case LIBXL_GFX_PASSTHRU_KIND_DEFAULT: > case LIBXL_GFX_PASSTHRU_KIND_IGD: > flexarray_append(dm_args, "-gfx_passthru"); > break; > default: > LOG(ERROR, "unsupported gfx_passthru_kind.\n"); > return NULL; > } > } > > (notice that the error message above doesn't refer to the xl specific > option naming). > Sorry for this delay response. This looks reasonable and I regenerate this patch based on this comment: libxl: introduce gfx_passthru_kind Although we already have 'gfx_passthru' in b_info, this doesn't suffice 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 true 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 now "gfx_passthru" is supported both with the qemu-xen-traditional device-model and upstream qemu-xen device-model. But when given as a string this option describes the type of device to enable. Note this behavior is only supported with the upstream qemu-xen device-model. Signed-off-by: Tiejun Chen --- docs/man/xl.cfg.pod.5 | 34 +++++++++++++++++++++++++++++---- tools/libxl/libxl.h | 6 ++++++ tools/libxl/libxl_dm.c | 46 +++++++++++++++++++++++++++++++++++++++++---- tools/libxl/libxl_types.idl | 6 ++++++ tools/libxl/xl_cmdimpl.c | 14 ++++++++++++-- 5 files changed, 96 insertions(+), 10 deletions(-) switch (xlu_cfg_get_list_as_string_list(config, "serial", &b_info->u.hvm.serial_list, 1)) Thanks Tiejun diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 index 408653f..dfde92d 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 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,35 @@ 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. + +When given as a boolean the B option either disables gfx +passthru or enables autodetection. + +But when given as a string the B option describes the type +of device to enable. Note this behavior is only supported with the upstream +qemu-xen device-model. + +Currently, valid options are: + +=over 4 + +=item B + +Disables graphics device PCI passthrough. + +=item B, B + +Enables graphics device PCI passthrough and autodetects the type of device +which is being used. + +=item "igd" + +Enables graphics device PCI passthrough but forcing the type +of device to Intel Graphics Device. + +=back Note that some graphics adapters (AMD/ATI cards, for example) do not necessarily require gfx_passthru option, so you can use the normal Xen diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 5eec092..1144c5e 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -720,6 +720,12 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src); #define LIBXL_HAVE_PSR_MBM 1 #endif +/* + * libxl_domain_build_info has the u.hvm.gfx_passthru_kind field and + * the libxl_gfx_passthru_kind enumeration is defined. +*/ +#define LIBXL_HAVE_GFX_PASSTHRU_KIND + typedef char **libxl_string_list; void libxl_string_list_dispose(libxl_string_list *sl); int libxl_string_list_length(const libxl_string_list *sl); diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index a8b08f2..4fd6310 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -325,7 +325,15 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc, flexarray_vappend(dm_args, "-net", "none", NULL); } if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { - flexarray_append(dm_args, "-gfx_passthru"); + switch (b_info->u.hvm.gfx_passthru_kind) { + case LIBXL_GFX_PASSTHRU_KIND_DEFAULT: + case LIBXL_GFX_PASSTHRU_KIND_IGD: + flexarray_append(dm_args, "-gfx_passthru"); + break; + default: + LOG(ERROR, "unsupported gfx_passthru_kind.\n"); + return NULL; + } } } else { if (!sdl && !vnc) @@ -416,6 +424,22 @@ static char *dm_spice_options(libxl__gc *gc, return opt; } +static enum libxl_gfx_passthru_kind +libxl__detect_gfx_passthru_kind(libxl__gc *gc, + const libxl_domain_config *guest_config) +{ + const libxl_domain_build_info *b_info = &guest_config->b_info; + + if (b_info->u.hvm.gfx_passthru_kind != LIBXL_GFX_PASSTHRU_KIND_DEFAULT) + return b_info->u.hvm.gfx_passthru_kind; + + if (libxl__is_igd_vga_passthru(gc, guest_config)) { + return LIBXL_GFX_PASSTHRU_KIND_IGD; + } + + return LIBXL_GFX_PASSTHRU_KIND_DEFAULT; +} + static char ** libxl__build_device_model_args_new(libxl__gc *gc, const char *dm, int guest_domid, const libxl_domain_config *guest_config, @@ -727,9 +751,6 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, flexarray_append(dm_args, "-net"); flexarray_append(dm_args, "none"); } - if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { - flexarray_append(dm_args, "-gfx_passthru"); - } } else { if (!sdl && !vnc) { flexarray_append(dm_args, "-nographic"); @@ -774,6 +795,23 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, machinearg, max_ram_below_4g); } } + + if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { + enum libxl_gfx_passthru_kind gfx_passthru_kind = + libxl__detect_gfx_passthru_kind(gc, guest_config); + switch (gfx_passthru_kind) { + case LIBXL_GFX_PASSTHRU_KIND_IGD: + machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg); + break; + case LIBXL_GFX_PASSTHRU_KIND_DEFAULT: + LOG(ERROR, "unable to detect required gfx_passthru_kind"); + return NULL; + default: + LOG(ERROR, "invalid value for gfx_passthru_kind"); + return NULL; + } + } + flexarray_append(dm_args, machinearg); for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++) flexarray_append(dm_args, b_info->extra_hvm[i]); diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 47af340..76642a8 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -140,6 +140,11 @@ libxl_tsc_mode = Enumeration("tsc_mode", [ (3, "native_paravirt"), ]) +libxl_gfx_passthru_kind = Enumeration("gfx_passthru_kind", [ + (0, "default"), + (1, "igd"), + ]) + # Consistent with the values defined for HVM_PARAM_TIMER_MODE. libxl_timer_mode = Enumeration("timer_mode", [ (-1, "unknown"), @@ -430,6 +435,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("spice", libxl_spice_info), ("gfx_passthru", libxl_defbool), + ("gfx_passthru_kind", libxl_gfx_passthru_kind), ("serial", string), ("boot", string), diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 04faf98..b78b4ec 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -1956,8 +1956,18 @@ skip_vfb: xlu_cfg_replace_string (config, "spice_streaming_video", &b_info->u.hvm.spice.streaming_video, 0); xlu_cfg_get_defbool(config, "nographic", &b_info->u.hvm.nographic, 0); - xlu_cfg_get_defbool(config, "gfx_passthru", - &b_info->u.hvm.gfx_passthru, 0); + if (!xlu_cfg_get_long(config, "gfx_passthru", &l, 1)) { + libxl_defbool_set(&b_info->u.hvm.gfx_passthru, l); + } else if (!xlu_cfg_get_string(config, "gfx_passthru", &buf, 0)) { + if (libxl_gfx_passthru_kind_from_string(buf, + &b_info->u.hvm.gfx_passthru_kind)) { + fprintf(stderr, + "ERROR: invalid value \"%s\" for \"gfx_passthru\"\n", + buf); + exit (1); + } + libxl_defbool_set(&b_info->u.hvm.gfx_passthru, true); + }