* [PATCH AUTOSEL 5.4 15/24] powerpc: Force inlining of cpu_has_feature() to avoid build failure
From: Sasha Levin @ 2021-03-25 11:26 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev
In-Reply-To: <20210325112651.1927828-1-sashal@kernel.org>
From: Christophe Leroy <christophe.leroy@csgroup.eu>
[ Upstream commit eed5fae00593ab9d261a0c1ffc1bdb786a87a55a ]
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>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/b231dfa040ce4cc37f702f5c3a595fdeabfe0462.1615378209.git.christophe.leroy@csgroup.eu
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
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);
}
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 11/20] powerpc: Force inlining of cpu_has_feature() to avoid build failure
From: Sasha Levin @ 2021-03-25 11:27 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev
In-Reply-To: <20210325112724.1928174-1-sashal@kernel.org>
From: Christophe Leroy <christophe.leroy@csgroup.eu>
[ Upstream commit eed5fae00593ab9d261a0c1ffc1bdb786a87a55a ]
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>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/b231dfa040ce4cc37f702f5c3a595fdeabfe0462.1615378209.git.christophe.leroy@csgroup.eu
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
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);
}
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 09/16] powerpc: Force inlining of cpu_has_feature() to avoid build failure
From: Sasha Levin @ 2021-03-25 11:27 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev
In-Reply-To: <20210325112751.1928421-1-sashal@kernel.org>
From: Christophe Leroy <christophe.leroy@csgroup.eu>
[ Upstream commit eed5fae00593ab9d261a0c1ffc1bdb786a87a55a ]
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>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/b231dfa040ce4cc37f702f5c3a595fdeabfe0462.1615378209.git.christophe.leroy@csgroup.eu
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
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);
}
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 06/13] powerpc: Force inlining of cpu_has_feature() to avoid build failure
From: Sasha Levin @ 2021-03-25 11:28 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev
In-Reply-To: <20210325112814.1928637-1-sashal@kernel.org>
From: Christophe Leroy <christophe.leroy@csgroup.eu>
[ Upstream commit eed5fae00593ab9d261a0c1ffc1bdb786a87a55a ]
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>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/b231dfa040ce4cc37f702f5c3a595fdeabfe0462.1615378209.git.christophe.leroy@csgroup.eu
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
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 6e834caa3720..7b10b3ef7739 100644
--- a/arch/powerpc/include/asm/cpu_has_feature.h
+++ b/arch/powerpc/include/asm/cpu_has_feature.h
@@ -6,7 +6,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));
@@ -45,7 +45,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);
}
--
2.30.1
^ permalink raw reply related
* [PATCH v3 2/2] powerpc/perf: Add platform specific check_attr_config
From: Madhavan Srinivasan @ 2021-03-25 11:53 UTC (permalink / raw)
To: mpe; +Cc: Madhavan Srinivasan, linuxppc-dev
In-Reply-To: <20210325115326.143151-1-maddy@linux.ibm.com>
Add platform specific attr.config value checks. Patch
includes checks for both power9 and power10.
Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
---
Changelog v2:
- Changed function name as suggested.
- Added name of source document referred for reserved values
Changelog v1:
- No changes
arch/powerpc/perf/isa207-common.c | 42 +++++++++++++++++++++++++++++++
arch/powerpc/perf/isa207-common.h | 2 ++
arch/powerpc/perf/power10-pmu.c | 13 ++++++++++
arch/powerpc/perf/power9-pmu.c | 13 ++++++++++
4 files changed, 70 insertions(+)
diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
index e4f577da33d8..358a0e95ba5f 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ -694,3 +694,45 @@ int isa207_get_alternatives(u64 event, u64 alt[], int size, unsigned int flags,
return num_alt;
}
+
+int isa3XX_check_attr_config(struct perf_event *ev)
+{
+ u64 val, sample_mode;
+ u64 event = ev->attr.config;
+
+ val = (event >> EVENT_SAMPLE_SHIFT) & EVENT_SAMPLE_MASK;
+ sample_mode = val & 0x3;
+
+ /*
+ * MMCRA[61:62] is Random Sampling Mode (SM).
+ * value of 0b11 is reserved.
+ */
+ if (sample_mode == 0x3)
+ return -EINVAL;
+
+ /*
+ * Check for all reserved value
+ * Source: Performance Monitoring Unit User Guide
+ */
+ switch (val) {
+ case 0x5:
+ case 0x9:
+ case 0xD:
+ case 0x19:
+ case 0x1D:
+ case 0x1A:
+ case 0x1E:
+ return -EINVAL;
+ }
+
+ /*
+ * MMCRA[48:51]/[52:55]) Threshold Start/Stop
+ * Events Selection.
+ * 0b11110000/0b00001111 is reserved.
+ */
+ val = (event >> EVENT_THR_CTL_SHIFT) & EVENT_THR_CTL_MASK;
+ if (((val & 0xF0) == 0xF0) || ((val & 0xF) == 0xF))
+ return -EINVAL;
+
+ return 0;
+}
diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h
index 1af0e8c97ac7..b4d2a2b2b346 100644
--- a/arch/powerpc/perf/isa207-common.h
+++ b/arch/powerpc/perf/isa207-common.h
@@ -280,4 +280,6 @@ void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
struct pt_regs *regs);
void isa207_get_mem_weight(u64 *weight);
+int isa3XX_check_attr_config(struct perf_event *ev);
+
#endif
diff --git a/arch/powerpc/perf/power10-pmu.c b/arch/powerpc/perf/power10-pmu.c
index a901c1348cad..f9d64c63bb4a 100644
--- a/arch/powerpc/perf/power10-pmu.c
+++ b/arch/powerpc/perf/power10-pmu.c
@@ -106,6 +106,18 @@ static int power10_get_alternatives(u64 event, unsigned int flags, u64 alt[])
return num_alt;
}
+static int power10_check_attr_config(struct perf_event *ev)
+{
+ u64 val;
+ u64 event = ev->attr.config;
+
+ val = (event >> EVENT_SAMPLE_SHIFT) & EVENT_SAMPLE_MASK;
+ if (val == 0x10 || isa3XX_check_attr_config(ev))
+ return -EINVAL;
+
+ return 0;
+}
+
GENERIC_EVENT_ATTR(cpu-cycles, PM_RUN_CYC);
GENERIC_EVENT_ATTR(instructions, PM_RUN_INST_CMPL);
GENERIC_EVENT_ATTR(branch-instructions, PM_BR_CMPL);
@@ -559,6 +571,7 @@ static struct power_pmu power10_pmu = {
.attr_groups = power10_pmu_attr_groups,
.bhrb_nr = 32,
.capabilities = PERF_PMU_CAP_EXTENDED_REGS,
+ .check_attr_config = power10_check_attr_config,
};
int init_power10_pmu(void)
diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
index 2a57e93a79dc..ff3382140d7e 100644
--- a/arch/powerpc/perf/power9-pmu.c
+++ b/arch/powerpc/perf/power9-pmu.c
@@ -151,6 +151,18 @@ static int power9_get_alternatives(u64 event, unsigned int flags, u64 alt[])
return num_alt;
}
+static int power9_check_attr_config(struct perf_event *ev)
+{
+ u64 val;
+ u64 event = ev->attr.config;
+
+ val = (event >> EVENT_SAMPLE_SHIFT) & EVENT_SAMPLE_MASK;
+ if (val == 0xC || isa3XX_check_attr_config(ev))
+ return -EINVAL;
+
+ return 0;
+}
+
GENERIC_EVENT_ATTR(cpu-cycles, PM_CYC);
GENERIC_EVENT_ATTR(stalled-cycles-frontend, PM_ICT_NOSLOT_CYC);
GENERIC_EVENT_ATTR(stalled-cycles-backend, PM_CMPLU_STALL);
@@ -437,6 +449,7 @@ static struct power_pmu power9_pmu = {
.attr_groups = power9_pmu_attr_groups,
.bhrb_nr = 32,
.capabilities = PERF_PMU_CAP_EXTENDED_REGS,
+ .check_attr_config = power9_check_attr_config,
};
int init_power9_pmu(void)
--
2.26.2
^ permalink raw reply related
* [PATCH v3 1/2] powerpc/perf: Infrastructure to support checking of attr.config*
From: Madhavan Srinivasan @ 2021-03-25 11:53 UTC (permalink / raw)
To: mpe; +Cc: Madhavan Srinivasan, linuxppc-dev
Introduce code to support the checking of attr.config* for
values which are reserved for a given platform.
Performance Monitoring Unit (PMU) configuration registers
have fields that are reserved and some specific values for
bit fields are reserved. For ex., MMCRA[61:62] is
Random Sampling Mode (SM) and value of 0b11 for this field
is reserved.
Writing non-zero or invalid values in these fields will
have unknown behaviours.
Patch adds a generic call-back function "check_attr_config"
in "struct power_pmu", to be called in event_init to
check for attr.config* values for a given platform.
"check_attr_config" is valid only for raw event type.
Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
---
Changelog v2:
-Fixed commit message
Changelog v1:
-Fixed commit message and in-code comments
arch/powerpc/include/asm/perf_event_server.h | 6 ++++++
arch/powerpc/perf/core-book3s.c | 14 ++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
index 00e7e671bb4b..dde97d7d9253 100644
--- a/arch/powerpc/include/asm/perf_event_server.h
+++ b/arch/powerpc/include/asm/perf_event_server.h
@@ -67,6 +67,12 @@ struct power_pmu {
* the pmu supports extended perf regs capability
*/
int capabilities;
+ /*
+ * Function to check event code for values which are
+ * reserved. Function takes struct perf_event as input,
+ * since event code could be spread in attr.config*
+ */
+ int (*check_attr_config)(struct perf_event *ev);
};
/*
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;
default:
return -ENOENT;
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v2 3/7] powerpc: convert config files to generic cmdline
From: Christophe Leroy @ 2021-03-25 12:03 UTC (permalink / raw)
To: Rob Herring
Cc: Daniel Gimpelevich, linuxppc-dev, X86 ML, open list:MIPS,
linux-kernel@vger.kernel.org, Paul Mackerras, xe-linux-external,
Andrew Morton, Will Deacon, Daniel Walker
In-Reply-To: <CAL_JsqKm76jRQYDcu3rGyUWKPLspoO=EZW_WFy=zAK+m_JYCTg@mail.gmail.com>
Le 24/03/2021 à 18:32, Rob Herring a écrit :
> On Wed, Mar 24, 2021 at 11:01 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 09/03/2021 à 22:29, Daniel Walker a écrit :
>>> On Tue, Mar 09, 2021 at 08:47:09AM +0100, Christophe Leroy wrote:
>>>>
>>>>
>>>> Le 09/03/2021 à 01:02, Daniel Walker a écrit :
>>>>> This is a scripted mass convert of the config files to use
>>>>> the new generic cmdline. There is a bit of a trim effect here.
>>>>> It would seems that some of the config haven't been trimmed in
>>>>> a while.
>>>>
>>>> If you do that in a separate patch, you loose bisectability.
>>>>
>>>> I think it would have been better to do things in a different way, more or less like I did in my series:
>>>> 1/ Provide GENERIC cmdline at the same functionnality level as what is
>>>> spread in the different architectures
>>>> 2/ Convert architectures to the generic with least churn.
>>>> 3/ Add new features to the generic
>>>
>>> You have to have the churn eventually, no matter how you do it. The only way you
>>> don't have churn is if you never upgrade the feature set.
>>>
>>>
>>>>>
>>>>> The bash script used to convert is as follows,
>>>>>
>>>>> if [[ -z "$1" || -z "$2" ]]; then
>>>>> echo "Two arguments are needed."
>>>>> exit 1
>>>>> fi
>>>>> mkdir $1
>>>>> cp $2 $1/.config
>>>>> sed -i 's/CONFIG_CMDLINE=/CONFIG_CMDLINE_BOOL=y\nCONFIG_CMDLINE_PREPEND=/g' $1/.config
>>>>
>>>> This is not correct.
>>>>
>>>> By default, on powerpc the provided command line is used only if the bootloader doesn't provide one.
>>>>
>>>> Otherwise:
>>>> - the builtin command line is appended to the one provided by the bootloader
>>>> if CONFIG_CMDLINE_EXTEND is selected
>>>> - the builtin command line replaces to the one provided by the bootloader if
>>>> CONFIG_CMDLINE_FORCE is selected
>>>
>>> I think my changes maintain most of this due to the override of
>>> CONFIG_CMDLINE_PREPEND. This is an upgrade and the inflexibility in powerpc is
>>> an example of why these changes were created in the first place.
>>
>> "inflexibility in powerpc" : Can you elaborate ?
>>
>>>
>>> For example , say the default command line is "root=/dev/issblk0" from iss476
>>> platform. And the bootloader adds "root=/dev/sda1"
>>>
>>> The result is <prepend><bootloader><append>.
>>
>>
>> I'm still having hard time understanding the benefit of having both <prepend> and <append>.
>> Could you please provide a complete exemple from real life, ie what exactly the problem is and what
>> it solves ?
>
> It doesn't matter. We already have both cases and 'extend' has meant either one.
>
> What someone wants is policy and the kernel shouldn't be defining the policy.
>
Ok, so you agree we don't need to provide two CMDLINE, one to be appended and one to be prepended.
Let's only provide once CMDLINE as of today, and ask the user to select whether he wants it appended
or prepended or replacee. Then no need to change all existing config to rename CONFIG_CMDLINE into
either of the new ones.
That's the main difference between my series and Daniel's series. So I'll finish taking Will's
comment into account and we'll send out a v3 soon.
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH v7] powerpc/irq: Inline call_do_irq() and call_do_softirq()
From: Segher Boessenkool @ 2021-03-25 12:44 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <87k0pwhh5y.fsf@mpe.ellerman.id.au>
On Wed, Mar 24, 2021 at 11:26:01PM +1100, Michael Ellerman wrote:
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> > Hmm. It is the first time we use named parameters in powerpc assembly, isn't it ?
> Yeah I'd like us to use it more, I think it helps readability a lot.
..in some cases. Not in most cases :-(
> > Wondering, how would the below look like with named parameters (from __put_user_asm2_goto) ?
> >
> > stw%X1 %L0, %L1
>
> Not sure, possibly that's too complicated for it :)
asm("stw%X[name1] %L[name0],%L[name1]" :: [name0]"r"(x), [name1]"m"(p));
Yes, it is not more readable *at all*.
Segher
^ permalink raw reply
* Re: [PATCH V2 1/5] powerpc/perf: Expose processor pipeline stage cycles using PERF_SAMPLE_WEIGHT_STRUCT
From: Arnaldo Carvalho de Melo @ 2021-03-25 13:01 UTC (permalink / raw)
To: Madhavan Srinivasan
Cc: ravi.bangoria, Athira Rajeev, peterz, linux-kernel,
linux-perf-users, jolsa, kjain, linuxppc-dev, kan.liang
In-Reply-To: <d7dd633b-e28a-155a-a8e2-0e5a83b4eead@linux.ibm.com>
Em Wed, Mar 24, 2021 at 10:05:23AM +0530, Madhavan Srinivasan escreveu:
>
> On 3/22/21 8:27 PM, Athira Rajeev wrote:
> > Performance Monitoring Unit (PMU) registers in powerpc provides
> > information on cycles elapsed between different stages in the
> > pipeline. This can be used for application tuning. On ISA v3.1
> > platform, this information is exposed by sampling registers.
> > Patch adds kernel support to capture two of the cycle counters
> > as part of perf sample using the sample type:
> > PERF_SAMPLE_WEIGHT_STRUCT.
> >
> > The power PMU function 'get_mem_weight' currently uses 64 bit weight
> > field of perf_sample_data to capture memory latency. But following the
> > introduction of PERF_SAMPLE_WEIGHT_TYPE, weight field could contain
> > 64-bit or 32-bit value depending on the architexture support for
> > PERF_SAMPLE_WEIGHT_STRUCT. Patches uses WEIGHT_STRUCT to expose the
> > pipeline stage cycles info. Hence update the ppmu functions to work for
> > 64-bit and 32-bit weight values.
> >
> > If the sample type is PERF_SAMPLE_WEIGHT, use the 64-bit weight field.
> > if the sample type is PERF_SAMPLE_WEIGHT_STRUCT, memory subsystem
> > latency is stored in the low 32bits of perf_sample_weight structure.
> > Also for CPU_FTR_ARCH_31, capture the two cycle counter information in
> > two 16 bit fields of perf_sample_weight structure.
>
> Changes looks fine to me.
>
> Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
So who will process the kernel bits? I'm merging the tooling parts,
Thanks,
- Arnaldo
>
> > Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/include/asm/perf_event_server.h | 2 +-
> > arch/powerpc/perf/core-book3s.c | 4 ++--
> > arch/powerpc/perf/isa207-common.c | 29 +++++++++++++++++++++++++---
> > arch/powerpc/perf/isa207-common.h | 6 +++++-
> > 4 files changed, 34 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
> > index 00e7e671bb4b..112cf092d7b3 100644
> > --- a/arch/powerpc/include/asm/perf_event_server.h
> > +++ b/arch/powerpc/include/asm/perf_event_server.h
> > @@ -43,7 +43,7 @@ struct power_pmu {
> > u64 alt[]);
> > void (*get_mem_data_src)(union perf_mem_data_src *dsrc,
> > u32 flags, struct pt_regs *regs);
> > - void (*get_mem_weight)(u64 *weight);
> > + void (*get_mem_weight)(u64 *weight, u64 type);
> > unsigned long group_constraint_mask;
> > unsigned long group_constraint_val;
> > u64 (*bhrb_filter_map)(u64 branch_sample_type);
> > diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> > index 766f064f00fb..6936763246bd 100644
> > --- a/arch/powerpc/perf/core-book3s.c
> > +++ b/arch/powerpc/perf/core-book3s.c
> > @@ -2206,9 +2206,9 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
> > ppmu->get_mem_data_src)
> > ppmu->get_mem_data_src(&data.data_src, ppmu->flags, regs);
> > - if (event->attr.sample_type & PERF_SAMPLE_WEIGHT &&
> > + if (event->attr.sample_type & PERF_SAMPLE_WEIGHT_TYPE &&
> > ppmu->get_mem_weight)
> > - ppmu->get_mem_weight(&data.weight.full);
> > + ppmu->get_mem_weight(&data.weight.full, event->attr.sample_type);
> > if (perf_event_overflow(event, &data, regs))
> > power_pmu_stop(event, 0);
> > diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
> > index e4f577da33d8..5dcbdbd54598 100644
> > --- a/arch/powerpc/perf/isa207-common.c
> > +++ b/arch/powerpc/perf/isa207-common.c
> > @@ -284,8 +284,10 @@ void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
> > }
> > }
> > -void isa207_get_mem_weight(u64 *weight)
> > +void isa207_get_mem_weight(u64 *weight, u64 type)
> > {
> > + union perf_sample_weight *weight_fields;
> > + u64 weight_lat;
> > u64 mmcra = mfspr(SPRN_MMCRA);
> > u64 exp = MMCRA_THR_CTR_EXP(mmcra);
> > u64 mantissa = MMCRA_THR_CTR_MANT(mmcra);
> > @@ -296,9 +298,30 @@ void isa207_get_mem_weight(u64 *weight)
> > mantissa = P10_MMCRA_THR_CTR_MANT(mmcra);
> > if (val == 0 || val == 7)
> > - *weight = 0;
> > + weight_lat = 0;
> > else
> > - *weight = mantissa << (2 * exp);
> > + weight_lat = mantissa << (2 * exp);
> > +
> > + /*
> > + * Use 64 bit weight field (full) if sample type is
> > + * WEIGHT.
> > + *
> > + * if sample type is WEIGHT_STRUCT:
> > + * - store memory latency in the lower 32 bits.
> > + * - For ISA v3.1, use remaining two 16 bit fields of
> > + * perf_sample_weight to store cycle counter values
> > + * from sier2.
> > + */
> > + weight_fields = (union perf_sample_weight *)weight;
> > + if (type & PERF_SAMPLE_WEIGHT)
> > + weight_fields->full = weight_lat;
> > + else {
> > + weight_fields->var1_dw = (u32)weight_lat;
> > + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
> > + weight_fields->var2_w = P10_SIER2_FINISH_CYC(mfspr(SPRN_SIER2));
> > + weight_fields->var3_w = P10_SIER2_DISPATCH_CYC(mfspr(SPRN_SIER2));
> > + }
> > + }
> > }
> > int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp, u64 event_config1)
> > diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h
> > index 1af0e8c97ac7..fc30d43c4d0c 100644
> > --- a/arch/powerpc/perf/isa207-common.h
> > +++ b/arch/powerpc/perf/isa207-common.h
> > @@ -265,6 +265,10 @@
> > #define ISA207_SIER_DATA_SRC_SHIFT 53
> > #define ISA207_SIER_DATA_SRC_MASK (0x7ull << ISA207_SIER_DATA_SRC_SHIFT)
> > +/* Bits in SIER2/SIER3 for Power10 */
> > +#define P10_SIER2_FINISH_CYC(sier2) (((sier2) >> (63 - 37)) & 0x7fful)
> > +#define P10_SIER2_DISPATCH_CYC(sier2) (((sier2) >> (63 - 13)) & 0x7fful)
> > +
> > #define P(a, b) PERF_MEM_S(a, b)
> > #define PH(a, b) (P(LVL, HIT) | P(a, b))
> > #define PM(a, b) (P(LVL, MISS) | P(a, b))
> > @@ -278,6 +282,6 @@ int isa207_get_alternatives(u64 event, u64 alt[], int size, unsigned int flags,
> > const unsigned int ev_alt[][MAX_ALT]);
> > void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
> > struct pt_regs *regs);
> > -void isa207_get_mem_weight(u64 *weight);
> > +void isa207_get_mem_weight(u64 *weight, u64 type);
> > #endif
--
- Arnaldo
^ permalink raw reply
* Re: [PATCH V2 1/5] powerpc/perf: Expose processor pipeline stage cycles using PERF_SAMPLE_WEIGHT_STRUCT
From: Arnaldo Carvalho de Melo @ 2021-03-25 13:06 UTC (permalink / raw)
To: Madhavan Srinivasan
Cc: ravi.bangoria, Athira Rajeev, peterz, linux-kernel,
linux-perf-users, jolsa, kjain, linuxppc-dev, kan.liang
In-Reply-To: <d7dd633b-e28a-155a-a8e2-0e5a83b4eead@linux.ibm.com>
Em Wed, Mar 24, 2021 at 10:05:23AM +0530, Madhavan Srinivasan escreveu:
>
> On 3/22/21 8:27 PM, Athira Rajeev wrote:
> > Performance Monitoring Unit (PMU) registers in powerpc provides
> > information on cycles elapsed between different stages in the
> > pipeline. This can be used for application tuning. On ISA v3.1
> > platform, this information is exposed by sampling registers.
> > Patch adds kernel support to capture two of the cycle counters
> > as part of perf sample using the sample type:
> > PERF_SAMPLE_WEIGHT_STRUCT.
> >
> > The power PMU function 'get_mem_weight' currently uses 64 bit weight
> > field of perf_sample_data to capture memory latency. But following the
> > introduction of PERF_SAMPLE_WEIGHT_TYPE, weight field could contain
> > 64-bit or 32-bit value depending on the architexture support for
> > PERF_SAMPLE_WEIGHT_STRUCT. Patches uses WEIGHT_STRUCT to expose the
> > pipeline stage cycles info. Hence update the ppmu functions to work for
> > 64-bit and 32-bit weight values.
> >
> > If the sample type is PERF_SAMPLE_WEIGHT, use the 64-bit weight field.
> > if the sample type is PERF_SAMPLE_WEIGHT_STRUCT, memory subsystem
> > latency is stored in the low 32bits of perf_sample_weight structure.
> > Also for CPU_FTR_ARCH_31, capture the two cycle counter information in
> > two 16 bit fields of perf_sample_weight structure.
>
> Changes looks fine to me.
You mean just the kernel part or can I add your Reviewed-by to all the
patchset?
> Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
>
>
> > Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/include/asm/perf_event_server.h | 2 +-
> > arch/powerpc/perf/core-book3s.c | 4 ++--
> > arch/powerpc/perf/isa207-common.c | 29 +++++++++++++++++++++++++---
> > arch/powerpc/perf/isa207-common.h | 6 +++++-
> > 4 files changed, 34 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
> > index 00e7e671bb4b..112cf092d7b3 100644
> > --- a/arch/powerpc/include/asm/perf_event_server.h
> > +++ b/arch/powerpc/include/asm/perf_event_server.h
> > @@ -43,7 +43,7 @@ struct power_pmu {
> > u64 alt[]);
> > void (*get_mem_data_src)(union perf_mem_data_src *dsrc,
> > u32 flags, struct pt_regs *regs);
> > - void (*get_mem_weight)(u64 *weight);
> > + void (*get_mem_weight)(u64 *weight, u64 type);
> > unsigned long group_constraint_mask;
> > unsigned long group_constraint_val;
> > u64 (*bhrb_filter_map)(u64 branch_sample_type);
> > diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> > index 766f064f00fb..6936763246bd 100644
> > --- a/arch/powerpc/perf/core-book3s.c
> > +++ b/arch/powerpc/perf/core-book3s.c
> > @@ -2206,9 +2206,9 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
> > ppmu->get_mem_data_src)
> > ppmu->get_mem_data_src(&data.data_src, ppmu->flags, regs);
> > - if (event->attr.sample_type & PERF_SAMPLE_WEIGHT &&
> > + if (event->attr.sample_type & PERF_SAMPLE_WEIGHT_TYPE &&
> > ppmu->get_mem_weight)
> > - ppmu->get_mem_weight(&data.weight.full);
> > + ppmu->get_mem_weight(&data.weight.full, event->attr.sample_type);
> > if (perf_event_overflow(event, &data, regs))
> > power_pmu_stop(event, 0);
> > diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
> > index e4f577da33d8..5dcbdbd54598 100644
> > --- a/arch/powerpc/perf/isa207-common.c
> > +++ b/arch/powerpc/perf/isa207-common.c
> > @@ -284,8 +284,10 @@ void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
> > }
> > }
> > -void isa207_get_mem_weight(u64 *weight)
> > +void isa207_get_mem_weight(u64 *weight, u64 type)
> > {
> > + union perf_sample_weight *weight_fields;
> > + u64 weight_lat;
> > u64 mmcra = mfspr(SPRN_MMCRA);
> > u64 exp = MMCRA_THR_CTR_EXP(mmcra);
> > u64 mantissa = MMCRA_THR_CTR_MANT(mmcra);
> > @@ -296,9 +298,30 @@ void isa207_get_mem_weight(u64 *weight)
> > mantissa = P10_MMCRA_THR_CTR_MANT(mmcra);
> > if (val == 0 || val == 7)
> > - *weight = 0;
> > + weight_lat = 0;
> > else
> > - *weight = mantissa << (2 * exp);
> > + weight_lat = mantissa << (2 * exp);
> > +
> > + /*
> > + * Use 64 bit weight field (full) if sample type is
> > + * WEIGHT.
> > + *
> > + * if sample type is WEIGHT_STRUCT:
> > + * - store memory latency in the lower 32 bits.
> > + * - For ISA v3.1, use remaining two 16 bit fields of
> > + * perf_sample_weight to store cycle counter values
> > + * from sier2.
> > + */
> > + weight_fields = (union perf_sample_weight *)weight;
> > + if (type & PERF_SAMPLE_WEIGHT)
> > + weight_fields->full = weight_lat;
> > + else {
> > + weight_fields->var1_dw = (u32)weight_lat;
> > + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
> > + weight_fields->var2_w = P10_SIER2_FINISH_CYC(mfspr(SPRN_SIER2));
> > + weight_fields->var3_w = P10_SIER2_DISPATCH_CYC(mfspr(SPRN_SIER2));
> > + }
> > + }
> > }
> > int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp, u64 event_config1)
> > diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h
> > index 1af0e8c97ac7..fc30d43c4d0c 100644
> > --- a/arch/powerpc/perf/isa207-common.h
> > +++ b/arch/powerpc/perf/isa207-common.h
> > @@ -265,6 +265,10 @@
> > #define ISA207_SIER_DATA_SRC_SHIFT 53
> > #define ISA207_SIER_DATA_SRC_MASK (0x7ull << ISA207_SIER_DATA_SRC_SHIFT)
> > +/* Bits in SIER2/SIER3 for Power10 */
> > +#define P10_SIER2_FINISH_CYC(sier2) (((sier2) >> (63 - 37)) & 0x7fful)
> > +#define P10_SIER2_DISPATCH_CYC(sier2) (((sier2) >> (63 - 13)) & 0x7fful)
> > +
> > #define P(a, b) PERF_MEM_S(a, b)
> > #define PH(a, b) (P(LVL, HIT) | P(a, b))
> > #define PM(a, b) (P(LVL, MISS) | P(a, b))
> > @@ -278,6 +282,6 @@ int isa207_get_alternatives(u64 event, u64 alt[], int size, unsigned int flags,
> > const unsigned int ev_alt[][MAX_ALT]);
> > void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
> > struct pt_regs *regs);
> > -void isa207_get_mem_weight(u64 *weight);
> > +void isa207_get_mem_weight(u64 *weight, u64 type);
> > #endif
--
- Arnaldo
^ permalink raw reply
* Re: [PATCH v2 3/7] powerpc: convert config files to generic cmdline
From: Rob Herring @ 2021-03-25 13:45 UTC (permalink / raw)
To: Christophe Leroy
Cc: Daniel Gimpelevich, linuxppc-dev, X86 ML, open list:MIPS,
linux-kernel@vger.kernel.org, Paul Mackerras, xe-linux-external,
Andrew Morton, Will Deacon, Daniel Walker
In-Reply-To: <20fd7d44-8c39-48bc-25c3-990be9d9d911@csgroup.eu>
On Thu, Mar 25, 2021 at 6:06 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 24/03/2021 à 18:32, Rob Herring a écrit :
> > On Wed, Mar 24, 2021 at 11:01 AM Christophe Leroy
> > <christophe.leroy@csgroup.eu> wrote:
> >>
> >>
> >>
> >> Le 09/03/2021 à 22:29, Daniel Walker a écrit :
> >>> On Tue, Mar 09, 2021 at 08:47:09AM +0100, Christophe Leroy wrote:
> >>>>
> >>>>
> >>>> Le 09/03/2021 à 01:02, Daniel Walker a écrit :
> >>>>> This is a scripted mass convert of the config files to use
> >>>>> the new generic cmdline. There is a bit of a trim effect here.
> >>>>> It would seems that some of the config haven't been trimmed in
> >>>>> a while.
> >>>>
> >>>> If you do that in a separate patch, you loose bisectability.
> >>>>
> >>>> I think it would have been better to do things in a different way, more or less like I did in my series:
> >>>> 1/ Provide GENERIC cmdline at the same functionnality level as what is
> >>>> spread in the different architectures
> >>>> 2/ Convert architectures to the generic with least churn.
> >>>> 3/ Add new features to the generic
> >>>
> >>> You have to have the churn eventually, no matter how you do it. The only way you
> >>> don't have churn is if you never upgrade the feature set.
> >>>
> >>>
> >>>>>
> >>>>> The bash script used to convert is as follows,
> >>>>>
> >>>>> if [[ -z "$1" || -z "$2" ]]; then
> >>>>> echo "Two arguments are needed."
> >>>>> exit 1
> >>>>> fi
> >>>>> mkdir $1
> >>>>> cp $2 $1/.config
> >>>>> sed -i 's/CONFIG_CMDLINE=/CONFIG_CMDLINE_BOOL=y\nCONFIG_CMDLINE_PREPEND=/g' $1/.config
> >>>>
> >>>> This is not correct.
> >>>>
> >>>> By default, on powerpc the provided command line is used only if the bootloader doesn't provide one.
> >>>>
> >>>> Otherwise:
> >>>> - the builtin command line is appended to the one provided by the bootloader
> >>>> if CONFIG_CMDLINE_EXTEND is selected
> >>>> - the builtin command line replaces to the one provided by the bootloader if
> >>>> CONFIG_CMDLINE_FORCE is selected
> >>>
> >>> I think my changes maintain most of this due to the override of
> >>> CONFIG_CMDLINE_PREPEND. This is an upgrade and the inflexibility in powerpc is
> >>> an example of why these changes were created in the first place.
> >>
> >> "inflexibility in powerpc" : Can you elaborate ?
> >>
> >>>
> >>> For example , say the default command line is "root=/dev/issblk0" from iss476
> >>> platform. And the bootloader adds "root=/dev/sda1"
> >>>
> >>> The result is <prepend><bootloader><append>.
> >>
> >>
> >> I'm still having hard time understanding the benefit of having both <prepend> and <append>.
> >> Could you please provide a complete exemple from real life, ie what exactly the problem is and what
> >> it solves ?
> >
> > It doesn't matter. We already have both cases and 'extend' has meant either one.
> >
> > What someone wants is policy and the kernel shouldn't be defining the policy.
> >
>
> Ok, so you agree we don't need to provide two CMDLINE, one to be appended and one to be prepended.
Well, I wasn't thinking about that part of it, but yes as long as no
arch currently needs that.
> Let's only provide once CMDLINE as of today, and ask the user to select whether he wants it appended
> or prepended or replacee. Then no need to change all existing config to rename CONFIG_CMDLINE into
> either of the new ones.
>
> That's the main difference between my series and Daniel's series. So I'll finish taking Will's
> comment into account and we'll send out a v3 soon.
Great.
Rob
^ permalink raw reply
* Re: [PATCH V2 1/5] powerpc/perf: Expose processor pipeline stage cycles using PERF_SAMPLE_WEIGHT_STRUCT
From: Peter Zijlstra @ 2021-03-25 14:38 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: ravi.bangoria, Athira Rajeev, linux-kernel, linux-perf-users,
Madhavan Srinivasan, jolsa, kjain, linuxppc-dev, kan.liang
In-Reply-To: <YFyJr+R24TlrMNrC@kernel.org>
On Thu, Mar 25, 2021 at 10:01:35AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 24, 2021 at 10:05:23AM +0530, Madhavan Srinivasan escreveu:
> >
> > On 3/22/21 8:27 PM, Athira Rajeev wrote:
> > > Performance Monitoring Unit (PMU) registers in powerpc provides
> > > information on cycles elapsed between different stages in the
> > > pipeline. This can be used for application tuning. On ISA v3.1
> > > platform, this information is exposed by sampling registers.
> > > Patch adds kernel support to capture two of the cycle counters
> > > as part of perf sample using the sample type:
> > > PERF_SAMPLE_WEIGHT_STRUCT.
> > >
> > > The power PMU function 'get_mem_weight' currently uses 64 bit weight
> > > field of perf_sample_data to capture memory latency. But following the
> > > introduction of PERF_SAMPLE_WEIGHT_TYPE, weight field could contain
> > > 64-bit or 32-bit value depending on the architexture support for
> > > PERF_SAMPLE_WEIGHT_STRUCT. Patches uses WEIGHT_STRUCT to expose the
> > > pipeline stage cycles info. Hence update the ppmu functions to work for
> > > 64-bit and 32-bit weight values.
> > >
> > > If the sample type is PERF_SAMPLE_WEIGHT, use the 64-bit weight field.
> > > if the sample type is PERF_SAMPLE_WEIGHT_STRUCT, memory subsystem
> > > latency is stored in the low 32bits of perf_sample_weight structure.
> > > Also for CPU_FTR_ARCH_31, capture the two cycle counter information in
> > > two 16 bit fields of perf_sample_weight structure.
> >
> > Changes looks fine to me.
> >
> > Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
>
> So who will process the kernel bits? I'm merging the tooling parts,
I was sorta expecting these to go through the powerpc tree. Let me know
if you want them in tip/perf/core instead.
^ permalink raw reply
* Re: [PATCH V2 1/5] powerpc/perf: Expose processor pipeline stage cycles using PERF_SAMPLE_WEIGHT_STRUCT
From: Arnaldo @ 2021-03-25 16:42 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo
Cc: ravi.bangoria, Athira Rajeev, linux-kernel, linux-perf-users,
Madhavan Srinivasan, jolsa, kjain, linuxppc-dev, kan.liang
In-Reply-To: <YFygSdFOT5B0DwRU@hirez.programming.kicks-ass.net>
On March 25, 2021 11:38:01 AM GMT-03:00, Peter Zijlstra <peterz@infradead.org> wrote:
>On Thu, Mar 25, 2021 at 10:01:35AM -0300, Arnaldo Carvalho de Melo
>wrote:.
>> > > Also for CPU_FTR_ARCH_31, capture the two cycle counter
>information in
>> > > two 16 bit fields of perf_sample_weight structure.
>> >
>> > Changes looks fine to me.
>> >
>> > Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
>>
>> So who will process the kernel bits? I'm merging the tooling parts,
>
>I was sorta expecting these to go through the powerpc tree. Let me know
>if you want them in tip/perf/core instead.
Shouldn't matter by which tree it gets upstream, as long as it gets picked :-)
- Arnaldo
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: VDSO ELF header
From: Christophe Leroy @ 2021-03-25 16:46 UTC (permalink / raw)
To: Laurent Dufour, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <c45ae4f8-1cbc-c687-b6a2-9a431fafc85c@linux.ibm.com>
Hi Laurent
Le 25/03/2021 à 17:11, Laurent Dufour a écrit :
> Hi Christophe,
>
> Since v5.11 and the changes you made to the VDSO code, it no more exposing the ELF header at the
> beginning of the VDSO mapping in user space.
>
> This is confusing CRIU which is checking for this ELF header cookie
> (https://github.com/checkpoint-restore/criu/issues/1417).
How does it do on other architectures ?
>
> I'm not an expert in loading and ELF part and reading the change you made, I can't identify how this
> could work now as I'm expecting the loader to need that ELF header to do the relocation.
I think the loader is able to find it at the expected place.
>
> From my investigation it seems that the first bytes of the VDSO area are now the vdso_arch_data.
>
> Is the ELF header put somewhere else?
> How could the loader process the VDSO without that ELF header?
>
Like most other architectures, we now have the data section as first page and the text section
follows. So you will likely find the elf header on the second page.
Done in this commit: https://github.com/linuxppc/linux/commit/511157ab641eb6bedd00d62673388e78a4f871cf
Christophe
^ permalink raw reply
* Re: VDSO ELF header
From: Laurent Dufour @ 2021-03-25 16:56 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <9366c258-127f-f105-abd1-6baa9a6745c5@csgroup.eu>
Le 25/03/2021 à 17:46, Christophe Leroy a écrit :
> Hi Laurent
>
> Le 25/03/2021 à 17:11, Laurent Dufour a écrit :
>> Hi Christophe,
>>
>> Since v5.11 and the changes you made to the VDSO code, it no more exposing the
>> ELF header at the beginning of the VDSO mapping in user space.
>>
>> This is confusing CRIU which is checking for this ELF header cookie
>> (https://github.com/checkpoint-restore/criu/issues/1417).
>
> How does it do on other architectures ?
Good question, I'll double check the CRIU code.
>
>>
>> I'm not an expert in loading and ELF part and reading the change you made, I
>> can't identify how this could work now as I'm expecting the loader to need
>> that ELF header to do the relocation.
>
> I think the loader is able to find it at the expected place.
Actually, it seems the loader relies on the AUX vector AT_SYSINFO_EHDR. I guess
CRIU should do the same.
>>
>> From my investigation it seems that the first bytes of the VDSO area are now
>> the vdso_arch_data.
>>
>> Is the ELF header put somewhere else?
>> How could the loader process the VDSO without that ELF header?
>>
>
> Like most other architectures, we now have the data section as first page and
> the text section follows. So you will likely find the elf header on the second
> page.
>
> Done in this commit:
> https://github.com/linuxppc/linux/commit/511157ab641eb6bedd00d62673388e78a4f871cf
I'll double check on x86, but anyway, I think CRIU should rely on
AT_SYSINFO_EHDR and not assume that the ELF header is at the beginning of VDSO
mapping.
Thanks for your help.
Laurent.
^ permalink raw reply
* Re: [PATCH 1/2] ASoC: fsl-asoc-card: Add support for WM8958 codec
From: Mark Brown @ 2021-03-25 17:37 UTC (permalink / raw)
To: tiwai, perex, festevam, Xiubo.Lee, devicetree, linuxppc-dev,
nicoleotsuka, linux-kernel, timur, alsa-devel, Shengjiu Wang,
lgirdwood, robh+dt
Cc: Mark Brown
In-Reply-To: <1615986303-27959-1-git-send-email-shengjiu.wang@nxp.com>
On Wed, 17 Mar 2021 21:05:02 +0800, Shengjiu Wang wrote:
> WM8958 codec is used on some i.MX based platform.
> So add it support in this generic driver.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ASoC: fsl-asoc-card: Add support for WM8958 codec
commit: efd0b1660829a987354cea6a446179c7ac7cd0e6
[2/2] ASoC: bindings: fsl-asoc-card: add compatible string for WM8958 codec
commit: df8077c6fe64fe98c1b1c1f9ecf84afc773e726f
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 v5 0/6] Add audio driver base on rpmsg on i.MX platform
From: Mark Brown @ 2021-03-25 17:37 UTC (permalink / raw)
To: tiwai, perex, festevam, Xiubo.Lee, devicetree, nicoleotsuka,
linuxppc-dev, linux-kernel, timur, alsa-devel, Shengjiu Wang,
lgirdwood, robh+dt
Cc: Mark Brown
In-Reply-To: <1615516725-4975-1-git-send-email-shengjiu.wang@nxp.com>
On Fri, 12 Mar 2021 10:38:39 +0800, Shengjiu Wang wrote:
> On Asymmetric multiprocessor, there is Cortex-A core and Cortex-M core,
> Linux is running on A core, RTOS is running on M core.
> The audio hardware device can be controlled by Cortex-M device,
> So audio playback/capture can be handled by M core.
>
> Rpmsg is the interface for sending and receiving msg to and from M
> core, that we can create a virtual sound on Cortex-A core side.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/6] ASoC: soc-component: Add snd_soc_pcm_component_ack
commit: 8bdfc0455e3a59e2c1207a56be22e910fae0e0d5
[2/6] ASoC: fsl_rpmsg: Add CPU DAI driver for audio base on rpmsg
commit: b73d9e6225e86492f6a901223a34ecfa7b55c178
[3/6] ASoC: dt-bindings: fsl_rpmsg: Add binding doc for rpmsg audio device
commit: 49c6bf62498344fa8f8af2314231f3eb37e0e150
[4/6] ASoC: imx-audio-rpmsg: Add rpmsg_driver for audio channel
commit: 1935050de0b6c6c961e9de51d5b5d05642f861f1
[5/6] ASoC: imx-pcm-rpmsg: Add platform driver for audio base on rpmsg
commit: 3c00eceb2a5391ed1ca6703b71cad35ab8cd4352
[6/6] ASoC: imx-rpmsg: Add machine driver for audio base on rpmsg
commit: 39f8405c3e502e7b9d0533fa0b0bfe715b3e89c1
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] docs: powerpc: Fix a typo
From: Jonathan Corbet @ 2021-03-25 17:48 UTC (permalink / raw)
To: Bhaskar Chowdhury, mpe, benh, paulus, linuxppc-dev, linux-doc,
linux-kernel
Cc: rdunlap, Bhaskar Chowdhury
In-Reply-To: <20210322062237.2971314-1-unixbhaskar@gmail.com>
Bhaskar Chowdhury <unixbhaskar@gmail.com> writes:
> s/struture/structure/
>
> Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
> ---
> Documentation/powerpc/firmware-assisted-dump.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/powerpc/firmware-assisted-dump.rst b/Documentation/powerpc/firmware-assisted-dump.rst
> index 20ea8cdee0aa..6c0ae070ba67 100644
> --- a/Documentation/powerpc/firmware-assisted-dump.rst
> +++ b/Documentation/powerpc/firmware-assisted-dump.rst
> @@ -171,7 +171,7 @@ that were present in CMA region::
> (meta area) |
> |
> |
> - Metadata: This area holds a metadata struture whose
> + Metadata: This area holds a metadata structure whose
> address is registered with f/w and retrieved in the
> second kernel after crash, on platforms that support
Applied, thanks.
jon
^ permalink raw reply
* Re: VDSO ELF header
From: Laurent Dufour @ 2021-03-25 19:02 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev
In-Reply-To: <5b03e966-2cfd-5f0c-c48d-dea5e0001833@linux.ibm.com>
Le 25/03/2021 à 17:56, Laurent Dufour a écrit :
> Le 25/03/2021 à 17:46, Christophe Leroy a écrit :
>> Hi Laurent
>>
>> Le 25/03/2021 à 17:11, Laurent Dufour a écrit :
>>> Hi Christophe,
>>>
>>> Since v5.11 and the changes you made to the VDSO code, it no more exposing
>>> the ELF header at the beginning of the VDSO mapping in user space.
>>>
>>> This is confusing CRIU which is checking for this ELF header cookie
>>> (https://github.com/checkpoint-restore/criu/issues/1417).
>>
>> How does it do on other architectures ?
>
> Good question, I'll double check the CRIU code.
On x86, there are 2 VDSO entries:
7ffff7fcb000-7ffff7fce000 r--p 00000000 00:00 0 [vvar]
7ffff7fce000-7ffff7fcf000 r-xp 00000000 00:00 0 [vdso]
And the VDSO is starting with the ELF header.
>
>>
>>>
>>> I'm not an expert in loading and ELF part and reading the change you made, I
>>> can't identify how this could work now as I'm expecting the loader to need
>>> that ELF header to do the relocation.
>>
>> I think the loader is able to find it at the expected place.
>
> Actually, it seems the loader relies on the AUX vector AT_SYSINFO_EHDR. I guess
> CRIU should do the same.
>
>>>
>>> From my investigation it seems that the first bytes of the VDSO area are now
>>> the vdso_arch_data.
>>>
>>> Is the ELF header put somewhere else?
>>> How could the loader process the VDSO without that ELF header?
>>>
>>
>> Like most other architectures, we now have the data section as first page and
>> the text section follows. So you will likely find the elf header on the second
>> page.
I'm wondering if the data section you're refering to is the vvar section I can
see on x86.
>>
>> Done in this commit:
>> https://github.com/linuxppc/linux/commit/511157ab641eb6bedd00d62673388e78a4f871cf
>
> I'll double check on x86, but anyway, I think CRIU should rely on
> AT_SYSINFO_EHDR and not assume that the ELF header is at the beginning of VDSO
> mapping.
>
> Thanks for your help.
> Laurent.
>
^ permalink raw reply
* Re: [PATCH v2 6/7] cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation
From: Will Deacon @ 2021-03-25 19:32 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, robh, daniel, devicetree, linux-kernel,
Paul Mackerras, linuxppc-dev, danielwa
In-Reply-To: <8db81511-3f28-4ef1-5e66-188cf7cafad1@csgroup.eu>
On Thu, Mar 25, 2021 at 12:18:38PM +0100, Christophe Leroy wrote:
>
>
> Le 03/03/2021 à 18:57, Will Deacon a écrit :
> > On Tue, Mar 02, 2021 at 05:25:22PM +0000, Christophe Leroy wrote:
> > > Most architectures have similar boot command line manipulation
> > > options. This patchs adds the definition in init/Kconfig, gated by
> > > CONFIG_HAVE_CMDLINE that the architectures can select to use them.
> > >
> > > In order to use this, a few architectures will have to change their
> > > CONFIG options:
> > > - riscv has to replace CMDLINE_FALLBACK by CMDLINE_FROM_BOOTLOADER
> > > - architectures using CONFIG_CMDLINE_OVERRIDE or
> > > CONFIG_CMDLINE_OVERWRITE have to replace them by CONFIG_CMDLINE_FORCE.
> > >
> > > Architectures also have to define CONFIG_DEFAULT_CMDLINE.
> > >
> > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > > ---
> > > init/Kconfig | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 56 insertions(+)
> > >
> > > diff --git a/init/Kconfig b/init/Kconfig
> > > index 22946fe5ded9..a0f2ad9467df 100644
> > > --- a/init/Kconfig
> > > +++ b/init/Kconfig
> > > @@ -117,6 +117,62 @@ config INIT_ENV_ARG_LIMIT
> > > Maximum of each of the number of arguments and environment
> > > variables passed to init from the kernel command line.
> > > +config HAVE_CMDLINE
> > > + bool
> > > +
> > > +config CMDLINE_BOOL
> > > + bool "Default bootloader kernel arguments"
> > > + depends on HAVE_CMDLINE
> > > + help
> > > + On some platforms, there is currently no way for the boot loader to
> > > + pass arguments to the kernel. For these platforms, you can supply
> > > + some command-line options at build time by entering them here. In
> > > + most cases you will need to specify the root device here.
> >
> > Why is this needed as well as CMDLINE_FROM_BOOTLOADER? IIUC, the latter
> > will use CONFIG_CMDLINE if it fails to get anything from the bootloader,
> > which sounds like the same scenario.
> >
> > > +config CMDLINE
> > > + string "Initial kernel command string"
> >
> > s/Initial/Default
> >
> > which is then consistent with the rest of the text here.
> >
> > > + depends on CMDLINE_BOOL
> >
> > Ah, so this is a bit different and I don't think lines-up with the
> > CMDLINE_BOOL help text.
> >
> > > + default DEFAULT_CMDLINE
> > > + help
> > > + On some platforms, there is currently no way for the boot loader to
> > > + pass arguments to the kernel. For these platforms, you can supply
> > > + some command-line options at build time by entering them here. In
> > > + most cases you will need to specify the root device here.
> >
> > (same stale text)
> >
> > > +choice
> > > + prompt "Kernel command line type" if CMDLINE != ""
> > > + default CMDLINE_FROM_BOOTLOADER
> > > + help
> > > + Selects the way you want to use the default kernel arguments.
> >
> > How about:
> >
> > "Determines how the default kernel arguments are combined with any
> > arguments passed by the bootloader"
> >
> > > +config CMDLINE_FROM_BOOTLOADER
> > > + bool "Use bootloader kernel arguments if available"
> > > + help
> > > + Uses the command-line options passed by the boot loader. If
> > > + the boot loader doesn't provide any, the default kernel command
> > > + string provided in CMDLINE will be used.
> > > +
> > > +config CMDLINE_EXTEND
> >
> > Can we rename this to CMDLINE_APPEND, please? There is code in the tree
> > which disagrees about what CMDLINE_EXTEND means, so that will need be
> > to be updated to be consistent (e.g. the EFI stub parsing order). Having
> > the generic option with a different name means we won't accidentally end
> > up with the same inconsistent behaviours.
>
> Argh, yes. Seems like the problem is even larger than that IIUC:
>
> - For ARM it means to append the bootloader arguments to the CONFIG_CMDLINE
> - For Powerpc it means to append the CONFIG_CMDLINE to the bootloader arguments
> - For SH it means to append the CONFIG_CMDLINE to the bootloader arguments
> - For EFI it means to append the bootloader arguments to the CONFIG_CMDLINE
> - For OF it means to append the CONFIG_CMDLINE to the bootloader arguments
>
> So what happens on ARM for instance when it selects CONFIG_OF for instance ?
I think ARM gets different behaviour depending on whether it uses ATAGs or
FDT.
> Or should we consider that EXTEND means APPEND or PREPEND, no matter which ?
> Because EXTEND is for instance used for:
>
> config INITRAMFS_FORCE
> bool "Ignore the initramfs passed by the bootloader"
> depends on CMDLINE_EXTEND || CMDLINE_FORCE
Oh man, I didn't spot that one :(
I think I would make the generic options explicit: either APPEND or PREPEND.
Then architectures which choose to define CMDLINE_EXTEND in their Kconfigs
can select the generic option that matches their behaviour.
INITRAMFS_FORCE sounds like it should depend on APPEND (assuming that means
CONFIG_CMDLINE is appended to the bootloader arguments).
Will
^ permalink raw reply
* Re: [PATCH v2 3/7] powerpc: convert config files to generic cmdline
From: Daniel Walker @ 2021-03-25 19:56 UTC (permalink / raw)
To: Christophe Leroy
Cc: Rob Herring, Daniel Gimpelevich, linuxppc-dev, x86, linux-mips,
linux-kernel, Paul Mackerras, xe-linux-external, Andrew Morton,
Will Deacon
In-Reply-To: <e4899874-1684-fa1b-443e-f4e478e05e31@csgroup.eu>
On Wed, Mar 24, 2021 at 05:59:59PM +0100, Christophe Leroy wrote:
> > I think my changes maintain most of this due to the override of
> > CONFIG_CMDLINE_PREPEND. This is an upgrade and the inflexibility in powerpc is
> > an example of why these changes were created in the first place.
>
> "inflexibility in powerpc" : Can you elaborate ?
the prom environment.
> >
> > For example , say the default command line is "root=/dev/issblk0" from iss476
> > platform. And the bootloader adds "root=/dev/sda1"
> >
> > The result is <prepend><bootloader><append>.
>
>
> I'm still having hard time understanding the benefit of having both <prepend> and <append>.
> Could you please provide a complete exemple from real life, ie what exactly
> the problem is and what it solves ?
Say the boot loader of an old product is released with a command line of
"root=/dev/sda" and per the needs of the company or product the boot loader can
not be upgraded to change this command line. To change this behavior you would
need append or EXTEND.
Below I detail an example of PREPEND due to your list question.
> >
> > Then you have,
> >
> > root=/dev/issblk0 root=/dev/sda1
> >
> > and the bootloader has precedent over the default command line. So root= in the
> > above cases is defined by the bootloader.
A person could input a command line into a boot loader, and it would override
the PREPEND values.
Can you imagine you have a default command line which makes root=/dev/issblk0 ,
but that doesn't work for you testing purpose. So you input into the boot loader
root=/dev/sda1 , since you have the default input in the bootloader OVERRIDEABLE
you can do this without re-compiling and just input the single root= command
into the bootloader.
Daniel
^ permalink raw reply
* Re: [PATCH v2 3/7] powerpc: convert config files to generic cmdline
From: Daniel Walker @ 2021-03-25 19:59 UTC (permalink / raw)
To: Christophe Leroy
Cc: Rob Herring, Daniel Gimpelevich, linuxppc-dev, X86 ML,
open list:MIPS, linux-kernel@vger.kernel.org, Paul Mackerras,
xe-linux-external, Andrew Morton, Will Deacon
In-Reply-To: <20fd7d44-8c39-48bc-25c3-990be9d9d911@csgroup.eu>
On Thu, Mar 25, 2021 at 01:03:55PM +0100, Christophe Leroy wrote:
>
> Ok, so you agree we don't need to provide two CMDLINE, one to be appended and one to be prepended.
>
> Let's only provide once CMDLINE as of today, and ask the user to select
> whether he wants it appended or prepended or replacee. Then no need to
> change all existing config to rename CONFIG_CMDLINE into either of the new
> ones.
>
> That's the main difference between my series and Daniel's series. So I'll
> finish taking Will's comment into account and we'll send out a v3 soon.
It doesn't solve the needs of Cisco, I've stated many times your changes have
little value. Please stop submitting them.
Daniel
^ permalink raw reply
* Re: [PATCH v2 4/7] CMDLINE: powerpc: convert to generic builtin command line
From: Daniel Walker @ 2021-03-25 20:03 UTC (permalink / raw)
To: Christophe Leroy
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: <9c5b8e33-026e-c9d6-c267-a5dd4a2b999c@csgroup.eu>
On Wed, Mar 24, 2021 at 04:31:35PM +0100, Christophe Leroy wrote:
>
>
> Le 09/03/2021 à 22:40, Daniel Walker a écrit :
> > On Tue, Mar 09, 2021 at 08:56:47AM +0100, Christophe Leroy wrote:
> > >
> > >
> > > Le 09/03/2021 à 01:02, Daniel Walker a écrit :
> > > > This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
> > > > option.
> > > >
> > > > Cc: xe-linux-external@cisco.com
> > > > Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
> > > > Signed-off-by: Ruslan Bilovol <rbilovol@cisco.com>
> > > > Signed-off-by: Daniel Walker <danielwa@cisco.com>
> > > > ---
> > > > arch/powerpc/Kconfig | 37 +--------------------------------
> > > > arch/powerpc/kernel/prom.c | 1 +
> > > > arch/powerpc/kernel/prom_init.c | 35 ++++++++++++++++++-------------
> > > > 3 files changed, 23 insertions(+), 50 deletions(-)
> > > >
> > > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > > > index 107bb4319e0e..276b06d5c961 100644
> > > > --- a/arch/powerpc/Kconfig
> > > > +++ b/arch/powerpc/Kconfig
> > > > @@ -167,6 +167,7 @@ config PPC
> > > > select EDAC_SUPPORT
> > > > select GENERIC_ATOMIC64 if PPC32
> > > > select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> > > > + select GENERIC_CMDLINE
> > > > select GENERIC_CMOS_UPDATE
> > > > select GENERIC_CPU_AUTOPROBE
> > > > select GENERIC_CPU_VULNERABILITIES if PPC_BARRIER_NOSPEC
> > > > @@ -906,42 +907,6 @@ config PPC_DENORMALISATION
> > > > Add support for handling denormalisation of single precision
> > > > values. Useful for bare metal only. If unsure say Y here.
> > > > -config CMDLINE
> > > > - string "Initial kernel command string"
> > > > - default ""
> > > > - help
> > > > - On some platforms, there is currently no way for the boot loader to
> > > > - pass arguments to the kernel. For these platforms, you can supply
> > > > - some command-line options at build time by entering them here. In
> > > > - most cases you will need to specify the root device here.
> > > > -
> > > > -choice
> > > > - prompt "Kernel command line type" if CMDLINE != ""
> > > > - default CMDLINE_FROM_BOOTLOADER
> > > > -
> > > > -config CMDLINE_FROM_BOOTLOADER
> > > > - bool "Use bootloader kernel arguments if available"
> > > > - help
> > > > - Uses the command-line options passed by the boot loader. If
> > > > - the boot loader doesn't provide any, the default kernel command
> > > > - string provided in CMDLINE will be used.
>
> I can't see how the above is supported in the generic builtin.
>
> Taking into account that it is the default on powerpc, I'm having hardtime with that.
Hmm, so this ignores the built in changes. You just don't enable it, or you
don't add PREPEND or APPEND.
> Any feedback on the proposed changes I made on the 13th ? I know it is
> partly buggy but that was more for the principle. I can make clean working
> patch if it helps.
The reason I added it into the function parameters is because I can get free
type checking on the functions. If you use macro's then you don't know if the
function is compatible.
Daniel
^ permalink raw reply
* [PATCH] selftests: powerpc: unmark non-kernel-doc comments
From: Randy Dunlap @ 2021-03-25 20:08 UTC (permalink / raw)
To: linux-kernel; +Cc: Randy Dunlap, linuxppc-dev
Drop the 'beginning of kernel-doc' notation markers (/**)
in places that are not in kernel-doc format.
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
---
tools/testing/selftests/powerpc/tm/tm-trap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- linux-next-20210323.orig/tools/testing/selftests/powerpc/tm/tm-trap.c
+++ linux-next-20210323/tools/testing/selftests/powerpc/tm/tm-trap.c
@@ -66,7 +66,7 @@ void trap_signal_handler(int signo, sigi
/* Get thread endianness: extract bit LE from MSR */
thread_endianness = MSR_LE & ucp->uc_mcontext.gp_regs[PT_MSR];
- /***
+ /*
* Little-Endian Machine
*/
@@ -126,7 +126,7 @@ void trap_signal_handler(int signo, sigi
}
}
- /***
+ /*
* Big-Endian Machine
*/
^ permalink raw reply
* Re: [PATCH v2 05/15] powerpc/uaccess: Move get_user_instr helpers in asm/inst.h
From: Daniel Axtens @ 2021-03-25 21:59 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <2c6e83581b4fa434aa7cf2fa7714c41e98f57007.1615398265.git.christophe.leroy@csgroup.eu>
Hi Christophe,
> Those helpers use get_user helpers but they don't participate
> in their implementation, so they do not belong to asm/uaccess.h
>
> Move them in asm/inst.h
Hmm, is asm/inst.h the right place for this?
asm/inst.h seems to be entirely concerned with the ppc_inst type:
converting things to and from ppc_inst, print ppc_inst as a string,
dealing with prefixed instructs, etc., etc. The only things currently
that look at memory are the probe_user_read_inst and
probe_kernel_read_inst prototypes...
Having said that, I'm not sure quite where else to put it, and none of
the other places in arch/powerpc/include that currently reference
ppc_inst seem any better...
If we do use asm/inst.h, I think maybe it makes sense to put the
code towards the end rather than at the start, as uses structs and calls
macros that are defined later on in the function.
Kind regards,
Daniel
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/inst.h | 34 ++++++++++++++++++++++++++++++
> arch/powerpc/include/asm/uaccess.h | 34 ------------------------------
> 2 files changed, 34 insertions(+), 34 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> index cc73c1267572..19e18af2fac9 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -4,6 +4,40 @@
>
> #include <asm/ppc-opcode.h>
>
> +#ifdef CONFIG_PPC64
> +
> +#define ___get_user_instr(gu_op, dest, ptr) \
> +({ \
> + long __gui_ret = 0; \
> + unsigned long __gui_ptr = (unsigned long)ptr; \
> + struct ppc_inst __gui_inst; \
> + unsigned int __prefix, __suffix; \
> + __gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr); \
> + if (__gui_ret == 0) { \
> + if ((__prefix >> 26) == OP_PREFIX) { \
> + __gui_ret = gu_op(__suffix, \
> + (unsigned int __user *)__gui_ptr + 1); \
> + __gui_inst = ppc_inst_prefix(__prefix, \
> + __suffix); \
> + } else { \
> + __gui_inst = ppc_inst(__prefix); \
> + } \
> + if (__gui_ret == 0) \
> + (dest) = __gui_inst; \
> + } \
> + __gui_ret; \
> +})
> +#else /* !CONFIG_PPC64 */
> +#define ___get_user_instr(gu_op, dest, ptr) \
> + gu_op((dest).val, (u32 __user *)(ptr))
> +#endif /* CONFIG_PPC64 */
> +
> +#define get_user_instr(x, ptr) \
> + ___get_user_instr(get_user, x, ptr)
> +
> +#define __get_user_instr(x, ptr) \
> + ___get_user_instr(__get_user, x, ptr)
> +
> /*
> * Instruction data type for POWER
> */
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 01aea0df4dd0..eaa828a6a419 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -53,40 +53,6 @@ static inline bool __access_ok(unsigned long addr, unsigned long size)
> #define __put_user(x, ptr) \
> __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
>
> -#ifdef CONFIG_PPC64
> -
> -#define ___get_user_instr(gu_op, dest, ptr) \
> -({ \
> - long __gui_ret = 0; \
> - unsigned long __gui_ptr = (unsigned long)ptr; \
> - struct ppc_inst __gui_inst; \
> - unsigned int __prefix, __suffix; \
> - __gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr); \
> - if (__gui_ret == 0) { \
> - if ((__prefix >> 26) == OP_PREFIX) { \
> - __gui_ret = gu_op(__suffix, \
> - (unsigned int __user *)__gui_ptr + 1); \
> - __gui_inst = ppc_inst_prefix(__prefix, \
> - __suffix); \
> - } else { \
> - __gui_inst = ppc_inst(__prefix); \
> - } \
> - if (__gui_ret == 0) \
> - (dest) = __gui_inst; \
> - } \
> - __gui_ret; \
> -})
> -#else /* !CONFIG_PPC64 */
> -#define ___get_user_instr(gu_op, dest, ptr) \
> - gu_op((dest).val, (u32 __user *)(ptr))
> -#endif /* CONFIG_PPC64 */
> -
> -#define get_user_instr(x, ptr) \
> - ___get_user_instr(get_user, x, ptr)
> -
> -#define __get_user_instr(x, ptr) \
> - ___get_user_instr(__get_user, x, ptr)
> -
> extern long __put_user_bad(void);
>
> #define __put_user_size(x, ptr, size, retval) \
> --
> 2.25.0
^ 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