Re: [PATCH v2] powerpc/64s/radix: Fix radix segment exception handling

["Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>]

Nicholas Piggin <npiggin@gmail.com> writes:

> Commit 48e7b76957 ("powerpc/64s/hash: Convert SLB miss handlers to C")
> broke the radix-mode segment exception handler. In radix mode, this is
> exception is not an SLB miss, rather it signals that the EA is outside
> the range translated by any page table.
>
> The commit lost the radix feature alternate code patch, which can
> cause faults to some EAs to kernel BUG at arch/powerpc/mm/slb.c:639!
>
> The original radix code would send faults to slb_miss_large_addr,
> which would end up faulting due to slb_addr_limit being 0. This patch
> sends radix directly to do_bad_slb_fault, which is a bit clearer.
>

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

> Fixes: 48e7b76957 ("powerpc/64s/hash: Convert SLB miss handlers to C")
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Reported-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> - Add a selftests that triggers the crash
>
>  arch/powerpc/kernel/exceptions-64s.S          | 12 +++
>  tools/testing/selftests/powerpc/mm/Makefile   |  3 +-
>  .../selftests/powerpc/mm/access_tests.c       | 94 +++++++++++++++++++
>  3 files changed, 108 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/powerpc/mm/access_tests.c
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index a5b8fbae56a0..9481a117e242 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -656,11 +656,17 @@ EXC_COMMON_BEGIN(data_access_slb_common)
>  	ld	r4,PACA_EXSLB+EX_DAR(r13)
>  	std	r4,_DAR(r1)
>  	addi	r3,r1,STACK_FRAME_OVERHEAD
> +BEGIN_MMU_FTR_SECTION
> +	/* HPT case, do SLB fault */
>  	bl	do_slb_fault
>  	cmpdi	r3,0
>  	bne-	1f
>  	b	fast_exception_return
>  1:	/* Error case */
> +MMU_FTR_SECTION_ELSE
> +	/* Radix case, access is outside page table range */
> +	li	r3,-EFAULT
> +ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
>  	std	r3,RESULT(r1)
>  	bl	save_nvgprs
>  	RECONCILE_IRQ_STATE(r10, r11)
> @@ -705,11 +711,17 @@ EXC_COMMON_BEGIN(instruction_access_slb_common)
>  	EXCEPTION_PROLOG_COMMON(0x480, PACA_EXSLB)
>  	ld	r4,_NIP(r1)
>  	addi	r3,r1,STACK_FRAME_OVERHEAD
> +BEGIN_MMU_FTR_SECTION
> +	/* HPT case, do SLB fault */
>  	bl	do_slb_fault
>  	cmpdi	r3,0
>  	bne-	1f
>  	b	fast_exception_return
>  1:	/* Error case */
> +MMU_FTR_SECTION_ELSE
> +	/* Radix case, access is outside page table range */
> +	li	r3,-EFAULT
> +ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
>  	std	r3,RESULT(r1)
>  	bl	save_nvgprs
>  	RECONCILE_IRQ_STATE(r10, r11)
> diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile
> index 43d68420e363..68b7add5086d 100644
> --- a/tools/testing/selftests/powerpc/mm/Makefile
> +++ b/tools/testing/selftests/powerpc/mm/Makefile
> @@ -2,7 +2,7 @@
>  noarg:
>  	$(MAKE) -C ../
>  
> -TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao segv_errors wild_bctr
> +TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao segv_errors wild_bctr access_tests
>  TEST_GEN_FILES := tempfile
>  
>  top_srcdir = ../../../../..
> @@ -13,6 +13,7 @@ $(TEST_GEN_PROGS): ../harness.c
>  $(OUTPUT)/prot_sao: ../utils.c
>  
>  $(OUTPUT)/wild_bctr: CFLAGS += -m64
> +$(OUTPUT)/access_tests: CFLAGS += -m64
>  
>  $(OUTPUT)/tempfile:
>  	dd if=/dev/zero of=$@ bs=64k count=1
> diff --git a/tools/testing/selftests/powerpc/mm/access_tests.c b/tools/testing/selftests/powerpc/mm/access_tests.c
> new file mode 100644
> index 000000000000..ad300d7d9d43
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/mm/access_tests.c
> @@ -0,0 +1,94 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright 2017 John Sperbeck
> + *
> + * Test faults to "interesting" locations.
> + */
> +
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <signal.h>
> +#include <sys/mman.h>
> +#include <assert.h>
> +#include <ucontext.h>
> +
> +#include "utils.h"
> +
> +#define PAGE_SIZE	(64*1024)
> +#define TB		(1024ULL*1024*1024*1024)
> +static volatile bool faulted;
> +static volatile int si_code;
> +
> +static void segv_handler(int n, siginfo_t *info, void *ctxt_v)
> +{
> +	ucontext_t *ctxt = (ucontext_t *)ctxt_v;
> +	struct pt_regs *regs = ctxt->uc_mcontext.regs;
> +
> +	faulted = true;
> +	si_code = info->si_code;
> +	regs->nip += 4;
> +}
> +
> +int test_segv_errors(void)
> +{
> +	struct sigaction act = {
> +		.sa_sigaction = segv_handler,
> +		.sa_flags = SA_SIGINFO,
> +	};
> +	static unsigned long ptrs[] = {
> +		0x0f00000000000000ULL, /* Radix Q0 out of pgtable range */
> +		0x4000000000000000ULL, /* Radix Q1 */
> +		0x4f00000000000000ULL, /* Radix Q1 out of pgtable range */
> +		0x8000000000000000ULL, /* Radix Q2 */
> +		0x8f00000000000000ULL, /* Radix Q2 out of pgtable range */
> +		0xc000000000000000ULL, /* Radix Q3 */
> +		0xcf00000000000000ULL, /* Radix Q3 out of pgtable range */
> +		0xc000000000000000ULL, /* Hash kernel region */
> +		0xc000000000000000ULL + TB, /* Hash kernel region + 1 segment */
> +		0xc000000000000000ULL + TB - 1,
> +		0xd000000000000000ULL, /* Hash vmalloc region */
> +		0xd000000000000000ULL + TB,
> +		0xd000000000000000ULL + TB - 1,
> +		0xe000000000000000ULL,
> +		0xe000000000000000ULL + TB,
> +		0xe000000000000000ULL + TB - 1,
> +		0xf000000000000000ULL, /* Hash vmemmap region */
> +		0xf000000000000000ULL + TB,
> +		0xf000000000000000ULL + TB - 1,
> +	};
> +	size_t i;
> +
> +	FAIL_IF(sigaction(SIGSEGV, &act, NULL) != 0);
> +
> +	for (i = 0; i < sizeof(ptrs)/sizeof(ptrs[0]); i++) {
> +		volatile char *p = (void *)ptrs[i];
> +
> +		/*
> +		 * We just need a compiler barrier, but mb() works and has the
> +		 * nice property of being easy to spot in the disassembly.
> +		 */
> +		printf("testing %p...\n", p);
> +		faulted = false;
> +		si_code = 0;
> +		mb();
> +		(void)*p;
> +		mb();
> +		FAIL_IF(!faulted);
> +		FAIL_IF(si_code != SEGV_MAPERR && si_code != SEGV_BNDERR);
> +		/*
> +		 * Some accesses throw MAPERR, others BNDERR. Possibly all
> +		 * Q>0 accesses should cause BNDERR.
> +		 */
> +	}
> +
> +	return 0;
> +}
> +
> +int main(void)
> +{
> +	return test_harness(test_segv_errors, "segv_errors");
> +}
> -- 
> 2.20.1