[PATCH][RFC]Move PCI Configuration Spaces from Dom0 to Xen

[PATCH][RFC]Move PCI Configuration Spaces from Dom0 to Xen

[Shan, Haitao]

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

Hi, Keir,

This patch will move reading and writing of PCI configuration spaces
from dom0 to Xen. It also changes VTD code, so that they can touch the
PCI configuration spaces with proper lock.
This will also benefit MSI support in Xen.
Can you give some comments? Thanks!
 <<pci_conf_xen.patch>> 

Best Regards
Haitao Shan


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

[-- Attachment #2: pci_conf_xen.patch --]
[-- Type: application/octet-stream, Size: 29744 bytes --]

diff -r 4e02b9ea9c69 xen/arch/x86/Makefile
--- a/xen/arch/x86/Makefile	Fri Apr 11 00:11:26 2008 +0800
+++ b/xen/arch/x86/Makefile	Fri Apr 11 00:11:28 2008 +0800
@@ -48,6 +48,7 @@ obj-y += machine_kexec.o
 obj-y += machine_kexec.o
 obj-y += crash.o
 obj-y += tboot.o
+obj-y += pci_conf.o
 
 obj-$(crash_debug) += gdbstub.o
 
diff -r 4e02b9ea9c69 xen/arch/x86/domain_build.c
--- a/xen/arch/x86/domain_build.c	Fri Apr 11 00:11:26 2008 +0800
+++ b/xen/arch/x86/domain_build.c	Fri Apr 11 00:11:28 2008 +0800
@@ -957,6 +957,9 @@ int __init construct_dom0(
     rc |= ioports_deny_access(dom0, 0x40, 0x43);
     /* PIT Channel 2 / PC Speaker Control. */
     rc |= ioports_deny_access(dom0, 0x61, 0x61);
+    /* PCI configuration spaces. */
+    rc |= ioports_deny_access(dom0, 0xcf8, 0xcf8);
+    rc |= ioports_deny_access(dom0, 0xcfc, 0xcff);
     /* Command-line I/O ranges. */
     process_dom0_ioports_disable();
 
diff -r 4e02b9ea9c69 xen/arch/x86/pci_conf.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/arch/x86/pci_conf.c	Fri Apr 11 00:11:28 2008 +0800
@@ -0,0 +1,140 @@
+/*
+ * pci_conf.c - Low-level direct PCI config space access
+ */
+
+#include <xen/sched.h>
+#include <asm/types.h>
+#include <asm/pci_conf.h>
+
+#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
+    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
+
+DEFINE_SPINLOCK(pci_config_lock);
+
+int pci_conf_read(unsigned int bus, unsigned int dev, unsigned int func,
+                  int reg, int len, u32 *value)
+{
+    unsigned long flags;
+
+    if ( (bus > 255) || (dev > 32) || (func > 8) || (reg > 255) )
+    {
+        *value = -1;
+        return -EINVAL;
+    }
+
+    spin_lock_irqsave(&pci_config_lock, flags);
+
+    outl(PCI_CONF_ADDRESS(bus, dev, func, reg), 0xCF8);
+
+    switch ( len )
+    {
+    case 1:
+        *value = inb(0xCFC + (reg & 3));
+        break;
+    case 2:
+        *value = inw(0xCFC + (reg & 2));
+        break;
+    case 4:
+        *value = inl(0xCFC);
+        break;
+    }
+
+    spin_unlock_irqrestore(&pci_config_lock, flags);
+
+    return 0;
+}
+
+int pci_conf_write(unsigned int bus, unsigned int dev, unsigned int func,
+                   int reg, int len, u32 value)
+{
+    unsigned long flags;
+
+    if ( (bus > 255) || (dev > 32) || (func > 255) || (reg > 255) )
+        return -EINVAL;
+
+    spin_lock_irqsave(&pci_config_lock, flags);
+
+    outl(PCI_CONF_ADDRESS(bus, dev, func, reg), 0xCF8);
+
+    switch ( len )
+    {
+    case 1:
+        outb((u8)value, 0xCFC + (reg & 3));
+        break;
+    case 2:
+        outw((u16)value, 0xCFC + (reg & 2));
+        break;
+    case 4:
+        outl((u32)value, 0xCFC);
+        break;
+    }
+
+    spin_unlock_irqrestore(&pci_config_lock, flags);
+
+    return 0;
+}
+
+u32 pv_pci_conf_read(struct vcpu *v, unsigned int offset, unsigned int len)
+{
+    unsigned long flags;
+    u32 value = -1;
+
+    ASSERT(IS_PRIV(v->domain));
+
+    spin_lock_irqsave(&pci_config_lock, flags);
+
+    outl(v->domain->arch.shadow_ioport_cf8, 0xCF8);
+
+    if ( offset + len > 4 )
+        gdprintk(XENLOG_WARNING, "Domain %d's access to PCI configuration \
+                 space is beyond range [0xCFC,0xCFF]\n", v->domain->domain_id);
+
+    switch ( len )
+    {
+    case 1:
+        value = inb(0xCFC + offset);
+        break;
+    case 2:
+        value = inw(0xCFC + offset);
+        break;
+    case 4:
+        value = inl(0xCFC + offset);
+        break;
+    }
+
+    spin_unlock_irqrestore(&pci_config_lock, flags);
+
+    return value;
+}
+
+void pv_pci_conf_write(struct vcpu *v, unsigned int offset,
+                       unsigned int len, u32 value)
+{
+    unsigned long flags;
+
+    ASSERT(IS_PRIV(v->domain));
+
+    if ( offset + len > 4 )
+        gdprintk(XENLOG_WARNING, "Domain %d's access to PCI configuration \
+                 space is beyond range [0xCFC,0xCFF]\n", v->domain->domain_id);
+
+    spin_lock_irqsave(&pci_config_lock, flags);
+
+    outl(v->domain->arch.shadow_ioport_cf8, 0xCF8);
+
+    switch ( len )
+    {
+    case 1:
+        outb((u8)value, 0xCFC + offset);
+        break;
+    case 2:
+        outw((u16)value, 0xCFC + offset);
+        break;
+    case 4:
+        outl(value, 0xCFC + offset);
+        break;
+    }
+
+    spin_unlock_irqrestore(&pci_config_lock, flags);
+}
+
diff -r 4e02b9ea9c69 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c	Fri Apr 11 00:11:26 2008 +0800
+++ b/xen/arch/x86/traps.c	Fri Apr 11 00:11:28 2008 +0800
@@ -63,6 +63,7 @@
 #include <asm/x86_emulate.h>
 #include <asm/hvm/vpt.h>
 #include <public/arch-x86/cpuid.h>
+#include <asm/pci_conf.h>
 
 /*
  * opt_nmi: one of 'ignore', 'dom0', or 'fatal'.
@@ -1612,13 +1613,25 @@ static int emulate_privileged_op(struct 
                 /* emulate PIT counter 2 */
                 data = (u8)(guest_inb_okay(port, v, regs) ? inb(port) : 
                        ((port == 0x42 || port == 0x43 || port == 0x61) ?
-                       pv_pit_handler(port, 0, 0) : ~0)); 
+                       pv_pit_handler(port, 0, 0) : ~0));
+                if ( IS_PRIV(v->domain) && port >= 0xcfc && port <= 0xcff )
+                    data = (u8)pv_pci_conf_read(v, port - 0xcfc, 1);
+                else if ( IS_PRIV(v->domain) && port == 0xcf8 )
+                    data = (u8)inb(port);
                 break;
             case 2:
                 data = (u16)(guest_inw_okay(port, v, regs) ? inw(port) : ~0);
+                if ( IS_PRIV(v->domain) && port >= 0xcfc && port <= 0xcff )
+                    data = (u16)pv_pci_conf_read(v, port - 0xcfc, 2);
+                else if ( IS_PRIV(v->domain) && port == 0xcf8 )
+                    data = (u16)inw(port);
                 break;
             case 4:
                 data = (u32)(guest_inl_okay(port, v, regs) ? inl(port) : ~0);
+                if ( IS_PRIV(v->domain) && port >= 0xcfc && port <= 0xcff )
+                    data = (u32)pv_pci_conf_read(v, port - 0xcfc, 4);
+                else if ( IS_PRIV(v->domain) && port == 0xcf8 )
+                    data = (u32)v->domain->arch.shadow_ioport_cf8;
                 break;
             }
             if ( (rc = copy_to_user((void *)data_base + rd_ad(edi), &data, op_bytes)) != 0 )
@@ -1654,14 +1667,31 @@ static int emulate_privileged_op(struct 
                 }
                 else if ( port == 0x42 || port == 0x43 || port == 0x61 )
                     pv_pit_handler(port, data, 1);
+                else if ( IS_PRIV(v->domain) && port >=0xcfc && port <= 0xcff )
+                    pv_pci_conf_write(v, port - 0xcfc, 1, (u8)data);
+                else if ( IS_PRIV(v->domain) && port ==0xcf8 )
+                    outb((u8)data, port);
                 break;
             case 2:
                 if ( guest_outw_okay(port, v, regs) )
+                    outw((u16)data, port);
+                else if ( IS_PRIV(v->domain) && port >=0xcfc && port <= 0xcff )
+                    pv_pci_conf_write(v, port - 0xcfc, 2, (u16)data);
+                else if ( IS_PRIV(v->domain) && port ==0xcf8 )
                     outw((u16)data, port);
                 break;
             case 4:
                 if ( guest_outl_okay(port, v, regs) )
                     outl((u32)data, port);
+                else if ( IS_PRIV(v->domain) && port >=0xcfc && port <= 0xcff )
+                    pv_pci_conf_write(v, port - 0xcfc, 4, data);
+                else if ( IS_PRIV(v->domain) && port ==0xcf8 )
+                {
+                    if ( data & 0x80000000 )
+                        v->domain->arch.shadow_ioport_cf8 = (u32)data;
+                    else
+                        outl((u32)data, port);
+                }
                 break;
             }
             wr_ad(esi, regs->esi + (int)((regs->eflags & EF_DF) ? -op_bytes : op_bytes));
@@ -1737,19 +1767,49 @@ static int emulate_privileged_op(struct 
             {
                 regs->eax &= ~0xffUL;
                 regs->eax |= pv_pit_handler(port, 0, 0);
-            } 
+            }
+            else if ( IS_PRIV(v->domain) && port >= 0xcfc && port <= 0xcff )
+            {
+                regs->eax &= ~0xffUL;
+                regs->eax |= (u8)pv_pci_conf_read(v, port - 0xcfc, 1);
+            }
+            else if ( IS_PRIV(v->domain) && port == 0xcf8 )
+            {
+                regs->eax &= ~0xffUL;
+                regs->eax |= (u8)inb(0xcf8);
+            }
             else
                 regs->eax |= (u8)~0;
             break;
         case 2:
             if ( guest_inw_okay(port, v, regs) )
                 io_emul(regs);
+            else if ( IS_PRIV(v->domain) && port >= 0xcfc && port <= 0xcff )
+            {
+                regs->eax &= ~0xffffUL;
+                regs->eax |= (u16)pv_pci_conf_read(v, port - 0xcfc, 2);
+            }
+            else if ( IS_PRIV(v->domain) && port == 0xcf8 )
+            {
+                regs->eax &= ~0xffffUL;
+                regs->eax |= (u16)inw(0xcf8);
+            }
             else
                 regs->eax |= (u16)~0;
             break;
         case 4:
             if ( guest_inl_okay(port, v, regs) )
                 io_emul(regs);
