[PATCH 00/13] PCI: MSI: Getting rid of msi_controller, and other cleanups

[PATCH 00/13] PCI: MSI: Getting rid of msi_controller, and other cleanups

[Marc Zyngier]

The msi_controller data structure was the first attempt at treating
MSIs like any other interrupt. We replaced it a few years ago with the
generic MSI framework, but as it turns out, some older drivers are
still using it.

This series aims at converting these stragglers, drop msi_controller,
and fix some other nits such as having ways for a host bridge to
advertise whether it supports MSIs or not.

A few notes:

- The Tegra patch is the result of back and forth work with Thierry: I
  wrote the initial patch, which didn't work (I didn't have any HW at
  the time). Thierry made it work, and I subsequently fixed a couple
  of bugs/cleanups. I'm responsible for the result, so don't blame
  Thierry for any of it! FWIW, I'm now running a Jetson TX2 with its
  root fs over NVME, and MSIs are OK.

- RCAR is totally untested, though Marek had a go at a previous
  version. More testing required.

- The xilinx stuff is *really* untested. Paul, if you have a RISC-V
  board that uses it, could you please give it a go? Michal, same
  thing for the stuff you have at hand...

- hyperv: I don't have access to such hypervisor, and no way to test
  it. Help welcomed.

- The patches dealing with the advertising of MSI handling are the
  result of a long discussion that took place here[1]. I took the
  liberty to rejig Thomas' initial patches, and add what I needed for
  the MSI domain stuff. Again, blame me if something is wrong, and not
  Thomas.

Feedback welcome.

	M.

[1] https://lore.kernel.org/r/20201031140330.83768-1-linux@fw-web.de

Marc Zyngier (11):
  PCI: tegra: Convert to MSI domains
  PCI: rcar: Convert to MSI domains
  PCI: xilinx: Convert to MSI domains
  PCI: hyperv: Drop msi_controller structure
  PCI: MSI: Drop use of msi_controller from core code
  PCI: MSI: Kill msi_controller structure
  PCI: MSI: Kill default_teardown_msi_irqs()
  PCI: MSI: Let PCI host bridges declare their reliance on MSI domains
  PCI: Make pci_host_common_probe() declare its reliance on MSI domains
  PCI: MSI: Document the various ways of ending up with NO_MSI
  PCI: quirks: Refactor advertising of the NO_MSI flag

Thomas Gleixner (2):
  PCI: MSI: Let PCI host bridges declare their lack of MSI handling
  PCI: mediatek: Advertise lack of MSI handling

 drivers/pci/controller/Kconfig           |   4 +-
 drivers/pci/controller/pci-host-common.c |   1 +
 drivers/pci/controller/pci-hyperv.c      |   4 -
 drivers/pci/controller/pci-tegra.c       | 343 ++++++++++++-----------
 drivers/pci/controller/pcie-mediatek.c   |   4 +
 drivers/pci/controller/pcie-rcar-host.c  | 342 +++++++++++-----------
 drivers/pci/controller/pcie-xilinx.c     | 238 +++++++---------
 drivers/pci/msi.c                        |  46 +--
 drivers/pci/probe.c                      |   4 +-
 drivers/pci/quirks.c                     |  15 +-
 include/linux/msi.h                      |  17 +-
 include/linux/pci.h                      |   4 +-
 12 files changed, 463 insertions(+), 559 deletions(-)

-- 
2.29.2

[PATCH 01/13] PCI: tegra: Convert to MSI domains

[Marc Zyngier]

In anticipation of the removal of the msi_controller structure, convert
the Tegra host controller driver to MSI domains.

We end-up with the usual two domain structure, the top one being a
generic PCI/MSI domain, the bottom one being Tegra-specific and handling
the actual HW interrupt allocation.

While at it, convert the normal interrupt handler to a chained handler,
handle the controller's MSI IRQ edge triggered, support multiple MSIs
per device and use the AFI_MSI_EN_VEC* registers to provide MSI masking.

[treding@nvidia.com: fix, clean up and address TODOs from Marc's draft]
Signed-off-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/controller/Kconfig     |   1 -
 drivers/pci/controller/pci-tegra.c | 343 ++++++++++++++++-------------
 2 files changed, 185 insertions(+), 159 deletions(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 64e2f5e379aa..e7007e4028fc 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -41,7 +41,6 @@ config PCI_TEGRA
 	bool "NVIDIA Tegra PCIe controller"
 	depends on ARCH_TEGRA || COMPILE_TEST
 	depends on PCI_MSI_IRQ_DOMAIN
-	select PCI_MSI_ARCH_FALLBACKS
 	help
 	  Say Y here if you want support for the PCIe host controller found
 	  on NVIDIA Tegra SoCs.
diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 8fcabed7c6a6..eaba7b2fab4a 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -21,6 +21,7 @@
 #include <linux/interrupt.h>
 #include <linux/iopoll.h>
 #include <linux/irq.h>
+#include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
@@ -78,23 +79,8 @@
 #define AFI_MSI_FPCI_BAR_ST	0x64
 #define AFI_MSI_AXI_BAR_ST	0x68
 
-#define AFI_MSI_VEC0		0x6c
-#define AFI_MSI_VEC1		0x70
-#define AFI_MSI_VEC2		0x74
-#define AFI_MSI_VEC3		0x78
-#define AFI_MSI_VEC4		0x7c
-#define AFI_MSI_VEC5		0x80
-#define AFI_MSI_VEC6		0x84
-#define AFI_MSI_VEC7		0x88
-
-#define AFI_MSI_EN_VEC0		0x8c
-#define AFI_MSI_EN_VEC1		0x90
-#define AFI_MSI_EN_VEC2		0x94
-#define AFI_MSI_EN_VEC3		0x98
-#define AFI_MSI_EN_VEC4		0x9c
-#define AFI_MSI_EN_VEC5		0xa0
-#define AFI_MSI_EN_VEC6		0xa4
-#define AFI_MSI_EN_VEC7		0xa8
+#define AFI_MSI_VEC(x)		(0x6c + ((x) * 4))
+#define AFI_MSI_EN_VEC(x)	(0x8c + ((x) * 4))
 
 #define AFI_CONFIGURATION		0xac
 #define  AFI_CONFIGURATION_EN_FPCI		(1 << 0)
@@ -280,10 +266,10 @@
 #define LINK_RETRAIN_TIMEOUT 100000 /* in usec */
 
 struct tegra_msi {
-	struct msi_controller chip;
 	DECLARE_BITMAP(used, INT_PCI_MSI_NR);
 	struct irq_domain *domain;
-	struct mutex lock;
+	struct mutex map_lock;
+	spinlock_t mask_lock;
 	void *virt;
 	dma_addr_t phys;
 	int irq;
@@ -333,11 +319,6 @@ struct tegra_pcie_soc {
 	} ectl;
 };
 
-static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
-{
-	return container_of(chip, struct tegra_msi, chip);
-}
-
 struct tegra_pcie {
 	struct device *dev;
 
@@ -372,6 +353,11 @@ struct tegra_pcie {
 	struct dentry *debugfs;
 };
 
+static inline struct tegra_pcie *msi_to_pcie(struct tegra_msi *msi)
+{
+	return container_of(msi, struct tegra_pcie, msi);
+}
+
 struct tegra_pcie_port {
 	struct tegra_pcie *pcie;
 	struct device_node *np;
@@ -1432,7 +1418,6 @@ static void tegra_pcie_phys_put(struct tegra_pcie *pcie)
 	}
 }
 
-
 static int tegra_pcie_get_resources(struct tegra_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
@@ -1509,6 +1494,7 @@ static int tegra_pcie_get_resources(struct tegra_pcie *pcie)
 phys_put:
 	if (soc->program_uphy)
 		tegra_pcie_phys_put(pcie);
+
 	return err;
 }
 
@@ -1551,161 +1537,227 @@ static void tegra_pcie_pme_turnoff(struct tegra_pcie_port *port)
 	afi_writel(pcie, val, AFI_PCIE_PME);
 }
 
-static int tegra_msi_alloc(struct tegra_msi *chip)
-{
-	int msi;
-
-	mutex_lock(&chip->lock);
-
-	msi = find_first_zero_bit(chip->used, INT_PCI_MSI_NR);
-	if (msi < INT_PCI_MSI_NR)
-		set_bit(msi, chip->used);
-	else
-		msi = -ENOSPC;
-
-	mutex_unlock(&chip->lock);
-
-	return msi;
-}
-
-static void tegra_msi_free(struct tegra_msi *chip, unsigned long irq)
+static void tegra_pcie_msi_irq(struct irq_desc *desc)
 {
-	struct device *dev = chip->chip.dev;
-
-	mutex_lock(&chip->lock);
-
-	if (!test_bit(irq, chip->used))
-		dev_err(dev, "trying to free unused MSI#%lu\n", irq);
-	else
-		clear_bit(irq, chip->used);
-
-	mutex_unlock(&chip->lock);
-}
-
-static irqreturn_t tegra_pcie_msi_irq(int irq, void *data)
-{
-	struct tegra_pcie *pcie = data;
-	struct device *dev = pcie->dev;
+	struct tegra_pcie *pcie = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
 	struct tegra_msi *msi = &pcie->msi;
-	unsigned int i, processed = 0;
+	struct device *dev = pcie->dev;
+	unsigned int i;
+
+	chained_irq_enter(chip, desc);
 
 	for (i = 0; i < 8; i++) {
-		unsigned long reg = afi_readl(pcie, AFI_MSI_VEC0 + i * 4);
+		unsigned long reg = afi_readl(pcie, AFI_MSI_VEC(i));
 
 		while (reg) {
 			unsigned int offset = find_first_bit(&reg, 32);
 			unsigned int index = i * 32 + offset;
 			unsigned int irq;
 
-			/* clear the interrupt */
-			afi_writel(pcie, 1 << offset, AFI_MSI_VEC0 + i * 4);
-
-			irq = irq_find_mapping(msi->domain, index);
+			irq = irq_find_mapping(msi->domain->parent, index);
 			if (irq) {
-				if (test_bit(index, msi->used))
-					generic_handle_irq(irq);
-				else
-					dev_info(dev, "unhandled MSI\n");
+				generic_handle_irq(irq);
 			} else {
 				/*
 				 * that's weird who triggered this?
 				 * just clear it
 				 */
 				dev_info(dev, "unexpected MSI\n");
+				afi_writel(pcie, BIT(index % 32), AFI_MSI_VEC(index));
 			}
 
 			/* see if there's any more pending in this vector */
-			reg = afi_readl(pcie, AFI_MSI_VEC0 + i * 4);
-
-			processed++;
+			reg = afi_readl(pcie, AFI_MSI_VEC(i));
 		}
 	}
 
-	return processed > 0 ? IRQ_HANDLED : IRQ_NONE;
+	chained_irq_exit(chip, desc);
 }
 
-static int tegra_msi_setup_irq(struct msi_controller *chip,
-			       struct pci_dev *pdev, struct msi_desc *desc)
+static void tegra_msi_top_irq_ack(struct irq_data *d)
 {
-	struct tegra_msi *msi = to_tegra_msi(chip);
-	struct msi_msg msg;
-	unsigned int irq;
-	int hwirq;
+	irq_chip_ack_parent(d);
+}
 
-	hwirq = tegra_msi_alloc(msi);
-	if (hwirq < 0)
-		return hwirq;
+static void tegra_msi_top_irq_mask(struct irq_data *d)
+{
+	pci_msi_mask_irq(d);
+	irq_chip_mask_parent(d);
+}
 
-	irq = irq_create_mapping(msi->domain, hwirq);
-	if (!irq) {
-		tegra_msi_free(msi, hwirq);
-		return -EINVAL;
-	}
+static void tegra_msi_top_irq_unmask(struct irq_data *d)
+{
+	pci_msi_unmask_irq(d);
+	irq_chip_unmask_parent(d);
+}
+
+static struct irq_chip tegra_msi_top_chip = {
+	.name		= "tegra PCIe MSI",
+	.irq_ack	= tegra_msi_top_irq_ack,
+	.irq_mask	= tegra_msi_top_irq_mask,
+	.irq_unmask	= tegra_msi_top_irq_unmask,
+};
 
-	irq_set_msi_desc(irq, desc);
+static void tegra_msi_irq_ack(struct irq_data *d)
+{
+	struct tegra_msi *msi = irq_data_get_irq_chip_data(d);
+	struct tegra_pcie *pcie = msi_to_pcie(msi);
+	unsigned int index = d->hwirq / 32;
 
-	msg.address_lo = lower_32_bits(msi->phys);
-	msg.address_hi = upper_32_bits(msi->phys);
-	msg.data = hwirq;
+	/* clear the interrupt */
+	afi_writel(pcie, BIT(d->hwirq % 32), AFI_MSI_VEC(index));
+}
 
-	pci_write_msi_msg(irq, &msg);
+static void tegra_msi_irq_mask(struct irq_data *d)
+{
+	struct tegra_msi *msi = irq_data_get_irq_chip_data(d);
+	struct tegra_pcie *pcie = msi_to_pcie(msi);
+	unsigned int index = d->hwirq / 32;
+	unsigned long flags;
+	u32 value;
 
-	return 0;
+	spin_lock_irqsave(&msi->mask_lock, flags);
+	value = afi_readl(pcie, AFI_MSI_EN_VEC(index));
+	value &= ~BIT(d->hwirq % 32);
+	afi_writel(pcie, value, AFI_MSI_EN_VEC(index));
+	spin_unlock_irqrestore(&msi->mask_lock, flags);
+}
+
+static void tegra_msi_irq_unmask(struct irq_data *d)
+{
+	struct tegra_msi *msi = irq_data_get_irq_chip_data(d);
+	struct tegra_pcie *pcie = msi_to_pcie(msi);
+	unsigned int index = d->hwirq / 32;
+	unsigned long flags;
+	u32 value;
+
+	spin_lock_irqsave(&msi->mask_lock, flags);
+	value = afi_readl(pcie, AFI_MSI_EN_VEC(index));
+	value |= BIT(d->hwirq % 32);
+	afi_writel(pcie, value, AFI_MSI_EN_VEC(index));
+	spin_unlock_irqrestore(&msi->mask_lock, flags);
+}
+
+static int tegra_msi_set_affinity(struct irq_data *d, const struct cpumask *mask, bool force)
+{
+	return -EINVAL;
 }
 
-static void tegra_msi_teardown_irq(struct msi_controller *chip,
-				   unsigned int irq)
+static void tegra_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 {
-	struct tegra_msi *msi = to_tegra_msi(chip);
-	struct irq_data *d = irq_get_irq_data(irq);
-	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	struct tegra_msi *msi = irq_data_get_irq_chip_data(data);
 
-	irq_dispose_mapping(irq);
-	tegra_msi_free(msi, hwirq);
+	msg->address_lo = lower_32_bits(msi->phys);
+	msg->address_hi = upper_32_bits(msi->phys);
+	msg->data = data->hwirq;
 }
 
-static struct irq_chip tegra_msi_irq_chip = {
-	.name = "Tegra PCIe MSI",
-	.irq_enable = pci_msi_unmask_irq,
-	.irq_disable = pci_msi_mask_irq,
-	.irq_mask = pci_msi_mask_irq,
-	.irq_unmask = pci_msi_unmask_irq,
+static struct irq_chip tegra_msi_bottom_chip = {
+	.name			= "Tegra MSI",
+	.irq_ack		= tegra_msi_irq_ack,
+	.irq_mask		= tegra_msi_irq_mask,
+	.irq_unmask		= tegra_msi_irq_unmask,
+	.irq_set_affinity 	= tegra_msi_set_affinity,
+	.irq_compose_msi_msg	= tegra_compose_msi_msg,
 };
 
-static int tegra_msi_map(struct irq_domain *domain, unsigned int irq,
-			 irq_hw_number_t hwirq)
+static int tegra_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				  unsigned int nr_irqs, void *args)
 {
-	irq_set_chip_and_handler(irq, &tegra_msi_irq_chip, handle_simple_irq);
-	irq_set_chip_data(irq, domain->host_data);
+	struct tegra_msi *msi = domain->host_data;
+	unsigned int i;
+	int hwirq;
+
+	mutex_lock(&msi->map_lock);
+
+	hwirq = bitmap_find_free_region(msi->used, INT_PCI_MSI_NR, order_base_2(nr_irqs));
+
+	mutex_unlock(&msi->map_lock);
+
+	if (hwirq < 0)
+		return -ENOSPC;
+
+	for (i = 0; i < nr_irqs; i++)
+		irq_domain_set_info(domain, virq + i, hwirq + i,
+				    &tegra_msi_bottom_chip, domain->host_data,
+				    handle_edge_irq, NULL, NULL);
 
 	tegra_cpuidle_pcie_irqs_in_use();
 
 	return 0;
 }
 
-static const struct irq_domain_ops msi_domain_ops = {
-	.map = tegra_msi_map,
+static void tegra_msi_domain_free(struct irq_domain *domain, unsigned int virq,
+				  unsigned int nr_irqs)
+{
+	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
+	struct tegra_msi *msi = domain->host_data;
+
+	mutex_lock(&msi->map_lock);
+
+	bitmap_release_region(msi->used, d->hwirq, order_base_2(nr_irqs));
+
+	mutex_unlock(&msi->map_lock);
+}
+
+static const struct irq_domain_ops tegra_msi_domain_ops = {
+	.alloc = tegra_msi_domain_alloc,
+	.free = tegra_msi_domain_free,
+};
+
+static struct msi_domain_info tegra_msi_info = {
+	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+		   MSI_FLAG_PCI_MSIX),
+	.chip	= &tegra_msi_top_chip,
 };
 
+static int tegra_allocate_domains(struct tegra_msi *msi)
+{
+	struct tegra_pcie *pcie = msi_to_pcie(msi);
+	struct fwnode_handle *fwnode = dev_fwnode(pcie->dev);
+	struct irq_domain *parent;
+
+	parent = irq_domain_create_linear(fwnode, INT_PCI_MSI_NR,
+					  &tegra_msi_domain_ops, msi);
+	if (!parent) {
+		dev_err(pcie->dev, "failed to create IRQ domain\n");
+		return -ENOMEM;
+	}
+	irq_domain_update_bus_token(parent, DOMAIN_BUS_NEXUS);
+
+	msi->domain = pci_msi_create_irq_domain(fwnode, &tegra_msi_info, parent);
+	if (!msi->domain) {
+		dev_err(pcie->dev, "failed to create MSI domain\n");
+		irq_domain_remove(parent);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static void tegra_free_domains(struct tegra_msi *msi)
+{
+	struct irq_domain *parent = msi->domain->parent;
+
+	irq_domain_remove(msi->domain);
+	irq_domain_remove(parent);
+}
+
 static int tegra_pcie_msi_setup(struct tegra_pcie *pcie)
 {
-	struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
 	struct platform_device *pdev = to_platform_device(pcie->dev);
 	struct tegra_msi *msi = &pcie->msi;
 	struct device *dev = pcie->dev;
 	int err;
 
-	mutex_init(&msi->lock);
-
-	msi->chip.dev = dev;
-	msi->chip.setup_irq = tegra_msi_setup_irq;
-	msi->chip.teardown_irq = tegra_msi_teardown_irq;
+	mutex_init(&msi->map_lock);
+	spin_lock_init(&msi->mask_lock);
 
-	msi->domain = irq_domain_add_linear(dev->of_node, INT_PCI_MSI_NR,
-					    &msi_domain_ops, &msi->chip);
-	if (!msi->domain) {
-		dev_err(dev, "failed to create IRQ domain\n");
-		return -ENOMEM;
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		err = tegra_allocate_domains(msi);
+		if (err)
+			return err;
 	}
 
 	err = platform_get_irq_byname(pdev, "msi");
@@ -1714,12 +1766,7 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie)
 
 	msi->irq = err;
 
-	err = request_irq(msi->irq, tegra_pcie_msi_irq, IRQF_NO_THREAD,
-			  tegra_msi_irq_chip.name, pcie);
-	if (err < 0) {
-		dev_err(dev, "failed to request IRQ: %d\n", err);
-		goto free_irq_domain;
-	}
+	irq_set_chained_handler_and_data(msi->irq, tegra_pcie_msi_irq, pcie);
 
 	/* Though the PCIe controller can address >32-bit address space, to
 	 * facilitate endpoints that support only 32-bit MSI target address,
@@ -1740,14 +1787,14 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie)
 		goto free_irq;
 	}
 
-	host->msi = &msi->chip;
-
 	return 0;
 
 free_irq:
-	free_irq(msi->irq, pcie);
+	irq_set_chained_handler_and_data(msi->irq, NULL, NULL);
 free_irq_domain:
-	irq_domain_remove(msi->domain);
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		tegra_free_domains(msi);
+
 	return err;
 }
 
@@ -1762,16 +1809,6 @@ static void tegra_pcie_enable_msi(struct tegra_pcie *pcie)
 	/* this register is in 4K increments */
 	afi_writel(pcie, 1, AFI_MSI_BAR_SZ);
 
-	/* enable all MSI vectors */
-	afi_writel(pcie, 0xffffffff, AFI_MSI_EN_VEC0);
-	afi_writel(pcie, 0xffffffff, AFI_MSI_EN_VEC1);
-	afi_writel(pcie, 0xffffffff, AFI_MSI_EN_VEC2);
-	afi_writel(pcie, 0xffffffff, AFI_MSI_EN_VEC3);
-	afi_writel(pcie, 0xffffffff, AFI_MSI_EN_VEC4);
-	afi_writel(pcie, 0xffffffff, AFI_MSI_EN_VEC5);
-	afi_writel(pcie, 0xffffffff, AFI_MSI_EN_VEC6);
-	afi_writel(pcie, 0xffffffff, AFI_MSI_EN_VEC7);
-
 	/* and unmask the MSI interrupt */
 	reg = afi_readl(pcie, AFI_INTR_MASK);
 	reg |= AFI_INTR_MASK_MSI_MASK;
@@ -1786,16 +1823,16 @@ static void tegra_pcie_msi_teardown(struct tegra_pcie *pcie)
 	dma_free_attrs(pcie->dev, PAGE_SIZE, msi->virt, msi->phys,
 		       DMA_ATTR_NO_KERNEL_MAPPING);
 
-	if (msi->irq > 0)
-		free_irq(msi->irq, pcie);
-
 	for (i = 0; i < INT_PCI_MSI_NR; i++) {
 		irq = irq_find_mapping(msi->domain, i);
 		if (irq > 0)
-			irq_dispose_mapping(irq);
+			irq_domain_free_irqs(irq, 1);
 	}
 
-	irq_domain_remove(msi->domain);
+	irq_set_chained_handler_and_data(msi->irq, NULL, NULL);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		tegra_free_domains(msi);
 }
 
 static int tegra_pcie_disable_msi(struct tegra_pcie *pcie)
@@ -1807,16 +1844,6 @@ static int tegra_pcie_disable_msi(struct tegra_pcie *pcie)
 	value &= ~AFI_INTR_MASK_MSI_MASK;
 	afi_writel(pcie, value, AFI_INTR_MASK);
 
-	/* disable all MSI vectors */
-	afi_writel(pcie, 0, AFI_MSI_EN_VEC0);
-	afi_writel(pcie, 0, AFI_MSI_EN_VEC1);
-	afi_writel(pcie, 0, AFI_MSI_EN_VEC2);
-	afi_writel(pcie, 0, AFI_MSI_EN_VEC3);
-	afi_writel(pcie, 0, AFI_MSI_EN_VEC4);
-	afi_writel(pcie, 0, AFI_MSI_EN_VEC5);
-	afi_writel(pcie, 0, AFI_MSI_EN_VEC6);
-	afi_writel(pcie, 0, AFI_MSI_EN_VEC7);
-
 	return 0;
 }
 
-- 
2.29.2

[PATCH 02/13] PCI: rcar: Convert to MSI domains

[Marc Zyngier]

In anticipation of the removal of the msi_controller structure, convert
the Rcar host controller driver to MSI domains.

We end-up with the usual two domain structure, the top one being a
generic PCI/MSI domain, the bottom one being Rcar-specific and handling
the actual HW interrupt allocation.

Also take the opportunity to get rid of the cargo-culted memory allocation
for the MSI capture address. *ANY* sufficiently aligned address should
be good enough, so use the physical address of the msi structure instead.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/controller/Kconfig          |   1 -
 drivers/pci/controller/pcie-rcar-host.c | 342 +++++++++++-------------
 2 files changed, 156 insertions(+), 187 deletions(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index e7007e4028fc..ccbf034512d6 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -67,7 +67,6 @@ config PCIE_RCAR_HOST
 	bool "Renesas R-Car PCIe host controller"
 	depends on ARCH_RENESAS || COMPILE_TEST
 	depends on PCI_MSI_IRQ_DOMAIN
-	select PCI_MSI_ARCH_FALLBACKS
 	help
 	  Say Y here if you want PCIe controller support on R-Car SoCs in host
 	  mode.
diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c
index 4d1c4b24e537..2526cd487a39 100644
--- a/drivers/pci/controller/pcie-rcar-host.c
+++ b/drivers/pci/controller/pcie-rcar-host.c
@@ -35,18 +35,12 @@
 struct rcar_msi {
 	DECLARE_BITMAP(used, INT_PCI_MSI_NR);
 	struct irq_domain *domain;
-	struct msi_controller chip;
-	unsigned long pages;
-	struct mutex lock;
+	struct mutex map_lock;
+	spinlock_t mask_lock;
 	int irq1;
 	int irq2;
 };
 
-static inline struct rcar_msi *to_rcar_msi(struct msi_controller *chip)
-{
-	return container_of(chip, struct rcar_msi, chip);
-}
-
 /* Structure representing the PCIe interface */
 struct rcar_pcie_host {
 	struct rcar_pcie	pcie;
@@ -56,6 +50,11 @@ struct rcar_pcie_host {
 	int			(*phy_init_fn)(struct rcar_pcie_host *host);
 };
 
+static struct rcar_pcie_host *msi_to_host(struct rcar_msi *msi)
+{
+	return container_of(msi, struct rcar_pcie_host, msi);
+}
+
 static u32 rcar_read_conf(struct rcar_pcie *pcie, int where)
 {
 	unsigned int shift = BITS_PER_BYTE * (where & 3);
@@ -292,8 +291,6 @@ static int rcar_pcie_enable(struct rcar_pcie_host *host)
 
 	bridge->sysdata = host;
 	bridge->ops = &rcar_pcie_ops;
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		bridge->msi = &host->msi.chip;
 
 	return pci_host_probe(bridge);
 }
@@ -473,42 +470,6 @@ static int rcar_pcie_phy_init_gen3(struct rcar_pcie_host *host)
 	return err;
 }
 
-static int rcar_msi_alloc(struct rcar_msi *chip)
-{
-	int msi;
-
-	mutex_lock(&chip->lock);
-
-	msi = find_first_zero_bit(chip->used, INT_PCI_MSI_NR);
-	if (msi < INT_PCI_MSI_NR)
-		set_bit(msi, chip->used);
-	else
-		msi = -ENOSPC;
-
-	mutex_unlock(&chip->lock);
-
-	return msi;
-}
-
-static int rcar_msi_alloc_region(struct rcar_msi *chip, int no_irqs)
-{
-	int msi;
-
-	mutex_lock(&chip->lock);
-	msi = bitmap_find_free_region(chip->used, INT_PCI_MSI_NR,
-				      order_base_2(no_irqs));
-	mutex_unlock(&chip->lock);
-
-	return msi;
-}
-
-static void rcar_msi_free(struct rcar_msi *chip, unsigned long irq)
-{
-	mutex_lock(&chip->lock);
-	clear_bit(irq, chip->used);
-	mutex_unlock(&chip->lock);
-}
-
 static irqreturn_t rcar_pcie_msi_irq(int irq, void *data)
 {
 	struct rcar_pcie_host *host = data;
@@ -527,18 +488,13 @@ static irqreturn_t rcar_pcie_msi_irq(int irq, void *data)
 		unsigned int index = find_first_bit(&reg, 32);
 		unsigned int msi_irq;
 
-		/* clear the interrupt */
-		rcar_pci_write_reg(pcie, 1 << index, PCIEMSIFR);
-
-		msi_irq = irq_find_mapping(msi->domain, index);
+		msi_irq = irq_find_mapping(msi->domain->parent, index);
 		if (msi_irq) {
-			if (test_bit(index, msi->used))
-				generic_handle_irq(msi_irq);
-			else
-				dev_info(dev, "unhandled MSI\n");
+			generic_handle_irq(msi_irq);
 		} else {
 			/* Unknown MSI, just clear it */
 			dev_dbg(dev, "unexpected MSI\n");
+			rcar_pci_write_reg(pcie, BIT(index), PCIEMSIFR);
 		}
 
 		/* see if there's any more pending in this vector */
@@ -548,149 +504,170 @@ static irqreturn_t rcar_pcie_msi_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static int rcar_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
-			      struct msi_desc *desc)
+static void rcar_msi_top_irq_ack(struct irq_data *d)
 {
-	struct rcar_msi *msi = to_rcar_msi(chip);
-	struct rcar_pcie_host *host = container_of(chip, struct rcar_pcie_host,
-						   msi.chip);
-	struct rcar_pcie *pcie = &host->pcie;
-	struct msi_msg msg;
-	unsigned int irq;
-	int hwirq;
+	irq_chip_ack_parent(d);
+}
 
-	hwirq = rcar_msi_alloc(msi);
-	if (hwirq < 0)
-		return hwirq;
+static void rcar_msi_top_irq_mask(struct irq_data *d)
+{
+	pci_msi_mask_irq(d);
+	irq_chip_mask_parent(d);
+}
 
-	irq = irq_find_mapping(msi->domain, hwirq);
-	if (!irq) {
-		rcar_msi_free(msi, hwirq);
-		return -EINVAL;
-	}
+static void rcar_msi_top_irq_unmask(struct irq_data *d)
+{
+	pci_msi_unmask_irq(d);
+	irq_chip_unmask_parent(d);
+}
 
-	irq_set_msi_desc(irq, desc);
+static struct irq_chip rcar_msi_top_chip = {
+	.name		= "PCIe MSI",
+	.irq_ack	= rcar_msi_top_irq_ack,
+	.irq_mask	= rcar_msi_top_irq_mask,
+	.irq_unmask	= rcar_msi_top_irq_unmask,
+};
+
+static void rcar_msi_irq_ack(struct irq_data *d)
+{
+	struct rcar_msi *msi = irq_data_get_irq_chip_data(d);
+	struct rcar_pcie *pcie = &msi_to_host(msi)->pcie;
 
-	msg.address_lo = rcar_pci_read_reg(pcie, PCIEMSIALR) & ~MSIFE;
-	msg.address_hi = rcar_pci_read_reg(pcie, PCIEMSIAUR);
-	msg.data = hwirq;
+	/* clear the interrupt */
+	rcar_pci_write_reg(pcie, BIT(d->hwirq), PCIEMSIFR);
+}
 
-	pci_write_msi_msg(irq, &msg);
+static void rcar_msi_irq_mask(struct irq_data *d)
+{
+	struct rcar_msi *msi = irq_data_get_irq_chip_data(d);
+	struct rcar_pcie *pcie = &msi_to_host(msi)->pcie;
+	unsigned long flags;
+	u32 value;
 
-	return 0;
+	spin_lock_irqsave(&msi->mask_lock, flags);
+	value = rcar_pci_read_reg(pcie, PCIEMSIIER);
+	value &= ~BIT(d->hwirq);
+	rcar_pci_write_reg(pcie, value, PCIEMSIIER);
+	spin_unlock_irqrestore(&msi->mask_lock, flags);
 }
 
-static int rcar_msi_setup_irqs(struct msi_controller *chip,
-			       struct pci_dev *pdev, int nvec, int type)
+static void rcar_msi_irq_unmask(struct irq_data *d)
 {
-	struct rcar_msi *msi = to_rcar_msi(chip);
-	struct rcar_pcie_host *host = container_of(chip, struct rcar_pcie_host,
-						   msi.chip);
-	struct rcar_pcie *pcie = &host->pcie;
-	struct msi_desc *desc;
-	struct msi_msg msg;
-	unsigned int irq;
-	int hwirq;
-	int i;
+	struct rcar_msi *msi = irq_data_get_irq_chip_data(d);
+	struct rcar_pcie *pcie = &msi_to_host(msi)->pcie;
+	unsigned long flags;
+	u32 value;
 
-	/* MSI-X interrupts are not supported */
-	if (type == PCI_CAP_ID_MSIX)
-		return -EINVAL;
+	spin_lock_irqsave(&msi->mask_lock, flags);
+	value = rcar_pci_read_reg(pcie, PCIEMSIIER);
+	value |= BIT(d->hwirq);
+	rcar_pci_write_reg(pcie, value, PCIEMSIIER);
+	spin_unlock_irqrestore(&msi->mask_lock, flags);
+}
 
-	WARN_ON(!list_is_singular(&pdev->dev.msi_list));
-	desc = list_entry(pdev->dev.msi_list.next, struct msi_desc, list);
+static int rcar_msi_set_affinity(struct irq_data *d, const struct cpumask *mask, bool force)
+{
+	return -EINVAL;
+}
 
-	hwirq = rcar_msi_alloc_region(msi, nvec);
-	if (hwirq < 0)
-		return -ENOSPC;
+static void rcar_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
+{
+	struct rcar_msi *msi = irq_data_get_irq_chip_data(data);
+	unsigned long pa = virt_to_phys(msi);
 
-	irq = irq_find_mapping(msi->domain, hwirq);
-	if (!irq)
-		return -ENOSPC;
+	/* Use the msi structure as the PA for the MSI doorbell */
+	msg->address_lo = lower_32_bits(pa);
+	msg->address_hi = upper_32_bits(pa);
+	msg->data = data->hwirq;
+}
 
-	for (i = 0; i < nvec; i++) {
-		/*
-		 * irq_create_mapping() called from rcar_pcie_probe() pre-
-		 * allocates descs,  so there is no need to allocate descs here.
-		 * We can therefore assume that if irq_find_mapping() above
-		 * returns non-zero, then the descs are also successfully
-		 * allocated.
-		 */
-		if (irq_set_msi_desc_off(irq, i, desc)) {
-			/* TODO: clear */
-			return -EINVAL;
-		}
-	}
+static struct irq_chip rcar_msi_bottom_chip = {
+	.name			= "Rcar MSI",
+	.irq_ack		= rcar_msi_irq_ack,
+	.irq_mask		= rcar_msi_irq_mask,
+	.irq_unmask		= rcar_msi_irq_unmask,
+	.irq_set_affinity 	= rcar_msi_set_affinity,
+	.irq_compose_msi_msg	= rcar_compose_msi_msg,
+};
+
+static int rcar_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				  unsigned int nr_irqs, void *args)
+{
+	struct rcar_msi *msi = domain->host_data;
+	unsigned int i;
+	int hwirq;
 
-	desc->nvec_used = nvec;
-	desc->msi_attrib.multiple = order_base_2(nvec);
+	mutex_lock(&msi->map_lock);
 
-	msg.address_lo = rcar_pci_read_reg(pcie, PCIEMSIALR) & ~MSIFE;
-	msg.address_hi = rcar_pci_read_reg(pcie, PCIEMSIAUR);
-	msg.data = hwirq;
+	hwirq = bitmap_find_free_region(msi->used, INT_PCI_MSI_NR, order_base_2(nr_irqs));
 
-	pci_write_msi_msg(irq, &msg);
+	mutex_unlock(&msi->map_lock);
+
+	if (hwirq < 0)
+		return -ENOSPC;
+
+	for (i = 0; i < nr_irqs; i++)
+		irq_domain_set_info(domain, virq + i, hwirq + i,
+				    &rcar_msi_bottom_chip, domain->host_data,
+				    handle_edge_irq, NULL, NULL);
 
 	return 0;
 }
 
-static void rcar_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
+static void rcar_msi_domain_free(struct irq_domain *domain, unsigned int virq,
+				  unsigned int nr_irqs)
 {
-	struct rcar_msi *msi = to_rcar_msi(chip);
-	struct irq_data *d = irq_get_irq_data(irq);
-
-	rcar_msi_free(msi, d->hwirq);
-}
+	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
+	struct rcar_msi *msi = domain->host_data;
 
-static struct irq_chip rcar_msi_irq_chip = {
-	.name = "R-Car PCIe MSI",
-	.irq_enable = pci_msi_unmask_irq,
-	.irq_disable = pci_msi_mask_irq,
-	.irq_mask = pci_msi_mask_irq,
-	.irq_unmask = pci_msi_unmask_irq,
-};
+	mutex_lock(&msi->map_lock);
 
-static int rcar_msi_map(struct irq_domain *domain, unsigned int irq,
-			irq_hw_number_t hwirq)
-{
-	irq_set_chip_and_handler(irq, &rcar_msi_irq_chip, handle_simple_irq);
-	irq_set_chip_data(irq, domain->host_data);
+	bitmap_release_region(msi->used, d->hwirq, order_base_2(nr_irqs));
 
-	return 0;
+	mutex_unlock(&msi->map_lock);
 }
 
-static const struct irq_domain_ops msi_domain_ops = {
-	.map = rcar_msi_map,
+static const struct irq_domain_ops rcar_msi_domain_ops = {
+	.alloc	= rcar_msi_domain_alloc,
+	.free	= rcar_msi_domain_free,
 };
 
-static void rcar_pcie_unmap_msi(struct rcar_pcie_host *host)
+static struct msi_domain_info rcar_msi_info = {
+	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+		   MSI_FLAG_MULTI_PCI_MSI),
+	.chip	= &rcar_msi_top_chip,
+};
+
+static int rcar_allocate_domains(struct rcar_msi *msi)
 {
-	struct rcar_msi *msi = &host->msi;
-	int i, irq;
+	struct rcar_pcie *pcie = &msi_to_host(msi)->pcie;
+	struct fwnode_handle *fwnode = dev_fwnode(pcie->dev);
+	struct irq_domain *parent;
 
-	for (i = 0; i < INT_PCI_MSI_NR; i++) {
-		irq = irq_find_mapping(msi->domain, i);
-		if (irq > 0)
-			irq_dispose_mapping(irq);
+	parent = irq_domain_create_linear(fwnode, INT_PCI_MSI_NR,
+					  &rcar_msi_domain_ops, msi);
+	if (!parent) {
+		dev_err(pcie->dev, "failed to create IRQ domain\n");
+		return -ENOMEM;
 	}
+	irq_domain_update_bus_token(parent, DOMAIN_BUS_NEXUS);
 
-	irq_domain_remove(msi->domain);
+	msi->domain = pci_msi_create_irq_domain(fwnode, &rcar_msi_info, parent);
+	if (!msi->domain) {
+		dev_err(pcie->dev, "failed to create MSI domain\n");
+		irq_domain_remove(parent);
+		return -ENOMEM;
+	}
+
+	return 0;
 }
 
-static void rcar_pcie_hw_enable_msi(struct rcar_pcie_host *host)
+static void rcar_free_domains(struct rcar_msi *msi)
 {
-	struct rcar_pcie *pcie = &host->pcie;
-	struct rcar_msi *msi = &host->msi;
-	unsigned long base;
-
-	/* setup MSI data target */
-	base = virt_to_phys((void *)msi->pages);
-
-	rcar_pci_write_reg(pcie, lower_32_bits(base) | MSIFE, PCIEMSIALR);
-	rcar_pci_write_reg(pcie, upper_32_bits(base), PCIEMSIAUR);
+	struct irq_domain *parent = msi->domain->parent;
 
-	/* enable all MSI interrupts */
-	rcar_pci_write_reg(pcie, 0xffffffff, PCIEMSIIER);
+	irq_domain_remove(msi->domain);
+	irq_domain_remove(parent);
 }
 
 static int rcar_pcie_enable_msi(struct rcar_pcie_host *host)
@@ -698,29 +675,20 @@ static int rcar_pcie_enable_msi(struct rcar_pcie_host *host)
 	struct rcar_pcie *pcie = &host->pcie;
 	struct device *dev = pcie->dev;
 	struct rcar_msi *msi = &host->msi;
-	int err, i;
-
-	mutex_init(&msi->lock);
+	unsigned long base;
+	int err;
 
-	msi->chip.dev = dev;
-	msi->chip.setup_irq = rcar_msi_setup_irq;
-	msi->chip.setup_irqs = rcar_msi_setup_irqs;
-	msi->chip.teardown_irq = rcar_msi_teardown_irq;
+	mutex_init(&msi->map_lock);
+	spin_lock_init(&msi->mask_lock);
 
-	msi->domain = irq_domain_add_linear(dev->of_node, INT_PCI_MSI_NR,
-					    &msi_domain_ops, &msi->chip);
-	if (!msi->domain) {
-		dev_err(dev, "failed to create IRQ domain\n");
-		return -ENOMEM;
-	}
-
-	for (i = 0; i < INT_PCI_MSI_NR; i++)
-		irq_create_mapping(msi->domain, i);
+	err = rcar_allocate_domains(msi);
+	if (err)
+		return err;
 
 	/* Two irqs are for MSI, but they are also used for non-MSI irqs */
 	err = devm_request_irq(dev, msi->irq1, rcar_pcie_msi_irq,
 			       IRQF_SHARED | IRQF_NO_THREAD,
-			       rcar_msi_irq_chip.name, host);
+			       rcar_msi_bottom_chip.name, host);
 	if (err < 0) {
 		dev_err(dev, "failed to request IRQ: %d\n", err);
 		goto err;
@@ -728,27 +696,31 @@ static int rcar_pcie_enable_msi(struct rcar_pcie_host *host)
 
 	err = devm_request_irq(dev, msi->irq2, rcar_pcie_msi_irq,
 			       IRQF_SHARED | IRQF_NO_THREAD,
-			       rcar_msi_irq_chip.name, host);
+			       rcar_msi_bottom_chip.name, host);
 	if (err < 0) {
 		dev_err(dev, "failed to request IRQ: %d\n", err);
 		goto err;
 	}
 
-	/* setup MSI data target */
-	msi->pages = __get_free_pages(GFP_KERNEL, 0);
-	rcar_pcie_hw_enable_msi(host);
+	/* disable all MSIs */
+	rcar_pci_write_reg(pcie, 0, PCIEMSIIER);
+
+	/* setup MSI data target using the msi structure address */
+	base = virt_to_phys(&host->msi);
+
+	rcar_pci_write_reg(pcie, lower_32_bits(base) | MSIFE, PCIEMSIALR);
+	rcar_pci_write_reg(pcie, upper_32_bits(base), PCIEMSIAUR);
 
 	return 0;
 
 err:
-	rcar_pcie_unmap_msi(host);
+	rcar_free_domains(msi);
 	return err;
 }
 
 static void rcar_pcie_teardown_msi(struct rcar_pcie_host *host)
 {
 	struct rcar_pcie *pcie = &host->pcie;
-	struct rcar_msi *msi = &host->msi;
 
 	/* Disable all MSI interrupts */
 	rcar_pci_write_reg(pcie, 0, PCIEMSIIER);
@@ -756,9 +728,7 @@ static void rcar_pcie_teardown_msi(struct rcar_pcie_host *host)
 	/* Disable address decoding of the MSI interrupt, MSIFE */
 	rcar_pci_write_reg(pcie, 0, PCIEMSIALR);
 
-	free_pages(msi->pages, 0);
-
-	rcar_pcie_unmap_msi(host);
+	rcar_free_domains(&host->msi);
 }
 
 static int rcar_pcie_get_resources(struct rcar_pcie_host *host)
@@ -1012,7 +982,7 @@ static int __maybe_unused rcar_pcie_resume(struct device *dev)
 
 	/* Enable MSI */
 	if (IS_ENABLED(CONFIG_PCI_MSI))
-		rcar_pcie_hw_enable_msi(host);
+		rcar_pcie_enable_msi(host);
 
 	rcar_pcie_hw_enable(host);
 
-- 
2.29.2

Re: [PATCH 02/13] PCI: rcar: Convert to MSI domains

[Marek Vasut]

On 2/25/21 4:10 PM, Marc Zyngier wrote:
> In anticipation of the removal of the msi_controller structure, convert
> the Rcar host controller driver to MSI domains.
> 
> We end-up with the usual two domain structure, the top one being a
> generic PCI/MSI domain, the bottom one being Rcar-specific and handling
> the actual HW interrupt allocation.
> 
> Also take the opportunity to get rid of the cargo-culted memory allocation
> for the MSI capture address. *ANY* sufficiently aligned address should
> be good enough, so use the physical address of the msi structure instead.

On R8A7795 with Intel 600p NVMe SSD and IWLwifi 6235 card,
Tested-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Thanks.

[PATCH 03/13] PCI: xilinx: Convert to MSI domains

[Marc Zyngier]

In anticipation of the removal of the msi_controller structure, convert
the ancient xilinx host controller driver to MSI domains.

We end-up with the usual two domain structure, the top one being a
generic PCI/MSI domain, the bottom one being xilinx-specific and handling
the actual HW interrupt allocation.

This allows us to fix some of the most appaling MSI programming, where
the message programmed in the device is the virtual IRQ number instead
of the allocated vector number. The allocator is also made safe with
a mutex. This should allow support for MultiMSI, but I decided not to
even try, since I cannot test it.

Also take the opportunity to get rid of the cargo-culted memory allocation
for the MSI capture address. *ANY* sufficiently aligned address should
be good enough, so use the physical address of the xilinx_pcie_host
structure instead.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/controller/Kconfig       |   2 +-
 drivers/pci/controller/pcie-xilinx.c | 238 +++++++++++----------------
 2 files changed, 96 insertions(+), 144 deletions(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index ccbf034512d6..049c60016904 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -95,7 +95,7 @@ config PCI_HOST_GENERIC
 config PCIE_XILINX
 	bool "Xilinx AXI PCIe host bridge support"
 	depends on OF || COMPILE_TEST
-	select PCI_MSI_ARCH_FALLBACKS
+	depends on PCI_MSI_IRQ_DOMAIN
 	help
 	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
 	  Host Bridge driver.
diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
index fa5baeb82653..ad9abf405167 100644
--- a/drivers/pci/controller/pcie-xilinx.c
+++ b/drivers/pci/controller/pcie-xilinx.c
@@ -93,25 +93,23 @@
 /**
  * struct xilinx_pcie_port - PCIe port information
  * @reg_base: IO Mapped Register Base
- * @irq: Interrupt number
- * @msi_pages: MSI pages
  * @dev: Device pointer
+ * @msi_map: Bitmap of allocated MSIs
+ * @map_lock: Mutex protecting the MSI allocation
  * @msi_domain: MSI IRQ domain pointer
  * @leg_domain: Legacy IRQ domain pointer
  * @resources: Bus Resources
  */
 struct xilinx_pcie_port {
 	void __iomem *reg_base;
-	u32 irq;
-	unsigned long msi_pages;
 	struct device *dev;
+	unsigned long msi_map[BITS_TO_LONGS(XILINX_NUM_MSI_IRQS)];
+	struct mutex map_lock;
 	struct irq_domain *msi_domain;
 	struct irq_domain *leg_domain;
 	struct list_head resources;
 };
 
-static DECLARE_BITMAP(msi_irq_in_use, XILINX_NUM_MSI_IRQS);
-
 static inline u32 pcie_read(struct xilinx_pcie_port *port, u32 reg)
 {
 	return readl(port->reg_base + reg);
@@ -196,151 +194,108 @@ static struct pci_ops xilinx_pcie_ops = {
 
 /* MSI functions */
 
-/**
- * xilinx_pcie_destroy_msi - Free MSI number
- * @irq: IRQ to be freed
- */
-static void xilinx_pcie_destroy_msi(unsigned int irq)
+static struct irq_chip xilinx_msi_top_chip = {
+	.name		= "PCIe MSI",
+};
+
+static int xilinx_msi_set_affinity(struct irq_data *d, const struct cpumask *mask, bool force)
 {
-	struct msi_desc *msi;
-	struct xilinx_pcie_port *port;
-	struct irq_data *d = irq_get_irq_data(irq);
-	irq_hw_number_t hwirq = irqd_to_hwirq(d);
-
-	if (!test_bit(hwirq, msi_irq_in_use)) {
-		msi = irq_get_msi_desc(irq);
-		port = msi_desc_to_pci_sysdata(msi);
-		dev_err(port->dev, "Trying to free unused MSI#%d\n", irq);
-	} else {
-		clear_bit(hwirq, msi_irq_in_use);
-	}
+	return -EINVAL;
 }
 
-/**
- * xilinx_pcie_assign_msi - Allocate MSI number
- *
- * Return: A valid IRQ on success and error value on failure.
- */
-static int xilinx_pcie_assign_msi(void)
+static void xilinx_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 {
-	int pos;
+	struct xilinx_pcie_port *pcie = irq_data_get_irq_chip_data(data);
+	phys_addr_t pa = virt_to_phys(pcie);
 
-	pos = find_first_zero_bit(msi_irq_in_use, XILINX_NUM_MSI_IRQS);
-	if (pos < XILINX_NUM_MSI_IRQS)
-		set_bit(pos, msi_irq_in_use);
-	else
-		return -ENOSPC;
-
-	return pos;
+	msg->address_lo = lower_32_bits(pa);
+	msg->address_hi = upper_32_bits(pa);
+	msg->data = data->hwirq;
 }
 
-/**
- * xilinx_msi_teardown_irq - Destroy the MSI
- * @chip: MSI Chip descriptor
- * @irq: MSI IRQ to destroy
- */
-static void xilinx_msi_teardown_irq(struct msi_controller *chip,
-				    unsigned int irq)
-{
-	xilinx_pcie_destroy_msi(irq);
-	irq_dispose_mapping(irq);
-}
+static struct irq_chip xilinx_msi_bottom_chip = {
+	.name			= "Xilinx MSI",
+	.irq_set_affinity 	= xilinx_msi_set_affinity,
+	.irq_compose_msi_msg	= xilinx_compose_msi_msg,
+};
 
-/**
- * xilinx_pcie_msi_setup_irq - Setup MSI request
- * @chip: MSI chip pointer
- * @pdev: PCIe device pointer
- * @desc: MSI descriptor pointer
- *
- * Return: '0' on success and error value on failure
- */
-static int xilinx_pcie_msi_setup_irq(struct msi_controller *chip,
-				     struct pci_dev *pdev,
-				     struct msi_desc *desc)
+static int xilinx_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				  unsigned int nr_irqs, void *args)
 {
-	struct xilinx_pcie_port *port = pdev->bus->sysdata;
-	unsigned int irq;
-	int hwirq;
-	struct msi_msg msg;
-	phys_addr_t msg_addr;
+	struct xilinx_pcie_port *port = domain->host_data;
+	int hwirq, i;
 
-	hwirq = xilinx_pcie_assign_msi();
-	if (hwirq < 0)
-		return hwirq;
-
-	irq = irq_create_mapping(port->msi_domain, hwirq);
-	if (!irq)
-		return -EINVAL;
+	mutex_lock(&port->map_lock);
 
-	irq_set_msi_desc(irq, desc);
+	hwirq = bitmap_find_free_region(port->msi_map, XILINX_NUM_MSI_IRQS, order_base_2(nr_irqs));
 
-	msg_addr = virt_to_phys((void *)port->msi_pages);
+	mutex_unlock(&port->map_lock);
 
-	msg.address_hi = 0;
-	msg.address_lo = msg_addr;
-	msg.data = irq;
+	if (hwirq < 0)
+		return -ENOSPC;
 
-	pci_write_msi_msg(irq, &msg);
+	for (i = 0; i < nr_irqs; i++)
+		irq_domain_set_info(domain, virq + i, hwirq + i,
+				    &xilinx_msi_bottom_chip, domain->host_data,
+				    handle_edge_irq, NULL, NULL);
 
 	return 0;
 }
 
-/* MSI Chip Descriptor */
-static struct msi_controller xilinx_pcie_msi_chip = {
-	.setup_irq = xilinx_pcie_msi_setup_irq,
-	.teardown_irq = xilinx_msi_teardown_irq,
-};
+static void xilinx_msi_domain_free(struct irq_domain *domain, unsigned int virq,
+				  unsigned int nr_irqs)
+{
+	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
+	struct xilinx_pcie_port *port = domain->host_data;
 
-/* HW Interrupt Chip Descriptor */
-static struct irq_chip xilinx_msi_irq_chip = {
-	.name = "Xilinx PCIe MSI",
-	.irq_enable = pci_msi_unmask_irq,
-	.irq_disable = pci_msi_mask_irq,
-	.irq_mask = pci_msi_mask_irq,
-	.irq_unmask = pci_msi_unmask_irq,
-};
+	mutex_lock(&port->map_lock);
 
-/**
- * xilinx_pcie_msi_map - Set the handler for the MSI and mark IRQ as valid
- * @domain: IRQ domain
- * @irq: Virtual IRQ number
- * @hwirq: HW interrupt number
- *
- * Return: Always returns 0.
- */
-static int xilinx_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
-			       irq_hw_number_t hwirq)
-{
-	irq_set_chip_and_handler(irq, &xilinx_msi_irq_chip, handle_simple_irq);
-	irq_set_chip_data(irq, domain->host_data);
+	bitmap_release_region(port->msi_map, d->hwirq, order_base_2(nr_irqs));
 
-	return 0;
+	mutex_unlock(&port->map_lock);
 }
 
-/* IRQ Domain operations */
-static const struct irq_domain_ops msi_domain_ops = {
-	.map = xilinx_pcie_msi_map,
+static const struct irq_domain_ops xilinx_msi_domain_ops = {
+	.alloc	= xilinx_msi_domain_alloc,
+	.free	= xilinx_msi_domain_free,
 };
 
-/**
- * xilinx_pcie_enable_msi - Enable MSI support
- * @port: PCIe port information
- */
-static int xilinx_pcie_enable_msi(struct xilinx_pcie_port *port)
+static struct msi_domain_info xilinx_msi_info = {
+	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS),
+	.chip	= &xilinx_msi_top_chip,
+};
+
+static int xilinx_allocate_msi_domains(struct xilinx_pcie_port *pcie)
 {
-	phys_addr_t msg_addr;
+	struct fwnode_handle *fwnode = dev_fwnode(pcie->dev);
+	struct irq_domain *parent;
 
-	port->msi_pages = __get_free_pages(GFP_KERNEL, 0);
-	if (!port->msi_pages)
+	parent = irq_domain_create_linear(fwnode, XILINX_NUM_MSI_IRQS,
+					  &xilinx_msi_domain_ops, pcie);
+	if (!parent) {
+		dev_err(pcie->dev, "failed to create IRQ domain\n");
 		return -ENOMEM;
+	}
+	irq_domain_update_bus_token(parent, DOMAIN_BUS_NEXUS);
 
-	msg_addr = virt_to_phys((void *)port->msi_pages);
-	pcie_write(port, 0x0, XILINX_PCIE_REG_MSIBASE1);
-	pcie_write(port, msg_addr, XILINX_PCIE_REG_MSIBASE2);
+	pcie->msi_domain = pci_msi_create_irq_domain(fwnode, &xilinx_msi_info, parent);
+	if (!pcie->msi_domain) {
+		dev_err(pcie->dev, "failed to create MSI domain\n");
+		irq_domain_remove(parent);
+		return -ENOMEM;
+	}
 
 	return 0;
 }
 
+static void xilinx_free_msi_domains(struct xilinx_pcie_port *pcie)
+{
+	struct irq_domain *parent = pcie->msi_domain->parent;
+
+	irq_domain_remove(pcie->msi_domain);
+	irq_domain_remove(parent);
+}
+
 /* INTx Functions */
 
 /**
@@ -420,6 +375,8 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
 	}
 
 	if (status & (XILINX_PCIE_INTR_INTX | XILINX_PCIE_INTR_MSI)) {
+		unsigned int irq;
+
 		val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
 
 		/* Check whether interrupt valid */
@@ -432,20 +389,19 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
 		if (val & XILINX_PCIE_RPIFR1_MSI_INTR) {
 			val = pcie_read(port, XILINX_PCIE_REG_RPIFR2) &
 				XILINX_PCIE_RPIFR2_MSG_DATA;
+			irq = irq_find_mapping(port->msi_domain->parent, val);
 		} else {
 			val = (val & XILINX_PCIE_RPIFR1_INTR_MASK) >>
 				XILINX_PCIE_RPIFR1_INTR_SHIFT;
-			val = irq_find_mapping(port->leg_domain, val);
+			irq = irq_find_mapping(port->leg_domain, val);
 		}
 
 		/* Clear interrupt FIFO register 1 */
 		pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
 			   XILINX_PCIE_REG_RPIFR1);
 
-		/* Handle the interrupt */
-		if (IS_ENABLED(CONFIG_PCI_MSI) ||
-		    !(val & XILINX_PCIE_RPIFR1_MSI_INTR))
-			generic_handle_irq(val);
+		if (irq)
+			generic_handle_irq(irq);
 	}
 
 	if (status & XILINX_PCIE_INTR_SLV_UNSUPP)
@@ -491,12 +447,11 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
 static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port)
 {
 	struct device *dev = port->dev;
-	struct device_node *node = dev->of_node;
 	struct device_node *pcie_intc_node;
 	int ret;
 
 	/* Setup INTx */
-	pcie_intc_node = of_get_next_child(node, NULL);
+	pcie_intc_node = of_get_next_child(dev->of_node, NULL);
 	if (!pcie_intc_node) {
 		dev_err(dev, "No PCIe Intc node found\n");
 		return -ENODEV;
@@ -513,18 +468,14 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port)
 
 	/* Setup MSI */
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		port->msi_domain = irq_domain_add_linear(node,
-							 XILINX_NUM_MSI_IRQS,
-							 &msi_domain_ops,
-							 &xilinx_pcie_msi_chip);
-		if (!port->msi_domain) {
-			dev_err(dev, "Failed to get a MSI IRQ domain\n");
-			return -ENODEV;
-		}
+		phys_addr_t pa = virt_to_phys(port);
 
-		ret = xilinx_pcie_enable_msi(port);
+		ret = xilinx_allocate_msi_domains(port);
 		if (ret)
 			return ret;
+
+		pcie_write(port, upper_32_bits(pa), XILINX_PCIE_REG_MSIBASE1);
+		pcie_write(port, lower_32_bits(pa), XILINX_PCIE_REG_MSIBASE2);
 	}
 
 	return 0;
@@ -572,6 +523,7 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
 	struct device *dev = port->dev;
 	struct device_node *node = dev->of_node;
 	struct resource regs;
+	unsigned int irq;
 	int err;
 
 	err = of_address_to_resource(node, 0, &regs);
@@ -584,12 +536,12 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
 	if (IS_ERR(port->reg_base))
 		return PTR_ERR(port->reg_base);
 
-	port->irq = irq_of_parse_and_map(node, 0);
-	err = devm_request_irq(dev, port->irq, xilinx_pcie_intr_handler,
+	irq = irq_of_parse_and_map(node, 0);
+	err = devm_request_irq(dev, irq, xilinx_pcie_intr_handler,
 			       IRQF_SHARED | IRQF_NO_THREAD,
 			       "xilinx-pcie", port);
 	if (err) {
-		dev_err(dev, "unable to request irq %d\n", port->irq);
+		dev_err(dev, "unable to request irq %d\n", irq);
 		return err;
 	}
 
@@ -617,7 +569,7 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 		return -ENODEV;
 
 	port = pci_host_bridge_priv(bridge);
-
+	mutex_init(&port->map_lock);
 	port->dev = dev;
 
 	err = xilinx_pcie_parse_dt(port);
@@ -637,11 +589,11 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 	bridge->sysdata = port;
 	bridge->ops = &xilinx_pcie_ops;
 
-#ifdef CONFIG_PCI_MSI
-	xilinx_pcie_msi_chip.dev = dev;
-	bridge->msi = &xilinx_pcie_msi_chip;
-#endif
-	return pci_host_probe(bridge);
+	err = pci_host_probe(bridge);
+	if (err)
+		xilinx_free_msi_domains(port);
+
+	return err;
 }
 
 static const struct of_device_id xilinx_pcie_of_match[] = {
-- 
2.29.2

Re: [PATCH 03/13] PCI: xilinx: Convert to MSI domains

[Lorenzo Pieralisi]

On Thu, Feb 25, 2021 at 03:10:13PM +0000, Marc Zyngier wrote:
> In anticipation of the removal of the msi_controller structure, convert
> the ancient xilinx host controller driver to MSI domains.
> 
> We end-up with the usual two domain structure, the top one being a
> generic PCI/MSI domain, the bottom one being xilinx-specific and handling
> the actual HW interrupt allocation.
> 
> This allows us to fix some of the most appaling MSI programming, where
> the message programmed in the device is the virtual IRQ number instead
> of the allocated vector number. The allocator is also made safe with
> a mutex. This should allow support for MultiMSI, but I decided not to
> even try, since I cannot test it.
> 
> Also take the opportunity to get rid of the cargo-culted memory allocation
> for the MSI capture address. *ANY* sufficiently aligned address should
> be good enough, so use the physical address of the xilinx_pcie_host
> structure instead.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/pci/controller/Kconfig       |   2 +-
>  drivers/pci/controller/pcie-xilinx.c | 238 +++++++++++----------------
>  2 files changed, 96 insertions(+), 144 deletions(-)

Michal,

can you please test these changes or make sure someone does and report
back on the mailing list please ?

I would like to merge this series for v5.13.

Thanks,
Lorenzo

> 
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index ccbf034512d6..049c60016904 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -95,7 +95,7 @@ config PCI_HOST_GENERIC
>  config PCIE_XILINX
>  	bool "Xilinx AXI PCIe host bridge support"
>  	depends on OF || COMPILE_TEST
> -	select PCI_MSI_ARCH_FALLBACKS
> +	depends on PCI_MSI_IRQ_DOMAIN
>  	help
>  	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
>  	  Host Bridge driver.
> diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
> index fa5baeb82653..ad9abf405167 100644
> --- a/drivers/pci/controller/pcie-xilinx.c
> +++ b/drivers/pci/controller/pcie-xilinx.c
> @@ -93,25 +93,23 @@
>  /**
>   * struct xilinx_pcie_port - PCIe port information
>   * @reg_base: IO Mapped Register Base
> - * @irq: Interrupt number
> - * @msi_pages: MSI pages
>   * @dev: Device pointer
> + * @msi_map: Bitmap of allocated MSIs
> + * @map_lock: Mutex protecting the MSI allocation
>   * @msi_domain: MSI IRQ domain pointer
>   * @leg_domain: Legacy IRQ domain pointer
>   * @resources: Bus Resources
>   */
>  struct xilinx_pcie_port {
>  	void __iomem *reg_base;
> -	u32 irq;
> -	unsigned long msi_pages;
>  	struct device *dev;
> +	unsigned long msi_map[BITS_TO_LONGS(XILINX_NUM_MSI_IRQS)];
> +	struct mutex map_lock;
>  	struct irq_domain *msi_domain;
>  	struct irq_domain *leg_domain;
>  	struct list_head resources;
>  };
>  
> -static DECLARE_BITMAP(msi_irq_in_use, XILINX_NUM_MSI_IRQS);
> -
>  static inline u32 pcie_read(struct xilinx_pcie_port *port, u32 reg)
>  {
>  	return readl(port->reg_base + reg);
> @@ -196,151 +194,108 @@ static struct pci_ops xilinx_pcie_ops = {
>  
>  /* MSI functions */
>  
> -/**
> - * xilinx_pcie_destroy_msi - Free MSI number
> - * @irq: IRQ to be freed
> - */
> -static void xilinx_pcie_destroy_msi(unsigned int irq)
> +static struct irq_chip xilinx_msi_top_chip = {
> +	.name		= "PCIe MSI",
> +};
> +
> +static int xilinx_msi_set_affinity(struct irq_data *d, const struct cpumask *mask, bool force)
>  {
> -	struct msi_desc *msi;
> -	struct xilinx_pcie_port *port;
> -	struct irq_data *d = irq_get_irq_data(irq);
> -	irq_hw_number_t hwirq = irqd_to_hwirq(d);
> -
> -	if (!test_bit(hwirq, msi_irq_in_use)) {
> -		msi = irq_get_msi_desc(irq);
> -		port = msi_desc_to_pci_sysdata(msi);
> -		dev_err(port->dev, "Trying to free unused MSI#%d\n", irq);
> -	} else {
> -		clear_bit(hwirq, msi_irq_in_use);
> -	}
> +	return -EINVAL;
>  }
>  
> -/**
> - * xilinx_pcie_assign_msi - Allocate MSI number
> - *
> - * Return: A valid IRQ on success and error value on failure.
> - */
> -static int xilinx_pcie_assign_msi(void)
> +static void xilinx_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>  {
> -	int pos;
> +	struct xilinx_pcie_port *pcie = irq_data_get_irq_chip_data(data);
> +	phys_addr_t pa = virt_to_phys(pcie);
>  
> -	pos = find_first_zero_bit(msi_irq_in_use, XILINX_NUM_MSI_IRQS);
> -	if (pos < XILINX_NUM_MSI_IRQS)
> -		set_bit(pos, msi_irq_in_use);
> -	else
> -		return -ENOSPC;
> -
> -	return pos;
> +	msg->address_lo = lower_32_bits(pa);
> +	msg->address_hi = upper_32_bits(pa);
> +	msg->data = data->hwirq;
>  }
>  
> -/**
> - * xilinx_msi_teardown_irq - Destroy the MSI
> - * @chip: MSI Chip descriptor
> - * @irq: MSI IRQ to destroy
> - */
> -static void xilinx_msi_teardown_irq(struct msi_controller *chip,
> -				    unsigned int irq)
> -{
> -	xilinx_pcie_destroy_msi(irq);
> -	irq_dispose_mapping(irq);
> -}
> +static struct irq_chip xilinx_msi_bottom_chip = {
> +	.name			= "Xilinx MSI",
> +	.irq_set_affinity 	= xilinx_msi_set_affinity,
> +	.irq_compose_msi_msg	= xilinx_compose_msi_msg,
> +};
>  
> -/**
> - * xilinx_pcie_msi_setup_irq - Setup MSI request
> - * @chip: MSI chip pointer
> - * @pdev: PCIe device pointer
> - * @desc: MSI descriptor pointer
> - *
> - * Return: '0' on success and error value on failure
> - */
> -static int xilinx_pcie_msi_setup_irq(struct msi_controller *chip,
> -				     struct pci_dev *pdev,
> -				     struct msi_desc *desc)
> +static int xilinx_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
> +				  unsigned int nr_irqs, void *args)
>  {
> -	struct xilinx_pcie_port *port = pdev->bus->sysdata;
> -	unsigned int irq;
> -	int hwirq;
> -	struct msi_msg msg;
> -	phys_addr_t msg_addr;
> +	struct xilinx_pcie_port *port = domain->host_data;
> +	int hwirq, i;
>  
> -	hwirq = xilinx_pcie_assign_msi();
> -	if (hwirq < 0)
> -		return hwirq;
> -
> -	irq = irq_create_mapping(port->msi_domain, hwirq);
> -	if (!irq)
> -		return -EINVAL;
> +	mutex_lock(&port->map_lock);
>  
> -	irq_set_msi_desc(irq, desc);
> +	hwirq = bitmap_find_free_region(port->msi_map, XILINX_NUM_MSI_IRQS, order_base_2(nr_irqs));
>  
> -	msg_addr = virt_to_phys((void *)port->msi_pages);
> +	mutex_unlock(&port->map_lock);
>  
> -	msg.address_hi = 0;
> -	msg.address_lo = msg_addr;
> -	msg.data = irq;
> +	if (hwirq < 0)
> +		return -ENOSPC;
>  
> -	pci_write_msi_msg(irq, &msg);
> +	for (i = 0; i < nr_irqs; i++)
> +		irq_domain_set_info(domain, virq + i, hwirq + i,
> +				    &xilinx_msi_bottom_chip, domain->host_data,
> +				    handle_edge_irq, NULL, NULL);
>  
>  	return 0;
>  }
>  
> -/* MSI Chip Descriptor */
> -static struct msi_controller xilinx_pcie_msi_chip = {
> -	.setup_irq = xilinx_pcie_msi_setup_irq,
> -	.teardown_irq = xilinx_msi_teardown_irq,
> -};
> +static void xilinx_msi_domain_free(struct irq_domain *domain, unsigned int virq,
> +				  unsigned int nr_irqs)
> +{
> +	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
> +	struct xilinx_pcie_port *port = domain->host_data;
>  
> -/* HW Interrupt Chip Descriptor */
> -static struct irq_chip xilinx_msi_irq_chip = {
> -	.name = "Xilinx PCIe MSI",
> -	.irq_enable = pci_msi_unmask_irq,
> -	.irq_disable = pci_msi_mask_irq,
> -	.irq_mask = pci_msi_mask_irq,
> -	.irq_unmask = pci_msi_unmask_irq,
> -};
> +	mutex_lock(&port->map_lock);
>  
> -/**
> - * xilinx_pcie_msi_map - Set the handler for the MSI and mark IRQ as valid
> - * @domain: IRQ domain
> - * @irq: Virtual IRQ number
> - * @hwirq: HW interrupt number
> - *
> - * Return: Always returns 0.
> - */
> -static int xilinx_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
> -			       irq_hw_number_t hwirq)
> -{
> -	irq_set_chip_and_handler(irq, &xilinx_msi_irq_chip, handle_simple_irq);
> -	irq_set_chip_data(irq, domain->host_data);
> +	bitmap_release_region(port->msi_map, d->hwirq, order_base_2(nr_irqs));
>  
> -	return 0;
> +	mutex_unlock(&port->map_lock);
>  }
>  
> -/* IRQ Domain operations */
> -static const struct irq_domain_ops msi_domain_ops = {
> -	.map = xilinx_pcie_msi_map,
> +static const struct irq_domain_ops xilinx_msi_domain_ops = {
> +	.alloc	= xilinx_msi_domain_alloc,
> +	.free	= xilinx_msi_domain_free,
>  };
>  
> -/**
> - * xilinx_pcie_enable_msi - Enable MSI support
> - * @port: PCIe port information
> - */
> -static int xilinx_pcie_enable_msi(struct xilinx_pcie_port *port)
> +static struct msi_domain_info xilinx_msi_info = {
> +	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS),
> +	.chip	= &xilinx_msi_top_chip,
> +};
> +
> +static int xilinx_allocate_msi_domains(struct xilinx_pcie_port *pcie)
>  {
> -	phys_addr_t msg_addr;
> +	struct fwnode_handle *fwnode = dev_fwnode(pcie->dev);
> +	struct irq_domain *parent;
>  
> -	port->msi_pages = __get_free_pages(GFP_KERNEL, 0);
> -	if (!port->msi_pages)
> +	parent = irq_domain_create_linear(fwnode, XILINX_NUM_MSI_IRQS,
> +					  &xilinx_msi_domain_ops, pcie);
> +	if (!parent) {
> +		dev_err(pcie->dev, "failed to create IRQ domain\n");
>  		return -ENOMEM;
> +	}
> +	irq_domain_update_bus_token(parent, DOMAIN_BUS_NEXUS);
>  
> -	msg_addr = virt_to_phys((void *)port->msi_pages);
> -	pcie_write(port, 0x0, XILINX_PCIE_REG_MSIBASE1);
> -	pcie_write(port, msg_addr, XILINX_PCIE_REG_MSIBASE2);
> +	pcie->msi_domain = pci_msi_create_irq_domain(fwnode, &xilinx_msi_info, parent);
> +	if (!pcie->msi_domain) {
> +		dev_err(pcie->dev, "failed to create MSI domain\n");
> +		irq_domain_remove(parent);
> +		return -ENOMEM;
> +	}
>  
>  	return 0;
>  }
>  
> +static void xilinx_free_msi_domains(struct xilinx_pcie_port *pcie)
> +{
> +	struct irq_domain *parent = pcie->msi_domain->parent;
> +
> +	irq_domain_remove(pcie->msi_domain);
> +	irq_domain_remove(parent);
> +}
> +
>  /* INTx Functions */
>  
>  /**
> @@ -420,6 +375,8 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
>  	}
>  
>  	if (status & (XILINX_PCIE_INTR_INTX | XILINX_PCIE_INTR_MSI)) {
> +		unsigned int irq;
> +
>  		val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
>  
>  		/* Check whether interrupt valid */
> @@ -432,20 +389,19 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
>  		if (val & XILINX_PCIE_RPIFR1_MSI_INTR) {
>  			val = pcie_read(port, XILINX_PCIE_REG_RPIFR2) &
>  				XILINX_PCIE_RPIFR2_MSG_DATA;
> +			irq = irq_find_mapping(port->msi_domain->parent, val);
>  		} else {
>  			val = (val & XILINX_PCIE_RPIFR1_INTR_MASK) >>
>  				XILINX_PCIE_RPIFR1_INTR_SHIFT;
> -			val = irq_find_mapping(port->leg_domain, val);
> +			irq = irq_find_mapping(port->leg_domain, val);
>  		}
>  
>  		/* Clear interrupt FIFO register 1 */
>  		pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
>  			   XILINX_PCIE_REG_RPIFR1);
>  
> -		/* Handle the interrupt */
> -		if (IS_ENABLED(CONFIG_PCI_MSI) ||
> -		    !(val & XILINX_PCIE_RPIFR1_MSI_INTR))
> -			generic_handle_irq(val);
> +		if (irq)
> +			generic_handle_irq(irq);
>  	}
>  
>  	if (status & XILINX_PCIE_INTR_SLV_UNSUPP)
> @@ -491,12 +447,11 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
>  static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port)
>  {
>  	struct device *dev = port->dev;
> -	struct device_node *node = dev->of_node;
>  	struct device_node *pcie_intc_node;
>  	int ret;
>  
>  	/* Setup INTx */
> -	pcie_intc_node = of_get_next_child(node, NULL);
> +	pcie_intc_node = of_get_next_child(dev->of_node, NULL);
>  	if (!pcie_intc_node) {
>  		dev_err(dev, "No PCIe Intc node found\n");
>  		return -ENODEV;
> @@ -513,18 +468,14 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port)
>  
>  	/* Setup MSI */
>  	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> -		port->msi_domain = irq_domain_add_linear(node,
> -							 XILINX_NUM_MSI_IRQS,
> -							 &msi_domain_ops,
> -							 &xilinx_pcie_msi_chip);
> -		if (!port->msi_domain) {
> -			dev_err(dev, "Failed to get a MSI IRQ domain\n");
> -			return -ENODEV;
> -		}
> +		phys_addr_t pa = virt_to_phys(port);
>  
> -		ret = xilinx_pcie_enable_msi(port);
> +		ret = xilinx_allocate_msi_domains(port);
>  		if (ret)
>  			return ret;
> +
> +		pcie_write(port, upper_32_bits(pa), XILINX_PCIE_REG_MSIBASE1);
> +		pcie_write(port, lower_32_bits(pa), XILINX_PCIE_REG_MSIBASE2);
>  	}
>  
>  	return 0;
> @@ -572,6 +523,7 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
>  	struct device *dev = port->dev;
>  	struct device_node *node = dev->of_node;
>  	struct resource regs;
> +	unsigned int irq;
>  	int err;
>  
>  	err = of_address_to_resource(node, 0, &regs);
> @@ -584,12 +536,12 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
>  	if (IS_ERR(port->reg_base))
>  		return PTR_ERR(port->reg_base);
>  
> -	port->irq = irq_of_parse_and_map(node, 0);
> -	err = devm_request_irq(dev, port->irq, xilinx_pcie_intr_handler,
> +	irq = irq_of_parse_and_map(node, 0);
> +	err = devm_request_irq(dev, irq, xilinx_pcie_intr_handler,
>  			       IRQF_SHARED | IRQF_NO_THREAD,
>  			       "xilinx-pcie", port);
>  	if (err) {
> -		dev_err(dev, "unable to request irq %d\n", port->irq);
> +		dev_err(dev, "unable to request irq %d\n", irq);
>  		return err;
>  	}
>  
> @@ -617,7 +569,7 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  
>  	port = pci_host_bridge_priv(bridge);
> -
> +	mutex_init(&port->map_lock);
>  	port->dev = dev;
>  
>  	err = xilinx_pcie_parse_dt(port);
> @@ -637,11 +589,11 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
>  	bridge->sysdata = port;
>  	bridge->ops = &xilinx_pcie_ops;
>  
> -#ifdef CONFIG_PCI_MSI
> -	xilinx_pcie_msi_chip.dev = dev;
> -	bridge->msi = &xilinx_pcie_msi_chip;
> -#endif
> -	return pci_host_probe(bridge);
> +	err = pci_host_probe(bridge);
> +	if (err)
> +		xilinx_free_msi_domains(port);
> +
> +	return err;
>  }
>  
>  static const struct of_device_id xilinx_pcie_of_match[] = {
> -- 
> 2.29.2
> 

Re: [PATCH 03/13] PCI: xilinx: Convert to MSI domains

[Michal Simek]

Hi,

On 3/22/21 1:21 PM, Lorenzo Pieralisi wrote:
> On Thu, Feb 25, 2021 at 03:10:13PM +0000, Marc Zyngier wrote:
>> In anticipation of the removal of the msi_controller structure, convert
>> the ancient xilinx host controller driver to MSI domains.
>>
>> We end-up with the usual two domain structure, the top one being a
>> generic PCI/MSI domain, the bottom one being xilinx-specific and handling
>> the actual HW interrupt allocation.
>>
>> This allows us to fix some of the most appaling MSI programming, where
>> the message programmed in the device is the virtual IRQ number instead
>> of the allocated vector number. The allocator is also made safe with
>> a mutex. This should allow support for MultiMSI, but I decided not to
>> even try, since I cannot test it.
>>
>> Also take the opportunity to get rid of the cargo-culted memory allocation
>> for the MSI capture address. *ANY* sufficiently aligned address should
>> be good enough, so use the physical address of the xilinx_pcie_host
>> structure instead.
>>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>>  drivers/pci/controller/Kconfig       |   2 +-
>>  drivers/pci/controller/pcie-xilinx.c | 238 +++++++++++----------------
>>  2 files changed, 96 insertions(+), 144 deletions(-)
> 
> Michal,
> 
> can you please test these changes or make sure someone does and report
> back on the mailing list please ?
> 
> I would like to merge this series for v5.13.

I got just private response (not sure why) from Bharat March 5 that
changes are fine.
It means go ahead with it.

Thanks,
Michal

Re: [PATCH 03/13] PCI: xilinx: Convert to MSI domains

[Marc Zyngier]

On Mon, 22 Mar 2021 12:33:10 +0000,
Michal Simek <monstr@monstr.eu> wrote:
> 
> Hi,
> 
> On 3/22/21 1:21 PM, Lorenzo Pieralisi wrote:
> > On Thu, Feb 25, 2021 at 03:10:13PM +0000, Marc Zyngier wrote:
> >> In anticipation of the removal of the msi_controller structure, convert
> >> the ancient xilinx host controller driver to MSI domains.
> >>
> >> We end-up with the usual two domain structure, the top one being a
> >> generic PCI/MSI domain, the bottom one being xilinx-specific and handling
> >> the actual HW interrupt allocation.
> >>
> >> This allows us to fix some of the most appaling MSI programming, where
> >> the message programmed in the device is the virtual IRQ number instead
> >> of the allocated vector number. The allocator is also made safe with
> >> a mutex. This should allow support for MultiMSI, but I decided not to
> >> even try, since I cannot test it.
> >>
> >> Also take the opportunity to get rid of the cargo-culted memory allocation
> >> for the MSI capture address. *ANY* sufficiently aligned address should
> >> be good enough, so use the physical address of the xilinx_pcie_host
> >> structure instead.
> >>
> >> Signed-off-by: Marc Zyngier <maz@kernel.org>
> >> ---
> >>  drivers/pci/controller/Kconfig       |   2 +-
> >>  drivers/pci/controller/pcie-xilinx.c | 238 +++++++++++----------------
> >>  2 files changed, 96 insertions(+), 144 deletions(-)
> > 
> > Michal,
> > 
> > can you please test these changes or make sure someone does and report
> > back on the mailing list please ?
> > 
> > I would like to merge this series for v5.13.
> 
> I got just private response (not sure why) from Bharat March 5 that
> changes are fine.
> It means go ahead with it.

Can I take this as a full Tested-by tag?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 03/13] PCI: xilinx: Convert to MSI domains

[Lorenzo Pieralisi]

On Thu, Feb 25, 2021 at 03:10:13PM +0000, Marc Zyngier wrote:
> In anticipation of the removal of the msi_controller structure, convert
> the ancient xilinx host controller driver to MSI domains.
> 
> We end-up with the usual two domain structure, the top one being a
> generic PCI/MSI domain, the bottom one being xilinx-specific and handling
> the actual HW interrupt allocation.
> 
> This allows us to fix some of the most appaling MSI programming, where
> the message programmed in the device is the virtual IRQ number instead
> of the allocated vector number. The allocator is also made safe with
> a mutex. This should allow support for MultiMSI, but I decided not to
> even try, since I cannot test it.
> 
> Also take the opportunity to get rid of the cargo-culted memory allocation
> for the MSI capture address. *ANY* sufficiently aligned address should
> be good enough, so use the physical address of the xilinx_pcie_host
> structure instead.

I'd agree with Bjorn that the MSI doorbell change is better split into
a separate patch, I can do it myself at merge if you agree.

Thanks,
Lorenzo

> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/pci/controller/Kconfig       |   2 +-
>  drivers/pci/controller/pcie-xilinx.c | 238 +++++++++++----------------
>  2 files changed, 96 insertions(+), 144 deletions(-)
> 
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index ccbf034512d6..049c60016904 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -95,7 +95,7 @@ config PCI_HOST_GENERIC
>  config PCIE_XILINX
>  	bool "Xilinx AXI PCIe host bridge support"
>  	depends on OF || COMPILE_TEST
> -	select PCI_MSI_ARCH_FALLBACKS
> +	depends on PCI_MSI_IRQ_DOMAIN
>  	help
>  	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
>  	  Host Bridge driver.
> diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
> index fa5baeb82653..ad9abf405167 100644
> --- a/drivers/pci/controller/pcie-xilinx.c
> +++ b/drivers/pci/controller/pcie-xilinx.c
> @@ -93,25 +93,23 @@
>  /**
>   * struct xilinx_pcie_port - PCIe port information
>   * @reg_base: IO Mapped Register Base
> - * @irq: Interrupt number
> - * @msi_pages: MSI pages
>   * @dev: Device pointer
> + * @msi_map: Bitmap of allocated MSIs
> + * @map_lock: Mutex protecting the MSI allocation
>   * @msi_domain: MSI IRQ domain pointer
>   * @leg_domain: Legacy IRQ domain pointer
>   * @resources: Bus Resources
>   */
>  struct xilinx_pcie_port {
>  	void __iomem *reg_base;
> -	u32 irq;
> -	unsigned long msi_pages;
>  	struct device *dev;
> +	unsigned long msi_map[BITS_TO_LONGS(XILINX_NUM_MSI_IRQS)];
> +	struct mutex map_lock;
>  	struct irq_domain *msi_domain;
>  	struct irq_domain *leg_domain;
>  	struct list_head resources;
>  };
>  
> -static DECLARE_BITMAP(msi_irq_in_use, XILINX_NUM_MSI_IRQS);
> -
>  static inline u32 pcie_read(struct xilinx_pcie_port *port, u32 reg)
>  {
>  	return readl(port->reg_base + reg);
> @@ -196,151 +194,108 @@ static struct pci_ops xilinx_pcie_ops = {
>  
>  /* MSI functions */
>  
> -/**
> - * xilinx_pcie_destroy_msi - Free MSI number
> - * @irq: IRQ to be freed
> - */
> -static void xilinx_pcie_destroy_msi(unsigned int irq)
> +static struct irq_chip xilinx_msi_top_chip = {
> +	.name		= "PCIe MSI",
> +};
> +
> +static int xilinx_msi_set_affinity(struct irq_data *d, const struct cpumask *mask, bool force)
>  {
> -	struct msi_desc *msi;
> -	struct xilinx_pcie_port *port;
> -	struct irq_data *d = irq_get_irq_data(irq);
> -	irq_hw_number_t hwirq = irqd_to_hwirq(d);
> -
> -	if (!test_bit(hwirq, msi_irq_in_use)) {
> -		msi = irq_get_msi_desc(irq);
> -		port = msi_desc_to_pci_sysdata(msi);
> -		dev_err(port->dev, "Trying to free unused MSI#%d\n", irq);
> -	} else {
> -		clear_bit(hwirq, msi_irq_in_use);
> -	}
> +	return -EINVAL;
>  }
>  
> -/**
> - * xilinx_pcie_assign_msi - Allocate MSI number
> - *
> - * Return: A valid IRQ on success and error value on failure.
> - */
> -static int xilinx_pcie_assign_msi(void)
> +static void xilinx_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>  {
> -	int pos;
> +	struct xilinx_pcie_port *pcie = irq_data_get_irq_chip_data(data);
> +	phys_addr_t pa = virt_to_phys(pcie);
>  
> -	pos = find_first_zero_bit(msi_irq_in_use, XILINX_NUM_MSI_IRQS);
> -	if (pos < XILINX_NUM_MSI_IRQS)
> -		set_bit(pos, msi_irq_in_use);
> -	else
> -		return -ENOSPC;
> -
> -	return pos;
> +	msg->address_lo = lower_32_bits(pa);
> +	msg->address_hi = upper_32_bits(pa);
> +	msg->data = data->hwirq;
>  }
>  
> -/**
> - * xilinx_msi_teardown_irq - Destroy the MSI
> - * @chip: MSI Chip descriptor
> - * @irq: MSI IRQ to destroy
> - */
> -static void xilinx_msi_teardown_irq(struct msi_controller *chip,
> -				    unsigned int irq)
> -{
> -	xilinx_pcie_destroy_msi(irq);
> -	irq_dispose_mapping(irq);
> -}
> +static struct irq_chip xilinx_msi_bottom_chip = {
> +	.name			= "Xilinx MSI",
> +	.irq_set_affinity 	= xilinx_msi_set_affinity,
> +	.irq_compose_msi_msg	= xilinx_compose_msi_msg,
> +};
>  
> -/**
> - * xilinx_pcie_msi_setup_irq - Setup MSI request
> - * @chip: MSI chip pointer
> - * @pdev: PCIe device pointer
> - * @desc: MSI descriptor pointer
> - *
> - * Return: '0' on success and error value on failure
> - */
> -static int xilinx_pcie_msi_setup_irq(struct msi_controller *chip,
> -				     struct pci_dev *pdev,
> -				     struct msi_desc *desc)
> +static int xilinx_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
> +				  unsigned int nr_irqs, void *args)
>  {
> -	struct xilinx_pcie_port *port = pdev->bus->sysdata;
> -	unsigned int irq;
> -	int hwirq;
> -	struct msi_msg msg;
> -	phys_addr_t msg_addr;
> +	struct xilinx_pcie_port *port = domain->host_data;
> +	int hwirq, i;
>  
> -	hwirq = xilinx_pcie_assign_msi();
> -	if (hwirq < 0)
> -		return hwirq;
> -
> -	irq = irq_create_mapping(port->msi_domain, hwirq);
> -	if (!irq)
> -		return -EINVAL;
> +	mutex_lock(&port->map_lock);
>  
> -	irq_set_msi_desc(irq, desc);
> +	hwirq = bitmap_find_free_region(port->msi_map, XILINX_NUM_MSI_IRQS, order_base_2(nr_irqs));
>  
> -	msg_addr = virt_to_phys((void *)port->msi_pages);
> +	mutex_unlock(&port->map_lock);
>  
> -	msg.address_hi = 0;
> -	msg.address_lo = msg_addr;
> -	msg.data = irq;
> +	if (hwirq < 0)
> +		return -ENOSPC;
>  
> -	pci_write_msi_msg(irq, &msg);
> +	for (i = 0; i < nr_irqs; i++)
> +		irq_domain_set_info(domain, virq + i, hwirq + i,
> +				    &xilinx_msi_bottom_chip, domain->host_data,
> +				    handle_edge_irq, NULL, NULL);
>  
>  	return 0;
>  }
>  
> -/* MSI Chip Descriptor */
> -static struct msi_controller xilinx_pcie_msi_chip = {
> -	.setup_irq = xilinx_pcie_msi_setup_irq,
> -	.teardown_irq = xilinx_msi_teardown_irq,
> -};
> +static void xilinx_msi_domain_free(struct irq_domain *domain, unsigned int virq,
> +				  unsigned int nr_irqs)
> +{
> +	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
> +	struct xilinx_pcie_port *port = domain->host_data;
>  
> -/* HW Interrupt Chip Descriptor */
> -static struct irq_chip xilinx_msi_irq_chip = {
> -	.name = "Xilinx PCIe MSI",
> -	.irq_enable = pci_msi_unmask_irq,
> -	.irq_disable = pci_msi_mask_irq,
> -	.irq_mask = pci_msi_mask_irq,
> -	.irq_unmask = pci_msi_unmask_irq,
> -};
> +	mutex_lock(&port->map_lock);
>  
> -/**
> - * xilinx_pcie_msi_map - Set the handler for the MSI and mark IRQ as valid
> - * @domain: IRQ domain
> - * @irq: Virtual IRQ number
> - * @hwirq: HW interrupt number
> - *
> - * Return: Always returns 0.
> - */
> -static int xilinx_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
> -			       irq_hw_number_t hwirq)
> -{
> -	irq_set_chip_and_handler(irq, &xilinx_msi_irq_chip, handle_simple_irq);
> -	irq_set_chip_data(irq, domain->host_data);
> +	bitmap_release_region(port->msi_map, d->hwirq, order_base_2(nr_irqs));
>  
> -	return 0;
> +	mutex_unlock(&port->map_lock);
>  }
>  
> -/* IRQ Domain operations */
> -static const struct irq_domain_ops msi_domain_ops = {
> -	.map = xilinx_pcie_msi_map,
> +static const struct irq_domain_ops xilinx_msi_domain_ops = {
> +	.alloc	= xilinx_msi_domain_alloc,
> +	.free	= xilinx_msi_domain_free,
>  };
>  
> -/**
> - * xilinx_pcie_enable_msi - Enable MSI support
> - * @port: PCIe port information
> - */
> -static int xilinx_pcie_enable_msi(struct xilinx_pcie_port *port)
> +static struct msi_domain_info xilinx_msi_info = {
> +	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS),
> +	.chip	= &xilinx_msi_top_chip,
> +};
> +
> +static int xilinx_allocate_msi_domains(struct xilinx_pcie_port *pcie)
>  {
> -	phys_addr_t msg_addr;
> +	struct fwnode_handle *fwnode = dev_fwnode(pcie->dev);
> +	struct irq_domain *parent;
>  
> -	port->msi_pages = __get_free_pages(GFP_KERNEL, 0);
> -	if (!port->msi_pages)
> +	parent = irq_domain_create_linear(fwnode, XILINX_NUM_MSI_IRQS,
> +					  &xilinx_msi_domain_ops, pcie);
> +	if (!parent) {
> +		dev_err(pcie->dev, "failed to create IRQ domain\n");
>  		return -ENOMEM;
> +	}
> +	irq_domain_update_bus_token(parent, DOMAIN_BUS_NEXUS);
>  
> -	msg_addr = virt_to_phys((void *)port->msi_pages);
> -	pcie_write(port, 0x0, XILINX_PCIE_REG_MSIBASE1);
> -	pcie_write(port, msg_addr, XILINX_PCIE_REG_MSIBASE2);
> +	pcie->msi_domain = pci_msi_create_irq_domain(fwnode, &xilinx_msi_info, parent);
> +	if (!pcie->msi_domain) {
> +		dev_err(pcie->dev, "failed to create MSI domain\n");
> +		irq_domain_remove(parent);
> +		return -ENOMEM;
> +	}
>  
>  	return 0;
>  }
>  
> +static void xilinx_free_msi_domains(struct xilinx_pcie_port *pcie)
> +{
> +	struct irq_domain *parent = pcie->msi_domain->parent;
> +
> +	irq_domain_remove(pcie->msi_domain);
> +	irq_domain_remove(parent);
> +}
> +
>  /* INTx Functions */
>  
>  /**
> @@ -420,6 +375,8 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
>  	}
>  
>  	if (status & (XILINX_PCIE_INTR_INTX | XILINX_PCIE_INTR_MSI)) {
> +		unsigned int irq;
> +
>  		val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
>  
>  		/* Check whether interrupt valid */
> @@ -432,20 +389,19 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
>  		if (val & XILINX_PCIE_RPIFR1_MSI_INTR) {
>  			val = pcie_read(port, XILINX_PCIE_REG_RPIFR2) &
>  				XILINX_PCIE_RPIFR2_MSG_DATA;
> +			irq = irq_find_mapping(port->msi_domain->parent, val);
>  		} else {
>  			val = (val & XILINX_PCIE_RPIFR1_INTR_MASK) >>
>  				XILINX_PCIE_RPIFR1_INTR_SHIFT;
> -			val = irq_find_mapping(port->leg_domain, val);
> +			irq = irq_find_mapping(port->leg_domain, val);
>  		}
>  
>  		/* Clear interrupt FIFO register 1 */
>  		pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
>  			   XILINX_PCIE_REG_RPIFR1);
>  
> -		/* Handle the interrupt */
> -		if (IS_ENABLED(CONFIG_PCI_MSI) ||
> -		    !(val & XILINX_PCIE_RPIFR1_MSI_INTR))
> -			generic_handle_irq(val);
> +		if (irq)
> +			generic_handle_irq(irq);
>  	}
>  
>  	if (status & XILINX_PCIE_INTR_SLV_UNSUPP)
> @@ -491,12 +447,11 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
>  static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port)
>  {
>  	struct device *dev = port->dev;
> -	struct device_node *node = dev->of_node;
>  	struct device_node *pcie_intc_node;
>  	int ret;
>  
>  	/* Setup INTx */
> -	pcie_intc_node = of_get_next_child(node, NULL);
> +	pcie_intc_node = of_get_next_child(dev->of_node, NULL);
>  	if (!pcie_intc_node) {
>  		dev_err(dev, "No PCIe Intc node found\n");
>  		return -ENODEV;
> @@ -513,18 +468,14 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port)
>  
>  	/* Setup MSI */
>  	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> -		port->msi_domain = irq_domain_add_linear(node,
> -							 XILINX_NUM_MSI_IRQS,
> -							 &msi_domain_ops,
> -							 &xilinx_pcie_msi_chip);
> -		if (!port->msi_domain) {
> -			dev_err(dev, "Failed to get a MSI IRQ domain\n");
> -			return -ENODEV;
> -		}
> +		phys_addr_t pa = virt_to_phys(port);
>  
> -		ret = xilinx_pcie_enable_msi(port);
> +		ret = xilinx_allocate_msi_domains(port);
>  		if (ret)
>  			return ret;
> +
> +		pcie_write(port, upper_32_bits(pa), XILINX_PCIE_REG_MSIBASE1);
> +		pcie_write(port, lower_32_bits(pa), XILINX_PCIE_REG_MSIBASE2);
>  	}
>  
>  	return 0;
> @@ -572,6 +523,7 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
>  	struct device *dev = port->dev;
>  	struct device_node *node = dev->of_node;
>  	struct resource regs;
> +	unsigned int irq;
>  	int err;
>  
>  	err = of_address_to_resource(node, 0, &regs);
> @@ -584,12 +536,12 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
>  	if (IS_ERR(port->reg_base))
>  		return PTR_ERR(port->reg_base);
>  
> -	port->irq = irq_of_parse_and_map(node, 0);
> -	err = devm_request_irq(dev, port->irq, xilinx_pcie_intr_handler,
> +	irq = irq_of_parse_and_map(node, 0);
> +	err = devm_request_irq(dev, irq, xilinx_pcie_intr_handler,
>  			       IRQF_SHARED | IRQF_NO_THREAD,
>  			       "xilinx-pcie", port);
>  	if (err) {
> -		dev_err(dev, "unable to request irq %d\n", port->irq);
> +		dev_err(dev, "unable to request irq %d\n", irq);
>  		return err;
>  	}
>  
> @@ -617,7 +569,7 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  
>  	port = pci_host_bridge_priv(bridge);
> -
> +	mutex_init(&port->map_lock);
>  	port->dev = dev;
>  
>  	err = xilinx_pcie_parse_dt(port);
> @@ -637,11 +589,11 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
>  	bridge->sysdata = port;
>  	bridge->ops = &xilinx_pcie_ops;
>  
> -#ifdef CONFIG_PCI_MSI
> -	xilinx_pcie_msi_chip.dev = dev;
> -	bridge->msi = &xilinx_pcie_msi_chip;
> -#endif
> -	return pci_host_probe(bridge);
> +	err = pci_host_probe(bridge);
> +	if (err)
> +		xilinx_free_msi_domains(port);
> +
> +	return err;
>  }
>  
>  static const struct of_device_id xilinx_pcie_of_match[] = {
> -- 
> 2.29.2
> 

Re: [PATCH 03/13] PCI: xilinx: Convert to MSI domains

[Michal Simek]

On 3/22/21 1:23 PM, Lorenzo Pieralisi wrote:
> On Thu, Feb 25, 2021 at 03:10:13PM +0000, Marc Zyngier wrote:
>> In anticipation of the removal of the msi_controller structure, convert
>> the ancient xilinx host controller driver to MSI domains.
>>
>> We end-up with the usual two domain structure, the top one being a
>> generic PCI/MSI domain, the bottom one being xilinx-specific and handling
>> the actual HW interrupt allocation.
>>
>> This allows us to fix some of the most appaling MSI programming, where
>> the message programmed in the device is the virtual IRQ number instead
>> of the allocated vector number. The allocator is also made safe with
>> a mutex. This should allow support for MultiMSI, but I decided not to
>> even try, since I cannot test it.
>>
>> Also take the opportunity to get rid of the cargo-culted memory allocation
>> for the MSI capture address. *ANY* sufficiently aligned address should
>> be good enough, so use the physical address of the xilinx_pcie_host
>> structure instead.
> 
> I'd agree with Bjorn that the MSI doorbell change is better split into
> a separate patch, I can do it myself at merge if you agree.

Thank you for doing it.

Thanks,
Michal

Re: [PATCH 03/13] PCI: xilinx: Convert to MSI domains

[Marc Zyngier]

Hi Lorenzo,

On Mon, 22 Mar 2021 12:23:15 +0000,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> 
> On Thu, Feb 25, 2021 at 03:10:13PM +0000, Marc Zyngier wrote:
> > In anticipation of the removal of the msi_controller structure, convert
> > the ancient xilinx host controller driver to MSI domains.
> > 
> > We end-up with the usual two domain structure, the top one being a
> > generic PCI/MSI domain, the bottom one being xilinx-specific and handling
> > the actual HW interrupt allocation.
> > 
> > This allows us to fix some of the most appaling MSI programming, where
> > the message programmed in the device is the virtual IRQ number instead
> > of the allocated vector number. The allocator is also made safe with
> > a mutex. This should allow support for MultiMSI, but I decided not to
> > even try, since I cannot test it.
> > 
> > Also take the opportunity to get rid of the cargo-culted memory allocation
> > for the MSI capture address. *ANY* sufficiently aligned address should
> > be good enough, so use the physical address of the xilinx_pcie_host
> > structure instead.
> 
> I'd agree with Bjorn that the MSI doorbell change is better split into
> a separate patch, I can do it myself at merge if you agree.

I need to respin the series as it now conflicts badly with the current
state of the tree (rcar has introduced one subtle change that needs
addressing). I'll post that later this week (hopefully tomorrow) with
rcar and xilinx having the doorbell fix in separate patches.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

[PATCH 04/13] PCI: hyperv: Drop msi_controller structure

[Marc Zyngier]

The Hyper-V PCI driver still makes use of a msi_controller structure,
but it looks more like a distant leftover than anything actually
useful, since it is initialised to 0 and never used for anything.

Just remove it.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/controller/pci-hyperv.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 6db8d96a78eb..93dc0fd004a3 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -473,7 +473,6 @@ struct hv_pcibus_device {
 	struct list_head dr_list;
 
 	struct msi_domain_info msi_info;
-	struct msi_controller msi_chip;
 	struct irq_domain *irq_domain;
 
 	spinlock_t retarget_msi_interrupt_lock;
@@ -1866,9 +1865,6 @@ static int create_root_hv_pci_bus(struct hv_pcibus_device *hbus)
 	if (!hbus->pci_bus)
 		return -ENODEV;
 
-	hbus->pci_bus->msi = &hbus->msi_chip;
-	hbus->pci_bus->msi->dev = &hbus->hdev->device;
-
 	pci_lock_rescan_remove();
 	pci_scan_child_bus(hbus->pci_bus);
 	hv_pci_assign_numa_node(hbus);
-- 
2.29.2

RE: [PATCH 04/13] PCI: hyperv: Drop msi_controller structure

[Michael Kelley]

From: Marc Zyngier <maz@kernel.org> Sent: Thursday, February 25, 2021 7:10 AM
> 
> The Hyper-V PCI driver still makes use of a msi_controller structure,
> but it looks more like a distant leftover than anything actually
> useful, since it is initialised to 0 and never used for anything.
> 
> Just remove it.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/pci/controller/pci-hyperv.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 6db8d96a78eb..93dc0fd004a3 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -473,7 +473,6 @@ struct hv_pcibus_device {
>  	struct list_head dr_list;
> 
>  	struct msi_domain_info msi_info;
> -	struct msi_controller msi_chip;
>  	struct irq_domain *irq_domain;
> 
>  	spinlock_t retarget_msi_interrupt_lock;
> @@ -1866,9 +1865,6 @@ static int create_root_hv_pci_bus(struct hv_pcibus_device *hbus)
>  	if (!hbus->pci_bus)
>  		return -ENODEV;
> 
> -	hbus->pci_bus->msi = &hbus->msi_chip;
> -	hbus->pci_bus->msi->dev = &hbus->hdev->device;
> -
>  	pci_lock_rescan_remove();
>  	pci_scan_child_bus(hbus->pci_bus);
>  	hv_pci_assign_numa_node(hbus);
> --
> 2.29.2

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

I also tested in an Azure VM on Hyper-V, with a Mellanox ConnectX-4
virtual function as the PCI device.  Started with a 5.11 kernel and
applied all 13 patches in the series.  The VM booted up correctly with
nothing anomalous in the 'dmesg' output. The Mellanox VF was
detected and configured properly.  'ethtool -S' output showed that
the Mellanox VF was handling network traffic as expected.  I also
hot-removed the Mellanox VF, and then hot-added it back again.
All worked as expected.  So,

Tested-by: Michael Kelley <mikelley@microsoft.com>

[PATCH 05/13] PCI: MSI: Drop use of msi_controller from core code

[Marc Zyngier]

As there is no driver using msi_controller, we can now safely
remove its use from the PCI probe code.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/msi.c   | 23 +----------------------
 drivers/pci/probe.c |  2 --
 include/linux/pci.h |  2 --
 3 files changed, 1 insertion(+), 26 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 3162f88fe940..79b5a995bd02 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -64,39 +64,18 @@ static void pci_msi_teardown_msi_irqs(struct pci_dev *dev)
 /* Arch hooks */
 int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
 {
-	struct msi_controller *chip = dev->bus->msi;
-	int err;
-
-	if (!chip || !chip->setup_irq)
-		return -EINVAL;
-
-	err = chip->setup_irq(chip, dev, desc);
-	if (err < 0)
-		return err;
-
-	irq_set_chip_data(desc->irq, chip);
-
-	return 0;
+	return -EINVAL;
 }
 
 void __weak arch_teardown_msi_irq(unsigned int irq)
 {
-	struct msi_controller *chip = irq_get_chip_data(irq);
-
-	if (!chip || !chip->teardown_irq)
-		return;
-
-	chip->teardown_irq(chip, irq);
 }
 
 int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 {
-	struct msi_controller *chip = dev->bus->msi;
 	struct msi_desc *entry;
 	int ret;
 
-	if (chip && chip->setup_irqs)
-		return chip->setup_irqs(chip, dev, nvec, type);
 	/*
 	 * If an architecture wants to support multiple MSI, it needs to
 	 * override arch_setup_msi_irqs()
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 953f15abc850..fb04fc81a8bd 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -895,7 +895,6 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
 	/* Temporarily move resources off the list */
 	list_splice_init(&bridge->windows, &resources);
 	bus->sysdata = bridge->sysdata;
-	bus->msi = bridge->msi;
 	bus->ops = bridge->ops;
 	bus->number = bus->busn_res.start = bridge->busnr;
 #ifdef CONFIG_PCI_DOMAINS_GENERIC
@@ -1053,7 +1052,6 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 		return NULL;
 
 	child->parent = parent;
-	child->msi = parent->msi;
 	child->sysdata = parent->sysdata;
 	child->bus_flags = parent->bus_flags;
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b32126d26997..105ef1a5191e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -540,7 +540,6 @@ struct pci_host_bridge {
 	int (*map_irq)(const struct pci_dev *, u8, u8);
 	void (*release_fn)(struct pci_host_bridge *);
 	void		*release_data;
-	struct msi_controller *msi;
 	unsigned int	ignore_reset_delay:1;	/* For entire hierarchy */
 	unsigned int	no_ext_tags:1;		/* No Extended Tags */
 	unsigned int	native_aer:1;		/* OS may use PCIe AER */
@@ -621,7 +620,6 @@ struct pci_bus {
 	struct resource busn_res;	/* Bus numbers routed to this bus */
 
 	struct pci_ops	*ops;		/* Configuration access functions */
-	struct msi_controller *msi;	/* MSI controller */
 	void		*sysdata;	/* Hook for sys-specific extension */
 	struct proc_dir_entry *procdir;	/* Directory entry in /proc/bus/pci */
 
-- 
2.29.2

[PATCH 06/13] PCI: MSI: Kill msi_controller structure

[Marc Zyngier]

msi_controller had a good, long life as the abstraction for
a driver providing MSIs to PCI devices. But it has been replaced
in all drivers by the more expressive generic MSI framework.

Farewell, struct msi_controller.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 include/linux/msi.h | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/include/linux/msi.h b/include/linux/msi.h
index aef35fd1cf11..3f21e77b57b7 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -240,8 +240,7 @@ void pci_msi_unmask_irq(struct irq_data *data);
 /*
  * The arch hooks to setup up msi irqs. Default functions are implemented
  * as weak symbols so that they /can/ be overriden by architecture specific
- * code if needed. These hooks must be enabled by the architecture or by
- * drivers which depend on them via msi_controller based MSI handling.
+ * code if needed. These hooks can only be enabled by the architecture.
  *
  * If CONFIG_PCI_MSI_ARCH_FALLBACKS is not selected they are replaced by
  * stubs with warnings.
@@ -272,19 +271,6 @@ static inline void arch_teardown_msi_irqs(struct pci_dev *dev)
 void arch_restore_msi_irqs(struct pci_dev *dev);
 void default_restore_msi_irqs(struct pci_dev *dev);
 
-struct msi_controller {
-	struct module *owner;
-	struct device *dev;
-	struct device_node *of_node;
-	struct list_head list;
-
-	int (*setup_irq)(struct msi_controller *chip, struct pci_dev *dev,
-			 struct msi_desc *desc);
-	int (*setup_irqs)(struct msi_controller *chip, struct pci_dev *dev,
-			  int nvec, int type);
-	void (*teardown_irq)(struct msi_controller *chip, unsigned int irq);
-};
-
 #ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
 
 #include <linux/irqhandler.h>
-- 
2.29.2

[PATCH 07/13] PCI: MSI: Kill default_teardown_msi_irqs()

[Marc Zyngier]

It doesn't have any caller left.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/msi.c   | 11 +----------
 include/linux/msi.h |  1 -
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 79b5a995bd02..d9c73c173c14 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -94,11 +94,7 @@ int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 	return 0;
 }
 
-/*
- * We have a default implementation available as a separate non-weak
- * function, as it is used by the Xen x86 PCI code
- */
-void default_teardown_msi_irqs(struct pci_dev *dev)
+void __weak arch_teardown_msi_irqs(struct pci_dev *dev)
 {
 	int i;
 	struct msi_desc *entry;
@@ -108,11 +104,6 @@ void default_teardown_msi_irqs(struct pci_dev *dev)
 			for (i = 0; i < entry->nvec_used; i++)
 				arch_teardown_msi_irq(entry->irq + i);
 }
-
-void __weak arch_teardown_msi_irqs(struct pci_dev *dev)
-{
-	return default_teardown_msi_irqs(dev);
-}
 #endif /* CONFIG_PCI_MSI_ARCH_FALLBACKS */
 
 static void default_restore_msi_irq(struct pci_dev *dev, int irq)
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 3f21e77b57b7..6aff469e511d 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -250,7 +250,6 @@ int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc);
 void arch_teardown_msi_irq(unsigned int irq);
 int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
 void arch_teardown_msi_irqs(struct pci_dev *dev);
-void default_teardown_msi_irqs(struct pci_dev *dev);
 #else
 static inline int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 {
-- 
2.29.2

[PATCH 08/13] PCI: MSI: Let PCI host bridges declare their lack of MSI handling

[Marc Zyngier]

From: Thomas Gleixner <tglx@linutronix.de>

Some PCI host bridges cannot deal with MSIs at all. This has
the unfortunate effect of triggering ugly warnings when an end-point
driver requests MSIs.

Instead, let the bridge advertise such lack of MSIs, so that it
can be flagged correctly by the core code.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
[maz: commit message]
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/probe.c | 2 ++
 include/linux/pci.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index fb04fc81a8bd..146bd85c037e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -925,6 +925,8 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
 	device_enable_async_suspend(bus->bridge);
 	pci_set_bus_of_node(bus);
 	pci_set_bus_msi_domain(bus);
+	if (bridge->no_msi)
+		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
 
 	if (!parent)
 		set_dev_node(bus->bridge, pcibus_to_node(bus));
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 105ef1a5191e..9db3abf9fd90 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -550,6 +550,7 @@ struct pci_host_bridge {
 	unsigned int	native_dpc:1;		/* OS may use PCIe DPC */
 	unsigned int	preserve_config:1;	/* Preserve FW resource setup */
 	unsigned int	size_windows:1;		/* Enable root bus sizing */
+	unsigned int	no_msi:1;		/* Bridge has no MSI support */
 
 	/* Resource alignment requirements */
 	resource_size_t (*align_resource)(struct pci_dev *dev,
-- 
2.29.2

[PATCH 09/13] PCI: mediatek: Advertise lack of MSI handling

[Marc Zyngier]

From: Thomas Gleixner <tglx@linutronix.de>

Some Mediatek host bridges cannot handle MSIs, which is sad.
This also results in an ugly warning at device probe time,
as the core PCI code wasn't told that MSIs were not available.

Advertise this fact to the rest of the core PCI code by
using the 'no_msi' attribute.

Reported-by: Frank Wunderlich <frank-w@public-files.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
[maz: commit message]
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/controller/pcie-mediatek.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index cf4c18f0c25a..27241e7e1eb6 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -143,6 +143,7 @@ struct mtk_pcie_port;
  * struct mtk_pcie_soc - differentiate between host generations
  * @need_fix_class_id: whether this host's class ID needed to be fixed or not
  * @need_fix_device_id: whether this host's device ID needed to be fixed or not
+ * @no_msi: Bridge has no MSI support
  * @device_id: device ID which this host need to be fixed
  * @ops: pointer to configuration access functions
  * @startup: pointer to controller setting functions
@@ -151,6 +152,7 @@ struct mtk_pcie_port;
 struct mtk_pcie_soc {
 	bool need_fix_class_id;
 	bool need_fix_device_id;
+	bool no_msi;
 	unsigned int device_id;
 	struct pci_ops *ops;
 	int (*startup)(struct mtk_pcie_port *port);
@@ -1084,6 +1086,7 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 
 	host->ops = pcie->soc->ops;
 	host->sysdata = pcie;
+	host->no_msi = pcie->soc->no_msi;
 
 	err = pci_host_probe(host);
 	if (err)
@@ -1173,6 +1176,7 @@ static const struct dev_pm_ops mtk_pcie_pm_ops = {
 };
 
 static const struct mtk_pcie_soc mtk_pcie_soc_v1 = {
+	.no_msi = true,
 	.ops = &mtk_pcie_ops,
 	.startup = mtk_pcie_startup_port,
 };
-- 
2.29.2

Aw: [PATCH 09/13] PCI: mediatek: Advertise lack of MSI handling

[Frank Wunderlich]

tested full series on bananapi-r2 and r64

r2 (with mt7615) looks good.

on r64 (with atheros card WLE900VX) i see this while loading ath10k driver:

[    6.525981] ath10k_pci 0000:01:00.0: enabling device (0000 -> 0002)
[    6.537810] ath10k_pci 0000:01:00.0: enabling bus mastering
[    6.543831] Unable to handle kernel paging request at virtual address ffffff4
013be2a80
[    6.551890] Mem abort info:
[    6.554744]   ESR = 0x96000044
[    6.557870]   EC = 0x25: DABT (current EL), IL = 32 bits
[    6.563267]   SET = 0, FnV = 0
[    6.566396]   EA = 0, S1PTW = 0
[    6.569611] Data abort info:
[    6.572501]   ISV = 0, ISS = 0x00000044
[    6.576411]   CM = 0, WnR = 1
[    6.579450] [ffffff4013be2a80] address between user and kernel address ranges
[    6.586659] Internal error: Oops: 96000044 [#1] PREEMPT SMP
[    6.592248] Modules linked in: ath10k_pci(+) ath10k_core ath mac80211 libarc4
 btmtkuart cfg80211 bluetooth ecdh_generic ecc rfkill libaes ip_tables x_tables
[    6.606329] CPU: 1 PID: 114 Comm: systemd-udevd Not tainted 5.11.0-bpi-r64-pc
i #3
[    6.613819] Hardware name: Bananapi BPI-R64 (DT)
[    6.618439] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
[    6.624452] pc : queued_spin_lock_slowpath+0x1e8/0x31c
[    6.629608] lr : queued_spin_lock_slowpath+0xac/0x31c
[    6.634666] sp : ffffffc010f63550
[    6.637982] x29: ffffffc010f63550 x28: 000000000000fc7e
[    6.643306] x27: ffffffc010c67410 x26: 0000000000080000
[    6.648629] x25: ffffffc010c67880 x24: ffffffc010f63810
[    6.653950] x23: 0000000000000000 x22: ffffffc010ba8860
[    6.659270] x21: ffffff803fdcc540 x20: ffffffc010a1c540
[    6.664591] x19: ffffff80016a1708 x18: 0000000000000000
[    6.669914] x17: 0000000000000000 x16: 0000000000000000
[    6.675236] x15: 000000000000000a x14: 0000000000000092
[    6.680560] x13: ffffff8006671004 x12: 0000000000000000
[    6.685883] x11: 0101010101010101 x10: ffffff8001635568
[    6.691206] x9 : 0000000000080000 x8 : ffffff8001635560
[    6.696529] x7 : 0000000000000000 x6 : ffffff803fdcc540
[    6.701849] x5 : 0000000000000002 x4 : 0000000000080000
[    6.707170] x3 : ffffff80016a170a x2 : 000000000000016a
[    6.712493] x1 : ffffff80031c6520 x0 : ffffffc010a1c560
[    6.717818] Call trace:
[    6.720276]  queued_spin_lock_slowpath+0x1e8/0x31c
[    6.725086]  do_raw_spin_lock+0x2c/0x38
[    6.728931]  _raw_spin_lock+0x24/0x34
[    6.732606]  __mutex_lock.isra.0+0xc4/0x29c
[    6.736799]  __mutex_lock_slowpath+0x14/0x20
[    6.741078]  mutex_lock+0x28/0x34
[    6.744402]  mtk_pcie_irq_domain_alloc+0x3c/0xd0
[    6.749037]  irq_domain_alloc_irqs_hierarchy+0x50/0x54
[    6.754187]  irq_domain_alloc_irqs_parent+0x18/0x2c
[    6.759073]  msi_domain_alloc+0x8c/0x12c
[    6.763007]  irq_domain_alloc_irqs_hierarchy+0x50/0x54
[    6.768154]  __irq_domain_alloc_irqs+0x114/0x344
[    6.772780]  __msi_domain_alloc_irqs+0x110/0x318
[    6.777408]  msi_domain_alloc_irqs+0x1c/0x28
[    6.781685]  pci_msi_setup_msi_irqs.isra.0+0x2c/0x44
[    6.786662]  __pci_enable_msi_range+0x230/0x320
[    6.791202]  pci_enable_msi+0x1c/0x30
[    6.794874]  ath10k_pci_probe+0x480/0x748 [ath10k_pci]
[    6.800058]  pci_device_probe+0xbc/0x14c
[    6.804014]  really_probe+0x2a0/0x470
[    6.807701]  driver_probe_device+0x12c/0x13c
[    6.811981]  device_driver_attach+0x44/0x70
[    6.816181]  __driver_attach+0x13c/0x140
[    6.820126]  bus_for_each_dev+0x70/0xc0
[    6.823971]  driver_attach+0x24/0x30
[    6.827556]  bus_add_driver+0x1a4/0x1ec
[    6.831401]  driver_register+0xb4/0xec
[    6.835168]  __pci_register_driver+0x44/0x50
[    6.839465]  ath10k_pci_init+0x28/0x1000 [ath10k_pci]
[    6.844563]  do_one_initcall+0x6c/0x188
[    6.848431]  do_init_module+0x5c/0x1e8
[    6.852205]  load_module+0x1124/0x11c8
[    6.855967]  __do_sys_finit_module+0xdc/0x100
[    6.860335]  __arm64_sys_finit_module+0x1c/0x28
[    6.864877]  el0_svc_common.constprop.0+0x124/0x198
[    6.869766]  do_el0_svc+0x48/0x78
[    6.873089]  el0_svc+0x14/0x20
[    6.876158]  el0_sync_handler+0xcc/0x154
[    6.880091]  el0_sync+0x174/0x180
[    6.883425] Code: d37c0400 51000421 8b000280 f861dac1 (f8216806)
[    6.889525] ---[ end trace 62498e1f489ea3ab ]---

i guess it's a bug in ath10k driver or my r64 board (it is a v1.1 which has missing capacitors on tx lines).
Tried with an mt7612e, this seems to work without any errors.

so for mt7622/mt7623

Tested-by: Frank Wunderlich <frank-w@public-files.de>

regards Frank

Re: Aw: [PATCH 09/13] PCI: mediatek: Advertise lack of MSI handling

[Marc Zyngier]

Frank,

On 2021-03-01 10:43, Frank Wunderlich wrote:
> tested full series on bananapi-r2 and r64
> 
> r2 (with mt7615) looks good.
> 
> on r64 (with atheros card WLE900VX) i see this while loading ath10k 
> driver:
> 
> [    6.525981] ath10k_pci 0000:01:00.0: enabling device (0000 -> 0002)
> [    6.537810] ath10k_pci 0000:01:00.0: enabling bus mastering
> [    6.543831] Unable to handle kernel paging request at virtual 
> address ffffff4
> 013be2a80
> [    6.551890] Mem abort info:
> [    6.554744]   ESR = 0x96000044
> [    6.557870]   EC = 0x25: DABT (current EL), IL = 32 bits
> [    6.563267]   SET = 0, FnV = 0
> [    6.566396]   EA = 0, S1PTW = 0
> [    6.569611] Data abort info:
> [    6.572501]   ISV = 0, ISS = 0x00000044
> [    6.576411]   CM = 0, WnR = 1
> [    6.579450] [ffffff4013be2a80] address between user and kernel 
> address ranges
> [    6.586659] Internal error: Oops: 96000044 [#1] PREEMPT SMP
> [    6.592248] Modules linked in: ath10k_pci(+) ath10k_core ath 
> mac80211 libarc4
>  btmtkuart cfg80211 bluetooth ecdh_generic ecc rfkill libaes ip_tables 
> x_tables
> [    6.606329] CPU: 1 PID: 114 Comm: systemd-udevd Not tainted 
> 5.11.0-bpi-r64-pc
> i #3
> [    6.613819] Hardware name: Bananapi BPI-R64 (DT)
> [    6.618439] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
> [    6.624452] pc : queued_spin_lock_slowpath+0x1e8/0x31c
> [    6.629608] lr : queued_spin_lock_slowpath+0xac/0x31c
> [    6.634666] sp : ffffffc010f63550
> [    6.637982] x29: ffffffc010f63550 x28: 000000000000fc7e
> [    6.643306] x27: ffffffc010c67410 x26: 0000000000080000
> [    6.648629] x25: ffffffc010c67880 x24: ffffffc010f63810
> [    6.653950] x23: 0000000000000000 x22: ffffffc010ba8860
> [    6.659270] x21: ffffff803fdcc540 x20: ffffffc010a1c540
> [    6.664591] x19: ffffff80016a1708 x18: 0000000000000000
> [    6.669914] x17: 0000000000000000 x16: 0000000000000000
> [    6.675236] x15: 000000000000000a x14: 0000000000000092
> [    6.680560] x13: ffffff8006671004 x12: 0000000000000000
> [    6.685883] x11: 0101010101010101 x10: ffffff8001635568
> [    6.691206] x9 : 0000000000080000 x8 : ffffff8001635560
> [    6.696529] x7 : 0000000000000000 x6 : ffffff803fdcc540
> [    6.701849] x5 : 0000000000000002 x4 : 0000000000080000
> [    6.707170] x3 : ffffff80016a170a x2 : 000000000000016a
> [    6.712493] x1 : ffffff80031c6520 x0 : ffffffc010a1c560
> [    6.717818] Call trace:
> [    6.720276]  queued_spin_lock_slowpath+0x1e8/0x31c
> [    6.725086]  do_raw_spin_lock+0x2c/0x38
> [    6.728931]  _raw_spin_lock+0x24/0x34
> [    6.732606]  __mutex_lock.isra.0+0xc4/0x29c
> [    6.736799]  __mutex_lock_slowpath+0x14/0x20
> [    6.741078]  mutex_lock+0x28/0x34
> [    6.744402]  mtk_pcie_irq_domain_alloc+0x3c/0xd0
> [    6.749037]  irq_domain_alloc_irqs_hierarchy+0x50/0x54
> [    6.754187]  irq_domain_alloc_irqs_parent+0x18/0x2c
> [    6.759073]  msi_domain_alloc+0x8c/0x12c
> [    6.763007]  irq_domain_alloc_irqs_hierarchy+0x50/0x54
> [    6.768154]  __irq_domain_alloc_irqs+0x114/0x344
> [    6.772780]  __msi_domain_alloc_irqs+0x110/0x318
> [    6.777408]  msi_domain_alloc_irqs+0x1c/0x28
> [    6.781685]  pci_msi_setup_msi_irqs.isra.0+0x2c/0x44
> [    6.786662]  __pci_enable_msi_range+0x230/0x320
> [    6.791202]  pci_enable_msi+0x1c/0x30
> [    6.794874]  ath10k_pci_probe+0x480/0x748 [ath10k_pci]
> [    6.800058]  pci_device_probe+0xbc/0x14c
> [    6.804014]  really_probe+0x2a0/0x470
> [    6.807701]  driver_probe_device+0x12c/0x13c
> [    6.811981]  device_driver_attach+0x44/0x70
> [    6.816181]  __driver_attach+0x13c/0x140
> [    6.820126]  bus_for_each_dev+0x70/0xc0
> [    6.823971]  driver_attach+0x24/0x30
> [    6.827556]  bus_add_driver+0x1a4/0x1ec
> [    6.831401]  driver_register+0xb4/0xec
> [    6.835168]  __pci_register_driver+0x44/0x50
> [    6.839465]  ath10k_pci_init+0x28/0x1000 [ath10k_pci]
> [    6.844563]  do_one_initcall+0x6c/0x188
> [    6.848431]  do_init_module+0x5c/0x1e8
> [    6.852205]  load_module+0x1124/0x11c8
> [    6.855967]  __do_sys_finit_module+0xdc/0x100
> [    6.860335]  __arm64_sys_finit_module+0x1c/0x28
> [    6.864877]  el0_svc_common.constprop.0+0x124/0x198
> [    6.869766]  do_el0_svc+0x48/0x78
> [    6.873089]  el0_svc+0x14/0x20
> [    6.876158]  el0_sync_handler+0xcc/0x154
> [    6.880091]  el0_sync+0x174/0x180
> [    6.883425] Code: d37c0400 51000421 8b000280 f861dac1 (f8216806)
> [    6.889525] ---[ end trace 62498e1f489ea3ab ]---
> 
> i guess it's a bug in ath10k driver or my r64 board (it is a v1.1
> which has missing capacitors on tx lines).

No, this definitely looks like a bug in the MTK PCIe driver,
where the mutex is either not properly initialised, corrupted,
or the wrong pointer is passed.

This r64 machine is supposed to have working MSIs, right?
Do you get the same issue without this series?

> Tried with an mt7612e, this seems to work without any errors.
> 
> so for mt7622/mt7623
> 
> Tested-by: Frank Wunderlich <frank-w@public-files.de>

We definitely need to understand the above.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

Aw: Re: [PATCH 09/13] PCI: mediatek: Advertise lack of MSI handling

[Frank Wunderlich]

regards Frank


> Gesendet: Montag, 01. März 2021 um 12:49 Uhr
> Von: "Marc Zyngier" <maz@kernel.org>
> Frank,
> 
> On 2021-03-01 10:43, Frank Wunderlich wrote:
> > tested full series on bananapi-r2 and r64
> > 
> > r2 (with mt7615) looks good.
> > 
> > on r64 (with atheros card WLE900VX) i see this while loading ath10k 
> > driver:
> > 
> > [    6.525981] ath10k_pci 0000:01:00.0: enabling device (0000 -> 0002)
> > [    6.537810] ath10k_pci 0000:01:00.0: enabling bus mastering
> > [    6.543831] Unable to handle kernel paging request at virtual 
> > address ffffff4
> > 013be2a80
> > [    6.551890] Mem abort info:
> > [    6.554744]   ESR = 0x96000044
> > [    6.557870]   EC = 0x25: DABT (current EL), IL = 32 bits
> > [    6.563267]   SET = 0, FnV = 0
> > [    6.566396]   EA = 0, S1PTW = 0
> > [    6.569611] Data abort info:
> > [    6.572501]   ISV = 0, ISS = 0x00000044
> > [    6.576411]   CM = 0, WnR = 1
> > [    6.579450] [ffffff4013be2a80] address between user and kernel 
> > address ranges
> > [    6.586659] Internal error: Oops: 96000044 [#1] PREEMPT SMP
> > [    6.592248] Modules linked in: ath10k_pci(+) ath10k_core ath 
> > mac80211 libarc4
> >  btmtkuart cfg80211 bluetooth ecdh_generic ecc rfkill libaes ip_tables 
> > x_tables
> > [    6.606329] CPU: 1 PID: 114 Comm: systemd-udevd Not tainted 
> > 5.11.0-bpi-r64-pc
> > i #3
> > [    6.613819] Hardware name: Bananapi BPI-R64 (DT)
> > [    6.618439] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
> > [    6.624452] pc : queued_spin_lock_slowpath+0x1e8/0x31c
> > [    6.629608] lr : queued_spin_lock_slowpath+0xac/0x31c
> > [    6.634666] sp : ffffffc010f63550
> > [    6.637982] x29: ffffffc010f63550 x28: 000000000000fc7e
> > [    6.643306] x27: ffffffc010c67410 x26: 0000000000080000
> > [    6.648629] x25: ffffffc010c67880 x24: ffffffc010f63810
> > [    6.653950] x23: 0000000000000000 x22: ffffffc010ba8860
> > [    6.659270] x21: ffffff803fdcc540 x20: ffffffc010a1c540
> > [    6.664591] x19: ffffff80016a1708 x18: 0000000000000000
> > [    6.669914] x17: 0000000000000000 x16: 0000000000000000
> > [    6.675236] x15: 000000000000000a x14: 0000000000000092
> > [    6.680560] x13: ffffff8006671004 x12: 0000000000000000
> > [    6.685883] x11: 0101010101010101 x10: ffffff8001635568
> > [    6.691206] x9 : 0000000000080000 x8 : ffffff8001635560
> > [    6.696529] x7 : 0000000000000000 x6 : ffffff803fdcc540
> > [    6.701849] x5 : 0000000000000002 x4 : 0000000000080000
> > [    6.707170] x3 : ffffff80016a170a x2 : 000000000000016a
> > [    6.712493] x1 : ffffff80031c6520 x0 : ffffffc010a1c560
> > [    6.717818] Call trace:
> > [    6.720276]  queued_spin_lock_slowpath+0x1e8/0x31c
> > [    6.725086]  do_raw_spin_lock+0x2c/0x38
> > [    6.728931]  _raw_spin_lock+0x24/0x34
> > [    6.732606]  __mutex_lock.isra.0+0xc4/0x29c
> > [    6.736799]  __mutex_lock_slowpath+0x14/0x20
> > [    6.741078]  mutex_lock+0x28/0x34
> > [    6.744402]  mtk_pcie_irq_domain_alloc+0x3c/0xd0
> > [    6.749037]  irq_domain_alloc_irqs_hierarchy+0x50/0x54
> > [    6.754187]  irq_domain_alloc_irqs_parent+0x18/0x2c
> > [    6.759073]  msi_domain_alloc+0x8c/0x12c
> > [    6.763007]  irq_domain_alloc_irqs_hierarchy+0x50/0x54
> > [    6.768154]  __irq_domain_alloc_irqs+0x114/0x344
> > [    6.772780]  __msi_domain_alloc_irqs+0x110/0x318
> > [    6.777408]  msi_domain_alloc_irqs+0x1c/0x28
> > [    6.781685]  pci_msi_setup_msi_irqs.isra.0+0x2c/0x44
> > [    6.786662]  __pci_enable_msi_range+0x230/0x320
> > [    6.791202]  pci_enable_msi+0x1c/0x30
> > [    6.794874]  ath10k_pci_probe+0x480/0x748 [ath10k_pci]
> > [    6.800058]  pci_device_probe+0xbc/0x14c
> > [    6.804014]  really_probe+0x2a0/0x470
> > [    6.807701]  driver_probe_device+0x12c/0x13c
> > [    6.811981]  device_driver_attach+0x44/0x70
> > [    6.816181]  __driver_attach+0x13c/0x140
> > [    6.820126]  bus_for_each_dev+0x70/0xc0
> > [    6.823971]  driver_attach+0x24/0x30
> > [    6.827556]  bus_add_driver+0x1a4/0x1ec
> > [    6.831401]  driver_register+0xb4/0xec
> > [    6.835168]  __pci_register_driver+0x44/0x50
> > [    6.839465]  ath10k_pci_init+0x28/0x1000 [ath10k_pci]
> > [    6.844563]  do_one_initcall+0x6c/0x188
> > [    6.848431]  do_init_module+0x5c/0x1e8
> > [    6.852205]  load_module+0x1124/0x11c8
> > [    6.855967]  __do_sys_finit_module+0xdc/0x100
> > [    6.860335]  __arm64_sys_finit_module+0x1c/0x28
> > [    6.864877]  el0_svc_common.constprop.0+0x124/0x198
> > [    6.869766]  do_el0_svc+0x48/0x78
> > [    6.873089]  el0_svc+0x14/0x20
> > [    6.876158]  el0_sync_handler+0xcc/0x154
> > [    6.880091]  el0_sync+0x174/0x180
> > [    6.883425] Code: d37c0400 51000421 8b000280 f861dac1 (f8216806)
> > [    6.889525] ---[ end trace 62498e1f489ea3ab ]---
> > 
> > i guess it's a bug in ath10k driver or my r64 board (it is a v1.1
> > which has missing capacitors on tx lines).
> 
> No, this definitely looks like a bug in the MTK PCIe driver,
> where the mutex is either not properly initialised, corrupted,
> or the wrong pointer is passed.

but why does it happen only with the ath10k-card and not the mt7612 in same slot?

> This r64 machine is supposed to have working MSIs, right?

imho mt7622 have working MSI

> Do you get the same issue without this series?

tested 5.11.0 [1] without this series (but with your/thomas' patch from discussion about my old patch) and got same trace. so this series does not break anything here.

> > Tried with an mt7612e, this seems to work without any errors.
> > 
> > so for mt7622/mt7623
> > 
> > Tested-by: Frank Wunderlich <frank-w@public-files.de>
> 
> We definitely need to understand the above.

there is a hardware-bug which may cause this...afair i saw this with the card in r64 with earlier Kernel-versions where other cards work (like the mt7612e).

regards Frank

[1] https://github.com/frank-w/BPI-R2-4.14/commits/5.11-main (pci: fix MSI issue part X)

Re: Aw: Re: [PATCH 09/13] PCI: mediatek: Advertise lack of MSI handling

[Marc Zyngier]

Frank,

>> > i guess it's a bug in ath10k driver or my r64 board (it is a v1.1
>> > which has missing capacitors on tx lines).
>> 
>> No, this definitely looks like a bug in the MTK PCIe driver,
>> where the mutex is either not properly initialised, corrupted,
>> or the wrong pointer is passed.
> 
> but why does it happen only with the ath10k-card and not the mt7612 in
> same slot?

Does mt7612 use MSI? What we have here is a bogus mutex in the
MTK PCIe driver, and the only way not to get there would be
to avoid using MSIs.

> 
>> This r64 machine is supposed to have working MSIs, right?
> 
> imho mt7622 have working MSI
> 
>> Do you get the same issue without this series?
> 
> tested 5.11.0 [1] without this series (but with your/thomas' patch
> from discussion about my old patch) and got same trace. so this series
> does not break anything here.

Can you retest without any additional patch on top of 5.11?
These two patches only affect platforms that do *not* have MSIs at all.

> 
>> > Tried with an mt7612e, this seems to work without any errors.
>> >
>> > so for mt7622/mt7623
>> >
>> > Tested-by: Frank Wunderlich <frank-w@public-files.de>
>> 
>> We definitely need to understand the above.
> 
> there is a hardware-bug which may cause this...afair i saw this with
> the card in r64 with earlier Kernel-versions where other cards work
> (like the mt7612e).

I don't think a HW bug affecting PCI would cause what we are seeing
here, unless it results in memory corruption.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

Aw: Re: Re: [PATCH 09/13] PCI: mediatek: Advertise lack of MSI handling

[Frank Wunderlich]

> Gesendet: Montag, 01. März 2021 um 14:31 Uhr
> Von: "Marc Zyngier" <maz@kernel.org>
>
> Frank,
> 
> >> > i guess it's a bug in ath10k driver or my r64 board (it is a v1.1
> >> > which has missing capacitors on tx lines).
> >> 
> >> No, this definitely looks like a bug in the MTK PCIe driver,
> >> where the mutex is either not properly initialised, corrupted,
> >> or the wrong pointer is passed.
> > 
> > but why does it happen only with the ath10k-card and not the mt7612 in
> > same slot?
> 
> Does mt7612 use MSI? What we have here is a bogus mutex in the
> MTK PCIe driver, and the only way not to get there would be
> to avoid using MSIs.

i guess this card/its driver does not use MSI. Did not found anything in "datasheet" [1] or driver [2] about msi

> > 
> >> This r64 machine is supposed to have working MSIs, right?
> > 
> > imho mt7622 have working MSI
> > 
> >> Do you get the same issue without this series?
> > 
> > tested 5.11.0 [1] without this series (but with your/thomas' patch
> > from discussion about my old patch) and got same trace. so this series
> > does not break anything here.
> 
> Can you retest without any additional patch on top of 5.11?
> These two patches only affect platforms that do *not* have MSIs at all.

i can revert these 2, but still need patches for mt7622 pcie-support [3]...btw. i see that i miss these in 5.11-main...do not see traceback with them (have firmware not installed...)

root@bpi-r64:~# dmesg | grep ath                                                
[    6.450765] ath10k_pci 0000:01:00.0: assign IRQ: got 146                     
[    6.661752] ath10k_pci 0000:01:00.0: enabling device (0000 -> 0002)          
[    6.697811] ath10k_pci 0000:01:00.0: enabling bus mastering                  
[    6.721293] ath10k_pci 0000:01:00.0: pci irq msi oper_irq_mode 2 irq_mode 0 r
eset_mode 0                                                                     
[    6.921030] ath10k_pci 0000:01:00.0: Failed to find firmware-N.bin (N between
 2 and 6) from ath10k/QCA988X/hw2.0: -2                                         
[    6.931698] ath10k_pci 0000:01:00.0: could not fetch firmware files (-2)     
[    6.940417] ath10k_pci 0000:01:00.0: could not probe fw (-2)

so traceback was caused by missing changes in mtk pcie-driver not yet upstream, added Chuanjia Liu

> > 
> >> > Tried with an mt7612e, this seems to work without any errors.
> >> >
> >> > so for mt7622/mt7623
> >> >
> >> > Tested-by: Frank Wunderlich <frank-w@public-files.de>
> >> 
> >> We definitely need to understand the above.
> > 
> > there is a hardware-bug which may cause this...afair i saw this with
> > the card in r64 with earlier Kernel-versions where other cards work
> > (like the mt7612e).
> 
> I don't think a HW bug affecting PCI would cause what we are seeing
> here, unless it results in memory corruption.


[1] https://www.asiarf.com/shop/wifi-wlan/wifi_mini_pcie/ws2433-wifi-11ac-mini-pcie-module-manufacturer/
[2] grep -Rni 'msi' drivers/net/wireless/mediatek/mt76/mt76x2/
[3] https://patchwork.kernel.org/project/linux-mediatek/list/?series=372885

Re: Aw: Re: Re: [PATCH 09/13] PCI: mediatek: Advertise lack of MSI handling

[Robin Murphy]

On 2021-03-01 14:06, Frank Wunderlich wrote:
>> Gesendet: Montag, 01. März 2021 um 14:31 Uhr
>> Von: "Marc Zyngier" <maz@kernel.org>
>>
>> Frank,
>>
>>>>> i guess it's a bug in ath10k driver or my r64 board (it is a v1.1
>>>>> which has missing capacitors on tx lines).
>>>>
>>>> No, this definitely looks like a bug in the MTK PCIe driver,
>>>> where the mutex is either not properly initialised, corrupted,
>>>> or the wrong pointer is passed.
>>>
>>> but why does it happen only with the ath10k-card and not the mt7612 in
>>> same slot?
>>
>> Does mt7612 use MSI? What we have here is a bogus mutex in the
>> MTK PCIe driver, and the only way not to get there would be
>> to avoid using MSIs.
> 
> i guess this card/its driver does not use MSI. Did not found anything in "datasheet" [1] or driver [2] about msi

FWIW, no need to guess - `lspci -v` (as root) should tell you whether 
the card has MSI (and/or MSI-X) capability, and whether it is enabled if so.

Robin.

>>>
>>>> This r64 machine is supposed to have working MSIs, right?
>>>
>>> imho mt7622 have working MSI
>>>
>>>> Do you get the same issue without this series?
>>>
>>> tested 5.11.0 [1] without this series (but with your/thomas' patch
>>> from discussion about my old patch) and got same trace. so this series
>>> does not break anything here.
>>
>> Can you retest without any additional patch on top of 5.11?
>> These two patches only affect platforms that do *not* have MSIs at all.
> 
> i can revert these 2, but still need patches for mt7622 pcie-support [3]...btw. i see that i miss these in 5.11-main...do not see traceback with them (have firmware not installed...)
> 
> root@bpi-r64:~# dmesg | grep ath
> [    6.450765] ath10k_pci 0000:01:00.0: assign IRQ: got 146
> [    6.661752] ath10k_pci 0000:01:00.0: enabling device (0000 -> 0002)
> [    6.697811] ath10k_pci 0000:01:00.0: enabling bus mastering
> [    6.721293] ath10k_pci 0000:01:00.0: pci irq msi oper_irq_mode 2 irq_mode 0 r
> eset_mode 0
> [    6.921030] ath10k_pci 0000:01:00.0: Failed to find firmware-N.bin (N between
>   2 and 6) from ath10k/QCA988X/hw2.0: -2
> [    6.931698] ath10k_pci 0000:01:00.0: could not fetch firmware files (-2)
> [    6.940417] ath10k_pci 0000:01:00.0: could not probe fw (-2)
> 
> so traceback was caused by missing changes in mtk pcie-driver not yet upstream, added Chuanjia Liu
> 
>>>
>>>>> Tried with an mt7612e, this seems to work without any errors.
>>>>>
>>>>> so for mt7622/mt7623
>>>>>
>>>>> Tested-by: Frank Wunderlich <frank-w@public-files.de>
>>>>
>>>> We definitely need to understand the above.
>>>
>>> there is a hardware-bug which may cause this...afair i saw this with
>>> the card in r64 with earlier Kernel-versions where other cards work
>>> (like the mt7612e).
>>
>> I don't think a HW bug affecting PCI would cause what we are seeing
>> here, unless it results in memory corruption.
> 
> 
> [1] https://www.asiarf.com/shop/wifi-wlan/wifi_mini_pcie/ws2433-wifi-11ac-mini-pcie-module-manufacturer/
> [2] grep -Rni 'msi' drivers/net/wireless/mediatek/mt76/mt76x2/
> [3] https://patchwork.kernel.org/project/linux-mediatek/list/?series=372885
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH 10/13] PCI: MSI: Let PCI host bridges declare their reliance on MSI domains

[Marc Zyngier]

The new 'no_msi' attribute solves the problem of advertising the lack
of MSI capability for host bridges that know for sure that there will
be no MSI for their end-points.

However, there is a whole class of host bridges that cannot know
whether MSIs will be provided or not, as they rely on other blocks
to provide the MSI functionnality, using MSI domains.  This is
the case for example on systems that use the ARM GIC architecture.

Introduce a new attribute ('msi_domain') indicating that implicit
dependency, and use this property to set the NO_MSI flag when
no MSI domain is found at probe time.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/probe.c | 2 +-
 include/linux/pci.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 146bd85c037e..bac9f69a06a8 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -925,7 +925,7 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
 	device_enable_async_suspend(bus->bridge);
 	pci_set_bus_of_node(bus);
 	pci_set_bus_msi_domain(bus);
-	if (bridge->no_msi)
+	if (bridge->no_msi || (bridge->msi_domain && !bus->dev.msi_domain))
 		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
 
 	if (!parent)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9db3abf9fd90..accc2454eab0 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -551,6 +551,7 @@ struct pci_host_bridge {
 	unsigned int	preserve_config:1;	/* Preserve FW resource setup */
 	unsigned int	size_windows:1;		/* Enable root bus sizing */
 	unsigned int	no_msi:1;		/* Bridge has no MSI support */
+	unsigned int	msi_domain:1;		/* Bridge wants MSI domain */
 
 	/* Resource alignment requirements */
 	resource_size_t (*align_resource)(struct pci_dev *dev,
-- 
2.29.2

[PATCH 11/13] PCI: Make pci_host_common_probe() declare its reliance on MSI domains

[Marc Zyngier]

The generic PCI host driver relies on MSI domains for MSIs to
be provided to its end-points. Make this dependency explicit.

This cures the warnings occuring on arm/arm64 VMs when booted
with PCI virtio devices and no MSI controller (no GICv3 ITS,
for example).

It is likely that other drivers will need to express the same
dependency.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/controller/pci-host-common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index 6ce34a1deecb..603f6fbbe68a 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -77,6 +77,7 @@ int pci_host_common_probe(struct platform_device *pdev)
 
 	bridge->sysdata = cfg;
 	bridge->ops = (struct pci_ops *)&ops->pci_ops;
+	bridge->msi_domain = true;
 
 	platform_set_drvdata(pdev, bridge);
 
-- 
2.29.2

[PATCH 12/13] PCI: MSI: Document the various ways of ending up with NO_MSI

[Marc Zyngier]

We have now 4 ways of ending up with NO_MSI being set.
Document them.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/msi.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index d9c73c173c14..425abcdfcaca 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -871,8 +871,16 @@ static int pci_msi_supported(struct pci_dev *dev, int nvec)
 	 * Any bridge which does NOT route MSI transactions from its
 	 * secondary bus to its primary bus must set NO_MSI flag on
 	 * the secondary pci_bus.
-	 * We expect only arch-specific PCI host bus controller driver
-	 * or quirks for specific PCI bridges to be setting NO_MSI.
+	 *
+	 * The NO_MSI flag can either be set directly by:
+	 * - arch-specific PCI host bus controller drivers (deprecated)
+	 * - quirks for specific PCI bridges
+	 *
+	 * or indirectly by platform-specific PCI host bridge drivers by:
+	 * - unconditionally advertising the 'no_msi' property
+	 * - advertising the 'msi_domain' property, which results in
+	 *   the NO_MSI flag when no MSI domain is found for this bridge
+	 *   at probe time.
 	 */
 	for (bus = dev->bus; bus; bus = bus->parent)
 		if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
-- 
2.29.2

[PATCH 13/13] PCI: quirks: Refactor advertising of the NO_MSI flag

[Marc Zyngier]

The few quirks that deal with NO_MSI tend to be copy-paste heavy.
Refactor them so that the hierarchy of conditions is slightly
cleaner.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/quirks.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 653660e3ba9e..972bb0f9f994 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2585,10 +2585,8 @@ static int msi_ht_cap_enabled(struct pci_dev *dev)
 /* Check the HyperTransport MSI mapping to know whether MSI is enabled or not */
 static void quirk_msi_ht_cap(struct pci_dev *dev)
 {
-	if (dev->subordinate && !msi_ht_cap_enabled(dev)) {
-		pci_warn(dev, "MSI quirk detected; subordinate MSI disabled\n");
-		dev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
-	}
+	if (!msi_ht_cap_enabled(dev))
+		quirk_disable_msi(dev);
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SERVERWORKS, PCI_DEVICE_ID_SERVERWORKS_HT2000_PCIE,
 			quirk_msi_ht_cap);
@@ -2601,9 +2599,6 @@ static void quirk_nvidia_ck804_msi_ht_cap(struct pci_dev *dev)
 {
 	struct pci_dev *pdev;
 
-	if (!dev->subordinate)
-		return;
-
 	/*
 	 * Check HT MSI cap on this chipset and the root one.  A single one
 	 * having MSI is enough to be sure that MSI is supported.
@@ -2611,10 +2606,8 @@ static void quirk_nvidia_ck804_msi_ht_cap(struct pci_dev *dev)
 	pdev = pci_get_slot(dev->bus, 0);
 	if (!pdev)
 		return;
-	if (!msi_ht_cap_enabled(dev) && !msi_ht_cap_enabled(pdev)) {
-		pci_warn(dev, "MSI quirk detected; subordinate MSI disabled\n");
-		dev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
-	}
+	if (!msi_ht_cap_enabled(pdev))
+		quirk_msi_ht_cap(dev);
 	pci_dev_put(pdev);
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_CK804_PCIE,
-- 
2.29.2

Re: [PATCH 00/13] PCI: MSI: Getting rid of msi_controller, and other cleanups

[Bjorn Helgaas]

On Thu, Feb 25, 2021 at 03:10:10PM +0000, Marc Zyngier wrote:
> The msi_controller data structure was the first attempt at treating
> MSIs like any other interrupt. We replaced it a few years ago with the
> generic MSI framework, but as it turns out, some older drivers are
> still using it.
> 
> This series aims at converting these stragglers, drop msi_controller,
> and fix some other nits such as having ways for a host bridge to
> advertise whether it supports MSIs or not.
> 
> A few notes:
> 
> - The Tegra patch is the result of back and forth work with Thierry: I
>   wrote the initial patch, which didn't work (I didn't have any HW at
>   the time). Thierry made it work, and I subsequently fixed a couple
>   of bugs/cleanups. I'm responsible for the result, so don't blame
>   Thierry for any of it! FWIW, I'm now running a Jetson TX2 with its
>   root fs over NVME, and MSIs are OK.
> 
> - RCAR is totally untested, though Marek had a go at a previous
>   version. More testing required.
> 
> - The xilinx stuff is *really* untested. Paul, if you have a RISC-V
>   board that uses it, could you please give it a go? Michal, same
>   thing for the stuff you have at hand...
> 
> - hyperv: I don't have access to such hypervisor, and no way to test
>   it. Help welcomed.
> 
> - The patches dealing with the advertising of MSI handling are the
>   result of a long discussion that took place here[1]. I took the
>   liberty to rejig Thomas' initial patches, and add what I needed for
>   the MSI domain stuff. Again, blame me if something is wrong, and not
>   Thomas.
> 
> Feedback welcome.
> 
> 	M.
> 
> [1] https://lore.kernel.org/r/20201031140330.83768-1-linux@fw-web.de
> 
> Marc Zyngier (11):
>   PCI: tegra: Convert to MSI domains
>   PCI: rcar: Convert to MSI domains
>   PCI: xilinx: Convert to MSI domains
>   PCI: hyperv: Drop msi_controller structure
>   PCI: MSI: Drop use of msi_controller from core code
>   PCI: MSI: Kill msi_controller structure
>   PCI: MSI: Kill default_teardown_msi_irqs()
>   PCI: MSI: Let PCI host bridges declare their reliance on MSI domains
>   PCI: Make pci_host_common_probe() declare its reliance on MSI domains
>   PCI: MSI: Document the various ways of ending up with NO_MSI
>   PCI: quirks: Refactor advertising of the NO_MSI flag
> 
> Thomas Gleixner (2):
>   PCI: MSI: Let PCI host bridges declare their lack of MSI handling
>   PCI: mediatek: Advertise lack of MSI handling

All looks good to me; I'm guessing Lorenzo will want to apply it or at
least take a look since the bulk of this is in the native host
drivers.

s|PCI: MSI:|PCI/MSI:| above (I use "PCI/<FEATURE>:" and "PCI: <driver>:")
s|PCI: hyperv:|PCI: hv:| to match previous practice

Maybe:

  PCI: Refactor HT advertising of NO_MSI flag

since "HT" contains more information than "quirks"?

In the 03/13 commit log, s/appaling/appalling/ :)
In the patch, it sounds like the MSI capture address change might be
separable into its own patch?  If it were separate, it would be easier
to see the problem/fix and watch for it elsewhere.

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

>  drivers/pci/controller/Kconfig           |   4 +-
>  drivers/pci/controller/pci-host-common.c |   1 +
>  drivers/pci/controller/pci-hyperv.c      |   4 -
>  drivers/pci/controller/pci-tegra.c       | 343 ++++++++++++-----------
>  drivers/pci/controller/pcie-mediatek.c   |   4 +
>  drivers/pci/controller/pcie-rcar-host.c  | 342 +++++++++++-----------
>  drivers/pci/controller/pcie-xilinx.c     | 238 +++++++---------
>  drivers/pci/msi.c                        |  46 +--
>  drivers/pci/probe.c                      |   4 +-
>  drivers/pci/quirks.c                     |  15 +-
>  include/linux/msi.h                      |  17 +-
>  include/linux/pci.h                      |   4 +-
>  12 files changed, 463 insertions(+), 559 deletions(-)
> 
> -- 
> 2.29.2
>