On Thu, Dec 02, 2021 at 08:25:48PM +0100, Thomas Gleixner wrote: > Jason, > > On Thu, Dec 02 2021 at 09:55, Jason Gunthorpe wrote: > > On Thu, Dec 02, 2021 at 01:01:42AM +0100, Thomas Gleixner wrote: > >> On Wed, Dec 01 2021 at 21:21, Thomas Gleixner wrote: > >> > On Wed, Dec 01 2021 at 14:14, Jason Gunthorpe wrote: > >> > Which in turn is consistent all over the place and does not require any > >> > special case for anything. Neither for interrupts nor for anything else. > >> > >> that said, feel free to tell me that I'm getting it all wrong. > >> > >> The reason I'm harping on this is that we are creating ABIs on several > >> ends and we all know that getting that wrong is a major pain. > > > > I don't really like coupling the method to fetch IRQs with needing > > special struct devices. Struct devices have a sysfs presence and it is > > not always appropriate to create sysfs stuff just to allocate some > > IRQs. > > > > A queue is simply not a device, it doesn't make any sense. A queue is > > more like a socket(). > > Let's put the term 'device' for a bit please. > > > That said, we often have enough struct devices floating about to make > > this work. Between netdev/ib_device/aux device/mdev we can use them to > > do this. > > > > I think it is conceptual nonsense to attach an IMS IRQ domain to a > > netdev or a cdev, but it will solve this problem. > > The IMS irqdomain is _NOT_ part of the netdev or cdev or whatever. I > explained that several times now. > > We seem to have a serious problem of terminology and the understanding > of topology which is why we continue to talk past each other forever. I think I understand and agree with everything you said below. The point we diverge is where to put the vector storage: > Of course we can store them in pci_dev.dev.msi.data.store. Either with a > dedicated xarray or by partitioning the xarray space. Both have their > pro and cons. This decision seems to drive the question of how many 'struct devices' do we need, and where do we get them.. > But what I really struggle with is how this is further represented when > the queues are allocated for VFIO, cdev, whatever usage. Yes, this seems to be the primary question > The fact that the irqdomain is instantiated by the device driver does > not make it any different. As explained above it's just an > implementation detail which makes it possible to handle the device > specific message storage requirement in a sane way. The actual interrupt > resources come still from the underlying irqdomains as for PCI/MSI. Yes! This is not under debate > Now I was looking for a generic representation of such a container and > my initial reaction was to bind it to a struct device, which also makes > it trivial to store these MSI descriptors in that struct device. Right, I've been trying to argue around just this choice. > I can understand your resistance against that to some extent, but I > think we really have to come up with a proper abstract representation > for these. Ok > Such a logical function would be the entity to hand out for VFIO or > cdev. What is a logical function, concretely? Does it have struct device? Can I instead suggest a name like 'message interrupt table' ? Ie a device has two linearly indexed message interrupt tables - the PCI SIG defined MSI/MSI-X one created by the PCI core and the IMS one created by the driver. Both start at 0 index and they have different irq_domains. Instead of asking the driver to create a domain we ask the driver to create a new 'message interrupt table'. The driver provides the irq_chip to program the messages and the pci_device. The core code manages the irq domain setup. Using what you say below: > If this is not split out, then every driver and wrapper has to come up > with it's own representation of this instead of being able to do: > > request_irq(msi_get_virq(lfunc, idx=0), handler0, ...); > request_irq(msi_get_virq(lfunc, idx=1), handler1, ...); We could say: msi_get_virq(device.pci_msi_table, index=0) Is the 0th PCI SIG MSI vector Something like: ims_table = pci_create_msi_table(pci_dev, my_irq_chip,..) msi_get_virq(ims_table, index=0) Is the 0th IMS vector Is it close to what you are thinking with lfunc? Regards, Jason
