Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 07/18] arm64: remove sigreturn wrappers
From: Dave Martin @ 2018-05-14 11:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180514094640.27569-8-mark.rutland@arm.com>

On Mon, May 14, 2018 at 10:46:29AM +0100, Mark Rutland wrote:
> The arm64 sigreturn* syscall handlers are non-standard. Rather than
> taking a number of user parameters in registers as per the AAPCS,
> they expect the pt_regs as their sole argument.
> 
> To make this work, we override the syscall definitions to invoke
> wrappers written in assembly, which mov the SP into x0, and branch to
> their respective C functions.
> 
> On other architectures (such as x86), the sigreturn* functions take no
> argument and instead use current_pt_regs() to acquire the user
> registers. This requires less boilerplate code, and allows for other
> features such as interposing C code in this path.

Hmmm, I always wondered why rt_sigreturn() et al. were handled
specially.

If there's effectively no underlying reason for that, it makes sense
to regularise the interface to these syscalls.

Tentatively-reviewed-by: Dave Martin <Dave.Martin@arm.com>

> 
> This patch takes the same approach for arm64.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/include/asm/unistd32.h |  4 ++--
>  arch/arm64/kernel/entry.S         |  8 --------
>  arch/arm64/kernel/entry32.S       | 10 ----------
>  arch/arm64/kernel/signal.c        |  3 ++-
>  arch/arm64/kernel/signal32.c      |  6 ++++--
>  arch/arm64/kernel/sys.c           |  3 +--
>  arch/arm64/kernel/sys32.c         |  4 ++--
>  7 files changed, 11 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
> index ef292160748c..ab95554b1734 100644
> --- a/arch/arm64/include/asm/unistd32.h
> +++ b/arch/arm64/include/asm/unistd32.h
> @@ -260,7 +260,7 @@ __SYSCALL(117, sys_ni_syscall)
>  #define __NR_fsync 118
>  __SYSCALL(__NR_fsync, sys_fsync)
>  #define __NR_sigreturn 119
> -__SYSCALL(__NR_sigreturn, compat_sys_sigreturn_wrapper)
> +__SYSCALL(__NR_sigreturn, compat_sys_sigreturn)
>  #define __NR_clone 120
>  __SYSCALL(__NR_clone, sys_clone)
>  #define __NR_setdomainname 121
> @@ -368,7 +368,7 @@ __SYSCALL(__NR_getresgid, sys_getresgid16)
>  #define __NR_prctl 172
>  __SYSCALL(__NR_prctl, sys_prctl)
>  #define __NR_rt_sigreturn 173
> -__SYSCALL(__NR_rt_sigreturn, compat_sys_rt_sigreturn_wrapper)
> +__SYSCALL(__NR_rt_sigreturn, compat_sys_rt_sigreturn)
>  #define __NR_rt_sigaction 174
>  __SYSCALL(__NR_rt_sigaction, compat_sys_rt_sigaction)
>  #define __NR_rt_sigprocmask 175
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index ec2ee720e33e..08ea3cbfb08f 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -1108,14 +1108,6 @@ __entry_tramp_data_start:
>  #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
>  
>  /*
> - * Special system call wrappers.
> - */
> -ENTRY(sys_rt_sigreturn_wrapper)
> -	mov	x0, sp
> -	b	sys_rt_sigreturn
> -ENDPROC(sys_rt_sigreturn_wrapper)
> -
> -/*
>   * Register switch for AArch64. The callee-saved registers need to be saved
>   * and restored. On entry:
>   *   x0 = previous task_struct (must be preserved across the switch)
> diff --git a/arch/arm64/kernel/entry32.S b/arch/arm64/kernel/entry32.S
> index f332d5d1f6b4..f9461696dde4 100644
> --- a/arch/arm64/kernel/entry32.S
> +++ b/arch/arm64/kernel/entry32.S
> @@ -30,16 +30,6 @@
>   * System call wrappers for the AArch32 compatibility layer.
>   */
>  
> -ENTRY(compat_sys_sigreturn_wrapper)
> -	mov	x0, sp
> -	b	compat_sys_sigreturn
> -ENDPROC(compat_sys_sigreturn_wrapper)
> -
> -ENTRY(compat_sys_rt_sigreturn_wrapper)
> -	mov	x0, sp
> -	b	compat_sys_rt_sigreturn
> -ENDPROC(compat_sys_rt_sigreturn_wrapper)
> -
>  ENTRY(compat_sys_statfs64_wrapper)
>  	mov	w3, #84
>  	cmp	w1, #88
> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> index 8e624fec4707..caa7a68cf2d2 100644
> --- a/arch/arm64/kernel/signal.c
> +++ b/arch/arm64/kernel/signal.c
> @@ -538,8 +538,9 @@ static int restore_sigframe(struct pt_regs *regs,
>  	return err;
>  }
>  
> -asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
> +asmlinkage long sys_rt_sigreturn(void)
>  {
> +	struct pt_regs *regs = current_pt_regs();
>  	struct rt_sigframe __user *frame;
>  
>  	/* Always make any pending restarted system calls return -EINTR */
> diff --git a/arch/arm64/kernel/signal32.c b/arch/arm64/kernel/signal32.c
> index 77b91f478995..cb10588a7cb2 100644
> --- a/arch/arm64/kernel/signal32.c
> +++ b/arch/arm64/kernel/signal32.c
> @@ -282,8 +282,9 @@ static int compat_restore_sigframe(struct pt_regs *regs,
>  	return err;
>  }
>  
> -asmlinkage int compat_sys_sigreturn(struct pt_regs *regs)
> +asmlinkage int compat_sys_sigreturn(void)
>  {
> +	struct pt_regs *regs = current_pt_regs();
>  	struct compat_sigframe __user *frame;
>  
>  	/* Always make any pending restarted system calls return -EINTR */
> @@ -312,8 +313,9 @@ asmlinkage int compat_sys_sigreturn(struct pt_regs *regs)
>  	return 0;
>  }
>  
> -asmlinkage int compat_sys_rt_sigreturn(struct pt_regs *regs)
> +asmlinkage int compat_sys_rt_sigreturn(void)
>  {
> +	struct pt_regs *regs = current_pt_regs();
>  	struct compat_rt_sigframe __user *frame;
>  
>  	/* Always make any pending restarted system calls return -EINTR */
> diff --git a/arch/arm64/kernel/sys.c b/arch/arm64/kernel/sys.c
> index 72981bae10eb..31045f3fed92 100644
> --- a/arch/arm64/kernel/sys.c
> +++ b/arch/arm64/kernel/sys.c
> @@ -48,8 +48,7 @@ SYSCALL_DEFINE1(arm64_personality, unsigned int, personality)
>  /*
>   * Wrappers to pass the pt_regs argument.
>   */
> -asmlinkage long sys_rt_sigreturn_wrapper(void);
> -#define sys_rt_sigreturn	sys_rt_sigreturn_wrapper
> +asmlinkage long sys_rt_sigreturn(void);
>  #define sys_personality		sys_arm64_personality
>  
>  #undef __SYSCALL
> diff --git a/arch/arm64/kernel/sys32.c b/arch/arm64/kernel/sys32.c
> index a40b1343b819..1ef103c95410 100644
> --- a/arch/arm64/kernel/sys32.c
> +++ b/arch/arm64/kernel/sys32.c
> @@ -25,8 +25,8 @@
>  #include <linux/compiler.h>
>  #include <linux/syscalls.h>
>  
> -asmlinkage long compat_sys_sigreturn_wrapper(void);
> -asmlinkage long compat_sys_rt_sigreturn_wrapper(void);
> +asmlinkage long compat_sys_sigreturn(void);
> +asmlinkage long compat_sys_rt_sigreturn(void);
>  asmlinkage long compat_sys_statfs64_wrapper(void);
>  asmlinkage long compat_sys_fstatfs64_wrapper(void);
>  asmlinkage long compat_sys_pread64_wrapper(void);
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 06/18] arm64: move sve_user_{enable, disable} to <asm/fpsimd.h>
From: Dave Martin @ 2018-05-14 11:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180514094640.27569-7-mark.rutland@arm.com>

On Mon, May 14, 2018 at 10:46:28AM +0100, Mark Rutland wrote:
> In subsequent patches, we'll want to make use of sve_user_enable() and
> sve_user_disable() outside of kernel/fpsimd.c. Let's move these to
> <asm/fpsimd.h> where we can make use of them.
> 
> To avoid ifdeffery in sequences like:
> 
> if (system_supports_sve() && some_condition
> 	sve_user_disable();
> 
> ... empty stubs are provided when support for SVE is not enabled.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Dave Martin <dave.martin@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/include/asm/fpsimd.h | 17 ++++++++++++++++-
>  arch/arm64/kernel/fpsimd.c      | 11 -----------
>  2 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
> index aa7162ae93e3..7377d7593c06 100644
> --- a/arch/arm64/include/asm/fpsimd.h
> +++ b/arch/arm64/include/asm/fpsimd.h
> @@ -16,11 +16,13 @@
>  #ifndef __ASM_FP_H
>  #define __ASM_FP_H
>  
> -#include <asm/ptrace.h>
>  #include <asm/errno.h>
> +#include <asm/ptrace.h>
> +#include <asm/sysreg.h>
>  
>  #ifndef __ASSEMBLY__
>  
> +#include <linux/build_bug.h>
>  #include <linux/cache.h>
>  #include <linux/init.h>
>  #include <linux/stddef.h>
> @@ -81,6 +83,16 @@ extern int sve_set_vector_length(struct task_struct *task,
>  extern int sve_set_current_vl(unsigned long arg);
>  extern int sve_get_current_vl(void);
>  
> +static inline void sve_user_disable(void)
> +{
> +	sysreg_clear_set(cpacr_el1, CPACR_EL1_ZEN_EL0EN, 0);
> +}
> +
> +static inline void sve_user_enable(void)
> +{
> +	sysreg_clear_set(cpacr_el1, 0, CPACR_EL1_ZEN_EL0EN);
> +}
> +
>  /*
>   * Probing and setup functions.
>   * Calls to these functions must be serialised with one another.
> @@ -107,6 +119,9 @@ static inline int sve_get_current_vl(void)
>  	return -EINVAL;
>  }
>  
> +static inline void sve_user_disable(void) { }
> +static inline void sve_user_enable(void) { }
> +

Alternatively, just move the full definitions outside the #ifdef
CONFIG_ARM64_SVE.

All calls to these should be shadowed by an if
(system_supports_sve()) in any case, and setting/clearing ZEN_EL0EN
in the CPACR_EL1 ought to be harmless now that the meaning of these
bits architecturally committed.

Ideally we would have a BUG_ON(!system_supports_sve()) in those
functions, but we won't won't to pay the cost in a production kernel.

>  static inline void sve_init_vq_map(void) { }
>  static inline void sve_update_vq_map(void) { }
>  static inline int sve_verify_vq_map(void) { return 0; }
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 088940387a4d..79a81c7d85c6 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -159,7 +159,6 @@ static void sve_free(struct task_struct *task)
>  	__sve_free(task);
>  }
>  
> -

Hmmm, Ack.  Check for conflicts with the KVM FPSIMD rework [1] (though
trivial).

[...]

Cheers
---Dave


[1]

[PATCH v7 10/16] arm64/sve: Switch sve_pffr() argument from task to thread
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/576601.html

[PATCH v7 11/16] arm64/sve: Move sve_pffr() to fpsimd.h and make inline
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/576606.html

^ permalink raw reply

* [PATCH 2/2] [stable 4.4] arm64: Add work around for Arm Cortex-A55 Erratum 1024718
From: Suzuki K Poulose @ 2018-05-14 11:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526295664-22187-1-git-send-email-suzuki.poulose@arm.com>

commit ece1397cbc89c51914fae1aec729539cfd8bd62b upstream

Some variants of the Arm Cortex-55 cores (r0p0, r0p1, r1p0) suffer
from an erratum 1024718, which causes incorrect updates when DBM/AP
bits in a page table entry is modified without a break-before-make
sequence. The work around is to disable the hardware DBM feature
on the affected cores. The hardware Access Flag management features
is not affected.

The hardware DBM feature is a non-conflicting capability, i.e, the
kernel could handle cores using the feature and those without having
the features running at the same time. So this work around is detected
at early boot time, rather than delaying it until the CPUs are brought
up into the kernel with MMU turned on. This also avoids other complexities
with late CPUs turning online, with or without the hardware DBM features.

Cc: stable at vger.kernel.org # v4.4
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
Note: The upstream commit is on top of a reworked capability
infrastructure for arm64 heterogeneous systems, which allows
delaying the CPU model checks. This backport is based on the
original version of the patch [0], which checks the affected
CPU models during the early boot.

[0] https://lkml.kernel.org/r/20180116102323.3470-1-suzuki.poulose at arm.com
---
 arch/arm64/Kconfig                 | 14 +++++++++++++
 arch/arm64/include/asm/assembler.h | 40 ++++++++++++++++++++++++++++++++++++++
 arch/arm64/include/asm/cputype.h   | 11 +++++++++++
 arch/arm64/mm/proc.S               |  5 +++++
 4 files changed, 70 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 02c0867..5b47218 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -375,6 +375,20 @@ config ARM64_ERRATUM_843419
 
 	  If unsure, say Y.
 
+config ARM64_ERRATUM_1024718
+	bool "Cortex-A55: 1024718: Update of DBM/AP bits without break before make might result in incorrect update"
+	default y
+	help
+	  This option adds work around for Arm Cortex-A55 Erratum 1024718.
+
+	  Affected Cortex-A55 cores (r0p0, r0p1, r1p0) could cause incorrect
+	  update of the hardware dirty bit when the DBM/AP bits are updated
+	  without a break-before-make. The work around is to disable the usage
+	  of hardware DBM locally on the affected cores. CPUs not affected by
+	  erratum will continue to use the feature.
+
+	  If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
 	bool "Cavium erratum 22375, 24313"
 	default y
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 83e2309..f68abb1 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -23,6 +23,7 @@
 #ifndef __ASM_ASSEMBLER_H
 #define __ASM_ASSEMBLER_H
 
+#include <asm/cputype.h>
 #include <asm/ptrace.h>
 #include <asm/thread_info.h>
 
@@ -224,4 +225,43 @@ lr	.req	x30		// link register
 	movk	\reg, :abs_g0_nc:\val
 	.endm
 
+/*
+ * Check the MIDR_EL1 of the current CPU for a given model and a range of
+ * variant/revision. See asm/cputype.h for the macros used below.
+ *
+ *	model:		MIDR_CPU_PART of CPU
+ *	rv_min:		Minimum of MIDR_CPU_VAR_REV()
+ *	rv_max:		Maximum of MIDR_CPU_VAR_REV()
+ *	res:		Result register.
+ *	tmp1, tmp2, tmp3: Temporary registers
+ *
+ * Corrupts: res, tmp1, tmp2, tmp3
+ * Returns:  0, if the CPU id doesn't match. Non-zero otherwise
+ */
+	.macro	cpu_midr_match model, rv_min, rv_max, res, tmp1, tmp2, tmp3
+	mrs		\res, midr_el1
+	mov_q		\tmp1, (MIDR_REVISION_MASK | MIDR_VARIANT_MASK)
+	mov_q		\tmp2, MIDR_CPU_PART_MASK
+	and		\tmp3, \res, \tmp2	// Extract model
+	and		\tmp1, \res, \tmp1	// rev & variant
+	mov_q		\tmp2, \model
+	cmp		\tmp3, \tmp2
+	cset		\res, eq
+	cbz		\res, .Ldone\@		// Model matches ?
+
+	.if (\rv_min != 0)			// Skip min check if rv_min == 0
+	mov_q		\tmp3, \rv_min
+	cmp		\tmp1, \tmp3
+	cset		\res, ge
+	.endif					// \rv_min != 0
+	/* Skip rv_max check if rv_min == rv_max && rv_min != 0 */
+	.if ((\rv_min != \rv_max) || \rv_min == 0)
+	mov_q		\tmp2, \rv_max
+	cmp		\tmp1, \tmp2
+	cset		\tmp2, le
+	and		\res, \res, \tmp2
+	.endif
+.Ldone\@:
+	.endm
+
 #endif	/* __ASM_ASSEMBLER_H */
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 1a59493..f43e10c 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -57,6 +57,14 @@
 #define MIDR_IMPLEMENTOR(midr)	\
 	(((midr) & MIDR_IMPLEMENTOR_MASK) >> MIDR_IMPLEMENTOR_SHIFT)
 
+#define MIDR_CPU_VAR_REV(var, rev) \
+	(((var) << MIDR_VARIANT_SHIFT) | (rev))
+
+#define MIDR_CPU_PART_MASK	  \
+	(MIDR_IMPLEMENTOR_MASK	| \
+	 MIDR_ARCHITECTURE_MASK | \
+	 MIDR_PARTNUM_MASK)
+
 #define MIDR_CPU_PART(imp, partnum) \
 	(((imp)			<< MIDR_IMPLEMENTOR_SHIFT) | \
 	(0xf			<< MIDR_ARCHITECTURE_SHIFT) | \
@@ -70,11 +78,14 @@
 #define ARM_CPU_PART_FOUNDATION		0xD00
 #define ARM_CPU_PART_CORTEX_A57		0xD07
 #define ARM_CPU_PART_CORTEX_A53		0xD03
+#define ARM_CPU_PART_CORTEX_A55		0xD05
 
 #define APM_CPU_PART_POTENZA		0x000
 
 #define CAVIUM_CPU_PART_THUNDERX	0x0A1
 
+#define MIDR_CORTEX_A55 MIDR_CPU_PART(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A55)
+
 #ifndef __ASSEMBLY__
 
 /*
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 18201e9..f096367 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -221,6 +221,11 @@ ENTRY(__cpu_setup)
 	cbz	x9, 2f
 	cmp	x9, #2
 	b.lt	1f
+#ifdef CONFIG_ARM64_ERRATUM_1024718
+	/* Disable hardware DBM on Cortex-A55 r0p0, r0p1 & r1p0 */
+	cpu_midr_match MIDR_CORTEX_A55, MIDR_CPU_VAR_REV(0, 0), MIDR_CPU_VAR_REV(1, 0), x1, x2, x3, x4
+	cbnz	x1, 1f
+#endif
 	orr	x10, x10, #TCR_HD		// hardware Dirty flag update
 1:	orr	x10, x10, #TCR_HA		// hardware Access flag update
 2:
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/2][stable-4.4] arm64: introduce mov_q macro to move a constant into a 64-bit register
From: Suzuki K Poulose @ 2018-05-14 11:01 UTC (permalink / raw)
  To: linux-arm-kernel

commit 30b5ba5cf333cc650e474eaf2cc1ae91bc7cf89f upstream

Implement a macro mov_q that can be used to move an immediate constant
into a 64-bit register, using between 2 and 4 movz/movk instructions
(depending on the operand)

Cc: stable at vger.kernel.org # v4.4
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
  Cherry picked for backport of
  "commit ece1397cbc89c51914fae1aec729539cfd8bd62b upstream"
---
 arch/arm64/include/asm/assembler.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 12eff92..83e2309 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -204,4 +204,24 @@ lr	.req	x30		// link register
 	.size	__pi_##x, . - x;	\
 	ENDPROC(x)
 
+	/*
+	 * mov_q - move an immediate constant into a 64-bit register using
+	 *         between 2 and 4 movz/movk instructions (depending on the
+	 *         magnitude and sign of the operand)
+	 */
+	.macro	mov_q, reg, val
+	.if (((\val) >> 31) == 0 || ((\val) >> 31) == 0x1ffffffff)
+	movz	\reg, :abs_g1_s:\val
+	.else
+	.if (((\val) >> 47) == 0 || ((\val) >> 47) == 0x1ffff)
+	movz	\reg, :abs_g2_s:\val
+	.else
+	movz	\reg, :abs_g3:\val
+	movk	\reg, :abs_g2_nc:\val
+	.endif
+	movk	\reg, :abs_g1_nc:\val
+	.endif
+	movk	\reg, :abs_g0_nc:\val
+	.endm
+
 #endif	/* __ASM_ASSEMBLER_H */
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd
From: Richard Genoud @ 2018-05-14 10:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180511103822.31698-7-radu.pirea@microchip.com>

Hi,

On 11/05/2018 12:38, Radu Pirea wrote:
> This patch modifies the place where resources and device tree properties
> are searched.
> 
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
>  drivers/tty/serial/Kconfig        |  1 +
>  drivers/tty/serial/atmel_serial.c | 29 +++++++++++++++--------------
>  2 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 3682fd3e960c..25e55332f8b1 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -119,6 +119,7 @@ config SERIAL_ATMEL
>  	depends on ARCH_AT91 || COMPILE_TEST
>  	select SERIAL_CORE
>  	select SERIAL_MCTRL_GPIO if GPIOLIB
> +	select MFD_AT91_USART
>  	help
>  	  This enables the driver for the on-chip UARTs of the Atmel
>  	  AT91 processors.
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index df46a9e88c34..6b4494352853 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -193,8 +193,8 @@ static struct console atmel_console;
>  
>  #if defined(CONFIG_OF)
>  static const struct of_device_id atmel_serial_dt_ids[] = {
> -	{ .compatible = "atmel,at91rm9200-usart" },
> -	{ .compatible = "atmel,at91sam9260-usart" },
> +	{ .compatible = "atmel,at91rm9200-usart-serial" },
> +	{ .compatible = "atmel,at91sam9260-usart-serial" },
>  	{ /* sentinel */ }
>  };
>  #endif
> @@ -1631,7 +1631,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
>  static void atmel_init_property(struct atmel_uart_port *atmel_port,
>  				struct platform_device *pdev)
>  {
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.parent->of_node;
>  
>  	/* DMA/PDC usage specification */
>  	if (of_property_read_bool(np, "atmel,use-dma-rx")) {
> @@ -2223,7 +2223,8 @@ static const char *atmel_type(struct uart_port *port)
>  static void atmel_release_port(struct uart_port *port)
>  {
>  	struct platform_device *pdev = to_platform_device(port->dev);
> -	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> +	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
> +		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
I think it may be simpler with something like:
+	struct platform_device *mfd_pdev = to_platform_device(port->dev->parent);
+	int size = mfd_pdev->resource[0].end - mfd_pdev->resource[0].start + 1;

>  
>  	release_mem_region(port->mapbase, size);
>  
> @@ -2239,7 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
>  static int atmel_request_port(struct uart_port *port)
>  {
>  	struct platform_device *pdev = to_platform_device(port->dev);
> -	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> +	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
> +		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
>  
ditto

>  	if (!request_mem_region(port->mapbase, size, "atmel_serial"))
>  		return -EBUSY;
> @@ -2345,23 +2347,23 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
Here, we could also add:
+	struct device *mfd_dev = pdev->dev.parent;
+	struct platform_device *mfd_pdev = to_platform_device(mfd_dev);

>  	atmel_init_property(atmel_port, pdev);
>  	atmel_set_ops(port);
>  
> -	uart_get_rs485_mode(&pdev->dev, &port->rs485);
> +	uart_get_rs485_mode(pdev->dev.parent, &port->rs485);
...and use them here

>  
>  	port->iotype		= UPIO_MEM;
>  	port->flags		= UPF_BOOT_AUTOCONF | UPF_IOREMAP;
>  	port->ops		= &atmel_pops;
>  	port->fifosize		= 1;
>  	port->dev		= &pdev->dev;
> -	port->mapbase	= pdev->resource[0].start;
> -	port->irq	= pdev->resource[1].start;
> +	port->mapbase		= to_platform_device(pdev->dev.parent)->resource[0].start;
> +	port->irq		= to_platform_device(pdev->dev.parent)->resource[1].start;
and here
I think it would be easier to read.

>  	port->rs485_config	= atmel_config_rs485;
> -	port->membase	= NULL;
> +	port->membase		= NULL;
>  
>  	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
>  
>  	/* for console, the clock could already be configured */
>  	if (!atmel_port->clk) {
> -		atmel_port->clk = clk_get(&pdev->dev, "usart");
> +		atmel_port->clk = clk_get(pdev->dev.parent, "usart");
and here

>  		if (IS_ERR(atmel_port->clk)) {
>  			ret = PTR_ERR(atmel_port->clk);
>  			atmel_port->clk = NULL;
> @@ -2656,7 +2658,7 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
>  	atmel_port->rts_low = 0;
>  	atmel_port->rts_high = 0;
>  
> -	if (of_property_read_u32(pdev->dev.of_node,
> +	if (of_property_read_u32(pdev->dev.parent->of_node,
>  				 "atmel,fifo-size",
>  				 &atmel_port->fifo_size))
>  		return;
> @@ -2694,11 +2696,10 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
>  static int atmel_serial_probe(struct platform_device *pdev)
>  {
>  	struct atmel_uart_port *atmel_port;
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.parent->of_node;
>  	void *data;
>  	int ret = -ENODEV;
>  	bool rs485_enabled;
> -
I think this line feed wasn't so bad.

>  	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
>  
>  	ret = of_alias_get_id(np, "serial");
> @@ -2845,7 +2846,7 @@ static struct platform_driver atmel_serial_driver = {
>  	.suspend	= atmel_serial_suspend,
>  	.resume		= atmel_serial_resume,
>  	.driver		= {
> -		.name			= "atmel_usart",
> +		.name			= "atmel_usart_serial",
>  		.of_match_table		= of_match_ptr(atmel_serial_dt_ids),
>  	},
>  };
> 

After your patch, the DMA is not selected anymore:
atmel_usart_serial atmel_usart_serial.0.auto: TX channel not available, switch to pio
instead of:
atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers

And the kernel doesn't log anymore on the serial console, despite the loglevel=8
(after reverting this series, the kernel logs reappears on the serial console)

(tests done on sam9g35)

regards,
Richard

^ permalink raw reply

* Difference between IOVA and bus address when SMMU is enabled
From: Russell King - ARM Linux @ 2018-05-14 10:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <c4fe00a3-33b1-7cd4-f613-768dee06ef6b@gmail.com>

On Sat, May 12, 2018 at 06:25:13PM +0530, valmiki wrote:
> Hi All,
> 
> What is the difference between IOVA address and bus address
> when SMMU is enabled ?
> 
> Is IOVA address term used only when hypervisor is present ?

IOVA = IO virtual address.  IOVA is the term normally used to describe
the address used on the _device_ side of an IOMMU.

For any general setup:

RAM ----- MMU ----- DEVICE
      ^         ^
  physical   virtual
  address    address

where "device" can be an IO device or a CPU, the terms still apply.

If you have something like this:

RAM ----- PCI bridge ----- MMU ----- DEVICE
      ^                ^         ^
   physical           bus     virtual
   address          address   address

You could also have (eg, in the case of a system MMU):

RAM ----- MMU ----- PCI bridge ----- DEVICE
      ^         ^                ^
   physical  virtual            bus
   address   address          address
           (this can also be
            considered a bus
            address!)

In both of the above two cases, the PCI bridge may perform some address
translation, meaning that the bus address is different from the address
seen on the other side of the bridge.

So, the terms used depend exactly on the overall bus topology.

In the case of a system MMU, where the system MMU sits between peripheral
devices and RAM, then the bus addresses are the same as the
_IOVA of the system MMU_.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply

* [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight
From: Adam Ford @ 2018-05-14 10:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <aba13450-1e8f-4aed-6248-16b71abcc0dd@ti.com>

On Mon, May 14, 2018 at 12:29 AM, Sekhar Nori <nsekhar@ti.com> wrote:
> Hi Adam,
>
> On Monday 14 May 2018 04:50 AM, Adam Ford wrote:
>> When using the board files the LCD works, but not with the DT.
>> This adds enables the original da850-evm to work with the same
>> LCD in device tree mode.
>>
>> The EVM has a gpio for the regulator and a gpio enable.  The LCD and
>> the vpif display pins are mutually exclusive, so if using the LCD,
>> do not load the vpif driver.
>
> Its not sufficient just note this in patch description.
>
> a) Disable (status = "disabled") the VPIF node which clashes for pins
> with LCD.
> b) Add a comment on top of the status = "disabled" giving information on
> how user can enable it (disable lcdc node and then change to status =
> "okay").
>
>>
>> Signed-off-by: Adam Ford <aford173@gmail.com>
>> ---
>> V3:  Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to
>>      backlight which better matches the schematic.  Updated the description to explain
>>      that it cannot be used at the same time as the vpif driver.
>>
>> V2:  Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO
>>
>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
>> index 2e817da37fdb..3f1c8be07efe 100644
>> --- a/arch/arm/boot/dts/da850-evm.dts
>> +++ b/arch/arm/boot/dts/da850-evm.dts
>> @@ -27,6 +27,50 @@
>>               spi0 = &spi1;
>>       };
>>
>> +     backlight {
>> +             compatible = "gpio-backlight";
>> +             enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */
>
> The gpio-backlight binding does not describe a property called
> enable-gpios. It should just be gpios.

I will fix that.

>
> a) Are you using gpio-backlight because you are not able to get the PWM
> to work?
>
Yes,  You told me not to worry about doing a PWM backlight because the
legacy board does not PWM either.

> b) What is GP0[7] connected to in the schematic you have? In the
> schematic I have I see LCD_PWM0 is connected to
> SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12.

I have schematic 1016572 dated Wednesday, August 18, 2010.  According
to it, AXR15 / EPWMN0_TZ[0] / ECAP2_APWM2 / GPIO0[7] connects to U25,
Pin 46 to generate M_LCD_PWM0.  You might have one of the early,
pre-release versions.

>
> c) The /* GP0[7] */ comment is not really useful on its own as it can be
> computed. What I wanted to see is the schematic symbol like "LCD_PWM0".
> Same for other places like this below.

I can do that.
>
>> @@ -35,6 +79,16 @@
>>               regulator-boot-on;
>>       };
>>
>> +     backlight_reg: backlight-regulator {
>> +             compatible = "regulator-fixed";
>> +             regulator-name = "lcd_backlight_pwr";
>> +             regulator-min-microvolt = <3300000>;
>> +             regulator-max-microvolt = <3300000>;
>> +             gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */
>> +             regulator-always-on;
>
> Why should this regulator never be disabled?

The gpio-backlight does not have a way that I can see to associate the
regulator to it.  I read through the bindings, but I didn't see an
option to associate a regulator it.  I use this regulator to drive
lcd_backlight_pwr and the backlight driver to write lcd_pwm0.  Without
this option, the system disables lcd_backlight_pwr and the screen is
blank


adam
>
> Thanks,
> Sekhar

^ permalink raw reply

* [PATCH V3] ARM: dts: da850-evm: add WP and CD to MMC
From: Sekhar Nori @ 2018-05-14 10:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180512122358.17459-1-aford173@gmail.com>

On Saturday 12 May 2018 05:53 PM, Adam Ford wrote:
> The legacy mode uses the write protect and card detect but DT does not.
> This patch enables Write-Protect and Card-Detect pins for the MMC card, and
> the gpio-ranges property sets the gpio pinmuxing for those respective pins.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>

Applied to v4.18/dt.

Thanks,
Sekhar

^ permalink raw reply

* Difference between IOVA and bus address when SMMU is enabled
From: Jean-Philippe Brucker @ 2018-05-14 10:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <c4fe00a3-33b1-7cd4-f613-768dee06ef6b@gmail.com>

Hi Valmiki,

On 12/05/18 13:55, valmiki wrote:
> Hi All,
> 
> What is the difference between IOVA address and bus address
> when SMMU is enabled ?

They are the same. You'll use one term or the other depending on what
system component you're talking about. "IOVA" only means something when
talking about IOMMUs, where it represents the input address. If you're
discussing bus transactions without caring whether an SMMU is enabled or
not, then "bus address" makes more sense.

We distinguish "IOVA" from "VA", which represents the input address of
the CPU's MMU (e.g. any userspace pointer). The distinction is useful
because the SMMU page tables are usually separate from the CPU page
tables. In this case if you want to share a buffer between application
and device, you'll have to allocate and map both a VA on the CPU side,
and an IOVA on the device side. When sharing MMU page tables with the
SMMU (see the SVA work), then we tend to talk about VA instead of IOVA,
because they are identical.

> Is IOVA address term used only when hypervisor is present ?

No, the term is used in bare-metal setups as well.

Thanks,
Jean

^ permalink raw reply

* [PATCH v2 4/4] arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and Makefile
From: Marc Zyngier @ 2018-05-14 10:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526293351-32794-5-git-send-email-erin.lo@mediatek.com>

On 14/05/18 11:22, Erin Lo wrote:
> From: Ben Ho <Ben.Ho@mediatek.com>
> 
> Add basic chip support for Mediatek 8183
> 
> Signed-off-by: Ben Ho <Ben.Ho@mediatek.com>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/Makefile       |   1 +
>  arch/arm64/boot/dts/mediatek/mt8183-evb.dts |  31 +++++
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi    | 178 ++++++++++++++++++++++++++++
>  3 files changed, 210 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/mediatek/mt8183-evb.dts
>  create mode 100644 arch/arm64/boot/dts/mediatek/mt8183.dtsi
> 

[...]

> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> new file mode 100644
> index 0000000..8564a26
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi

[...]

> +	gic: interrupt-controller at 0c000000 {
> +		compatible = "arm,gic-v3";
> +		#interrupt-cells = <3>;
> +		interrupt-parent = <&gic>;
> +		interrupt-controller;
> +		reg = <0 0x0c000000 0 0x40000>,  // CID
> +		      <0 0x0c100000 0 0x200000>; // CIR

You're missing the GICV and GICH regions that are present on both A53
and A73 at an offset from PERIPHBASE.

> +		interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> +	};

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH v3 6/8] arm64: module-plts: Extend veneer to address 52-bit VAs
From: Ard Biesheuvel @ 2018-05-14 10:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180511101104.6cv5mvqc6juh2mgt@capper-debian.cambridge.arm.com>

On 11 May 2018 at 12:11, Steve Capper <steve.capper@arm.com> wrote:
> On Fri, May 11, 2018 at 12:01:05AM +0200, Ard Biesheuvel wrote:
>> On 10 May 2018 at 18:23, Steve Capper <steve.capper@arm.com> wrote:
>> > From: Ard Bieusheuval <ard.biesheuvel@linaro.org>
>> >
>> > In preparation for 52-bit VA support in the Linux kernel, we extend the
>> > plts veneer to support 52-bit addresses via an extra movk instruction.
>> >
>> > [Steve: code from Ard off-list, changed the #ifdef logic to inequality]
>> > Signed-off-by: Steve Capper <steve.capper@arm.com>
>> >
>> > ---
>> >
>> > New in V3 of the series.
>> >
>> > I'm not sure if this is strictly necessary as the VAs of the module
>> > space will fit within 48-bits of addressing even when a 52-bit VA space
>> > is enabled.
>>
>> What about the kernel text itself? Is that also guaranteed to have
>> bits [51:48] of its VAs equal 0xf, even under randomization?
>>
>> If so, I agree we don't need the patch.
>>
>
> Hi Ard,
> The kernel modules and text are guaranteed to have addresses greater
> than or equal to KASAN_SHADOW_END (same for both 48, 52-bit VAs) or
> _VA_START(VA_BITS_MIN) (same for both 48, 52-bit VAs). Also, IIUC, the
> KASLR displacemnt is always non-negative?
>

Correct.

> So I think we're safe in that modules and kernel text will be 48-bit
> addressable in 52-bit configurations.
>
> I'll have a think about a BUILD_BUG to capture any change to the above.
>

Yes please

^ permalink raw reply

* [PATCH 09/21] arm64: dts: allwinner: a64: Add HDMI support
From: Jagan Teki @ 2018-05-14 10:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180514084050.ix5corfuvx33gsyn@flea>

On Mon, May 14, 2018 at 2:10 PM, Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
> On Mon, May 14, 2018 at 02:03:36PM +0530, Jagan Teki wrote:
>> On Wed, May 2, 2018 at 5:04 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>> > Hi,
>> >
>> > On Mon, Apr 30, 2018 at 05:10:46PM +0530, Jagan Teki wrote:
>> >> +             hdmi_phy: hdmi-phy at 1ef0000 {
>> >> +                     compatible = "allwinner,sun50i-a64-hdmi-phy",
>> >> +                                  "allwinner,sun8i-h3-hdmi-phy";
>> >> +                     reg = <0x01ef0000 0x10000>;
>> >> +                     clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI_DDC>,
>> >> +                              <&ccu CLK_PLL_VIDEO1>;
>> >
>> > You were discussing that the PLL0 could also be used to clock the PHY,
>> > has that been figured out?
>>
>> This is what I understand from Fig: 3-3. Module Clock Diagram, both
>> tcon0 and tcon1 are using HDMI. I'm thinking based on the tcon
>> configuration we need use proper PLL or some logic to get common PLL
>> don't know yet. Since this series adding tcon1 I've attached PLL1.
>
> You're not describing the TCON node here though, but the HDMI one, and
> the HDMI block is listed in both the PLL video 0 and 1.

So how can we attach particular PLL with particular HDMI(PLL0 to HDMI0
and so-on) or do we need to attached both the PLL's any suggestion?

^ permalink raw reply

* [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input
From: Niklas Söderlund @ 2018-05-14 10:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180514073520.GA5956@w540>

Hi Jacopo,

On 2018-05-14 09:39:34 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Sun, May 13, 2018 at 02:57:55PM +0200, Niklas S?derlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your patch.
> >
> > On 2018-05-11 12:00:02 +0200, Jacopo Mondi wrote:
> > > Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> > > development board.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 ++++++++++++++++++++++++++
> > >  1 file changed, 68 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > > index d03f194..e0ce462 100644
> > > --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > > +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > > @@ -59,6 +59,17 @@
> > >  		};
> > >  	};
> > >
> > > +	hdmi-in {
> > > +		compatible = "hdmi-connector";
> > > +		type = "a";
> > > +
> > > +		port {
> > > +			hdmi_con_in: endpoint {
> > > +				remote-endpoint = <&adv7612_in>;
> > > +			};
> > > +		};
> > > +	};
> > > +
> > >  	memory at 48000000 {
> > >  		device_type = "memory";
> > >  		/* first 128MB is reserved for secure area. */
> > > @@ -142,6 +153,11 @@
> > >  		groups = "usb0";
> > >  		function = "usb0";
> > >  	};
> > > +
> > > +	vin4_pins: vin4 {
> > > +		groups = "vin4_data24", "vin4_sync", "vin4_clk", "vin4_clkenb";
> > > +		function = "vin4";
> > > +	};
> > >  };
> > >
> > >  &i2c0 {
> > > @@ -154,6 +170,35 @@
> > >  		reg = <0x50>;
> > >  		pagesize = <8>;
> > >  	};
> > > +
> > > +	hdmi-decoder at 4c {
> > > +		compatible = "adi,adv7612";
> > > +		reg = <0x4c>;
> > > +		default-input = <0>;
> > > +
> > > +		ports {
> > > +			#address-cells = <1>;
> > > +			#size-cells = <0>;
> > > +
> > > +			port at 0 {
> > > +				reg = <0>;
> > > +				adv7612_in: endpoint {
> > > +					remote-endpoint = <&hdmi_con_in>;
> > > +				};
> > > +			};
> > > +
> > > +			port at 2 {
> > > +				reg = <2>;
> > > +				adv7612_out: endpoint {
> > > +					pclk-sample = <0>;
> > > +					hsync-active = <0>;
> > > +					vsync-active = <0>;
> >
> > This differs from the Gen2 DT bindings which is a very similar hardware
> > setup using the same components. Defining these properties will make the
> > bus marked as V4L2_MBUS_PARALLEL instead of V4L2_MBUS_BT656.
> 
> And that's what we want....
> 
> >
> > This will change how the hardware is configured for capture if the media
> > bus is in a UYVY format, see VNMC_INF register in rvin_setup(). Maybe
> > this it not an issue here but still I'm curious to why this differ
> > between Gen2 and Gen3 :-)
> 
> Actually this won't impact the VIN configuration as this is the
> 'remote endpoint' from VIN perspective and the properties used to
> configure the interface are the ones in the 'local endpoint'.

You are right, sorry for the confusion and thanks for educating me :-)

> 
> >
> > > +
> > > +					remote-endpoint = <&vin4_in>;
> > > +				};
> > > +			};
> > > +		};
> > > +	};
> > >  };
> > >
> > >  &i2c1 {
> > > @@ -246,3 +291,26 @@
> > >  	timeout-sec = <60>;
> > >  	status = "okay";
> > >  };
> > > +
> > > +&vin4 {
> > > +	pinctrl-0 = <&vin4_pins>;
> > > +	pinctrl-names = "default";
> > > +
> > > +	status = "okay";
> > > +
> > > +	ports {
> > > +		#address-cells = <1>;
> > > +		#size-cells = <0>;
> > > +
> > > +		port at 0 {
> > > +			reg = <0>;
> > > +
> > > +			vin4_in: endpoint {
> > > +				hsync-active = <0>;
> > > +				vsync-active = <0>;
> >
> > Comparing this to the Gen2 bindings some properties are missing,
> >
> > bus-width = <24>;
> > pclk-sample = <1>;
> > data-active = <1>;
> 
> The VIN driver does not parse them, so there is no value in having
> them there, if not confusing people as it happened to me reading the
> Gen2 DT.

I have no objection removing them. Trying to understand why the 
description differed from Gen2.

> 
> >
> > This is not a big deal as the VIN driver don't use these properties so
> > no functional change should come of this but still a difference.
> 
> Exactly.
> 
> On a side note. I have not seen a way to configure the pixel clock
> sampling level in the interface datasheet. The register used to
> configure synchronism signals polarities is VnDMR2, and there I read
> we can configure HSYNC/VSYNC and CLOCKENB (which is data enable, not
> pixel clock) polarities. Is it configured through some other
> register?

I have not seen such a register no.

> >
> > Over all I'm happy with this change but before I add my tag I would like
> > to understand why it differs from the Gen2 configuration for the adv7612
> > properties.
> >
> > Also on a side not it is possible with hardware switches on the board
> > switch the VIN4 source to a completely different pipeline CVBS connector
> > -> adv7180 -> VIN4. But I think it's best we keep the HDMI as default as
> > this seems to be how the boards are shipped. But maybe mentioning this
> > in the commit message would not hurt if you end-up resending the patch.
> 
> Oh I see. SW-49 to SW-52 enables the HDMI input, SW53-SW54 CVBS one.
> And actually, reading the 'initial setting of slide switches' in the
> Draak board manual, it turns out that the board default configuration
> is with CVBS input selected... What should we do here? reflect
> defaults in the DT, or prioritize HDMI?

I feel this is a question for Laurent. My feeling for how we handled 
this in other cases is to go with the board default settings. I'm 
however sure there are exceptions to the rule. So maybe we should go 
with the most useful (what ever that is) configuration?

> 
> Thanks
>    j
> 
> >
> > > +
> > > +				remote-endpoint = <&adv7612_out>;
> > > +			};
> > > +		};
> > > +	};
> > > +};
> > > --
> > > 2.7.4
> > >
> >
> > --
> > Regards,
> > Niklas S?derlund



-- 
Regards,
Niklas S?derlund

^ permalink raw reply

* [PATCH v2 4/4] arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and Makefile
From: Erin Lo @ 2018-05-14 10:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526293351-32794-1-git-send-email-erin.lo@mediatek.com>

From: Ben Ho <Ben.Ho@mediatek.com>

Add basic chip support for Mediatek 8183

Signed-off-by: Ben Ho <Ben.Ho@mediatek.com>
Signed-off-by: Erin Lo <erin.lo@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/Makefile       |   1 +
 arch/arm64/boot/dts/mediatek/mt8183-evb.dts |  31 +++++
 arch/arm64/boot/dts/mediatek/mt8183.dtsi    | 178 ++++++++++++++++++++++++++++
 3 files changed, 210 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt8183-evb.dts
 create mode 100644 arch/arm64/boot/dts/mediatek/mt8183.dtsi

diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index ac17f60..2836261 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -5,3 +5,4 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-evb.dtb
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt8183-evb.dtb
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
new file mode 100644
index 0000000..9a3d6b7
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (c) 2017 MediaTek Inc.
+ * Author: Ben Ho <ben.ho@mediatek.com>
+ *	   Erin Lo <erin.lo@mediatek.com>
+ */
+
+/dts-v1/;
+#include "mt8183.dtsi"
+
+/ {
+	model = "MediaTek MT8183 evaluation board";
+	compatible = "mediatek,mt8183-evb", "mediatek,mt8183";
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	memory at 40000000 {
+		device_type = "memory";
+		reg = <0 0x40000000 0 0x80000000>;
+	};
+
+	chosen {
+		stdout-path = "serial0:921600n8";
+	};
+};
+
+&uart0 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
new file mode 100644
index 0000000..8564a26
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (c) 2017 MediaTek Inc.
+ * Author: Ben Ho <ben.ho@mediatek.com>
+ *	   Erin Lo <erin.lo@mediatek.com>
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+	compatible = "mediatek,mt8183";
+	interrupt-parent = <&sysirq>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&cpu0>;
+				};
+				core1 {
+					cpu = <&cpu1>;
+				};
+				core2 {
+					cpu = <&cpu2>;
+				};
+				core3 {
+					cpu = <&cpu3>;
+				};
+			};
+
+			cluster1 {
+				core0 {
+					cpu = <&cpu4>;
+				};
+				core1 {
+					cpu = <&cpu5>;
+				};
+				core2 {
+					cpu = <&cpu6>;
+				};
+				core3 {
+					cpu = <&cpu7>;
+				};
+			};
+		};
+
+		cpu0: cpu at 000 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x000>;
+			enable-method = "psci";
+		};
+
+		cpu1: cpu at 001 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x001>;
+			enable-method = "psci";
+		};
+
+		cpu2: cpu at 002 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x002>;
+			enable-method = "psci";
+		};
+
+		cpu3: cpu at 003 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x003>;
+			enable-method = "psci";
+		};
+
+		cpu4: cpu at 100 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a73";
+			reg = <0x100>;
+			enable-method = "psci";
+		};
+
+		cpu5: cpu at 101 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a73";
+			reg = <0x101>;
+			enable-method = "psci";
+		};
+
+		cpu6: cpu at 102 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a73";
+			reg = <0x102>;
+			enable-method = "psci";
+		};
+
+		cpu7: cpu at 103 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a73";
+			reg = <0x103>;
+			enable-method = "psci";
+		};
+	};
+
+	psci {
+		compatible      = "arm,psci-1.0";
+		method          = "smc";
+	};
+
+	uart_clk: dummy26m {
+		compatible = "fixed-clock";
+		clock-frequency = <26000000>;
+		#clock-cells = <0>;
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupt-parent = <&gic>;
+		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+	};
+
+	gic: interrupt-controller at 0c000000 {
+		compatible = "arm,gic-v3";
+		#interrupt-cells = <3>;
+		interrupt-parent = <&gic>;
+		interrupt-controller;
+		reg = <0 0x0c000000 0 0x40000>,  // CID
+		      <0 0x0c100000 0 0x200000>; // CIR
+		interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+	sysirq: intpol-controller at 0c530a80 {
+		compatible = "mediatek,mt8183-sysirq",
+			     "mediatek,mt6577-sysirq";
+		interrupt-controller;
+		#interrupt-cells = <3>;
+		interrupt-parent = <&gic>;
+		reg = <0 0x0c530a80 0 0x50>;
+	};
+
+	uart0: serial at 11002000 {
+		compatible = "mediatek,mt8183-uart",
+			     "mediatek,mt6577-uart";
+		reg = <0 0x11002000 0 0x1000>;
+		interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&uart_clk>, <&uart_clk>;
+		clock-names = "baud", "bus";
+		status = "disabled";
+	};
+
+	uart1: serial at 11003000 {
+		compatible = "mediatek,mt8183-uart",
+			     "mediatek,mt6577-uart";
+		reg = <0 0x11003000 0 0x1000>;
+		interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&uart_clk>, <&uart_clk>;
+		clock-names = "baud", "bus";
+		status = "disabled";
+	};
+
+	uart2: serial at 11004000 {
+		compatible = "mediatek,mt8183-uart",
+			     "mediatek,mt6577-uart";
+		reg = <0 0x11004000 0 0x1000>;
+		interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&uart_clk>, <&uart_clk>;
+		clock-names = "baud", "bus";
+		status = "disabled";
+	};
+};
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 3/4] dt-bindings: serial: Add compatible for Mediatek MT8183
From: Erin Lo @ 2018-05-14 10:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526293351-32794-1-git-send-email-erin.lo@mediatek.com>

This adds dt-binding documentation of uart for Mediatek MT8183 SoC
Platform.

Signed-off-by: Erin Lo <erin.lo@mediatek.com>
---
 Documentation/devicetree/bindings/serial/mtk-uart.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
index f73abff..4783336 100644
--- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
+++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
@@ -15,6 +15,7 @@ Required properties:
   * "mediatek,mt8127-uart" for MT8127 compatible UARTS
   * "mediatek,mt8135-uart" for MT8135 compatible UARTS
   * "mediatek,mt8173-uart" for MT8173 compatible UARTS
+  * "mediatek,mt8183-uart", "mediatek,mt6577-uart" for MT8183 compatible UARTS
   * "mediatek,mt6577-uart" for MT6577 and all of the above
 
 - reg: The base address of the UART register bank.
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 2/4] dt-bindings: mtk-sysirq: Add compatible for Mediatek MT8183
From: Erin Lo @ 2018-05-14 10:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526293351-32794-1-git-send-email-erin.lo@mediatek.com>

This adds dt-binding documentation of SYSIRQ for Mediatek MT8183 SoC
Platform.

Signed-off-by: Erin Lo <erin.lo@mediatek.com>
---
 .../devicetree/bindings/interrupt-controller/mediatek,sysirq.txt         | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt b/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
index 07bf0b9..5ff48a8 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
@@ -5,6 +5,7 @@ interrupt.
 
 Required properties:
 - compatible: should be
+	"mediatek,mt8183-sysirq", "mediatek,mt6577-sysirq": for MT8183
 	"mediatek,mt8173-sysirq", "mediatek,mt6577-sysirq": for MT8173
 	"mediatek,mt8135-sysirq", "mediatek,mt6577-sysirq": for MT8135
 	"mediatek,mt8127-sysirq", "mediatek,mt6577-sysirq": for MT8127
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 1/4] dt-bindings: arm: Add bindings for Mediatek MT8183 SoC Platform
From: Erin Lo @ 2018-05-14 10:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526293351-32794-1-git-send-email-erin.lo@mediatek.com>

This adds dt-binding documentation of cpu for Mediatek MT8183.

Signed-off-by: Erin Lo <erin.lo@mediatek.com>
---
 Documentation/devicetree/bindings/arm/mediatek.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek.txt b/Documentation/devicetree/bindings/arm/mediatek.txt
index 7d21ab3..2754535 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.txt
+++ b/Documentation/devicetree/bindings/arm/mediatek.txt
@@ -19,6 +19,7 @@ compatible: Must contain one of
    "mediatek,mt8127"
    "mediatek,mt8135"
    "mediatek,mt8173"
