On 16/04/15 03:57, Minghuan.Lian@xxxxxxxxxxxxx wrote: > Hi Marc, > > Please see my comments inline > >> -----Original Message----- From: Marc Zyngier >> [mailto:marc.zyngier@xxxxxxx] Sent: Thursday, April 16, 2015 12:37 >> AM To: Lian Minghuan-B31939 Cc: linux-pci@xxxxxxxxxxxxxxx; Arnd >> Bergmann; Hu Mingkai-B21284; Zang Roy-R61911; Yoder Stuart-B08248; >> Bjorn Helgaas; Wood Scott-B07421; linux- >> arm-kernel@xxxxxxxxxxxxxxxxxxx; Jason Cooper; Thomas Gleixner >> Subject: Re: [PATCH 1/2] irqchip/gicv3-its: Support share device >> ID >> >> On Wed, 15 Apr 2015 17:49:23 +0800 Minghuan Lian >> <Minghuan.Lian@xxxxxxxxxxxxx> wrote: >> >>> SMMU of some platforms can only isolate limited device ID. This >>> may require that all PCI devices share the same ITS device with >>> the fixed device ID. The patch adds function >>> arch_msi_share_devid_update used for these platforms to update >>> the fixed device ID and maximum MSI interrupts number. >>> >>> Signed-off-by: Minghuan Lian <Minghuan.Lian@xxxxxxxxxxxxx> --- >>> drivers/irqchip/irq-gic-v3-its.c | 11 +++++++++++ 1 file changed, >>> 11 insertions(+) >>> >>> diff --git a/drivers/irqchip/irq-gic-v3-its.c >>> b/drivers/irqchip/irq-gic-v3-its.c index d0374a6..be78d0a 100644 >>> --- a/drivers/irqchip/irq-gic-v3-its.c +++ >>> b/drivers/irqchip/irq-gic-v3-its.c @@ -1169,6 +1169,15 @@ static >>> int its_get_pci_alias(struct pci_dev *pdev, >> u16 alias, void *data) >>> return 0; } >>> >>> +void __weak +arch_msi_share_devid_update(struct pci_dev *pdev, >>> u32 *dev_id, u32 +*nvesc) { + /* + * use PCI_DEVID NOT share >>> device ID as default + * so nothing need to do + */ +} + >> >> NAK. On top of being ugly as sin, this breaks any form of >> multiplatform support. No way anything like this is going in. Guys, >> you really should know better. >> > [Minghuan] The current ITS MSI will create an individual ITS device > for each PCIe device, and use PCI_DEVID as ITS dev_id However, out > platform only supports ITS dev_id 0 -63. A normal PCIe DEVID of > 0000:01:00.0 is 256 bigger than 63. Besides, because of the limited > dev_id number, all the PCIe device will share the same ITS dev. Our > platform provides a hardware module LUT to map PCI DEVID to ITS > dev_id. So, when creating ITS device, we need to update dev_id and > the nvesc. I may change pci_for_each_dma_alias to add a new flag to > use alias_bus and alias_devfn. Yes, that's where you should take care of this hack. > But I also need to update nvesc which should contains all the PCI > device MSI/MSIX nvesc and PCIe PME, aerdrv interrupts. The main > difference is that we need a ITS device to service multiple PCIe > devices. Could you give me some suggestions how to implement this > requirement. It you take the time to read the code in its_msi_prepare(), you'll quickly notice that we already handle aliasing of multiple PCI devices to a single DeviceID. Once you have the aliasing taken care of in pci_for_each_dma_alias, the ITS code will automatically adjust the number of vectors (using its_get_pci_alias as a callback from pci_for_each_dma_alias). >>> static int its_msi_prepare(struct irq_domain *domain, struct >>> device *dev, int nvec, msi_alloc_info_t *info) { @@ -1185,6 >> +1194,8 @@ >>> static int its_msi_prepare(struct irq_domain *domain, struct >>> device *dev, dev_alias.count = nvec; >>> >>> pci_for_each_dma_alias(pdev, its_get_pci_alias, &dev_alias); + >>> arch_msi_share_devid_update(pdev, &dev_alias.dev_id, >>> +&dev_alias.count); + >> >> See the function above? That's where the aliasing should be taken >> care of. >> > [Minghuan] The alias will use dma_alias_devfn, but it does not > contains alias_bus. I need to translate PCI_DEVID to a fixed ID. Then add another flag to deal with that. Your hardware is "creative" (some might even argue it is broken), so deal with the creativity as a quirk, which has no business in the ITS driver (or any other driver). Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
