* [PATCHv8 0/5] Driver for new "VMD" device
@ 2016-01-12 20:18 Keith Busch
2016-01-12 20:18 ` [PATCHv8 1/5] msi: Relax msi_domain_alloc() to support parentless MSI irqdomains Keith Busch
` (6 more replies)
0 siblings, 7 replies; 32+ messages in thread
From: Keith Busch @ 2016-01-12 20:18 UTC (permalink / raw)
To: LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas
Cc: Dan Williams, Bryan Veal, Jon Derrick, Keith Busch
Nothing changed; just contained in a single series, as requested.
Keith Busch (4):
x86/IRQ: Export IRQ domain function for module use
x86/PCI: Allow PCI domain specific dma ops
PCI/AER: Use 32 bit int type domains
x86/PCI: Initial commit for new VMD device driver
Liu Jiang (1):
msi: Relax msi_domain_alloc() to support parentless MSI irqdomains
MAINTAINERS | 6 +
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 | 699 ++++++++++++++++++++++++++++++++++++++
drivers/pci/pcie/aer/aer_inject.c | 16 +-
kernel/irq/irqdomain.c | 1 +
kernel/irq/msi.c | 8 +-
10 files changed, 787 insertions(+), 11 deletions(-)
create mode 100644 arch/x86/pci/vmd.c
--
2.6.2.307.g37023ba
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCHv8 1/5] msi: Relax msi_domain_alloc() to support parentless MSI irqdomains
2016-01-12 20:18 [PATCHv8 0/5] Driver for new "VMD" device Keith Busch
@ 2016-01-12 20:18 ` Keith Busch
2016-01-12 20:18 ` [PATCHv8 2/5] x86/IRQ: Export IRQ domain function for module use Keith Busch
` (5 subsequent siblings)
6 siblings, 0 replies; 32+ messages in thread
From: Keith Busch @ 2016-01-12 20:18 UTC (permalink / raw)
To: LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas
Cc: Dan Williams, Bryan Veal, Jon Derrick, Liu Jiang, Keith Busch
From: Liu Jiang <jiang.liu@linux.intel.com>
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.
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
kernel/irq/msi.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 15b249e..38e89ce 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCHv8 2/5] x86/IRQ: Export IRQ domain function for module use
2016-01-12 20:18 [PATCHv8 0/5] Driver for new "VMD" device Keith Busch
2016-01-12 20:18 ` [PATCHv8 1/5] msi: Relax msi_domain_alloc() to support parentless MSI irqdomains Keith Busch
@ 2016-01-12 20:18 ` Keith Busch
2016-01-12 20:18 ` [PATCHv8 3/5] x86/PCI: Allow PCI domain specific dma ops Keith Busch
` (4 subsequent siblings)
6 siblings, 0 replies; 32+ messages in thread
From: Keith Busch @ 2016-01-12 20:18 UTC (permalink / raw)
To: LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas
Cc: Dan Williams, Bryan Veal, Jon Derrick, Keith Busch
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
kernel/irq/irqdomain.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 8cf95de..6e655f7 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1061,6 +1061,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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCHv8 3/5] x86/PCI: Allow PCI domain specific dma ops
2016-01-12 20:18 [PATCHv8 0/5] Driver for new "VMD" device Keith Busch
2016-01-12 20:18 ` [PATCHv8 1/5] msi: Relax msi_domain_alloc() to support parentless MSI irqdomains Keith Busch
2016-01-12 20:18 ` [PATCHv8 2/5] x86/IRQ: Export IRQ domain function for module use Keith Busch
@ 2016-01-12 20:18 ` Keith Busch
2016-01-12 20:18 ` [PATCHv8 4/5] PCI/AER: Use 32 bit int type domains Keith Busch
` (3 subsequent siblings)
6 siblings, 0 replies; 32+ messages in thread
From: Keith Busch @ 2016-01-12 20:18 UTC (permalink / raw)
To: LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas
Cc: Dan Williams, Bryan Veal, Jon Derrick, Keith Busch
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.
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
arch/x86/include/asm/device.h | 10 ++++++++++
arch/x86/pci/common.c | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/arch/x86/include/asm/device.h b/arch/x86/include/asm/device.h
index 03dd729..3b23897 100644
--- a/arch/x86/include/asm/device.h
+++ b/arch/x86/include/asm/device.h
@@ -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 {
};
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index eccd4d9..106fd13 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCHv8 4/5] PCI/AER: Use 32 bit int type domains
2016-01-12 20:18 [PATCHv8 0/5] Driver for new "VMD" device Keith Busch
` (2 preceding siblings ...)
2016-01-12 20:18 ` [PATCHv8 3/5] x86/PCI: Allow PCI domain specific dma ops Keith Busch
@ 2016-01-12 20:18 ` Keith Busch
2016-01-12 20:18 ` [PATCHv8 5/5] x86/PCI: Initial commit for new VMD device driver Keith Busch
` (2 subsequent siblings)
6 siblings, 0 replies; 32+ messages in thread
From: Keith Busch @ 2016-01-12 20:18 UTC (permalink / raw)
To: LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas
Cc: Dan Williams, Bryan Veal, Jon Derrick, Keith Busch
New pci device provides additional pci domains that start above what 16
bits can address.
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
drivers/pci/pcie/aer/aer_inject.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/pcie/aer/aer_inject.c b/drivers/pci/pcie/aer/aer_inject.c
index 182224a..20db790 100644
--- a/drivers/pci/pcie/aer/aer_inject.c
+++ b/drivers/pci/pcie/aer/aer_inject.c
@@ -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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCHv8 5/5] x86/PCI: Initial commit for new VMD device driver
2016-01-12 20:18 [PATCHv8 0/5] Driver for new "VMD" device Keith Busch
` (3 preceding siblings ...)
2016-01-12 20:18 ` [PATCHv8 4/5] PCI/AER: Use 32 bit int type domains Keith Busch
@ 2016-01-12 20:18 ` Keith Busch
2016-01-15 18:19 ` [PATCHv8 0/5] Driver for new "VMD" device Bjorn Helgaas
2016-01-17 17:58 ` Christoph Hellwig
6 siblings, 0 replies; 32+ messages in thread
From: Keith Busch @ 2016-01-12 20:18 UTC (permalink / raw)
To: LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas
Cc: Dan Williams, Bryan Veal, Jon Derrick, Keith Busch
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.
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
MAINTAINERS | 6 +
arch/x86/Kconfig | 13 +
arch/x86/include/asm/hw_irq.h | 5 +
arch/x86/pci/Makefile | 2 +
arch/x86/pci/vmd.c | 699 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 725 insertions(+)
create mode 100644 arch/x86/pci/vmd.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 020a3be..d86a23c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8207,6 +8207,12 @@ S: Maintained
F: Documentation/devicetree/bindings/pci/host-generic-pci.txt
F: drivers/pci/host/pci-host-generic.c
+PCI DRIVER FOR VMD
+M: Keith Busch <keith.busch@intel.com>
+L: linux-pci@vger.kernel.org
+S: Supported
+F: arch/x86/pci/vmd.c
+
PCIE DRIVER FOR ST SPEAR13XX
M: Pratyush Anand <pratyush.anand@gmail.com>
L: linux-pci@vger.kernel.org
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 258965d..8cbcd64 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2684,6 +2684,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"
diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h
index 1e3408e..1815b73 100644
--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -130,6 +130,11 @@ struct irq_alloc_info {
char *uv_name;
};
#endif
+#if IS_ENABLED(CONFIG_VMD)
+ struct {
+ struct msi_desc *desc;
+ };
+#endif
};
};
diff --git a/arch/x86/pci/Makefile b/arch/x86/pci/Makefile
index 5c6fc35..97062a6 100644
--- a/arch/x86/pci/Makefile
+++ b/arch/x86/pci/Makefile
@@ -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
diff --git a/arch/x86/pci/vmd.c b/arch/x86/pci/vmd.c
new file mode 100644
index 0000000..3d6c23e
--- /dev/null
+++ b/arch/x86/pci/vmd.c
@@ -0,0 +1,699 @@
+/*
+ * Volume Management Device driver
+ * Copyright (c) 2015, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/msi.h>
+#include <linux/pci.h>
+#include <linux/rculist.h>
+#include <linux/rcupdate.h>
+
+#include <asm/irqdomain.h>
+#include <asm/device.h>
+#include <asm/msi.h>
+#include <asm/msidef.h>
+
+/*
+ * Lock for manipulating vmd irq lists.
+ */
+static DEFINE_RAW_SPINLOCK(list_lock);
+
+/**
+ * struct vmd_irq - private data to map driver irq to the VMD shared vector
+ * @node: list item for parent traversal.
+ * @rcu: rcu callback item for freeing.
+ * @irq: back pointer to parent.
+ * @virq: the virtual irq value provided to the requesting driver.
+ *
+ * Every MSI/MSI-x irq requested for a device in a VMD domain will be mapped to
+ * a VMD irq using this structure.
+ */
+struct vmd_irq {
+ struct list_head node;
+ struct rcu_head rcu;
+ struct vmd_irq_list *irq;
+ unsigned int virq;
+};
+
+/**
+ * struct vmd_irq_list - list of driver requested irq's mapping to a vmd vector
+ * @irq_list: the list of irq's the VMD one demuxes to.
+ * @vmd_vector: the h/w irq assigned to the VMD device.
+ * @index: index into the VMD MSI-x table; used for message routing.
+ * @count: number of child irqs assigned to this vector; used to track
+ * sharing.
+ */
+struct vmd_irq_list {
+ struct list_head irq_list;
+ struct vmd_dev *vmd;
+ unsigned int vmd_vector;
+ unsigned int index;
+ unsigned int count;
+};
+
+struct vmd_dev {
+ struct pci_dev *dev;
+
+ spinlock_t cfg_lock;
+ char __iomem *cfgbar;
+
+ int msix_count;
+ struct msix_entry *msix_entries;
+ struct vmd_irq_list *irqs;
+
+ struct pci_sysdata sysdata;
+ struct resource resources[3];
+ struct irq_domain *irq_domain;
+ struct pci_bus *bus;
+
+#ifdef CONFIG_X86_DEV_DMA_OPS
+ struct dma_map_ops dma_ops;
+ struct dma_domain dma_domain;
+#endif
+};
+
+static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus)
+{
+ return container_of(bus->sysdata, struct vmd_dev, sysdata);
+}
+
+/*
+ * Drivers managing a device in a VMD domain allocate their own irqs as before,
+ * but the MSI entry for the hardware it's driving will be programmed with a
+ * destination id for the VMD MSI-x table. The VMD device muxes interrupts in
+ * its domain into one of its own, and the VMD driver de-muxes these for the
+ * handlers sharing that VMD irq. The vmd irq_domain provides the operations
+ * and irq_chip to set this up.
+ */
+static void vmd_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
+{
+ struct vmd_irq *vmdirq = data->chip_data;
+ struct vmd_irq_list *irq = vmdirq->irq;
+
+ msg->address_hi = MSI_ADDR_BASE_HI;
+ msg->address_lo = MSI_ADDR_BASE_LO | MSI_ADDR_DEST_ID(irq->index);
+ msg->data = 0;
+}
+
+/*
+ * We rely on MSI_FLAG_USE_DEF_CHIP_OPS to set the irq mask/unmask ops.
+ */
+static void vmd_irq_enable(struct irq_data *data)
+{
+ struct vmd_irq *vmdirq = data->chip_data;
+
+ raw_spin_lock(&list_lock);
+ list_add_tail_rcu(&vmdirq->node, &vmdirq->irq->irq_list);
+ raw_spin_unlock(&list_lock);
+
+ data->chip->irq_unmask(data);
+}
+
+static void vmd_irq_disable(struct irq_data *data)
+{
+ struct vmd_irq *vmdirq = data->chip_data;
+
+ data->chip->irq_mask(data);
+
+ raw_spin_lock(&list_lock);
+ list_del_rcu(&vmdirq->node);
+ raw_spin_unlock(&list_lock);
+}
+
+/*
+ * XXX: Stubbed until we develop acceptable way to not create conflicts with
+ * other devices sharing the same vector.
+ */
+static int vmd_irq_set_affinity(struct irq_data *data,
+ const struct cpumask *dest, bool force)
+{
+ return -EINVAL;
+}
+
+static struct irq_chip vmd_msi_controller = {
+ .name = "VMD-MSI",
+ .irq_enable = vmd_irq_enable,
+ .irq_disable = vmd_irq_disable,
+ .irq_compose_msi_msg = vmd_compose_msi_msg,
+ .irq_set_affinity = vmd_irq_set_affinity,
+};
+
+static irq_hw_number_t vmd_get_hwirq(struct msi_domain_info *info,
+ msi_alloc_info_t *arg)
+{
+ return 0;
+}
+
+/*
+ * XXX: We can be even smarter selecting the best irq once we solve the
+ * affinity problem.
+ */
+static struct vmd_irq_list *vmd_next_irq(struct vmd_dev *vmd)
+{
+ int i, best = 0;
+
+ raw_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++;
+ raw_spin_unlock(&list_lock);
+
+ return &vmd->irqs[best];
+}
+
+static int vmd_msi_init(struct irq_domain *domain,
+ struct msi_domain_info *info,
+ unsigned int virq, irq_hw_number_t hwirq,
+ msi_alloc_info_t *arg)
+{
+ struct vmd_dev *vmd = vmd_from_bus(msi_desc_to_pci_dev(arg->desc)->bus);
+ struct vmd_irq *vmdirq = kzalloc(sizeof(*vmdirq), GFP_KERNEL);
+
+ if (!vmdirq)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&vmdirq->node);
+ vmdirq->irq = vmd_next_irq(vmd);
+ vmdirq->virq = virq;
+
+ irq_domain_set_info(domain, virq, vmdirq->irq->vmd_vector,
+ info->chip, vmdirq, handle_simple_irq, vmd, NULL);
+ return 0;
+}
+
+static void vmd_msi_free(struct irq_domain *domain,
+ struct msi_domain_info *info,
+ unsigned int virq)
+{
+ struct vmd_irq *vmdirq = irq_get_chip_data(virq);
+
+ /* XXX: Potential optimization to rebalance */
+ raw_spin_lock(&list_lock);
+ vmdirq->irq->count--;
+ raw_spin_unlock(&list_lock);
+
+ kfree_rcu(vmdirq, rcu);
+}
+
+static int vmd_msi_prepare(struct irq_domain *domain,
+ struct device *dev, int nvec,
+ msi_alloc_info_t *arg)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct vmd_dev *vmd = vmd_from_bus(pdev->bus);
+
+ if (nvec > vmd->msix_count)
+ return vmd->msix_count;
+ memset(arg, 0, sizeof(*arg));
+ return 0;
+}
+
+static void vmd_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc)
+{
+ arg->desc = desc;
+}
+
+static struct msi_domain_ops vmd_msi_domain_ops = {
+ .get_hwirq = vmd_get_hwirq,
+ .msi_init = vmd_msi_init,
+ .msi_free = vmd_msi_free,
+ .msi_prepare = vmd_msi_prepare,
+ .set_desc = vmd_set_desc,
+};
+
+static struct msi_domain_info vmd_msi_domain_info = {
+ .flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+ MSI_FLAG_PCI_MSIX,
+ .ops = &vmd_msi_domain_ops,
+ .chip = &vmd_msi_controller,
+};
+
+#ifdef CONFIG_X86_DEV_DMA_OPS
+/*
+ * VMD replaces the requester id with its own. DMA mappings for devices in a
+ * VMD domain need to be mapped for the VMD device, not the device requiring
+ * the mapping.
+ */
+static struct device *to_vmd_dev(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct vmd_dev *vmd = vmd_from_bus(pdev->bus);
+
+ return &vmd->dev->dev;
+}
+
+static struct dma_map_ops *vmd_dma_ops(struct device *dev)
+{
+ return to_vmd_dev(dev)->archdata.dma_ops;
+}
+
+static void *vmd_alloc(struct device *dev, size_t size, dma_addr_t *addr,
+ gfp_t flag, struct dma_attrs *attrs)
+{
+ return vmd_dma_ops(dev)->alloc(to_vmd_dev(dev), size, addr, flag,
+ attrs);
+}
+
+static void vmd_free(struct device *dev, size_t size, void *vaddr,
+ dma_addr_t addr, struct dma_attrs *attrs)
+{
+ return vmd_dma_ops(dev)->free(to_vmd_dev(dev), size, vaddr, addr,
+ attrs);
+}
+
+static int vmd_mmap(struct device *dev, struct vm_area_struct *vma,
+ void *cpu_addr, dma_addr_t addr,
+ size_t size, struct dma_attrs *attrs)
+{
+ return vmd_dma_ops(dev)->mmap(to_vmd_dev(dev), vma, cpu_addr, addr,
+ size, attrs);
+}
+
+static int vmd_get_sgtable(struct device *dev, struct sg_table *sgt,
+ void *cpu_addr, dma_addr_t addr,
+ size_t size, struct dma_attrs *attrs)
+{
+ return vmd_dma_ops(dev)->get_sgtable(to_vmd_dev(dev), sgt, cpu_addr,
+ addr, size, attrs);
+}
+
+static dma_addr_t vmd_map_page(struct device *dev, struct page *page,
+ unsigned long offset, size_t size,
+ enum dma_data_direction dir,
+ struct dma_attrs *attrs)
+{
+ return vmd_dma_ops(dev)->map_page(to_vmd_dev(dev), page, offset, size,
+ dir, attrs);
+}
+
+static void vmd_unmap_page(struct device *dev, dma_addr_t addr, size_t size,
+ enum dma_data_direction dir,
+ struct dma_attrs *attrs)
+{
+ vmd_dma_ops(dev)->unmap_page(to_vmd_dev(dev), addr, size, dir, attrs);
+}
+
+static int vmd_map_sg(struct device *dev, struct scatterlist *sg, int nents,
+ enum dma_data_direction dir,
+ struct dma_attrs *attrs)
+{
+ return vmd_dma_ops(dev)->map_sg(to_vmd_dev(dev), sg, nents, dir, attrs);
+}
+
+static void vmd_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
+ enum dma_data_direction dir,
+ struct dma_attrs *attrs)
+{
+ vmd_dma_ops(dev)->unmap_sg(to_vmd_dev(dev), sg, nents, dir, attrs);
+}
+
+static void vmd_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
+ size_t size, enum dma_data_direction dir)
+{
+ vmd_dma_ops(dev)->sync_single_for_cpu(to_vmd_dev(dev), addr, size, dir);
+}
+
+static void vmd_sync_single_for_device(struct device *dev, dma_addr_t addr,
+ size_t size, enum dma_data_direction dir)
+{
+ vmd_dma_ops(dev)->sync_single_for_device(to_vmd_dev(dev), addr, size,
+ dir);
+}
+
+static void vmd_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
+ int nents, enum dma_data_direction dir)
+{
+ vmd_dma_ops(dev)->sync_sg_for_cpu(to_vmd_dev(dev), sg, nents, dir);
+}
+
+static void vmd_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
+ int nents, enum dma_data_direction dir)
+{
+ vmd_dma_ops(dev)->sync_sg_for_device(to_vmd_dev(dev), sg, nents, dir);
+}
+
+static int vmd_mapping_error(struct device *dev, dma_addr_t addr)
+{
+ return vmd_dma_ops(dev)->mapping_error(to_vmd_dev(dev), addr);
+}
+
+static int vmd_dma_supported(struct device *dev, u64 mask)
+{
+ return vmd_dma_ops(dev)->dma_supported(to_vmd_dev(dev), mask);
+}
+
+#ifdef ARCH_HAS_DMA_GET_REQUIRED_MASK
+static u64 vmd_get_required_mask(struct device *dev)
+{
+ return vmd_dma_ops(dev)->get_required_mask(to_vmd_dev(dev));
+}
+#endif
+
+static void vmd_teardown_dma_ops(struct vmd_dev *vmd)
+{
+ struct dma_domain *domain = &vmd->dma_domain;
+
+ if (vmd->dev->dev.archdata.dma_ops)
+ del_dma_domain(domain);
+}
+
+#define ASSIGN_VMD_DMA_OPS(source, dest, fn) \
+ do { \
+ if (source->fn) \
+ dest->fn = vmd_##fn; \
+ } while (0)
+
+static void vmd_setup_dma_ops(struct vmd_dev *vmd)
+{
+ const struct dma_map_ops *source = vmd->dev->dev.archdata.dma_ops;
+ struct dma_map_ops *dest = &vmd->dma_ops;
+ struct dma_domain *domain = &vmd->dma_domain;
+
+ domain->domain_nr = vmd->sysdata.domain;
+ domain->dma_ops = dest;
+
+ if (!source)
+ return;
+ ASSIGN_VMD_DMA_OPS(source, dest, alloc);
+ ASSIGN_VMD_DMA_OPS(source, dest, free);
+ ASSIGN_VMD_DMA_OPS(source, dest, mmap);
+ ASSIGN_VMD_DMA_OPS(source, dest, get_sgtable);
+ ASSIGN_VMD_DMA_OPS(source, dest, map_page);
+ ASSIGN_VMD_DMA_OPS(source, dest, unmap_page);
+ ASSIGN_VMD_DMA_OPS(source, dest, map_sg);
+ ASSIGN_VMD_DMA_OPS(source, dest, unmap_sg);
+ ASSIGN_VMD_DMA_OPS(source, dest, sync_single_for_cpu);
+ ASSIGN_VMD_DMA_OPS(source, dest, sync_single_for_device);
+ ASSIGN_VMD_DMA_OPS(source, dest, sync_sg_for_cpu);
+ ASSIGN_VMD_DMA_OPS(source, dest, sync_sg_for_device);
+ ASSIGN_VMD_DMA_OPS(source, dest, mapping_error);
+ ASSIGN_VMD_DMA_OPS(source, dest, dma_supported);
+#ifdef ARCH_HAS_DMA_GET_REQUIRED_MASK
+ ASSIGN_VMD_DMA_OPS(source, dest, get_required_mask);
+#endif
+ add_dma_domain(domain);
+}
+#undef ASSIGN_VMD_DMA_OPS
+#else
+static void vmd_teardown_dma_ops(struct vmd_dev *vmd) {}
+static void vmd_setup_dma_ops(struct vmd_dev *vmd) {}
+#endif
+
+/*
+ * CPU may deadlock if config space is not serialized on some versions of this
+ * hardware, so all config space access is done under a spinlock.
+ */
+static int vmd_pci_read(struct pci_bus *bus, unsigned int devfn, int reg,
+ int len, u32 *value)
+{
+ int ret = 0;
+ unsigned long flags;
+ struct vmd_dev *vmd = vmd_from_bus(bus);
+ char __iomem *addr = vmd->cfgbar + (bus->number << 20) +
+ (devfn << 12) + reg;
+
+ if ((addr - vmd->cfgbar) + len >= resource_size(&vmd->dev->resource[0]))
+ return -EFAULT;
+
+ spin_lock_irqsave(&vmd->cfg_lock, flags);
+ switch (len) {
+ case 1:
+ *value = readb(addr);
+ break;
+ case 2:
+ *value = readw(addr);
+ break;
+ case 4:
+ *value = readl(addr);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ spin_unlock_irqrestore(&vmd->cfg_lock, flags);
+ return ret;
+}
+
+/*
+ * VMD h/w converts non-posted config writes to posted memory writes. The
+ * read-back in this function forces the completion so it returns only after
+ * the config space was written, as expected.
+ */
+static int vmd_pci_write(struct pci_bus *bus, unsigned int devfn, int reg,
+ int len, u32 value)
+{
+ int ret = 0;
+ unsigned long flags;
+ struct vmd_dev *vmd = vmd_from_bus(bus);
+ char __iomem *addr = vmd->cfgbar + (bus->number << 20) +
+ (devfn << 12) + reg;
+
+ if ((addr - vmd->cfgbar) + len >= resource_size(&vmd->dev->resource[0]))
+ return -EFAULT;
+
+ spin_lock_irqsave(&vmd->cfg_lock, flags);
+ switch (len) {
+ case 1:
+ writeb(value, addr);
+ readb(addr);
+ break;
+ case 2:
+ writew(value, addr);
+ readw(addr);
+ break;
+ case 4:
+ writel(value, addr);
+ readl(addr);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ spin_unlock_irqrestore(&vmd->cfg_lock, flags);
+ return ret;
+}
+
+static struct pci_ops vmd_ops = {
+ .read = vmd_pci_read,
+ .write = vmd_pci_write,
+};
+
+/*
+ * VMD domains start at 10000h to not clash with domains defining ACPI _SEG.
+ */
+static int vmd_find_free_domain(void)
+{
+ int domain = 0xffff;
+ struct pci_bus *bus = NULL;
+
+ while ((bus = pci_find_next_bus(bus)) != NULL)
+ domain = max_t(int, domain, pci_domain_nr(bus));
+ return domain + 1;
+}
+
+static int vmd_enable_domain(struct vmd_dev *vmd)
+{
+ struct pci_sysdata *sd = &vmd->sysdata;
+ struct resource *res;
+ LIST_HEAD(resources);
+
+ res = &vmd->resources[0];
+ res->name = "VMD CFGBAR";
+ res->start = 0;
+ res->end = (resource_size(&vmd->dev->resource[0]) >> 20) - 1;
+ res->flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED;
+
+ res = &vmd->resources[1];
+ res->name = "VMD MEMBAR1";
+ res->start = vmd->dev->resource[2].start;
+ res->end = vmd->dev->resource[2].end;
+ res->flags = (vmd->dev->resource[2].flags & ~IORESOURCE_SIZEALIGN) &
+ (!upper_32_bits(vmd->dev->resource[2].end) ?
+ ~IORESOURCE_MEM_64 : ~0);
+
+ res = &vmd->resources[2];
+ res->name = "VMD MEMBAR2";
+ res->start = vmd->dev->resource[4].start + 0x2000;
+ res->end = vmd->dev->resource[4].end;
+ res->flags = (vmd->dev->resource[4].flags & ~IORESOURCE_SIZEALIGN) &
+ (!upper_32_bits(vmd->dev->resource[4].end) ?
+ ~IORESOURCE_MEM_64 : ~0);
+
+ sd->domain = vmd_find_free_domain();
+ if (sd->domain < 0)
+ return sd->domain;
+ sd->node = pcibus_to_node(vmd->dev->bus);
+
+ vmd->irq_domain = pci_msi_create_irq_domain(NULL, &vmd_msi_domain_info,
+ NULL);
+ if (!vmd->irq_domain)
+ return -ENODEV;
+
+ pci_add_resource(&resources, &vmd->resources[0]);
+ pci_add_resource(&resources, &vmd->resources[1]);
+ pci_add_resource(&resources, &vmd->resources[2]);
+ vmd->bus = pci_create_root_bus(&vmd->dev->dev, 0, &vmd_ops, sd,
+ &resources);
+ if (!vmd->bus) {
+ pci_free_resource_list(&resources);
+ irq_domain_remove(vmd->irq_domain);
+ return -ENODEV;
+ }
+
+ vmd_setup_dma_ops(vmd);
+ dev_set_msi_domain(&vmd->bus->dev, vmd->irq_domain);
+ pci_rescan_bus(vmd->bus);
+
+ WARN(sysfs_create_link(&vmd->dev->dev.kobj, &vmd->bus->dev.kobj,
+ "domain"),
+ "Can't create symlink to domain\n");
+ return 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;
+}
+
+static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
+{
+ struct vmd_dev *vmd;
+ int i, err;
+
+ if (resource_size(&dev->resource[0]) < (1 << 20))
+ return -ENOMEM;
+
+ vmd = devm_kzalloc(&dev->dev, sizeof(*vmd), GFP_KERNEL);
+ if (!vmd)
+ return -ENOMEM;
+
+ vmd->dev = dev;
+ err = pcim_enable_device(dev);
+ if (err < 0)
+ return err;
+
+ vmd->cfgbar = pcim_iomap(dev, 0, 0);
+ if (!vmd->cfgbar)
+ return -ENOMEM;
+
+ pci_set_master(dev);
+ if (dma_set_mask_and_coherent(&dev->dev, DMA_BIT_MASK(64)) &&
+ dma_set_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32)))
+ return -ENODEV;
+
+ vmd->msix_count = pci_msix_vec_count(dev);
+ if (vmd->msix_count < 0)
+ return -ENODEV;
+
+ vmd->irqs = devm_kcalloc(&dev->dev, vmd->msix_count, sizeof(*vmd->irqs),
+ GFP_KERNEL);
+ if (!vmd->irqs)
+ return -ENOMEM;
+
+ vmd->msix_entries = devm_kcalloc(&dev->dev, vmd->msix_count,
+ sizeof(*vmd->msix_entries), GFP_KERNEL);
+ if (!vmd->msix_entries)
+ return -ENOMEM;
+ for (i = 0; i < vmd->msix_count; i++)
+ vmd->msix_entries[i].entry = i;
+
+ vmd->msix_count = pci_enable_msix_range(vmd->dev, vmd->msix_entries, 1,
+ vmd->msix_count);
+ if (vmd->msix_count < 0)
+ return vmd->msix_count;
+
+ 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, 0, "vmd", &vmd->irqs[i]);
+ if (err)
+ return err;
+ }
+
+ spin_lock_init(&vmd->cfg_lock);
+ pci_set_drvdata(dev, vmd);
+ err = vmd_enable_domain(vmd);
+ if (err)
+ return err;
+ dev_info(&vmd->dev->dev, "Bound to PCI domain:%x\n",
+ vmd->sysdata.domain);
+ return 0;
+}
+
+static void vmd_remove(struct pci_dev *dev)
+{
+ struct vmd_dev *vmd = pci_get_drvdata(dev);
+
+ pci_set_drvdata(dev, NULL);
+ sysfs_remove_link(&vmd->dev->dev.kobj, "domain");
+ pci_stop_root_bus(vmd->bus);
+ pci_remove_root_bus(vmd->bus);
+ vmd_teardown_dma_ops(vmd);
+ irq_domain_remove(vmd->irq_domain);
+}
+
+#ifdef CONFIG_PM
+static int vmd_suspend(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ pci_save_state(pdev);
+ return 0;
+}
+
+static int vmd_resume(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ pci_restore_state(pdev);
+ return 0;
+}
+#endif
+static SIMPLE_DEV_PM_OPS(vmd_dev_pm_ops, vmd_suspend, vmd_resume);
+
+static const struct pci_device_id vmd_ids[] = {
+ {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x201d),},
+ {0,}
+};
+MODULE_DEVICE_TABLE(pci, vmd_ids);
+
+static struct pci_driver vmd_drv = {
+ .name = "vmd",
+ .id_table = vmd_ids,
+ .probe = vmd_probe,
+ .remove = vmd_remove,
+ .driver = {
+ .pm = &vmd_dev_pm_ops,
+ },
+};
+module_pci_driver(vmd_drv);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION("0.6");
--
2.6.2.307.g37023ba
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-12 20:18 [PATCHv8 0/5] Driver for new "VMD" device Keith Busch
` (4 preceding siblings ...)
2016-01-12 20:18 ` [PATCHv8 5/5] x86/PCI: Initial commit for new VMD device driver Keith Busch
@ 2016-01-15 18:19 ` Bjorn Helgaas
2016-01-15 19:31 ` Veal, Bryan E.
` (3 more replies)
2016-01-17 17:58 ` Christoph Hellwig
6 siblings, 4 replies; 32+ messages in thread
From: Bjorn Helgaas @ 2016-01-15 18:19 UTC (permalink / raw)
To: Keith Busch
Cc: LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas,
Dan Williams, Bryan Veal, Jon Derrick
Hi Keith,
On Tue, Jan 12, 2016 at 01:18:05PM -0700, Keith Busch wrote:
> Nothing changed; just contained in a single series, as requested.
>
> Keith Busch (4):
> x86/IRQ: Export IRQ domain function for module use
> x86/PCI: Allow PCI domain specific dma ops
> PCI/AER: Use 32 bit int type domains
> x86/PCI: Initial commit for new VMD device driver
>
> Liu Jiang (1):
> msi: Relax msi_domain_alloc() to support parentless MSI irqdomains
>
> MAINTAINERS | 6 +
> 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 | 699 ++++++++++++++++++++++++++++++++++++++
> drivers/pci/pcie/aer/aer_inject.c | 16 +-
> kernel/irq/irqdomain.c | 1 +
> kernel/irq/msi.c | 8 +-
> 10 files changed, 787 insertions(+), 11 deletions(-)
> create mode 100644 arch/x86/pci/vmd.c
I applied these to pci/host-vmd with the changes below. Most of them
are cosmetic (rewrapping changelogs, fixing whitespace, etc.), but
there are a few I'd like you to take a close look at:
- Added VMD_CFGBAR and similar #defines
- Added vmd_cfg_addr() to factor out the addr computation and
validation
- Resource setup in vmd_enable_domain(). I suggested a temporary to
make the lines shorter. I had the vmd->dev->resource[n] in mind,
but you added a temporary for vmd->resources[n]. Either way is
fine, but I liked the look of the v7 init, so I reverted to that,
with a temporary for vmd->dev->resource[n].
- Flags setup in vmd_enable_domain(). This was pretty confusing,
and I *think* what I did is equivalent, but you should verify.
I also have a more substantive question about the flags setup. I
think you should not clear IORESOURCE_MEM_64. The intent of
IORESOURCE_MEM_64 is to describe the *capability* of a BAR, not its
contents. But I assume you cleared it for a reason. vmd->resources[n]
are not BARs, so the PCI core won't assign resources to them like it
does for BAR, so we shouldn't care about IORESOURCE_MEM_64 for that
reason. Is there some other reason IORESOURCE_MEM_64 makes a
difference there?
I'm still hoping to get this in during the merge window.
If you want to test this, I recommend using my git branch
https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-vmd
instead of applying the patch below on top of your v8. If you want
to make changes, post an incremental patch based on that branch.
Bjorn
@@ -1,114 +1,123 @@
-commit 7b1cd49008b8
+commit a0fea74a2b8f
Author: Keith Busch <keith.busch@intel.com>
Date: Tue Jan 12 13:18:10 2016 -0700
- x86/PCI: Initial commit for new VMD device driver
+ x86/PCI: Add driver for Intel Volume Management Device (VMD)
- 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.
+ The Intel Volume Management Device (VMD) is a Root Complex Integrated
+ Endpoint 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 PCIe domains
+ allow more than 256 buses in a system by letting bus numbers 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.
+ 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 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
+ 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
+ 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 an 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 as
+ if they were issued using one of VMD's MSI-X table entries. Each MSI
+ and MSI-X address of VMD-owned devices and ports has a special format
+ where the address refers to specific entries in the 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.
+ 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
+ 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.
+ [bhelgaas: add VMD BAR #defines, factor out vmd_cfg_addr(), whitespace]
Signed-off-by: Keith Busch <keith.busch@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
-commit 05e97692e217
+commit 60707d6ebbbe
Author: Keith Busch <keith.busch@intel.com>
Date: Tue Jan 12 13:18:09 2016 -0700
- PCI/AER: Use 32 bit int type domains
+ PCI/AER: Use 32 bit PCI domain numbers
- New pci device provides additional pci domains that start above what 16
- bits can address.
+ The Intel Volume Management Device (VMD) supports 32-bit domain numbers.
+ To accommodate this, use u32 instead of u16 to store domain numbers.
+ [bhelgaas: changelog]
Signed-off-by: Keith Busch <keith.busch@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
-commit 323121f05c8d
+commit cbab4ba90c61
Author: Keith Busch <keith.busch@intel.com>
Date: Tue Jan 12 13:18:08 2016 -0700
- x86/PCI: Allow PCI domain specific dma ops
+ x86/PCI: Allow DMA ops specific to a PCI domain
- 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.
+ The Intel Volume Management Device (VMD) is a PCIe endpoint that acts as a
+ host bridge to another PCI domain. When devices below the VMD perform DMA,
+ the VMD replaces their DMA source IDs with its own source ID. Therefore,
+ those devices require special DMA ops.
+ Add interfaces to allow the VMD driver to set up dma_ops for the devices
+ below it.
+
+ [bhelgaas: remove "extern", add "static", changelog]
Signed-off-by: Keith Busch <keith.busch@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
-commit b54bc8bc78d0
+commit db6a7b7a3887
Author: Keith Busch <keith.busch@intel.com>
Date: Tue Jan 12 13:18:07 2016 -0700
- x86/IRQ: Export IRQ domain function for module use
+ irqdomain: Export irq_domain_set_info() for module use
+
+ Export irq_domain_set_info() for module use. It will be used by the Volume
+ Management Device driver.
+ [bhelgaas: changelog]
Signed-off-by: Keith Busch <keith.busch@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
-commit 7c8c9dc397f2
+commit 23e42d853208
Author: Liu Jiang <jiang.liu@linux.intel.com>
Date: Tue Jan 12 13:18:06 2016 -0700
- msi: Relax msi_domain_alloc() to support parentless MSI irqdomains
+ genirq/MSI: Relax msi_domain_alloc() to support parentless MSI irqdomains
- Previously msi_domain_alloc() assumes MSI irqdomains always have parent
- irqdomains, but that's not true for the new Intel VMD devices. So relax
+ Previously msi_domain_alloc() assumed MSI irqdomains always had parent
+ irqdomains, but that's not true for the new Intel VMD devices. Relax
msi_domain_alloc() to support parentless MSI irqdomains.
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
diff --git a/MAINTAINERS b/MAINTAINERS
index 76369d4..ce47e08 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8216,7 +8216,7 @@ S: Maintained
F: Documentation/devicetree/bindings/pci/host-generic-pci.txt
F: drivers/pci/host/pci-host-generic.c
-PCI DRIVER FOR VMD
+PCI DRIVER FOR INTEL VOLUME MANAGEMENT DEVICE (VMD)
M: Keith Busch <keith.busch@intel.com>
L: linux-pci@vger.kernel.org
S: Supported
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 9a5ab69..3e6aca8 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2670,13 +2670,13 @@ config VMD
tristate "Volume Management Device Driver"
default N
---help---
- Adds support for the Intel Volume Manage Device (VMD). VMD is a
+ Adds support for the Intel Volume Management 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
+ more 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.
+ has devices attached to it, say Y; if you are not sure, say N.
source "net/Kconfig"
diff --git a/arch/x86/include/asm/device.h b/arch/x86/include/asm/device.h
index 3b23897..684ed6c 100644
--- a/arch/x86/include/asm/device.h
+++ b/arch/x86/include/asm/device.h
@@ -16,8 +16,8 @@ struct dma_domain {
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);
+void add_dma_domain(struct dma_domain *domain);
+void del_dma_domain(struct dma_domain *domain);
#endif
struct pdev_archdata {
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 106fd13..2879efc 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -642,8 +642,8 @@ unsigned int pcibios_assign_all_busses(void)
}
#if defined(CONFIG_X86_DEV_DMA_OPS) && defined(CONFIG_PCI_DOMAINS)
-LIST_HEAD(dma_domain_list);
-DEFINE_SPINLOCK(dma_domain_list_lock);
+static LIST_HEAD(dma_domain_list);
+static DEFINE_SPINLOCK(dma_domain_list_lock);
void add_dma_domain(struct dma_domain *domain)
{
diff --git a/arch/x86/pci/vmd.c b/arch/x86/pci/vmd.c
index 56ef447..d57e480 100644
--- a/arch/x86/pci/vmd.c
+++ b/arch/x86/pci/vmd.c
@@ -27,20 +27,24 @@
#include <asm/msi.h>
#include <asm/msidef.h>
+#define VMD_CFGBAR 0
+#define VMD_MEMBAR1 2
+#define VMD_MEMBAR2 4
+
/*
- * Lock for manipulating vmd irq lists.
+ * Lock for manipulating VMD IRQ lists.
*/
static DEFINE_RAW_SPINLOCK(list_lock);
/**
- * struct vmd_irq - private data to map driver irq to the VMD shared vector
+ * struct vmd_irq - private data to map driver IRQ to the VMD shared vector
* @node: list item for parent traversal.
- * @rcu: rcu callback item for freeing.
+ * @rcu: RCU callback item for freeing.
* @irq: back pointer to parent.
- * @virq: the virtual irq value provided to the requesting driver.
+ * @virq: the virtual IRQ value provided to the requesting driver.
*
- * Every MSI/MSI-x irq requested for a device in a VMD domain will be mapped to
- * a VMD irq using this structure.
+ * Every MSI/MSI-X IRQ requested for a device in a VMD domain will be mapped to
+ * a VMD IRQ using this structure.
*/
struct vmd_irq {
struct list_head node;
@@ -50,11 +54,11 @@ struct vmd_irq {
};
/**
- * struct vmd_irq_list - list of driver requested irq's mapping to a vmd vector
+ * struct vmd_irq_list - list of driver requested IRQs mapping to a VMD vector
* @irq_list: the list of irq's the VMD one demuxes to.
- * @vmd_vector: the h/w irq assigned to the VMD device.
- * @index: index into the VMD MSI-x table; used for message routing.
- * @count: number of child irqs assigned to this vector; used to track
+ * @vmd_vector: the h/w IRQ assigned to the VMD.
+ * @index: index into the VMD MSI-X table; used for message routing.
+ * @count: number of child IRQs assigned to this vector; used to track
* sharing.
*/
struct vmd_irq_list {
@@ -92,11 +96,11 @@ static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus)
}
/*
- * Drivers managing a device in a VMD domain allocate their own irqs as before,
+ * Drivers managing a device in a VMD domain allocate their own IRQs as before,
* but the MSI entry for the hardware it's driving will be programmed with a
- * destination id for the VMD MSI-x table. The VMD device muxes interrupts in
- * its domain into one of its own, and the VMD driver de-muxes these for the
- * handlers sharing that VMD irq. The vmd irq_domain provides the operations
+ * destination ID for the VMD MSI-X table. The VMD muxes interrupts in its
+ * domain into one of its own, and the VMD driver de-muxes these for the
+ * handlers sharing that VMD IRQ. The vmd irq_domain provides the operations
* and irq_chip to set this up.
*/
static void vmd_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
@@ -110,7 +114,7 @@ static void vmd_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
}
/*
- * We rely on MSI_FLAG_USE_DEF_CHIP_OPS to set the irq mask/unmask ops.
+ * We rely on MSI_FLAG_USE_DEF_CHIP_OPS to set the IRQ mask/unmask ops.
*/
static void vmd_irq_enable(struct irq_data *data)
{
@@ -139,7 +143,7 @@ static void vmd_irq_disable(struct irq_data *data)
* other devices sharing the same vector.
*/
static int vmd_irq_set_affinity(struct irq_data *data,
- const struct cpumask *dest, bool force)
+ const struct cpumask *dest, bool force)
{
return -EINVAL;
}
@@ -159,7 +163,7 @@ static irq_hw_number_t vmd_get_hwirq(struct msi_domain_info *info,
}
/*
- * XXX: We can be even smarter selecting the best irq once we solve the
+ * XXX: We can be even smarter selecting the best IRQ once we solve the
* affinity problem.
*/
static struct vmd_irq_list *vmd_next_irq(struct vmd_dev *vmd)
@@ -176,8 +180,7 @@ static struct vmd_irq_list *vmd_next_irq(struct vmd_dev *vmd)
return &vmd->irqs[best];
}
-static int vmd_msi_init(struct irq_domain *domain,
- struct msi_domain_info *info,
+static int vmd_msi_init(struct irq_domain *domain, struct msi_domain_info *info,
unsigned int virq, irq_hw_number_t hwirq,
msi_alloc_info_t *arg)
{
@@ -191,14 +194,13 @@ static int vmd_msi_init(struct irq_domain *domain,
vmdirq->irq = vmd_next_irq(vmd);
vmdirq->virq = virq;
- irq_domain_set_info(domain, virq, vmdirq->irq->vmd_vector,
- info->chip, vmdirq, handle_simple_irq, vmd, NULL);
+ irq_domain_set_info(domain, virq, vmdirq->irq->vmd_vector, info->chip,
+ vmdirq, handle_simple_irq, vmd, NULL);
return 0;
}
static void vmd_msi_free(struct irq_domain *domain,
- struct msi_domain_info *info,
- unsigned int virq)
+ struct msi_domain_info *info, unsigned int virq)
{
struct vmd_irq *vmdirq = irq_get_chip_data(virq);
@@ -210,15 +212,15 @@ static void vmd_msi_free(struct irq_domain *domain,
kfree_rcu(vmdirq, rcu);
}
-static int vmd_msi_prepare(struct irq_domain *domain,
- struct device *dev, int nvec,
- msi_alloc_info_t *arg)
+static int vmd_msi_prepare(struct irq_domain *domain, struct device *dev,
+ int nvec, msi_alloc_info_t *arg)
{
struct pci_dev *pdev = to_pci_dev(dev);
struct vmd_dev *vmd = vmd_from_bus(pdev->bus);
if (nvec > vmd->msix_count)
return vmd->msix_count;
+
memset(arg, 0, sizeof(*arg));
return 0;
}
@@ -245,8 +247,8 @@ static struct msi_domain_info vmd_msi_domain_info = {
#ifdef CONFIG_X86_DEV_DMA_OPS
/*
- * VMD replaces the requester id with its own. DMA mappings for devices in a
- * VMD domain need to be mapped for the VMD device, not the device requiring
+ * VMD replaces the requester ID with its own. DMA mappings for devices in a
+ * VMD domain need to be mapped for the VMD, not the device requiring
* the mapping.
*/
static struct device *to_vmd_dev(struct device *dev)
@@ -263,76 +265,73 @@ static struct dma_map_ops *vmd_dma_ops(struct device *dev)
}
static void *vmd_alloc(struct device *dev, size_t size, dma_addr_t *addr,
- gfp_t flag, struct dma_attrs *attrs)
+ gfp_t flag, struct dma_attrs *attrs)
{
return vmd_dma_ops(dev)->alloc(to_vmd_dev(dev), size, addr, flag,
- attrs);
+ attrs);
}
static void vmd_free(struct device *dev, size_t size, void *vaddr,
- dma_addr_t addr, struct dma_attrs *attrs)
+ dma_addr_t addr, struct dma_attrs *attrs)
{
return vmd_dma_ops(dev)->free(to_vmd_dev(dev), size, vaddr, addr,
- attrs);
+ attrs);
}
static int vmd_mmap(struct device *dev, struct vm_area_struct *vma,
- void *cpu_addr, dma_addr_t addr,
- size_t size, struct dma_attrs *attrs)
+ void *cpu_addr, dma_addr_t addr, size_t size,
+ struct dma_attrs *attrs)
{
return vmd_dma_ops(dev)->mmap(to_vmd_dev(dev), vma, cpu_addr, addr,
- size, attrs);
+ size, attrs);
}
static int vmd_get_sgtable(struct device *dev, struct sg_table *sgt,
- void *cpu_addr, dma_addr_t addr,
- size_t size, struct dma_attrs *attrs)
+ void *cpu_addr, dma_addr_t addr, size_t size,
+ struct dma_attrs *attrs)
{
return vmd_dma_ops(dev)->get_sgtable(to_vmd_dev(dev), sgt, cpu_addr,
- addr, size, attrs);
+ addr, size, attrs);
}
static dma_addr_t vmd_map_page(struct device *dev, struct page *page,
- unsigned long offset, size_t size,
- enum dma_data_direction dir,
- struct dma_attrs *attrs)
+ unsigned long offset, size_t size,
+ enum dma_data_direction dir,
+ struct dma_attrs *attrs)
{
return vmd_dma_ops(dev)->map_page(to_vmd_dev(dev), page, offset, size,
- dir, attrs);
+ dir, attrs);
}
static void vmd_unmap_page(struct device *dev, dma_addr_t addr, size_t size,
- enum dma_data_direction dir,
- struct dma_attrs *attrs)
+ enum dma_data_direction dir, struct dma_attrs *attrs)
{
vmd_dma_ops(dev)->unmap_page(to_vmd_dev(dev), addr, size, dir, attrs);
}
static int vmd_map_sg(struct device *dev, struct scatterlist *sg, int nents,
- enum dma_data_direction dir,
- struct dma_attrs *attrs)
+ enum dma_data_direction dir, struct dma_attrs *attrs)
{
return vmd_dma_ops(dev)->map_sg(to_vmd_dev(dev), sg, nents, dir, attrs);
}
static void vmd_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
- enum dma_data_direction dir,
- struct dma_attrs *attrs)
+ enum dma_data_direction dir, struct dma_attrs *attrs)
{
vmd_dma_ops(dev)->unmap_sg(to_vmd_dev(dev), sg, nents, dir, attrs);
}
static void vmd_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
- size_t size, enum dma_data_direction dir)
+ size_t size, enum dma_data_direction dir)
{
vmd_dma_ops(dev)->sync_single_for_cpu(to_vmd_dev(dev), addr, size, dir);
}
static void vmd_sync_single_for_device(struct device *dev, dma_addr_t addr,
- size_t size, enum dma_data_direction dir)
+ size_t size, enum dma_data_direction dir)
{
vmd_dma_ops(dev)->sync_single_for_device(to_vmd_dev(dev), addr, size,
- dir);
+ dir);
}
static void vmd_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
@@ -342,7 +341,7 @@ static void vmd_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
}
static void vmd_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
- int nents, enum dma_data_direction dir)
+ int nents, enum dma_data_direction dir)
{
vmd_dma_ops(dev)->sync_sg_for_device(to_vmd_dev(dev), sg, nents, dir);
}
@@ -414,20 +413,32 @@ static void vmd_teardown_dma_ops(struct vmd_dev *vmd) {}
static void vmd_setup_dma_ops(struct vmd_dev *vmd) {}
#endif
+static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
+ unsigned int devfn, int reg, int len)
+{
+ char __iomem *addr = vmd->cfgbar +
+ (bus->number << 20) + (devfn << 12) + reg;
+
+ if ((addr - vmd->cfgbar) + len >=
+ resource_size(&vmd->dev->resource[VMD_CFGBAR]))
+ return NULL;
+
+ return addr;
+}
+
/*
* CPU may deadlock if config space is not serialized on some versions of this
* hardware, so all config space access is done under a spinlock.
*/
static int vmd_pci_read(struct pci_bus *bus, unsigned int devfn, int reg,
- int len, u32 *value)
+ int len, u32 *value)
{
- int ret = 0;
- unsigned long flags;
struct vmd_dev *vmd = vmd_from_bus(bus);
- char __iomem *addr = vmd->cfgbar + (bus->number << 20) +
- (devfn << 12) + reg;
+ char __iomem *addr = vmd_cfg_addr(vmd, bus, devfn, reg, len);
+ unsigned long flags;
+ int ret = 0;
- if ((addr - vmd->cfgbar) + len >= resource_size(&vmd->dev->resource[0]))
+ if (!addr)
return -EFAULT;
spin_lock_irqsave(&vmd->cfg_lock, flags);
@@ -455,15 +466,14 @@ static int vmd_pci_read(struct pci_bus *bus, unsigned int devfn, int reg,
* the config space was written, as expected.
*/
static int vmd_pci_write(struct pci_bus *bus, unsigned int devfn, int reg,
- int len, u32 value)
+ int len, u32 value)
{
- int ret = 0;
- unsigned long flags;
struct vmd_dev *vmd = vmd_from_bus(bus);
- char __iomem *addr = vmd->cfgbar + (bus->number << 20) +
- (devfn << 12) + reg;
+ char __iomem *addr = vmd_cfg_addr(vmd, bus, devfn, reg, len);
+ unsigned long flags;
+ int ret = 0;
- if ((addr - vmd->cfgbar) + len >= resource_size(&vmd->dev->resource[0]))
+ if (!addr)
return -EFAULT;
spin_lock_irqsave(&vmd->cfg_lock, flags);
@@ -494,7 +504,7 @@ static struct pci_ops vmd_ops = {
};
/*
- * VMD domains start at 10000h to not clash with domains defining ACPI _SEG.
+ * VMD domains start at 0x1000 to not clash with ACPI _SEG domains.
*/
static int vmd_find_free_domain(void)
{
@@ -510,37 +520,50 @@ static int vmd_enable_domain(struct vmd_dev *vmd)
{
struct pci_sysdata *sd = &vmd->sysdata;
struct resource *res;
+ u32 upper_bits;
+ unsigned long flags;
LIST_HEAD(resources);
- res = &vmd->resources[0];
- res->name = "VMD CFGBAR";
- res->start = 0;
- res->end = (resource_size(&vmd->dev->resource[0]) >> 20) - 1;
- res->flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED;
-
- res = &vmd->resources[1];
- res->name = "VMD MEMBAR1";
- res->start = vmd->dev->resource[2].start;
- res->end = vmd->dev->resource[2].end;
- res->flags = (vmd->dev->resource[2].flags & ~IORESOURCE_SIZEALIGN) &
- (!upper_32_bits(vmd->dev->resource[2].end) ?
- ~IORESOURCE_MEM_64 : ~0);
-
- res = &vmd->resources[2];
- res->name = "VMD MEMBAR2";
- res->start = vmd->dev->resource[4].start + 0x2000;
- res->end = vmd->dev->resource[4].end;
- res->flags = (vmd->dev->resource[4].flags & ~IORESOURCE_SIZEALIGN) &
- (!upper_32_bits(vmd->dev->resource[4].end) ?
- ~IORESOURCE_MEM_64 : ~0);
+ res = &vmd->dev->resource[VMD_CFGBAR];
+ vmd->resources[0] = (struct resource) {
+ .name = "VMD CFGBAR",
+ .start = res->start,
+ .end = (resource_size(res) >> 20) - 1,
+ .flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
+ };
+
+ res = &vmd->dev->resource[VMD_MEMBAR1];
+ upper_bits = upper_32_bits(res->end);
+ flags = res->flags & ~IORESOURCE_SIZEALIGN;
+ if (!upper_bits)
+ flags &= ~IORESOURCE_MEM_64;
+ vmd->resources[1] = (struct resource) {
+ .name = "VMD MEMBAR1",
+ .start = res->start,
+ .end = res->end,
+ .flags = flags,
+ };
+
+ res = &vmd->dev->resource[VMD_MEMBAR2];
+ upper_bits = upper_32_bits(res->end);
+ flags = res->flags & ~IORESOURCE_SIZEALIGN;
+ if (!upper_bits)
+ flags &= ~IORESOURCE_MEM_64;
+ vmd->resources[2] = (struct resource) {
+ .name = "VMD MEMBAR2",
+ .start = res->start + 0x2000,
+ .end = res->end,
+ .flags = flags,
+ };
sd->domain = vmd_find_free_domain();
if (sd->domain < 0)
return sd->domain;
+
sd->node = pcibus_to_node(vmd->dev->bus);
vmd->irq_domain = pci_msi_create_irq_domain(NULL, &vmd_msi_domain_info,
- NULL);
+ NULL);
if (!vmd->irq_domain)
return -ENODEV;
@@ -548,7 +571,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd)
pci_add_resource(&resources, &vmd->resources[1]);
pci_add_resource(&resources, &vmd->resources[2]);
vmd->bus = pci_create_root_bus(&vmd->dev->dev, 0, &vmd_ops, sd,
- &resources);
+ &resources);
if (!vmd->bus) {
pci_free_resource_list(&resources);
irq_domain_remove(vmd->irq_domain);
@@ -560,8 +583,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd)
pci_rescan_bus(vmd->bus);
WARN(sysfs_create_link(&vmd->dev->dev.kobj, &vmd->bus->dev.kobj,
- "domain"),
- "Can't create symlink to domain\n");
+ "domain"), "Can't create symlink to domain\n");
return 0;
}
@@ -583,7 +605,7 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
struct vmd_dev *vmd;
int i, err;
- if (resource_size(&dev->resource[0]) < (1 << 20))
+ if (resource_size(&dev->resource[VMD_CFGBAR]) < (1 << 20))
return -ENOMEM;
vmd = devm_kzalloc(&dev->dev, sizeof(*vmd), GFP_KERNEL);
@@ -595,7 +617,7 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
if (err < 0)
return err;
- vmd->cfgbar = pcim_iomap(dev, 0, 0);
+ vmd->cfgbar = pcim_iomap(dev, VMD_CFGBAR, 0);
if (!vmd->cfgbar)
return -ENOMEM;
@@ -609,19 +631,20 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
return -ENODEV;
vmd->irqs = devm_kcalloc(&dev->dev, vmd->msix_count, sizeof(*vmd->irqs),
- GFP_KERNEL);
+ GFP_KERNEL);
if (!vmd->irqs)
return -ENOMEM;
vmd->msix_entries = devm_kcalloc(&dev->dev, vmd->msix_count,
- sizeof(*vmd->msix_entries), GFP_KERNEL);
+ sizeof(*vmd->msix_entries),
+ GFP_KERNEL);
if (!vmd->msix_entries)
return -ENOMEM;
for (i = 0; i < vmd->msix_count; i++)
vmd->msix_entries[i].entry = i;
vmd->msix_count = pci_enable_msix_range(vmd->dev, vmd->msix_entries, 1,
- vmd->msix_count);
+ vmd->msix_count);
if (vmd->msix_count < 0)
return vmd->msix_count;
@@ -631,7 +654,7 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
vmd->irqs[i].index = i;
err = devm_request_irq(&dev->dev, vmd->irqs[i].vmd_vector,
- vmd_irq, 0, "vmd", &vmd->irqs[i]);
+ vmd_irq, 0, "vmd", &vmd->irqs[i]);
if (err)
return err;
}
@@ -641,8 +664,9 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
err = vmd_enable_domain(vmd);
if (err)
return err;
- dev_info(&vmd->dev->dev, "Bound to PCI domain:%x\n",
- vmd->sysdata.domain);
+
+ dev_info(&vmd->dev->dev, "Bound to PCI domain %04x\n",
+ vmd->sysdata.domain);
return 0;
}
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-15 18:19 ` [PATCHv8 0/5] Driver for new "VMD" device Bjorn Helgaas
@ 2016-01-15 19:31 ` Veal, Bryan E.
2016-01-15 21:49 ` Bjorn Helgaas
2016-01-15 19:32 ` Keith Busch
` (2 subsequent siblings)
3 siblings, 1 reply; 32+ messages in thread
From: Veal, Bryan E. @ 2016-01-15 19:31 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Keith Busch, LKML, x86, linux-pci, Thomas Gleixner,
Bjorn Helgaas, Dan Williams, Jon Derrick
On Fri, Jan 15, 2016 at 12:19:38PM -0600, Bjorn Helgaas wrote:
> I also have a more substantive question about the flags setup. I
> think you should not clear IORESOURCE_MEM_64. The intent of
> IORESOURCE_MEM_64 is to describe the *capability* of a BAR, not its
> contents. But I assume you cleared it for a reason. vmd->resources[n]
> are not BARs, so the PCI core won't assign resources to them like it
> does for BAR, so we shouldn't care about IORESOURCE_MEM_64 for that
> reason. Is there some other reason IORESOURCE_MEM_64 makes a
> difference there?
Hi Bjorn & Keith:
I did this to fix an issue in pre-RFC code.
The flag is subtly restrictive in one specific scenario: spec-compliant
PCIe ports lack the ability to specify a 64-bit, non-prefetchable range.
IORESOURCE_MEM_64 directs the PCI subsystem to put the address into the
64-bit *prefetchable* range. Below the port, the "prefetchable" propoerty
*is* restrictive: the addresses can't be used for non-prefetchable BARs.
Thus, in the specific case where a 64-bit non-prefetchable VMD bar happens
to contain a 32-bit address, removing the IORESOURCE_MEM_64 flag allows
the address resource to be used for *any* non-prefetchable BARs (32-bit or
64-bit) downstream.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-15 18:19 ` [PATCHv8 0/5] Driver for new "VMD" device Bjorn Helgaas
2016-01-15 19:31 ` Veal, Bryan E.
@ 2016-01-15 19:32 ` Keith Busch
2016-01-15 19:44 ` Thomas Gleixner
2016-01-15 19:48 ` Derrick, Jonathan
3 siblings, 0 replies; 32+ messages in thread
From: Keith Busch @ 2016-01-15 19:32 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas,
Dan Williams, Bryan Veal, Jon Derrick
On Fri, Jan 15, 2016 at 12:19:38PM -0600, Bjorn Helgaas wrote:
> I applied these to pci/host-vmd with the changes below. Most of them
> are cosmetic (rewrapping changelogs, fixing whitespace, etc.), but
> there are a few I'd like you to take a close look at:
>
> - Added VMD_CFGBAR and similar #defines
> - Added vmd_cfg_addr() to factor out the addr computation and
> validation
> - Resource setup in vmd_enable_domain(). I suggested a temporary to
> make the lines shorter. I had the vmd->dev->resource[n] in mind,
> but you added a temporary for vmd->resources[n]. Either way is
> fine, but I liked the look of the v7 init, so I reverted to that,
> with a temporary for vmd->dev->resource[n].
> - Flags setup in vmd_enable_domain(). This was pretty confusing,
> and I *think* what I did is equivalent, but you should verify.
Thanks for the cleanups. All the new changes look good to me, and I will
test your tree today to confirm no regressions.
> I'm still hoping to get this in during the merge window.
Thanks a bunch. This would be great timing to align the hardware
availability with various software and OEM vendors.
> If you want to test this, I recommend using my git branch
> https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-vmd
> instead of applying the patch below on top of your v8. If you want
> to make changes, post an incremental patch based on that branch.
I'll give this a test today.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-15 18:19 ` [PATCHv8 0/5] Driver for new "VMD" device Bjorn Helgaas
2016-01-15 19:31 ` Veal, Bryan E.
2016-01-15 19:32 ` Keith Busch
@ 2016-01-15 19:44 ` Thomas Gleixner
2016-01-15 20:03 ` Bjorn Helgaas
2016-01-15 19:48 ` Derrick, Jonathan
3 siblings, 1 reply; 32+ messages in thread
From: Thomas Gleixner @ 2016-01-15 19:44 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Keith Busch, LKML, x86, linux-pci, Bjorn Helgaas, Dan Williams,
Bryan Veal, Jon Derrick
On Fri, 15 Jan 2016, Bjorn Helgaas wrote:
> + [bhelgaas: add VMD BAR #defines, factor out vmd_cfg_addr(), whitespace]
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Just for the record. You forgot my Acked-bys
Other than that the changes look good!
Thanks,
tglx
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-15 18:19 ` [PATCHv8 0/5] Driver for new "VMD" device Bjorn Helgaas
` (2 preceding siblings ...)
2016-01-15 19:44 ` Thomas Gleixner
@ 2016-01-15 19:48 ` Derrick, Jonathan
2016-01-15 19:54 ` Keith Busch
2016-01-15 22:06 ` Bjorn Helgaas
3 siblings, 2 replies; 32+ messages in thread
From: Derrick, Jonathan @ 2016-01-15 19:48 UTC (permalink / raw)
To: Bjorn Helgaas, Busch, Keith
Cc: LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas, Williams,
Dan J, Veal, Bryan E
Hi folks,
The VMD driver can be built as a module, so the following symbols need to be exported:
pci_msi_create_irq_domain
msi_desc_to_pci_dev
I'll post a follow-up patch shortly which applies to pci/host-vmd
Additionally, what are your thoughts of moving the menuconfig option from the root-tier to 'Processor Type and Features'?
-----Original Message-----
From: Bjorn Helgaas [mailto:helgaas@kernel.org]
Sent: Friday, January 15, 2016 11:20 AM
To: Busch, Keith <keith.busch@intel.com>
Cc: LKML <linux-kernel@vger.kernel.org>; x86@kernel.org; linux-pci@vger.kernel.org; Thomas Gleixner <tglx@linutronix.de>; Bjorn Helgaas <bhelgaas@google.com>; Williams, Dan J <dan.j.williams@intel.com>; Veal, Bryan E <bryan.e.veal@intel.com>; Derrick, Jonathan <jonathan.derrick@intel.com>
Subject: Re: [PATCHv8 0/5] Driver for new "VMD" device
Hi Keith,
On Tue, Jan 12, 2016 at 01:18:05PM -0700, Keith Busch wrote:
> Nothing changed; just contained in a single series, as requested.
>
> Keith Busch (4):
> x86/IRQ: Export IRQ domain function for module use
> x86/PCI: Allow PCI domain specific dma ops
> PCI/AER: Use 32 bit int type domains
> x86/PCI: Initial commit for new VMD device driver
>
> Liu Jiang (1):
> msi: Relax msi_domain_alloc() to support parentless MSI irqdomains
>
> MAINTAINERS | 6 +
> 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 | 699 ++++++++++++++++++++++++++++++++++++++
> drivers/pci/pcie/aer/aer_inject.c | 16 +-
> kernel/irq/irqdomain.c | 1 +
> kernel/irq/msi.c | 8 +-
> 10 files changed, 787 insertions(+), 11 deletions(-) create mode
> 100644 arch/x86/pci/vmd.c
I applied these to pci/host-vmd with the changes below. Most of them are cosmetic (rewrapping changelogs, fixing whitespace, etc.), but there are a few I'd like you to take a close look at:
- Added VMD_CFGBAR and similar #defines
- Added vmd_cfg_addr() to factor out the addr computation and
validation
- Resource setup in vmd_enable_domain(). I suggested a temporary to
make the lines shorter. I had the vmd->dev->resource[n] in mind,
but you added a temporary for vmd->resources[n]. Either way is
fine, but I liked the look of the v7 init, so I reverted to that,
with a temporary for vmd->dev->resource[n].
- Flags setup in vmd_enable_domain(). This was pretty confusing,
and I *think* what I did is equivalent, but you should verify.
I also have a more substantive question about the flags setup. I think you should not clear IORESOURCE_MEM_64. The intent of
IORESOURCE_MEM_64 is to describe the *capability* of a BAR, not its contents. But I assume you cleared it for a reason. vmd->resources[n] are not BARs, so the PCI core won't assign resources to them like it does for BAR, so we shouldn't care about IORESOURCE_MEM_64 for that reason. Is there some other reason IORESOURCE_MEM_64 makes a difference there?
I'm still hoping to get this in during the merge window.
If you want to test this, I recommend using my git branch https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-vmd
instead of applying the patch below on top of your v8. If you want to make changes, post an incremental patch based on that branch.
Bjorn
@@ -1,114 +1,123 @@
-commit 7b1cd49008b8
+commit a0fea74a2b8f
Author: Keith Busch <keith.busch@intel.com>
Date: Tue Jan 12 13:18:10 2016 -0700
- x86/PCI: Initial commit for new VMD device driver
+ x86/PCI: Add driver for Intel Volume Management Device (VMD)
- 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.
+ The Intel Volume Management Device (VMD) is a Root Complex Integrated
+ Endpoint 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 PCIe domains
+ allow more than 256 buses in a system by letting bus numbers 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.
+ 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 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
+ 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
+ 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 an 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 as
+ if they were issued using one of VMD's MSI-X table entries. Each MSI
+ and MSI-X address of VMD-owned devices and ports has a special format
+ where the address refers to specific entries in the 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.
+ 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
+ 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.
+ [bhelgaas: add VMD BAR #defines, factor out vmd_cfg_addr(),
+ whitespace]
Signed-off-by: Keith Busch <keith.busch@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
-commit 05e97692e217
+commit 60707d6ebbbe
Author: Keith Busch <keith.busch@intel.com>
Date: Tue Jan 12 13:18:09 2016 -0700
- PCI/AER: Use 32 bit int type domains
+ PCI/AER: Use 32 bit PCI domain numbers
- New pci device provides additional pci domains that start above what 16
- bits can address.
+ The Intel Volume Management Device (VMD) supports 32-bit domain numbers.
+ To accommodate this, use u32 instead of u16 to store domain numbers.
+ [bhelgaas: changelog]
Signed-off-by: Keith Busch <keith.busch@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
-commit 323121f05c8d
+commit cbab4ba90c61
Author: Keith Busch <keith.busch@intel.com>
Date: Tue Jan 12 13:18:08 2016 -0700
- x86/PCI: Allow PCI domain specific dma ops
+ x86/PCI: Allow DMA ops specific to a PCI domain
- 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.
+ The Intel Volume Management Device (VMD) is a PCIe endpoint that acts as a
+ host bridge to another PCI domain. When devices below the VMD perform DMA,
+ the VMD replaces their DMA source IDs with its own source ID. Therefore,
+ those devices require special DMA ops.
+ Add interfaces to allow the VMD driver to set up dma_ops for the devices
+ below it.
+
+ [bhelgaas: remove "extern", add "static", changelog]
Signed-off-by: Keith Busch <keith.busch@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
-commit b54bc8bc78d0
+commit db6a7b7a3887
Author: Keith Busch <keith.busch@intel.com>
Date: Tue Jan 12 13:18:07 2016 -0700
- x86/IRQ: Export IRQ domain function for module use
+ irqdomain: Export irq_domain_set_info() for module use
+
+ Export irq_domain_set_info() for module use. It will be used by the Volume
+ Management Device driver.
+ [bhelgaas: changelog]
Signed-off-by: Keith Busch <keith.busch@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
-commit 7c8c9dc397f2
+commit 23e42d853208
Author: Liu Jiang <jiang.liu@linux.intel.com>
Date: Tue Jan 12 13:18:06 2016 -0700
- msi: Relax msi_domain_alloc() to support parentless MSI irqdomains
+ genirq/MSI: Relax msi_domain_alloc() to support parentless MSI
+ irqdomains
- Previously msi_domain_alloc() assumes MSI irqdomains always have parent
- irqdomains, but that's not true for the new Intel VMD devices. So relax
+ Previously msi_domain_alloc() assumed MSI irqdomains always had parent
+ irqdomains, but that's not true for the new Intel VMD devices.
+ Relax
msi_domain_alloc() to support parentless MSI irqdomains.
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
diff --git a/MAINTAINERS b/MAINTAINERS
index 76369d4..ce47e08 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8216,7 +8216,7 @@ S: Maintained
F: Documentation/devicetree/bindings/pci/host-generic-pci.txt
F: drivers/pci/host/pci-host-generic.c
-PCI DRIVER FOR VMD
+PCI DRIVER FOR INTEL VOLUME MANAGEMENT DEVICE (VMD)
M: Keith Busch <keith.busch@intel.com>
L: linux-pci@vger.kernel.org
S: Supported
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 9a5ab69..3e6aca8 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2670,13 +2670,13 @@ config VMD
tristate "Volume Management Device Driver"
default N
---help---
- Adds support for the Intel Volume Manage Device (VMD). VMD is a
+ Adds support for the Intel Volume Management 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
+ more 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.
+ has devices attached to it, say Y; if you are not sure, say N.
source "net/Kconfig"
diff --git a/arch/x86/include/asm/device.h b/arch/x86/include/asm/device.h index 3b23897..684ed6c 100644
--- a/arch/x86/include/asm/device.h
+++ b/arch/x86/include/asm/device.h
@@ -16,8 +16,8 @@ struct dma_domain {
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);
+void add_dma_domain(struct dma_domain *domain); void
+del_dma_domain(struct dma_domain *domain);
#endif
struct pdev_archdata {
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c index 106fd13..2879efc 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -642,8 +642,8 @@ unsigned int pcibios_assign_all_busses(void) }
#if defined(CONFIG_X86_DEV_DMA_OPS) && defined(CONFIG_PCI_DOMAINS) -LIST_HEAD(dma_domain_list); -DEFINE_SPINLOCK(dma_domain_list_lock);
+static LIST_HEAD(dma_domain_list);
+static DEFINE_SPINLOCK(dma_domain_list_lock);
void add_dma_domain(struct dma_domain *domain) { diff --git a/arch/x86/pci/vmd.c b/arch/x86/pci/vmd.c index 56ef447..d57e480 100644
--- a/arch/x86/pci/vmd.c
+++ b/arch/x86/pci/vmd.c
@@ -27,20 +27,24 @@
#include <asm/msi.h>
#include <asm/msidef.h>
+#define VMD_CFGBAR 0
+#define VMD_MEMBAR1 2
+#define VMD_MEMBAR2 4
+
/*
- * Lock for manipulating vmd irq lists.
+ * Lock for manipulating VMD IRQ lists.
*/
static DEFINE_RAW_SPINLOCK(list_lock);
/**
- * struct vmd_irq - private data to map driver irq to the VMD shared vector
+ * struct vmd_irq - private data to map driver IRQ to the VMD shared
+ vector
* @node: list item for parent traversal.
- * @rcu: rcu callback item for freeing.
+ * @rcu: RCU callback item for freeing.
* @irq: back pointer to parent.
- * @virq: the virtual irq value provided to the requesting driver.
+ * @virq: the virtual IRQ value provided to the requesting driver.
*
- * Every MSI/MSI-x irq requested for a device in a VMD domain will be mapped to
- * a VMD irq using this structure.
+ * Every MSI/MSI-X IRQ requested for a device in a VMD domain will be
+ mapped to
+ * a VMD IRQ using this structure.
*/
struct vmd_irq {
struct list_head node;
@@ -50,11 +54,11 @@ struct vmd_irq {
};
/**
- * struct vmd_irq_list - list of driver requested irq's mapping to a vmd vector
+ * struct vmd_irq_list - list of driver requested IRQs mapping to a VMD
+ vector
* @irq_list: the list of irq's the VMD one demuxes to.
- * @vmd_vector: the h/w irq assigned to the VMD device.
- * @index: index into the VMD MSI-x table; used for message routing.
- * @count: number of child irqs assigned to this vector; used to track
+ * @vmd_vector: the h/w IRQ assigned to the VMD.
+ * @index: index into the VMD MSI-X table; used for message routing.
+ * @count: number of child IRQs assigned to this vector; used to track
* sharing.
*/
struct vmd_irq_list {
@@ -92,11 +96,11 @@ static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus) }
/*
- * Drivers managing a device in a VMD domain allocate their own irqs as before,
+ * Drivers managing a device in a VMD domain allocate their own IRQs as
+ before,
* but the MSI entry for the hardware it's driving will be programmed with a
- * destination id for the VMD MSI-x table. The VMD device muxes interrupts in
- * its domain into one of its own, and the VMD driver de-muxes these for the
- * handlers sharing that VMD irq. The vmd irq_domain provides the operations
+ * destination ID for the VMD MSI-X table. The VMD muxes interrupts in
+ its
+ * domain into one of its own, and the VMD driver de-muxes these for
+ the
+ * handlers sharing that VMD IRQ. The vmd irq_domain provides the
+ operations
* and irq_chip to set this up.
*/
static void vmd_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) @@ -110,7 +114,7 @@ static void vmd_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) }
/*
- * We rely on MSI_FLAG_USE_DEF_CHIP_OPS to set the irq mask/unmask ops.
+ * We rely on MSI_FLAG_USE_DEF_CHIP_OPS to set the IRQ mask/unmask ops.
*/
static void vmd_irq_enable(struct irq_data *data) { @@ -139,7 +143,7 @@ static void vmd_irq_disable(struct irq_data *data)
* other devices sharing the same vector.
*/
static int vmd_irq_set_affinity(struct irq_data *data,
- const struct cpumask *dest, bool force)
+ const struct cpumask *dest, bool force)
{
return -EINVAL;
}
@@ -159,7 +163,7 @@ static irq_hw_number_t vmd_get_hwirq(struct msi_domain_info *info, }
/*
- * XXX: We can be even smarter selecting the best irq once we solve the
+ * XXX: We can be even smarter selecting the best IRQ once we solve the
* affinity problem.
*/
static struct vmd_irq_list *vmd_next_irq(struct vmd_dev *vmd) @@ -176,8 +180,7 @@ static struct vmd_irq_list *vmd_next_irq(struct vmd_dev *vmd)
return &vmd->irqs[best];
}
-static int vmd_msi_init(struct irq_domain *domain,
- struct msi_domain_info *info,
+static int vmd_msi_init(struct irq_domain *domain, struct
+msi_domain_info *info,
unsigned int virq, irq_hw_number_t hwirq,
msi_alloc_info_t *arg)
{
@@ -191,14 +194,13 @@ static int vmd_msi_init(struct irq_domain *domain,
vmdirq->irq = vmd_next_irq(vmd);
vmdirq->virq = virq;
- irq_domain_set_info(domain, virq, vmdirq->irq->vmd_vector,
- info->chip, vmdirq, handle_simple_irq, vmd, NULL);
+ irq_domain_set_info(domain, virq, vmdirq->irq->vmd_vector, info->chip,
+ vmdirq, handle_simple_irq, vmd, NULL);
return 0;
}
static void vmd_msi_free(struct irq_domain *domain,
- struct msi_domain_info *info,
- unsigned int virq)
+ struct msi_domain_info *info, unsigned int virq)
{
struct vmd_irq *vmdirq = irq_get_chip_data(virq);
@@ -210,15 +212,15 @@ static void vmd_msi_free(struct irq_domain *domain,
kfree_rcu(vmdirq, rcu);
}
-static int vmd_msi_prepare(struct irq_domain *domain,
- struct device *dev, int nvec,
- msi_alloc_info_t *arg)
+static int vmd_msi_prepare(struct irq_domain *domain, struct device *dev,
+ int nvec, msi_alloc_info_t *arg)
{
struct pci_dev *pdev = to_pci_dev(dev);
struct vmd_dev *vmd = vmd_from_bus(pdev->bus);
if (nvec > vmd->msix_count)
return vmd->msix_count;
+
memset(arg, 0, sizeof(*arg));
return 0;
}
@@ -245,8 +247,8 @@ static struct msi_domain_info vmd_msi_domain_info = {
#ifdef CONFIG_X86_DEV_DMA_OPS
/*
- * VMD replaces the requester id with its own. DMA mappings for devices in a
- * VMD domain need to be mapped for the VMD device, not the device requiring
+ * VMD replaces the requester ID with its own. DMA mappings for
+ devices in a
+ * VMD domain need to be mapped for the VMD, not the device requiring
* the mapping.
*/
static struct device *to_vmd_dev(struct device *dev) @@ -263,76 +265,73 @@ static struct dma_map_ops *vmd_dma_ops(struct device *dev) }
static void *vmd_alloc(struct device *dev, size_t size, dma_addr_t *addr,
- gfp_t flag, struct dma_attrs *attrs)
+ gfp_t flag, struct dma_attrs *attrs)
{
return vmd_dma_ops(dev)->alloc(to_vmd_dev(dev), size, addr, flag,
- attrs);
+ attrs);
}
static void vmd_free(struct device *dev, size_t size, void *vaddr,
- dma_addr_t addr, struct dma_attrs *attrs)
+ dma_addr_t addr, struct dma_attrs *attrs)
{
return vmd_dma_ops(dev)->free(to_vmd_dev(dev), size, vaddr, addr,
- attrs);
+ attrs);
}
static int vmd_mmap(struct device *dev, struct vm_area_struct *vma,
- void *cpu_addr, dma_addr_t addr,
- size_t size, struct dma_attrs *attrs)
+ void *cpu_addr, dma_addr_t addr, size_t size,
+ struct dma_attrs *attrs)
{
return vmd_dma_ops(dev)->mmap(to_vmd_dev(dev), vma, cpu_addr, addr,
- size, attrs);
+ size, attrs);
}
static int vmd_get_sgtable(struct device *dev, struct sg_table *sgt,
- void *cpu_addr, dma_addr_t addr,
- size_t size, struct dma_attrs *attrs)
+ void *cpu_addr, dma_addr_t addr, size_t size,
+ struct dma_attrs *attrs)
{
return vmd_dma_ops(dev)->get_sgtable(to_vmd_dev(dev), sgt, cpu_addr,
- addr, size, attrs);
+ addr, size, attrs);
}
static dma_addr_t vmd_map_page(struct device *dev, struct page *page,
- unsigned long offset, size_t size,
- enum dma_data_direction dir,
- struct dma_attrs *attrs)
+ unsigned long offset, size_t size,
+ enum dma_data_direction dir,
+ struct dma_attrs *attrs)
{
return vmd_dma_ops(dev)->map_page(to_vmd_dev(dev), page, offset, size,
- dir, attrs);
+ dir, attrs);
}
static void vmd_unmap_page(struct device *dev, dma_addr_t addr, size_t size,
- enum dma_data_direction dir,
- struct dma_attrs *attrs)
+ enum dma_data_direction dir, struct dma_attrs *attrs)
{
vmd_dma_ops(dev)->unmap_page(to_vmd_dev(dev), addr, size, dir, attrs); }
static int vmd_map_sg(struct device *dev, struct scatterlist *sg, int nents,
- enum dma_data_direction dir,
- struct dma_attrs *attrs)
+ enum dma_data_direction dir, struct dma_attrs *attrs)
{
return vmd_dma_ops(dev)->map_sg(to_vmd_dev(dev), sg, nents, dir, attrs); }
static void vmd_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
- enum dma_data_direction dir,
- struct dma_attrs *attrs)
+ enum dma_data_direction dir, struct dma_attrs *attrs)
{
vmd_dma_ops(dev)->unmap_sg(to_vmd_dev(dev), sg, nents, dir, attrs); }
static void vmd_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
- size_t size, enum dma_data_direction dir)
+ size_t size, enum dma_data_direction dir)
{
vmd_dma_ops(dev)->sync_single_for_cpu(to_vmd_dev(dev), addr, size, dir); }
static void vmd_sync_single_for_device(struct device *dev, dma_addr_t addr,
- size_t size, enum dma_data_direction dir)
+ size_t size, enum dma_data_direction dir)
{
vmd_dma_ops(dev)->sync_single_for_device(to_vmd_dev(dev), addr, size,
- dir);
+ dir);
}
static void vmd_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg, @@ -342,7 +341,7 @@ static void vmd_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg, }
static void vmd_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
- int nents, enum dma_data_direction dir)
+ int nents, enum dma_data_direction dir)
{
vmd_dma_ops(dev)->sync_sg_for_device(to_vmd_dev(dev), sg, nents, dir); } @@ -414,20 +413,32 @@ static void vmd_teardown_dma_ops(struct vmd_dev *vmd) {} static void vmd_setup_dma_ops(struct vmd_dev *vmd) {} #endif
+static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
+ unsigned int devfn, int reg, int len) {
+ char __iomem *addr = vmd->cfgbar +
+ (bus->number << 20) + (devfn << 12) + reg;
+
+ if ((addr - vmd->cfgbar) + len >=
+ resource_size(&vmd->dev->resource[VMD_CFGBAR]))
+ return NULL;
+
+ return addr;
+}
+
/*
* CPU may deadlock if config space is not serialized on some versions of this
* hardware, so all config space access is done under a spinlock.
*/
static int vmd_pci_read(struct pci_bus *bus, unsigned int devfn, int reg,
- int len, u32 *value)
+ int len, u32 *value)
{
- int ret = 0;
- unsigned long flags;
struct vmd_dev *vmd = vmd_from_bus(bus);
- char __iomem *addr = vmd->cfgbar + (bus->number << 20) +
- (devfn << 12) + reg;
+ char __iomem *addr = vmd_cfg_addr(vmd, bus, devfn, reg, len);
+ unsigned long flags;
+ int ret = 0;
- if ((addr - vmd->cfgbar) + len >= resource_size(&vmd->dev->resource[0]))
+ if (!addr)
return -EFAULT;
spin_lock_irqsave(&vmd->cfg_lock, flags); @@ -455,15 +466,14 @@ static int vmd_pci_read(struct pci_bus *bus, unsigned int devfn, int reg,
* the config space was written, as expected.
*/
static int vmd_pci_write(struct pci_bus *bus, unsigned int devfn, int reg,
- int len, u32 value)
+ int len, u32 value)
{
- int ret = 0;
- unsigned long flags;
struct vmd_dev *vmd = vmd_from_bus(bus);
- char __iomem *addr = vmd->cfgbar + (bus->number << 20) +
- (devfn << 12) + reg;
+ char __iomem *addr = vmd_cfg_addr(vmd, bus, devfn, reg, len);
+ unsigned long flags;
+ int ret = 0;
- if ((addr - vmd->cfgbar) + len >= resource_size(&vmd->dev->resource[0]))
+ if (!addr)
return -EFAULT;
spin_lock_irqsave(&vmd->cfg_lock, flags); @@ -494,7 +504,7 @@ static struct pci_ops vmd_ops = { };
/*
- * VMD domains start at 10000h to not clash with domains defining ACPI _SEG.
+ * VMD domains start at 0x1000 to not clash with ACPI _SEG domains.
*/
static int vmd_find_free_domain(void)
{
@@ -510,37 +520,50 @@ static int vmd_enable_domain(struct vmd_dev *vmd) {
struct pci_sysdata *sd = &vmd->sysdata;
struct resource *res;
+ u32 upper_bits;
+ unsigned long flags;
LIST_HEAD(resources);
- res = &vmd->resources[0];
- res->name = "VMD CFGBAR";
- res->start = 0;
- res->end = (resource_size(&vmd->dev->resource[0]) >> 20) - 1;
- res->flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED;
-
- res = &vmd->resources[1];
- res->name = "VMD MEMBAR1";
- res->start = vmd->dev->resource[2].start;
- res->end = vmd->dev->resource[2].end;
- res->flags = (vmd->dev->resource[2].flags & ~IORESOURCE_SIZEALIGN) &
- (!upper_32_bits(vmd->dev->resource[2].end) ?
- ~IORESOURCE_MEM_64 : ~0);
-
- res = &vmd->resources[2];
- res->name = "VMD MEMBAR2";
- res->start = vmd->dev->resource[4].start + 0x2000;
- res->end = vmd->dev->resource[4].end;
- res->flags = (vmd->dev->resource[4].flags & ~IORESOURCE_SIZEALIGN) &
- (!upper_32_bits(vmd->dev->resource[4].end) ?
- ~IORESOURCE_MEM_64 : ~0);
+ res = &vmd->dev->resource[VMD_CFGBAR];
+ vmd->resources[0] = (struct resource) {
+ .name = "VMD CFGBAR",
+ .start = res->start,
+ .end = (resource_size(res) >> 20) - 1,
+ .flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
+ };
+
+ res = &vmd->dev->resource[VMD_MEMBAR1];
+ upper_bits = upper_32_bits(res->end);
+ flags = res->flags & ~IORESOURCE_SIZEALIGN;
+ if (!upper_bits)
+ flags &= ~IORESOURCE_MEM_64;
+ vmd->resources[1] = (struct resource) {
+ .name = "VMD MEMBAR1",
+ .start = res->start,
+ .end = res->end,
+ .flags = flags,
+ };
+
+ res = &vmd->dev->resource[VMD_MEMBAR2];
+ upper_bits = upper_32_bits(res->end);
+ flags = res->flags & ~IORESOURCE_SIZEALIGN;
+ if (!upper_bits)
+ flags &= ~IORESOURCE_MEM_64;
+ vmd->resources[2] = (struct resource) {
+ .name = "VMD MEMBAR2",
+ .start = res->start + 0x2000,
+ .end = res->end,
+ .flags = flags,
+ };
sd->domain = vmd_find_free_domain();
if (sd->domain < 0)
return sd->domain;
+
sd->node = pcibus_to_node(vmd->dev->bus);
vmd->irq_domain = pci_msi_create_irq_domain(NULL, &vmd_msi_domain_info,
- NULL);
+ NULL);
if (!vmd->irq_domain)
return -ENODEV;
@@ -548,7 +571,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd)
pci_add_resource(&resources, &vmd->resources[1]);
pci_add_resource(&resources, &vmd->resources[2]);
vmd->bus = pci_create_root_bus(&vmd->dev->dev, 0, &vmd_ops, sd,
- &resources);
+ &resources);
if (!vmd->bus) {
pci_free_resource_list(&resources);
irq_domain_remove(vmd->irq_domain);
@@ -560,8 +583,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd)
pci_rescan_bus(vmd->bus);
WARN(sysfs_create_link(&vmd->dev->dev.kobj, &vmd->bus->dev.kobj,
- "domain"),
- "Can't create symlink to domain\n");
+ "domain"), "Can't create symlink to domain\n");
return 0;
}
@@ -583,7 +605,7 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
struct vmd_dev *vmd;
int i, err;
- if (resource_size(&dev->resource[0]) < (1 << 20))
+ if (resource_size(&dev->resource[VMD_CFGBAR]) < (1 << 20))
return -ENOMEM;
vmd = devm_kzalloc(&dev->dev, sizeof(*vmd), GFP_KERNEL); @@ -595,7 +617,7 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
if (err < 0)
return err;
- vmd->cfgbar = pcim_iomap(dev, 0, 0);
+ vmd->cfgbar = pcim_iomap(dev, VMD_CFGBAR, 0);
if (!vmd->cfgbar)
return -ENOMEM;
@@ -609,19 +631,20 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
return -ENODEV;
vmd->irqs = devm_kcalloc(&dev->dev, vmd->msix_count, sizeof(*vmd->irqs),
- GFP_KERNEL);
+ GFP_KERNEL);
if (!vmd->irqs)
return -ENOMEM;
vmd->msix_entries = devm_kcalloc(&dev->dev, vmd->msix_count,
- sizeof(*vmd->msix_entries), GFP_KERNEL);
+ sizeof(*vmd->msix_entries),
+ GFP_KERNEL);
if (!vmd->msix_entries)
return -ENOMEM;
for (i = 0; i < vmd->msix_count; i++)
vmd->msix_entries[i].entry = i;
vmd->msix_count = pci_enable_msix_range(vmd->dev, vmd->msix_entries, 1,
- vmd->msix_count);
+ vmd->msix_count);
if (vmd->msix_count < 0)
return vmd->msix_count;
@@ -631,7 +654,7 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
vmd->irqs[i].index = i;
err = devm_request_irq(&dev->dev, vmd->irqs[i].vmd_vector,
- vmd_irq, 0, "vmd", &vmd->irqs[i]);
+ vmd_irq, 0, "vmd", &vmd->irqs[i]);
if (err)
return err;
}
@@ -641,8 +664,9 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
err = vmd_enable_domain(vmd);
if (err)
return err;
- dev_info(&vmd->dev->dev, "Bound to PCI domain:%x\n",
- vmd->sysdata.domain);
+
+ dev_info(&vmd->dev->dev, "Bound to PCI domain %04x\n",
+ vmd->sysdata.domain);
return 0;
}
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-15 19:48 ` Derrick, Jonathan
@ 2016-01-15 19:54 ` Keith Busch
2016-01-15 20:02 ` Jon Derrick
2016-01-15 22:06 ` Bjorn Helgaas
1 sibling, 1 reply; 32+ messages in thread
From: Keith Busch @ 2016-01-15 19:54 UTC (permalink / raw)
To: Derrick, Jonathan
Cc: Bjorn Helgaas, LKML, x86, linux-pci, Thomas Gleixner,
Bjorn Helgaas, Williams, Dan J, Veal, Bryan E
On Fri, Jan 15, 2016 at 11:48:11AM -0800, Derrick, Jonathan wrote:
> Hi folks,
>
> The VMD driver can be built as a module, so the following symbols need to be exported:
> pci_msi_create_irq_domain
> msi_desc_to_pci_dev
Hi Jon,
Your tree is a little old. It's added in commit a4289dc2ec3a5:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a4289dc2ec3a5821076a78ee9678909b4eff297e
But since you mention it, this is not in the pci repo yet either, but
it's a clean cherry-pick for us to test.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-15 19:54 ` Keith Busch
@ 2016-01-15 20:02 ` Jon Derrick
0 siblings, 0 replies; 32+ messages in thread
From: Jon Derrick @ 2016-01-15 20:02 UTC (permalink / raw)
To: Keith Busch
Cc: Bjorn Helgaas, LKML, x86, linux-pci, Thomas Gleixner,
Bjorn Helgaas, Williams, Dan J, Veal, Bryan E
On Fri, Jan 15, 2016 at 07:54:08PM +0000, Keith Busch wrote:
> On Fri, Jan 15, 2016 at 11:48:11AM -0800, Derrick, Jonathan wrote:
> > Hi folks,
> >
> > The VMD driver can be built as a module, so the following symbols need to be exported:
> > pci_msi_create_irq_domain
> > msi_desc_to_pci_dev
>
> Hi Jon,
>
> Your tree is a little old. It's added in commit a4289dc2ec3a5:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a4289dc2ec3a5821076a78ee9678909b4eff297e
>
> But since you mention it, this is not in the pci repo yet either, but
> it's a clean cherry-pick for us to test.
Aha, good catch
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-15 19:44 ` Thomas Gleixner
@ 2016-01-15 20:03 ` Bjorn Helgaas
2016-01-15 20:14 ` Thomas Gleixner
0 siblings, 1 reply; 32+ messages in thread
From: Bjorn Helgaas @ 2016-01-15 20:03 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Bjorn Helgaas, Keith Busch, LKML, x86, linux-pci, Dan Williams,
Bryan Veal, Jon Derrick
On Fri, Jan 15, 2016 at 11:44 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 15 Jan 2016, Bjorn Helgaas wrote:
>> + [bhelgaas: add VMD BAR #defines, factor out vmd_cfg_addr(), whitespace]
>> Signed-off-by: Keith Busch <keith.busch@intel.com>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>
> Just for the record. You forgot my Acked-bys
Sorry, I added them to the first two patches:
irqdomain: Export irq_domain_set_info() for module use
genirq/MSI: Relax msi_domain_alloc() to support parentless MSI irqdomains
Let me know if you intended to ack any of the others, too.
Bjorn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-15 20:03 ` Bjorn Helgaas
@ 2016-01-15 20:14 ` Thomas Gleixner
0 siblings, 0 replies; 32+ messages in thread
From: Thomas Gleixner @ 2016-01-15 20:14 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bjorn Helgaas, Keith Busch, LKML, x86, linux-pci, Dan Williams,
Bryan Veal, Jon Derrick
On Fri, 15 Jan 2016, Bjorn Helgaas wrote:
> On Fri, Jan 15, 2016 at 11:44 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Fri, 15 Jan 2016, Bjorn Helgaas wrote:
> >> + [bhelgaas: add VMD BAR #defines, factor out vmd_cfg_addr(), whitespace]
> >> Signed-off-by: Keith Busch <keith.busch@intel.com>
> >> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> >
> > Just for the record. You forgot my Acked-bys
>
> Sorry, I added them to the first two patches:
>
> irqdomain: Export irq_domain_set_info() for module use
> genirq/MSI: Relax msi_domain_alloc() to support parentless MSI irqdomains
>
> Let me know if you intended to ack any of the others, too.
The vmd driver itself (at least for the irq related parts). But you don't have
to redo it.
Thanks,
tglx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-15 19:31 ` Veal, Bryan E.
@ 2016-01-15 21:49 ` Bjorn Helgaas
2016-01-16 22:19 ` Veal, Bryan E.
0 siblings, 1 reply; 32+ messages in thread
From: Bjorn Helgaas @ 2016-01-15 21:49 UTC (permalink / raw)
To: Veal, Bryan E.
Cc: Keith Busch, LKML, x86, linux-pci, Thomas Gleixner,
Bjorn Helgaas, Dan Williams, Jon Derrick
On Fri, Jan 15, 2016 at 11:31:03AM -0800, Veal, Bryan E. wrote:
> On Fri, Jan 15, 2016 at 12:19:38PM -0600, Bjorn Helgaas wrote:
> > I also have a more substantive question about the flags setup. I
> > think you should not clear IORESOURCE_MEM_64. The intent of
> > IORESOURCE_MEM_64 is to describe the *capability* of a BAR, not its
> > contents. But I assume you cleared it for a reason. vmd->resources[n]
> > are not BARs, so the PCI core won't assign resources to them like it
> > does for BAR, so we shouldn't care about IORESOURCE_MEM_64 for that
> > reason. Is there some other reason IORESOURCE_MEM_64 makes a
> > difference there?
>
> I did this to fix an issue in pre-RFC code.
Even though you found this issue before posting the RFC code, I assume
the issue is still relevant in the current code, and you still want to
clear IORESOURCE_MEM_64, right?
> The flag is subtly restrictive in one specific scenario: spec-compliant
> PCIe ports lack the ability to specify a 64-bit, non-prefetchable range.
Right; I think this is just a consequence of PCIe ports being PCI
bridges, and bridges having:
- optional 32-bit I/O window
- required 32-bit non-prefetchable memory window
- optional prefetchable memory window (either 32-bit or 64-bit)
If we have a device with a 64-bit non-prefetchable BAR, we can assign
a 64-bit address if the device is on a root bus and the host bridge
has a 64-bit non-prefetchable window. Otherwise, the device is below
a P2P bridge and we have to assign space from the 32-bit
non-prefetchable window.
So far this is all standard PCI stuff, not VMD- or even PCIe-specific.
> IORESOURCE_MEM_64 directs the PCI subsystem to put the address into the
> 64-bit *prefetchable* range.
This is where I get confused. IORESOURCE_MEM_64 *should* mean "the
hardware register associated with this resource can accommodate a
64-bit value." If we're using IORESOURCE_MEM_64 to decide whether to
use a prefetchable vs. a non-prefetchable window, that sounds broken.
Can you point me to the relevant code, and maybe give an example? I'm
pretty sure the code doesn't completely match the spec, and maybe this
is a case where we have to set the flags non-intuitively to get the
desired result.
> Below the port, the "prefetchable" propoerty
> *is* restrictive: the addresses can't be used for non-prefetchable BARs.
>
> Thus, in the specific case where a 64-bit non-prefetchable VMD bar happens
> to contain a 32-bit address, removing the IORESOURCE_MEM_64 flag allows
> the address resource to be used for *any* non-prefetchable BARs (32-bit or
> 64-bit) downstream.
If I understand correctly, these VMD BARs (VMD_MEMBAR1 and
VMD_MEMBAR2) effectively become the host bridge windows available for
devices below the VMD.
I infer that if the VMD host bridge window is non-prefetchable and has
IORESOURCE_MEM_64 set, we won't put a 32-bit non-prefetchable BAR in
that window. That sounds like a bug, but let me be the first to admit
that I don't understand our PCI resource allocation code.
BTW, I forgot to ask about the 0x2000 offset applied to VMD_MEMBAR2.
Can we add a comment about what that offset is doing?
BTW2, is one of these (VMD_MEMBAR1 and VMD_MEMBAR2) prefetchable and
the other non-prefetchable? If so, a comment would really help.
Bjorn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-15 19:48 ` Derrick, Jonathan
2016-01-15 19:54 ` Keith Busch
@ 2016-01-15 22:06 ` Bjorn Helgaas
2016-01-19 15:38 ` Keith Busch
1 sibling, 1 reply; 32+ messages in thread
From: Bjorn Helgaas @ 2016-01-15 22:06 UTC (permalink / raw)
To: Derrick, Jonathan
Cc: Busch, Keith, LKML, x86, linux-pci, Thomas Gleixner,
Bjorn Helgaas, Williams, Dan J, Veal, Bryan E
On Fri, Jan 15, 2016 at 07:48:11PM +0000, Derrick, Jonathan wrote:
> Additionally, what are your thoughts of moving the menuconfig option from the root-tier to 'Processor Type and Features'?
I don't know how Intel is going to market this feature, but from a
developer point of view, the code looks more like the PCI host bridge
drivers in drivers/pci/host than it does like a processor feature.
Most of those drivers are specific to a particular architecture or
SoC.
Bjorn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-15 21:49 ` Bjorn Helgaas
@ 2016-01-16 22:19 ` Veal, Bryan E.
2016-01-20 22:01 ` Bjorn Helgaas
0 siblings, 1 reply; 32+ messages in thread
From: Veal, Bryan E. @ 2016-01-16 22:19 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Keith Busch, LKML, x86, linux-pci, Thomas Gleixner,
Bjorn Helgaas, Dan Williams, Jon Derrick
On Fri, Jan 15, 2016 at 03:49:02PM -0600, Bjorn Helgaas wrote:
> Even though you found this issue before posting the RFC code, I assume
> the issue is still relevant in the current code, and you still want to
> clear IORESOURCE_MEM_64, right?
Yes.
> This is where I get confused. IORESOURCE_MEM_64 *should* mean "the
> hardware register associated with this resource can accommodate a
> 64-bit value." If we're using IORESOURCE_MEM_64 to decide whether to
> use a prefetchable vs. a non-prefetchable window, that sounds broken.
>
> Can you point me to the relevant code, and maybe give an example? I'm
> pretty sure the code doesn't completely match the spec, and maybe this
> is a case where we have to set the flags non-intuitively to get the
> desired result.
>
> > Below the port, the "prefetchable" propoerty
> > *is* restrictive: the addresses can't be used for non-prefetchable BARs.
> >
> > Thus, in the specific case where a 64-bit non-prefetchable VMD bar happens
> > to contain a 32-bit address, removing the IORESOURCE_MEM_64 flag allows
> > the address resource to be used for *any* non-prefetchable BARs (32-bit or
> > 64-bit) downstream.
>
> If I understand correctly, these VMD BARs (VMD_MEMBAR1 and
> VMD_MEMBAR2) effectively become the host bridge windows available for
> devices below the VMD.
>
> I infer that if the VMD host bridge window is non-prefetchable and has
> IORESOURCE_MEM_64 set, we won't put a 32-bit non-prefetchable BAR in
> that window. That sounds like a bug, but let me be the first to admit
> that I don't understand our PCI resource allocation code.
I don't think anything is broken. You are correct that the MEMBARs are
used as a host bridge window. The reason to clear the flag is a side
effect of that.
For BARs, the flags describe capabilities. For resources, they are
interpreted as restrictions.
If VMD has a 32-bit resource in a 64-bit non-prefetchable BAR, without
clearing the flag, it yields a host bridge resource, and thus root bus
resource, with IORESOURCE_MEM_64 set.
Downstream of VMD, the root port's 32-bit non-prefetchable base/limit
registers can't handle the 64-bit resource, but the 64-bit prefetchable
window can, so that's where it ends up. (See pci_bus_alloc_resource().)
Downstream of the root port, the resource is now "upcast" to
IORESOURCE_PREFETCH, which can't be used in a non-prefetchable BAR.
> BTW, I forgot to ask about the 0x2000 offset applied to VMD_MEMBAR2.
> Can we add a comment about what that offset is doing?
I'll rely on Keith to add the comments. This range is reserved for VMD's
MSI-X table and PBA.
> BTW2, is one of these (VMD_MEMBAR1 and VMD_MEMBAR2) prefetchable and
> the other non-prefetchable? If so, a comment would really help.
BIOS can configure the types pre-boot, but having one of each type is
the only real reason to need both BARs.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-12 20:18 [PATCHv8 0/5] Driver for new "VMD" device Keith Busch
` (5 preceding siblings ...)
2016-01-15 18:19 ` [PATCHv8 0/5] Driver for new "VMD" device Bjorn Helgaas
@ 2016-01-17 17:58 ` Christoph Hellwig
6 siblings, 0 replies; 32+ messages in thread
From: Christoph Hellwig @ 2016-01-17 17:58 UTC (permalink / raw)
To: Keith Busch
Cc: LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas,
Dan Williams, Bryan Veal, Jon Derrick
Btw, are there any pointer to what VMD is and why we need an almost,
but not quite PCI-e root driver for it?
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-15 22:06 ` Bjorn Helgaas
@ 2016-01-19 15:38 ` Keith Busch
2016-01-19 16:02 ` Christoph Hellwig
0 siblings, 1 reply; 32+ messages in thread
From: Keith Busch @ 2016-01-19 15:38 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Derrick, Jonathan, LKML, x86, linux-pci, Thomas Gleixner,
Bjorn Helgaas, Williams, Dan J, Veal, Bryan E
On Fri, Jan 15, 2016 at 04:06:31PM -0600, Bjorn Helgaas wrote:
> On Fri, Jan 15, 2016 at 07:48:11PM +0000, Derrick, Jonathan wrote:
> > Additionally, what are your thoughts of moving the menuconfig option from the root-tier to 'Processor Type and Features'?
>
> I don't know how Intel is going to market this feature, but from a
> developer point of view, the code looks more like the PCI host bridge
> drivers in drivers/pci/host than it does like a processor feature.
> Most of those drivers are specific to a particular architecture or
> SoC.
We had tighter dependencies on x86 in earlier revisions of this driver. It
probably now looks more sensible to be in drivers/pci/host instead of
arch specific.
Do you want us to make this change and resend the series? Or can we
provide patches for in tree development? We'll also add the requested
code comments to explain more about the device.
BTW, we completed tests with your pci/host-vmd branch, and that was
successful.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-19 15:38 ` Keith Busch
@ 2016-01-19 16:02 ` Christoph Hellwig
2016-01-19 16:36 ` Keith Busch
2016-01-20 20:43 ` Bjorn Helgaas
0 siblings, 2 replies; 32+ messages in thread
From: Christoph Hellwig @ 2016-01-19 16:02 UTC (permalink / raw)
To: Keith Busch
Cc: Bjorn Helgaas, Derrick, Jonathan, LKML, x86, linux-pci,
Thomas Gleixner, Bjorn Helgaas, Williams, Dan J, Veal, Bryan E
As this seems to require special drivers to bind to it, and Intel
people refuse to even publicly tell what the code does I'd like
to NAK this code until we get an explanation and use cases for it.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-19 16:02 ` Christoph Hellwig
@ 2016-01-19 16:36 ` Keith Busch
2016-01-19 22:05 ` Veal, Bryan E.
2016-01-20 20:43 ` Bjorn Helgaas
1 sibling, 1 reply; 32+ messages in thread
From: Keith Busch @ 2016-01-19 16:36 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Bjorn Helgaas, Derrick, Jonathan, LKML, x86, linux-pci,
Thomas Gleixner, Bjorn Helgaas, Williams, Dan J, Veal, Bryan E
On Tue, Jan 19, 2016 at 08:02:20AM -0800, Christoph Hellwig wrote:
> As this seems to require special drivers to bind to it, and Intel
> people refuse to even publicly tell what the code does I'd like
> to NAK this code until we get an explanation and use cases for it.
We haven't opened the h/w specification, but we've been pretty open with
what it provides, how the code works, and our intended use case. The
device provides additional pci domains for people who need more than
the 256 busses a single domain provides.
What information may I provide to satisfy your use case concerns? Are
you wanting to know what devices we have in mind that require additional
domains?
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-19 16:36 ` Keith Busch
@ 2016-01-19 22:05 ` Veal, Bryan E.
0 siblings, 0 replies; 32+ messages in thread
From: Veal, Bryan E. @ 2016-01-19 22:05 UTC (permalink / raw)
To: Keith Busch
Cc: Christoph Hellwig, Bjorn Helgaas, Derrick, Jonathan, LKML, x86,
linux-pci, Thomas Gleixner, Bjorn Helgaas, Williams, Dan J
On Tue, Jan 19, 2016 at 04:36:36PM +0000, Keith Busch wrote:
> On Tue, Jan 19, 2016 at 08:02:20AM -0800, Christoph Hellwig wrote:
> > As this seems to require special drivers to bind to it, and Intel
> > people refuse to even publicly tell what the code does I'd like
> > to NAK this code until we get an explanation and use cases for it.
>
> We haven't opened the h/w specification, but we've been pretty open with
> what it provides, how the code works, and our intended use case. The
> device provides additional pci domains for people who need more than
> the 256 busses a single domain provides.
>
> What information may I provide to satisfy your use case concerns? Are
> you wanting to know what devices we have in mind that require additional
> domains?
VMD is simply a convenient way to create a new PCIe host bridge that
happens to sit on the existing PCIe root bus. It changes how I/O is
routed (i.e. BDF translation), but not its contents. We've actually gone
through some effort in the code *avoid* special drivers by implementing
the existing host bridge abstractions. The cases where existing drivers
wouldn't work are due to limitations, not arbitrary filters. (For
example, it doesn't know how to route legacy IO ports or INTx.)
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-19 16:02 ` Christoph Hellwig
2016-01-19 16:36 ` Keith Busch
@ 2016-01-20 20:43 ` Bjorn Helgaas
2016-01-26 16:46 ` Christoph Hellwig
1 sibling, 1 reply; 32+ messages in thread
From: Bjorn Helgaas @ 2016-01-20 20:43 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Keith Busch, Derrick, Jonathan, LKML, x86, linux-pci,
Thomas Gleixner, Bjorn Helgaas, Williams, Dan J, Veal, Bryan E
Hi Christoph,
On Tue, Jan 19, 2016 at 08:02:20AM -0800, Christoph Hellwig wrote:
>
> As this seems to require special drivers to bind to it, and Intel
> people refuse to even publicly tell what the code does I'd like
> to NAK this code until we get an explanation and use cases for it.
I saw responses from Keith and Bryan, and I hope they answer your
questions. As far as I can tell, the VMD driver is grossly similar to
other host bridge drivers we've already merged, and I don't think we
have public specs for all of them.
Unless you have further concerns, I'm going to ask Linus to pull this
tomorrow, along with the rest of the PCI changes for v4.5.
Bjorn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-16 22:19 ` Veal, Bryan E.
@ 2016-01-20 22:01 ` Bjorn Helgaas
2016-02-22 22:10 ` Bjorn Helgaas
0 siblings, 1 reply; 32+ messages in thread
From: Bjorn Helgaas @ 2016-01-20 22:01 UTC (permalink / raw)
To: Veal, Bryan E.
Cc: Keith Busch, LKML, x86, linux-pci, Thomas Gleixner,
Bjorn Helgaas, Dan Williams, Jon Derrick
On Sat, Jan 16, 2016 at 02:19:38PM -0800, Veal, Bryan E. wrote:
> On Fri, Jan 15, 2016 at 03:49:02PM -0600, Bjorn Helgaas wrote:
> > Even though you found this issue before posting the RFC code, I assume
> > the issue is still relevant in the current code, and you still want to
> > clear IORESOURCE_MEM_64, right?
>
> Yes.
>
> > This is where I get confused. IORESOURCE_MEM_64 *should* mean "the
> > hardware register associated with this resource can accommodate a
> > 64-bit value." If we're using IORESOURCE_MEM_64 to decide whether to
> > use a prefetchable vs. a non-prefetchable window, that sounds broken.
> >
> > Can you point me to the relevant code, and maybe give an example? I'm
> > pretty sure the code doesn't completely match the spec, and maybe this
> > is a case where we have to set the flags non-intuitively to get the
> > desired result.
> >
> > > Below the port, the "prefetchable" propoerty
> > > *is* restrictive: the addresses can't be used for non-prefetchable BARs.
> > >
> > > Thus, in the specific case where a 64-bit non-prefetchable VMD bar happens
> > > to contain a 32-bit address, removing the IORESOURCE_MEM_64 flag allows
> > > the address resource to be used for *any* non-prefetchable BARs (32-bit or
> > > 64-bit) downstream.
> >
> > If I understand correctly, these VMD BARs (VMD_MEMBAR1 and
> > VMD_MEMBAR2) effectively become the host bridge windows available for
> > devices below the VMD.
> >
> > I infer that if the VMD host bridge window is non-prefetchable and has
> > IORESOURCE_MEM_64 set, we won't put a 32-bit non-prefetchable BAR in
> > that window. That sounds like a bug, but let me be the first to admit
> > that I don't understand our PCI resource allocation code.
>
> I don't think anything is broken. You are correct that the MEMBARs are
> used as a host bridge window. The reason to clear the flag is a side
> effect of that.
>
> For BARs, the flags describe capabilities. For resources, they are
> interpreted as restrictions.
>
> If VMD has a 32-bit resource in a 64-bit non-prefetchable BAR, without
> clearing the flag, it yields a host bridge resource, and thus root bus
> resource, with IORESOURCE_MEM_64 set.
>
> Downstream of VMD, the root port's 32-bit non-prefetchable base/limit
> registers can't handle the 64-bit resource, but the 64-bit prefetchable
> window can, so that's where it ends up. (See pci_bus_alloc_resource().)
OK, I think I finally found the critical comment, which is in
__pci_assign_resource():
Even if a 64-bit prefetchable bridge window is below 4GB, we can't
put a 32-bit prefetchable resource in it because pbus_size_mem()
assumes a 64-bit window will contain no 32-bit resources. If we
assign things differently than they were sized, not everything will
fit.
There's no reason we can't put a Root Port's 32-bit non-prefetchable
window inside a 64-bit VMD window that happens to be below 4GB,
*except* for the fact that pbus_size_mem() assumes we won't do that.
The VMD code needs a reference to that comment.
I guess you're relying on BIOS to assign your non-prefetchable VMD BAR
below 4GB even though it's a 64-bit BAR? If Linux assigned that BAR,
e.g., after a hot-add of a VMD, we might put it above 4GB, and then
Root Ports downstream from the VMD would not be able to use any
non-prefetchable space.
Bjorn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-20 20:43 ` Bjorn Helgaas
@ 2016-01-26 16:46 ` Christoph Hellwig
2016-01-26 18:23 ` Veal, Bryan E.
0 siblings, 1 reply; 32+ messages in thread
From: Christoph Hellwig @ 2016-01-26 16:46 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Christoph Hellwig, Keith Busch, Derrick, Jonathan, LKML, x86,
linux-pci, Thomas Gleixner, Bjorn Helgaas, Williams, Dan J, Veal,
Bryan E
On Wed, Jan 20, 2016 at 02:43:08PM -0600, Bjorn Helgaas wrote:
> I saw responses from Keith and Bryan, and I hope they answer your
> questions. As far as I can tell, the VMD driver is grossly similar to
> other host bridge drivers we've already merged, and I don't think we
> have public specs for all of them.
>
> Unless you have further concerns, I'm going to ask Linus to pull this
> tomorrow, along with the rest of the PCI changes for v4.5.
I still think it's a bad idea to merge something odd like this without
a good explanation or showing what devices can actually sit under it.
But you're the maintainer in the end..
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-26 16:46 ` Christoph Hellwig
@ 2016-01-26 18:23 ` Veal, Bryan E.
0 siblings, 0 replies; 32+ messages in thread
From: Veal, Bryan E. @ 2016-01-26 18:23 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Bjorn Helgaas, Keith Busch, Derrick, Jonathan, LKML, x86,
linux-pci, Thomas Gleixner, Bjorn Helgaas, Williams, Dan J
On Tue, Jan 26, 2016 at 08:46:09AM -0800, Christoph Hellwig wrote:
> On Wed, Jan 20, 2016 at 02:43:08PM -0600, Bjorn Helgaas wrote:
> > I saw responses from Keith and Bryan, and I hope they answer your
> > questions. As far as I can tell, the VMD driver is grossly similar to
> > other host bridge drivers we've already merged, and I don't think we
> > have public specs for all of them.
> >
> > Unless you have further concerns, I'm going to ask Linus to pull this
> > tomorrow, along with the rest of the PCI changes for v4.5.
>
> I still think it's a bad idea to merge something odd like this without
> a good explanation or showing what devices can actually sit under it.
>
> But you're the maintainer in the end..
Any PCIe devices and and bridges should work with existing upstream
drivers. The only exceptions would be anything depndent on INTx or IO
ports.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-01-20 22:01 ` Bjorn Helgaas
@ 2016-02-22 22:10 ` Bjorn Helgaas
2016-02-23 18:24 ` Keith Busch
0 siblings, 1 reply; 32+ messages in thread
From: Bjorn Helgaas @ 2016-02-22 22:10 UTC (permalink / raw)
To: Veal, Bryan E.
Cc: Keith Busch, LKML, x86, linux-pci, Thomas Gleixner,
Bjorn Helgaas, Dan Williams, Jon Derrick
On Wed, Jan 20, 2016 at 04:01:11PM -0600, Bjorn Helgaas wrote:
> On Sat, Jan 16, 2016 at 02:19:38PM -0800, Veal, Bryan E. wrote:
> > On Fri, Jan 15, 2016 at 03:49:02PM -0600, Bjorn Helgaas wrote:
> > > Even though you found this issue before posting the RFC code, I assume
> > > the issue is still relevant in the current code, and you still want to
> > > clear IORESOURCE_MEM_64, right?
> >
> > Yes.
> >
> > > This is where I get confused. IORESOURCE_MEM_64 *should* mean "the
> > > hardware register associated with this resource can accommodate a
> > > 64-bit value." If we're using IORESOURCE_MEM_64 to decide whether to
> > > use a prefetchable vs. a non-prefetchable window, that sounds broken.
> > >
> > > Can you point me to the relevant code, and maybe give an example? I'm
> > > pretty sure the code doesn't completely match the spec, and maybe this
> > > is a case where we have to set the flags non-intuitively to get the
> > > desired result.
> > >
> > > > Below the port, the "prefetchable" propoerty
> > > > *is* restrictive: the addresses can't be used for non-prefetchable BARs.
> > > >
> > > > Thus, in the specific case where a 64-bit non-prefetchable VMD bar happens
> > > > to contain a 32-bit address, removing the IORESOURCE_MEM_64 flag allows
> > > > the address resource to be used for *any* non-prefetchable BARs (32-bit or
> > > > 64-bit) downstream.
> > >
> > > If I understand correctly, these VMD BARs (VMD_MEMBAR1 and
> > > VMD_MEMBAR2) effectively become the host bridge windows available for
> > > devices below the VMD.
> > >
> > > I infer that if the VMD host bridge window is non-prefetchable and has
> > > IORESOURCE_MEM_64 set, we won't put a 32-bit non-prefetchable BAR in
> > > that window. That sounds like a bug, but let me be the first to admit
> > > that I don't understand our PCI resource allocation code.
> >
> > I don't think anything is broken. You are correct that the MEMBARs are
> > used as a host bridge window. The reason to clear the flag is a side
> > effect of that.
> >
> > For BARs, the flags describe capabilities. For resources, they are
> > interpreted as restrictions.
> >
> > If VMD has a 32-bit resource in a 64-bit non-prefetchable BAR, without
> > clearing the flag, it yields a host bridge resource, and thus root bus
> > resource, with IORESOURCE_MEM_64 set.
> >
> > Downstream of VMD, the root port's 32-bit non-prefetchable base/limit
> > registers can't handle the 64-bit resource, but the 64-bit prefetchable
> > window can, so that's where it ends up. (See pci_bus_alloc_resource().)
>
> OK, I think I finally found the critical comment, which is in
> __pci_assign_resource():
>
> Even if a 64-bit prefetchable bridge window is below 4GB, we can't
> put a 32-bit prefetchable resource in it because pbus_size_mem()
> assumes a 64-bit window will contain no 32-bit resources. If we
> assign things differently than they were sized, not everything will
> fit.
>
> There's no reason we can't put a Root Port's 32-bit non-prefetchable
> window inside a 64-bit VMD window that happens to be below 4GB,
> *except* for the fact that pbus_size_mem() assumes we won't do that.
>
> The VMD code needs a reference to that comment.
>
> I guess you're relying on BIOS to assign your non-prefetchable VMD BAR
> below 4GB even though it's a 64-bit BAR? If Linux assigned that BAR,
> e.g., after a hot-add of a VMD, we might put it above 4GB, and then
> Root Ports downstream from the VMD would not be able to use any
> non-prefetchable space.
I see another VMD patch on the list, but I'm still waiting for
resolution to this comment and question. For the first one, about
clearing IORESOURCE_MEM_64, I have in mind something like the
following patch.
I'm not sure how to deal with the question of a hot-added VMD. Maybe
all we can do now is add a comment to the effect that we assume BIOS
has assigned the non-prefetchable BAR below 4GB, and if Linux assigns
that BAR for hot-added VMDs, that assumption will likely break.
Bjorn
diff --git a/arch/x86/pci/vmd.c b/arch/x86/pci/vmd.c
index d57e480..7554722 100644
--- a/arch/x86/pci/vmd.c
+++ b/arch/x86/pci/vmd.c
@@ -532,6 +532,16 @@ static int vmd_enable_domain(struct vmd_dev *vmd)
.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
};
+ /*
+ * If the window is below 4GB, clear IORESOURCE_MEM_64 so we can
+ * put 32-bit resources in the window.
+ *
+ * There's no hardware reason why a 64-bit window *couldn't*
+ * contain a 32-bit resource, but pbus_size_mem() computes the
+ * bridge window size assuming a 64-bit window will contain no
+ * 32-bit resources. __pci_assign_resource() enforces that
+ * artificial restriction to make sure everything will fit.
+ */
res = &vmd->dev->resource[VMD_MEMBAR1];
upper_bits = upper_32_bits(res->end);
flags = res->flags & ~IORESOURCE_SIZEALIGN;
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-02-22 22:10 ` Bjorn Helgaas
@ 2016-02-23 18:24 ` Keith Busch
2016-02-25 14:42 ` Bjorn Helgaas
0 siblings, 1 reply; 32+ messages in thread
From: Keith Busch @ 2016-02-23 18:24 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Veal, Bryan E.,
LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas,
Dan Williams, Jon Derrick
On Mon, Feb 22, 2016 at 04:10:24PM -0600, Bjorn Helgaas wrote:
> I'm not sure how to deal with the question of a hot-added VMD. Maybe
> all we can do now is add a comment to the effect that we assume BIOS
> has assigned the non-prefetchable BAR below 4GB, and if Linux assigns
> that BAR for hot-added VMDs, that assumption will likely break.
Yes, we can assume BIOS always assigns. There are other BIOS dependencies
in order for the host to see the h/w as a VMD endpoint.
> diff --git a/arch/x86/pci/vmd.c b/arch/x86/pci/vmd.c
> index d57e480..7554722 100644
> --- a/arch/x86/pci/vmd.c
> +++ b/arch/x86/pci/vmd.c
> @@ -532,6 +532,16 @@ static int vmd_enable_domain(struct vmd_dev *vmd)
> .flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
> };
>
> + /*
> + * If the window is below 4GB, clear IORESOURCE_MEM_64 so we can
> + * put 32-bit resources in the window.
> + *
> + * There's no hardware reason why a 64-bit window *couldn't*
> + * contain a 32-bit resource, but pbus_size_mem() computes the
> + * bridge window size assuming a 64-bit window will contain no
> + * 32-bit resources. __pci_assign_resource() enforces that
> + * artificial restriction to make sure everything will fit.
> + */
This sounds good to me. Thanks!
> res = &vmd->dev->resource[VMD_MEMBAR1];
> upper_bits = upper_32_bits(res->end);
> flags = res->flags & ~IORESOURCE_SIZEALIGN;
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-02-23 18:24 ` Keith Busch
@ 2016-02-25 14:42 ` Bjorn Helgaas
2016-02-25 14:50 ` Keith Busch
2016-02-26 15:29 ` Keith Busch
0 siblings, 2 replies; 32+ messages in thread
From: Bjorn Helgaas @ 2016-02-25 14:42 UTC (permalink / raw)
To: Keith Busch
Cc: Veal, Bryan E.,
LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas,
Dan Williams, Jon Derrick
On Tue, Feb 23, 2016 at 06:24:00PM +0000, Keith Busch wrote:
> On Mon, Feb 22, 2016 at 04:10:24PM -0600, Bjorn Helgaas wrote:
> > I'm not sure how to deal with the question of a hot-added VMD. Maybe
> > all we can do now is add a comment to the effect that we assume BIOS
> > has assigned the non-prefetchable BAR below 4GB, and if Linux assigns
> > that BAR for hot-added VMDs, that assumption will likely break.
>
> Yes, we can assume BIOS always assigns. There are other BIOS dependencies
> in order for the host to see the h/w as a VMD endpoint.
>
> > diff --git a/arch/x86/pci/vmd.c b/arch/x86/pci/vmd.c
> > index d57e480..7554722 100644
> > --- a/arch/x86/pci/vmd.c
> > +++ b/arch/x86/pci/vmd.c
> > @@ -532,6 +532,16 @@ static int vmd_enable_domain(struct vmd_dev *vmd)
> > .flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
> > };
> >
> > + /*
> > + * If the window is below 4GB, clear IORESOURCE_MEM_64 so we can
> > + * put 32-bit resources in the window.
> > + *
> > + * There's no hardware reason why a 64-bit window *couldn't*
> > + * contain a 32-bit resource, but pbus_size_mem() computes the
> > + * bridge window size assuming a 64-bit window will contain no
> > + * 32-bit resources. __pci_assign_resource() enforces that
> > + * artificial restriction to make sure everything will fit.
> > + */
>
> This sounds good to me. Thanks!
>
> > res = &vmd->dev->resource[VMD_MEMBAR1];
> > upper_bits = upper_32_bits(res->end);
> > flags = res->flags & ~IORESOURCE_SIZEALIGN;
Can you prepare a patch, that adds both comments, please? (The one
about how we assume BIOS assigns the BAR below 4GB, and the one I
drafted above.)
Bjorn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-02-25 14:42 ` Bjorn Helgaas
@ 2016-02-25 14:50 ` Keith Busch
2016-02-26 15:29 ` Keith Busch
1 sibling, 0 replies; 32+ messages in thread
From: Keith Busch @ 2016-02-25 14:50 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Veal, Bryan E.,
LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas,
Dan Williams, Jon Derrick
On Thu, Feb 25, 2016 at 08:42:19AM -0600, Bjorn Helgaas wrote:
> On Tue, Feb 23, 2016 at 06:24:00PM +0000, Keith Busch wrote:
> > On Mon, Feb 22, 2016 at 04:10:24PM -0600, Bjorn Helgaas wrote:
> > > + /*
> > > + * If the window is below 4GB, clear IORESOURCE_MEM_64 so we can
> > > + * put 32-bit resources in the window.
> > > + *
> > > + * There's no hardware reason why a 64-bit window *couldn't*
> > > + * contain a 32-bit resource, but pbus_size_mem() computes the
> > > + * bridge window size assuming a 64-bit window will contain no
> > > + * 32-bit resources. __pci_assign_resource() enforces that
> > > + * artificial restriction to make sure everything will fit.
> > > + */
> >
> > This sounds good to me. Thanks!
> >
> > > res = &vmd->dev->resource[VMD_MEMBAR1];
> > > upper_bits = upper_32_bits(res->end);
> > > flags = res->flags & ~IORESOURCE_SIZEALIGN;
>
> Can you prepare a patch, that adds both comments, please? (The one
> about how we assume BIOS assigns the BAR below 4GB, and the one I
> drafted above.)
Yep, will send it out be end of today.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCHv8 0/5] Driver for new "VMD" device
2016-02-25 14:42 ` Bjorn Helgaas
2016-02-25 14:50 ` Keith Busch
@ 2016-02-26 15:29 ` Keith Busch
1 sibling, 0 replies; 32+ messages in thread
From: Keith Busch @ 2016-02-26 15:29 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Veal, Bryan E.,
LKML, x86, linux-pci, Thomas Gleixner, Bjorn Helgaas,
Dan Williams, Jon Derrick
Hi Bryan
I want to make sure I get the right message for Bjorn, so held off on
sending anything before confirming.
As far as clearing the 64-bit resource flag when it's a 32-bit address,
that seems pretty straight forward as captured in the comment.
But do we actually rely on a memory window below 4GB? If the memory
window is above 4GB, we just won't be able to support devices with 32-bit
BARs there, but is that a concern? Should the comment just say that if
the window is assigned >4GB, we simply won't be able to support 32-bit
BAR devices on that domain, and that's 'ok'?
Thanks,
Keith
On Thu, Feb 25, 2016 at 08:42:19AM -0600, Bjorn Helgaas wrote:
> On Tue, Feb 23, 2016 at 06:24:00PM +0000, Keith Busch wrote:
> > On Mon, Feb 22, 2016 at 04:10:24PM -0600, Bjorn Helgaas wrote:
> > > I'm not sure how to deal with the question of a hot-added VMD. Maybe
> > > all we can do now is add a comment to the effect that we assume BIOS
> > > has assigned the non-prefetchable BAR below 4GB, and if Linux assigns
> > > that BAR for hot-added VMDs, that assumption will likely break.
> >
> > Yes, we can assume BIOS always assigns. There are other BIOS dependencies
> > in order for the host to see the h/w as a VMD endpoint.
> >
> > > diff --git a/arch/x86/pci/vmd.c b/arch/x86/pci/vmd.c
> > > index d57e480..7554722 100644
> > > --- a/arch/x86/pci/vmd.c
> > > +++ b/arch/x86/pci/vmd.c
> > > @@ -532,6 +532,16 @@ static int vmd_enable_domain(struct vmd_dev *vmd)
> > > .flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
> > > };
> > >
> > > + /*
> > > + * If the window is below 4GB, clear IORESOURCE_MEM_64 so we can
> > > + * put 32-bit resources in the window.
> > > + *
> > > + * There's no hardware reason why a 64-bit window *couldn't*
> > > + * contain a 32-bit resource, but pbus_size_mem() computes the
> > > + * bridge window size assuming a 64-bit window will contain no
> > > + * 32-bit resources. __pci_assign_resource() enforces that
> > > + * artificial restriction to make sure everything will fit.
> > > + */
> >
> > This sounds good to me. Thanks!
> >
> > > res = &vmd->dev->resource[VMD_MEMBAR1];
> > > upper_bits = upper_32_bits(res->end);
> > > flags = res->flags & ~IORESOURCE_SIZEALIGN;
>
> Can you prepare a patch, that adds both comments, please? (The one
> about how we assume BIOS assigns the BAR below 4GB, and the one I
> drafted above.)
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2016-02-26 15:29 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-12 20:18 [PATCHv8 0/5] Driver for new "VMD" device Keith Busch
2016-01-12 20:18 ` [PATCHv8 1/5] msi: Relax msi_domain_alloc() to support parentless MSI irqdomains Keith Busch
2016-01-12 20:18 ` [PATCHv8 2/5] x86/IRQ: Export IRQ domain function for module use Keith Busch
2016-01-12 20:18 ` [PATCHv8 3/5] x86/PCI: Allow PCI domain specific dma ops Keith Busch
2016-01-12 20:18 ` [PATCHv8 4/5] PCI/AER: Use 32 bit int type domains Keith Busch
2016-01-12 20:18 ` [PATCHv8 5/5] x86/PCI: Initial commit for new VMD device driver Keith Busch
2016-01-15 18:19 ` [PATCHv8 0/5] Driver for new "VMD" device Bjorn Helgaas
2016-01-15 19:31 ` Veal, Bryan E.
2016-01-15 21:49 ` Bjorn Helgaas
2016-01-16 22:19 ` Veal, Bryan E.
2016-01-20 22:01 ` Bjorn Helgaas
2016-02-22 22:10 ` Bjorn Helgaas
2016-02-23 18:24 ` Keith Busch
2016-02-25 14:42 ` Bjorn Helgaas
2016-02-25 14:50 ` Keith Busch
2016-02-26 15:29 ` Keith Busch
2016-01-15 19:32 ` Keith Busch
2016-01-15 19:44 ` Thomas Gleixner
2016-01-15 20:03 ` Bjorn Helgaas
2016-01-15 20:14 ` Thomas Gleixner
2016-01-15 19:48 ` Derrick, Jonathan
2016-01-15 19:54 ` Keith Busch
2016-01-15 20:02 ` Jon Derrick
2016-01-15 22:06 ` Bjorn Helgaas
2016-01-19 15:38 ` Keith Busch
2016-01-19 16:02 ` Christoph Hellwig
2016-01-19 16:36 ` Keith Busch
2016-01-19 22:05 ` Veal, Bryan E.
2016-01-20 20:43 ` Bjorn Helgaas
2016-01-26 16:46 ` Christoph Hellwig
2016-01-26 18:23 ` Veal, Bryan E.
2016-01-17 17:58 ` Christoph Hellwig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.