Re: [PATCH 1/2] arm64: Reduce add_map() complexity

Fabio Estevam
2024-03-18T20:09:58Z

Hi Pierre,

On Mon, Mar 18, 2024 at 4:59 PM Pierre-Clément Tosi <ptosi@google.com> wrote:

> I notice that the mem_map in these logs have overlapping ranges, which results
> in unnecessary work when creating the PTs. For this reason, it would make sense
> to prune it in arch/arm/mach-imx/imx8/cpu.c before calling dcache_enable(), IMO.
> On this point, you also have contiguous ranges with identical attributes
> (mem_map[0] and mem_map[6]), which could be merged into a single entry. This
> could result in more efficient MMU mappings if the mem_map entries can share a
> block mapping. Also note that mem_map[4].size=0 so could be dropped.
>
> In any case, if we assume that overlapping mem_map entries are a valid input to
> the arch code (maybe not as it leads to potentially ambiguous behavior?), then
> 41e2787f5ec4 had removed support for splitting existing block mappings.
>
> In your case, my assumption is that the function was then treating block
> mappings in the range 0x1c000000-0x80000000 (which get mapped, at least
> partially, by mem_map[0], mem_map[1], then mem_map[2]) as table mappings and was
> issuing read/write instructions in that range (accessing them as PTEs). As those
> seem to be device memory (I see they get mapped as MT_DEVICE_NGNRNE), these
> accesses might explain the SError you were experiencing.
>
> Would you mind testing [1] and giving it "Tested-by:" if it addresses the issue?

Your patch fixed the boot regression. Thanks for your fix, appreciated it!

I have replied with my Tested-by tag.

Cheers,

Fabio Estevam