[PATCH][VTD] pci mmcfg patch for x86-64 - version 3

[PATCH][VTD] pci mmcfg patch for x86-64 - version 3

[Kay, Allen M]

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


This is version 3 of mmconfig patch that addresses all of the feedbacks I received from Jan and Espen yesterday. 

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

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

diff -r 6595393a3d28 xen/arch/x86/Makefile
--- a/xen/arch/x86/Makefile	Tue Dec 09 16:28:02 2008 +0000
+++ b/xen/arch/x86/Makefile	Tue Dec 09 06:23:29 2008 -0800
@@ -54,6 +54,9 @@ obj-y += crash.o
 obj-y += crash.o
 obj-y += tboot.o
 obj-y += hpet.o
+obj-y += mmconfig-shared.o
+obj-$(CONFIG_X86_64) += mmconfig_64.o
+obj-$(CONFIG_X86_32) += mmconfig_32.o
 
 obj-$(crash_debug) += gdbstub.o
 
diff -r 6595393a3d28 xen/arch/x86/acpi/boot.c
--- a/xen/arch/x86/acpi/boot.c	Tue Dec 09 16:28:02 2008 +0000
+++ b/xen/arch/x86/acpi/boot.c	Tue Dec 09 06:56:15 2008 -0800
@@ -134,6 +134,60 @@ char *__acpi_map_table(unsigned long phy
 	return ((char *) base + offset);
 }
 
+
+static int acpi_mcfg_64bit_base_addr __initdata = FALSE;
+
+/* The physical address of the MMCONFIG aperture.  Set from ACPI tables. */
+struct acpi_mcfg_allocation *pci_mmcfg_config;
+int pci_mmcfg_config_num;
+
+int __init acpi_parse_mcfg(struct acpi_table_header *header)
+{
+	struct acpi_table_mcfg *mcfg;
+	unsigned long i;
+	int config_size;
+
+	if (!header)
+		return -EINVAL;
+
+	mcfg = (struct acpi_table_mcfg *)header;
+
+	/* how many config structures do we have */
+	pci_mmcfg_config_num = 0;
+	i = header->length - sizeof(struct acpi_table_mcfg);
+	while (i >= sizeof(struct acpi_mcfg_allocation)) {
+		++pci_mmcfg_config_num;
+		i -= sizeof(struct acpi_mcfg_allocation);
+	};
+	if (pci_mmcfg_config_num == 0) {
+		printk(KERN_ERR PREFIX "MMCONFIG has no entries\n");
+		return -ENODEV;
+	}
+
+	config_size = pci_mmcfg_config_num * sizeof(*pci_mmcfg_config);
+	pci_mmcfg_config = xmalloc_bytes(config_size);
+	if (!pci_mmcfg_config) {
+		printk(KERN_WARNING PREFIX
+		       "No memory for MCFG config tables\n");
+		return -ENOMEM;
+	}
+
+	memcpy(pci_mmcfg_config, &mcfg[1], config_size);
+
+	for (i = 0; i < pci_mmcfg_config_num; ++i) {
+		if ((pci_mmcfg_config[i].address > 0xFFFFFFFF) &&
+		    !acpi_mcfg_64bit_base_addr) {
+			printk(KERN_ERR PREFIX
+			       "MMCONFIG not in low 4GB of memory\n");
+			xfree(pci_mmcfg_config);
+			pci_mmcfg_config_num = 0;
+			return -ENODEV;
+		}
+	}
+
+	return 0;
+}
+
 #ifdef CONFIG_X86_LOCAL_APIC
 static int __init acpi_parse_madt(struct acpi_table_header *table)
 {
@@ -986,6 +1040,10 @@ int __init acpi_boot_init(void)
 
 	acpi_dmar_init();
 
+#ifdef __x86_64__
+	acpi_mmcfg_init();
+#endif
+
 	return 0;
 }
 
diff -r 6595393a3d28 xen/arch/x86/pci.c
--- a/xen/arch/x86/pci.c	Tue Dec 09 16:28:02 2008 +0000
+++ b/xen/arch/x86/pci.c	Tue Dec 09 06:39:25 2008 -0800
@@ -5,6 +5,7 @@
  */
 
 #include <xen/spinlock.h>
+#include <xen/pci.h>
 #include <asm/io.h>
 
 #define PCI_CONF_ADDRESS(bus, dev, func, reg) \
@@ -73,45 +74,89 @@ uint8_t pci_conf_read8(
 uint8_t pci_conf_read8(
     unsigned int bus, unsigned int dev, unsigned int func, unsigned int reg)
 {
-    BUG_ON((bus > 255) || (dev > 31) || (func > 7) || (reg > 255));
-    return pci_conf_read(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 3, 1);
+    u32 value;
+
+    if ( reg > 255 )
+    {
+        pci_mmcfg_read(0, bus, PCI_DEVFN(dev, func), reg, 1, &value);
+        return value;
+    }
+    else
+    {
+        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
+        return pci_conf_read(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 3, 1);
+    }
 }
 
 uint16_t pci_conf_read16(
     unsigned int bus, unsigned int dev, unsigned int func, unsigned int reg)
 {
-    BUG_ON((bus > 255) || (dev > 31) || (func > 7) || (reg > 255));
-    return pci_conf_read(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 2, 2);
+    u32 value;
+
+    if ( reg > 255 )
+    {
+        pci_mmcfg_read(0, bus, PCI_DEVFN(dev, func), reg, 2, &value);
+        return value;
+    }
+    else
+    {
+        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
+        return pci_conf_read(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 2, 2);
+    }
 }
 
 uint32_t pci_conf_read32(
     unsigned int bus, unsigned int dev, unsigned int func, unsigned int reg)
 {
-    BUG_ON((bus > 255) || (dev > 31) || (func > 7) || (reg > 255));
-    return pci_conf_read(PCI_CONF_ADDRESS(bus, dev, func, reg), 0, 4);
+    u32 value;
+
+    if ( reg > 255 )
+    {
+        pci_mmcfg_read(0, bus, PCI_DEVFN(dev, func), reg, 4, &value);
+        return value;
+    }
+    else
+    {
+        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
+        return pci_conf_read(PCI_CONF_ADDRESS(bus, dev, func, reg), 0, 4);
+    }
 }
 
 void pci_conf_write8(
     unsigned int bus, unsigned int dev, unsigned int func, unsigned int reg,
     uint8_t data)
 {
-    BUG_ON((bus > 255) || (dev > 31) || (func > 7) || (reg > 255));
-    pci_conf_write(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 3, 1, data);
+    if ( reg > 255 )
+        pci_mmcfg_write(0, bus, PCI_DEVFN(dev, func), reg, 1, data);
+    else
+    {
+        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
+        pci_conf_write(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 3, 1, data);
+    }
 }
 
 void pci_conf_write16(
     unsigned int bus, unsigned int dev, unsigned int func, unsigned int reg,
     uint16_t data)
 {
-    BUG_ON((bus > 255) || (dev > 31) || (func > 7) || (reg > 255));
-    pci_conf_write(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 2, 2, data);
+    if ( reg > 255 )
+        pci_mmcfg_write(0, bus, PCI_DEVFN(dev, func), reg, 2, data);
+    else
+    {
+        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
+        pci_conf_write(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 2, 2, data);
+    }
 }
 
 void pci_conf_write32(
     unsigned int bus, unsigned int dev, unsigned int func, unsigned int reg,
     uint32_t data)
 {
-    BUG_ON((bus > 255) || (dev > 31) || (func > 7) || (reg > 255));
-    pci_conf_write(PCI_CONF_ADDRESS(bus, dev, func, reg), 0, 4, data);
+    if ( reg > 255 )
+        pci_mmcfg_write(0, bus, PCI_DEVFN(dev, func), reg, 4, data);
+    else
+    {
+        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
+        pci_conf_write(PCI_CONF_ADDRESS(bus, dev, func, reg), 0, 4, data);
+    }
 }
-
diff -r 6595393a3d28 xen/include/asm-x86/acpi.h
--- a/xen/include/asm-x86/acpi.h	Tue Dec 09 16:28:02 2008 +0000
+++ b/xen/include/asm-x86/acpi.h	Tue Dec 09 06:23:29 2008 -0800
@@ -164,7 +164,8 @@ extern u8 x86_acpiid_to_apicid[];
 extern u8 x86_acpiid_to_apicid[];
 #define MAX_LOCAL_APIC 256
 
-extern int acpi_dmar_init(void);
+int acpi_dmar_init(void);
+void acpi_mmcfg_init(void);
 
 /* Incremented whenever we transition through S3. Value is 1 during boot. */
 extern uint32_t system_reset_counter;
diff -r 6595393a3d28 xen/include/asm-x86/config.h
--- a/xen/include/asm-x86/config.h	Tue Dec 09 16:28:02 2008 +0000
+++ b/xen/include/asm-x86/config.h	Tue Dec 09 06:23:29 2008 -0800
@@ -142,7 +142,7 @@ extern unsigned int video_mode, video_fl
  *  0xffff804000000000 - 0xffff807fffffffff [256GB, 2^38 bytes, PML4:256]
  *    Reserved for future shared info with the guest OS (GUEST ACCESSIBLE).
  *  0xffff808000000000 - 0xffff80ffffffffff [512GB, 2^39 bytes, PML4:257]
- *    Reserved for future use.
+ *    ioremap for PCI mmconfig space
  *  0xffff810000000000 - 0xffff817fffffffff [512GB, 2^39 bytes, PML4:258]
  *    Guest linear page table.
  *  0xffff818000000000 - 0xffff81ffffffffff [512GB, 2^39 bytes, PML4:259]
@@ -195,6 +195,12 @@ extern unsigned int video_mode, video_fl
 /* Slot 256: read-only guest-accessible machine-to-phys translation table. */
 #define RO_MPT_VIRT_START       (PML4_ADDR(256))
 #define RO_MPT_VIRT_END         (RO_MPT_VIRT_START + PML4_ENTRY_BYTES/2)
+/* Slot 257: ioremap for PCI mmconfig space for 2048 segments (512GB)
+ *     - full 16-bit segment support needs 44 bits
+ *     - since PML4 slot has 39 bits, we limit segments to 2048 (11-bits)
+ */
+#define MCFG_VIRT_START         (PML4_ADDR(257))
+#define MCFG_VIRT_END           (MCFG_VIRT_START + PML4_ENTRY_BYTES)
 /* Slot 258: linear page table (guest table). */
 #define LINEAR_PT_VIRT_START    (PML4_ADDR(258))
 #define LINEAR_PT_VIRT_END      (LINEAR_PT_VIRT_START + PML4_ENTRY_BYTES)
diff -r 6595393a3d28 xen/include/xen/pci.h
--- a/xen/include/xen/pci.h	Tue Dec 09 16:28:02 2008 +0000
+++ b/xen/include/xen/pci.h	Tue Dec 09 06:57:32 2008 -0800
@@ -11,6 +11,7 @@
 #include <xen/types.h>
 #include <xen/list.h>
 #include <xen/spinlock.h>
+#include <xen/acpi.h>
 
 /*
  * The PCI interface treats multi-function devices as independent
@@ -29,6 +30,54 @@
 #define PCI_BDF(b,d,f)  ((((b) & 0xff) << 8) | PCI_DEVFN(d,f))
 #define PCI_BDF2(b,df)  ((((b) & 0xff) << 8) | ((df) & 0xff))
 
+#define PCI_VENDOR_ID_INTEL		0x8086
+#define PCI_DEVICE_ID_INTEL_E7520_MCH	0x3590
+#define PCI_DEVICE_ID_INTEL_82945G_HB	0x2770
+
+#define PCI_PROBE_CONF1		0x0002
+#define PCI_PROBE_MMCONF	0x0008
+#define PCI_PROBE_MASK		0x000f
+
+/* used by mmcfg */
+static inline unsigned char mmio_config_readb(void __iomem *pos)
+{
+	u8 val;
+	asm volatile("movb (%1),%%al" : "=a" (val) : "r" (pos));
+	return val;
+}
+
+static inline unsigned short mmio_config_readw(void __iomem *pos)
+{
+	u16 val;
+	asm volatile("movw (%1),%%ax" : "=a" (val) : "r" (pos));
+	return val;
+}
+
+static inline unsigned int mmio_config_readl(void __iomem *pos)
+{
+	u32 val;
+	asm volatile("movl (%1),%%eax" : "=a" (val) : "r" (pos));
+	return val;
+}
+
+static inline void mmio_config_writeb(void __iomem *pos, u8 val)
+{
+	asm volatile("movb %%al,(%1)" :: "a" (val), "r" (pos) : "memory");
+}
+
+static inline void mmio_config_writew(void __iomem *pos, u16 val)
+{
+	asm volatile("movw %%ax,(%1)" :: "a" (val), "r" (pos) : "memory");
+}
+
+static inline void mmio_config_writel(void __iomem *pos, u32 val)
+{
+	asm volatile("movl %%eax,(%1)" :: "a" (val), "r" (pos) : "memory");
+}
+
+
+/* structure definitions */
+
 struct pci_dev {
     struct list_head alldevs_list;
     struct list_head domain_list;
@@ -42,6 +91,11 @@ struct pci_dev {
 #define for_each_pdev(domain, pdev) \
     list_for_each_entry(pdev, &(domain->arch.pdev_list), domain_list)
 
+/* global variable */
+
+extern int pci_mmcfg_config_num;
+extern struct acpi_mcfg_allocation *pci_mmcfg_config;
+
 /*
  * The pcidevs_lock write-lock must be held when doing alloc_pdev() or
  * free_pdev().  Never de-reference pdev without holding pdev->lock or
@@ -51,15 +105,16 @@ struct pci_dev {
 
 extern rwlock_t pcidevs_lock;
 
+
+/* function prototypes */
+
 struct pci_dev *alloc_pdev(u8 bus, u8 devfn);
 void free_pdev(struct pci_dev *pdev);
 struct pci_dev *pci_lock_pdev(int bus, int devfn);
 struct pci_dev *pci_lock_domain_pdev(struct domain *d, int bus, int devfn);
-
 void pci_release_devices(struct domain *d);
 int pci_add_device(u8 bus, u8 devfn);
 int pci_remove_device(u8 bus, u8 devfn);
-
 uint8_t pci_conf_read8(
     unsigned int bus, unsigned int dev, unsigned int func, unsigned int reg);
 uint16_t pci_conf_read16(
@@ -77,5 +132,13 @@ void pci_conf_write32(
     uint32_t data);
 int pci_find_cap_offset(u8 bus, u8 dev, u8 func, u8 cap);
 int pci_find_next_cap(u8 bus, unsigned int devfn, u8 pos, int cap);
+int pci_find_ext_capability(int seg, int bus, int devfn, int cap);
+int pci_mmcfg_read(unsigned int seg, unsigned int bus,
+                   unsigned int devfn, int reg, int len, u32 *value);
+int pci_mmcfg_write(unsigned int seg, unsigned int bus,
+                    unsigned int devfn, int reg, int len, u32 value);
+int acpi_parse_mcfg(struct acpi_table_header *header);
+int pci_mmcfg_arch_init(void);
+void pci_mmcfg_arch_free(void);
 
 #endif /* __XEN_PCI_H__ */
diff -r 6595393a3d28 xen/include/xen/pci_regs.h
--- a/xen/include/xen/pci_regs.h	Tue Dec 09 16:28:02 2008 +0000
+++ b/xen/include/xen/pci_regs.h	Tue Dec 09 06:23:29 2008 -0800
@@ -419,6 +419,9 @@
 #define PCI_EXT_CAP_ID_VC	2
 #define PCI_EXT_CAP_ID_DSN	3
 #define PCI_EXT_CAP_ID_PWR	4
+#define PCI_EXT_CAP_ID_ARI	0xE
+#define PCI_EXT_CAP_ID_ATS	0xF
+#define PCI_EXT_CAP_ID_IOV	0x10
 
 /* Advanced Error Reporting */
 #define PCI_ERR_UNCOR_STATUS	4	/* Uncorrectable Error Status */
diff -r 6595393a3d28 xen/arch/x86/mmconfig-shared.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/arch/x86/mmconfig-shared.c	Tue Dec 09 06:42:35 2008 -0800
@@ -0,0 +1,230 @@
+/*
+ * mmconfig-shared.c - Low-level direct PCI config space access via
+ *                     MMCONFIG - common code between i386 and x86-64.
+ *
+ * This code does:
+ * - known chipset handling
+ * - ACPI decoding and validation
+ *
+ * Per-architecture code takes care of the mappings and accesses
+ * themselves.
+ *
+ * Author: Allen Kay <allen.m.kay@intel.com> -  adapted to xen from Linux
+ */
+
+#include <xen/mm.h>
+#include <xen/acpi.h>
+#include <xen/xmalloc.h>
+#include <xen/pci.h>
+#include <xen/pci_regs.h>
+
+static const char __init *pci_mmcfg_e7520(void)
+{
+    u32 win;
+
+        win = pci_conf_read16(0, 0, 0, 0xce);
+
+    win = win & 0xf000;
+    if(win == 0x0000 || win == 0xf000)
+        pci_mmcfg_config_num = 0;
+    else {
+        pci_mmcfg_config_num = 1;
+        pci_mmcfg_config = xmalloc_bytes(sizeof(pci_mmcfg_config[0]));
+        if (!pci_mmcfg_config)
+            return NULL;
+        memset(pci_mmcfg_config, 0, sizeof(pci_mmcfg_config[0]));
+        pci_mmcfg_config[0].address = win << 16;
+        pci_mmcfg_config[0].pci_segment = 0;
+        pci_mmcfg_config[0].start_bus_number = 0;
+        pci_mmcfg_config[0].end_bus_number = 255;
+    }
+
+    return "Intel Corporation E7520 Memory Controller Hub";
+}
+
+static const char __init *pci_mmcfg_intel_945(void)
+{
+    u32 pciexbar, mask = 0, len = 0;
+
+    pci_mmcfg_config_num = 1;
+
+        pciexbar = pci_conf_read32(0, 0, 0, 0x48);
+
+    /* Enable bit */
+    if (!(pciexbar & 1))
+        pci_mmcfg_config_num = 0;
+
+    /* Size bits */
+    switch ((pciexbar >> 1) & 3) {
+    case 0:
+        mask = 0xf0000000U;
+        len  = 0x10000000U;
+        break;
+    case 1:
+        mask = 0xf8000000U;
+        len  = 0x08000000U;
+        break;
+    case 2:
+        mask = 0xfc000000U;
+        len  = 0x04000000U;
+        break;
+    default:
+        pci_mmcfg_config_num = 0;
+    }
+
+    /* Errata #2, things break when not aligned on a 256Mb boundary */
+    /* Can only happen in 64M/128M mode */
+
+    if ((pciexbar & mask) & 0x0fffffffU)
+        pci_mmcfg_config_num = 0;
+
+    /* Don't hit the APIC registers and their friends */
+    if ((pciexbar & mask) >= 0xf0000000U)
+        pci_mmcfg_config_num = 0;
+
+    if (pci_mmcfg_config_num) {
+        pci_mmcfg_config = xmalloc_bytes(sizeof(pci_mmcfg_config[0]));
+        if (!pci_mmcfg_config)
+            return NULL;
+        memset(pci_mmcfg_config, 0, sizeof(pci_mmcfg_config[0]));
+        pci_mmcfg_config[0].address = pciexbar & mask;
+        pci_mmcfg_config[0].pci_segment = 0;
+        pci_mmcfg_config[0].start_bus_number = 0;
+        pci_mmcfg_config[0].end_bus_number = (len >> 20) - 1;
+    }
+
+    return "Intel Corporation 945G/GZ/P/PL Express Memory Controller Hub";
+}
+
+struct pci_mmcfg_hostbridge_probe {
+    u32 bus;
+    u32 devfn;
+    u32 vendor;
+    u32 device;
+    const char *(*probe)(void);
+};
+
+static struct pci_mmcfg_hostbridge_probe pci_mmcfg_probes[] __initdata = {
+    { 0, PCI_DEVFN(0, 0), PCI_VENDOR_ID_INTEL,
+      PCI_DEVICE_ID_INTEL_E7520_MCH, pci_mmcfg_e7520 },
+    { 0, PCI_DEVFN(0, 0), PCI_VENDOR_ID_INTEL,
+      PCI_DEVICE_ID_INTEL_82945G_HB, pci_mmcfg_intel_945 },
+};
+
+static int __init pci_mmcfg_check_hostbridge(void)
+{
+    u32 l;
+    u32 bus, devfn;
+    u16 vendor, device;
+    int i;
+    const char *name;
+
+    pci_mmcfg_config_num = 0;
+    pci_mmcfg_config = NULL;
+    name = NULL;
+
+    for (i = 0; !name && i < ARRAY_SIZE(pci_mmcfg_probes); i++) {
+        bus =  pci_mmcfg_probes[i].bus;
+        devfn = pci_mmcfg_probes[i].devfn;
+        l = pci_conf_read32(bus, PCI_SLOT(devfn), PCI_FUNC(devfn), 0);
+        vendor = l & 0xffff;
+        device = (l >> 16) & 0xffff;
+
+        if (pci_mmcfg_probes[i].vendor == vendor &&
+            pci_mmcfg_probes[i].device == device)
+            name = pci_mmcfg_probes[i].probe();
+    }
+
+    if (name) {
+        printk(KERN_INFO "PCI: Found %s %s MMCONFIG support.\n",
+            name, pci_mmcfg_config_num ? "with" : "without");
+    }
+
+    return name != NULL;
+}
+
+static int __initdata known_bridge;
+unsigned int pci_probe = PCI_PROBE_CONF1 | PCI_PROBE_MMCONF;
+
+void __init __pci_mmcfg_init(int early)
+{
+    /* MMCONFIG disabled */
+    if ((pci_probe & PCI_PROBE_MMCONF) == 0)
+        return;
+
+    /* MMCONFIG already enabled */
+    if (!early && !(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF))
+        return;
+
+    /* for late to exit */
+    if (known_bridge)
+        return;
+
+    if (early) {
+        if (pci_mmcfg_check_hostbridge())
+            known_bridge = 1;
+    }
+
+    if (!known_bridge) {
+        acpi_table_parse(ACPI_SIG_MCFG, acpi_parse_mcfg);
+    }
+
+    if ((pci_mmcfg_config_num == 0) ||
+        (pci_mmcfg_config == NULL) ||
+        (pci_mmcfg_config[0].address == 0))
+        return;
+
+    if (pci_mmcfg_arch_init()) {
+        pci_probe = (pci_probe & ~PCI_PROBE_MASK) | PCI_PROBE_MMCONF;
+    }
+}
+
+void acpi_mmcfg_init(void)
+{
+    __pci_mmcfg_init(1);
+}
+
+/**
+ * pci_find_ext_capability - Find an extended capability
+ * @dev: PCI device to query
+ * @cap: capability code
+ *
+ * Returns the address of the requested extended capability structure
+ * within the device's PCI configuration space or 0 if the device does
+ * not support it.  Possible values for @cap:
+ *
+ *  %PCI_EXT_CAP_ID_ERR         Advanced Error Reporting
+ *  %PCI_EXT_CAP_ID_VC          Virtual Channel
+ *  %PCI_EXT_CAP_ID_DSN         Device Serial Number
+ *  %PCI_EXT_CAP_ID_PWR         Power Budgeting
+ */
+int pci_find_ext_capability(int seg, int bus, int devfn, int cap)
+{
+    u32 header;
+    int ttl = 480; /* 3840 bytes, minimum 8 bytes per capability */
+    int pos = 0x100;
+
+    header = pci_conf_read32(bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pos);
+
+    /*
+     * If we have no capabilities, this is indicated by cap ID,
+     * cap version and next pointer all being 0.
+     */
+    if ( (header == 0) || (header == -1) )
+    {
+        dprintk(XENLOG_INFO VTDPREFIX,
+                "next cap:%x:%x.%x:  no extended config\n",
+                bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+        return 0;
+    }
+
+    while ( ttl-- > 0 ) {
+        if ( PCI_EXT_CAP_ID(header) == cap )
+            return pos;
+        pos = PCI_EXT_CAP_NEXT(header);
+        if ( pos < 0x100 )
+            break;
+        header = pci_conf_read32(bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pos);
+    }
+    return 0;
+}
diff -r 6595393a3d28 xen/arch/x86/mmconfig_32.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/arch/x86/mmconfig_32.c	Tue Dec 09 06:42:04 2008 -0800
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2004 Matthew Wilcox <matthew@wil.cx>
+ * Copyright (C) 2004 Intel Corp.
+ *
+ * This code is released under the GNU General Public License version 2.
+ */
+
+#include <xen/mm.h>
+#include <xen/acpi.h>
+#include <xen/xmalloc.h>
+#include <xen/pci.h>
+#include <xen/pci_regs.h>
+
+int pci_mmcfg_read(unsigned int seg, unsigned int bus,
+			  unsigned int devfn, int reg, int len, u32 *value)
+{
+	*value = -1;
+	return 0;
+}
+
+int pci_mmcfg_write(unsigned int seg, unsigned int bus,
+			   unsigned int devfn, int reg, int len, u32 value)
+{
+	return -EINVAL;
+}
+
+int __init pci_mmcfg_arch_init(void)
+{
+	return 1;
+}
+
+void __init pci_mmcfg_arch_free(void)
+{
+}
diff -r 6595393a3d28 xen/arch/x86/mmconfig_64.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/arch/x86/mmconfig_64.c	Tue Dec 09 06:42:13 2008 -0800
@@ -0,0 +1,171 @@
+/*
+ * mmconfig.c - Low-level direct PCI config space access via MMCONFIG
+ *
+ * This is an 64bit optimized version that always keeps the full mmconfig
+ * space mapped. This allows lockless config space operation.
+ *
+ * copied from Linux
+ */
+
+#include <xen/mm.h>
+#include <xen/acpi.h>
+#include <xen/xmalloc.h>
+#include <xen/pci.h>
+#include <xen/pci_regs.h>
+
+/* Static virtual mapping of the MMCONFIG aperture */
+struct mmcfg_virt {
+	struct acpi_mcfg_allocation *cfg;
+	char __iomem *virt;
+};
+static struct mmcfg_virt *pci_mmcfg_virt;
+
+static char __iomem *get_virt(unsigned int seg, unsigned bus)
+{
+	struct acpi_mcfg_allocation *cfg;
+	int cfg_num;
+
+	for (cfg_num = 0; cfg_num < pci_mmcfg_config_num; cfg_num++) {
+		cfg = pci_mmcfg_virt[cfg_num].cfg;
+		if (cfg->pci_segment == seg &&
+		    (cfg->start_bus_number <= bus) &&
+		    (cfg->end_bus_number >= bus))
+			return pci_mmcfg_virt[cfg_num].virt;
+	}
+
+	/* Fall back to type 0 */
+	return NULL;
+}
+
+static char __iomem *pci_dev_base(unsigned int seg, unsigned int bus, unsigned int devfn)
+{
+	char __iomem *addr;
+
+	addr = get_virt(seg, bus);
+	if (!addr)
+		return NULL;
+ 	return addr + ((bus << 20) | (devfn << 12));
+}
+
+int pci_mmcfg_read(unsigned int seg, unsigned int bus,
+			  unsigned int devfn, int reg, int len, u32 *value)
+{
+	char __iomem *addr;
+
+	/* Why do we have this when nobody checks it. How about a BUG()!? -AK */
+	if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095))) {
+err:		*value = -1;
+		return -EINVAL;
+	}
+
+	addr = pci_dev_base(seg, bus, devfn);
+	if (!addr)
+		goto err;
+
+	switch (len) {
+	case 1:
+		*value = mmio_config_readb(addr + reg);
+		break;
+	case 2:
+		*value = mmio_config_readw(addr + reg);
+		break;
+	case 4:
+		*value = mmio_config_readl(addr + reg);
+		break;
+	}
+
+	return 0;
+}
+
+int pci_mmcfg_write(unsigned int seg, unsigned int bus,
+			   unsigned int devfn, int reg, int len, u32 value)
+{
+	char __iomem *addr;
+
+	/* Why do we have this when nobody checks it. How about a BUG()!? -AK */
+	if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095)))
+		return -EINVAL;
+
+	addr = pci_dev_base(seg, bus, devfn);
+	if (!addr)
+		return -EINVAL;
+
+	switch (len) {
+	case 1:
+		mmio_config_writeb(addr + reg, value);
+		break;
+	case 2:
+		mmio_config_writew(addr + reg, value);
+		break;
+	case 4:
+		mmio_config_writel(addr + reg, value);
+		break;
+	}
+
+	return 0;
+}
+
+static void __iomem * __init mcfg_ioremap(struct acpi_mcfg_allocation *cfg)
+{
+	void __iomem *addr;
+	unsigned long virt;
+	unsigned long mfn;
+	unsigned long size, nr_mfn;
+
+        if ( cfg->pci_segment >= ((MCFG_VIRT_END - MCFG_VIRT_START) >> 28) )
+	    return NULL;
+
+	virt = MCFG_VIRT_START + (cfg->pci_segment * (1UL << 28)) +
+               (cfg->start_bus_number * (1UL << 20));
+	mfn = cfg->address >> PAGE_SHIFT;
+        size = (cfg->end_bus_number - cfg->start_bus_number + 1) << 20;
+        nr_mfn = size >> PAGE_SHIFT;
+
+	map_pages_to_xen(virt, mfn, nr_mfn, PAGE_HYPERVISOR_NOCACHE);
+	addr = (void __iomem *) virt;
+
+	return addr;
+}
+
+int __init pci_mmcfg_arch_init(void)
+{
+	int i;
+	pci_mmcfg_virt = xmalloc_bytes(sizeof(*pci_mmcfg_virt) * pci_mmcfg_config_num);
+	if (pci_mmcfg_virt == NULL) {
+		printk(KERN_ERR "PCI: Can not allocate memory for mmconfig structures\n");
+		return 0;
+	}
+	memset(pci_mmcfg_virt, 0, sizeof(*pci_mmcfg_virt) * pci_mmcfg_config_num);
+
+	for (i = 0; i < pci_mmcfg_config_num; ++i) {
+		pci_mmcfg_virt[i].cfg = &pci_mmcfg_config[i];
+		pci_mmcfg_virt[i].virt = mcfg_ioremap(&pci_mmcfg_config[i]);
+		if (!pci_mmcfg_virt[i].virt) {
+			printk(KERN_ERR "PCI: Cannot map mmconfig aperture for "
+					"segment %d\n",
+				pci_mmcfg_config[i].pci_segment);
+			pci_mmcfg_arch_free();
+			return 0;
+		}
+	}
+	return 1;
+}
+
+void __init pci_mmcfg_arch_free(void)
+{
+	int i;
+
+	if (pci_mmcfg_virt == NULL)
+		return;
+
+	for (i = 0; i < pci_mmcfg_config_num; ++i) {
+		if (pci_mmcfg_virt[i].virt) {
+			iounmap(pci_mmcfg_virt[i].virt);
+			pci_mmcfg_virt[i].virt = NULL;
+			pci_mmcfg_virt[i].cfg = NULL;
+		}
+	}
+
+	xfree(pci_mmcfg_virt);
+	pci_mmcfg_virt = NULL;
+}

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

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

Re: [PATCH][VTD] pci mmcfg patch for x86-64 - version 3

[Jan Beulich]

>>> "Kay, Allen M" <allen.m.kay@intel.com> 10.12.08 04:14 >>>
>This is version 3 of mmconfig patch that addresses all of the feedbacks I received from Jan and Espen yesterday. 

Thanks. Two more (new) items, though:

>--- a/xen/arch/x86/Makefile	Tue Dec 09 16:28:02 2008 +0000
>+++ b/xen/arch/x86/Makefile	Tue Dec 09 06:23:29 2008 -0800
>@@ -54,6 +54,9 @@ obj-y += crash.o
> obj-y += crash.o
> obj-y += tboot.o
> obj-y += hpet.o
>+obj-y += mmconfig-shared.o
>+obj-$(CONFIG_X86_64) += mmconfig_64.o
>+obj-$(CONFIG_X86_32) += mmconfig_32.o

These should now really go into xen/arch/x86/x86_{32,64}/ rather than
adding _{32,64} suffixes.

>+int pci_mmcfg_read(unsigned int seg, unsigned int bus,
>+			  unsigned int devfn, int reg, int len, u32 *value)
>+{
>+	*value = -1;
>+	return 0;
>+}

Shouldn't you return -EINVAL (or -ENOSYS, but an error in any case) here?

And one thing I didn't notice before:

>+/* used by mmcfg */
>+static inline unsigned char mmio_config_readb(void __iomem *pos)
>+{
>...

This is preceded by a rather important comment regarding AMD Fam10
CPUs in Linux. Without that comment, no-one will easily understand why
you must use eax/ax/al here. I'm also surprised you didn't copy over
pci_mmcfg_amd_fam10h() from Linux...

And as far as I'm concerned I would not exactly follow Linux in how the
assembly is written, e.g. replace

+	asm volatile("movb (%1),%%al" : "=a" (val) : "r" (pos));

by

    asm volatile("movb (%1),%0" : "=a" (val) : "r" (pos));

(and use proper Xen-style indentation).

Oh, and one more thing I remembered just now: Linux verifies the mmcfg
values it gets from ACPI against the E820 map - shouldn't Xen do this too?

Jan

RE: Re: [PATCH][VTD] pci mmcfg patch for x86-64 - version 3

[Kay, Allen M]

>
>>--- a/xen/arch/x86/Makefile	Tue Dec 09 16:28:02 2008 +0000
>>+++ b/xen/arch/x86/Makefile	Tue Dec 09 06:23:29 2008 -0800
>>@@ -54,6 +54,9 @@ obj-y += crash.o
>> obj-y += crash.o
>> obj-y += tboot.o
>> obj-y += hpet.o
>>+obj-y += mmconfig-shared.o
>>+obj-$(CONFIG_X86_64) += mmconfig_64.o
>>+obj-$(CONFIG_X86_32) += mmconfig_32.o
>
>These should now really go into xen/arch/x86/x86_{32,64}/ rather than
>adding _{32,64} suffixes.
>

I was trying to maintain the same names as Linux for ease of reference.  You are right, it would be more appropriate to follow Xen directory structure here.  I also tried to maintain file format of mmconfig_x?? to conform to Linux as they are pretty much straight copies of Linux files.  Sounds like these file should also be converted to Xen indentation here.

>>+int pci_mmcfg_read(unsigned int seg, unsigned int bus,
>>+			  unsigned int devfn, int reg, int len, 
>u32 *value)
>>+{
>>+	*value = -1;
>>+	return 0;
>>+}
>
>Shouldn't you return -EINVAL (or -ENOSYS, but an error in any 
>case) here?
>

Yes it should return -EINVAL as in the original Linux code.  The change to 0 was unintentional.

>And one thing I didn't notice before:
>
>>+/* used by mmcfg */
>>+static inline unsigned char mmio_config_readb(void __iomem *pos)
>>+{
>>...
>
>This is preceded by a rather important comment regarding AMD Fam10
>CPUs in Linux. Without that comment, no-one will easily understand why
>you must use eax/ax/al here. I'm also surprised you didn't copy over
>pci_mmcfg_amd_fam10h() from Linux...
>

OK, I will add the comments back in.  Where is pci_mmcfg_amd_family10h() defined?  I'm having trouble finding it.

>Oh, and one more thing I remembered just now: Linux verifies the mmcfg
>values it gets from ACPI against the E820 map - shouldn't Xen 
>do this too?

My original intent was to keep mmconfig-shared.c and mmconfig_64.c as closely to Linux as possible.  However, I soon found out including everything in mmconfig-shared.c involved pulling in a lot of code from Linux.

As my main goal of this round is enabling ATS, I tried to limited the scope of mmconfig work to be somewhat manageable for now and then add more stuff to it as needed.  As I'm planning to work on multi-segment PCI support in Q1, this stuff will get revisited again.

Other than E820 checking, do you see anything else in mmconfig-shared.c need to be included for this round?

Allen

RE: Re: [PATCH][VTD] pci mmcfg patch for x86-64 -version 3

[Jan Beulich]

>>> "Kay, Allen M" <allen.m.kay@intel.com> 12.12.08 03:47 >>>
>>>+/* used by mmcfg */
>>>+static inline unsigned char mmio_config_readb(void __iomem *pos)
>>>+{
>>>...
>>
>>This is preceded by a rather important comment regarding AMD Fam10
>>CPUs in Linux. Without that comment, no-one will easily understand why
>>you must use eax/ax/al here. I'm also surprised you didn't copy over
>>pci_mmcfg_amd_fam10h() from Linux...
>>
>
>OK, I will add the comments back in.  Where is pci_mmcfg_amd_family10h() defined?  I'm having trouble finding it.

The name really is pci_mmcfg_amd_fam10h(), and it's in 2.6.27's
arch/x86/pci/mmconfig-shared.c, right along with the other host bridge
probe functions.

>>Oh, and one more thing I remembered just now: Linux verifies the mmcfg
>>values it gets from ACPI against the E820 map - shouldn't Xen 
>>do this too?
>
>My original intent was to keep mmconfig-shared.c and mmconfig_64.c as closely to Linux as possible.  However, I soon found out
>including everything in mmconfig-shared.c involved pulling in a lot of code from Linux.

Understandable. But this particular check may save us from dying on systems
where Linux works. And it shouldn't be that much code that needs adding
here if you restrict this to the E820 check; I'm not certain how significant
the ACPI check is (which would require Dom0 assistance in my opinion, as
it requires the ACPI interpreter to be available).

>As my main goal of this round is enabling ATS, I tried to limited the scope of mmconfig work to be somewhat manageable for now
>and then add more stuff to it as needed.  As I'm planning to work on multi-segment PCI support in Q1, this stuff will get revisited
>again.

Is that going to come through Linux again, or is this a Xen-only plan?

>Other than E820 checking, do you see anything else in mmconfig-shared.c need to be included for this round?

I didn't notice anything.

Jan

Re: Re: [PATCH][VTD] pci mmcfg patch for x86-64 -version 3

[Konrad Rzeszutek]

> >>Oh, and one more thing I remembered just now: Linux verifies the mmcfg
> >>values it gets from ACPI against the E820 map - shouldn't Xen 
> >>do this too?
> >
> >My original intent was to keep mmconfig-shared.c and mmconfig_64.c as closely to Linux as possible.  However, I soon found out
> >including everything in mmconfig-shared.c involved pulling in a lot of code from Linux.
> 
> Understandable. But this particular check may save us from dying on systems
> where Linux works. And it shouldn't be that much code that needs adding

My recollection of this was that the check was added for buggy pre-release
Intel boxes (the address that was coded in the ACPI MMCONFIG table was
completly bogus). Follow this link for more details:
http://lkml.indiana.edu/hypermail/linux/kernel/0605.2/0923.html

I honestly can't remember anymore the details of the E820 check, but the
check isn't that difficult:

Read the addresses and make sure they are the host E820 as reserved.

> here if you restrict this to the E820 check; I'm not certain how significant
> the ACPI check is (which would require Dom0 assistance in my opinion, as
> it requires the ACPI interpreter to be available).

RE: Re: [PATCH][VTD] pci mmcfg patch for x86-64 -version 3

[Kay, Allen M]

>As I'm planning to 
>work on multi-segment PCI support in Q1, this stuff will get revisited
>>again.
>
>Is that going to come through Linux again, or is this a Xen-only plan?
>

It will involve both xen and Linux.  Segment information for the device will come from dom0 Linux.  Segment knowledge in VT-d will be added in xen.  For example, current vtd/dmar.c/acpi_find_matched_drhd_unit() assumes the entire system has only one INCLUDE_ALL vt-d engine.  In a multi PCI segment system, each segment can have a INCLUDE_ALL VT-d engine.

Allen

Re: [PATCH][VTD] pci mmcfg patch for x86-64 - version 3

[Espen Skoglund]

[Allen M Kay]
> This is version 3 of mmconfig patch that addresses all of the
> feedbacks I received from Jan and Espen yesterday.

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

Some comments inline.

	eSk


================================================================

> diff -r 6595393a3d28 xen/arch/x86/mmconfig_32.c
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/xen/arch/x86/mmconfig_32.c	Tue Dec 09 06:42:04 2008 -0800

> +int pci_mmcfg_read(unsigned int seg, unsigned int bus,
> +			  unsigned int devfn, int reg, int len, u32 *value)
> +{
> +	*value = -1;
> +	return 0;
> +}
> +
> +int pci_mmcfg_write(unsigned int seg, unsigned int bus,
> +			   unsigned int devfn, int reg, int len, u32 value)
> +{
> +	return -EINVAL;
> +}

These two can be removed (see further down).

> +int __init pci_mmcfg_arch_init(void)
> +{
> +	return 1;
> +}

Should return 0.  And add a dummy pci_dev_base() function.


> diff -r 6595393a3d28 xen/arch/x86/mmconfig_64.c
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/xen/arch/x86/mmconfig_64.c	Tue Dec 09 06:42:13 2008 -0800

Put following two functions in generic x86.

> +int pci_mmcfg_read(unsigned int seg, unsigned int bus,
> +			  unsigned int devfn, int reg, int len, u32 *value)
> +{
> +	char __iomem *addr;
> +
> +	/* Why do we have this when nobody checks it. How about a BUG()!? -AK */
> +	if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095))) {
> +err:		*value = -1;
> +		return -EINVAL;
> +	}

Also fail if (pci_probe & PCI_PROBE_MMCONF) == 0.  Or alternatively do
this check in pci_dev_base().

> +	addr = pci_dev_base(seg, bus, devfn);
> +	if (!addr)
> +		goto err;
> +
> +	switch (len) {
> +	case 1:
> +		*value = mmio_config_readb(addr + reg);
> +		break;
> +	case 2:
> +		*value = mmio_config_readw(addr + reg);
> +		break;
> +	case 4:
> +		*value = mmio_config_readl(addr + reg);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +int pci_mmcfg_write(unsigned int seg, unsigned int bus,
> +			   unsigned int devfn, int reg, int len, u32 value)
> +{
> +	char __iomem *addr;
> +
> +	/* Why do we have this when nobody checks it. How about a BUG()!? -AK */
> +	if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095)))
> +		return -EINVAL;

Also fail if (pci_probe & PCI_PROBE_MMCONF) == 0.

> +	addr = pci_dev_base(seg, bus, devfn);
> +	if (!addr)
> +		return -EINVAL;
> +
> +	switch (len) {
> +	case 1:
> +		mmio_config_writeb(addr + reg, value);
> +		break;
> +	case 2:
> +		mmio_config_writew(addr + reg, value);
> +		break;
> +	case 4:
> +		mmio_config_writel(addr + reg, value);
> +		break;
> +	}
> +
> +	return 0;
> +}

