Driver for new "VMD" device

Now on version 7, mostly cosmetic.

v6->v7:

Removed PCI resource window patch rejecting child bus that doesn't fit in parent's window. There are additional caveats the maintainer brought up that need more consideration. Since we are not dependent on that patch, it is removed from the series.

Dropped the pciutils patch since it is an external projet not part of this series anyway. Will send tooling updates to the appropriate mailing list once this settles.

Modified commit log messages to align with maintainer's requested format.

Using u32's for domains instead of ints.

Fixed code comment on posted/non-posted clarification.

Added a dmesg info message to show which domain a VMD end point bound to.

Fixed the vmd domain's root bus parent device so power management has the correct dependency tree.

Keith Busch (5):
PCI/MSI: Export msi functions for module use x86/IRQ: Export IRQ domain function for module use x86/PCI: Allow PCI domain specific dma ops x86/PCI: Initial commit for new VMD device driver PCI/AER: Use 32 bit int type domains

Liu Jiang (1):
msi: Relax msi_domain_alloc() to support parentless MSI irqdomains

 arch/x86/Kconfig                  |  13 +
 arch/x86/include/asm/device.h     |  10 +
 arch/x86/include/asm/hw_irq.h     |   5 +
 arch/x86/pci/Makefile             |   2 +
 arch/x86/pci/common.c             |  38 +++
 arch/x86/pci/vmd.c                | 698 ++++++++++++++++++++++++++++++++++++++
 drivers/pci/msi.c                 |   2 +
 drivers/pci/pcie/aer/aer_inject.c |  16 +-
 kernel/irq/irqdomain.c            |   1 +
 kernel/irq/msi.c                  |   8 +-
 10 files changed, 782 insertions(+), 11 deletions(-)
 create mode 100644 arch/x86/pci/vmd.c

2.6.2.307.g37023ba

From: Liu Jiang <[email protected]>

Previously msi_domain_alloc() assumes MSI irqdomains always have parent irqdomains, but that's not true for the new Intel VMD devices. So relax msi_domain_alloc() to support parentless MSI irqdomains.

---
 kernel/irq/msi.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

index 6b0c0b7..5e15cb4 100644
@@ -109,9 +109,11 @@ static int msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
 	if (irq_find_mapping(domain, hwirq) > 0)
 		return -EEXIST;
 
-	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
-	if (ret < 0)
-		return ret;
+	if (domain->parent) {
+		ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
+		if (ret < 0)
+			return ret;
+	}
 
 	for (i = 0; i < nr_irqs; i++) {
 		ret = ops->msi_init(domain, info, virq + i, hwirq + i, arg);

2.6.2.307.g37023ba

---
 drivers/pci/msi.c | 2 ++
 1 file changed, 2 insertions(+)

index 7eaa4c8..fb7506f 100644
@@ -1126,6 +1126,7 @@ struct pci_dev *msi_desc_to_pci_dev(struct msi_desc *desc)
 {
 	return to_pci_dev(desc->dev);
 }
+EXPORT_SYMBOL_GPL(msi_desc_to_pci_dev);
 
 void *msi_desc_to_pci_sysdata(struct msi_desc *desc)
 {

@@ -1285,6 +1286,7 @@ struct irq_domain *pci_msi_create_irq_domain(struct fwnode_handle *fwnode,
 	domain->bus_token = DOMAIN_BUS_PCI_MSI;
 	return domain;
 }
+EXPORT_SYMBOL_GPL(pci_msi_create_irq_domain);
 
 /**
  * pci_msi_domain_alloc_irqs - Allocate interrupts for @dev in @domain

2.6.2.307.g37023ba

---
 kernel/irq/irqdomain.c | 1 +
 1 file changed, 1 insertion(+)

index 22aa961..ca05cc8 100644
@@ -1058,6 +1058,7 @@ void irq_domain_set_info(struct irq_domain *domain, unsigned int virq,
 	__irq_set_handler(virq, handler, 0, handler_name);
 	irq_set_handler_data(virq, handler_data);
 }
+EXPORT_SYMBOL(irq_domain_set_info);
 
 /**
  * irq_domain_reset_irq_data - Clear hwirq, chip and chip_data in @irq_data

2.6.2.307.g37023ba

New x86 pci h/w will require dma operations specific to that domain. This patch allows those domains to register their operations, and sets devices as they are discovered in that domain to use them.

---
 arch/x86/include/asm/device.h | 10 ++++++++++
 arch/x86/pci/common.c         | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

index 03dd729..3b23897 100644
@@ -10,6 +10,16 @@ struct dev_archdata {
 #endif
 };
 
+#if defined(CONFIG_X86_DEV_DMA_OPS) && defined(CONFIG_PCI_DOMAINS)
+struct dma_domain {
+	struct list_head node;
+	struct dma_map_ops *dma_ops;
+	int domain_nr;
+};
+extern void add_dma_domain(struct dma_domain *domain);
+extern void del_dma_domain(struct dma_domain *domain);
+#endif
+
 struct pdev_archdata {
 };
 

index eccd4d9..106fd13 100644
@@ -641,6 +641,43 @@ unsigned int pcibios_assign_all_busses(void)
 	return (pci_probe & PCI_ASSIGN_ALL_BUSSES) ? 1 : 0;
 }
 
+#if defined(CONFIG_X86_DEV_DMA_OPS) && defined(CONFIG_PCI_DOMAINS)
+LIST_HEAD(dma_domain_list);
+DEFINE_SPINLOCK(dma_domain_list_lock);
+
+void add_dma_domain(struct dma_domain *domain)
+{
+	spin_lock(&dma_domain_list_lock);
+	list_add(&domain->node, &dma_domain_list);
+	spin_unlock(&dma_domain_list_lock);
+}
+EXPORT_SYMBOL_GPL(add_dma_domain);
+
+void del_dma_domain(struct dma_domain *domain)
+{
+	spin_lock(&dma_domain_list_lock);
+	list_del(&domain->node);
+	spin_unlock(&dma_domain_list_lock);
+}
+EXPORT_SYMBOL_GPL(del_dma_domain);
+
+static void set_dma_domain_ops(struct pci_dev *pdev)
+{
+	struct dma_domain *domain;
+
+	spin_lock(&dma_domain_list_lock);
+	list_for_each_entry(domain, &dma_domain_list, node) {
+		if (pci_domain_nr(pdev->bus) == domain->domain_nr) {
+			pdev->dev.archdata.dma_ops = domain->dma_ops;
+			break;
+		}
+	}
+	spin_unlock(&dma_domain_list_lock);
+}
+#else
+static void set_dma_domain_ops(struct pci_dev *pdev) {}
+#endif
+
 int pcibios_add_device(struct pci_dev *dev)
 {
 	struct setup_data *data;

@@ -670,6 +707,7 @@ int pcibios_add_device(struct pci_dev *dev)
 		pa_data = data->next;
 		iounmap(data);
 	}
+	set_dma_domain_ops(dev);
 	return 0;
 }
 

2.6.2.307.g37023ba

The Intel Volume Management Device (VMD) is an integrated endpoint on the platform's PCIe root complex that acts as a host bridge to a secondary PCIe domain. BIOS can reassign one or more root ports to appear within a VMD domain instead of the primary domain. The immediate benefit is that additional PCI-e domains allow more than 256 buses in a system by letting bus number be reused across different domains.

VMD domains do not define ACPI _SEG, so to avoid domain clashing with host bridges defining this segment, VMD domains start at 0x10000 which is greater than the highest possible 16-bit ACPI defined _SEG.

This driver enumerates and enables the domain using the root bus configuration interface provided by the PCI subsystem. The driver provides configuration space accessor functions (pci_ops), bus and memory resources, an MSI irq domain with irq_chip implementation, and dma operations necessary to use devices in through the VMD endpoint's interface.

VMD routes I/O as follows:

1) Configuration Space: BAR 0 ("CFGBAR") of VMD provides the base address and size for configuration space register access to VMD-owned root ports. It works similarly to MMCONFIG for extended configuration space. Bus numbering is independent and does not conflict with the primary domain.

2) MMIO Space: BARs 2 and 4 ("MEMBAR1" and "MEMBAR2") of VMD provide the base address, size, and type for MMIO register access. These addresses are not translated by VMD hardware; they are simply reservations to be distributed to root ports' memory base/limit registers and subdivided among devices downstream.

3) DMA: To interact appropriately with IOMMU, the source ID DMA read and write requests are translated to the bus-device-function of the VMD endpoint. Otherwise, DMA operates normally without VMD-specific address translation.

4) Interrupts: Part of VMD's BAR 4 is reserved for VMD's MSI-X Table and PBA. MSIs from VMD domain devices and ports are remapped to appear if they were issued using one of VMD's MSI-X table entries. Each MSI and MSI-X addresses of VMD-owned devices and ports have a special format where the address refers to specific entries in VMD's MSI-X table. As with DMA, the interrupt source id is translated to VMD's bus-device-function.

The driver provides its own MSI and MSI-X configuration functions specific to how MSI messages are used within the VMD domain, and provides an irq_chip for independent IRQ allocation to relay interrupts from VMD's interrupt handler to the appropriate device driver's handler.

5) Errors: PCIe error message are intercepted by the root ports normally (e.g. AER), except with VMD, system errors (i.e. firmware first) are disabled by default. AER and hotplug interrupts are translated in the same way as endpoint interrupts.

6) VMD does not support INTx interrupts or IO ports. Devices or drivers requiring these features should either not be placed below VMD-owned root ports, or VMD should be disabled by BIOS for such endpoints.

---
 arch/x86/Kconfig              |  13 +
 arch/x86/include/asm/hw_irq.h |   5 +
 arch/x86/pci/Makefile         |   2 +
 arch/x86/pci/vmd.c            | 698 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 718 insertions(+)
 create mode 100644 arch/x86/pci/vmd.c

index db3622f..9a5ab69 100644
@@ -2665,6 +2665,19 @@ config PMC_ATOM
 	def_bool y
         depends on PCI
 
+config VMD
+	depends on PCI_MSI
+	tristate "Volume Management Device Driver"
+	default N
+	---help---
+	  Adds support for the Intel Volume Manage Device (VMD). VMD is a
+	  secondary PCI host bridge that allows PCI Express root ports,
+	  and devices attached to them, to be removed from the default
+	  PCI domain and placed within the VMD domain. This provides
+	  additional bus resources than are otherwise possible with a
+	  single domain. If you know your system provides one of these and
+	  have devices attached to it, say Y; if you are not sure, say N.
+
 source "net/Kconfig"
 
 source "drivers/Kconfig"

index 1e3408e..79c35c4 100644
@@ -108,6 +108,11 @@ struct irq_alloc_info {
 			struct IO_APIC_route_entry *ioapic_entry;
 		};
 #endif
+#if IS_ENABLED(CONFIG_VMD)
+		struct {
+			struct msi_desc	*desc;
+		};
+#endif
 #ifdef	CONFIG_DMAR_TABLE
 		struct {
 			int		dmar_id;

index 5c6fc35..97062a6 100644
@@ -23,6 +23,8 @@ obj-y				+= bus_numa.o
 obj-$(CONFIG_AMD_NB)		+= amd_bus.o
 obj-$(CONFIG_PCI_CNB20LE_QUIRK)	+= broadcom_bus.o
 
+obj-$(CONFIG_VMD) += vmd.o
+
 ifeq ($(CONFIG_PCI_DEBUG),y)
 EXTRA_CFLAGS += -DDEBUG
 endif

2.6.2.307.g37023ba

New pci device provides additional pci domains that start above what 16 bits can address.

---
 drivers/pci/pcie/aer/aer_inject.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

index 182224a..20db790 100644
@@ -41,12 +41,12 @@ struct aer_error_inj {
 	u32 header_log1;
 	u32 header_log2;
 	u32 header_log3;
-	u16 domain;
+	u32 domain;
 };
 
 struct aer_error {
 	struct list_head list;
-	u16 domain;
+	u32 domain;
 	unsigned int bus;
 	unsigned int devfn;
 	int pos_cap_err;

@@ -74,7 +74,7 @@ static LIST_HEAD(pci_bus_ops_list);
 /* Protect einjected and pci_bus_ops_list */
 static DEFINE_SPINLOCK(inject_lock);
 
-static void aer_error_init(struct aer_error *err, u16 domain,
+static void aer_error_init(struct aer_error *err, u32 domain,
 			   unsigned int bus, unsigned int devfn,
 			   int pos_cap_err)
 {

@@ -86,7 +86,7 @@ static void aer_error_init(struct aer_error *err, u16 domain,
 }
 
 /* inject_lock must be held before calling */
-static struct aer_error *__find_aer_error(u16 domain, unsigned int bus,
+static struct aer_error *__find_aer_error(u32 domain, unsigned int bus,
 					  unsigned int devfn)
 {
 	struct aer_error *err;

@@ -106,7 +106,7 @@ static struct aer_error *__find_aer_error_by_dev(struct pci_dev *dev)
 	int domain = pci_domain_nr(dev->bus);
 	if (domain < 0)
 		return NULL;
-	return __find_aer_error((u16)domain, dev->bus->number, dev->devfn);
+	return __find_aer_error(domain, dev->bus->number, dev->devfn);
 }
 
 /* inject_lock must be held before calling */

@@ -196,7 +196,7 @@ static int pci_read_aer(struct pci_bus *bus, unsigned int devfn, int where,
 	domain = pci_domain_nr(bus);
 	if (domain < 0)
 		goto out;
-	err = __find_aer_error((u16)domain, bus->number, devfn);
+	err = __find_aer_error(domain, bus->number, devfn);
 	if (!err)
 		goto out;
 

@@ -228,7 +228,7 @@ static int pci_write_aer(struct pci_bus *bus, unsigned int devfn, int where,
 	domain = pci_domain_nr(bus);
 	if (domain < 0)
 		goto out;
-	err = __find_aer_error((u16)domain, bus->number, devfn);
+	err = __find_aer_error(domain, bus->number, devfn);
 	if (!err)
 		goto out;
 

@@ -329,7 +329,7 @@ static int aer_inject(struct aer_error_inj *einj)
 	u32 sever, cor_mask, uncor_mask, cor_mask_orig = 0, uncor_mask_orig = 0;
 	int ret = 0;
 
-	dev = pci_get_domain_bus_and_slot((int)einj->domain, einj->bus, devfn);
+	dev = pci_get_domain_bus_and_slot(einj->domain, einj->bus, devfn);
 	if (!dev)
 		return -ENODEV;
 	rpdev = pcie_find_root_port(dev);

2.6.2.307.g37023ba

Hi all,

I very much appreciate all the feedback received. I dropped the controversial PCI parts and the user space pci-utils from this series, and I think satisfied all the requests I am aware of. Is there anything else preventing a 4.5 inclusion?

Thanks a bunch!
Keith

decided by Bjorn.

Thanks,

tglx

Hi Keith,

Checkpatch complained about a few things that should be easy to fix up:

x86-pci-allow-pci-domain
-:26: WARNING: struct dma_map_ops should normally be const x86-pci-initial-commit-for-new
-:125: WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? -:168: WARNING: please, no space before tabs
-:187: WARNING: please, no space before tabs
-:212: WARNING: struct dma_map_ops should normally be const
-:269: WARNING: line over 80 characters
-:384: WARNING: Missing a blank line after declarations
-:391: WARNING: Missing a blank line after declarations

This is from:

+ struct device *vdev = dev_to_vmd_dev(dev);
+ return vdev->archdata.dma_ops->alloc(vdev, size, addr, flag, attrs);

There are a lot of these, and I'm not going to be obsessive about all these two-line functions. But maybe a macro or helper function could turn them all into one-line functions and side-step it, e.g.,:

  static struct dma_ops *vmd_dma_ops(struct device *dev)
  {
    return &dev_to_vmd_dev(dev)->archdata.dma_ops;
  }

  static void *vmd_alloc(...)
  {
    return vmd_dma_ops(dev)->alloc(...);
  }

-:398: WARNING: Missing a blank line after declarations
-:406: WARNING: Missing a blank line after declarations
-:415: WARNING: Missing a blank line after declarations
-:425: WARNING: Missing a blank line after declarations
-:434: WARNING: Missing a blank line after declarations
-:442: WARNING: Missing a blank line after declarations
-:450: WARNING: Missing a blank line after declarations
-:457: WARNING: Missing a blank line after declarations
-:464: WARNING: Missing a blank line after declarations
-:471: WARNING: Missing a blank line after declarations
-:478: WARNING: Missing a blank line after declarations
-:484: WARNING: Missing a blank line after declarations
-:490: WARNING: Missing a blank line after declarations
-:497: WARNING: Missing a blank line after declarations
-:509: ERROR: Macros with multiple statements should be enclosed in a do - while loop -:509: WARNING: macros should not use a trailing semicolon
-:510: ERROR: trailing statements should be on next line

We should definitely fix this one.

-:514: WARNING: struct dma_map_ops should normally be const -:515: WARNING: struct dma_map_ops should normally be const -:654: WARNING: line over 80 characters
-:656: WARNING: line over 80 characters
-:662: WARNING: line over 80 characters
-:664: WARNING: line over 80 characters
-:692: WARNING: line over 80 characters

These could be easily fixed with a "struct resource *res", which would make the code more readable as well.

-:747: ERROR: space required before the open parenthesis '('

Hi Bjorn,

Thanks for the catches. Most were simple fixes, and I used your cleanup suggestions for others. Will resend shortly.

Question on the maintenance: does this new driver file need a maintainer, or does it fall under arch/x86/pci/ umbrella? I'm happy to supply updates and/or fixes once this is merged, but I just want to understand who to direct patches/pull requests to if the need arises.

It might be worth adding a MAINTAINERS update, similar to the "PCI DRIVER" entries we have for host bridge drivers. In those cases, I take care of merging updates and sending them to Linus, but I do ask for an ack from the listed maintainer for non-trivial changes.

Bjorn