[PATCH v14 23/26] ELF: Introduce arch_setup_elf_property()
An ELF file's .note.gnu.property indicates arch features supported by the file. These features are extracted by arch_parse_elf_property() and stored in 'arch_elf_state'. Introduce arch_setup_elf_property() for enabling such features. The first use-case of this function is shadow stack. ARM64 is the other arch that has ARCH_USER_GNU_PROPERTY and arch_parse_elf_ property(). Add arch_setup_elf_property() for it. Signed-off-by: Yu-cheng Yu Cc: Mark Brown Cc: Catalin Marinas Cc: Dave Martin --- arch/arm64/include/asm/elf.h | 5 + arch/x86/Kconfig | 2 ++ arch/x86/include/asm/elf.h | 13 + arch/x86/kernel/process_64.c | 32 fs/binfmt_elf.c | 4 include/linux/elf.h | 6 ++ 6 files changed, 62 insertions(+) diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h index 8d1c8dcb87fd..d37bc7915935 100644 --- a/arch/arm64/include/asm/elf.h +++ b/arch/arm64/include/asm/elf.h @@ -281,6 +281,11 @@ static inline int arch_parse_elf_property(u32 type, const void *data, return 0; } +static inline int arch_setup_elf_property(struct arch_elf_state *arch) +{ + return 0; +} + static inline int arch_elf_pt_proc(void *ehdr, void *phdr, struct file *f, bool is_interp, struct arch_elf_state *state) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 7578327226e3..4b28a0ce4594 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1950,6 +1950,8 @@ config X86_SHADOW_STACK_USER select X86_CET select ARCH_MAYBE_MKWRITE select ARCH_HAS_SHADOW_STACK + select ARCH_USE_GNU_PROPERTY + select ARCH_BINFMT_ELF_STATE help Shadow Stacks provides protection against program stack corruption. It's a hardware feature. This only matters diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h index b9a5d488f1a5..0e1be2a13359 100644 --- a/arch/x86/include/asm/elf.h +++ b/arch/x86/include/asm/elf.h @@ -385,6 +385,19 @@ extern int compat_arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp); #define compat_arch_setup_additional_pages compat_arch_setup_additional_pages +#ifdef CONFIG_ARCH_BINFMT_ELF_STATE +struct arch_elf_state { + unsigned int gnu_property; +}; + +#define INIT_ARCH_ELF_STATE { \ + .gnu_property = 0, \ +} + +#define arch_elf_pt_proc(ehdr, phdr, elf, interp, state) (0) +#define arch_check_elf(ehdr, interp, interp_ehdr, state) (0) +#endif + /* Do not change the values. See get_align_mask() */ enum align_flags { ALIGN_VA_32 = BIT(0), diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index 9afefe325acb..8725e67bcd44 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -837,3 +837,35 @@ unsigned long KSTK_ESP(struct task_struct *task) { return task_pt_regs(task)->sp; } + +#ifdef CONFIG_ARCH_USE_GNU_PROPERTY +int arch_parse_elf_property(u32 type, const void *data, size_t datasz, +bool compat, struct arch_elf_state *state) +{ + if (type != GNU_PROPERTY_X86_FEATURE_1_AND) + return 0; + + if (datasz != sizeof(unsigned int)) + return -ENOEXEC; + + state->gnu_property = *(unsigned int *)data; + return 0; +} + +int arch_setup_elf_property(struct arch_elf_state *state) +{ + int r = 0; + + if (!IS_ENABLED(CONFIG_X86_CET)) + return r; + + memset(>thread.cet, 0, sizeof(struct cet_status)); + + if (static_cpu_has(X86_FEATURE_SHSTK)) { + if (state->gnu_property & GNU_PROPERTY_X86_FEATURE_1_SHSTK) + r = cet_setup_shstk(); + } + + return r; +} +#endif diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 13d053982dd7..2b4cfc256895 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -1217,6 +1217,10 @@ static int load_elf_binary(struct linux_binprm *bprm) set_binfmt(_format); + retval = arch_setup_elf_property(_state); + if (retval < 0) + goto out; + #ifdef ARCH_HAS_SETUP_ADDITIONAL_PAGES retval = arch_setup_additional_pages(bprm, !!interpreter); if (retval < 0) diff --git a/include/linux/elf.h b/include/linux/elf.h index 5d5b0321da0b..4827695ca415 100644 --- a/include/linux/elf.h +++ b/include/linux/elf.h @@ -82,9 +82,15 @@ static inline int arch_parse_elf_property(u32 type, const void *data, { return 0; } + +static inline int arch_setup_elf_property(struct arch_elf_state *arch) +{ + return 0; +} #else extern int arch_parse_elf_property(u32 type, const void *data, size_t datasz, bool compat, struct arch_elf_state *arch); +extern int arch_setup_elf_property(struct arch_elf_state *arch); #endif #ifdef CONFIG_ARCH_HAVE_ELF_PROT --
[PATCH v14 10/26] x86/mm: Update pte_modify for _PAGE_COW
Pte_modify() changes a PTE to 'newprot'. It doesn't use the pte_*() helpers that a previous patch fixed up, so we need a new site. Introduce fixup_dirty_pte() to set the dirty bits based on _PAGE_RW, and apply the same changes to pmd_modify(). Signed-off-by: Yu-cheng Yu --- arch/x86/include/asm/pgtable.h | 33 + 1 file changed, 33 insertions(+) diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h index ac4ed814be96..8d4c09831e67 100644 --- a/arch/x86/include/asm/pgtable.h +++ b/arch/x86/include/asm/pgtable.h @@ -727,6 +727,21 @@ static inline pmd_t pmd_mkinvalid(pmd_t pmd) static inline u64 flip_protnone_guard(u64 oldval, u64 val, u64 mask); +static inline pteval_t fixup_dirty_pte(pteval_t pteval) +{ + pte_t pte = __pte(pteval); + + if (cpu_feature_enabled(X86_FEATURE_SHSTK) && pte_dirty(pte)) { + pte = pte_mkclean(pte); + + if (pte_flags(pte) & _PAGE_RW) + pte = pte_set_flags(pte, _PAGE_DIRTY_HW); + else + pte = pte_set_flags(pte, _PAGE_COW); + } + return pte_val(pte); +} + static inline pte_t pte_modify(pte_t pte, pgprot_t newprot) { pteval_t val = pte_val(pte), oldval = val; @@ -737,16 +752,34 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t newprot) */ val &= _PAGE_CHG_MASK; val |= check_pgprot(newprot) & ~_PAGE_CHG_MASK; + val = fixup_dirty_pte(val); val = flip_protnone_guard(oldval, val, PTE_PFN_MASK); return __pte(val); } +static inline int pmd_write(pmd_t pmd); +static inline pmdval_t fixup_dirty_pmd(pmdval_t pmdval) +{ + pmd_t pmd = __pmd(pmdval); + + if (cpu_feature_enabled(X86_FEATURE_SHSTK) && pmd_dirty(pmd)) { + pmd = pmd_mkclean(pmd); + + if (pmd_flags(pmd) & _PAGE_RW) + pmd = pmd_set_flags(pmd, _PAGE_DIRTY_HW); + else + pmd = pmd_set_flags(pmd, _PAGE_COW); + } + return pmd_val(pmd); +} + static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot) { pmdval_t val = pmd_val(pmd), oldval = val; val &= _HPAGE_CHG_MASK; val |= check_pgprot(newprot) & ~_HPAGE_CHG_MASK; + val = fixup_dirty_pmd(val); val = flip_protnone_guard(oldval, val, PHYSICAL_PMD_PAGE_MASK); return __pmd(val); } -- 2.21.0
Re: [tip: objtool/core] x86/insn: Support big endian cross-compiles
On Mon, Oct 12, 2020 at 09:12:36AM +0900, Masami Hiramatsu wrote: > On Sat, 10 Oct 2020 12:44:15 -0500 > Josh Poimboeuf wrote: > > > On Fri, Oct 09, 2020 at 10:49:21PM +0200, Borislav Petkov wrote: > > > On Fri, Oct 09, 2020 at 10:38:22PM +0200, Peter Zijlstra wrote: > > > > On Wed, Oct 07, 2020 at 04:20:19PM -, tip-bot2 for Martin > > > > Schwidefsky wrote: > > > > > The following commit has been merged into the objtool/core branch of > > > > > tip: > > > > > > > > > > Commit-ID: 2a522b53c47051d3bf98748418f4f8e5f20d2c04 > > > > > Gitweb: > > > > > https://git.kernel.org/tip/2a522b53c47051d3bf98748418f4f8e5f20d2c04 > > > > > Author:Martin Schwidefsky > > > > > AuthorDate:Mon, 05 Oct 2020 17:50:31 +02:00 > > > > > Committer: Josh Poimboeuf > > > > > CommitterDate: Tue, 06 Oct 2020 09:32:29 -05:00 > > > > > > > > > > x86/insn: Support big endian cross-compiles > > > > > > > > > > x86 instruction decoder code is shared across the kernel source and > > > > > the > > > > > tools. Currently objtool seems to be the only tool from build tools > > > > > needed > > > > > which breaks x86 cross compilation on big endian systems. Make the x86 > > > > > instruction decoder build host endianness agnostic to support x86 > > > > > cross > > > > > compilation and enable objtool to implement endianness awareness for > > > > > big endian architectures support. > > > > > > > > > > Signed-off-by: Martin Schwidefsky > > > > > Co-developed-by: Vasily Gorbik > > > > > Signed-off-by: Vasily Gorbik > > > > > Acked-by: Masami Hiramatsu > > > > > Signed-off-by: Josh Poimboeuf > > > > > > > > This commit breaks the x86 build with CONFIG_X86_DECODER_SELFTEST=y. > > > > > > > > I've asked Boris to truncate tip/objtool/core. > > > > > > Yeah, top 4 are gone until this is resolved. > > > > Masami, I wonder if we even need these selftests anymore? Objtool > > already decodes the entire kernel. > > No, they have different roles. The selftest checks if the decoder > works correctly by comparing with the output of objdump. > > As far as I can see, the objtool relies on the sanity of the decoder > (it trusts the output of the decoder). Ok. I wonder if we should move the decoder selftest to the 'tools' subdirectory. -- Josh
[PATCH v14 25/26] x86/cet/shstk: Add arch_prctl functions for shadow stack
arch_prctl(ARCH_X86_CET_STATUS, u64 *args) Get CET feature status. The parameter 'args' is a pointer to a user buffer. The kernel returns the following information: *args = shadow stack/IBT status *(args + 1) = shadow stack base address *(args + 2) = shadow stack size arch_prctl(ARCH_X86_CET_DISABLE, unsigned int features) Disable CET features specified in 'features'. Return -EPERM if CET is locked. arch_prctl(ARCH_X86_CET_LOCK) Lock in CET features. Also change do_arch_prctl_common()'s parameter 'cpuid_enabled' to 'arg2', as it is now also passed to prctl_cet(). Signed-off-by: Yu-cheng Yu --- arch/x86/include/asm/cet.h | 3 ++ arch/x86/include/uapi/asm/prctl.h | 4 ++ arch/x86/kernel/Makefile| 2 +- arch/x86/kernel/cet_prctl.c | 68 + arch/x86/kernel/process.c | 6 +-- tools/arch/x86/include/uapi/asm/prctl.h | 4 ++ 6 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 arch/x86/kernel/cet_prctl.c diff --git a/arch/x86/include/asm/cet.h b/arch/x86/include/asm/cet.h index ec4b5e62d0ce..16870e5bc8eb 100644 --- a/arch/x86/include/asm/cet.h +++ b/arch/x86/include/asm/cet.h @@ -14,9 +14,11 @@ struct sc_ext; struct cet_status { unsigned long shstk_base; unsigned long shstk_size; + unsigned intlocked:1; }; #ifdef CONFIG_X86_CET +int prctl_cet(int option, u64 arg2); int cet_setup_shstk(void); int cet_setup_thread_shstk(struct task_struct *p, unsigned long clone_flags); void cet_disable_shstk(void); @@ -25,6 +27,7 @@ int cet_verify_rstor_token(bool ia32, unsigned long ssp, unsigned long *new_ssp) void cet_restore_signal(struct sc_ext *sc); int cet_setup_signal(bool ia32, unsigned long rstor, struct sc_ext *sc); #else +static inline int prctl_cet(int option, u64 arg2) { return -EINVAL; } static inline int cet_setup_thread_shstk(struct task_struct *p, unsigned long clone_flags) { return 0; } static inline void cet_disable_shstk(void) {} diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h index 5a6aac9fa41f..9245bf629120 100644 --- a/arch/x86/include/uapi/asm/prctl.h +++ b/arch/x86/include/uapi/asm/prctl.h @@ -14,4 +14,8 @@ #define ARCH_MAP_VDSO_32 0x2002 #define ARCH_MAP_VDSO_64 0x2003 +#define ARCH_X86_CET_STATUS0x3001 +#define ARCH_X86_CET_DISABLE 0x3002 +#define ARCH_X86_CET_LOCK 0x3003 + #endif /* _ASM_X86_PRCTL_H */ diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile index 1fb85595afa7..321ef52e4470 100644 --- a/arch/x86/kernel/Makefile +++ b/arch/x86/kernel/Makefile @@ -145,7 +145,7 @@ obj-$(CONFIG_UNWINDER_ORC) += unwind_orc.o obj-$(CONFIG_UNWINDER_FRAME_POINTER) += unwind_frame.o obj-$(CONFIG_UNWINDER_GUESS) += unwind_guess.o -obj-$(CONFIG_X86_CET) += cet.o +obj-$(CONFIG_X86_CET) += cet.o cet_prctl.o ### # 64 bit specific files diff --git a/arch/x86/kernel/cet_prctl.c b/arch/x86/kernel/cet_prctl.c new file mode 100644 index ..bd5ad11763e4 --- /dev/null +++ b/arch/x86/kernel/cet_prctl.c @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* See Documentation/x86/intel_cet.rst. */ + +static int copy_status_to_user(struct cet_status *cet, u64 arg2) +{ + u64 buf[3] = {0, 0, 0}; + + if (cet->shstk_size) { + buf[0] |= GNU_PROPERTY_X86_FEATURE_1_SHSTK; + buf[1] = (u64)cet->shstk_base; + buf[2] = (u64)cet->shstk_size; + } + + return copy_to_user((u64 __user *)arg2, buf, sizeof(buf)); +} + +int prctl_cet(int option, u64 arg2) +{ + struct cet_status *cet; + unsigned int features; + + /* +* GLIBC's ENOTSUPP == EOPNOTSUPP == 95, and it does not recognize +* the kernel's ENOTSUPP (524). So return EOPNOTSUPP here. +*/ + if (!IS_ENABLED(CONFIG_X86_CET)) + return -EOPNOTSUPP; + + cet = >thread.cet; + + if (option == ARCH_X86_CET_STATUS) + return copy_status_to_user(cet, arg2); + + if (!static_cpu_has(X86_FEATURE_SHSTK)) + return -EOPNOTSUPP; + + switch (option) { + case ARCH_X86_CET_DISABLE: + if (cet->locked) + return -EPERM; + + features = (unsigned int)arg2; + + if (features & GNU_PROPERTY_X86_FEATURE_1_INVAL) + return -EINVAL; + if (features & GNU_PROPERTY_X86_FEATURE_1_SHSTK) + cet_disable_shstk(); + return 0; + + case ARCH_X86_CET_LOCK: + cet->locked = 1; + return 0; + + default: + return -ENOSYS; + } +} diff --git
[PATCH v14 19/26] mm: Re-introduce vm_flags to do_mmap()
There was no more caller passing vm_flags to do_mmap(), and vm_flags was removed from the function's input by: commit 45e55300f114 ("mm: remove unnecessary wrapper function do_mmap_pgoff()"). There is a new user now. Shadow stack allocation passes VM_SHSTK to do_mmap(). Re-introduce vm_flags to do_mmap(), but without the old wrapper do_mmap_pgoff(). Instead, make all callers of the wrapper pass a zero vm_flags to do_mmap(). Signed-off-by: Yu-cheng Yu Reviewed-by: Peter Collingbourne Cc: Andrew Morton Cc: Oleg Nesterov Cc: linux...@kvack.org --- fs/aio.c | 2 +- include/linux/mm.h | 3 ++- ipc/shm.c | 2 +- mm/mmap.c | 10 +- mm/nommu.c | 4 ++-- mm/util.c | 2 +- 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/fs/aio.c b/fs/aio.c index d5ec30385566..ca8c11665eea 100644 --- a/fs/aio.c +++ b/fs/aio.c @@ -527,7 +527,7 @@ static int aio_setup_ring(struct kioctx *ctx, unsigned int nr_events) ctx->mmap_base = do_mmap(ctx->aio_ring_file, 0, ctx->mmap_size, PROT_READ | PROT_WRITE, -MAP_SHARED, 0, , NULL); +MAP_SHARED, 0, 0, , NULL); mmap_write_unlock(mm); if (IS_ERR((void *)ctx->mmap_base)) { ctx->mmap_size = 0; diff --git a/include/linux/mm.h b/include/linux/mm.h index ce461795fd8b..71677d498300 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2560,7 +2560,8 @@ extern unsigned long mmap_region(struct file *file, unsigned long addr, struct list_head *uf); extern unsigned long do_mmap(struct file *file, unsigned long addr, unsigned long len, unsigned long prot, unsigned long flags, - unsigned long pgoff, unsigned long *populate, struct list_head *uf); + vm_flags_t vm_flags, unsigned long pgoff, unsigned long *populate, + struct list_head *uf); extern int __do_munmap(struct mm_struct *, unsigned long, size_t, struct list_head *uf, bool downgrade); extern int do_munmap(struct mm_struct *, unsigned long, size_t, diff --git a/ipc/shm.c b/ipc/shm.c index e25c7c6106bc..91474258933d 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -1556,7 +1556,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, goto invalid; } - addr = do_mmap(file, addr, size, prot, flags, 0, , NULL); + addr = do_mmap(file, addr, size, prot, flags, 0, 0, , NULL); *raddr = addr; err = 0; if (IS_ERR_VALUE(addr)) diff --git a/mm/mmap.c b/mm/mmap.c index 574b3f273462..fc04184d2eae 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1365,11 +1365,11 @@ static inline bool file_mmap_ok(struct file *file, struct inode *inode, */ unsigned long do_mmap(struct file *file, unsigned long addr, unsigned long len, unsigned long prot, - unsigned long flags, unsigned long pgoff, - unsigned long *populate, struct list_head *uf) + unsigned long flags, vm_flags_t vm_flags, + unsigned long pgoff, unsigned long *populate, + struct list_head *uf) { struct mm_struct *mm = current->mm; - vm_flags_t vm_flags; int pkey = 0; *populate = 0; @@ -1431,7 +1431,7 @@ unsigned long do_mmap(struct file *file, unsigned long addr, * to. we assume access permissions have been handled by the open * of the memory object, so we don't do any here. */ - vm_flags = calc_vm_prot_bits(prot, pkey) | calc_vm_flag_bits(flags) | + vm_flags |= calc_vm_prot_bits(prot, pkey) | calc_vm_flag_bits(flags) | mm->def_flags | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC; if (flags & MAP_LOCKED) @@ -3007,7 +3007,7 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size, file = get_file(vma->vm_file); ret = do_mmap(vma->vm_file, start, size, - prot, flags, pgoff, , NULL); + prot, flags, 0, pgoff, , NULL); fput(file); out: mmap_write_unlock(mm); diff --git a/mm/nommu.c b/mm/nommu.c index 75a327149af1..f67d6bcdfc9f 100644 --- a/mm/nommu.c +++ b/mm/nommu.c @@ -1078,6 +1078,7 @@ unsigned long do_mmap(struct file *file, unsigned long len, unsigned long prot, unsigned long flags, + vm_flags_t vm_flags, unsigned long pgoff, unsigned long *populate, struct list_head *uf) @@ -1085,7 +1086,6 @@ unsigned long do_mmap(struct file *file, struct vm_area_struct *vma; struct vm_region *region; struct rb_node *rb; - vm_flags_t vm_flags; unsigned long capabilities, result; int ret; @@ -1104,7 +1104,7 @@ unsigned long
[PATCH v14 21/26] x86/cet/shstk: Handle signals for shadow stack
To deliver a signal, create a shadow stack restore token and put a restore token and the signal restorer address on the shadow stack. For sigreturn, verify the token and restore the shadow stack pointer. Introduce WRUSS, which is a kernel-mode instruction but writes directly to user shadow stack. It is used to construct the user signal stack as described above. Introduce a signal context extension struct 'sc_ext', which is used to save shadow stack restore token address and WAIT_ENDBR status. WAIT_ENDBR will be introduced later in the Indirect Branch Tracking (IBT) series, but add that into sc_ext now to keep the struct stable in case the IBT series is applied later. Signed-off-by: Yu-cheng Yu --- arch/x86/ia32/ia32_signal.c| 17 +++ arch/x86/include/asm/cet.h | 8 ++ arch/x86/include/asm/fpu/internal.h| 10 ++ arch/x86/include/asm/special_insns.h | 32 ++ arch/x86/include/uapi/asm/sigcontext.h | 9 ++ arch/x86/kernel/cet.c | 152 + arch/x86/kernel/fpu/signal.c | 100 arch/x86/kernel/signal.c | 10 ++ 8 files changed, 338 insertions(+) diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c index 81cf22398cd1..cec9cf0a00cf 100644 --- a/arch/x86/ia32/ia32_signal.c +++ b/arch/x86/ia32/ia32_signal.c @@ -35,6 +35,7 @@ #include #include #include +#include static inline void reload_segments(struct sigcontext_32 *sc) { @@ -205,6 +206,7 @@ static void __user *get_sigframe(struct ksignal *ksig, struct pt_regs *regs, void __user **fpstate) { unsigned long sp, fx_aligned, math_size; + void __user *restorer = NULL; /* Default to using normal stack */ sp = regs->sp; @@ -218,8 +220,23 @@ static void __user *get_sigframe(struct ksignal *ksig, struct pt_regs *regs, ksig->ka.sa.sa_restorer) sp = (unsigned long) ksig->ka.sa.sa_restorer; + if (ksig->ka.sa.sa_flags & SA_RESTORER) { + restorer = ksig->ka.sa.sa_restorer; + } else if (current->mm->context.vdso) { + if (ksig->ka.sa.sa_flags & SA_SIGINFO) + restorer = current->mm->context.vdso + + vdso_image_32.sym___kernel_rt_sigreturn; + else + restorer = current->mm->context.vdso + + vdso_image_32.sym___kernel_sigreturn; + } + sp = fpu__alloc_mathframe(sp, 1, _aligned, _size); *fpstate = (struct _fpstate_32 __user *) sp; + + if (save_cet_to_sigframe(1, *fpstate, (unsigned long)restorer)) + return (void __user *) -1L; + if (copy_fpstate_to_sigframe(*fpstate, (void __user *)fx_aligned, math_size) < 0) return (void __user *) -1L; diff --git a/arch/x86/include/asm/cet.h b/arch/x86/include/asm/cet.h index 5750fbcbb952..73435856ce54 100644 --- a/arch/x86/include/asm/cet.h +++ b/arch/x86/include/asm/cet.h @@ -6,6 +6,8 @@ #include struct task_struct; +struct sc_ext; + /* * Per-thread CET status */ @@ -18,9 +20,15 @@ struct cet_status { int cet_setup_shstk(void); void cet_disable_shstk(void); void cet_free_shstk(struct task_struct *p); +int cet_verify_rstor_token(bool ia32, unsigned long ssp, unsigned long *new_ssp); +void cet_restore_signal(struct sc_ext *sc); +int cet_setup_signal(bool ia32, unsigned long rstor, struct sc_ext *sc); #else static inline void cet_disable_shstk(void) {} static inline void cet_free_shstk(struct task_struct *p) {} +static inline void cet_restore_signal(struct sc_ext *sc) { return; } +static inline int cet_setup_signal(bool ia32, unsigned long rstor, + struct sc_ext *sc) { return -EINVAL; } #endif #endif /* __ASSEMBLY__ */ diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h index 0a460f2a3f90..ec900a7a4786 100644 --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -442,6 +442,16 @@ static inline void copy_kernel_to_fpregs(union fpregs_state *fpstate) __copy_kernel_to_fpregs(fpstate, -1); } +#ifdef CONFIG_X86_CET +extern int save_cet_to_sigframe(int ia32, void __user *fp, + unsigned long restorer); +#else +static inline int save_cet_to_sigframe(int ia32, void __user *fp, + unsigned long restorer) +{ + return 0; +} +#endif extern int copy_fpstate_to_sigframe(void __user *buf, void __user *fp, int size); /* diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h index 59a3e13204c3..ee86c19da532 100644 --- a/arch/x86/include/asm/special_insns.h +++ b/arch/x86/include/asm/special_insns.h @@ -232,6 +232,38 @@ static inline void clwb(volatile void *__p) : [pax] "a" (p)); } +#ifdef CONFIG_X86_CET
[PATCH v14 15/26] mm: Fixup places that call pte_mkwrite() directly
A shadow stack page is made writable by pte_mkwrite_shstk(), which sets _PAGE_DIRTY_HW. There are a few places that call pte_mkwrite() directly and miss the maybe_mkwrite() fixup in the previous patch. Fix them with maybe_mkwrite(): - do_anonymous_page() and migrate_vma_insert_page() check VM_WRITE directly and call pte_mkwrite(), which is the same as maybe_mkwrite(). Change them to maybe_mkwrite(). - In do_numa_page(), if the numa entry 'was-writable', then pte_mkwrite() is called directly. Fix it by doing maybe_mkwrite(). - In change_pte_range(), pte_mkwrite() is called directly. Replace it with maybe_mkwrite(). Signed-off-by: Yu-cheng Yu --- mm/memory.c | 5 ++--- mm/migrate.c | 3 +-- mm/mprotect.c | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/mm/memory.c b/mm/memory.c index fcfc4ca36eba..5fde329791b8 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -3563,8 +3563,7 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf) entry = mk_pte(page, vma->vm_page_prot); entry = pte_sw_mkyoung(entry); - if (vma->vm_flags & VM_WRITE) - entry = pte_mkwrite(pte_mkdirty(entry)); + entry = maybe_mkwrite(pte_mkdirty(entry), vma); vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address, >ptl); @@ -4218,7 +4217,7 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf) pte = pte_modify(old_pte, vma->vm_page_prot); pte = pte_mkyoung(pte); if (was_writable) - pte = pte_mkwrite(pte); + pte = maybe_mkwrite(pte, vma); ptep_modify_prot_commit(vma, vmf->address, vmf->pte, old_pte, pte); update_mmu_cache(vma, vmf->address, vmf->pte); diff --git a/mm/migrate.c b/mm/migrate.c index 04a98bb2f568..bba81bbcee80 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -2904,8 +2904,7 @@ static void migrate_vma_insert_page(struct migrate_vma *migrate, } } else { entry = mk_pte(page, vma->vm_page_prot); - if (vma->vm_flags & VM_WRITE) - entry = pte_mkwrite(pte_mkdirty(entry)); + entry = maybe_mkwrite(pte_mkdirty(entry), vma); } ptep = pte_offset_map_lock(mm, pmdp, addr, ); diff --git a/mm/mprotect.c b/mm/mprotect.c index ce8b8a5eacbb..a8edbcb3af99 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -135,7 +135,7 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd, if (dirty_accountable && pte_dirty(ptent) && (pte_soft_dirty(ptent) || !(vma->vm_flags & VM_SOFTDIRTY))) { - ptent = pte_mkwrite(ptent); + ptent = maybe_mkwrite(ptent, vma); } ptep_modify_prot_commit(vma, addr, pte, oldpte, ptent); pages++; -- 2.21.0
[PATCH v14 22/26] binfmt_elf: Define GNU_PROPERTY_X86_FEATURE_1_AND properties
An ELF file's .note.gnu.property indicates architecture features of the file.. Introduce feature definitions for Shadow Stack and Indirect Branch Tracking. Signed-off-by: Yu-cheng Yu --- include/uapi/linux/elf.h | 9 + 1 file changed, 9 insertions(+) diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h index 0945a5fd..ca5875f384f6 100644 --- a/include/uapi/linux/elf.h +++ b/include/uapi/linux/elf.h @@ -454,4 +454,13 @@ typedef struct elf64_note { /* Bits for GNU_PROPERTY_AARCH64_FEATURE_1_BTI */ #define GNU_PROPERTY_AARCH64_FEATURE_1_BTI (1U << 0) +/* .note.gnu.property types for x86: */ +#define GNU_PROPERTY_X86_FEATURE_1_AND 0xc002 + +/* Bits for GNU_PROPERTY_X86_FEATURE_1_AND */ +#define GNU_PROPERTY_X86_FEATURE_1_IBT 0x0001 +#define GNU_PROPERTY_X86_FEATURE_1_SHSTK 0x0002 +#define GNU_PROPERTY_X86_FEATURE_1_INVAL ~(GNU_PROPERTY_X86_FEATURE_1_IBT | \ + GNU_PROPERTY_X86_FEATURE_1_SHSTK) + #endif /* _UAPI_LINUX_ELF_H */ -- 2.21.0
[PATCH v14 26/26] mm: Introduce PROT_SHSTK for shadow stack
There are three possible options to create a shadow stack allocation API: an arch_prctl, a new syscall, or adding PROT_SHSTK to mmap()/mprotect(). Each has its advantages and compromises. An arch_prctl() is the least intrusive. However, the existing x86 arch_prctl() takes only two parameters. Multiple parameters must be passed in a memory buffer. There is a proposal to pass more parameters in registers [1], but no active discussion on that. A new syscall minimizes compatibility issues and offers an extensible frame work to other architectures, but this will likely result in some overlap of mmap()/mprotect(). The introduction of PROT_SHSTK to mmap()/mprotect() takes advantage of existing APIs. The x86-specific PROT_SHSTK is translated to VM_SHSTK and a shadow stack mapping is created without reinventing the wheel. There are potential pitfalls though. The most obvious one would be using this as a bypass to shadow stack protection. However, the attacker would have to get to the syscall first. Since arch_calc_vm_prot_bits() is modified, I have moved arch_vm_get_page _prot() and arch_calc_vm_prot_bits() to x86/include/asm/mman.h. This will be more consistent with other architectures. [1] https://lore.kernel.org/lkml/20200828121624.108243-1-hjl.to...@gmail.com/ Signed-off-by: Yu-cheng Yu --- arch/x86/include/asm/mman.h | 83 arch/x86/include/uapi/asm/mman.h | 28 ++- include/linux/mm.h | 1 + mm/mmap.c| 8 ++- 4 files changed, 95 insertions(+), 25 deletions(-) create mode 100644 arch/x86/include/asm/mman.h diff --git a/arch/x86/include/asm/mman.h b/arch/x86/include/asm/mman.h new file mode 100644 index ..0dcaef6f889a --- /dev/null +++ b/arch/x86/include/asm/mman.h @@ -0,0 +1,83 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_X86_MMAN_H +#define _ASM_X86_MMAN_H + +#include +#include + +#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS +/* + * Take the 4 protection key bits out of the vma->vm_flags + * value and turn them in to the bits that we can put in + * to a pte. + * + * Only override these if Protection Keys are available + * (which is only on 64-bit). + */ +#define arch_vm_get_page_prot(vm_flags)__pgprot( \ + ((vm_flags) & VM_PKEY_BIT0 ? _PAGE_PKEY_BIT0 : 0) | \ + ((vm_flags) & VM_PKEY_BIT1 ? _PAGE_PKEY_BIT1 : 0) | \ + ((vm_flags) & VM_PKEY_BIT2 ? _PAGE_PKEY_BIT2 : 0) | \ + ((vm_flags) & VM_PKEY_BIT3 ? _PAGE_PKEY_BIT3 : 0)) + +#define pkey_vm_prot_bits(prot, key) ( \ + ((key) & 0x1 ? VM_PKEY_BIT0 : 0) | \ + ((key) & 0x2 ? VM_PKEY_BIT1 : 0) | \ + ((key) & 0x4 ? VM_PKEY_BIT2 : 0) | \ + ((key) & 0x8 ? VM_PKEY_BIT3 : 0)) +#else +#define pkey_vm_prot_bits(prot, key) (0) +#endif + +static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot, + unsigned long pkey) +{ + unsigned long vm_prot_bits = pkey_vm_prot_bits(prot, pkey); + + if (!(prot & PROT_WRITE) && (prot & PROT_SHSTK)) + vm_prot_bits |= VM_SHSTK; + + return vm_prot_bits; +} +#define arch_calc_vm_prot_bits(prot, pkey) arch_calc_vm_prot_bits(prot, pkey) + +#ifdef CONFIG_X86_SHADOW_STACK_USER +static inline bool arch_validate_prot(unsigned long prot, unsigned long addr) +{ + unsigned long valid = PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM; + + if (prot & ~(valid | PROT_SHSTK)) + return false; + + if (prot & PROT_SHSTK) { + struct vm_area_struct *vma; + + if (!current->thread.cet.shstk_size) + return false; + + /* +* A shadow stack mapping is indirectly writable by only +* the CALL and WRUSS instructions, but not other write +* instructions). PROT_SHSTK and PROT_WRITE are mutually +* exclusive. +*/ + if (prot & PROT_WRITE) + return false; + + vma = find_vma(current->mm, addr); + if (!vma) + return false; + + /* +* Shadow stack cannot be backed by a file or shared. +*/ + if (vma->vm_file || (vma->vm_flags & VM_SHARED)) + return false; + } + + return true; +} +#define arch_validate_prot arch_validate_prot +#endif + +#endif /* _ASM_X86_MMAN_H */ diff --git a/arch/x86/include/uapi/asm/mman.h b/arch/x86/include/uapi/asm/mman.h index d4a8d0424bfb..39bb7db344a6 100644 --- a/arch/x86/include/uapi/asm/mman.h +++ b/arch/x86/include/uapi/asm/mman.h @@ -1,31 +1,11 @@ /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ -#ifndef _ASM_X86_MMAN_H -#define _ASM_X86_MMAN_H +#ifndef _UAPI_ASM_X86_MMAN_H +#define _UAPI_ASM_X86_MMAN_H #define MAP_32BIT 0x40
[PATCH v14 18/26] mm: Update can_follow_write_pte() for shadow stack
Can_follow_write_pte() ensures a read-only page is COWed by checking the FOLL_COW flag, and uses pte_dirty() to validate the flag is still valid. Like a writable data page, a shadow stack page is writable, and becomes read-only during copy-on-write, but it is always dirty. Thus, in the can_follow_write_pte() check, it belongs to the writable page case and should be excluded from the read-only page pte_dirty() check. Apply the same changes to can_follow_write_pmd(). Signed-off-by: Yu-cheng Yu --- mm/gup.c | 8 +--- mm/huge_memory.c | 8 +--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/mm/gup.c b/mm/gup.c index e869c634cc9a..10e32f574822 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -384,10 +384,12 @@ static int follow_pfn_pte(struct vm_area_struct *vma, unsigned long address, * FOLL_FORCE can write to even unwritable pte's, but only * after we've gone through a COW cycle and they are dirty. */ -static inline bool can_follow_write_pte(pte_t pte, unsigned int flags) +static inline bool can_follow_write_pte(pte_t pte, unsigned int flags, + struct vm_area_struct *vma) { return pte_write(pte) || - ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte)); + ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte) && + !arch_shadow_stack_mapping(vma->vm_flags)); } static struct page *follow_page_pte(struct vm_area_struct *vma, @@ -430,7 +432,7 @@ static struct page *follow_page_pte(struct vm_area_struct *vma, } if ((flags & FOLL_NUMA) && pte_protnone(pte)) goto no_page; - if ((flags & FOLL_WRITE) && !can_follow_write_pte(pte, flags)) { + if ((flags & FOLL_WRITE) && !can_follow_write_pte(pte, flags, vma)) { pte_unmap_unlock(ptep, ptl); return NULL; } diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 01252b00cd06..fd22ceaba945 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -1324,10 +1324,12 @@ vm_fault_t do_huge_pmd_wp_page(struct vm_fault *vmf, pmd_t orig_pmd) * FOLL_FORCE can write to even unwritable pmd's, but only * after we've gone through a COW cycle and they are dirty. */ -static inline bool can_follow_write_pmd(pmd_t pmd, unsigned int flags) +static inline bool can_follow_write_pmd(pmd_t pmd, unsigned int flags, + struct vm_area_struct *vma) { return pmd_write(pmd) || - ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pmd_dirty(pmd)); + ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pmd_dirty(pmd) && + !arch_shadow_stack_mapping(vma->vm_flags)); } struct page *follow_trans_huge_pmd(struct vm_area_struct *vma, @@ -1340,7 +1342,7 @@ struct page *follow_trans_huge_pmd(struct vm_area_struct *vma, assert_spin_locked(pmd_lockptr(mm, pmd)); - if (flags & FOLL_WRITE && !can_follow_write_pmd(*pmd, flags)) + if (flags & FOLL_WRITE && !can_follow_write_pmd(*pmd, flags, vma)) goto out; /* Avoid dumping huge zero page */ -- 2.21.0
[PATCH v14 12/26] mm: Introduce VM_SHSTK for shadow stack memory
A Shadow Stack PTE must be read-only and have _PAGE_DIRTY set. However, read-only and Dirty PTEs also exist for copy-on-write (COW) pages. These two cases are handled differently for page faults. Introduce VM_SHSTK to track shadow stack VMAs. Signed-off-by: Yu-cheng Yu Reviewed-by: Kees Cook --- arch/x86/mm/mmap.c | 2 ++ fs/proc/task_mmu.c | 3 +++ include/linux/mm.h | 8 3 files changed, 13 insertions(+) diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c index c90c20904a60..a22c6b6fc607 100644 --- a/arch/x86/mm/mmap.c +++ b/arch/x86/mm/mmap.c @@ -165,6 +165,8 @@ unsigned long get_mmap_base(int is_legacy) const char *arch_vma_name(struct vm_area_struct *vma) { + if (vma->vm_flags & VM_SHSTK) + return "[shadow stack]"; return NULL; } diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index 5066b0251ed8..436dd37f2d4c 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -663,6 +663,9 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma) [ilog2(VM_PKEY_BIT4)] = "", #endif #endif /* CONFIG_ARCH_HAS_PKEYS */ +#ifdef CONFIG_X86_SHADOW_STACK_USER + [ilog2(VM_SHSTK)] = "ss", +#endif }; size_t i; diff --git a/include/linux/mm.h b/include/linux/mm.h index 16b799a0522c..12be96b061c7 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -304,11 +304,13 @@ extern unsigned int kobjsize(const void *objp); #define VM_HIGH_ARCH_BIT_2 34 /* bit only usable on 64-bit architectures */ #define VM_HIGH_ARCH_BIT_3 35 /* bit only usable on 64-bit architectures */ #define VM_HIGH_ARCH_BIT_4 36 /* bit only usable on 64-bit architectures */ +#define VM_HIGH_ARCH_BIT_5 37 /* bit only usable on 64-bit architectures */ #define VM_HIGH_ARCH_0 BIT(VM_HIGH_ARCH_BIT_0) #define VM_HIGH_ARCH_1 BIT(VM_HIGH_ARCH_BIT_1) #define VM_HIGH_ARCH_2 BIT(VM_HIGH_ARCH_BIT_2) #define VM_HIGH_ARCH_3 BIT(VM_HIGH_ARCH_BIT_3) #define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4) +#define VM_HIGH_ARCH_5 BIT(VM_HIGH_ARCH_BIT_5) #endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */ #ifdef CONFIG_ARCH_HAS_PKEYS @@ -324,6 +326,12 @@ extern unsigned int kobjsize(const void *objp); #endif #endif /* CONFIG_ARCH_HAS_PKEYS */ +#ifdef CONFIG_X86_SHADOW_STACK_USER +# define VM_SHSTK VM_HIGH_ARCH_5 +#else +# define VM_SHSTK VM_NONE +#endif + #if defined(CONFIG_X86) # define VM_PATVM_ARCH_1 /* PAT reserves whole VMA at once (x86) */ #elif defined(CONFIG_PPC) -- 2.21.0
Re: [PATCH 02/18] dt-bindings: usb: usb-hcd: Add "wireless" maximum-speed property value
On Sun, Oct 11, 2020 at 08:53:33PM +0300, Serge Semin wrote: > On Sun, Oct 11, 2020 at 04:42:36PM +0200, Greg Kroah-Hartman wrote: > > On Sun, Oct 11, 2020 at 01:41:05AM +0300, Serge Semin wrote: > > > It appears that the "maximum-speed" property can also accept the > > > "wireless" value. Add it to the enumeration of the possible property > > > values then. > > > > > > Signed-off-by: Serge Semin > > > --- > > > Documentation/devicetree/bindings/usb/usb-hcd.yaml | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/usb/usb-hcd.yaml > > > b/Documentation/devicetree/bindings/usb/usb-hcd.yaml > > > index 815de24127db..e1a82a2b3295 100644 > > > --- a/Documentation/devicetree/bindings/usb/usb-hcd.yaml > > > +++ b/Documentation/devicetree/bindings/usb/usb-hcd.yaml > > > @@ -28,7 +28,7 @@ properties: > > > isn't passed via DT, USB controllers should default to their > > > maximum HW > > > capability. > > > $ref: /schemas/types.yaml#/definitions/string > > > - enum: ["low-speed", "full-speed", "high-speed", "super-speed", > > > + enum: ["low-speed", "full-speed", "high-speed", "wireless", > > > "super-speed", > > >"super-speed-plus"] > > > > > Are you sure? wireless usb has been removed from the kernel, where do > > you see a user of this? If it's still in there, we need to just drop > > it. > > My decision on suggesting this patch has been based purely on the speed types > the USB core API supports and what usb_get_maximum_speed() can return. > USB_SPEED_WIRELESS type is one of the possible return values. As I can see > aside the rest of the USB speeds the wireless speed is also defined > in the kernel USB subsystem. Moreover it is used in some kernel drivers. > (See the USB_SPEED_WIRELESS enumeration constant usage.) > Are you sure that the wireless speed support has been really removed? All of the drivers that implement and support this should have been removed. Code in the USB core is probably not removed, but patches are gladly welcome. Please do not add new wireless support as it is not going to happen, because there are no wireless devices in the world. thanks, greg k-h
[PATCH v14 14/26] x86/mm: Update maybe_mkwrite() for shadow stack
Shadow stack memory is writable, but its VMA has VM_SHSTK instead of VM_WRITE. Update maybe_mkwrite() to include the shadow stack. Signed-off-by: Yu-cheng Yu --- arch/x86/Kconfig| 4 arch/x86/mm/pgtable.c | 18 ++ include/linux/mm.h | 2 ++ include/linux/pgtable.h | 24 mm/huge_memory.c| 2 ++ 5 files changed, 50 insertions(+) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 415fcc869afc..7578327226e3 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1935,6 +1935,9 @@ config AS_HAS_SHADOW_STACK config X86_CET def_bool n +config ARCH_MAYBE_MKWRITE + def_bool n + config ARCH_HAS_SHADOW_STACK def_bool n @@ -1945,6 +1948,7 @@ config X86_SHADOW_STACK_USER depends on AS_HAS_SHADOW_STACK select ARCH_USES_HIGH_VMA_FLAGS select X86_CET + select ARCH_MAYBE_MKWRITE select ARCH_HAS_SHADOW_STACK help Shadow Stacks provides protection against program stack diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c index dfd82f51ba66..a9666b64bc05 100644 --- a/arch/x86/mm/pgtable.c +++ b/arch/x86/mm/pgtable.c @@ -610,6 +610,24 @@ int pmdp_clear_flush_young(struct vm_area_struct *vma, } #endif +#ifdef CONFIG_ARCH_MAYBE_MKWRITE +pte_t arch_maybe_mkwrite(pte_t pte, struct vm_area_struct *vma) +{ + if (likely(vma->vm_flags & VM_SHSTK)) + pte = pte_mkwrite_shstk(pte); + return pte; +} + +#ifdef CONFIG_TRANSPARENT_HUGEPAGE +pmd_t arch_maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma) +{ + if (likely(vma->vm_flags & VM_SHSTK)) + pmd = pmd_mkwrite_shstk(pmd); + return pmd; +} +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */ +#endif /* CONFIG_ARCH_MAYBE_MKWRITE */ + /** * reserve_top_address - reserves a hole in the top of kernel address space * @reserve - size of hole to reserve diff --git a/include/linux/mm.h b/include/linux/mm.h index 12be96b061c7..4f6305106feb 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -969,6 +969,8 @@ static inline pte_t maybe_mkwrite(pte_t pte, struct vm_area_struct *vma) { if (likely(vma->vm_flags & VM_WRITE)) pte = pte_mkwrite(pte); + else + pte = arch_maybe_mkwrite(pte, vma); return pte; } diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h index 90654cb63e9e..157f5e726896 100644 --- a/include/linux/pgtable.h +++ b/include/linux/pgtable.h @@ -1356,6 +1356,30 @@ static inline bool arch_has_pfn_modify_check(void) } #endif /* !_HAVE_ARCH_PFN_MODIFY_ALLOWED */ +#ifdef CONFIG_MMU +#ifdef CONFIG_ARCH_MAYBE_MKWRITE +pte_t arch_maybe_mkwrite(pte_t pte, struct vm_area_struct *vma); + +#ifdef CONFIG_TRANSPARENT_HUGEPAGE +pmd_t arch_maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma); +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */ + +#else /* !CONFIG_ARCH_MAYBE_MKWRITE */ +static inline pte_t arch_maybe_mkwrite(pte_t pte, struct vm_area_struct *vma) +{ + return pte; +} + +#ifdef CONFIG_TRANSPARENT_HUGEPAGE +static inline pmd_t arch_maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma) +{ + return pmd; +} +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */ + +#endif /* CONFIG_ARCH_MAYBE_MKWRITE */ +#endif /* CONFIG_MMU */ + /* * Architecture PAGE_KERNEL_* fallbacks * diff --git a/mm/huge_memory.c b/mm/huge_memory.c index da397779a6d4..01252b00cd06 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -464,6 +464,8 @@ pmd_t maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma) { if (likely(vma->vm_flags & VM_WRITE)) pmd = pmd_mkwrite(pmd); + else + pmd = arch_maybe_pmd_mkwrite(pmd, vma); return pmd; } -- 2.21.0
[PATCH v14 08/26] x86/mm: Introduce _PAGE_COW
There is essentially no room left in the x86 hardware PTEs on some OSes (not Linux). That left the hardware architects looking for a way to represent a new memory type (shadow stack) within the existing bits. They chose to repurpose a lightly-used state: Write=0,Dirty=1. The reason it's lightly used is that Dirty=1 is normally set by hardware and cannot normally be set by hardware on a Write=0 PTE. Software must normally be involved to create one of these PTEs, so software can simply opt to not create them. But that leaves us with a Linux problem: we need to ensure we never create Write=0,Dirty=1 PTEs. In places where we do create them, we need to find an alternative way to represent them _without_ using the same hardware bit combination. Thus, enter _PAGE_COW. This results in the following: (a) A modified, copy-on-write (COW) page: (R/O + _PAGE_COW) (b) A R/O page that has been COW'ed: (R/O + _PAGE_COW) The user page is in a R/O VMA, and get_user_pages() needs a writable copy. The page fault handler creates a copy of the page and sets the new copy's PTE as R/O and _PAGE_COW. (c) A shadow stack PTE: (R/O + _PAGE_DIRTY_HW) (d) A shared shadow stack PTE: (R/O + _PAGE_COW) When a shadow stack page is being shared among processes (this happens at fork()), its PTE is cleared of _PAGE_DIRTY_HW, so the next shadow stack access causes a fault, and the page is duplicated and _PAGE_DIRTY_HW is set again. This is the COW equivalent for shadow stack pages, even though it's copy-on-access rather than copy-on-write. (e) A page where the processor observed a Write=1 PTE, started a write, set Dirty=1, but then observed a Write=0 PTE. That's possible today, but will not happen on processors that support shadow stack. Use _PAGE_COW in pte_wrprotect() and _PAGE_DIRTY_HW in pte_mkwrite(). Apply the same changes to pmd and pud. When this patch is applied, there are six free bits left in the 64-bit PTE. There are no more free bits in the 32-bit PTE (except for PAE) and shadow stack is not implemented for the 32-bit kernel. Signed-off-by: Yu-cheng Yu Reviewed-by: Kees Cook --- arch/x86/include/asm/pgtable.h | 120 --- arch/x86/include/asm/pgtable_types.h | 41 - 2 files changed, 150 insertions(+), 11 deletions(-) diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h index 86b7acd221c1..ac4ed814be96 100644 --- a/arch/x86/include/asm/pgtable.h +++ b/arch/x86/include/asm/pgtable.h @@ -122,9 +122,9 @@ extern pmdval_t early_pmd_flags; * The following only work if pte_present() is true. * Undefined behaviour if not.. */ -static inline int pte_dirty(pte_t pte) +static inline bool pte_dirty(pte_t pte) { - return pte_flags(pte) & _PAGE_DIRTY_HW; + return pte_flags(pte) & _PAGE_DIRTY_BITS; } @@ -161,9 +161,9 @@ static inline int pte_young(pte_t pte) return pte_flags(pte) & _PAGE_ACCESSED; } -static inline int pmd_dirty(pmd_t pmd) +static inline bool pmd_dirty(pmd_t pmd) { - return pmd_flags(pmd) & _PAGE_DIRTY_HW; + return pmd_flags(pmd) & _PAGE_DIRTY_BITS; } static inline int pmd_young(pmd_t pmd) @@ -171,9 +171,9 @@ static inline int pmd_young(pmd_t pmd) return pmd_flags(pmd) & _PAGE_ACCESSED; } -static inline int pud_dirty(pud_t pud) +static inline bool pud_dirty(pud_t pud) { - return pud_flags(pud) & _PAGE_DIRTY_HW; + return pud_flags(pud) & _PAGE_DIRTY_BITS; } static inline int pud_young(pud_t pud) @@ -183,6 +183,12 @@ static inline int pud_young(pud_t pud) static inline int pte_write(pte_t pte) { + /* +* If _PAGE_DIRTY_HW is set, the PTE must either have +* _PAGE_RW or be a shadow stack PTE, which is logically writable. +*/ + if (cpu_feature_enabled(X86_FEATURE_SHSTK)) + return pte_flags(pte) & (_PAGE_RW | _PAGE_DIRTY_HW); return pte_flags(pte) & _PAGE_RW; } @@ -334,7 +340,7 @@ static inline pte_t pte_clear_uffd_wp(pte_t pte) static inline pte_t pte_mkclean(pte_t pte) { - return pte_clear_flags(pte, _PAGE_DIRTY_HW); + return pte_clear_flags(pte, _PAGE_DIRTY_BITS); } static inline pte_t pte_mkold(pte_t pte) @@ -344,6 +350,17 @@ static inline pte_t pte_mkold(pte_t pte) static inline pte_t pte_wrprotect(pte_t pte) { + /* +* Blindly clearing _PAGE_RW might accidentally create +* a shadow stack PTE (RW=0,Dirty=1). Move the hardware +* dirty value to the software bit. +*/ + if (cpu_feature_enabled(X86_FEATURE_SHSTK)) { + pte.pte |= (pte.pte & _PAGE_DIRTY_HW) >> + _PAGE_BIT_DIRTY_HW << _PAGE_BIT_COW; + pte = pte_clear_flags(pte, _PAGE_DIRTY_HW); + } + return pte_clear_flags(pte, _PAGE_RW); } @@ -354,6 +371,18 @@ static inline pte_t pte_mkexec(pte_t pte) static inline pte_t pte_mkdirty(pte_t pte) { + pteval_t dirty = _PAGE_DIRTY_HW; +
[PATCH v14 02/26] x86/cpufeatures: Add CET CPU feature flags for Control-flow Enforcement Technology (CET)
Add CPU feature flags for Control-flow Enforcement Technology (CET). CPUID.(EAX=7,ECX=0):ECX[bit 7] Shadow stack CPUID.(EAX=7,ECX=0):EDX[bit 20] Indirect Branch Tracking Signed-off-by: Yu-cheng Yu Reviewed-by: Borislav Petkov Reviewed-by: Kees Cook --- arch/x86/include/asm/cpufeatures.h | 2 ++ arch/x86/kernel/cpu/cpuid-deps.c | 2 ++ tools/arch/x86/include/asm/cpufeatures.h | 2 ++ 3 files changed, 6 insertions(+) diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index 2901d5df4366..c794e18e8a14 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -341,6 +341,7 @@ #define X86_FEATURE_OSPKE (16*32+ 4) /* OS Protection Keys Enable */ #define X86_FEATURE_WAITPKG(16*32+ 5) /* UMONITOR/UMWAIT/TPAUSE Instructions */ #define X86_FEATURE_AVX512_VBMI2 (16*32+ 6) /* Additional AVX512 Vector Bit Manipulation Instructions */ +#define X86_FEATURE_SHSTK (16*32+ 7) /* Shadow Stack */ #define X86_FEATURE_GFNI (16*32+ 8) /* Galois Field New Instructions */ #define X86_FEATURE_VAES (16*32+ 9) /* Vector AES */ #define X86_FEATURE_VPCLMULQDQ (16*32+10) /* Carry-Less Multiplication Double Quadword */ @@ -370,6 +371,7 @@ #define X86_FEATURE_SERIALIZE (18*32+14) /* SERIALIZE instruction */ #define X86_FEATURE_PCONFIG(18*32+18) /* Intel PCONFIG */ #define X86_FEATURE_ARCH_LBR (18*32+19) /* Intel ARCH LBR */ +#define X86_FEATURE_IBT(18*32+20) /* Indirect Branch Tracking */ #define X86_FEATURE_SPEC_CTRL (18*32+26) /* "" Speculation Control (IBRS + IBPB) */ #define X86_FEATURE_INTEL_STIBP(18*32+27) /* "" Single Thread Indirect Branch Predictors */ #define X86_FEATURE_FLUSH_L1D (18*32+28) /* Flush L1D cache */ diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c index 3cbe24ca80ab..fec83cc74b9e 100644 --- a/arch/x86/kernel/cpu/cpuid-deps.c +++ b/arch/x86/kernel/cpu/cpuid-deps.c @@ -69,6 +69,8 @@ static const struct cpuid_dep cpuid_deps[] = { { X86_FEATURE_CQM_MBM_TOTAL,X86_FEATURE_CQM_LLC }, { X86_FEATURE_CQM_MBM_LOCAL,X86_FEATURE_CQM_LLC }, { X86_FEATURE_AVX512_BF16, X86_FEATURE_AVX512VL }, + { X86_FEATURE_SHSTK,X86_FEATURE_XSAVES}, + { X86_FEATURE_IBT, X86_FEATURE_XSAVES}, {} }; diff --git a/tools/arch/x86/include/asm/cpufeatures.h b/tools/arch/x86/include/asm/cpufeatures.h index 2901d5df4366..c794e18e8a14 100644 --- a/tools/arch/x86/include/asm/cpufeatures.h +++ b/tools/arch/x86/include/asm/cpufeatures.h @@ -341,6 +341,7 @@ #define X86_FEATURE_OSPKE (16*32+ 4) /* OS Protection Keys Enable */ #define X86_FEATURE_WAITPKG(16*32+ 5) /* UMONITOR/UMWAIT/TPAUSE Instructions */ #define X86_FEATURE_AVX512_VBMI2 (16*32+ 6) /* Additional AVX512 Vector Bit Manipulation Instructions */ +#define X86_FEATURE_SHSTK (16*32+ 7) /* Shadow Stack */ #define X86_FEATURE_GFNI (16*32+ 8) /* Galois Field New Instructions */ #define X86_FEATURE_VAES (16*32+ 9) /* Vector AES */ #define X86_FEATURE_VPCLMULQDQ (16*32+10) /* Carry-Less Multiplication Double Quadword */ @@ -370,6 +371,7 @@ #define X86_FEATURE_SERIALIZE (18*32+14) /* SERIALIZE instruction */ #define X86_FEATURE_PCONFIG(18*32+18) /* Intel PCONFIG */ #define X86_FEATURE_ARCH_LBR (18*32+19) /* Intel ARCH LBR */ +#define X86_FEATURE_IBT(18*32+20) /* Indirect Branch Tracking */ #define X86_FEATURE_SPEC_CTRL (18*32+26) /* "" Speculation Control (IBRS + IBPB) */ #define X86_FEATURE_INTEL_STIBP(18*32+27) /* "" Single Thread Indirect Branch Predictors */ #define X86_FEATURE_FLUSH_L1D (18*32+28) /* Flush L1D cache */ -- 2.21.0
[PATCH v14 00/26] Control-flow Enforcement: Shadow Stack
Control-flow Enforcement (CET) is a new Intel processor feature that blocks return/jump-oriented programming attacks. Details are in "Intel 64 and IA-32 Architectures Software Developer's Manual" [1]. CET can protect applications and the kernel. This series enables only application-level protection, and has three parts: - Shadow stack [2], - Indirect branch tracking [3], and - Selftests [4]. I have run tests on these patches for quite some time, and they have been very stable. Linux distributions with CET are available now, and Intel processors with CET are becoming available. It would be nice if CET support can be accepted into the kernel. I will be working to address any issues should they come up. Changes in v14: - Update patch #10, add cpu_feature_enabled() to fixup_dirty_*. - Update patch #19, instead of re-introducing do_mmap_pgoff(), make all callers of the wrapper pass a zero vm_flags. - Update patch #26, move checking vm_flags into arch_validate_prot(); check the task's shadow stack status, instead of static_cpu_has(). [1] Intel 64 and IA-32 Architectures Software Developer's Manual: https://software.intel.com/en-us/download/intel-64-and-ia-32- architectures-sdm-combined-volumes-1-2a-2b-2c-2d-3a-3b-3c-3d-and-4 [2] CET Shadow Stack patches v13: https://lkml.kernel.org/r/20200925145649.5438-2-yu-cheng...@intel.com/ [3] Indirect Branch Tracking patches v13. https://lkml.kernel.org/r/20200925145804.5821-1-yu-cheng...@intel.com/ [4] I am holding off the selftests changes and working to get Acked-by's. The earlier version of the selftests patches: https://lkml.kernel.org/r/20200521211720.20236-1-yu-cheng...@intel.com/ [5] The kernel ptrace patch is tested with an Intel-internal updated GDB. I am holding off the kernel ptrace patch to re-test it with my earlier patch for fixing regset holes. Yu-cheng Yu (26): Documentation/x86: Add CET description x86/cpufeatures: Add CET CPU feature flags for Control-flow Enforcement Technology (CET) x86/fpu/xstate: Introduce CET MSR XSAVES supervisor states x86/cet: Add control-protection fault handler x86/cet/shstk: Add Kconfig option for user-mode Shadow Stack x86/mm: Change _PAGE_DIRTY to _PAGE_DIRTY_HW x86/mm: Remove _PAGE_DIRTY_HW from kernel RO pages x86/mm: Introduce _PAGE_COW drm/i915/gvt: Change _PAGE_DIRTY to _PAGE_DIRTY_BITS x86/mm: Update pte_modify for _PAGE_COW x86/mm: Update ptep_set_wrprotect() and pmdp_set_wrprotect() for transition from _PAGE_DIRTY_HW to _PAGE_COW mm: Introduce VM_SHSTK for shadow stack memory x86/mm: Shadow Stack page fault error checking x86/mm: Update maybe_mkwrite() for shadow stack mm: Fixup places that call pte_mkwrite() directly mm: Add guard pages around a shadow stack. mm/mmap: Add shadow stack pages to memory accounting mm: Update can_follow_write_pte() for shadow stack mm: Re-introduce vm_flags to do_mmap() x86/cet/shstk: User-mode shadow stack support x86/cet/shstk: Handle signals for shadow stack binfmt_elf: Define GNU_PROPERTY_X86_FEATURE_1_AND properties ELF: Introduce arch_setup_elf_property() x86/cet/shstk: Handle thread shadow stack x86/cet/shstk: Add arch_prctl functions for shadow stack mm: Introduce PROT_SHSTK for shadow stack .../admin-guide/kernel-parameters.txt | 6 + Documentation/x86/index.rst | 1 + Documentation/x86/intel_cet.rst | 133 +++ arch/arm64/include/asm/elf.h | 5 + arch/x86/Kconfig | 39 ++ arch/x86/ia32/ia32_signal.c | 17 + arch/x86/include/asm/cet.h| 42 +++ arch/x86/include/asm/cpufeatures.h| 2 + arch/x86/include/asm/disabled-features.h | 8 +- arch/x86/include/asm/elf.h| 13 + arch/x86/include/asm/fpu/internal.h | 10 + arch/x86/include/asm/fpu/types.h | 23 +- arch/x86/include/asm/fpu/xstate.h | 5 +- arch/x86/include/asm/idtentry.h | 4 + arch/x86/include/asm/mman.h | 83 + arch/x86/include/asm/mmu_context.h| 3 + arch/x86/include/asm/msr-index.h | 20 + arch/x86/include/asm/pgtable.h| 209 ++- arch/x86/include/asm/pgtable_types.h | 58 ++- arch/x86/include/asm/processor.h | 15 + arch/x86/include/asm/special_insns.h | 32 ++ arch/x86/include/asm/traps.h | 2 + arch/x86/include/uapi/asm/mman.h | 28 +- arch/x86/include/uapi/asm/prctl.h | 4 + arch/x86/include/uapi/asm/processor-flags.h | 2 + arch/x86/include/uapi/asm/sigcontext.h| 9 + arch/x86/kernel/Makefile | 2 + arch/x86/kernel/cet.c | 343 ++ arch/x86/kernel/cet_prctl.c | 68 arch/x86/kernel/cpu/common.c | 28
[PATCH v14 03/26] x86/fpu/xstate: Introduce CET MSR XSAVES supervisor states
Control-flow Enforcement Technology (CET) adds five MSRs. Introduce them and their XSAVES supervisor states: MSR_IA32_U_CET (user-mode CET settings), MSR_IA32_PL3_SSP (user-mode Shadow Stack pointer), MSR_IA32_PL0_SSP (kernel-mode Shadow Stack pointer), MSR_IA32_PL1_SSP (Privilege Level 1 Shadow Stack pointer), MSR_IA32_PL2_SSP (Privilege Level 2 Shadow Stack pointer). Signed-off-by: Yu-cheng Yu Reviewed-by: Kees Cook --- arch/x86/include/asm/fpu/types.h| 23 +++-- arch/x86/include/asm/fpu/xstate.h | 5 ++-- arch/x86/include/asm/msr-index.h| 20 +++ arch/x86/include/uapi/asm/processor-flags.h | 2 ++ arch/x86/kernel/fpu/xstate.c| 28 ++--- tools/arch/x86/include/asm/msr-index.h | 20 +++ 6 files changed, 91 insertions(+), 7 deletions(-) diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h index c87364ea6446..2a7037a6f960 100644 --- a/arch/x86/include/asm/fpu/types.h +++ b/arch/x86/include/asm/fpu/types.h @@ -115,8 +115,8 @@ enum xfeature { XFEATURE_PT_UNIMPLEMENTED_SO_FAR, XFEATURE_PKRU, XFEATURE_RSRVD_COMP_10, - XFEATURE_RSRVD_COMP_11, - XFEATURE_RSRVD_COMP_12, + XFEATURE_CET_USER, + XFEATURE_CET_KERNEL, XFEATURE_RSRVD_COMP_13, XFEATURE_RSRVD_COMP_14, XFEATURE_LBR, @@ -134,6 +134,8 @@ enum xfeature { #define XFEATURE_MASK_Hi16_ZMM (1 << XFEATURE_Hi16_ZMM) #define XFEATURE_MASK_PT (1 << XFEATURE_PT_UNIMPLEMENTED_SO_FAR) #define XFEATURE_MASK_PKRU (1 << XFEATURE_PKRU) +#define XFEATURE_MASK_CET_USER (1 << XFEATURE_CET_USER) +#define XFEATURE_MASK_CET_KERNEL (1 << XFEATURE_CET_KERNEL) #define XFEATURE_MASK_LBR (1 << XFEATURE_LBR) #define XFEATURE_MASK_FPSSE(XFEATURE_MASK_FP | XFEATURE_MASK_SSE) @@ -236,6 +238,23 @@ struct pkru_state { u32 pad; } __packed; +/* + * State component 11 is Control-flow Enforcement user states + */ +struct cet_user_state { + u64 user_cet; /* user control-flow settings */ + u64 user_ssp; /* user shadow stack pointer */ +}; + +/* + * State component 12 is Control-flow Enforcement kernel states + */ +struct cet_kernel_state { + u64 kernel_ssp; /* kernel shadow stack */ + u64 pl1_ssp;/* privilege level 1 shadow stack */ + u64 pl2_ssp;/* privilege level 2 shadow stack */ +}; + /* * State component 15: Architectural LBR configuration state. * The size of Arch LBR state depends on the number of LBRs (lbr_depth). diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h index 14ab815132d4..e4408db88bca 100644 --- a/arch/x86/include/asm/fpu/xstate.h +++ b/arch/x86/include/asm/fpu/xstate.h @@ -35,7 +35,7 @@ XFEATURE_MASK_BNDCSR) /* All currently supported supervisor features */ -#define XFEATURE_MASK_SUPERVISOR_SUPPORTED (0) +#define XFEATURE_MASK_SUPERVISOR_SUPPORTED (XFEATURE_MASK_CET_USER) /* * A supervisor state component may not always contain valuable information, @@ -62,7 +62,8 @@ * Unsupported supervisor features. When a supervisor feature in this mask is * supported in the future, move it to the supported supervisor feature mask. */ -#define XFEATURE_MASK_SUPERVISOR_UNSUPPORTED (XFEATURE_MASK_PT) +#define XFEATURE_MASK_SUPERVISOR_UNSUPPORTED (XFEATURE_MASK_PT | \ + XFEATURE_MASK_CET_KERNEL) /* All supervisor states including supported and unsupported states. */ #define XFEATURE_MASK_SUPERVISOR_ALL (XFEATURE_MASK_SUPERVISOR_SUPPORTED | \ diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h index 2859ee4f39a8..f6f3a0e6c664 100644 --- a/arch/x86/include/asm/msr-index.h +++ b/arch/x86/include/asm/msr-index.h @@ -912,4 +912,24 @@ #define MSR_VM_IGNNE0xc0010115 #define MSR_VM_HSAVE_PA 0xc0010117 +/* Control-flow Enforcement Technology MSRs */ +#define MSR_IA32_U_CET 0x6a0 /* user mode cet setting */ +#define MSR_IA32_S_CET 0x6a2 /* kernel mode cet setting */ +#define MSR_IA32_PL0_SSP 0x6a4 /* kernel shstk pointer */ +#define MSR_IA32_PL1_SSP 0x6a5 /* ring-1 shstk pointer */ +#define MSR_IA32_PL2_SSP 0x6a6 /* ring-2 shstk pointer */ +#define MSR_IA32_PL3_SSP 0x6a7 /* user shstk pointer */ +#define MSR_IA32_INT_SSP_TAB 0x6a8 /* exception shstk table */ + +/* MSR_IA32_U_CET and MSR_IA32_S_CET bits */ +#define CET_SHSTK_EN BIT_ULL(0) +#define CET_WRSS_ENBIT_ULL(1) +#define CET_ENDBR_EN BIT_ULL(2) +#define CET_LEG_IW_EN BIT_ULL(3) +#define CET_NO_TRACK_ENBIT_ULL(4) +#define CET_SUPPRESS_DISABLE BIT_ULL(5) +#define CET_RESERVED
[PATCH v14 11/26] x86/mm: Update ptep_set_wrprotect() and pmdp_set_wrprotect() for transition from _PAGE_DIRTY_HW to _PAGE_COW
When shadow stack is introduced, [R/O + _PAGE_DIRTY_HW] PTE is reserved for shadow stack. Copy-on-write PTEs have [R/O + _PAGE_COW]. When a PTE goes from [R/W + _PAGE_DIRTY_HW] to [R/O + _PAGE_COW], it could become a transient shadow stack PTE in two cases: The first case is that some processors can start a write but end up seeing a read-only PTE by the time they get to the Dirty bit, creating a transient shadow stack PTE. However, this will not occur on processors supporting shadow stack, therefore we don't need a TLB flush here. The second case is that when the software, without atomic, tests & replaces _PAGE_DIRTY_HW with _PAGE_COW, a transient shadow stack PTE can exist. This is prevented with cmpxchg. Dave Hansen, Jann Horn, Andy Lutomirski, and Peter Zijlstra provided many insights to the issue. Jann Horn provided the cmpxchg solution. Signed-off-by: Yu-cheng Yu Reviewed-by: Kees Cook --- arch/x86/include/asm/pgtable.h | 52 ++ 1 file changed, 52 insertions(+) diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h index 8d4c09831e67..8e637a5ed9e4 100644 --- a/arch/x86/include/asm/pgtable.h +++ b/arch/x86/include/asm/pgtable.h @@ -1230,6 +1230,32 @@ static inline pte_t ptep_get_and_clear_full(struct mm_struct *mm, static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addr, pte_t *ptep) { + /* +* Some processors can start a write, but end up seeing a read-only +* PTE by the time they get to the Dirty bit. In this case, they +* will set the Dirty bit, leaving a read-only, Dirty PTE which +* looks like a shadow stack PTE. +* +* However, this behavior has been improved and will not occur on +* processors supporting shadow stack. Without this guarantee, a +* transition to a non-present PTE and flush the TLB would be +* needed. +* +* When changing a writable PTE to read-only and if the PTE has +* _PAGE_DIRTY_HW set, move that bit to _PAGE_COW so that the +* PTE is not a shadow stack PTE. +*/ + if (cpu_feature_enabled(X86_FEATURE_SHSTK)) { + pte_t old_pte, new_pte; + + do { + old_pte = READ_ONCE(*ptep); + new_pte = pte_wrprotect(old_pte); + + } while (!try_cmpxchg(>pte, _pte.pte, new_pte.pte)); + + return; + } clear_bit(_PAGE_BIT_RW, (unsigned long *)>pte); } @@ -1286,6 +1312,32 @@ static inline pud_t pudp_huge_get_and_clear(struct mm_struct *mm, static inline void pmdp_set_wrprotect(struct mm_struct *mm, unsigned long addr, pmd_t *pmdp) { + /* +* Some processors can start a write, but end up seeing a read-only +* PMD by the time they get to the Dirty bit. In this case, they +* will set the Dirty bit, leaving a read-only, Dirty PMD which +* looks like a Shadow Stack PMD. +* +* However, this behavior has been improved and will not occur on +* processors supporting Shadow Stack. Without this guarantee, a +* transition to a non-present PMD and flush the TLB would be +* needed. +* +* When changing a writable PMD to read-only and if the PMD has +* _PAGE_DIRTY_HW set, we move that bit to _PAGE_COW so that the +* PMD is not a shadow stack PMD. +*/ + if (cpu_feature_enabled(X86_FEATURE_SHSTK)) { + pmd_t old_pmd, new_pmd; + + do { + old_pmd = READ_ONCE(*pmdp); + new_pmd = pmd_wrprotect(old_pmd); + + } while (!try_cmpxchg((pmdval_t *)pmdp, (pmdval_t *)_pmd, pmd_val(new_pmd))); + + return; + } clear_bit(_PAGE_BIT_RW, (unsigned long *)pmdp); } -- 2.21.0
[PATCH v14 01/26] Documentation/x86: Add CET description
Explain no_user_shstk/no_user_ibt kernel parameters, and introduce a new document on Control-flow Enforcement Technology (CET). Signed-off-by: Yu-cheng Yu Reviewed-by: Kees Cook --- .../admin-guide/kernel-parameters.txt | 6 + Documentation/x86/index.rst | 1 + Documentation/x86/intel_cet.rst | 133 ++ 3 files changed, 140 insertions(+) create mode 100644 Documentation/x86/intel_cet.rst diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index a1068742a6df..7c7124a6a7ac 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -3164,6 +3164,12 @@ noexec=on: enable non-executable mappings (default) noexec=off: disable non-executable mappings + no_user_shstk [X86-64] Disable Shadow Stack for user-mode + applications + + no_user_ibt [X86-64] Disable Indirect Branch Tracking for user-mode + applications + nosmap [X86,PPC] Disable SMAP (Supervisor Mode Access Prevention) even if it is supported by processor. diff --git a/Documentation/x86/index.rst b/Documentation/x86/index.rst index 265d9e9a093b..2aef972a868d 100644 --- a/Documentation/x86/index.rst +++ b/Documentation/x86/index.rst @@ -19,6 +19,7 @@ x86-specific Documentation tlb mtrr pat + intel_cet intel-iommu intel_txt amd-memory-encryption diff --git a/Documentation/x86/intel_cet.rst b/Documentation/x86/intel_cet.rst new file mode 100644 index ..c9ebb3d9dd00 --- /dev/null +++ b/Documentation/x86/intel_cet.rst @@ -0,0 +1,133 @@ +.. SPDX-License-Identifier: GPL-2.0 + += +Control-flow Enforcement Technology (CET) += + +[1] Overview + + +Control-flow Enforcement Technology (CET) is an Intel processor feature +that provides protection against return/jump-oriented programming (ROP) +attacks. It can be set up to protect both applications and the kernel. +Only user-mode protection is implemented in the 64-bit kernel, including +support for running legacy 32-bit applications. + +CET introduces Shadow Stack and Indirect Branch Tracking. Shadow stack is +a secondary stack allocated from memory and cannot be directly modified by +applications. When executing a CALL, the processor pushes the return +address to both the normal stack and the shadow stack. Upon function +return, the processor pops the shadow stack copy and compares it to the +normal stack copy. If the two differ, the processor raises a control- +protection fault. Indirect branch tracking verifies indirect CALL/JMP +targets are intended as marked by the compiler with 'ENDBR' opcodes. + +There are two kernel configuration options: + +X86_SHADOW_STACK_USER, and +X86_BRANCH_TRACKING_USER. + +These need to be enabled to build a CET-enabled kernel, and Binutils v2.31 +and GCC v8.1 or later are required to build a CET kernel. To build a CET- +enabled application, GLIBC v2.28 or later is also required. + +There are two command-line options for disabling CET features:: + +no_user_shstk - disables user shadow stack, and +no_user_ibt - disables user indirect branch tracking. + +At run time, /proc/cpuinfo shows CET features if the processor supports +CET. + +[2] Application Enabling + + +An application's CET capability is marked in its ELF header and can be +verified from the following command output, in the NT_GNU_PROPERTY_TYPE_0 +field: + +readelf -n + +If an application supports CET and is statically linked, it will run with +CET protection. If the application needs any shared libraries, the loader +checks all dependencies and enables CET when all requirements are met. + +[3] Backward Compatibility +== + +GLIBC provides a few tunables for backward compatibility. + +GLIBC_TUNABLES=glibc.tune.hwcaps=-SHSTK,-IBT +Turn off SHSTK/IBT for the current shell. + +GLIBC_TUNABLES=glibc.tune.x86_shstk= +This controls how dlopen() handles SHSTK legacy libraries:: + +on - continue with SHSTK enabled; +permissive - continue with SHSTK off. + +[4] CET arch_prctl()'s +== + +Several arch_prctl()'s have been added for CET: + +arch_prctl(ARCH_X86_CET_STATUS, u64 *addr) +Return CET feature status. + +The parameter 'addr' is a pointer to a user buffer. +On returning to the caller, the kernel fills the following +information:: + +*addr = shadow stack/indirect branch tracking status +*(addr + 1) = shadow stack base address +*(addr + 2) = shadow stack size + +arch_prctl(ARCH_X86_CET_DISABLE, unsigned int features) +Disable shadow stack and/or indirect branch tracking as
Re: [PATCH 0/5] tracing: Synthetic event dynamic string fixes
Hi Steve, On Mon, 2020-10-12 at 11:13 -0400, Steven Rostedt wrote: > On Fri, 9 Oct 2020 10:17:06 -0500 > Tom Zanussi wrote: > > > These patches provide fixes for the problems observed by Masami in > > the > > new synthetic event dynamic string patchset. > > > > The first patch (tracing: Don't show dynamic string internals in > > synthetic event description) removes the __data_loc from the event > > description but leaves it in the format. > > > > The patch (tracing: Add synthetic event error logging) addresses > > the > > lack of error messages when parse errors occur. > > > > The remaining three patches address the other problems Masami noted > > which result from allowing illegal characters in synthetic event > > and > > field names when defining an event. The is_good_name() function is > > used to check that's not possible for the probe events, but should > > also be used for the synthetic events as well. > > > > (tracing: Move is_good_name() from trace_probe.h to trace.h) makes > > that function available to other trace subsystems by putting it in > > trace.h. (tracing: Check that the synthetic event and field names > > are > > legal) applies it to the synthetic events, and (selftests/ftrace: > > Change synthetic event name for inter-event-combined test) changes > > a > > testcase that now fails because it uses an illegal name. > > > > > Hi Tom, > > Would you be able to address Masami's concerns on patches 1 and 4? Yes, I'll submit a v2 fixing those soon (today). Thanks, Tom > > -- Steve
Re: [PATCH] kthread: Add kthread_work tracepoints
On Mon, 12 Oct 2020 08:14:50 -0700 Rob Clark wrote: > > DEFINE_EVENT(sched_kthread_work_execute_template, > > sched_kthread_work_execute_start, > > TP_PROTO(struct kthread_work *work), > > TP_ARGS(work)); > > > > DEFINE_EVENT(sched_kthread_work_execute_template, > > sched_kthread_work_execute_end, > > TP_PROTO(struct kthread_work *work), > > TP_ARGS(work)); > > > > As events are cheap, classes are expensive (size wise), and a TRACE_EVENT() > > is really just a CLASS and EVENT for a single instance. > > I think we cannot quite do this, because we should not rely on being Ah I missed seeing that one used work->func and the other passed in the function. > able to dereference work after it finishes. Although I suppose I > could just define it to explicitly pass the fxn ptr to both > tracepoints.. But yes, I would rather see that. It could save up to 5K in text and data. -- Steve
Re: [PATCH 1/5] tracing: Don't show dynamic string internals in synthetic event description
Hi Masami, On Sun, 2020-10-11 at 00:03 +0900, Masami Hiramatsu wrote: > Hi Tom, > > On Fri, 9 Oct 2020 10:17:07 -0500 > Tom Zanussi wrote: > > > For synthetic event dynamic fields, the type contains "__data_loc", > > which is basically an internal part of the type which is only meant > > to > > be displayed in the format, not in the event description itself, > > which > > is confusing to users since they can't use __data_loc on the > > command-line to define an event field, which printing it would lead > > them to believe. > > > > So filter it out from the description, while leaving it in the > > type. > > > > OK, I confirmed this removes __data_loc from synth_events interface. > However, I also found another issue. > > /sys/kernel/debug/tracing # echo "myevent char str[]; int v" >> > synthetic_events > /sys/kernel/debug/tracing # cat synthetic_events > myevent char[]; str; int v > > It seems that the type "char[]" includes ";" as a type, this results > Yeah, this isn't a result of this patchset - it's a different bug which I'll submit a new fix for. Basically in the array case it doesn't effectively strip off trailing characters when creating the array type. Thanks, Tom > > /sys/kernel/debug/tracing # cat events/synthetic/myevent/format > name: myevent > ID: 1220 > format: > field:unsigned short common_type; offset:0; size:2; signe > d:0; > field:unsigned char common_flags; offset:2; size:1; signe > d:0; > field:unsigned char common_preempt_count; offset:3; size: > 1;signed:0; > field:int common_pid; offset:4; size:4; signed:1; > > field:__data_loc char[]; str; offset:8; size:8; signe > d:1; > field:int v;offset:16; size:4; signed:1; > > print fmt: "str=%.*s, v=%d", __get_str(str), REC->v > > > As you can see, the field type has ";" in format file too. This will > prevent > parsing event information correctly. > I also try to remove ";" as below, it seems to work correctly. > > /sys/kernel/debug/tracing # echo "myevent char[] str; int v" >> > synthetic_events > /sys/kernel/debug/tracing # cat events/synthetic/myevent/format > name: myevent > ID: 1221 > format: > field:unsigned short common_type; offset:0; size:2; signe > d:0; > field:unsigned char common_flags; offset:2; size:1; signe > d:0; > field:unsigned char common_preempt_count; offset:3; size: > 1;signed:0; > field:int common_pid; offset:4; size:4; signed:1; > > field:__data_loc char[] str;offset:8; size:8; signe > d:1; > field:int v;offset:16; size:4; signed:1; > > print fmt: "str=%.*s, v=%d", __get_str(str), REC->v > > > Thank you, > > > Reported-by: Masami Hiramatsu > > Signed-off-by: Tom Zanussi > > --- > > kernel/trace/trace_events_synth.c | 10 +- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/trace/trace_events_synth.c > > b/kernel/trace/trace_events_synth.c > > index 3b2dcc42b8ee..b19e2f4159ab 100644 > > --- a/kernel/trace/trace_events_synth.c > > +++ b/kernel/trace/trace_events_synth.c > > @@ -1867,14 +1867,22 @@ static int __synth_event_show(struct > > seq_file *m, struct synth_event *event) > > { > > struct synth_field *field; > > unsigned int i; > > + char *type, *t; > > > > seq_printf(m, "%s\t", event->name); > > > > for (i = 0; i < event->n_fields; i++) { > > field = event->fields[i]; > > > > + type = field->type; > > + t = strstr(type, "__data_loc"); > > + if (t) { /* __data_loc belongs in format but not event > > desc */ > > + t += sizeof("__data_loc"); > > + type = t; > > + } > > + > > /* parameter values */ > > - seq_printf(m, "%s %s%s", field->type, field->name, > > + seq_printf(m, "%s %s%s", type, field->name, > >i == event->n_fields - 1 ? "" : "; "); > > } > > > > -- > > 2.17.1 > > > >
Re: [PATCH v3 0/2] nvmem: skip nodes with compatibles other than "nvmem-cell"
Hello Rob, Hello Srini, On 5/12/20 4:18 PM, Rob Herring wrote: > On Tue, Apr 28, 2020 at 01:18:25PM +0200, Ahmad Fatoum wrote: >> The nvmem cell binding applies to all objects which match "^.*@[0-9a-f]+$", >> without taking a compatible into account. This precludes extension of e.g. >> eeprom nodes by any child nodes other than nvmem. Consider following example: >> >> eeprom@0 { >> reg = <0 64>; >> #address-cells = <1>; >> #size-cells = <1>; >> >> partitions { >> compatible = "fixed-partitions"; >> #address-cells = <1>; >> #size-cells = <1>; >> bits = <64 64 64>; /* to verify it's skipped */ >> >> part@0 { >> reg = <0x00 16>; >> }; >> }; >> >> no-cell@10 { >> compatible = "not-nvmem-cell"; >> reg = <0x10 4>; >> bits = <64 64 64>; /* to verify it's skipped */ >> }; >> >> cell-old@14 { >> reg = <0x14 0x2>; >> }; >> >> cell-new@16 { >> compatible = "nvmem-cell"; >> reg = <0x16 4>; >> }; >> }; >> >> Without this series, the NVMEM driver interprets all direct children of >> eeprom@0 >> as NVMEM cells and driver probe fails, because the partitions node lacks a >> reg >> property, e.g.: >> >> nvmem 0-0: nvmem: invalid reg on /eeprom@0 >> >> Running dtbs_check on the snippet will skip partitions (it doesn't match >> above >> regex), but will flag no-cell@10 and cell-new@16 as invalid. >> >> With this series applied, the driver will skip partitions and no-cell@10, >> because they have a compatible but it's not "nvmem-cell". > > Because you have to support no compatible (forever), there's no point > adding this compatible. IMO nvmem cells should have had a compatible since the beginning and I figured better late than never. >> Both cell-old@14 and cell-new@16 will be interpreted as cells. >> >> Likewise, running dtbs_check on the snippet will skip partitions (compatible >> doesn't match and regex doesn't either) and no-cell@10, but accept the other >> two. >> >> This series resolves an existing clash between this nvmem-cell binding and >> the barebox bootloader binding that extends the fixed-partitions MTD >> binding to EEPROMs[1]. It's also a building block for getting nvmem cells and >> partitions in MTD devices to co-exist in the same device tree node[2]. > > This violates having multiple nodes at the same address because you are > independently overlaying partitions and nvmem cells on same address > ranges. It also seems seems pretty fragile if you want to update > partitions. > > I think instead, nvmem cells should be contained within a partition. > The partition should then have a compatible to indicate it contains > nvmem cells. I thought I had understood what needs to be done, but now that I finally have time to do it, I see that this only solves the second issue "extending the NVMEM binding to nodes that already have other child nodes, e.g., MTD and its partitions". The first issue: "future extension of e.g. eeprom nodes by any child nodes other than nvmem cells" isn't solved by having a containing partition. My issue is that the bootloader fixes up a partitions { compatible = "fixed-partitions"; } child node into the kernel device tree. The NVMEM core driver tries to parse all eeprom child nodes as cells and will make the driver probe of the EEPROM fail, because it can't parse that fixed-partitions node as a nvmem cell. To allow for co-existence of NVMEM cells and other subnodes, would following patch be acceptable to you and Srini? 8< -- --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml @@ -45,7 +45,15 @@ properties: patternProperties: "^.*@[0-9a-f]+$": type: object - +if: + not: +properties: + compatible: +then: + $ref: "#/definitions/nvmem-cell" + +definitions: + nvmem-cell: properties: reg: maxItems: 1
Re: [PATCH v7 2/2] PCI: dwc: Fix MSI page leakage in suspend/resume
On 10/12/2020 5:07 PM, Robin Murphy wrote: External email: Use caution opening links or attachments On 2020-10-09 08:55, Jisheng Zhang wrote: Currently, dw_pcie_msi_init() allocates and maps page for msi, then program the PCIE_MSI_ADDR_LO and PCIE_MSI_ADDR_HI. The Root Complex may lose power during suspend-to-RAM, so when we resume, we want to redo the latter but not the former. If designware based driver (for example, pcie-tegra194.c) calls dw_pcie_msi_init() in resume path, the msi page will be leaked. As pointed out by Rob and Ard, there's no need to allocate a page for the MSI address, we could use an address in the driver data. To avoid map the MSI msg again during resume, we move the map MSI msg from dw_pcie_msi_init() to dw_pcie_host_init(). You should move the unmap there as well. As soon as you know what the relevant address would be if you *were* to do DMA to this location, then the exercise is complete. Leaving it mapped for the lifetime of the device in order to do not-DMA to it seems questionable (and represents technically incorrect API usage without at least a sync_for_cpu call before any other access to the data). Another point of note is that using streaming DMA mappings at all is a bit fragile (regardless of this change). If the host controller itself has a limited DMA mask relative to physical memory (which integrators still seem to keep doing...) then you could end up punching your MSI hole right in the middle of the SWIOTLB bounce buffer, where it's then almost *guaranteed* to interfere with real DMA :( Agree with Robin. Since the MSI page is going to be locked till shutdown/reboot, wouldn't it make sense to use dma_alloc_coherent() API? Also, shouldn't we call dma_set_mask() to limit the address to only 32-bits so as to enable MSI for even those legacy PCIe devices with only 32-bit MSI capability? - Vidya Sagar If no DWC users have that problem and the current code is working well enough, then I see little reason not to make this partucular change to tidy up the implementation, just bear in mind that there's always the possibility of having to come back and change it yet again in future to make it more robust. I had it in mind that this trick was done with a coherent DMA allocation, which would be safe from addressing problems but would need to be kept around for the lifetime of the device, but maybe that was a different driver :/ Robin. Suggested-by: Rob Herring Signed-off-by: Jisheng Zhang Reviewed-by: Rob Herring --- drivers/pci/controller/dwc/pci-dra7xx.c | 18 +- .../pci/controller/dwc/pcie-designware-host.c | 33 ++- drivers/pci/controller/dwc/pcie-designware.h | 2 +- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c index 8f0b6d644e4b..6d012d2b1e90 100644 --- a/drivers/pci/controller/dwc/pci-dra7xx.c +++ b/drivers/pci/controller/dwc/pci-dra7xx.c @@ -466,7 +466,9 @@ static struct irq_chip dra7xx_pci_msi_bottom_irq_chip = { static int dra7xx_pcie_msi_host_init(struct pcie_port *pp) { struct dw_pcie *pci = to_dw_pcie_from_pp(pp); + struct device *dev = pci->dev; u32 ctrl, num_ctrls; + int ret; pp->msi_irq_chip = _pci_msi_bottom_irq_chip; @@ -482,7 +484,21 @@ static int dra7xx_pcie_msi_host_init(struct pcie_port *pp) ~0); } - return dw_pcie_allocate_domains(pp); + ret = dw_pcie_allocate_domains(pp); + if (ret) + return ret; + + pp->msi_data = dma_map_single_attrs(dev, >msi_msg, + sizeof(pp->msi_msg), + DMA_FROM_DEVICE, + DMA_ATTR_SKIP_CPU_SYNC); + ret = dma_mapping_error(dev, pp->msi_data); + if (ret) { + dev_err(dev, "Failed to map MSI data\n"); + pp->msi_data = 0; + dw_pcie_free_msi(pp); + } + return ret; } static const struct dw_pcie_host_ops dra7xx_pcie_host_ops = { diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index d3e9ea11ce9e..d02c7e74738d 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -266,30 +266,23 @@ void dw_pcie_free_msi(struct pcie_port *pp) irq_domain_remove(pp->msi_domain); irq_domain_remove(pp->irq_domain); - if (pp->msi_page) - __free_page(pp->msi_page); + if (pp->msi_data) { + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); + struct device *dev = pci->dev; + + dma_unmap_single_attrs(dev, pp->msi_data, sizeof(pp->msi_msg), + DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); + } } void dw_pcie_msi_init(struct pcie_port *pp) { struct dw_pcie *pci = to_dw_pcie_from_pp(pp); -
Re: [PATCH] ARM: dts: sun8i: h2+: Enable optional SPI flash on Orange Pi Zero board
On Thu, Oct 08, 2020 at 07:40:44PM +0200, Michal Suchánek wrote: > On Thu, Oct 08, 2020 at 07:14:54PM +0200, Maxime Ripard wrote: > > On Thu, Oct 08, 2020 at 06:02:19PM +0200, Michal Suchánek wrote: > > > On Thu, Oct 08, 2020 at 05:13:15PM +0200, Maxime Ripard wrote: > > > > Hi, > > > > > > > > On Tue, Sep 29, 2020 at 10:30:25AM +0200, Michal Suchanek wrote: > > > > > The flash is present on all new boards and users went out of their way > > > > > to add it on the old ones. > > > > > > > > > > Enabling it makes a more reasonable default. > > > > > > > > > > Signed-off-by: Michal Suchanek > > > > > --- > > > > > arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts | 4 ++-- > > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts > > > > > b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts > > > > > index f19ed981da9d..061d295bbba7 100644 > > > > > --- a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts > > > > > +++ b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts > > > > > @@ -163,8 +163,8 @@ { > > > > > }; > > > > > > > > > > { > > > > > - /* Disable SPI NOR by default: it optional on Orange Pi Zero > > > > > boards */ > > > > > - status = "disabled"; > > > > > + /* Enable optional SPI NOR by default */ > > > > > + status = "okay"; > > > > > > > > > > flash@0 { > > > > > #address-cells = <1>; > > > > > > > > Unfortunately, it's optional, so there's really no reason to enable it > > > > all the time. If it's troublesome to users, then the distros or vendors > > > > should make the changes necessary to the hardware, bootloader or their > > > > documentation to make it easier for those users. > > > > > > I don't understand the reasoning. Why must it be disabled when optional? > > > > Think about it the other way around. If we enable everything that is > > optional, we're going to have a multitude of conflicts everywhere, and > > without a clear decision as to who is "best" and thus how we should > > resolve it. > Conflicts with what? > > The SPI0 bus is routed the the flash memory pads. Either there is the > flash mounted or there are free pads. Nothing else on the board uses > these pins. You could possily solder something else there but that's > definitely not part of the board. > > > > On a separate platform, recently I've been using a VGA bridge for the > > RaspberryPi that takes the UART pins as well. It's definitely optional, > > should I enable it by default? At the same time, enabling by default the > > UART is just as arbitrary and will result in people using the VGA bridge > > to complain about their regression (rightfully so). > > That's completely different situation. That bridge is probably not even > part of the board. > > > > > So, really, if it's optional, it means that it not always there. If it's > > not always there, it's meant to be supported by an overlay. > > > > > By the same reasoning there is no reason to disable it all the time. > > > > I'm not sure I follow you here. The least common denominator is that > > it's not there, so it's not enabled. > > You have two options - have a flash mounted or not. You ask why enable > flash when it is not always present. By the same logic I can ask why > disable it when it is not always absent. Enabling is the more useful > option because it degrades gracefully in the case it is not present. It > does not work the other way around. > > > > > > Also the boards that do not have the flsh are either broken or > > > obsolete. > > > > Making general statements without arguments doesn't really make it true > > though. Plenty of boards to have flash and are neither broken nor > > obsolete. > > Cannot parse this. "Plenty of boards do not have flash and are neither broken nor obsolete" > > > > > So most of the time enabling the flash chip is the right thing. > > > > > > Or do we need two DTBs like sun8i-h2-plus-orangepi-zero.dts and > > > sun8i-h2-plus-orangepi-zero-no-spi-nor.dts > > > > No, you need sun8i-h2-plus-orangepi-zero plus an overlay for the > > SPI-NOR. > > The flash is part of the board. Not always though. > There is no need for an overlay. Overlays are here to deal with the "not always though" situation... > And overlays don't exist. If you want to believe that, please go ahead. But there's support for it in libfdt, and you can either apply them directly through the U-Boot command line, or bundle them in a FIT image. Plenty of support for something that doesn't exist. > > > > > There is no way to change the setting on a runnig system, the pins are > > > routed to the flash pads anyway so are not usable for anything else. The > > > only thing that happens on boards that do not have the flash is kernel > > > probing it and complaining that the ID 00 00 00 is not valid SPI NOR > > > flash memory ID. > > > > We have people reporting bugs about completely innocuous error messages > >
Re: [PATCH v2 1/3] objtool: check: Fully validate the stack frame
On Mon, Oct 12, 2020 at 11:21:49AM +0100, Julien Thierry wrote: > On 9/29/20 8:18 PM, Josh Poimboeuf wrote: > > "Stack frame" has more than one meaning now, I suppose. i.e. it could > > also include the callee-saved registers and any other stack space > > allocated by the function. > > > > Would "call frame" be clearer? > > > >CALL_FRAME_BP_OFFSET > >CALL_FRAME_RA_OFFSET > > > > ? > > I would've thought that the call-frame could include the stackframe + other > callee saved regs. Hm, probably so. > Whereas stackframe tends to used for the caller's frame pointer + > return address (i.e. what allows unwinding). Unless I'm getting lost > with things. I've always seen "stack frame" used to indicate the function's entire stack. > And if call frame is associated with the region starting from the stack > pointer at the parent call point (since this is what CFA is), then it > shouldn't be associated with the framepointer + return address structure > since this could be anywhere on the call frame (not at a fixed offset) as > long as the new frame pointer points to the structure. I suppose "call frame" and "stack frame" probably mean the same thing, in which case neither is appropriate here... In fact, maybe we could forget the concept of a frame (or even a struct) here. If cfa.base is CFI_BP, then is regs[CFI_BP].offset always the same as -cfa.offset? i.e. could the BP checks could it just be a simple regs[CFI_BP].offset == -cfa.offset check? And then is RA at regs[CFI_BP].offset + 8? -- Josh
Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier
On Mon, Oct 12, 2020 at 05:08:30PM +0300, Kirill A. Shutemov wrote: > On Sat, Oct 10, 2020 at 03:26:24PM -0400, Arvind Sankar wrote: > > On Sat, Oct 10, 2020 at 03:11:10PM -0400, Arvind Sankar wrote: > > > Commit > > > ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table") > > > started using a new set of pagetables even without KASLR. > > > > > > After that commit, initialize_identity_maps() is called before the > > > 5-level paging variables are setup in choose_random_location(), which > > > will not work if 5-level paging is actually enabled. > > > > Note that I don't have hardware that supports 5-level paging, so this > > is not actually tested with 5-level, but based on code inspection, it > > shouldn't work. > > qemu supports it. -cpu "qemu64,+la57" > > -- > Kirill A. Shutemov Thanks! On QEMU, it does crash without this patch.
[GIT PULL] core/build changes for v5.10: Add orphan section checking for x86, ARM and ARM64
Linus, Please pull the latest core/build git tree from: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core-build-2020-10-12 # HEAD: 6e0bf0e0e55000742a53c5f3b58f8669e0091a11 x86/boot/compressed: Warn on orphan section placement Orphan link sections were a long-standing source of obscure bugs, because the heuristics that various linkers & compilers use to handle them (include these bits into the output image vs discarding them silently) are both highly idiosyncratic and also version dependent. Instead of this historically problematic mess, this tree by Kees Cook (et al) adds build time asserts and build time warnings if there's any orphan section in the kernel or if a section is not sized as expected. And because we relied on so many silent assumptions in this area, fix a metric ton of dependencies and some outright bugs related to this, before we can finally enable the checks on the x86, ARM and ARM64 platforms. Thanks, Ingo --> Ard Biesheuvel (3): x86/boot/compressed: Move .got.plt entries out of the .got section x86/boot/compressed: Force hidden visibility for all symbol references x86/boot/compressed: Get rid of GOT fixup code Arvind Sankar (4): x86/boot: Add .text.* to setup.ld x86/boot: Remove run-time relocations from .head.text code x86/boot: Remove run-time relocations from head_{32,64}.S x86/boot: Check that there are no run-time relocations Kees Cook (28): vmlinux.lds.h: Create COMMON_DISCARDS vmlinux.lds.h: Add .gnu.version* to COMMON_DISCARDS vmlinux.lds.h: Avoid KASAN and KCSAN's unwanted sections vmlinux.lds.h: Split ELF_DETAILS from STABS_DEBUG vmlinux.lds.h: Add .symtab, .strtab, and .shstrtab to ELF_DETAILS efi/libstub: Disable -mbranch-protection arm64/mm: Remove needless section quotes arm64/kernel: Remove needless Call Frame Information annotations arm64/build: Remove .eh_frame* sections due to unwind tables arm64/build: Use common DISCARDS in linker script arm64/build: Add missing DWARF sections arm64/build: Assert for unwanted sections arm/build: Refactor linker script headers arm/build: Explicitly keep .ARM.attributes sections arm/build: Add missing sections arm/build: Assert for unwanted sections arm/boot: Handle all sections explicitly x86/asm: Avoid generating unused kprobe sections x86/build: Enforce an empty .got.plt section x86/build: Add asserts for unwanted sections x86/boot/compressed: Reorganize zero-size section asserts x86/boot/compressed: Remove, discard, or assert for unwanted sections x86/boot/compressed: Add missing debugging sections to output arm64/build: Warn on orphan section placement arm/build: Warn on orphan section placement arm/boot: Warn on orphan section placement x86/build: Warn on orphan section placement x86/boot/compressed: Warn on orphan section placement Nick Desaulniers (1): vmlinux.lds.h: Add PGO and AutoFDO input sections arch/alpha/kernel/vmlinux.lds.S| 1 + arch/arc/kernel/vmlinux.lds.S | 1 + arch/arm/Makefile | 4 + arch/arm/boot/compressed/Makefile | 2 + arch/arm/boot/compressed/vmlinux.lds.S | 20 +-- arch/arm/{kernel => include/asm}/vmlinux.lds.h | 30 - arch/arm/kernel/vmlinux-xip.lds.S | 8 +- arch/arm/kernel/vmlinux.lds.S | 8 +- arch/arm64/Makefile| 9 +- arch/arm64/kernel/smccc-call.S | 2 - arch/arm64/kernel/vmlinux.lds.S| 28 - arch/arm64/mm/mmu.c| 2 +- arch/csky/kernel/vmlinux.lds.S | 1 + arch/hexagon/kernel/vmlinux.lds.S | 1 + arch/ia64/kernel/vmlinux.lds.S | 1 + arch/mips/kernel/vmlinux.lds.S | 1 + arch/nds32/kernel/vmlinux.lds.S| 1 + arch/nios2/kernel/vmlinux.lds.S| 1 + arch/openrisc/kernel/vmlinux.lds.S | 1 + arch/parisc/boot/compressed/vmlinux.lds.S | 1 + arch/parisc/kernel/vmlinux.lds.S | 1 + arch/powerpc/kernel/vmlinux.lds.S | 2 +- arch/riscv/kernel/vmlinux.lds.S| 1 + arch/s390/kernel/vmlinux.lds.S | 1 + arch/sh/kernel/vmlinux.lds.S | 1 + arch/sparc/kernel/vmlinux.lds.S| 1 + arch/um/kernel/dyn.lds.S | 2 +- arch/um/kernel/uml.lds.S | 2 +- arch/x86/Makefile | 4 + arch/x86/boot/compressed/Makefile | 41 ++ arch/x86/boot/compressed/head_32.S | 99 +-- arch/x86/boot/compressed/head_64.S | 165 ++--- arch/x86/boot/compressed/mkpiggy.c
Re: [PATCH RFC 2/2] kbuild: use interpreters to invoke scripts
On Sun, Oct 4, 2020 at 12:21 AM Ujjwal Kumar wrote: > > We cannot rely on execute bits to be set on files in the repository. > The build script should use the explicit interpreter when invoking any > script from the repository. > > Link: > https://lore.kernel.org/lkml/20200830174409.c24c3f67addcce0cea9a9...@linux-foundation.org/ > Link: https://lore.kernel.org/lkml/202008271102.FEB906C88@keescook/ > > Suggested-by: Andrew Morton > Suggested-by: Kees Cook > Suggested-by: Lukas Bulwahn > Signed-off-by: Ujjwal Kumar > --- > Makefile | 4 ++-- > arch/arm64/kernel/vdso/Makefile | 2 +- > arch/arm64/kernel/vdso32/Makefile | 2 +- > arch/ia64/Makefile| 4 ++-- > arch/nds32/kernel/vdso/Makefile | 2 +- > scripts/Makefile.build| 2 +- > scripts/Makefile.package | 4 ++-- > 7 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/Makefile b/Makefile > index f93dbae71248..5f1399a576d4 100644 > --- a/Makefile > +++ b/Makefile > @@ -1258,7 +1258,7 @@ include/generated/utsrelease.h: > include/config/kernel.release FORCE > PHONY += headerdep > headerdep: > $(Q)find $(srctree)/include/ -name '*.h' | xargs --max-args 1 \ > - $(srctree)/scripts/headerdep.pl -I$(srctree)/include > + $(PERL) $(srctree)/scripts/headerdep.pl -I$(srctree)/include > > # --- > # Kernel headers > @@ -1314,7 +1314,7 @@ PHONY += kselftest-merge > kselftest-merge: > $(if $(wildcard $(objtree)/.config),, $(error No .config exists, > config your kernel first!)) > $(Q)find $(srctree)/tools/testing/selftests -name config | \ > - xargs $(srctree)/scripts/kconfig/merge_config.sh -m > $(objtree)/.config > + xargs $(CONFIG_SHELL) > $(srctree)/scripts/kconfig/merge_config.sh -m $(objtree)/.config > $(Q)$(MAKE) -f $(srctree)/Makefile olddefconfig > > # --- > diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile > index de981f7b4546..30fe93bb5488 100644 > --- a/arch/arm64/kernel/vdso/Makefile > +++ b/arch/arm64/kernel/vdso/Makefile > @@ -65,7 +65,7 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE > # Generate VDSO offsets using helper script > gen-vdsosym := $(srctree)/$(src)/gen_vdso_offsets.sh > quiet_cmd_vdsosym = VDSOSYM $@ > - cmd_vdsosym = $(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ > + cmd_vdsosym = $(NM) $< | $(CONFIG_SHELL) $(gen-vdsosym) | LC_ALL=C > sort > $@ > > include/generated/vdso-offsets.h: $(obj)/vdso.so.dbg FORCE > $(call if_changed,vdsosym) > diff --git a/arch/arm64/kernel/vdso32/Makefile > b/arch/arm64/kernel/vdso32/Makefile > index 572475b7b7ed..4f8fe34bc75a 100644 > --- a/arch/arm64/kernel/vdso32/Makefile > +++ b/arch/arm64/kernel/vdso32/Makefile > @@ -205,7 +205,7 @@ quiet_cmd_vdsomunge = MUNGE $@ > gen-vdsosym := $(srctree)/$(src)/../vdso/gen_vdso_offsets.sh > quiet_cmd_vdsosym = VDSOSYM $@ > # The AArch64 nm should be able to read an AArch32 binary > - cmd_vdsosym = $(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ > + cmd_vdsosym = $(NM) $< | $(CONFIG_SHELL) $(gen-vdsosym) | LC_ALL=C > sort > $@ > > # Install commands for the unstripped file > quiet_cmd_vdso_install = INSTALL32 $@ > diff --git a/arch/ia64/Makefile b/arch/ia64/Makefile > index 2876a7df1b0a..5f6cc3c3da50 100644 > --- a/arch/ia64/Makefile > +++ b/arch/ia64/Makefile > @@ -28,8 +28,8 @@ cflags-y := -pipe $(EXTRA) -ffixed-r13 > -mfixed-range=f12-f15,f32-f127 \ >-falign-functions=32 -frename-registers > -fno-optimize-sibling-calls > KBUILD_CFLAGS_KERNEL := -mconstant-gp > > -GAS_STATUS = $(shell $(srctree)/arch/ia64/scripts/check-gas "$(CC)" > "$(OBJDUMP)") > -KBUILD_CPPFLAGS += $(shell $(srctree)/arch/ia64/scripts/toolchain-flags > "$(CC)" "$(OBJDUMP)" "$(READELF)") > +GAS_STATUS = $($(CONFIG_SHELL) $(srctree)/arch/ia64/scripts/check-gas > "$(CC)" "$(OBJDUMP)") > +KBUILD_CPPFLAGS += $($(CONFIG_SHELL) > $(srctree)/arch/ia64/scripts/toolchain-flags "$(CC)" "$(OBJDUMP)" > "$(READELF)") These changes look wrong to me. $($(CONFIG_SHELL)-> $(shell $(CONFIG_SHELL) > ifeq ($(GAS_STATUS),buggy) > $(error Sorry, you need a newer version of the assember, one that is built > from\ > diff --git a/arch/nds32/kernel/vdso/Makefile b/arch/nds32/kernel/vdso/Makefile > index 55df25ef0057..e77d4bcfa7c1 100644 > --- a/arch/nds32/kernel/vdso/Makefile > +++ b/arch/nds32/kernel/vdso/Makefile > @@ -39,7 +39,7 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE > # Generate VDSO offsets using helper script > gen-vdsosym := $(srctree)/$(src)/gen_vdso_offsets.sh > quiet_cmd_vdsosym = VDSOSYM $@ > - cmd_vdsosym = $(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ > + cmd_vdsosym = $(NM) $< | $(CONFIG_SHELL) $(gen-vdsosym) | LC_ALL=C > sort > $@ > > include/generated/vdso-offsets.h:
Re: [PATCH v2 1/4] dt-bindings: usb: convert usb-device.txt to YAML schema
On Sat, 10 Oct 2020 16:43:11 +0800, Chunfeng Yun wrote: > Convert usb-device.txt to YAML schema usb-device.yaml > > Signed-off-by: Chunfeng Yun > --- > v2: new patch suggested by Rob > --- > .../devicetree/bindings/usb/usb-device.txt| 102 -- > .../devicetree/bindings/usb/usb-device.yaml | 129 ++ > 2 files changed, 129 insertions(+), 102 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/usb/usb-device.txt > create mode 100644 Documentation/devicetree/bindings/usb/usb-device.yaml > My bot found errors running 'make dt_binding_check' on your patch: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/amlogic,meson-gx-ao-cec.example.dt.yaml: cec@100: compatible:0: 'amlogic,meson-gx-ao-cec' does not match '^usb[0-9a-f]+,[0-9a-f]+$' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/amlogic,meson-gx-ao-cec.example.dt.yaml: cec@100: 'compatile' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/st,stm32-cec.example.dt.yaml: cec@40006c00: compatible:0: 'st,stm32-cec' does not match '^usb[0-9a-f]+,[0-9a-f]+$' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/st,stm32-cec.example.dt.yaml: cec@40006c00: 'compatile' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.example.dt.yaml: ec@0: compatible:0: 'google,cros-ec-spi' does not match '^usb[0-9a-f]+,[0-9a-f]+$' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.example.dt.yaml: ec@0: 'compatile' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/regulator/qcom,usb-vbus-regulator.example.dt.yaml: dcdc@1100: compatible:0: 'qcom,pm8150b-vbus-reg' does not match '^usb[0-9a-f]+,[0-9a-f]+$' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/regulator/qcom,usb-vbus-regulator.example.dt.yaml: dcdc@1100: 'compatile' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.example.dt.yaml: ec@0: compatible:0: 'google,cros-ec-spi' does not match '^usb[0-9a-f]+,[0-9a-f]+$' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.example.dt.yaml: ec@0: 'compatile' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/dac/lltc,ltc1660.example.dt.yaml: dac@0: compatible:0: 'lltc,ltc1660' does not match '^usb[0-9a-f]+,[0-9a-f]+$' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/dac/lltc,ltc1660.example.dt.yaml: dac@0: 'compatile' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/dac/st,stm32-dac.example.dt.yaml: dac@40017000: compatible:0: 'st,stm32h7-dac-core' does not match '^usb[0-9a-f]+,[0-9a-f]+$' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/dac/st,stm32-dac.example.dt.yaml: dac@40017000: 'compatile' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/dac/st,stm32-dac.example.dt.yaml: dac@1: compatible:0: 'st,stm32-dac' does not match '^usb[0-9a-f]+,[0-9a-f]+$' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb-device.yaml
Re: linux-next: Tree for Oct 12 (drivers/staging/media/atomisp/)
On 10/12/20 2:59 AM, Stephen Rothwell wrote: > Hi all, > > Changes since 20201009: > on x86_64: In file included from ../drivers/staging/media/atomisp//pci/ia_css_control.h:25:0, from ../drivers/staging/media/atomisp//pci/ia_css.h:28, from ../drivers/staging/media/atomisp//pci/atomisp_compat_css20.h:24, from ../drivers/staging/media/atomisp//pci/atomisp_compat.h:22, from ../drivers/staging/media/atomisp/pci/mmu/sh_mmu_mrfld.c:23: ../drivers/staging/media/atomisp//pci/ia_css_firmware.h:52:29: warning: ‘struct device’ declared inside parameter list will not be visible outside of this definition or declaration ia_css_load_firmware(struct device *dev, const struct ia_css_env *env, ^~ In file included from ../drivers/staging/media/atomisp//pci/ia_css.h:28:0, from ../drivers/staging/media/atomisp//pci/atomisp_compat_css20.h:24, from ../drivers/staging/media/atomisp//pci/atomisp_compat.h:22, from ../drivers/staging/media/atomisp/pci/mmu/sh_mmu_mrfld.c:23: ../drivers/staging/media/atomisp//pci/ia_css_control.h:49:24: warning: ‘struct device’ declared inside parameter list will not be visible outside of this definition or declaration int ia_css_init(struct device *dev, ^~ -- ~Randy Reported-by: Randy Dunlap
Re: [PATCH 4/5] tracing: Add synthetic event error logging
Hi Masami, On Sat, 2020-10-10 at 23:57 +0900, Masami Hiramatsu wrote: > On Fri, 9 Oct 2020 10:17:10 -0500 > Tom Zanussi wrote: > > > Add support for synthetic event error logging, which entails adding > > a > > logging function for it, a way to save the synthetic event command, > > and a set of specific synthetic event parse error strings and > > handling. > > Thanks for fixing this. But it seems that the INVALID_TYPE error > position is not correct. > > /sys/kernel/debug/tracing # echo "myevent chr arg" >> > synthetic_events > sh: write error: Invalid argument > /sys/kernel/debug/tracing # cat error_log > [ 1354.611660] synthetic_events: error: Invalid type > Command: myevent chr arg >^ > > If the type is invalid, it should point "chr" instead of "arg". > If you add a syntax error testcase as same as kprobe_events, you can > check the error position too. > Right, it's using name where it should be using type. Will fix in the next version. Thanks, Tom > Thank you, > > > > > Signed-off-by: Tom Zanussi > > --- > > kernel/trace/trace_events_synth.c | 114 > > +- > > 1 file changed, 112 insertions(+), 2 deletions(-) > > > > diff --git a/kernel/trace/trace_events_synth.c > > b/kernel/trace/trace_events_synth.c > > index 8c9d6e464da0..ce4cae5cfd47 100644 > > --- a/kernel/trace/trace_events_synth.c > > +++ b/kernel/trace/trace_events_synth.c > > @@ -20,6 +20,53 @@ > > > > #include "trace_synth.h" > > > > +#undef ERRORS > > +#define ERRORS \ > > + C(BAD_NAME, "Illegal name"),\ > > + C(CMD_INCOMPLETE, "Incomplete command"), \ > > + C(EVENT_EXISTS, "Event already exists"),\ > > + C(TOO_MANY_FIELDS, "Too many fields"), \ > > + C(INCOMPLETE_TYPE, "Incomplete type"), \ > > + C(INVALID_TYPE, "Invalid type"),\ > > + C(INVALID_FIELD,"Invalid field"), \ > > + C(CMD_TOO_LONG, "Command too long"), > > + > > +#undef C > > +#define C(a, b)SYNTH_ERR_##a > > + > > +enum { ERRORS }; > > + > > +#undef C > > +#define C(a, b)b > > + > > +static const char *err_text[] = { ERRORS }; > > + > > +static char last_cmd[MAX_FILTER_STR_VAL]; > > + > > +static int errpos(const char *str) > > +{ > > + return err_pos(last_cmd, str); > > +} > > + > > +static void last_cmd_set(char *str) > > +{ > > + if (!str) > > + return; > > + > > + strncat(last_cmd, str, MAX_FILTER_STR_VAL - 1); > > +} > > + > > +static void synth_err(u8 err_type, u8 err_pos) > > +{ > > + tracing_log_err(NULL, "synthetic_events", last_cmd, err_text, > > + err_type, err_pos); > > +} > > + > > +static void synth_err_clear(void) > > +{ > > + last_cmd[0] = '\0'; > > +} > > + > > static int create_synth_event(int argc, const char **argv); > > static int synth_event_show(struct seq_file *m, struct dyn_event > > *ev); > > static int synth_event_release(struct dyn_event *ev); > > @@ -545,8 +592,10 @@ static struct synth_field > > *parse_synth_field(int argc, const char **argv, > > field_type++; > > > > if (!strcmp(field_type, "unsigned")) { > > - if (argc < 3) > > + if (argc < 3) { > > + synth_err(SYNTH_ERR_INCOMPLETE_TYPE, > > errpos(field_type)); > > return ERR_PTR(-EINVAL); > > + } > > prefix = "unsigned "; > > field_type = argv[1]; > > field_name = argv[2]; > > @@ -573,6 +622,7 @@ static struct synth_field > > *parse_synth_field(int argc, const char **argv, > > goto free; > > } > > if (!is_good_name(field->name)) { > > + synth_err(SYNTH_ERR_BAD_NAME, errpos(field_name)); > > ret = -EINVAL; > > goto free; > > } > > @@ -601,6 +651,7 @@ static struct synth_field > > *parse_synth_field(int argc, const char **argv, > > > > size = synth_field_size(field->type); > > if (size < 0) { > > + synth_err(SYNTH_ERR_INVALID_TYPE, errpos(field->name)); > > ret = -EINVAL; > > goto free; > > } else if (size == 0) { > > @@ -621,6 +672,7 @@ static struct synth_field > > *parse_synth_field(int argc, const char **argv, > > field->is_dynamic = true; > > size = sizeof(u64); > > } else { > > + synth_err(SYNTH_ERR_INVALID_TYPE, errpos(field- > > >name)); > > ret = -EINVAL; > > goto free; > > } > > @@ -1098,12 +1150,64 @@ int synth_event_gen_cmd_array_start(struct > > dynevent_cmd *cmd, const char *name, > > } > > EXPORT_SYMBOL_GPL(synth_event_gen_cmd_array_start); > > > > +static int cmdstr_append(char *buf, const char *str, int > > *remaining) > > +{ > > + int len = strlen(str); > > + > > + if (len + 1 >= *remaining) { > > +
re: RDMA/rxe: Fix skb lifetime in rxe_rcv_mcast_pkt()
Hi, Static analysis with Coverity has detected a potential issue with the following commit: commit e7ec96fc7932f48a6d6cdd05bf82004a1a04285b Author: Bob Pearson Date: Thu Oct 8 15:36:52 2020 -0500 RDMA/rxe: Fix skb lifetime in rxe_rcv_mcast_pkt() The analysis is as follows: 16. Condition mce->qp_list.next != >qp_list, taking true branch. 269if (mce->qp_list.next != >qp_list) 17. returned_null: skb_clone returns NULL (checked 153 out of 178 times). 18. var_assigned: Assigning: per_qp_skb = NULL return value from skb_clone. 19. Falling through to end of if statement. 270per_qp_skb = skb_clone(skb, GFP_ATOMIC); 271else 272per_qp_skb = skb; 273 274per_qp_pkt = SKB_TO_PKT(per_qp_skb); 275per_qp_pkt->qp = qp; 276rxe_add_ref(qp); Dereference null return value (NULL_RETURNS) 20. dereference: Dereferencing a pointer that might be NULL per_qp_skb when calling rxe_rcv_pkt. 277rxe_rcv_pkt(per_qp_pkt, per_qp_skb); 278} 279 It is possible for skb_clone to return NULL, so dereferencing the NULL return pointer per_qp_skb is a potential issue. Colin
Re: [ANNOUNCE] libtraceevent.git
On Mon, Oct 12, 2020 at 4:19 PM Steven Rostedt wrote: > > On Mon, 12 Oct 2020 12:12:08 +0200 > Jiri Olsa wrote: > > > On Wed, Oct 07, 2020 at 01:07:50PM -0400, Steven Rostedt wrote: > > > I split out tools/lib/traceevent from the kernel tree using "git subtree", > > > which recreates all the commits of a directory and makes that directory a > > > stand alone. I then updated the Makefiles, and copied over some of the > > > header files used to build the library. I pushed this up to: > > > > > > https://git.kernel.org/pub/scm/libs/libtrace/libtraceevent.git/ > > > > > > My hope is that this will now be the source of all updates to the > > > libtraceevent library that can be used as a stand alone package that both > > > perf and tracecmd can use. I would also like powertop and rasdaemon to use > > > this as well. > > > > hi, > > I'm adding this as fedora package, is there a source arhive somewhere > > in git.kernel.org for libtraceevent that spec could download? > > > > Hi Jiri! > > Once it's shown that it works for all the package maintainers, I will tag > it which should create the tarballs automatically on the above link. But I > wanted to fix all the packaging bugs before doing so. I hope this doesn't > make it into a catch-22. Where you can't package till there's a source > tarball, but I can't make a source tarball until I know you can package > it ;-) For Debian I have raised https://bugs.debian.org/971976 but I will package it locally today just to check there is no packaging bugs for our packaging. -- Regards Sudip
KINDEST MESSAGE.
Dear, My name is Mr Tofil Bama, I am the Bill and Exchange assistant Manager in Bank of Africa Ouagadougou Burkina Faso. In my department I discovered an abandoned sum of eighteen million three hundred thousand United State of American dollars (18.3MILLION USA DOLLARS) in an account that belongs to one of our foreign customer (late Mr Shitu Nuri) who died in Ethiopian Airlines Flight 409 that crashed into the Mediterranean Sea on 25th January 2010. Since I got information about his death I have been expecting his next of kin to come over and claim his money because we cannot release it unless somebody applies for it as the next of kin or relation to the deceased as indicated in our banking guidelines, unfortunately we learnt that all his supposed next of kin or relation died alongside with him in the plane crash leaving nobody behind for the claim. It is therefore upon this discovery that I decided to make this business proposal to you and release the money to you as next of kin to the deceased for safety and subsequent disbursement since nobody is coming for the fund, it is 10 years now the money is lying pending in the account of our deceased and I don't want the money to go into the bank treasury as unclaimed bill. You will be entitled with 40% of the total sum while 60% will be for me after which I will visit your Country to invest my own share when the fund is successfully transferred into your account, Please I would like you to keep this transaction confidential and as a top secret between me and you until we successfully achieve this golden opportunity. Yours sincerely, Mr Tofil Bama.
Re: [PATCH] drm/panel: rm68200: fix mode to 50fps
On 9/25/20 4:16 PM, Yannick Fertre wrote: > Compute new timings to get a framerate of 50fps with a pixel clock > @54Mhz. > > Signed-off-by: Yannick Fertre > --- > drivers/gpu/drm/panel/panel-raydium-rm68200.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-raydium-rm68200.c > b/drivers/gpu/drm/panel/panel-raydium-rm68200.c > index 2b9e48b0a491..412c0dbcb2b6 100644 > --- a/drivers/gpu/drm/panel/panel-raydium-rm68200.c > +++ b/drivers/gpu/drm/panel/panel-raydium-rm68200.c > @@ -82,15 +82,15 @@ struct rm68200 { > }; > > static const struct drm_display_mode default_mode = { > - .clock = 52582, > + .clock = 54000, > .hdisplay = 720, > - .hsync_start = 720 + 38, > - .hsync_end = 720 + 38 + 8, > - .htotal = 720 + 38 + 8 + 38, > + .hsync_start = 720 + 48, > + .hsync_end = 720 + 48 + 9, > + .htotal = 720 + 48 + 9 + 48, > .vdisplay = 1280, > .vsync_start = 1280 + 12, > - .vsync_end = 1280 + 12 + 4, > - .vtotal = 1280 + 12 + 4 + 12, > + .vsync_end = 1280 + 12 + 5, > + .vtotal = 1280 + 12 + 5 + 12, > .flags = 0, > .width_mm = 68, > .height_mm = 122, > Hi Yannick, Tested-by: Philippe Cornu Thank you, Philippe
Re: [PATCH 08/13] m68k: m68328: use legacy_timer_tick()
On Mon, Oct 12, 2020 at 3:15 PM Geert Uytterhoeven wrote: > > Given this feature is SoC-specific, not platform-specific, perhaps > it makes sense to move the selects to the M68{,EZ,VZ}328 symbols? > > Regardless: > Reviewed-by: Geert Uytterhoeven Ok, folded in the change blow, using one less line. I couldn't figure out whether it should just be part of the CONFIG_M68000 instead, which doesn't appear to have any machine support by itself. The dragonball CPU configuration looks really odd, because you have to build exactly one of M68{,EZ,VZ}328 into the kernel to get a successful compilation, while Kconfig allows many other combinations. Arnd diff --git a/arch/m68k/Kconfig.cpu b/arch/m68k/Kconfig.cpu index 322a35ef14c6..648054d4f860 100644 --- a/arch/m68k/Kconfig.cpu +++ b/arch/m68k/Kconfig.cpu @@ -104,6 +104,7 @@ config M68060 config M68328 bool "MC68328" depends on !MMU + select LEGACY_TIMER_TICK select M68000 help Motorola 68328 processor support. @@ -111,6 +112,7 @@ config M68328 config M68EZ328 bool "MC68EZ328" depends on !MMU + select LEGACY_TIMER_TICK select M68000 help Motorola 68EX328 processor support. @@ -118,6 +120,7 @@ config M68EZ328 config M68VZ328 bool "MC68VZ328" depends on !MMU + select LEGACY_TIMER_TICK select M68000 help Motorola 68VZ328 processor support. diff --git a/arch/m68k/Kconfig.machine b/arch/m68k/Kconfig.machine index 0ff9338b958e..e3c835440d9a 100644 --- a/arch/m68k/Kconfig.machine +++ b/arch/m68k/Kconfig.machine @@ -146,7 +146,6 @@ config PILOT config PILOT3 bool "Pilot 1000/5000, PalmPilot Personal/Pro, or PalmIII support" depends on M68328 - select LEGACY_TIMER_TICK select PILOT help Support for the Palm Pilot 1000/5000, Personal/Pro and PalmIII. @@ -160,21 +159,18 @@ config XCOPILOT_BUGS config UCSIMM bool "uCsimm module support" depends on M68EZ328 - select LEGACY_TIMER_TICK help Support for the Arcturus Networks uCsimm module. config UCDIMM bool "uDsimm module support" depends on M68VZ328 - select LEGACY_TIMER_TICK help Support for the Arcturus Networks uDsimm module. config DRAGEN2 bool "DragenEngine II board support" depends on M68VZ328 - select LEGACY_TIMER_TICK help Support for the DragenEngine II board.
Re: [PATCH 15/18] dt-bindings: usb: meson-g12a-usb: Discard FL-adj property
On 12/10/2020 17:13, Serge Semin wrote: > On Mon, Oct 12, 2020 at 05:01:43PM +0200, Neil Armstrong wrote: >> Hi, >> >> On 12/10/2020 16:22, Serge Semin wrote: >>> On Mon, Oct 12, 2020 at 09:54:25AM +0200, Neil Armstrong wrote: Hi, On 11/10/2020 00:41, Serge Semin wrote: > An empty snps,quirk-frame-length-adjustment won't cause any change > performed by the driver. Moreover the DT schema validation will fail, > since it expects the property being assigned with some value. So just > discard the property declaration then from the example. > > Signed-off-by: Serge Semin > > --- > > Note the same problem is in the DT source file > arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi . > --- > .../devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml | 1 - > 1 file changed, 1 deletion(-) > > diff --git > a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml > b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml > index 5b04a7dfa018..88184d7e26cc 100644 > --- > a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml > +++ > b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml > @@ -209,6 +209,5 @@ examples: >interrupts = <30>; >dr_mode = "host"; >snps,dis_u2_susphy_quirk; > - snps,quirk-frame-length-adjustment; >}; > }; > >>> Thanks for reporting this, actually the fladj must be 0x20 on this hw, but we do set this on the PHY side, so we can let the dwc3 side 0 here. >>> >>> I can convert this patch to initializing the >>> "snps,quirk-frame-length-adjustment" >>> property with 0x20 value instead. Since most likely I'll have to send a >>> v2/v3/etc >>> of this patchset, that modification won't be too much work to do. What do >>> you think? >> > >> Yes, do this please, > > Ok. Shall I preserve your Acked-by tag in the new patch or you'd prefer to > review it first? Yes, preserve it, Thanks, Neil >> anyway it's only an example so it's ok. > > Actually examples are also validated by "make dt_binding_check". That's why I > had to fix the amlogic,meson-g12a-usb-ctrl example for at least so the new > snps,dwc3.yaml DT schema wouldn't break that full DT bindings > validation procedure.) > > -Sergey > >> >>> >>> Anyway please note, that I've fixed the improper property usage in the DT >>> schema >>> example only. "snps,quirk-frame-length-adjustment" defined as boolean still >>> persists in the DTS file: arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi >>> . >>> So if you ever try to validate that dts file with "make dtbs_check" >>> scenario, it >>> will fail. >> >> Yes, I'll push a fix to pass the dtbs_check when this is merged. >> >> Thanks, >> Neil >> >>> >>> -Sergey >>> Acked-by: Neil Armstrong Neil >>
Re: [PATCHv3] selftests: rtnetlink: load fou module for kci_test_encap_fou() test
On Mon, 12 Oct 2020 13:56:15 +0800 Po-Hsu Lin wrote: > Is there any update on this patch? You received feedback. Don't remove modules after tests, something else could be using them.
[PATCH v2 2/2] dt-bindings: extcon: add binding for TUSB320
Add a device tree binding for the TI TUSB320. Signed-off-by: Michael Auchter --- Changes since v1: - use tusb320 instead of extcon in the unit name .../bindings/extcon/extcon-usbc-tusb320.yaml | 41 +++ 1 file changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usbc-tusb320.yaml diff --git a/Documentation/devicetree/bindings/extcon/extcon-usbc-tusb320.yaml b/Documentation/devicetree/bindings/extcon/extcon-usbc-tusb320.yaml new file mode 100644 index ..9875b4d5c356 --- /dev/null +++ b/Documentation/devicetree/bindings/extcon/extcon-usbc-tusb320.yaml @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/extcon/extcon-usbc-tusb320.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TI TUSB320 USB Type-C CC Logic controller + +maintainers: + - Michael Auchter + +properties: + compatible: +const: ti,tusb320 + + reg: +maxItems: 1 + + interrupts: +maxItems: 1 + +required: + - compatible + - reg + - interrupts + +additionalProperties: false + +examples: + - | +i2c0 { +#address-cells = <1>; +#size-cells = <0>; +tusb320@61 { +compatible = "ti,tusb320"; +reg = <0x61>; +interrupt-parent = <>; +interrupts = <27 1>; +}; +}; +... -- 2.25.4
Re: [PATCH] perf c2c: Update usage for showing memory events
On Mon, Oct 12, 2020 at 2:13 AM Jiri Olsa wrote: > > On Sun, Oct 11, 2020 at 08:10:22PM +0800, Leo Yan wrote: > > Since commit b027cc6fdf1b ("perf c2c: Fix 'perf c2c record -e list' to > > show the default events used"), "perf c2c" tool can show the memory > > events properly, it's no reason to still suggest user to use the > > command "perf mem record -e list" for showing events. > > > > This patch updates the usage for showing memory events with command > > "perf c2c record -e list". > > > > Signed-off-by: Leo Yan > > Acked-by: Jiri Olsa > > thanks, > jirka Acked-by: Ian Rogers Thanks, Ian > > --- > > tools/perf/builtin-c2c.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c > > index 5938b100eaf4..57bb6cce43e3 100644 > > --- a/tools/perf/builtin-c2c.c > > +++ b/tools/perf/builtin-c2c.c > > @@ -2916,7 +2916,7 @@ static int perf_c2c__record(int argc, const char > > **argv) > > bool event_set = false; > > struct option options[] = { > > OPT_CALLBACK('e', "event", _set, "event", > > - "event selector. Use 'perf mem record -e list' to list > > available events", > > + "event selector. Use 'perf c2c record -e list' to list > > available events", > >parse_record_events), > > OPT_BOOLEAN('u', "all-user", _user, "collect only user level > > data"), > > OPT_BOOLEAN('k', "all-kernel", _kernel, "collect only kernel > > level data"), > > -- > > 2.17.1 > > >
Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()
On Sat, Oct 10, 2020 at 10:12 AM Nicolas Saenz Julienne wrote: > > The function provides the CPU physical address addressable by the most > constrained bus in the system. It might be useful in order to > dynamically set up memory zones during boot. > > Signed-off-by: Nicolas Saenz Julienne > --- > drivers/of/address.c | 34 ++ > include/linux/of.h | 7 +++ > 2 files changed, 41 insertions(+) > > diff --git a/drivers/of/address.c b/drivers/of/address.c > index eb9ab4f1e80b..755e97b65096 100644 > --- a/drivers/of/address.c > +++ b/drivers/of/address.c > @@ -1024,6 +1024,40 @@ int of_dma_get_range(struct device_node *np, const > struct bus_dma_region **map) > } > #endif /* CONFIG_HAS_DMA */ > > +/** > + * of_dma_safe_phys_limit - Get system wide DMA safe address space > + * > + * Gets the CPU physical address limit for safe DMA addressing system wide by > + * searching for the most constraining dma-range. Otherwise it returns ~0ULL. > + */ > +u64 __init of_dma_safe_phys_limit(void) > +{ > + struct device_node *np = NULL; > + struct of_range_parser parser; > + const __be32 *ranges = NULL; > + u64 phys_dma_limit = ~0ULL; > + struct of_range range; > + int len; > + > + for_each_of_allnodes(np) { > + dma_addr_t cpu_end = 0; > + > + ranges = of_get_property(np, "dma-ranges", ); > + if (!ranges || !len) > + continue; > + > + of_dma_range_parser_init(, np); > + for_each_of_range(, ) > + if (range.cpu_addr + range.size > cpu_end) > + cpu_end = range.cpu_addr + range.size; This doesn't work if you have more than one level of dma-ranges. The address has to be translated first. It should be okay to do that on the start or end address (if not, your DT is broken). Please add/extend a unittest for this. > + > + if (phys_dma_limit > cpu_end) > + phys_dma_limit = cpu_end; > + } > + > + return phys_dma_limit; > +} > + > /** > * of_dma_is_coherent - Check if device is coherent > * @np:device node > diff --git a/include/linux/of.h b/include/linux/of.h > index 481ec0467285..958c64cffa92 100644 > --- a/include/linux/of.h > +++ b/include/linux/of.h > @@ -558,6 +558,8 @@ int of_map_id(struct device_node *np, u32 id, >const char *map_name, const char *map_mask_name, >struct device_node **target, u32 *id_out); > > +u64 of_dma_safe_phys_limit(void); > + > #else /* CONFIG_OF */ > > static inline void of_core_init(void) > @@ -995,6 +997,11 @@ static inline int of_map_id(struct device_node *np, u32 > id, > return -EINVAL; > } > > +static inline u64 of_dma_safe_phys_limit(void) > +{ > + return ~0ULL; > +} > + > #define of_match_ptr(_ptr) NULL > #define of_match_node(_matches, _node) NULL > #endif /* CONFIG_OF */ > -- > 2.28.0 >
[PATCH v3] kcov, usb: specify contexts for remote coverage sections
Currently there's a KCOV remote coverage collection section in __usb_hcd_giveback_urb(). Initially that section was added based on the assumption that usb_hcd_giveback_urb() can only be called in interrupt context as indicated by a comment before it. This is what happens when syzkaller is fuzzing the USB stack via the dummy_hcd driver. As it turns out, it's actually valid to call usb_hcd_giveback_urb() in task context, provided that the caller turned off the interrupts; USB/IP does exactly that. This can lead to a nested KCOV remote coverage collection sections both trying to collect coverage in task context. This isn't supported by KCOV, and leads to a WARNING. The approach this patch takes is to add another set of kcov_remote_*() callbacks that specify the context they are supposed to be executed in. If the current context doesn't match the mask provided to a callback, that callback is ignored. KCOV currently only supports collecting remote coverage in two contexts: task and softirq. This patch constraints KCOV to only collect coverage from __usb_hcd_giveback_urb() when it's executed in softirq context. As the result, the coverage from USB/IP related usb_hcd_giveback_urb() calls won't be collected, but the WARNING is fixed. A potential future improvement would be to support nested remote coverage collection sections, but this patch doesn't address that. Signed-off-by: Andrey Konovalov --- Changes v2->v3: - Keep behavoir of existing callbacks the same except for the __usb_hcd_giveback_urb() one. - Fix build error with KOV disabled. --- Documentation/dev-tools/kcov.rst | 6 ++ drivers/usb/core/hcd.c | 4 ++-- include/linux/kcov.h | 31 +-- kernel/kcov.c| 26 +++--- 4 files changed, 56 insertions(+), 11 deletions(-) diff --git a/Documentation/dev-tools/kcov.rst b/Documentation/dev-tools/kcov.rst index 8548b0b04e43..2c0f58988512 100644 --- a/Documentation/dev-tools/kcov.rst +++ b/Documentation/dev-tools/kcov.rst @@ -235,6 +235,12 @@ saved to the kcov_handle field in the current task_struct and needs to be passed to the newly spawned threads via custom annotations. Those threads should in turn be annotated with kcov_remote_start()/kcov_remote_stop(). +Besides the annotations that only accept a handle, there are also +kcov_remote_start_context()/kcov_remote_stop_context() that accept a +context mask. This mask describes the contexts in which these annotations +should be applied. E.g. specifying KCOV_CONTEXT_SOFTIRQ will result in the +corresponding annotations being ignored in any context other than softirq. + Internally kcov stores handles as u64 integers. The top byte of a handle is used to denote the id of a subsystem that this handle belongs to, and the lower 4 bytes are used to denote the id of a thread instance within diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index a33b849e8beb..ea93d9ebcb2e 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1646,9 +1646,9 @@ static void __usb_hcd_giveback_urb(struct urb *urb) /* pass ownership to the completion handler */ urb->status = status; - kcov_remote_start_usb((u64)urb->dev->bus->busnum); + kcov_remote_start_usb_softirq((u64)urb->dev->bus->busnum); urb->complete(urb); - kcov_remote_stop(); + kcov_remote_stop_softirq(); usb_anchor_resume_wakeups(anchor); atomic_dec(>use_count); diff --git a/include/linux/kcov.h b/include/linux/kcov.h index a10e84707d82..9e31b71ee3f9 100644 --- a/include/linux/kcov.h +++ b/include/linux/kcov.h @@ -22,6 +22,10 @@ enum kcov_mode { KCOV_MODE_TRACE_CMP = 3, }; +#define KCOV_CONTEXT_TASK (1u << 0) +#define KCOV_CONTEXT_SOFTIRQ (1u << 1) +#define KCOV_CONTEXT_MASK (KCOV_CONTEXT_TASK | KCOV_CONTEXT_SOFTIRQ) + #define KCOV_IN_CTXSW (1 << 30) void kcov_task_init(struct task_struct *t); @@ -38,10 +42,21 @@ do {\ } while (0) /* See Documentation/dev-tools/kcov.rst for usage details. */ -void kcov_remote_start(u64 handle); -void kcov_remote_stop(void); + +void kcov_remote_start_context(u64 handle, unsigned int context); +void kcov_remote_stop_context(unsigned int context); u64 kcov_common_handle(void); +static inline void kcov_remote_start(u64 handle) +{ + return kcov_remote_start_context(handle, KCOV_CONTEXT_MASK); +} + +static inline void kcov_remote_stop(void) +{ + return kcov_remote_stop_context(KCOV_CONTEXT_MASK); +} + static inline void kcov_remote_start_common(u64 id) { kcov_remote_start(kcov_remote_handle(KCOV_SUBSYSTEM_COMMON, id)); @@ -52,6 +67,16 @@ static inline void kcov_remote_start_usb(u64 id) kcov_remote_start(kcov_remote_handle(KCOV_SUBSYSTEM_USB, id)); } +static inline void kcov_remote_start_usb_softirq(u64 id) +{ + kcov_remote_start_context(kcov_remote_handle(KCOV_SUBSYSTEM_USB, id),
Re: [linux-safety] [PATCH] usb: host: ehci-sched: add comment about find_tt() not returning error
Hi Lukas, On Mon, Oct 12, 2020 at 3:11 PM Lukas Bulwahn wrote: > > > > On Sun, 11 Oct 2020, Sudip Mukherjee wrote: > > > Add a comment explaining why find_tt() will not return error even though > > find_tt() is checking for NULL and other errors. > > > > Signed-off-by: Sudip Mukherjee > > I get the intent of the comment but there is a large risk that somebody > could remove the find_tt() call before the calling the function and there > is no chance to then see later that the comment is actually wrong. Removing the find_tt() will mean a major rework of the code. > > I guess you want to link this comment here to a code line above and > request anyone touching the line above to reconsider the comment then. > > But there is no 'concept' for such kind of requests to changes and > comments. > > So, the comment is true now, but might be true or wrong later. If it is wrong later due to some code change then I guess someone will need to send a patch to correct the comment. > > I am wondering if such comment deserves to be included if we cannot check > its validity later... I am failing to understand why will you not be able to check its validity later. You just need to read the code to check it. > > I would prefer a simple tool that could check that your basic assumption > on the code is valid and if it the basic assumption is still valid, > just shut up any stupid tool that simply does not get that these calls > here cannot return any error. > > We encountered this issue because of clang analyzer complaining, but it is > clear that it is a false positive of that (smart but) incomplete tool. I dont think it is a false positive. The error return value is not checked and that is true. Only that it is not applicable because of the way the coding is done. > > Do you intend to add comment for all false positives from all tools or are > we going to find a better solution than that? I think tools will always give you some false positives and you will need to read the code to know if its false positive or not. I dont think there is any tool yet which will only give true positives. -- Regards Sudip
Re: [RFC PATCH] checkpatch: add shebang check to EXECUTE_PERMISSIONS
On Mon, 2020-10-12 at 16:16 +0200, Lukas Bulwahn wrote: > On Mon, 12 Oct 2020, Ujjwal Kumar wrote: > > On 12/10/20 11:47 am, Joe Perches wrote: > > > On Mon, 2020-10-12 at 11:19 +0530, Ujjwal Kumar wrote: > > > > checkpatch.pl checks for invalid EXECUTE_PERMISSIONS on source > > > > files. The script leverages filename extensions and its path in > > > > the repository to decide whether to allow execute permissions on > > > > the file or not. > > > > > > > > Based on current check conditions, a perl script file having > > > > execute permissions, without '.pl' extension in its filename > > > > and not belonging to 'scripts/' directory is reported as ERROR > > > > which is a false-positive. > > > > > > > > Adding a shebang check along with current conditions will make > > > > the check more generalised and improve checkpatch reports. > > > > To do so, without breaking the core design decision of checkpatch, > > > > we can fetch the first line from the patch itself and match it for > > > > a shebang pattern. > > > > > > > > There can be cases where the first line is not part of the patch. > > > > > > For instance: a patch that only changes permissions > > > without changing any of the file content. [] > > Should these new changes go as a separate patch or can they be > > included in the next iteration of this patch? [] The commit log should be updated with the example shown. Please send a clean V2.
Re: [v4 PATCH 0/2] fix scrolling of panel with small hfp or hbp
Hi, Jitao: Jitao Shi 於 2020年10月10日 週六 下午3:09寫道: > > Changes since v3: > - Revert v2, for v2 will cause some bridge ic no output. the cause >the video linetime doesn't match display mode from get mode. > - Make sure the horizontal_frontporch_byte and horizontal_backporch_byte >are > 0. Because v2 is merged into mainline, I think you should merge 1/2 and 2/2 to one patch which fix the problem caused by v2. Regards, Chun-Kuang. > > Jitao Shi (2): > Revert "drm/mediatek: dsi: Fix scrolling of panel with small hfp or > hbp" > drm/mediatek: dsi: fix scrolling of panel with small hfp or hbp > > drivers/gpu/drm/mediatek/mtk_dsi.c | 65 > +++--- > 1 file changed, 25 insertions(+), 40 deletions(-) > > -- > 2.12.5 > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] mm: Make allocator take care of memoryless numa node
On Mon, 12 Oct 2020, Xianting Tian wrote: > In architecture like powerpc, we can have cpus without any local memory > attached to it. In such cases the node does not have real memory. > > In many places of current kernel code, it doesn't judge whether the node is > memoryless numa node before calling allocator interface. That is intentional. SLUB relies on the page allocator to pick a node. > This patch is to use local_memory_node(), which is guaranteed to have > memory, in allocator interface. local_memory_node() is a noop in other > architectures that don't support memoryless nodes. The patch would destroy the support for memory policies in the SLUB allocator and likely in the other ones as well.
[GIT PULL] EFI changes for v5.10
Linus, Please pull the latest efi/core git tree from: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git efi-core-2020-10-12 # HEAD: 4d0a4388ccdd9482fef6b26f879d0f6099143f80 Merge branch 'efi/urgent' into efi/core, to pick up fixes EFI changes for v5.10: - Preliminary RISC-V enablement - the bulk of it will arrive via the RISCV tree. - Relax decompressed image placement rules for 32-bit ARM - Add support for passing MOK certificate table contents via a config table rather than a EFI variable. - Add support for 18 bit DIMM row IDs in the CPER records. - Work around broken Dell firmware that passes the entire Boot variable contents as the command line - Add definition of the EFI_MEMORY_CPU_CRYPTO memory attribute so we can identify it in the memory map listings. - Don't abort the boot on arm64 if the EFI RNG protocol is available but returns with an error - Replace slashes with exclamation marks in efivarfs file names - Split efi-pstore from the deprecated efivars sysfs code, so we can disable the latter on !x86. - Misc fixes, cleanups and updates. Thanks, Ingo --> Alex Kluver (2): edac,ghes,cper: Add Row Extension to Memory Error Record cper,edac,efi: Memory Error Record: bank group/address and chip id Ard Biesheuvel (13): efi/libstub: arm32: Base FDT and initrd placement on image address efi/libstub: Export efi_low_alloc_above() to other units efi/libstub: arm32: Use low allocation for the uncompressed kernel efi: Add definition of EFI_MEMORY_CPU_CRYPTO and ability to report it efi/arm64: libstub: Deal gracefully with EFI_RNG_PROTOCOL failure efi: mokvar-table: fix some issues in new code efi: pstore: disentangle from deprecated efivars module efi: pstore: move workqueue handling out of efivars efi: efivars: un-export efivars_sysfs_init() efi: gsmi: fix false dependency on CONFIG_EFI_VARS efi: remove some false dependencies on CONFIG_EFI_VARS efi: efivars: limit availability to X86 builds efi: mokvar: add missing include of asm/early_ioremap.h Arvind Sankar (2): efi/libstub: Add efi_warn and *_once logging helpers efi/x86: Add a quirk to support command line arguments on Dell EFI firmware Atish Patra (2): include: pe.h: Add RISC-V related PE definition efi: Rename arm-init to efi-init common for all arch Lenny Szubowicz (3): efi: Support for MOK variable config table integrity: Move import of MokListRT certs to a separate routine integrity: Load certs from the EFI MOK config table Michael Schaller (1): efivarfs: Replace invalid slashes with exclamation marks in dentries. Tian Tao (3): efi/printf: remove unneeded semicolon efi/libstub: Fix missing-prototypes in string.c efi: Delete deprecated parameter comments Documentation/arm/uefi.rst | 2 +- arch/arm/include/asm/efi.h | 23 +- arch/arm64/include/asm/efi.h| 5 +- arch/x86/kernel/setup.c | 1 + arch/x86/platform/efi/efi.c | 3 + drivers/edac/ghes_edac.c| 17 +- drivers/firmware/efi/Kconfig| 18 +- drivers/firmware/efi/Makefile | 3 +- drivers/firmware/efi/cper.c | 18 +- drivers/firmware/efi/{arm-init.c => efi-init.c} | 1 + drivers/firmware/efi/efi-pstore.c | 83 +- drivers/firmware/efi/efi.c | 53 ++-- drivers/firmware/efi/efivars.c | 45 +-- drivers/firmware/efi/libstub/arm32-stub.c | 178 +++- drivers/firmware/efi/libstub/arm64-stub.c | 9 +- drivers/firmware/efi/libstub/efi-stub-helper.c | 101 ++- drivers/firmware/efi/libstub/efi-stub.c | 48 +--- drivers/firmware/efi/libstub/efistub.h | 61 +++- drivers/firmware/efi/libstub/fdt.c | 4 +- drivers/firmware/efi/libstub/file.c | 5 +- drivers/firmware/efi/libstub/relocate.c | 4 +- drivers/firmware/efi/libstub/string.c | 1 + drivers/firmware/efi/libstub/vsprintf.c | 2 +- drivers/firmware/efi/mokvar-table.c | 359 drivers/firmware/efi/vars.c | 22 -- drivers/firmware/google/Kconfig | 2 +- drivers/firmware/google/gsmi.c | 8 +- fs/efivarfs/super.c | 3 + include/linux/cper.h| 24 +- include/linux/efi.h | 46 ++- include/linux/pe.h | 3 + security/integrity/platform_certs/load_uefi.c | 85 -- 32 files changed, 871 insertions(+), 366 deletions(-) rename drivers/firmware/efi/{arm-init.c => efi-init.c} (99%) create mode 100644 drivers/firmware/efi/mokvar-table.c
Re: [ANNOUNCE] libtraceevent.git
On Mon, 12 Oct 2020 12:12:08 +0200 Jiri Olsa wrote: > On Wed, Oct 07, 2020 at 01:07:50PM -0400, Steven Rostedt wrote: > > I split out tools/lib/traceevent from the kernel tree using "git subtree", > > which recreates all the commits of a directory and makes that directory a > > stand alone. I then updated the Makefiles, and copied over some of the > > header files used to build the library. I pushed this up to: > > > > https://git.kernel.org/pub/scm/libs/libtrace/libtraceevent.git/ > > > > My hope is that this will now be the source of all updates to the > > libtraceevent library that can be used as a stand alone package that both > > perf and tracecmd can use. I would also like powertop and rasdaemon to use > > this as well. > > hi, > I'm adding this as fedora package, is there a source arhive somewhere > in git.kernel.org for libtraceevent that spec could download? > Hi Jiri! Once it's shown that it works for all the package maintainers, I will tag it which should create the tarballs automatically on the above link. But I wanted to fix all the packaging bugs before doing so. I hope this doesn't make it into a catch-22. Where you can't package till there's a source tarball, but I can't make a source tarball until I know you can package it ;-) -- Steve
[PATCH] checkpatch: improve EXECUTE_PERMISSIONS tests
1. Use group capture regexp for file mode test to improve code readability. 2. The 'scripts/' directory test on filenames can be excluded as it has become obsolete because there are many source files that are not scripts in this directory and its subdirectories. 3. Replace unnecessary group capture regexp with non-capturing group. Suggested-by: Joe Perches Signed-off-by: Ujjwal Kumar --- scripts/checkpatch.pl | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index fab38b493cef..aa8417b5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2678,10 +2678,11 @@ sub process { } # Check for incorrect file permissions - if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) { + if ($line =~ /^new (?:file )?mode (\d+)$/) { + my $mode = substr($1, -3); my $permhere = $here . "FILE: $realfile\n"; - if ($realfile !~ m@scripts/@ && - $realfile !~ /\.(py|pl|awk|sh)$/) { + if ($mode =~ /[1357]/ && + $realfile !~ /\.(?:py|pl|awk|sh)$/) { ERROR("EXECUTE_PERMISSIONS", "do not set execute permissions for source files\n" . $permhere); } base-commit: d67bc7812221606e1886620a357b13f906814af7 -- 2.26.2
Re: [linux-safety] [PATCH] usb: host: ehci-sched: add comment about find_tt() not returning error
On Mon, Oct 12, 2020 at 05:10:21PM +0200, Lukas Bulwahn wrote: > And for the static analysis finding, we need to find a way to ignore this > finding without simply ignoring all findings or new findings that just > look very similar to the original finding, but which are valid. Then I suggest you fix the tool that "flagged" this, surely this is not the only thing it detected with a test like this, right? What tool reported this? thanks, greg k-h
Re: [PATCH] net: ethernet: ixgbe: don't propagate -ENODEV from ixgbe_mii_bus_init()
On Mon, 12 Oct 2020 14:20:16 +0200 Bartosz Golaszewski wrote: > On Mon, Sep 28, 2020 at 9:17 AM Bartosz Golaszewski wrote: > > > > From: Bartosz Golaszewski > > > > It's a valid use-case for ixgbe_mii_bus_init() to return -ENODEV - we > > still want to finalize the registration of the ixgbe device. Check the > > error code and don't bail out if err == -ENODEV. > > > > This fixes an issue on C3000 family of SoCs where four ixgbe devices > > share a single MDIO bus and ixgbe_mii_bus_init() returns -ENODEV for > > three of them but we still want to register them. > > > > Fixes: 09ef193fef7e ("net: ethernet: ixgbe: check the return value of > > ixgbe_mii_bus_init()") > > Reported-by: Yongxin Liu > > Signed-off-by: Bartosz Golaszewski > > --- > > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > index 2f8a4cfc5fa1..d1623af30125 100644 > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > @@ -11032,7 +11032,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const > > struct pci_device_id *ent) > > true); > > > > err = ixgbe_mii_bus_init(hw); > > - if (err) > > + if (err && err != -ENODEV) > > goto err_netdev; > > > > return 0; > > Gentle ping for this patch. Who's picking up networking patches now > that David is OoO? Should I Cc someone else? Intel went through a maintainer change of its own, and they usually pick up their patches and send a PR. Tony, do you want me to apply this directly?
Re: [PATCH] kthread: Add kthread_work tracepoints
On Mon, Oct 12, 2020 at 6:59 AM Steven Rostedt wrote: > > On Sat, 10 Oct 2020 11:03:22 -0700 > Rob Clark wrote: > > > /** > > + * sched_kthread_work_execute_start - called immediately before the work > > callback > > + * @work:pointer to struct kthread_work > > + * > > + * Allows to track kthread work execution. > > + */ > > +TRACE_EVENT(sched_kthread_work_execute_start, > > + > > + TP_PROTO(struct kthread_work *work), > > + > > + TP_ARGS(work), > > + > > + TP_STRUCT__entry( > > + __field( void *,work) > > + __field( void *,function) > > + ), > > + > > + TP_fast_assign( > > + __entry->work = work; > > + __entry->function = work->func; > > + ), > > + > > + TP_printk("work struct %p: function %ps", __entry->work, > > __entry->function) > > +); > > + > > +/** > > + * sched_kthread_work_execute_end - called immediately after the work > > callback > > + * @work:pointer to struct work_struct > > + * @function: pointer to worker function > > + * > > + * Allows to track workqueue execution. > > + */ > > +TRACE_EVENT(sched_kthread_work_execute_end, > > + > > + TP_PROTO(struct kthread_work *work, kthread_work_func_t function), > > + > > + TP_ARGS(work, function), > > + > > + TP_STRUCT__entry( > > + __field( void *,work) > > + __field( void *,function) > > + ), > > + > > + TP_fast_assign( > > + __entry->work = work; > > + __entry->function = function; > > + ), > > + > > + TP_printk("work struct %p: function %ps", __entry->work, > > __entry->function) > > +); > > + > > > Please combine the above into: > > DECLARE_EVENT_CLASS(sched_kthread_work_execute_template, > > TP_PROTO(struct kthread_work *work), > > TP_ARGS(work), > > TP_STRUCT__entry( > __field( void *,work) > __field( void *,function) > ), > > TP_fast_assign( > __entry->work = work; > __entry->function = work->func; > ), > > TP_printk("work struct %p: function %ps", __entry->work, > __entry->function) > ); > > DEFINE_EVENT(sched_kthread_work_execute_template, > sched_kthread_work_execute_start, > TP_PROTO(struct kthread_work *work), > TP_ARGS(work)); > > DEFINE_EVENT(sched_kthread_work_execute_template, > sched_kthread_work_execute_end, > TP_PROTO(struct kthread_work *work), > TP_ARGS(work)); > > As events are cheap, classes are expensive (size wise), and a TRACE_EVENT() > is really just a CLASS and EVENT for a single instance. I think we cannot quite do this, because we should not rely on being able to dereference work after it finishes. Although I suppose I could just define it to explicitly pass the fxn ptr to both tracepoints.. BR, -R
Re: [PATCH 0/5] tracing: Synthetic event dynamic string fixes
On Fri, 9 Oct 2020 10:17:06 -0500 Tom Zanussi wrote: > These patches provide fixes for the problems observed by Masami in the > new synthetic event dynamic string patchset. > > The first patch (tracing: Don't show dynamic string internals in > synthetic event description) removes the __data_loc from the event > description but leaves it in the format. > > The patch (tracing: Add synthetic event error logging) addresses the > lack of error messages when parse errors occur. > > The remaining three patches address the other problems Masami noted > which result from allowing illegal characters in synthetic event and > field names when defining an event. The is_good_name() function is > used to check that's not possible for the probe events, but should > also be used for the synthetic events as well. > > (tracing: Move is_good_name() from trace_probe.h to trace.h) makes > that function available to other trace subsystems by putting it in > trace.h. (tracing: Check that the synthetic event and field names are > legal) applies it to the synthetic events, and (selftests/ftrace: > Change synthetic event name for inter-event-combined test) changes a > testcase that now fails because it uses an illegal name. > Hi Tom, Would you be able to address Masami's concerns on patches 1 and 4? -- Steve
Re: [PATCH 15/18] dt-bindings: usb: meson-g12a-usb: Discard FL-adj property
On Mon, Oct 12, 2020 at 05:01:43PM +0200, Neil Armstrong wrote: > Hi, > > On 12/10/2020 16:22, Serge Semin wrote: > > On Mon, Oct 12, 2020 at 09:54:25AM +0200, Neil Armstrong wrote: > >> Hi, > >> > >> On 11/10/2020 00:41, Serge Semin wrote: > >>> An empty snps,quirk-frame-length-adjustment won't cause any change > >>> performed by the driver. Moreover the DT schema validation will fail, > >>> since it expects the property being assigned with some value. So just > >>> discard the property declaration then from the example. > >>> > >>> Signed-off-by: Serge Semin > >>> > >>> --- > >>> > >>> Note the same problem is in the DT source file > >>> arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi . > >>> --- > >>> .../devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml | 1 - > >>> 1 file changed, 1 deletion(-) > >>> > >>> diff --git > >>> a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml > >>> b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml > >>> index 5b04a7dfa018..88184d7e26cc 100644 > >>> --- > >>> a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml > >>> +++ > >>> b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml > >>> @@ -209,6 +209,5 @@ examples: > >>>interrupts = <30>; > >>>dr_mode = "host"; > >>>snps,dis_u2_susphy_quirk; > >>> - snps,quirk-frame-length-adjustment; > >>>}; > >>> }; > >>> > >> > > > >> Thanks for reporting this, actually the fladj must be 0x20 on this hw, > >> but we do set this on the PHY side, so we can let the dwc3 side 0 here. > > > > I can convert this patch to initializing the > > "snps,quirk-frame-length-adjustment" > > property with 0x20 value instead. Since most likely I'll have to send a > > v2/v3/etc > > of this patchset, that modification won't be too much work to do. What do > > you think? > > Yes, do this please, Ok. Shall I preserve your Acked-by tag in the new patch or you'd prefer to review it first? > anyway it's only an example so it's ok. Actually examples are also validated by "make dt_binding_check". That's why I had to fix the amlogic,meson-g12a-usb-ctrl example for at least so the new snps,dwc3.yaml DT schema wouldn't break that full DT bindings validation procedure.) -Sergey > > > > > Anyway please note, that I've fixed the improper property usage in the DT > > schema > > example only. "snps,quirk-frame-length-adjustment" defined as boolean still > > persists in the DTS file: arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi > > . > > So if you ever try to validate that dts file with "make dtbs_check" > > scenario, it > > will fail. > > Yes, I'll push a fix to pass the dtbs_check when this is merged. > > Thanks, > Neil > > > > > -Sergey > > > >> > >> Acked-by: Neil Armstrong > >> > >> Neil > >> >
Re: [PATCH 1/1] tracing/boot: Add ftrace.instance.*.alloc_snapshot option
On Sun, 11 Oct 2020 00:28:09 +0900 Masami Hiramatsu wrote: > Add ftrace.instance.*.alloc_snapshot option. > > This option has been described in Documentation/trace/boottime-trace.rst > but not implemented yet. > > ftrace.[instance.INSTANCE.]alloc_snapshot >Allocate snapshot buffer. > > The difference from kernel.alloc_snapshot is that the kernel.alloc_snapshot > will allocate the buffer only for the main instance, but this can allocate > buffer for any new instances. I queued this up. Thanks Masami! -- Steve > > Signed-off-by: Masami Hiramatsu > --- > kernel/trace/trace_boot.c |6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c > index 754e3cf2df3a..c22a152ef0b4 100644 > --- a/kernel/trace/trace_boot.c > +++ b/kernel/trace/trace_boot.c > @@ -284,6 +284,12 @@ trace_boot_enable_tracer(struct trace_array *tr, struct > xbc_node *node) > if (tracing_set_tracer(tr, p) < 0) > pr_err("Failed to set given tracer: %s\n", p); > } > + > + /* Since tracer can free snapshot buffer, allocate snapshot here.*/ > + if (xbc_node_find_value(node, "alloc_snapshot", NULL)) { > + if (tracing_alloc_snapshot_instance(tr) < 0) > + pr_err("Failed to allocate snapshot buffer\n"); > + } > } > > static void __init
Re: [linux-safety] [PATCH] usb: host: ehci-sched: add comment about find_tt() not returning error
On Mon, 12 Oct 2020, Alan Stern wrote: > On Mon, Oct 12, 2020 at 04:11:38PM +0200, Lukas Bulwahn wrote: > > > > > > On Sun, 11 Oct 2020, Sudip Mukherjee wrote: > > > > > Add a comment explaining why find_tt() will not return error even though > > > find_tt() is checking for NULL and other errors. > > > > > > Signed-off-by: Sudip Mukherjee > > > > I get the intent of the comment but there is a large risk that somebody > > could remove the find_tt() call before the calling the function and there > > is no chance to then see later that the comment is actually wrong. > > Why would anybody do that? Who would deliberately go change a driver in > a way that is obviously wrong and would break it? Especially when such > changes are likely to cause compile errors? > > There are tons of changes one might make to any program that will leave > it syntactically valid but will cause it to fail. What's special about > removing the early calls to find_tt()? > > > I guess you want to link this comment here to a code line above and > > request anyone touching the line above to reconsider the comment then. > > That really seems unnecessary. > > > But there is no 'concept' for such kind of requests to changes and > > comments. > > > > So, the comment is true now, but might be true or wrong later. > > > > I am wondering if such comment deserves to be included if we cannot check > > its validity later... > > > > I would prefer a simple tool that could check that your basic assumption > > on the code is valid and if it the basic assumption is still valid, > > just shut up any stupid tool that simply does not get that these calls > > here cannot return any error. > > Real code contains so many assumptions, especially if you include ones > which are obvious to everybody, that such a tool seems impractical. > I fear that problem applies to all static code analysis tools I have seen; at some point, the remaining findings are simply obviously wrong to everybody but the tool does not get those assumptions and continues complaining, making the tool seem impractical. Alan, so would you be willing to take patches where _anyone_ simply adds comments on what functions returns, depending on what this person might consider just not obvious enough? Or are you going to simply reject this 'added a comment' patch here? I am not arguing either way, it is just that it is unclear to me what the added value of the comment really is here. And for the static analysis finding, we need to find a way to ignore this finding without simply ignoring all findings or new findings that just look very similar to the original finding, but which are valid. Lukas
Re: [PATCH] x86/x86_64_defconfig: Enable the serial console
On 12/10/2020 15:40, Willy Tarreau wrote: > On Mon, Oct 12, 2020 at 04:32:12PM +0200, Borislav Petkov wrote: >> On Mon, Oct 12, 2020 at 11:22:10AM +0100, Guillaume Tucker wrote: >>> However, it was found while adding some x86 Chromebooks[1] to >>> KernelCI that x86_64_defconfig lacked some basic things for >>> anyone to be able to boot a kernel with a serial console enabled >>> on those. >> >> Hold on, those are laptops, right? How come they do have serial console? >> Because laptops don't have serial console - that has been the eternal >> problem with debugging kernels on laptops. Yes the link you pointed at is a prerequisite to enable serial console in the firmware (Coreboot/Depthcharge). > Well, to be precise, they don't have *anymore*. I used to exclusively > select laptops having a serial port given that I was using it daily with > routers, until I had to resign when I abandonned my good old NC8000 :-/ You can get serial console on recent enough Chromebooks with a debug interface such as SuzyQable: https://www.sparkfun.com/products/14746 It's not a USB Type-C adapter, it has a debug interface which works with Chromebooks that support Case-Closed Debugging. Anyone can do that without modifying the Chromebook, and with a bit of patience to go through the documentation[1]... The KernelCI sample results from my previous email were run using just that: off-the-shelf Chromebooks + SuzyQ + rebuilt firmware for interactive console and tftp boot + kernel with the config options in Enric's patch. Thanks, Guillaume [1] https://chromium.googlesource.com/chromiumos/platform/ec/+/cr50_stab/docs/case_closed_debugging_cr50.md
Re: [PATCH] trace: Fix some typos in comment
On Sat, 10 Oct 2020 22:09:24 +0800 Qiujun Huang wrote: > s/wihin/within/ > s/retrieven/retrieved/ > s/suppport/support/ > s/wil/will/ > s/accidently/accidentally/ > s/if the if the/if the/ > > Signed-off-by: Qiujun Huang Thanks, I queued this up. -- Steve
Re: [PATCH v2 22/22] drm/msm: Don't implicit-sync if only a single ring
On Mon, Oct 12, 2020 at 7:40 AM Daniel Vetter wrote: > > On Sun, Oct 11, 2020 at 07:09:49PM -0700, Rob Clark wrote: > > From: Rob Clark > > > > Any cross-device sync use-cases *must* use explicit sync. And if there > > is only a single ring (no-preemption), everything is FIFO order and > > there is no need to implicit-sync. > > > > Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as behavior > > is undefined when fences are not used to synchronize buffer usage across > > contexts (which is the only case where multiple different priority rings > > could come into play). > > Uh does this mean msm is broken on dri2/3 and wayland? Or I'm I just > confused by your commit message? No, I don't think so. If there is only a single priority level ringbuffer (ie. no preemption to higher priority ring) then everything is inherently FIFO order. For cases where we are sharing buffers with something external to drm, explicit sync will be used. And we don't implicit sync with display, otherwise x11 (frontbuffer rendering) would not work BR, -R > Since for these protocols we do expect implicit sync accross processes to > work. Even across devices (and nvidia have actually provided quite a bunch > of patches to make this work in i915 - ttm based drivers get this right, > plus dumb scanout drivers using the right helpers also get this all > right). > -Daniel > > > > > Signed-off-by: Rob Clark > > --- > > drivers/gpu/drm/msm/msm_gem_submit.c | 7 --- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c > > b/drivers/gpu/drm/msm/msm_gem_submit.c > > index 3151a0ca8904..c69803ea53c8 100644 > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > > @@ -277,7 +277,7 @@ static int submit_lock_objects(struct msm_gem_submit > > *submit) > > return ret; > > } > > > > -static int submit_fence_sync(struct msm_gem_submit *submit, bool > > no_implicit) > > +static int submit_fence_sync(struct msm_gem_submit *submit, bool > > implicit_sync) > > { > > int i, ret = 0; > > > > @@ -297,7 +297,7 @@ static int submit_fence_sync(struct msm_gem_submit > > *submit, bool no_implicit) > > return ret; > > } > > > > - if (no_implicit) > > + if (!implicit_sync) > > continue; > > > > ret = msm_gem_sync_object(_obj->base, submit->ring->fctx, > > @@ -768,7 +768,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void > > *data, > > if (ret) > > goto out; > > > > - ret = submit_fence_sync(submit, !!(args->flags & > > MSM_SUBMIT_NO_IMPLICIT)); > > + ret = submit_fence_sync(submit, (gpu->nr_rings > 1) && > > + !(args->flags & MSM_SUBMIT_NO_IMPLICIT)); > > if (ret) > > goto out; > > > > -- > > 2.26.2 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
Re: [RFC PATCH] checkpatch: add shebang check to EXECUTE_PERMISSIONS
On Mon, 2020-10-12 at 19:22 +0530, Ujjwal Kumar wrote: > On 12/10/20 11:47 am, Joe Perches wrote: > > On Mon, 2020-10-12 at 11:19 +0530, Ujjwal Kumar wrote: > > > checkpatch.pl checks for invalid EXECUTE_PERMISSIONS on source > > > files. The script leverages filename extensions and its path in > > > the repository to decide whether to allow execute permissions on > > > the file or not. > > > > > > Based on current check conditions, a perl script file having > > > execute permissions, without '.pl' extension in its filename > > > and not belonging to 'scripts/' directory is reported as ERROR > > > which is a false-positive. > > > > > > Adding a shebang check along with current conditions will make > > > the check more generalised and improve checkpatch reports. > > > To do so, without breaking the core design decision of checkpatch, > > > we can fetch the first line from the patch itself and match it for > > > a shebang pattern. > > > > > > There can be cases where the first line is not part of the patch. > > > > For instance: a patch that only changes permissions > > without changing any of the file content. Please add verbiage like this to the commit message. > Should these new changes go as a separate patch or can they be > included in the next iteration of this patch? V2 please.
Re: [PATCH] mm: Make allocator take care of memoryless numa node
On Mon 12-10-20 16:27:39, Xianting Tian wrote: > In architecture like powerpc, we can have cpus without any local memory > attached to it. In such cases the node does not have real memory. Yes, this is normal (unfortunately). > In many places of current kernel code, it doesn't judge whether the node is > memoryless numa node before calling allocator interface. And that is correct. It shouldn't make any assumption on the memory on a given node because that memory might be depleted (similar to no memory) or it can disappear at any moment because of the memory offlining. > This patch is to use local_memory_node(), which is guaranteed to have > memory, in allocator interface. local_memory_node() is a noop in other > architectures that don't support memoryless nodes. > > As the call path: > alloc_pages_node > __alloc_pages_node > __alloc_pages_nodemask > and __alloc_pages_node,__alloc_pages_nodemask may be called directly, > so only add local_memory_node() in __alloc_pages_nodemask. Page allocator should deal with memory less nodes just fine. It has zonelists constructed for each possible nodes. And it will automatically fall back into a node with is closest to the requested node. local_memory_node might be incorrect choice from the topology POV. What kind of problem are you trying to fix? > Signed-off-by: Xianting Tian > --- > include/linux/slab.h | 3 +++ > mm/page_alloc.c | 1 + > mm/slab.c| 6 +- > mm/slob.c| 1 + > mm/slub.c| 10 -- > 5 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/include/linux/slab.h b/include/linux/slab.h > index 24df2393e..527e811e0 100644 > --- a/include/linux/slab.h > +++ b/include/linux/slab.h > @@ -574,6 +574,7 @@ static __always_inline void *kmalloc_node(size_t size, > gfp_t flags, int node) > flags, node, size); > } > #endif > + node = local_memory_node(node); > return __kmalloc_node(size, flags, node); > } > > @@ -626,6 +627,8 @@ static inline void *kmalloc_array_node(size_t n, size_t > size, gfp_t flags, > return NULL; > if (__builtin_constant_p(n) && __builtin_constant_p(size)) > return kmalloc_node(bytes, flags, node); > + > + node = local_memory_node(node); > return __kmalloc_node(bytes, flags, node); > } > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 6866533de..be63c62c2 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -4878,6 +4878,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int > order, int preferred_nid, > return NULL; > } > > + preferred_nid = local_memory_node(preferred_nid); > gfp_mask &= gfp_allowed_mask; > alloc_mask = gfp_mask; > if (!prepare_alloc_pages(gfp_mask, order, preferred_nid, nodemask, , > _mask, _flags)) > diff --git a/mm/slab.c b/mm/slab.c > index f658e86ec..263c2f2e1 100644 > --- a/mm/slab.c > +++ b/mm/slab.c > @@ -3575,7 +3575,10 @@ EXPORT_SYMBOL(kmem_cache_alloc_trace); > */ > void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int > nodeid) > { > - void *ret = slab_alloc_node(cachep, flags, nodeid, _RET_IP_); > + void *ret; > + > + nodeid = local_memory_node(nodeid); > + ret = slab_alloc_node(cachep, flags, nodeid, _RET_IP_); > > trace_kmem_cache_alloc_node(_RET_IP_, ret, > cachep->object_size, cachep->size, > @@ -3593,6 +3596,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache > *cachep, > { > void *ret; > > + nodeid = local_memory_node(nodeid); > ret = slab_alloc_node(cachep, flags, nodeid, _RET_IP_); > > ret = kasan_kmalloc(cachep, ret, size, flags); > diff --git a/mm/slob.c b/mm/slob.c > index 7cc9805c8..1f1c25e06 100644 > --- a/mm/slob.c > +++ b/mm/slob.c > @@ -636,6 +636,7 @@ EXPORT_SYMBOL(__kmalloc_node); > > void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t gfp, int node) > { > + node = local_memory_node(node); > return slob_alloc_node(cachep, gfp, node); > } > EXPORT_SYMBOL(kmem_cache_alloc_node); > diff --git a/mm/slub.c b/mm/slub.c > index 6d3574013..6e5e12b04 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -2921,7 +2921,10 @@ EXPORT_SYMBOL(kmem_cache_alloc_trace); > #ifdef CONFIG_NUMA > void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node) > { > - void *ret = slab_alloc_node(s, gfpflags, node, _RET_IP_); > + void *ret; > + > + node = local_memory_node(node); > + ret = slab_alloc_node(s, gfpflags, node, _RET_IP_); > > trace_kmem_cache_alloc_node(_RET_IP_, ret, > s->object_size, s->size, gfpflags, node); > @@ -2935,7 +2938,10 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *s, > gfp_t gfpflags, > int node, size_t size) > { > - void *ret =
Re: [PATCH -v3] checkpatch: Check for .byte-spelled insn opcodes documentation on x86
On Mon, 2020-10-12 at 16:23 +0200, Borislav Petkov wrote: > From: Borislav Petkov [] > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > @@ -408,6 +408,7 @@ our $Lval = qr{$Ident(?:$Member)*}; > our $Int_type= qr{(?i)llu|ull|ll|lu|ul|l|u}; > our $Binary = qr{(?i)0b[01]+$Int_type?}; > our $Hex = qr{(?i)0x[0-9a-f]+$Int_type?}; > +our $Hex_byte= qr{(?i)0x[0-9a-f]{1,2}}; $Hex_byte needs to be generic and this needs to have a trailing \b otherwise it would match 0x12 from 0x1234 and leave 34
Re: Missing [GIT PULL] request for
On Mon, Oct 12, 2020 at 3:42 PM Ingo Molnar wrote: > > > * Sedat Dilek wrote: > > > Hi, > > > > yesterday, I saw Ingo tagged "locking-urgent-2020-10-11" in tip Git. > > > > Did you drop it or was this for Linux v5.9 final and the git-pull > > request was simply forgotten? > > > > Just curious. > > So I ran the pull request script to send the tree to Linus, but on final > review decided not to send it, as there was a pending bugreport against the > tree, it was very late in the cycle and the commits were pretty fresh. You mean the lockdep recursion in debug_lockdep_rcu_enabled()? - Sedat -
Re: [PATCH 15/18] dt-bindings: usb: meson-g12a-usb: Discard FL-adj property
Hi, On 12/10/2020 16:22, Serge Semin wrote: > On Mon, Oct 12, 2020 at 09:54:25AM +0200, Neil Armstrong wrote: >> Hi, >> >> On 11/10/2020 00:41, Serge Semin wrote: >>> An empty snps,quirk-frame-length-adjustment won't cause any change >>> performed by the driver. Moreover the DT schema validation will fail, >>> since it expects the property being assigned with some value. So just >>> discard the property declaration then from the example. >>> >>> Signed-off-by: Serge Semin >>> >>> --- >>> >>> Note the same problem is in the DT source file >>> arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi . >>> --- >>> .../devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml | 1 - >>> 1 file changed, 1 deletion(-) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml >>> b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml >>> index 5b04a7dfa018..88184d7e26cc 100644 >>> --- a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml >>> +++ b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml >>> @@ -209,6 +209,5 @@ examples: >>>interrupts = <30>; >>>dr_mode = "host"; >>>snps,dis_u2_susphy_quirk; >>> - snps,quirk-frame-length-adjustment; >>>}; >>> }; >>> >> > >> Thanks for reporting this, actually the fladj must be 0x20 on this hw, >> but we do set this on the PHY side, so we can let the dwc3 side 0 here. > > I can convert this patch to initializing the > "snps,quirk-frame-length-adjustment" > property with 0x20 value instead. Since most likely I'll have to send a > v2/v3/etc > of this patchset, that modification won't be too much work to do. What do you > think? Yes, do this please, anyway it's only an example so it's ok. > > Anyway please note, that I've fixed the improper property usage in the DT > schema > example only. "snps,quirk-frame-length-adjustment" defined as boolean still > persists in the DTS file: arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi . > So if you ever try to validate that dts file with "make dtbs_check" scenario, > it > will fail. Yes, I'll push a fix to pass the dtbs_check when this is merged. Thanks, Neil > > -Sergey > >> >> Acked-by: Neil Armstrong >> >> Neil >>
Re: [PATCH] checkpatch: Check for .byte-spelled insn opcodes documentation on x86
On Mon, 2020-10-12 at 16:21 +0200, Borislav Petkov wrote: > On Sat, Oct 10, 2020 at 09:47:59AM -0700, Joe Perches wrote: > > > '/\s*\.byte\s+(?:0x[0-9a-f]{1,2}[\s,]*){2,}/i' > > ^^^ ^ > > now useless without the " > > There are \.byte specifications without " so not useless. Matching optional leading spaces is useless.
Re: [PATCH v3 2/2] media: mtk-vcodec: fix build breakage when one of VPU or SCP is enabled
On 10/11/20 10:35 PM, Alexandre Courbot wrote: > The addition of MT8183 support added a dependency on the SCP remoteproc > module. However the initial patch used the "select" Kconfig directive, > which may result in the SCP module to not be compiled if remoteproc was > disabled. In such a case, mtk-vcodec would try to link against > non-existent SCP symbols. "select" was clearly misused here as explained > in kconfig-language.txt. > > Replace this by a "depends" directive on at least one of the VPU and > SCP modules, to allow the driver to be compiled as long as one of these > is enabled, and adapt the code to support this new scenario. > > Also adapt the Kconfig text to explain the extra requirements for MT8173 > and MT8183. > > Reported-by: Sakari Ailus > Signed-off-by: Alexandre Courbot > Acked-by: Randy Dunlap # build-tested That Ack applied to v2. I have not tested nor acked this version of the patch. > Acked-by: Sakari Ailus > --- > drivers/media/platform/Kconfig| 22 +++ > drivers/media/platform/mtk-vcodec/Makefile| 10 +++-- > .../platform/mtk-vcodec/mtk_vcodec_fw_priv.h | 18 +++ > 3 files changed, 44 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index a3cb104956d5..457b6c39ddc0 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -253,18 +253,32 @@ config VIDEO_MEDIATEK_VCODEC > depends on MTK_IOMMU || COMPILE_TEST > depends on VIDEO_DEV && VIDEO_V4L2 > depends on ARCH_MEDIATEK || COMPILE_TEST > + depends on VIDEO_MEDIATEK_VPU || MTK_SCP > + # The two following lines ensure we have the same state ("m" or "y") as > + # our dependencies, to avoid missing symbols during link. > + depends on VIDEO_MEDIATEK_VPU || !VIDEO_MEDIATEK_VPU > + depends on MTK_SCP || !MTK_SCP > select VIDEOBUF2_DMA_CONTIG > select V4L2_MEM2MEM_DEV > - select VIDEO_MEDIATEK_VPU > - select MTK_SCP > + select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU > + select VIDEO_MEDIATEK_VCODEC_SCP if MTK_SCP > help > Mediatek video codec driver provides HW capability to > - encode and decode in a range of video formats > - This driver rely on VPU driver to communicate with VPU. > + encode and decode in a range of video formats on MT8173 > + and MT8183. > + > + Note that support for MT8173 requires VIDEO_MEDIATEK_VPU to > + also be selected. Support for MT8183 depends on MTK_SCP. > > To compile this driver as modules, choose M here: the > modules will be called mtk-vcodec-dec and mtk-vcodec-enc. > > +config VIDEO_MEDIATEK_VCODEC_VPU > + bool > + > +config VIDEO_MEDIATEK_VCODEC_SCP > + bool > + > config VIDEO_MEM2MEM_DEINTERLACE > tristate "Deinterlace support" > depends on VIDEO_DEV && VIDEO_V4L2 > diff --git a/drivers/media/platform/mtk-vcodec/Makefile > b/drivers/media/platform/mtk-vcodec/Makefile > index 6e1ea3a9f052..4618d43dbbc8 100644 > --- a/drivers/media/platform/mtk-vcodec/Makefile > +++ b/drivers/media/platform/mtk-vcodec/Makefile > @@ -25,5 +25,11 @@ mtk-vcodec-enc-y := venc/venc_vp8_if.o \ > mtk-vcodec-common-y := mtk_vcodec_intr.o \ > mtk_vcodec_util.o \ > mtk_vcodec_fw.o \ > - mtk_vcodec_fw_vpu.o \ > - mtk_vcodec_fw_scp.o > + > +ifneq ($(CONFIG_VIDEO_MEDIATEK_VCODEC_VPU),) > +mtk-vcodec-common-y += mtk_vcodec_fw_vpu.o > +endif > + > +ifneq ($(CONFIG_VIDEO_MEDIATEK_VCODEC_SCP),) > +mtk-vcodec-common-y += mtk_vcodec_fw_scp.o > +endif > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw_priv.h > b/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw_priv.h > index 51f1694a7c7d..b41e66185cec 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw_priv.h > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw_priv.h > @@ -27,8 +27,26 @@ struct mtk_vcodec_fw_ops { > void (*release)(struct mtk_vcodec_fw *fw); > }; > > +#if IS_ENABLED(CONFIG_VIDEO_MEDIATEK_VCODEC_VPU) > struct mtk_vcodec_fw *mtk_vcodec_fw_vpu_init(struct mtk_vcodec_dev *dev, >enum mtk_vcodec_fw_use fw_use); > +#else > +static inline struct mtk_vcodec_fw * > +mtk_vcodec_fw_vpu_init(struct mtk_vcodec_dev *dev, > +enum mtk_vcodec_fw_use fw_use) > +{ > + return ERR_PTR(-ENODEV); > +} > +#endif /* CONFIG_VIDEO_MEDIATEK_VCODEC_VPU */ > + > +#if IS_ENABLED(CONFIG_VIDEO_MEDIATEK_VCODEC_SCP) > struct mtk_vcodec_fw *mtk_vcodec_fw_scp_init(struct mtk_vcodec_dev *dev); > +#else > +static inline struct mtk_vcodec_fw * > +mtk_vcodec_fw_scp_init(struct mtk_vcodec_dev *dev) > +{ > + return ERR_PTR(-ENODEV); > +} > +#endif /* CONFIG_VIDEO_MEDIATEK_VCODEC_SCP */ > > #endif /* _MTK_VCODEC_FW_PRIV_H_ */ > -- ~Randy Reported-by: Randy Dunlap
Re: LPC 2020 Hackroom Session: summary and next steps for isolated user namespaces
On Mon, Oct 12, 2020 at 12:01:09AM -0500, Eric W. Biederman wrote: > Andy Lutomirski writes: > > > On Sun, Oct 11, 2020 at 1:53 PM Josh Triplett wrote: > >> > >> On Fri, Oct 09, 2020 at 11:26:06PM -0500, Serge E. Hallyn wrote: > >> > > 3. Find a way to allow setgroups() in a user namespace while keeping > >> > >in mind the case of groups used for negative access control. > >> > >This was suggested by Josh Triplett and Geoffrey Thomas. Their idea > >> > > was to > >> > >investigate adding a prctl() to allow setgroups() to be called in a > >> > > user > >> > >namespace at the cost of restricting paths to the most restrictive > >> > >permission. So if something is 0707 it needs to be treated as if > >> > > it's > >> > >even though the caller is not in its owning group which is used for > >> > > negative > >> > >access control (how these new semantics will interact with ACLs > >> > > will also > >> > >need to be looked into). > >> > > >> > I should probably think this through more, but for this problem, would it > >> > not suffice to add a new prevgroups grouplist to the struct cred, maybe > >> > struct group_info *locked_groups, and every time an unprivileged task > >> > creates > >> > a new user namespace, add all its current groups to this list? > >> > >> So, effectively, you would be allowed to drop permissions, but > >> locked_groups would still be checked for restrictions? > >> > >> That seems like it'd introduce a new level of complexity (a new facet of > >> permission) to manage. Not opposed, but it does seem more complex than > >> just opting out of using groups for negative permissions. Yeah, it would, but I basically hoped that we could catch most of this at e.g. generic_permission(), and/or we could introduce a helper which automatically adds a check for permission denied from locked_groups, so it shouldn't be too wide-spread. If it does end up showing up all over the place, then that's a good reason not to do this. > > Is there any context other than regular UNIX DAC in which groups can > > act as negative permissions or is this literally just an issue for > > files with a more restrictive group mode than other mode? > > Just that. > > The ideas kicked around in the conversation were some variant of having > a sysctl that says "This system never uses groups for negative > permissions". > > It was also suggested that if the sysctl was set the the permission > checks would be altered such that even if someone tried to set a > negative permission, the more liberal permissions of other would be used > instead. So then this would touch all the same code points which the locked_groups approach would have to touch? > Given that creating /etc/subgid is effectively opting out of negative > permissions already have a sysctl that says that upfront feels like a > very clean solution. > > Eric That feels like a cop-out to me. If some young admin at Roxxon Corp decides she needs to run a container, so installs subuid package and sets that sysctl, how does she know whether or not some previous admin, who has since retired and did not keep good docs, set things up so that a negative acl is keeping nginx from reading some supersecret doc? Now personally I'm not a great believer in the negative acls so I think the above is a very unlikely scenario, but if we're going to worry about it, then we should worry about it :) "Click this button if noone has ever used feature X on this server" -serge
Re: [PATCH v3 11/15] MIPS: generic: Add support for Ingenic SoCs
Hi Guenter, Le lun. 12 oct. 2020 à 7:33, Guenter Roeck a écrit : On Sun, Sep 06, 2020 at 09:29:31PM +0200, Paul Cercueil wrote: Add support for Ingenic SoCs in arch/mips/generic/. The Kconfig changes are here to ensure that it is possible to compile either a generic kernel that supports Ingenic SoCs, or a Ingenic-only kernel, both using the same code base, to avoid duplicated code. Signed-off-by: Paul Cercueil This patch results in the following build error (mips:allmodconfig). In file included from : arch/mips/mm/init.c: In function 'mem_init': include/linux/compiler_types.h:319:38: error: call to '__compiletime_assert_331' declared with attribute error: BUILD_BUG_ON failed: IS_ENABLED(CONFIG_32BIT) && (_PFN_SHIFT > PAGE_SHIFT) Bisect log attached. This doesn't seem to be something that was added with this patch. This COMPILE_BUG_ON() has been here for quite some time... I'm not sure why it triggers now. The mips:allmodconfig works here as long as I switch to CPU_LITTLE_ENDIAN (no big-endian compiler). But I'm at a different HEAD, and I can't find commit d67bc7812221606e1886620a357b13f906814af7 anywhere, in which repo is that found? Cheers, -Paul --- # bad: [d67bc7812221606e1886620a357b13f906814af7] Add linux-next specific files for 20201009 # good: [549738f15da0e5a00275977623be199fbbf7df50] Linux 5.9-rc8 git bisect start 'HEAD' 'v5.9-rc8' # bad: [b71be15b496cc71a3434a198fc1a1b9e08af6c57] Merge remote-tracking branch 'bpf-next/master' into master git bisect bad b71be15b496cc71a3434a198fc1a1b9e08af6c57 # bad: [6be11f939f380ef14bc94242cb0262197ce2a054] Merge remote-tracking branch 'i2c/i2c/for-next' into master git bisect bad 6be11f939f380ef14bc94242cb0262197ce2a054 # good: [c03a115d8ad8a87b6d275c3c91c13bc111217bf6] Merge remote-tracking branch 'samsung-krzk/for-next' into master git bisect good c03a115d8ad8a87b6d275c3c91c13bc111217bf6 # bad: [bdd0ef71b0d7d6a8f1d59af57dc73d19ddc26ad0] Merge remote-tracking branch 'f2fs/dev' into master git bisect bad bdd0ef71b0d7d6a8f1d59af57dc73d19ddc26ad0 # bad: [0c4bd40a7ccd06122c1942f525b714abcd9efe36] Merge remote-tracking branch 'powerpc/next' into master git bisect bad 0c4bd40a7ccd06122c1942f525b714abcd9efe36 # bad: [744d2c114d58c11fd76d572021d7ef3c55a1a225] Merge remote-tracking branch 'nds32/next' into master git bisect bad 744d2c114d58c11fd76d572021d7ef3c55a1a225 # good: [1e9f9330cea616f9f2baf8144f049e4b405715dd] Merge remote-tracking branch 'csky/linux-next' into master git bisect good 1e9f9330cea616f9f2baf8144f049e4b405715dd # bad: [b350041e6f23a71f63f1eee6d939c846838e7e25] MIPS: alchemy: remove unused ALCHEMY_GPIOINT_AU1000 git bisect bad b350041e6f23a71f63f1eee6d939c846838e7e25 # good: [43df4eb2fc9511e09c66252c3fec4f8933a77c73] MIPS: Replace SIBYTE_1956_WAR by CONFIG_SB1_PASS_2_WORKAROUNDS git bisect good 43df4eb2fc9511e09c66252c3fec4f8933a77c73 # good: [13a0ea28e8c698cc0d600fdeed8da3e4d478b97e] MIPS: generic: Init command line with fw_init_cmdline() git bisect good 13a0ea28e8c698cc0d600fdeed8da3e4d478b97e # bad: [d41afc398fbc9dfb8c40b951e97a7f0283346c6a] MAINTAINERS: Update paths to Ingenic platform code git bisect bad d41afc398fbc9dfb8c40b951e97a7f0283346c6a # bad: [f0f4a753079c636d5d43a102edbde0dad1e7de51] MIPS: generic: Add support for Ingenic SoCs git bisect bad f0f4a753079c636d5d43a102edbde0dad1e7de51 # good: [c3e2ee657418f4f2bff1269c0550f8135ed0c927] MIPS: generic: Add support for zboot git bisect good c3e2ee657418f4f2bff1269c0550f8135ed0c927 # good: [02bd530f888c6d6ba4995c3afcd10f87c136f173] MIPS: generic: Increase NR_IRQS to 256 git bisect good 02bd530f888c6d6ba4995c3afcd10f87c136f173 # first bad commit: [f0f4a753079c636d5d43a102edbde0dad1e7de51] MIPS: generic: Add support for Ingenic SoCs
[PATCH v2 1/2] extcon: add driver for TI TUSB320
This patch adds an extcon driver for the TI TUSB320 USB Type-C device. This can be used to detect whether the port is configured as a downstream or upstream facing port. Signed-off-by: Michael Auchter --- Changes since v1: - Drop license text that's redundant with SPDX tag - Cleanup, sort list of includes - Add additional register defines - Switch to use regmap API - Fix Kconfig to depend on I2C, not GPIOLIB drivers/extcon/Kconfig | 8 ++ drivers/extcon/Makefile | 1 + drivers/extcon/extcon-usbc-tusb320.c | 191 +++ 3 files changed, 200 insertions(+) create mode 100644 drivers/extcon/extcon-usbc-tusb320.c diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig index aac507bff135..af58ebca2bf6 100644 --- a/drivers/extcon/Kconfig +++ b/drivers/extcon/Kconfig @@ -186,4 +186,12 @@ config EXTCON_USBC_CROS_EC Say Y here to enable USB Type C cable detection extcon support when using Chrome OS EC based USB Type-C ports. +config EXTCON_USBC_TUSB320 + tristate "TI TUSB320 USB-C extcon support" + depends on I2C + select REGMAP_I2C + help + Say Y here to enable support for USB Type C cable detection extcon + support using a TUSB320. + endif diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile index 52096fd8a216..fe10a1b7d18b 100644 --- a/drivers/extcon/Makefile +++ b/drivers/extcon/Makefile @@ -25,3 +25,4 @@ obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o obj-$(CONFIG_EXTCON_SM5502)+= extcon-sm5502.o obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o obj-$(CONFIG_EXTCON_USBC_CROS_EC) += extcon-usbc-cros-ec.o +obj-$(CONFIG_EXTCON_USBC_TUSB320) += extcon-usbc-tusb320.o diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c new file mode 100644 index ..93f1843ca89b --- /dev/null +++ b/drivers/extcon/extcon-usbc-tusb320.c @@ -0,0 +1,191 @@ +// SPDX-License-Identifier: GPL-2.0 +/** + * drivers/extcon/extcon-tusb320.c - TUSB320 extcon driver + * + * Copyright (C) 2020 National Instruments Corporation + * Author: Michael Auchter + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define TUSB320_REG9 0x9 +#define TUSB320_REG9_ATTACHED_STATE_SHIFT 6 +#define TUSB320_REG9_ATTACHED_STATE_MASK 0x3 +#define TUSB320_REG9_CABLE_DIRECTION BIT(5) +#define TUSB320_REG9_INTERRUPT_STATUS BIT(4) +#define TUSB320_ATTACHED_STATE_NONE0x0 +#define TUSB320_ATTACHED_STATE_DFP 0x1 +#define TUSB320_ATTACHED_STATE_UFP 0x2 +#define TUSB320_ATTACHED_STATE_ACC 0x3 + +struct tusb320_priv { + struct device *dev; + struct regmap *regmap; + struct extcon_dev *edev; +}; + +static const char * const tusb_attached_states[] = { + [TUSB320_ATTACHED_STATE_NONE] = "not attached", + [TUSB320_ATTACHED_STATE_DFP] = "downstream facing port", + [TUSB320_ATTACHED_STATE_UFP] = "upstream facing port", + [TUSB320_ATTACHED_STATE_ACC] = "accessory", +}; + +static const unsigned int tusb320_extcon_cable[] = { + EXTCON_USB, + EXTCON_USB_HOST, + EXTCON_NONE, +}; + +static int tusb320_check_signature(struct tusb320_priv *priv) +{ + static const char sig[] = { '\0', 'T', 'U', 'S', 'B', '3', '2', '0' }; + unsigned val; + int i, ret; + + for (i = 0; i < sizeof(sig); i++) { + ret = regmap_read(priv->regmap, sizeof(sig) - 1 - i, ); + if (ret < 0) + return ret; + if (val != sig[i]) { + dev_err(priv->dev, "signature mismatch!\n"); + return -ENODEV; + } + } + + return 0; +} + +static irqreturn_t tusb320_irq_handler(int irq, void *dev_id) +{ + struct tusb320_priv *priv = dev_id; + int state, polarity; + unsigned reg; + + if (regmap_read(priv->regmap, TUSB320_REG9, )) { + dev_err(priv->dev, "error during i2c read!\n"); + return IRQ_NONE; + } + + if (!(reg & TUSB320_REG9_INTERRUPT_STATUS)) + return IRQ_NONE; + + state = (reg >> TUSB320_REG9_ATTACHED_STATE_SHIFT) & + TUSB320_REG9_ATTACHED_STATE_MASK; + polarity = !!(reg & TUSB320_REG9_CABLE_DIRECTION); + + dev_dbg(priv->dev, "attached state: %s, polarity: %d\n", + tusb_attached_states[state], polarity); + + extcon_set_state(priv->edev, EXTCON_USB, +state == TUSB320_ATTACHED_STATE_UFP); + extcon_set_state(priv->edev, EXTCON_USB_HOST, +state == TUSB320_ATTACHED_STATE_DFP); + extcon_set_property(priv->edev, EXTCON_USB, + EXTCON_PROP_USB_TYPEC_POLARITY, + (union extcon_property_value)polarity); + extcon_set_property(priv->edev,
Re: [linux-safety] [PATCH] usb: host: ehci-sched: add comment about find_tt() not returning error
On Mon, Oct 12, 2020 at 04:11:38PM +0200, Lukas Bulwahn wrote: > > > On Sun, 11 Oct 2020, Sudip Mukherjee wrote: > > > Add a comment explaining why find_tt() will not return error even though > > find_tt() is checking for NULL and other errors. > > > > Signed-off-by: Sudip Mukherjee > > I get the intent of the comment but there is a large risk that somebody > could remove the find_tt() call before the calling the function and there > is no chance to then see later that the comment is actually wrong. Why would anybody do that? Who would deliberately go change a driver in a way that is obviously wrong and would break it? Especially when such changes are likely to cause compile errors? There are tons of changes one might make to any program that will leave it syntactically valid but will cause it to fail. What's special about removing the early calls to find_tt()? > I guess you want to link this comment here to a code line above and > request anyone touching the line above to reconsider the comment then. That really seems unnecessary. > But there is no 'concept' for such kind of requests to changes and > comments. > > So, the comment is true now, but might be true or wrong later. > > I am wondering if such comment deserves to be included if we cannot check > its validity later... > > I would prefer a simple tool that could check that your basic assumption > on the code is valid and if it the basic assumption is still valid, > just shut up any stupid tool that simply does not get that these calls > here cannot return any error. Real code contains so many assumptions, especially if you include ones which are obvious to everybody, that such a tool seems impractical. Alan Stern
Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call
On 10/12/2020 1:13 AM, Kuppuswamy, Sathyanarayanan wrote: > Hi Sinan, > > On 9/28/20 11:32 AM, Kuppuswamy, Sathyanarayanan wrote: >> >> >> On 9/28/20 11:25 AM, Sinan Kaya wrote: >>> On 9/28/2020 2:02 PM, Sinan Kaya wrote: Since there is no state restoration for FATAL errors, I am wondering whether calls to ->error_detected(), ->mmio_enabled() and ->slot_reset() are required? >>> >>> I also would like to ask someone closer to the spec language double >>> check this. >>> >>> When we recover the link at the end of the DPC handler, what is the >>> expected state of the endpoint? >>> >>> Is it a some kind of a reset like secondary bus reset? (I assumed this >>> one) >> I think it will be in reset state. >>> >>> Undefined? >>> >>> or just plain link recovery with everything else as intact as it used >>> to be? >>> >> > > Please check the following version. It should fix most of the reset issues > properly. > > https://lore.kernel.org/linux-pci/5c5bca0bdb958e456176fe6ede10ba8f838fbafc.1602263264.git.sathyanarayanan.kuppusw...@linux.intel.com/T/#t > > Thanks, good stuff.
general protection fault in scsi_queue_rq
Hello, syzbot found the following issue on: HEAD commit:e4fb79c7 Add linux-next specific files for 20201008 git tree: linux-next console output: https://syzkaller.appspot.com/x/log.txt?x=125c9a9f90 kernel config: https://syzkaller.appspot.com/x/.config?x=568d41fe4341ed0f dashboard link: https://syzkaller.appspot.com/bug?extid=0796b72dc61f223d8cc5 compiler: gcc (GCC) 10.1.0-syz 20200507 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12582fe790 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=124ac7d050 The issue was bisected to: commit 2ceda20f0a99a74a82b78870f3b3e5fa93087a7f Author: Christoph Hellwig Date: Mon Oct 5 08:41:23 2020 + scsi: core: Move command size detection out of the fast path bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=135ea77790 final oops: https://syzkaller.appspot.com/x/report.txt?x=10dea77790 console output: https://syzkaller.appspot.com/x/log.txt?x=175ea77790 IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+0796b72dc61f223d8...@syzkaller.appspotmail.com Fixes: 2ceda20f0a99 ("scsi: core: Move command size detection out of the fast path") general protection fault, probably for non-canonical address 0xdc00: [#1] PREEMPT SMP KASAN KASAN: null-ptr-deref in range [0x-0x0007] CPU: 1 PID: 6889 Comm: syz-executor086 Not tainted 5.9.0-rc8-next-20201008-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:scsi_command_size include/scsi/scsi_common.h:24 [inline] RIP: 0010:scsi_setup_scsi_cmnd drivers/scsi/scsi_lib.c:1186 [inline] RIP: 0010:scsi_prepare_cmd drivers/scsi/scsi_lib.c:1565 [inline] RIP: 0010:scsi_queue_rq+0x2155/0x3020 drivers/scsi/scsi_lib.c:1654 Code: 00 fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 98 0c 00 00 48 8b 83 58 02 00 00 48 ba 00 00 00 00 00 fc ff df 48 89 c1 48 c1 e9 03 <0f> b6 14 11 48 89 c1 83 e1 07 38 ca 7f 08 84 d2 0f 85 53 0c 00 00 RSP: 0018:c90005627580 EFLAGS: 00010246 RAX: RBX: 88801b6bd400 RCX: RDX: dc00 RSI: 84aaad82 RDI: 0003 RBP: 88801967 R08: 0001 R09: 88801b6bd7c0 R10: R11: R12: R13: 88801b6bd658 R14: 8881341bc000 R15: FS: 7f1d217b3700() GS:8880ae50() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 5617bf6e4410 CR3: 1f08b000 CR4: 001506e0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Call Trace: blk_mq_dispatch_rq_list+0x3a1/0x1eb0 block/blk-mq.c:1388 __blk_mq_sched_dispatch_requests+0x263/0x490 block/blk-mq-sched.c:308 blk_mq_sched_dispatch_requests+0xfb/0x180 block/blk-mq-sched.c:341 __blk_mq_run_hw_queue+0x13a/0x2d0 block/blk-mq.c:1532 __blk_mq_delay_run_hw_queue+0x522/0x5f0 block/blk-mq.c:1609 blk_mq_run_hw_queue+0x16c/0x2f0 block/blk-mq.c:1662 blk_mq_sched_insert_request+0x4d7/0x5e0 block/blk-mq-sched.c:473 blk_execute_rq+0xd4/0x1b0 block/blk-exec.c:86 sg_io+0x609/0xf50 block/scsi_ioctl.c:360 scsi_cmd_ioctl+0x5ce/0x660 block/scsi_ioctl.c:808 scsi_cmd_blk_ioctl block/scsi_ioctl.c:866 [inline] scsi_cmd_blk_ioctl+0xe1/0x130 block/scsi_ioctl.c:857 sd_ioctl_common+0x17e/0x280 drivers/scsi/sd.c:1575 sd_ioctl+0x26/0xf0 drivers/scsi/sd.c:1765 __blkdev_driver_ioctl block/ioctl.c:228 [inline] blkdev_ioctl+0x2a7/0x7f0 block/ioctl.c:623 block_ioctl+0xf9/0x140 fs/block_dev.c:1869 vfs_ioctl fs/ioctl.c:48 [inline] __do_sys_ioctl fs/ioctl.c:753 [inline] __se_sys_ioctl fs/ioctl.c:739 [inline] __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:739 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x446059 Code: e8 fc b8 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 0b 12 fc ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f1d217b2d98 EFLAGS: 0246 ORIG_RAX: 0010 RAX: ffda RBX: 006ddc48 RCX: 00446059 RDX: 200046c0 RSI: 2285 RDI: 0004 RBP: 006ddc40 R08: R09: R10: R11: 0246 R12: 006ddc4c R13: 2000 R14: 004ae698 R15: 0003 Modules linked in: ---[ end trace 2bb961546eae45fe ]--- RIP: 0010:scsi_command_size include/scsi/scsi_common.h:24 [inline] RIP: 0010:scsi_setup_scsi_cmnd drivers/scsi/scsi_lib.c:1186 [inline] RIP: 0010:scsi_prepare_cmd drivers/scsi/scsi_lib.c:1565 [inline] RIP: 0010:scsi_queue_rq+0x2155/0x3020 drivers/scsi/scsi_lib.c:1654 Code: 00 fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 98 0c 00 00 48 8b 83 58 02 00 00 48 ba 00 00 00 00 00 fc ff df 48 89 c1
[PATCH 3/3] selftests: Add VRF route leaking tests
From: Michael Jeanson The objective of the tests is to check that ICMP errors generated while crossing between VRFs are properly routed back to the source host. The first ttl test sends a ping with a ttl of 1 from h1 to h2 and parses the output of the command to check that a ttl expired error is received. The second ttl test runs traceroute from h1 to h2 and parses the output to check for a hop on r1. The mtu test sends a ping with a payload of 1450 from h1 to h2, through r1 which has an interface with a mtu of 1400 and parses the output of the command to check that a fragmentation needed error is received. [ The IPv6 MTU test still fails with the symmetric routing setup. It appears to be caused by source address selection picking ::1. Fixing this is beyond the scope of this series. ] Signed-off-by: Michael Jeanson Reviewed-by: David Ahern Cc: David Ahern Cc: Jakub Kicinski Cc: David S. Miller Cc: net...@vger.kernel.org --- Changes since v2: - Renamed test script to vrf_route_leaking.sh - Added a default symmetric routing topology - Documented the asymmetric routing topology Changes since v1: - Formating and indentation fixes - Added '-t' to getopts - Reworked verbose output of grep'd commands with a new function - Expanded ip command names - Added fragmentation tests --- tools/testing/selftests/net/Makefile | 1 + .../selftests/net/vrf_route_leaking.sh| 626 ++ 2 files changed, 627 insertions(+) create mode 100755 tools/testing/selftests/net/vrf_route_leaking.sh diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile index 9491bbaa0831..3e7fb1e70c77 100644 --- a/tools/testing/selftests/net/Makefile +++ b/tools/testing/selftests/net/Makefile @@ -19,6 +19,7 @@ TEST_PROGS += txtimestamp.sh TEST_PROGS += vrf-xfrm-tests.sh TEST_PROGS += rxtimestamp.sh TEST_PROGS += devlink_port_split.py +TEST_PROGS += vrf_route_leaking.sh TEST_PROGS_EXTENDED := in_netns.sh TEST_GEN_FILES = socket nettest TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy reuseport_addr_any diff --git a/tools/testing/selftests/net/vrf_route_leaking.sh b/tools/testing/selftests/net/vrf_route_leaking.sh new file mode 100755 index ..23cf924754a5 --- /dev/null +++ b/tools/testing/selftests/net/vrf_route_leaking.sh @@ -0,0 +1,626 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# Copyright (c) 2019 David Ahern . All rights reserved. +# Copyright (c) 2020 Michael Jeanson . All rights reserved. +# +# Requires CONFIG_NET_VRF, CONFIG_VETH, CONFIG_BRIDGE and CONFIG_NET_NS. +# +# +# Symmetric routing topology +# +# blue red +# ++ .253 ++ .253 ++ +# | h1 |---| r1 |---| h2 | +# ++ .1++.2 ++ +# 172.16.1/24 172.16.2/24 +#2001:db8:16:1/64 2001:db8:16:2/64 +# +# +# Route from h1 to h2 and back goes through r1, incoming vrf blue has a route +# to the outgoing vrf red for the n2 network and red has a route back to n1. +# The red VRF interface has a MTU of 1400. +# +# The first test sends a ping with a ttl of 1 from h1 to h2 and parses the +# output of the command to check that a ttl expired error is received. +# +# The second test runs traceroute from h1 to h2 and parses the output to check +# for a hop on r1. +# +# The third test sends a ping with a packet size of 1450 from h1 to h2 and +# parses the output of the command to check that a fragmentation error is +# received. +# +# +# Asymmetric routing topology +# +# This topology represents a customer setup where the issue with icmp errors +# and VRF route leaking was initialy reported. The MTU test isn't done here +# because of the lack of a return route in the red VRF. +# +# blue red +# .253 ++ .253 +# +| r1 |+ +# |++| +# ++ | | ++ +# | h1 |--+ +--| h2 | +# ++ .1 | | .2 ++ +# 172.16.1/24 |++| 172.16.2/24 +#2001:db8:16:1/64 +| r2 |+ 2001:db8:16:2/64 +# .254 ++ .254 +# +# +# Route from h1 to h2 goes through r1, incoming vrf blue has a route to the +# outgoing vrf red for the n2 network but red doesn't have a route back to n1. +# Route from h2 to h1 goes through r2. +# +# The objective is to check that the incoming vrf routing table is selected +# to send an ICMP error back to the source when the ttl of a packet reaches 1 +# while it is forwarded between different vrfs. + +VERBOSE=0 +PAUSE_ON_FAIL=no +DEFAULT_TTYPE=sym + +H1_N1=172.16.1.0/24 +H1_N1_6=2001:db8:16:1::/64 + +H1_N1_IP=172.16.1.1 +R1_N1_IP=172.16.1.253 +R2_N1_IP=172.16.1.254 + +H1_N1_IP6=2001:db8:16:1::1 +R1_N1_IP6=2001:db8:16:1::253
Re: [PATCH v4 1/2] PCI/ERR: Call pci_bus_reset() before calling ->slot_reset() callback
On 10/12/2020 1:03 AM, sathyanarayanan.nkuppusw...@gmail.com wrote: > From: Kuppuswamy Sathyanarayanan > > Currently if report_error_detected() or report_mmio_enabled() > functions requests PCI_ERS_RESULT_NEED_RESET, current > pcie_do_recovery() implementation does not do the requested > explicit device reset, but instead just calls the > report_slot_reset() on all affected devices. Notifying about the > reset via report_slot_reset() without doing the actual device > reset is incorrect. So call pci_bus_reset() before triggering > ->slot_reset() callback. > > Signed-off-by: Kuppuswamy Sathyanarayanan > > --- > drivers/pci/pcie/err.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c > index c543f419d8f9..067c58728b88 100644 > --- a/drivers/pci/pcie/err.c > +++ b/drivers/pci/pcie/err.c > @@ -181,11 +181,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev, > } > > if (status == PCI_ERS_RESULT_NEED_RESET) { > - /* > - * TODO: Should call platform-specific > - * functions to reset slot before calling > - * drivers' slot_reset callbacks? > - */ > + pci_reset_bus(dev); > status = PCI_ERS_RESULT_RECOVERED; > pci_dbg(dev, "broadcast slot_reset message\n"); > pci_walk_bus(bus, report_slot_reset, ); > Reviewed-by: Sinan Kaya
[PATCH 2/3] ipv6/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2)
As per RFC4443, the destination address field for ICMPv6 error messages is copied from the source address field of the invoking packet. In configurations with Virtual Routing and Forwarding tables, looking up which routing table to use for sending ICMPv6 error messages is currently done by using the destination net_device. If the source and destination interfaces are within separate VRFs, or one in the global routing table and the other in a VRF, looking up the source address of the invoking packet in the destination interface's routing table will fail if the destination interface's routing table contains no route to the invoking packet's source address. One observable effect of this issue is that traceroute6 does not work in the following cases: - Route leaking between global routing table and VRF - Route leaking between VRFs Use the source device routing table when sending ICMPv6 error messages. [ In the context of ipv4, it has been pointed out that a similar issue may exist with ICMP errors triggered when forwarding between network namespaces. It would be worthwhile to investigate whether ipv6 has similar issues, but is outside of the scope of this investigation. ] [ Testing shows that similar issues exist with ipv6 unreachable / fragmentation needed messages. However, investigation of this additional failure mode is beyond this investigation's scope. ] Link: https://tools.ietf.org/html/rfc4443 Signed-off-by: Mathieu Desnoyers Reviewed-by: David Ahern Cc: David Ahern Cc: Jakub Kicinski Cc: David S. Miller Cc: net...@vger.kernel.org --- Changes since v1: - Introduce icmp6_get_route_lookup_dev. - Use skb->dev for routing table lookup, because it is guaranteed to be non-NULL. --- net/ipv6/icmp.c | 7 +-- net/ipv6/ip6_output.c | 2 -- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c index a4e4912ad607..91209a2760aa 100644 --- a/net/ipv6/icmp.c +++ b/net/ipv6/icmp.c @@ -501,8 +501,11 @@ void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info, if (__ipv6_addr_needs_scope_id(addr_type)) { iif = icmp6_iif(skb); } else { - dst = skb_dst(skb); - iif = l3mdev_master_ifindex(dst ? dst->dev : skb->dev); + /* +* The source device is used for looking up which routing table +* to use for sending an ICMP error. +*/ + iif = l3mdev_master_ifindex(skb->dev); } /* diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index c78e67d7747f..cd623068de53 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -468,8 +468,6 @@ int ip6_forward(struct sk_buff *skb) * check and decrement ttl */ if (hdr->hop_limit <= 1) { - /* Force OUTPUT device used as source address */ - skb->dev = dst->dev; icmpv6_send(skb, ICMPV6_TIME_EXCEED, ICMPV6_EXC_HOPLIMIT, 0); __IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS); -- 2.17.1
[PATCH 1/3] ipv4/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2)
As per RFC792, ICMP errors should be sent to the source host. However, in configurations with Virtual Routing and Forwarding tables, looking up which routing table to use is currently done by using the destination net_device. commit 9d1a6c4ea43e ("net: icmp_route_lookup should use rt dev to determine L3 domain") changes the interface passed to l3mdev_master_ifindex() and inet_addr_type_dev_table() from skb_in->dev to skb_dst(skb_in)->dev. This effectively uses the destination device rather than the source device for choosing which routing table should be used to lookup where to send the ICMP error. Therefore, if the source and destination interfaces are within separate VRFs, or one in the global routing table and the other in a VRF, looking up the source host in the destination interface's routing table will fail if the destination interface's routing table contains no route to the source host. One observable effect of this issue is that traceroute does not work in the following cases: - Route leaking between global routing table and VRF - Route leaking between VRFs Preferably use the source device routing table when sending ICMP error messages. If no source device is set, fall-back on the destination device routing table. Else, use the main routing table (index 0). [ It has been pointed out that a similar issue may exist with ICMP errors triggered when forwarding between network namespaces. It would be worthwhile to investigate, but is outside of the scope of this investigation. ] [ It has also been pointed out that a similar issue exists with unreachable / fragmentation needed messages, which can be triggered by changing the MTU of eth1 in r1 to 1400 and running: ip netns exec h1 ping -s 1450 -Mdo -c1 172.16.2.2 Some investigation points to raw_icmp_error() and raw_err() as being involved in this last scenario. The focus of this patch is TTL expired ICMP messages, which go through icmp_route_lookup. Investigation of failure modes related to raw_icmp_error() is beyond this investigation's scope. ] Fixes: 9d1a6c4ea43e ("net: icmp_route_lookup should use rt dev to determine L3 domain") Link: https://tools.ietf.org/html/rfc792 Signed-off-by: Mathieu Desnoyers Reviewed-by: David Ahern Cc: David Ahern Cc: Jakub Kicinski Cc: David S. Miller Cc: net...@vger.kernel.org --- Changes since v1: - Introduce icmp_get_route_lookup_dev. --- net/ipv4/icmp.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index cf36f955bfe6..9ea66d903c41 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -457,6 +457,23 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb) local_bh_enable(); } +/* + * The device used for looking up which routing table to use for sending an ICMP + * error is preferably the source whenever it is set, which should ensure the + * icmp error can be sent to the source host, else lookup using the routing + * table of the destination device, else use the main routing table (index 0). + */ +static struct net_device *icmp_get_route_lookup_dev(struct sk_buff *skb) +{ + struct net_device *route_lookup_dev = NULL; + + if (skb->dev) + route_lookup_dev = skb->dev; + else if (skb_dst(skb)) + route_lookup_dev = skb_dst(skb)->dev; + return route_lookup_dev; +} + static struct rtable *icmp_route_lookup(struct net *net, struct flowi4 *fl4, struct sk_buff *skb_in, @@ -465,6 +482,7 @@ static struct rtable *icmp_route_lookup(struct net *net, int type, int code, struct icmp_bxm *param) { + struct net_device *route_lookup_dev; struct rtable *rt, *rt2; struct flowi4 fl4_dec; int err; @@ -479,7 +497,8 @@ static struct rtable *icmp_route_lookup(struct net *net, fl4->flowi4_proto = IPPROTO_ICMP; fl4->fl4_icmp_type = type; fl4->fl4_icmp_code = code; - fl4->flowi4_oif = l3mdev_master_ifindex(skb_dst(skb_in)->dev); + route_lookup_dev = icmp_get_route_lookup_dev(skb_in); + fl4->flowi4_oif = l3mdev_master_ifindex(route_lookup_dev); security_skb_classify_flow(skb_in, flowi4_to_flowi(fl4)); rt = ip_route_output_key_hash(net, fl4, skb_in); @@ -503,7 +522,7 @@ static struct rtable *icmp_route_lookup(struct net *net, if (err) goto relookup_failed; - if (inet_addr_type_dev_table(net, skb_dst(skb_in)->dev, + if (inet_addr_type_dev_table(net, route_lookup_dev, fl4_dec.saddr) == RTN_LOCAL) { rt2 = __ip_route_output_key(net, _dec); if (IS_ERR(rt2)) -- 2.17.1
[PATCH 0/3] l3mdev icmp error route lookup fixes
Hi, Here is a series of fixes for ipv4 and ipv6 which ensure the route lookup is performed on the right routing table in VRF configurations when sending TTL expired icmp errors (useful for traceroute). It includes tests for both ipv4 and ipv6. These fixes address specifically address the code paths involved in sending TTL expired icmp errors. As detailed in the individual commit messages, those fixes do not address similar icmp errors related to network namespaces and unreachable / fragmentation needed messages, which appear to use different code paths. Thanks, Mathieu Mathieu Desnoyers (2): ipv4/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2) ipv6/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2) Michael Jeanson (1): selftests: Add VRF route leaking tests net/ipv4/icmp.c | 23 +- net/ipv6/icmp.c | 7 +- net/ipv6/ip6_output.c | 2 - tools/testing/selftests/net/Makefile | 1 + .../selftests/net/vrf_route_leaking.sh| 626 ++ 5 files changed, 653 insertions(+), 6 deletions(-) create mode 100755 tools/testing/selftests/net/vrf_route_leaking.sh -- 2.17.1
Re: [PATCH v4 2/2] PCI/ERR: Split the fatal and non-fatal error recovery handling
On 10/12/2020 1:03 AM, sathyanarayanan.nkuppusw...@gmail.com wrote: > From: Kuppuswamy Sathyanarayanan > > Commit bdb5ac85777d ("PCI/ERR: Handle fatal error recovery") > merged fatal and non-fatal error recovery paths, and also made > recovery code depend on hotplug handler for "remove affected > device + rescan" support. But this change also complicated the > error recovery path and which in turn led to the following > issues. > > 1. We depend on hotplug handler for removing the affected > devices/drivers on DLLSC LINK down event (on DPC event > trigger) and DPC handler for handling the error recovery. Since > both handlers operate on same set of affected devices, it leads > to race condition, which in turn leads to NULL pointer > exceptions or error recovery failures.You can find more details > about this issue in following link. > > https://lore.kernel.org/linux-pci/20201007113158.48933-1-haifeng.z...@intel.com/T/#t > > 2. For non-hotplug capable devices fatal (DPC) error recovery > is currently broken. Current fatal error recovery implementation > relies on PCIe hotplug (pciehp) handler for detaching and > re-enumerating the affected devices/drivers. So when dealing with > non-hotplug capable devices, recovery code does not restore the state > of the affected devices correctly. You can find more details about > this issue in the following links. > > https://lore.kernel.org/linux-pci/20200527083130.4137-1-zhiqiang@nxp.com/ > https://lore.kernel.org/linux-pci/12115.1588207324@famine/ > https://lore.kernel.org/linux-pci/0e6f89cd6b9e4a72293cc90fafe93487d7c2d295.158584.git.sathyanarayanan.kuppusw...@linux.intel.com/ > > In order to fix the above two issues, we should stop relying on hotplug > handler for cleaning the affected devices/drivers and let error recovery > handler own this functionality. So this patch reverts Commit bdb5ac85777d > ("PCI/ERR: Handle fatal error recovery") and re-introduce the "remove > affected device + rescan" functionality in fatal error recovery handler. > > Also holding pci_lock_rescan_remove() will prevent the race between hotplug > and DPC handler. > > Fixes: bdb5ac85777d ("PCI/ERR: Handle fatal error recovery") > Signed-off-by: Kuppuswamy Sathyanarayanan > > --- > Documentation/PCI/pci-error-recovery.rst | 47 ++-- > drivers/pci/pcie/err.c | 71 +++- > 2 files changed, 87 insertions(+), 31 deletions(-) I'm not sure about locks involved but I do like the concept. Current fatal error handling is best effort. There is no way to recover if link is down by the time we reach to fatal error handling routine. This change will make the solution more reliable. Reviewed-by: Sinan Kaya
[GIT PULL] printk for 5.10 (includes lockless ringbuffer)
Linus, please pull the latest printk changes from git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux tags/printk-for-5.10 - Fully lockless ringbuffer implementation, including the support for continuous lines. It will allow to store and read messages in any situation wihtout the risk of deadlocks and without the need of temporary per-CPU buffers. The access is still serialized by logbuf_lock. It synchronizes few more operations, for example, temporary buffer for formatting the message, syslog and kmsg_dump operations. The lock removal is being discussed and should be ready for the next release. The continuous lines are handled exactly the same way as before to avoid regressions in user space. It means that they are appended to the last message when the caller is the same. Only the last message can be extended. The data ring includes plain text of the messages. Except for an integer at the beginning of each message that points back to the descriptor ring with other metadata. The dictionary has to stay. journalctl uses it to filter the log. It allows to show messages related to a given device. The dictionary values are stored in the descriptor ring with the other metadata. This is the first part of the printk rework as discussed at Plumbers 2019, see https://lore.kernel.org/r/87k1acz5rx@linutronix.de. The next big step will be handling consoles by kthreads during the normal system operation. It will require special handling of situations when the kthreads could not get scheduled, for example, early boot, suspend, panic. Other changes: + Add John Ogness as a reviewer for printk subsystem. He is author of the rework and is familiar with the code and history. + Fix locking in serial8250_do_startup() to prevent lockdep report. + Few code cleanups. = The fully lockless ringbuffer code has been tested in linux-next since Jul 10. The pr_cont() handling has been pushed in Sep 15. I am not aware of any regression at the moment. But of course, there will be some. And we are ready to work on the fixes. Andy Shevchenko (1): kernel.h: Move oops_in_progress to printk.h Gustavo A. R. Silva (1): printk: Use fallthrough pseudo-keyword John Ogness (22): crash: add VMCOREINFO macro to define offset in a struct declared by typedef printk: add lockless ringbuffer Revert "printk: lock/unlock console only for new logbuf entries" printk: use the lockless ringbuffer printk: ringbuffer: support dataless records printk: reduce LOG_BUF_SHIFT range for H8300 docs: vmcoreinfo: add lockless printk ringbuffer vmcoreinfo scripts/gdb: add utils.read_ulong() scripts/gdb: update for lockless printk ringbuffer printk: ringbuffer: fix setting state in desc_read() printk: ringbuffer: avoid memcpy() on state_var printk: ringbuffer: relocate get_data() printk: ringbuffer: add BLK_DATALESS() macro printk: ringbuffer: clear initial reserved fields printk: ringbuffer: change representation of states printk: ringbuffer: add finalization/extension support printk: reimplement log_cont using record extension printk: move printk_info into separate array printk: move dictionary keys to dev_printk_info printk: remove dict ring printk: avoid and/or handle record truncation printk: reduce setup_text_buf size to LOG_LINE_MAX Petr Mladek (2): MAINTAIERS: Add John Ogness as printk reviewer Merge branch 'printk-rework' into for-linus Randy Dunlap (1): kernel: printk: delete repeated words in comments Sergey Senozhatsky (1): serial: 8250: change lock order in serial8250_do_startup() Documentation/admin-guide/kdump/gdbmacros.txt | 159 +- Documentation/admin-guide/kdump/vmcoreinfo.rst | 131 +- MAINTAINERS|1 + drivers/base/core.c| 46 +- drivers/tty/serial/8250/8250_port.c|9 +- include/linux/crash_core.h |3 + include/linux/debug_locks.h|2 +- include/linux/dev_printk.h |8 + include/linux/kernel.h |1 - include/linux/printk.h |8 +- init/Kconfig |3 +- kernel/printk/Makefile |1 + kernel/printk/internal.h |4 +- kernel/printk/printk.c | 1153 +++-- kernel/printk/printk_ringbuffer.c | 2083 kernel/printk/printk_ringbuffer.h | 382 + kernel/printk/printk_safe.c|2 +- scripts/gdb/linux/dmesg.py | 147 +- scripts/gdb/linux/utils.py |7 + 19
Re: [PATCH 2/2] dt-bindings: arm: sunxi: Fix Orange Pi Zero bindings
On Thu, Oct 08, 2020 at 08:40:06PM +0200, Michal Suchanek wrote: > There are two models of Orange Pi zero which are confusingly marketed > under the same name. Old model comes without a flash memory and current > model does have a flash memory. Add bindings for each model. > > Signed-off-by: Michal Suchanek Unfortunately, changing a compatible or a DT filename is not an option Maxime signature.asc Description: PGP signature
Re: [PATCH v3 1/2] tracing: support "bool" type in synthetic trace events
On Mon, 12 Oct 2020 09:26:13 -0500 Tom Zanussi wrote: > Hi Steve, > > Looks ok to me. > > Acked-by: Tom Zanussi Great! I'll pull this patch into my tree. It doesn't look like patch 2/2 is dependent on this and these two can go through different trees. Is everyone OK if I take this patch through my tree? -- Steve
Re: [PATCH] trace: Return ENOTCONN instead of EBADF
On Mon, 12 Oct 2020 14:26:48 + "Enderborg, Peter" wrote: > On 10/12/20 3:53 PM, Steven Rostedt wrote: > > On Mon, 12 Oct 2020 10:26:42 +0200 > > Peter Enderborg wrote: > > > >> When there is no clients listening on event the trace return > >> EBADF. The file is not a bad file descriptor and to get the > >> userspace able to do a proper error handling it need a different > >> error code that separate a bad file descriptor from a empty listening. > > I have no problem with this patch, but your description is incorrect. And > > before making this change, I want to make sure that what you think is > > happening is actually happening. > > > > This has nothing to do with "clients listening". This happens when the ring > > buffer is disabled for some reason. The most likely case of this happening > > is if someone sets /sys/kernel/tracing/tracing_on to zero. > > I see that as no one is listening. You start to listen by setting this > tracing on > some instance, but that is for trace_pipe. Is it the same flag for raw access > and all ways you > can invoke a trace? I don't know what you mean by "setting this tracing on some instance". When you boot up, who is listening? tracing_on is enabled. But there's no listener. > > Would > > "When there is no instances listening on events the trace return What instance are you talking about? What do you think needs to be listening. And no, trace_pipe plays zero role in this. > EBADF, it is when the tracing_on is off globally. The file is not a bad file > descriptor and to get the userspace able to do a proper error handling it > need a different > error code that separate a bad file descriptor from a empty listening." > > be a ok? > No. Because I don't understand any of the above! There's no a producer / consumer here. It's a read / write enabled or disabled. If the ring buffer is disabled from write, and you try to write to the ring buffer, you get an error. Basically, it's the same analogy as trying to write to a read-only file. Perhaps the proper error is EPERM. -- Steve
Re: [PATCH v2 08/17] s390/pci: Remove races against pte updates
... snip ... >>> Cc: linux-me...@vger.kernel.org >>> Cc: Niklas Schnelle >>> Cc: Gerald Schaefer >>> Cc: linux-s...@vger.kernel.org >>> -- >>> v2: Move VM_IO | VM_PFNMAP checks around so they keep returning EINVAL >>> like before (Gerard) >> >> I think the above should go before the CC/Signed-off/Reviewev block. > > This is a per-subsystem bikeshed :-) drivers/gpu definitely wants it > above, but most core subsystems want it below. I'll move it. Today I learned, thanks! That said I think most of the time I've actually not seen version change information in the commit message itself only in the cover letters. I really don't care just looked odd to me. > >>> --- >>> arch/s390/pci/pci_mmio.c | 98 +++- >>> 1 file changed, 57 insertions(+), 41 deletions(-) >>> >>> diff --git a/arch/s390/pci/pci_mmio.c b/arch/s390/pci/pci_mmio.c >>> index 401cf670a243..1a6adbc68ee8 100644 >>> --- a/arch/s390/pci/pci_mmio.c >>> +++ b/arch/s390/pci/pci_mmio.c >>> @@ -119,33 +119,15 @@ static inline int __memcpy_toio_inuser(void __iomem >>> *dst, >>> return rc; >>> } >>> >>> -static long get_pfn(unsigned long user_addr, unsigned long access, >>> - unsigned long *pfn) >>> -{ >>> - struct vm_area_struct *vma; >>> - long ret; >>> - >>> - mmap_read_lock(current->mm); >>> - ret = -EINVAL; >>> - vma = find_vma(current->mm, user_addr); >>> - if (!vma) >>> - goto out; >>> - ret = -EACCES; >>> - if (!(vma->vm_flags & access)) >>> - goto out; >>> - ret = follow_pfn(vma, user_addr, pfn); >>> -out: >>> - mmap_read_unlock(current->mm); >>> - return ret; >>> -} >>> - >>> SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long, mmio_addr, >>> const void __user *, user_buffer, size_t, length) >>> { >>> u8 local_buf[64]; >>> void __iomem *io_addr; >>> void *buf; >>> - unsigned long pfn; >>> + struct vm_area_struct *vma; >>> + pte_t *ptep; >>> + spinlock_t *ptl; >> >> With checkpatch.pl --strict the above yields a complained >> "CHECK: spinlock_t definition without comment" but I think >> that's really okay since your commit description is very clear. >> Same oin line 277. > > I think this is a falls positive, checkpatch doesn't realize that > SYSCALL_DEFINE3 is a function, not a structure. And in a structure I'd > have added the kerneldoc or comment. Interesting, your theory sounds convincing, I too thought this was a bit too pedantic. > > I'll fix up all the nits you've found for the next round. Thanks for > taking a look. You're welcome hope I didn't sound pedantic. I think you've a lot more experience actually and this can indeed turn into bikeshedding but since I was answering anyway and most of this was checkpatch… > -Daniel >
Re: [PATCH] x86/x86_64_defconfig: Enable the serial console
On Mon, Oct 12, 2020 at 04:32:12PM +0200, Borislav Petkov wrote: > On Mon, Oct 12, 2020 at 11:22:10AM +0100, Guillaume Tucker wrote: > > However, it was found while adding some x86 Chromebooks[1] to > > KernelCI that x86_64_defconfig lacked some basic things for > > anyone to be able to boot a kernel with a serial console enabled > > on those. > > Hold on, those are laptops, right? How come they do have serial console? > Because laptops don't have serial console - that has been the eternal > problem with debugging kernels on laptops. Well, to be precise, they don't have *anymore*. I used to exclusively select laptops having a serial port given that I was using it daily with routers, until I had to resign when I abandonned my good old NC8000 :-/ Willy
Re: [PATCH v2 22/22] drm/msm: Don't implicit-sync if only a single ring
On Sun, Oct 11, 2020 at 07:09:49PM -0700, Rob Clark wrote: > From: Rob Clark > > Any cross-device sync use-cases *must* use explicit sync. And if there > is only a single ring (no-preemption), everything is FIFO order and > there is no need to implicit-sync. > > Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as behavior > is undefined when fences are not used to synchronize buffer usage across > contexts (which is the only case where multiple different priority rings > could come into play). Uh does this mean msm is broken on dri2/3 and wayland? Or I'm I just confused by your commit message? Since for these protocols we do expect implicit sync accross processes to work. Even across devices (and nvidia have actually provided quite a bunch of patches to make this work in i915 - ttm based drivers get this right, plus dumb scanout drivers using the right helpers also get this all right). -Daniel > > Signed-off-by: Rob Clark > --- > drivers/gpu/drm/msm/msm_gem_submit.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c > b/drivers/gpu/drm/msm/msm_gem_submit.c > index 3151a0ca8904..c69803ea53c8 100644 > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > @@ -277,7 +277,7 @@ static int submit_lock_objects(struct msm_gem_submit > *submit) > return ret; > } > > -static int submit_fence_sync(struct msm_gem_submit *submit, bool no_implicit) > +static int submit_fence_sync(struct msm_gem_submit *submit, bool > implicit_sync) > { > int i, ret = 0; > > @@ -297,7 +297,7 @@ static int submit_fence_sync(struct msm_gem_submit > *submit, bool no_implicit) > return ret; > } > > - if (no_implicit) > + if (!implicit_sync) > continue; > > ret = msm_gem_sync_object(_obj->base, submit->ring->fctx, > @@ -768,7 +768,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void > *data, > if (ret) > goto out; > > - ret = submit_fence_sync(submit, !!(args->flags & > MSM_SUBMIT_NO_IMPLICIT)); > + ret = submit_fence_sync(submit, (gpu->nr_rings > 1) && > + !(args->flags & MSM_SUBMIT_NO_IMPLICIT)); > if (ret) > goto out; > > -- > 2.26.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [tip:x86/pti] BUILD SUCCESS WITH WARNING 767d46ab566dd489733666efe48732d523c8c332
On Mon, Oct 12, 2020 at 05:16:54PM +0800, Rong Chen wrote: > We have added the reported links in the report, you can find it in the > latest tip report: > > [tip:master] BUILD REGRESSION 820e6f502f021417140bc8ee11f9c7be148ea844 > > tree/branch:https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git > master > branch HEAD: 820e6f502f021417140bc8ee11f9c7be148ea844 Merge branch 'efi/core' > > Error/Warning reports: > > https://lore.kernel.org/lkml/202010112007.jdl1bsci-...@intel.com Thanks, that link has all the info needed to reproduce AFAICT, as long as you make sure to send it to a mailing list which gets archived by lore.kernel.org - otherwise the redirection won't work. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 07/22] drm/msm: Do rpm get sooner in the submit path
On Sun, Oct 11, 2020 at 07:09:34PM -0700, Rob Clark wrote: > From: Rob Clark > > Unfortunately, due to an dev_pm_opp locking interaction with > mm->mmap_sem, we need to do pm get before aquiring obj locks, > otherwise we can have anger lockdep with the chain: tbh this sounds like a bug in that subsystem, since it means we cannot use said subsystem in mmap handlers either. So if you have some remapping unit or need to wake up your gpu to blt the buffer into system memory first, we're toast. That doesn't sound right. So maybe Cc: pm folks and figure out how to fix this long term properly? Imo not a good reason to hold up this patch set, since unwrangling mmap_sem tends to be work ... -Daniel > > opp_table_lock --> >mmap_sem --> reservation_ww_class_mutex > > For an explicit fencing userspace, the impact should be minimal > as we do all the fence waits before this point. It could result > in some needless resumes in error cases, etc. > > Signed-off-by: Rob Clark > --- > drivers/gpu/drm/msm/msm_gem_submit.c | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c > b/drivers/gpu/drm/msm/msm_gem_submit.c > index 002130d826aa..a9422d043bfe 100644 > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > @@ -744,11 +744,20 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void > *data, > > ret = submit_lookup_objects(submit, args, file); > if (ret) > - goto out; > + goto out_pre_pm; > > ret = submit_lookup_cmds(submit, args, file); > if (ret) > - goto out; > + goto out_pre_pm; > + > + /* > + * Thanks to dev_pm_opp opp_table_lock interactions with mm->mmap_sem > + * in the resume path, we need to to rpm get before we lock objs. > + * Which unfortunately might involve powering up the GPU sooner than > + * is necessary. But at least in the explicit fencing case, we will > + * have already done all the fence waiting. > + */ > + pm_runtime_get_sync(>pdev->dev); > > /* copy_*_user while holding a ww ticket upsets lockdep */ > ww_acquire_init(>ticket, _ww_class); > @@ -825,6 +834,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void > *data, > > > out: > + pm_runtime_put(>pdev->dev); > +out_pre_pm: > submit_cleanup(submit); > if (has_ww_ticket) > ww_acquire_fini(>ticket); > -- > 2.26.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: Missing [GIT PULL] request for
On Mon, Oct 12, 2020 at 3:42 PM Ingo Molnar wrote: > > > * Sedat Dilek wrote: > > > Hi, > > > > yesterday, I saw Ingo tagged "locking-urgent-2020-10-11" in tip Git. > > > > Did you drop it or was this for Linux v5.9 final and the git-pull > > request was simply forgotten? > > > > Just curious. > > So I ran the pull request script to send the tree to Linus, but on final > review decided not to send it, as there was a pending bugreport against the > tree, it was very late in the cycle and the commits were pretty fresh. I > sent two other trees (x86/urgent and perf/urgent). > > This is why there's a signed tag for locking/urgent, but no pull request. > :-) > OK, I see and thanks for the information. - Sedat -
Re: [PATCH] trace: Return ENOTCONN instead of EBADF
On 10/12/20 3:53 PM, Steven Rostedt wrote: > On Mon, 12 Oct 2020 10:26:42 +0200 > Peter Enderborg wrote: > >> When there is no clients listening on event the trace return >> EBADF. The file is not a bad file descriptor and to get the >> userspace able to do a proper error handling it need a different >> error code that separate a bad file descriptor from a empty listening. > I have no problem with this patch, but your description is incorrect. And > before making this change, I want to make sure that what you think is > happening is actually happening. > > This has nothing to do with "clients listening". This happens when the ring > buffer is disabled for some reason. The most likely case of this happening > is if someone sets /sys/kernel/tracing/tracing_on to zero. I see that as no one is listening. You start to listen by setting this tracing on some instance, but that is for trace_pipe. Is it the same flag for raw access and all ways you can invoke a trace? Would "When there is no instances listening on events the trace return EBADF, it is when the tracing_on is off globally. The file is not a bad file descriptor and to get the userspace able to do a proper error handling it need a different error code that separate a bad file descriptor from a empty listening." be a ok? > If this is still something you want applied, please update the change log > to a more accurate scenario. > > Thanks, > > -- Steve > > >> Signed-off-by: Peter Enderborg >> --- >> kernel/trace/trace.c | 8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c >> index d3e5de717df2..6e592bf736df 100644 >> --- a/kernel/trace/trace.c >> +++ b/kernel/trace/trace.c >> @@ -6651,8 +6651,8 @@ tracing_mark_write(struct file *filp, const char >> __user *ubuf, >> event = __trace_buffer_lock_reserve(buffer, TRACE_PRINT, size, >> irq_flags, preempt_count()); >> if (unlikely(!event)) >> -/* Ring buffer disabled, return as if not open for write */ >> -return -EBADF; >> +/* Ring buffer disabled, return as if not connected */ >> +return -ENOTCONN; >> >> entry = ring_buffer_event_data(event); >> entry->ip = _THIS_IP_; >> @@ -6731,8 +6731,8 @@ tracing_mark_raw_write(struct file *filp, const char >> __user *ubuf, >> event = __trace_buffer_lock_reserve(buffer, TRACE_RAW_DATA, size, >> irq_flags, preempt_count()); >> if (!event) >> -/* Ring buffer disabled, return as if not open for write */ >> -return -EBADF; >> +/* Ring buffer disabled, return not connected */ >> +return -ENOTCONN; >> >> entry = ring_buffer_event_data(event); >>
Re: [PATCH v3 11/15] MIPS: generic: Add support for Ingenic SoCs
On Sun, Sep 06, 2020 at 09:29:31PM +0200, Paul Cercueil wrote: > Add support for Ingenic SoCs in arch/mips/generic/. > > The Kconfig changes are here to ensure that it is possible to compile > either a generic kernel that supports Ingenic SoCs, or a Ingenic-only > kernel, both using the same code base, to avoid duplicated code. > > Signed-off-by: Paul Cercueil This patch results in the following build error (mips:allmodconfig). In file included from : arch/mips/mm/init.c: In function 'mem_init': include/linux/compiler_types.h:319:38: error: call to '__compiletime_assert_331' declared with attribute error: BUILD_BUG_ON failed: IS_ENABLED(CONFIG_32BIT) && (_PFN_SHIFT > PAGE_SHIFT) Bisect log attached. Guenter --- # bad: [d67bc7812221606e1886620a357b13f906814af7] Add linux-next specific files for 20201009 # good: [549738f15da0e5a00275977623be199fbbf7df50] Linux 5.9-rc8 git bisect start 'HEAD' 'v5.9-rc8' # bad: [b71be15b496cc71a3434a198fc1a1b9e08af6c57] Merge remote-tracking branch 'bpf-next/master' into master git bisect bad b71be15b496cc71a3434a198fc1a1b9e08af6c57 # bad: [6be11f939f380ef14bc94242cb0262197ce2a054] Merge remote-tracking branch 'i2c/i2c/for-next' into master git bisect bad 6be11f939f380ef14bc94242cb0262197ce2a054 # good: [c03a115d8ad8a87b6d275c3c91c13bc111217bf6] Merge remote-tracking branch 'samsung-krzk/for-next' into master git bisect good c03a115d8ad8a87b6d275c3c91c13bc111217bf6 # bad: [bdd0ef71b0d7d6a8f1d59af57dc73d19ddc26ad0] Merge remote-tracking branch 'f2fs/dev' into master git bisect bad bdd0ef71b0d7d6a8f1d59af57dc73d19ddc26ad0 # bad: [0c4bd40a7ccd06122c1942f525b714abcd9efe36] Merge remote-tracking branch 'powerpc/next' into master git bisect bad 0c4bd40a7ccd06122c1942f525b714abcd9efe36 # bad: [744d2c114d58c11fd76d572021d7ef3c55a1a225] Merge remote-tracking branch 'nds32/next' into master git bisect bad 744d2c114d58c11fd76d572021d7ef3c55a1a225 # good: [1e9f9330cea616f9f2baf8144f049e4b405715dd] Merge remote-tracking branch 'csky/linux-next' into master git bisect good 1e9f9330cea616f9f2baf8144f049e4b405715dd # bad: [b350041e6f23a71f63f1eee6d939c846838e7e25] MIPS: alchemy: remove unused ALCHEMY_GPIOINT_AU1000 git bisect bad b350041e6f23a71f63f1eee6d939c846838e7e25 # good: [43df4eb2fc9511e09c66252c3fec4f8933a77c73] MIPS: Replace SIBYTE_1956_WAR by CONFIG_SB1_PASS_2_WORKAROUNDS git bisect good 43df4eb2fc9511e09c66252c3fec4f8933a77c73 # good: [13a0ea28e8c698cc0d600fdeed8da3e4d478b97e] MIPS: generic: Init command line with fw_init_cmdline() git bisect good 13a0ea28e8c698cc0d600fdeed8da3e4d478b97e # bad: [d41afc398fbc9dfb8c40b951e97a7f0283346c6a] MAINTAINERS: Update paths to Ingenic platform code git bisect bad d41afc398fbc9dfb8c40b951e97a7f0283346c6a # bad: [f0f4a753079c636d5d43a102edbde0dad1e7de51] MIPS: generic: Add support for Ingenic SoCs git bisect bad f0f4a753079c636d5d43a102edbde0dad1e7de51 # good: [c3e2ee657418f4f2bff1269c0550f8135ed0c927] MIPS: generic: Add support for zboot git bisect good c3e2ee657418f4f2bff1269c0550f8135ed0c927 # good: [02bd530f888c6d6ba4995c3afcd10f87c136f173] MIPS: generic: Increase NR_IRQS to 256 git bisect good 02bd530f888c6d6ba4995c3afcd10f87c136f173 # first bad commit: [f0f4a753079c636d5d43a102edbde0dad1e7de51] MIPS: generic: Add support for Ingenic SoCs
Re: [PATCH] x86/x86_64_defconfig: Enable the serial console
On Mon, Oct 12, 2020 at 11:22:10AM +0100, Guillaume Tucker wrote: > However, it was found while adding some x86 Chromebooks[1] to > KernelCI that x86_64_defconfig lacked some basic things for > anyone to be able to boot a kernel with a serial console enabled > on those. Hold on, those are laptops, right? How come they do have serial console? Because laptops don't have serial console - that has been the eternal problem with debugging kernels on laptops. Or do they do some sort of serial over USB emulation and this is something which only chromebooks have? Or is that how it is done: https://www.chromium.org/chromium-os/how-tos-and-troubleshooting/serial-debugging-howto ? Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v9 1/6] fpga: dfl: fix the definitions of type & feature_id for dfl devices
On Mon, Oct 12, 2020 at 07:07:55AM -0700, Tom Rix wrote: > > On 10/11/20 7:41 PM, Xu Yilun wrote: > > On Sat, Oct 10, 2020 at 08:07:07AM -0700, Tom Rix wrote: > >> On 10/10/20 12:09 AM, Xu Yilun wrote: > >>> The value of the field dfl_device.type comes from the 12 bits register > >>> field DFH_ID according to DFL spec. So this patch changes the definition > >>> of the type field to u16. > >>> > >>> Also it is not necessary to illustrate the valid bits of the type field > >>> in comments. Instead we should explicitly define the possible values in > >>> the enumeration type for it, because they are shared by hardware spec. > >>> We should not let the compiler decide these values. > >>> > >>> Similar changes are also applied to dfl_device.feature_id. > >>> > >>> This patch also fixed the MODALIAS format according to the changes > >>> above. > >>> > >>> Signed-off-by: Xu Yilun > >>> --- > >>> v9: no change > >>> --- > >>> drivers/fpga/dfl.c | 3 +-- > >>> drivers/fpga/dfl.h | 14 +++--- > >>> 2 files changed, 8 insertions(+), 9 deletions(-) > >>> > >>> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c > >>> index b450870..5a6ba3b 100644 > >>> --- a/drivers/fpga/dfl.c > >>> +++ b/drivers/fpga/dfl.c > >>> @@ -298,8 +298,7 @@ static int dfl_bus_uevent(struct device *dev, struct > >>> kobj_uevent_env *env) > >>> { > >>> struct dfl_device *ddev = to_dfl_dev(dev); > >>> > >>> - /* The type has 4 valid bits and feature_id has 12 valid bits */ > >>> - return add_uevent_var(env, "MODALIAS=dfl:t%01Xf%03X", > >>> + return add_uevent_var(env, "MODALIAS=dfl:t%04Xf%04X", > >>> ddev->type, ddev->feature_id); > >>> } > >>> > >>> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h > >>> index 5dc758f..ac373b1 100644 > >>> --- a/drivers/fpga/dfl.h > >>> +++ b/drivers/fpga/dfl.h > >>> @@ -520,19 +520,19 @@ long dfl_feature_ioctl_set_irq(struct > >>> platform_device *pdev, > >>> * enum dfl_id_type - define the DFL FIU types > >>> */ > >>> enum dfl_id_type { > >>> - FME_ID, > >>> - PORT_ID, > >>> + FME_ID = 0, > >>> + PORT_ID = 1, > >> This is redundant, why make this change ? > > These values are shared by hardware spec, so it is suggested that the > > values of the enum type should be explicitly set, otherwise the compiler > > is in its right to do whatever it wants with them (within reason...) > > > > Please see the original discussion: > > https://lore.kernel.org/linux-fpga/20200923055436.ga2629...@kroah.com/ > > I don't believe this is undefined behavior for the compiler > > from c11 6.7.2.2,3 > > The identifiers in an enumerator list are declared as constants that have > type int and may appear wherever such are permitted.127) An enumerator with = > defines its enumeration constant as the value of the constant expression. If > the first enumerator has no =, the value of its enumeration constant is 0. > Each subsequent enumerator with no = defines its enumeration constant as the > value of the constant expression obtained by adding 1 to the value of the > previous enumeration constant. (The use of enumerators with = may produce > enumeration constants with values that duplicate other values in the same > enumeration.) The enumerators of an enumeration are also known as its members. > > setting them again has some use for documentation so this change is ok if you > have strong feeling for it. The kernel developer community has "strong feelings" for this, please be specific and list the values when they matter. thanks, greg k-h
[GIT PULL] hwmon updates for v5.10
Hi Linus, Please pull hwmon updates for Linux v5.10 from signed tag: git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-for-v5.10 Thanks, Guenter -- The following changes since commit ba4f184e126b751d1bffad5897f263108befc780: Linux 5.9-rc6 (2020-09-20 16:33:55 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git tags/hwmon-for-v5.10 for you to fetch changes up to 9b20aec24b8ab2860ea41182ba554dfcc444c0c8: hwmon: (pmbus/max20730) adjust the vout reading given voltage divider (2020-10-06 14:54:55 -0700) hwmon changes for v5.10-rc1 New driver and chip support: - Moortec MR75203 PVT controller - MPS Multi-phase mp2975 controller - ADM1266 - Zen3 CPUs - Intel MAX 10 BMC Enhancements: - Support for rated attributes in hwmon core - MAX20730 - Device monitoring via debugfs - VOUT readin adjustment vie devicetree bindings - LM75 - Devicetree support - Regulator support - Improved accumulationm logic in amd_energy driver - Added fan sensor to gsc-hwmon driver - Support for simplified I2C probing Various other minor fixes and improvements. Akshay Gupta (1): hwmon: (amd_energy) Move label out of accumulation structure Alban Bedel (3): hwmon: (lm75) Add regulator support dt-bindings: hwmon: Convert lm75 bindings to yaml dt-bindings: hwmon: Add the +vs supply to the lm75 bindings Alexandru Ardelean (1): docs: hwmon: (ltc2945) update datasheet link Alexandru Tachici (6): hwmon: (pmbus) Add support for ADM1266 hwmon: (pmbus/adm1266) Add Block process call hwmon: (pmbus/adm1266) Add support for GPIOs hwmon: (pmbus/adm1266) add debugfs for states hwmon: (pmbus/adm1266) read blackbox dt-bindings: hwmon: Add bindings for ADM1266 Andrew Jeffery (1): hwmon: (pmbus) Expose PEC debugfs attribute Anson Huang (1): hwmon: (pwm-fan) Use dev_err_probe() to simplify error handling Chris Packham (3): hwmon: (adm9240) Use loops to avoid duplicated code hwmon: (adm9240) Create functions for updating measure and config hwmon: (adm9240) Convert to regmap Chris Ruehl (2): hwmon: shtc1: add support for device tree bindings devicetree: hwmon: shtc1: add sensirion,shtc1.yaml Chu Lin (2): dt-bindings: hwmon: max20730: adding device tree doc for max20730 hwmon: (pmbus/max20730) adjust the vout reading given voltage divider Dan Carpenter (1): hwmon: (w83627ehf) Fix a resource leak in probe Geert Uytterhoeven (1): hwmon: (mlxreg-fan) Fix double "Mellanox" Guenter Roeck (5): hwmon: (pmbus/max34440) Fix status register reads for MAX344{51,60,61} hwmon: (drivetemp) Add usage not describing impact on drive spin-down hwmon: (k10temp) Take out debugfs code hwmon: (pmbus) Stop caching register values hwmon: (pmbus) Move boolean error condition check to generating code Joe Perches (1): hwmon: (scmi-hwmon) Avoid comma separated statements Lars Povlsen (1): hwmon: (sparx5) Fix initial reading of temperature Naveen Krishna Chatradhi (3): hwmon: (amd_energy) optimize accumulation interval hwmon: (amd_energy) Improve the accumulation logic hwmon: (amd_energy) Update driver documentation Rahul Tanwar (2): hwmon: Add DT bindings schema for PVT controller hwmon: Add hardware monitoring driver for Moortec MR75203 PVT controller Serge Semin (3): hwmon: (bt1-pvt) Test sensor power supply on probe hwmon: (bt1-pvt) Cache current update timeout hwmon: (bt1-pvt) Wait for the completion with timeout Stephen Kitt (14): hwmon (pmbus) use simple i2c probe function hwmon: use simple i2c probe function hwmon: (adm1177) use simple i2c probe hwmon: (adm1029) use simple i2c probe hwmon: (w83793) use simple i2c probe hwmon: (w83791d) use simple i2c probe hwmon: (lm73) use simple i2c probe hwmon: (asc7621) use simple i2c probe hwmon: (emc2103) use simple i2c probe hwmon: (ltc2947) use simple i2c probe hwmon: use simple i2c probe function (take 2) hwmon: (tmp513) use simple i2c probe hwmon: (f75375s) use simple i2c probe hwmon: (dme1737) use simple i2c probe Steve Foreman (1): hwmon: (pmbus/max34440) Fix OC fault limits Tim Harvey (1): hwmon: (gsc-hwmon) add fan sensor Ugur Usug (1): hwmon: (pmbus/max20730) add device monitoring via debugfs Vadim Pasternak (2): hwmon: (pmbus) Add support for MPS Multi-phase mp2975 controller dt-bindings: Add MP2975 voltage regulator device Wang Qing (1): hwmon: (tmp513) fix spelling typo in comments Wei Huang (3): hwmon: (k10temp) Create common functions and macros for Zen CPU families hwmon: (k10temp) Define SVI telemetry
Re: [PATCH v2] leds: lm3697: Rename struct into more appropriate name
Pavel On 10/10/20 4:50 PM, Marek Behun wrote: On Sat, 10 Oct 2020 20:57:00 +0200 Pavel Machek wrote: On Fri 2020-10-09 15:51:35, Gabriel David wrote: The mentioned struct, lm3697_led, was renamed to lm3697_bank since the structure is representing the control banks. This name, in my opinion, is more semantically correct. The pointers referring to it were also renamed. Signed-off-by: Gabriel David --- Yes, this is the same Gabriel David from ultracool...@tutanota.org and ultracool...@disroot.org. If you want me to confirm it I'll gladly do it. No problem with that, and no need to resend. This can proably wait for 5.11... I'd like some comment from Dan... and perhaps I'd want to understand what the difference between LED and bank is. ...there can be more than one LED connected to the given bank, that's what you are pointing out? ...but these LEDs will always work in unison, and they are handled as single LED by Linux, right? Pavel, the controller can connect 3 LED strips (to 3 different pins on the chip). There are 2 LED control banks (this is where you can set brightness). For each LED strip (each output pin) you can configure to which control bank it connects. So you have 3 LED strips and 2 control banks, that is 2^3 = 8 different configurations of connecting LED control bank to LED strip. From the perspective of Linux you see the two control banks as 2 LED class devices (because you are setting brightness for control banks, not for the LED strips). The way Marek explains it is correct and the way I wrote the driver intially. There is no direct control of the LEDs only controlling the 2 banks. As an example a device can put LED string 1 and 2 on a single bank to control the backlight for a display and put LED string 3 on a different bank to control the backlight of a keyboard. Like in the Droid and Droid 4 devices. 2 strings illuminate the display backlight and 1 string illuminates the keyboard the display backlight can have a independent brightness then the keyboard. To me the name of the structure does not impose any functional changes just semantic changes. And it just makes it a bit more difficult to back port functional fixes as this patch would be made mandatory for cherry picking. But I do not get many requests to back port this driver so it maybe be a moot point. Dan
Re: [RFC PATCH 0/3] l3mdev icmp error route lookup fixes
On 10/12/20 7:10 AM, Mathieu Desnoyers wrote: > - On Oct 12, 2020, at 9:45 AM, David Ahern dsah...@gmail.com wrote: > >> On 10/12/20 5:57 AM, Mathieu Desnoyers wrote: >>> OK, do you want to pick up the RFC patch series, or should I re-send it >>> without RFC tag ? >> >> you need to re-send for Dave or Jakub to pick them up via patchworks > > OK. Can I have your Acked-by or Reviewed-by for all three patches ? > sure. Reviewed-by: David Ahern
Re: [PATCH v3 1/2] tracing: support "bool" type in synthetic trace events
Hi Steve, Looks ok to me. Acked-by: Tom Zanussi Thanks, Tom On Mon, 2020-10-12 at 10:15 -0400, Steven Rostedt wrote: > Tom, > > Can you ack this patch for me? > > -- Steve > > > On Fri, 9 Oct 2020 15:05:23 -0700 > Axel Rasmussen wrote: > > > It's common [1] to define tracepoint fields as "bool" when they > > contain > > a true / false value. Currently, defining a synthetic event with a > > "bool" field yields EINVAL. It's possible to work around this by > > using > > e.g. u8 (assuming sizeof(bool) is 1, and bool is unsigned; if > > either of > > these properties don't match, you get EINVAL [2]). > > > > Supporting "bool" explicitly makes hooking this up easier and more > > portable for userspace. > > > > [1]: grep -r "bool" include/trace/events/ > > [2]: check_synth_field() in kernel/trace/trace_events_hist.c > > > > Acked-by: Michel Lespinasse > > Signed-off-by: Axel Rasmussen > > --- > > kernel/trace/trace_events_synth.c | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/kernel/trace/trace_events_synth.c > > b/kernel/trace/trace_events_synth.c > > index 8e1974fbad0e..8f94c84349a6 100644 > > --- a/kernel/trace/trace_events_synth.c > > +++ b/kernel/trace/trace_events_synth.c > > @@ -234,6 +234,8 @@ static int synth_field_size(char *type) > > size = sizeof(long); > > else if (strcmp(type, "unsigned long") == 0) > > size = sizeof(unsigned long); > > + else if (strcmp(type, "bool") == 0) > > + size = sizeof(bool); > > else if (strcmp(type, "pid_t") == 0) > > size = sizeof(pid_t); > > else if (strcmp(type, "gfp_t") == 0) > > @@ -276,6 +278,8 @@ static const char *synth_field_fmt(char *type) > > fmt = "%ld"; > > else if (strcmp(type, "unsigned long") == 0) > > fmt = "%lu"; > > + else if (strcmp(type, "bool") == 0) > > + fmt = "%d"; > > else if (strcmp(type, "pid_t") == 0) > > fmt = "%d"; > > else if (strcmp(type, "gfp_t") == 0) > > -- > > 2.28.0.1011.ga647a8990f-goog > >
Re: [PATCH][next] io_uring: Fix sizeof() mismatch
On 10/12/20 8:03 AM, Colin King wrote: > From: Colin Ian King > > An incorrect sizeof() is being used, sizeof(file_data->table) is not > correct, it should be sizeof(*file_data->table). Thanks, should be a no-op, which is why KASAN didn't complain in my testing. I'll queue this up, thanks. -- Jens Axboe
Re: [PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()
On Mon, 12 Oct 2020 at 13:37, Catalin Marinas wrote: > > On Sat, Oct 10, 2020 at 05:12:31PM +0200, Nicolas Saenz Julienne wrote: > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c > > index f6902a2b4ea6..0eca5865dcb1 100644 > > --- a/arch/arm64/mm/init.c > > +++ b/arch/arm64/mm/init.c > > @@ -196,14 +196,16 @@ static void __init zone_sizes_init(unsigned long min, > > unsigned long max) > > unsigned long max_zone_pfns[MAX_NR_ZONES] = {0}; > > > > #ifdef CONFIG_ZONE_DMA > > + zone_dma_bits = ARM64_ZONE_DMA_BITS; > > + > > if (IS_ENABLED(CONFIG_ACPI)) { > > extern unsigned int acpi_iort_get_zone_dma_size(void); > > > > zone_dma_bits = min(zone_dma_bits, > > acpi_iort_get_zone_dma_size()); > > - arm64_dma_phys_limit = max_zone_phys(zone_dma_bits); > > } > > > > + arm64_dma_phys_limit = max_zone_phys(zone_dma_bits); > > max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit); > > #endif > > #ifdef CONFIG_ZONE_DMA32 > > @@ -394,11 +396,6 @@ void __init arm64_memblock_init(void) > > > > early_init_fdt_scan_reserved_mem(); > > > > - if (IS_ENABLED(CONFIG_ZONE_DMA)) { > > - zone_dma_bits = ARM64_ZONE_DMA_BITS; > > - arm64_dma_phys_limit = max_zone_phys(ARM64_ZONE_DMA_BITS); > > - } > > arm64_dma_phys_limit is used by memblock_alloc_low() (via > ARCH_LOW_ADDRESS_LIMIT). I think it's too late to leave its > initialisation to zone_sizes_init(). > The only generic caller of memblock_alloc_low() is swiotlb_init(), which is called much later. So at that point, we definitely need ARCH_LOW_ADDRESS_LIMIT to be set correctly, but that means doing it in zone_sizes_init() is early enough. So the only problematic reference seems to be crashkernel_reserve() afaict.
Re: [PATCH 2/2] mm: introduce vma_set_file function v4
Hi "Christian, I love your patch! Yet something to improve: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on staging/staging-testing linus/master v5.9 next-20201012] [cannot apply to mmotm/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Christian-K-nig/mm-mmap-fix-fput-in-error-path-v2/20201012-165336 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: arm-randconfig-r034-20201012 (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/4ff869f185acba6d9c37ab6abdb0d9f93f31d15b git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Christian-K-nig/mm-mmap-fix-fput-in-error-path-v2/20201012-165336 git checkout 4ff869f185acba6d9c37ab6abdb0d9f93f31d15b # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): arm-linux-gnueabi-ld: drivers/gpu/drm/vgem/vgem_drv.o: in function `vgem_prime_mmap': >> drivers/gpu/drm/vgem/vgem_drv.c:396: undefined reference to `vma_set_file' arm-linux-gnueabi-ld: drivers/dma-buf/dma-buf.o: in function `dma_buf_mmap': >> drivers/dma-buf/dma-buf.c:1163: undefined reference to `vma_set_file' vim +396 drivers/gpu/drm/vgem/vgem_drv.c 380 381 static int vgem_prime_mmap(struct drm_gem_object *obj, 382 struct vm_area_struct *vma) 383 { 384 int ret; 385 386 if (obj->size < vma->vm_end - vma->vm_start) 387 return -EINVAL; 388 389 if (!obj->filp) 390 return -ENODEV; 391 392 ret = call_mmap(obj->filp, vma); 393 if (ret) 394 return ret; 395 > 396 vma_set_file(vma, obj->filp); 397 vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP; 398 vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); 399 400 return 0; 401 } 402 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip