Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 7/8] arm64, kexec: configure transitional page table for kexec
From: Pavel Tatashin @ 2019-08-01 15:24 UTC (permalink / raw)
  To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
	corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
	james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190801152439.11363-1-pasha.tatashin@soleen.com>

Configure a page table located in kexec-safe memory that has
the following mappings:

1. identity mapping for text of relocation function with executable permission.
2. identity mapping for argument for relocation function.
3. linear mappings for all source ranges
4. linear mappings for all destination ranges.

Also, configure el2_vector, that is used to jump to new kernel from EL2 on
non-VHE kernels.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 arch/arm64/include/asm/kexec.h      |  32 +++++++
 arch/arm64/kernel/asm-offsets.c     |   6 ++
 arch/arm64/kernel/machine_kexec.c   | 129 ++++++++++++++++++++++++++--
 arch/arm64/kernel/relocate_kernel.S |  16 +++-
 4 files changed, 174 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index d5b79d4c7fae..450d8440f597 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -90,6 +90,23 @@ static inline void crash_prepare_suspend(void) {}
 static inline void crash_post_resume(void) {}
 #endif
 
+#if defined(CONFIG_KEXEC_CORE)
+/* Global variables for the arm64_relocate_new_kernel routine. */
+extern const unsigned char arm64_relocate_new_kernel[];
+extern const unsigned long arm64_relocate_new_kernel_size;
+
+/* Body of the vector for escalating to EL2 from relocation routine */
+extern const unsigned char kexec_el1_sync[];
+extern const unsigned long kexec_el1_sync_size;
+
+#define KEXEC_EL2_VECTOR_TABLE_SIZE	2048
+#define KEXEC_EL2_SYNC_OFFSET		(KEXEC_EL2_VECTOR_TABLE_SIZE / 2)
+
+#endif
+
+#define KEXEC_SRC_START	PAGE_OFFSET
+#define KEXEC_DST_START	(PAGE_OFFSET + \
+			((UL(0xffffffffffffffff) - PAGE_OFFSET) >> 1) + 1)
 /*
  * kern_reloc_arg is passed to kernel relocation function as an argument.
  * head		kimage->head, allows to traverse through relocation segments.
@@ -97,6 +114,15 @@ static inline void crash_post_resume(void) {}
  *		kernel, or purgatory entry address).
  * kern_arg0	first argument to kernel is its dtb address. The other
  *		arguments are currently unused, and must be set to 0
+ * trans_ttbr0	idmap for relocation function and its argument
+ * trans_ttbr1	linear map for source/destination addresses.
+ * el2_vector	If present means that relocation routine will go to EL1
+ *		from EL2 to do the copy, and then back to EL2 to do the jump
+ *		to new world. This vector contains only the final jump
+ *		instruction at KEXEC_EL2_SYNC_OFFSET.
+ * src_addr	linear map for source pages.
+ * dst_addr	linear map for destination pages.
+ * copy_len	Number of bytes that need to be copied
  */
 struct kern_reloc_arg {
 	unsigned long	head;
@@ -105,6 +131,12 @@ struct kern_reloc_arg {
 	unsigned long	kern_arg1;
 	unsigned long	kern_arg2;
 	unsigned long	kern_arg3;
+	unsigned long	trans_ttbr0;
+	unsigned long	trans_ttbr1;
+	unsigned long	el2_vector;
+	unsigned long	src_addr;
+	unsigned long	dst_addr;
+	unsigned long	copy_len;
 };
 
 #define ARCH_HAS_KIMAGE_ARCH
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 900394907fd8..7c2ba09a8ceb 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -135,6 +135,12 @@ int main(void)
   DEFINE(KRELOC_KERN_ARG1,	offsetof(struct kern_reloc_arg, kern_arg1));
   DEFINE(KRELOC_KERN_ARG2,	offsetof(struct kern_reloc_arg, kern_arg2));
   DEFINE(KRELOC_KERN_ARG3,	offsetof(struct kern_reloc_arg, kern_arg3));
+  DEFINE(KRELOC_TRANS_TTBR0,	offsetof(struct kern_reloc_arg, trans_ttbr0));
+  DEFINE(KRELOC_TRANS_TTBR1,	offsetof(struct kern_reloc_arg, trans_ttbr1));
+  DEFINE(KRELOC_EL2_VECTOR,	offsetof(struct kern_reloc_arg, el2_vector));
+  DEFINE(KRELOC_SRC_ADDR,	offsetof(struct kern_reloc_arg, src_addr));
+  DEFINE(KRELOC_DST_ADDR,	offsetof(struct kern_reloc_arg, dst_addr));
+  DEFINE(KRELOC_COPY_LEN,	offsetof(struct kern_reloc_arg, copy_len));
 #endif
   return 0;
 }
diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c
index d745ea2051df..16f761fc50c8 100644
--- a/arch/arm64/kernel/machine_kexec.c
+++ b/arch/arm64/kernel/machine_kexec.c
@@ -20,13 +20,10 @@
 #include <asm/mmu.h>
 #include <asm/mmu_context.h>
 #include <asm/page.h>
+#include <asm/trans_table.h>
 
 #include "cpu-reset.h"
 
-/* Global variables for the arm64_relocate_new_kernel routine. */
-extern const unsigned char arm64_relocate_new_kernel[];
-extern const unsigned long arm64_relocate_new_kernel_size;
-
 /**
  * kexec_image_info - For debugging output.
  */
@@ -72,15 +69,128 @@ static void *kexec_page_alloc(void *arg)
 	return page_address(page);
 }
 
+/*
+ * Map source segments starting from KEXEC_SRC_START, and map destination
+ * segments starting from KEXEC_DST_START, and return size of copy in
+ * *copy_len argument.
+ * Relocation function essentially needs to do:
+ * memcpy(KEXEC_DST_START, KEXEC_SRC_START, copy_len);
+ */
+static int map_segments(struct kimage *kimage, pgd_t *pgdp,
+			struct trans_table_info *info,
+			unsigned long *copy_len)
+{
+	unsigned long *ptr = 0;
+	unsigned long dest = 0;
+	unsigned long src_va = KEXEC_SRC_START;
+	unsigned long dst_va = KEXEC_DST_START;
+	unsigned long len = 0;
+	unsigned long entry, addr;
+	int rc;
+
+	for (entry = kimage->head; !(entry & IND_DONE); entry = *ptr++) {
+		addr = entry & PAGE_MASK;
+
+		switch (entry & IND_FLAGS) {
+		case IND_DESTINATION:
+			dest = addr;
+			break;
+		case IND_INDIRECTION:
+			ptr = __va(addr);
+			if (rc)
+				return rc;
+			break;
+		case IND_SOURCE:
+			rc = trans_table_map_page(info, pgdp, __va(addr),
+						  src_va, PAGE_KERNEL);
+			if (rc)
+				return rc;
+			rc = trans_table_map_page(info, pgdp, __va(dest),
+						  dst_va, PAGE_KERNEL);
+			if (rc)
+				return rc;
+			dest += PAGE_SIZE;
+			src_va += PAGE_SIZE;
+			dst_va += PAGE_SIZE;
+			len += PAGE_SIZE;
+		}
+	}
+	*copy_len = len;
+
+	return 0;
+}
+
+static int mmu_relocate_setup(struct kimage *kimage, unsigned long kern_reloc,
+			      struct kern_reloc_arg *kern_reloc_arg)
+{
+	struct trans_table_info info = {
+		.trans_alloc_page	= kexec_page_alloc,
+		.trans_alloc_arg	= kimage,
+		.trans_flags		= 0,
+	};
+	pgd_t *trans_ttbr0, *trans_ttbr1;
+	int rc;
+
+	rc = trans_table_create_empty(&info, &trans_ttbr0);
+	if (rc)
+		return rc;
+
+	rc = trans_table_create_empty(&info, &trans_ttbr1);
+	if (rc)
+		return rc;
+
+	rc = map_segments(kimage, trans_ttbr1, &info,
+			  &kern_reloc_arg->copy_len);
+	if (rc)
+		return rc;
+
+	/* Map relocation function va == pa */
+	rc = trans_table_map_page(&info, trans_ttbr0,  __va(kern_reloc),
+				  kern_reloc, PAGE_KERNEL_EXEC);
+	if (rc)
+		return rc;
+
+	/* Map relocation function argument va == pa */
+	rc = trans_table_map_page(&info, trans_ttbr0, kern_reloc_arg,
+				  __pa(kern_reloc_arg), PAGE_KERNEL);
+	if (rc)
+		return rc;
+
+	kern_reloc_arg->trans_ttbr0 = phys_to_ttbr(__pa(trans_ttbr0));
+	kern_reloc_arg->trans_ttbr1 = phys_to_ttbr(__pa(trans_ttbr1));
+	kern_reloc_arg->src_addr = KEXEC_SRC_START;
+	kern_reloc_arg->dst_addr = KEXEC_DST_START;
+
+	return 0;
+}
+
 int machine_kexec_post_load(struct kimage *kimage)
 {
+	unsigned long el2_vector = 0;
 	unsigned long kern_reloc;
 	struct kern_reloc_arg *kern_reloc_arg;
+	int rc = 0;
+
+	/*
+	 * Sanity check that relocation function + el2_vector fit into one
+	 * page.
+	 */
+	if (arm64_relocate_new_kernel_size > KEXEC_EL2_VECTOR_TABLE_SIZE) {
+		pr_err("can't fit relocation function and el2_vector in one page");
+		return -ENOMEM;
+	}
 
 	kern_reloc = page_to_phys(kimage->control_code_page);
 	memcpy(__va(kern_reloc), arm64_relocate_new_kernel,
 	       arm64_relocate_new_kernel_size);
 
+	/* Setup vector table only when EL2 is available, but no VHE */
+	if (is_hyp_mode_available() && !is_kernel_in_hyp_mode()) {
+		el2_vector = kern_reloc + KEXEC_EL2_VECTOR_TABLE_SIZE;
+		memcpy(__va(el2_vector + KEXEC_EL2_SYNC_OFFSET), kexec_el1_sync,
+		       kexec_el1_sync_size);
+	}
+
 	kern_reloc_arg = kexec_page_alloc(kimage);
 	if (!kern_reloc_arg)
 		return -ENOMEM;
@@ -91,10 +201,19 @@ int machine_kexec_post_load(struct kimage *kimage)
 
 	kern_reloc_arg->head = kimage->head;
 	kern_reloc_arg->entry_addr = kimage->start;
+	kern_reloc_arg->el2_vector = el2_vector;
 	kern_reloc_arg->kern_arg0 = kimage->arch.dtb_mem;
 
+	/*
+	 * If relocation is not needed, we do not need to enable MMU in
+	 * relocation routine, therefore do not create page tables for
+	 * scenarios such as crash kernel
+	 */
+	if (!(kimage->head & IND_DONE))
+		rc = mmu_relocate_setup(kimage, kern_reloc, kern_reloc_arg);
+
 	kexec_image_info(kimage);
-	return 0;
+	return rc;
 }
 
 /**
diff --git a/arch/arm64/kernel/relocate_kernel.S b/arch/arm64/kernel/relocate_kernel.S
index d352faf7cbe6..14243a678277 100644
--- a/arch/arm64/kernel/relocate_kernel.S
+++ b/arch/arm64/kernel/relocate_kernel.S
@@ -83,17 +83,25 @@ ENTRY(arm64_relocate_new_kernel)
 	ldr	x1, [x0, #KRELOC_KERN_ARG1]
 	ldr	x0, [x0, #KRELOC_KERN_ARG0]	/* x0 = dtb address */
 	br	x4
+.ltorg
+.Larm64_relocate_new_kernel_end:
 END(arm64_relocate_new_kernel)
 
-.ltorg
+ENTRY(kexec_el1_sync)
+	br	x4				/* Jump to new world from el2 */
+.Lkexec_el1_sync_end:
+END(kexec_el1_sync)
+
 .align 3	/* To keep the 64-bit values below naturally aligned. */
-.Lcopy_end:
 .org	KEXEC_CONTROL_PAGE_SIZE
-
 /*
  * arm64_relocate_new_kernel_size - Number of bytes to copy to the
  * control_code_page.
  */
 .globl arm64_relocate_new_kernel_size
 arm64_relocate_new_kernel_size:
-	.quad	.Lcopy_end - arm64_relocate_new_kernel
+	.quad	.Larm64_relocate_new_kernel_end - arm64_relocate_new_kernel
+
+.globl kexec_el1_sync_size
+kexec_el1_sync_size:
+	.quad	.Lkexec_el1_sync_end - kexec_el1_sync
-- 
2.22.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v1 8/8] arm64, kexec: enable MMU during kexec relocation
From: Pavel Tatashin @ 2019-08-01 15:24 UTC (permalink / raw)
  To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
	corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
	james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190801152439.11363-1-pasha.tatashin@soleen.com>

Now, that we have transitional page tables configured, temporarily enable
MMU to allow faster relocation of segments to final destination.

The performance data: for a moderate size kernel + initramfs: 25M the
relocation was taking 0.382s, with enabled MMU it now takes
0.019s only or x20 improvement.

The time is proportional to the size of relocation, therefore if initramfs
is larger, 100M it could take over a second.

Also, remove reloc_arg->head, as it is not needed anymore once MMU is
enabled.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 arch/arm64/include/asm/kexec.h      |   2 -
 arch/arm64/kernel/asm-offsets.c     |   1 -
 arch/arm64/kernel/machine_kexec.c   |   1 -
 arch/arm64/kernel/relocate_kernel.S | 136 +++++++++++++++++-----------
 4 files changed, 84 insertions(+), 56 deletions(-)

diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 450d8440f597..ad81ed3e5751 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -109,7 +109,6 @@ extern const unsigned long kexec_el1_sync_size;
 			((UL(0xffffffffffffffff) - PAGE_OFFSET) >> 1) + 1)
 /*
  * kern_reloc_arg is passed to kernel relocation function as an argument.
- * head		kimage->head, allows to traverse through relocation segments.
  * entry_addr	kimage->start, where to jump from relocation function (new
  *		kernel, or purgatory entry address).
  * kern_arg0	first argument to kernel is its dtb address. The other
@@ -125,7 +124,6 @@ extern const unsigned long kexec_el1_sync_size;
  * copy_len	Number of bytes that need to be copied
  */
 struct kern_reloc_arg {
-	unsigned long	head;
 	unsigned long	entry_addr;
 	unsigned long	kern_arg0;
 	unsigned long	kern_arg1;
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 7c2ba09a8ceb..13ad00b1b90f 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -129,7 +129,6 @@ int main(void)
   DEFINE(SDEI_EVENT_PRIORITY,	offsetof(struct sdei_registered_event, priority));
 #endif
 #ifdef CONFIG_KEXEC_CORE
-  DEFINE(KRELOC_HEAD,		offsetof(struct kern_reloc_arg, head));
   DEFINE(KRELOC_ENTRY_ADDR,	offsetof(struct kern_reloc_arg, entry_addr));
   DEFINE(KRELOC_KERN_ARG0,	offsetof(struct kern_reloc_arg, kern_arg0));
   DEFINE(KRELOC_KERN_ARG1,	offsetof(struct kern_reloc_arg, kern_arg1));
diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c
index 16f761fc50c8..b5ff5fdb4777 100644
--- a/arch/arm64/kernel/machine_kexec.c
+++ b/arch/arm64/kernel/machine_kexec.c
@@ -199,7 +199,6 @@ int machine_kexec_post_load(struct kimage *kimage)
 	kimage->arch.kern_reloc = kern_reloc;
 	kimage->arch.kern_reloc_arg = __pa(kern_reloc_arg);
 
-	kern_reloc_arg->head = kimage->head;
 	kern_reloc_arg->entry_addr = kimage->start;
 	kern_reloc_arg->el2_vector = el2_vector;
 	kern_reloc_arg->kern_arg0 = kimage->arch.dtb_mem;
diff --git a/arch/arm64/kernel/relocate_kernel.S b/arch/arm64/kernel/relocate_kernel.S
index 14243a678277..96ff6760bd9c 100644
--- a/arch/arm64/kernel/relocate_kernel.S
+++ b/arch/arm64/kernel/relocate_kernel.S
@@ -4,6 +4,8 @@
  *
  * Copyright (C) Linaro.
  * Copyright (C) Huawei Futurewei Technologies.
+ * Copyright (c) 2019, Microsoft Corporation.
+ * Pavel Tatashin <patatash@linux.microsoft.com>
  */
 
 #include <linux/kexec.h>
@@ -14,6 +16,49 @@
 #include <asm/page.h>
 #include <asm/sysreg.h>
 
+/* Invalidae TLB */
+.macro tlb_invalidate
+	dsb	sy
+	dsb	ish
+	tlbi	vmalle1
+	dsb	ish
+	isb
+.endm
+
+/* Turn-off mmu at level specified by sctlr */
+.macro turn_off_mmu sctlr, tmp1, tmp2
+	mrs	\tmp1, \sctlr
+	ldr	\tmp2, =SCTLR_ELx_FLAGS
+	bic	\tmp1, \tmp1, \tmp2
+	pre_disable_mmu_workaround
+	msr	\sctlr, \tmp1
+	isb
+.endm
+
+/* Turn-on mmu at level specified by sctlr */
+.macro turn_on_mmu sctlr, tmp1, tmp2
+	mrs	\tmp1, \sctlr
+	ldr	\tmp2, =SCTLR_ELx_FLAGS
+	orr	\tmp1, \tmp1, \tmp2
+	msr	\sctlr, \tmp1
+	ic	iallu
+	dsb	nsh
+	isb
+.endm
+
+/*
+ * Set ttbr0 and ttbr1, called while MMU is disabled, so no need to temporarily
+ * set zero_page table. Invalidate TLB after new tables are set.
+ */
+.macro set_ttbr arg, tmp
+	ldr	\tmp, [\arg, #KRELOC_TRANS_TTBR0]
+	msr	ttbr0_el1, \tmp
+	ldr	\tmp, [\arg, #KRELOC_TRANS_TTBR1]
+	offset_ttbr1 \tmp
+	msr	ttbr1_el1, \tmp
+	isb
+.endm
+
 /*
  * arm64_relocate_new_kernel - Put a 2nd stage image in place and boot it.
  *
@@ -24,65 +69,52 @@
  * symbols arm64_relocate_new_kernel and arm64_relocate_new_kernel_end.  The
  * machine_kexec() routine will copy arm64_relocate_new_kernel to the kexec
  * safe memory that has been set up to be preserved during the copy operation.
+ *
+ * This function temporarily enables MMU if kernel relocation is needed.
+ * Also, if we enter this function at EL2 on non-VHE kernel, we temporarily go
+ * to EL1 to enable MMU, and escalate back to EL2 at the end to do the jump to
+ * the new kernel. This is determined by presence of el2_vector.
  */
 ENTRY(arm64_relocate_new_kernel)
-	/* Clear the sctlr_el2 flags. */
-	mrs	x2, CurrentEL
-	cmp	x2, #CurrentEL_EL2
+	mrs	x1, CurrentEL
+	cmp	x1, #CurrentEL_EL2
 	b.ne	1f
-	mrs	x2, sctlr_el2
-	ldr	x1, =SCTLR_ELx_FLAGS
-	bic	x2, x2, x1
-	pre_disable_mmu_workaround
-	msr	sctlr_el2, x2
-	isb
-1:	/* Check if the new image needs relocation. */
-	ldr	x16, [x0, #KRELOC_HEAD]		/* x16 = kimage_head */
-	tbnz	x16, IND_DONE_BIT, .Ldone
-	raw_dcache_line_size x15, x1		/* x15 = dcache line size */
-.Lloop:
-	and	x12, x16, PAGE_MASK		/* x12 = addr */
-	/* Test the entry flags. */
-.Ltest_source:
-	tbz	x16, IND_SOURCE_BIT, .Ltest_indirection
-
-	/* Invalidate dest page to PoC. */
-	mov     x2, x13
-	add     x20, x2, #PAGE_SIZE
-	sub     x1, x15, #1
-	bic     x2, x2, x1
-2:	dc      ivac, x2
-	add     x2, x2, x15
-	cmp     x2, x20
-	b.lo    2b
-	dsb     sy
-
-	copy_page x13, x12, x1, x2, x3, x4, x5, x6, x7, x8
-	b	.Lnext
-.Ltest_indirection:
-	tbz	x16, IND_INDIRECTION_BIT, .Ltest_destination
-	mov	x14, x12			/* ptr = addr */
-	b	.Lnext
-.Ltest_destination:
-	tbz	x16, IND_DESTINATION_BIT, .Lnext
-	mov	x13, x12			/* dest = addr */
-.Lnext:
-	ldr	x16, [x14], #8			/* entry = *ptr++ */
-	tbz	x16, IND_DONE_BIT, .Lloop	/* while (!(entry & DONE)) */
-.Ldone:
-	/* wait for writes from copy_page to finish */
-	dsb	nsh
-	ic	iallu
-	dsb	nsh
-	isb
-
-	/* Start new image. */
-	ldr	x4, [x0, #KRELOC_ENTRY_ADDR]	/* x4 = kimage_start */
+	turn_off_mmu sctlr_el2, x1, x2		/* Turn off MMU at EL2 */
+1:	mov	x20, xzr			/* x20 will hold vector value */
+	ldr	x11, [x0, #KRELOC_COPY_LEN]
+	cbz	x11, 5f				/* Check if need to relocate */
+	ldr	x20, [x0, #KRELOC_EL2_VECTOR]
+	cbz	x20, 2f				/* need to reduce to EL1? */
+	msr	vbar_el2, x20			/* el2_vector present, means */
+	adr	x1, 2f				/* we will do copy in el1 but */
+	msr	elr_el2, x1			/* do final jump from el2 */
+	eret					/* Reduce to EL1 */
+2:	set_ttbr x0, x1				/* Set our page tables */
+	tlb_invalidate
+	turn_on_mmu sctlr_el1, x1, x2		/* Turn MMU back on */
+	ldr	x1, [x0, #KRELOC_DST_ADDR];
+	ldr	x2, [x0, #KRELOC_SRC_ADDR];
+	mov	x12, x1				/* x12 dst backup */
+3:	copy_page x1, x2, x3, x4, x5, x6, x7, x8, x9, x10
+	sub	x11, x11, #PAGE_SIZE
+	cbnz	x11, 3b				/* page copy loop */
+	raw_dcache_line_size x2, x3		/* x2 = dcache line size */
+	sub	x3, x2, #1			/* x3 = dcache_size - 1 */
+	bic	x12, x12, x3
+4:	dc	cvau, x12			/* Flush D-cache */
+	add	x12, x12, x2
+	cmp	x12, x1				/* Compare to dst + len */
+	b.ne	4b				/* D-cache flush loop */
+	turn_off_mmu sctlr_el1, x1, x2		/* Turn off MMU */
+	tlb_invalidate				/* Invalidate TLB */
+5:	ldr	x4, [x0, #KRELOC_ENTRY_ADDR]	/* x4 = kimage_start */
 	ldr	x3, [x0, #KRELOC_KERN_ARG3]
 	ldr	x2, [x0, #KRELOC_KERN_ARG2]
 	ldr	x1, [x0, #KRELOC_KERN_ARG1]
 	ldr	x0, [x0, #KRELOC_KERN_ARG0]	/* x0 = dtb address */
-	br	x4
+	cbnz	x20, 6f				/* need to escalate to el2? */
+	br	x4				/* Jump to new world */
+6:	hvc	#0				/* enters kexec_el1_sync */
 .ltorg
 .Larm64_relocate_new_kernel_end:
 END(arm64_relocate_new_kernel)
-- 
2.22.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v2] ARM: kirkwood: ts219: disable the SoC's RTC
From: Martin Michlmayr @ 2019-08-01 15:30 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Andrew Lunn, Alexandre Belloni, Jason Cooper, Oliver Hartkopp,
	Gregory Clement, linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20190731223233.GA15900@taurus.defre.kleine-koenig.org>

* Uwe Kleine-König <uwe@kleine-koenig.org> [2019-08-01 00:32]:
> The internal RTC doesn't work, loading the driver only yields
> 
> 	rtc-mv f1010300.rtc: internal RTC not ticking
> 
> . So disable it.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>

Acked-by: Martin Michlmayr <tbm@cyrius.com>

This solves an issue I reported back in 2009, as Oliver Hartkopp
pointed out to my privately:
https://linux-arm-kernel.infradead.narkive.com/jySn4Wsw/patch-kirkwood-don-t-initialize-marvell-rtc-for-all-boards

(I contributed to the QNAP board support file in past but that was
replaced by the DT files.)

-- 
Martin Michlmayr
https://www.cyrius.com/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v19 00/15] arm64: untag user pointers passed to the kernel
From: Dave Hansen @ 2019-08-01 15:36 UTC (permalink / raw)
  To: Andrey Konovalov, Kevin Brodsky
  Cc: Mark Rutland, kvm, Szabolcs Nagy, Catalin Marinas, Will Deacon,
	dri-devel, Linux Memory Management List, Khalid Aziz,
	open list:KERNEL SELFTEST FRAMEWORK, Felix Kuehling,
	Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, linux-rdma,
	amd-gfx, Christoph Hellwig, Jason Gunthorpe, Dmitry Vyukov,
	Dave Martin, Evgeniy Stepanov, linux-media, Kees Cook,
	Ruben Ayrapetyan, Ramana Radhakrishnan, Alex Williamson,
	Mauro Carvalho Chehab, Linux ARM, Kostya Serebryany,
	Greg Kroah-Hartman, Yishai Hadas, LKML, Jens Wiklander, Lee Smith,
	Alexander Deucher, Andrew Morton, enh, Robin Murphy,
	Christian Koenig, Luc Van Oostenryck
In-Reply-To: <CAAeHK+zTFqsLiB3Wf0bAi5A8ukQX5ZuvfUg4td-=r5UhBsUBOQ@mail.gmail.com>

On 8/1/19 5:48 AM, Andrey Konovalov wrote:
> On Thu, Aug 1, 2019 at 2:11 PM Kevin Brodsky <kevin.brodsky@arm.com> wrote:
>> On 31/07/2019 17:50, Dave Hansen wrote:
>>> On 7/23/19 10:58 AM, Andrey Konovalov wrote:
>>>> The mmap and mremap (only new_addr) syscalls do not currently accept
>>>> tagged addresses. Architectures may interpret the tag as a background
>>>> colour for the corresponding vma.
>>> What the heck is a "background colour"? :)
>> Good point, this is some jargon that we started using for MTE, the idea being that
>> the kernel could set a tag value (specified during mmap()) as "background colour" for
>> anonymous pages allocated in that range.
>>
>> Anyway, this patch series is not about MTE. Andrey, for v20 (if any), I think it's
>> best to drop this last sentence to avoid any confusion.
> Sure, thanks!

OK, but what does that mean for tagged addresses getting passed to
mmap/mremap?  That sentence read to me like "architectures might allow
tags for ...something...".  So do we accept tagged addresses into those
syscalls?


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 0/2] arm64: entry: Move ct_user_exit calls earlier
From: James Morse @ 2019-08-01 15:41 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, James Morse, Masami Hiramatsu

Hi,

Masami's kprobes series adds a bunch of warnings to do_debug_exception().
When booted with nohz_full and lockdep, these things go off. This is
because the entry code is calling some of the C handlers before
ct_user_exit.

Patch 2 is for consistency, it doesn't look like anything minds
this today.


Thanks,

James Morse (2):
  arm64: entry: Move ct_user_exit before any user of RCU
  arm64: entry: Move ct_user_exit before we can take another exception

 arch/arm64/kernel/entry.S | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 1/2] arm64: entry: Move ct_user_exit before any user of RCU
From: James Morse @ 2019-08-01 15:41 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, James Morse, Masami Hiramatsu
In-Reply-To: <20190801154150.195959-1-james.morse@arm.com>

When taking an SError or Debug exception from EL0, we run the C
handler for these exceptions before updating the context tracking
code and unmasking lower priority interrupts.

When booting with nohz_full lockdep tells us we got this wrong:
| =============================
| WARNING: suspicious RCU usage
| 5.3.0-rc2-00010-gb4b5e9dcb11b-dirty #11271 Not tainted
| -----------------------------
| include/linux/rcupdate.h:643 rcu_read_unlock() used illegally wh!
|
| other info that might help us debug this:
|
|
| RCU used illegally from idle CPU!
| rcu_scheduler_active = 2, debug_locks = 1
| RCU used illegally from extended quiescent state!
| 1 lock held by a.out/432:
|  #0: 00000000c7a79515 (rcu_read_lock){....}, at: brk_handler+0x00
|
| stack backtrace:
| CPU: 1 PID: 432 Comm: a.out Not tainted 5.3.0-rc2-00010-gb4b5e9d1
| Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno De8
| Call trace:
|  dump_backtrace+0x0/0x140
|  show_stack+0x14/0x20
|  dump_stack+0xbc/0x104
|  lockdep_rcu_suspicious+0xf8/0x108
|  brk_handler+0x164/0x1b0
|  do_debug_exception+0x11c/0x278
|  el0_dbg+0x14/0x20

Move the ct_user_exit calls to be before do_debug_exception()
and do_serror() respectively.

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Fixes: 6c81fe7925cc4c42 ("arm64: enable context tracking")
Signed-off-by: James Morse <james.morse@arm.com>
---
The context to this is Masami Hiramatsu's patch adding sanity checks
for this:
Link: https://lore.kernel.org/r/156466954920.8995.14798868044005509434.stgit@devnote2
---
 arch/arm64/kernel/entry.S | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 320a30dbe35e..28681034d599 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -902,13 +902,14 @@ el0_dbg:
 	 * Debug exception handling
 	 */
 	tbnz	x24, #0, el0_inv		// EL0 only
+	mrs	x24, far_el1
 	gic_prio_kentry_setup tmp=x3
-	mrs	x0, far_el1
+	ct_user_exit
+	mov	x0, x24
 	mov	x1, x25
 	mov	x2, sp
 	bl	do_debug_exception
 	enable_da_f
-	ct_user_exit
 	b	ret_to_user
 el0_inv:
 	enable_daif
@@ -958,13 +959,14 @@ ENDPROC(el1_error)
 el0_error:
 	kernel_entry 0
 el0_error_naked:
-	mrs	x1, esr_el1
+	mrs	x25, esr_el1
 	gic_prio_kentry_setup tmp=x2
+	ct_user_exit
 	enable_dbg
 	mov	x0, sp
+	mov	x1, x25
 	bl	do_serror
 	enable_da_f
-	ct_user_exit
 	b	ret_to_user
 ENDPROC(el0_error)
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 2/2] arm64: entry: Move ct_user_exit before we can take another exception
From: James Morse @ 2019-08-01 15:41 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, James Morse, Masami Hiramatsu
In-Reply-To: <20190801154150.195959-1-james.morse@arm.com>

When taking an exception from EL0 we unmask exceptions before calling
ct_user_exit. This means we could take an interrupt or be single-stepped
in the gap. The entry from EL1 code sees that we took an exception from
the kernel, whereas the context_tracking code believes we are still in
user-space.

To keep these things consistent, move the ct_user_exit calls before
any unmask of exceptions.

Fixes: 6c81fe7925cc4c42 ("arm64: enable context tracking")
Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/arm64/kernel/entry.S | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 28681034d599..88f4ab21cb66 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -792,8 +792,8 @@ el0_cp15:
 	/*
 	 * Trapped CP15 (MRC, MCR, MRRC, MCRR) instructions
 	 */
-	enable_daif
 	ct_user_exit
+	enable_daif
 	mov	x0, x25
 	mov	x1, sp
 	bl	do_cp15instr
@@ -805,8 +805,8 @@ el0_da:
 	 * Data abort handling
 	 */
 	mrs	x26, far_el1
-	enable_daif
 	ct_user_exit
+	enable_daif
 	clear_address_tag x0, x26
 	mov	x1, x25
 	mov	x2, sp
@@ -818,11 +818,11 @@ el0_ia:
 	 */
 	mrs	x26, far_el1
 	gic_prio_kentry_setup tmp=x0
+	ct_user_exit
 	enable_da_f
 #ifdef CONFIG_TRACE_IRQFLAGS
 	bl	trace_hardirqs_off
 #endif
-	ct_user_exit
 	mov	x0, x26
 	mov	x1, x25
 	mov	x2, sp
@@ -832,8 +832,8 @@ el0_fpsimd_acc:
 	/*
 	 * Floating Point or Advanced SIMD access
 	 */
-	enable_daif
 	ct_user_exit
+	enable_daif
 	mov	x0, x25
 	mov	x1, sp
 	bl	do_fpsimd_acc
@@ -842,8 +842,8 @@ el0_sve_acc:
 	/*
 	 * Scalable Vector Extension access
 	 */
-	enable_daif
 	ct_user_exit
+	enable_daif
 	mov	x0, x25
 	mov	x1, sp
 	bl	do_sve_acc
@@ -852,8 +852,8 @@ el0_fpsimd_exc:
 	/*
 	 * Floating Point, Advanced SIMD or SVE exception
 	 */
-	enable_daif
 	ct_user_exit
+	enable_daif
 	mov	x0, x25
 	mov	x1, sp
 	bl	do_fpsimd_exc
@@ -868,11 +868,11 @@ el0_sp_pc:
 	 * Stack or PC alignment exception handling
 	 */
 	gic_prio_kentry_setup tmp=x0
+	ct_user_exit
 	enable_da_f
 #ifdef CONFIG_TRACE_IRQFLAGS
 	bl	trace_hardirqs_off
 #endif
-	ct_user_exit
 	mov	x0, x26
 	mov	x1, x25
 	mov	x2, sp
@@ -882,8 +882,8 @@ el0_undef:
 	/*
 	 * Undefined instruction
 	 */
-	enable_daif
 	ct_user_exit
+	enable_daif
 	mov	x0, sp
 	bl	do_undefinstr
 	b	ret_to_user
@@ -891,8 +891,8 @@ el0_sys:
 	/*
 	 * System instructions, for trapped cache maintenance instructions
 	 */
-	enable_daif
 	ct_user_exit
+	enable_daif
 	mov	x0, x25
 	mov	x1, sp
 	bl	do_sysinstr
@@ -912,8 +912,8 @@ el0_dbg:
 	enable_da_f
 	b	ret_to_user
 el0_inv:
-	enable_daif
 	ct_user_exit
+	enable_daif
 	mov	x0, sp
 	mov	x1, #BAD_SYNC
 	mov	x2, x25
@@ -926,13 +926,13 @@ el0_irq:
 	kernel_entry 0
 el0_irq_naked:
 	gic_prio_irq_setup pmr=x20, tmp=x0
+	ct_user_exit
 	enable_da_f
 
 #ifdef CONFIG_TRACE_IRQFLAGS
 	bl	trace_hardirqs_off
 #endif
 
-	ct_user_exit
 #ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
 	tbz	x22, #55, 1f
 	bl	do_el0_irq_bp_hardening
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH 5/8] arm64: use ZONE_DMA on DMA addressing limited devices
From: Nicolas Saenz Julienne @ 2019-08-01 15:44 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: phill, devicetree, f.fainelli, linux-mm, marc.zyngier,
	Will Deacon, linux-kernel, eric, iommu, robh+dt, linux-rpi-kernel,
	mbrugger, Robin Murphy, akpm, m.szyprowski, frowand.list, hch,
	linux-arm-kernel, wahrenst
In-Reply-To: <20190731170742.GC17773@arrakis.emea.arm.com>


[-- Attachment #1.1: Type: text/plain, Size: 1734 bytes --]

On Wed, 2019-07-31 at 18:07 +0100, Catalin Marinas wrote:
> On Wed, Jul 31, 2019 at 05:47:48PM +0200, Nicolas Saenz Julienne wrote:
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index 1c4ffabbe1cb..f5279ef85756 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -50,6 +50,13 @@
> >  s64 memstart_addr __ro_after_init = -1;
> >  EXPORT_SYMBOL(memstart_addr);
> >  
> > +/*
> > + * We might create both a ZONE_DMA and ZONE_DMA32. ZONE_DMA is needed if
> > there
> > + * are periferals unable to address the first naturally aligned 4GB of ram.
> > + * ZONE_DMA32 will be expanded to cover the rest of that memory. If such
> > + * limitations doesn't exist only ZONE_DMA32 is created.
> > + */
> 
> Shouldn't we instead only create ZONE_DMA to cover the whole 32-bit
> range and leave ZONE_DMA32 empty? Can__GFP_DMA allocations fall back
> onto ZONE_DMA32?

Hi Catalin, thanks for the review.

You're right, the GFP_DMA page allocation will fail with a nasty dmesg error if
ZONE_DMA is configured but empty. Unsurprisingly the opposite situation is fine
(GFP_DMA32 with an empty ZONE_DMA32).

I switched to the scheme you're suggesting for the next version of the series.
The comment will be something the likes of this:

/*
 * We create both a ZONE_DMA and ZONE_DMA32. ZONE_DMA's size is decided based
 * on whether the SoC's peripherals are able to address the first naturally
 * aligned 4 GB of ram.
 *
 * If limited, ZONE_DMA covers that area and ZONE_DMA32 the rest of that 32 bit
 * addressable memory.
 *
 * If not ZONE_DMA is expanded to cover the whole 32 bit addressable memory and
 * ZONE_DMA32 is left empty.
 */

 Regards,
 Nicolas



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 2/3] media: imx7-media-csi: add i.MX6UL support
From: Rui Miguel Silva @ 2019-08-01 15:50 UTC (permalink / raw)
  To: Sébastien Szymanski
  Cc: Mark Rutland, devicetree, devel, Pengutronix Kernel Team,
	Greg Kroah-Hartman, Fabio Estevam, Sascha Hauer, Rob Herring,
	NXP Linux Team, Philipp Zabel, Steve Longerbeam,
	Mauro Carvalho Chehab, Shawn Guo, linux-arm-kernel, linux-media
In-Reply-To: <20190731163330.32532-1-sebastien.szymanski@armadeus.com>

Hi Sebastien,
Thanks for the patch.

On Wed 31 Jul 2019 at 17:33, Sébastien Szymanski wrote:
> i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
> to imx7-media-csi driver.
>
> Signed-off-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>
>

LGTM. Thanks for adding the support to this imx6ul/l soc.

Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com>

---
Cheers,
	Rui

> ---
>
> Changes for v3:
> - rebase on Linux v5.3-rc2
> - remove csi_soc_id var as it's not needed anymore thanks to commit
>   e0c76a7d3428 ("media: imx7-media-csi: get csi upstream endpoint")
>
> Changes for v2:
>  - rebase on top of linuxtv/master
>  - mention i.MX6UL/L in header and Kconfig help text
>  - rename csi_type to csi_soc_id
>
>  drivers/staging/media/imx/Kconfig          |  4 +--
>  drivers/staging/media/imx/imx7-media-csi.c | 30 +++++++++++++++-------
>  2 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/media/imx/Kconfig b/drivers/staging/media/imx/Kconfig
> index 4c726345dc25..f51476243016 100644
> --- a/drivers/staging/media/imx/Kconfig
> +++ b/drivers/staging/media/imx/Kconfig
> @@ -22,11 +22,11 @@ config VIDEO_IMX_CSI
>  	  A video4linux camera sensor interface driver for i.MX5/6.
>
>  config VIDEO_IMX7_CSI
> -	tristate "i.MX7 Camera Sensor Interface driver"
> +	tristate "i.MX6UL/L / i.MX7 Camera Sensor Interface driver"
>  	depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
>  	default y
>  	help
>  	  Enable support for video4linux camera sensor interface driver for
> -	  i.MX7.
> +	  i.MX6UL/L or i.MX7.
>  endmenu
>  endif
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index 500b4c08d967..4ca79ff4c9b3 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC
> + * V4L2 Capture CSI Subdev for Freescale i.MX6UL/L / i.MX7 SOC
>   *
>   * Copyright (c) 2019 Linaro Ltd
>   *
> @@ -765,6 +765,7 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>  	struct v4l2_pix_format *out_pix = &vdev->fmt.fmt.pix;
>  	__u32 in_code = csi->format_mbus[IMX7_CSI_PAD_SINK].code;
>  	u32 cr1, cr18;
> +	int width = out_pix->width;
>
>  	if (out_pix->field == V4L2_FIELD_INTERLACED) {
>  		imx7_csi_deinterlace_enable(csi, true);
> @@ -774,15 +775,27 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>  		imx7_csi_buf_stride_set(csi, 0);
>  	}
>
> -	imx7_csi_set_imagpara(csi, out_pix->width, out_pix->height);
> +	cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
> +
> +	if (!csi->is_csi2) {
> +		if (out_pix->pixelformat == V4L2_PIX_FMT_UYVY ||
> +		    out_pix->pixelformat == V4L2_PIX_FMT_YUYV)
> +			width *= 2;
> +
> +		imx7_csi_set_imagpara(csi, width, out_pix->height);
> +
> +		cr18 |= (BIT_BASEADDR_SWITCH_EN | BIT_BASEADDR_SWITCH_SEL |
> +			BIT_BASEADDR_CHG_ERR_EN);
> +		imx7_csi_reg_write(csi, cr18, CSI_CSICR18);
>
> -	if (!csi->is_csi2)
>  		return 0;
> +	}
> +
> +	imx7_csi_set_imagpara(csi, width, out_pix->height);
>
>  	cr1 = imx7_csi_reg_read(csi, CSI_CSICR1);
>  	cr1 &= ~BIT_GCLK_MODE;
>
> -	cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
>  	cr18 &= BIT_MIPI_DATA_FORMAT_MASK;
>  	cr18 |= BIT_DATA_FROM_MIPI;
>
> @@ -817,11 +830,9 @@ static void imx7_csi_enable(struct imx7_csi *csi)
>  {
>  	imx7_csi_sw_reset(csi);
>
> -	if (csi->is_csi2) {
> -		imx7_csi_dmareq_rff_enable(csi);
> -		imx7_csi_hw_enable_irq(csi);
> -		imx7_csi_hw_enable(csi);
> -	}
> +	imx7_csi_dmareq_rff_enable(csi);
> +	imx7_csi_hw_enable_irq(csi);
> +	imx7_csi_hw_enable(csi);
>  }
>
>  static void imx7_csi_disable(struct imx7_csi *csi)
> @@ -1302,6 +1313,7 @@ static int imx7_csi_remove(struct platform_device *pdev)
>
>  static const struct of_device_id imx7_csi_of_match[] = {
>  	{ .compatible = "fsl,imx7-csi" },
> +	{ .compatible = "fsl,imx6ul-csi" },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, imx7_csi_of_match);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 2/6] irqchip/irq-pruss-intc: Add a PRUSS irqchip driver for PRUSS interrupts
From: Suman Anna @ 2019-08-01 15:51 UTC (permalink / raw)
  To: Marc Zyngier, Thomas Gleixner, Jason Cooper
  Cc: devicetree, Grygorii Strashko, David Lechner, Tony Lindgren,
	Sekhar Nori, linux-kernel, Andrew F. Davis, Lokesh Vutla,
	Rob Herring, Murali Karicheri, linux-omap, linux-arm-kernel,
	Roger Quadros
In-Reply-To: <d0d539c2-4247-d74e-3bb2-ddb98e0bafb5@kernel.org>

Hi Marc,

On 8/1/19 4:42 AM, Marc Zyngier wrote:
> On 31/07/2019 23:41, Suman Anna wrote:
>> From: "Andrew F. Davis" <afd@ti.com>
>>
>> The Programmable Real-Time Unit Subsystem (PRUSS) contains a local
>> interrupt controller (INTC) that can handle various system input events
>> and post interrupts back to the device-level initiators. The INTC can
>> support upto 64 input events with individual control configuration and
>> hardware prioritization. These events are mapped onto 10 output interrupt
>> lines through two levels of many-to-one mapping support. Different
>> interrupt lines are routed to the individual PRU cores or to the host
>> CPU, or to other devices on the SoC. Some of these events are sourced
>> from peripherals or other sub-modules within that PRUSS, while a few
>> others are sourced from SoC-level peripherals/devices.
>>
>> The PRUSS INTC platform driver manages this PRUSS interrupt controller
>> and implements an irqchip driver to provide a Linux standard way for
>> the PRU client users to enable/disable/ack/re-trigger a PRUSS system
>> event. The system events to interrupt channels and output interrupts
>> relies on the mapping configuration provided either through the PRU
>> firmware blob or via the PRU application's device tree node. The
>> mappings will be programmed during the boot/shutdown of a PRU core.
>>
>> The PRUSS INTC module is reference counted during the interrupt
>> setup phase through the irqchip's irq_request_resources() and
>> irq_release_resources() ops. This restricts the module from being
>> removed as long as there are active interrupt users.
>>
>> The driver currently supports and can be built for OMAP architecture
>> based AM335x, AM437x and AM57xx SoCs; Keystone2 architecture based
>> 66AK2G SoCs and Davinci architecture based OMAP-L13x/AM18x/DA850 SoCs.
>> All of these SoCs support 64 system events, 10 interrupt channels and
>> 10 output interrupt lines per PRUSS INTC with a few SoC integration
>> differences.
>>
>> NOTE:
>> Each PRU-ICSS's INTC on AM57xx SoCs is preceded by a Crossbar that
>> enables multiple external events to be routed to a specific number
>> of input interrupt events. Any non-default external interrupt event
>> directed towards PRUSS needs this crossbar to be setup properly.
>>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> v2: 
>>  - Addressed all of David Lechner's comments
>>  - Dropped irq_retrigger callback
>>  - Updated interrupt names from "hostX" to "host_intrX"
>>  - Moved host_mask variable to patch 4
>> v1: https://patchwork.kernel.org/patch/11034545/
>> v0: https://patchwork.kernel.org/patch/10795761/
>>
>>  drivers/irqchip/Kconfig          |  10 +
>>  drivers/irqchip/Makefile         |   1 +
>>  drivers/irqchip/irq-pruss-intc.c | 338 +++++++++++++++++++++++++++++++
>>  3 files changed, 349 insertions(+)
>>  create mode 100644 drivers/irqchip/irq-pruss-intc.c
>>
>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>> index 80e10f4e213a..dc6b5aa77a5d 100644
>> --- a/drivers/irqchip/Kconfig
>> +++ b/drivers/irqchip/Kconfig
>> @@ -471,6 +471,16 @@ config TI_SCI_INTA_IRQCHIP
>>  	  If you wish to use interrupt aggregator irq resources managed by the
>>  	  TI System Controller, say Y here. Otherwise, say N.
>>  
>> +config TI_PRUSS_INTC
>> +	tristate "TI PRU-ICSS Interrupt Controller"
>> +	depends on ARCH_DAVINCI || SOC_AM33XX || SOC_AM437X || SOC_DRA7XX || ARCH_KEYSTONE
>> +	select IRQ_DOMAIN
>> +	help
>> +	   This enables support for the PRU-ICSS Local Interrupt Controller
>> +	   present within a PRU-ICSS subsystem present on various TI SoCs.
>> +	   The PRUSS INTC enables various interrupts to be routed to multiple
>> +	   different processors within the SoC.
>> +
>>  endmenu
>>  
>>  config SIFIVE_PLIC
>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> index 8d0fcec6ab23..a02e652ca805 100644
>> --- a/drivers/irqchip/Makefile
>> +++ b/drivers/irqchip/Makefile
>> @@ -102,3 +102,4 @@ obj-$(CONFIG_MADERA_IRQ)		+= irq-madera.o
>>  obj-$(CONFIG_LS1X_IRQ)			+= irq-ls1x.o
>>  obj-$(CONFIG_TI_SCI_INTR_IRQCHIP)	+= irq-ti-sci-intr.o
>>  obj-$(CONFIG_TI_SCI_INTA_IRQCHIP)	+= irq-ti-sci-inta.o
>> +obj-$(CONFIG_TI_PRUSS_INTC)		+= irq-pruss-intc.o
>> diff --git a/drivers/irqchip/irq-pruss-intc.c b/drivers/irqchip/irq-pruss-intc.c
>> new file mode 100644
>> index 000000000000..4a9456544fd0
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-pruss-intc.c
>> @@ -0,0 +1,338 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * PRU-ICSS INTC IRQChip driver for various TI SoCs
>> + *
>> + * Copyright (C) 2016-2019 Texas Instruments Incorporated - http://www.ti.com/
>> + *	Andrew F. Davis <afd@ti.com>
>> + *	Suman Anna <s-anna@ti.com>
>> + */
>> +
>> +#include <linux/irq.h>
>> +#include <linux/irqchip/chained_irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +
>> +/*
>> + * Number of host interrupts reaching the main MPU sub-system. Note that this
>> + * is not the same as the total number of host interrupts supported by the PRUSS
>> + * INTC instance
>> + */
>> +#define MAX_NUM_HOST_IRQS	8
>> +
>> +/* minimum starting host interrupt number for MPU */
>> +#define MIN_PRU_HOST_INT	2
>> +
>> +/* maximum number of system events */
>> +#define MAX_PRU_SYS_EVENTS	64
>> +
>> +/* PRU_ICSS_INTC registers */
>> +#define PRU_INTC_REVID		0x0000
>> +#define PRU_INTC_CR		0x0004
>> +#define PRU_INTC_GER		0x0010
>> +#define PRU_INTC_GNLR		0x001c
>> +#define PRU_INTC_SISR		0x0020
>> +#define PRU_INTC_SICR		0x0024
>> +#define PRU_INTC_EISR		0x0028
>> +#define PRU_INTC_EICR		0x002c
>> +#define PRU_INTC_HIEISR		0x0034
>> +#define PRU_INTC_HIDISR		0x0038
>> +#define PRU_INTC_GPIR		0x0080
>> +#define PRU_INTC_SRSR0		0x0200
>> +#define PRU_INTC_SRSR1		0x0204
>> +#define PRU_INTC_SECR0		0x0280
>> +#define PRU_INTC_SECR1		0x0284
>> +#define PRU_INTC_ESR0		0x0300
>> +#define PRU_INTC_ESR1		0x0304
>> +#define PRU_INTC_ECR0		0x0380
>> +#define PRU_INTC_ECR1		0x0384
>> +#define PRU_INTC_CMR(x)		(0x0400 + (x) * 4)
>> +#define PRU_INTC_HMR(x)		(0x0800 + (x) * 4)
>> +#define PRU_INTC_HIPIR(x)	(0x0900 + (x) * 4)
>> +#define PRU_INTC_SIPR0		0x0d00
>> +#define PRU_INTC_SIPR1		0x0d04
>> +#define PRU_INTC_SITR0		0x0d80
>> +#define PRU_INTC_SITR1		0x0d84
>> +#define PRU_INTC_HINLR(x)	(0x1100 + (x) * 4)
>> +#define PRU_INTC_HIER		0x1500
>> +
>> +/* HIPIR register bit-fields */
>> +#define INTC_HIPIR_NONE_HINT	0x80000000
>> +
>> +/**
>> + * struct pruss_intc - PRUSS interrupt controller structure
>> + * @irqs: kernel irq numbers corresponding to PRUSS host interrupts
>> + * @base: base virtual address of INTC register space
>> + * @irqchip: irq chip for this interrupt controller
>> + * @domain: irq domain for this interrupt controller
>> + * @lock: mutex to serialize access to INTC
>> + */
>> +struct pruss_intc {
>> +	unsigned int irqs[MAX_NUM_HOST_IRQS];
>> +	void __iomem *base;
>> +	struct irq_chip *irqchip;
>> +	struct irq_domain *domain;
>> +	struct mutex lock; /* PRUSS INTC lock */
> 
> Nothing seem to use that lock in this patch (other than to initialize it).

Correct, will move this to patch 4 where the actual usage is similar to
the host_mask move done in this version.

> 
>> +};
>> +
>> +static inline u32 pruss_intc_read_reg(struct pruss_intc *intc, unsigned int reg)
>> +{
>> +	return readl_relaxed(intc->base + reg);
>> +}
>> +
>> +static inline void pruss_intc_write_reg(struct pruss_intc *intc,
>> +					unsigned int reg, u32 val)
>> +{
>> +	writel_relaxed(val, intc->base + reg);
>> +}
>> +
>> +static int pruss_intc_check_write(struct pruss_intc *intc, unsigned int reg,
>> +				  unsigned int sysevent)
>> +{
>> +	if (!intc)
>> +		return -EINVAL;
>> +
>> +	if (sysevent >= MAX_PRU_SYS_EVENTS)
>> +		return -EINVAL;
> 
> How can any of these happen?  That'd be a bug in the driver surely.
> Also, nothing ever checks the return value.

Yeah, I think I can drop this. The ack/mask/unmask callbacks are all
void returning functions, but we do return the value in
irq_set_irqchip_state() callback added in patch 5 (will reorder that
patch for the next version). This is similar to the check in
gic_irq_set_irqchip_state().

> 
>> +
>> +	pruss_intc_write_reg(intc, reg, sysevent);
>> +
>> +	return 0;
>> +}
>> +
>> +static void pruss_intc_init(struct pruss_intc *intc)
>> +{
>> +	int i;
>> +
>> +	/* configure polarity to active high for all system interrupts */
>> +	pruss_intc_write_reg(intc, PRU_INTC_SIPR0, 0xffffffff);
>> +	pruss_intc_write_reg(intc, PRU_INTC_SIPR1, 0xffffffff);
>> +
>> +	/* configure type to pulse interrupt for all system interrupts */
>> +	pruss_intc_write_reg(intc, PRU_INTC_SITR0, 0);
>> +	pruss_intc_write_reg(intc, PRU_INTC_SITR1, 0);
>> +
>> +	/* clear all 16 interrupt channel map registers */
>> +	for (i = 0; i < 16; i++)
>> +		pruss_intc_write_reg(intc, PRU_INTC_CMR(i), 0);
>> +
>> +	/* clear all 3 host interrupt map registers */
>> +	for (i = 0; i < 3; i++)
>> +		pruss_intc_write_reg(intc, PRU_INTC_HMR(i), 0);
>> +}
>> +
>> +static void pruss_intc_irq_ack(struct irq_data *data)
>> +{
>> +	struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
>> +	unsigned int hwirq = data->hwirq;
>> +
>> +	pruss_intc_check_write(intc, PRU_INTC_SICR, hwirq);
>> +}
>> +
>> +static void pruss_intc_irq_mask(struct irq_data *data)
>> +{
>> +	struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
>> +	unsigned int hwirq = data->hwirq;
>> +
>> +	pruss_intc_check_write(intc, PRU_INTC_EICR, hwirq);
>> +}
>> +
>> +static void pruss_intc_irq_unmask(struct irq_data *data)
>> +{
>> +	struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
>> +	unsigned int hwirq = data->hwirq;
>> +
>> +	pruss_intc_check_write(intc, PRU_INTC_EISR, hwirq);
>> +}
>> +
>> +static int pruss_intc_irq_reqres(struct irq_data *data)
>> +{
>> +	if (!try_module_get(THIS_MODULE))
>> +		return -ENODEV;
>> +
>> +	return 0;
>> +}
>> +
>> +static void pruss_intc_irq_relres(struct irq_data *data)
>> +{
>> +	module_put(THIS_MODULE);
>> +}
>> +
>> +static int pruss_intc_irq_domain_map(struct irq_domain *d, unsigned int virq,
>> +				     irq_hw_number_t hw)
>> +{
>> +	struct pruss_intc *intc = d->host_data;
>> +
>> +	irq_set_chip_data(virq, intc);
>> +	irq_set_chip_and_handler(virq, intc->irqchip, handle_level_irq);
>> +
>> +	return 0;
>> +}
>> +
>> +static void pruss_intc_irq_domain_unmap(struct irq_domain *d, unsigned int virq)
>> +{
>> +	irq_set_chip_and_handler(virq, NULL, NULL);
>> +	irq_set_chip_data(virq, NULL);
>> +}
>> +
>> +static const struct irq_domain_ops pruss_intc_irq_domain_ops = {
>> +	.xlate	= irq_domain_xlate_onecell,
>> +	.map	= pruss_intc_irq_domain_map,
>> +	.unmap	= pruss_intc_irq_domain_unmap,
>> +};
>> +
>> +static void pruss_intc_irq_handler(struct irq_desc *desc)
>> +{
>> +	unsigned int irq = irq_desc_get_irq(desc);
>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>> +	struct pruss_intc *intc = irq_get_handler_data(irq);
>> +	u32 hipir;
>> +	unsigned int virq;
>> +	int i, hwirq;
>> +
>> +	chained_irq_enter(chip, desc);
>> +
>> +	/* find our host irq number */
>> +	for (i = 0; i < MAX_NUM_HOST_IRQS; i++)
>> +		if (intc->irqs[i] == irq)
>> +			break;
>> +	if (i == MAX_NUM_HOST_IRQS)
>> +		goto err;
>> +
>> +	i += MIN_PRU_HOST_INT;
>> +
>> +	/* get highest priority pending PRUSS system event */
>> +	hipir = pruss_intc_read_reg(intc, PRU_INTC_HIPIR(i));
>> +	while (!(hipir & INTC_HIPIR_NONE_HINT)) {
>> +		hwirq = hipir & GENMASK(9, 0);
>> +		virq = irq_linear_revmap(intc->domain, hwirq);
>> +
>> +		/*
>> +		 * NOTE: manually ACK any system events that do not have a
>> +		 * handler mapped yet
>> +		 */
>> +		if (unlikely(!virq))
>> +			pruss_intc_check_write(intc, PRU_INTC_SICR, hwirq);
> 
> How were they configured the first place?

Ideally, this should not occur, I probably should add a WARN_ON here
catching any misuse. The PRUSS INTC is touched by multiple processors,
and each of them have to use some of the same registers to ack the
internal event. The current design is limited to only acking and
triggering the interrupts from PRU firmwares while the mappings are all
done by Linux. We are forced to do this to save some instruction space
in the tiny Instruction RAM that the PRUs (smallest is 4K and largest is
12K) have.

> 
>> +		else
>> +			generic_handle_irq(virq);
>> +
>> +		/* get next system event */
>> +		hipir = pruss_intc_read_reg(intc, PRU_INTC_HIPIR(i));
>> +	}
>> +err:
>> +	chained_irq_exit(chip, desc);
>> +}
>> +
>> +static int pruss_intc_probe(struct platform_device *pdev)
>> +{
>> +	static const char * const irq_names[] = {
> 
> Should this be sized with MAX_NUM_HOST_IRQS, given that this is how you
> parse it?

Yes, will update in the next version.

> 
>> +		"host_intr0", "host_intr1", "host_intr2", "host_intr3",
>> +		"host_intr4", "host_intr5", "host_intr6", "host_intr7", };
>> +	struct device *dev = &pdev->dev;
>> +	struct pruss_intc *intc;
>> +	struct resource *res;
>> +	struct irq_chip *irqchip;
>> +	int i, irq;
>> +
>> +	intc = devm_kzalloc(dev, sizeof(*intc), GFP_KERNEL);
>> +	if (!intc)
>> +		return -ENOMEM;
>> +	platform_set_drvdata(pdev, intc);
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	intc->base = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(intc->base)) {
>> +		dev_err(dev, "failed to parse and map intc memory resource\n");
>> +		return PTR_ERR(intc->base);
>> +	}
>> +
>> +	dev_dbg(dev, "intc memory: pa %pa size 0x%zx va %pK\n", &res->start,
>> +		(size_t)resource_size(res), intc->base);
>> +
>> +	mutex_init(&intc->lock);
>> +
>> +	pruss_intc_init(intc);
>> +
>> +	irqchip = devm_kzalloc(dev, sizeof(*irqchip), GFP_KERNEL);
>> +	if (!irqchip)
>> +		return -ENOMEM;
>> +
>> +	irqchip->irq_ack = pruss_intc_irq_ack;
>> +	irqchip->irq_mask = pruss_intc_irq_mask;
>> +	irqchip->irq_unmask = pruss_intc_irq_unmask;
>> +	irqchip->irq_request_resources = pruss_intc_irq_reqres;
>> +	irqchip->irq_release_resources = pruss_intc_irq_relres;
>> +	irqchip->name = dev_name(dev);
>> +	intc->irqchip = irqchip;
> 
> Given that each and every pruss_intc ends up with a pointer to its own
> irqchip, why is it a separate allocation instead of directly embedding
> the structure?
> 
> Alternatively, have a single 'static const struct irq_chip' and lose the
> slightly pointless dev_name as the irqchip name.

Agreed, this can be optimized.

> 
>> +
>> +	/* always 64 events */
>> +	intc->domain = irq_domain_add_linear(dev->of_node, MAX_PRU_SYS_EVENTS,
>> +					     &pruss_intc_irq_domain_ops, intc);
>> +	if (!intc->domain)
>> +		return -ENOMEM;
>> +
>> +	for (i = 0; i < MAX_NUM_HOST_IRQS; i++) {
>> +		irq = platform_get_irq_byname(pdev, irq_names[i]);
>> +		if (irq < 0) {
> 
> irq == 0 is also an error.

OK, will fix.

> 
>> +			dev_err(dev, "platform_get_irq_byname failed for %s : %d\n",
>> +				irq_names[i], irq);
>> +			goto fail_irq;
>> +		}
>> +
>> +		intc->irqs[i] = irq;
>> +		irq_set_handler_data(irq, intc);
>> +		irq_set_chained_handler(irq, pruss_intc_irq_handler);
>> +	}
>> +
>> +	return 0;
>> +
>> +fail_irq:
>> +	while (--i >= 0) {
>> +		if (intc->irqs[i])
> 
> This 'if' seems supperfluous.

So, some interrupts among the MAX_NUM_HOST_IRQS are not connected to the
Arm processor on some SoCs. I will move this check to the next patch
which introduces the skipping of interrupts.

> 
>> +			irq_set_chained_handler_and_data(intc->irqs[i], NULL,
>> +							 NULL);
>> +	}
>> +	irq_domain_remove(intc->domain);
>> +	return irq;
>> +}
>> +
>> +static int pruss_intc_remove(struct platform_device *pdev)
>> +{
>> +	struct pruss_intc *intc = platform_get_drvdata(pdev);
>> +	unsigned int hwirq;
>> +	int i;
>> +
>> +	for (i = 0; i < MAX_NUM_HOST_IRQS; i++) {
>> +		if (intc->irqs[i])
> 
> Same here.

Will move this as well.

> 
>> +			irq_set_chained_handler_and_data(intc->irqs[i], NULL,
>> +							 NULL);
>> +	}
>> +
>> +	for (hwirq = 0; hwirq < MAX_PRU_SYS_EVENTS; hwirq++)
>> +		irq_dispose_mapping(irq_find_mapping(intc->domain, hwirq));
>> +	irq_domain_remove(intc->domain);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id pruss_intc_of_match[] = {
>> +	{ .compatible = "ti,pruss-intc", },
>> +	{ /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, pruss_intc_of_match);
>> +
>> +static struct platform_driver pruss_intc_driver = {
>> +	.driver = {
>> +		.name = "pruss-intc",
>> +		.of_match_table = pruss_intc_of_match,
>> +	},
>> +	.probe  = pruss_intc_probe,
>> +	.remove = pruss_intc_remove,
>> +};
>> +module_platform_driver(pruss_intc_driver);
>> +
>> +MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
>> +MODULE_AUTHOR("Suman Anna <s-anna@ti.com>");
>> +MODULE_DESCRIPTION("TI PRU-ICSS INTC Driver");
>> +MODULE_LICENSE("GPL v2");
>>
> 
> Thanks,
> 	

Thank you for all the review comments.

regards
Suman


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 1/2] ACPI/PPTT: Add support for ACPI 6.3 thread flag
From: Sudeep Holla @ 2019-08-01 15:57 UTC (permalink / raw)
  To: Jeremy Linton
  Cc: lorenzo.pieralisi, catalin.marinas, rjw, linux-acpi, Sudeep Holla,
	will, linux-arm-kernel, lenb
In-Reply-To: <20190801034634.26913-2-jeremy.linton@arm.com>


Hi Jeremy,

On Wed, Jul 31, 2019 at 10:46:33PM -0500, Jeremy Linton wrote:
> ACPI 6.3 adds a flag to the CPU node to indicate whether
> the given PE is a thread. Add a function to return that
> information for a given linux logical CPU.
>

Apart from few minor nits,

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  drivers/acpi/pptt.c  | 54 +++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/acpi.h |  5 ++++
>  2 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
> index 1e7ac0bd0d3a..84718f6cb741 100644
> --- a/drivers/acpi/pptt.c
> +++ b/drivers/acpi/pptt.c
> @@ -540,6 +540,44 @@ static int find_acpi_cpu_topology_tag(unsigned int cpu, int level, int flag)
>  	return retval;
>  }
>
> +/**
> + * check_acpi_cpu_flag() - Determine if CPU node has a flag set
> + * @cpu: Kernel logical CPU number
> + * @rev: The PPTT revision defining the flag

[nit] I would rather put it as minimum PPTT revision that supports the
flag. It aligns with the code too as we are not looking for exact match.

> + * @flag: The flag itself
> + *
> + * Check the node representing a CPU for a given flag.
> + *
> + * Return: -ENOENT if the PPTT doesn't exist, the CPU cannot be found or
> + *	   the table revision isn't new enough.
> + *	   1, any passed flag set
> + *	   0, flag unset
> + */
> +static int check_acpi_cpu_flag(unsigned int cpu, int rev, u32 flag)
> +{
> +	struct acpi_table_header *table;
> +	acpi_status status;
> +	u32 acpi_cpu_id = get_acpi_id_for_cpu(cpu);
> +	struct acpi_pptt_processor *cpu_node = NULL;
> +	int ret = -ENOENT;
> +
> +	status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
> +	if (ACPI_FAILURE(status)) {
> +		acpi_pptt_warn_missing();
> +		return ret;
> +	}
> +
> +	if (table->revision >= rev)
> +		cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
> +
> +	if (cpu_node)
> +		ret = (cpu_node->flags & flag) != 0;
> +
> +	acpi_put_table(table);
> +
> +	return ret;
> +}
> +
>  /**
>   * acpi_find_last_cache_level() - Determines the number of cache levels for a PE
>   * @cpu: Kernel logical CPU number
> @@ -604,6 +642,21 @@ int cache_setup_acpi(unsigned int cpu)
>  	return status;
>  }
>
> +/**
> + * acpi_pptt_cpu_is_thread() - Determine if CPU is a thread
> + * @cpu: Kernel logical CPU number
> + *

[nit] If you spin the patch again, you can drop extra line space here.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 2/2] arm64: topology: Use PPTT to determine if PE is a thread
From: Sudeep Holla @ 2019-08-01 15:58 UTC (permalink / raw)
  To: Jeremy Linton
  Cc: lorenzo.pieralisi, catalin.marinas, rjw, linux-acpi, Sudeep Holla,
	will, linux-arm-kernel, lenb
In-Reply-To: <20190801034634.26913-3-jeremy.linton@arm.com>

On Wed, Jul 31, 2019 at 10:46:34PM -0500, Jeremy Linton wrote:
> ACPI 6.3 adds a thread flag to represent if a CPU/PE is
> actually a thread. Given that the MPIDR_MT bit may not
> represent this information consistently on homogeneous machines
> we should prefer the PPTT flag if its available.
>

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2] arm64/mm: fix variable 'tag' set but not used
From: Matthew Wilcox @ 2019-08-01 16:00 UTC (permalink / raw)
  To: Qian Cai
  Cc: catalin.marinas, linux-kernel, kasan-dev, linux-mm, glider,
	linux-arm-kernel, andreyknvl, aryabinin, will, dvyukov
In-Reply-To: <1564670825-4050-1-git-send-email-cai@lca.pw>

On Thu, Aug 01, 2019 at 10:47:05AM -0400, Qian Cai wrote:

Given this:

> -#define __tag_set(addr, tag)	(addr)
> +static inline const void *__tag_set(const void *addr, u8 tag)
> +{
> +	return addr;
> +}
> +
>  #define __tag_reset(addr)	(addr)
>  #define __tag_get(addr)		0
>  #endif
> @@ -301,8 +305,8 @@ static inline void *phys_to_virt(phys_addr_t x)
>  #define page_to_virt(page)	({					\
>  	unsigned long __addr =						\
>  		((__page_to_voff(page)) | PAGE_OFFSET);			\
> -	unsigned long __addr_tag =					\
> -		 __tag_set(__addr, page_kasan_tag(page));		\
> +	const void *__addr_tag =					\
> +		__tag_set((void *)__addr, page_kasan_tag(page));	\
>  	((void *)__addr_tag);						\
>  })

Can't you simplify that macro to:

 #define page_to_virt(page)	({					\
 	unsigned long __addr =						\
 		((__page_to_voff(page)) | PAGE_OFFSET);			\
-	unsigned long __addr_tag =					\
-		 __tag_set(__addr, page_kasan_tag(page));		\
-	((void *)__addr_tag);						\
+	__tag_set((void *)__addr, page_kasan_tag(page));		\
 })

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 6/8] dma-direct: turn ARCH_ZONE_DMA_BITS into a variable
From: Nicolas Saenz Julienne @ 2019-08-01 15:59 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Benjamin Herrenschmidt, Heiko Carstens, eric, Paul Mackerras,
	will, Marek Szyprowski, phill, linux-s390, f.fainelli,
	frowand.list, Michael Ellerman, Christian Borntraeger,
	catalin.marinas, devicetree, Vasily Gorbik, marc.zyngier, robh+dt,
	linux-rpi-kernel, linux-arm-kernel, mbrugger, linux-mm,
	linuxppc-dev, linux-kernel, iommu, wahrenst, akpm, Robin Murphy
In-Reply-To: <20190801140452.GB23435@lst.de>


[-- Attachment #1.1: Type: text/plain, Size: 2485 bytes --]

Hi Christoph, thanks for the review.

On Thu, 2019-08-01 at 16:04 +0200, Christoph Hellwig wrote:
> A few nitpicks, otherwise this looks great:
> 
> > @@ -201,7 +202,7 @@ static int __init mark_nonram_nosave(void)
> >   * everything else. GFP_DMA32 page allocations automatically fall back to
> >   * ZONE_DMA.
> >   *
> > - * By using 31-bit unconditionally, we can exploit ARCH_ZONE_DMA_BITS to
> > + * By using 31-bit unconditionally, we can exploit arch_zone_dma_bits to
> >   * inform the generic DMA mapping code.  32-bit only devices (if not
> > handled
> >   * by an IOMMU anyway) will take a first dip into ZONE_NORMAL and get
> >   * otherwise served by ZONE_DMA.
> > @@ -237,9 +238,18 @@ void __init paging_init(void)
> >  	printk(KERN_DEBUG "Memory hole size: %ldMB\n",
> >  	       (long int)((top_of_ram - total_ram) >> 20));
> >  
> > +	/*
> > +	 * Allow 30-bit DMA for very limited Broadcom wifi chips on many
> > +	 * powerbooks.
> > +	 */
> > +	if (IS_ENABLED(CONFIG_PPC32))
> > +		arch_zone_dma_bits = 30;
> > +	else
> > +		arch_zone_dma_bits = 31;
> > +
> 
> So the above unconditionally comment obviously isn't true any more, and
> Ben also said for the recent ppc32 hack he'd prefer dynamic detection.
> 
> Maybe Ben and or other ppc folks can chime in an add a patch to the series
> to sort this out now that we have a dynamic ZONE_DMA threshold?

Noted, for now I'll remove the comment.

> > diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> > index 59bdceea3737..40dfc9b4ee4c 100644
> > --- a/kernel/dma/direct.c
> > +++ b/kernel/dma/direct.c
> > @@ -19,9 +19,7 @@
> >   * Most architectures use ZONE_DMA for the first 16 Megabytes, but
> >   * some use it for entirely different regions:
> >   */
> > -#ifndef ARCH_ZONE_DMA_BITS
> > -#define ARCH_ZONE_DMA_BITS 24
> > -#endif
> > +unsigned int arch_zone_dma_bits __ro_after_init = 24;
> 
> I'd prefer to drop the arch_ prefix and just calls this zone_dma_bits.
> In the long run we really need to find a way to just automatically set
> this from the meminit code, but that is out of scope for this series.
> For now can you please just update the comment above to say something
> like:
> 
> /*
>  * Most architectures use ZONE_DMA for the first 16 Megabytes, but some use it
>  * it for entirely different regions.  In that case the arch code needs to
>  * override the variable below for dma-direct to work properly.
>  */

Ok perfect.


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 5/8] arm64: use ZONE_DMA on DMA addressing limited devices
From: Robin Murphy @ 2019-08-01 16:07 UTC (permalink / raw)
  To: Nicolas Saenz Julienne, Catalin Marinas
  Cc: phill, devicetree, f.fainelli, linux-mm, marc.zyngier,
	Will Deacon, linux-kernel, eric, iommu, robh+dt, linux-rpi-kernel,
	mbrugger, akpm, m.szyprowski, frowand.list, hch, linux-arm-kernel,
	wahrenst
In-Reply-To: <d8b4a7cb9c06824ca88a0602a5bf38b6324b43c0.camel@suse.de>

On 2019-08-01 4:44 pm, Nicolas Saenz Julienne wrote:
> On Wed, 2019-07-31 at 18:07 +0100, Catalin Marinas wrote:
>> On Wed, Jul 31, 2019 at 05:47:48PM +0200, Nicolas Saenz Julienne wrote:
>>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>>> index 1c4ffabbe1cb..f5279ef85756 100644
>>> --- a/arch/arm64/mm/init.c
>>> +++ b/arch/arm64/mm/init.c
>>> @@ -50,6 +50,13 @@
>>>   s64 memstart_addr __ro_after_init = -1;
>>>   EXPORT_SYMBOL(memstart_addr);
>>>   
>>> +/*
>>> + * We might create both a ZONE_DMA and ZONE_DMA32. ZONE_DMA is needed if
>>> there
>>> + * are periferals unable to address the first naturally aligned 4GB of ram.
>>> + * ZONE_DMA32 will be expanded to cover the rest of that memory. If such
>>> + * limitations doesn't exist only ZONE_DMA32 is created.
>>> + */
>>
>> Shouldn't we instead only create ZONE_DMA to cover the whole 32-bit
>> range and leave ZONE_DMA32 empty? Can__GFP_DMA allocations fall back
>> onto ZONE_DMA32?
> 
> Hi Catalin, thanks for the review.
> 
> You're right, the GFP_DMA page allocation will fail with a nasty dmesg error if
> ZONE_DMA is configured but empty. Unsurprisingly the opposite situation is fine
> (GFP_DMA32 with an empty ZONE_DMA32).

Was that tested on something other than RPi4 with more than 4GB of RAM? 
(i.e. with a non-empty ZONE_NORMAL either way)

Robin.

> I switched to the scheme you're suggesting for the next version of the series.
> The comment will be something the likes of this:
> 
> /*
>   * We create both a ZONE_DMA and ZONE_DMA32. ZONE_DMA's size is decided based
>   * on whether the SoC's peripherals are able to address the first naturally
>   * aligned 4 GB of ram.
>   *
>   * If limited, ZONE_DMA covers that area and ZONE_DMA32 the rest of that 32 bit
>   * addressable memory.
>   *
>   * If not ZONE_DMA is expanded to cover the whole 32 bit addressable memory and
>   * ZONE_DMA32 is left empty.
>   */
> 
>   Regards,
>   Nicolas
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 1/2] ACPI/PPTT: Add support for ACPI 6.3 thread flag
From: Jeremy Linton @ 2019-08-01 16:10 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: lorenzo.pieralisi, catalin.marinas, rjw, linux-acpi, will,
	linux-arm-kernel, lenb
In-Reply-To: <20190801155736.GD23424@e107155-lin>

Hi,


Thanks for looking at this.

On 8/1/19 10:57 AM, Sudeep Holla wrote:
> 
> Hi Jeremy,
> 
> On Wed, Jul 31, 2019 at 10:46:33PM -0500, Jeremy Linton wrote:
>> ACPI 6.3 adds a flag to the CPU node to indicate whether
>> the given PE is a thread. Add a function to return that
>> information for a given linux logical CPU.
>>
> 
> Apart from few minor nits,
> 
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> 
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>>   drivers/acpi/pptt.c  | 54 +++++++++++++++++++++++++++++++++++++++++++-
>>   include/linux/acpi.h |  5 ++++
>>   2 files changed, 58 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
>> index 1e7ac0bd0d3a..84718f6cb741 100644
>> --- a/drivers/acpi/pptt.c
>> +++ b/drivers/acpi/pptt.c
>> @@ -540,6 +540,44 @@ static int find_acpi_cpu_topology_tag(unsigned int cpu, int level, int flag)
>>   	return retval;
>>   }
>>
>> +/**
>> + * check_acpi_cpu_flag() - Determine if CPU node has a flag set
>> + * @cpu: Kernel logical CPU number
>> + * @rev: The PPTT revision defining the flag
> 
> [nit] I would rather put it as minimum PPTT revision that supports the
> flag. It aligns with the code too as we are not looking for exact match.

Ok, sure.


> 
>> + * @flag: The flag itself
>> + *
>> + * Check the node representing a CPU for a given flag.
>> + *
>> + * Return: -ENOENT if the PPTT doesn't exist, the CPU cannot be found or
>> + *	   the table revision isn't new enough.
>> + *	   1, any passed flag set
>> + *	   0, flag unset
>> + */
>> +static int check_acpi_cpu_flag(unsigned int cpu, int rev, u32 flag)
>> +{
>> +	struct acpi_table_header *table;
>> +	acpi_status status;
>> +	u32 acpi_cpu_id = get_acpi_id_for_cpu(cpu);
>> +	struct acpi_pptt_processor *cpu_node = NULL;
>> +	int ret = -ENOENT;
>> +
>> +	status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
>> +	if (ACPI_FAILURE(status)) {
>> +		acpi_pptt_warn_missing();
>> +		return ret;
>> +	}
>> +
>> +	if (table->revision >= rev)
>> +		cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
>> +
>> +	if (cpu_node)
>> +		ret = (cpu_node->flags & flag) != 0;
>> +
>> +	acpi_put_table(table);
>> +
>> +	return ret;
>> +}
>> +
>>   /**
>>    * acpi_find_last_cache_level() - Determines the number of cache levels for a PE
>>    * @cpu: Kernel logical CPU number
>> @@ -604,6 +642,21 @@ int cache_setup_acpi(unsigned int cpu)
>>   	return status;
>>   }
>>
>> +/**
>> + * acpi_pptt_cpu_is_thread() - Determine if CPU is a thread
>> + * @cpu: Kernel logical CPU number
>> + *
> 
> [nit] If you spin the patch again, you can drop extra line space here.

Sure..


> 
> --
> Regards,
> Sudeep
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 2/3] media: imx7-media-csi: add i.MX6UL support
From: Fabio Estevam @ 2019-08-01 16:14 UTC (permalink / raw)
  To: Sébastien Szymanski
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:STAGING SUBSYSTEM, Pengutronix Kernel Team,
	Greg Kroah-Hartman, Sascha Hauer, Rui Miguel Silva, Rob Herring,
	NXP Linux Team, Philipp Zabel, Steve Longerbeam,
	Mauro Carvalho Chehab, Shawn Guo,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-media
In-Reply-To: <20190731163330.32532-1-sebastien.szymanski@armadeus.com>

On Wed, Jul 31, 2019 at 1:33 PM Sébastien Szymanski
<sebastien.szymanski@armadeus.com> wrote:
>
> i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
> to imx7-media-csi driver.
>
> Signed-off-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>

Reviewed-by: Fabio Estevam <festevam@gmail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 3/3] media: dt-bindings: imx7-csi: add i.MX6UL/L support
From: Fabio Estevam @ 2019-08-01 16:15 UTC (permalink / raw)
  To: Sébastien Szymanski
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:STAGING SUBSYSTEM, Pengutronix Kernel Team, Rob Herring,
	Greg Kroah-Hartman, Sascha Hauer, Rui Miguel Silva, Rob Herring,
	NXP Linux Team, Philipp Zabel, Steve Longerbeam,
	Mauro Carvalho Chehab, Shawn Guo,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-media
In-Reply-To: <20190731163358.32622-1-sebastien.szymanski@armadeus.com>

On Wed, Jul 31, 2019 at 1:34 PM Sébastien Szymanski
<sebastien.szymanski@armadeus.com> wrote:
>
> Document "fsl,imx6ul-csi" entry.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>

Reviewed-by: Fabio Estevam <festevam@gmail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: next/master boot: 265 boots: 17 failed, 184 passed with 64 offline (next-20190730)
From: Jeffrey Hugo @ 2019-08-01 16:17 UTC (permalink / raw)
  To: Linus Walleij, Timur Tabi
  Cc: Kernel Build Reports Mailman List, Lina Iyer, Stephen Boyd,
	Mark Brown, Bjorn Andersson, Lee Jones, Linux ARM
In-Reply-To: <CACRpkdbpBnO4KgpjYReafWKKoq0AX5kAb99tCC9LXE0_7sW7Jg@mail.gmail.com>

On 8/1/2019 1:09 AM, Linus Walleij wrote:
> On Thu, Aug 1, 2019 at 5:49 AM Timur Tabi <timur@kernel.org> wrote:
>> On 7/31/19 12:58 PM, Jeffrey Hugo wrote:
>>>>>
>>>>> Timur/Stephen: any ideas?
>>>
>>> Timur's CAF account is no longer valid, I added his @kernel one.
>>
>> Delete everything specific to the QDF2400.
> 
> It appears Mark is still using it in his test farm, and now its sole
> role is finding bugs in my code. Which it did! With so much elegance
> that we could fix it up quickly.
> 
>> Qualcomm has made it very clear that they have no interest in developing
>> ARM server chips.  No QDF2400 system ever made it to official production.
> 
> That's sad. I remember we had lots of discussions around this at the
> time. The ACPI code base and quirks we added is however used in
> other Qualcomm-based machines now (what Lee is doing), so the effort
> is not wasted at all.

There are a few QDF2400 systems around that are being supported. 
Certainly not to the scale that was intended/envisioned, but they do exist.

Feel free to poke me for questions/issues, although its not my primary 
job so responses may be a bit slow.


-- 
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2] arm64/mm: fix variable 'tag' set but not used
From: Qian Cai @ 2019-08-01 16:20 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: catalin.marinas, linux-kernel, kasan-dev, linux-mm, glider,
	linux-arm-kernel, andreyknvl, aryabinin, will, dvyukov
In-Reply-To: <20190801160013.GK4700@bombadil.infradead.org>

On Thu, 2019-08-01 at 09:00 -0700, Matthew Wilcox wrote:
> On Thu, Aug 01, 2019 at 10:47:05AM -0400, Qian Cai wrote:
> 
> Given this:
> 
> > -#define __tag_set(addr, tag)	(addr)
> > +static inline const void *__tag_set(const void *addr, u8 tag)
> > +{
> > +	return addr;
> > +}
> > +
> >  #define __tag_reset(addr)	(addr)
> >  #define __tag_get(addr)		0
> >  #endif
> > @@ -301,8 +305,8 @@ static inline void *phys_to_virt(phys_addr_t x)
> >  #define page_to_virt(page)	({					
> > \
> >  	unsigned long __addr =						
> > \
> >  		((__page_to_voff(page)) | PAGE_OFFSET);			
> > \
> > -	unsigned long __addr_tag =					\
> > -		 __tag_set(__addr, page_kasan_tag(page));		\
> > +	const void *__addr_tag =					\
> > +		__tag_set((void *)__addr, page_kasan_tag(page));	\
> >  	((void *)__addr_tag);						
> > \
> >  })
> 
> Can't you simplify that macro to:
> 
>  #define page_to_virt(page)	({					\
>  	unsigned long __addr =						
> \
>  		((__page_to_voff(page)) | PAGE_OFFSET);			
> \
> -	unsigned long __addr_tag =					\
> -		 __tag_set(__addr, page_kasan_tag(page));		\
> -	((void *)__addr_tag);						
> \
> +	__tag_set((void *)__addr, page_kasan_tag(page));		\
>  })

It still need a cast or lowmem_page_address() will complain of a discarded
"const". It might be a bit harder to read when adding a cast as in,

((void *)__tag_set((void *)__addr, page_kasan_tag(page)));

But, that feel like more of a followup patch for me if ever needed.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] dma-mapping: fix page attributes for dma_mmap_*
From: Will Deacon @ 2019-08-01 16:23 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Shawn Anastasio, Michael Ellerman, linuxppc-dev, Russell King,
	linux-kernel, iommu, Catalin Marinas, Robin Murphy,
	linux-arm-kernel
In-Reply-To: <20190801142118.21225-2-hch@lst.de>

On Thu, Aug 01, 2019 at 05:21:18PM +0300, Christoph Hellwig wrote:
> All the way back to introducing dma_common_mmap we've defaulyed to mark
> the pages as uncached.  But this is wrong for DMA coherent devices or
> if using DMA_ATTR_NON_CONSISTENT.  Later on DMA_ATTR_WRITE_COMBINE
> also got incorrect treatment as that flag is only treated special on
> the alloc side for non-coherent devices.
> 
> Introduce a new dma_mmap_pgprot helper that deals with the check
> for coherent devices and DMA_ATTR_NON_CONSISTENT so that only the
> remapping cases even reach arch_dma_mmap_pgprot and we thus ensure
> no aliasing of page attributes happens.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/arm/mm/dma-mapping.c        |  4 +---
>  arch/arm64/mm/dma-mapping.c      |  4 +---
>  arch/powerpc/kernel/Makefile     |  3 +--
>  arch/powerpc/kernel/dma-common.c | 17 -----------------
>  drivers/iommu/dma-iommu.c        |  6 +++---
>  include/linux/dma-mapping.h      |  1 +
>  include/linux/dma-noncoherent.h  |  5 -----
>  kernel/dma/mapping.c             | 11 ++++++++++-
>  kernel/dma/remap.c               |  2 +-
>  9 files changed, 18 insertions(+), 35 deletions(-)
>  delete mode 100644 arch/powerpc/kernel/dma-common.c
> 
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 9c9a23e5600d..cfe44df169c5 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -2397,9 +2397,7 @@ long arch_dma_coherent_to_pfn(struct device *dev, void *cpu_addr,
>  pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
>  		unsigned long attrs)
>  {
> -	if (!dev_is_dma_coherent(dev))
> -		return __get_dma_pgprot(attrs, prot);
> -	return prot;
> +	return __get_dma_pgprot(attrs, prot);
>  }
>  
>  void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 1d3f0b5a9940..bd2b039f43a6 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -14,9 +14,7 @@
>  pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
>  		unsigned long attrs)
>  {
> -	if (!dev_is_dma_coherent(dev) || (attrs & DMA_ATTR_WRITE_COMBINE))
> -		return pgprot_writecombine(prot);
> -	return prot;
> +	return pgprot_writecombine(prot);
>  }

Seems like a sensible cleanup to me:

Acked-by: Will Deacon <will@kernel.org>

Although arch_dma_mmap_pgprot() is a bit of a misnomer now that it only
gets involved in the non-coherent case.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] dma-mapping: fix page attributes for dma_mmap_*
From: Christoph Hellwig @ 2019-08-01 16:34 UTC (permalink / raw)
  To: Will Deacon
  Cc: Shawn Anastasio, Michael Ellerman, linuxppc-dev, Russell King,
	linux-kernel, iommu, Catalin Marinas, Robin Murphy,
	Christoph Hellwig, linux-arm-kernel
In-Reply-To: <20190801162305.3m32chycsdjmdejk@willie-the-truck>

On Thu, Aug 01, 2019 at 05:23:06PM +0100, Will Deacon wrote:
> > -	if (!dev_is_dma_coherent(dev) || (attrs & DMA_ATTR_WRITE_COMBINE))
> > -		return pgprot_writecombine(prot);
> > -	return prot;
> > +	return pgprot_writecombine(prot);
> >  }
> 
> Seems like a sensible cleanup to me:
> 
> Acked-by: Will Deacon <will@kernel.org>
> 
> Although arch_dma_mmap_pgprot() is a bit of a misnomer now that it only
> gets involved in the non-coherent case.

A better name is welcome.  My other idea would be to just remove it
entirely and do something like:

#ifndef pgprot_dmacoherent
#define pgprot_dmacoherent pgprot_noncached
#endif

pgprot_t dma_mmap_pgprot(struct device *dev, pgprot_t prot, unsigned long attrs)
{
	if (dev_is_dma_coherent(dev) || (attrs & DMA_ATTR_NON_CONSISTENT))
		return prot;
#ifdef pgprot_writecombine
	if (attrs & DMA_ATTR_WRITE_COMBINE)
		return pgprot_writecombine(prot);
#endif
	return pgprot_dmacoherent(prot);
}

But my worry is how this interacts with architectures that have an
uncached segment (mips, nios2, microblaze, extensa) where we'd have
the kernel access DMA_ATTR_WRITE_COMBINE mappigns using the uncached
segment, and userspace mmaps using pgprot_writecombine, which could
lead to aliasing issues.  But then again mips already supports
DMA_ATTR_WRITE_COMBINE, so this must be ok somehow.  I guess I'll
need to field that question to the relevant parties.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 2/2] arm64: entry: Move ct_user_exit before we can take another exception
From: Will Deacon @ 2019-08-01 16:39 UTC (permalink / raw)
  To: James Morse; +Cc: Catalin Marinas, Masami Hiramatsu, linux-arm-kernel
In-Reply-To: <20190801154150.195959-3-james.morse@arm.com>

On Thu, Aug 01, 2019 at 04:41:50PM +0100, James Morse wrote:
> When taking an exception from EL0 we unmask exceptions before calling
> ct_user_exit. This means we could take an interrupt or be single-stepped
> in the gap. The entry from EL1 code sees that we took an exception from
> the kernel, whereas the context_tracking code believes we are still in
> user-space.
> 
> To keep these things consistent, move the ct_user_exit calls before
> any unmask of exceptions.
> 
> Fixes: 6c81fe7925cc4c42 ("arm64: enable context tracking")
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  arch/arm64/kernel/entry.S | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 28681034d599..88f4ab21cb66 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -792,8 +792,8 @@ el0_cp15:
>  	/*
>  	 * Trapped CP15 (MRC, MCR, MRRC, MCRR) instructions
>  	 */
> -	enable_daif
>  	ct_user_exit
> +	enable_daif
>  	mov	x0, x25
>  	mov	x1, sp
>  	bl	do_cp15instr
> @@ -805,8 +805,8 @@ el0_da:
>  	 * Data abort handling
>  	 */
>  	mrs	x26, far_el1
> -	enable_daif
>  	ct_user_exit
> +	enable_daif

This strikes me as a bit dodgy, since we end up in context_tracking_exit(),
which calls local_irq_{save,restore}() and I think our accounting is
probably off at this point. I think we need to call via
user_{enter,exit}_irqoff() instead to make this work.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 5/8] arm64: use ZONE_DMA on DMA addressing limited devices
From: Nicolas Saenz Julienne @ 2019-08-01 16:40 UTC (permalink / raw)
  To: Robin Murphy, Catalin Marinas
  Cc: phill, devicetree, f.fainelli, wahrenst, eric, marc.zyngier,
	frowand.list, linux-kernel, linux-mm, iommu, robh+dt,
	linux-rpi-kernel, mbrugger, akpm, Will Deacon, hch,
	linux-arm-kernel, m.szyprowski
In-Reply-To: <e35dd4a5-281b-d281-59c9-3fc7108eb8be@arm.com>


[-- Attachment #1.1: Type: text/plain, Size: 2044 bytes --]

On Thu, 2019-08-01 at 17:07 +0100, Robin Murphy wrote:
> On 2019-08-01 4:44 pm, Nicolas Saenz Julienne wrote:
> > On Wed, 2019-07-31 at 18:07 +0100, Catalin Marinas wrote:
> > > On Wed, Jul 31, 2019 at 05:47:48PM +0200, Nicolas Saenz Julienne wrote:
> > > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > > > index 1c4ffabbe1cb..f5279ef85756 100644
> > > > --- a/arch/arm64/mm/init.c
> > > > +++ b/arch/arm64/mm/init.c
> > > > @@ -50,6 +50,13 @@
> > > >   s64 memstart_addr __ro_after_init = -1;
> > > >   EXPORT_SYMBOL(memstart_addr);
> > > >   
> > > > +/*
> > > > + * We might create both a ZONE_DMA and ZONE_DMA32. ZONE_DMA is needed
> > > > if
> > > > there
> > > > + * are periferals unable to address the first naturally aligned 4GB of
> > > > ram.
> > > > + * ZONE_DMA32 will be expanded to cover the rest of that memory. If
> > > > such
> > > > + * limitations doesn't exist only ZONE_DMA32 is created.
> > > > + */
> > > 
> > > Shouldn't we instead only create ZONE_DMA to cover the whole 32-bit
> > > range and leave ZONE_DMA32 empty? Can__GFP_DMA allocations fall back
> > > onto ZONE_DMA32?
> > 
> > Hi Catalin, thanks for the review.
> > 
> > You're right, the GFP_DMA page allocation will fail with a nasty dmesg error
> > if
> > ZONE_DMA is configured but empty. Unsurprisingly the opposite situation is
> > fine
> > (GFP_DMA32 with an empty ZONE_DMA32).
> 
> Was that tested on something other than RPi4 with more than 4GB of RAM? 
> (i.e. with a non-empty ZONE_NORMAL either way)

No, all I did is play around with RPi4's memory size (1 GB vs 4 GB).

I'll see If I can get access to a dts based board with more than 4 GB, If not
I'll try to fake it. It's not ideal but I can set the limit on 3 GB and have
the 3 areas created (with and witouth an empty ZONE_DMA32).

On top of that, now that you ask, I realise I neglected all the ACPI based
servers. I have access to some so I'll make sure I test everything on them too
for the next series.

Regards,
Nicolas


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] dma-mapping: fix page attributes for dma_mmap_*
From: Will Deacon @ 2019-08-01 16:44 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Shawn Anastasio, Michael Ellerman, linuxppc-dev, Russell King,
	linux-kernel, iommu, Catalin Marinas, Robin Murphy,
	linux-arm-kernel
In-Reply-To: <20190801163457.GB26588@lst.de>

On Thu, Aug 01, 2019 at 06:34:57PM +0200, Christoph Hellwig wrote:
> On Thu, Aug 01, 2019 at 05:23:06PM +0100, Will Deacon wrote:
> > > -	if (!dev_is_dma_coherent(dev) || (attrs & DMA_ATTR_WRITE_COMBINE))
> > > -		return pgprot_writecombine(prot);
> > > -	return prot;
> > > +	return pgprot_writecombine(prot);
> > >  }
> > 
> > Seems like a sensible cleanup to me:
> > 
> > Acked-by: Will Deacon <will@kernel.org>
> > 
> > Although arch_dma_mmap_pgprot() is a bit of a misnomer now that it only
> > gets involved in the non-coherent case.
> 
> A better name is welcome.

How about arch_dma_noncoherent_mmap_pgprot() ? Too long?

> My other idea would be to just remove it entirely and do something like:
> 
> #ifndef pgprot_dmacoherent
> #define pgprot_dmacoherent pgprot_noncached
> #endif
> 
> pgprot_t dma_mmap_pgprot(struct device *dev, pgprot_t prot, unsigned long attrs)
> {
> 	if (dev_is_dma_coherent(dev) || (attrs & DMA_ATTR_NON_CONSISTENT))
> 		return prot;
> #ifdef pgprot_writecombine
> 	if (attrs & DMA_ATTR_WRITE_COMBINE)
> 		return pgprot_writecombine(prot);
> #endif
> 	return pgprot_dmacoherent(prot);
> }

Oh, I prefer that!

> But my worry is how this interacts with architectures that have an
> uncached segment (mips, nios2, microblaze, extensa) where we'd have
> the kernel access DMA_ATTR_WRITE_COMBINE mappigns using the uncached
> segment, and userspace mmaps using pgprot_writecombine, which could
> lead to aliasing issues.  But then again mips already supports
> DMA_ATTR_WRITE_COMBINE, so this must be ok somehow.  I guess I'll
> need to field that question to the relevant parties.

Or it's always been busted and happens to work out in practice...

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ 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