All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv4 0/6] Driver for new VMD device
@ 2015-11-06 22:35 Keith Busch
  2015-11-06 22:35 ` [PATCHv4 1/6] msi: Relax msi_domain_alloc() to support parentless MSI irqdomains Keith Busch
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Keith Busch @ 2015-11-06 22:35 UTC (permalink / raw)
  To: LKML, x86, linux-pci
  Cc: Jiang Liu, Thomas Gleixner, Dan Williams, Bjorn Helgaas,
	Bryan Veal, Ingo Molnar, H. Peter Anvin, Martin Mares,
	Jon Derrick, Keith Busch

Version 4 after re-working the irq domain hierarchies.

v3 -> v4:

  Patch from Jiang to allow parent-less msi irq domain.

  Separated kernel API dependency exports into their own patch.

  Removed the vmd irq domain hierarchy, and instead use a msi irq
  domain. This pushed all the irq initialization and teardown handling
  into the irq_chip implementation and reduced kernel dependencies.

  Using devm_request_irq instead of an irq flow handler. This moves the
  irq life management to the devres API which simplifies teardown.

  Implemented a smarter method of selecting the VMD vector to share.

  Lots style fixes/updates and additional code comments.

The one review comment I have not fixed is the affinity hint. We are
still developing a way to better handle this, so have left it as a error
returning stub. It's less than optimal, but isn't more harmful than that.

Keith Busch (5):
  pci: skip child bus with conflicting resources
  Export msi and irq functions for module use
  x86-pci: allow pci domain specific dma ops
  x86/pci: Initial commit for new VMD device driver
  pciutils: Allow 32-bit domains

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

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

-- 
2.6.2.307.g37023ba


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCHv4 1/6] msi: Relax msi_domain_alloc() to support parentless MSI irqdomains
  2015-11-06 22:35 [PATCHv4 0/6] Driver for new VMD device Keith Busch
@ 2015-11-06 22:35 ` Keith Busch
  2015-11-07 11:43   ` Thomas Gleixner
  2015-11-06 22:35 ` [PATCHv4 2/6] pci: skip child bus with conflicting resources Keith Busch
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Keith Busch @ 2015-11-06 22:35 UTC (permalink / raw)
  To: LKML, x86, linux-pci
  Cc: Jiang Liu, Thomas Gleixner, Dan Williams, Bjorn Helgaas,
	Bryan Veal, Ingo Molnar, H. Peter Anvin, Martin Mares,
	Jon Derrick

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: Liu Jiang <jiang.liu@linux.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 be9149f..01644d4 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] 9+ messages in thread

* [PATCHv4 2/6] pci: skip child bus with conflicting resources
  2015-11-06 22:35 [PATCHv4 0/6] Driver for new VMD device Keith Busch
  2015-11-06 22:35 ` [PATCHv4 1/6] msi: Relax msi_domain_alloc() to support parentless MSI irqdomains Keith Busch
@ 2015-11-06 22:35 ` Keith Busch
  2015-11-06 22:35 ` [PATCHv4 3/6] Export msi and irq functions for module use Keith Busch
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Keith Busch @ 2015-11-06 22:35 UTC (permalink / raw)
  To: LKML, x86, linux-pci
  Cc: Jiang Liu, Thomas Gleixner, Dan Williams, Bjorn Helgaas,
	Bryan Veal, Ingo Molnar, H. Peter Anvin, Martin Mares,
	Jon Derrick, Keith Busch

And use the max bus resource from the parent rather than assume 255.

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 drivers/pci/probe.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8361d27..1cb3be7 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -856,7 +856,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
 			if (!child)
 				goto out;
 			child->primary = primary;
-			pci_bus_insert_busn_res(child, secondary, subordinate);
+			if (!pci_bus_insert_busn_res(child, secondary, subordinate)) {
+				pci_remove_bus(child);
+				goto out;
+			}
 			child->bridge_ctl = bctl;
 		}
 
@@ -896,7 +899,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
 			child = pci_add_new_bus(bus, dev, max+1);
 			if (!child)
 				goto out;
-			pci_bus_insert_busn_res(child, max+1, 0xff);
+			if (!pci_bus_insert_busn_res(child, max+1, bus->busn_res.end)) {
+				pci_remove_bus(child);
+				goto out;
+			}
 		}
 		max++;
 		buses = (buses & 0xff000000)
-- 
2.6.2.307.g37023ba


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCHv4 3/6] Export msi and irq functions for module use
  2015-11-06 22:35 [PATCHv4 0/6] Driver for new VMD device Keith Busch
  2015-11-06 22:35 ` [PATCHv4 1/6] msi: Relax msi_domain_alloc() to support parentless MSI irqdomains Keith Busch
  2015-11-06 22:35 ` [PATCHv4 2/6] pci: skip child bus with conflicting resources Keith Busch
@ 2015-11-06 22:35 ` Keith Busch
  2015-11-06 22:35 ` [PATCHv4 4/6] x86-pci: allow pci domain specific dma ops Keith Busch
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Keith Busch @ 2015-11-06 22:35 UTC (permalink / raw)
  To: LKML, x86, linux-pci
  Cc: Jiang Liu, Thomas Gleixner, Dan Williams, Bjorn Helgaas,
	Bryan Veal, Ingo Molnar, H. Peter Anvin, Martin Mares,
	Jon Derrick, Keith Busch

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 drivers/pci/msi.c      | 2 ++
 kernel/irq/irqdomain.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 4a7da3c..5fb932b 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1119,6 +1119,7 @@ struct pci_dev *msi_desc_to_pci_dev(struct msi_desc *desc)
 {
 	return to_pci_dev(desc->dev);
 }
+EXPORT_SYMBOL_GPL(msi_desc_to_pci_dev);
 
 void *msi_desc_to_pci_sysdata(struct msi_desc *desc)
 {
@@ -1278,6 +1279,7 @@ struct irq_domain *pci_msi_create_irq_domain(struct device_node *node,
 	domain->bus_token = DOMAIN_BUS_PCI_MSI;
 	return domain;
 }
+EXPORT_SYMBOL_GPL(pci_msi_create_irq_domain);
 
 /**
  * pci_msi_domain_alloc_irqs - Allocate interrupts for @dev in @domain
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index dc9d27c..43f503f 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -955,6 +955,7 @@ void irq_domain_set_info(struct irq_domain *domain, unsigned int virq,
 	__irq_set_handler(virq, handler, 0, handler_name);
 	irq_set_handler_data(virq, handler_data);
 }
+EXPORT_SYMBOL(irq_domain_set_info);
 
 /**
  * irq_domain_reset_irq_data - Clear hwirq, chip and chip_data in @irq_data
-- 
2.6.2.307.g37023ba


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCHv4 4/6] x86-pci: allow pci domain specific dma ops
  2015-11-06 22:35 [PATCHv4 0/6] Driver for new VMD device Keith Busch
                   ` (2 preceding siblings ...)
  2015-11-06 22:35 ` [PATCHv4 3/6] Export msi and irq functions for module use Keith Busch
@ 2015-11-06 22:35 ` Keith Busch
  2015-11-06 22:35 ` [PATCHv4 5/6] x86/pci: Initial commit for new VMD device driver Keith Busch
  2015-11-06 22:35 ` [PATCHv4 6/6] pciutils: Allow 32-bit domains Keith Busch
  5 siblings, 0 replies; 9+ messages in thread
From: Keith Busch @ 2015-11-06 22:35 UTC (permalink / raw)
  To: LKML, x86, linux-pci
  Cc: Jiang Liu, Thomas Gleixner, Dan Williams, Bjorn Helgaas,
	Bryan Veal, Ingo Molnar, H. Peter Anvin, Martin Mares,
	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 dc78a4a..e227ba2 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] 9+ messages in thread

