* [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
@ 2010-06-04 0:18 Kay, Allen M
2010-06-04 15:29 ` Stefano Stabellini
2010-06-07 7:45 ` Isaku Yamahata
0 siblings, 2 replies; 16+ messages in thread
From: Kay, Allen M @ 2010-06-04 0:18 UTC (permalink / raw)
To: xen-devel; +Cc: ian.pratt, Han, Weidong, Ross.Philipson, jean.guyader
[-- 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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-04 0:18 [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge Kay, Allen M
@ 2010-06-04 15:29 ` Stefano Stabellini
2010-06-08 21:46 ` Kay, Allen M
2010-06-07 7:45 ` Isaku Yamahata
1 sibling, 1 reply; 16+ messages in thread
From: Stefano Stabellini @ 2010-06-04 15:29 UTC (permalink / raw)
To: Kay, Allen M; +Cc: xen-devel, Han, Weidong, Jean Guyader, Ian Pratt, Philipson
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-04 0:18 [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge Kay, Allen M
2010-06-04 15:29 ` Stefano Stabellini
@ 2010-06-07 7:45 ` Isaku Yamahata
2010-06-08 21:15 ` Kay, Allen M
1 sibling, 1 reply; 16+ messages in thread
From: Isaku Yamahata @ 2010-06-07 7:45 UTC (permalink / raw)
To: Kay, Allen M
Cc: ian.pratt, xen-devel, Han, Weidong, jean.guyader, Ross.Philipson
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-07 7:45 ` Isaku Yamahata
@ 2010-06-08 21:15 ` Kay, Allen M
2010-06-10 13:21 ` Stefano Stabellini
2010-06-11 11:08 ` Stefano Stabellini
0 siblings, 2 replies; 16+ messages in thread
From: Kay, Allen M @ 2010-06-08 21:15 UTC (permalink / raw)
To: Isaku Yamahata
Cc: ian.pratt, xen-devel, Han, Weidong, jean.guyader, Ross.Philipson
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-04 15:29 ` Stefano Stabellini
@ 2010-06-08 21:46 ` Kay, Allen M
2010-06-09 15:57 ` Stefano Stabellini
0 siblings, 1 reply; 16+ messages in thread
From: Kay, Allen M @ 2010-06-08 21:46 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, Han, Weidong, Jean Guyader, Ian Pratt, Philipson
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-08 21:46 ` Kay, Allen M
@ 2010-06-09 15:57 ` Stefano Stabellini
2010-06-10 14:17 ` Jean Guyader
0 siblings, 1 reply; 16+ messages in thread
From: Stefano Stabellini @ 2010-06-09 15:57 UTC (permalink / raw)
To: Kay, Allen M
Cc: xen-devel, Stefano Stabellini, Han, Weidong, Jean Guyader,
Ian Pratt, Philipson
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);
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-08 21:15 ` Kay, Allen M
@ 2010-06-10 13:21 ` Stefano Stabellini
2010-06-10 16:46 ` Kay, Allen M
2010-06-11 11:08 ` Stefano Stabellini
1 sibling, 1 reply; 16+ messages in thread
From: Stefano Stabellini @ 2010-06-10 13:21 UTC (permalink / raw)
To: Kay, Allen M
Cc: xen-devel, Han, Weidong, Guyader, Isaku Yamahata, Ian Pratt,
Ross Philipson
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?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-09 15:57 ` Stefano Stabellini
@ 2010-06-10 14:17 ` Jean Guyader
0 siblings, 0 replies; 16+ messages in thread
From: Jean Guyader @ 2010-06-10 14:17 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, Kay, Allen M, Han, Weidong, Jean Guyader, Ian Pratt,
Philipson
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-10 13:21 ` Stefano Stabellini
@ 2010-06-10 16:46 ` Kay, Allen M
0 siblings, 0 replies; 16+ messages in thread
From: Kay, Allen M @ 2010-06-10 16:46 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, Han, Weidong, Guyader, Isaku Yamahata, Ian Pratt,
Ross Philipson
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-08 21:15 ` Kay, Allen M
2010-06-10 13:21 ` Stefano Stabellini
@ 2010-06-11 11:08 ` Stefano Stabellini
2010-06-14 3:30 ` Isaku Yamahata
1 sibling, 1 reply; 16+ messages in thread
From: Stefano Stabellini @ 2010-06-11 11:08 UTC (permalink / raw)
To: Kay, Allen M
Cc: xen-devel, Han, Weidong, Guyader, Isaku Yamahata, Ian Pratt,
Ross Philipson
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?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-11 11:08 ` Stefano Stabellini
@ 2010-06-14 3:30 ` Isaku Yamahata
2010-06-14 11:02 ` Stefano Stabellini
2010-06-16 0:58 ` Kay, Allen M
0 siblings, 2 replies; 16+ messages in thread
From: Isaku Yamahata @ 2010-06-14 3:30 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, Kay, Allen M, Han, Weidong, Jean Guyader, Ian Pratt,
Ross Philipson
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-14 3:30 ` Isaku Yamahata
@ 2010-06-14 11:02 ` Stefano Stabellini
2010-06-16 0:58 ` Kay, Allen M
1 sibling, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2010-06-14 11:02 UTC (permalink / raw)
To: Isaku Yamahata
Cc: xen-devel, Stefano Stabellini, Kay, Allen M, Han, Weidong,
Jean Guyader, Ian Pratt, Ross Philipson
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!
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-14 3:30 ` Isaku Yamahata
2010-06-14 11:02 ` Stefano Stabellini
@ 2010-06-16 0:58 ` Kay, Allen M
2010-06-16 2:08 ` Isaku Yamahata
1 sibling, 1 reply; 16+ messages in thread
From: Kay, Allen M @ 2010-06-16 0:58 UTC (permalink / raw)
To: Isaku Yamahata, Stefano Stabellini
Cc: Ian, Han, Weidong, Jean Guyader, Pratt, xen-devel, Ross Philipson
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-16 0:58 ` Kay, Allen M
@ 2010-06-16 2:08 ` Isaku Yamahata
2010-06-18 0:01 ` Kay, Allen M
0 siblings, 1 reply; 16+ messages in thread
From: Isaku Yamahata @ 2010-06-16 2:08 UTC (permalink / raw)
To: Kay, Allen M
Cc: xen-devel, Stefano Stabellini, Han, Weidong, Jean Guyader,
Ian Pratt, Ross Philipson
[-- 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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-16 2:08 ` Isaku Yamahata
@ 2010-06-18 0:01 ` Kay, Allen M
2010-06-18 11:40 ` Stefano Stabellini
0 siblings, 1 reply; 16+ messages in thread
From: Kay, Allen M @ 2010-06-18 0:01 UTC (permalink / raw)
To: Isaku Yamahata
Cc: Ian, Stefano Stabellini, Han, Weidong, Jean Guyader, Pratt,
xen-devel, Ross Philipson
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge
2010-06-18 0:01 ` Kay, Allen M
@ 2010-06-18 11:40 ` Stefano Stabellini
0 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2010-06-18 11:40 UTC (permalink / raw)
To: Kay, Allen M
Cc: xen-devel, Stefano Stabellini, Han, Weidong, Jean Guyader,
Isaku Yamahata, Ian Pratt, Ross Philipson
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2010-06-18 11:40 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-04 0:18 [PATCH][VTD] enable integrated graphics passthrough for Calpella and Sandybridge Kay, Allen M
2010-06-04 15:29 ` Stefano Stabellini
2010-06-08 21:46 ` Kay, Allen M
2010-06-09 15:57 ` Stefano Stabellini
2010-06-10 14:17 ` Jean Guyader
2010-06-07 7:45 ` Isaku Yamahata
2010-06-08 21:15 ` Kay, Allen M
2010-06-10 13:21 ` Stefano Stabellini
2010-06-10 16:46 ` Kay, Allen M
2010-06-11 11:08 ` Stefano Stabellini
2010-06-14 3:30 ` Isaku Yamahata
2010-06-14 11:02 ` Stefano Stabellini
2010-06-16 0:58 ` Kay, Allen M
2010-06-16 2:08 ` Isaku Yamahata
2010-06-18 0:01 ` Kay, Allen M
2010-06-18 11:40 ` Stefano Stabellini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.