+
+ if (b_info->u.hvm.gfx_passthru_kind ==
+ LIBXL_GFX_PASSTHRU_KIND_DEFAULT) {
+ if (libxl__is_igd_vga_passthru(gc, guest_config))
+ machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg);
+ } else if (b_info->u.hvm.gfx_passthru_kind ==
+ LIBXL_GFX_PASSTHRU_KIND_IGD) {
+ machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg);
+ } else {
+ LOG(WARN, "gfx_passthru_kind is invalid so ignored.\n");
+ }
I think this whole block should be inside an:
if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)
the semantics should be that kind is ignored unless passthru is enabled.
and then the if/else chain could become a switch perhaps?
Okay.
I'm unsure what we should do if kind==DEFAULT but
libxl__is_igd_vga_passthru fails. At a minimum a warning seems
desirable. I'm not sure if it should warrant a failure or not.
I think a warning message plus abort() should be enough, and then user
can determine what's next,
Please take a look at this,
+
+ 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(WARN, "No supported IGD to passthru,"
+ " or please force set gfx_passthru=\"igd\".\n");
+ abort();
+ }
+ 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_pci.c b/tools/libxl/libxl_pci.c
index fc060c6..9a534cc 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -608,11 +608,11 @@ bool libxl__is_igd_vga_passthru(libxl__gc *gc,
device = fixup_ids[j].device;
if (pt_vendor == vendor && pt_device == device)
- return 1;
+ return true;
}
}
- return 0;
+ return false;
Please fold this into the previous patch. (You may retain the ack I
gave).
Thanks for your catch.
}
/*
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 02be466..d64ad10 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),
Please add a #define LIBXL_HAVE_... to libxl.h to indicate that this
functionality is now available. There is a comment near the top
explaining why etc and a bunch of examples.
Note that the #define is for 3rd party code only, there is no need to
actually use it in either libxl or xl code.
Like this,
@@ -720,6 +720,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst,
libxl_mac *src);
#define LIBXL_HAVE_PSR_MBM 1
#endif
+/*
+ * LIBXL_HAVE_GFX_PASSTHRU_KIND
+ *
+ * If this is defined, the Graphic Device Passthrough Override is
supported.
+ */
+#define LIBXL_HAVE_GFX_PASSTHRU_KIND 1
+
typedef char **libxl_string_list;
void libxl_string_list_dispose(libxl_string_list *sl);
int libxl_string_list_length(const libxl_string_list *sl);
@@ -1501,6 +1508,11 @@ int libxl_psr_cmt_get_sample(libxl_ctx *ctx,
uint64_t *tsc_r);
#endif
+#ifdef LIBXL_HAVE_GFX_PASSTHRU_KIND
+bool libxl__is_igd_vga_passthru(libxl__gc *gc,
+ const libxl_domain_config *d_config);
+#endif
+
/* misc */
/* Each of these sets or clears the flag according to whether the
But looks libxl__gc{} is defined in the libxl_internal.h file... I guess
we need a conversion, or other idea?
("serial", string),
("boot", string),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 440db78..d0d6ce3 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1953,8 +1953,26 @@ 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)) {
+ if (!l) {
+ libxl_defbool_set(&b_info->u.hvm.gfx_passthru, false);
+ } else if (i == 1) {
+ libxl_defbool_set(&b_info->u.hvm.gfx_passthru, true);
+ } else {
+ fprintf(stderr,
+ "ERROR: invalid value %ld for \"gfx_passthru\"\n", l);
+ exit (1);
+ }
The docs (xl.cfg man page) say, regarding booleans "A C<NUMBER>
interpreted as C<False> (C<0>) or C<True> (any other value)."
So just libxl_defbool_set(&b_info->u.hvm.gfx_passthru, l); should be
fine here.
+ } 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, false);
Should be true, I see you wrote false in the commit message so I assume
this was my mistake from earlier as well, sorry about that.
Here I update this chunk of code based on your two comments:
@@ -1953,8 +1953,22 @@ 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)) {
+ if (l) {
+ libxl_defbool_set(&b_info->u.hvm.gfx_passthru, true);
+ } else {
+ libxl_defbool_set(&b_info->u.hvm.gfx_passthru, false);
+ }
+ } 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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel