Re: [PATCH v2 1/1] powerpc/iommu: Enable remaining IOMMU Pagesizes present in LoPAR
Leonardo Bras writes:
> According to LoPAR, ibm,query-pe-dma-window output named "IO Page Sizes"
> will let the OS know all possible pagesizes that can be used for creating a
> new DDW.
>
> Currently Linux will only try using 3 of the 8 available options:
> 4K, 64K and 16M. According to LoPAR, Hypervisor may also offer 32M, 64M,
> 128M, 256M and 16G.
Do we know of any hardware & hypervisor combination that will actually
give us bigger pages?
> Enabling bigger pages would be interesting for direct mapping systems
> with a lot of RAM, while using less TCE entries.
>
> Signed-off-by: Leonardo Bras
> ---
> arch/powerpc/platforms/pseries/iommu.c | 49 ++
> 1 file changed, 42 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c
> b/arch/powerpc/platforms/pseries/iommu.c
> index 9fc5217f0c8e..6cda1c92597d 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -53,6 +53,20 @@ enum {
> DDW_EXT_QUERY_OUT_SIZE = 2
> };
A comment saying where the values come from would be good.
> +#define QUERY_DDW_PGSIZE_4K 0x01
> +#define QUERY_DDW_PGSIZE_64K 0x02
> +#define QUERY_DDW_PGSIZE_16M 0x04
> +#define QUERY_DDW_PGSIZE_32M 0x08
> +#define QUERY_DDW_PGSIZE_64M 0x10
> +#define QUERY_DDW_PGSIZE_128M0x20
> +#define QUERY_DDW_PGSIZE_256M0x40
> +#define QUERY_DDW_PGSIZE_16G 0x80
I'm not sure the #defines really gain us much vs just putting the
literal values in the array below?
> +struct iommu_ddw_pagesize {
> + u32 mask;
> + int shift;
> +};
> +
> static struct iommu_table_group *iommu_pseries_alloc_group(int node)
> {
> struct iommu_table_group *table_group;
> @@ -1099,6 +1113,31 @@ static void reset_dma_window(struct pci_dev *dev,
> struct device_node *par_dn)
>ret);
> }
>
> +/* Returns page shift based on "IO Page Sizes" output at
> ibm,query-pe-dma-window. See LoPAR */
> +static int iommu_get_page_shift(u32 query_page_size)
> +{
> + const struct iommu_ddw_pagesize ddw_pagesize[] = {
> + { QUERY_DDW_PGSIZE_16G, __builtin_ctz(SZ_16G) },
> + { QUERY_DDW_PGSIZE_256M, __builtin_ctz(SZ_256M) },
> + { QUERY_DDW_PGSIZE_128M, __builtin_ctz(SZ_128M) },
> + { QUERY_DDW_PGSIZE_64M, __builtin_ctz(SZ_64M) },
> + { QUERY_DDW_PGSIZE_32M, __builtin_ctz(SZ_32M) },
> + { QUERY_DDW_PGSIZE_16M, __builtin_ctz(SZ_16M) },
> + { QUERY_DDW_PGSIZE_64K, __builtin_ctz(SZ_64K) },
> + { QUERY_DDW_PGSIZE_4K, __builtin_ctz(SZ_4K) }
> + };
cheers
Re: [PATCH 2/2] powerpc: make 'boot_text_mapped' static
Le 08/04/2021 à 03:18, Yu Kuai a écrit :
The sparse tool complains as follow:
arch/powerpc/kernel/btext.c:48:5: warning:
symbol 'boot_text_mapped' was not declared. Should it be static?
This symbol is not used outside of btext.c, so this commit make
it static.
Signed-off-by: Yu Kuai
---
arch/powerpc/kernel/btext.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/btext.c b/arch/powerpc/kernel/btext.c
index 359d0f4ca532..8df9230be6fa 100644
--- a/arch/powerpc/kernel/btext.c
+++ b/arch/powerpc/kernel/btext.c
@@ -45,7 +45,7 @@ unsigned long disp_BAT[2] __initdata = {0, 0};
static unsigned char vga_font[cmapsz];
-int boot_text_mapped __force_data = 0;
+static int boot_text_mapped __force_data;
Are you sure the initialisation to 0 can be removed ? Usually initialisation to 0 is not needed
because not initialised variables go in the BSS section which is zeroed at startup. But here the
variable is flagged with __force_data so it is not going in the BSS section.
extern void rmci_on(void);
extern void rmci_off(void);
Re: [PATCH 1/2] powerpc: remove set but not used variable 'force_printk_to_btext'
Le 08/04/2021 à 03:18, Yu Kuai a écrit :
Fixes gcc '-Wunused-but-set-variable' warning:
arch/powerpc/kernel/btext.c:49:12: error: 'force_printk_to_btext'
defined but not used.
You don't get this error as it is now.
You will get this error only if you make it 'static', which is what you did in your first patch
based on the 'sparse' report.
When removing a non static variable, you should explain that you can remove it after you have
verifier that it is nowhere used, neither in that file nor in any other one.
It is never used, and so can be removed.
Signed-off-by: Yu Kuai
---
arch/powerpc/kernel/btext.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/kernel/btext.c b/arch/powerpc/kernel/btext.c
index 803c2a45b22a..359d0f4ca532 100644
--- a/arch/powerpc/kernel/btext.c
+++ b/arch/powerpc/kernel/btext.c
@@ -46,7 +46,6 @@ 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;
extern void rmci_on(void);
extern void rmci_off(void);
Re: [PATCH-next] powerpc/interrupt: Remove duplicate header file
Le 08/04/2021 à 05:56, johnny.che...@huawei.com a écrit : From: Chen Yi Delete one of the header files that are included twice. Guys, we have been flooded with such tiny patches over the last weeks, some changes being sent several times by different people. That one is included in https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210323062916.295346-1-wanjiab...@vivo.com/ And was already submitted a few hours earlier by someone else: https://patchwork.ozlabs.org/project/linuxppc-dev/patch/1616464656-59372-1-git-send-email-zhouchuan...@vivo.com/ Could you work all together and cook an overall patch including all duplicate removal from arch/powerpc/ files ? Best way would be I think to file an issue at https://github.com/linuxppc/issues/issues , then you do a complete analysis and list in the issue all places to be modified, then once the analysis is complete you send a full single patch. Thanks Christophe Signed-off-by: Chen Yi --- arch/powerpc/kernel/interrupt.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c index c4dd4b8f9cfa..f64ace0208b7 100644 --- a/arch/powerpc/kernel/interrupt.c +++ b/arch/powerpc/kernel/interrupt.c @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include
Re: [PATCH] powerpc: remove old workaround for GCC < 4.9
Le 08/04/2021 à 05:05, Masahiro Yamada a écrit : According to Documentation/process/changes.rst, the minimum supported GCC version is 4.9. This workaround is dead code. This workaround is already on the way out, see https://github.com/linuxppc/linux/commit/802b5560393423166e436c7914b565f3cda9e6b9 Signed-off-by: Masahiro Yamada --- arch/powerpc/Makefile | 6 -- 1 file changed, 6 deletions(-) diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index 5f8544cf724a..32dd693b4e42 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile @@ -181,12 +181,6 @@ CC_FLAGS_FTRACE := -pg ifdef CONFIG_MPROFILE_KERNEL CC_FLAGS_FTRACE += -mprofile-kernel endif -# Work around gcc code-gen bugs with -pg / -fno-omit-frame-pointer in gcc <= 4.8 -# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199 -# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52828 -ifndef CONFIG_CC_IS_CLANG -CC_FLAGS_FTRACE+= $(call cc-ifversion, -lt, 0409, -mno-sched-epilog) -endif endif CFLAGS-$(CONFIG_TARGET_CPU_BOOL) += $(call cc-option,-mcpu=$(CONFIG_TARGET_CPU))
[PATCH-next] powerpc/interrupt: Remove duplicate header file
From: Chen Yi Delete one of the header files that are included twice. Signed-off-by: Chen Yi --- arch/powerpc/kernel/interrupt.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c index c4dd4b8f9cfa..f64ace0208b7 100644 --- a/arch/powerpc/kernel/interrupt.c +++ b/arch/powerpc/kernel/interrupt.c @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include -- 2.31.0
[PATCH -next] powerpc/mce: Make symbol 'mce_ue_event_work' static
The sparse tool complains as follows:
arch/powerpc/kernel/mce.c:43:1: warning:
symbol 'mce_ue_event_work' was not declared. Should it be static?
This symbol is not used outside of mce.c, so this commit marks it
static.
Signed-off-by: Li Huafei
---
arch/powerpc/kernel/mce.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index 11f0cae086ed..6aa6b1cda1ed 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -40,7 +40,7 @@ static struct irq_work mce_ue_event_irq_work = {
.func = machine_check_ue_irq_work,
};
-DECLARE_WORK(mce_ue_event_work, machine_process_ue_event);
+static DECLARE_WORK(mce_ue_event_work, machine_process_ue_event);
static BLOCKING_NOTIFIER_HEAD(mce_notifier_list);
--
2.17.1
[PATCH -next] powerpc/security: Make symbol 'stf_barrier' static
The sparse tool complains as follows:
arch/powerpc/kernel/security.c:253:6: warning:
symbol 'stf_barrier' was not declared. Should it be static?
This symbol is not used outside of security.c, so this commit marks it
static.
Signed-off-by: Li Huafei
---
arch/powerpc/kernel/security.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index e4e1a94ccf6a..4de6bbd9672e 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -250,7 +250,7 @@ ssize_t cpu_show_spectre_v2(struct device *dev, struct
device_attribute *attr, c
static enum stf_barrier_type stf_enabled_flush_types;
static bool no_stf_barrier;
-bool stf_barrier;
+static bool stf_barrier;
static int __init handle_no_stf_barrier(char *p)
{
--
2.17.1
[PATCH] powerpc: remove old workaround for GCC < 4.9
According to Documentation/process/changes.rst, the minimum supported GCC version is 4.9. This workaround is dead code. Signed-off-by: Masahiro Yamada --- arch/powerpc/Makefile | 6 -- 1 file changed, 6 deletions(-) diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index 5f8544cf724a..32dd693b4e42 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile @@ -181,12 +181,6 @@ CC_FLAGS_FTRACE := -pg ifdef CONFIG_MPROFILE_KERNEL CC_FLAGS_FTRACE += -mprofile-kernel endif -# Work around gcc code-gen bugs with -pg / -fno-omit-frame-pointer in gcc <= 4.8 -# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199 -# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52828 -ifndef CONFIG_CC_IS_CLANG -CC_FLAGS_FTRACE+= $(call cc-ifversion, -lt, 0409, -mno-sched-epilog) -endif endif CFLAGS-$(CONFIG_TARGET_CPU_BOOL) += $(call cc-option,-mcpu=$(CONFIG_TARGET_CPU)) -- 2.27.0
[PATCH v7] soc: fsl: enable acpi support in RCPM driver
From: Peng Ma This patch enables ACPI support in RCPM driver. Signed-off-by: Peng Ma Signed-off-by: Ran Wang --- Change in v7: - Update comment for checking RCPM node which refferred to Change in v6: - Remove copyright udpate to rebase on latest mainline Change in v5: - Fix panic when dev->of_node is null Change in v4: - Make commit subject more accurate - Remove unrelated new blank line Change in v3: - Add #ifdef CONFIG_ACPI for acpi_device_id - Rename rcpm_acpi_imx_ids to rcpm_acpi_ids Change in v2: - Update acpi_device_id to fix conflict with other driver drivers/soc/fsl/rcpm.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c index 4ace28cab314..90d3f4060b0c 100644 --- a/drivers/soc/fsl/rcpm.c +++ b/drivers/soc/fsl/rcpm.c @@ -13,6 +13,7 @@ #include #include #include +#include #define RCPM_WAKEUP_CELL_MAX_SIZE 7 @@ -78,10 +79,20 @@ static int rcpm_pm_prepare(struct device *dev) "fsl,rcpm-wakeup", value, rcpm->wakeup_cells + 1); - /* Wakeup source should refer to current rcpm device */ - if (ret || (np->phandle != value[0])) + if (ret) continue; + /* +* For DT mode, would handle devices with "fsl,rcpm-wakeup" +* pointing to the current RCPM node. +* +* For ACPI mode, currently we assume there is only one +* RCPM controller existing. +*/ + if (is_of_node(dev->fwnode)) + if (np->phandle != value[0]) + continue; + /* Property "#fsl,rcpm-wakeup-cells" of rcpm node defines the * number of IPPDEXPCR register cells, and "fsl,rcpm-wakeup" * of wakeup source IP contains an integer array:
Re: [PATCH v3 1/2] powerpc/perf: Infrastructure to support checking of attr.config*
On 4/7/21 5:08 PM, Michael Ellerman wrote: Madhavan Srinivasan 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 Reason for not including HARDWARE and CACHE events are thats, they are straight forward, meaning they dont use sampling or thresholding features. Currently, checks are mostly in that spaces to check for any invalid values. We could include the check for all types the events. I will respin the patch with that change Thanks for review Maddy event. cheers
Re: [PATCH] powerpc: Make some symbols static
On 2021/04/08 0:57, kernel test robot wrote: 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 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 Will remove this variable in another patch. Thanks Yu Kuai --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
[PATCH 0/2] code optimizations for btext.c
Yu Kuai (2): powerpc: remove set but not used variable 'force_printk_to_btext' powerpc: make 'boot_text_mapped' static arch/powerpc/kernel/btext.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) -- 2.25.4
[PATCH 1/2] powerpc: remove set but not used variable 'force_printk_to_btext'
Fixes gcc '-Wunused-but-set-variable' warning:
arch/powerpc/kernel/btext.c:49:12: error: 'force_printk_to_btext'
defined but not used.
It is never used, and so can be removed.
Signed-off-by: Yu Kuai
---
arch/powerpc/kernel/btext.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/kernel/btext.c b/arch/powerpc/kernel/btext.c
index 803c2a45b22a..359d0f4ca532 100644
--- a/arch/powerpc/kernel/btext.c
+++ b/arch/powerpc/kernel/btext.c
@@ -46,7 +46,6 @@ 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;
extern void rmci_on(void);
extern void rmci_off(void);
--
2.25.4
[PATCH 2/2] powerpc: make 'boot_text_mapped' static
The sparse tool complains as follow:
arch/powerpc/kernel/btext.c:48:5: warning:
symbol 'boot_text_mapped' was not declared. Should it be static?
This symbol is not used outside of btext.c, so this commit make
it static.
Signed-off-by: Yu Kuai
---
arch/powerpc/kernel/btext.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/btext.c b/arch/powerpc/kernel/btext.c
index 359d0f4ca532..8df9230be6fa 100644
--- a/arch/powerpc/kernel/btext.c
+++ b/arch/powerpc/kernel/btext.c
@@ -45,7 +45,7 @@ unsigned long disp_BAT[2] __initdata = {0, 0};
static unsigned char vga_font[cmapsz];
-int boot_text_mapped __force_data = 0;
+static int boot_text_mapped __force_data;
extern void rmci_on(void);
extern void rmci_off(void);
--
2.25.4
Re: [PATCH v2 1/1] powerpc/iommu: Enable remaining IOMMU Pagesizes present in LoPAR
Hi Leonardo,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on powerpc/next]
[also build test WARNING 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/Leonardo-Bras/powerpc-iommu-Enable-remaining-IOMMU-Pagesizes-present-in-LoPAR/20210408-035800
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (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/faa8b10e5b9652dbd56ed8e759a1cc09b95805be
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Leonardo-Bras/powerpc-iommu-Enable-remaining-IOMMU-Pagesizes-present-in-LoPAR/20210408-035800
git checkout faa8b10e5b9652dbd56ed8e759a1cc09b95805be
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
In file included from include/vdso/const.h:5,
from include/linux/const.h:4,
from include/linux/bits.h:5,
from include/linux/bitops.h:6,
from include/linux/kernel.h:11,
from include/asm-generic/bug.h:20,
from arch/powerpc/include/asm/bug.h:109,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/slab.h:15,
from arch/powerpc/platforms/pseries/iommu.c:15:
arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_get_page_shift':
>> include/uapi/linux/const.h:20:19: warning: conversion from 'long long
>> unsigned int' to 'unsigned int' changes value from '17179869184' to '0'
>> [-Woverflow]
20 | #define __AC(X,Y) (X##Y)
| ^~
include/uapi/linux/const.h:21:18: note: in expansion of macro '__AC'
21 | #define _AC(X,Y) __AC(X,Y)
| ^~~~
include/linux/sizes.h:48:19: note: in expansion of macro '_AC'
48 | #define SZ_16G_AC(0x4, ULL)
| ^~~
arch/powerpc/platforms/pseries/iommu.c:1120:42: note: in expansion of macro
'SZ_16G'
1120 | { QUERY_DDW_PGSIZE_16G, __builtin_ctz(SZ_16G) },
| ^~
vim +20 include/uapi/linux/const.h
9d291e787b2b71 include/asm-x86_64/const.h Vivek Goyal 2007-05-02 6
9d291e787b2b71 include/asm-x86_64/const.h Vivek Goyal 2007-05-02 7
/* Some constant macros are used in both assembler and
9d291e787b2b71 include/asm-x86_64/const.h Vivek Goyal 2007-05-02 8
* C code. Therefore we cannot annotate them always with
6df95fd7ad9a84 include/linux/const.h Randy Dunlap2007-05-08 9
* 'UL' and other type specifiers unilaterally. We
9d291e787b2b71 include/asm-x86_64/const.h Vivek Goyal 2007-05-02 10
* use the following macros to deal with this.
74ef649fe847fd include/linux/const.h Jeremy Fitzhardinge 2008-01-30 11 *
74ef649fe847fd include/linux/const.h Jeremy Fitzhardinge 2008-01-30 12
* Similarly, _AT() will cast an expression with a type in C, but
74ef649fe847fd include/linux/const.h Jeremy Fitzhardinge 2008-01-30 13
* leave it unchanged in asm.
9d291e787b2b71 include/asm-x86_64/const.h Vivek Goyal 2007-05-02 14
*/
9d291e787b2b71 include/asm-x86_64/const.h Vivek Goyal 2007-05-02 15
9d291e787b2b71 include/asm-x86_64/const.h Vivek Goyal 2007-05-02 16
#ifdef __ASSEMBLY__
9d291e787b2b71 include/asm-x86_64/const.h Vivek Goyal 2007-05-02 17
#define _AC(X,Y) X
74ef649fe847fd include/linux/const.h Jeremy Fitzhardinge 2008-01-30 18
#define _AT(T,X) X
9d291e787b2b71 include/asm-x86_64/const.h Vivek Goyal 2007-05-02 19
#else
9d291e787b2b71 include/asm-x86_64/const.h Vivek Goyal 2007-05-02 @20
#define __AC(X,Y) (X##Y)
9d291e787b2b71 include/asm-x86_64/const.h Vivek Goyal 2007-05-02 21
#define _AC(X,Y) __AC(X,Y)
74ef649fe847fd include/linux/const.h Jeremy Fitzhardinge 2008-01-30 22
#define _AT(T,X) ((T)(X))
9d291e787b2b71 include/asm-x86_64/const.h Vivek Goyal 2007-05-02 23
#endif
9d291e787b2b71 include/asm-x86_64/const.h Vivek Goyal 2007-05-02 24
---
0-DAY CI Kernel Test Se
Re: [PATCH] sound:ppc: fix spelling typo of values
On 3/23/21 1:55 AM, caizhichao wrote: > From: caizhichao > > vaules -> values > > Signed-off-by: caizhichao > --- > sound/ppc/snd_ps3_reg.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Seems fine. Thanks for your contribution. Acked-by: Geoff Levand
Re: [PATCH net-next v3 1/2] of: net: pass the dst buffer to of_get_mac_address()
Am 2021-04-07 00:09, schrieb Michael Walle:
[..]
diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index bc0a27de69d4..2d5d5e59aea5 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -45,42 +45,35 @@ int of_get_phy_mode(struct device_node *np,
phy_interface_t *interface)
}
EXPORT_SYMBOL_GPL(of_get_phy_mode);
-static const void *of_get_mac_addr(struct device_node *np, const char
*name)
+static int of_get_mac_addr(struct device_node *np, const char *name,
u8 *addr)
{
struct property *pp = of_find_property(np, name, NULL);
- if (pp && pp->length == ETH_ALEN && is_valid_ether_addr(pp->value))
- return pp->value;
- return NULL;
+ if (pp && pp->length == ETH_ALEN && is_valid_ether_addr(pp->value)) {
+ ether_addr_copy(addr, pp->value);
Mh, I guess this should rather be memcpy(addr, pp->value, ETH_ALEN)
because
ether_addr_copy() needs 2 byte aligned source and destination buffers.
-michael
Re: [PATCH 2/8] CMDLINE: drivers: of: ifdef out cmdline section
On Tue, Mar 30, 2021 at 04:17:53PM -0700, Daniel Walker wrote: > On Tue, Mar 30, 2021 at 02:49:13PM -0500, Rob Herring wrote: > > On Tue, Mar 30, 2021 at 12:57 PM Daniel Walker wrote: > > > > > > It looks like there's some seepage of cmdline stuff into > > > the generic device tree code. This conflicts with the > > > generic cmdline implementation so I remove it in the case > > > when that's enabled. > > > > > > Cc: xe-linux-exter...@cisco.com > > > Signed-off-by: Ruslan Ruslichenko > > > Signed-off-by: Daniel Walker > > > --- > > > drivers/of/fdt.c | 14 ++ > > > 1 file changed, 14 insertions(+) > > > > > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c > > > index dcc1dd96911a..d8805cd9717a 100644 > > > --- a/drivers/of/fdt.c > > > +++ b/drivers/of/fdt.c > > > @@ -25,6 +25,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > > > > #include /* for COMMAND_LINE_SIZE */ > > > #include > > > @@ -1050,6 +1051,18 @@ int __init early_init_dt_scan_chosen(unsigned long > > > node, const char *uname, > > > > > > /* Retrieve command line */ > > > p = of_get_flat_dt_prop(node, "bootargs", ); > > > + > > > +#if defined(CONFIG_GENERIC_CMDLINE) && defined(CONFIG_GENERIC_CMDLINE_OF) > > > > Moving in the wrong direction... This code already has too many > > #ifdef's. I like Christophe's version as it gets rid of all the code > > here. > > It's temporary .. Notice CONFIG_GENERIC_CMDLINE_OF is only used on PowerPC. I > experienced doubling on arm64 when this was used (i.e. the append and prepend > was added twice). > > I don't think there are any other users which can't be moved outside the > device > tree code, but powerpc uses this function three times during boot up plus the > prom_init user. It's possible to use the generic command line in all four > places, > but it become space inefficient. What's the 3rd use? I count kaslr code and in early_init_dt_scan_chosen_ppc. Do we need to build the command line for kaslr seed? Getting any build time value from the kernel is pointless. Rob
Re: [OpenRISC] [PATCH v6 1/9] locking/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
On Wed, Apr 07, 2021 at 11:47:49AM +0200, Peter Zijlstra wrote: > On Wed, Apr 07, 2021 at 08:52:08AM +0900, Stafford Horne wrote: > > Why doesn't RISC-V add the xchg16 emulation code similar to OpenRISC? For > > OpenRISC we added xchg16 and xchg8 emulation code to enable qspinlocks. So > > one thought is with CONFIG_ARCH_USE_QUEUED_SPINLOCKS_XCHG32=y, can we > > remove our > > xchg16/xchg8 emulation code? > > CONFIG_ARCH_USE_QUEUED_SPINLOCKS_XCHG32 is guaranteed crap. > > All the architectures that have wanted it are RISC style LL/SC archs, > and for them a cmpxchg loop is a daft thing to do, since it reduces the > chance of it behaving sanely. > > Why would we provide something that's known to be suboptimal? If an > architecture chooses to not care about determinism and or fwd progress, > then that's their choice. But not one, I feel, we should encourage. Thanks, this is the response I was hoping my comment would provoke. So not enabling CONFIG_ARCH_USE_QUEUED_SPINLOCKS_XCHG32 for architectures unless they really want it should be the way. -Stafford
Re: [PATCH] powerpc/dts: fix not include DTC_FLAGS
On Wed, Apr 7, 2021 at 6:27 AM Michael Ellerman wrote:
>
> Youlin Song writes:
> > I wanted to build the fsl dts in my machine and found that
> > the dtb have not extra space,so uboot will cause about
> > FDT_ERR_NOSPACE issue.
How do we not have issues with arm and arm64 boards which don't have
padding? Or what took so long to notice on powerpc?
> >
> > Signed-off-by: Youlin Song
> > ---
> > arch/powerpc/boot/dts/Makefile | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/powerpc/boot/dts/Makefile b/arch/powerpc/boot/dts/Makefile
> > index fb335d05aae8..c21165c0cd76 100644
> > --- a/arch/powerpc/boot/dts/Makefile
> > +++ b/arch/powerpc/boot/dts/Makefile
> > @@ -2,5 +2,6 @@
> >
> > subdir-y += fsl
> >
> > +DTC_FLAGS ?= -p 1024
> > dtstree := $(srctree)/$(src)
> > dtb-$(CONFIG_OF_ALL_DTBS) := $(patsubst $(dtstree)/%.dts,%.dtb, $(wildcard
> > $(dtstree)/*.dts))
>
> I guess that was missed in 1acf1cf8638a ("powerpc: build .dtb files in dts
> directory").
>
> Which I think means the assignment to DTC_FLAGS in
> arch/powerpc/boot/Makefile is not needed anymore.
>
> Can you send a v2 removing that assignment and explaining that's what
> happened?
I've wanted to make this common, but I guess that's a separate change.
Rob
Re: [PATCH 1/1] powerpc/iommu: Enable remaining IOMMU Pagesizes present in LoPAR
Hello Alexey, On Tue, 2021-03-23 at 18:41 +1100, Alexey Kardashevskiy wrote: [...] > > +#define IOMMU_PAGE_SHIFT_16G 34 > > +#define IOMMU_PAGE_SHIFT_256M 28 > > +#define IOMMU_PAGE_SHIFT_128M 27 > > +#define IOMMU_PAGE_SHIFT_64M 26 > > +#define IOMMU_PAGE_SHIFT_32M 25 > > +#define IOMMU_PAGE_SHIFT_16M 24 > > +#define IOMMU_PAGE_SHIFT_64K 16 > > > These are not very descriptive, these are just normal shifts, could be > as simple as __builtin_ctz(SZ_4K) (gcc will optimize this) and so on. > > OTOH the PAPR page sizes need macros as they are the ones which are > weird and screaming for macros. > > I'd steal/rework spapr_page_mask_to_query_mask() from QEMU. Thanks, > Thanks for this feedback! I just sent a v2 applying your suggestions. Best regards, Leonardo Bras
[PATCH v2 1/1] powerpc/iommu: Enable remaining IOMMU Pagesizes present in LoPAR
According to LoPAR, ibm,query-pe-dma-window output named "IO Page Sizes"
will let the OS know all possible pagesizes that can be used for creating a
new DDW.
Currently Linux will only try using 3 of the 8 available options:
4K, 64K and 16M. According to LoPAR, Hypervisor may also offer 32M, 64M,
128M, 256M and 16G.
Enabling bigger pages would be interesting for direct mapping systems
with a lot of RAM, while using less TCE entries.
Signed-off-by: Leonardo Bras
---
arch/powerpc/platforms/pseries/iommu.c | 49 ++
1 file changed, 42 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c
b/arch/powerpc/platforms/pseries/iommu.c
index 9fc5217f0c8e..6cda1c92597d 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -53,6 +53,20 @@ enum {
DDW_EXT_QUERY_OUT_SIZE = 2
};
+#define QUERY_DDW_PGSIZE_4K0x01
+#define QUERY_DDW_PGSIZE_64K 0x02
+#define QUERY_DDW_PGSIZE_16M 0x04
+#define QUERY_DDW_PGSIZE_32M 0x08
+#define QUERY_DDW_PGSIZE_64M 0x10
+#define QUERY_DDW_PGSIZE_128M 0x20
+#define QUERY_DDW_PGSIZE_256M 0x40
+#define QUERY_DDW_PGSIZE_16G 0x80
+
+struct iommu_ddw_pagesize {
+ u32 mask;
+ int shift;
+};
+
static struct iommu_table_group *iommu_pseries_alloc_group(int node)
{
struct iommu_table_group *table_group;
@@ -1099,6 +1113,31 @@ static void reset_dma_window(struct pci_dev *dev, struct
device_node *par_dn)
ret);
}
+/* Returns page shift based on "IO Page Sizes" output at
ibm,query-pe-dma-window. See LoPAR */
+static int iommu_get_page_shift(u32 query_page_size)
+{
+ const struct iommu_ddw_pagesize ddw_pagesize[] = {
+ { QUERY_DDW_PGSIZE_16G, __builtin_ctz(SZ_16G) },
+ { QUERY_DDW_PGSIZE_256M, __builtin_ctz(SZ_256M) },
+ { QUERY_DDW_PGSIZE_128M, __builtin_ctz(SZ_128M) },
+ { QUERY_DDW_PGSIZE_64M, __builtin_ctz(SZ_64M) },
+ { QUERY_DDW_PGSIZE_32M, __builtin_ctz(SZ_32M) },
+ { QUERY_DDW_PGSIZE_16M, __builtin_ctz(SZ_16M) },
+ { QUERY_DDW_PGSIZE_64K, __builtin_ctz(SZ_64K) },
+ { QUERY_DDW_PGSIZE_4K, __builtin_ctz(SZ_4K) }
+ };
+
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(ddw_pagesize); i++) {
+ if (query_page_size & ddw_pagesize[i].mask)
+ return ddw_pagesize[i].shift;
+ }
+
+ /* No valid page size found. */
+ return 0;
+}
+
/*
* If the PE supports dynamic dma windows, and there is space for a table
* that can map all pages in a linear offset, then setup such a table,
@@ -1206,13 +1245,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct
device_node *pdn)
goto out_failed;
}
}
- if (query.page_size & 4) {
- page_shift = 24; /* 16MB */
- } else if (query.page_size & 2) {
- page_shift = 16; /* 64kB */
- } else if (query.page_size & 1) {
- page_shift = 12; /* 4kB */
- } else {
+
+ page_shift = iommu_get_page_shift(query.page_size);
+ if (!page_shift) {
dev_dbg(>dev, "no supported direct page size in mask %x",
query.page_size);
goto out_failed;
--
2.30.2
Re: [PATCH v4] pseries: prevent free CPU ids to be reused on another node
Laurent Dufour writes: > > 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 Thanks Laurent. Reviewed-by: Nathan Lynch
Re: [PATCH 1/1] powerpc/smp: Set numa node before updating mask
Srikar Dronamraju writes: > * Nathan Lynch [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. I don't know. I guess this question just makes me wonder whether powerpc needs to have the additional lookup table. How is it different from the generic per_cpu numa_node?
[PATCH v1 7/8] powerpc/mem: Inline flush_dcache_page()
flush_dcache_page() is only a few lines, it is worth
inlining.
ia64, csky, mips, openrisc and riscv have a similar
flush_dcache_page() and inline it.
On pmac32_defconfig, we get a small size reduction.
On ppc64_defconfig, we get a very small size increase.
In both case that's in the noise (less than 0.1%).
textdatabss dec hex filename
189911555934744 1497624 2642352319330e3 vmlinux64.before
189948295936732 1497624 264291851934701 vmlinux64.after
9150963 2467502 184548 11803013 b41985 vmlinux32.before
9149689 2467302 184548 11801539 b413c3 vmlinux32.after
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/cacheflush.h | 14 +-
arch/powerpc/mm/mem.c | 15 ---
2 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/include/asm/cacheflush.h
b/arch/powerpc/include/asm/cacheflush.h
index 9110489ea411..7564dd4fd12b 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -30,7 +30,19 @@ static inline void flush_cache_vmap(unsigned long start,
unsigned long end)
#endif /* CONFIG_PPC_BOOK3S_64 */
#define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE 1
-extern void flush_dcache_page(struct page *page);
+/*
+ * This is called when a page has been modified by the kernel.
+ * It just marks the page as not i-cache clean. We do the i-cache
+ * flush later when the page is given to a user process, if necessary.
+ */
+static inline void flush_dcache_page(struct page *page)
+{
+ if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE))
+ return;
+ /* avoid an atomic op if possible */
+ if (test_bit(PG_dcache_clean, >flags))
+ clear_bit(PG_dcache_clean, >flags);
+}
void flush_icache_range(unsigned long start, unsigned long stop);
#define flush_icache_range flush_icache_range
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 460ab5000a3f..65b2205839fe 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -458,21 +458,6 @@ static void flush_dcache_icache_phys(unsigned long
physaddr)
}
#endif
-/*
- * This is called when a page has been modified by the kernel.
- * It just marks the page as not i-cache clean. We do the i-cache
- * flush later when the page is given to a user process, if necessary.
- */
-void flush_dcache_page(struct page *page)
-{
- if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE))
- return;
- /* avoid an atomic op if possible */
- if (test_bit(PG_dcache_clean, >flags))
- clear_bit(PG_dcache_clean, >flags);
-}
-EXPORT_SYMBOL(flush_dcache_page);
-
static void __flush_dcache_icache(void *p);
static void flush_dcache_icache_hugepage(struct page *page)
--
2.25.0
[PATCH v1 6/8] powerpc/mem: Help GCC realise __flush_dcache_icache() flushes single pages
'And' the given page address with PAGE_MASK to help GCC.
With the patch:
0024 <__flush_dcache_icache>:
24: 54 63 00 26 rlwinm r3,r3,0,0,19
28: 39 40 00 40 li r10,64
2c: 7c 69 1b 78 mr r9,r3
30: 7d 49 03 a6 mtctr r10
34: 7c 00 48 6c dcbst 0,r9
38: 39 29 00 20 addir9,r9,32
3c: 7c 00 48 6c dcbst 0,r9
40: 39 29 00 20 addir9,r9,32
44: 42 00 ff f0 bdnz34 <__flush_dcache_icache+0x10>
48: 7c 00 04 ac hwsync
4c: 39 20 00 40 li r9,64
50: 7d 29 03 a6 mtctr r9
54: 7c 00 1f ac icbi0,r3
58: 38 63 00 20 addir3,r3,32
5c: 7c 00 1f ac icbi0,r3
60: 38 63 00 20 addir3,r3,32
64: 42 00 ff f0 bdnz54 <__flush_dcache_icache+0x30>
68: 7c 00 04 ac hwsync
6c: 4c 00 01 2c isync
70: 4e 80 00 20 blr
Without the patch:
0024 <__flush_dcache_icache>:
24: 54 6a 00 34 rlwinm r10,r3,0,0,26
28: 39 23 10 1f addir9,r3,4127
2c: 7d 2a 48 50 subfr9,r10,r9
30: 55 29 d9 7f rlwinm. r9,r9,27,5,31
34: 41 82 00 94 beq c8 <__flush_dcache_icache+0xa4>
38: 71 28 00 01 andi. r8,r9,1
3c: 38 c9 ff ff addir6,r9,-1
40: 7d 48 53 78 mr r8,r10
44: 7d 27 4b 78 mr r7,r9
48: 40 82 00 6c bne b4 <__flush_dcache_icache+0x90>
4c: 54 e7 f8 7e rlwinm r7,r7,31,1,31
50: 7c e9 03 a6 mtctr r7
54: 7c 00 40 6c dcbst 0,r8
58: 39 08 00 20 addir8,r8,32
5c: 7c 00 40 6c dcbst 0,r8
60: 39 08 00 20 addir8,r8,32
64: 42 00 ff f0 bdnz54 <__flush_dcache_icache+0x30>
68: 7c 00 04 ac hwsync
6c: 71 28 00 01 andi. r8,r9,1
70: 39 09 ff ff addir8,r9,-1
74: 40 82 00 2c bne a0 <__flush_dcache_icache+0x7c>
78: 55 29 f8 7e rlwinm r9,r9,31,1,31
7c: 7d 29 03 a6 mtctr r9
80: 7c 00 57 ac icbi0,r10
84: 39 4a 00 20 addir10,r10,32
88: 7c 00 57 ac icbi0,r10
8c: 39 4a 00 20 addir10,r10,32
90: 42 00 ff f0 bdnz80 <__flush_dcache_icache+0x5c>
94: 7c 00 04 ac hwsync
98: 4c 00 01 2c isync
9c: 4e 80 00 20 blr
a0: 7c 00 57 ac icbi0,r10
a4: 2c 08 00 00 cmpwi r8,0
a8: 39 4a 00 20 addir10,r10,32
ac: 40 82 ff cc bne 78 <__flush_dcache_icache+0x54>
b0: 4b ff ff e4 b 94 <__flush_dcache_icache+0x70>
b4: 7c 00 50 6c dcbst 0,r10
b8: 2c 06 00 00 cmpwi r6,0
bc: 39 0a 00 20 addir8,r10,32
c0: 40 82 ff 8c bne 4c <__flush_dcache_icache+0x28>
c4: 4b ff ff a4 b 68 <__flush_dcache_icache+0x44>
c8: 7c 00 04 ac hwsync
cc: 7c 00 04 ac hwsync
d0: 4c 00 01 2c isync
d4: 4e 80 00 20 blr
Signed-off-by: Christophe Leroy
---
arch/powerpc/mm/mem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 9a5542f4de92..460ab5000a3f 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -522,7 +522,7 @@ EXPORT_SYMBOL(flush_dcache_icache_page);
*/
static void __flush_dcache_icache(void *p)
{
- unsigned long addr = (unsigned long)p;
+ unsigned long addr = (unsigned long)p & PAGE_MASK;
clean_dcache_range(addr, addr + PAGE_SIZE);
--
2.25.0
[PATCH v1 8/8] powerpc/mem: Use kmap_local_page() in flushing functions
Flushing functions don't rely on preemption being disabled, so
use kmap_local_page() instead of kmap_atomic().
Signed-off-by: Christophe Leroy
---
arch/powerpc/mm/mem.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 65b2205839fe..1895bd64191a 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -464,16 +464,16 @@ static void flush_dcache_icache_hugepage(struct page
*page)
{
int i;
int nr = compound_nr(page);
- void *start;
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);
+ void *start = kmap_local_page(page + i);
+
__flush_dcache_icache(start);
- kunmap_atomic(start);
+ kunmap_local(start);
}
}
}
@@ -489,9 +489,10 @@ void flush_dcache_icache_page(struct page *page)
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);
+ void *start = kmap_local_page(page);
+
__flush_dcache_icache(start);
- kunmap_atomic(start);
+ kunmap_local(start);
} else {
flush_dcache_icache_phys(page_to_phys(page));
}
@@ -564,11 +565,11 @@ void copy_user_page(void *vto, void *vfrom, unsigned long
vaddr,
void flush_icache_user_page(struct vm_area_struct *vma, struct page *page,
unsigned long addr, int len)
{
- unsigned long maddr;
+ void *maddr;
- maddr = (unsigned long) kmap(page) + (addr & ~PAGE_MASK);
- flush_icache_range(maddr, maddr + len);
- kunmap(page);
+ maddr = kmap_local_page(page) + (addr & ~PAGE_MASK);
+ flush_icache_range((unsigned long)maddr, (unsigned long)maddr + len);
+ kunmap_local(maddr);
}
/*
--
2.25.0
[PATCH v1 5/8] powerpc/mem: flush_dcache_icache_phys() is for HIGHMEM pages only
__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
---
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
[PATCH v1 2/8] powerpc/mem: Remove address argument to flush_coherent_icache()
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
---
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
[PATCH v1 1/8] powerpc/mem: Declare __flush_dcache_icache() static
__flush_dcache_icache() is only used in mem.c.
Declare it static.
Signed-off-by: Christophe Leroy
---
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
[PATCH v1 4/8] powerpc/mem: Optimise flush_dcache_icache_hugepage()
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
---
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
[PATCH v1 3/8] powerpc/mem: Call flush_coherent_icache() at higher level
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
---
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
Re: [PATCH] powerpc: Make some symbols static
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 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-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH 1/1] powerpc/smp: Set numa node before updating mask
* Nathan Lynch [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
[PATCH v4] pseries: prevent free CPU ids to be reused on another node
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
---
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", );
@@ -163,9 +169,17 @@ static int pseries_add_processor(struct device_node *np)
zalloc_cpumask_var(_mask, GFP_KERNEL);
zalloc_cpumask_var(, 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
Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
On Wed, Apr 07, 2021 at 05:59:19PM +0300, Andy Shevchenko wrote: > On Wed, Apr 7, 2021 at 5:30 PM Luis Chamberlain 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 > > > 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
Re: [PATCH v3] pseries: prevent free CPU ids to be reused on another node
Le 07/04/2021 à 16:55, Nathan Lynch a écrit :
Laurent Dufour 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", );
@@ -163,9 +169,17 @@ static int pseries_add_processor(struct device_node *np)
zalloc_cpumask_var(_mask, GFP_KERNEL);
zalloc_cpumask_var(, 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.
Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
On Wed, Apr 7, 2021 at 5:30 PM Luis Chamberlain 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 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
Re: [PATCH v3] pseries: prevent free CPU ids to be reused on another node
Laurent Dufour 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", );
> @@ -163,9 +169,17 @@ static int pseries_add_processor(struct device_node *np)
> zalloc_cpumask_var(_mask, GFP_KERNEL);
> zalloc_cpumask_var(, 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.
Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
On Wed, Apr 07, 2021 at 10:33:44AM +0300, Andy Shevchenko wrote: > On Wed, Apr 7, 2021 at 10:25 AM Luis Chamberlain 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 ..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 > > > +#include > > > + > > > +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
Re: [PATCH v2] KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
Nicholas Piggin writes:
>> 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, _hv);
>> restore_hv_regs(vcpu, _hv);
>>
>> +sanitise_vcpu_entry_state(vcpu, _hv, _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, _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
[PATCH] macintosh/windfarm: Make symbol 'pm121_sys_state' static
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
Signed-off-by: Yu Kuai
---
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
[PATCH] powerpc: Make some symbols static
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
Signed-off-by: Yu Kuai
---
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
[PATCH] powerpc/smp: Make some symbols static
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 Signed-off-by: Yu Kuai --- 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
[PATCH] tty: hvc: make symbol 'hvc_udbg_dev' static
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
Signed-off-by: Yu Kuai
---
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
[PATCH] macintosh/via-pmu: Make some symbols static
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 Signed-off-by: Yu Kuai --- 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
[PATCH] windfarm: make symbol 'wf_thread' static
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
Signed-off-by: Yu Kuai
---
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
Re: [PATCH 1/1] powerpc/smp: Set numa node before updating mask
Sorry for the delay in following up here.
Srikar Dronamraju 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.
Re: [PATCH] powerpc/powernv: Enable HAIL (HV AIL) for ISA v3.1 processors
Excerpts from Michael Ellerman's message of April 7, 2021 9:33 pm:
> Nicholas Piggin 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(0x8000)
>> #define LPCR_RMLS 0x1C00 /* Implementation dependent RMO
>> limit sel */
>> #define LPCR_RMLS_SH 26
>> +#define LPCR_HAIL ASM_CONST(0x0400) /* HV AIL
>> (ISAv3.1) */
>> #define LPCR_ILE ASM_CONST(0x0200) /* !HV irqs set
>> MSR:LE */
>> #define LPCR_AIL ASM_CONST(0x0180) /* Alternate
>> interrupt location */
>> #define LPCR_AIL_0ASM_CONST(0x) /* 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
>
Re: [PATCH 16/20] kbuild: powerpc: use common install script
On Wed, Apr 7, 2021 at 2:34 PM Greg Kroah-Hartman
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
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Greg Kroah-Hartman
> ---
> 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..
> --- 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
> +
Re: [PATCH v3 1/2] powerpc/perf: Infrastructure to support checking of attr.config*
Madhavan Srinivasan 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
Re: [PATCH] powerpc/powernv: Enable HAIL (HV AIL) for ISA v3.1 processors
Nicholas Piggin 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(0x8000)
> #define LPCR_RMLS 0x1C00 /* Implementation dependent RMO
> limit sel */
> #define LPCR_RMLS_SH 26
> +#define LPCR_HAIL ASM_CONST(0x0400) /* HV AIL
> (ISAv3.1) */
> #define LPCR_ILE ASM_CONST(0x0200) /* !HV irqs set
> MSR:LE */
> #define LPCR_AIL ASM_CONST(0x0180) /* Alternate
> interrupt location */
> #define LPCR_AIL_0 ASM_CONST(0x) /* 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
Re: [PATCH] powerpc/dts: fix not include DTC_FLAGS
Youlin Song writes:
> I wanted to build the fsl dts in my machine and found that
> the dtb have not extra space,so uboot will cause about
> FDT_ERR_NOSPACE issue.
>
> Signed-off-by: Youlin Song
> ---
> arch/powerpc/boot/dts/Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/boot/dts/Makefile b/arch/powerpc/boot/dts/Makefile
> index fb335d05aae8..c21165c0cd76 100644
> --- a/arch/powerpc/boot/dts/Makefile
> +++ b/arch/powerpc/boot/dts/Makefile
> @@ -2,5 +2,6 @@
>
> subdir-y += fsl
>
> +DTC_FLAGS ?= -p 1024
> dtstree := $(srctree)/$(src)
> dtb-$(CONFIG_OF_ALL_DTBS) := $(patsubst $(dtstree)/%.dts,%.dtb, $(wildcard
> $(dtstree)/*.dts))
I guess that was missed in 1acf1cf8638a ("powerpc: build .dtb files in dts
directory").
Which I think means the assignment to DTC_FLAGS in
arch/powerpc/boot/Makefile is not needed anymore.
Can you send a v2 removing that assignment and explaining that's what
happened?
cheers
[PATCH] ASoC: fsl: sunxi: remove redundant dev_err call
devm_ioremap_resource() prints error message in itself. Remove the
dev_err call to avoid redundant error message.
Signed-off-by: Muhammad Usama Anjum
---
sound/soc/fsl/fsl_aud2htx.c | 4 +---
sound/soc/fsl/fsl_easrc.c | 4 +---
sound/soc/sunxi/sun4i-codec.c | 4 +---
3 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/sound/soc/fsl/fsl_aud2htx.c b/sound/soc/fsl/fsl_aud2htx.c
index d70d5e75a30c..a328697511f7 100644
--- a/sound/soc/fsl/fsl_aud2htx.c
+++ b/sound/soc/fsl/fsl_aud2htx.c
@@ -198,10 +198,8 @@ static int fsl_aud2htx_probe(struct platform_device *pdev)
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
regs = devm_ioremap_resource(>dev, res);
- if (IS_ERR(regs)) {
- dev_err(>dev, "failed ioremap\n");
+ if (IS_ERR(regs))
return PTR_ERR(regs);
- }
aud2htx->regmap = devm_regmap_init_mmio(>dev, regs,
_aud2htx_regmap_config);
diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
index 5e33afe87c4a..b1765c7d3bcd 100644
--- a/sound/soc/fsl/fsl_easrc.c
+++ b/sound/soc/fsl/fsl_easrc.c
@@ -1889,10 +1889,8 @@ static int fsl_easrc_probe(struct platform_device *pdev)
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
regs = devm_ioremap_resource(dev, res);
- if (IS_ERR(regs)) {
- dev_err(>dev, "failed ioremap\n");
+ if (IS_ERR(regs))
return PTR_ERR(regs);
- }
easrc->paddr = res->start;
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 2173991c13db..6f3d9148a185 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -1711,10 +1711,8 @@ static int sun4i_codec_probe(struct platform_device
*pdev)
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
base = devm_ioremap_resource(>dev, res);
- if (IS_ERR(base)) {
- dev_err(>dev, "Failed to map the registers\n");
+ if (IS_ERR(base))
return PTR_ERR(base);
- }
quirks = of_device_get_match_data(>dev);
if (quirks == NULL) {
--
2.25.1
Re: [PATCH v2] KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
Excerpts from Fabiano Rosas's message of April 7, 2021 7:46 am:
> As one of the arguments of the H_ENTER_NESTED hypercall, the nested
> hypervisor (L1) prepares a structure containing the values of various
> hypervisor-privileged registers with which it wants the nested guest
> (L2) to run. Since the nested HV runs in supervisor mode it needs the
> host to write to these registers.
>
> To stop a nested HV manipulating this mechanism and using a nested
> guest as a proxy to access a facility that has been made unavailable
> to it, we have a routine that sanitises the values of the HV registers
> before copying them into the nested guest's vcpu struct.
>
> However, when coming out of the guest the values are copied as they
> were back into L1 memory, which means that any sanitisation we did
> during guest entry will be exposed to L1 after H_ENTER_NESTED returns.
>
> This patch alters this sanitisation to have effect on the vcpu->arch
> registers directly before entering and after exiting the guest,
> leaving the structure that is copied back into L1 unchanged (except
> when we really want L1 to access the value, e.g the Cause bits of
> HFSCR).
>
> Signed-off-by: Fabiano Rosas
I like this direction better. Now "sanitise" may be the wrong word
because you aren't cleaning the data in place any more but copying a
cleaned version across.
Which is fine but I wouldn't call it sanitise. Actually I would
prefer if it is just done as part of the copy rather than
copy first and then apply this (explained below in the code).
> ---
> I'm taking another shot at fixing this locally without resorting to
> more complex things such as error handling and feature
> advertisement/negotiation.
That's okay, I think those are orthogonal problems. This won't help if
a nested HV tries to use some HFSCR feature it believes should be
available to a (say) POWER10 processor but got secretly masked away.
But also such negotiation doesn't help with trying to minimise L0 HV
data accessible to guests.
(As before a guest can easily find out many of these things if it is
determined to, but that does not mean I'm strongly against what you
are doing here)
>
> Changes since v1:
>
> - made the change more generic, not only applies to hfscr anymore;
> - sanitisation is now done directly on the vcpu struct, l2_hv is left
> unchanged;
>
> v1:
>
> https://lkml.kernel.org/r/20210305231055.2913892-1-faro...@linux.ibm.com
> ---
> arch/powerpc/kvm/book3s_hv_nested.c | 33 +++--
> 1 file changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c
> b/arch/powerpc/kvm/book3s_hv_nested.c
> index 0cd0e7aad588..a60fccb2c4f2 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -132,21 +132,37 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu,
> int trap,
> }
> }
>
> -static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state
> *hr)
> +static void sanitise_vcpu_entry_state(struct kvm_vcpu *vcpu,
> + const struct hv_guest_state *l2_hv,
> + const struct hv_guest_state *l1_hv)
> {
> /*
>* Don't let L1 enable features for L2 which we've disabled for L1,
>* but preserve the interrupt cause field.
>*/
> - hr->hfscr &= (HFSCR_INTR_CAUSE | vcpu->arch.hfscr);
> + vcpu->arch.hfscr = l2_hv->hfscr & (HFSCR_INTR_CAUSE | l1_hv->hfscr);
>
> /* Don't let data address watchpoint match in hypervisor state */
> - hr->dawrx0 &= ~DAWRX_HYP;
> - hr->dawrx1 &= ~DAWRX_HYP;
> + vcpu->arch.dawrx0 = l2_hv->dawrx0 & ~DAWRX_HYP;
> + vcpu->arch.dawrx1 = l2_hv->dawrx1 & ~DAWRX_HYP;
>
> /* Don't let completed instruction address breakpt match in HV state */
> - if ((hr->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
> - hr->ciabr &= ~CIABR_PRIV;
> + if ((l2_hv->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
> + vcpu->arch.ciabr = l2_hv->ciabr & ~CIABR_PRIV;
> +}
> +
> +
> +/*
> + * During sanitise_vcpu_entry_state() we might have used bits from L1
> + * state to restrict what the L2 state is allowed to be. Since L1 is
> + * not allowed to read the HV registers, do not include these
> + * modifications in the return state.
> + */
> +static void sanitise_vcpu_return_state(struct kvm_vcpu *vcpu,
> +const struct hv_guest_state *l2_hv)
> +{
> + vcpu->arch.hfscr = ((~HFSCR_INTR_CAUSE & l2_hv->hfscr) |
> + (HFSCR_INTR_CAUSE & vcpu->arch.hfscr));
> }
>
> 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, _hv);
>
Re: [OpenRISC] [PATCH v6 1/9] locking/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
On Wed, Apr 07, 2021 at 08:52:08AM +0900, Stafford Horne wrote: > Why doesn't RISC-V add the xchg16 emulation code similar to OpenRISC? For > OpenRISC we added xchg16 and xchg8 emulation code to enable qspinlocks. So > one thought is with CONFIG_ARCH_USE_QUEUED_SPINLOCKS_XCHG32=y, can we remove > our > xchg16/xchg8 emulation code? CONFIG_ARCH_USE_QUEUED_SPINLOCKS_XCHG32 is guaranteed crap. All the architectures that have wanted it are RISC style LL/SC archs, and for them a cmpxchg loop is a daft thing to do, since it reduces the chance of it behaving sanely. Why would we provide something that's known to be suboptimal? If an architecture chooses to not care about determinism and or fwd progress, then that's their choice. But not one, I feel, we should encourage.
Re: [PATCH v6 38/48] KVM: PPC: Book3S HV: Remove support for dependent threads mode on P9
Excerpts from Nicholas Piggin's message of April 7, 2021 5:44 pm: > Excerpts from Paul Mackerras's message of April 7, 2021 4:51 pm: >> On Mon, Apr 05, 2021 at 11:19:38AM +1000, Nicholas Piggin wrote: >>> Radix guest support will be removed from the P7/8 path, so disallow >>> dependent threads mode on P9. >> >> Dependent threads mode on P9 was added in order to support the mode >> where for security reasons you want to restrict the vcpus that run on >> a core to all be from the same VM, without requiring all guests to run >> single-threaded. This was (at least at one stage) thought to be a >> useful mode for users that are worried about side-channel data leaks. > > Right. > >> >> Now it's possible that that mode is not practically useful for some >> reason, or that no-one actually wants it, but its removal should be >> discussed. Also, the fact that we are losing that mode should be >> explicit in the commit message. > > Let's discuss. Did / does anyone really use it or ask for it that you > know of? What do other archs do? Compared with using standard options > that would achive this kind of security (disable SMT, I guess?) how > much is this worth keeping? > > It was pretty simple to support when the P8 dependent theads code had > to support P9 anyway. After this series, now all that code is only for > that one feature, so it would be pretty nice to be able to remove it. > How do we reach a point where you'd be okay to remove this and tell > people to just turn off SMT? Assuming we do decide to remove it, how's about something like this for a changelog. Does a bit more justice to the feature and its removal. Thanks, Nick Dependent-threads mode is the normal KVM mode for pre-POWER9 SMT processors, where all threads in a core (or subcore) would run the same partition at the same time, or they would run the host. This design was mandated by MMU state that is shared between threads in a processor, so the synchronisation point is in hypervisor real-mode that has essentially no shared state, so it's safe for multiple threads to gather and switch to the correct mode. It is implemented by having the host unplug all secondary threads and always run in SMT1 mode, and host QEMU threads essentially represent virtual cores that wake these secondary threads out of unplug when the ioctl is called to run the guest. This happens via a side-path that is mostly invisible to the rest of the Linux host and the secondary threads still appear to be unplugged. POWER9 / ISA v3.0 has a more flexible MMU design that is independent per-thread and allows a much simpler KVM implementation. Before the new "P9 fast path" was added that began to take advantage of this, POWER9 support was implemented in the existing path which has support to run in the dependent threads mode. So it was not much work to add support to run POWER9 in this dependent threads mode. The mode is not required by the POWER9 MMU (although "mixed-mode" hash / radix MMU limitations of early processors were worked around using this mode). But it is one way to run SMT guests without running different guests or guest and host on different threads of the same core, so it could avoid or reduce some SMT attack surfaces without turning off SMT entirely. This security feature has some real, if indeterminate, value. However the old path is lagging in features (nested HV), and with this series the new P9 path adds remaining missing features (radix prefetch bug and hash support, in later patches), so POWER9 dependent threads mode support would be the only remaining reason to keep that code in and keep supporting POWER9/POWER10 in the old path. So here we make the call to drop this feature. Remove dependent threads mode support for POWER9 and above processors. Systems can still achieve this security by disabling SMT entirely, but that would generally come at a larger performance cost for guests.
Re: cleanup unused or almost unused IOMMU APIs and the FSL PAMU driver v3
On Thu, Apr 01, 2021 at 05:52:36PM +0200, Christoph Hellwig wrote: > Diffstat: > arch/powerpc/include/asm/fsl_pamu_stash.h | 12 > drivers/gpu/drm/msm/adreno/adreno_gpu.c |5 > drivers/iommu/amd/iommu.c | 23 > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 75 --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |1 > drivers/iommu/arm/arm-smmu/arm-smmu.c | 111 +--- > drivers/iommu/arm/arm-smmu/arm-smmu.h |2 > drivers/iommu/dma-iommu.c |9 > drivers/iommu/fsl_pamu.c| 293 --- > drivers/iommu/fsl_pamu.h| 12 > drivers/iommu/fsl_pamu_domain.c | 688 > ++-- > drivers/iommu/fsl_pamu_domain.h | 46 - > drivers/iommu/intel/iommu.c | 95 --- > drivers/iommu/iommu.c | 118 +--- > drivers/soc/fsl/qbman/qman_portal.c | 55 -- > drivers/vfio/vfio_iommu_type1.c | 31 - > drivers/vhost/vdpa.c| 10 > include/linux/io-pgtable.h |4 > include/linux/iommu.h | 76 --- > 19 files changed, 203 insertions(+), 1463 deletions(-) Applied, thanks.
Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
On Wed, Apr 7, 2021 at 11:17 AM Kees Cook wrote: > > On Tue, Apr 06, 2021 at 04:31:58PM +0300, Andy Shevchenko wrote: > > kernel.h is being used as a dump for all kinds of stuff for a long time. > > Here is the attempt to start cleaning it up by splitting out panic and > > oops helpers. > > > > At the same time convert users in header and lib folder to use new header. > > Though for time being include new header back to kernel.h to avoid twisted > > indirected includes for existing users. > > > > Signed-off-by: Andy Shevchenko > > I like it! Do you have a multi-arch CI to do allmodconfig builds to > double-check this? Unfortunately no, I rely on plenty of bots that are harvesting mailing lists. But I will appreciate it if somebody can run this through various build tests. > Acked-by: Kees Cook Thanks! -- With Best Regards, Andy Shevchenko
Re: [PATCH v6 38/48] KVM: PPC: Book3S HV: Remove support for dependent threads mode on P9
Excerpts from Paul Mackerras's message of April 7, 2021 4:51 pm: > On Mon, Apr 05, 2021 at 11:19:38AM +1000, Nicholas Piggin wrote: >> Radix guest support will be removed from the P7/8 path, so disallow >> dependent threads mode on P9. > > Dependent threads mode on P9 was added in order to support the mode > where for security reasons you want to restrict the vcpus that run on > a core to all be from the same VM, without requiring all guests to run > single-threaded. This was (at least at one stage) thought to be a > useful mode for users that are worried about side-channel data leaks. Right. > > Now it's possible that that mode is not practically useful for some > reason, or that no-one actually wants it, but its removal should be > discussed. Also, the fact that we are losing that mode should be > explicit in the commit message. Let's discuss. Did / does anyone really use it or ask for it that you know of? What do other archs do? Compared with using standard options that would achive this kind of security (disable SMT, I guess?) how much is this worth keeping? It was pretty simple to support when the P8 dependent theads code had to support P9 anyway. After this series, now all that code is only for that one feature, so it would be pretty nice to be able to remove it. How do we reach a point where you'd be okay to remove this and tell people to just turn off SMT? Thanks, Nick
Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
On Wed, Apr 7, 2021 at 10:25 AM Luis Chamberlain 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 ..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 > > +#include > > + > > +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. > Seems like a very > small file. If it is an argument, it's kinda strange. We have much smaller headers. -- With Best Regards, Andy Shevchenko
Re: [PATCH v6 38/48] KVM: PPC: Book3S HV: Remove support for dependent threads mode on P9
On Mon, Apr 05, 2021 at 11:19:38AM +1000, Nicholas Piggin wrote: > Radix guest support will be removed from the P7/8 path, so disallow > dependent threads mode on P9. Dependent threads mode on P9 was added in order to support the mode where for security reasons you want to restrict the vcpus that run on a core to all be from the same VM, without requiring all guests to run single-threaded. This was (at least at one stage) thought to be a useful mode for users that are worried about side-channel data leaks. Now it's possible that that mode is not practically useful for some reason, or that no-one actually wants it, but its removal should be discussed. Also, the fact that we are losing that mode should be explicit in the commit message. Paul.