* [PATCHv4 5/6] x86/pci: Initial commit for new VMD device driver
  2015-11-06 22:35 [PATCHv4 0/6] Driver for new VMD device Keith Busch
                   ` (3 preceding siblings ...)
  2015-11-06 22:35 ` [PATCHv4 4/6] x86-pci: allow pci domain specific dma ops Keith Busch
@ 2015-11-06 22:35 ` Keith Busch
  2015-11-07 11:55   ` Thomas Gleixner
  2015-11-06 22:35 ` [PATCHv4 6/6] pciutils: Allow 32-bit domains Keith Busch
  5 siblings, 1 reply; 9+ messages in thread
From: Keith Busch @ 2015-11-06 22:35 UTC (permalink / raw)
  To: LKML, x86, linux-pci
  Cc: Jiang Liu, Thomas Gleixner, Dan Williams, Bjorn Helgaas,
	Bryan Veal, Ingo Molnar, H. Peter Anvin, Martin Mares,
	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>
---
 arch/x86/Kconfig              |  13 +
 arch/x86/include/asm/hw_irq.h |   5 +
 arch/x86/pci/Makefile         |   2 +
 arch/x86/pci/vmd.c            | 646 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 666 insertions(+)
 create mode 100644 arch/x86/pci/vmd.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 96d058a..48a3656 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2632,6 +2632,19 @@ config PMC_ATOM
 	def_bool y
         depends on PCI
 
+config VMD
+	depends on PCI_MSI
+	tristate "Volume Management Device Driver"
+	default N
+	---help---
+	  Adds support for the Intel Volume Manage Device (VMD). VMD is a
+	  secondary PCI host bridge that allows PCI Express root ports,
+	  and devices attached to them, to be removed from the default
+	  PCI domain and placed within the VMD domain. This provides
+	  additional bus resources than are otherwise possible with a
+	  single domain. If you know your system provides one of these and
+	  have devices attached to it, say Y; if you are not sure, say N.
+
 source "net/Kconfig"
 
 source "drivers/Kconfig"
diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h
index 1e3408e..79c35c4 100644
--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -108,6 +108,11 @@ struct irq_alloc_info {
 			struct IO_APIC_route_entry *ioapic_entry;
 		};
 #endif
+#if IS_ENABLED(CONFIG_VMD)
+		struct {
+			struct msi_desc	*desc;
+		};
+#endif
 #ifdef	CONFIG_DMAR_TABLE
 		struct {
 			int		dmar_id;
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..87096bb
--- /dev/null
+++ b/arch/x86/pci/vmd.c
@@ -0,0 +1,646 @@
+/*
+ * 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>
+
+static DEFINE_SPINLOCK(list_lock);
+
+struct vmd_irq {
+	struct list_head	node;
+	struct rcu_head		rcu;	/* rcu callback list */
+	struct vmd_irq_list	*irq;	/* back pointer */
+	unsigned int		virq;
+};
+
+struct vmd_irq_list {
+	struct list_head	irq_list;
+	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;
+}
+
+static void vmd_irq_enable(struct irq_data *data)
+{
+	struct vmd_irq *vmdirq = data->chip_data;
+
+	spin_lock(&list_lock);
+	list_add_tail_rcu(&vmdirq->node, &vmdirq->irq->irq_list);
+	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);
+
+	spin_lock(&list_lock);
+	list_del_rcu(&vmdirq->node);
+	spin_unlock(&list_lock);
+}
+
+/**
+ * XXX: Stubbed until a good way to not create conflicts with other devices
+ * sharing the same vector is developed.
+ */
+static int vmd_irq_set_affinity(struct irq_data *data, const struct cpumask *dest,
+								bool force)
+{
+	return -EINVAL;
+}
+
+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;
+}
+
+static struct vmd_irq_list *vmd_next_irq(struct vmd_dev *vmd)
+{
+	int i, best = 0;
+
+	spin_lock(&list_lock);
+	for (i = 1; i < vmd->msix_count; i++)
+		if (vmd->irqs[i].count < vmd->irqs[best].count)
+			best = i;
+	vmd->irqs[best].count++;
+	spin_unlock(&list_lock);
+
+	return &vmd->irqs[best];
+}
+
+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);
+
+	spin_lock(&list_lock);
+	vmdirq->irq->count--;
+	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 *dev_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 void *vmd_alloc(struct device *dev, size_t size, dma_addr_t *addr,
+				gfp_t flag, struct dma_attrs *attrs)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	return vdev->archdata.dma_ops->alloc(vdev, size, addr, flag, attrs);
+}
+
+static void vmd_free(struct device *dev, size_t size, void *vaddr,
+				dma_addr_t addr, struct dma_attrs *attrs)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	vdev->archdata.dma_ops->free(vdev, 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)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	return vdev->archdata.dma_ops->mmap(vdev, 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)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	return vdev->archdata.dma_ops->get_sgtable(vdev, 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)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	return vdev->archdata.dma_ops->map_page(vdev, 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)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	vdev->archdata.dma_ops->unmap_page(vdev, 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)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	return vdev->archdata.dma_ops->map_sg(vdev, 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)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	vdev->archdata.dma_ops->unmap_sg(vdev, 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)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	vdev->archdata.dma_ops->sync_single_for_cpu(vdev, 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)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	vdev->archdata.dma_ops->sync_single_for_device(vdev, addr, size, dir);
+}
+
+static void vmd_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
+				int nents, enum dma_data_direction dir)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	vdev->archdata.dma_ops->sync_sg_for_cpu(vdev, sg, nents, dir);
+}
+
+static void vmd_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
+				int nents, enum dma_data_direction dir)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	vdev->archdata.dma_ops->sync_sg_for_device(vdev, sg, nents, dir);
+}
+
+static int vmd_mapping_error(struct device *dev, dma_addr_t addr)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	return vdev->archdata.dma_ops->mapping_error(vdev, addr);
+}
+
+static int vmd_dma_supported(struct device *dev, u64 mask)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	return vdev->archdata.dma_ops->dma_supported(vdev, mask);
+}
+
+#ifdef ARCH_HAS_DMA_GET_REQUIRED_MASK
+u64 vmd_get_required_mask(struct device *dev)
+{
+	struct device *vdev = dev_to_vmd_dev(dev);
+	return vdev->archdata.dma_ops->get_required_mask(vdev);
+}
+#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) \
+	if (source->fn) dest->fn = vmd_##fn;
+static void vmd_setup_dma_ops(struct vmd_dev *vmd)
+{
+	struct dma_domain *domain = &vmd->dma_domain;
+	struct dma_map_ops *source = vmd->dev->dev.archdata.dma_ops;
+	struct dma_map_ops *dest = &vmd->dma_ops;
+
+	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 posted config writes to non-posted. 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,
+};
+
+static int vmd_find_free_domain(void)
+{
+	/*
+	 * VMD Domains start at 10000h to not clash with domains defining
+	 * ACPI _SEG.
+	 */
+	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;
+	LIST_HEAD(resources);
+
+	vmd->resources[0] = (struct resource) {
+		.name	= "VMD CFGBAR",
+		.start	= 0,
+		.end	= (resource_size(&vmd->dev->resource[0]) >> 20) - 1,
+		.flags	= IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
+	};
+	vmd->resources[1] = (struct resource) {
+		.name	= "VMD MEMBAR1",
+		.start	= vmd->dev->resource[2].start,
+		.end	= vmd->dev->resource[2].end,
+		.flags	= (vmd->dev->resource[2].flags & ~IORESOURCE_SIZEALIGN) &
+				(!upper_32_bits(vmd->dev->resource[2].end) ?
+							~IORESOURCE_MEM_64 : ~0),
+	};
+	vmd->resources[2] = (struct resource) {
+		.name	= "VMD MEMBAR2",
+		.start	= vmd->dev->resource[4].start + 0x2000,
+		.end	= vmd->dev->resource[4].end,
+		.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(NULL, 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, IRQF_SHARED, "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;
+
+	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);
+}
+
+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,
+};
+module_pci_driver(vmd_drv);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION("0.3");
-- 
2.6.2.307.g37023ba


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCHv4 6/6] pciutils: Allow 32-bit domains
  2015-11-06 22:35 [PATCHv4 0/6] Driver for new VMD device Keith Busch
                   ` (4 preceding siblings ...)
  2015-11-06 22:35 ` [PATCHv4 5/6] x86/pci: Initial commit for new VMD device driver Keith Busch
@ 2015-11-06 22:35 ` Keith Busch
  5 siblings, 0 replies; 9+ messages in thread
From: Keith Busch @ 2015-11-06 22:35 UTC (permalink / raw)
  To: LKML, x86, linux-pci
  Cc: Jiang Liu, Thomas Gleixner, Dan Williams, Bjorn Helgaas,
	Bryan Veal, Ingo Molnar, H. Peter Anvin, Martin Mares,
	Jon Derrick, Keith Busch

PCI-e segments will continue to use the lower 16 bits as required by
ACPI. Special domains may use the full 32-bits.

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 lib/filter.c |    2 +-
 lib/pci.h    |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/filter.c b/lib/filter.c
index d4254a0..075dc2f 100644
--- a/lib/filter.c
+++ b/lib/filter.c
@@ -45,7 +45,7 @@ pci_filter_parse_slot_v33(struct pci_filter *f, char *str)
 	  if (str[0] && strcmp(str, "*"))
 	    {
 	      long int x = strtol(str, &e, 16);
-	      if ((e && *e) || (x < 0 || x > 0xffff))
+	      if ((e && *e) || (x < 0))
 		return "Invalid domain number";
 	      f->domain = x;
 	    }
diff --git a/lib/pci.h b/lib/pci.h
index 10ba831..7e42765 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -119,7 +119,7 @@ struct pci_param *pci_walk_params(struct pci_access *acc, struct pci_param *prev
 
 struct pci_dev {
   struct pci_dev *next;			/* Next device in the chain */
-  u16 domain;				/* PCI domain (host bridge) */
+  int32_t domain;			/* PCI domain (host bridge) */
   u8 bus, dev, func;			/* Bus inside domain, device and function */
 
   /* These fields are set by pci_fill_info() */
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCHv4 1/6] msi: Relax msi_domain_alloc() to support parentless MSI irqdomains
  2015-11-06 22:35 ` [PATCHv4 1/6] msi: Relax msi_domain_alloc() to support parentless MSI irqdomains Keith Busch
@ 2015-11-07 11:43   ` Thomas Gleixner
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Gleixner @ 2015-11-07 11:43 UTC (permalink / raw)
  To: Keith Busch
  Cc: LKML, x86, linux-pci, Jiang Liu, Dan Williams, Bjorn Helgaas,
	Bryan Veal, Ingo Molnar, H. Peter Anvin, Martin Mares,
	Jon Derrick

On Fri, 6 Nov 2015, Keith Busch wrote:
> From: Liu Jiang <jiang.liu@linux.intel.com>
... 
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> Signed-off-by: Liu Jiang <jiang.liu@linux.intel.com>

Hehehe. Let me read that from/sob chain:

Liu wrote the patch and then handed it over to Jiang who sent it back
to Liu and Liu sent it to Keith.

Now what's missing is the SOB from Keith to make that chain perfect

Other than that, this is the right solution.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCHv4 5/6] x86/pci: Initial commit for new VMD device driver
  2015-11-06 22:35 ` [PATCHv4 5/6] x86/pci: Initial commit for new VMD device driver Keith Busch
@ 2015-11-07 11:55   ` Thomas Gleixner
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Gleixner @ 2015-11-07 11:55 UTC (permalink / raw)
  To: Keith Busch
  Cc: LKML, x86, linux-pci, Jiang Liu, Dan Williams, Bjorn Helgaas,
	Bryan Veal, Ingo Molnar, H. Peter Anvin, Martin Mares,
	Jon Derrick

Keith,

On Fri, 6 Nov 2015, Keith Busch wrote:
> +
> +static DEFINE_SPINLOCK(list_lock);

Can you please make that DEFINE_RAW_SPINLOCK as it nests into the irq
descriptor lock.

> +
> +struct vmd_irq {
> +	struct list_head	node;
> +	struct rcu_head		rcu;	/* rcu callback list */
> +	struct vmd_irq_list	*irq;	/* back pointer */

Please use KernelDoc style documentation if you want to document your
data structures.

> +/**
> + * XXX: Stubbed until a good way to not create conflicts with other devices
> + * sharing the same vector is developed.
> + */
> +static int vmd_irq_set_affinity(struct irq_data *data, const struct cpumask *dest,
> +								bool force)
> +{
> +	return -EINVAL;
> +}

Well, this is a hard problem for shared vectors. The question is
whether we really want to expose that at the per device level or
rather at the demultiplexing level.

> +static struct vmd_irq_list *vmd_next_irq(struct vmd_dev *vmd)
> +{
> +	int i, best = 0;
> +
> +	spin_lock(&list_lock);
> +	for (i = 1; i < vmd->msix_count; i++)
> +		if (vmd->irqs[i].count < vmd->irqs[best].count)
> +			best = i;
> +	vmd->irqs[best].count++;
> +	spin_unlock(&list_lock);
> +
> +	return &vmd->irqs[best];

That looks smarter. Though we might need some more complex way to do
that when interrupt affinity comes into play.

> +static irqreturn_t vmd_irq(int irq, void *data)
> +{
> +	struct vmd_irq_list *irqs = data;
> +	struct vmd_irq *vmdirq;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(vmdirq, &irqs->irq_list, node)
> +		generic_handle_irq(vmdirq->virq);
> +	rcu_read_unlock();
> +
> +	return IRQ_HANDLED;

Please remind me, that I still need to fix that life time problem in
the core.

> +	for (i = 0; i < vmd->msix_count; i++) {
> +		INIT_LIST_HEAD(&vmd->irqs[i].irq_list);
> +		vmd->irqs[i].vmd_vector = vmd->msix_entries[i].vector;
> +		vmd->irqs[i].index = i;
> +
> +		err = devm_request_irq(&dev->dev, vmd->irqs[i].vmd_vector,
> +				vmd_irq, IRQF_SHARED, "vmd", &vmd->irqs[i]);

This is a MSI interrupt which is never shared.

Aside of the few nitpicks above I'm really happy with the interrupt
side of that driver. Thanks for following up on all the comments!

Thanks,

	tglx



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-11-07 11:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-06 22:35 [PATCHv4 0/6] Driver for new VMD device Keith Busch
2015-11-06 22:35 ` [PATCHv4 1/6] msi: Relax msi_domain_alloc() to support parentless MSI irqdomains Keith Busch
2015-11-07 11:43   ` Thomas Gleixner
2015-11-06 22:35 ` [PATCHv4 2/6] pci: skip child bus with conflicting resources Keith Busch
2015-11-06 22:35 ` [PATCHv4 3/6] Export msi and irq functions for module use Keith Busch
2015-11-06 22:35 ` [PATCHv4 4/6] x86-pci: allow pci domain specific dma ops Keith Busch
2015-11-06 22:35 ` [PATCHv4 5/6] x86/pci: Initial commit for new VMD device driver Keith Busch
2015-11-07 11:55   ` Thomas Gleixner
2015-11-06 22:35 ` [PATCHv4 6/6] pciutils: Allow 32-bit domains Keith Busch

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.