* Re: [PATCH 1/2] kprobes: Allow architectures to override optinsn page allocation
From: Masami Hiramatsu @ 2021-05-13 0:33 UTC (permalink / raw)
To: kernel test robot
Cc: kbuild-all, linux-kernel, anil.s.keshavamurthy, clang-built-linux,
Paul Mackerras, naveen.n.rao, linuxppc-dev, davem, mhiramat
In-Reply-To: <202105130218.8xHlehr7-lkp@intel.com>
On Thu, 13 May 2021 03:04:51 +0800
kernel test robot <lkp@intel.com> wrote:
> Hi Christophe,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on powerpc/next]
> [also build test WARNING on linus/master v5.13-rc1 next-20210512]
> [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/Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121
> base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
> config: i386-randconfig-r012-20210512 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
> # https://github.com/0day-ci/linux/commit/2a1f135a9ce3c4d86d3bdefed561aa17760f430f
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121
> git checkout 2a1f135a9ce3c4d86d3bdefed561aa17760f430f
> # save the attached .config to linux build tree
> make W=1 W=1 ARCH=i386
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> >> kernel/kprobes.c:324:14: warning: no previous prototype for 'alloc_optinsn_page' [-Wmissing-prototypes]
> 324 | void __weak *alloc_optinsn_page(void)
> | ^~~~~~~~~~~~~~~~~~
> >> kernel/kprobes.c:329:13: warning: no previous prototype for 'free_optinsn_page' [-Wmissing-prototypes]
> 329 | void __weak free_optinsn_page(void *page)
> | ^~~~~~~~~~~~~~~~~
Ah, we need a prototype for those in include/linux/kprobes.h
as same as alloc_insn_page() and free_insn_page().
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v13 1/8] powerpc/mm: Implement set_memory() routines
From: Jordan Niethe @ 2021-05-13 2:08 UTC (permalink / raw)
To: linuxppc-dev
Cc: ajd, cmr, Nicholas Piggin, Aneesh Kumar K.V, naveen.n.rao,
Daniel Axtens
In-Reply-To: <20210510011828.4006623-2-jniethe5@gmail.com>
On Mon, May 10, 2021 at 11:18 AM Jordan Niethe <jniethe5@gmail.com> wrote:
>
> From: Russell Currey <ruscur@russell.cc>
>
> The set_memory_{ro/rw/nx/x}() functions are required for
> STRICT_MODULE_RWX, and are generally useful primitives to have. This
> implementation is designed to be generic across powerpc's many MMUs.
> It's possible that this could be optimised to be faster for specific
> MMUs.
>
> This implementation does not handle cases where the caller is attempting
> to change the mapping of the page it is executing from, or if another
> CPU is concurrently using the page being altered. These cases likely
> shouldn't happen, but a more complex implementation with MMU-specific code
> could safely handle them.
>
> On hash, the linear mapping is not kept in the linux pagetable, so this
> will not change the protection if used on that range. Currently these
> functions are not used on the linear map so just WARN for now.
>
> apply_to_existing_page_range() does not work on huge pages so for now
> disallow changing the protection of huge pages.
>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> [jpn: - Allow set memory functions to be used without Strict RWX
> - Hash: Disallow certain regions
> - Have change_page_attr() take function pointers to manipulate ptes
> - Radix: Add ptesync after set_pte_at()]
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v10: WARN if trying to change the hash linear map
> v11: - Update copywrite dates
> - Allow set memory functions to be used without Strict RWX
> - Hash: Disallow certain regions and add comment explaining why
> - Have change_page_attr() take function pointers to manipulate ptes
> - Clarify change_page_attr()'s comment
> - Radix: Add ptesync after set_pte_at()
> v12: - change_page_attr() back to taking an action value
> - disallow operating on huge pages
> ---
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/set_memory.h | 32 +++++++++
> arch/powerpc/mm/Makefile | 2 +-
> arch/powerpc/mm/pageattr.c | 100 ++++++++++++++++++++++++++
> 4 files changed, 134 insertions(+), 1 deletion(-)
> create mode 100644 arch/powerpc/include/asm/set_memory.h
> create mode 100644 arch/powerpc/mm/pageattr.c
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 3f863dd21374..cce0a137b046 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -138,6 +138,7 @@ config PPC
> select ARCH_HAS_MEMBARRIER_CALLBACKS
> select ARCH_HAS_MEMBARRIER_SYNC_CORE
> select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
> + select ARCH_HAS_SET_MEMORY
> select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !HIBERNATION)
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_HAS_UACCESS_FLUSHCACHE
> diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h
> new file mode 100644
> index 000000000000..64011ea444b4
> --- /dev/null
> +++ b/arch/powerpc/include/asm/set_memory.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_POWERPC_SET_MEMORY_H
> +#define _ASM_POWERPC_SET_MEMORY_H
> +
> +#define SET_MEMORY_RO 0
> +#define SET_MEMORY_RW 1
> +#define SET_MEMORY_NX 2
> +#define SET_MEMORY_X 3
> +
> +int change_memory_attr(unsigned long addr, int numpages, long action);
> +
> +static inline int set_memory_ro(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, SET_MEMORY_RO);
> +}
> +
> +static inline int set_memory_rw(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, SET_MEMORY_RW);
> +}
> +
> +static inline int set_memory_nx(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, SET_MEMORY_NX);
> +}
> +
> +static inline int set_memory_x(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, SET_MEMORY_X);
> +}
> +
> +#endif
> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> index c3df3a8501d4..9142cf1fb0d5 100644
> --- a/arch/powerpc/mm/Makefile
> +++ b/arch/powerpc/mm/Makefile
> @@ -5,7 +5,7 @@
>
> ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>
> -obj-y := fault.o mem.o pgtable.o mmap.o maccess.o \
> +obj-y := fault.o mem.o pgtable.o mmap.o maccess.o pageattr.o \
> init_$(BITS).o pgtable_$(BITS).o \
> pgtable-frag.o ioremap.o ioremap_$(BITS).o \
> init-common.o mmu_context.o drmem.o \
> diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
> new file mode 100644
> index 000000000000..acfdde78e0fb
> --- /dev/null
> +++ b/arch/powerpc/mm/pageattr.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * MMU-generic set_memory implementation for powerpc
> + *
> + * Copyright 2019-2021, IBM Corporation.
> + */
> +
> +#include <linux/mm.h>
> +#include <linux/vmalloc.h>
> +#include <linux/set_memory.h>
> +
> +#include <asm/mmu.h>
> +#include <asm/page.h>
> +#include <asm/pgtable.h>
> +
> +
> +/*
> + * Updates the attributes of a page in three steps:
> + *
> + * 1. invalidate the page table entry
> + * 2. flush the TLB
> + * 3. install the new entry with the updated attributes
> + *
> + * Invalidating the pte means there are situations where this will not work
> + * when in theory it should.
> + * For example:
> + * - removing write from page whilst it is being executed
> + * - setting a page read-only whilst it is being read by another CPU
> + *
> + */
> +static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
> +{
> + long action = (long)data;
> + pte_t pte;
> +
> + spin_lock(&init_mm.page_table_lock);
> +
> + /* invalidate the PTE so it's safe to modify */
> + pte = ptep_get_and_clear(&init_mm, addr, ptep);
> + flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
> +
> + /* modify the PTE bits as desired, then apply */
> + switch (action) {
> + case SET_MEMORY_RO:
> + pte = pte_wrprotect(pte);
> + break;
> + case SET_MEMORY_RW:
> + pte = pte_mkwrite(pte_mkdirty(pte));
> + break;
> + case SET_MEMORY_NX:
> + pte = pte_exprotect(pte);
> + break;
> + case SET_MEMORY_X:
> + pte = pte_mkexec(pte);
> + break;
> + default:
> + WARN_ON_ONCE(1);
> + break;
> + }
> +
> + set_pte_at(&init_mm, addr, ptep, pte);
> +
> + /* See ptesync comment in radix__set_pte_at() */
> + if (radix_enabled())
> + asm volatile("ptesync": : :"memory");
> + spin_unlock(&init_mm.page_table_lock);
> +
> + return 0;
> +}
> +
> +int change_memory_attr(unsigned long addr, int numpages, long action)
> +{
> + unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
> + unsigned long size = numpages * PAGE_SIZE;
> +
> + if (!numpages)
> + return 0;
> +
> + if (WARN_ON_ONCE(is_vm_area_hugepages((void *)addr)))
> + return -EINVAL;
I think this test actually needs to be something like
if (WARN_ON_ONCE(is_vmalloc_or_module_addr((void *)addr) &&
is_vm_area_hugepages((void *)addr)))
> +
> +#ifdef CONFIG_PPC_BOOK3S_64
> + /*
> + * On hash, the linear mapping is not in the Linux page table so
> + * apply_to_existing_page_range() will have no effect. If in the future
> + * the set_memory_* functions are used on the linear map this will need
> + * to be updated.
> + */
> + if (!radix_enabled()) {
> + int region = get_region_id(addr);
> +
> + if (WARN_ON_ONCE(region != VMALLOC_REGION_ID && region != IO_REGION_ID))
> + return -EINVAL;
> + }
> +#endif
> +
> + return apply_to_existing_page_range(&init_mm, start, size,
> + change_page_attr, (void *)action);
> +}
> --
> 2.25.1
>
^ permalink raw reply
* [powerpc:merge] BUILD SUCCESS 1fab3666d738e4af3a7450c44441310e4d7a7e53
From: kernel test robot @ 2021-05-13 3:33 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge
branch HEAD: 1fab3666d738e4af3a7450c44441310e4d7a7e53 Automatic merge of 'master' into merge (2021-05-09 08:09)
elapsed time: 6060m
configs tested: 97
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
x86_64 allyesconfig
riscv allmodconfig
i386 allyesconfig
powerpc adder875_defconfig
powerpc linkstation_defconfig
powerpc lite5200b_defconfig
arm mv78xx0_defconfig
sh urquell_defconfig
sh sh7724_generic_defconfig
powerpc mpc837x_mds_defconfig
openrisc simple_smp_defconfig
m68k m5307c3_defconfig
arm cns3420vb_defconfig
m68k sun3x_defconfig
ia64 gensparse_defconfig
arm alldefconfig
s390 zfcpdump_defconfig
powerpc ppc64e_defconfig
sparc sparc64_defconfig
ia64 alldefconfig
mips ip22_defconfig
x86_64 allnoconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
i386 randconfig-a003-20210509
i386 randconfig-a006-20210509
i386 randconfig-a001-20210509
i386 randconfig-a005-20210509
i386 randconfig-a004-20210509
i386 randconfig-a002-20210509
i386 randconfig-a013-20210509
i386 randconfig-a015-20210509
i386 randconfig-a014-20210509
i386 randconfig-a016-20210509
i386 randconfig-a011-20210509
i386 randconfig-a012-20210509
x86_64 randconfig-a005-20210509
x86_64 randconfig-a003-20210509
x86_64 randconfig-a001-20210509
x86_64 randconfig-a002-20210509
x86_64 randconfig-a006-20210509
x86_64 randconfig-a004-20210509
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
um allmodconfig
um allnoconfig
um allyesconfig
um defconfig
x86_64 rhel-8.3-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kbuiltin
x86_64 kexec
clang tested configs:
x86_64 randconfig-a014-20210509
x86_64 randconfig-a015-20210509
x86_64 randconfig-a011-20210509
x86_64 randconfig-a013-20210509
x86_64 randconfig-a012-20210509
x86_64 randconfig-a016-20210509
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH 2/3] hotplug-memory.c: enhance dlpar_memory_remove* LMB checks
From: David Gibson @ 2021-05-13 5:22 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: linuxppc-dev
In-Reply-To: <c735c4ad-7477-b07a-e84b-86eccd8d9978@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3982 bytes --]
On Wed, May 12, 2021 at 05:35:39PM -0300, Daniel Henrique Barboza wrote:
>
> On 5/3/21 10:02 PM, David Gibson wrote:
> > On Fri, Apr 30, 2021 at 09:09:16AM -0300, Daniel Henrique Barboza wrote:
> > > dlpar_memory_remove_by_ic() validates the amount of LMBs to be removed
> > > by checking !DRCONF_MEM_RESERVED, and in the following loop before
> > > dlpar_remove_lmb() a check for DRCONF_MEM_ASSIGNED is made before
> > > removing it. This means that a LMB that is both !DRCONF_MEM_RESERVED and
> > > !DRCONF_MEM_ASSIGNED will be counted as valid, but then not being
> > > removed. The function will end up not removing all 'lmbs_to_remove'
> > > LMBs while also not reporting any errors.
> > >
> > > Comparing it to dlpar_memory_remove_by_count(), the validation is done
> > > via lmb_is_removable(), which checks for DRCONF_MEM_ASSIGNED and fadump
> > > constraints. No additional check is made afterwards, and
> > > DRCONF_MEM_RESERVED is never checked before dlpar_remove_lmb(). The
> > > function doesn't have the same 'check A for validation, then B for
> > > removal' issue as remove_by_ic(), but it's not checking if the LMB is
> > > reserved.
> > >
> > > There is no reason for these functions to validate the same operation in
> > > two different manners.
> >
> > Actually, I think there is: remove_by_ic() is handling a request to
> > remove a specific range of LMBs. If any are reserved, they can't be
> > removed and so this needs to fail. But if they are !ASSIGNED, that
> > essentially means they're *already* removed (or never added), so
> > "removing" them is, correctly, a no-op.
> >
> > remove_by_count(), in contrast, is being asked to remove a fixed
> > number of LMBs from wherever they can be found, and for that it needs
> > to find LMBs that haven't already been removed.
> >
> > Basically remove_by_ic() is an absolute request: "make this set of
> > LMBs be not-plugged", whereas remove_by_count() is a relative request
> > "make N less LMBs be plugged".
> >
> >
> > So I think remove_by_ic()s existing handling is correct. I'm less
> > sure if remove_by_count() ignoring RESERVED is correct - I couldn't
> > quickly find under what circumstances RESERVED gets set.
>
> RESERVED is never set by the kernel. It is written in the DT by the
> firmware/hypervisor and the kernel just checks its value. QEMU sets it in
> spapr_dt_dynamic_memory() with the following comment:
>
>
> /*
> * LMB information for RMA, boot time RAM and gap b/n RAM and
> * device memory region -- all these are marked as reserved
> * and as having no valid DRC.
> */
> dynamic_memory[0] = cpu_to_be32(addr >> 32);
> dynamic_memory[1] = cpu_to_be32(addr & 0xffffffff);
> dynamic_memory[2] = cpu_to_be32(0);
> dynamic_memory[3] = cpu_to_be32(0); /* reserved */
> dynamic_memory[4] = cpu_to_be32(-1);
> dynamic_memory[5] = cpu_to_be32(SPAPR_LMB_FLAGS_RESERVED |
> SPAPR_LMB_FLAGS_DRC_INVALID);
>
>
> The flag is formally described in LOPAR section 4.2.8, "Reserved Memory":
>
> "Memory nodes marked with the special value of the “status” property of
> “reserved” is not to be used or altered by the base OS."
>
>
> This makes me confident that we should check DRCONF_MEM_RESERVED in
> remove_by_count() as well, since phyp needs do adhere to these semantics and
> shouldn't be able to remove a LMB marked as RESERVED.
Right. I doubt it would have caused a problem in practice, because
I'm pretty sure we should never get an LMB which is RESERVED &&
ASSIGNED, but it's probably safer to make it explicit.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/4] powerpc/pseries: check DRCONF_MEM_RESERVED in lmb_is_removable()
From: David Gibson @ 2021-05-13 5:12 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: linuxppc-dev
In-Reply-To: <20210512202809.95363-3-danielhb413@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2355 bytes --]
On Wed, May 12, 2021 at 05:28:07PM -0300, Daniel Henrique Barboza wrote:
> DRCONF_MEM_RESERVED is a flag that represents the "Reserved Memory"
> status in LOPAR v2.10, section 4.2.8. If a LMB is marked as reserved,
> quoting LOPAR, "is not to be used or altered by the base OS". This flag
> is read only in the kernel, being set by the firmware/hypervisor in the
> DT. As an example, QEMU will set this flag in hw/ppc/spapr.c,
> spapr_dt_dynamic_memory().
>
> lmb_is_removable() does not check for DRCONF_MEM_RESERVED. This function
> is used in dlpar_remove_lmb() as a guard before the removal logic. Since
> it is failing to check for !RESERVED, dlpar_remove_lmb() will fail in a
> later stage instead of failing in the validation when receiving a
> reserved LMB as input.
>
> lmb_is_removable() is also used in dlpar_memory_remove_by_count() to
> evaluate if we have enough LMBs to complete the request. The missing
> !RESERVED check in this case is causing dlpar_memory_remove_by_count()
> to miscalculate the number of elegible LMBs for the removal, and can
> make it error out later on instead of failing in the validation with the
> 'not enough LMBs to satisfy request' message.
>
> Making a DRCONF_MEM_RESERVED check in lmb_is_removable() fixes all these
> issues.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> arch/powerpc/platforms/pseries/hotplug-memory.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index bb98574a84a2..c21d9278c1ce 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -348,7 +348,8 @@ static int pseries_remove_mem_node(struct device_node *np)
>
> static bool lmb_is_removable(struct drmem_lmb *lmb)
> {
> - if (!(lmb->flags & DRCONF_MEM_ASSIGNED))
> + if ((lmb->flags & DRCONF_MEM_RESERVED) ||
> + !(lmb->flags & DRCONF_MEM_ASSIGNED))
> return false;
>
> #ifdef CONFIG_FA_DUMP
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 4/4] powerpc/pseries: minor enhancements in dlpar_memory_remove_by_ic()
From: David Gibson @ 2021-05-13 5:21 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: linuxppc-dev
In-Reply-To: <20210512202809.95363-5-danielhb413@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2964 bytes --]
On Wed, May 12, 2021 at 05:28:09PM -0300, Daniel Henrique Barboza wrote:
> We don't need the 'lmbs_available' variable to count the valid LMBs and
> to check if we have less than 'lmbs_to_remove'. We must ensure that the
> entire LMB range must be removed, so we can error out immediately if any
> LMB in the range is marked as reserved.
>
> Add a couple of comments explaining the reasoning behind the differences
> we have in this function in contrast to what it is done in its sister
> function, dlpar_memory_remove_by_count().
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> .../platforms/pseries/hotplug-memory.c | 28 +++++++++++++------
> 1 file changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index 3c7ce5361ce3..ee88c1540fba 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -517,7 +517,6 @@ static int dlpar_memory_remove_by_index(u32 drc_index)
> static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
> {
> struct drmem_lmb *lmb, *start_lmb, *end_lmb;
> - int lmbs_available = 0;
> int rc;
>
> pr_info("Attempting to hot-remove %u LMB(s) at %x\n",
> @@ -530,18 +529,29 @@ static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
> if (rc)
> return -EINVAL;
>
> - /* Validate that there are enough LMBs to satisfy the request */
> + /*
> + * Validate that all LMBs in range are not reserved. Note that it
> + * is ok if they are !ASSIGNED since our goal here is to remove the
> + * LMB range, regardless of whether some LMBs were already removed
> + * by any other reason.
> + *
> + * This is a contrast to what is done in remove_by_count() where we
> + * check for both RESERVED and !ASSIGNED (via lmb_is_removable()),
> + * because we want to remove a fixed amount of LMBs in that function.
> + */
> for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
> - if (lmb->flags & DRCONF_MEM_RESERVED)
> - break;
> -
> - lmbs_available++;
> + if (lmb->flags & DRCONF_MEM_RESERVED) {
> + pr_err("Memory at %llx (drc index %x) is reserved\n",
> + lmb->base_addr, lmb->drc_index);
> + return -EINVAL;
> + }
> }
>
> - if (lmbs_available < lmbs_to_remove)
> - return -EINVAL;
> -
> for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
> + /*
> + * dlpar_remove_lmb() will error out if the LMB is already
> + * !ASSIGNED, but this case is a no-op for us.
> + */
> if (!(lmb->flags & DRCONF_MEM_ASSIGNED))
> continue;
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 3/4] powerpc/pseries: break early in dlpar_memory_remove_by_count() loops
From: David Gibson @ 2021-05-13 5:20 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: linuxppc-dev
In-Reply-To: <20210512202809.95363-4-danielhb413@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2922 bytes --]
On Wed, May 12, 2021 at 05:28:08PM -0300, Daniel Henrique Barboza wrote:
> After marking the LMBs as reserved depending on dlpar_remove_lmb() rc,
> we evaluate whether we need to add the LMBs back or if we can release
> the LMB DRCs. In both cases, a for_each_drmem_lmb() loop without a break
> condition is used. This means that we're going to cycle through all LMBs
> of the partition even after we're done with what we were going to do.
>
> This patch adds break conditions in both loops to avoid this. The
> 'lmbs_removed' variable was renamed to 'lmbs_reserved', and it's now
> being decremented each time a lmb reservation is removed, indicating
> that the operation we're doing (adding back LMBs or releasing DRCs) is
> completed.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
The fact that DRCONF_MEM_RESERVED and DRMEM_LMB_RESERVED look so
similar but have totally different meanings doesn't make this easy to
follow :/.
> ---
> arch/powerpc/platforms/pseries/hotplug-memory.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index c21d9278c1ce..3c7ce5361ce3 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -402,7 +402,7 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
> static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
> {
> struct drmem_lmb *lmb;
> - int lmbs_removed = 0;
> + int lmbs_reserved = 0;
> int lmbs_available = 0;
> int rc;
>
> @@ -436,12 +436,12 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
> */
> drmem_mark_lmb_reserved(lmb);
>
> - lmbs_removed++;
> - if (lmbs_removed == lmbs_to_remove)
> + lmbs_reserved++;
> + if (lmbs_reserved == lmbs_to_remove)
> break;
> }
>
> - if (lmbs_removed != lmbs_to_remove) {
> + if (lmbs_reserved != lmbs_to_remove) {
> pr_err("Memory hot-remove failed, adding LMB's back\n");
>
> for_each_drmem_lmb(lmb) {
> @@ -454,6 +454,10 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
> lmb->drc_index);
>
> drmem_remove_lmb_reservation(lmb);
> +
> + lmbs_reserved--;
> + if (lmbs_reserved == 0)
> + break;
> }
>
> rc = -EINVAL;
> @@ -467,6 +471,10 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
> lmb->base_addr);
>
> drmem_remove_lmb_reservation(lmb);
> +
> + lmbs_reserved--;
> + if (lmbs_reserved == 0)
> + break;
> }
> rc = 0;
> }
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christian Zigotzky @ 2021-05-13 6:47 UTC (permalink / raw)
To: Christophe Leroy
Cc: Darren Stevens, R.T.Dickinson, mad skateman, linuxppc-dev,
Christian Zigotzky
In-Reply-To: <72e138cd-5201-a059-e8aa-cebc3db4029c@csgroup.eu>
Hi Christophe,
> On 9. May 2021, at 19:37, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>
> Did I do an error in my analysis ?
No, you didn’t. On the contrary you have found the issue. ;-) The issue is somewhere in the new interrupt code.
> ZZ | * | ceff77efa4f8 powerpc/64e/interrupt: Use new interrupt context tracking scheme
Could you please create a patch for reverting the new interrupt code? I would like to confirm your result.
Thanks for your help,
Christian
>
> BAD * c70a4be130de Merge tag 'powerpc-5.13-1' of git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
> |\
> | * 525642624783 powerpc/signal32: Fix erroneous SIGSEGV on RT signal return
> | * f9cd5f91a897 powerpc: Avoid clang uninitialized warning in __get_user_size_allowed
> | * adb68c38d8d4 powerpc/papr_scm: Mark nvdimm as unarmed if needed during probe
> | * ee1bc694fbae powerpc/kvm: Fix build error when PPC_MEM_KEYS/PPC_PSERIES=n
> | * 30c400886bad powerpc/kasan: Fix shadow start address with modules
> | * fc5590fd56c9 powerpc/kernel/iommu: Use largepool as a last resort when !largealloc
> | * 3c0468d4451e powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE() to save TCEs
> | * ee6b25fa7c03 powerpc/44x: fix spelling mistake in Kconfig "varients" -> "variants"
> | * cc7130bf119a powerpc/iommu: Annotate nested lock for lockdep
> | * 4be518d83880 powerpc/iommu: Do not immediately panic when failed IOMMU table allocation
> | * 7f1fa82d7994 powerpc/iommu: Allocate it_map by vmalloc
> | * 0db11461677a selftests/powerpc: remove unneeded semicolon
> | * caea7b833d86 powerpc/64s: remove unneeded semicolon
> | * f3d03fc748d4 powerpc/eeh: remove unneeded semicolon
> | * 290f7d8ce2b1 powerpc/selftests: Add selftest to test concurrent perf/ptrace events
> | * c65c64cc7bbd powerpc/selftests/perf-hwbreak: Add testcases for 2nd DAWR
> | * c9cb0afb4eaa powerpc/selftests/perf-hwbreak: Coalesce event creation code
> | * dae4ff8031b4 powerpc/selftests/ptrace-hwbreak: Add testcases for 2nd DAWR
> | * 421a7483878c powerpc/configs: Add IBMVNIC to some 64-bit configs
> | * da650ada1009 selftests/powerpc: Add uaccess flush test
> | * 8a87a5077143 powerpc/52xx: Fix an invalid ASM expression ('addi' used instead of 'add')
> | * 0f197ddce403 powerpc/64s: Fix mm_cpumask memory ordering comment
> | * 66d9b7492887 powerpc/perf: Fix the threshold event selection for memory events in power10
> | * b4ded42268ee powerpc/perf: Fix sampled instruction type for larx/stcx
> | * 0bd3f9e953bd powerpc/legacy_serial: Use early_ioremap()
> | * 9ccba66d4d2a powerpc/64: Fix the definition of the fixmap area
> | * 389586333c02 powerpc: make ALTIVEC select PPC_FPU
> | * 7d9462765707 powerpc/64s: Add FA_DUMP to defconfig
> | * d936f8182e1b powerpc/powernv: Fix type of opal_mpipl_query_tag() addr argument
> | * 2e341f56a16a powerpc/fadump: Fix sparse warnings
> | * 39352430aaa0 powerpc: Move copy_inst_from_kernel_nofault()
> | * 41d6cf68b5f6 powerpc: Rename probe_kernel_read_inst()
> | * 6449078d5011 powerpc: Make probe_kernel_read_inst() common to PPC32 and PPC64
> | * 6ac7897f08e0 powerpc: Remove probe_user_read_inst()
> | * ee7c3ec3b4b1 powerpc/ebpf32: Use standard function call for functions within 32M distance
> | * e7de0023e123 powerpc/ebpf32: Rework 64 bits shifts to avoid tests and branches
> | * d228cc496966 powerpc/ebpf32: Fix comment on BPF_ALU{64} | BPF_LSH | BPF_K
> | * 867e762480f4 powerpc/32: Use r2 in wrtspr() instead of r0
> | * f56607e85ee3 selftests/timens: Fix gettime_perf to work on powerpc
> | * 92d9d61be519 powerpc/mce: save ignore_event flag unconditionally for UE
> | * eacf4c020265 powerpc: Enable OPTPROBES on PPC32
> | * 693557ebf407 powerpc/inst: ppc_inst_as_u64() becomes ppc_inst_as_ulong()
> | * e522331173ec powerpc/irq: Enhance readability of trap types
> | * 7fab639729ce powerpc/32s: Enhance readability of trap types
> | * 0f5eb28a6ce6 powerpc/8xx: Enhance readability of trap types
> | * a9d2f9bb225f powerpc/pseries/iommu: Fix window size for direct mapping with pmem
> | * e4e8bc1df691 powerpc/kvm: Fix PR KVM with KUAP/MEM_KEYS enabled
> | * ed8029d7b472 powerpc/pseries: Stop calling printk in rtas_stop_self()
> | * 3027a37c06be powerpc: Only define _TASK_CPU for 32-bit
> | * 39d0099f9439 powerpc/pseries: Add shutdown() to vio_driver and vio_bus
> | * af31fd0c9107 powerpc/perf: Expose processor pipeline stage cycles using PERF_SAMPLE_WEIGHT_STRUCT
> | * 2886e2df10be Documentation/powerpc: Add proper links for manual and tests
> | * 29c9a2699e71 powerpc/pseries: Set UNISOLATE on dlpar_cpu_remove() failure
> | * 0e3b3ff83ce2 powerpc/pseries: Introduce dlpar_unisolate_drc()
> | * 864ec4d40c83 powerpc/pseries/mce: Fix a typo in error type assignment
> | * cbd3d5ba46b6 powerpc/fadump: Fix compile error since trap type change
> | * d8a1d6c58986 powerpc/perf: Add platform specific check_attr_config
> | * a38cb4171928 Merge branch 'topic/ppc-kvm' into next
> | |\
> | | * 732f21a3053c KVM: PPC: Book3S HV: Ensure MSR[HV] is always clear in guest MSR
> | | * 946cf44ac6ce KVM: PPC: Book3S HV: Ensure MSR[ME] is always set in guest MSR
> | | * da487a5d1bee powerpc/64s: remove KVM SKIP test from instruction breakpoint handler
> | | * 5eee8371828a powerpc/64s: Remove KVM handler support from CBE_RAS interrupts
> | | * 0fd85cb83fbd KVM: PPC: Book3S HV: Fix CONFIG_SPAPR_TCE_IOMMU=n default hcalls
> | | * 6c12c4376bbb KVM: PPC: Book3S HV: remove unused kvmppc_h_protect argument
> | | * 4b5f0a0d49e6 KVM: PPC: Book3S HV: Remove redundant mtspr PSPB
> | | * 72c15287210f KVM: PPC: Book3S HV: Prevent radix guests setting LPCR[TC]
> | | * bcc92a0d6d6e KVM: PPC: Book3S HV: Disallow LPCR[AIL] to be set to 1 or 2
> | | * 67145ef4960f KVM: PPC: Book3S HV: Add a function to filter guest LPCR bits
> | | * a19b70abc69a KVM: PPC: Book3S HV: Nested move LPCR sanitising to sanitise_hv_regs
> | | * 5088eb4092df KVM: PPC: Book3S HV P9: Restore host CTRL SPR after guest exit
> BAD | * | 49c1d07fd04f powerpc/powernv: Enable HAIL (HV AIL) for ISA v3.1 processors
> | * | 6980d13f0dd1 powerpc/smp: Set numa node before updating mask
> | * | 7153d4bf0b37 powerpc/traps: Enhance readability for trap types
> | * | 7de21e679e6a powerpc: fix EDEADLOCK redefinition error in uapi/asm/errno.h
> | * | c1e53367dab1 powerpc/smp: Cache CPU to chip lookup
> | * | 131c82b6a1d2 Revert "powerpc/topology: Update topology_core_cpumask"
> | * | c47f892d7aa6 powerpc/smp: Reintroduce cpu_core_mask
> | * | e9e16917bc38 powerpc/xive: Use the "ibm, chip-id" property only under PowerNV
> | * | 38d0b1c9cec7 powerpc/pseries: extract host bridge from pci_bus prior to bus removal
> | * | 0751fdf28041 macintosh/via-pmu: Fix build warning
> | * | 7767d9ac89ce powerpc/papr_scm: Fix build error due to wrong printf specifier
> | * | d6481a7195df powerpc/configs: Add PAPR_SCM to pseries_defconfig
> | * | 7098f8f0cf03 powerpc/mm/radix: Make radix__change_memory_range() static
> | * | 74205b3fc2ef powerpc/vdso: Add support for time namespaces
> | * | 1c4bce675385 powerpc/vdso: Separate vvar vma from vdso
> | * | 808094fcbf41 lib/vdso: Add vdso_data pointer as input to __arch_get_timens_vdso_data()
> | * | 58efe9f696cf lib/vdso: Mark do_hres_timens() and do_coarse_timens() __always_inline()
> | * | 8f6cc75a97d1 powerpc: move norestart trap flag to bit 0
> | * | 8dc7f0229b78 powerpc: remove partial register save logic
> | * | c45ba4f44f6b powerpc: clean up do_page_fault
> | * | d738ee8d56de powerpc/64e/interrupt: handle bad_page_fault in C
> ZZ | * | ceff77efa4f8 powerpc/64e/interrupt: Use new interrupt context tracking scheme
> ZZ | * | 097157e16cf8 powerpc/64e/interrupt: reconcile irq soft-mask state in C
> | * | 3db8aa10de9a powerpc/64e/interrupt: NMI save irq soft-mask state in C
> ZZ | * | 0c2472de23ae powerpc/64e/interrupt: use new interrupt return
> | * | dc6231821a14 powerpc/interrupt: update common interrupt code for
> | * | 4228b2c3d20e powerpc/64e/interrupt: always save nvgprs on interrupt
> | * | 5a5a893c4ad8 powerpc/syscall: switch user_exit_irqoff and trace_hardirqs_off order
> ZZ | * | 2e2a441d2c0b powerpc/perf: Infrastructure to support checking of attr.config*
> | * | 59fd366b9bef powerpc/fadump: make symbol 'rtas_fadump_set_regval' static
> | * | 7e9ab144c128 powerpc/mem: Use kmap_local_page() in flushing functions
> | * | 6c96020882b1 powerpc/mem: Inline flush_dcache_page()
> | * | 67b8e6af191a powerpc/mem: Help GCC realise __flush_dcache_icache() flushes single pages
> | * | 52d490437ffb powerpc/mem: flush_dcache_icache_phys() is for HIGHMEM pages only
> | * | cd97d9e8b5aa powerpc/mem: Optimise flush_dcache_icache_hugepage()
> | * | e618c7aea1f2 powerpc/mem: Call flush_coherent_icache() at higher level
> | * | 131637a17dc9 powerpc/mem: Remove address argument to flush_coherent_icache()
> | * | bf26e0bbd2f8 powerpc/mem: Declare __flush_dcache_icache() static
> | * | b26e8f27253a powerpc/mem: Move cache flushing functions into mm/cacheflush.c
> | * | ff0b4155ae99 powerpc/powernv: make symbol 'mpipl_kobj' static
> | * | f234ad405a35 powerpc/xmon: Make symbol 'spu_inst_dump' static
> | * | cc331eee03ea powerpc/perf/hv-24x7: Make some symbols static
> ZZZ | * | 107dadb04617 powerpc/perf: Make symbol 'isa207_pmu_format_attr' static
> ZZZ | * | 2235dea17d56 powerpc/pseries/pmem: Make symbol 'drc_pmem_match' static
> | * | 193e4cd8ed9d powerpc/pseries: Make symbol '__pcpu_scope_hcall_stats' static
> ZZ | * | 472724111f0f powerpc/iommu: Enable remaining IOMMU Pagesizes present in LoPAR
> | * | 672bff581e19 powerpc/syscalls: switch to generic syscallhdr.sh
> XX | * | 14b3c9d24a7a powerpc/syscalls: switch to generic syscalltbl.sh
> ZZ | * | e5d56763525e powerpc/rtas: rename RTAS_RMOBUF_MAX to RTAS_USER_REGION_SIZE
> ZZ | * | 0649cdc82379 powerpc/rtas: move syscall filter setup into separate function
> | * | 0ab1c929ae38 powerpc/rtas: remove ibm_suspend_me_token
> ZZZ | * | 01c1b9984a12 powerpc/rtas-proc: remove unused RMO_READ_BUF_MAX
> ZZ | * | c13ff6f32513 powerpc/rtas: improve ppc_rtas_rmo_buf_show documentation
> | * | 5ae5bc12d072 powerpc/eeh: Fix EEH handling for hugepages in ioremap space.
> XX | * | fd6db2892eba powerpc/xive: Modernize XIVE-IPI domain with an 'alloc' handler
> ZZ | * | 7dcc37b3eff9 powerpc/xive: Map one IPI interrupt per node
> | * | 33e4bc594643 powerpc/xive: Fix xmon command "dxi"
> | * | 6bf66eb8f404 powerpc/xive: Simplify the dump of XIVE interrupts under xmon
> | * | a74ce5926b20 powerpc/xive: Drop check on irq_data in xive_core_debug_show()
> | * | 5159d9872823 powerpc/xive: Simplify xive_core_debug_show()
> | * | 1835e72942b5 powerpc/xive: Remove useless check on XIVE_IPI_HW_IRQ
> | * | 7d348494136c powerpc/xive: Introduce an IPI interrupt domain
> | * | 078277acbd7c powerpc/smp: Make some symbols static
> | * | 95d143923379 macintosh/via-pmu: Make some symbols static
> | * | 4204ecd598cb windfarm: make symbol 'wf_thread' static
> | * | 13ddd0e3acf9 macintosh/windfarm: Make symbol 'pm121_sys_state' static
> | * | f6f1f48e8b3b powerpc/mce: Make symbol 'mce_ue_event_work' static
> | * | 7f262b4dcf7e powerpc/security: Make symbol 'stf_barrier' static
> | * | 80edc68e0479 powerpc/32s: Define a MODULE area below kernel text all the time
> | * | 9132a2e82adc powerpc/8xx: Define a MODULE area below kernel text
> | * | 2ec13df16704 powerpc/modules: Load modules closer to kernel text
> | * | a5d6a3e73acb powerpc/mm: Add cond_resched() while removing hpte mappings
> | * | 75b7c05ebf90 powerpc/papr_scm: Implement support for H_SCM_FLUSH hcall
> | * | af072b1a9d4d powerpc/signal32: Fix build failure with CONFIG_SPE
> | * | c46bbf5d2def powerpc/32: Remove powerpc specific definition of 'ptrdiff_t'
> | * | b27dadecdf91 powerpc: iommu: fix build when neither PCI or IBMVIO is set
> | * | 01ed0510941a powerpc/pseries: remove unneeded semicolon
> ZZ | * | 98db179a78dd powerpc/64s: power4 nap fixup in C
> | * | 10f8f96179ec powerpc/perf: Fix PMU constraint check for EBB events
> | * | 812aa68ef7d4 selftests/powerpc: Suggest memtrace instead of /dev/mem for ci memory
> ZZ | * | 08a022ad3dfa powerpc/powernv/memtrace: Allow mmaping trace buffers
> | * | acd4dfeb49c8 powerpc/kexec: Don't use .machine ppc64 in trampoline_64.S
> | * | c6b4c9147f8b powerpc/64: Move security code into security.c
> | * | bd573a81312f powerpc/mm/64s: Allow STRICT_KERNEL_RWX again
> | * | 87e65ad7bd3a powerpc/mm/64s/hash: Add real-mode change_memory_range() for hash LPAR
> | * | 6f223ebe9c3f powerpc/mm/64s/hash: Factor out change_memory_range()
> ZZZ | * | 2c02e656a29d powerpc/64s: Use htab_convert_pte_flags() in hash__mark_rodata_ro()
> | * | b56d55a5aa4a powerpc/pseries: Add key to flags in pSeries_lpar_hpte_updateboltedpp()
> ZZZ | * | 56bec2f9d4d0 powerpc/mm/64s: Add _PAGE_KERNEL_ROX
> | * | 29e3ea8cbd29 selftests/powerpc: Test for spurious kernel memory faults on radix
> | * | b8b2f37cf632 powerpc/64s: Fix pte update for kernel memory on radix
> | * | 4763d3782764 powerpc: Spelling/typo fixes
> | * | b0b3b2c78ec0 powerpc: Switch to relative jump labels
> | * | 40272035e1d0 powerpc/bpf: Reallocate BPF registers to volatile registers when possible on PPC32
> | * | 51c66ad849a7 powerpc/bpf: Implement extended BPF on PPC32
> | * | 355a8d26cd04 powerpc/asm: Add some opcodes in asm/ppc-opcode.h for PPC32 eBPF
> | * | c426810fcf9f powerpc/bpf: Change values of SEEN_ flags
> | * | 4ea76e90a97d powerpc/bpf: Move common functions into bpf_jit_comp.c
> | * | f1b1583d5faa powerpc/bpf: Move common helpers into bpf_jit.h
> | * | ed573b57e77a powerpc/bpf: Change register numbering for bpf_set/is_seen_register()
> | * | 6944caad78fc powerpc/bpf: Remove classical BPF support for PPC32
> | * | c7393a71eb1a powerpc/signal32: Simplify logging in sigreturn()
> BAD? | * | 887f3ceb51cd powerpc/signal32: Convert do_setcontext[_tm]() to user access block
> GOOD | * | 627b72bee84d powerpc/signal32: Convert restore_[tm]_user_regs() to user access block
> | * | 036fc2cb1dc2 powerpc/signal32: Reorder user reads in restore_tm_user_regs()
>
>
> Christophe
^ permalink raw reply
* [PATCH v2 1/2] kprobes: Allow architectures to override optinsn page allocation
From: Christophe Leroy @ 2021-05-13 9:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
naveen.n.rao, anil.s.keshavamurthy, davem, mhiramat
Cc: clang-built-linux, linuxppc-dev, linux-kernel
Some architectures like powerpc require a non standard
allocation of optinsn page, because module pages are
too far from the kernel for direct branches.
Define weak alloc_optinsn_page() and free_optinsn_page(), that
fall back on alloc_insn_page() and free_insn_page() when not
overriden by the architecture.
Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
---
v2: Added missing prototypes in linux/kprobes.h
---
include/linux/kprobes.h | 3 +++
kernel/kprobes.c | 14 ++++++++++++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 1883a4a9f16a..02d4020615a7 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -409,6 +409,9 @@ void dump_kprobe(struct kprobe *kp);
void *alloc_insn_page(void);
void free_insn_page(void *page);
+void *alloc_optinsn_page(void);
+void free_optinsn_page(void *page);
+
int kprobe_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
char *sym);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 745f08fdd7a6..8c0a6fdef771 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -321,11 +321,21 @@ int kprobe_cache_get_kallsym(struct kprobe_insn_cache *c, unsigned int *symnum,
}
#ifdef CONFIG_OPTPROBES
+void __weak *alloc_optinsn_page(void)
+{
+ return alloc_insn_page();
+}
+
+void __weak free_optinsn_page(void *page)
+{
+ free_insn_page(page);
+}
+
/* For optimized_kprobe buffer */
struct kprobe_insn_cache kprobe_optinsn_slots = {
.mutex = __MUTEX_INITIALIZER(kprobe_optinsn_slots.mutex),
- .alloc = alloc_insn_page,
- .free = free_insn_page,
+ .alloc = alloc_optinsn_page,
+ .free = free_optinsn_page,
.sym = KPROBE_OPTINSN_PAGE_SYM,
.pages = LIST_HEAD_INIT(kprobe_optinsn_slots.pages),
/* .insn_size is initialized later */
--
2.25.0
^ permalink raw reply related
* [PATCH v2 2/2] powerpc/kprobes: Replace ppc_optinsn by common optinsn
From: Christophe Leroy @ 2021-05-13 9:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
naveen.n.rao, anil.s.keshavamurthy, davem, mhiramat
Cc: clang-built-linux, linuxppc-dev, linux-kernel
In-Reply-To: <40a43d6df1fdf41ade36e9a46e60a4df774ca9f6.1620896780.git.christophe.leroy@csgroup.eu>
Commit 51c9c0843993 ("powerpc/kprobes: Implement Optprobes")
implemented a powerpc specific version of optinsn in order
to workaround the 32Mb limitation for direct branches.
Instead of implementing a dedicated powerpc version, use the
common optinsn and override the allocation and freeing functions.
This also indirectly remove the CLANG warning about
is_kprobe_ppc_optinsn_slot() not being use, and the powerpc will
now benefit from commit 5b485629ba0d ("kprobes, extable: Identify
kprobes trampolines as kernel text area")
Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
---
v2: no change
---
arch/powerpc/kernel/optprobes.c | 23 +++++------------------
1 file changed, 5 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index cdf87086fa33..a370190cd02a 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -31,11 +31,9 @@
#define TMPL_END_IDX \
(optprobe_template_end - optprobe_template_entry)
-DEFINE_INSN_CACHE_OPS(ppc_optinsn);
-
static bool insn_page_in_use;
-static void *__ppc_alloc_insn_page(void)
+void *alloc_optinsn_page(void)
{
if (insn_page_in_use)
return NULL;
@@ -43,20 +41,11 @@ static void *__ppc_alloc_insn_page(void)
return &optinsn_slot;
}
-static void __ppc_free_insn_page(void *page __maybe_unused)
+void free_optinsn_page(void *page)
{
insn_page_in_use = false;
}
-struct kprobe_insn_cache kprobe_ppc_optinsn_slots = {
- .mutex = __MUTEX_INITIALIZER(kprobe_ppc_optinsn_slots.mutex),
- .pages = LIST_HEAD_INIT(kprobe_ppc_optinsn_slots.pages),
- /* insn_size initialized later */
- .alloc = __ppc_alloc_insn_page,
- .free = __ppc_free_insn_page,
- .nr_garbage = 0,
-};
-
/*
* Check if we can optimize this probe. Returns NIP post-emulation if this can
* be optimized and 0 otherwise.
@@ -136,7 +125,7 @@ NOKPROBE_SYMBOL(optimized_callback);
void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
{
if (op->optinsn.insn) {
- free_ppc_optinsn_slot(op->optinsn.insn, 1);
+ free_optinsn_slot(op->optinsn.insn, 1);
op->optinsn.insn = NULL;
}
}
@@ -203,14 +192,12 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
unsigned long nip, size;
int rc, i;
- kprobe_ppc_optinsn_slots.insn_size = MAX_OPTINSN_SIZE;
-
nip = can_optimize(p);
if (!nip)
return -EILSEQ;
/* Allocate instruction slot for detour buffer */
- buff = get_ppc_optinsn_slot();
+ buff = get_optinsn_slot();
if (!buff)
return -ENOMEM;
@@ -297,7 +284,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
return 0;
error:
- free_ppc_optinsn_slot(buff, 0);
+ free_optinsn_slot(buff, 0);
return -ERANGE;
}
--
2.25.0
^ permalink raw reply related
* [PATCH v3] powerpc/papr_scm: Make 'perf_stats' invisible if perf-stats unavailable
From: Vaibhav Jain @ 2021-05-13 9:23 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Santosh Sivaraj, Aneesh Kumar K . V, Vaibhav Jain, Dan Williams,
Ira Weiny
In case performance stats for an nvdimm are not available, reading the
'perf_stats' sysfs file returns an -ENOENT error. A better approach is
to make the 'perf_stats' file entirely invisible to indicate that
performance stats for an nvdimm are unavailable.
So this patch updates 'papr_nd_attribute_group' to add a 'is_visible'
callback implemented as newly introduced 'papr_nd_attribute_visible()'
that returns an appropriate mode in case performance stats aren't
supported in a given nvdimm.
Also the initialization of 'papr_scm_priv.stat_buffer_len' is moved
from papr_scm_nvdimm_init() to papr_scm_probe() so that it value is
available when 'papr_nd_attribute_visible()' is called during nvdimm
initialization.
Even though 'perf_stats' attribute is available since v5.9, there are
no known user-space tools/scripts that are dependent on presence of its
sysfs file. Hence I dont expect any user-space breakage with this
patch.
Fixes: 2d02bf835e57('powerpc/papr_scm: Fetch nvdimm performance stats from PHYP')
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
v3:
* Minor spell corrections [ Dan Williams ]
* Switched to kobj_to_dev() helper in papr_nd_attribute_visible() [
Dan Williams ]
* Updated ABI/../sysfs-bus-papr-pmem to indicate that the attribute is
only available for devices that support performance stats.
* Updated patch description.
v2:
* Removed a redundant dev_info() from pap_scm_nvdimm_init() [ Aneesh ]
* Marked papr_nd_attribute_visible() as static which also fixed the
build warning reported by kernel build robot
---
Documentation/ABI/testing/sysfs-bus-papr-pmem | 8 +++--
arch/powerpc/platforms/pseries/papr_scm.c | 35 +++++++++++++------
2 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-papr-pmem b/Documentation/ABI/testing/sysfs-bus-papr-pmem
index 8316c33862a0..0aa02bf2bde5 100644
--- a/Documentation/ABI/testing/sysfs-bus-papr-pmem
+++ b/Documentation/ABI/testing/sysfs-bus-papr-pmem
@@ -39,9 +39,11 @@ KernelVersion: v5.9
Contact: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, linux-nvdimm@lists.01.org,
Description:
(RO) Report various performance stats related to papr-scm NVDIMM
- device. Each stat is reported on a new line with each line
- composed of a stat-identifier followed by it value. Below are
- currently known dimm performance stats which are reported:
+ device. This attribute is only available for NVDIMM devices
+ that support reporting NVDIMM performance stats. Each stat is
+ reported on a new line with each line composed of a
+ stat-identifier followed by it value. Below are currently known
+ dimm performance stats which are reported:
* "CtlResCt" : Controller Reset Count
* "CtlResTm" : Controller Reset Elapsed Time
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index e2b69cc3beaf..e8577e7e49ab 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -907,6 +907,20 @@ static ssize_t flags_show(struct device *dev,
}
DEVICE_ATTR_RO(flags);
+static umode_t papr_nd_attribute_visible(struct kobject *kobj,
+ struct attribute *attr, int n)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct nvdimm *nvdimm = to_nvdimm(dev);
+ struct papr_scm_priv *p = nvdimm_provider_data(nvdimm);
+
+ /* For if perf-stats not available remove perf_stats sysfs */
+ if (attr == &dev_attr_perf_stats.attr && p->stat_buffer_len == 0)
+ return 0;
+
+ return attr->mode;
+}
+
/* papr_scm specific dimm attributes */
static struct attribute *papr_nd_attributes[] = {
&dev_attr_flags.attr,
@@ -916,6 +930,7 @@ static struct attribute *papr_nd_attributes[] = {
static struct attribute_group papr_nd_attribute_group = {
.name = "papr",
+ .is_visible = papr_nd_attribute_visible,
.attrs = papr_nd_attributes,
};
@@ -931,7 +946,6 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
struct nd_region_desc ndr_desc;
unsigned long dimm_flags;
int target_nid, online_nid;
- ssize_t stat_size;
p->bus_desc.ndctl = papr_scm_ndctl;
p->bus_desc.module = THIS_MODULE;
@@ -1016,16 +1030,6 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
list_add_tail(&p->region_list, &papr_nd_regions);
mutex_unlock(&papr_ndr_lock);
- /* Try retriving the stat buffer and see if its supported */
- stat_size = drc_pmem_query_stats(p, NULL, 0);
- if (stat_size > 0) {
- p->stat_buffer_len = stat_size;
- dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
- p->stat_buffer_len);
- } else {
- dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n");
- }
-
return 0;
err: nvdimm_bus_unregister(p->bus);
@@ -1102,6 +1106,7 @@ static int papr_scm_probe(struct platform_device *pdev)
u64 blocks, block_size;
struct papr_scm_priv *p;
const char *uuid_str;
+ ssize_t stat_size;
u64 uuid[2];
int rc;
@@ -1179,6 +1184,14 @@ static int papr_scm_probe(struct platform_device *pdev)
p->res.name = pdev->name;
p->res.flags = IORESOURCE_MEM;
+ /* Try retrieving the stat buffer and see if its supported */
+ stat_size = drc_pmem_query_stats(p, NULL, 0);
+ if (stat_size > 0) {
+ p->stat_buffer_len = stat_size;
+ dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
+ p->stat_buffer_len);
+ }
+
rc = papr_scm_nvdimm_init(p);
if (rc)
goto err2;
--
2.31.1
^ permalink raw reply related
* [powerpc:next-test] BUILD SUCCESS d64ef8451fbc85489928bea033a63d9c09fca786
From: kernel test robot @ 2021-05-13 9:23 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test
branch HEAD: d64ef8451fbc85489928bea033a63d9c09fca786 powerpc/32s: Remove asm/book3s/32/hash.h
elapsed time: 1129m
configs tested: 9
configs skipped: 83
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 allyesconfig
x86_64 rhel-8.3-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kbuiltin
x86_64 kexec
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH v2] powerpc/papr_scm: Make 'perf_stats' invisible if perf-stats unavailable
From: Vaibhav Jain @ 2021-05-13 9:26 UTC (permalink / raw)
To: Dan Williams
Cc: Santosh Sivaraj, Ira Weiny, linux-nvdimm, Aneesh Kumar K . V,
linuxppc-dev
In-Reply-To: <CAPcyv4g7E39Hqe+fuv-J3gX1kiNVuenUfV4td_k9QtCNMV=C1w@mail.gmail.com>
Thanks for looking into this patch Dan,
Dan Williams <dan.j.williams@intel.com> writes:
> On Fri, May 7, 2021 at 4:40 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>>
>> In case performance stats for an nvdimm are not available, reading the
>> 'perf_stats' sysfs file returns an -ENOENT error. A better approach is
>> to make the 'perf_stats' file entirely invisible to indicate that
>> performance stats for an nvdimm are unavailable.
>>
>> So this patch updates 'papr_nd_attribute_group' to add a 'is_visible'
>> callback implemented as newly introduced 'papr_nd_attribute_visible()'
>> that returns an appropriate mode in case performance stats aren't
>> supported in a given nvdimm.
>>
>> Also the initialization of 'papr_scm_priv.stat_buffer_len' is moved
>> from papr_scm_nvdimm_init() to papr_scm_probe() so that it value is
>> available when 'papr_nd_attribute_visible()' is called during nvdimm
>> initialization.
>>
>> Fixes: 2d02bf835e57('powerpc/papr_scm: Fetch nvdimm performance stats from PHYP')
>
> Since this has been the exposed ABI since v5.9 perhaps a note /
> analysis is needed here that the disappearance of this file will not
> break any existing scripts/tools.
>
Yes, I have added a note in patch description and also updated
Documentation/ABI/testing/sysfs-bus-papr-pmem with a note on when this
sysfs attr would be available. Thanks for this suggestion.
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> ---
>> Changelog:
>>
>> v2:
>> * Removed a redundant dev_info() from pap_scm_nvdimm_init() [ Aneesh ]
>> * Marked papr_nd_attribute_visible() as static which also fixed the
>> build warning reported by kernel build robot
>> ---
>> arch/powerpc/platforms/pseries/papr_scm.c | 35 ++++++++++++++++-------
>> 1 file changed, 24 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>> index e2b69cc3beaf..11e7b90a3360 100644
>> --- a/arch/powerpc/platforms/pseries/papr_scm.c
>> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>> @@ -907,6 +907,20 @@ static ssize_t flags_show(struct device *dev,
>> }
>> DEVICE_ATTR_RO(flags);
>>
>> +static umode_t papr_nd_attribute_visible(struct kobject *kobj,
>> + struct attribute *attr, int n)
>> +{
>> + struct device *dev = container_of(kobj, typeof(*dev), kobj);
>
> This can use the kobj_to_dev() helper.
>
Fixed in v3
>> + struct nvdimm *nvdimm = to_nvdimm(dev);
>> + struct papr_scm_priv *p = nvdimm_provider_data(nvdimm);
>> +
>> + /* For if perf-stats not available remove perf_stats sysfs */
>> + if (attr == &dev_attr_perf_stats.attr && p->stat_buffer_len == 0)
>> + return 0;
>> +
>> + return attr->mode;
>> +}
>> +
>> /* papr_scm specific dimm attributes */
>> static struct attribute *papr_nd_attributes[] = {
>> &dev_attr_flags.attr,
>> @@ -916,6 +930,7 @@ static struct attribute *papr_nd_attributes[] = {
>>
>> static struct attribute_group papr_nd_attribute_group = {
>> .name = "papr",
>> + .is_visible = papr_nd_attribute_visible,
>> .attrs = papr_nd_attributes,
>> };
>>
>> @@ -931,7 +946,6 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
>> struct nd_region_desc ndr_desc;
>> unsigned long dimm_flags;
>> int target_nid, online_nid;
>> - ssize_t stat_size;
>>
>> p->bus_desc.ndctl = papr_scm_ndctl;
>> p->bus_desc.module = THIS_MODULE;
>> @@ -1016,16 +1030,6 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
>> list_add_tail(&p->region_list, &papr_nd_regions);
>> mutex_unlock(&papr_ndr_lock);
>>
>> - /* Try retriving the stat buffer and see if its supported */
>> - stat_size = drc_pmem_query_stats(p, NULL, 0);
>> - if (stat_size > 0) {
>> - p->stat_buffer_len = stat_size;
>> - dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
>> - p->stat_buffer_len);
>> - } else {
>> - dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n");
>> - }
>> -
>> return 0;
>>
>> err: nvdimm_bus_unregister(p->bus);
>> @@ -1102,6 +1106,7 @@ static int papr_scm_probe(struct platform_device *pdev)
>> u64 blocks, block_size;
>> struct papr_scm_priv *p;
>> const char *uuid_str;
>> + ssize_t stat_size;
>> u64 uuid[2];
>> int rc;
>>
>> @@ -1179,6 +1184,14 @@ static int papr_scm_probe(struct platform_device *pdev)
>> p->res.name = pdev->name;
>> p->res.flags = IORESOURCE_MEM;
>>
>> + /* Try retriving the stat buffer and see if its supported */
>
> s/retriving/retrieving/
>
Fixed in v3.
>> + stat_size = drc_pmem_query_stats(p, NULL, 0);
>> + if (stat_size > 0) {
>> + p->stat_buffer_len = stat_size;
>> + dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
>> + p->stat_buffer_len);
>> + }
>> +
>> rc = papr_scm_nvdimm_init(p);
>> if (rc)
>> goto err2;
>> --
>> 2.31.1
>>
>
> After the minor fixups above you can add:
>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Thanks
>
> ...I assume this will go through the PPC tree.
Yes
--
Cheers
~ Vaibhav
^ permalink raw reply
* [PATCH v2] mm/ioremap: Fix iomap_max_page_shift
From: Christophe Leroy @ 2021-05-13 9:29 UTC (permalink / raw)
To: Nicholas Piggin, Andrew Morton; +Cc: linux-mm, linuxppc-dev, linux-kernel
iomap_max_page_shift is expected to contain a page shift,
so it can't be a 'bool', has to be an 'unsigned int'
And fix the default values: P4D_SHIFT is when huge iomap is allowed.
However, on some architectures (eg: powerpc book3s/64), P4D_SHIFT is
not a constant so it can't be used to initialise a static variable.
So, initialise iomap_max_page_shift with a maximum shift supported
by the architecture, it is gated by P4D_SHIFT in vmap_try_huge_p4d()
anyway.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Fixes: bbc180a5adb0 ("mm: HUGE_VMAP arch support cleanup")
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
v2: Initialise with BITS_PER_LONG - 1 instead of P4D_SHIFT which is not a constant on powerpc/64.
---
mm/ioremap.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/mm/ioremap.c b/mm/ioremap.c
index d1dcc7e744ac..8ee0136f8cb0 100644
--- a/mm/ioremap.c
+++ b/mm/ioremap.c
@@ -16,16 +16,16 @@
#include "pgalloc-track.h"
#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
-static bool __ro_after_init iomap_max_page_shift = PAGE_SHIFT;
+static unsigned int __ro_after_init iomap_max_page_shift = BITS_PER_LONG - 1;
static int __init set_nohugeiomap(char *str)
{
- iomap_max_page_shift = P4D_SHIFT;
+ iomap_max_page_shift = PAGE_SHIFT;
return 0;
}
early_param("nohugeiomap", set_nohugeiomap);
#else /* CONFIG_HAVE_ARCH_HUGE_VMAP */
-static const bool iomap_max_page_shift = PAGE_SHIFT;
+static const unsigned int iomap_max_page_shift = PAGE_SHIFT;
#endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */
int ioremap_page_range(unsigned long addr,
--
2.25.0
^ permalink raw reply related
* Re: [RFC 01/10] powerpc/rtas: new APIs for busy and extended delay statuses
From: Alexey Kardashevskiy @ 2021-05-13 9:59 UTC (permalink / raw)
To: Nathan Lynch, linuxppc-dev; +Cc: ajd, Nicholas Piggin, tyreld
In-Reply-To: <20210504030358.1715034-2-nathanl@linux.ibm.com>
On 04/05/2021 13:03, Nathan Lynch wrote:
> Add new APIs for handling busy (-2) and extended delay
> hint (9900...9905) statuses from RTAS. These are intended to be
> drop-in replacements for existing uses of rtas_busy_delay().
>
> A problem with rtas_busy_delay() and rtas_busy_delay_time() is that
> they consider -2/busy to be equivalent to 9900 (wait 1ms). In fact,
> the OS should call again as soon as it wants on -2, which at least on
> PowerVM means RTAS is returning only to uphold the general requirement
> that RTAS must return control to the OS in a "timely fashion" (250us).
>
> Combine this with the fact that msleep(1) actually sleeps for more
> like 20ms in practice: on busy VMs we schedule away for much longer
> than necessary on -2 and 9900.
>
> This is fixed in rtas_sched_if_busy(), which uses usleep_range() for
> small delay hints, and only schedules away on -2 if there is other
> work available. It also refuses to sleep longer than one second
> regardless of the hinted value, on the assumption that even longer
> running operations can tolerate polling at 1HZ.
>
> rtas_spin_if_busy() and rtas_force_spin_if_busy() are provided for
> atomic contexts which need to handle busy status and extended delay
> hints.
>
> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
> ---
> arch/powerpc/include/asm/rtas.h | 4 +
> arch/powerpc/kernel/rtas.c | 168 ++++++++++++++++++++++++++++++++
> 2 files changed, 172 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 9dc97d2f9d27..555ff3290f92 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -266,6 +266,10 @@ extern int rtas_set_rtc_time(struct rtc_time *rtc_time);
> extern unsigned int rtas_busy_delay_time(int status);
> extern unsigned int rtas_busy_delay(int status);
>
> +bool rtas_sched_if_busy(int status);
> +bool rtas_spin_if_busy(int status);
> +bool rtas_force_spin_if_busy(int status);
> +
> extern int early_init_dt_scan_rtas(unsigned long node,
> const char *uname, int depth, void *data);
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 6bada744402b..4a1dfbfa51ba 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -519,6 +519,174 @@ unsigned int rtas_busy_delay(int status)
> }
> EXPORT_SYMBOL(rtas_busy_delay);
>
> +/**
> + * rtas_force_spin_if_busy() - Consume a busy or extended delay status
> + * in atomic context.
> + * @status: Return value from rtas_call() or similar function.
> + *
> + * Use this function when you cannot avoid using an RTAS function
> + * which may return an extended delay hint in atomic context. If
> + * possible, use rtas_spin_if_busy() or rtas_sched_if_busy() instead
> + * of this function.
> + *
> + * Return: True if @status is -2 or 990x, in which case
> + * rtas_spin_if_busy() will have delayed an appropriate amount
> + * of time, and the caller should call the RTAS function
> + * again. False otherwise.
> + */
> +bool rtas_force_spin_if_busy(int status)
rtas_force_delay_if_busy()? neither this one nor rtas_spin_if_busy()
actually spins.
> +{
> + bool was_busy = true;
> +
> + switch (status) {
> + case RTAS_BUSY:
> + /* OK to call again immediately; do nothing. */
> + break;
> + case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
> + mdelay(1);
> + break;
> + default:
> + was_busy = false;
> + break;
> + }
> +
> + return was_busy;
> +}
> +
> +/**
> + * rtas_spin_if_busy() - Consume a busy status in atomic context.
> + * @status: Return value from rtas_call() or similar function.
> + *
> + * Prefer rtas_sched_if_busy() over this function. Prefer this
> + * function over rtas_force_spin_if_busy(). Use this function in
> + * atomic contexts with RTAS calls that are specified to return -2 but
> + * not 990x. This function will complain and execute a minimal delay
> + * if passed a 990x status.
> + *
> + * Return: True if @status is -2 or 990x, in which case
> + * rtas_spin_if_busy() will have delayed an appropriate amount
> + * of time, and the caller should call the RTAS function
> + * again. False otherwise.
> + */
> +bool rtas_spin_if_busy(int status)
rtas_delay_if_busy()?
> +{
> + bool was_busy = true;
> +
> + switch (status) {
> + case RTAS_BUSY:
> + /* OK to call again immediately; do nothing. */
> + break;
> + case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
> + /*
> + * Generally, RTAS functions which can return this
> + * status should be considered too expensive to use in
> + * atomic context. Change the calling code to use
> + * rtas_sched_if_busy(), or if that's not possible,
> + * use rtas_force_spin_if_busy().
> + */
> + pr_warn_once("%pS may use RTAS call in atomic context which returns extended delay.\n",
> + __builtin_return_address(0));
> + mdelay(1);
> + break;
> + default:
> + was_busy = false;
> + break;
> + }
> +
> + return was_busy;
> +}
> +
> +static unsigned long extended_delay_ms(unsigned int status)
extended_delay_to_ms()?
> +{
> + unsigned int extdelay;
> + unsigned int order;
> + unsigned int ms;
> +
> + extdelay = clamp((int)status, RTAS_EXTENDED_DELAY_MIN, RTAS_EXTENDED_DELAY_MAX);
> + WARN_ONCE(extdelay != status, "%s passed invalid status %u\n", __func__, status);
> +
> + order = status - RTAS_EXTENDED_DELAY_MIN;
Using extdelay instead of status seems safer.
> + for (ms = 1; order > 0; order--)
> + ms *= 10;
> +
> + return ms;
> +}
> +
> +static void handle_extended_delay(unsigned int status)
rtas_sleep()? rtas_extended_delay()?
> +{
> + unsigned long usecs;
> +
> + usecs = 1000 * extended_delay_ms(status);
usecs could be msecs, you would need fewer zeroes and would not need
DIV_ROUND_UP below...
> +
> + /*
> + * If we have no other work pending, there's no reason to
> + * sleep.
> + */
> + if (!need_resched())
> + return;
> +
> + /*
> + * The extended delay hint can be as high as 100
> + * seconds. Surely any function returning such a status is
> + * either buggy or isn't going to be significantly slowed by
> + * us polling at 1HZ. Clamp the sleep time to one second.
> + */
> + usecs = clamp(usecs, 1000UL, 1000000UL);
> +
> + /*
> + * The delay hint is an order-of-magnitude suggestion, not a
> + * minimum. It is fine, possibly even advantageous, for us to
> + * pause for less time than suggested. For small values, use
> + * usleep_range() to ensure we don't sleep much longer than
> + * actually suggested.
> + *
> + * See Documentation/timers/timers-howto.rst for explanation
> + * of the threshold used here.
> + */
> + if (usecs <= 20000)
> + usleep_range(usecs / 2, 2 * usecs);
... and this would be usleep_range(msecs*500, msecs*2000).
> + else
> + msleep(DIV_ROUND_UP(usecs, 1000));
> +}
> +
> +/**
> + * rtas_sched_if_busy() - Consume a busy or extended delay status.
> + * @status: Return value from rtas_call() or similar function.
> + *
> + * Prefer this function over rtas_spin_if_busy().
> + *
> + * Context: This function may sleep.
> + *
> + * Return: True if @status is -2 or 990x, in which case
> + * rtas_sched_if_busy() will have slept an appropriate amount
> + * of time, and the caller should call the RTAS function
> + * again. False otherwise.
> + */
> +bool rtas_sched_if_busy(int status)
> +{
> + bool was_busy = true;
> +
> + might_sleep();
> +
> + switch (status) {
> + case RTAS_BUSY:
> + /*
> + * OK to call again immediately. Schedule if there's
> + * other work to do, but no sleep is necessary.
> + */
> + cond_resched();
> + break;
> + case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
> + handle_extended_delay(status);
> + break;
> + default:
> + was_busy = false;
> + break;
> + }
> +
> + return was_busy;
> +}
Throughout the series this one is called instead of simple spinning,
03/10..05/10, and some of those have in_interrupt() checks for a reason
(which I do not know but nevertheless) so they may be called with
interrupts disabled which in turn means we should not be calling
cond_resched() unconditionally. I am told this is should be done:
if (!preemptible())
/* print warning, don't sleep or cond_resched */
> +
> static int rtas_error_rc(int rtas_rc)
> {
> int rc;
>
--
Alexey
^ permalink raw reply
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christophe Leroy @ 2021-05-13 10:01 UTC (permalink / raw)
To: Christian Zigotzky
Cc: Darren Stevens, R.T.Dickinson, mad skateman, linuxppc-dev,
Christian Zigotzky
In-Reply-To: <65116250-6EEB-4419-8A7D-B0882379A68B@xenosoft.de>
Le 13/05/2021 à 08:47, Christian Zigotzky a écrit :
> Hi Christophe,
>
>> On 9. May 2021, at 19:37, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>
>> Did I do an error in my analysis ?
>
> No, you didn’t. On the contrary you have found the issue. ;-) The issue is somewhere in the new interrupt code.
I'm not convinced, but let's give it a try.
>
>> ZZ | * | ceff77efa4f8 powerpc/64e/interrupt: Use new interrupt context tracking scheme
>
> Could you please create a patch for reverting the new interrupt code? I would like to confirm your result.
Please fetch https://github.com/chleroy/linux.git and try the branch for_christian.
This is a revert of approx a dozen of commits around the changes to 64e on top of v5.13-rc1.
If the top of the branch works for you, it would be great if you can find out which one of the
reverts fixes the problem for real.
Thanks
Christophe
^ permalink raw reply
* [PATCH 0/8] xen: harden frontends against malicious backends
From: Juergen Gross @ 2021-05-13 10:02 UTC (permalink / raw)
To: xen-devel, linux-kernel, linux-block, netdev, linuxppc-dev
Cc: Juergen Gross, Jens Axboe, Stefano Stabellini,
Konrad Rzeszutek Wilk, Greg Kroah-Hartman, Jakub Kicinski,
Boris Ostrovsky, Jiri Slaby, David S. Miller,
Roger Pau Monné
Xen backends of para-virtualized devices can live in dom0 kernel, dom0
user land, or in a driver domain. This means that a backend might
reside in a less trusted environment than the Xen core components, so
a backend should not be able to do harm to a Xen guest (it can still
mess up I/O data, but it shouldn't be able to e.g. crash a guest by
other means or cause a privilege escalation in the guest).
Unfortunately many frontends in the Linux kernel are fully trusting
their respective backends. This series is starting to fix the most
important frontends: console, disk and network.
It was discussed to handle this as a security problem, but the topic
was discussed in public before, so it isn't a real secret.
Juergen Gross (8):
xen: sync include/xen/interface/io/ring.h with Xen's newest version
xen/blkfront: read response from backend only once
xen/blkfront: don't take local copy of a request from the ring page
xen/blkfront: don't trust the backend response data blindly
xen/netfront: read response from backend only once
xen/netfront: don't read data from request on the ring page
xen/netfront: don't trust the backend response data blindly
xen/hvc: replace BUG_ON() with negative return value
drivers/block/xen-blkfront.c | 118 +++++++++-----
drivers/net/xen-netfront.c | 184 ++++++++++++++-------
drivers/tty/hvc/hvc_xen.c | 15 +-
include/xen/interface/io/ring.h | 278 ++++++++++++++++++--------------
4 files changed, 369 insertions(+), 226 deletions(-)
--
2.26.2
^ permalink raw reply
* [PATCH 8/8] xen/hvc: replace BUG_ON() with negative return value
From: Juergen Gross @ 2021-05-13 10:03 UTC (permalink / raw)
To: xen-devel, linuxppc-dev, linux-kernel
Cc: Juergen Gross, Greg Kroah-Hartman, Jiri Slaby
In-Reply-To: <20210513100302.22027-1-jgross@suse.com>
Xen frontends shouldn't BUG() in case of illegal data received from
their backends. So replace the BUG_ON()s when reading illegal data from
the ring page with negative return values.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/tty/hvc/hvc_xen.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 92c9a476defc..30d7ffb1e04c 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -86,6 +86,11 @@ static int __write_console(struct xencons_info *xencons,
cons = intf->out_cons;
prod = intf->out_prod;
mb(); /* update queue values before going on */
+
+ if (WARN_ONCE((prod - cons) > sizeof(intf->out),
+ "Illegal ring page indices"))
+ return -EINVAL;
+
BUG_ON((prod - cons) > sizeof(intf->out));
while ((sent < len) && ((prod - cons) < sizeof(intf->out)))
@@ -114,7 +119,10 @@ static int domU_write_console(uint32_t vtermno, const char *data, int len)
*/
while (len) {
int sent = __write_console(cons, data, len);
-
+
+ if (sent < 0)
+ return sent;
+
data += sent;
len -= sent;
@@ -138,7 +146,10 @@ static int domU_read_console(uint32_t vtermno, char *buf, int len)
cons = intf->in_cons;
prod = intf->in_prod;
mb(); /* get pointers before reading ring */
- BUG_ON((prod - cons) > sizeof(intf->in));
+
+ if (WARN_ONCE((prod - cons) > sizeof(intf->in),
+ "Illegal ring page indices"))
+ return -EINVAL;
while (cons != prod && recv < len)
buf[recv++] = intf->in[MASK_XENCONS_IDX(cons++, intf->in)];
--
2.26.2
^ permalink raw reply related
* [powerpc:fixes-test] BUILD SUCCESS da3bb206c9ceb0736d9e2897ea697acabad35833
From: kernel test robot @ 2021-05-13 10:14 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test
branch HEAD: da3bb206c9ceb0736d9e2897ea697acabad35833 KVM: PPC: Book3S HV: Fix kvm_unmap_gfn_range_hv() for Hash MMU
elapsed time: 1183m
configs tested: 9
configs skipped: 83
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 allyesconfig
x86_64 rhel-8.3-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kbuiltin
x86_64 kexec
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH 8/8] xen/hvc: replace BUG_ON() with negative return value
From: Christophe Leroy @ 2021-05-13 10:16 UTC (permalink / raw)
To: Juergen Gross, xen-devel, linuxppc-dev, linux-kernel
Cc: Greg Kroah-Hartman, Jiri Slaby
In-Reply-To: <20210513100302.22027-9-jgross@suse.com>
Le 13/05/2021 à 12:03, Juergen Gross a écrit :
> Xen frontends shouldn't BUG() in case of illegal data received from
> their backends. So replace the BUG_ON()s when reading illegal data from
> the ring page with negative return values.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> drivers/tty/hvc/hvc_xen.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index 92c9a476defc..30d7ffb1e04c 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -86,6 +86,11 @@ static int __write_console(struct xencons_info *xencons,
> cons = intf->out_cons;
> prod = intf->out_prod;
> mb(); /* update queue values before going on */
> +
> + if (WARN_ONCE((prod - cons) > sizeof(intf->out),
> + "Illegal ring page indices"))
> + return -EINVAL;
> +
> BUG_ON((prod - cons) > sizeof(intf->out));
Why keep the BUG_ON() ?
>
> while ((sent < len) && ((prod - cons) < sizeof(intf->out)))
> @@ -114,7 +119,10 @@ static int domU_write_console(uint32_t vtermno, const char *data, int len)
> */
> while (len) {
> int sent = __write_console(cons, data, len);
> -
> +
> + if (sent < 0)
> + return sent;
> +
> data += sent;
> len -= sent;
>
> @@ -138,7 +146,10 @@ static int domU_read_console(uint32_t vtermno, char *buf, int len)
> cons = intf->in_cons;
> prod = intf->in_prod;
> mb(); /* get pointers before reading ring */
> - BUG_ON((prod - cons) > sizeof(intf->in));
> +
> + if (WARN_ONCE((prod - cons) > sizeof(intf->in),
> + "Illegal ring page indices"))
> + return -EINVAL;
>
> while (cons != prod && recv < len)
> buf[recv++] = intf->in[MASK_XENCONS_IDX(cons++, intf->in)];
>
^ permalink raw reply
* Re: [PATCH 8/8] xen/hvc: replace BUG_ON() with negative return value
From: Juergen Gross @ 2021-05-13 10:20 UTC (permalink / raw)
To: Christophe Leroy, xen-devel, linuxppc-dev, linux-kernel
Cc: Greg Kroah-Hartman, Jiri Slaby
In-Reply-To: <6da4cc91-ccde-fce8-707c-e7544783c2fa@csgroup.eu>
[-- Attachment #1.1.1: Type: text/plain, Size: 1232 bytes --]
On 13.05.21 12:16, Christophe Leroy wrote:
>
>
> Le 13/05/2021 à 12:03, Juergen Gross a écrit :
>> Xen frontends shouldn't BUG() in case of illegal data received from
>> their backends. So replace the BUG_ON()s when reading illegal data from
>> the ring page with negative return values.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> drivers/tty/hvc/hvc_xen.c | 15 +++++++++++++--
>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
>> index 92c9a476defc..30d7ffb1e04c 100644
>> --- a/drivers/tty/hvc/hvc_xen.c
>> +++ b/drivers/tty/hvc/hvc_xen.c
>> @@ -86,6 +86,11 @@ static int __write_console(struct xencons_info
>> *xencons,
>> cons = intf->out_cons;
>> prod = intf->out_prod;
>> mb(); /* update queue values before going on */
>> +
>> + if (WARN_ONCE((prod - cons) > sizeof(intf->out),
>> + "Illegal ring page indices"))
>> + return -EINVAL;
>> +
>> BUG_ON((prod - cons) > sizeof(intf->out));
>
> Why keep the BUG_ON() ?
Oh, failed to delete it. Thanks for noticing.
Juergen
[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply
* Re: [PATCH 8/8] xen/hvc: replace BUG_ON() with negative return value
From: Greg Kroah-Hartman @ 2021-05-13 10:25 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel, linuxppc-dev, linux-kernel, Jiri Slaby
In-Reply-To: <20210513100302.22027-9-jgross@suse.com>
On Thu, May 13, 2021 at 12:03:02PM +0200, Juergen Gross wrote:
> Xen frontends shouldn't BUG() in case of illegal data received from
> their backends. So replace the BUG_ON()s when reading illegal data from
> the ring page with negative return values.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> drivers/tty/hvc/hvc_xen.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index 92c9a476defc..30d7ffb1e04c 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -86,6 +86,11 @@ static int __write_console(struct xencons_info *xencons,
> cons = intf->out_cons;
> prod = intf->out_prod;
> mb(); /* update queue values before going on */
> +
> + if (WARN_ONCE((prod - cons) > sizeof(intf->out),
> + "Illegal ring page indices"))
> + return -EINVAL;
How nice, you just rebooted on panic-on-warn systems :(
> +
> BUG_ON((prod - cons) > sizeof(intf->out));
Why keep this line?
Please just fix this up properly, if userspace can trigger this, then
both the WARN_ON() and BUG_ON() are not correct and need to be correctly
handled.
>
> while ((sent < len) && ((prod - cons) < sizeof(intf->out)))
> @@ -114,7 +119,10 @@ static int domU_write_console(uint32_t vtermno, const char *data, int len)
> */
> while (len) {
> int sent = __write_console(cons, data, len);
> -
> +
> + if (sent < 0)
> + return sent;
> +
> data += sent;
> len -= sent;
>
> @@ -138,7 +146,10 @@ static int domU_read_console(uint32_t vtermno, char *buf, int len)
> cons = intf->in_cons;
> prod = intf->in_prod;
> mb(); /* get pointers before reading ring */
> - BUG_ON((prod - cons) > sizeof(intf->in));
> +
> + if (WARN_ONCE((prod - cons) > sizeof(intf->in),
> + "Illegal ring page indices"))
> + return -EINVAL;
Same here, you still just paniced a machine :(
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 8/8] xen/hvc: replace BUG_ON() with negative return value
From: Juergen Gross @ 2021-05-13 10:35 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: xen-devel, linuxppc-dev, linux-kernel, Jiri Slaby
In-Reply-To: <YJz+qK8snI64/TKh@kroah.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 1447 bytes --]
On 13.05.21 12:25, Greg Kroah-Hartman wrote:
> On Thu, May 13, 2021 at 12:03:02PM +0200, Juergen Gross wrote:
>> Xen frontends shouldn't BUG() in case of illegal data received from
>> their backends. So replace the BUG_ON()s when reading illegal data from
>> the ring page with negative return values.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> drivers/tty/hvc/hvc_xen.c | 15 +++++++++++++--
>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
>> index 92c9a476defc..30d7ffb1e04c 100644
>> --- a/drivers/tty/hvc/hvc_xen.c
>> +++ b/drivers/tty/hvc/hvc_xen.c
>> @@ -86,6 +86,11 @@ static int __write_console(struct xencons_info *xencons,
>> cons = intf->out_cons;
>> prod = intf->out_prod;
>> mb(); /* update queue values before going on */
>> +
>> + if (WARN_ONCE((prod - cons) > sizeof(intf->out),
>> + "Illegal ring page indices"))
>> + return -EINVAL;
>
> How nice, you just rebooted on panic-on-warn systems :(
>
>> +
>> BUG_ON((prod - cons) > sizeof(intf->out));
>
> Why keep this line?
Failed to delete it, sorry.
>
> Please just fix this up properly, if userspace can trigger this, then
> both the WARN_ON() and BUG_ON() are not correct and need to be correctly
> handled.
It can be triggered by the console backend, but I agree a WARN isn't the
way to go here.
Juergen
[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply
* [PATCH] selftests/powerpc: Fix duplicate included pthread.h
From: Jiapeng Chong @ 2021-05-13 11:03 UTC (permalink / raw)
To: mpe
Cc: Jiapeng Chong, linuxppc-dev, linux-kernel, paulus,
linux-kselftest, shuah
Clean up the following includecheck warning:
./tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c: pthread.h is
included more than once.
No functional change.
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
index e2a0c07..9ef37a9 100644
--- a/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
+++ b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
@@ -17,7 +17,6 @@
#include <pthread.h>
#include <sys/mman.h>
#include <unistd.h>
-#include <pthread.h>
#include "tm.h"
#include "utils.h"
--
1.8.3.1
^ permalink raw reply related
* [PATCH kernel v3] powerpc/makefile: Do not redefine $(CPP) for preprocessor
From: Alexey Kardashevskiy @ 2021-05-13 11:59 UTC (permalink / raw)
To: linuxppc-dev
Cc: Michal Marek, linux-kbuild, Alexey Kardashevskiy, Masahiro Yamada,
Nick Desaulniers, linux-kernel, Nathan Chancellor,
clang-built-linux
The $(CPP) (do only preprocessing) macro is already defined in Makefile.
However POWERPC redefines it and adds $(KBUILD_CFLAGS) which results
in flags duplication. Which is not a big deal by itself except for
the flags which depend on other flags and the compiler checks them
as it parses the command line.
Specifically, scripts/Makefile.build:304 generates ksyms for .S files.
If clang+llvm+sanitizer are enabled, this results in
-emit-llvm-bc -fno-lto -flto -fvisibility=hidden \
-fsanitize=cfi-mfcall -fno-lto ...
in the clang command line and triggers error:
clang-13: error: invalid argument '-fsanitize=cfi-mfcall' only allowed with '-flto'
This removes unnecessary CPP redefinition. Which works fine as in most
place KBUILD_CFLAGS is passed to $CPP except
arch/powerpc/kernel/vdso64/vdso(32|64).lds. To fix vdso, this does:
1. add -m(big|little)-endian to $CPP
2. add target to $KBUILD_CPPFLAGS as otherwise clang ignores -m(big|little)-endian if
the building platform does not support big endian (such as x86).
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* moved vdso cleanup in a separate patch
* only add target to KBUILD_CPPFLAGS for CLANG
v2:
* fix KBUILD_CPPFLAGS
* add CLANG_FLAGS to CPPFLAGS
---
Makefile | 1 +
arch/powerpc/Makefile | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 15b6476d0f89..5b545bef7653 100644
--- a/Makefile
+++ b/Makefile
@@ -576,6 +576,7 @@ CC_VERSION_TEXT = $(subst $(pound),,$(shell $(CC) --version 2>/dev/null | head -
ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)
ifneq ($(CROSS_COMPILE),)
CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
+KBUILD_CPPFLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
endif
ifeq ($(LLVM_IAS),1)
CLANG_FLAGS += -integrated-as
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 3212d076ac6a..306bfd2797ad 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -76,6 +76,7 @@ endif
ifdef CONFIG_CPU_LITTLE_ENDIAN
KBUILD_CFLAGS += -mlittle-endian
+KBUILD_CPPFLAGS += -mlittle-endian
KBUILD_LDFLAGS += -EL
LDEMULATION := lppc
GNUTARGET := powerpcle
@@ -83,6 +84,7 @@ MULTIPLEWORD := -mno-multiple
KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-save-toc-indirect)
else
KBUILD_CFLAGS += $(call cc-option,-mbig-endian)
+KBUILD_CPPFLAGS += $(call cc-option,-mbig-endian)
KBUILD_LDFLAGS += -EB
LDEMULATION := ppc
GNUTARGET := powerpc
@@ -208,7 +210,6 @@ KBUILD_CPPFLAGS += -I $(srctree)/arch/$(ARCH) $(asinstr)
KBUILD_AFLAGS += $(AFLAGS-y)
KBUILD_CFLAGS += $(call cc-option,-msoft-float)
KBUILD_CFLAGS += -pipe $(CFLAGS-y)
-CPP = $(CC) -E $(KBUILD_CFLAGS)
CHECKFLAGS += -m$(BITS) -D__powerpc__ -D__powerpc$(BITS)__
ifdef CONFIG_CPU_BIG_ENDIAN
--
2.30.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox