Skip to contentSkip to conversation

xen: add Intel IGD passthrough support

PATCH Superseded Tiejun Chen 9 participants 30 messages May 30, 2014 – Jun 6, 2014

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v4 cover xen: add Intel IGD passthrough support

v4:

* Fix some typos in the patch head description.
* Improve some comments.
* Given that xen_pt_register_vga_regions()/xen_pt_unregister_vga_regions() are called unconditionally, so we just return 0 there.
* Remove one spurious change.
* Remove some unnecessary "return" in void foo().
* Given that pci_create_pch() is called unconditionally, so we just return 0 even if its failed to check xen_has_gfx_passthru.
* Use (xen_enabled() && xen_has_gfx_passthru) to make sure we only work in this scenario.

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     | 517 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx              |   9 +++
 vl.c                         |  10 +++
 10 files changed, 667 insertions(+), 5 deletions(-)
 create mode 100644 hw/xen/xen_pt_graphics.c

Thanks
Tiejun

#1 lore raw

PATCH v4 1/5 xen, gfx passthrough: basic graphics passthrough support

basic gfx passthrough support:
- add a vga type for gfx passthrough
- retrieve VGA bios from sysfs, then load it to guest at 0xC0000
- register/unregister legacy VGA I/O ports and MMIOs for passthrough GFX

The original patch is from Weidong Han <[email protected]>

---
v4:

* Fix some typos in the patch head description.
* Improve some comments.
* Given that xen_pt_register_vga_regions()/xen_pt_unregister_vga_regions() are called unconditionally, so we just return 0 there.
* Remove one spurious change.

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                         |  10 ++
 8 files changed, 272 insertions(+), 1 deletion(-)
 create mode 100644 hw/xen/xen_pt_graphics.c
hw/xen/Makefile.objs +1 -1
index a0ca0aa..77b7dae 100644
@@ -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
hw/xen/xen-host-pci-device.c +5 -0
index 743b37b..a54b7de 100644
@@ -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;
hw/xen/xen-host-pci-device.h +1 -0
index c2486f0..f1e1c30 100644
@@ -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];
hw/xen/xen_pt.c +10 -0
index be4220b..dac4238 100644
@@ -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 passthrough GFX */
+    if (xen_pt_setup_vga(&s->real_device) < 0) {
+        XEN_PT_ERR(d, "Setup VGA BIOS of passthrough 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");
hw/xen/xen_pt.h +4 -0
index 942dc60..4d3a18d 100644
@@ -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 */
hw/xen/xen_pt_graphics.c new file +232 -0

232 lines not on this page · raw message

qemu-options.hx +9 -0
index c2c0823..9cb324f 100644
@@ -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)
vl.c +10 -0
index 673148e..c615b41 100644
@@ -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;
@@ -3957,6 +3964,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);
             }
Signature · 1 line
1.9.1
Reviewed-by on v4 1/5
#2 lore raw

Re: [Xen-devel] [v4][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support

Quoted 10 lines · Tiejun Chen

basic gfx passthrough support:
- add a vga type for gfx passthrough
- retrieve VGA bios from sysfs, then load it to guest at 0xC0000
- register/unregister legacy VGA I/O ports and MMIOs for passthrough GFX

The original patch is from Weidong Han <[email protected]>

Quoted 412 lines

---
v4:

* Fix some typos in the patch head description.
* Improve some comments.
* Given that xen_pt_register_vga_regions()/xen_pt_unregister_vga_regions() are called unconditionally, so we just return 0 there.
* Remove one spurious change.

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                         |  10 ++
 8 files changed, 272 insertions(+), 1 deletion(-)
 create mode 100644 hw/xen/xen_pt_graphics.c
hw/xen/Makefile.objs +1 -1
index a0ca0aa..77b7dae 100644
@@ -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
hw/xen/xen-host-pci-device.c +5 -0
index 743b37b..a54b7de 100644
@@ -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;
hw/xen/xen-host-pci-device.h +1 -0
index c2486f0..f1e1c30 100644
@@ -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];
hw/xen/xen_pt.c +10 -0
index be4220b..dac4238 100644
@@ -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 passthrough GFX */
+    if (xen_pt_setup_vga(&s->real_device) < 0) {
+        XEN_PT_ERR(d, "Setup VGA BIOS of passthrough 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");
hw/xen/xen_pt.h +4 -0
index 942dc60..4d3a18d 100644
@@ -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 */
hw/xen/xen_pt_graphics.c new file +232 -0
new file mode 100644
index 0000000..461e526
@@ -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 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 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_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;
+}
qemu-options.hx +9 -0
index c2c0823..9cb324f 100644
@@ -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)
vl.c +10 -0
index 673148e..c615b41 100644
@@ -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;
@@ -3957,6 +3964,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);
             }
