Re: [PATCH v7 2/2] s390/pci: Migrate s390 IRQ logic to IRQ domain API

[Tobias Schumacher]

On Fri Nov 28, 2025 at 10:47 AM CET, Niklas Schnelle wrote:
> On Thu, 2025-11-27 at 16:07 +0100, Tobias Schumacher wrote:
>> s390 is one of the last architectures using the legacy API for setup and
>> teardown of PCI MSI IRQs. Migrate the s390 IRQ allocation and teardown
>> to the MSI parent domain API. For details, see:
>> 
>> https://lore.kernel.org/lkml/20221111120501.026511281@xxxxxxxxxxxxx
>> 
>> In detail, create an MSI parent domain for each PCI domain. When a PCI
>> device sets up MSI or MSI-X IRQs, the library creates a per-device IRQ
>> domain for this device, which is used by the device for allocating and
>> freeing IRQs.
>> 
>> The per-device domain delegates this allocation and freeing to the
>> parent-domain. In the end, the corresponding callbacks of the parent
>> domain are responsible for allocating and freeing the IRQs.
>> 
>> The allocation is split into two parts:
>> - zpci_msi_prepare() is called once for each device and allocates the
>>   required resources. On s390, each PCI function has its own airq
>>   vector and a summary bit, which must be configured once per function.
>>   This is done in prepare().
>> - zpci_msi_alloc() can be called multiple times for allocating one or
>>   more MSI/MSI-X IRQs. This creates a mapping between the virtual IRQ
>>   number in the kernel and the hardware IRQ number.
>> 
>> Freeing is split into two counterparts:
>> - zpci_msi_free() reverts the effects of zpci_msi_alloc() and
>> - zpci_msi_teardown() reverts the effects of zpci_msi_prepare(). This is
>>   called once when all IRQs are freed before a device is removed.
>> 
>> Since the parent domain in the end allocates the IRQs, the hwirq
>> encoding must be unambiguous for all IRQs of all devices. This is
>> achieved by encoding the hwirq using the devfn and the MSI index.
>> 
>> Signed-off-by: Tobias Schumacher <ts@xxxxxxxxxxxxx>
>> ---
>>  arch/s390/Kconfig           |   1 +
>>  arch/s390/include/asm/pci.h |   5 +
>>  arch/s390/pci/pci.c         |   6 +
>>  arch/s390/pci/pci_bus.c     |  18 ++-
>>  arch/s390/pci/pci_irq.c     | 310 ++++++++++++++++++++++++++++----------------
>>  5 files changed, 224 insertions(+), 116 deletions(-)
>> 
> --- snip ---
>
>> +
>> +static inline u16 zpci_decode_hwirq_msi_index(u32 hwirq)
>
> I think the parameter's type should by irq_hw_number_t here. It doesn't
> matter for correctness since we're only using 32 bits now and the cast
> just cuts off the upper 32 bits but I'd like to preserve the type until
> you explicitly mask off the bits we don't use.

Makes sense, I'll change that.

> I also considered making zpci_encode_hwirq()'s return type
> irq_hw_number_t but I think it's not needed. This is because there
> we're still in the process of encoding the hwirq and want to emphasize
> that our encoding output uses 32 bits.

Agreed.

>> +{
>> +	return hwirq & 0xffff;
>> +}
>>  
>
> The changes versus v6 look good to me and I agree that putting the
> zpci_set_irq() in zpci_reenable_device() looks like the cleanest fix
> for the recovery issue. Also good catch on the msg->data assignment. So
> feel free to (re-)add my R-b as per below.
>
> Reviewed-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>

Thanks, since this is only a minor change let's wait if Gerd and Farhan
want to add anything before sending a new version.

Tobias