From: Marc Zyngier <maz@kernel.org> Date: 2021-02-25 15:11:56
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
From: Marc Zyngier <maz@kernel.org> Date: 2021-02-25 15:12:02
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 <redacted>
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(-)
@@ -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);+returnerr;}
@@ -1551,161 +1537,227 @@ static void tegra_pcie_pme_turnoff(struct tegra_pcie_port *port)afi_writel(pcie,val,AFI_PCIE_PME);}-staticinttegra_msi_alloc(structtegra_msi*chip)-{-intmsi;--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);--returnmsi;-}--staticvoidtegra_msi_free(structtegra_msi*chip,unsignedlongirq)+staticvoidtegra_pcie_msi_irq(structirq_desc*desc){-structdevice*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);-}--staticirqreturn_ttegra_pcie_msi_irq(intirq,void*data)-{-structtegra_pcie*pcie=data;-structdevice*dev=pcie->dev;+structtegra_pcie*pcie=irq_desc_get_handler_data(desc);+structirq_chip*chip=irq_desc_get_chip(desc);structtegra_msi*msi=&pcie->msi;-unsignedinti,processed=0;+structdevice*dev=pcie->dev;+unsignedinti;++chained_irq_enter(chip,desc);for(i=0;i<8;i++){-unsignedlongreg=afi_readl(pcie,AFI_MSI_VEC0+i*4);+unsignedlongreg=afi_readl(pcie,AFI_MSI_VEC(i));while(reg){unsignedintoffset=find_first_bit(®,32);unsignedintindex=i*32+offset;unsignedintirq;-/* 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'sweirdwhotriggeredthis?*justclearit*/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));}}-returnprocessed>0?IRQ_HANDLED:IRQ_NONE;+chained_irq_exit(chip,desc);}-staticinttegra_msi_setup_irq(structmsi_controller*chip,-structpci_dev*pdev,structmsi_desc*desc)+staticvoidtegra_msi_top_irq_ack(structirq_data*d){-structtegra_msi*msi=to_tegra_msi(chip);-structmsi_msgmsg;-unsignedintirq;-inthwirq;+irq_chip_ack_parent(d);+}-hwirq=tegra_msi_alloc(msi);-if(hwirq<0)-returnhwirq;+staticvoidtegra_msi_top_irq_mask(structirq_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;-}+staticvoidtegra_msi_top_irq_unmask(structirq_data*d)+{+pci_msi_unmask_irq(d);+irq_chip_unmask_parent(d);+}++staticstructirq_chiptegra_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);+staticvoidtegra_msi_irq_ack(structirq_data*d)+{+structtegra_msi*msi=irq_data_get_irq_chip_data(d);+structtegra_pcie*pcie=msi_to_pcie(msi);+unsignedintindex=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);+staticvoidtegra_msi_irq_mask(structirq_data*d)+{+structtegra_msi*msi=irq_data_get_irq_chip_data(d);+structtegra_pcie*pcie=msi_to_pcie(msi);+unsignedintindex=d->hwirq/32;+unsignedlongflags;+u32value;-return0;+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);+}++staticvoidtegra_msi_irq_unmask(structirq_data*d)+{+structtegra_msi*msi=irq_data_get_irq_chip_data(d);+structtegra_pcie*pcie=msi_to_pcie(msi);+unsignedintindex=d->hwirq/32;+unsignedlongflags;+u32value;++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);+}++staticinttegra_msi_set_affinity(structirq_data*d,conststructcpumask*mask,boolforce)+{+return-EINVAL;}-staticvoidtegra_msi_teardown_irq(structmsi_controller*chip,-unsignedintirq)+staticvoidtegra_compose_msi_msg(structirq_data*data,structmsi_msg*msg){-structtegra_msi*msi=to_tegra_msi(chip);-structirq_data*d=irq_get_irq_data(irq);-irq_hw_number_thwirq=irqd_to_hwirq(d);+structtegra_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;}-staticstructirq_chiptegra_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,+staticstructirq_chiptegra_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,};-staticinttegra_msi_map(structirq_domain*domain,unsignedintirq,-irq_hw_number_thwirq)+staticinttegra_msi_domain_alloc(structirq_domain*domain,unsignedintvirq,+unsignedintnr_irqs,void*args){-irq_set_chip_and_handler(irq,&tegra_msi_irq_chip,handle_simple_irq);-irq_set_chip_data(irq,domain->host_data);+structtegra_msi*msi=domain->host_data;+unsignedinti;+inthwirq;++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();return0;}-staticconststructirq_domain_opsmsi_domain_ops={-.map=tegra_msi_map,+staticvoidtegra_msi_domain_free(structirq_domain*domain,unsignedintvirq,+unsignedintnr_irqs)+{+structirq_data*d=irq_domain_get_irq_data(domain,virq);+structtegra_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);+}++staticconststructirq_domain_opstegra_msi_domain_ops={+.alloc=tegra_msi_domain_alloc,+.free=tegra_msi_domain_free,+};++staticstructmsi_domain_infotegra_msi_info={+.flags=(MSI_FLAG_USE_DEF_DOM_OPS|MSI_FLAG_USE_DEF_CHIP_OPS|+MSI_FLAG_PCI_MSIX),+.chip=&tegra_msi_top_chip,};+staticinttegra_allocate_domains(structtegra_msi*msi)+{+structtegra_pcie*pcie=msi_to_pcie(msi);+structfwnode_handle*fwnode=dev_fwnode(pcie->dev);+structirq_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;+}++return0;+}++staticvoidtegra_free_domains(structtegra_msi*msi)+{+structirq_domain*parent=msi->domain->parent;++irq_domain_remove(msi->domain);+irq_domain_remove(parent);+}+staticinttegra_pcie_msi_setup(structtegra_pcie*pcie){-structpci_host_bridge*host=pci_host_bridge_from_priv(pcie);structplatform_device*pdev=to_platform_device(pcie->dev);structtegra_msi*msi=&pcie->msi;structdevice*dev=pcie->dev;interr;-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)+returnerr;}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);-gotofree_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*facilitateendpointsthatsupportonly32-bitMSItargetaddress,
@@ -1740,14 +1787,14 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie)gotofree_irq;}-host->msi=&msi->chip;-return0;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);+returnerr;}
@@ -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;
From: Marc Zyngier <maz@kernel.org> Date: 2021-02-25 15:12:37
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(-)
@@ -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;returnpci_host_probe(bridge);}
@@ -473,42 +470,6 @@ static int rcar_pcie_phy_init_gen3(struct rcar_pcie_host *host)returnerr;}-staticintrcar_msi_alloc(structrcar_msi*chip)-{-intmsi;--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);--returnmsi;-}--staticintrcar_msi_alloc_region(structrcar_msi*chip,intno_irqs)-{-intmsi;--mutex_lock(&chip->lock);-msi=bitmap_find_free_region(chip->used,INT_PCI_MSI_NR,-order_base_2(no_irqs));-mutex_unlock(&chip->lock);--returnmsi;-}--staticvoidrcar_msi_free(structrcar_msi*chip,unsignedlongirq)-{-mutex_lock(&chip->lock);-clear_bit(irq,chip->used);-mutex_unlock(&chip->lock);-}-staticirqreturn_trcar_pcie_msi_irq(intirq,void*data){structrcar_pcie_host*host=data;
@@ -527,18 +488,13 @@ static irqreturn_t rcar_pcie_msi_irq(int irq, void *data)unsignedintindex=find_first_bit(®,32);unsignedintmsi_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)returnIRQ_HANDLED;}-staticintrcar_msi_setup_irq(structmsi_controller*chip,structpci_dev*pdev,-structmsi_desc*desc)+staticvoidrcar_msi_top_irq_ack(structirq_data*d){-structrcar_msi*msi=to_rcar_msi(chip);-structrcar_pcie_host*host=container_of(chip,structrcar_pcie_host,-msi.chip);-structrcar_pcie*pcie=&host->pcie;-structmsi_msgmsg;-unsignedintirq;-inthwirq;+irq_chip_ack_parent(d);+}-hwirq=rcar_msi_alloc(msi);-if(hwirq<0)-returnhwirq;+staticvoidrcar_msi_top_irq_mask(structirq_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;-}+staticvoidrcar_msi_top_irq_unmask(structirq_data*d)+{+pci_msi_unmask_irq(d);+irq_chip_unmask_parent(d);+}-irq_set_msi_desc(irq,desc);+staticstructirq_chiprcar_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,+};++staticvoidrcar_msi_irq_ack(structirq_data*d)+{+structrcar_msi*msi=irq_data_get_irq_chip_data(d);+structrcar_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);+staticvoidrcar_msi_irq_mask(structirq_data*d)+{+structrcar_msi*msi=irq_data_get_irq_chip_data(d);+structrcar_pcie*pcie=&msi_to_host(msi)->pcie;+unsignedlongflags;+u32value;-return0;+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);}-staticintrcar_msi_setup_irqs(structmsi_controller*chip,-structpci_dev*pdev,intnvec,inttype)+staticvoidrcar_msi_irq_unmask(structirq_data*d){-structrcar_msi*msi=to_rcar_msi(chip);-structrcar_pcie_host*host=container_of(chip,structrcar_pcie_host,-msi.chip);-structrcar_pcie*pcie=&host->pcie;-structmsi_desc*desc;-structmsi_msgmsg;-unsignedintirq;-inthwirq;-inti;+structrcar_msi*msi=irq_data_get_irq_chip_data(d);+structrcar_pcie*pcie=&msi_to_host(msi)->pcie;+unsignedlongflags;+u32value;-/* 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,structmsi_desc,list);+staticintrcar_msi_set_affinity(structirq_data*d,conststructcpumask*mask,boolforce)+{+return-EINVAL;+}-hwirq=rcar_msi_alloc_region(msi,nvec);-if(hwirq<0)-return-ENOSPC;+staticvoidrcar_compose_msi_msg(structirq_data*data,structmsi_msg*msg)+{+structrcar_msi*msi=irq_data_get_irq_chip_data(data);+unsignedlongpa=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()calledfromrcar_pcie_probe()pre--*allocatesdescs,sothereisnoneedtoallocatedescshere.-*Wecanthereforeassumethatifirq_find_mapping()above-*returnsnon-zero,thenthedescsarealsosuccessfully-*allocated.-*/-if(irq_set_msi_desc_off(irq,i,desc)){-/* TODO: clear */-return-EINVAL;-}-}+staticstructirq_chiprcar_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,+};++staticintrcar_msi_domain_alloc(structirq_domain*domain,unsignedintvirq,+unsignedintnr_irqs,void*args)+{+structrcar_msi*msi=domain->host_data;+unsignedinti;+inthwirq;-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);return0;}-staticvoidrcar_msi_teardown_irq(structmsi_controller*chip,unsignedintirq)+staticvoidrcar_msi_domain_free(structirq_domain*domain,unsignedintvirq,+unsignedintnr_irqs){-structrcar_msi*msi=to_rcar_msi(chip);-structirq_data*d=irq_get_irq_data(irq);--rcar_msi_free(msi,d->hwirq);-}+structirq_data*d=irq_domain_get_irq_data(domain,virq);+structrcar_msi*msi=domain->host_data;-staticstructirq_chiprcar_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);-staticintrcar_msi_map(structirq_domain*domain,unsignedintirq,-irq_hw_number_thwirq)-{-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));-return0;+mutex_unlock(&msi->map_lock);}-staticconststructirq_domain_opsmsi_domain_ops={-.map=rcar_msi_map,+staticconststructirq_domain_opsrcar_msi_domain_ops={+.alloc=rcar_msi_domain_alloc,+.free=rcar_msi_domain_free,};-staticvoidrcar_pcie_unmap_msi(structrcar_pcie_host*host)+staticstructmsi_domain_inforcar_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,+};++staticintrcar_allocate_domains(structrcar_msi*msi){-structrcar_msi*msi=&host->msi;-inti,irq;+structrcar_pcie*pcie=&msi_to_host(msi)->pcie;+structfwnode_handle*fwnode=dev_fwnode(pcie->dev);+structirq_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;+}++return0;}-staticvoidrcar_pcie_hw_enable_msi(structrcar_pcie_host*host)+staticvoidrcar_free_domains(structrcar_msi*msi){-structrcar_pcie*pcie=&host->pcie;-structrcar_msi*msi=&host->msi;-unsignedlongbase;--/* 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);+structirq_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);}staticintrcar_pcie_enable_msi(structrcar_pcie_host*host)
@@ -698,29 +675,20 @@ static int rcar_pcie_enable_msi(struct rcar_pcie_host *host)structrcar_pcie*pcie=&host->pcie;structdevice*dev=pcie->dev;structrcar_msi*msi=&host->msi;-interr,i;--mutex_init(&msi->lock);+unsignedlongbase;+interr;-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)+returnerr;/* 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);gotoerr;
@@ -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);gotoerr;}-/* 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);return0;err:-rcar_pcie_unmap_msi(host);+rcar_free_domains(msi);returnerr;}staticvoidrcar_pcie_teardown_msi(structrcar_pcie_host*host){structrcar_pcie*pcie=&host->pcie;-structrcar_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);}staticintrcar_pcie_get_resources(structrcar_pcie_host*host)
From: Marek Vasut <hidden> Date: 2021-02-28 18:49:08
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.
From: Marc Zyngier <maz@kernel.org> Date: 2021-02-25 15:12:57
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(-)
From: Michael Kelley <hidden> Date: 2021-03-01 04:25:56
From: Marc Zyngier <maz@kernel.org> Sent: Thursday, February 25, 2021 7:10 AM
quoted hunk
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(-)
@@ -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 <redacted>
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 <redacted>
From: Marc Zyngier <maz@kernel.org> Date: 2021-02-25 15:13:00
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(-)
@@ -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_tpa=virt_to_phys(port);-ret=xilinx_pcie_enable_msi(port);+ret=xilinx_allocate_msi_domains(port);if(ret)returnret;++pcie_write(port,upper_32_bits(pa),XILINX_PCIE_REG_MSIBASE1);+pcie_write(port,lower_32_bits(pa),XILINX_PCIE_REG_MSIBASE2);}return0;
@@ -572,6 +523,7 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)structdevice*dev=port->dev;structdevice_node*node=dev->of_node;structresourceregs;+unsignedintirq;interr;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))returnPTR_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);returnerr;}
@@ -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-returnpci_host_probe(bridge);+err=pci_host_probe(bridge);+if(err)+xilinx_free_msi_domains(port);++returnerr;}staticconststructof_device_idxilinx_pcie_of_match[]={
From: Lorenzo Pieralisi <hidden> Date: 2021-03-22 12:21:53
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
@@ -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_tpa=virt_to_phys(port);-ret=xilinx_pcie_enable_msi(port);+ret=xilinx_allocate_msi_domains(port);if(ret)returnret;++pcie_write(port,upper_32_bits(pa),XILINX_PCIE_REG_MSIBASE1);+pcie_write(port,lower_32_bits(pa),XILINX_PCIE_REG_MSIBASE2);}return0;
@@ -572,6 +523,7 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)structdevice*dev=port->dev;structdevice_node*node=dev->of_node;structresourceregs;+unsignedintirq;interr;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))returnPTR_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);returnerr;}
@@ -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-returnpci_host_probe(bridge);+err=pci_host_probe(bridge);+if(err)+xilinx_free_msi_domains(port);++returnerr;}staticconststructof_device_idxilinx_pcie_of_match[]={
From: Michal Simek <monstr@monstr.eu> Date: 2021-03-22 12:37:40
Hi,
On 3/22/21 1:21 PM, Lorenzo Pieralisi wrote:
On Thu, Feb 25, 2021 at 03:10:13PM +0000, Marc Zyngier wrote:
quoted
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
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-22 14:05:54
On Mon, 22 Mar 2021 12:33:10 +0000,
Michal Simek [off-list ref] wrote:
Hi,
On 3/22/21 1:21 PM, Lorenzo Pieralisi wrote:
quoted
On Thu, Feb 25, 2021 at 03:10:13PM +0000, Marc Zyngier wrote:
quoted
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.
From: Lorenzo Pieralisi <hidden> Date: 2021-03-22 12:24:34
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
@@ -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_tpa=virt_to_phys(port);-ret=xilinx_pcie_enable_msi(port);+ret=xilinx_allocate_msi_domains(port);if(ret)returnret;++pcie_write(port,upper_32_bits(pa),XILINX_PCIE_REG_MSIBASE1);+pcie_write(port,lower_32_bits(pa),XILINX_PCIE_REG_MSIBASE2);}return0;
@@ -572,6 +523,7 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)structdevice*dev=port->dev;structdevice_node*node=dev->of_node;structresourceregs;+unsignedintirq;interr;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))returnPTR_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);returnerr;}
@@ -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-returnpci_host_probe(bridge);+err=pci_host_probe(bridge);+if(err)+xilinx_free_msi_domains(port);++returnerr;}staticconststructof_device_idxilinx_pcie_of_match[]={
From: Michal Simek <monstr@monstr.eu> Date: 2021-03-22 12:38:43
On 3/22/21 1:23 PM, Lorenzo Pieralisi wrote:
On Thu, Feb 25, 2021 at 03:10:13PM +0000, Marc Zyngier wrote:
quoted
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.
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-22 14:04:51
Hi Lorenzo,
On Mon, 22 Mar 2021 12:23:15 +0000,
Lorenzo Pieralisi [off-list ref] wrote:
On Thu, Feb 25, 2021 at 03:10:13PM +0000, Marc Zyngier wrote:
quoted
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.
From: Marc Zyngier <maz@kernel.org> Date: 2021-02-25 15:13:56
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(-)
@@ -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
@@ -540,7 +540,6 @@ struct pci_host_bridge {int(*map_irq)(conststructpci_dev*,u8,u8);void(*release_fn)(structpci_host_bridge*);void*release_data;-structmsi_controller*msi;unsignedintignore_reset_delay:1;/* For entire hierarchy */unsignedintno_ext_tags:1;/* No Extended Tags */unsignedintnative_aer:1;/* OS may use PCIe AER */
@@ -621,7 +620,6 @@ struct pci_bus {structresourcebusn_res;/* Bus numbers routed to this bus */structpci_ops*ops;/* Configuration access functions */-structmsi_controller*msi;/* MSI controller */void*sysdata;/* Hook for sys-specific extension */structproc_dir_entry*procdir;/* Directory entry in /proc/bus/pci */
From: Marc Zyngier <maz@kernel.org> Date: 2021-02-25 15:14:45
From: Thomas Gleixner <redacted>
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 <redacted>
[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(+)
@@ -550,6 +550,7 @@ struct pci_host_bridge {unsignedintnative_dpc:1;/* OS may use PCIe DPC */unsignedintpreserve_config:1;/* Preserve FW resource setup */unsignedintsize_windows:1;/* Enable root bus sizing */+unsignedintno_msi:1;/* Bridge has no MSI support *//* Resource alignment requirements */resource_size_t(*align_resource)(structpci_dev*dev,
From: Marc Zyngier <maz@kernel.org> Date: 2021-02-25 15:15:04
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(-)
@@ -94,11 +94,7 @@ int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)return0;}-/*-*Wehaveadefaultimplementationavailableasaseparatenon-weak-*function,asitisusedbytheXenx86PCIcode-*/-voiddefault_teardown_msi_irqs(structpci_dev*dev)+void__weakarch_teardown_msi_irqs(structpci_dev*dev){inti;structmsi_desc*entry;
From: Marc Zyngier <maz@kernel.org> Date: 2021-02-25 15:15:54
From: Thomas Gleixner <redacted>
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 <redacted>
Signed-off-by: Thomas Gleixner <redacted>
[maz: commit message]
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/pci/controller/pcie-mediatek.c | 4 ++++
1 file changed, 4 insertions(+)
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 <redacted>
We definitely need to understand the above.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
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.
quoted
Tried with an mt7612e, this seems to work without any errors.
so for mt7622/mt7623
Tested-by: Frank Wunderlich <redacted>
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)
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-01 13:32:59
Frank,
quoted
quoted
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.
quoted
This r64 machine is supposed to have working MSIs, right?
imho mt7622 have working MSI
quoted
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.
quoted
quoted
Tried with an mt7612e, this seems to work without any errors.
so for mt7622/mt7623
Tested-by: Frank Wunderlich <redacted>
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...
From: Frank Wunderlich <hidden> Date: 2021-03-01 14:11:17
Gesendet: Montag, 01. März 2021 um 14:31 Uhr
Von: "Marc Zyngier" [off-list ref]
Frank,
quoted
quoted
quoted
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
quoted
quoted
This r64 machine is supposed to have working MSIs, right?
imho mt7622 have working MSI
quoted
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
quoted
quoted
quoted
Tried with an mt7612e, this seems to work without any errors.
so for mt7622/mt7623
Tested-by: Frank Wunderlich <redacted>
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.
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-03-02 11:02:22
On 2021-03-01 14:06, Frank Wunderlich wrote:
quoted
Gesendet: Montag, 01. März 2021 um 14:31 Uhr
Von: "Marc Zyngier" [off-list ref]
Frank,
quoted
quoted
quoted
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.
quoted
quoted
quoted
This r64 machine is supposed to have working MSIs, right?
imho mt7622 have working MSI
quoted
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
quoted
quoted
quoted
quoted
Tried with an mt7612e, this seems to work without any errors.
so for mt7622/mt7623
Tested-by: Frank Wunderlich <redacted>
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.
From: Marc Zyngier <maz@kernel.org> Date: 2021-02-25 15:24:40
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(-)
@@ -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 */staticvoidquirk_msi_ht_cap(structpci_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);
From: Marc Zyngier <maz@kernel.org> Date: 2021-02-25 15:25:03
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(-)
From: Marc Zyngier <maz@kernel.org> Date: 2021-02-25 15:25:46
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(+)
From: Marc Zyngier <maz@kernel.org> Date: 2021-02-25 15:27:15
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(-)
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>