+   "mediatek,mt8183"
 
 
 Supported boards:
@@ -73,3 +74,6 @@ Supported boards:
 - MTK mt8173 tablet EVB:
     Required root node properties:
       - compatible = "mediatek,mt8173-evb", "mediatek,mt8173";
+- Evaluation board for MT8183:
+    Required root node properties:
+      - compatible = "mediatek,mt8183-evb", "mediatek,mt8183";
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 0/4] Add basic support for Mediatek MT8183 SoC
From: Erin Lo @ 2018-05-14 10:22 UTC (permalink / raw)
  To: linux-arm-kernel

MT8183 is a SoC based on 64bit ARMv8 architecture.
It contains 4 CA53 and 4 CA73 cores.
MT8183 share many HW IP with MT65xx series.
This patchset was tested on MT8183 evaluation board, and boot to shell ok.

This series contains document bindings, device tree including interrupt, uart.

Change in v2:
1. Split dt-bindings into different patches
2. Correct bindings for supported SoCs (mtk-uart.txt)

Ben Ho (1):
  arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and
    Makefile

Erin Lo (3):
  dt-bindings: arm: Add bindings for Mediatek MT8183 SoC Platform
  dt-bindings: mtk-sysirq: Add compatible for Mediatek MT8183
  dt-bindings: serial: Add compatible for Mediatek MT8183

 Documentation/devicetree/bindings/arm/mediatek.txt |   4 +
 .../interrupt-controller/mediatek,sysirq.txt       |   1 +
 .../devicetree/bindings/serial/mtk-uart.txt        |   1 +
 arch/arm64/boot/dts/mediatek/Makefile              |   1 +
 arch/arm64/boot/dts/mediatek/mt8183-evb.dts        |  31 ++++
 arch/arm64/boot/dts/mediatek/mt8183.dtsi           | 178 +++++++++++++++++++++
 6 files changed, 216 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt8183-evb.dts
 create mode 100644 arch/arm64/boot/dts/mediatek/mt8183.dtsi

