Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 19/20] arm64:ilp32: add vdso-ilp32 and use for signal return
From: Yury Norov @ 2017-01-09 11:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483961397-8599-1-git-send-email-ynorov@caviumnetworks.com>

From: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

ILP32 VDSO exports next symbols:
 __kernel_rt_sigreturn;
 __kernel_gettimeofday;
 __kernel_clock_gettime;
 __kernel_clock_getres.

What shared object to use, kernel selects depending on result of
is_ilp32_compat_task() in arch/arm64/kernel/vdso.c, so it substitutes
correct pages and spec.

Adjusted to move the move data page before code pages in sync with
commit 601255ae3c98 ("arm64: vdso: move data page before code pages")

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
---
 arch/arm64/include/asm/vdso.h                 |  6 ++
 arch/arm64/kernel/Makefile                    | 11 ++++
 arch/arm64/kernel/asm-offsets.c               |  7 ++
 arch/arm64/kernel/signal.c                    |  2 +
 arch/arm64/kernel/vdso-ilp32/.gitignore       |  2 +
 arch/arm64/kernel/vdso-ilp32/Makefile         | 74 +++++++++++++++++++++
 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S     | 33 ++++++++++
 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S | 95 +++++++++++++++++++++++++++
 arch/arm64/kernel/vdso.c                      | 66 ++++++++++++++++---
 arch/arm64/kernel/vdso/gettimeofday.S         | 20 +++++-
 arch/arm64/kernel/vdso/vdso.S                 |  6 +-
 11 files changed, 306 insertions(+), 16 deletions(-)
 create mode 100644 arch/arm64/kernel/vdso-ilp32/.gitignore
 create mode 100644 arch/arm64/kernel/vdso-ilp32/Makefile
 create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S
 create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S

diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h
index 839ce00..649a9a4 100644
--- a/arch/arm64/include/asm/vdso.h
+++ b/arch/arm64/include/asm/vdso.h
@@ -29,6 +29,12 @@
 
 #include <generated/vdso-offsets.h>
 
