From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 82C54C433C1 for ; Mon, 22 Mar 2021 12:25:27 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 027576198B for ; Mon, 22 Mar 2021 12:25:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 027576198B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=nZ7R0kwjCe2uN1/1Xayx3EjNuvWKVpXOPJWCmFe2mCk=; b=CXMRFFfa0bdYfCrxW8MQVU5WI I+cBhpaAFM4TT6CSo2kRJUmmrAWbBAn56ax7qeiB2wM7hZWuhcieNcW5xa9CQEeFrb3BV/ZeQgAX6 LTjTFfKKI0zs/xE+u7/W0A1OhHcggaZwOKn///vFbaYfZm4smBcdaQhixWYe3GGa0IXnTARrx2V6Y ZiNYrl79hkGaV0LVTHNuoXg+6X88gCjo5/VgTAkIn9RxnvOmtDXc2Kv+uvpY5bEd3KP9ID0lDX1ls b3C/uNUAtsVah2Je1XzzeXuVkCoQ3G4vFBrirFFRlcy8CuruZBjS6UFU7zN0EArzejFN6u8pWvp+M BcnLyreow==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lOJai-00BZNZ-1y; Mon, 22 Mar 2021 12:23:28 +0000 Received: from foss.arm.com ([217.140.110.172]) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lOJab-00BZMJ-V9; Mon, 22 Mar 2021 12:23:24 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 827931063; Mon, 22 Mar 2021 05:23:20 -0700 (PDT) Received: from e121166-lin.cambridge.arm.com (e121166-lin.cambridge.arm.com [10.1.196.255]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 812573F718; Mon, 22 Mar 2021 05:23:17 -0700 (PDT) Date: Mon, 22 Mar 2021 12:23:15 +0000 From: Lorenzo Pieralisi To: Marc Zyngier Cc: Bjorn Helgaas , Frank Wunderlich , Thierry Reding , Thomas Gleixner , Rob Herring , Will Deacon , "K. Y. Srinivasan" , Haiyang Zhang , Stephen Hemminger , Wei Liu , Thierry Reding , Jonathan Hunter , Ryder Lee , Marek Vasut , Yoshihiro Shimoda , Michal Simek , Paul Walmsley , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-hyperv@vger.kernel.org, linux-tegra@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-renesas-soc@vger.kernel.org Subject: Re: [PATCH 03/13] PCI: xilinx: Convert to MSI domains Message-ID: <20210322122315.GB11469@e121166-lin.cambridge.arm.com> References: <20210225151023.3642391-1-maz@kernel.org> <20210225151023.3642391-4-maz@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210225151023.3642391-4-maz@kernel.org> User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210322_122322_672812_2AE75123 X-CRM114-Status: GOOD ( 41.63 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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 > --- > 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, ®s); > @@ -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 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel