[PATCH] powerpc/xive: Change IRQ domain to a tree domain

[Cédric Le Goater]

On 11/16/21 17:58, Marc Zyngier wrote:
> On Tue, 16 Nov 2021 13:40:22 +0000,
> Cédric Le Goater <clg at kaod.org> wrote:
>>
>> Commit 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains
>> exclusive") introduced an IRQ_DOMAIN_FLAG_NO_MAP flag to isolate the
>> 'nomap' domains still in use under the powerpc arch. With this new
>> flag, the revmap_tree of the IRQ domain is not used anymore. This
>> change broke the support of shared LSIs [1] in the XIVE driver because
>> it was relying on a lookup in the revmap_tree to query previously
>> mapped interrupts.
> 
> Just a lookup? Surely there is more to it, no?

nope. The HW IRQ for the INTx is defined in the DT. It is caught by
of_irq_parse_and_map_pci() which simply adds an extra mapping on the
same INTx since the previous one is not found.

Using an INTx is quite rare now days and a shared one is even more
uncommon I guess, I could only reproduced on the baremetal platform
with the QEMU PowerNV machine using the same virtio devices.

Thanks,

C.
  
> 
>> Linux now creates two distinct IRQ mappings on the
>> same HW IRQ which can lead to unexpected behavior in the drivers.
>>
>> The XIVE IRQ domain is not a direct mapping domain and its HW IRQ
>> interrupt number space is rather large : 1M/socket on POWER9 and
>> POWER10, change the XIVE driver to use a 'tree' domain type instead.
>>
>> [1] For instance, a linux KVM guest with virtio-rng and virtio-balloon
>>      devices.
>>
>> Cc: Marc Zyngier <maz at kernel.org>
>> Cc: stable at vger.kernel.org # v5.14+
>> Fixes: 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains exclusive")
>> Signed-off-by: Cédric Le Goater <clg at kaod.org>
>> ---
>>
>>   Marc,
>>
>>   The Fixes tag is there because the patch in question revealed that
>>   something was broken in XIVE. genirq is not in cause. However, I
>>   don't know for PS3 and Cell. May be less critical for now.
> 
> Depends if they expect something that a no-map domain cannot provide.> 
>>   
>>   arch/powerpc/sysdev/xive/common.c | 3 +--
>>   arch/powerpc/sysdev/xive/Kconfig  | 1 -
>>   2 files changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
>> index fed6fd16c8f4..9d0f0fe25598 100644
>> --- a/arch/powerpc/sysdev/xive/common.c
>> +++ b/arch/powerpc/sysdev/xive/common.c
>> @@ -1536,8 +1536,7 @@ static const struct irq_domain_ops xive_irq_domain_ops = {
>>   
>>   static void __init xive_init_host(struct device_node *np)
>>   {
>> -	xive_irq_domain = irq_domain_add_nomap(np, XIVE_MAX_IRQ,
>> -					       &xive_irq_domain_ops, NULL);
>> +	xive_irq_domain = irq_domain_add_tree(np, &xive_irq_domain_ops, NULL);
>>   	if (WARN_ON(xive_irq_domain == NULL))
>>   		return;
>>   	irq_set_default_host(xive_irq_domain);
>> diff --git a/arch/powerpc/sysdev/xive/Kconfig b/arch/powerpc/sysdev/xive/Kconfig
>> index 97796c6b63f0..785c292d104b 100644
>> --- a/arch/powerpc/sysdev/xive/Kconfig
>> +++ b/arch/powerpc/sysdev/xive/Kconfig
>> @@ -3,7 +3,6 @@ config PPC_XIVE
>>   	bool
>>   	select PPC_SMP_MUXED_IPI
>>   	select HARDIRQS_SW_RESEND
>> -	select IRQ_DOMAIN_NOMAP
>>   
>>   config PPC_XIVE_NATIVE
>>   	bool
> 
> As long as this works, I'm happy with one less no-map user.
> 
> Acked-by: Marc Zyngier <maz at kernel.org>
> 
> 	M.
>