| Message ID | 54651BE2.9080008@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Thu, 13 Nov 2014, Marc Zyngier wrote: > With the new stacked irq domains, it becomes pretty tempting > to allocate an MSI domain per PCI bus, which would remove > the requirement of either relying on arch-specific code, or > a default PCI MSI domain. Right. That's what I roughly had in mind. And that would solve the multi-iommu issue on x86 nicely as well. We establish the association at the time where the bus gets populated. So the whole lookup magic simply goes away. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 13/11/14 21:11, Thomas Gleixner wrote: > On Thu, 13 Nov 2014, Marc Zyngier wrote: >> With the new stacked irq domains, it becomes pretty tempting >> to allocate an MSI domain per PCI bus, which would remove >> the requirement of either relying on arch-specific code, or >> a default PCI MSI domain. > > Right. That's what I roughly had in mind. And that would solve the > multi-iommu issue on x86 nicely as well. We establish the association > at the time where the bus gets populated. So the whole lookup magic > simply goes away. Great. I've pushed the whole thing out with this patch, the couple of fixes I mentioned earlier, as well as the whole ITS code: git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/stacked-its-v2 Time to go home. M.
On 2014/11/14 5:00, Marc Zyngier wrote: > On 13/11/14 11:43, Jiang Liu wrote: >> This patch set is based on tip/irq/irqdomain and tries to refine >> interfaces to support irqdomain for generic MSI and PCI MSI. >> >> With this patch set applied, the generic MSI and PCI MSI interfaces >> are much easier to use. For extreme case, you only need to define >> a "struct msi_domain_info" and don't need to implement any callbacks, >> just using the default callbacks is OK:) >> >> This patch set is also a preparation for: >> 1) Kill all weak functions in drivers/pci/msi.c >> 2) Implement support for non-PCI-compliant MSI device > > I've rebased (once more!) the GICv3 ITS driver on top of this, and this > is definitely a major improvement. This is basically the first version > I can use without having to hack into the core code (apart from the > couple of nits I've mentioned earlier). Sorry for the rebasing, but I hope it worthy rebasing:) > > Now, Thomas' idea of putting the irq_domain close to the bus is very > appealing, and I've tweaked an earlier patch in order to do this: I feel that's the right direction. There are other threads discussing associating an MSI controller structure with each PCI bus (at least root bus). http://www.spinics.net/lists/arm-kernel/msg376328.html Regards! Gerry -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014/11/14 8:25, Jiang Liu wrote: > On 2014/11/14 5:00, Marc Zyngier wrote: >> On 13/11/14 11:43, Jiang Liu wrote: >>> This patch set is based on tip/irq/irqdomain and tries to refine >>> interfaces to support irqdomain for generic MSI and PCI MSI. >>> >>> With this patch set applied, the generic MSI and PCI MSI interfaces >>> are much easier to use. For extreme case, you only need to define >>> a "struct msi_domain_info" and don't need to implement any callbacks, >>> just using the default callbacks is OK:) >>> >>> This patch set is also a preparation for: >>> 1) Kill all weak functions in drivers/pci/msi.c >>> 2) Implement support for non-PCI-compliant MSI device >> >> I've rebased (once more!) the GICv3 ITS driver on top of this, and this >> is definitely a major improvement. This is basically the first version >> I can use without having to hack into the core code (apart from the >> couple of nits I've mentioned earlier). > Sorry for the rebasing, but I hope it worthy rebasing:) > >> >> Now, Thomas' idea of putting the irq_domain close to the bus is very >> appealing, and I've tweaked an earlier patch in order to do this: > I feel that's the right direction. There are other threads discussing > associating an MSI controller structure with each PCI bus (at least > root bus). > http://www.spinics.net/lists/arm-kernel/msg376328.html Associate the irq domain and PCI bus is not necessary, because all PCI buses under same host bridge always share same MSI chip/irq domain, we only need associate them and pci host bridge. I'm refactoring the pci_host_bridge, make it be a generic one, rip out of the pci root bus creation, so we could put the irq domain and pci domain etc.. in it. Finally, we could eliminate lots platform arch functions. I will post it out within one week. Thanks! Yijing. > Regards! > Gerry > > . >
On 2014/11/14 9:09, Yijing Wang wrote: > On 2014/11/14 8:25, Jiang Liu wrote: >> On 2014/11/14 5:00, Marc Zyngier wrote: >>> On 13/11/14 11:43, Jiang Liu wrote: >>>> This patch set is based on tip/irq/irqdomain and tries to refine >>>> interfaces to support irqdomain for generic MSI and PCI MSI. >>>> >>>> With this patch set applied, the generic MSI and PCI MSI interfaces >>>> are much easier to use. For extreme case, you only need to define >>>> a "struct msi_domain_info" and don't need to implement any callbacks, >>>> just using the default callbacks is OK:) >>>> >>>> This patch set is also a preparation for: >>>> 1) Kill all weak functions in drivers/pci/msi.c >>>> 2) Implement support for non-PCI-compliant MSI device >>> >>> I've rebased (once more!) the GICv3 ITS driver on top of this, and this >>> is definitely a major improvement. This is basically the first version >>> I can use without having to hack into the core code (apart from the >>> couple of nits I've mentioned earlier). >> Sorry for the rebasing, but I hope it worthy rebasing:) >> >>> >>> Now, Thomas' idea of putting the irq_domain close to the bus is very >>> appealing, and I've tweaked an earlier patch in order to do this: >> I feel that's the right direction. There are other threads discussing >> associating an MSI controller structure with each PCI bus (at least >> root bus). >> http://www.spinics.net/lists/arm-kernel/msg376328.html > > Associate the irq domain and PCI bus is not necessary, because all PCI buses under same host bridge > always share same MSI chip/irq domain, we only need associate them and pci host bridge. > I'm refactoring the pci_host_bridge, make it be a generic one, rip out of the pci root bus > creation, so we could put the irq domain and pci domain etc.. in it. Finally, we could > eliminate lots platform arch functions. I will post it out within one week. Hi Yijing, Theoretically, it's not true on x86 when interrupt remapping is enabled. There may be multiple IOMMU(interrupt remapping) units serving the same host bridge, so we need different MSI domains to serve different PCI devices. Regards! Gerry > > Thanks! > Yijing. > >> Regards! >> Gerry >> >> . >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 14 Nov 2014, Yijing Wang wrote: Could you please use a mail client which does proper line wraps or configure yours to do so? > Associate the irq domain and PCI bus is not necessary, because all > PCI buses under same host bridge always share same MSI chip/irq > domain, we only need associate them and pci host bridge. > > I'm refactoring the pci_host_bridge, make it be a generic one, rip > out of the pci root bus creation, so we could put the irq domain and > pci domain etc.. in it. Finally, we could eliminate lots platform > arch functions. I will post it out within one week. That's a completely orthogonal problem. From the MSI/interrupt handling POV it does not matter at all where that information is stored. All we care about is that it is retrievable via the (pci) device which tries to setup MSI[X]. So we can store/retrieve it via generic functions into/from whatever is available right now. If the irq side has generic interfaces to do so then this wont conflict with your decisions to change the final storage point because all it takes is to tweak the storage/retrieve functions. So all we need at the moment is an agreed on way to store/retrieve that information which is based on the current shared infrastructure, aka. Linus tree. If we can utilize that you are completely free to change the association mechanism underneath. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014/11/14 9:31, Thomas Gleixner wrote: > On Fri, 14 Nov 2014, Yijing Wang wrote: > > Could you please use a mail client which does proper line wraps or > configure yours to do so? > >> Associate the irq domain and PCI bus is not necessary, because all >> PCI buses under same host bridge always share same MSI chip/irq >> domain, we only need associate them and pci host bridge. >> >> I'm refactoring the pci_host_bridge, make it be a generic one, rip >> out of the pci root bus creation, so we could put the irq domain and >> pci domain etc.. in it. Finally, we could eliminate lots platform >> arch functions. I will post it out within one week. > > That's a completely orthogonal problem. From the MSI/interrupt > handling POV it does not matter at all where that information is > stored. All we care about is that it is retrievable via the (pci) > device which tries to setup MSI[X]. > > So we can store/retrieve it via generic functions into/from whatever > is available right now. If the irq side has generic interfaces to do > so then this wont conflict with your decisions to change the final > storage point because all it takes is to tweak the storage/retrieve > functions. > > So all we need at the moment is an agreed on way to store/retrieve > that information which is based on the current shared infrastructure, > aka. Linus tree. If we can utilize that you are completely free to > change the association mechanism underneath. Hi Thomas, So we need something like: struct msi_chip *pci_get_msi_chip(struct pci_dev *); or: struct irq_domain *pci_get_msi_domain(struct pci_dev *); BTW, there's a conflict when merging tip/irq/irqdomain into tip/x86/apic. It's my first time to deal with merging conflicts, what's the preferred way? Is it working like this? 1) I merge the two branch 2) I rebase my x86 irqdomain patch sets and send them to you 3) You merge the two branch and apply my patch set. Regards! Gerry > > Thanks, > > tglx > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014/11/14 9:31, Thomas Gleixner wrote: > On Fri, 14 Nov 2014, Yijing Wang wrote: > > Could you please use a mail client which does proper line wraps or > configure yours to do so? > >> Associate the irq domain and PCI bus is not necessary, because all >> PCI buses under same host bridge always share same MSI chip/irq >> domain, we only need associate them and pci host bridge. >> >> I'm refactoring the pci_host_bridge, make it be a generic one, rip >> out of the pci root bus creation, so we could put the irq domain and >> pci domain etc.. in it. Finally, we could eliminate lots platform >> arch functions. I will post it out within one week. > > That's a completely orthogonal problem. From the MSI/interrupt > handling POV it does not matter at all where that information is > stored. All we care about is that it is retrievable via the (pci) > device which tries to setup MSI[X]. > > So we can store/retrieve it via generic functions into/from whatever > is available right now. If the irq side has generic interfaces to do > so then this wont conflict with your decisions to change the final > storage point because all it takes is to tweak the storage/retrieve > functions. > > So all we need at the moment is an agreed on way to store/retrieve > that information which is based on the current shared infrastructure, > aka. Linus tree. If we can utilize that you are completely free to > change the association mechanism underneath. Hi Thomas, thanks for your explanation. Now I got it, I will consider more about it. Thanks! Yijing. > > Thanks, > > tglx > > . >
On Fri, 14 Nov 2014, Jiang Liu wrote: > On 2014/11/14 9:31, Thomas Gleixner wrote: > > On Fri, 14 Nov 2014, Yijing Wang wrote: > > > > Could you please use a mail client which does proper line wraps or > > configure yours to do so? > > > >> Associate the irq domain and PCI bus is not necessary, because all > >> PCI buses under same host bridge always share same MSI chip/irq > >> domain, we only need associate them and pci host bridge. > >> > >> I'm refactoring the pci_host_bridge, make it be a generic one, rip > >> out of the pci root bus creation, so we could put the irq domain and > >> pci domain etc.. in it. Finally, we could eliminate lots platform > >> arch functions. I will post it out within one week. > > > > That's a completely orthogonal problem. From the MSI/interrupt > > handling POV it does not matter at all where that information is > > stored. All we care about is that it is retrievable via the (pci) > > device which tries to setup MSI[X]. > > > > So we can store/retrieve it via generic functions into/from whatever > > is available right now. If the irq side has generic interfaces to do > > so then this wont conflict with your decisions to change the final > > storage point because all it takes is to tweak the storage/retrieve > > functions. > > > > So all we need at the moment is an agreed on way to store/retrieve > > that information which is based on the current shared infrastructure, > > aka. Linus tree. If we can utilize that you are completely free to > > change the association mechanism underneath. > Hi Thomas, > So we need something like: > struct msi_chip *pci_get_msi_chip(struct pci_dev *); > or: > struct irq_domain *pci_get_msi_domain(struct pci_dev *); > > BTW, there's a conflict when merging tip/irq/irqdomain into > tip/x86/apic. It's my first time to deal with merging conflicts, > what's the preferred way? Is it working like this? > 1) I merge the two branch > 2) I rebase my x86 irqdomain patch sets and send them to you > 3) You merge the two branch and apply my patch set. When we have the generic parts sorted out, i'll make the irq/irqdomain branch official and immutable and then merge it into x86/apic fix the conflicts and add the x86 specific stuff on top. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
? 2014/11/14 9:39, Jiang Liu ??: > On 2014/11/14 9:31, Thomas Gleixner wrote: >> On Fri, 14 Nov 2014, Yijing Wang wrote: >> >> Could you please use a mail client which does proper line wraps or >> configure yours to do so? >> >>> Associate the irq domain and PCI bus is not necessary, because all >>> PCI buses under same host bridge always share same MSI chip/irq >>> domain, we only need associate them and pci host bridge. >>> >>> I'm refactoring the pci_host_bridge, make it be a generic one, rip >>> out of the pci root bus creation, so we could put the irq domain and >>> pci domain etc.. in it. Finally, we could eliminate lots platform >>> arch functions. I will post it out within one week. >> That's a completely orthogonal problem. From the MSI/interrupt >> handling POV it does not matter at all where that information is >> stored. All we care about is that it is retrievable via the (pci) >> device which tries to setup MSI[X]. >> >> So we can store/retrieve it via generic functions into/from whatever >> is available right now. If the irq side has generic interfaces to do >> so then this wont conflict with your decisions to change the final >> storage point because all it takes is to tweak the storage/retrieve >> functions. >> >> So all we need at the moment is an agreed on way to store/retrieve >> that information which is based on the current shared infrastructure, >> aka. Linus tree. If we can utilize that you are completely free to >> change the association mechanism underneath. > Hi Thomas, > So we need something like: > struct msi_chip *pci_get_msi_chip(struct pci_dev *); > or: > struct irq_domain *pci_get_msi_domain(struct pci_dev *); Hi Gerry, what about associate the platform specific struct msi_chip *pci_get_msi_chip(struct pci_dev *) with struct pci_host_bridge. we could provide the private "pci_get_msi_chip()" in the PCI host drivers. Thanks! Yijing. > > BTW, there's a conflict when merging tip/irq/irqdomain into > tip/x86/apic. It's my first time to deal with merging conflicts, > what's the preferred way? Is it working like this? > 1) I merge the two branch > 2) I rebase my x86 irqdomain patch sets and send them to you > 3) You merge the two branch and apply my patch set. > Regards! > Gerry >> Thanks, >> >> tglx >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014/11/14 22:11, Yijing Wang wrote: > > ? 2014/11/14 9:39, Jiang Liu ??: >> On 2014/11/14 9:31, Thomas Gleixner wrote: >>> On Fri, 14 Nov 2014, Yijing Wang wrote: >>> >> Hi Thomas, >> So we need something like: >> struct msi_chip *pci_get_msi_chip(struct pci_dev *); >> or: >> struct irq_domain *pci_get_msi_domain(struct pci_dev *); > > Hi Gerry, > what about associate the platform specific struct msi_chip > *pci_get_msi_chip(struct pci_dev *) > with struct pci_host_bridge. we could provide the private > "pci_get_msi_chip()" in the PCI > host drivers. Hi Yijing, Still need some time to dig into msi_chip related code. When refining the PCI MSI code, I feel the best way is: 1) Every PCI device is associated with an PCI MSI irqdomain. 2) PCI MSI core directly talks to irqdomain to allocate/free interrupts. 3) Kill all weak functions in pci/drivers/msi.c. 4) Kill struct msi_chip. We have achieved 1 and 2. And seems we could also achieve 3 by converting all arch specific PCI MSI code to use hierarchy irqdomain. But not sure whether we could achieve 4, not familiar with ARM world:) On x86, we could kill all PCI MSI weak function after converting Xen to irqdomain. I think we may have a prototype for x86 in next week. Thanks! Gerry -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14/11/14 14:26, Jiang Liu wrote: > On 2014/11/14 22:11, Yijing Wang wrote: >> >> ? 2014/11/14 9:39, Jiang Liu ??: >>> On 2014/11/14 9:31, Thomas Gleixner wrote: >>>> On Fri, 14 Nov 2014, Yijing Wang wrote: >>>> >>> Hi Thomas, >>> So we need something like: >>> struct msi_chip *pci_get_msi_chip(struct pci_dev *); >>> or: >>> struct irq_domain *pci_get_msi_domain(struct pci_dev *); >> >> Hi Gerry, >> what about associate the platform specific struct msi_chip >> *pci_get_msi_chip(struct pci_dev *) >> with struct pci_host_bridge. we could provide the private >> "pci_get_msi_chip()" in the PCI >> host drivers. > Hi Yijing, > Still need some time to dig into msi_chip related code. > When refining the PCI MSI code, I feel the best way is: > 1) Every PCI device is associated with an PCI MSI irqdomain. > 2) PCI MSI core directly talks to irqdomain to allocate/free > interrupts. > 3) Kill all weak functions in pci/drivers/msi.c. > 4) Kill struct msi_chip. > > We have achieved 1 and 2. And seems we could also achieve 3 by > converting all arch specific PCI MSI code to use hierarchy > irqdomain. But not sure whether we could achieve 4, not familiar > with ARM world:) Killing all the weak functions shouldn't be a problem for ARM, we're trying very hard not to rely on them. Killing msi_chip is a different story, as this is what we use to match a PCI host controller with its MSI controller (that's what the of_node field in msi_chip is for). See drivers/of/of_pci.c for details. Also, we use msi_chip directly in the MSI drivers as a way to go from a pci_dev to the MSI controller specific structure: http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/tree/drivers/irqchip/irq-gic-v3-its.c?h=irq/stacked-its-v2#n1143 If we're going to kill msi_chip, we must make sure we have mechanisms that allow the conversion of the existing code. Thanks, M.
On 2014/11/14 23:16, Marc Zyngier wrote: > On 14/11/14 14:26, Jiang Liu wrote: >> On 2014/11/14 22:11, Yijing Wang wrote: >>> >> We have achieved 1 and 2. And seems we could also achieve 3 by >> converting all arch specific PCI MSI code to use hierarchy >> irqdomain. But not sure whether we could achieve 4, not familiar >> with ARM world:) > > Killing all the weak functions shouldn't be a problem for ARM, we're > trying very hard not to rely on them. > > Killing msi_chip is a different story, as this is what we use to match a > PCI host controller with its MSI controller (that's what the of_node > field in msi_chip is for). See drivers/of/of_pci.c for details. > > Also, we use msi_chip directly in the MSI drivers as a way to go from a > pci_dev to the MSI controller specific structure: > > http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/tree/drivers/irqchip/irq-gic-v3-its.c?h=irq/stacked-its-v2#n1143 > > If we're going to kill msi_chip, we must make sure we have mechanisms > that allow the conversion of the existing code. Hi Marc, Seem it doesn't worth the effort to remove irq_chip, so I will focus on killing all weak functions first:) After that, we may kill setup_irq and teardown_irq in irq_chip, but seems irq_chip still has other usages, so I won't try to kill irq_chip. Regards! Gerry > > Thanks, > > M. > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014/11/14 5:28, Marc Zyngier wrote: > On 13/11/14 21:11, Thomas Gleixner wrote: >> On Thu, 13 Nov 2014, Marc Zyngier wrote: >>> With the new stacked irq domains, it becomes pretty tempting >>> to allocate an MSI domain per PCI bus, which would remove >>> the requirement of either relying on arch-specific code, or >>> a default PCI MSI domain. >> >> Right. That's what I roughly had in mind. And that would solve the >> multi-iommu issue on x86 nicely as well. We establish the association >> at the time where the bus gets populated. So the whole lookup magic >> simply goes away. > > Great. I've pushed the whole thing out with this patch, the couple > of fixes I mentioned earlier, as well as the whole ITS code: > > git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/stacked-its-v2 Hi Marc, I have looked at the code, and found some issues. 1) With my next version, no need to implemeent its_pci_msi_free() anymore. msi_domain_free_irqs() will reset desc->irq to zero for all architectures. 2) This piece of code in its_msi_prepare() may run into trouble for PCI device with both MSI and MSIX capability. I will change msi_prepare() prototype to pass in the "nvec" parameter. And you may access first_pci_msi_entry()->msi_attrib.is_msix to get allocation type if needed. nvec = pci_msix_vec_count(pdev); if (nvec < 0) nvec = pci_msi_vec_count(pdev); if (nvec < 0) return nvec; 3) Do we need to increase the default of NUM_MSI_ALLOC_SCRATCHPAD_REGS to 4? 2 is a little too limited. Regards! Gerry > > Time to go home. > > M. > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14/11/14 15:25, Jiang Liu wrote: > On 2014/11/14 23:16, Marc Zyngier wrote: >> On 14/11/14 14:26, Jiang Liu wrote: >>> On 2014/11/14 22:11, Yijing Wang wrote: >>>> >>> We have achieved 1 and 2. And seems we could also achieve 3 by >>> converting all arch specific PCI MSI code to use hierarchy >>> irqdomain. But not sure whether we could achieve 4, not familiar >>> with ARM world:) >> >> Killing all the weak functions shouldn't be a problem for ARM, we're >> trying very hard not to rely on them. >> >> Killing msi_chip is a different story, as this is what we use to match a >> PCI host controller with its MSI controller (that's what the of_node >> field in msi_chip is for). See drivers/of/of_pci.c for details. >> >> Also, we use msi_chip directly in the MSI drivers as a way to go from a >> pci_dev to the MSI controller specific structure: >> >> http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/tree/drivers/irqchip/irq-gic-v3-its.c?h=irq/stacked-its-v2#n1143 >> >> If we're going to kill msi_chip, we must make sure we have mechanisms >> that allow the conversion of the existing code. > Hi Marc, > Seem it doesn't worth the effort to remove irq_chip, > so I will focus on killing all weak functions first:) > After that, we may kill setup_irq and teardown_irq in irq_chip, > but seems irq_chip still has other usages, so I won't try to > kill irq_chip. I assume you mean msi_chip! ;-) Killing setup/teardown_irq is going to require some work. There are a few users in the tree (I can see three in drivers/pci/host and one in drivers/irqchip). Hopefully, we can get the maintainers to convert them. Thanks, M.
On 14/11/14 15:54, Jiang Liu wrote: > On 2014/11/14 5:28, Marc Zyngier wrote: >> On 13/11/14 21:11, Thomas Gleixner wrote: >>> On Thu, 13 Nov 2014, Marc Zyngier wrote: >>>> With the new stacked irq domains, it becomes pretty tempting >>>> to allocate an MSI domain per PCI bus, which would remove >>>> the requirement of either relying on arch-specific code, or >>>> a default PCI MSI domain. >>> >>> Right. That's what I roughly had in mind. And that would solve the >>> multi-iommu issue on x86 nicely as well. We establish the association >>> at the time where the bus gets populated. So the whole lookup magic >>> simply goes away. >> >> Great. I've pushed the whole thing out with this patch, the couple >> of fixes I mentioned earlier, as well as the whole ITS code: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/stacked-its-v2 > Hi Marc, > I have looked at the code, and found some issues. > 1) With my next version, no need to implemeent its_pci_msi_free() > anymore. msi_domain_free_irqs() will reset desc->irq to zero > for all architectures. Great. The less code, the better. > 2) This piece of code in its_msi_prepare() may run into trouble > for PCI device with both MSI and MSIX capability. I will change > msi_prepare() prototype to pass in the "nvec" parameter. And you > may access first_pci_msi_entry()->msi_attrib.is_msix to get > allocation type if needed. > nvec = pci_msix_vec_count(pdev); > if (nvec < 0) > nvec = pci_msi_vec_count(pdev); > if (nvec < 0) > return nvec; Ah, good point. Having nvec will make things simpler indeed. I have the feeling that the ITS driver may not be the only one to require such a thing. > 3) Do we need to increase the default of NUM_MSI_ALLOC_SCRATCHPAD_REGS > to 4? 2 is a little too limited. I'd say that 2 is enough for now. The ITS only uses one (the second one is for debug only). Should the need arise, we can bump it up later. Thanks, M.
Am Freitag, den 14.11.2014, 16:03 +0000 schrieb Marc Zyngier: > On 14/11/14 15:25, Jiang Liu wrote: > > On 2014/11/14 23:16, Marc Zyngier wrote: > >> On 14/11/14 14:26, Jiang Liu wrote: > >>> On 2014/11/14 22:11, Yijing Wang wrote: > >>>> > >>> We have achieved 1 and 2. And seems we could also achieve 3 by > >>> converting all arch specific PCI MSI code to use hierarchy > >>> irqdomain. But not sure whether we could achieve 4, not familiar > >>> with ARM world:) > >> > >> Killing all the weak functions shouldn't be a problem for ARM, we're > >> trying very hard not to rely on them. > >> > >> Killing msi_chip is a different story, as this is what we use to match a > >> PCI host controller with its MSI controller (that's what the of_node > >> field in msi_chip is for). See drivers/of/of_pci.c for details. > >> > >> Also, we use msi_chip directly in the MSI drivers as a way to go from a > >> pci_dev to the MSI controller specific structure: > >> > >> http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/tree/drivers/irqchip/irq-gic-v3-its.c?h=irq/stacked-its-v2#n1143 > >> > >> If we're going to kill msi_chip, we must make sure we have mechanisms > >> that allow the conversion of the existing code. > > Hi Marc, > > Seem it doesn't worth the effort to remove irq_chip, > > so I will focus on killing all weak functions first:) > > After that, we may kill setup_irq and teardown_irq in irq_chip, > > but seems irq_chip still has other usages, so I won't try to > > kill irq_chip. > > I assume you mean msi_chip! ;-) > > Killing setup/teardown_irq is going to require some work. There are a > few users in the tree (I can see three in drivers/pci/host and one in > drivers/irqchip). > > Hopefully, we can get the maintainers to convert them. > I can help with both the Designware and Tegra host bridge drivers. But that requires: 1. CCing patches to me, so I know what is going on 2. Providing good descriptions on what is done in the patchset and how the msi_chip users should adjust With all the MSI changes by various people going on in parallel it's hard to follow for someone not looking at this stuff every day. Regards, Lucas
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 9947fb4..1752537 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -86,9 +86,14 @@ int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type) static int pci_msi_setup_msi_irqs(struct pci_dev *dev, int nvec, int type) { - struct irq_domain *domain; + struct irq_domain *domain = NULL; - domain = arch_get_pci_msi_domain(dev); +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN + if (dev->bus->msi) + domain = dev->bus->msi->domain; +#endif + if (!domain) + domain = arch_get_pci_msi_domain(dev); if (domain) return pci_msi_domain_alloc_irqs(domain, type, dev); diff --git a/include/linux/msi.h b/include/linux/msi.h index 9b2e73e..2325950 100644 --- a/include/linux/msi.h +++ b/include/linux/msi.h @@ -89,6 +89,9 @@ struct msi_chip { struct device *dev; struct device_node *of_node; struct list_head list; +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN + struct irq_domain *domain; +#endif int (*setup_irq)(struct msi_chip *chip, struct pci_dev *dev, struct msi_desc *desc);