* [RFC PATCH v3 10/11] arm64: compat: Use vDSO sigreturn trampolines if available
From: Kevin Brodsky @ 2016-12-06 16:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206160353.14581-1-kevin.brodsky@arm.com>
If the compat vDSO is enabled, it replaces the sigreturn page.
Therefore, we use the sigreturn trampolines the vDSO provides instead.
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Nathan Lynch <nathan_lynch@mentor.com>
Cc: Christopher Covington <cov@codeaurora.org>
Cc: Dmitry Safonov <dsafonov@virtuozzo.com>
Cc: Jisheng Zhang <jszhang@marvell.com>
Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
arch/arm64/include/asm/vdso.h | 3 +++
arch/arm64/kernel/signal32.c | 15 +++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h
index 839ce0031bd5..f2a952338f1e 100644
--- a/arch/arm64/include/asm/vdso.h
+++ b/arch/arm64/include/asm/vdso.h
@@ -28,6 +28,9 @@
#ifndef __ASSEMBLY__
#include <generated/vdso-offsets.h>
+#ifdef CONFIG_VDSO32
+#include <generated/vdso32-offsets.h>
+#endif
#define VDSO_SYMBOL(base, name) \
({ \
diff --git a/arch/arm64/kernel/signal32.c b/arch/arm64/kernel/signal32.c
index 281df761e208..b6e8ff7949a4 100644
--- a/arch/arm64/kernel/signal32.c
+++ b/arch/arm64/kernel/signal32.c
@@ -28,6 +28,7 @@
#include <asm/signal32.h>
#include <asm/uaccess.h>
#include <asm/unistd.h>
+#include <asm/vdso.h>
struct compat_vfp_sigframe {
compat_ulong_t magic;
@@ -438,6 +439,19 @@ static void compat_setup_return(struct pt_regs *regs, struct k_sigaction *ka,
retcode = ptr_to_compat(ka->sa.sa_restorer);
} else {
/* Set up sigreturn pointer */
+#ifdef CONFIG_VDSO32
+ void *vdso_base = current->mm->context.vdso;
+ void *trampoline =
+ (ka->sa.sa_flags & SA_SIGINFO
+ ? (thumb
+ ? VDSO_SYMBOL(vdso_base, compat_rt_sigreturn_thumb)
+ : VDSO_SYMBOL(vdso_base, compat_rt_sigreturn_arm))
+ : (thumb
+ ? VDSO_SYMBOL(vdso_base, compat_sigreturn_thumb)
+ : VDSO_SYMBOL(vdso_base, compat_sigreturn_arm)));
+
+ retcode = ptr_to_compat(trampoline) + thumb;
+#else
void *sigreturn_base = current->mm->context.vdso;
unsigned int idx = thumb << 1;
@@ -445,6 +459,7 @@ static void compat_setup_return(struct pt_regs *regs, struct k_sigaction *ka,
idx += 3;
retcode = ptr_to_compat(sigreturn_base) + (idx << 2) + thumb;
+#endif
}
regs->regs[0] = usig;
--
2.10.2
^ permalink raw reply related
* [RFC PATCH v3 11/11] arm64: Wire up and expose the new compat vDSO
From: Kevin Brodsky @ 2016-12-06 16:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206160353.14581-1-kevin.brodsky@arm.com>
Expose the new compat vDSO via the COMPAT_VDSO config option.
The option is not enabled in defconfig because we really need a 32-bit
compiler this time, and we rely on the user to provide it themselves
by setting CROSS_COMPILE_ARM32. Therefore enabling the option by
default would make little sense, since the user must explicitly set a
non-standard environment variable anyway.
CONFIG_COMPAT_VDSO is not directly used in the code, because we want
to ignore it (build as if it were not set) if the user didn't set
CROSS_COMPILE_ARM32 properly. If the variable has been set to a valid
prefix, CONFIG_VDSO32 will be set; this is the option that the code
and Makefiles test.
For more flexibility, like CROSS_COMPILE, CROSS_COMPILE_ARM32 can also
be set via CONFIG_CROSS_COMPILE_ARM32 (the environment variable
overrides the config option, as expected).
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Nathan Lynch <nathan_lynch@mentor.com>
Cc: Christopher Covington <cov@codeaurora.org>
Cc: Dmitry Safonov <dsafonov@virtuozzo.com>
Cc: Jisheng Zhang <jszhang@marvell.com>
Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
arch/arm64/Kconfig | 23 +++++++++++++++++++++++
arch/arm64/Makefile | 28 ++++++++++++++++++++++++++--
arch/arm64/kernel/Makefile | 8 ++++++--
3 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 265d88c44cfc..721b8151ff8c 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1046,6 +1046,29 @@ config SYSVIPC_COMPAT
def_bool y
depends on COMPAT && SYSVIPC
+config COMPAT_VDSO
+ bool "32-bit vDSO"
+ depends on COMPAT
+ default n
+ help
+ Warning: a 32-bit toolchain is necessary to build the vDSO. You
+ must explicitly define which toolchain should be used by setting
+ CROSS_COMPILE_ARM32 to the prefix of the 32-bit toolchain (same format
+ as CROSS_COMPILE). If a 32-bit compiler cannot be found, a warning
+ will be printed and the kernel will be built as if COMPAT_VDSO had not
+ been set.
+
+ Provide a vDSO to 32-bit processes. It includes the symbols provided
+ by the vDSO from the 32-bit kernel, so that a 32-bit libc can use
+ the compat vDSO without modification. It also provides sigreturn
+ trampolines, replacing the sigreturn page.
+
+config CROSS_COMPILE_ARM32
+ string "32-bit toolchain prefix"
+ help
+ Same as setting CROSS_COMPILE_ARM32 in the environment, but saved for
+ future builds. The environment variable overrides this config option.
+
endmenu
menu "Power management options"
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 3635b8662724..370d8de0c100 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -37,10 +37,32 @@ $(warning LSE atomics not supported by binutils)
endif
endif
-KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr)
+ifeq ($(CONFIG_COMPAT_VDSO), y)
+ CROSS_COMPILE_ARM32 ?= $(CONFIG_CROSS_COMPILE_ARM32:"%"=%)
+
+ # Check that the user has provided a valid prefix for the 32-bit toolchain.
+ # To prevent selecting the system gcc by default, the prefix is not allowed to
+ # be empty, unlike CROSS_COMPILE. In the unlikely event that the system gcc
+ # is actually the 32-bit ARM compiler to be used, the variable can be set to
+ # the dirname (e.g. CROSS_COMPILE_ARM32=/usr/bin/).
+ # Note: this Makefile is read both before and after regenerating the
+ # config (if needed). Any warning appearing before the config has been
+ # regenerated should be ignored.
+ ifeq ($(CROSS_COMPILE_ARM32),)
+ $(warning CROSS_COMPILE_ARM32 not defined or empty, the compat vDSO will not be built)
+ else ifeq ($(shell which $(CROSS_COMPILE_ARM32)gcc 2> /dev/null),)
+ $(warning $(CROSS_COMPILE_ARM32)gcc not found, the compat vDSO will not be built)
+ else
+ export CROSS_COMPILE_ARM32
+ export CONFIG_VDSO32 := y
+ vdso32 := -DCONFIG_VDSO32=1
+ endif
+endif
+
+KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(vdso32)
KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
KBUILD_CFLAGS += $(call cc-option, -mpc-relative-literal-loads)
-KBUILD_AFLAGS += $(lseinstr)
+KBUILD_AFLAGS += $(lseinstr) $(vdso32)
ifeq ($(CONFIG_CPU_BIG_ENDIAN), y)
KBUILD_CPPFLAGS += -mbig-endian
@@ -139,6 +161,8 @@ archclean:
prepare: vdso_prepare
vdso_prepare: prepare0
$(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso include/generated/vdso-offsets.h
+ $(if $(CONFIG_VDSO32),$(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 \
+ include/generated/vdso32-offsets.h)
define archhelp
echo '* Image.gz - Compressed kernel image (arch/$(ARCH)/boot/Image.gz)'
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 0850506d0217..b0e1005037cd 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -27,8 +27,11 @@ OBJCOPYFLAGS := --prefix-symbols=__efistub_
$(obj)/%.stub.o: $(obj)/%.o FORCE
$(call if_changed,objcopy)
-arm64-obj-$(CONFIG_COMPAT) += sys32.o sigreturn32.o signal32.o \
- sys_compat.o entry32.o
+arm64-obj-$(CONFIG_COMPAT) += sys32.o signal32.o sys_compat.o \
+ entry32.o
+ifneq ($(CONFIG_VDSO32),y)
+arm64-obj-y += sigreturn32.o
+endif
arm64-obj-$(CONFIG_KUSER_HELPERS) += kuser32.o
arm64-obj-$(CONFIG_FUNCTION_TRACER) += ftrace.o entry-ftrace.o
arm64-obj-$(CONFIG_MODULES) += arm64ksyms.o module.o
@@ -53,6 +56,7 @@ arm64-obj-$(CONFIG_KEXEC) += machine_kexec.o relocate_kernel.o \
cpu-reset.o
obj-y += $(arm64-obj-y) vdso/ probes/
+obj-$(CONFIG_VDSO32) += vdso32/
obj-m += $(arm64-obj-m)
head-y := head.o
extra-y += $(head-y) vmlinux.lds
--
2.10.2
^ permalink raw reply related
* [PATCH v4 2/3] doc: dt: add cyclone-spi binding document
From: Rob Herring @ 2016-12-06 16:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <137f03de76e5f865430b17ca247e8f73e3315c8d.1480551148.git.stillcompiling@gmail.com>
On Thu, Dec 01, 2016 at 09:04:51AM -0800, Joshua Clayton wrote:
> Describe a cyclonei-ps-spi devicetree entry, required features
cyclonei?
>
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
> ---
>
> .../bindings/fpga/cyclone-ps-spi-fpga-mgr.txt | 25 ++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCHv4 05/10] arm64: Use __pa_symbol for kernel symbols
From: Mark Rutland @ 2016-12-06 16:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <584011CB.3050505@arm.com>
On Thu, Dec 01, 2016 at 12:04:27PM +0000, James Morse wrote:
> On 29/11/16 18:55, Laura Abbott wrote:
> > diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> > index d55a7b0..4f0c77d 100644
> > --- a/arch/arm64/kernel/hibernate.c
> > +++ b/arch/arm64/kernel/hibernate.c
> > @@ -484,7 +481,7 @@ int swsusp_arch_resume(void)
> > * Since we only copied the linear map, we need to find restore_pblist's
> > * linear map address.
> > */
> > - lm_restore_pblist = LMADDR(restore_pblist);
> > + lm_restore_pblist = lm_alias(restore_pblist);
> >
> > /*
> > * We need a zero page that is zero before & after resume in order to
>
> This change causes resume from hibernate to panic in:
> > VIRTUAL_BUG_ON(x < (unsigned long) KERNEL_START ||
> > x > (unsigned long) KERNEL_END);
>
> It looks like kaslr's relocation code has already fixed restore_pblist, so your
> debug virtual check catches this doing the wrong thing. My bug.
>
> readelf -s vmlinux | grep ...
> > 103495: ffff000008080000 0 NOTYPE GLOBAL DEFAULT 1 _text
> > 92104: ffff000008e43860 8 OBJECT GLOBAL DEFAULT 24 restore_pblist
> > 105442: ffff000008e85000 0 NOTYPE GLOBAL DEFAULT 24 _end
>
> But restore_pblist == 0xffff800971b7f998 when passed to __phys_addr_symbol().
I think KASLR's a red herring; it shouldn't change the distance between
the restore_pblist symbol and {_text,_end}.
Above, ffff000008e43860 is the location of the pointer in the kernel
image (i.e. it's &restore_pblist). 0xffff800971b7f998 is the pointer
that was assigned to restore_pblist. For KASLR, the low bits (at least
up to a page boundary) shouldn't change across relocation.
Assuming it's only ever assigned a dynamic allocation, which should fall
in the linear map, the LMADDR() dance doesn't appear to be necessary.
> This fixes the problem:
> ----------------%<----------------
> diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> index 4f0c77d2ff7a..8bed26a2d558 100644
> --- a/arch/arm64/kernel/hibernate.c
> +++ b/arch/arm64/kernel/hibernate.c
> @@ -457,7 +457,6 @@ int swsusp_arch_resume(void)
> void *zero_page;
> size_t exit_size;
> pgd_t *tmp_pg_dir;
> - void *lm_restore_pblist;
> phys_addr_t phys_hibernate_exit;
> void __noreturn (*hibernate_exit)(phys_addr_t, phys_addr_t, void *,
> void *, phys_addr_t, phys_addr_t);
> @@ -478,12 +477,6 @@ int swsusp_arch_resume(void)
> goto out;
>
> /*
> - * Since we only copied the linear map, we need to find restore_pblist's
> - * linear map address.
> - */
> - lm_restore_pblist = lm_alias(restore_pblist);
> -
> - /*
> * We need a zero page that is zero before & after resume in order to
> * to break before make on the ttbr1 page tables.
> */
> @@ -534,7 +527,7 @@ int swsusp_arch_resume(void)
> }
>
> hibernate_exit(virt_to_phys(tmp_pg_dir), resume_hdr.ttbr1_el1,
> - resume_hdr.reenter_kernel, lm_restore_pblist,
> + resume_hdr.reenter_kernel, restore_pblist,
> resume_hdr.__hyp_stub_vectors, virt_to_phys(zero_page));
>
> out:
> ----------------%<----------------
Folding that in (or having it as a preparatory cleanup patch) makes
sense to me. AFAICT the logic was valid (albeit confused) until now, so
it's not strictly a fix.
Thanks,
Mark.
^ permalink raw reply
* [PATCH 1/1] arm64: Correcting format specifier for printing 64 bit addresses
From: Christoffer Dall @ 2016-12-06 16:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480925393-8386-1-git-send-email-maninder1.s@samsung.com>
On Mon, Dec 05, 2016 at 01:39:53PM +0530, Maninder Singh wrote:
> This patch corrects format specifier for printing 64 bit addresses.
>
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
> ---
> arch/arm64/kernel/signal.c | 2 +-
> arch/arm64/kvm/sys_regs.c | 8 ++++++--
> arch/arm64/mm/fault.c | 15 ++++++++++-----
> arch/arm64/mm/mmu.c | 4 ++--
> 4 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> index c7b6de6..c89d5fd 100644
> --- a/arch/arm64/kernel/signal.c
> +++ b/arch/arm64/kernel/signal.c
> @@ -155,7 +155,7 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
>
> badframe:
> if (show_unhandled_signals)
> - pr_info_ratelimited("%s[%d]: bad frame in %s: pc=%08llx sp=%08llx\n",
> + pr_info_ratelimited("%s[%d]: bad frame in %s: pc=%016llx sp=%016llx\n",
> current->comm, task_pid_nr(current), __func__,
> regs->pc, regs->sp);
> force_sig(SIGSEGV, current);
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 87e7e66..89bf5c1 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1554,8 +1554,12 @@ static void unhandled_cp_access(struct kvm_vcpu *vcpu,
> WARN_ON(1);
> }
>
> - kvm_err("Unsupported guest CP%d access at: %08lx\n",
> - cp, *vcpu_pc(vcpu));
> + if (params->is_32bit)
> + kvm_err("Unsupported guest CP%d access at: %08lx\n",
> + cp, *vcpu_pc(vcpu));
> + else
> + kvm_err("Unsupported guest CP%d access at: %016lx\n",
> + cp, *vcpu_pc(vcpu));
It feels a bit much to me to have an if-statement to differentiate the
number of leading zeros, so if it's important to always have fixed
widths then I would just use %016lx in both cases.
> print_sys_reg_instr(params);
> kvm_inject_undefined(vcpu);
> }
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index a78a5c4..d96a42a 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -77,7 +77,7 @@ void show_pte(struct mm_struct *mm, unsigned long addr)
>
> pr_alert("pgd = %p\n", mm->pgd);
> pgd = pgd_offset(mm, addr);
> - pr_alert("[%08lx] *pgd=%016llx", addr, pgd_val(*pgd));
> + pr_alert("[%016lx] *pgd=%016llx", addr, pgd_val(*pgd));
>
> do {
> pud_t *pud;
> @@ -177,7 +177,7 @@ static void __do_kernel_fault(struct mm_struct *mm, unsigned long addr,
> * No handler, we'll have to terminate things with extreme prejudice.
> */
> bust_spinlocks(1);
> - pr_alert("Unable to handle kernel %s at virtual address %08lx\n",
> + pr_alert("Unable to handle kernel %s at virtual address %016lx\n",
> (addr < PAGE_SIZE) ? "NULL pointer dereference" :
> "paging request", addr);
>
> @@ -198,9 +198,14 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
> struct siginfo si;
>
> if (unhandled_signal(tsk, sig) && show_unhandled_signals_ratelimited()) {
> - pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x\n",
> - tsk->comm, task_pid_nr(tsk), fault_name(esr), sig,
> - addr, esr);
> + if (compat_user_mode(regs))
> + pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x\n",
> + tsk->comm, task_pid_nr(tsk), fault_name(esr), sig,
> + addr, esr);
> + else
> + pr_info("%s[%d]: unhandled %s (%d) at 0x%016lx, esr 0x%03x\n",
> + tsk->comm, task_pid_nr(tsk), fault_name(esr), sig,
> + addr, esr);
same here.
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH v2 2/2] arm64: Work around broken .inst when defective gas is detected
From: Dave Martin @ 2016-12-06 16:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481038065-32153-4-git-send-email-marc.zyngier@arm.com>
On Tue, Dec 06, 2016 at 03:27:45PM +0000, Marc Zyngier wrote:
> .inst being largely broken with older binutils, it'd be better not
> to emit it altogether when detecting such configuration (as it
> leads to all kind of horrors when using alternatives).
GAS seems to provide few guarantees about the constantness of anything
other than an expression composed purely of literal constants. So,
this may rather be lack of a useful feature rather than a bug per se.
> Generalize the __emit_inst macro and use it extensively in
> asm/sysreg.h, and make it generate a .long when a broken gas is
> detected. The disassembly will be crap, but at least we can write
> semi-sane code.
Don't know how much this helps, but if we want decent disassembly then
one option is to use macros instead of symbols, so that the argument to
.inst involves only literals after macro expansion.
Something like:
.ifndef .L__with_reg_defined
.set .L__with_reg_defined, 1
.macro __def_reg name, num
.macro __with_reg__\name instr:vararg
\instr \num
.endm
.endm
.irp n, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30
__def_reg x\n, \n
.endr
__def_reg xzr, 31
.macro __with_reg regname, instr:vararg
__with_reg__\regname \instr
.endm
.endif
.ifndef .L__msr_s_defined
.set .L__msr_s_defined, 1
.macro __msr_s sreg, rn
.inst 0xd5000000 | (\sreg) | (\rn)
.endm
.macro msr_s sreg, rn
__with_reg \rn, __msr_s \sreg,
.endm
.endif
For the inline asm case, gcc will think that the asm assembles to much
larger code that it really does in this, but I suspect this won't matter
in realistic situations.
For the .S case, we would define the macros once only, in the header (as
at present) -- the guards won't be necessary for that.
Cheers
---Dave
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/include/asm/sysreg.h | 29 +++++++++++++++++++++++++----
> 1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 9e16a18..98ae03f 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -35,12 +35,33 @@
> #define sys_reg(op0, op1, crn, crm, op2) \
> ((((op0)&3)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
>
> +#ifndef CONFIG_BROKEN_GAS_INST
> +
> #ifdef __ASSEMBLY__
> #define __emit_inst(x) .inst (x)
> #else
> #define __emit_inst(x) ".inst " __stringify((x)) "\n\t"
> #endif
>
> +#else /* CONFIG_BROKEN_GAS_INST */
> +
> +#ifndef CONFIG_CPU_BIG_ENDIAN
> +#define __INSTR_BSWAP(x) (x)
> +#else /* CONFIG_CPU_BIG_ENDIAN */
> +#define __INSTR_BSWAP(x) ((((x) << 24) & 0xff000000) | \
> + (((x) << 8) & 0x00ff0000) | \
> + (((x) >> 8) & 0x0000ff00) | \
> + (((x) >> 24) & 0x000000ff))
> +#endif /* CONFIG_CPU_BIG_ENDIAN */
> +
> +#ifdef __ASSEMBLY__
> +#define __emit_inst(x) .long __INSTR_BSWAP(x)
> +#else /* __ASSEMBLY__ */
> +#define __emit_inst(x) ".long " __stringify(__INSTR_BSWAP(x)) "\n\t"
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* CONFIG_BROKEN_GAS_INST */
> +
> #define SYS_MIDR_EL1 sys_reg(3, 0, 0, 0, 0)
> #define SYS_MPIDR_EL1 sys_reg(3, 0, 0, 0, 5)
> #define SYS_REVIDR_EL1 sys_reg(3, 0, 0, 0, 6)
> @@ -232,11 +253,11 @@
> .equ .L__reg_num_xzr, 31
>
> .macro mrs_s, rt, sreg
> - .inst 0xd5200000|(\sreg)|(.L__reg_num_\rt)
> + __emit_inst(0xd5200000|(\sreg)|(.L__reg_num_\rt))
> .endm
>
> .macro msr_s, sreg, rt
> - .inst 0xd5000000|(\sreg)|(.L__reg_num_\rt)
> + __emit_inst(0xd5000000|(\sreg)|(.L__reg_num_\rt))
> .endm
>
> #else
> @@ -250,11 +271,11 @@ asm(
> " .equ .L__reg_num_xzr, 31\n"
> "\n"
> " .macro mrs_s, rt, sreg\n"
> -" .inst 0xd5200000|(\\sreg)|(.L__reg_num_\\rt)\n"
> + __emit_inst(0xd5200000|(\\sreg)|(.L__reg_num_\\rt))
> " .endm\n"
> "\n"
> " .macro msr_s, sreg, rt\n"
> -" .inst 0xd5000000|(\\sreg)|(.L__reg_num_\\rt)\n"
> + __emit_inst(0xd5000000|(\\sreg)|(.L__reg_num_\\rt))
> " .endm\n"
> );
>
> --
> 2.1.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2] arm64: KVM: pmu: Reset PMSELR_EL0.SEL to a sane value before entering the guest
From: Christoffer Dall @ 2016-12-06 16:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481036210-31816-1-git-send-email-marc.zyngier@arm.com>
On Tue, Dec 06, 2016 at 02:56:50PM +0000, Marc Zyngier wrote:
> The ARMv8 architecture allows the cycle counter to be configured
> by setting PMSELR_EL0.SEL==0x1f and then accessing PMXEVTYPER_EL0,
> hence accessing PMCCFILTR_EL0. But it disallows the use of
> PMSELR_EL0.SEL==0x1f to access the cycle counter itself through
> PMXEVCNTR_EL0.
>
> Linux itself doesn't violate this rule, but we may end up with
> PMSELR_EL0.SEL being set to 0x1f when we enter a guest. If that
> guest accesses PMXEVCNTR_EL0, the access may UNDEF at EL1,
> despite the guest not having done anything wrong.
>
> In order to avoid this unfortunate course of events (haha!), let's
I actually did find this funny.
> sanitize PMSELR_EL0 on guest entry. This ensures that the guest
> won't explode unexpectedly.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> This is another approach to fix this issue, this time nuking PMSELR_EL0
> on guest entry instead of relying on perf not to clobber the register.
>
> Tested on v4.9-rc8 with a Rev A3 X-Gene.
>
> arch/arm64/kvm/hyp/switch.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 83037cd..3b7cfbd 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -85,7 +85,13 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
> write_sysreg(val, hcr_el2);
> /* Trap on AArch32 cp15 c15 accesses (EL1 or EL0) */
> write_sysreg(1 << 15, hstr_el2);
> - /* Make sure we trap PMU access from EL0 to EL2 */
> + /*
> + * Make sure we trap PMU access from EL0 to EL2. Also sanitize
> + * PMSELR_EL0 to make sure it never contains the cycle
> + * counter, which could make a PMXEVCNTR_EL0 access UNDEF.
> + */
> + if (vcpu->arch.mdcr_el2 & MDCR_EL2_HPMN_MASK)
> + write_sysreg(0, pmselr_el0);
I'm a bit confused about how we use the HPMN field. This value is
always set to the full number of counters available on the system and
never modified by the guest, right? So this is in essence a check that
says 'do you have any performance counters, then make sure accesses
don't undef to el1 instead of trapping to el2', but then my question is,
why not just set pmselr_el0 to zero unconditionally, because in the case
where (vcpu->arch.mdcr_el2 & MDCR_EL2_HPMN_MASK) == 0, it means we have
no counters, which we'll have exposed to the guest anyhow, and we should
undef at el1 in the guest, or did I get this completely wrong (like
everything else today)?
> write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0);
> write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
> __activate_traps_arch()();
> --
> 2.1.4
>
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH] i2c: rk3x: keep i2c irq ON in suspend
From: Doug Anderson @ 2016-12-06 16:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1d570232-af87-467e-e6f4-5b514d583a02@rock-chips.com>
Hi,
On Tue, Dec 6, 2016 at 12:12 AM, David.Wu <david.wu@rock-chips.com> wrote:
> Hi Heiko,
>
> ? 2016/12/5 18:54, Heiko Stuebner ??:
>>
>> Hi David,
>>
>> Am Montag, 5. Dezember 2016, 16:02:59 CET schrieb David Wu:
>>>
>>> During suspend there may still be some i2c access happening.
>>> And if we don't keep i2c irq ON, there may be i2c access timeout if
>>> i2c is in irq mode of operation.
>>
>>
>> can you describe the issue you're trying to fix a bit more please?
>
>
> Sometimes we could see the i2c timeout errors during suspend/resume, which
> makes the duration of suspend/resume too longer.
>
> [ 484.171541] CPU4: Booted secondary processor [410fd082]
> [ 485.172777] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
> [ 486.172760] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
> [ 487.172759] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
> [ 487.172840] cpu cpu4: _set_opp_voltage: failed to set voltage (800000
> 800000 800000 mV): -110
> [ 487.172874] cpu cpu4: failed to set volt 800000
>
>>
>> I.e. I'd think the i2c-core does suspend i2c-client devices first, so that
>> these should be able to finish up their ongoing transfers and not start
>> any
>> new ones instead?
>>
>> Your irq can still happen slightly after the system started going to
>> actually
>> sleep, so to me it looks like you just widened the window where irqs can
>> be
>> handled. Especially as your irq could also just simply stem from the start
>> state, so you cannot even be sure if your transaction actually is
>> finished.
>
>
> Okay, you are right. I want to give it a double insurance at first, but it
> may hide the unhappend issue.
>
>>
>> So to me it looks like the i2c-connected device driver should be fixed
>> instead?
>
>
> I tell them to fix it in rk808 driver.
To me it seems like perhaps cpufreq should not be changing frequencies
until it is resumed properly. Presumably if all the ordering is done
right then cpufreq should be resumed _after_ the i2c regulator so you
should be OK. ...or am I somehow confused about that?
Also note that previous i2c busses I worked with simply returned -EIO
in the case where they were called when suspended. See
"i2c-exynos5.c" and "i2c-s3c2410.c".
-Doug
^ permalink raw reply
* [PATCH 1/1] arm64: Correcting format specifier for printing 64 bit addresses
From: Robin Murphy @ 2016-12-06 16:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206161116.GD4816@cbox>
On 06/12/16 16:11, Christoffer Dall wrote:
> On Mon, Dec 05, 2016 at 01:39:53PM +0530, Maninder Singh wrote:
>> This patch corrects format specifier for printing 64 bit addresses.
>>
>> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
>> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
>> ---
>> arch/arm64/kernel/signal.c | 2 +-
>> arch/arm64/kvm/sys_regs.c | 8 ++++++--
>> arch/arm64/mm/fault.c | 15 ++++++++++-----
>> arch/arm64/mm/mmu.c | 4 ++--
>> 4 files changed, 19 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
>> index c7b6de6..c89d5fd 100644
>> --- a/arch/arm64/kernel/signal.c
>> +++ b/arch/arm64/kernel/signal.c
>> @@ -155,7 +155,7 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
>>
>> badframe:
>> if (show_unhandled_signals)
>> - pr_info_ratelimited("%s[%d]: bad frame in %s: pc=%08llx sp=%08llx\n",
>> + pr_info_ratelimited("%s[%d]: bad frame in %s: pc=%016llx sp=%016llx\n",
>> current->comm, task_pid_nr(current), __func__,
>> regs->pc, regs->sp);
>> force_sig(SIGSEGV, current);
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index 87e7e66..89bf5c1 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -1554,8 +1554,12 @@ static void unhandled_cp_access(struct kvm_vcpu *vcpu,
>> WARN_ON(1);
>> }
>>
>> - kvm_err("Unsupported guest CP%d access at: %08lx\n",
>> - cp, *vcpu_pc(vcpu));
>> + if (params->is_32bit)
>> + kvm_err("Unsupported guest CP%d access at: %08lx\n",
>> + cp, *vcpu_pc(vcpu));
>> + else
>> + kvm_err("Unsupported guest CP%d access at: %016lx\n",
>> + cp, *vcpu_pc(vcpu));
>
> It feels a bit much to me to have an if-statement to differentiate the
> number of leading zeros, so if it's important to always have fixed
> widths then I would just use %016lx in both cases.
Actually, it looks like vsnprintf does support the '*' field width
specifier, so even if the format _is_ critical there's still no reason
to have such duplicated code.
Robin.
>> print_sys_reg_instr(params);
>> kvm_inject_undefined(vcpu);
>> }
>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
>> index a78a5c4..d96a42a 100644
>> --- a/arch/arm64/mm/fault.c
>> +++ b/arch/arm64/mm/fault.c
>> @@ -77,7 +77,7 @@ void show_pte(struct mm_struct *mm, unsigned long addr)
>>
>> pr_alert("pgd = %p\n", mm->pgd);
>> pgd = pgd_offset(mm, addr);
>> - pr_alert("[%08lx] *pgd=%016llx", addr, pgd_val(*pgd));
>> + pr_alert("[%016lx] *pgd=%016llx", addr, pgd_val(*pgd));
>>
>> do {
>> pud_t *pud;
>> @@ -177,7 +177,7 @@ static void __do_kernel_fault(struct mm_struct *mm, unsigned long addr,
>> * No handler, we'll have to terminate things with extreme prejudice.
>> */
>> bust_spinlocks(1);
>> - pr_alert("Unable to handle kernel %s at virtual address %08lx\n",
>> + pr_alert("Unable to handle kernel %s at virtual address %016lx\n",
>> (addr < PAGE_SIZE) ? "NULL pointer dereference" :
>> "paging request", addr);
>>
>> @@ -198,9 +198,14 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
>> struct siginfo si;
>>
>> if (unhandled_signal(tsk, sig) && show_unhandled_signals_ratelimited()) {
>> - pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x\n",
>> - tsk->comm, task_pid_nr(tsk), fault_name(esr), sig,
>> - addr, esr);
>> + if (compat_user_mode(regs))
>> + pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x\n",
>> + tsk->comm, task_pid_nr(tsk), fault_name(esr), sig,
>> + addr, esr);
>> + else
>> + pr_info("%s[%d]: unhandled %s (%d) at 0x%016lx, esr 0x%03x\n",
>> + tsk->comm, task_pid_nr(tsk), fault_name(esr), sig,
>> + addr, esr);
>
> same here.
>
> Thanks,
> -Christoffer
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH v2] arm64: KVM: pmu: Reset PMSELR_EL0.SEL to a sane value before entering the guest
From: Will Deacon @ 2016-12-06 16:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206162918.GE4816@cbox>
On Tue, Dec 06, 2016 at 05:29:18PM +0100, Christoffer Dall wrote:
> On Tue, Dec 06, 2016 at 02:56:50PM +0000, Marc Zyngier wrote:
> > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > index 83037cd..3b7cfbd 100644
> > --- a/arch/arm64/kvm/hyp/switch.c
> > +++ b/arch/arm64/kvm/hyp/switch.c
> > @@ -85,7 +85,13 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
> > write_sysreg(val, hcr_el2);
> > /* Trap on AArch32 cp15 c15 accesses (EL1 or EL0) */
> > write_sysreg(1 << 15, hstr_el2);
> > - /* Make sure we trap PMU access from EL0 to EL2 */
> > + /*
> > + * Make sure we trap PMU access from EL0 to EL2. Also sanitize
> > + * PMSELR_EL0 to make sure it never contains the cycle
> > + * counter, which could make a PMXEVCNTR_EL0 access UNDEF.
> > + */
> > + if (vcpu->arch.mdcr_el2 & MDCR_EL2_HPMN_MASK)
> > + write_sysreg(0, pmselr_el0);
>
> I'm a bit confused about how we use the HPMN field. This value is
> always set to the full number of counters available on the system and
> never modified by the guest, right? So this is in essence a check that
> says 'do you have any performance counters, then make sure accesses
> don't undef to el1 instead of trapping to el2', but then my question is,
> why not just set pmselr_el0 to zero unconditionally, because in the case
> where (vcpu->arch.mdcr_el2 & MDCR_EL2_HPMN_MASK) == 0, it means we have
> no counters, which we'll have exposed to the guest anyhow, and we should
> undef at el1 in the guest, or did I get this completely wrong (like
> everything else today)?
I think Marc and I came to the same conclusion a few minutes ago. The check
you might want is "Have I instantiated a virtual PMU for this device?",
but that's probably a micro-optimisation.
Will
^ permalink raw reply
* [RFC PATCH] arm64: fpsimd: improve stacking logic in non-interruptible context
From: Dave Martin @ 2016-12-06 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481038740-11633-1-git-send-email-ard.biesheuvel@linaro.org>
On Tue, Dec 06, 2016 at 03:39:00PM +0000, Ard Biesheuvel wrote:
> Currently, we allow kernel mode NEON in softirq or hardirq context by
> stacking and unstacking a slice of the NEON register file for each call
> to kernel_neon_begin() and kernel_neon_end(), respectively.
>
> Given that
> a) a CPU typically spends most of its time in userland, during which time
> no kernel mode NEON in process context is in progress,
> b) a CPU spends most of its time in the kernel doing other things than
> kernel mode NEON when it gets interrupted to perform kernel mode NEON
> in softirq context
>
> the stacking and subsequent unstacking is only necessary if we are
> interrupting a thread while it is performing kernel mode NEON in process
> context, which means that in all other cases, we can simply preserve the
> userland FPSIMD state once, and only restore it upon return to userland,
> even if we are being invoked from softirq or hardirq context.
>
> So instead of checking whether we are running in interrupt context, keep
> track of the level of nested kernel mode NEON calls in progress, and only
> perform the eager stack/unstack of the level exceeds 1.
Looks reasonable, and this does avoid treating SVE as a special case.
Later on, we may want to enable SVE use in the kernel too, in which case
kernel_neon_{begin,end}_partial() and fpsimd_partial_state will need
more changes -- but we don't need to address that right now.
Minor comments below.
Cheers
---Dave
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/kernel/fpsimd.c | 40 +++++++++++++++++++++++++++-------------
> 1 file changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 394c61db5566..2614a216ac5d 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -220,20 +220,31 @@ void fpsimd_flush_task_state(struct task_struct *t)
>
> #ifdef CONFIG_KERNEL_MODE_NEON
>
> -static DEFINE_PER_CPU(struct fpsimd_partial_state, hardirq_fpsimdstate);
> -static DEFINE_PER_CPU(struct fpsimd_partial_state, softirq_fpsimdstate);
> +/*
> + * Although unlikely, it is possible for three kernel mode NEON contexts to
> + * be live at the same time: process context, softirq context and hardirq
> + * context. So while the userland context is stashed in the thread's fpsimd
> + * state structure, we need two additional levels of storage.
> + */
> +static DEFINE_PER_CPU(struct fpsimd_partial_state, nested_fpsimdstate[2]);
> +static DEFINE_PER_CPU(int, kernel_neon_nesting_level);
>
> /*
> * Kernel-side NEON support functions
> */
> void kernel_neon_begin_partial(u32 num_regs)
> {
> - if (in_interrupt()) {
> - struct fpsimd_partial_state *s = this_cpu_ptr(
> - in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
> + struct fpsimd_partial_state *s;
> + int level;
> +
> + preempt_disable();
> +
> + level = this_cpu_read(kernel_neon_nesting_level);
> + if (level > 0) {
> + s = this_cpu_ptr(nested_fpsimdstate);
>
> BUG_ON(num_regs > 32);
> - fpsimd_save_partial_state(s, roundup(num_regs, 2));
> + fpsimd_save_partial_state(&s[level - 1], roundup(num_regs, 2));
> } else {
> /*
> * Save the userland FPSIMD state if we have one and if we
> @@ -241,24 +252,27 @@ void kernel_neon_begin_partial(u32 num_regs)
> * that there is no longer userland FPSIMD state in the
> * registers.
> */
> - preempt_disable();
> if (current->mm &&
> !test_and_set_thread_flag(TIF_FOREIGN_FPSTATE))
> fpsimd_save_state(¤t->thread.fpsimd_state);
> this_cpu_write(fpsimd_last_state, NULL);
> }
> + this_cpu_write(kernel_neon_nesting_level, level + 1);
Should we BUG_ON overflow/underflow of the nesting level? That Should
Not Happen (tm), but we'll make a mess if it does.
For the underflow case, perhaps DEBUG_PREEMPT is adequate for detecting
this via preempt count underflow.
> }
> EXPORT_SYMBOL(kernel_neon_begin_partial);
>
> void kernel_neon_end(void)
> {
> - if (in_interrupt()) {
> - struct fpsimd_partial_state *s = this_cpu_ptr(
> - in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
> - fpsimd_load_partial_state(s);
> - } else {
> - preempt_enable();
> + struct fpsimd_partial_state *s;
> + int level;
> +
> + level = this_cpu_read(kernel_neon_nesting_level);
> + if (level > 1) {
> + s = this_cpu_ptr(nested_fpsimdstate);
> + fpsimd_load_partial_state(&s[level - 2]);
> }
> + this_cpu_write(kernel_neon_nesting_level, level - 1);
> + preempt_enable();
> }
> EXPORT_SYMBOL(kernel_neon_end);
>
> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 1/1] arm64: Correcting format specifier for printing 64 bit addresses
From: Robin Murphy @ 2016-12-06 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480925393-8386-1-git-send-email-maninder1.s@samsung.com>
On 05/12/16 08:09, Maninder Singh wrote:
> This patch corrects format specifier for printing 64 bit addresses.
>
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
> ---
> arch/arm64/kernel/signal.c | 2 +-
> arch/arm64/kvm/sys_regs.c | 8 ++++++--
> arch/arm64/mm/fault.c | 15 ++++++++++-----
> arch/arm64/mm/mmu.c | 4 ++--
> 4 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> index c7b6de6..c89d5fd 100644
> --- a/arch/arm64/kernel/signal.c
> +++ b/arch/arm64/kernel/signal.c
> @@ -155,7 +155,7 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
>
> badframe:
> if (show_unhandled_signals)
> - pr_info_ratelimited("%s[%d]: bad frame in %s: pc=%08llx sp=%08llx\n",
> + pr_info_ratelimited("%s[%d]: bad frame in %s: pc=%016llx sp=%016llx\n",
> current->comm, task_pid_nr(current), __func__,
> regs->pc, regs->sp);
> force_sig(SIGSEGV, current);
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 87e7e66..89bf5c1 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1554,8 +1554,12 @@ static void unhandled_cp_access(struct kvm_vcpu *vcpu,
> WARN_ON(1);
> }
>
> - kvm_err("Unsupported guest CP%d access at: %08lx\n",
> - cp, *vcpu_pc(vcpu));
> + if (params->is_32bit)
> + kvm_err("Unsupported guest CP%d access at: %08lx\n",
> + cp, *vcpu_pc(vcpu));
> + else
> + kvm_err("Unsupported guest CP%d access at: %016lx\n",
> + cp, *vcpu_pc(vcpu));
As with the other patch - use '%0*lx' in these cases rather than
pointlessly duplicating everything.
Robin.
> print_sys_reg_instr(params);
> kvm_inject_undefined(vcpu);
> }
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index a78a5c4..d96a42a 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -77,7 +77,7 @@ void show_pte(struct mm_struct *mm, unsigned long addr)
>
> pr_alert("pgd = %p\n", mm->pgd);
> pgd = pgd_offset(mm, addr);
> - pr_alert("[%08lx] *pgd=%016llx", addr, pgd_val(*pgd));
> + pr_alert("[%016lx] *pgd=%016llx", addr, pgd_val(*pgd));
>
> do {
> pud_t *pud;
> @@ -177,7 +177,7 @@ static void __do_kernel_fault(struct mm_struct *mm, unsigned long addr,
> * No handler, we'll have to terminate things with extreme prejudice.
> */
> bust_spinlocks(1);
> - pr_alert("Unable to handle kernel %s at virtual address %08lx\n",
> + pr_alert("Unable to handle kernel %s at virtual address %016lx\n",
> (addr < PAGE_SIZE) ? "NULL pointer dereference" :
> "paging request", addr);
>
> @@ -198,9 +198,14 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
> struct siginfo si;
>
> if (unhandled_signal(tsk, sig) && show_unhandled_signals_ratelimited()) {
> - pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x\n",
> - tsk->comm, task_pid_nr(tsk), fault_name(esr), sig,
> - addr, esr);
> + if (compat_user_mode(regs))
> + pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x\n",
> + tsk->comm, task_pid_nr(tsk), fault_name(esr), sig,
> + addr, esr);
> + else
> + pr_info("%s[%d]: unhandled %s (%d) at 0x%016lx, esr 0x%03x\n",
> + tsk->comm, task_pid_nr(tsk), fault_name(esr), sig,
> + addr, esr);
> show_pte(tsk->mm, addr);
> show_regs(regs);
> }
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 17243e4..cbf444c 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -683,9 +683,9 @@ void __init early_fixmap_init(void)
> pr_warn("pmd %p != %p, %p\n",
> pmd, fixmap_pmd(fix_to_virt(FIX_BTMAP_BEGIN)),
> fixmap_pmd(fix_to_virt(FIX_BTMAP_END)));
> - pr_warn("fix_to_virt(FIX_BTMAP_BEGIN): %08lx\n",
> + pr_warn("fix_to_virt(FIX_BTMAP_BEGIN): %016lx\n",
> fix_to_virt(FIX_BTMAP_BEGIN));
> - pr_warn("fix_to_virt(FIX_BTMAP_END): %08lx\n",
> + pr_warn("fix_to_virt(FIX_BTMAP_END): %016lx\n",
> fix_to_virt(FIX_BTMAP_END));
>
> pr_warn("FIX_BTMAP_END: %d\n", FIX_BTMAP_END);
>
^ permalink raw reply
* [RFC PATCH] arm64: fpsimd: improve stacking logic in non-interruptible context
From: Ard Biesheuvel @ 2016-12-06 16:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206164317.GX1574@e103592.cambridge.arm.com>
On 6 December 2016 at 16:43, Dave Martin <Dave.Martin@arm.com> wrote:
> On Tue, Dec 06, 2016 at 03:39:00PM +0000, Ard Biesheuvel wrote:
>> Currently, we allow kernel mode NEON in softirq or hardirq context by
>> stacking and unstacking a slice of the NEON register file for each call
>> to kernel_neon_begin() and kernel_neon_end(), respectively.
>>
>> Given that
>> a) a CPU typically spends most of its time in userland, during which time
>> no kernel mode NEON in process context is in progress,
>> b) a CPU spends most of its time in the kernel doing other things than
>> kernel mode NEON when it gets interrupted to perform kernel mode NEON
>> in softirq context
>>
>> the stacking and subsequent unstacking is only necessary if we are
>> interrupting a thread while it is performing kernel mode NEON in process
>> context, which means that in all other cases, we can simply preserve the
>> userland FPSIMD state once, and only restore it upon return to userland,
>> even if we are being invoked from softirq or hardirq context.
>>
>> So instead of checking whether we are running in interrupt context, keep
>> track of the level of nested kernel mode NEON calls in progress, and only
>> perform the eager stack/unstack of the level exceeds 1.
>
> Looks reasonable, and this does avoid treating SVE as a special case.
>
> Later on, we may want to enable SVE use in the kernel too, in which case
> kernel_neon_{begin,end}_partial() and fpsimd_partial_state will need
> more changes -- but we don't need to address that right now.
>
> Minor comments below.
>
> Cheers
> ---Dave
>
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> arch/arm64/kernel/fpsimd.c | 40 +++++++++++++++++++++++++++-------------
>> 1 file changed, 27 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
>> index 394c61db5566..2614a216ac5d 100644
>> --- a/arch/arm64/kernel/fpsimd.c
>> +++ b/arch/arm64/kernel/fpsimd.c
>> @@ -220,20 +220,31 @@ void fpsimd_flush_task_state(struct task_struct *t)
>>
>> #ifdef CONFIG_KERNEL_MODE_NEON
>>
>> -static DEFINE_PER_CPU(struct fpsimd_partial_state, hardirq_fpsimdstate);
>> -static DEFINE_PER_CPU(struct fpsimd_partial_state, softirq_fpsimdstate);
>> +/*
>> + * Although unlikely, it is possible for three kernel mode NEON contexts to
>> + * be live at the same time: process context, softirq context and hardirq
>> + * context. So while the userland context is stashed in the thread's fpsimd
>> + * state structure, we need two additional levels of storage.
>> + */
>> +static DEFINE_PER_CPU(struct fpsimd_partial_state, nested_fpsimdstate[2]);
>> +static DEFINE_PER_CPU(int, kernel_neon_nesting_level);
>>
>> /*
>> * Kernel-side NEON support functions
>> */
>> void kernel_neon_begin_partial(u32 num_regs)
>> {
>> - if (in_interrupt()) {
>> - struct fpsimd_partial_state *s = this_cpu_ptr(
>> - in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
>> + struct fpsimd_partial_state *s;
>> + int level;
>> +
>> + preempt_disable();
>> +
>> + level = this_cpu_read(kernel_neon_nesting_level);
>> + if (level > 0) {
>> + s = this_cpu_ptr(nested_fpsimdstate);
>>
>> BUG_ON(num_regs > 32);
>> - fpsimd_save_partial_state(s, roundup(num_regs, 2));
>> + fpsimd_save_partial_state(&s[level - 1], roundup(num_regs, 2));
>> } else {
>> /*
>> * Save the userland FPSIMD state if we have one and if we
>> @@ -241,24 +252,27 @@ void kernel_neon_begin_partial(u32 num_regs)
>> * that there is no longer userland FPSIMD state in the
>> * registers.
>> */
>> - preempt_disable();
>> if (current->mm &&
>> !test_and_set_thread_flag(TIF_FOREIGN_FPSTATE))
>> fpsimd_save_state(¤t->thread.fpsimd_state);
>> this_cpu_write(fpsimd_last_state, NULL);
>> }
>> + this_cpu_write(kernel_neon_nesting_level, level + 1);
>
> Should we BUG_ON overflow/underflow of the nesting level? That Should
> Not Happen (tm), but we'll make a mess if it does.
>
True.
> For the underflow case, perhaps DEBUG_PREEMPT is adequate for detecting
> this via preempt count underflow.
>
I think it makes sense for the increment to check for overflow and the
decrement to check for underflow, regardless of whether DEBUG_PREEMPT
(or just PREEMPT) is enabled or not.
>> }
>> EXPORT_SYMBOL(kernel_neon_begin_partial);
>>
>> void kernel_neon_end(void)
>> {
>> - if (in_interrupt()) {
>> - struct fpsimd_partial_state *s = this_cpu_ptr(
>> - in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
>> - fpsimd_load_partial_state(s);
>> - } else {
>> - preempt_enable();
>> + struct fpsimd_partial_state *s;
>> + int level;
>> +
>> + level = this_cpu_read(kernel_neon_nesting_level);
>> + if (level > 1) {
>> + s = this_cpu_ptr(nested_fpsimdstate);
>> + fpsimd_load_partial_state(&s[level - 2]);
>> }
>> + this_cpu_write(kernel_neon_nesting_level, level - 1);
>> + preempt_enable();
>> }
>> EXPORT_SYMBOL(kernel_neon_end);
>>
>> --
>> 2.7.4
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v4 1/4] [media] davinci: vpif_capture: don't lock over s_stream
From: Kevin Hilman @ 2016-12-06 16:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4747860.QGGHSuFRpz@avalon>
Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
> Hi Kevin,
>
> Thank you for the patch.
>
> On Tuesday 29 Nov 2016 15:57:09 Kevin Hilman wrote:
>> Video capture subdevs may be over I2C and may sleep during xfer, so we
>> cannot do IRQ-disabled locking when calling the subdev.
>>
>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>> ---
>> drivers/media/platform/davinci/vpif_capture.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c
>> b/drivers/media/platform/davinci/vpif_capture.c index
>> 5104cc0ee40e..9f8f41c0f251 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -193,7 +193,10 @@ static int vpif_start_streaming(struct vb2_queue *vq,
>> unsigned int count) }
>> }
>>
>> + spin_unlock_irqrestore(&common->irqlock, flags);
>> ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
>> + spin_lock_irqsave(&common->irqlock, flags);
>
> I always get anxious when I see a spinlock being released randomly with an
> operation in the middle of a protected section. Looking at the code it looks
> like the spinlock is abused here. irqlock should only protect the dma_queue
> and should thus only be taken around the following code:
>
> spin_lock_irqsave(&common->irqlock, flags);
> /* Get the next frame from the buffer queue */
> common->cur_frm = common->next_frm = list_entry(common->dma_queue.next,
> struct vpif_cap_buffer, list);
> /* Remove buffer from the buffer queue */
> list_del(&common->cur_frm->list);
> spin_unlock_irqrestore(&common->irqlock, flags);
Yes, that looks correct. Will update.
> The code that is currently protected by the lock in the start and stop
> streaming functions should be protected by a mutex instead.
I tried taking the mutex here, but lockdep pointed out a deadlock. I
may not be fully understanding the V4L2 internals here, but it seems
that the ioctl is already taking a mutex, so taking it again in
start/stop streaming is a deadlock. Unless you think the locking should
be nested here, it seems to me that the mutex isn't needed.
Kevin
^ permalink raw reply
* [PATCH 1/4] serial: core: Add LED trigger support
From: One Thousand Gnomes @ 2016-12-06 16:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <9225f2da-c6c4-73c8-829a-1033716b4c71@gmail.com>
> > If we have different UART drivers, only one of them provides ttyS*, no?
> > Other drivers will have to use another namespace.
>
> I remember this was a problem a couple of years ago last I tried, with
> the 8250 driver being actually preventing other drivers from using
> ttyS*, but if you don't have 8250 taking over the low ttyS numbers, you
> may run into a case where several drivers successfully register their
> character devices under a ttyS* numbering scheme.
>
> Whether this is hypothetical or not nowadays, it may be nicer to provide
> a more uniquely distinct name like what dev_name() returns, or
> ttyS*-driver-tx for instance?
It needs to be at the tty layer anyway or you will break low end
machines. On a low end system with a 16450A UART you've got something
like 160 microseconds at 57600 baud before you must have exited the
IRQ handler, re-entered it and be reading the data register again. The
current proposed patches touching the 8250 IRQ path are almost certainly
going to cause regressions on old machines.
So you need to hook receive at a point outside of the IRQ layer (eg when
the workqueue starts feeding it into the ldisc), but on the tx side you
need to hook the write method of the tty really, because tty_write sends
data to the ldisc and what happens then is up to the ldisc.
Even there though you have cases that are terribly latency dependant like
some of the dumb programming tools that don't window. If you halve
someones programming rate for microcontrollers you are going to get hate
mail 8) For RX that can mostly be avoided by queuing the data then doing
the LEDs, for TX I'm not clear you can easily hide the latency.
Alan
^ permalink raw reply
* [PATCHv4 05/10] arm64: Use __pa_symbol for kernel symbols
From: Mark Rutland @ 2016-12-06 17:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480445729-27130-6-git-send-email-labbott@redhat.com>
Hi,
As a heads-up, it looks like this got mangled somewhere. In the hunk at
arch/arm64/mm/kasan_init.c:68, 'do' in the context became 'edo'.
Deleting the 'e' makes it apply.
I think this is almost there; other than James's hibernate bug I only
see one real issue, and everything else is a minor nit.
On Tue, Nov 29, 2016 at 10:55:24AM -0800, Laura Abbott wrote:
> __pa_symbol is technically the marco that should be used for kernel
> symbols. Switch to this as a pre-requisite for DEBUG_VIRTUAL which
> will do bounds checking. As part of this, introduce lm_alias, a
> macro which wraps the __va(__pa(...)) idiom used a few places to
> get the alias.
I think the addition of the lm_alias() macro under include/mm should be
a separate preparatory patch. That way it's separate from the
arm64-specific parts, and more obvious to !arm64 people reviewing the
other parts.
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> v4: Stop calling __va early, conversion of a few more sites. I decided against
> wrapping the __p*d_populate calls into new functions since the call sites
> should be limited.
> ---
> arch/arm64/include/asm/kvm_mmu.h | 4 ++--
> arch/arm64/include/asm/memory.h | 2 ++
> arch/arm64/include/asm/mmu_context.h | 6 +++---
> arch/arm64/include/asm/pgtable.h | 2 +-
> arch/arm64/kernel/acpi_parking_protocol.c | 2 +-
> arch/arm64/kernel/cpu-reset.h | 2 +-
> arch/arm64/kernel/cpufeature.c | 2 +-
> arch/arm64/kernel/hibernate.c | 13 +++++--------
> arch/arm64/kernel/insn.c | 2 +-
> arch/arm64/kernel/psci.c | 2 +-
> arch/arm64/kernel/setup.c | 8 ++++----
> arch/arm64/kernel/smp_spin_table.c | 2 +-
> arch/arm64/kernel/vdso.c | 4 ++--
> arch/arm64/mm/init.c | 11 ++++++-----
> arch/arm64/mm/kasan_init.c | 21 +++++++++++++-------
> arch/arm64/mm/mmu.c | 32 +++++++++++++++++++------------
> drivers/firmware/psci.c | 2 +-
> include/linux/mm.h | 4 ++++
> 18 files changed, 70 insertions(+), 51 deletions(-)
It looks like we need to make sure these (directly) include <linux/mm.h>
for __pa_symbol() and lm_alias(), or there's some fragility, e.g.
[mark at leverpostej:~/src/linux]% uselinaro 15.08 make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j10 -s
arch/arm64/kernel/psci.c: In function 'cpu_psci_cpu_boot':
arch/arm64/kernel/psci.c:48:50: error: implicit declaration of function '__pa_symbol' [-Werror=implicit-function-declaration]
int err = psci_ops.cpu_on(cpu_logical_map(cpu), __pa_symbol(secondary_entry));
^
cc1: some warnings being treated as errors
make[1]: *** [arch/arm64/kernel/psci.o] Error 1
make: *** [arch/arm64/kernel] Error 2
make: *** Waiting for unfinished jobs....
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -205,6 +205,8 @@ static inline void *phys_to_virt(phys_addr_t x)
> #define __va(x) ((void *)__phys_to_virt((phys_addr_t)(x)))
> #define pfn_to_kaddr(pfn) __va((pfn) << PAGE_SHIFT)
> #define virt_to_pfn(x) __phys_to_pfn(__virt_to_phys((unsigned long)(x)))
> +#define sym_to_pfn(x) __phys_to_pfn(__pa_symbol(x))
> +#define lm_alias(x) __va(__pa_symbol(x))
As Catalin mentioned, we should be able to drop this copy of lm_alias(),
given we have the same in the core headers.
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index a2c2478..79cd86b 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -140,11 +140,11 @@ static int __init vdso_init(void)
> return -ENOMEM;
>
> /* Grab the vDSO data page. */
> - vdso_pagelist[0] = pfn_to_page(PHYS_PFN(__pa(vdso_data)));
> + vdso_pagelist[0] = phys_to_page(__pa_symbol(vdso_data));
>
> /* 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_symbol(&vdso_start)) + i);
I see you added sym_to_pfn(), which we can use here to keep this short
and legible. It might also be worth using a temporary pfn_t, e.g.
pfn = sym_to_pfn(&vdso_start);
for (i = 0; i < vdso_pages; i++)
vdso_pagelist[i + 1] = pfn_to_page(pfn + i);
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index 8263429..9defbe2 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -383,7 +383,7 @@ static int psci_suspend_finisher(unsigned long index)
> u32 *state = __this_cpu_read(psci_power_state);
>
> return psci_ops.cpu_suspend(state[index - 1],
> - virt_to_phys(cpu_resume));
> + __pa_symbol(cpu_resume));
> }
>
> int psci_cpu_suspend_enter(unsigned long index)
This should probably be its own patch since it's not under arch/arm64/.
I'm happy for this to go via the arm64 tree with the rest regardless
(assuming Lorenzo has no objections).
Thanks,
Mark.
^ permalink raw reply
* [RFC PATCH] arm64: fpsimd: improve stacking logic in non-interruptible context
From: Dave Martin @ 2016-12-06 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu8dAumcEV8Wk+8rXO3VnN0e9rAHac=3-FMhra=tZgeVxg@mail.gmail.com>
On Tue, Dec 06, 2016 at 04:48:54PM +0000, Ard Biesheuvel wrote:
> On 6 December 2016 at 16:43, Dave Martin <Dave.Martin@arm.com> wrote:
> > On Tue, Dec 06, 2016 at 03:39:00PM +0000, Ard Biesheuvel wrote:
> >> Currently, we allow kernel mode NEON in softirq or hardirq context by
> >> stacking and unstacking a slice of the NEON register file for each call
> >> to kernel_neon_begin() and kernel_neon_end(), respectively.
> >>
> >> Given that
> >> a) a CPU typically spends most of its time in userland, during which time
> >> no kernel mode NEON in process context is in progress,
> >> b) a CPU spends most of its time in the kernel doing other things than
> >> kernel mode NEON when it gets interrupted to perform kernel mode NEON
> >> in softirq context
> >>
> >> the stacking and subsequent unstacking is only necessary if we are
> >> interrupting a thread while it is performing kernel mode NEON in process
> >> context, which means that in all other cases, we can simply preserve the
> >> userland FPSIMD state once, and only restore it upon return to userland,
> >> even if we are being invoked from softirq or hardirq context.
> >>
> >> So instead of checking whether we are running in interrupt context, keep
> >> track of the level of nested kernel mode NEON calls in progress, and only
> >> perform the eager stack/unstack of the level exceeds 1.
[...]
> >> + this_cpu_write(kernel_neon_nesting_level, level + 1);
> >
> > Should we BUG_ON overflow/underflow of the nesting level? That Should
> > Not Happen (tm), but we'll make a mess if it does.
> >
>
> True.
>
> > For the underflow case, perhaps DEBUG_PREEMPT is adequate for detecting
> > this via preempt count underflow.
> >
>
> I think it makes sense for the increment to check for overflow and the
> decrement to check for underflow, regardless of whether DEBUG_PREEMPT
> (or just PREEMPT) is enabled or not.
Fair enough
Cheers
---Dave
[...]
^ permalink raw reply
* How do I define SRAM chip in GPMC device tree ?
From: Tony Lindgren @ 2016-12-06 17:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3f98534e-2176-e03e-ac53-b60b8d0ead7c@newflow.co.uk>
Hi,
* Mark Jackson <mpfj-list@newflow.co.uk> [161206 04:33]:
> Hi
>
> We have an existing board supported in Linux v4.1 (see am335x-nano.dts)
> and I was looking to update to a more recent kernel but have hit a stumbling
> block.
>
> Our unit has an SRAM device connected to CS1 and we currently rely on uboot
> to init the GPMC settings.
>
> From v4.2+ I now see that the GPMC code has been changed to reset all the
> GPMC settings when the kernel starts, so our SRAM is now inaccessible.
Yes we've changed to use device tree to describe what's on the GPMC and
refuse to probe it unless configured properly. This is because of the
problems trying to use unknown bootloader configuration, such as kernel
hangs if GPMC was accessed in some cases.
If you enable CONFIG_OMAP_GPMC_DEBUG you should see the bootloader
configured timings in mostly suitable format for adding to dts.
> I've tried the following to add an extra device to the GPMC, but no go:-
>
> sram at 1,0 {
> reg = <1 0x00000000 0x01000000>;
> /*compatible = "mmio-sram";*/
> bank-width = <2>;
>
> gpmc,mux-add-data = <2>;
>
> gpmc,sync-clk-ps = <0>;
> gpmc,cs-on-ns = <0>;
> gpmc,cs-rd-off-ns = <160>;
> gpmc,cs-wr-off-ns = <160>;
> gpmc,adv-on-ns = <10>;
> gpmc,adv-rd-off-ns = <30>;
> gpmc,adv-wr-off-ns = <30>;
> gpmc,oe-on-ns = <40>;
> gpmc,oe-off-ns = <160>;
> gpmc,we-on-ns = <40>;
> gpmc,we-off-ns = <160>;
> gpmc,rd-cycle-ns = <160>;
> gpmc,wr-cycle-ns = <160>;
> gpmc,access-ns = <150>;
> gpmc,page-burst-access-ns = <10>;
> gpmc,cycle2cycle-samecsen;
> gpmc,cycle2cycle-delay-ns = <20>;
> gpmc,wr-data-mux-bus-ns = <70>;
> gpmc,wr-access-ns = <80>;
> };
>
> Looking at drivers/memory/omap-gpmc.c, I can see that when the GPMC
> children are scanned, only the following are matched:-
>
> "nand","onenand","ethernet","nor","uart"
Those are to set up things for probing the device driver for the
connected device. Some have both sync and async accaess to configure,
so at least so far we have not been able to come up with a generic
init.
> So can anyone explain how to add a standard SRAM chip ?
Assuming you've verified that the dts configuration produces working
timings for you, pobably gpmc_probe_generic_child() is the way to
go for adding "sram" option. Note that we have similar interfaces
already for onenand and ethernet for their buffers so you may want
to take a look what the init does for those.
But yeah basically a little bit of new code is needed for that.
Regards,
Tony
^ permalink raw reply
* [PATCH 0/7] arm: Add livepatch support
From: Abel Vesa @ 2016-12-06 17:06 UTC (permalink / raw)
To: linux-arm-kernel
This is just an idea I've been trying out for a while now.
Just in case somebody wants to play with it, this applies to linux-arm/for-next.
Also please note that this was only tested in qemu, but I will do some testing
on some real hardware in the following days.
FWICT, on this arch the compiler always generates a function prologue somewhere
between these lines:
e1a0c00d mov ip, sp
e92ddff0 push {r4-r9, sl, fp, ip, lr, pc}
e24cb004 sub fp, ip, #4
e24dd064 sub sp, sp, #100 ; 0x64 <--- local variables
e52de004 push {lr} ; (str lr, [sp, #-4]!)
ebf9c2c9 bl 80110364 <__gnu_mcount_nc>
....
Every function that follows this pattern (the number of registers pushed and the
sp subtraction for the local variables being the only acceptable exception) can
be patched with this mechanism. IIRC, only the inline functions and notrace
functions do not follow this pattern.
Considering that the function is livepatchable, when the time comes to call
ftrace_call, the ftrace_regs_caller is called instead.
Because this arch didn't have a ftrace with regs implementation, the
ftrace_regs_caller was added.
This new function adds the regs saving/restoring part, plus the part necessary
for the livepatch mechanism to work. After the regs are saved and the r3 is set
to contain the sp's value, we're keeping the old pc into r10 in order to be
checked later against the new pc.
Next, the r1 and r0 are set for the ftrace_func, then, the ftrace_stub is called
and the klp_ftrace_handler overwrites the old pc with the new one.
Here comes the tricky part. We're checking if the pc is still the old one, if it
is we jump the whole livepatching and go ahead with restoring the saved regs.
If the pc is modified, it means we're livepatching current function and we need
to pop all regs from r1 through r12, jump over the next two regs saved on stack
(we're not interested in those since we're trying to get the same regs context
as it was at the point the function-to-be-patched was called) and put the new pc
into r11.
Since r12 contains the sp from when the function just got branched to, we need
to set the sp back to that.
Then we need to put the new pc on stack so that when we're popping r11 through
pc, we will actually jump to the first instruction from the new function.
We don't need to worry about the returning phase since the epilogue of the new
function will take care of that and from there on everything goes back to
normal.
The whole advantage of this over adding compiler support is that we're not
introducing nops at the beginning of the function. As a matter of fact, we're
not changing anything between an image with livepatch and an image without it
(except the ftrace_regs_call addition and the livepatch necessary code).
As for the implementation of the ftrace_regs_caller, I still think there might
be some unsafe stack handling since I'm getting some build warnings. Those are
due to pushing/popping of a list of regs in which the sp resides. I'll try to
get around those in a next iteration (if necessary), but first I would like to
hear some opinions about this work and if it's worth going forward.
Everything else should be pretty straightforward, so I'll skip explaining that.
Abel Vesa (7):
arm: Add livepatch arch specific code
arm: ftrace: Add call modify mechanism
arm: module: Add apply_relocate_add
arm: Add ftrace with regs support
arm: ftrace: Add ARCH_SUPPORTS_FTRACE_OPS for ftrace with regs
arm: Add livepatch to build if CONFIG_LIVEPATCH
arm: Add livepatch necessary arch selects into Kconfig
MAINTAINERS | 3 +++
arch/arm/Kconfig | 4 ++++
arch/arm/include/asm/ftrace.h | 4 ++++
arch/arm/include/asm/livepatch.h | 46 +++++++++++++++++++++++++++++++++++++
arch/arm/kernel/Makefile | 1 +
arch/arm/kernel/entry-ftrace.S | 49 ++++++++++++++++++++++++++++++++++++++++
arch/arm/kernel/ftrace.c | 21 +++++++++++++++++
arch/arm/kernel/livepatch.c | 43 +++++++++++++++++++++++++++++++++++
arch/arm/kernel/module.c | 9 ++++++++
9 files changed, 180 insertions(+)
create mode 100644 arch/arm/include/asm/livepatch.h
create mode 100644 arch/arm/kernel/livepatch.c
--
2.7.4
^ permalink raw reply
* [PATCH 1/7] arm: Add livepatch arch specific code
From: Abel Vesa @ 2016-12-06 17:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481043967-15602-1-git-send-email-abelvesa@linux.com>
klp_get_ftrace_location is used by ftrace to get the entry for a
specific function from the mcount list. klp_arch_set_pc is used
to set the pc from the regs passed as an argument to the
ftrace_ops_no_ops function to the starting address of the patched
function. klp_write_module_reloc is not doing anything at this
moment.
Signed-off-by: Abel Vesa <abelvesa@linux.com>
---
MAINTAINERS | 3 +++
arch/arm/include/asm/livepatch.h | 46 ++++++++++++++++++++++++++++++++++++++++
arch/arm/kernel/livepatch.c | 43 +++++++++++++++++++++++++++++++++++++
3 files changed, 92 insertions(+)
create mode 100644 arch/arm/include/asm/livepatch.h
create mode 100644 arch/arm/kernel/livepatch.c
diff --git a/MAINTAINERS b/MAINTAINERS
index bd182a1..d43b790 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7466,12 +7466,15 @@ M: Josh Poimboeuf <jpoimboe@redhat.com>
M: Jessica Yu <jeyu@redhat.com>
M: Jiri Kosina <jikos@kernel.org>
M: Miroslav Benes <mbenes@suse.cz>
+M: Abel Vesa <abelvesa@linux.com>
R: Petr Mladek <pmladek@suse.com>
S: Maintained
F: kernel/livepatch/
F: include/linux/livepatch.h
F: arch/x86/include/asm/livepatch.h
F: arch/x86/kernel/livepatch.c
+F: arch/arm/include/asm/livepatch.h
+F: arch/arm/kernel/livepatch.c
F: Documentation/livepatch/
F: Documentation/ABI/testing/sysfs-kernel-livepatch
F: samples/livepatch/
diff --git a/arch/arm/include/asm/livepatch.h b/arch/arm/include/asm/livepatch.h
new file mode 100644
index 0000000..d4e3ff0
--- /dev/null
+++ b/arch/arm/include/asm/livepatch.h
@@ -0,0 +1,46 @@
+/*
+ * livepatch.h - arm specific Kernel Live Patching Core
+ *
+ * Copyright (C) 2016 Abel Vesa <abelvesa@linux.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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/>.
+ */
+
+#ifndef _ASM_ARM_LIVEPATCH_H
+#define _ASM_ARM_LIVEPATCH_H
+
+#include <asm/setup.h>
+#include <linux/module.h>
+#include <linux/ftrace.h>
+
+static inline int klp_check_compiler_support(void)
+{
+ return 0;
+}
+
+int klp_write_module_reloc(struct module *mod, unsigned long type,
+ unsigned long loc, unsigned long value);
+
+static inline void klp_arch_set_pc(struct pt_regs *regs, unsigned long ip)
+{
+ regs->uregs[15] = ip;
+}
+
+#define klp_get_ftrace_location klp_get_ftrace_location
+static inline unsigned long klp_get_ftrace_location(unsigned long faddr)
+{
+ return ftrace_location_range(faddr, faddr + 24);
+}
+
+#endif /* _ASM_ARM_LIVEPATCH_H */
diff --git a/arch/arm/kernel/livepatch.c b/arch/arm/kernel/livepatch.c
new file mode 100644
index 0000000..0656cd6
--- /dev/null
+++ b/arch/arm/kernel/livepatch.c
@@ -0,0 +1,43 @@
+/*
+ * livepatch.c - arm specific Kernel Live Patching Core
+ *
+ * Copyright (C) 2016 Abel Vesa <abelvesa@linux.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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/>.
+ */
+
+#include <linux/module.h>
+#include <linux/uaccess.h>
+#include <linux/ftrace.h>
+#include <asm/elf.h>
+#include <asm/livepatch.h>
+#include <asm/insn.h>
+#include <asm/ftrace.h>
+
+/**
+ * klp_write_module_reloc() - write a relocation in a module
+ * @mod: module in which the section to be modified is found
+ * @type: ELF relocation type (see asm/elf.h)
+ * @loc: address that the relocation should be written to
+ * @value: relocation value (sym address + addend)
+ *
+ * This function writes a relocation to the specified location for
+ * a particular module.
+ */
+int klp_write_module_reloc(struct module *mod, unsigned long type,
+ unsigned long loc, unsigned long value)
+{
+ /* Not implemented yet */
+ return 0;
+}
--
2.7.4
^ permalink raw reply related
* [PATCH 2/7] arm: ftrace: Add call modify mechanism
From: Abel Vesa @ 2016-12-06 17:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481043967-15602-1-git-send-email-abelvesa@linux.com>
Function ftrace_modify_call provides a way to replace ftrace_stub
with the ftrace function. This helps the klp_ftrace_handler to be
called via ftrace_ops_no_ops, which in turn will set the pc with
the patched function's starting address. This is used for
livepatching.
Signed-off-by: Abel Vesa <abelvesa@linux.com>
---
arch/arm/kernel/ftrace.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index 3f17594..cb4543a 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -139,6 +139,15 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
ret = ftrace_modify_code(pc, 0, new, false);
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+ if (!ret) {
+ pc = (unsigned long)&ftrace_regs_call;
+ new = ftrace_call_replace(pc, (unsigned long)func);
+
+ ret = ftrace_modify_code(pc, 0, new, false);
+ }
+#endif
+
#ifdef CONFIG_OLD_MCOUNT
if (!ret) {
pc = (unsigned long)&ftrace_call_old;
@@ -151,6 +160,18 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
return ret;
}
+int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
+ unsigned long addr)
+{
+ unsigned long pc = rec->ip;
+ u32 old, new;
+
+ old = arm_gen_branch(pc, old_addr);
+ new = arm_gen_branch(pc, addr);
+
+ return ftrace_modify_code(pc, old, new, true);
+}
+
int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
{
unsigned long new, old;
--
2.7.4
^ permalink raw reply related
* [PATCH 3/7] arm: module: Add apply_relocate_add
From: Abel Vesa @ 2016-12-06 17:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481043967-15602-1-git-send-email-abelvesa@linux.com>
It was only added to fix compiler error. It is not implemented
yet.
Signed-off-by: Abel Vesa <abelvesa@linux.com>
---
arch/arm/kernel/module.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index 4f14b5c..bf94922 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -52,6 +52,15 @@ void *module_alloc(unsigned long size)
#endif
int
+apply_relocate_add(Elf32_Shdr *sechdrs, const char *strtab,
+ unsigned int symindex, unsigned int relindex,
+ struct module *module)
+{
+ /* Not implemented yet */
+ return 0;
+}
+
+int
apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex,
unsigned int relindex, struct module *module)
{
--
2.7.4
^ permalink raw reply related
* [PATCH 4/7] arm: Add ftrace with regs support
From: Abel Vesa @ 2016-12-06 17:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481043967-15602-1-git-send-email-abelvesa@linux.com>
This adds __ftrace_regs_caller which, unlike __ftrace_caller,
adds register saving/restoring and livepatch handling if
the pc register gets modified by klp_ftrace_handler.
Signed-off-by: Abel Vesa <abelvesa@linux.com>
---
arch/arm/kernel/entry-ftrace.S | 49 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
index c73c403..b6ada5c 100644
--- a/arch/arm/kernel/entry-ftrace.S
+++ b/arch/arm/kernel/entry-ftrace.S
@@ -92,6 +92,46 @@
2: mcount_exit
.endm
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+
+.macro __ftrace_regs_caller suffix
+
+ stmdb sp!, {r0-r15}
+ mov r3, sp
+
+ ldr r10, [sp, #60]
+
+ mcount_get_lr r1 @ lr of instrumented func
+ mcount_adjust_addr r0, lr @ instrumented function
+
+ .globl ftrace_regs_call\suffix
+ftrace_regs_call\suffix:
+ bl ftrace_stub
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ .globl ftrace_regs_graph_call\suffix
+ftrace_regs_graph_call\suffix:
+ mov r0, r0
+#endif
+#ifdef CONFIG_LIVEPATCH
+ ldr r0, [sp, #60]
+ cmp r0, r10
+ beq ftrace_regs_caller_end
+ ldmia sp!, {r0-r12}
+ add sp, sp, #8
+ ldmia sp!, {r11}
+ sub sp, r12, #16
+ str r11, [sp, #12]
+ ldmia sp!, {r11, r12, lr, pc}
+#endif
+ftrace_regs_caller_end\suffix:
+ ldmia sp!, {r0-r14}
+ add sp, sp, #4
+ mov pc, lr
+.endm
+
+#endif
+
.macro __ftrace_caller suffix
mcount_enter
@@ -212,6 +252,15 @@ UNWIND(.fnstart)
__ftrace_caller
UNWIND(.fnend)
ENDPROC(ftrace_caller)
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+ENTRY(ftrace_regs_caller)
+UNWIND(.fnstart)
+ __ftrace_regs_caller
+UNWIND(.fnend)
+ENDPROC(ftrace_regs_caller)
+#endif
+
#endif
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
--
2.7.4
^ permalink raw reply related
* [PATCH 5/7] arm: ftrace: Add ARCH_SUPPORTS_FTRACE_OPS for ftrace with regs
From: Abel Vesa @ 2016-12-06 17:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481043967-15602-1-git-send-email-abelvesa@linux.com>
ARCH_SUPPORTS_FTRACE_OPS is needed for livepatch if
CONFIG_DYNAMIC_FTRACE_WITH_REGS is defined.
Signed-off-by: Abel Vesa <abelvesa@linux.com>
---
arch/arm/include/asm/ftrace.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
index bfe2a2f..f434ce9 100644
--- a/arch/arm/include/asm/ftrace.h
+++ b/arch/arm/include/asm/ftrace.h
@@ -1,6 +1,10 @@
#ifndef _ASM_ARM_FTRACE
#define _ASM_ARM_FTRACE
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#define ARCH_SUPPORTS_FTRACE_OPS 1
+#endif
+
#ifdef CONFIG_FUNCTION_TRACER
#define MCOUNT_ADDR ((unsigned long)(__gnu_mcount_nc))
#define MCOUNT_INSN_SIZE 4 /* sizeof mcount call */
--
2.7.4
^ permalink raw reply related
* [PATCH 6/7] arm: Add livepatch to build if CONFIG_LIVEPATCH
From: Abel Vesa @ 2016-12-06 17:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481043967-15602-1-git-send-email-abelvesa@linux.com>
Necessary livepatch file added to makefile.
Signed-off-by: Abel Vesa <abelvesa@linux.com>
---
arch/arm/kernel/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index ad325a8..9e70220 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_HAVE_ARM_TWD) += smp_twd.o
obj-$(CONFIG_ARM_ARCH_TIMER) += arch_timer.o
obj-$(CONFIG_FUNCTION_TRACER) += entry-ftrace.o
obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o insn.o
+obj-$(CONFIG_LIVEPATCH) += livepatch.o
obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o insn.o
obj-$(CONFIG_JUMP_LABEL) += jump_label.o insn.o patch.o
obj-$(CONFIG_KEXEC) += machine_kexec.o relocate_kernel.o
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox