* [Qemu-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream
@ 2015-03-06 9:08 Tiejun Chen
2015-03-06 9:08 ` [Qemu-devel] [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru Tiejun Chen
` (6 more replies)
0 siblings, 7 replies; 37+ messages in thread
From: Tiejun Chen @ 2015-03-06 9:08 UTC (permalink / raw)
To: ian.campbell, wei.liu2, Ian.Jackson, stefano.stabellini
Cc: qemu-devel, xen-devel
When we're working to support IGD GFX passthrough with qemu
upstream, instead of "-gfx_passthru" we'd like to make that
a machine option, "-machine xxx,igd-passthru=on". This need
to bring a change on tool side.
After a discussion with Campbell, we'd like to construct a table to record
all IGD devices we can support. If we hit that table, we should pass that
option. And so we also introduce a new field of type, 'gfx_passthru_kind',
to cooperate with 'gfx_passthru' to cover all scenarios like this,
gfx_passthru = 0 => sets build_info.u.gfx_passthru to false
gfx_passthru = 1 => sets build_info.u.gfx_passthru to false and
build_info.u.gfx_passthru_kind to DEFAULT
gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
and build_info.u.gfx_passthru_kind to IGD
----------------------------------------------------------------
Tiejun Chen (2):
libxl: introduce libxl__is_igd_vga_passthru
libxl: introduce gfx_passthru_kind
tools/libxl/libxl_dm.c | 13 +++++++++++++
tools/libxl/libxl_internal.h | 2 ++
tools/libxl/libxl_pci.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_types.idl | 6 ++++++
tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
5 files changed, 162 insertions(+), 2 deletions(-)
Thanks
Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru
2015-03-06 9:08 [Qemu-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream Tiejun Chen
@ 2015-03-06 9:08 ` Tiejun Chen
2015-03-06 12:40 ` Wei Liu
2015-03-06 9:08 ` Tiejun Chen
` (5 subsequent siblings)
6 siblings, 1 reply; 37+ messages in thread
From: Tiejun Chen @ 2015-03-06 9:08 UTC (permalink / raw)
To: ian.campbell, wei.liu2, Ian.Jackson, stefano.stabellini
Cc: qemu-devel, xen-devel
While working with qemu, IGD is a specific device in the case of pass through
so we need to identify that to handle more later. Here we define a table to
record all IGD types currently we can support. Also we need to introduce two
helper functions to get vendor and device ids to lookup that table.
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
tools/libxl/libxl_internal.h | 2 +
tools/libxl/libxl_pci.c | 124 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 126 insertions(+)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 934465a..8b952b8 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1176,6 +1176,8 @@ _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pc
_hidden int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
libxl_device_pci *pcidev, int num);
_hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
+_hidden int libxl__is_igd_vga_passthru(libxl__gc *gc,
+ const libxl_domain_config *d_config);
/*----- xswait: wait for a xenstore node to be suitable -----*/
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index f3ae132..dc5a89e 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -491,6 +491,130 @@ static int sysfs_dev_unbind(libxl__gc *gc, libxl_device_pci *pcidev,
return 0;
}
+static unsigned long sysfs_dev_get_vendor(libxl__gc *gc,
+ libxl_device_pci *pcidev)
+{
+ char *pci_device_vendor_path =
+ libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/vendor",
+ pcidev->domain, pcidev->bus, pcidev->dev,
+ pcidev->func);
+ int read_items;
+ unsigned long pci_device_vendor;
+
+ FILE *f = fopen(pci_device_vendor_path, "r");
+ if (!f) {
+ LOGE(ERROR,
+ "pci device "PCI_BDF" does not have vendor attribute",
+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+ return 0xffff;
+ }
+ read_items = fscanf(f, "0x%lx\n", &pci_device_vendor);
+ fclose(f);
+ if (read_items != 1) {
+ LOGE(ERROR,
+ "cannot read vendor of pci device "PCI_BDF,
+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+ return 0xffff;
+ }
+
+ return pci_device_vendor;
+}
+
+static unsigned long sysfs_dev_get_device(libxl__gc *gc,
+ libxl_device_pci *pcidev)
+{
+ char *pci_device_device_path =
+ libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/device",
+ pcidev->domain, pcidev->bus, pcidev->dev,
+ pcidev->func);
+ int read_items;
+ unsigned long pci_device_device;
+
+ FILE *f = fopen(pci_device_device_path, "r");
+ if (!f) {
+ LOGE(ERROR,
+ "pci device "PCI_BDF" does not have device attribute",
+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+ return 0xffff;
+ }
+ read_items = fscanf(f, "0x%lx\n", &pci_device_device);
+ fclose(f);
+ if (read_items != 1) {
+ LOGE(ERROR,
+ "cannot read device of pci device "PCI_BDF,
+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+ return 0xffff;
+ }
+
+ return pci_device_device;
+}
+
+typedef struct {
+ uint16_t vendor;
+ uint16_t device;
+} pci_info;
+
+static const pci_info fixup_ids[] = {
+ /* Intel HSW Classic */
+ {0x8086, 0x0402}, /* HSWGT1D, HSWD_w7 */
+ {0x8086, 0x0406}, /* HSWGT1M, HSWM_w7 */
+ {0x8086, 0x0412}, /* HSWGT2D, HSWD_w7 */
+ {0x8086, 0x0416}, /* HSWGT2M, HSWM_w7 */
+ {0x8086, 0x041E}, /* HSWGT15D, HSWD_w7 */
+ /* Intel HSW ULT */
+ {0x8086, 0x0A06}, /* HSWGT1UT, HSWM_w7 */
+ {0x8086, 0x0A16}, /* HSWGT2UT, HSWM_w7 */
+ {0x8086, 0x0A26}, /* HSWGT3UT, HSWM_w7 */
+ {0x8086, 0x0A2E}, /* HSWGT3UT28W, HSWM_w7 */
+ {0x8086, 0x0A1E}, /* HSWGT2UX, HSWM_w7 */
+ {0x8086, 0x0A0E}, /* HSWGT1ULX, HSWM_w7 */
+ /* Intel HSW CRW */
+ {0x8086, 0x0D26}, /* HSWGT3CW, HSWM_w7 */
+ {0x8086, 0x0D22}, /* HSWGT3CWDT, HSWD_w7 */
+ /* Intel HSW Server */
+ {0x8086, 0x041A}, /* HSWSVGT2, HSWD_w7 */
+ /* Intel HSW SRVR */
+ {0x8086, 0x040A}, /* HSWSVGT1, HSWD_w7 */
+ /* Intel BSW */
+ {0x8086, 0x1606}, /* BDWULTGT1, BDWM_w7 */
+ {0x8086, 0x1616}, /* BDWULTGT2, BDWM_w7 */
+ {0x8086, 0x1626}, /* BDWULTGT3, BDWM_w7 */
+ {0x8086, 0x160E}, /* BDWULXGT1, BDWM_w7 */
+ {0x8086, 0x161E}, /* BDWULXGT2, BDWM_w7 */
+ {0x8086, 0x1602}, /* BDWHALOGT1, BDWM_w7 */
+ {0x8086, 0x1612}, /* BDWHALOGT2, BDWM_w7 */
+ {0x8086, 0x1622}, /* BDWHALOGT3, BDWM_w7 */
+ {0x8086, 0x162B}, /* BDWHALO28W, BDWM_w7 */
+ {0x8086, 0x162A}, /* BDWGT3WRKS, BDWM_w7 */
+ {0x8086, 0x162D}, /* BDWGT3SRVR, BDWM_w7 */
+};
+
+/*
+ * Some devices may need some ways to work well. Here like IGD,
+ * we have to pass a specific option to qemu.
+ */
+int libxl__is_igd_vga_passthru(libxl__gc *gc,
+ const libxl_domain_config *d_config)
+{
+ unsigned int i, j, num = ARRAY_SIZE(fixup_ids);
+ uint16_t vendor, device;
+
+ for (i = 0 ; i < d_config->num_pcidevs ; i++) {
+ libxl_device_pci *pcidev = &d_config->pcidevs[i];
+
+ for (j = 0 ; j < num ; j++) {
+ vendor = fixup_ids[j].vendor;
+ device = fixup_ids[j].device;
+
+ if (sysfs_dev_get_vendor(gc, pcidev) == vendor &&
+ sysfs_dev_get_device(gc, pcidev) == device)
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
/*
* A brief comment about slots. I don't know what slots are for; however,
* I have by experimentation determined:
--
1.9.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru
2015-03-06 9:08 [Qemu-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream Tiejun Chen
2015-03-06 9:08 ` [Qemu-devel] [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru Tiejun Chen
@ 2015-03-06 9:08 ` Tiejun Chen
2015-03-06 9:08 ` [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind Tiejun Chen
` (4 subsequent siblings)
6 siblings, 0 replies; 37+ messages in thread
From: Tiejun Chen @ 2015-03-06 9:08 UTC (permalink / raw)
To: ian.campbell, wei.liu2, Ian.Jackson, stefano.stabellini
Cc: qemu-devel, xen-devel
While working with qemu, IGD is a specific device in the case of pass through
so we need to identify that to handle more later. Here we define a table to
record all IGD types currently we can support. Also we need to introduce two
helper functions to get vendor and device ids to lookup that table.
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
tools/libxl/libxl_internal.h | 2 +
tools/libxl/libxl_pci.c | 124 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 126 insertions(+)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 934465a..8b952b8 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1176,6 +1176,8 @@ _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pc
_hidden int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
libxl_device_pci *pcidev, int num);
_hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
+_hidden int libxl__is_igd_vga_passthru(libxl__gc *gc,
+ const libxl_domain_config *d_config);
/*----- xswait: wait for a xenstore node to be suitable -----*/
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index f3ae132..dc5a89e 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -491,6 +491,130 @@ static int sysfs_dev_unbind(libxl__gc *gc, libxl_device_pci *pcidev,
return 0;
}
+static unsigned long sysfs_dev_get_vendor(libxl__gc *gc,
+ libxl_device_pci *pcidev)
+{
+ char *pci_device_vendor_path =
+ libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/vendor",
+ pcidev->domain, pcidev->bus, pcidev->dev,
+ pcidev->func);
+ int read_items;
+ unsigned long pci_device_vendor;
+
+ FILE *f = fopen(pci_device_vendor_path, "r");
+ if (!f) {
+ LOGE(ERROR,
+ "pci device "PCI_BDF" does not have vendor attribute",
+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+ return 0xffff;
+ }
+ read_items = fscanf(f, "0x%lx\n", &pci_device_vendor);
+ fclose(f);
+ if (read_items != 1) {
+ LOGE(ERROR,
+ "cannot read vendor of pci device "PCI_BDF,
+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+ return 0xffff;
+ }
+
+ return pci_device_vendor;
+}
+
+static unsigned long sysfs_dev_get_device(libxl__gc *gc,
+ libxl_device_pci *pcidev)
+{
+ char *pci_device_device_path =
+ libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/device",
+ pcidev->domain, pcidev->bus, pcidev->dev,
+ pcidev->func);
+ int read_items;
+ unsigned long pci_device_device;
+
+ FILE *f = fopen(pci_device_device_path, "r");
+ if (!f) {
+ LOGE(ERROR,
+ "pci device "PCI_BDF" does not have device attribute",
+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+ return 0xffff;
+ }
+ read_items = fscanf(f, "0x%lx\n", &pci_device_device);
+ fclose(f);
+ if (read_items != 1) {
+ LOGE(ERROR,
+ "cannot read device of pci device "PCI_BDF,
+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+ return 0xffff;
+ }
+
+ return pci_device_device;
+}
+
+typedef struct {
+ uint16_t vendor;
+ uint16_t device;
+} pci_info;
+
+static const pci_info fixup_ids[] = {
+ /* Intel HSW Classic */
+ {0x8086, 0x0402}, /* HSWGT1D, HSWD_w7 */
+ {0x8086, 0x0406}, /* HSWGT1M, HSWM_w7 */
+ {0x8086, 0x0412}, /* HSWGT2D, HSWD_w7 */
+ {0x8086, 0x0416}, /* HSWGT2M, HSWM_w7 */
+ {0x8086, 0x041E}, /* HSWGT15D, HSWD_w7 */
+ /* Intel HSW ULT */
+ {0x8086, 0x0A06}, /* HSWGT1UT, HSWM_w7 */
+ {0x8086, 0x0A16}, /* HSWGT2UT, HSWM_w7 */
+ {0x8086, 0x0A26}, /* HSWGT3UT, HSWM_w7 */
+ {0x8086, 0x0A2E}, /* HSWGT3UT28W, HSWM_w7 */
+ {0x8086, 0x0A1E}, /* HSWGT2UX, HSWM_w7 */
+ {0x8086, 0x0A0E}, /* HSWGT1ULX, HSWM_w7 */
+ /* Intel HSW CRW */
+ {0x8086, 0x0D26}, /* HSWGT3CW, HSWM_w7 */
+ {0x8086, 0x0D22}, /* HSWGT3CWDT, HSWD_w7 */
+ /* Intel HSW Server */
+ {0x8086, 0x041A}, /* HSWSVGT2, HSWD_w7 */
+ /* Intel HSW SRVR */
+ {0x8086, 0x040A}, /* HSWSVGT1, HSWD_w7 */
+ /* Intel BSW */
+ {0x8086, 0x1606}, /* BDWULTGT1, BDWM_w7 */
+ {0x8086, 0x1616}, /* BDWULTGT2, BDWM_w7 */
+ {0x8086, 0x1626}, /* BDWULTGT3, BDWM_w7 */
+ {0x8086, 0x160E}, /* BDWULXGT1, BDWM_w7 */
+ {0x8086, 0x161E}, /* BDWULXGT2, BDWM_w7 */
+ {0x8086, 0x1602}, /* BDWHALOGT1, BDWM_w7 */
+ {0x8086, 0x1612}, /* BDWHALOGT2, BDWM_w7 */
+ {0x8086, 0x1622}, /* BDWHALOGT3, BDWM_w7 */
+ {0x8086, 0x162B}, /* BDWHALO28W, BDWM_w7 */
+ {0x8086, 0x162A}, /* BDWGT3WRKS, BDWM_w7 */
+ {0x8086, 0x162D}, /* BDWGT3SRVR, BDWM_w7 */
+};
+
+/*
+ * Some devices may need some ways to work well. Here like IGD,
+ * we have to pass a specific option to qemu.
+ */
+int libxl__is_igd_vga_passthru(libxl__gc *gc,
+ const libxl_domain_config *d_config)
+{
+ unsigned int i, j, num = ARRAY_SIZE(fixup_ids);
+ uint16_t vendor, device;
+
+ for (i = 0 ; i < d_config->num_pcidevs ; i++) {
+ libxl_device_pci *pcidev = &d_config->pcidevs[i];
+
+ for (j = 0 ; j < num ; j++) {
+ vendor = fixup_ids[j].vendor;
+ device = fixup_ids[j].device;
+
+ if (sysfs_dev_get_vendor(gc, pcidev) == vendor &&
+ sysfs_dev_get_device(gc, pcidev) == device)
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
/*
* A brief comment about slots. I don't know what slots are for; however,
* I have by experimentation determined:
--
1.9.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-06 9:08 [Qemu-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream Tiejun Chen
2015-03-06 9:08 ` [Qemu-devel] [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru Tiejun Chen
2015-03-06 9:08 ` Tiejun Chen
@ 2015-03-06 9:08 ` Tiejun Chen
2015-03-06 9:18 ` Chen, Tiejun
` (3 more replies)
2015-03-06 9:08 ` Tiejun Chen
` (3 subsequent siblings)
6 siblings, 4 replies; 37+ messages in thread
From: Tiejun Chen @ 2015-03-06 9:08 UTC (permalink / raw)
To: ian.campbell, wei.liu2, Ian.Jackson, stefano.stabellini
Cc: qemu-devel, xen-devel
Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
build_info.u.gfx_passthru_kind to DEFAULT
gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
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.
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
tools/libxl/libxl_dm.c | 13 +++++++++++++
tools/libxl/libxl_types.idl | 6 ++++++
tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
3 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 8599a6a..780dd2a 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -757,6 +757,19 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
machinearg, max_ram_below_4g);
}
}
+
+ 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");
+ }
+
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 02be466..e209460 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"),
+ ], init_val = "LIBXL_GFX_PASSTHRU_KIND_DEFAULT")
+
# 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 440db78..3f7fede 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1953,8 +1953,23 @@ 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);
+ }
+ } 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);
+ }
switch (xlu_cfg_get_list_as_string_list(config, "serial",
&b_info->u.hvm.serial_list,
1))
--
1.9.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-06 9:08 [Qemu-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream Tiejun Chen
` (2 preceding siblings ...)
2015-03-06 9:08 ` [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind Tiejun Chen
@ 2015-03-06 9:08 ` Tiejun Chen
2015-03-06 12:28 ` Wei Liu
` (2 subsequent siblings)
6 siblings, 0 replies; 37+ messages in thread
From: Tiejun Chen @ 2015-03-06 9:08 UTC (permalink / raw)
To: ian.campbell, wei.liu2, Ian.Jackson, stefano.stabellini
Cc: qemu-devel, xen-devel
Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
build_info.u.gfx_passthru_kind to DEFAULT
gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
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.
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
tools/libxl/libxl_dm.c | 13 +++++++++++++
tools/libxl/libxl_types.idl | 6 ++++++
tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
3 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 8599a6a..780dd2a 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -757,6 +757,19 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
machinearg, max_ram_below_4g);
}
}
+
+ 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");
+ }
+
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 02be466..e209460 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"),
+ ], init_val = "LIBXL_GFX_PASSTHRU_KIND_DEFAULT")
+
# 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 440db78..3f7fede 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1953,8 +1953,23 @@ 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);
+ }
+ } 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);
+ }
switch (xlu_cfg_get_list_as_string_list(config, "serial",
&b_info->u.hvm.serial_list,
1))
--
1.9.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-06 9:08 ` [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind Tiejun Chen
2015-03-06 9:18 ` Chen, Tiejun
@ 2015-03-06 9:18 ` Chen, Tiejun
2015-03-06 12:59 ` Wei Liu
2015-03-06 12:59 ` Wei Liu
2015-03-06 12:55 ` Wei Liu
2015-03-06 12:55 ` Wei Liu
3 siblings, 2 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-06 9:18 UTC (permalink / raw)
To: ian.campbell, wei.liu2, Ian.Jackson, stefano.stabellini
Cc: qemu-devel, xen-devel
On 2015/3/6 17:08, Tiejun Chen wrote:
> Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
> build_info.u.gfx_passthru_kind to DEFAULT
> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
> 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.
>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
> tools/libxl/libxl_dm.c | 13 +++++++++++++
> tools/libxl/libxl_types.idl | 6 ++++++
> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
> 3 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8599a6a..780dd2a 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
Sorry, looks I'm missing to remove '-gfx_passthru' since this should be
gone in the case of qemu upstream,
@@ -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");
I will fold this into next revision after this review.
Thanks
Tiejun
> @@ -757,6 +757,19 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> machinearg, max_ram_below_4g);
> }
> }
> +
> + 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");
> + }
> +
> 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 02be466..e209460 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"),
> + ], init_val = "LIBXL_GFX_PASSTHRU_KIND_DEFAULT")
> +
> # 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 440db78..3f7fede 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1953,8 +1953,23 @@ 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);
> + }
> + } 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);
> + }
> switch (xlu_cfg_get_list_as_string_list(config, "serial",
> &b_info->u.hvm.serial_list,
> 1))
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-06 9:08 ` [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind Tiejun Chen
@ 2015-03-06 9:18 ` Chen, Tiejun
2015-03-06 9:18 ` Chen, Tiejun
` (2 subsequent siblings)
3 siblings, 0 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-06 9:18 UTC (permalink / raw)
To: ian.campbell, wei.liu2, Ian.Jackson, stefano.stabellini
Cc: qemu-devel, xen-devel
On 2015/3/6 17:08, Tiejun Chen wrote:
> Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
> build_info.u.gfx_passthru_kind to DEFAULT
> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
> 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.
>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
> tools/libxl/libxl_dm.c | 13 +++++++++++++
> tools/libxl/libxl_types.idl | 6 ++++++
> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
> 3 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8599a6a..780dd2a 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
Sorry, looks I'm missing to remove '-gfx_passthru' since this should be
gone in the case of qemu upstream,
@@ -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");
I will fold this into next revision after this review.
Thanks
Tiejun
> @@ -757,6 +757,19 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> machinearg, max_ram_below_4g);
> }
> }
> +
> + 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");
> + }
> +
> 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 02be466..e209460 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"),
> + ], init_val = "LIBXL_GFX_PASSTHRU_KIND_DEFAULT")
> +
> # 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 440db78..3f7fede 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1953,8 +1953,23 @@ 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);
> + }
> + } 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);
> + }
> switch (xlu_cfg_get_list_as_string_list(config, "serial",
> &b_info->u.hvm.serial_list,
> 1))
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream
2015-03-06 9:08 [Qemu-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream Tiejun Chen
@ 2015-03-06 12:28 ` Wei Liu
2015-03-06 9:08 ` Tiejun Chen
` (5 subsequent siblings)
6 siblings, 0 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-06 12:28 UTC (permalink / raw)
To: Tiejun Chen
Cc: wei.liu2, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Fri, Mar 06, 2015 at 05:08:21PM +0800, Tiejun Chen wrote:
> When we're working to support IGD GFX passthrough with qemu
> upstream, instead of "-gfx_passthru" we'd like to make that
> a machine option, "-machine xxx,igd-passthru=on". This need
> to bring a change on tool side.
>
> After a discussion with Campbell, we'd like to construct a table to record
> all IGD devices we can support. If we hit that table, we should pass that
> option. And so we also introduce a new field of type, 'gfx_passthru_kind',
> to cooperate with 'gfx_passthru' to cover all scenarios like this,
>
> gfx_passthru = 0 => sets build_info.u.gfx_passthru to false
> gfx_passthru = 1 => sets build_info.u.gfx_passthru to false and
> build_info.u.gfx_passthru_kind to DEFAULT
> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
> and build_info.u.gfx_passthru_kind to IGD
>
> ----------------------------------------------------------------
> Tiejun Chen (2):
> libxl: introduce libxl__is_igd_vga_passthru
> libxl: introduce gfx_passthru_kind
>
> tools/libxl/libxl_dm.c | 13 +++++++++++++
> tools/libxl/libxl_internal.h | 2 ++
> tools/libxl/libxl_pci.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
It might be helpful to use --stat-width=72 or some other options to
limit the width of your diffstat. :-)
Wei.
> tools/libxl/libxl_types.idl | 6 ++++++
> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
> 5 files changed, 162 insertions(+), 2 deletions(-)
>
> Thanks
> Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream
@ 2015-03-06 12:28 ` Wei Liu
0 siblings, 0 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-06 12:28 UTC (permalink / raw)
To: Tiejun Chen
Cc: wei.liu2, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Fri, Mar 06, 2015 at 05:08:21PM +0800, Tiejun Chen wrote:
> When we're working to support IGD GFX passthrough with qemu
> upstream, instead of "-gfx_passthru" we'd like to make that
> a machine option, "-machine xxx,igd-passthru=on". This need
> to bring a change on tool side.
>
> After a discussion with Campbell, we'd like to construct a table to record
> all IGD devices we can support. If we hit that table, we should pass that
> option. And so we also introduce a new field of type, 'gfx_passthru_kind',
> to cooperate with 'gfx_passthru' to cover all scenarios like this,
>
> gfx_passthru = 0 => sets build_info.u.gfx_passthru to false
> gfx_passthru = 1 => sets build_info.u.gfx_passthru to false and
> build_info.u.gfx_passthru_kind to DEFAULT
> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
> and build_info.u.gfx_passthru_kind to IGD
>
> ----------------------------------------------------------------
> Tiejun Chen (2):
> libxl: introduce libxl__is_igd_vga_passthru
> libxl: introduce gfx_passthru_kind
>
> tools/libxl/libxl_dm.c | 13 +++++++++++++
> tools/libxl/libxl_internal.h | 2 ++
> tools/libxl/libxl_pci.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
It might be helpful to use --stat-width=72 or some other options to
limit the width of your diffstat. :-)
Wei.
> tools/libxl/libxl_types.idl | 6 ++++++
> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
> 5 files changed, 162 insertions(+), 2 deletions(-)
>
> Thanks
> Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru
2015-03-06 9:08 ` [Qemu-devel] [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru Tiejun Chen
@ 2015-03-06 12:40 ` Wei Liu
0 siblings, 0 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-06 12:40 UTC (permalink / raw)
To: Tiejun Chen
Cc: wei.liu2, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Fri, Mar 06, 2015 at 05:08:22PM +0800, Tiejun Chen wrote:
> While working with qemu, IGD is a specific device in the case of pass through
> so we need to identify that to handle more later. Here we define a table to
> record all IGD types currently we can support. Also we need to introduce two
> helper functions to get vendor and device ids to lookup that table.
>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
> tools/libxl/libxl_internal.h | 2 +
> tools/libxl/libxl_pci.c | 124 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 126 insertions(+)
>
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 934465a..8b952b8 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1176,6 +1176,8 @@ _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pc
> _hidden int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
> libxl_device_pci *pcidev, int num);
> _hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
> +_hidden int libxl__is_igd_vga_passthru(libxl__gc *gc,
> + const libxl_domain_config *d_config);
>
> /*----- xswait: wait for a xenstore node to be suitable -----*/
>
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index f3ae132..dc5a89e 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -491,6 +491,130 @@ static int sysfs_dev_unbind(libxl__gc *gc, libxl_device_pci *pcidev,
> return 0;
> }
>
> +static unsigned long sysfs_dev_get_vendor(libxl__gc *gc,
> + libxl_device_pci *pcidev)
uint16_t?
> +{
> + char *pci_device_vendor_path =
> + libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/vendor",
> + pcidev->domain, pcidev->bus, pcidev->dev,
> + pcidev->func);
Please use GCSPRINTF macro.
> + int read_items;
> + unsigned long pci_device_vendor;
uint16_t?
Same comments apply to _get_device function.
> +
> + FILE *f = fopen(pci_device_vendor_path, "r");
> + if (!f) {
> + LOGE(ERROR,
> + "pci device "PCI_BDF" does not have vendor attribute",
> + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
> + return 0xffff;
> + }
> + read_items = fscanf(f, "0x%lx\n", &pci_device_vendor);
> + fclose(f);
> + if (read_items != 1) {
> + LOGE(ERROR,
> + "cannot read vendor of pci device "PCI_BDF,
> + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
> + return 0xffff;
> + }
> +
> + return pci_device_vendor;
> +}
> +
[...]
> +/*
> + * Some devices may need some ways to work well. Here like IGD,
> + * we have to pass a specific option to qemu.
> + */
> +int libxl__is_igd_vga_passthru(libxl__gc *gc,
bool.
> + const libxl_domain_config *d_config)
> +{
> + unsigned int i, j, num = ARRAY_SIZE(fixup_ids);
> + uint16_t vendor, device;
> +
> + for (i = 0 ; i < d_config->num_pcidevs ; i++) {
> + libxl_device_pci *pcidev = &d_config->pcidevs[i];
> +
> + for (j = 0 ; j < num ; j++) {
> + vendor = fixup_ids[j].vendor;
> + device = fixup_ids[j].device;
> +
> + if (sysfs_dev_get_vendor(gc, pcidev) == vendor &&
> + sysfs_dev_get_device(gc, pcidev) == device)
> + return 1;
Get vendor and device in outer loop to avoid wasting cpu cycles. :-)
Wei.
> + }
> + }
> +
> + return 0;
> +}
> +
> /*
> * A brief comment about slots. I don't know what slots are for; however,
> * I have by experimentation determined:
> --
> 1.9.1
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru
@ 2015-03-06 12:40 ` Wei Liu
0 siblings, 0 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-06 12:40 UTC (permalink / raw)
To: Tiejun Chen
Cc: wei.liu2, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Fri, Mar 06, 2015 at 05:08:22PM +0800, Tiejun Chen wrote:
> While working with qemu, IGD is a specific device in the case of pass through
> so we need to identify that to handle more later. Here we define a table to
> record all IGD types currently we can support. Also we need to introduce two
> helper functions to get vendor and device ids to lookup that table.
>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
> tools/libxl/libxl_internal.h | 2 +
> tools/libxl/libxl_pci.c | 124 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 126 insertions(+)
>
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 934465a..8b952b8 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1176,6 +1176,8 @@ _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pc
> _hidden int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
> libxl_device_pci *pcidev, int num);
> _hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
> +_hidden int libxl__is_igd_vga_passthru(libxl__gc *gc,
> + const libxl_domain_config *d_config);
>
> /*----- xswait: wait for a xenstore node to be suitable -----*/
>
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index f3ae132..dc5a89e 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -491,6 +491,130 @@ static int sysfs_dev_unbind(libxl__gc *gc, libxl_device_pci *pcidev,
> return 0;
> }
>
> +static unsigned long sysfs_dev_get_vendor(libxl__gc *gc,
> + libxl_device_pci *pcidev)
uint16_t?
> +{
> + char *pci_device_vendor_path =
> + libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/vendor",
> + pcidev->domain, pcidev->bus, pcidev->dev,
> + pcidev->func);
Please use GCSPRINTF macro.
> + int read_items;
> + unsigned long pci_device_vendor;
uint16_t?
Same comments apply to _get_device function.
> +
> + FILE *f = fopen(pci_device_vendor_path, "r");
> + if (!f) {
> + LOGE(ERROR,
> + "pci device "PCI_BDF" does not have vendor attribute",
> + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
> + return 0xffff;
> + }
> + read_items = fscanf(f, "0x%lx\n", &pci_device_vendor);
> + fclose(f);
> + if (read_items != 1) {
> + LOGE(ERROR,
> + "cannot read vendor of pci device "PCI_BDF,
> + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
> + return 0xffff;
> + }
> +
> + return pci_device_vendor;
> +}
> +
[...]
> +/*
> + * Some devices may need some ways to work well. Here like IGD,
> + * we have to pass a specific option to qemu.
> + */
> +int libxl__is_igd_vga_passthru(libxl__gc *gc,
bool.
> + const libxl_domain_config *d_config)
> +{
> + unsigned int i, j, num = ARRAY_SIZE(fixup_ids);
> + uint16_t vendor, device;
> +
> + for (i = 0 ; i < d_config->num_pcidevs ; i++) {
> + libxl_device_pci *pcidev = &d_config->pcidevs[i];
> +
> + for (j = 0 ; j < num ; j++) {
> + vendor = fixup_ids[j].vendor;
> + device = fixup_ids[j].device;
> +
> + if (sysfs_dev_get_vendor(gc, pcidev) == vendor &&
> + sysfs_dev_get_device(gc, pcidev) == device)
> + return 1;
Get vendor and device in outer loop to avoid wasting cpu cycles. :-)
Wei.
> + }
> + }
> +
> + return 0;
> +}
> +
> /*
> * A brief comment about slots. I don't know what slots are for; however,
> * I have by experimentation determined:
> --
> 1.9.1
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-06 9:08 ` [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind Tiejun Chen
2015-03-06 9:18 ` Chen, Tiejun
2015-03-06 9:18 ` Chen, Tiejun
@ 2015-03-06 12:55 ` Wei Liu
2015-03-09 6:45 ` Chen, Tiejun
2015-03-09 6:45 ` [Qemu-devel] " Chen, Tiejun
2015-03-06 12:55 ` Wei Liu
3 siblings, 2 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-06 12:55 UTC (permalink / raw)
To: Tiejun Chen
Cc: wei.liu2, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Fri, Mar 06, 2015 at 05:08:23PM +0800, Tiejun Chen wrote:
> Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
^^^^^
true?
> build_info.u.gfx_passthru_kind to DEFAULT
> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
> 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.
>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
> tools/libxl/libxl_dm.c | 13 +++++++++++++
> tools/libxl/libxl_types.idl | 6 ++++++
> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
> 3 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8599a6a..780dd2a 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -757,6 +757,19 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> machinearg, max_ram_below_4g);
> }
> }
> +
> + 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);
> + }
You can remove that {} around machinearg -- it's only one line after
`if'.
> + } 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");
> + }
> +
> 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 02be466..e209460 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"),
> + ], init_val = "LIBXL_GFX_PASSTHRU_KIND_DEFAULT")
> +
You don't need to set init_val if the default value is 0.
> # 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),
^^^^
One space is enough, I think.
>
> ("serial", string),
> ("boot", string),
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 440db78..3f7fede 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1953,8 +1953,23 @@ 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);
Please wrap this line to be < 80 column.
> + exit (1);
> + }
> + } 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)) {
Ditto.
> + fprintf(stderr, "ERROR: invalid value \"%s\" for \"gfx_passthru\"\n",
Ditto.
Wei.
> + buf);
> + exit (1);
> + }
> + libxl_defbool_set(&b_info->u.hvm.gfx_passthru, false);
> + }
> switch (xlu_cfg_get_list_as_string_list(config, "serial",
> &b_info->u.hvm.serial_list,
> 1))
> --
> 1.9.1
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-06 9:08 ` [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind Tiejun Chen
` (2 preceding siblings ...)
2015-03-06 12:55 ` Wei Liu
@ 2015-03-06 12:55 ` Wei Liu
3 siblings, 0 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-06 12:55 UTC (permalink / raw)
To: Tiejun Chen
Cc: wei.liu2, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Fri, Mar 06, 2015 at 05:08:23PM +0800, Tiejun Chen wrote:
> Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
^^^^^
true?
> build_info.u.gfx_passthru_kind to DEFAULT
> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
> 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.
>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
> tools/libxl/libxl_dm.c | 13 +++++++++++++
> tools/libxl/libxl_types.idl | 6 ++++++
> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
> 3 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8599a6a..780dd2a 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -757,6 +757,19 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> machinearg, max_ram_below_4g);
> }
> }
> +
> + 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);
> + }
You can remove that {} around machinearg -- it's only one line after
`if'.
> + } 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");
> + }
> +
> 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 02be466..e209460 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"),
> + ], init_val = "LIBXL_GFX_PASSTHRU_KIND_DEFAULT")
> +
You don't need to set init_val if the default value is 0.
> # 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),
^^^^
One space is enough, I think.
>
> ("serial", string),
> ("boot", string),
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 440db78..3f7fede 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1953,8 +1953,23 @@ 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);
Please wrap this line to be < 80 column.
> + exit (1);
> + }
> + } 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)) {
Ditto.
> + fprintf(stderr, "ERROR: invalid value \"%s\" for \"gfx_passthru\"\n",
Ditto.
Wei.
> + buf);
> + exit (1);
> + }
> + libxl_defbool_set(&b_info->u.hvm.gfx_passthru, false);
> + }
> switch (xlu_cfg_get_list_as_string_list(config, "serial",
> &b_info->u.hvm.serial_list,
> 1))
> --
> 1.9.1
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-06 9:18 ` Chen, Tiejun
2015-03-06 12:59 ` Wei Liu
@ 2015-03-06 12:59 ` Wei Liu
2015-03-09 6:50 ` Chen, Tiejun
2015-03-09 6:50 ` Chen, Tiejun
1 sibling, 2 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-06 12:59 UTC (permalink / raw)
To: Chen, Tiejun
Cc: wei.liu2, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Fri, Mar 06, 2015 at 05:18:36PM +0800, Chen, Tiejun wrote:
> On 2015/3/6 17:08, Tiejun Chen wrote:
> >Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
> > build_info.u.gfx_passthru_kind to DEFAULT
> > gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
> > 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.
> >
> >Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >---
> > tools/libxl/libxl_dm.c | 13 +++++++++++++
> > tools/libxl/libxl_types.idl | 6 ++++++
> > tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
> > 3 files changed, 36 insertions(+), 2 deletions(-)
> >
> >diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> >index 8599a6a..780dd2a 100644
> >--- a/tools/libxl/libxl_dm.c
> >+++ b/tools/libxl/libxl_dm.c
>
> Sorry, looks I'm missing to remove '-gfx_passthru' since this should be gone
> in the case of qemu upstream,
>
> @@ -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");
>
> I will fold this into next revision after this review.
>
I think this change alone warrants a separate changeset. It should be
patch 0 of your series as a preparatory patch. Please remember to
elaborate in commit message why that hunk is not needed in upstream
QEMU.
Wei.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-06 9:18 ` Chen, Tiejun
@ 2015-03-06 12:59 ` Wei Liu
2015-03-06 12:59 ` Wei Liu
1 sibling, 0 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-06 12:59 UTC (permalink / raw)
To: Chen, Tiejun
Cc: wei.liu2, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Fri, Mar 06, 2015 at 05:18:36PM +0800, Chen, Tiejun wrote:
> On 2015/3/6 17:08, Tiejun Chen wrote:
> >Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
> > build_info.u.gfx_passthru_kind to DEFAULT
> > gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
> > 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.
> >
> >Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >---
> > tools/libxl/libxl_dm.c | 13 +++++++++++++
> > tools/libxl/libxl_types.idl | 6 ++++++
> > tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
> > 3 files changed, 36 insertions(+), 2 deletions(-)
> >
> >diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> >index 8599a6a..780dd2a 100644
> >--- a/tools/libxl/libxl_dm.c
> >+++ b/tools/libxl/libxl_dm.c
>
> Sorry, looks I'm missing to remove '-gfx_passthru' since this should be gone
> in the case of qemu upstream,
>
> @@ -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");
>
> I will fold this into next revision after this review.
>
I think this change alone warrants a separate changeset. It should be
patch 0 of your series as a preparatory patch. Please remember to
elaborate in commit message why that hunk is not needed in upstream
QEMU.
Wei.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [Xen-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream
2015-03-06 9:08 [Qemu-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream Tiejun Chen
` (4 preceding siblings ...)
2015-03-06 12:28 ` Wei Liu
@ 2015-03-06 15:53 ` Konrad Rzeszutek Wilk
2015-03-09 6:58 ` Chen, Tiejun
2015-03-09 6:58 ` [Qemu-devel] [Xen-devel] " Chen, Tiejun
2015-03-06 15:53 ` Konrad Rzeszutek Wilk
6 siblings, 2 replies; 37+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-03-06 15:53 UTC (permalink / raw)
To: Tiejun Chen
Cc: wei.liu2, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Fri, Mar 06, 2015 at 05:08:21PM +0800, Tiejun Chen wrote:
> When we're working to support IGD GFX passthrough with qemu
> upstream, instead of "-gfx_passthru" we'd like to make that
Could you also include in the cover letter an URL link to the latest
discussion on this?
Thank you.
> a machine option, "-machine xxx,igd-passthru=on". This need
> to bring a change on tool side.
>
> After a discussion with Campbell, we'd like to construct a table to record
> all IGD devices we can support. If we hit that table, we should pass that
> option. And so we also introduce a new field of type, 'gfx_passthru_kind',
> to cooperate with 'gfx_passthru' to cover all scenarios like this,
>
> gfx_passthru = 0 => sets build_info.u.gfx_passthru to false
> gfx_passthru = 1 => sets build_info.u.gfx_passthru to false and
> build_info.u.gfx_passthru_kind to DEFAULT
> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
> and build_info.u.gfx_passthru_kind to IGD
>
> ----------------------------------------------------------------
> Tiejun Chen (2):
> libxl: introduce libxl__is_igd_vga_passthru
> libxl: introduce gfx_passthru_kind
>
> tools/libxl/libxl_dm.c | 13 +++++++++++++
> tools/libxl/libxl_internal.h | 2 ++
> tools/libxl/libxl_pci.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/libxl/libxl_types.idl | 6 ++++++
> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
> 5 files changed, 162 insertions(+), 2 deletions(-)
>
> Thanks
> Tiejun
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream
2015-03-06 9:08 [Qemu-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream Tiejun Chen
` (5 preceding siblings ...)
2015-03-06 15:53 ` [Qemu-devel] [Xen-devel] " Konrad Rzeszutek Wilk
@ 2015-03-06 15:53 ` Konrad Rzeszutek Wilk
6 siblings, 0 replies; 37+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-03-06 15:53 UTC (permalink / raw)
To: Tiejun Chen
Cc: wei.liu2, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Fri, Mar 06, 2015 at 05:08:21PM +0800, Tiejun Chen wrote:
> When we're working to support IGD GFX passthrough with qemu
> upstream, instead of "-gfx_passthru" we'd like to make that
Could you also include in the cover letter an URL link to the latest
discussion on this?
Thank you.
> a machine option, "-machine xxx,igd-passthru=on". This need
> to bring a change on tool side.
>
> After a discussion with Campbell, we'd like to construct a table to record
> all IGD devices we can support. If we hit that table, we should pass that
> option. And so we also introduce a new field of type, 'gfx_passthru_kind',
> to cooperate with 'gfx_passthru' to cover all scenarios like this,
>
> gfx_passthru = 0 => sets build_info.u.gfx_passthru to false
> gfx_passthru = 1 => sets build_info.u.gfx_passthru to false and
> build_info.u.gfx_passthru_kind to DEFAULT
> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
> and build_info.u.gfx_passthru_kind to IGD
>
> ----------------------------------------------------------------
> Tiejun Chen (2):
> libxl: introduce libxl__is_igd_vga_passthru
> libxl: introduce gfx_passthru_kind
>
> tools/libxl/libxl_dm.c | 13 +++++++++++++
> tools/libxl/libxl_internal.h | 2 ++
> tools/libxl/libxl_pci.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/libxl/libxl_types.idl | 6 ++++++
> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
> 5 files changed, 162 insertions(+), 2 deletions(-)
>
> Thanks
> Tiejun
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream
2015-03-06 12:28 ` Wei Liu
(?)
(?)
@ 2015-03-09 5:32 ` Chen, Tiejun
-1 siblings, 0 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-09 5:32 UTC (permalink / raw)
To: Wei Liu
Cc: qemu-devel, stefano.stabellini, Ian.Jackson, ian.campbell, xen-devel
On 2015/3/6 20:28, Wei Liu wrote:
> On Fri, Mar 06, 2015 at 05:08:21PM +0800, Tiejun Chen wrote:
>> When we're working to support IGD GFX passthrough with qemu
>> upstream, instead of "-gfx_passthru" we'd like to make that
>> a machine option, "-machine xxx,igd-passthru=on". This need
>> to bring a change on tool side.
>>
>> After a discussion with Campbell, we'd like to construct a table to record
>> all IGD devices we can support. If we hit that table, we should pass that
>> option. And so we also introduce a new field of type, 'gfx_passthru_kind',
>> to cooperate with 'gfx_passthru' to cover all scenarios like this,
>>
>> gfx_passthru = 0 => sets build_info.u.gfx_passthru to false
>> gfx_passthru = 1 => sets build_info.u.gfx_passthru to false and
>> build_info.u.gfx_passthru_kind to DEFAULT
>> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
>> and build_info.u.gfx_passthru_kind to IGD
>>
>> ----------------------------------------------------------------
>> Tiejun Chen (2):
>> libxl: introduce libxl__is_igd_vga_passthru
>> libxl: introduce gfx_passthru_kind
>>
>> tools/libxl/libxl_dm.c | 13 +++++++++++++
>> tools/libxl/libxl_internal.h | 2 ++
>> tools/libxl/libxl_pci.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>
> It might be helpful to use --stat-width=72 or some other options to
> limit the width of your diffstat. :-)
>
Will do.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream
2015-03-06 12:28 ` Wei Liu
(?)
@ 2015-03-09 5:32 ` Chen, Tiejun
-1 siblings, 0 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-09 5:32 UTC (permalink / raw)
To: Wei Liu
Cc: qemu-devel, stefano.stabellini, Ian.Jackson, ian.campbell, xen-devel
On 2015/3/6 20:28, Wei Liu wrote:
> On Fri, Mar 06, 2015 at 05:08:21PM +0800, Tiejun Chen wrote:
>> When we're working to support IGD GFX passthrough with qemu
>> upstream, instead of "-gfx_passthru" we'd like to make that
>> a machine option, "-machine xxx,igd-passthru=on". This need
>> to bring a change on tool side.
>>
>> After a discussion with Campbell, we'd like to construct a table to record
>> all IGD devices we can support. If we hit that table, we should pass that
>> option. And so we also introduce a new field of type, 'gfx_passthru_kind',
>> to cooperate with 'gfx_passthru' to cover all scenarios like this,
>>
>> gfx_passthru = 0 => sets build_info.u.gfx_passthru to false
>> gfx_passthru = 1 => sets build_info.u.gfx_passthru to false and
>> build_info.u.gfx_passthru_kind to DEFAULT
>> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
>> and build_info.u.gfx_passthru_kind to IGD
>>
>> ----------------------------------------------------------------
>> Tiejun Chen (2):
>> libxl: introduce libxl__is_igd_vga_passthru
>> libxl: introduce gfx_passthru_kind
>>
>> tools/libxl/libxl_dm.c | 13 +++++++++++++
>> tools/libxl/libxl_internal.h | 2 ++
>> tools/libxl/libxl_pci.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>
> It might be helpful to use --stat-width=72 or some other options to
> limit the width of your diffstat. :-)
>
Will do.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru
2015-03-06 12:40 ` Wei Liu
(?)
(?)
@ 2015-03-09 6:27 ` Chen, Tiejun
2015-03-09 10:12 ` Wei Liu
2015-03-09 10:12 ` Wei Liu
-1 siblings, 2 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-09 6:27 UTC (permalink / raw)
To: Wei Liu
Cc: qemu-devel, stefano.stabellini, Ian.Jackson, ian.campbell, xen-devel
On 2015/3/6 20:40, Wei Liu wrote:
> On Fri, Mar 06, 2015 at 05:08:22PM +0800, Tiejun Chen wrote:
>> While working with qemu, IGD is a specific device in the case of pass through
>> so we need to identify that to handle more later. Here we define a table to
>> record all IGD types currently we can support. Also we need to introduce two
>> helper functions to get vendor and device ids to lookup that table.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>> tools/libxl/libxl_internal.h | 2 +
>> tools/libxl/libxl_pci.c | 124 +++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 126 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
>> index 934465a..8b952b8 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -1176,6 +1176,8 @@ _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pc
>> _hidden int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
>> libxl_device_pci *pcidev, int num);
>> _hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
>> +_hidden int libxl__is_igd_vga_passthru(libxl__gc *gc,
>> + const libxl_domain_config *d_config);
>>
>> /*----- xswait: wait for a xenstore node to be suitable -----*/
>>
>> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
>> index f3ae132..dc5a89e 100644
>> --- a/tools/libxl/libxl_pci.c
>> +++ b/tools/libxl/libxl_pci.c
>> @@ -491,6 +491,130 @@ static int sysfs_dev_unbind(libxl__gc *gc, libxl_device_pci *pcidev,
>> return 0;
>> }
>>
>> +static unsigned long sysfs_dev_get_vendor(libxl__gc *gc,
>> + libxl_device_pci *pcidev)
>
> uint16_t?
>
>> +{
>> + char *pci_device_vendor_path =
>> + libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/vendor",
>> + pcidev->domain, pcidev->bus, pcidev->dev,
>> + pcidev->func);
>
> Please use GCSPRINTF macro.
Okay.
>
>> + int read_items;
>> + unsigned long pci_device_vendor;
>
> uint16_t?
Yes, I can but I don't see other similar helpers are doing this in this
file :)
>
> Same comments apply to _get_device function.
And especially, if we really set that as uint16_t,
>
>> +
>> + FILE *f = fopen(pci_device_vendor_path, "r");
>> + if (!f) {
>> + LOGE(ERROR,
>> + "pci device "PCI_BDF" does not have vendor attribute",
>> + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
>> + return 0xffff;
>> + }
>> + read_items = fscanf(f, "0x%lx\n", &pci_device_vendor);
we have to refactor this as well,
read_items = fscanf(f, "0x%hx\n", &pci_device_vendor);
Right?
>> + fclose(f);
>> + if (read_items != 1) {
>> + LOGE(ERROR,
>> + "cannot read vendor of pci device "PCI_BDF,
>> + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
>> + return 0xffff;
>> + }
>> +
>> + return pci_device_vendor;
>> +}
>> +
>
> [...]
>
>> +/*
>> + * Some devices may need some ways to work well. Here like IGD,
>> + * we have to pass a specific option to qemu.
>> + */
>> +int libxl__is_igd_vga_passthru(libxl__gc *gc,
>
> bool.
Okay.
>
>> + const libxl_domain_config *d_config)
>> +{
>> + unsigned int i, j, num = ARRAY_SIZE(fixup_ids);
>> + uint16_t vendor, device;
>> +
>> + for (i = 0 ; i < d_config->num_pcidevs ; i++) {
>> + libxl_device_pci *pcidev = &d_config->pcidevs[i];
>> +
>> + for (j = 0 ; j < num ; j++) {
>> + vendor = fixup_ids[j].vendor;
>> + device = fixup_ids[j].device;
>> +
>> + if (sysfs_dev_get_vendor(gc, pcidev) == vendor &&
>> + sysfs_dev_get_device(gc, pcidev) == device)
>> + return 1;
>
> Get vendor and device in outer loop to avoid wasting cpu cycles. :-)
>
Yeah.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru
2015-03-06 12:40 ` Wei Liu
(?)
@ 2015-03-09 6:27 ` Chen, Tiejun
-1 siblings, 0 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-09 6:27 UTC (permalink / raw)
To: Wei Liu
Cc: qemu-devel, stefano.stabellini, Ian.Jackson, ian.campbell, xen-devel
On 2015/3/6 20:40, Wei Liu wrote:
> On Fri, Mar 06, 2015 at 05:08:22PM +0800, Tiejun Chen wrote:
>> While working with qemu, IGD is a specific device in the case of pass through
>> so we need to identify that to handle more later. Here we define a table to
>> record all IGD types currently we can support. Also we need to introduce two
>> helper functions to get vendor and device ids to lookup that table.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>> tools/libxl/libxl_internal.h | 2 +
>> tools/libxl/libxl_pci.c | 124 +++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 126 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
>> index 934465a..8b952b8 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -1176,6 +1176,8 @@ _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pc
>> _hidden int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
>> libxl_device_pci *pcidev, int num);
>> _hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
>> +_hidden int libxl__is_igd_vga_passthru(libxl__gc *gc,
>> + const libxl_domain_config *d_config);
>>
>> /*----- xswait: wait for a xenstore node to be suitable -----*/
>>
>> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
>> index f3ae132..dc5a89e 100644
>> --- a/tools/libxl/libxl_pci.c
>> +++ b/tools/libxl/libxl_pci.c
>> @@ -491,6 +491,130 @@ static int sysfs_dev_unbind(libxl__gc *gc, libxl_device_pci *pcidev,
>> return 0;
>> }
>>
>> +static unsigned long sysfs_dev_get_vendor(libxl__gc *gc,
>> + libxl_device_pci *pcidev)
>
> uint16_t?
>
>> +{
>> + char *pci_device_vendor_path =
>> + libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/vendor",
>> + pcidev->domain, pcidev->bus, pcidev->dev,
>> + pcidev->func);
>
> Please use GCSPRINTF macro.
Okay.
>
>> + int read_items;
>> + unsigned long pci_device_vendor;
>
> uint16_t?
Yes, I can but I don't see other similar helpers are doing this in this
file :)
>
> Same comments apply to _get_device function.
And especially, if we really set that as uint16_t,
>
>> +
>> + FILE *f = fopen(pci_device_vendor_path, "r");
>> + if (!f) {
>> + LOGE(ERROR,
>> + "pci device "PCI_BDF" does not have vendor attribute",
>> + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
>> + return 0xffff;
>> + }
>> + read_items = fscanf(f, "0x%lx\n", &pci_device_vendor);
we have to refactor this as well,
read_items = fscanf(f, "0x%hx\n", &pci_device_vendor);
Right?
>> + fclose(f);
>> + if (read_items != 1) {
>> + LOGE(ERROR,
>> + "cannot read vendor of pci device "PCI_BDF,
>> + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
>> + return 0xffff;
>> + }
>> +
>> + return pci_device_vendor;
>> +}
>> +
>
> [...]
>
>> +/*
>> + * Some devices may need some ways to work well. Here like IGD,
>> + * we have to pass a specific option to qemu.
>> + */
>> +int libxl__is_igd_vga_passthru(libxl__gc *gc,
>
> bool.
Okay.
>
>> + const libxl_domain_config *d_config)
>> +{
>> + unsigned int i, j, num = ARRAY_SIZE(fixup_ids);
>> + uint16_t vendor, device;
>> +
>> + for (i = 0 ; i < d_config->num_pcidevs ; i++) {
>> + libxl_device_pci *pcidev = &d_config->pcidevs[i];
>> +
>> + for (j = 0 ; j < num ; j++) {
>> + vendor = fixup_ids[j].vendor;
>> + device = fixup_ids[j].device;
>> +
>> + if (sysfs_dev_get_vendor(gc, pcidev) == vendor &&
>> + sysfs_dev_get_device(gc, pcidev) == device)
>> + return 1;
>
> Get vendor and device in outer loop to avoid wasting cpu cycles. :-)
>
Yeah.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-06 12:55 ` Wei Liu
2015-03-09 6:45 ` Chen, Tiejun
@ 2015-03-09 6:45 ` Chen, Tiejun
2015-03-09 10:17 ` Wei Liu
` (3 more replies)
1 sibling, 4 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-09 6:45 UTC (permalink / raw)
To: Wei Liu
Cc: qemu-devel, stefano.stabellini, Ian.Jackson, ian.campbell, xen-devel
On 2015/3/6 20:55, Wei Liu wrote:
> On Fri, Mar 06, 2015 at 05:08:23PM +0800, Tiejun Chen wrote:
>> Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
> ^^^^^
> true?
I just simply pick up this from Campbell's comment but I agree you.
>
>> build_info.u.gfx_passthru_kind to DEFAULT
>> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
>> 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.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>> tools/libxl/libxl_dm.c | 13 +++++++++++++
>> tools/libxl/libxl_types.idl | 6 ++++++
>> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
>> 3 files changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> index 8599a6a..780dd2a 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -757,6 +757,19 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>> machinearg, max_ram_below_4g);
>> }
>> }
>> +
>> + 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);
>> + }
>
> You can remove that {} around machinearg -- it's only one line after
> `if'.
Okay.
>
>> + } 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");
>> + }
>> +
>> 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 02be466..e209460 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"),
>> + ], init_val = "LIBXL_GFX_PASSTHRU_KIND_DEFAULT")
>> +
>
> You don't need to set init_val if the default value is 0.
Got it.
>
>> # 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),
> ^^^^
> One space is enough, I think.
Okay.
>
>>
>> ("serial", string),
>> ("boot", string),
>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>> index 440db78..3f7fede 100644
>> --- a/tools/libxl/xl_cmdimpl.c
>> +++ b/tools/libxl/xl_cmdimpl.c
>> @@ -1953,8 +1953,23 @@ 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);
>
> Please wrap this line to be < 80 column.
>
>> + exit (1);
>> + }
>> + } 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)) {
>
> Ditto.
>
>> + fprintf(stderr, "ERROR: invalid value \"%s\" for \"gfx_passthru\"\n",
>
> Ditto.
So,
@@ -1959,13 +1959,15 @@ skip_vfb:
} 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);
+ fprintf(stderr, "ERROR: invalid value %ld for"
+ " \"gfx_passthru\"\n", l);
exit (1);
}
} 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);
+ 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);
Thanks
Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-06 12:55 ` Wei Liu
@ 2015-03-09 6:45 ` Chen, Tiejun
2015-03-09 6:45 ` [Qemu-devel] " Chen, Tiejun
1 sibling, 0 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-09 6:45 UTC (permalink / raw)
To: Wei Liu
Cc: qemu-devel, stefano.stabellini, Ian.Jackson, ian.campbell, xen-devel
On 2015/3/6 20:55, Wei Liu wrote:
> On Fri, Mar 06, 2015 at 05:08:23PM +0800, Tiejun Chen wrote:
>> Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
> ^^^^^
> true?
I just simply pick up this from Campbell's comment but I agree you.
>
>> build_info.u.gfx_passthru_kind to DEFAULT
>> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
>> 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.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>> tools/libxl/libxl_dm.c | 13 +++++++++++++
>> tools/libxl/libxl_types.idl | 6 ++++++
>> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
>> 3 files changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> index 8599a6a..780dd2a 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -757,6 +757,19 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>> machinearg, max_ram_below_4g);
>> }
>> }
>> +
>> + 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);
>> + }
>
> You can remove that {} around machinearg -- it's only one line after
> `if'.
Okay.
>
>> + } 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");
>> + }
>> +
>> 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 02be466..e209460 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"),
>> + ], init_val = "LIBXL_GFX_PASSTHRU_KIND_DEFAULT")
>> +
>
> You don't need to set init_val if the default value is 0.
Got it.
>
>> # 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),
> ^^^^
> One space is enough, I think.
Okay.
>
>>
>> ("serial", string),
>> ("boot", string),
>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>> index 440db78..3f7fede 100644
>> --- a/tools/libxl/xl_cmdimpl.c
>> +++ b/tools/libxl/xl_cmdimpl.c
>> @@ -1953,8 +1953,23 @@ 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);
>
> Please wrap this line to be < 80 column.
>
>> + exit (1);
>> + }
>> + } 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)) {
>
> Ditto.
>
>> + fprintf(stderr, "ERROR: invalid value \"%s\" for \"gfx_passthru\"\n",
>
> Ditto.
So,
@@ -1959,13 +1959,15 @@ skip_vfb:
} 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);
+ fprintf(stderr, "ERROR: invalid value %ld for"
+ " \"gfx_passthru\"\n", l);
exit (1);
}
} 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);
+ 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);
Thanks
Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-06 12:59 ` Wei Liu
@ 2015-03-09 6:50 ` Chen, Tiejun
2015-03-09 10:13 ` Wei Liu
2015-03-09 10:13 ` Wei Liu
2015-03-09 6:50 ` Chen, Tiejun
1 sibling, 2 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-09 6:50 UTC (permalink / raw)
To: Wei Liu
Cc: stefano.stabellini, xen-devel, Ian.Jackson, ian.campbell, qemu-devel
On 2015/3/6 20:59, Wei Liu wrote:
> On Fri, Mar 06, 2015 at 05:18:36PM +0800, Chen, Tiejun wrote:
>> On 2015/3/6 17:08, Tiejun Chen wrote:
>>> Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
>>> build_info.u.gfx_passthru_kind to DEFAULT
>>> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
>>> 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.
>>>
>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>> ---
>>> tools/libxl/libxl_dm.c | 13 +++++++++++++
>>> tools/libxl/libxl_types.idl | 6 ++++++
>>> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
>>> 3 files changed, 36 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>> index 8599a6a..780dd2a 100644
>>> --- a/tools/libxl/libxl_dm.c
>>> +++ b/tools/libxl/libxl_dm.c
>>
>> Sorry, looks I'm missing to remove '-gfx_passthru' since this should be gone
>> in the case of qemu upstream,
>>
>> @@ -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");
>>
>> I will fold this into next revision after this review.
>>
>
> I think this change alone warrants a separate changeset. It should be
> patch 0 of your series as a preparatory patch. Please remember to
> elaborate in commit message why that hunk is not needed in upstream
> QEMU.
>
What about this?
"-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.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-06 12:59 ` Wei Liu
2015-03-09 6:50 ` Chen, Tiejun
@ 2015-03-09 6:50 ` Chen, Tiejun
1 sibling, 0 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-09 6:50 UTC (permalink / raw)
To: Wei Liu
Cc: stefano.stabellini, xen-devel, Ian.Jackson, ian.campbell, qemu-devel
On 2015/3/6 20:59, Wei Liu wrote:
> On Fri, Mar 06, 2015 at 05:18:36PM +0800, Chen, Tiejun wrote:
>> On 2015/3/6 17:08, Tiejun Chen wrote:
>>> Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
>>> build_info.u.gfx_passthru_kind to DEFAULT
>>> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
>>> 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.
>>>
>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>> ---
>>> tools/libxl/libxl_dm.c | 13 +++++++++++++
>>> tools/libxl/libxl_types.idl | 6 ++++++
>>> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
>>> 3 files changed, 36 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>> index 8599a6a..780dd2a 100644
>>> --- a/tools/libxl/libxl_dm.c
>>> +++ b/tools/libxl/libxl_dm.c
>>
>> Sorry, looks I'm missing to remove '-gfx_passthru' since this should be gone
>> in the case of qemu upstream,
>>
>> @@ -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");
>>
>> I will fold this into next revision after this review.
>>
>
> I think this change alone warrants a separate changeset. It should be
> patch 0 of your series as a preparatory patch. Please remember to
> elaborate in commit message why that hunk is not needed in upstream
> QEMU.
>
What about this?
"-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.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [Xen-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream
2015-03-06 15:53 ` [Qemu-devel] [Xen-devel] " Konrad Rzeszutek Wilk
2015-03-09 6:58 ` Chen, Tiejun
@ 2015-03-09 6:58 ` Chen, Tiejun
1 sibling, 0 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-09 6:58 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: wei.liu2, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On 2015/3/6 23:53, Konrad Rzeszutek Wilk wrote:
> On Fri, Mar 06, 2015 at 05:08:21PM +0800, Tiejun Chen wrote:
>> When we're working to support IGD GFX passthrough with qemu
>> upstream, instead of "-gfx_passthru" we'd like to make that
>
> Could you also include in the cover letter an URL link to the latest
> discussion on this?
Thanks for reminding. I really should do this.
https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg02050.html
Thanks
Tiejun
>
> Thank you.
>> a machine option, "-machine xxx,igd-passthru=on". This need
>> to bring a change on tool side.
>>
>> After a discussion with Campbell, we'd like to construct a table to record
>> all IGD devices we can support. If we hit that table, we should pass that
>> option. And so we also introduce a new field of type, 'gfx_passthru_kind',
>> to cooperate with 'gfx_passthru' to cover all scenarios like this,
>>
>> gfx_passthru = 0 => sets build_info.u.gfx_passthru to false
>> gfx_passthru = 1 => sets build_info.u.gfx_passthru to false and
>> build_info.u.gfx_passthru_kind to DEFAULT
>> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
>> and build_info.u.gfx_passthru_kind to IGD
>>
>> ----------------------------------------------------------------
>> Tiejun Chen (2):
>> libxl: introduce libxl__is_igd_vga_passthru
>> libxl: introduce gfx_passthru_kind
>>
>> tools/libxl/libxl_dm.c | 13 +++++++++++++
>> tools/libxl/libxl_internal.h | 2 ++
>> tools/libxl/libxl_pci.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> tools/libxl/libxl_types.idl | 6 ++++++
>> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
>> 5 files changed, 162 insertions(+), 2 deletions(-)
>>
>> Thanks
>> Tiejun
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream
2015-03-06 15:53 ` [Qemu-devel] [Xen-devel] " Konrad Rzeszutek Wilk
@ 2015-03-09 6:58 ` Chen, Tiejun
2015-03-09 6:58 ` [Qemu-devel] [Xen-devel] " Chen, Tiejun
1 sibling, 0 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-09 6:58 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: wei.liu2, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On 2015/3/6 23:53, Konrad Rzeszutek Wilk wrote:
> On Fri, Mar 06, 2015 at 05:08:21PM +0800, Tiejun Chen wrote:
>> When we're working to support IGD GFX passthrough with qemu
>> upstream, instead of "-gfx_passthru" we'd like to make that
>
> Could you also include in the cover letter an URL link to the latest
> discussion on this?
Thanks for reminding. I really should do this.
https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg02050.html
Thanks
Tiejun
>
> Thank you.
>> a machine option, "-machine xxx,igd-passthru=on". This need
>> to bring a change on tool side.
>>
>> After a discussion with Campbell, we'd like to construct a table to record
>> all IGD devices we can support. If we hit that table, we should pass that
>> option. And so we also introduce a new field of type, 'gfx_passthru_kind',
>> to cooperate with 'gfx_passthru' to cover all scenarios like this,
>>
>> gfx_passthru = 0 => sets build_info.u.gfx_passthru to false
>> gfx_passthru = 1 => sets build_info.u.gfx_passthru to false and
>> build_info.u.gfx_passthru_kind to DEFAULT
>> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
>> and build_info.u.gfx_passthru_kind to IGD
>>
>> ----------------------------------------------------------------
>> Tiejun Chen (2):
>> libxl: introduce libxl__is_igd_vga_passthru
>> libxl: introduce gfx_passthru_kind
>>
>> tools/libxl/libxl_dm.c | 13 +++++++++++++
>> tools/libxl/libxl_internal.h | 2 ++
>> tools/libxl/libxl_pci.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> tools/libxl/libxl_types.idl | 6 ++++++
>> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
>> 5 files changed, 162 insertions(+), 2 deletions(-)
>>
>> Thanks
>> Tiejun
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru
2015-03-09 6:27 ` [Qemu-devel] " Chen, Tiejun
@ 2015-03-09 10:12 ` Wei Liu
2015-03-09 10:12 ` Wei Liu
1 sibling, 0 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-09 10:12 UTC (permalink / raw)
To: Chen, Tiejun
Cc: Wei Liu, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Mon, Mar 09, 2015 at 02:27:46PM +0800, Chen, Tiejun wrote:
[...]
> >>+
> >>+ FILE *f = fopen(pci_device_vendor_path, "r");
> >>+ if (!f) {
> >>+ LOGE(ERROR,
> >>+ "pci device "PCI_BDF" does not have vendor attribute",
> >>+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
> >>+ return 0xffff;
> >>+ }
> >>+ read_items = fscanf(f, "0x%lx\n", &pci_device_vendor);
>
> we have to refactor this as well,
>
> read_items = fscanf(f, "0x%hx\n", &pci_device_vendor);
>
> Right?
>
Sure. Just eliminate any warnings / errors.
Wei.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru
2015-03-09 6:27 ` [Qemu-devel] " Chen, Tiejun
2015-03-09 10:12 ` Wei Liu
@ 2015-03-09 10:12 ` Wei Liu
1 sibling, 0 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-09 10:12 UTC (permalink / raw)
To: Chen, Tiejun
Cc: Wei Liu, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Mon, Mar 09, 2015 at 02:27:46PM +0800, Chen, Tiejun wrote:
[...]
> >>+
> >>+ FILE *f = fopen(pci_device_vendor_path, "r");
> >>+ if (!f) {
> >>+ LOGE(ERROR,
> >>+ "pci device "PCI_BDF" does not have vendor attribute",
> >>+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
> >>+ return 0xffff;
> >>+ }
> >>+ read_items = fscanf(f, "0x%lx\n", &pci_device_vendor);
>
> we have to refactor this as well,
>
> read_items = fscanf(f, "0x%hx\n", &pci_device_vendor);
>
> Right?
>
Sure. Just eliminate any warnings / errors.
Wei.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-09 6:50 ` Chen, Tiejun
2015-03-09 10:13 ` Wei Liu
@ 2015-03-09 10:13 ` Wei Liu
1 sibling, 0 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-09 10:13 UTC (permalink / raw)
To: Chen, Tiejun
Cc: Wei Liu, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Mon, Mar 09, 2015 at 02:50:42PM +0800, Chen, Tiejun wrote:
> On 2015/3/6 20:59, Wei Liu wrote:
> >On Fri, Mar 06, 2015 at 05:18:36PM +0800, Chen, Tiejun wrote:
> >>On 2015/3/6 17:08, Tiejun Chen wrote:
> >>>Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
> >>> build_info.u.gfx_passthru_kind to DEFAULT
> >>> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
> >>> 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.
> >>>
> >>>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>>---
> >>> tools/libxl/libxl_dm.c | 13 +++++++++++++
> >>> tools/libxl/libxl_types.idl | 6 ++++++
> >>> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
> >>> 3 files changed, 36 insertions(+), 2 deletions(-)
> >>>
> >>>diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> >>>index 8599a6a..780dd2a 100644
> >>>--- a/tools/libxl/libxl_dm.c
> >>>+++ b/tools/libxl/libxl_dm.c
> >>
> >>Sorry, looks I'm missing to remove '-gfx_passthru' since this should be gone
> >>in the case of qemu upstream,
> >>
> >>@@ -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");
> >>
> >>I will fold this into next revision after this review.
> >>
> >
> >I think this change alone warrants a separate changeset. It should be
> >patch 0 of your series as a preparatory patch. Please remember to
> >elaborate in commit message why that hunk is not needed in upstream
> >QEMU.
> >
>
> What about this?
>
> "-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.
>
This looks good.
Wei.
> Thanks
> Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-09 6:50 ` Chen, Tiejun
@ 2015-03-09 10:13 ` Wei Liu
2015-03-09 10:13 ` Wei Liu
1 sibling, 0 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-09 10:13 UTC (permalink / raw)
To: Chen, Tiejun
Cc: Wei Liu, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Mon, Mar 09, 2015 at 02:50:42PM +0800, Chen, Tiejun wrote:
> On 2015/3/6 20:59, Wei Liu wrote:
> >On Fri, Mar 06, 2015 at 05:18:36PM +0800, Chen, Tiejun wrote:
> >>On 2015/3/6 17:08, Tiejun Chen wrote:
> >>>Although we already have 'gfx_passthru' in b_info, this doesn' 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 false and
> >>> build_info.u.gfx_passthru_kind to DEFAULT
> >>> gfx_passthru = "igd" => sets build_info.u.gfx_passthru to false
> >>> 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.
> >>>
> >>>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>>---
> >>> tools/libxl/libxl_dm.c | 13 +++++++++++++
> >>> tools/libxl/libxl_types.idl | 6 ++++++
> >>> tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
> >>> 3 files changed, 36 insertions(+), 2 deletions(-)
> >>>
> >>>diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> >>>index 8599a6a..780dd2a 100644
> >>>--- a/tools/libxl/libxl_dm.c
> >>>+++ b/tools/libxl/libxl_dm.c
> >>
> >>Sorry, looks I'm missing to remove '-gfx_passthru' since this should be gone
> >>in the case of qemu upstream,
> >>
> >>@@ -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");
> >>
> >>I will fold this into next revision after this review.
> >>
> >
> >I think this change alone warrants a separate changeset. It should be
> >patch 0 of your series as a preparatory patch. Please remember to
> >elaborate in commit message why that hunk is not needed in upstream
> >QEMU.
> >
>
> What about this?
>
> "-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.
>
This looks good.
Wei.
> Thanks
> Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-09 6:45 ` [Qemu-devel] " Chen, Tiejun
@ 2015-03-09 10:17 ` Wei Liu
2015-03-10 0:28 ` Chen, Tiejun
2015-03-10 0:28 ` [Qemu-devel] " Chen, Tiejun
2015-03-09 10:17 ` Wei Liu
` (2 subsequent siblings)
3 siblings, 2 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-09 10:17 UTC (permalink / raw)
To: Chen, Tiejun
Cc: Wei Liu, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Mon, Mar 09, 2015 at 02:45:36PM +0800, Chen, Tiejun wrote:
[...]
> >
> >>+ exit (1);
> >>+ }
> >>+ } 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)) {
> >
> >Ditto.
> >
> >>+ fprintf(stderr, "ERROR: invalid value \"%s\" for \"gfx_passthru\"\n",
> >
> >Ditto.
>
> So,
>
> @@ -1959,13 +1959,15 @@ skip_vfb:
> } 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);
> + fprintf(stderr, "ERROR: invalid value %ld for"
> + " \"gfx_passthru\"\n", l);
> exit (1);
> }
> } 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);
> + 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);
>
Your changes are mangled by your email client. But we don't normally
split the format string so that it's easier to grep.
What we normally do is
fprintf(stderr,
"FORMAT STRING",
a, b, c);
If format string still treads over 80 column it's fine. We can live
with that.
Wei.
> Thanks
> Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-09 6:45 ` [Qemu-devel] " Chen, Tiejun
2015-03-09 10:17 ` Wei Liu
@ 2015-03-09 10:17 ` Wei Liu
2015-03-11 11:23 ` Ian Campbell
2015-03-11 11:23 ` [Qemu-devel] " Ian Campbell
3 siblings, 0 replies; 37+ messages in thread
From: Wei Liu @ 2015-03-09 10:17 UTC (permalink / raw)
To: Chen, Tiejun
Cc: Wei Liu, ian.campbell, Ian.Jackson, qemu-devel, xen-devel,
stefano.stabellini
On Mon, Mar 09, 2015 at 02:45:36PM +0800, Chen, Tiejun wrote:
[...]
> >
> >>+ exit (1);
> >>+ }
> >>+ } 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)) {
> >
> >Ditto.
> >
> >>+ fprintf(stderr, "ERROR: invalid value \"%s\" for \"gfx_passthru\"\n",
> >
> >Ditto.
>
> So,
>
> @@ -1959,13 +1959,15 @@ skip_vfb:
> } 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);
> + fprintf(stderr, "ERROR: invalid value %ld for"
> + " \"gfx_passthru\"\n", l);
> exit (1);
> }
> } 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);
> + 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);
>
Your changes are mangled by your email client. But we don't normally
split the format string so that it's easier to grep.
What we normally do is
fprintf(stderr,
"FORMAT STRING",
a, b, c);
If format string still treads over 80 column it's fine. We can live
with that.
Wei.
> Thanks
> Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-09 10:17 ` Wei Liu
2015-03-10 0:28 ` Chen, Tiejun
@ 2015-03-10 0:28 ` Chen, Tiejun
1 sibling, 0 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-10 0:28 UTC (permalink / raw)
To: Wei Liu
Cc: qemu-devel, stefano.stabellini, Ian.Jackson, ian.campbell, xen-devel
On 2015/3/9 18:17, Wei Liu wrote:
> On Mon, Mar 09, 2015 at 02:45:36PM +0800, Chen, Tiejun wrote:
> [...]
>>>
>>>> + exit (1);
>>>> + }
>>>> + } 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)) {
>>>
>>> Ditto.
>>>
>>>> + fprintf(stderr, "ERROR: invalid value \"%s\" for \"gfx_passthru\"\n",
>>>
>>> Ditto.
>>
>> So,
>>
>> @@ -1959,13 +1959,15 @@ skip_vfb:
>> } 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);
>> + fprintf(stderr, "ERROR: invalid value %ld for"
>> + " \"gfx_passthru\"\n", l);
>> exit (1);
>> }
>> } 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);
>> + 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);
>>
>
> Your changes are mangled by your email client. But we don't normally
> split the format string so that it's easier to grep.
Yeah.
>
> What we normally do is
>
> fprintf(stderr,
> "FORMAT STRING",
> a, b, c);
>
> If format string still treads over 80 column it's fine. We can live
> with that.
>
Understood.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-09 10:17 ` Wei Liu
@ 2015-03-10 0:28 ` Chen, Tiejun
2015-03-10 0:28 ` [Qemu-devel] " Chen, Tiejun
1 sibling, 0 replies; 37+ messages in thread
From: Chen, Tiejun @ 2015-03-10 0:28 UTC (permalink / raw)
To: Wei Liu
Cc: qemu-devel, stefano.stabellini, Ian.Jackson, ian.campbell, xen-devel
On 2015/3/9 18:17, Wei Liu wrote:
> On Mon, Mar 09, 2015 at 02:45:36PM +0800, Chen, Tiejun wrote:
> [...]
>>>
>>>> + exit (1);
>>>> + }
>>>> + } 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)) {
>>>
>>> Ditto.
>>>
>>>> + fprintf(stderr, "ERROR: invalid value \"%s\" for \"gfx_passthru\"\n",
>>>
>>> Ditto.
>>
>> So,
>>
>> @@ -1959,13 +1959,15 @@ skip_vfb:
>> } 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);
>> + fprintf(stderr, "ERROR: invalid value %ld for"
>> + " \"gfx_passthru\"\n", l);
>> exit (1);
>> }
>> } 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);
>> + 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);
>>
>
> Your changes are mangled by your email client. But we don't normally
> split the format string so that it's easier to grep.
Yeah.
>
> What we normally do is
>
> fprintf(stderr,
> "FORMAT STRING",
> a, b, c);
>
> If format string still treads over 80 column it's fine. We can live
> with that.
>
Understood.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-09 6:45 ` [Qemu-devel] " Chen, Tiejun
` (2 preceding siblings ...)
2015-03-11 11:23 ` Ian Campbell
@ 2015-03-11 11:23 ` Ian Campbell
3 siblings, 0 replies; 37+ messages in thread
From: Ian Campbell @ 2015-03-11 11:23 UTC (permalink / raw)
To: Chen, Tiejun
Cc: Ian.Jackson, Wei Liu, qemu-devel, stefano.stabellini, xen-devel
On Mon, 2015-03-09 at 14:45 +0800, Chen, Tiejun wrote:
> On 2015/3/6 20:55, Wei Liu wrote:
> > On Fri, Mar 06, 2015 at 05:08:23PM +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 false and
> > ^^^^^
> > true?
>
> I just simply pick up this from Campbell's comment but I agree you.
Sorry! Wei is right.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] libxl: introduce gfx_passthru_kind
2015-03-09 6:45 ` [Qemu-devel] " Chen, Tiejun
2015-03-09 10:17 ` Wei Liu
2015-03-09 10:17 ` Wei Liu
@ 2015-03-11 11:23 ` Ian Campbell
2015-03-11 11:23 ` [Qemu-devel] " Ian Campbell
3 siblings, 0 replies; 37+ messages in thread
From: Ian Campbell @ 2015-03-11 11:23 UTC (permalink / raw)
To: Chen, Tiejun
Cc: Ian.Jackson, Wei Liu, qemu-devel, stefano.stabellini, xen-devel
On Mon, 2015-03-09 at 14:45 +0800, Chen, Tiejun wrote:
> On 2015/3/6 20:55, Wei Liu wrote:
> > On Fri, Mar 06, 2015 at 05:08:23PM +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 false and
> > ^^^^^
> > true?
>
> I just simply pick up this from Campbell's comment but I agree you.
Sorry! Wei is right.
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2015-03-11 11:23 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-06 9:08 [Qemu-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream Tiejun Chen
2015-03-06 9:08 ` [Qemu-devel] [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru Tiejun Chen
2015-03-06 12:40 ` Wei Liu
2015-03-06 12:40 ` Wei Liu
2015-03-09 6:27 ` Chen, Tiejun
2015-03-09 6:27 ` [Qemu-devel] " Chen, Tiejun
2015-03-09 10:12 ` Wei Liu
2015-03-09 10:12 ` Wei Liu
2015-03-06 9:08 ` Tiejun Chen
2015-03-06 9:08 ` [Qemu-devel] [PATCH 2/2] libxl: introduce gfx_passthru_kind Tiejun Chen
2015-03-06 9:18 ` Chen, Tiejun
2015-03-06 9:18 ` Chen, Tiejun
2015-03-06 12:59 ` Wei Liu
2015-03-06 12:59 ` Wei Liu
2015-03-09 6:50 ` Chen, Tiejun
2015-03-09 10:13 ` Wei Liu
2015-03-09 10:13 ` Wei Liu
2015-03-09 6:50 ` Chen, Tiejun
2015-03-06 12:55 ` Wei Liu
2015-03-09 6:45 ` Chen, Tiejun
2015-03-09 6:45 ` [Qemu-devel] " Chen, Tiejun
2015-03-09 10:17 ` Wei Liu
2015-03-10 0:28 ` Chen, Tiejun
2015-03-10 0:28 ` [Qemu-devel] " Chen, Tiejun
2015-03-09 10:17 ` Wei Liu
2015-03-11 11:23 ` Ian Campbell
2015-03-11 11:23 ` [Qemu-devel] " Ian Campbell
2015-03-06 12:55 ` Wei Liu
2015-03-06 9:08 ` Tiejun Chen
2015-03-06 12:28 ` [Qemu-devel] [PATCH 0/2] libxl: try to support IGD passthrough for qemu upstream Wei Liu
2015-03-06 12:28 ` Wei Liu
2015-03-09 5:32 ` Chen, Tiejun
2015-03-09 5:32 ` [Qemu-devel] " Chen, Tiejun
2015-03-06 15:53 ` [Qemu-devel] [Xen-devel] " Konrad Rzeszutek Wilk
2015-03-09 6:58 ` Chen, Tiejun
2015-03-09 6:58 ` [Qemu-devel] [Xen-devel] " Chen, Tiejun
2015-03-06 15:53 ` Konrad Rzeszutek Wilk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.