Re: [kvm-unit-tests PATCH v9 22/31] powerpc: Add MMU support

["Nicholas Piggin" <npiggin@gmail.com>]

On Tue Jun 4, 2024 at 5:30 PM AEST, Thomas Huth wrote:
> On 04/05/2024 14.28, Nicholas Piggin wrote:
> > Add support for radix MMU, 4kB and 64kB pages.
> > 
> > This also adds MMU interrupt test cases, and runs the interrupts
> > test entirely with MMU enabled if it is available (aside from
> > machine check tests).
> > 
> > Acked-by: Andrew Jones <andrew.jones@linux.dev> (configure changes)
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> ...
> > diff --git a/lib/ppc64/mmu.c b/lib/ppc64/mmu.c
> > new file mode 100644
> > index 000000000..5307cd862
> > --- /dev/null
> > +++ b/lib/ppc64/mmu.c
> > @@ -0,0 +1,281 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Radix MMU support
> > + *
> > + * Copyright (C) 2024, IBM Inc, Nicholas Piggin <npiggin@gmail.com>
> > + *
> > + * Derived from Linux kernel MMU code.
> > + */
> > +#include <asm/mmu.h>
> > +#include <asm/setup.h>
> > +#include <asm/smp.h>
> > +#include <asm/page.h>
> > +#include <asm/io.h>
> > +#include <asm/processor.h>
> > +#include <asm/hcall.h>
> > +
> > +#include "alloc_page.h"
> > +#include "vmalloc.h"
> > +#include <asm/pgtable-hwdef.h>
> > +#include <asm/pgtable.h>
> > +
> > +#include <linux/compiler.h>
> > +
> > +static pgd_t *identity_pgd;
> > +
> > +bool vm_available(void)
> > +{
> > +	return cpu_has_radix;
> > +}
> > +
> > +bool mmu_enabled(void)
> > +{
> > +	return current_cpu()->pgtable != NULL;
> > +}
> > +
> > +void mmu_enable(pgd_t *pgtable)
> > +{
> > +	struct cpu *cpu = current_cpu();
> > +
> > +	if (!pgtable)
> > +		pgtable = identity_pgd;
> > +
> > +	cpu->pgtable = pgtable;
> > +
> > +	mtmsr(mfmsr() | (MSR_IR|MSR_DR));
> > +}
> > +
> > +void mmu_disable(void)
> > +{
> > +	struct cpu *cpu = current_cpu();
> > +
> > +	cpu->pgtable = NULL;
> > +
> > +	mtmsr(mfmsr() & ~(MSR_IR|MSR_DR));
> > +}
> > +
> > +static inline void tlbie(unsigned long rb, unsigned long rs, int ric, int prs, int r)
> > +{
> > +	asm volatile(".machine push ; .machine power9; ptesync ; tlbie %0,%1,%2,%3,%4 ; eieio ; tlbsync ; ptesync ; .machine pop" :: "r"(rb), "r"(rs), "i"(ric), "i"(prs), "i"(r) : "memory");
>
> That's a very long line, please split it up after every assembly instruction 
> (using \n for new lines).
>
> > +}
> ...
> > diff --git a/powerpc/mmu.c b/powerpc/mmu.c
> > new file mode 100644
> > index 000000000..fef790506
> > --- /dev/null
> > +++ b/powerpc/mmu.c
> > @@ -0,0 +1,283 @@
> > +/* SPDX-License-Identifier: LGPL-2.0-only */
> > +/*
> > + * MMU Tests
> > + *
> > + * Copyright 2024 Nicholas Piggin, IBM Corp.
> > + */
> > +#include <libcflat.h>
> > +#include <asm/atomic.h>
> > +#include <asm/barrier.h>
> > +#include <asm/processor.h>
> > +#include <asm/mmu.h>
> > +#include <asm/smp.h>
> > +#include <asm/setup.h>
> > +#include <asm/ppc_asm.h>
> > +#include <vmalloc.h>
> > +#include <devicetree.h>
> > +
> > +static inline void tlbie(unsigned long rb, unsigned long rs, int ric, int prs, int r)
> > +{
> > +	asm volatile(".machine push ; .machine power9; ptesync ; tlbie %0,%1,%2,%3,%4 ; eieio ; tlbsync ; ptesync ; .machine pop" :: "r"(rb), "r"(rs), "i"(ric), "i"(prs), "i"(r) : "memory");
> > +}
>
> Same function again? Maybe it could go into mmu.h instead?
>
> > +static inline void tlbiel(unsigned long rb, unsigned long rs, int ric, int prs, int r)
> > +{
> > +	asm volatile(".machine push ; .machine power9; ptesync ; tlbiel %0,%1,%2,%3,%4 ; ptesync ; .machine pop" :: "r"(rb), "r"(rs), "i"(ric), "i"(prs), "i"(r) : "memory");
> > +}
>
> Please also split up the above long line.

I'll try to improve the lines.

> It would also be cool if you could get one of the other ppc guys at IBM to 
> review this patch, since I don't have a clue about this MMU stuff at all.

It would be.

Thanks,
Nick