* [PATCH v2 2/2] powerpc/numa: Fill distance_lookup_table for offline nodes
From: Srikar Dronamraju @ 2021-07-01 4:15 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot,
Srikar Dronamraju, Rik van Riel, linuxppc-dev,
Geetika Moolchandani, LKML, Dietmar Eggemann, Thomas Gleixner,
Laurent Dufour, Mel Gorman, Valentin Schneider
In-Reply-To: <20210701041552.112072-1-srikar@linux.vnet.ibm.com>
Currently scheduler populates the distance map by looking at distance
of each node from all other nodes. This should work for most
architectures and platforms.
Scheduler expects unique number of node distances to be available at
boot. It uses node distance to calculate this unique node distances.
On Power Servers, node distances for offline nodes is not available.
However, Power Servers already knows unique possible node distances.
Fake the offline node's distance_lookup_table entries so that all
possible node distances are updated.
For example distance info from numactl from a fully populated 8 node
system at boot may look like this.
node distances:
node 0 1 2 3 4 5 6 7
0: 10 20 40 40 40 40 40 40
1: 20 10 40 40 40 40 40 40
2: 40 40 10 20 40 40 40 40
3: 40 40 20 10 40 40 40 40
4: 40 40 40 40 10 20 40 40
5: 40 40 40 40 20 10 40 40
6: 40 40 40 40 40 40 10 20
7: 40 40 40 40 40 40 20 10
However the same system when only two nodes are online at boot, then
distance info from numactl will look like
node distances:
node 0 1
0: 10 20
1: 20 10
It may be implementation dependent on what node_distance(0,3) where
node 0 is online and node 3 is offline. In Power Servers case, it returns
LOCAL_DISTANCE(10). Here at boot the scheduler would assume that the max
distance between nodes is 20. However that would not be true.
When Nodes are onlined and CPUs from those nodes are hotplugged,
the max node distance would be 40.
However this only needs to be done if the number of unique node
distances that can be computed for online nodes is less than the
number of possible unique node distances as represented by
distance_ref_points_depth. When the node is actually onlined,
distance_lookup_table will be updated with actual entries.
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Rik van Riel <riel@surriel.com>
Cc: Geetika Moolchandani <Geetika.Moolchandani1@ibm.com>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Reported-by: Geetika Moolchandani <Geetika.Moolchandani1@ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changelog v1->v2:
Move to a Powerpc specific solution as suggested by Peter and Valentin
arch/powerpc/mm/numa.c | 70 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index f2bf98bdcea2..6d0d89127190 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -860,6 +860,75 @@ void __init dump_numa_cpu_topology(void)
}
}
+/*
+ * Scheduler expects unique number of node distances to be available at
+ * boot. It uses node distance to calculate this unique node distances. On
+ * POWER, node distances for offline nodes is not available. However, POWER
+ * already knows unique possible node distances. Fake the offline node's
+ * distance_lookup_table entries so that all possible node distances are
+ * updated.
+ */
+void __init fake_update_distance_lookup_table(void)
+{
+ unsigned long distance_map;
+ int i, nr_levels, nr_depth, node;
+
+ if (!numa_enabled)
+ return;
+
+ if (!form1_affinity)
+ return;
+
+ /*
+ * distance_ref_points_depth lists the unique numa domains
+ * available. However it ignore LOCAL_DISTANCE. So add +1
+ * to get the actual number of unique distances.
+ */
+ nr_depth = distance_ref_points_depth + 1;
+
+ WARN_ON(nr_depth > sizeof(distance_map));
+
+ bitmap_zero(&distance_map, nr_depth);
+ bitmap_set(&distance_map, 0, 1);
+
+ for_each_online_node(node) {
+ int nd, distance = LOCAL_DISTANCE;
+
+ if (node == first_online_node)
+ continue;
+
+ nd = __node_distance(node, first_online_node);
+ for (i = 0; i < nr_depth; i++, distance *= 2) {
+ if (distance == nd) {
+ bitmap_set(&distance_map, i, 1);
+ break;
+ }
+ }
+ nr_levels = bitmap_weight(&distance_map, nr_depth);
+ if (nr_levels == nr_depth)
+ return;
+ }
+
+ for_each_node(node) {
+ if (node_online(node))
+ continue;
+
+ i = find_first_zero_bit(&distance_map, nr_depth);
+ if (i >= nr_depth || i == 0) {
+ pr_warn("Levels(%d) not matching levels(%d)", nr_levels, nr_depth);
+ return;
+ }
+
+ bitmap_set(&distance_map, i, 1);
+ while (i--)
+ distance_lookup_table[node][i] = node;
+
+ nr_levels = bitmap_weight(&distance_map, nr_depth);
+ if (nr_levels == nr_depth)
+ return;
+ }
+}
+
/* Initialize NODE_DATA for a node on the local memory */
static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn)
{
@@ -975,6 +1044,7 @@ void __init mem_topology_setup(void)
*/
numa_setup_cpu(cpu);
}
+ fake_update_distance_lookup_table();
}
void __init initmem_init(void)
--
2.27.0
^ permalink raw reply related
* [PATCH v2 0/2] Skip numa distance for offline nodes
From: Srikar Dronamraju @ 2021-07-01 4:15 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot,
Srikar Dronamraju, Rik van Riel, linuxppc-dev,
Geetika Moolchandani, LKML, Dietmar Eggemann, Thomas Gleixner,
Laurent Dufour, Mel Gorman, Valentin Schneider
Changelog v1->v2:
v1: http://lore.kernel.org/lkml/20210520154427.1041031-1-srikar@linux.vnet.ibm.com/t/#u
- Update the numa masks, whenever 1st CPU is added to cpuless node
- Populate all possible nodes distances in boot in a
powerpc specific function
Geetika reported yet another trace while doing a dlpar CPU add
operation. This was true even on top of a recent commit
6980d13f0dd1 ("powerpc/smp: Set numa node before updating mask") which fixed
a similar trace.
WARNING: CPU: 40 PID: 2954 at kernel/sched/topology.c:2088 build_sched_domains+0x6e8/0x1540
Modules linked in: nft_counter nft_compat rpadlpar_io rpaphp mptcp_diag
xsk_diag tcp_diag udp_diag raw_diag inet_diag unix_diag af_packet_diag
netlink_diag bonding tls nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib
nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat
nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set rfkill nf_tables
nfnetlink dm_multipath pseries_rng xts vmx_crypto binfmt_misc ip_tables xfs
libcrc32c sd_mod t10_pi sg ibmvscsi ibmveth scsi_transport_srp dm_mirror
dm_region_hash dm_log dm_mod fuse
CPU: 40 PID: 2954 Comm: kworker/40:0 Not tainted 5.13.0-rc1+ #19
Workqueue: events cpuset_hotplug_workfn
NIP: c0000000001de588 LR: c0000000001de584 CTR: 00000000006cd36c
REGS: c00000002772b250 TRAP: 0700 Not tainted (5.12.0-rc5-master+)
MSR: 8000000000029033 <SF,EE,ME,IR,DR,RI,LE> CR: 28828422 XER: 0000000d
CFAR: c00000000020c2f8 IRQMASK: 0 #012GPR00: c0000000001de584 c00000002772b4f0
c000000001f55400 0000000000000036 #012GPR04: c0000063c6368010 c0000063c63f0a00
0000000000000027 c0000063c6368018 #012GPR08: 0000000000000023 c0000063c636ef48
00000063c4de0000 c0000063bfe9ffe8 #012GPR12: 0000000028828424 c0000063fe68fe80
0000000000000000 0000000000000417 #012GPR16: 0000000000000028 c00000000740dcd8
c00000000205db68 c000000001a3a4a0 #012GPR20: c000000091ed7d20 c000000091ed8520
0000000000000001 0000000000000000 #012GPR24: c0000000113a9600 0000000000000190
0000000000000028 c0000000010e3ac0 #012GPR28: 0000000000000000 c00000000740dd00
c0000000317b5900 0000000000000190
NIP [c0000000001de588] build_sched_domains+0x6e8/0x1540
LR [c0000000001de584] build_sched_domains+0x6e4/0x1540
Call Trace:
[c00000002772b4f0] [c0000000001de584] build_sched_domains+0x6e4/0x1540 (unreliable)
[c00000002772b640] [c0000000001e08dc] partition_sched_domains_locked+0x3ec/0x530
[c00000002772b6e0] [c0000000002a2144] rebuild_sched_domains_locked+0x524/0xbf0
[c00000002772b7e0] [c0000000002a5620] rebuild_sched_domains+0x40/0x70
[c00000002772b810] [c0000000002a58e4] cpuset_hotplug_workfn+0x294/0xe20
[c00000002772bc30] [c000000000187510] process_one_work+0x300/0x670
[c00000002772bd10] [c0000000001878f8] worker_thread+0x78/0x520
[c00000002772bda0] [c0000000001937f0] kthread+0x1a0/0x1b0
[c00000002772be10] [c00000000000d6ec] ret_from_kernel_thread+0x5c/0x70
Instruction dump:
7ee5bb78 7f0ac378 7f29cb78 7f68db78 7f46d378 7f84e378 f8610068 3c62ff19
fbe10060 3863e558 4802dd31 60000000 <0fe00000> 3920fff4 f9210080 e86100b0
Detailed analysis of the failing scenario showed that the span in
question belongs to NODE domain and further the cpumasks for some
cpus in NODE overlapped. There are two possible reasons how we ended
up here:
(1) The numa node was offline or blank with no CPUs or memory. Hence
the sched_max_numa_distance could not be set correctly, or the
sched_domains_numa_distance happened to be partially populated.
(2) Depending on a bogus node_distance of an offline node to populate
cpumasks is the issue. On POWER platform the node_distance is
correctly available only for an online node which has some CPU or
memory resource associated with it.
For example distance info from numactl from a fully populated 8 node
system at boot may look like this.
node distances:
node 0 1 2 3 4 5 6 7
0: 10 20 40 40 40 40 40 40
1: 20 10 40 40 40 40 40 40
2: 40 40 10 20 40 40 40 40
3: 40 40 20 10 40 40 40 40
4: 40 40 40 40 10 20 40 40
5: 40 40 40 40 20 10 40 40
6: 40 40 40 40 40 40 10 20
7: 40 40 40 40 40 40 20 10
However the same system when only two nodes are online at boot, then the
numa topology will look like
node distances:
node 0 1
0: 10 20
1: 20 10
This series tries to fix both these problems.
Note: These problems are now visible, thanks to
Commit ccf74128d66c ("sched/topology: Assert non-NUMA topology masks don't
(partially) overlap")
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Rik van Riel <riel@surriel.com>
Cc: Geetika Moolchandani <Geetika.Moolchandani1@ibm.com>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Srikar Dronamraju (2):
sched/topology: Skip updating masks for non-online nodes
powerpc/numa: Fill distance_lookup_table for offline nodes
arch/powerpc/mm/numa.c | 70 +++++++++++++++++++++++++++++++++++++++++
kernel/sched/topology.c | 25 +++++++++++++--
2 files changed, 93 insertions(+), 2 deletions(-)
base-commit: 031e3bd8986fffe31e1ddbf5264cccfe30c9abd7
--
2.27.0
^ permalink raw reply
* Re: [RESEND PATCH v4 05/11] powerpc/64s: Add ability to skip SLB preload
From: Nicholas Piggin @ 2021-07-01 4:15 UTC (permalink / raw)
To: Christopher M. Riedl, Daniel Axtens, linuxppc-dev
Cc: tglx, x86, keescook, linux-hardening
In-Reply-To: <CCHHVUNV216M.1825LSMNZ1XG7@oc8246131445.ibm.com>
Excerpts from Christopher M. Riedl's message of July 1, 2021 1:48 pm:
> On Sun Jun 20, 2021 at 10:13 PM CDT, Daniel Axtens wrote:
>> "Christopher M. Riedl" <cmr@linux.ibm.com> writes:
>>
>> > Switching to a different mm with Hash translation causes SLB entries to
>> > be preloaded from the current thread_info. This reduces SLB faults, for
>> > example when threads share a common mm but operate on different address
>> > ranges.
>> >
>> > Preloading entries from the thread_info struct may not always be
>> > appropriate - such as when switching to a temporary mm. Introduce a new
>> > boolean in mm_context_t to skip the SLB preload entirely. Also move the
>> > SLB preload code into a separate function since switch_slb() is already
>> > quite long. The default behavior (preloading SLB entries from the
>> > current thread_info struct) remains unchanged.
>> >
>> > Signed-off-by: Christopher M. Riedl <cmr@linux.ibm.com>
>> >
>> > ---
>> >
>> > v4: * New to series.
>> > ---
>> > arch/powerpc/include/asm/book3s/64/mmu.h | 3 ++
>> > arch/powerpc/include/asm/mmu_context.h | 13 ++++++
>> > arch/powerpc/mm/book3s64/mmu_context.c | 2 +
>> > arch/powerpc/mm/book3s64/slb.c | 56 ++++++++++++++----------
>> > 4 files changed, 50 insertions(+), 24 deletions(-)
>> >
>> > diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
>> > index eace8c3f7b0a1..b23a9dcdee5af 100644
>> > --- a/arch/powerpc/include/asm/book3s/64/mmu.h
>> > +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
>> > @@ -130,6 +130,9 @@ typedef struct {
>> > u32 pkey_allocation_map;
>> > s16 execute_only_pkey; /* key holding execute-only protection */
>> > #endif
>> > +
>> > + /* Do not preload SLB entries from thread_info during switch_slb() */
>> > + bool skip_slb_preload;
>> > } mm_context_t;
>> >
>> > static inline u16 mm_ctx_user_psize(mm_context_t *ctx)
>> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
>> > index 4bc45d3ed8b0e..264787e90b1a1 100644
>> > --- a/arch/powerpc/include/asm/mmu_context.h
>> > +++ b/arch/powerpc/include/asm/mmu_context.h
>> > @@ -298,6 +298,19 @@ static inline int arch_dup_mmap(struct mm_struct *oldmm,
>> > return 0;
>> > }
>> >
>> > +#ifdef CONFIG_PPC_BOOK3S_64
>> > +
>> > +static inline void skip_slb_preload_mm(struct mm_struct *mm)
>> > +{
>> > + mm->context.skip_slb_preload = true;
>> > +}
>> > +
>> > +#else
>> > +
>> > +static inline void skip_slb_preload_mm(struct mm_struct *mm) {}
>> > +
>> > +#endif /* CONFIG_PPC_BOOK3S_64 */
>> > +
>> > #include <asm-generic/mmu_context.h>
>> >
>> > #endif /* __KERNEL__ */
>> > diff --git a/arch/powerpc/mm/book3s64/mmu_context.c b/arch/powerpc/mm/book3s64/mmu_context.c
>> > index c10fc8a72fb37..3479910264c59 100644
>> > --- a/arch/powerpc/mm/book3s64/mmu_context.c
>> > +++ b/arch/powerpc/mm/book3s64/mmu_context.c
>> > @@ -202,6 +202,8 @@ int init_new_context(struct task_struct *tsk, struct mm_struct *mm)
>> > atomic_set(&mm->context.active_cpus, 0);
>> > atomic_set(&mm->context.copros, 0);
>> >
>> > + mm->context.skip_slb_preload = false;
>> > +
>> > return 0;
>> > }
>> >
>> > diff --git a/arch/powerpc/mm/book3s64/slb.c b/arch/powerpc/mm/book3s64/slb.c
>> > index c91bd85eb90e3..da0836cb855af 100644
>> > --- a/arch/powerpc/mm/book3s64/slb.c
>> > +++ b/arch/powerpc/mm/book3s64/slb.c
>> > @@ -441,10 +441,39 @@ static void slb_cache_slbie_user(unsigned int index)
>> > asm volatile("slbie %0" : : "r" (slbie_data));
>> > }
>> >
>> > +static void preload_slb_entries(struct task_struct *tsk, struct mm_struct *mm)
>> Should this be explicitly inline or even __always_inline? I'm thinking
>> switch_slb is probably a fairly hot path on hash?
>
> Yes absolutely. I'll make this change in v5.
>
>>
>> > +{
>> > + struct thread_info *ti = task_thread_info(tsk);
>> > + unsigned char i;
>> > +
>> > + /*
>> > + * We gradually age out SLBs after a number of context switches to
>> > + * reduce reload overhead of unused entries (like we do with FP/VEC
>> > + * reload). Each time we wrap 256 switches, take an entry out of the
>> > + * SLB preload cache.
>> > + */
>> > + tsk->thread.load_slb++;
>> > + if (!tsk->thread.load_slb) {
>> > + unsigned long pc = KSTK_EIP(tsk);
>> > +
>> > + preload_age(ti);
>> > + preload_add(ti, pc);
>> > + }
>> > +
>> > + for (i = 0; i < ti->slb_preload_nr; i++) {
>> > + unsigned char idx;
>> > + unsigned long ea;
>> > +
>> > + idx = (ti->slb_preload_tail + i) % SLB_PRELOAD_NR;
>> > + ea = (unsigned long)ti->slb_preload_esid[idx] << SID_SHIFT;
>> > +
>> > + slb_allocate_user(mm, ea);
>> > + }
>> > +}
>> > +
>> > /* Flush all user entries from the segment table of the current processor. */
>> > void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
>> > {
>> > - struct thread_info *ti = task_thread_info(tsk);
>> > unsigned char i;
>> >
>> > /*
>> > @@ -502,29 +531,8 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
>> >
>> > copy_mm_to_paca(mm);
>> >
>> > - /*
>> > - * We gradually age out SLBs after a number of context switches to
>> > - * reduce reload overhead of unused entries (like we do with FP/VEC
>> > - * reload). Each time we wrap 256 switches, take an entry out of the
>> > - * SLB preload cache.
>> > - */
>> > - tsk->thread.load_slb++;
>> > - if (!tsk->thread.load_slb) {
>> > - unsigned long pc = KSTK_EIP(tsk);
>> > -
>> > - preload_age(ti);
>> > - preload_add(ti, pc);
>> > - }
>> > -
>> > - for (i = 0; i < ti->slb_preload_nr; i++) {
>> > - unsigned char idx;
>> > - unsigned long ea;
>> > -
>> > - idx = (ti->slb_preload_tail + i) % SLB_PRELOAD_NR;
>> > - ea = (unsigned long)ti->slb_preload_esid[idx] << SID_SHIFT;
>> > -
>> > - slb_allocate_user(mm, ea);
>> > - }
>> > + if (!mm->context.skip_slb_preload)
>> > + preload_slb_entries(tsk, mm);
>>
>> Should this be wrapped in likely()?
>
> Seems like a good idea - yes.
>
>>
>> >
>> > /*
>> > * Synchronize slbmte preloads with possible subsequent user memory
>>
>> Right below this comment is the isync. It seems to be specifically
>> concerned with synchronising preloaded slbs. Do you need it if you are
>> skipping SLB preloads?
>>
>> It's probably not a big deal to have an extra isync in the fairly rare
>> path when we're skipping preloads, but I thought I'd check.
>
> I don't _think_ we need the `isync` if we are skipping the SLB preloads,
> but then again it was always in the code-path before. If someone can
> make a compelling argument to drop it when not preloading SLBs I will,
> otherwise (considering some of the other non-obvious things I stepped
> into with the Hash code) I will keep it here for now.
The ISA says slbia wants an isync afterward, so we probably should keep
it. The comment is a bit misleading in that case.
Why isn't preloading appropriate for a temporary mm?
Thanks,
Nick
^ permalink raw reply
* Re: [RESEND PATCH v4 05/11] powerpc/64s: Add ability to skip SLB preload
From: Christopher M. Riedl @ 2021-07-01 3:48 UTC (permalink / raw)
To: Daniel Axtens, linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <87sg1bj4ex.fsf@dja-thinkpad.axtens.net>
On Sun Jun 20, 2021 at 10:13 PM CDT, Daniel Axtens wrote:
> "Christopher M. Riedl" <cmr@linux.ibm.com> writes:
>
> > Switching to a different mm with Hash translation causes SLB entries to
> > be preloaded from the current thread_info. This reduces SLB faults, for
> > example when threads share a common mm but operate on different address
> > ranges.
> >
> > Preloading entries from the thread_info struct may not always be
> > appropriate - such as when switching to a temporary mm. Introduce a new
> > boolean in mm_context_t to skip the SLB preload entirely. Also move the
> > SLB preload code into a separate function since switch_slb() is already
> > quite long. The default behavior (preloading SLB entries from the
> > current thread_info struct) remains unchanged.
> >
> > Signed-off-by: Christopher M. Riedl <cmr@linux.ibm.com>
> >
> > ---
> >
> > v4: * New to series.
> > ---
> > arch/powerpc/include/asm/book3s/64/mmu.h | 3 ++
> > arch/powerpc/include/asm/mmu_context.h | 13 ++++++
> > arch/powerpc/mm/book3s64/mmu_context.c | 2 +
> > arch/powerpc/mm/book3s64/slb.c | 56 ++++++++++++++----------
> > 4 files changed, 50 insertions(+), 24 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
> > index eace8c3f7b0a1..b23a9dcdee5af 100644
> > --- a/arch/powerpc/include/asm/book3s/64/mmu.h
> > +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
> > @@ -130,6 +130,9 @@ typedef struct {
> > u32 pkey_allocation_map;
> > s16 execute_only_pkey; /* key holding execute-only protection */
> > #endif
> > +
> > + /* Do not preload SLB entries from thread_info during switch_slb() */
> > + bool skip_slb_preload;
> > } mm_context_t;
> >
> > static inline u16 mm_ctx_user_psize(mm_context_t *ctx)
> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> > index 4bc45d3ed8b0e..264787e90b1a1 100644
> > --- a/arch/powerpc/include/asm/mmu_context.h
> > +++ b/arch/powerpc/include/asm/mmu_context.h
> > @@ -298,6 +298,19 @@ static inline int arch_dup_mmap(struct mm_struct *oldmm,
> > return 0;
> > }
> >
> > +#ifdef CONFIG_PPC_BOOK3S_64
> > +
> > +static inline void skip_slb_preload_mm(struct mm_struct *mm)
> > +{
> > + mm->context.skip_slb_preload = true;
> > +}
> > +
> > +#else
> > +
> > +static inline void skip_slb_preload_mm(struct mm_struct *mm) {}
> > +
> > +#endif /* CONFIG_PPC_BOOK3S_64 */
> > +
> > #include <asm-generic/mmu_context.h>
> >
> > #endif /* __KERNEL__ */
> > diff --git a/arch/powerpc/mm/book3s64/mmu_context.c b/arch/powerpc/mm/book3s64/mmu_context.c
> > index c10fc8a72fb37..3479910264c59 100644
> > --- a/arch/powerpc/mm/book3s64/mmu_context.c
> > +++ b/arch/powerpc/mm/book3s64/mmu_context.c
> > @@ -202,6 +202,8 @@ int init_new_context(struct task_struct *tsk, struct mm_struct *mm)
> > atomic_set(&mm->context.active_cpus, 0);
> > atomic_set(&mm->context.copros, 0);
> >
> > + mm->context.skip_slb_preload = false;
> > +
> > return 0;
> > }
> >
> > diff --git a/arch/powerpc/mm/book3s64/slb.c b/arch/powerpc/mm/book3s64/slb.c
> > index c91bd85eb90e3..da0836cb855af 100644
> > --- a/arch/powerpc/mm/book3s64/slb.c
> > +++ b/arch/powerpc/mm/book3s64/slb.c
> > @@ -441,10 +441,39 @@ static void slb_cache_slbie_user(unsigned int index)
> > asm volatile("slbie %0" : : "r" (slbie_data));
> > }
> >
> > +static void preload_slb_entries(struct task_struct *tsk, struct mm_struct *mm)
> Should this be explicitly inline or even __always_inline? I'm thinking
> switch_slb is probably a fairly hot path on hash?
Yes absolutely. I'll make this change in v5.
>
> > +{
> > + struct thread_info *ti = task_thread_info(tsk);
> > + unsigned char i;
> > +
> > + /*
> > + * We gradually age out SLBs after a number of context switches to
> > + * reduce reload overhead of unused entries (like we do with FP/VEC
> > + * reload). Each time we wrap 256 switches, take an entry out of the
> > + * SLB preload cache.
> > + */
> > + tsk->thread.load_slb++;
> > + if (!tsk->thread.load_slb) {
> > + unsigned long pc = KSTK_EIP(tsk);
> > +
> > + preload_age(ti);
> > + preload_add(ti, pc);
> > + }
> > +
> > + for (i = 0; i < ti->slb_preload_nr; i++) {
> > + unsigned char idx;
> > + unsigned long ea;
> > +
> > + idx = (ti->slb_preload_tail + i) % SLB_PRELOAD_NR;
> > + ea = (unsigned long)ti->slb_preload_esid[idx] << SID_SHIFT;
> > +
> > + slb_allocate_user(mm, ea);
> > + }
> > +}
> > +
> > /* Flush all user entries from the segment table of the current processor. */
> > void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
> > {
> > - struct thread_info *ti = task_thread_info(tsk);
> > unsigned char i;
> >
> > /*
> > @@ -502,29 +531,8 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
> >
> > copy_mm_to_paca(mm);
> >
> > - /*
> > - * We gradually age out SLBs after a number of context switches to
> > - * reduce reload overhead of unused entries (like we do with FP/VEC
> > - * reload). Each time we wrap 256 switches, take an entry out of the
> > - * SLB preload cache.
> > - */
> > - tsk->thread.load_slb++;
> > - if (!tsk->thread.load_slb) {
> > - unsigned long pc = KSTK_EIP(tsk);
> > -
> > - preload_age(ti);
> > - preload_add(ti, pc);
> > - }
> > -
> > - for (i = 0; i < ti->slb_preload_nr; i++) {
> > - unsigned char idx;
> > - unsigned long ea;
> > -
> > - idx = (ti->slb_preload_tail + i) % SLB_PRELOAD_NR;
> > - ea = (unsigned long)ti->slb_preload_esid[idx] << SID_SHIFT;
> > -
> > - slb_allocate_user(mm, ea);
> > - }
> > + if (!mm->context.skip_slb_preload)
> > + preload_slb_entries(tsk, mm);
>
> Should this be wrapped in likely()?
Seems like a good idea - yes.
>
> >
> > /*
> > * Synchronize slbmte preloads with possible subsequent user memory
>
> Right below this comment is the isync. It seems to be specifically
> concerned with synchronising preloaded slbs. Do you need it if you are
> skipping SLB preloads?
>
> It's probably not a big deal to have an extra isync in the fairly rare
> path when we're skipping preloads, but I thought I'd check.
I don't _think_ we need the `isync` if we are skipping the SLB preloads,
but then again it was always in the code-path before. If someone can
make a compelling argument to drop it when not preloading SLBs I will,
otherwise (considering some of the other non-obvious things I stepped
into with the Hash code) I will keep it here for now.
Thanks for the comments!
>
> Kind regards,
> Daniel
>
> > --
> > 2.26.1
^ permalink raw reply
* [powerpc:merge] BUILD SUCCESS 086d9878e1092e7e69a69676ee9ec792690abb1d
From: kernel test robot @ 2021-07-01 2:44 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge
branch HEAD: 086d9878e1092e7e69a69676ee9ec792690abb1d Automatic merge of 'master' into merge (2021-06-30 23:05)
elapsed time: 727m
configs tested: 101
configs skipped: 3
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
sh urquell_defconfig
arm pcm027_defconfig
arc vdk_hs38_smp_defconfig
arc nsimosci_defconfig
arm vt8500_v6_v7_defconfig
powerpc tqm8xx_defconfig
powerpc mpc866_ads_defconfig
powerpc tqm8548_defconfig
sh se7619_defconfig
ia64 allmodconfig
openrisc defconfig
powerpc katmai_defconfig
powerpc eiger_defconfig
mips e55_defconfig
powerpc pseries_defconfig
mips bmips_be_defconfig
powerpc currituck_defconfig
mips ath25_defconfig
sh j2_defconfig
ia64 tiger_defconfig
xtensa audio_kc705_defconfig
arm mv78xx0_defconfig
um defconfig
x86_64 allnoconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a002-20210630
x86_64 randconfig-a001-20210630
x86_64 randconfig-a004-20210630
x86_64 randconfig-a005-20210630
x86_64 randconfig-a006-20210630
x86_64 randconfig-a003-20210630
i386 randconfig-a004-20210630
i386 randconfig-a001-20210630
i386 randconfig-a003-20210630
i386 randconfig-a002-20210630
i386 randconfig-a005-20210630
i386 randconfig-a006-20210630
i386 randconfig-a014-20210630
i386 randconfig-a011-20210630
i386 randconfig-a016-20210630
i386 randconfig-a012-20210630
i386 randconfig-a013-20210630
i386 randconfig-a015-20210630
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel-8.3-kselftests
um x86_64_defconfig
um i386_defconfig
um kunit_defconfig
x86_64 allyesconfig
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kbuiltin
x86_64 kexec
clang tested configs:
x86_64 randconfig-b001-20210630
x86_64 randconfig-a012-20210630
x86_64 randconfig-a015-20210630
x86_64 randconfig-a016-20210630
x86_64 randconfig-a013-20210630
x86_64 randconfig-a011-20210630
x86_64 randconfig-a014-20210630
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [powerpc:next] BUILD REGRESSION 91fc46eced0f70526d74468ac6c932c90a8585b3
From: kernel test robot @ 2021-07-01 2:44 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
branch HEAD: 91fc46eced0f70526d74468ac6c932c90a8585b3 powerpc/64s: move ret_from_fork etc above __end_soft_masked
Error/Warning in current branch:
arch/powerpc/lib/restart_table.c:14:15: error: no previous prototype for function 'search_kernel_restart_table' [-Werror,-Wmissing-prototypes]
arch/powerpc/lib/restart_table.c:14:15: warning: no previous prototype for function 'search_kernel_restart_table' [-Wmissing-prototypes]
arch/powerpc/lib/restart_table.c:22:6: error: no previous prototype for function 'search_kernel_soft_mask_table' [-Werror,-Wmissing-prototypes]
arch/powerpc/lib/restart_table.c:22:6: warning: no previous prototype for function 'search_kernel_soft_mask_table' [-Wmissing-prototypes]
possible Error/Warning in current branch:
arch/powerpc/platforms/pseries/vas.c:186:13: warning: no previous prototype for 'pseries_vas_fault_thread_fn' [-Wmissing-prototypes]
Error/Warning ids grouped by kconfigs:
gcc_recent_errors
`-- powerpc64-randconfig-r034-20210630
`-- arch-powerpc-platforms-pseries-vas.c:warning:no-previous-prototype-for-pseries_vas_fault_thread_fn
clang_recent_errors
|-- powerpc-buildonly-randconfig-r003-20210630
| |-- arch-powerpc-lib-restart_table.c:error:no-previous-prototype-for-function-search_kernel_restart_table-Werror-Wmissing-prototypes
| `-- arch-powerpc-lib-restart_table.c:error:no-previous-prototype-for-function-search_kernel_soft_mask_table-Werror-Wmissing-prototypes
`-- powerpc-randconfig-r012-20210630
|-- arch-powerpc-lib-restart_table.c:warning:no-previous-prototype-for-function-search_kernel_restart_table
`-- arch-powerpc-lib-restart_table.c:warning:no-previous-prototype-for-function-search_kernel_soft_mask_table
elapsed time: 726m
configs tested: 137
configs skipped: 4
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
mips workpad_defconfig
arm at91_dt_defconfig
sh se7712_defconfig
ia64 bigsur_defconfig
mips maltasmvp_defconfig
arm aspeed_g4_defconfig
sh se7724_defconfig
arm s5pv210_defconfig
m68k m5272c3_defconfig
ia64 alldefconfig
m68k m5307c3_defconfig
sh sdk7786_defconfig
sh dreamcast_defconfig
sh edosk7705_defconfig
arm zeus_defconfig
powerpc maple_defconfig
powerpc mpc834x_mds_defconfig
arm collie_defconfig
sh se7721_defconfig
powerpc mpc8540_ads_defconfig
parisc alldefconfig
mips db1xxx_defconfig
sh r7785rp_defconfig
mips xway_defconfig
powerpc sbc8548_defconfig
arc axs103_defconfig
sh rsk7203_defconfig
mips qi_lb60_defconfig
powerpc warp_defconfig
arm h5000_defconfig
mips sb1250_swarm_defconfig
s390 allyesconfig
arm hackkit_defconfig
powerpc bluestone_defconfig
mips capcella_defconfig
arm pxa255-idp_defconfig
powerpc pseries_defconfig
mips bmips_be_defconfig
powerpc currituck_defconfig
mips ath25_defconfig
mips rt305x_defconfig
sh se7705_defconfig
powerpc fsp2_defconfig
ia64 tiger_defconfig
sh rts7751r2dplus_defconfig
powerpc mpc836x_mds_defconfig
arm pxa_defconfig
sh se7206_defconfig
mips mpc30x_defconfig
arc nsimosci_hs_defconfig
sh se7722_defconfig
powerpc canyonlands_defconfig
sh se7619_defconfig
sh sh7785lcr_defconfig
powerpc mpc885_ads_defconfig
powerpc tqm8xx_defconfig
nios2 alldefconfig
mips loongson1c_defconfig
sh se7343_defconfig
x86_64 allnoconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a002-20210630
x86_64 randconfig-a001-20210630
x86_64 randconfig-a004-20210630
x86_64 randconfig-a005-20210630
x86_64 randconfig-a003-20210630
x86_64 randconfig-a006-20210630
i386 randconfig-a004-20210630
i386 randconfig-a001-20210630
i386 randconfig-a003-20210630
i386 randconfig-a002-20210630
i386 randconfig-a005-20210630
i386 randconfig-a006-20210630
i386 randconfig-a014-20210630
i386 randconfig-a011-20210630
i386 randconfig-a016-20210630
i386 randconfig-a012-20210630
i386 randconfig-a013-20210630
i386 randconfig-a015-20210630
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel-8.3-kselftests
um x86_64_defconfig
um i386_defconfig
um kunit_defconfig
x86_64 allyesconfig
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kbuiltin
x86_64 kexec
clang tested configs:
x86_64 randconfig-b001-20210630
x86_64 randconfig-a012-20210630
x86_64 randconfig-a015-20210630
x86_64 randconfig-a016-20210630
x86_64 randconfig-a013-20210630
x86_64 randconfig-a011-20210630
x86_64 randconfig-a014-20210630
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH v3 3/9] powerpc/64e: remove implicit soft-masking and interrupt exit restart logic
From: Nicholas Piggin @ 2021-07-01 1:26 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev; +Cc: Sachin Sant
In-Reply-To: <a7eef3ac-7ea8-8bf8-f656-c88aa876f5f6@csgroup.eu>
Excerpts from Christophe Leroy's message of June 30, 2021 5:56 pm:
>
>
> Le 30/06/2021 à 09:46, Nicholas Piggin a écrit :
>> The implicit soft-masking to speed up interrupt return was going to be
>> used by 64e as well, but it has not been extensively tested on that
>> platform and is not considered ready. It was intended to be disabled
>> before merge. Disable it for now.
>>
>> Most of the restart code is common with 64s, so with more correctness
>> and performance testing this could be re-enabled again by adding the
>> extra soft-mask checks to interrupt handlers and flipping
>> exit_must_hard_disable().
>>
>> Fixes: 9d1988ca87dd ("powerpc/64: treat low kernel text as irqs soft-masked")
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/include/asm/interrupt.h | 33 ++++++++++++++++++++--------
>> arch/powerpc/kernel/exceptions-64e.S | 12 +---------
>> arch/powerpc/kernel/interrupt.c | 2 +-
>> arch/powerpc/kernel/interrupt_64.S | 16 ++++++++++++--
>> 4 files changed, 40 insertions(+), 23 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
>> index 8b4b1e84e110..f13c93b033c7 100644
>> --- a/arch/powerpc/include/asm/interrupt.h
>> +++ b/arch/powerpc/include/asm/interrupt.h
>> @@ -73,20 +73,34 @@
>> #include <asm/kprobes.h>
>> #include <asm/runlatch.h>
>>
>> -#ifdef CONFIG_PPC64
>> +#ifdef CONFIG_PPC_BOOK3S_64
>
> Can we avoid that ifdef and use IS_ENABLED(CONFIG_PPC_BOOK3S_64) below ?
Hey Christophe,
Thanks for the review, sorry it was a bit rushed to get these fixes in
before the pull. I agree with this and there's a few other cleanups we
might do as well. Something to look at next.
>
>> extern char __end_soft_masked[];
>> unsigned long search_kernel_restart_table(unsigned long addr);
>> -#endif
>>
>> -#ifdef CONFIG_PPC_BOOK3S_64
>> DECLARE_STATIC_KEY_FALSE(interrupt_exit_not_reentrant);
>>
>> +static inline bool is_implicit_soft_masked(struct pt_regs *regs)
>> +{
>> + if (regs->msr & MSR_PR)
>> + return false;
>> +
>> + if (regs->nip >= (unsigned long)__end_soft_masked)
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> static inline void srr_regs_clobbered(void)
>> {
>> local_paca->srr_valid = 0;
>> local_paca->hsrr_valid = 0;
>> }
>> #else
>> +static inline bool is_implicit_soft_masked(struct pt_regs *regs)
>> +{
>> + return false;
>> +}
>> +
>> static inline void srr_regs_clobbered(void)
>> {
>> }
>> @@ -150,11 +164,13 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
>> */
>> if (TRAP(regs) != INTERRUPT_PROGRAM) {
>> CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
>> - BUG_ON(regs->nip < (unsigned long)__end_soft_masked);
>> + BUG_ON(is_implicit_soft_masked(regs));
>> }
>> +#ifdef CONFIG_PPC_BOOK3S
>
> Allthough we are already in a PPC64 section, wouldn't it be better to use CONFIG_PPC_BOOK3S_64 ?
>
> Can we use IS_ENABLED(CONFIG_PPC_BOOK3S_64) instead ?
Good question, it's a matter of preference. I have used PPC_BOOK3S in
other places, but maybe in files shared by 32-bit it would be better to
have the _64?
On the other hand, in cases where you have an else or #else, then you
still need the PPC64 context to understand that.
I don't really have a preference, I would go with either. Making some
convention and using it everywhere is probably a good idea though.
Thanks,
Nick
^ permalink raw reply
* Re: [RFC PATCH 19/43] KVM: PPC: Book3S HV P9: Add kvmppc_stop_thread to match kvmppc_start_thread
From: Fabiano Rosas @ 2021-06-30 20:18 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210622105736.633352-20-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Small cleanup makes it a bit easier to match up entry and exit
> operations.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index b8b0695a9312..86c85e303a6d 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -2948,6 +2948,13 @@ static void kvmppc_start_thread(struct kvm_vcpu *vcpu, struct kvmppc_vcore *vc)
> kvmppc_ipi_thread(cpu);
> }
>
> +/* Old path does this in asm */
> +static void kvmppc_stop_thread(struct kvm_vcpu *vcpu)
> +{
> + vcpu->cpu = -1;
> + vcpu->arch.thread_cpu = -1;
> +}
> +
> static void kvmppc_wait_for_nap(int n_threads)
> {
> int cpu = smp_processor_id();
> @@ -4154,8 +4161,6 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
> dec = (s32) dec;
> tb = mftb();
> vcpu->arch.dec_expires = dec + tb;
> - vcpu->cpu = -1;
> - vcpu->arch.thread_cpu = -1;
>
> store_spr_state(vcpu);
>
> @@ -4627,6 +4632,8 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
>
> guest_exit_irqoff();
>
> + kvmppc_stop_thread(vcpu);
> +
> powerpc_local_irq_pmu_restore(flags);
>
> cpumask_clear_cpu(pcpu, &kvm->arch.cpu_in_guest);
^ permalink raw reply
* Re: [RFC PATCH 07/43] KVM: PPC: Book3S HV: POWER10 enable HAIL when running radix guests
From: Fabiano Rosas @ 2021-06-30 19:41 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210622105736.633352-8-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> HV interrupts may be taken with the MMU enabled when radix guests are
> running. Enable LPCR[HAIL] on ISA v3.1 processors for radix guests.
> Make this depend on the host LPCR[HAIL] being enabled. Currently that is
> always enabled, but having this test means any issue that might require
> LPCR[HAIL] to be disabled in the host will not have to be duplicated in
> KVM.
>
> -1380 cycles on P10 NULL hcall entry+exit
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 29 +++++++++++++++++++++++++----
> 1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 36e1db48fccf..ed713f49fbd5 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -4896,6 +4896,8 @@ static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu)
> */
> int kvmppc_switch_mmu_to_hpt(struct kvm *kvm)
> {
> + unsigned long lpcr, lpcr_mask;
> +
> if (nesting_enabled(kvm))
> kvmhv_release_all_nested(kvm);
> kvmppc_rmap_reset(kvm);
> @@ -4905,8 +4907,13 @@ int kvmppc_switch_mmu_to_hpt(struct kvm *kvm)
> kvm->arch.radix = 0;
> spin_unlock(&kvm->mmu_lock);
> kvmppc_free_radix(kvm);
> - kvmppc_update_lpcr(kvm, LPCR_VPM1,
> - LPCR_VPM1 | LPCR_UPRT | LPCR_GTSE | LPCR_HR);
> +
> + lpcr = LPCR_VPM1;
> + lpcr_mask = LPCR_VPM1 | LPCR_UPRT | LPCR_GTSE | LPCR_HR;
> + if (cpu_has_feature(CPU_FTR_ARCH_31))
> + lpcr_mask |= LPCR_HAIL;
> + kvmppc_update_lpcr(kvm, lpcr, lpcr_mask);
> +
> return 0;
> }
>
> @@ -4916,6 +4923,7 @@ int kvmppc_switch_mmu_to_hpt(struct kvm *kvm)
> */
> int kvmppc_switch_mmu_to_radix(struct kvm *kvm)
> {
> + unsigned long lpcr, lpcr_mask;
> int err;
>
> err = kvmppc_init_vm_radix(kvm);
> @@ -4927,8 +4935,17 @@ int kvmppc_switch_mmu_to_radix(struct kvm *kvm)
> kvm->arch.radix = 1;
> spin_unlock(&kvm->mmu_lock);
> kvmppc_free_hpt(&kvm->arch.hpt);
> - kvmppc_update_lpcr(kvm, LPCR_UPRT | LPCR_GTSE | LPCR_HR,
> - LPCR_VPM1 | LPCR_UPRT | LPCR_GTSE | LPCR_HR);
> +
> + lpcr = LPCR_UPRT | LPCR_GTSE | LPCR_HR;
> + lpcr_mask = LPCR_VPM1 | LPCR_UPRT | LPCR_GTSE | LPCR_HR;
> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
> + lpcr_mask |= LPCR_HAIL;
> + if (cpu_has_feature(CPU_FTR_HVMODE) &&
> + (kvm->arch.host_lpcr & LPCR_HAIL))
> + lpcr |= LPCR_HAIL;
> + }
> + kvmppc_update_lpcr(kvm, lpcr, lpcr_mask);
> +
> return 0;
> }
>
> @@ -5092,6 +5109,10 @@ static int kvmppc_core_init_vm_hv(struct kvm *kvm)
> kvm->arch.mmu_ready = 1;
> lpcr &= ~LPCR_VPM1;
> lpcr |= LPCR_UPRT | LPCR_GTSE | LPCR_HR;
> + if (cpu_has_feature(CPU_FTR_HVMODE) &&
> + cpu_has_feature(CPU_FTR_ARCH_31) &&
> + (kvm->arch.host_lpcr & LPCR_HAIL))
> + lpcr |= LPCR_HAIL;
> ret = kvmppc_init_vm_radix(kvm);
> if (ret) {
> kvmppc_free_lpid(kvm->arch.lpid);
^ permalink raw reply
* Re: [RFC PATCH 08/43] powerpc/64s: Keep AMOR SPR a constant ~0 at runtime
From: Fabiano Rosas @ 2021-06-30 19:17 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210622105736.633352-9-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> This register controls supervisor SPR modifications, and as such is only
> relevant for KVM. KVM always sets AMOR to ~0 on guest entry, and never
> restores it coming back out to the host, so it can be kept constant and
> avoid the mtSPR in KVM guest entry.
>
> -21 cycles (9116) cycles POWER9 virt-mode NULL hcall
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kernel/cpu_setup_power.c | 8 ++++++++
> arch/powerpc/kernel/dt_cpu_ftrs.c | 2 ++
> arch/powerpc/kvm/book3s_hv_p9_entry.c | 2 --
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 2 --
> arch/powerpc/mm/book3s64/radix_pgtable.c | 15 ---------------
> arch/powerpc/platforms/powernv/idle.c | 8 +++-----
> 6 files changed, 13 insertions(+), 24 deletions(-)
>
> diff --git a/arch/powerpc/kernel/cpu_setup_power.c b/arch/powerpc/kernel/cpu_setup_power.c
> index 3cca88ee96d7..a29dc8326622 100644
> --- a/arch/powerpc/kernel/cpu_setup_power.c
> +++ b/arch/powerpc/kernel/cpu_setup_power.c
> @@ -137,6 +137,7 @@ void __setup_cpu_power7(unsigned long offset, struct cpu_spec *t)
> return;
>
> mtspr(SPRN_LPID, 0);
> + mtspr(SPRN_AMOR, ~0);
> mtspr(SPRN_PCR, PCR_MASK);
> init_LPCR_ISA206(mfspr(SPRN_LPCR), LPCR_LPES1 >> LPCR_LPES_SH);
> }
> @@ -150,6 +151,7 @@ void __restore_cpu_power7(void)
> return;
>
> mtspr(SPRN_LPID, 0);
> + mtspr(SPRN_AMOR, ~0);
> mtspr(SPRN_PCR, PCR_MASK);
> init_LPCR_ISA206(mfspr(SPRN_LPCR), LPCR_LPES1 >> LPCR_LPES_SH);
> }
> @@ -164,6 +166,7 @@ void __setup_cpu_power8(unsigned long offset, struct cpu_spec *t)
> return;
>
> mtspr(SPRN_LPID, 0);
> + mtspr(SPRN_AMOR, ~0);
> mtspr(SPRN_PCR, PCR_MASK);
> init_LPCR_ISA206(mfspr(SPRN_LPCR) | LPCR_PECEDH, 0); /* LPES = 0 */
> init_HFSCR();
> @@ -184,6 +187,7 @@ void __restore_cpu_power8(void)
> return;
>
> mtspr(SPRN_LPID, 0);
> + mtspr(SPRN_AMOR, ~0);
> mtspr(SPRN_PCR, PCR_MASK);
> init_LPCR_ISA206(mfspr(SPRN_LPCR) | LPCR_PECEDH, 0); /* LPES = 0 */
> init_HFSCR();
> @@ -202,6 +206,7 @@ void __setup_cpu_power9(unsigned long offset, struct cpu_spec *t)
> mtspr(SPRN_PSSCR, 0);
> mtspr(SPRN_LPID, 0);
> mtspr(SPRN_PID, 0);
> + mtspr(SPRN_AMOR, ~0);
> mtspr(SPRN_PCR, PCR_MASK);
> init_LPCR_ISA300((mfspr(SPRN_LPCR) | LPCR_PECEDH | LPCR_PECE_HVEE |\
> LPCR_HVICE | LPCR_HEIC) & ~(LPCR_UPRT | LPCR_HR), 0);
> @@ -223,6 +228,7 @@ void __restore_cpu_power9(void)
> mtspr(SPRN_PSSCR, 0);
> mtspr(SPRN_LPID, 0);
> mtspr(SPRN_PID, 0);
> + mtspr(SPRN_AMOR, ~0);
> mtspr(SPRN_PCR, PCR_MASK);
> init_LPCR_ISA300((mfspr(SPRN_LPCR) | LPCR_PECEDH | LPCR_PECE_HVEE |\
> LPCR_HVICE | LPCR_HEIC) & ~(LPCR_UPRT | LPCR_HR), 0);
> @@ -242,6 +248,7 @@ void __setup_cpu_power10(unsigned long offset, struct cpu_spec *t)
> mtspr(SPRN_PSSCR, 0);
> mtspr(SPRN_LPID, 0);
> mtspr(SPRN_PID, 0);
> + mtspr(SPRN_AMOR, ~0);
> mtspr(SPRN_PCR, PCR_MASK);
> init_LPCR_ISA300((mfspr(SPRN_LPCR) | LPCR_PECEDH | LPCR_PECE_HVEE |\
> LPCR_HVICE | LPCR_HEIC) & ~(LPCR_UPRT | LPCR_HR), 0);
> @@ -264,6 +271,7 @@ void __restore_cpu_power10(void)
> mtspr(SPRN_PSSCR, 0);
> mtspr(SPRN_LPID, 0);
> mtspr(SPRN_PID, 0);
> + mtspr(SPRN_AMOR, ~0);
> mtspr(SPRN_PCR, PCR_MASK);
> init_LPCR_ISA300((mfspr(SPRN_LPCR) | LPCR_PECEDH | LPCR_PECE_HVEE |\
> LPCR_HVICE | LPCR_HEIC) & ~(LPCR_UPRT | LPCR_HR), 0);
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index 358aee7c2d79..0a6b36b4bda8 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -80,6 +80,7 @@ static void __restore_cpu_cpufeatures(void)
> mtspr(SPRN_LPCR, system_registers.lpcr);
> if (hv_mode) {
> mtspr(SPRN_LPID, 0);
> + mtspr(SPRN_AMOR, ~0);
> mtspr(SPRN_HFSCR, system_registers.hfscr);
> mtspr(SPRN_PCR, system_registers.pcr);
> }
> @@ -216,6 +217,7 @@ static int __init feat_enable_hv(struct dt_cpu_feature *f)
> }
>
> mtspr(SPRN_LPID, 0);
> + mtspr(SPRN_AMOR, ~0);
>
> lpcr = mfspr(SPRN_LPCR);
> lpcr &= ~LPCR_LPES0; /* HV external interrupts */
> diff --git a/arch/powerpc/kvm/book3s_hv_p9_entry.c b/arch/powerpc/kvm/book3s_hv_p9_entry.c
> index c4f3e066fcb4..a3281f0c9214 100644
> --- a/arch/powerpc/kvm/book3s_hv_p9_entry.c
> +++ b/arch/powerpc/kvm/book3s_hv_p9_entry.c
> @@ -286,8 +286,6 @@ int kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu, u64 time_limit, unsigned long lpc
> mtspr(SPRN_SPRG2, vcpu->arch.shregs.sprg2);
> mtspr(SPRN_SPRG3, vcpu->arch.shregs.sprg3);
>
> - mtspr(SPRN_AMOR, ~0UL);
> -
> local_paca->kvm_hstate.in_guest = KVM_GUEST_MODE_HV_P9;
>
> /*
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 8dd437d7a2c6..007f87b97184 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -772,10 +772,8 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
> /* Restore AMR and UAMOR, set AMOR to all 1s */
> ld r5,VCPU_AMR(r4)
> ld r6,VCPU_UAMOR(r4)
> - li r7,-1
> mtspr SPRN_AMR,r5
> mtspr SPRN_UAMOR,r6
> - mtspr SPRN_AMOR,r7
>
> /* Restore state of CTRL run bit; assume 1 on entry */
> lwz r5,VCPU_CTRL(r4)
> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
> index fe236c38ce00..b985cfead5d7 100644
> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
> @@ -566,18 +566,6 @@ void __init radix__early_init_devtree(void)
> return;
> }
>
> -static void radix_init_amor(void)
> -{
> - /*
> - * In HV mode, we init AMOR (Authority Mask Override Register) so that
> - * the hypervisor and guest can setup IAMR (Instruction Authority Mask
> - * Register), enable key 0 and set it to 1.
> - *
> - * AMOR = 0b1100 .... 0000 (Mask for key 0 is 11)
> - */
> - mtspr(SPRN_AMOR, (3ul << 62));
> -}
> -
> void __init radix__early_init_mmu(void)
> {
> unsigned long lpcr;
> @@ -638,7 +626,6 @@ void __init radix__early_init_mmu(void)
> lpcr = mfspr(SPRN_LPCR);
> mtspr(SPRN_LPCR, lpcr | LPCR_UPRT | LPCR_HR);
> radix_init_partition_table();
> - radix_init_amor();
> } else {
> radix_init_pseries();
> }
> @@ -662,8 +649,6 @@ void radix__early_init_mmu_secondary(void)
>
> set_ptcr_when_no_uv(__pa(partition_tb) |
> (PATB_SIZE_SHIFT - 12));
> -
> - radix_init_amor();
> }
>
> radix__switch_mmu_context(NULL, &init_mm);
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 180baecad914..f791ca041854 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -306,8 +306,8 @@ struct p7_sprs {
> /* per thread SPRs that get lost in shallow states */
> u64 amr;
> u64 iamr;
> - u64 amor;
> u64 uamor;
> + /* amor is restored to constant ~0 */
> };
>
> static unsigned long power7_idle_insn(unsigned long type)
> @@ -378,7 +378,6 @@ static unsigned long power7_idle_insn(unsigned long type)
> if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
> sprs.amr = mfspr(SPRN_AMR);
> sprs.iamr = mfspr(SPRN_IAMR);
> - sprs.amor = mfspr(SPRN_AMOR);
> sprs.uamor = mfspr(SPRN_UAMOR);
> }
>
> @@ -397,7 +396,7 @@ static unsigned long power7_idle_insn(unsigned long type)
> */
> mtspr(SPRN_AMR, sprs.amr);
> mtspr(SPRN_IAMR, sprs.iamr);
> - mtspr(SPRN_AMOR, sprs.amor);
> + mtspr(SPRN_AMOR, ~0);
> mtspr(SPRN_UAMOR, sprs.uamor);
> }
> }
> @@ -687,7 +686,6 @@ static unsigned long power9_idle_stop(unsigned long psscr)
>
> sprs.amr = mfspr(SPRN_AMR);
> sprs.iamr = mfspr(SPRN_IAMR);
> - sprs.amor = mfspr(SPRN_AMOR);
> sprs.uamor = mfspr(SPRN_UAMOR);
>
> srr1 = isa300_idle_stop_mayloss(psscr); /* go idle */
> @@ -708,7 +706,7 @@ static unsigned long power9_idle_stop(unsigned long psscr)
> */
> mtspr(SPRN_AMR, sprs.amr);
> mtspr(SPRN_IAMR, sprs.iamr);
> - mtspr(SPRN_AMOR, sprs.amor);
> + mtspr(SPRN_AMOR, ~0);
> mtspr(SPRN_UAMOR, sprs.uamor);
>
> /*
^ permalink raw reply
* Re: [RFC PATCH 38/43] KVM: PPC: Book3S HV P9: Test dawr_enabled() before saving host DAWR SPRs
From: Fabiano Rosas @ 2021-06-30 17:51 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210622105736.633352-39-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Some of the DAWR SPR access is already predicated on dawr_enabled(),
> apply this to the remainder of the accesses.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kvm/book3s_hv_p9_entry.c | 34 ++++++++++++++++-----------
> 1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_p9_entry.c b/arch/powerpc/kvm/book3s_hv_p9_entry.c
> index 7aa72efcac6c..f305d1d6445c 100644
> --- a/arch/powerpc/kvm/book3s_hv_p9_entry.c
> +++ b/arch/powerpc/kvm/book3s_hv_p9_entry.c
> @@ -638,13 +638,16 @@ int kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu, u64 time_limit, unsigned long lpc
>
> host_hfscr = mfspr(SPRN_HFSCR);
> host_ciabr = mfspr(SPRN_CIABR);
> - host_dawr0 = mfspr(SPRN_DAWR0);
> - host_dawrx0 = mfspr(SPRN_DAWRX0);
> host_psscr = mfspr(SPRN_PSSCR);
> host_pidr = mfspr(SPRN_PID);
> - if (cpu_has_feature(CPU_FTR_DAWR1)) {
> - host_dawr1 = mfspr(SPRN_DAWR1);
> - host_dawrx1 = mfspr(SPRN_DAWRX1);
> +
> + if (dawr_enabled()) {
> + host_dawr0 = mfspr(SPRN_DAWR0);
> + host_dawrx0 = mfspr(SPRN_DAWRX0);
> + if (cpu_has_feature(CPU_FTR_DAWR1)) {
> + host_dawr1 = mfspr(SPRN_DAWR1);
> + host_dawrx1 = mfspr(SPRN_DAWRX1);
The userspace needs to enable DAWR1 via KVM_CAP_PPC_DAWR1. That cap is
not even implemented in QEMU currently, so we never allow the guest to
set vcpu->arch.dawr1. If we check for kvm->arch.dawr1_enabled instead of
the CPU feature, we could shave some more time here.
> + }
> }
>
> local_paca->kvm_hstate.host_purr = mfspr(SPRN_PURR);
> @@ -951,15 +954,18 @@ int kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu, u64 time_limit, unsigned long lpc
> mtspr(SPRN_HFSCR, host_hfscr);
> if (vcpu->arch.ciabr != host_ciabr)
> mtspr(SPRN_CIABR, host_ciabr);
> - if (vcpu->arch.dawr0 != host_dawr0)
> - mtspr(SPRN_DAWR0, host_dawr0);
> - if (vcpu->arch.dawrx0 != host_dawrx0)
> - mtspr(SPRN_DAWRX0, host_dawrx0);
> - if (cpu_has_feature(CPU_FTR_DAWR1)) {
> - if (vcpu->arch.dawr1 != host_dawr1)
> - mtspr(SPRN_DAWR1, host_dawr1);
> - if (vcpu->arch.dawrx1 != host_dawrx1)
> - mtspr(SPRN_DAWRX1, host_dawrx1);
> +
> + if (dawr_enabled()) {
> + if (vcpu->arch.dawr0 != host_dawr0)
> + mtspr(SPRN_DAWR0, host_dawr0);
> + if (vcpu->arch.dawrx0 != host_dawrx0)
> + mtspr(SPRN_DAWRX0, host_dawrx0);
> + if (cpu_has_feature(CPU_FTR_DAWR1)) {
> + if (vcpu->arch.dawr1 != host_dawr1)
> + mtspr(SPRN_DAWR1, host_dawr1);
> + if (vcpu->arch.dawrx1 != host_dawrx1)
> + mtspr(SPRN_DAWRX1, host_dawrx1);
> + }
> }
>
> if (vc->dpdes)
^ permalink raw reply
* Re: [RFC PATCH 01/43] powerpc/64s: Remove WORT SPR from POWER9/10
From: Fabiano Rosas @ 2021-06-30 17:29 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210622105736.633352-2-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> This register is not architected and not implemented in POWER9 or 10,
> it just reads back zeroes for compatibility.
>
> -78 cycles (9255) cycles POWER9 virt-mode NULL hcall
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 3 ---
> arch/powerpc/platforms/powernv/idle.c | 2 --
> 2 files changed, 5 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 9228042bd54f..97f3d6d54b61 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3640,7 +3640,6 @@ static void load_spr_state(struct kvm_vcpu *vcpu)
> mtspr(SPRN_EBBHR, vcpu->arch.ebbhr);
> mtspr(SPRN_EBBRR, vcpu->arch.ebbrr);
> mtspr(SPRN_BESCR, vcpu->arch.bescr);
> - mtspr(SPRN_WORT, vcpu->arch.wort);
> mtspr(SPRN_TIDR, vcpu->arch.tid);
> mtspr(SPRN_AMR, vcpu->arch.amr);
> mtspr(SPRN_UAMOR, vcpu->arch.uamor);
> @@ -3667,7 +3666,6 @@ static void store_spr_state(struct kvm_vcpu *vcpu)
> vcpu->arch.ebbhr = mfspr(SPRN_EBBHR);
> vcpu->arch.ebbrr = mfspr(SPRN_EBBRR);
> vcpu->arch.bescr = mfspr(SPRN_BESCR);
> - vcpu->arch.wort = mfspr(SPRN_WORT);
> vcpu->arch.tid = mfspr(SPRN_TIDR);
> vcpu->arch.amr = mfspr(SPRN_AMR);
> vcpu->arch.uamor = mfspr(SPRN_UAMOR);
> @@ -3699,7 +3697,6 @@ static void restore_p9_host_os_sprs(struct kvm_vcpu *vcpu,
> struct p9_host_os_sprs *host_os_sprs)
> {
> mtspr(SPRN_PSPB, 0);
> - mtspr(SPRN_WORT, 0);
> mtspr(SPRN_UAMOR, 0);
>
> mtspr(SPRN_DSCR, host_os_sprs->dscr);
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 528a7e0cf83a..180baecad914 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -667,7 +667,6 @@ static unsigned long power9_idle_stop(unsigned long psscr)
> sprs.purr = mfspr(SPRN_PURR);
> sprs.spurr = mfspr(SPRN_SPURR);
> sprs.dscr = mfspr(SPRN_DSCR);
> - sprs.wort = mfspr(SPRN_WORT);
> sprs.ciabr = mfspr(SPRN_CIABR);
>
> sprs.mmcra = mfspr(SPRN_MMCRA);
> @@ -785,7 +784,6 @@ static unsigned long power9_idle_stop(unsigned long psscr)
> mtspr(SPRN_PURR, sprs.purr);
> mtspr(SPRN_SPURR, sprs.spurr);
> mtspr(SPRN_DSCR, sprs.dscr);
> - mtspr(SPRN_WORT, sprs.wort);
> mtspr(SPRN_CIABR, sprs.ciabr);
>
> mtspr(SPRN_MMCRA, sprs.mmcra);
^ permalink raw reply
* Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
From: Nathan Chancellor @ 2021-06-30 15:56 UTC (permalink / raw)
To: Will Deacon
Cc: heikki.krogerus, thomas.hellstrom, peterz, joonas.lahtinen,
dri-devel, chris, grant.likely, paulus, Frank Rowand, mingo,
Marek Szyprowski, Stefano Stabellini, Saravana Kannan,
Joerg Roedel, Rafael J . Wysocki, Christoph Hellwig,
Bartosz Golaszewski, bskeggs, linux-pci, xen-devel,
Thierry Reding, intel-gfx, matthew.auld, linux-devicetree,
Jianxiong Gao, Daniel Vetter, Konrad Rzeszutek Wilk,
maarten.lankhorst, airlied, Dan Williams, linuxppc-dev,
jani.nikula, Rob Herring, rodrigo.vivi, Bjorn Helgaas,
Claire Chang, boris.ostrovsky, Andy Shevchenko, jgross,
Nicolas Boichat, Greg KH, Randy Dunlap, Qian Cai, lkml,
Tomasz Figa, list@263.net:IOMMU DRIVERS, Jim Quinlan, xypron.glpk,
Tom Lendacky, Robin Murphy, bauerman
In-Reply-To: <20210630114348.GA8383@willie-the-truck>
Hi Will and Claire,
On Wed, Jun 30, 2021 at 12:43:48PM +0100, Will Deacon wrote:
> On Wed, Jun 30, 2021 at 05:17:27PM +0800, Claire Chang wrote:
> > On Wed, Jun 30, 2021 at 9:43 AM Nathan Chancellor <nathan@kernel.org> wrote:
> > >
> > > On Thu, Jun 24, 2021 at 11:55:20PM +0800, Claire Chang wrote:
> > > > Propagate the swiotlb_force into io_tlb_default_mem->force_bounce and
> > > > use it to determine whether to bounce the data or not. This will be
> > > > useful later to allow for different pools.
> > > >
> > > > Signed-off-by: Claire Chang <tientzu@chromium.org>
> > > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > > Tested-by: Stefano Stabellini <sstabellini@kernel.org>
> > > > Tested-by: Will Deacon <will@kernel.org>
> > > > Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> > >
> > > This patch as commit af452ec1b1a3 ("swiotlb: Use is_swiotlb_force_bounce
> > > for swiotlb data bouncing") causes my Ryzen 3 4300G system to fail to
> > > get to an X session consistently (although not every single time),
> > > presumably due to a crash in the AMDGPU driver that I see in dmesg.
> > >
> > > I have attached logs at af452ec1b1a3 and f127c9556a8e and I am happy
> > > to provide any further information, debug, or test patches as necessary.
> >
> > Are you using swiotlb=force? or the swiotlb_map is called because of
> > !dma_capable? (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/kernel/dma/direct.h#n93)
>
> The command line is in the dmesg:
>
> | Kernel command line: initrd=\amd-ucode.img initrd=\initramfs-linux-next-llvm.img root=PARTUUID=8680aa0c-cf09-4a69-8cf3-970478040ee7 rw intel_pstate=no_hwp irqpoll
>
> but I worry that this looks _very_ similar to the issue reported by Qian
> Cai which we thought we had fixed. Nathan -- is the failure deterministic?
Yes, for the most part. It does not happen every single boot so when I
was bisecting, I did a series of seven boots and only considered the
revision good when all seven of them made it to LightDM's greeter. My
results that I notated show most bad revisions failed anywhere from four
to six times.
> > `BUG: unable to handle page fault for address: 00000000003a8290` and
> > the fact it crashed at `_raw_spin_lock_irqsave` look like the memory
> > (maybe dev->dma_io_tlb_mem) was corrupted?
> > The dev->dma_io_tlb_mem should be set here
> > (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/pci/probe.c#n2528)
> > through device_initialize.
>
> I'm less sure about this. 'dma_io_tlb_mem' should be pointing at
> 'io_tlb_default_mem', which is a page-aligned allocation from memblock.
> The spinlock is at offset 0x24 in that structure, and looking at the
> register dump from the crash:
>
> Jun 29 18:28:42 hp-4300G kernel: RSP: 0018:ffffadb4013db9e8 EFLAGS: 00010006
> Jun 29 18:28:42 hp-4300G kernel: RAX: 00000000003a8290 RBX: 0000000000000000 RCX: ffff8900572ad580
> Jun 29 18:28:42 hp-4300G kernel: RDX: ffff89005653f024 RSI: 00000000000c0000 RDI: 0000000000001d17
> Jun 29 18:28:42 hp-4300G kernel: RBP: 000000000a20d000 R08: 00000000000c0000 R09: 0000000000000000
> Jun 29 18:28:42 hp-4300G kernel: R10: 000000000a20d000 R11: ffff89005653f000 R12: 0000000000000212
> Jun 29 18:28:42 hp-4300G kernel: R13: 0000000000001000 R14: 0000000000000002 R15: 0000000000200000
> Jun 29 18:28:42 hp-4300G kernel: FS: 00007f1f8898ea40(0000) GS:ffff890057280000(0000) knlGS:0000000000000000
> Jun 29 18:28:42 hp-4300G kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> Jun 29 18:28:42 hp-4300G kernel: CR2: 00000000003a8290 CR3: 00000001020d0000 CR4: 0000000000350ee0
> Jun 29 18:28:42 hp-4300G kernel: Call Trace:
> Jun 29 18:28:42 hp-4300G kernel: _raw_spin_lock_irqsave+0x39/0x50
> Jun 29 18:28:42 hp-4300G kernel: swiotlb_tbl_map_single+0x12b/0x4c0
>
> Then that correlates with R11 holding the 'dma_io_tlb_mem' pointer and
> RDX pointing at the spinlock. Yet RAX is holding junk :/
>
> I agree that enabling KASAN would be a good idea, but I also think we
> probably need to get some more information out of swiotlb_tbl_map_single()
> to see see what exactly is going wrong in there.
I can certainly enable KASAN and if there is any debug print I can add
or dump anything, let me know!
Cheers,
Nathan
^ permalink raw reply
* Re: [PATCH] powerpc/32s: Fix setup_{kuap/kuep}() on SMP
From: Michael Ellerman @ 2021-06-30 13:14 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Christophe Leroy
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <42f4bd12b476942e4d5dc81c0e839d8871b20b1c.1624863319.git.christophe.leroy@csgroup.eu>
On Mon, 28 Jun 2021 06:56:11 +0000 (UTC), Christophe Leroy wrote:
> On SMP, setup_kup() is also called from start_secondary().
>
> start_secondary() is not an __init function.
>
> Remove the __init marker from setup_kuep() and and setup_kuap().
Applied to powerpc/next.
[1/1] powerpc/32s: Fix setup_{kuap/kuep}() on SMP
https://git.kernel.org/powerpc/c/c89e632658e793fbbdcbfbe80a6c13bbf7203e9b
cheers
^ permalink raw reply
* Re: [PATCH v3 0/9] powerpc: fast interrupt exit bug and misc fixes
From: Michael Ellerman @ 2021-06-30 13:14 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Sachin Sant
In-Reply-To: <20210630074621.2109197-1-npiggin@gmail.com>
On Wed, 30 Jun 2021 17:46:12 +1000, Nicholas Piggin wrote:
> This is a bunch of fixes for powerpc next, mostly a nasty hole in fast
> interrupt exit code found by Sachin and some other bits along the way
> while looking at it.
>
> Since v2:
> - Fixed 64e patch 3 to really set exit_must_hard_disable.
> - Reworded some changelogs.
>
> [...]
Applied to powerpc/next.
[1/9] powerpc/64s: fix hash page fault interrupt handler
https://git.kernel.org/powerpc/c/5567b1ee29b7a83e8c01d99d34b5bbd306ce0bcf
[2/9] powerpc/64e: fix CONFIG_RELOCATABLE build warnings
https://git.kernel.org/powerpc/c/fce01acf830a697110ed72ecace4b0afdbcd53cb
[3/9] powerpc/64e: remove implicit soft-masking and interrupt exit restart logic
https://git.kernel.org/powerpc/c/9b69d48c7516a29cdaacd18d8bf5f575014a42a1
[4/9] powerpc/64s: add a table of implicit soft-masked addresses
https://git.kernel.org/powerpc/c/325678fd052259e7c05ef29060a73c705ea90432
[5/9] powerpc/64s/interrupt: preserve regs->softe for NMI interrupts
https://git.kernel.org/powerpc/c/1b0482229c302a3c6afd00d6b3bf0169cf279b44
[6/9] powerpc/64: enable MSR[EE] in irq replay pt_regs
https://git.kernel.org/powerpc/c/2b43dd7653cca47d297756980846ebbfe8887fa1
[7/9] powerpc/64/interrupt: add missing kprobe annotations on interrupt exit symbols
https://git.kernel.org/powerpc/c/98798f33c6be5a511ab61958b40835b3ef08def2
[8/9] powerpc/64s/interrupt: clean up interrupt return labels
https://git.kernel.org/powerpc/c/c59458b00aec4ba580d9628d36d6c984af94d192
[9/9] powerpc/64s: move ret_from_fork etc above __end_soft_masked
https://git.kernel.org/powerpc/c/91fc46eced0f70526d74468ac6c932c90a8585b3
cheers
^ permalink raw reply
* Re: [PATCH v2] powerpc/4xx: Fix setup_kuep() on SMP
From: Michael Ellerman @ 2021-06-30 13:14 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Christophe Leroy
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <8ee05934288994a65743a987acb1558f12c0c8c1.1624969450.git.christophe.leroy@csgroup.eu>
On Tue, 29 Jun 2021 12:24:21 +0000 (UTC), Christophe Leroy wrote:
> On SMP, setup_kuep() is also called from start_secondary() since
> commit 86f46f343272 ("powerpc/32s: Initialise KUAP and KUEP in C").
>
> start_secondary() is not an __init function.
>
> Remove the __init marker from setup_kuep() and bail out when
> not caller on the first CPU as the work is already done.
Applied to powerpc/next.
[1/1] powerpc/4xx: Fix setup_kuep() on SMP
https://git.kernel.org/powerpc/c/fc4999864bca323f1b844fefe1b402632443c076
cheers
^ permalink raw reply
* Re: [PATCH] crypto: DRBG - select SHA512
From: Sachin Sant @ 2021-06-30 12:13 UTC (permalink / raw)
To: Stephan Mueller; +Cc: linuxppc-dev, linux-crypto
In-Reply-To: <304ee0376383d9ceecddbfd216c035215bbff861.camel@chronox.de>
> On 30-Jun-2021, at 4:02 PM, Stephan Mueller <smueller@chronox.de> wrote:
>
> With the swtich to use HMAC(SHA-512) as the default DRBG type, the
> configuration must now also select SHA-512.
>
> Fixes: 9b7b94683a9b "crypto: DRBG - switch to HMAC SHA512 DRBG as default
> DRBG"
> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Signed-off-by: Stephan Mueller <smueller@chronox.com>
> ---
Thanks Stephan. This patch fixes the reported problem.
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
-Sachin
^ permalink raw reply
* [PATCH] powerpc/papr_scm: Move duplicate definitions to common header files
From: Shivaprasad G Bhat @ 2021-06-30 12:08 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, ellerman
Cc: nvdimm, santosh, vaibhav, dan.j.williams, aneesh.kumar
The papr_scm uses PDSM structures like nd_papr_pdsm_health as the
PDSM payload which would be reused by ndtest. Since ndtest is arch
independent, move the PDSM header from arch/powerpc/include/uapi/
to the generic include/uapi/linux directory.
Also, there are some #defines common between papr_scm and ndtest
which are not exported to the user space. So, move them to a header
file which can be shared across ndtest and papr_scm via newly
introduced include/linux/papr_scm.h.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
Suggested-by: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
---
MAINTAINERS | 2
arch/powerpc/include/uapi/asm/papr_pdsm.h | 147 -----------------------------
arch/powerpc/platforms/pseries/papr_scm.c | 43 --------
include/linux/papr_scm.h | 48 +++++++++
include/uapi/linux/papr_pdsm.h | 147 +++++++++++++++++++++++++++++
tools/testing/nvdimm/test/ndtest.c | 1
tools/testing/nvdimm/test/ndtest.h | 31 ------
7 files changed, 200 insertions(+), 219 deletions(-)
delete mode 100644 arch/powerpc/include/uapi/asm/papr_pdsm.h
create mode 100644 include/linux/papr_scm.h
create mode 100644 include/uapi/linux/papr_pdsm.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 008fcad7ac008..5d2d8a6b5eb53 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10494,6 +10494,8 @@ F: drivers/rtc/rtc-opal.c
F: drivers/scsi/ibmvscsi/
F: drivers/tty/hvc/hvc_opal.c
F: drivers/watchdog/wdrtas.c
+F: include/linux/papr_scm.h
+F: include/uapi/linux/papr_pdsm.h
F: tools/testing/selftests/powerpc
N: /pmac
N: powermac
diff --git a/arch/powerpc/include/uapi/asm/papr_pdsm.h b/arch/powerpc/include/uapi/asm/papr_pdsm.h
deleted file mode 100644
index 82488b1e7276e..0000000000000
--- a/arch/powerpc/include/uapi/asm/papr_pdsm.h
+++ /dev/null
@@ -1,147 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-/*
- * PAPR nvDimm Specific Methods (PDSM) and structs for libndctl
- *
- * (C) Copyright IBM 2020
- *
- * Author: Vaibhav Jain <vaibhav at linux.ibm.com>
- */
-
-#ifndef _UAPI_ASM_POWERPC_PAPR_PDSM_H_
-#define _UAPI_ASM_POWERPC_PAPR_PDSM_H_
-
-#include <linux/types.h>
-#include <linux/ndctl.h>
-
-/*
- * PDSM Envelope:
- *
- * The ioctl ND_CMD_CALL exchange data between user-space and kernel via
- * envelope which consists of 2 headers sections and payload sections as
- * illustrated below:
- * +-----------------+---------------+---------------------------+
- * | 64-Bytes | 8-Bytes | Max 184-Bytes |
- * +-----------------+---------------+---------------------------+
- * | ND-HEADER | PDSM-HEADER | PDSM-PAYLOAD |
- * +-----------------+---------------+---------------------------+
- * | nd_family | | |
- * | nd_size_out | cmd_status | |
- * | nd_size_in | reserved | nd_pdsm_payload |
- * | nd_command | payload --> | |
- * | nd_fw_size | | |
- * | nd_payload ---> | | |
- * +---------------+-----------------+---------------------------+
- *
- * ND Header:
- * This is the generic libnvdimm header described as 'struct nd_cmd_pkg'
- * which is interpreted by libnvdimm before passed on to papr_scm. Important
- * member fields used are:
- * 'nd_family' : (In) NVDIMM_FAMILY_PAPR_SCM
- * 'nd_size_in' : (In) PDSM-HEADER + PDSM-IN-PAYLOAD (usually 0)
- * 'nd_size_out' : (In) PDSM-HEADER + PDSM-RETURN-PAYLOAD
- * 'nd_command' : (In) One of PAPR_PDSM_XXX
- * 'nd_fw_size' : (Out) PDSM-HEADER + size of actual payload returned
- *
- * PDSM Header:
- * This is papr-scm specific header that precedes the payload. This is defined
- * as nd_cmd_pdsm_pkg. Following fields aare available in this header:
- *
- * 'cmd_status' : (Out) Errors if any encountered while servicing PDSM.
- * 'reserved' : Not used, reserved for future and should be set to 0.
- * 'payload' : A union of all the possible payload structs
- *
- * PDSM Payload:
- *
- * The layout of the PDSM Payload is defined by various structs shared between
- * papr_scm and libndctl so that contents of payload can be interpreted. As such
- * its defined as a union of all possible payload structs as
- * 'union nd_pdsm_payload'. Based on the value of 'nd_cmd_pkg.nd_command'
- * appropriate member of the union is accessed.
- */
-
-/* Max payload size that we can handle */
-#define ND_PDSM_PAYLOAD_MAX_SIZE 184
-
-/* Max payload size that we can handle */
-#define ND_PDSM_HDR_SIZE \
- (sizeof(struct nd_pkg_pdsm) - ND_PDSM_PAYLOAD_MAX_SIZE)
-
-/* Various nvdimm health indicators */
-#define PAPR_PDSM_DIMM_HEALTHY 0
-#define PAPR_PDSM_DIMM_UNHEALTHY 1
-#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
-
-/* Indicate that the 'dimm_dsc' field is valid */
-#define PDSM_DIMM_DSC_VALID 2
-
-/*
- * Struct exchanged between kernel & ndctl in for PAPR_PDSM_HEALTH
- * Various flags indicate the health status of the dimm.
- *
- * extension_flags : Any extension fields present in the struct.
- * dimm_unarmed : Dimm not armed. So contents wont persist.
- * dimm_bad_shutdown : Previous shutdown did not persist contents.
- * dimm_bad_restore : Contents from previous shutdown werent restored.
- * dimm_scrubbed : Contents of the dimm have been scrubbed.
- * 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 {
- struct {
- __u32 extension_flags;
- __u8 dimm_unarmed;
- __u8 dimm_bad_shutdown;
- __u8 dimm_bad_restore;
- __u8 dimm_scrubbed;
- __u8 dimm_locked;
- __u8 dimm_encrypted;
- __u16 dimm_health;
-
- /* Extension flag PDSM_DIMM_HEALTH_RUN_GAUGE_VALID */
- __u16 dimm_fuel_gauge;
-
- /* Extension flag PDSM_DIMM_DSC_VALID */
- __u64 dimm_dsc;
- };
- __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
- };
-};
-
-/*
- * Methods to be embedded in ND_CMD_CALL request. These are sent to the kernel
- * via 'nd_cmd_pkg.nd_command' member of the ioctl struct
- */
-enum papr_pdsm {
- PAPR_PDSM_MIN = 0x0,
- PAPR_PDSM_HEALTH,
- PAPR_PDSM_MAX,
-};
-
-/* Maximal union that can hold all possible payload types */
-union nd_pdsm_payload {
- struct nd_papr_pdsm_health health;
- __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
-} __packed;
-
-/*
- * PDSM-header + payload expected with ND_CMD_CALL ioctl from libnvdimm
- * Valid member of union 'payload' is identified via 'nd_cmd_pkg.nd_command'
- * that should always precede this struct when sent to papr_scm via CMD_CALL
- * interface.
- */
-struct nd_pkg_pdsm {
- __s32 cmd_status; /* Out: Sub-cmd status returned back */
- __u16 reserved[2]; /* Ignored and to be set as '0' */
- union nd_pdsm_payload payload;
-} __packed;
-
-#endif /* _UAPI_ASM_POWERPC_PAPR_PDSM_H_ */
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index f48e87ac89c9b..0c56db5a14276 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -16,7 +16,8 @@
#include <linux/nd.h>
#include <asm/plpar_wrappers.h>
-#include <asm/papr_pdsm.h>
+#include <uapi/linux/papr_pdsm.h>
+#include <linux/papr_scm.h>
#include <asm/mce.h>
#include <asm/unaligned.h>
@@ -28,46 +29,6 @@
(1ul << ND_CMD_SET_CONFIG_DATA) | \
(1ul << ND_CMD_CALL))
-/* DIMM health bitmap bitmap indicators */
-/* SCM device is unable to persist memory contents */
-#define PAPR_PMEM_UNARMED (1ULL << (63 - 0))
-/* SCM device failed to persist memory contents */
-#define PAPR_PMEM_SHUTDOWN_DIRTY (1ULL << (63 - 1))
-/* SCM device contents are persisted from previous IPL */
-#define PAPR_PMEM_SHUTDOWN_CLEAN (1ULL << (63 - 2))
-/* SCM device contents are not persisted from previous IPL */
-#define PAPR_PMEM_EMPTY (1ULL << (63 - 3))
-/* SCM device memory life remaining is critically low */
-#define PAPR_PMEM_HEALTH_CRITICAL (1ULL << (63 - 4))
-/* SCM device will be garded off next IPL due to failure */
-#define PAPR_PMEM_HEALTH_FATAL (1ULL << (63 - 5))
-/* SCM contents cannot persist due to current platform health status */
-#define PAPR_PMEM_HEALTH_UNHEALTHY (1ULL << (63 - 6))
-/* SCM device is unable to persist memory contents in certain conditions */
-#define PAPR_PMEM_HEALTH_NON_CRITICAL (1ULL << (63 - 7))
-/* SCM device is encrypted */
-#define PAPR_PMEM_ENCRYPTED (1ULL << (63 - 8))
-/* SCM device has been scrubbed and locked */
-#define PAPR_PMEM_SCRUBBED_AND_LOCKED (1ULL << (63 - 9))
-
-/* Bits status indicators for health bitmap indicating unarmed dimm */
-#define PAPR_PMEM_UNARMED_MASK (PAPR_PMEM_UNARMED | \
- PAPR_PMEM_HEALTH_UNHEALTHY)
-
-/* Bits status indicators for health bitmap indicating unflushed dimm */
-#define PAPR_PMEM_BAD_SHUTDOWN_MASK (PAPR_PMEM_SHUTDOWN_DIRTY)
-
-/* Bits status indicators for health bitmap indicating unrestored dimm */
-#define PAPR_PMEM_BAD_RESTORE_MASK (PAPR_PMEM_EMPTY)
-
-/* Bit status indicators for smart event notification */
-#define PAPR_PMEM_SMART_EVENT_MASK (PAPR_PMEM_HEALTH_CRITICAL | \
- 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];
diff --git a/include/linux/papr_scm.h b/include/linux/papr_scm.h
new file mode 100644
index 0000000000000..f116e5ffef369
--- /dev/null
+++ b/include/linux/papr_scm.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __LINUX_PAPR_SCM_H
+#define __LINUX_PAPR_SCM_H
+
+/* DIMM health bitmap bitmap indicators */
+/* SCM device is unable to persist memory contents */
+#define PAPR_PMEM_UNARMED (1ULL << (63 - 0))
+/* SCM device failed to persist memory contents */
+#define PAPR_PMEM_SHUTDOWN_DIRTY (1ULL << (63 - 1))
+/* SCM device contents are persisted from previous IPL */
+#define PAPR_PMEM_SHUTDOWN_CLEAN (1ULL << (63 - 2))
+/* SCM device contents are not persisted from previous IPL */
+#define PAPR_PMEM_EMPTY (1ULL << (63 - 3))
+/* SCM device memory life remaining is critically low */
+#define PAPR_PMEM_HEALTH_CRITICAL (1ULL << (63 - 4))
+/* SCM device will be garded off next IPL due to failure */
+#define PAPR_PMEM_HEALTH_FATAL (1ULL << (63 - 5))
+/* SCM contents cannot persist due to current platform health status */
+#define PAPR_PMEM_HEALTH_UNHEALTHY (1ULL << (63 - 6))
+/* SCM device is unable to persist memory contents in certain conditions */
+#define PAPR_PMEM_HEALTH_NON_CRITICAL (1ULL << (63 - 7))
+/* SCM device is encrypted */
+#define PAPR_PMEM_ENCRYPTED (1ULL << (63 - 8))
+/* SCM device has been scrubbed and locked */
+#define PAPR_PMEM_SCRUBBED_AND_LOCKED (1ULL << (63 - 9))
+
+#define PAPR_PMEM_SAVE_FAILED (1ULL << (63 - 10))
+
+/* Bits status indicators for health bitmap indicating unarmed dimm */
+#define PAPR_PMEM_UNARMED_MASK (PAPR_PMEM_UNARMED | PAPR_PMEM_HEALTH_UNHEALTHY)
+
+/* Bits status indicators for health bitmap indicating unflushed dimm */
+#define PAPR_PMEM_BAD_SHUTDOWN_MASK (PAPR_PMEM_SHUTDOWN_DIRTY)
+
+/* Bits status indicators for health bitmap indicating unrestored dimm */
+#define PAPR_PMEM_BAD_RESTORE_MASK (PAPR_PMEM_EMPTY)
+
+/* Bit status indicators for smart event notification */
+#define PAPR_PMEM_SMART_EVENT_MASK (PAPR_PMEM_HEALTH_CRITICAL | \
+ PAPR_PMEM_HEALTH_FATAL | \
+ PAPR_PMEM_HEALTH_UNHEALTHY)
+
+#define PAPR_PMEM_SAVE_MASK (PAPR_PMEM_SAVE_FAILED)
+
+#define PAPR_SCM_PERF_STATS_EYECATCHER __stringify(SCMSTATS)
+#define PAPR_SCM_PERF_STATS_VERSION 0x1
+
+#endif /* __LINUX_PAPR_SCM_H */
diff --git a/include/uapi/linux/papr_pdsm.h b/include/uapi/linux/papr_pdsm.h
new file mode 100644
index 0000000000000..1ef46fe8d9052
--- /dev/null
+++ b/include/uapi/linux/papr_pdsm.h
@@ -0,0 +1,147 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * PAPR nvDimm Specific Methods (PDSM) and structs for libndctl
+ *
+ * (C) Copyright IBM 2020-2021
+ *
+ * Author: Vaibhav Jain <vaibhav at linux.ibm.com>
+ */
+
+#ifndef _UAPI_LINUX_PAPR_PDSM_H_
+#define _UAPI_LINUX_PAPR_PDSM_H_
+
+#include <linux/types.h>
+#include <linux/ndctl.h>
+
+/*
+ * PDSM Envelope:
+ *
+ * The ioctl ND_CMD_CALL exchange data between user-space and kernel via
+ * envelope which consists of 2 headers sections and payload sections as
+ * illustrated below:
+ * +-----------------+---------------+---------------------------+
+ * | 64-Bytes | 8-Bytes | Max 184-Bytes |
+ * +-----------------+---------------+---------------------------+
+ * | ND-HEADER | PDSM-HEADER | PDSM-PAYLOAD |
+ * +-----------------+---------------+---------------------------+
+ * | nd_family | | |
+ * | nd_size_out | cmd_status | |
+ * | nd_size_in | reserved | nd_pdsm_payload |
+ * | nd_command | payload --> | |
+ * | nd_fw_size | | |
+ * | nd_payload ---> | | |
+ * +---------------+-----------------+---------------------------+
+ *
+ * ND Header:
+ * This is the generic libnvdimm header described as 'struct nd_cmd_pkg'
+ * which is interpreted by libnvdimm before passed on to papr_scm. Important
+ * member fields used are:
+ * 'nd_family' : (In) NVDIMM_FAMILY_PAPR_SCM
+ * 'nd_size_in' : (In) PDSM-HEADER + PDSM-IN-PAYLOAD (usually 0)
+ * 'nd_size_out' : (In) PDSM-HEADER + PDSM-RETURN-PAYLOAD
+ * 'nd_command' : (In) One of PAPR_PDSM_XXX
+ * 'nd_fw_size' : (Out) PDSM-HEADER + size of actual payload returned
+ *
+ * PDSM Header:
+ * This is papr-scm specific header that precedes the payload. This is defined
+ * as nd_cmd_pdsm_pkg. Following fields aare available in this header:
+ *
+ * 'cmd_status' : (Out) Errors if any encountered while servicing PDSM.
+ * 'reserved' : Not used, reserved for future and should be set to 0.
+ * 'payload' : A union of all the possible payload structs
+ *
+ * PDSM Payload:
+ *
+ * The layout of the PDSM Payload is defined by various structs shared between
+ * papr_scm and libndctl so that contents of payload can be interpreted. As such
+ * its defined as a union of all possible payload structs as
+ * 'union nd_pdsm_payload'. Based on the value of 'nd_cmd_pkg.nd_command'
+ * appropriate member of the union is accessed.
+ */
+
+/* Max payload size that we can handle */
+#define ND_PDSM_PAYLOAD_MAX_SIZE 184
+
+/* Max payload size that we can handle */
+#define ND_PDSM_HDR_SIZE \
+ (sizeof(struct nd_pkg_pdsm) - ND_PDSM_PAYLOAD_MAX_SIZE)
+
+/* Various nvdimm health indicators */
+#define PAPR_PDSM_DIMM_HEALTHY 0
+#define PAPR_PDSM_DIMM_UNHEALTHY 1
+#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
+
+/* Indicate that the 'dimm_dsc' field is valid */
+#define PDSM_DIMM_DSC_VALID 2
+
+/*
+ * Struct exchanged between kernel & ndctl in for PAPR_PDSM_HEALTH
+ * Various flags indicate the health status of the dimm.
+ *
+ * extension_flags : Any extension fields present in the struct.
+ * dimm_unarmed : Dimm not armed. So contents wont persist.
+ * dimm_bad_shutdown : Previous shutdown did not persist contents.
+ * dimm_bad_restore : Contents from previous shutdown werent restored.
+ * dimm_scrubbed : Contents of the dimm have been scrubbed.
+ * 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 {
+ struct {
+ __u32 extension_flags;
+ __u8 dimm_unarmed;
+ __u8 dimm_bad_shutdown;
+ __u8 dimm_bad_restore;
+ __u8 dimm_scrubbed;
+ __u8 dimm_locked;
+ __u8 dimm_encrypted;
+ __u16 dimm_health;
+
+ /* Extension flag PDSM_DIMM_HEALTH_RUN_GAUGE_VALID */
+ __u16 dimm_fuel_gauge;
+
+ /* Extension flag PDSM_DIMM_DSC_VALID */
+ __u64 dimm_dsc;
+ };
+ __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
+ };
+};
+
+/*
+ * Methods to be embedded in ND_CMD_CALL request. These are sent to the kernel
+ * via 'nd_cmd_pkg.nd_command' member of the ioctl struct
+ */
+enum papr_pdsm {
+ PAPR_PDSM_MIN = 0x0,
+ PAPR_PDSM_HEALTH,
+ PAPR_PDSM_MAX,
+};
+
+/* Maximal union that can hold all possible payload types */
+union nd_pdsm_payload {
+ struct nd_papr_pdsm_health health;
+ __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
+} __packed;
+
+/*
+ * PDSM-header + payload expected with ND_CMD_CALL ioctl from libnvdimm
+ * Valid member of union 'payload' is identified via 'nd_cmd_pkg.nd_command'
+ * that should always precede this struct when sent to papr_scm via CMD_CALL
+ * interface.
+ */
+struct nd_pkg_pdsm {
+ __s32 cmd_status; /* Out: Sub-cmd status returned back */
+ __u16 reserved[2]; /* Ignored and to be set as '0' */
+ union nd_pdsm_payload payload;
+} __packed;
+
+#endif /* _UAPI_LINUX_PAPR_PDSM_H_ */
diff --git a/tools/testing/nvdimm/test/ndtest.c b/tools/testing/nvdimm/test/ndtest.c
index 6862915f1fb0c..00ec2c2130614 100644
--- a/tools/testing/nvdimm/test/ndtest.c
+++ b/tools/testing/nvdimm/test/ndtest.c
@@ -13,6 +13,7 @@
#include <nd-core.h>
#include <linux/printk.h>
#include <linux/seq_buf.h>
+#include <linux/papr_scm.h>
#include "../watermark.h"
#include "nfit_test.h"
diff --git a/tools/testing/nvdimm/test/ndtest.h b/tools/testing/nvdimm/test/ndtest.h
index 2c54c9cbb90c7..8f27ad6f7319f 100644
--- a/tools/testing/nvdimm/test/ndtest.h
+++ b/tools/testing/nvdimm/test/ndtest.h
@@ -5,37 +5,6 @@
#include <linux/platform_device.h>
#include <linux/libnvdimm.h>
-/* SCM device is unable to persist memory contents */
-#define PAPR_PMEM_UNARMED (1ULL << (63 - 0))
-/* SCM device failed to persist memory contents */
-#define PAPR_PMEM_SHUTDOWN_DIRTY (1ULL << (63 - 1))
-/* SCM device contents are not persisted from previous IPL */
-#define PAPR_PMEM_EMPTY (1ULL << (63 - 3))
-#define PAPR_PMEM_HEALTH_CRITICAL (1ULL << (63 - 4))
-/* SCM device will be garded off next IPL due to failure */
-#define PAPR_PMEM_HEALTH_FATAL (1ULL << (63 - 5))
-/* SCM contents cannot persist due to current platform health status */
-#define PAPR_PMEM_HEALTH_UNHEALTHY (1ULL << (63 - 6))
-
-/* Bits status indicators for health bitmap indicating unarmed dimm */
-#define PAPR_PMEM_UNARMED_MASK (PAPR_PMEM_UNARMED | \
- PAPR_PMEM_HEALTH_UNHEALTHY)
-
-#define PAPR_PMEM_SAVE_FAILED (1ULL << (63 - 10))
-
-/* Bits status indicators for health bitmap indicating unflushed dimm */
-#define PAPR_PMEM_BAD_SHUTDOWN_MASK (PAPR_PMEM_SHUTDOWN_DIRTY)
-
-/* Bits status indicators for health bitmap indicating unrestored dimm */
-#define PAPR_PMEM_BAD_RESTORE_MASK (PAPR_PMEM_EMPTY)
-
-/* Bit status indicators for smart event notification */
-#define PAPR_PMEM_SMART_EVENT_MASK (PAPR_PMEM_HEALTH_CRITICAL | \
- PAPR_PMEM_HEALTH_FATAL | \
- PAPR_PMEM_HEALTH_UNHEALTHY)
-
-#define PAPR_PMEM_SAVE_MASK (PAPR_PMEM_SAVE_FAILED)
-
struct ndtest_config;
struct ndtest_priv {
^ permalink raw reply related
* Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
From: Will Deacon @ 2021-06-30 11:43 UTC (permalink / raw)
To: Claire Chang
Cc: heikki.krogerus, thomas.hellstrom, peterz, joonas.lahtinen,
dri-devel, chris, grant.likely, paulus, Frank Rowand, mingo,
Marek Szyprowski, Stefano Stabellini, Saravana Kannan,
Joerg Roedel, Rafael J . Wysocki, Christoph Hellwig,
Bartosz Golaszewski, bskeggs, linux-pci, xen-devel,
Thierry Reding, intel-gfx, matthew.auld, linux-devicetree,
Jianxiong Gao, Daniel Vetter, Konrad Rzeszutek Wilk,
maarten.lankhorst, airlied, Dan Williams, linuxppc-dev,
jani.nikula, Nathan Chancellor, Rob Herring, rodrigo.vivi,
Bjorn Helgaas, boris.ostrovsky, Andy Shevchenko, jgross,
Nicolas Boichat, Greg KH, Randy Dunlap, Qian Cai, lkml,
Tomasz Figa, list@263.net:IOMMU DRIVERS, Jim Quinlan, xypron.glpk,
Tom Lendacky, Robin Murphy, bauerman
In-Reply-To: <CALiNf2-a-haQN0-4+gX8+wa++52-0CnO2O4BEkxrQCxoTa_47w@mail.gmail.com>
On Wed, Jun 30, 2021 at 05:17:27PM +0800, Claire Chang wrote:
> On Wed, Jun 30, 2021 at 9:43 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > On Thu, Jun 24, 2021 at 11:55:20PM +0800, Claire Chang wrote:
> > > Propagate the swiotlb_force into io_tlb_default_mem->force_bounce and
> > > use it to determine whether to bounce the data or not. This will be
> > > useful later to allow for different pools.
> > >
> > > Signed-off-by: Claire Chang <tientzu@chromium.org>
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > Tested-by: Stefano Stabellini <sstabellini@kernel.org>
> > > Tested-by: Will Deacon <will@kernel.org>
> > > Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> >
> > This patch as commit af452ec1b1a3 ("swiotlb: Use is_swiotlb_force_bounce
> > for swiotlb data bouncing") causes my Ryzen 3 4300G system to fail to
> > get to an X session consistently (although not every single time),
> > presumably due to a crash in the AMDGPU driver that I see in dmesg.
> >
> > I have attached logs at af452ec1b1a3 and f127c9556a8e and I am happy
> > to provide any further information, debug, or test patches as necessary.
>
> Are you using swiotlb=force? or the swiotlb_map is called because of
> !dma_capable? (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/kernel/dma/direct.h#n93)
The command line is in the dmesg:
| Kernel command line: initrd=\amd-ucode.img initrd=\initramfs-linux-next-llvm.img root=PARTUUID=8680aa0c-cf09-4a69-8cf3-970478040ee7 rw intel_pstate=no_hwp irqpoll
but I worry that this looks _very_ similar to the issue reported by Qian
Cai which we thought we had fixed. Nathan -- is the failure deterministic?
> `BUG: unable to handle page fault for address: 00000000003a8290` and
> the fact it crashed at `_raw_spin_lock_irqsave` look like the memory
> (maybe dev->dma_io_tlb_mem) was corrupted?
> The dev->dma_io_tlb_mem should be set here
> (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/pci/probe.c#n2528)
> through device_initialize.
I'm less sure about this. 'dma_io_tlb_mem' should be pointing at
'io_tlb_default_mem', which is a page-aligned allocation from memblock.
The spinlock is at offset 0x24 in that structure, and looking at the
register dump from the crash:
Jun 29 18:28:42 hp-4300G kernel: RSP: 0018:ffffadb4013db9e8 EFLAGS: 00010006
Jun 29 18:28:42 hp-4300G kernel: RAX: 00000000003a8290 RBX: 0000000000000000 RCX: ffff8900572ad580
Jun 29 18:28:42 hp-4300G kernel: RDX: ffff89005653f024 RSI: 00000000000c0000 RDI: 0000000000001d17
Jun 29 18:28:42 hp-4300G kernel: RBP: 000000000a20d000 R08: 00000000000c0000 R09: 0000000000000000
Jun 29 18:28:42 hp-4300G kernel: R10: 000000000a20d000 R11: ffff89005653f000 R12: 0000000000000212
Jun 29 18:28:42 hp-4300G kernel: R13: 0000000000001000 R14: 0000000000000002 R15: 0000000000200000
Jun 29 18:28:42 hp-4300G kernel: FS: 00007f1f8898ea40(0000) GS:ffff890057280000(0000) knlGS:0000000000000000
Jun 29 18:28:42 hp-4300G kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Jun 29 18:28:42 hp-4300G kernel: CR2: 00000000003a8290 CR3: 00000001020d0000 CR4: 0000000000350ee0
Jun 29 18:28:42 hp-4300G kernel: Call Trace:
Jun 29 18:28:42 hp-4300G kernel: _raw_spin_lock_irqsave+0x39/0x50
Jun 29 18:28:42 hp-4300G kernel: swiotlb_tbl_map_single+0x12b/0x4c0
Then that correlates with R11 holding the 'dma_io_tlb_mem' pointer and
RDX pointing at the spinlock. Yet RAX is holding junk :/
I agree that enabling KASAN would be a good idea, but I also think we
probably need to get some more information out of swiotlb_tbl_map_single()
to see see what exactly is going wrong in there.
Will
^ permalink raw reply
* [PATCH] crypto: DRBG - select SHA512
From: Stephan Mueller @ 2021-06-30 10:32 UTC (permalink / raw)
To: Sachin Sant, linux-crypto; +Cc: linuxppc-dev
In-Reply-To: <73D2DF91-CC7A-46CD-8D48-63FFB1857D24@linux.vnet.ibm.com>
With the swtich to use HMAC(SHA-512) as the default DRBG type, the
configuration must now also select SHA-512.
Fixes: 9b7b94683a9b "crypto: DRBG - switch to HMAC SHA512 DRBG as default
DRBG"
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Signed-off-by: Stephan Mueller <smueller@chronox.com>
---
crypto/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/Kconfig b/crypto/Kconfig
index ca3b02dcbbfa..64b772c5d1c9 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1768,7 +1768,7 @@ config CRYPTO_DRBG_HMAC
bool
default y
select CRYPTO_HMAC
- select CRYPTO_SHA256
+ select CRYPTO_SHA512
config CRYPTO_DRBG_HASH
bool "Enable Hash DRBG"
--
2.31.1
^ permalink raw reply related
* Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
From: Claire Chang @ 2021-06-30 9:17 UTC (permalink / raw)
To: Nathan Chancellor
Cc: heikki.krogerus, thomas.hellstrom, peterz, joonas.lahtinen,
dri-devel, chris, grant.likely, paulus, Frank Rowand, mingo,
Marek Szyprowski, Stefano Stabellini, Saravana Kannan,
Joerg Roedel, Rafael J . Wysocki, Christoph Hellwig,
Bartosz Golaszewski, bskeggs, linux-pci, xen-devel,
Thierry Reding, intel-gfx, matthew.auld, linux-devicetree,
Jianxiong Gao, Daniel Vetter, Will Deacon, Konrad Rzeszutek Wilk,
maarten.lankhorst, airlied, Dan Williams, linuxppc-dev,
jani.nikula, Rob Herring, rodrigo.vivi, Bjorn Helgaas,
boris.ostrovsky, Andy Shevchenko, jgross, Nicolas Boichat,
Greg KH, Randy Dunlap, Qian Cai, lkml, Tomasz Figa,
list@263.net:IOMMU DRIVERS, Jim Quinlan, xypron.glpk,
Tom Lendacky, Robin Murphy, bauerman
In-Reply-To: <YNvMDFWKXSm4LRfZ@Ryzen-9-3900X.localdomain>
On Wed, Jun 30, 2021 at 9:43 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Thu, Jun 24, 2021 at 11:55:20PM +0800, Claire Chang wrote:
> > Propagate the swiotlb_force into io_tlb_default_mem->force_bounce and
> > use it to determine whether to bounce the data or not. This will be
> > useful later to allow for different pools.
> >
> > Signed-off-by: Claire Chang <tientzu@chromium.org>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > Tested-by: Stefano Stabellini <sstabellini@kernel.org>
> > Tested-by: Will Deacon <will@kernel.org>
> > Acked-by: Stefano Stabellini <sstabellini@kernel.org>
>
> This patch as commit af452ec1b1a3 ("swiotlb: Use is_swiotlb_force_bounce
> for swiotlb data bouncing") causes my Ryzen 3 4300G system to fail to
> get to an X session consistently (although not every single time),
> presumably due to a crash in the AMDGPU driver that I see in dmesg.
>
> I have attached logs at af452ec1b1a3 and f127c9556a8e and I am happy
> to provide any further information, debug, or test patches as necessary.
Are you using swiotlb=force? or the swiotlb_map is called because of
!dma_capable? (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/kernel/dma/direct.h#n93)
`BUG: unable to handle page fault for address: 00000000003a8290` and
the fact it crashed at `_raw_spin_lock_irqsave` look like the memory
(maybe dev->dma_io_tlb_mem) was corrupted?
The dev->dma_io_tlb_mem should be set here
(https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/pci/probe.c#n2528)
through device_initialize.
I can't tell what happened from the logs, but maybe we could try KASAN
to see if it provides more clue.
Thanks,
Claire
>
> Cheers,
> Nathan
^ permalink raw reply
* Re: [PATCH v3 3/9] powerpc/64e: remove implicit soft-masking and interrupt exit restart logic
From: Christophe Leroy @ 2021-06-30 7:56 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Sachin Sant
In-Reply-To: <20210630074621.2109197-4-npiggin@gmail.com>
Le 30/06/2021 à 09:46, Nicholas Piggin a écrit :
> The implicit soft-masking to speed up interrupt return was going to be
> used by 64e as well, but it has not been extensively tested on that
> platform and is not considered ready. It was intended to be disabled
> before merge. Disable it for now.
>
> Most of the restart code is common with 64s, so with more correctness
> and performance testing this could be re-enabled again by adding the
> extra soft-mask checks to interrupt handlers and flipping
> exit_must_hard_disable().
>
> Fixes: 9d1988ca87dd ("powerpc/64: treat low kernel text as irqs soft-masked")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/interrupt.h | 33 ++++++++++++++++++++--------
> arch/powerpc/kernel/exceptions-64e.S | 12 +---------
> arch/powerpc/kernel/interrupt.c | 2 +-
> arch/powerpc/kernel/interrupt_64.S | 16 ++++++++++++--
> 4 files changed, 40 insertions(+), 23 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index 8b4b1e84e110..f13c93b033c7 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -73,20 +73,34 @@
> #include <asm/kprobes.h>
> #include <asm/runlatch.h>
>
> -#ifdef CONFIG_PPC64
> +#ifdef CONFIG_PPC_BOOK3S_64
Can we avoid that ifdef and use IS_ENABLED(CONFIG_PPC_BOOK3S_64) below ?
> extern char __end_soft_masked[];
> unsigned long search_kernel_restart_table(unsigned long addr);
> -#endif
>
> -#ifdef CONFIG_PPC_BOOK3S_64
> DECLARE_STATIC_KEY_FALSE(interrupt_exit_not_reentrant);
>
> +static inline bool is_implicit_soft_masked(struct pt_regs *regs)
> +{
> + if (regs->msr & MSR_PR)
> + return false;
> +
> + if (regs->nip >= (unsigned long)__end_soft_masked)
> + return false;
> +
> + return true;
> +}
> +
> static inline void srr_regs_clobbered(void)
> {
> local_paca->srr_valid = 0;
> local_paca->hsrr_valid = 0;
> }
> #else
> +static inline bool is_implicit_soft_masked(struct pt_regs *regs)
> +{
> + return false;
> +}
> +
> static inline void srr_regs_clobbered(void)
> {
> }
> @@ -150,11 +164,13 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
> */
> if (TRAP(regs) != INTERRUPT_PROGRAM) {
> CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
> - BUG_ON(regs->nip < (unsigned long)__end_soft_masked);
> + BUG_ON(is_implicit_soft_masked(regs));
> }
> +#ifdef CONFIG_PPC_BOOK3S
Allthough we are already in a PPC64 section, wouldn't it be better to use CONFIG_PPC_BOOK3S_64 ?
Can we use IS_ENABLED(CONFIG_PPC_BOOK3S_64) instead ?
> /* Move this under a debugging check */
> if (arch_irq_disabled_regs(regs))
> BUG_ON(search_kernel_restart_table(regs->nip));
> +#endif
> }
> #endif
>
> @@ -244,10 +260,9 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte
> local_paca->irq_soft_mask = IRQS_ALL_DISABLED;
> local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
>
> - if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !(regs->msr & MSR_PR) &&
> - regs->nip < (unsigned long)__end_soft_masked) {
> - // Kernel code running below __end_soft_masked is
> - // implicitly soft-masked.
> + if (is_implicit_soft_masked(regs)) {
> + // Adjust regs->softe soft implicit soft-mask, so
> + // arch_irq_disabled_regs(regs) behaves as expected.
> regs->softe = IRQS_ALL_DISABLED;
> }
>
> @@ -282,6 +297,7 @@ static inline void interrupt_nmi_exit_prepare(struct pt_regs *regs, struct inter
> */
>
> #ifdef CONFIG_PPC64
> +#ifdef CONFIG_PPC_BOOK3S
IS_ENABLED(CONFIG_PPC_BOOK3S_64) instead ?
> if (arch_irq_disabled_regs(regs)) {
> unsigned long rst = search_kernel_restart_table(regs->nip);
> if (rst)
> @@ -289,7 +305,6 @@ static inline void interrupt_nmi_exit_prepare(struct pt_regs *regs, struct inter
> }
> #endif
>
> -#ifdef CONFIG_PPC64
> if (nmi_disables_ftrace(regs))
> this_cpu_set_ftrace_enabled(state->ftrace_enabled);
>
> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
> index d634bfceed2c..1401787b0b93 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -342,17 +342,7 @@ ret_from_mc_except:
> #define PROLOG_ADDITION_MASKABLE_GEN(n) \
> lbz r10,PACAIRQSOFTMASK(r13); /* are irqs soft-masked? */ \
> andi. r10,r10,IRQS_DISABLED; /* yes -> go out of line */ \
> - bne masked_interrupt_book3e_##n; \
> - /* Kernel code below __end_soft_masked is implicitly masked */ \
> - andi. r10,r11,MSR_PR; \
> - bne 1f; /* user -> not masked */ \
> - std r14,PACA_EXGEN+EX_R14(r13); \
> - LOAD_REG_IMMEDIATE_SYM(r14, r10, __end_soft_masked); \
> - mfspr r10,SPRN_SRR0; \
> - cmpld r10,r14; \
> - ld r14,PACA_EXGEN+EX_R14(r13); \
> - blt masked_interrupt_book3e_##n; \
> -1:
> + bne masked_interrupt_book3e_##n
>
> /*
> * Additional regs must be re-loaded from paca before EXCEPTION_COMMON* is
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 0052702ee5ac..21bbd615ca41 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -36,7 +36,7 @@ static inline bool exit_must_hard_disable(void)
> #else
> static inline bool exit_must_hard_disable(void)
> {
> - return IS_ENABLED(CONFIG_PPC32);
> + return true;
> }
> #endif
>
> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
> index e7a50613a570..09b8d8846c67 100644
> --- a/arch/powerpc/kernel/interrupt_64.S
> +++ b/arch/powerpc/kernel/interrupt_64.S
> @@ -231,7 +231,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_emulate)
> li r10,IRQS_ALL_DISABLED
> stb r10,PACAIRQSOFTMASK(r13)
> b system_call_vectored_common
> -#endif
> +#endif /* CONFIG_PPC_BOOK3S */
>
> .balign IFETCH_ALIGN_BYTES
> .globl system_call_common_real
> @@ -320,10 +320,12 @@ END_BTB_FLUSH_SECTION
> li r5,0 /* !scv */
> bl syscall_exit_prepare
> std r1,PACA_EXIT_SAVE_R1(r13) /* save r1 for restart */
> +#ifdef CONFIG_PPC_BOOK3S
> .Lsyscall_rst_start:
> lbz r11,PACAIRQHAPPENED(r13)
> andi. r11,r11,(~PACA_IRQ_HARD_DIS)@l
> bne- syscall_restart
> +#endif
> li r11,IRQS_ENABLED
> stb r11,PACAIRQSOFTMASK(r13)
> li r11,0
> @@ -396,6 +398,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> b .Lsyscall_restore_regs_cont
> .Lsyscall_rst_end:
>
> +#ifdef CONFIG_PPC_BOOK3S
> syscall_restart:
> GET_PACA(r13)
> ld r1,PACA_EXIT_SAVE_R1(r13)
> @@ -409,6 +412,7 @@ syscall_restart:
> b .Lsyscall_rst_start
>
> RESTART_TABLE(.Lsyscall_rst_start, .Lsyscall_rst_end, syscall_restart)
> +#endif
>
> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> tabort_syscall:
> @@ -504,10 +508,12 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\())
> bne- .Lrestore_nvgprs_\srr
> .Lrestore_nvgprs_\srr\()_cont:
> std r1,PACA_EXIT_SAVE_R1(r13) /* save r1 for restart */
> +#ifdef CONFIG_PPC_BOOK3S
> .Linterrupt_return_\srr\()_user_rst_start:
> lbz r11,PACAIRQHAPPENED(r13)
> andi. r11,r11,(~PACA_IRQ_HARD_DIS)@l
> bne- interrupt_return_\srr\()_user_restart
> +#endif
> li r11,IRQS_ENABLED
> stb r11,PACAIRQSOFTMASK(r13)
> li r11,0
> @@ -590,6 +596,7 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
> REST_NVGPRS(r1)
> b .Lrestore_nvgprs_\srr\()_cont
>
> +#ifdef CONFIG_PPC_BOOK3S
> interrupt_return_\srr\()_user_restart:
> GET_PACA(r13)
> ld r1,PACA_EXIT_SAVE_R1(r13)
> @@ -602,6 +609,7 @@ interrupt_return_\srr\()_user_restart:
> b .Linterrupt_return_\srr\()_user_rst_start
>
> RESTART_TABLE(.Linterrupt_return_\srr\()_user_rst_start, .Linterrupt_return_\srr\()_user_rst_end, interrupt_return_\srr\()_user_restart)
> +#endif
>
> .balign IFETCH_ALIGN_BYTES
> .Lkernel_interrupt_return_\srr\():
> @@ -615,9 +623,11 @@ RESTART_TABLE(.Linterrupt_return_\srr\()_user_rst_start, .Linterrupt_return_\srr
> cmpwi r11,IRQS_ENABLED
> stb r11,PACAIRQSOFTMASK(r13)
> bne 1f
> +#ifdef CONFIG_PPC_BOOK3S
> lbz r11,PACAIRQHAPPENED(r13)
> andi. r11,r11,(~PACA_IRQ_HARD_DIS)@l
> bne- interrupt_return_\srr\()_kernel_restart
> +#endif
> li r11,0
> stb r11,PACAIRQHAPPENED(r13) # clear out possible HARD_DIS
> 1:
> @@ -717,6 +727,7 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
> b . /* prevent speculative execution */
> .Linterrupt_return_\srr\()_kernel_rst_end:
>
> +#ifdef CONFIG_PPC_BOOK3S
> interrupt_return_\srr\()_kernel_restart:
> GET_PACA(r13)
> ld r1,PACA_EXIT_SAVE_R1(r13)
> @@ -729,14 +740,15 @@ interrupt_return_\srr\()_kernel_restart:
> b .Linterrupt_return_\srr\()_kernel_rst_start
>
> RESTART_TABLE(.Linterrupt_return_\srr\()_kernel_rst_start, .Linterrupt_return_\srr\()_kernel_rst_end, interrupt_return_\srr\()_kernel_restart)
> +#endif
>
> .endm
>
> interrupt_return_macro srr
> #ifdef CONFIG_PPC_BOOK3S
> interrupt_return_macro hsrr
> -#endif /* CONFIG_PPC_BOOK3S */
>
> .globl __end_soft_masked
> __end_soft_masked:
> DEFINE_FIXED_SYMBOL(__end_soft_masked)
> +#endif /* CONFIG_PPC_BOOK3S */
>
^ permalink raw reply
* [PATCH v3 9/9] powerpc/64s: move ret_from_fork etc above __end_soft_masked
From: Nicholas Piggin @ 2021-06-30 7:46 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Sachin Sant, Nicholas Piggin
In-Reply-To: <20210630074621.2109197-1-npiggin@gmail.com>
Code which runs with interrupts enabled should be moved above
__end_soft_masked where possible, because maskable interrupts that hit
below that symbol will need to consult the soft mask table, which is an
extra cost.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/interrupt_64.S | 52 +++++++++++++++---------------
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index c4336e2e2ce8..4063e8a3f704 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -449,32 +449,6 @@ _ASM_NOKPROBE_SYMBOL(tabort_syscall)
b . /* prevent speculative execution */
#endif
-#ifdef CONFIG_PPC_BOOK3S
-_GLOBAL(ret_from_fork_scv)
- bl schedule_tail
- REST_NVGPRS(r1)
- li r3,0 /* fork() return value */
- b .Lsyscall_vectored_common_exit
-#endif
-
-_GLOBAL(ret_from_fork)
- bl schedule_tail
- REST_NVGPRS(r1)
- li r3,0 /* fork() return value */
- b .Lsyscall_exit
-
-_GLOBAL(ret_from_kernel_thread)
- bl schedule_tail
- REST_NVGPRS(r1)
- mtctr r14
- mr r3,r15
-#ifdef PPC64_ELF_ABI_v2
- mr r12,r14
-#endif
- bctrl
- li r3,0
- b .Lsyscall_exit
-
/*
* If MSR EE/RI was never enabled, IRQs not reconciled, NVGPRs not
* touched, no exit work created, then this can be used.
@@ -768,3 +742,29 @@ interrupt_return_macro hsrr
__end_soft_masked:
DEFINE_FIXED_SYMBOL(__end_soft_masked)
#endif /* CONFIG_PPC_BOOK3S */
+
+#ifdef CONFIG_PPC_BOOK3S
+_GLOBAL(ret_from_fork_scv)
+ bl schedule_tail
+ REST_NVGPRS(r1)
+ li r3,0 /* fork() return value */
+ b .Lsyscall_vectored_common_exit
+#endif
+
+_GLOBAL(ret_from_fork)
+ bl schedule_tail
+ REST_NVGPRS(r1)
+ li r3,0 /* fork() return value */
+ b .Lsyscall_exit
+
+_GLOBAL(ret_from_kernel_thread)
+ bl schedule_tail
+ REST_NVGPRS(r1)
+ mtctr r14
+ mr r3,r15
+#ifdef PPC64_ELF_ABI_v2
+ mr r12,r14
+#endif
+ bctrl
+ li r3,0
+ b .Lsyscall_exit
--
2.23.0
^ permalink raw reply related
* [PATCH v3 8/9] powerpc/64s/interrupt: clean up interrupt return labels
From: Nicholas Piggin @ 2021-06-30 7:46 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Sachin Sant, Nicholas Piggin
In-Reply-To: <20210630074621.2109197-1-npiggin@gmail.com>
Normal kernel-interrupt exits can get interrupt_return_srr_user_restart
in their backtrace, which is an unusual and notable function, and it is
part of the user-interrupt exit path, which is doubly confusing.
Add non-local labels for both user and kernel interrupt exit cases to
address this and make the user and kernel cases more symmetric. Also get
rid of an unused label.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/interrupt_64.S | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 5c18362693fe..c4336e2e2ce8 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -509,7 +509,9 @@ interrupt_return_\srr\():
_ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\())
ld r4,_MSR(r1)
andi. r0,r4,MSR_PR
- beq .Lkernel_interrupt_return_\srr
+ beq interrupt_return_\srr\()_kernel
+interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
+_ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user)
addi r3,r1,STACK_FRAME_OVERHEAD
bl interrupt_exit_user_prepare
cmpdi r3,0
@@ -623,8 +625,8 @@ RESTART_TABLE(.Linterrupt_return_\srr\()_user_rst_start, .Linterrupt_return_\srr
#endif
.balign IFETCH_ALIGN_BYTES
-.Lkernel_interrupt_return_\srr\():
-.Linterrupt_return_\srr\()_kernel:
+interrupt_return_\srr\()_kernel:
+_ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_kernel)
addi r3,r1,STACK_FRAME_OVERHEAD
bl interrupt_exit_kernel_prepare
--
2.23.0
^ permalink raw reply related
* [PATCH v3 7/9] powerpc/64/interrupt: add missing kprobe annotations on interrupt exit symbols
From: Nicholas Piggin @ 2021-06-30 7:46 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Sachin Sant, Nicholas Piggin
In-Reply-To: <20210630074621.2109197-1-npiggin@gmail.com>
If one interrupt exit symbol must not be kprobed, none of them can be,
without more justification for why it's safe. Disallow kprobing on any
of the (non-local) labels in the exit paths.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/interrupt_64.S | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 2c92bbca02ca..5c18362693fe 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -197,6 +197,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
.Lsyscall_vectored_\name\()_rst_end:
syscall_vectored_\name\()_restart:
+_ASM_NOKPROBE_SYMBOL(syscall_vectored_\name\()_restart)
GET_PACA(r13)
ld r1,PACA_EXIT_SAVE_R1(r13)
ld r2,PACATOC(r13)
@@ -238,6 +239,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_emulate)
.balign IFETCH_ALIGN_BYTES
.globl system_call_common_real
system_call_common_real:
+_ASM_NOKPROBE_SYMBOL(system_call_common_real)
ld r10,PACAKMSR(r13) /* get MSR value for kernel */
mtmsrd r10
@@ -402,6 +404,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
#ifdef CONFIG_PPC_BOOK3S
syscall_restart:
+_ASM_NOKPROBE_SYMBOL(syscall_restart)
GET_PACA(r13)
ld r1,PACA_EXIT_SAVE_R1(r13)
ld r2,PACATOC(r13)
@@ -420,6 +423,7 @@ RESTART_TABLE(.Lsyscall_rst_start, .Lsyscall_rst_end, syscall_restart)
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
tabort_syscall:
+_ASM_NOKPROBE_SYMBOL(tabort_syscall)
/* Firstly we need to enable TM in the kernel */
mfmsr r10
li r9, 1
@@ -602,6 +606,7 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
#ifdef CONFIG_PPC_BOOK3S
interrupt_return_\srr\()_user_restart:
+_ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user_restart)
GET_PACA(r13)
ld r1,PACA_EXIT_SAVE_R1(r13)
ld r2,PACATOC(r13)
@@ -735,6 +740,7 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
#ifdef CONFIG_PPC_BOOK3S
interrupt_return_\srr\()_kernel_restart:
+_ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_kernel_restart)
GET_PACA(r13)
ld r1,PACA_EXIT_SAVE_R1(r13)
ld r2,PACATOC(r13)
--
2.23.0
^ 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