+            else if ( IS_PRIV(v->domain) && port >= 0xcfc && port <= 0xcff )
+            {
+                regs->eax &= ~0xffffffffUL;
+                regs->eax |= pv_pci_conf_read(v, port - 0xcfc, 4);
+            }
+            else if ( IS_PRIV(v->domain) && port == 0xcf8 )
+            {
+                regs->eax &= ~0xffffffffUL;
+                regs->eax |= v->domain->arch.shadow_ioport_cf8;
+            }
             else
                 regs->eax = (u32)~0;
             break;
@@ -1782,14 +1842,31 @@ static int emulate_privileged_op(struct 
             }
             else if ( port == 0x42 || port == 0x43 || port == 0x61 )
                 pv_pit_handler(port, regs->eax, 1);
+            else if ( IS_PRIV(v->domain) && port ==0xcfc && port <= 0xcff )
+                pv_pci_conf_write(v, port - 0xcfc, 1, (u8)regs->eax);
+            else if ( IS_PRIV(v->domain) && port ==0xcf8 )
+                outb((u8)regs->eax, port);
             break;
         case 2:
             if ( guest_outw_okay(port, v, regs) )
                 io_emul(regs);
+            else if ( IS_PRIV(v->domain) && port ==0xcfc && port <= 0xcff )
+                pv_pci_conf_write(v, port - 0xcfc, 2, (u16)regs->eax);
+            else if ( IS_PRIV(v->domain) && port ==0xcf8 )
+                outw((u16)regs->eax, port);
             break;
         case 4:
             if ( guest_outl_okay(port, v, regs) )
                 io_emul(regs);
+            else if ( IS_PRIV(v->domain) && port >=0xcfc && port <= 0xcff )
+                pv_pci_conf_write(v, port - 0xcfc, 4, (u32)regs->eax);
+            else if ( IS_PRIV(v->domain) && port ==0xcf8 )
+            {
+                if ( regs->eax & 0x80000000 )
+                    v->domain->arch.shadow_ioport_cf8 = (u32)regs->eax;
+                else
+                    outl((u32)regs->eax, port);
+            }
             break;
         }
         bpmatch = check_guest_io_breakpoint(v, port, op_bytes);
diff -r 4e02b9ea9c69 xen/drivers/passthrough/amd/iommu_detect.c
--- a/xen/drivers/passthrough/amd/iommu_detect.c	Fri Apr 11 00:11:26 2008 +0800
+++ b/xen/drivers/passthrough/amd/iommu_detect.c	Fri Apr 11 00:11:28 2008 +0800
@@ -31,9 +31,9 @@ static int __init valid_bridge_bus_confi
 {
     int pri_bus;
 
-    pri_bus = read_pci_config_byte(bus, dev, func, PCI_PRIMARY_BUS);
-    *sec_bus = read_pci_config_byte(bus, dev, func, PCI_SECONDARY_BUS);
-    *sub_bus = read_pci_config_byte(bus, dev, func, PCI_SUBORDINATE_BUS);
+    pri_bus = read_dpci_config_byte(bus, dev, func, PCI_PRIMARY_BUS);
+    *sec_bus = read_dpci_config_byte(bus, dev, func, PCI_SECONDARY_BUS);
+    *sub_bus = read_dpci_config_byte(bus, dev, func, PCI_SUBORDINATE_BUS);
 
     return ((pri_bus == bus) && (*sec_bus > bus) && (*sub_bus >= *sec_bus));
 }
@@ -60,10 +60,10 @@ int __init get_iommu_last_downstream_bus
         func = PCI_FUNC(devfn);
  
         if ( !VALID_PCI_VENDOR_ID(
-            read_pci_config_16(bus, dev, func, PCI_VENDOR_ID)) )
+            read_dpci_config_word(bus, dev, func, PCI_VENDOR_ID)) )
             continue;
 
-        hdr_type = read_pci_config_byte(bus, dev, func,
+        hdr_type = read_dpci_config_byte(bus, dev, func,
                                         PCI_HEADER_TYPE);
         if ( func == 0 )
             multi_func = IS_PCI_MULTI_FUNCTION(hdr_type);
@@ -92,9 +92,9 @@ int __init get_iommu_capabilities(u8 bus
     u32 cap_header, cap_range, misc_info;
     u64 mmio_bar;
 
-    mmio_bar = (u64)read_pci_config(
+    mmio_bar = (u64)read_dpci_config_dw(
         bus, dev, func, cap_ptr + PCI_CAP_MMIO_BAR_HIGH_OFFSET) << 32;
-    mmio_bar |= read_pci_config(bus, dev, func,
+    mmio_bar |= read_dpci_config_dw(bus, dev, func,
                                 cap_ptr + PCI_CAP_MMIO_BAR_LOW_OFFSET);
     iommu->mmio_base_phys = mmio_bar & (u64)~0x3FFF;
 
@@ -108,7 +108,7 @@ int __init get_iommu_capabilities(u8 bus
     iommu->bdf = (bus << 8) | PCI_DEVFN(dev, func);
     iommu->cap_offset = cap_ptr;
 
-    cap_header = read_pci_config(bus, dev, func, cap_ptr);
+    cap_header = read_dpci_config_dw(bus, dev, func, cap_ptr);
     iommu->revision = get_field_from_reg_u32(
         cap_header, PCI_CAP_REV_MASK, PCI_CAP_REV_SHIFT);
     iommu->iotlb_support = get_field_from_reg_u32(
@@ -118,7 +118,7 @@ int __init get_iommu_capabilities(u8 bus
     iommu->pte_not_present_cached = get_field_from_reg_u32(
         cap_header, PCI_CAP_NP_CACHE_MASK, PCI_CAP_NP_CACHE_SHIFT);
 
-    cap_range = read_pci_config(bus, dev, func,
+    cap_range = read_dpci_config_dw(bus, dev, func,
                                 cap_ptr + PCI_CAP_RANGE_OFFSET);
     iommu->unit_id = get_field_from_reg_u32(
         cap_range, PCI_CAP_UNIT_ID_MASK, PCI_CAP_UNIT_ID_SHIFT);
@@ -129,7 +129,7 @@ int __init get_iommu_capabilities(u8 bus
     iommu->last_devfn = get_field_from_reg_u32(
         cap_range, PCI_CAP_LAST_DEVICE_MASK, PCI_CAP_LAST_DEVICE_SHIFT);
 
-    misc_info = read_pci_config(bus, dev, func,
+    misc_info = read_dpci_config_dw(bus, dev, func,
                                 cap_ptr + PCI_MISC_INFO_OFFSET);
     iommu->msi_number = get_field_from_reg_u32(
         misc_info, PCI_CAP_MSI_NUMBER_MASK, PCI_CAP_MSI_NUMBER_SHIFT);
@@ -146,14 +146,14 @@ static int __init scan_caps_for_iommu(
     int count, error = 0;
 
     count = 0;
-    cap_ptr = read_pci_config_byte(bus, dev, func,
+    cap_ptr = read_dpci_config_byte(bus, dev, func,
                                    PCI_CAPABILITY_LIST);
     while ( (cap_ptr >= PCI_MIN_CAP_OFFSET) &&
             (count < PCI_MAX_CAP_BLOCKS) &&
             !error )
     {
         cap_ptr &= PCI_CAP_PTR_MASK;
-        cap_header = read_pci_config(bus, dev, func, cap_ptr);
+        cap_header = read_dpci_config_dw(bus, dev, func, cap_ptr);
         cap_id = get_field_from_reg_u32(
             cap_header, PCI_CAP_ID_MASK, PCI_CAP_ID_SHIFT);
 
@@ -182,11 +182,11 @@ static int __init scan_functions_for_iom
 
     func = 0;
     count = 1;
-    while ( VALID_PCI_VENDOR_ID(read_pci_config_16(bus, dev, func,
+    while ( VALID_PCI_VENDOR_ID(read_dpci_config_word(bus, dev, func,
                                                    PCI_VENDOR_ID)) &&
             !error && (func < count) )
     {
-        hdr_type = read_pci_config_byte(bus, dev, func,
+        hdr_type = read_dpci_config_byte(bus, dev, func,
                                         PCI_HEADER_TYPE);
 
         if ( func == 0 && IS_PCI_MULTI_FUNCTION(hdr_type) )
diff -r 4e02b9ea9c69 xen/drivers/passthrough/amd/pci_amd_iommu.c
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c	Fri Apr 11 00:11:26 2008 +0800
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c	Fri Apr 11 00:11:28 2008 +0800
@@ -316,7 +316,7 @@ void __init amd_iommu_setup_dom0_devices
         {
             for ( func = 0; func < 8; func++ )
             {
-                l = read_pci_config(bus, dev, func, PCI_VENDOR_ID);
+                l = read_dpci_config_dw(bus, dev, func, PCI_VENDOR_ID);
                 /* some broken boards return 0 or ~0 if a slot is empty: */
                 if ( (l == 0xffffffff) || (l == 0x00000000) ||
                      (l == 0x0000ffff) || (l == 0xffff0000) )
diff -r 4e02b9ea9c69 xen/drivers/passthrough/pci-direct.h
--- a/xen/drivers/passthrough/pci-direct.h	Fri Apr 11 00:11:26 2008 +0800
+++ b/xen/drivers/passthrough/pci-direct.h	Fri Apr 11 00:11:28 2008 +0800
@@ -3,46 +3,46 @@
 
 #include <xen/types.h>
 #include <asm/io.h>
+#include <asm/pci_conf.h>
 
 /* Direct PCI access. This is used for PCI accesses in early boot before
    the PCI subsystem works. */
 
 #define PDprintk(x...)
 
-static inline u32 read_pci_config(u8 bus, u8 slot, u8 func, u8 offset)
+static inline u8 read_dpci_config_byte(u8 bus, u8 slot, u8 func, u8 offset)
 {
-    u32 v;
-    outl(0x80000000 | (bus<<16) | (slot<<11) | (func<<8) | offset, 0xcf8);
-    v = inl(0xcfc);
-    if (v != 0xffffffff)
-        PDprintk("%x reading 4 from %x: %x\n", slot, offset, v);
-    return v;
+    u32 value;
+
+    pci_conf_read(bus, slot, func, offset, 1, &value);
+    PDprintk("%x reading 1 from %x: %x\n", slot, offset, value);
+    return (u8)value;
 }
 
-static inline u8 read_pci_config_byte(u8 bus, u8 slot, u8 func, u8 offset)
+static inline u16 read_dpci_config_word(u8 bus, u8 slot, u8 func, u8 offset)
 {
-    u8 v;
-    outl(0x80000000 | (bus<<16) | (slot<<11) | (func<<8) | offset, 0xcf8);
-    v = inb(0xcfc + (offset&3));
-    PDprintk("%x reading 1 from %x: %x\n", slot, offset, v);
-    return v;
+    u32 value;
+
+    pci_conf_read(bus, slot, func, offset, 2, &value);
+    PDprintk("%x reading 1 from %x: %x\n", slot, offset, value);
+    return (u16)value;
 }
 
-static inline u16 read_pci_config_16(u8 bus, u8 slot, u8 func, u8 offset)
+static inline u32 read_dpci_config_dw(u8 bus, u8 slot, u8 func, u8 offset)
 {
-    u16 v;
-    outl(0x80000000 | (bus<<16) | (slot<<11) | (func<<8) | offset, 0xcf8);
-    v = inw(0xcfc + (offset&2));
-    PDprintk("%x reading 2 from %x: %x\n", slot, offset, v);
-    return v;
+    u32 value;
+
+    pci_conf_read(bus, slot, func, offset, 4, &value);
+    if ( value != 0xffffffff )
+        PDprintk("%x reading 4 from %x: %x\n", slot, offset, value);
+    return value;
 }
 
-static inline void write_pci_config(
+static inline void write_dpci_config(
     u8 bus, u8 slot, u8 func, u8 offset, u32 val)
 {
     PDprintk("%x writing to %x: %x\n", slot, offset, val);
-    outl(0x80000000 | (bus<<16) | (slot<<11) | (func<<8) | offset, 0xcf8);
-    outl(val, 0xcfc);
+    pci_conf_write(bus, slot, func, offset, 4, val);
 }
 
 #endif
diff -r 4e02b9ea9c69 xen/drivers/passthrough/vtd/dmar.c
--- a/xen/drivers/passthrough/vtd/dmar.c	Fri Apr 11 00:11:26 2008 +0800
+++ b/xen/drivers/passthrough/vtd/dmar.c	Fri Apr 11 00:11:28 2008 +0800
@@ -233,7 +233,7 @@ static int scope_device_count(void *star
 		    / sizeof(struct acpi_pci_path);
         while ( --depth >= 0 )
         {
-            bus = read_pci_config_byte(
+            bus = read_dpci_config_byte(
                 bus, path->dev, path->fn, PCI_SECONDARY_BUS);
             path++;
         }
@@ -250,9 +250,9 @@ static int scope_device_count(void *star
             dprintk(XENLOG_INFO VTDPREFIX,
                     "found bridge: bdf = %x:%x:%x\n",
                     bus, path->dev, path->fn);
-            sec_bus = read_pci_config_byte(
+            sec_bus = read_dpci_config_byte(
                 bus, path->dev, path->fn, PCI_SECONDARY_BUS);
-            sub_bus = read_pci_config_byte(
+            sub_bus = read_dpci_config_byte(
                 bus, path->dev, path->fn, PCI_SUBORDINATE_BUS);
 
             while ( sec_bus <= sub_bus )
@@ -261,7 +261,7 @@ static int scope_device_count(void *star
                 {
                     for ( func = 0; func < 8; func++ )
                     {
-                        l = read_pci_config(
+                        l = read_dpci_config_dw(
                             sec_bus, dev, func, PCI_VENDOR_ID);
 
                         /* some broken boards return 0 or
@@ -355,7 +355,7 @@ static int __init acpi_parse_dev_scope(
 
         while ( --depth >= 0 )
         {
-            bus = read_pci_config_byte(
+            bus = read_dpci_config_byte(
                 bus, path->dev, path->fn, PCI_SECONDARY_BUS);
             path++;
         }
@@ -374,9 +374,9 @@ static int __init acpi_parse_dev_scope(
             dprintk(XENLOG_INFO VTDPREFIX,
                     "found bridge: bus = %x dev = %x func = %x\n",
                     bus, path->dev, path->fn);
-            sec_bus = read_pci_config_byte(
+            sec_bus = read_dpci_config_byte(
                 bus, path->dev, path->fn, PCI_SECONDARY_BUS);
-            sub_bus = read_pci_config_byte(
+            sub_bus = read_dpci_config_byte(
                 bus, path->dev, path->fn, PCI_SUBORDINATE_BUS);
 
             while ( sec_bus <= sub_bus )
@@ -385,7 +385,7 @@ static int __init acpi_parse_dev_scope(
                 {
                     for ( func = 0; func < 8; func++ )
                     {
-                        l = read_pci_config(
+                        l = read_dpci_config_dw(
                             sec_bus, dev, func, PCI_VENDOR_ID);
 
                         /* some broken boards return 0 or
diff -r 4e02b9ea9c69 xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Fri Apr 11 00:11:26 2008 +0800
+++ b/xen/drivers/passthrough/vtd/iommu.c	Fri Apr 11 00:11:28 2008 +0800
@@ -1241,13 +1241,13 @@ static int __pci_find_next_cap(u8 bus, u
 
     while ( ttl-- )
     {
-        pos = read_pci_config_byte(bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pos);
+        pos = read_dpci_config_byte(bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pos);
         if ( pos < 0x40 )
             break;
 
         pos &= ~3;
-        id = read_pci_config_byte(bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                                  pos + PCI_CAP_LIST_ID);
+        id = read_dpci_config_byte(bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
+                                   pos + PCI_CAP_LIST_ID);
 
         if ( id == 0xff )
             break;
@@ -1271,12 +1271,12 @@ int pdev_type(struct pci_dev *dev)
     u16 class_device;
     u16 status;
 
-    class_device = read_pci_config_16(dev->bus, PCI_SLOT(dev->devfn),
+    class_device = read_dpci_config_word(dev->bus, PCI_SLOT(dev->devfn),
                                       PCI_FUNC(dev->devfn), PCI_CLASS_DEVICE);
     if ( class_device == PCI_CLASS_BRIDGE_PCI )
         return DEV_TYPE_PCI_BRIDGE;
 
-    status = read_pci_config_16(dev->bus, PCI_SLOT(dev->devfn),
+    status = read_dpci_config_word(dev->bus, PCI_SLOT(dev->devfn),
                                 PCI_FUNC(dev->devfn), PCI_STATUS);
 
     if ( !(status & PCI_STATUS_CAP_LIST) )
@@ -1305,7 +1305,7 @@ static int domain_context_mapping(
     switch ( type )
     {
     case DEV_TYPE_PCI_BRIDGE:
-        sec_bus = read_pci_config_byte(
+        sec_bus = read_dpci_config_byte(
             pdev->bus, PCI_SLOT(pdev->devfn),
             PCI_FUNC(pdev->devfn), PCI_SECONDARY_BUS);
 
@@ -1315,7 +1315,7 @@ static int domain_context_mapping(
             bus2bridge[sec_bus].devfn =  pdev->devfn;
         }
 
-        sub_bus = read_pci_config_byte(
+        sub_bus = read_dpci_config_byte(
             pdev->bus, PCI_SLOT(pdev->devfn),
             PCI_FUNC(pdev->devfn), PCI_SUBORDINATE_BUS);
 
@@ -1432,10 +1432,10 @@ static int domain_context_unmap(
     switch ( type )
     {
     case DEV_TYPE_PCI_BRIDGE:
-        sec_bus = read_pci_config_byte(
+        sec_bus = read_dpci_config_byte(
             pdev->bus, PCI_SLOT(pdev->devfn),
             PCI_FUNC(pdev->devfn), PCI_SECONDARY_BUS);
-        sub_bus = read_pci_config_byte(
+        sub_bus = read_dpci_config_byte(
             pdev->bus, PCI_SLOT(pdev->devfn),
             PCI_FUNC(pdev->devfn), PCI_SUBORDINATE_BUS);
 
@@ -1817,7 +1817,7 @@ void __init setup_dom0_devices(void)
         {
             for ( func = 0; func < 8; func++ )
             {
-                l = read_pci_config(bus, dev, func, PCI_VENDOR_ID);
+                l = read_dpci_config_dw(bus, dev, func, PCI_VENDOR_ID);
                 /* some broken boards return 0 or ~0 if a slot is empty: */
                 if ( (l == 0xffffffff) || (l == 0x00000000) ||
                      (l == 0x0000ffff) || (l == 0xffff0000) )
diff -r 4e02b9ea9c69 xen/drivers/passthrough/vtd/utils.c
--- a/xen/drivers/passthrough/vtd/utils.c	Fri Apr 11 00:11:26 2008 +0800
+++ b/xen/drivers/passthrough/vtd/utils.c	Fri Apr 11 00:11:28 2008 +0800
@@ -35,7 +35,7 @@ int is_usb_device(struct pci_dev *pdev)
     u8 bus = pdev->bus;
     u8 dev = PCI_SLOT(pdev->devfn);
     u8 func = PCI_FUNC(pdev->devfn);
-    u16 class = read_pci_config_16(bus, dev, func, PCI_CLASS_DEVICE);
+    u16 class = read_dpci_config_word(bus, dev, func, PCI_CLASS_DEVICE);
     return (class == 0xc03);
 }
 
@@ -44,9 +44,9 @@ int vtd_hw_check(void)
     u16 vendor, device;
     u8 revision, stepping;
 
-    vendor   = read_pci_config_16(0, 0, 0, PCI_VENDOR_ID);
-    device   = read_pci_config_16(0, 0, 0, PCI_DEVICE_ID);
-    revision = read_pci_config_byte(0, 0, 0, PCI_REVISION_ID);
+    vendor   = read_dpci_config_word(0, 0, 0, PCI_VENDOR_ID);
+    device   = read_dpci_config_word(0, 0, 0, PCI_DEVICE_ID);
+    revision = read_dpci_config_byte(0, 0, 0, PCI_REVISION_ID);
     stepping = revision & 0xf;
 
     if ( (vendor == INTEL) && (device == SEABURG) )
@@ -101,18 +101,18 @@ static u8 find_cap_offset(u8 bus, u8 dev
     u8 pos = PCI_CAPABILITY_LIST;
     u16 status;
 
-    status = read_pci_config_16(bus, dev, func, PCI_STATUS);
+    status = read_dpci_config_word(bus, dev, func, PCI_STATUS);
     if ( (status & PCI_STATUS_CAP_LIST) == 0 )
         return 0;
 
     while ( max_cap-- )
     {
-        pos = read_pci_config_byte(bus, dev, func, pos);
+        pos = read_dpci_config_byte(bus, dev, func, pos);
         if ( pos < 0x40 )
             break;
 
         pos &= ~3;
-        id = read_pci_config_byte(bus, dev, func, pos + PCI_CAP_LIST_ID);
+        id = read_dpci_config_byte(bus, dev, func, pos + PCI_CAP_LIST_ID);
 
         if ( id == 0xff )
             break;
@@ -141,13 +141,13 @@ void pdev_flr(u8 bus, u8 devfn)
     pos = find_cap_offset(bus, dev, func, PCI_CAP_ID_EXP);
     if ( pos != 0 )
     {
-        dev_cap = read_pci_config(bus, dev, func, pos + PCI_EXP_DEVCAP);
+        dev_cap = read_dpci_config_dw(bus, dev, func, pos + PCI_EXP_DEVCAP);
         if ( dev_cap & PCI_EXP_DEVCAP_FLR )
         {
-            write_pci_config(bus, dev, func,
+            write_dpci_config(bus, dev, func,
                              pos + PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_FLR);
             do {
-                dev_status = read_pci_config(bus, dev, func,
+                dev_status = read_dpci_config_dw(bus, dev, func,
                                              pos + PCI_EXP_DEVSTA);
             } while ( dev_status & PCI_EXP_DEVSTA_TRPND );
 
@@ -167,23 +167,23 @@ void pdev_flr(u8 bus, u8 devfn)
             int i;
             u32 config[PCI_CONFIG_DWORD_SIZE];
             for ( i = 0; i < PCI_CONFIG_DWORD_SIZE; i++ )
-                config[i] = read_pci_config(bus, dev, func, i*4);
+                config[i] = read_dpci_config_dw(bus, dev, func, i*4);
 
             /* Enter D3hot without soft reset */
-            pm_ctl = read_pci_config(bus, dev, func, pos + PCI_PM_CTRL);
+            pm_ctl = read_dpci_config_dw(bus, dev, func, pos + PCI_PM_CTRL);
             pm_ctl |= PCI_PM_CTRL_NO_SOFT_RESET;
             pm_ctl &= ~PCI_PM_CTRL_STATE_MASK;
             pm_ctl |= PCI_D3hot;
-            write_pci_config(bus, dev, func, pos + PCI_PM_CTRL, pm_ctl);
+            write_dpci_config(bus, dev, func, pos + PCI_PM_CTRL, pm_ctl);
             mdelay(10);
 
             /* From D3hot to D0 */
-            write_pci_config(bus, dev, func, pos + PCI_PM_CTRL, 0);
+            write_dpci_config(bus, dev, func, pos + PCI_PM_CTRL, 0);
             mdelay(10);
 
             /* Write saved configurations to device */
             for ( i = 0; i < PCI_CONFIG_DWORD_SIZE; i++ )
-                write_pci_config(bus, dev, func, i*4, config[i]);
+                write_dpci_config(bus, dev, func, i*4, config[i]);
 
             flr = 1;
         }
diff -r 4e02b9ea9c69 xen/include/asm-x86/domain.h
--- a/xen/include/asm-x86/domain.h	Fri Apr 11 00:11:26 2008 +0800
+++ b/xen/include/asm-x86/domain.h	Fri Apr 11 00:11:28 2008 +0800
@@ -209,6 +209,7 @@ struct arch_domain
 
     /* I/O-port admin-specified access capabilities. */
     struct rangeset *ioport_caps;
+    u32 shadow_ioport_cf8;
 
     struct hvm_domain hvm_domain;
 
diff -r 4e02b9ea9c69 xen/include/asm-x86/pci_conf.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/include/asm-x86/pci_conf.h	Fri Apr 11 00:11:28 2008 +0800
@@ -0,0 +1,18 @@
+/*
+ * pci_conf.h - Low-level direct PCI config space access
+ */
+
+#ifndef __X86_PCI_CONF_H__
+#define __X86_PCI_CONF_H__
+
+#include <xen/sched.h>
+#include <asm/types.h>
+
+int pci_conf_read(unsigned int bus, unsigned int dev, unsigned int func,
+                  int reg, int len, u32 *value);
+int pci_conf_write(unsigned int bus, unsigned int dev, unsigned int func,
+                   int reg, int len, u32 value);
+u32 pv_pci_conf_read(struct vcpu *v, unsigned int offset, unsigned int len);
+void pv_pci_conf_write(struct vcpu *v, unsigned int offset,
+                       unsigned int len, u32 value);
+#endif /* __X86_PCI_CONF_H__ */

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

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

Re: [PATCH][RFC]Move PCI Configuration Spaces from Dom0 to Xen

[Keir Fraser]

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

On 10/4/08 10:45, "Shan, Haitao" <haitao.shan@intel.com> wrote:

> This patch will move reading and writing of PCI configuration spaces from dom0
> to Xen. It also changes VTD code, so that they can touch the PCI configuration
> spaces with proper lock.
> 
> This will also benefit MSI support in Xen.
> Can you give some comments? Thanks!

The approach is fine. I will read it more thoroughly, clean it up a bit if
necessary, and certainly check it in.

 -- Keir


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

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

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

Re: [PATCH][RFC]Move PCI Configuration Spaces from Dom0 to Xen

[Haitao Shan]

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

Thanks, Keir!

2008/4/10, Keir Fraser <keir.fraser@eu.citrix.com>:
>
>  On 10/4/08 10:45, "Shan, Haitao" <haitao.shan@intel.com> wrote:
>
> This patch will move reading and writing of PCI configuration spaces from
> dom0 to Xen. It also changes VTD code, so that they can touch the PCI
> configuration spaces with proper lock.
>
> This will also benefit MSI support in Xen.
> Can you give some comments? Thanks!
>
>
> The approach is fine. I will read it more thoroughly, clean it up a bit if
> necessary, and certainly check it in.
>
>  -- Keir
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>

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

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

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

Re: [PATCH][RFC]Move PCI Configuration Spaces from Dom0 to Xen

[Espen Skoglund]

I believe dword writes to 0xCF8 should always be latched into the
internal shadow register, irrespective of whether the enable bit (bit
31) is set.  Accesseses to 0xCFC can then safely be skipped if enable
bit in latch register is not set.

	eSk



[Haitao Shan]
> Thanks, Keir!
> 2008/4/10, Keir Fraser <keir.fraser@eu.citrix.com>:
>> 
>> On 10/4/08 10:45, "Shan, Haitao" <haitao.shan@intel.com> wrote:
>> 
>> This patch will move reading and writing of PCI configuration
>> spaces from dom0 to Xen. It also changes VTD code, so that they can
>> touch the PCI configuration spaces with proper lock.
>> 
>> This will also benefit MSI support in Xen.
>> Can you give some comments? Thanks!
>> 
>> 
>> The approach is fine. I will read it more thoroughly, clean it up a bit if
>> necessary, and certainly check it in.
>> 
>> -- Keir
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>> 
>> 
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: http://lists.xensource.com/archives/html/xen-devel/attachments/20080410/263aa734/attachment.htm

RE: [PATCH][RFC]Move PCI Configuration Spacesfrom Dom0 to Xen

[Shan, Haitao]

Actually I do not find a clear explanation in MCH specification on this. Luckily, dom0's kernel does not write 0xcf8 without valid bit set.
Does access to 0xCFC return data based on the last valid latched data in 0xCF8, or last latched data without valid bit set in 0xCF8?
For example:
Outl(0x80000000, 0xcf8);
Outl(0x00000001, 0xcf8);
Inl(0xcfc); <----This is skipped? Or data at 00:00.0 register 0.
Another question is:
Outl(0x80000000, 0xcf8);
Inb(0xcfc);
Inb(0xcfd); <----This is skipped? Or data at 00:00.0 register 1?
Can you tell me where do you find these answers? Thanks!

Shan Haitao

-----Original Message-----
From: Espen Skoglund [mailto:espen.skoglund@netronome.com] 
Sent: 2008年4月11日 0:55
To: Haitao Shan
Cc: Keir Fraser; Shan, Haitao; Tian, Kevin; xen-devel; Kay, Allen M; Jiang, Yunhong; Han, Weidong
Subject: Re: [Xen-devel] [PATCH][RFC]Move PCI Configuration Spacesfrom Dom0 to Xen

I believe dword writes to 0xCF8 should always be latched into the
internal shadow register, irrespective of whether the enable bit (bit
31) is set.  Accesseses to 0xCFC can then safely be skipped if enable
bit in latch register is not set.

	eSk



[Haitao Shan]
> Thanks, Keir!
> 2008/4/10, Keir Fraser <keir.fraser@eu.citrix.com>:
>> 
>> On 10/4/08 10:45, "Shan, Haitao" <haitao.shan@intel.com> wrote:
>> 
>> This patch will move reading and writing of PCI configuration
>> spaces from dom0 to Xen. It also changes VTD code, so that they can
>> touch the PCI configuration spaces with proper lock.
>> 
>> This will also benefit MSI support in Xen.
>> Can you give some comments? Thanks!
>> 
>> 
>> The approach is fine. I will read it more thoroughly, clean it up a bit if
>> necessary, and certainly check it in.
>> 
>> -- Keir
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>> 
>> 
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: http://lists.xensource.com/archives/html/xen-devel/attachments/20080410/263aa734/attachment.htm

Re: [PATCH][RFC]Move PCI Configuration Spacesfrom Dom0 to Xen

[Keir Fraser]

I think we can safely follow qemu here and return all-1s on reads when the
enable bit in cf8 is clear. I'll make that change.

 -- Keir

On 11/4/08 01:42, "Shan, Haitao" <haitao.shan@intel.com> wrote:

> Actually I do not find a clear explanation in MCH specification on this.
> Luckily, dom0's kernel does not write 0xcf8 without valid bit set.
> Does access to 0xCFC return data based on the last valid latched data in
> 0xCF8, or last latched data without valid bit set in 0xCF8?
> For example:
> Outl(0x80000000, 0xcf8);
> Outl(0x00000001, 0xcf8);
> Inl(0xcfc); <----This is skipped? Or data at 00:00.0 register 0.
> Another question is:
> Outl(0x80000000, 0xcf8);
> Inb(0xcfc);
> Inb(0xcfd); <----This is skipped? Or data at 00:00.0 register 1?
> Can you tell me where do you find these answers? Thanks!
> 
> Shan Haitao
> 
> -----Original Message-----
> From: Espen Skoglund [mailto:espen.skoglund@netronome.com]
> Sent: 2008年4月11日 0:55
> To: Haitao Shan
> Cc: Keir Fraser; Shan, Haitao; Tian, Kevin; xen-devel; Kay, Allen M; Jiang,
> Yunhong; Han, Weidong
> Subject: Re: [Xen-devel] [PATCH][RFC]Move PCI Configuration Spacesfrom Dom0 to
> Xen
> 
> I believe dword writes to 0xCF8 should always be latched into the
> internal shadow register, irrespective of whether the enable bit (bit
> 31) is set.  Accesseses to 0xCFC can then safely be skipped if enable
> bit in latch register is not set.
> 
> eSk
> 
> 
> 
> [Haitao Shan]
>> Thanks, Keir!
>> 2008/4/10, Keir Fraser <keir.fraser@eu.citrix.com>:
>>> 
>>> On 10/4/08 10:45, "Shan, Haitao" <haitao.shan@intel.com> wrote:
>>> 
>>> This patch will move reading and writing of PCI configuration
>>> spaces from dom0 to Xen. It also changes VTD code, so that they can
>>> touch the PCI configuration spaces with proper lock.
>>> 
>>> This will also benefit MSI support in Xen.
>>> Can you give some comments? Thanks!
>>> 
>>> 
>>> The approach is fine. I will read it more thoroughly, clean it up a bit if
>>> necessary, and certainly check it in.
>>> 
>>> -- Keir
>>> 
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xensource.com
>>> http://lists.xensource.com/xen-devel
>>> 
>>> 
>> -------------- next part --------------
>> An HTML attachment was scrubbed...
>> URL: 
>> http://lists.xensource.com/archives/html/xen-devel/attachments/20080410/263aa
>> 734/attachment.htm
> 

Re: [PATCH][RFC]Move PCI Configuration Spaces from Dom0 toXen

[Jan Beulich]

>>> "Shan, Haitao" <haitao.shan@intel.com> 10.04.08 11:45 >>>
>Hi, Keir,
>
>This patch will move reading and writing of PCI configuration spaces
>from dom0 to Xen. It also changes VTD code, so that they can touch the
>PCI configuration spaces with proper lock.
>This will also benefit MSI support in Xen.
>Can you give some comments? Thanks!
> <<pci_conf_xen.patch>> 

I don't think this is going to do what you want when the kernel is using
mmconfig, or AMD's extended register space access method (not to
speak of the hopefully not in wide spread use type 2 access method).

Also I find it a little odd that you allow access to ports cf9...cfb rather
than properly handling namely byte accesses to cf9. Likewise it
doesn't consider the (albeit unlikely) case of multi-byte accesses to
ports crossing the cf8/cfc boundary.

Jan

Re: [PATCH][RFC]Move PCI Configuration Spaces from Dom0 toXen

[Keir Fraser]

On 11/4/08 13:08, "Jan Beulich" <jbeulich@novell.com> wrote:

> I don't think this is going to do what you want when the kernel is using
> mmconfig, or AMD's extended register space access method (not to
> speak of the hopefully not in wide spread use type 2 access method).

The aim is to synchronise accesses to the PCI window registers cf8/cfc. It's
not to synchronise the PCI config space accesses themselves.

> Also I find it a little odd that you allow access to ports cf9...cfb rather
> than properly handling namely byte accesses to cf9. Likewise it
> doesn't consider the (albeit unlikely) case of multi-byte accesses to
> ports crossing the cf8/cfc boundary.

I've cleaned this up and basically rewritten the patch. I'll be checking it
in this afternoon.

 -- Keir

Re: [PATCH][RFC]Move PCI Configuration Spaces from Dom0 toXen

[Espen Skoglund]

Actually, byte or word accesses in the cf8-cfb range should be passed
through as is.  Only dword accesses to cf8 should be latched into the
config-address register.  The original patch is right in the way it
handles sub-dword accesses.

	eSk


[Jan Beulich]
> Also I find it a little odd that you allow access to ports cf9...cfb
> rather than properly handling namely byte accesses to cf9. Likewise
> it doesn't consider the (albeit unlikely) case of multi-byte
> accesses to ports crossing the cf8/cfc boundary.