[PATCH] bsps/arm: fix off-by-1 in gicv3 processor count

[Sebastian Huber]

On 04/08/2021 19:45, Sebastian Huber wrote:
> On 04/08/2021 19:18, Gedare Bloom wrote:
>> Hi Sebastian,
>>
>>
>> On Wed, Aug 4, 2021 at 11:15 AM Gedare Bloom<gedare at rtems.org>  wrote:
>>> ---
>>>   bsps/shared/dev/irq/arm-gicv3.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/bsps/shared/dev/irq/arm-gicv3.c 
>>> b/bsps/shared/dev/irq/arm-gicv3.c
>>> index ea123d325e..95021f6ddf 100644
>>> --- a/bsps/shared/dev/irq/arm-gicv3.c
>>> +++ b/bsps/shared/dev/irq/arm-gicv3.c
>>> @@ -551,11 +551,11 @@ uint32_t arm_gic_irq_processor_count(void)
>>>       for (i = 0; i < CPU_MAXIMUM_PROCESSORS; ++i) {
>>>         volatile gic_redist *redist = gicv3_get_redist(i);
>>>
>>> +      ++cpu_count;
>>> +
>>>         if ((redist->icrtyper & GIC_REDIST_ICRTYPER_LAST) != 0) {
>>>           break;
>>>         }
>>> -
>>> -      ++cpu_count;
>>>       }
>> Can you check my thinking here? I'm trying to implement SMP support
>> for the versal on aarch64. The call to arm_gic_irq_processor_count()
>> returns 1 through this code path. Since cpu_count is initialized to 0,
>> I think we need to increment it each time we find an active processor,
>> until we find the last active processor, which we also need to include
>> in the count, because the cpu_count should be NUM_PROC + 1?
>>
>> After I make this change, I encounter other problems with the
>> secondary processor initialization, but at least it tries to
>> initialize/wait for core#1 then.
> 
> The current logic is based on this text in the Cortex-R52 TRM for 
> GICR_TYPER[Last]:
> 
> "In a processor configured with an interrupt export port, this bit is 
> set for the Redistributor associated with
> the export port. Otherwise, in a system with n cores, this bit is set 
> for the Redistributor associated with
> core n-1."
> 
> I don't know how you can figure out if a processor has an interrupt 
> export port. Maybe there are better ways to figure out the processor 
> count. I would have a look at the Linux or U-Boot sources.
> 
> Independent of this, I had problems with the software generated 
> interrupts on Qemu and AArch64. They worked only once. This cases the 
> new interrupt manager tests to fail with a timeout.

I have to check this on the FVP simulator. Maybe this simulator has two 
interrupt export ports or we have an off by one error.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber at embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/