[PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Kay, Allen M]

[-- Attachment #1.1: Type: text/plain, Size: 224 bytes --]

Added Calpella and Sandybridge integrated graphics pass-through support, consolidated graphics pass-through code into pt-graphics.c.


Signed-off-by: Allen Kay <allen.m.kay@intel.com<mailto:allen.m.kay@intel.com>>




[-- Attachment #1.2: Type: text/html, Size: 5096 bytes --]

[-- Attachment #2: igd-ioemu06032.patch --]
[-- Type: application/octet-stream, Size: 24082 bytes --]

From 268656a6d2fd90b5671d9b31cd5c1ee11a95d09b Mon Sep 17 00:00:00 2001
From: root <root@akay-cpt.(none)>
Date: Fri, 4 Jun 2010 09:17:47 -0700
Subject: [PATCH 1/1] graphics passthrough.

Signed-off-by: root <root@akay-cpt.(none)>
---
 hw/pass-through.c |  228 +++++++++------------------------------------
 hw/pass-through.h |   14 ++-
 hw/pc.c           |    6 +
 hw/pci.c          |   53 ++---------
 hw/pci.h          |   32 ++++++-
 hw/pt-graphics.c  |  266 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 xen-hooks.mak     |    2 +-
 7 files changed, 365 insertions(+), 236 deletions(-)
 create mode 100644 hw/pt-graphics.c

diff --git a/hw/pass-through.c b/hw/pass-through.c
index 5a76e8d..971c7f1 100644
--- a/hw/pass-through.c
+++ b/hw/pass-through.c
@@ -1865,50 +1865,6 @@ static int pt_dev_is_virtfn(struct pci_dev *dev)
     return rc;
 }
 
-/*
- * register VGA resources for the domain with assigned gfx
- */
-static int register_vga_regions(struct pt_dev *real_device)
-{
-    int ret = 0;
-
-    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3B0,
-            0x3B0, 0xC, DPCI_ADD_MAPPING);
-
-    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3C0,
-            0x3C0, 0x20, DPCI_ADD_MAPPING);
-
-    ret |= xc_domain_memory_mapping(xc_handle, domid,
-            0xa0000 >> XC_PAGE_SHIFT,
-            0xa0000 >> XC_PAGE_SHIFT,
-            0x20,
-            DPCI_ADD_MAPPING);
-
-    return ret;
-}
-
-/*
- * unregister VGA resources for the domain with assigned gfx
- */
-static int unregister_vga_regions(struct pt_dev *real_device)
-{
-    int ret = 0;
-
-    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3B0,
-            0x3B0, 0xC, DPCI_REMOVE_MAPPING);
-
-    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3C0,
-            0x3C0, 0x20, DPCI_REMOVE_MAPPING);
-
-    ret |= xc_domain_memory_mapping(xc_handle, domid,
-            0xa0000 >> XC_PAGE_SHIFT,
-            0xa0000 >> XC_PAGE_SHIFT,
-            0x20,
-            DPCI_REMOVE_MAPPING);
-
-    return ret;
-}
-
 static int pt_register_regions(struct pt_dev *assigned_device)
 {
     int i = 0;
@@ -1970,17 +1926,7 @@ static int pt_register_regions(struct pt_dev *assigned_device)
         PT_LOG("Expansion ROM registered (size=0x%08x base_addr=0x%08x)\n",
             (uint32_t)(pci_dev->rom_size), (uint32_t)(pci_dev->rom_base_addr));
     }
-
-    if ( gfx_passthru && (pci_dev->device_class == 0x0300) )
-    {
-        ret = register_vga_regions(assigned_device);
-        if ( ret != 0 )
-        {
-            PT_LOG("VGA region mapping failed\n");
-            return ret;
-        }
-    }
-
+    register_vga_regions(assigned_device);
     return 0;
 }
 
@@ -2029,13 +1975,7 @@ static void pt_unregister_regions(struct pt_dev *assigned_device)
         }
 
     }
-
-    if ( gfx_passthru && (assigned_device->pci_dev->device_class == 0x0300) )
-    {
-        ret = unregister_vga_regions(assigned_device);
-        if ( ret != 0 )
-            PT_LOG("VGA region unmapping failed\n");
-    }
+    unregister_vga_regions(assigned_device);
 }
 
 static uint8_t find_cap_offset(struct pci_dev *pci_dev, uint8_t cap)
@@ -2097,46 +2037,66 @@ static uint32_t find_ext_cap_offset(struct pci_dev *pci_dev, uint32_t cap)
     return 0;
 }
 
-u8 pt_pci_host_read_byte(int bus, int dev, int fn, u32 addr)
+static void pci_access_init(void)
 {
-    struct pci_dev *pci_dev;
-    u8 val;
+    struct pci_access *pci_access;
 
-    pci_dev = pci_get_dev(dpci_infos.pci_access, 0, bus, dev, fn);
-    if ( !pci_dev )
-        return 0;
+    if (dpci_infos.pci_access)
+        return;
 
-    val = pci_read_byte(pci_dev, addr);
-    pci_free_dev(pci_dev);
-    return val;
+    /* Initialize libpci */
+    pci_access = pci_alloc();
+    if ( pci_access == NULL ) {
+        PT_LOG("Error: pci_access is NULL\n");
+        return -1;
+    }
+    pci_init(pci_access);
+    pci_scan_bus(pci_access);
+    dpci_infos.pci_access = pci_access;
 }
 
-u16 pt_pci_host_read_word(int bus, int dev, int fn, u32 addr)
+u32 pt_pci_host_read(int bus, int dev, int fn, u32 addr, int len)
 {
+
     struct pci_dev *pci_dev;
-    u16 val;
+    u32 val = -1;
 
+    pci_access_init();
     pci_dev = pci_get_dev(dpci_infos.pci_access, 0, bus, dev, fn);
     if ( !pci_dev )
         return 0;
 
-    val = pci_read_word(pci_dev, addr);
-    pci_free_dev(pci_dev);
+    switch (len)
+    {
+        case 1: val = pci_read_byte(pci_dev, addr); break;
+        case 2: val = pci_read_word(pci_dev, addr); break;
+        case 4: val = pci_read_long(pci_dev, addr); break;
+        default:
+            fprintf(stderr, "error: pt_pci_host_read: invalid len = %d\n", len);
+    }
     return val;
 }
 
-u32 pt_pci_host_read_long(int bus, int dev, int fn, u32 addr)
+int pt_pci_host_write(int bus, int dev, int fn, u32 addr, u32 val, int len)
 {
     struct pci_dev *pci_dev;
-    u32 val;
+    int ret = 0;
 
+    pci_access_init();
     pci_dev = pci_get_dev(dpci_infos.pci_access, 0, bus, dev, fn);
     if ( !pci_dev )
         return 0;
 
-    val = pci_read_long(pci_dev, addr);
+    switch (len)
+    {
+        case 1: ret = pci_write_byte(pci_dev, addr, val); break;
+        case 2: ret = pci_write_word(pci_dev, addr, val); break;
+        case 4: ret = pci_write_long(pci_dev, addr, val); break;
+        default:
+            fprintf(stderr, "error: pt_pci_host_write: invalid len = %d\n", len);
+    }
     pci_free_dev(pci_dev);
-    return val;
+    return ret;
 }
 
 /* parse BAR */
@@ -4200,92 +4160,6 @@ static int pt_pmcsr_reg_restore(struct pt_dev *ptdev,
     return 0;
 }
 
-static int get_vgabios(unsigned char *buf)
-{
-    int fd;
-    uint32_t bios_size = 0;
-    uint32_t start = 0xC0000;
-    uint16_t magic = 0;
-
-    if ( (fd = open("/dev/mem", O_RDONLY)) < 0 )
-    {
-        PT_LOG("Error: Can't open /dev/mem: %s\n", strerror(errno));
-        return 0;
-    }
-
-    /*
-     * Check if it a real bios extension.
-     * The magic number is 0xAA55.
-     */
-    if ( start != lseek(fd, start, SEEK_SET) )
-        goto out;
-    if ( read(fd, &magic, 2) != 2 )
-        goto out;
-    if ( magic != 0xAA55 )
-        goto out;
-
-    /* Find the size of the rom extension */
-    if ( start != lseek(fd, start, SEEK_SET) )
-        goto out;
-    if ( lseek(fd, 2, SEEK_CUR) != (start + 2) )
-        goto out;
-    if ( read(fd, &bios_size, 1) != 1 )
-        goto out;
-
-    /* This size is in 512 bytes */
-    bios_size *= 512;
-
-    /*
-     * Set the file to the begining of the rombios,
-     * to start the copy.
-     */
-    if ( start != lseek(fd, start, SEEK_SET) )
-        goto out;
-
-    if ( bios_size != read(fd, buf, bios_size))
-        bios_size = 0;
-
-out:
-    close(fd);
-    return bios_size;
-}
-
-static int setup_vga_pt(void)
-{
-    unsigned char *bios = NULL;
-    int bios_size = 0;
-    char *c = NULL;
-    char checksum = 0;
-    int rc = 0;
-
-    /* Allocated 64K for the vga bios */
-    if ( !(bios = malloc(64 * 1024)) )
-        return -1;
-
-    bios_size = get_vgabios(bios);
-    if ( bios_size == 0 || bios_size > 64 * 1024)
-    {
-        PT_LOG("vga bios size (0x%x) is invalid!\n", bios_size);
-        rc = -1;
-        goto out;
-    }
-
-    /* Adjust the bios checksum */
-    for ( c = (char*)bios; c < ((char*)bios + bios_size); c++ )
-        checksum += *c;
-    if ( checksum )
-    {
-        bios[bios_size - 1] -= checksum;
-        PT_LOG("vga bios checksum is adjusted!\n");
-    }
-
-    cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
-
-out:
-    free(bios);
-    return rc;
-}
-
 static struct pt_dev * register_real_device(PCIBus *e_bus,
         const char *e_dev_name, int e_devfn, uint8_t r_bus, uint8_t r_dev,
         uint8_t r_func, uint32_t machine_irq, struct pci_access *pci_access,
@@ -4387,16 +4261,13 @@ static struct pt_dev * register_real_device(PCIBus *e_bus,
     pt_register_regions(assigned_device);
 
     /* Setup VGA bios for passthroughed gfx */
-    if ( gfx_passthru && (assigned_device->pci_dev->device_class == 0x0300) )
+    if ( setup_vga_pt(assigned_device) < 0 )
     {
-        rc = setup_vga_pt();
-        if ( rc < 0 )
-        {
-            PT_LOG("Setup VGA BIOS of passthroughed gfx failed!\n");
-            return NULL;
-        }
+        PT_LOG("Setup VGA BIOS of passthroughed gfx failed!\n");
+        return NULL;
     }
 
+
     /* reinitialize each config register to be emulated */
     rc = pt_config_init(assigned_device);
     if ( rc < 0 ) {
@@ -4548,19 +4419,8 @@ int power_on_php_devfn(int devfn)
 {
     struct php_dev *php_dev = &dpci_infos.php_devs[devfn];
     struct pt_dev *pt_dev;
-    struct pci_access *pci_access;
 
-    if (!dpci_infos.pci_access) {
-        /* Initialize libpci */
-        pci_access = pci_alloc();
-        if ( pci_access == NULL ) {
-            PT_LOG("Error: pci_access is NULL\n");
-            return -1;
-        }
-        pci_init(pci_access);
-        pci_scan_bus(pci_access);
-        dpci_infos.pci_access = pci_access;
-    }
+    pci_access_init();
 
     pt_dev =
         register_real_device(dpci_infos.e_bus,
diff --git a/hw/pass-through.h b/hw/pass-through.h
index f8a0c73..6c5e8ca 100644
--- a/hw/pass-through.h
+++ b/hw/pass-through.h
@@ -406,10 +406,16 @@ static inline pciaddr_t pt_pci_base_addr(pciaddr_t base)
 }
 
 uint8_t pci_intx(struct pt_dev *ptdev);
-
-u8 pt_pci_host_read_byte(int bus, int dev, int fn, u32 addr);
-u16 pt_pci_host_read_word(int bus, int dev, int fn, u32 addr);
-u32 pt_pci_host_read_long(int bus, int dev, int fn, u32 addr);
+u32 pt_pci_host_read(int bus, int dev, int fn, u32 addr, int len);
+int pt_pci_host_write(int bus, int dev, int fn, u32 addr, u32 val, int len);
+PCIBus *intel_pch_init(PCIBus *bus);
+int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len);
+int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len);
+int register_vga_regions(struct pt_dev *real_device);
+int unregister_vga_regions(struct pt_dev *real_device);
+int setup_vga_pt(struct pt_dev *real_device);
+PCIBus *intel_pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid,
+           uint16_t did, const char *name, uint16_t revision);
 
 #endif /* __PASSTHROUGH_H__ */
 
diff --git a/hw/pc.c b/hw/pc.c
index 9375951..7364cb8 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -42,6 +42,10 @@
 #include "virtio-console.h"
 #include "hpet_emul.h"
 
+#ifdef CONFIG_PASSTHROUGH
+#include "pass-through.h"
+#endif
+
 /* output Bochs bios info messages */
 //#define DEBUG_BIOS
 
@@ -978,6 +982,8 @@ vga_bios_error:
         pci_bus = NULL;
     }
 
+    intel_pch_init(pci_bus);
+
     /* init basic PC hardware */
     register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
 
diff --git a/hw/pci.c b/hw/pci.c
index b07e5ea..fa32ed9 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -39,24 +39,6 @@ extern int igd_passthru;
 
 //#define DEBUG_PCI
 
-struct PCIBus {
-    int bus_num;
-    int devfn_min;
-    pci_set_irq_fn set_irq;
-    pci_map_irq_fn map_irq;
-    uint32_t config_reg; /* XXX: suppress */
-    /* low level pic */
-    SetIRQFunc *low_set_irq;
-    qemu_irq *irq_opaque;
-    PCIDevice *devices[256];
-    PCIDevice *parent_dev;
-    PCIBus *next;
-    /* The bus IRQ state is the logical OR of the connected devices.
-       Keep a count of the number of devices with raised IRQs.  */
-    int nirq;
-    int irq_count[];
-};
-
 static void pci_update_mappings(PCIDevice *d);
 static void pci_set_irq(void *opaque, int irq_num, int level);
 
@@ -96,7 +78,7 @@ PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
     return bus;
 }
 
-static PCIBus *pci_register_secondary_bus(PCIDevice *dev, pci_map_irq_fn map_irq)
+PCIBus *pci_register_secondary_bus(PCIDevice *dev, pci_map_irq_fn map_irq)
 {
     PCIBus *bus;
     bus = qemu_mallocz(sizeof(PCIBus));
@@ -590,6 +572,10 @@ void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len)
     printf("pci_config_write: %s: addr=%02x val=%08x len=%d\n",
            pci_dev->name, config_addr, val, len);
 #endif
+
+#ifdef CONFIG_PASSTHROUGH
+    if (igd_pci_write(pci_dev, config_addr, val, len) == 0)
+#endif
     pci_dev->config_write(pci_dev, config_addr, val, len);
 }
 
@@ -598,7 +584,7 @@ uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
     PCIBus *s = opaque;
     PCIDevice *pci_dev;
     int config_addr, bus_num;
-    uint32_t val;
+    uint32_t val = 0;
 
     bus_num = (addr >> 16) & 0xff;
     while (s && s->bus_num != bus_num)
@@ -625,26 +611,8 @@ uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
     config_addr = addr & 0xff;
 
 #ifdef CONFIG_PASSTHROUGH
-    /* host bridge reads for IGD passthrough */
-    if ( igd_passthru && pci_dev->devfn == 0x00 ) {
-        val = pci_dev->config_read(pci_dev, config_addr, len);
-
-        if ( config_addr == 0x00 && len == 4 )
-            val = pt_pci_host_read_long(0, 0, 0, 0x00);
-        else if ( config_addr == 0x02 ) // Device ID
-            val = pt_pci_host_read_word(0, 0, 0, 0x02);
-        else if ( config_addr == 0x52 ) // GMCH Graphics Control Register
-            val = pt_pci_host_read_word(0, 0, 0, 0x52);
-        else if ( config_addr == 0xa0 ) // GMCH Top of Memory Register
-            val = pt_pci_host_read_word(0, 0, 0, 0xa0);
-        goto done_config_read;
-    } else if ( igd_passthru && pci_dev->devfn == 0x10 &&
-              config_addr == 0xfc ) { // read on IGD device
-        val = 0;  // use SMI to communicate with the system BIOS
-        goto done_config_read;
-    }
+    if ( igd_pci_read(pci_dev, config_addr, &val, len) == 0)
 #endif
-
     val = pci_dev->config_read(pci_dev, config_addr, len);
 
  done_config_read:
@@ -892,12 +860,7 @@ void pci_unplug_netifs(void)
     }
 }
 