Signature · 7 lines
1.9.1


_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Reviewed-by on v4 1/5
#3 lore raw

Re: [Qemu-devel] [v4][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support

In reply to Tiejun Chen
Quoted 10 lines · Tiejun Chen

basic gfx passthrough support:
- add a vga type for gfx passthrough
- retrieve VGA bios from sysfs, then load it to guest at 0xC0000
- register/unregister legacy VGA I/O ports and MMIOs for passthrough GFX

The original patch is from Weidong Han <[email protected]>

Quoted 406 lines

v4:

* Fix some typos in the patch head description.
* Improve some comments.
* Given that xen_pt_register_vga_regions()/xen_pt_unregister_vga_regions() are called unconditionally, so we just return 0 there.
* Remove one spurious change.

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                         |  10 ++
 8 files changed, 272 insertions(+), 1 deletion(-)
 create mode 100644 hw/xen/xen_pt_graphics.c
hw/xen/Makefile.objs +1 -1
index a0ca0aa..77b7dae 100644
@@ -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
hw/xen/xen-host-pci-device.c +5 -0
index 743b37b..a54b7de 100644
@@ -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;
hw/xen/xen-host-pci-device.h +1 -0
index c2486f0..f1e1c30 100644
@@ -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];
hw/xen/xen_pt.c +10 -0
index be4220b..dac4238 100644
@@ -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 passthrough GFX */
+    if (xen_pt_setup_vga(&s->real_device) < 0) {
+        XEN_PT_ERR(d, "Setup VGA BIOS of passthrough 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");
hw/xen/xen_pt.h +4 -0
index 942dc60..4d3a18d 100644
@@ -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 */
hw/xen/xen_pt_graphics.c new file +232 -0
new file mode 100644
index 0000000..461e526
@@ -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 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 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_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;
+}
qemu-options.hx +9 -0
index c2c0823..9cb324f 100644
@@ -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)
vl.c +10 -0
index 673148e..c615b41 100644
@@ -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;
@@ -3957,6 +3964,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);
             }
Signature · 1 line
1.9.1
#4 lore raw

PATCH v4 2/5 xen, gfx passthrough: create intel isa bridge

In reply to Tiejun Chen

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 [[email protected]]

---
v4:

* Remove some unnecessary "return" in void foo().

v3:

* Fix some typos.
* Improve some return paths.

v2:

* Nothing is changed.

 hw/xen/xen_pt_graphics.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)
hw/xen/xen_pt_graphics.c +59 -0
index 461e526..236b13a 100644
@@ -230,3 +230,62 @@ 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);
+}
+
+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;
+};
+
+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;
+}
Signature · 1 line
1.9.1
Reviewed-by on v4 2/5
#5 lore raw

Re: [Xen-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

Quoted 9 lines · Tiejun Chen

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 [[email protected]]

Quoted 92 lines

---
v4:

* Remove some unnecessary "return" in void foo().

v3:

* Fix some typos.
* Improve some return paths.

v2:

* Nothing is changed.

 hw/xen/xen_pt_graphics.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)
hw/xen/xen_pt_graphics.c +59 -0
index 461e526..236b13a 100644
@@ -230,3 +230,62 @@ 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);
+}
+
+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;
+};
+
+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;
+}
Signature · 7 lines
1.9.1


_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Reviewed-by on v4 2/5
#6 lore raw

Re: [Qemu-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

In reply to Tiejun Chen
Quoted 9 lines · Tiejun Chen

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 [[email protected]]

Quoted 86 lines

v4:

* Remove some unnecessary "return" in void foo().

v3:

* Fix some typos.
* Improve some return paths.

v2:

* Nothing is changed.

 hw/xen/xen_pt_graphics.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)
hw/xen/xen_pt_graphics.c +59 -0
index 461e526..236b13a 100644
@@ -230,3 +230,62 @@ 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);
+}
+
+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;
+};
+
+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;
+}
Signature · 1 line
1.9.1
#7 lore raw

Re: [Qemu-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

In reply to Tiejun Chen

Il 30/05/2014 10:59, Tiejun Chen ha scritto:

hw/xen/xen_pt_graphics.c out: · line 277

Quoted 7 lines
hw/xen/xen_pt_graphics.c +7 -0
+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");

This is really a huge hack. You're going to have two ISA bridge devices in the machine, with the BIOS imagining that the "good" one is at 1f.0 and the ACPI tables actually describing the other one. But the PCI device at 1f.0 remains there and a driver in the OS could catch it---not just intel_detect_pch---and if you want to add such a hack it should be done in the Xen management layers.

If possible, the host bridge patches are even worse. If you change the vendor and device ID while keeping the registers of the i440FX you're going to get conflicts or break firmware badly; TianoCore and SeaBIOS both expect the normal i440FX vendor and device IDs, for example.

The hardcoded list of offsets is also not acceptable. It is also not clear who is accessing the registers, whether the BIOS or the driver. For Linux, a cursory look at the driver shows that it only accesses 0x50/0x52 of the listed offsets, but also 0x44/0x48 ("MCH BAR"), what happens if that code path is encountered?

The main problem with IGD passthrough is the incestuous (and that's a euphemism) relationship between the MCH, PCH and graphics driver. It may make sense at the hardware level, but for virtualization it doesn't.
A virt-specific driver for GPU command passthrough (with aid from the
kernel driver, but abstracting all the MCH/PCH-dependent details) would make much more sense.

It's really not your fault, there's not much you can do given the hardware architecture. But I don't think this code can be accepted upstream, sorry.

Paolo

#8 lore raw

Re: [Qemu-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

Quoted 34 lines · Paolo Bonzini

Il 30/05/2014 10:59, Tiejun Chen ha scritto:

hw/xen/xen_pt_graphics.c out: · line 277

Quoted 7 lines
hw/xen/xen_pt_graphics.c +7 -0
+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");

This is really a huge hack. You're going to have two ISA bridge devices in the machine, with the BIOS imagining that the "good" one is at 1f.0 and the ACPI tables actually describing the other one. But the PCI device at 1f.0 remains there and a driver in the OS could catch it---not just intel_detect_pch---and if you want to add such a hack it should be done in the Xen management layers.

If possible, the host bridge patches are even worse. If you change the vendor and device ID while keeping the registers of the i440FX you're going to get conflicts or break firmware badly; TianoCore and SeaBIOS both expect the normal i440FX vendor and device IDs, for example.

The hardcoded list of offsets is also not acceptable. It is also not clear who is accessing the registers, whether the BIOS or the driver. For Linux, a cursory look at the driver shows that it only accesses 0x50/0x52 of the listed offsets, but also 0x44/0x48 ("MCH BAR"), what happens if that code path is encountered?

The main problem with IGD passthrough is the incestuous (and that's a euphemism) relationship between the MCH, PCH and graphics driver. It may make sense at the hardware level, but for virtualization it doesn't. A virt-specific driver for GPU command passthrough (with aid from the kernel driver, but abstracting all the MCH/PCH-dependent details) would make much more sense.

Quoted 2 lines

It's really not your fault, there's not much you can do given the hardware architecture. But I don't think this code can be accepted upstream, sorry.

Yeah, the code is not nice and it is not Tiejun's fault.

Is there any way at all he could change the patch series to make it more appealing to you? Or maybe we could having more clearly separated from the rest of the codebase?

Otherwise I hate to have to diverge again from upstream QEMU but given that we were already carrying these changes in the old qemu-xen-traditional tree without issues, I feel that it would be unfair for me not to merge them in the upstream based qemu-xen tree. Unfortunately I imagine that the lack of this feature could be considered a regression for us.

Do the other Xen maintainters have any opinions on this? I would appreciate your opinions.

#9 lore raw

Re: [Qemu-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

Il 03/06/2014 13:29, Stefano Stabellini ha scritto:

Quoted 8 lines
Quoted 2 lines

It's really not your fault, there's not much you can do given the hardware architecture. But I don't think this code can be accepted upstream, sorry.

Yeah, the code is not nice and it is not Tiejun's fault.

Is there any way at all he could change the patch series to make it more appealing to you? Or maybe we could having more clearly separated from the rest of the codebase?

I don't know, perhaps a new "-M pc-xen-igd-passthrough" machine type based on what you're using now (pc-1.6?) but with all the required hacks?

Paolo

#10 lore raw

Re: [Qemu-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

Quoted 5 lines · Paolo Bonzini

Il 03/06/2014 13:29, Stefano Stabellini ha scritto:

Quoted 4 lines
Quoted 3 lines

It's really not your fault, there's not much you can do given the hardware architecture. But I don't think this code can be accepted upstream, sorry.

Quoted 8 lines
Quoted 5 lines

Yeah, the code is not nice and it is not Tiejun's fault.

Is there any way at all he could change the patch series to make it more appealing to you? Or maybe we could having more clearly separated from the rest of the codebase?

I don't know, perhaps a new "-M pc-xen-igd-passthrough" machine type based on what you're using now (pc-1.6?) but with all the required hacks?

That might be a good compromise as it would make it easier to identify regressions caused by these changes. I would be happy with that.

#11 lore raw

Re: [Qemu-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

In reply to Paolo Bonzini
Quoted 7 lines

From: Paolo Bonzini
Sent: Tuesday, June 03, 2014 4:40 AM

Il 03/06/2014 13:29, Stefano Stabellini ha scritto:

Quoted 3 lines
Quoted 2 lines

It's really not your fault, there's not much you can do given the hardware architecture. But I don't think this code can be accepted upstream, sorry.

Quoted 9 lines
Quoted 5 lines

Yeah, the code is not nice and it is not Tiejun's fault.

Is there any way at all he could change the patch series to make it more appealing to you? Or maybe we could having more clearly separated from the rest of the codebase?

I don't know, perhaps a new "-M pc-xen-igd-passthrough" machine type based on what you're using now (pc-1.6?) but with all the required hacks?

Note the tricks here are not Xen specific, because it's a driver assumption. If KVM wants to support igd passthrough, same tricks are required too.

Thanks
Kevin

#12 lore raw

Re: [Qemu-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

In reply to Stefano Stabellini
Quoted 50 lines · Stefano Stabellini
Quoted 33 lines · Paolo Bonzini

Il 30/05/2014 10:59, Tiejun Chen ha scritto:

hw/xen/xen_pt_graphics.c out: · line 277

Quoted 7 lines
hw/xen/xen_pt_graphics.c +7 -0
+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");

This is really a huge hack. You're going to have two ISA bridge devices in the machine, with the BIOS imagining that the "good" one is at 1f.0 and the ACPI tables actually describing the other one. But the PCI device at 1f.0 remains there and a driver in the OS could catch it---not just intel_detect_pch---and if you want to add such a hack it should be done in the Xen management layers.

If possible, the host bridge patches are even worse. If you change the vendor and device ID while keeping the registers of the i440FX you're going to get conflicts or break firmware badly; TianoCore and SeaBIOS both expect the normal i440FX vendor and device IDs, for example.

The hardcoded list of offsets is also not acceptable. It is also not clear who is accessing the registers, whether the BIOS or the driver. For Linux, a cursory look at the driver shows that it only accesses 0x50/0x52 of the listed offsets, but also 0x44/0x48 ("MCH BAR"), what happens if that code path is encountered?

The main problem with IGD passthrough is the incestuous (and that's a euphemism) relationship between the MCH, PCH and graphics driver. It may make sense at the hardware level, but for virtualization it doesn't. A virt-specific driver for GPU command passthrough (with aid from the kernel driver, but abstracting all the MCH/PCH-dependent details) would make much more sense.

Quoted 2 lines

It's really not your fault, there's not much you can do given the hardware architecture. But I don't think this code can be accepted upstream, sorry.

Yeah, the code is not nice and it is not Tiejun's fault.

Is there any way at all he could change the patch series to make it more appealing to you? Or maybe we could having more clearly separated from the rest of the codebase?

Otherwise I hate to have to diverge again from upstream QEMU but given that we were already carrying these changes in the old qemu-xen-traditional tree without issues, I feel that it would be unfair for me not to merge them in the upstream based qemu-xen tree. Unfortunately I imagine that the lack of this feature could be considered a regression for us.

Quoted 2 lines

Do the other Xen maintainters have any opinions on this? I would appreciate your opinions.

Well my very initial take is to say that it was a mistake to accept the IGD stuff into qemu-xen-traditional before making sure that it would be suitable for qemu-upstream. Avoiding having a fork again (or maintaining a set of out-of-tree patches) is more important than this one feature, IMHO.

When Intel submitted this for qemu-xen-traditional, we should have recommended to them at that time to start with qemu-upstream; or, we should have made it clear that if they chose to submit it to qemu-xen-traditional first, they would be taking the risk of having it only be there. If we didn't warn them of that, that was a mistake on our part; but I don't think we can do anything other than apologize. (And of course see if there *is* a way to actually get it upstream.)

-George

#13 lore raw

Re: [Qemu-devel] [Xen-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

Quoted 68 lines · you
Quoted 50 lines · Stefano Stabellini
Quoted 33 lines · Paolo Bonzini

Il 30/05/2014 10:59, Tiejun Chen ha scritto:

hw/xen/xen_pt_graphics.c out: · line 277

Quoted 7 lines
hw/xen/xen_pt_graphics.c +7 -0
+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");

This is really a huge hack. You're going to have two ISA bridge devices in the machine, with the BIOS imagining that the "good" one is at 1f.0 and the ACPI tables actually describing the other one. But the PCI device at 1f.0 remains there and a driver in the OS could catch it---not just intel_detect_pch---and if you want to add such a hack it should be done in the Xen management layers.

If possible, the host bridge patches are even worse. If you change the vendor and device ID while keeping the registers of the i440FX you're going to get conflicts or break firmware badly; TianoCore and SeaBIOS both expect the normal i440FX vendor and device IDs, for example.

The hardcoded list of offsets is also not acceptable. It is also not clear who is accessing the registers, whether the BIOS or the driver. For Linux, a cursory look at the driver shows that it only accesses 0x50/0x52 of the listed offsets, but also 0x44/0x48 ("MCH BAR"), what happens if that code path is encountered?

The main problem with IGD passthrough is the incestuous (and that's a euphemism) relationship between the MCH, PCH and graphics driver. It may make sense at the hardware level, but for virtualization it doesn't. A virt-specific driver for GPU command passthrough (with aid from the kernel driver, but abstracting all the MCH/PCH-dependent details) would make much more sense.

Quoted 2 lines

It's really not your fault, there's not much you can do given the hardware architecture. But I don't think this code can be accepted upstream, sorry.

Yeah, the code is not nice and it is not Tiejun's fault.

Is there any way at all he could change the patch series to make it more appealing to you? Or maybe we could having more clearly separated from the rest of the codebase?

Otherwise I hate to have to diverge again from upstream QEMU but given that we were already carrying these changes in the old qemu-xen-traditional tree without issues, I feel that it would be unfair for me not to merge them in the upstream based qemu-xen tree. Unfortunately I imagine that the lack of this feature could be considered a regression for us.

Quoted 2 lines

Do the other Xen maintainters have any opinions on this? I would appreciate your opinions.

Well my very initial take is to say that it was a mistake to accept the IGD stuff into qemu-xen-traditional before making sure that it would be suitable for qemu-upstream. Avoiding having a fork again (or maintaining a set of out-of-tree patches) is more important than this one feature, IMHO.

When Intel submitted this for qemu-xen-traditional, we should have recommended to them at that time to start with qemu-upstream; or, we should have made it clear that if they chose to submit it to qemu-xen-traditional first, they would be taking the risk of having it only be there. If we didn't warn them of that, that was a mistake on our part; but I don't think we can do anything other than apologize. (And of course see if there *is* a way to actually get it upstream.)

Quoted 1 line

-George

Which make me wonder how much of the hackery is due to fundamental passthrough issues of vga devices and how much is due to driver/firmware assumptions made because the driver/firmware was made with an integrated device in mind ?

The first part (probably legacy vga stuff) would probably be acceptable since it's probably shared for all vga devices (vendor independent) and would also be required for KVM.

The last part should perhaps be fixed in the driver/firmware (for instance the assumption the device is always device 00:02.0, that's valid for real intel hardware, but not necessary for passthrough or emulation). A requirement for using the latest firmware/drivers for passthrough to work could be acceptable i guess ?

--
Sander

#14 lore raw

Re: [Qemu-devel] [Xen-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

Il 03/06/2014 14:21, Sander Eikelenboom ha scritto:

Quoted 6 lines

The last part should perhaps be fixed in the driver/firmware (for instance the assumption the device is always device 00:02.0, that's valid for real intel hardware, but not necessary for passthrough or emulation). A requirement for using the latest firmware/drivers for passthrough to work could be acceptable i guess ?

The problem is worse than that. The assumptions are that the driver can make choices based on the device/vendor IDs of 00:1f.0, and that the driver can read/write to the config space of 00:00.0.

Paolo

#15 lore raw

Re: [Qemu-devel] [Xen-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

Quoted 8 lines · you

Il 03/06/2014 14:21, Sander Eikelenboom ha scritto:

Quoted 6 lines

The last part should perhaps be fixed in the driver/firmware (for instance the assumption the device is always device 00:02.0, that's valid for real intel hardware, but not necessary for passthrough or emulation). A requirement for using the latest firmware/drivers for passthrough to work could be acceptable i guess ?

Quoted 5 lines

The problem is worse than that. The assumptions are that the driver can make choices based on the device/vendor IDs of 00:1f.0, and that the driver can read/write to the config space of 00:00.0.

Paolo

It's quite tempting to start making assumptions and shortcuts when writing drivers for hardware fixed in a platfrom (IGD's), they probably wouldn't have done that when they were also making seperate PCI(-e) graphic cards with these chips because it would blow up on all sorts of machines (which is what happens now when trying to pass it through (or emulate it) on another platform).

--
Sander

#16 lore raw

Re: [Qemu-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

In reply to Paolo Bonzini

Paolo Bonzini wrote on 2014-06-03:

Quoted 11 lines

Il 30/05/2014 10:59, Tiejun Chen ha scritto:

hw/xen/xen_pt_graphics.c out: · line 277

Quoted 7 lines
hw/xen/xen_pt_graphics.c +7 -0
+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");

This is really a huge hack. You're going to have two ISA bridge devices in the machine, with the BIOS imagining that the "good" one is at 1f.0

Definitely. So how about expose a fake device at 00:1f.0 but not Isa Bridge? I have discussion with gfx driver developer, it is ok for them to check the device on 00:1f.0 no matter what device it is.

Quoted 9 lines

and the ACPI tables actually describing the other one. But the PCI device at 1f.0 remains there and a driver in the OS could catch it---not just intel_detect_pch---and if you want to add such a hack it should be done in the Xen management layers.

If possible, the host bridge patches are even worse. If you change the vendor and device ID while keeping the registers of the i440FX you're going to get conflicts or break firmware badly; TianoCore and SeaBIOS both expect the normal i440FX vendor and device IDs, for example.

I only see the class id is changed but not vendor and device id.

Quoted 6 lines

The hardcoded list of offsets is also not acceptable. It is also not clear who is accessing the registers, whether the BIOS or the driver. For Linux, a cursory look at the driver shows that it only accesses 0x50/0x52 of the listed offsets, but also 0x44/0x48 ("MCH BAR"), what happens if that code path is encountered?

Will have a double check.

Quoted 7 lines

The main problem with IGD passthrough is the incestuous (and that's a euphemism) relationship between the MCH, PCH and graphics driver. It may make sense at the hardware level, but for virtualization it doesn't.
A virt-specific driver for GPU command passthrough (with aid from the
kernel driver, but abstracting all the MCH/PCH-dependent details) would make much more sense.

Agree. But it is too hard.

Quoted 6 lines

It's really not your fault, there's not much you can do given the hardware architecture. But I don't think this code can be accepted upstream, sorry.

Paolo

Best regards,
Yang

#17 lore raw

Re: [Qemu-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

Il 06/06/2014 05:06, Zhang, Yang Z ha scritto:

Quoted 13 lines

Paolo Bonzini wrote on 2014-06-03:

Quoted 11 lines

Il 30/05/2014 10:59, Tiejun Chen ha scritto:

hw/xen/xen_pt_graphics.c out: · line 277

Quoted 7 lines
hw/xen/xen_pt_graphics.c +7 -0
+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");

This is really a huge hack. You're going to have two ISA bridge devices in the machine, with the BIOS imagining that the "good" one is at 1f.0

Quoted 1 line

Definitely. So how about expose a fake device at 00:1f.0 but not Isa Bridge? I have discussion with gfx driver developer, it is ok for them to check the device on 00:1f.0 no matter what device it is.

That would be slightly better.

Quoted 11 lines
Quoted 9 lines

and the ACPI tables actually describing the other one. But the PCI device at 1f.0 remains there and a driver in the OS could catch it---not just intel_detect_pch---and if you want to add such a hack it should be done in the Xen management layers.

If possible, the host bridge patches are even worse. If you change the vendor and device ID while keeping the registers of the i440FX you're going to get conflicts or break firmware badly; TianoCore and SeaBIOS both expect the normal i440FX vendor and device IDs, for example.

I only see the class id is changed but not vendor and device id.

Yes, and the class ID is a typo probably.

But when the guest reads the vendor and device ID, igd_pci_read passes it through. So effectively it changes, if I read the code correctly.

Paolo

Quoted 33 lines
Quoted 6 lines

The hardcoded list of offsets is also not acceptable. It is also not clear who is accessing the registers, whether the BIOS or the driver. For Linux, a cursory look at the driver shows that it only accesses 0x50/0x52 of the listed offsets, but also 0x44/0x48 ("MCH BAR"), what happens if that code path is encountered?

Will have a double check.

Quoted 7 lines

The main problem with IGD passthrough is the incestuous (and that's a euphemism) relationship between the MCH, PCH and graphics driver. It may make sense at the hardware level, but for virtualization it doesn't.
A virt-specific driver for GPU command passthrough (with aid from the
kernel driver, but abstracting all the MCH/PCH-dependent details) would make much more sense.

Agree. But it is too hard.

Quoted 6 lines

It's really not your fault, there's not much you can do given the hardware architecture. But I don't think this code can be accepted upstream, sorry.

Paolo

Best regards,
Yang

#18 lore raw

PATCH v4 3/5 xen, gfx passthrough: support Intel IGD passthrough with VT-D

In reply to Tiejun Chen

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 >

---
v4:

* Given that pci_create_pch() is called unconditionally, so we just return 0 even if its failed to check xen_has_gfx_passthru.
* Remove one spurious change.

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 | 162 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 166 insertions(+)
hw/xen/xen_pt.h +4 -0
index 4d3a18d..507165c 100644
@@ -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 */
hw/xen/xen_pt_graphics.c +162 -0

171 lines not on this page · raw message

Signature · 1 line
1.9.1
Reviewed-by on v4 3/5
#19 lore raw

Re: [v4][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D

Quoted 9 lines · Tiejun Chen

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 >

Quoted 219 lines

---
v4:

* Given that pci_create_pch() is called unconditionally, so we just return 0 even if its failed to check xen_has_gfx_passthru.
* Remove one spurious change.

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 | 162 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 166 insertions(+)
hw/xen/xen_pt.h +4 -0
index 4d3a18d..507165c 100644
@@ -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 */
hw/xen/xen_pt_graphics.c +162 -0
index 236b13a..c1f314a 100644
@@ -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)
 {
@@ -289,3 +290,164 @@ 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 r;
+    }
+
+    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.
+ *
+ * Here pci_dev is just that host bridge, so we have to get that real
+ * passthrough device by that given devfn to further confirm.
+ */
+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;
+}
Signature · 7 lines
1.9.1


_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Reviewed-by on v4 3/5
#20 lore raw

Re: [Qemu-devel] [v4][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D

In reply to Tiejun Chen
Quoted 9 lines · Tiejun Chen

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 >

Quoted 213 lines

v4:

* Given that pci_create_pch() is called unconditionally, so we just return 0 even if its failed to check xen_has_gfx_passthru.
* Remove one spurious change.

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 | 162 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 166 insertions(+)
hw/xen/xen_pt.h +4 -0
index 4d3a18d..507165c 100644
@@ -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 */
hw/xen/xen_pt_graphics.c +162 -0
index 236b13a..c1f314a 100644
@@ -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)
 {
@@ -289,3 +290,164 @@ 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 r;
+    }
+
+    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.
+ *
+ * Here pci_dev is just that host bridge, so we have to get that real
+ * passthrough device by that given devfn to further confirm.
+ */
+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;
+}
Signature · 1 line
1.9.1
#21 lore raw

PATCH v4 4/5 xen, gfx passthrough: create host bridge to passthrough

In reply to Tiejun Chen

Implement that pci host bridge to specific to passthrough. Actually this just inherit the standard one.

---
v4:

* Fix one typo in the patch head description.
* Use (xen_enabled() && xen_has_gfx_passthru) to make sure we only work in this scenario.

v3:

* Just fix this patch head description typo.

v2:

* New patch.

 hw/pci-host/piix.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 54 insertions(+), 2 deletions(-)
hw/pci-host/piix.c +54 -2
index ffdc853..52382f4 100644
@@ -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() && 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);
Signature · 1 line
1.9.1
Reviewed-by on v4 4/5
#22 lore raw

Re: [v4][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough

Quoted 4 lines · Tiejun Chen

Implement that pci host bridge to specific to passthrough. Actually this just inherit the standard one.

Quoted 128 lines

---
v4:

* Fix one typo in the patch head description.
* Use (xen_enabled() && xen_has_gfx_passthru) to make sure we only work in this scenario.

v3:

* Just fix this patch head description typo.

v2:

* New patch.

 hw/pci-host/piix.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 54 insertions(+), 2 deletions(-)
hw/pci-host/piix.c +54 -2
index ffdc853..52382f4 100644
@@ -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() && 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);
Signature · 7 lines
1.9.1


_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Reviewed-by on v4 4/5
#23 lore raw

Re: [Qemu-devel] [v4][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough

In reply to Tiejun Chen
Quoted 4 lines · Tiejun Chen

Implement that pci host bridge to specific to passthrough. Actually this just inherit the standard one.

Quoted 122 lines

v4:

* Fix one typo in the patch head description.
* Use (xen_enabled() && xen_has_gfx_passthru) to make sure we only work in this scenario.

v3:

* Just fix this patch head description typo.

v2:

* New patch.

 hw/pci-host/piix.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 54 insertions(+), 2 deletions(-)
hw/pci-host/piix.c +54 -2
index ffdc853..52382f4 100644
@@ -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() && 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);
Signature · 1 line
1.9.1
#24 lore raw

Re: [Qemu-devel] [v4][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough

In reply to Tiejun Chen
Quoted 4 lines · Tiejun Chen

Implement that pci host bridge to specific to passthrough. Actually this just inherit the standard one.

Can this actually inherit TYPE_I440FX_PCI_DEVICE?
The parent is TYPE_PCI_DEVICE, but if it was TYPE_I440FX_PCI_DEVICE then you could re-use i440fx_initfn.
And you could do *pi440fx_state = I440FX_PCI_DEVICE(d) unconditionally.

Quoted 122 lines

---
v4:

* Fix one typo in the patch head description.
* Use (xen_enabled() && xen_has_gfx_passthru) to make sure we only work in this scenario.

v3:

* Just fix this patch head description typo.

v2:

* New patch.

 hw/pci-host/piix.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 54 insertions(+), 2 deletions(-)
hw/pci-host/piix.c +54 -2
index ffdc853..52382f4 100644
@@ -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() && 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);
Signature · 1 line
1.9.1
#25 lore raw

Re: [Qemu-devel] [v4][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough

Quoted 11 lines

-----Original Message-----
From: Michael S. Tsirkin [mailto:[email protected]]
Sent: Tuesday, June 03, 2014 4:37 AM
To: Chen, Tiejun
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Kay, Allen M; Zhang, Yang Z
Subject: Re: [v4][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough

Quoted 7 lines
Quoted 4 lines · Tiejun Chen

Implement that pci host bridge to specific to passthrough. Actually this just inherit the standard one.

Can this actually inherit TYPE_I440FX_PCI_DEVICE?

Yes, but

Quoted 4 lines

The parent is TYPE_PCI_DEVICE, but if it was TYPE_I440FX_PCI_DEVICE then you could re-use i440fx_initfn.
And you could do *pi440fx_state = I440FX_PCI_DEVICE(d) unconditionally.

one of reasons to implement our own pci host bridge which just inherit that standard one, please refer to some comments from v1:

http://patchwork.ozlabs.org/patch/322443/

And I also think this separation may be flexible to extend more in the future.

Thanks
Tiejun

#26 lore raw

PATCH v4 5/5 xen, gfx passthrough: add opregion mapping

In reply to Tiejun Chen

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 <[email protected]>

---
v4:

* Nothing is changed.

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(-)
hw/xen/xen_pt.h +3 -1
index 507165c..25147cf 100644
@@ -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 */
hw/xen/xen_pt_config_init.c +49 -1

80 lines not on this page · raw message

hw/xen/xen_pt_graphics.c +64 -0

85 lines not on this page · raw message

Signature · 1 line
1.9.1
Reviewed-by on v4 5/5
#27 lore raw

Re: [v4][PATCH 5/5] xen, gfx passthrough: add opregion mapping

Quoted 11 lines · Tiejun Chen

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 <[email protected]>

Quoted 236 lines

---
v4:

* Nothing is changed.

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(-)
hw/xen/xen_pt.h +3 -1
index 507165c..25147cf 100644
@@ -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 */
hw/xen/xen_pt_config_init.c +49 -1
index de9a20f..6eaaa9a 100644
@@ -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(&reg_grp_entry->reg_tbl_list);
         QLIST_INSERT_HEAD(&s->reg_grps, reg_grp_entry, entries);
hw/xen/xen_pt_graphics.c +64 -0
index c1f314a..6185adb 100644
@@ -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 0;
@@ -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;
 }
 
@@ -451,3 +466,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));
+}
Signature · 7 lines
1.9.1


_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Reviewed-by on v4 5/5
#28 lore raw

Re: [Qemu-devel] [v4][PATCH 5/5] xen, gfx passthrough: add opregion mapping

In reply to Tiejun Chen
Quoted 11 lines · Tiejun Chen

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 <[email protected]>

Quoted 230 lines

v4:

* Nothing is changed.

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(-)
hw/xen/xen_pt.h +3 -1
index 507165c..25147cf 100644
@@ -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 */
hw/xen/xen_pt_config_init.c +49 -1
index de9a20f..6eaaa9a 100644
@@ -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(&reg_grp_entry->reg_tbl_list);
         QLIST_INSERT_HEAD(&s->reg_grps, reg_grp_entry, entries);
hw/xen/xen_pt_graphics.c +64 -0
index c1f314a..6185adb 100644
@@ -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 0;
@@ -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;
 }
 
@@ -451,3 +466,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));
+}
Signature · 1 line
1.9.1
#29 lore raw
In reply to Tiejun Chen

The patch series is OK from my point of view.

I would appreciate if Paolo or Peter could give their feedback on patch #1 and patch #4 as they modify non-Xen specific files.

If you are OK with the patches, I'll send a pull request.

Quoted 80 lines · Tiejun Chen

v4:

* Fix some typos in the patch head description.
* Improve some comments.
* Given that xen_pt_register_vga_regions()/xen_pt_unregister_vga_regions() are called unconditionally, so we just return 0 there.
* Remove one spurious change.
* Remove some unnecessary "return" in void foo().
* Given that pci_create_pch() is called unconditionally, so we just return 0 even if its failed to check xen_has_gfx_passthru.
* Use (xen_enabled() && xen_has_gfx_passthru) to make sure we only work in this scenario.

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     | 517 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx              |   9 +++
 vl.c                         |  10 +++
 10 files changed, 667 insertions(+), 5 deletions(-)
 create mode 100644 hw/xen/xen_pt_graphics.c

Thanks
Tiejun