* Re: [PATCH v3 5/6] powerpc/pseries: implement paravirt qspinlocks for SPLPAR
From: Waiman Long @ 2020-07-25 17:36 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-arch, Boqun Feng, virtualization, linuxppc-dev,
Nicholas Piggin, linux-kernel, Ingo Molnar, kvm-ppc, Will Deacon
In-Reply-To: <20200725172630.GF10769@hirez.programming.kicks-ass.net>
On 7/25/20 1:26 PM, Peter Zijlstra wrote:
> On Fri, Jul 24, 2020 at 03:10:59PM -0400, Waiman Long wrote:
>> On 7/24/20 4:16 AM, Will Deacon wrote:
>>> On Thu, Jul 23, 2020 at 08:47:59PM +0200, peterz@infradead.org wrote:
>>>> On Thu, Jul 23, 2020 at 02:32:36PM -0400, Waiman Long wrote:
>>>>> BTW, do you have any comment on my v2 lock holder cpu info qspinlock patch?
>>>>> I will have to update the patch to fix the reported 0-day test problem, but
>>>>> I want to collect other feedback before sending out v3.
>>>> I want to say I hate it all, it adds instructions to a path we spend an
>>>> aweful lot of time optimizing without really getting anything back for
>>>> it.
>>>>
>>>> Will, how do you feel about it?
>>> I can see it potentially being useful for debugging, but I hate the
>>> limitation to 256 CPUs. Even arm64 is hitting that now.
>> After thinking more about that, I think we can use all the remaining bits in
>> the 16-bit locked_pending. Reserving 1 bit for locked and 1 bit for pending,
>> there are 14 bits left. So as long as NR_CPUS < 16k (requirement for 16-bit
>> locked_pending), we can put all possible cpu numbers into the lock. We can
>> also just use smp_processor_id() without additional percpu data.
> That sounds horrific, wouldn't that destroy the whole point of using a
> byte for pending?
You are right. I realized that later on and had sent a follow-up mail to
correct that.
>>> Also, you're talking ~1% gains here. I think our collective time would
>>> be better spent off reviewing the CNA series and trying to make it more
>>> deterministic.
>> I thought you guys are not interested in CNA. I do want to get CNA merged,
>> if possible. Let review the current version again and see if there are ways
>> we can further improve it.
> It's not a lack of interrest. We were struggling with the fairness
> issues and the complexity of the thing. I forgot the current state of
> matters, but at one point UNLOCK was O(n) in waiters, which is, of
> course, 'unfortunate'.
>
> I'll have to look up whatever notes remain, but the basic idea of
> keeping remote nodes on a secondary list is obviously breaking all sorts
> of fairness. After that they pile on a bunch of hacks to fix the worst
> of them, but it feels exactly like that, a bunch of hacks.
>
> One of the things I suppose we ought to do is see if some of the ideas
> of phase-fair locks can be applied to this.
That could be a possible solution to ensure better fairness.
>
> That coupled with a chronic lack of time for anything :-(
>
That is always true and I feel this way too:-)
Cheers,
Longman
^ permalink raw reply
* Re: [PATCH 1/2] lockdep: improve current->(hard|soft)irqs_enabled synchronisation with actual irq state
From: Peter Zijlstra @ 2020-07-25 20:26 UTC (permalink / raw)
To: Nicholas Piggin
Cc: linux-arch, Alexey Kardashevskiy, linuxppc-dev, linux-kernel,
Ingo Molnar, Will Deacon
In-Reply-To: <20200723105615.1268126-1-npiggin@gmail.com>
On Thu, Jul 23, 2020 at 08:56:14PM +1000, Nicholas Piggin wrote:
> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
> index 3a0db7b0b46e..35060be09073 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -200,17 +200,14 @@ static inline bool arch_irqs_disabled(void)
> #define powerpc_local_irq_pmu_save(flags) \
> do { \
> raw_local_irq_pmu_save(flags); \
> - trace_hardirqs_off(); \
> + if (!raw_irqs_disabled_flags(flags)) \
> + trace_hardirqs_off(); \
> } while(0)
So one problem with the above is something like this:
raw_local_irq_save();
<NMI>
powerpc_local_irq_pmu_save();
that would now no longer call into tracing/lockdep at all. As a
consequence, lockdep and tracing would show the NMI ran with IRQs
enabled, which is exceptionally weird..
Similar problem with:
raw_local_irq_disable();
local_irq_save()
Now, most architectures today seem to do what x86 also did:
<NMI>
trace_hardirqs_off()
...
if (irqs_unmasked(regs))
trace_hardirqs_on()
</NMI>
Which is 'funny' when it interleaves like:
local_irq_disable();
...
local_irq_enable()
trace_hardirqs_on();
<NMI/>
raw_local_irq_enable();
Because then it will undo the trace_hardirqs_on() we just did. With the
result that both tracing and lockdep will see a hardirqs-disable without
a matching enable, while the hardware state is enabled.
Which is exactly the state Alexey seems to have ran into.
Now, x86, and at least arm64 call nmi_enter() before
trace_hardirqs_off(), but AFAICT Power never did that, and that's part
of the problem. nmi_enter() does lockdep_off() and that _used_ to also
kill IRQ tracking.
Now, my patch changed that, it makes IRQ tracking not respect
lockdep_off(). And that exposed x86 (and everybody else :/) to the same
problem you have.
And this is why I made x86 look at software state in NMIs. Because then
it all works out. For bonus points, trace_hardirqs_*() also has some
do-it-once logic for tracing.
Anyway, it's Saturday evening, time for a beer. I'll stare at this more
later.
^ permalink raw reply
* [PATCH] selftests/powerpc: Skip vmx/vsx tests on older CPUs
From: Michael Ellerman @ 2020-07-26 0:34 UTC (permalink / raw)
To: linuxppc-dev
These tests use VSX or newer VMX instructions, so need to be skipped
on older CPUs to avoid SIGILL'ing.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
tools/testing/selftests/powerpc/math/Makefile | 8 ++++----
tools/testing/selftests/powerpc/math/vmx_preempt.c | 3 +++
tools/testing/selftests/powerpc/math/vmx_signal.c | 3 +++
tools/testing/selftests/powerpc/math/vmx_syscall.c | 7 ++++++-
tools/testing/selftests/powerpc/math/vsx_preempt.c | 2 ++
5 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/powerpc/math/Makefile b/tools/testing/selftests/powerpc/math/Makefile
index 4e2049d2fd8d..fcc91c205984 100644
--- a/tools/testing/selftests/powerpc/math/Makefile
+++ b/tools/testing/selftests/powerpc/math/Makefile
@@ -11,9 +11,9 @@ $(OUTPUT)/fpu_syscall: fpu_asm.S
$(OUTPUT)/fpu_preempt: fpu_asm.S
$(OUTPUT)/fpu_signal: fpu_asm.S
-$(OUTPUT)/vmx_syscall: vmx_asm.S
-$(OUTPUT)/vmx_preempt: vmx_asm.S
-$(OUTPUT)/vmx_signal: vmx_asm.S
+$(OUTPUT)/vmx_syscall: vmx_asm.S ../utils.c
+$(OUTPUT)/vmx_preempt: vmx_asm.S ../utils.c
+$(OUTPUT)/vmx_signal: vmx_asm.S ../utils.c
$(OUTPUT)/vsx_preempt: CFLAGS += -mvsx
-$(OUTPUT)/vsx_preempt: vsx_asm.S
+$(OUTPUT)/vsx_preempt: vsx_asm.S ../utils.c
diff --git a/tools/testing/selftests/powerpc/math/vmx_preempt.c b/tools/testing/selftests/powerpc/math/vmx_preempt.c
index 2e059f154e77..6761d6ce30ec 100644
--- a/tools/testing/selftests/powerpc/math/vmx_preempt.c
+++ b/tools/testing/selftests/powerpc/math/vmx_preempt.c
@@ -57,6 +57,9 @@ int test_preempt_vmx(void)
int i, rc, threads;
pthread_t *tids;
+ // vcmpequd used in vmx_asm.S is v2.07
+ SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_2_07));
+
threads = sysconf(_SC_NPROCESSORS_ONLN) * THREAD_FACTOR;
tids = malloc(threads * sizeof(pthread_t));
FAIL_IF(!tids);
diff --git a/tools/testing/selftests/powerpc/math/vmx_signal.c b/tools/testing/selftests/powerpc/math/vmx_signal.c
index 785a48e0976f..b340a5c4e79d 100644
--- a/tools/testing/selftests/powerpc/math/vmx_signal.c
+++ b/tools/testing/selftests/powerpc/math/vmx_signal.c
@@ -96,6 +96,9 @@ int test_signal_vmx(void)
void *rc_p;
pthread_t *tids;
+ // vcmpequd used in vmx_asm.S is v2.07
+ SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_2_07));
+
threads = sysconf(_SC_NPROCESSORS_ONLN) * THREAD_FACTOR;
tids = malloc(threads * sizeof(pthread_t));
FAIL_IF(!tids);
diff --git a/tools/testing/selftests/powerpc/math/vmx_syscall.c b/tools/testing/selftests/powerpc/math/vmx_syscall.c
index 9ee293cc868e..03c78dfe3444 100644
--- a/tools/testing/selftests/powerpc/math/vmx_syscall.c
+++ b/tools/testing/selftests/powerpc/math/vmx_syscall.c
@@ -49,9 +49,14 @@ int test_vmx_syscall(void)
* Setup an environment with much context switching
*/
pid_t pid2;
- pid_t pid = fork();
+ pid_t pid;
int ret;
int child_ret;
+
+ // vcmpequd used in vmx_asm.S is v2.07
+ SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_2_07));
+
+ pid = fork();
FAIL_IF(pid == -1);
pid2 = fork();
diff --git a/tools/testing/selftests/powerpc/math/vsx_preempt.c b/tools/testing/selftests/powerpc/math/vsx_preempt.c
index 63de9c6e2cd3..d1601bb889d4 100644
--- a/tools/testing/selftests/powerpc/math/vsx_preempt.c
+++ b/tools/testing/selftests/powerpc/math/vsx_preempt.c
@@ -92,6 +92,8 @@ int test_preempt_vsx(void)
int i, rc, threads;
pthread_t *tids;
+ SKIP_IF(!have_hwcap(PPC_FEATURE_HAS_VSX));
+
threads = sysconf(_SC_NPROCESSORS_ONLN) * THREAD_FACTOR;
tids = malloc(threads * sizeof(pthread_t));
FAIL_IF(!tids);
--
2.25.1
^ permalink raw reply related
* [PATCH 5/9] powerpc: hw_breakpoint.h: delete duplicated word
From: Randy Dunlap @ 2020-07-26 0:38 UTC (permalink / raw)
To: linux-kernel; +Cc: Randy Dunlap, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200726003809.20454-1-rdunlap@infradead.org>
Drop the repeated word "the".
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
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
---
arch/powerpc/include/asm/hw_breakpoint.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next-20200720.orig/arch/powerpc/include/asm/hw_breakpoint.h
+++ linux-next-20200720/arch/powerpc/include/asm/hw_breakpoint.h
@@ -17,7 +17,7 @@ struct arch_hw_breakpoint {
u16 hw_len; /* length programmed in hw */
};
-/* Note: Don't change the the first 6 bits below as they are in the same order
+/* Note: Don't change the first 6 bits below as they are in the same order
* as the dabr and dabrx.
*/
#define HW_BRK_TYPE_READ 0x01
^ permalink raw reply
* [PATCH 3/9] powerpc: cputime.h: delete duplicated word
From: Randy Dunlap @ 2020-07-26 0:38 UTC (permalink / raw)
To: linux-kernel; +Cc: Randy Dunlap, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200726003809.20454-1-rdunlap@infradead.org>
Drop the repeated word "use".
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
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
---
arch/powerpc/include/asm/cputime.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next-20200720.orig/arch/powerpc/include/asm/cputime.h
+++ linux-next-20200720/arch/powerpc/include/asm/cputime.h
@@ -67,7 +67,7 @@ static inline void arch_vtime_task_switc
/*
* account_cpu_user_entry/exit runs "unreconciled", so can't trace,
- * can't use use get_paca()
+ * can't use get_paca()
*/
static notrace inline void account_cpu_user_entry(void)
{
^ permalink raw reply
* [PATCH 4/9] powerpc: epapr_hcalls.h: delete duplicated words
From: Randy Dunlap @ 2020-07-26 0:38 UTC (permalink / raw)
To: linux-kernel; +Cc: Randy Dunlap, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200726003809.20454-1-rdunlap@infradead.org>
Drop the repeated words "file" and "the".
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
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
---
arch/powerpc/include/asm/epapr_hcalls.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- linux-next-20200720.orig/arch/powerpc/include/asm/epapr_hcalls.h
+++ linux-next-20200720/arch/powerpc/include/asm/epapr_hcalls.h
@@ -37,7 +37,7 @@
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-/* A "hypercall" is an "sc 1" instruction. This header file file provides C
+/* A "hypercall" is an "sc 1" instruction. This header file provides C
* wrapper functions for the ePAPR hypervisor interface. It is inteded
* for use by Linux device drivers and other operating systems.
*
@@ -246,7 +246,7 @@ static inline unsigned int ev_int_get_ma
* ev_int_eoi - signal the end of interrupt processing
* @interrupt: the interrupt number
*
- * This function signals the end of processing for the the specified
+ * This function signals the end of processing for the specified
* interrupt, which must be the interrupt currently in service. By
* definition, this is also the highest-priority interrupt.
*
^ permalink raw reply
* [PATCH 0/9] powerpc: delete duplicated words
From: Randy Dunlap @ 2020-07-26 0:38 UTC (permalink / raw)
To: linux-kernel; +Cc: Randy Dunlap, Paul Mackerras, linuxppc-dev
Drop duplicated words in arch/powerpc/ header files.
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
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 2 +-
arch/powerpc/include/asm/book3s/64/radix-4k.h | 2 +-
arch/powerpc/include/asm/cputime.h | 2 +-
arch/powerpc/include/asm/epapr_hcalls.h | 4 ++--
arch/powerpc/include/asm/hw_breakpoint.h | 2 +-
arch/powerpc/include/asm/ppc_asm.h | 2 +-
arch/powerpc/include/asm/reg.h | 2 +-
arch/powerpc/include/asm/smu.h | 2 +-
arch/powerpc/platforms/powernv/pci.h | 2 +-
9 files changed, 10 insertions(+), 10 deletions(-)
^ permalink raw reply
* [PATCH 1/9] powerpc: book3s: mmu-hash.h: delete duplicated word
From: Randy Dunlap @ 2020-07-26 0:38 UTC (permalink / raw)
To: linux-kernel; +Cc: Randy Dunlap, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200726003809.20454-1-rdunlap@infradead.org>
Drop the repeated word "below".
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
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
---
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next-20200720.orig/arch/powerpc/include/asm/book3s/64/mmu-hash.h
+++ linux-next-20200720/arch/powerpc/include/asm/book3s/64/mmu-hash.h
@@ -793,7 +793,7 @@ static inline unsigned long get_vsid(uns
}
/*
- * For kernel space, we use context ids as below
+ * For kernel space, we use context ids as
* below. Range is 512TB per context.
*
* 0x00001 - [ 0xc000000000000000 - 0xc001ffffffffffff]
^ permalink raw reply
* [PATCH 2/9] powerpc: book3s: radix-4k.h: delete duplicated word
From: Randy Dunlap @ 2020-07-26 0:38 UTC (permalink / raw)
To: linux-kernel; +Cc: Randy Dunlap, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200726003809.20454-1-rdunlap@infradead.org>
Drop the repeated word "per".
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
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
---
arch/powerpc/include/asm/book3s/64/radix-4k.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next-20200720.orig/arch/powerpc/include/asm/book3s/64/radix-4k.h
+++ linux-next-20200720/arch/powerpc/include/asm/book3s/64/radix-4k.h
@@ -11,7 +11,7 @@
#define RADIX_PGD_INDEX_SIZE 13 // size: 8B << 13 = 64KB, maps 2^13 x 512GB = 4PB
/*
- * One fragment per per page
+ * One fragment per page
*/
#define RADIX_PTE_FRAG_SIZE_SHIFT (RADIX_PTE_INDEX_SIZE + 3)
#define RADIX_PTE_FRAG_NR (PAGE_SIZE >> RADIX_PTE_FRAG_SIZE_SHIFT)
^ permalink raw reply
* [PATCH 6/9] powerpc: ppc_asm.h: delete duplicated word
From: Randy Dunlap @ 2020-07-26 0:38 UTC (permalink / raw)
To: linux-kernel; +Cc: Randy Dunlap, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200726003809.20454-1-rdunlap@infradead.org>
Drop the repeated word "in".
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
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
---
arch/powerpc/include/asm/ppc_asm.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next-20200720.orig/arch/powerpc/include/asm/ppc_asm.h
+++ linux-next-20200720/arch/powerpc/include/asm/ppc_asm.h
@@ -774,7 +774,7 @@ END_FTR_SECTION_NESTED(CPU_FTR_CELL_TB_B
#define FIXUP_ENDIAN
#else
/*
- * This version may be used in in HV or non-HV context.
+ * This version may be used in HV or non-HV context.
* MSR[EE] must be disabled.
*/
#define FIXUP_ENDIAN \
^ permalink raw reply
* [PATCH 7/9] powerpc: reg.h: delete duplicated word
From: Randy Dunlap @ 2020-07-26 0:38 UTC (permalink / raw)
To: linux-kernel; +Cc: Randy Dunlap, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200726003809.20454-1-rdunlap@infradead.org>
Drop the repeated word "a".
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
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
---
arch/powerpc/include/asm/reg.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next-20200720.orig/arch/powerpc/include/asm/reg.h
+++ linux-next-20200720/arch/powerpc/include/asm/reg.h
@@ -1472,7 +1472,7 @@ static inline void update_power8_hid0(un
{
/*
* The HID0 update on Power8 should at the very least be
- * preceded by a a SYNC instruction followed by an ISYNC
+ * preceded by a SYNC instruction followed by an ISYNC
* instruction
*/
asm volatile("sync; mtspr %0,%1; isync":: "i"(SPRN_HID0), "r"(hid0));
^ permalink raw reply
* [PATCH 8/9] powerpc: smu.h: delete duplicated word
From: Randy Dunlap @ 2020-07-26 0:38 UTC (permalink / raw)
To: linux-kernel; +Cc: Randy Dunlap, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200726003809.20454-1-rdunlap@infradead.org>
Drop the repeated word "the".
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
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
---
arch/powerpc/include/asm/smu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next-20200720.orig/arch/powerpc/include/asm/smu.h
+++ linux-next-20200720/arch/powerpc/include/asm/smu.h
@@ -108,7 +108,7 @@
/*
* i2c commands
*
- * To issue an i2c command, first is to send a parameter block to the
+ * To issue an i2c command, first is to send a parameter block to
* the SMU. This is a command of type 0x9a with 9 bytes of header
* eventually followed by data for a write:
*
^ permalink raw reply
* [PATCH 9/9] powerpc: powernv: pci.h: delete duplicated word
From: Randy Dunlap @ 2020-07-26 0:38 UTC (permalink / raw)
To: linux-kernel; +Cc: Randy Dunlap, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200726003809.20454-1-rdunlap@infradead.org>
Drop the repeated word "for".
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
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
---
arch/powerpc/platforms/powernv/pci.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next-20200720.orig/arch/powerpc/platforms/powernv/pci.h
+++ linux-next-20200720/arch/powerpc/platforms/powernv/pci.h
@@ -87,7 +87,7 @@ struct pnv_ioda_pe {
bool tce_bypass_enabled;
uint64_t tce_bypass_base;
- /* MSIs. MVE index is identical for for 32 and 64 bit MSI
+ /* MSIs. MVE index is identical for 32 and 64 bit MSI
* and -1 if not supported. (It's actually identical to the
* PE number)
*/
^ permalink raw reply
* [PATCH v3 0/3] powerpc/pseries: IPI doorbell improvements
From: Nicholas Piggin @ 2020-07-26 3:51 UTC (permalink / raw)
To: linuxppc-dev
Cc: kvm-ppc, Nicholas Piggin, Anton Blanchard, Cédric Le Goater,
David Gibson
Since v2:
- Fixed ppc32 compile error
- Tested-by from Cedric
Nicholas Piggin (3):
powerpc: inline doorbell sending functions
powerpc/pseries: Use doorbells even if XIVE is available
powerpc/pseries: Add KVM guest doorbell restrictions
arch/powerpc/include/asm/dbell.h | 67 ++++++++++++++++++++++++++--
arch/powerpc/include/asm/firmware.h | 6 +++
arch/powerpc/include/asm/kvm_para.h | 26 ++---------
arch/powerpc/kernel/Makefile | 5 +--
arch/powerpc/kernel/dbell.c | 55 -----------------------
arch/powerpc/kernel/firmware.c | 19 ++++++++
arch/powerpc/platforms/pseries/smp.c | 62 +++++++++++++++++--------
7 files changed, 138 insertions(+), 102 deletions(-)
--
2.23.0
^ permalink raw reply
* [PATCH v3 1/3] powerpc: inline doorbell sending functions
From: Nicholas Piggin @ 2020-07-26 3:51 UTC (permalink / raw)
To: linuxppc-dev
Cc: kvm-ppc, Nicholas Piggin, Anton Blanchard, Cédric Le Goater,
David Gibson
In-Reply-To: <20200726035155.1424103-1-npiggin@gmail.com>
These are only called in one place for a given platform, so inline them
for performance.
Tested-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/dbell.h | 67 ++++++++++++++++++++++++++++++--
arch/powerpc/kernel/dbell.c | 55 --------------------------
2 files changed, 64 insertions(+), 58 deletions(-)
diff --git a/arch/powerpc/include/asm/dbell.h b/arch/powerpc/include/asm/dbell.h
index 4ce6808deed3..1f04f3de96ba 100644
--- a/arch/powerpc/include/asm/dbell.h
+++ b/arch/powerpc/include/asm/dbell.h
@@ -13,6 +13,11 @@
#include <asm/ppc-opcode.h>
#include <asm/feature-fixups.h>
+#ifdef CONFIG_KVM
+#include <asm/kvm_ppc.h>
+#else
+static inline void kvmppc_set_host_ipi(int cpu) {}
+#endif
#define PPC_DBELL_MSG_BRDCAST (0x04000000)
#define PPC_DBELL_TYPE(x) (((x) & 0xf) << (63-36))
@@ -87,9 +92,6 @@ static inline void ppc_msgsync(void)
#endif /* CONFIG_PPC_BOOK3S */
-extern void doorbell_global_ipi(int cpu);
-extern void doorbell_core_ipi(int cpu);
-extern int doorbell_try_core_ipi(int cpu);
extern void doorbell_exception(struct pt_regs *regs);
static inline void ppc_msgsnd(enum ppc_dbell type, u32 flags, u32 tag)
@@ -100,4 +102,63 @@ static inline void ppc_msgsnd(enum ppc_dbell type, u32 flags, u32 tag)
_ppc_msgsnd(msg);
}
+#ifdef CONFIG_SMP
+
+/*
+ * Doorbells must only be used if CPU_FTR_DBELL is available.
+ * msgsnd is used in HV, and msgsndp is used in !HV.
+ *
+ * These should be used by platform code that is aware of restrictions.
+ * Other arch code should use ->cause_ipi.
+ *
+ * doorbell_global_ipi() sends a dbell to any target CPU.
+ * Must be used only by architectures that address msgsnd target
+ * by PIR/get_hard_smp_processor_id.
+ */
+static inline void doorbell_global_ipi(int cpu)
+{
+ u32 tag = get_hard_smp_processor_id(cpu);
+
+ kvmppc_set_host_ipi(cpu);
+ /* Order previous accesses vs. msgsnd, which is treated as a store */
+ ppc_msgsnd_sync();
+ ppc_msgsnd(PPC_DBELL_MSGTYPE, 0, tag);
+}
+
+/*
+ * doorbell_core_ipi() sends a dbell to a target CPU in the same core.
+ * Must be used only by architectures that address msgsnd target
+ * by TIR/cpu_thread_in_core.
+ */
+static inline void doorbell_core_ipi(int cpu)
+{
+ u32 tag = cpu_thread_in_core(cpu);
+
+ kvmppc_set_host_ipi(cpu);
+ /* Order previous accesses vs. msgsnd, which is treated as a store */
+ ppc_msgsnd_sync();
+ ppc_msgsnd(PPC_DBELL_MSGTYPE, 0, tag);
+}
+
+/*
+ * Attempt to cause a core doorbell if destination is on the same core.
+ * Returns 1 on success, 0 on failure.
+ */
+static inline int doorbell_try_core_ipi(int cpu)
+{
+ int this_cpu = get_cpu();
+ int ret = 0;
+
+ if (cpumask_test_cpu(cpu, cpu_sibling_mask(this_cpu))) {
+ doorbell_core_ipi(cpu);
+ ret = 1;
+ }
+
+ put_cpu();
+
+ return ret;
+}
+
+#endif /* CONFIG_SMP */
+
#endif /* _ASM_POWERPC_DBELL_H */
diff --git a/arch/powerpc/kernel/dbell.c b/arch/powerpc/kernel/dbell.c
index f17ff1200eaa..52680cf07c9d 100644
--- a/arch/powerpc/kernel/dbell.c
+++ b/arch/powerpc/kernel/dbell.c
@@ -18,61 +18,6 @@
#ifdef CONFIG_SMP
-/*
- * Doorbells must only be used if CPU_FTR_DBELL is available.
- * msgsnd is used in HV, and msgsndp is used in !HV.
- *
- * These should be used by platform code that is aware of restrictions.
- * Other arch code should use ->cause_ipi.
- *
- * doorbell_global_ipi() sends a dbell to any target CPU.
- * Must be used only by architectures that address msgsnd target
- * by PIR/get_hard_smp_processor_id.
- */
-void doorbell_global_ipi(int cpu)
-{
- u32 tag = get_hard_smp_processor_id(cpu);
-
- kvmppc_set_host_ipi(cpu);
- /* Order previous accesses vs. msgsnd, which is treated as a store */
- ppc_msgsnd_sync();
- ppc_msgsnd(PPC_DBELL_MSGTYPE, 0, tag);
-}
-
-/*
- * doorbell_core_ipi() sends a dbell to a target CPU in the same core.
- * Must be used only by architectures that address msgsnd target
- * by TIR/cpu_thread_in_core.
- */
-void doorbell_core_ipi(int cpu)
-{
- u32 tag = cpu_thread_in_core(cpu);
-
- kvmppc_set_host_ipi(cpu);
- /* Order previous accesses vs. msgsnd, which is treated as a store */
- ppc_msgsnd_sync();
- ppc_msgsnd(PPC_DBELL_MSGTYPE, 0, tag);
-}
-
-/*
- * Attempt to cause a core doorbell if destination is on the same core.
- * Returns 1 on success, 0 on failure.
- */
-int doorbell_try_core_ipi(int cpu)
-{
- int this_cpu = get_cpu();
- int ret = 0;
-
- if (cpumask_test_cpu(cpu, cpu_sibling_mask(this_cpu))) {
- doorbell_core_ipi(cpu);
- ret = 1;
- }
-
- put_cpu();
-
- return ret;
-}
-
void doorbell_exception(struct pt_regs *regs)
{
struct pt_regs *old_regs = set_irq_regs(regs);
--
2.23.0
^ permalink raw reply related
* [PATCH v3 2/3] powerpc/pseries: Use doorbells even if XIVE is available
From: Nicholas Piggin @ 2020-07-26 3:51 UTC (permalink / raw)
To: linuxppc-dev
Cc: kvm-ppc, Nicholas Piggin, Anton Blanchard, Cédric Le Goater,
David Gibson
In-Reply-To: <20200726035155.1424103-1-npiggin@gmail.com>
KVM supports msgsndp in guests by trapping and emulating the
instruction, so it was decided to always use XIVE for IPIs if it is
available. However on PowerVM systems, msgsndp can be used and gives
better performance. On large systems, high XIVE interrupt rates can
have sub-linear scaling, and using msgsndp can reduce the load on
the interrupt controller.
So switch to using core local doorbells even if XIVE is available.
This reduces performance for KVM guests with an SMT topology by
about 50% for ping-pong context switching between SMT vCPUs. An
option vector (or dt-cpu-ftrs) could be defined to disable msgsndp
to get KVM performance back.
Tested-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/platforms/pseries/smp.c | 54 ++++++++++++++++++----------
1 file changed, 36 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index 6891710833be..67e6ad5076ce 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -188,13 +188,16 @@ static int pseries_smp_prepare_cpu(int cpu)
return 0;
}
-static void smp_pseries_cause_ipi(int cpu)
+/* Cause IPI as setup by the interrupt controller (xics or xive) */
+static void (*ic_cause_ipi)(int cpu) __ro_after_init;
+
+/* Use msgsndp doorbells target is a sibling, else use interrupt controller */
+static void dbell_or_ic_cause_ipi(int cpu)
{
- /* POWER9 should not use this handler */
if (doorbell_try_core_ipi(cpu))
return;
- icp_ops->cause_ipi(cpu);
+ ic_cause_ipi(cpu);
}
static int pseries_cause_nmi_ipi(int cpu)
@@ -218,26 +221,41 @@ static int pseries_cause_nmi_ipi(int cpu)
return 0;
}
-static __init void pSeries_smp_probe_xics(void)
-{
- xics_smp_probe();
-
- if (cpu_has_feature(CPU_FTR_DBELL) && !is_secure_guest())
- smp_ops->cause_ipi = smp_pseries_cause_ipi;
- else
- smp_ops->cause_ipi = icp_ops->cause_ipi;
-}
-
static __init void pSeries_smp_probe(void)
{
if (xive_enabled())
- /*
- * Don't use P9 doorbells when XIVE is enabled. IPIs
- * using MMIOs should be faster
- */
xive_smp_probe();
else
- pSeries_smp_probe_xics();
+ xics_smp_probe();
+
+ /* No doorbell facility, must use the interrupt controller for IPIs */
+ if (!cpu_has_feature(CPU_FTR_DBELL))
+ return;
+
+ /* Doorbells can only be used for IPIs between SMT siblings */
+ if (!cpu_has_feature(CPU_FTR_SMT))
+ return;
+
+ /*
+ * KVM emulates doorbells by disabling FSCR[MSGP] so msgsndp faults
+ * to the hypervisor which then reads the instruction from guest
+ * memory. This can't be done if the guest is secure, so don't use
+ * doorbells in secure guests.
+ *
+ * Under PowerVM, FSCR[MSGP] is enabled so doorbells could be used
+ * by secure guests if we distinguished this from KVM.
+ */
+ if (is_secure_guest())
+ return;
+
+ /*
+ * The guest can use doobells for SMT sibling IPIs, which stay in
+ * the core rather than going to the interrupt controller. This
+ * tends to be slower under KVM where doorbells are emulated, but
+ * faster for PowerVM where they're enabled.
+ */
+ ic_cause_ipi = smp_ops->cause_ipi;
+ smp_ops->cause_ipi = dbell_or_ic_cause_ipi;
}
static struct smp_ops_t pseries_smp_ops = {
--
2.23.0
^ permalink raw reply related
* [PATCH v3 3/3] powerpc/pseries: Add KVM guest doorbell restrictions
From: Nicholas Piggin @ 2020-07-26 3:51 UTC (permalink / raw)
To: linuxppc-dev
Cc: kvm-ppc, Nicholas Piggin, Anton Blanchard, Cédric Le Goater,
David Gibson
In-Reply-To: <20200726035155.1424103-1-npiggin@gmail.com>
KVM guests have certain restrictions and performance quirks when using
doorbells. This patch moves the EPAPR KVM guest test so it can be shared
with PSERIES, and uses that in doorbell setup code to apply the KVM
guest quirks and improves IPI performance for two cases:
- PowerVM guests may now use doorbells even if they are secure.
- KVM guests no longer use doorbells if XIVE is available.
There is a valid complaint that "KVM guest" is not a very reasonable
thing to test for, it's preferable for the hypervisor to advertise
particular behaviours to the guest so they could change if the
hypervisor implementation or configuration changes. However in this case
we were already assuming a KVM guest worst case, so this patch is about
containing those quirks. If KVM later advertises fast doorbells, we
should test for that and override the quirks.
Tested-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/firmware.h | 6 +++++
arch/powerpc/include/asm/kvm_para.h | 26 +++----------------
arch/powerpc/kernel/Makefile | 5 ++--
arch/powerpc/kernel/firmware.c | 19 ++++++++++++++
arch/powerpc/platforms/pseries/smp.c | 38 +++++++++++++++++-----------
5 files changed, 53 insertions(+), 41 deletions(-)
diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index 6003c2e533a0..f67efbaba17f 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -132,6 +132,12 @@ extern int ibm_nmi_interlock_token;
extern unsigned int __start___fw_ftr_fixup, __stop___fw_ftr_fixup;
+#if defined(CONFIG_PPC_PSERIES) || defined(CONFIG_KVM_GUEST)
+bool is_kvm_guest(void);
+#else
+static inline bool is_kvm_guest(void) { return false; }
+#endif
+
#ifdef CONFIG_PPC_PSERIES
void pseries_probe_fw_features(void);
#else
diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
index 9c1f6b4b9bbf..744612054c94 100644
--- a/arch/powerpc/include/asm/kvm_para.h
+++ b/arch/powerpc/include/asm/kvm_para.h
@@ -8,35 +8,15 @@
#ifndef __POWERPC_KVM_PARA_H__
#define __POWERPC_KVM_PARA_H__
-#include <uapi/asm/kvm_para.h>
-
-#ifdef CONFIG_KVM_GUEST
-
-#include <linux/of.h>
-
-static inline int kvm_para_available(void)
-{
- struct device_node *hyper_node;
-
- hyper_node = of_find_node_by_path("/hypervisor");
- if (!hyper_node)
- return 0;
+#include <asm/firmware.h>
- if (!of_device_is_compatible(hyper_node, "linux,kvm"))
- return 0;
-
- return 1;
-}
-
-#else
+#include <uapi/asm/kvm_para.h>
static inline int kvm_para_available(void)
{
- return 0;
+ return IS_ENABLED(CONFIG_KVM_GUEST) && is_kvm_guest();
}
-#endif
-
static inline unsigned int kvm_arch_para_features(void)
{
unsigned long r;
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 244542ae2a91..852164439dcb 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -45,11 +45,10 @@ obj-y := cputable.o syscalls.o \
signal.o sysfs.o cacheinfo.o time.o \
prom.o traps.o setup-common.o \
udbg.o misc.o io.o misc_$(BITS).o \
- of_platform.o prom_parse.o
+ of_platform.o prom_parse.o firmware.o
obj-y += ptrace/
obj-$(CONFIG_PPC64) += setup_64.o \
- paca.o nvram_64.o firmware.o note.o \
- syscall_64.o
+ paca.o nvram_64.o note.o syscall_64.o
obj-$(CONFIG_COMPAT) += sys_ppc32.o signal_32.o
obj-$(CONFIG_VDSO32) += vdso32/
obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
diff --git a/arch/powerpc/kernel/firmware.c b/arch/powerpc/kernel/firmware.c
index cc4a5e3f51f1..fe48d319d490 100644
--- a/arch/powerpc/kernel/firmware.c
+++ b/arch/powerpc/kernel/firmware.c
@@ -11,8 +11,27 @@
#include <linux/export.h>
#include <linux/cache.h>
+#include <linux/of.h>
#include <asm/firmware.h>
+#ifdef CONFIG_PPC64
unsigned long powerpc_firmware_features __read_mostly;
EXPORT_SYMBOL_GPL(powerpc_firmware_features);
+#endif
+
+#if defined(CONFIG_PPC_PSERIES) || defined(CONFIG_KVM_GUEST)
+bool is_kvm_guest(void)
+{
+ struct device_node *hyper_node;
+
+ hyper_node = of_find_node_by_path("/hypervisor");
+ if (!hyper_node)
+ return 0;
+
+ if (!of_device_is_compatible(hyper_node, "linux,kvm"))
+ return 0;
+
+ return 1;
+}
+#endif
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index 67e6ad5076ce..7af0003b40b6 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -236,24 +236,32 @@ static __init void pSeries_smp_probe(void)
if (!cpu_has_feature(CPU_FTR_SMT))
return;
- /*
- * KVM emulates doorbells by disabling FSCR[MSGP] so msgsndp faults
- * to the hypervisor which then reads the instruction from guest
- * memory. This can't be done if the guest is secure, so don't use
- * doorbells in secure guests.
- *
- * Under PowerVM, FSCR[MSGP] is enabled so doorbells could be used
- * by secure guests if we distinguished this from KVM.
- */
- if (is_secure_guest())
- return;
+ if (is_kvm_guest()) {
+ /*
+ * KVM emulates doorbells by disabling FSCR[MSGP] so msgsndp
+ * faults to the hypervisor which then reads the instruction
+ * from guest memory, which tends to be slower than using XIVE.
+ */
+ if (xive_enabled())
+ return;
+
+ /*
+ * XICS hcalls aren't as fast, so we can use msgsndp (which
+ * also helps exercise KVM emulation), however KVM can't
+ * emulate secure guests because it can't read the instruction
+ * out of their memory.
+ */
+ if (is_secure_guest())
+ return;
+ }
/*
- * The guest can use doobells for SMT sibling IPIs, which stay in
- * the core rather than going to the interrupt controller. This
- * tends to be slower under KVM where doorbells are emulated, but
- * faster for PowerVM where they're enabled.
+ * Under PowerVM, FSCR[MSGP] is enabled as guest vCPU siblings are
+ * gang scheduled on the same physical core, so doorbells are always
+ * faster than the interrupt controller, and they can be used by
+ * secure guests.
*/
+
ic_cause_ipi = smp_ops->cause_ipi;
smp_ops->cause_ipi = dbell_or_ic_cause_ipi;
}
--
2.23.0
^ permalink raw reply related
* Re: [PATCH 1/2] lockdep: improve current->(hard|soft)irqs_enabled synchronisation with actual irq state
From: Nicholas Piggin @ 2020-07-26 4:14 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-arch, Alexey Kardashevskiy, Will Deacon, linux-kernel,
Ingo Molnar, linuxppc-dev
In-Reply-To: <20200725202617.GI10769@hirez.programming.kicks-ass.net>
Excerpts from Peter Zijlstra's message of July 26, 2020 6:26 am:
> On Thu, Jul 23, 2020 at 08:56:14PM +1000, Nicholas Piggin wrote:
>> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
>> index 3a0db7b0b46e..35060be09073 100644
>> --- a/arch/powerpc/include/asm/hw_irq.h
>> +++ b/arch/powerpc/include/asm/hw_irq.h
>> @@ -200,17 +200,14 @@ static inline bool arch_irqs_disabled(void)
>> #define powerpc_local_irq_pmu_save(flags) \
>> do { \
>> raw_local_irq_pmu_save(flags); \
>> - trace_hardirqs_off(); \
>> + if (!raw_irqs_disabled_flags(flags)) \
>> + trace_hardirqs_off(); \
>> } while(0)
>
> So one problem with the above is something like this:
>
> raw_local_irq_save();
> <NMI>
> powerpc_local_irq_pmu_save();
>
> that would now no longer call into tracing/lockdep at all. As a
> consequence, lockdep and tracing would show the NMI ran with IRQs
> enabled, which is exceptionally weird..
powerpc perf NMIs will trace_hardirqs_off() if they were enabled,
and trace_hardirqs_on() at exit in that case too.
Machine check and system reset (like x86's "nmi") don't, but that's
deliberate to avoid any tracing in those cases which often causes
more problems than it's worth (and we have flags to prevent ftrace,
etc too for those cases).
> Similar problem with:
>
> raw_local_irq_disable();
> local_irq_save()
>
> Now, most architectures today seem to do what x86 also did:
>
> <NMI>
> trace_hardirqs_off()
> ...
> if (irqs_unmasked(regs))
> trace_hardirqs_on()
> </NMI>
>
> Which is 'funny' when it interleaves like:
>
> local_irq_disable();
> ...
> local_irq_enable()
> trace_hardirqs_on();
> <NMI/>
> raw_local_irq_enable();
>
> Because then it will undo the trace_hardirqs_on() we just did. With the
> result that both tracing and lockdep will see a hardirqs-disable without
> a matching enable, while the hardware state is enabled.
Seems like an arch problem -- why not disable if it was enabled only?
I guess the local_irq tracing calls are a mess so maybe they copied
those.
> Which is exactly the state Alexey seems to have ran into.
No his was what I said, the interruptee's trace_hardirqs_on() in
local_irq_enable getting lost because the NMI's local_irq_disable
always disables, but the enable doesn't re-enable.
It's all just weird asymmetrical special case hacks AFAIKS, the
code should just be symmetric and lockdep handle it's own weirdness.
>
> Now, x86, and at least arm64 call nmi_enter() before
> trace_hardirqs_off(), but AFAICT Power never did that, and that's part
> of the problem. nmi_enter() does lockdep_off() and that _used_ to also
> kill IRQ tracking.
Power does do nmi_enter -- __perf_event_interrupt()
nmi = perf_intr_is_nmi(regs);
if (nmi)
nmi_enter();
else
irq_enter();
Thanks,
Nick
> Now, my patch changed that, it makes IRQ tracking not respect
> lockdep_off(). And that exposed x86 (and everybody else :/) to the same
> problem you have.
>
> And this is why I made x86 look at software state in NMIs. Because then
> it all works out. For bonus points, trace_hardirqs_*() also has some
> do-it-once logic for tracing.
>
>
>
> Anyway, it's Saturday evening, time for a beer. I'll stare at this more
> later.
>
^ permalink raw reply
* Re: [PATCH 1/2] lockdep: improve current->(hard|soft)irqs_enabled synchronisation with actual irq state
From: Alexey Kardashevskiy @ 2020-07-26 7:40 UTC (permalink / raw)
To: Nicholas Piggin, Peter Zijlstra
Cc: linux-arch, Will Deacon, Ingo Molnar, linuxppc-dev, linux-kernel
In-Reply-To: <1595568105.4eodjnxzwp.astroid@bobo.none>
On 24/07/2020 15:59, Nicholas Piggin wrote:
> Excerpts from Alexey Kardashevskiy's message of July 24, 2020 2:16 pm:
>>
>>
>> On 23/07/2020 23:11, Nicholas Piggin wrote:
>>> Excerpts from Peter Zijlstra's message of July 23, 2020 9:40 pm:
>>>> On Thu, Jul 23, 2020 at 08:56:14PM +1000, Nicholas Piggin wrote:
>>>>
>>>>> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
>>>>> index 3a0db7b0b46e..35060be09073 100644
>>>>> --- a/arch/powerpc/include/asm/hw_irq.h
>>>>> +++ b/arch/powerpc/include/asm/hw_irq.h
>>>>> @@ -200,17 +200,14 @@ static inline bool arch_irqs_disabled(void)
>>>>> #define powerpc_local_irq_pmu_save(flags) \
>>>>> do { \
>>>>> raw_local_irq_pmu_save(flags); \
>>>>> - trace_hardirqs_off(); \
>>>>> + if (!raw_irqs_disabled_flags(flags)) \
>>>>> + trace_hardirqs_off(); \
>>>>> } while(0)
>>>>> #define powerpc_local_irq_pmu_restore(flags) \
>>>>> do { \
>>>>> - if (raw_irqs_disabled_flags(flags)) { \
>>>>> - raw_local_irq_pmu_restore(flags); \
>>>>> - trace_hardirqs_off(); \
>>>>> - } else { \
>>>>> + if (!raw_irqs_disabled_flags(flags)) \
>>>>> trace_hardirqs_on(); \
>>>>> - raw_local_irq_pmu_restore(flags); \
>>>>> - } \
>>>>> + raw_local_irq_pmu_restore(flags); \
>>>>> } while(0)
>>>>
>>>> You shouldn't be calling lockdep from NMI context!
>>>
>>> After this patch it doesn't.
>>>
>>> trace_hardirqs_on/off implementation appears to expect to be called in NMI
>>> context though, for some reason.
>>>
>>>> That is, I recently
>>>> added suport for that on x86:
>>>>
>>>> https://lkml.kernel.org/r/20200623083721.155449112@infradead.org
>>>> https://lkml.kernel.org/r/20200623083721.216740948@infradead.org
>>>>
>>>> But you need to be very careful on how you order things, as you can see
>>>> the above relies on preempt_count() already having been incremented with
>>>> NMI_MASK.
>>>
>>> Hmm. My patch seems simpler.
>>
>> And your patches fix my error while Peter's do not:
>>
>>
>> IRQs not enabled as expected
>> WARNING: CPU: 0 PID: 1377 at /home/aik/p/kernel/kernel/softirq.c:169
>> __local_bh_enable_ip+0x118/0x190
>
> I think they would have needed some powerpc bits as well.
True, there is quite a lot to repeat of what x86 does, I was in a hurry
and did not think it through :)
> But I don't
> see a reason we can't merge my patches, at least they fix this case and
> don't seem to make things worse in any way.
True. Or we could keep these lockdep_stats::redundant_softirqs_on/etc
and make powerpc_local_irq_pmu_restore()/local_irq_restore() call
trace_hardirqs_on() always and let lockdep do reference counting, may be?
--
Alexey
^ permalink raw reply
* [PATCH] ASoC: fsl-asoc-card: Remove fsl_asoc_card_set_bias_level function
From: Shengjiu Wang @ 2020-07-26 11:20 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, lgirdwood, broonie,
perex, tiwai, alsa-devel, linuxppc-dev, linux-kernel
With this case:
aplay -Dhw:x 16khz.wav 24khz.wav
There is sound distortion for 24khz.wav. The reason is that setting
PLL of WM8962 with set_bias_level function, the bias level is not
changed when 24khz.wav is played, then the PLL won't be reset, the
clock is not correct, so distortion happens.
The resolution of this issue is to remove fsl_asoc_card_set_bias_level.
Move PLL configuration to hw_params and hw_free.
After removing fsl_asoc_card_set_bias_level, also test WM8960 case,
it can work.
Fixes: 708b4351f08c ("ASoC: fsl: Add Freescale Generic ASoC Sound Card with ASRC support")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl-asoc-card.c | 149 +++++++++++++++-------------------
1 file changed, 66 insertions(+), 83 deletions(-)
diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 4848ba61d083..0517dbb3e908 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -73,6 +73,7 @@ struct cpu_priv {
* @codec_priv: CODEC private data
* @cpu_priv: CPU private data
* @card: ASoC card structure
+ * @is_stream_in_use: flags for release resource in hw_free
* @sample_rate: Current sample rate
* @sample_format: Current sample format
* @asrc_rate: ASRC sample rate used by Back-Ends
@@ -89,6 +90,7 @@ struct fsl_asoc_card_priv {
struct codec_priv codec_priv;
struct cpu_priv cpu_priv;
struct snd_soc_card card;
+ bool is_stream_in_use[2];
u32 sample_rate;
snd_pcm_format_t sample_format;
u32 asrc_rate;
@@ -151,21 +153,17 @@ static int fsl_asoc_card_hw_params(struct snd_pcm_substream *substream,
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct fsl_asoc_card_priv *priv = snd_soc_card_get_drvdata(rtd->card);
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
+ struct codec_priv *codec_priv = &priv->codec_priv;
struct cpu_priv *cpu_priv = &priv->cpu_priv;
struct device *dev = rtd->card->dev;
+ unsigned int pll_out;
int ret;
priv->sample_rate = params_rate(params);
priv->sample_format = params_format(params);
+ priv->is_stream_in_use[tx] = true;
- /*
- * If codec-dai is DAI Master and all configurations are already in the
- * set_bias_level(), bypass the remaining settings in hw_params().
- * Note: (dai_fmt & CBM_CFM) includes CBM_CFM and CBM_CFS.
- */
- if ((priv->card.set_bias_level &&
- priv->dai_fmt & SND_SOC_DAIFMT_CBM_CFM) ||
- fsl_asoc_card_is_ac97(priv))
+ if (fsl_asoc_card_is_ac97(priv))
return 0;
/* Specific configurations of DAIs starts from here */
@@ -185,12 +183,72 @@ static int fsl_asoc_card_hw_params(struct snd_pcm_substream *substream,
return ret;
}
}
+ /* Specific configuration for PLL */
+ if (codec_priv->pll_id && codec_priv->fll_id) {
+ if (priv->sample_format == SNDRV_PCM_FORMAT_S24_LE)
+ pll_out = priv->sample_rate * 384;
+ else
+ pll_out = priv->sample_rate * 256;
+
+ ret = snd_soc_dai_set_pll(asoc_rtd_to_codec(rtd, 0),
+ codec_priv->pll_id,
+ codec_priv->mclk_id,
+ codec_priv->mclk_freq, pll_out);
+ if (ret) {
+ dev_err(dev, "failed to start FLL: %d\n", ret);
+ return ret;
+ }
+
+ ret = snd_soc_dai_set_sysclk(asoc_rtd_to_codec(rtd, 0),
+ codec_priv->fll_id,
+ pll_out, SND_SOC_CLOCK_IN);
+
+ if (ret && ret != -ENOTSUPP) {
+ dev_err(dev, "failed to set SYSCLK: %d\n", ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int fsl_asoc_card_hw_free(struct snd_pcm_substream *substream)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct fsl_asoc_card_priv *priv = snd_soc_card_get_drvdata(rtd->card);
+ bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
+ struct codec_priv *codec_priv = &priv->codec_priv;
+ struct device *dev = rtd->card->dev;
+ int ret;
+
+ priv->is_stream_in_use[tx] = false;
+
+ if (!priv->is_stream_in_use[!tx] && codec_priv->pll_id &&
+ codec_priv->fll_id) {
+ /* Force freq to be 0 to avoid error message in codec */
+ ret = snd_soc_dai_set_sysclk(asoc_rtd_to_codec(rtd, 0),
+ codec_priv->mclk_id,
+ 0,
+ SND_SOC_CLOCK_IN);
+ if (ret) {
+ dev_err(dev, "failed to switch away from FLL: %d\n", ret);
+ return ret;
+ }
+
+ ret = snd_soc_dai_set_pll(asoc_rtd_to_codec(rtd, 0),
+ codec_priv->pll_id, 0, 0, 0);
+ if (ret && ret != -ENOTSUPP) {
+ dev_err(dev, "failed to stop FLL: %d\n", ret);
+ return ret;
+ }
+ }
return 0;
}
static const struct snd_soc_ops fsl_asoc_card_ops = {
.hw_params = fsl_asoc_card_hw_params,
+ .hw_free = fsl_asoc_card_hw_free,
};
static int be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
@@ -254,75 +312,6 @@ static struct snd_soc_dai_link fsl_asoc_card_dai[] = {
},
};
-static int fsl_asoc_card_set_bias_level(struct snd_soc_card *card,
- struct snd_soc_dapm_context *dapm,
- enum snd_soc_bias_level level)
-{
- struct fsl_asoc_card_priv *priv = snd_soc_card_get_drvdata(card);
- struct snd_soc_pcm_runtime *rtd;
- struct snd_soc_dai *codec_dai;
- struct codec_priv *codec_priv = &priv->codec_priv;
- struct device *dev = card->dev;
- unsigned int pll_out;
- int ret;
-
- rtd = snd_soc_get_pcm_runtime(card, &card->dai_link[0]);
- codec_dai = asoc_rtd_to_codec(rtd, 0);
- if (dapm->dev != codec_dai->dev)
- return 0;
-
- switch (level) {
- case SND_SOC_BIAS_PREPARE:
- if (dapm->bias_level != SND_SOC_BIAS_STANDBY)
- break;
-
- if (priv->sample_format == SNDRV_PCM_FORMAT_S24_LE)
- pll_out = priv->sample_rate * 384;
- else
- pll_out = priv->sample_rate * 256;
-
- ret = snd_soc_dai_set_pll(codec_dai, codec_priv->pll_id,
- codec_priv->mclk_id,
- codec_priv->mclk_freq, pll_out);
- if (ret) {
- dev_err(dev, "failed to start FLL: %d\n", ret);
- return ret;
- }
-
- ret = snd_soc_dai_set_sysclk(codec_dai, codec_priv->fll_id,
- pll_out, SND_SOC_CLOCK_IN);
- if (ret && ret != -ENOTSUPP) {
- dev_err(dev, "failed to set SYSCLK: %d\n", ret);
- return ret;
- }
- break;
-
- case SND_SOC_BIAS_STANDBY:
- if (dapm->bias_level != SND_SOC_BIAS_PREPARE)
- break;
-
- ret = snd_soc_dai_set_sysclk(codec_dai, codec_priv->mclk_id,
- codec_priv->mclk_freq,
- SND_SOC_CLOCK_IN);
- if (ret && ret != -ENOTSUPP) {
- dev_err(dev, "failed to switch away from FLL: %d\n", ret);
- return ret;
- }
-
- ret = snd_soc_dai_set_pll(codec_dai, codec_priv->pll_id, 0, 0, 0);
- if (ret) {
- dev_err(dev, "failed to stop FLL: %d\n", ret);
- return ret;
- }
- break;
-
- default:
- break;
- }
-
- return 0;
-}
-
static int fsl_asoc_card_audmux_init(struct device_node *np,
struct fsl_asoc_card_priv *priv)
{
@@ -611,7 +600,6 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
/* Diversify the card configurations */
if (of_device_is_compatible(np, "fsl,imx-audio-cs42888")) {
codec_dai_name = "cs42888";
- priv->card.set_bias_level = NULL;
priv->cpu_priv.sysclk_freq[TX] = priv->codec_priv.mclk_freq;
priv->cpu_priv.sysclk_freq[RX] = priv->codec_priv.mclk_freq;
priv->cpu_priv.sysclk_dir[TX] = SND_SOC_CLOCK_OUT;
@@ -628,26 +616,22 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
} else if (of_device_is_compatible(np, "fsl,imx-audio-wm8962")) {
codec_dai_name = "wm8962";
- priv->card.set_bias_level = fsl_asoc_card_set_bias_level;
priv->codec_priv.mclk_id = WM8962_SYSCLK_MCLK;
priv->codec_priv.fll_id = WM8962_SYSCLK_FLL;
priv->codec_priv.pll_id = WM8962_FLL;
priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
} else if (of_device_is_compatible(np, "fsl,imx-audio-wm8960")) {
codec_dai_name = "wm8960-hifi";
- priv->card.set_bias_level = fsl_asoc_card_set_bias_level;
priv->codec_priv.fll_id = WM8960_SYSCLK_AUTO;
priv->codec_priv.pll_id = WM8960_SYSCLK_AUTO;
priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
} else if (of_device_is_compatible(np, "fsl,imx-audio-ac97")) {
codec_dai_name = "ac97-hifi";
- priv->card.set_bias_level = NULL;
priv->dai_fmt = SND_SOC_DAIFMT_AC97;
priv->card.dapm_routes = audio_map_ac97;
priv->card.num_dapm_routes = ARRAY_SIZE(audio_map_ac97);
} else if (of_device_is_compatible(np, "fsl,imx-audio-mqs")) {
codec_dai_name = "fsl-mqs-dai";
- priv->card.set_bias_level = NULL;
priv->dai_fmt = SND_SOC_DAIFMT_LEFT_J |
SND_SOC_DAIFMT_CBS_CFS |
SND_SOC_DAIFMT_NB_NF;
@@ -657,7 +641,6 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
priv->card.num_dapm_routes = ARRAY_SIZE(audio_map_tx);
} else if (of_device_is_compatible(np, "fsl,imx-audio-wm8524")) {
codec_dai_name = "wm8524-hifi";
- priv->card.set_bias_level = NULL;
priv->dai_fmt |= SND_SOC_DAIFMT_CBS_CFS;
priv->dai_link[1].dpcm_capture = 0;
priv->dai_link[2].dpcm_capture = 0;
--
2.27.0
^ permalink raw reply related
* Re: [PATCH 1/2] lockdep: improve current->(hard|soft)irqs_enabled synchronisation with actual irq state
From: peterz @ 2020-07-26 11:59 UTC (permalink / raw)
To: Nicholas Piggin
Cc: linux-arch, Alexey Kardashevskiy, Will Deacon, linux-kernel,
Ingo Molnar, linuxppc-dev
In-Reply-To: <1595735694.b784cvipam.astroid@bobo.none>
On Sun, Jul 26, 2020 at 02:14:34PM +1000, Nicholas Piggin wrote:
> > Now, x86, and at least arm64 call nmi_enter() before
> > trace_hardirqs_off(), but AFAICT Power never did that, and that's part
> > of the problem. nmi_enter() does lockdep_off() and that _used_ to also
> > kill IRQ tracking.
>
> Power does do nmi_enter -- __perf_event_interrupt()
>
> nmi = perf_intr_is_nmi(regs);
> if (nmi)
> nmi_enter();
> else
> irq_enter();
That's _waaaay_ too late, you've done the trace_hardirqs_{off,on} in
arch/powerpc/kernel/exceptions-64e.S, you need to _first_ do nmi_enter()
and _then_ trace_hardirqs_off(), or rather, that's still broken, but
then we get into the details of entry ordering.
^ permalink raw reply
* Re: [PATCH 1/2] lockdep: improve current->(hard|soft)irqs_enabled synchronisation with actual irq state
From: peterz @ 2020-07-26 12:11 UTC (permalink / raw)
To: Nicholas Piggin
Cc: linux-arch, Alexey Kardashevskiy, Will Deacon, linux-kernel,
Ingo Molnar, linuxppc-dev
In-Reply-To: <1595735694.b784cvipam.astroid@bobo.none>
On Sun, Jul 26, 2020 at 02:14:34PM +1000, Nicholas Piggin wrote:
> Excerpts from Peter Zijlstra's message of July 26, 2020 6:26 am:
> > Which is 'funny' when it interleaves like:
> >
> > local_irq_disable();
> > ...
> > local_irq_enable()
> > trace_hardirqs_on();
> > <NMI/>
> > raw_local_irq_enable();
> >
> > Because then it will undo the trace_hardirqs_on() we just did. With the
> > result that both tracing and lockdep will see a hardirqs-disable without
> > a matching enable, while the hardware state is enabled.
>
> Seems like an arch problem -- why not disable if it was enabled only?
> I guess the local_irq tracing calls are a mess so maybe they copied
> those.
Because, as I wrote earlier, then we can miss updating software state.
So your proposal has:
raw_local_irq_disable()
<NMI>
if (!arch_irqs_disabled(regs->flags) // false
trace_hardirqs_off();
// tracing/lockdep still think IRQs are enabled
// hardware IRQ state is disabled.
With the current code we have:
local_irq_enable()
trace_hardirqs_on();
<NMI>
trace_hardirqs_off();
...
if (!arch_irqs_disabled(regs->flags)) // false
trace_hardirqs_on();
</NMI>
// and now the NMI disabled software state again
// while we're about to enable the hardware state
raw_local_irq_enable();
> > Which is exactly the state Alexey seems to have ran into.
>
> No his was what I said, the interruptee's trace_hardirqs_on() in
> local_irq_enable getting lost because the NMI's local_irq_disable
> always disables, but the enable doesn't re-enable.
That's _exactly_ the case above. It doesn't re-enable because hardirqs
are actually still disabled. You _cannot_ rely on hardirq state for
NMIs, that'll get you wrong state.
> It's all just weird asymmetrical special case hacks AFAIKS, the
> code should just be symmetric and lockdep handle it's own weirdness.
It's for non-maskable exceptions/interrupts, because there the hardware
and software state changes non-atomically. For maskable interrupts doing
the software state transitions inside the disabled region makes perfect
sense, because that keeps it atomic.
^ permalink raw reply
* [RESEND PATCH v2 0/2] powerpc/papr_scm: add support for reporting NVDIMM 'life_used_percentage' metric
From: Vaibhav Jain @ 2020-07-26 12:20 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Santosh Sivaraj, Oliver O'Halloran, Aneesh Kumar K . V,
Vaibhav Jain, Dan Williams
Changes since v2[1]:
* Rebased the patch series on latest ppc-next tree located [2]
[1] https://lore.kernel.org/linux-nvdimm/20200701133510.4613-1-vaibhav@linux.ibm.com/
[2] git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git
---
This small patchset implements kernel side support for reporting
'life_used_percentage' metric in NDCTL with dimm health output for
papr-scm NVDIMMs. With corresponding NDCTL side changes output for
should be like:
$ sudo ndctl list -DH
[
{
"dev":"nmem0",
"health":{
"health_state":"ok",
"life_used_percentage":0,
"shutdown_state":"clean"
}
}
]
PHYP supports H_SCM_PERFORMANCE_STATS hcall through which an LPAR can
fetch various performance stats including 'fuel_gauge' percentage for
an NVDIMM. 'fuel_gauge' metric indicates the usable life remaining of
an NVDIMM expressed as percentage and 'life_used_percentage' can be
calculated as 'life_used_percentage = 100 - fuel_gauge'.
Structure of the patchset
=========================
First patch implements necessary scaffolding needed to issue the
H_SCM_PERFORMANCE_STATS hcall and fetch performance stats
catalogue. The patch also implements support for 'perf_stats' sysfs
attribute to report the full catalogue of supported performance stats
by PHYP.
Second and final patch implements support for sending this value to
libndctl by extending the PAPR_PDSM_HEALTH pdsm payload to add a new
field named 'dimm_fuel_gauge' to it.
Vaibhav Jain (2):
powerpc/papr_scm: Fetch nvdimm performance stats from PHYP
powerpc/papr_scm: Add support for fetching nvdimm 'fuel-gauge' metric
Documentation/ABI/testing/sysfs-bus-papr-pmem | 27 +++
arch/powerpc/include/uapi/asm/papr_pdsm.h | 9 +
arch/powerpc/platforms/pseries/papr_scm.c | 184 ++++++++++++++++++
3 files changed, 220 insertions(+)
--
2.26.2
^ permalink raw reply
* [RESEND PATCH v2 1/2] powerpc/papr_scm: Fetch nvdimm performance stats from PHYP
From: Vaibhav Jain @ 2020-07-26 12:20 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Santosh Sivaraj, Oliver O'Halloran, Aneesh Kumar K . V,
Vaibhav Jain, Dan Williams
In-Reply-To: <20200726122030.31529-1-vaibhav@linux.ibm.com>
Update papr_scm.c to query dimm performance statistics from PHYP via
H_SCM_PERFORMANCE_STATS hcall and export them to user-space as PAPR
specific NVDIMM attribute 'perf_stats' in sysfs. The patch also
provide a sysfs ABI documentation for the stats being reported and
their meanings.
During NVDIMM probe time in papr_scm_nvdimm_init() a special variant
of H_SCM_PERFORMANCE_STATS hcall is issued to check if collection of
performance statistics is supported or not. If successful then a PHYP
returns a maximum possible buffer length needed to read all
performance stats. This returned value is stored in a per-nvdimm
attribute 'stat_buffer_len'.
The layout of request buffer for reading NVDIMM performance stats from
PHYP is defined in 'struct papr_scm_perf_stats' and 'struct
papr_scm_perf_stat'. These structs are used in newly introduced
drc_pmem_query_stats() that issues the H_SCM_PERFORMANCE_STATS hcall.
The sysfs access function perf_stats_show() uses value
'stat_buffer_len' to allocate a buffer large enough to hold all
possible NVDIMM performance stats and passes it to
drc_pmem_query_stats() to populate. Finally statistics reported in the
buffer are formatted into the sysfs access function output buffer.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
Resend:
None
v2:
* Updated 'struct papr_scm_perf_stats' and 'struct papr_scm_perf_stat'
to use big-endian types. [ Aneesh ]
* s/len_stat_buffer/stat_buffer_len/ [ Aneesh ]
* s/statistics_id/stat_id/ , s/statistics_val/stat_val/ [ Aneesh ]
* Conversion from Big endian to cpu endian happens later rather than
just after its fetched from PHYP.
* Changed a log statement to unambiguously report dimm performance
stats are not available for the given nvdimm [ Ira ]
* Restructed some code to handle error case first [ Ira ]
---
Documentation/ABI/testing/sysfs-bus-papr-pmem | 27 ++++
arch/powerpc/platforms/pseries/papr_scm.c | 135 ++++++++++++++++++
2 files changed, 162 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-papr-pmem b/Documentation/ABI/testing/sysfs-bus-papr-pmem
index 5b10d036a8d4..c1a67275c43f 100644
--- a/Documentation/ABI/testing/sysfs-bus-papr-pmem
+++ b/Documentation/ABI/testing/sysfs-bus-papr-pmem
@@ -25,3 +25,30 @@ Description:
NVDIMM have been scrubbed.
* "locked" : Indicating that NVDIMM contents cant
be modified until next power cycle.
+
+What: /sys/bus/nd/devices/nmemX/papr/perf_stats
+Date: May, 2020
+KernelVersion: v5.9
+Contact: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, linux-nvdimm@lists.01.org,
+Description:
+ (RO) Report various performance stats related to papr-scm NVDIMM
+ device. Each stat is reported on a new line with each line
+ composed of a stat-identifier followed by it value. Below are
+ currently known dimm performance stats which are reported:
+
+ * "CtlResCt" : Controller Reset Count
+ * "CtlResTm" : Controller Reset Elapsed Time
+ * "PonSecs " : Power-on Seconds
+ * "MemLife " : Life Remaining
+ * "CritRscU" : Critical Resource Utilization
+ * "HostLCnt" : Host Load Count
+ * "HostSCnt" : Host Store Count
+ * "HostSDur" : Host Store Duration
+ * "HostLDur" : Host Load Duration
+ * "MedRCnt " : Media Read Count
+ * "MedWCnt " : Media Write Count
+ * "MedRDur " : Media Read Duration
+ * "MedWDur " : Media Write Duration
+ * "CchRHCnt" : Cache Read Hit Count
+ * "CchWHCnt" : Cache Write Hit Count
+ * "FastWCnt" : Fast Write Count
\ No newline at end of file
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 8fd441d32487..3172a01ffff8 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -64,6 +64,26 @@
PAPR_PMEM_HEALTH_FATAL | \
PAPR_PMEM_HEALTH_UNHEALTHY)
+#define PAPR_SCM_PERF_STATS_EYECATCHER __stringify(SCMSTATS)
+#define PAPR_SCM_PERF_STATS_VERSION 0x1
+
+/* Struct holding a single performance metric */
+struct papr_scm_perf_stat {
+ u8 stat_id[8];
+ __be64 stat_val;
+} __packed;
+
+/* Struct exchanged between kernel and PHYP for fetching drc perf stats */
+struct papr_scm_perf_stats {
+ u8 eye_catcher[8];
+ /* Should be PAPR_SCM_PERF_STATS_VERSION */
+ __be32 stats_version;
+ /* Number of stats following */
+ __be32 num_statistics;
+ /* zero or more performance matrics */
+ struct papr_scm_perf_stat scm_statistic[];
+} __packed;
+
/* private struct associated with each region */
struct papr_scm_priv {
struct platform_device *pdev;
@@ -92,6 +112,9 @@ struct papr_scm_priv {
/* Health information for the dimm */
u64 health_bitmap;
+
+ /* length of the stat buffer as expected by phyp */
+ size_t stat_buffer_len;
};
LIST_HEAD(papr_nd_regions);
@@ -200,6 +223,65 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)
return drc_pmem_bind(p);
}
+/*
+ * Query the Dimm performance stats from PHYP and copy them (if returned) to
+ * provided struct papr_scm_perf_stats instance 'stats' of 'size' in bytes.
+ * The value of R4 is copied to 'out' if the pointer is provided.
+ */
+static int drc_pmem_query_stats(struct papr_scm_priv *p,
+ struct papr_scm_perf_stats *buff_stats,
+ size_t size, unsigned int num_stats,
+ uint64_t *out)
+{
+ unsigned long ret[PLPAR_HCALL_BUFSIZE];
+ s64 rc;
+
+ /* Setup the out buffer */
+ if (buff_stats) {
+ memcpy(buff_stats->eye_catcher,
+ PAPR_SCM_PERF_STATS_EYECATCHER, 8);
+ buff_stats->stats_version =
+ cpu_to_be32(PAPR_SCM_PERF_STATS_VERSION);
+ buff_stats->num_statistics =
+ cpu_to_be32(num_stats);
+ } else {
+ /* In case of no out buffer ignore the size */
+ size = 0;
+ }
+
+ /*
+ * Do the HCALL asking PHYP for info and if R4 was requested
+ * return its value in 'out' variable.
+ */
+ rc = plpar_hcall(H_SCM_PERFORMANCE_STATS, ret, p->drc_index,
+ buff_stats ? virt_to_phys(buff_stats) : 0,
+ size);
+ if (out)
+ *out = ret[0];
+
+ if (rc == H_PARTIAL) {
+ dev_err(&p->pdev->dev,
+ "Unknown performance stats, Err:0x%016lX\n", ret[0]);
+ return -ENOENT;
+ } else if (rc != H_SUCCESS) {
+ dev_err(&p->pdev->dev,
+ "Failed to query performance stats, Err:%lld\n", rc);
+ return -EIO;
+ }
+
+ /* Successfully fetched the requested stats from phyp */
+ if (size != 0)
+ dev_dbg(&p->pdev->dev,
+ "Performance stats returned %d stats\n",
+ be32_to_cpu(buff_stats->num_statistics));
+ else
+ /* Handle case where stat buffer size was requested */
+ dev_dbg(&p->pdev->dev,
+ "Performance stats size %ld\n", ret[0]);
+
+ return 0;
+}
+
/*
* Issue hcall to retrieve dimm health info and populate papr_scm_priv with the
* health information.
@@ -637,6 +719,48 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
return 0;
}
+static ssize_t perf_stats_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int index, rc;
+ struct seq_buf s;
+ struct papr_scm_perf_stat *stat;
+ struct papr_scm_perf_stats *stats;
+ struct nvdimm *dimm = to_nvdimm(dev);
+ struct papr_scm_priv *p = nvdimm_provider_data(dimm);
+
+ if (!p->stat_buffer_len)
+ return -ENOENT;
+
+ /* Allocate the buffer for phyp where stats are written */
+ stats = kzalloc(p->stat_buffer_len, GFP_KERNEL);
+ if (!stats)
+ return -ENOMEM;
+
+ /* Ask phyp to return all dimm perf stats */
+ rc = drc_pmem_query_stats(p, stats, p->stat_buffer_len, 0, NULL);
+ if (rc)
+ goto free_stats;
+ /*
+ * Go through the returned output buffer and print stats and
+ * values. Since stat_id is essentially a char string of
+ * 8 bytes, simply use the string format specifier to print it.
+ */
+ seq_buf_init(&s, buf, PAGE_SIZE);
+ for (index = 0, stat = stats->scm_statistic;
+ index < be32_to_cpu(stats->num_statistics);
+ ++index, ++stat) {
+ seq_buf_printf(&s, "%.8s = 0x%016llX\n",
+ stat->stat_id,
+ be64_to_cpu(stat->stat_val));
+ }
+
+free_stats:
+ kfree(stats);
+ return rc ? rc : seq_buf_used(&s);
+}
+DEVICE_ATTR_RO(perf_stats);
+
static ssize_t flags_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -682,6 +806,7 @@ DEVICE_ATTR_RO(flags);
/* papr_scm specific dimm attributes */
static struct attribute *papr_nd_attributes[] = {
&dev_attr_flags.attr,
+ &dev_attr_perf_stats.attr,
NULL,
};
@@ -702,6 +827,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
struct nd_region_desc ndr_desc;
unsigned long dimm_flags;
int target_nid, online_nid;
+ u64 stat_size;
p->bus_desc.ndctl = papr_scm_ndctl;
p->bus_desc.module = THIS_MODULE;
@@ -769,6 +895,15 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
list_add_tail(&p->region_list, &papr_nd_regions);
mutex_unlock(&papr_ndr_lock);
+ /* Try retriving the stat buffer and see if its supported */
+ if (!drc_pmem_query_stats(p, NULL, 0, 0, &stat_size)) {
+ p->stat_buffer_len = (size_t)stat_size;
+ dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
+ p->stat_buffer_len);
+ } else {
+ dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n");
+ }
+
return 0;
err: nvdimm_bus_unregister(p->bus);
--
2.26.2
^ permalink raw reply related
* [RESEND PATCH v2 2/2] powerpc/papr_scm: Add support for fetching nvdimm 'fuel-gauge' metric
From: Vaibhav Jain @ 2020-07-26 12:20 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Santosh Sivaraj, Oliver O'Halloran, Aneesh Kumar K . V,
Vaibhav Jain, Dan Williams
In-Reply-To: <20200726122030.31529-1-vaibhav@linux.ibm.com>
We add support for reporting 'fuel-gauge' NVDIMM metric via
PAPR_PDSM_HEALTH pdsm payload. 'fuel-gauge' metric indicates the usage
life remaining of a papr-scm compatible NVDIMM. PHYP exposes this
metric via the H_SCM_PERFORMANCE_STATS.
The metric value is returned from the pdsm by extending the return
payload 'struct nd_papr_pdsm_health' without breaking the ABI. A new
field 'dimm_fuel_gauge' to hold the metric value is introduced at the
end of the payload struct and its presence is indicated by by
extension flag PDSM_DIMM_HEALTH_RUN_GAUGE_VALID.
The patch introduces a new function papr_pdsm_fuel_gauge() that is
called from papr_pdsm_health(). If fetching NVDIMM performance stats
is supported then 'papr_pdsm_fuel_gauge()' allocated an output buffer
large enough to hold the performance stat and passes it to
drc_pmem_query_stats() that issues the HCALL to PHYP. The return value
of the stat is then populated in the 'struct
nd_papr_pdsm_health.dimm_fuel_gauge' field with extension flag
'PDSM_DIMM_HEALTH_RUN_GAUGE_VALID' set in 'struct
nd_papr_pdsm_health.extension_flags'
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
Resend:
None
v2:
* Restructure code in papr_pdsm_fuel_gauge() to handle error case
first [ Ira ]
* Ignore the return value of papr_pdsm_fuel_gauge() in
papr_psdm_health() [ Ira ]
---
arch/powerpc/include/uapi/asm/papr_pdsm.h | 9 +++++
arch/powerpc/platforms/pseries/papr_scm.c | 49 +++++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/arch/powerpc/include/uapi/asm/papr_pdsm.h b/arch/powerpc/include/uapi/asm/papr_pdsm.h
index 9ccecc1d6840..50ef95e2f5b1 100644
--- a/arch/powerpc/include/uapi/asm/papr_pdsm.h
+++ b/arch/powerpc/include/uapi/asm/papr_pdsm.h
@@ -72,6 +72,11 @@
#define PAPR_PDSM_DIMM_CRITICAL 2
#define PAPR_PDSM_DIMM_FATAL 3
+/* struct nd_papr_pdsm_health.extension_flags field flags */
+
+/* Indicate that the 'dimm_fuel_gauge' field is valid */
+#define PDSM_DIMM_HEALTH_RUN_GAUGE_VALID 1
+
/*
* Struct exchanged between kernel & ndctl in for PAPR_PDSM_HEALTH
* Various flags indicate the health status of the dimm.
@@ -84,6 +89,7 @@
* dimm_locked : Contents of the dimm cant be modified until CEC reboot
* dimm_encrypted : Contents of dimm are encrypted.
* dimm_health : Dimm health indicator. One of PAPR_PDSM_DIMM_XXXX
+ * dimm_fuel_gauge : Life remaining of DIMM as a percentage from 0-100
*/
struct nd_papr_pdsm_health {
union {
@@ -96,6 +102,9 @@ struct nd_papr_pdsm_health {
__u8 dimm_locked;
__u8 dimm_encrypted;
__u16 dimm_health;
+
+ /* Extension flag PDSM_DIMM_HEALTH_RUN_GAUGE_VALID */
+ __u16 dimm_fuel_gauge;
};
__u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
};
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 3172a01ffff8..80261d241831 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -504,6 +504,51 @@ static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
return 0;
}
+static int papr_pdsm_fuel_gauge(struct papr_scm_priv *p,
+ union nd_pdsm_payload *payload)
+{
+ int rc, size;
+ u64 statval;
+ struct papr_scm_perf_stat *stat;
+ struct papr_scm_perf_stats *stats;
+
+ /* Silently fail if fetching performance metrics isn't supported */
+ if (!p->stat_buffer_len)
+ return 0;
+
+ /* Allocate request buffer enough to hold single performance stat */
+ size = sizeof(struct papr_scm_perf_stats) +
+ sizeof(struct papr_scm_perf_stat);
+
+ stats = kzalloc(size, GFP_KERNEL);
+ if (!stats)
+ return -ENOMEM;
+
+ stat = &stats->scm_statistic[0];
+ memcpy(&stat->stat_id, "MemLife ", sizeof(stat->stat_id));
+ stat->stat_val = 0;
+
+ /* Fetch the fuel gauge and populate it in payload */
+ rc = drc_pmem_query_stats(p, stats, size, 1, NULL);
+ if (rc) {
+ dev_dbg(&p->pdev->dev, "Err(%d) fetching fuel gauge\n", rc);
+ goto free_stats;
+ }
+
+ statval = be64_to_cpu(stat->stat_val);
+ dev_dbg(&p->pdev->dev,
+ "Fetched fuel-gauge %llu", statval);
+ payload->health.extension_flags |=
+ PDSM_DIMM_HEALTH_RUN_GAUGE_VALID;
+ payload->health.dimm_fuel_gauge = statval;
+
+ rc = sizeof(struct nd_papr_pdsm_health);
+
+free_stats:
+ kfree(stats);
+ return rc;
+}
+
/* Fetch the DIMM health info and populate it in provided package. */
static int papr_pdsm_health(struct papr_scm_priv *p,
union nd_pdsm_payload *payload)
@@ -544,6 +589,10 @@ static int papr_pdsm_health(struct papr_scm_priv *p,
/* struct populated hence can release the mutex now */
mutex_unlock(&p->health_mutex);
+
+ /* Populate the fuel gauge meter in the payload */
+ papr_pdsm_fuel_gauge(p, payload);
+
rc = sizeof(struct nd_papr_pdsm_health);
out:
--
2.26.2
^ permalink raw reply related
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