From: Marc Zyngier <maz@kernel.org> Date: 2021-03-22 18:47:15
This is a respin of the series described at [1].
* From v1:
- Extracted the changes dealing with the MSI capture address
for rcar and xilinx and moved them to separate patches
- Changed the rcar code to cope with c4e0fec2f7ee ("PCI: rcar: Always
allocate MSI addresses in 32bit space")
- Fixed rcar resume code
- Reworked commit messages
- Rebased onto v5.12-rc4
- Collected Acks, and TBs, with thanks.
[1] https://lore.kernel.org/r/20210225151023.3642391-1-maz@kernel.org
Marc Zyngier (13):
PCI: tegra: Convert to MSI domains
PCI: rcar: Don't allocate extra memory for the MSI capture address
PCI: rcar: Convert to MSI domains
PCI: xilinx: Don't allocate extra memory for the MSI capture address
PCI: xilinx: Convert to MSI domains
PCI: hv: 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/MSI: Make pci_host_common_probe() declare its reliance on MSI
domains
PCI/MSI: Document the various ways of ending up with NO_MSI
PCI: Refactor HT advertising of 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 | 356 +++++++++++------------
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, 477 insertions(+), 559 deletions(-)
--
2.29.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-22 18:47:17
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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
[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-03-22 18:47:17
A long cargo-culted behaviour of PCI drivers is to allocate memory
to obtain an address that is fed to the controller as the MSI
capture address (i.e. the MSI doorbell).
But there is no actual requirement for this address to be RAM.
All it needs to be is a suitable aligned address that will
*not* be DMA'd to.
Since the rcar platform already has a requirement that this
address should be in the first 4GB of the physical address space,
use the controller's own base address as the capture address.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/pci/controller/pcie-rcar-host.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-22 18:47:18
A long cargo-culted behaviour of PCI drivers is to allocate memory
to obtain an address that is fed to the controller as the MSI
capture address (i.e. the MSI doorbell).
But there is no actual requirement for this address to be RAM.
All it needs to be is a suitable aligned address that will
*not* be DMA'd to.
Use the physical address of the 'port' data structure as the MSI
capture address.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/pci/controller/pcie-xilinx.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
Subject: [PATCH v2 04/15] PCI: xilinx: Don't allocate extra memory for the
MSI capture address
A long cargo-culted behaviour of PCI drivers is to allocate memory to obtain
an address that is fed to the controller as the MSI capture address (i.e. the
MSI doorbell).
But there is no actual requirement for this address to be RAM.
All it needs to be is a suitable aligned address that will
*not* be DMA'd to.
Use the physical address of the 'port' data structure as the MSI capture
address.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/pci/controller/pcie-xilinx.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
The XILINX_PCIE_REG_MSIBASE2 register expects 4KB aligned address.
The lower 12-bits are always set to 0 in this register. So we need to mask the address
while programming address to EP.
#define XILINX_PCIE_MSI_ADDR_MASK GENMASK(31, 12)
msg.address_lo = lower_32_bits(msg_addr) & XILINX_PCIE_MSI_ADDR_MASK;
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-24 12:56:30
On Wed, 24 Mar 2021 12:35:58 +0000,
Bharat Kumar Gogada [off-list ref] wrote:
Thanks Marc for the patch.
quoted
Subject: [PATCH v2 04/15] PCI: xilinx: Don't allocate extra memory for the
MSI capture address
A long cargo-culted behaviour of PCI drivers is to allocate memory to obtain
an address that is fed to the controller as the MSI capture address (i.e. the
MSI doorbell).
But there is no actual requirement for this address to be RAM.
All it needs to be is a suitable aligned address that will
*not* be DMA'd to.
Use the physical address of the 'port' data structure as the MSI capture
address.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/pci/controller/pcie-xilinx.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
The XILINX_PCIE_REG_MSIBASE2 register expects 4KB aligned address.
The lower 12-bits are always set to 0 in this register. So we need
to mask the address while programming address to
Thanks for the heads up, I'll fix this up. Does it work correctly once
the address is aligned?
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-22 18:47:18
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 appalling 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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/pci/controller/Kconfig | 2 +-
drivers/pci/controller/pcie-xilinx.c | 234 +++++++++++----------------
2 files changed, 97 insertions(+), 139 deletions(-)
@@ -507,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;
@@ -566,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);
@@ -578,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;}
@@ -611,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);
@@ -631,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[]={
--
2.29.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Subject: [PATCH v2 05/15] PCI: xilinx: Convert to MSI domains
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 appalling 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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/pci/controller/Kconfig | 2 +-
drivers/pci/controller/pcie-xilinx.c | 234 +++++++++++----------------
2 files changed, 97 insertions(+), 139 deletions(-)
I see a crash while testing MSI in handle_edge_irq
[<c015bdd4>] (handle_edge_irq) from [<c0157164>] (generic_handle_irq+0x28/0x38)
[<c0157164>] (generic_handle_irq) from [<c03a9714>] (xilinx_pcie_intr_handler+0x17c/0x2b0)
[<c03a9714>] (xilinx_pcie_intr_handler) from [<c0157d94>] (__handle_irq_event_percpu+0x3c/0xc0)
[<c0157d94>] (__handle_irq_event_percpu) from [<c0157e44>] (handle_irq_event_percpu+0x2c/0x7c)
[<c0157e44>] (handle_irq_event_percpu) from [<c0157ecc>] (handle_irq_event+0x38/0x5c)
[<c0157ecc>] (handle_irq_event) from [<c015bc8c>] (handle_fasteoi_irq+0x9c/0x114)
void handle_edge_irq(struct irq_desc *desc) {
...
kstat_incr_irqs_this_cpu(desc);
/* Start handling the irq */
desc->irq_data.chip->irq_ack(&desc->irq_data); //There is no check here for ack function is registered for chip
..
}
-/**
- * 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;
+
+ mutex_lock(&port->map_lock);
+
+ hwirq = bitmap_find_free_region(port->msi_map,
XILINX_NUM_MSI_IRQS,
+order_base_2(nr_irqs));
+
+ mutex_unlock(&port->map_lock);
- hwirq = xilinx_pcie_assign_msi();
if (hwirq < 0)
- return hwirq;
+ return -ENOSPC;
- irq = irq_create_mapping(port->msi_domain, hwirq);
- if (!irq)
- return -EINVAL;
+ for (i = 0; i < nr_irqs; i++)
+ irq_domain_set_info(domain, virq + i, hwirq + i,
+ &xilinx_msi_bottom_chip, domain-
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-24 13:17:38
On Wed, 24 Mar 2021 12:42:24 +0000,
Bharat Kumar Gogada [off-list ref] wrote:
Hi Marc,
Thanks for the patch.
quoted
Subject: [PATCH v2 05/15] PCI: xilinx: Convert to MSI domains
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 appalling 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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/pci/controller/Kconfig | 2 +-
drivers/pci/controller/pcie-xilinx.c | 234 +++++++++++----------------
2 files changed, 97 insertions(+), 139 deletions(-)
I see a crash while testing MSI in handle_edge_irq
[<c015bdd4>] (handle_edge_irq) from [<c0157164>] (generic_handle_irq+0x28/0x38)
[<c0157164>] (generic_handle_irq) from [<c03a9714>] (xilinx_pcie_intr_handler+0x17c/0x2b0)
[<c03a9714>] (xilinx_pcie_intr_handler) from [<c0157d94>] (__handle_irq_event_percpu+0x3c/0xc0)
[<c0157d94>] (__handle_irq_event_percpu) from [<c0157e44>] (handle_irq_event_percpu+0x2c/0x7c)
[<c0157e44>] (handle_irq_event_percpu) from [<c0157ecc>] (handle_irq_event+0x38/0x5c)
[<c0157ecc>] (handle_irq_event) from [<c015bc8c>] (handle_fasteoi_irq+0x9c/0x114)
Thanks for that. Can you please try the following patch and let me
know if it helps?
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Subject: [PATCH v2 05/15] PCI: xilinx: Convert to MSI domains
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 appalling 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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/pci/controller/Kconfig | 2 +-
drivers/pci/controller/pcie-xilinx.c | 234
+++++++++++----------------
2 files changed, 97 insertions(+), 139 deletions(-)
diff --git a/drivers/pci/controller/Kconfig
b/drivers/pci/controller/Kconfig index 5cc07d28a3a0..60045f7aafc5
100644
I see a crash while testing MSI in handle_edge_irq [<c015bdd4>]
(handle_edge_irq) from [<c0157164>] (generic_handle_irq+0x28/0x38)
[<c0157164>] (generic_handle_irq) from [<c03a9714>]
(xilinx_pcie_intr_handler+0x17c/0x2b0)
[<c03a9714>] (xilinx_pcie_intr_handler) from [<c0157d94>]
(__handle_irq_event_percpu+0x3c/0xc0)
[<c0157d94>] (__handle_irq_event_percpu) from [<c0157e44>]
(handle_irq_event_percpu+0x2c/0x7c)
[<c0157e44>] (handle_irq_event_percpu) from [<c0157ecc>]
(handle_irq_event+0x38/0x5c) [<c0157ecc>] (handle_irq_event) from
[<c015bc8c>] (handle_fasteoi_irq+0x9c/0x114)
Thanks for that. Can you please try the following patch and let me know if it
helps?
Thanks,
M.
@@ -468,7 +478,7 @@ static int xilinx_pcie_init_irq_domain(struct
xilinx_pcie_port *port)
/* Setup MSI */
if (IS_ENABLED(CONFIG_PCI_MSI)) {
- phys_addr_t pa = virt_to_phys(port);
+ phys_addr_t pa = ALIGN_DOWN(virt_to_phys(port), SZ_4K);
ret = xilinx_allocate_msi_domains(port);
if (ret)
Thanks Marc.
With above patch now everything works fine, tested a Samsung NVMe SSD.
tst~# lspci
00:00.0 PCI bridge: Xilinx Corporation Device 0706
01:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd NVMe SSD Controller 172Xa/172Xb (rev 01)
Regards,
Bharat
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
@@ -468,7 +478,7 @@ static int xilinx_pcie_init_irq_domain(struct
xilinx_pcie_port *port)
/* Setup MSI */
if (IS_ENABLED(CONFIG_PCI_MSI)) {
- phys_addr_t pa = virt_to_phys(port);
+ phys_addr_t pa = ALIGN_DOWN(virt_to_phys(port), SZ_4K);
ret = xilinx_allocate_msi_domains(port);
if (ret)
Thanks Marc.
With above patch now everything works fine, tested a Samsung NVMe SSD.
tst~# lspci
00:00.0 PCI bridge: Xilinx Corporation Device 0706
01:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd NVMe SSD Controller 172Xa/172Xb (rev 01)
Great, thanks for giving it a shot. Can I take this as a Tested-by:
tag?
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-22 18:47:18
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.
Reviewed-by: Michael Kelley <redacted>
Tested-by: Michael Kelley <redacted>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/pci/controller/pci-hyperv.c | 4 ----
1 file changed, 4 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-03-22 18:47:18
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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/pci/controller/Kconfig | 1 -
drivers/pci/controller/pcie-rcar-host.c | 352 ++++++++++++------------
2 files changed, 170 insertions(+), 183 deletions(-)
@@ -291,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);}
@@ -472,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;
@@ -526,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 */
@@ -547,150 +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,+};-msg.address_lo=rcar_pci_read_reg(pcie,PCIEMSIALR)&~MSIFE;-msg.address_hi=rcar_pci_read_reg(pcie,PCIEMSIAUR);-msg.data=hwirq;+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;-pci_write_msi_msg(irq,&msg);+/* clear the interrupt */+rcar_pci_write_reg(pcie,BIT(d->hwirq),PCIEMSIFR);+}-return0;+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;++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;++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);+}-/* MSI-X interrupts are not supported */-if(type==PCI_CAP_ID_MSIX)-return-EINVAL;+staticintrcar_msi_set_affinity(structirq_data*d,conststructcpumask*mask,boolforce)+{+return-EINVAL;+}-WARN_ON(!list_is_singular(&pdev->dev.msi_list));-desc=list_entry(pdev->dev.msi_list.next,structmsi_desc,list);+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);-hwirq=rcar_msi_alloc_region(msi,nvec);-if(hwirq<0)-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;+}-irq=irq_find_mapping(msi->domain,hwirq);-if(!irq)-return-ENOSPC;+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,+};-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;-}-}+staticintrcar_msi_domain_alloc(structirq_domain*domain,unsignedintvirq,+unsignedintnr_irqs,void*args)+{+structrcar_msi*msi=domain->host_data;+unsignedinti;+inthwirq;++mutex_lock(&msi->map_lock);-desc->nvec_used=nvec;-desc->msi_attrib.multiple=order_base_2(nvec);+hwirq=bitmap_find_free_region(msi->used,INT_PCI_MSI_NR,order_base_2(nr_irqs));-msg.address_lo=rcar_pci_read_reg(pcie,PCIEMSIALR)&~MSIFE;-msg.address_hi=rcar_pci_read_reg(pcie,PCIEMSIAUR);-msg.data=hwirq;+mutex_unlock(&msi->map_lock);++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,+&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);+structirq_data*d=irq_domain_get_irq_data(domain,virq);+structrcar_msi*msi=domain->host_data;-rcar_msi_free(msi,d->hwirq);-}--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,+};++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,};-staticvoidrcar_pcie_unmap_msi(structrcar_pcie_host*host)+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;++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);-for(i=0;i<INT_PCI_MSI_NR;i++){-irq=irq_find_mapping(msi->domain,i);-if(irq>0)-irq_dispose_mapping(irq);+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;}-irq_domain_remove(msi->domain);+return0;}-staticvoidrcar_pcie_hw_enable_msi(structrcar_pcie_host*host)+staticvoidrcar_free_domains(structrcar_msi*msi){-structrcar_pcie*pcie=&host->pcie;-structdevice*dev=pcie->dev;-structresourceres;+structirq_domain*parent=msi->domain->parent;-if(WARN_ON(of_address_to_resource(dev->of_node,0,&res)))-return;--/* setup MSI data target */-rcar_pci_write_reg(pcie,lower_32_bits(res.start)|MSIFE,PCIEMSIALR);-rcar_pci_write_reg(pcie,upper_32_bits(res.start),PCIEMSIAUR);--/* 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,24 @@ 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);+structresourceres;+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;-}+err=of_address_to_resource(dev->of_node,0,&res);+if(err)+returnerr;-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,19 +700,26 @@ 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 */-rcar_pcie_hw_enable_msi(host);+/* disable all MSIs */+rcar_pci_write_reg(pcie,0,PCIEMSIIER);++/*+*SetupMSIdatatargetusingRCbaseaddressaddress,which+*isguaranteedtobeinthelow32bitrangeonanyRCarHW.+*/+rcar_pci_write_reg(pcie,lower_32_bits(res.start)|MSIFE,PCIEMSIALR);+rcar_pci_write_reg(pcie,upper_32_bits(res.start),PCIEMSIAUR);return0;err:-rcar_pcie_unmap_msi(host);+rcar_free_domains(msi);returnerr;}
@@ -754,7 +733,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);-rcar_pcie_unmap_msi(host);+rcar_free_domains(&host->msi);}staticintrcar_pcie_get_resources(structrcar_pcie_host*host)
@@ -1007,8 +986,17 @@ static int __maybe_unused rcar_pcie_resume(struct device *dev)dev_info(dev,"PCIe x%d: link up\n",(data>>20)&0x3f);/* Enable MSI */-if(IS_ENABLED(CONFIG_PCI_MSI))-rcar_pcie_hw_enable_msi(host);+if(IS_ENABLED(CONFIG_PCI_MSI)){+structresourceres;+u32val;++of_address_to_resource(dev->of_node,0,&res);+rcar_pci_write_reg(pcie,upper_32_bits(res.start),PCIEMSIAUR);+rcar_pci_write_reg(pcie,lower_32_bits(res.start)|MSIFE,PCIEMSIALR);++bitmap_to_arr32(&val,host->msi.used,INT_PCI_MSI_NR);+rcar_pci_write_reg(pcie,val,PCIEMSIIER);+}rcar_pcie_hw_enable(host);
--
2.29.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-22 18:47:18
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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
include/linux/msi.h | 16 +---------------
1 file changed, 1 insertion(+), 15 deletions(-)
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-22 18:47:42
As there is no driver using msi_controller, we can now safely
remove its use from the PCI probe code.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
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 */
--
2.29.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-22 18:54:41
We have now 4 ways of ending up with NO_MSI being set.
Document them.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
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-03-22 18:54:43
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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
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,
--
2.29.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-22 18:54:43
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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
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(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-03-23 11:45:52
On 2021-03-22 18:46, Marc Zyngier wrote:
quoted hunk
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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
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(-)
@@ -551,6 +551,7 @@ struct pci_host_bridge {unsignedintpreserve_config:1;/* Preserve FW resource setup */unsignedintsize_windows:1;/* Enable root bus sizing */unsignedintno_msi:1;/* Bridge has no MSI support */+unsignedintmsi_domain:1;/* Bridge wants MSI domain */
Aren't these really the same thing? Either way we're saying the bridge
itself doesn't handle MSIs, it's just in one case we're effectively
encoding a platform-specific assumption that an external domain won't be
provided. I can't help wondering whether that distinction is really
necessary...
Robin.
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-23 18:10:41
Hi Robin,
On Tue, 23 Mar 2021 11:45:02 +0000,
Robin Murphy [off-list ref] wrote:
On 2021-03-22 18:46, Marc Zyngier wrote:
quoted
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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
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(-)
@@ -551,6 +551,7 @@ struct pci_host_bridge {unsignedintpreserve_config:1;/* Preserve FW resource setup */unsignedintsize_windows:1;/* Enable root bus sizing */unsignedintno_msi:1;/* Bridge has no MSI support */+unsignedintmsi_domain:1;/* Bridge wants MSI domain */
Aren't these really the same thing? Either way we're saying the bridge
itself doesn't handle MSIs, it's just in one case we're effectively
encoding a platform-specific assumption that an external domain won't
be provided. I can't help wondering whether that distinction is really
necessary...
There is a subtle difference: no_msi indicates that there is no way
*any* MSI can be dealt with whatsoever (maybe because the RC doesn't
forward the corresponding TLPs?). msi_domain says "no MSI unless...".
We could implement the former with the latter, but I have the feeling
that's not totally bullet proof. Happy to revisit this if you think it
really matters.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-03-23 19:05:47
On 2021-03-23 18:09, Marc Zyngier wrote:
Hi Robin,
On Tue, 23 Mar 2021 11:45:02 +0000,
Robin Murphy [off-list ref] wrote:
quoted
On 2021-03-22 18:46, Marc Zyngier wrote:
quoted
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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
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(-)
@@ -551,6 +551,7 @@ struct pci_host_bridge {unsignedintpreserve_config:1;/* Preserve FW resource setup */unsignedintsize_windows:1;/* Enable root bus sizing */unsignedintno_msi:1;/* Bridge has no MSI support */+unsignedintmsi_domain:1;/* Bridge wants MSI domain */
Aren't these really the same thing? Either way we're saying the bridge
itself doesn't handle MSIs, it's just in one case we're effectively
encoding a platform-specific assumption that an external domain won't
be provided. I can't help wondering whether that distinction is really
necessary...
There is a subtle difference: no_msi indicates that there is no way
*any* MSI can be dealt with whatsoever (maybe because the RC doesn't
forward the corresponding TLPs?). msi_domain says "no MSI unless...".
PCI says that MSIs are simply memory writes at the transaction level, so
AFAIK unless the host bridge can't support DMA at all, it shouldn't be
in a position to make any assumptions about what transactions mean what.
I suppose there could in theory be an issue in the other direction,
where config space somehow didn't allow access to the MSI capability in
the first place, but then we'd presumably just never detect any device
as being MSI-capable in the first place, and it wouldn't matter either way.
We could implement the former with the latter, but I have the feeling
that's not totally bullet proof. Happy to revisit this if you think it
really matters.
I would expect it to be a fairly safe assumption that a host bridge
which "doesn't support MSIs" wouldn't have an msi-parent set, but I
don't have a strong opinion either way - I just figured we could
probably save a little bit of complexity here.
Cheers,
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Lorenzo Pieralisi <hidden> Date: 2021-03-24 13:20:51
On Tue, Mar 23, 2021 at 06:09:36PM +0000, Marc Zyngier wrote:
Hi Robin,
On Tue, 23 Mar 2021 11:45:02 +0000,
Robin Murphy [off-list ref] wrote:
quoted
On 2021-03-22 18:46, Marc Zyngier wrote:
quoted
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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
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(-)
@@ -551,6 +551,7 @@ struct pci_host_bridge {unsignedintpreserve_config:1;/* Preserve FW resource setup */unsignedintsize_windows:1;/* Enable root bus sizing */unsignedintno_msi:1;/* Bridge has no MSI support */+unsignedintmsi_domain:1;/* Bridge wants MSI domain */
Aren't these really the same thing? Either way we're saying the bridge
itself doesn't handle MSIs, it's just in one case we're effectively
encoding a platform-specific assumption that an external domain won't
be provided. I can't help wondering whether that distinction is really
necessary...
There is a subtle difference: no_msi indicates that there is no way
*any* MSI can be dealt with whatsoever (maybe because the RC doesn't
forward the corresponding TLPs?). msi_domain says "no MSI unless...".
We could implement the former with the latter, but I have the feeling
that's not totally bullet proof. Happy to revisit this if you think it
really matters.
IIUC msi_domain == 1 means: this host bridge needs an msi_domain to enable
MSIs, which in turn means that there are bridges that do _not_ require
an msi_domain to enable MSIs. I don't know how other arches handle the
msi_domain pointer but I am asking whether making:
if (bridge->no_msi || !bus->dev.msi_domain))
bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
is a possibility (removing the need for the msi_domain flag).
At least this looks more like an arch property than a host bridge
specific property (eg patch [13] pci_host_common_probe() may be used on
arches other than ARM where it is not necessary true that it requires an
msi_domain to enable MSIs).
I agree that's complicated to untangle - just asking if there is way
to simplify it.
Thanks,
Lorenzo
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-24 16:12:58
On Wed, 24 Mar 2021 13:19:38 +0000,
Lorenzo Pieralisi [off-list ref] wrote:
On Tue, Mar 23, 2021 at 06:09:36PM +0000, Marc Zyngier wrote:
quoted
Hi Robin,
On Tue, 23 Mar 2021 11:45:02 +0000,
Robin Murphy [off-list ref] wrote:
quoted
On 2021-03-22 18:46, Marc Zyngier wrote:
quoted
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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
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(-)
@@ -551,6 +551,7 @@ struct pci_host_bridge {unsignedintpreserve_config:1;/* Preserve FW resource setup */unsignedintsize_windows:1;/* Enable root bus sizing */unsignedintno_msi:1;/* Bridge has no MSI support */+unsignedintmsi_domain:1;/* Bridge wants MSI domain */
Aren't these really the same thing? Either way we're saying the bridge
itself doesn't handle MSIs, it's just in one case we're effectively
encoding a platform-specific assumption that an external domain won't
be provided. I can't help wondering whether that distinction is really
necessary...
There is a subtle difference: no_msi indicates that there is no way
*any* MSI can be dealt with whatsoever (maybe because the RC doesn't
forward the corresponding TLPs?). msi_domain says "no MSI unless...".
We could implement the former with the latter, but I have the feeling
that's not totally bullet proof. Happy to revisit this if you think it
really matters.
IIUC msi_domain == 1 means: this host bridge needs an msi_domain to enable
MSIs, which in turn means that there are bridges that do _not_ require
an msi_domain to enable MSIs. I don't know how other arches handle the
msi_domain pointer but I am asking whether making:
if (bridge->no_msi || !bus->dev.msi_domain))
bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
is a possibility (removing the need for the msi_domain flag).
At least this looks more like an arch property than a host bridge
specific property (eg patch [13] pci_host_common_probe() may be used on
arches other than ARM where it is not necessary true that it requires an
msi_domain to enable MSIs).
I agree that's complicated to untangle - just asking if there is way
to simplify it.
I tried to simplify that in the past (see the original discussion at
[1]), and tglx reported some breakages on systems that do not use MSI
domains, which is why we ended up with an explicit flag.
What I have done for now is to go with Robin's proposal of dropping
'no_msi' and rely on solely on 'msi_domain' to set
PCI_BUS_FLAGS_NO_MSI when no domain is found.
Note that if we indeed have a host bridge that uses
pci_host_common_probe() that doesn't use MSI domains, we may indeed
run into problems. I don't have a good way around that, unfortunately.
Thanks,
M.
[1] https://lore.kernel.org/r/20201031140330.83768-1-linux@fw-web.de
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-22 18:54:43
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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
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-03-22 18:54:43
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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
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(+)
From: Marc Zyngier <maz@kernel.org> Date: 2021-03-22 18:54:43
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.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/pci/controller/pci-host-common.c | 1 +
1 file changed, 1 insertion(+)