-typedef struct {
-    PCIDevice dev;
-    PCIBus *bus;
-} PCIBridge;
-
-static void pci_bridge_write_config(PCIDevice *d,
+void pci_bridge_write_config(PCIDevice *d,
                              uint32_t address, uint32_t val, int len)
 {
     PCIBridge *s = (PCIBridge *)d;
diff --git a/hw/pci.h b/hw/pci.h
index de5a4e1..18e7b6f 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -207,6 +207,32 @@ struct PCIDevice {
     int irq_state[4];
 };
 
+typedef void (*pci_set_irq_fn)(qemu_irq *pic, int irq_num, int level);
+typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
+
+struct PCIBus {
+    int bus_num;
+    int devfn_min;
+    pci_set_irq_fn set_irq;
+    pci_map_irq_fn map_irq;
+    uint32_t config_reg; /* XXX: suppress */
+    /* low level pic */
+    SetIRQFunc *low_set_irq;
+    qemu_irq *irq_opaque;
+    PCIDevice *devices[256];
+    PCIDevice *parent_dev;
+    PCIBus *next;
+    /* The bus IRQ state is the logical OR of the connected devices.
+       Keep a count of the number of devices with raised IRQs.  */
+    int nirq;
+    int irq_count[];
+};
+
+typedef struct {
+    PCIDevice dev;
+    PCIBus *bus;
+} PCIBridge;
+
 extern char direct_pci_str[];
 extern int direct_pci_msitranslate;
 extern int direct_pci_power_mgmt;
@@ -235,8 +261,6 @@ void pci_default_write_config(PCIDevice *d,
 void pci_device_save(PCIDevice *s, QEMUFile *f);
 int pci_device_load(PCIDevice *s, QEMUFile *f);
 
-typedef void (*pci_set_irq_fn)(qemu_irq *pic, int irq_num, int level);
-typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
 PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                          qemu_irq *pic, int devfn_min, int nirq);
 
@@ -341,5 +365,9 @@ PCIBus *sh_pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
 
 /* pass-through.c */
 int pt_init(PCIBus *e_bus);
+void pci_bridge_write_config(PCIDevice *d,
+                             uint32_t address, uint32_t val, int len);
+PCIBus *pci_register_secondary_bus(PCIDevice *dev, pci_map_irq_fn map_irq);
+
 
 #endif
diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c
new file mode 100644
index 0000000..3b9ce95
--- /dev/null
+++ b/hw/pt-graphics.c
@@ -0,0 +1,266 @@
+/*
+ * graphics passthrough
+ */
+
+#include "pass-through.h"
+#include "pci/header.h"
+#include "pci/pci.h"
+#include "pt-msi.h"
+#include "qemu-xen.h"
+#include "iomulti.h"
+
+#include <unistd.h>
+#include <sys/ioctl.h>
+
+extern int gfx_passthru;
+extern int igd_passthru;
+
+static int pch_irq_function(PCIDevice *pci_dev, int irq_num)
+{
+    PT_LOG("pch_irq_function called\n");
+    return irq_num;
+}
+
+PCIBus *intel_pch_init(PCIBus *bus)
+{
+    PCIBridge *pch;
+    u16 vendor_id, device_id;
+    u8  rev_id;
+
+    if ( !gfx_passthru )
+        return NULL;
+
+    vendor_id = pt_pci_host_read(0, 0x1f, 0, 0, 2);
+    device_id = pt_pci_host_read(0, 0x1f, 0, 2, 2);
+    rev_id    = pt_pci_host_read(0, 0x1f, 0, 8, 1);
+
+    pch = (PCIBridge *)
+          pci_register_device(bus, "intel_bridge_1f", sizeof(PCIBridge),
+                     PCI_DEVFN(0x1f, 0), NULL, pci_bridge_write_config);
+
+    pci_config_set_vendor_id(pch->dev.config, vendor_id);
+    pci_config_set_device_id(pch->dev.config, device_id);
+
+    pch->dev.config[0x04] = 0x06; // command = bus master, pci mem
+    pch->dev.config[0x05] = 0x00;
+    pch->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
+    pch->dev.config[0x07] = 0x00; // status = fast devsel
+    pch->dev.config[0x08] = rev_id; 
+    pch->dev.config[0x09] = 0x00; // programming i/f
+    pci_config_set_class(pch->dev.config, PCI_CLASS_BRIDGE_ISA);
+    pch->dev.config[0x0D] = 0x10; // latency_timer
+    pch->dev.config[0x0E] = 0x81; // header_type
+    pch->dev.config[0x1E] = 0xa0; // secondary status
+
+    pch->bus = pci_register_secondary_bus(&pch->dev, pch_irq_function);
+    return pch->bus;
+}
+
+int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
+{
+    if ( !igd_passthru || (pci_dev->devfn != 0x00 ) )
+        return 0;
+
+    switch (config_addr)
+    {
+        case 0x58:        // PAVPC Offset
+            pt_pci_host_write(0, 0, 0, config_addr, val, len);
+            PT_LOG("pci_config_write: %x:%x.%x: addr=%x len=%x val=%x\n",
+                   pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn),
+                   PCI_FUNC(pci_dev->devfn), config_addr, len, val);
+            break;
+        default:
+            pci_dev->config_write(pci_dev, config_addr, val, len);
+    }
+    return 1;
+}
+
+int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len)
+{
+    if ( !igd_passthru || (pci_dev->devfn != 0) )
+        return 0;
+
+    switch (config_addr)
+    {
+        case 0x00:        /* vendor id */
+        case 0x02:        /* device id */
+        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 */
+            *val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
+                                   0, config_addr, len);
+            PT_LOG("pci_config_read: %x:%x.%x: addr=%x len=%x val=%x\n",
+                   pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn),
+                   PCI_FUNC(pci_dev->devfn), config_addr, len, *val);
+
+            break;
+        default:
+            *val = pci_dev->config_read(pci_dev, config_addr, len);
+    }
+    return 1;
+}
+
+/*
+ * register VGA resources for the domain with assigned gfx
+ */
+int register_vga_regions(struct pt_dev *real_device)
+{
+    u32 igd_opregion, igd_bsm;
+    int ret = 0;
+
+    if ( !gfx_passthru || real_device->pci_dev->device_class != 0x0300 )
+        return ret;
+
+    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3B0,
+            0x3B0, 0xC, DPCI_ADD_MAPPING);
+
+    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3C0,
+            0x3C0, 0x20, DPCI_ADD_MAPPING);
+
+    ret |= xc_domain_memory_mapping(xc_handle, domid,
+            0xa0000 >> XC_PAGE_SHIFT,
+            0xa0000 >> XC_PAGE_SHIFT,
+            0x20,
+            DPCI_ADD_MAPPING);
+
+    /* 1:1 map ASL Storage register value */
+    igd_opregion = pt_pci_host_read(0, 2, 0, 0xfc, 4);
+    PT_LOG("register_vga: igd_opregion = %x\n", igd_opregion);
+    ret |= xc_domain_memory_mapping(xc_handle, domid,
+            igd_opregion >> XC_PAGE_SHIFT,
+            igd_opregion >> XC_PAGE_SHIFT,
+            2,
+            DPCI_ADD_MAPPING);
+
+    if ( ret != 0 )
+        PT_LOG("VGA region mapping failed\n");
+
+    return ret;
+}
+
+/*
+ * unregister VGA resources for the domain with assigned gfx
+ */
+int unregister_vga_regions(struct pt_dev *real_device)
+{
+    u32 igd_opregion, igd_bsm;
+    int ret = 0;
+
+    if ( !gfx_passthru || real_device->pci_dev->device_class != 0x0300 )
+        return ret;
+
+    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3B0,
+            0x3B0, 0xC, DPCI_REMOVE_MAPPING);
+
+    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3C0,
+            0x3C0, 0x20, DPCI_REMOVE_MAPPING);
+
+    ret |= xc_domain_memory_mapping(xc_handle, domid,
+            0xa0000 >> XC_PAGE_SHIFT,
+            0xa0000 >> XC_PAGE_SHIFT,
+            20,
+            DPCI_REMOVE_MAPPING);
+
+    igd_opregion = pt_pci_host_read(0, 2, 0, 0xfc, 4);
+    ret |= xc_domain_memory_mapping(xc_handle, domid,
+            igd_opregion >> XC_PAGE_SHIFT,
+            igd_opregion >> XC_PAGE_SHIFT,
+            2,
+            DPCI_REMOVE_MAPPING);
+
+    if ( ret != 0 )
+        PT_LOG("VGA region unmapping failed\n");
+
+    return ret;
+}
+
+static int get_vgabios(unsigned char *buf)
+{
+    int fd;
+    uint32_t bios_size = 0;
+    uint32_t start = 0xC0000;
+    uint16_t magic = 0;
+
+    if ( (fd = open("/dev/mem", O_RDONLY)) < 0 )
+    {
+        PT_LOG("Error: Can't open /dev/mem: %s\n", strerror(errno));
+        return 0;
+    }
+
+    /*
+     * Check if it a real bios extension.
+     * The magic number is 0xAA55.
+     */
+    if ( start != lseek(fd, start, SEEK_SET) )
+        goto out;
+    if ( read(fd, &magic, 2) != 2 )
+        goto out;
+    if ( magic != 0xAA55 )
+        goto out;
+
+    /* Find the size of the rom extension */
+    if ( start != lseek(fd, start, SEEK_SET) )
+        goto out;
+    if ( lseek(fd, 2, SEEK_CUR) != (start + 2) )
+        goto out;
+    if ( read(fd, &bios_size, 1) != 1 )
+        goto out;
+
+    /* This size is in 512 bytes */
+    bios_size *= 512;
+
+    /*
+     * Set the file to the begining of the rombios,
+     * to start the copy.
+     */
+    if ( start != lseek(fd, start, SEEK_SET) )
+        goto out;
+
+    if ( bios_size != read(fd, buf, bios_size))
+        bios_size = 0;
+
+out:
+    close(fd);
+    return bios_size;
+}
+
+int setup_vga_pt(struct pt_dev *real_device)
+{
+    unsigned char *bios = NULL;
+    int bios_size = 0;
+    char *c = NULL;
+    char checksum = 0;
+    int rc = 0;
+
+    if ( !gfx_passthru || real_device->pci_dev->device_class != 0x0300 )
+        return rc;
+
+    /* Allocated 64K for the vga bios */
+    if ( !(bios = malloc(64 * 1024)) )
+        return -1;
+
+    bios_size = get_vgabios(bios);
+    if ( bios_size == 0 || bios_size > 64 * 1024)
+    {
+        PT_LOG("vga bios size (0x%x) is invalid!\n", bios_size);
+        rc = -1;
+        goto out;
+    }
+
+    /* Adjust the bios checksum */
+    for ( c = (char*)bios; c < ((char*)bios + bios_size); c++ )
+        checksum += *c;
+    if ( checksum )
+    {
+        bios[bios_size - 1] -= checksum;
+        PT_LOG("vga bios checksum is adjusted!\n");
+    }
+
+    cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
+out:
+    free(bios);
+    return rc;
+}
diff --git a/xen-hooks.mak b/xen-hooks.mak
index 211416e..93f4402 100644
--- a/xen-hooks.mak
+++ b/xen-hooks.mak
@@ -61,7 +61,7 @@ CONFIG_PASSTHROUGH=1
 endif
 
 ifdef CONFIG_PASSTHROUGH
-OBJS+= pass-through.o pt-msi.o
+OBJS+= pass-through.o pt-msi.o pt-graphics.o
 LIBS += -lpci
 CFLAGS += -DCONFIG_PASSTHROUGH 
 $(info === PCI passthrough capability has been enabled ===)
-- 
1.6.5.2


[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Stefano Stabellini]

On Fri, 4 Jun 2010, Kay, Allen M wrote:
> 
> Added Calpella and Sandybridge integrated graphics pass-through support, consolidated graphics pass-through code into
> pt-graphics.c.
> 
> Signed-off-by: Allen Kay <allen.m.kay@intel.com>
> 

some comments follow

> diff --git a/hw/pass-through.c b/hw/pass-through.c
> index 5a76e8d..971c7f1 100644
> --- a/hw/pass-through.c
> +++ b/hw/pass-through.c
> @@ -1865,50 +1865,6 @@ static int pt_dev_is_virtfn(struct pci_dev *dev)
>      return rc;
>  }
>  
> -/*
> - * register VGA resources for the domain with assigned gfx
> - */
> -static int register_vga_regions(struct pt_dev *real_device)
> -{
> -    int ret = 0;
> -
> -    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3B0,
> -            0x3B0, 0xC, DPCI_ADD_MAPPING);
> -
> -    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3C0,
> -            0x3C0, 0x20, DPCI_ADD_MAPPING);
> -
> -    ret |= xc_domain_memory_mapping(xc_handle, domid,
> -            0xa0000 >> XC_PAGE_SHIFT,
> -            0xa0000 >> XC_PAGE_SHIFT,
> -            0x20,
> -            DPCI_ADD_MAPPING);
> -
> -    return ret;
> -}
> -
> -/*
> - * unregister VGA resources for the domain with assigned gfx
> - */
> -static int unregister_vga_regions(struct pt_dev *real_device)
> -{
> -    int ret = 0;
> -
> -    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3B0,
> -            0x3B0, 0xC, DPCI_REMOVE_MAPPING);
> -
> -    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3C0,
> -            0x3C0, 0x20, DPCI_REMOVE_MAPPING);
> -
> -    ret |= xc_domain_memory_mapping(xc_handle, domid,
> -            0xa0000 >> XC_PAGE_SHIFT,
> -            0xa0000 >> XC_PAGE_SHIFT,
> -            0x20,
> -            DPCI_REMOVE_MAPPING);
> -
> -    return ret;
> -}
> -
>  static int pt_register_regions(struct pt_dev *assigned_device)
>  {
>      int i = 0;
> @@ -1970,17 +1926,7 @@ static int pt_register_regions(struct pt_dev *assigned_device)
>          PT_LOG("Expansion ROM registered (size=0x%08x base_addr=0x%08x)\n",
>              (uint32_t)(pci_dev->rom_size), (uint32_t)(pci_dev->rom_base_addr));
>      }
> -
> -    if ( gfx_passthru && (pci_dev->device_class == 0x0300) )
> -    {
> -        ret = register_vga_regions(assigned_device);
> -        if ( ret != 0 )
> -        {
> -            PT_LOG("VGA region mapping failed\n");
> -            return ret;
> -        }
> -    }
> -
> +    register_vga_regions(assigned_device);
>      return 0;
>  }
>  
> @@ -2029,13 +1975,7 @@ static void pt_unregister_regions(struct pt_dev *assigned_device)
>          }
>  
>      }
> -
> -    if ( gfx_passthru && (assigned_device->pci_dev->device_class == 0x0300) )
> -    {
> -        ret = unregister_vga_regions(assigned_device);
> -        if ( ret != 0 )
> -            PT_LOG("VGA region unmapping failed\n");
> -    }
> +    unregister_vga_regions(assigned_device);
>  }
>  
>  static uint8_t find_cap_offset(struct pci_dev *pci_dev, uint8_t cap)
> @@ -2097,46 +2037,66 @@ static uint32_t find_ext_cap_offset(struct pci_dev *pci_dev, uint32_t cap)
>      return 0;
>  }
>  
> -u8 pt_pci_host_read_byte(int bus, int dev, int fn, u32 addr)
> +static void pci_access_init(void)
>  {
> -    struct pci_dev *pci_dev;
> -    u8 val;
> +    struct pci_access *pci_access;
>  
> -    pci_dev = pci_get_dev(dpci_infos.pci_access, 0, bus, dev, fn);
> -    if ( !pci_dev )
> -        return 0;
> +    if (dpci_infos.pci_access)
> +        return;
>  
> -    val = pci_read_byte(pci_dev, addr);
> -    pci_free_dev(pci_dev);
> -    return val;
> +    /* Initialize libpci */
> +    pci_access = pci_alloc();
> +    if ( pci_access == NULL ) {
> +        PT_LOG("Error: pci_access is NULL\n");
> +        return -1;
> +    }
> +    pci_init(pci_access);
> +    pci_scan_bus(pci_access);
> +    dpci_infos.pci_access = pci_access;
>  }
>  
> -u16 pt_pci_host_read_word(int bus, int dev, int fn, u32 addr)
> +u32 pt_pci_host_read(int bus, int dev, int fn, u32 addr, int len)
>  {
> +
>      struct pci_dev *pci_dev;
> -    u16 val;
> +    u32 val = -1;
>  
> +    pci_access_init();
>      pci_dev = pci_get_dev(dpci_infos.pci_access, 0, bus, dev, fn);
>      if ( !pci_dev )
>          return 0;
>  
> -    val = pci_read_word(pci_dev, addr);
> -    pci_free_dev(pci_dev);
> +    switch (len)
> +    {
> +        case 1: val = pci_read_byte(pci_dev, addr); break;
> +        case 2: val = pci_read_word(pci_dev, addr); break;
> +        case 4: val = pci_read_long(pci_dev, addr); break;
> +        default:
> +            fprintf(stderr, "error: pt_pci_host_read: invalid len = %d\n", len);
> +    }
>      return val;
>  }
>  
> -u32 pt_pci_host_read_long(int bus, int dev, int fn, u32 addr)
> +int pt_pci_host_write(int bus, int dev, int fn, u32 addr, u32 val, int len)
>  {
>      struct pci_dev *pci_dev;
> -    u32 val;
> +    int ret = 0;
>  
> +    pci_access_init();
>      pci_dev = pci_get_dev(dpci_infos.pci_access, 0, bus, dev, fn);
>      if ( !pci_dev )
>          return 0;
>  
> -    val = pci_read_long(pci_dev, addr);
> +    switch (len)
> +    {
> +        case 1: ret = pci_write_byte(pci_dev, addr, val); break;
> +        case 2: ret = pci_write_word(pci_dev, addr, val); break;
> +        case 4: ret = pci_write_long(pci_dev, addr, val); break;
> +        default:
> +            fprintf(stderr, "error: pt_pci_host_write: invalid len = %d\n", len);
> +    }
>      pci_free_dev(pci_dev);
> -    return val;
> +    return ret;
>  }
>  
>  /* parse BAR */
> @@ -4200,92 +4160,6 @@ static int pt_pmcsr_reg_restore(struct pt_dev *ptdev,
>      return 0;
>  }
>  
> -static int get_vgabios(unsigned char *buf)
> -{
> -    int fd;
> -    uint32_t bios_size = 0;
> -    uint32_t start = 0xC0000;
> -    uint16_t magic = 0;
> -
> -    if ( (fd = open("/dev/mem", O_RDONLY)) < 0 )
> -    {
> -        PT_LOG("Error: Can't open /dev/mem: %s\n", strerror(errno));
> -        return 0;
> -    }
> -
> -    /*
> -     * Check if it a real bios extension.
> -     * The magic number is 0xAA55.
> -     */
> -    if ( start != lseek(fd, start, SEEK_SET) )
> -        goto out;
> -    if ( read(fd, &magic, 2) != 2 )
> -        goto out;
> -    if ( magic != 0xAA55 )
> -        goto out;
> -
> -    /* Find the size of the rom extension */
> -    if ( start != lseek(fd, start, SEEK_SET) )
> -        goto out;
> -    if ( lseek(fd, 2, SEEK_CUR) != (start + 2) )
> -        goto out;
> -    if ( read(fd, &bios_size, 1) != 1 )
> -        goto out;
> -
> -    /* This size is in 512 bytes */
> -    bios_size *= 512;
> -
> -    /*
> -     * Set the file to the begining of the rombios,
> -     * to start the copy.
> -     */
> -    if ( start != lseek(fd, start, SEEK_SET) )
> -        goto out;
> -
> -    if ( bios_size != read(fd, buf, bios_size))
> -        bios_size = 0;
> -
> -out:
> -    close(fd);
> -    return bios_size;
> -}
> -
> -static int setup_vga_pt(void)
> -{
> -    unsigned char *bios = NULL;
> -    int bios_size = 0;
> -    char *c = NULL;
> -    char checksum = 0;
> -    int rc = 0;
> -
> -    /* Allocated 64K for the vga bios */
> -    if ( !(bios = malloc(64 * 1024)) )
> -        return -1;
> -
> -    bios_size = get_vgabios(bios);
> -    if ( bios_size == 0 || bios_size > 64 * 1024)
> -    {
> -        PT_LOG("vga bios size (0x%x) is invalid!\n", bios_size);
> -        rc = -1;
> -        goto out;
> -    }
> -
> -    /* Adjust the bios checksum */
> -    for ( c = (char*)bios; c < ((char*)bios + bios_size); c++ )
> -        checksum += *c;
> -    if ( checksum )
> -    {
> -        bios[bios_size - 1] -= checksum;
> -        PT_LOG("vga bios checksum is adjusted!\n");
> -    }
> -
> -    cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
> -
> -out:
> -    free(bios);
> -    return rc;
> -}
> -
>  static struct pt_dev * register_real_device(PCIBus *e_bus,
>          const char *e_dev_name, int e_devfn, uint8_t r_bus, uint8_t r_dev,
>          uint8_t r_func, uint32_t machine_irq, struct pci_access *pci_access,
> @@ -4387,16 +4261,13 @@ static struct pt_dev * register_real_device(PCIBus *e_bus,
>      pt_register_regions(assigned_device);
>  
>      /* Setup VGA bios for passthroughed gfx */
> -    if ( gfx_passthru && (assigned_device->pci_dev->device_class == 0x0300) )
> +    if ( setup_vga_pt(assigned_device) < 0 )
>      {
> -        rc = setup_vga_pt();
> -        if ( rc < 0 )
> -        {
> -            PT_LOG("Setup VGA BIOS of passthroughed gfx failed!\n");
> -            return NULL;
> -        }
> +        PT_LOG("Setup VGA BIOS of passthroughed gfx failed!\n");
> +        return NULL;
>      }
>  
> +
>      /* reinitialize each config register to be emulated */
>      rc = pt_config_init(assigned_device);
>      if ( rc < 0 ) {
> @@ -4548,19 +4419,8 @@ int power_on_php_devfn(int devfn)
>  {
>      struct php_dev *php_dev = &dpci_infos.php_devs[devfn];
>      struct pt_dev *pt_dev;
> -    struct pci_access *pci_access;
>  
> -    if (!dpci_infos.pci_access) {
> -        /* Initialize libpci */
> -        pci_access = pci_alloc();
> -        if ( pci_access == NULL ) {
> -            PT_LOG("Error: pci_access is NULL\n");
> -            return -1;
> -        }
> -        pci_init(pci_access);
> -        pci_scan_bus(pci_access);
> -        dpci_infos.pci_access = pci_access;
> -    }
> +    pci_access_init();
>  
>      pt_dev =
>          register_real_device(dpci_infos.e_bus,
> diff --git a/hw/pass-through.h b/hw/pass-through.h
> index f8a0c73..6c5e8ca 100644
> --- a/hw/pass-through.h
> +++ b/hw/pass-through.h
> @@ -406,10 +406,16 @@ static inline pciaddr_t pt_pci_base_addr(pciaddr_t base)
>  }
>  
>  uint8_t pci_intx(struct pt_dev *ptdev);
> -
> -u8 pt_pci_host_read_byte(int bus, int dev, int fn, u32 addr);
> -u16 pt_pci_host_read_word(int bus, int dev, int fn, u32 addr);
> -u32 pt_pci_host_read_long(int bus, int dev, int fn, u32 addr);
> +u32 pt_pci_host_read(int bus, int dev, int fn, u32 addr, int len);
> +int pt_pci_host_write(int bus, int dev, int fn, u32 addr, u32 val, int len);
> +PCIBus *intel_pch_init(PCIBus *bus);
> +int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len);
> +int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len);
> +int register_vga_regions(struct pt_dev *real_device);
> +int unregister_vga_regions(struct pt_dev *real_device);
> +int setup_vga_pt(struct pt_dev *real_device);
> +PCIBus *intel_pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid,
> +           uint16_t did, const char *name, uint16_t revision);
>  
>  #endif /* __PASSTHROUGH_H__ */
>  
> diff --git a/hw/pc.c b/hw/pc.c
> index 9375951..7364cb8 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -42,6 +42,10 @@
>  #include "virtio-console.h"
>  #include "hpet_emul.h"
>  
> +#ifdef CONFIG_PASSTHROUGH
> +#include "pass-through.h"
> +#endif
> +
>  /* output Bochs bios info messages */
>  //#define DEBUG_BIOS
>  
> @@ -978,6 +982,8 @@ vga_bios_error:
>          pci_bus = NULL;
>      }
>  
> +    intel_pch_init(pci_bus);
> +
>      /* init basic PC hardware */
>      register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
>  

You are missing a ifdef CONFIG_PASSTHROUGH here


> diff --git a/hw/pci.c b/hw/pci.c
> index b07e5ea..fa32ed9 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -39,24 +39,6 @@ extern int igd_passthru;
>  
>  //#define DEBUG_PCI
>  
> -struct PCIBus {
> -    int bus_num;
> -    int devfn_min;
> -    pci_set_irq_fn set_irq;
> -    pci_map_irq_fn map_irq;
> -    uint32_t config_reg; /* XXX: suppress */
> -    /* low level pic */
> -    SetIRQFunc *low_set_irq;
> -    qemu_irq *irq_opaque;
> -    PCIDevice *devices[256];
> -    PCIDevice *parent_dev;
> -    PCIBus *next;
> -    /* The bus IRQ state is the logical OR of the connected devices.
> -       Keep a count of the number of devices with raised IRQs.  */
> -    int nirq;
> -    int irq_count[];
> -};
> -
>  static void pci_update_mappings(PCIDevice *d);
>  static void pci_set_irq(void *opaque, int irq_num, int level);
>  
> @@ -96,7 +78,7 @@ PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>      return bus;
>  }
>  
> -static PCIBus *pci_register_secondary_bus(PCIDevice *dev, pci_map_irq_fn map_irq)
> +PCIBus *pci_register_secondary_bus(PCIDevice *dev, pci_map_irq_fn map_irq)
>  {
>      PCIBus *bus;
>      bus = qemu_mallocz(sizeof(PCIBus));
> @@ -590,6 +572,10 @@ void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len)
>      printf("pci_config_write: %s: addr=%02x val=%08x len=%d\n",
>             pci_dev->name, config_addr, val, len);
>  #endif
> +
> +#ifdef CONFIG_PASSTHROUGH
> +    if (igd_pci_write(pci_dev, config_addr, val, len) == 0)
> +#endif
>      pci_dev->config_write(pci_dev, config_addr, val, len);
>  }
>  
> @@ -598,7 +584,7 @@ uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
>      PCIBus *s = opaque;
>      PCIDevice *pci_dev;
>      int config_addr, bus_num;
> -    uint32_t val;
> +    uint32_t val = 0;
>  
>      bus_num = (addr >> 16) & 0xff;
>      while (s && s->bus_num != bus_num)
> @@ -625,26 +611,8 @@ uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
>      config_addr = addr & 0xff;
>  
>  #ifdef CONFIG_PASSTHROUGH
> -    /* host bridge reads for IGD passthrough */
> -    if ( igd_passthru && pci_dev->devfn == 0x00 ) {
> -        val = pci_dev->config_read(pci_dev, config_addr, len);
> -
> -        if ( config_addr == 0x00 && len == 4 )
> -            val = pt_pci_host_read_long(0, 0, 0, 0x00);
> -        else if ( config_addr == 0x02 ) // Device ID
> -            val = pt_pci_host_read_word(0, 0, 0, 0x02);
> -        else if ( config_addr == 0x52 ) // GMCH Graphics Control Register
> -            val = pt_pci_host_read_word(0, 0, 0, 0x52);
> -        else if ( config_addr == 0xa0 ) // GMCH Top of Memory Register
> -            val = pt_pci_host_read_word(0, 0, 0, 0xa0);
> -        goto done_config_read;
> -    } else if ( igd_passthru && pci_dev->devfn == 0x10 &&
> -              config_addr == 0xfc ) { // read on IGD device
> -        val = 0;  // use SMI to communicate with the system BIOS
> -        goto done_config_read;
> -    }
> +    if ( igd_pci_read(pci_dev, config_addr, &val, len) == 0)
>  #endif
> -
>      val = pci_dev->config_read(pci_dev, config_addr, len);
>  
>   done_config_read:
> @@ -892,12 +860,7 @@ void pci_unplug_netifs(void)
>      }
>  }
>  
> -typedef struct {
> -    PCIDevice dev;
> -    PCIBus *bus;
> -} PCIBridge;
> -
> -static void pci_bridge_write_config(PCIDevice *d,
> +void pci_bridge_write_config(PCIDevice *d,
>                               uint32_t address, uint32_t val, int len)
>  {
>      PCIBridge *s = (PCIBridge *)d;
> diff --git a/hw/pci.h b/hw/pci.h
> index de5a4e1..18e7b6f 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -207,6 +207,32 @@ struct PCIDevice {
>      int irq_state[4];
>  };
>  
> +typedef void (*pci_set_irq_fn)(qemu_irq *pic, int irq_num, int level);
> +typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
> +
> +struct PCIBus {
> +    int bus_num;
> +    int devfn_min;
> +    pci_set_irq_fn set_irq;
> +    pci_map_irq_fn map_irq;
> +    uint32_t config_reg; /* XXX: suppress */
> +    /* low level pic */
> +    SetIRQFunc *low_set_irq;
> +    qemu_irq *irq_opaque;
> +    PCIDevice *devices[256];
> +    PCIDevice *parent_dev;
> +    PCIBus *next;
> +    /* The bus IRQ state is the logical OR of the connected devices.
> +       Keep a count of the number of devices with raised IRQs.  */
> +    int nirq;
> +    int irq_count[];
> +};
> +
> +typedef struct {
> +    PCIDevice dev;
> +    PCIBus *bus;
> +} PCIBridge;
> +
>  extern char direct_pci_str[];
>  extern int direct_pci_msitranslate;
>  extern int direct_pci_power_mgmt;
> @@ -235,8 +261,6 @@ void pci_default_write_config(PCIDevice *d,
>  void pci_device_save(PCIDevice *s, QEMUFile *f);
>  int pci_device_load(PCIDevice *s, QEMUFile *f);
>  
> -typedef void (*pci_set_irq_fn)(qemu_irq *pic, int irq_num, int level);
> -typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
>  PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>                           qemu_irq *pic, int devfn_min, int nirq);
>  
> @@ -341,5 +365,9 @@ PCIBus *sh_pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>  
>  /* pass-through.c */
>  int pt_init(PCIBus *e_bus);
> +void pci_bridge_write_config(PCIDevice *d,
> +                             uint32_t address, uint32_t val, int len);
> +PCIBus *pci_register_secondary_bus(PCIDevice *dev, pci_map_irq_fn map_irq);
> +
>  
>  #endif
> diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c
> new file mode 100644
> index 0000000..3b9ce95
> --- /dev/null
> +++ b/hw/pt-graphics.c
> @@ -0,0 +1,266 @@
> +/*
> + * graphics passthrough
> + */
> +
> +#include "pass-through.h"
> +#include "pci/header.h"
> +#include "pci/pci.h"
> +#include "pt-msi.h"
> +#include "qemu-xen.h"
> +#include "iomulti.h"
> +
> +#include <unistd.h>
> +#include <sys/ioctl.h>
> +
> +extern int gfx_passthru;
> +extern int igd_passthru;
> +
> +static int pch_irq_function(PCIDevice *pci_dev, int irq_num)
> +{
> +    PT_LOG("pch_irq_function called\n");
> +    return irq_num;
> +}
> +
> +PCIBus *intel_pch_init(PCIBus *bus)
> +{
> +    PCIBridge *pch;
> +    u16 vendor_id, device_id;
> +    u8  rev_id;
> +
> +    if ( !gfx_passthru )
> +        return NULL;
> +
> +    vendor_id = pt_pci_host_read(0, 0x1f, 0, 0, 2);
> +    device_id = pt_pci_host_read(0, 0x1f, 0, 2, 2);
> +    rev_id    = pt_pci_host_read(0, 0x1f, 0, 8, 1);
> +
> +    pch = (PCIBridge *)
> +          pci_register_device(bus, "intel_bridge_1f", sizeof(PCIBridge),
> +                     PCI_DEVFN(0x1f, 0), NULL, pci_bridge_write_config);
> +
> +    pci_config_set_vendor_id(pch->dev.config, vendor_id);
> +    pci_config_set_device_id(pch->dev.config, device_id);
> +
> +    pch->dev.config[0x04] = 0x06; // command = bus master, pci mem
> +    pch->dev.config[0x05] = 0x00;
> +    pch->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> +    pch->dev.config[0x07] = 0x00; // status = fast devsel
> +    pch->dev.config[0x08] = rev_id; 
> +    pch->dev.config[0x09] = 0x00; // programming i/f
> +    pci_config_set_class(pch->dev.config, PCI_CLASS_BRIDGE_ISA);
> +    pch->dev.config[0x0D] = 0x10; // latency_timer
> +    pch->dev.config[0x0E] = 0x81; // header_type
> +    pch->dev.config[0x1E] = 0xa0; // secondary status
> +
> +    pch->bus = pci_register_secondary_bus(&pch->dev, pch_irq_function);
> +    return pch->bus;
> +}
> +

What happens if the hardware is a pre-Calpella Intel graphic card? Is it
still going to work?


> +int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
> +{
> +    if ( !igd_passthru || (pci_dev->devfn != 0x00 ) )
> +        return 0;
> +
> +    switch (config_addr)
> +    {
> +        case 0x58:        // PAVPC Offset
> +            pt_pci_host_write(0, 0, 0, config_addr, val, len);
> +            PT_LOG("pci_config_write: %x:%x.%x: addr=%x len=%x val=%x\n",
> +                   pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn),
> +                   PCI_FUNC(pci_dev->devfn), config_addr, len, val);
> +            break;
> +        default:
> +            pci_dev->config_write(pci_dev, config_addr, val, len);
> +    }
> +    return 1;
> +}
> +
> +int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len)
> +{
> +    if ( !igd_passthru || (pci_dev->devfn != 0) )
> +        return 0;
> +
> +    switch (config_addr)
> +    {
> +        case 0x00:        /* vendor id */
> +        case 0x02:        /* device id */
> +        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 */
> +            *val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
> +                                   0, config_addr, len);
> +            PT_LOG("pci_config_read: %x:%x.%x: addr=%x len=%x val=%x\n",
> +                   pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn),
> +                   PCI_FUNC(pci_dev->devfn), config_addr, len, *val);
> +
> +            break;
> +        default:
> +            *val = pci_dev->config_read(pci_dev, config_addr, len);
> +    }
> +    return 1;
> +}
> +
> +/*
> + * register VGA resources for the domain with assigned gfx
> + */
> +int register_vga_regions(struct pt_dev *real_device)
> +{
> +    u32 igd_opregion, igd_bsm;
> +    int ret = 0;
> +
> +    if ( !gfx_passthru || real_device->pci_dev->device_class != 0x0300 )
> +        return ret;
> +
> +    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3B0,
> +            0x3B0, 0xC, DPCI_ADD_MAPPING);
> +
> +    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3C0,
> +            0x3C0, 0x20, DPCI_ADD_MAPPING);
> +
> +    ret |= xc_domain_memory_mapping(xc_handle, domid,
> +            0xa0000 >> XC_PAGE_SHIFT,
> +            0xa0000 >> XC_PAGE_SHIFT,
> +            0x20,
> +            DPCI_ADD_MAPPING);
> +
> +    /* 1:1 map ASL Storage register value */
> +    igd_opregion = pt_pci_host_read(0, 2, 0, 0xfc, 4);
> +    PT_LOG("register_vga: igd_opregion = %x\n", igd_opregion);
> +    ret |= xc_domain_memory_mapping(xc_handle, domid,
> +            igd_opregion >> XC_PAGE_SHIFT,
> +            igd_opregion >> XC_PAGE_SHIFT,
> +            2,
> +            DPCI_ADD_MAPPING);

Again, what happens if the hardware is older? Do all Intel graphic cards
have an opregion?

> +    igd_opregion = pt_pci_host_read(0, 2, 0, 0xfc, 4);
> +    ret |= xc_domain_memory_mapping(xc_handle, domid,
> +            igd_opregion >> XC_PAGE_SHIFT,
> +            igd_opregion >> XC_PAGE_SHIFT,
> +            2,
> +            DPCI_REMOVE_MAPPING);
> +

ditto

RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Kay, Allen M]

Stephano,

Thanks for your feedback.  I add "#ifdef CONFIG_PASSTHROUGH" in the next version of the patch.

I have tested the patch on a Intel Montevina software development platform and found it has the same behavior as current xen qemu upstream.

Can others who have been playing with IGD passthrough give the patch a try on older platforms?

Allen

-----Original Message-----
From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
Sent: Friday, June 04, 2010 8:29 AM
To: Kay, Allen M
Cc: xen-devel@lists.xensource.com; Ian Pratt; Han, Weidong; Ross Philipson; Jean Guyader
Subject: Re: [Xen-devel] [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

On Fri, 4 Jun 2010, Kay, Allen M wrote:
>
> Added Calpella and Sandybridge integrated graphics pass-through support, consolidated graphics pass-through code into
> pt-graphics.c.
>
> Signed-off-by: Allen Kay <allen.m.kay@intel.com>
>

some comments follow

> diff --git a/hw/pass-through.c b/hw/pass-through.c
> index 5a76e8d..971c7f1 100644
> --- a/hw/pass-through.c
> +++ b/hw/pass-through.c
> @@ -1865,50 +1865,6 @@ static int pt_dev_is_virtfn(struct pci_dev *dev)
>      return rc;
>  }
>
> -/*
> - * register VGA resources for the domain with assigned gfx
> - */
> -static int register_vga_regions(struct pt_dev *real_device)
> -{
> -    int ret = 0;
> -
> -    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3B0,
> -            0x3B0, 0xC, DPCI_ADD_MAPPING);
> -
> -    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3C0,
> -            0x3C0, 0x20, DPCI_ADD_MAPPING);
> -
> -    ret |= xc_domain_memory_mapping(xc_handle, domid,
> -            0xa0000 >> XC_PAGE_SHIFT,
> -            0xa0000 >> XC_PAGE_SHIFT,
> -            0x20,
> -            DPCI_ADD_MAPPING);
> -
> -    return ret;
> -}
> -
> -/*
> - * unregister VGA resources for the domain with assigned gfx
> - */
> -static int unregister_vga_regions(struct pt_dev *real_device)
> -{
> -    int ret = 0;
> -
> -    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3B0,
> -            0x3B0, 0xC, DPCI_REMOVE_MAPPING);
> -
> -    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3C0,
> -            0x3C0, 0x20, DPCI_REMOVE_MAPPING);
> -
> -    ret |= xc_domain_memory_mapping(xc_handle, domid,
> -            0xa0000 >> XC_PAGE_SHIFT,
> -            0xa0000 >> XC_PAGE_SHIFT,
> -            0x20,
> -            DPCI_REMOVE_MAPPING);
> -
> -    return ret;
> -}
> -
>  static int pt_register_regions(struct pt_dev *assigned_device)
>  {
>      int i = 0;
> @@ -1970,17 +1926,7 @@ static int pt_register_regions(struct pt_dev *assigned_device)
>          PT_LOG("Expansion ROM registered (size=0x%08x base_addr=0x%08x)\n",
>              (uint32_t)(pci_dev->rom_size), (uint32_t)(pci_dev->rom_base_addr));
>      }
> -
> -    if ( gfx_passthru && (pci_dev->device_class == 0x0300) )
> -    {
> -        ret = register_vga_regions(assigned_device);
> -        if ( ret != 0 )
> -        {
> -            PT_LOG("VGA region mapping failed\n");
> -            return ret;
> -        }
> -    }
> -
> +    register_vga_regions(assigned_device);
>      return 0;
>  }
>
> @@ -2029,13 +1975,7 @@ static void pt_unregister_regions(struct pt_dev *assigned_device)
>          }
>
>      }
> -
> -    if ( gfx_passthru && (assigned_device->pci_dev->device_class == 0x0300) )
> -    {
> -        ret = unregister_vga_regions(assigned_device);
> -        if ( ret != 0 )
> -            PT_LOG("VGA region unmapping failed\n");
> -    }
> +    unregister_vga_regions(assigned_device);
>  }
>
>  static uint8_t find_cap_offset(struct pci_dev *pci_dev, uint8_t cap)
> @@ -2097,46 +2037,66 @@ static uint32_t find_ext_cap_offset(struct pci_dev *pci_dev, uint32_t cap)
>      return 0;
>  }
>
> -u8 pt_pci_host_read_byte(int bus, int dev, int fn, u32 addr)
> +static void pci_access_init(void)
>  {
> -    struct pci_dev *pci_dev;
> -    u8 val;
> +    struct pci_access *pci_access;
>
> -    pci_dev = pci_get_dev(dpci_infos.pci_access, 0, bus, dev, fn);
> -    if ( !pci_dev )
> -        return 0;
> +    if (dpci_infos.pci_access)
> +        return;
>
> -    val = pci_read_byte(pci_dev, addr);
> -    pci_free_dev(pci_dev);
> -    return val;
> +    /* Initialize libpci */
> +    pci_access = pci_alloc();
> +    if ( pci_access == NULL ) {
> +        PT_LOG("Error: pci_access is NULL\n");
> +        return -1;
> +    }
> +    pci_init(pci_access);
> +    pci_scan_bus(pci_access);
> +    dpci_infos.pci_access = pci_access;
>  }
>
> -u16 pt_pci_host_read_word(int bus, int dev, int fn, u32 addr)
> +u32 pt_pci_host_read(int bus, int dev, int fn, u32 addr, int len)
>  {
> +
>      struct pci_dev *pci_dev;
> -    u16 val;
> +    u32 val = -1;
>
> +    pci_access_init();
>      pci_dev = pci_get_dev(dpci_infos.pci_access, 0, bus, dev, fn);
>      if ( !pci_dev )
>          return 0;
>
> -    val = pci_read_word(pci_dev, addr);
> -    pci_free_dev(pci_dev);
> +    switch (len)
> +    {
> +        case 1: val = pci_read_byte(pci_dev, addr); break;
> +        case 2: val = pci_read_word(pci_dev, addr); break;
> +        case 4: val = pci_read_long(pci_dev, addr); break;
> +        default:
> +            fprintf(stderr, "error: pt_pci_host_read: invalid len = %d\n", len);
> +    }
>      return val;
>  }
>
> -u32 pt_pci_host_read_long(int bus, int dev, int fn, u32 addr)
> +int pt_pci_host_write(int bus, int dev, int fn, u32 addr, u32 val, int len)
>  {
>      struct pci_dev *pci_dev;
> -    u32 val;
> +    int ret = 0;
>
> +    pci_access_init();
>      pci_dev = pci_get_dev(dpci_infos.pci_access, 0, bus, dev, fn);
>      if ( !pci_dev )
>          return 0;
>
> -    val = pci_read_long(pci_dev, addr);
> +    switch (len)
> +    {
> +        case 1: ret = pci_write_byte(pci_dev, addr, val); break;
> +        case 2: ret = pci_write_word(pci_dev, addr, val); break;
> +        case 4: ret = pci_write_long(pci_dev, addr, val); break;
> +        default:
> +            fprintf(stderr, "error: pt_pci_host_write: invalid len = %d\n", len);
> +    }
>      pci_free_dev(pci_dev);
> -    return val;
> +    return ret;
>  }
>
>  /* parse BAR */
> @@ -4200,92 +4160,6 @@ static int pt_pmcsr_reg_restore(struct pt_dev *ptdev,
>      return 0;
>  }
>
> -static int get_vgabios(unsigned char *buf)
> -{
> -    int fd;
> -    uint32_t bios_size = 0;
> -    uint32_t start = 0xC0000;
> -    uint16_t magic = 0;
> -
> -    if ( (fd = open("/dev/mem", O_RDONLY)) < 0 )
> -    {
> -        PT_LOG("Error: Can't open /dev/mem: %s\n", strerror(errno));
> -        return 0;
> -    }
> -
> -    /*
> -     * Check if it a real bios extension.
> -     * The magic number is 0xAA55.
> -     */
> -    if ( start != lseek(fd, start, SEEK_SET) )
> -        goto out;
> -    if ( read(fd, &magic, 2) != 2 )
> -        goto out;
> -    if ( magic != 0xAA55 )
> -        goto out;
> -
> -    /* Find the size of the rom extension */
> -    if ( start != lseek(fd, start, SEEK_SET) )
> -        goto out;
> -    if ( lseek(fd, 2, SEEK_CUR) != (start + 2) )
> -        goto out;
> -    if ( read(fd, &bios_size, 1) != 1 )
> -        goto out;
> -
> -    /* This size is in 512 bytes */
> -    bios_size *= 512;
> -
> -    /*
> -     * Set the file to the begining of the rombios,
> -     * to start the copy.
> -     */
> -    if ( start != lseek(fd, start, SEEK_SET) )
> -        goto out;
> -
> -    if ( bios_size != read(fd, buf, bios_size))
> -        bios_size = 0;
> -
> -out:
> -    close(fd);
> -    return bios_size;
> -}
> -
> -static int setup_vga_pt(void)
> -{
> -    unsigned char *bios = NULL;
> -    int bios_size = 0;
> -    char *c = NULL;
> -    char checksum = 0;
> -    int rc = 0;
> -
> -    /* Allocated 64K for the vga bios */
> -    if ( !(bios = malloc(64 * 1024)) )
> -        return -1;
> -
> -    bios_size = get_vgabios(bios);
> -    if ( bios_size == 0 || bios_size > 64 * 1024)
> -    {
> -        PT_LOG("vga bios size (0x%x) is invalid!\n", bios_size);
> -        rc = -1;
> -        goto out;
> -    }
> -
> -    /* Adjust the bios checksum */
> -    for ( c = (char*)bios; c < ((char*)bios + bios_size); c++ )
> -        checksum += *c;
> -    if ( checksum )
> -    {
> -        bios[bios_size - 1] -= checksum;
> -        PT_LOG("vga bios checksum is adjusted!\n");
> -    }
> -
> -    cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
> -
> -out:
> -    free(bios);
> -    return rc;
> -}
> -
>  static struct pt_dev * register_real_device(PCIBus *e_bus,
>          const char *e_dev_name, int e_devfn, uint8_t r_bus, uint8_t r_dev,
>          uint8_t r_func, uint32_t machine_irq, struct pci_access *pci_access,
> @@ -4387,16 +4261,13 @@ static struct pt_dev * register_real_device(PCIBus *e_bus,
>      pt_register_regions(assigned_device);
>
>      /* Setup VGA bios for passthroughed gfx */
> -    if ( gfx_passthru && (assigned_device->pci_dev->device_class == 0x0300) )
> +    if ( setup_vga_pt(assigned_device) < 0 )
>      {
> -        rc = setup_vga_pt();
> -        if ( rc < 0 )
> -        {
> -            PT_LOG("Setup VGA BIOS of passthroughed gfx failed!\n");
> -            return NULL;
> -        }
> +        PT_LOG("Setup VGA BIOS of passthroughed gfx failed!\n");
> +        return NULL;
>      }
>
> +
>      /* reinitialize each config register to be emulated */
>      rc = pt_config_init(assigned_device);
>      if ( rc < 0 ) {
> @@ -4548,19 +4419,8 @@ int power_on_php_devfn(int devfn)
>  {
>      struct php_dev *php_dev = &dpci_infos.php_devs[devfn];
>      struct pt_dev *pt_dev;
> -    struct pci_access *pci_access;
>
> -    if (!dpci_infos.pci_access) {
> -        /* Initialize libpci */
> -        pci_access = pci_alloc();
> -        if ( pci_access == NULL ) {
> -            PT_LOG("Error: pci_access is NULL\n");
> -            return -1;
> -        }
> -        pci_init(pci_access);
> -        pci_scan_bus(pci_access);
> -        dpci_infos.pci_access = pci_access;
> -    }
> +    pci_access_init();
>
>      pt_dev =
>          register_real_device(dpci_infos.e_bus,
> diff --git a/hw/pass-through.h b/hw/pass-through.h
> index f8a0c73..6c5e8ca 100644
> --- a/hw/pass-through.h
> +++ b/hw/pass-through.h
> @@ -406,10 +406,16 @@ static inline pciaddr_t pt_pci_base_addr(pciaddr_t base)
>  }
>
>  uint8_t pci_intx(struct pt_dev *ptdev);
> -
> -u8 pt_pci_host_read_byte(int bus, int dev, int fn, u32 addr);
> -u16 pt_pci_host_read_word(int bus, int dev, int fn, u32 addr);
> -u32 pt_pci_host_read_long(int bus, int dev, int fn, u32 addr);
> +u32 pt_pci_host_read(int bus, int dev, int fn, u32 addr, int len);
> +int pt_pci_host_write(int bus, int dev, int fn, u32 addr, u32 val, int len);
> +PCIBus *intel_pch_init(PCIBus *bus);
> +int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len);
> +int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len);
> +int register_vga_regions(struct pt_dev *real_device);
> +int unregister_vga_regions(struct pt_dev *real_device);
> +int setup_vga_pt(struct pt_dev *real_device);
> +PCIBus *intel_pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid,
> +           uint16_t did, const char *name, uint16_t revision);
>
>  #endif /* __PASSTHROUGH_H__ */
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 9375951..7364cb8 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -42,6 +42,10 @@
>  #include "virtio-console.h"
>  #include "hpet_emul.h"
>
> +#ifdef CONFIG_PASSTHROUGH
> +#include "pass-through.h"
> +#endif
> +
>  /* output Bochs bios info messages */
>  //#define DEBUG_BIOS
>
> @@ -978,6 +982,8 @@ vga_bios_error:
>          pci_bus = NULL;
>      }
>
> +    intel_pch_init(pci_bus);
> +
>      /* init basic PC hardware */
>      register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
>

You are missing a ifdef CONFIG_PASSTHROUGH here


> diff --git a/hw/pci.c b/hw/pci.c
> index b07e5ea..fa32ed9 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -39,24 +39,6 @@ extern int igd_passthru;
>
>  //#define DEBUG_PCI
>
> -struct PCIBus {
> -    int bus_num;
> -    int devfn_min;
> -    pci_set_irq_fn set_irq;
> -    pci_map_irq_fn map_irq;
> -    uint32_t config_reg; /* XXX: suppress */
> -    /* low level pic */
> -    SetIRQFunc *low_set_irq;
> -    qemu_irq *irq_opaque;
> -    PCIDevice *devices[256];
> -    PCIDevice *parent_dev;
> -    PCIBus *next;
> -    /* The bus IRQ state is the logical OR of the connected devices.
> -       Keep a count of the number of devices with raised IRQs.  */
> -    int nirq;
> -    int irq_count[];
> -};
> -
>  static void pci_update_mappings(PCIDevice *d);
>  static void pci_set_irq(void *opaque, int irq_num, int level);
>
> @@ -96,7 +78,7 @@ PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>      return bus;
>  }
>
> -static PCIBus *pci_register_secondary_bus(PCIDevice *dev, pci_map_irq_fn map_irq)
> +PCIBus *pci_register_secondary_bus(PCIDevice *dev, pci_map_irq_fn map_irq)
>  {
>      PCIBus *bus;
>      bus = qemu_mallocz(sizeof(PCIBus));
> @@ -590,6 +572,10 @@ void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len)
>      printf("pci_config_write: %s: addr=%02x val=%08x len=%d\n",
>             pci_dev->name, config_addr, val, len);
>  #endif
> +
> +#ifdef CONFIG_PASSTHROUGH
> +    if (igd_pci_write(pci_dev, config_addr, val, len) == 0)
> +#endif
>      pci_dev->config_write(pci_dev, config_addr, val, len);
>  }
>
> @@ -598,7 +584,7 @@ uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
>      PCIBus *s = opaque;
>      PCIDevice *pci_dev;
>      int config_addr, bus_num;
> -    uint32_t val;
> +    uint32_t val = 0;
>
>      bus_num = (addr >> 16) & 0xff;
>      while (s && s->bus_num != bus_num)
> @@ -625,26 +611,8 @@ uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
>      config_addr = addr & 0xff;
>
>  #ifdef CONFIG_PASSTHROUGH
> -    /* host bridge reads for IGD passthrough */
> -    if ( igd_passthru && pci_dev->devfn == 0x00 ) {
> -        val = pci_dev->config_read(pci_dev, config_addr, len);
> -
> -        if ( config_addr == 0x00 && len == 4 )
> -            val = pt_pci_host_read_long(0, 0, 0, 0x00);
> -        else if ( config_addr == 0x02 ) // Device ID
> -            val = pt_pci_host_read_word(0, 0, 0, 0x02);
> -        else if ( config_addr == 0x52 ) // GMCH Graphics Control Register
> -            val = pt_pci_host_read_word(0, 0, 0, 0x52);
> -        else if ( config_addr == 0xa0 ) // GMCH Top of Memory Register
> -            val = pt_pci_host_read_word(0, 0, 0, 0xa0);
> -        goto done_config_read;
> -    } else if ( igd_passthru && pci_dev->devfn == 0x10 &&
> -              config_addr == 0xfc ) { // read on IGD device
> -        val = 0;  // use SMI to communicate with the system BIOS
> -        goto done_config_read;
> -    }
> +    if ( igd_pci_read(pci_dev, config_addr, &val, len) == 0)
>  #endif
> -
>      val = pci_dev->config_read(pci_dev, config_addr, len);
>
>   done_config_read:
> @@ -892,12 +860,7 @@ void pci_unplug_netifs(void)
>      }
>  }
>
> -typedef struct {
> -    PCIDevice dev;
> -    PCIBus *bus;
> -} PCIBridge;
> -
> -static void pci_bridge_write_config(PCIDevice *d,
> +void pci_bridge_write_config(PCIDevice *d,
>                               uint32_t address, uint32_t val, int len)
>  {
>      PCIBridge *s = (PCIBridge *)d;
> diff --git a/hw/pci.h b/hw/pci.h
> index de5a4e1..18e7b6f 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -207,6 +207,32 @@ struct PCIDevice {
>      int irq_state[4];
>  };
>
> +typedef void (*pci_set_irq_fn)(qemu_irq *pic, int irq_num, int level);
> +typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
> +
> +struct PCIBus {
> +    int bus_num;
> +    int devfn_min;
> +    pci_set_irq_fn set_irq;
> +    pci_map_irq_fn map_irq;
> +    uint32_t config_reg; /* XXX: suppress */
> +    /* low level pic */
> +    SetIRQFunc *low_set_irq;
> +    qemu_irq *irq_opaque;
> +    PCIDevice *devices[256];
> +    PCIDevice *parent_dev;
> +    PCIBus *next;
> +    /* The bus IRQ state is the logical OR of the connected devices.
> +       Keep a count of the number of devices with raised IRQs.  */
> +    int nirq;
> +    int irq_count[];
> +};
> +
> +typedef struct {
> +    PCIDevice dev;
> +    PCIBus *bus;
> +} PCIBridge;
> +
>  extern char direct_pci_str[];
>  extern int direct_pci_msitranslate;
>  extern int direct_pci_power_mgmt;
> @@ -235,8 +261,6 @@ void pci_default_write_config(PCIDevice *d,
>  void pci_device_save(PCIDevice *s, QEMUFile *f);
>  int pci_device_load(PCIDevice *s, QEMUFile *f);
>
> -typedef void (*pci_set_irq_fn)(qemu_irq *pic, int irq_num, int level);
> -typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
>  PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>                           qemu_irq *pic, int devfn_min, int nirq);
>
> @@ -341,5 +365,9 @@ PCIBus *sh_pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>
>  /* pass-through.c */
>  int pt_init(PCIBus *e_bus);
> +void pci_bridge_write_config(PCIDevice *d,
> +                             uint32_t address, uint32_t val, int len);
> +PCIBus *pci_register_secondary_bus(PCIDevice *dev, pci_map_irq_fn map_irq);
> +
>
>  #endif
> diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c
> new file mode 100644
> index 0000000..3b9ce95
> --- /dev/null
> +++ b/hw/pt-graphics.c
> @@ -0,0 +1,266 @@
> +/*
> + * graphics passthrough
> + */
> +
> +#include "pass-through.h"
> +#include "pci/header.h"
> +#include "pci/pci.h"
> +#include "pt-msi.h"
> +#include "qemu-xen.h"
> +#include "iomulti.h"
> +
> +#include <unistd.h>
> +#include <sys/ioctl.h>
> +
> +extern int gfx_passthru;
> +extern int igd_passthru;
> +
> +static int pch_irq_function(PCIDevice *pci_dev, int irq_num)
> +{
> +    PT_LOG("pch_irq_function called\n");
> +    return irq_num;
> +}
> +
> +PCIBus *intel_pch_init(PCIBus *bus)
> +{
> +    PCIBridge *pch;
> +    u16 vendor_id, device_id;
> +    u8  rev_id;
> +
> +    if ( !gfx_passthru )
> +        return NULL;
> +
> +    vendor_id = pt_pci_host_read(0, 0x1f, 0, 0, 2);
> +    device_id = pt_pci_host_read(0, 0x1f, 0, 2, 2);
> +    rev_id    = pt_pci_host_read(0, 0x1f, 0, 8, 1);
> +
> +    pch = (PCIBridge *)
> +          pci_register_device(bus, "intel_bridge_1f", sizeof(PCIBridge),
> +                     PCI_DEVFN(0x1f, 0), NULL, pci_bridge_write_config);
> +
> +    pci_config_set_vendor_id(pch->dev.config, vendor_id);
> +    pci_config_set_device_id(pch->dev.config, device_id);
> +
> +    pch->dev.config[0x04] = 0x06; // command = bus master, pci mem
> +    pch->dev.config[0x05] = 0x00;
> +    pch->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> +    pch->dev.config[0x07] = 0x00; // status = fast devsel
> +    pch->dev.config[0x08] = rev_id;
> +    pch->dev.config[0x09] = 0x00; // programming i/f
> +    pci_config_set_class(pch->dev.config, PCI_CLASS_BRIDGE_ISA);
> +    pch->dev.config[0x0D] = 0x10; // latency_timer
> +    pch->dev.config[0x0E] = 0x81; // header_type
> +    pch->dev.config[0x1E] = 0xa0; // secondary status
> +
> +    pch->bus = pci_register_secondary_bus(&pch->dev, pch_irq_function);
> +    return pch->bus;
> +}
> +

What happens if the hardware is a pre-Calpella Intel graphic card? Is it
still going to work?


> +int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
> +{
> +    if ( !igd_passthru || (pci_dev->devfn != 0x00 ) )
> +        return 0;
> +
> +    switch (config_addr)
> +    {
> +        case 0x58:        // PAVPC Offset
> +            pt_pci_host_write(0, 0, 0, config_addr, val, len);
> +            PT_LOG("pci_config_write: %x:%x.%x: addr=%x len=%x val=%x\n",
> +                   pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn),
> +                   PCI_FUNC(pci_dev->devfn), config_addr, len, val);
> +            break;
> +        default:
> +            pci_dev->config_write(pci_dev, config_addr, val, len);
> +    }
> +    return 1;
> +}
> +
> +int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len)
> +{
> +    if ( !igd_passthru || (pci_dev->devfn != 0) )
> +        return 0;
> +
> +    switch (config_addr)
> +    {
> +        case 0x00:        /* vendor id */
> +        case 0x02:        /* device id */
> +        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 */
> +            *val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
> +                                   0, config_addr, len);
> +            PT_LOG("pci_config_read: %x:%x.%x: addr=%x len=%x val=%x\n",
> +                   pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn),
> +                   PCI_FUNC(pci_dev->devfn), config_addr, len, *val);
> +
> +            break;
> +        default:
> +            *val = pci_dev->config_read(pci_dev, config_addr, len);
> +    }
> +    return 1;
> +}
> +
> +/*
> + * register VGA resources for the domain with assigned gfx
> + */
> +int register_vga_regions(struct pt_dev *real_device)
> +{
> +    u32 igd_opregion, igd_bsm;
> +    int ret = 0;
> +
> +    if ( !gfx_passthru || real_device->pci_dev->device_class != 0x0300 )
> +        return ret;
> +
> +    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3B0,
> +            0x3B0, 0xC, DPCI_ADD_MAPPING);
> +
> +    ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3C0,
> +            0x3C0, 0x20, DPCI_ADD_MAPPING);
> +
> +    ret |= xc_domain_memory_mapping(xc_handle, domid,
> +            0xa0000 >> XC_PAGE_SHIFT,
> +            0xa0000 >> XC_PAGE_SHIFT,
> +            0x20,
> +            DPCI_ADD_MAPPING);
> +
> +    /* 1:1 map ASL Storage register value */
> +    igd_opregion = pt_pci_host_read(0, 2, 0, 0xfc, 4);
> +    PT_LOG("register_vga: igd_opregion = %x\n", igd_opregion);
> +    ret |= xc_domain_memory_mapping(xc_handle, domid,
> +            igd_opregion >> XC_PAGE_SHIFT,
> +            igd_opregion >> XC_PAGE_SHIFT,
> +            2,
> +            DPCI_ADD_MAPPING);

Again, what happens if the hardware is older? Do all Intel graphic cards
have an opregion?

> +    igd_opregion = pt_pci_host_read(0, 2, 0, 0xfc, 4);
> +    ret |= xc_domain_memory_mapping(xc_handle, domid,
> +            igd_opregion >> XC_PAGE_SHIFT,
> +            igd_opregion >> XC_PAGE_SHIFT,
> +            2,
> +            DPCI_REMOVE_MAPPING);
> +

ditto

RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Stefano Stabellini]

On Tue, 8 Jun 2010, Kay, Allen M wrote:
> Stephano,
> 
> Thanks for your feedback.  I add "#ifdef CONFIG_PASSTHROUGH" in the next version of the patch.
> 
> I have tested the patch on a Intel Montevina software development platform and found it has the same behavior as current xen qemu upstream.
> 
> Can others who have been playing with IGD passthrough give the patch a try on older platforms?
> 

Are there any older platforms without opregion support?
If that is the case what is the result of executing this code on such a
platform?

+    /* 1:1 map ASL Storage register value */
+    igd_opregion = pt_pci_host_read(0, 2, 0, 0xfc, 4);
+    PT_LOG("register_vga: igd_opregion = %x\n", igd_opregion);
+    ret |= xc_domain_memory_mapping(xc_handle, domid,
+            igd_opregion >> XC_PAGE_SHIFT,
+            igd_opregion >> XC_PAGE_SHIFT,
+            2,
+            DPCI_ADD_MAPPING);

Re: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Jean Guyader]

On 9 June 2010 16:57, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Tue, 8 Jun 2010, Kay, Allen M wrote:
>> Stephano,
>>
>> Thanks for your feedback.  I add "#ifdef CONFIG_PASSTHROUGH" in the next version of the patch.
>>
>> I have tested the patch on a Intel Montevina software development platform and found it has the same behavior as current xen qemu upstream.
>>
>> Can others who have been playing with IGD passthrough give the patch a try on older platforms?
>>
>
> Are there any older platforms without opregion support?
> If that is the case what is the result of executing this code on such a
> platform?
>
> +    /* 1:1 map ASL Storage register value */
> +    igd_opregion = pt_pci_host_read(0, 2, 0, 0xfc, 4);
> +    PT_LOG("register_vga: igd_opregion = %x\n", igd_opregion);
> +    ret |= xc_domain_memory_mapping(xc_handle, domid,
> +            igd_opregion >> XC_PAGE_SHIFT,
> +            igd_opregion >> XC_PAGE_SHIFT,
> +            2,
> +            DPCI_ADD_MAPPING);
>
>

We are using a fixed and reserved address in the guest to map this
region, 0xfed00000.
You need to add that in the e820 table as ACPI_NVS so the guest won't
try to use it for
something else. You also have to trap the pci_config space read for
0xfc and give 0xfed00000
back to the guest.

We've notice that on some system the value at 0xfc wasn't align on a
page, so make sure
you do that first.

Jean

Re: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Isaku Yamahata]

On Thu, Jun 03, 2010 at 05:18:33PM -0700, Kay, Allen M wrote:
> Added Calpella and Sandybridge integrated graphics pass-through support,
> consolidated graphics pass-through code into pt-graphics.c.
> 
>  
> 
> Signed-off-by: Allen Kay <allen.m.kay@intel.com>
>

Some minor comments.
pci_{read, write}_block() would be better than
switch(len) case 1: case 2: case4:...
Is it really necessary to move PCIBus and PCIBridge to the header file?
Doesn't pci_bridge_init() work?
Overriding pci config read/write methods of i440fx would be much cleaner
than hooking pci_data_read/write. (pass igd_pci_read/write to 
pci_register_device() in i440fx_init() in hw/piix_pci.c instead of NULL)
-- 
yamahata

RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Kay, Allen M]

Isaku,

Thanks for the feedback.

> pci_{read, write}_block() would be better than
> switch(len) case 1: case 2: case4:...

Done!

> Is it really necessary to move PCIBus and PCIBridge to the header file?
> Doesn't pci_bridge_init() work?

I changed to code to utilize pci_bridge_init().  However, I still need to move
PCIBus and PCIBridge defines to the header file.  The alternative is to pollute
pc.c with graphics passthrough specific code.

> Overriding pci config read/write methods of i440fx would be much cleaner
> than hooking pci_data_read/write. (pass igd_pci_read/write to 
> pci_register_device() in i440fx_init() in hw/piix_pci.c instead of NULL)

Doing this resulted in a lot of duplicated code and also force code path to change
even when IGD passthrough is not used.  To do it correctly, I also need to
put in IGD or PASSTHROGH awareness in piix_pci.c.  For now, I'm going to keep
the original patch.

Allen

RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Stefano Stabellini]

On Tue, 8 Jun 2010, Kay, Allen M wrote:
> Doing this resulted in a lot of duplicated code and also force code path to change
> even when IGD passthrough is not used.  To do it correctly, I also need to
> put in IGD or PASSTHROGH awareness in piix_pci.c.  For now, I'm going to keep
> the original patch.

Can you register a dummy device at the address corresponding to IGD and
get the pci conf read and write calls directly from the functions you
pass to pci_register_device?

RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Kay, Allen M]

> Can you register a dummy device at the address corresponding to IGD and
> get the pci conf read and write calls directly from the functions you
> pass to pci_register_device?

I'm not sure what do you mean...

The issue is IGD driver accesses some registers in device 0:0.0.  However, we don't want to passthrough platform's 0:0.0 to the guest as it will change the guest's chipset wholesale thus create more problems than it solves.  For now, we are keeping the 440 chipset's 0:0.0 device but passthrough certain register accesses.

Isaku proposed register another set of read/write function for guest's device 0:0.0 instead of modifying existing functions.  The advantage of this approach is that original QEMU read/write function will be left unmodified for IGD purpose.  The disadvantage is this requires making a copy of exiting code and redirect all read/write handling of 0:0.0 device to a function in pt-graphics.c.  I tried it out but found the resulting code a bit confusing.

Allen

RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Stefano Stabellini]

On Tue, 8 Jun 2010, Kay, Allen M wrote:
> Isaku,
> 
> Thanks for the feedback.
> 
> > pci_{read, write}_block() would be better than
> > switch(len) case 1: case 2: case4:...
> 
> Done!
> 
> > Is it really necessary to move PCIBus and PCIBridge to the header file?
> > Doesn't pci_bridge_init() work?
> 
> I changed to code to utilize pci_bridge_init().  However, I still need to move
> PCIBus and PCIBridge defines to the header file.  The alternative is to pollute
> pc.c with graphics passthrough specific code.
> 
> > Overriding pci config read/write methods of i440fx would be much cleaner
> > than hooking pci_data_read/write. (pass igd_pci_read/write to 
> > pci_register_device() in i440fx_init() in hw/piix_pci.c instead of NULL)
> 
> Doing this resulted in a lot of duplicated code and also force code path to change
> even when IGD passthrough is not used.  To do it correctly, I also need to
> put in IGD or PASSTHROGH awareness in piix_pci.c.  For now, I'm going to keep
> the original patch.
> 

I see.

Even though the patch is an improvement over what we currently have in
qemu-xen, it is still not acceptable in upstream qemu as it is.
Considering that we are pretty close to have the merge complete, I think
it worth trying to come up with something cleaner.

Isaku, you are the latest one to touch the pci_host code in qemu, do you
have any other suggestion?

Re: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Isaku Yamahata]

On Fri, Jun 11, 2010 at 12:08:29PM +0100, Stefano Stabellini wrote:
> On Tue, 8 Jun 2010, Kay, Allen M wrote:
> > Isaku,
> > 
> > Thanks for the feedback.
> > 
> > > pci_{read, write}_block() would be better than
> > > switch(len) case 1: case 2: case4:...
> > 
> > Done!
> > 
> > > Is it really necessary to move PCIBus and PCIBridge to the header file?
> > > Doesn't pci_bridge_init() work?
> > 
> > I changed to code to utilize pci_bridge_init().  However, I still need to move
> > PCIBus and PCIBridge defines to the header file.  The alternative is to pollute
> > pc.c with graphics passthrough specific code.
> > 
> > > Overriding pci config read/write methods of i440fx would be much cleaner
> > > than hooking pci_data_read/write. (pass igd_pci_read/write to 
> > > pci_register_device() in i440fx_init() in hw/piix_pci.c instead of NULL)
> > 
> > Doing this resulted in a lot of duplicated code and also force code path to change
> > even when IGD passthrough is not used.  To do it correctly, I also need to
> > put in IGD or PASSTHROGH awareness in piix_pci.c.  For now, I'm going to keep
> > the original patch.
> > 
> 
> I see.
> 
> Even though the patch is an improvement over what we currently have in
> qemu-xen, it is still not acceptable in upstream qemu as it is.
> Considering that we are pretty close to have the merge complete, I think
> it worth trying to come up with something cleaner.
> 
> Isaku, you are the latest one to touch the pci_host code in qemu, do you
> have any other suggestion?

How about the following patch which is on top of the v2 patch?
I did only compile test, but I think it's enough to show my idea.

For PCI bridge, I have a patches to clean up/enhance the implementation and
I'm planning to push to the qemu upstream soon.


>From 9a22525e523e834ab3822d14e715388c19a7d02e Mon Sep 17 00:00:00 2001
Message-Id: <9a22525e523e834ab3822d14e715388c19a7d02e.1276485897.git.yamahata@valinux.co.jp>
In-Reply-To: <cover.1276485897.git.yamahata@valinux.co.jp>
References: <cover.1276485897.git.yamahata@valinux.co.jp>
From: Isaku Yamahata <yamahata@valinux.co.jp>
Date: Mon, 14 Jun 2010 12:24:31 +0900
Subject: [PATCH] pci passthrough: some clean up and unexport PCIBus and PCIBridge.

some clean up and unexport PCIBus and PCIBridge.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pass-through.h |   10 ++++++++--
 hw/pci.c          |   29 +++++++++++++++++++++++------
 hw/pci.h          |   23 -----------------------
 hw/piix_pci.c     |    4 ++--
 hw/pt-graphics.c  |   32 +++++++++++++++++++-------------
 5 files changed, 52 insertions(+), 46 deletions(-)

diff --git a/hw/pass-through.h b/hw/pass-through.h
index 242d079..adc9f58 100644
--- a/hw/pass-through.h
+++ b/hw/pass-through.h
@@ -409,13 +409,19 @@ uint8_t pci_intx(struct pt_dev *ptdev);
 u32 pt_pci_host_read(int bus, int dev, int fn, u32 addr, int len);
 int pt_pci_host_write(int bus, int dev, int fn, u32 addr, u32 val, int len);
 void intel_pch_init(PCIBus *bus);
-int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len);
-int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len);
 int register_vga_regions(struct pt_dev *real_device);
 int unregister_vga_regions(struct pt_dev *real_device);
 int setup_vga_pt(struct pt_dev *real_device);
 PCIBus *intel_pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid,
            uint16_t did, const char *name, uint16_t revision);
 
+#ifdef CONFIG_PASSTHROUGH
+void igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len);
+uint32_t igd_pci_read(PCIDevice *pci_dev, int config_addr, int len);
+#else
+#define igd_pci_write(pci_dev, config_addr, val, len)   NULL
+#define igd_pci_read(pci_dev, config_addr, val, len)    NULL
+#endif
+
 #endif /* __PASSTHROUGH_H__ */
 
diff --git a/hw/pci.c b/hw/pci.c
index a5cb378..b6400f3 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -39,6 +39,24 @@ extern int igd_passthru;
 
 //#define DEBUG_PCI
 
+struct PCIBus {
+    int bus_num;
+    int devfn_min;
+    pci_set_irq_fn set_irq;
+    pci_map_irq_fn map_irq;
+    uint32_t config_reg; /* XXX: suppress */
+    /* low level pic */
+    SetIRQFunc *low_set_irq;
+    qemu_irq *irq_opaque;
+    PCIDevice *devices[256];
+    PCIDevice *parent_dev;
+    PCIBus *next;
+    /* The bus IRQ state is the logical OR of the connected devices.
+       Keep a count of the number of devices with raised IRQs.  */
+    int nirq;
+    int irq_count[];
+};
+
 static void pci_update_mappings(PCIDevice *d);
 static void pci_set_irq(void *opaque, int irq_num, int level);
 
@@ -573,9 +591,6 @@ void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len)
            pci_dev->name, config_addr, val, len);
 #endif
 
-#ifdef CONFIG_PASSTHROUGH
-    if (igd_pci_write(pci_dev, config_addr, val, len) == 0)
-#endif
     pci_dev->config_write(pci_dev, config_addr, val, len);
 }
 
@@ -610,9 +625,6 @@ uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
     }
     config_addr = addr & 0xff;
 
-#ifdef CONFIG_PASSTHROUGH
-    if ( igd_pci_read(pci_dev, config_addr, &val, len) == 0)
-#endif
     val = pci_dev->config_read(pci_dev, config_addr, len);
 
  done_config_read:
@@ -860,6 +872,11 @@ void pci_unplug_netifs(void)
     }
 }
 
+typedef struct {
+    PCIDevice dev;
+    PCIBus *bus;
+} PCIBridge;
+
 void pci_bridge_write_config(PCIDevice *d,
                              uint32_t address, uint32_t val, int len)
 {
diff --git a/hw/pci.h b/hw/pci.h
index 7a44035..8abbad7 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -210,29 +210,6 @@ struct PCIDevice {
 typedef void (*pci_set_irq_fn)(qemu_irq *pic, int irq_num, int level);
 typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
 
-struct PCIBus {
-    int bus_num;
-    int devfn_min;
-    pci_set_irq_fn set_irq;
-    pci_map_irq_fn map_irq;
-    uint32_t config_reg; /* XXX: suppress */
-    /* low level pic */
-    SetIRQFunc *low_set_irq;
-    qemu_irq *irq_opaque;
-    PCIDevice *devices[256];
-    PCIDevice *parent_dev;
-    PCIBus *next;
-    /* The bus IRQ state is the logical OR of the connected devices.
-       Keep a count of the number of devices with raised IRQs.  */
-    int nirq;
-    int irq_count[];
-};
-
-typedef struct {
-    PCIDevice dev;
-    PCIBus *bus;
-} PCIBridge;
-
 extern char direct_pci_str[];
 extern int direct_pci_msitranslate;
 extern int direct_pci_power_mgmt;
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 49d72b2..1bfe0fb 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -25,7 +25,7 @@
 #include "hw.h"
 #include "pc.h"
 #include "pci.h"
-
+#include "pass-through.h"
 
 static void i440fx_set_irq(qemu_irq *pic, int irq_num, int level);
 static void piix3_write_config(PCIDevice *d, 
@@ -207,7 +207,7 @@ PCIBus *i440fx_init(PCIDevice **pi440fx_state, qemu_irq *pic)
     register_ioport_read(0xcfc, 4, 4, pci_host_data_readl, s);
 
     d = pci_register_device(b, "i440FX", sizeof(PCIDevice), 0,
-                            NULL, NULL);
+                            igd_pci_read, igd_pci_write);
 
     pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_INTEL);
     pci_config_set_device_id(d->config, PCI_DEVICE_ID_INTEL_82441);
diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c
index 4923715..32c174d 100644
--- a/hw/pt-graphics.c
+++ b/hw/pt-graphics.c
@@ -8,6 +8,7 @@
 
 #include <unistd.h>
 #include <sys/ioctl.h>
+#include <assert.h>
 
 extern int gfx_passthru;
 extern int igd_passthru;
@@ -34,10 +35,13 @@ void intel_pch_init(PCIBus *bus)
                     pch_map_irq, "intel_bridge_1f");
 }
 
-int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
+void igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
 {
-    if ( !igd_passthru || (pci_dev->devfn != 0x00 ) )
-        return 0;
+    assert(pci_dev->devfn == 0x00);
+    if ( !igd_passthru ) {
+        pci_default_write_config(pci_dev, config_addr, val, len);
+        return;
+    }
 
     switch (config_addr)
     {
@@ -48,15 +52,18 @@ int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
                    PCI_FUNC(pci_dev->devfn), config_addr, len, val);
             break;
         default:
-            pci_dev->config_write(pci_dev, config_addr, val, len);
+            pci_default_write_config(pci_dev, config_addr, val, len);
     }
-    return 1;
 }
 
-int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len)
+uint32_t igd_pci_read(PCIDevice *pci_dev, int config_addr, int len)
 {
-    if ( !igd_passthru || (pci_dev->devfn != 0) )
-        return 0;
+    uint32_t val;
+
+    assert(pci_dev->devfn == 0x00);
+    if ( !igd_passthru ) {
+        return pci_default_read_config(pci_dev, config_addr, len);
+    }
 
     switch (config_addr)
     {
@@ -68,17 +75,16 @@ int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len)
         case 0x58:        /* SNB: PAVPC Offset */
         case 0xa4:        /* SNB: graphics base of stolen memory */
         case 0xa8:        /* SNB: base of GTT stolen memory */
-            *val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
+            val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
                                    0, config_addr, len);
             PT_LOG("pci_config_read: %x:%x.%x: addr=%x len=%x val=%x\n",
                    pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn),
-                   PCI_FUNC(pci_dev->devfn), config_addr, len, *val);
-
+                   PCI_FUNC(pci_dev->devfn), config_addr, len, val);
             break;
         default:
-            *val = pci_dev->config_read(pci_dev, config_addr, len);
+            val = pci_default_read_config(pci_dev, config_addr, len);
     }
-    return 1;
+    return val;
 }
 
 /*
-- 
1.6.6.1

Re: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Stefano Stabellini]

On Mon, 14 Jun 2010, Isaku Yamahata wrote:
> On Fri, Jun 11, 2010 at 12:08:29PM +0100, Stefano Stabellini wrote:
> > On Tue, 8 Jun 2010, Kay, Allen M wrote:
> > > Isaku,
> > > 
> > > Thanks for the feedback.
> > > 
> > > > pci_{read, write}_block() would be better than
> > > > switch(len) case 1: case 2: case4:...
> > > 
> > > Done!
> > > 
> > > > Is it really necessary to move PCIBus and PCIBridge to the header file?
> > > > Doesn't pci_bridge_init() work?
> > > 
> > > I changed to code to utilize pci_bridge_init().  However, I still need to move
> > > PCIBus and PCIBridge defines to the header file.  The alternative is to pollute
> > > pc.c with graphics passthrough specific code.
> > > 
> > > > Overriding pci config read/write methods of i440fx would be much cleaner
> > > > than hooking pci_data_read/write. (pass igd_pci_read/write to 
> > > > pci_register_device() in i440fx_init() in hw/piix_pci.c instead of NULL)
> > > 
> > > Doing this resulted in a lot of duplicated code and also force code path to change
> > > even when IGD passthrough is not used.  To do it correctly, I also need to
> > > put in IGD or PASSTHROGH awareness in piix_pci.c.  For now, I'm going to keep
> > > the original patch.
> > > 
> > 
> > I see.
> > 
> > Even though the patch is an improvement over what we currently have in
> > qemu-xen, it is still not acceptable in upstream qemu as it is.
> > Considering that we are pretty close to have the merge complete, I think
> > it worth trying to come up with something cleaner.
> > 
> > Isaku, you are the latest one to touch the pci_host code in qemu, do you
> > have any other suggestion?
> 
> How about the following patch which is on top of the v2 patch?
> I did only compile test, but I think it's enough to show my idea.
> 
> For PCI bridge, I have a patches to clean up/enhance the implementation and
> I'm planning to push to the qemu upstream soon.
> 

I think this looks much better, thanks Isaku!

RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Kay, Allen M]

Isaku,

I cannot apply the patch below because format problems.  Can you resend it as a attachment?

Allen

-----Original Message-----
From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Isaku Yamahata
Sent: Sunday, June 13, 2010 8:30 PM
To: Stefano Stabellini
Cc: xen-devel@lists.xensource.com; Kay, Allen M; Han, Weidong; Jean Guyader; Ian Pratt; Ross Philipson
Subject: Re: [Xen-devel] [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

On Fri, Jun 11, 2010 at 12:08:29PM +0100, Stefano Stabellini wrote:
> On Tue, 8 Jun 2010, Kay, Allen M wrote:
> > Isaku,
> > 
> > Thanks for the feedback.
> > 
> > > pci_{read, write}_block() would be better than
> > > switch(len) case 1: case 2: case4:...
> > 
> > Done!
> > 
> > > Is it really necessary to move PCIBus and PCIBridge to the header file?
> > > Doesn't pci_bridge_init() work?
> > 
> > I changed to code to utilize pci_bridge_init().  However, I still need to move
> > PCIBus and PCIBridge defines to the header file.  The alternative is to pollute
> > pc.c with graphics passthrough specific code.
> > 
> > > Overriding pci config read/write methods of i440fx would be much cleaner
> > > than hooking pci_data_read/write. (pass igd_pci_read/write to 
> > > pci_register_device() in i440fx_init() in hw/piix_pci.c instead of NULL)
> > 
> > Doing this resulted in a lot of duplicated code and also force code path to change
> > even when IGD passthrough is not used.  To do it correctly, I also need to
> > put in IGD or PASSTHROGH awareness in piix_pci.c.  For now, I'm going to keep
> > the original patch.
> > 
> 
> I see.
> 
> Even though the patch is an improvement over what we currently have in
> qemu-xen, it is still not acceptable in upstream qemu as it is.
> Considering that we are pretty close to have the merge complete, I think
> it worth trying to come up with something cleaner.
> 
> Isaku, you are the latest one to touch the pci_host code in qemu, do you
> have any other suggestion?

How about the following patch which is on top of the v2 patch?
I did only compile test, but I think it's enough to show my idea.

For PCI bridge, I have a patches to clean up/enhance the implementation and
I'm planning to push to the qemu upstream soon.


>From 9a22525e523e834ab3822d14e715388c19a7d02e Mon Sep 17 00:00:00 2001
Message-Id: <9a22525e523e834ab3822d14e715388c19a7d02e.1276485897.git.yamahata@valinux.co.jp>
In-Reply-To: <cover.1276485897.git.yamahata@valinux.co.jp>
References: <cover.1276485897.git.yamahata@valinux.co.jp>
From: Isaku Yamahata <yamahata@valinux.co.jp>
Date: Mon, 14 Jun 2010 12:24:31 +0900
Subject: [PATCH] pci passthrough: some clean up and unexport PCIBus and PCIBridge.

some clean up and unexport PCIBus and PCIBridge.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pass-through.h |   10 ++++++++--
 hw/pci.c          |   29 +++++++++++++++++++++++------
 hw/pci.h          |   23 -----------------------
 hw/piix_pci.c     |    4 ++--
 hw/pt-graphics.c  |   32 +++++++++++++++++++-------------
 5 files changed, 52 insertions(+), 46 deletions(-)

diff --git a/hw/pass-through.h b/hw/pass-through.h
index 242d079..adc9f58 100644
--- a/hw/pass-through.h
+++ b/hw/pass-through.h
@@ -409,13 +409,19 @@ uint8_t pci_intx(struct pt_dev *ptdev);
 u32 pt_pci_host_read(int bus, int dev, int fn, u32 addr, int len);
 int pt_pci_host_write(int bus, int dev, int fn, u32 addr, u32 val, int len);
 void intel_pch_init(PCIBus *bus);
-int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len);
-int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len);
 int register_vga_regions(struct pt_dev *real_device);
 int unregister_vga_regions(struct pt_dev *real_device);
 int setup_vga_pt(struct pt_dev *real_device);
 PCIBus *intel_pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid,
            uint16_t did, const char *name, uint16_t revision);
 
+#ifdef CONFIG_PASSTHROUGH
+void igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len);
+uint32_t igd_pci_read(PCIDevice *pci_dev, int config_addr, int len);
+#else
+#define igd_pci_write(pci_dev, config_addr, val, len)   NULL
+#define igd_pci_read(pci_dev, config_addr, val, len)    NULL
+#endif
+
 #endif /* __PASSTHROUGH_H__ */
 
diff --git a/hw/pci.c b/hw/pci.c
index a5cb378..b6400f3 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -39,6 +39,24 @@ extern int igd_passthru;
 
 //#define DEBUG_PCI
 
+struct PCIBus {
+    int bus_num;
+    int devfn_min;
+    pci_set_irq_fn set_irq;
+    pci_map_irq_fn map_irq;
+    uint32_t config_reg; /* XXX: suppress */
+    /* low level pic */
+    SetIRQFunc *low_set_irq;
+    qemu_irq *irq_opaque;
+    PCIDevice *devices[256];
+    PCIDevice *parent_dev;
+    PCIBus *next;
+    /* The bus IRQ state is the logical OR of the connected devices.
+       Keep a count of the number of devices with raised IRQs.  */
+    int nirq;
+    int irq_count[];
+};
+
 static void pci_update_mappings(PCIDevice *d);
 static void pci_set_irq(void *opaque, int irq_num, int level);
 
@@ -573,9 +591,6 @@ void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len)
            pci_dev->name, config_addr, val, len);
 #endif
 
-#ifdef CONFIG_PASSTHROUGH
-    if (igd_pci_write(pci_dev, config_addr, val, len) == 0)
-#endif
     pci_dev->config_write(pci_dev, config_addr, val, len);
 }
 
@@ -610,9 +625,6 @@ uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
     }
     config_addr = addr & 0xff;
 
-#ifdef CONFIG_PASSTHROUGH
-    if ( igd_pci_read(pci_dev, config_addr, &val, len) == 0)
-#endif
     val = pci_dev->config_read(pci_dev, config_addr, len);
 
  done_config_read:
@@ -860,6 +872,11 @@ void pci_unplug_netifs(void)
     }
 }
 
+typedef struct {
+    PCIDevice dev;
+    PCIBus *bus;
+} PCIBridge;
+
 void pci_bridge_write_config(PCIDevice *d,
                              uint32_t address, uint32_t val, int len)
 {
diff --git a/hw/pci.h b/hw/pci.h
index 7a44035..8abbad7 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -210,29 +210,6 @@ struct PCIDevice {
 typedef void (*pci_set_irq_fn)(qemu_irq *pic, int irq_num, int level);
 typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
 
-struct PCIBus {
-    int bus_num;
-    int devfn_min;
-    pci_set_irq_fn set_irq;
-    pci_map_irq_fn map_irq;
-    uint32_t config_reg; /* XXX: suppress */
-    /* low level pic */
-    SetIRQFunc *low_set_irq;
-    qemu_irq *irq_opaque;
-    PCIDevice *devices[256];
-    PCIDevice *parent_dev;
-    PCIBus *next;
-    /* The bus IRQ state is the logical OR of the connected devices.
-       Keep a count of the number of devices with raised IRQs.  */
-    int nirq;
-    int irq_count[];
-};
-
-typedef struct {
-    PCIDevice dev;
-    PCIBus *bus;
-} PCIBridge;
-
 extern char direct_pci_str[];
 extern int direct_pci_msitranslate;
 extern int direct_pci_power_mgmt;
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 49d72b2..1bfe0fb 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -25,7 +25,7 @@
 #include "hw.h"
 #include "pc.h"
 #include "pci.h"
-
+#include "pass-through.h"
 
 static void i440fx_set_irq(qemu_irq *pic, int irq_num, int level);
 static void piix3_write_config(PCIDevice *d, 
@@ -207,7 +207,7 @@ PCIBus *i440fx_init(PCIDevice **pi440fx_state, qemu_irq *pic)
     register_ioport_read(0xcfc, 4, 4, pci_host_data_readl, s);
 
     d = pci_register_device(b, "i440FX", sizeof(PCIDevice), 0,
-                            NULL, NULL);
+                            igd_pci_read, igd_pci_write);
 
     pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_INTEL);
     pci_config_set_device_id(d->config, PCI_DEVICE_ID_INTEL_82441);
diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c
index 4923715..32c174d 100644
--- a/hw/pt-graphics.c
+++ b/hw/pt-graphics.c
@@ -8,6 +8,7 @@
 
 #include <unistd.h>
 #include <sys/ioctl.h>
+#include <assert.h>
 
 extern int gfx_passthru;
 extern int igd_passthru;
@@ -34,10 +35,13 @@ void intel_pch_init(PCIBus *bus)
                     pch_map_irq, "intel_bridge_1f");
 }
 
-int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
+void igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
 {
-    if ( !igd_passthru || (pci_dev->devfn != 0x00 ) )
-        return 0;
+    assert(pci_dev->devfn == 0x00);
+    if ( !igd_passthru ) {
+        pci_default_write_config(pci_dev, config_addr, val, len);
+        return;
+    }
 
     switch (config_addr)
     {
@@ -48,15 +52,18 @@ int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
                    PCI_FUNC(pci_dev->devfn), config_addr, len, val);
             break;
         default:
-            pci_dev->config_write(pci_dev, config_addr, val, len);
+            pci_default_write_config(pci_dev, config_addr, val, len);
     }
-    return 1;
 }
 
-int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len)
+uint32_t igd_pci_read(PCIDevice *pci_dev, int config_addr, int len)
 {
-    if ( !igd_passthru || (pci_dev->devfn != 0) )
-        return 0;
+    uint32_t val;
+
+    assert(pci_dev->devfn == 0x00);
+    if ( !igd_passthru ) {
+        return pci_default_read_config(pci_dev, config_addr, len);
+    }
 
     switch (config_addr)
     {
@@ -68,17 +75,16 @@ int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len)
         case 0x58:        /* SNB: PAVPC Offset */
         case 0xa4:        /* SNB: graphics base of stolen memory */
         case 0xa8:        /* SNB: base of GTT stolen memory */
-            *val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
+            val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
                                    0, config_addr, len);
             PT_LOG("pci_config_read: %x:%x.%x: addr=%x len=%x val=%x\n",
                    pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn),
-                   PCI_FUNC(pci_dev->devfn), config_addr, len, *val);
-
+                   PCI_FUNC(pci_dev->devfn), config_addr, len, val);
             break;
         default:
-            *val = pci_dev->config_read(pci_dev, config_addr, len);
+            val = pci_default_read_config(pci_dev, config_addr, len);
     }
-    return 1;
+    return val;
 }
 
 /*
-- 
1.6.6.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Isaku Yamahata]

[-- Attachment #1: Type: text/plain, Size: 11109 bytes --]

Attached.

On Tue, Jun 15, 2010 at 05:58:57PM -0700, Kay, Allen M wrote:
> Isaku,
> 
> I cannot apply the patch below because format problems.  Can you resend it as a attachment?
> 
> Allen
> 
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Isaku Yamahata
> Sent: Sunday, June 13, 2010 8:30 PM
> To: Stefano Stabellini
> Cc: xen-devel@lists.xensource.com; Kay, Allen M; Han, Weidong; Jean Guyader; Ian Pratt; Ross Philipson
> Subject: Re: [Xen-devel] [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
> 
> On Fri, Jun 11, 2010 at 12:08:29PM +0100, Stefano Stabellini wrote:
> > On Tue, 8 Jun 2010, Kay, Allen M wrote:
> > > Isaku,
> > > 
> > > Thanks for the feedback.
> > > 
> > > > pci_{read, write}_block() would be better than
> > > > switch(len) case 1: case 2: case4:...
> > > 
> > > Done!
> > > 
> > > > Is it really necessary to move PCIBus and PCIBridge to the header file?
> > > > Doesn't pci_bridge_init() work?
> > > 
> > > I changed to code to utilize pci_bridge_init().  However, I still need to move
> > > PCIBus and PCIBridge defines to the header file.  The alternative is to pollute
> > > pc.c with graphics passthrough specific code.
> > > 
> > > > Overriding pci config read/write methods of i440fx would be much cleaner
> > > > than hooking pci_data_read/write. (pass igd_pci_read/write to 
> > > > pci_register_device() in i440fx_init() in hw/piix_pci.c instead of NULL)
> > > 
> > > Doing this resulted in a lot of duplicated code and also force code path to change
> > > even when IGD passthrough is not used.  To do it correctly, I also need to
> > > put in IGD or PASSTHROGH awareness in piix_pci.c.  For now, I'm going to keep
> > > the original patch.
> > > 
> > 
> > I see.
> > 
> > Even though the patch is an improvement over what we currently have in
> > qemu-xen, it is still not acceptable in upstream qemu as it is.
> > Considering that we are pretty close to have the merge complete, I think
> > it worth trying to come up with something cleaner.
> > 
> > Isaku, you are the latest one to touch the pci_host code in qemu, do you
> > have any other suggestion?
> 
> How about the following patch which is on top of the v2 patch?
> I did only compile test, but I think it's enough to show my idea.
> 
> For PCI bridge, I have a patches to clean up/enhance the implementation and
> I'm planning to push to the qemu upstream soon.
> 
> 
> >From 9a22525e523e834ab3822d14e715388c19a7d02e Mon Sep 17 00:00:00 2001
> Message-Id: <9a22525e523e834ab3822d14e715388c19a7d02e.1276485897.git.yamahata@valinux.co.jp>
> In-Reply-To: <cover.1276485897.git.yamahata@valinux.co.jp>
> References: <cover.1276485897.git.yamahata@valinux.co.jp>
> From: Isaku Yamahata <yamahata@valinux.co.jp>
> Date: Mon, 14 Jun 2010 12:24:31 +0900
> Subject: [PATCH] pci passthrough: some clean up and unexport PCIBus and PCIBridge.
> 
> some clean up and unexport PCIBus and PCIBridge.
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
>  hw/pass-through.h |   10 ++++++++--
>  hw/pci.c          |   29 +++++++++++++++++++++++------
>  hw/pci.h          |   23 -----------------------
>  hw/piix_pci.c     |    4 ++--
>  hw/pt-graphics.c  |   32 +++++++++++++++++++-------------
>  5 files changed, 52 insertions(+), 46 deletions(-)
> 
> diff --git a/hw/pass-through.h b/hw/pass-through.h
> index 242d079..adc9f58 100644
> --- a/hw/pass-through.h
> +++ b/hw/pass-through.h
> @@ -409,13 +409,19 @@ uint8_t pci_intx(struct pt_dev *ptdev);
>  u32 pt_pci_host_read(int bus, int dev, int fn, u32 addr, int len);
>  int pt_pci_host_write(int bus, int dev, int fn, u32 addr, u32 val, int len);
>  void intel_pch_init(PCIBus *bus);
> -int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len);
> -int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len);
>  int register_vga_regions(struct pt_dev *real_device);
>  int unregister_vga_regions(struct pt_dev *real_device);
>  int setup_vga_pt(struct pt_dev *real_device);
>  PCIBus *intel_pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid,
>             uint16_t did, const char *name, uint16_t revision);
>  
> +#ifdef CONFIG_PASSTHROUGH
> +void igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len);
> +uint32_t igd_pci_read(PCIDevice *pci_dev, int config_addr, int len);
> +#else
> +#define igd_pci_write(pci_dev, config_addr, val, len)   NULL
> +#define igd_pci_read(pci_dev, config_addr, val, len)    NULL
> +#endif
> +
>  #endif /* __PASSTHROUGH_H__ */
>  
> diff --git a/hw/pci.c b/hw/pci.c
> index a5cb378..b6400f3 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -39,6 +39,24 @@ extern int igd_passthru;
>  
>  //#define DEBUG_PCI
>  
> +struct PCIBus {
> +    int bus_num;
> +    int devfn_min;
> +    pci_set_irq_fn set_irq;
> +    pci_map_irq_fn map_irq;
> +    uint32_t config_reg; /* XXX: suppress */
> +    /* low level pic */
> +    SetIRQFunc *low_set_irq;
> +    qemu_irq *irq_opaque;
> +    PCIDevice *devices[256];
> +    PCIDevice *parent_dev;
> +    PCIBus *next;
> +    /* The bus IRQ state is the logical OR of the connected devices.
> +       Keep a count of the number of devices with raised IRQs.  */
> +    int nirq;
> +    int irq_count[];
> +};
> +
>  static void pci_update_mappings(PCIDevice *d);
>  static void pci_set_irq(void *opaque, int irq_num, int level);
>  
> @@ -573,9 +591,6 @@ void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len)
>             pci_dev->name, config_addr, val, len);
>  #endif
>  
> -#ifdef CONFIG_PASSTHROUGH
> -    if (igd_pci_write(pci_dev, config_addr, val, len) == 0)
> -#endif
>      pci_dev->config_write(pci_dev, config_addr, val, len);
>  }
>  
> @@ -610,9 +625,6 @@ uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
>      }
>      config_addr = addr & 0xff;
>  
> -#ifdef CONFIG_PASSTHROUGH
> -    if ( igd_pci_read(pci_dev, config_addr, &val, len) == 0)
> -#endif
>      val = pci_dev->config_read(pci_dev, config_addr, len);
>  
>   done_config_read:
> @@ -860,6 +872,11 @@ void pci_unplug_netifs(void)
>      }
>  }
>  
> +typedef struct {
> +    PCIDevice dev;
> +    PCIBus *bus;
> +} PCIBridge;
> +
>  void pci_bridge_write_config(PCIDevice *d,
>                               uint32_t address, uint32_t val, int len)
>  {
> diff --git a/hw/pci.h b/hw/pci.h
> index 7a44035..8abbad7 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -210,29 +210,6 @@ struct PCIDevice {
>  typedef void (*pci_set_irq_fn)(qemu_irq *pic, int irq_num, int level);
>  typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
>  
> -struct PCIBus {
> -    int bus_num;
> -    int devfn_min;
> -    pci_set_irq_fn set_irq;
> -    pci_map_irq_fn map_irq;
> -    uint32_t config_reg; /* XXX: suppress */
> -    /* low level pic */
> -    SetIRQFunc *low_set_irq;
> -    qemu_irq *irq_opaque;
> -    PCIDevice *devices[256];
> -    PCIDevice *parent_dev;
> -    PCIBus *next;
> -    /* The bus IRQ state is the logical OR of the connected devices.
> -       Keep a count of the number of devices with raised IRQs.  */
> -    int nirq;
> -    int irq_count[];
> -};
> -
> -typedef struct {
> -    PCIDevice dev;
> -    PCIBus *bus;
> -} PCIBridge;
> -
>  extern char direct_pci_str[];
>  extern int direct_pci_msitranslate;
>  extern int direct_pci_power_mgmt;
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 49d72b2..1bfe0fb 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -25,7 +25,7 @@
>  #include "hw.h"
>  #include "pc.h"
>  #include "pci.h"
> -
> +#include "pass-through.h"
>  
>  static void i440fx_set_irq(qemu_irq *pic, int irq_num, int level);
>  static void piix3_write_config(PCIDevice *d, 
> @@ -207,7 +207,7 @@ PCIBus *i440fx_init(PCIDevice **pi440fx_state, qemu_irq *pic)
>      register_ioport_read(0xcfc, 4, 4, pci_host_data_readl, s);
>  
>      d = pci_register_device(b, "i440FX", sizeof(PCIDevice), 0,
> -                            NULL, NULL);
> +                            igd_pci_read, igd_pci_write);
>  
>      pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_INTEL);
>      pci_config_set_device_id(d->config, PCI_DEVICE_ID_INTEL_82441);
> diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c
> index 4923715..32c174d 100644
> --- a/hw/pt-graphics.c
> +++ b/hw/pt-graphics.c
> @@ -8,6 +8,7 @@
>  
>  #include <unistd.h>
>  #include <sys/ioctl.h>
> +#include <assert.h>
>  
>  extern int gfx_passthru;
>  extern int igd_passthru;
> @@ -34,10 +35,13 @@ void intel_pch_init(PCIBus *bus)
>                      pch_map_irq, "intel_bridge_1f");
>  }
>  
> -int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
> +void igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
>  {
> -    if ( !igd_passthru || (pci_dev->devfn != 0x00 ) )
> -        return 0;
> +    assert(pci_dev->devfn == 0x00);
> +    if ( !igd_passthru ) {
> +        pci_default_write_config(pci_dev, config_addr, val, len);
> +        return;
> +    }
>  
>      switch (config_addr)
>      {
> @@ -48,15 +52,18 @@ int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
>                     PCI_FUNC(pci_dev->devfn), config_addr, len, val);
>              break;
>          default:
> -            pci_dev->config_write(pci_dev, config_addr, val, len);
> +            pci_default_write_config(pci_dev, config_addr, val, len);
>      }
> -    return 1;
>  }
>  
> -int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len)
> +uint32_t igd_pci_read(PCIDevice *pci_dev, int config_addr, int len)
>  {
> -    if ( !igd_passthru || (pci_dev->devfn != 0) )
> -        return 0;
> +    uint32_t val;
> +
> +    assert(pci_dev->devfn == 0x00);
> +    if ( !igd_passthru ) {
> +        return pci_default_read_config(pci_dev, config_addr, len);
> +    }
>  
>      switch (config_addr)
>      {
> @@ -68,17 +75,16 @@ int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len)
>          case 0x58:        /* SNB: PAVPC Offset */
>          case 0xa4:        /* SNB: graphics base of stolen memory */
>          case 0xa8:        /* SNB: base of GTT stolen memory */
> -            *val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
> +            val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
>                                     0, config_addr, len);
>              PT_LOG("pci_config_read: %x:%x.%x: addr=%x len=%x val=%x\n",
>                     pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn),
> -                   PCI_FUNC(pci_dev->devfn), config_addr, len, *val);
> -
> +                   PCI_FUNC(pci_dev->devfn), config_addr, len, val);
>              break;
>          default:
> -            *val = pci_dev->config_read(pci_dev, config_addr, len);
> +            val = pci_default_read_config(pci_dev, config_addr, len);
>      }
> -    return 1;
> +    return val;
>  }
>  
>  /*
> -- 
> 1.6.6.1
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 

-- 
yamahata

[-- Attachment #2: 0002-pci-passthrough-some-clean-up-and-unexport-PCIBus-an.patch --]
[-- Type: text/plain, Size: 7979 bytes --]

>From 9a22525e523e834ab3822d14e715388c19a7d02e Mon Sep 17 00:00:00 2001
Message-Id: <9a22525e523e834ab3822d14e715388c19a7d02e.1276485897.git.yamahata@valinux.co.jp>
In-Reply-To: <cover.1276485897.git.yamahata@valinux.co.jp>
References: <cover.1276485897.git.yamahata@valinux.co.jp>
From: Isaku Yamahata <yamahata@valinux.co.jp>
Date: Mon, 14 Jun 2010 12:24:31 +0900
Subject: [PATCH] pci passthrough: some clean up and unexport PCIBus and PCIBridge.

some clean up and unexport PCIBus and PCIBridge.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pass-through.h |   10 ++++++++--
 hw/pci.c          |   29 +++++++++++++++++++++++------
 hw/pci.h          |   23 -----------------------
 hw/piix_pci.c     |    4 ++--
 hw/pt-graphics.c  |   32 +++++++++++++++++++-------------
 5 files changed, 52 insertions(+), 46 deletions(-)

diff --git a/hw/pass-through.h b/hw/pass-through.h
index 242d079..adc9f58 100644
--- a/hw/pass-through.h
+++ b/hw/pass-through.h
@@ -409,13 +409,19 @@ uint8_t pci_intx(struct pt_dev *ptdev);
 u32 pt_pci_host_read(int bus, int dev, int fn, u32 addr, int len);
 int pt_pci_host_write(int bus, int dev, int fn, u32 addr, u32 val, int len);
 void intel_pch_init(PCIBus *bus);
-int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len);
-int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len);
 int register_vga_regions(struct pt_dev *real_device);
 int unregister_vga_regions(struct pt_dev *real_device);
 int setup_vga_pt(struct pt_dev *real_device);
 PCIBus *intel_pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid,
            uint16_t did, const char *name, uint16_t revision);
 
+#ifdef CONFIG_PASSTHROUGH
+void igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len);
+uint32_t igd_pci_read(PCIDevice *pci_dev, int config_addr, int len);
+#else
+#define igd_pci_write(pci_dev, config_addr, val, len)   NULL
+#define igd_pci_read(pci_dev, config_addr, val, len)    NULL
+#endif
+
 #endif /* __PASSTHROUGH_H__ */
 
diff --git a/hw/pci.c b/hw/pci.c
index a5cb378..b6400f3 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -39,6 +39,24 @@ extern int igd_passthru;
 
 //#define DEBUG_PCI
 
+struct PCIBus {
+    int bus_num;
+    int devfn_min;
+    pci_set_irq_fn set_irq;
+    pci_map_irq_fn map_irq;
+    uint32_t config_reg; /* XXX: suppress */
+    /* low level pic */
+    SetIRQFunc *low_set_irq;
+    qemu_irq *irq_opaque;
+    PCIDevice *devices[256];
+    PCIDevice *parent_dev;
+    PCIBus *next;
+    /* The bus IRQ state is the logical OR of the connected devices.
+       Keep a count of the number of devices with raised IRQs.  */
+    int nirq;
+    int irq_count[];
+};
+
 static void pci_update_mappings(PCIDevice *d);
 static void pci_set_irq(void *opaque, int irq_num, int level);
 
@@ -573,9 +591,6 @@ void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len)
            pci_dev->name, config_addr, val, len);
 #endif
 
-#ifdef CONFIG_PASSTHROUGH
-    if (igd_pci_write(pci_dev, config_addr, val, len) == 0)
-#endif
     pci_dev->config_write(pci_dev, config_addr, val, len);
 }
 
@@ -610,9 +625,6 @@ uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
     }
     config_addr = addr & 0xff;
 
-#ifdef CONFIG_PASSTHROUGH
-    if ( igd_pci_read(pci_dev, config_addr, &val, len) == 0)
-#endif
     val = pci_dev->config_read(pci_dev, config_addr, len);
 
  done_config_read:
@@ -860,6 +872,11 @@ void pci_unplug_netifs(void)
     }
 }
 
+typedef struct {
+    PCIDevice dev;
+    PCIBus *bus;
+} PCIBridge;
+
 void pci_bridge_write_config(PCIDevice *d,
                              uint32_t address, uint32_t val, int len)
 {
diff --git a/hw/pci.h b/hw/pci.h
index 7a44035..8abbad7 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -210,29 +210,6 @@ struct PCIDevice {
 typedef void (*pci_set_irq_fn)(qemu_irq *pic, int irq_num, int level);
 typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
 
-struct PCIBus {
-    int bus_num;
-    int devfn_min;
-    pci_set_irq_fn set_irq;
-    pci_map_irq_fn map_irq;
-    uint32_t config_reg; /* XXX: suppress */
-    /* low level pic */
-    SetIRQFunc *low_set_irq;
-    qemu_irq *irq_opaque;
-    PCIDevice *devices[256];
-    PCIDevice *parent_dev;
-    PCIBus *next;
-    /* The bus IRQ state is the logical OR of the connected devices.
-       Keep a count of the number of devices with raised IRQs.  */
-    int nirq;
-    int irq_count[];
-};
-
-typedef struct {
-    PCIDevice dev;
-    PCIBus *bus;
-} PCIBridge;
-
 extern char direct_pci_str[];
 extern int direct_pci_msitranslate;
 extern int direct_pci_power_mgmt;
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 49d72b2..1bfe0fb 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -25,7 +25,7 @@
 #include "hw.h"
 #include "pc.h"
 #include "pci.h"
-
+#include "pass-through.h"
 
 static void i440fx_set_irq(qemu_irq *pic, int irq_num, int level);
 static void piix3_write_config(PCIDevice *d, 
@@ -207,7 +207,7 @@ PCIBus *i440fx_init(PCIDevice **pi440fx_state, qemu_irq *pic)
     register_ioport_read(0xcfc, 4, 4, pci_host_data_readl, s);
 
     d = pci_register_device(b, "i440FX", sizeof(PCIDevice), 0,
-                            NULL, NULL);
+                            igd_pci_read, igd_pci_write);
 
     pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_INTEL);
     pci_config_set_device_id(d->config, PCI_DEVICE_ID_INTEL_82441);
diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c
index 4923715..32c174d 100644
--- a/hw/pt-graphics.c
+++ b/hw/pt-graphics.c
@@ -8,6 +8,7 @@
 
 #include <unistd.h>
 #include <sys/ioctl.h>
+#include <assert.h>
 
 extern int gfx_passthru;
 extern int igd_passthru;
@@ -34,10 +35,13 @@ void intel_pch_init(PCIBus *bus)
                     pch_map_irq, "intel_bridge_1f");
 }
 
-int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
+void igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
 {
-    if ( !igd_passthru || (pci_dev->devfn != 0x00 ) )
-        return 0;
+    assert(pci_dev->devfn == 0x00);
+    if ( !igd_passthru ) {
+        pci_default_write_config(pci_dev, config_addr, val, len);
+        return;
+    }
 
     switch (config_addr)
     {
@@ -48,15 +52,18 @@ int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
                    PCI_FUNC(pci_dev->devfn), config_addr, len, val);
             break;
         default:
-            pci_dev->config_write(pci_dev, config_addr, val, len);
+            pci_default_write_config(pci_dev, config_addr, val, len);
     }
-    return 1;
 }
 
-int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len)
+uint32_t igd_pci_read(PCIDevice *pci_dev, int config_addr, int len)
 {
-    if ( !igd_passthru || (pci_dev->devfn != 0) )
-        return 0;
+    uint32_t val;
+
+    assert(pci_dev->devfn == 0x00);
+    if ( !igd_passthru ) {
+        return pci_default_read_config(pci_dev, config_addr, len);
+    }
 
     switch (config_addr)
     {
@@ -68,17 +75,16 @@ int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len)
         case 0x58:        /* SNB: PAVPC Offset */
         case 0xa4:        /* SNB: graphics base of stolen memory */
         case 0xa8:        /* SNB: base of GTT stolen memory */
-            *val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
+            val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
                                    0, config_addr, len);
             PT_LOG("pci_config_read: %x:%x.%x: addr=%x len=%x val=%x\n",
                    pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn),
-                   PCI_FUNC(pci_dev->devfn), config_addr, len, *val);
-
+                   PCI_FUNC(pci_dev->devfn), config_addr, len, val);
             break;
         default:
-            *val = pci_dev->config_read(pci_dev, config_addr, len);
+            val = pci_default_read_config(pci_dev, config_addr, len);
     }
-    return 1;
+    return val;
 }
 
 /*
-- 
1.6.6.1


[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Kay, Allen M]

Isaku/Stafano,

Just want to report that this patch worked without any problems.  As long as everyone is OK with hooking igd_pci_read/write functions in piix_pci.c, I'm fine with it too.  I wasn't sure about it earlier ...

Allen

-----Original Message-----
From: Isaku Yamahata [mailto:yamahata@valinux.co.jp] 
Sent: Tuesday, June 15, 2010 7:08 PM
To: Kay, Allen M
Cc: Stefano Stabellini; xen-devel@lists.xensource.com; Han, Weidong; Jean Guyader; Ian Pratt; Ross Philipson
Subject: Re: [Xen-devel] [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

Attached.

On Tue, Jun 15, 2010 at 05:58:57PM -0700, Kay, Allen M wrote:
> Isaku,
> 
> I cannot apply the patch below because format problems.  Can you resend it as a attachment?
> 
> Allen
> 
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com 
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Isaku 
> Yamahata
> Sent: Sunday, June 13, 2010 8:30 PM
> To: Stefano Stabellini
> Cc: xen-devel@lists.xensource.com; Kay, Allen M; Han, Weidong; Jean 
> Guyader; Ian Pratt; Ross Philipson
> Subject: Re: [Xen-devel] [PATCH][VTD] enable integrated graphics 
> passthrough for Calpella and Sandybridge
> 
> On Fri, Jun 11, 2010 at 12:08:29PM +0100, Stefano Stabellini wrote:
> > On Tue, 8 Jun 2010, Kay, Allen M wrote:
> > > Isaku,
> > > 
> > > Thanks for the feedback.
> > > 
> > > > pci_{read, write}_block() would be better than
> > > > switch(len) case 1: case 2: case4:...
> > > 
> > > Done!
> > > 
> > > > Is it really necessary to move PCIBus and PCIBridge to the header file?
> > > > Doesn't pci_bridge_init() work?
> > > 
> > > I changed to code to utilize pci_bridge_init().  However, I still 
> > > need to move PCIBus and PCIBridge defines to the header file.  The 
> > > alternative is to pollute pc.c with graphics passthrough specific code.
> > > 
> > > > Overriding pci config read/write methods of i440fx would be much 
> > > > cleaner than hooking pci_data_read/write. (pass 
> > > > igd_pci_read/write to
> > > > pci_register_device() in i440fx_init() in hw/piix_pci.c instead 
> > > > of NULL)
> > > 
> > > Doing this resulted in a lot of duplicated code and also force 
> > > code path to change even when IGD passthrough is not used.  To do 
> > > it correctly, I also need to put in IGD or PASSTHROGH awareness in 
> > > piix_pci.c.  For now, I'm going to keep the original patch.
> > > 
> > 
> > I see.
> > 
> > Even though the patch is an improvement over what we currently have 
> > in qemu-xen, it is still not acceptable in upstream qemu as it is.
> > Considering that we are pretty close to have the merge complete, I 
> > think it worth trying to come up with something cleaner.
> > 
> > Isaku, you are the latest one to touch the pci_host code in qemu, do 
> > you have any other suggestion?
> 
> How about the following patch which is on top of the v2 patch?
> I did only compile test, but I think it's enough to show my idea.
> 
> For PCI bridge, I have a patches to clean up/enhance the 
> implementation and I'm planning to push to the qemu upstream soon.
> 
> 
> >From 9a22525e523e834ab3822d14e715388c19a7d02e Mon Sep 17 00:00:00 
> >2001
> Message-Id: 
> <9a22525e523e834ab3822d14e715388c19a7d02e.1276485897.git.yamahata@vali
> nux.co.jp>
> In-Reply-To: <cover.1276485897.git.yamahata@valinux.co.jp>
> References: <cover.1276485897.git.yamahata@valinux.co.jp>
> From: Isaku Yamahata <yamahata@valinux.co.jp>
> Date: Mon, 14 Jun 2010 12:24:31 +0900
> Subject: [PATCH] pci passthrough: some clean up and unexport PCIBus and PCIBridge.
> 
> some clean up and unexport PCIBus and PCIBridge.
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
>  hw/pass-through.h |   10 ++++++++--
>  hw/pci.c          |   29 +++++++++++++++++++++++------
>  hw/pci.h          |   23 -----------------------
>  hw/piix_pci.c     |    4 ++--
>  hw/pt-graphics.c  |   32 +++++++++++++++++++-------------
>  5 files changed, 52 insertions(+), 46 deletions(-)
> 
> diff --git a/hw/pass-through.h b/hw/pass-through.h index 
> 242d079..adc9f58 100644
> --- a/hw/pass-through.h
> +++ b/hw/pass-through.h
> @@ -409,13 +409,19 @@ uint8_t pci_intx(struct pt_dev *ptdev);
>  u32 pt_pci_host_read(int bus, int dev, int fn, u32 addr, int len);  
> int pt_pci_host_write(int bus, int dev, int fn, u32 addr, u32 val, int 
> len);  void intel_pch_init(PCIBus *bus); -int igd_pci_write(PCIDevice 
> *pci_dev, int config_addr, uint32_t val, int len); -int 
> igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int 
> len);  int register_vga_regions(struct pt_dev *real_device);  int 
> unregister_vga_regions(struct pt_dev *real_device);  int 
> setup_vga_pt(struct pt_dev *real_device);  PCIBus 
> *intel_pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid,
>             uint16_t did, const char *name, uint16_t revision);
>  
> +#ifdef CONFIG_PASSTHROUGH
> +void igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, 
> +int len); uint32_t igd_pci_read(PCIDevice *pci_dev, int config_addr, 
> +int len); #else
> +#define igd_pci_write(pci_dev, config_addr, val, len)   NULL
> +#define igd_pci_read(pci_dev, config_addr, val, len)    NULL
> +#endif
> +
>  #endif /* __PASSTHROUGH_H__ */
>  
> diff --git a/hw/pci.c b/hw/pci.c
> index a5cb378..b6400f3 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -39,6 +39,24 @@ extern int igd_passthru;
>  
>  //#define DEBUG_PCI
>  
> +struct PCIBus {
> +    int bus_num;
> +    int devfn_min;
> +    pci_set_irq_fn set_irq;
> +    pci_map_irq_fn map_irq;
> +    uint32_t config_reg; /* XXX: suppress */
> +    /* low level pic */
> +    SetIRQFunc *low_set_irq;
> +    qemu_irq *irq_opaque;
> +    PCIDevice *devices[256];
> +    PCIDevice *parent_dev;
> +    PCIBus *next;
> +    /* The bus IRQ state is the logical OR of the connected devices.
> +       Keep a count of the number of devices with raised IRQs.  */
> +    int nirq;
> +    int irq_count[];
> +};
> +
>  static void pci_update_mappings(PCIDevice *d);  static void 
> pci_set_irq(void *opaque, int irq_num, int level);
>  
> @@ -573,9 +591,6 @@ void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len)
>             pci_dev->name, config_addr, val, len);  #endif
>  
> -#ifdef CONFIG_PASSTHROUGH
> -    if (igd_pci_write(pci_dev, config_addr, val, len) == 0)
> -#endif
>      pci_dev->config_write(pci_dev, config_addr, val, len);  }
>  
> @@ -610,9 +625,6 @@ uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
>      }
>      config_addr = addr & 0xff;
>  
> -#ifdef CONFIG_PASSTHROUGH
> -    if ( igd_pci_read(pci_dev, config_addr, &val, len) == 0)
> -#endif
>      val = pci_dev->config_read(pci_dev, config_addr, len);
>  
>   done_config_read:
> @@ -860,6 +872,11 @@ void pci_unplug_netifs(void)
>      }
>  }
>  
> +typedef struct {
> +    PCIDevice dev;
> +    PCIBus *bus;
> +} PCIBridge;
> +
>  void pci_bridge_write_config(PCIDevice *d,
>                               uint32_t address, uint32_t val, int len)  
> { diff --git a/hw/pci.h b/hw/pci.h index 7a44035..8abbad7 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -210,29 +210,6 @@ struct PCIDevice {  typedef void 
> (*pci_set_irq_fn)(qemu_irq *pic, int irq_num, int level);  typedef int 
> (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
>  
> -struct PCIBus {
> -    int bus_num;
> -    int devfn_min;
> -    pci_set_irq_fn set_irq;
> -    pci_map_irq_fn map_irq;
> -    uint32_t config_reg; /* XXX: suppress */
> -    /* low level pic */
> -    SetIRQFunc *low_set_irq;
> -    qemu_irq *irq_opaque;
> -    PCIDevice *devices[256];
> -    PCIDevice *parent_dev;
> -    PCIBus *next;
> -    /* The bus IRQ state is the logical OR of the connected devices.
> -       Keep a count of the number of devices with raised IRQs.  */
> -    int nirq;
> -    int irq_count[];
> -};
> -
> -typedef struct {
> -    PCIDevice dev;
> -    PCIBus *bus;
> -} PCIBridge;
> -
>  extern char direct_pci_str[];
>  extern int direct_pci_msitranslate;
>  extern int direct_pci_power_mgmt;
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c index 49d72b2..1bfe0fb 
> 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -25,7 +25,7 @@
>  #include "hw.h"
>  #include "pc.h"
>  #include "pci.h"
> -
> +#include "pass-through.h"
>  
>  static void i440fx_set_irq(qemu_irq *pic, int irq_num, int level);  
> static void piix3_write_config(PCIDevice *d, @@ -207,7 +207,7 @@ 
> PCIBus *i440fx_init(PCIDevice **pi440fx_state, qemu_irq *pic)
>      register_ioport_read(0xcfc, 4, 4, pci_host_data_readl, s);
>  
>      d = pci_register_device(b, "i440FX", sizeof(PCIDevice), 0,
> -                            NULL, NULL);
> +                            igd_pci_read, igd_pci_write);
>  
>      pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_INTEL);
>      pci_config_set_device_id(d->config, PCI_DEVICE_ID_INTEL_82441); 
> diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c index 
> 4923715..32c174d 100644
> --- a/hw/pt-graphics.c
> +++ b/hw/pt-graphics.c
> @@ -8,6 +8,7 @@
>  
>  #include <unistd.h>
>  #include <sys/ioctl.h>
> +#include <assert.h>
>  
>  extern int gfx_passthru;
>  extern int igd_passthru;
> @@ -34,10 +35,13 @@ void intel_pch_init(PCIBus *bus)
>                      pch_map_irq, "intel_bridge_1f");  }
>  
> -int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, 
> int len)
> +void igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, 
> +int len)
>  {
> -    if ( !igd_passthru || (pci_dev->devfn != 0x00 ) )
> -        return 0;
> +    assert(pci_dev->devfn == 0x00);
> +    if ( !igd_passthru ) {
> +        pci_default_write_config(pci_dev, config_addr, val, len);
> +        return;
> +    }
>  
>      switch (config_addr)
>      {
> @@ -48,15 +52,18 @@ int igd_pci_write(PCIDevice *pci_dev, int config_addr, uint32_t val, int len)
>                     PCI_FUNC(pci_dev->devfn), config_addr, len, val);
>              break;
>          default:
> -            pci_dev->config_write(pci_dev, config_addr, val, len);
> +            pci_default_write_config(pci_dev, config_addr, val, len);
>      }
> -    return 1;
>  }
>  
> -int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, 
> int len)
> +uint32_t igd_pci_read(PCIDevice *pci_dev, int config_addr, int len)
>  {
> -    if ( !igd_passthru || (pci_dev->devfn != 0) )
> -        return 0;
> +    uint32_t val;
> +
> +    assert(pci_dev->devfn == 0x00);
> +    if ( !igd_passthru ) {
> +        return pci_default_read_config(pci_dev, config_addr, len);
> +    }
>  
>      switch (config_addr)
>      {
> @@ -68,17 +75,16 @@ int igd_pci_read(PCIDevice *pci_dev, int config_addr, uint32_t *val, int len)
>          case 0x58:        /* SNB: PAVPC Offset */
>          case 0xa4:        /* SNB: graphics base of stolen memory */
>          case 0xa8:        /* SNB: base of GTT stolen memory */
> -            *val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
> +            val = pt_pci_host_read(0, PCI_SLOT(pci_dev->devfn),
>                                     0, config_addr, len);
>              PT_LOG("pci_config_read: %x:%x.%x: addr=%x len=%x val=%x\n",
>                     pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn),
> -                   PCI_FUNC(pci_dev->devfn), config_addr, len, *val);
> -
> +                   PCI_FUNC(pci_dev->devfn), config_addr, len, val);
>              break;
>          default:
> -            *val = pci_dev->config_read(pci_dev, config_addr, len);
> +            val = pci_default_read_config(pci_dev, config_addr, len);
>      }
> -    return 1;
> +    return val;
>  }
>  
>  /*
> --
> 1.6.6.1
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 

--
yamahata

RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge

[Stefano Stabellini]

On Fri, 18 Jun 2010, Kay, Allen M wrote:
> Isaku/Stafano,
> 
> Just want to report that this patch worked without any problems.  As long as everyone is OK with hooking igd_pci_read/write functions in piix_pci.c, I'm fine with it too.  I wasn't sure about it earlier ...
> 

It is OK by me.