* Re: [PATCH V2] powerpc/perf: Fix handling of privilege level checks in perf interrupt context
From: Michael Ellerman @ 2021-03-14 10:01 UTC (permalink / raw)
To: Athira Rajeev, mpe
Cc: maddy, peterz, omosnace, acme, jolsa, linuxppc-dev, kan.liang
In-Reply-To: <1614247839-1428-1-git-send-email-atrajeev@linux.vnet.ibm.com>
On Thu, 25 Feb 2021 05:10:39 -0500, Athira Rajeev wrote:
> Running "perf mem record" in powerpc platforms with selinux enabled
> resulted in soft lockup's. Below call-trace was seen in the logs:
>
> CPU: 58 PID: 3751 Comm: sssd_nss Not tainted 5.11.0-rc7+ #2
> NIP: c000000000dff3d4 LR: c000000000dff3d0 CTR: 0000000000000000
> REGS: c000007fffab7d60 TRAP: 0100 Not tainted (5.11.0-rc7+)
> <<>>
> NIP [c000000000dff3d4] _raw_spin_lock_irqsave+0x94/0x120
> LR [c000000000dff3d0] _raw_spin_lock_irqsave+0x90/0x120
> Call Trace:
> [c00000000fd471a0] [c00000000fd47260] 0xc00000000fd47260 (unreliable)
> [c00000000fd471e0] [c000000000b5fbbc] skb_queue_tail+0x3c/0x90
> [c00000000fd47220] [c000000000296edc] audit_log_end+0x6c/0x180
> [c00000000fd47260] [c0000000006a3f20] common_lsm_audit+0xb0/0xe0
> [c00000000fd472a0] [c00000000066c664] slow_avc_audit+0xa4/0x110
> [c00000000fd47320] [c00000000066cff4] avc_has_perm+0x1c4/0x260
> [c00000000fd47430] [c00000000066e064] selinux_perf_event_open+0x74/0xd0
> [c00000000fd47450] [c000000000669888] security_perf_event_open+0x68/0xc0
> [c00000000fd47490] [c00000000013d788] record_and_restart+0x6e8/0x7f0
> [c00000000fd476c0] [c00000000013dabc] perf_event_interrupt+0x22c/0x560
> [c00000000fd477d0] [c00000000002d0fc] performance_monitor_exception0x4c/0x60
> [c00000000fd477f0] [c00000000000b378] performance_monitor_common_virt+0x1c8/0x1d0
> interrupt: f00 at _raw_spin_lock_irqsave+0x38/0x120
> NIP: c000000000dff378 LR: c000000000b5fbbc CTR: c0000000007d47f0
> REGS: c00000000fd47860 TRAP: 0f00 Not tainted (5.11.0-rc7+)
> <<>>
> NIP [c000000000dff378] _raw_spin_lock_irqsave+0x38/0x120
> LR [c000000000b5fbbc] skb_queue_tail+0x3c/0x90
> interrupt: f00
> [c00000000fd47b00] [0000000000000038] 0x38 (unreliable)
> [c00000000fd47b40] [c00000000aae6200] 0xc00000000aae6200
> [c00000000fd47b80] [c000000000296edc] audit_log_end+0x6c/0x180
> [c00000000fd47bc0] [c00000000029f494] audit_log_exit+0x344/0xf80
> [c00000000fd47d10] [c0000000002a2b00] __audit_syscall_exit+0x2c0/0x320
> [c00000000fd47d60] [c000000000032878] do_syscall_trace_leave+0x148/0x200
> [c00000000fd47da0] [c00000000003d5b4] syscall_exit_prepare+0x324/0x390
> [c00000000fd47e10] [c00000000000d76c] system_call_common+0xfc/0x27c
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/perf: Fix handling of privilege level checks in perf interrupt context
https://git.kernel.org/powerpc/c/5ae5fbd2107959b68ac69a8b75412208663aea88
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: Force inlining of mmu_has_feature to fix build failure
From: Michael Ellerman @ 2021-03-14 10:01 UTC (permalink / raw)
To: Michael Ellerman, Paul Mackerras, Christophe Leroy,
Benjamin Herrenschmidt
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cf61345912c078c96f171afd0fcc48ef27cbdc3f.1614443418.git.christophe.leroy@csgroup.eu>
On Sat, 27 Feb 2021 16:30:48 +0000 (UTC), Christophe Leroy wrote:
> The test robot has managed to generate a random config leading
> to following build failure:
>
> LD .tmp_vmlinux.kallsyms1
> powerpc64-linux-ld: arch/powerpc/mm/pgtable.o: in function `ptep_set_access_flags':
> pgtable.c:(.text.ptep_set_access_flags+0xf0): undefined reference to `hash__flush_tlb_page'
> powerpc64-linux-ld: arch/powerpc/mm/book3s32/mmu.o: in function `MMU_init_hw_patch':
> mmu.c:(.init.text+0x452): undefined reference to `patch__hash_page_A0'
> powerpc64-linux-ld: mmu.c:(.init.text+0x45e): undefined reference to `patch__hash_page_A0'
> powerpc64-linux-ld: mmu.c:(.init.text+0x46a): undefined reference to `patch__hash_page_A1'
> powerpc64-linux-ld: mmu.c:(.init.text+0x476): undefined reference to `patch__hash_page_A1'
> powerpc64-linux-ld: mmu.c:(.init.text+0x482): undefined reference to `patch__hash_page_A2'
> powerpc64-linux-ld: mmu.c:(.init.text+0x48e): undefined reference to `patch__hash_page_A2'
> powerpc64-linux-ld: mmu.c:(.init.text+0x49e): undefined reference to `patch__hash_page_B'
> powerpc64-linux-ld: mmu.c:(.init.text+0x4aa): undefined reference to `patch__hash_page_B'
> powerpc64-linux-ld: mmu.c:(.init.text+0x4b6): undefined reference to `patch__hash_page_C'
> powerpc64-linux-ld: mmu.c:(.init.text+0x4c2): undefined reference to `patch__hash_page_C'
> powerpc64-linux-ld: mmu.c:(.init.text+0x4ce): undefined reference to `patch__flush_hash_A0'
> powerpc64-linux-ld: mmu.c:(.init.text+0x4da): undefined reference to `patch__flush_hash_A0'
> powerpc64-linux-ld: mmu.c:(.init.text+0x4e6): undefined reference to `patch__flush_hash_A1'
> powerpc64-linux-ld: mmu.c:(.init.text+0x4f2): undefined reference to `patch__flush_hash_A1'
> powerpc64-linux-ld: mmu.c:(.init.text+0x4fe): undefined reference to `patch__flush_hash_A2'
> powerpc64-linux-ld: mmu.c:(.init.text+0x50a): undefined reference to `patch__flush_hash_A2'
> powerpc64-linux-ld: mmu.c:(.init.text+0x522): undefined reference to `patch__flush_hash_B'
> powerpc64-linux-ld: mmu.c:(.init.text+0x532): undefined reference to `patch__flush_hash_B'
> powerpc64-linux-ld: arch/powerpc/mm/book3s32/mmu.o: in function `update_mmu_cache':
> mmu.c:(.text.update_mmu_cache+0xa0): undefined reference to `add_hash_page'
> powerpc64-linux-ld: mm/memory.o: in function `zap_pte_range':
> memory.c:(.text.zap_pte_range+0x160): undefined reference to `flush_hash_pages'
> powerpc64-linux-ld: mm/memory.o: in function `handle_pte_fault':
> memory.c:(.text.handle_pte_fault+0x180): undefined reference to `hash__flush_tlb_page'
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc: Force inlining of mmu_has_feature to fix build failure
https://git.kernel.org/powerpc/c/acdad8fb4a1574323db88f98a38b630691574e16
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()
From: Michael Ellerman @ 2021-03-14 10:01 UTC (permalink / raw)
To: Michael Ellerman, geert, Paul Mackerras, Benjamin Herrenschmidt,
Christophe Leroy
Cc: alexdeucher, linuxppc-dev, christian.koenig, amd-gfx,
linux-kernel
In-Reply-To: <8d7d285a027e9d21f5ff7f850fa71a2655b0c4af.1615279170.git.christophe.leroy@csgroup.eu>
On Tue, 9 Mar 2021 08:39:39 +0000 (UTC), Christophe Leroy wrote:
> Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
> when CONFIG_VSX is not set, to avoid following build failure.
>
> CC [M] drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
> In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
> from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
> from drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 'dcn_bw_apply_registry_override':
> ./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? [-Werror=implicit-function-declaration]
> 64 | enable_kernel_vsx(); \
> | ^~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in expansion of macro 'DC_FP_START'
> 640 | DC_FP_START();
> | ^~~~~~~~~~~
> ./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit declaration of function 'disable_kernel_vsx'; did you mean 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
> 75 | disable_kernel_vsx(); \
> | ^~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in expansion of macro 'DC_FP_END'
> 676 | DC_FP_END();
> | ^~~~~~~~~
> cc1: some warnings being treated as errors
> make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] Error 1
Applied to powerpc/fixes.
[1/1] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()
https://git.kernel.org/powerpc/c/bd73758803c2eedc037c2268b65a19542a832594
cheers
^ permalink raw reply
* [PATCH] powerpc/fsl-pci: Fix section mismatch warning
From: Michael Ellerman @ 2021-03-14 9:33 UTC (permalink / raw)
To: linuxppc-dev
Section mismatch in reference from the function .fsl_add_bridge() to
the function .init.text:.setup_pci_cmd()
fsl_add_bridge() is not __init, and can't be, and is the only caller
of setup_pci_cmd(). Fix it by making setup_pci_cmd() non-init.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/sysdev/fsl_pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 040b9d01c079..69af73765783 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -455,7 +455,7 @@ static void setup_pci_atmu(struct pci_controller *hose)
}
}
-static void __init setup_pci_cmd(struct pci_controller *hose)
+static void setup_pci_cmd(struct pci_controller *hose)
{
u16 cmd;
int cap_x;
--
2.25.1
^ permalink raw reply related
* [PATCH] powerpc: Fix section mismatch warning in smp_setup_pacas()
From: Michael Ellerman @ 2021-03-14 9:33 UTC (permalink / raw)
To: linuxppc-dev
Section mismatch in reference from the function .smp_setup_pacas() to
the function .init.text:.allocate_paca()
The only caller of smp_setup_pacas() is setup_arch() which is __init,
so mark smp_setup_pacas() __init.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/setup-common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index bee984b1887b..55caaa211b9f 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -829,7 +829,7 @@ static __init void print_system_info(void)
}
#ifdef CONFIG_SMP
-static void smp_setup_pacas(void)
+static void __init smp_setup_pacas(void)
{
int cpu;
--
2.25.1
^ permalink raw reply related
* [PATCH] powerpc/64s: Fold update_current_thread_[i]amr() into their only callers
From: Michael Ellerman @ 2021-03-14 9:33 UTC (permalink / raw)
To: linuxppc-dev
lkp reported warnings in some configuration due to
update_current_thread_amr() being unused:
arch/powerpc/mm/book3s64/pkeys.c:284:20: error: unused function 'update_current_thread_amr'
static inline void update_current_thread_amr(u64 value)
Which is because it's only use is inside an ifdef. We could move it
inside the ifdef, but it's a single line function and only has one
caller, so just fold it in.
Similarly update_current_thread_iamr() is small and only called once,
so fold it in also.
Fixes: 48a8ab4eeb82 ("powerpc/book3s64/pkeys: Don't update SPRN_AMR when in kernel mode.")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/mm/book3s64/pkeys.c | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index 15dcc5ad91c5..a2d9ad138709 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -301,19 +301,6 @@ void setup_kuap(bool disabled)
}
#endif
-static inline void update_current_thread_amr(u64 value)
-{
- current->thread.regs->amr = value;
-}
-
-static inline void update_current_thread_iamr(u64 value)
-{
- if (!likely(pkey_execute_disable_supported))
- return;
-
- current->thread.regs->iamr = value;
-}
-
#ifdef CONFIG_PPC_MEM_KEYS
void pkey_mm_init(struct mm_struct *mm)
{
@@ -328,7 +315,7 @@ static inline void init_amr(int pkey, u8 init_bits)
u64 new_amr_bits = (((u64)init_bits & 0x3UL) << pkeyshift(pkey));
u64 old_amr = current_thread_amr() & ~((u64)(0x3ul) << pkeyshift(pkey));
- update_current_thread_amr(old_amr | new_amr_bits);
+ current->thread.regs->amr = old_amr | new_amr_bits;
}
static inline void init_iamr(int pkey, u8 init_bits)
@@ -336,7 +323,10 @@ static inline void init_iamr(int pkey, u8 init_bits)
u64 new_iamr_bits = (((u64)init_bits & 0x1UL) << pkeyshift(pkey));
u64 old_iamr = current_thread_iamr() & ~((u64)(0x1ul) << pkeyshift(pkey));
- update_current_thread_iamr(old_iamr | new_iamr_bits);
+ if (!likely(pkey_execute_disable_supported))
+ return;
+
+ current->thread.regs->iamr = old_iamr | new_iamr_bits;
}
/*
--
2.25.1
^ permalink raw reply related
* [PATCH] powerpc/eeh: Fix build failure with CONFIG_PROC_FS=n
From: Michael Ellerman @ 2021-03-14 9:33 UTC (permalink / raw)
To: linuxppc-dev
The build fails with CONFIG_PROC_FS=n:
arch/powerpc/kernel/eeh.c:1571:12: error: ‘proc_eeh_show’ defined but not used
1571 | static int proc_eeh_show(struct seq_file *m, void *v)
Wrap proc_eeh_show() in an ifdef to avoid it.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/eeh.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index cd60bc1c8701..01dbb44a0fe3 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1568,6 +1568,7 @@ int eeh_pe_inject_err(struct eeh_pe *pe, int type, int func,
}
EXPORT_SYMBOL_GPL(eeh_pe_inject_err);
+#ifdef CONFIG_PROC_FS
static int proc_eeh_show(struct seq_file *m, void *v)
{
if (!eeh_enabled()) {
@@ -1594,6 +1595,7 @@ static int proc_eeh_show(struct seq_file *m, void *v)
return 0;
}
+#endif /* CONFIG_PROC_FS */
#ifdef CONFIG_DEBUG_FS
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] sound: soc: fsl: Remove unnecessary THIS_MODULE
From: Fabio Estevam @ 2021-03-13 20:25 UTC (permalink / raw)
To: Wang Qing
Cc: Linux-ALSA, linuxppc-dev, linux-kernel, Timur Tabi, Xiubo Li,
Shawn Guo, Sascha Hauer, Takashi Iwai, Liam Girdwood,
Jaroslav Kysela, Nicolin Chen, Mark Brown, NXP Linux Team,
Pengutronix Kernel Team, Shengjiu Wang,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <1615623820-16592-1-git-send-email-wangqing@vivo.com>
Hi Wang,
On Sat, Mar 13, 2021 at 5:25 AM Wang Qing <wangqing@vivo.com> wrote:
>
> As THIS_MODULE has been set in module_platform_driver(), so remove it.
>
> Signed-off-by: Wang Qing <wangqing@vivo.com>
Thanks for the patch, but someone else has already sent the same.
^ permalink raw reply
* Re: [PATCH v2] ASoC: imx-hdmi: fix platform_no_drv_owner.cocci warnings
From: Fabio Estevam @ 2021-03-13 20:25 UTC (permalink / raw)
To: Yang Li
Cc: Linux-ALSA, linuxppc-dev, linux-kernel, Timur Tabi, Xiubo Li,
Shawn Guo, Sascha Hauer, Takashi Iwai, Liam Girdwood,
Jaroslav Kysela, Nicolin Chen, Mark Brown, NXP Linux Team,
Sascha Hauer, Shengjiu Wang,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <1614848881-29637-1-git-send-email-yang.lee@linux.alibaba.com>
Hi Yang,
On Thu, Mar 4, 2021 at 6:08 AM Yang Li <yang.lee@linux.alibaba.com> wrote:
>
> ./sound/soc/fsl/imx-hdmi.c:226:3-8: No need to set .owner here. The core
> will do it.
>
> Remove .owner field if calls are used which set it automatically
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
Reviewed-by: Fabio Estevam <festevam@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 4/7] CMDLINE: powerpc: convert to generic builtin command line
From: Christophe Leroy @ 2021-03-13 9:29 UTC (permalink / raw)
To: Daniel Walker
Cc: Rob Herring, Ruslan Ruslichenko, Ruslan Bilovol,
Daniel Gimpelevich, linuxppc-dev, x86, linux-mips, linux-kernel,
Paul Mackerras, xe-linux-external, Andrew Morton, Will Deacon
In-Reply-To: <20210309214051.GS109100@zorba>
Le 09/03/2021 à 22:40, Daniel Walker a écrit :
> On Tue, Mar 09, 2021 at 08:56:47AM +0100, Christophe Leroy wrote:
>>
>> So we are referencing a function that doesn't exist (namely prom_strlcat).
>> But it works because cmdline_add_builtin_custom() looks like a function but
>> is in fact an obscure macro that doesn't use prom_strlcat() unless
>> GENERIC_CMDLINE_NEED_STRLCAT is defined.
>>
>> IMHO that's awful for readability and code maintenance.
>
> powerpc is a special case, there's no other users like this. The reason is
> because of all the difficulty in this prom_init.c code. A lot of the generic
> code has similar kind of changes to work across architectures.
>
I'd suggest the following (sorry if Thunderbird damages whitespaces, you'll get the idea anyway)
diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
new file mode 100644
index 000000000000..30b9eefc802f
--- /dev/null
+++ b/include/linux/cmdline.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_CMDLINE_H
+#define _LINUX_CMDLINE_H
+
+#ifdef CONFIG_GENERIC_CMDLINE
+
+#ifndef cmdline_strlcpy
+#define cmdline_strlcpy strlcpy
+#endif
+#ifndef cmdline_strlcat
+#define cmdline_strlcat strlcat
+#endif
+
+static __always_inline void
+cmdline_add_builtin_custom(char *dest, const char *src, char *tmp, unsigned long length)
+{
+ if (WARN_ON(sizeof(CONFIG_CMDLINE_PREPEND) > 1 &&
+ !IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) &&
+ !tmp && src == dest))
+ return;
+
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1 &&
+ !IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) && src == dest)
+ cmdline_strlcpy(tmp, src, length);
+ else
+ tmp = (char *)src;
+
+ cmdline_strlcpy(dest, CONFIG_CMDLINE_PREPEND " ", length);
+ if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) && tmp)
+ cmdline_strlcat(dest, tmp, length);
+ cmdline_strlcat(dest, " " CONFIG_CMDLINE_APPEND, length);
+}
+
+#define cmdline_add_builtin(dest, src, length) do { \
+ static __init char cmdline_tmp[length]; \
+ \
+ cmdline_add_builtin_custom(dest, src, cmdline_tmp, length); \
+} while (0)
+
+#endif /* CONFIG_GENERIC_CMDLINE */
+
+#endif /* _LINUX_CMDLINE_H */
diff --git a/init/Kconfig b/init/Kconfig
index 22946fe5ded9..aeb134f0703b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2035,6 +2035,27 @@ config PROFILING
config TRACEPOINTS
bool
+config GENERIC_CMDLINE
+ bool
+
+if GENERIC_CMDLINE
+
+config CMDLINE_BOOL
+ bool "Built-in kernel command line"
+
+config CMDLINE_APPEND
+ string "Built-in kernel command string append" if CMDLINE_BOOL
+ default ""
+
+config CMDLINE_PREPEND
+ string "Built-in kernel command string prepend" if CMDLINE_BOOL
+ default ""
+
+config CMDLINE_OVERRIDE
+ bool "Built-in command line overrides boot loader arguments" if CMDLINE_BOOL
+
+endif
+
endmenu # General setup
source "arch/Kconfig"
--
Then on powerpc you do:
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 2c2f33155317..1649787c3953 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -152,7 +152,7 @@ static struct prom_t __prombss prom;
static unsigned long __prombss prom_entry;
static char __prombss of_stdout_device[256];
-static char __prombss prom_scratch[256];
+static char __prombss prom_scratch[COMMAND_LINE_SIZE];
static unsigned long __prombss dt_header_start;
static unsigned long __prombss dt_struct_start, dt_struct_end;
@@ -770,6 +770,12 @@ static unsigned long prom_memparse(const char *ptr, const char **retptr)
* Early parsing of the command line passed to the kernel, used for
* "mem=x" and the options that affect the iommu
*/
+
+#define cmdline_strlcpy prom_strlcpy
+#define cmdline_strlcat prom_strlcat
+
+#include <linux/cmdline.h>
+
static void __init early_cmdline_parse(void)
{
const char *opt;
@@ -780,12 +786,11 @@ static void __init early_cmdline_parse(void)
prom_cmd_line[0] = 0;
p = prom_cmd_line;
- if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && (long)prom.chosen > 0)
+ if ((long)prom.chosen > 0)
l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
- if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\0')
- prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
- sizeof(prom_cmd_line));
+ cmdline_add_builtin_custom(prom_cmd_line, (l > 0 ? p : NULL),
+ prom_scratch, sizeof(prom_cmd_line));
prom_printf("command line: %s\n", prom_cmd_line);
--
2.25.0
Christophe
^ permalink raw reply related
* Re: [PATCH] rpadlpar: fix potential drc_name corruption in store functions
From: Michal Suchánek @ 2021-03-13 9:17 UTC (permalink / raw)
To: Tyrel Datwyler; +Cc: bhelgaas, linux-pci, mmc, linuxppc-dev, linux-kernel
In-Reply-To: <20210310223021.423155-1-tyreld@linux.ibm.com>
On Wed, Mar 10, 2021 at 04:30:21PM -0600, Tyrel Datwyler wrote:
> Both add_slot_store() and remove_slot_store() try to fix up the drc_name
> copied from the store buffer by placing a NULL terminator at nbyte + 1
> or in place of a '\n' if present. However, the static buffer that we
> copy the drc_name data into is not zeored and can contain anything past
> the n-th byte. This is problematic if a '\n' byte appears in that buffer
> after nbytes and the string copied into the store buffer was not NULL
> terminated to start with as the strchr() search for a '\n' byte will mark
> this incorrectly as the end of the drc_name string resulting in a drc_name
> string that contains garbage data after the n-th byte. The following
> debugging shows an example of the drmgr utility writing "PHB 4543" to
> the add_slot sysfs attribute, but add_slot_store logging a corrupted
> string value.
>
> [135823.702864] drmgr: drmgr: -c phb -a -s PHB 4543 -d 1
> [135823.702879] add_slot_store: drc_name = PHB 4543°|<82>!, rc = -19
>
> Fix this by NULL terminating the string when we copy it into our static
> buffer by coping nbytes + 1 of data from the store buffer. The code has
Why is it OK to copy nbytes + 1 and why is it expected that the buffer
contains a nul after the content?
Isn't it much saner to just nul terminate the string after copying?
diff --git a/drivers/pci/hotplug/rpadlpar_sysfs.c b/drivers/pci/hotplug/rpadlpar_sysfs.c
index cdbfa5df3a51..cfbad67447da 100644
--- a/drivers/pci/hotplug/rpadlpar_sysfs.c
+++ b/drivers/pci/hotplug/rpadlpar_sysfs.c
@@ -35,11 +35,11 @@ static ssize_t add_slot_store(struct kobject *kobj, struct kobj_attribute *attr,
return 0;
memcpy(drc_name, buf, nbytes);
+ &drc_name[nbytes] = '\0';
end = strchr(drc_name, '\n');
- if (!end)
- end = &drc_name[nbytes];
- *end = '\0';
+ if (end)
+ *end = '\0';
rc = dlpar_add_slot(drc_name);
if (rc)
@@ -66,11 +66,11 @@ static ssize_t remove_slot_store(struct kobject *kobj,
return 0;
memcpy(drc_name, buf, nbytes);
+ &drc_name[nbytes] = '\0';
end = strchr(drc_name, '\n');
- if (!end)
- end = &drc_name[nbytes];
- *end = '\0';
+ if (end)
+ *end = '\0';
rc = dlpar_remove_slot(drc_name);
if (rc)
Thanks
Michal
> already made sure that nbytes is not >= MAX_DRC_NAME_LEN and the store
> buffer is guaranteed to be zeroed beyond the nth-byte of data copied
> from the user. Further, since the string is now NULL terminated the code
> only needs to change '\n' to '\0' when present.
>
> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
> ---
> drivers/pci/hotplug/rpadlpar_sysfs.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pci/hotplug/rpadlpar_sysfs.c b/drivers/pci/hotplug/rpadlpar_sysfs.c
> index cdbfa5df3a51..375087921284 100644
> --- a/drivers/pci/hotplug/rpadlpar_sysfs.c
> +++ b/drivers/pci/hotplug/rpadlpar_sysfs.c
> @@ -34,12 +34,11 @@ static ssize_t add_slot_store(struct kobject *kobj, struct kobj_attribute *attr,
> if (nbytes >= MAX_DRC_NAME_LEN)
> return 0;
>
> - memcpy(drc_name, buf, nbytes);
> + memcpy(drc_name, buf, nbytes + 1);
>
> end = strchr(drc_name, '\n');
> - if (!end)
> - end = &drc_name[nbytes];
> - *end = '\0';
> + if (end)
> + *end = '\0';
>
> rc = dlpar_add_slot(drc_name);
> if (rc)
> @@ -65,12 +64,11 @@ static ssize_t remove_slot_store(struct kobject *kobj,
> if (nbytes >= MAX_DRC_NAME_LEN)
> return 0;
>
> - memcpy(drc_name, buf, nbytes);
> + memcpy(drc_name, buf, nbytes + 1);
>
> end = strchr(drc_name, '\n');
> - if (!end)
> - end = &drc_name[nbytes];
> - *end = '\0';
> + if (end)
> + *end = '\0';
>
> rc = dlpar_remove_slot(drc_name);
> if (rc)
> --
> 2.27.0
>
^ permalink raw reply related
* [PATCH] sound: soc: fsl: Remove unnecessary THIS_MODULE
From: Wang Qing @ 2021-03-13 8:23 UTC (permalink / raw)
To: Timur Tabi, Nicolin Chen, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
alsa-devel, linuxppc-dev, linux-arm-kernel, linux-kernel
Cc: Wang Qing
As THIS_MODULE has been set in module_platform_driver(), so remove it.
Signed-off-by: Wang Qing <wangqing@vivo.com>
---
sound/soc/fsl/imx-hdmi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/sound/soc/fsl/imx-hdmi.c b/sound/soc/fsl/imx-hdmi.c
index dbbb761..cd0235a
--- a/sound/soc/fsl/imx-hdmi.c
+++ b/sound/soc/fsl/imx-hdmi.c
@@ -223,7 +223,6 @@ MODULE_DEVICE_TABLE(of, imx_hdmi_dt_ids);
static struct platform_driver imx_hdmi_driver = {
.driver = {
.name = "imx-hdmi",
- .owner = THIS_MODULE,
.pm = &snd_soc_pm_ops,
.of_match_table = imx_hdmi_dt_ids,
},
--
2.7.4
^ permalink raw reply related
* [PATCH] powerpc: define the variable 'uaccess_flush' as static
From: He Ying @ 2021-03-12 11:06 UTC (permalink / raw)
To: mpe, benh, paulus, npiggin, dja, akpm, rppt, aneesh.kumar, clg
Cc: johnny.chenyi, linuxppc-dev, linux-kernel
The variable 'uaccess_fulsh' is not referenced outside the file. Perhaps we
should define it as static to avoid the warning as follows:
arch/powerpc/kernel/setup_64.c:953:6: warning: symbol 'uaccess_flush'
was not declared. Should it be static?
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: He Ying <heying24@huawei.com>
---
arch/powerpc/kernel/setup_64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 560ed8b975e7..22aca271496b 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -950,7 +950,7 @@ static bool no_entry_flush;
static bool no_uaccess_flush;
bool rfi_flush;
bool entry_flush;
-bool uaccess_flush;
+static bool uaccess_flush;
DEFINE_STATIC_KEY_FALSE(uaccess_flush_key);
EXPORT_SYMBOL(uaccess_flush_key);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] ASoC: fsl_asrc_dma: request dma channel from specific controller
From: Mark Brown @ 2021-03-12 23:00 UTC (permalink / raw)
To: perex, Xiubo.Lee, Shengjiu Wang, timur, alsa-devel, nicoleotsuka,
festevam, tiwai
Cc: Mark Brown, linuxppc-dev, linux-kernel
In-Reply-To: <1614935977-21638-1-git-send-email-shengjiu.wang@nxp.com>
On Fri, 5 Mar 2021 17:19:37 +0800, Shengjiu Wang wrote:
> Request dma channel from specific dma controller instead of generic
> dma controller list, otherwise, may get the wrong dma controller
> if there are multi dma controllers such as i.MX8MP.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: fsl_asrc_dma: request dma channel from specific controller
commit: ee427ea4f12672e5d7874abaa634ddee0ff2bb97
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH] powerpc: mm: book3s64: Fix a typo in the file mmu_context.c
From: Randy Dunlap @ 2021-03-12 16:41 UTC (permalink / raw)
To: Bhaskar Chowdhury, mpe, benh, paulus, npiggin, aneesh.kumar,
linuxppc-dev, linux-kernel
In-Reply-To: <20210312112537.4585-1-unixbhaskar@gmail.com>
On 3/12/21 3:25 AM, Bhaskar Chowdhury wrote:
>
> s/detalis/details/
>
> Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
> ---
> arch/powerpc/mm/book3s64/mmu_context.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/book3s64/mmu_context.c b/arch/powerpc/mm/book3s64/mmu_context.c
> index 0c8557220ae2..c10fc8a72fb3 100644
> --- a/arch/powerpc/mm/book3s64/mmu_context.c
> +++ b/arch/powerpc/mm/book3s64/mmu_context.c
> @@ -119,7 +119,7 @@ static int hash__init_new_context(struct mm_struct *mm)
> /* This is fork. Copy hash_context details from current->mm */
> memcpy(mm->context.hash_context, current->mm->context.hash_context, sizeof(struct hash_mm_context));
> #ifdef CONFIG_PPC_SUBPAGE_PROT
> - /* inherit subpage prot detalis if we have one. */
> + /* inherit subpage prot details if we have one. */
> if (current->mm->context.hash_context->spt) {
> mm->context.hash_context->spt = kmalloc(sizeof(struct subpage_prot_table),
> GFP_KERNEL);
> --
--
~Randy
^ permalink raw reply
* Re: [PATCH 14/17] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
From: Robin Murphy @ 2021-03-12 16:18 UTC (permalink / raw)
To: Christoph Hellwig
Cc: kvm, Will Deacon, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang,
iommu, netdev, linux-arm-kernel, virtualization, freedreno,
David Woodhouse, linux-arm-msm
In-Reply-To: <20210311082609.GA6990@lst.de>
On 2021-03-11 08:26, Christoph Hellwig wrote:
> On Wed, Mar 10, 2021 at 06:39:57PM +0000, Robin Murphy wrote:
>>> Actually... Just mirroring the iommu_dma_strict value into
>>> struct iommu_domain should solve all of that with very little
>>> boilerplate code.
>>
>> Yes, my initial thought was to directly replace the attribute with a
>> common flag at iommu_domain level, but since in all cases the behaviour
>> is effectively global rather than actually per-domain, it seemed
>> reasonable to take it a step further. This passes compile-testing for
>> arm64 and x86, what do you think?
>
> It seems to miss a few bits, and also generally seems to be not actually
> apply to recent mainline or something like it due to different empty
> lines in a few places.
Yeah, that was sketched out on top of some other development patches,
and in being so focused on not breaking any of the x86 behaviours I did
indeed overlook fully converting the SMMU drivers... oops!
(my thought was to do the conversion for its own sake, then clean up the
redundant attribute separately, but I guess it's fine either way)
> Let me know what you think of the version here:
>
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/iommu-cleanup
>
> I'll happily switch the patch to you as the author if you're fine with
> that as well.
I still have reservations about removing the attribute API entirely and
pretending that io_pgtable_cfg is anything other than a SoC-specific
private interface, but the reworked patch on its own looks reasonable to
me, thanks! (I wasn't too convinced about the iommu_cmd_line wrappers
either...) Just iommu_get_dma_strict() needs an export since the SMMU
drivers can be modular - I consciously didn't add that myself since I
was mistakenly thinking only iommu-dma would call it.
Robin.
^ permalink raw reply
* [PATCH] powerpc/numa: Fix topology_physical_package_id() on pSeries
From: Cédric Le Goater @ 2021-03-12 14:31 UTC (permalink / raw)
To: linuxppc-dev
Cc: Nathan Lynch, Srikar Dronamraju, Daniel Henrique Barboza,
Greg Kurz, Vasant Hegde, Cédric Le Goater, David Gibson
Initial commit 15863ff3b8da ("powerpc: Make chip-id information
available to userspace") introduce a cpu_to_chip_id() routine for the
PowerNV platform using the "ibm,chip-id" property to query the chip id
of a CPU. But PAPR does not specify such a property and the node id
query is broken.
Use cpu_to_node() instead which guarantees to have a correct value on
all platforms, PowerNV an pSeries.
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/asm/topology.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index 3beeb030cd78..887c42a4e43d 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -123,7 +123,7 @@ static inline int cpu_to_coregroup_id(int cpu)
#ifdef CONFIG_PPC64
#include <asm/smp.h>
-#define topology_physical_package_id(cpu) (cpu_to_chip_id(cpu))
+#define topology_physical_package_id(cpu) (cpu_to_node(cpu))
#define topology_sibling_cpumask(cpu) (per_cpu(cpu_sibling_map, cpu))
#define topology_core_cpumask(cpu) (cpu_cpu_mask(cpu))
--
2.26.2
^ permalink raw reply related
* Re: [PATCH] powerpc: define the variable 'uaccess_flush' as static
From: Cédric Le Goater @ 2021-03-12 13:33 UTC (permalink / raw)
To: He Ying, mpe, benh, paulus, npiggin, dja, akpm, rppt,
aneesh.kumar
Cc: johnny.chenyi, linuxppc-dev, linux-kernel
In-Reply-To: <20210312110638.178974-1-heying24@huawei.com>
On 3/12/21 12:06 PM, He Ying wrote:
> The variable 'uaccess_fulsh' is not referenced outside the file. Perhaps we
> should define it as static to avoid the warning as follows:
>
> arch/powerpc/kernel/setup_64.c:953:6: warning: symbol 'uaccess_flush'
> was not declared. Should it be static?
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: He Ying <heying24@huawei.com>
> ---
> arch/powerpc/kernel/setup_64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 560ed8b975e7..22aca271496b 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -950,7 +950,7 @@ static bool no_entry_flush;
> static bool no_uaccess_flush;
> bool rfi_flush;
> bool entry_flush;
I think this is the case also for entry_flush. compiling with W=1 will tell you more.
Thanks,
C.
> -bool uaccess_flush;
> +static bool uaccess_flush;
> DEFINE_STATIC_KEY_FALSE(uaccess_flush_key);
> EXPORT_SYMBOL(uaccess_flush_key);
>
>
^ permalink raw reply
* Re: [PATCH v2 1/8] powerpc/xive: Use cpu_to_node() instead of ibm,chip-id property
From: Greg Kurz @ 2021-03-12 13:03 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: QEMU Developers, list@suse.de:PowerPC, Cédric Le Goater,
David Gibson, linuxppc-dev
In-Reply-To: <0f27271d-cb4d-986c-95c6-3173b43f70e5@linux.ibm.com>
On Fri, 12 Mar 2021 09:18:39 -0300
Daniel Henrique Barboza <danielhb@linux.ibm.com> wrote:
>
>
> On 3/12/21 6:53 AM, Cédric Le Goater wrote:
> > On 3/12/21 2:55 AM, David Gibson wrote:
> >> On Tue, 9 Mar 2021 18:26:35 +0100
> >> Cédric Le Goater <clg@kaod.org> wrote:
> >>
> >>> On 3/9/21 6:08 PM, Daniel Henrique Barboza wrote:
> >>>>
> >>>>
> >>>> On 3/9/21 12:33 PM, Cédric Le Goater wrote:
> >>>>> On 3/8/21 6:13 PM, Greg Kurz wrote:
> >>>>>> On Wed, 3 Mar 2021 18:48:50 +0100
> >>>>>> Cédric Le Goater <clg@kaod.org> wrote:
> >>>>>>
> >>>>>>> The 'chip_id' field of the XIVE CPU structure is used to choose a
> >>>>>>> target for a source located on the same chip when possible. This field
> >>>>>>> is assigned on the PowerNV platform using the "ibm,chip-id" property
> >>>>>>> on pSeries under KVM when NUMA nodes are defined but it is undefined
> >>>>>>
> >>>>>> This sentence seems to have a syntax problem... like it is missing an
> >>>>>> 'and' before 'on pSeries'.
> >>>>>
> >>>>> ah yes, or simply a comma.
> >>>>>
> >>>>>>> under PowerVM. The XIVE source structure has a similar field
> >>>>>>> 'src_chip' which is only assigned on the PowerNV platform.
> >>>>>>>
> >>>>>>> cpu_to_node() returns a compatible value on all platforms, 0 being the
> >>>>>>> default node. It will also give us the opportunity to set the affinity
> >>>>>>> of a source on pSeries when we can localize them.
> >>>>>>>
> >>>>>>
> >>>>>> IIUC this relies on the fact that the NUMA node id is == to chip id
> >>>>>> on PowerNV, i.e. xc->chip_id which is passed to OPAL remain stable
> >>>>>> with this change.
> >>>>>
> >>>>> Linux sets the NUMA node in numa_setup_cpu(). On pseries, the hcall
> >>>>> H_HOME_NODE_ASSOCIATIVITY returns the node id if I am correct (Daniel
> >>>>> in Cc:)
> >>> [...]
> >>>>>
> >>>>> On PowerNV, Linux uses "ibm,associativity" property of the CPU to find
> >>>>> the node id. This value is built from the chip id in OPAL, so the
> >>>>> value returned by cpu_to_node(cpu) and the value of the "ibm,chip-id"
> >>>>> property are unlikely to be different.
> >>>>>
> >>>>> cpu_to_node(cpu) is used in many places to allocate the structures
> >>>>> locally to the owning node. XIVE is not an exception (see below in the
> >>>>> same patch), it is better to be consistent and get the same information
> >>>>> (node id) using the same routine.
> >>>>>
> >>>>>
> >>>>> In Linux, "ibm,chip-id" is only used in low level PowerNV drivers :
> >>>>> LPC, XSCOM, RNG, VAS, NX. XIVE should be in that list also but skiboot
> >>>>> unifies the controllers of the system to only expose one the OS. This
> >>>>> is problematic and should be changed but it's another topic.
> >>>>>
> >>>>>
> >>>>>> On the other hand, you have the pSeries case under PowerVM that
> >>>>>> doesn't xc->chip_id, which isn't passed to any hcall AFAICT.
> >>>>>
> >>>>> yes "ibm,chip-id" is an OPAL concept unfortunately and it has no meaning
> >>>>> under PAPR. xc->chip_id on pseries (PowerVM) will contains an invalid
> >>>>> chip id.
> >>>>>
> >>>>> QEMU/KVM exposes "ibm,chip-id" but it's not used. (its value is not
> >>>>> always correct btw)
> >>>>
> >>>>
> >>>> If you have a way to reliably reproduce this, let me know and I'll fix it
> >>>> up in QEMU.
> >>>
> >>> with :
> >>>
> >>> -smp 4,cores=1,maxcpus=8 -object memory-backend-ram,id=ram-node0,size=2G -numa node,nodeid=0,cpus=0-1,cpus=4-5,memdev=ram-node0 -object memory-backend-ram,id=ram-node1,size=2G -numa node,nodeid=1,cpus=2-3,cpus=6-7,memdev=ram-node1
> >>>
> >>> # dmesg | grep numa
> >>> [ 0.013106] numa: Node 0 CPUs: 0-1
> >>> [ 0.013136] numa: Node 1 CPUs: 2-3
> >>>
> >>> # dtc -I fs /proc/device-tree/cpus/ -f | grep ibm,chip-id
> >>> ibm,chip-id = <0x01>;
> >>> ibm,chip-id = <0x02>;
> >>> ibm,chip-id = <0x00>;
> >>> ibm,chip-id = <0x03>;
> >>>
> >>> with :
> >>>
> >>> -smp 4,cores=4,maxcpus=8,threads=1 -object memory-backend-ram,id=ram-node0,size=2G -numa node,nodeid=0,cpus=0-1,cpus=4-5,memdev=ram-node0 -object memory-backend-ram,id=ram-node1,size=2G -numa node,nodeid=1,cpus=2-3,cpus=6-7,memdev=ram-node1
> >>>
> >>> # dmesg | grep numa
> >>> [ 0.013106] numa: Node 0 CPUs: 0-1
> >>> [ 0.013136] numa: Node 1 CPUs: 2-3
> >>>
> >>> # dtc -I fs /proc/device-tree/cpus/ -f | grep ibm,chip-id
> >>> ibm,chip-id = <0x00>;
> >>> ibm,chip-id = <0x00>;
> >>> ibm,chip-id = <0x00>;
> >>> ibm,chip-id = <0x00>;
> >>>
> >>> I think we should simply remove "ibm,chip-id" since it's not used and
> >>> not in the PAPR spec.
> >>
> >> As I mentioned to Daniel on our call this morning, oddly it *does*
> >> appear to be used in the RHEL kernel, even though that's 4.18 based.
> >> This patch seems to have caused a minor regression; not in the
> >> identification of NUMA nodes, but in the number of sockets shown be
> >> lscpu, etc. See https://bugzilla.redhat.com/show_bug.cgi?id=1934421
> >> for more information.
> >
> > Yes. The property "ibm,chip-id" is wrongly calculated in QEMU. If we
> > remove it, we get with 4.18.0-295.el8.ppc64le or 5.12.0-rc2 :
> >
> > [root@localhost ~]# lscpu
> > Architecture: ppc64le
> > Byte Order: Little Endian
> > CPU(s): 128
> > On-line CPU(s) list: 0-127
> > Thread(s) per core: 4
> > Core(s) per socket: 16
> > Socket(s): 2
> > NUMA node(s): 2
> > Model: 2.2 (pvr 004e 1202)
> > Model name: POWER9 (architected), altivec supported
> > Hypervisor vendor: KVM
> > Virtualization type: para
> > L1d cache: 32K
> > L1i cache: 32K
> > NUMA node0 CPU(s): 0-63
> > NUMA node1 CPU(s): 64-127
> >
> > [root@localhost ~]# grep . /sys/devices/system/cpu/*/topology/physical_package_id
> > /sys/devices/system/cpu/cpu0/topology/physical_package_id:-1
> > /sys/devices/system/cpu/cpu100/topology/physical_package_id:-1
> > /sys/devices/system/cpu/cpu101/topology/physical_package_id:-1
> > /sys/devices/system/cpu/cpu102/topology/physical_package_id:-1
> > /sys/devices/system/cpu/cpu103/topology/physical_package_id:-1
> > ....
> >
> > "ibm,chip-id" is still being used on some occasion on pSeries machines.
> > This is wrong :/ The problem is :
> >
> > #define topology_physical_package_id(cpu) (cpu_to_chip_id(cpu))
> >
> > We should be using cpu_to_node().
>
>
> IIUC the "real fix" then is this change you mentioned above, together with
> this xive patch as well, to stop using ibm,chip-id for good in the pserie
> kernel. With these changes QEMU can remove 'ibm,chip-id' from the pseries
> machine without impact. Is this correct?
>
> If that's the case, then I believe it's ok to go forward with the QEMU side
> change (just for 6.0.0 and newer machines). Or should I wait for the kernel
> changes to be merged upstream first?
>
I'd say the latter since this is a breaking change and people will want
to identify the upstream commits they have to backport to their kernel
in order to support the disappearance of "ibm,chip-id".
Cheers,
--
Greg
>
> Thanks,
>
>
> DHB
>
>
> >
> > C.
> >
> >>
> >> Since the value was used by some PAPR kernels - even if they shouldn't
> >> have - I think we should only remove this for newer machine types. We
> >> also need to check what we're not supplying that the guest kernel is
> >> showing a different number of sockets than specified on the qemu
> >> command line.
> >>
> >>>
> >>> Thanks,
> >>>
> >>> C.
> >>>
> >>>
> >>>
> >>> [...]
> >>> [...]
> >>> [...]
> >>> [...]
> >>> [...]
> >>> [...]
> >>> [...]
> >>> [...]
> >>> [...]
> >>>
> >>
> >>
> >
^ permalink raw reply
* Re: [PATCH v2 1/8] powerpc/xive: Use cpu_to_node() instead of ibm,chip-id property
From: Cédric Le Goater @ 2021-03-12 13:28 UTC (permalink / raw)
To: Daniel Henrique Barboza, David Gibson
Cc: list@suse.de:PowerPC, linuxppc-dev, Greg Kurz, QEMU Developers
In-Reply-To: <0f27271d-cb4d-986c-95c6-3173b43f70e5@linux.ibm.com>
On 3/12/21 1:18 PM, Daniel Henrique Barboza wrote:
>
>
> On 3/12/21 6:53 AM, Cédric Le Goater wrote:
>> On 3/12/21 2:55 AM, David Gibson wrote:
>>> On Tue, 9 Mar 2021 18:26:35 +0100
>>> Cédric Le Goater <clg@kaod.org> wrote:
>>>
>>>> On 3/9/21 6:08 PM, Daniel Henrique Barboza wrote:
>>>>>
>>>>>
>>>>> On 3/9/21 12:33 PM, Cédric Le Goater wrote:
>>>>>> On 3/8/21 6:13 PM, Greg Kurz wrote:
>>>>>>> On Wed, 3 Mar 2021 18:48:50 +0100
>>>>>>> Cédric Le Goater <clg@kaod.org> wrote:
>>>>>>>
>>>>>>>> The 'chip_id' field of the XIVE CPU structure is used to choose a
>>>>>>>> target for a source located on the same chip when possible. This field
>>>>>>>> is assigned on the PowerNV platform using the "ibm,chip-id" property
>>>>>>>> on pSeries under KVM when NUMA nodes are defined but it is undefined
>>>>>>>
>>>>>>> This sentence seems to have a syntax problem... like it is missing an
>>>>>>> 'and' before 'on pSeries'.
>>>>>>
>>>>>> ah yes, or simply a comma.
>>>>>>
>>>>>>>> under PowerVM. The XIVE source structure has a similar field
>>>>>>>> 'src_chip' which is only assigned on the PowerNV platform.
>>>>>>>>
>>>>>>>> cpu_to_node() returns a compatible value on all platforms, 0 being the
>>>>>>>> default node. It will also give us the opportunity to set the affinity
>>>>>>>> of a source on pSeries when we can localize them.
>>>>>>>>
>>>>>>>
>>>>>>> IIUC this relies on the fact that the NUMA node id is == to chip id
>>>>>>> on PowerNV, i.e. xc->chip_id which is passed to OPAL remain stable
>>>>>>> with this change.
>>>>>>
>>>>>> Linux sets the NUMA node in numa_setup_cpu(). On pseries, the hcall
>>>>>> H_HOME_NODE_ASSOCIATIVITY returns the node id if I am correct (Daniel
>>>>>> in Cc:)
>>>> [...]
>>>>>>
>>>>>> On PowerNV, Linux uses "ibm,associativity" property of the CPU to find
>>>>>> the node id. This value is built from the chip id in OPAL, so the
>>>>>> value returned by cpu_to_node(cpu) and the value of the "ibm,chip-id"
>>>>>> property are unlikely to be different.
>>>>>>
>>>>>> cpu_to_node(cpu) is used in many places to allocate the structures
>>>>>> locally to the owning node. XIVE is not an exception (see below in the
>>>>>> same patch), it is better to be consistent and get the same information
>>>>>> (node id) using the same routine.
>>>>>>
>>>>>>
>>>>>> In Linux, "ibm,chip-id" is only used in low level PowerNV drivers :
>>>>>> LPC, XSCOM, RNG, VAS, NX. XIVE should be in that list also but skiboot
>>>>>> unifies the controllers of the system to only expose one the OS. This
>>>>>> is problematic and should be changed but it's another topic.
>>>>>>
>>>>>>
>>>>>>> On the other hand, you have the pSeries case under PowerVM that
>>>>>>> doesn't xc->chip_id, which isn't passed to any hcall AFAICT.
>>>>>>
>>>>>> yes "ibm,chip-id" is an OPAL concept unfortunately and it has no meaning
>>>>>> under PAPR. xc->chip_id on pseries (PowerVM) will contains an invalid
>>>>>> chip id.
>>>>>>
>>>>>> QEMU/KVM exposes "ibm,chip-id" but it's not used. (its value is not
>>>>>> always correct btw)
>>>>>
>>>>>
>>>>> If you have a way to reliably reproduce this, let me know and I'll fix it
>>>>> up in QEMU.
>>>>
>>>> with :
>>>>
>>>> -smp 4,cores=1,maxcpus=8 -object memory-backend-ram,id=ram-node0,size=2G -numa node,nodeid=0,cpus=0-1,cpus=4-5,memdev=ram-node0 -object memory-backend-ram,id=ram-node1,size=2G -numa node,nodeid=1,cpus=2-3,cpus=6-7,memdev=ram-node1
>>>>
>>>> # dmesg | grep numa
>>>> [ 0.013106] numa: Node 0 CPUs: 0-1
>>>> [ 0.013136] numa: Node 1 CPUs: 2-3
>>>>
>>>> # dtc -I fs /proc/device-tree/cpus/ -f | grep ibm,chip-id
>>>> ibm,chip-id = <0x01>;
>>>> ibm,chip-id = <0x02>;
>>>> ibm,chip-id = <0x00>;
>>>> ibm,chip-id = <0x03>;
>>>>
>>>> with :
>>>>
>>>> -smp 4,cores=4,maxcpus=8,threads=1 -object memory-backend-ram,id=ram-node0,size=2G -numa node,nodeid=0,cpus=0-1,cpus=4-5,memdev=ram-node0 -object memory-backend-ram,id=ram-node1,size=2G -numa node,nodeid=1,cpus=2-3,cpus=6-7,memdev=ram-node1
>>>>
>>>> # dmesg | grep numa
>>>> [ 0.013106] numa: Node 0 CPUs: 0-1
>>>> [ 0.013136] numa: Node 1 CPUs: 2-3
>>>>
>>>> # dtc -I fs /proc/device-tree/cpus/ -f | grep ibm,chip-id
>>>> ibm,chip-id = <0x00>;
>>>> ibm,chip-id = <0x00>;
>>>> ibm,chip-id = <0x00>;
>>>> ibm,chip-id = <0x00>;
>>>>
>>>> I think we should simply remove "ibm,chip-id" since it's not used and
>>>> not in the PAPR spec.
>>>
>>> As I mentioned to Daniel on our call this morning, oddly it *does*
>>> appear to be used in the RHEL kernel, even though that's 4.18 based.
>>> This patch seems to have caused a minor regression; not in the
>>> identification of NUMA nodes, but in the number of sockets shown be
>>> lscpu, etc. See https://bugzilla.redhat.com/show_bug.cgi?id=1934421
>>> for more information.
>>
>> Yes. The property "ibm,chip-id" is wrongly calculated in QEMU. If we
>> remove it, we get with 4.18.0-295.el8.ppc64le or 5.12.0-rc2 :
>>
>> [root@localhost ~]# lscpu
>> Architecture: ppc64le
>> Byte Order: Little Endian
>> CPU(s): 128
>> On-line CPU(s) list: 0-127
>> Thread(s) per core: 4
>> Core(s) per socket: 16
>> Socket(s): 2
>> NUMA node(s): 2
>> Model: 2.2 (pvr 004e 1202)
>> Model name: POWER9 (architected), altivec supported
>> Hypervisor vendor: KVM
>> Virtualization type: para
>> L1d cache: 32K
>> L1i cache: 32K
>> NUMA node0 CPU(s): 0-63
>> NUMA node1 CPU(s): 64-127
>>
>> [root@localhost ~]# grep . /sys/devices/system/cpu/*/topology/physical_package_id
>> /sys/devices/system/cpu/cpu0/topology/physical_package_id:-1
>> /sys/devices/system/cpu/cpu100/topology/physical_package_id:-1
>> /sys/devices/system/cpu/cpu101/topology/physical_package_id:-1
>> /sys/devices/system/cpu/cpu102/topology/physical_package_id:-1
>> /sys/devices/system/cpu/cpu103/topology/physical_package_id:-1
>> ....
>>
>> "ibm,chip-id" is still being used on some occasion on pSeries machines.
>> This is wrong :/ The problem is :
>>
>> #define topology_physical_package_id(cpu) (cpu_to_chip_id(cpu))
>>
>> We should be using cpu_to_node().
>
>
> IIUC the "real fix" then is this change you mentioned above, together with
> this xive patch as well,
These are independent.
The XIVE patch just raised the issue because it's another usage example of
cpu_to_chip_id() or directly "ibm,chip-id" in the XIVE case, on a pseries
machine.
The use of cpu_to_node(cpu) for topology_physical_package_id(cpu) is a fix
for the sysfs issue reported in the redhat BZ.
> to stop using ibm,chip-id for good in the pserie
> kernel. With these changes QEMU can remove 'ibm,chip-id' from the pseries
> machine without impact. Is this correct?
Linux is already "broken" on PowerVM today since we don't have the "ibm,chip-id"
property. QEMU is just hiding the problem on KVM.
But we have to be bug compatible :) if the QEMU fix is under the pseries-6.x
machine we should be fine.
> If that's the case, then I believe it's ok to go forward with the QEMU side
> change (just for 6.0.0 and newer machines). Or should I wait for the kernel
> changes to be merged upstream first?
Once Linux is fixed, we shouldn't care if QEMU exports 'ibm,chip-id' or not.
I don't think the order is very important. These are independent.
C.
^ permalink raw reply
* [PATCH v3 03/15] powerpc/align: Convert emulate_spe() to user_access_begin
From: Christophe Leroy @ 2021-03-12 13:25 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Daniel Axtens
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <0ad4629c2d222019e82fcdfccc70d372beb4adf9.1615398265.git.christophe.leroy@csgroup.eu>
This patch converts emulate_spe() to using user_access_begin
logic.
Since commit 662bbcb2747c ("mm, sched: Allow uaccess in atomic with
pagefault_disable()"), might_fault() doesn't fire when called from
sections where pagefaults are disabled, which must be the case
when using _inatomic variants of __get_user and __put_user. So
the might_fault() in user_access_begin() is not a problem.
There was a verification of user_mode() together with the access_ok(),
but there is a second verification of user_mode() just after, that
leads to immediate return. The access_ok() is now part of the
user_access_begin which is called after that other user_mode()
verification, so no need to check user_mode() again.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Daniel Axtens <dja@axtens.net>
---
v3:
- Changed the second user_read_access_begin() to user_write_access_begin()
- Reworded explaination about the user_mode() with access_ok() in the commit message
---
arch/powerpc/kernel/align.c | 61 ++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index c7797eb958c7..f362c99213be 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -107,7 +107,6 @@ static struct aligninfo spe_aligninfo[32] = {
static int emulate_spe(struct pt_regs *regs, unsigned int reg,
struct ppc_inst ppc_instr)
{
- int ret;
union {
u64 ll;
u32 w[2];
@@ -127,11 +126,6 @@ static int emulate_spe(struct pt_regs *regs, unsigned int reg,
nb = spe_aligninfo[instr].len;
flags = spe_aligninfo[instr].flags;
- /* Verify the address of the operand */
- if (unlikely(user_mode(regs) &&
- !access_ok(addr, nb)))
- return -EFAULT;
-
/* userland only */
if (unlikely(!user_mode(regs)))
return 0;
@@ -169,26 +163,27 @@ static int emulate_spe(struct pt_regs *regs, unsigned int reg,
}
} else {
temp.ll = data.ll = 0;
- ret = 0;
p = addr;
+ if (!user_read_access_begin(addr, nb))
+ return -EFAULT;
+
switch (nb) {
case 8:
- ret |= __get_user_inatomic(temp.v[0], p++);
- ret |= __get_user_inatomic(temp.v[1], p++);
- ret |= __get_user_inatomic(temp.v[2], p++);
- ret |= __get_user_inatomic(temp.v[3], p++);
+ unsafe_get_user(temp.v[0], p++, Efault_read);
+ unsafe_get_user(temp.v[1], p++, Efault_read);
+ unsafe_get_user(temp.v[2], p++, Efault_read);
+ unsafe_get_user(temp.v[3], p++, Efault_read);
fallthrough;
case 4:
- ret |= __get_user_inatomic(temp.v[4], p++);
- ret |= __get_user_inatomic(temp.v[5], p++);
+ unsafe_get_user(temp.v[4], p++, Efault_read);
+ unsafe_get_user(temp.v[5], p++, Efault_read);
fallthrough;
case 2:
- ret |= __get_user_inatomic(temp.v[6], p++);
- ret |= __get_user_inatomic(temp.v[7], p++);
- if (unlikely(ret))
- return -EFAULT;
+ unsafe_get_user(temp.v[6], p++, Efault_read);
+ unsafe_get_user(temp.v[7], p++, Efault_read);
}
+ user_read_access_end();
switch (instr) {
case EVLDD:
@@ -255,31 +250,41 @@ static int emulate_spe(struct pt_regs *regs, unsigned int reg,
/* Store result to memory or update registers */
if (flags & ST) {
- ret = 0;
p = addr;
+
+ if (!user_write_access_begin(addr, nb))
+ return -EFAULT;
+
switch (nb) {
case 8:
- ret |= __put_user_inatomic(data.v[0], p++);
- ret |= __put_user_inatomic(data.v[1], p++);
- ret |= __put_user_inatomic(data.v[2], p++);
- ret |= __put_user_inatomic(data.v[3], p++);
+ unsafe_put_user(data.v[0], p++, Efault_write);
+ unsafe_put_user(data.v[1], p++, Efault_write);
+ unsafe_put_user(data.v[2], p++, Efault_write);
+ unsafe_put_user(data.v[3], p++, Efault_write);
fallthrough;
case 4:
- ret |= __put_user_inatomic(data.v[4], p++);
- ret |= __put_user_inatomic(data.v[5], p++);
+ unsafe_put_user(data.v[4], p++, Efault_write);
+ unsafe_put_user(data.v[5], p++, Efault_write);
fallthrough;
case 2:
- ret |= __put_user_inatomic(data.v[6], p++);
- ret |= __put_user_inatomic(data.v[7], p++);
+ unsafe_put_user(data.v[6], p++, Efault_write);
+ unsafe_put_user(data.v[7], p++, Efault_write);
}
- if (unlikely(ret))
- return -EFAULT;
+ user_write_access_end();
} else {
*evr = data.w[0];
regs->gpr[reg] = data.w[1];
}
return 1;
+
+Efault_read:
+ user_read_access_end();
+ return -EFAULT;
+
+Efault_write:
+ user_write_access_end();
+ return -EFAULT;
}
#endif /* CONFIG_SPE */
--
2.25.0
^ permalink raw reply related
* Re: [PATCH] powerpc: Force inlining of cpu_has_feature() to avoid build failure
From: Christophe Leroy @ 2021-03-12 13:12 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <b231dfa040ce4cc37f702f5c3a595fdeabfe0462.1615378209.git.christophe.leroy@csgroup.eu>
Le 10/03/2021 à 13:10, Christophe Leroy a écrit :
> The code relies on constant folding of cpu_has_feature() based
> on possible and always true values as defined per
> CPU_FTRS_ALWAYS and CPU_FTRS_POSSIBLE.
>
> Build failure is encountered with for instance
> book3e_all_defconfig on kisskb in the AMDGPU driver which uses
> cpu_has_feature(CPU_FTR_VSX_COMP) to decide whether calling
> kernel_enable_vsx() or not.
>
> The failure is due to cpu_has_feature() not being inlined with
> that configuration with gcc 4.9.
>
> In the same way as commit acdad8fb4a15 ("powerpc: Force inlining of
> mmu_has_feature to fix build failure"), for inlining of
> cpu_has_feature().
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Fixes following build
http://kisskb.ellerman.id.au/kisskb/buildresult/14489391/
> ---
> arch/powerpc/include/asm/cpu_has_feature.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cpu_has_feature.h b/arch/powerpc/include/asm/cpu_has_feature.h
> index 7897d16e0990..727d4b321937 100644
> --- a/arch/powerpc/include/asm/cpu_has_feature.h
> +++ b/arch/powerpc/include/asm/cpu_has_feature.h
> @@ -7,7 +7,7 @@
> #include <linux/bug.h>
> #include <asm/cputable.h>
>
> -static inline bool early_cpu_has_feature(unsigned long feature)
> +static __always_inline bool early_cpu_has_feature(unsigned long feature)
> {
> return !!((CPU_FTRS_ALWAYS & feature) ||
> (CPU_FTRS_POSSIBLE & cur_cpu_spec->cpu_features & feature));
> @@ -46,7 +46,7 @@ static __always_inline bool cpu_has_feature(unsigned long feature)
> return static_branch_likely(&cpu_feature_keys[i]);
> }
> #else
> -static inline bool cpu_has_feature(unsigned long feature)
> +static __always_inline bool cpu_has_feature(unsigned long feature)
> {
> return early_cpu_has_feature(feature);
> }
>
^ permalink raw reply
* Re: [PATCH] powerpc/vdso32: Add missing _restgpr_31_x to fix build failure
From: Christophe Leroy @ 2021-03-12 13:09 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <a7aa198a88bcd33c6e35e99f70f86c7b7f2f9440.1615270757.git.christophe.leroy@csgroup.eu>
Le 09/03/2021 à 07:19, Christophe Leroy a écrit :
> With some defconfig including CONFIG_CC_OPTIMIZE_FOR_SIZE,
> (for instance mvme5100_defconfig and ps3_defconfig), gcc 5
> generates a call to _restgpr_31_x.
>
> Until recently it went unnoticed, but
> commit 42ed6d56ade2 ("powerpc/vdso: Block R_PPC_REL24 relocations")
> made it rise to the surface.
>
> Provide that function (copied from lib/crtsavres.S) in
> gettimeofday.S
>
> Fixes: ab037dd87a2f ("powerpc/vdso: Switch VDSO to generic C implementation.")
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Fixes the following builds:
http://kisskb.ellerman.id.au/kisskb/buildresult/14492138/
http://kisskb.ellerman.id.au/kisskb/buildresult/14492041/
Christophe
> ---
> I don't know if there is a way to tell GCC not to emit that call, because at the end we get more instructions than needed.
> ---
> arch/powerpc/kernel/vdso32/gettimeofday.S | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
> index a6e29f880e0e..d21d08140a5e 100644
> --- a/arch/powerpc/kernel/vdso32/gettimeofday.S
> +++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
> @@ -65,3 +65,14 @@ V_FUNCTION_END(__kernel_clock_getres)
> V_FUNCTION_BEGIN(__kernel_time)
> cvdso_call_time __c_kernel_time
> V_FUNCTION_END(__kernel_time)
> +
> +/* Routines for restoring integer registers, called by the compiler. */
> +/* Called with r11 pointing to the stack header word of the caller of the */
> +/* function, just beyond the end of the integer restore area. */
> +_GLOBAL(_restgpr_31_x)
> +_GLOBAL(_rest32gpr_31_x)
> + lwz r0,4(r11)
> + lwz r31,-4(r11)
> + mtlr r0
> + mr r1,r11
> + blr
>
^ permalink raw reply
* Re: [PATCH 2/4] tools/perf: Add dynamic headers for perf report columns
From: Jiri Olsa @ 2021-03-12 12:57 UTC (permalink / raw)
To: Athira Rajeev
Cc: ravi.bangoria, maddy, peterz, linux-kernel, acme,
linux-perf-users, jolsa, kjain, linuxppc-dev, kan.liang
In-Reply-To: <1615298640-1529-3-git-send-email-atrajeev@linux.vnet.ibm.com>
On Tue, Mar 09, 2021 at 09:03:58AM -0500, Athira Rajeev wrote:
> Currently the header string for different columns in perf report
> is fixed. Some fields of perf sample could have different meaning
> for different architectures than the meaning conveyed by the header
> string. An example is the new field 'var2_w' of perf_sample_weight
> structure. This is presently captured as 'Local INSTR Latency' in
> perf mem report. But this could be used to denote a different latency
> cycle in another architecture.
>
> Introduce a weak function arch_perf_header_entry__add() to set
> the arch specific header string for the fields which can contain dynamic
> header. If the architecture do not have this function, fall back to the
> default header string value.
>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> tools/perf/util/event.h | 1 +
> tools/perf/util/sort.c | 19 ++++++++++++++++++-
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index f603edbbbc6f..89b149e2e70a 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -427,5 +427,6 @@ void cpu_map_data__synthesize(struct perf_record_cpu_map_data *data, struct per
>
> void arch_perf_parse_sample_weight(struct perf_sample *data, const __u64 *array, u64 type);
> void arch_perf_synthesize_sample_weight(const struct perf_sample *data, __u64 *array, u64 type);
> +const char *arch_perf_header_entry__add(const char *se_header);
>
> #endif /* __PERF_RECORD_H */
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 0d5ad42812b9..741a6df29fa0 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -25,6 +25,7 @@
> #include <traceevent/event-parse.h>
> #include "mem-events.h"
> #include "annotate.h"
> +#include "event.h"
> #include "time-utils.h"
> #include "cgroup.h"
> #include "machine.h"
> @@ -45,6 +46,7 @@
> regex_t ignore_callees_regex;
> int have_ignore_callees = 0;
> enum sort_mode sort__mode = SORT_MODE__NORMAL;
> +const char *dynamic_headers[] = {"local_ins_lat"};
>
> /*
> * Replaces all occurrences of a char used with the:
> @@ -1816,6 +1818,16 @@ struct sort_dimension {
> int taken;
> };
>
> +const char * __weak arch_perf_header_entry__add(const char *se_header)
no need for the __add suffix in here
jirka
> +{
> + return se_header;
> +}
> +
> +static void sort_dimension_add_dynamic_header(struct sort_dimension *sd)
> +{
> + sd->entry->se_header = arch_perf_header_entry__add(sd->entry->se_header);
> +}
> +
> #define DIM(d, n, func) [d] = { .name = n, .entry = &(func) }
>
> static struct sort_dimension common_sort_dimensions[] = {
> @@ -2739,11 +2751,16 @@ int sort_dimension__add(struct perf_hpp_list *list, const char *tok,
> struct evlist *evlist,
> int level)
> {
> - unsigned int i;
> + unsigned int i, j;
>
> for (i = 0; i < ARRAY_SIZE(common_sort_dimensions); i++) {
> struct sort_dimension *sd = &common_sort_dimensions[i];
>
> + for (j = 0; j < ARRAY_SIZE(dynamic_headers); j++) {
> + if (!strcmp(dynamic_headers[j], sd->name))
> + sort_dimension_add_dynamic_header(sd);
> + }
> +
> if (strncasecmp(tok, sd->name, strlen(tok)))
> continue;
>
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH 4/4] tools/perf: Support pipeline stage cycles for powerpc
From: Jiri Olsa @ 2021-03-12 12:56 UTC (permalink / raw)
To: Athira Rajeev
Cc: ravi.bangoria, maddy, peterz, linux-kernel, acme,
linux-perf-users, jolsa, kjain, linuxppc-dev, kan.liang
In-Reply-To: <1615298640-1529-5-git-send-email-atrajeev@linux.vnet.ibm.com>
On Tue, Mar 09, 2021 at 09:04:00AM -0500, Athira Rajeev wrote:
> The pipeline stage cycles details can be recorded on powerpc from
> the contents of Performance Monitor Unit (PMU) registers. On
> ISA v3.1 platform, sampling registers exposes the cycles spent in
> different pipeline stages. Patch adds perf tools support to present
> two of the cycle counter information along with memory latency (weight).
>
> Re-use the field 'ins_lat' for storing the first pipeline stage cycle.
> This is stored in 'var2_w' field of 'perf_sample_weight'.
>
> Add a new field 'p_stage_cyc' to store the second pipeline stage cycle
> which is stored in 'var3_w' field of perf_sample_weight.
>
> Add new sort function 'Pipeline Stage Cycle' and include this in
> default_mem_sort_order[]. This new sort function may be used to denote
> some other pipeline stage in another architecture. So add this to
> list of sort entries that can have dynamic header string.
>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> tools/perf/Documentation/perf-report.txt | 1 +
> tools/perf/arch/powerpc/util/event.c | 18 ++++++++++++++++--
> tools/perf/util/event.h | 1 +
> tools/perf/util/hist.c | 11 ++++++++---
> tools/perf/util/hist.h | 1 +
> tools/perf/util/session.c | 4 +++-
> tools/perf/util/sort.c | 24 ++++++++++++++++++++++--
> tools/perf/util/sort.h | 2 ++
> 8 files changed, 54 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> index f546b5e9db05..9691d9c227ba 100644
> --- a/tools/perf/Documentation/perf-report.txt
> +++ b/tools/perf/Documentation/perf-report.txt
> @@ -112,6 +112,7 @@ OPTIONS
> - ins_lat: Instruction latency in core cycles. This is the global instruction
> latency
> - local_ins_lat: Local instruction latency version
> + - p_stage_cyc: Number of cycles spent in a pipeline stage.
please specify in here that it's ppc only
SNIP
> +struct sort_entry sort_p_stage_cyc = {
> + .se_header = "Pipeline Stage Cycle",
> + .se_cmp = sort__global_p_stage_cyc_cmp,
> + .se_snprintf = hist_entry__p_stage_cyc_snprintf,
> + .se_width_idx = HISTC_P_STAGE_CYC,
> +};
> +
> struct sort_entry sort_mem_daddr_sym = {
> .se_header = "Data Symbol",
> .se_cmp = sort__daddr_cmp,
> @@ -1853,6 +1872,7 @@ static void sort_dimension_add_dynamic_header(struct sort_dimension *sd)
> DIM(SORT_CODE_PAGE_SIZE, "code_page_size", sort_code_page_size),
> DIM(SORT_LOCAL_INS_LAT, "local_ins_lat", sort_local_ins_lat),
> DIM(SORT_GLOBAL_INS_LAT, "ins_lat", sort_global_ins_lat),
> + DIM(SORT_P_STAGE_CYC, "p_stage_cyc", sort_p_stage_cyc),
this might be out of scope for this patch, but would it make sense
to add arch specific sort dimension? so the specific column is
not even visible on arch that it's not supported on
> };
>
> #undef DIM
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index 63f67a3f3630..23b20cbbc846 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -51,6 +51,7 @@ struct he_stat {
> u64 period_guest_us;
> u64 weight;
> u64 ins_lat;
> + u64 p_stage_cyc;
> u32 nr_events;
> };
>
> @@ -234,6 +235,7 @@ enum sort_type {
> SORT_CODE_PAGE_SIZE,
> SORT_LOCAL_INS_LAT,
> SORT_GLOBAL_INS_LAT,
> + SORT_P_STAGE_CYC,
we could have the whole 'SORT_PEPELINE_STAGE_CYC',
so it's more obvious
thanks,
jirka
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox