All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [v7][PATCH 00/10] xen: add Intel IGD passthrough support
@ 2015-03-18  9:06 Tiejun Chen
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 01/10] i440fx: make types configurable at run-time Tiejun Chen
                   ` (9 more replies)
  0 siblings, 10 replies; 22+ messages in thread
From: Tiejun Chen @ 2015-03-18  9:06 UTC (permalink / raw)
  To: mst, kraxel, pbonzini, rth; +Cc: allen.m.kay, qemu-devel

v7:

* Instead of "-gfx_passthru" we'd like to make that a machine
  option, "-machine xxx,igd-passthru=on"" 
* try to make something as common shared by others like KvmGT in
  the future
* Just read those real value from host bridge pci
  configuration space when create host bridge then put in dev->config.

v6:

* Drop introducing a new machine specific to IGD passthrough
* Try to share some codes from KVM stuff in qemu to retrive VGA BIOS
* Currently IGD drivers always need to access PCH by 1f.0. But we
  don't want to poke that directly to get ID, and although in real
  world different GPU should have different PCH. But actually the
  different PCH DIDs likely map to different PCH SKUs. We do the
  same thing for the GPU. For PCH, the different SKUs are going to
  be all the same silicon design and implementation, just different
  features turn on and off with fuses. The SW interfaces should be
  consistent across all SKUs in a given family (eg LPT). But just
  same features may not be supported.
 
  Most of these different PCH features probably don't matter to the
  Gfx driver, but obviously any difference in display port connections
  will so it should be fine with any PCH in case of passthrough.
 
  So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
  scenarios, 0x9cc3 for BDW(Broadwell).
* Drop igd write ops since its fine to emulate that, and we also shrink
  those igd read ops as necessary.
* Rebase and cleanup all patches.

v5:

* Simplify to make sure its really inherited from the standard one in patch #3
* Then drop the original patch #3

v4:

* Rebase on latest tree
* Drop patch #2
* Regenerate patches after Michael introduce patch #1
* We need to use this pci_type as a index to reuse I440FX_PCI_DEVICE()
* Test: boot with a preinstalled winxp
  ./i386-softmmu/qemu-system-i386 -hda winxp-32.img -m 2560 -boot c -machine pc

v3:

* Drop patch #4
* Add one patch #1 from Michael
* Rebase
* In./i386-softmmu/qemu-system-i386 -hda test.img -m 2560 -boot c -machine pc

v2:

* Fix some coding style
* New patch to separate i440fx_init
* Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough
* Based on patch #2 to regenerate
* Unify prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough like patch #3
* Test: boot with a preinstalled ubuntu 14.04
  ./i386-softmmu/qemu-system-i386 -hda test.img -m 2560 -boot c -machine pc

As we discussed we need to create a separate machine to support current
IGD passthrough.

----------------------------------------------------------------
Michael S. Tsirkin (1):
      i440fx: make types configurable at run-time
 
Tiejun Chen (9):
      pc_init1: pass parameters just with types
      piix: create host bridge to passthrough
      hw/pci-assign: split pci-assign.c
      xen, gfx passthrough: basic graphics passthrough support
      xen, gfx passthrough: retrieve VGA BIOS to work
      igd gfx passthrough: create a isa bridge
      xen, gfx passthrough: register a isa bridge
      xen, gfx passthrough: register host bridge specific to passthrough
      xen, gfx passthrough: add opregion mapping
 
 hw/core/machine.c             |  20 +++
 hw/i386/Makefile.objs         |   1 +
 hw/i386/kvm/pci-assign.c      |  82 +---------
 hw/i386/pc_piix.c             | 149 ++++++++++++++++++-
 hw/i386/pci-assign-load-rom.c |  93 ++++++++++++
 hw/pci-host/piix.c            |  91 +++++++++++-
 hw/xen/Makefile.objs          |   1 +
 hw/xen/xen-host-pci-device.c  |   5 +
 hw/xen/xen-host-pci-device.h  |   1 +
 hw/xen/xen_pt.c               |  32 ++++
 hw/xen/xen_pt.h               |  21 ++-
 hw/xen/xen_pt_config_init.c   |  52 ++++++-
 hw/xen/xen_pt_graphics.c      | 272 ++++++++++++++++++++++++++++++++++
 include/hw/boards.h           |   1 +
 include/hw/i386/pc.h          |   8 +-
 include/hw/pci/pci-assign.h   |  27 ++++
 include/hw/xen/xen.h          |   1 +
 qemu-options.hx               |   3 +
 vl.c                          |  10 ++
 19 files changed, 778 insertions(+), 92 deletions(-)
 create mode 100644 hw/i386/pci-assign-load-rom.c
 create mode 100644 hw/xen/xen_pt_graphics.c
 create mode 100644 include/hw/pci/pci-assign.h

Thanks
Tiejun

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Qemu-devel] [v7][PATCH 01/10] i440fx: make types configurable at run-time
  2015-03-18  9:06 [Qemu-devel] [v7][PATCH 00/10] xen: add Intel IGD passthrough support Tiejun Chen
@ 2015-03-18  9:06 ` Tiejun Chen
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 02/10] pc_init1: pass parameters just with types Tiejun Chen
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Tiejun Chen @ 2015-03-18  9:06 UTC (permalink / raw)
  To: mst, kraxel, pbonzini, rth; +Cc: allen.m.kay, qemu-devel

From: "Michael S. Tsirkin" <mst@redhat.com>

IGD passthrough wants to supply a different pci and
host devices, inheriting i440fx devices. Make types
configurable.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/pc_piix.c    | 4 +++-
 hw/pci-host/piix.c   | 9 ++++-----
 include/hw/i386/pc.h | 6 +++++-
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 36c69d7..07faec9 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -202,7 +202,9 @@ static void pc_init1(MachineState *machine,
     }
 
     if (pci_enabled) {
-        pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
+        pci_bus = i440fx_init(TYPE_I440FX_PCI_HOST_BRIDGE,
+                              TYPE_I440FX_PCI_DEVICE,
+                              &i440fx_state, &piix3_devfn, &isa_bus, gsi,
                               system_memory, system_io, machine->ram_size,
                               below_4g_mem_size,
                               above_4g_mem_size,
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 723836f..c812eaa 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -40,7 +40,6 @@
  * http://download.intel.com/design/chipsets/datashts/29054901.pdf
  */
 
-#define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
 #define I440FX_PCI_HOST_BRIDGE(obj) \
     OBJECT_CHECK(I440FXState, (obj), TYPE_I440FX_PCI_HOST_BRIDGE)
 
@@ -91,7 +90,6 @@ typedef struct PIIX3State {
     MemoryRegion rcr_mem;
 } PIIX3State;
 
-#define TYPE_I440FX_PCI_DEVICE "i440FX"
 #define I440FX_PCI_DEVICE(obj) \
     OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
 
@@ -304,7 +302,8 @@ static void i440fx_realize(PCIDevice *dev, Error **errp)
     cpu_smm_register(&i440fx_set_smm, d);
 }
 
-PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
+PCIBus *i440fx_init(const char *host_type, const char *pci_type,
+                    PCII440FXState **pi440fx_state,
                     int *piix3_devfn,
                     ISABus **isa_bus, qemu_irq *pic,
                     MemoryRegion *address_space_mem,
@@ -324,7 +323,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
     unsigned i;
     I440FXState *i440fx;
 
-    dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
+    dev = qdev_create(NULL, host_type);
     s = PCI_HOST_BRIDGE(dev);
     b = pci_bus_new(dev, NULL, pci_address_space,
                     address_space_io, 0, TYPE_PCI_BUS);
@@ -332,7 +331,7 @@ 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);
+    d = pci_create_simple(b, 0, pci_type);
     *pi440fx_state = I440FX_PCI_DEVICE(d);
     f = *pi440fx_state;
     f->system_memory = address_space_mem;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 1b35168..99dc26b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -230,7 +230,11 @@ extern int no_hpet;
 struct PCII440FXState;
 typedef struct PCII440FXState PCII440FXState;
 
-PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
+#define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
+#define TYPE_I440FX_PCI_DEVICE "i440FX"
+
+PCIBus *i440fx_init(const char *host_type, const char *pci_type,
+                    PCII440FXState **pi440fx_state, int *piix_devfn,
                     ISABus **isa_bus, qemu_irq *pic,
                     MemoryRegion *address_space_mem,
                     MemoryRegion *address_space_io,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [v7][PATCH 02/10] pc_init1: pass parameters just with types
  2015-03-18  9:06 [Qemu-devel] [v7][PATCH 00/10] xen: add Intel IGD passthrough support Tiejun Chen
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 01/10] i440fx: make types configurable at run-time Tiejun Chen
@ 2015-03-18  9:06 ` Tiejun Chen
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough Tiejun Chen
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Tiejun Chen @ 2015-03-18  9:06 UTC (permalink / raw)
  To: mst, kraxel, pbonzini, rth; +Cc: allen.m.kay, qemu-devel

Pass types to configure pc_init1().

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/pc_piix.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 07faec9..cea3a5c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -75,7 +75,9 @@ static bool has_reserved_memory = true;
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine,
                      int pci_enabled,
-                     int kvmclock_enabled)
+                     int kvmclock_enabled,
+                     const char *host_type,
+                     const char *pci_type)
 {
     PCMachineState *pc_machine = PC_MACHINE(machine);
     MemoryRegion *system_memory = get_system_memory();
@@ -202,8 +204,8 @@ static void pc_init1(MachineState *machine,
     }
 
     if (pci_enabled) {
-        pci_bus = i440fx_init(TYPE_I440FX_PCI_HOST_BRIDGE,
-                              TYPE_I440FX_PCI_DEVICE,
+        pci_bus = i440fx_init(host_type,
+                              pci_type,
                               &i440fx_state, &piix3_devfn, &isa_bus, gsi,
                               system_memory, system_io, machine->ram_size,
                               below_4g_mem_size,
@@ -309,7 +311,8 @@ static void pc_init1(MachineState *machine,
 
 static void pc_init_pci(MachineState *machine)
 {
-    pc_init1(machine, 1, 1);
+    pc_init1(machine, 1, 1, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_I440FX_PCI_DEVICE);
 }
 
 static void pc_compat_2_2(MachineState *machine)
@@ -478,7 +481,8 @@ static void pc_init_pci_1_2(MachineState *machine)
 static void pc_init_pci_no_kvmclock(MachineState *machine)
 {
     pc_compat_1_2(machine);
-    pc_init1(machine, 1, 0);
+    pc_init1(machine, 1, 0, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_I440FX_PCI_DEVICE);
 }
 
 static void pc_init_isa(MachineState *machine)
@@ -495,7 +499,8 @@ static void pc_init_isa(MachineState *machine)
     }
     x86_cpu_compat_kvm_no_autoenable(FEAT_KVM, 1 << KVM_FEATURE_PV_EOI);
     enable_compat_apic_id_mode();
-    pc_init1(machine, 0, 1);
+    pc_init1(machine, 0, 1, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_I440FX_PCI_DEVICE);
 }
 
 #ifdef CONFIG_XEN
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough
  2015-03-18  9:06 [Qemu-devel] [v7][PATCH 00/10] xen: add Intel IGD passthrough support Tiejun Chen
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 01/10] i440fx: make types configurable at run-time Tiejun Chen
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 02/10] pc_init1: pass parameters just with types Tiejun Chen
@ 2015-03-18  9:06 ` Tiejun Chen
  2015-03-18 10:21   ` Gerd Hoffmann
  2015-04-15  9:17   ` eraul_cn
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 04/10] hw/pci-assign: split pci-assign.c Tiejun Chen
                   ` (6 subsequent siblings)
  9 siblings, 2 replies; 22+ messages in thread
From: Tiejun Chen @ 2015-03-18  9:06 UTC (permalink / raw)
  To: mst, kraxel, pbonzini, rth; +Cc: allen.m.kay, qemu-devel

Implement a pci host bridge specific to passthrough. Actually
this just inherits the standard one. And we also just expose
a minimal real host bridge pci configuration subset.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/pci-host/piix.c   | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/i386/pc.h |  2 ++
 2 files changed, 84 insertions(+)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index c812eaa..0906ba5 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -728,6 +728,87 @@ static const TypeInfo i440fx_info = {
     .class_init    = i440fx_class_init,
 };
 
+/* IGD Passthrough Host Bridge. */
+typedef struct {
+    uint8_t offset;
+    uint8_t len;
+} IGDHostInfo;
+
+/* Here we just expose minimal host bridge offset subset. */
+static const IGDHostInfo igd_host_bridge_infos[] = {
+    {0x08, 2},  /* revision id */
+    {0x2c, 2},  /* sybsystem vendor id */
+    {0x2e, 2},  /* sybsystem id */
+    {0x50, 2},  /* SNB: processor graphics control register */
+    {0x52, 2},  /* processor graphics control register */
+    {0xa4, 4},  /* SNB: graphics base of stolen memory */
+    {0xa8, 4},  /* SNB: base of GTT stolen memory */
+};
+
+static int host_pci_config_read(int pos, int len, uint32_t val)
+{
+    char path[PATH_MAX];
+    int config_fd;
+    ssize_t size = sizeof(path);
+    /* Access real host bridge. */
+    int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
+                      0, 0, 0, 0, "config");
+
+    if (rc >= size || rc < 0) {
+        return -ENODEV;
+    }
+
+    config_fd = open(path, O_RDWR);
+    if (config_fd < 0) {
+        return -ENODEV;
+    }
+
+    do {
+        rc = pread(config_fd, (uint8_t *)&val, len, pos);
+    } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
+    if (rc != len) {
+        return -errno;
+    }
+
+    return 0;
+}
+
+static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
+{
+    uint32_t val = 0;
+    int rc, i, num;
+    int pos, len;
+
+    num = ARRAY_SIZE(igd_host_bridge_infos);
+    for (i = 0; i < num; i++) {
+        pos = igd_host_bridge_infos[i].offset;
+        len = igd_host_bridge_infos[i].len;
+        rc = host_pci_config_read(pos, len, val);
+        if (rc) {
+            return -ENODEV;
+        }
+        pci_default_write_config(pci_dev, pos, val, len);
+    }
+
+    return 0;
+}
+
+static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->init = igd_pt_i440fx_initfn;
+    dc->desc = "IGD Passthrough Host bridge";
+}
+
+static const TypeInfo igd_passthrough_i440fx_info = {
+    .name          = TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
+    .parent        = TYPE_I440FX_PCI_DEVICE,
+    .instance_size = sizeof(PCII440FXState),
+    .class_init    = igd_passthrough_i440fx_class_init,
+};
+
 static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
                                                 PCIBus *rootbus)
 {
@@ -769,6 +850,7 @@ static const TypeInfo i440fx_pcihost_info = {
 static void i440fx_register_types(void)
 {
     type_register_static(&i440fx_info);
+    type_register_static(&igd_passthrough_i440fx_info);
     type_register_static(&piix3_info);
     type_register_static(&piix3_xen_info);
     type_register_static(&i440fx_pcihost_info);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 99dc26b..ec3ca88 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -233,6 +233,8 @@ typedef struct PCII440FXState PCII440FXState;
 #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
 #define TYPE_I440FX_PCI_DEVICE "i440FX"
 
+#define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
+
 PCIBus *i440fx_init(const char *host_type, const char *pci_type,
                     PCII440FXState **pi440fx_state, int *piix_devfn,
                     ISABus **isa_bus, qemu_irq *pic,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [v7][PATCH 04/10] hw/pci-assign: split pci-assign.c
  2015-03-18  9:06 [Qemu-devel] [v7][PATCH 00/10] xen: add Intel IGD passthrough support Tiejun Chen
                   ` (2 preceding siblings ...)
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough Tiejun Chen
@ 2015-03-18  9:06 ` Tiejun Chen
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 05/10] xen, gfx passthrough: basic graphics passthrough support Tiejun Chen
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Tiejun Chen @ 2015-03-18  9:06 UTC (permalink / raw)
  To: mst, kraxel, pbonzini, rth; +Cc: allen.m.kay, qemu-devel

We will try to reuse assign_dev_load_option_rom in xen side, and
especially its a good beginning to unify pci assign codes both on
kvm and xen in the future.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/Makefile.objs         |  1 +
 hw/i386/kvm/pci-assign.c      | 82 ++++----------------------------------
 hw/i386/pci-assign-load-rom.c | 93 +++++++++++++++++++++++++++++++++++++++++++
 include/hw/pci/pci-assign.h   | 27 +++++++++++++
 4 files changed, 128 insertions(+), 75 deletions(-)
 create mode 100644 hw/i386/pci-assign-load-rom.c
 create mode 100644 include/hw/pci/pci-assign.h

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index e058a39..8bd1932 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -7,6 +7,7 @@ obj-$(CONFIG_XEN) += ../xenpv/ xen/
 
 obj-y += kvmvapic.o
 obj-y += acpi-build.o
+obj-y += pci-assign-load-rom.o
 hw/i386/acpi-build.o: hw/i386/acpi-build.c \
 	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
 	hw/i386/ssdt-tpm.hex
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 9db7c77..fe870c9 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -37,6 +37,7 @@
 #include "hw/pci/pci.h"
 #include "hw/pci/msi.h"
 #include "kvm_i386.h"
+#include "hw/pci/pci-assign.h"
 
 #define MSIX_PAGE_SIZE 0x1000
 
@@ -48,17 +49,6 @@
 #define IORESOURCE_PREFETCH 0x00002000  /* No side effects */
 #define IORESOURCE_MEM_64   0x00100000
 
-//#define DEVICE_ASSIGNMENT_DEBUG
-
-#ifdef DEVICE_ASSIGNMENT_DEBUG
-#define DEBUG(fmt, ...)                                       \
-    do {                                                      \
-        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
-    } while (0)
-#else
-#define DEBUG(fmt, ...)
-#endif
-
 typedef struct PCIRegion {
     int type;           /* Memory or port I/O */
     int valid;
@@ -1893,73 +1883,15 @@ static void assign_register_types(void)
 
 type_init(assign_register_types)
 
-/*
- * Scan the assigned devices for the devices that have an option ROM, and then
- * load the corresponding ROM data to RAM. If an error occurs while loading an
- * option ROM, we just ignore that option ROM and continue with the next one.
- */
 static void assigned_dev_load_option_rom(AssignedDevice *dev)
 {
-    char name[32], rom_file[64];
-    FILE *fp;
-    uint8_t val;
-    struct stat st;
-    void *ptr;
-
-    /* If loading ROM from file, pci handles it */
-    if (dev->dev.romfile || !dev->dev.rom_bar) {
-        return;
-    }
+    int size = 0;
 
-    snprintf(rom_file, sizeof(rom_file),
-             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
-             dev->host.domain, dev->host.bus, dev->host.slot,
-             dev->host.function);
+    pci_assign_dev_load_option_rom(&dev->dev, OBJECT(dev), &size,
+                                   dev->host.domain, dev->host.bus,
+                                   dev->host.slot, dev->host.function);
 
-    if (stat(rom_file, &st)) {
-        return;
-    }
-
-    if (access(rom_file, F_OK)) {
-        error_report("pci-assign: Insufficient privileges for %s", rom_file);
-        return;
-    }
-
-    /* Write "1" to the ROM file to enable it */
-    fp = fopen(rom_file, "r+");
-    if (fp == NULL) {
-        return;
+    if (!size) {
+        error_report("pci-assign: Invalid ROM.");
     }
-    val = 1;
-    if (fwrite(&val, 1, 1, fp) != 1) {
-        goto close_rom;
-    }
-    fseek(fp, 0, SEEK_SET);
-
-    snprintf(name, sizeof(name), "%s.rom",
-            object_get_typename(OBJECT(dev)));
-    memory_region_init_ram(&dev->dev.rom, OBJECT(dev), name, st.st_size,
-                           &error_abort);
-    vmstate_register_ram(&dev->dev.rom, &dev->dev.qdev);
-    ptr = memory_region_get_ram_ptr(&dev->dev.rom);
-    memset(ptr, 0xff, st.st_size);
-
-    if (!fread(ptr, 1, st.st_size, fp)) {
-        error_report("pci-assign: Cannot read from host %s", rom_file);
-        error_printf("Device option ROM contents are probably invalid "
-                     "(check dmesg).\nSkip option ROM probe with rombar=0, "
-                     "or load from file with romfile=\n");
-        goto close_rom;
-    }
-
-    pci_register_bar(&dev->dev, PCI_ROM_SLOT, 0, &dev->dev.rom);
-    dev->dev.has_rom = true;
-close_rom:
-    /* Write "0" to disable ROM */
-    fseek(fp, 0, SEEK_SET);
-    val = 0;
-    if (!fwrite(&val, 1, 1, fp)) {
-        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
-    }
-    fclose(fp);
 }
diff --git a/hw/i386/pci-assign-load-rom.c b/hw/i386/pci-assign-load-rom.c
new file mode 100644
index 0000000..a04b540
--- /dev/null
+++ b/hw/i386/pci-assign-load-rom.c
@@ -0,0 +1,93 @@
+/*
+ * This is splited from hw/i386/kvm/pci-assign.c
+ */
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/io.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include "hw/hw.h"
+#include "hw/i386/pc.h"
+#include "qemu/error-report.h"
+#include "ui/console.h"
+#include "hw/loader.h"
+#include "monitor/monitor.h"
+#include "qemu/range.h"
+#include "sysemu/sysemu.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci-assign.h"
+
+/*
+ * Scan the assigned devices for the devices that have an option ROM, and then
+ * load the corresponding ROM data to RAM. If an error occurs while loading an
+ * option ROM, we just ignore that option ROM and continue with the next one.
+ */
+void *pci_assign_dev_load_option_rom(PCIDevice *dev, struct Object *owner,
+                                     int *size, unsigned int domain,
+                                     unsigned int bus, unsigned int slot,
+                                     unsigned int function)
+{
+    char name[32], rom_file[64];
+    FILE *fp;
+    uint8_t val;
+    struct stat st;
+    void *ptr;
+
+    /* If loading ROM from file, pci handles it */
+    if (dev->romfile || !dev->rom_bar) {
+        return NULL;
+    }
+
+    snprintf(rom_file, sizeof(rom_file),
+             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
+             domain, bus, slot, function);
+
+    if (stat(rom_file, &st)) {
+        return NULL;
+    }
+
+    if (access(rom_file, F_OK)) {
+        error_report("pci-assign: Insufficient privileges for %s", rom_file);
+        return NULL;
+    }
+
+    /* Write "1" to the ROM file to enable it */
+    fp = fopen(rom_file, "r+");
+    if (fp == NULL) {
+        return NULL;
+    }
+    val = 1;
+    if (fwrite(&val, 1, 1, fp) != 1) {
+        goto close_rom;
+    }
+    fseek(fp, 0, SEEK_SET);
+
+    snprintf(name, sizeof(name), "%s.rom", object_get_typename(owner));
+    memory_region_init_ram(&dev->rom, owner, name, st.st_size, &error_abort);
+    vmstate_register_ram(&dev->rom, &dev->qdev);
+    ptr = memory_region_get_ram_ptr(&dev->rom);
+    memset(ptr, 0xff, st.st_size);
+
+    if (!fread(ptr, 1, st.st_size, fp)) {
+        error_report("pci-assign: Cannot read from host %s", rom_file);
+        error_printf("Device option ROM contents are probably invalid "
+                     "(check dmesg).\nSkip option ROM probe with rombar=0, "
+                     "or load from file with romfile=\n");
+        goto close_rom;
+    }
+
+    pci_register_bar(dev, PCI_ROM_SLOT, 0, &dev->rom);
+    dev->has_rom = true;
+    *size = st.st_size;
+close_rom:
+    /* Write "0" to disable ROM */
+    fseek(fp, 0, SEEK_SET);
+    val = 0;
+    if (!fwrite(&val, 1, 1, fp)) {
+        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
+    }
+    fclose(fp);
+
+    return ptr;
+}
diff --git a/include/hw/pci/pci-assign.h b/include/hw/pci/pci-assign.h
new file mode 100644
index 0000000..55f42c5
--- /dev/null
+++ b/include/hw/pci/pci-assign.h
@@ -0,0 +1,27 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Just split from hw/i386/kvm/pci-assign.c.
+ */
+#ifndef PCI_ASSIGN_H
+#define PCI_ASSIGN_H
+
+#include "hw/pci/pci.h"
+
+//#define DEVICE_ASSIGNMENT_DEBUG
+
+#ifdef DEVICE_ASSIGNMENT_DEBUG
+#define DEBUG(fmt, ...)                                       \
+    do {                                                      \
+        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
+    } while (0)
+#else
+#define DEBUG(fmt, ...)
+#endif
+
+void *pci_assign_dev_load_option_rom(PCIDevice *dev, struct Object *owner,
+                                     int *size, unsigned int domain,
+                                     unsigned int bus, unsigned int slot,
+                                     unsigned int function);
+#endif /* PCI_ASSIGN_H */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [v7][PATCH 05/10] xen, gfx passthrough: basic graphics passthrough support
  2015-03-18  9:06 [Qemu-devel] [v7][PATCH 00/10] xen: add Intel IGD passthrough support Tiejun Chen
                   ` (3 preceding siblings ...)
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 04/10] hw/pci-assign: split pci-assign.c Tiejun Chen
@ 2015-03-18  9:06 ` Tiejun Chen
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 06/10] xen, gfx passthrough: retrieve VGA BIOS to work Tiejun Chen
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Tiejun Chen @ 2015-03-18  9:06 UTC (permalink / raw)
  To: mst, kraxel, pbonzini, rth; +Cc: allen.m.kay, qemu-devel

basic gfx passthrough support:
- add a vga type for gfx passthrough
- register/unregister legacy VGA I/O ports and MMIOs for passthrough GFX

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 hw/core/machine.c            |  20 ++++++++
 hw/xen/Makefile.objs         |   1 +
 hw/xen/xen-host-pci-device.c |   5 ++
 hw/xen/xen-host-pci-device.h |   1 +
 hw/xen/xen_pt.c              |   4 ++
 hw/xen/xen_pt.h              |  10 +++-
 hw/xen/xen_pt_graphics.c     | 111 +++++++++++++++++++++++++++++++++++++++++++
 include/hw/boards.h          |   1 +
 qemu-options.hx              |   3 ++
 vl.c                         |  10 ++++
 10 files changed, 165 insertions(+), 1 deletion(-)
 create mode 100644 hw/xen/xen_pt_graphics.c

diff --git a/hw/core/machine.c b/hw/core/machine.c
index cb1185a..6ac3ee4 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -225,6 +225,20 @@ static void machine_set_usb(Object *obj, bool value, Error **errp)
     ms->usb = value;
 }
 
+static bool machine_get_igd_gfx_passthru(Object *obj, Error **errp)
+{
+    MachineState *ms = MACHINE(obj);
+
+    return ms->igd_gfx_passthru;
+}
+
+static void machine_set_igd_gfx_passthru(Object *obj, bool value, Error **errp)
+{
+    MachineState *ms = MACHINE(obj);
+
+    ms->igd_gfx_passthru = value;
+}
+
 static char *machine_get_firmware(Object *obj, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
@@ -379,6 +393,12 @@ static void machine_initfn(Object *obj)
     object_property_set_description(obj, "usb",
                                     "Set on/off to enable/disable usb",
                                     NULL);
+    object_property_add_bool(obj, "igd-passthru",
+                             machine_get_igd_gfx_passthru,
+                             machine_set_igd_gfx_passthru, NULL);
+    object_property_set_description(obj, "igd-passthru",
+                                    "Set on/off to enable/disable igd passthrou",
+                                    NULL);
     object_property_add_str(obj, "firmware",
                             machine_get_firmware,
                             machine_set_firmware, NULL);
diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
index a0ca0aa..a9ad7e7 100644
--- a/hw/xen/Makefile.objs
+++ b/hw/xen/Makefile.objs
@@ -3,3 +3,4 @@ common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
 
 obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
 obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o
+obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o xen_pt_graphics.o
diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
index 743b37b..a54b7de 100644
--- a/hw/xen/xen-host-pci-device.c
+++ b/hw/xen/xen-host-pci-device.c
@@ -376,6 +376,11 @@ int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
         goto error;
     }
     d->irq = v;
+    rc = xen_host_pci_get_hex_value(d, "class", &v);
+    if (rc) {
+        goto error;
+    }
+    d->class_code = v;
     d->is_virtfn = xen_host_pci_dev_is_virtfn(d);
 
     return 0;
diff --git a/hw/xen/xen-host-pci-device.h b/hw/xen/xen-host-pci-device.h
index c2486f0..f1e1c30 100644
--- a/hw/xen/xen-host-pci-device.h
+++ b/hw/xen/xen-host-pci-device.h
@@ -25,6 +25,7 @@ typedef struct XenHostPCIDevice {
 
     uint16_t vendor_id;
     uint16_t device_id;
+    uint32_t class_code;
     int irq;
 
     XenHostPCIIORegion io_regions[PCI_NUM_REGIONS - 1];
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index f2893b2..1d78021 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -450,6 +450,7 @@ static int xen_pt_register_regions(XenPCIPassthroughState *s)
                    d->rom.size, d->rom.base_addr);
     }
 
+    xen_pt_register_vga_regions(d);
     return 0;
 }
 
@@ -748,6 +749,7 @@ out:
 static void xen_pt_unregister_device(PCIDevice *d)
 {
     XenPCIPassthroughState *s = DO_UPCAST(XenPCIPassthroughState, dev, d);
+    XenHostPCIDevice *host_dev = &s->real_device;
     uint8_t machine_irq = s->machine_irq;
     uint8_t intx = xen_pt_pci_intx(s);
     int rc;
@@ -791,6 +793,8 @@ static void xen_pt_unregister_device(PCIDevice *d)
     /* delete all emulated config registers */
     xen_pt_config_delete(s);
 
+    xen_pt_unregister_vga_regions(host_dev);
+
     memory_listener_unregister(&s->memory_listener);
     memory_listener_unregister(&s->io_listener);
 
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 942dc60..3325387 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -298,5 +298,13 @@ static inline bool xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
     return s->msix && s->msix->bar_index == bar;
 }
 
-
+extern bool has_igd_gfx_passthru;
+static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
+{
+    return (has_igd_gfx_passthru
+            && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
+}
+int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
+int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
+int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev);
 #endif /* !XEN_PT_H */
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
new file mode 100644
index 0000000..9b3df81
--- /dev/null
+++ b/hw/xen/xen_pt_graphics.c
@@ -0,0 +1,111 @@
+/*
+ * graphics passthrough
+ */
+#include "xen_pt.h"
+#include "xen-host-pci-device.h"
+#include "hw/xen/xen_backend.h"
+
+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_igd_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_igd_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;
+}
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 1feea2b..9dd3017 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -141,6 +141,7 @@ struct MachineState {
     bool dump_guest_core;
     bool mem_merge;
     bool usb;
+    bool igd_gfx_passthru;
     char *firmware;
     bool iommu;
     bool suppress_vmdesc;
diff --git a/qemu-options.hx b/qemu-options.hx
index c513352..448803f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -38,6 +38,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
     "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
     "                mem-merge=on|off controls memory merge support (default: on)\n"
     "                iommu=on|off controls emulated Intel IOMMU (VT-d) support (default=off)\n"
+    "                igd-passthru=on|off controls IGD GFX passthrough support (default=off)\n"
     "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
     "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
     "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n",
@@ -55,6 +56,8 @@ than one accelerator specified, the next one is used if the previous one fails
 to initialize.
 @item kernel_irqchip=on|off
 Enables in-kernel irqchip support for the chosen accelerator when available.
+@item gfx_passthru=on|off
+Enables IGD GFX passthrough support for the chosen machine when available.
 @item vmport=on|off|auto
 Enables emulation of VMWare IO port, for vmmouse etc. auto says to select the
 value based on accel. For accel=xen the default is off otherwise the default
diff --git a/vl.c b/vl.c
index 694deb4..3f0c724 100644
--- a/vl.c
+++ b/vl.c
@@ -1221,6 +1221,13 @@ static void configure_msg(QemuOpts *opts)
     enable_timestamp_msg = qemu_opt_get_bool(opts, "timestamp", true);
 }
 
+/* Now we still need this for compatibility with XEN. */
+bool has_igd_gfx_passthru;
+static void igd_gfx_passthru(void)
+{
+    has_igd_gfx_passthru = current_machine->igd_gfx_passthru;
+}
+
 /***********************************************************/
 /* USB devices */
 
@@ -4235,6 +4242,9 @@ int main(int argc, char **argv, char **envp)
             exit(1);
     }
 
+    /* Check if IGD GFX passthrough. */
+    igd_gfx_passthru();
+
     /* init generic devices */
     if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, 1) != 0)
         exit(1);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [v7][PATCH 06/10] xen, gfx passthrough: retrieve VGA BIOS to work
  2015-03-18  9:06 [Qemu-devel] [v7][PATCH 00/10] xen: add Intel IGD passthrough support Tiejun Chen
                   ` (4 preceding siblings ...)
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 05/10] xen, gfx passthrough: basic graphics passthrough support Tiejun Chen
@ 2015-03-18  9:06 ` Tiejun Chen
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 07/10] igd gfx passthrough: create a isa bridge Tiejun Chen
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Tiejun Chen @ 2015-03-18  9:06 UTC (permalink / raw)
  To: mst, kraxel, pbonzini, rth; +Cc: allen.m.kay, qemu-devel

Now we retrieve VGA bios like kvm stuff in qemu but we need to
fix Device Identification in case if its not matched with the
real IGD device since Seabios is always trying to compare this
ID to work out VGA BIOS.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/xen/xen_pt.c          | 10 ++++++
 hw/xen/xen_pt.h          |  5 +++
 hw/xen/xen_pt_graphics.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+)

diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index 1d78021..fcc9f1c 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -672,6 +672,16 @@ static int xen_pt_initfn(PCIDevice *d)
     s->memory_listener = xen_pt_memory_listener;
     s->io_listener = xen_pt_io_listener;
 
+    /* Setup VGA bios for passthrough GFX */
+    if ((s->real_device.domain == 0) && (s->real_device.bus == 0) &&
+        (s->real_device.dev == 2) && (s->real_device.func == 0)) {
+        if (xen_pt_setup_vga(s, &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;
+        }
+    }
+
     /* Handle real device's MMIO/PIO BARs */
     xen_pt_register_regions(s);
 
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 3325387..d9b1ce0 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -298,6 +298,11 @@ static inline bool xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
     return s->msix && s->msix->bar_index == bar;
 }
 
+extern void *pci_assign_dev_load_option_rom(PCIDevice *dev,
+                                            struct Object *owner, int *size,
+                                            unsigned int domain,
+                                            unsigned int bus, unsigned int slot,
+                                            unsigned int function);
 extern bool has_igd_gfx_passthru;
 static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
 {
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 9b3df81..3232296 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -109,3 +109,82 @@ int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
 
     return 0;
 }
+
+static void *get_vgabios(XenPCIPassthroughState *s, int *size,
+                       XenHostPCIDevice *dev)
+{
+    return pci_assign_dev_load_option_rom(&s->dev, OBJECT(&s->dev), size,
+                                          dev->domain, dev->bus,
+                                          dev->dev, dev->func);
+}
+
+/* Refer to Seabios. */
+struct rom_header {
+    uint16_t signature;
+    uint8_t size;
+    uint8_t initVector[4];
+    uint8_t reserved[17];
+    uint16_t pcioffset;
+    uint16_t pnpoffset;
+} __attribute__((packed));
+
+struct pci_data {
+    uint32_t signature;
+    uint16_t vendor;
+    uint16_t device;
+    uint16_t vitaldata;
+    uint16_t dlen;
+    uint8_t drevision;
+    uint8_t class_lo;
+    uint16_t class_hi;
+    uint16_t ilen;
+    uint16_t irevision;
+    uint8_t type;
+    uint8_t indicator;
+    uint16_t reserved;
+} __attribute__((packed));
+
+int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev)
+{
+    unsigned char *bios = NULL;
+    struct rom_header *rom;
+    int bios_size;
+    char *c = NULL;
+    char checksum = 0;
+    uint32_t len = 0;
+    struct pci_data *pd = NULL;
+
+    if (!is_igd_vga_passthrough(dev)) {
+        return -1;
+    }
+
+    bios = get_vgabios(s, &bios_size, dev);
+    if (!bios) {
+        XEN_PT_ERR(&s->dev, "VGA: Can't getting VBIOS!\n");
+        return -1;
+    }
+
+    /* Currently we fixed this address as a primary. */
+    rom = (struct rom_header *)bios;
+    pd = (void *)(bios + (unsigned char)rom->pcioffset);
+
+    /* We may need to fixup Device Identification. */
+    if (pd->device != s->real_device.device_id) {
+        pd->device = s->real_device.device_id;
+
+        len = rom->size * 512;
+        /* Then adjust the bios checksum */
+        for (c = (char *)bios; c < ((char *)bios + len); c++) {
+            checksum += *c;
+        }
+        if (checksum) {
+            bios[len - 1] -= checksum;
+            XEN_PT_LOG(&s->dev, "vga bios checksum is adjusted %x!\n",
+                       checksum);
+        }
+    }
+
+    /* Currently we fixed this address as a primary for legacy BIOS. */
+    cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
+    return 0;
+}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [v7][PATCH 07/10] igd gfx passthrough: create a isa bridge
  2015-03-18  9:06 [Qemu-devel] [v7][PATCH 00/10] xen: add Intel IGD passthrough support Tiejun Chen
                   ` (5 preceding siblings ...)
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 06/10] xen, gfx passthrough: retrieve VGA BIOS to work Tiejun Chen
@ 2015-03-18  9:06 ` Tiejun Chen
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 08/10] xen, gfx passthrough: register " Tiejun Chen
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Tiejun Chen @ 2015-03-18  9:06 UTC (permalink / raw)
  To: mst, kraxel, pbonzini, rth; +Cc: allen.m.kay, qemu-devel

Currently IGD drivers always need to access PCH by 1f.0. But we
don't want to poke that directly to get ID, and although in real
world different GPU should have different PCH. But actually the
different PCH DIDs likely map to different PCH SKUs. We do the
same thing for the GPU. For PCH, the different SKUs are going to
be all the same silicon design and implementation, just different
features turn on and off with fuses. The SW interfaces should be
consistent across all SKUs in a given family (eg LPT). But just
same features may not be supported.

Most of these different PCH features probably don't matter to the
Gfx driver, but obviously any difference in display port connections
will so it should be fine with any PCH in case of passthrough.

So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
scenarios, 0x9cc3 for BDW(Broadwell).

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/pc_piix.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 113 insertions(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index cea3a5c..8fbfc09 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -956,6 +956,119 @@ static QEMUMachine pc_machine_v0_10 = {
     .hw_version = "0.10",
 };
 
+typedef struct {
+    uint16_t gpu_device_id;
+    uint16_t pch_device_id;
+    uint8_t pch_revision_id;
+} IGDDeviceIDInfo;
+
+/* In real world different GPU should have different PCH. But actually
+ * the different PCH DIDs likely map to different PCH SKUs. We do the
+ * same thing for the GPU. For PCH, the different SKUs are going to be
+ * all the same silicon design and implementation, just different
+ * features turn on and off with fuses. The SW interfaces should be
+ * consistent across all SKUs in a given family (eg LPT). But just same
+ * features may not be supported.
+ *
+ * Most of these different PCH features probably don't matter to the
+ * Gfx driver, but obviously any difference in display port connections
+ * will so it should be fine with any PCH in case of passthrough.
+ *
+ * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
+ * scenarios, 0x9cc3 for BDW(Broadwell).
+ */
+static const IGDDeviceIDInfo igd_combo_id_infos[] = {
+    /* HSW Classic */
+    {0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */
+    {0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */
+    {0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */
+    {0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */
+    {0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */
+    /* HSW ULT */
+    {0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */
+    {0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */
+    {0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */
+    {0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */
+    {0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */
+    {0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */
+    /* HSW CRW */
+    {0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */
+    {0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */
+    /* HSW Server */
+    {0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */
+    /* HSW SRVR */
+    {0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */
+    /* BSW */
+    {0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */
+    {0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */
+    {0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */
+    {0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */
+    {0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */
+    {0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */
+    {0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */
+    {0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */
+    {0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */
+    {0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */
+    {0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */
+};
+
+static void isa_bridge_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    dc->desc        = "ISA bridge faked to support IGD PT";
+    k->vendor_id    = PCI_VENDOR_ID_INTEL;
+    k->class_id     = PCI_CLASS_BRIDGE_ISA;
+};
+
+static TypeInfo isa_bridge_info = {
+    .name          = "igd-passthrough-isa-bridge",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCIDevice),
+    .class_init = isa_bridge_class_init,
+};
+
+static void pt_graphics_register_types(void)
+{
+    type_register_static(&isa_bridge_info);
+}
+type_init(pt_graphics_register_types)
+
+void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id)
+{
+    struct PCIDevice *bridge_dev;
+    int i, num;
+    uint16_t pch_dev_id = 0xffff;
+    uint8_t pch_rev_id;
+
+    num = ARRAY_SIZE(igd_combo_id_infos);
+    for (i = 0; i < num; i++) {
+        if (gpu_dev_id == igd_combo_id_infos[i].gpu_device_id) {
+            pch_dev_id = igd_combo_id_infos[i].pch_device_id;
+            pch_rev_id = igd_combo_id_infos[i].pch_revision_id;
+        }
+    }
+
+    if (pch_dev_id == 0xffff) {
+        return;
+    }
+
+    /* Currently IGD drivers always need to access PCH by 1f.0. */
+    bridge_dev = pci_create_simple(bus, PCI_DEVFN(0x1f, 0),
+                                   "igd-passthrough-isa-bridge");
+
+    /*
+     * Note that vendor id is always PCI_VENDOR_ID_INTEL.
+     */
+    if (!bridge_dev) {
+        fprintf(stderr, "set igd-passthrough-isa-bridge failed!\n");
+        return;
+    }
+    pci_config_set_device_id(bridge_dev->config, pch_dev_id);
+    pci_config_set_revision(bridge_dev->config, pch_rev_id);
+}
+
 static QEMUMachine isapc_machine = {
     PC_COMMON_MACHINE_OPTIONS,
     .name = "isapc",
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [v7][PATCH 08/10] xen, gfx passthrough: register a isa bridge
  2015-03-18  9:06 [Qemu-devel] [v7][PATCH 00/10] xen: add Intel IGD passthrough support Tiejun Chen
                   ` (6 preceding siblings ...)
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 07/10] igd gfx passthrough: create a isa bridge Tiejun Chen
@ 2015-03-18  9:06 ` Tiejun Chen
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 09/10] xen, gfx passthrough: register host bridge specific to passthrough Tiejun Chen
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 10/10] xen, gfx passthrough: add opregion mapping Tiejun Chen
  9 siblings, 0 replies; 22+ messages in thread
From: Tiejun Chen @ 2015-03-18  9:06 UTC (permalink / raw)
  To: mst, kraxel, pbonzini, rth; +Cc: allen.m.kay, qemu-devel

Currently we just register this isa bridge when we use IGD
passthrough in Xen side.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/xen/xen_pt.c      | 18 ++++++++++++++++++
 include/hw/xen/xen.h |  1 +
 2 files changed, 19 insertions(+)

diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index fcc9f1c..2d5cebb 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -632,6 +632,21 @@ static const MemoryListener xen_pt_io_listener = {
     .priority = 10,
 };
 
+static void
+xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
+                                      XenHostPCIDevice *dev)
+{
+    uint16_t gpu_dev_id;
+    PCIDevice *d = &s->dev;
+
+    if (!is_igd_vga_passthrough(dev)) {
+        return;
+    }
+
+    gpu_dev_id = dev->device_id;
+    igd_passthrough_isa_bridge_create(d->bus, gpu_dev_id);
+}
+
 /* init */
 
 static int xen_pt_initfn(PCIDevice *d)
@@ -680,6 +695,9 @@ static int xen_pt_initfn(PCIDevice *d)
             xen_host_pci_device_put(&s->real_device);
             return -1;
         }
+
+        /* Register ISA bridge for passthrough GFX. */
+        xen_igd_passthrough_isa_bridge_create(s, &s->real_device);
     }
 
     /* Handle real device's MMIO/PIO BARs */
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 4356af4..703148e 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -51,4 +51,5 @@ void xen_register_framebuffer(struct MemoryRegion *mr);
 #  define HVM_MAX_VCPUS 32
 #endif
 
+extern void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id);
 #endif /* QEMU_HW_XEN_H */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [v7][PATCH 09/10] xen, gfx passthrough: register host bridge specific to passthrough
  2015-03-18  9:06 [Qemu-devel] [v7][PATCH 00/10] xen: add Intel IGD passthrough support Tiejun Chen
                   ` (7 preceding siblings ...)
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 08/10] xen, gfx passthrough: register " Tiejun Chen
@ 2015-03-18  9:06 ` Tiejun Chen
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 10/10] xen, gfx passthrough: add opregion mapping Tiejun Chen
  9 siblings, 0 replies; 22+ messages in thread
From: Tiejun Chen @ 2015-03-18  9:06 UTC (permalink / raw)
  To: mst, kraxel, pbonzini, rth; +Cc: allen.m.kay, qemu-devel

Just register that pci host bridge specific to passthrough.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/pc_piix.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 8fbfc09..eae2d20 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -50,7 +50,8 @@
 #include "cpu.h"
 #include "qemu/error-report.h"
 #ifdef CONFIG_XEN
-#  include <xen/hvm/hvm_info_table.h>
+#include <xen/hvm/hvm_info_table.h>
+#include "hw/xen/xen_pt.h"
 #endif
 
 #define MAX_IDE_BUS 2
@@ -504,11 +505,25 @@ static void pc_init_isa(MachineState *machine)
 }
 
 #ifdef CONFIG_XEN
+static void igd_passthrough_pc_init_pci(MachineState *machine)
+{
+    pc_init1(machine, 1, 1, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);
+}
+
+static void pc_igd_init_pci(MachineState *machine)
+{
+    if (has_igd_gfx_passthru)
+        igd_passthrough_pc_init_pci(machine);
+    else
+        pc_init_pci(machine);
+}
+
 static void pc_xen_hvm_init(MachineState *machine)
 {
     PCIBus *bus;
 
-    pc_init_pci(machine);
+    pc_igd_init_pci(machine);
 
     bus = pci_find_primary_bus();
     if (bus != NULL) {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [v7][PATCH 10/10] xen, gfx passthrough: add opregion mapping
  2015-03-18  9:06 [Qemu-devel] [v7][PATCH 00/10] xen: add Intel IGD passthrough support Tiejun Chen
                   ` (8 preceding siblings ...)
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 09/10] xen, gfx passthrough: register host bridge specific to passthrough Tiejun Chen
@ 2015-03-18  9:06 ` Tiejun Chen
  9 siblings, 0 replies; 22+ messages in thread
From: Tiejun Chen @ 2015-03-18  9:06 UTC (permalink / raw)
  To: mst, kraxel, pbonzini, rth; +Cc: allen.m.kay, qemu-devel

The OpRegion shouldn't be mapped 1:1 because the address in the host
can't be used in the guest directly.

This patch traps read and write access to the opregion of the Intel
GPU config space (offset 0xfc).

The original patch is from Jean Guyader <jean.guyader@eu.citrix.com>

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 hw/xen/xen_pt.h             |  6 +++-
 hw/xen/xen_pt_config_init.c | 52 ++++++++++++++++++++++++++--
 hw/xen/xen_pt_graphics.c    | 82 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 137 insertions(+), 3 deletions(-)

diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index d9b1ce0..20cc9b9 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -36,6 +36,9 @@ typedef struct XenPTReg XenPTReg;
 
 typedef struct XenPCIPassthroughState XenPCIPassthroughState;
 
+uint32_t igd_read_opregion(XenPCIPassthroughState *s);
+void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
+
 /* function type for config reg */
 typedef int (*xen_pt_conf_reg_init)
     (XenPCIPassthroughState *, XenPTRegInfo *, uint32_t real_offset,
@@ -62,8 +65,9 @@ typedef int (*xen_pt_conf_byte_read)
 #define XEN_PT_BAR_ALLF 0xFFFFFFFF
 #define XEN_PT_BAR_UNMAPPED (-1)
 
-#define PCI_CAP_MAX 48
+#define XEN_PCI_CAP_MAX 48
 
+#define XEN_PCI_INTEL_OPREGION 0xfc
 
 typedef enum {
     XEN_PT_GRP_TYPE_HARDWIRED = 0,  /* 0 Hardwired reg group */
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index d99c22e..abcfe1b 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -573,6 +573,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 */
@@ -1438,6 +1454,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
@@ -1675,6 +1705,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      = XEN_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,
     },
@@ -1725,7 +1763,7 @@ out:
 static uint8_t find_cap_offset(XenPCIPassthroughState *s, uint8_t cap)
 {
     uint8_t id;
-    unsigned max_cap = PCI_CAP_MAX;
+    unsigned max_cap = XEN_PCI_CAP_MAX;
     uint8_t pos = PCI_CAPABILITY_LIST;
     uint8_t status = 0;
 
@@ -1804,7 +1842,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 != XEN_PCI_INTEL_OPREGION) {
             if (xen_pt_hide_dev_cap(&s->real_device,
                                     xen_pt_emu_reg_grps[i].grp_id)) {
                 continue;
@@ -1817,6 +1856,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 == XEN_PCI_INTEL_OPREGION) {
+            reg_grp_offset = XEN_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);
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 3232296..df6069b 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -5,6 +5,11 @@
 #include "xen-host-pci-device.h"
 #include "hw/xen/xen_backend.h"
 
+static unsigned long igd_guest_opregion;
+static unsigned long igd_host_opregion;
+
+#define XEN_PCI_INTEL_OPREGION_MASK 0xfff
+
 typedef struct VGARegion {
     int type;           /* Memory or port I/O */
     uint64_t guest_base_addr;
@@ -81,6 +86,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_igd_vga_passthrough(dev)) {
         return 0;
@@ -107,6 +113,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;
 }
 
@@ -188,3 +205,68 @@ int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev)
     cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
     return 0;
 }
+
+uint32_t igd_read_opregion(XenPCIPassthroughState *s)
+{
+    uint32_t val = 0;
+
+    if (!igd_guest_opregion) {
+        return val;
+    }
+
+    val = igd_guest_opregion;
+
+    XEN_PT_LOG(&s->dev, "Read opregion val=%x\n", val);
+    return val;
+}
+
+#define XEN_PCI_INTEL_OPREGION_PAGES 0x3
+#define XEN_PCI_INTEL_OPREGION_ENABLE_ACCESSED 0x1
+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;
+    }
+
+    /* We just work with LE. */
+    xen_host_pci_get_block(&s->real_device, XEN_PCI_INTEL_OPREGION,
+            (uint8_t *)&igd_host_opregion, 4);
+    igd_guest_opregion = (unsigned long)(val & ~XEN_PCI_INTEL_OPREGION_MASK)
+                            | (igd_host_opregion & XEN_PCI_INTEL_OPREGION_MASK);
+
+    ret = xc_domain_iomem_permission(xen_xc, xen_domid,
+            (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
+            XEN_PCI_INTEL_OPREGION_PAGES,
+            XEN_PCI_INTEL_OPREGION_ENABLE_ACCESSED);
+
+    if (ret) {
+        XEN_PT_ERR(&s->dev, "[%d]:Can't enable to access IGD host opregion:"
+                    " 0x%lx.\n", ret,
+                    (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT)),
+        igd_guest_opregion = 0;
+        return;
+    }
+
+    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),
+            XEN_PCI_INTEL_OPREGION_PAGES,
+            DPCI_ADD_MAPPING);
+
+    if (ret) {
+        XEN_PT_ERR(&s->dev, "[%d]:Can't map IGD host opregion:0x%lx to"
+                    " guest opregion:0x%lx.\n", ret,
+                    (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
+                    (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT));
+        igd_guest_opregion = 0;
+        return;
+    }
+
+    XEN_PT_LOG(&s->dev, "Map OpRegion: 0x%lx -> 0x%lx\n",
+                    (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
+                    (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT));
+}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough Tiejun Chen
@ 2015-03-18 10:21   ` Gerd Hoffmann
  2015-03-19  1:01     ` Chen, Tiejun
  2015-04-15  9:17   ` eraul_cn
  1 sibling, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2015-03-18 10:21 UTC (permalink / raw)
  To: Tiejun Chen; +Cc: pbonzini, rth, allen.m.kay, qemu-devel, mst

On Mi, 2015-03-18 at 17:06 +0800, Tiejun Chen wrote:
> Implement a pci host bridge specific to passthrough. Actually
> this just inherits the standard one. And we also just expose
> a minimal real host bridge pci configuration subset.

> +/* Here we just expose minimal host bridge offset subset. */
> +static const IGDHostInfo igd_host_bridge_infos[] = {
> +    {0x08, 2},  /* revision id */
> +    {0x2c, 2},  /* sybsystem vendor id */
> +    {0x2e, 2},  /* sybsystem id */
> +    {0x50, 2},  /* SNB: processor graphics control register */
> +    {0x52, 2},  /* processor graphics control register */
> +    {0xa4, 4},  /* SNB: graphics base of stolen memory */
> +    {0xa8, 4},  /* SNB: base of GTT stolen memory */
> +};

Hmm, no vendor/device id here?  Will the idg guest drivers happily read
graphics control registers from i440fx even though this chipset never
existed in a igd variant?

[ just wondering, if it works that way fine, it certainly makes things
  easier for the firmware which uses host bridge pci id to figure
  whenever it is running @ i440fx or q35 ].

> +static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
> +{
> +    uint32_t val = 0;
> +    int rc, i, num;
> +    int pos, len;
> +
> +    num = ARRAY_SIZE(igd_host_bridge_infos);
> +    for (i = 0; i < num; i++) {
> +        pos = igd_host_bridge_infos[i].offset;
> +        len = igd_host_bridge_infos[i].len;
> +        rc = host_pci_config_read(pos, len, val);
> +        if (rc) {
> +            return -ENODEV;
> +        }
> +        pci_default_write_config(pci_dev, pos, val, len);
> +    }
> +
> +    return 0;
> +}

Nothing i440fx specific in here, just copying some host bridge pci
config space bits.  I guess we can sub-class the q35 host bridge the
same way and even reuse the init function?

> +#define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"

One xen leftover slipped through here.

cheers,
  Gerd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough
  2015-03-18 10:21   ` Gerd Hoffmann
@ 2015-03-19  1:01     ` Chen, Tiejun
  2015-03-26  7:57       ` Chen, Tiejun
  2015-05-31 18:11       ` Michael S. Tsirkin
  0 siblings, 2 replies; 22+ messages in thread
From: Chen, Tiejun @ 2015-03-19  1:01 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: pbonzini, rth, allen.m.kay, qemu-devel, mst

On 2015/3/18 18:21, Gerd Hoffmann wrote:
> On Mi, 2015-03-18 at 17:06 +0800, Tiejun Chen wrote:
>> Implement a pci host bridge specific to passthrough. Actually
>> this just inherits the standard one. And we also just expose
>> a minimal real host bridge pci configuration subset.
>
>> +/* Here we just expose minimal host bridge offset subset. */
>> +static const IGDHostInfo igd_host_bridge_infos[] = {
>> +    {0x08, 2},  /* revision id */
>> +    {0x2c, 2},  /* sybsystem vendor id */
>> +    {0x2e, 2},  /* sybsystem id */
>> +    {0x50, 2},  /* SNB: processor graphics control register */
>> +    {0x52, 2},  /* processor graphics control register */
>> +    {0xa4, 4},  /* SNB: graphics base of stolen memory */
>> +    {0xa8, 4},  /* SNB: base of GTT stolen memory */
>> +};
>
> Hmm, no vendor/device id here?  Will the idg guest drivers happily read

These two emulated values are already fine.

And this is a long story. At the beginning, we were initially trying to 
expose more,

+    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 */

But this brought some discussions with Paolo and Michael, and then plus 
our further experiment, now as everyone expect, this is a minimal real 
host bridge pci configuration subset which we need to expose.

> graphics control registers from i440fx even though this chipset never
> existed in a igd variant?
>
> [ just wondering, if it works that way fine, it certainly makes things

Yes, it works fine.

>    easier for the firmware which uses host bridge pci id to figure
>    whenever it is running @ i440fx or q35 ].
>
>> +static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
>> +{
>> +    uint32_t val = 0;
>> +    int rc, i, num;
>> +    int pos, len;
>> +
>> +    num = ARRAY_SIZE(igd_host_bridge_infos);
>> +    for (i = 0; i < num; i++) {
>> +        pos = igd_host_bridge_infos[i].offset;
>> +        len = igd_host_bridge_infos[i].len;
>> +        rc = host_pci_config_read(pos, len, val);
>> +        if (rc) {
>> +            return -ENODEV;
>> +        }
>> +        pci_default_write_config(pci_dev, pos, val, len);
>> +    }
>> +
>> +    return 0;
>> +}
>
> Nothing i440fx specific in here, just copying some host bridge pci
> config space bits.  I guess we can sub-class the q35 host bridge the
> same way and even reuse the init function?