--
1.9.1

^ permalink raw reply

* [FAIL bisect] Sound card probe error
From: Krzysztof Kozlowski @ 2018-05-14 10:17 UTC (permalink / raw)
  To: linux-arm-kernel

Hi All,

I experienced an issue on today's next 20180514. Found on Odroid XU3
board (Exynos5422, ARMv7, octa-core) with exynos_defconfig.

1. No sound card is probed:
[    6.717698] ALSA device list:
[    6.719303]   No soundcards found.
(expected: #0: Odroid-XU3)


2. Oops happens:
[  OK  ] Started udev Kernel Device Manager.
Unable to handle kernel paging request at virtual address c0f324ac
[   36.229430] [<c05764ac>] (platform_drv_probe) from [<c0573fd8>]
(driver_probe_device+0x2e0/0x4b4)
[   36.238258] [<c0573fd8>] (driver_probe_device) from [<c0571fb8>]
(bus_for_each_drv+0x74/0xb8)
[   36.246746] [<c0571fb8>] (bus_for_each_drv) from [<c0573bb0>]
(__device_attach+0xd4/0x168)
[   36.254974] [<c0573bb0>] (__device_attach) from [<c0572e78>]
(bus_probe_device+0x88/0x90)
[   36.263121] [<c0572e78>] (bus_probe_device) from [<c0573434>]
(deferred_probe_work_func+0x64/0x184)
[   36.272144] [<c0573434>] (deferred_probe_work_func) from
[<c0146950>] (process_one_work+0x220/0x8e8)
[   36.281236] [<c0146950>] (process_one_work) from [<c0147050>]
(worker_thread+0x38/0x584)
[   36.289293] [<c0147050>] (worker_thread) from [<c014d96c>]
(kthread+0x138/0x168)
[   36.296652] [<c014d96c>] (kthread) from [<c01010b4>]
(ret_from_fork+0x14/0x20)


3. No DRM device is probed/found.

I could not find any specific reasons of this failure (like earlier
failures... except some probe deferrals which are happening almost
alwats).

Full kernel log of issue:
http://www.krzk.eu/#/builders/1/builds/1978/steps/10/logs/serial0
Example of good boot:
http://www.krzk.eu/#/builders/1/builds/1953/steps/10/logs/serial0
\
Bisected to:
8c123c14bbba4add148536b6d47a9226deda2f7a is the first bad commit
commit 8c123c14bbba4add148536b6d47a9226deda2f7a
Author: Ulf Hansson <ulf.hansson@linaro.org>
Date:   Thu Apr 26 10:53:06 2018 +0200

    driver core: Respect all error codes from dev_pm_domain_attach()

    The limitation of being able to check only for -EPROBE_DEFER from
    dev_pm_domain_attach() has been removed. Hence let's respect all error
    codes and bail out accordingly.

    Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
    Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

:040000 040000 26c27c583ac4557a3982b4a6bf6b3b4ff8386449
997a872ddca1c876024cf49c3e7fc68d15cd8fed M drivers


bisect log:
git bisect start
# good: [66e1c94db3cd4e094de66a6be68c3ab6d17e0c52] Merge branch
'x86-pti-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good 66e1c94db3cd4e094de66a6be68c3ab6d17e0c52
# bad: [af37b24ae6b3e7eac272417c9af591c58baa8f93] Add linux-next
specific files for 20180514
git bisect bad af37b24ae6b3e7eac272417c9af591c58baa8f93
# bad: [0e9cda8f772f6c66693629b383d33a63a8333b48] Merge
remote-tracking branch 'bluetooth/master'
git bisect bad 0e9cda8f772f6c66693629b383d33a63a8333b48
# good: [a458037d65ab42098252b52889543fb836226d33] Merge
remote-tracking branch 'jc_docs/docs-next'
git bisect good a458037d65ab42098252b52889543fb836226d33
# good: [a37fb855f6e8e9eafac046721393f68f48eb5f91] net: dsa: fix
added_by_user switchdev notification
git bisect good a37fb855f6e8e9eafac046721393f68f48eb5f91
# bad: [3ee48ebb5dfef1a167b15ab9a4168134837b6e7c] Merge
remote-tracking branch 'pm/linux-next'
git bisect bad 3ee48ebb5dfef1a167b15ab9a4168134837b6e7c
# good: [89eaaf2d19a6cfff0a16014405c2a65b5257b8a1] media: lgdt330x:
don't use an uninitialized state
git bisect good 89eaaf2d19a6cfff0a16014405c2a65b5257b8a1
# good: [89eaaf2d19a6cfff0a16014405c2a65b5257b8a1] media: lgdt330x:
don't use an uninitialized state
git bisect good 89eaaf2d19a6cfff0a16014405c2a65b5257b8a1
# good: [6d2eaece20ffe783cdbcedd7ef2d6ae51d846d33] media: xilinx:
regroup caps on querycap
git bisect good 6d2eaece20ffe783cdbcedd7ef2d6ae51d846d33
# bad: [91965db07c20058e207e8ea14a47fe0ec891896d] Merge branch
'pm-core' into linux-next
git bisect bad 91965db07c20058e207e8ea14a47fe0ec891896d
# bad: [e298050322459ccd8aaa0f7edd422f630ce4e9b4] Merge branch
'pm-domains' into linux-next
git bisect bad e298050322459ccd8aaa0f7edd422f630ce4e9b4
# good: [097b29f3e6f5ec6781e5e3048e4a285edd79c357] Merge branch
'pm-cpufreq-sched' into linux-next
git bisect good 097b29f3e6f5ec6781e5e3048e4a285edd79c357
# good: [4d73e6885c07847488201e8df4da9ed56c25e21b] amba: Respect all
error codes from dev_pm_domain_attach()
git bisect good 4d73e6885c07847488201e8df4da9ed56c25e21b
# bad: [ecbc0460e7f16aca23f6dd34527fbf751eaf1453] mmc: sdio: Respect
all error codes from dev_pm_domain_attach()
git bisect bad ecbc0460e7f16aca23f6dd34527fbf751eaf1453
# bad: [f24b7891ddbf4f1d509253d6e0bcfdcfbdadf4fd] i2c: Respect all
error codes from dev_pm_domain_attach()
git bisect bad f24b7891ddbf4f1d509253d6e0bcfdcfbdadf4fd
# bad: [8c123c14bbba4add148536b6d47a9226deda2f7a] driver core: Respect
all error codes from dev_pm_domain_attach()
git bisect bad 8c123c14bbba4add148536b6d47a9226deda2f7a
# first bad commit: [8c123c14bbba4add148536b6d47a9226deda2f7a] driver
core: Respect all error codes from dev_pm_domain_attach()

Let me know if you need any more data.

Best regards,
Krzysztof

^ permalink raw reply

* [PATCH v6 3/6] kernel/reboot.c: export pm_power_off_prepare
From: Oleksij Rempel @ 2018-05-14 10:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <091ccf85-ae4c-f6fd-0455-1a70e973c85c@pengutronix.de>



On 14.05.2018 07:10, Oleksij Rempel wrote:
> 
> 
> On 14.05.2018 06:33, Oleksij Rempel wrote:
>>
>>
>> On 12.05.2018 13:13, Rafael J. Wysocki wrote:
>>> On Friday, May 4, 2018 8:50:52 PM CEST Oleksij Rempel wrote:
>>>> Hallo Andrew,
>>>> I need your ACK or NACK for this patch.
>>>>
>>>> This function is used to configure external PMIC to interpret
>>>> signal which will be triggered by pm_power_off as power off.
>>>> Since same signal can be used for stand by, I linked PMIC configuration
>>>> with pm_power_off_prepare to avoid possible conflicts.
>>>>
>>>> On Mon, Mar 05, 2018 at 11:25:20AM +0100, Oleksij Rempel wrote:
>>>>> Export pm_power_off_prepare. It is needed to implement power off on
>>>>> Freescale/NXP iMX6 based boards with external power management
>>>>> integrated circuit (PMIC).
>>>>>
>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>>> ---
>>>>>  kernel/reboot.c | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/kernel/reboot.c b/kernel/reboot.c
>>>>> index e4ced883d8de..350be6baa60d 100644
>>>>> --- a/kernel/reboot.c
>>>>> +++ b/kernel/reboot.c
>>>>> @@ -49,6 +49,7 @@ int reboot_force;
>>>>>   */
>>>>>  
>>>>>  void (*pm_power_off_prepare)(void);
>>>>> +EXPORT_SYMBOL(pm_power_off_prepare);
>>>
>>> Why not EXPORT_SYMBOL_GPL() ?
>>
>> No special reason. Fixed.
>> Any other comments?
>>
> 
> Or with other words, will it be enough to get your Signed-of-by for this
> patch?

Hi again,

i was punished by my colleagues and actually by Russel for asking
"Signed-of-by". So, I correct my self, is it enough for a Reviewed-by?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180514/61f11d48/attachment.sig>

^ permalink raw reply

* [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input
From: Niklas Söderlund @ 2018-05-14 10:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180514094900.GF30519@bigcity.dyn.berto.se>

Hi again,

On 2018-05-14 11:49:00 +0200, Niklas S?derlund wrote:
> Hi Laurent,
> 
> On 2018-05-14 05:49:41 +0300, Laurent Pinchart wrote:
> 
> [snip]
> 
> > > > +&vin4 {
> > > > +	pinctrl-0 = <&vin4_pins>;
> > > > +	pinctrl-names = "default";
> > > > +
> > > > +	status = "okay";
> > > > +
> > > > +	ports {
> > > > +		#address-cells = <1>;
> > > > +		#size-cells = <0>;
> > > > +
> > > > +		port at 0 {
> > > > +			reg = <0>;
> > > > +
> > > > +			vin4_in: endpoint {
> > > > +				hsync-active = <0>;
> > > > +				vsync-active = <0>;
> > > 
> > > Comparing this to the Gen2 bindings some properties are missing,
> > > 
> > > bus-width = <24>;
> > > pclk-sample = <1>;
> > > data-active = <1>;
> > > 
> > > This is not a big deal as the VIN driver don't use these properties so
> > > no functional change should come of this but still a difference.
> > 
> > I think the VIN DT bindings should be updated to explicitly list the endpoint 
> > properties that are mandatory, optional, or not allowed.
> 
> I think it's documented as it reference video-interfaces.txt which lists 
> all these properties as optional. And in deed they are all optional.  If 
> the VIN driver makes use of all the optional ones is another matter. How 
> do we know that the remote subdevice is not looking at its remote 
> endpoint for bus parameters not considered by the rcar-vin driver?
> 
> The thing is that the rcar-vin driver only looks at the remote endpoint 
> for these properties and ignores the on its local endpoint. Maybe some 
> v4l2 framework change is needed here to make sure the bus properties are 
> the same on both endpoints of a link. But I fear such a change would 
> break a lot of stuff.

Jacopo pointed out this statement is untrue. The rcar-vin only looks at 
it's local endpoint not the remote endpoint for it's bus parameters. The 
callback provided to v4l2_async_notifier_parse_fwnode_endpoints() 
confused me as the subdevice passed to it is the one describe the remote 
endpoint while the v4l2_fwnode_endpoint argument is that of the local 
endpoint. Sorry for the confusion and thanks Jacopo for correcting me.

-- 
Regards,
Niklas S?derlund

^ permalink raw reply

* [PATCH 02/18] arm64: move SCTLR_EL{1,2} assertions to <asm/sysreg.h>
From: Mark Rutland @ 2018-05-14 10:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180514100053.GX7753@e103592.cambridge.arm.com>

On Mon, May 14, 2018 at 11:00:53AM +0100, Dave Martin wrote:
> On Mon, May 14, 2018 at 10:46:24AM +0100, Mark Rutland wrote:
> > -/* Check all the bits are accounted for */
> > -#define SCTLR_EL2_BUILD_BUG_ON_MISSING_BITS	BUILD_BUG_ON((SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != ~0)
> > -
> > +#if (SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != 0xffffffff
> > +#error "Inconsistent SCTLR_EL2 set/clear bits"
> > +#endif
> 
> Can we have a comment on the != 0xffffffff versus != ~0 here?
> 
> The subtle differences in evaluation semantics between #if and
> other contexts here may well trip people up during maintenance...

Do you have any suggestion as to the wording?

I'm happy to add a comment, but I don't really know what to say.

Thanks,
Mark.

^ permalink raw reply

* [PATCH 05/18] arm64: kill change_cpacr()
From: Dave Martin @ 2018-05-14 10:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180514094640.27569-6-mark.rutland@arm.com>

On Mon, May 14, 2018 at 10:46:27AM +0100, Mark Rutland wrote:
> Now that we have sysreg_clear_set(), we can use this instead of
> change_cpacr().
> 
> Note that the order of the set and clear arguments differs between
> change_cpacr() and sysreg_clear_set(), so these are flipped as part of
> the conversion. Also, sve_user_enable() redundantly clears
> CPACR_EL1_ZEN_EL0EN before setting it; this is removed for clarity.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Dave Martin <dave.martin@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>

Looks right to me.

Reviewed-by: Dave Martin <Dave.Martin@arm.com>

> ---
>  arch/arm64/kernel/fpsimd.c | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 87a35364e750..088940387a4d 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -172,23 +172,14 @@ static void *sve_pffr(struct task_struct *task)
>  		sve_ffr_offset(task->thread.sve_vl);
>  }
>  
> -static void change_cpacr(u64 val, u64 mask)
> -{
> -	u64 cpacr = read_sysreg(CPACR_EL1);
> -	u64 new = (cpacr & ~mask) | val;
> -
> -	if (new != cpacr)
> -		write_sysreg(new, CPACR_EL1);
> -}
> -
>  static void sve_user_disable(void)
>  {
> -	change_cpacr(0, CPACR_EL1_ZEN_EL0EN);
> +	sysreg_clear_set(cpacr_el1, CPACR_EL1_ZEN_EL0EN, 0);
>  }
>  
>  static void sve_user_enable(void)
>  {
> -	change_cpacr(CPACR_EL1_ZEN_EL0EN, CPACR_EL1_ZEN_EL0EN);
> +	sysreg_clear_set(cpacr_el1, 0, CPACR_EL1_ZEN_EL0EN);
>  }
>  
>  /*
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] KVM: arm/arm64: fix unaligned hva start and end in handle_hva_to_gpa
From: Suzuki K Poulose @ 2018-05-14 10:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <aa5b37dc-7ac3-2999-fa67-848cd693977f@gmail.com>

On 14/05/18 03:30, Jia He wrote:
> 
> 
> On 5/11/2018 9:39 PM, Suzuki K Poulose Wrote:
>> Marc
>>
>> Thanks for looping me in. Comments below.
>>
>>
>> On 03/05/18 03:02, Jia He wrote:
>>> Hi Marc
>>>
>>> Thanks for the review
>>>
>>>
>>> On 5/2/2018 10:26 PM, Marc Zyngier Wrote:
>>>> [+ Suzuki]
>>>>
>>>> On 02/05/18 08:08, Jia He wrote:
>>>>> From: Jia He <jia.he@hxt-semitech.com>
>>>>>
>>>>> In our armv8a server (QDF2400), I noticed a WARN_ON as follows:
>>>>>
>>>>> [? 800.202850] WARNING: CPU: 33 PID: 255 at arch/arm64/kvm/../../../virt/kvm/arm/mmu.c:1670 kvm_age_hva_handler+0xcc/0xd4
>>>> Which kernel version is that? I don't have a WARN_ON() at this line in
>>>> 4.17. Do you have a reproducer?
>>> My running kernel version is v4.14-15, but I can reproduced it in 4.17 (start 20 guests and run memhog in the host)
>>> In 4.17, the warn_on is at
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/virt/kvm/arm/mmu.c#n1826
>>>>> [? 800.213535] Modules linked in: vhost_net vhost tap xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 ip6t_rpfilter ipt_REJECT nf_reject_ipv4 ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter rpcrdma ib_isert iscsi_target_mod ib_iser libiscsi scsi_transport_iscsi ib_srpt target_core_mod ib_srp scsi_transport_srp ib_ipoib rdma_ucm ib_ucm ib_uverbs ib_umad rdma_cm ib_cm vfat fat iw_cm mlx5_ib ib_core dm_mirror dm_region_hash dm_log dm_mod crc32_ce ipmi_ssif sg nfsd
>>>>> [? 800.284115]? auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs libcrc32c mlx5_core ixgbe mlxfw devlink mdio ahci_platform libahci_platform qcom_emac libahci hdma hdma_mgmt i2c_qup
>>>>> [? 800.300382] CPU: 33 PID: 255 Comm: khugepaged Tainted: G??????? W?????? 4.14.36+ #6
>>>>> [? 800.308030] Hardware name: <snip for confidential issues>
>>>> Well, that's QDF2400, right? ;-)
>>> yes, exactly :)
>>>>> [? 800.318717] task: ffff8017c949c000 task.stack: ffff8017c9498000
>>>>> [? 800.324629] PC is at kvm_age_hva_handler+0xcc/0xd4
>>>>> [? 800.329412] LR is at handle_hva_to_gpa+0xec/0x15c
>>>>> [? 800.334109] pc : [<ffff0000080b4f2c>] lr : [<ffff0000080b4838>] pstate: 20400145
>>>>> [? 800.341496] sp : ffff8017c949b260
>>>>> [? 800.344804] x29: ffff8017c949b260 x28: ffff801663e25008
>>>>> [? 800.350110] x27: 0000000000020000 x26: 00000001fb1a0000
>>>>> [? 800.355416] x25: 0000ffff605b0200 x24: 0000ffff605a0200
>>>>> [? 800.360722] x23: 0000000000000000 x22: 000000000000ffff
>>>>> [? 800.366028] x21: 00000001fb1a0000 x20: ffff8017c085a000
>>>>> [? 800.371334] x19: ffff801663e20008 x18: 0000000000000000
>>>>> [? 800.376641] x17: 0000000000000000 x16: 0000000000000000
>>>>> [? 800.381947] x15: 0000000000000000 x14: 3d646e655f617668
>>>>> [? 800.387254] x13: 2c30303230623530 x12: 36666666663d646e
>>>>> [? 800.392560] x11: 652c303032306135 x10: 3036666666663d74
>>>>> [? 800.397867] x9 : 0000000000003796 x8 : 655f6e66672c3030
>>>>> [? 800.403173] x7 : ffff00000859434c x6 : ffff8017f9c30cb8
>>>>> [? 800.408479] x5 : ffff8017f9c30cb8 x4 : ffff0000080b4e60
>>>>> [? 800.413786] x3 : 0000000000000000 x2 : 0000000000020000
>>>>> [? 800.419092] x1 : 00000001fb1a0000 x0 : 0000000020000000
>>>>> [? 800.424398] Call trace:
>>>>> [? 800.426838] Exception stack(0xffff8017c949b120 to 0xffff8017c949b260)
>>>>> [? 800.433272] b120: 0000000020000000 00000001fb1a0000 0000000000020000 0000000000000000
>>>>> [? 800.441095] b140: ffff0000080b4e60 ffff8017f9c30cb8 ffff8017f9c30cb8 ffff00000859434c
>>>>> [? 800.448918] b160: 655f6e66672c3030 0000000000003796 3036666666663d74 652c303032306135
>>>>> [? 800.456740] b180: 36666666663d646e 2c30303230623530 3d646e655f617668 0000000000000000
>>>>> [? 800.464563] b1a0: 0000000000000000 0000000000000000 0000000000000000 ffff801663e20008
>>>>> [? 800.472385] b1c0: ffff8017c085a000 00000001fb1a0000 000000000000ffff 0000000000000000
>>>>> [? 800.480208] b1e0: 0000ffff605a0200 0000ffff605b0200 00000001fb1a0000 0000000000020000
>>>>> [? 800.488030] b200: ffff801663e25008 ffff8017c949b260 ffff0000080b4838 ffff8017c949b260
>>>>> [? 800.495853] b220: ffff0000080b4f2c 0000000020400145 0000000000000001 ffff8017c949b2a0
>>>>> [? 800.503676] b240: ffffffffffffffff ffff8017c949b260 ffff8017c949b260 ffff0000080b4f2c
>>>>> [? 800.511498] [<ffff0000080b4f2c>] kvm_age_hva_handler+0xcc/0xd4
>>>>> [? 800.517324] [<ffff0000080b4838>] handle_hva_to_gpa+0xec/0x15c
>>>>> [? 800.523063] [<ffff0000080b6c5c>] kvm_age_hva+0x5c/0xcc
>>>>> [? 800.528194] [<ffff0000080a7c3c>] kvm_mmu_notifier_clear_flush_young+0x54/0x90
>>>>> [? 800.535324] [<ffff00000827a0e8>] __mmu_notifier_clear_flush_young+0x6c/0xa8
>>>>> [? 800.542279] [<ffff00000825a644>] page_referenced_one+0x1e0/0x1fc
>>>>> [? 800.548279] [<ffff00000827e8f8>] rmap_walk_ksm+0x124/0x1a0
>>>>> [? 800.553759] [<ffff00000825c974>] rmap_walk+0x94/0x98
>>>>> [? 800.558717] [<ffff00000825ca98>] page_referenced+0x120/0x180
>>>>> [? 800.564369] [<ffff000008228c58>] shrink_active_list+0x218/0x4a4
>>>>> [? 800.570281] [<ffff000008229470>] shrink_node_memcg+0x58c/0x6fc
>>>>> [? 800.576107] [<ffff0000082296c4>] shrink_node+0xe4/0x328
>>>>> [? 800.581325] [<ffff000008229c9c>] do_try_to_free_pages+0xe4/0x3b8
>>>>> [? 800.587324] [<ffff00000822a094>] try_to_free_pages+0x124/0x234
>>>>> [? 800.593150] [<ffff000008216aa0>] __alloc_pages_nodemask+0x564/0xf7c
>>>>> [? 800.599412] [<ffff000008292814>] khugepaged_alloc_page+0x38/0xb8
>>>>> [? 800.605411] [<ffff0000082933bc>] collapse_huge_page+0x74/0xd70
>>>>> [? 800.611238] [<ffff00000829470c>] khugepaged_scan_mm_slot+0x654/0xa98
>>>>> [? 800.617585] [<ffff000008294e0c>] khugepaged+0x2bc/0x49c
>>>>> [? 800.622803] [<ffff0000080ffb70>] kthread+0x124/0x150
>>>>> [? 800.627762] [<ffff0000080849f0>] ret_from_fork+0x10/0x1c
>>>>> [? 800.633066] ---[ end trace 944c130b5252fb01 ]---
>>>>> -------------------------------------------------------------------------
>>>>>
>>>>> The root cause might be: we can't guarantee that the parameter start and end
>>>>> in handle_hva_to_gpa is PAGE_SIZE aligned, let alone hva_start and hva_end.
>>>> So why not aligning them the first place?
>>> at the first place of handle_hva_to_gpa()?
>>> but boundary check is needed in each loop of kvm_for_each_memslot. Am I missing anything here?
>>
>>>>> This bug is introduced by commit 056aad67f836 ("kvm: arm/arm64: Rework gpa
>>>>> callback handlers")
>>>>>
>>>>> It fixes the bug by use pfn size converted.
>>>>>
>>>>> Fixes: 056aad67f836 ("kvm: arm/arm64: Rework gpa callback handlers")
>>>>>
>>>>> Signed-off-by: jia.he at hxt-semitech.com
>>>>> Signed-off-by: li.zhang at hxt-semitech.com
>>>>> ---
>>>>> ?? virt/kvm/arm/mmu.c | 10 ++++++----
>>>>> ?? 1 file changed, 6 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>>>>> index 7f6a944..9dd7ae4 100644
>>>>> --- a/virt/kvm/arm/mmu.c
>>>>> +++ b/virt/kvm/arm/mmu.c
>>>>> @@ -1744,7 +1744,7 @@ static int handle_hva_to_gpa(struct kvm *kvm,
>>>>> ?????? /* we only care about the pages that the guest sees */
>>>>> ?????? kvm_for_each_memslot(memslot, slots) {
>>>>> ?????????? unsigned long hva_start, hva_end;
>>>>> -??????? gfn_t gpa;
>>>>> +??????? gpa_t gpa, gpa_end;
>>>>> ?????????? hva_start = max(start, memslot->userspace_addr);
>>>>> ?????????? hva_end = min(end, memslot->userspace_addr +
>>>>> @@ -1753,7 +1753,9 @@ static int handle_hva_to_gpa(struct kvm *kvm,
>>>>> ?????????????? continue;
>>>>> ?????????? gpa = hva_to_gfn_memslot(hva_start, memslot) << PAGE_SHIFT;
>>>>> -??????? ret |= handler(kvm, gpa, (u64)(hva_end - hva_start), data);
>>>>> +??????? gpa_end = hva_to_gfn_memslot(hva_end + PAGE_SIZE - 1, memslot)
>>>>> +???????????????????????????? << PAGE_SHIFT;
>>>>> +??????? ret |= handler(kvm, gpa, (u64)(gpa_end - gpa), data);
>>>> But we're looking for the mapping in the same memslot, so the distance
>>>> between hva and hva_end is the same as the one between gpa and gpa_end
>>>> if you didn't align it.
>>> maybe not, sometimes hva_end-hva != gpa_end-gpa
>>> start=fffdc37f0200,hva_start=fffdc37f0200,end=fffdc3800200,hva_end=fffdc3800000,gpa=3ff0000,gfn_end=4000000
>>>
>>> but sometimes it is:
>>> start=ffff60590200,hva_start=ffff60590200,end=ffff605a0200,hva_end=ffff605a0200,gpa=1fb190000,gfn_end=1fb1b0000
>>>
>>> IMO, the unalignment is caused by the ksm stable page flag STABLE_FLAG. I will
>>> propose another ksm patch to fix it?
>>> But from handle_hva_to_gpa's point of view, arm kvm needs to void the followup
>>> exception, just like what powerpc andx86 have done.
>>
>>
>> As far as I can see this is triggered by someone (in this page_referenced_one via ksm?)
>> triggering a clear_flush_young for a page, with a non-aligned page address.
>>
>> If you look at the code path, the __mmu_notifier_clear_flush_young is invoked
>> via 2 code paths with the "given" address.
>>
>> ptep_clear_flush_young_notify(), in which case the end is set to + PAGE_SIZE
>> pmdp_clear_flush_young_notify(), in which case the end is set to + PMD_SIZE
>>
>> We were supposed to only clear_flush_young for *the page* containing
>> address (start), but we do a clear_flush_young for the next page
>> as well, which (I think) is not something intended. So to me, it looks like, either
>> page_referenced_one() or its caller must align the address to the PAGE_SIZE
>> or PMD_SIZE depending on what it really wants to do, to avoid touching
>> the adjacent entries (page or block pages).
>>
>> Suzuki
> 

Jia He,

> Suzuki, thanks for the comments.
> 
> I proposed another ksm patch https://lkml.org/lkml/2018/5/3/1042
> The root cause is ksm will add some extra flags to indicate that the page
> is in/not_in the stable tree. This makes address not be aligned with PAGE_SIZE.

Thanks for the pointer. In the future, please Cc the people relevant to the
discussion in the patches.

> 
>  From arm kvm mmu point of view, do you think handle_hva_to_gpa still need to handle
> the unalignment case?

I don't think we should do that. Had we done this, we would never have caught this bug
in KSM. Eventually if some other new implementation comes up with the a new notifier
consumer which doesn't check alignment and doesn't WARN, it could simply do the wrong
thing. So I believe what we have is a good measure to make sure that things are
in the right order.

> IMO, the PAGE_SIZE alignment is still needed because we should not let the bottom function
> kvm_age_hva_handler to handle the exception. Please refer to the implementation in X86 and
> powerpc kvm_handle_hva_range(). They both aligned the hva with hva_to_gfn_memslot.
> 

 From an API perspective, you are passed on a "start" and "end" address. So, you could potentially
do the wrong thing if you align the "start" and "end". May be those handlers should also do the
same thing as we do.

Cheers
Suzuki

> Cheers,
> Jia
> 
>>>> So why not align both start and end and skip the double lookup?
>>>>
>>>>> ?????? }
>>>>> ?????? return ret;
>>>>> @@ -1823,7 +1825,7 @@ static int kvm_age_hva_handler(struct kvm *kvm, gpa_t gpa, u64 size, void *data)
>>>>> ?????? pmd_t *pmd;
>>>>> ?????? pte_t *pte;
>>>>> -??? WARN_ON(size != PAGE_SIZE && size != PMD_SIZE);
>>>>> +??? WARN_ON((size & ~PAGE_MASK) != 0);
>>>>> ?????? pmd = stage2_get_pmd(kvm, NULL, gpa);
>>>>> ?????? if (!pmd || pmd_none(*pmd))??? /* Nothing there */
>>>>> ?????????? return 0;
>>>>> @@ -1843,7 +1845,7 @@ static int kvm_test_age_hva_handler(struct kvm *kvm, gpa_t gpa, u64 size, void *
>>>>> ?????? pmd_t *pmd;
>>>>> ?????? pte_t *pte;
>>>>> -??? WARN_ON(size != PAGE_SIZE && size != PMD_SIZE);
>>>>> +??? WARN_ON((size & ~PAGE_MASK) != 0);
>>>>> ?????? pmd = stage2_get_pmd(kvm, NULL, gpa);
>>>>> ?????? if (!pmd || pmd_none(*pmd))??? /* Nothing there */
>>>>> ?????????? return 0;
>>>>>
>>>> I'll let Suzuki comment on this, but I'm a bit suspicious of this patch.
>>> sure, more comments, more clear for the issue.
>>>
>>
> 

^ permalink raw reply

* [PATCH 2/2] arm64: Clear the stack
From: Mark Rutland @ 2018-05-14 10:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <6c311899-7131-d21b-10f6-d2ba7380a392@linux.com>

On Mon, May 14, 2018 at 12:35:25PM +0300, Alexander Popov wrote:
> On 14.05.2018 08:15, Mark Rutland wrote:
> > On Sun, May 13, 2018 at 11:40:07AM +0300, Alexander Popov wrote:
> >> So what would you think if I do the following in check_alloca():
> >>
> >> 	if (size >= stack_left) {
> >> #if !defined(CONFIG_VMAP_STACK) && defined(CONFIG_SCHED_STACK_END_CHECK)
> >> 		panic("alloca over the kernel stack boundary\n");
> >> #else
> >> 		BUG();
> >> #endif
> > 
> > Given this is already out-of-line, how about we always use panic(), regardless
> > of VMAP_STACK and SCHED_STACK_END_CHECK? i.e. just
> > 
> > 	if (unlikely(size >= stack_left))
> > 		panic("alloca over the kernel stack boundary");
> > 
> > If we have VMAP_STACK selected, and overflow during the panic, it's the same as
> > if we overflowed during the BUG(). It's likely that panic() will use less stack
> > space than BUG(), and the compiler can put the call in a slow path that
> > shouldn't affect most calls, so in all cases it's likely preferable.
> 
> I'm sure that maintainers and Linus will strongly dislike my patch if I always
> use panic() here. panic() kills the whole kernel and we shouldn't use it when we
> can safely continue to work.
> 
> Let me describe my logic. So let's have size >= stack_left on a thread stack.
> 
> 1. If CONFIG_VMAP_STACK is enabled, we can safely use BUG(). Even if BUG()
> handling overflows the thread stack into the guard page, handle_stack_overflow()
> is called and the neighbour memory is not corrupted. The kernel can proceed to live.

On arm64 with CONFIG_VMAP_STACK, a stack overflow will result in a
panic(). My understanding was that the same is true on x86.

> 2. If CONFIG_VMAP_STACK is disabled, BUG() handling can corrupt the neighbour
> kernel memory and cause the undefined behaviour of the whole kernel. I see it on
> my lkdtm test. That is a cogent reason for panic().

In this case, panic() can also corrupt the neighbour stack, and could
also fail.

When CONFIG_VMAP_STACK is not selected, a stack overflow simply cannot
be handled reliably -- while panic() may be more likely to succeed, it
is not gauranteed to.

> 2.a. If CONFIG_SCHED_STACK_END_CHECK is enabled, the kernel already does panic()
> when STACK_END_MAGIC is corrupted. So we will _not_ break the safety policy if
> we do panic() in a similar situation in check_alloca().

Sure, I'm certainly happy with panic() here.

> 2.b. If CONFIG_SCHED_STACK_END_CHECK is disabled, the user has some real reasons
> not to do panic() when the kernel stack is corrupted. 

I believe that CONFIG_SCHED_STACK_END_CHECK is seen as a debug feature,
and hence people don't select it. I strongly doubt that people have
reasons to disable it other than not wanting the overhead associated
with debug features.

I think it is reasonable to panic() here even with CONFIG_VMAP_STACK
selected.

> So we should not do it in check_alloca() as well, just use BUG() and
> hope for the best.

Regardless of whether we BUG() or panic(), we're hoping for the best.

Consistently using panic() here will keep things simpler, so any failure
reported will be easier to reason about, and easier to debug.

Thanks,
Mark.

^ 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