Re: [PATCH 04/57] ARM: GICv3: simplify GICv3 redistributor stride handling

[Julien Grall <julien.grall@arm.com>]

On 08/03/18 12:40, Andre Przywara wrote:
> Hi,

Hi,

> 
> On 06/03/18 13:49, Julien Grall wrote:
>>
>>
>> On 05/03/18 17:08, Julien Grall wrote:
>>> On 05/03/18 16:03, Andre Przywara wrote:
>>>> Instead of hard coding the architected redistributor stride into the
>>>> code, lets use a clear #define to the two values for GICv3 and GICv4 and
>>>> clarify the algorithm to determine the needed stride value.
>>>>
>>>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>>>> ---
>>>> Changelog RFC ... v1:
>>>> - no changes
>>>>
>>>>    xen/arch/arm/gic-v3.c             | 18 ++++++++++--------
>>>>    xen/include/asm-arm/gic_v3_defs.h |  5 +++++
>>>>    2 files changed, 15 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>>>> index b1f8a86409..be1787b39a 100644
>>>> --- a/xen/arch/arm/gic-v3.c
>>>> +++ b/xen/arch/arm/gic-v3.c
>>>> @@ -690,6 +690,15 @@ static int __init gicv3_populate_rdist(void)
>>>>            do {
>>>>                typer = readq_relaxed(ptr + GICR_TYPER);
>>>> +            /* Set the architectural redist size if not overridden
>>>> by DT. */
>>>> +            if ( !gicv3.rdist_stride )
>>>> +            {
>>>> +                if ( typer & GICR_TYPER_VLPIS )
>>>
>>> Is there anything in the spec promising you that *all* the
>>> redistributors will support vLPIs?
>>
>> Answering to myself, Marc pointed out that nothing was promising all
>> redistributors will support vLPIs. This is confirmed by the section 9.7
>> "Mixing GICv3 and GICv4" in "GICv3 and GICv4 Software Overview" DAI 0492B.
>>
>> So I would prefer to drop that patch and move GICV*_GICR_SIZE definition
>> in patch #5.
> 
> So I can surely do that, but then we have a definition of
> GICV*_GICR_SIZE, but still the hardcoded values in the code.
> So what about I keep this as a patch, but remove the actual code change
> and just replace it with using the symbol names instead of the "SZ_64K *
> 2" plus comment?

GICV*_GICR_SIZE feels a bit weird to read. More that reading the spec 
gives the impression that it is possible to have GICv4 without vLPIS 
(see the description of GICR_TYPER.vLPIS).

So I think I would prefer to keep the hardcoding values here.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel