Re: [PATCH v6 01/14] hw/intc: Allow gaps in hartids for aclint and aplic

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 24/9/25 10:00, Djordje Todorovic wrote:
> 
> On 22. 9. 25. 08:52, Philippe Mathieu-Daudé wrote:
>> CAUTION: This email originated from outside of the organization. Do
>> not click links or open attachments unless you recognize the sender
>> and know the content is safe.
>>
>>
>> On 3/9/25 14:35, Djordje Todorovic wrote:
>>>
>>> On 1. 9. 25. 13:05, Philippe Mathieu-Daudé wrote:
>>>> CAUTION: This email originated from outside of the organization. Do
>>>> not click links or open attachments unless you recognize the sender
>>>> and know the content is safe.
>>>>
>>>>
>>>> On 1/9/25 10:17, Djordje Todorovic wrote:
>>>>> On 8. 8. 25. 17:52, Philippe Mathieu-Daudé wrote:
>>>>>
>>>>>> CAUTION: This email originated from outside of the organization. Do
>>>>>> not click links or open attachments unless you recognize the sender
>>>>>> and know the content is safe.
>>>>>>
>>>>>>
>>>>>> On 17/7/25 11:38, Djordje Todorovic wrote:
>>>>>>> This is needed for riscv based CPUs by MIPS since those may have
>>>>>>> sparse hart-ID layouts. ACLINT and APLIC still assume a dense
>>>>>>> range, and if a hart is missing, this causes NULL derefs.
>>>>>>>
>>>>>>> Signed-off-by: Chao-ying Fu <cfu@mips.com>
>>>>>>> Signed-off-by: Djordje Todorovic <djordje.todorovic@htecgroup.com>
>>>>>>> ---
>>>>>>>      hw/intc/riscv_aclint.c | 21 +++++++++++++++++++--
>>>>>>>      hw/intc/riscv_aplic.c  | 11 ++++++++---
>>>>>>>      2 files changed, 27 insertions(+), 5 deletions(-)


>>> +    CPU_FOREACH(cpu) {
>>> +      if (cpu == NULL)
>>> +        abort();
>>
>> Why do you end having a NULL vcpu in the global cpus_queue?
>> (this is the 'elsewhere problem').
>>
> Well, it is true, for our case, we would never get into vcpu == NULL case.
> 
> 
> After several attempts to come up with a better solution for this, I think
> 
> we are back to the existing one. I will try to elaborate why.
> 
> The sparse hart-ID layout in this case is not a programming mistake but
> 
> an intentional hardware design characteristic of the P8700. The P8700
> 
> RISC-V implementation has a sparse hart-ID layout where not all hart IDs
> 
> in a range are populated. This is explicitly supported by the RISC-V APLIC
> 
> specification. The current ACLINT/APLIC implementation assumes a dense
> 
> range of hart IDs (from hartid_base to hartid_base + num_harts - 1).
> 
> For the P8700 board:
> 
>     - We iterate through the theoretical hart ID range for a cluster
> 
>     - Some hart IDs legitimately don't have corresponding CPUs (sparse
> layout)
> 
>     - We need to skip these without failing
> 
> The CPU_FOREACH approach doesn't work here because:
> 
>     - The cpu==NULL will never happen
> 
>     - It iterates over all CPUs system-wide, not just those in the current
> 
>       cluster

Correct, we simply need to iterate over the CPUs in the cluster, which
IFAICT this device model doesn't use (TYPE_CPU_CLUSTER).