[RFC PATCH 1/4] meminfo: add memory details for armv8

[Tom Rini]

On Thu, Jan 30, 2025 at 02:36:40PM +0100, Heinrich Schuchardt wrote:
> On 1/30/25 10:35, Ilias Apalodimas wrote:
> > HI Heinrich
> > 
> > On Thu, 30 Jan 2025 at 12:01, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > 
> > > On 1/30/25 07:20, Ilias Apalodimas wrote:
> > > > Upcoming patches are mapping memory with RO, RW^X etc permsissions.
> > > > Fix the meminfo command to display them properly
> > > > 
> > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > > > ---
> > > >    arch/arm/cpu/armv8/cache_v8.c    | 18 +++++++++++++++++-
> > > >    arch/arm/include/asm/armv8/mmu.h |  2 ++
> > > >    cmd/meminfo.c                    |  5 +++++
> > > >    3 files changed, 24 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
> > > > index 5d6953ffedd1..a4ca56c8ed42 100644
> > > > --- a/arch/arm/cpu/armv8/cache_v8.c
> > > > +++ b/arch/arm/cpu/armv8/cache_v8.c
> > > > @@ -421,7 +421,7 @@ static int count_ranges(void)
> > > >        return count;
> > > >    }
> > > > 
> > > > -#define ALL_ATTRS (3 << 8 | PMD_ATTRINDX_MASK)
> > > > +#define ALL_ATTRS (3 << 8 | PMD_ATTRMASK)
> > > >    #define PTE_IS_TABLE(pte, level) (pte_type(&(pte)) == PTE_TYPE_TABLE && (level) < 3)
> > > > 
> > > >    enum walker_state {
> > > > @@ -568,6 +568,17 @@ static void pretty_print_table_attrs(u64 pte)
> > > >    static void pretty_print_block_attrs(u64 pte)
> > > >    {
> > > >        u64 attrs = pte & PMD_ATTRINDX_MASK;
> > > > +     u64 perm_attrs = pte & PMD_ATTRMASK;
> > > 
> > > The variable perm_attrs is not needed. You could use pte directly.
> > > 
> > > > +
> > > > +     if ((perm_attrs & PTE_BLOCK_PXN) && (perm_attrs & PTE_BLOCK_UXN) &&
> > > > +         perm_attrs & PTE_BLOCK_RO)
> > > > +             printf("%-5s", " | RO");
> > > > +     else if ((perm_attrs & PTE_BLOCK_PXN) && (perm_attrs & PTE_BLOCK_UXN))
> > > > +             printf("%-5s", " | RW");
> > > > +     else if (perm_attrs & PTE_BLOCK_RO)
> > > > +             printf("%-5s", " | RX");
> > > > +     else
> > > > +             printf("%-5s", " | RWX");
> > > 
> > > The string has 6 characters. "%-5s" does not match!
> > 
> > Ah thanks! That's the misalignment i was chasing
> > 
> > > As the string length is known using a length identifier is superfluous.
> > > 
> > > It would be helpful to add descriptions to the flags in
> > > arch/arm/include/asm/armv8/mmu.h.
> > > 
> > > https://documentation-service.arm.com/static/5efa1d23dbdee951c1ccdec5
> > > describes:
> > > 
> > > "The Unprivileged Execute Never (UXN) and Privileged Execute Never (PXN)
> > > attributes can be set separately. This is used to prevent, for example,
> > > application code running with kernel privilege, or attempts to execute
> > > kernel code while in an unprivileged state."
> > > 
> > > Shouldn't we show the values of UXN and PXN separately?
> > 
> > I had that on my initial patches. But then I decided it's too Arm
> > specific to print it.
> > I was hoping more archs would fix that so I preferred RO, RW, RX since
> > it's arch agnosticI don't mind re-adding it.
> 
> This code in arch/arm/cpu/armv8/cache_v8.c is architecture specific.
> 
> RISC-V has flags R, W, X, U where U controls access by user mode.
> 
> When printing the flags I guess we should use an architecture specific
> notation.

... and update doc/usage/cmd/meminfo.rst to note that it's printing
arch-specific flags here, perhaps with an arm64 subsection explaining
any shorthand and links to appropriate documentation ?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250130/db2fa8fc/attachment.sig>