Re: [PATCH 1/1] ARM: LPAE: use phys_addr_t instead of unsigned long in outercache hooks

Arnd Bergmann
2020-12-28T07:05:47Z

On Fri, Dec 25, 2020 at 12:48 PM Zhen Lei <thunder.leizhen@huawei.com> wrote:
>
> The outercache of some Hisilicon SOCs support physical addresses wider
> than 32-bits. The unsigned long datatype is not sufficient for mapping
> physical addresses >= 4GB. The commit ad6b9c9d78b9 ("ARM: 6671/1: LPAE:
> use phys_addr_t instead of unsigned long in outercache functions") has
> already modified the outercache functions. But the parameters of the
> outercache hooks are not changed. This patch use phys_addr_t instead of
> unsigned long in outercache hooks: inv_range, clean_range, flush_range.
>
> To ensure the outercache that does not support LPAE works properly, do
> cast phys_addr_t to unsigned long by adding a middle-tier function.
> For example:
> -static void l2c220_inv_range(unsigned long start, unsigned long end)
> +static void __l2c220_inv_range(unsigned long start, unsigned long end)
> {
> ...
> }
> +static void l2c220_inv_range(phys_addr_t start, phys_addr_t end)
> +{
> + __l2c220_inv_range(start, end);
> +}
>
> Note that the outercache functions have been doing this cast before this
> patch. So now, the cast is just moved to the middle-tier function.
>
> No functional change.
>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

This looks reasonable in principle, but it would be helpful to
understand better which SoCs are affected. In which way is
this specific to Hisilicon implementations, and why would others
not need this?

Wouldn't this also be needed by an Armada XP that supports
more than 4GB of RAM but has an outer cache?

I suppose those SoCs using off-the-shelf Arm cores are either
pre-LPAE and cannot address memory above 4GB, or they do
not need the outer_cache interfaces.

> diff --git a/arch/arm/mm/cache-feroceon-l2.c b/arch/arm/mm/cache-feroceon-l2.c
> index 5c1b7a7b9af6300..ab1d8051bf832c9 100644
> --- a/arch/arm/mm/cache-feroceon-l2.c
> +++ b/arch/arm/mm/cache-feroceon-l2.c
> @@ -168,7 +168,7 @@ static unsigned long calc_range_end(unsigned long start, unsigned long end)
> return range_end;
> }
>
> -static void feroceon_l2_inv_range(unsigned long start, unsigned long end)
> +static void __feroceon_l2_inv_range(unsigned long start, unsigned long end)
> {
> /*
> * Clean and invalidate partial first cache line.
> @@ -198,7 +198,12 @@ static void feroceon_l2_inv_range(unsigned long start, unsigned long end)
> dsb();
> }
>
> -static void feroceon_l2_clean_range(unsigned long start, unsigned long end)
> +static void feroceon_l2_inv_range(phys_addr_t start, phys_addr_t end)
> +{
> + __feroceon_l2_inv_range(start, end);
> +}
> +

What is this indirection for? It looks like you do this for all implementations,
so the actual address gets truncated here.

Arnd