RE: [PATCH][VTD] pci mmcfg patch for x86-64 - version 3

[Kay, Allen M]

 
>> +int __init pci_mmcfg_arch_init(void)
>> +{
>> +	return 1;
>> +}
>
>Should return 0.  And add a dummy pci_dev_base() function.
>
>

Done.  Added pci_dev_base() returning 0 in x86_32 version of mmconfig.c.


>> diff -r 6595393a3d28 xen/arch/x86/mmconfig_64.c
>> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>> +++ b/xen/arch/x86/mmconfig_64.c	Tue Dec 09 06:42:13 2008 -0800
>
>Put following two functions in generic x86.
>
>> +int pci_mmcfg_read(unsigned int seg, unsigned int bus,
>> +			  unsigned int devfn, int reg, int len, 
>u32 *value)
>> +{
>> +	char __iomem *addr;
>> +
>> +	/* Why do we have this when nobody checks it. How about 
>a BUG()!? -AK */
>> +	if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095))) {
>> +err:		*value = -1;
>> +		return -EINVAL;
>> +	}
>
>Also fail if (pci_probe & PCI_PROBE_MMCONF) == 0.  Or alternatively do
>this check in pci_dev_base().
>

I added the check in both read/write functions instead of pci_dev_base().  It is probably better from maintainence point as it is more straight forward.

Allen