This is our another discussion long time ago :)

Currently Xen don't run with q35. If I remember those discussions 
correctly, something is still restricted to Windows.

>
>> +#define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
>
> One xen leftover slipped through here.

Fixed.

Thanks
Tiejun

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough
  2015-03-19  1:01     ` Chen, Tiejun
@ 2015-03-26  7:57       ` Chen, Tiejun
  2015-03-26  8:15         ` Michael S. Tsirkin
  2015-05-31 18:11       ` Michael S. Tsirkin
  1 sibling, 1 reply; 22+ messages in thread
From: Chen, Tiejun @ 2015-03-26  7:57 UTC (permalink / raw)
  To: Gerd Hoffmann, mst; +Cc: pbonzini, allen.m.kay, qemu-devel, rth

Michael and Gerd,

I don't see any more comments for this revision, so what's next that I 
should do?

Thanks
Tiejun

On 2015/3/19 9:01, Chen, Tiejun wrote:
> On 2015/3/18 18:21, Gerd Hoffmann wrote:
>> On Mi, 2015-03-18 at 17:06 +0800, Tiejun Chen wrote:
>>> Implement a pci host bridge specific to passthrough. Actually
>>> this just inherits the standard one. And we also just expose
>>> a minimal real host bridge pci configuration subset.
>>
>>> +/* Here we just expose minimal host bridge offset subset. */
>>> +static const IGDHostInfo igd_host_bridge_infos[] = {
>>> +    {0x08, 2},  /* revision id */
>>> +    {0x2c, 2},  /* sybsystem vendor id */
>>> +    {0x2e, 2},  /* sybsystem id */
>>> +    {0x50, 2},  /* SNB: processor graphics control register */
>>> +    {0x52, 2},  /* processor graphics control register */
>>> +    {0xa4, 4},  /* SNB: graphics base of stolen memory */
>>> +    {0xa8, 4},  /* SNB: base of GTT stolen memory */
>>> +};
>>
>> Hmm, no vendor/device id here?  Will the idg guest drivers happily read
>
> These two emulated values are already fine.
>
> And this is a long story. At the beginning, we were initially trying to
> expose more,
>
> +    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 */
>
> But this brought some discussions with Paolo and Michael, and then plus
> our further experiment, now as everyone expect, this is a minimal real
> host bridge pci configuration subset which we need to expose.
>
>> graphics control registers from i440fx even though this chipset never
>> existed in a igd variant?
>>
>> [ just wondering, if it works that way fine, it certainly makes things
>
> Yes, it works fine.
>
>>    easier for the firmware which uses host bridge pci id to figure
>>    whenever it is running @ i440fx or q35 ].
>>
>>> +static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
>>> +{
>>> +    uint32_t val = 0;
>>> +    int rc, i, num;
>>> +    int pos, len;
>>> +
>>> +    num = ARRAY_SIZE(igd_host_bridge_infos);
>>> +    for (i = 0; i < num; i++) {
>>> +        pos = igd_host_bridge_infos[i].offset;
>>> +        len = igd_host_bridge_infos[i].len;
>>> +        rc = host_pci_config_read(pos, len, val);
>>> +        if (rc) {
>>> +            return -ENODEV;
>>> +        }
>>> +        pci_default_write_config(pci_dev, pos, val, len);
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>
>> Nothing i440fx specific in here, just copying some host bridge pci
>> config space bits.  I guess we can sub-class the q35 host bridge the
>> same way and even reuse the init function?
>
> This is our another discussion long time ago :)
>
> Currently Xen don't run with q35. If I remember those discussions
> correctly, something is still restricted to Windows.
>
>>
>>> +#define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE
>>> "xen-igd-passthrough-i440FX"
>>
>> One xen leftover slipped through here.
>
> Fixed.
>
> Thanks
> Tiejun
>
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough
  2015-03-26  7:57       ` Chen, Tiejun
@ 2015-03-26  8:15         ` Michael S. Tsirkin
  2015-03-26  8:36           ` Chen, Tiejun
  2015-05-20  8:17           ` Chen, Tiejun
  0 siblings, 2 replies; 22+ messages in thread
From: Michael S. Tsirkin @ 2015-03-26  8:15 UTC (permalink / raw)
  To: Chen, Tiejun; +Cc: qemu-devel, pbonzini, allen.m.kay, Gerd Hoffmann, rth

On Thu, Mar 26, 2015 at 03:57:18PM +0800, Chen, Tiejun wrote:
> Michael and Gerd,
> 
> I don't see any more comments for this revision, so what's next that I
> should do?
> 
> Thanks
> Tiejun
> 
> On 2015/3/19 9:01, Chen, Tiejun wrote:

It's only been a week, and we are busy finalizing 2.3.  So please give
people time to review.

-- 
MST

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough
  2015-03-26  8:15         ` Michael S. Tsirkin
@ 2015-03-26  8:36           ` Chen, Tiejun
  2015-05-20  8:17           ` Chen, Tiejun
  1 sibling, 0 replies; 22+ messages in thread
From: Chen, Tiejun @ 2015-03-26  8:36 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, pbonzini, allen.m.kay, Gerd Hoffmann, rth

On 2015/3/26 16:15, Michael S. Tsirkin wrote:
> On Thu, Mar 26, 2015 at 03:57:18PM +0800, Chen, Tiejun wrote:
>> Michael and Gerd,
>>
>> I don't see any more comments for this revision, so what's next that I
>> should do?
>>
>> Thanks
>> Tiejun
>>
>> On 2015/3/19 9:01, Chen, Tiejun wrote:
>
> It's only been a week, and we are busy finalizing 2.3.  So please give

Oh, understood.

Thanks
Tiejun

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough
  2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough Tiejun Chen
  2015-03-18 10:21   ` Gerd Hoffmann
@ 2015-04-15  9:17   ` eraul_cn
  1 sibling, 0 replies; 22+ messages in thread
From: eraul_cn @ 2015-04-15  9:17 UTC (permalink / raw)
  To: qemu-devel

Chen, Tiejun wrote
> +/* Here we just expose minimal host bridge offset subset. */
> +static const IGDHostInfo igd_host_bridge_infos[] = {
> +    {0x08, 2},  /* revision id */
> +    {0x2c, 2},  /* sybsystem vendor id */
> +    {0x2e, 2},  /* sybsystem id */
> +    {0x50, 2},  /* SNB: processor graphics control register */
> +    {0x52, 2},  /* processor graphics control register */
> +    {0xa4, 4},  /* SNB: graphics base of stolen memory */
> +    {0xa8, 4},  /* SNB: base of GTT stolen memory */
> +};
> +

I tested this patch and found that the registers 0xa0(top of memory) and
0xb0(ILK: BSM) were necessary for Win XP. Both of them are 4 bytes.



Chen, Tiejun wrote
> +static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
> +{
> +    uint32_t val = 0;
> +    int rc, i, num;
> +    int pos, len;
> +
> +    num = ARRAY_SIZE(igd_host_bridge_infos);
> +    for (i = 0; i < num; i++) {
> +        pos = igd_host_bridge_infos[i].offset;
> +        len = igd_host_bridge_infos[i].len;
> +        rc = host_pci_config_read(pos, len, val);

Here, when we call function host_pci_config_read, the parameter val is
passed by value not address, so the value of val will not be changed after
call host_pci_config_read. So I think host_pci_config_read need update and
the third parameter should be an address.

Thanks



--
View this message in context: http://qemu.11.n7.nabble.com/v7-PATCH-00-10-xen-add-Intel-IGD-passthrough-support-tp319698p323854.html
Sent from the Developer mailing list archive at Nabble.com.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough
  2015-03-26  8:15         ` Michael S. Tsirkin
  2015-03-26  8:36           ` Chen, Tiejun
@ 2015-05-20  8:17           ` Chen, Tiejun
  1 sibling, 0 replies; 22+ messages in thread
From: Chen, Tiejun @ 2015-05-20  8:17 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, pbonzini, allen.m.kay, Gerd Hoffmann, rth

On 2015/3/26 16:15, Michael S. Tsirkin wrote:
> On Thu, Mar 26, 2015 at 03:57:18PM +0800, Chen, Tiejun wrote:
>> Michael and Gerd,
>>
>> I don't see any more comments for this revision, so what's next that I
>> should do?
>>
>> Thanks
>> Tiejun
>>
>> On 2015/3/19 9:01, Chen, Tiejun wrote:
>
> It's only been a week, and we are busy finalizing 2.3.  So please give
> people time to review.

Ping

Thanks
Tiejun

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough
  2015-03-19  1:01     ` Chen, Tiejun
  2015-03-26  7:57       ` Chen, Tiejun
@ 2015-05-31 18:11       ` Michael S. Tsirkin
  2015-06-02  0:50         ` Chen, Tiejun
  1 sibling, 1 reply; 22+ messages in thread
From: Michael S. Tsirkin @ 2015-05-31 18:11 UTC (permalink / raw)
  To: Chen, Tiejun; +Cc: qemu-devel, pbonzini, allen.m.kay, Gerd Hoffmann, rth

On Thu, Mar 19, 2015 at 09:01:27AM +0800, Chen, Tiejun wrote:
> On 2015/3/18 18:21, Gerd Hoffmann wrote:
> >On Mi, 2015-03-18 at 17:06 +0800, Tiejun Chen wrote:
> >>Implement a pci host bridge specific to passthrough. Actually
> >>this just inherits the standard one. And we also just expose
> >>a minimal real host bridge pci configuration subset.
> >
> >>+/* Here we just expose minimal host bridge offset subset. */
> >>+static const IGDHostInfo igd_host_bridge_infos[] = {
> >>+    {0x08, 2},  /* revision id */
> >>+    {0x2c, 2},  /* sybsystem vendor id */
> >>+    {0x2e, 2},  /* sybsystem id */
> >>+    {0x50, 2},  /* SNB: processor graphics control register */
> >>+    {0x52, 2},  /* processor graphics control register */
> >>+    {0xa4, 4},  /* SNB: graphics base of stolen memory */
> >>+    {0xa8, 4},  /* SNB: base of GTT stolen memory */
> >>+};
> >
> >Hmm, no vendor/device id here?  Will the idg guest drivers happily read
> 
> These two emulated values are already fine.
> 
> And this is a long story. At the beginning, we were initially trying to
> expose more,
> 
> +    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 */
> 
> But this brought some discussions with Paolo and Michael, and then plus our
> further experiment, now as everyone expect, this is a minimal real host
> bridge pci configuration subset which we need to expose.
> 
> >graphics control registers from i440fx even though this chipset never
> >existed in a igd variant?
> >
> >[ just wondering, if it works that way fine, it certainly makes things
> 
> Yes, it works fine.
> 
> >   easier for the firmware which uses host bridge pci id to figure
> >   whenever it is running @ i440fx or q35 ].
> >
> >>+static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
> >>+{
> >>+    uint32_t val = 0;
> >>+    int rc, i, num;
> >>+    int pos, len;
> >>+
> >>+    num = ARRAY_SIZE(igd_host_bridge_infos);
> >>+    for (i = 0; i < num; i++) {
> >>+        pos = igd_host_bridge_infos[i].offset;
> >>+        len = igd_host_bridge_infos[i].len;
> >>+        rc = host_pci_config_read(pos, len, val);
> >>+        if (rc) {
> >>+            return -ENODEV;
> >>+        }
> >>+        pci_default_write_config(pci_dev, pos, val, len);
> >>+    }
> >>+
> >>+    return 0;
> >>+}
> >
> >Nothing i440fx specific in here, just copying some host bridge pci
> >config space bits.  I guess we can sub-class the q35 host bridge the
> >same way and even reuse the init function?
> 
> This is our another discussion long time ago :)
> 
> Currently Xen don't run with q35. If I remember those discussions correctly,
> something is still restricted to Windows.
> 
> >
> >>+#define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
> >
> >One xen leftover slipped through here.
> 
> Fixed.

So should I expect v8 then?

> Thanks
> Tiejun

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough
  2015-05-31 18:11       ` Michael S. Tsirkin
@ 2015-06-02  0:50         ` Chen, Tiejun
  2015-06-02  9:17           ` Michael S. Tsirkin
  0 siblings, 1 reply; 22+ messages in thread
From: Chen, Tiejun @ 2015-06-02  0:50 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, pbonzini, allen.m.kay, Gerd Hoffmann, rth

On 2015/6/1 2:11, Michael S. Tsirkin wrote:
> On Thu, Mar 19, 2015 at 09:01:27AM +0800, Chen, Tiejun wrote:
>> On 2015/3/18 18:21, Gerd Hoffmann wrote:
>>> On Mi, 2015-03-18 at 17:06 +0800, Tiejun Chen wrote:
>>>> Implement a pci host bridge specific to passthrough. Actually
>>>> this just inherits the standard one. And we also just expose
>>>> a minimal real host bridge pci configuration subset.
>>>
>>>> +/* Here we just expose minimal host bridge offset subset. */
>>>> +static const IGDHostInfo igd_host_bridge_infos[] = {
>>>> +    {0x08, 2},  /* revision id */
>>>> +    {0x2c, 2},  /* sybsystem vendor id */
>>>> +    {0x2e, 2},  /* sybsystem id */
>>>> +    {0x50, 2},  /* SNB: processor graphics control register */
>>>> +    {0x52, 2},  /* processor graphics control register */
>>>> +    {0xa4, 4},  /* SNB: graphics base of stolen memory */
>>>> +    {0xa8, 4},  /* SNB: base of GTT stolen memory */
>>>> +};
>>>
>>> Hmm, no vendor/device id here?  Will the idg guest drivers happily read
>>
>> These two emulated values are already fine.
>>
>> And this is a long story. At the beginning, we were initially trying to
>> expose more,
>>
>> +    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 */
>>
>> But this brought some discussions with Paolo and Michael, and then plus our
>> further experiment, now as everyone expect, this is a minimal real host
>> bridge pci configuration subset which we need to expose.
>>
>>> graphics control registers from i440fx even though this chipset never
>>> existed in a igd variant?
>>>
>>> [ just wondering, if it works that way fine, it certainly makes things
>>
>> Yes, it works fine.
>>
>>>    easier for the firmware which uses host bridge pci id to figure
>>>    whenever it is running @ i440fx or q35 ].
>>>
>>>> +static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
>>>> +{
>>>> +    uint32_t val = 0;
>>>> +    int rc, i, num;
>>>> +    int pos, len;
>>>> +
>>>> +    num = ARRAY_SIZE(igd_host_bridge_infos);
>>>> +    for (i = 0; i < num; i++) {
>>>> +        pos = igd_host_bridge_infos[i].offset;
>>>> +        len = igd_host_bridge_infos[i].len;
>>>> +        rc = host_pci_config_read(pos, len, val);
>>>> +        if (rc) {
>>>> +            return -ENODEV;
>>>> +        }
>>>> +        pci_default_write_config(pci_dev, pos, val, len);
>>>> +    }
>>>> +
>>>> +    return 0;
>>>> +}
>>>
>>> Nothing i440fx specific in here, just copying some host bridge pci
>>> config space bits.  I guess we can sub-class the q35 host bridge the
>>> same way and even reuse the init function?
>>
>> This is our another discussion long time ago :)
>>
>> Currently Xen don't run with q35. If I remember those discussions correctly,
>> something is still restricted to Windows.
>>
>>>
>>>> +#define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
>>>
>>> One xen leftover slipped through here.
>>
>> Fixed.
>
> So should I expect v8 then?
>

For this revision we just had only this valuable comment, and that is 
just about to rename a macro, so I think its not worth resend this, right?

If I'm wrong let me know :)

Thanks
Tiejun

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough
  2015-06-02  0:50         ` Chen, Tiejun
