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

[Thomas Huth <thuth@redhat.com>]

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.

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.

  Thanks,
   Thomas