* [PATCH] powerpc: kernel: Trivial spelling fixes throughout the file head_fsl_booke.S
From: Bhaskar Chowdhury @ 2021-03-15 3:37 UTC (permalink / raw)
To: mpe, benh, paulus, rppt, akpm, christophe.leroy, oss, npiggin,
unixbhaskar, linuxppc-dev, linux-kernel
Cc: rdunlap
s/virutal/virtual/
s/mismach/mismatch/
Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
---
As Randy pointed out I was changing the predefined macro name,so, reverted
or leave it alone.
Michael,sorry to run down a cold weave in your spine with my stupdity,this is
okay.
arch/powerpc/kernel/head_fsl_booke.S | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 3f4a40cccef5..a955403247f1 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -113,7 +113,7 @@ _ENTRY(_start);
1:
/*
- * We have the runtime (virutal) address of our base.
+ * We have the runtime (virtual) address of our base.
* We calculate our shift of offset from a 64M page.
* We could map the 64M page we belong to at PAGE_OFFSET and
* get going from there.
@@ -497,7 +497,7 @@ END_BTB_FLUSH_SECTION
#endif
#endif
- bne 2f /* Bail if permission/valid mismach */
+ bne 2f /* Bail if permission/valid mismatch */
/* Jump to common tlb load */
b finish_tlb_load
@@ -592,7 +592,7 @@ END_BTB_FLUSH_SECTION
#endif
#endif
- bne 2f /* Bail if permission mismach */
+ bne 2f /* Bail if permission mismatch */
/* Jump to common TLB load point */
b finish_tlb_load
--
2.30.2
^ permalink raw reply related
* [PATCH] powerpc/kexec: Don't use .machine ppc64 in trampoline_64.S
From: Michael Ellerman @ 2021-03-15 3:41 UTC (permalink / raw)
To: linuxppc-dev; +Cc: dja
The ".machine" directive allows changing the machine for which code is
being generated. It's equivalent to passing an -mcpu option on the
command line.
Although it can be useful, it's generally a bad idea because it adds
another way to influence code generation separate from the flags
passed via the build system. ie. if we need to build different pieces
of code with different flags we should do that via our Makefiles, not
using ".machine".
However as best as I can tell the ".machine" directive in
trampoline_64.S is not necessary at all.
It was added in commit 0d97631392c2 ("powerpc: Add purgatory for
kexec_file_load() implementation."), which created the file based on
the kexec-tools purgatory. It may be/have-been necessary in the
kexec-tools version, but we have a completely different build system,
and we already pass the desired CPU flags, eg:
gcc ... -m64 -Wl,-a64 -mabi=elfv2 -Wa,-maltivec -Wa,-mpower4 -Wa,-many
... arch/powerpc/purgatory/trampoline_64.S
So drop the ".machine" directive and rely on the assembler flags.
Reported-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/purgatory/trampoline_64.S | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/purgatory/trampoline_64.S b/arch/powerpc/purgatory/trampoline_64.S
index d956b8a35fd1..b35837c13852 100644
--- a/arch/powerpc/purgatory/trampoline_64.S
+++ b/arch/powerpc/purgatory/trampoline_64.S
@@ -12,7 +12,6 @@
#include <asm/asm-compat.h>
#include <asm/crashdump-ppc64.h>
- .machine ppc64
.balign 256
.globl purgatory_start
purgatory_start:
--
2.25.1
^ permalink raw reply related
* Re: [PATCH V2 1/2] powerpc/perf: Infrastructure to support checking of attr.config*
From: Madhavan Srinivasan @ 2021-03-15 4:42 UTC (permalink / raw)
To: Paul A. Clarke; +Cc: linuxppc-dev
In-Reply-To: <20210226140340.GA36821@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com>
On 2/26/21 7:33 PM, Paul A. Clarke wrote:
> Another drive-by review... just some minor nits, below...
>
> On Fri, Feb 26, 2021 at 12:20:24PM +0530, Madhavan Srinivasan wrote:
>> 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 specific value to bit field
> I'd reword to "some specific values for bit fields are reserved".
>
>> as reserved. For ex., MMCRA[61:62] is Randome Sampling Mode (SM)
> s/Randome/Random/
> This occurs here, and below, and in patch 2/2.
>
>> and value of 0b11 to this field is reserved.
> s/to/for/
>
>> Writing a non-zero values in these fields or writing invalid
>> value to bit fields will have unknown behaviours.
> Suggest: Writing non-zero or invalid values in these fields
> will have unknown behaviors. (or "behaviours" ;-)
>
> PC
Thanks for the review. Will fix it.
Maddy
>
>> 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 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
* Re: [PATCH V2 2/2] powerpc/perf: Add platform specific check_attr_config
From: Madhavan Srinivasan @ 2021-03-15 4:39 UTC (permalink / raw)
To: Alexey Kardashevskiy, mpe; +Cc: linuxppc-dev
In-Reply-To: <dde47591-db00-4c4f-ed24-a8ab5a7a4c6a@ozlabs.ru>
[-- Attachment #1: Type: text/plain, Size: 6757 bytes --]
On 3/10/21 6:46 PM, Alexey Kardashevskiy wrote:
>
>
> On 26/02/2021 17:50, Madhavan Srinivasan wrote:
>> 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 v1:
>> - No changes.
>>
>> arch/powerpc/perf/isa207-common.c | 41 +++++++++++++++++++++++++++++++
>> arch/powerpc/perf/isa207-common.h | 2 ++
>> arch/powerpc/perf/power10-pmu.c | 13 ++++++++++
>> arch/powerpc/perf/power9-pmu.c | 13 ++++++++++
>> 4 files changed, 69 insertions(+)
>>
>> diff --git a/arch/powerpc/perf/isa207-common.c
>> b/arch/powerpc/perf/isa207-common.c
>> index e4f577da33d8..b255799f5b51 100644
>> --- a/arch/powerpc/perf/isa207-common.c
>> +++ b/arch/powerpc/perf/isa207-common.c
>> @@ -694,3 +694,44 @@ int isa207_get_alternatives(u64 event, u64
>> alt[], int size, unsigned int flags,
>> return num_alt;
>> }
>> +
>> +int isa3_X_check_attr_config(struct perf_event *ev)
>
>
> "isa300" is used everywhere else to refer to ISA 3.00.
ok will rename if as isa3XX_check_attr_config then.
>
>
>> +{
>> + u64 val, sample_mode;
>> + u64 event = ev->attr.config;
>> +
>> + val = (event >> EVENT_SAMPLE_SHIFT) & EVENT_SAMPLE_MASK;
>
> I am not familiar with the code - "Raw event encoding for Power9" from
> arch/powerpc/perf/power9-pmu.c - where is this from? Is this how linux
> defines encoding or it is P9 UM or something?
mpe and pmu folks defined the encoding format for power8, and then it
was carried forward slightly modified for P9.
>
>> + sample_mode = val & 0x3;
>> +
>> + /*
>> + * MMCRA[61:62] is Randome Sampling Mode (SM).
>> + * value of 0b11 is reserved.
>> + */
>> + if (sample_mode == 0x3)
>> + return -1;
>> +
>> + /*
>> + * Check for all reserved value
>> + */
>> + switch (val) {
>> + case 0x5:
>> + case 0x9:
>> + case 0xD:
>> + case 0x19:
>> + case 0x1D:
>> + case 0x1A:
>> + case 0x1E:
>
>
> What spec did these numbers come from?
Thanks for asking. Yes, these are published as part of Performance
monitoring unit user guide.
https://wiki.raptorcs.com/w/images/6/6b/POWER9_PMU_UG_v12_28NOV2018_pub.pdf
Will add a comment about the source of these bits.
>
>> + return -1;
>> + }
>> +
>> + /*
>> + * MMCRA[48:51]/[52:55]) Threshold Start/Stop
>> + * Events Selection.
>> + * 0b11110000/0b00001111 is reserved.
>
> The mapping between the event and MMCRA is very unclear :) But there
> are more reserved values in MMCRA in PowerISA_public.v3.0B.pdf:
>
> ===
> 0000 Reserved
>
> Problem state access (SPR 770)
> 1000 - 1111 - ReservedPrivileged access (SPR 770 or 786)
> 1000 - 1111 - Implementation-dependent
> ===
>
> Do not you need to filter these too?
Most of these bits are not architected but one should refer user guide
spec which talks about each bit and supported values.
https://wiki.raptorcs.com/w/images/6/6b/POWER9_PMU_UG_v12_28NOV2018_pub.pdf
>
>> + */
>> + val = (event >> EVENT_THR_CTL_SHIFT) & EVENT_THR_CTL_MASK;
>> + if (((val & 0xF0) == 0xF0) || ((val & 0xF) == 0xF))
>> + return -1;
>
> Since the filters may differ for problem and privileged, may be make
> these check_attr_config() hooks return EINVAL or EPERM and pass it on
> in the caller? Not sure there is much value in it though.
Since these are reserved values, privilege state does not matter. Will
change it to return EINVAL.
Thanks for the review.
Maddy
>
>
>
>> +
>> + return 0;
>> +}
>> diff --git a/arch/powerpc/perf/isa207-common.h
>> b/arch/powerpc/perf/isa207-common.h
>> index 1af0e8c97ac7..ae8eaf05efd1 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 isa3_X_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..bc64354cab6a 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 || isa3_X_check_attr_config(ev))
>> + return -1;
>> +
>> + 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..b3b9b226d053 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 || isa3_X_check_attr_config(ev))
>> + return -1;
>> +
>> + 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)
>>
>
[-- Attachment #2: Type: text/html, Size: 11998 bytes --]
^ permalink raw reply
* [PATCH] powerpc/xive: use true and false for bool variable
From: Yang Li @ 2021-03-15 7:24 UTC (permalink / raw)
To: mpe; +Cc: Yang Li, paulus, linuxppc-dev, linux-kernel
fixed the following coccicheck:
./arch/powerpc/sysdev/xive/spapr.c:552:8-9: WARNING: return of 0/1 in
function 'xive_spapr_match' with return type bool
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
---
arch/powerpc/sysdev/xive/spapr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
index 01ccc078..f143b6f 100644
--- a/arch/powerpc/sysdev/xive/spapr.c
+++ b/arch/powerpc/sysdev/xive/spapr.c
@@ -549,7 +549,7 @@ static void xive_spapr_cleanup_queue(unsigned int cpu, struct xive_cpu *xc,
static bool xive_spapr_match(struct device_node *node)
{
/* Ignore cascaded controllers for the moment */
- return 1;
+ return true;
}
#ifdef CONFIG_SMP
--
1.8.3.1
^ permalink raw reply related
* [PATCH] powerpc/pci: fix warning comparing pointer to 0
From: Jiapeng Chong @ 2021-03-15 7:35 UTC (permalink / raw)
To: mpe; +Cc: Jiapeng Chong, paulus, linuxppc-dev, linux-kernel
Fix the following coccicheck warning:
./arch/powerpc/platforms/maple/pci.c:37:16-17: WARNING comparing pointer
to 0.
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
arch/powerpc/platforms/maple/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/maple/pci.c b/arch/powerpc/platforms/maple/pci.c
index a20b957..37875e4 100644
--- a/arch/powerpc/platforms/maple/pci.c
+++ b/arch/powerpc/platforms/maple/pci.c
@@ -34,7 +34,7 @@
static int __init fixup_one_level_bus_range(struct device_node *node, int higher)
{
- for (; node != 0;node = node->sibling) {
+ for (; node; node = node->sibling) {
const int *bus_range;
const unsigned int *class_code;
int len;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 2/4] tools/perf: Add dynamic headers for perf report columns
From: Athira Rajeev @ 2021-03-15 7:41 UTC (permalink / raw)
To: Jiri Olsa
Cc: ravi.bangoria, Madhavan Srinivasan, peterz, linux-kernel,
Arnaldo Carvalho de Melo, linux-perf-users, jolsa, kjain,
linuxppc-dev, kan.liang
In-Reply-To: <YEtlHzsJ4z19pB/M@krava>
> On 12-Mar-2021, at 6:27 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>
> 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
>
Thanks Jiri for the review.
I will include this change in next version.
Thanks
Athira
>> +{
>> + 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 03/10] powerpc/64e/interrupt: use new interrupt return
From: Christophe Leroy @ 2021-03-15 7:50 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Scott Wood
In-Reply-To: <20210315031716.3940350-4-npiggin@gmail.com>
Le 15/03/2021 à 04:17, Nicholas Piggin a écrit :
> Update the new C and asm interrupt return code to account for 64e
> specifics, switch over to use it.
>
> The now-unused old ret_from_except code, that was moved to 64e after the
> 64s conversion, is removed.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/asm-prototypes.h | 2 -
> arch/powerpc/kernel/entry_64.S | 9 +-
> arch/powerpc/kernel/exceptions-64e.S | 321 ++--------------------
> arch/powerpc/kernel/interrupt.c | 27 +-
> arch/powerpc/kernel/irq.c | 76 -----
> 5 files changed, 56 insertions(+), 379 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
> index 939f3c94c8f3..1c7b75834e04 100644
> --- a/arch/powerpc/include/asm/asm-prototypes.h
> +++ b/arch/powerpc/include/asm/asm-prototypes.h
> @@ -77,8 +77,6 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
> u32 len_high, u32 len_low);
> long sys_switch_endian(void);
> -notrace unsigned int __check_irq_replay(void);
> -void notrace restore_interrupts(void);
>
> /* prom_init (OpenFirmware) */
> unsigned long __init prom_init(unsigned long r3, unsigned long r4,
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 853534b2ae2e..555b3d0a3f38 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -632,7 +632,6 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
> addi r1,r1,SWITCH_FRAME_SIZE
> blr
>
> -#ifdef CONFIG_PPC_BOOK3S
> /*
> * If MSR EE/RI was never enabled, IRQs not reconciled, NVGPRs not
> * touched, no exit work created, then this can be used.
> @@ -644,6 +643,7 @@ _ASM_NOKPROBE_SYMBOL(fast_interrupt_return)
> kuap_check_amr r3, r4
> ld r5,_MSR(r1)
> andi. r0,r5,MSR_PR
> +#ifdef CONFIG_PPC_BOOK3S
> bne .Lfast_user_interrupt_return_amr
> kuap_kernel_restore r3, r4
> andi. r0,r5,MSR_RI
> @@ -652,6 +652,10 @@ _ASM_NOKPROBE_SYMBOL(fast_interrupt_return)
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl unrecoverable_exception
> b . /* should not get here */
> +#else
> + bne .Lfast_user_interrupt_return
> + b .Lfast_kernel_interrupt_return
> +#endif
>
> .balign IFETCH_ALIGN_BYTES
> .globl interrupt_return
> @@ -665,8 +669,10 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return)
> cmpdi r3,0
> bne- .Lrestore_nvgprs
>
> +#ifdef CONFIG_PPC_BOOK3S
> .Lfast_user_interrupt_return_amr:
> kuap_user_restore r3, r4
> +#endif
> .Lfast_user_interrupt_return:
> ld r11,_NIP(r1)
> ld r12,_MSR(r1)
> @@ -775,7 +781,6 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
>
> RFI_TO_KERNEL
> b . /* prevent speculative execution */
> -#endif /* CONFIG_PPC_BOOK3S */
>
> #ifdef CONFIG_PPC_RTAS
> /*
> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
> index da78eb6ab92f..1bb4e9b37748 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -139,7 +139,8 @@ ret_from_level_except:
> ld r3,_MSR(r1)
> andi. r3,r3,MSR_PR
> beq 1f
> - b ret_from_except
> + REST_NVGPRS(r1)
Could this be in a separate preceding patch (only the adding of REST_NVGPRS(), the call to
ret_from_except can remain as is by removing the REST_NVGPRS() which is there to make
ret_from_except and ret_from_except_lite identical).
Or maybe you can also do the name change to interrupt_return in that preceeding patch, so than the
"use new interrupt return" patch only contains the interesting parts.
> + b interrupt_return
> 1:
>
> LOAD_REG_ADDR(r11,extlb_level_exc)
> @@ -208,7 +209,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
> /*
> * Restore PACAIRQHAPPENED rather than setting it based on
> * the return MSR[EE], since we could have interrupted
> - * __check_irq_replay() or other inconsistent transitory
> + * interrupt replay or other inconsistent transitory
> * states that must remain that way.
> */
> SPECIAL_EXC_LOAD(r10,IRQHAPPENED)
> @@ -511,7 +512,7 @@ exc_##n##_bad_stack: \
> CHECK_NAPPING(); \
> addi r3,r1,STACK_FRAME_OVERHEAD; \
> bl hdlr; \
> - b ret_from_except_lite;
> + b interrupt_return
>
> /* This value is used to mark exception frames on the stack. */
> .section ".toc","aw"
> @@ -623,7 +624,8 @@ __end_interrupts:
> addi r3,r1,STACK_FRAME_OVERHEAD
> ld r14,PACA_EXGEN+EX_R14(r13)
> bl program_check_exception
> - b ret_from_except
> + REST_NVGPRS(r1)
> + b interrupt_return
>
> /* Floating Point Unavailable Interrupt */
> START_EXCEPTION(fp_unavailable);
> @@ -635,11 +637,11 @@ __end_interrupts:
> andi. r0,r12,MSR_PR;
> beq- 1f
> bl load_up_fpu
> - b fast_exception_return
> + b fast_interrupt_return
> 1: INTS_DISABLE
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl kernel_fp_unavailable_exception
> - b ret_from_except
> + b interrupt_return
>
> /* Altivec Unavailable Interrupt */
> START_EXCEPTION(altivec_unavailable);
> @@ -653,14 +655,14 @@ BEGIN_FTR_SECTION
> andi. r0,r12,MSR_PR;
> beq- 1f
> bl load_up_altivec
> - b fast_exception_return
> + b fast_interrupt_return
> 1:
> END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
> #endif
> INTS_DISABLE
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl altivec_unavailable_exception
> - b ret_from_except
> + b interrupt_return
>
> /* AltiVec Assist */
> START_EXCEPTION(altivec_assist);
> @@ -674,10 +676,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
> BEGIN_FTR_SECTION
> bl altivec_assist_exception
> END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
> + REST_NVGPRS(r1)
> #else
> bl unknown_exception
> #endif
> - b ret_from_except
> + b interrupt_return
>
>
> /* Decrementer Interrupt */
> @@ -719,7 +722,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
> INTS_DISABLE
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl unknown_exception
> - b ret_from_except
> + b interrupt_return
>
> /* Debug exception as a critical interrupt*/
> START_EXCEPTION(debug_crit);
> @@ -786,7 +789,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
> ld r14,PACA_EXCRIT+EX_R14(r13)
> ld r15,PACA_EXCRIT+EX_R15(r13)
> bl DebugException
> - b ret_from_except
> + REST_NVGPRS(r1)
> + b interrupt_return
>
> kernel_dbg_exc:
> b . /* NYI */
> @@ -857,7 +861,8 @@ kernel_dbg_exc:
> ld r14,PACA_EXDBG+EX_R14(r13)
> ld r15,PACA_EXDBG+EX_R15(r13)
> bl DebugException
> - b ret_from_except
> + REST_NVGPRS(r1)
> + b interrupt_return
>
> START_EXCEPTION(perfmon);
> NORMAL_EXCEPTION_PROLOG(0x260, BOOKE_INTERRUPT_PERFORMANCE_MONITOR,
> @@ -867,7 +872,7 @@ kernel_dbg_exc:
> CHECK_NAPPING()
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl performance_monitor_exception
> - b ret_from_except_lite
> + b interrupt_return
>
> /* Doorbell interrupt */
> MASKABLE_EXCEPTION(0x280, BOOKE_INTERRUPT_DOORBELL,
> @@ -895,7 +900,7 @@ kernel_dbg_exc:
> addi r3,r1,STACK_FRAME_OVERHEAD
> INTS_RESTORE_HARD
> bl unknown_exception
> - b ret_from_except
> + b interrupt_return
>
> /* Guest Doorbell critical Interrupt */
> START_EXCEPTION(guest_doorbell_crit);
> @@ -916,7 +921,7 @@ kernel_dbg_exc:
> addi r3,r1,STACK_FRAME_OVERHEAD
> INTS_RESTORE_HARD
> bl unknown_exception
> - b ret_from_except
> + b interrupt_return
>
> /* Embedded Hypervisor priviledged */
> START_EXCEPTION(ehpriv);
> @@ -926,7 +931,7 @@ kernel_dbg_exc:
> addi r3,r1,STACK_FRAME_OVERHEAD
> INTS_RESTORE_HARD
> bl unknown_exception
> - b ret_from_except
> + b interrupt_return
>
> /* LRAT Error interrupt */
> START_EXCEPTION(lrat_error);
> @@ -936,7 +941,7 @@ kernel_dbg_exc:
> addi r3,r1,STACK_FRAME_OVERHEAD
> INTS_RESTORE_HARD
> bl unknown_exception
> - b ret_from_except
> + b interrupt_return
>
> /*
> * An interrupt came in while soft-disabled; We mark paca->irq_happened
> @@ -998,11 +1003,11 @@ storage_fault_common:
> bl do_page_fault
> cmpdi r3,0
> bne- 1f
> - b ret_from_except_lite
> + b interrupt_return
> mr r4,r3
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl __bad_page_fault
> - b ret_from_except
> + b interrupt_return
>
> /*
> * Alignment exception doesn't fit entirely in the 0x100 bytes so it
> @@ -1016,284 +1021,8 @@ alignment_more:
...
> -fast_exception_return:
> - wrteei 0
> -1: mr r0,r13
> - ld r10,_MSR(r1)
> - REST_4GPRS(2, r1)
> - andi. r6,r10,MSR_PR
> - REST_2GPRS(6, r1)
> - beq 1f
> - ACCOUNT_CPU_USER_EXIT(r13, r10, r11)
Then ACCOUNT_CPU_USER_EXIT can be removed from asm/ppc_asm.h
...
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index fbabb49888d3..ae7b058b2970 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -235,6 +235,10 @@ static notrace void booke_load_dbcr0(void)
> #endif
> }
>
> +/* temporary hack for context tracking, removed in later patch */
> +#include <linux/sched/debug.h>
> +asmlinkage __visible void __sched schedule_user(void);
> +
> /*
> * This should be called after a syscall returns, with r3 the return value
> * from the syscall. If this function returns non-zero, the system call
> @@ -292,7 +296,11 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
> while (unlikely(ti_flags & (_TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM))) {
> local_irq_enable();
> if (ti_flags & _TIF_NEED_RESCHED) {
> +#ifdef CONFIG_PPC_BOOK3E_64
> + schedule_user();
> +#else
> schedule();
> +#endif
> } else {
> /*
> * SIGPENDING must restore signal handler function
> @@ -360,7 +368,6 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
> return ret;
> }
>
> -#ifndef CONFIG_PPC_BOOK3E_64 /* BOOK3E not yet using this */
> notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned long msr)
> {
> unsigned long ti_flags;
> @@ -372,7 +379,9 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
> BUG_ON(!(regs->msr & MSR_PR));
> BUG_ON(!FULL_REGS(regs));
> BUG_ON(arch_irq_disabled_regs(regs));
> +#ifdef CONFIG_PPC_BOOK3S_64
> CT_WARN_ON(ct_state() == CONTEXT_USER);
> +#endif
>
> /*
> * We don't need to restore AMR on the way back to userspace for KUAP.
> @@ -387,7 +396,11 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
> while (unlikely(ti_flags & (_TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM))) {
> local_irq_enable(); /* returning to user: may enable */
> if (ti_flags & _TIF_NEED_RESCHED) {
> +#ifdef CONFIG_PPC_BOOK3E_64
> + schedule_user();
> +#else
> schedule();
> +#endif
> } else {
> if (ti_flags & _TIF_SIGPENDING)
> ret |= _TIF_RESTOREALL;
> @@ -435,7 +448,10 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
> /*
> * We do this at the end so that we do context switch with KERNEL AMR
> */
> +#ifndef CONFIG_PPC_BOOK3E_64
> kuap_user_restore(regs);
Why do you need to ifdef this out ?
Only PPC_8xx, PPC_BOOK3S_32 and PPC_RADIX_MMU select PPC_HAVE_KUAP.
When PPC_KUAP is not selected, kuap_user_restore() is a static inline {} defined in asm/kup.h
> +#endif
> +
> return ret;
> }
>
> @@ -445,7 +461,9 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> {
> unsigned long flags;
> unsigned long ret = 0;
> +#ifndef CONFIG_PPC_BOOK3E_64
> unsigned long kuap;
> +#endif
>
> if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) &&
> unlikely(!(regs->msr & MSR_RI)))
> @@ -456,10 +474,12 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> * CT_WARN_ON comes here via program_check_exception,
> * so avoid recursion.
> */
> - if (TRAP(regs) != 0x700)
> + if (IS_ENABLED(CONFIG_BOOKS) && TRAP(regs) != 0x700)
> CT_WARN_ON(ct_state() == CONTEXT_USER);
>
> +#ifndef CONFIG_PPC_BOOK3E_64
> kuap = kuap_get_and_assert_locked();
Same, kuap_get_and_assert_locked() always exists, no need to ifdef it.
> +#endif
>
> if (unlikely(current_thread_info()->flags & _TIF_EMULATE_STACK_STORE)) {
> clear_bits(_TIF_EMULATE_STACK_STORE, ¤t_thread_info()->flags);
> @@ -501,8 +521,9 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> * which would cause Read-After-Write stalls. Hence, we take the AMR
> * value from the check above.
> */
> +#ifndef CONFIG_PPC_BOOK3E_64
> kuap_kernel_restore(regs, kuap);
Same
> +#endif
>
> return ret;
> }
> -#endif
^ permalink raw reply
* Re: [PATCH 4/4] tools/perf: Support pipeline stage cycles for powerpc
From: Athira Rajeev @ 2021-03-15 7:52 UTC (permalink / raw)
To: Jiri Olsa
Cc: Ravi Bangoria, Madhavan Srinivasan, Peter Zijlstra, kjain,
linux-kernel, acme, linux-perf-users, jolsa, linuxppc-dev,
kan.liang
In-Reply-To: <YEtlEyb2z33qHhvO@krava>
> On 12-Mar-2021, at 6:26 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>
> 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
Ok Sure,
>
> 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
>
Hi Jiri,
Thanks for the suggestions.
Below is an approach I came up with for adding dynamic sort key based on architecture support.
With this patch, perf report for mem mode will display new sort key only in supported archs.
Please help to review if this approach looks good. I have created this on top of my current set. If this looks fine,
I can include this in version2 patch set.
From 8ebbe6ae802d895103335899e4e60dde5e562f33 Mon Sep 17 00:00:00 2001
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Date: Mon, 15 Mar 2021 02:33:28 +0000
Subject: [PATCH] tools/perf: Add dynamic sort dimensions for mem mode
Add dynamic sort dimensions for mem mode.
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
tools/perf/arch/powerpc/util/event.c | 7 +++++
tools/perf/util/event.h | 1 +
tools/perf/util/sort.c | 43 +++++++++++++++++++++++++++-
3 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/tools/perf/arch/powerpc/util/event.c b/tools/perf/arch/powerpc/util/event.c
index b80fbee83b6e..fddfc288c415 100644
--- a/tools/perf/arch/powerpc/util/event.c
+++ b/tools/perf/arch/powerpc/util/event.c
@@ -44,3 +44,10 @@ const char *arch_perf_header_entry__add(const char *se_header)
return "Dispatch Cyc";
return se_header;
}
+
+int arch_support_dynamic_key(const char *sort_key)
+{
+ if (!strcmp(sort_key, "p_stage_cyc"))
+ return 1;
+ return 0;
+}
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 65f89e80916f..6cd4bf54dbdc 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -429,5 +429,6 @@ char *get_page_size_name(u64 size, char *str);
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);
+int arch_support_dynamic_key(const char *sort_key);
#endif /* __PERF_RECORD_H */
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index cbb3899e7eca..e194b1187db8 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -37,7 +37,7 @@ const char default_parent_pattern[] = "^sys_|^do_page_fault";
const char *parent_pattern = default_parent_pattern;
const char *default_sort_order = "comm,dso,symbol";
const char default_branch_sort_order[] = "comm,dso_from,symbol_from,symbol_to,cycles";
-const char default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,p_stage_cyc";
+const char default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat";
const char default_top_sort_order[] = "dso,symbol";
const char default_diff_sort_order[] = "dso,symbol";
const char default_tracepoint_sort_order[] = "trace";
@@ -47,6 +47,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", "p_stage_cyc"};
+const char *dynamic_sort_keys_mem[] = {"p_stage_cyc"};
/*
* Replaces all occurrences of a char used with the:
@@ -2997,6 +2998,20 @@ static char *prefix_if_not_in(const char *pre, char *str)
return n;
}
+/*
+ * Adds 'suff,' suffix into 'str' if 'suff' is
+ * not already part of 'str'.
+ */
+static char *suffix_if_not_in(const char *suff, char *str)
+{
+ if (!str || strstr(str, suff))
+ return str;
+
+ if (asprintf(&str, "%s,%s", str, suff) < 0)
+ str = NULL;
+ return str;
+}
+
static char *setup_overhead(char *keys)
{
if (sort__mode == SORT_MODE__DIFF)
@@ -3010,6 +3025,26 @@ static char *setup_overhead(char *keys)
return keys;
}
+int __weak arch_support_dynamic_key(const char *sort_key __maybe_unused)
+{
+ return 0;
+}
+
+static char *setup_dynamic_sort_keys(char *str)
+{
+ unsigned int j;
+
+ if (sort__mode == SORT_MODE__MEMORY)
+ for (j = 0; j < ARRAY_SIZE(dynamic_sort_keys_mem); j++)
+ if (arch_support_dynamic_key(dynamic_sort_keys_mem[j])) {
+ str = suffix_if_not_in(dynamic_sort_keys_mem[j], str);
+ if (str == NULL)
+ return str;
+ }
+
+ return str;
+}
+
static int __setup_sorting(struct evlist *evlist)
{
char *str;
@@ -3050,6 +3085,12 @@ static int __setup_sorting(struct evlist *evlist)
}
}
+ str = setup_dynamic_sort_keys(str);
+ if (str == NULL) {
+ pr_err("Not enough memory to setup dynamic sort keys");
+ return -ENOMEM;
+ }
+
ret = setup_sort_list(&perf_hpp_list, str, evlist);
free(str);
--
2.26.2
Thanks,
Athira
>
>> };
>>
>> #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
Ok.
>
> thanks,
> jirka
^ permalink raw reply related
* Re: [PATCH 15/17] iommu: remove DOMAIN_ATTR_NESTING
From: Auger Eric @ 2021-03-15 7:52 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: <20210314155813.GA788@lst.de>
Hi Christoph,
On 3/14/21 4:58 PM, Christoph Hellwig wrote:
> On Sun, Mar 14, 2021 at 11:44:52AM +0100, Auger Eric wrote:
>> As mentionned by Robin, there are series planning to use
>> DOMAIN_ATTR_NESTING to get info about the nested caps of the iommu (ARM
>> and Intel):
>>
>> [Patch v8 00/10] vfio: expose virtual Shared Virtual Addressing to VMs
>> patches 1, 2, 3
>>
>> Is the plan to introduce a new domain_get_nesting_info ops then?
>
> The plan as usual would be to add it the series adding that support.
> Not sure what the merge plans are - if the series is ready to be
> merged I could rebase on top of it, otherwise that series will need
> to add the method.
OK I think your series may be upstreamed first.
Thanks
Eric
>
^ permalink raw reply
* Re: VIO bus not initialized
From: Michael Ellerman @ 2021-03-15 7:53 UTC (permalink / raw)
To: Paul Menzel, Benjamin Herrenschmidt, Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <41c04182-20c1-6124-a221-90aef704e310@molgen.mpg.de>
Paul Menzel <pmenzel@molgen.mpg.de> writes:
> Dear Linux folks,
>
>
> On the POWER8 system IBM S822LC, Linux 5.12-rc2+ logs the errors below.
That's a bare metal system, you can see that from the line "Using
PowerNV machine description" in the boot log.
> $ dmesg --level=err
> [ 1.555668] Driver 'hvc_console' was unable to register with
> bus_type 'vio' because the bus was not initialized.
> [ 1.558434] Driver 'tpm_ibmvtpm' was unable to register with
> bus_type 'vio' because the bus was not initialized.
> $ grep VIO /boot/config-5.12.0-rc2+
> CONFIG_IBMVIO=y
The "vio" bus is not a real bus, it's a fake bus we use for hypervisor
provided devices in LPARs (guests).
So on bare metal machines there is no vio bus, the devices that would
appear on the vio bus are found via other mechanisms.
> [ 1.555668] Driver 'hvc_console' was unable to register with bus_type 'vio' because the bus was not initialized.
> [ 1.555866] hvc0: raw protocol on /ibm,opal/consoles/serial@0 (boot console)
> [ 1.555875] hvc0: No interrupts property, using OPAL event
You can see here that the hvc opal driver found the console, which is
the bare metal equivalent of the hvc console (vio) driver.
> [ 1.556205] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
> [ 1.558377] Non-volatile memory driver v1.3
> [ 1.558404] Linux agpgart interface v0.103
> [ 1.558434] Driver 'tpm_ibmvtpm' was unable to register with bus_type 'vio' because the bus was not initialized.
The ibmvtpm driver is a Virtual TPM driver, so it also isn't present on
a bare metal system.
I don't think that system has a TPM, but if it did you'd need the
appropriate bare metal TPM driver to use it.
cheers
^ permalink raw reply
* [PATCH 1/2] powerpc/pseries/mobility: use struct for shared state
From: Nathan Lynch @ 2021-03-15 8:00 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tyreld, brking, npiggin
In-Reply-To: <20210315080045.460331-1-nathanl@linux.ibm.com>
The atomic_t counter is the only shared state for the join/suspend
sequence so far, but that will change. Contain it in a
struct (pseries_suspend_info), and document its intended use. No
functional change.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/platforms/pseries/mobility.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index ea4d6a660e0d..a6739ce9feac 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -452,9 +452,21 @@ static int do_suspend(void)
return ret;
}
+/**
+ * struct pseries_suspend_info - State shared between CPUs for join/suspend.
+ * @counter: Threads are to increment this upon resuming from suspend
+ * or if an error is received from H_JOIN. The thread which performs
+ * the first increment (i.e. sets it to 1) is responsible for
+ * waking the other threads.
+ */
+struct pseries_suspend_info {
+ atomic_t counter;
+};
+
static int do_join(void *arg)
{
- atomic_t *counter = arg;
+ struct pseries_suspend_info *info = arg;
+ atomic_t *counter = &info->counter;
long hvrc;
int ret;
@@ -535,11 +547,15 @@ static int pseries_suspend(u64 handle)
int ret;
while (true) {
- atomic_t counter = ATOMIC_INIT(0);
+ struct pseries_suspend_info info;
unsigned long vasi_state;
int vasi_err;
- ret = stop_machine(do_join, &counter, cpu_online_mask);
+ info = (struct pseries_suspend_info) {
+ .counter = ATOMIC_INIT(0),
+ };
+
+ ret = stop_machine(do_join, &info, cpu_online_mask);
if (ret == 0)
break;
/*
--
2.29.2
^ permalink raw reply related
* [PATCH 0/2] handle premature return from H_JOIN in pseries mobility code
From: Nathan Lynch @ 2021-03-15 8:00 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tyreld, brking, npiggin
pseries VMs in shared processor mode are susceptible to failed
migrations becasue stray H_PRODs from the paravirt spinlock
implementation can bump threads out of joining state before the
suspend has occurred. Fix this by adding a small amount of shared
state and ordering accesses to it with respect to H_PROD and H_JOIN.
Nathan Lynch (2):
powerpc/pseries/mobility: use struct for shared state
powerpc/pseries/mobility: handle premature return from H_JOIN
arch/powerpc/platforms/pseries/mobility.c | 48 +++++++++++++++++++++--
1 file changed, 44 insertions(+), 4 deletions(-)
--
2.29.2
^ permalink raw reply
* [PATCH 2/2] powerpc/pseries/mobility: handle premature return from H_JOIN
From: Nathan Lynch @ 2021-03-15 8:00 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tyreld, brking, npiggin
In-Reply-To: <20210315080045.460331-1-nathanl@linux.ibm.com>
The pseries join/suspend sequence in its current form was written with
the assumption that it was the only user of H_PROD and that it needn't
handle spurious successful returns from H_JOIN. That's wrong;
powerpc's paravirt spinlock code uses H_PROD, and CPUs entering
do_join() can be woken prematurely from H_JOIN with a status of
H_SUCCESS as a result. This causes all CPUs to exit the sequence
early, preventing suspend from occurring at all.
Add a 'done' boolean flag to the pseries_suspend_info struct, and have
the waking thread set it before waking the other threads. Threads
which receive H_SUCCESS from H_JOIN retry if the 'done' flag is still
unset.
Fixes: 9327dc0aeef3 ("powerpc/pseries/mobility: use stop_machine for join/suspend")
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/platforms/pseries/mobility.c | 26 ++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index a6739ce9feac..e83e0891272d 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -458,9 +458,12 @@ static int do_suspend(void)
* or if an error is received from H_JOIN. The thread which performs
* the first increment (i.e. sets it to 1) is responsible for
* waking the other threads.
+ * @done: False if join/suspend is in progress. True if the operation is
+ * complete (successful or not).
*/
struct pseries_suspend_info {
atomic_t counter;
+ bool done;
};
static int do_join(void *arg)
@@ -470,6 +473,7 @@ static int do_join(void *arg)
long hvrc;
int ret;
+retry:
/* Must ensure MSR.EE off for H_JOIN. */
hard_irq_disable();
hvrc = plpar_hcall_norets(H_JOIN);
@@ -485,8 +489,20 @@ static int do_join(void *arg)
case H_SUCCESS:
/*
* The suspend is complete and this cpu has received a
- * prod.
+ * prod, or we've received a stray prod from unrelated
+ * code (e.g. paravirt spinlocks) and we need to join
+ * again.
+ *
+ * This barrier orders the return from H_JOIN above vs
+ * the load of info->done. It pairs with the barrier
+ * in the wakeup/prod path below.
*/
+ smp_mb();
+ if (READ_ONCE(info->done) == false) {
+ pr_info_ratelimited("premature return from H_JOIN on CPU %i, retrying",
+ smp_processor_id());
+ goto retry;
+ }
ret = 0;
break;
case H_BAD_MODE:
@@ -500,6 +516,13 @@ static int do_join(void *arg)
if (atomic_inc_return(counter) == 1) {
pr_info("CPU %u waking all threads\n", smp_processor_id());
+ WRITE_ONCE(info->done, true);
+ /*
+ * This barrier orders the store to info->done vs subsequent
+ * H_PRODs to wake the other CPUs. It pairs with the barrier
+ * in the H_SUCCESS case above.
+ */
+ smp_mb();
prod_others();
}
/*
@@ -553,6 +576,7 @@ static int pseries_suspend(u64 handle)
info = (struct pseries_suspend_info) {
.counter = ATOMIC_INIT(0),
+ .done = false,
};
ret = stop_machine(do_join, &info, cpu_online_mask);
--
2.29.2
^ permalink raw reply related
* Re: VIO bus not initialized
From: Paul Menzel @ 2021-03-15 8:11 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <87mtv44zut.fsf@mpe.ellerman.id.au>
Dear Michael,
Thank you very much for your response.
Am 15.03.21 um 08:53 schrieb Michael Ellerman:
> Paul Menzel writes:
>> On the POWER8 system IBM S822LC, Linux 5.12-rc2+ logs the errors below.
>
> That's a bare metal system, you can see that from the line "Using
> PowerNV machine description" in the boot log.
>
>> $ dmesg --level=err
>> [ 1.555668] Driver 'hvc_console' was unable to register with bus_type 'vio' because the bus was not initialized.
>> [ 1.558434] Driver 'tpm_ibmvtpm' was unable to register with bus_type 'vio' because the bus was not initialized.
>> $ grep VIO /boot/config-5.12.0-rc2+
>> CONFIG_IBMVIO=y
>
> The "vio" bus is not a real bus, it's a fake bus we use for hypervisor
> provided devices in LPARs (guests).
>
> So on bare metal machines there is no vio bus, the devices that would
> appear on the vio bus are found via other mechanisms.
Thank you for the explanation. Two questions:
1. Could a bare metal system be detected, and the VIO “be skipped”?
2. Should the log level be changed to notice or info then, as it’s an
expected failure?
[…]
Kind regards,
Paul
^ permalink raw reply
* Re: [PATCH 10/10] powerpc: move norestart trap flag to bit 0
From: Christophe Leroy @ 2021-03-15 8:14 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Scott Wood
In-Reply-To: <20210315031716.3940350-11-npiggin@gmail.com>
Le 15/03/2021 à 04:17, Nicholas Piggin a écrit :
> Compact the trap flags down to use the low 4 bits of regs.trap.
>
> A few 64e interrupt trap numbers set bit 4. Although they tended to be
> trivial so it wasn't a real problem[1], it is not the right thing to do,
> and confusing.
>
> [*] E.g., 0x310 hypercall goes to unknown_exception, which prints
> regs->trap directly so 0x310 will appear fine, and only the syscall
> interrupt will test norestart, so it won't be confused by 0x310.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/ptrace.h | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> index 91194fdd5d01..6a04abfe5eb6 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -185,15 +185,21 @@ static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc)
> #define current_pt_regs() \
> ((struct pt_regs *)((unsigned long)task_stack_page(current) + THREAD_SIZE) - 1)
>
> +/*
> + * The 4 low bits (0xf) are available as flags to overload the trap word,
> + * because interrupt vectors have minimum alignment of 0x10. TRAP_FLAGS_MASK
> + * must cover the bits used as flags, including bit 0 which is used as the
> + * "norestart" bit.
> + */
> #ifdef __powerpc64__
> -#define TRAP_FLAGS_MASK 0x10
> +#define TRAP_FLAGS_MASK 0x1
> #define TRAP(regs) ((regs)->trap & ~TRAP_FLAGS_MASK)
> #else
> /*
> * On 4xx we use bit 1 in the trap word to indicate whether the exception
> * is a critical exception (1 means it is).
> */
> -#define TRAP_FLAGS_MASK 0x1E
> +#define TRAP_FLAGS_MASK 0xf
Could we set 0xf for all and remove the ifdef __powerpc64__ ?
> #define TRAP(regs) ((regs)->trap & ~TRAP_FLAGS_MASK)
> #define IS_CRITICAL_EXC(regs) (((regs)->trap & 2) != 0)
> #define IS_MCHECK_EXC(regs) (((regs)->trap & 4) != 0)
> @@ -222,12 +228,12 @@ static inline bool trap_is_syscall(struct pt_regs *regs)
>
> static inline bool trap_norestart(struct pt_regs *regs)
> {
> - return regs->trap & 0x10;
> + return regs->trap & 0x1;
> }
>
> static inline void set_trap_norestart(struct pt_regs *regs)
> {
> - regs->trap |= 0x10;
> + regs->trap |= 0x1;
> }
>
> #define arch_has_single_step() (1)
>
While we are playing with ->trap, in mm/book3s64/hash_utils.c there is an if (regs->trap == 0x400).
Should be TRAP(regs) == 0x400 ?
Christophe
^ permalink raw reply
* Re: [PATCH 03/10] powerpc/64e/interrupt: use new interrupt return
From: Christophe Leroy @ 2021-03-15 8:20 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Scott Wood
In-Reply-To: <186d3513-d7ab-a658-cdb2-6fe5146c1fc4@csgroup.eu>
Le 15/03/2021 à 08:50, Christophe Leroy a écrit :
>
>
> Le 15/03/2021 à 04:17, Nicholas Piggin a écrit :
>> Update the new C and asm interrupt return code to account for 64e
>> specifics, switch over to use it.
>>
>> The now-unused old ret_from_except code, that was moved to 64e after the
>> 64s conversion, is removed.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/include/asm/asm-prototypes.h | 2 -
>> arch/powerpc/kernel/entry_64.S | 9 +-
>> arch/powerpc/kernel/exceptions-64e.S | 321 ++--------------------
>> arch/powerpc/kernel/interrupt.c | 27 +-
>> arch/powerpc/kernel/irq.c | 76 -----
>> 5 files changed, 56 insertions(+), 379 deletions(-)
>>
>> @@ -1016,284 +1021,8 @@ alignment_more:
>
> ...
>
>> -fast_exception_return:
>> - wrteei 0
>> -1: mr r0,r13
>> - ld r10,_MSR(r1)
>> - REST_4GPRS(2, r1)
>> - andi. r6,r10,MSR_PR
>> - REST_2GPRS(6, r1)
>> - beq 1f
>> - ACCOUNT_CPU_USER_EXIT(r13, r10, r11)
>
> Then ACCOUNT_CPU_USER_EXIT can be removed from asm/ppc_asm.h
>
And all associated definitions in asm-offsets.c
And also ACCOUNT_USER_TIME which was likely left over after the removal of ACCOUNT_CPU_USER_ENTRY()
^ permalink raw reply
* Re: [PATCH 14/17] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
From: Christoph Hellwig @ 2021-03-15 8:33 UTC (permalink / raw)
To: Robin Murphy
Cc: kvm, Will Deacon, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang,
iommu, netdev, David Woodhouse, linux-arm-kernel, virtualization,
freedreno, Christoph Hellwig, linux-arm-msm
In-Reply-To: <dff8eb80-8f74-972b-17e9-496c1fc0396f@arm.com>
On Fri, Mar 12, 2021 at 04:18:24PM +0000, Robin Murphy wrote:
>> 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,
I think a private inteface would make more sense. For now I've just
condensed it down to a generic set of quirk bits and dropped the
attrs structure, which seems like an ok middle ground for now. That
being said I wonder why that quirk isn't simply set in the device
tree?
> 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.
Fixed. Can I get your signoff for the patch? Then I'll switch it to
over to being attributed to you.
^ permalink raw reply
* Re: [PATCH] powerpc/asm: Remove unused symbols in asm-offsets.c
From: Christophe Leroy @ 2021-03-15 8:35 UTC (permalink / raw)
To: Rashmica Gupta, linuxppc-dev, mpe, benh, paulus
In-Reply-To: <1464821807-3154-1-git-send-email-rashmicy@gmail.com>
Le 02/06/2016 à 00:56, Rashmica Gupta a écrit :
> ---
> GPR15, GPR16, GPR17, GPR18, GPR19, GPR20, GPR21, GPR22, GPR23, GPR24,
> GPR25, GPR26, GPR27, GPR28, GPR29 and GPR30: Added in commit
> a4bcbe6a41ad ("powerpc: Remove old compile time disabled syscall tracing
> code") and were never used.
> ---
GPR14 has never been used either. You only find it in a couple of comments.
^ permalink raw reply
* RE: [PATCH] powerpc/603: Fix protection of user pages mapped with PROT_NONE
From: PLATTNER Christoph @ 2021-03-15 8:55 UTC (permalink / raw)
To: Michael Ellerman, Michael Ellerman, Paul Mackerras,
Benjamin Herrenschmidt, Christophe Leroy
Cc: PLATTNER Christoph, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org
In-Reply-To: <161571587047.138988.3119931595106306211.b4-ty@ellerman.id.au>
Thank you for maintenance and for following this request.
Regards
Christoph
-----Original Message-----
From: Michael Ellerman <patch-notifications@ellerman.id.au>
Sent: Sonntag, 14. März 2021 11:01
To: Michael Ellerman <mpe@ellerman.id.au>; Paul Mackerras <paulus@samba.org>; Benjamin Herrenschmidt <benh@kernel.crashing.org>; PLATTNER Christoph <christoph.plattner@thalesgroup.com>; Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] powerpc/603: Fix protection of user pages mapped with PROT_NONE
On Mon, 1 Feb 2021 06:29:50 +0000 (UTC), Christophe Leroy wrote:
> On book3s/32, page protection is defined by the PP bits in the PTE
> which provide the following protection depending on the access keys
> defined in the matching segment register:
> - PP 00 means RW with key 0 and N/A with key 1.
> - PP 01 means RW with key 0 and RO with key 1.
> - PP 10 means RW with both key 0 and key 1.
> - PP 11 means RO with both key 0 and key 1.
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/603: Fix protection of user pages mapped with PROT_NONE
https://git.kernel.org/powerpc/c/c119565a15a628efdfa51352f9f6c5186e506a1c
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: define the variable 'uaccess_flush' as static
From: heying (H) @ 2021-03-15 6:51 UTC (permalink / raw)
To: Cédric Le Goater, mpe, benh, paulus, npiggin, dja, akpm,
rppt, aneesh.kumar
Cc: johnny.chenyi, linuxppc-dev, linux-kernel
In-Reply-To: <a06a0dc8-c717-da4c-c5ad-eaf56bbbd896@kaod.org>
[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]
> I think this is the case also for entry_flush. compiling with W=1 will tell you more.
When I use these commands:
make allmodconfig ARCH=powerpc CROSS_COMPILE=powerpc64-linux-gnu-
make C=2 arch/powerpc/kernel/setup_64.o ARCH=powerpc CROSS_COMPILE=powerpc64-linux-gnu-
I find warnings as followings:
arch/powerpc/kernel/setup_64.c:422:6: warning: symbol
'panic_smp_self_stop' was not declared. Should it be static?
arch/powerpc/kernel/setup_64.c:951:6: warning: symbol 'rfi_flush' was
not declared. Should it be static?
arch/powerpc/kernel/setup_64.c:952:6: warning: symbol 'entry_flush' was
not declared. Should it be static?
arch/powerpc/kernel/setup_64.c:953:6: warning: symbol 'uaccess_flush'
was not declared. Should it be static?
When I use the command "make W=1 arch/powerpc/kernel/setup_64.o
ARCH=powerpc CROSS_COMPILE=powerpc64-linux-gnu-", warning becomes this:
arch/powerpc/kernel/setup_64.c:422:6: warning: no previous prototype for
‘panic_smp_self_stop’ [-Wmissing-prototypes]
void panic_smp_self_stop(void)
^~~~~~~~~~~~~~~~~~~
My sparse tool is the latest one with the version "v0.6.3". So, should I
fix all the warnings reported by sparse?
Thanks.
[-- Attachment #2: Type: text/html, Size: 2430 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc: define the variable 'uaccess_flush' as static
From: Christophe Leroy @ 2021-03-15 9:16 UTC (permalink / raw)
To: heying (H), Cédric Le Goater, mpe, benh, paulus, npiggin,
dja, akpm, rppt, aneesh.kumar
Cc: johnny.chenyi, linuxppc-dev, linux-kernel
In-Reply-To: <2c7bf6e0-d950-c728-bfe9-2db99a4d18a9@huawei.com>
Le 15/03/2021 à 07:51, heying (H) a écrit :
>
>> I think this is the case also for entry_flush. compiling with W=1 will tell you more.
>
> When I use these commands:
>
> make allmodconfig ARCH=powerpc CROSS_COMPILE=powerpc64-linux-gnu-
> make C=2 arch/powerpc/kernel/setup_64.o ARCH=powerpc CROSS_COMPILE=powerpc64-linux-gnu-
>
> I find warnings as followings:
>
> arch/powerpc/kernel/setup_64.c:422:6: warning: symbol 'panic_smp_self_stop' was not declared. Should
> it be static?
> arch/powerpc/kernel/setup_64.c:951:6: warning: symbol 'rfi_flush' was not declared. Should it be static?
> arch/powerpc/kernel/setup_64.c:952:6: warning: symbol 'entry_flush' was not declared. Should it be
> static?
> arch/powerpc/kernel/setup_64.c:953:6: warning: symbol 'uaccess_flush' was not declared. Should it be
> static?
>
> When I use the command "make W=1 arch/powerpc/kernel/setup_64.o ARCH=powerpc
> CROSS_COMPILE=powerpc64-linux-gnu-", warning becomes this:
>
> arch/powerpc/kernel/setup_64.c:422:6: warning: no previous prototype for ‘panic_smp_self_stop’
> [-Wmissing-prototypes]
> void panic_smp_self_stop(void)
> ^~~~~~~~~~~~~~~~~~~
>
> My sparse tool is the latest one with the version "v0.6.3". So, should I fix all the warnings
> reported by sparse?
I think W=1 will only report missing function prototypes.
sparse also reports missing variables prototypes so that's better. All should be fixed.
Christophe
^ permalink raw reply
* Re: [PATCH] powerpc: define the variable 'uaccess_flush' as static
From: heying (H) @ 2021-03-15 9:32 UTC (permalink / raw)
To: Christophe Leroy, Cédric Le Goater, mpe, benh, paulus,
npiggin, dja, akpm, rppt, aneesh.kumar
Cc: johnny.chenyi, linuxppc-dev
In-Reply-To: <c522ca02-4053-3e3d-f25e-2c1482c5a8bd@csgroup.eu>
Hello,
在 2021/3/15 17:16, Christophe Leroy 写道:
>
>
> I think W=1 will only report missing function prototypes.
>
> sparse also reports missing variables prototypes so that's better. All
> should be fixed.
>
OK. I'll try to fix all the warnings in the file
"arch/powerpc/kernel/setup_64.c" reported by sparse and send the patch
V2 soon.
Thanks.
^ permalink raw reply
* RE: Errant readings on LM81 with T2080 SoC
From: David Laight @ 2021-03-15 9:46 UTC (permalink / raw)
To: 'Chris Packham', 'Guenter Roeck', Wolfram Sang
Cc: linux-hwmon@vger.kernel.org, jdelvare@suse.com,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
linux-i2c@vger.kernel.org
In-Reply-To: <ec89dfda-a321-6ec7-9da0-b4949f1f28b5@alliedtelesis.co.nz>
From: Chris Packham
> Sent: 14 March 2021 21:26
>
> On 12/03/21 10:25 pm, David Laight wrote:
> > From: Linuxppc-dev Guenter Roeck
> >> Sent: 11 March 2021 21:35
> >>
> >> On 3/11/21 1:17 PM, Chris Packham wrote:
> >>> On 11/03/21 9:18 pm, Wolfram Sang wrote:
> >>>>> Bummer. What is really weird is that you see clock stretching under
> >>>>> CPU load. Normally clock stretching is triggered by the device, not
> >>>>> by the host.
> >>>> One example: Some hosts need an interrupt per byte to know if they
> >>>> should send ACK or NACK. If that interrupt is delayed, they stretch the
> >>>> clock.
> >>>>
> >>> It feels like something like that is happening. Looking at the T2080
> >>> Reference manual there is an interesting timing diagram (Figure 14-2 if
> >>> someone feels like looking it up). It shows SCL low between the ACK for
> >>> the address and the data byte. I think if we're delayed in sending the
> >>> next byte we could violate Ttimeout or Tlow:mext from the SMBUS spec.
> >>>
> >> I think that really leaves you only two options that I can see:
> >> Rework the driver to handle critical actions (such as setting TXAK,
> >> and everything else that might result in clock stretching) in the
> >> interrupt handler, or rework the driver to handle everything in
> >> a high priority kernel thread.
> >
> > I'm not sure a high priority kernel thread will help.
> > Without CONFIG_PREEMPT (which has its own set of nasties)
> > a RT process won't be scheduled until the processor it last
> > ran on does a reschedule.
> > I don't think a kernel thread will be any different from a
> > user process running under the RT scheduler.
> >
> > I'm trying to remember the smbus spec (without remembering the I2C one).
> For those following along the spec is available here[0]. I know there's
> a 3.0 version[1] as well but the devices I'm dealing with are from a 2.0
> vintage.
> > While basically a clock+data bit-bang the slave is allowed to drive
> > the clock low to extend a cycle.
> > It may be allowed to do this at any point?
>
> From what I can see it's actually the master extending the clock. Or
> more accurately holding it low between the address and data bytes (which
> from the T2080 reference manual looks expected). I think this may cause
> a strictly compliant SMBUS device to determine that Tlow:mext has been
> violated.
Yes, the spec does seem to assume that is a signal is stable
for 20ms something has gone 'horribly wrong'.
I wasn't worries about that, our fpga does the whole transaction
as a single command.
None of our slaves generate interrupts - so it is purely master/slave.
If you run your process under the RT scheduler it is unlikely
that pre-emption will be delayed by long enough to stop the process
running for 10ms.
I've seen >1ms delays (testing RTP audio), but most of the long
loops have a cond_resched() in them.
...
> Probably depends on the device implementation. I've got multiple other
> I2C/SMBUS devices and the LM81 seems to be the one that objects.
I bet most don't implement any of the timeouts.
I found one interesting pmbus device.
Sometimes it would detect a STOP condition because the data line
went high when it tri-stated its output driver in response to the
rising clock edge!
So it saw the same clock edge twice.
> [0] - http://www.smbus.org/specs/smbus20.pdf
> [1] - https://pmbus.org/Assets/PDFS/Public/SMBus_3_0_20141220.pdf
I should have both those - I've copied them to the directory where
I'd look for them first!
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: Build regressions/improvements in v5.12-rc3
From: Geert Uytterhoeven @ 2021-03-15 10:49 UTC (permalink / raw)
To: Linux Kernel Mailing List; +Cc: linuxppc-dev
In-Reply-To: <20210315104409.1598822-1-geert@linux-m68k.org>
On Mon, Mar 15, 2021 at 11:46 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> JFYI, when comparing v5.12-rc3[1] to v5.12-rc2[3], the summaries are:
> - build errors: +2/-2
> 2 error regressions:
> + /kisskb/src/include/linux/compiler_types.h: error: call to '__compiletime_assert_248' declared with attribute error: BUILD_BUG failed: => 320:38
> + /kisskb/src/include/linux/compiler_types.h: error: call to '__compiletime_assert_249' declared with attribute error: BUILD_BUG failed: => 320:38
powerpc-gcc4.9/ppc64_book3e_allmodconfig
So we traded implicit declaration errors:
- /kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:
error: implicit declaration of function 'disable_kernel_vsx'
[-Werror=implicit-function-declaration]: 674:2 =>
- /kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:
error: implicit declaration of function 'enable_kernel_vsx'
[-Werror=implicit-function-declaration]: 638:2 =>
for compile-time assertions.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ 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