LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] KVM: selftests: Add support for ppc64le
From: Fabiano Rosas @ 2022-01-20 17:01 UTC (permalink / raw)
  To: kvm; +Cc: aik, linuxppc-dev, npiggin, paulus, linux-kselftest, pbonzini,
	shuah
In-Reply-To: <20220120170109.948681-1-farosas@linux.ibm.com>

This adds the infrastructure for writing tests for the powerpc
platform (Only Radix MMU for now).

This patch also enables two tests:

- a dummy sample test that creates a guest with one vcpu, issues
  hypercalls and reads/writes test values from memory.

- the kvm_page_table test, although at this point I'm not using it to
  test KVM, but mostly as a way to stress test this code.

$ make -C tools/testing/selftests TARGETS=kvm
$ make -C tools/testing/selftests TARGETS=kvm run_tests

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 MAINTAINERS                                   |   3 +
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |  14 +-
 .../selftests/kvm/include/kvm_util_base.h     |   7 +
 .../selftests/kvm/include/ppc64le/processor.h |  43 +++
 tools/testing/selftests/kvm/lib/kvm_util.c    |   5 +
 .../testing/selftests/kvm/lib/powerpc/hcall.S |   6 +
 .../selftests/kvm/lib/powerpc/processor.c     | 343 ++++++++++++++++++
 .../testing/selftests/kvm/lib/powerpc/ucall.c |  67 ++++
 .../selftests/kvm/powerpc/sample_test.c       | 144 ++++++++
 10 files changed, 630 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/include/ppc64le/processor.h
 create mode 100644 tools/testing/selftests/kvm/lib/powerpc/hcall.S
 create mode 100644 tools/testing/selftests/kvm/lib/powerpc/processor.c
 create mode 100644 tools/testing/selftests/kvm/lib/powerpc/ucall.c
 create mode 100644 tools/testing/selftests/kvm/powerpc/sample_test.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a76e7558b151..15c89d33d584 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10537,6 +10537,9 @@ F:	arch/powerpc/include/asm/kvm*
 F:	arch/powerpc/include/uapi/asm/kvm*
 F:	arch/powerpc/kernel/kvm*
 F:	arch/powerpc/kvm/
+F:	tools/testing/selftests/kvm/include/ppc64le/
+F:	tools/testing/selftests/kvm/lib/powerpc/
+F:	tools/testing/selftests/kvm/powerpc/
 
 KERNEL VIRTUAL MACHINE FOR RISC-V (KVM/riscv)
 M:	Anup Patel <anup@brainfault.org>
diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 8c129961accf..45ab993e2845 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -46,6 +46,7 @@
 /x86_64/xen_vmcall_test
 /x86_64/xss_msr_test
 /x86_64/vmx_pmu_msrs_test
+/powerpc/sample_test
 /access_tracking_perf_test
 /demand_paging_test
 /dirty_log_test
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 556da71c33b8..5ae27109e9b9 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -17,9 +17,9 @@ KSFT_KHDR_INSTALL := 1
 # LINUX_TOOL_ARCH_INCLUDE is set using ARCH variable.
 #
 # x86_64 targets are named to include x86_64 as a suffix and directories
-# for includes are in x86_64 sub-directory. s390x and aarch64 follow the
-# same convention. "uname -m" doesn't result in the correct mapping for
-# s390x and aarch64.
+# for includes are in x86_64 sub-directory. s390x, aarch64 and ppc64le
+# follow the same convention. "uname -m" doesn't result in the correct
+# mapping for s390x, aarch64 and ppc64le.
 #
 # No change necessary for x86_64
 UNAME_M := $(shell uname -m)
@@ -36,12 +36,17 @@ endif
 ifeq ($(ARCH),riscv)
 	UNAME_M := riscv
 endif
+# Set UNAME_M for ppc64le compile/install to work
+ifeq ($(ARCH),powerpc)
+	UNAME_M := ppc64le
+endif
 
 LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/rbtree.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c lib/perf_test_util.c
 LIBKVM_x86_64 = lib/x86_64/apic.c lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S
 LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c lib/aarch64/handlers.S lib/aarch64/spinlock.c lib/aarch64/gic.c lib/aarch64/gic_v3.c lib/aarch64/vgic.c
 LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c lib/s390x/diag318_test_handler.c
 LIBKVM_riscv = lib/riscv/processor.c lib/riscv/ucall.c
+LIBKVM_ppc64le = lib/powerpc/processor.c lib/powerpc/ucall.c lib/powerpc/hcall.S
 
 TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
 TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features
@@ -133,6 +138,9 @@ TEST_GEN_PROGS_riscv += kvm_page_table_test
 TEST_GEN_PROGS_riscv += set_memory_region_test
 TEST_GEN_PROGS_riscv += kvm_binary_stats_test
 
+TEST_GEN_PROGS_ppc64le += powerpc/sample_test
+TEST_GEN_PROGS_ppc64le += kvm_page_table_test
+
 TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
 LIBKVM += $(LIBKVM_$(UNAME_M))
 
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index 66775de26952..a930d663fe67 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -54,6 +54,7 @@ enum vm_guest_mode {
 	VM_MODE_P36V48_16K,
 	VM_MODE_P36V48_64K,
 	VM_MODE_P36V47_16K,
+	VM_MODE_P51V52_64K,
 	NUM_VM_MODES,
 };
 
@@ -87,6 +88,12 @@ extern enum vm_guest_mode vm_mode_default;
 #define MIN_PAGE_SHIFT			12U
 #define ptes_per_page(page_size)	((page_size) / 8)
 
+#elif defined(__powerpc__)
+
+#define VM_MODE_DEFAULT			VM_MODE_P51V52_64K
+#define MIN_PAGE_SHIFT			16U
+#define ptes_per_page(page_size)	((page_size) / 8)
+
 #endif
 
 #define MIN_PAGE_SIZE		(1U << MIN_PAGE_SHIFT)
diff --git a/tools/testing/selftests/kvm/include/ppc64le/processor.h b/tools/testing/selftests/kvm/include/ppc64le/processor.h
new file mode 100644
index 000000000000..fbc1332b2b80
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/ppc64le/processor.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * powerpc processor specific defines
+ */
+#ifndef SELFTEST_KVM_PROCESSOR_H
+#define SELFTEST_KVM_PROCESSOR_H
+
+#define PPC_BIT(x) (1ULL << (63 - x))
+
+#define MSR_SF  PPC_BIT(0)
+#define MSR_IR  PPC_BIT(58)
+#define MSR_DR  PPC_BIT(59)
+#define MSR_LE  PPC_BIT(63)
+
+#define LPCR_UPRT  PPC_BIT(41)
+#define LPCR_EVIRT PPC_BIT(42)
+#define LPCR_HR    PPC_BIT(43)
+#define LPCR_GTSE  PPC_BIT(53)
+
+#define PATB_GR	PPC_BIT(0)
+
+#define PTE_VALID PPC_BIT(0)
+#define PTE_LEAF  PPC_BIT(1)
+#define PTE_R	  PPC_BIT(55)
+#define PTE_C	  PPC_BIT(56)
+#define PTE_RC	  (PTE_R | PTE_C)
+#define PTE_READ  0x4
+#define PTE_WRITE 0x2
+#define PTE_EXEC  0x1
+#define PTE_RWX   (PTE_READ|PTE_WRITE|PTE_EXEC)
+
+extern uint64_t hcall(uint64_t nr, ...);
+
+static inline uint32_t mfpvr(void)
+{
+	uint32_t pvr;
+
+	asm ("mfpvr %0"
+	     : "=r"(pvr));
+	return pvr;
+}
+
+#endif
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index c22a17aac6b0..cc5247c2cfeb 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -205,6 +205,7 @@ const char *vm_guest_mode_string(uint32_t i)
 		[VM_MODE_P36V48_16K]	= "PA-bits:36,  VA-bits:48, 16K pages",
 		[VM_MODE_P36V48_64K]	= "PA-bits:36,  VA-bits:48, 64K pages",
 		[VM_MODE_P36V47_16K]	= "PA-bits:36,  VA-bits:47, 16K pages",
+		[VM_MODE_P51V52_64K]    = "PA-bits:51,  VA-bits:52, 64K pages",
 	};
 	_Static_assert(sizeof(strings)/sizeof(char *) == NUM_VM_MODES,
 		       "Missing new mode strings?");
@@ -230,6 +231,7 @@ const struct vm_guest_mode_params vm_guest_mode_params[] = {
 	[VM_MODE_P36V48_16K]	= { 36, 48,  0x4000, 14 },
 	[VM_MODE_P36V48_64K]	= { 36, 48, 0x10000, 16 },
 	[VM_MODE_P36V47_16K]	= { 36, 47,  0x4000, 14 },
+	[VM_MODE_P51V52_64K]    = { 51, 52, 0x10000, 16 },
 };
 _Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct vm_guest_mode_params) == NUM_VM_MODES,
 	       "Missing new mode params?");
@@ -331,6 +333,9 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
 	case VM_MODE_P44V64_4K:
 		vm->pgtable_levels = 5;
 		break;
+	case VM_MODE_P51V52_64K:
+		vm->pgtable_levels = 4;
+		break;
 	default:
 		TEST_FAIL("Unknown guest mode, mode: 0x%x", mode);
 	}
diff --git a/tools/testing/selftests/kvm/lib/powerpc/hcall.S b/tools/testing/selftests/kvm/lib/powerpc/hcall.S
new file mode 100644
index 000000000000..a78b88f3b207
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/powerpc/hcall.S
@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+.globl hcall;
+
+hcall:
+	sc	1
+	blr
diff --git a/tools/testing/selftests/kvm/lib/powerpc/processor.c b/tools/testing/selftests/kvm/lib/powerpc/processor.c
new file mode 100644
index 000000000000..2ffd5423a968
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/powerpc/processor.c
@@ -0,0 +1,343 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * KVM selftest powerpc library code
+ *
+ * Copyright (C) 2021, IBM Corp.
+ */
+
+#define _GNU_SOURCE
+//#define DEBUG
+
+#include "kvm_util.h"
+#include "../kvm_util_internal.h"
+#include "processor.h"
+
+/*
+ * 2^(12+PRTS) = Process table size
+ *
+ * But the hardware doesn't seem to care, so 0 for now.
+ */
+#define PRTS 0
+#define RTS ((0x5UL << 5) | (0x2UL << 61)) /* 2^(RTS+31) = 2^52 */
+#define RPDS 0xd
+#define RPDB_MASK 0x0fffffffffffff00UL
+#define RPN_MASK  0x01fffffffffff000UL
+
+#define MIN_FRAME_SZ 32
+
+static const int radix_64k_index_sizes[4] = { 5, 9, 9, 13 };
+
+static inline uint64_t mk_pte(uint64_t pte_val)
+{
+	return cpu_to_be64(PTE_VALID | pte_val);
+}
+
+static inline uint64_t get_pte(uint64_t pte)
+{
+	return be64_to_cpu(pte);
+}
+
+static inline uint64_t pte_rpn(uint64_t entry)
+{
+	return get_pte(entry) & RPN_MASK;
+}
+
+static inline uint64_t next_pde(uint64_t entry)
+{
+	return get_pte(entry) & RPDB_MASK;
+}
+
+static inline uint64_t ptrs_per_pgd(int level)
+{
+	return 1UL << radix_64k_index_sizes[level];
+}
+
+static inline uint64_t level_size(int level)
+{
+	return sizeof(vm_paddr_t) << (radix_64k_index_sizes[level] + 3);
+}
+
+static vm_paddr_t alloc_pgd(struct kvm_vm *vm, int level)
+{
+	static vm_paddr_t base;
+	vm_paddr_t addr;
+	uint64_t size = level_size(level);
+
+	if (!base || (base + size) >> vm->page_shift != base >> vm->page_shift)
+		addr = vm_alloc_page_table(vm);
+	else
+		addr = base;
+	base = addr + size;
+
+	return addr;
+}
+
+static vm_paddr_t pgtable_walk(struct kvm_vm *vm, vm_vaddr_t gva, uint64_t gpa,
+			       bool alloc)
+{
+	uint64_t index_bits, shift, base, index;
+	uint64_t *ptep, ptep_gpa;
+	int level;
+
+	if (!vm->pgd_created)
+		goto unmapped_gva;
+
+	pr_debug("%s %#lx ", (alloc ? "mapping" : "lookup "), gva);
+
+	base = vm->pgd;
+	shift = vm->va_bits;
+
+	for (level = 3; level >= 0; --level) {
+
+		index_bits = radix_64k_index_sizes[level];
+		shift -= index_bits;
+
+		index = (gva >> shift) & ((1UL << index_bits) - 1);
+		ptep_gpa = base + index * sizeof(*ptep);
+		ptep = addr_gpa2hva(vm, ptep_gpa);
+
+		if (!*ptep) {
+			if (!alloc)
+				goto unmapped_gva;
+			if (level)
+				*ptep = mk_pte(alloc_pgd(vm, level - 1) |
+					       radix_64k_index_sizes[level - 1]);
+		}
+
+		if (get_pte(*ptep) & PTE_LEAF)
+			break;
+
+		base = next_pde(*ptep);
+	}
+
+	if (alloc)
+		*ptep = mk_pte(PTE_LEAF | gpa | PTE_RC | PTE_RWX);
+	else
+		gpa = pte_rpn(*ptep);
+
+	pr_debug("-> %#lx pte: %#lx (@%#lx)\n", gpa, get_pte(*ptep), ptep_gpa);
+
+	return gpa | (gva & (vm->page_size - 1));
+
+unmapped_gva:
+	TEST_FAIL("No mapping for vm virtual address, gva: %#lx", gva);
+	exit(1);
+}
+
+void virt_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr)
+{
+	TEST_ASSERT((vaddr % vm->page_size) == 0,
+		    "Virtual address not on page boundary,\n"
+		    "  vaddr: 0x%lx vm->page_size: 0x%x", vaddr, vm->page_size);
+
+	TEST_ASSERT(sparsebit_is_set(vm->vpages_valid,
+				     (vaddr >> vm->page_shift)),
+		    "Invalid virtual address, vaddr: 0x%lx", vaddr);
+
+	TEST_ASSERT((paddr % vm->page_size) == 0,
+		    "Physical address not on page boundary,\n"
+		    "  paddr: 0x%lx vm->page_size: 0x%x", paddr, vm->page_size);
+
+	TEST_ASSERT((paddr >> vm->page_shift) <= vm->max_gfn,
+		    "Physical address beyond maximum supported,\n"
+		    "  paddr: 0x%lx vm->max_gfn: 0x%lx vm->page_size: 0x%x",
+		    paddr, vm->max_gfn, vm->page_size);
+
+	TEST_ASSERT(vm->pgd_created, "Page table not created\n");
+
+	pgtable_walk(vm, vaddr, paddr, true);
+}
+
+vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva)
+{
+	return pgtable_walk(vm, gva, 0, false);
+}
+
+void virt_pgd_alloc(struct kvm_vm *vm)
+{
+	struct kvm_ppc_mmuv3_cfg cfg = { 0 };
+	vm_paddr_t proc_tb;
+	uint64_t *proc_tb_hva;
+
+	if (!kvm_check_cap(KVM_CAP_PPC_MMU_RADIX)) {
+		print_skip("Tests only support Radix MMU");
+		exit(KSFT_SKIP);
+	}
+
+	if (!kvm_check_cap(KVM_CAP_PPC_PAPR)) {
+		print_skip("Tests only support Book3s");
+		exit(KSFT_SKIP);
+	}
+
+	if (vm->pgd_created)
+		return;
+
+	/*
+	 * Allocate the process table in guest memory and set the
+	 * first doubleword of the pid 0 entry.
+	 */
+	proc_tb = vm_alloc_page_table(vm);
+	vm->pgd = vm_alloc_page_table(vm);
+
+	proc_tb_hva = addr_gpa2hva(vm, proc_tb);
+	*proc_tb_hva = cpu_to_be64(RTS | vm->pgd | RPDS);
+
+	pr_debug("process table gpa: %#lx\n", proc_tb);
+	pr_debug("process table hva: %p\n", proc_tb_hva);
+	pr_debug("process table entry 0 dw0: %#lx\n", *proc_tb_hva);
+
+	/* Register the process table with the HV */
+	cfg.process_table = PATB_GR | proc_tb | PRTS;
+	cfg.flags = KVM_PPC_MMUV3_RADIX | KVM_PPC_MMUV3_GTSE;
+
+	pr_debug("MMU config proc table: %#llx\n", cfg.process_table);
+
+	vm_ioctl(vm, KVM_PPC_CONFIGURE_V3_MMU, &cfg);
+	vm->pgd_created = true;
+}
+
+void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
+{
+	struct kvm_enable_cap cap = { 0 };
+	struct kvm_regs regs;
+	struct kvm_sregs sregs;
+	vm_vaddr_t stack_vaddr;
+	size_t stack_sz;
+
+	vm_vcpu_add(vm, vcpuid);
+
+	cap.cap = KVM_CAP_PPC_PAPR;
+	vcpu_enable_cap(vm, vcpuid, &cap);
+
+	stack_sz = DEFAULT_STACK_PGS * vm->page_size;
+	stack_vaddr = vm_vaddr_alloc(vm, stack_sz,
+				     DEFAULT_GUEST_STACK_VADDR_MIN);
+
+	regs.msr = MSR_SF | MSR_LE;
+	regs.msr |= MSR_IR | MSR_DR;
+	regs.pc = (unsigned long) guest_code;
+	regs.pid = 0;
+	regs.gpr[1] = stack_vaddr + stack_sz - MIN_FRAME_SZ;
+
+	pr_debug("stack - low: %#lx high: %#lx size: %#lx SP: %#llx\n",
+		 stack_vaddr, stack_vaddr + stack_sz, stack_sz, regs.gpr[1]);
+
+	vcpu_regs_set(vm, vcpuid, &regs);
+
+	sregs.pvr = mfpvr();
+	vcpu_sregs_set(vm, vcpuid, &sregs);
+
+	if (kvm_check_cap(KVM_CAP_ONE_REG)) {
+		uint64_t lpcr = LPCR_UPRT | LPCR_HR | LPCR_GTSE;
+		struct kvm_one_reg reg = {
+			.id = KVM_REG_PPC_LPCR_64,
+			.addr = (uintptr_t) &lpcr,
+		};
+
+		vcpu_ioctl(vm, vcpuid, KVM_SET_ONE_REG, &reg);
+	}
+}
+
+void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
+{
+	va_list ap;
+	struct kvm_regs regs;
+	int i;
+
+	TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
+		    "  num: %u\n", num);
+
+	va_start(ap, num);
+	vcpu_regs_get(vm, vcpuid, &regs);
+
+	for (i = 0; i < num; i++)
+		regs.gpr[i + 3] = va_arg(ap, uint64_t);
+
+	vcpu_regs_set(vm, vcpuid, &regs);
+	va_end(ap);
+}
+
+static void pte_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent,
+		     uint64_t addr, int level)
+{
+	static const char * const type[] = { "pte", "pmd", "pud", "pgd" };
+	uint64_t pde, *hva;
+
+	if (level < 0)
+		return;
+
+	fprintf(stream, "%*s (%#lx):\n", indent, type[level], addr);
+
+	for (pde = addr; pde < addr + (ptrs_per_pgd(level) * sizeof(vm_paddr_t));
+	     pde += sizeof(vm_paddr_t)) {
+
+		hva = addr_gpa2hva(vm, pde);
+		if (!*hva)
+			continue;
+		fprintf(stream, "%*s %#lx: %#lx\n", indent + 1, "", pde,
+			get_pte(*hva));
+		pte_dump(stream, vm, indent + 2, next_pde(*hva), level - 1);
+	}
+}
+
+void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
+{
+	if (!vm->pgd_created)
+		return;
+
+	pte_dump(stream, vm, indent, vm->pgd, 3);
+}
+
+void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent)
+{
+	struct kvm_regs regs;
+
+	fprintf(stream, "%*scpuid: %u\n", indent, "", vcpuid);
+
+	vcpu_regs_get(vm, vcpuid, &regs);
+	fprintf(stream, "%*sregs:\n", indent + 2, "");
+
+	fprintf(stream, "%*spc: %#llx\n", indent + 4, "", regs.pc);
+	fprintf(stream, "%*smsr: %#llx\n", indent + 4, "", regs.msr);
+	fprintf(stream, "%*ssrr0: %#llx\n", indent + 4, "", regs.srr0);
+	fprintf(stream, "%*ssrr1: %#llx\n", indent + 4, "", regs.srr1);
+
+	fprintf(stream, "\n%*sr1: %#llx\n", indent + 4, "", regs.gpr[1]);
+	fprintf(stream, "%*sr2: %#llx\n", indent + 4, "", regs.gpr[2]);
+	fprintf(stream, "%*sr3: %#llx\n", indent + 4, "", regs.gpr[3]);
+	fprintf(stream, "%*sr4: %#llx\n", indent + 4, "", regs.gpr[4]);
+
+	if (kvm_check_cap(KVM_CAP_ONE_REG)) {
+		uint64_t lpcr;
+		struct kvm_one_reg reg = {
+			.id = KVM_REG_PPC_LPCR_64,
+			.addr = (uintptr_t) &lpcr,
+		};
+
+		vcpu_ioctl(vm, vcpuid, KVM_GET_ONE_REG, &reg);
+		fprintf(stream, "%*slpcr: %#lx\n", indent + 4, "", lpcr);
+	}
+	fprintf(stream, "%*slr: %#llx\n", indent + 4, "", regs.lr);
+}
+
+void assert_on_unhandled_exception(struct kvm_vm *vm, uint32_t vcpuid)
+{
+	struct kvm_run *run;
+
+	run = vcpu_state(vm, vcpuid);
+	if (run) {
+		switch (run->exit_reason) {
+		case KVM_EXIT_PAPR_HCALL:
+		case KVM_EXIT_MMIO:
+			return;
+		default:
+			printf("reason: %s\n",
+			       exit_reason_str(run->exit_reason));
+			break;
+		}
+	}
+#ifdef DEBUG
+	vm_dump(stderr, vm, 2);
+#endif
+	TEST_ASSERT(false, "Unhandled exception");
+}
diff --git a/tools/testing/selftests/kvm/lib/powerpc/ucall.c b/tools/testing/selftests/kvm/lib/powerpc/ucall.c
new file mode 100644
index 000000000000..fc76ef796f02
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/powerpc/ucall.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "kvm_util.h"
+#include "processor.h"
+
+/*
+ * Using this hypercall for now because it is already defined. It is
+ * used by SLOF to ask QEMU to copy memory regions, so it is close
+ * enough for our purposes.
+ */
+#define KVMPPC_H_LOGICAL_MEMOP 0xf001
+
+
+void ucall_init(struct kvm_vm *vm, void *arg)
+{
+}
+
+void ucall_uninit(struct kvm_vm *vm)
+{
+}
+
+static inline int __ucall(uint64_t args)
+{
+	return hcall(KVMPPC_H_LOGICAL_MEMOP, args);
+}
+
+/*
+ * This function runs inside the guest, so avoid optimizations that
+ * could add an indirect call via PLT and disable vector instructions
+ * like the kernel does.
+ */
+__attribute__((optimize(0), target("no-altivec,no-vsx")))
+void ucall(uint64_t cmd, int nargs, ...)
+{
+	struct ucall uc = {
+		.cmd = cmd,
+	};
+	va_list va;
+	int i;
+
+	nargs = nargs <= UCALL_MAX_ARGS ? nargs : UCALL_MAX_ARGS;
+
+	va_start(va, nargs);
+	for (i = 0; i < nargs; ++i)
+		uc.args[i] = va_arg(va, uint64_t);
+	va_end(va);
+
+	__ucall((uint64_t)&uc);
+}
+
+uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc)
+{
+	struct kvm_run *run = vcpu_state(vm, vcpu_id);
+	struct ucall ucall = {};
+
+	if (uc)
+		memset(uc, 0, sizeof(*uc));
+
+	if (run->exit_reason == KVM_EXIT_PAPR_HCALL &&
+	    run->papr_hcall.nr == KVMPPC_H_LOGICAL_MEMOP) {
+		memcpy(&ucall, addr_gva2hva(vm, run->papr_hcall.args[0]),
+					    sizeof(ucall));
+		if (uc)
+			memcpy(uc, &ucall, sizeof(ucall));
+	}
+
+	return ucall.cmd;
+}
diff --git a/tools/testing/selftests/kvm/powerpc/sample_test.c b/tools/testing/selftests/kvm/powerpc/sample_test.c
new file mode 100644
index 000000000000..16f0df920d98
--- /dev/null
+++ b/tools/testing/selftests/kvm/powerpc/sample_test.c
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define _GNU_SOURCE /* for program_invocation_short_name */
+#include <pthread.h>
+#include <stdio.h>
+#include <signal.h>
+
+#define DEBUG
+#include "kvm_util.h"
+#include "test_util.h"
+#include "processor.h"
+
+#define H_PUT_TERM_CHAR 0x58
+#define TEST_VAL 0x8badf00d
+#define PASS_VAL 0xdeadbeef
+#define FAIL_VAL 0x2badd00d
+
+
+struct kvm_vm *vm;
+
+/*
+ * Call the hypervisor to write a character to the console. KVM does
+ * not handle this hypercall so it goes out to userspace. Which in
+ * this case is the vcpu_worker() below.
+ */
+static inline void put_char(char c)
+{
+	hcall(H_PUT_TERM_CHAR, 0, 1, cpu_to_be64(c));
+}
+
+static void guest_code(uint64_t *ptr, uint64_t val)
+{
+	/*
+	 * Test making a hypercall and give a visual indication that
+	 * the guest code is running.
+	 */
+	put_char('.');
+
+	/* Make sure we can receive values */
+	GUEST_ASSERT(ptr);
+	GUEST_ASSERT(val == TEST_VAL);
+
+	put_char('.');
+
+	/* Read/write to memory */
+	if (*ptr == val)
+		*ptr = PASS_VAL;
+	else
+		*ptr = FAIL_VAL;
+
+	put_char('.');
+
+	/* Signal we're done */
+	GUEST_DONE();
+}
+
+static bool guest_done(struct kvm_vm *vm)
+{
+	struct ucall uc;
+	bool done;
+
+	switch (get_ucall(vm, 0, &uc)) {
+	case UCALL_ABORT:
+		TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
+			  __FILE__, uc.args[1]);
+		/* not reached */
+	case UCALL_DONE:
+		done = true;
+		break;
+	default:
+		done = false;
+		break;
+	}
+
+	return done;
+}
+
+static void *vcpu_worker(void *data)
+{
+	struct kvm_vm *vm = data;
+	struct kvm_run *run;
+	uint64_t *hva;
+	static uint64_t test_buf = TEST_VAL;
+
+	/* Pass arguments to the guest code */
+	vcpu_args_set(vm, 0, 2, &test_buf, TEST_VAL);
+
+	run = vcpu_state(vm, 0);
+	while (1) {
+		vcpu_run(vm, 0);
+
+		if (guest_done(vm))
+			break;
+
+		switch (run->exit_reason) {
+		case KVM_EXIT_PAPR_HCALL:
+			if (run->papr_hcall.nr == H_PUT_TERM_CHAR) {
+				char c = be64_to_cpu(run->papr_hcall.args[2]);
+
+				pr_debug("%c", c);
+			}
+			break;
+		default:
+			printf("exit reason: %s\n", exit_reason_str(run->exit_reason));
+			break;
+		}
+	}
+
+	hva = addr_gva2hva(vm, (vm_vaddr_t)&test_buf);
+	TEST_ASSERT(*hva != FAIL_VAL,
+		    "Guest failed to read test value at gva %p", &test_buf);
+	TEST_ASSERT(*hva == PASS_VAL,
+		    "Guest failed to write test value to gva %p", &test_buf);
+
+	pr_debug("PASS\n");
+
+	return NULL;
+}
+
+void dump_vm(int sig)
+{
+	vm_dump(stderr, vm, 2);
+	exit(1);
+}
+
+int main(int argc, char *argv[])
+{
+	pthread_t vcpu_thread;
+
+	signal(SIGINT, dump_vm);
+
+	/*
+	 * Do not buffer stdout so we can implement put_char without
+	 * flushing.
+	 */
+	setbuf(stdout, NULL);
+
+	vm = vm_create_default(0, 0, guest_code);
+	pthread_create(&vcpu_thread, NULL, vcpu_worker, vm);
+
+	pthread_join(vcpu_thread, NULL);
+	kvm_vm_free(vm);
+
+	return 0;
+}
-- 
2.34.1


^ permalink raw reply related

* [PATCH 0/2] KVM: selftests: Add powerpc support
From: Fabiano Rosas @ 2022-01-20 17:01 UTC (permalink / raw)
  To: kvm; +Cc: aik, linuxppc-dev, npiggin, paulus, linux-kselftest, pbonzini,
	shuah

This series adds the initial support for ppc64le Book3s with Radix
MMU.

At this time I'm including only the kvm_page_table test and a dummy
test to serve as a sample of what can be done with these tests. I
intend to make a pass over the remaining common tests and add the ones
which could be built for powerpc as well.

patch 1: a prerequisite small fix for the powerpc vcpu_ioctl. It is
         the same I already sent to the ppc mailing list but I'll
         include it here to make this a complete series.

patch 2: the actual infrastructure support.

Fabiano Rosas (2):
  KVM: PPC: Book3S HV: Stop returning internal values to userspace
  KVM: selftests: Add support for ppc64le

 MAINTAINERS                                   |   3 +
 arch/powerpc/kvm/powerpc.c                    |   8 +
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |  14 +-
 .../selftests/kvm/include/kvm_util_base.h     |   7 +
 .../selftests/kvm/include/ppc64le/processor.h |  43 +++
 tools/testing/selftests/kvm/lib/kvm_util.c    |   5 +
 .../testing/selftests/kvm/lib/powerpc/hcall.S |   6 +
 .../selftests/kvm/lib/powerpc/processor.c     | 343 ++++++++++++++++++
 .../testing/selftests/kvm/lib/powerpc/ucall.c |  67 ++++
 .../selftests/kvm/powerpc/sample_test.c       | 144 ++++++++
 11 files changed, 638 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/include/ppc64le/processor.h
 create mode 100644 tools/testing/selftests/kvm/lib/powerpc/hcall.S
 create mode 100644 tools/testing/selftests/kvm/lib/powerpc/processor.c
 create mode 100644 tools/testing/selftests/kvm/lib/powerpc/ucall.c
 create mode 100644 tools/testing/selftests/kvm/powerpc/sample_test.c

-- 
2.34.1


^ permalink raw reply

* [PATCH 1/2] KVM: PPC: Book3S HV: Stop returning internal values to userspace
From: Fabiano Rosas @ 2022-01-20 17:01 UTC (permalink / raw)
  To: kvm; +Cc: aik, linuxppc-dev, npiggin, paulus, linux-kselftest, pbonzini,
	shuah
In-Reply-To: <20220120170109.948681-1-farosas@linux.ibm.com>

Our kvm_arch_vcpu_ioctl_run currently returns the RESUME_HOST values
to userspace, against the API of the KVM_RUN ioctl which returns 0 on
success.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
This was noticed while enabling the kvm selftests for powerpc. There's
an assert at the _vcpu_run function when we return a value different
from the expected.
---
 arch/powerpc/kvm/powerpc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 2ad0ccd202d5..50414fb2a5ea 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -1841,6 +1841,14 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 #ifdef CONFIG_ALTIVEC
 out:
 #endif
+
+	/*
+	 * We're already returning to userspace, don't pass the
+	 * RESUME_HOST flags along.
+	 */
+	if (r > 0)
+		r = 0;
+
 	vcpu_put(vcpu);
 	return r;
 }
-- 
2.34.1


^ permalink raw reply related

* RE: [PATCH V3 07/17] riscv: compat: Re-implement TASK_SIZE for COMPAT_32BIT
From: David Laight @ 2022-01-20 15:46 UTC (permalink / raw)
  To: 'Arnd Bergmann', Guo Ren
  Cc: linux-s390, Guo Ren, gregkh, Drew Fustini, Anup Patel,
	Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky@vger.kernel.org,
	inux-parisc@vger.kernel.org, Christoph Hellwig, Palmer Dabbelt,
	liush, sparclinux, linux-riscv, open list:BROADCOM NVRAM DRIVER,
	linuxppc-dev, Christoph Hellwig, Linux ARM, Wei Fu
In-Reply-To: <CAK8P3a3FLnqibdXCGR8c+mm92HSiuvocZDeW8MyoTO_L1sYT=w@mail.gmail.com>

...
> One example of software that runs into virtual memory size limitations is
> the gnu linker when building large applications, but it's unlikely that you'll
> actually need to run applications that run into this, while also needing to
> build them natively.

There are also database programs that want to mmap() large sparse files.
To some extent that is where the pressure for 64bit addresses comes from.

While (I think) most of the current riscv systems are 'toy' ones
there are definitely press reports of some quite high power systems.

I suspect they are less 'toy' than the Altera (Intel) Nios processors
we use on out fpga - you can run linux on the Nios cpu, but you probably
don't really want to do so.
Better to find an fpga with a 'proper' ARM core in the corner.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [PATCH V3 07/17] riscv: compat: Re-implement TASK_SIZE for COMPAT_32BIT
From: Arnd Bergmann @ 2022-01-20 14:57 UTC (permalink / raw)
  To: Guo Ren
  Cc: Guo Ren, open list:BROADCOM NVRAM DRIVER, sparclinux, linux-riscv,
	inux-parisc@vger.kernel.org, linux-s390, Anup Patel,
	the arch/x86 maintainers, linux-csky@vger.kernel.org,
	Christoph Hellwig, Christoph Hellwig, Arnd Bergmann, Linux ARM,
	Drew Fustini, gregkh, Wang Junqiang, Linux Kernel Mailing List,
	David Laight, Palmer Dabbelt, liush, linuxppc-dev, Wei Fu
In-Reply-To: <CAJF2gTRwh40xDBkoRJWZEUketKFDAy7_z=-WW7E=T46yH4zPvw@mail.gmail.com>

On Thu, Jan 20, 2022 at 2:27 PM Guo Ren <guoren@kernel.org> wrote:
> On Thu, Jan 20, 2022 at 8:53 PM David Laight <David.Laight@aculab.com> wrote:
> > I think the x86-64 32bit compat code even puts the boundary at 4GB.
> Yes, we could give rv32 compat for 4GB with some effort. But it's unnecessary.
>
> There are no history issues for rv32, we use compat mode to reduce
> memory footprint. eg: only 64MB memory available.
>
> At end compat for 4GB is another topic, let's give the initial compat
> for 2GB support to riscv.

I think it's fine either way. Having the entire 4GB space available is nice
when you are trying to build 32-bit software natively rather then using a
cross-compiler, as you can just do it on a larger machine that supports both.

One example of software that runs into virtual memory size limitations is
the gnu linker when building large applications, but it's unlikely that you'll
actually need to run applications that run into this, while also needing to
build them natively.

Using the same limit as on native 32-bit machines can help with compatibility
of certain software, but again this is rarely a problem and I have not seen any
reports of issues with the 4GB TASK_SIZE_32 on arm64. On x86, there
is an option to use the native 3GB TASK_SIZE for compat tasks. This was
introduced to work around buggy applications a long time ago, but is
probably not used any more in practice.

       Arnd

^ permalink raw reply

* Re: [PATCH V3 08/17] riscv: compat: syscall: Add compat_sys_call_table implementation
From: Arnd Bergmann @ 2022-01-20 14:42 UTC (permalink / raw)
  To: Guo Ren
  Cc: linux-s390, Guo Ren, Arnd Bergmann, gregkh, Drew Fustini,
	Anup Patel, Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky, inux-parisc,
	Christoph Hellwig, Palmer Dabbelt, liush, sparclinux, linux-riscv,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Christoph Hellwig,
	Linux ARM, Wei Fu
In-Reply-To: <20220120073911.99857-9-guoren@kernel.org>

On Thu, Jan 20, 2022 at 8:39 AM <guoren@kernel.org> wrote:
>
>  /* The array of function pointers for syscalls. */
>  extern void * const sys_call_table[];
> +#ifdef CONFIG_COMPAT
> +extern void * const compat_sys_call_table[];
> +#endif

No need for the #ifdef, the normal convention is to just define the
extern declaration unconditionally for symbols that may or may not be defined.

> +COMPAT_SYSCALL_DEFINE3(truncate64, const char __user *, pathname,
> +                      arg_u32p(length))
> +{
> +       return ksys_truncate(pathname, arg_u64(length));
> +}

Are you sure these are the right calling conventions? According to [1],
I think the 64-bit argument should be in an aligned pair of registers,
which means you need an extra pad argument as in the arm64 version
of these functions. Same for ftruncate64, pread64, pwrite64, and
readahead.

> +COMPAT_SYSCALL_DEFINE3(ftruncate64, unsigned int, fd, arg_u32p(length))
> +{
> +       return ksys_ftruncate(fd, arg_u64(length));
> +}
> +
> +COMPAT_SYSCALL_DEFINE6(fallocate, int, fd, int, mode,
> +                      arg_u32p(offset), arg_u32p(len))
> +{
> +       return ksys_fallocate(fd, mode, arg_u64(offset), arg_u64(len));
> +}
> +
> +COMPAT_SYSCALL_DEFINE5(pread64, unsigned int, fd, char __user *, buf,
> +                      size_t, count, arg_u32p(pos))
> +{
> +       return ksys_pread64(fd, buf, count, arg_u64(pos));
> +}
> +
> +COMPAT_SYSCALL_DEFINE5(pwrite64, unsigned int, fd,
> +                      const char __user *, buf, size_t, count, arg_u32p(pos))
> +{
> +       return ksys_pwrite64(fd, buf, count, arg_u64(pos));
> +}
> +
> +COMPAT_SYSCALL_DEFINE6(sync_file_range, int, fd, arg_u32p(offset),
> +                      arg_u32p(nbytes), unsigned int, flags)
> +{
> +       return ksys_sync_file_range(fd, arg_u64(offset), arg_u64(nbytes),
> +                                   flags);
> +}
> +
> +COMPAT_SYSCALL_DEFINE4(readahead, int, fd, arg_u32p(offset),
> +                      size_t, count)
> +{
> +       return ksys_readahead(fd, arg_u64(offset), count);
> +}
> +
> +COMPAT_SYSCALL_DEFINE6(fadvise64_64, int, fd, int, advice, arg_u32p(offset),
> +                      arg_u32p(len))
> +{
> +       return ksys_fadvise64_64(fd, arg_u64(offset), arg_u64(len), advice);
> +}

I still feel like these should be the common implementations next to the
native handlers inside of an #ifdef CONFIG_COMPAT.

The names clash with the custom versions defined for powerpc and sparc,
but the duplicates look compatible if you can account for the padded
argument and the lo/hi order of the pairs, so could just be removed here
(all other architectures use custom function names instead).

        Arnd

[1] https://riscv.org/wp-content/uploads/2015/01/riscv-calling.pdf

^ permalink raw reply

* Re: [PATCH V3 15/17] riscv: compat: Add UXL_32 support in start_thread
From: Guo Ren @ 2022-01-20 13:43 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-s390, Guo Ren, gregkh, Drew Fustini, Anup Patel,
	Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky, inux-parisc,
	Christoph Hellwig, Palmer Dabbelt, liush, sparclinux, linux-riscv,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Christoph Hellwig,
	Linux ARM, Wei Fu
In-Reply-To: <CAK8P3a0Mr2m2dVoVss59cN-9X7GVBD29VQLo3m4xswRznk_WUQ@mail.gmail.com>

On Thu, Jan 20, 2022 at 9:39 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Jan 20, 2022 at 8:39 AM <guoren@kernel.org> wrote:
> > +
> > +#ifdef CONFIG_COMPAT
> > +       if (is_compat_task())
> > +               regs->status |= SR_UXL_32;
> > +#endif
>
>
> You should not need that #ifdef, as the is_compat_task() definition is
> meant to drop the code at compile time, unless the SR_UXL_32
> definition is not visible here.
I almost put CONFIG_COMPAT in every compat related code, because I
hope the next arch that wants to support COMPAT could easily find
where to be modified.

>
>          Arnd



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

^ permalink raw reply

* Re: [PATCH V3 10/17] riscv: compat: Add elf.h implementation
From: Guo Ren @ 2022-01-20 13:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-s390, Guo Ren, gregkh, Drew Fustini, Anup Patel,
	Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky, inux-parisc,
	Christoph Hellwig, Palmer Dabbelt, liush, sparclinux, linux-riscv,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Christoph Hellwig,
	Linux ARM, Wei Fu
In-Reply-To: <CAK8P3a2HaeFGdOr4Rm_ZcEP0rjh2Mo53=-ZRnEPtFfnmzfeE6g@mail.gmail.com>

On Thu, Jan 20, 2022 at 9:33 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Jan 20, 2022 at 8:39 AM <guoren@kernel.org> wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Implement necessary type and macro for compat elf. See the code
> > comment for detail.
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>
> > +
> > +/*
> > + * FIXME: not sure SET_PERSONALITY for compat process is right!
> > + */
> > +#define SET_PERSONALITY(ex)                                    \
> > +do {    if ((ex).e_ident[EI_CLASS] == ELFCLASS32)              \
> > +               set_thread_flag(TIF_32BIT);                     \
> > +       else                                                    \
> > +               clear_thread_flag(TIF_32BIT);                   \
> > +       if (personality(current->personality) != PER_LINUX32)   \
> > +               set_personality(PER_LINUX |                     \
> > +                       (current->personality & (~PER_MASK)));  \
> > +} while (0)
> > +
>
> The implementation looks good to me now,  you can remove that comment above it.
Thx for pointing it out. I forgot.

>
>         Arnd



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

^ permalink raw reply

* Re: [PATCH V3 07/17] riscv: compat: Re-implement TASK_SIZE for COMPAT_32BIT
From: Guo Ren @ 2022-01-20 13:27 UTC (permalink / raw)
  To: David Laight
  Cc: linux-s390, Guo Ren, Arnd Bergmann, gregkh, Drew Fustini,
	Anup Patel, Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky@vger.kernel.org,
	inux-parisc@vger.kernel.org, Christoph Hellwig, Palmer Dabbelt,
	liush, sparclinux, linux-riscv, open list:BROADCOM NVRAM DRIVER,
	linuxppc-dev, Christoph Hellwig, Linux ARM, Wei Fu
In-Reply-To: <f16cf10425a14c2e8183d5c90667ce72@AcuMS.aculab.com>

On Thu, Jan 20, 2022 at 8:53 PM David Laight <David.Laight@aculab.com> wrote:
>
> > > Limit 32-bit compatible process in 0-2GB virtual address range
> > > (which is enough for real scenarios), because it could avoid
> > > address sign extend problem when 32-bit enter 64-bit and ease
> > > software design.
>
> Eh?
> I thought nearly all the other 32bit unix ports (of any flavour)
> put the user-kernel boundary at 3GB.
No, riscv32 is about 2.4G, csky is 2G/2.5G.

> (Apart from some very old sparc ones that use 3.5GB.)
>
> 2GB is used by Windows.
>
> I think the x86-64 32bit compat code even puts the boundary at 4GB.
Yes, we could give rv32 compat for 4GB with some effort. But it's unnecessary.

There are no history issues for rv32, we use compat mode to reduce
memory footprint. eg: only 64MB memory available.

At end compat for 4GB is another topic, let's give the initial compat
for 2GB support to riscv.

>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

^ permalink raw reply

* Re: bug: usb: gadget: FSL_UDC_CORE Corrupted request list leads to unrecoverable loop.
From: Thorsten Leemhuis @ 2022-01-20 12:54 UTC (permalink / raw)
  To: Leo Li, jocke@infinera.com, regressions@leemhuis.info,
	Eugene_Bordenkircher@selinc.com, linux-usb@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org
  Cc: gregkh@linuxfoundation.org, balbi@kernel.org
In-Reply-To: <AS8PR04MB89464D90D2E92A5D401474BD8F6B9@AS8PR04MB8946.eurprd04.prod.outlook.com>

Hi, this is your Linux kernel regression tracker speaking.

On 04.12.21 01:40, Leo Li wrote:
>> -----Original Message-----
>> From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
>> Sent: Thursday, December 2, 2021 4:45 PM
>> To: regressions@leemhuis.info; Leo Li <leoyang.li@nxp.com>;
>> Eugene_Bordenkircher@selinc.com; linux-usb@vger.kernel.org; linuxppc-
>> dev@lists.ozlabs.org
>> Cc: gregkh@linuxfoundation.org; balbi@kernel.org
>> Subject: Re: bug: usb: gadget: FSL_UDC_CORE Corrupted request list leads to
>> unrecoverable loop.
>>
>> On Thu, 2021-12-02 at 20:35 +0000, Leo Li wrote:
>>>
>>>> -----Original Message-----
>>>> From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
>>>> Sent: Wednesday, December 1, 2021 8:19 AM
>>>> To: regressions@leemhuis.info; Leo Li <leoyang.li@nxp.com>;
>>>> Eugene_Bordenkircher@selinc.com; linux-usb@vger.kernel.org;
>>>> linuxppc- dev@lists.ozlabs.org
>>>> Cc: gregkh@linuxfoundation.org; balbi@kernel.org
>>>> Subject: Re: bug: usb: gadget: FSL_UDC_CORE Corrupted request list
>>>> leads to unrecoverable loop.
>>>>
>>>> On Tue, 2021-11-30 at 12:56 +0100, Joakim Tjernlund wrote:
>>>>> On Mon, 2021-11-29 at 23:48 +0000, Eugene Bordenkircher wrote:
>>>>>> Agreed,
>>>>>>
>>>>>> We are happy pick up the torch on this, but I'd like to try and
>>>>>> hear from
>>>> Joakim first before we do.  The patch set is his, so I'd like to
>>>> give him the opportunity.  I think he's the only one that can add a
>>>> truly proper description as well because he mentioned that this
>>>> includes a "few more fixes" than just the one we ran into.  I'd
>>>> rather hear from him than try to reverse engineer what was being
>> addressed.
>>>>>>
>>>>>> Joakim, if you are still watching the thread, would you like to
>>>>>> take a stab
>>>> at it?  If I don't hear from you in a couple days, we'll pick up the
>>>> torch and do what we can.

Did anything happen? Sure, it's a old regression from the v3.4-rc4 days,
but there iirc was already a tested proto-patch in that thread that
fixes the issue. Or was progress made and I just missed it?

Ciao, Thorsten

P.S.: As a Linux kernel regression tracker I'm getting a lot of reports
on my table. I can only look briefly into most of them. Unfortunately
therefore I sometimes will get things wrong or miss something important.
I hope that's not the case here; if you think it is, don't hesitate to
tell me about it in a public reply, that's in everyone's interest.

BTW, I have no personal interest in this issue, which is tracked using
regzbot, my Linux kernel regression tracking bot
(https://linux-regtracking.leemhuis.info/regzbot/). I'm only posting
this mail to get things rolling again and hence don't need to be CC on
all further activities wrt to this regression.

#regzbot ignore-activity

>>>>> I am far away from this now and still on 4.19. I don't mind if you
>>>>> tweak
>>>> tweak the patches for better "upstreamability"
>>>>
>>>> Even better would be to migrate to the chipidea driver, I am told
>>>> just a few tweaks are needed but this is probably something NXP
>>>> should do as they have access to other SOC's using chipidea.
>>>
>>> I agree with this direction but the problem was with bandwidth.  As this
>> controller was only used on legacy platforms, it is harder to justify new effort
>> on it now.
>>
>> Legacy? All PPC is legacy and not supported now?
> 
> I'm not saying that they are not supported, but they are in maintenance only mode.

^ permalink raw reply

* RE: [PATCH V3 03/17] asm-generic: compat: Cleanup duplicate definitions
From: David Laight @ 2022-01-20 13:02 UTC (permalink / raw)
  To: 'Arnd Bergmann', Guo Ren
  Cc: linux-s390, Guo Ren, liush, Drew Fustini, Anup Patel,
	Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky@vger.kernel.org,
	open list:BROADCOM NVRAM DRIVER, Christoph Hellwig,
	Palmer Dabbelt, gregkh, sparclinux, linux-riscv, Wei Fu,
	linuxppc-dev, inux-parisc@vger.kernel.org, Linux ARM,
	Christoph Hellwig
In-Reply-To: <CAK8P3a1UmnjHk8B6hSULiKv3FKoY5BW9=4=ESerQzc+4=LR5Zw@mail.gmail.com>

From: Arnd Bergmann
> Sent: 20 January 2022 11:52
..
> As with compat_flock, the packed attribute has no impact on the layout
> here, but please drop it anyway for consistency.

Never mind the structure layout, because 'packed' allows the
structure to be aligned on any boundary it forces the compiler
to use byte memory accesses and shifts on some architectures.
This is a horrid performance penalty.

'packed' should only be specified for structures that can
occur on any address boundary.
ie basically never.

If you need to remove the implicit pad before a field (eg 64bit
fields on x64-32) then you can mark the field itself as 'packed'.
Although, in that case, putting the attribute on the type (compat_s64)
is generally cleaner.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [BUG] mtd: cfi_cmdset_0002: write regression since v4.17-rc1
From: Thorsten Leemhuis @ 2022-01-20 13:00 UTC (permalink / raw)
  To: Tokunori Ikegami, Thorsten Leemhuis, Ahmad Fatoum, linux-mtd,
	Joakim.Tjernlund, miquel.raynal, vigneshr, richard,
	regressions@lists.linux.dev
  Cc: linuxppc-dev, linux-kernel@vger.kernel.org, marek.vasut,
	Chris Packham, Pengutronix Kernel Team, cyrille.pitchen,
	Shaohui.Xie, Brian Norris, David Woodhouse
In-Reply-To: <e11b76dc-5539-fb7e-da1c-a5005713d6b0@gmail.com>

Hi, this is your Linux kernel regression tracker speaking.

On 15.12.21 18:34, Tokunori Ikegami wrote:
> Hi Ahmad-san,
> 
> Sorry for the regression issue by the change: dfeae1073583.
> To make sure could you please try with the word write instead of the
> buffered writes?

Ahmad, did you try what Tokunori asked? Was any progress made to get
this regression fixed? To me it looks like it fell through the cracks.
Can anyone provide a status update please?

Ciao, Thorsten

P.S.: As a Linux kernel regression tracker I'm getting a lot of reports
on my table. I can only look briefly into most of them. Unfortunately
therefore I sometimes will get things wrong or miss something important.
I hope that's not the case here; if you think it is, don't hesitate to
tell me about it in a public reply, that's in everyone's interest.

BTW, I have no personal interest in this issue, which is tracked using
regzbot, my Linux kernel regression tracking bot
(https://linux-regtracking.leemhuis.info/regzbot/). I'm only posting
this mail to get things rolling again and hence don't need to be CC on
all further activities wrt to this regression.

#regzbot poke

> FYI: There are some changes to disable the buffered writes as below.
>   1.
> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ar71xx/patches-4.9/411-mtd-cfi_cmdset_0002-force-word-write.patch;h=ddd69f17e1ac16e8fc3a694c56231fee1e2ef149;hb=fec8fe806963c96a6506c2aebc3572d3a11f285f
> 
>   2.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mtd/chips/cfi_cmdset_0002.c?h=v5.16-rc5&id=7e4404113686868858a34210c28ae122e967aa64
> 
> 
> Note:
>   Currently I am not able to investigate the issue on the product for
> the change before.
> 
>   By the way in the past I had investigated the similar issue on Buffalo
> WZR-HP-G300NH using the S29GL256N.
>   It was not able to find the root cause by the investigation since not
> required actually at that time.
>   Also actually the buffered writes were disabled on the OpenWrt
> firmware as the change [1] above.
>   But I am not sure the reason detail to disable the buffered writes on
> the OpenWrt firmware.
>   I thought the issue not caused by the change: dfeae1073583 since the
> issue happened without the change.
> 
>   So I am not sure why the above change [2] needed to disable the
> buffered writes on Buffalo WZR-HP-G300NH.
>   Probably seems needed to disable the buffered writes on the other
> firmware also but not OpenWrt firmware.
> 
>   Anyway there are difference with your regression issue as below.
>     1. Flash device: S29GL064N (Your regression issue), S29GL256N
> (WZR-HP-G300NH)
>     2. Regression issue: Yes (Your regression issue), No (WZR-HP-G300NH
> as I investigated before)
> 
> Regards,
> Ikegami
> 
> On 2021/12/14 16:23, Thorsten Leemhuis wrote:
>> [TLDR: adding this regression to regzbot; most of this mail is compiled
>> from a few templates paragraphs some of you might have seen already.]
>>
>> Hi, this is your Linux kernel regression tracker speaking.
>>
>> Top-posting for once, to make this easy accessible to everyone.
>>
>> Thanks for the report.
>>
>> Adding the regression mailing list to the list of recipients, as it
>> should be in the loop for all regressions, as explained here:
>> https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html
>>
>> To be sure this issue doesn't fall through the cracks unnoticed, I'm
>> adding it to regzbot, my Linux kernel regression tracking bot:
>>
>> #regzbot ^introduced dfeae1073583
>> #regzbot title mtd: cfi_cmdset_0002: flash write accesses on the
>> hardware fail on a PowerPC MPC8313 to a 8-bit-parallel S29GL064N flash
>> #regzbot ignore-activity
>>
>> Reminder: when fixing the issue, please add a 'Link:' tag with the URL
>> to the report (the parent of this mail), then regzbot will automatically
>> mark the regression as resolved once the fix lands in the appropriate
>> tree. For more details about regzbot see footer.
>>
>> Sending this to everyone that got the initial report, to make all aware
>> of the tracking. I also hope that messages like this motivate people to
>> directly get at least the regression mailing list and ideally even
>> regzbot involved when dealing with regressions, as messages like this
>> wouldn't be needed then.
>>
>> Don't worry, I'll send further messages wrt to this regression just to
>> the lists (with a tag in the subject so people can filter them away), as
>> long as they are intended just for regzbot. With a bit of luck no such
>> messages will be needed anyway.
>>
>> Ciao, Thorsten (wearing his 'Linux kernel regression tracker' hat).
>>
>> P.S.: As a Linux kernel regression tracker I'm getting a lot of reports
>> on my table. I can only look briefly into most of them. Unfortunately
>> therefore I sometimes will get things wrong or miss something important.
>> I hope that's not the case here; if you think it is, don't hesitate to
>> tell me about it in a public reply. That's in everyone's interest, as
>> what I wrote above might be misleading to everyone reading this; any
>> suggestion I gave thus might sent someone reading this down the wrong
>> rabbit hole, which none of us wants.
>>
>> BTW, I have no personal interest in this issue, which is tracked using
>> regzbot, my Linux kernel regression tracking bot
>> (https://linux-regtracking.leemhuis.info/regzbot/). I'm only posting
>> this mail to get things rolling again and hence don't need to be CC on
>> all further activities wrt to this regression.
>>
>> On 13.12.21 14:24, Ahmad Fatoum wrote:
>>> Hi,
>>>
>>> I've been investigating a breakage on a PowerPC MPC8313: The SoC is
>>> connected
>>> via the "Enhanced Local Bus Controller" to a 8-bit-parallel S29GL064N
>>> flash,
>>> which is represented as a memory-mapped cfi-flash.
>>>
>>> The regression began in v4.17-rc1 with
>>>
>>>    dfeae1073583 ("mtd: cfi_cmdset_0002: Change write buffer to check
>>> correct value")
>>>
>>> and causes all flash write accesses on the hardware to fail. Example
>>> output
>>> after v5.1-rc2[1]:
>>>
>>>    root@host:~# mount -t jffs2 /dev/mtdblock0 /mnt
>>>    MTD do_write_buffer_wait(): software timeout, address:0x000c000b.
>>>    jffs2: Write clean marker to block at 0x000c0000 failed: -5
>>>
>>> This issue still persists with v5.16-rc. Reverting aforementioned
>>> patch fixes
>>> it, but I am still looking for a change that keeps both Tokunori's
>>> and my
>>> hardware happy.
>>>
>>> What Tokunori's patch did is that it strengthened the success condition
>>> for flash writes:
>>>
>>>   - Prior to the patch, DQ polling was done until bits
>>>     stopped toggling. This was taken as an indicator that the write
>>> succeeded
>>>     and was reported up the stack. i.e. success condition is
>>> chip_ready()
>>>
>>>   - After the patch, polling continues until the just written data is
>>>     actually read back, i.e. success condition is chip_good()
>>>
>>> This new condition never holds for me, when DQ stabilizes, it reads
>>> 0xFF,
>>> never the just written data. The data is still written and can be
>>> read back
>>> on subsequent reads, just not at that point of time in the poll loop.
>>>
>>> We haven't had write issues for the years predating that patch. As the
>>> regression has been mainline for a while, I am wondering what about
>>> my setup
>>> that makes it pop up here, but not elsewhere?
>>>
>>> I consulted the data sheet[2] and found Figure 27, which describes DQ
>>> polling
>>> during embedded algorithms. DQ switches from status output to "True"
>>> (I assume
>>> True == all bits set == 0xFF) until CS# is reasserted.
>>>
>>> I compared with another chip's datasheet, and it (Figure 8.4) doesn't
>>> describe
>>> such an intermittent "True" state. In any case, the driver polls a
>>> few hundred
>>> times, however, before giving up, so there should be enough CS# toggles.
>>>
>>>
>>> Locally, I'll revert this patch for now. I think accepting 0xFF as a
>>> success
>>> condition may be appropriate, but I don't yet have the rationale to
>>> back it up.
>>>
>>> I am investigating this some more, probably with a logic trace, but I
>>> wanted
>>> to report this in case someone has pointers and in case other people
>>> run into
>>> the same issue.
>>>
>>>
>>> Cheers,
>>> Ahmad
>>>
>>> [1] Prior to d9b8a67b3b95 ("mtd: cfi: fix deadloop in
>>> cfi_cmdset_0002.c do_write_buffer")
>>>      first included with v5.1-rc2, failing writes just hung
>>> indefinitely in kernel space.
>>>      That's fixed, but the writes still fail.
>>>
>>> [2]: 001-98525 Rev. *B,
>>> https://www.infineon.com/dgdl/Infineon-S29GL064N_S29GL032N_64_Mbit_32_Mbit_3_V_Page_Mode_MirrorBit_Flash-DataSheet-v03_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ed556fd548b
>>>
>>>
>>> [3]:
>>> https://www.mouser.com/datasheet/2/268/SST39VF1601C-SST39VF1602C-16-Mbit-x16-Multi-Purpos-709008.pdf
>>>
>>>       Note that "true data" means valid data here, not all bits one.
>>>


^ permalink raw reply

* RE: [PATCH V3 07/17] riscv: compat: Re-implement TASK_SIZE for COMPAT_32BIT
From: David Laight @ 2022-01-20 12:53 UTC (permalink / raw)
  To: 'Arnd Bergmann', Guo Ren
  Cc: linux-s390, Guo Ren, gregkh, Drew Fustini, Anup Patel,
	Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky@vger.kernel.org,
	inux-parisc@vger.kernel.org, Christoph Hellwig, Palmer Dabbelt,
	liush, sparclinux, linux-riscv, open list:BROADCOM NVRAM DRIVER,
	linuxppc-dev, Christoph Hellwig, Linux ARM, Wei Fu
In-Reply-To: <CAK8P3a1UvqsS-D7cVXBkp4KCRWDfquQ6QTkvrQ=FqLxhsAi7Rw@mail.gmail.com>

> > Limit 32-bit compatible process in 0-2GB virtual address range
> > (which is enough for real scenarios), because it could avoid
> > address sign extend problem when 32-bit enter 64-bit and ease
> > software design.

Eh?
I thought nearly all the other 32bit unix ports (of any flavour)
put the user-kernel boundary at 3GB.
(Apart from some very old sparc ones that use 3.5GB.)

2GB is used by Windows.

I think the x86-64 32bit compat code even puts the boundary at 4GB.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [PATCH V3 03/17] asm-generic: compat: Cleanup duplicate definitions
From: Arnd Bergmann @ 2022-01-20 11:52 UTC (permalink / raw)
  To: Guo Ren
  Cc: linux-s390, Guo Ren, Arnd Bergmann, gregkh, Drew Fustini,
	Anup Patel, Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky, inux-parisc,
	Christoph Hellwig, Palmer Dabbelt, liush, sparclinux, linux-riscv,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Christoph Hellwig,
	Linux ARM, Wei Fu
In-Reply-To: <20220120073911.99857-4-guoren@kernel.org>

  On Thu, Jan 20, 2022 at 8:38 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> There are 7 64bit architectures that support Linux COMPAT mode to
> run 32bit applications. A lot of definitions are duplicate:
>  - COMPAT_USER_HZ
>  - COMPAT_RLIM_INFINITY
>  - COMPAT_OFF_T_MAX
>  - __compat_uid_t, __compat_uid_t
>  - compat_dev_t
>  - compat_ipc_pid_t
>  - struct compat_flock
>  - struct compat_flock64
>  - struct compat_statfs
>  - struct compat_ipc64_perm, compat_semid64_ds,
>           compat_msqid64_ds, compat_shmid64_ds
>
> Cleanup duplicate definitions and merge them into asm-generic.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>

> ---
>  arch/arm64/include/asm/compat.h   | 108 +++-----------------------
>  arch/mips/include/asm/compat.h    |  24 ++----
>  arch/parisc/include/asm/compat.h  |  47 ++----------
>  arch/powerpc/include/asm/compat.h |  47 ++----------
>  arch/s390/include/asm/compat.h    | 109 +++-----------------------
>  arch/sparc/include/asm/compat.h   |  39 ++++------
>  arch/x86/include/asm/compat.h     | 114 +++-------------------------
>  include/asm-generic/compat.h      | 122 ++++++++++++++++++++++++++++++
>  8 files changed, 191 insertions(+), 419 deletions(-)
>
> diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
> index eaa6ca062d89..f54f295efae3 100644
> --- a/arch/arm64/include/asm/compat.h
> +++ b/arch/arm64/include/asm/compat.h
> @@ -5,9 +5,18 @@
>  #ifndef __ASM_COMPAT_H
>  #define __ASM_COMPAT_H
>
> +#define COMPAT_RLIM_INFINITY           0xffffffff
...
> +#ifndef COMPAT_RLIM_INFINITY
> +#define COMPAT_RLIM_INFINITY   0x7fffffff
> +#endif

While this is a correct conversion, I think the default should
be 0xffffffff, to match the asm-generic RLIM_INFINITY
definition, with only mips and sparc getting the exception

> -struct compat_flock {
> -       short           l_type;
> -       short           l_whence;
> -       compat_off_t    l_start;
> -       compat_off_t    l_len;
> -       compat_pid_t    l_pid;
> -};
...
> +#ifndef compat_flock
> +struct compat_flock {
> +       compat_short_t  l_type;
> +       compat_short_t  l_whence;
> +       compat_off_t    l_start;
> +       compat_off_t    l_len;
> +       compat_pid_t    l_pid;
> +} __attribute__((packed));
> +#endif

You are adding __attribute__((packed)) here, which I think has
no effect on the layout on the structure on any of the architectures
but it does change the alignment requirements needlessly.

Better leave it without the attribute.

> -struct compat_flock64 {
> -       short           l_type;
> -       short           l_whence;
> -       compat_loff_t   l_start;
> -       compat_loff_t   l_len;
> -       compat_pid_t    l_pid;
> -};
...
> +#ifndef compat_flock64
> +struct compat_flock64 {
> +       compat_short_t  l_type;
> +       compat_short_t  l_whence;
> +       compat_loff_t   l_start;
> +       compat_loff_t   l_len;
> +       compat_pid_t    l_pid;
> +} __attribute__((packed));
> +#endif

This one is different: on all architectures other than x86,
the added packed attribute changes the size of the
structure by removing the four padding bytes at the
end. x86 originally added the attribute here to work around
the weirdness of the x86-32 ABI that aligns 64-bit values
on a 4-byte boundary.

The easiest workaround would be to have x86 keep its
custom definition. A slightly nicer version would drop the
attribute on x86 as well but instead change the compat_loff_t
definition to use compat_s64 instead of s64, giving it the
correct alignment.

> -struct compat_statfs {
> -       int             f_type;
> -       int             f_bsize;
> -       int             f_blocks;
> -       int             f_bfree;
> -       int             f_bavail;
> -       int             f_files;
> -       int             f_ffree;
> -       compat_fsid_t   f_fsid;
> -       int             f_namelen;      /* SunOS ignores this field. */
> -       int             f_frsize;
> -       int             f_flags;
> -       int             f_spare[4];
> -};
...
> +#ifndef compat_statfs
> +struct compat_statfs {
> +       compat_uint_t   f_type;
> +       compat_uint_t   f_bsize;
> +       compat_uint_t   f_blocks;
> +       compat_uint_t   f_bfree;
> +       compat_uint_t   f_bavail;
> +       compat_uint_t   f_files;
> +       compat_uint_t   f_ffree;
> +       __kernel_fsid_t f_fsid;
> +       compat_uint_t   f_namelen;
> +       compat_uint_t   f_frsize;
> +       compat_uint_t   f_flags;
> +       compat_uint_t   f_spare[4];
> +} __attribute__((packed));
> +#endif

None of the architectures use the packed attribute at the moment,
so please don't add one here.

Changing compat_fsid_t to __kernel_fsid_t is harmless, but seems
unnecessary.

Changing the signed int to an unsigned int (regardless of notation)
may be a change in behavior. s390 is the only architecture
using unsigned members here at the moment, as of b8668fd0a7e1
("s390/uapi: change struct statfs[64] member types to unsigned
values").
The description of that patch sounds like this was changed to fix
a bug, but I don't see what the actual problem would be in the
put_compat_statfs().

For the moment I'd suggest leaving this with the signed version,
with s390 being another exception next to mips. We can follow-up
with merging s390 into the common definition using either the
signed or unsigned types, but I think that needs to be a separate
patch with a detailed explanation.

 +#ifndef compat_ipc64_perm
> +struct compat_ipc64_perm {
> +       compat_key_t key;
> +       __compat_uid32_t uid;
> +       __compat_gid32_t gid;
> +       __compat_uid32_t cuid;
> +       __compat_gid32_t cgid;
> +       compat_mode_t   mode;
> +       unsigned char   __pad1[4 - sizeof(compat_mode_t)];
> +       compat_ushort_t seq;
> +       compat_ushort_t __pad2;
> +       compat_ulong_t  unused1;
> +       compat_ulong_t  unused2;
> +} __attribute__((packed));
> +
> +struct compat_semid64_ds {
> +       struct compat_ipc64_perm sem_perm;
> +       compat_ulong_t sem_otime;
> +       compat_ulong_t sem_otime_high;
> +       compat_ulong_t sem_ctime;
> +       compat_ulong_t sem_ctime_high;
> +       compat_ulong_t sem_nsems;
> +       compat_ulong_t __unused3;
> +       compat_ulong_t __unused4;
> +} __attribute__((packed));
> +
> +struct compat_msqid64_ds {
> +       struct compat_ipc64_perm msg_perm;
> +       compat_ulong_t msg_stime;
> +       compat_ulong_t msg_stime_high;
> +       compat_ulong_t msg_rtime;
> +       compat_ulong_t msg_rtime_high;
> +       compat_ulong_t msg_ctime;
> +       compat_ulong_t msg_ctime_high;
> +       compat_ulong_t msg_cbytes;
> +       compat_ulong_t msg_qnum;
> +       compat_ulong_t msg_qbytes;
> +       compat_pid_t   msg_lspid;
> +       compat_pid_t   msg_lrpid;
> +       compat_ulong_t __unused4;
> +       compat_ulong_t __unused5;
> +} __attribute__((packed));
> +
> +struct compat_shmid64_ds {
> +       struct compat_ipc64_perm shm_perm;
> +       compat_size_t  shm_segsz;
> +       compat_ulong_t shm_atime;
> +       compat_ulong_t shm_atime_high;
> +       compat_ulong_t shm_dtime;
> +       compat_ulong_t shm_dtime_high;
> +       compat_ulong_t shm_ctime;
> +       compat_ulong_t shm_ctime_high;
> +       compat_pid_t   shm_cpid;
> +       compat_pid_t   shm_lpid;
> +       compat_ulong_t shm_nattch;
> +       compat_ulong_t __unused4;
> +       compat_ulong_t __unused5;
> +} __attribute__((packed));
> +#endif

I checked these in detail, looking at the seven architectures, and your
conversion looks exactly right (I had initially missed the part about
compat_mode_t that you got right).

As with compat_flock, the packed attribute has no impact on the layout
here, but please drop it anyway for consistency.

        Arnd

^ permalink raw reply

* Re: [PATCH V3 13/17] riscv: compat: signal: Add rt_frame implementation
From: Arnd Bergmann @ 2022-01-20  9:37 UTC (permalink / raw)
  To: Guo Ren
  Cc: linux-s390, Guo Ren, Arnd Bergmann, gregkh, Drew Fustini,
	Anup Patel, Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky, inux-parisc,
	Christoph Hellwig, Palmer Dabbelt, liush, sparclinux, linux-riscv,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Christoph Hellwig,
	Linux ARM, Wei Fu
In-Reply-To: <20220120073911.99857-14-guoren@kernel.org>

On Thu, Jan 20, 2022 at 8:39 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Implement compat_setup_rt_frame for sigcontext save & restore. The
> main process is the same with signal, but the rv32 pt_regs' size
> is different from rv64's, so we needs convert them.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>

I hope someone else can properly review this part, it's not my area
but it looks complex enough that it could bring subtle bugs.

       Arnd

^ permalink raw reply

* [PATCH v5] powerpc/mce: Avoid using irq_work_queue() in realmode
From: Ganesh Goudar @ 2022-01-20 12:19 UTC (permalink / raw)
  To: linuxppc-dev, mpe; +Cc: Ganesh Goudar, mahesh, npiggin, dja

In realmode mce handler we use irq_work_queue() to defer
the processing of mce events, irq_work_queue() can only
be called when translation is enabled because it touches
memory outside RMA, hence we enable translation before
calling irq_work_queue and disable on return, though it
is not safe to do in realmode.

To avoid this, program the decrementer and call the event
processing functions from timer handler.

Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
---
V2:
* Use arch_irq_work_raise to raise decrementer interrupt.
* Avoid having atomic variable.

V3:
* Fix build error.
  Reported by kernel test bot.

V4:
* Rename some functions and variables
* Remove mces_to_process counter and add a flag to indicate
  there is a mce info to process.

V5:
* Fix the build warning, reported by kernel test robot.
---
 arch/powerpc/include/asm/machdep.h       |  2 +
 arch/powerpc/include/asm/mce.h           | 13 +++++
 arch/powerpc/include/asm/paca.h          |  1 +
 arch/powerpc/kernel/mce.c                | 60 +++++++++++++-----------
 arch/powerpc/kernel/time.c               |  2 +
 arch/powerpc/platforms/pseries/pseries.h |  1 +
 arch/powerpc/platforms/pseries/ras.c     | 32 +------------
 arch/powerpc/platforms/pseries/setup.c   |  1 +
 8 files changed, 53 insertions(+), 59 deletions(-)

diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index e821037f74f0..36d2f34aa352 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -99,6 +99,8 @@ struct machdep_calls {
 	/* Called during machine check exception to retrive fixup address. */
 	bool		(*mce_check_early_recovery)(struct pt_regs *regs);
 
+	void            (*machine_check_log_err)(void);
+
 	/* Motherboard/chipset features. This is a kind of general purpose
 	 * hook used to control some machine specific features (like reset
 	 * lines, chip power control, etc...).
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index 331d944280b8..c9f0936bd3c9 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -235,8 +235,21 @@ extern void machine_check_print_event_info(struct machine_check_event *evt,
 unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr);
 extern void mce_common_process_ue(struct pt_regs *regs,
 				  struct mce_error_info *mce_err);
+void mce_irq_work_queue(void);
 int mce_register_notifier(struct notifier_block *nb);
 int mce_unregister_notifier(struct notifier_block *nb);
+
+#ifdef CONFIG_PPC_BOOK3S_64
+void mce_run_irq_context_handlers(void);
+#else
+static inline void mce_run_irq_context_handlers(void) { };
+#endif /* CONFIG_PPC_BOOK3S_64 */
+
+#ifdef CONFIG_PPC_BOOK3S_64
+void set_mce_pending_irq_work(void);
+void clear_mce_pending_irq_work(void);
+#endif /* CONFIG_PPC_BOOK3S_64 */
+
 #ifdef CONFIG_PPC_BOOK3S_64
 void flush_and_reload_slb(void);
 void flush_erat(void);
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 295573a82c66..8330968ca346 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -288,6 +288,7 @@ struct paca_struct {
 #endif
 #ifdef CONFIG_PPC_BOOK3S_64
 	struct mce_info *mce_info;
+	u8 mce_pending_irq_work;
 #endif /* CONFIG_PPC_BOOK3S_64 */
 } ____cacheline_aligned;
 
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index 2503dd4713b9..6cd4b1409874 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -28,19 +28,9 @@
 
 #include "setup.h"
 
-static void machine_check_process_queued_event(struct irq_work *work);
-static void machine_check_ue_irq_work(struct irq_work *work);
 static void machine_check_ue_event(struct machine_check_event *evt);
 static void machine_process_ue_event(struct work_struct *work);
 
-static struct irq_work mce_event_process_work = {
-        .func = machine_check_process_queued_event,
-};
-
-static struct irq_work mce_ue_event_irq_work = {
-	.func = machine_check_ue_irq_work,
-};
-
 static DECLARE_WORK(mce_ue_event_work, machine_process_ue_event);
 
 static BLOCKING_NOTIFIER_HEAD(mce_notifier_list);
@@ -89,6 +79,13 @@ static void mce_set_error_info(struct machine_check_event *mce,
 	}
 }
 
+void mce_irq_work_queue(void)
+{
+	/* Raise decrementer interrupt */
+	arch_irq_work_raise();
+	set_mce_pending_irq_work();
+}
+
 /*
  * Decode and save high level MCE information into per cpu buffer which
  * is an array of machine_check_event structure.
@@ -217,7 +214,7 @@ void release_mce_event(void)
 	get_mce_event(NULL, true);
 }
 
-static void machine_check_ue_irq_work(struct irq_work *work)
+static void machine_check_ue_work(void)
 {
 	schedule_work(&mce_ue_event_work);
 }
@@ -239,7 +236,7 @@ static void machine_check_ue_event(struct machine_check_event *evt)
 	       evt, sizeof(*evt));
 
 	/* Queue work to process this event later. */
-	irq_work_queue(&mce_ue_event_irq_work);
+	mce_irq_work_queue();
 }
 
 /*
@@ -249,7 +246,6 @@ void machine_check_queue_event(void)
 {
 	int index;
 	struct machine_check_event evt;
-	unsigned long msr;
 
 	if (!get_mce_event(&evt, MCE_EVENT_RELEASE))
 		return;
@@ -263,20 +259,7 @@ void machine_check_queue_event(void)
 	memcpy(&local_paca->mce_info->mce_event_queue[index],
 	       &evt, sizeof(evt));
 
-	/*
-	 * Queue irq work to process this event later. Before
-	 * queuing the work enable translation for non radix LPAR,
-	 * as irq_work_queue may try to access memory outside RMO
-	 * region.
-	 */
-	if (!radix_enabled() && firmware_has_feature(FW_FEATURE_LPAR)) {
-		msr = mfmsr();
-		mtmsr(msr | MSR_IR | MSR_DR);
-		irq_work_queue(&mce_event_process_work);
-		mtmsr(msr);
-	} else {
-		irq_work_queue(&mce_event_process_work);
-	}
+	mce_irq_work_queue();
 }
 
 void mce_common_process_ue(struct pt_regs *regs,
@@ -338,7 +321,7 @@ static void machine_process_ue_event(struct work_struct *work)
  * process pending MCE event from the mce event queue. This function will be
  * called during syscall exit.
  */
-static void machine_check_process_queued_event(struct irq_work *work)
+static void machine_check_process_queued_event(void)
 {
 	int index;
 	struct machine_check_event *evt;
@@ -363,6 +346,27 @@ static void machine_check_process_queued_event(struct irq_work *work)
 	}
 }
 
+void set_mce_pending_irq_work(void)
+{
+	local_paca->mce_pending_irq_work = 1;
+}
+
+void clear_mce_pending_irq_work(void)
+{
+	local_paca->mce_pending_irq_work = 0;
+}
+
+void mce_run_irq_context_handlers(void)
+{
+	if (unlikely(local_paca->mce_pending_irq_work)) {
+		if (ppc_md.machine_check_log_err)
+			ppc_md.machine_check_log_err();
+		machine_check_process_queued_event();
+		machine_check_ue_work();
+		clear_mce_pending_irq_work();
+	}
+}
+
 void machine_check_print_event_info(struct machine_check_event *evt,
 				    bool user_mode, bool in_guest)
 {
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 62361cc7281c..13a7360b6199 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -70,6 +70,7 @@
 #include <asm/vdso_datapage.h>
 #include <asm/firmware.h>
 #include <asm/asm-prototypes.h>
+#include <asm/mce.h>
 
 /* powerpc clocksource/clockevent code */
 
@@ -638,6 +639,7 @@ DEFINE_INTERRUPT_HANDLER_ASYNC(timer_interrupt)
 
 	if (test_irq_work_pending()) {
 		clear_irq_work_pending();
+		mce_run_irq_context_handlers();
 		irq_work_run();
 	}
 
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 56c9ef9052e9..af162aeeae86 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -21,6 +21,7 @@ struct pt_regs;
 extern int pSeries_system_reset_exception(struct pt_regs *regs);
 extern int pSeries_machine_check_exception(struct pt_regs *regs);
 extern long pseries_machine_check_realmode(struct pt_regs *regs);
+void pSeries_machine_check_log_err(void);
 
 #ifdef CONFIG_SMP
 extern void smp_init_pseries(void);
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 74c9b1b5bc66..f2e9eb2195f1 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -23,11 +23,6 @@ static DEFINE_SPINLOCK(ras_log_buf_lock);
 
 static int ras_check_exception_token;
 
-static void mce_process_errlog_event(struct irq_work *work);
-static struct irq_work mce_errlog_process_work = {
-	.func = mce_process_errlog_event,
-};
-
 #define EPOW_SENSOR_TOKEN	9
 #define EPOW_SENSOR_INDEX	0
 
@@ -717,7 +712,6 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
 	struct pseries_errorlog *pseries_log;
 	struct pseries_mc_errorlog *mce_log = NULL;
 	int disposition = rtas_error_disposition(errp);
-	unsigned long msr;
 	u8 error_type;
 
 	if (!rtas_error_extended(errp))
@@ -731,40 +725,16 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
 	error_type = mce_log->error_type;
 
 	disposition = mce_handle_err_realmode(disposition, error_type);
-
-	/*
-	 * Enable translation as we will be accessing per-cpu variables
-	 * in save_mce_event() which may fall outside RMO region, also
-	 * leave it enabled because subsequently we will be queuing work
-	 * to workqueues where again per-cpu variables accessed, besides
-	 * fwnmi_release_errinfo() crashes when called in realmode on
-	 * pseries.
-	 * Note: All the realmode handling like flushing SLB entries for
-	 *       SLB multihit is done by now.
-	 */
 out:
-	msr = mfmsr();
-	mtmsr(msr | MSR_IR | MSR_DR);
-
 	disposition = mce_handle_err_virtmode(regs, errp, mce_log,
 					      disposition);
-
-	/*
-	 * Queue irq work to log this rtas event later.
-	 * irq_work_queue uses per-cpu variables, so do this in virt
-	 * mode as well.
-	 */
-	irq_work_queue(&mce_errlog_process_work);
-
-	mtmsr(msr);
-
 	return disposition;
 }
 
 /*
  * Process MCE rtas errlog event.
  */
-static void mce_process_errlog_event(struct irq_work *work)
+void pSeries_machine_check_log_err(void)
 {
 	struct rtas_error_log *err;
 
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 83a04d967a59..069d7b3bb142 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -1086,6 +1086,7 @@ define_machine(pseries) {
 	.system_reset_exception = pSeries_system_reset_exception,
 	.machine_check_early	= pseries_machine_check_realmode,
 	.machine_check_exception = pSeries_machine_check_exception,
+	.machine_check_log_err	= pSeries_machine_check_log_err,
 #ifdef CONFIG_KEXEC_CORE
 	.machine_kexec          = pSeries_machine_kexec,
 	.kexec_cpu_down         = pseries_kexec_cpu_down,
-- 
2.31.1


^ permalink raw reply related

* Re: [PATCH V3 16/17] riscv: compat: Add COMPAT Kbuild skeletal support
From: Arnd Bergmann @ 2022-01-20  9:41 UTC (permalink / raw)
  To: Guo Ren
  Cc: linux-s390, Guo Ren, Arnd Bergmann, gregkh, Drew Fustini,
	Anup Patel, Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky, inux-parisc,
	Christoph Hellwig, Palmer Dabbelt, liush, sparclinux, linux-riscv,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Christoph Hellwig,
	Linux ARM, Wei Fu
In-Reply-To: <20220120073911.99857-17-guoren@kernel.org>

On Thu, Jan 20, 2022 at 8:39 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Adds initial skeletal COMPAT Kbuild (Runing 32bit U-mode on 64bit
> S-mode) support.
>  - Setup kconfig & dummy functions for compiling.
>  - Implement compat_start_thread by the way.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* Re: [PATCH V3 07/17] riscv: compat: Re-implement TASK_SIZE for COMPAT_32BIT
From: Arnd Bergmann @ 2022-01-20  9:31 UTC (permalink / raw)
  To: Guo Ren
  Cc: linux-s390, Guo Ren, Arnd Bergmann, gregkh, Drew Fustini,
	Anup Patel, Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky, inux-parisc,
	Christoph Hellwig, Palmer Dabbelt, liush, sparclinux, linux-riscv,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Christoph Hellwig,
	Linux ARM, Wei Fu
In-Reply-To: <20220120073911.99857-8-guoren@kernel.org>

On Thu, Jan 20, 2022 at 8:39 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Make TASK_SIZE from const to dynamic detect TIF_32BIT flag
> function. Refer to arm64 to implement DEFAULT_MAP_WINDOW_64 for
> efi-stub.
>
> Limit 32-bit compatible process in 0-2GB virtual address range
> (which is enough for real scenarios), because it could avoid
> address sign extend problem when 32-bit enter 64-bit and ease
> software design.
>
> The standard 32-bit TASK_SIZE is 0x9dc00000:FIXADDR_START, and
> compared to a compatible 32-bit, it increases 476MB for the
> application's virtual address.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* Re: [PATCH V3 17/17] KVM: compat: riscv: Prevent KVM_COMPAT from being selected
From: Arnd Bergmann @ 2022-01-20 10:10 UTC (permalink / raw)
  To: Guo Ren
  Cc: linux-s390, Guo Ren, Arnd Bergmann, gregkh, Drew Fustini,
	Anup Patel, Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky, inux-parisc,
	Christoph Hellwig, Palmer Dabbelt, liush, sparclinux, linux-riscv,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Christoph Hellwig,
	Linux ARM, Wei Fu
In-Reply-To: <20220120073911.99857-18-guoren@kernel.org>

On Thu, Jan 20, 2022 at 8:39 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Current riscv doesn't support the 32bit KVM/arm API. Let's make it
> clear by not selecting KVM_COMPAT.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> ---
>  virt/kvm/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> index f4834c20e4a6..a8c5c9f06b3c 100644
> --- a/virt/kvm/Kconfig
> +++ b/virt/kvm/Kconfig
> @@ -53,7 +53,7 @@ config KVM_GENERIC_DIRTYLOG_READ_PROTECT
>
>  config KVM_COMPAT
>         def_bool y
> -       depends on KVM && COMPAT && !(S390 || ARM64)
> +       depends on KVM && COMPAT && !(S390 || ARM64 || RISCV)

Maybe this should be flipped around into a positive list now?
The remaining architectures would be mips, powerpc and x86, but it's unclear
if this actually meant to work on all of them, or any potential ones
added in the
future.

       Arnd

^ permalink raw reply

* Re: [PATCH v2 1/2] powerpc: Fix virt_addr_valid() check
From: Kefeng Wang @ 2022-01-20 11:09 UTC (permalink / raw)
  To: Christophe Leroy, Nicholas Piggin, Andrew Morton,
	Benjamin Herrenschmidt, Kees Cook, Laura Abbott,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linuxppc-dev@lists.ozlabs.org, Mark Rutland, Michael Ellerman,
	Paul Mackerras
In-Reply-To: <017744c1-1252-a25c-3dcc-057ee18d0769@csgroup.eu>


On 2022/1/20 15:31, Christophe Leroy wrote:
>
> Le 19/01/2022 à 02:15, Kefeng Wang a écrit :
>> On 2022/1/11 14:04, Christophe Leroy wrote:
>>> Le 11/01/2022 à 05:37, Nicholas Piggin a écrit :
>>>> Excerpts from Kefeng Wang's message of January 8, 2022 9:58 pm:
>>>>> Hi PPC maintainers, ping..
>>>> Hmm. I might have confused myself about this. I'm going back and
>>>> trying to work out what I was thinking when I suggested it. This
>>>> works on 64e because vmalloc space is below the kernel linear map,
>>>> right?
>>>>
>>>> On 64s it is the other way around and it is still possible to enable
>>>> flatmem on 64s. Altough we might just not hit the problem there because
>>>> __pa() will not mask away the vmalloc offset for 64s so it will still
>>>> return something that's outside the pfn_valid range for flatmem. That's
>>>> very subtle though.
>>> That's the way it works on PPC32 at least, so for me it's not chocking
>>> to have it work the same way on PPC64s.
>>>
>>> The main issue here is the way __pa() works. On PPC32 __pa = va -
>>> PAGE_OFFSET, so it works correctly for any address.
>>> On PPC64, __pa() works by masking out the 2 top bits instead of
>>> substracting PAGE_OFFSET, so the test must add a verification that we
>>> really have the 2 top bits set at first. This is what (addr >=
>>> PAGE_OFFSET) does. Once this first test is done, we can perfectly rely
>>> on pfn_valid() just like PPC32, I see absolutely no point in an
>>> additionnal test checking the addr is below KERN_VIRT_START.
>>
>> Hi Christophe and Nicholas, for ppc32, I think we need check the upper
>> limit,
> Why ? Have you experimented any problem at all on PPC32 with the way it
> is done at the moment ?
>
> I don't think we have to change PPC32 at all unless we have a real
> reason to do it.

yes, I missed this commit in old kernel(lts5.10), you have fixed the 
upper limit.

commit 602946ec2f90d5bd965857753880db29d2d9a1e9
Author: Christophe Leroy <christophe.leroy@csgroup.eu>
Date:   Tue Oct 12 12:40:37 2021 +0200

     powerpc: Set max_mapnr correctly


>
>> eg,  addr >= PAGE_OFFSET && addr < high_memory
> Isn't it exactly what pfn_valid() already do today ?
> Why change that at all ?
>
> Christophe
>
>> arch/powerpc/mm/mem.c:  high_memory = (void *) __va(max_low_pfn *
>> PAGE_SIZE);
>>
>> for ppc32 max_low_pfn is the upper low memory pfn,  and For ppc64,
>> high_memory is
>>
>> the max memory pfn, it looks good too, correct me if I'm wrong, if the
>> above check
>>
>> is ok, I will send a new v3,  thanks.
>>
>>
>>
>>
>>>
>>>> The checks added to __pa actually don't prevent vmalloc memory from
>>>> being passed to it either on 64s, only a more basic test.
>>> That's correct. It is the role of pfn_valid() to check that.
>>>
>>> Christophe
>>>
>>>> I think 64s wants (addr >= PAGE_OFFSET && addr < KERN_VIRT_START) as
>>>> the condition.  Could possibly add that check to __pa as well to
>>>> catch vmalloc addresses.
>>>>
>>>> Thanks,
>>>> Nick
>>>>
>>> >

^ permalink raw reply

* Re: [PATCH] powerpc/fixmap: Fix VM debug warning on unmap
From: Christophe Leroy @ 2022-01-20 10:56 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Paul Mackerras, Maxime Bizon
In-Reply-To: <b0b752f6f6ecc60653e873f385c6f0dce4e9ab6a.1638789098.git.christophe.leroy@csgroup.eu>

Hi Michael,

ping ?

Le 06/12/2021 à 12:11, Christophe Leroy a écrit :
> Unmapping a fixmap entry is done by calling __set_fixmap()
> with FIXMAP_PAGE_CLEAR as flags.
> 
> Today, powerpc __set_fixmap() calls map_kernel_page().
> 
> map_kernel_page() is not happy when called a second time
> for the same page.
> 
> 	WARNING: CPU: 0 PID: 1 at arch/powerpc/mm/pgtable.c:194 set_pte_at+0xc/0x1e8
> 	CPU: 0 PID: 1 Comm: swapper Not tainted 5.16.0-rc3-s3k-dev-01993-g350ff07feb7d-dirty #682
> 	NIP:  c0017cd4 LR: c00187f0 CTR: 00000010
> 	REGS: e1011d50 TRAP: 0700   Not tainted  (5.16.0-rc3-s3k-dev-01993-g350ff07feb7d-dirty)
> 	MSR:  00029032 <EE,ME,IR,DR,RI>  CR: 42000208  XER: 00000000
> 
> 	GPR00: c0165fec e1011e10 c14c0000 c0ee2550 ff800000 c0f3d000 00000000 c001686c
> 	GPR08: 00001000 b00045a9 00000001 c0f58460 c0f50000 00000000 c0007e10 00000000
> 	GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 	GPR24: 00000000 00000000 c0ee2550 00000000 c0f57000 00000ff8 00000000 ff800000
> 	NIP [c0017cd4] set_pte_at+0xc/0x1e8
> 	LR [c00187f0] map_kernel_page+0x9c/0x100
> 	Call Trace:
> 	[e1011e10] [c0736c68] vsnprintf+0x358/0x6c8 (unreliable)
> 	[e1011e30] [c0165fec] __set_fixmap+0x30/0x44
> 	[e1011e40] [c0c13bdc] early_iounmap+0x11c/0x170
> 	[e1011e70] [c0c06cb0] ioremap_legacy_serial_console+0x88/0xc0
> 	[e1011e90] [c0c03634] do_one_initcall+0x80/0x178
> 	[e1011ef0] [c0c0385c] kernel_init_freeable+0xb4/0x250
> 	[e1011f20] [c0007e34] kernel_init+0x24/0x140
> 	[e1011f30] [c0016268] ret_from_kernel_thread+0x5c/0x64
> 	Instruction dump:
> 	7fe3fb78 48019689 80010014 7c630034 83e1000c 5463d97e 7c0803a6 38210010
> 	4e800020 81250000 712a0001 41820008 <0fe00000> 9421ffe0 93e1001c 48000030
> 
> Implement unmap_kernel_page() which clears an existing pte.
> 
> Reported-by: Maxime Bizon <mbizon@freebox.fr>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>   arch/powerpc/include/asm/book3s/32/pgtable.h | 1 +
>   arch/powerpc/include/asm/book3s/64/pgtable.h | 2 ++
>   arch/powerpc/include/asm/fixmap.h            | 6 ++++--
>   arch/powerpc/include/asm/nohash/32/pgtable.h | 1 +
>   arch/powerpc/include/asm/nohash/64/pgtable.h | 1 +
>   arch/powerpc/mm/pgtable.c                    | 9 +++++++++
>   6 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
> index 609c80f67194..f8b94f78403f 100644
> --- a/arch/powerpc/include/asm/book3s/32/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
> @@ -178,6 +178,7 @@ static inline bool pte_user(pte_t pte)
>   #ifndef __ASSEMBLY__
>   
>   int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot);
> +void unmap_kernel_page(unsigned long va);
>   
>   #endif /* !__ASSEMBLY__ */
>   
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index 33e073d6b0c4..875730d5af40 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -1082,6 +1082,8 @@ static inline int map_kernel_page(unsigned long ea, unsigned long pa, pgprot_t p
>   	return hash__map_kernel_page(ea, pa, prot);
>   }
>   
> +void unmap_kernel_page(unsigned long va);
> +
>   static inline int __meminit vmemmap_create_mapping(unsigned long start,
>   						   unsigned long page_size,
>   						   unsigned long phys)
> diff --git a/arch/powerpc/include/asm/fixmap.h b/arch/powerpc/include/asm/fixmap.h
> index 947b5b9c4424..a832aeafe560 100644
> --- a/arch/powerpc/include/asm/fixmap.h
> +++ b/arch/powerpc/include/asm/fixmap.h
> @@ -111,8 +111,10 @@ static inline void __set_fixmap(enum fixed_addresses idx,
>   		BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
>   	else if (WARN_ON(idx >= __end_of_fixed_addresses))
>   		return;
> -
> -	map_kernel_page(__fix_to_virt(idx), phys, flags);
> +	if (pgprot_val(flags))
> +		map_kernel_page(__fix_to_virt(idx), phys, flags);
> +	else
> +		unmap_kernel_page(__fix_to_virt(idx));
>   }
>   
>   #define __early_set_fixmap	__set_fixmap
> diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h
> index b67742e2a9b2..d959c2a73fbf 100644
> --- a/arch/powerpc/include/asm/nohash/32/pgtable.h
> +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
> @@ -64,6 +64,7 @@ extern int icache_44x_need_flush;
>   #ifndef __ASSEMBLY__
>   
>   int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot);
> +void unmap_kernel_page(unsigned long va);
>   
>   #endif /* !__ASSEMBLY__ */
>   
> diff --git a/arch/powerpc/include/asm/nohash/64/pgtable.h b/arch/powerpc/include/asm/nohash/64/pgtable.h
> index 9d2905a47410..2225991c69b5 100644
> --- a/arch/powerpc/include/asm/nohash/64/pgtable.h
> +++ b/arch/powerpc/include/asm/nohash/64/pgtable.h
> @@ -308,6 +308,7 @@ static inline void __ptep_set_access_flags(struct vm_area_struct *vma,
>   #define __swp_entry_to_pte(x)		__pte((x).val)
>   
>   int map_kernel_page(unsigned long ea, unsigned long pa, pgprot_t prot);
> +void unmap_kernel_page(unsigned long va);
>   extern int __meminit vmemmap_create_mapping(unsigned long start,
>   					    unsigned long page_size,
>   					    unsigned long phys);
> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
> index ce9482383144..b7385e637e3e 100644
> --- a/arch/powerpc/mm/pgtable.c
> +++ b/arch/powerpc/mm/pgtable.c
> @@ -203,6 +203,15 @@ void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
>   	__set_pte_at(mm, addr, ptep, pte, 0);
>   }
>   
> +void unmap_kernel_page(unsigned long va)
> +{
> +	pmd_t *pmdp = pmd_off_k(va);
> +	pte_t *ptep = pte_offset_kernel(pmdp, va);
> +
> +	pte_clear(&init_mm, va, ptep);
> +	flush_tlb_kernel_range(va, va + PAGE_SIZE);
> +}
> +
>   /*
>    * This is called when relaxing access to a PTE. It's also called in the page
>    * fault path when we don't hit any of the major fault cases, ie, a minor

^ permalink raw reply

* Re: [PATCH V3 15/17] riscv: compat: Add UXL_32 support in start_thread
From: Arnd Bergmann @ 2022-01-20  9:39 UTC (permalink / raw)
  To: Guo Ren
  Cc: linux-s390, Guo Ren, Arnd Bergmann, gregkh, Drew Fustini,
	Anup Patel, Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky, inux-parisc,
	Christoph Hellwig, Palmer Dabbelt, liush, sparclinux, linux-riscv,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Christoph Hellwig,
	Linux ARM, Wei Fu
In-Reply-To: <20220120073911.99857-16-guoren@kernel.org>

On Thu, Jan 20, 2022 at 8:39 AM <guoren@kernel.org> wrote:
> +
> +#ifdef CONFIG_COMPAT
> +       if (is_compat_task())
> +               regs->status |= SR_UXL_32;
> +#endif


You should not need that #ifdef, as the is_compat_task() definition is
meant to drop the code at compile time, unless the SR_UXL_32
definition is not visible here.

         Arnd

^ permalink raw reply

* Re: [PATCH V3 10/17] riscv: compat: Add elf.h implementation
From: Arnd Bergmann @ 2022-01-20  9:36 UTC (permalink / raw)
  To: Guo Ren
  Cc: linux-s390, Guo Ren, Arnd Bergmann, gregkh, Drew Fustini,
	Anup Patel, Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky, inux-parisc,
	Christoph Hellwig, Palmer Dabbelt, liush, sparclinux, linux-riscv,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Christoph Hellwig,
	Linux ARM, Wei Fu
In-Reply-To: <20220120073911.99857-11-guoren@kernel.org>

On Thu, Jan 20, 2022 at 8:39 AM <guoren@kernel.org> wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Implement necessary type and macro for compat elf. See the code
> comment for detail.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

> +
> +/*
> + * FIXME: not sure SET_PERSONALITY for compat process is right!
> + */
> +#define SET_PERSONALITY(ex)                                    \
> +do {    if ((ex).e_ident[EI_CLASS] == ELFCLASS32)              \
> +               set_thread_flag(TIF_32BIT);                     \
> +       else                                                    \
> +               clear_thread_flag(TIF_32BIT);                   \
> +       if (personality(current->personality) != PER_LINUX32)   \
> +               set_personality(PER_LINUX |                     \
> +                       (current->personality & (~PER_MASK)));  \
> +} while (0)
> +

The implementation looks good to me now,  you can remove that comment above it.

        Arnd

^ permalink raw reply

* Re: [PATCH V3 06/17] riscv: compat: Add basic compat date type implementation
From: Arnd Bergmann @ 2022-01-20  9:29 UTC (permalink / raw)
  To: Guo Ren
  Cc: linux-s390, Guo Ren, Arnd Bergmann, gregkh, Drew Fustini,
	Anup Patel, Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky, inux-parisc,
	Christoph Hellwig, Palmer Dabbelt, liush, sparclinux, linux-riscv,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Christoph Hellwig,
	Linux ARM, Wei Fu
In-Reply-To: <20220120073911.99857-7-guoren@kernel.org>

On Thu, Jan 20, 2022 at 8:38 AM <guoren@kernel.org> wrote:

> @@ -0,0 +1,136 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __ASM_COMPAT_H
> +#define __ASM_COMPAT_H
> +
> +#define compat_mode_t  compat_mode_t
> +typedef u16            compat_mode_t;

I think this one is wrong, as rv32 should get the native definition from

include/uapi/asm-generic/posix_types.h:typedef unsigned int     __kernel_mode_t;

I think it works if you just remove those two lines. The rest looks good to me.

       Arnd

^ permalink raw reply

* Re: [PATCH V3 04/17] syscalls: compat: Fix the missing part for __SYSCALL_COMPAT
From: Arnd Bergmann @ 2022-01-20  9:26 UTC (permalink / raw)
  To: Guo Ren
  Cc: linux-s390, Guo Ren, Arnd Bergmann, gregkh, Drew Fustini,
	Anup Patel, Wang Junqiang, the arch/x86 maintainers,
	Linux Kernel Mailing List, linux-csky, inux-parisc,
	Christoph Hellwig, Palmer Dabbelt, liush, sparclinux, linux-riscv,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Christoph Hellwig,
	Linux ARM, Wei Fu
In-Reply-To: <20220120073911.99857-5-guoren@kernel.org>

On Thu, Jan 20, 2022 at 8:38 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Make "uapi asm unistd.h" could be used for architectures' COMPAT
> mode. The __SYSCALL_COMPAT is first used in riscv.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox