| Message ID | 1568338328-22458-1-git-send-email-megha.dey@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add support for a new IMS interrupt mechanism | expand |
On Thu, Sep 12, 2019 at 06:32:01PM -0700, Megha Dey wrote: > This series is a basic patchset to get the ball rolling and receive some > inital comments. As per my discussion with Marc Zyngier and Thomas Gleixner > at the Linux Plumbers, I need to do the following: > 1. Since a device can support MSI-X and IMS simultaneously, ensure proper > locking mechanism for the 'msi_list' in the device structure. > 2. Introduce dynamic allocation of IMS vectors perhaps by using a group ID > 3. IMS support of a device needs to be discoverable. A bit in the vendor > specific capability in the PCI config is to be added rather than getting > this information from each device driver. Why #3? The point of this scheme is to delegate programming the addr/data pairs to the driver so it can be done in some device-specific way. There is no PCI standard behind this, and no change in PCI semantics. I think it would be a tall ask to get a config space bit from PCI-SIG for something that has little to do with PCI. After seeing that we already have a platform device based version of this same idea (drivers/base/platform-msi.c), I think the task here is really just to extend and expand that approach to work generically for platform and PCI devices. Along the way tidying the arch interface so x86 and ARM's stuff to support that uses the same generic interfaces. ie it is re-organizing code and ideas already in Linux, not defining some new standard. I also think refering to this existing idea by some new IMS name is only confusing people what the goal is... Which is perhaps why #3 was suggested?? Stated more clearly, I think all uses would be satisfied if platform_msi_domain_alloc_irqs() could be called for struct pci_device, could be called multiple times for the same struct pci_device, and co-existed with MSI and MSI-X on the same pci_device. Jason
On Fri, Sep 13, 2019 at 07:50:50PM +0000, Jason Gunthorpe wrote: > On Thu, Sep 12, 2019 at 06:32:01PM -0700, Megha Dey wrote: > > > This series is a basic patchset to get the ball rolling and receive some > > inital comments. As per my discussion with Marc Zyngier and Thomas Gleixner > > at the Linux Plumbers, I need to do the following: > > 1. Since a device can support MSI-X and IMS simultaneously, ensure proper > > locking mechanism for the 'msi_list' in the device structure. > > 2. Introduce dynamic allocation of IMS vectors perhaps by using a group ID > > 3. IMS support of a device needs to be discoverable. A bit in the vendor > > specific capability in the PCI config is to be added rather than getting > > this information from each device driver. > > Why #3? The point of this scheme is to delegate programming the > addr/data pairs to the driver so it can be done in some > device-specific way. There is no PCI standard behind this, and no > change in PCI semantics. > > I think it would be a tall ask to get a config space bit from PCI-SIG > for something that has little to do with PCI. This isn't a standard config capability. Its Designated Vendor Specific Capability (DVSEC). The device is responsible for managing the addr-data pair. This provides a hint to the OS framework that this device has device specific methods. Agreed its not required, but some OSV's like a generic way to discover these capabilities, hence its there so device vendors can have a common guideline. Check here for some of those details: https://software.intel.com/en-us/blogs/2018/06/25/introducing-intel-scalable-io-virtualization > > After seeing that we already have a platform device based version of > this same idea (drivers/base/platform-msi.c), I think the task here is > really just to extend and expand that approach to work generically for > platform and PCI devices. Along the way tidying the arch interface so > x86 and ARM's stuff to support that uses the same generic interfaces. > > ie it is re-organizing code and ideas already in Linux, not defining > some new standard. > > I also think refering to this existing idea by some new IMS name is > only confusing people what the goal is... Which is perhaps why #3 was > suggested?? > > Stated more clearly, I think all uses would be satisfied if > platform_msi_domain_alloc_irqs() could be called for struct > pci_device, could be called multiple times for the same struct > pci_device, and co-existed with MSI and MSI-X on the same pci_device. > > Jason
On Fri, Sep 13, 2019 at 01:27:10PM -0700, Raj, Ashok wrote: > On Fri, Sep 13, 2019 at 07:50:50PM +0000, Jason Gunthorpe wrote: > > On Thu, Sep 12, 2019 at 06:32:01PM -0700, Megha Dey wrote: > > > > > This series is a basic patchset to get the ball rolling and receive some > > > inital comments. As per my discussion with Marc Zyngier and Thomas Gleixner > > > at the Linux Plumbers, I need to do the following: > > > 1. Since a device can support MSI-X and IMS simultaneously, ensure proper > > > locking mechanism for the 'msi_list' in the device structure. > > > 2. Introduce dynamic allocation of IMS vectors perhaps by using a group ID > > > 3. IMS support of a device needs to be discoverable. A bit in the vendor > > > specific capability in the PCI config is to be added rather than getting > > > this information from each device driver. > > > > Why #3? The point of this scheme is to delegate programming the > > addr/data pairs to the driver so it can be done in some > > device-specific way. There is no PCI standard behind this, and no > > change in PCI semantics. > > > > I think it would be a tall ask to get a config space bit from PCI-SIG > > for something that has little to do with PCI. > > This isn't a standard config capability. Its Designated Vendor Specific > Capability (DVSEC). The device is responsible for managing the addr-data > pair. This provides a hint to the OS framework that this device has > device specific methods. > > Agreed its not required, but some OSV's like a generic way to discover > these capabilities, hence its there so device vendors can have > a common guideline. I think it would be reasonable for a specific device driver to test the DVSEC, if it wishes too. Since it is not required it does not make sense for the core kernel to enforce this on all devices, at least for such a nebulous reason. Jason