When a device has multiple aliases that all are from the same bus, we program the IRTE to accept requests from any matching device on the bus.

This is so NTB devices which can have requests from multiple bus-devfns can pass MSI interrupts through across the bridge.

Signed-off-by: Logan Gunthorpe logang@deltatee.com Cc: David Woodhouse dwmw2@infradead.org --- drivers/iommu/intel_irq_remapping.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c index 24d45b07f425..8d3107a6b2b4 100644 --- a/drivers/iommu/intel_irq_remapping.c +++ b/drivers/iommu/intel_irq_remapping.c @@ -356,6 +356,8 @@ static int set_hpet_sid(struct irte *irte, u8 id) struct set_msi_sid_data { struct pci_dev *pdev; u16 alias; + int count; + int busmatch_count; };

static int set_msi_sid_cb(struct pci_dev *pdev, u16 alias, void *opaque) @@ -364,6 +366,10 @@ static int set_msi_sid_cb(struct pci_dev *pdev, u16 alias, void *opaque)

data->pdev = pdev; data->alias = alias; + data->count++; + + if (PCI_BUS_NUM(alias) == pdev->bus->number) + data->busmatch_count++;

return 0; } @@ -375,6 +381,8 @@ static int set_msi_sid(struct irte *irte, struct pci_dev *dev) if (!irte || !dev) return -1;

+ data.count = 0; + data.busmatch_count = 0; pci_for_each_dma_alias(dev, set_msi_sid_cb, &data);

/* @@ -394,6 +402,10 @@ static int set_msi_sid(struct irte *irte, struct pci_dev *dev) set_irte_sid(irte, SVT_VERIFY_BUS, SQ_ALL_16, PCI_DEVID(PCI_BUS_NUM(data.alias), dev->bus->number)); + else if (data.count >= 2 && data.busmatch_count == data.count) + set_irte_sid(irte, SVT_VERIFY_BUS, SQ_ALL_16, + PCI_DEVID(dev->bus->number, + dev->bus->number)); else if (data.pdev->bus->number != dev->bus->number) set_irte_sid(irte, SVT_VERIFY_SID_SQ, SQ_ALL_16, data.alias); else

Seeing the we want to use more interrupts in the NTB MSI code we need to be able allocate more (sometimes virtual) interrupts in the switchtec driver. Therefore add a module parameter to request to allocate additional interrupts.

This puts virtually no limit on the number of MSI interrupts available to NTB clients.

Signed-off-by: Logan Gunthorpe logang@deltatee.com Cc: Bjorn Helgaas bhelgaas@google.com --- drivers/pci/switch/switchtec.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c index e22766c79fe9..8b1db78197d9 100644 --- a/drivers/pci/switch/switchtec.c +++ b/drivers/pci/switch/switchtec.c @@ -30,6 +30,10 @@ module_param(use_dma_mrpc, bool, 0644); MODULE_PARM_DESC(use_dma_mrpc, "Enable the use of the DMA MRPC feature");

+static int nirqs = 32; +module_param(nirqs, int, 0644); +MODULE_PARM_DESC(nirqs, "number of interrupts to allocate (more may be useful for NTB applications)"); + static dev_t switchtec_devt; static DEFINE_IDA(switchtec_minor_ida);

@@ -1247,8 +1251,12 @@ static int switchtec_init_isr(struct switchtec_dev *stdev) int dma_mrpc_irq; int rc;

- nvecs = pci_alloc_irq_vectors(stdev->pdev, 1, 4, - PCI_IRQ_MSIX | PCI_IRQ_MSI); + if (nirqs < 4) + nirqs = 4; + + nvecs = pci_alloc_irq_vectors(stdev->pdev, 1, nirqs, + PCI_IRQ_MSIX | PCI_IRQ_MSI | + PCI_IRQ_VIRTUAL); if (nvecs < 0) return nvecs;

The kbuild system does not support having multiple source files in a module if one of those source files has the same name as the module.

Therefore, we must rename ntb.c to core.c, while the module remains ntb.ko.

This is similar to the way the nvme modules are structured.

Signed-off-by: Logan Gunthorpe logang@deltatee.com Cc: Jon Mason jdmason@kudzu.us Cc: Dave Jiang dave.jiang@intel.com Cc: Allen Hubbe allenbh@gmail.com --- drivers/ntb/Makefile | 2 ++ drivers/ntb/{ntb.c => core.c} | 0 2 files changed, 2 insertions(+) rename drivers/ntb/{ntb.c => core.c} (100%)

diff --git a/drivers/ntb/Makefile b/drivers/ntb/Makefile index 1921dec1949d..537226f8e78d 100644 --- a/drivers/ntb/Makefile +++ b/drivers/ntb/Makefile @@ -1,2 +1,4 @@ obj-$(CONFIG_NTB) += ntb.o hw/ test/ obj-$(CONFIG_NTB_TRANSPORT) += ntb_transport.o + +ntb-y := core.o diff --git a/drivers/ntb/ntb.c b/drivers/ntb/core.c similarity index 100% rename from drivers/ntb/ntb.c rename to drivers/ntb/core.c

Introduce a tool to test NTB MSI interrupts similar to the other NTB test tools. This tool creates a debugfs directory for each NTB device with the following files:

port irqX_occurrences peerX/port peerX/count peerX/trigger

The 'port' file tells the user the local port number and the 'occurrences' files tell the number of local interrupts that have been received for each interrupt.

For each peer, the 'port' file and the 'count' file tell you the peer's port number and number of interrupts respectively. Writing the interrupt number to the 'trigger' file triggers the interrupt handler for the peer which should increment their corresponding 'occurrences' file. The 'ready' file indicates if a peer is ready, writing to this file blocks until it is ready.

The module parameter num_irqs can be used to set the number of local interrupts. By default this is 4. This is only limited by the number of unused MSI interrupts registered by the hardware (this will require support of the hardware driver) and there must be at least 2*num_irqs + 1 spads registers available.

Signed-off-by: Logan Gunthorpe logang@deltatee.com Cc: Jon Mason jdmason@kudzu.us Cc: Dave Jiang dave.jiang@intel.com Cc: Allen Hubbe allenbh@gmail.com --- drivers/ntb/test/Kconfig | 9 + drivers/ntb/test/Makefile | 1 + drivers/ntb/test/ntb_msi_test.c | 416 ++++++++++++++++++++++++++++++++ 3 files changed, 426 insertions(+) create mode 100644 drivers/ntb/test/ntb_msi_test.c

diff --git a/drivers/ntb/test/Kconfig b/drivers/ntb/test/Kconfig index a5d0eda44438..a3f3e2638935 100644 --- a/drivers/ntb/test/Kconfig +++ b/drivers/ntb/test/Kconfig @@ -25,3 +25,12 @@ config NTB_PERF to and from the window without additional software interaction.

If unsure, say N. + +config NTB_MSI_TEST + tristate "NTB MSI Test Client" + depends on NTB_MSI + help + This tool demonstrates the use of the NTB MSI library to + send MSI interrupts between peers. + + If unsure, say N. diff --git a/drivers/ntb/test/Makefile b/drivers/ntb/test/Makefile index 9e77e0b761c2..d2895ca995e4 100644 --- a/drivers/ntb/test/Makefile +++ b/drivers/ntb/test/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_NTB_PINGPONG) += ntb_pingpong.o obj-$(CONFIG_NTB_TOOL) += ntb_tool.o obj-$(CONFIG_NTB_PERF) += ntb_perf.o +obj-$(CONFIG_NTB_MSI_TEST) += ntb_msi_test.o diff --git a/drivers/ntb/test/ntb_msi_test.c b/drivers/ntb/test/ntb_msi_test.c new file mode 100644 index 000000000000..2d7435591a65 --- /dev/null +++ b/drivers/ntb/test/ntb_msi_test.c @@ -0,0 +1,416 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) + +#include <linux/module.h> +#include <linux/debugfs.h> +#include <linux/ntb.h> +#include <linux/pci.h> +#include <linux/radix-tree.h> +#include <linux/workqueue.h> + +MODULE_LICENSE("Dual BSD/GPL"); +MODULE_VERSION("0.1"); +MODULE_AUTHOR("Logan Gunthorpe logang@deltatee.com"); +MODULE_DESCRIPTION("Test for sending MSI interrupts over an NTB memory window"); + +static int num_irqs = 4; +module_param(num_irqs, int, 0644); +MODULE_PARM_DESC(num_irqs, "number of irqs to use"); + +struct ntb_msit_ctx { + struct ntb_dev *ntb; + struct dentry *dbgfs_dir; + struct work_struct setup_work; + + struct ntb_msit_isr_ctx { + int irq_idx; + int irq_num; + int occurrences; + struct ntb_msit_ctx *nm; + struct ntb_msi_desc desc; + } *isr_ctx; + + struct ntb_msit_peer { + struct ntb_msit_ctx *nm; + int pidx; + int num_irqs; + struct completion init_comp; + struct ntb_msi_desc *msi_desc; + } peers[]; +}; + +static struct dentry *ntb_msit_dbgfs_topdir; + +static irqreturn_t ntb_msit_isr(int irq, void *dev) +{ + struct ntb_msit_isr_ctx *isr_ctx = dev; + struct ntb_msit_ctx *nm = isr_ctx->nm; + + dev_dbg(&nm->ntb->dev, "Interrupt Occurred: %d", + isr_ctx->irq_idx); + + isr_ctx->occurrences++; + + return IRQ_HANDLED; +} + +static void ntb_msit_setup_work(struct work_struct *work) +{ + struct ntb_msit_ctx *nm = container_of(work, struct ntb_msit_ctx, + setup_work); + int irq_count = 0; + int irq; + int ret; + uintptr_t i; + + ret = ntb_msi_setup_mws(nm->ntb); + if (ret) { + dev_err(&nm->ntb->dev, "Unable to setup MSI windows: %d\n", + ret); + return; + } + + for (i = 0; i < num_irqs; i++) { + nm->isr_ctx[i].irq_idx = i; + nm->isr_ctx[i].nm = nm; + + if (!nm->isr_ctx[i].irq_num) { + irq = ntbm_msi_request_irq(nm->ntb, ntb_msit_isr, + KBUILD_MODNAME, + &nm->isr_ctx[i], + &nm->isr_ctx[i].desc); + if (irq < 0) + break; + + nm->isr_ctx[i].irq_num = irq; + } + + ret = ntb_spad_write(nm->ntb, 2 * i + 1, + nm->isr_ctx[i].desc.addr_offset); + if (ret) + break; + + ret = ntb_spad_write(nm->ntb, 2 * i + 2, + nm->isr_ctx[i].desc.data); + if (ret) + break; + + irq_count++; + } + + ntb_spad_write(nm->ntb, 0, irq_count); + ntb_peer_db_set(nm->ntb, BIT(ntb_port_number(nm->ntb))); +} + +static void ntb_msit_link_event(void *ctx) +{ + struct ntb_msit_ctx *nm = ctx; + + if (!ntb_link_is_up(nm->ntb, NULL, NULL)) + return; + + schedule_work(&nm->setup_work); +} + +static void ntb_msit_copy_peer_desc(struct ntb_msit_ctx *nm, int peer) +{ + int i; + struct ntb_msi_desc *desc = nm->peers[peer].msi_desc; + int irq_count = nm->peers[peer].num_irqs; + + for (i = 0; i < irq_count; i++) { + desc[i].addr_offset = ntb_peer_spad_read(nm->ntb, peer, + 2 * i + 1); + desc[i].data = ntb_peer_spad_read(nm->ntb, peer, 2 * i + 2); + } + + dev_info(&nm->ntb->dev, "Found %d interrupts on peer %d\n", + irq_count, peer); + + complete_all(&nm->peers[peer].init_comp); +} + +static void ntb_msit_db_event(void *ctx, int vec) +{ + struct ntb_msit_ctx *nm = ctx; + struct ntb_msi_desc *desc; + u64 peer_mask = ntb_db_read(nm->ntb); + u32 irq_count; + int peer; + + ntb_db_clear(nm->ntb, peer_mask); + + for (peer = 0; peer < sizeof(peer_mask) * 8; peer++) { + if (!(peer_mask & BIT(peer))) + continue; + + irq_count = ntb_peer_spad_read(nm->ntb, peer, 0); + if (irq_count == -1) + continue; + + desc = kcalloc(irq_count, sizeof(*desc), GFP_ATOMIC); + if (!desc) + continue; + + kfree(nm->peers[peer].msi_desc); + nm->peers[peer].msi_desc = desc; + nm->peers[peer].num_irqs = irq_count; + + ntb_msit_copy_peer_desc(nm, peer); + } +} + +static const struct ntb_ctx_ops ntb_msit_ops = { + .link_event = ntb_msit_link_event, + .db_event = ntb_msit_db_event, +}; + +static int ntb_msit_dbgfs_trigger(void *data, u64 idx) +{ + struct ntb_msit_peer *peer = data; + + if (idx >= peer->num_irqs) + return -EINVAL; + + dev_dbg(&peer->nm->ntb->dev, "trigger irq %llu on peer %u\n", + idx, peer->pidx); + + return ntb_msi_peer_trigger(peer->nm->ntb, peer->pidx, + &peer->msi_desc[idx]); +} + +DEFINE_DEBUGFS_ATTRIBUTE(ntb_msit_trigger_fops, NULL, + ntb_msit_dbgfs_trigger, "%llu\n"); + +static int ntb_msit_dbgfs_port_get(void *data, u64 *port) +{ + struct ntb_msit_peer *peer = data; + + *port = ntb_peer_port_number(peer->nm->ntb, peer->pidx); + + return 0; +} + +DEFINE_DEBUGFS_ATTRIBUTE(ntb_msit_port_fops, ntb_msit_dbgfs_port_get, + NULL, "%llu\n"); + +static int ntb_msit_dbgfs_count_get(void *data, u64 *count) +{ + struct ntb_msit_peer *peer = data; + + *count = peer->num_irqs; + + return 0; +} + +DEFINE_DEBUGFS_ATTRIBUTE(ntb_msit_count_fops, ntb_msit_dbgfs_count_get, + NULL, "%llu\n"); + +static int ntb_msit_dbgfs_ready_get(void *data, u64 *ready) +{ + struct ntb_msit_peer *peer = data; + + *ready = try_wait_for_completion(&peer->init_comp); + + return 0; +} + +static int ntb_msit_dbgfs_ready_set(void *data, u64 ready) +{ + struct ntb_msit_peer *peer = data; + + return wait_for_completion_interruptible(&peer->init_comp); +} + +DEFINE_DEBUGFS_ATTRIBUTE(ntb_msit_ready_fops, ntb_msit_dbgfs_ready_get, + ntb_msit_dbgfs_ready_set, "%llu\n"); + +static int ntb_msit_dbgfs_occurrences_get(void *data, u64 *occurrences) +{ + struct ntb_msit_isr_ctx *isr_ctx = data; + + *occurrences = isr_ctx->occurrences; + + return 0; +} + +DEFINE_DEBUGFS_ATTRIBUTE(ntb_msit_occurrences_fops, + ntb_msit_dbgfs_occurrences_get, + NULL, "%llu\n"); + +static int ntb_msit_dbgfs_local_port_get(void *data, u64 *port) +{ + struct ntb_msit_ctx *nm = data; + + *port = ntb_port_number(nm->ntb); + + return 0; +} + +DEFINE_DEBUGFS_ATTRIBUTE(ntb_msit_local_port_fops, + ntb_msit_dbgfs_local_port_get, + NULL, "%llu\n"); + +static void ntb_msit_create_dbgfs(struct ntb_msit_ctx *nm) +{ + struct pci_dev *pdev = nm->ntb->pdev; + char buf[32]; + int i; + struct dentry *peer_dir; + + nm->dbgfs_dir = debugfs_create_dir(pci_name(pdev), + ntb_msit_dbgfs_topdir); + debugfs_create_file("port", 0400, nm->dbgfs_dir, nm, + &ntb_msit_local_port_fops); + + for (i = 0; i < ntb_peer_port_count(nm->ntb); i++) { + nm->peers[i].pidx = i; + nm->peers[i].nm = nm; + init_completion(&nm->peers[i].init_comp); + + snprintf(buf, sizeof(buf), "peer%d", i); + peer_dir = debugfs_create_dir(buf, nm->dbgfs_dir); + + debugfs_create_file_unsafe("trigger", 0200, peer_dir, + &nm->peers[i], + &ntb_msit_trigger_fops); + + debugfs_create_file_unsafe("port", 0400, peer_dir, + &nm->peers[i], &ntb_msit_port_fops); + + debugfs_create_file_unsafe("count", 0400, peer_dir, + &nm->peers[i], + &ntb_msit_count_fops); + + debugfs_create_file_unsafe("ready", 0600, peer_dir, + &nm->peers[i], + &ntb_msit_ready_fops); + } + + for (i = 0; i < num_irqs; i++) { + snprintf(buf, sizeof(buf), "irq%d_occurrences", i); + debugfs_create_file_unsafe(buf, 0400, nm->dbgfs_dir, + &nm->isr_ctx[i], + &ntb_msit_occurrences_fops); + } +} + +static void ntb_msit_remove_dbgfs(struct ntb_msit_ctx *nm) +{ + debugfs_remove_recursive(nm->dbgfs_dir); +} + +static int ntb_msit_probe(struct ntb_client *client, struct ntb_dev *ntb) +{ + struct ntb_msit_ctx *nm; + size_t struct_size; + int peers; + int ret; + + peers = ntb_peer_port_count(ntb); + if (peers <= 0) + return -EINVAL; + + if (ntb_spad_is_unsafe(ntb) || ntb_spad_count(ntb) < 2 * num_irqs + 1) { + dev_err(&ntb->dev, "NTB MSI test requires at least %d spads for %d irqs\n", + 2 * num_irqs + 1, num_irqs); + return -EFAULT; + } + + ret = ntb_spad_write(ntb, 0, -1); + if (ret) { + dev_err(&ntb->dev, "Unable to write spads: %d\n", ret); + return ret; + } + + ret = ntb_db_clear_mask(ntb, GENMASK(peers - 1, 0)); + if (ret) { + dev_err(&ntb->dev, "Unable to clear doorbell mask: %d\n", ret); + return ret; + } + + ret = ntb_msi_init(ntb); + if (ret) { + dev_err(&ntb->dev, "Unable to initialize MSI library: %d\n", + ret); + return ret; + } + + struct_size = sizeof(*nm) + sizeof(*nm->peers) * peers; + + nm = devm_kzalloc(&ntb->dev, struct_size, GFP_KERNEL); + if (!nm) + return -ENOMEM; + + nm->isr_ctx = devm_kcalloc(&ntb->dev, num_irqs, sizeof(*nm->isr_ctx), + GFP_KERNEL); + if (!nm->isr_ctx) + return -ENOMEM; + + INIT_WORK(&nm->setup_work, ntb_msit_setup_work); + nm->ntb = ntb; + + ntb_msit_create_dbgfs(nm); + + ret = ntb_set_ctx(ntb, nm, &ntb_msit_ops); + if (ret) + goto remove_dbgfs; + + if (!nm->isr_ctx) + goto remove_dbgfs; + + ntb_link_enable(ntb, NTB_SPEED_AUTO, NTB_WIDTH_AUTO); + + return 0; + +remove_dbgfs: + ntb_msit_remove_dbgfs(nm); + devm_kfree(&ntb->dev, nm->isr_ctx); + devm_kfree(&ntb->dev, nm); + return ret; +} + +static void ntb_msit_remove(struct ntb_client *client, struct ntb_dev *ntb) +{ + struct ntb_msit_ctx *nm = ntb->ctx; + int i; + + ntb_link_disable(ntb); + ntb_db_set_mask(ntb, ntb_db_valid_mask(ntb)); + ntb_msi_clear_mws(ntb); + + for (i = 0; i < ntb_peer_port_count(ntb); i++) + kfree(nm->peers[i].msi_desc); + + ntb_clear_ctx(ntb); + ntb_msit_remove_dbgfs(nm); +} + +static struct ntb_client ntb_msit_client = { + .ops = { + .probe = ntb_msit_probe, + .remove = ntb_msit_remove + } +}; + +static int __init ntb_msit_init(void) +{ + int ret; + + if (debugfs_initialized()) + ntb_msit_dbgfs_topdir = debugfs_create_dir(KBUILD_MODNAME, + NULL); + + ret = ntb_register_client(&ntb_msit_client); + if (ret) + debugfs_remove_recursive(ntb_msit_dbgfs_topdir); + + return ret; +} +module_init(ntb_msit_init); + +static void __exit ntb_msit_exit(void) +{ + ntb_unregister_client(&ntb_msit_client); + debugfs_remove_recursive(ntb_msit_dbgfs_topdir); +} +module_exit(ntb_msit_exit);

Introduce the module parameter 'use_msi' which, when set uses MSI interrupts instead of doorbells for each queue pair (QP). T he parameter is only available if NTB MSI support is configured into the kernel. We also require there to be more than one memory window (MW) so that an extra one is available to forward the APIC region.

To use MSIs, we request one interrupt per QP and forward the MSI address and data to the peer using scratch pad registers (SPADS) above the MW spads. (If there are not enough SPADS the MSI interrupt will not be used.)

Once registered, we simply use ntb_msi_peer_trigger and the recieving ISR simply queues up the rxc_db_work for the queue.

This addition can significantly improve performance of ntb_transport. In a simple, untuned, apples-to-apples comparision using ntb_netdev and iperf with switchtec hardware, I see 3.88Gb/s without MSI interrupts and 14.1Gb/s which is a more than 3x improvement.

Signed-off-by: Logan Gunthorpe logang@deltatee.com Cc: Jon Mason jdmason@kudzu.us Cc: Dave Jiang dave.jiang@intel.com Cc: Allen Hubbe allenbh@gmail.com --- drivers/ntb/ntb_transport.c | 134 +++++++++++++++++++++++++++++++++++- 1 file changed, 133 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c index 526b65afc16a..d9e61bf28830 100644 --- a/drivers/ntb/ntb_transport.c +++ b/drivers/ntb/ntb_transport.c @@ -93,6 +93,12 @@ static bool use_dma; module_param(use_dma, bool, 0644); MODULE_PARM_DESC(use_dma, "Use DMA engine to perform large data copy");

+static bool use_msi; +#ifdef CONFIG_NTB_MSI +module_param(use_msi, bool, 0644); +MODULE_PARM_DESC(use_msi, "Use MSI interrupts instead of doorbells"); +#endif + static struct dentry *nt_debugfs_dir;

/* Only two-ports NTB devices are supported */ @@ -188,6 +194,11 @@ struct ntb_transport_qp { u64 tx_err_no_buf; u64 tx_memcpy; u64 tx_async; + + bool use_msi; + int msi_irq; + struct ntb_msi_desc msi_desc; + struct ntb_msi_desc peer_msi_desc; };

struct ntb_transport_mw { @@ -221,6 +232,9 @@ struct ntb_transport_ctx { u64 qp_bitmap; u64 qp_bitmap_free;

+ bool use_msi; + unsigned int msi_spad_offset; + bool link_is_up; struct delayed_work link_work; struct work_struct link_cleanup; @@ -667,6 +681,91 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt, return 0; }

+static irqreturn_t ntb_transport_isr(int irq, void *dev) +{ + struct ntb_transport_qp *qp = dev; + + tasklet_schedule(&qp->rxc_db_work); + + return IRQ_HANDLED; +} + +static void ntb_transport_setup_qp_peer_msi(struct ntb_transport_ctx *nt, + unsigned int qp_num) +{ + struct ntb_transport_qp *qp = &nt->qp_vec[qp_num]; + int spad = qp_num * 2 + nt->msi_spad_offset; + + if (!nt->use_msi) + return; + + if (spad >= ntb_spad_count(nt->ndev)) + return; + + qp->peer_msi_desc.addr_offset = + ntb_peer_spad_read(qp->ndev, PIDX, spad); + qp->peer_msi_desc.data = + ntb_peer_spad_read(qp->ndev, PIDX, spad + 1); + + dev_dbg(&qp->ndev->pdev->dev, "QP%d Peer MSI addr=%x data=%x\n", + qp_num, qp->peer_msi_desc.addr_offset, qp->peer_msi_desc.data); + + if (qp->peer_msi_desc.addr_offset) { + qp->use_msi = true; + dev_info(&qp->ndev->pdev->dev, + "Using MSI interrupts for QP%d\n", qp_num); + } +} + +static void ntb_transport_setup_qp_msi(struct ntb_transport_ctx *nt, + unsigned int qp_num) +{ + struct ntb_transport_qp *qp = &nt->qp_vec[qp_num]; + int spad = qp_num * 2 + nt->msi_spad_offset; + int rc; + + if (!nt->use_msi) + return; + + if (spad >= ntb_spad_count(nt->ndev)) { + dev_warn_once(&qp->ndev->pdev->dev, + "Not enough SPADS to use MSI interrupts\n"); + return; + } + + ntb_spad_write(qp->ndev, spad, 0); + ntb_spad_write(qp->ndev, spad + 1, 0); + + if (!qp->msi_irq) { + qp->msi_irq = ntbm_msi_request_irq(qp->ndev, ntb_transport_isr, + KBUILD_MODNAME, qp, + &qp->msi_desc); + if (qp->msi_irq < 0) { + dev_warn(&qp->ndev->pdev->dev, + "Unable to allocate MSI interrupt for qp%d\n", + qp_num); + return; + } + } + + rc = ntb_spad_write(qp->ndev, spad, qp->msi_desc.addr_offset); + if (rc) + goto err_free_interrupt; + + rc = ntb_spad_write(qp->ndev, spad + 1, qp->msi_desc.data); + if (rc) + goto err_free_interrupt; + + dev_dbg(&qp->ndev->pdev->dev, "QP%d MSI %d addr=%x data=%x\n", + qp_num, qp->msi_irq, qp->msi_desc.addr_offset, + qp->msi_desc.data); + + return; + +err_free_interrupt: + devm_free_irq(&nt->ndev->dev, qp->msi_irq, qp); +} + static void ntb_free_mw(struct ntb_transport_ctx *nt, int num_mw) { struct ntb_transport_mw *mw = &nt->mw_vec[num_mw]; @@ -902,6 +1001,20 @@ static void ntb_transport_link_work(struct work_struct *work) int rc = 0, i, spad;

/* send the local info, in the opposite order of the way we read it */ + + if (nt->use_msi) { + rc = ntb_msi_setup_mws(ndev); + if (rc) { + dev_warn(&pdev->dev, + "Failed to register MSI memory window: %d\n", + rc); + nt->use_msi = false; + } + } + + for (i = 0; i < nt->qp_count; i++) + ntb_transport_setup_qp_msi(nt, i); + for (i = 0; i < nt->mw_count; i++) { size = nt->mw_vec[i].phys_size;

@@ -959,6 +1072,7 @@ static void ntb_transport_link_work(struct work_struct *work) struct ntb_transport_qp *qp = &nt->qp_vec[i];

ntb_transport_setup_qp_mw(nt, i); + ntb_transport_setup_qp_peer_msi(nt, i);

if (qp->client_ready) schedule_delayed_work(&qp->link_work, 0); @@ -1132,6 +1246,19 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev) return -ENOMEM;

nt->ndev = ndev; + + /* + * If we are using MSI, and have at least one extra memory window, + * we will reserve the last MW for the MSI window. + */ + if (use_msi && mw_count > 1) { + rc = ntb_msi_init(ndev); + if (!rc) { + mw_count -= 1; + nt->use_msi = true; + } + } + spad_count = ntb_spad_count(ndev);

/* Limit the MW's based on the availability of scratchpads */ @@ -1145,6 +1272,8 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev) max_mw_count_for_spads = (spad_count - MW0_SZ_HIGH) / 2; nt->mw_count = min(mw_count, max_mw_count_for_spads);

+ nt->msi_spad_offset = nt->mw_count * 2 + MW0_SZ_HIGH; + nt->mw_vec = kcalloc_node(mw_count, sizeof(*nt->mw_vec), GFP_KERNEL, node); if (!nt->mw_vec) { @@ -1598,7 +1727,10 @@ static void ntb_tx_copy_callback(void *data,

iowrite32(entry->flags | DESC_DONE_FLAG, &hdr->flags);

- ntb_peer_db_set(qp->ndev, BIT_ULL(qp->qp_num)); + if (qp->use_msi) + ntb_msi_peer_trigger(qp->ndev, PIDX, &qp->peer_msi_desc); + else + ntb_peer_db_set(qp->ndev, BIT_ULL(qp->qp_num));

/* The entry length can only be zero if the packet is intended to be a * "link down" or similar. Since no payload is being sent in these

[+cc Thomas, Marc]

On Thu, Jan 31, 2019 at 11:56:49AM -0700, Logan Gunthorpe wrote:

For NTB devices, we want to be able to trigger MSI interrupts through a memory window. In these cases we may want to use more interrupts than the NTB PCI device has available in its MSI-X table.

We allow for this by creating a new 'virtual' interrupt. These interrupts are allocated as usual but are not programmed into the MSI-X table (as there may not be space for them).

The MSI address and data will then handled through an NTB MSI library introduced later in this series.

Signed-off-by: Logan Gunthorpe logang@deltatee.com Cc: Bjorn Helgaas bhelgaas@google.com

I assume you'll merge this along with the rest of the series, so:

Acked-by: Bjorn Helgaas bhelgaas@google.com

Minor question and typo below.

---

drivers/pci/msi.c | 51 +++++++++++++++++++++++++++++++++++++-------- include/linux/msi.h | 1 + include/linux/pci.h | 9 ++++++++ 3 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 4c0b47867258..145587da686c 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -192,6 +192,9 @@ static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag) static void __iomem *pci_msix_desc_addr(struct msi_desc *desc) {

• if (desc->msi_attrib.is_virtual)

• return NULL;
  

• return desc->mask_base + desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE;

} @@ -206,14 +209,19 @@ static void __iomem *pci_msix_desc_addr(struct msi_desc *desc) u32 __pci_msix_desc_mask_irq(struct msi_desc *desc, u32 flag) { u32 mask_bits = desc->masked;

• void __iomem *desc_addr;

if (pci_msi_ignore_mask) return 0;

• desc_addr = pci_msix_desc_addr(desc);
• if (!desc_addr)

• return 0;
  

mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT; if (flag) mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT;

• writel(mask_bits, pci_msix_desc_addr(desc) + PCI_MSIX_ENTRY_VECTOR_CTRL);

• writel(mask_bits, desc_addr + PCI_MSIX_ENTRY_VECTOR_CTRL);

return mask_bits; } @@ -273,6 +281,11 @@ void __pci_read_msi_msg(struct msi_desc *entry, struct msi_msg *msg) if (entry->msi_attrib.is_msix) { void __iomem *base = pci_msix_desc_addr(entry);

• if (!base) {
  

• 	WARN_ON(1);
  

• 	return;
  

• }
  

• msg->address_lo = readl(base + PCI_MSIX_ENTRY_LOWER_ADDR); msg->address_hi = readl(base + PCI_MSIX_ENTRY_UPPER_ADDR); msg->data = readl(base + PCI_MSIX_ENTRY_DATA);

@@ -303,6 +316,9 @@ void __pci_write_msi_msg(struct msi_desc *entry, struct msi_msg *msg) } else if (entry->msi_attrib.is_msix) { void __iomem *base = pci_msix_desc_addr(entry);

• if (!base)
  

• 	goto skip;
  

• writel(msg->address_lo, base + PCI_MSIX_ENTRY_LOWER_ADDR); writel(msg->address_hi, base + PCI_MSIX_ENTRY_UPPER_ADDR); writel(msg->data, base + PCI_MSIX_ENTRY_DATA);

@@ -327,6 +343,8 @@ void __pci_write_msi_msg(struct msi_desc *entry, struct msi_msg *msg) msg->data); } }

+skip: entry->msg = *msg; } @@ -550,6 +568,7 @@ msi_setup_entry(struct pci_dev *dev, int nvec, const struct irq_affinity *affd) entry->msi_attrib.is_msix = 0; entry->msi_attrib.is_64 = !!(control & PCI_MSI_FLAGS_64BIT);

• entry->msi_attrib.is_virtual = 0; entry->msi_attrib.entry_nr = 0; entry->msi_attrib.maskbit = !!(control & PCI_MSI_FLAGS_MASKBIT); entry->msi_attrib.default_irq = dev->irq; /* Save IOAPIC IRQ */

@@ -674,6 +693,7 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base, struct irq_affinity_desc *curmsk, *masks = NULL; struct msi_desc *entry; int ret, i;

• int vec_count = pci_msix_vec_count(dev);

if (affd) masks = irq_create_affinity_masks(nvec, affd); @@ -696,6 +716,10 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base, entry->msi_attrib.entry_nr = entries[i].entry; else entry->msi_attrib.entry_nr = i;

• entry->msi_attrib.is_virtual =
  

• 	entry->msi_attrib.entry_nr >= vec_count;
  

• entry->msi_attrib.default_irq = dev->irq; entry->mask_base = base;

@@ -714,12 +738,19 @@ static void msix_program_entries(struct pci_dev *dev, { struct msi_desc *entry; int i = 0;

• void __iomem *desc_addr;

for_each_pci_msi_entry(entry, dev) { if (entries) entries[i++].vector = entry->irq;

• entry->masked = readl(pci_msix_desc_addr(entry) +
  

• 		PCI_MSIX_ENTRY_VECTOR_CTRL);
  

• desc_addr = pci_msix_desc_addr(entry);
  

• if (desc_addr)
  

• 	entry->masked = readl(desc_addr +
  

• 			      PCI_MSIX_ENTRY_VECTOR_CTRL);
  

• else
  

• 	entry->masked = 0;
  

• msix_mask_irq(entry, 1); }

} @@ -932,7 +963,8 @@ int pci_msix_vec_count(struct pci_dev *dev) EXPORT_SYMBOL(pci_msix_vec_count); static int __pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries,

• 	     int nvec, const struct irq_affinity *affd)
  

• 	     int nvec, const struct irq_affinity *affd,
  

• 	     int flags)
  

{ int nr_entries; int i, j; @@ -943,7 +975,7 @@ static int __pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, nr_entries = pci_msix_vec_count(dev); if (nr_entries < 0) return nr_entries;

• if (nvec > nr_entries)

• if (nvec > nr_entries && !(flags & PCI_IRQ_VIRTUAL)) return nr_entries;

if (entries) { @@ -1086,7 +1118,8 @@ EXPORT_SYMBOL(pci_enable_msi); static int __pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries, int minvec,

• 		   int maxvec, const struct irq_affinity *affd)
  

• 		   int maxvec, const struct irq_affinity *affd,
  

• 		   int flags)
  

{ int rc, nvec = maxvec; @@ -1110,7 +1143,7 @@ static int __pci_enable_msix_range(struct pci_dev *dev, return -ENOSPC; }

• rc = __pci_enable_msix(dev, entries, nvec, affd);
  

• rc = __pci_enable_msix(dev, entries, nvec, affd, flags);
  

  if (rc == 0) return nvec;

@@ -1141,7 +1174,7 @@ static int __pci_enable_msix_range(struct pci_dev *dev, int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries, int minvec, int maxvec) {

• return __pci_enable_msix_range(dev, entries, minvec, maxvec, NULL);

• return __pci_enable_msix_range(dev, entries, minvec, maxvec, NULL, 0);

} EXPORT_SYMBOL(pci_enable_msix_range); @@ -1181,7 +1214,7 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs, if (flags & PCI_IRQ_MSIX) { msix_vecs = __pci_enable_msix_range(dev, NULL, min_vecs,

• 				    max_vecs, affd);
  

• 				    max_vecs, affd, flags);
  

  if (msix_vecs > 0) return msix_vecs; }

diff --git a/include/linux/msi.h b/include/linux/msi.h index 784fb52b9900..6458ab049852 100644 --- a/include/linux/msi.h +++ b/include/linux/msi.h @@ -88,6 +88,7 @@ struct msi_desc { __u8 multi_cap : 3; __u8 maskbit : 1; __u8 is_64 : 1;

• 		__u8    is_virtual      : 1;
  

You did the right thing by using the same style as what's already here, but does anybody know why are we using __u8 and __u16 here?

Those typedefs are in include/uapi/asm-generic/int-l64.h, which suggests they're for things exported to user space, but I don't think that's the case here, so I'm wondering if we could someday replace these with u8 and u16. Obviously that wouldn't be part of *this* series.

		__u16	entry_nr;
		unsigned default_irq;
	} msi_attrib;

diff --git a/include/linux/pci.h b/include/linux/pci.h index 65f1d8c2f082..ce0815c2c498 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1352,6 +1352,15 @@ int pci_set_vga_state(struct pci_dev *pdev, bool decode, #define PCI_IRQ_MSI (1 << 1) /* Allow MSI interrupts */ #define PCI_IRQ_MSIX (1 << 2) /* Allow MSI-X interrupts */ #define PCI_IRQ_AFFINITY (1 << 3) /* Auto-assign affinity */

+/*

• • Virtual interrupts allow for more interrupts to be allocated
• • than the device has interrupts for. These are not programmed
• • into the devices MSI-X table and must be handled by some

s/devices/device's/

• • other driver means.
• */

+#define PCI_IRQ_VIRTUAL (1 << 4)

#define PCI_IRQ_ALL_TYPES \ (PCI_IRQ_LEGACY | PCI_IRQ_MSI | PCI_IRQ_MSIX) -- 2.19.0

On 2019-02-01 9:44 a.m., Joerg Roedel wrote:

On Thu, Jan 31, 2019 at 11:56:48AM -0700, Logan Gunthorpe wrote:

...

@@ -394,6 +402,10 @@ static int set_msi_sid(struct irte *irte, struct pci_dev *dev) set_irte_sid(irte, SVT_VERIFY_BUS, SQ_ALL_16, PCI_DEVID(PCI_BUS_NUM(data.alias), dev->bus->number));

• else if (data.count >= 2 && data.busmatch_count == data.count)

• set_irte_sid(irte, SVT_VERIFY_BUS, SQ_ALL_16,
  

• 	     PCI_DEVID(dev->bus->number,
  

• 		       dev->bus->number));
  

The dev->bus->number argument for the devfn parameter of PCI_DEVID is not needed, right?

Oh, yes. I think you are right.

Also, this requires at least a comment to document that special case.

I'll add a comment for v2.

Thanks for the review!

Logan

On Fri, 1 Feb 2019 10:27:29 -0700 Logan Gunthorpe logang@deltatee.com wrote:

On 2019-02-01 9:44 a.m., Joerg Roedel wrote:

...

On Thu, Jan 31, 2019 at 11:56:48AM -0700, Logan Gunthorpe wrote:

...

@@ -394,6 +402,10 @@ static int set_msi_sid(struct irte *irte, struct pci_dev *dev) set_irte_sid(irte, SVT_VERIFY_BUS, SQ_ALL_16, PCI_DEVID(PCI_BUS_NUM(data.alias), dev->bus->number));

I guess devfn can be removed also. but that is separate cleanup.

...

...

• else if (data.count >= 2 && data.busmatch_count ==

data.count)

• set_irte_sid(irte, SVT_VERIFY_BUS, SQ_ALL_16,
  

• 	     PCI_DEVID(dev->bus->number,
  

• 		       dev->bus->number));  
  

The dev->bus->number argument for the devfn parameter of PCI_DEVID is not needed, right?

Oh, yes. I think you are right.

...

Also, this requires at least a comment to document that special case.

I'll add a comment for v2.

Thanks for the review!

Logan _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu

[Jacob Pan]

On Tue, 5 Feb 2019 13:40:36 -0700 Logan Gunthorpe logang@deltatee.com wrote:

On 2019-02-05 12:19 p.m., Jacob Pan wrote:

...

On Fri, 1 Feb 2019 10:27:29 -0700 Logan Gunthorpe logang@deltatee.com wrote:

...

On 2019-02-01 9:44 a.m., Joerg Roedel wrote:

...

On Thu, Jan 31, 2019 at 11:56:48AM -0700, Logan Gunthorpe wrote:

...

@@ -394,6 +402,10 @@ static int set_msi_sid(struct irte *irte, struct pci_dev *dev) set_irte_sid(irte, SVT_VERIFY_BUS, SQ_ALL_16, PCI_DEVID(PCI_BUS_NUM(data.alias), dev->bus->number));

I guess devfn can be removed also. but that is separate cleanup.

Actually, no, I've dug into this and we *do* need the devfn here but it's needlessly confusing. We should not be using PCI_DEVID() as we aren't actually representing a DEVID in this case...

According to the Intel VT-D spec, when using SVT_VERIFY_BUS, the MSB of the SID field represents the starting bus number and the LSB represents the end bus number. The requester id's bus number must then be within that range. The PCI_DEVID macro matches these semantics if you assume the devfn is the end bus, but doesn't really make sense here and just confuses the issue.

So the code was correct, I'll just try to clean it up to make it less confusing.

you are right, thanks for explaining.

Thanks,

Logan