Re: [patch 08/16] genirq: Introduce callback irq_chip.irq_write_msi_msg

[Jiang Liu]

On 2014/11/19 14:57, Yun Wu (Abel) wrote:
> On 2014/11/18 22:32, Thomas Gleixner wrote:
> 
>> On Tue, 18 Nov 2014, Yun Wu (Abel) wrote:
>>
>> Can you please trim the messages when you're replying?
>>
>>> The above you described is absolutely right, but not the things I want
>>> to know. :)
>>> Take GICv3 ITS for example, it deals with both PCI and non PCI message
>>> interrupts. IIUC, several irq_chips need to be implemented in the ITS
>>> driver (i.e. pci_msi_chip, A_msi_chip and B_msi_chip). What should we
>>> do to the ITS driver if new MSI-capable devices come out?
>>
>> You seem to miss the stacking here
>>
>> PCI-MSI	->
>> A-MSI	->   ITS  -> GIC
>> B-MSI	->
>>
>> So each of the device types has its own MSI controller. Each of them
>> will have their own callbacks and are backed by the underlying ITS/GIC
>> implementation.
> 
> Yes, this hits the key point. Once a new device type becomes available,
> we need to add pieces of code outside the new device's driver to make
> it work, which in my opinion is due to lack of core infrastructure.
> More specifically, the core infrastructure needs to support mechanism
> of MSI, not the various types of devices.
It depends on the implementation details whether we need to change ITS
to support new devices. One possible way to avoid changing ITS to
support new devices is:
1) ITS only allocates and manages interrupt translation tables and pass
   some type of identifier back to MSI controller drivers.
2) Each MSI controller driver implements its own compose_msi_msg()
   and write_msi_msg() according to hardware specifications.
By this way, there is no need to change ITS to support new types of
MSI controllers.

On x86 systems, interrupt remapping is optional and we also want to hide
implementation details, so we chose another solution:
1) interrupt remapping driver allocates and manages remapping resources,
   it also implements different compose_msi_msg() to support different
   MSI controllers.
2) MSI controller implement write_msi_msg(), it also implements
   compose_msi_msg() if interrupt remapping is disabled.
By this way, we do need to change interrupt remapping driver(ITS on ARM)
to support new type of MSI controllers.

So it totally depends on how you implement the hierarchy irqdomain,
there are no limitations from the framework side.

For the proposal to add some type of ops structure into struct msi_desc,
actually I have a serial of patches to support device specific
mask/unmask/write/compose/startup/shutdown operations by adding
msi_desc.chip_ops. But there's no actual users of those interfaces
currently and we don't want to over-engineering, so I abandoned
that patch set. I may rework on it  when enabling generic MSI
support later.
Regards!
Gerry
> 
>>
>> And that's the only sensible solution.
>>
> 
> It's sensible, but not perfect.
> What I suggested is to add a generic layer:
> 
> PCI-MSI	->
> A-MSI	-> (generic layer) -> ITS -> GICR
> B-MSI	->
> 
> The PCI/A/B/... passes its hardware properties to the generic layer which
> gets configurations ready by calling ITS's domain/chip callbacks. When
> a new device type arrives, the only thing need to do is to implement the
> driver of that device, with nothing to do with the generic layer or ITS.
> 
> Thanks,
> 	Abel
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/