From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind Date: Wed, 18 Mar 2015 15:32:52 +0800 Message-ID: <55092A24.8010409__36764.9707281545$1426664099$gmane$org@intel.com> References: <1425980538-5508-1-git-send-email-tiejun.chen@intel.com> <1425980538-5508-3-git-send-email-tiejun.chen@intel.com> <1426073641.21353.192.camel@citrix.com> <55010596.2030106@intel.com> <1426163186.21353.409.camel@citrix.com> <55023FDA.7030204@intel.com> <1426241479.32572.94.camel@citrix.com> <55062CD2.8050305@intel.com> <1426508400.18247.33.camel@citrix.com> <5507DBD6.8030401@intel.com> <1426584396.18247.136.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1426584396.18247.136.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Ian.Jackson@eu.citrix.com, wei.liu2@citrix.com, qemu-devel@nongnu.org, stefano.stabellini@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org > I think the _libxl_ message needs to be just "Unable to detect graphics > passthru kind". i.e. it can't/shouldn't reference anything to do with xl > config options etc (which would make no sense if libvirt was being > used). > > That's not very user friendly though, so you may want to consider adding > a new specific error code for this case and returning it here, such that > xl or libvirt can then give a more comprehensible error message. But, args = libxl__build_device_model_args(gc, "stubdom-dm", guest_domid, guest_config, d_state, NULL); | + return libxl__build_device_model_args_new(gc, dm,guest_domid, guest_config, state, dm_state_fd); | + Currently we always return "NULL" inside. if (!args) { ret = ERROR_FAIL; goto out; } So I'm not very sure how to pass this new specific error code to xl/libvirt. > >> So looks the whole policy should be something like this, >> >> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c >> index 5c40e84..5518759 100644 >> --- a/tools/libxl/xl_cmdimpl.c >> +++ b/tools/libxl/xl_cmdimpl.c >> @@ -1953,8 +1953,27 @@ 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); >> + } > > Up to here is fine. Thanks but I guess I'd better to paste this whole patch to avoid I'm still missing something :) --- tools/libxl/libxl.h | 6 ++++++ tools/libxl/libxl_dm.c | 25 ++++++++++++++++++++++--- tools/libxl/libxl_internal.h | 5 +++++ tools/libxl/libxl_types.idl | 6 ++++++ tools/libxl/xl_cmdimpl.c | 14 ++++++++++++-- 5 files changed, 51 insertions(+), 5 deletions(-) diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 6bbc52d..62b3ae5 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 8599a6a..045d48a 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -710,9 +710,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"); @@ -757,6 +754,28 @@ 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)) { + switch (b_info->u.hvm.gfx_passthru_kind) { + case LIBXL_GFX_PASSTHRU_KIND_DEFAULT: + if (libxl__is_igd_vga_passthru(gc, guest_config)) { + machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg); + } else { + LOG(ERROR, "Unable to detect graphics passthru kind," + " please set gfx_passthru_kind. See xl.cfg(5) for more" + " information.\n"); + return NULL; + } + break; + case LIBXL_GFX_PASSTHRU_KIND_IGD: + machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg); + break; + default: + LOG(WARN, "gfx_passthru_kind is invalid so ignored.\n"); + break; + } + } + 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_internal.h b/tools/libxl/libxl_internal.h index c97c62d..8912421 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -3632,6 +3632,11 @@ 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); + +#ifdef LIBXL_HAVE_GFX_PASSTHRU_KIND +bool libxl__is_igd_vga_passthru(libxl__gc *gc, + const libxl_domain_config *d_config); +#endif #endif /* 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 5c40e84..3ea3833 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -1953,8 +1953,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); + } switch (xlu_cfg_get_list_as_string_list(config, "serial", &b_info->u.hvm.serial_list, 1)) Thanks Tiejun