* Re: [PATCH v2 04/10] powerpc/perf: Add power10_feat to dt_cpu_ftrs
From: Athira Rajeev @ 2020-07-08 2:13 UTC (permalink / raw)
To: Michael Neuling; +Cc: maddy, linuxppc-dev
In-Reply-To: <fbc244b88f9291e83b2eabedd73ec9672b1fa12d.camel@neuling.org>
> On 07-Jul-2020, at 11:52 AM, Michael Neuling <mikey@neuling.org> wrote:
>
> On Wed, 2020-07-01 at 05:20 -0400, Athira Rajeev wrote:
>> From: Madhavan Srinivasan <maddy@linux.ibm.com>
>>
>> Add power10 feature function to dt_cpu_ftrs.c along
>> with a power10 specific init() to initialize pmu sprs.
>
> Can you say why you're doing this?
>
> Can you add some text about what you're doing to the BHRB in this patch?
Sure, I will include these information for commit message in the next version
Thanks
Athira
>
> Mikey
>
>>
>> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/reg.h | 3 +++
>> arch/powerpc/kernel/cpu_setup_power.S | 7 +++++++
>> arch/powerpc/kernel/dt_cpu_ftrs.c | 26 ++++++++++++++++++++++++++
>> 3 files changed, 36 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
>> index 21a1b2d..900ada1 100644
>> --- a/arch/powerpc/include/asm/reg.h
>> +++ b/arch/powerpc/include/asm/reg.h
>> @@ -1068,6 +1068,9 @@
>> #define MMCR0_PMC2_LOADMISSTIME 0x5
>> #endif
>>
>> +/* BHRB disable bit for PowerISA v3.10 */
>> +#define MMCRA_BHRB_DISABLE 0x0000002000000000
>> +
>> /*
>> * SPRG usage:
>> *
>> diff --git a/arch/powerpc/kernel/cpu_setup_power.S
>> b/arch/powerpc/kernel/cpu_setup_power.S
>> index efdcfa7..e8b3370c 100644
>> --- a/arch/powerpc/kernel/cpu_setup_power.S
>> +++ b/arch/powerpc/kernel/cpu_setup_power.S
>> @@ -233,3 +233,10 @@ __init_PMU_ISA207:
>> li r5,0
>> mtspr SPRN_MMCRS,r5
>> blr
>> +
>> +__init_PMU_ISA31:
>> + li r5,0
>> + mtspr SPRN_MMCR3,r5
>> + LOAD_REG_IMMEDIATE(r5, MMCRA_BHRB_DISABLE)
>> + mtspr SPRN_MMCRA,r5
>> + blr
>> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c
>> b/arch/powerpc/kernel/dt_cpu_ftrs.c
>> index a0edeb3..14a513f 100644
>> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
>> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
>> @@ -449,6 +449,31 @@ static int __init feat_enable_pmu_power9(struct
>> dt_cpu_feature *f)
>> return 1;
>> }
>>
>> +static void init_pmu_power10(void)
>> +{
>> + init_pmu_power9();
>> +
>> + mtspr(SPRN_MMCR3, 0);
>> + mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);
>> +}
>> +
>> +static int __init feat_enable_pmu_power10(struct dt_cpu_feature *f)
>> +{
>> + hfscr_pmu_enable();
>> +
>> + init_pmu_power10();
>> + init_pmu_registers = init_pmu_power10;
>> +
>> + cur_cpu_spec->cpu_features |= CPU_FTR_MMCRA;
>> + cur_cpu_spec->cpu_user_features |= PPC_FEATURE_PSERIES_PERFMON_COMPAT;
>> +
>> + cur_cpu_spec->num_pmcs = 6;
>> + cur_cpu_spec->pmc_type = PPC_PMC_IBM;
>> + cur_cpu_spec->oprofile_cpu_type = "ppc64/power10";
>> +
>> + return 1;
>> +}
>> +
>> static int __init feat_enable_tm(struct dt_cpu_feature *f)
>> {
>> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>> @@ -638,6 +663,7 @@ struct dt_cpu_feature_match {
>> {"pc-relative-addressing", feat_enable, 0},
>> {"machine-check-power9", feat_enable_mce_power9, 0},
>> {"performance-monitor-power9", feat_enable_pmu_power9, 0},
>> + {"performance-monitor-power10", feat_enable_pmu_power10, 0},
>> {"event-based-branch-v3", feat_enable, 0},
>> {"random-number-generator", feat_enable, 0},
>> {"system-call-vectored", feat_disable, 0},
>
^ permalink raw reply
* Re: [PATCH v2 2/4] powerpc/mm/radix: Free PUD table when freeing pagetable
From: Aneesh Kumar K.V @ 2020-07-08 2:48 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: Bharata B Rao
In-Reply-To: <87a70a4uw2.fsf@mpe.ellerman.id.au>
On 7/8/20 7:42 AM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> From: Bharata B Rao <bharata@linux.ibm.com>
>>
>> remove_pagetable() isn't freeing PUD table. This causes memory
>> leak during memory unplug. Fix this.
>
Fixes: 4b5d62ca17a1 ("powerpc/mm: add radix__remove_section_mapping()")
> Fixes: ??
>
> cheers
>
>> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> arch/powerpc/mm/book3s64/radix_pgtable.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
>> index 80be96d3018f..af57604f295f 100644
>> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
>> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
>> @@ -708,6 +708,21 @@ static void free_pmd_table(pmd_t *pmd_start, pud_t *pud)
>> pud_clear(pud);
>> }
>>
>> +static void free_pud_table(pud_t *pud_start, p4d_t *p4d)
>> +{
>> + pud_t *pud;
>> + int i;
>> +
>> + for (i = 0; i < PTRS_PER_PUD; i++) {
>> + pud = pud_start + i;
>> + if (!pud_none(*pud))
>> + return;
>> + }
>> +
>> + pud_free(&init_mm, pud_start);
>> + p4d_clear(p4d);
>> +}
>> +
>> struct change_mapping_params {
>> pte_t *pte;
>> unsigned long start;
>> @@ -882,6 +897,7 @@ static void __meminit remove_pagetable(unsigned long start, unsigned long end)
>>
>> pud_base = (pud_t *)p4d_page_vaddr(*p4d);
>> remove_pud_table(pud_base, addr, next);
>> + free_pud_table(pud_base, p4d);
>> }
>>
>> spin_unlock(&init_mm.page_table_lock);
>> --
>> 2.26.2
^ permalink raw reply
* Re: [PATCH v3 0/6] powerpc: queued spinlocks and rwlocks
From: Waiman Long @ 2020-07-08 3:33 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel, kvm-ppc,
virtualization, Ingo Molnar, Will Deacon
In-Reply-To: <1594101082.hfq9x5yact.astroid@bobo.none>
[-- Attachment #1: Type: text/plain, Size: 1488 bytes --]
On 7/7/20 1:57 AM, Nicholas Piggin wrote:
> Yes, powerpc could certainly get more performance out of the slow
> paths, and then there are a few parameters to tune.
>
> We don't have a good alternate patching for function calls yet, but
> that would be something to do for native vs pv.
>
> And then there seem to be one or two tunable parameters we could
> experiment with.
>
> The paravirt locks may need a bit more tuning. Some simple testing
> under KVM shows we might be a bit slower in some cases. Whether this
> is fairness or something else I'm not sure. The current simple pv
> spinlock code can do a directed yield to the lock holder CPU, whereas
> the pv qspl here just does a general yield. I think we might actually
> be able to change that to also support directed yield. Though I'm
> not sure if this is actually the cause of the slowdown yet.
Regarding the paravirt lock, I have taken a further look into the
current PPC spinlock code. There is an equivalent of pv_wait() but no
pv_kick(). Maybe PPC doesn't really need that. Attached are two
additional qspinlock patches that adds a CONFIG_PARAVIRT_QSPINLOCKS_LITE
option to not require pv_kick(). There is also a fixup patch to be
applied after your patchset.
I don't have access to a PPC LPAR with shared processor at the moment,
so I can't test the performance of the paravirt code. Would you mind
adding my patches and do some performance test on your end to see if it
gives better result?
Thanks,
Longman
[-- Attachment #2: 0001-locking-pvqspinlock-Code-relocation-and-extraction.patch --]
[-- Type: text/x-patch, Size: 11938 bytes --]
From 161e545523a7eb4c42c145c04e9a5a15903ba3d9 Mon Sep 17 00:00:00 2001
From: Waiman Long <longman@redhat.com>
Date: Tue, 7 Jul 2020 20:46:51 -0400
Subject: [PATCH 1/9] locking/pvqspinlock: Code relocation and extraction
Move pv_kick_node() and the unlock functions up and extract out the hash
and lock code from pv_wait_head_or_lock() into pv_hash_lock(). There
is no functional change.
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/locking/qspinlock_paravirt.h | 302 ++++++++++++++--------------
1 file changed, 156 insertions(+), 146 deletions(-)
diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h
index e84d21aa0722..8eec58320b85 100644
--- a/kernel/locking/qspinlock_paravirt.h
+++ b/kernel/locking/qspinlock_paravirt.h
@@ -55,6 +55,7 @@ struct pv_node {
/*
* Hybrid PV queued/unfair lock
+ * ----------------------------
*
* By replacing the regular queued_spin_trylock() with the function below,
* it will be called once when a lock waiter enter the PV slowpath before
@@ -259,6 +260,156 @@ static struct pv_node *pv_unhash(struct qspinlock *lock)
BUG();
}
+/*
+ * Insert lock into hash and set _Q_SLOW_VAL.
+ * Return true if lock acquired.
+ */
+static inline bool pv_hash_lock(struct qspinlock *lock, struct pv_node *node)
+{
+ struct qspinlock **lp = pv_hash(lock, node);
+
+ /*
+ * We must hash before setting _Q_SLOW_VAL, such that
+ * when we observe _Q_SLOW_VAL in __pv_queued_spin_unlock()
+ * we'll be sure to be able to observe our hash entry.
+ *
+ * [S] <hash> [Rmw] l->locked == _Q_SLOW_VAL
+ * MB RMB
+ * [RmW] l->locked = _Q_SLOW_VAL [L] <unhash>
+ *
+ * Matches the smp_rmb() in __pv_queued_spin_unlock().
+ */
+ if (xchg(&lock->locked, _Q_SLOW_VAL) == 0) {
+ /*
+ * The lock was free and now we own the lock.
+ * Change the lock value back to _Q_LOCKED_VAL
+ * and unhash the table.
+ */
+ WRITE_ONCE(lock->locked, _Q_LOCKED_VAL);
+ WRITE_ONCE(*lp, NULL);
+ return true;
+ }
+ return false;
+}
+
+/*
+ * Called after setting next->locked = 1 when we're the lock owner.
+ *
+ * Instead of waking the waiters stuck in pv_wait_node() advance their state
+ * such that they're waiting in pv_wait_head_or_lock(), this avoids a
+ * wake/sleep cycle.
+ */
+static void pv_kick_node(struct qspinlock *lock, struct mcs_spinlock *node)
+{
+ struct pv_node *pn = (struct pv_node *)node;
+
+ /*
+ * If the vCPU is indeed halted, advance its state to match that of
+ * pv_wait_node(). If OTOH this fails, the vCPU was running and will
+ * observe its next->locked value and advance itself.
+ *
+ * Matches with smp_store_mb() and cmpxchg() in pv_wait_node()
+ *
+ * The write to next->locked in arch_mcs_spin_unlock_contended()
+ * must be ordered before the read of pn->state in the cmpxchg()
+ * below for the code to work correctly. To guarantee full ordering
+ * irrespective of the success or failure of the cmpxchg(),
+ * a relaxed version with explicit barrier is used. The control
+ * dependency will order the reading of pn->state before any
+ * subsequent writes.
+ */
+ smp_mb__before_atomic();
+ if (cmpxchg_relaxed(&pn->state, vcpu_halted, vcpu_hashed)
+ != vcpu_halted)
+ return;
+
+ /*
+ * Put the lock into the hash table and set the _Q_SLOW_VAL.
+ *
+ * As this is the same vCPU that will check the _Q_SLOW_VAL value and
+ * the hash table later on at unlock time, no atomic instruction is
+ * needed.
+ */
+ WRITE_ONCE(lock->locked, _Q_SLOW_VAL);
+ (void)pv_hash(lock, pn);
+}
+
+/*
+ * PV versions of the unlock fastpath and slowpath functions to be used
+ * instead of queued_spin_unlock().
+ */
+__visible void
+__pv_queued_spin_unlock_slowpath(struct qspinlock *lock, u8 locked)
+{
+ struct pv_node *node;
+
+ if (unlikely(locked != _Q_SLOW_VAL)) {
+ WARN(!debug_locks_silent,
+ "pvqspinlock: lock 0x%lx has corrupted value 0x%x!\n",
+ (unsigned long)lock, atomic_read(&lock->val));
+ return;
+ }
+
+ /*
+ * A failed cmpxchg doesn't provide any memory-ordering guarantees,
+ * so we need a barrier to order the read of the node data in
+ * pv_unhash *after* we've read the lock being _Q_SLOW_VAL.
+ *
+ * Matches the cmpxchg() in pv_wait_head_or_lock() setting _Q_SLOW_VAL.
+ */
+ smp_rmb();
+
+ /*
+ * Since the above failed to release, this must be the SLOW path.
+ * Therefore start by looking up the blocked node and unhashing it.
+ */
+ node = pv_unhash(lock);
+
+ /*
+ * Now that we have a reference to the (likely) blocked pv_node,
+ * release the lock.
+ */
+ smp_store_release(&lock->locked, 0);
+
+ /*
+ * At this point the memory pointed at by lock can be freed/reused,
+ * however we can still use the pv_node to kick the CPU.
+ * The other vCPU may not really be halted, but kicking an active
+ * vCPU is harmless other than the additional latency in completing
+ * the unlock.
+ */
+ lockevent_inc(pv_kick_unlock);
+ pv_kick(node->cpu);
+}
+
+/*
+ * Include the architecture specific callee-save thunk of the
+ * __pv_queued_spin_unlock(). This thunk is put together with
+ * __pv_queued_spin_unlock() to make the callee-save thunk and the real unlock
+ * function close to each other sharing consecutive instruction cachelines.
+ * Alternatively, architecture specific version of __pv_queued_spin_unlock()
+ * can be defined.
+ */
+#include <asm/qspinlock_paravirt.h>
+
+#ifndef __pv_queued_spin_unlock
+__visible void __pv_queued_spin_unlock(struct qspinlock *lock)
+{
+ u8 locked;
+
+ /*
+ * We must not unlock if SLOW, because in that case we must first
+ * unhash. Otherwise it would be possible to have multiple @lock
+ * entries, which would be BAD.
+ */
+ locked = cmpxchg_release(&lock->locked, _Q_LOCKED_VAL, 0);
+ if (likely(locked == _Q_LOCKED_VAL))
+ return;
+
+ __pv_queued_spin_unlock_slowpath(lock, locked);
+}
+#endif /* __pv_queued_spin_unlock */
+
/*
* Return true if when it is time to check the previous node which is not
* in a running state.
@@ -350,48 +501,6 @@ static void pv_wait_node(struct mcs_spinlock *node, struct mcs_spinlock *prev)
*/
}
-/*
- * Called after setting next->locked = 1 when we're the lock owner.
- *
- * Instead of waking the waiters stuck in pv_wait_node() advance their state
- * such that they're waiting in pv_wait_head_or_lock(), this avoids a
- * wake/sleep cycle.
- */
-static void pv_kick_node(struct qspinlock *lock, struct mcs_spinlock *node)
-{
- struct pv_node *pn = (struct pv_node *)node;
-
- /*
- * If the vCPU is indeed halted, advance its state to match that of
- * pv_wait_node(). If OTOH this fails, the vCPU was running and will
- * observe its next->locked value and advance itself.
- *
- * Matches with smp_store_mb() and cmpxchg() in pv_wait_node()
- *
- * The write to next->locked in arch_mcs_spin_unlock_contended()
- * must be ordered before the read of pn->state in the cmpxchg()
- * below for the code to work correctly. To guarantee full ordering
- * irrespective of the success or failure of the cmpxchg(),
- * a relaxed version with explicit barrier is used. The control
- * dependency will order the reading of pn->state before any
- * subsequent writes.
- */
- smp_mb__before_atomic();
- if (cmpxchg_relaxed(&pn->state, vcpu_halted, vcpu_hashed)
- != vcpu_halted)
- return;
-
- /*
- * Put the lock into the hash table and set the _Q_SLOW_VAL.
- *
- * As this is the same vCPU that will check the _Q_SLOW_VAL value and
- * the hash table later on at unlock time, no atomic instruction is
- * needed.
- */
- WRITE_ONCE(lock->locked, _Q_SLOW_VAL);
- (void)pv_hash(lock, pn);
-}
-
/*
* Wait for l->locked to become clear and acquire the lock;
* halt the vcpu after a short spin.
@@ -403,16 +512,13 @@ static u32
pv_wait_head_or_lock(struct qspinlock *lock, struct mcs_spinlock *node)
{
struct pv_node *pn = (struct pv_node *)node;
- struct qspinlock **lp = NULL;
int waitcnt = 0;
int loop;
-
/*
- * If pv_kick_node() already advanced our state, we don't need to
+ * If pv_kick_node() had already advanced our state, we don't need to
* insert ourselves into the hash table anymore.
*/
- if (READ_ONCE(pn->state) == vcpu_hashed)
- lp = (struct qspinlock **)1;
+ bool hashed = (READ_ONCE(pn->state) == vcpu_hashed);
/*
* Tracking # of slowpath locking operations
@@ -439,30 +545,10 @@ pv_wait_head_or_lock(struct qspinlock *lock, struct mcs_spinlock *node)
clear_pending(lock);
- if (!lp) { /* ONCE */
- lp = pv_hash(lock, pn);
-
- /*
- * We must hash before setting _Q_SLOW_VAL, such that
- * when we observe _Q_SLOW_VAL in __pv_queued_spin_unlock()
- * we'll be sure to be able to observe our hash entry.
- *
- * [S] <hash> [Rmw] l->locked == _Q_SLOW_VAL
- * MB RMB
- * [RmW] l->locked = _Q_SLOW_VAL [L] <unhash>
- *
- * Matches the smp_rmb() in __pv_queued_spin_unlock().
- */
- if (xchg(&lock->locked, _Q_SLOW_VAL) == 0) {
- /*
- * The lock was free and now we own the lock.
- * Change the lock value back to _Q_LOCKED_VAL
- * and unhash the table.
- */
- WRITE_ONCE(lock->locked, _Q_LOCKED_VAL);
- WRITE_ONCE(*lp, NULL);
+ if (!hashed) { /* ONCE */
+ if (pv_hash_lock(lock, pn))
goto gotlock;
- }
+ hashed = true;
}
WRITE_ONCE(pn->state, vcpu_hashed);
lockevent_inc(pv_wait_head);
@@ -484,79 +570,3 @@ pv_wait_head_or_lock(struct qspinlock *lock, struct mcs_spinlock *node)
gotlock:
return (u32)(atomic_read(&lock->val) | _Q_LOCKED_VAL);
}
-
-/*
- * PV versions of the unlock fastpath and slowpath functions to be used
- * instead of queued_spin_unlock().
- */
-__visible void
-__pv_queued_spin_unlock_slowpath(struct qspinlock *lock, u8 locked)
-{
- struct pv_node *node;
-
- if (unlikely(locked != _Q_SLOW_VAL)) {
- WARN(!debug_locks_silent,
- "pvqspinlock: lock 0x%lx has corrupted value 0x%x!\n",
- (unsigned long)lock, atomic_read(&lock->val));
- return;
- }
-
- /*
- * A failed cmpxchg doesn't provide any memory-ordering guarantees,
- * so we need a barrier to order the read of the node data in
- * pv_unhash *after* we've read the lock being _Q_SLOW_VAL.
- *
- * Matches the cmpxchg() in pv_wait_head_or_lock() setting _Q_SLOW_VAL.
- */
- smp_rmb();
-
- /*
- * Since the above failed to release, this must be the SLOW path.
- * Therefore start by looking up the blocked node and unhashing it.
- */
- node = pv_unhash(lock);
-
- /*
- * Now that we have a reference to the (likely) blocked pv_node,
- * release the lock.
- */
- smp_store_release(&lock->locked, 0);
-
- /*
- * At this point the memory pointed at by lock can be freed/reused,
- * however we can still use the pv_node to kick the CPU.
- * The other vCPU may not really be halted, but kicking an active
- * vCPU is harmless other than the additional latency in completing
- * the unlock.
- */
- lockevent_inc(pv_kick_unlock);
- pv_kick(node->cpu);
-}
-
-/*
- * Include the architecture specific callee-save thunk of the
- * __pv_queued_spin_unlock(). This thunk is put together with
- * __pv_queued_spin_unlock() to make the callee-save thunk and the real unlock
- * function close to each other sharing consecutive instruction cachelines.
- * Alternatively, architecture specific version of __pv_queued_spin_unlock()
- * can be defined.
- */
-#include <asm/qspinlock_paravirt.h>
-
-#ifndef __pv_queued_spin_unlock
-__visible void __pv_queued_spin_unlock(struct qspinlock *lock)
-{
- u8 locked;
-
- /*
- * We must not unlock if SLOW, because in that case we must first
- * unhash. Otherwise it would be possible to have multiple @lock
- * entries, which would be BAD.
- */
- locked = cmpxchg_release(&lock->locked, _Q_LOCKED_VAL, 0);
- if (likely(locked == _Q_LOCKED_VAL))
- return;
-
- __pv_queued_spin_unlock_slowpath(lock, locked);
-}
-#endif /* __pv_queued_spin_unlock */
--
2.18.1
[-- Attachment #3: 0002-locking-pvqspinlock-Introduce-CONFIG_PARAVIRT_QSPINL.patch --]
[-- Type: text/x-patch, Size: 6166 bytes --]
From 5d7941a498935fb225b2c7a3108cbf590114c3db Mon Sep 17 00:00:00 2001
From: Waiman Long <longman@redhat.com>
Date: Tue, 7 Jul 2020 22:29:16 -0400
Subject: [PATCH 2/9] locking/pvqspinlock: Introduce
CONFIG_PARAVIRT_QSPINLOCKS_LITE
Add a new PARAVIRT_QSPINLOCKS_LITE config option that allows
architectures to use the PV qspinlock code without the need to use or
implement a pv_kick() function, thus eliminating the atomic unlock
overhead. The non-atomic queued_spin_unlock() can be used instead.
The pv_wait() function will still be needed, but it can be a dummy
function.
With that option set, the hybrid PV queued/unfair locking code should
still be able to make it performant enough in a paravirtualized
environment.
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/Kconfig.locks | 4 +++
kernel/locking/lock_events_list.h | 3 ++
kernel/locking/qspinlock_paravirt.h | 49 ++++++++++++++++++++++++-----
kernel/locking/qspinlock_stat.h | 5 +--
4 files changed, 52 insertions(+), 9 deletions(-)
diff --git a/kernel/Kconfig.locks b/kernel/Kconfig.locks
index 3de8fd11873b..1824ba8c44a9 100644
--- a/kernel/Kconfig.locks
+++ b/kernel/Kconfig.locks
@@ -243,6 +243,10 @@ config QUEUED_SPINLOCKS
def_bool y if ARCH_USE_QUEUED_SPINLOCKS
depends on SMP
+config PARAVIRT_QSPINLOCKS_LITE
+ bool
+ depends on QUEUED_SPINLOCKS && PARAVIRT_SPINLOCKS
+
config BPF_ARCH_SPINLOCK
bool
diff --git a/kernel/locking/lock_events_list.h b/kernel/locking/lock_events_list.h
index 239039d0ce21..9ae07a7148e8 100644
--- a/kernel/locking/lock_events_list.h
+++ b/kernel/locking/lock_events_list.h
@@ -22,11 +22,14 @@
/*
* Locking events for PV qspinlock.
*/
+#ifndef CONFIG_PARAVIRT_QSPINLOCKS_LITE
LOCK_EVENT(pv_hash_hops) /* Average # of hops per hashing operation */
LOCK_EVENT(pv_kick_unlock) /* # of vCPU kicks issued at unlock time */
LOCK_EVENT(pv_kick_wake) /* # of vCPU kicks for pv_latency_wake */
LOCK_EVENT(pv_latency_kick) /* Average latency (ns) of vCPU kick */
LOCK_EVENT(pv_latency_wake) /* Average latency (ns) of kick-to-wakeup */
+#endif
+
LOCK_EVENT(pv_lock_stealing) /* # of lock stealing operations */
LOCK_EVENT(pv_spurious_wakeup) /* # of spurious wakeups in non-head vCPUs */
LOCK_EVENT(pv_wait_again) /* # of wait's after queue head vCPU kick */
diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h
index 8eec58320b85..2d24563aa9b9 100644
--- a/kernel/locking/qspinlock_paravirt.h
+++ b/kernel/locking/qspinlock_paravirt.h
@@ -77,6 +77,23 @@ struct pv_node {
* This hybrid PV queued/unfair lock combines the best attributes of a
* queued lock (no lock starvation) and an unfair lock (good performance
* on not heavily contended locks).
+ *
+ * PV lock lite
+ * ------------
+ *
+ * By default, the PV lock uses two hypervisor specific functions pv_wait()
+ * and pv_kick() to release the vcpu back to the hypervisor and request the
+ * hypervisor to put the given vcpu online again respectively.
+ *
+ * The pv_kick() function is called at unlock time and requires the use of
+ * an atomic instruction to prevent missed wakeup. The unlock overhead of
+ * the PV lock is a major reason why the PV lock is slightly slower than
+ * the native lock. Not all the hypervisors need to really use both
+ * pv_wait() and pv_kick(). The PARAVIRT_QSPINLOCKS_LITE config option
+ * enables a lighter version of PV lock that relies mainly on the hybrid
+ * queued/unfair lock. The pv_wait() function will be used if provided.
+ * The pv_kick() function isn't used to eliminate the unlock overhead and
+ * the non-atomic queued_spin_unlock() can be used.
*/
#define queued_spin_trylock(l) pv_hybrid_queued_unfair_trylock(l)
static inline bool pv_hybrid_queued_unfair_trylock(struct qspinlock *lock)
@@ -153,6 +170,7 @@ static __always_inline int trylock_clear_pending(struct qspinlock *lock)
}
#endif /* _Q_PENDING_BITS == 8 */
+#ifndef CONFIG_PARAVIRT_QSPINLOCKS_LITE
/*
* Lock and MCS node addresses hash table for fast lookup
*
@@ -410,6 +428,29 @@ __visible void __pv_queued_spin_unlock(struct qspinlock *lock)
}
#endif /* __pv_queued_spin_unlock */
+static inline void set_pv_node_running(struct pv_node *pn)
+{
+ /*
+ * If pv_kick_node() changed us to vcpu_hashed, retain that value so
+ * that pv_wait_head_or_lock() will not try to hash this lock.
+ */
+ cmpxchg(&pn->state, vcpu_halted, vcpu_running);
+}
+#else
+static inline bool pv_hash_lock(struct qspinlock *lock, struct pv_node *node)
+{
+ return false;
+}
+
+static inline void pv_kick_node(struct qspinlock *lock,
+ struct mcs_spinlock *node) { }
+
+static inline void set_pv_node_running(struct pv_node *pn)
+{
+ pn->state = vcpu_running;
+}
+#endif /* CONFIG_PARAVIRT_QSPINLOCKS_LITE */
+
/*
* Return true if when it is time to check the previous node which is not
* in a running state.
@@ -475,13 +516,7 @@ static void pv_wait_node(struct mcs_spinlock *node, struct mcs_spinlock *prev)
lockevent_cond_inc(pv_wait_early, wait_early);
pv_wait(&pn->state, vcpu_halted);
}
-
- /*
- * If pv_kick_node() changed us to vcpu_hashed, retain that
- * value so that pv_wait_head_or_lock() knows to not also try
- * to hash this lock.
- */
- cmpxchg(&pn->state, vcpu_halted, vcpu_running);
+ set_pv_node_running(pn);
/*
* If the locked flag is still not set after wakeup, it is a
diff --git a/kernel/locking/qspinlock_stat.h b/kernel/locking/qspinlock_stat.h
index e625bb410aa2..e9f63240785b 100644
--- a/kernel/locking/qspinlock_stat.h
+++ b/kernel/locking/qspinlock_stat.h
@@ -7,7 +7,8 @@
#include "lock_events.h"
#ifdef CONFIG_LOCK_EVENT_COUNTS
-#ifdef CONFIG_PARAVIRT_SPINLOCKS
+#if defined(CONFIG_PARAVIRT_SPINLOCKS) && \
+ !defined(CONFIG_PARAVIRT_QSPINLOCKS_LITE)
/*
* Collect pvqspinlock locking event counts
*/
@@ -133,7 +134,7 @@ static inline void __pv_wait(u8 *ptr, u8 val)
#define pv_kick(c) __pv_kick(c)
#define pv_wait(p, v) __pv_wait(p, v)
-#endif /* CONFIG_PARAVIRT_SPINLOCKS */
+#endif /* CONFIG_PARAVIRT_SPINLOCKS && !CONFIG_PARAVIRT_QSPINLOCKS_LITE */
#else /* CONFIG_LOCK_EVENT_COUNTS */
--
2.18.1
[-- Attachment #4: 0009-powerpc-pseries-Fixup.patch --]
[-- Type: text/x-patch, Size: 3086 bytes --]
From 655d3a5d48a494a5674cf57454bf3e1f36b6eb83 Mon Sep 17 00:00:00 2001
From: Waiman Long <longman@redhat.com>
Date: Tue, 7 Jul 2020 22:29:20 -0400
Subject: [PATCH 9/9] powerpc/pseries: Fixup
Signed-off-by: Waiman Long <longman@redhat.com>
---
arch/powerpc/include/asm/qspinlock.h | 22 ----------------------
arch/powerpc/platforms/pseries/Kconfig | 1 +
arch/powerpc/platforms/pseries/setup.c | 6 +-----
3 files changed, 2 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/include/asm/qspinlock.h b/arch/powerpc/include/asm/qspinlock.h
index b752d34517b3..1fa724d27a2d 100644
--- a/arch/powerpc/include/asm/qspinlock.h
+++ b/arch/powerpc/include/asm/qspinlock.h
@@ -10,7 +10,6 @@
#ifdef CONFIG_PARAVIRT_SPINLOCKS
extern void native_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
-extern void __pv_queued_spin_unlock(struct qspinlock *lock);
static __always_inline void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
{
@@ -20,15 +19,6 @@ static __always_inline void queued_spin_lock_slowpath(struct qspinlock *lock, u3
__pv_queued_spin_lock_slowpath(lock, val);
}
-#define queued_spin_unlock queued_spin_unlock
-static inline void queued_spin_unlock(struct qspinlock *lock)
-{
- if (!is_shared_processor())
- smp_store_release(&lock->locked, 0);
- else
- __pv_queued_spin_unlock(lock);
-}
-
#else
extern void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
#endif
@@ -72,18 +62,6 @@ static __always_inline void pv_wait(u8 *ptr, u8 val)
*/
}
-static __always_inline void pv_kick(int cpu)
-{
- prod_cpu(cpu);
-}
-
-extern void __pv_init_lock_hash(void);
-
-static inline void pv_spinlocks_init(void)
-{
- __pv_init_lock_hash();
-}
-
#endif
#include <asm-generic/qspinlock.h>
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index 756e727b383f..1e3bbe27d664 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -33,6 +33,7 @@ config PPC_SPLPAR
depends on PPC_PSERIES
bool "Support for shared-processor logical partitions"
select PARAVIRT_SPINLOCKS if PPC_QUEUED_SPINLOCKS
+ select PARAVIRT_QSPINLOCKS_LITE if PPC_QUEUED_SPINLOCKS
help
Enabling this option will make the kernel run more efficiently
on logically-partitioned pSeries systems which use shared
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 747a203d9453..2db8469e475f 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -771,12 +771,8 @@ static void __init pSeries_setup_arch(void)
if (firmware_has_feature(FW_FEATURE_LPAR)) {
vpa_init(boot_cpuid);
- if (lppaca_shared_proc(get_lppaca())) {
+ if (lppaca_shared_proc(get_lppaca()))
static_branch_enable(&shared_processor);
-#ifdef CONFIG_PARAVIRT_SPINLOCKS
- pv_spinlocks_init();
-#endif
- }
ppc_md.power_save = pseries_lpar_idle;
ppc_md.enable_pmcs = pseries_lpar_enable_pmcs;
--
2.18.1
^ permalink raw reply related
* [Bug 208197] OF: /pci@f2000000/mac-io@17/gpio@50/...: could not find phandle
From: bugzilla-daemon @ 2020-07-08 4:05 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-208197-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=208197
--- Comment #7 from Michael Ellerman (michael@ellerman.id.au) ---
I couldn't really make sense of your bisect log, it doesn't have any good/bad
commits in it.
And I don't see how reverting a merge of v5.7-rc7 can be helping, because you
said v5.7 is good, and that contains v5.7-rc7.
Can you attach the output of "git bisect log".
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v5 0/4] vmalloc kernel mapping and relocatable kernel
From: Alex Ghiti @ 2020-07-08 4:21 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel, Atish Patra,
Zong Li, linux-kernel, linuxppc-dev, linux-riscv
In-Reply-To: <20200607075949.665-1-alex@ghiti.fr>
Hi Palmer,
Le 6/7/20 à 3:59 AM, Alexandre Ghiti a écrit :
> This patchset originally implemented relocatable kernel support but now
> also moves the kernel mapping into the vmalloc zone.
>
> The first patch explains why we need to move the kernel into vmalloc
> zone (instead of memcpying it around). That patch should ease KASLR
> implementation a lot.
>
> The second patch allows to build relocatable kernels but is not selected
> by default.
>
> The third and fourth patches take advantage of an already existing powerpc
> script that checks relocations at compile-time, and uses it for riscv.
>
> Changes in v5:
> * Add "static __init" to create_kernel_page_table function as reported by
> Kbuild test robot
> * Add reviewed-by from Zong
> * Rebase onto v5.7
>
> Changes in v4:
> * Fix BPF region that overlapped with kernel's as suggested by Zong
> * Fix end of module region that could be larger than 2GB as suggested by Zong
> * Fix the size of the vm area reserved for the kernel as we could lose
> PMD_SIZE if the size was already aligned on PMD_SIZE
> * Split compile time relocations check patch into 2 patches as suggested by Anup
> * Applied Reviewed-by from Zong and Anup
>
> Changes in v3:
> * Move kernel mapping to vmalloc
>
> Changes in v2:
> * Make RELOCATABLE depend on MMU as suggested by Anup
> * Rename kernel_load_addr into kernel_virt_addr as suggested by Anup
> * Use __pa_symbol instead of __pa, as suggested by Zong
> * Rebased on top of v5.6-rc3
> * Tested with sv48 patchset
> * Add Reviewed/Tested-by from Zong and Anup
>
> Alexandre Ghiti (4):
> riscv: Move kernel mapping to vmalloc zone
> riscv: Introduce CONFIG_RELOCATABLE
> powerpc: Move script to check relocations at compile time in scripts/
> riscv: Check relocations at compile time
>
> arch/powerpc/tools/relocs_check.sh | 18 +----
> arch/riscv/Kconfig | 12 +++
> arch/riscv/Makefile | 5 +-
> arch/riscv/Makefile.postlink | 36 +++++++++
> arch/riscv/boot/loader.lds.S | 3 +-
> arch/riscv/include/asm/page.h | 10 ++-
> arch/riscv/include/asm/pgtable.h | 38 ++++++---
> arch/riscv/kernel/head.S | 3 +-
> arch/riscv/kernel/module.c | 4 +-
> arch/riscv/kernel/vmlinux.lds.S | 9 ++-
> arch/riscv/mm/Makefile | 4 +
> arch/riscv/mm/init.c | 121 +++++++++++++++++++++++++----
> arch/riscv/mm/physaddr.c | 2 +-
> arch/riscv/tools/relocs_check.sh | 26 +++++++
> scripts/relocs_check.sh | 20 +++++
> 15 files changed, 259 insertions(+), 52 deletions(-)
> create mode 100644 arch/riscv/Makefile.postlink
> create mode 100755 arch/riscv/tools/relocs_check.sh
> create mode 100755 scripts/relocs_check.sh
>
Do you have any remark regarding this series ?
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH v5 01/12] vmlinux.lds.h: add linker section for KUnit test suites
From: Luis Chamberlain @ 2020-07-08 4:31 UTC (permalink / raw)
To: Brendan Higgins
Cc: open list:DOCUMENTATION, catalin.marinas, jcmvbkbc, will, paulus,
open list:KERNEL SELFTEST FRAMEWORK, Frank Rowand, Anton Ivanov,
linux-arch, Richard Weinberger, rppt, Iurii Zaikin, linux-xtensa,
Kees Cook, Arnd Bergmann, Jeff Dike, linux-um, linuxppc-dev,
David Gow, Shuah Khan, Linux ARM, KUnit Development, chris,
monstr, Stephen Boyd, Greg KH, Linux Kernel Mailing List,
Logan Gunthorpe, Andrew Morton, Alan Maguire
In-Reply-To: <CAFd5g47vu5vmrXnS0sLu+hdC2HmYz7GY82sE8rhcHfNkuC1NRw@mail.gmail.com>
On Fri, Jun 26, 2020 at 02:22:11PM -0700, Brendan Higgins wrote:
> On Fri, Jun 26, 2020 at 2:20 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Fri, Jun 26, 2020 at 02:09:06PM -0700, Brendan Higgins wrote:
> > > Add a linker section where KUnit can put references to its test suites.
> > > This patch is the first step in transitioning to dispatching all KUnit
> > > tests from a centralized executor rather than having each as its own
> > > separate late_initcall.
> > >
> > > Co-developed-by: Iurii Zaikin <yzaikin@google.com>
> > > Signed-off-by: Iurii Zaikin <yzaikin@google.com>
> > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > > Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> > > ---
> > > include/asm-generic/vmlinux.lds.h | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > > index db600ef218d7d..4f9b036fc9616 100644
> > > --- a/include/asm-generic/vmlinux.lds.h
> > > +++ b/include/asm-generic/vmlinux.lds.h
> > > @@ -881,6 +881,13 @@
> > > KEEP(*(.con_initcall.init)) \
> > > __con_initcall_end = .;
> > >
> > > +/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
> >
> > Nit on naming:
> >
> > > +#define KUNIT_TEST_SUITES \
> >
> > I would call this KUNIT_TABLE to maintain the same names as other things
> > of this nature.
> >
> > > + . = ALIGN(8); \
> > > + __kunit_suites_start = .; \
> > > + KEEP(*(.kunit_test_suites)) \
> > > + __kunit_suites_end = .;
> > > +
> > > #ifdef CONFIG_BLK_DEV_INITRD
> > > #define INIT_RAM_FS \
> > > . = ALIGN(4); \
> > > @@ -1056,6 +1063,7 @@
> > > INIT_CALLS \
> > > CON_INITCALL \
> > > INIT_RAM_FS \
> > > + KUNIT_TEST_SUITES \
> > > }
> >
> > Nack: this must be in INIT_DATA, not in INIT_DATA_SECTION. Not all
> > architectures use the INIT_DATA_SECTION macro (e.g. arm64), but everything
> > uses INIT_DATA.
>
> Oh, maybe that would eliminate the need for the other linkerscript
> patches? That would be nice.
Curious, did changing it as Kees suggest fix it for m68k?
Luis
^ permalink raw reply
* Re: [PATCH v5 08/12] init: main: add KUnit to kernel init
From: Luis Chamberlain @ 2020-07-08 4:38 UTC (permalink / raw)
To: Brendan Higgins
Cc: linux-doc, catalin.marinas, jcmvbkbc, will, paulus,
linux-kselftest, frowand.list, anton.ivanov, linux-arch, richard,
rppt, yzaikin, linux-xtensa, keescook, arnd, jdike, linux-um,
linuxppc-dev, davidgow, skhan, linux-arm-kernel, kunit-dev, chris,
monstr, sboyd, gregkh, linux-kernel, logang, akpm, alan.maguire
In-Reply-To: <20200626210917.358969-9-brendanhiggins@google.com>
On Fri, Jun 26, 2020 at 02:09:13PM -0700, Brendan Higgins wrote:
> Remove KUnit from init calls entirely, instead call directly from
> kernel_init().
The commit log does not explain *why*.
> Co-developed-by: Alan Maguire <alan.maguire@oracle.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Other than that:
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Luis
^ permalink raw reply
* Re: [PATCH v2 4/4] powerpc/mm/radix: Create separate mappings for hot-plugged memory
From: Michael Ellerman @ 2020-07-08 4:44 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V, Bharata B Rao
In-Reply-To: <20200625064547.228448-5-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> To enable memory unplug without splitting kernel page table
> mapping, we force the max mapping size to the LMB size. LMB
> size is the unit in which hypervisor will do memory add/remove
> operation.
>
> This implies on pseries system, we now end up mapping
Please expand on why it "implies" that for pseries.
> memory with 2M page size instead of 1G. To improve
> that we want hypervisor to hint the kernel about the hotplug
> memory range. This was added that as part of
That
>
> commit b6eca183e23e ("powerpc/kernel: Enables memory
> hot-remove after reboot on pseries guests")
>
> But we still don't do that on PowerVM. Once we get PowerVM
I think you mean PowerVM doesn't provide that hint yet?
Realistically it won't until P10. So this means we'll always use 2MB on
Power9 PowerVM doesn't it?
What about KVM?
Have you done any benchmarking on the impact of switching the linear
mapping to 2MB pages?
> updated, we can then force the 2M mapping only to hot-pluggable
> memory region using memblock_is_hotpluggable(). Till then
> let's depend on LMB size for finding the mapping page size
> for linear range.
>
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/mm/book3s64/radix_pgtable.c | 83 ++++++++++++++++++++----
> arch/powerpc/platforms/powernv/setup.c | 10 ++-
> 2 files changed, 81 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
> index 78ad11812e0d..a4179e4da49d 100644
> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
> @@ -15,6 +15,7 @@
> #include <linux/mm.h>
> #include <linux/hugetlb.h>
> #include <linux/string_helpers.h>
> +#include <linux/memory.h>
>
> #include <asm/pgtable.h>
> #include <asm/pgalloc.h>
> @@ -34,6 +35,7 @@
>
> unsigned int mmu_pid_bits;
> unsigned int mmu_base_pid;
> +unsigned int radix_mem_block_size;
static? __ro_after_init?
> @@ -266,6 +268,7 @@ static unsigned long next_boundary(unsigned long addr, unsigned long end)
>
> static int __meminit create_physical_mapping(unsigned long start,
> unsigned long end,
> + unsigned long max_mapping_size,
> int nid, pgprot_t _prot)
> {
> unsigned long vaddr, addr, mapping_size = 0;
> @@ -279,6 +282,8 @@ static int __meminit create_physical_mapping(unsigned long start,
> int rc;
>
> gap = next_boundary(addr, end) - addr;
> + if (gap > max_mapping_size)
> + gap = max_mapping_size;
> previous_size = mapping_size;
> prev_exec = exec;
>
> @@ -329,8 +334,9 @@ static void __init radix_init_pgtable(void)
>
> /* We don't support slb for radix */
> mmu_slb_size = 0;
> +
> /*
> - * Create the linear mapping, using standard page size for now
> + * Create the linear mapping
> */
> for_each_memblock(memory, reg) {
> /*
> @@ -346,6 +352,7 @@ static void __init radix_init_pgtable(void)
>
> WARN_ON(create_physical_mapping(reg->base,
> reg->base + reg->size,
> + radix_mem_block_size,
> -1, PAGE_KERNEL));
> }
>
> @@ -486,6 +493,49 @@ static int __init radix_dt_scan_page_sizes(unsigned long node,
> return 1;
> }
>
> +static int __init probe_memory_block_size(unsigned long node, const char *uname, int
> + depth, void *data)
> +{
> + const __be32 *block_size;
> + int len;
> +
> + if (depth != 1)
> + return 0;
> +
> + if (!strcmp(uname, "ibm,dynamic-reconfiguration-memory")) {
if (strcmp(uname, "ibm,dynamic-reconfiguration-memory") != 0)
return 0;
Then you can de-indent the rest of the body.
> + block_size = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
> + if (!block_size || len < dt_root_size_cells * sizeof(__be32))
This will give us a sparse warning, please do the endian conversion
before looking at the value.
> + /*
> + * Nothing in the device tree
> + */
> + return MIN_MEMORY_BLOCK_SIZE;
> +
> + return dt_mem_next_cell(dt_root_size_cells, &block_size);
Just of_read_number() ?
This is misusing the return value, as I explained on one of your other
recent patches. You should return !0 to indicate that scanning should
stop, and the actual value can go via the data pointer, or better just
set the global.
> + }
> +
> + return 0;
> +}
> +
> +static unsigned long radix_memory_block_size(void)
> +{
> + unsigned long mem_block_size = MIN_MEMORY_BLOCK_SIZE;
> +
> + if (firmware_has_feature(FW_FEATURE_OPAL)) {
> +
> + mem_block_size = 1UL * 1024 * 1024 * 1024;
We have 1GB hardcoded here and also in pnv_memory_block_size().
Shouldn't pnv_memory_block_size() at least be using radix_mem_block_size?
> + } else if (firmware_has_feature(FW_FEATURE_LPAR)) {
> + mem_block_size = of_scan_flat_dt(probe_memory_block_size, NULL);
> + if (!mem_block_size)
> + mem_block_size = MIN_MEMORY_BLOCK_SIZE;
> + }
> +
> + return mem_block_size;
> +}
It would probably be simpler if that was just inlined below.
> +
> +
> void __init radix__early_init_devtree(void)
> {
> int rc;
> @@ -494,17 +544,27 @@ void __init radix__early_init_devtree(void)
> * Try to find the available page sizes in the device-tree
> */
> rc = of_scan_flat_dt(radix_dt_scan_page_sizes, NULL);
> - if (rc != 0) /* Found */
> - goto found;
> + if (rc == 0) {
> + /*
> + * no page size details found in device tree
> + * let's assume we have page 4k and 64k support
Capitals and punctuation please?
> + */
> + mmu_psize_defs[MMU_PAGE_4K].shift = 12;
> + mmu_psize_defs[MMU_PAGE_4K].ap = 0x0;
> +
> + mmu_psize_defs[MMU_PAGE_64K].shift = 16;
> + mmu_psize_defs[MMU_PAGE_64K].ap = 0x5;
> + }
Moving that seems like an unrelated change. It's a reasonable change but
I'd rather you did it in a standalone patch.
> /*
> - * let's assume we have page 4k and 64k support
> + * Max mapping size used when mapping pages. We don't use
> + * ppc_md.memory_block_size() here because this get called
> + * early and we don't have machine probe called yet. Also
> + * the pseries implementation only check for ibm,lmb-size.
> + * All hypervisor supporting radix do expose that device
> + * tree node.
> */
> - mmu_psize_defs[MMU_PAGE_4K].shift = 12;
> - mmu_psize_defs[MMU_PAGE_4K].ap = 0x0;
> -
> - mmu_psize_defs[MMU_PAGE_64K].shift = 16;
> - mmu_psize_defs[MMU_PAGE_64K].ap = 0x5;
> -found:
> + radix_mem_block_size = radix_memory_block_size();
If you did that earlier in the function, before
radix_dt_scan_page_sizes(), the logic would be simpler.
> return;
> }
>
> @@ -856,7 +916,8 @@ int __meminit radix__create_section_mapping(unsigned long start,
> return -1;
> }
>
> - return create_physical_mapping(__pa(start), __pa(end), nid, prot);
> + return create_physical_mapping(__pa(start), __pa(end),
> + radix_mem_block_size, nid, prot);
> }
>
> int __meminit radix__remove_section_mapping(unsigned long start, unsigned long end)
> diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
> index 3bc188da82ba..6e2f614590a3 100644
> --- a/arch/powerpc/platforms/powernv/setup.c
> +++ b/arch/powerpc/platforms/powernv/setup.c
> @@ -399,7 +399,15 @@ static void pnv_kexec_cpu_down(int crash_shutdown, int secondary)
> #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> static unsigned long pnv_memory_block_size(void)
> {
> - return 256UL * 1024 * 1024;
> + /*
> + * We map the kernel linear region with 1GB large pages on radix. For
> + * memory hot unplug to work our memory block size must be at least
> + * this size.
> + */
> + if (radix_enabled())
> + return 1UL * 1024 * 1024 * 1024;
> + else
> + return 256UL * 1024 * 1024;
> }
> #endif
>
> --
> 2.26.2
cheers
^ permalink raw reply
* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()
From: Christophe Leroy @ 2020-07-08 4:49 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, npiggin,
segher
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <faa6759a-8188-104b-a9f9-a5ff3b060cfa@csgroup.eu>
Le 07/07/2020 à 21:02, Christophe Leroy a écrit :
>
>
> Le 07/07/2020 à 14:44, Christophe Leroy a écrit :
>>
>>
>> Le 30/06/2020 à 03:19, Michael Ellerman a écrit :
>>> Michael Ellerman <mpe@ellerman.id.au> writes:
>>>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>>>> Hi Michael,
>>>>>
>>>>> I see this patch is marked as "defered" in patchwork, but I can't see
>>>>> any related discussion. Is it normal ?
>>>>
>>>> Because it uses the "m<>" constraint which didn't work on GCC 4.6.
>>>>
>>>> https://github.com/linuxppc/issues/issues/297
>>>>
>>>> So we should be able to pick it up for v5.9 hopefully.
>>>
>>> It seems to break the build with the kernel.org 4.9.4 compiler and
>>> corenet64_smp_defconfig:
>>
>> Most likely a GCC bug ?
>>
>> It seems the problem vanishes with patch
>> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/173de3b659fa3a5f126a0eb170522cccd909950f.1594125164.git.christophe.leroy@csgroup.eu/
>>
>
> Same kind of issue in signal_64.c now.
>
> The following patch fixes it:
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/810bd8840ef990a200f58c9dea9abe767ca02a3a.1594146723.git.christophe.leroy@csgroup.eu/
>
>
This time I confirm, with the two above mentioned patches, it builds OK
with 4.9, see
http://kisskb.ellerman.id.au/kisskb/head/810bd8840ef990a200f58c9dea9abe767ca02a3a/
Christophe
^ permalink raw reply
* [PATCH v3 0/9] powerpc/watchpoint: Enable 2nd DAWR on baremetal and powervm
From: Ravi Bangoria @ 2020-07-08 4:50 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec,
miltonm, oleg, npiggin, linux-kernel, paulus, jolsa, pedromfc,
naveen.n.rao, linuxppc-dev, mingo
Last series[1] was to add basic infrastructure support for more than
one watchpoint on Book3S powerpc. This series actually enables the 2nd
DAWR for baremetal and powervm. Kvm guest is still not supported.
v2: https://lore.kernel.org/linuxppc-dev/20200604033443.70591-1-ravi.bangoria@linux.ibm.com/
v2->v3:
- patch #2 is new. It fixes an issue with DAWR exception constraint
- Rename dawr1 to debug-facilities-v31 in dt cpu feature, suggested
by Nick Piggin.
- Rebased to powerpc/next
[1]: https://lore.kernel.org/linuxppc-dev/20200514111741.97993-1-ravi.bangoria@linux.ibm.com/
Ravi Bangoria (9):
powerpc/watchpoint: Fix 512 byte boundary limit
powerpc/watchpoint: Fix DAWR exception constraint
powerpc/watchpoint: Enable watchpoint functionality on power10 guest
powerpc/dt_cpu_ftrs: Add feature for 2nd DAWR
powerpc/watchpoint: Set CPU_FTR_DAWR1 based on pa-features bit
powerpc/watchpoint: Rename current H_SET_MODE DAWR macro
powerpc/watchpoint: Guest support for 2nd DAWR hcall
powerpc/watchpoint: Return available watchpoints dynamically
powerpc/watchpoint: Remove 512 byte boundary
arch/powerpc/include/asm/cputable.h | 13 ++-
arch/powerpc/include/asm/hvcall.h | 3 +-
arch/powerpc/include/asm/hw_breakpoint.h | 5 +-
arch/powerpc/include/asm/machdep.h | 2 +-
arch/powerpc/include/asm/plpar_wrappers.h | 7 +-
arch/powerpc/kernel/dawr.c | 2 +-
arch/powerpc/kernel/dt_cpu_ftrs.c | 7 ++
arch/powerpc/kernel/hw_breakpoint.c | 98 +++++++++++++++--------
arch/powerpc/kernel/prom.c | 2 +
arch/powerpc/kvm/book3s_hv.c | 2 +-
arch/powerpc/platforms/pseries/setup.c | 7 +-
11 files changed, 103 insertions(+), 45 deletions(-)
--
2.26.2
^ permalink raw reply
* [PATCH v3 1/9] powerpc/watchpoint: Fix 512 byte boundary limit
From: Ravi Bangoria @ 2020-07-08 4:50 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec,
miltonm, oleg, npiggin, linux-kernel, paulus, jolsa, pedromfc,
naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200708045046.135702-1-ravi.bangoria@linux.ibm.com>
Milton Miller reported that we are aligning start and end address to
wrong size SZ_512M. It should be SZ_512. Fix that.
While doing this change I also found a case where ALIGN() comparison
fails. Within a given aligned range, ALIGN() of two addresses does not
match when start address is pointing to the first byte and end address
is pointing to any other byte except the first one. But that's not true
for ALIGN_DOWN(). ALIGN_DOWN() of any two addresses within that range
will always point to the first byte. So use ALIGN_DOWN() instead of
ALIGN().
Fixes: e68ef121c1f4 ("powerpc/watchpoint: Use builtin ALIGN*() macros")
Reported-by: Milton Miller <miltonm@us.ibm.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/kernel/hw_breakpoint.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 0000daf0e1da..031e6defc08e 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -419,7 +419,7 @@ static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw)
if (dawr_enabled()) {
max_len = DAWR_MAX_LEN;
/* DAWR region can't cross 512 bytes boundary */
- if (ALIGN(start_addr, SZ_512M) != ALIGN(end_addr - 1, SZ_512M))
+ if (ALIGN_DOWN(start_addr, SZ_512) != ALIGN_DOWN(end_addr - 1, SZ_512))
return -EINVAL;
} else if (IS_ENABLED(CONFIG_PPC_8xx)) {
/* 8xx can setup a range without limitation */
--
2.26.2
^ permalink raw reply related
* [PATCH v3 2/9] powerpc/watchpoint: Fix DAWR exception constraint
From: Ravi Bangoria @ 2020-07-08 4:50 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec,
miltonm, oleg, npiggin, linux-kernel, paulus, jolsa, pedromfc,
naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200708045046.135702-1-ravi.bangoria@linux.ibm.com>
Pedro Miraglia Franco de Carvalho noticed that on p8, DAR value is
inconsistent with different type of load/store. Like for byte,word
etc. load/stores, DAR is set to the address of the first byte of
overlap between watch range and real access. But for quadword load/
store it's set to the address of the first byte of real access. This
issue has been fixed in p10. In p10(ISA 3.1), DAR is always set to
the address of the first byte of overlap. Commit 27985b2a640e
("powerpc/watchpoint: Don't ignore extraneous exceptions blindly")
wrongly assumes that DAR is set to the address of the first byte of
overlap for all load/stores on p8 as well. Fix that. With the fix,
we now rely on 'ea' provided by analyse_instr(). If analyse_instr()
fails, generate event unconditionally on p8, and on p10 generate
event only if DAR is within a DAWR range.
Note: 8xx is not affected.
Fixes: 27985b2a640e ("powerpc/watchpoint: Don't ignore extraneous exceptions blindly")
Fixes: 74c6881019b7 ("powerpc/watchpoint: Prepare handler to handle more than one watchpoint")
Reported-by: Pedro Miraglia Franco de Carvalho <pedromfc@br.ibm.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/kernel/hw_breakpoint.c | 93 +++++++++++++++++++----------
1 file changed, 63 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 031e6defc08e..7a66c370a105 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -498,11 +498,11 @@ static bool dar_in_user_range(unsigned long dar, struct arch_hw_breakpoint *info
return ((info->address <= dar) && (dar - info->address < info->len));
}
-static bool dar_user_range_overlaps(unsigned long dar, int size,
- struct arch_hw_breakpoint *info)
+static bool ea_user_range_overlaps(unsigned long ea, int size,
+ struct arch_hw_breakpoint *info)
{
- return ((dar < info->address + info->len) &&
- (dar + size > info->address));
+ return ((ea < info->address + info->len) &&
+ (ea + size > info->address));
}
static bool dar_in_hw_range(unsigned long dar, struct arch_hw_breakpoint *info)
@@ -515,20 +515,22 @@ static bool dar_in_hw_range(unsigned long dar, struct arch_hw_breakpoint *info)
return ((hw_start_addr <= dar) && (hw_end_addr > dar));
}
-static bool dar_hw_range_overlaps(unsigned long dar, int size,
- struct arch_hw_breakpoint *info)
+static bool ea_hw_range_overlaps(unsigned long ea, int size,
+ struct arch_hw_breakpoint *info)
{
unsigned long hw_start_addr, hw_end_addr;
hw_start_addr = ALIGN_DOWN(info->address, HW_BREAKPOINT_SIZE);
hw_end_addr = ALIGN(info->address + info->len, HW_BREAKPOINT_SIZE);
- return ((dar < hw_end_addr) && (dar + size > hw_start_addr));
+ return ((ea < hw_end_addr) && (ea + size > hw_start_addr));
}
/*
* If hw has multiple DAWR registers, we also need to check all
* dawrx constraint bits to confirm this is _really_ a valid event.
+ * If type is UNKNOWN, but privilege level matches, consider it as
+ * a positive match.
*/
static bool check_dawrx_constraints(struct pt_regs *regs, int type,
struct arch_hw_breakpoint *info)
@@ -536,7 +538,12 @@ static bool check_dawrx_constraints(struct pt_regs *regs, int type,
if (OP_IS_LOAD(type) && !(info->type & HW_BRK_TYPE_READ))
return false;
- if (OP_IS_STORE(type) && !(info->type & HW_BRK_TYPE_WRITE))
+ /*
+ * The Cache Management instructions other than dcbz never
+ * cause a match. i.e. if type is CACHEOP, the instruction
+ * is dcbz, and dcbz is treated as Store.
+ */
+ if ((OP_IS_STORE(type) || type == CACHEOP) && !(info->type & HW_BRK_TYPE_WRITE))
return false;
if (is_kernel_addr(regs->nip) && !(info->type & HW_BRK_TYPE_KERNEL))
@@ -553,7 +560,8 @@ static bool check_dawrx_constraints(struct pt_regs *regs, int type,
* including extraneous exception. Otherwise return false.
*/
static bool check_constraints(struct pt_regs *regs, struct ppc_inst instr,
- int type, int size, struct arch_hw_breakpoint *info)
+ unsigned long ea, int type, int size,
+ struct arch_hw_breakpoint *info)
{
bool in_user_range = dar_in_user_range(regs->dar, info);
bool dawrx_constraints;
@@ -569,11 +577,10 @@ static bool check_constraints(struct pt_regs *regs, struct ppc_inst instr,
}
if (unlikely(ppc_inst_equal(instr, ppc_inst(0)))) {
- if (in_user_range)
- return true;
-
- if (dar_in_hw_range(regs->dar, info)) {
- info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
+ if (cpu_has_feature(CPU_FTR_ARCH_31)) {
+ if (dar_in_hw_range(regs->dar, info))
+ return true;
+ } else {
return true;
}
return false;
@@ -581,10 +588,20 @@ static bool check_constraints(struct pt_regs *regs, struct ppc_inst instr,
dawrx_constraints = check_dawrx_constraints(regs, type, info);
- if (dar_user_range_overlaps(regs->dar, size, info))
+ if (type == UNKNOWN) {
+ if (cpu_has_feature(CPU_FTR_ARCH_31)) {
+ if (dar_in_hw_range(regs->dar, info))
+ return dawrx_constraints;
+ } else {
+ return dawrx_constraints;
+ }
+ return false;
+ }
+
+ if (ea_user_range_overlaps(ea, size, info))
return dawrx_constraints;
- if (dar_hw_range_overlaps(regs->dar, size, info)) {
+ if (ea_hw_range_overlaps(ea, size, info)) {
if (dawrx_constraints) {
info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
return true;
@@ -593,8 +610,17 @@ static bool check_constraints(struct pt_regs *regs, struct ppc_inst instr,
return false;
}
+static int cache_op_size(void)
+{
+#ifdef __powerpc64__
+ return ppc64_caches.l1d.block_size;
+#else
+ return L1_CACHE_BYTES;
+#endif
+}
+
static void get_instr_detail(struct pt_regs *regs, struct ppc_inst *instr,
- int *type, int *size, bool *larx_stcx)
+ int *type, int *size, unsigned long *ea)
{
struct instruction_op op;
@@ -602,16 +628,23 @@ static void get_instr_detail(struct pt_regs *regs, struct ppc_inst *instr,
return;
analyse_instr(&op, regs, *instr);
-
- /*
- * Set size = 8 if analyse_instr() fails. If it's a userspace
- * watchpoint(valid or extraneous), we can notify user about it.
- * If it's a kernel watchpoint, instruction emulation will fail
- * in stepping_handler() and watchpoint will be disabled.
- */
*type = GETTYPE(op.type);
- *size = !(*type == UNKNOWN) ? GETSIZE(op.type) : 8;
- *larx_stcx = (*type == LARX || *type == STCX);
+ *ea = op.ea;
+#ifdef __powerpc64__
+ if (!(regs->msr & MSR_64BIT))
+ *ea &= 0xffffffffUL;
+#endif
+
+ *size = GETSIZE(op.type);
+ if (*type == CACHEOP) {
+ *size = cache_op_size();
+ *ea &= ~(*size - 1);
+ }
+}
+
+static bool is_larx_stcx_instr(int type)
+{
+ return type == LARX || type == STCX;
}
/*
@@ -678,7 +711,7 @@ int hw_breakpoint_handler(struct die_args *args)
struct ppc_inst instr = ppc_inst(0);
int type = 0;
int size = 0;
- bool larx_stcx = false;
+ unsigned long ea;
/* Disable breakpoints during exception handling */
hw_breakpoint_disable();
@@ -692,7 +725,7 @@ int hw_breakpoint_handler(struct die_args *args)
rcu_read_lock();
if (!IS_ENABLED(CONFIG_PPC_8xx))
- get_instr_detail(regs, &instr, &type, &size, &larx_stcx);
+ get_instr_detail(regs, &instr, &type, &size, &ea);
for (i = 0; i < nr_wp_slots(); i++) {
bp[i] = __this_cpu_read(bp_per_reg[i]);
@@ -702,7 +735,7 @@ int hw_breakpoint_handler(struct die_args *args)
info[i] = counter_arch_bp(bp[i]);
info[i]->type &= ~HW_BRK_TYPE_EXTRANEOUS_IRQ;
- if (check_constraints(regs, instr, type, size, info[i])) {
+ if (check_constraints(regs, instr, ea, type, size, info[i])) {
if (!IS_ENABLED(CONFIG_PPC_8xx) &&
ppc_inst_equal(instr, ppc_inst(0))) {
handler_error(bp[i], info[i]);
@@ -744,7 +777,7 @@ int hw_breakpoint_handler(struct die_args *args)
}
if (!IS_ENABLED(CONFIG_PPC_8xx)) {
- if (larx_stcx) {
+ if (is_larx_stcx_instr(type)) {
for (i = 0; i < nr_wp_slots(); i++) {
if (!hit[i])
continue;
--
2.26.2
^ permalink raw reply related
* [PATCH v3 3/9] powerpc/watchpoint: Enable watchpoint functionality on power10 guest
From: Ravi Bangoria @ 2020-07-08 4:50 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec,
miltonm, oleg, npiggin, linux-kernel, paulus, jolsa, pedromfc,
naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200708045046.135702-1-ravi.bangoria@linux.ibm.com>
CPU_FTR_DAWR is by default enabled for host via CPU_FTRS_DT_CPU_BASE
(controlled by CONFIG_PPC_DT_CPU_FTRS). But cpu-features device-tree
node is not PAPR compatible and thus not yet used by kvm or pHyp
guests. Enable watchpoint functionality on power10 guest (both kvm
and powervm) by adding CPU_FTR_DAWR to CPU_FTRS_POWER10. Note that
this change does not enable 2nd DAWR support.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/cputable.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index bac2252c839e..e506d429b1af 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -478,7 +478,7 @@ static inline void cpu_feature_keys_init(void) { }
CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | CPU_FTR_PKEY | \
- CPU_FTR_ARCH_31)
+ CPU_FTR_ARCH_31 | CPU_FTR_DAWR)
#define CPU_FTRS_CELL (CPU_FTR_LWSYNC | \
CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
--
2.26.2
^ permalink raw reply related
* [PATCH v3 4/9] powerpc/dt_cpu_ftrs: Add feature for 2nd DAWR
From: Ravi Bangoria @ 2020-07-08 4:50 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec,
miltonm, oleg, npiggin, linux-kernel, paulus, jolsa, pedromfc,
naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200708045046.135702-1-ravi.bangoria@linux.ibm.com>
Add new device-tree feature for 2nd DAWR. If this feature is present,
2nd DAWR is supported, otherwise not.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/cputable.h | 7 +++++--
arch/powerpc/kernel/dt_cpu_ftrs.c | 7 +++++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index e506d429b1af..3445c86e1f6f 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -214,6 +214,7 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTR_P9_TLBIE_ERAT_BUG LONG_ASM_CONST(0x0001000000000000)
#define CPU_FTR_P9_RADIX_PREFETCH_BUG LONG_ASM_CONST(0x0002000000000000)
#define CPU_FTR_ARCH_31 LONG_ASM_CONST(0x0004000000000000)
+#define CPU_FTR_DAWR1 LONG_ASM_CONST(0x0008000000000000)
#ifndef __ASSEMBLY__
@@ -497,14 +498,16 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTRS_POSSIBLE \
(CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | CPU_FTRS_POWER8 | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_VSX_COMP | CPU_FTRS_POWER9 | \
- CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
+ CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 | \
+ CPU_FTR_DAWR1)
#else
#define CPU_FTRS_POSSIBLE \
(CPU_FTRS_PPC970 | CPU_FTRS_POWER5 | \
CPU_FTRS_POWER6 | CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | \
CPU_FTRS_POWER8 | CPU_FTRS_CELL | CPU_FTRS_PA6T | \
CPU_FTR_VSX_COMP | CPU_FTR_ALTIVEC_COMP | CPU_FTRS_POWER9 | \
- CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
+ CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 | \
+ CPU_FTR_DAWR1)
#endif /* CONFIG_CPU_LITTLE_ENDIAN */
#endif
#else
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index a0edeb391e3e..be694567cebd 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -573,6 +573,12 @@ static int __init feat_enable_mma(struct dt_cpu_feature *f)
return 1;
}
+static int __init feat_enable_debug_facilities_v31(struct dt_cpu_feature *f)
+{
+ cur_cpu_spec->cpu_features |= CPU_FTR_DAWR1;
+ return 1;
+}
+
struct dt_cpu_feature_match {
const char *name;
int (*enable)(struct dt_cpu_feature *f);
@@ -648,6 +654,7 @@ static struct dt_cpu_feature_match __initdata
{"wait-v3", feat_enable, 0},
{"prefix-instructions", feat_enable, 0},
{"matrix-multiply-assist", feat_enable_mma, 0},
+ {"debug-facilities-v31", feat_enable_debug_facilities_v31, 0},
};
static bool __initdata using_dt_cpu_ftrs;
--
2.26.2
^ permalink raw reply related
* [PATCH v3 5/9] powerpc/watchpoint: Set CPU_FTR_DAWR1 based on pa-features bit
From: Ravi Bangoria @ 2020-07-08 4:50 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec,
miltonm, oleg, npiggin, linux-kernel, paulus, jolsa, pedromfc,
naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200708045046.135702-1-ravi.bangoria@linux.ibm.com>
As per the PAPR, bit 0 of byte 64 in pa-features property indicates
availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
DAWR is present, otherwise not. Host generally uses "cpu-features",
which masks "pa-features". But "cpu-features" are still not used for
guests and thus this change is mostly applicable for guests only.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/kernel/prom.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 9cc49f265c86..c76c09b97bc8 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -175,6 +175,8 @@ static struct ibm_pa_feature {
*/
{ .pabyte = 22, .pabit = 0, .cpu_features = CPU_FTR_TM_COMP,
.cpu_user_ftrs2 = PPC_FEATURE2_HTM_COMP | PPC_FEATURE2_HTM_NOSC_COMP },
+
+ { .pabyte = 64, .pabit = 0, .cpu_features = CPU_FTR_DAWR1 },
};
static void __init scan_features(unsigned long node, const unsigned char *ftrs,
--
2.26.2
^ permalink raw reply related
* [PATCH v3 6/9] powerpc/watchpoint: Rename current H_SET_MODE DAWR macro
From: Ravi Bangoria @ 2020-07-08 4:50 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec,
miltonm, oleg, npiggin, linux-kernel, paulus, jolsa, pedromfc,
naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200708045046.135702-1-ravi.bangoria@linux.ibm.com>
Current H_SET_MODE hcall macro name for setting/resetting DAWR0 is
H_SET_MODE_RESOURCE_SET_DAWR. Add suffix 0 to macro name as well.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/hvcall.h | 2 +-
arch/powerpc/include/asm/plpar_wrappers.h | 2 +-
arch/powerpc/kvm/book3s_hv.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index e90c073e437e..a7f6f1aeda6b 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -354,7 +354,7 @@
/* Values for 2nd argument to H_SET_MODE */
#define H_SET_MODE_RESOURCE_SET_CIABR 1
-#define H_SET_MODE_RESOURCE_SET_DAWR 2
+#define H_SET_MODE_RESOURCE_SET_DAWR0 2
#define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE 3
#define H_SET_MODE_RESOURCE_LE 4
diff --git a/arch/powerpc/include/asm/plpar_wrappers.h b/arch/powerpc/include/asm/plpar_wrappers.h
index 4497c8afb573..93eb133d572c 100644
--- a/arch/powerpc/include/asm/plpar_wrappers.h
+++ b/arch/powerpc/include/asm/plpar_wrappers.h
@@ -312,7 +312,7 @@ static inline long plpar_set_ciabr(unsigned long ciabr)
static inline long plpar_set_watchpoint0(unsigned long dawr0, unsigned long dawrx0)
{
- return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR, dawr0, dawrx0);
+ return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR0, dawr0, dawrx0);
}
static inline long plpar_signal_sys_reset(long cpu)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 6bf66649ab92..7ad692c2d7c7 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -764,7 +764,7 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu, unsigned long mflags,
return H_P3;
vcpu->arch.ciabr = value1;
return H_SUCCESS;
- case H_SET_MODE_RESOURCE_SET_DAWR:
+ case H_SET_MODE_RESOURCE_SET_DAWR0:
if (!kvmppc_power8_compatible(vcpu))
return H_P2;
if (!ppc_breakpoint_available())
--
2.26.2
^ permalink raw reply related
* [PATCH v3 7/9] powerpc/watchpoint: Guest support for 2nd DAWR hcall
From: Ravi Bangoria @ 2020-07-08 4:50 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec,
miltonm, oleg, npiggin, linux-kernel, paulus, jolsa, pedromfc,
naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200708045046.135702-1-ravi.bangoria@linux.ibm.com>
2nd DAWR can be set/unset using H_SET_MODE hcall with resource value 5.
Enable powervm guest support with that. This has no effect on kvm guest
because kvm will return error if guest does hcall with resource value 5.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/hvcall.h | 1 +
arch/powerpc/include/asm/machdep.h | 2 +-
arch/powerpc/include/asm/plpar_wrappers.h | 5 +++++
arch/powerpc/kernel/dawr.c | 2 +-
arch/powerpc/platforms/pseries/setup.c | 7 +++++--
5 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index a7f6f1aeda6b..3f170b9496a1 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -357,6 +357,7 @@
#define H_SET_MODE_RESOURCE_SET_DAWR0 2
#define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE 3
#define H_SET_MODE_RESOURCE_LE 4
+#define H_SET_MODE_RESOURCE_SET_DAWR1 5
/* Values for argument to H_SIGNAL_SYS_RESET */
#define H_SIGNAL_SYS_RESET_ALL -1
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index 7bcb64444a39..a90b892f0bfe 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -131,7 +131,7 @@ struct machdep_calls {
unsigned long dabrx);
/* Set DAWR for this platform, leave empty for default implementation */
- int (*set_dawr)(unsigned long dawr,
+ int (*set_dawr)(int nr, unsigned long dawr,
unsigned long dawrx);
#ifdef CONFIG_PPC32 /* XXX for now */
diff --git a/arch/powerpc/include/asm/plpar_wrappers.h b/arch/powerpc/include/asm/plpar_wrappers.h
index 93eb133d572c..d7a1acc83593 100644
--- a/arch/powerpc/include/asm/plpar_wrappers.h
+++ b/arch/powerpc/include/asm/plpar_wrappers.h
@@ -315,6 +315,11 @@ static inline long plpar_set_watchpoint0(unsigned long dawr0, unsigned long dawr
return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR0, dawr0, dawrx0);
}
+static inline long plpar_set_watchpoint1(unsigned long dawr1, unsigned long dawrx1)
+{
+ return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR1, dawr1, dawrx1);
+}
+
static inline long plpar_signal_sys_reset(long cpu)
{
return plpar_hcall_norets(H_SIGNAL_SYS_RESET, cpu);
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index 500f52fa4711..cdc2dccb987d 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -37,7 +37,7 @@ int set_dawr(int nr, struct arch_hw_breakpoint *brk)
dawrx |= (mrd & 0x3f) << (63 - 53);
if (ppc_md.set_dawr)
- return ppc_md.set_dawr(dawr, dawrx);
+ return ppc_md.set_dawr(nr, dawr, dawrx);
if (nr == 0) {
mtspr(SPRN_DAWR0, dawr);
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 2db8469e475f..d516ee8eb7fc 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -831,12 +831,15 @@ static int pseries_set_xdabr(unsigned long dabr, unsigned long dabrx)
return plpar_hcall_norets(H_SET_XDABR, dabr, dabrx);
}
-static int pseries_set_dawr(unsigned long dawr, unsigned long dawrx)
+static int pseries_set_dawr(int nr, unsigned long dawr, unsigned long dawrx)
{
/* PAPR says we can't set HYP */
dawrx &= ~DAWRX_HYP;
- return plpar_set_watchpoint0(dawr, dawrx);
+ if (nr == 0)
+ return plpar_set_watchpoint0(dawr, dawrx);
+ else
+ return plpar_set_watchpoint1(dawr, dawrx);
}
#define CMO_CHARACTERISTICS_TOKEN 44
--
2.26.2
^ permalink raw reply related
* [PATCH v3 8/9] powerpc/watchpoint: Return available watchpoints dynamically
From: Ravi Bangoria @ 2020-07-08 4:50 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec,
miltonm, oleg, npiggin, linux-kernel, paulus, jolsa, pedromfc,
naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200708045046.135702-1-ravi.bangoria@linux.ibm.com>
So far Book3S Powerpc supported only one watchpoint. Power10 is
introducing 2nd DAWR. Enable 2nd DAWR support for Power10.
Availability of 2nd DAWR will depend on CPU_FTR_DAWR1.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/cputable.h | 4 +++-
arch/powerpc/include/asm/hw_breakpoint.h | 5 +++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 3445c86e1f6f..36a0851a7a9b 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -633,7 +633,9 @@ enum {
* Maximum number of hw breakpoint supported on powerpc. Number of
* breakpoints supported by actual hw might be less than this.
*/
-#define HBP_NUM_MAX 1
+#define HBP_NUM_MAX 2
+#define HBP_NUM_ONE 1
+#define HBP_NUM_TWO 2
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index cb424799da0d..d4eab1694bcd 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -5,10 +5,11 @@
* Copyright 2010, IBM Corporation.
* Author: K.Prasad <prasad@linux.vnet.ibm.com>
*/
-
#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
+#include <asm/cpu_has_feature.h>
+
#ifdef __KERNEL__
struct arch_hw_breakpoint {
unsigned long address;
@@ -46,7 +47,7 @@ struct arch_hw_breakpoint {
static inline int nr_wp_slots(void)
{
- return HBP_NUM_MAX;
+ return cpu_has_feature(CPU_FTR_DAWR1) ? HBP_NUM_TWO : HBP_NUM_ONE;
}
#ifdef CONFIG_HAVE_HW_BREAKPOINT
--
2.26.2
^ permalink raw reply related
* [PATCH v3 9/9] powerpc/watchpoint: Remove 512 byte boundary
From: Ravi Bangoria @ 2020-07-08 4:50 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec,
miltonm, oleg, npiggin, linux-kernel, paulus, jolsa, pedromfc,
naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200708045046.135702-1-ravi.bangoria@linux.ibm.com>
Power10 has removed 512 bytes boundary from match criteria. i.e. The watch
range can cross 512 bytes boundary.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/kernel/hw_breakpoint.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 7a66c370a105..270fbb4d01ce 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -418,8 +418,9 @@ static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw)
if (dawr_enabled()) {
max_len = DAWR_MAX_LEN;
- /* DAWR region can't cross 512 bytes boundary */
- if (ALIGN_DOWN(start_addr, SZ_512) != ALIGN_DOWN(end_addr - 1, SZ_512))
+ /* DAWR region can't cross 512 bytes boundary on p10 predecessors */
+ if (!cpu_has_feature(CPU_FTR_ARCH_31) &&
+ (ALIGN_DOWN(start_addr, SZ_512) != ALIGN_DOWN(end_addr - 1, SZ_512)))
return -EINVAL;
} else if (IS_ENABLED(CONFIG_PPC_8xx)) {
/* 8xx can setup a range without limitation */
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v3 0/6] powerpc: queued spinlocks and rwlocks
From: Nicholas Piggin @ 2020-07-08 5:10 UTC (permalink / raw)
To: linuxppc-dev, Waiman Long
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel, kvm-ppc,
virtualization, Ingo Molnar, Will Deacon
In-Reply-To: <de3ead58-7f81-8ebd-754d-244f6be24af4@redhat.com>
Excerpts from Waiman Long's message of July 8, 2020 1:33 pm:
> On 7/7/20 1:57 AM, Nicholas Piggin wrote:
>> Yes, powerpc could certainly get more performance out of the slow
>> paths, and then there are a few parameters to tune.
>>
>> We don't have a good alternate patching for function calls yet, but
>> that would be something to do for native vs pv.
>>
>> And then there seem to be one or two tunable parameters we could
>> experiment with.
>>
>> The paravirt locks may need a bit more tuning. Some simple testing
>> under KVM shows we might be a bit slower in some cases. Whether this
>> is fairness or something else I'm not sure. The current simple pv
>> spinlock code can do a directed yield to the lock holder CPU, whereas
>> the pv qspl here just does a general yield. I think we might actually
>> be able to change that to also support directed yield. Though I'm
>> not sure if this is actually the cause of the slowdown yet.
>
> Regarding the paravirt lock, I have taken a further look into the
> current PPC spinlock code. There is an equivalent of pv_wait() but no
> pv_kick(). Maybe PPC doesn't really need that.
So powerpc has two types of wait, either undirected "all processors" or
directed to a specific processor which has been preempted by the
hypervisor.
The simple spinlock code does a directed wait, because it knows the CPU
which is holding the lock. In this case, there is a sequence that is
used to ensure we don't wait if the condition has become true, and the
target CPU does not need to kick the waiter it will happen automatically
(see splpar_spin_yield). This is preferable because we only wait as
needed and don't require the kick operation.
The pv spinlock code I did uses the undirected wait, because we don't
know the CPU number which we are waiting on. This is undesirable because
it's higher overhead and the wait is not so accurate.
I think perhaps we could change things so we wait on the correct CPU
when queued, which might be good enough (we could also put the lock
owner CPU in the spinlock word, if we add another format).
> Attached are two
> additional qspinlock patches that adds a CONFIG_PARAVIRT_QSPINLOCKS_LITE
> option to not require pv_kick(). There is also a fixup patch to be
> applied after your patchset.
>
> I don't have access to a PPC LPAR with shared processor at the moment,
> so I can't test the performance of the paravirt code. Would you mind
> adding my patches and do some performance test on your end to see if it
> gives better result?
Great, I'll do some tests. Any suggestions for what to try?
Thanks,
Nick
^ permalink raw reply
* Re: Using Firefox hangs system
From: Nicholas Piggin @ 2020-07-08 5:11 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras,
Paul Menzel
Cc: linuxppc-dev
In-Reply-To: <604fa0d7-ddb5-7302-fd28-b9fee57ce015@molgen.mpg.de>
Excerpts from Paul Menzel's message of July 8, 2020 3:42 am:
> Dear Nicholas,
>
>
> Am 07.07.20 um 09:03 schrieb Nicholas Piggin:
>> Excerpts from Paul Menzel's message of July 6, 2020 3:20 pm:
>
>>> Am 06.07.20 um 02:41 schrieb Nicholas Piggin:
>>>> Excerpts from Paul Menzel's message of July 5, 2020 8:30 pm:
>>>
>>>>> Am 05.07.20 um 11:22 schrieb Paul Menzel:
>>>>>> [ 572.253008] Oops: Exception in kernel mode, sig: 5 [#1]
>>>>>> [ 572.253198] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV
>>>>>> [ 572.253232] Modules linked in: tcp_diag inet_diag unix_diag xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp ip6table_mangle ip6table_nat iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables nfnetlink ip6table_filter ip6_tables iptable_filter bridge stp llc overlay xfs kvm_hv kvm binfmt_misc joydev uas usb_storage vmx_crypto bnx2x crct10dif_vpmsum ofpart cmdlinepart powernv_flash mtd mdio ibmpowernv at24 ipmi_powernv ipmi_devintf ipmi_msghandler opal_prd powernv_rng sch_fq_codel parport_pc ppdev lp nfsd parport auth_rpcgss nfs_acl lockd grace sunrpc ip_tables x_tables autofs4 btrfs blake2b_generic libcrc32c xor zstd_compress raid6_pq input_leds mac_hid hid_generic ast drm_vram_helper drm_ttm_helper i2c_algo_bit ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm ahci drm_panel_orientation_quirks libahci usbhid hid crc32c_vpmsum uio_pdrv_genirq uio
>>>>>> [ 572.253639] CPU: 4 PID: 6728 Comm: Web Content Not tainted 5.8.0-rc3+ #1
>>>>>> [ 572.253659] NIP: c00000000000ff5c LR: c00000000001a8f8 CTR: c0000000001d5f00
>>>>>> [ 572.253835] REGS: c000007f31f0f420 TRAP: 1500 Not tainted (5.8.0-rc3+)
>>>>>> [ 572.253854] MSR: 900000000290b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 28c48482 XER: 20000000
>>>>>> [ 572.253888] CFAR: c00000000000fecc IRQMASK: 1
>>>>>> [ 572.253888] GPR00: c00000000001b228 c000007f31f0f6b0 c000000001f9a900 c000007f351544d0
>>>>>> [ 572.253888] GPR04: 0000000000000000 c000007f31f0fe90 c000007f351544f0 c000007f32e522b0
>>>>>> [ 572.253888] GPR08: 0000000000000000 0000000000002000 9000000000009033 c000007fbcd85800
>>>>>> [ 572.253888] GPR12: 0000000000008800 c000007fffffb680 0000000000000005 0000000000000004
>>>>>> [ 572.253888] GPR16: c000007f35153800 c000007f35154130 0000000000000005 0000000000000001
>>>>>> [ 572.253888] GPR20: 0000000000000024 c000007f32e51e68 c000007f35154028 0000007fd8da0000
>>>>>> [ 572.253888] GPR24: 0000007fd8da0000 c000007f351544d0 c000007e9a4024d0 c000000001665f18
>>>>>> [ 572.253888] GPR28: c000007f351544d0 c000007f35153800 900000000290f033 c000007f35153800
>>>>>> [ 572.254079] NIP [c00000000000ff5c] save_fpu+0xa8/0x2ac
>>>>>> [ 572.254098] LR [c00000000001a8f8] __giveup_fpu+0x28/0x80
>>>>>> [ 572.254114] Call Trace:
>>>>>> [ 572.254128] [c000007f31f0f6b0] [c000007f35153980] 0xc000007f35153980 (unreliable)
>>>>>> [ 572.254156] [c000007f31f0f6e0] [c00000000001b228] giveup_all+0x128/0x150
>>>>>> [ 572.254327] [c000007f31f0f710] [c00000000001c124] __switch_to+0x104/0x490
>>>>>> [ 572.254352] [c000007f31f0f770] [c0000000010d2e34] __schedule+0x2e4/0xa10
>>>>>> [ 572.254374] [c000007f31f0f840] [c0000000010d35d4] schedule+0x74/0x140
>>>>>> [ 572.254397] [c000007f31f0f870] [c0000000010d9478] schedule_timeout+0x358/0x5d0
>>>>>> [ 572.254424] [c000007f31f0f980] [c0000000010d5638] wait_for_completion+0xc8/0x210
>>>>>> [ 572.254451] [c000007f31f0fa00] [c000000000608ed4] do_coredump+0x3a4/0xd60
>>>>>> [ 572.254625] [c000007f31f0fba0] [c00000000018d1cc] get_signal+0x1dc/0xd00
>>>>>> [ 572.254648] [c000007f31f0fcc0] [c00000000001f088] do_notify_resume+0x158/0x450
>>>>>> [ 572.254672] [c000007f31f0fda0] [c000000000037d04] interrupt_exit_user_prepare+0x1c4/0x230
>>>>>> [ 572.254699] [c000007f31f0fe20] [c00000000000f2b4] interrupt_return+0x14/0x1c0
>>>>>> [ 572.254720] Instruction dump:
>>>>>> [ 572.254882] dae60170 db060180 db260190 db4601a0 db6601b0 db8601c0 dba601d0 dbc601e0
>>>>>> [ 572.254912] dbe601f0 48000204 38800000 f0000250 <7c062798> f0000250 38800010 f0210a50
>>>>>> [ 572.254946] ---[ end trace ba4452ee5c77d58e ]---
>>>>>
>>>>> Please find all the messages attached.
>>>>
>>>> "Oops: Exception in kernel mode, sig: 5 [#1]"
>>>>
>>>> Unfortunately it's a very poor error message. I think it is a 0x1500
>>>> exception triggering in the kernel FP register saving. Do you have the
>>>> CONFIG_PPC_DENORMALISATION config option set?
>>>
>>> Yes, as it’s set in the Ubuntu Linux kernel configuration, I have it set
>>> too.
>>>
>>> $ grep DENORMALI /boot/config-*
>>> /boot/config-4.15.0-23-generic:CONFIG_PPC_DENORMALISATION=y
>>> /boot/config-5.4.0-40-generic:CONFIG_PPC_DENORMALISATION=y
>>> /boot/config-5.7.0-rc5+:CONFIG_PPC_DENORMALISATION=y
>>> /boot/config-5.8.0-rc3+:CONFIG_PPC_DENORMALISATION=y
>>
>> Ah thanks I was able to reproduce with a little denorm test case.
>>
>> The denorm interrupt handler got broken by some careless person.
>>
>> This patch should hopefully fix it for you?
>
> Yes, it does. Thank you.
>
>> ---
>> arch/powerpc/kernel/exceptions-64s.S | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
>> index fa080694e581..0fc8bad878b2 100644
>> --- a/arch/powerpc/kernel/exceptions-64s.S
>> +++ b/arch/powerpc/kernel/exceptions-64s.S
>> @@ -2551,7 +2551,7 @@ EXC_VIRT_NONE(0x5400, 0x100)
>> INT_DEFINE_BEGIN(denorm_exception)
>> IVEC=0x1500
>> IHSRR=1
>> - IBRANCH_COMMON=0
>> + IBRANCH_TO_COMMON=0
>> IKVM_REAL=1
>> INT_DEFINE_END(denorm_exception)
>
> Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>
Thanks for reporting and testing, sorry for breaking it for you!
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH] powerpc: select ARCH_HAS_MEMBARRIER_SYNC_CORE
From: Nicholas Piggin @ 2020-07-08 5:17 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: linux-arch, linuxppc-dev
In-Reply-To: <638683144.970.1594121101349.JavaMail.zimbra@efficios.com>
Excerpts from Mathieu Desnoyers's message of July 7, 2020 9:25 pm:
> ----- On Jul 7, 2020, at 1:50 AM, Nicholas Piggin npiggin@gmail.com wrote:
>
>> Excerpts from Christophe Leroy's message of July 6, 2020 7:53 pm:
>>>
>>>
>>> Le 06/07/2020 à 04:18, Nicholas Piggin a écrit :
>>>> diff --git a/arch/powerpc/include/asm/exception-64s.h
>>>> b/arch/powerpc/include/asm/exception-64s.h
>>>> index 47bd4ea0837d..b88cb3a989b6 100644
>>>> --- a/arch/powerpc/include/asm/exception-64s.h
>>>> +++ b/arch/powerpc/include/asm/exception-64s.h
>>>> @@ -68,6 +68,10 @@
>>>> *
>>>> * The nop instructions allow us to insert one or more instructions to flush the
>>>> * L1-D cache when returning to userspace or a guest.
>>>> + *
>>>> + * powerpc relies on return from interrupt/syscall being context synchronising
>>>> + * (which hrfid, rfid, and rfscv are) to support ARCH_HAS_MEMBARRIER_SYNC_CORE
>>>> + * without additional additional synchronisation instructions.
>>>
>>> This file is dedicated to BOOK3S/64. What about other ones ?
>>>
>>> On 32 bits, this is also valid as 'rfi' is also context synchronising,
>>> but then why just add some comment in exception-64s.h and only there ?
>>
>> Yeah you're right, I basically wanted to keep a note there just in case,
>> because it's possible we would get a less synchronising return (maybe
>> unlikely with meltdown) or even return from a kernel interrupt using a
>> something faster (e.g., bctar if we don't use tar register in the kernel
>> anywhere).
>>
>> So I wonder where to add the note, entry_32.S and 64e.h as well?
>>
>
> For 64-bit powerpc, I would be tempted to either place the comment in the header
> implementing the RFI_TO_USER and RFI_TO_USER_OR_KERNEL macros or the .S files
> using them, e.g. either:
>
> arch/powerpc/include/asm/exception-64e.h
> arch/powerpc/include/asm/exception-64s.h
>
> or
>
> arch/powerpc/kernel/exceptions-64s.S
> arch/powerpc/kernel/entry_64.S
>
> And for 32-bit powerpc, AFAIU
>
> arch/powerpc/kernel/entry_32.S
>
> uses SYNC + RFI to return to user-space. RFI is defined in
>
> arch/powerpc/include/asm/ppc_asm.h
>
> So a comment either near the RFI define and its uses should work.
>
>> I should actually change the comment for 64-bit because soft masked
>> interrupt replay is an interesting case. I thought it was okay (because
>> the IPI would cause a hard interrupt which does do the rfi) but that
>> should at least be written.
>
> Yes.
>
>> The context synchronisation happens before
>> the Linux IPI function is called, but for the purpose of membarrier I
>> think that is okay (the membarrier just needs to have caused a memory
>> barrier + context synchronistaion by the time it has done).
>
> Can you point me to the code implementing this logic ?
It's mostly in arch/powerpc/kernel/exception-64s.S and
powerpc/kernel/irq.c, but a lot of asm so easier to explain.
When any Linux code does local_irq_disable(), we set interrupts as
software-masked in a per-cpu flag. When interrupts (including IPIs) come
in, the first thing we do is check that flag and if we are masked, then
record that the interrupt needs to be "replayed" in another per-cpu
flag. The interrupt handler then exits back using RFI (which is context
synchronising the CPU). Later, when the kernel code does
local_irq_enable(), it checks the replay flag to see if anything needs
to be done. At that point we basically just call the interrupt handler
code like a normal function, and when that returns there is no context
synchronising instruction.
So membarrier IPI will always cause target CPUs to perform a context
synchronising instruction, but sometimes it happens before the IPI
handler function runs.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 06/20] Documentation: gpu/komeda-kms: eliminate duplicated word
From: james qian wang (Arm Technology China) @ 2020-07-08 5:08 UTC (permalink / raw)
To: Randy Dunlap
Cc: kvm, linux-doc, David Airlie, kgdb-bugreport, linux-fpga,
Liviu Dudau, dri-devel, Douglas Anderson, Paul Cercueil, keyrings,
Paul Mackerras, linux-i2c, Pavel Machek, Srinivas Pandruvada,
Mihail Atanassov, linux-leds, linux-s390, Daniel Thompson,
linux-scsi, Jonathan Corbet, Masahiro Yamada, Matthew Wilcox,
Halil Pasic, Jarkko Sakkinen, Mali DP Maintainers, linux-input,
Derek Kiernan, linux-mips, Dragan Cvetic, Wu Hao, Tony Krowiak,
linux-kbuild, James E.J. Bottomley, Jiri Kosina, Hannes Reinecke,
linux-block, Thomas Bogendoerfer, Jacek Anaszewski, linux-mm,
Dan Williams, nd, Andrew Morton, Mimi Zohar, Jens Axboe,
Michal Marek, Martin K. Petersen, Pierre Morel, linux-kernel,
Wolfram Sang, Daniel Vetter, Jason Wessel, Paolo Bonzini,
linux-integrity, linuxppc-dev, Mike Rapoport, Dan Murphy
In-Reply-To: <20200707180414.10467-7-rdunlap@infradead.org>
Hi Randy
On Tue, Jul 07, 2020 at 11:04:00AM -0700, Randy Dunlap wrote:
> Drop the doubled word "and".
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: linux-doc@vger.kernel.org
> Cc: James (Qian) Wang <james.qian.wang@arm.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Mihail Atanassov <mihail.atanassov@arm.com>
> Cc: Mali DP Maintainers <malidp@foss.arm.com>
> ---
> Documentation/gpu/komeda-kms.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-next-20200701.orig/Documentation/gpu/komeda-kms.rst
> +++ linux-next-20200701/Documentation/gpu/komeda-kms.rst
> @@ -41,7 +41,7 @@ Compositor blends multiple layers or pix
> frame. its output frame can be fed into post image processor for showing it on
> the monitor or fed into wb_layer and written to memory at the same time.
> user can also insert a scaler between compositor and wb_layer to down scale
> -the display frame first and and then write to memory.
> +the display frame first and then write to memory.
Thank you for the patch.
Reviewed-by: James Qian Wang <james.qian.wang@arm.com>
> Writeback Layer (wb_layer)
> --------------------------
^ permalink raw reply
* RE: FSL P5020/P5040: DPAA Ethernet issue with the latest Git kernel
From: Madalin Bucur (OSS) @ 2020-07-08 6:03 UTC (permalink / raw)
To: Christian Zigotzky, Madalin Bucur (OSS)
Cc: Darren Stevens, mad skateman, netdev@vger.kernel.org,
linuxppc-dev@ozlabs.org, Camelia Alexandra Groza, R.T.Dickinson
In-Reply-To: <4E3069C3-B777-4460-A781-84214C4539DA@xenosoft.de>
> From: Christian Zigotzky <chzigotzky@xenosoft.de>
> Sent: Tuesday, July 7, 2020 9:26 PM
> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> Cc: mad skateman <madskateman@gmail.com>; Camelia Alexandra Groza <camelia.groza@nxp.com>;
> linuxppc-dev@ozlabs.org; netdev@vger.kernel.org; R.T.Dickinson <rtd2@xtra.co.nz>;
> Darren Stevens <darren@stevens-zone.net>
> Subject: Re: FSL P5020/P5040: DPAA Ethernet issue with the latest Git kernel
>
>
> On 7. Jul 2020, at 17:53, Madalin Bucur (OSS) <mailto:madalin.bucur@oss.nxp.com> wrote:
> Was DPAA functional before commit A?
> How about after commit A and before commit B?
> The DPAA Ethernet works from the kernel 5.6-rc4 [1] till the Git kernel from the
> 11 of June [2]. It doesn’t work since the commit “fix bitmap_parse” [3].
> [1] https://forum.hyperion-entertainment.com/viewtopic.php?p=49936#p49936
> [2] https://forum.hyperion-entertainment.com/viewtopic.php?p=50848#p50848
> [3] https://forum.hyperion-entertainment.com/viewtopic.php?p=50980#p50980
Hi,
can you please try to disable the network manager (see [1]), then boot with
the latest kernel, that does not work, and setup the interfaces manually?
Madalin
[1] https://help.ubuntu.com/community/NetworkManager#Stopping_and_Disabling_NetworkManager
^ permalink raw reply
* Re: [PATCH 00/20] Documentation: eliminate duplicated words
From: Martin K. Petersen @ 2020-07-08 6:06 UTC (permalink / raw)
To: linux-kernel, Randy Dunlap
Cc: kvm, linux-doc, David Airlie, kgdb-bugreport, linux-fpga,
Liviu Dudau, Jarkko Sakkinen, linux-mips, Paul Cercueil, keyrings,
Paul Mackerras, linux-i2c, Pavel Machek, Srinivas Pandruvada,
Mihail Atanassov, linux-leds, linux-s390, Daniel Thompson,
linux-scsi, Jonathan Corbet, Masahiro Yamada, Matthew Wilcox,
Halil Pasic, James Wang, linux-input, Mali DP Maintainers,
Derek Kiernan, Dragan Cvetic, Wu Hao, Jens Axboe, linux-kbuild,
James E.J. Bottomley, Jiri Kosina, Hannes Reinecke, linux-block,
Thomas Bogendoerfer, Jacek Anaszewski, linux-mm, Dan Williams,
Andrew Morton, Mimi Zohar, Tony Krowiak, Michal Marek,
Pierre Morel, Martin K . Petersen, dri-devel, Douglas Anderson,
Wolfram Sang, Daniel Vetter, Jason Wessel, Paolo Bonzini,
linux-integrity, linuxppc-dev, Mike Rapoport, Dan Murphy
In-Reply-To: <20200707180414.10467-1-rdunlap@infradead.org>
On Tue, 7 Jul 2020 11:03:54 -0700, Randy Dunlap wrote:
> Drop doubled words in various parts of Documentation/.
>
> [...]
Applied to 5.9/scsi-queue, thanks!
[17/20] scsi: advansys: docs: Eliminate duplicated word
https://git.kernel.org/mkp/scsi/c/3010dfb0b77c
--
Martin K. Petersen Oracle Linux Engineering
^ 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