Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v2 05/12] irqchip/gic: Prepare for more than 16 PPIs
To:     Julien <julien.thierry.kdev@gmail.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Jason Cooper <jason@lakedaemon.net>,
        Rob Herring <robh+dt@kernel.org>
Cc:     John Garry <john.garry@huawei.com>,
        Shameerali Kolothum Thodi 
        <shameerali.kolothum.thodi@huawei.com>,
        Lokesh Vutla <lokeshvutla@ti.com>,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
References: <20190806100121.240767-1-maz@kernel.org>
 <20190806100121.240767-6-maz@kernel.org>
 <1b2675f6-f839-80f8-b7d8-a7d402085745@gmail.com>
From:   Marc Zyngier <maz@kernel.org>
Organization: Approximate
Message-ID: <ccd1a1f2-d1a5-4f35-eb5e-ba5acd33e3ea@kernel.org>
Date:   Thu, 22 Aug 2019 17:32:27 +0100
User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <1b2675f6-f839-80f8-b7d8-a7d402085745@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Julien,

On 22/08/2019 17:11, Julien wrote:
> Hi Marc,
> 
> On 06/08/19 11:01, Marc Zyngier wrote:
>> GICv3.1 allows up to 80 PPIs (16 legaci PPIs and 64 Extended PPIs),
>> meaning we can't just leave the old 16 hardcoded everywhere.
>>
>> We also need to add the infrastructure to discover the number of PPIs
>> on a per redistributor basis, although we still pretend there is only
>> 16 of them for now.
>>
>> No functional change.
>>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>>   drivers/irqchip/irq-gic-common.c | 19 ++++++++++++-------
>>   drivers/irqchip/irq-gic-common.h |  2 +-
>>   drivers/irqchip/irq-gic-v3.c     | 22 +++++++++++++++-------
>>   drivers/irqchip/irq-gic.c        |  2 +-
>>   drivers/irqchip/irq-hip04.c      |  2 +-
>>   5 files changed, 30 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
>> index 6900b6f0921c..14110db01c05 100644
>> --- a/drivers/irqchip/irq-gic-common.c
>> +++ b/drivers/irqchip/irq-gic-common.c
>> @@ -128,26 +128,31 @@ void gic_dist_config(void __iomem *base, int gic_irqs,
>>   		sync_access();
>>   }
>>   
>> -void gic_cpu_config(void __iomem *base, void (*sync_access)(void))
>> +void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void))
>>   {
>>   	int i;
>>   
>>   	/*
>>   	 * Deal with the banked PPI and SGI interrupts - disable all
>> -	 * PPI interrupts, ensure all SGI interrupts are enabled.
>> -	 * Make sure everything is deactivated.
>> +	 * private interrupts. Make sure everything is deactivated.
>>   	 */
>> -	writel_relaxed(GICD_INT_EN_CLR_X32, base + GIC_DIST_ACTIVE_CLEAR);
>> -	writel_relaxed(GICD_INT_EN_CLR_PPI, base + GIC_DIST_ENABLE_CLEAR);
>> -	writel_relaxed(GICD_INT_EN_SET_SGI, base + GIC_DIST_ENABLE_SET);
>> +	for (i = 0; i < nr; i += 32) {
> 
> You added "nr" as argument but if "nr" isn't a multiple of 32 weird 
> things might happen, no?
>
> It would be worth specifying that somewhere, and checking it with a WARN().

TBH, I'm unsure whether that's worth it. The architecture is completely
built around having the private interrupts in blocks of 32, and you can
only get something wrong if you misdecode the number of interrupts from
the registers.

> Maybe it might be worth reducing the granularity to manipulating 16 irqs 
> since there are 16 SGI + 16 PPI + 64 EPPI, but that might not be very 
> useful right now.

I don't see what this brings us at this point. The architecture doesn't
seem to go in the direction of adding more SGIs, so we're pretty safe on
that front...

Thanks,

	M.
-- 
Jazz is not dead, it just smells funny...