Message ID | 20200617162938.743439-3-clg@kaod.org (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | powerpc/pci: unmap interrupts when a PHB is removed | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (c3405d517d606e965030026daec198d314f20195) |
snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 57 lines checked |
snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On 18/06/2020 02:29, Cédric Le Goater wrote: > Some PCI adapters, like GPUs, use the "interrupt-map" property to > describe interrupt mappings other than the legacy INTx interrupts. > There can be more than 4 mappings. > > To clear all interrupts when a PHB is removed, we need to increase the > 'irq_map' array in which mappings are recorded. Compute the number of > interrupt mappings from the "interrupt-map" property and allocate a > bigger 'irq_map' array. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > arch/powerpc/kernel/pci-common.c | 49 +++++++++++++++++++++++++++++++- > 1 file changed, 48 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c > index 515480a4bac6..deb831f0ae13 100644 > --- a/arch/powerpc/kernel/pci-common.c > +++ b/arch/powerpc/kernel/pci-common.c > @@ -353,9 +353,56 @@ struct pci_controller *pci_find_controller_for_domain(int domain_nr) > return NULL; > } > > +/* > + * Assumption is made on the interrupt parent. All interrupt-map > + * entries are considered to have the same parent. > + */ > +static int pcibios_irq_map_count(struct pci_controller *phb) I wonder if int of_irq_count(struct device_node *dev) could work here too. If it does not, then never mind. Other than that, the only other comment is - merge this one into 1/2 as 1/2 alone won't properly fix the problem but it may look like that it does: for phyp, the test machine just happens to have 4 entries in the map but this is the phyp implementation detail; for qemu, there are more but we only unregister 4 but kvm does not care in general so it is ok which is also implementation detail; and 2/2 just makes these details not matter. Thanks, > +{ > + const __be32 *imap; > + int imaplen; > + struct device_node *parent; > + u32 intsize, addrsize, parintsize, paraddrsize; > + > + if (of_property_read_u32(phb->dn, "#interrupt-cells", &intsize)) > + return 0; > + if (of_property_read_u32(phb->dn, "#address-cells", &addrsize)) > + return 0; > + > + imap = of_get_property(phb->dn, "interrupt-map", &imaplen); > + if (!imap) { > + pr_debug("%pOF : no interrupt-map\n", phb->dn); > + return 0; > + } > + imaplen /= sizeof(u32); > + pr_debug("%pOF : imaplen=%d\n", phb->dn, imaplen); > + > + if (imaplen < (addrsize + intsize + 1)) > + return 0; > + > + imap += intsize + addrsize; > + parent = of_find_node_by_phandle(be32_to_cpup(imap)); > + if (!parent) { > + pr_debug("%pOF : no imap parent found !\n", phb->dn); > + return 0; > + } > + > + if (of_property_read_u32(parent, "#interrupt-cells", &parintsize)) { > + pr_debug("%pOF : parent lacks #interrupt-cells!\n", phb->dn); > + return 0; > + } > + > + if (of_property_read_u32(parent, "#address-cells", ¶ddrsize)) > + paraddrsize = 0; > + > + return imaplen / (addrsize + intsize + 1 + paraddrsize + parintsize); > +} > + > static void pcibios_irq_map_init(struct pci_controller *phb) > { > - phb->irq_count = PCI_NUM_INTX; > + phb->irq_count = pcibios_irq_map_count(phb); > + if (phb->irq_count < PCI_NUM_INTX) > + phb->irq_count = PCI_NUM_INTX; > > pr_debug("%pOF : interrupt map #%d\n", phb->dn, phb->irq_count); > >
On 8/7/20 8:01 AM, Alexey Kardashevskiy wrote: > > > On 18/06/2020 02:29, Cédric Le Goater wrote: >> Some PCI adapters, like GPUs, use the "interrupt-map" property to >> describe interrupt mappings other than the legacy INTx interrupts. >> There can be more than 4 mappings. >> >> To clear all interrupts when a PHB is removed, we need to increase the >> 'irq_map' array in which mappings are recorded. Compute the number of >> interrupt mappings from the "interrupt-map" property and allocate a >> bigger 'irq_map' array. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> arch/powerpc/kernel/pci-common.c | 49 +++++++++++++++++++++++++++++++- >> 1 file changed, 48 insertions(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c >> index 515480a4bac6..deb831f0ae13 100644 >> --- a/arch/powerpc/kernel/pci-common.c >> +++ b/arch/powerpc/kernel/pci-common.c >> @@ -353,9 +353,56 @@ struct pci_controller *pci_find_controller_for_domain(int domain_nr) >> return NULL; >> } >> >> +/* >> + * Assumption is made on the interrupt parent. All interrupt-map >> + * entries are considered to have the same parent. >> + */ >> +static int pcibios_irq_map_count(struct pci_controller *phb) > > I wonder if > int of_irq_count(struct device_node *dev) > could work here too. If it does not, then never mind. I wished it would, but no. > Other than that, the only other comment is - merge this one into 1/2 as > 1/2 alone won't properly fix the problem but it may look like that it does: > > for phyp, the test machine just happens to have 4 entries in the map but > this is the phyp implementation detail; yes > for qemu, there are more but we only unregister 4 but kvm does not care > in general so it is ok which is also implementation detail; > > and 2/2 just makes these details not matter. Thanks, OK. It will ease backport. Sending a v2. Thanks for the review Alexey ! C. > > >> +{ >> + const __be32 *imap; >> + int imaplen; >> + struct device_node *parent; >> + u32 intsize, addrsize, parintsize, paraddrsize; >> + >> + if (of_property_read_u32(phb->dn, "#interrupt-cells", &intsize)) >> + return 0; >> + if (of_property_read_u32(phb->dn, "#address-cells", &addrsize)) >> + return 0; >> + >> + imap = of_get_property(phb->dn, "interrupt-map", &imaplen); >> + if (!imap) { >> + pr_debug("%pOF : no interrupt-map\n", phb->dn); >> + return 0; >> + } >> + imaplen /= sizeof(u32); >> + pr_debug("%pOF : imaplen=%d\n", phb->dn, imaplen); >> + >> + if (imaplen < (addrsize + intsize + 1)) >> + return 0; >> + >> + imap += intsize + addrsize; >> + parent = of_find_node_by_phandle(be32_to_cpup(imap)); >> + if (!parent) { >> + pr_debug("%pOF : no imap parent found !\n", phb->dn); >> + return 0; >> + } >> + >> + if (of_property_read_u32(parent, "#interrupt-cells", &parintsize)) { >> + pr_debug("%pOF : parent lacks #interrupt-cells!\n", phb->dn); >> + return 0; >> + } >> + >> + if (of_property_read_u32(parent, "#address-cells", ¶ddrsize)) >> + paraddrsize = 0; >> + >> + return imaplen / (addrsize + intsize + 1 + paraddrsize + parintsize); >> +} >> + >> static void pcibios_irq_map_init(struct pci_controller *phb) >> { >> - phb->irq_count = PCI_NUM_INTX; >> + phb->irq_count = pcibios_irq_map_count(phb); >> + if (phb->irq_count < PCI_NUM_INTX) >> + phb->irq_count = PCI_NUM_INTX; >> >> pr_debug("%pOF : interrupt map #%d\n", phb->dn, phb->irq_count); >> >> >
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index 515480a4bac6..deb831f0ae13 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -353,9 +353,56 @@ struct pci_controller *pci_find_controller_for_domain(int domain_nr) return NULL; } +/* + * Assumption is made on the interrupt parent. All interrupt-map + * entries are considered to have the same parent. + */ +static int pcibios_irq_map_count(struct pci_controller *phb) +{ + const __be32 *imap; + int imaplen; + struct device_node *parent; + u32 intsize, addrsize, parintsize, paraddrsize; + + if (of_property_read_u32(phb->dn, "#interrupt-cells", &intsize)) + return 0; + if (of_property_read_u32(phb->dn, "#address-cells", &addrsize)) + return 0; + + imap = of_get_property(phb->dn, "interrupt-map", &imaplen); + if (!imap) { + pr_debug("%pOF : no interrupt-map\n", phb->dn); + return 0; + } + imaplen /= sizeof(u32); + pr_debug("%pOF : imaplen=%d\n", phb->dn, imaplen); + + if (imaplen < (addrsize + intsize + 1)) + return 0; + + imap += intsize + addrsize; + parent = of_find_node_by_phandle(be32_to_cpup(imap)); + if (!parent) { + pr_debug("%pOF : no imap parent found !\n", phb->dn); + return 0; + } + + if (of_property_read_u32(parent, "#interrupt-cells", &parintsize)) { + pr_debug("%pOF : parent lacks #interrupt-cells!\n", phb->dn); + return 0; + } + + if (of_property_read_u32(parent, "#address-cells", ¶ddrsize)) + paraddrsize = 0; + + return imaplen / (addrsize + intsize + 1 + paraddrsize + parintsize); +} + static void pcibios_irq_map_init(struct pci_controller *phb) { - phb->irq_count = PCI_NUM_INTX; + phb->irq_count = pcibios_irq_map_count(phb); + if (phb->irq_count < PCI_NUM_INTX) + phb->irq_count = PCI_NUM_INTX; pr_debug("%pOF : interrupt map #%d\n", phb->dn, phb->irq_count);
Some PCI adapters, like GPUs, use the "interrupt-map" property to describe interrupt mappings other than the legacy INTx interrupts. There can be more than 4 mappings. To clear all interrupts when a PHB is removed, we need to increase the 'irq_map' array in which mappings are recorded. Compute the number of interrupt mappings from the "interrupt-map" property and allocate a bigger 'irq_map' array. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- arch/powerpc/kernel/pci-common.c | 49 +++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-)