* [PATCH v2 09/16] powerpc/kuap: Prepare for supporting KUAP on BOOK3E/64
From: Christophe Leroy @ 2021-10-08 15:40 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1633707305.git.christophe.leroy@csgroup.eu>
Also call kuap_lock() and kuap_save_and_lock() from
interrupt functions with CONFIG_PPC64.
For book3s/64 we keep them empty as it is done in assembly.
Also do the locked assert when switching task unless it is
book3s/64.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/book3s/64/kup.h | 9 +++++++++
arch/powerpc/include/asm/interrupt.h | 2 ++
arch/powerpc/include/asm/kup.h | 2 --
arch/powerpc/kernel/process.c | 6 +++---
4 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
index 503828709d55..69fcf63eec94 100644
--- a/arch/powerpc/include/asm/book3s/64/kup.h
+++ b/arch/powerpc/include/asm/book3s/64/kup.h
@@ -298,6 +298,15 @@ static inline unsigned long __kuap_get_and_assert_locked(void)
return amr;
}
+/* Do nothing, book3s/64 does that in ASM */
+static inline void __kuap_lock(void)
+{
+}
+
+static inline void __kuap_save_and_lock(struct pt_regs *regs)
+{
+}
+
/*
* We support individually allowing read or write, but we don't support nesting
* because that would require an expensive read/modify write of the AMR.
diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
index ae719e200c80..cd78dbca49e5 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -154,12 +154,14 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
if (user_mode(regs)) {
+ kuap_lock();
CT_WARN_ON(ct_state() != CONTEXT_USER);
user_exit_irqoff();
account_cpu_user_entry();
account_stolen_time();
} else {
+ kuap_save_and_lock(regs);
/*
* CT_WARN_ON comes here via program_check_exception,
* so avoid recursion.
diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
index 2e0c2df21b3b..0cafcf8319cd 100644
--- a/arch/powerpc/include/asm/kup.h
+++ b/arch/powerpc/include/asm/kup.h
@@ -94,7 +94,6 @@ static __always_inline void kuap_assert_locked(void)
__kuap_get_and_assert_locked();
}
-#ifdef CONFIG_PPC32
static __always_inline void kuap_lock(void)
{
if (kuap_is_disabled())
@@ -110,7 +109,6 @@ static __always_inline void kuap_save_and_lock(struct pt_regs *regs)
__kuap_save_and_lock(regs);
}
-#endif
static __always_inline void kuap_kernel_restore(struct pt_regs *regs, unsigned long amr)
{
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 50436b52c213..2c637740c0c2 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1281,9 +1281,9 @@ struct task_struct *__switch_to(struct task_struct *prev,
set_return_regs_changed(); /* _switch changes stack (and regs) */
-#ifdef CONFIG_PPC32
- kuap_assert_locked();
-#endif
+ if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64))
+ kuap_assert_locked();
+
last = _switch(old_thread, new_thread);
/*
--
2.31.1
^ permalink raw reply related
* [PATCH v2 08/16] powerpc/config: Add CONFIG_BOOKE_OR_40x
From: Christophe Leroy @ 2021-10-08 15:40 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1633707305.git.christophe.leroy@csgroup.eu>
We have many functionnalities common to 40x and BOOKE, it leads to
many places with #if defined(CONFIG_BOOKE) || defined(CONFIG_40x).
We are going to add a few more with KUAP for booke/40x, so create
a new symbol which is defined when either BOOKE or 40x is defined.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/hw_irq.h | 8 ++++----
arch/powerpc/include/asm/irq.h | 2 +-
arch/powerpc/include/asm/ptrace.h | 2 +-
arch/powerpc/include/asm/reg.h | 4 ++--
arch/powerpc/kernel/asm-offsets.c | 2 +-
arch/powerpc/kernel/entry_32.S | 2 +-
arch/powerpc/kernel/irq.c | 2 +-
arch/powerpc/kernel/kgdb.c | 4 ++--
arch/powerpc/kernel/setup.h | 2 +-
arch/powerpc/kernel/setup_32.c | 2 +-
arch/powerpc/kernel/time.c | 2 +-
arch/powerpc/platforms/Kconfig.cputype | 5 +++++
12 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
index 21cc571ea9c2..276e9dd7348b 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -61,7 +61,7 @@
static inline void __hard_irq_enable(void)
{
- if (IS_ENABLED(CONFIG_BOOKE) || IS_ENABLED(CONFIG_40x))
+ if (IS_ENABLED(CONFIG_BOOKE_OR_40x))
wrtee(MSR_EE);
else if (IS_ENABLED(CONFIG_PPC_8xx))
wrtspr(SPRN_EIE);
@@ -73,7 +73,7 @@ static inline void __hard_irq_enable(void)
static inline void __hard_irq_disable(void)
{
- if (IS_ENABLED(CONFIG_BOOKE) || IS_ENABLED(CONFIG_40x))
+ if (IS_ENABLED(CONFIG_BOOKE_OR_40x))
wrtee(0);
else if (IS_ENABLED(CONFIG_PPC_8xx))
wrtspr(SPRN_EID);
@@ -85,7 +85,7 @@ static inline void __hard_irq_disable(void)
static inline void __hard_EE_RI_disable(void)
{
- if (IS_ENABLED(CONFIG_BOOKE) || IS_ENABLED(CONFIG_40x))
+ if (IS_ENABLED(CONFIG_BOOKE_OR_40x))
wrtee(0);
else if (IS_ENABLED(CONFIG_PPC_8xx))
wrtspr(SPRN_NRI);
@@ -97,7 +97,7 @@ static inline void __hard_EE_RI_disable(void)
static inline void __hard_RI_enable(void)
{
- if (IS_ENABLED(CONFIG_BOOKE) || IS_ENABLED(CONFIG_40x))
+ if (IS_ENABLED(CONFIG_BOOKE_OR_40x))
return;
if (IS_ENABLED(CONFIG_PPC_8xx))
diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index 2b3278534bc1..13f0409dd617 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -36,7 +36,7 @@ extern int distribute_irqs;
struct pt_regs;
-#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
+#ifdef CONFIG_BOOKE_OR_40x
/*
* Per-cpu stacks for handling critical, debug and machine check
* level interrupts.
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 6e560f035614..42f89e2d8f04 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -291,7 +291,7 @@ static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc)
static inline bool cpu_has_msr_ri(void)
{
- return !IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x);
+ return !IS_ENABLED(CONFIG_BOOKE_OR_40x);
}
static inline bool regs_is_unrecoverable(struct pt_regs *regs)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index e9d27265253b..50478738c8f1 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -18,9 +18,9 @@
#include <asm/feature-fixups.h>
/* Pickup Book E specific registers. */
-#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
+#ifdef CONFIG_BOOKE_OR_40x
#include <asm/reg_booke.h>
-#endif /* CONFIG_BOOKE || CONFIG_40x */
+#endif
#ifdef CONFIG_FSL_EMB_PERFMON
#include <asm/reg_fsl_emb.h>
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index e563d3222d69..cf4a94891bd0 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -56,7 +56,7 @@
#endif
#ifdef CONFIG_PPC32
-#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
+#ifdef CONFIG_BOOKE_OR_40x
#include "head_booke.h"
#endif
#endif
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 9d31ba2af901..df01da3ca3fa 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -108,7 +108,7 @@ transfer_to_syscall:
stw r11, 0(r1)
mflr r12
stw r12, _LINK(r1)
-#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
+#ifdef CONFIG_BOOKE_OR_40x
rlwinm r9,r9,0,14,12 /* clear MSR_WE (necessary?) */
#endif
lis r12,STACK_FRAME_REGS_MARKER@ha /* exception frame marker */
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index c4f1d6b7d992..8207f97d51e8 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -811,7 +811,7 @@ void __init init_IRQ(void)
ppc_md.init_IRQ();
}
-#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
+#ifdef CONFIG_BOOKE_OR_40x
void *critirq_ctx[NR_CPUS] __read_mostly;
void *dbgirq_ctx[NR_CPUS] __read_mostly;
void *mcheckirq_ctx[NR_CPUS] __read_mostly;
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index bdee7262c080..9f8d0fa7b718 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -48,7 +48,7 @@ static struct hard_trap_info
{ 0x0800, 0x08 /* SIGFPE */ }, /* fp unavailable */
{ 0x0900, 0x0e /* SIGALRM */ }, /* decrementer */
{ 0x0c00, 0x14 /* SIGCHLD */ }, /* system call */
-#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+#ifdef CONFIG_BOOKE_OR_40x
{ 0x2002, 0x05 /* SIGTRAP */ }, /* debug */
#if defined(CONFIG_FSL_BOOKE)
{ 0x2010, 0x08 /* SIGFPE */ }, /* spe unavailable */
@@ -67,7 +67,7 @@ static struct hard_trap_info
{ 0x2010, 0x08 /* SIGFPE */ }, /* fp unavailable */
{ 0x2020, 0x08 /* SIGFPE */ }, /* ap unavailable */
#endif
-#else /* ! (defined(CONFIG_40x) || defined(CONFIG_BOOKE)) */
+#else /* !CONFIG_BOOKE_OR_40x */
{ 0x0d00, 0x05 /* SIGTRAP */ }, /* single-step */
#if defined(CONFIG_PPC_8xx)
{ 0x1000, 0x04 /* SIGILL */ }, /* software emulation */
diff --git a/arch/powerpc/kernel/setup.h b/arch/powerpc/kernel/setup.h
index 84058bbc8fe9..93f22da12abe 100644
--- a/arch/powerpc/kernel/setup.h
+++ b/arch/powerpc/kernel/setup.h
@@ -29,7 +29,7 @@ void setup_tlb_core_data(void);
static inline void setup_tlb_core_data(void) { }
#endif
-#if defined(CONFIG_PPC_BOOK3E) || defined(CONFIG_BOOKE) || defined(CONFIG_40x)
+#ifdef CONFIG_BOOKE_OR_40x
void exc_lvl_early_init(void);
#else
static inline void exc_lvl_early_init(void) { }
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index 7ec5c47fce0e..15e7386584f9 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -175,7 +175,7 @@ void __init emergency_stack_init(void)
}
#endif
-#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
+#ifdef CONFIG_BOOKE_OR_40x
void __init exc_lvl_early_init(void)
{
unsigned int i, hw_cpu;
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 934d8ae66cc6..f7bb2866a1c4 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -738,7 +738,7 @@ static int __init get_freq(char *name, int cells, unsigned long *val)
static void start_cpu_decrementer(void)
{
-#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
+#ifdef CONFIG_BOOKE_OR_40x
unsigned int tcr;
/* Clear any pending timer interrupts */
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index a208997ade88..dce1cf31047b 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -278,6 +278,11 @@ config BOOKE
depends on E500 || 44x || PPC_BOOK3E
default y
+config BOOKE_OR_40x
+ bool
+ depends on BOOKE || 40x
+ default y
+
config FSL_BOOKE
bool
depends on E500 && PPC32
--
2.31.1
^ permalink raw reply related
* [PATCH v2 15/16] powerpc/kuap: Wire-up KUAP on book3e/64
From: Christophe Leroy @ 2021-10-08 15:40 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1633707305.git.christophe.leroy@csgroup.eu>
This adds KUAP support to book3e/64.
This is done by reading the content of SPRN_MAS1 and checking
the TID at the time user pgtable is loaded.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/mm/nohash/tlb_low_64e.S | 40 ++++++++++++++++++++++----
arch/powerpc/platforms/Kconfig.cputype | 1 +
2 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/mm/nohash/tlb_low_64e.S b/arch/powerpc/mm/nohash/tlb_low_64e.S
index bf24451f3e71..b43524ca2b95 100644
--- a/arch/powerpc/mm/nohash/tlb_low_64e.S
+++ b/arch/powerpc/mm/nohash/tlb_low_64e.S
@@ -128,6 +128,13 @@ END_BTB_FLUSH_SECTION
bne tlb_miss_kernel_bolted
+tlb_miss_user_bolted:
+#ifdef CONFIG_PPC_KUAP
+ mfspr r10,SPRN_MAS1
+ rlwinm. r10,r10,0,0x3fff0000
+ beq- tlb_miss_fault_bolted /* KUAP fault */
+#endif
+
tlb_miss_common_bolted:
/*
* This is the guts of the TLB miss handler for bolted-linear.
@@ -246,7 +253,7 @@ itlb_miss_fault_bolted:
cmpldi cr0,r15,0 /* Check for user region */
oris r11,r11,_PAGE_ACCESSED@h
- beq tlb_miss_common_bolted
+ beq tlb_miss_user_bolted
b itlb_miss_kernel_bolted
#ifdef CONFIG_PPC_FSL_BOOK3E
@@ -676,6 +683,11 @@ finish_normal_tlb_miss:
/* Check if required permissions are met */
andc. r15,r11,r14
bne- normal_tlb_miss_access_fault
+#ifdef CONFIG_PPC_KUAP
+ mfspr r11,SPRN_MAS1
+ rlwinm. r10,r11,0,0x3fff0000
+ beq- normal_tlb_miss_access_fault /* KUAP fault */
+#endif
/* Now we build the MAS:
*
@@ -689,15 +701,17 @@ finish_normal_tlb_miss:
*
* TODO: mix up code below for better scheduling
*/
- clrrdi r11,r16,12 /* Clear low crap in EA */
- rlwimi r11,r14,32-19,27,31 /* Insert WIMGE */
- mtspr SPRN_MAS2,r11
+ clrrdi r10,r16,12 /* Clear low crap in EA */
+ rlwimi r10,r14,32-19,27,31 /* Insert WIMGE */
+ mtspr SPRN_MAS2,r10
/* Check page size, if not standard, update MAS1 */
- rldicl r11,r14,64-8,64-8
- cmpldi cr0,r11,BOOK3E_PAGESZ_4K
+ rldicl r10,r14,64-8,64-8
+ cmpldi cr0,r10,BOOK3E_PAGESZ_4K
beq- 1f
+#ifndef CONFIG_PPC_KUAP
mfspr r11,SPRN_MAS1
+#endif
rlwimi r11,r14,31,21,24
rlwinm r11,r11,0,21,19
mtspr SPRN_MAS1,r11
@@ -786,7 +800,16 @@ virt_page_table_tlb_miss:
mfspr r10,SPRN_MAS1
rlwinm r10,r10,0,16,1 /* Clear TID */
mtspr SPRN_MAS1,r10
+#ifdef CONFIG_PPC_KUAP
+ b 2f
+1:
+ mfspr r10,SPRN_MAS1
+ rlwinm. r10,r10,0,0x3fff0000
+ beq- virt_page_table_tlb_miss_fault /* KUAP fault */
+2:
+#else
1:
+#endif
BEGIN_MMU_FTR_SECTION
/* Search if we already have a TLB entry for that virtual address, and
* if we do, bail out.
@@ -1027,6 +1050,11 @@ virt_page_table_tlb_miss_whacko_fault:
* avoid too much complication, it will save/restore things for us
*/
htw_tlb_miss:
+#ifdef CONFIG_PPC_KUAP
+ mfspr r10,SPRN_MAS1
+ rlwinm. r10,r10,0,0x3fff0000
+ beq- htw_tlb_miss_fault /* KUAP fault */
+#endif
/* Search if we already have a TLB entry for that virtual address, and
* if we do, bail out.
*
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 4ac99614d865..d0e52ad45d73 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -118,6 +118,7 @@ config PPC_BOOK3E_64
select PPC_SMP_MUXED_IPI
select PPC_DOORBELL
select ZONE_DMA
+ select PPC_HAVE_KUAP
endchoice
--
2.31.1
^ permalink raw reply related
* [PATCH v2 12/16] powerpc/kuap: Wire-up KUAP on 44x
From: Christophe Leroy @ 2021-10-08 15:40 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1633707305.git.christophe.leroy@csgroup.eu>
This adds KUAP support to 44x. This is done by checking
the content of SPRN_PID at the time it is read and written
into SPRN_MMUCR.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/head_44x.S | 16 ++++++++++++++++
arch/powerpc/platforms/Kconfig.cputype | 1 +
2 files changed, 17 insertions(+)
diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
index 02d2928d1e01..cf92a3434acd 100644
--- a/arch/powerpc/kernel/head_44x.S
+++ b/arch/powerpc/kernel/head_44x.S
@@ -334,6 +334,10 @@ interrupt_base:
mfspr r12,SPRN_MMUCR
mfspr r13,SPRN_PID /* Get PID */
rlwimi r12,r13,0,24,31 /* Set TID */
+#ifdef CONFIG_PPC_KUAP
+ cmpwi r13,0
+ beq 2f /* KUAP Fault */
+#endif
4:
mtspr SPRN_MMUCR,r12
@@ -444,6 +448,10 @@ interrupt_base:
mfspr r12,SPRN_MMUCR
mfspr r13,SPRN_PID /* Get PID */
rlwimi r12,r13,0,24,31 /* Set TID */
+#ifdef CONFIG_PPC_KUAP
+ cmpwi r13,0
+ beq 2f /* KUAP Fault */
+#endif
4:
mtspr SPRN_MMUCR,r12
@@ -575,6 +583,10 @@ finish_tlb_load_44x:
3: mfspr r11,SPRN_SPRG3
lwz r11,PGDIR(r11)
mfspr r12,SPRN_PID /* Get PID */
+#ifdef CONFIG_PPC_KUAP
+ cmpwi r12,0
+ beq 2f /* KUAP Fault */
+#endif
4: mtspr SPRN_MMUCR,r12 /* Set MMUCR */
/* Mask of required permission bits. Note that while we
@@ -672,6 +684,10 @@ finish_tlb_load_44x:
3: mfspr r11,SPRN_SPRG_THREAD
lwz r11,PGDIR(r11)
mfspr r12,SPRN_PID /* Get PID */
+#ifdef CONFIG_PPC_KUAP
+ cmpwi r12,0
+ beq 2f /* KUAP Fault */
+#endif
4: mtspr SPRN_MMUCR,r12 /* Set MMUCR */
/* Make up the required permissions */
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index a66ab0a991f9..30091551ab24 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -62,6 +62,7 @@ config 44x
select HAVE_PCI
select PHYS_64BIT
select PPC_HAVE_KUEP
+ select PPC_HAVE_KUAP
endchoice
--
2.31.1
^ permalink raw reply related
* Re: [PATCH 0/5] powerpc: various interrupt handling fixes
From: Michael Ellerman @ 2021-10-08 13:23 UTC (permalink / raw)
To: linuxppc-dev, Nicholas Piggin; +Cc: Ganesh Goudar
In-Reply-To: <20211004145642.1331214-1-npiggin@gmail.com>
On Tue, 5 Oct 2021 00:56:37 +1000, Nicholas Piggin wrote:
> This fixes a number of bugs found mostly looking at a MCE handler issue,
> which should be fixed in patch 5 of the series, previous attempt here
> which Ganesh found to be wrong.
>
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210922020247.209409-1-npiggin@gmail.com/
>
> I didn't increment to patch v2 because it's a different approach (so I
> gave it a different title).
>
> [...]
Applied to powerpc/fixes.
[1/5] powerpc/64s: fix program check interrupt emergency stack path
https://git.kernel.org/powerpc/c/3e607dc4df180b72a38e75030cb0f94d12808712
[2/5] powerpc/traps: do not enable irqs in _exception
https://git.kernel.org/powerpc/c/d0afd44c05f8f4e4c91487c02d43c87a31552462
[3/5] powerpc/64: warn if local irqs are enabled in NMI or hardirq context
https://git.kernel.org/powerpc/c/ff058a8ada5df0d84e5537cfaf89d06d71501580
[4/5] powerpc/64/interrupt: Reconcile soft-mask state in NMI and fix false BUG
https://git.kernel.org/powerpc/c/768c47010392ece9766a56479b4e0cf04a536916
[5/5] powerpc/64s: Fix unrecoverable MCE calling async handler from NMI
https://git.kernel.org/powerpc/c/f08fb25bc66986b0952724530a640d9970fa52c1
cheers
^ permalink raw reply
* Re: [PATCH v2 00/10] powerpc/bpf: Various fixes
From: Michael Ellerman @ 2021-10-08 13:22 UTC (permalink / raw)
To: Song Liu, Nicholas Piggin, Jordan Niethe, Alexei Starovoitov,
Naveen N. Rao, Michael Ellerman, Christophe Leroy, Johan Almbladh,
Daniel Borkmann
Cc: bpf, linuxppc-dev
In-Reply-To: <cover.1633464148.git.naveen.n.rao@linux.vnet.ibm.com>
On Wed, 6 Oct 2021 01:55:19 +0530, Naveen N. Rao wrote:
> This is v2 of the series posted at:
> http://lkml.kernel.org/r/cover.1633104510.git.naveen.n.rao@linux.vnet.ibm.com
>
> Only patches from v1 that need to go into powerpc/fixes are included.
> Other patches will be posted as a separate series for inclusion into
> powerpc/next.
>
> [...]
Applied to powerpc/fixes.
[01/10] powerpc/lib: Add helper to check if offset is within conditional branch range
https://git.kernel.org/powerpc/c/4549c3ea3160fa8b3f37dfe2f957657bb265eda9
[02/10] powerpc/bpf: Validate branch ranges
https://git.kernel.org/powerpc/c/3832ba4e283d7052b783dab8311df7e3590fed93
[03/10] powerpc/bpf: Fix BPF_MOD when imm == 1
https://git.kernel.org/powerpc/c/8bbc9d822421d9ac8ff9ed26a3713c9afc69d6c8
[04/10] powerpc/bpf: Fix BPF_SUB when imm == 0x80000000
https://git.kernel.org/powerpc/c/5855c4c1f415ca3ba1046e77c0b3d3dfc96c9025
[05/10] powerpc/security: Add a helper to query stf_barrier type
https://git.kernel.org/powerpc/c/030905920f32e91a52794937f67434ac0b3ea41a
[06/10] powerpc/bpf: Emit stf barrier instruction sequences for BPF_NOSPEC
https://git.kernel.org/powerpc/c/b7540d62509453263604a155bf2d5f0ed450cba2
[07/10] powerpc/bpf ppc32: Fix ALU32 BPF_ARSH operation
https://git.kernel.org/powerpc/c/c9b8da77f22d28348d1f89a6c4d3fec102e9b1c4
[08/10] powerpc/bpf ppc32: Fix JMP32_JSET_K
https://git.kernel.org/powerpc/c/e8278d44443207bb6609c7b064073f353e6f4978
[09/10] powerpc/bpf ppc32: Do not emit zero extend instruction for 64-bit BPF_END
https://git.kernel.org/powerpc/c/48164fccdff6d5cc11308126c050bd25a329df25
[10/10] powerpc/bpf ppc32: Fix BPF_SUB when imm == 0x80000000
https://git.kernel.org/powerpc/c/548b762763b885b81850db676258df47c55dd5f9
cheers
^ permalink raw reply
* Re: [PATCH] pseries/eeh: fix the kdump kernel crash during eeh_pseries_init
From: Michael Ellerman @ 2021-10-08 13:23 UTC (permalink / raw)
To: Mahesh Salgaonkar, linuxppc-dev; +Cc: Oliver O'Halloran, Wen Xiong
In-Reply-To: <163215558252.413351.8600189949820258982.stgit@jupiter>
On Mon, 20 Sep 2021 22:03:26 +0530, Mahesh Salgaonkar wrote:
> On pseries lpar when an empty slot is assigned to partition OR on single
> lpar mode, kdump kernel crashes during issuing PHB reset. In the kdump
> scenario, we traverse all PHBs and issue reset using the pe_config_addr of
> first child device present under each PHB. However the code assumes that
> none of the PHB slot can be empty and uses list_first_entry() to get first
> child device under PHB. Since list_first_entry() expect list to be not
> empty, it returns invalid pci_dn entry and ends up accessing NULL phb
> pointer under pci_dn->phb causing kdump kernel crash.
>
> [...]
Applied to powerpc/fixes.
[1/1] pseries/eeh: fix the kdump kernel crash during eeh_pseries_init
https://git.kernel.org/powerpc/c/eb8257a12192f43ffd41bd90932c39dade958042
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/pseries/msi: Add an empty irq_write_msi_msg() handler
From: Michael Ellerman @ 2021-10-08 13:23 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev; +Cc: Brian King, Wen Xiong, Douglas Miller
In-Reply-To: <20210930102535.1047230-1-clg@kaod.org>
On Thu, 30 Sep 2021 12:25:35 +0200, Cédric Le Goater wrote:
> The IPR drivers tests for MSI support at probe time with MSI vector 0
> and when done, frees the IRQ with free_irq(). This test was introduced
> by 95fecd90397e ("ipr: add test for MSI interrupt support") as an
> improvement of commit 5a9ef25b14d3 ("[SCSI] ipr: add MSI support")
> because a boot failure was reported on a Bimini PowerPC system :
>
> https://x-lore.kernel.org/all/1242926159.3007.5.camel@localhost.localdomain/
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/pseries/msi: Add an empty irq_write_msi_msg() handler
https://git.kernel.org/powerpc/c/5a4b0320783a19f877dd595813569b3c25f4ff81
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/32s: Fix kuap_kernel_restore()
From: Michael Ellerman @ 2021-10-08 13:23 UTC (permalink / raw)
To: Paul Mackerras, Michael Ellerman, Christophe Leroy,
Benjamin Herrenschmidt
Cc: Stan Johnson, linuxppc-dev, linux-kernel, Finn Thain
In-Reply-To: <0d0c4d0f050a637052287c09ba521bad960a2790.1631715131.git.christophe.leroy@csgroup.eu>
On Wed, 15 Sep 2021 16:12:24 +0200, Christophe Leroy wrote:
> At interrupt exit, kuap_kernel_restore() calls kuap_unclok() with the
> value contained in regs->kuap. However, when regs->kuap contains
> 0xffffffff it means that KUAP was not unlocked so calling
> kuap_unlock() is unrelevant and results in jeopardising the contents
> of kernel space segment registers.
>
> So check that regs->kuap doesn't contain KUAP_NONE before calling
> kuap_unlock(). In the meantime it also means that if KUAP has not
> been correcly locked back at interrupt exit, it must be locked
> before continuing. This is done by checking the content of
> current->thread.kuap which was returned by kuap_get_and_assert_locked()
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/32s: Fix kuap_kernel_restore()
https://git.kernel.org/powerpc/c/d93f9e23744b7bf11a98b2ddb091d129482ae179
cheers
^ permalink raw reply
* Re: [PATCH 07/12] powerpc: Use of_get_cpu_hwid()
From: Michael Ellerman @ 2021-10-08 11:01 UTC (permalink / raw)
To: Rob Herring, Russell King, James Morse, Catalin Marinas,
Will Deacon, Guo Ren, Jonas Bonn, Stefan Kristiansson,
Stafford Horne, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Yoshinori Sato, Rich Felker, x86, Greg Kroah-Hartman
Cc: devicetree, Florian Fainelli, Scott Branden, Rafael J. Wysocki,
linux-sh, Ray Jui, H. Peter Anvin, linux-kernel, linux-csky,
openrisc, linuxppc-dev, Ingo Molnar, Paul Mackerras,
Borislav Petkov, bcm-kernel-feedback-list, Thomas Gleixner,
Frank Rowand, linux-riscv, linux-arm-kernel
In-Reply-To: <20211006164332.1981454-8-robh@kernel.org>
Rob Herring <robh@kernel.org> writes:
> Replace open coded parsing of CPU nodes' 'reg' property with
> of_get_cpu_hwid().
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> arch/powerpc/kernel/smp.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 9cc7d3dbf439..d96b0e361a73 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1313,18 +1313,13 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
> int cpu_to_core_id(int cpu)
> {
> struct device_node *np;
> - const __be32 *reg;
> int id = -1;
>
> np = of_get_cpu_node(cpu, NULL);
> if (!np)
> goto out;
>
> - reg = of_get_property(np, "reg", NULL);
> - if (!reg)
> - goto out;
> -
> - id = be32_to_cpup(reg);
> + id = of_get_cpu_hwid(np, 0);
> out:
> of_node_put(np);
> return id;
This looks OK to me.
All the systems I can find have a /cpus/#address-cells of 1, so the
change to use of_n_addr_cells() in of_get_cpu_hwid() should be fine.
I booted it on a bunch of systems with no issues.
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
^ permalink raw reply
* Re: [PATCH v6 19/22] powerpc/book3s64/hash/kuap: Enable kuap on hash
From: Christophe Leroy @ 2021-10-08 9:32 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Sandipan Das
In-Reply-To: <20201125051634.509286-20-aneesh.kumar@linux.ibm.com>
Le 25/11/2020 à 06:16, Aneesh Kumar K.V a écrit :
> Reviewed-by: Sandipan Das <sandipan@linux.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/mm/book3s64/pkeys.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index f747d66cc87d..84f8664ffc47 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -257,7 +257,12 @@ void __init setup_kuep(bool disabled)
> #ifdef CONFIG_PPC_KUAP
> void __init setup_kuap(bool disabled)
> {
> - if (disabled || !early_radix_enabled())
> + if (disabled)
> + return;
> + /*
> + * On hash if PKEY feature is not enabled, disable KUAP too.
> + */
> + if (!early_radix_enabled() && !early_mmu_has_feature(MMU_FTR_PKEY))
pkey_early_init_devtree() bails out without setting MMU_FTR_PKEY with
the following:
/*
* Only P7 and above supports SPRN_AMR update with MSR[PR] = 1
*/
if (!early_cpu_has_feature(CPU_FTR_ARCH_206))
return;
Why would it be impossible to do KUAP in that case ? KUAP doesn't
require updating SPRN_AMR with MSR[PR] = 1
> return;
>
> if (smp_processor_id() == boot_cpuid) {
>
^ permalink raw reply
* Re: [PATCH] perf vendor events power10: Add metric events json file for power10 platform
From: kajoljain @ 2021-10-08 7:48 UTC (permalink / raw)
To: Paul A. Clarke
Cc: maddy, rnsastry, jolsa, linux-kernel, acme, linux-perf-users,
atrajeev, linuxppc-dev
In-Reply-To: <20211006173248.GA7389@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com>
On 10/6/21 11:02 PM, Paul A. Clarke wrote:
> Kajol,
>
> On Wed, Oct 06, 2021 at 01:01:19PM +0530, Kajol Jain wrote:
>> Add pmu metric json file for power10 platform.
>
> Thanks for producing this! A few minor corrections, plus a number of
> stylistic comments below...
Hi Paul,
I will make mentioned nit changes and send a v2.
Thanks,
Kajol Jain
>
>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>> ---
>> .../arch/powerpc/power10/metrics.json | 772 ++++++++++++++++++
>> 1 file changed, 772 insertions(+)
>> create mode 100644 tools/perf/pmu-events/arch/powerpc/power10/metrics.json
>>
>> diff --git a/tools/perf/pmu-events/arch/powerpc/power10/metrics.json b/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
>> new file mode 100644
>> index 000000000000..028c9777a516
>> --- /dev/null
>> +++ b/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
>> @@ -0,0 +1,772 @@
>> +[
>> + {
>> + "BriefDescription": "Percentage of cycles that are run cycles",
>> + "MetricExpr": "PM_RUN_CYC / PM_CYC * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "RUN_CYCLES_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per completed instruction",
>> + "MetricExpr": "PM_CYC / PM_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "CYCLES_PER_INSTRUCTION"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled for any reason",
>> + "MetricExpr": "PM_DISP_STALL_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled because there was a flush",
>> + "MetricExpr": "PM_DISP_STALL_FLUSH / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_FLUSH_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled because the MMU was handling a translation miss",
>> + "MetricExpr": "PM_DISP_STALL_TRANSLATION / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_TRANSLATION_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled waiting to resolve an instruction ERAT miss",
>> + "MetricExpr": "PM_DISP_STALL_IERAT_ONLY_MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_IERAT_ONLY_MISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled waiting to resolve an instruction TLB miss",
>> + "MetricExpr": "PM_DISP_STALL_ITLB_MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_ITLB_MISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled due to an icache miss",
>> + "MetricExpr": "PM_DISP_STALL_IC_MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_IC_MISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled while the instruction was fetched form the local L2",
>
> s/form/from/
>
>> + "MetricExpr": "PM_DISP_STALL_IC_L2 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_IC_L2_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled while the instruction was fetched form the local L3",
>
> s/form/from/
>
>> + "MetricExpr": "PM_DISP_STALL_IC_L3 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_IC_L3_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled while the instruction was fetched from any source beyond the local L3",
>> + "MetricExpr": "PM_DISP_STALL_IC_L3MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_IC_L3MISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled due to an icache miss after a branch mispredict",
>> + "MetricExpr": "PM_DISP_STALL_BR_MPRED_ICMISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_BR_MPRED_ICMISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled while instruction was fetched from the local L2 after suffering a branch mispredict",
>> + "MetricExpr": "PM_DISP_STALL_BR_MPRED_IC_L2 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_BR_MPRED_IC_L2_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled while instruction was fetched from the local L3 after suffering a branch mispredict",
>> + "MetricExpr": "PM_DISP_STALL_BR_MPRED_IC_L3 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_BR_MPRED_IC_L3_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled while instruction was fetched from any source beyond the local L3 after suffering a branch mispredict",
>
> extra space after "beyond"
>
>> + "MetricExpr": "PM_DISP_STALL_BR_MPRED_IC_L3MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_BR_MPRED_IC_L3MISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled due to a branch mispredict",
>> + "MetricExpr": "PM_DISP_STALL_BR_MPRED / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_BR_MPRED_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch for any reason",
>
> s/ntc/NTC/ or "next-to-complete"
> I do see uses of "NTC" below.
> Same comment for other instances of "ntc", below...
>
>> + "MetricExpr": "PM_DISP_STALL_HELD_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_HELD_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch because of a synchronizing instruction that requires the ICT to be empty before dispatch",
>> + "MetricExpr": "PM_DISP_STALL_HELD_SYNC_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISP_HELD_STALL_SYNC_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch while waiting on the scoreboard",
>> + "MetricExpr": "PM_DISP_STALL_HELD_SCOREBOARD_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISP_HELD_STALL_SCOREBOARD_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch due to issue q full",
>
> s/q/queue/
>
>> + "MetricExpr": "PM_DISP_STALL_HELD_ISSQ_FULL_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISP_HELD_STALL_ISSQ_FULL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch because the mapper/SRB was full",
>> + "MetricExpr": "PM_DISP_STALL_HELD_RENAME_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_HELD_RENAME_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch because the STF mapper/SRB was full",
>> + "MetricExpr": "PM_DISP_STALL_HELD_STF_MAPPER_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_HELD_STF_MAPPER_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch because the XVFC mapper/SRB was full",
>> + "MetricExpr": "PM_DISP_STALL_HELD_XVFC_MAPPER_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_HELD_XVFC_MAPPER_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch for any other reason",
>> + "MetricExpr": "PM_DISP_STALL_HELD_OTHER_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_HELD_OTHER_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction has been dispatched but not issued for any reason",
>> + "MetricExpr": "PM_ISSUE_STALL / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "ISSUE_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting to be finished in one of the execution units",
>> + "MetricExpr": "PM_EXEC_STALL / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "EXECUTION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction spent executing an NTC instruction that gets flushed some time after dispatch",
>> + "MetricExpr": "PM_EXEC_STALL_NTC_FLUSH / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "NTC_FLUSH_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the instruction finishes at dispatch",
>
> I'm not sure what that means.
>
>> + "MetricExpr": "PM_EXEC_STALL_FIN_AT_DISP / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "FIN_AT_DISP_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is executing in the branch unit",
>> + "MetricExpr": "PM_EXEC_STALL_BRU / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "BRU_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a simple fixed point instr that is executing in the lsu unit",
>
> s/instr/instruction/
> s/lsu unit/LSU/
>
>> + "MetricExpr": "PM_EXEC_STALL_SIMPLE_FX / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "SIMPLE_FX_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is executing in the vsu unit",
>
> s/vsu unit/VSU/
>
>> + "MetricExpr": "PM_EXEC_STALL_VSU / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "VSU_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting to be finished in one of the execution units",
>> + "MetricExpr": "PM_EXEC_STALL_TRANSLATION / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "TRANSLATION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a load or store that suffered a translation miss",
>> + "MetricExpr": "PM_EXEC_STALL_DERAT_ONLY_MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DERAT_ONLY_MISS_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is recovering from a TLB miss",
>> + "MetricExpr": "PM_EXEC_STALL_DERAT_DTLB_MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DERAT_DTLB_MISS_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is executing in the lsu unit",
>
> s/lsu unit/LSU/
>
>> + "MetricExpr": "PM_EXEC_STALL_LSU / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "LSU_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a load that is executing in the lsu unit",
>
> s/lsu unit/LSU/
>
>> + "MetricExpr": "PM_EXEC_STALL_LOAD / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "LOAD_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from either the local L2 or local L3",
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_L2L3 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_L2L3_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from either the local L2 or local L3, with an RC dispatch conflict",
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_L2L3_CONFLICT / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_L2L3_CONFLICT_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from either the local L2 or local L3, without an RC dispatch conflict",
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_L2L3_NOCONFLICT / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_L2L3_NOCONFLICT_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from a source beyond the local L2 and local L3",
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_L3MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_L3MISS_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from a neighbor chiplet's L2 or L3 in the same chip",
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_L21_L31 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_L21_L31_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from local memory, L4 or OpenCapp chip",
>
> Most descriptions put L4 before memory.
> (My preference is to use an "Oxford comma", as in "memory, L4, or ..."
> (comma after "L4"), but acknowledge there are those who prefer otherwise.)
>
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_LMEM / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_LMEM_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from a remote chip (cache, L4, memory or CAPP) in the same group",
>
> Is there a distinction between "OpenCapp" and "CAPP"? If not, pick one throughout.
> Is this supposed to be "OpenCAPI"?
>
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_OFF_CHIP / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_OFF_CHIP_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from a distant chip (cache, L4, memory or CAPP chip)",
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_OFF_NODE / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_OFF_NODE_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is executing a TLBIEL instruction",
>> + "MetricExpr": "PM_EXEC_STALL_TLBIEL / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "TLBIEL_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is finishing a load after its data has been reloaded from a data source beyond the local L1, OR when the LSU is processing an L1-hit, OR when the NTF instruction merged with another load in the LMQ",
>> + "MetricExpr": "PM_EXEC_STALL_LOAD_FINISH / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "LOAD_FINISH_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a store that is executing in the lsu unit",
>
> s/lsu unit/LSU/
>
>> + "MetricExpr": "PM_EXEC_STALL_STORE / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "STORE_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is in the store unit outside of handling store misses or other special store operations",
>
> s/store unit/LSU/ ?
>
>> + "MetricExpr": "PM_EXEC_STALL_STORE_PIPE / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "STORE_PIPE_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a store whose cache line was not resident in the L1 and had to wait for allocation of the missing line into the L1",
>> + "MetricExpr": "PM_EXEC_STALL_STORE_MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "STORE_MISS_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a TLBIE instruction waiting for a response from the L2",
>> + "MetricExpr": "PM_EXEC_STALL_TLBIE / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "TLBIE_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is executing a PTESYNC instruction",
>> + "MetricExpr": "PM_EXEC_STALL_PTESYNC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "PTESYNC_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction cannot complete because the thread was blocked",
>> + "MetricExpr": "PM_CMPL_STALL / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction cannot complete because it was interrupted by ANY exception",
>> + "MetricExpr": "PM_CMPL_STALL_EXCEPTION / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "EXCEPTION_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is stuck at finish waiting for the non-speculative finish of either a stcx waiting for its result or a load waiting for non-critical sectors of data and ECC",
>> + "MetricExpr": "PM_CMPL_STALL_MEM_ECC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "MEM_ECC_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction cannot complete the instruction is a stcx waiting for resolution from the nest",
>> + "MetricExpr": "PM_CMPL_STALL_STCX / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "STCX_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a LWSYNC instruction waiting to complete",
>
> Sometimes instruction mnemonics are ALL CAPS, like here, and sometimes not,
> like "stcx", above. Pick one style. Also pick whether the mnemonic is
> followed by "instruction" or not. I prefer including "instruction" for
> clarity.
>
>> + "MetricExpr": "PM_CMPL_STALL_LWSYNC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "LWSYNC_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a HWSYNC instruction stuck at finish waiting for a response from the L2",
>> + "MetricExpr": "PM_CMPL_STALL_HWSYNC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "HWSYNC_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction required special handling before completion",
>> + "MetricExpr": "PM_CMPL_STALL_SPECIAL / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "SPECIAL_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, disp_stall_translation or children are miscounting",
>
> Are these "Should equal 0" metrics generally useful?
>
>> + "MetricExpr": "DISPATCHED_TRANSLATION_CPI - (DISPATCHED_IERAT_ONLY_MISS_CPI + DISPATCHED_ITLB_MISS_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DISPATCHED_TRANSLATION_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, disp_stall_ic_miss or children are miscounting",
>> + "MetricExpr": "DISPATCHED_IC_MISS_CPI - (DISPATCHED_IC_L2_CPI + DISPATCHED_IC_L3_CPI + DISPATCHED_IC_L3MISS_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DISPATCHED_IC_MISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, disp_stall_br_mpred_icmiss or children are miscounting",
>> + "MetricExpr": "DISPATCHED_BR_MPRED_ICMISS_CPI - (DISPATCHED_BR_MPRED_IC_L2_CPI + DISPATCHED_BR_MPRED_IC_L3_CPI + DISPATCHED_BR_MPRED_IC_L3MISS_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DISPATCHED_BR_MPRED_ICMISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, disp_stall_held_rename or children are miscounting",
>> + "MetricExpr": "DISPATCHED_HELD_RENAME_CPI - (DISPATCHED_HELD_STF_MAPPER_CPI + DISPATCHED_HELD_XVFC_MAPPER_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DISPATCHED_HELD_RENAME_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, disp_stall_held or children are miscounting",
>> + "MetricExpr": "DISPATCHED_HELD_CPI - (DISP_HELD_STALL_SYNC_CPI + DISP_HELD_STALL_SCOREBOARD_CPI + DISP_HELD_STALL_ISSQ_FULL_CPI + DISPATCHED_HELD_RENAME_CPI + DISPATCHED_HELD_OTHER_CPI + DISPATCHED_HELD_HALT_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DISPATCHED_HELD_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, disp_stall or children are miscounting",
>> + "MetricExpr": "DISPATCHED_CPI - (DISPATCHED_FLUSH_CPI + DISPATCHED_TRANSLATION_CPI + DISPATCHED_IC_MISS_CPI + DISPATCHED_BR_MPRED_ICMISS_CPI + DISPATCHED_BR_MPRED_CPI + DISPATCHED_HELD_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DISPATCHED_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall_translation or children are miscounting",
>> + "MetricExpr": "TRANSLATION_STALL_CPI - (DERAT_ONLY_MISS_STALL_CPI + DERAT_DTLB_MISS_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_TRANSLATION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall_dmiss_l2l3 or children are miscounting",
>> + "MetricExpr": "DMISS_L2L3_STALL_CPI - (DMISS_L2L3_CONFLICT_STALL_CPI + DMISS_L2L3_NOCONFLICT_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DMISS_L2L3_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall_dmiss_l3miss or children are miscounting",
>> + "MetricExpr": "DMISS_L3MISS_STALL_CPI - (DMISS_L21_L31_STALL_CPI + DMISS_LMEM_STALL_CPI + DMISS_OFF_CHIP_STALL_CPI + DMISS_OFF_NODE_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DMISS_L3MISS_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall_load or children are miscounting",
>> + "MetricExpr": "LOAD_STALL_CPI - (DMISS_L2L3_STALL_CPI + DMISS_L3MISS_STALL_CPI + TLBIEL_STALL_CPI + LOAD_FINISH_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_LOAD_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall_store or children are miscounting",
>> + "MetricExpr": "STORE_STALL_CPI - (STORE_PIPE_STALL_CPI + STORE_MISS_STALL_CPI + TLBIE_STALL_CPI + PTESYNC_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_STORE_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall_lsu or children are miscounting",
>> + "MetricExpr": "LSU_STALL_CPI - (LOAD_STALL_CPI + STORE_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_LSU_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, cmpl_stall or children are miscounting",
>> + "MetricExpr": "COMPLETION_STALL_CPI - (EXCEPTION_COMPLETION_STALL_CPI + MEM_ECC_COMPLETION_STALL_CPI + STCX_COMPLETION_STALL_CPI + LWSYNC_COMPLETION_STALL_CPI + HWSYNC_COMPLETION_STALL_CPI + SPECIAL_COMPLETION_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall or children are miscounting",
>> + "MetricExpr": "EXECUTION_STALL_CPI - (NTC_FLUSH_STALL_CPI + FIN_AT_DISP_STALL_CPI + BRU_STALL_CPI + SIMPLE_FX_STALL_CPI + VSU_STALL_CPI + TRANSLATION_STALL_CPI + LSU_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, pm_cyc or children are miscounting",
>> + "MetricExpr": "CYCLES_PER_INSTRUCTION - (DISPATCHED_CPI + ISSUE_STALL_CPI + EXECUTION_STALL_CPI + COMPLETION_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled because Fetch was being held, so there was nothing in the pipeline for this thread",
>
> s/Fetch/fetch/
> extra space after "held,"
>
>> + "MetricExpr": "PM_DISP_STALL_FETCH / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_FETCH_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch because of power management",
>> + "MetricExpr": "PM_DISP_STALL_HELD_HALT_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_HELD_HALT_CPI"
>> + },
>> + {
>> + "BriefDescription": "Percentage of flushes per completed instruction",
>> + "MetricExpr": "PM_FLUSH / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Others",
>> + "MetricName": "FLUSH_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of flushes due to a branch mispredict per instruction",
>> + "MetricExpr": "PM_FLUSH_MPRED / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Others",
>> + "MetricName": "BR_MPRED_FLUSH_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of branch mispredictions per completed instruction",
>> + "MetricExpr": "PM_BR_MPRED_CMPL / PM_RUN_INST_CMPL",
>> + "MetricGroup": "Others",
>> + "MetricName": "BRANCH_MISPREDICTION_RATE"
>> + },
>> + {
>> + "BriefDescription": "Percentage of finished loads that missed in the L1",
>> + "MetricExpr": "PM_LD_MISS_L1 / PM_LD_REF_L1 * 100",
>> + "MetricGroup": "Others",
>> + "MetricName": "L1_LD_MISS_RATIO",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of completed instructions that were loads that missed the L1",
>> + "MetricExpr": "PM_LD_MISS_L1 / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Others",
>> + "MetricName": "L1_LD_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of instructions when the DPTEG required for the load/store instruction in execution was missing from the TLB",
>> + "MetricExpr": "PM_DTLB_MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Others",
>> + "MetricName": "DTLB_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Average number of instruction dispatched per instruction completed",
>
> s/instruction/instrucions/
>
>> + "MetricExpr": "PM_INST_DISP / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "DISPATCH_PER_INST_CMPL"
>> + },
>> + {
>> + "BriefDescription": "Percentage of completed instructions that were a demand load that did not hit in the L1 or L2",
>> + "MetricExpr": "PM_DATA_FROM_L2MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "L2_LD_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of completed instructions that were demand fetches that missed the L1 instruction cache",
>> + "MetricExpr": "PM_L1_ICACHE_MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Instruction_Misses",
>> + "MetricName": "L1_INST_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of completed instructions that were demand fetches that reloaded from beyond the L3 instruction cache",
>> + "MetricExpr": "PM_INST_FROM_L3MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "L3_INST_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Average number of completed instructions per cycle",
>> + "MetricExpr": "PM_INST_CMPL / PM_CYC",
>> + "MetricGroup": "General",
>> + "MetricName": "IPC"
>> + },
>> + {
>> + "BriefDescription": "Average number of cycles per completed instruction group",
>> + "MetricExpr": "PM_CYC / PM_1PLUS_PPC_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "CYCLES_PER_COMPLETED_INSTRUCTIONS_SET"
>> + },
>> + {
>> + "BriefDescription": "Percentage of cycles when at least 1 instruction dispatched",
>> + "MetricExpr": "PM_1PLUS_PPC_DISP / PM_RUN_CYC * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "CYCLES_ATLEAST_ONE_INST_DISPATCHED",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Rate of finished loads per completed instruction",
>
> Most similar "rate" metrics are using the phrase "average number of".
> Do we want to use that here as well? (Applies to all "rate" metrics.)
>
>> + "MetricExpr": "PM_LD_REF_L1 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "LOADS_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of finished stores per completed instruction",
>> + "MetricExpr": "PM_ST_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "STORES_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Percentage of demand loads that reloaded from beyond the L2 per completed instruction",
>> + "MetricExpr": "PM_DATA_FROM_L2MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "dL1_Reloads",
>> + "MetricName": "DL1_RELOAD_FROM_L2_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of demand loads that reloaded from beyond the L3 per completed instruction",
>> + "MetricExpr": "PM_DATA_FROM_L3MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "dL1_Reloads",
>> + "MetricName": "DL1_RELOAD_FROM_L3_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of DERAT misses with 4k page size per completed run instruction",
>
> When PM_RUN_INST_CMPL is used, sometimes we say "run instruction",
> and sometimes we say "completed instruction". Let's pick one.
>
>> + "MetricExpr": "PM_DERAT_MISS_4K / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_4K_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of DERAT misses with 64k page size per completed run instruction",
>> + "MetricExpr": "PM_DERAT_MISS_64K / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_64K_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Average number of run cycles per completed run instruction",
>
> Here we cover our bases and say "completed run instruction". ;-)
> Let's convert this one to whichever phrase is chosen for PM_RUN_INST_CMPL.
> Seen below, too.
>
>> + "MetricExpr": "PM_RUN_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "RUN_CPI"
>> + },
>> + {
>> + "BriefDescription": "Total number of run cycles",
>> + "MetricExpr": "PM_RUN_CYC",
>
> Isn't this more an event than a metric?
> Does it need to be included here?
>
>> + "MetricGroup": "General",
>> + "MetricName": "TOTAL_RUN_CYCLES"
>> + },
>> + {
>> + "BriefDescription": "Percentage of DERAT misses per completed run instruction",
>> + "MetricExpr": "PM_DERAT_MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Average number of completed run instructions per run cycle",
>> + "MetricExpr": "PM_RUN_INST_CMPL / PM_RUN_CYC",
>> + "MetricGroup": "General",
>> + "MetricName": "RUN_IPC"
>> + },
>> + {
>> + "BriefDescription": "Average number of instruction completed per instruction group",
>
> s/instruction/instructions/
>
>> + "MetricExpr": "PM_RUN_INST_CMPL / PM_1PLUS_PPC_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "AVERAGE_COMPLETED_INSTRUCTION_SET_SIZE"
>> + },
>> + {
>> + "BriefDescription": "Rate of finished instructions per completed instructions",
>
>
>> + "MetricExpr": "PM_INST_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "INST_FIN_PER_CMPL"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntf instruction is completing and the finish was overlooked",
>
> s/ntf/NTF/
> "overlooked" seems like an odd term.
>
>> + "MetricExpr": "PM_EXEC_STALL_UNKNOWN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "EXEC_STALL_UNKOWN_CPI"
>> + },
>> + {
>> + "BriefDescription": "Percentage of finished branches that were taken",
>> + "MetricExpr": "PM_BR_TAKEN_CMPL / PM_BR_FIN * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "TAKEN_BRANCHES",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of completed instructions that were a demand load that did not hit in the L1, L2, or the L3",
>> + "MetricExpr": "PM_DATA_FROM_L3MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "L3_LD_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Rate of finished branches per completed instruction",
>> + "MetricExpr": "PM_BR_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "BRANCHES_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of instructions finished in the LSU per completed instruction",
>> + "MetricExpr": "PM_LSU_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "LSU_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of instructions finished in the VSU per completed instruction",
>> + "MetricExpr": "PM_VSU_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "VSU_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of TLBIE instructions finished in the LSU per completed instruction",
>> + "MetricExpr": "PM_TLBIE_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "TLBIE_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of STCX instructions finshed per completed instruction",
>> + "MetricExpr": "PM_STCX_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "STXC_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of LARX instructions finshed per completed instruction",
>> + "MetricExpr": "PM_LARX_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "LARX_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of ptesync instructions finshed per completed instruction",
>> + "MetricExpr": "PM_PTESYNC_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "PTESYNC_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of simple fixed-point instructions finshed in the store unit per completed instruction",
>
> s/store unit/LSU/ ?
>
>> + "MetricExpr": "PM_FX_LSU_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "FX_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Percentage of demand load misses that reloaded the L1 cache",
>> + "MetricExpr": "PM_LD_DEMAND_MISS_L1 / PM_LD_MISS_L1 * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "DL1_MISS_RELOADS",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of demand load misses that reloaded from beyond the local L2",
>> + "MetricExpr": "PM_DATA_FROM_L2MISS / PM_LD_DEMAND_MISS_L1 * 100",
>> + "MetricGroup": "dL1_Reloads",
>> + "MetricName": "DL1_RELOAD_FROM_L2_MISS",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of demand load misses that reloaded from beyond the local L3",
>> + "MetricExpr": "PM_DATA_FROM_L3MISS / PM_LD_DEMAND_MISS_L1 * 100",
>> + "MetricGroup": "dL1_Reloads",
>> + "MetricName": "DL1_RELOAD_FROM_L3_MISS",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of cycles stalled due to the ntc instruction waiting for a load miss to resolve from a source beyond the local L2 and local L3",
>> + "MetricExpr": "DMISS_L3MISS_STALL_CPI / RUN_CPI * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "DCACHE_MISS_CPI",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of DERAT misses with 2M page size per completed run instruction",
>> + "MetricExpr": "PM_DERAT_MISS_2M / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_2M_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of DERAT misses with 16M page size per completed run instruction",
>> + "MetricExpr": "PM_DERAT_MISS_16M / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_16M_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "DERAT miss ratio for 4K page size",
>> + "MetricExpr": "PM_DERAT_MISS_4K / PM_DERAT_MISS",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_4K_MISS_RATIO"
>> + },
>> + {
>> + "BriefDescription": "DERAT miss ratio for 2M page size",
>> + "MetricExpr": "PM_DERAT_MISS_2M / PM_DERAT_MISS",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_2M_MISS_RATIO"
>> + },
>> + {
>> + "BriefDescription": "DERAT miss ratio for 16M page size",
>> + "MetricExpr": "PM_DERAT_MISS_16M / PM_DERAT_MISS",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_16M_MISS_RATIO"
>> + },
>> + {
>> + "BriefDescription": "DERAT miss ratio for 64K page size",
>> + "MetricExpr": "PM_DERAT_MISS_64K / PM_DERAT_MISS",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_64K_MISS_RATIO"
>> + },
>> + {
>> + "BriefDescription": "Percentage of DERAT misses that resulted in TLB reloads",
>> + "MetricExpr": "PM_DTLB_MISS / PM_DERAT_MISS * 100",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_MISS_RELOAD",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of ICache misses that were reloaded from beyond the local L3",
>
> Sometimes we use "ICache" and sometimes "icache". Pick one.
>
>> + "MetricExpr": "PM_INST_FROM_L3MISS / PM_L1_ICACHE_MISS * 100",
>> + "MetricGroup": "Instruction_Misses",
>> + "MetricName": "INST_FROM_L3_MISS",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of ICache reloads from the beyond the L3 per completed run instruction",
>> + "MetricExpr": "PM_INST_FROM_L3MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Instruction_Misses",
>> + "MetricName": "INST_FROM_L3_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + }
>> +]
>
> PC
>
^ permalink raw reply
* Re: [PATCH] perf vendor events power10: Add metric events json file for power10 platform
From: Michael Ellerman @ 2021-10-08 5:58 UTC (permalink / raw)
To: Segher Boessenkool, Paul A. Clarke
Cc: maddy, rnsastry, Kajol Jain, jolsa, linux-kernel, acme,
linux-perf-users, atrajeev, linuxppc-dev
In-Reply-To: <20211007184337.GN10333@gate.crashing.org>
Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Thu, Oct 07, 2021 at 08:47:50AM -0500, Paul A. Clarke wrote:
>> On Wed, Oct 06, 2021 at 12:32:48PM -0500, Paul A. Clarke wrote:
>> > > + {
>> > > + "BriefDescription": "Average cycles per instruction when the instruction finishes at dispatch",
>> >
>> > I'm not sure what that means.
>>
>> After doing a bit of research, I think it might be a bit more clear as:
>> "Average cycles per instruction when the NTF instruction finishes at dispatch"
>
> Is "next to finish" some defined and/or sensible term in this context?
> Or do you mean NTC here? Or what :-)
Yeah I am also more familiar with NTC.
But those descriptions originally come from a spreadsheet we're given,
and they do include NTF as an acronym.
eg. search for NTF in here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/pmu-events/arch/powerpc/power9/metrics.json
From the context it does presumably mean "next to finish".
cheers
^ permalink raw reply
* Re: [PATCH v2] KVM: PPC: Defer vtime accounting 'til after IRQ handling
From: Greg Kurz @ 2021-10-08 5:53 UTC (permalink / raw)
To: Laurent Vivier
Cc: linux-kernel, Nicholas Piggin, kvm-ppc, stable, linuxppc-dev
In-Reply-To: <20211007142856.41205-1-lvivier@redhat.com>
On Thu, 7 Oct 2021 16:28:56 +0200
Laurent Vivier <lvivier@redhat.com> wrote:
> Commit 112665286d08 moved guest_exit() in the interrupt protected
> area to avoid wrong context warning (or worse), but the tick counter
> cannot be updated and the guest time is accounted to the system time.
>
> To fix the problem port to POWER the x86 fix
> 160457140187 ("Defer vtime accounting 'til after IRQ handling"):
>
> "Defer the call to account guest time until after servicing any IRQ(s)
> that happened in the guest or immediately after VM-Exit. Tick-based
> accounting of vCPU time relies on PF_VCPU being set when the tick IRQ
> handler runs, and IRQs are blocked throughout the main sequence of
> vcpu_enter_guest(), including the call into vendor code to actually
> enter and exit the guest."
>
> Fixes: 112665286d08 ("KVM: PPC: Book3S HV: Context tracking exit guest context before enabling irqs")
> Cc: npiggin@gmail.com
> Cc: <stable@vger.kernel.org> # 5.12
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>
> Notes:
> v2: remove reference to commit 61bd0f66ff92
> cc stable 5.12
> add the same comment in the code as for x86
>
Works for me. As you stated in your answer, someone can polish the
code later on.
Reviewed-by: Greg Kurz <groug@kaod.org>
> arch/powerpc/kvm/book3s_hv.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 2acb1c96cfaf..a694d1a8f6ce 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3695,6 +3695,8 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
>
> srcu_read_unlock(&vc->kvm->srcu, srcu_idx);
>
> + context_tracking_guest_exit();
> +
> set_irq_happened(trap);
>
> spin_lock(&vc->lock);
> @@ -3726,9 +3728,15 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
>
> kvmppc_set_host_core(pcpu);
>
> - guest_exit_irqoff();
> -
> local_irq_enable();
> + /*
> + * Wait until after servicing IRQs to account guest time so that any
> + * ticks that occurred while running the guest are properly accounted
> + * to the guest. Waiting until IRQs are enabled degrades the accuracy
> + * of accounting via context tracking, but the loss of accuracy is
> + * acceptable for all known use cases.
> + */
> + vtime_account_guest_exit();
>
> /* Let secondaries go back to the offline loop */
> for (i = 0; i < controlled_threads; ++i) {
> @@ -4506,13 +4514,21 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
>
> srcu_read_unlock(&kvm->srcu, srcu_idx);
>
> + context_tracking_guest_exit();
> +
> set_irq_happened(trap);
>
> kvmppc_set_host_core(pcpu);
>
> - guest_exit_irqoff();
> -
> local_irq_enable();
> + /*
> + * Wait until after servicing IRQs to account guest time so that any
> + * ticks that occurred while running the guest are properly accounted
> + * to the guest. Waiting until IRQs are enabled degrades the accuracy
> + * of accounting via context tracking, but the loss of accuracy is
> + * acceptable for all known use cases.
> + */
> + vtime_account_guest_exit();
>
> cpumask_clear_cpu(pcpu, &kvm->arch.cpu_in_guest);
>
^ permalink raw reply
* linux-next: build warnings in Linus' tree
From: Stephen Rothwell @ 2021-10-08 5:47 UTC (permalink / raw)
To: Michael Ellerman, PowerPC
Cc: Linux Next Mailing List, Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 2696 bytes --]
Hi all,
After merging the origin tree, today's linux-next build (powerpc
allyesconfig) produced these warnings (along with many others):
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
Given that arch/powerpc/boot/dts/mpc5200b.dtsi is oncluded by several
other dts files, fixing this one file would go quite a long way to
silencing our allyesoncig build. Unfotunatley, I have no idea how to
fix this file (ad maybe some fo the interactions it has with other files).
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/eeh:Fix docstrings in eeh
From: Daniel Axtens @ 2021-10-08 5:28 UTC (permalink / raw)
To: Kai Song, linuxppc-dev; +Cc: paulus, Kai Song, oohall, linux-kernel
In-Reply-To: <20211004085842.255813-1-songkai01@inspur.com>
Hi Kai,
Thank you for your patch! I have 3 very minor tweaks and I am otherwise
very happy with it.
Firstly, in your commit name, there should be a space between
"powerpc/eeh:" and "Fix docstrings". You might also want to say "in
eeh.c" rather than "in eeh" because there is eeh code in a number of
other files too.
> We fix the following warnings when building kernel with W=1:
> arch/powerpc/kernel/eeh.c:598: warning: Function parameter or member 'function' not described in 'eeh_pci_enable'
> arch/powerpc/kernel/eeh.c:774: warning: Function parameter or member 'edev' not described in 'eeh_set_dev_freset'
> arch/powerpc/kernel/eeh.c:774: warning: expecting prototype for eeh_set_pe_freset(). Prototype was for eeh_set_dev_freset() instead
> arch/powerpc/kernel/eeh.c:814: warning: Function parameter or member 'include_passed' not described in 'eeh_pe_reset_full'
> arch/powerpc/kernel/eeh.c:944: warning: Function parameter or member 'ops' not described in 'eeh_init'
> arch/powerpc/kernel/eeh.c:1451: warning: Function parameter or member 'include_passed' not described in 'eeh_pe_reset'
> arch/powerpc/kernel/eeh.c:1526: warning: Function parameter or member 'func' not described in 'eeh_pe_inject_err'
> arch/powerpc/kernel/eeh.c:1526: warning: Excess function parameter 'function' described in 'eeh_pe_inject_err'
>
> Signed-off-by: Kai Song <songkai01@inspur.com>
> ---
> arch/powerpc/kernel/eeh.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index e9b597ed423c..57a6868a41ab 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -589,6 +589,7 @@ EXPORT_SYMBOL(eeh_check_failure);
> /**
> * eeh_pci_enable - Enable MMIO or DMA transfers for this slot
> * @pe: EEH PE
> + * @function : EEH function
I don't think there should be a space between '@function' and ':'.
I know the parameter is called 'function' and I think "EEH function" was
a good guess for the docstring. However, if we look at the way
'function' is used, it is compared with EEH_OPT_xxx constants, and then
it's passed to eeh_ops->set_option(), eeh_pci_enable() is also called in
eeh_pe_set_option() with a parameter called 'option'. So I think maybe
'function' should be described as the "EEH option"?
This is still very unsatisfactory but it's not the fault of your patch -
the EEH codebase is very messy and it's worth fixing the W=1 warnings
even if we don't fully clean up the EEH codebase.
> - * eeh_set_pe_freset - Check the required reset for the indicated device
> - * @data: EEH device
> + * eeh_set_dev_freset - Check the required reset for the indicated device
> + * @edev: EEH device
> * @flag: return value
> *
This is good.
> /**
> * eeh_pe_reset_full - Complete a full reset process on the indicated PE
> * @pe: EEH PE
> + * @include_passed: include passed-through devices?
> *
> * This function executes a full reset procedure on a PE, including setting
> * the appropriate flags, performing a fundamental or hot reset, and then
> @@ -937,6 +939,7 @@ static struct notifier_block eeh_device_nb = {
This is OK.
> /**
> * eeh_init - System wide EEH initialization
> + * @ops: struct to trace EEH operation callback functions
I think "@ops: platform-specific functions for EEH operations" is
probably clearer?
> *
> * It's the platform's job to call this from an arch_initcall().
> */
> @@ -1442,6 +1445,7 @@ static int eeh_pe_reenable_devices(struct eeh_pe *pe, bool include_passed)
> * eeh_pe_reset - Issue PE reset according to specified type
> * @pe: EEH PE
> * @option: reset type
> + * @include_passed: include passed-through devices?
> *
This is OK.
> * The routine is called to reset the specified PE with the
> * indicated type, either fundamental reset or hot reset.
> @@ -1513,12 +1517,12 @@ EXPORT_SYMBOL_GPL(eeh_pe_configure);
> * eeh_pe_inject_err - Injecting the specified PCI error to the indicated PE
> * @pe: the indicated PE
> * @type: error type
> - * @function: error function
> + * @func: error function
This is good.
> * @addr: address
> * @mask: address mask
> *
> * The routine is called to inject the specified PCI error, which
> - * is determined by @type and @function, to the indicated PE for
> + * is determined by @type and @func, to the indicated PE for
This is good.
When you resend, you can include:
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
^ permalink raw reply
* [powerpc:next-test] BUILD SUCCESS 880c5cbb35b2c6e7d9627a53d26799f7affb6472
From: kernel test robot @ 2021-10-08 3:42 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test
branch HEAD: 880c5cbb35b2c6e7d9627a53d26799f7affb6472 powerpc/paravirt: correct preempt debug splat in vcpu_is_preempted()
elapsed time: 772m
configs tested: 121
configs skipped: 3
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allmodconfig
arm allyesconfig
powerpc motionpro_defconfig
arc nsimosci_hs_smp_defconfig
m68k atari_defconfig
powerpc ppc40x_defconfig
powerpc stx_gp3_defconfig
m68k multi_defconfig
sh landisk_defconfig
sh microdev_defconfig
powerpc ppc64_defconfig
mips ath25_defconfig
arm orion5x_defconfig
arm collie_defconfig
s390 zfcpdump_defconfig
m68k m5475evb_defconfig
sparc alldefconfig
sparc sparc64_defconfig
sh sh7757lcr_defconfig
arc axs101_defconfig
sh kfr2r09_defconfig
mips maltaup_defconfig
mips loongson3_defconfig
powerpc ps3_defconfig
h8300 allyesconfig
powerpc pcm030_defconfig
powerpc pasemi_defconfig
powerpc64 defconfig
mips capcella_defconfig
arm moxart_defconfig
arm sama5_defconfig
arc nsimosci_hs_defconfig
m68k stmark2_defconfig
arm davinci_all_defconfig
sh polaris_defconfig
xtensa iss_defconfig
arm multi_v4t_defconfig
powerpc ksi8560_defconfig
mips bcm63xx_defconfig
x86_64 randconfig-c001-20211003
i386 randconfig-c001-20211003
arm randconfig-c002-20211003
x86_64 randconfig-c001-20211004
i386 randconfig-c001-20211004
arm randconfig-c002-20211004
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k defconfig
m68k allmodconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
csky defconfig
alpha defconfig
alpha allyesconfig
nios2 allyesconfig
arc defconfig
sh allmodconfig
xtensa allyesconfig
parisc defconfig
parisc allyesconfig
s390 defconfig
s390 allyesconfig
s390 allmodconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
i386 allyesconfig
mips allyesconfig
mips allmodconfig
powerpc allmodconfig
powerpc allnoconfig
powerpc allyesconfig
x86_64 randconfig-a015-20211004
x86_64 randconfig-a012-20211004
x86_64 randconfig-a016-20211004
x86_64 randconfig-a014-20211004
x86_64 randconfig-a013-20211004
x86_64 randconfig-a011-20211004
i386 randconfig-a013-20211004
i386 randconfig-a016-20211004
i386 randconfig-a014-20211004
i386 randconfig-a011-20211004
i386 randconfig-a012-20211004
i386 randconfig-a015-20211004
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
um x86_64_defconfig
um i386_defconfig
x86_64 allyesconfig
x86_64 rhel-8.3-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-a003-20211004
x86_64 randconfig-a005-20211004
x86_64 randconfig-a001-20211004
x86_64 randconfig-a002-20211004
x86_64 randconfig-a004-20211004
x86_64 randconfig-a006-20211004
i386 randconfig-a001-20211004
i386 randconfig-a003-20211004
i386 randconfig-a005-20211004
i386 randconfig-a002-20211004
i386 randconfig-a004-20211004
i386 randconfig-a006-20211004
hexagon randconfig-r045-20211007
hexagon randconfig-r041-20211007
s390 randconfig-r044-20211007
riscv randconfig-r042-20211007
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [powerpc:fixes-test] BUILD SUCCESS eb8257a12192f43ffd41bd90932c39dade958042
From: kernel test robot @ 2021-10-08 2:58 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test
branch HEAD: eb8257a12192f43ffd41bd90932c39dade958042 pseries/eeh: Fix the kdump kernel crash during eeh_pseries_init
elapsed time: 731m
configs tested: 96
configs skipped: 96
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
powerpc motionpro_defconfig
arc nsimosci_hs_smp_defconfig
m68k atari_defconfig
powerpc ppc40x_defconfig
powerpc stx_gp3_defconfig
m68k multi_defconfig
sh landisk_defconfig
sh microdev_defconfig
powerpc ppc64_defconfig
mips ath25_defconfig
arm orion5x_defconfig
arm collie_defconfig
powerpc ps3_defconfig
h8300 allyesconfig
powerpc pcm030_defconfig
powerpc pasemi_defconfig
powerpc64 defconfig
mips capcella_defconfig
arm davinci_all_defconfig
sh polaris_defconfig
xtensa iss_defconfig
arm multi_v4t_defconfig
powerpc ksi8560_defconfig
mips bcm63xx_defconfig
x86_64 randconfig-c001-20211003
i386 randconfig-c001-20211003
arm randconfig-c002-20211003
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nds32 defconfig
csky defconfig
alpha defconfig
alpha allyesconfig
nios2 allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
parisc defconfig
parisc allyesconfig
s390 defconfig
s390 allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
i386 allyesconfig
mips allyesconfig
mips allmodconfig
powerpc allnoconfig
powerpc allyesconfig
powerpc allmodconfig
x86_64 randconfig-a015-20211004
x86_64 randconfig-a012-20211004
x86_64 randconfig-a016-20211004
x86_64 randconfig-a014-20211004
x86_64 randconfig-a013-20211004
x86_64 randconfig-a011-20211004
i386 randconfig-a013-20211004
i386 randconfig-a016-20211004
i386 randconfig-a014-20211004
i386 randconfig-a011-20211004
i386 randconfig-a012-20211004
i386 randconfig-a015-20211004
riscv nommu_k210_defconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
um x86_64_defconfig
um i386_defconfig
x86_64 rhel-8.3-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-a003-20211004
x86_64 randconfig-a005-20211004
x86_64 randconfig-a001-20211004
x86_64 randconfig-a002-20211004
x86_64 randconfig-a004-20211004
x86_64 randconfig-a006-20211004
i386 randconfig-a001-20211004
i386 randconfig-a003-20211004
i386 randconfig-a005-20211004
i386 randconfig-a002-20211004
i386 randconfig-a004-20211004
i386 randconfig-a006-20211004
hexagon randconfig-r045-20211007
hexagon randconfig-r041-20211007
s390 randconfig-r044-20211007
riscv randconfig-r042-20211007
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH] perf vendor events power10: Add metric events json file for power10 platform
From: Segher Boessenkool @ 2021-10-07 18:43 UTC (permalink / raw)
To: Paul A. Clarke
Cc: maddy, rnsastry, Kajol Jain, jolsa, linux-kernel, acme,
linux-perf-users, atrajeev, linuxppc-dev
In-Reply-To: <20211007134750.GA243632@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com>
On Thu, Oct 07, 2021 at 08:47:50AM -0500, Paul A. Clarke wrote:
> On Wed, Oct 06, 2021 at 12:32:48PM -0500, Paul A. Clarke wrote:
> > > + {
> > > + "BriefDescription": "Average cycles per instruction when the instruction finishes at dispatch",
> >
> > I'm not sure what that means.
>
> After doing a bit of research, I think it might be a bit more clear as:
> "Average cycles per instruction when the NTF instruction finishes at dispatch"
Is "next to finish" some defined and/or sensible term in this context?
Or do you mean NTC here? Or what :-)
Instructions do not finish in order at all generally.
Segher
^ permalink raw reply
* [PATCH v2] KVM: PPC: Defer vtime accounting 'til after IRQ handling
From: Laurent Vivier @ 2021-10-07 14:28 UTC (permalink / raw)
To: kvm-ppc
Cc: Laurent Vivier, Greg Kurz, Nicholas Piggin, linux-kernel, stable,
linuxppc-dev
Commit 112665286d08 moved guest_exit() in the interrupt protected
area to avoid wrong context warning (or worse), but the tick counter
cannot be updated and the guest time is accounted to the system time.
To fix the problem port to POWER the x86 fix
160457140187 ("Defer vtime accounting 'til after IRQ handling"):
"Defer the call to account guest time until after servicing any IRQ(s)
that happened in the guest or immediately after VM-Exit. Tick-based
accounting of vCPU time relies on PF_VCPU being set when the tick IRQ
handler runs, and IRQs are blocked throughout the main sequence of
vcpu_enter_guest(), including the call into vendor code to actually
enter and exit the guest."
Fixes: 112665286d08 ("KVM: PPC: Book3S HV: Context tracking exit guest context before enabling irqs")
Cc: npiggin@gmail.com
Cc: <stable@vger.kernel.org> # 5.12
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
Notes:
v2: remove reference to commit 61bd0f66ff92
cc stable 5.12
add the same comment in the code as for x86
arch/powerpc/kvm/book3s_hv.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 2acb1c96cfaf..a694d1a8f6ce 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3695,6 +3695,8 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
srcu_read_unlock(&vc->kvm->srcu, srcu_idx);
+ context_tracking_guest_exit();
+
set_irq_happened(trap);
spin_lock(&vc->lock);
@@ -3726,9 +3728,15 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
kvmppc_set_host_core(pcpu);
- guest_exit_irqoff();
-
local_irq_enable();
+ /*
+ * Wait until after servicing IRQs to account guest time so that any
+ * ticks that occurred while running the guest are properly accounted
+ * to the guest. Waiting until IRQs are enabled degrades the accuracy
+ * of accounting via context tracking, but the loss of accuracy is
+ * acceptable for all known use cases.
+ */
+ vtime_account_guest_exit();
/* Let secondaries go back to the offline loop */
for (i = 0; i < controlled_threads; ++i) {
@@ -4506,13 +4514,21 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
srcu_read_unlock(&kvm->srcu, srcu_idx);
+ context_tracking_guest_exit();
+
set_irq_happened(trap);
kvmppc_set_host_core(pcpu);
- guest_exit_irqoff();
-
local_irq_enable();
+ /*
+ * Wait until after servicing IRQs to account guest time so that any
+ * ticks that occurred while running the guest are properly accounted
+ * to the guest. Waiting until IRQs are enabled degrades the accuracy
+ * of accounting via context tracking, but the loss of accuracy is
+ * acceptable for all known use cases.
+ */
+ vtime_account_guest_exit();
cpumask_clear_cpu(pcpu, &kvm->arch.cpu_in_guest);
--
2.31.1
^ permalink raw reply related
* Re: [PATCH] perf vendor events power10: Add metric events json file for power10 platform
From: Paul A. Clarke @ 2021-10-07 13:47 UTC (permalink / raw)
To: Kajol Jain
Cc: maddy, rnsastry, linuxppc-dev, linux-kernel, acme,
linux-perf-users, atrajeev, jolsa
In-Reply-To: <20211006173248.GA7389@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com>
On Wed, Oct 06, 2021 at 12:32:48PM -0500, Paul A. Clarke wrote:
> > + {
> > + "BriefDescription": "Average cycles per instruction when the instruction finishes at dispatch",
>
> I'm not sure what that means.
After doing a bit of research, I think it might be a bit more clear as:
"Average cycles per instruction when the NTF instruction finishes at dispatch"
> > + "MetricExpr": "PM_EXEC_STALL_FIN_AT_DISP / PM_RUN_INST_CMPL",
> > + "MetricGroup": "CPI",
> > + "MetricName": "FIN_AT_DISP_STALL_CPI"
> > + },
PC
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Defer vtime accounting 'til after IRQ handling
From: Laurent Vivier @ 2021-10-07 13:45 UTC (permalink / raw)
To: Greg Kurz; +Cc: linux-kernel, Nicholas Piggin, kvm-ppc, linuxppc-dev
In-Reply-To: <20211006124204.4741bb5c@bahia.huguette>
On 06/10/2021 12:42, Greg Kurz wrote:
> On Wed, 6 Oct 2021 09:37:45 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> Commit 61bd0f66ff92 has moved guest_enter() out of the interrupt
>> protected area to be able to have updated tick counters, but
>> commit 112665286d08 moved back to this area to avoid wrong
>> context warning (or worse).
>>
>> None of them are correct, to fix the problem port to POWER
>> the x86 fix 160457140187 ("KVM: x86: Defer vtime accounting 'til
>> after IRQ handling"):
>>
>> "Defer the call to account guest time until after servicing any IRQ(s)
>> that happened in the guest or immediately after VM-Exit. Tick-based
>> accounting of vCPU time relies on PF_VCPU being set when the tick IRQ
>> handler runs, and IRQs are blocked throughout the main sequence of
>> vcpu_enter_guest(), including the call into vendor code to actually
>> enter and exit the guest."
>>
>> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2009312
>> Fixes: 61bd0f66ff92 ("KVM: PPC: Book3S HV: Fix guest time accounting with VIRT_CPU_ACCOUNTING_GEN")
>
> This patch was merged in linux 4.16 and thus is on the 4.16.y
> stable branch and it was backported to stable 4.14.y. It would
> make sense to Cc: stable # v4.14 also, but...
>
>> Fixes: 112665286d08 ("KVM: PPC: Book3S HV: Context tracking exit guest context before enabling irqs")
>
> ... this one, which was merged in linux 5.12, was never backported
> anywhere because it wasn't considered as a fix, as commented here:
>
> https://lore.kernel.org/linuxppc-dev/1610793296.fjhomer31g.astroid@bobo.none/
>
> AFAICT commit 61bd0f66ff92 was never mentioned anywhere in a bug. The
> first Fixes: tag thus looks a bit misleading. I'd personally drop it
> and Cc: stable # v5.12.
>
Ok, I update the comment.
>> Cc: npiggin@gmail.com
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> arch/powerpc/kvm/book3s_hv.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 2acb1c96cfaf..43e1ce853785 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -3695,6 +3695,8 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
>>
>> srcu_read_unlock(&vc->kvm->srcu, srcu_idx);
>>
>> + context_tracking_guest_exit();
>> +
>> set_irq_happened(trap);
>>
>> spin_lock(&vc->lock);
>> @@ -3726,9 +3728,8 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
>>
>> kvmppc_set_host_core(pcpu);
>>
>> - guest_exit_irqoff();
>> -
>
>
> Change looks ok but it might be a bit confusing for the
> occasional reader that guest_enter_irqoff() isn't matched
> by a guest_exit_irqoff().
>
>> local_irq_enable();
>> + vtime_account_guest_exit();
>>
>
> Maybe add a comment like in x86 ?
>
done
>> /* Let secondaries go back to the offline loop */
>> for (i = 0; i < controlled_threads; ++i) {
>> @@ -4506,13 +4507,14 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
>>
>> srcu_read_unlock(&kvm->srcu, srcu_idx);
>>
>> + context_tracking_guest_exit();
>> +
>> set_irq_happened(trap);
>>
>> kvmppc_set_host_core(pcpu);
>>
>> - guest_exit_irqoff();
>> -
>> local_irq_enable();
>> + vtime_account_guest_exit();
>>
>> cpumask_clear_cpu(pcpu, &kvm->arch.cpu_in_guest);
>>
>
> Same remarks. FWIW a followup was immediately added to x86 to
> encapsulate the enter/exit logic in common helpers :
>
> ommit bc908e091b3264672889162733020048901021fb
> Author: Sean Christopherson <seanjc@google.com>
> Date: Tue May 4 17:27:35 2021 -0700
>
> KVM: x86: Consolidate guest enter/exit logic to common helpers
>
> This makes the code nicer. Maybe do something similar for POWER ?
I don't like to modify kernel code when it's not needed. I just want to fix a bug, if
someone wants this nicer I let this to him...
Thanks,
Laurent
^ permalink raw reply
* Re: [PATCH] powerpc/eeh:Fix some mistakes in comments
From: Michael Ellerman @ 2021-10-07 12:55 UTC (permalink / raw)
To: Daniel Axtens, Kai Song, linuxppc-dev
Cc: paulus, Kai Song, oohall, linux-kernel
In-Reply-To: <878rze60by.fsf@dja-thinkpad.axtens.net>
Daniel Axtens <dja@axtens.net> writes:
> Hi Kai,
>
> Thank you for your contribution to the powerpc kernel!
>
>> Get rid of warning:
>> arch/powerpc/kernel/eeh.c:774: warning: expecting prototype for eeh_set_pe_freset(). Prototype was for eeh_set_dev_freset() instead
>
> You haven't said where this warning is from. I thought it might be from
> sparse but I couldn't seem to reproduce it - is my version of sparse too
> old or are you using a different tool?
>
>> /**
>> - * eeh_set_pe_freset - Check the required reset for the indicated device
>> - * @data: EEH device
>> + * eeh_set_dev_freset - Check the required reset for the indicated device
>> + * @edev: EEH device
>> * @flag: return value
>> *
>> * Each device might have its preferred reset type: fundamental or
>
> This looks like a good and correct change.
>
> I checked through git history with git blame to see when the function
> was renamed. There are 2 commits that should have updated the comment:
> one renamed the function and one renamed an argument. So, I think this
> commit could have:
>
> Fixes: d6c4932fbf24 ("powerpc/eeh: Strengthen types of eeh traversal functions")
> Fixes: c270a24c59bd ("powerpc/eeh: Do reset based on PE")
>
> But I don't know if an out of date comment is enough of a 'bug' to
> justify a Fixes: tag? (mpe, I'm sure I've asked this before, sorry!)
It depends. If you think it's important that the fix gets backported
then you should add the Fixes tag.
In this case I would say no. The comments have been broken for years,
and it's a pretty obscure API.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/mce: check if event info is valid
From: Michael Ellerman @ 2021-10-07 12:09 UTC (permalink / raw)
To: Ganesh, linuxppc-dev; +Cc: mahesh, npiggin
In-Reply-To: <4f9431fd-74b0-2ce4-807e-9796b326d729@linux.ibm.com>
Ganesh <ganeshgr@linux.ibm.com> writes:
> On 8/6/21 6:53 PM, Ganesh Goudar wrote:
>
>> Check if the event info is valid before printing the
>> event information. When a fwnmi enabled nested kvm guest
>> hits a machine check exception L0 and L2 would generate
>> machine check event info, But L1 would not generate any
>> machine check event info as it won't go through 0x200
>> vector and prints some unwanted message.
>>
>> To fix this, 'in_use' variable in machine check event info is
>> no more in use, rename it to 'valid' and check if the event
>> information is valid before logging the event information.
>>
>> without this patch L1 would print following message for
>> exceptions encountered in L2, as event structure will be
>> empty in L1.
>>
>> "Machine Check Exception, Unknown event version 0".
>>
>> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
>> ---
>
> Hi mpe, Any comments on this patch.
The variable rename is a bit of a distraction.
But ignoring that, how do we end up processing a machine_check_event
that has in_use = 0?
You don't give much detail on what call path you're talking about. I
guess we're coming in via the calls in the KVM code?
In the definition of kvm_vcpu_arch we have:
struct machine_check_event mce_evt; /* Valid if trap == 0x200 */
And you said we're not going via 0x200 in L1. But so shouldn't we be
teaching the KVM code not to use mce_evt when trap is not 0x200?
cheers
^ permalink raw reply
* Re: [PATCH] video: fbdev: use memset_io() instead of memset()
From: Michael Ellerman @ 2021-10-07 11:28 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Andrew Morton
Cc: linux-fbdev, Finn Thain, Stan Johnson, linux-kernel, dri-devel,
linuxppc-dev
In-Reply-To: <884a54f1e5cb774c1d9b4db780209bee5d4f6718.1631712563.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> While investigating a lockup at startup on Powerbook 3400C, it was
> identified that the fbdev driver generates alignment exception at
> startup:
...
>
> Use fb_memset() instead of memset(). fb_memset() is defined as
> memset_io() for powerpc.
>
> Fixes: 8c8709334cec ("[PATCH] ppc32: Remove CONFIG_PMAC_PBOOK")
> Reported-by: Stan Johnson <userm57@yahoo.com>
> Cc: Finn Thain <fthain@linux-m68k.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> drivers/video/fbdev/chipsfb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Looks like drivers/video/fbdev is orphaned, so I'll pick this up via
powerpc.
cheers
^ 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