+#ifdef CONFIG_ARM64_ILP32
+#include <generated/vdso-ilp32-offsets.h>
+#else
+#define vdso_offset_sigtramp_ilp32
+#endif
+
 #define VDSO_SYMBOL(base, name)						   \
 ({									   \
 	(void *)(vdso_offset_##name - VDSO_LBASE + (unsigned long)(base)); \
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index af400fb..43e680a 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -55,6 +55,17 @@ arm64-obj-$(CONFIG_KEXEC)		+= machine_kexec.o relocate_kernel.o	\
 					   cpu-reset.o
 
 obj-y					+= $(arm64-obj-y) vdso/ probes/
+obj-$(CONFIG_ARM64_ILP32)		+= vdso-ilp32/
 obj-m					+= $(arm64-obj-m)
 head-y					:= head.o
 extra-y					+= $(head-y) vmlinux.lds
+
+# vDSO - this must be built first to generate the symbol offsets
+$(call objectify,$(arm64-obj-y)): $(obj)/vdso/vdso-offsets.h
+$(obj)/vdso/vdso-offsets.h: $(obj)/vdso
+
+ifeq ($(CONFIG_ARM64_ILP32),y)
+# vDSO - this must be built first to generate the symbol offsets
+$(call objectify,$(arm64-obj-y)): $(obj)/vdso-ilp32/vdso-ilp32-offsets.h
+$(obj)/vdso-ilp32/vdso-ilp32-offsets.h: $(obj)/vdso-ilp32
+endif
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index a5a3154..947d26c 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -121,6 +121,13 @@ int main(void)
   DEFINE(TSPEC_TV_SEC,		offsetof(struct timespec, tv_sec));
   DEFINE(TSPEC_TV_NSEC,		offsetof(struct timespec, tv_nsec));
   BLANK();
+#ifdef CONFIG_COMPAT
+  DEFINE(COMPAT_TVAL_TV_SEC,	offsetof(struct compat_timeval, tv_sec));
+  DEFINE(COMPAT_TVAL_TV_USEC,	offsetof(struct compat_timeval, tv_usec));
+  DEFINE(COMPAT_TSPEC_TV_SEC,	offsetof(struct compat_timespec, tv_sec));
+  DEFINE(COMPAT_TSPEC_TV_NSEC,	offsetof(struct compat_timespec, tv_nsec));
+  BLANK();
+#endif
   DEFINE(TZ_MINWEST,		offsetof(struct timezone, tz_minuteswest));
   DEFINE(TZ_DSTTIME,		offsetof(struct timezone, tz_dsttime));
   BLANK();
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index b78f4c2..a7f201d 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -267,6 +267,8 @@ void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
 
 	if (ka->sa.sa_flags & SA_RESTORER)
 		sigtramp = ka->sa.sa_restorer;
+	else if (is_ilp32_compat_task())
+		sigtramp = VDSO_SYMBOL(current->mm->context.vdso, sigtramp_ilp32);
 	else
 		sigtramp = VDSO_SYMBOL(current->mm->context.vdso, sigtramp);
 
diff --git a/arch/arm64/kernel/vdso-ilp32/.gitignore b/arch/arm64/kernel/vdso-ilp32/.gitignore
new file mode 100644
index 0000000..61806c3
--- /dev/null
+++ b/arch/arm64/kernel/vdso-ilp32/.gitignore
@@ -0,0 +1,2 @@
+vdso-ilp32.lds
+vdso-ilp32-offsets.h
diff --git a/arch/arm64/kernel/vdso-ilp32/Makefile b/arch/arm64/kernel/vdso-ilp32/Makefile
new file mode 100644
index 0000000..0671e88
--- /dev/null
+++ b/arch/arm64/kernel/vdso-ilp32/Makefile
@@ -0,0 +1,74 @@
+#
+# Building a vDSO image for AArch64.
+#
+# Author: Will Deacon <will.deacon@arm.com>
+# Heavily based on the vDSO Makefiles for other archs.
+#
+
+obj-ilp32-vdso := gettimeofday-ilp32.o note-ilp32.o sigreturn-ilp32.o
+
+# Build rules
+targets := $(obj-ilp32-vdso) vdso-ilp32.so vdso-ilp32.so.dbg
+obj-ilp32-vdso := $(addprefix $(obj)/, $(obj-ilp32-vdso))
+
+ccflags-y := -shared -fno-common -fno-builtin
+ccflags-y += -nostdlib -Wl,-soname=linux-ilp32-vdso.so.1 \
+		$(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
+
+obj-y += vdso-ilp32.o
+extra-y += vdso-ilp32.lds vdso-ilp32-offsets.h
+CPPFLAGS_vdso-ilp32.lds += -P -C -U$(ARCH) -mabi=ilp32
+
+# Force dependency (incbin is bad)
+$(obj)/vdso-ilp32.o : $(obj)/vdso-ilp32.so
+
+# Link rule for the .so file, .lds has to be first
+$(obj)/vdso-ilp32.so.dbg: $(src)/vdso-ilp32.lds $(obj-ilp32-vdso)
+	$(call if_changed,vdso-ilp32ld)
+
+# Strip rule for the .so file
+$(obj)/%.so: OBJCOPYFLAGS := -S
+$(obj)/%.so: $(obj)/%.so.dbg FORCE
+	$(call if_changed,objcopy)
+
+# Generate VDSO offsets using helper script
+gen-vdsosym := $(srctree)/$(src)/../vdso/gen_vdso_offsets.sh
+quiet_cmd_vdsosym = VDSOSYM $@
+define cmd_vdsosym
+	$(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ && \
+	cp $@ include/generated/
+endef
+
+$(obj)/vdso-ilp32-offsets.h: $(obj)/vdso-ilp32.so.dbg FORCE
+	$(call if_changed,vdsosym)
+
+# Assembly rules for the .S files
+#$(obj-ilp32-vdso): %.o: $(src)/../vdso/$(subst -ilp32,,%.S)
+#	$(call if_changed_dep,vdso-ilp32as)
+
+$(obj)/gettimeofday-ilp32.o: $(src)/../vdso/gettimeofday.S
+	$(call if_changed_dep,vdso-ilp32as)
+
+$(obj)/note-ilp32.o: $(src)/../vdso/note.S
+	$(call if_changed_dep,vdso-ilp32as)
+
+# This one should be fine because ILP32 uses the same generic
+# __NR_rt_sigreturn syscall number.
+$(obj)/sigreturn-ilp32.o: $(src)/../vdso/sigreturn.S
+	$(call if_changed_dep,vdso-ilp32as)
+
+# Actual build commands
+quiet_cmd_vdso-ilp32ld = VDSOILP32L $@
+      cmd_vdso-ilp32ld = $(CC) $(c_flags) -mabi=ilp32  -Wl,-n -Wl,-T $^ -o $@
+quiet_cmd_vdso-ilp32as = VDSOILP32A $@
+      cmd_vdso-ilp32as = $(CC) $(a_flags) -mabi=ilp32 -c -o $@ $<
+
+# Install commands for the unstripped file
+quiet_cmd_vdso_install = INSTALL $@
+      cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
+
+vdso-ilp32.so: $(obj)/vdso-ilp32.so.dbg
+	@mkdir -p $(MODLIB)/vdso
+	$(call cmd,vdso_install)
+
+vdso_install: vdso-ilp32.so
diff --git a/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S b/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S
new file mode 100644
index 0000000..46ac072
--- /dev/null
+++ b/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2012 ARM Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Will Deacon <will.deacon@arm.com>
+ */
+
+#include <linux/init.h>
+#include <linux/linkage.h>
+#include <linux/const.h>
+#include <asm/page.h>
+
+	__PAGE_ALIGNED_DATA
+
+	.globl vdso_ilp32_start, vdso_ilp32_end
+	.balign PAGE_SIZE
+vdso_ilp32_start:
+	.incbin "arch/arm64/kernel/vdso-ilp32/vdso-ilp32.so"
+	.balign PAGE_SIZE
+vdso_ilp32_end:
+
+	.previous
diff --git a/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S b/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S
new file mode 100644
index 0000000..ba98794
--- /dev/null
+++ b/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S
@@ -0,0 +1,95 @@
+/*
+ * GNU linker script for the VDSO library.
+ *
+ * Copyright (C) 2012 ARM Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Will Deacon <will.deacon@arm.com>
+ * Heavily based on the vDSO linker scripts for other archs.
+ */
+
+#include <linux/const.h>
+#include <asm/page.h>
+#include <asm/vdso.h>
+
+SECTIONS
+{
+	PROVIDE(_vdso_data = . - PAGE_SIZE);
+	. = VDSO_LBASE + SIZEOF_HEADERS;
+
+	.hash		: { *(.hash) }			:text
+	.gnu.hash	: { *(.gnu.hash) }
+	.dynsym		: { *(.dynsym) }
+	.dynstr		: { *(.dynstr) }
+	.gnu.version	: { *(.gnu.version) }
+	.gnu.version_d	: { *(.gnu.version_d) }
+	.gnu.version_r	: { *(.gnu.version_r) }
+
+	.note		: { *(.note.*) }		:text	:note
+
+	. = ALIGN(16);
+
+	.text		: { *(.text*) }			:text	=0xd503201f
+	PROVIDE (__etext = .);
+	PROVIDE (_etext = .);
+	PROVIDE (etext = .);
+
+	.eh_frame_hdr	: { *(.eh_frame_hdr) }		:text	:eh_frame_hdr
+	.eh_frame	: { KEEP (*(.eh_frame)) }	:text
+
+	.dynamic	: { *(.dynamic) }		:text	:dynamic
+
+	.rodata		: { *(.rodata*) }		:text
+
+	_end = .;
+	PROVIDE(end = .);
+
+	/DISCARD/	: {
+		*(.note.GNU-stack)
+		*(.data .data.* .gnu.linkonce.d.* .sdata*)
+		*(.bss .sbss .dynbss .dynsbss)
+	}
+}
+
+/*
+ * We must supply the ELF program headers explicitly to get just one
+ * PT_LOAD segment, and set the flags explicitly to make segments read-only.
+ */
+PHDRS
+{
+	text		PT_LOAD		FLAGS(5) FILEHDR PHDRS; /* PF_R|PF_X */
+	dynamic		PT_DYNAMIC	FLAGS(4);		/* PF_R */
+	note		PT_NOTE		FLAGS(4);		/* PF_R */
+	eh_frame_hdr	PT_GNU_EH_FRAME;
+}
+
+/*
+ * This controls what symbols we export from the DSO.
+ */
+VERSION
+{
+	LINUX_4.10 {
+	global:
+		__kernel_rt_sigreturn;
+		__kernel_gettimeofday;
+		__kernel_clock_gettime;
+		__kernel_clock_getres;
+	local: *;
+	};
+}
+
+/*
+ * Make the sigreturn code visible to the kernel.
+ */
+VDSO_sigtramp_ilp32		= __kernel_rt_sigreturn;
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 7f822cd..f073332 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -37,8 +37,13 @@
 #include <asm/vdso.h>
 #include <asm/vdso_datapage.h>
 
-extern char vdso_start, vdso_end;
-static unsigned long vdso_pages __ro_after_init;
+extern char vdso_lp64_start, vdso_lp64_end;
+static unsigned long vdso_lp64_pages __ro_after_init;
+
+#ifdef CONFIG_ARM64_ILP32
+extern char vdso_ilp32_start, vdso_ilp32_end;
+static unsigned long vdso_ilp32_pages __ro_after_init;
+#endif
 
 /*
  * The vDSO data page.
@@ -110,7 +115,17 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp)
 }
 #endif /* CONFIG_AARCH32_EL0 */
 
-static struct vm_special_mapping vdso_spec[2] __ro_after_init = {
+static struct vm_special_mapping vdso_lp64_spec[2] __ro_after_init = {
+	{
+		.name	= "[vvar]",
+	},
+	{
+		.name	= "[vdso]",
+	},
+};
+
+#ifdef CONFIG_ARM64_ILP32
+static struct vm_special_mapping vdso_ilp32_spec[2] __ro_after_init = {
 	{
 		.name	= "[vvar]",
 	},
@@ -118,20 +133,26 @@ static struct vm_special_mapping vdso_spec[2] __ro_after_init = {
 		.name	= "[vdso]",
 	},
 };
+#endif
 
-static int __init vdso_init(void)
+static int __init vdso_init(char *vdso_start, char *vdso_end,
+					  unsigned long *vdso_pagesp,
+					  struct vm_special_mapping *vdso_spec)
 {
 	int i;
+	unsigned long vdso_pages;
 	struct page **vdso_pagelist;
 
-	if (memcmp(&vdso_start, "\177ELF", 4)) {
+	if (memcmp(vdso_start, "\177ELF", 4)) {
 		pr_err("vDSO is not a valid ELF object!\n");
 		return -EINVAL;
 	}
 
-	vdso_pages = (&vdso_end - &vdso_start) >> PAGE_SHIFT;
+	vdso_pages = (vdso_end - vdso_start) >> PAGE_SHIFT;
+	*vdso_pagesp = vdso_pages;
 	pr_info("vdso: %ld pages (%ld code @ %p, %ld data @ %p)\n",
-		vdso_pages + 1, vdso_pages, &vdso_start, 1L, vdso_data);
+					vdso_pages + 1, vdso_pages,
+					vdso_start, 1L, vdso_data);
 
 	/* Allocate the vDSO pagelist, plus a page for the data. */
 	vdso_pagelist = kcalloc(vdso_pages + 1, sizeof(struct page *),
@@ -144,14 +165,30 @@ static int __init vdso_init(void)
 
 	/* Grab the vDSO code pages. */
 	for (i = 0; i < vdso_pages; i++)
-		vdso_pagelist[i + 1] = pfn_to_page(PHYS_PFN(__pa(&vdso_start)) + i);
+		vdso_pagelist[i + 1] =
+			pfn_to_page(PHYS_PFN(__pa(vdso_start)) + i);
 
 	vdso_spec[0].pages = &vdso_pagelist[0];
 	vdso_spec[1].pages = &vdso_pagelist[1];
 
 	return 0;
 }
-arch_initcall(vdso_init);
+
+static int __init vdso_lp64_init(void)
+{
+	return vdso_init(&vdso_lp64_start, &vdso_lp64_end,
+				&vdso_lp64_pages, vdso_lp64_spec);
+}
+arch_initcall(vdso_lp64_init);
+
+#ifdef CONFIG_ARM64_ILP32
+static int __init vdso_ilp32_init(void)
+{
+	return vdso_init(&vdso_ilp32_start, &vdso_ilp32_end,
+				&vdso_ilp32_pages, vdso_ilp32_spec);
+}
+arch_initcall(vdso_ilp32_init);
+#endif
 
 int arch_setup_additional_pages(struct linux_binprm *bprm,
 				int uses_interp)
@@ -159,8 +196,17 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
 	struct mm_struct *mm = current->mm;
 	unsigned long vdso_base, vdso_text_len, vdso_mapping_len;
 	void *ret;
+	unsigned long pages = vdso_lp64_pages;
+	struct vm_special_mapping *vdso_spec = vdso_lp64_spec;
+
+#ifdef CONFIG_ARM64_ILP32
+	if (is_ilp32_compat_task()) {
+	        pages = vdso_ilp32_pages;
+	        vdso_spec = vdso_ilp32_spec;
+	}
+#endif
 
-	vdso_text_len = vdso_pages << PAGE_SHIFT;
+	vdso_text_len = pages << PAGE_SHIFT;
 	/* Be sure to map the data page */
 	vdso_mapping_len = vdso_text_len + PAGE_SIZE;
 
diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S
index e00b467..062a33d 100644
--- a/arch/arm64/kernel/vdso/gettimeofday.S
+++ b/arch/arm64/kernel/vdso/gettimeofday.S
@@ -25,6 +25,16 @@
 #define NSEC_PER_SEC_LO16	0xca00
 #define NSEC_PER_SEC_HI16	0x3b9a
 
+#ifdef __LP64__
+#define PTR_REG(n)	x##n
+#define OFFSET(n)	n
+#define DELOUSE(n)
+#else
+#define PTR_REG(n)	w##n
+#define OFFSET(n)	COMPAT_##n
+#define DELOUSE(n)	mov     w##n, w##n
+#endif
+
 vdso_data	.req	x6
 seqcnt		.req	w7
 w_tmp		.req	w8
@@ -119,7 +129,7 @@ x_tmp		.req	x8
 	.if \shift == 1
 	lsr	x11, x11, x12
 	.endif
-	stp	x10, x11, [x1, #TSPEC_TV_SEC]
+	stp     PTR_REG(10), PTR_REG(11), [x1, #OFFSET(TSPEC_TV_SEC)]
 	mov	x0, xzr
 	ret
 	.endm
@@ -136,6 +146,8 @@ x_tmp		.req	x8
 /* int __kernel_gettimeofday(struct timeval *tv, struct timezone *tz); */
 ENTRY(__kernel_gettimeofday)
 	.cfi_startproc
+	DELOUSE(0)
+	DELOUSE(1)
 	adr	vdso_data, _vdso_data
 	/* If tv is NULL, skip to the timezone code. */
 	cbz	x0, 2f
@@ -160,7 +172,7 @@ ENTRY(__kernel_gettimeofday)
 	mov	x13, #1000
 	lsl	x13, x13, x12
 	udiv	x11, x11, x13
-	stp	x10, x11, [x0, #TVAL_TV_SEC]
+	stp	PTR_REG(10), PTR_REG(11), [x0, #OFFSET(TVAL_TV_SEC)]
 2:
 	/* If tz is NULL, return 0. */
 	cbz	x1, 3f
@@ -182,6 +194,7 @@ ENDPROC(__kernel_gettimeofday)
 /* int __kernel_clock_gettime(clockid_t clock_id, struct timespec *tp); */
 ENTRY(__kernel_clock_gettime)
 	.cfi_startproc
+	DELOUSE(1)
 	cmp	w0, #JUMPSLOT_MAX
 	b.hi	syscall
 	adr	vdso_data, _vdso_data
@@ -297,6 +310,7 @@ ENDPROC(__kernel_clock_gettime)
 /* int __kernel_clock_getres(clockid_t clock_id, struct timespec *res); */
 ENTRY(__kernel_clock_getres)
 	.cfi_startproc
+	DELOUSE(1)
 	cmp	w0, #CLOCK_REALTIME
 	ccmp	w0, #CLOCK_MONOTONIC, #0x4, ne
 	ccmp	w0, #CLOCK_MONOTONIC_RAW, #0x4, ne
@@ -311,7 +325,7 @@ ENTRY(__kernel_clock_getres)
 	ldr	x2, 6f
 2:
 	cbz	w1, 3f
-	stp	xzr, x2, [x1]
+	stp	PTR_REG(zr), PTR_REG(2), [x1]
 
 3:	/* res == NULL. */
 	mov	w0, wzr
diff --git a/arch/arm64/kernel/vdso/vdso.S b/arch/arm64/kernel/vdso/vdso.S
index 82379a7..a40ae24 100644
--- a/arch/arm64/kernel/vdso/vdso.S
+++ b/arch/arm64/kernel/vdso/vdso.S
@@ -21,12 +21,12 @@
 #include <linux/const.h>
 #include <asm/page.h>
 
-	.globl vdso_start, vdso_end
+	.globl vdso_lp64_start, vdso_lp64_end
 	.section .rodata
 	.balign PAGE_SIZE
-vdso_start:
+vdso_lp64_start:
 	.incbin "arch/arm64/kernel/vdso/vdso.so"
 	.balign PAGE_SIZE
-vdso_end:
+vdso_lp64_end:
 
 	.previous
-- 
2.7.4

^ permalink raw reply related

* [PATCH 20/20] arm64:ilp32: add ARM64_ILP32 to Kconfig
From: Yury Norov @ 2017-01-09 11:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483961397-8599-1-git-send-email-ynorov@caviumnetworks.com>

From: Andrew Pinski <apinski@cavium.com>

This patch adds the config option for ILP32.

Signed-off-by: Andrew Pinski <Andrew.Pinski@caviumnetworks.com>
Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
Reviewed-by: David Daney <ddaney@caviumnetworks.com>
---
 arch/arm64/Kconfig | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index e668ace..bf34424 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1009,7 +1009,7 @@ source "fs/Kconfig.binfmt"
 
 config COMPAT
 	bool
-	depends on AARCH32_EL0
+	depends on AARCH32_EL0 || ARM64_ILP32
 
 config AARCH32_EL0
 	bool "Kernel support for 32-bit EL0"
@@ -1031,6 +1031,14 @@ config AARCH32_EL0
 
 	  If you want to execute 32-bit userspace applications, say Y.
 
+config ARM64_ILP32
+	bool "Kernel support for ILP32"
+	select COMPAT
+	help
+	  This option enables support for AArch64 ILP32 user space.  ILP32
+	  is an ABI where long and pointers are 32bits but it uses the AARCH64
+	  instruction set.
+
 config SYSVIPC_COMPAT
 	def_bool y
 	depends on COMPAT && SYSVIPC
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 1/7] arm: put types.h in uapi
From: Arnd Bergmann @ 2017-01-09 11:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483695839-18660-2-git-send-email-nicolas.dichtel@6wind.com>

On Friday, January 6, 2017 10:43:53 AM CET Nicolas Dichtel wrote:
> 
> diff --git a/arch/arm/include/asm/types.h b/arch/arm/include/asm/types.h
> index a53cdb8f068c..c48fee3d7b3b 100644
> --- a/arch/arm/include/asm/types.h
> +++ b/arch/arm/include/asm/types.h
> @@ -1,40 +1,6 @@
>  #ifndef _ASM_TYPES_H
>  #define _ASM_TYPES_H
>  
> -#include <asm-generic/int-ll64.h>
...
> -#define __UINTPTR_TYPE__       unsigned long
> -#endif
> +#include <uapi/asm/types.h>
>  
>  #endif /* _ASM_TYPES_H */
> 

Moving the file is correct as far as I can tell, but the extra
#include is not necessary here, as the kernel will automatically
search both arch/arm/include/ and arch/arm/include/uapi/.

The same applies to patches 2 and 4.

	Arnd

^ permalink raw reply

* [PATCH v2 3/7] nios2: put setup.h in uapi
From: Arnd Bergmann @ 2017-01-09 11:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483695839-18660-4-git-send-email-nicolas.dichtel@6wind.com>

On Friday, January 6, 2017 10:43:55 AM CET Nicolas Dichtel wrote:

> diff --git a/arch/nios2/include/uapi/asm/setup.h b/arch/nios2/include/uapi/asm/setup.h
> new file mode 100644
> index 000000000000..8d8285997ba8
> --- /dev/null
> +++ b/arch/nios2/include/uapi/asm/setup.h
> @@ -0,0 +1,6 @@
> +#ifndef _UAPI_ASM_NIOS2_SETUP_H
> +#define _UAPI_ASM_NIOS2_SETUP_H
> +
> +#include <asm-generic/setup.h>
> +
> +#endif /* _UAPI_ASM_NIOS2_SETUP_H */
> 

This one is only a redirect to an asm-generic header, so it can be
removed completely and replaced with a line in the 
arch/nios2/include/uapi/asm/ file:

generic-y += setup.h

	Arnd

^ permalink raw reply

* [PATCH v2 0/7] uapi: export all headers under uapi directories
From: Arnd Bergmann @ 2017-01-09 11:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483695839-18660-1-git-send-email-nicolas.dichtel@6wind.com>

On Friday, January 6, 2017 10:43:52 AM CET Nicolas Dichtel wrote:
> Here is the v2 of this series. The first 5 patches are just cleanup: some
> exported headers were still under a non-uapi directory.

Since this is meant as a cleanup, I commented on this to point out a cleaner
way to do the same.

> The patch 6 was spotted by code review: there is no in-tree user of this
> functionality.
> The last patch remove the use of header-y. Now all files under an uapi
> directory are exported.

Very nice!

> asm is a bit special, most of architectures export asm/<arch>/include/uapi/asm
> only, but there is two exceptions:
>  - cris which exports arch/cris/include/uapi/arch-v[10|32];

This is interesting, though not your problem. Maybe someone who understands
cris better can comment on this: How is the decision made about which of
the arch/user.h headers gets used? I couldn't find that in the sources,
but it appears to be based on kernel compile-time settings, which is
wrong for user space header files that should be independent of the kernel
config.

>  - tile which exports arch/tile/include/uapi/arch.
> Because I don't know if the output of 'make headers_install_all' can be changed,
> I introduce subdir-y in Kbuild file. The headers_install_all target copies all
> asm/<arch>/include/uapi/asm to usr/include/asm-<arch> but
> arch/cris/include/uapi/arch-v[10|32] and arch/tile/include/uapi/arch are not
> prefixed (they are put asis in usr/include/). If it's acceptable to modify the
> output of 'make headers_install_all' to export asm headers in
> usr/include/asm-<arch>/asm, then I could remove this new subdir-y and exports
> everything under arch/<arch>/include/uapi/.

I don't know if anyone still uses "make headers_install_all", I suspect
distros these days all use "make headers_install", so it probably
doesn't matter much.

In case of cris, it should be easy enough to move all the contents of the
uapi/arch-*/*.h headers into the respective uapi/asm/*.h headers, they
only seem to be referenced from there.

For tile, I suspect that would not work as the arch/*.h headers are
apparently defined as interfaces for both user space and kernel.

> Note also that exported files for asm are a mix of files listed by:
>  - include/uapi/asm-generic/Kbuild.asm;
>  - arch/x86/include/uapi/asm/Kbuild;
>  - arch/x86/include/asm/Kbuild.
> This complicates a lot the processing (arch/x86/include/asm/Kbuild is also
> used by scripts/Makefile.asm-generic).
> 
> This series has been tested with a 'make headers_install' on x86 and a
> 'make headers_install_all'. I've checked the result of both commands.
> 
> This patch is built against linus tree. I don't know if it should be
> made against antoher tree.

The series should probably get merged through the kbuild tree, but testing
it on mainline is fine here.

	Arnd

^ permalink raw reply

* APM smmu implementation
From: Robin Murphy @ 2017-01-09 11:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAL85gmA_SSCwM80TKdkZqEe+S1beWzDEvdki1kpkmUTDRmSP7g@mail.gmail.com>

On 06/01/17 23:21, Feng Kan wrote:
> Hi Will/Robin:
> 
> The APM IOMMU implementation is mostly just the ARM SMMU 500 variant.

"Mostly"? Have APM actually modified it (which I strongly doubt) or do
you mean it's simply been integrated with the upper address lines tied
off? MMU-500 reports a 48-bit IAS because MMU-500 has 48-bit-wide slave
interfaces; that's all there is to it. Whether or not you use all of
those bits is up to you as a system integrator.

> However, our internal bus is only 42 bits wide. Our IAS field is coded
> as 48 bits, which cause IPA to truncated to 42 bits on the physical
> bus. In order for our system to work with the arm-smmu.c, there needs
> to be a way to force the ipa_size to 42. The current internal solution
> is to use the cpuid, but that is quite ugly. I was thinking of using
> the model
> as indication to right the ipa_size, but I am not too sure of the ACPI
> side. Would it be okay to add an APM MMU500 variant? I would also
> appreciated it if you guys have any alternate solutions.

This is something we've been axpecting to run into for a while now - the
appropriate solution is to use a "dma-ranges" property on the master
device(s) to describe that they have 42 bits of address wired up, from
which they will then inherit the appropriate DMA mask. The outstanding
issue which remains is that we're still missing some way of preventing
drivers simply clobbering that with a 64-bit mask later, but that's a
more general problem[1].

Robin.

[1]:https://www.mail-archive.com/linux-renesas-soc at vger.kernel.org/msg10209.html

> 
> Thanks
> Feng Kan
> 

^ permalink raw reply

* [RFC PATCH] arm64: defconfig: enable SMMUv3 config
From: Zhou Wang @ 2017-01-09 11:50 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 869dded..3520c50 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -440,6 +440,7 @@ CONFIG_PLATFORM_MHU=y
 CONFIG_BCM2835_MBOX=y
 CONFIG_HI6220_MBOX=y
 CONFIG_ARM_SMMU=y
+CONFIG_ARM_SMMU_V3=y
 CONFIG_RASPBERRYPI_POWER=y
 CONFIG_QCOM_SMEM=y
 CONFIG_QCOM_SMD=y
-- 
1.9.1

^ permalink raw reply related

* [QUESTION] Early Write Acknowledge for PCIe configuration space
From: Arnd Bergmann @ 2017-01-09 11:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e085504e-74fd-e8be-8287-4baef88551c9@huawei.com>

On Monday, January 9, 2017 10:59:47 AM CET John Garry wrote:
> On 06/01/2017 11:24, Arnd Bergmann wrote:
> > On Friday, January 6, 2017 11:15:22 AM CET John Garry wrote:
> >
> > Probably nobody thought about this properly in the Linux drivers. The
> > ARMv8 ARM sounds correct here.
> >
> > I/O space may have the same issue, as it also requires non-posted
> > accesses.
> 
> Right, so our HW team's recommendation - from ARM's memory model and 
> also PCIe order model - is that not only config space but also PCIe 
> memory mapped IO has the same attribute (nE).

Just to be sure we are talking about the same thing: "PCIe memory
mapped IO" could refer to either PCI I/O space or PCI memory space.

As far as I can tell, PCI memory space should *not* be using the nE
attribute, while PCI I/O space and PCI config space should.
Does this match what your HW team recomments?

	Arnd

^ permalink raw reply

* [PATCH v3] arm64: mm: Fix NOMAP page initialization
From: Robert Richter @ 2017-01-09 11:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKv+Gu8-+0LUTN0+8OGWRhd22Ls5cMQqTJcjKQK_0N=Uc-0jog@mail.gmail.com>

On 06.01.17 08:37:25, Ard Biesheuvel wrote:
> Any comments on the performance impact (including boot time) ?

I did a kernel compile test and kernel mode time increases by about
2.2%. Though this is already significant, we need a more suitable mem
benchmark here for further testing.

For boot time I dont see significant changes.

-Robert


Boot times:

pfn_valid_within():
[   25.929134]
[   25.548830]
[   25.503225]

early_pfn_valid() v3:
[   25.773814]
[   25.548428]
[   25.765290]


Kernel compile times (3 runs each):

pfn_valid_within():

real    6m4.088s
user    372m57.607s
sys     16m55.158s

real    6m1.532s
user    372m48.453s
sys     16m50.370s

real    6m4.061s
user    373m18.753s
sys     16m57.027s


early_pfn_valid() v3:

real    6m3.261s
user    373m15.816s
sys     16m30.019s

real    6m2.980s
user    373m9.019s
sys     16m32.992s

real    6m2.574s
user    372m45.146s
sys     16m33.218s

^ permalink raw reply

* [RFC 2/8] KVM: arm/arm64: Decouple kvm timer functions from virtual timer
From: Christoffer Dall @ 2017-01-09 11:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482772326-29110-3-git-send-email-jintack@cs.columbia.edu>

On Mon, Dec 26, 2016 at 12:12:00PM -0500, Jintack Lim wrote:
> Now that we have a separate structure for timer context, make functions
> general so that they can work on any timer context, not just the virtual
> timer context.  This does not change the virtual timer functionality.
> 
> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> ---
>  arch/arm/kvm/arm.c           |  2 +-
>  include/kvm/arm_arch_timer.h |  3 +-
>  virt/kvm/arm/arch_timer.c    | 65 +++++++++++++++++++++++++-------------------
>  3 files changed, 40 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 19b5f5c..37d1623 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -295,7 +295,7 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
>  
>  int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
>  {
> -	return kvm_timer_should_fire(vcpu);
> +	return kvm_timer_should_fire(vcpu, vcpu_vtimer(vcpu));
>  }
>  
>  void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu)
> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
> index 7dabe56..cf84145 100644
> --- a/include/kvm/arm_arch_timer.h
> +++ b/include/kvm/arm_arch_timer.h
> @@ -69,7 +69,8 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
>  u64 kvm_arm_timer_get_reg(struct kvm_vcpu *, u64 regid);
>  int kvm_arm_timer_set_reg(struct kvm_vcpu *, u64 regid, u64 value);
>  
> -bool kvm_timer_should_fire(struct kvm_vcpu *vcpu);
> +bool kvm_timer_should_fire(struct kvm_vcpu *vcpu,
> +			   struct arch_timer_context *timer_ctx);
>  void kvm_timer_schedule(struct kvm_vcpu *vcpu);
>  void kvm_timer_unschedule(struct kvm_vcpu *vcpu);
>  
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 30a64df..3bd6063 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -91,7 +91,7 @@ static void kvm_timer_inject_irq_work(struct work_struct *work)
>  	vcpu = container_of(work, struct kvm_vcpu, arch.timer_cpu.expired);
>  	vcpu->arch.timer_cpu.armed = false;
>  
> -	WARN_ON(!kvm_timer_should_fire(vcpu));
> +	WARN_ON(!kvm_timer_should_fire(vcpu, vcpu_vtimer(vcpu)));
>  
>  	/*
>  	 * If the vcpu is blocked we want to wake it up so that it will see
> @@ -100,12 +100,22 @@ static void kvm_timer_inject_irq_work(struct work_struct *work)
>  	kvm_vcpu_kick(vcpu);
>  }
>  
> -static u64 kvm_timer_compute_delta(struct kvm_vcpu *vcpu)
> +static u64 kvm_timer_cntvoff(struct kvm_vcpu *vcpu,
> +			     struct arch_timer_context *timer_ctx)
> +{
> +	if (timer_ctx == vcpu_vtimer(vcpu))
> +		return vcpu->kvm->arch.timer.cntvoff;
> +
> +	return 0;
> +}
> +
> +static u64 kvm_timer_compute_delta(struct kvm_vcpu *vcpu,
> +				   struct arch_timer_context *timer_ctx)
>  {
>  	cycle_t cval, now;
>  
> -	cval = vcpu_vtimer(vcpu)->cnt_cval;
> -	now = kvm_phys_timer_read() - vcpu->kvm->arch.timer.cntvoff;
> +	cval = timer_ctx->cnt_cval;
> +	now = kvm_phys_timer_read() - kvm_timer_cntvoff(vcpu, timer_ctx);
>  
>  	if (now < cval) {
>  		u64 ns;
> @@ -134,7 +144,7 @@ static enum hrtimer_restart kvm_timer_expire(struct hrtimer *hrt)
>  	 * PoV (NTP on the host may have forced it to expire
>  	 * early). If we should have slept longer, restart it.
>  	 */
> -	ns = kvm_timer_compute_delta(vcpu);
> +	ns = kvm_timer_compute_delta(vcpu, vcpu_vtimer(vcpu));
>  	if (unlikely(ns)) {
>  		hrtimer_forward_now(hrt, ns_to_ktime(ns));
>  		return HRTIMER_RESTART;
> @@ -144,42 +154,40 @@ static enum hrtimer_restart kvm_timer_expire(struct hrtimer *hrt)
>  	return HRTIMER_NORESTART;
>  }
>  
> -static bool kvm_timer_irq_can_fire(struct kvm_vcpu *vcpu)
> +static bool kvm_timer_irq_can_fire(struct arch_timer_context *timer_ctx)
>  {
> -	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> -
> -	return !(vtimer->cnt_ctl & ARCH_TIMER_CTRL_IT_MASK) &&
> -		(vtimer->cnt_ctl & ARCH_TIMER_CTRL_ENABLE);
> +	return !(timer_ctx->cnt_ctl & ARCH_TIMER_CTRL_IT_MASK) &&
> +		(timer_ctx->cnt_ctl & ARCH_TIMER_CTRL_ENABLE);
>  }
>  
> -bool kvm_timer_should_fire(struct kvm_vcpu *vcpu)
> +bool kvm_timer_should_fire(struct kvm_vcpu *vcpu,
> +			   struct arch_timer_context *timer_ctx)
>  {
> -	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
>  	cycle_t cval, now;
>  
> -	if (!kvm_timer_irq_can_fire(vcpu))
> +	if (!kvm_timer_irq_can_fire(timer_ctx))
>  		return false;
>  
> -	cval = vtimer->cnt_cval;
> -	now = kvm_phys_timer_read() - vcpu->kvm->arch.timer.cntvoff;
> +	cval = timer_ctx->cnt_cval;
> +	now = kvm_phys_timer_read() - kvm_timer_cntvoff(vcpu, timer_ctx);
>  

I'm wondering if we should move the cntvoff field to the
arch_timer_context struct so that all these functions can just take
the timer_ctx pointer without a vcpu pointer?

(That would pave the way for ever doing adjustments of the cntvoff on a
per-CPU basis if that should ever make sense, which it maybe won't but
still...)

The challenge would be to ensure that the cntvoff stays in sync between
all the contexts when being modified from userspace and during init.

Thoughts?

Thanks,
-Christoffer

^ permalink raw reply

* [PATCH v2 1/7] arm: put types.h in uapi
From: Russell King - ARM Linux @ 2017-01-09 12:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1990589.0aJHWbJK4F@wuerfel>

On Mon, Jan 09, 2017 at 12:33:02PM +0100, Arnd Bergmann wrote:
> On Friday, January 6, 2017 10:43:53 AM CET Nicolas Dichtel wrote:
> > 
> > diff --git a/arch/arm/include/asm/types.h b/arch/arm/include/asm/types.h
> > index a53cdb8f068c..c48fee3d7b3b 100644
> > --- a/arch/arm/include/asm/types.h
> > +++ b/arch/arm/include/asm/types.h
> > @@ -1,40 +1,6 @@
> >  #ifndef _ASM_TYPES_H
> >  #define _ASM_TYPES_H
> >  
> > -#include <asm-generic/int-ll64.h>
> ...
> > -#define __UINTPTR_TYPE__       unsigned long
> > -#endif
> > +#include <uapi/asm/types.h>
> >  
> >  #endif /* _ASM_TYPES_H */
> > 
> 
> Moving the file is correct as far as I can tell, but the extra
> #include is not necessary here, as the kernel will automatically
> search both arch/arm/include/ and arch/arm/include/uapi/.

Indeed, I'd like to see the include/asm file gone.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [PATCH v2 7/7] uapi: export all headers under uapi directories
From: Russell King - ARM Linux @ 2017-01-09 12:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483695839-18660-8-git-send-email-nicolas.dichtel@6wind.com>

On Fri, Jan 06, 2017 at 10:43:59AM +0100, Nicolas Dichtel wrote:
> diff --git a/arch/arm/include/uapi/asm/Kbuild b/arch/arm/include/uapi/asm/Kbuild
> index 46a76cd6acb6..607f702c2d62 100644
> --- a/arch/arm/include/uapi/asm/Kbuild
> +++ b/arch/arm/include/uapi/asm/Kbuild
> @@ -1,23 +1,6 @@
>  # UAPI Header export list
>  include include/uapi/asm-generic/Kbuild.asm
>  
> -header-y += auxvec.h
> -header-y += byteorder.h
> -header-y += fcntl.h
> -header-y += hwcap.h
> -header-y += ioctls.h
> -header-y += kvm_para.h
> -header-y += mman.h
> -header-y += perf_regs.h
> -header-y += posix_types.h
> -header-y += ptrace.h
> -header-y += setup.h
> -header-y += sigcontext.h
> -header-y += signal.h
> -header-y += stat.h
> -header-y += statfs.h
> -header-y += swab.h
> -header-y += unistd.h
>  genhdr-y += unistd-common.h
>  genhdr-y += unistd-oabi.h
>  genhdr-y += unistd-eabi.h

Acked-by: Russell King <rmk+kernel@armlinux.org.uk>

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [RFC 4/8] KVM: arm/arm64: Initialize the emulated EL1 physical timer
From: Christoffer Dall @ 2017-01-09 12:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482772326-29110-5-git-send-email-jintack@cs.columbia.edu>

On Mon, Dec 26, 2016 at 12:12:02PM -0500, Jintack Lim wrote:
> Initialize the emulated EL1 physical timer with the default irq number.
> 
> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> ---
>  arch/arm/kvm/reset.c         |  9 ++++++++-
>  arch/arm64/kvm/reset.c       |  9 ++++++++-
>  include/kvm/arm_arch_timer.h |  3 ++-
>  virt/kvm/arm/arch_timer.c    | 12 ++++++++++--
>  4 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/kvm/reset.c b/arch/arm/kvm/reset.c
> index 4b5e802..1da8b2d 100644
> --- a/arch/arm/kvm/reset.c
> +++ b/arch/arm/kvm/reset.c
> @@ -37,6 +37,11 @@
>  	.usr_regs.ARM_cpsr = SVC_MODE | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT,
>  };
>  
> +static const struct kvm_irq_level cortexa_ptimer_irq = {
> +	{ .irq = 30 },
> +	.level = 1,
> +};
> +
>  static const struct kvm_irq_level cortexa_vtimer_irq = {
>  	{ .irq = 27 },
>  	.level = 1,
> @@ -58,6 +63,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_regs *reset_regs;
>  	const struct kvm_irq_level *cpu_vtimer_irq;
> +	const struct kvm_irq_level *cpu_ptimer_irq;
>  
>  	switch (vcpu->arch.target) {
>  	case KVM_ARM_TARGET_CORTEX_A7:
> @@ -65,6 +71,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>  		reset_regs = &cortexa_regs_reset;
>  		vcpu->arch.midr = read_cpuid_id();
>  		cpu_vtimer_irq = &cortexa_vtimer_irq;
> +		cpu_ptimer_irq = &cortexa_ptimer_irq;
>  		break;
>  	default:
>  		return -ENODEV;
> @@ -77,5 +84,5 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>  	kvm_reset_coprocs(vcpu);
>  
>  	/* Reset arch_timer context */
> -	return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
> +	return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq, cpu_ptimer_irq);
>  }
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 5bc4608..74322c2 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -46,6 +46,11 @@
>  			COMPAT_PSR_I_BIT | COMPAT_PSR_F_BIT),
>  };
>  
> +static const struct kvm_irq_level default_ptimer_irq = {
> +	.irq	= 30,
> +	.level	= 1,
> +};
> +
>  static const struct kvm_irq_level default_vtimer_irq = {
>  	.irq	= 27,
>  	.level	= 1,
> @@ -110,6 +115,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
>  int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>  {
>  	const struct kvm_irq_level *cpu_vtimer_irq;
> +	const struct kvm_irq_level *cpu_ptimer_irq;
>  	const struct kvm_regs *cpu_reset;
>  
>  	switch (vcpu->arch.target) {
> @@ -123,6 +129,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>  		}
>  
>  		cpu_vtimer_irq = &default_vtimer_irq;
> +		cpu_ptimer_irq = &default_ptimer_irq;
>  		break;
>  	}
>  
> @@ -136,5 +143,5 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>  	kvm_pmu_vcpu_reset(vcpu);
>  
>  	/* Reset timer */
> -	return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
> +	return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq, cpu_ptimer_irq);
>  }
> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
> index d21652a..04ed9c1 100644
> --- a/include/kvm/arm_arch_timer.h
> +++ b/include/kvm/arm_arch_timer.h
> @@ -61,7 +61,8 @@ struct arch_timer_cpu {
>  int kvm_timer_enable(struct kvm_vcpu *vcpu);
>  void kvm_timer_init(struct kvm *kvm);
>  int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> -			 const struct kvm_irq_level *irq);
> +			 const struct kvm_irq_level *virt_irq,
> +			 const struct kvm_irq_level *phys_irq);
>  void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu);
>  void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu);
>  void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu);
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 3bd6063..ed80864 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -339,9 +339,11 @@ void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
>  }
>  
>  int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> -			 const struct kvm_irq_level *irq)
> +			 const struct kvm_irq_level *virt_irq,
> +			 const struct kvm_irq_level *phys_irq)
>  {
>  	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> +	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>  
>  	/*
>  	 * The vcpu timer irq number cannot be determined in
> @@ -349,7 +351,8 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
>  	 * kvm_vcpu_set_target(). To handle this, we determine
>  	 * vcpu timer irq number when the vcpu is reset.
>  	 */
> -	vtimer->irq.irq = irq->irq;
> +	vtimer->irq.irq = virt_irq->irq;
> +	ptimer->irq.irq = phys_irq->irq;
>  
>  	/*
>  	 * The bits in CNTV_CTL are architecturally reset to UNKNOWN for ARMv8
> @@ -358,6 +361,7 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
>  	 * the ARMv7 architecture.
>  	 */
>  	vtimer->cnt_ctl = 0;
> +	ptimer->cnt_ctl = 0;
>  	kvm_timer_update_state(vcpu);
>  
>  	return 0;
> @@ -477,11 +481,15 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu)
>  int kvm_timer_enable(struct kvm_vcpu *vcpu)
>  {
>  	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> +	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>  	struct irq_desc *desc;
>  	struct irq_data *data;
>  	int phys_irq;
>  	int ret;
>  
> +	/* Always enable emulated the EL1 physical timer */

Dubious comment the way it stands.

Does the rest of the code really support one timer being enabled while
another one not so?

In any case, the comment should explain the rationale as opposed to
reiterate what the following code line does, or just not be there.

> +	ptimer->enabled = 1;
> +
>  	if (vtimer->enabled)
>  		return 0;
>  
> -- 
> 1.9.1
> 
> 

Thanks,
-Christoffer

^ permalink raw reply

* APM smmu implementation
From: Will Deacon @ 2017-01-09 12:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <d9e40de8-82e2-0f19-5b99-d756ded21e80@arm.com>

On Mon, Jan 09, 2017 at 11:34:42AM +0000, Robin Murphy wrote:
> On 06/01/17 23:21, Feng Kan wrote:
> > The APM IOMMU implementation is mostly just the ARM SMMU 500 variant.
> 
> "Mostly"? Have APM actually modified it (which I strongly doubt) or do
> you mean it's simply been integrated with the upper address lines tied
> off? MMU-500 reports a 48-bit IAS because MMU-500 has 48-bit-wide slave
> interfaces; that's all there is to it. Whether or not you use all of
> those bits is up to you as a system integrator.

That's a good point; MMU-500 doesn't appear to let you change the IAS
anyway. That should also mean that UBS and OAS are unchanged.

> > However, our internal bus is only 42 bits wide. Our IAS field is coded
> > as 48 bits, which cause IPA to truncated to 42 bits on the physical
> > bus. In order for our system to work with the arm-smmu.c, there needs
> > to be a way to force the ipa_size to 42. The current internal solution
> > is to use the cpuid, but that is quite ugly. I was thinking of using
> > the model
> > as indication to right the ipa_size, but I am not too sure of the ACPI
> > side. Would it be okay to add an APM MMU500 variant? I would also
> > appreciated it if you guys have any alternate solutions.
> 
> This is something we've been axpecting to run into for a while now - the
> appropriate solution is to use a "dma-ranges" property on the master
> device(s) to describe that they have 42 bits of address wired up, from
> which they will then inherit the appropriate DMA mask. The outstanding
> issue which remains is that we're still missing some way of preventing
> drivers simply clobbering that with a 64-bit mask later, but that's a
> more general problem[1].

I wonder if the driver is actually using IAS, OAS and UBS incorrectly.
We're using them to parameterise the DMA aperture, which is then used
to size the IOVA domain, but that's wrong because the IAS, OAS and UBS
are upper bounds and we can still end up allocating unusable/unreachable
addresses.

So I do think that this should be fixed on the SMMU firmware node, rather
than restricting the range of each master device.

Will

^ permalink raw reply

* [PATCH v3 4/9] arm64: cpufeature: Document the rules of safe value for features
From: Catalin Marinas @ 2017-01-09 12:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2338875a-6a5b-ba9e-59fe-d681091c2923@arm.com>

On Mon, Jan 09, 2017 at 10:43:07AM +0000, Suzuki K. Poulose wrote:
> On 06/01/17 12:30, Catalin Marinas wrote:
> >On Wed, Jan 04, 2017 at 05:49:02PM +0000, Suzuki K. Poulose wrote:
> >>--- a/arch/arm64/include/asm/cpufeature.h
> >>+++ b/arch/arm64/include/asm/cpufeature.h
> >>@@ -29,7 +29,21 @@
> >> #include <linux/jump_label.h>
> >> #include <linux/kernel.h>
> >>
> >>-/* CPU feature register tracking */
> >>+/*
> >>+ * CPU feature register tracking
> >>+ *
> >>+ * The safe value of a CPUID feature field is dependent on the implications
> >>+ * of the values assigned to it by the architecture. Based on the relationship
> >>+ * between the values, the features are classified into 3 types.
> >>+ *
> >>+ * a) LOWER_SAFE - The value 'n+1' indicates, value 'n' and some
> >>+ *    additional features. (where n >= 0). The smaller value (n) is
> >>+ *    considered safer in this case.
> >>+ * b) HIGHER_SAFE - The value 'n+1' is safer than 'n' (for n>= 0).
> >>+ * c) EXACT - If the values of the feature don't have any relationship,
> >>+ *    a predefined safe value is used.
> >>+ */
> >
> >I don't think this text fully describes what is actually compared. You
> >could say something that the lowest value of all the CPUs is chosen for
> >LOWER_SAFE, highest for HIGHER_SAFE and it is expected that all CPUs
> >have the same value for a field when EXACT is specified.
> 
> Ok. I have changed it as below :
> 
> /*
>  * CPU feature register tracking
>  *
>  * The safe value of a CPUID feature field is dependent on the implications
>  * of the values assigned to it by the architecture. Based on the relationship
>  * between the values, the features are classified into 3 types - LOWER_SAFE,
>  * HIGHER_SAFE and EXACT.
>  *
>  * The lowest value of all the CPUs is chosen for LOWER_SAFE and highest
>  * for HIGHER_SAFE. It is expected that all CPUs have the same value for
>  * a field when EXACT is specified, failing which, the safe value specified
>  * in the table is chosen.
>  */

It looks better to me. Thanks.

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply

* [PATCH] [media] atmel-isc: add the isc pipeline function
From: Hans Verkuil @ 2017-01-09 12:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161223092420.30466-1-songjun.wu@microchip.com>

On 12/23/2016 10:24 AM, Songjun Wu wrote:
> Image Sensor Controller has an internal image processor.
> It can convert raw format to the other formats, like
> RGB565, YUV420P. A module parameter 'sensor_preferred'
> is used to enable or disable the pipeline function.
> Some v4l2 controls are added to tuning the image when
> the pipeline function is enabled.
> 
> Signed-off-by: Songjun Wu <songjun.wu@microchip.com>
> ---
> 
>  drivers/media/platform/atmel/atmel-isc-regs.h | 102 ++++-
>  drivers/media/platform/atmel/atmel-isc.c      | 629 +++++++++++++++++++++-----
>  2 files changed, 623 insertions(+), 108 deletions(-)
> 
> diff --git a/drivers/media/platform/atmel/atmel-isc-regs.h b/drivers/media/platform/atmel/atmel-isc-regs.h
> index 00c449717cde..6936ac467609 100644
> --- a/drivers/media/platform/atmel/atmel-isc-regs.h
> +++ b/drivers/media/platform/atmel/atmel-isc-regs.h
> @@ -65,6 +65,7 @@
>  #define ISC_INTSR       0x00000034
>  
>  #define ISC_INT_DDONE		BIT(8)
> +#define ISC_INT_HISDONE		BIT(12)
>  
>  /* ISC White Balance Control Register */
>  #define ISC_WB_CTRL     0x00000058
> @@ -72,30 +73,98 @@
>  /* ISC White Balance Configuration Register */
>  #define ISC_WB_CFG      0x0000005c
>  
> +/* ISC White Balance Offset for R, GR Register */
> +#define ISC_WB_O_RGR	0x00000060
> +
> +/* ISC White Balance Offset for B, GB Register */
> +#define ISC_WB_O_BGR	0x00000064
> +
> +/* ISC White Balance Gain for R, GR Register */
> +#define ISC_WB_G_RGR	0x00000068
> +
> +/* ISC White Balance Gain for B, GB Register */
> +#define ISC_WB_G_BGR	0x0000006c
> +
>  /* ISC Color Filter Array Control Register */
>  #define ISC_CFA_CTRL    0x00000070
>  
>  /* ISC Color Filter Array Configuration Register */
>  #define ISC_CFA_CFG     0x00000074
> +#define ISC_CFA_CFG_EITPOL	BIT(4)
>  
>  #define ISC_BAY_CFG_GRGR	0x0
>  #define ISC_BAY_CFG_RGRG	0x1
>  #define ISC_BAY_CFG_GBGB	0x2
>  #define ISC_BAY_CFG_BGBG	0x3
> -#define ISC_BAY_CFG_MASK	GENMASK(1, 0)
>  
>  /* ISC Color Correction Control Register */
>  #define ISC_CC_CTRL     0x00000078
>  
> +/* ISC Color Correction RR RG Register */
> +#define ISC_CC_RR_RG	0x0000007c
> +
> +/* ISC Color Correction RB OR Register */
> +#define ISC_CC_RB_OR	0x00000080
> +
> +/* ISC Color Correction GR GG Register */
> +#define ISC_CC_GR_GG	0x00000084
> +
> +/* ISC Color Correction GB OG Register */
> +#define ISC_CC_GB_OG	0x00000088
> +
> +/* ISC Color Correction BR BG Register */
> +#define ISC_CC_BR_BG	0x0000008c
> +
> +/* ISC Color Correction BB OB Register */
> +#define ISC_CC_BB_OB	0x00000090
> +
>  /* ISC Gamma Correction Control Register */
>  #define ISC_GAM_CTRL    0x00000094
>  
> +/* ISC_Gamma Correction Blue Entry Register */
> +#define ISC_GAM_BENTRY	0x00000098
> +
> +/* ISC_Gamma Correction Green Entry Register */
> +#define ISC_GAM_GENTRY	0x00000198
> +
> +/* ISC_Gamma Correction Green Entry Register */
> +#define ISC_GAM_RENTRY	0x00000298
> +
>  /* Color Space Conversion Control Register */
>  #define ISC_CSC_CTRL    0x00000398
>  
> +/* Color Space Conversion YR YG Register */
> +#define ISC_CSC_YR_YG	0x0000039c
> +
> +/* Color Space Conversion YB OY Register */
> +#define ISC_CSC_YB_OY	0x000003a0
> +
> +/* Color Space Conversion CBR CBG Register */
> +#define ISC_CSC_CBR_CBG	0x000003a4
> +
> +/* Color Space Conversion CBB OCB Register */
> +#define ISC_CSC_CBB_OCB	0x000003a8
> +
> +/* Color Space Conversion CRR CRG Register */
> +#define ISC_CSC_CRR_CRG	0x000003ac
> +
> +/* Color Space Conversion CRB OCR Register */
> +#define ISC_CSC_CRB_OCR	0x000003b0
> +
>  /* Contrast And Brightness Control Register */
>  #define ISC_CBC_CTRL    0x000003b4
>  
> +/* Contrast And Brightness Configuration Register */
> +#define ISC_CBC_CFG	0x000003b8
> +
> +/* Brightness Register */
> +#define ISC_CBC_BRIGHT	0x000003bc
> +#define ISC_CBC_BRIGHT_MASK	GENMASK(10, 0)
> +
> +/* Contrast Register */
> +#define ISC_CBC_CONTRAST	0x000003c0
> +#define ISC_CBC_CONTRAST_MASK	GENMASK(11, 0)
> +
>  /* Subsampling 4:4:4 to 4:2:2 Control Register */
>  #define ISC_SUB422_CTRL 0x000003c4
>  
> @@ -120,6 +189,27 @@
>  #define ISC_RLP_CFG_MODE_YYCC_LIMITED   0xc
>  #define ISC_RLP_CFG_MODE_MASK           GENMASK(3, 0)
>  
> +/* Histogram Control Register */
> +#define ISC_HIS_CTRL	0x000003d4
> +
> +#define ISC_HIS_CTRL_EN			BIT(0)
> +#define ISC_HIS_CTRL_DIS		0x0
> +
> +/* Histogram Configuration Register */
> +#define ISC_HIS_CFG	0x000003d8
> +
> +#define ISC_HIS_CFG_MODE_GR		0x0
> +#define ISC_HIS_CFG_MODE_R		0x1
> +#define ISC_HIS_CFG_MODE_GB		0x2
> +#define ISC_HIS_CFG_MODE_B		0x3
> +#define ISC_HIS_CFG_MODE_Y		0x4
> +#define ISC_HIS_CFG_MODE_RAW		0x5
> +#define ISC_HIS_CFG_MODE_YCCIR656	0x6
> +
> +#define ISC_HIS_CFG_BAYSEL_SHIFT	4
> +
> +#define ISC_HIS_CFG_RAR			BIT(8)
> +
>  /* DMA Configuration Register */
>  #define ISC_DCFG        0x000003e0
>  #define ISC_DCFG_IMODE_PACKED8          0x0
> @@ -159,7 +249,13 @@
>  /* DMA Address 0 Register */
>  #define ISC_DAD0        0x000003ec
>  
> -/* DMA Stride 0 Register */
> -#define ISC_DST0        0x000003f0
> +/* DMA Address 1 Register */
> +#define ISC_DAD1        0x000003f4
> +
> +/* DMA Address 2 Register */
> +#define ISC_DAD2        0x000003fc
> +
> +/* Histogram Entry */
> +#define ISC_HIS_ENTRY	0x00000410
>  
>  #endif
> diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
> index fa68fe912c95..ff526022e103 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -29,6 +29,7 @@
>  #include <linux/clk-provider.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
> +#include <linux/math64.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> @@ -36,7 +37,9 @@
>  #include <linux/regmap.h>
>  #include <linux/videodev2.h>
>  
> +#include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
> +#include <media/v4l2-event.h>
>  #include <media/v4l2-image-sizes.h>
>  #include <media/v4l2-ioctl.h>
>  #include <media/v4l2-of.h>
> @@ -89,10 +92,12 @@ struct isc_subdev_entity {
>   * struct isc_format - ISC media bus format information
>   * @fourcc:		Fourcc code for this format
>   * @mbus_code:		V4L2 media bus format code.
> - * @bpp:		Bytes per pixel (when stored in memory)
> + * @bpp:		Bits per pixel (when stored in memory)
>   * @reg_bps:		reg value for bits per sample
>   *			(when transferred over a bus)
> - * @support:		Indicates format supported by subdev
> + * @pipeline:		pipeline switch
> + * @sd_support:		Subdev supports this format
> + * @isc_support:	ISC can convert raw format to this format
>   */
>  struct isc_format {
>  	u32	fourcc;
> @@ -100,11 +105,42 @@ struct isc_format {
>  	u8	bpp;
>  
>  	u32	reg_bps;
> +	u32	reg_bay_cfg;
>  	u32	reg_rlp_mode;
>  	u32	reg_dcfg_imode;
>  	u32	reg_dctrl_dview;
>  
> -	bool	support;
> +	u32	pipeline;
> +
> +	bool	sd_support;
> +	bool	isc_support;
> +};
> +
> +
> +#define HIST_ENTRIES		512
> +#define HIST_BAYER		(ISC_HIS_CFG_MODE_B + 1)
> +
> +enum{
> +	HIST_INIT = 0,
> +	HIST_ENABLED,
> +	HIST_DISABLED,
> +};
> +
> +struct isc_ctrls {
> +	struct v4l2_ctrl_handler handler;
> +
> +	u32 brightness;
> +	u32 contrast;
> +	u8 gamma_index;
> +	u8 awb;
> +
> +	u32 r_gain;
> +	u32 b_gain;
> +
> +	u32 hist_entry[HIST_ENTRIES];
> +	u32 hist_count[HIST_BAYER];
> +	u8 hist_id;
> +	u8 hist_stat;
>  };
>  
>  #define ISC_PIPE_LINE_NODE_NUM	11
> @@ -131,6 +167,10 @@ struct isc_device {
>  	struct isc_format	**user_formats;
>  	unsigned int		num_user_formats;
>  	const struct isc_format	*current_fmt;
> +	const struct isc_format	*raw_fmt;
> +
> +	struct isc_ctrls	ctrls;
> +	struct work_struct	awb_work;
>  
>  	struct mutex		lock;
>  
> @@ -140,51 +180,134 @@ struct isc_device {
>  	struct list_head		subdev_entities;
>  };
>  
> +#define RAW_FMT_INDEX_START	0
> +#define RAW_FMT_INDEX_END	11
> +#define ISC_FMT_INDEX_START	12
> +#define ISC_FMT_INDEX_END	14
> +
>  static struct isc_format isc_formats[] = {
> -	{ V4L2_PIX_FMT_SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8,
> -	  1, ISC_PFE_CFG0_BPS_EIGHT, ISC_RLP_CFG_MODE_DAT8,
> -	  ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, false },
> -	{ V4L2_PIX_FMT_SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8,
> -	  1, ISC_PFE_CFG0_BPS_EIGHT, ISC_RLP_CFG_MODE_DAT8,
> -	  ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, false },
> -	{ V4L2_PIX_FMT_SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8,
> -	  1, ISC_PFE_CFG0_BPS_EIGHT, ISC_RLP_CFG_MODE_DAT8,
> -	  ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, false },
> -	{ V4L2_PIX_FMT_SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8,
> -	  1, ISC_PFE_CFG0_BPS_EIGHT, ISC_RLP_CFG_MODE_DAT8,
> -	  ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, false },
> -
> -	{ V4L2_PIX_FMT_SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10,
> -	  2, ISC_PFG_CFG0_BPS_TEN, ISC_RLP_CFG_MODE_DAT10,
> -	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, false },
> -	{ V4L2_PIX_FMT_SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10,
> -	  2, ISC_PFG_CFG0_BPS_TEN, ISC_RLP_CFG_MODE_DAT10,
> -	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, false },
> -	{ V4L2_PIX_FMT_SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10,
> -	  2, ISC_PFG_CFG0_BPS_TEN, ISC_RLP_CFG_MODE_DAT10,
> -	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, false },
> -	{ V4L2_PIX_FMT_SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10,
> -	  2, ISC_PFG_CFG0_BPS_TEN, ISC_RLP_CFG_MODE_DAT10,
> -	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, false },
> -
> -	{ V4L2_PIX_FMT_SBGGR12, MEDIA_BUS_FMT_SBGGR12_1X12,
> -	  2, ISC_PFG_CFG0_BPS_TWELVE, ISC_RLP_CFG_MODE_DAT12,
> -	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, false },
> -	{ V4L2_PIX_FMT_SGBRG12, MEDIA_BUS_FMT_SGBRG12_1X12,
> -	  2, ISC_PFG_CFG0_BPS_TWELVE, ISC_RLP_CFG_MODE_DAT12,
> -	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, false },
> -	{ V4L2_PIX_FMT_SGRBG12, MEDIA_BUS_FMT_SGRBG12_1X12,
> -	  2, ISC_PFG_CFG0_BPS_TWELVE, ISC_RLP_CFG_MODE_DAT12,
> -	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, false },
> -	{ V4L2_PIX_FMT_SRGGB12, MEDIA_BUS_FMT_SRGGB12_1X12,
> -	  2, ISC_PFG_CFG0_BPS_TWELVE, ISC_RLP_CFG_MODE_DAT12,
> -	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, false },
> -
> -	{ V4L2_PIX_FMT_YUYV, MEDIA_BUS_FMT_YUYV8_2X8,
> -	  2, ISC_PFE_CFG0_BPS_EIGHT, ISC_RLP_CFG_MODE_DAT8,
> -	  ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, false },
> +	{ V4L2_PIX_FMT_SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8, 8,
> +	  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DAT8,
> +	  ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
> +	  false, false },
> +	{ V4L2_PIX_FMT_SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8, 8,
> +	  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_GBGB, ISC_RLP_CFG_MODE_DAT8,
> +	  ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
> +	  false, false },
> +	{ V4L2_PIX_FMT_SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8, 8,
> +	  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_GRGR, ISC_RLP_CFG_MODE_DAT8,
> +	  ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
> +	  false, false },
> +	{ V4L2_PIX_FMT_SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8, 8,
> +	  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_RGRG, ISC_RLP_CFG_MODE_DAT8,
> +	  ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
> +	  false, false },
> +
> +	{ V4L2_PIX_FMT_SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10, 16,
> +	  ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DAT10,
> +	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
> +	  false, false },
> +	{ V4L2_PIX_FMT_SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10, 16,
> +	  ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_GBGB, ISC_RLP_CFG_MODE_DAT10,
> +	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
> +	  false, false },
> +	{ V4L2_PIX_FMT_SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10, 16,
> +	  ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_GRGR, ISC_RLP_CFG_MODE_DAT10,
> +	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
> +	  false, false },
> +	{ V4L2_PIX_FMT_SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10, 16,
> +	  ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_RGRG, ISC_RLP_CFG_MODE_DAT10,
> +	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
> +	  false, false },
> +
> +	{ V4L2_PIX_FMT_SBGGR12, MEDIA_BUS_FMT_SBGGR12_1X12, 16,
> +	  ISC_PFG_CFG0_BPS_TWELVE, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DAT12,
> +	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
> +	  false, false },
> +	{ V4L2_PIX_FMT_SGBRG12, MEDIA_BUS_FMT_SGBRG12_1X12, 16,
> +	  ISC_PFG_CFG0_BPS_TWELVE, ISC_BAY_CFG_GBGB, ISC_RLP_CFG_MODE_DAT12,
> +	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
> +	  false, false },
> +	{ V4L2_PIX_FMT_SGRBG12, MEDIA_BUS_FMT_SGRBG12_1X12, 16,
> +	  ISC_PFG_CFG0_BPS_TWELVE, ISC_BAY_CFG_GRGR, ISC_RLP_CFG_MODE_DAT12,
> +	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
> +	  false, false },
> +	{ V4L2_PIX_FMT_SRGGB12, MEDIA_BUS_FMT_SRGGB12_1X12, 16,
> +	  ISC_PFG_CFG0_BPS_TWELVE, ISC_BAY_CFG_RGRG, ISC_RLP_CFG_MODE_DAT12,
> +	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
> +	  false, false },
> +
> +	{ V4L2_PIX_FMT_YUV420, 0x0, 12,
> +	  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_YYCC,
> +	  ISC_DCFG_IMODE_YC420P | ISC_DCFG_YMBSIZE_BEATS8 |
> +	  ISC_DCFG_CMBSIZE_BEATS8, ISC_DCTRL_DVIEW_PLANAR, 0x7fb,
> +	  false, false },
> +	{ V4L2_PIX_FMT_YUV422P, 0x0, 16,
> +	  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_YYCC,
> +	  ISC_DCFG_IMODE_YC422P | ISC_DCFG_YMBSIZE_BEATS8 |
> +	  ISC_DCFG_CMBSIZE_BEATS8, ISC_DCTRL_DVIEW_PLANAR, 0x3fb,
> +	  false, false },
> +	{ V4L2_PIX_FMT_RGB565, MEDIA_BUS_FMT_RGB565_2X8_LE, 16,
> +	  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_RGB565,
> +	  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x7b,
> +	  false, false },
> +
> +	{ V4L2_PIX_FMT_YUYV, MEDIA_BUS_FMT_YUYV8_2X8, 16,
> +	  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DAT8,
> +	  ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
> +	  false, false },
> +};
> +
> +#define GAMMA_MAX	3
> +#define GAMMA_ENTRIES	64
> +
> +/* Gamma table with gamma 1/2.2 */
> +static const u32 isc_gamma_table[GAMMA_MAX][GAMMA_ENTRIES] = {
> +	/* 0 --> gamma 1/1.8 */
> +	{      0x65,  0x66002F,  0x950025,  0xBB0020,  0xDB001D,  0xF8001A,
> +	  0x1130018, 0x12B0017, 0x1420016, 0x1580014, 0x16D0013, 0x1810012,
> +	  0x1940012, 0x1A60012, 0x1B80011, 0x1C90010, 0x1DA0010, 0x1EA000F,
> +	  0x1FA000F, 0x209000F, 0x218000F, 0x227000E, 0x235000E, 0x243000E,
> +	  0x251000E, 0x25F000D, 0x26C000D, 0x279000D, 0x286000D, 0x293000C,
> +	  0x2A0000C, 0x2AC000C, 0x2B8000C, 0x2C4000C, 0x2D0000B, 0x2DC000B,
> +	  0x2E7000B, 0x2F3000B, 0x2FE000B, 0x309000B, 0x314000B, 0x31F000A,
> +	  0x32A000A, 0x334000B, 0x33F000A, 0x349000A, 0x354000A, 0x35E000A,
> +	  0x368000A, 0x372000A, 0x37C000A, 0x386000A, 0x3900009, 0x399000A,
> +	  0x3A30009, 0x3AD0009, 0x3B60009, 0x3BF000A, 0x3C90009, 0x3D20009,
> +	  0x3DB0009, 0x3E40009, 0x3ED0009, 0x3F60009 },
> +
> +	/* 1 --> gamma 1/2 */
> +	{      0x7F,  0x800034,  0xB50028,  0xDE0021, 0x100001E, 0x11E001B,
> +	  0x1390019, 0x1520017, 0x16A0015, 0x1800014, 0x1940014, 0x1A80013,
> +	  0x1BB0012, 0x1CD0011, 0x1DF0010, 0x1EF0010, 0x200000F, 0x20F000F,
> +	  0x21F000E, 0x22D000F, 0x23C000E, 0x24A000E, 0x258000D, 0x265000D,
> +	  0x273000C, 0x27F000D, 0x28C000C, 0x299000C, 0x2A5000C, 0x2B1000B,
> +	  0x2BC000C, 0x2C8000B, 0x2D3000C, 0x2DF000B, 0x2EA000A, 0x2F5000A,
> +	  0x2FF000B, 0x30A000A, 0x314000B, 0x31F000A, 0x329000A, 0x333000A,
> +	  0x33D0009, 0x3470009, 0x350000A, 0x35A0009, 0x363000A, 0x36D0009,
> +	  0x3760009, 0x37F0009, 0x3880009, 0x3910009, 0x39A0009, 0x3A30009,
> +	  0x3AC0008, 0x3B40009, 0x3BD0008, 0x3C60008, 0x3CE0008, 0x3D60009,
> +	  0x3DF0008, 0x3E70008, 0x3EF0008, 0x3F70008 },
> +
> +	/* 2 --> gamma 1/2.2 */
> +	{      0x99,  0x9B0038,  0xD4002A,  0xFF0023, 0x122001F, 0x141001B,
> +	  0x15D0019, 0x1760017, 0x18E0015, 0x1A30015, 0x1B80013, 0x1CC0012,
> +	  0x1DE0011, 0x1F00010, 0x2010010, 0x2110010, 0x221000F, 0x230000F,
> +	  0x23F000E, 0x24D000E, 0x25B000D, 0x269000C, 0x276000C, 0x283000C,
> +	  0x28F000C, 0x29B000C, 0x2A7000C, 0x2B3000B, 0x2BF000B, 0x2CA000B,
> +	  0x2D5000B, 0x2E0000A, 0x2EB000A, 0x2F5000A, 0x2FF000A, 0x30A000A,
> +	  0x3140009, 0x31E0009, 0x327000A, 0x3310009, 0x33A0009, 0x3440009,
> +	  0x34D0009, 0x3560009, 0x35F0009, 0x3680008, 0x3710008, 0x3790009,
> +	  0x3820008, 0x38A0008, 0x3930008, 0x39B0008, 0x3A30008, 0x3AB0008,
> +	  0x3B30008, 0x3BB0008, 0x3C30008, 0x3CB0007, 0x3D20008, 0x3DA0007,
> +	  0x3E20007, 0x3E90007, 0x3F00008, 0x3F80007 },
>  };
>  
> +static unsigned int sensor_preferred = 1;
> +module_param(sensor_preferred, uint, 0644);
> +MODULE_PARM_DESC(sensor_preferred,
> +		 "Sensor is preferred to output the specified format (1-on 0-off), default 1");
> +
>  static int isc_clk_enable(struct clk_hw *hw)
>  {
>  	struct isc_clk *isc_clk = to_isc_clk(hw);
> @@ -447,27 +570,158 @@ static int isc_buffer_prepare(struct vb2_buffer *vb)
>  	return 0;
>  }
>  
> -static inline void isc_start_dma(struct regmap *regmap,
> -				  struct isc_buffer *frm, u32 dview)
> +static inline bool sensor_is_preferred(const struct isc_format *isc_fmt)
> +{
> +	if ((sensor_preferred && isc_fmt->sd_support) ||
> +	    !isc_fmt->isc_support)
> +		return true;
> +	else
> +		return false;

Just do:

	return (sensor_preferred && isc_fmt->sd_support) ||
	       !isc_fmt->isc_support;

> +}
> +
> +static void isc_start_dma(struct isc_device *isc)
>  {
> -	dma_addr_t addr;
> +	struct regmap *regmap = isc->regmap;
> +	struct v4l2_pix_format *pixfmt = &isc->fmt.fmt.pix;
> +	u32 sizeimage = pixfmt->sizeimage;
> +	u32 dctrl_dview;
> +	dma_addr_t addr0;
> +
> +	addr0 = vb2_dma_contig_plane_dma_addr(&isc->cur_frm->vb.vb2_buf, 0);
> +	regmap_write(regmap, ISC_DAD0, addr0);
> +
> +	switch (pixfmt->pixelformat) {
> +	case V4L2_PIX_FMT_YUV420:
> +		regmap_write(regmap, ISC_DAD1, addr0 + (sizeimage*2)/3);
> +		regmap_write(regmap, ISC_DAD2, addr0 + (sizeimage*5)/6);
> +		break;
> +	case V4L2_PIX_FMT_YUV422P:
> +		regmap_write(regmap, ISC_DAD1, addr0 + sizeimage/2);
> +		regmap_write(regmap, ISC_DAD2, addr0 + (sizeimage*3)/4);

Add spaces around operators.

> +		break;
> +	default:
> +		break;
> +	}
>  
> -	addr = vb2_dma_contig_plane_dma_addr(&frm->vb.vb2_buf, 0);
> +	if (sensor_is_preferred(isc->current_fmt))
> +		dctrl_dview = ISC_DCTRL_DVIEW_PACKED;
> +	else
> +		dctrl_dview = isc->current_fmt->reg_dctrl_dview;
>  
> -	regmap_write(regmap, ISC_DCTRL, dview | ISC_DCTRL_IE_IS);
> -	regmap_write(regmap, ISC_DAD0, addr);
> +	regmap_write(regmap, ISC_DCTRL, dctrl_dview | ISC_DCTRL_IE_IS);
>  	regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_CAPTURE);
>  }
>  
>  static void isc_set_pipeline(struct isc_device *isc, u32 pipeline)
>  {
> -	u32 val;
> +	struct regmap *regmap = isc->regmap;
> +	struct isc_ctrls *ctrls = &isc->ctrls;
> +	u32 val, bay_cfg;
> +	const u32 *gamma;
>  	unsigned int i;
>  
> +	/* WB-->CFA-->CC-->GAM-->CSC-->CBC-->SUB422-->SUB420 */
>  	for (i = 0; i < ISC_PIPE_LINE_NODE_NUM; i++) {
>  		val = pipeline & BIT(i) ? 1 : 0;
>  		regmap_field_write(isc->pipeline[i], val);
>  	}
> +
> +	if (!pipeline)
> +		return;
> +
> +	bay_cfg = isc->raw_fmt->reg_bay_cfg;
> +
> +	regmap_write(regmap, ISC_WB_CFG, bay_cfg);
> +	regmap_write(regmap, ISC_WB_O_RGR, 0x0);
> +	regmap_write(regmap, ISC_WB_O_BGR, 0x0);
> +	regmap_write(regmap, ISC_WB_G_RGR, ctrls->r_gain | (0x1 << 25));
> +	regmap_write(regmap, ISC_WB_G_BGR, ctrls->b_gain | (0x1 << 25));
> +
> +	regmap_write(regmap, ISC_CFA_CFG, bay_cfg | ISC_CFA_CFG_EITPOL);
> +
> +	gamma = &isc_gamma_table[ctrls->gamma_index][0];
> +	regmap_bulk_write(regmap, ISC_GAM_BENTRY, gamma, GAMMA_ENTRIES);
> +	regmap_bulk_write(regmap, ISC_GAM_GENTRY, gamma, GAMMA_ENTRIES);
> +	regmap_bulk_write(regmap, ISC_GAM_RENTRY, gamma, GAMMA_ENTRIES);
> +
> +	/* Convert RGB to YUV */
> +	regmap_write(regmap, ISC_CSC_YR_YG, 0x42 | (0x81 << 16));
> +	regmap_write(regmap, ISC_CSC_YB_OY, 0x19 | (0x10 << 16));
> +	regmap_write(regmap, ISC_CSC_CBR_CBG, 0xFDA | (0xFB6 << 16));
> +	regmap_write(regmap, ISC_CSC_CBB_OCB, 0x70 | (0x80 << 16));
> +	regmap_write(regmap, ISC_CSC_CRR_CRG, 0x70 | (0xFA2 << 16));
> +	regmap_write(regmap, ISC_CSC_CRB_OCR, 0xFEE | (0x80 << 16));
> +
> +	regmap_write(regmap, ISC_CBC_BRIGHT, ctrls->brightness);
> +	regmap_write(regmap, ISC_CBC_CONTRAST, ctrls->contrast);
> +}
> +
> +static int isc_update_profile(struct isc_device *isc)
> +{
> +	struct regmap *regmap = isc->regmap;
> +	u32 sr;
> +	int counter = 100;
> +
> +	regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_UPPRO);
> +
> +	regmap_read(regmap, ISC_CTRLSR, &sr);
> +	while ((sr & ISC_CTRL_UPPRO) && counter--) {
> +		usleep_range(1000, 2000);
> +		regmap_read(regmap, ISC_CTRLSR, &sr);
> +	}
> +
> +	if (counter < 0) {
> +		v4l2_warn(&isc->v4l2_dev, "Time out to update profie\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +
> +static void isc_set_histogram(struct isc_device *isc)
> +{
> +	struct regmap *regmap = isc->regmap;
> +	struct isc_ctrls *ctrls = &isc->ctrls;
> +
> +	if (ctrls->awb && (ctrls->hist_stat != HIST_ENABLED)) {
> +		regmap_write(regmap, ISC_HIS_CFG, ISC_HIS_CFG_MODE_R |
> +		      (isc->raw_fmt->reg_bay_cfg << ISC_HIS_CFG_BAYSEL_SHIFT) |
> +		      ISC_HIS_CFG_RAR);
> +		regmap_write(regmap, ISC_HIS_CTRL, ISC_HIS_CTRL_EN);
> +		regmap_write(regmap, ISC_INTEN, ISC_INT_HISDONE);
> +		ctrls->hist_id = ISC_HIS_CFG_MODE_R;
> +		isc_update_profile(isc);
> +		regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_HISREQ);
> +
> +		ctrls->hist_stat = HIST_ENABLED;
> +	} else if (!ctrls->awb && (ctrls->hist_stat != HIST_DISABLED)) {
> +		regmap_write(regmap, ISC_INTDIS, ISC_INT_HISDONE);
> +		regmap_write(regmap, ISC_HIS_CTRL, ISC_HIS_CTRL_DIS);
> +
> +		ctrls->hist_stat = HIST_DISABLED;
> +	}
> +}
> +
> +static inline void isc_get_param(const struct isc_format *fmt,
> +				     u32 *rlp_mode, u32 *dcfg_imode)
> +{
> +	switch (fmt->fourcc) {
> +	case V4L2_PIX_FMT_SBGGR10:
> +	case V4L2_PIX_FMT_SGBRG10:
> +	case V4L2_PIX_FMT_SGRBG10:
> +	case V4L2_PIX_FMT_SRGGB10:
> +	case V4L2_PIX_FMT_SBGGR12:
> +	case V4L2_PIX_FMT_SGBRG12:
> +	case V4L2_PIX_FMT_SGRBG12:
> +	case V4L2_PIX_FMT_SRGGB12:
> +		*rlp_mode = fmt->reg_rlp_mode;
> +		*dcfg_imode = fmt->reg_dcfg_imode;
> +		break;
> +	default:
> +		*rlp_mode = ISC_RLP_CFG_MODE_DAT8;
> +		*dcfg_imode = ISC_DCFG_IMODE_PACKED8;
> +		break;
> +	}
>  }
>  
>  static int isc_configure(struct isc_device *isc)
> @@ -475,39 +729,40 @@ static int isc_configure(struct isc_device *isc)
>  	struct regmap *regmap = isc->regmap;
>  	const struct isc_format *current_fmt = isc->current_fmt;
>  	struct isc_subdev_entity *subdev = isc->current_subdev;
> -	u32 val, mask;
> -	int counter = 10;
> +	u32 pfe_cfg0, rlp_mode, dcfg_imode, mask, pipeline;
> +
> +	if (sensor_is_preferred(current_fmt)) {
> +		pfe_cfg0 = current_fmt->reg_bps;
> +		pipeline = 0x0;
> +		isc_get_param(current_fmt, &rlp_mode, &dcfg_imode);
> +		isc->ctrls.hist_stat = HIST_INIT;
> +	} else {
> +		pfe_cfg0  = isc->raw_fmt->reg_bps;
> +		pipeline = current_fmt->pipeline;
> +		rlp_mode = current_fmt->reg_rlp_mode;
> +		dcfg_imode = current_fmt->reg_dcfg_imode;
> +	}
>  
> -	val = current_fmt->reg_bps | subdev->pfe_cfg0 |
> -	      ISC_PFE_CFG0_MODE_PROGRESSIVE;
> +	pfe_cfg0  |= subdev->pfe_cfg0 | ISC_PFE_CFG0_MODE_PROGRESSIVE;
>  	mask = ISC_PFE_CFG0_BPS_MASK | ISC_PFE_CFG0_HPOL_LOW |
>  	       ISC_PFE_CFG0_VPOL_LOW | ISC_PFE_CFG0_PPOL_LOW |
>  	       ISC_PFE_CFG0_MODE_MASK;
>  
> -	regmap_update_bits(regmap, ISC_PFE_CFG0, mask, val);
> +	regmap_update_bits(regmap, ISC_PFE_CFG0, mask, pfe_cfg0);
>  
>  	regmap_update_bits(regmap, ISC_RLP_CFG, ISC_RLP_CFG_MODE_MASK,
> -			   current_fmt->reg_rlp_mode);
> +			   rlp_mode);
>  
> -	regmap_update_bits(regmap, ISC_DCFG, ISC_DCFG_IMODE_MASK,
> -			   current_fmt->reg_dcfg_imode);
> +	regmap_update_bits(regmap, ISC_DCFG, ISC_DCFG_IMODE_MASK, dcfg_imode);
>  
> -	/* Disable the pipeline */
> -	isc_set_pipeline(isc, 0x0);
> +	/* Set the pipeline */
> +	isc_set_pipeline(isc, pipeline);
>  
> -	/* Update profile */
> -	regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_UPPRO);
> +	if (pipeline)
> +		isc_set_histogram(isc);
>  
> -	regmap_read(regmap, ISC_CTRLSR, &val);
> -	while ((val & ISC_CTRL_UPPRO) && counter--) {
> -		usleep_range(1000, 2000);
> -		regmap_read(regmap, ISC_CTRLSR, &val);
> -	}
> -
> -	if (counter < 0)
> -		return -ETIMEDOUT;
> -
> -	return 0;
> +	/* Update profile */
> +	return isc_update_profile(isc);
>  }
>  
>  static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
> @@ -517,7 +772,6 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
>  	struct isc_buffer *buf;
>  	unsigned long flags;
>  	int ret;
> -	u32 val;
>  
>  	/* Enable stream on the sub device */
>  	ret = v4l2_subdev_call(isc->current_subdev->sd, video, s_stream, 1);
> @@ -528,12 +782,6 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
>  
>  	pm_runtime_get_sync(isc->dev);
>  
> -	/* Disable all the interrupts */
> -	regmap_write(isc->regmap, ISC_INTDIS, (u32)~0UL);
> -
> -	/* Clean the interrupt status register */
> -	regmap_read(regmap, ISC_INTSR, &val);
> -
>  	ret = isc_configure(isc);
>  	if (unlikely(ret))
>  		goto err_configure;
> @@ -551,7 +799,7 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
>  					struct isc_buffer, list);
>  	list_del(&isc->cur_frm->list);
>  
> -	isc_start_dma(regmap, isc->cur_frm, isc->current_fmt->reg_dctrl_dview);
> +	isc_start_dma(isc);
>  
>  	spin_unlock_irqrestore(&isc->dma_queue_lock, flags);
>  
> @@ -620,8 +868,7 @@ static void isc_buffer_queue(struct vb2_buffer *vb)
>  	if (!isc->cur_frm && list_empty(&isc->dma_queue) &&
>  		vb2_is_streaming(vb->vb2_queue)) {
>  		isc->cur_frm = buf;
> -		isc_start_dma(isc->regmap, isc->cur_frm,
> -			isc->current_fmt->reg_dctrl_dview);
> +		isc_start_dma(isc);
>  	} else
>  		list_add_tail(&buf->list, &isc->dma_queue);
>  	spin_unlock_irqrestore(&isc->dma_queue_lock, flags);
> @@ -691,13 +938,14 @@ static struct isc_format *find_format_by_fourcc(struct isc_device *isc,
>  }
>  
>  static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,
> -			struct isc_format **current_fmt)
> +			struct isc_format **current_fmt, u32 *code)
>  {
>  	struct isc_format *isc_fmt;
>  	struct v4l2_pix_format *pixfmt = &f->fmt.pix;
>  	struct v4l2_subdev_format format = {
>  		.which = V4L2_SUBDEV_FORMAT_TRY,
>  	};
> +	u32 mbus_code;
>  	int ret;
>  
>  	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> @@ -717,7 +965,12 @@ static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,
>  	if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)
>  		pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;
>  
> -	v4l2_fill_mbus_format(&format.format, pixfmt, isc_fmt->mbus_code);
> +	if (sensor_is_preferred(isc_fmt))
> +		mbus_code = isc_fmt->mbus_code;
> +	else
> +		mbus_code = isc->raw_fmt->mbus_code;
> +
> +	v4l2_fill_mbus_format(&format.format, pixfmt, mbus_code);
>  	ret = v4l2_subdev_call(isc->current_subdev->sd, pad, set_fmt,
>  			       isc->current_subdev->config, &format);
>  	if (ret < 0)
> @@ -726,12 +979,15 @@ static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,
>  	v4l2_fill_pix_format(pixfmt, &format.format);
>  
>  	pixfmt->field = V4L2_FIELD_NONE;
> -	pixfmt->bytesperline = pixfmt->width * isc_fmt->bpp;
> +	pixfmt->bytesperline = (pixfmt->width * isc_fmt->bpp) >> 3;
>  	pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height;
>  
>  	if (current_fmt)
>  		*current_fmt = isc_fmt;
>  
> +	if (code)
> +		*code = mbus_code;
> +
>  	return 0;
>  }
>  
> @@ -741,14 +997,14 @@ static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)
>  		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
>  	};
>  	struct isc_format *current_fmt;
> +	u32 mbus_code;
>  	int ret;
>  
> -	ret = isc_try_fmt(isc, f, &current_fmt);
> +	ret = isc_try_fmt(isc, f, &current_fmt, &mbus_code);
>  	if (ret)
>  		return ret;
>  
> -	v4l2_fill_mbus_format(&format.format, &f->fmt.pix,
> -			      current_fmt->mbus_code);
> +	v4l2_fill_mbus_format(&format.format, &f->fmt.pix, mbus_code);
>  	ret = v4l2_subdev_call(isc->current_subdev->sd, pad,
>  			       set_fmt, NULL, &format);
>  	if (ret < 0)
> @@ -776,7 +1032,7 @@ static int isc_try_fmt_vid_cap(struct file *file, void *priv,
>  {
>  	struct isc_device *isc = video_drvdata(file);
>  
> -	return isc_try_fmt(isc, f, NULL);
> +	return isc_try_fmt(isc, f, NULL, NULL);
>  }
>  
>  static int isc_enum_input(struct file *file, void *priv,
> @@ -842,7 +1098,10 @@ static int isc_enum_framesizes(struct file *file, void *fh,
>  	if (!isc_fmt)
>  		return -EINVAL;
>  
> -	fse.code = isc_fmt->mbus_code;
> +	if (sensor_is_preferred(isc_fmt))
> +		fse.code = isc_fmt->mbus_code;
> +	else
> +		fse.code = isc->raw_fmt->mbus_code;
>  
>  	ret = v4l2_subdev_call(isc->current_subdev->sd, pad, enum_frame_size,
>  			       NULL, &fse);
> @@ -873,7 +1132,10 @@ static int isc_enum_frameintervals(struct file *file, void *fh,
>  	if (!isc_fmt)
>  		return -EINVAL;
>  
> -	fie.code = isc_fmt->mbus_code;
> +	if (sensor_is_preferred(isc_fmt))
> +		fie.code = isc_fmt->mbus_code;
> +	else
> +		fie.code = isc->raw_fmt->mbus_code;
>  
>  	ret = v4l2_subdev_call(isc->current_subdev->sd, pad,
>  			       enum_frame_interval, NULL, &fie);
> @@ -911,6 +1173,10 @@ static const struct v4l2_ioctl_ops isc_ioctl_ops = {
>  	.vidioc_s_parm			= isc_s_parm,
>  	.vidioc_enum_framesizes		= isc_enum_framesizes,
>  	.vidioc_enum_frameintervals	= isc_enum_frameintervals,
> +
> +	.vidioc_log_status		= v4l2_ctrl_log_status,
> +	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
> +	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
>  };
>  
>  static int isc_open(struct file *file)
> @@ -984,14 +1250,13 @@ static irqreturn_t isc_interrupt(int irq, void *dev_id)
>  	u32 isc_intsr, isc_intmask, pending;
>  	irqreturn_t ret = IRQ_NONE;
>  
> -	spin_lock(&isc->dma_queue_lock);
> -
>  	regmap_read(regmap, ISC_INTSR, &isc_intsr);
>  	regmap_read(regmap, ISC_INTMASK, &isc_intmask);
>  
>  	pending = isc_intsr & isc_intmask;
>  
>  	if (likely(pending & ISC_INT_DDONE)) {
> +		spin_lock(&isc->dma_queue_lock);
>  		if (isc->cur_frm) {
>  			struct vb2_v4l2_buffer *vbuf = &isc->cur_frm->vb;
>  			struct vb2_buffer *vb = &vbuf->vb2_buf;
> @@ -1007,21 +1272,144 @@ static irqreturn_t isc_interrupt(int irq, void *dev_id)
>  						     struct isc_buffer, list);
>  			list_del(&isc->cur_frm->list);
>  
> -			isc_start_dma(regmap, isc->cur_frm,
> -				      isc->current_fmt->reg_dctrl_dview);
> +			isc_start_dma(isc);
>  		}
>  
>  		if (isc->stop)
>  			complete(&isc->comp);
>  
>  		ret = IRQ_HANDLED;
> +		spin_unlock(&isc->dma_queue_lock);
>  	}
>  
> -	spin_unlock(&isc->dma_queue_lock);
> +	if (pending & ISC_INT_HISDONE) {
> +		schedule_work(&isc->awb_work);
> +		ret = IRQ_HANDLED;
> +	}
>  
>  	return ret;
>  }
>  
> +static void isc_hist_count(struct isc_device *isc)
> +{
> +	struct regmap *regmap = isc->regmap;
> +	struct isc_ctrls *ctrls = &isc->ctrls;
> +	u32 *hist_count = &ctrls->hist_count[ctrls->hist_id];
> +	u32 *hist_entry = &ctrls->hist_entry[0];
> +	u32 i;
> +
> +	regmap_bulk_read(regmap, ISC_HIS_ENTRY, hist_entry, HIST_ENTRIES);
> +
> +	*hist_count = 0;
> +	for (i = 0; i <= HIST_ENTRIES; i++)
> +		*hist_count += i * (*hist_entry++);
> +}
> +
> +static void isc_wb_update(struct isc_ctrls *ctrls)
> +{
> +	u32 *hist_count = &ctrls->hist_count[0];
> +	u64 g_count = (u64)hist_count[ISC_HIS_CFG_MODE_GB] << 9;
> +	u32 hist_r = hist_count[ISC_HIS_CFG_MODE_R];
> +	u32 hist_b = hist_count[ISC_HIS_CFG_MODE_B];
> +
> +	if (hist_r)
> +		ctrls->r_gain = div_u64(g_count, hist_r);
> +
> +	if (hist_b)
> +		ctrls->b_gain = div_u64(g_count, hist_b);
> +}
> +
> +static void isc_awb_work(struct work_struct *w)
> +{
> +	struct isc_device *isc =
> +		container_of(w, struct isc_device, awb_work);
> +	struct regmap *regmap = isc->regmap;
> +	struct isc_ctrls *ctrls = &isc->ctrls;
> +	u32 hist_id = ctrls->hist_id;
> +	u32 baysel;
> +
> +	if (ctrls->hist_stat != HIST_ENABLED)
> +		return;
> +
> +	isc_hist_count(isc);
> +
> +	if (hist_id != ISC_HIS_CFG_MODE_B) {
> +		hist_id++;
> +	} else {
> +		isc_wb_update(ctrls);
> +		hist_id = ISC_HIS_CFG_MODE_R;
> +	}
> +
> +	ctrls->hist_id = hist_id;
> +	baysel = isc->raw_fmt->reg_bay_cfg << ISC_HIS_CFG_BAYSEL_SHIFT;
> +
> +	pm_runtime_get_sync(isc->dev);
> +
> +	regmap_write(regmap, ISC_HIS_CFG, hist_id | baysel | ISC_HIS_CFG_RAR);
> +	isc_update_profile(isc);
> +	regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_HISREQ);
> +
> +	pm_runtime_put_sync(isc->dev);
> +}
> +
> +static int isc_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct isc_device *isc = container_of(ctrl->handler,
> +					     struct isc_device, ctrls.handler);
> +	struct isc_ctrls *ctrls = &isc->ctrls;
> +
> +	switch (ctrl->id) {
> +	case V4L2_CID_BRIGHTNESS:
> +		ctrls->brightness = ctrl->val & ISC_CBC_BRIGHT_MASK;
> +		break;
> +	case V4L2_CID_CONTRAST:
> +		ctrls->contrast = (ctrl->val << 8) & ISC_CBC_CONTRAST_MASK;

As I understand it only bits 11-8 contain the contrast in the register?

Wouldn't '(ctrl->val & ISC_CBC_CONTRAST_MASK) << 8' be more readable?

Either that or the mask should be 0xf00, not 0xfff.

> +		break;
> +	case V4L2_CID_GAMMA:
> +		ctrls->gamma_index = ctrl->val;
> +		break;
> +	case V4L2_CID_AUTO_WHITE_BALANCE:
> +		ctrls->awb = ctrl->val;
> +		if (ctrls->hist_stat != HIST_ENABLED) {
> +			ctrls->r_gain = 0x1 << 9;
> +			ctrls->b_gain = 0x1 << 9;
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops isc_ctrl_ops = {
> +	.s_ctrl	= isc_s_ctrl,
> +};
> +
> +static int isc_ctrl_init(struct isc_device *isc)
> +{
> +	const struct v4l2_ctrl_ops *ops = &isc_ctrl_ops;
> +	struct isc_ctrls *ctrls = &isc->ctrls;
> +	struct v4l2_ctrl_handler *hdl = &ctrls->handler;
> +	int ret;
> +
> +	ctrls->hist_stat = HIST_INIT;
> +
> +	ret = v4l2_ctrl_handler_init(hdl, 4);
> +	if (ret < 0)
> +		return ret;
> +
> +	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_BRIGHTNESS, -1024, 1023, 1, 0);
> +	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_CONTRAST, -8, 7, 1, 1);
> +	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_GAMMA, 0, GAMMA_MAX - 1, 1, 2);

Why is the maximum GAMMA_MAX - 1? I would assume that GAMMA_MAX is the maximum.

Looks weird. It's either a bug or it needs a comment.

> +	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_AUTO_WHITE_BALANCE, 0, 1, 1, 1);
> +
> +	v4l2_ctrl_handler_setup(hdl);
> +
> +	return 0;
> +}
> +
> +
>  static int isc_async_bound(struct v4l2_async_notifier *notifier,
>  			    struct v4l2_subdev *subdev,
>  			    struct v4l2_async_subdev *asd)
> @@ -1047,10 +1435,11 @@ static void isc_async_unbind(struct v4l2_async_notifier *notifier,
>  {
>  	struct isc_device *isc = container_of(notifier->v4l2_dev,
>  					      struct isc_device, v4l2_dev);
> -
> +	cancel_work_sync(&isc->awb_work);
>  	video_unregister_device(&isc->video_dev);
>  	if (isc->current_subdev->config)
>  		v4l2_subdev_free_pad_config(isc->current_subdev->config);
> +	v4l2_ctrl_handler_free(&isc->ctrls.handler);
>  }
>  
>  static struct isc_format *find_format_by_code(unsigned int code, int *index)
> @@ -1081,7 +1470,9 @@ static int isc_formats_init(struct isc_device *isc)
>  
>  	fmt = &isc_formats[0];
>  	for (i = 0; i < ARRAY_SIZE(isc_formats); i++) {
> -		fmt->support = false;
> +		fmt->isc_support = false;
> +		fmt->sd_support = false;
> +
>  		fmt++;
>  	}
>  
> @@ -1092,8 +1483,22 @@ static int isc_formats_init(struct isc_device *isc)
>  		if (!fmt)
>  			continue;
>  
> -		fmt->support = true;
> -		num_fmts++;
> +		fmt->sd_support = true;
> +
> +		if (i <= RAW_FMT_INDEX_END) {
> +			for (j = ISC_FMT_INDEX_START;
> +			     j <= ISC_FMT_INDEX_END; j++)

Just merge these two lines, easier to read.

> +				isc_formats[j].isc_support = true;
> +
> +			isc->raw_fmt = fmt;
> +		}
> +	}
> +
> +	for (i = 0, num_fmts = 0; i < ARRAY_SIZE(isc_formats); i++) {
> +		if (fmt->isc_support || fmt->sd_support)
> +			num_fmts++;
> +
> +		fmt++;
>  	}
>  
>  	if (!num_fmts)
> @@ -1110,7 +1515,7 @@ static int isc_formats_init(struct isc_device *isc)
>  
>  	fmt = &isc_formats[0];
>  	for (i = 0, j = 0; i < ARRAY_SIZE(isc_formats); i++) {
> -		if (fmt->support)
> +		if (fmt->isc_support || fmt->sd_support)
>  			isc->user_formats[j++] = fmt;
>  
>  		fmt++;
> @@ -1132,7 +1537,7 @@ static int isc_set_default_fmt(struct isc_device *isc)
>  	};
>  	int ret;
>  
> -	ret = isc_try_fmt(isc, &f, NULL);
> +	ret = isc_try_fmt(isc, &f, NULL, NULL);
>  	if (ret)
>  		return ret;
>  
> @@ -1151,6 +1556,12 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
>  	struct vb2_queue *q = &isc->vb2_vidq;
>  	int ret;
>  
> +	ret = v4l2_device_register_subdev_nodes(&isc->v4l2_dev);
> +	if (ret < 0) {
> +		v4l2_err(&isc->v4l2_dev, "Failed to register subdev nodes\n");
> +		return ret;
> +	}
> +
>  	isc->current_subdev = container_of(notifier,
>  					   struct isc_subdev_entity, notifier);
>  	sd_entity = isc->current_subdev;
> @@ -1198,6 +1609,14 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
>  		return ret;
>  	}
>  
> +	ret = isc_ctrl_init(isc);
> +	if (ret) {
> +		v4l2_err(&isc->v4l2_dev, "Init isc ctrols failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	INIT_WORK(&isc->awb_work, isc_awb_work);
> +
>  	/* Register video device */
>  	strlcpy(vdev->name, ATMEL_ISC_NAME, sizeof(vdev->name));
>  	vdev->release		= video_device_release_empty;
> @@ -1207,7 +1626,7 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
>  	vdev->vfl_dir		= VFL_DIR_RX;
>  	vdev->queue		= q;
>  	vdev->lock		= &isc->lock;
> -	vdev->ctrl_handler	= isc->current_subdev->sd->ctrl_handler;
> +	vdev->ctrl_handler	= &isc->ctrls.handler;
>  	vdev->device_caps	= V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE;
>  	video_set_drvdata(vdev, isc);
>  
> 

Regards,

	Hans

^ permalink raw reply

* [RFC 7/8] KVM: arm/arm64: Set up a background timer for the physical timer emulation
From: Christoffer Dall @ 2017-01-09 12:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482772326-29110-8-git-send-email-jintack@cs.columbia.edu>

On Mon, Dec 26, 2016 at 12:12:05PM -0500, Jintack Lim wrote:
> Set a background timer for the EL1 physical timer emulation while VMs are
> running, so that VMs get interrupts for the physical timer in a timely
> manner.
> 
> We still use just one background timer. When a VM is runnable, we use
> the background timer for the physical timer emulation.  When the VM is
> about to be blocked, we use the background timer to wake up the vcpu at
> the earliest timer expiration among timers the VM is using.
> 
> As a result, the assumption that the background timer is not armed while
> VMs are running does not hold any more. So, remove BUG_ON()s and
> WARN_ON()s accordingly.
> 
> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> ---
>  virt/kvm/arm/arch_timer.c | 42 +++++++++++++++++++++++++++++++-----------
>  1 file changed, 31 insertions(+), 11 deletions(-)
> 
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index aa7e243..be8d953 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -91,9 +91,6 @@ static void kvm_timer_inject_irq_work(struct work_struct *work)
>  	vcpu = container_of(work, struct kvm_vcpu, arch.timer_cpu.expired);
>  	vcpu->arch.timer_cpu.armed = false;
>  
> -	WARN_ON(!kvm_timer_should_fire(vcpu, vcpu_vtimer(vcpu)) &&
> -		!kvm_timer_should_fire(vcpu, vcpu_ptimer(vcpu)));
> -

This seems misplaced and has been addressed here:
https://lists.cs.columbia.edu/pipermail/kvmarm/2017-January/022933.html

When you respin you can benefit from basing on that (assuming it gets
acked and goes int).

>  	/*
>  	 * If the vcpu is blocked we want to wake it up so that it will see
>  	 * the timer has expired when entering the guest.
> @@ -139,7 +136,6 @@ static bool kvm_timer_irq_can_fire(struct arch_timer_context *timer_ctx)
>  
>  /*
>   * Returns minimal timer expiration time in ns among guest timers.
> - * Note that it will return inf time if none of timers can fire.
>   */
>  static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu)
>  {
> @@ -153,7 +149,9 @@ static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu)
>  	if (kvm_timer_irq_can_fire(ptimer))
>  		min_phys = kvm_timer_compute_delta(vcpu, ptimer);
>  
> -	WARN_ON((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX));
> +	/* If none of timers can fire, then return 0 */
> +	if ((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX))
> +		return 0;

Why didn't you have this semantics in the previous patch?

>  
>  	return min(min_virt, min_phys);
>  }
> @@ -257,6 +255,26 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)
>  }
>  
>  /*
> + * Schedule the background timer for the emulated timer. The background timer
> + * runs whenever vcpu is runnable and the timer is not expired.
> + */
> +static void kvm_timer_emulate(struct kvm_vcpu *vcpu,
> +		       struct arch_timer_context *timer_ctx)
> +{
> +	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> +
> +	if (kvm_timer_should_fire(vcpu, timer_ctx))
> +		return;
> +
> +	if (!kvm_timer_irq_can_fire(timer_ctx))
> +		return;
> +
> +	/*  The timer has not yet expired, schedule a background timer */
> +	timer_disarm(timer);
> +	timer_arm(timer, kvm_timer_compute_delta(vcpu, timer_ctx));

I'm wondering what the effect of this thing really is.  Isn't the soft
timer programmed in timer_arm() based on Linux's own timekeeping
schedule, such that the physical timer will be programmed to the next
tick, regardless of what you program here, so all you have to do is
check if you need to inject the phys timer on entry to the VM?

On the other hand, if this can cause Linux to program the phys timer to
expire sooner, then I guess it makes sense.  Thinking about it, would
that be the case on a tickless system?

> +}
> +
> +/*
>   * Schedule the background timer before calling kvm_vcpu_block, so that this
>   * thread is removed from its waitqueue and made runnable when there's a timer
>   * interrupt to handle.
> @@ -267,8 +285,6 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu)
>  	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
>  	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>  
> -	BUG_ON(timer_is_armed(timer));
> -
>  	/*
>  	 * No need to schedule a background timer if any guest timer has
>  	 * already expired, because kvm_vcpu_block will return before putting
> @@ -290,13 +306,21 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu)
>  	 * The guest timers have not yet expired, schedule a background timer.
>  	 * Pick smaller expiration time between phys and virt timer.
>  	 */
> +	timer_disarm(timer);
>  	timer_arm(timer, kvm_timer_min_block(vcpu));
>  }
>  
>  void kvm_timer_unschedule(struct kvm_vcpu *vcpu)
>  {
>  	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> +
>  	timer_disarm(timer);
> +
> +	/*
> +	 * Now we return from the blocking. If we have any timer to emulate,
> +	 * and it's not expired, set the background timer for it.
> +	 */
> +	kvm_timer_emulate(vcpu, vcpu_ptimer(vcpu));

hmm, this is only called when returning from the kvm_vcpu_block() path.
What about when you do vcpu_load/put, don't you need to schedule/cancel
it there too?

Maybe it's simpler to just program the soft timer during flush_hwstate
and cancel the timer during sync_hwstate.  Does that work?

>  }
>  
>  /**
> @@ -375,10 +399,6 @@ void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu)
>   */
>  void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
>  {
> -	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> -
> -	BUG_ON(timer_is_armed(timer));
> -
>  	/*
>  	 * The guest could have modified the timer registers or the timer
>  	 * could have expired, update the timer state.
> -- 
> 1.9.1
> 
> 

Thanks,
-Christoffer

^ permalink raw reply

* [RFC 6/8] KVM: arm/arm64: Update the physical timer interrupt level
From: Christoffer Dall @ 2017-01-09 12:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482772326-29110-7-git-send-email-jintack@cs.columbia.edu>

On Mon, Dec 26, 2016 at 12:12:04PM -0500, Jintack Lim wrote:
> Now that we maintain the EL1 physical timer register states of the VM,
> update the physical timer interrupt level along with the virtual one.
> 
> Note that the emulated EL1 physical timer is not mapped to any hardware
> timer, so we let vgic know that.
> 
> With this commit, VMs are able to get the physical timer interrupts
> while they are runnable. But they won't get interrupts once vcpus go to
> sleep since we don't have code to wake vcpus up on the emulated physical
> timer expiration yet.

I'm not sure I understand.  It looks to me like it's the other way
around and that this code supports waking up a sleeping VCPU sooner if
it has a physical timer scheduled, but doesn't yet support causing an
early exit from running the VPCU base on programing the physical timer?


Thanks,
-Christoffer

> 
> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> ---
>  arch/arm/kvm/arm.c        |  3 +-
>  virt/kvm/arm/arch_timer.c | 76 ++++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 64 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 37d1623..d2dfa32 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -295,7 +295,8 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
>  
>  int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
>  {
> -	return kvm_timer_should_fire(vcpu, vcpu_vtimer(vcpu));
> +	return kvm_timer_should_fire(vcpu, vcpu_vtimer(vcpu)) ||
> +		kvm_timer_should_fire(vcpu, vcpu_ptimer(vcpu));
>  }
>  
>  void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu)
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index ed80864..aa7e243 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -91,7 +91,8 @@ static void kvm_timer_inject_irq_work(struct work_struct *work)
>  	vcpu = container_of(work, struct kvm_vcpu, arch.timer_cpu.expired);
>  	vcpu->arch.timer_cpu.armed = false;
>  
> -	WARN_ON(!kvm_timer_should_fire(vcpu, vcpu_vtimer(vcpu)));
> +	WARN_ON(!kvm_timer_should_fire(vcpu, vcpu_vtimer(vcpu)) &&
> +		!kvm_timer_should_fire(vcpu, vcpu_ptimer(vcpu)));
>  
>  	/*
>  	 * If the vcpu is blocked we want to wake it up so that it will see
> @@ -130,6 +131,33 @@ static u64 kvm_timer_compute_delta(struct kvm_vcpu *vcpu,
>  	return 0;
>  }
>  
> +static bool kvm_timer_irq_can_fire(struct arch_timer_context *timer_ctx)
> +{
> +	return !(timer_ctx->cnt_ctl & ARCH_TIMER_CTRL_IT_MASK) &&
> +		(timer_ctx->cnt_ctl & ARCH_TIMER_CTRL_ENABLE);
> +}
> +
> +/*
> + * Returns minimal timer expiration time in ns among guest timers.
> + * Note that it will return inf time if none of timers can fire.
> + */
> +static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu)
> +{
> +	u64 min_virt = ULLONG_MAX, min_phys = ULLONG_MAX;
> +	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> +	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> +
> +	if (kvm_timer_irq_can_fire(vtimer))
> +		min_virt = kvm_timer_compute_delta(vcpu, vtimer);
> +
> +	if (kvm_timer_irq_can_fire(ptimer))
> +		min_phys = kvm_timer_compute_delta(vcpu, ptimer);
> +
> +	WARN_ON((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX));
> +
> +	return min(min_virt, min_phys);
> +}
> +
>  static enum hrtimer_restart kvm_timer_expire(struct hrtimer *hrt)
>  {
>  	struct arch_timer_cpu *timer;
> @@ -144,7 +172,7 @@ static enum hrtimer_restart kvm_timer_expire(struct hrtimer *hrt)
>  	 * PoV (NTP on the host may have forced it to expire
>  	 * early). If we should have slept longer, restart it.
>  	 */
> -	ns = kvm_timer_compute_delta(vcpu, vcpu_vtimer(vcpu));
> +	ns = kvm_timer_min_block(vcpu);
>  	if (unlikely(ns)) {
>  		hrtimer_forward_now(hrt, ns_to_ktime(ns));
>  		return HRTIMER_RESTART;
> @@ -154,12 +182,6 @@ static enum hrtimer_restart kvm_timer_expire(struct hrtimer *hrt)
>  	return HRTIMER_NORESTART;
>  }
>  
> -static bool kvm_timer_irq_can_fire(struct arch_timer_context *timer_ctx)
> -{
> -	return !(timer_ctx->cnt_ctl & ARCH_TIMER_CTRL_IT_MASK) &&
> -		(timer_ctx->cnt_ctl & ARCH_TIMER_CTRL_ENABLE);
> -}
> -
>  bool kvm_timer_should_fire(struct kvm_vcpu *vcpu,
>  			   struct arch_timer_context *timer_ctx)
>  {
> @@ -191,6 +213,21 @@ static void kvm_timer_update_mapped_irq(struct kvm_vcpu *vcpu, bool new_level,
>  	WARN_ON(ret);
>  }
>  
> +static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
> +				 struct arch_timer_context *timer)
> +{
> +	int ret;
> +
> +	BUG_ON(!vgic_initialized(vcpu->kvm));
> +
> +	timer->irq.level = new_level;
> +	trace_kvm_timer_update_irq(vcpu->vcpu_id, timer->irq.irq,
> +				   timer->irq.level);
> +	ret = kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id, timer->irq.irq,
> +				  timer->irq.level);
> +	WARN_ON(ret);
> +}
> +
>  /*
>   * Check if there was a change in the timer state (should we raise or lower
>   * the line level to the GIC).
> @@ -198,6 +235,7 @@ static void kvm_timer_update_mapped_irq(struct kvm_vcpu *vcpu, bool new_level,
>  static int kvm_timer_update_state(struct kvm_vcpu *vcpu)
>  {
>  	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> +	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>  
>  	/*
>  	 * If userspace modified the timer registers via SET_ONE_REG before
> @@ -211,6 +249,10 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)
>  	if (kvm_timer_should_fire(vcpu, vtimer) != vtimer->irq.level)
>  		kvm_timer_update_mapped_irq(vcpu, !vtimer->irq.level, vtimer);
>  
> +	/* The emulated EL1 physical timer irq is not mapped to hardware */
> +	if (kvm_timer_should_fire(vcpu, ptimer) != ptimer->irq.level)
> +		kvm_timer_update_irq(vcpu, !ptimer->irq.level, ptimer);
> +
>  	return 0;
>  }
>  
> @@ -223,26 +265,32 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu)
>  {
>  	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
>  	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> +	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>  
>  	BUG_ON(timer_is_armed(timer));
>  
>  	/*
> -	 * No need to schedule a background timer if the guest timer has
> +	 * No need to schedule a background timer if any guest timer has
>  	 * already expired, because kvm_vcpu_block will return before putting
>  	 * the thread to sleep.
>  	 */
> -	if (kvm_timer_should_fire(vcpu, vtimer))
> +	if (kvm_timer_should_fire(vcpu, vtimer) ||
> +	    kvm_timer_should_fire(vcpu, ptimer))
>  		return;
>  
>  	/*
> -	 * If the timer is not capable of raising interrupts (disabled or
> +	 * If both timers are not capable of raising interrupts (disabled or
>  	 * masked), then there's no more work for us to do.
>  	 */
> -	if (!kvm_timer_irq_can_fire(vtimer))
> +	if (!kvm_timer_irq_can_fire(vtimer) &&
> +	    !kvm_timer_irq_can_fire(ptimer))
>  		return;
>  
> -	/*  The timer has not yet expired, schedule a background timer */
> -	timer_arm(timer, kvm_timer_compute_delta(vcpu, vtimer));
> +	/*
> +	 * The guest timers have not yet expired, schedule a background timer.
> +	 * Pick smaller expiration time between phys and virt timer.
> +	 */
> +	timer_arm(timer, kvm_timer_min_block(vcpu));
>  }
>  
>  void kvm_timer_unschedule(struct kvm_vcpu *vcpu)
> -- 
> 1.9.1
> 
> 

^ permalink raw reply

* [RFC 8/8] KVM: arm/arm64: Emulate the EL1 phys timer register access
From: Christoffer Dall @ 2017-01-09 12:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482772326-29110-9-git-send-email-jintack@cs.columbia.edu>

On Mon, Dec 26, 2016 at 12:12:06PM -0500, Jintack Lim wrote:
> Emulate read and write operations to CNTP_TVAL, CNTP_CVAL and CNTP_CTL.
> Now the VM is able to use the EL1 physical timer.
> 
> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> ---
>  arch/arm64/kvm/sys_regs.c    | 35 ++++++++++++++++++++++++++++++++---
>  include/kvm/arm_arch_timer.h |  3 +++
>  virt/kvm/arm/arch_timer.c    |  4 ++--
>  3 files changed, 37 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index fd9e747..7cef94f 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -824,7 +824,15 @@ static bool access_cntp_tval(struct kvm_vcpu *vcpu,
>  		struct sys_reg_params *p,
>  		const struct sys_reg_desc *r)
>  {
> -	kvm_inject_undefined(vcpu);
> +	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> +	cycle_t now = kvm_phys_timer_read();
> +
> +	if (p->is_write) {
> +		ptimer->cnt_cval = p->regval + now;
> +		kvm_timer_emulate(vcpu, ptimer);

Hmm, do we really need those calls here?

I guess I'm a little confused about exactly what the kvm_timer_emulate()
function is supposed to do, and it feels to me like these handlers
should just record what the guest is asking the kernel to do and the
logic of handling the additional timer should be moved into the run path
as much as possible.

Thanks,
-Christoffer

> +	} else
> +		p->regval = ptimer->cnt_cval - now;
> +
>  	return true;
>  }
>  
> @@ -832,7 +840,21 @@ static bool access_cntp_ctl(struct kvm_vcpu *vcpu,
>  		struct sys_reg_params *p,
>  		const struct sys_reg_desc *r)
>  {
> -	kvm_inject_undefined(vcpu);
> +	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> +
> +	if (p->is_write) {
> +		/* ISTATUS bit is read-only */
> +		ptimer->cnt_ctl = p->regval & ~ARCH_TIMER_CTRL_IT_STAT;
> +		kvm_timer_emulate(vcpu, ptimer);
> +	} else {
> +		cycle_t now = kvm_phys_timer_read();
> +
> +		p->regval = ptimer->cnt_ctl;
> +		/* Set ISTATUS bit if it's expired */
> +		if (ptimer->cnt_cval <= now)
> +			p->regval |= ARCH_TIMER_CTRL_IT_STAT;
> +	}
> +
>  	return true;
>  }
>  
> @@ -840,7 +862,14 @@ static bool access_cntp_cval(struct kvm_vcpu *vcpu,
>  		struct sys_reg_params *p,
>  		const struct sys_reg_desc *r)
>  {
> -	kvm_inject_undefined(vcpu);
> +	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> +
> +	if (p->is_write) {
> +		ptimer->cnt_cval = p->regval;
> +		kvm_timer_emulate(vcpu, ptimer);
> +	} else
> +		p->regval = ptimer->cnt_cval;
> +
>  	return true;
>  }
>  
> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
> index 04ed9c1..776579b 100644
> --- a/include/kvm/arm_arch_timer.h
> +++ b/include/kvm/arm_arch_timer.h
> @@ -75,6 +75,9 @@ bool kvm_timer_should_fire(struct kvm_vcpu *vcpu,
>  			   struct arch_timer_context *timer_ctx);
>  void kvm_timer_schedule(struct kvm_vcpu *vcpu);
>  void kvm_timer_unschedule(struct kvm_vcpu *vcpu);
> +void kvm_timer_emulate(struct kvm_vcpu *vcpu, struct arch_timer_context *timer);
> +
> +cycle_t kvm_phys_timer_read(void);
>  
>  void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu);
>  
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index be8d953..7a161f8 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -39,7 +39,7 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)
>  	vcpu_vtimer(vcpu)->active_cleared_last = false;
>  }
>  
> -static cycle_t kvm_phys_timer_read(void)
> +cycle_t kvm_phys_timer_read(void)
>  {
>  	return timecounter->cc->read(timecounter->cc);
>  }
> @@ -258,7 +258,7 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)
>   * Schedule the background timer for the emulated timer. The background timer
>   * runs whenever vcpu is runnable and the timer is not expired.
>   */
> -static void kvm_timer_emulate(struct kvm_vcpu *vcpu,
> +void kvm_timer_emulate(struct kvm_vcpu *vcpu,
>  		       struct arch_timer_context *timer_ctx)
>  {
>  	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> -- 
> 1.9.1
> 
> 

^ permalink raw reply

* [PATCH v2 1/5] ARM: davinci_all_defconfig: enable DA8xx pinconf
From: Sekhar Nori @ 2017-01-09 12:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483677228-2325-2-git-send-email-david@lechnology.com>

On Friday 06 January 2017 10:03 AM, David Lechner wrote:
> This enables the DA8xx pinconf driver by default. It is needed by LEGO
> MINDSTORMS EV3.

Missing your sign-off. I cannot apply that myself. Please resend.

Thanks,
Sekhar

^ permalink raw reply

* [PATCH v2 2/5] ARM: davinci_all_defconfig: Enable PWM modules
From: Sekhar Nori @ 2017-01-09 12:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483677228-2325-3-git-send-email-david@lechnology.com>

On Friday 06 January 2017 10:03 AM, David Lechner wrote:
> This enables PWM and the TI ECAP and EHRWPM modules. These are used on LEGO
> MINDSTORMS EV3.

Here too. Please resend with sign-off.

Thanks,
Sekhar

^ permalink raw reply

* [PATCH 1/2] ARM: hyp-stub: improve ABI
From: Russell King - ARM Linux @ 2017-01-09 12:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170103095149.GA14242@cbox>

On Tue, Jan 03, 2017 at 10:51:49AM +0100, Christoffer Dall wrote:
> Hi Russell,
> 
> On Thu, Dec 15, 2016 at 06:57:18PM +0000, Russell King - ARM Linux wrote:
> > What's also coming clear is that there's very few people who understand
> > all the interactions here, and the whole thing seems to be an undocumented
> > mess.  
> 
> I think the hyp stub has just served a very limited purpose so far, and
> therefore is a somewhat immature implementation.  Now we've discovered a
> need to clean it up, and we're all for that.  Again, I don't think the
> problem is any larger than that, we just need to fix it, and it seems to
> me everyone is willing to work on that.

What I want to see is some documentation of the hyp-stub, so that there
can be some element of confidence that changes there are properly
coordinated.  As I said in a follow up email:

| Either we need more people to have an understanding (so if one of them
| gets run over by a bus, we're not left floundering around) or we need
| it to be documented - even if it's just a simple comment "the ABI in
| this file is shared with XYZ, if you change the ABI here, also update
| XYZ too."

> Marc even offered to work on your suggestion to support the general
> hyp ABI commands in KVM.

... which is pointless, because it's a duplication of the effort I've
already put in.  My patches already do the:

#define HVC_GET_VECTORS 0
#define HVC_SET_VECTORS 1
#define HVC_SOFT_RESTART 2

thing which ARM64 does, passing the arguments in via the appropriate
registers.  However, such a change is a major revision of hyp-stub's
ABI, which completely changes the way it works.

The issue here is that there seems to be only one person, Marc, who
understands that the KVM hypervisor also needs to be updated - everyone
else I've talked to was of the opinion that hyp-stub's ABI is limited
to hyp-stub.S.  That's worse than "I don't know" because it leads to
exactly the situation that my patches created: the two ABIs going out
of step.

What I'm asking for is a patch to add some sort of documentation ASAP
which ensures that this important non-obvious information isn't limited
to just one person.  As I mentioned in the quote above, it can be as
simple as merely pointing each file at its counterparts which have to
be updated at the same time.

Longer term, I'd like to see the existing hypervisor documentation in
Documentation/virtual/kvm/hypercalls.txt updated with the ARM details.
According to that document, KVM effectively only exists on PPC and x86
at the present time...

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [PATCH v2 3/5] ARM: davinci_all_defconfig: enable iio and ADS7950
From: Sekhar Nori @ 2017-01-09 12:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483677228-2325-4-git-send-email-david@lechnology.com>

On Friday 06 January 2017 10:03 AM, David Lechner wrote:
> This enables the iio subsystem and the TI ADS7950 driver. This is used by
> LEGO MINDSTORMS EV3, which has an ADS7957 chip.

Can you add your sign-off?

> ---
> 
> The CONFIG_TI_ADS7950 driver is currently in iio/testing, so some coordination
> may be needed before picking up this patch.
> 
>  arch/arm/configs/davinci_all_defconfig | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
> index 2b1967a..a899876 100644
> --- a/arch/arm/configs/davinci_all_defconfig
> +++ b/arch/arm/configs/davinci_all_defconfig
> @@ -200,6 +200,13 @@ CONFIG_TI_EDMA=y
>  CONFIG_MEMORY=y
>  CONFIG_TI_AEMIF=m
>  CONFIG_DA8XX_DDRCTL=y
> +CONFIG_IIO=m
> +CONFIG_IIO_BUFFER_CB=m
> +CONFIG_IIO_SW_DEVICE=m
> +CONFIG_IIO_SW_TRIGGER=m

> +CONFIG_TI_ADS7950=m

Can you separate this from rest of the patch. I would like to enable
this option only after I can find the symbol in linux-next.

> +CONFIG_IIO_HRTIMER_TRIGGER=m
> +CONFIG_IIO_SYSFS_TRIGGER=m

Need CONFIG_IIO_TRIGGER=y also for these two options to take effect.

Thanks,
Sekhar

^ permalink raw reply

* [PATCH 2/2] media: rc: add driver for IR remote receiver on MT7623 SoC
From: Sean Young @ 2017-01-09 12:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483931601.16976.48.camel@mtkswgap22>

On Mon, Jan 09, 2017 at 11:13:21AM +0800, Sean Wang wrote:
> I had another question. I found multiple and same IR messages being
> received when using SONY remote controller. Should driver needs to
> report each message or only one of these to the upper layer ?

In general the driver shouldn't try to change any IR message, this should
be done in rc-core if necessary.

rc-core should handle this correctly. If the same key is received twice
within IR_KEYPRESS_TIMEOUT (250ms) then it not reported to the input
layer.

Thanks
Sean

^ permalink raw reply

* [PATCH 1/2] ARM: hyp-stub: improve ABI
From: Russell King - ARM Linux @ 2017-01-09 12:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <72f93940-cf87-fd91-90f2-760b7ff050fb@arm.com>

On Thu, Dec 15, 2016 at 11:18:48AM +0000, Marc Zyngier wrote:
> On 14/12/16 10:46, Russell King wrote:
> > Improve the hyp-stub ABI to allow it to do more than just get/set the
> > vectors.  We follow the example in ARM64, where r0 is used as an opcode
> > with the other registers as an argument.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  arch/arm/kernel/hyp-stub.S | 27 ++++++++++++++++++++++-----
> >  1 file changed, 22 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S
> > index 15d073ae5da2..f3e9ba5fb642 100644
> > --- a/arch/arm/kernel/hyp-stub.S
> > +++ b/arch/arm/kernel/hyp-stub.S
> > @@ -22,6 +22,9 @@
> >  #include <asm/assembler.h>
> >  #include <asm/virt.h>
> >  
> > +#define HVC_GET_VECTORS 0
> > +#define HVC_SET_VECTORS 1
> > +
> >  #ifndef ZIMAGE
> >  /*
> >   * For the kernel proper, we need to find out the CPU boot mode long after
> > @@ -202,9 +205,19 @@ ARM_BE8(orr	r7, r7, #(1 << 25))     @ HSCTLR.EE
> >  ENDPROC(__hyp_stub_install_secondary)
> >  
> >  __hyp_stub_do_trap:
> > -	cmp	r0, #-1
> > -	mrceq	p15, 4, r0, c12, c0, 0	@ get HVBAR
> > -	mcrne	p15, 4, r0, c12, c0, 0	@ set HVBAR
> > +	teq	r0, #HVC_GET_VECTORS
> > +	bne	1f
> > +	mrc	p15, 4, r0, c12, c0, 0	@ get HVBAR
> > +	b	__hyp_stub_exit
> > +
> > +1:	teq	r0, #HVC_SET_VECTORS
> > +	bne	1f
> > +	mcr	p15, 4, r1, c12, c0, 0	@ set HVBAR
> > +	b	__hyp_stub_exit
> > +
> > +1:	mov	r0, #-1
> > +
> > +__hyp_stub_exit:
> >  	__ERET
> >  ENDPROC(__hyp_stub_do_trap)
> >  
> > @@ -231,10 +244,14 @@ ENDPROC(__hyp_stub_do_trap)
> >   * initialisation entry point.
> >   */
> >  ENTRY(__hyp_get_vectors)
> > -	mov	r0, #-1
> > +	mov	r0, #HVC_GET_VECTORS
> 
> This breaks the KVM implementation of __hyp_get_vectors, easily fixed
> with the following patchlet:
> 
> diff --git a/arch/arm/include/asm/virt.h b/arch/arm/include/asm/virt.h
> index a2e75b8..0fe637e 100644
> --- a/arch/arm/include/asm/virt.h
> +++ b/arch/arm/include/asm/virt.h
> @@ -89,6 +89,14 @@ extern char __hyp_text_start[];
>  extern char __hyp_text_end[];
>  #endif
>  
> +#else
> +
> +/* Only assembly code should need those */
> +
> +#define HVC_GET_VECTORS 0
> +#define HVC_SET_VECTORS 1
> +#define HVC_SOFT_RESTART 2
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif /* ! VIRT_H */
> diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S
> index ebc26f8..1c6888f 100644
> --- a/arch/arm/kernel/hyp-stub.S
> +++ b/arch/arm/kernel/hyp-stub.S
> @@ -22,10 +22,6 @@
>  #include <asm/assembler.h>
>  #include <asm/virt.h>
>  
> -#define HVC_GET_VECTORS 0
> -#define HVC_SET_VECTORS 1
> -#define HVC_SOFT_RESTART 2
> -
>  #ifndef ZIMAGE
>  /*
>   * For the kernel proper, we need to find out the CPU boot mode long after
> diff --git a/arch/arm/kvm/hyp/hyp-entry.S b/arch/arm/kvm/hyp/hyp-entry.S
> index 96beb53..1f8db7d 100644
> --- a/arch/arm/kvm/hyp/hyp-entry.S
> +++ b/arch/arm/kvm/hyp/hyp-entry.S
> @@ -127,7 +127,7 @@ hyp_hvc:
>  	pop	{r0, r1, r2}
>  
>  	/* Check for __hyp_get_vectors */
> -	cmp	r0, #-1
> +	cmp	r0, #HVC_GET_VECTORS
>  	mrceq	p15, 4, r0, c12, c0, 0	@ get HVBAR
>  	beq	1f

I don't think this is sufficient - the kdump case for ARM will still be
broken after these patches.

The new soft-restart ABI introduced by my patch 2 passes in:

r0 = HVC_SOFT_RESTART
r1 = non-zero
r2 = undefined
r3 = function pointer

and the assumption is that r3 will be preserved if the HVC call does
nothing - which probably isn't a safe assumption.

With these arguments passed to the KVM stub (which may be in place at
the point of a kdump), we end up executing this code:

        push    {lr}

        mov     lr, r0
        mov     r0, r1
        mov     r1, r2
        mov     r2, r3

THUMB(  orr     lr, #1)
        blx     lr                      @ Call the HYP function

This will result in an attempt to branch to address 2 or 3, which isn't
sane - a panic in the host kernel (eg due to a NULL pointer deref with
panic_on_oops enabled) will then cause kdump to try to execute code from
a stupid address.

So, we need KVM's stub to be (a) better documented so this stuff is
obvious, and (b) updated so that kdump stands a chance of working even
if the KVM stub is still in place at the point the host kernel panics.

Another reason why documentation is important here is that we need to
make it clear to alternative hypervisors that the host kernel may issue
a HVC call at any moment due to a crash with particular arguments, and
that the host kernel expects a certain behaviour in that case, and that
the hypervisor does not crash.

For example, how will Xen behave - is introducing these changes going
to cause a regression with Xen?  Does anyone even know the answer to
that?  From what I can see, it seems we'll end up calling Xen's
hypervisor with a random r12 value (which it uses as a reason code)
but without the 0xea1 immediate constant in the HVC instruction.
Beyond that, I've no idea.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ 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