* [v3][PATCH 2/5] xen, gfx passthrough: create intel isa bridge
2014-05-26 9:36 [v3][PATCH 0/8] " Tiejun Chen
@ 2014-05-26 9:36 ` Tiejun Chen
0 siblings, 0 replies; 28+ messages in thread
From: Tiejun Chen @ 2014-05-26 9:36 UTC (permalink / raw)
To: nthony.perard, stefano.stabellini, mst, Kelly.Zytaruk
Cc: peter.maydell, xen-devel, allen.m.kay, qemu-devel, nthony,
yang.z.zhang
ISA bridge is needed since Intel gfx drive will probe it instead
of Dev31:Fun0 to make graphics device passthrough work easy for VMM, that
only need to expose ISA bridge to let driver know the real hardware underneath.
The original patch is from Allen Kay [allen.m.kay@intel.com]
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Cc: Allen Kay <allen.m.kay@intel.com>
---
v3:
* Fix some typos.
* Improve some return paths.
v2:
* Nothing is changed.
hw/xen/xen_pt_graphics.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index e63bd6f..51b174f 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -230,3 +230,66 @@ out:
g_free(bios);
return rc;
}
+
+static uint32_t isa_bridge_read_config(PCIDevice *d, uint32_t addr, int len)
+{
+ return pci_default_read_config(d, addr, len);
+}
+
+static void isa_bridge_write_config(PCIDevice *d, uint32_t addr, uint32_t v,
+ int len)
+{
+ pci_default_write_config(d, addr, v, len);
+
+ return;
+}
+
+static void isa_bridge_class_init(ObjectClass *klass, void *data)
+{
+ PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+ k->config_read = isa_bridge_read_config;
+ k->config_write = isa_bridge_write_config;
+
+ return;
+};
+
+typedef struct {
+ PCIDevice dev;
+} ISABridgeState;
+
+static TypeInfo isa_bridge_info = {
+ .name = "intel-pch-isa-bridge",
+ .parent = TYPE_PCI_DEVICE,
+ .instance_size = sizeof(ISABridgeState),
+ .class_init = isa_bridge_class_init,
+};
+
+static void xen_pt_graphics_register_types(void)
+{
+ type_register_static(&isa_bridge_info);
+}
+
+type_init(xen_pt_graphics_register_types)
+
+static int create_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
+{
+ struct PCIDevice *dev;
+
+ char rid;
+
+ dev = pci_create(bus, PCI_DEVFN(0x1f, 0), "intel-pch-isa-bridge");
+
+ qdev_init_nofail(&dev->qdev);
+
+ pci_config_set_vendor_id(dev->config, hdev->vendor_id);
+ pci_config_set_device_id(dev->config, hdev->device_id);
+
+ xen_host_pci_get_block(hdev, PCI_REVISION_ID, (uint8_t *)&rid, 1);
+
+ pci_config_set_revision(dev->config, rid);
+ pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_ISA);
+
+ XEN_PT_LOG(dev, "Intel PCH ISA bridge created.\n");
+ return 0;
+}
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [v3][PATCH 0/5] xen: add Intel IGD passthrough support
@ 2014-05-26 9:43 Tiejun Chen
2014-05-26 9:43 ` [v3][PATCH 1/5] xen, gfx passthrough: basic graphics " Tiejun Chen
` (5 more replies)
0 siblings, 6 replies; 28+ messages in thread
From: Tiejun Chen @ 2014-05-26 9:43 UTC (permalink / raw)
To: anthony.perard, stefano.stabellini, mst, Kelly.Zytaruk
Cc: peter.maydell, xen-devel, allen.m.kay, qemu-devel, anthony,
yang.z.zhang
v3:
* In this case, as we discussed we will give priority to devices to
reserve a specific devfn by passing
"device_model_args_hvm = ['-device', 'xen-platform,addr=0x3']" and
"vga=none", so withdraw patch #1, #2 and #4.
* Fix some typos.
* Add more comments to make that readable.
* To unmap igd_opregion when call xen_pt_unregister_vga_regions().
* Improve some return paths.
* Force to convert igd_guest/host_opoegion as an unsigned long type
while calling xc_domain_memory_mapping().
* We need to map 3 pages for opregion as hvmloader set.
v2:
* rebase on current qemu tree.
* retrieve VGA bios from sysfs properly.
* redefine some function name.
* introduce bitmap to manage registe/runregister pci dev, and provide
a common way to reserve some specific devfn.
* introduce is_igd_passthrough() to make sure we touch physical host
bridge only in IGD case.
* We should return zero as an invalid address value while calling
igd_read_opregion().
Additionally, now its also not necessary to recompile seabios with some
extra steps like v1.
The following patches are ported partially from Xen Qemu-traditional
branch which are adding Intel IGD passthrough supporting to Qemu upstream.
To pass through IGD to guest, user need to add following lines in Xen config
file:
gfx_passthru=1
pci=['00:02.0 <at> 2']
Now successfully boot Ubuntu 14.04 guests with IGD assigned in Haswell
desktop with Latest Xen + Qemu upstream.
----------------------------------------------------------------
Tiejun Chen (2):
xen, gfx passthrough: create intel isa bridge
xen, gfx passthrough: create host bridge to passthrough
Yang Zhang (3):
xen, gfx passthrough: basic graphics passthrough support
xen, gfx passthrough: support Intel IGD passthrough with VT-D
xen, gfx passthrough: add opregion mapping
hw/pci-host/piix.c | 56 +++++++++++++-
hw/xen/Makefile.objs | 2 +-
hw/xen/xen-host-pci-device.c | 5 ++
hw/xen/xen-host-pci-device.h | 1 +
hw/xen/xen_pt.c | 10 +++
hw/xen/xen_pt.h | 12 ++-
hw/xen/xen_pt_config_init.c | 50 ++++++++++++-
hw/xen/xen_pt_graphics.c | 518 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
qemu-options.hx | 9 +++
vl.c | 11 ++-
10 files changed, 668 insertions(+), 6 deletions(-)
create mode 100644 hw/xen/xen_pt_graphics.c
Thanks
Tiejun
^ permalink raw reply [flat|nested] 28+ messages in thread
* [v3][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support
2014-05-26 9:43 [v3][PATCH 0/5] xen: add Intel IGD passthrough support Tiejun Chen
@ 2014-05-26 9:43 ` Tiejun Chen
2014-05-27 14:54 ` [Xen-devel] " Konrad Rzeszutek Wilk
2014-05-27 17:47 ` Stefano Stabellini
2014-05-26 9:43 ` [v3][PATCH 2/5] xen, gfx passthrough: create intel isa bridge Tiejun Chen
` (4 subsequent siblings)
5 siblings, 2 replies; 28+ messages in thread
From: Tiejun Chen @ 2014-05-26 9:43 UTC (permalink / raw)
To: anthony.perard, stefano.stabellini, mst, Kelly.Zytaruk
Cc: peter.maydell, xen-devel, allen.m.kay, qemu-devel, anthony,
yang.z.zhang
basic gfx passthrough support:
- add a vga type for gfx passthrough
- retrieve VGA bios from sysfs, then load it to guest 0xC0000
- register/unregister legacy VGA I/O ports and MMIOs for passthroughed gfx
The original patch is from Weidong Han <weidong.han@intel.com>
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Cc: Weidong Han <weidong.han@intel.com>
---
v3:
* Fix some typos.
* Add more comments to make that readable.
* Improve some return paths.
v2:
* retrieve VGA bios from sysfs properly.
* redefine some function name.
hw/xen/Makefile.objs | 2 +-
hw/xen/xen-host-pci-device.c | 5 +
hw/xen/xen-host-pci-device.h | 1 +
hw/xen/xen_pt.c | 10 ++
hw/xen/xen_pt.h | 4 +
hw/xen/xen_pt_graphics.c | 232 +++++++++++++++++++++++++++++++++++++++++++
qemu-options.hx | 9 ++
vl.c | 11 +-
8 files changed, 272 insertions(+), 2 deletions(-)
create mode 100644 hw/xen/xen_pt_graphics.c
diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
index a0ca0aa..77b7dae 100644
--- a/hw/xen/Makefile.objs
+++ b/hw/xen/Makefile.objs
@@ -2,4 +2,4 @@
common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
-obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o
+obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o xen_pt_graphics.o
diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
index 743b37b..a54b7de 100644
--- a/hw/xen/xen-host-pci-device.c
+++ b/hw/xen/xen-host-pci-device.c
@@ -376,6 +376,11 @@ int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
goto error;
}
d->irq = v;
+ rc = xen_host_pci_get_hex_value(d, "class", &v);
+ if (rc) {
+ goto error;
+ }
+ d->class_code = v;
d->is_virtfn = xen_host_pci_dev_is_virtfn(d);
return 0;
diff --git a/hw/xen/xen-host-pci-device.h b/hw/xen/xen-host-pci-device.h
index c2486f0..f1e1c30 100644
--- a/hw/xen/xen-host-pci-device.h
+++ b/hw/xen/xen-host-pci-device.h
@@ -25,6 +25,7 @@ typedef struct XenHostPCIDevice {
uint16_t vendor_id;
uint16_t device_id;
+ uint32_t class_code;
int irq;
XenHostPCIIORegion io_regions[PCI_NUM_REGIONS - 1];
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index be4220b..a0113ea 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -450,6 +450,7 @@ static int xen_pt_register_regions(XenPCIPassthroughState *s)
d->rom.size, d->rom.base_addr);
}
+ xen_pt_register_vga_regions(d);
return 0;
}
@@ -470,6 +471,8 @@ static void xen_pt_unregister_regions(XenPCIPassthroughState *s)
if (d->rom.base_addr && d->rom.size) {
memory_region_destroy(&s->rom);
}
+
+ xen_pt_unregister_vga_regions(d);
}
/* region mapping */
@@ -693,6 +696,13 @@ static int xen_pt_initfn(PCIDevice *d)
/* Handle real device's MMIO/PIO BARs */
xen_pt_register_regions(s);
+ /* Setup VGA bios for passthroughed gfx */
+ if (xen_pt_setup_vga(&s->real_device) < 0) {
+ XEN_PT_ERR(d, "Setup VGA BIOS of passthroughed gfx failed!\n");
+ xen_host_pci_device_put(&s->real_device);
+ return -1;
+ }
+
/* reinitialize each config register to be emulated */
if (xen_pt_config_init(s)) {
XEN_PT_ERR(d, "PCI Config space initialisation failed.\n");
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 942dc60..4d3a18d 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -298,5 +298,9 @@ static inline bool xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
return s->msix && s->msix->bar_index == bar;
}
+extern int xen_has_gfx_passthru;
+int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
+int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
+int xen_pt_setup_vga(XenHostPCIDevice *dev);
#endif /* !XEN_PT_H */
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
new file mode 100644
index 0000000..e63bd6f
--- /dev/null
+++ b/hw/xen/xen_pt_graphics.c
@@ -0,0 +1,232 @@
+/*
+ * graphics passthrough
+ */
+#include "xen_pt.h"
+#include "xen-host-pci-device.h"
+#include "hw/xen/xen_backend.h"
+
+static int is_vga_passthrough(XenHostPCIDevice *dev)
+{
+ return (xen_has_gfx_passthru
+ && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
+}
+
+typedef struct VGARegion {
+ int type; /* Memory or port I/O */
+ uint64_t guest_base_addr;
+ uint64_t machine_base_addr;
+ uint64_t size; /* size of the region */
+ int rc;
+} VGARegion;
+
+#define IORESOURCE_IO 0x00000100
+#define IORESOURCE_MEM 0x00000200
+
+static struct VGARegion vga_args[] = {
+ {
+ .type = IORESOURCE_IO,
+ .guest_base_addr = 0x3B0,
+ .machine_base_addr = 0x3B0,
+ .size = 0xC,
+ .rc = -1,
+ },
+ {
+ .type = IORESOURCE_IO,
+ .guest_base_addr = 0x3C0,
+ .machine_base_addr = 0x3C0,
+ .size = 0x20,
+ .rc = -1,
+ },
+ {
+ .type = IORESOURCE_MEM,
+ .guest_base_addr = 0xa0000 >> XC_PAGE_SHIFT,
+ .machine_base_addr = 0xa0000 >> XC_PAGE_SHIFT,
+ .size = 0x20,
+ .rc = -1,
+ },
+};
+
+/*
+ * register VGA resources for the domain with assigned gfx
+ */
+int xen_pt_register_vga_regions(XenHostPCIDevice *dev)
+{
+ int i = 0;
+
+ if (!is_vga_passthrough(dev)) {
+ return -1;
+ }
+
+ for (i = 0 ; i < ARRAY_SIZE(vga_args); i++) {
+ if (vga_args[i].type == IORESOURCE_IO) {
+ vga_args[i].rc = xc_domain_ioport_mapping(xen_xc, xen_domid,
+ vga_args[i].guest_base_addr,
+ vga_args[i].machine_base_addr,
+ vga_args[i].size, DPCI_ADD_MAPPING);
+ } else {
+ vga_args[i].rc = xc_domain_memory_mapping(xen_xc, xen_domid,
+ vga_args[i].guest_base_addr,
+ vga_args[i].machine_base_addr,
+ vga_args[i].size, DPCI_ADD_MAPPING);
+ }
+
+ if (vga_args[i].rc) {
+ XEN_PT_ERR(NULL, "VGA %s mapping failed! (rc: %i)\n",
+ vga_args[i].type == IORESOURCE_IO ? "ioport" : "memory",
+ vga_args[i].rc);
+ return vga_args[i].rc;
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * unregister VGA resources for the domain with assigned gfx
+ */
+int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
+{
+ int i = 0;
+
+ if (!is_vga_passthrough(dev)) {
+ return -1;
+ }
+
+ for (i = 0 ; i < ARRAY_SIZE(vga_args); i++) {
+ if (vga_args[i].type == IORESOURCE_IO) {
+ vga_args[i].rc = xc_domain_ioport_mapping(xen_xc, xen_domid,
+ vga_args[i].guest_base_addr,
+ vga_args[i].machine_base_addr,
+ vga_args[i].size, DPCI_REMOVE_MAPPING);
+ } else {
+ vga_args[i].rc = xc_domain_memory_mapping(xen_xc, xen_domid,
+ vga_args[i].guest_base_addr,
+ vga_args[i].machine_base_addr,
+ vga_args[i].size, DPCI_REMOVE_MAPPING);
+ }
+
+ if (vga_args[i].rc) {
+ XEN_PT_ERR(NULL, "VGA %s unmapping failed! (rc: %i)\n",
+ vga_args[i].type == IORESOURCE_IO ? "ioport" : "memory",
+ vga_args[i].rc);
+ return vga_args[i].rc;
+ }
+ }
+
+ return 0;
+}
+
+static int get_vgabios(unsigned char *buf, XenHostPCIDevice *dev)
+{
+ char rom_file[64];
+ FILE *fp;
+ uint8_t val;
+ struct stat st;
+ uint16_t magic = 0;
+ int ret = 0;
+
+ snprintf(rom_file, sizeof(rom_file),
+ "/sys/bus/pci/devices/%04x:%02x:%02x.%d/rom",
+ dev->domain, dev->bus, dev->dev,
+ dev->func);
+
+ if (stat(rom_file, &st)) {
+ return -ENODEV;
+ }
+
+ if (access(rom_file, F_OK)) {
+ XEN_PT_ERR(NULL, "pci-assign: Insufficient privileges for %s",
+ rom_file);
+ return -ENODEV;
+ }
+
+ /* Write "1" to the ROM file to enable it */
+ fp = fopen(rom_file, "r+");
+ if (fp == NULL) {
+ return -EACCES;
+ }
+ val = 1;
+ if (fwrite(&val, 1, 1, fp) != 1) {
+ XEN_PT_LOG("%s\n", "Failed to enable pci-sysfs rom file");
+ ret = -EIO;
+ goto close_rom;
+ }
+ fseek(fp, 0, SEEK_SET);
+
+ /*
+ * Check if it a real bios extension.
+ * The magic number is 0xAA55.
+ */
+ if (!fread(&magic, sizeof(magic), 1, fp)) {
+ XEN_PT_ERR(NULL, "VGA: can't get magic.\n");
+ ret = -ENODEV;
+ goto close_rom;
+ }
+ if (magic != 0xAA55) {
+ XEN_PT_ERR(NULL, "VGA: wrong magic %x.\n", magic);
+ ret = -ENODEV;
+ goto close_rom;
+ }
+ fseek(fp, 0, SEEK_SET);
+
+ if (!fread(buf, 1, st.st_size, fp)) {
+ XEN_PT_ERR(NULL, "VGA: pci-assign: Cannot read from host %s", rom_file);
+ XEN_PT_LOG(NULL, "VGA: Device option ROM contents are probably invalid "
+ "(check dmesg).\nSkip option ROM probe with rombar=0, "
+ "or load from file with romfile=\n");
+ }
+
+close_rom:
+ /* Write "0" to disable ROM */
+ fseek(fp, 0, SEEK_SET);
+ val = 0;
+ if (!fwrite(&val, 1, 1, fp)) {
+ ret = -EIO;
+ XEN_PT_LOG("%s\n", "Failed to disable pci-sysfs rom file");
+ }
+ fclose(fp);
+ return ret;
+}
+
+/* Allocated 128K for the vga bios */
+#define VGA_BIOS_DEFAULT_SIZE (0x20000)
+
+int xen_pt_setup_vga(XenHostPCIDevice *dev)
+{
+ unsigned char *bios = NULL;
+ int bios_size = 0;
+ char *c = NULL;
+ char checksum = 0;
+ int rc = 0;
+
+ if (!is_vga_passthrough(dev)) {
+ return rc;
+ }
+
+ bios = g_malloc0(VGA_BIOS_DEFAULT_SIZE);
+
+ bios_size = get_vgabios(bios, dev);
+ if (bios_size) {
+ XEN_PT_ERR(NULL, "VGA: Error %d getting VBIOS!\n", bios_size);
+ if (bios_size < 0) {
+ XEN_PT_ERR(NULL, "VGA: Error (%s).\n", strerror(bios_size));
+ }
+ rc = -1;
+ goto out;
+ }
+
+ /* Adjust the bios checksum */
+ for (c = (char *)bios; c < ((char *)bios + bios_size); c++) {
+ checksum += *c;
+ }
+ if (checksum) {
+ bios[bios_size - 1] -= checksum;
+ XEN_PT_LOG(NULL, "vga bios checksum is adjusted!\n");
+ }
+
+ /* Currently we fixed this address as a primary. */
+ cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
+out:
+ g_free(bios);
+ return rc;
+}
diff --git a/qemu-options.hx b/qemu-options.hx
index c2c0823..9cb324f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1053,6 +1053,15 @@ STEXI
Rotate graphical output some deg left (only PXA LCD).
ETEXI
+DEF("gfx_passthru", 0, QEMU_OPTION_gfx_passthru,
+ "-gfx_passthru enable Intel IGD passthrough by XEN\n",
+ QEMU_ARCH_ALL)
+STEXI
+@item -gfx_passthru
+@findex -gfx_passthru
+Enable Intel IGD passthrough by XEN
+ETEXI
+
DEF("vga", HAS_ARG, QEMU_OPTION_vga,
"-vga [std|cirrus|vmware|qxl|xenfb|tcx|cg3|none]\n"
" select video card type\n", QEMU_ARCH_ALL)
diff --git a/vl.c b/vl.c
index 673148e..c02207f 100644
--- a/vl.c
+++ b/vl.c
@@ -1438,6 +1438,13 @@ static void smp_parse(QemuOpts *opts)
}
+/* We still need this for compatibility with XEN. */
+int xen_has_gfx_passthru;
+static void xen_gfx_passthru(const char *optarg)
+{
+ xen_has_gfx_passthru = 1;
+}
+
static void configure_realtime(QemuOpts *opts)
{
bool enable_mlock;
@@ -2990,7 +2997,6 @@ int main(int argc, char **argv, char **envp)
const char *trace_file = NULL;
const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
1024 * 1024;
-
atexit(qemu_run_exit_notifiers);
error_set_progname(argv[0]);
qemu_init_exec_dir(argv[0]);
@@ -3957,6 +3963,9 @@ int main(int argc, char **argv, char **envp)
}
configure_msg(opts);
break;
+ case QEMU_OPTION_gfx_passthru:
+ xen_gfx_passthru(optarg);
+ break;
default:
os_parse_cmd_args(popt->index, optarg);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [v3][PATCH 2/5] xen, gfx passthrough: create intel isa bridge
2014-05-26 9:43 [v3][PATCH 0/5] xen: add Intel IGD passthrough support Tiejun Chen
2014-05-26 9:43 ` [v3][PATCH 1/5] xen, gfx passthrough: basic graphics " Tiejun Chen
@ 2014-05-26 9:43 ` Tiejun Chen
2014-05-27 14:55 ` [Xen-devel] " Konrad Rzeszutek Wilk
2014-05-26 9:43 ` [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D Tiejun Chen
` (3 subsequent siblings)
5 siblings, 1 reply; 28+ messages in thread
From: Tiejun Chen @ 2014-05-26 9:43 UTC (permalink / raw)
To: anthony.perard, stefano.stabellini, mst, Kelly.Zytaruk
Cc: peter.maydell, xen-devel, allen.m.kay, qemu-devel, anthony,
yang.z.zhang
ISA bridge is needed since Intel gfx drive will probe it instead
of Dev31:Fun0 to make graphics device passthrough work easy for VMM, that
only need to expose ISA bridge to let driver know the real hardware underneath.
The original patch is from Allen Kay [allen.m.kay@intel.com]
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Cc: Allen Kay <allen.m.kay@intel.com>
---
v3:
* Fix some typos.
* Improve some return paths.
v2:
* Nothing is changed.
hw/xen/xen_pt_graphics.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index e63bd6f..51b174f 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -230,3 +230,66 @@ out:
g_free(bios);
return rc;
}
+
+static uint32_t isa_bridge_read_config(PCIDevice *d, uint32_t addr, int len)
+{
+ return pci_default_read_config(d, addr, len);
+}
+
+static void isa_bridge_write_config(PCIDevice *d, uint32_t addr, uint32_t v,
+ int len)
+{
+ pci_default_write_config(d, addr, v, len);
+
+ return;
+}
+
+static void isa_bridge_class_init(ObjectClass *klass, void *data)
+{
+ PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+ k->config_read = isa_bridge_read_config;
+ k->config_write = isa_bridge_write_config;
+
+ return;
+};
+
+typedef struct {
+ PCIDevice dev;
+} ISABridgeState;
+
+static TypeInfo isa_bridge_info = {
+ .name = "intel-pch-isa-bridge",
+ .parent = TYPE_PCI_DEVICE,
+ .instance_size = sizeof(ISABridgeState),
+ .class_init = isa_bridge_class_init,
+};
+
+static void xen_pt_graphics_register_types(void)
+{
+ type_register_static(&isa_bridge_info);
+}
+
+type_init(xen_pt_graphics_register_types)
+
+static int create_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
+{
+ struct PCIDevice *dev;
+
+ char rid;
+
+ dev = pci_create(bus, PCI_DEVFN(0x1f, 0), "intel-pch-isa-bridge");
+
+ qdev_init_nofail(&dev->qdev);
+
+ pci_config_set_vendor_id(dev->config, hdev->vendor_id);
+ pci_config_set_device_id(dev->config, hdev->device_id);
+
+ xen_host_pci_get_block(hdev, PCI_REVISION_ID, (uint8_t *)&rid, 1);
+
+ pci_config_set_revision(dev->config, rid);
+ pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_ISA);
+
+ XEN_PT_LOG(dev, "Intel PCH ISA bridge created.\n");
+ return 0;
+}
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D
2014-05-26 9:43 [v3][PATCH 0/5] xen: add Intel IGD passthrough support Tiejun Chen
2014-05-26 9:43 ` [v3][PATCH 1/5] xen, gfx passthrough: basic graphics " Tiejun Chen
2014-05-26 9:43 ` [v3][PATCH 2/5] xen, gfx passthrough: create intel isa bridge Tiejun Chen
@ 2014-05-26 9:43 ` Tiejun Chen
2014-05-27 18:02 ` Stefano Stabellini
2014-05-26 9:43 ` [v3][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough Tiejun Chen
` (2 subsequent siblings)
5 siblings, 1 reply; 28+ messages in thread
From: Tiejun Chen @ 2014-05-26 9:43 UTC (permalink / raw)
To: anthony.perard, stefano.stabellini, mst, Kelly.Zytaruk
Cc: peter.maydell, xen-devel, allen.m.kay, qemu-devel, anthony,
yang.z.zhang
Some registers of Intel IGD are mapped in host bridge, so it needs to
passthrough these registers of physical host bridge to guest because
emulated host bridge in guest doesn't have these mappings.
The original patch is from Weidong Han < weidong.han @ intel.com >
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Cc:Weidong Han <weidong.han@intel.com>
---
v3:
* Improve comments to make that readable.
v2:
* To introduce is_igd_passthrough() to make sure we touch physical host bridge
only in IGD case.
hw/xen/xen_pt.h | 4 ++
hw/xen/xen_pt_graphics.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 163 insertions(+)
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 4d3a18d..507165c 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -302,5 +302,9 @@ extern int xen_has_gfx_passthru;
int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
int xen_pt_setup_vga(XenHostPCIDevice *dev);
+int pci_create_pch(PCIBus *bus);
+void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr,
+ uint32_t val, int len);
+uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len);
#endif /* !XEN_PT_H */
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 51b174f..72e30ac 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -4,6 +4,7 @@
#include "xen_pt.h"
#include "xen-host-pci-device.h"
#include "hw/xen/xen_backend.h"
+#include "hw/pci/pci_bus.h"
static int is_vga_passthrough(XenHostPCIDevice *dev)
{
@@ -293,3 +294,161 @@ static int create_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
XEN_PT_LOG(dev, "Intel PCH ISA bridge created.\n");
return 0;
}
+
+int pci_create_pch(PCIBus *bus)
+{
+ XenHostPCIDevice hdev;
+ int r = 0;
+
+ if (!xen_has_gfx_passthru) {
+ return -1;
+ }
+
+ r = xen_host_pci_device_get(&hdev, 0, 0, 0x1f, 0);
+ if (r) {
+ XEN_PT_ERR(NULL, "Failed to find Intel PCH on host\n");
+ goto err;
+ }
+
+ if (hdev.vendor_id == PCI_VENDOR_ID_INTEL) {
+ r = create_pch_isa_bridge(bus, &hdev);
+ if (r) {
+ XEN_PT_ERR(NULL, "Failed to create PCH ISA bridge.\n");
+ goto err;
+ }
+ }
+
+ xen_host_pci_device_put(&hdev);
+
+err:
+ return r;
+}
+
+/*
+ * Currently we just pass this physical host bridge for IGD, 00:02.0.
+ */
+static int is_igd_passthrough(PCIDevice *pci_dev)
+{
+ PCIDevice *f = pci_dev->bus->devices[PCI_DEVFN(2, 0)];
+ if (pci_dev->bus->devices[PCI_DEVFN(2, 0)]) {
+ XenPCIPassthroughState *s = DO_UPCAST(XenPCIPassthroughState, dev, f);
+ return (is_vga_passthrough(&s->real_device)
+ && (s->real_device.vendor_id == PCI_VENDOR_ID_INTEL));
+ } else {
+ return 0;
+ }
+}
+
+void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr,
+ uint32_t val, int len)
+{
+ XenHostPCIDevice dev;
+ int r;
+
+ /* IGD read/write is through the host bridge.
+ * ISA bridge is only for detect purpose. In i915 driver it will
+ * probe ISA bridge to discover the IGD, see comment in i915_drv.c:
+ * intel_detect_pch():
+ * The reason to probe ISA bridge instead of Dev31:Fun0 is to
+ * make graphics device passthrough work easy for VMM, that only
+ * need to expose ISA bridge to let driver know the real hardware
+ * underneath. This is a requirement from virtualization team.
+ */
+
+ assert(pci_dev->devfn == 0x00);
+
+ if (!is_igd_passthrough(pci_dev)) {
+ goto write_default;
+ }
+
+ switch (config_addr) {
+ case 0x58: /* PAVPC Offset */
+ break;
+ default:
+ /* Just sets the emulated values. */
+ goto write_default;
+ }
+
+ /* Host write */
+ r = xen_host_pci_device_get(&dev, 0, 0, 0, 0);
+ if (r) {
+ XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
+ abort();
+ }
+
+ r = xen_host_pci_set_block(&dev, config_addr, (uint8_t *)&val, len);
+ if (r) {
+ XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
+ abort();
+ }
+
+ xen_host_pci_device_put(&dev);
+
+ return;
+
+write_default:
+ pci_default_write_config(pci_dev, config_addr, val, len);
+}
+
+uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len)
+{
+ XenHostPCIDevice dev;
+ uint32_t val;
+ int r;
+
+ /* IGD read/write is through the host bridge.
+ * ISA bridge is only for detect purpose. In i915 driver it will
+ * probe ISA bridge to discover the IGD, see comment in i915_drv.c:
+ * intel_detect_pch():
+ * The reason to probe ISA bridge instead of Dev31:Fun0 is to
+ * make graphics device passthrough work easy for VMM, that only
+ * need to expose ISA bridge to let driver know the real hardware
+ * underneath. This is a requirement from virtualization team.
+ */
+ assert(pci_dev->devfn == 0x00);
+
+ if (!is_igd_passthrough(pci_dev)) {
+ goto read_default;
+ }
+
+ switch (config_addr) {
+ case 0x00: /* vendor id */
+ case 0x02: /* device id */
+ case 0x08: /* revision id */
+ case 0x2c: /* sybsystem vendor id */
+ case 0x2e: /* sybsystem id */
+ case 0x50: /* SNB: processor graphics control register */
+ case 0x52: /* processor graphics control register */
+ case 0xa0: /* top of memory */
+ case 0xb0: /* ILK: BSM: should read from dev 2 offset 0x5c */
+ case 0x58: /* SNB: PAVPC Offset */
+ case 0xa4: /* SNB: graphics base of stolen memory */
+ case 0xa8: /* SNB: base of GTT stolen memory */
+ break;
+ default:
+ /* Just gets the emulated values. */
+ goto read_default;
+ }
+
+ /* Host read */
+ r = xen_host_pci_device_get(&dev, 0, 0, 0, 0);
+ if (r) {
+ goto err_out;
+ }
+
+ r = xen_host_pci_get_block(&dev, config_addr, (uint8_t *)&val, len);
+ if (r) {
+ goto err_out;
+ }
+
+ xen_host_pci_device_put(&dev);
+
+ return val;
+
+read_default:
+ return pci_default_read_config(pci_dev, config_addr, len);
+
+err_out:
+ XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
+ return -1;
+}
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [v3][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough
2014-05-26 9:43 [v3][PATCH 0/5] xen: add Intel IGD passthrough support Tiejun Chen
` (2 preceding siblings ...)
2014-05-26 9:43 ` [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D Tiejun Chen
@ 2014-05-26 9:43 ` Tiejun Chen
2014-05-27 14:48 ` [Xen-devel] " Konrad Rzeszutek Wilk
2014-05-27 18:04 ` Stefano Stabellini
2014-05-26 9:43 ` [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping Tiejun Chen
2014-05-27 9:06 ` [Qemu-devel] [v3][PATCH 0/5] xen: add Intel IGD passthrough support Chen, Tiejun
5 siblings, 2 replies; 28+ messages in thread
From: Tiejun Chen @ 2014-05-26 9:43 UTC (permalink / raw)
To: anthony.perard, stefano.stabellini, mst, Kelly.Zytaruk
Cc: peter.maydell, xen-devel, allen.m.kay, qemu-devel, anthony,
yang.z.zhang
Implement that pci host bridge to specific to passthrough. Actually
thsi just inherit the standard one.
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
v3:
* Just fix this patch head description typo.
v2:
* New patch.
hw/pci-host/piix.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 54 insertions(+), 2 deletions(-)
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index ffdc853..8b11953 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -34,6 +34,7 @@
#include "sysemu/sysemu.h"
#include "hw/i386/ioapic.h"
#include "qapi/visitor.h"
+#include "hw/xen/xen_pt.h"
/*
* I440FX chipset data sheet.
@@ -95,6 +96,10 @@ typedef struct PIIX3State {
#define I440FX_PCI_DEVICE(obj) \
OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
+#define TYPE_I440FX_XEN_PCI_DEVICE "i440FX-xen"
+#define I440FX_XEN_PCI_DEVICE(obj) \
+ OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_XEN_PCI_DEVICE)
+
struct PCII440FXState {
/*< private >*/
PCIDevice parent_obj;
@@ -305,6 +310,16 @@ static int i440fx_initfn(PCIDevice *dev)
return 0;
}
+static int i440fx_xen_initfn(PCIDevice *dev)
+{
+ PCII440FXState *d = I440FX_XEN_PCI_DEVICE(dev);
+
+ dev->config[I440FX_SMRAM] = 0x02;
+
+ cpu_smm_register(&i440fx_set_smm, d);
+ return 0;
+}
+
PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
int *piix3_devfn,
ISABus **isa_bus, qemu_irq *pic,
@@ -333,8 +348,15 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
qdev_init_nofail(dev);
- d = pci_create_simple(b, 0, TYPE_I440FX_PCI_DEVICE);
- *pi440fx_state = I440FX_PCI_DEVICE(d);
+ if (xen_enabled()) {
+ d = pci_create_simple(b, 0, TYPE_I440FX_XEN_PCI_DEVICE);
+ *pi440fx_state = I440FX_XEN_PCI_DEVICE(d);
+ pci_create_pch(b);
+ } else {
+ d = pci_create_simple(b, 0, TYPE_I440FX_PCI_DEVICE);
+ *pi440fx_state = I440FX_PCI_DEVICE(d);
+ }
+
f = *pi440fx_state;
f->system_memory = address_space_mem;
f->pci_address_space = pci_address_space;
@@ -705,6 +727,35 @@ static const TypeInfo i440fx_info = {
.class_init = i440fx_class_init,
};
+static void i440fx_xen_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+ k->init = i440fx_xen_initfn;
+ k->config_write = igd_pci_write;
+ k->config_read = igd_pci_read;
+ k->vendor_id = PCI_VENDOR_ID_INTEL;
+ k->device_id = PCI_DEVICE_ID_INTEL_82441;
+ k->revision = 0x02;
+ k->class_id = PCI_CLASS_BRIDGE_ISA;
+ dc->desc = "XEN Host bridge";
+ dc->vmsd = &vmstate_i440fx;
+ /*
+ * PCI-facing part of the host bridge, not usable without the
+ * host-facing part, which can't be device_add'ed, yet.
+ */
+ dc->cannot_instantiate_with_device_add_yet = true;
+ dc->hotpluggable = false;
+}
+
+static const TypeInfo i440fx_xen_info = {
+ .name = TYPE_I440FX_XEN_PCI_DEVICE,
+ .parent = TYPE_PCI_DEVICE,
+ .instance_size = sizeof(PCII440FXState),
+ .class_init = i440fx_xen_class_init,
+};
+
static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
PCIBus *rootbus)
{
@@ -746,6 +797,7 @@ static const TypeInfo i440fx_pcihost_info = {
static void i440fx_register_types(void)
{
type_register_static(&i440fx_info);
+ type_register_static(&i440fx_xen_info);
type_register_static(&piix3_info);
type_register_static(&piix3_xen_info);
type_register_static(&i440fx_pcihost_info);
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping
2014-05-26 9:43 [v3][PATCH 0/5] xen: add Intel IGD passthrough support Tiejun Chen
` (3 preceding siblings ...)
2014-05-26 9:43 ` [v3][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough Tiejun Chen
@ 2014-05-26 9:43 ` Tiejun Chen
2014-05-27 17:35 ` Stefano Stabellini
2014-05-27 9:06 ` [Qemu-devel] [v3][PATCH 0/5] xen: add Intel IGD passthrough support Chen, Tiejun
5 siblings, 1 reply; 28+ messages in thread
From: Tiejun Chen @ 2014-05-26 9:43 UTC (permalink / raw)
To: anthony.perard, stefano.stabellini, mst, Kelly.Zytaruk
Cc: peter.maydell, xen-devel, allen.m.kay, qemu-devel, anthony,
yang.z.zhang
The OpRegion shouldn't be mapped 1:1 because the address in the host
can't be used in the guest directly.
This patch traps read and write access to the opregion of the Intel
GPU config space (offset 0xfc).
The original patch is from Jean Guyader <jean.guyader@eu.citrix.com>
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Cc: Jean Guyader <jean.guyader@eu.citrix.com>
---
v3:
* Fix some typos.
* Add more comments to make that readable.
* To unmap igd_opregion when call xen_pt_unregister_vga_regions().
* Improve some return paths.
* We need to map 3 pages for opregion as hvmloader set.
* Force to convert igd_guest/host_opoegion as an unsigned long type
while calling xc_domain_memory_mapping().
v2:
* We should return zero as an invalid address value while calling
igd_read_opregion().
hw/xen/xen_pt.h | 4 ++-
hw/xen/xen_pt_config_init.c | 50 ++++++++++++++++++++++++++++++++++-
hw/xen/xen_pt_graphics.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 116 insertions(+), 2 deletions(-)
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 507165c..25147cf 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -63,7 +63,7 @@ typedef int (*xen_pt_conf_byte_read)
#define XEN_PT_BAR_UNMAPPED (-1)
#define PCI_CAP_MAX 48
-
+#define PCI_INTEL_OPREGION 0xfc
typedef enum {
XEN_PT_GRP_TYPE_HARDWIRED = 0, /* 0 Hardwired reg group */
@@ -306,5 +306,7 @@ int pci_create_pch(PCIBus *bus);
void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr,
uint32_t val, int len);
uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len);
+uint32_t igd_read_opregion(XenPCIPassthroughState *s);
+void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
#endif /* !XEN_PT_H */
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index de9a20f..6eaaa9a 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -575,6 +575,22 @@ static int xen_pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s,
return 0;
}
+static int xen_pt_intel_opregion_read(XenPCIPassthroughState *s,
+ XenPTReg *cfg_entry,
+ uint32_t *value, uint32_t valid_mask)
+{
+ *value = igd_read_opregion(s);
+ return 0;
+}
+
+static int xen_pt_intel_opregion_write(XenPCIPassthroughState *s,
+ XenPTReg *cfg_entry, uint32_t *value,
+ uint32_t dev_value, uint32_t valid_mask)
+{
+ igd_write_opregion(s, *value);
+ return 0;
+}
+
/* Header Type0 reg static information table */
static XenPTRegInfo xen_pt_emu_reg_header0[] = {
/* Vendor ID reg */
@@ -1440,6 +1456,20 @@ static XenPTRegInfo xen_pt_emu_reg_msix[] = {
},
};
+static XenPTRegInfo xen_pt_emu_reg_igd_opregion[] = {
+ /* Intel IGFX OpRegion reg */
+ {
+ .offset = 0x0,
+ .size = 4,
+ .init_val = 0,
+ .no_wb = 1,
+ .u.dw.read = xen_pt_intel_opregion_read,
+ .u.dw.write = xen_pt_intel_opregion_write,
+ },
+ {
+ .size = 0,
+ },
+};
/****************************
* Capabilities
@@ -1677,6 +1707,14 @@ static const XenPTRegGroupInfo xen_pt_emu_reg_grps[] = {
.size_init = xen_pt_msix_size_init,
.emu_regs = xen_pt_emu_reg_msix,
},
+ /* Intel IGD Opregion group */
+ {
+ .grp_id = PCI_INTEL_OPREGION,
+ .grp_type = XEN_PT_GRP_TYPE_EMU,
+ .grp_size = 0x4,
+ .size_init = xen_pt_reg_grp_size_init,
+ .emu_regs = xen_pt_emu_reg_igd_opregion,
+ },
{
.grp_size = 0,
},
@@ -1806,7 +1844,8 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
uint32_t reg_grp_offset = 0;
XenPTRegGroup *reg_grp_entry = NULL;
- if (xen_pt_emu_reg_grps[i].grp_id != 0xFF) {
+ if (xen_pt_emu_reg_grps[i].grp_id != 0xFF
+ && xen_pt_emu_reg_grps[i].grp_id != PCI_INTEL_OPREGION) {
if (xen_pt_hide_dev_cap(&s->real_device,
xen_pt_emu_reg_grps[i].grp_id)) {
continue;
@@ -1819,6 +1858,15 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
}
}
+ /*
+ * By default we will trap up to 0x40 in the cfg space.
+ * If an intel device is pass through we need to trap 0xfc,
+ * therefore the size should be 0xff.
+ */
+ if (xen_pt_emu_reg_grps[i].grp_id == PCI_INTEL_OPREGION) {
+ reg_grp_offset = PCI_INTEL_OPREGION;
+ }
+
reg_grp_entry = g_new0(XenPTRegGroup, 1);
QLIST_INIT(®_grp_entry->reg_tbl_list);
QLIST_INSERT_HEAD(&s->reg_grps, reg_grp_entry, entries);
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 72e30ac..ee6fec0 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -6,6 +6,9 @@
#include "hw/xen/xen_backend.h"
#include "hw/pci/pci_bus.h"
+static unsigned long igd_guest_opregion;
+static unsigned long igd_host_opregion;
+
static int is_vga_passthrough(XenHostPCIDevice *dev)
{
return (xen_has_gfx_passthru
@@ -88,6 +91,7 @@ int xen_pt_register_vga_regions(XenHostPCIDevice *dev)
int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
{
int i = 0;
+ int ret = 0;
if (!is_vga_passthrough(dev)) {
return -1;
@@ -114,6 +118,17 @@ int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
}
}
+ if (igd_guest_opregion) {
+ ret = xc_domain_memory_mapping(xen_xc, xen_domid,
+ (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT),
+ (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
+ 3,
+ DPCI_REMOVE_MAPPING);
+ if (ret) {
+ return ret;
+ }
+ }
+
return 0;
}
@@ -452,3 +467,52 @@ err_out:
XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
return -1;
}
+
+uint32_t igd_read_opregion(XenPCIPassthroughState *s)
+{
+ uint32_t val = 0;
+
+ if (igd_guest_opregion == 0) {
+ return val;
+ }
+
+ val = igd_guest_opregion;
+
+ XEN_PT_LOG(&s->dev, "Read opregion val=%x\n", val);
+ return val;
+}
+
+void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val)
+{
+ int ret;
+
+ if (igd_guest_opregion) {
+ XEN_PT_LOG(&s->dev, "opregion register already been set, ignoring %x\n",
+ val);
+ return;
+ }
+
+ xen_host_pci_get_block(&s->real_device, PCI_INTEL_OPREGION,
+ (uint8_t *)&igd_host_opregion, 4);
+ igd_guest_opregion = (unsigned long)(val & ~0xfff)
+ | (igd_host_opregion & 0xfff);
+
+ ret = xc_domain_memory_mapping(xen_xc, xen_domid,
+ (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT),
+ (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
+ 3,
+ DPCI_ADD_MAPPING);
+
+ if (ret) {
+ XEN_PT_ERR(&s->dev, "[%d]:Can't map IGD host opregion:0x%lx to"
+ " guest opregion:0x%lx.\n", ret,
+ (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
+ (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT));
+ igd_guest_opregion = 0;
+ return;
+ }
+
+ XEN_PT_LOG(&s->dev, "Map OpRegion: 0x%lx -> 0x%lx\n",
+ (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
+ (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT));
+}
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [v3][PATCH 0/5] xen: add Intel IGD passthrough support
2014-05-26 9:43 [v3][PATCH 0/5] xen: add Intel IGD passthrough support Tiejun Chen
` (4 preceding siblings ...)
2014-05-26 9:43 ` [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping Tiejun Chen
@ 2014-05-27 9:06 ` Chen, Tiejun
5 siblings, 0 replies; 28+ messages in thread
From: Chen, Tiejun @ 2014-05-27 9:06 UTC (permalink / raw)
To: anthony.perard@citrix.com, stefano.stabellini@eu.citrix.com,
mst@redhat.com, Kelly.Zytaruk@amd.com
Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com,
Kay, Allen M, qemu-devel@nongnu.org, anthony@codemonkey.ws,
Zhang, Yang Z
Any further comments?
Thanks
Tiejun
> -----Original Message-----
> From: qemu-devel-bounces+tiejun.chen=intel.com@nongnu.org
> [mailto:qemu-devel-bounces+tiejun.chen=intel.com@nongnu.org] On Behalf Of
> Tiejun Chen
> Sent: Monday, May 26, 2014 5:43 PM
> To: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com;
> mst@redhat.com; Kelly.Zytaruk@amd.com
> Cc: peter.maydell@linaro.org; xen-devel@lists.xensource.com; Kay, Allen M;
> qemu-devel@nongnu.org; anthony@codemonkey.ws; Zhang, Yang Z
> Subject: [Qemu-devel] [v3][PATCH 0/5] xen: add Intel IGD passthrough support
>
> v3:
>
> * In this case, as we discussed we will give priority to devices to
> reserve a specific devfn by passing
> "device_model_args_hvm = ['-device', 'xen-platform,addr=0x3']" and
> "vga=none", so withdraw patch #1, #2 and #4.
> * Fix some typos.
> * Add more comments to make that readable.
> * To unmap igd_opregion when call xen_pt_unregister_vga_regions().
> * Improve some return paths.
> * Force to convert igd_guest/host_opoegion as an unsigned long type
> while calling xc_domain_memory_mapping().
> * We need to map 3 pages for opregion as hvmloader set.
>
> v2:
>
> * rebase on current qemu tree.
> * retrieve VGA bios from sysfs properly.
> * redefine some function name.
> * introduce bitmap to manage registe/runregister pci dev, and provide
> a common way to reserve some specific devfn.
> * introduce is_igd_passthrough() to make sure we touch physical host
> bridge only in IGD case.
> * We should return zero as an invalid address value while calling
> igd_read_opregion().
>
> Additionally, now its also not necessary to recompile seabios with some extra
> steps like v1.
>
>
> The following patches are ported partially from Xen Qemu-traditional branch
> which are adding Intel IGD passthrough supporting to Qemu upstream.
>
> To pass through IGD to guest, user need to add following lines in Xen config
> file:
> gfx_passthru=1
> pci=['00:02.0 <at> 2']
>
> Now successfully boot Ubuntu 14.04 guests with IGD assigned in Haswell
> desktop with Latest Xen + Qemu upstream.
>
> ----------------------------------------------------------------
> Tiejun Chen (2):
> xen, gfx passthrough: create intel isa bridge
> xen, gfx passthrough: create host bridge to passthrough
>
> Yang Zhang (3):
> xen, gfx passthrough: basic graphics passthrough support
> xen, gfx passthrough: support Intel IGD passthrough with VT-D
> xen, gfx passthrough: add opregion mapping
>
> hw/pci-host/piix.c | 56 +++++++++++++-
> hw/xen/Makefile.objs | 2 +-
> hw/xen/xen-host-pci-device.c | 5 ++
> hw/xen/xen-host-pci-device.h | 1 +
> hw/xen/xen_pt.c | 10 +++
> hw/xen/xen_pt.h | 12 ++-
> hw/xen/xen_pt_config_init.c | 50 ++++++++++++-
> hw/xen/xen_pt_graphics.c | 518
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++
> qemu-options.hx | 9 +++
> vl.c | 11 ++-
> 10 files changed, 668 insertions(+), 6 deletions(-) create mode 100644
> hw/xen/xen_pt_graphics.c
>
> Thanks
> Tiejun
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xen-devel] [v3][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough
2014-05-26 9:43 ` [v3][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough Tiejun Chen
@ 2014-05-27 14:48 ` Konrad Rzeszutek Wilk
2014-05-28 1:10 ` Chen, Tiejun
2014-05-27 18:04 ` Stefano Stabellini
1 sibling, 1 reply; 28+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-05-27 14:48 UTC (permalink / raw)
To: Tiejun Chen
Cc: peter.maydell, xen-devel, mst, stefano.stabellini, allen.m.kay,
Kelly.Zytaruk, qemu-devel, yang.z.zhang, anthony, anthony.perard
On Mon, May 26, 2014 at 05:43:08PM +0800, Tiejun Chen wrote:
> Implement that pci host bridge to specific to passthrough. Actually
> thsi just inherit the standard one.
s/thsi/this
>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
> v3:
>
> * Just fix this patch head description typo.
>
> v2:
>
> * New patch.
>
> hw/pci-host/piix.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index ffdc853..8b11953 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -34,6 +34,7 @@
> #include "sysemu/sysemu.h"
> #include "hw/i386/ioapic.h"
> #include "qapi/visitor.h"
> +#include "hw/xen/xen_pt.h"
>
> /*
> * I440FX chipset data sheet.
> @@ -95,6 +96,10 @@ typedef struct PIIX3State {
> #define I440FX_PCI_DEVICE(obj) \
> OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>
> +#define TYPE_I440FX_XEN_PCI_DEVICE "i440FX-xen"
> +#define I440FX_XEN_PCI_DEVICE(obj) \
> + OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_XEN_PCI_DEVICE)
> +
> struct PCII440FXState {
> /*< private >*/
> PCIDevice parent_obj;
> @@ -305,6 +310,16 @@ static int i440fx_initfn(PCIDevice *dev)
> return 0;
> }
>
> +static int i440fx_xen_initfn(PCIDevice *dev)
> +{
> + PCII440FXState *d = I440FX_XEN_PCI_DEVICE(dev);
> +
> + dev->config[I440FX_SMRAM] = 0x02;
> +
> + cpu_smm_register(&i440fx_set_smm, d);
> + return 0;
> +}
> +
> PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> int *piix3_devfn,
> ISABus **isa_bus, qemu_irq *pic,
> @@ -333,8 +348,15 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
> qdev_init_nofail(dev);
>
> - d = pci_create_simple(b, 0, TYPE_I440FX_PCI_DEVICE);
> - *pi440fx_state = I440FX_PCI_DEVICE(d);
> + if (xen_enabled()) {
> + d = pci_create_simple(b, 0, TYPE_I440FX_XEN_PCI_DEVICE);
> + *pi440fx_state = I440FX_XEN_PCI_DEVICE(d);
> + pci_create_pch(b);
> + } else {
> + d = pci_create_simple(b, 0, TYPE_I440FX_PCI_DEVICE);
> + *pi440fx_state = I440FX_PCI_DEVICE(d);
> + }
> +
> f = *pi440fx_state;
> f->system_memory = address_space_mem;
> f->pci_address_space = pci_address_space;
> @@ -705,6 +727,35 @@ static const TypeInfo i440fx_info = {
> .class_init = i440fx_class_init,
> };
>
> +static void i440fx_xen_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> + k->init = i440fx_xen_initfn;
> + k->config_write = igd_pci_write;
> + k->config_read = igd_pci_read;
> + k->vendor_id = PCI_VENDOR_ID_INTEL;
> + k->device_id = PCI_DEVICE_ID_INTEL_82441;
> + k->revision = 0x02;
> + k->class_id = PCI_CLASS_BRIDGE_ISA;
> + dc->desc = "XEN Host bridge";
> + dc->vmsd = &vmstate_i440fx;
> + /*
> + * PCI-facing part of the host bridge, not usable without the
> + * host-facing part, which can't be device_add'ed, yet.
> + */
> + dc->cannot_instantiate_with_device_add_yet = true;
> + dc->hotpluggable = false;
> +}
> +
> +static const TypeInfo i440fx_xen_info = {
> + .name = TYPE_I440FX_XEN_PCI_DEVICE,
> + .parent = TYPE_PCI_DEVICE,
> + .instance_size = sizeof(PCII440FXState),
> + .class_init = i440fx_xen_class_init,
> +};
> +
> static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
> PCIBus *rootbus)
> {
> @@ -746,6 +797,7 @@ static const TypeInfo i440fx_pcihost_info = {
> static void i440fx_register_types(void)
> {
> type_register_static(&i440fx_info);
> + type_register_static(&i440fx_xen_info);
> type_register_static(&piix3_info);
> type_register_static(&piix3_xen_info);
> type_register_static(&i440fx_pcihost_info);
> --
> 1.9.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xen-devel] [v3][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support
2014-05-26 9:43 ` [v3][PATCH 1/5] xen, gfx passthrough: basic graphics " Tiejun Chen
@ 2014-05-27 14:54 ` Konrad Rzeszutek Wilk
2014-05-28 1:16 ` Chen, Tiejun
2014-05-27 17:47 ` Stefano Stabellini
1 sibling, 1 reply; 28+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-05-27 14:54 UTC (permalink / raw)
To: Tiejun Chen
Cc: peter.maydell, xen-devel, mst, stefano.stabellini, allen.m.kay,
Kelly.Zytaruk, qemu-devel, yang.z.zhang, anthony, anthony.perard
On Mon, May 26, 2014 at 05:43:05PM +0800, Tiejun Chen wrote:
> basic gfx passthrough support:
> - add a vga type for gfx passthrough
> - retrieve VGA bios from sysfs, then load it to guest 0xC0000
s/guest/guest at/
> - register/unregister legacy VGA I/O ports and MMIOs for passthroughed gfx
s/passthrouged gfx/passthrough GFX/
>
> The original patch is from Weidong Han <weidong.han@intel.com>
>
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> Cc: Weidong Han <weidong.han@intel.com>
> ---
> v3:
>
> * Fix some typos.
> * Add more comments to make that readable.
> * Improve some return paths.
>
> v2:
>
> * retrieve VGA bios from sysfs properly.
> * redefine some function name.
>
> hw/xen/Makefile.objs | 2 +-
> hw/xen/xen-host-pci-device.c | 5 +
> hw/xen/xen-host-pci-device.h | 1 +
> hw/xen/xen_pt.c | 10 ++
> hw/xen/xen_pt.h | 4 +
> hw/xen/xen_pt_graphics.c | 232 +++++++++++++++++++++++++++++++++++++++++++
> qemu-options.hx | 9 ++
> vl.c | 11 +-
> 8 files changed, 272 insertions(+), 2 deletions(-)
> create mode 100644 hw/xen/xen_pt_graphics.c
>
> diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
> index a0ca0aa..77b7dae 100644
> --- a/hw/xen/Makefile.objs
> +++ b/hw/xen/Makefile.objs
> @@ -2,4 +2,4 @@
> common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
>
> obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
> -obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o
> +obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o xen_pt_graphics.o
> diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
> index 743b37b..a54b7de 100644
> --- a/hw/xen/xen-host-pci-device.c
> +++ b/hw/xen/xen-host-pci-device.c
> @@ -376,6 +376,11 @@ int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
> goto error;
> }
> d->irq = v;
> + rc = xen_host_pci_get_hex_value(d, "class", &v);
> + if (rc) {
> + goto error;
> + }
> + d->class_code = v;
> d->is_virtfn = xen_host_pci_dev_is_virtfn(d);
>
> return 0;
> diff --git a/hw/xen/xen-host-pci-device.h b/hw/xen/xen-host-pci-device.h
> index c2486f0..f1e1c30 100644
> --- a/hw/xen/xen-host-pci-device.h
> +++ b/hw/xen/xen-host-pci-device.h
> @@ -25,6 +25,7 @@ typedef struct XenHostPCIDevice {
>
> uint16_t vendor_id;
> uint16_t device_id;
> + uint32_t class_code;
> int irq;
>
> XenHostPCIIORegion io_regions[PCI_NUM_REGIONS - 1];
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index be4220b..a0113ea 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -450,6 +450,7 @@ static int xen_pt_register_regions(XenPCIPassthroughState *s)
> d->rom.size, d->rom.base_addr);
> }
>
> + xen_pt_register_vga_regions(d);
> return 0;
> }
>
> @@ -470,6 +471,8 @@ static void xen_pt_unregister_regions(XenPCIPassthroughState *s)
> if (d->rom.base_addr && d->rom.size) {
> memory_region_destroy(&s->rom);
> }
> +
> + xen_pt_unregister_vga_regions(d);
> }
>
> /* region mapping */
> @@ -693,6 +696,13 @@ static int xen_pt_initfn(PCIDevice *d)
> /* Handle real device's MMIO/PIO BARs */
> xen_pt_register_regions(s);
>
> + /* Setup VGA bios for passthroughed gfx */
Ditto
> + if (xen_pt_setup_vga(&s->real_device) < 0) {
> + XEN_PT_ERR(d, "Setup VGA BIOS of passthroughed gfx failed!\n");
Ditto
> + xen_host_pci_device_put(&s->real_device);
> + return -1;
> + }
> +
> /* reinitialize each config register to be emulated */
> if (xen_pt_config_init(s)) {
> XEN_PT_ERR(d, "PCI Config space initialisation failed.\n");
> diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
> index 942dc60..4d3a18d 100644
> --- a/hw/xen/xen_pt.h
> +++ b/hw/xen/xen_pt.h
> @@ -298,5 +298,9 @@ static inline bool xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
> return s->msix && s->msix->bar_index == bar;
> }
>
> +extern int xen_has_gfx_passthru;
> +int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
> +int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
> +int xen_pt_setup_vga(XenHostPCIDevice *dev);
>
> #endif /* !XEN_PT_H */
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> new file mode 100644
> index 0000000..e63bd6f
> --- /dev/null
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -0,0 +1,232 @@
> +/*
> + * graphics passthrough
> + */
> +#include "xen_pt.h"
> +#include "xen-host-pci-device.h"
> +#include "hw/xen/xen_backend.h"
> +
> +static int is_vga_passthrough(XenHostPCIDevice *dev)
> +{
> + return (xen_has_gfx_passthru
> + && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
> +}
> +
> +typedef struct VGARegion {
> + int type; /* Memory or port I/O */
> + uint64_t guest_base_addr;
> + uint64_t machine_base_addr;
> + uint64_t size; /* size of the region */
> + int rc;
> +} VGARegion;
> +
> +#define IORESOURCE_IO 0x00000100
> +#define IORESOURCE_MEM 0x00000200
> +
> +static struct VGARegion vga_args[] = {
> + {
> + .type = IORESOURCE_IO,
> + .guest_base_addr = 0x3B0,
> + .machine_base_addr = 0x3B0,
> + .size = 0xC,
> + .rc = -1,
> + },
> + {
> + .type = IORESOURCE_IO,
> + .guest_base_addr = 0x3C0,
> + .machine_base_addr = 0x3C0,
> + .size = 0x20,
> + .rc = -1,
> + },
> + {
> + .type = IORESOURCE_MEM,
> + .guest_base_addr = 0xa0000 >> XC_PAGE_SHIFT,
> + .machine_base_addr = 0xa0000 >> XC_PAGE_SHIFT,
> + .size = 0x20,
> + .rc = -1,
> + },
> +};
> +
> +/*
> + * register VGA resources for the domain with assigned gfx
> + */
> +int xen_pt_register_vga_regions(XenHostPCIDevice *dev)
> +{
> + int i = 0;
> +
> + if (!is_vga_passthrough(dev)) {
> + return -1;
> + }
> +
> + for (i = 0 ; i < ARRAY_SIZE(vga_args); i++) {
> + if (vga_args[i].type == IORESOURCE_IO) {
> + vga_args[i].rc = xc_domain_ioport_mapping(xen_xc, xen_domid,
> + vga_args[i].guest_base_addr,
> + vga_args[i].machine_base_addr,
> + vga_args[i].size, DPCI_ADD_MAPPING);
> + } else {
> + vga_args[i].rc = xc_domain_memory_mapping(xen_xc, xen_domid,
> + vga_args[i].guest_base_addr,
> + vga_args[i].machine_base_addr,
> + vga_args[i].size, DPCI_ADD_MAPPING);
> + }
> +
> + if (vga_args[i].rc) {
> + XEN_PT_ERR(NULL, "VGA %s mapping failed! (rc: %i)\n",
> + vga_args[i].type == IORESOURCE_IO ? "ioport" : "memory",
> + vga_args[i].rc);
> + return vga_args[i].rc;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * unregister VGA resources for the domain with assigned gfx
> + */
> +int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
> +{
> + int i = 0;
> +
> + if (!is_vga_passthrough(dev)) {
> + return -1;
> + }
> +
> + for (i = 0 ; i < ARRAY_SIZE(vga_args); i++) {
> + if (vga_args[i].type == IORESOURCE_IO) {
> + vga_args[i].rc = xc_domain_ioport_mapping(xen_xc, xen_domid,
> + vga_args[i].guest_base_addr,
> + vga_args[i].machine_base_addr,
> + vga_args[i].size, DPCI_REMOVE_MAPPING);
> + } else {
> + vga_args[i].rc = xc_domain_memory_mapping(xen_xc, xen_domid,
> + vga_args[i].guest_base_addr,
> + vga_args[i].machine_base_addr,
> + vga_args[i].size, DPCI_REMOVE_MAPPING);
> + }
> +
> + if (vga_args[i].rc) {
> + XEN_PT_ERR(NULL, "VGA %s unmapping failed! (rc: %i)\n",
> + vga_args[i].type == IORESOURCE_IO ? "ioport" : "memory",
> + vga_args[i].rc);
> + return vga_args[i].rc;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int get_vgabios(unsigned char *buf, XenHostPCIDevice *dev)
> +{
> + char rom_file[64];
> + FILE *fp;
> + uint8_t val;
> + struct stat st;
> + uint16_t magic = 0;
> + int ret = 0;
> +
> + snprintf(rom_file, sizeof(rom_file),
> + "/sys/bus/pci/devices/%04x:%02x:%02x.%d/rom",
> + dev->domain, dev->bus, dev->dev,
> + dev->func);
> +
> + if (stat(rom_file, &st)) {
> + return -ENODEV;
> + }
> +
> + if (access(rom_file, F_OK)) {
> + XEN_PT_ERR(NULL, "pci-assign: Insufficient privileges for %s",
> + rom_file);
> + return -ENODEV;
> + }
> +
> + /* Write "1" to the ROM file to enable it */
> + fp = fopen(rom_file, "r+");
> + if (fp == NULL) {
> + return -EACCES;
> + }
> + val = 1;
> + if (fwrite(&val, 1, 1, fp) != 1) {
> + XEN_PT_LOG("%s\n", "Failed to enable pci-sysfs rom file");
> + ret = -EIO;
> + goto close_rom;
> + }
> + fseek(fp, 0, SEEK_SET);
> +
> + /*
> + * Check if it a real bios extension.
> + * The magic number is 0xAA55.
> + */
> + if (!fread(&magic, sizeof(magic), 1, fp)) {
> + XEN_PT_ERR(NULL, "VGA: can't get magic.\n");
> + ret = -ENODEV;
> + goto close_rom;
> + }
> + if (magic != 0xAA55) {
> + XEN_PT_ERR(NULL, "VGA: wrong magic %x.\n", magic);
> + ret = -ENODEV;
> + goto close_rom;
> + }
> + fseek(fp, 0, SEEK_SET);
> +
> + if (!fread(buf, 1, st.st_size, fp)) {
> + XEN_PT_ERR(NULL, "VGA: pci-assign: Cannot read from host %s", rom_file);
> + XEN_PT_LOG(NULL, "VGA: Device option ROM contents are probably invalid "
> + "(check dmesg).\nSkip option ROM probe with rombar=0, "
> + "or load from file with romfile=\n");
> + }
> +
> +close_rom:
> + /* Write "0" to disable ROM */
> + fseek(fp, 0, SEEK_SET);
> + val = 0;
> + if (!fwrite(&val, 1, 1, fp)) {
> + ret = -EIO;
> + XEN_PT_LOG("%s\n", "Failed to disable pci-sysfs rom file");
> + }
> + fclose(fp);
> + return ret;
> +}
> +
> +/* Allocated 128K for the vga bios */
> +#define VGA_BIOS_DEFAULT_SIZE (0x20000)
> +
> +int xen_pt_setup_vga(XenHostPCIDevice *dev)
> +{
> + unsigned char *bios = NULL;
> + int bios_size = 0;
> + char *c = NULL;
> + char checksum = 0;
> + int rc = 0;
> +
> + if (!is_vga_passthrough(dev)) {
> + return rc;
> + }
> +
> + bios = g_malloc0(VGA_BIOS_DEFAULT_SIZE);
> +
> + bios_size = get_vgabios(bios, dev);
> + if (bios_size) {
> + XEN_PT_ERR(NULL, "VGA: Error %d getting VBIOS!\n", bios_size);
> + if (bios_size < 0) {
> + XEN_PT_ERR(NULL, "VGA: Error (%s).\n", strerror(bios_size));
> + }
> + rc = -1;
> + goto out;
> + }
> +
> + /* Adjust the bios checksum */
> + for (c = (char *)bios; c < ((char *)bios + bios_size); c++) {
> + checksum += *c;
> + }
> + if (checksum) {
> + bios[bios_size - 1] -= checksum;
> + XEN_PT_LOG(NULL, "vga bios checksum is adjusted!\n");
> + }
> +
> + /* Currently we fixed this address as a primary. */
> + cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
> +out:
> + g_free(bios);
> + return rc;
> +}
> diff --git a/qemu-options.hx b/qemu-options.hx
> index c2c0823..9cb324f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1053,6 +1053,15 @@ STEXI
> Rotate graphical output some deg left (only PXA LCD).
> ETEXI
>
> +DEF("gfx_passthru", 0, QEMU_OPTION_gfx_passthru,
> + "-gfx_passthru enable Intel IGD passthrough by XEN\n",
> + QEMU_ARCH_ALL)
> +STEXI
> +@item -gfx_passthru
> +@findex -gfx_passthru
> +Enable Intel IGD passthrough by XEN
> +ETEXI
> +
> DEF("vga", HAS_ARG, QEMU_OPTION_vga,
> "-vga [std|cirrus|vmware|qxl|xenfb|tcx|cg3|none]\n"
> " select video card type\n", QEMU_ARCH_ALL)
> diff --git a/vl.c b/vl.c
> index 673148e..c02207f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1438,6 +1438,13 @@ static void smp_parse(QemuOpts *opts)
>
> }
>
> +/* We still need this for compatibility with XEN. */
> +int xen_has_gfx_passthru;
> +static void xen_gfx_passthru(const char *optarg)
> +{
> + xen_has_gfx_passthru = 1;
> +}
> +
> static void configure_realtime(QemuOpts *opts)
> {
> bool enable_mlock;
> @@ -2990,7 +2997,6 @@ int main(int argc, char **argv, char **envp)
> const char *trace_file = NULL;
> const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
> 1024 * 1024;
> -
> atexit(qemu_run_exit_notifiers);
> error_set_progname(argv[0]);
> qemu_init_exec_dir(argv[0]);
> @@ -3957,6 +3963,9 @@ int main(int argc, char **argv, char **envp)
> }
> configure_msg(opts);
> break;
> + case QEMU_OPTION_gfx_passthru:
> + xen_gfx_passthru(optarg);
> + break;
> default:
> os_parse_cmd_args(popt->index, optarg);
> }
> --
> 1.9.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xen-devel] [v3][PATCH 2/5] xen, gfx passthrough: create intel isa bridge
2014-05-26 9:43 ` [v3][PATCH 2/5] xen, gfx passthrough: create intel isa bridge Tiejun Chen
@ 2014-05-27 14:55 ` Konrad Rzeszutek Wilk
2014-05-27 17:52 ` Stefano Stabellini
2014-05-28 1:20 ` Chen, Tiejun
0 siblings, 2 replies; 28+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-05-27 14:55 UTC (permalink / raw)
To: Tiejun Chen
Cc: peter.maydell, xen-devel, mst, stefano.stabellini, allen.m.kay,
Kelly.Zytaruk, qemu-devel, yang.z.zhang, anthony, anthony.perard
On Mon, May 26, 2014 at 05:43:06PM +0800, Tiejun Chen wrote:
> ISA bridge is needed since Intel gfx drive will probe it instead
> of Dev31:Fun0 to make graphics device passthrough work easy for VMM, that
> only need to expose ISA bridge to let driver know the real hardware underneath.
>
> The original patch is from Allen Kay [allen.m.kay@intel.com]
>
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> Cc: Allen Kay <allen.m.kay@intel.com>
> ---
> v3:
>
> * Fix some typos.
> * Improve some return paths.
>
> v2:
>
> * Nothing is changed.
>
> hw/xen/xen_pt_graphics.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 63 insertions(+)
>
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> index e63bd6f..51b174f 100644
> --- a/hw/xen/xen_pt_graphics.c
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -230,3 +230,66 @@ out:
> g_free(bios);
> return rc;
> }
> +
> +static uint32_t isa_bridge_read_config(PCIDevice *d, uint32_t addr, int len)
> +{
> + return pci_default_read_config(d, addr, len);
> +}
> +
> +static void isa_bridge_write_config(PCIDevice *d, uint32_t addr, uint32_t v,
> + int len)
> +{
> + pci_default_write_config(d, addr, v, len);
> +
> + return;
You don't need the return there.
> +}
> +
> +static void isa_bridge_class_init(ObjectClass *klass, void *data)
> +{
> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> + k->config_read = isa_bridge_read_config;
> + k->config_write = isa_bridge_write_config;
> +
> + return;
Ditto
> +};
> +
> +typedef struct {
> + PCIDevice dev;
> +} ISABridgeState;
> +
> +static TypeInfo isa_bridge_info = {
> + .name = "intel-pch-isa-bridge",
> + .parent = TYPE_PCI_DEVICE,
> + .instance_size = sizeof(ISABridgeState),
> + .class_init = isa_bridge_class_init,
> +};
> +
> +static void xen_pt_graphics_register_types(void)
> +{
> + type_register_static(&isa_bridge_info);
> +}
> +
> +type_init(xen_pt_graphics_register_types)
> +
> +static int create_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
> +{
> + struct PCIDevice *dev;
> +
> + char rid;
> +
> + dev = pci_create(bus, PCI_DEVFN(0x1f, 0), "intel-pch-isa-bridge");
> +
> + qdev_init_nofail(&dev->qdev);
> +
> + pci_config_set_vendor_id(dev->config, hdev->vendor_id);
> + pci_config_set_device_id(dev->config, hdev->device_id);
> +
> + xen_host_pci_get_block(hdev, PCI_REVISION_ID, (uint8_t *)&rid, 1);
> +
> + pci_config_set_revision(dev->config, rid);
> + pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_ISA);
> +
> + XEN_PT_LOG(dev, "Intel PCH ISA bridge created.\n");
> + return 0;
> +}
> --
> 1.9.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping
2014-05-26 9:43 ` [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping Tiejun Chen
@ 2014-05-27 17:35 ` Stefano Stabellini
2014-05-28 1:31 ` Chen, Tiejun
0 siblings, 1 reply; 28+ messages in thread
From: Stefano Stabellini @ 2014-05-27 17:35 UTC (permalink / raw)
To: Tiejun Chen
Cc: peter.maydell, xen-devel, mst, stefano.stabellini, allen.m.kay,
qemu-devel, Kelly.Zytaruk, yang.z.zhang, anthony, anthony.perard
On Mon, 26 May 2014, Tiejun Chen wrote:
> The OpRegion shouldn't be mapped 1:1 because the address in the host
> can't be used in the guest directly.
>
> This patch traps read and write access to the opregion of the Intel
> GPU config space (offset 0xfc).
>
> The original patch is from Jean Guyader <jean.guyader@eu.citrix.com>
>
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> Cc: Jean Guyader <jean.guyader@eu.citrix.com>
> ---
> v3:
>
> * Fix some typos.
> * Add more comments to make that readable.
> * To unmap igd_opregion when call xen_pt_unregister_vga_regions().
> * Improve some return paths.
> * We need to map 3 pages for opregion as hvmloader set.
> * Force to convert igd_guest/host_opoegion as an unsigned long type
> while calling xc_domain_memory_mapping().
>
> v2:
>
> * We should return zero as an invalid address value while calling
> igd_read_opregion().
>
> hw/xen/xen_pt.h | 4 ++-
> hw/xen/xen_pt_config_init.c | 50 ++++++++++++++++++++++++++++++++++-
> hw/xen/xen_pt_graphics.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 116 insertions(+), 2 deletions(-)
>
> diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
> index 507165c..25147cf 100644
> --- a/hw/xen/xen_pt.h
> +++ b/hw/xen/xen_pt.h
> @@ -63,7 +63,7 @@ typedef int (*xen_pt_conf_byte_read)
> #define XEN_PT_BAR_UNMAPPED (-1)
>
> #define PCI_CAP_MAX 48
> -
> +#define PCI_INTEL_OPREGION 0xfc
>
> typedef enum {
> XEN_PT_GRP_TYPE_HARDWIRED = 0, /* 0 Hardwired reg group */
> @@ -306,5 +306,7 @@ int pci_create_pch(PCIBus *bus);
> void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr,
> uint32_t val, int len);
> uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len);
> +uint32_t igd_read_opregion(XenPCIPassthroughState *s);
> +void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
>
> #endif /* !XEN_PT_H */
> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
> index de9a20f..6eaaa9a 100644
> --- a/hw/xen/xen_pt_config_init.c
> +++ b/hw/xen/xen_pt_config_init.c
> @@ -575,6 +575,22 @@ static int xen_pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s,
> return 0;
> }
>
> +static int xen_pt_intel_opregion_read(XenPCIPassthroughState *s,
> + XenPTReg *cfg_entry,
> + uint32_t *value, uint32_t valid_mask)
> +{
> + *value = igd_read_opregion(s);
> + return 0;
> +}
> +
> +static int xen_pt_intel_opregion_write(XenPCIPassthroughState *s,
> + XenPTReg *cfg_entry, uint32_t *value,
> + uint32_t dev_value, uint32_t valid_mask)
> +{
> + igd_write_opregion(s, *value);
> + return 0;
> +}
> +
> /* Header Type0 reg static information table */
> static XenPTRegInfo xen_pt_emu_reg_header0[] = {
> /* Vendor ID reg */
> @@ -1440,6 +1456,20 @@ static XenPTRegInfo xen_pt_emu_reg_msix[] = {
> },
> };
>
> +static XenPTRegInfo xen_pt_emu_reg_igd_opregion[] = {
> + /* Intel IGFX OpRegion reg */
> + {
> + .offset = 0x0,
> + .size = 4,
> + .init_val = 0,
> + .no_wb = 1,
> + .u.dw.read = xen_pt_intel_opregion_read,
> + .u.dw.write = xen_pt_intel_opregion_write,
> + },
> + {
> + .size = 0,
> + },
> +};
>
> /****************************
> * Capabilities
> @@ -1677,6 +1707,14 @@ static const XenPTRegGroupInfo xen_pt_emu_reg_grps[] = {
> .size_init = xen_pt_msix_size_init,
> .emu_regs = xen_pt_emu_reg_msix,
> },
> + /* Intel IGD Opregion group */
> + {
> + .grp_id = PCI_INTEL_OPREGION,
> + .grp_type = XEN_PT_GRP_TYPE_EMU,
> + .grp_size = 0x4,
> + .size_init = xen_pt_reg_grp_size_init,
> + .emu_regs = xen_pt_emu_reg_igd_opregion,
> + },
> {
> .grp_size = 0,
> },
> @@ -1806,7 +1844,8 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
> uint32_t reg_grp_offset = 0;
> XenPTRegGroup *reg_grp_entry = NULL;
>
> - if (xen_pt_emu_reg_grps[i].grp_id != 0xFF) {
> + if (xen_pt_emu_reg_grps[i].grp_id != 0xFF
> + && xen_pt_emu_reg_grps[i].grp_id != PCI_INTEL_OPREGION) {
> if (xen_pt_hide_dev_cap(&s->real_device,
> xen_pt_emu_reg_grps[i].grp_id)) {
> continue;
> @@ -1819,6 +1858,15 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
> }
> }
>
> + /*
> + * By default we will trap up to 0x40 in the cfg space.
> + * If an intel device is pass through we need to trap 0xfc,
> + * therefore the size should be 0xff.
> + */
> + if (xen_pt_emu_reg_grps[i].grp_id == PCI_INTEL_OPREGION) {
> + reg_grp_offset = PCI_INTEL_OPREGION;
> + }
> +
> reg_grp_entry = g_new0(XenPTRegGroup, 1);
> QLIST_INIT(®_grp_entry->reg_tbl_list);
> QLIST_INSERT_HEAD(&s->reg_grps, reg_grp_entry, entries);
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> index 72e30ac..ee6fec0 100644
> --- a/hw/xen/xen_pt_graphics.c
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -6,6 +6,9 @@
> #include "hw/xen/xen_backend.h"
> #include "hw/pci/pci_bus.h"
>
> +static unsigned long igd_guest_opregion;
> +static unsigned long igd_host_opregion;
> +
> static int is_vga_passthrough(XenHostPCIDevice *dev)
> {
> return (xen_has_gfx_passthru
> @@ -88,6 +91,7 @@ int xen_pt_register_vga_regions(XenHostPCIDevice *dev)
> int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
> {
> int i = 0;
> + int ret = 0;
>
> if (!is_vga_passthrough(dev)) {
> return -1;
> @@ -114,6 +118,17 @@ int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
> }
> }
>
> + if (igd_guest_opregion) {
> + ret = xc_domain_memory_mapping(xen_xc, xen_domid,
> + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT),
> + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
> + 3,
> + DPCI_REMOVE_MAPPING);
> + if (ret) {
> + return ret;
> + }
> + }
> +
> return 0;
> }
>
> @@ -452,3 +467,52 @@ err_out:
> XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
> return -1;
> }
> +
> +uint32_t igd_read_opregion(XenPCIPassthroughState *s)
> +{
> + uint32_t val = 0;
> +
> + if (igd_guest_opregion == 0) {
> + return val;
Sorry for not having spotted it before, but isn't this supposed to
return an error? The old code returns -1 in this case.
> + }
> +
> + val = igd_guest_opregion;
> +
> + XEN_PT_LOG(&s->dev, "Read opregion val=%x\n", val);
> + return val;
> +}
> +
> +void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val)
> +{
> + int ret;
> +
> + if (igd_guest_opregion) {
> + XEN_PT_LOG(&s->dev, "opregion register already been set, ignoring %x\n",
> + val);
> + return;
> + }
> +
> + xen_host_pci_get_block(&s->real_device, PCI_INTEL_OPREGION,
> + (uint8_t *)&igd_host_opregion, 4);
> + igd_guest_opregion = (unsigned long)(val & ~0xfff)
> + | (igd_host_opregion & 0xfff);
> +
> + ret = xc_domain_memory_mapping(xen_xc, xen_domid,
> + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT),
> + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
> + 3,
Why 3 pages instead of 2?
> + DPCI_ADD_MAPPING);
> +
> + if (ret) {
> + XEN_PT_ERR(&s->dev, "[%d]:Can't map IGD host opregion:0x%lx to"
> + " guest opregion:0x%lx.\n", ret,
> + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
> + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT));
> + igd_guest_opregion = 0;
> + return;
> + }
> +
> + XEN_PT_LOG(&s->dev, "Map OpRegion: 0x%lx -> 0x%lx\n",
> + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
> + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT));
> +}
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v3][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support
2014-05-26 9:43 ` [v3][PATCH 1/5] xen, gfx passthrough: basic graphics " Tiejun Chen
2014-05-27 14:54 ` [Xen-devel] " Konrad Rzeszutek Wilk
@ 2014-05-27 17:47 ` Stefano Stabellini
2014-05-28 1:43 ` Chen, Tiejun
1 sibling, 1 reply; 28+ messages in thread
From: Stefano Stabellini @ 2014-05-27 17:47 UTC (permalink / raw)
To: Tiejun Chen
Cc: peter.maydell, xen-devel, mst, stefano.stabellini, allen.m.kay,
qemu-devel, Kelly.Zytaruk, yang.z.zhang, anthony, anthony.perard
On Mon, 26 May 2014, Tiejun Chen wrote:
> basic gfx passthrough support:
> - add a vga type for gfx passthrough
> - retrieve VGA bios from sysfs, then load it to guest 0xC0000
> - register/unregister legacy VGA I/O ports and MMIOs for passthroughed gfx
>
> The original patch is from Weidong Han <weidong.han@intel.com>
>
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> Cc: Weidong Han <weidong.han@intel.com>
> ---
> v3:
>
> * Fix some typos.
> * Add more comments to make that readable.
> * Improve some return paths.
>
> v2:
>
> * retrieve VGA bios from sysfs properly.
> * redefine some function name.
>
> hw/xen/Makefile.objs | 2 +-
> hw/xen/xen-host-pci-device.c | 5 +
> hw/xen/xen-host-pci-device.h | 1 +
> hw/xen/xen_pt.c | 10 ++
> hw/xen/xen_pt.h | 4 +
> hw/xen/xen_pt_graphics.c | 232 +++++++++++++++++++++++++++++++++++++++++++
> qemu-options.hx | 9 ++
> vl.c | 11 +-
> 8 files changed, 272 insertions(+), 2 deletions(-)
> create mode 100644 hw/xen/xen_pt_graphics.c
>
> diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
> index a0ca0aa..77b7dae 100644
> --- a/hw/xen/Makefile.objs
> +++ b/hw/xen/Makefile.objs
> @@ -2,4 +2,4 @@
> common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
>
> obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
> -obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o
> +obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o xen_pt_graphics.o
> diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
> index 743b37b..a54b7de 100644
> --- a/hw/xen/xen-host-pci-device.c
> +++ b/hw/xen/xen-host-pci-device.c
> @@ -376,6 +376,11 @@ int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
> goto error;
> }
> d->irq = v;
> + rc = xen_host_pci_get_hex_value(d, "class", &v);
> + if (rc) {
> + goto error;
> + }
> + d->class_code = v;
> d->is_virtfn = xen_host_pci_dev_is_virtfn(d);
>
> return 0;
> diff --git a/hw/xen/xen-host-pci-device.h b/hw/xen/xen-host-pci-device.h
> index c2486f0..f1e1c30 100644
> --- a/hw/xen/xen-host-pci-device.h
> +++ b/hw/xen/xen-host-pci-device.h
> @@ -25,6 +25,7 @@ typedef struct XenHostPCIDevice {
>
> uint16_t vendor_id;
> uint16_t device_id;
> + uint32_t class_code;
> int irq;
>
> XenHostPCIIORegion io_regions[PCI_NUM_REGIONS - 1];
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index be4220b..a0113ea 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -450,6 +450,7 @@ static int xen_pt_register_regions(XenPCIPassthroughState *s)
> d->rom.size, d->rom.base_addr);
> }
>
> + xen_pt_register_vga_regions(d);
> return 0;
> }
>
> @@ -470,6 +471,8 @@ static void xen_pt_unregister_regions(XenPCIPassthroughState *s)
> if (d->rom.base_addr && d->rom.size) {
> memory_region_destroy(&s->rom);
> }
> +
> + xen_pt_unregister_vga_regions(d);
> }
>
> /* region mapping */
> @@ -693,6 +696,13 @@ static int xen_pt_initfn(PCIDevice *d)
> /* Handle real device's MMIO/PIO BARs */
> xen_pt_register_regions(s);
>
> + /* Setup VGA bios for passthroughed gfx */
> + if (xen_pt_setup_vga(&s->real_device) < 0) {
> + XEN_PT_ERR(d, "Setup VGA BIOS of passthroughed gfx failed!\n");
> + xen_host_pci_device_put(&s->real_device);
> + return -1;
> + }
> +
> /* reinitialize each config register to be emulated */
> if (xen_pt_config_init(s)) {
> XEN_PT_ERR(d, "PCI Config space initialisation failed.\n");
> diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
> index 942dc60..4d3a18d 100644
> --- a/hw/xen/xen_pt.h
> +++ b/hw/xen/xen_pt.h
> @@ -298,5 +298,9 @@ static inline bool xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
> return s->msix && s->msix->bar_index == bar;
> }
>
> +extern int xen_has_gfx_passthru;
> +int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
> +int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
> +int xen_pt_setup_vga(XenHostPCIDevice *dev);
>
> #endif /* !XEN_PT_H */
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> new file mode 100644
> index 0000000..e63bd6f
> --- /dev/null
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -0,0 +1,232 @@
> +/*
> + * graphics passthrough
> + */
> +#include "xen_pt.h"
> +#include "xen-host-pci-device.h"
> +#include "hw/xen/xen_backend.h"
> +
> +static int is_vga_passthrough(XenHostPCIDevice *dev)
> +{
> + return (xen_has_gfx_passthru
> + && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
> +}
> +
> +typedef struct VGARegion {
> + int type; /* Memory or port I/O */
> + uint64_t guest_base_addr;
> + uint64_t machine_base_addr;
> + uint64_t size; /* size of the region */
> + int rc;
> +} VGARegion;
> +
> +#define IORESOURCE_IO 0x00000100
> +#define IORESOURCE_MEM 0x00000200
> +
> +static struct VGARegion vga_args[] = {
> + {
> + .type = IORESOURCE_IO,
> + .guest_base_addr = 0x3B0,
> + .machine_base_addr = 0x3B0,
> + .size = 0xC,
> + .rc = -1,
> + },
> + {
> + .type = IORESOURCE_IO,
> + .guest_base_addr = 0x3C0,
> + .machine_base_addr = 0x3C0,
> + .size = 0x20,
> + .rc = -1,
> + },
> + {
> + .type = IORESOURCE_MEM,
> + .guest_base_addr = 0xa0000 >> XC_PAGE_SHIFT,
> + .machine_base_addr = 0xa0000 >> XC_PAGE_SHIFT,
> + .size = 0x20,
> + .rc = -1,
> + },
> +};
> +
> +/*
> + * register VGA resources for the domain with assigned gfx
> + */
> +int xen_pt_register_vga_regions(XenHostPCIDevice *dev)
> +{
> + int i = 0;
> +
> + if (!is_vga_passthrough(dev)) {
> + return -1;
Given that xen_pt_register_vga_regions is called unconditionally, this
is not an error condition, isn't it? It should return 0.
> + }
> +
> + for (i = 0 ; i < ARRAY_SIZE(vga_args); i++) {
> + if (vga_args[i].type == IORESOURCE_IO) {
> + vga_args[i].rc = xc_domain_ioport_mapping(xen_xc, xen_domid,
> + vga_args[i].guest_base_addr,
> + vga_args[i].machine_base_addr,
> + vga_args[i].size, DPCI_ADD_MAPPING);
> + } else {
> + vga_args[i].rc = xc_domain_memory_mapping(xen_xc, xen_domid,
> + vga_args[i].guest_base_addr,
> + vga_args[i].machine_base_addr,
> + vga_args[i].size, DPCI_ADD_MAPPING);
> + }
> +
> + if (vga_args[i].rc) {
> + XEN_PT_ERR(NULL, "VGA %s mapping failed! (rc: %i)\n",
> + vga_args[i].type == IORESOURCE_IO ? "ioport" : "memory",
> + vga_args[i].rc);
> + return vga_args[i].rc;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * unregister VGA resources for the domain with assigned gfx
> + */
> +int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
> +{
> + int i = 0;
> +
> + if (!is_vga_passthrough(dev)) {
> + return -1;
same here
> + }
> +
> + for (i = 0 ; i < ARRAY_SIZE(vga_args); i++) {
> + if (vga_args[i].type == IORESOURCE_IO) {
> + vga_args[i].rc = xc_domain_ioport_mapping(xen_xc, xen_domid,
> + vga_args[i].guest_base_addr,
> + vga_args[i].machine_base_addr,
> + vga_args[i].size, DPCI_REMOVE_MAPPING);
> + } else {
> + vga_args[i].rc = xc_domain_memory_mapping(xen_xc, xen_domid,
> + vga_args[i].guest_base_addr,
> + vga_args[i].machine_base_addr,
> + vga_args[i].size, DPCI_REMOVE_MAPPING);
> + }
> +
> + if (vga_args[i].rc) {
> + XEN_PT_ERR(NULL, "VGA %s unmapping failed! (rc: %i)\n",
> + vga_args[i].type == IORESOURCE_IO ? "ioport" : "memory",
> + vga_args[i].rc);
> + return vga_args[i].rc;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int get_vgabios(unsigned char *buf, XenHostPCIDevice *dev)
> +{
> + char rom_file[64];
> + FILE *fp;
> + uint8_t val;
> + struct stat st;
> + uint16_t magic = 0;
> + int ret = 0;
> +
> + snprintf(rom_file, sizeof(rom_file),
> + "/sys/bus/pci/devices/%04x:%02x:%02x.%d/rom",
> + dev->domain, dev->bus, dev->dev,
> + dev->func);
> +
> + if (stat(rom_file, &st)) {
> + return -ENODEV;
> + }
> +
> + if (access(rom_file, F_OK)) {
> + XEN_PT_ERR(NULL, "pci-assign: Insufficient privileges for %s",
> + rom_file);
> + return -ENODEV;
> + }
> +
> + /* Write "1" to the ROM file to enable it */
> + fp = fopen(rom_file, "r+");
> + if (fp == NULL) {
> + return -EACCES;
> + }
> + val = 1;
> + if (fwrite(&val, 1, 1, fp) != 1) {
> + XEN_PT_LOG("%s\n", "Failed to enable pci-sysfs rom file");
> + ret = -EIO;
> + goto close_rom;
> + }
> + fseek(fp, 0, SEEK_SET);
> +
> + /*
> + * Check if it a real bios extension.
> + * The magic number is 0xAA55.
> + */
> + if (!fread(&magic, sizeof(magic), 1, fp)) {
> + XEN_PT_ERR(NULL, "VGA: can't get magic.\n");
> + ret = -ENODEV;
> + goto close_rom;
> + }
> + if (magic != 0xAA55) {
> + XEN_PT_ERR(NULL, "VGA: wrong magic %x.\n", magic);
> + ret = -ENODEV;
> + goto close_rom;
> + }
> + fseek(fp, 0, SEEK_SET);
> +
> + if (!fread(buf, 1, st.st_size, fp)) {
> + XEN_PT_ERR(NULL, "VGA: pci-assign: Cannot read from host %s", rom_file);
> + XEN_PT_LOG(NULL, "VGA: Device option ROM contents are probably invalid "
> + "(check dmesg).\nSkip option ROM probe with rombar=0, "
> + "or load from file with romfile=\n");
> + }
> +
> +close_rom:
> + /* Write "0" to disable ROM */
> + fseek(fp, 0, SEEK_SET);
> + val = 0;
> + if (!fwrite(&val, 1, 1, fp)) {
> + ret = -EIO;
> + XEN_PT_LOG("%s\n", "Failed to disable pci-sysfs rom file");
> + }
> + fclose(fp);
> + return ret;
> +}
> +
> +/* Allocated 128K for the vga bios */
> +#define VGA_BIOS_DEFAULT_SIZE (0x20000)
> +
> +int xen_pt_setup_vga(XenHostPCIDevice *dev)
> +{
> + unsigned char *bios = NULL;
> + int bios_size = 0;
> + char *c = NULL;
> + char checksum = 0;
> + int rc = 0;
> +
> + if (!is_vga_passthrough(dev)) {
> + return rc;
> + }
> +
> + bios = g_malloc0(VGA_BIOS_DEFAULT_SIZE);
> +
> + bios_size = get_vgabios(bios, dev);
> + if (bios_size) {
> + XEN_PT_ERR(NULL, "VGA: Error %d getting VBIOS!\n", bios_size);
> + if (bios_size < 0) {
> + XEN_PT_ERR(NULL, "VGA: Error (%s).\n", strerror(bios_size));
> + }
> + rc = -1;
> + goto out;
> + }
> +
> + /* Adjust the bios checksum */
> + for (c = (char *)bios; c < ((char *)bios + bios_size); c++) {
> + checksum += *c;
> + }
> + if (checksum) {
> + bios[bios_size - 1] -= checksum;
> + XEN_PT_LOG(NULL, "vga bios checksum is adjusted!\n");
> + }
> +
> + /* Currently we fixed this address as a primary. */
> + cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
> +out:
> + g_free(bios);
> + return rc;
> +}
> diff --git a/qemu-options.hx b/qemu-options.hx
> index c2c0823..9cb324f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1053,6 +1053,15 @@ STEXI
> Rotate graphical output some deg left (only PXA LCD).
> ETEXI
>
> +DEF("gfx_passthru", 0, QEMU_OPTION_gfx_passthru,
> + "-gfx_passthru enable Intel IGD passthrough by XEN\n",
> + QEMU_ARCH_ALL)
> +STEXI
> +@item -gfx_passthru
> +@findex -gfx_passthru
> +Enable Intel IGD passthrough by XEN
> +ETEXI
> +
> DEF("vga", HAS_ARG, QEMU_OPTION_vga,
> "-vga [std|cirrus|vmware|qxl|xenfb|tcx|cg3|none]\n"
> " select video card type\n", QEMU_ARCH_ALL)
> diff --git a/vl.c b/vl.c
> index 673148e..c02207f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1438,6 +1438,13 @@ static void smp_parse(QemuOpts *opts)
>
> }
>
> +/* We still need this for compatibility with XEN. */
> +int xen_has_gfx_passthru;
> +static void xen_gfx_passthru(const char *optarg)
> +{
> + xen_has_gfx_passthru = 1;
> +}
> +
> static void configure_realtime(QemuOpts *opts)
> {
> bool enable_mlock;
> @@ -2990,7 +2997,6 @@ int main(int argc, char **argv, char **envp)
> const char *trace_file = NULL;
> const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
> 1024 * 1024;
> -
Spurious change
> atexit(qemu_run_exit_notifiers);
> error_set_progname(argv[0]);
> qemu_init_exec_dir(argv[0]);
> @@ -3957,6 +3963,9 @@ int main(int argc, char **argv, char **envp)
> }
> configure_msg(opts);
> break;
> + case QEMU_OPTION_gfx_passthru:
> + xen_gfx_passthru(optarg);
> + break;
> default:
> os_parse_cmd_args(popt->index, optarg);
> }
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xen-devel] [v3][PATCH 2/5] xen, gfx passthrough: create intel isa bridge
2014-05-27 14:55 ` [Xen-devel] " Konrad Rzeszutek Wilk
@ 2014-05-27 17:52 ` Stefano Stabellini
2014-05-28 1:44 ` Chen, Tiejun
2014-05-28 1:20 ` Chen, Tiejun
1 sibling, 1 reply; 28+ messages in thread
From: Stefano Stabellini @ 2014-05-27 17:52 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: peter.maydell, xen-devel, mst, stefano.stabellini, allen.m.kay,
Kelly.Zytaruk, qemu-devel, yang.z.zhang, anthony, anthony.perard,
Tiejun Chen
On Tue, 27 May 2014, Konrad Rzeszutek Wilk wrote:
> On Mon, May 26, 2014 at 05:43:06PM +0800, Tiejun Chen wrote:
> > ISA bridge is needed since Intel gfx drive will probe it instead
> > of Dev31:Fun0 to make graphics device passthrough work easy for VMM, that
> > only need to expose ISA bridge to let driver know the real hardware underneath.
> >
> > The original patch is from Allen Kay [allen.m.kay@intel.com]
> >
> > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> > Cc: Allen Kay <allen.m.kay@intel.com>
> > ---
> > v3:
> >
> > * Fix some typos.
> > * Improve some return paths.
> >
> > v2:
> >
> > * Nothing is changed.
> >
> > hw/xen/xen_pt_graphics.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 63 insertions(+)
> >
> > diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> > index e63bd6f..51b174f 100644
> > --- a/hw/xen/xen_pt_graphics.c
> > +++ b/hw/xen/xen_pt_graphics.c
> > @@ -230,3 +230,66 @@ out:
> > g_free(bios);
> > return rc;
> > }
> > +
> > +static uint32_t isa_bridge_read_config(PCIDevice *d, uint32_t addr, int len)
> > +{
> > + return pci_default_read_config(d, addr, len);
> > +}
> > +
> > +static void isa_bridge_write_config(PCIDevice *d, uint32_t addr, uint32_t v,
> > + int len)
> > +{
> > + pci_default_write_config(d, addr, v, len);
> > +
> > + return;
>
> You don't need the return there.
> > +}
> > +
> > +static void isa_bridge_class_init(ObjectClass *klass, void *data)
> > +{
> > + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> > +
> > + k->config_read = isa_bridge_read_config;
> > + k->config_write = isa_bridge_write_config;
> > +
> > + return;
>
> Ditto
Aside from these 2 minor changes, the patch looks good to me.
> > +};
> > +
> > +typedef struct {
> > + PCIDevice dev;
> > +} ISABridgeState;
> > +
> > +static TypeInfo isa_bridge_info = {
> > + .name = "intel-pch-isa-bridge",
> > + .parent = TYPE_PCI_DEVICE,
> > + .instance_size = sizeof(ISABridgeState),
> > + .class_init = isa_bridge_class_init,
> > +};
> > +
> > +static void xen_pt_graphics_register_types(void)
> > +{
> > + type_register_static(&isa_bridge_info);
> > +}
> > +
> > +type_init(xen_pt_graphics_register_types)
> > +
> > +static int create_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
> > +{
> > + struct PCIDevice *dev;
> > +
> > + char rid;
> > +
> > + dev = pci_create(bus, PCI_DEVFN(0x1f, 0), "intel-pch-isa-bridge");
> > +
> > + qdev_init_nofail(&dev->qdev);
> > +
> > + pci_config_set_vendor_id(dev->config, hdev->vendor_id);
> > + pci_config_set_device_id(dev->config, hdev->device_id);
> > +
> > + xen_host_pci_get_block(hdev, PCI_REVISION_ID, (uint8_t *)&rid, 1);
> > +
> > + pci_config_set_revision(dev->config, rid);
> > + pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_ISA);
> > +
> > + XEN_PT_LOG(dev, "Intel PCH ISA bridge created.\n");
> > + return 0;
> > +}
> > --
> > 1.9.1
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D
2014-05-26 9:43 ` [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D Tiejun Chen
@ 2014-05-27 18:02 ` Stefano Stabellini
2014-05-28 1:51 ` Chen, Tiejun
0 siblings, 1 reply; 28+ messages in thread
From: Stefano Stabellini @ 2014-05-27 18:02 UTC (permalink / raw)
To: Tiejun Chen
Cc: peter.maydell, xen-devel, mst, stefano.stabellini, allen.m.kay,
qemu-devel, Kelly.Zytaruk, yang.z.zhang, anthony, anthony.perard
On Mon, 26 May 2014, Tiejun Chen wrote:
> Some registers of Intel IGD are mapped in host bridge, so it needs to
> passthrough these registers of physical host bridge to guest because
> emulated host bridge in guest doesn't have these mappings.
>
> The original patch is from Weidong Han < weidong.han @ intel.com >
>
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> Cc:Weidong Han <weidong.han@intel.com>
> ---
> v3:
>
> * Improve comments to make that readable.
>
> v2:
>
> * To introduce is_igd_passthrough() to make sure we touch physical host bridge
> only in IGD case.
>
> hw/xen/xen_pt.h | 4 ++
> hw/xen/xen_pt_graphics.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 163 insertions(+)
>
> diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
> index 4d3a18d..507165c 100644
> --- a/hw/xen/xen_pt.h
> +++ b/hw/xen/xen_pt.h
> @@ -302,5 +302,9 @@ extern int xen_has_gfx_passthru;
> int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
> int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
> int xen_pt_setup_vga(XenHostPCIDevice *dev);
> +int pci_create_pch(PCIBus *bus);
> +void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr,
> + uint32_t val, int len);
> +uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len);
>
> #endif /* !XEN_PT_H */
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> index 51b174f..72e30ac 100644
> --- a/hw/xen/xen_pt_graphics.c
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -4,6 +4,7 @@
> #include "xen_pt.h"
> #include "xen-host-pci-device.h"
> #include "hw/xen/xen_backend.h"
> +#include "hw/pci/pci_bus.h"
>
> static int is_vga_passthrough(XenHostPCIDevice *dev)
> {
> @@ -293,3 +294,161 @@ static int create_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
> XEN_PT_LOG(dev, "Intel PCH ISA bridge created.\n");
> return 0;
> }
> +
> +int pci_create_pch(PCIBus *bus)
> +{
> + XenHostPCIDevice hdev;
> + int r = 0;
> +
> + if (!xen_has_gfx_passthru) {
> + return -1;
If this function ends up being called unconditionally, then this should
return 0;
> + }
> +
> + r = xen_host_pci_device_get(&hdev, 0, 0, 0x1f, 0);
> + if (r) {
> + XEN_PT_ERR(NULL, "Failed to find Intel PCH on host\n");
> + goto err;
> + }
> +
> + if (hdev.vendor_id == PCI_VENDOR_ID_INTEL) {
> + r = create_pch_isa_bridge(bus, &hdev);
> + if (r) {
> + XEN_PT_ERR(NULL, "Failed to create PCH ISA bridge.\n");
> + goto err;
> + }
> + }
> +
> + xen_host_pci_device_put(&hdev);
> +
> +err:
> + return r;
> +}
> +
> +/*
> + * Currently we just pass this physical host bridge for IGD, 00:02.0.
> + */
> +static int is_igd_passthrough(PCIDevice *pci_dev)
> +{
> + PCIDevice *f = pci_dev->bus->devices[PCI_DEVFN(2, 0)];
> + if (pci_dev->bus->devices[PCI_DEVFN(2, 0)]) {
Isn't the purpose of this function to check that the *current* device is
the graphic passthrough device?
In that case, shouldn't it just be:
if (pci_dev) {
> + XenPCIPassthroughState *s = DO_UPCAST(XenPCIPassthroughState, dev, f);
> + return (is_vga_passthrough(&s->real_device)
> + && (s->real_device.vendor_id == PCI_VENDOR_ID_INTEL));
> + } else {
> + return 0;
> + }
> +}
>
> +void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr,
> + uint32_t val, int len)
> +{
> + XenHostPCIDevice dev;
> + int r;
> +
> + /* IGD read/write is through the host bridge.
> + * ISA bridge is only for detect purpose. In i915 driver it will
> + * probe ISA bridge to discover the IGD, see comment in i915_drv.c:
> + * intel_detect_pch():
> + * The reason to probe ISA bridge instead of Dev31:Fun0 is to
> + * make graphics device passthrough work easy for VMM, that only
> + * need to expose ISA bridge to let driver know the real hardware
> + * underneath. This is a requirement from virtualization team.
> + */
> +
> + assert(pci_dev->devfn == 0x00);
> +
> + if (!is_igd_passthrough(pci_dev)) {
> + goto write_default;
> + }
> +
> + switch (config_addr) {
> + case 0x58: /* PAVPC Offset */
> + break;
> + default:
> + /* Just sets the emulated values. */
> + goto write_default;
> + }
> +
> + /* Host write */
> + r = xen_host_pci_device_get(&dev, 0, 0, 0, 0);
> + if (r) {
> + XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
> + abort();
> + }
> +
> + r = xen_host_pci_set_block(&dev, config_addr, (uint8_t *)&val, len);
> + if (r) {
> + XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
> + abort();
> + }
> +
> + xen_host_pci_device_put(&dev);
> +
> + return;
> +
> +write_default:
> + pci_default_write_config(pci_dev, config_addr, val, len);
> +}
> +
> +uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len)
> +{
> + XenHostPCIDevice dev;
> + uint32_t val;
> + int r;
> +
> + /* IGD read/write is through the host bridge.
> + * ISA bridge is only for detect purpose. In i915 driver it will
> + * probe ISA bridge to discover the IGD, see comment in i915_drv.c:
> + * intel_detect_pch():
> + * The reason to probe ISA bridge instead of Dev31:Fun0 is to
> + * make graphics device passthrough work easy for VMM, that only
> + * need to expose ISA bridge to let driver know the real hardware
> + * underneath. This is a requirement from virtualization team.
> + */
> + assert(pci_dev->devfn == 0x00);
> +
> + if (!is_igd_passthrough(pci_dev)) {
> + goto read_default;
> + }
> +
> + switch (config_addr) {
> + case 0x00: /* vendor id */
> + case 0x02: /* device id */
> + case 0x08: /* revision id */
> + case 0x2c: /* sybsystem vendor id */
> + case 0x2e: /* sybsystem id */
> + case 0x50: /* SNB: processor graphics control register */
> + case 0x52: /* processor graphics control register */
> + case 0xa0: /* top of memory */
> + case 0xb0: /* ILK: BSM: should read from dev 2 offset 0x5c */
> + case 0x58: /* SNB: PAVPC Offset */
> + case 0xa4: /* SNB: graphics base of stolen memory */
> + case 0xa8: /* SNB: base of GTT stolen memory */
> + break;
> + default:
> + /* Just gets the emulated values. */
> + goto read_default;
> + }
> +
> + /* Host read */
> + r = xen_host_pci_device_get(&dev, 0, 0, 0, 0);
> + if (r) {
> + goto err_out;
> + }
> +
> + r = xen_host_pci_get_block(&dev, config_addr, (uint8_t *)&val, len);
> + if (r) {
> + goto err_out;
> + }
> +
> + xen_host_pci_device_put(&dev);
> +
> + return val;
> +
> +read_default:
> + return pci_default_read_config(pci_dev, config_addr, len);
> +
> +err_out:
> + XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
> + return -1;
> +}
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v3][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough
2014-05-26 9:43 ` [v3][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough Tiejun Chen
2014-05-27 14:48 ` [Xen-devel] " Konrad Rzeszutek Wilk
@ 2014-05-27 18:04 ` Stefano Stabellini
2014-05-28 1:58 ` Chen, Tiejun
1 sibling, 1 reply; 28+ messages in thread
From: Stefano Stabellini @ 2014-05-27 18:04 UTC (permalink / raw)
To: Tiejun Chen
Cc: peter.maydell, xen-devel, mst, stefano.stabellini, allen.m.kay,
qemu-devel, Kelly.Zytaruk, yang.z.zhang, anthony, anthony.perard
On Mon, 26 May 2014, Tiejun Chen wrote:
> Implement that pci host bridge to specific to passthrough. Actually
> thsi just inherit the standard one.
>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
> v3:
>
> * Just fix this patch head description typo.
>
> v2:
>
> * New patch.
>
> hw/pci-host/piix.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index ffdc853..8b11953 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -34,6 +34,7 @@
> #include "sysemu/sysemu.h"
> #include "hw/i386/ioapic.h"
> #include "qapi/visitor.h"
> +#include "hw/xen/xen_pt.h"
>
> /*
> * I440FX chipset data sheet.
> @@ -95,6 +96,10 @@ typedef struct PIIX3State {
> #define I440FX_PCI_DEVICE(obj) \
> OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>
> +#define TYPE_I440FX_XEN_PCI_DEVICE "i440FX-xen"
> +#define I440FX_XEN_PCI_DEVICE(obj) \
> + OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_XEN_PCI_DEVICE)
> +
> struct PCII440FXState {
> /*< private >*/
> PCIDevice parent_obj;
> @@ -305,6 +310,16 @@ static int i440fx_initfn(PCIDevice *dev)
> return 0;
> }
>
> +static int i440fx_xen_initfn(PCIDevice *dev)
> +{
> + PCII440FXState *d = I440FX_XEN_PCI_DEVICE(dev);
> +
> + dev->config[I440FX_SMRAM] = 0x02;
> +
> + cpu_smm_register(&i440fx_set_smm, d);
> + return 0;
> +}
> +
> PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> int *piix3_devfn,
> ISABus **isa_bus, qemu_irq *pic,
> @@ -333,8 +348,15 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
> qdev_init_nofail(dev);
>
> - d = pci_create_simple(b, 0, TYPE_I440FX_PCI_DEVICE);
> - *pi440fx_state = I440FX_PCI_DEVICE(d);
> + if (xen_enabled()) {
if (xen_enabled() && xen_has_gfx_passthru) ?
> + d = pci_create_simple(b, 0, TYPE_I440FX_XEN_PCI_DEVICE);
> + *pi440fx_state = I440FX_XEN_PCI_DEVICE(d);
> + pci_create_pch(b);
> + } else {
> + d = pci_create_simple(b, 0, TYPE_I440FX_PCI_DEVICE);
> + *pi440fx_state = I440FX_PCI_DEVICE(d);
> + }
> +
> f = *pi440fx_state;
> f->system_memory = address_space_mem;
> f->pci_address_space = pci_address_space;
> @@ -705,6 +727,35 @@ static const TypeInfo i440fx_info = {
> .class_init = i440fx_class_init,
> };
>
> +static void i440fx_xen_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> + k->init = i440fx_xen_initfn;
> + k->config_write = igd_pci_write;
> + k->config_read = igd_pci_read;
> + k->vendor_id = PCI_VENDOR_ID_INTEL;
> + k->device_id = PCI_DEVICE_ID_INTEL_82441;
> + k->revision = 0x02;
> + k->class_id = PCI_CLASS_BRIDGE_ISA;
> + dc->desc = "XEN Host bridge";
> + dc->vmsd = &vmstate_i440fx;
> + /*
> + * PCI-facing part of the host bridge, not usable without the
> + * host-facing part, which can't be device_add'ed, yet.
> + */
> + dc->cannot_instantiate_with_device_add_yet = true;
> + dc->hotpluggable = false;
> +}
> +
> +static const TypeInfo i440fx_xen_info = {
> + .name = TYPE_I440FX_XEN_PCI_DEVICE,
> + .parent = TYPE_PCI_DEVICE,
> + .instance_size = sizeof(PCII440FXState),
> + .class_init = i440fx_xen_class_init,
> +};
> +
> static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
> PCIBus *rootbus)
> {
> @@ -746,6 +797,7 @@ static const TypeInfo i440fx_pcihost_info = {
> static void i440fx_register_types(void)
> {
> type_register_static(&i440fx_info);
> + type_register_static(&i440fx_xen_info);
> type_register_static(&piix3_info);
> type_register_static(&piix3_xen_info);
> type_register_static(&i440fx_pcihost_info);
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xen-devel] [v3][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough
2014-05-27 14:48 ` [Xen-devel] " Konrad Rzeszutek Wilk
@ 2014-05-28 1:10 ` Chen, Tiejun
0 siblings, 0 replies; 28+ messages in thread
From: Chen, Tiejun @ 2014-05-28 1:10 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com,
mst@redhat.com, stefano.stabellini@eu.citrix.com, Kay, Allen M,
Kelly.Zytaruk@amd.com, qemu-devel@nongnu.org, Zhang, Yang Z,
anthony@codemonkey.ws, anthony.perard@citrix.com
> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Tuesday, May 27, 2014 10:48 PM
> To: Chen, Tiejun
> Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com;
> mst@redhat.com; Kelly.Zytaruk@amd.com; peter.maydell@linaro.org;
> xen-devel@lists.xensource.com; Kay, Allen M; qemu-devel@nongnu.org;
> anthony@codemonkey.ws; Zhang, Yang Z
> Subject: Re: [Xen-devel] [v3][PATCH 4/5] xen, gfx passthrough: create host
> bridge to passthrough
>
> On Mon, May 26, 2014 at 05:43:08PM +0800, Tiejun Chen wrote:
> > Implement that pci host bridge to specific to passthrough. Actually
> > thsi just inherit the standard one.
>
> s/thsi/this
>
Fixed.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xen-devel] [v3][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support
2014-05-27 14:54 ` [Xen-devel] " Konrad Rzeszutek Wilk
@ 2014-05-28 1:16 ` Chen, Tiejun
0 siblings, 0 replies; 28+ messages in thread
From: Chen, Tiejun @ 2014-05-28 1:16 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com,
mst@redhat.com, stefano.stabellini@eu.citrix.com, Kay, Allen M,
Kelly.Zytaruk@amd.com, qemu-devel@nongnu.org, Zhang, Yang Z,
anthony@codemonkey.ws, anthony.perard@citrix.com
> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Tuesday, May 27, 2014 10:55 PM
> To: Chen, Tiejun
> Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com;
> mst@redhat.com; Kelly.Zytaruk@amd.com; peter.maydell@linaro.org;
> xen-devel@lists.xensource.com; Kay, Allen M; qemu-devel@nongnu.org;
> anthony@codemonkey.ws; Zhang, Yang Z
> Subject: Re: [Xen-devel] [v3][PATCH 1/5] xen, gfx passthrough: basic graphics
> passthrough support
>
> On Mon, May 26, 2014 at 05:43:05PM +0800, Tiejun Chen wrote:
> > basic gfx passthrough support:
> > - add a vga type for gfx passthrough
> > - retrieve VGA bios from sysfs, then load it to guest 0xC0000
>
> s/guest/guest at/
Fixed.
> > - register/unregister legacy VGA I/O ports and MMIOs for passthroughed
> > gfx
>
> s/passthrouged gfx/passthrough GFX/
Fixed.
> >
> > The original patch is from Weidong Han <weidong.han@intel.com>
> >
> > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> > Cc: Weidong Han <weidong.han@intel.com>
> > ---
> > v3:
> >
> > * Fix some typos.
> > * Add more comments to make that readable.
> > * Improve some return paths.
> >
> > v2:
> >
> > * retrieve VGA bios from sysfs properly.
> > * redefine some function name.
> >
> > hw/xen/Makefile.objs | 2 +-
> > hw/xen/xen-host-pci-device.c | 5 +
> > hw/xen/xen-host-pci-device.h | 1 +
> > hw/xen/xen_pt.c | 10 ++
> > hw/xen/xen_pt.h | 4 +
> > hw/xen/xen_pt_graphics.c | 232
> +++++++++++++++++++++++++++++++++++++++++++
> > qemu-options.hx | 9 ++
> > vl.c | 11 +-
> > 8 files changed, 272 insertions(+), 2 deletions(-) create mode
> > 100644 hw/xen/xen_pt_graphics.c
> >
> > diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs index
> > a0ca0aa..77b7dae 100644
> > --- a/hw/xen/Makefile.objs
> > +++ b/hw/xen/Makefile.objs
> > @@ -2,4 +2,4 @@
> > common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
> >
> > obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
> > -obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o
> > xen_pt_msi.o
> > +obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o
> > +xen_pt_msi.o xen_pt_graphics.o
> > diff --git a/hw/xen/xen-host-pci-device.c
> > b/hw/xen/xen-host-pci-device.c index 743b37b..a54b7de 100644
> > --- a/hw/xen/xen-host-pci-device.c
> > +++ b/hw/xen/xen-host-pci-device.c
> > @@ -376,6 +376,11 @@ int xen_host_pci_device_get(XenHostPCIDevice *d,
> uint16_t domain,
> > goto error;
> > }
> > d->irq = v;
> > + rc = xen_host_pci_get_hex_value(d, "class", &v);
> > + if (rc) {
> > + goto error;
> > + }
> > + d->class_code = v;
> > d->is_virtfn = xen_host_pci_dev_is_virtfn(d);
> >
> > return 0;
> > diff --git a/hw/xen/xen-host-pci-device.h
> > b/hw/xen/xen-host-pci-device.h index c2486f0..f1e1c30 100644
> > --- a/hw/xen/xen-host-pci-device.h
> > +++ b/hw/xen/xen-host-pci-device.h
> > @@ -25,6 +25,7 @@ typedef struct XenHostPCIDevice {
> >
> > uint16_t vendor_id;
> > uint16_t device_id;
> > + uint32_t class_code;
> > int irq;
> >
> > XenHostPCIIORegion io_regions[PCI_NUM_REGIONS - 1]; diff --git
> > a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index be4220b..a0113ea 100644
> > --- a/hw/xen/xen_pt.c
> > +++ b/hw/xen/xen_pt.c
> > @@ -450,6 +450,7 @@ static int
> xen_pt_register_regions(XenPCIPassthroughState *s)
> > d->rom.size, d->rom.base_addr);
> > }
> >
> > + xen_pt_register_vga_regions(d);
> > return 0;
> > }
> >
> > @@ -470,6 +471,8 @@ static void
> xen_pt_unregister_regions(XenPCIPassthroughState *s)
> > if (d->rom.base_addr && d->rom.size) {
> > memory_region_destroy(&s->rom);
> > }
> > +
> > + xen_pt_unregister_vga_regions(d);
> > }
> >
> > /* region mapping */
> > @@ -693,6 +696,13 @@ static int xen_pt_initfn(PCIDevice *d)
> > /* Handle real device's MMIO/PIO BARs */
> > xen_pt_register_regions(s);
> >
> > + /* Setup VGA bios for passthroughed gfx */
>
> Ditto
>
Fixed.
> > + if (xen_pt_setup_vga(&s->real_device) < 0) {
> > + XEN_PT_ERR(d, "Setup VGA BIOS of passthroughed gfx
> > + failed!\n");
>
> Ditto
>
Fixed.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xen-devel] [v3][PATCH 2/5] xen, gfx passthrough: create intel isa bridge
2014-05-27 14:55 ` [Xen-devel] " Konrad Rzeszutek Wilk
2014-05-27 17:52 ` Stefano Stabellini
@ 2014-05-28 1:20 ` Chen, Tiejun
1 sibling, 0 replies; 28+ messages in thread
From: Chen, Tiejun @ 2014-05-28 1:20 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com,
mst@redhat.com, stefano.stabellini@eu.citrix.com, Kay, Allen M,
Kelly.Zytaruk@amd.com, qemu-devel@nongnu.org, Zhang, Yang Z,
anthony@codemonkey.ws, anthony.perard@citrix.com
> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Tuesday, May 27, 2014 10:56 PM
> To: Chen, Tiejun
> Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com;
> mst@redhat.com; Kelly.Zytaruk@amd.com; peter.maydell@linaro.org;
> xen-devel@lists.xensource.com; Kay, Allen M; qemu-devel@nongnu.org;
> anthony@codemonkey.ws; Zhang, Yang Z
> Subject: Re: [Xen-devel] [v3][PATCH 2/5] xen, gfx passthrough: create intel isa
> bridge
>
> On Mon, May 26, 2014 at 05:43:06PM +0800, Tiejun Chen wrote:
> > ISA bridge is needed since Intel gfx drive will probe it instead of
> > Dev31:Fun0 to make graphics device passthrough work easy for VMM, that
> > only need to expose ISA bridge to let driver know the real hardware
> underneath.
> >
> > The original patch is from Allen Kay [allen.m.kay@intel.com]
> >
> > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> > Cc: Allen Kay <allen.m.kay@intel.com>
> > ---
> > v3:
> >
> > * Fix some typos.
> > * Improve some return paths.
> >
> > v2:
> >
> > * Nothing is changed.
> >
> > hw/xen/xen_pt_graphics.c | 63
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 63 insertions(+)
> >
> > diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c index
> > e63bd6f..51b174f 100644
> > --- a/hw/xen/xen_pt_graphics.c
> > +++ b/hw/xen/xen_pt_graphics.c
> > @@ -230,3 +230,66 @@ out:
> > g_free(bios);
> > return rc;
> > }
> > +
> > +static uint32_t isa_bridge_read_config(PCIDevice *d, uint32_t addr,
> > +int len) {
> > + return pci_default_read_config(d, addr, len); }
> > +
> > +static void isa_bridge_write_config(PCIDevice *d, uint32_t addr, uint32_t v,
> > + int len) {
> > + pci_default_write_config(d, addr, v, len);
> > +
> > + return;
>
> You don't need the return there.
Fixed.
> > +}
> > +
> > +static void isa_bridge_class_init(ObjectClass *klass, void *data) {
> > + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> > +
> > + k->config_read = isa_bridge_read_config;
> > + k->config_write = isa_bridge_write_config;
> > +
> > + return;
>
> Ditto
Fixed.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping
2014-05-27 17:35 ` Stefano Stabellini
@ 2014-05-28 1:31 ` Chen, Tiejun
2014-05-28 12:23 ` Stefano Stabellini
0 siblings, 1 reply; 28+ messages in thread
From: Chen, Tiejun @ 2014-05-28 1:31 UTC (permalink / raw)
To: Stefano Stabellini
Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com,
mst@redhat.com, Kay, Allen M, qemu-devel@nongnu.org,
Kelly.Zytaruk@amd.com, Zhang, Yang Z, anthony@codemonkey.ws,
anthony.perard@citrix.com
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: Wednesday, May 28, 2014 1:36 AM
> To: Chen, Tiejun
> Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com;
> mst@redhat.com; Kelly.Zytaruk@amd.com; qemu-devel@nongnu.org;
> xen-devel@lists.xensource.com; peter.maydell@linaro.org;
> anthony@codemonkey.ws; Kay, Allen M; Zhang, Yang Z
> Subject: Re: [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping
>
> On Mon, 26 May 2014, Tiejun Chen wrote:
> > The OpRegion shouldn't be mapped 1:1 because the address in the host
> > can't be used in the guest directly.
> >
> > This patch traps read and write access to the opregion of the Intel
> > GPU config space (offset 0xfc).
> >
> > The original patch is from Jean Guyader <jean.guyader@eu.citrix.com>
> >
> > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> > Cc: Jean Guyader <jean.guyader@eu.citrix.com>
> > ---
> > v3:
> >
> > * Fix some typos.
> > * Add more comments to make that readable.
> > * To unmap igd_opregion when call xen_pt_unregister_vga_regions().
> > * Improve some return paths.
> > * We need to map 3 pages for opregion as hvmloader set.
> > * Force to convert igd_guest/host_opoegion as an unsigned long type
> > while calling xc_domain_memory_mapping().
> >
> > v2:
> >
> > * We should return zero as an invalid address value while calling
> > igd_read_opregion().
> >
[snip]
> > +
> > +uint32_t igd_read_opregion(XenPCIPassthroughState *s) {
> > + uint32_t val = 0;
> > +
> > + if (igd_guest_opregion == 0) {
> > + return val;
>
> Sorry for not having spotted it before, but isn't this supposed to return an error?
> The old code returns -1 in this case.
I already commented this in v2 log above.
We shouldn't return "-1" to indicate an invalid address since we often think "!0" is a valid address value.
In Linux instance,
drivers/gpu/drm/i915/intel_opregion.c:
int intel_opregion_setup(struct drm_device *dev)
{
...
pci_read_config_dword(dev->pdev, PCI_ASLS, &asls);
DRM_DEBUG_DRIVER("graphic opregion physical addr: 0x%x\n", asls);
if (asls == 0) {
DRM_DEBUG_DRIVER("ACPI OpRegion not supported!\n");
return -ENOTSUPP;
}
...
>
>
> > + }
> > +
> > + val = igd_guest_opregion;
> > +
> > + XEN_PT_LOG(&s->dev, "Read opregion val=%x\n", val);
> > + return val;
> > +}
> > +
> > +void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val) {
> > + int ret;
> > +
> > + if (igd_guest_opregion) {
> > + XEN_PT_LOG(&s->dev, "opregion register already been set,
> ignoring %x\n",
> > + val);
> > + return;
> > + }
> > +
> > + xen_host_pci_get_block(&s->real_device, PCI_INTEL_OPREGION,
> > + (uint8_t *)&igd_host_opregion, 4);
> > + igd_guest_opregion = (unsigned long)(val & ~0xfff)
> > + | (igd_host_opregion & 0xfff);
> > +
> > + ret = xc_domain_memory_mapping(xen_xc, xen_domid,
> > + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT),
> > + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
> > + 3,
>
> Why 3 pages instead of 2?
commit 408a9e56343b006c9e58a334f0b97dd2deedf9ac
Author: Keir Fraser <keir@xen.org>
Date: Thu Jan 10 17:26:24 2013 +0000
hvmloader: Allocate 3 pages for Intel GPU OpRegion passthrough.
The 8kB region may not be page aligned, hence requiring 3 pages to
be mapped through.
Signed-off-by: Keir Fraser <keir@xen.org>
diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h
index 3a4e145..7f8a90f 100644
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -5,7 +5,9 @@
enum virtual_vga { VGA_none, VGA_std, VGA_cirrus, VGA_pt };
extern enum virtual_vga virtual_vga;
+
extern unsigned long igd_opregion_pgbase;
+#define IGD_OPREGION_PAGES 3
...
Thanks
Tiejun
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [v3][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support
2014-05-27 17:47 ` Stefano Stabellini
@ 2014-05-28 1:43 ` Chen, Tiejun
0 siblings, 0 replies; 28+ messages in thread
From: Chen, Tiejun @ 2014-05-28 1:43 UTC (permalink / raw)
To: Stefano Stabellini
Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com,
mst@redhat.com, Kay, Allen M, qemu-devel@nongnu.org,
Kelly.Zytaruk@amd.com, Zhang, Yang Z, anthony@codemonkey.ws,
anthony.perard@citrix.com
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: Wednesday, May 28, 2014 1:48 AM
> To: Chen, Tiejun
> Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com;
> mst@redhat.com; Kelly.Zytaruk@amd.com; qemu-devel@nongnu.org;
> xen-devel@lists.xensource.com; peter.maydell@linaro.org;
> anthony@codemonkey.ws; Kay, Allen M; Zhang, Yang Z
> Subject: Re: [v3][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough
> support
>
[snip]
> > + * register VGA resources for the domain with assigned gfx */ int
> > +xen_pt_register_vga_regions(XenHostPCIDevice *dev) {
> > + int i = 0;
> > +
> > + if (!is_vga_passthrough(dev)) {
> > + return -1;
>
> Given that xen_pt_register_vga_regions is called unconditionally, this is not an
> error condition, isn't it? It should return 0.
Okay.
>
[snip]
> > + * unregister VGA resources for the domain with assigned gfx */ int
> > +xen_pt_unregister_vga_regions(XenHostPCIDevice *dev) {
> > + int i = 0;
> > +
> > + if (!is_vga_passthrough(dev)) {
> > + return -1;
>
> same here
>
Fixed.
>
[snip]
> > const ram_addr_t default_ram_size =
> (ram_addr_t)DEFAULT_RAM_SIZE *
> > 1024 * 1024;
> > -
>
> Spurious change
>
Removed.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xen-devel] [v3][PATCH 2/5] xen, gfx passthrough: create intel isa bridge
2014-05-27 17:52 ` Stefano Stabellini
@ 2014-05-28 1:44 ` Chen, Tiejun
0 siblings, 0 replies; 28+ messages in thread
From: Chen, Tiejun @ 2014-05-28 1:44 UTC (permalink / raw)
To: Stefano Stabellini, Konrad Rzeszutek Wilk
Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com,
mst@redhat.com, Kay, Allen M, Kelly.Zytaruk@amd.com,
qemu-devel@nongnu.org, Zhang, Yang Z, anthony@codemonkey.ws,
anthony.perard@citrix.com
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: Wednesday, May 28, 2014 1:52 AM
> To: Konrad Rzeszutek Wilk
> Cc: Chen, Tiejun; anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com;
> mst@redhat.com; Kelly.Zytaruk@amd.com; peter.maydell@linaro.org;
> xen-devel@lists.xensource.com; Kay, Allen M; qemu-devel@nongnu.org;
> anthony@codemonkey.ws; Zhang, Yang Z
> Subject: Re: [Xen-devel] [v3][PATCH 2/5] xen, gfx passthrough: create intel isa
> bridge
>
> On Tue, 27 May 2014, Konrad Rzeszutek Wilk wrote:
> > On Mon, May 26, 2014 at 05:43:06PM +0800, Tiejun Chen wrote:
> > > ISA bridge is needed since Intel gfx drive will probe it instead of
> > > Dev31:Fun0 to make graphics device passthrough work easy for VMM,
> > > that only need to expose ISA bridge to let driver know the real hardware
> underneath.
> > >
> > > The original patch is from Allen Kay [allen.m.kay@intel.com]
> > >
> > > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> > > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> > > Cc: Allen Kay <allen.m.kay@intel.com>
> > > ---
> > > v3:
> > >
> > > * Fix some typos.
> > > * Improve some return paths.
> > >
> > > v2:
> > >
> > > * Nothing is changed.
> > >
> > > hw/xen/xen_pt_graphics.c | 63
> > > ++++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 63 insertions(+)
> > >
> > > diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> > > index e63bd6f..51b174f 100644
> > > --- a/hw/xen/xen_pt_graphics.c
> > > +++ b/hw/xen/xen_pt_graphics.c
> > > @@ -230,3 +230,66 @@ out:
> > > g_free(bios);
> > > return rc;
> > > }
> > > +
> > > +static uint32_t isa_bridge_read_config(PCIDevice *d, uint32_t addr,
> > > +int len) {
> > > + return pci_default_read_config(d, addr, len); }
> > > +
> > > +static void isa_bridge_write_config(PCIDevice *d, uint32_t addr, uint32_t
> v,
> > > + int len) {
> > > + pci_default_write_config(d, addr, v, len);
> > > +
> > > + return;
> >
> > You don't need the return there.
> > > +}
> > > +
> > > +static void isa_bridge_class_init(ObjectClass *klass, void *data) {
> > > + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> > > +
> > > + k->config_read = isa_bridge_read_config;
> > > + k->config_write = isa_bridge_write_config;
> > > +
> > > + return;
> >
> > Ditto
>
> Aside from these 2 minor changes, the patch looks good to me.
>
Already addressed
Thanks
Tiejun
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D
2014-05-27 18:02 ` Stefano Stabellini
@ 2014-05-28 1:51 ` Chen, Tiejun
2014-05-28 12:29 ` Stefano Stabellini
0 siblings, 1 reply; 28+ messages in thread
From: Chen, Tiejun @ 2014-05-28 1:51 UTC (permalink / raw)
To: Stefano Stabellini
Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com,
mst@redhat.com, Kay, Allen M, qemu-devel@nongnu.org,
Kelly.Zytaruk@amd.com, Zhang, Yang Z, anthony@codemonkey.ws,
anthony.perard@citrix.com
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: Wednesday, May 28, 2014 2:02 AM
> To: Chen, Tiejun
> Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com;
> mst@redhat.com; Kelly.Zytaruk@amd.com; qemu-devel@nongnu.org;
> xen-devel@lists.xensource.com; peter.maydell@linaro.org;
> anthony@codemonkey.ws; Kay, Allen M; Zhang, Yang Z
> Subject: Re: [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD
> passthrough with VT-D
>
> On Mon, 26 May 2014, Tiejun Chen wrote:
> > Some registers of Intel IGD are mapped in host bridge, so it needs to
> > passthrough these registers of physical host bridge to guest because
> > emulated host bridge in guest doesn't have these mappings.
> >
> > The original patch is from Weidong Han < weidong.han @ intel.com >
> >
> > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com> Cc:Weidong Han
> > <weidong.han@intel.com>
> > ---
> > v3:
> >
> > * Improve comments to make that readable.
> >
> > v2:
> >
> > * To introduce is_igd_passthrough() to make sure we touch physical host
> bridge
> > only in IGD case.
> >
[snip]
> > +int pci_create_pch(PCIBus *bus)
> > +{
> > + XenHostPCIDevice hdev;
> > + int r = 0;
> > +
> > + if (!xen_has_gfx_passthru) {
> > + return -1;
>
> If this function ends up being called unconditionally, then this should return 0;
>
Fixed.
>
> > + }
> > +
> > + r = xen_host_pci_device_get(&hdev, 0, 0, 0x1f, 0);
> > + if (r) {
> > + XEN_PT_ERR(NULL, "Failed to find Intel PCH on host\n");
> > + goto err;
> > + }
> > +
> > + if (hdev.vendor_id == PCI_VENDOR_ID_INTEL) {
> > + r = create_pch_isa_bridge(bus, &hdev);
> > + if (r) {
> > + XEN_PT_ERR(NULL, "Failed to create PCH ISA bridge.\n");
> > + goto err;
> > + }
> > + }
> > +
> > + xen_host_pci_device_put(&hdev);
> > +
> > +err:
> > + return r;
> > +}
> > +
> > +/*
> > + * Currently we just pass this physical host bridge for IGD, 00:02.0.
> > + */
> > +static int is_igd_passthrough(PCIDevice *pci_dev) {
> > + PCIDevice *f = pci_dev->bus->devices[PCI_DEVFN(2, 0)];
> > + if (pci_dev->bus->devices[PCI_DEVFN(2, 0)]) {
>
> Isn't the purpose of this function to check that the *current* device is the
> graphic passthrough device?
No.
> In that case, shouldn't it just be:
>
> if (pci_dev) {
>
Here pci_dev is just that host bridge, so here we have to get that real passthrough device by that given devfn to further confirm.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v3][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough
2014-05-27 18:04 ` Stefano Stabellini
@ 2014-05-28 1:58 ` Chen, Tiejun
0 siblings, 0 replies; 28+ messages in thread
From: Chen, Tiejun @ 2014-05-28 1:58 UTC (permalink / raw)
To: Stefano Stabellini
Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com,
mst@redhat.com, Kay, Allen M, Kelly.Zytaruk@amd.com,
qemu-devel@nongnu.org, anthony.perard@citrix.com,
anthony@codemonkey.ws, Zhang, Yang Z
> -----Original Message-----
> From: qemu-devel-bounces+tiejun.chen=intel.com@nongnu.org
> [mailto:qemu-devel-bounces+tiejun.chen=intel.com@nongnu.org] On Behalf Of
> Stefano Stabellini
> Sent: Wednesday, May 28, 2014 2:05 AM
> To: Chen, Tiejun
> Cc: peter.maydell@linaro.org; xen-devel@lists.xensource.com;
> mst@redhat.com; stefano.stabellini@eu.citrix.com; Kay, Allen M;
> qemu-devel@nongnu.org; Kelly.Zytaruk@amd.com; Zhang, Yang Z;
> anthony@codemonkey.ws; anthony.perard@citrix.com
> Subject: Re: [Qemu-devel] [v3][PATCH 4/5] xen, gfx passthrough: create host
> bridge to passthrough
>
> On Mon, 26 May 2014, Tiejun Chen wrote:
> > Implement that pci host bridge to specific to passthrough. Actually
> > thsi just inherit the standard one.
> >
> > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> > ---
> > v3:
> >
> > * Just fix this patch head description typo.
> >
> > v2:
> >
> > * New patch.
> >
[snip]
> > PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> > int *piix3_devfn,
> > ISABus **isa_bus, qemu_irq *pic, @@ -333,8
> > +348,15 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> > object_property_add_child(qdev_get_machine(), "i440fx",
> OBJECT(dev), NULL);
> > qdev_init_nofail(dev);
> >
> > - d = pci_create_simple(b, 0, TYPE_I440FX_PCI_DEVICE);
> > - *pi440fx_state = I440FX_PCI_DEVICE(d);
> > + if (xen_enabled()) {
>
> if (xen_enabled() && xen_has_gfx_passthru) ?
>
Fine.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping
2014-05-28 1:31 ` Chen, Tiejun
@ 2014-05-28 12:23 ` Stefano Stabellini
2014-05-29 1:38 ` Chen, Tiejun
0 siblings, 1 reply; 28+ messages in thread
From: Stefano Stabellini @ 2014-05-28 12:23 UTC (permalink / raw)
To: Chen, Tiejun
Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com,
mst@redhat.com, Stefano Stabellini, Kay, Allen M,
qemu-devel@nongnu.org, Kelly.Zytaruk@amd.com, Zhang, Yang Z,
anthony@codemonkey.ws, anthony.perard@citrix.com
On Wed, 28 May 2014, Chen, Tiejun wrote:
> > -----Original Message-----
> > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > Sent: Wednesday, May 28, 2014 1:36 AM
> > To: Chen, Tiejun
> > Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com;
> > mst@redhat.com; Kelly.Zytaruk@amd.com; qemu-devel@nongnu.org;
> > xen-devel@lists.xensource.com; peter.maydell@linaro.org;
> > anthony@codemonkey.ws; Kay, Allen M; Zhang, Yang Z
> > Subject: Re: [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping
> >
> > On Mon, 26 May 2014, Tiejun Chen wrote:
> > > The OpRegion shouldn't be mapped 1:1 because the address in the host
> > > can't be used in the guest directly.
> > >
> > > This patch traps read and write access to the opregion of the Intel
> > > GPU config space (offset 0xfc).
> > >
> > > The original patch is from Jean Guyader <jean.guyader@eu.citrix.com>
> > >
> > > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> > > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> > > Cc: Jean Guyader <jean.guyader@eu.citrix.com>
> > > ---
> > > v3:
> > >
> > > * Fix some typos.
> > > * Add more comments to make that readable.
> > > * To unmap igd_opregion when call xen_pt_unregister_vga_regions().
> > > * Improve some return paths.
> > > * We need to map 3 pages for opregion as hvmloader set.
> > > * Force to convert igd_guest/host_opoegion as an unsigned long type
> > > while calling xc_domain_memory_mapping().
> > >
> > > v2:
> > >
> > > * We should return zero as an invalid address value while calling
> > > igd_read_opregion().
> > >
>
> [snip]
>
> > > +
> > > +uint32_t igd_read_opregion(XenPCIPassthroughState *s) {
> > > + uint32_t val = 0;
> > > +
> > > + if (igd_guest_opregion == 0) {
> > > + return val;
> >
> > Sorry for not having spotted it before, but isn't this supposed to return an error?
> > The old code returns -1 in this case.
>
> I already commented this in v2 log above.
>
> We shouldn't return "-1" to indicate an invalid address since we often think "!0" is a valid address value.
>
> In Linux instance,
>
> drivers/gpu/drm/i915/intel_opregion.c:
>
> int intel_opregion_setup(struct drm_device *dev)
> {
> ...
> pci_read_config_dword(dev->pdev, PCI_ASLS, &asls);
> DRM_DEBUG_DRIVER("graphic opregion physical addr: 0x%x\n", asls);
> if (asls == 0) {
> DRM_DEBUG_DRIVER("ACPI OpRegion not supported!\n");
> return -ENOTSUPP;
> }
> ...
Ops, you are right! igd_read_opregion returns an address not an error
code.
In that case, given that xen_pt_intel_opregion_read can return an error
code as well as an address, maybe it makes sense to do the same in
igd_read_opregion and allow the function to return an error code and set
the value by pointer.
> >
> >
> > > + }
> > > +
> > > + val = igd_guest_opregion;
> > > +
> > > + XEN_PT_LOG(&s->dev, "Read opregion val=%x\n", val);
> > > + return val;
> > > +}
> > > +
> > > +void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val) {
> > > + int ret;
> > > +
> > > + if (igd_guest_opregion) {
> > > + XEN_PT_LOG(&s->dev, "opregion register already been set,
> > ignoring %x\n",
> > > + val);
> > > + return;
> > > + }
> > > +
> > > + xen_host_pci_get_block(&s->real_device, PCI_INTEL_OPREGION,
> > > + (uint8_t *)&igd_host_opregion, 4);
> > > + igd_guest_opregion = (unsigned long)(val & ~0xfff)
> > > + | (igd_host_opregion & 0xfff);
> > > +
> > > + ret = xc_domain_memory_mapping(xen_xc, xen_domid,
> > > + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT),
> > > + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
> > > + 3,
> >
> > Why 3 pages instead of 2?
Thanks.
> commit 408a9e56343b006c9e58a334f0b97dd2deedf9ac
> Author: Keir Fraser <keir@xen.org>
> Date: Thu Jan 10 17:26:24 2013 +0000
>
> hvmloader: Allocate 3 pages for Intel GPU OpRegion passthrough.
>
> The 8kB region may not be page aligned, hence requiring 3 pages to
> be mapped through.
>
> Signed-off-by: Keir Fraser <keir@xen.org>
>
> diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h
> index 3a4e145..7f8a90f 100644
> --- a/tools/firmware/hvmloader/config.h
> +++ b/tools/firmware/hvmloader/config.h
> @@ -5,7 +5,9 @@
>
> enum virtual_vga { VGA_none, VGA_std, VGA_cirrus, VGA_pt };
> extern enum virtual_vga virtual_vga;
> +
> extern unsigned long igd_opregion_pgbase;
> +#define IGD_OPREGION_PAGES 3
> ...
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D
2014-05-28 1:51 ` Chen, Tiejun
@ 2014-05-28 12:29 ` Stefano Stabellini
2014-05-29 1:15 ` Chen, Tiejun
0 siblings, 1 reply; 28+ messages in thread
From: Stefano Stabellini @ 2014-05-28 12:29 UTC (permalink / raw)
To: Chen, Tiejun
Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com,
mst@redhat.com, Stefano Stabellini, Kay, Allen M,
qemu-devel@nongnu.org, Kelly.Zytaruk@amd.com, Zhang, Yang Z,
anthony@codemonkey.ws, anthony.perard@citrix.com
On Wed, 28 May 2014, Chen, Tiejun wrote:
> > -----Original Message-----
> > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > Sent: Wednesday, May 28, 2014 2:02 AM
> > To: Chen, Tiejun
> > Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com;
> > mst@redhat.com; Kelly.Zytaruk@amd.com; qemu-devel@nongnu.org;
> > xen-devel@lists.xensource.com; peter.maydell@linaro.org;
> > anthony@codemonkey.ws; Kay, Allen M; Zhang, Yang Z
> > Subject: Re: [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD
> > passthrough with VT-D
> >
> > On Mon, 26 May 2014, Tiejun Chen wrote:
> > > Some registers of Intel IGD are mapped in host bridge, so it needs to
> > > passthrough these registers of physical host bridge to guest because
> > > emulated host bridge in guest doesn't have these mappings.
> > >
> > > The original patch is from Weidong Han < weidong.han @ intel.com >
> > >
> > > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> > > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com> Cc:Weidong Han
> > > <weidong.han@intel.com>
> > > ---
> > > v3:
> > >
> > > * Improve comments to make that readable.
> > >
> > > v2:
> > >
> > > * To introduce is_igd_passthrough() to make sure we touch physical host
> > bridge
> > > only in IGD case.
> > >
>
> [snip]
>
> > > +int pci_create_pch(PCIBus *bus)
> > > +{
> > > + XenHostPCIDevice hdev;
> > > + int r = 0;
> > > +
> > > + if (!xen_has_gfx_passthru) {
> > > + return -1;
> >
> > If this function ends up being called unconditionally, then this should return 0;
> >
>
> Fixed.
>
> >
> > > + }
> > > +
> > > + r = xen_host_pci_device_get(&hdev, 0, 0, 0x1f, 0);
> > > + if (r) {
> > > + XEN_PT_ERR(NULL, "Failed to find Intel PCH on host\n");
> > > + goto err;
> > > + }
> > > +
> > > + if (hdev.vendor_id == PCI_VENDOR_ID_INTEL) {
> > > + r = create_pch_isa_bridge(bus, &hdev);
> > > + if (r) {
> > > + XEN_PT_ERR(NULL, "Failed to create PCH ISA bridge.\n");
> > > + goto err;
> > > + }
> > > + }
> > > +
> > > + xen_host_pci_device_put(&hdev);
> > > +
> > > +err:
> > > + return r;
> > > +}
> > > +
> > > +/*
> > > + * Currently we just pass this physical host bridge for IGD, 00:02.0.
> > > + */
> > > +static int is_igd_passthrough(PCIDevice *pci_dev) {
> > > + PCIDevice *f = pci_dev->bus->devices[PCI_DEVFN(2, 0)];
> > > + if (pci_dev->bus->devices[PCI_DEVFN(2, 0)]) {
> >
> > Isn't the purpose of this function to check that the *current* device is the
> > graphic passthrough device?
>
> No.
>
> > In that case, shouldn't it just be:
> >
> > if (pci_dev) {
> >
>
> Here pci_dev is just that host bridge, so here we have to get that real passthrough device by that given devfn to further confirm.
I understand now, thanks for the explanation. Maybe you want to expand
the comment on top of is_igd_passthrough.
Cheers,
Stefano
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D
2014-05-28 12:29 ` Stefano Stabellini
@ 2014-05-29 1:15 ` Chen, Tiejun
0 siblings, 0 replies; 28+ messages in thread
From: Chen, Tiejun @ 2014-05-29 1:15 UTC (permalink / raw)
To: Stefano Stabellini
Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com,
mst@redhat.com, Kay, Allen M, qemu-devel@nongnu.org,
Kelly.Zytaruk@amd.com, Zhang, Yang Z, anthony@codemonkey.ws,
anthony.perard@citrix.com
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: Wednesday, May 28, 2014 8:30 PM
> To: Chen, Tiejun
> Cc: Stefano Stabellini; anthony.perard@citrix.com; mst@redhat.com;
> Kelly.Zytaruk@amd.com; qemu-devel@nongnu.org;
> xen-devel@lists.xensource.com; peter.maydell@linaro.org;
> anthony@codemonkey.ws; Kay, Allen M; Zhang, Yang Z
> Subject: RE: [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD
> passthrough with VT-D
>
[snip]
> > > > +/*
> > > > + * Currently we just pass this physical host bridge for IGD, 00:02.0.
> > > > + */
> > > > +static int is_igd_passthrough(PCIDevice *pci_dev) {
> > > > + PCIDevice *f = pci_dev->bus->devices[PCI_DEVFN(2, 0)];
> > > > + if (pci_dev->bus->devices[PCI_DEVFN(2, 0)]) {
> > >
> > > Isn't the purpose of this function to check that the *current*
> > > device is the graphic passthrough device?
> >
> > No.
> >
> > > In that case, shouldn't it just be:
> > >
> > > if (pci_dev) {
> > >
> >
> > Here pci_dev is just that host bridge, so here we have to get that real
> passthrough device by that given devfn to further confirm.
>
> I understand now, thanks for the explanation. Maybe you want to expand the
> comment on top of is_igd_passthrough.
Sure, its easy to do :)
Thanks
Tiejun
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping
2014-05-28 12:23 ` Stefano Stabellini
@ 2014-05-29 1:38 ` Chen, Tiejun
0 siblings, 0 replies; 28+ messages in thread
From: Chen, Tiejun @ 2014-05-29 1:38 UTC (permalink / raw)
To: Stefano Stabellini
Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com,
mst@redhat.com, Kay, Allen M, qemu-devel@nongnu.org,
Kelly.Zytaruk@amd.com, Zhang, Yang Z, anthony@codemonkey.ws,
anthony.perard@citrix.com
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: Wednesday, May 28, 2014 8:24 PM
> To: Chen, Tiejun
> Cc: Stefano Stabellini; anthony.perard@citrix.com; mst@redhat.com;
> Kelly.Zytaruk@amd.com; qemu-devel@nongnu.org;
> xen-devel@lists.xensource.com; peter.maydell@linaro.org;
> anthony@codemonkey.ws; Kay, Allen M; Zhang, Yang Z
> Subject: RE: [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping
>
> On Wed, 28 May 2014, Chen, Tiejun wrote:
> > > -----Original Message-----
> > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > > Sent: Wednesday, May 28, 2014 1:36 AM
> > > To: Chen, Tiejun
> > > Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com;
> > > mst@redhat.com; Kelly.Zytaruk@amd.com; qemu-devel@nongnu.org;
> > > xen-devel@lists.xensource.com; peter.maydell@linaro.org;
> > > anthony@codemonkey.ws; Kay, Allen M; Zhang, Yang Z
> > > Subject: Re: [v3][PATCH 5/5] xen, gfx passthrough: add opregion
> > > mapping
> > >
[snip]
> > > > +
> > > > +uint32_t igd_read_opregion(XenPCIPassthroughState *s) {
> > > > + uint32_t val = 0;
> > > > +
> > > > + if (igd_guest_opregion == 0) {
> > > > + return val;
> > >
> > > Sorry for not having spotted it before, but isn't this supposed to return an
> error?
> > > The old code returns -1 in this case.
> >
> > I already commented this in v2 log above.
> >
> > We shouldn't return "-1" to indicate an invalid address since we often think
> "!0" is a valid address value.
> >
> > In Linux instance,
> >
> > drivers/gpu/drm/i915/intel_opregion.c:
> >
> > int intel_opregion_setup(struct drm_device *dev) {
> > ...
> > pci_read_config_dword(dev->pdev, PCI_ASLS, &asls);
> > DRM_DEBUG_DRIVER("graphic opregion physical addr: 0x%x\n", asls);
> > if (asls == 0) {
> > DRM_DEBUG_DRIVER("ACPI OpRegion not supported!\n");
> > return -ENOTSUPP;
> > }
> > ...
>
> Ops, you are right! igd_read_opregion returns an address not an error code.
> In that case, given that xen_pt_intel_opregion_read can return an error code
> as well as an address, maybe it makes sense to do the same in
> igd_read_opregion and allow the function to return an error code and set the
> value by pointer.
>
I don't think so.
As I understand, we does check if that return value is valid, but not check if this read behavior is good. This pci read transmit should be guaranteed by the hardware, and this should be transparent to driver.
In qemu emulator, qemu should do this check on behavior of the hardware so we need an error code when use xen_pt_intel_oprerion() as a wrapper of igd_read_opregion(). But that return value we're talking about should be delivered directly if this pci read behavior is fine.
But even if this read is failed, we should do something in pci host controller of qemu to further emulate this issue. But as emulator, I think qemu often never do this thing just specific to a normal pci read if you have no any obvious reason.
Thanks
Tiejun
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2014-05-29 1:38 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-26 9:43 [v3][PATCH 0/5] xen: add Intel IGD passthrough support Tiejun Chen
2014-05-26 9:43 ` [v3][PATCH 1/5] xen, gfx passthrough: basic graphics " Tiejun Chen
2014-05-27 14:54 ` [Xen-devel] " Konrad Rzeszutek Wilk
2014-05-28 1:16 ` Chen, Tiejun
2014-05-27 17:47 ` Stefano Stabellini
2014-05-28 1:43 ` Chen, Tiejun
2014-05-26 9:43 ` [v3][PATCH 2/5] xen, gfx passthrough: create intel isa bridge Tiejun Chen
2014-05-27 14:55 ` [Xen-devel] " Konrad Rzeszutek Wilk
2014-05-27 17:52 ` Stefano Stabellini
2014-05-28 1:44 ` Chen, Tiejun
2014-05-28 1:20 ` Chen, Tiejun
2014-05-26 9:43 ` [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D Tiejun Chen
2014-05-27 18:02 ` Stefano Stabellini
2014-05-28 1:51 ` Chen, Tiejun
2014-05-28 12:29 ` Stefano Stabellini
2014-05-29 1:15 ` Chen, Tiejun
2014-05-26 9:43 ` [v3][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough Tiejun Chen
2014-05-27 14:48 ` [Xen-devel] " Konrad Rzeszutek Wilk
2014-05-28 1:10 ` Chen, Tiejun
2014-05-27 18:04 ` Stefano Stabellini
2014-05-28 1:58 ` Chen, Tiejun
2014-05-26 9:43 ` [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping Tiejun Chen
2014-05-27 17:35 ` Stefano Stabellini
2014-05-28 1:31 ` Chen, Tiejun
2014-05-28 12:23 ` Stefano Stabellini
2014-05-29 1:38 ` Chen, Tiejun
2014-05-27 9:06 ` [Qemu-devel] [v3][PATCH 0/5] xen: add Intel IGD passthrough support Chen, Tiejun
-- strict thread matches above, loose matches on Subject: below --
2014-05-26 9:36 [v3][PATCH 0/8] " Tiejun Chen
2014-05-26 9:36 ` [v3][PATCH 2/5] xen, gfx passthrough: create intel isa bridge Tiejun Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).