[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [v3][PATCH 2/2] libxl: introduce gfx_passthru_kind



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 <tiejun.chen@xxxxxxxxx>
---
 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(-)

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<seize|/"seize_boolean"> above.
 devices passed through to this VM. See L<power_mgt|/"power_mgmt_boolean">
 above.

-=item B<gfx_passthru=BOOLEAN>
+=item B<gfx_passthru=BOOLEAN|"STRING">

 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<http://wiki.xen.org/wiki/XenVGAPassthroughTestedAdapters> 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<gfx_passthru> option either disables gfx
+passthru or enables autodetection.
+
+But when given as a string the B<gfx_passthru> 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<gfx_passthru=0>
+
+Disables graphics device PCI passthrough.
+
+=item B<gfx_passthru=1>, B<gfx_passthru="default">
+
+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);
+        }
         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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.