@ 2015-06-02  9:17           ` Michael S. Tsirkin
  2015-06-05  8:36             ` Chen, Tiejun
  0 siblings, 1 reply; 22+ messages in thread
From: Michael S. Tsirkin @ 2015-06-02  9:17 UTC (permalink / raw)
  To: Chen, Tiejun; +Cc: qemu-devel, pbonzini, allen.m.kay, Gerd Hoffmann, rth

On Tue, Jun 02, 2015 at 08:50:58AM +0800, Chen, Tiejun wrote:
> On 2015/6/1 2:11, Michael S. Tsirkin wrote:
> >On Thu, Mar 19, 2015 at 09:01:27AM +0800, Chen, Tiejun wrote:
> >>On 2015/3/18 18:21, Gerd Hoffmann wrote:
> >>>On Mi, 2015-03-18 at 17:06 +0800, Tiejun Chen wrote:
> >>>>Implement a pci host bridge specific to passthrough. Actually
> >>>>this just inherits the standard one. And we also just expose
> >>>>a minimal real host bridge pci configuration subset.
> >>>
> >>>>+/* Here we just expose minimal host bridge offset subset. */
> >>>>+static const IGDHostInfo igd_host_bridge_infos[] = {
> >>>>+    {0x08, 2},  /* revision id */
> >>>>+    {0x2c, 2},  /* sybsystem vendor id */
> >>>>+    {0x2e, 2},  /* sybsystem id */
> >>>>+    {0x50, 2},  /* SNB: processor graphics control register */
> >>>>+    {0x52, 2},  /* processor graphics control register */
> >>>>+    {0xa4, 4},  /* SNB: graphics base of stolen memory */
> >>>>+    {0xa8, 4},  /* SNB: base of GTT stolen memory */
> >>>>+};
> >>>
> >>>Hmm, no vendor/device id here?  Will the idg guest drivers happily read
> >>
> >>These two emulated values are already fine.
> >>
> >>And this is a long story. At the beginning, we were initially trying to
> >>expose more,
> >>
> >>+    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 */
> >>
> >>But this brought some discussions with Paolo and Michael, and then plus our
> >>further experiment, now as everyone expect, this is a minimal real host
> >>bridge pci configuration subset which we need to expose.
> >>
> >>>graphics control registers from i440fx even though this chipset never
> >>>existed in a igd variant?
> >>>
> >>>[ just wondering, if it works that way fine, it certainly makes things
> >>
> >>Yes, it works fine.
> >>
> >>>   easier for the firmware which uses host bridge pci id to figure
> >>>   whenever it is running @ i440fx or q35 ].
> >>>
> >>>>+static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
> >>>>+{
> >>>>+    uint32_t val = 0;
> >>>>+    int rc, i, num;
> >>>>+    int pos, len;
> >>>>+
> >>>>+    num = ARRAY_SIZE(igd_host_bridge_infos);
> >>>>+    for (i = 0; i < num; i++) {
> >>>>+        pos = igd_host_bridge_infos[i].offset;
> >>>>+        len = igd_host_bridge_infos[i].len;
> >>>>+        rc = host_pci_config_read(pos, len, val);
> >>>>+        if (rc) {
> >>>>+            return -ENODEV;
> >>>>+        }
> >>>>+        pci_default_write_config(pci_dev, pos, val, len);
> >>>>+    }
> >>>>+
> >>>>+    return 0;
> >>>>+}
> >>>
> >>>Nothing i440fx specific in here, just copying some host bridge pci
> >>>config space bits.  I guess we can sub-class the q35 host bridge the
> >>>same way and even reuse the init function?
> >>
> >>This is our another discussion long time ago :)
> >>
> >>Currently Xen don't run with q35. If I remember those discussions correctly,
> >>something is still restricted to Windows.
> >>
> >>>
> >>>>+#define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
> >>>
> >>>One xen leftover slipped through here.
> >>
> >>Fixed.
> >
> >So should I expect v8 then?
> >
> 
> For this revision we just had only this valuable comment, and that is just
> about to rename a macro, so I think its not worth resend this, right?
> 
> If I'm wrong let me know :)
> 
> Thanks
> Tiejun

I didn't follow closely so I have no idea how valuable was the last
round of feedback, but when you say "Fixed" don't you mean "will be
fixed in the next revision of the patchset"?

-- 
MST

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough
  2015-06-02  9:17           ` Michael S. Tsirkin
@ 2015-06-05  8:36             ` Chen, Tiejun
  0 siblings, 0 replies; 22+ messages in thread
From: Chen, Tiejun @ 2015-06-05  8:36 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, pbonzini, allen.m.kay, Gerd Hoffmann, rth

On 2015/6/2 17:17, Michael S. Tsirkin wrote:
> On Tue, Jun 02, 2015 at 08:50:58AM +0800, Chen, Tiejun wrote:
>> On 2015/6/1 2:11, Michael S. Tsirkin wrote:
>>> On Thu, Mar 19, 2015 at 09:01:27AM +0800, Chen, Tiejun wrote:
>>>> On 2015/3/18 18:21, Gerd Hoffmann wrote:
>>>>> On Mi, 2015-03-18 at 17:06 +0800, Tiejun Chen wrote:
>>>>>> Implement a pci host bridge specific to passthrough. Actually
>>>>>> this just inherits the standard one. And we also just expose
>>>>>> a minimal real host bridge pci configuration subset.
>>>>>
>>>>>> +/* Here we just expose minimal host bridge offset subset. */
>>>>>> +static const IGDHostInfo igd_host_bridge_infos[] = {
>>>>>> +    {0x08, 2},  /* revision id */
>>>>>> +    {0x2c, 2},  /* sybsystem vendor id */
>>>>>> +    {0x2e, 2},  /* sybsystem id */
>>>>>> +    {0x50, 2},  /* SNB: processor graphics control register */
>>>>>> +    {0x52, 2},  /* processor graphics control register */
>>>>>> +    {0xa4, 4},  /* SNB: graphics base of stolen memory */
>>>>>> +    {0xa8, 4},  /* SNB: base of GTT stolen memory */
>>>>>> +};
>>>>>
>>>>> Hmm, no vendor/device id here?  Will the idg guest drivers happily read
>>>>
>>>> These two emulated values are already fine.
>>>>
>>>> And this is a long story. At the beginning, we were initially trying to
>>>> expose more,
>>>>
>>>> +    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 */
>>>>
>>>> But this brought some discussions with Paolo and Michael, and then plus our
>>>> further experiment, now as everyone expect, this is a minimal real host
>>>> bridge pci configuration subset which we need to expose.
>>>>
>>>>> graphics control registers from i440fx even though this chipset never
>>>>> existed in a igd variant?
>>>>>
>>>>> [ just wondering, if it works that way fine, it certainly makes things
>>>>
>>>> Yes, it works fine.
>>>>
>>>>>    easier for the firmware which uses host bridge pci id to figure
>>>>>    whenever it is running @ i440fx or q35 ].
>>>>>
>>>>>> +static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
>>>>>> +{
>>>>>> +    uint32_t val = 0;
>>>>>> +    int rc, i, num;
>>>>>> +    int pos, len;
>>>>>> +
>>>>>> +    num = ARRAY_SIZE(igd_host_bridge_infos);
>>>>>> +    for (i = 0; i < num; i++) {
>>>>>> +        pos = igd_host_bridge_infos[i].offset;
>>>>>> +        len = igd_host_bridge_infos[i].len;
>>>>>> +        rc = host_pci_config_read(pos, len, val);
>>>>>> +        if (rc) {
>>>>>> +            return -ENODEV;
>>>>>> +        }
>>>>>> +        pci_default_write_config(pci_dev, pos, val, len);
>>>>>> +    }
>>>>>> +
>>>>>> +    return 0;
>>>>>> +}
>>>>>
>>>>> Nothing i440fx specific in here, just copying some host bridge pci
>>>>> config space bits.  I guess we can sub-class the q35 host bridge the
>>>>> same way and even reuse the init function?
>>>>
>>>> This is our another discussion long time ago :)
>>>>
>>>> Currently Xen don't run with q35. If I remember those discussions correctly,
>>>> something is still restricted to Windows.
>>>>
>>>>>
>>>>>> +#define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
>>>>>
>>>>> One xen leftover slipped through here.
>>>>
>>>> Fixed.
>>>
>>> So should I expect v8 then?
>>>
>>
>> For this revision we just had only this valuable comment, and that is just
>> about to rename a macro, so I think its not worth resend this, right?
>>
>> If I'm wrong let me know :)
>>
>> Thanks
>> Tiejun
>
> I didn't follow closely so I have no idea how valuable was the last
> round of feedback, but when you say "Fixed" don't you mean "will be

It should be, but in this revision, I just received this sole comment 
until now so I mean its not worth resending a new review just with this 
fix :)

Anyway, its not big deal, just let me send this out as you expect.

Thanks
Tiejun

> fixed in the next revision of the patchset"?
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2015-06-05  8:36 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-18  9:06 [Qemu-devel] [v7][PATCH 00/10] xen: add Intel IGD passthrough support Tiejun Chen
2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 01/10] i440fx: make types configurable at run-time Tiejun Chen
2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 02/10] pc_init1: pass parameters just with types Tiejun Chen
2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough Tiejun Chen
2015-03-18 10:21   ` Gerd Hoffmann
2015-03-19  1:01     ` Chen, Tiejun
2015-03-26  7:57       ` Chen, Tiejun
2015-03-26  8:15         ` Michael S. Tsirkin
2015-03-26  8:36           ` Chen, Tiejun
2015-05-20  8:17           ` Chen, Tiejun
2015-05-31 18:11       ` Michael S. Tsirkin
2015-06-02  0:50         ` Chen, Tiejun
2015-06-02  9:17           ` Michael S. Tsirkin
2015-06-05  8:36             ` Chen, Tiejun
2015-04-15  9:17   ` eraul_cn
2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 04/10] hw/pci-assign: split pci-assign.c Tiejun Chen
2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 05/10] xen, gfx passthrough: basic graphics passthrough support Tiejun Chen
2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 06/10] xen, gfx passthrough: retrieve VGA BIOS to work Tiejun Chen
2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 07/10] igd gfx passthrough: create a isa bridge Tiejun Chen
2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 08/10] xen, gfx passthrough: register " Tiejun Chen
2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 09/10] xen, gfx passthrough: register host bridge specific to passthrough Tiejun Chen
2015-03-18  9:06 ` [Qemu-devel] [v7][PATCH 10/10] xen, gfx passthrough: add opregion mapping Tiejun Chen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.