* Re: [PATCH] powerpc/powernv: Enable HAIL (HV AIL) for ISA v3.1 processors
From: Michael Ellerman @ 2021-04-07 11:33 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20210402024124.545826-1-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Starting with ISA v3.1, LPCR[AIL] no longer controls the interrupt
> mode for HV=1 interrupts. Instead, a new LPCR[HAIL] bit is defined
> which behaves like AIL=3 for HV interrupts when set.
>
> Set HAIL on bare metal to give us mmu-on interrupts and improve
> performance.
>
> This also fixes an scv bug: we don't implement scv real mode (AIL=0)
> vectors because they are at an inconvenient location, so we just
> disable scv support when AIL can not be set. However powernv assumes
> that LPCR[AIL] will enable AIL mode so it enables scv support despite
> HV interrupts being AIL=0, which causes scv interrupts to go off into
> the weeds.
Should we tag this as fixing the initial P10 support, or the scv
support? Or neither?
cheers
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 1be20bc8dce2..9086a2644c89 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -441,6 +441,7 @@
> #define LPCR_VRMA_LP1 ASM_CONST(0x0000800000000000)
> #define LPCR_RMLS 0x1C000000 /* Implementation dependent RMO limit sel */
> #define LPCR_RMLS_SH 26
> +#define LPCR_HAIL ASM_CONST(0x0000000004000000) /* HV AIL (ISAv3.1) */
> #define LPCR_ILE ASM_CONST(0x0000000002000000) /* !HV irqs set MSR:LE */
> #define LPCR_AIL ASM_CONST(0x0000000001800000) /* Alternate interrupt location */
> #define LPCR_AIL_0 ASM_CONST(0x0000000000000000) /* MMU off exception offset 0x0 */
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 04a31586f760..671192afcdfd 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -233,10 +233,23 @@ static void cpu_ready_for_interrupts(void)
> * If we are not in hypervisor mode the job is done once for
> * the whole partition in configure_exceptions().
> */
> - if (cpu_has_feature(CPU_FTR_HVMODE) &&
> - cpu_has_feature(CPU_FTR_ARCH_207S)) {
> + if (cpu_has_feature(CPU_FTR_HVMODE)) {
> unsigned long lpcr = mfspr(SPRN_LPCR);
> - mtspr(SPRN_LPCR, lpcr | LPCR_AIL_3);
> + unsigned long new_lpcr = lpcr;
> +
> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
> + /* P10 DD1 does not have HAIL */
> + if (pvr_version_is(PVR_POWER10) &&
> + (mfspr(SPRN_PVR) & 0xf00) == 0x100)
> + new_lpcr |= LPCR_AIL_3;
> + else
> + new_lpcr |= LPCR_HAIL;
> + } else if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
> + new_lpcr |= LPCR_AIL_3;
> + }
> +
> + if (new_lpcr != lpcr)
> + mtspr(SPRN_LPCR, new_lpcr);
> }
>
> /*
> --
> 2.23.0
^ permalink raw reply
* Re: [PATCH v3 1/2] powerpc/perf: Infrastructure to support checking of attr.config*
From: Michael Ellerman @ 2021-04-07 11:38 UTC (permalink / raw)
To: Madhavan Srinivasan; +Cc: Madhavan Srinivasan, linuxppc-dev
In-Reply-To: <20210325115326.143151-1-maddy@linux.ibm.com>
Madhavan Srinivasan <maddy@linux.ibm.com> writes:
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 6817331e22ff..c6eeb4fdc5fd 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -1958,6 +1958,20 @@ static int power_pmu_event_init(struct perf_event *event)
>
> if (ppmu->blacklist_ev && is_event_blacklisted(ev))
> return -EINVAL;
> + /*
> + * PMU config registers have fields that are
> + * reserved and specific value to bit field as reserved.
> + * For ex., MMCRA[61:62] is Randome Sampling Mode (SM)
> + * and value of 0b11 to this field is reserved.
> + *
> + * This check is needed only for raw event type,
> + * since tools like fuzzer use raw event type to
> + * provide randomized event code values for test.
> + *
> + */
> + if (ppmu->check_attr_config &&
> + ppmu->check_attr_config(event))
> + return -EINVAL;
> break;
It's not obvious from the diff, but you're only doing the check for RAW
events.
But I think that's wrong, we should always check, even if the event code
comes from the kernel we should still apply the same checks. Otherwise
we might inadvertently use an invalid event code for a HARDWARE or CACHE
event.
cheers
^ permalink raw reply
* Re: [PATCH 16/20] kbuild: powerpc: use common install script
From: Masahiro Yamada @ 2021-04-07 11:32 UTC (permalink / raw)
To: Greg Kroah-Hartman, Michael Ellerman
Cc: linux-arch, Michal Marek, linuxppc-dev, Linux Kernel Mailing List,
Linux Kbuild mailing list
In-Reply-To: <20210407053419.449796-17-gregkh@linuxfoundation.org>
On Wed, Apr 7, 2021 at 2:34 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> The common scripts/install.sh script will now work for powerpc, all that
> is needed is to add it to the list of arches that do not put the version
> number in the installed file name.
>
> After the kernel is installed, powerpc also likes to install a few
> random files, so provide the ability to do that as well.
>
> With that we can remove the powerpc-only version of the install script.
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> arch/powerpc/boot/Makefile | 4 +--
> arch/powerpc/boot/install.sh | 55 ------------------------------------
> scripts/install.sh | 14 ++++++++-
> 3 files changed, 15 insertions(+), 58 deletions(-)
> delete mode 100644 arch/powerpc/boot/install.sh
>
> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> index 2b8da923ceca..bbfcbd33e0b7 100644
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -442,11 +442,11 @@ $(obj)/zImage.initrd: $(addprefix $(obj)/, $(initrd-y))
>
> # Only install the vmlinux
> install: $(CONFIGURE) $(addprefix $(obj)/, $(image-y))
> - sh -x $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" vmlinux System.map "$(INSTALL_PATH)"
> + sh -x $(srctree)/scripts/install.sh "$(KERNELRELEASE)" vmlinux System.map "$(INSTALL_PATH)"
>
> # Install the vmlinux and other built boot targets.
> zInstall: $(CONFIGURE) $(addprefix $(obj)/, $(image-y))
> - sh -x $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" vmlinux System.map "$(INSTALL_PATH)" $^
> + sh -x $(srctree)/scripts/install.sh "$(KERNELRELEASE)" vmlinux System.map "$(INSTALL_PATH)" $^
I want comments from the ppc maintainers
because this code is already broken.
This 'zInstall' target is unreachable.
See commit c913e5f95e546d8d3a9f99ba9908f7e095cbc1fb
It added the new target 'zInstall', but it is not hooked anywhere.
It is completely useless for 6 years, and nobody has pointed it out.
So, I think nobody is caring about this broken code.
One more thing, Kbuild does not recognize it as an installation target
because the 'I' in 'zInstall' is a capital letter.
The name of the installation target must be '*install',
all letters in lower cases.
> PHONY += install zInstall
>
> diff --git a/arch/powerpc/boot/install.sh b/arch/powerpc/boot/install.sh
> deleted file mode 100644
> index b6a256bc96ee..000000000000
> --- a/arch/powerpc/boot/install.sh
> +++ /dev/null
> @@ -1,55 +0,0 @@
> -#!/bin/sh
> -#
> -# This file is subject to the terms and conditions of the GNU General Public
> -# License. See the file "COPYING" in the main directory of this archive
> -# for more details.
> -#
> -# Copyright (C) 1995 by Linus Torvalds
> -#
> -# Blatantly stolen from in arch/i386/boot/install.sh by Dave Hansen
> -#
> -# "make install" script for ppc64 architecture
> -#
> -# Arguments:
> -# $1 - kernel version
> -# $2 - kernel image file
> -# $3 - kernel map file
> -# $4 - default install path (blank if root directory)
> -# $5 and more - kernel boot files; zImage*, uImage, cuImage.*, etc.
> -#
> -
> -# Bail with error code if anything goes wrong
> -set -e
> -
> -# User may have a custom install script
> -
> -if [ -x ~/bin/${INSTALLKERNEL} ]; then exec ~/bin/${INSTALLKERNEL} "$@"; fi
> -if [ -x /sbin/${INSTALLKERNEL} ]; then exec /sbin/${INSTALLKERNEL} "$@"; fi
> -
> -# Default install
> -
> -# this should work for both the pSeries zImage and the iSeries vmlinux.sm
> -image_name=`basename $2`
> -
> -if [ -f $4/$image_name ]; then
> - mv $4/$image_name $4/$image_name.old
> -fi
> -
> -if [ -f $4/System.map ]; then
> - mv $4/System.map $4/System.old
> -fi
> -
> -cat $2 > $4/$image_name
> -cp $3 $4/System.map
> -
> -# Copy all the bootable image files
> -path=$4
> -shift 4
> -while [ $# -ne 0 ]; do
> - image_name=`basename $1`
> - if [ -f $path/$image_name ]; then
> - mv $path/$image_name $path/$image_name.old
> - fi
> - cat $1 > $path/$image_name
> - shift
> -done;
> diff --git a/scripts/install.sh b/scripts/install.sh
> index e0ffb95737d4..67c0a5f74af2 100644
> --- a/scripts/install.sh
> +++ b/scripts/install.sh
> @@ -67,7 +67,7 @@ fi
> # Some architectures name their files based on version number, and
> # others do not. Call out the ones that do not to make it obvious.
> case "${ARCH}" in
> - ia64 | m68k | nios2 | x86)
> + ia64 | m68k | nios2 | powerpc | x86)
> version=""
> ;;
> *)
> @@ -93,6 +93,18 @@ case "${ARCH}" in
> /usr/sbin/elilo
> fi
> ;;
> + powerpc)
> + # powerpc installation can list other boot targets after the
> + # install path that should be copied to the correct location
Perhaps, we can remove this if the ppc maintainers approve it ?
> + path=$4
> + shift 4
> + while [ $# -ne 0 ]; do
> + image_name=$(basename "$1")
> + install "$1" "$path"/"$image_name"
> + shift
> + done;
> + sync
> + ;;
> x86)
> if [ -x /sbin/lilo ]; then
> /sbin/lilo
> --
> 2.31.1
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH] powerpc/powernv: Enable HAIL (HV AIL) for ISA v3.1 processors
From: Nicholas Piggin @ 2021-04-07 12:13 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <87v98ye3cn.fsf@mpe.ellerman.id.au>
Excerpts from Michael Ellerman's message of April 7, 2021 9:33 pm:
> Nicholas Piggin <npiggin@gmail.com> writes:
>> Starting with ISA v3.1, LPCR[AIL] no longer controls the interrupt
>> mode for HV=1 interrupts. Instead, a new LPCR[HAIL] bit is defined
>> which behaves like AIL=3 for HV interrupts when set.
>>
>> Set HAIL on bare metal to give us mmu-on interrupts and improve
>> performance.
>>
>> This also fixes an scv bug: we don't implement scv real mode (AIL=0)
>> vectors because they are at an inconvenient location, so we just
>> disable scv support when AIL can not be set. However powernv assumes
>> that LPCR[AIL] will enable AIL mode so it enables scv support despite
>> HV interrupts being AIL=0, which causes scv interrupts to go off into
>> the weeds.
>
> Should we tag this as fixing the initial P10 support, or the scv
> support? Or neither?
Good question. It does fix a nasty crash with scv so at least it should
be tagged I guess.
I don't know of anything else that assumes AIL on bare metal, so I don't
know of a crashy bug it fixes with initial P10 support. But it is a bit
odd for a HV OS running a v3.1 processor to set the old LPCR AIL bits,
so it is some kind of bug fix (performance at least).
Could go either way I guess.
Thanks,
Nick
>
> cheers
>
>> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
>> index 1be20bc8dce2..9086a2644c89 100644
>> --- a/arch/powerpc/include/asm/reg.h
>> +++ b/arch/powerpc/include/asm/reg.h
>> @@ -441,6 +441,7 @@
>> #define LPCR_VRMA_LP1 ASM_CONST(0x0000800000000000)
>> #define LPCR_RMLS 0x1C000000 /* Implementation dependent RMO limit sel */
>> #define LPCR_RMLS_SH 26
>> +#define LPCR_HAIL ASM_CONST(0x0000000004000000) /* HV AIL (ISAv3.1) */
>> #define LPCR_ILE ASM_CONST(0x0000000002000000) /* !HV irqs set MSR:LE */
>> #define LPCR_AIL ASM_CONST(0x0000000001800000) /* Alternate interrupt location */
>> #define LPCR_AIL_0 ASM_CONST(0x0000000000000000) /* MMU off exception offset 0x0 */
>> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
>> index 04a31586f760..671192afcdfd 100644
>> --- a/arch/powerpc/kernel/setup_64.c
>> +++ b/arch/powerpc/kernel/setup_64.c
>> @@ -233,10 +233,23 @@ static void cpu_ready_for_interrupts(void)
>> * If we are not in hypervisor mode the job is done once for
>> * the whole partition in configure_exceptions().
>> */
>> - if (cpu_has_feature(CPU_FTR_HVMODE) &&
>> - cpu_has_feature(CPU_FTR_ARCH_207S)) {
>> + if (cpu_has_feature(CPU_FTR_HVMODE)) {
>> unsigned long lpcr = mfspr(SPRN_LPCR);
>> - mtspr(SPRN_LPCR, lpcr | LPCR_AIL_3);
>> + unsigned long new_lpcr = lpcr;
>> +
>> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>> + /* P10 DD1 does not have HAIL */
>> + if (pvr_version_is(PVR_POWER10) &&
>> + (mfspr(SPRN_PVR) & 0xf00) == 0x100)
>> + new_lpcr |= LPCR_AIL_3;
>> + else
>> + new_lpcr |= LPCR_HAIL;
>> + } else if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
>> + new_lpcr |= LPCR_AIL_3;
>> + }
>> +
>> + if (new_lpcr != lpcr)
>> + mtspr(SPRN_LPCR, new_lpcr);
>> }
>>
>> /*
>> --
>> 2.23.0
>
^ permalink raw reply
* Re: [PATCH 1/1] powerpc/smp: Set numa node before updating mask
From: Nathan Lynch @ 2021-04-07 12:19 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Gautham R Shenoy, Peter Zijlstra, Scott Cheloha,
Geetika Moolchandani, Valentin Schneider, Laurent Dufour,
linuxppc-dev, Ingo Molnar
In-Reply-To: <20210402031815.GI2339179@linux.vnet.ibm.com>
Sorry for the delay in following up here.
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
>> > - set_numa_node(numa_cpu_lookup_table[cpu]);
>> > - set_numa_mem(local_memory_node(numa_cpu_lookup_table[cpu]));
>> > -
>>
>> Regardless of your change: at boot time, this set of calls to
>> set_numa_node() and set_numa_mem() is redundant, right? Because
>> smp_prepare_cpus() has:
>>
>> for_each_possible_cpu(cpu) {
>> ...
>> if (cpu_present(cpu)) {
>> set_cpu_numa_node(cpu, numa_cpu_lookup_table[cpu]);
>> set_cpu_numa_mem(cpu,
>> local_memory_node(numa_cpu_lookup_table[cpu]));
>> }
>>
>> I would rather that, when onlining a CPU that happens to have been
>> dynamically added after boot, we enter start_secondary() with conditions
>> equivalent to those at boot time. Or as close to that as is practical.
>>
>> So I'd suggest that pseries_add_processor() be made to update
>> these things when the CPUs are marked present, before onlining them.
>
> In pseries_add_processor, we are only marking the cpu as present. i.e
> I believe numa_setup_cpu() would not have been called. So we may not have a
> way to associate the CPU to the node. Otherwise we will have to call
> numa_setup_cpu() or the hcall_vphn.
>
> We could try calling numa_setup_cpu() immediately after we set the
> CPU to be present, but that would be one more extra hcall + I dont know if
> there are any more steps needed before CPU being made present and
> associating the CPU to the node.
An additional hcall in this path doesn't seem too expensive.
> Are we sure the node is already online?
I see that dlpar_online_cpu() calls find_and_online_cpu_nid(), so yes I
think that's covered.
> For the numa_mem, we are better of if the zonelists for the node are
> built.
>
> or the other solution would be to call this in map_cpu_to_node().
> Here also we have to be sure the zonelists for the node are already
> built.
^ permalink raw reply
* [PATCH] windfarm: make symbol 'wf_thread' static
From: Yu Kuai @ 2021-04-07 12:57 UTC (permalink / raw)
To: benh; +Cc: yukuai3, linuxppc-dev, linux-kernel, zhangxiaoxu5, yi.zhang
The sparse tool complains as follows:
drivers/macintosh/windfarm_core.c:59:20: warning:
symbol 'wf_thread' was not declared. Should it be static?
This symbol is not used outside of windfarm_core.c, so this
commit marks it static.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/macintosh/windfarm_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/macintosh/windfarm_core.c b/drivers/macintosh/windfarm_core.c
index 77612303841e..07f91ec1f960 100644
--- a/drivers/macintosh/windfarm_core.c
+++ b/drivers/macintosh/windfarm_core.c
@@ -56,7 +56,7 @@ static BLOCKING_NOTIFIER_HEAD(wf_client_list);
static int wf_client_count;
static unsigned int wf_overtemp;
static unsigned int wf_overtemp_counter;
-struct task_struct *wf_thread;
+static struct task_struct *wf_thread;
static struct platform_device wf_platform_device = {
.name = "windfarm",
--
2.25.4
^ permalink raw reply related
* [PATCH] macintosh/via-pmu: Make some symbols static
From: Yu Kuai @ 2021-04-07 12:58 UTC (permalink / raw)
To: benh; +Cc: yukuai3, linuxppc-dev, linux-kernel, zhangxiaoxu5, yi.zhang
The sparse tool complains as follows:
drivers/macintosh/via-pmu.c:183:5: warning:
symbol 'pmu_cur_battery' was not declared. Should it be static?
drivers/macintosh/via-pmu.c:190:5: warning:
symbol '__fake_sleep' was not declared. Should it be static?
These symbols are not used outside of via-pmu.c, so this
commit marks them static.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/macintosh/via-pmu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 73e6ae88fafd..478766434919 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -180,14 +180,14 @@ static struct proc_dir_entry *proc_pmu_options;
static int option_server_mode;
int pmu_battery_count;
-int pmu_cur_battery;
+static int pmu_cur_battery;
unsigned int pmu_power_flags = PMU_PWR_AC_PRESENT;
struct pmu_battery_info pmu_batteries[PMU_MAX_BATTERIES];
static int query_batt_timer = BATTERY_POLLING_COUNT;
static struct adb_request batt_req;
static struct proc_dir_entry *proc_pmu_batt[PMU_MAX_BATTERIES];
-int __fake_sleep;
+static int __fake_sleep;
int asleep;
#ifdef CONFIG_ADB
--
2.25.4
^ permalink raw reply related
* [PATCH] tty: hvc: make symbol 'hvc_udbg_dev' static
From: Yu Kuai @ 2021-04-07 12:58 UTC (permalink / raw)
To: gregkh, jirislaby
Cc: yukuai3, linuxppc-dev, linux-kernel, zhangxiaoxu5, yi.zhang
The sparse tool complains as follows:
drivers/tty/hvc/hvc_udbg.c:20:19: warning:
symbol 'hvc_udbg_dev' was not declared. Should it be static?
This symbol is not used outside of hvc_udbg.c, so this
commit marks it static.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/tty/hvc/hvc_udbg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/hvc/hvc_udbg.c b/drivers/tty/hvc/hvc_udbg.c
index a4c9913f76a0..ff0dcc56413c 100644
--- a/drivers/tty/hvc/hvc_udbg.c
+++ b/drivers/tty/hvc/hvc_udbg.c
@@ -17,7 +17,7 @@
#include "hvc_console.h"
-struct hvc_struct *hvc_udbg_dev;
+static struct hvc_struct *hvc_udbg_dev;
static int hvc_udbg_put(uint32_t vtermno, const char *buf, int count)
{
--
2.25.4
^ permalink raw reply related
* [PATCH] powerpc/smp: Make some symbols static
From: Yu Kuai @ 2021-04-07 12:59 UTC (permalink / raw)
To: mpe; +Cc: yukuai3, linuxppc-dev, linux-kernel, zhangxiaoxu5, yi.zhang
The sparse tool complains as follows:
arch/powerpc/kernel/smp.c:86:1: warning:
symbol '__pcpu_scope_cpu_coregroup_map' was not declared. Should it be static?
arch/powerpc/kernel/smp.c:125:1: warning:
symbol '__pcpu_scope_thread_group_l1_cache_map' was not declared. Should it be static?
arch/powerpc/kernel/smp.c:132:1: warning:
symbol '__pcpu_scope_thread_group_l2_cache_map' was not declared. Should it be static?
These symbols are not used outside of smp.c, so this
commit marks them static.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
arch/powerpc/kernel/smp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 5a4d59a1070d..63ccc70bdd0d 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -83,7 +83,7 @@ DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
DEFINE_PER_CPU(cpumask_var_t, cpu_smallcore_map);
DEFINE_PER_CPU(cpumask_var_t, cpu_l2_cache_map);
DEFINE_PER_CPU(cpumask_var_t, cpu_core_map);
-DEFINE_PER_CPU(cpumask_var_t, cpu_coregroup_map);
+static DEFINE_PER_CPU(cpumask_var_t, cpu_coregroup_map);
EXPORT_PER_CPU_SYMBOL(cpu_sibling_map);
EXPORT_PER_CPU_SYMBOL(cpu_l2_cache_map);
@@ -122,14 +122,14 @@ static struct thread_groups_list tgl[NR_CPUS] __initdata;
* On big-cores system, thread_group_l1_cache_map for each CPU corresponds to
* the set its siblings that share the L1-cache.
*/
-DEFINE_PER_CPU(cpumask_var_t, thread_group_l1_cache_map);
+static DEFINE_PER_CPU(cpumask_var_t, thread_group_l1_cache_map);
/*
* On some big-cores system, thread_group_l2_cache_map for each CPU
* corresponds to the set its siblings within the core that share the
* L2-cache.
*/
-DEFINE_PER_CPU(cpumask_var_t, thread_group_l2_cache_map);
+static DEFINE_PER_CPU(cpumask_var_t, thread_group_l2_cache_map);
/* SMP operations for this machine */
struct smp_ops_t *smp_ops;
--
2.25.4
^ permalink raw reply related
* [PATCH] powerpc: Make some symbols static
From: Yu Kuai @ 2021-04-07 12:59 UTC (permalink / raw)
To: mpe; +Cc: yukuai3, linuxppc-dev, linux-kernel, yi.zhang
The sparse tool complains as follows:
arch/powerpc/kernel/btext.c:48:5: warning:
symbol 'boot_text_mapped' was not declared. Should it be static?
arch/powerpc/kernel/btext.c:49:5: warning:
symbol 'force_printk_to_btext' was not declared. Should it be static?
These symbols are not used outside of btext.c, so this
commit marks them static.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
arch/powerpc/kernel/btext.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/btext.c b/arch/powerpc/kernel/btext.c
index 803c2a45b22a..6323304d7f6e 100644
--- a/arch/powerpc/kernel/btext.c
+++ b/arch/powerpc/kernel/btext.c
@@ -45,8 +45,8 @@ unsigned long disp_BAT[2] __initdata = {0, 0};
static unsigned char vga_font[cmapsz];
-int boot_text_mapped __force_data = 0;
-int force_printk_to_btext = 0;
+static int boot_text_mapped __force_data;
+static int force_printk_to_btext;
extern void rmci_on(void);
extern void rmci_off(void);
--
2.25.4
^ permalink raw reply related
* [PATCH] macintosh/windfarm: Make symbol 'pm121_sys_state' static
From: Yu Kuai @ 2021-04-07 12:57 UTC (permalink / raw)
To: benh; +Cc: yukuai3, linuxppc-dev, linux-kernel, zhangxiaoxu5, yi.zhang
The sparse tool complains as follows:
drivers/macintosh/windfarm_pm121.c:436:24: warning:
symbol 'pm121_sys_state' was not declared. Should it be static?
This symbol is not used outside of windfarm_pm121.c, so this
commit marks it static.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/macintosh/windfarm_pm121.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/macintosh/windfarm_pm121.c b/drivers/macintosh/windfarm_pm121.c
index ab467b9c31be..ba1ec6fc11d2 100644
--- a/drivers/macintosh/windfarm_pm121.c
+++ b/drivers/macintosh/windfarm_pm121.c
@@ -433,7 +433,7 @@ struct pm121_sys_state {
struct wf_pid_state pid;
};
-struct pm121_sys_state *pm121_sys_state[N_LOOPS] = {};
+static struct pm121_sys_state *pm121_sys_state[N_LOOPS] = {};
/*
* ****** CPU Fans Control Loop ******
--
2.25.4
^ permalink raw reply related
* Re: [PATCH v2] KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
From: Fabiano Rosas @ 2021-04-07 14:27 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <1617788184.45mdcz310i.astroid@bobo.none>
Nicholas Piggin <npiggin@gmail.com> writes:
<snip>
>> static void restore_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
>> @@ -324,9 +340,10 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
>> mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
>> LPCR_LPES | LPCR_MER;
>> lpcr = (vc->lpcr & ~mask) | (l2_hv.lpcr & mask);
>> - sanitise_hv_regs(vcpu, &l2_hv);
>> restore_hv_regs(vcpu, &l2_hv);
>>
>> + sanitise_vcpu_entry_state(vcpu, &l2_hv, &saved_l1_hv);
>
> So instead of doing this, can we just have one function that does
> load_hv_regs_for_l2()?
Yes. I would go even further and fold everything into a load_l2_state()
that takes care of hv and non-hv. The top level here could easily be:
save_l1_state();
load_l2_state();
do {
kvmhv_run_single_vcpu();
} while();
save_l2_state();
restore_l1_state();
I'll send a v3 with the change you suggested and then perhaps a small
refactoring on top of it. Let's see how it turns out.
>
>> +
>> vcpu->arch.ret = RESUME_GUEST;
>> vcpu->arch.trap = 0;
>> do {
>> @@ -338,6 +355,8 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
>> r = kvmhv_run_single_vcpu(vcpu, hdec_exp, lpcr);
>> } while (is_kvmppc_resume_guest(r));
>>
>> + sanitise_vcpu_return_state(vcpu, &l2_hv);
>
> And this could be done in save_hv_return_state().
>
> I think?
>
> Question about HFSCR. Is it possible for some interrupt cause bit
> reaching the nested hypervisor for a bit that we thought we had
> enabled but was secretly masked off? I.e., do we have to filter
> HFSCR causes according to the facilities we secretly disabled?
Yes, we're copying the Cause bits unmodified. Currently it makes no
difference because L1 only checks for doorbells and everything else
leads to injecting a program interrupt into L2.
What I think is the correct thing to do is to only return into L1 with
the Cause bits pertaining to the facilities it has disabled (if L1 state
has a bit set but L2 state has not).
For the facilities L0 has disabled in L1, we should handle them as if L1
had tried to use the facility and instead of returning from
H_ENTER_NESTED into L1, do whatever we currently do under
BOOK3S_INTERRUPT_H_FAC_UNAVAIL for non-nested guests. Which would
eventually mean injecting a program interrupt into L1 because we're not
L2s hypervisor - L1 is - so there is not much we'd want to do in L0 in
terms of emulating the facility.
Does that make sense?
>
> Thanks,
> Nick
^ permalink raw reply
* Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
From: Luis Chamberlain @ 2021-04-07 14:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Corey Minyard, Linux on Hyper-V List, Tetsuo Handa,
linux-remoteproc, Michael Kelley, Paul Mackerras, H. Peter Anvin,
Joel Fernandes, K. Y. Srinivasan, Thomas Gleixner, Linux-Arch,
Wei Liu, Andy Shevchenko, Stephen Hemminger, Corey Minyard,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Ingo Molnar,
Iurii Zaikin, Ohad Ben-Cohen, Joerg Roedel, Kees Cook,
Paul E. McKenney, Lai Jiangshan, Haiyang Zhang, Josh Triplett,
Steven Rostedt (VMware), rcu, Borislav Petkov, openipmi-developer,
Bjorn Andersson, Vlastimil Babka, Mathieu Poirier, kexec,
Linux Kernel Mailing List, Arnd Bergmann, Eric Biederman,
Linux FS Devel, Mathieu Desnoyers, Andrew Morton,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Mike Rapoport
In-Reply-To: <CAHp75Ve9vBQqSegM2-ch9NUN-MdevxxOs5ZdHkk1W7AacN+Wrw@mail.gmail.com>
On Wed, Apr 07, 2021 at 10:33:44AM +0300, Andy Shevchenko wrote:
> On Wed, Apr 7, 2021 at 10:25 AM Luis Chamberlain <mcgrof@kernel.org> wrote:
> >
> > On Tue, Apr 06, 2021 at 04:31:58PM +0300, Andy Shevchenko wrote:
> > > diff --git a/include/linux/panic_notifier.h b/include/linux/panic_notifier.h
> > > new file mode 100644
> > > index 000000000000..41e32483d7a7
> > > --- /dev/null
> > > +++ b/include/linux/panic_notifier.h
> > > @@ -0,0 +1,12 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +#ifndef _LINUX_PANIC_NOTIFIERS_H
> > > +#define _LINUX_PANIC_NOTIFIERS_H
> > > +
> > > +#include <linux/notifier.h>
> > > +#include <linux/types.h>
> > > +
> > > +extern struct atomic_notifier_head panic_notifier_list;
> > > +
> > > +extern bool crash_kexec_post_notifiers;
> > > +
> > > +#endif /* _LINUX_PANIC_NOTIFIERS_H */
> >
> > Why is it worth it to add another file just for this?
>
> The main point is to break tons of loops that prevent having clean
> headers anymore.
>
> In this case, see bug.h, which is very important in this sense.
OK based on the commit log this was not clear, it seemed more of moving
panic stuff to its own file, so just cleanup.
> > Seems like a very
> > small file.
>
> If it is an argument, it's kinda strange. We have much smaller headers.
The motivation for such separate file was just not clear on the commit
log.
Luis
^ permalink raw reply
* Re: [PATCH v3] pseries: prevent free CPU ids to be reused on another node
From: Nathan Lynch @ 2021-04-07 14:55 UTC (permalink / raw)
To: Laurent Dufour; +Cc: cheloha, linux-kernel, paulus, linuxppc-dev
In-Reply-To: <20210406182554.85197-1-ldufour@linux.ibm.com>
Laurent Dufour <ldufour@linux.ibm.com> writes:
> Changes since V2, addressing Nathan's comments:
> - Remove the retry feature
> - Reduce the number of local variables (removing 'i')
I was more interested in not having two variables for NUMA nodes in the
function named 'node' and 'nid', hoping at least one of them could have
a more descriptive name. See below.
> static int pseries_add_processor(struct device_node *np)
> {
> - unsigned int cpu;
> + unsigned int cpu, node;
> cpumask_var_t candidate_mask, tmp;
> - int err = -ENOSPC, len, nthreads, i;
> + int err = -ENOSPC, len, nthreads, nid;
> const __be32 *intserv;
>
> intserv = of_get_property(np, "ibm,ppc-interrupt-server#s", &len);
> @@ -163,9 +169,17 @@ static int pseries_add_processor(struct device_node *np)
> zalloc_cpumask_var(&candidate_mask, GFP_KERNEL);
> zalloc_cpumask_var(&tmp, GFP_KERNEL);
>
> + /*
> + * Fetch from the DT nodes read by dlpar_configure_connector() the NUMA
> + * node id the added CPU belongs to.
> + */
> + nid = of_node_to_nid(np);
> + if (nid < 0 || !node_possible(nid))
> + nid = first_online_node;
> +
> nthreads = len / sizeof(u32);
> - for (i = 0; i < nthreads; i++)
> - cpumask_set_cpu(i, tmp);
> + for (cpu = 0; cpu < nthreads; cpu++)
> + cpumask_set_cpu(cpu, tmp);
>
> cpu_maps_update_begin();
>
> @@ -173,6 +187,19 @@ static int pseries_add_processor(struct device_node *np)
>
> /* Get a bitmap of unoccupied slots. */
> cpumask_xor(candidate_mask, cpu_possible_mask, cpu_present_mask);
> +
> + /*
> + * Remove free ids previously assigned on the other nodes. We can walk
> + * only online nodes because once a node became online it is not turned
> + * offlined back.
> + */
> + for_each_online_node(node) {
> + if (node == nid) /* Keep our node's recorded ids */
> + continue;
> + cpumask_andnot(candidate_mask, candidate_mask,
> + node_recorded_ids_map[node]);
> + }
> +
e.g. change 'nid' to 'assigned_node' or similar, and I think this
becomes easier to follow.
Otherwise the patch looks fine to me now.
^ permalink raw reply
* Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
From: Andy Shevchenko @ 2021-04-07 14:59 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Corey Minyard, Linux on Hyper-V List, Tetsuo Handa,
linux-remoteproc, Michael Kelley, Paul Mackerras, H. Peter Anvin,
Joel Fernandes, K. Y. Srinivasan, Thomas Gleixner, Linux-Arch,
Wei Liu, Andy Shevchenko, Stephen Hemminger, Corey Minyard,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Ingo Molnar,
Iurii Zaikin, Ohad Ben-Cohen, Joerg Roedel, Kees Cook,
Paul E. McKenney, Lai Jiangshan, Haiyang Zhang, Josh Triplett,
Steven Rostedt (VMware), rcu, Borislav Petkov, openipmi-developer,
Bjorn Andersson, Vlastimil Babka, Mathieu Poirier, kexec,
Linux Kernel Mailing List, Arnd Bergmann, Eric Biederman,
Linux FS Devel, Mathieu Desnoyers, Andrew Morton,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Mike Rapoport
In-Reply-To: <20210407143040.GB4332@42.do-not-panic.com>
On Wed, Apr 7, 2021 at 5:30 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> On Wed, Apr 07, 2021 at 10:33:44AM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 7, 2021 at 10:25 AM Luis Chamberlain <mcgrof@kernel.org> wrote:
> > > On Tue, Apr 06, 2021 at 04:31:58PM +0300, Andy Shevchenko wrote:
...
> > > Why is it worth it to add another file just for this?
> >
> > The main point is to break tons of loops that prevent having clean
> > headers anymore.
> >
> > In this case, see bug.h, which is very important in this sense.
>
> OK based on the commit log this was not clear, it seemed more of moving
> panic stuff to its own file, so just cleanup.
Sorry for that. it should have mentioned the kernel folder instead of
lib. But I think it won't clarify the above.
In any case there are several purposes in this case
- dropping dependency in bug.h
- dropping a loop by moving out panic_notifier.h
- unload kernel.h from something which has its own domain
I think that you are referring to the commit message describing 3rd
one, but not 1st and 2nd.
I will amend this for the future splits, thanks!
> > > Seems like a very
> > > small file.
> >
> > If it is an argument, it's kinda strange. We have much smaller headers.
>
> The motivation for such separate file was just not clear on the commit
> log.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3] pseries: prevent free CPU ids to be reused on another node
From: Laurent Dufour @ 2021-04-07 15:00 UTC (permalink / raw)
To: Nathan Lynch; +Cc: cheloha, linux-kernel, paulus, linuxppc-dev
In-Reply-To: <87blaqkuty.fsf@linux.ibm.com>
Le 07/04/2021 à 16:55, Nathan Lynch a écrit :
> Laurent Dufour <ldufour@linux.ibm.com> writes:
>> Changes since V2, addressing Nathan's comments:
>> - Remove the retry feature
>> - Reduce the number of local variables (removing 'i')
>
> I was more interested in not having two variables for NUMA nodes in the
> function named 'node' and 'nid', hoping at least one of them could have
> a more descriptive name. See below.
>
>> static int pseries_add_processor(struct device_node *np)
>> {
>> - unsigned int cpu;
>> + unsigned int cpu, node;
>> cpumask_var_t candidate_mask, tmp;
>> - int err = -ENOSPC, len, nthreads, i;
>> + int err = -ENOSPC, len, nthreads, nid;
>> const __be32 *intserv;
>>
>> intserv = of_get_property(np, "ibm,ppc-interrupt-server#s", &len);
>> @@ -163,9 +169,17 @@ static int pseries_add_processor(struct device_node *np)
>> zalloc_cpumask_var(&candidate_mask, GFP_KERNEL);
>> zalloc_cpumask_var(&tmp, GFP_KERNEL);
>>
>> + /*
>> + * Fetch from the DT nodes read by dlpar_configure_connector() the NUMA
>> + * node id the added CPU belongs to.
>> + */
>> + nid = of_node_to_nid(np);
>> + if (nid < 0 || !node_possible(nid))
>> + nid = first_online_node;
>> +
>> nthreads = len / sizeof(u32);
>> - for (i = 0; i < nthreads; i++)
>> - cpumask_set_cpu(i, tmp);
>> + for (cpu = 0; cpu < nthreads; cpu++)
>> + cpumask_set_cpu(cpu, tmp);
>>
>> cpu_maps_update_begin();
>>
>> @@ -173,6 +187,19 @@ static int pseries_add_processor(struct device_node *np)
>>
>> /* Get a bitmap of unoccupied slots. */
>> cpumask_xor(candidate_mask, cpu_possible_mask, cpu_present_mask);
>> +
>> + /*
>> + * Remove free ids previously assigned on the other nodes. We can walk
>> + * only online nodes because once a node became online it is not turned
>> + * offlined back.
>> + */
>> + for_each_online_node(node) {
>> + if (node == nid) /* Keep our node's recorded ids */
>> + continue;
>> + cpumask_andnot(candidate_mask, candidate_mask,
>> + node_recorded_ids_map[node]);
>> + }
>> +
>
> e.g. change 'nid' to 'assigned_node' or similar, and I think this
> becomes easier to follow.
Fair enough, will send a v4
> Otherwise the patch looks fine to me now.
>
^ permalink raw reply
* Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
From: Luis Chamberlain @ 2021-04-07 15:29 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Corey Minyard, Linux on Hyper-V List, Tetsuo Handa,
linux-remoteproc, Michael Kelley, Paul Mackerras, H. Peter Anvin,
Joel Fernandes, K. Y. Srinivasan, Thomas Gleixner, Linux-Arch,
Wei Liu, Andy Shevchenko, Stephen Hemminger, Corey Minyard,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Ingo Molnar,
Iurii Zaikin, Ohad Ben-Cohen, Joerg Roedel, Kees Cook,
Paul E. McKenney, Lai Jiangshan, Haiyang Zhang, Josh Triplett,
Steven Rostedt (VMware), rcu, Borislav Petkov, openipmi-developer,
Bjorn Andersson, Vlastimil Babka, Mathieu Poirier, kexec,
Linux Kernel Mailing List, Arnd Bergmann, Eric Biederman,
Linux FS Devel, Mathieu Desnoyers, Andrew Morton,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Mike Rapoport
In-Reply-To: <CAHp75VeXiLa0b49eoZKVR1DSqTc9hKxpSgy294hMiaUzt0ugOA@mail.gmail.com>
On Wed, Apr 07, 2021 at 05:59:19PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 7, 2021 at 5:30 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> > On Wed, Apr 07, 2021 at 10:33:44AM +0300, Andy Shevchenko wrote:
> > > On Wed, Apr 7, 2021 at 10:25 AM Luis Chamberlain <mcgrof@kernel.org> wrote:
> > > > On Tue, Apr 06, 2021 at 04:31:58PM +0300, Andy Shevchenko wrote:
>
> ...
>
> > > > Why is it worth it to add another file just for this?
> > >
> > > The main point is to break tons of loops that prevent having clean
> > > headers anymore.
> > >
> > > In this case, see bug.h, which is very important in this sense.
> >
> > OK based on the commit log this was not clear, it seemed more of moving
> > panic stuff to its own file, so just cleanup.
>
> Sorry for that. it should have mentioned the kernel folder instead of
> lib. But I think it won't clarify the above.
>
> In any case there are several purposes in this case
> - dropping dependency in bug.h
> - dropping a loop by moving out panic_notifier.h
> - unload kernel.h from something which has its own domain
>
> I think that you are referring to the commit message describing 3rd
> one, but not 1st and 2nd.
Right!
> I will amend this for the future splits, thanks!
Don't get me wrong, I love the motivation behind just the 3rd purpose,
however I figured there might be something more when I saw panic_notifier.h.
It was just not clear.
But awesome stuff!
Luis
^ permalink raw reply
* [PATCH v4] pseries: prevent free CPU ids to be reused on another node
From: Laurent Dufour @ 2021-04-07 15:38 UTC (permalink / raw)
To: mpe, benh, paulus, nathanl; +Cc: cheloha, linuxppc-dev, linux-kernel
When a CPU is hot added, the CPU ids are taken from the available mask from
the lower possible set. If that set of values was previously used for CPU
attached to a different node, this seems to application like if these CPUs
have migrated from a node to another one which is not expected in real
life.
To prevent this, it is needed to record the CPU ids used for each node and
to not reuse them on another node. However, to prevent CPU hot plug to
fail, in the case the CPU ids is starved on a node, the capability to reuse
other nodes’ free CPU ids is kept. A warning is displayed in such a case
to warn the user.
A new CPU bit mask (node_recorded_ids_map) is introduced for each possible
node. It is populated with the CPU onlined at boot time, and then when a
CPU is hot plug to a node. The bits in that mask remain when the CPU is hot
unplugged, to remind this CPU ids have been used for this node.
The effect of this patch can be seen by removing and adding CPUs using the
Qemu monitor. In the following case, the first CPU from the node 2 is
removed, then the first one from the node 1 is removed too. Later, the
first CPU of the node 2 is added back. Without that patch, the kernel will
numbered these CPUs using the first CPU ids available which are the ones
freed when removing the second CPU of the node 0. This leads to the CPU ids
16-23 to move from the node 1 to the node 2. With the patch applied, the
CPU ids 32-39 are used since they are the lowest free ones which have not
been used on another node.
At boot time:
[root@vm40 ~]# numactl -H | grep cpus
node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
node 1 cpus: 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 2 cpus: 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
Vanilla kernel, after the CPU hot unplug/plug operations:
[root@vm40 ~]# numactl -H | grep cpus
node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
node 1 cpus: 24 25 26 27 28 29 30 31
node 2 cpus: 16 17 18 19 20 21 22 23 40 41 42 43 44 45 46 47
Patched kernel, after the CPU hot unplug/plug operations:
[root@vm40 ~]# numactl -H | grep cpus
node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
node 1 cpus: 24 25 26 27 28 29 30 31
node 2 cpus: 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
Changes since V3, addressing Nathan's comment:
- Rename the local variable named 'nid' into 'assigned_node'
Changes since V2, addressing Nathan's comments:
- Remove the retry feature
- Reduce the number of local variables (removing 'i')
- Add comment about the cpu_add_remove_lock protecting the added CPU mask.
Changes since V1 (no functional changes):
- update the test's output in the commit's description
- node_recorded_ids_map should be static
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
---
arch/powerpc/platforms/pseries/hotplug-cpu.c | 52 ++++++++++++++++++--
1 file changed, 47 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index ec478f8a98ff..cddf6d2db786 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -39,6 +39,12 @@
/* This version can't take the spinlock, because it never returns */
static int rtas_stop_self_token = RTAS_UNKNOWN_SERVICE;
+/*
+ * Record the CPU ids used on each nodes.
+ * Protected by cpu_add_remove_lock.
+ */
+static cpumask_var_t node_recorded_ids_map[MAX_NUMNODES];
+
static void rtas_stop_self(void)
{
static struct rtas_args args;
@@ -151,9 +157,9 @@ static void pseries_cpu_die(unsigned int cpu)
*/
static int pseries_add_processor(struct device_node *np)
{
- unsigned int cpu;
+ unsigned int cpu, node;
cpumask_var_t candidate_mask, tmp;
- int err = -ENOSPC, len, nthreads, i;
+ int err = -ENOSPC, len, nthreads, assigned_node;
const __be32 *intserv;
intserv = of_get_property(np, "ibm,ppc-interrupt-server#s", &len);
@@ -163,9 +169,17 @@ static int pseries_add_processor(struct device_node *np)
zalloc_cpumask_var(&candidate_mask, GFP_KERNEL);
zalloc_cpumask_var(&tmp, GFP_KERNEL);
+ /*
+ * Fetch from the DT nodes read by dlpar_configure_connector() the NUMA
+ * node id the added CPU belongs to.
+ */
+ assigned_node = of_node_to_nid(np);
+ if (assigned_node < 0 || !node_possible(assigned_node))
+ assigned_node = first_online_node;
+
nthreads = len / sizeof(u32);
- for (i = 0; i < nthreads; i++)
- cpumask_set_cpu(i, tmp);
+ for (cpu = 0; cpu < nthreads; cpu++)
+ cpumask_set_cpu(cpu, tmp);
cpu_maps_update_begin();
@@ -173,6 +187,19 @@ static int pseries_add_processor(struct device_node *np)
/* Get a bitmap of unoccupied slots. */
cpumask_xor(candidate_mask, cpu_possible_mask, cpu_present_mask);
+
+ /*
+ * Remove free ids previously assigned on the other nodes. We can walk
+ * only online nodes because once a node became online it is not turned
+ * offlined back.
+ */
+ for_each_online_node(node) {
+ if (node == assigned_node) /* Keep our node's recorded ids */
+ continue;
+ cpumask_andnot(candidate_mask, candidate_mask,
+ node_recorded_ids_map[node]);
+ }
+
if (cpumask_empty(candidate_mask)) {
/* If we get here, it most likely means that NR_CPUS is
* less than the partition's max processors setting.
@@ -197,6 +224,10 @@ static int pseries_add_processor(struct device_node *np)
goto out_unlock;
}
+ /* Record the newly used CPU ids for the associate node. */
+ cpumask_or(node_recorded_ids_map[assigned_node],
+ node_recorded_ids_map[assigned_node], tmp);
+
for_each_cpu(cpu, tmp) {
BUG_ON(cpu_present(cpu));
set_cpu_present(cpu, true);
@@ -903,6 +934,7 @@ static struct notifier_block pseries_smp_nb = {
static int __init pseries_cpu_hotplug_init(void)
{
int qcss_tok;
+ unsigned int node;
#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
ppc_md.cpu_probe = dlpar_cpu_probe;
@@ -924,8 +956,18 @@ static int __init pseries_cpu_hotplug_init(void)
smp_ops->cpu_die = pseries_cpu_die;
/* Processors can be added/removed only on LPAR */
- if (firmware_has_feature(FW_FEATURE_LPAR))
+ if (firmware_has_feature(FW_FEATURE_LPAR)) {
+ for_each_node(node) {
+ alloc_bootmem_cpumask_var(&node_recorded_ids_map[node]);
+
+ /* Record ids of CPU added at boot time */
+ cpumask_or(node_recorded_ids_map[node],
+ node_recorded_ids_map[node],
+ node_to_cpumask_map[node]);
+ }
+
of_reconfig_notifier_register(&pseries_smp_nb);
+ }
return 0;
}
--
2.31.1
^ permalink raw reply related
* Re: [PATCH 1/1] powerpc/smp: Set numa node before updating mask
From: Srikar Dronamraju @ 2021-04-07 16:49 UTC (permalink / raw)
To: Nathan Lynch
Cc: Gautham R Shenoy, Peter Zijlstra, Scott Cheloha,
Geetika Moolchandani, Valentin Schneider, Laurent Dufour,
linuxppc-dev, Ingo Molnar
In-Reply-To: <87eefml22p.fsf@linux.ibm.com>
* Nathan Lynch <nathanl@linux.ibm.com> [2021-04-07 07:19:10]:
> Sorry for the delay in following up here.
>
No issues.
> >> So I'd suggest that pseries_add_processor() be made to update
> >> these things when the CPUs are marked present, before onlining them.
> >
> > In pseries_add_processor, we are only marking the cpu as present. i.e
> > I believe numa_setup_cpu() would not have been called. So we may not have a
> > way to associate the CPU to the node. Otherwise we will have to call
> > numa_setup_cpu() or the hcall_vphn.
> >
> > We could try calling numa_setup_cpu() immediately after we set the
> > CPU to be present, but that would be one more extra hcall + I dont know if
> > there are any more steps needed before CPU being made present and
> > associating the CPU to the node.
>
> An additional hcall in this path doesn't seem too expensive.
>
> > Are we sure the node is already online?
>
> I see that dlpar_online_cpu() calls find_and_online_cpu_nid(), so yes I
> think that's covered.
Okay,
Can we just call set_cpu_numa_node() at the end of map_cpu_to_node().
The advantage would be the update to numa_cpu_lookup_table and cpu_to_node
would happen at the same time and would be in sync.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH] powerpc: Make some symbols static
From: kernel test robot @ 2021-04-07 16:57 UTC (permalink / raw)
To: Yu Kuai, mpe; +Cc: yukuai3, yi.zhang, kbuild-all, linuxppc-dev, linux-kernel
In-Reply-To: <20210407125942.4140430-1-yukuai3@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 1934 bytes --]
Hi Yu,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.12-rc6 next-20210407]
[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/Yu-Kuai/powerpc-Make-some-symbols-static/20210407-205258
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-defconfig (attached as .config)
compiler: powerpc64-linux-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/7c0f3f68006b9b42ce944b02a2059128cc5826ec
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Yu-Kuai/powerpc-Make-some-symbols-static/20210407-205258
git checkout 7c0f3f68006b9b42ce944b02a2059128cc5826ec
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> arch/powerpc/kernel/btext.c:49:12: error: 'force_printk_to_btext' defined but not used [-Werror=unused-variable]
49 | static int force_printk_to_btext;
| ^~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/force_printk_to_btext +49 arch/powerpc/kernel/btext.c
47
48 static int boot_text_mapped __force_data;
> 49 static int force_printk_to_btext;
50
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26728 bytes --]
^ permalink raw reply
* [PATCH v1 3/8] powerpc/mem: Call flush_coherent_icache() at higher level
From: Christophe Leroy @ 2021-04-07 17:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <311235752428dacbee81728767aacc2bf4222384.1617816138.git.christophe.leroy@csgroup.eu>
flush_coherent_icache() doesn't need the address anymore,
so it can be called immediately when entering the public
functions and doesn't need to be disseminated among
lower level functions.
And use page_to_phys() instead of open coding the calculation
of phys address to call flush_dcache_icache_phys().
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/mm/mem.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 19f807b87697..29ce215e491f 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -491,6 +491,8 @@ static void flush_dcache_icache_hugepage(struct page *page)
void flush_dcache_icache_page(struct page *page)
{
+ if (flush_coherent_icache())
+ return;
if (PageCompound(page))
return flush_dcache_icache_hugepage(page);
@@ -504,11 +506,7 @@ void flush_dcache_icache_page(struct page *page)
__flush_dcache_icache(start);
kunmap_atomic(start);
} else {
- unsigned long addr = page_to_pfn(page) << PAGE_SHIFT;
-
- if (flush_coherent_icache())
- return;
- flush_dcache_icache_phys(addr);
+ flush_dcache_icache_phys(page_to_phys(page));
}
#endif
}
@@ -525,9 +523,6 @@ static void __flush_dcache_icache(void *p)
{
unsigned long addr = (unsigned long)p;
- if (flush_coherent_icache())
- return;
-
clean_dcache_range(addr, addr + PAGE_SIZE);
/*
--
2.25.0
^ permalink raw reply related
* [PATCH v1 4/8] powerpc/mem: Optimise flush_dcache_icache_hugepage()
From: Christophe Leroy @ 2021-04-07 17:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <311235752428dacbee81728767aacc2bf4222384.1617816138.git.christophe.leroy@csgroup.eu>
flush_dcache_icache_hugepage() is a static function, with
only one caller. That caller calls it when PageCompound() is true,
so bugging on !PageCompound() is useless if we can trust the
compiler a little. Remove the BUG_ON(!PageCompound()).
The number of elements of a page won't change over time, but
GCC doesn't know about it, so it gets the value at every iteration.
To avoid that, call compound_nr() outside the loop and save it in
a local variable.
Whether the page is a HIGHMEM page or not doesn't change over time.
But GCC doesn't know it so it does the test on every iteration.
Do the test outside the loop.
When the page is not a HIGHMEM page, page_address() will fallback on
lowmem_page_address(), so call lowmem_page_address() directly and
don't suffer the call to page_address() on every iteration.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/mm/mem.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 29ce215e491f..d2c66827d9fd 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -474,14 +474,14 @@ static void __flush_dcache_icache(void *p);
static void flush_dcache_icache_hugepage(struct page *page)
{
int i;
+ int nr = compound_nr(page);
void *start;
- BUG_ON(!PageCompound(page));
-
- for (i = 0; i < compound_nr(page); i++) {
- if (!PageHighMem(page)) {
- __flush_dcache_icache(page_address(page+i));
- } else {
+ if (!PageHighMem(page)) {
+ for (i = 0; i < nr; i++)
+ __flush_dcache_icache(lowmem_page_address(page + i));
+ } else {
+ for (i = 0; i < nr; i++) {
start = kmap_atomic(page+i);
__flush_dcache_icache(start);
kunmap_atomic(start);
--
2.25.0
^ permalink raw reply related
* [PATCH v1 1/8] powerpc/mem: Declare __flush_dcache_icache() static
From: Christophe Leroy @ 2021-04-07 17:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
__flush_dcache_icache() is only used in mem.c.
Declare it static.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/cacheflush.h | 1 -
arch/powerpc/mm/mem.c | 4 +++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
index f63495109f63..9110489ea411 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -40,7 +40,6 @@ void flush_icache_user_page(struct vm_area_struct *vma, struct page *page,
#define flush_icache_user_page flush_icache_user_page
void flush_dcache_icache_page(struct page *page);
-void __flush_dcache_icache(void *page);
/**
* flush_dcache_range(): Write any modified data cache blocks out to memory and
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 7a59a5c9aa5d..ce6c81ce4362 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -472,6 +472,8 @@ void flush_dcache_page(struct page *page)
}
EXPORT_SYMBOL(flush_dcache_page);
+static void __flush_dcache_icache(void *p);
+
static void flush_dcache_icache_hugepage(struct page *page)
{
int i;
@@ -522,7 +524,7 @@ EXPORT_SYMBOL(flush_dcache_icache_page);
*
* @page: the address of the page to flush
*/
-void __flush_dcache_icache(void *p)
+static void __flush_dcache_icache(void *p)
{
unsigned long addr = (unsigned long)p;
--
2.25.0
^ permalink raw reply related
* [PATCH v1 2/8] powerpc/mem: Remove address argument to flush_coherent_icache()
From: Christophe Leroy @ 2021-04-07 17:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <311235752428dacbee81728767aacc2bf4222384.1617816138.git.christophe.leroy@csgroup.eu>
flush_coherent_icache() can use any valid address as mentionned
by the comment.
Use PAGE_OFFSET as base address. This allows removing the
user access stuff.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/mm/mem.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index ce6c81ce4362..19f807b87697 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -342,10 +342,9 @@ void free_initmem(void)
/**
* flush_coherent_icache() - if a CPU has a coherent icache, flush it
- * @addr: The base address to use (can be any valid address, the whole cache will be flushed)
* Return true if the cache was flushed, false otherwise
*/
-static inline bool flush_coherent_icache(unsigned long addr)
+static inline bool flush_coherent_icache(void)
{
/*
* For a snooping icache, we still need a dummy icbi to purge all the
@@ -355,9 +354,7 @@ static inline bool flush_coherent_icache(unsigned long addr)
*/
if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) {
mb(); /* sync */
- allow_read_from_user((const void __user *)addr, L1_CACHE_BYTES);
- icbi((void *)addr);
- prevent_read_from_user((const void __user *)addr, L1_CACHE_BYTES);
+ icbi((void *)PAGE_OFFSET);
mb(); /* sync */
isync();
return true;
@@ -397,7 +394,7 @@ static void invalidate_icache_range(unsigned long start, unsigned long stop)
*/
void flush_icache_range(unsigned long start, unsigned long stop)
{
- if (flush_coherent_icache(start))
+ if (flush_coherent_icache())
return;
clean_dcache_range(start, stop);
@@ -509,7 +506,7 @@ void flush_dcache_icache_page(struct page *page)
} else {
unsigned long addr = page_to_pfn(page) << PAGE_SHIFT;
- if (flush_coherent_icache(addr))
+ if (flush_coherent_icache())
return;
flush_dcache_icache_phys(addr);
}
@@ -528,7 +525,7 @@ static void __flush_dcache_icache(void *p)
{
unsigned long addr = (unsigned long)p;
- if (flush_coherent_icache(addr))
+ if (flush_coherent_icache())
return;
clean_dcache_range(addr, addr + PAGE_SIZE);
--
2.25.0
^ permalink raw reply related
* [PATCH v1 5/8] powerpc/mem: flush_dcache_icache_phys() is for HIGHMEM pages only
From: Christophe Leroy @ 2021-04-07 17:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <311235752428dacbee81728767aacc2bf4222384.1617816138.git.christophe.leroy@csgroup.eu>
__flush_dcache_icache() is usable for non HIGHMEM pages on
every platform.
It is only for HIGHMEM pages that BOOKE needs kmap() and
BOOK3S needs flush_dcache_icache_phys().
So make flush_dcache_icache_phys() dependent on CONFIG_HIGHMEM and
call it only when it is a HIGHMEM page.
We could make flush_dcache_icache_phys() available at all time,
but as it is declared NOKPROBE_SYMBOL(), GCC doesn't optimise
it out when it is not used.
So define a stub for !CONFIG_HIGHMEM in order to remove the #ifdef in
flush_dcache_icache_page() and use IS_ENABLED() instead.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/mm/mem.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index d2c66827d9fd..9a5542f4de92 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -413,7 +413,7 @@ void flush_icache_range(unsigned long start, unsigned long stop)
}
EXPORT_SYMBOL(flush_icache_range);
-#if !defined(CONFIG_PPC_8xx) && !defined(CONFIG_PPC64)
+#ifdef CONFIG_HIGHMEM
/**
* flush_dcache_icache_phys() - Flush a page by it's physical address
* @physaddr: the physical address of the page
@@ -452,7 +452,11 @@ static void flush_dcache_icache_phys(unsigned long physaddr)
: "ctr", "memory");
}
NOKPROBE_SYMBOL(flush_dcache_icache_phys)
-#endif // !defined(CONFIG_PPC_8xx) && !defined(CONFIG_PPC64)
+#else
+static void flush_dcache_icache_phys(unsigned long physaddr)
+{
+}
+#endif
/*
* This is called when a page has been modified by the kernel.
@@ -497,18 +501,15 @@ void flush_dcache_icache_page(struct page *page)
if (PageCompound(page))
return flush_dcache_icache_hugepage(page);
-#if defined(CONFIG_PPC_8xx) || defined(CONFIG_PPC64)
- /* On 8xx there is no need to kmap since highmem is not supported */
- __flush_dcache_icache(page_address(page));
-#else
- if (IS_ENABLED(CONFIG_BOOKE) || sizeof(phys_addr_t) > sizeof(void *)) {
+ if (!PageHighMem(page)) {
+ __flush_dcache_icache(lowmem_page_address(page));
+ } else if (IS_ENABLED(CONFIG_BOOKE) || sizeof(phys_addr_t) > sizeof(void *)) {
void *start = kmap_atomic(page);
__flush_dcache_icache(start);
kunmap_atomic(start);
} else {
flush_dcache_icache_phys(page_to_phys(page));
}
-#endif
}
EXPORT_SYMBOL(flush_dcache_icache_page);
--
2.25.0
^ 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