Driver for new VMD device
Subsystems (5)
v4 cover Driver for new VMD device
Version 4 after re-working the irq domain hierarchies.
v3 -> v4:
Patch from Jiang to allow parent-less msi irq domain.
Separated kernel API dependency exports into their own patch.
Removed the vmd irq domain hierarchy, and instead use a msi irq domain. This pushed all the irq initialization and teardown handling into the irq_chip implementation and reduced kernel dependencies.
Using devm_request_irq instead of an irq flow handler. This moves the irq life management to the devres API which simplifies teardown.
Implemented a smarter method of selecting the VMD vector to share.
Lots style fixes/updates and additional code comments.
The one review comment I have not fixed is the affinity hint. We are still developing a way to better handle this, so have left it as a error returning stub. It's less than optimal, but isn't more harmful than that.
Keith Busch (5):
pci: skip child bus with conflicting resources Export msi and irq functions for module use
x86-pci: allow pci domain specific dma ops x86/pci: Initial commit for new VMD device driver
pciutils: Allow 32-bit 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 | 646 ++++++++++++++++++++++++++++++++++++++++++ drivers/pci/msi.c | 2 + drivers/pci/probe.c | 10 +- kernel/irq/irqdomain.c | 1 + kernel/irq/msi.c | 8 +- 10 files changed, 730 insertions(+), 5 deletions(-) create mode 100644 arch/x86/pci/vmd.c
Signature · 1 line
2.6.2.307.g37023ba
PATCH v4 1/6 msi: Relax msi_domain_alloc() to support parentless MSI irqdomains
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(-)
kernel/irq/msi.c +5 -3
@@ -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);
Signature · 1 line
2.6.2.307.g37023ba
Re: [PATCHv4 1/6] msi: Relax msi_domain_alloc() to support parentless MSI irqdomains
Quoted 1 line · Keith Busch
From: Liu Jiang <[email protected]>
...
Quoted 2 lines
Signed-off-by Jiang Liu <[email protected]>Signed-off-by Liu Jiang <[email protected]>
Hehehe. Let me read that from/sob chain:
Liu wrote the patch and then handed it over to Jiang who sent it back to Liu and Liu sent it to Keith.
Now what's missing is the SOB from Keith to make that chain perfect
Other than that, this is the right solution.
Thanks,
tglx
PATCH v4 2/6 pci: skip child bus with conflicting resources
In reply to Keith BuschAnd use the max bus resource from the parent rather than assume 255.
--- drivers/pci/probe.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/pci/probe.c +8 -2
@@ -856,7 +856,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)if (!child) goto out; child->primary = primary; -pci_bus_insert_busn_res (child, secondary, subordinate); +if (!pci_bus_insert_busn_res (child, secondary, subordinate)) { +pci_remove_bus (child); + goto out; + } child->bridge_ctl = bctl; }
@@ -896,7 +899,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) child =pci_add_new_bus (bus, dev, max+1 );if (!child) goto out; -pci_bus_insert_busn_res (child, max+1 ,0xff ); +if (!pci_bus_insert_busn_res (child, max+1 , bus->busn_res.end)) { +pci_remove_bus (child); + goto out; + } } max++; buses = (buses &0xff000000 )
Signature · 1 line
2.6.2.307.g37023ba
PATCH v4 3/6 Export msi and irq functions for module use
In reply to Keith Busch--- drivers/pci/msi.c | 2 ++ kernel/irq/irqdomain.c | 1 + 2 files changed, 3 insertions(+)
drivers/pci/msi.c +2 -0
@@ -1119,6 +1119,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) {
@@ -1278,6 +1279,7 @@ struct irq_domain *pci_msi_create_irq_domain(struct device_node *node, 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
kernel/irq/irqdomain.c +1 -0
@@ -955,6 +955,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
Signature · 1 line
2.6.2.307.g37023ba
PATCH v4 4/6 x86-pci: allow pci domain specific dma ops
In reply to Keith BuschNew 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(+)
arch/x86/include/asm/device.h +10 -0
@@ -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 { };
arch/x86/pci/common.c +38 -0
@@ -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 ; }
Signature · 1 line
2.6.2.307.g37023ba
PATCH v4 5/6 x86/pci: Initial commit for new VMD device driver
In reply to Keith BuschThe 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 | 646 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 666 insertions(+) create mode 100644 arch/x86/pci/vmd.c
arch/x86/Kconfig +13 -0
@@ -2632,6 +2632,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"
arch/x86/include/asm/hw_irq.h +5 -0
@@ -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;
arch/x86/pci/Makefile +2 -0
@@ -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
arch/x86/pci/vmd.c new file +646 -0
646 lines not on this page · raw message
Signature · 1 line
2.6.2.307.g37023ba
Re: [PATCHv4 5/6] x86/pci: Initial commit for new VMD device driver
Keith,
Review
Quoted 2 lines · Keith Busch
(quoted diff) +2
-0+ +static DEFINE_SPINLOCK(list_lock);
Can you please make that DEFINE_RAW_SPINLOCK as it nests into the irq descriptor lock.
arch/x86/pci/vmd.c line 35
Quoted 5 lines
arch/x86/pci/vmd.c new file +5
-0+ +struct vmd_irq { +struct list_head node; +struct rcu_head rcu;/* rcu callback list */ +struct vmd_irq_list *irq;/* back pointer */
Please use KernelDoc style documentation if you want to document your data structures.
arch/x86/pci/vmd.c line 120
Quoted 9 lines
arch/x86/pci/vmd.c new file +9
-0+/** +* XXX: Stubbed until a good way to not create conflicts with other devices +* sharing the same vector is developed. +*/ +static int vmd_irq_set_affinity (struct irq_data *data,const struct cpumask *dest, +bool force) +{ +return -EINVAL ; +}
Well, this is a hard problem for shared vectors. The question is whether we really want to expose that at the per device level or rather at the demultiplexing level.
arch/x86/pci/vmd.c line 147
Quoted 12 lines
arch/x86/pci/vmd.c new file +12
-0+static struct vmd_irq_list *vmd_next_irq (struct vmd_dev *vmd) +{ +int i, best =0 ; + +spin_lock (&list_lock); +for (i =1 ; i < vmd->msix_count; i++) +if (vmd->irqs[i].count < vmd->irqs[best].count) + best = i; + vmd->irqs[best].count++; +spin_unlock (&list_lock); + +return &vmd->irqs[best];
That looks smarter. Though we might need some more complex way to do that when interrupt affinity comes into play.
arch/x86/pci/vmd.c line 549
Quoted 11 lines
arch/x86/pci/vmd.c new file +11
-0+static irqreturn_t vmd_irq (int irq,void *data) +{ +struct vmd_irq_list *irqs = data; +struct vmd_irq *vmdirq; + +rcu_read_lock (); +list_for_each_entry_rcu (vmdirq, &irqs->irq_list, node) +generic_handle_irq (vmdirq->virq); +rcu_read_unlock (); + +return IRQ_HANDLED ;
Please remind me, that I still need to fix that life time problem in the core.
arch/x86/pci/vmd.c line 605
Quoted 7 lines
arch/x86/pci/vmd.c new file +7
-0+for (i =0 ; i < vmd->msix_count; i++) { +INIT_LIST_HEAD (&vmd->irqs[i].irq_list); + vmd->irqs[i].vmd_vector = vmd->msix_entries[i].vector; + vmd->irqs[i].index = i; + + err =devm_request_irq (&dev->dev, vmd->irqs[i].vmd_vector, + vmd_irq,IRQF_SHARED ,"vmd" , &vmd->irqs[i]);
This is a MSI interrupt which is never shared.
Aside of the few nitpicks above I'm really happy with the interrupt side of that driver. Thanks for following up on all the comments!
Thanks,
tglx
PATCH v4 6/6 pciutils: Allow 32-bit domains
In reply to Keith BuschPCI-e segments will continue to use the lower 16 bits as required by ACPI. Special domains may use the full 32-bits.
--- lib/filter.c | 2 +- lib/pci.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
lib/filter.c +1 -1
@@ -45,7 +45,7 @@ pci_filter_parse_slot_v33(struct pci_filter *f, char *str)if (str[0 ] &&strcmp (str,"*" )) {long int x =strtol (str, &e,16 ); -if ((e && *e) || (x <0 || x >0xffff )) +if ((e && *e) || (x <0 ))return "Invalid domain number" ; f->domain = x; }
lib/pci.h +1 -1
@@ -119,7 +119,7 @@ struct pci_param *pci_walk_params(struct pci_access *acc, struct pci_param *prevstruct pci_dev {struct pci_dev *next;/* Next device in the chain */ -u16 domain;/* PCI domain (host bridge) */ +int32_t domain;/* PCI domain (host bridge) */ u8 bus, dev, func;/* Bus inside domain, device and function */ /* These fields are set by pci_fill_info() */
Signature · 1 line
1.7.10.4