>>> The method you describe here makes sense for PCI devices that are required to support >>> legacy interrupts and may or may not support MSI on a given system, but not so much >>> for platform devices for which we know exactly whether we want to use MSI >>> or legacy interrupts. >>> >>> In particular if you have a device that can only do MSI, the entire pci_enable_msi >>> step is pointless: all we need to do is program the correct MSI target address/message >>> pair into the device and register the handler. >> >> Yes, I almost agree if we won't change the existing hundreds of drivers, what >> I worried about is some drivers may want to know the IRQ numbers, and use the IRQ >> number to process different things, as I mentioned in another reply. >> But we can also provide the interface which integrate MSI configuration and request_irq(), >> if most drivers don't care the IRQ number. > > The driver would still have the option of getting the IRQ number for now: With > the interface I imagine, you would get a 'struct msi_desc' pointer, from which > you can look up the 'struct irq_desc' pointer (either embedded in msi_desc, > or using a pointer from a member of msi_desc), and you can already get the > interrupt number from the irq_desc. > > My point was that a well-written driver already does not care about the interrupt > number: the only information a driver needs in the interrupt handler is a pointer > to its own context, which we already derive from the irq_desc. Agree, I will try to introduce this similar interface in next version, thanks! > > The main interface that currently requires the irq number is free_irq(), but > I would argue that we can just add a wrapper that takes the msi_desc pointer > as its first argument so the driver does not have to worry about it. > > We can add additional wrappers like that as needed. OK >>>> This is a huge change for device drivers, and some device drivers don't know which msi_chip >>>> their MSI irq deliver to. I'm reworking the msi_chip, and try to use msi_chip to eliminate >>>> all arch_msi_xxx() under every arch in kernel. And the important point is how to create the >>>> binding for the MSI device to the target msi_chip. >>> >>> Which drivers are you thinking of? Again, I wouldn't expect to change any PCI drivers, >>> but only platform drivers that do native MSI, so we only have to change drivers that >>> do not support any MSI at all yet and that need to be changed anyway in order to add >>> support. >> >> I mean platform device drivers, because we can find the target msi_chip by some platform >> interfaces(like the existing of_pci_find_msi_chip_by_node()). So we no need to explicitly >> provide the msi_chip as the function argument. > > Right, that works too. I was thinking we might need an interface that allows us to > pick a particular msi_chip if there are several alternatives (e.g. one in the GIC > and one in the PCI host), but you are right: we should normally be able to hardwire > that information in DT or elsewhere, and just need the 'struct device pointer' which > should probably be the first argument here. > > As you pointed out, it's common to have multiple MSIs for a single device, so we > also need a context to pass around, so my suggestion would become something like: > > struct msi_desc *msi_request(struct device *dev, irq_handler_t handler, > unsigned long flags, const char *name, void *data); > > It's possible that we have to add one or two more arguments here. Good suggestion, thanks! > >>> A degenerate case of this would be a system where a PCI device sends its MSI into >>> the host controller, that generates a legacy interrupt and that in turn gets >>> sent to an irqchip which turns it back into an MSI for the GICv3. This would of >>> course be very inefficient, but I think we should be able to express this with >>> both the binding and the in-kernel framework just to be on the safe side. >> >> Yes, the best way to tell the kernel which msi_chip should deliver to is describe >> the binding in DTS file. If a real degenerate case found, we can update the platform >> interface which is responsible for getting the match msi_chip in future. > > Ok. > > Arnd > > . > -- Thanks! Yijing _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
