* [PATCH 06/11] powerpc/pseries: lparcfg calculate PURR on demand
From: Nicholas Piggin @ 2018-05-04 17:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt
In-Reply-To: <20180504171935.25410-1-npiggin@gmail.com>
For SPLPAR, lparcfg provides a sum of PURR registers for all CPUs.
Currently this is done by reading PURR in context switch and timer
interrupt, and storing that into a per-CPU variable. These are summed
to provide the value.
This does not work with all timer schemes (e.g., NO_HZ_FULL), and it
is sub-optimal for performance because it reads the PURR register on
every context switch, although that's been difficult to distinguish
from noise in the contxt_switch microbenchmark.
This patch implements the sum by calling a function on each CPU, to
read and add PURR values of each CPU.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/time.h | 8 --------
arch/powerpc/kernel/process.c | 14 --------------
arch/powerpc/kernel/time.c | 8 --------
arch/powerpc/platforms/pseries/lparcfg.c | 18 ++++++++++--------
4 files changed, 10 insertions(+), 38 deletions(-)
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index db546c034905..c965c79765c4 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -196,14 +196,6 @@ extern u64 mulhdu(u64, u64);
extern void div128_by_32(u64 dividend_high, u64 dividend_low,
unsigned divisor, struct div_result *dr);
-/* Used to store Processor Utilization register (purr) values */
-
-struct cpu_usage {
- u64 current_tb; /* Holds the current purr register values */
-};
-
-DECLARE_PER_CPU(struct cpu_usage, cpu_usage_array);
-
extern void secondary_cpu_time_init(void);
extern void __init time_init(void);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index ff7344d996e3..e6ff36923d84 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -845,10 +845,6 @@ bool ppc_breakpoint_available(void)
}
EXPORT_SYMBOL_GPL(ppc_breakpoint_available);
-#ifdef CONFIG_PPC64
-DEFINE_PER_CPU(struct cpu_usage, cpu_usage_array);
-#endif
-
static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
struct arch_hw_breakpoint *b)
{
@@ -1181,16 +1177,6 @@ struct task_struct *__switch_to(struct task_struct *prev,
WARN_ON(!irqs_disabled());
-#ifdef CONFIG_PPC64
- /*
- * Collect processor utilization data per process
- */
- if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
- struct cpu_usage *cu = this_cpu_ptr(&cpu_usage_array);
- cu->current_tb = mfspr(SPRN_PURR);
- }
-#endif /* CONFIG_PPC64 */
-
#ifdef CONFIG_PPC_BOOK3S_64
batch = this_cpu_ptr(&ppc64_tlb_batch);
if (batch->active) {
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index e7e8611e8863..1fe6a24357e7 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -597,14 +597,6 @@ static void __timer_interrupt(void)
__this_cpu_inc(irq_stat.timer_irqs_others);
}
-#ifdef CONFIG_PPC64
- /* collect purr register values often, for accurate calculations */
- if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
- struct cpu_usage *cu = this_cpu_ptr(&cpu_usage_array);
- cu->current_tb = mfspr(SPRN_PURR);
- }
-#endif
-
trace_timer_interrupt_exit(regs);
}
diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
index c508c938dc71..7c872dc01bdb 100644
--- a/arch/powerpc/platforms/pseries/lparcfg.c
+++ b/arch/powerpc/platforms/pseries/lparcfg.c
@@ -52,18 +52,20 @@
* Track sum of all purrs across all processors. This is used to further
* calculate usage values by different applications
*/
+static void cpu_get_purr(void *arg)
+{
+ atomic64_t *sum = arg;
+
+ atomic64_add(mfspr(SPRN_PURR), sum);
+}
+
static unsigned long get_purr(void)
{
- unsigned long sum_purr = 0;
- int cpu;
+ atomic64_t purr = ATOMIC64_INIT(0);
- for_each_possible_cpu(cpu) {
- struct cpu_usage *cu;
+ on_each_cpu(cpu_get_purr, &purr, 1);
- cu = &per_cpu(cpu_usage_array, cpu);
- sum_purr += cu->current_tb;
- }
- return sum_purr;
+ return atomic64_read(&purr);
}
/*
--
2.17.0
^ permalink raw reply related
* [PATCH 07/11] powerpc: generic clockevents broadcast receiver call tick_receive_broadcast
From: Nicholas Piggin @ 2018-05-04 17:19 UTC (permalink / raw)
To: linuxppc-dev
Cc: Nicholas Piggin, Benjamin Herrenschmidt, Srivatsa S . Bhat,
Preeti U Murthy
In-Reply-To: <20180504171935.25410-1-npiggin@gmail.com>
The broadcast tick recipient can call tick_receive_broadcast rather
than re-running the full timer interrupt.
It does not have to check for the next event time, because the sender
already determined the timer has expired. It does not have to test
irq_work_pending, because that's a direct decrementer interrupt and
does not go through the clock events subsystem. And it does not have
to read PURR because that was removed with the previous patch.
This results in no code size change, but both the decrementer and
broadcast path lengths are reduced.
Cc: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Cc: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/hw_irq.h | 1 +
arch/powerpc/include/asm/time.h | 1 -
arch/powerpc/kernel/smp.c | 4 +-
arch/powerpc/kernel/time.c | 84 ++++++++++++++-----------------
4 files changed, 42 insertions(+), 48 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
index fbc2d83808aa..46fe8307fc43 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -55,6 +55,7 @@ extern void replay_system_reset(void);
extern void __replay_interrupt(unsigned int vector);
extern void timer_interrupt(struct pt_regs *);
+extern void timer_broadcast_interrupt(void);
extern void performance_monitor_exception(struct pt_regs *regs);
extern void WatchdogException(struct pt_regs *regs);
extern void unknown_exception(struct pt_regs *regs);
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index c965c79765c4..69b89f941252 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -28,7 +28,6 @@ extern struct clock_event_device decrementer_clockevent;
struct rtc_time;
extern void to_tm(int tim, struct rtc_time * tm);
-extern void tick_broadcast_ipi_handler(void);
extern void generic_calibrate_decr(void);
extern void hdec_interrupt(struct pt_regs *regs);
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 9ca7148b5881..5441a47701b1 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -157,7 +157,7 @@ static irqreturn_t reschedule_action(int irq, void *data)
static irqreturn_t tick_broadcast_ipi_action(int irq, void *data)
{
- tick_broadcast_ipi_handler();
+ timer_broadcast_interrupt();
return IRQ_HANDLED;
}
@@ -278,7 +278,7 @@ irqreturn_t smp_ipi_demux_relaxed(void)
if (all & IPI_MESSAGE(PPC_MSG_RESCHEDULE))
scheduler_ipi();
if (all & IPI_MESSAGE(PPC_MSG_TICK_BROADCAST))
- tick_broadcast_ipi_handler();
+ timer_broadcast_interrupt();
#ifdef CONFIG_NMI_IPI
if (all & IPI_MESSAGE(PPC_MSG_NMI_IPI))
nmi_ipi_action(0, NULL);
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 1fe6a24357e7..ad876906f847 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -567,47 +567,16 @@ void arch_irq_work_raise(void)
#endif /* CONFIG_IRQ_WORK */
-static void __timer_interrupt(void)
-{
- struct pt_regs *regs = get_irq_regs();
- u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
- struct clock_event_device *evt = this_cpu_ptr(&decrementers);
- u64 now;
-
- trace_timer_interrupt_entry(regs);
-
- if (test_irq_work_pending()) {
- clear_irq_work_pending();
- irq_work_run();
- }
-
- now = get_tb_or_rtc();
- if (now >= *next_tb) {
- *next_tb = ~(u64)0;
- if (evt->event_handler)
- evt->event_handler(evt);
- __this_cpu_inc(irq_stat.timer_irqs_event);
- } else {
- now = *next_tb - now;
- if (now <= decrementer_max)
- set_dec(now);
- /* We may have raced with new irq work */
- if (test_irq_work_pending())
- set_dec(1);
- __this_cpu_inc(irq_stat.timer_irqs_others);
- }
-
- trace_timer_interrupt_exit(regs);
-}
-
/*
* timer_interrupt - gets called when the decrementer overflows,
* with interrupts disabled.
*/
-void timer_interrupt(struct pt_regs * regs)
+void timer_interrupt(struct pt_regs *regs)
{
- struct pt_regs *old_regs;
+ struct clock_event_device *evt = this_cpu_ptr(&decrementers);
u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
+ struct pt_regs *old_regs;
+ u64 now;
/* Ensure a positive value is written to the decrementer, or else
* some CPUs will continue to take decrementer exceptions.
@@ -638,13 +607,47 @@ void timer_interrupt(struct pt_regs * regs)
old_regs = set_irq_regs(regs);
irq_enter();
+ trace_timer_interrupt_entry(regs);
+
+ if (test_irq_work_pending()) {
+ clear_irq_work_pending();
+ irq_work_run();
+ }
+
+ now = get_tb_or_rtc();
+ if (now >= *next_tb) {
+ *next_tb = ~(u64)0;
+ if (evt->event_handler)
+ evt->event_handler(evt);
+ __this_cpu_inc(irq_stat.timer_irqs_event);
+ } else {
+ now = *next_tb - now;
+ if (now <= decrementer_max)
+ set_dec(now);
+ /* We may have raced with new irq work */
+ if (test_irq_work_pending())
+ set_dec(1);
+ __this_cpu_inc(irq_stat.timer_irqs_others);
+ }
- __timer_interrupt();
+ trace_timer_interrupt_exit(regs);
irq_exit();
set_irq_regs(old_regs);
}
EXPORT_SYMBOL(timer_interrupt);
+void timer_broadcast_interrupt(void)
+{
+ u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
+ struct pt_regs *regs = get_irq_regs();
+
+ trace_timer_interrupt_entry(regs);
+ *next_tb = ~(u64)0;
+ tick_receive_broadcast();
+ __this_cpu_inc(irq_stat.timer_irqs_event);
+ trace_timer_interrupt_exit(regs);
+}
+
/*
* Hypervisor decrementer interrupts shouldn't occur but are sometimes
* left pending on exit from a KVM guest. We don't need to do anything
@@ -992,15 +995,6 @@ static int decrementer_shutdown(struct clock_event_device *dev)
return 0;
}
-/* Interrupt handler for the timer broadcast IPI */
-void tick_broadcast_ipi_handler(void)
-{
- u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
-
- *next_tb = get_tb_or_rtc();
- __timer_interrupt();
-}
-
static void register_decrementer_clockevent(int cpu)
{
struct clock_event_device *dec = &per_cpu(decrementers, cpu);
--
2.17.0
^ permalink raw reply related
* [PATCH 08/11] powerpc: allow soft-NMI watchdog to cover timer interrupts with large decrementers
From: Nicholas Piggin @ 2018-05-04 17:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt
In-Reply-To: <20180504171935.25410-1-npiggin@gmail.com>
Large decrementers (e.g., POWER9) can take a very long time to wrap,
so when the timer iterrupt handler sets the decrementer to max so as
to avoid taking another decrementer interrupt when hard enabling
interrupts before running timers, it effectively disables the soft
NMI coverage for timer interrupts.
Fix this by using the traditional 31-bit value instead, which wraps
after a few seconds. masked interrupt code does the same thing, and
in normal operation neither of these paths would ever wrap even the
31 bit value.
Note: the SMP watchdog should catch timer interrupt lockups, but it
is preferable for the local soft-NMI to catch them, mainly to avoid
the IPI.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/time.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index ad876906f847..5862a3611795 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -578,22 +578,29 @@ void timer_interrupt(struct pt_regs *regs)
struct pt_regs *old_regs;
u64 now;
- /* Ensure a positive value is written to the decrementer, or else
- * some CPUs will continue to take decrementer exceptions.
- */
- set_dec(decrementer_max);
-
/* Some implementations of hotplug will get timer interrupts while
* offline, just ignore these and we also need to set
* decrementers_next_tb as MAX to make sure __check_irq_replay
* don't replay timer interrupt when return, otherwise we'll trap
* here infinitely :(
*/
- if (!cpu_online(smp_processor_id())) {
+ if (unlikely(!cpu_online(smp_processor_id()))) {
*next_tb = ~(u64)0;
+ set_dec(decrementer_max);
return;
}
+ /* Ensure a positive value is written to the decrementer, or else
+ * some CPUs will continue to take decrementer exceptions. When the
+ * PPC_WATCHDOG (decrementer based) is configured, keep this at most
+ * 31 bits, which is about 4 seconds on most systems, which gives
+ * the watchdog a chance of catching timer interrupt hard lockups.
+ */
+ if (IS_ENABLED(CONFIG_PPC_WATCHDOG))
+ set_dec(0x7fffffff);
+ else
+ set_dec(decrementer_max);
+
/* Conditionally hard-enable interrupts now that the DEC has been
* bumped to its maximum value
*/
--
2.17.0
^ permalink raw reply related
* [PATCH 09/11] powerpc: move timer broadcast code under GENERIC_CLOCKEVENTS_BROADCAST ifdef
From: Nicholas Piggin @ 2018-05-04 17:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt
In-Reply-To: <20180504171935.25410-1-npiggin@gmail.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/smp.c | 8 ++++++++
arch/powerpc/kernel/time.c | 2 ++
2 files changed, 10 insertions(+)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 5441a47701b1..914708eeb43f 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -155,11 +155,13 @@ static irqreturn_t reschedule_action(int irq, void *data)
return IRQ_HANDLED;
}
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
static irqreturn_t tick_broadcast_ipi_action(int irq, void *data)
{
timer_broadcast_interrupt();
return IRQ_HANDLED;
}
+#endif
#ifdef CONFIG_NMI_IPI
static irqreturn_t nmi_ipi_action(int irq, void *data)
@@ -172,7 +174,9 @@ static irqreturn_t nmi_ipi_action(int irq, void *data)
static irq_handler_t smp_ipi_action[] = {
[PPC_MSG_CALL_FUNCTION] = call_function_action,
[PPC_MSG_RESCHEDULE] = reschedule_action,
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
[PPC_MSG_TICK_BROADCAST] = tick_broadcast_ipi_action,
+#endif
#ifdef CONFIG_NMI_IPI
[PPC_MSG_NMI_IPI] = nmi_ipi_action,
#endif
@@ -186,7 +190,9 @@ static irq_handler_t smp_ipi_action[] = {
const char *smp_ipi_name[] = {
[PPC_MSG_CALL_FUNCTION] = "ipi call function",
[PPC_MSG_RESCHEDULE] = "ipi reschedule",
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
[PPC_MSG_TICK_BROADCAST] = "ipi tick-broadcast",
+#endif
[PPC_MSG_NMI_IPI] = "nmi ipi",
};
@@ -277,8 +283,10 @@ irqreturn_t smp_ipi_demux_relaxed(void)
generic_smp_call_function_interrupt();
if (all & IPI_MESSAGE(PPC_MSG_RESCHEDULE))
scheduler_ipi();
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
if (all & IPI_MESSAGE(PPC_MSG_TICK_BROADCAST))
timer_broadcast_interrupt();
+#endif
#ifdef CONFIG_NMI_IPI
if (all & IPI_MESSAGE(PPC_MSG_NMI_IPI))
nmi_ipi_action(0, NULL);
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 5862a3611795..23921f7b6e67 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -643,6 +643,7 @@ void timer_interrupt(struct pt_regs *regs)
}
EXPORT_SYMBOL(timer_interrupt);
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
void timer_broadcast_interrupt(void)
{
u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
@@ -654,6 +655,7 @@ void timer_broadcast_interrupt(void)
__this_cpu_inc(irq_stat.timer_irqs_event);
trace_timer_interrupt_exit(regs);
}
+#endif
/*
* Hypervisor decrementer interrupts shouldn't occur but are sometimes
--
2.17.0
^ permalink raw reply related
* [PATCH 10/11] powerpc: move a stray NMI IPI case under NMI_IPI ifdef
From: Nicholas Piggin @ 2018-05-04 17:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt
In-Reply-To: <20180504171935.25410-1-npiggin@gmail.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/smp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 914708eeb43f..28ec1638a540 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -193,7 +193,9 @@ const char *smp_ipi_name[] = {
#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
[PPC_MSG_TICK_BROADCAST] = "ipi tick-broadcast",
#endif
+#ifdef CONFIG_NMI_IPI
[PPC_MSG_NMI_IPI] = "nmi ipi",
+#endif
};
/* optional function to request ipi, for controllers with >= 4 ipis */
--
2.17.0
^ permalink raw reply related
* [PATCH 11/11] powerpc/time: account broadcast timer event interrupts separately
From: Nicholas Piggin @ 2018-05-04 17:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt
In-Reply-To: <20180504171935.25410-1-npiggin@gmail.com>
These are not local timer interrupts but IPIs. It's good to be able
to see how timer offloading is behaving, so split these out into
their own category.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/hardirq.h | 1 +
arch/powerpc/kernel/irq.c | 6 ++++++
arch/powerpc/kernel/time.c | 5 +----
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/hardirq.h b/arch/powerpc/include/asm/hardirq.h
index 5986d473722b..20b01897ea5d 100644
--- a/arch/powerpc/include/asm/hardirq.h
+++ b/arch/powerpc/include/asm/hardirq.h
@@ -8,6 +8,7 @@
typedef struct {
unsigned int __softirq_pending;
unsigned int timer_irqs_event;
+ unsigned int broadcast_irqs_event;
unsigned int timer_irqs_others;
unsigned int pmu_irqs;
unsigned int mce_exceptions;
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 6569b5ffff93..627db34bb79d 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -518,6 +518,11 @@ int arch_show_interrupts(struct seq_file *p, int prec)
seq_printf(p, "%10u ", per_cpu(irq_stat, j).timer_irqs_event);
seq_printf(p, " Local timer interrupts for timer event device\n");
+ seq_printf(p, "%*s: ", prec, "BCT");
+ for_each_online_cpu(j)
+ seq_printf(p, "%10u ", per_cpu(irq_stat, j).broadcast_irqs_event);
+ seq_printf(p, " Broadcast timer interrupts for timer event device\n");
+
seq_printf(p, "%*s: ", prec, "LOC");
for_each_online_cpu(j)
seq_printf(p, "%10u ", per_cpu(irq_stat, j).timer_irqs_others);
@@ -577,6 +582,7 @@ u64 arch_irq_stat_cpu(unsigned int cpu)
{
u64 sum = per_cpu(irq_stat, cpu).timer_irqs_event;
+ sum += per_cpu(irq_stat, cpu).broadcast_irqs_event;
sum += per_cpu(irq_stat, cpu).pmu_irqs;
sum += per_cpu(irq_stat, cpu).mce_exceptions;
sum += per_cpu(irq_stat, cpu).spurious_irqs;
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 23921f7b6e67..ed6b2abdde15 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -647,13 +647,10 @@ EXPORT_SYMBOL(timer_interrupt);
void timer_broadcast_interrupt(void)
{
u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
- struct pt_regs *regs = get_irq_regs();
- trace_timer_interrupt_entry(regs);
*next_tb = ~(u64)0;
tick_receive_broadcast();
- __this_cpu_inc(irq_stat.timer_irqs_event);
- trace_timer_interrupt_exit(regs);
+ __this_cpu_inc(irq_stat.broadcast_irqs_event);
}
#endif
--
2.17.0
^ permalink raw reply related
* [PATCH 1/2] powerpc: Disable attribute-alias warnings from gcc8
From: Khem Raj @ 2018-05-04 19:23 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Khem Raj, Benjamin Herrenschmidt
Fixes
alias between functions of incompatible types warnings
which are new with gcc8
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/kernel/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 2b4c40b255e4..7ac5a68ad6b1 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -3,7 +3,8 @@
# Makefile for the linux kernel.
#
-CFLAGS_ptrace.o += -DUTS_MACHINE='"$(UTS_MACHINE)"'
+CFLAGS_ptrace.o += -DUTS_MACHINE='"$(UTS_MACHINE)"' $(call cc-disable-warning, attribute-alias)
+CFLAGS_syscalls.o += $(call cc-disable-warning, attribute-alias)
subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
--
2.17.0
^ permalink raw reply related
* [PATCH 2/2] powerpc/ptrace: Disable array-bounds warning with gcc8
From: Khem Raj @ 2018-05-04 19:23 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Khem Raj, Benjamin Herrenschmidt
In-Reply-To: <20180504192313.18625-1-raj.khem@gmail.com>
This masks the new gcc8 warning
regset.h:270:4: error: 'memcpy' offset [-527, -529] is out
of the bounds [0, 16] of object 'vrsave' with type 'union <anonymous>'
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/kernel/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 7ac5a68ad6b1..ab159a34704a 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -4,6 +4,7 @@
#
CFLAGS_ptrace.o += -DUTS_MACHINE='"$(UTS_MACHINE)"' $(call cc-disable-warning, attribute-alias)
+CFLAGS_ptrace.o += $(call cc-disable-warning, array-bounds)
CFLAGS_syscalls.o += $(call cc-disable-warning, attribute-alias)
subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
--
2.17.0
^ permalink raw reply related
* [PATCH v3] powerpc, pkey: make protection key 0 less special
From: Ram Pai @ 2018-05-04 19:22 UTC (permalink / raw)
To: mpe
Cc: linuxppc-dev, benh, paulus, aneesh.kumar, bsingharora, hbabu,
mhocko, bauerman, linuxram, fweimer, msuchanek, Ulrich.Weigand,
Thomas Gleixner, Dave Hansen, Ingo Molnar, Andrew Morton
Applications need the ability to associate an address-range with some
key and latter revert to its initial default key. Pkey-0 comes close to
providing this function but falls short, because the current
implementation disallows applications to explicitly associate pkey-0 to
the address range.
Lets make pkey-0 less special and treat it almost like any other key.
Thus it can be explicitly associated with any address range, and can be
freed. This gives the application more flexibility and power. The
ability to free pkey-0 must be used responsibily, since pkey-0 is
associated with almost all address-range by default.
Even with this change pkey-0 continues to be slightly more special
from the following point of view.
(a) it is implicitly allocated.
(b) it is the default key assigned to any address-range.
(c) its permissions cannot be modified by userspace.
NOTE: (c) is specific to powerpc only. pkey-0 is associated by default
with all pages including kernel pages, and pkeys are also active in
kernel mode. If any permission is denied on pkey-0, the kernel running
in the context of the application will be unable to operate.
Tested on powerpc.
cc: Thomas Gleixner <tglx@linutronix.de>
cc: Dave Hansen <dave.hansen@intel.com>
cc: Michael Ellermen <mpe@ellerman.id.au>
cc: Ingo Molnar <mingo@kernel.org>
cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
History:
v3: . Corrected a comment in arch_set_user_pkey_access().
. Clarified the header, to capture the notion that
pkey-0 permissions cannot be modified by userspace on powerpc.
-- comment from Thiago
v2: . mm_pkey_is_allocated() continued to treat pkey-0 special.
fixed it.
arch/powerpc/include/asm/pkeys.h | 22 ++++++++++++++++++----
arch/powerpc/mm/pkeys.c | 26 +++++++++++++++-----------
2 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 0409c80..31a6976 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -101,10 +101,14 @@ static inline u16 pte_to_pkey_bits(u64 pteflags)
static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
{
- /* A reserved key is never considered as 'explicitly allocated' */
- return ((pkey < arch_max_pkey()) &&
- !__mm_pkey_is_reserved(pkey) &&
- __mm_pkey_is_allocated(mm, pkey));
+ if (pkey < 0 || pkey >= arch_max_pkey())
+ return false;
+
+ /* Reserved keys are never allocated. */
+ if (__mm_pkey_is_reserved(pkey))
+ return false;
+
+ return __mm_pkey_is_allocated(mm, pkey);
}
extern void __arch_activate_pkey(int pkey);
@@ -200,6 +204,16 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
{
if (static_branch_likely(&pkey_disabled))
return -EINVAL;
+
+ /*
+ * userspace should not change pkey-0 permissions.
+ * pkey-0 is associated with every page in the kernel.
+ * If userspace denies any permission on pkey-0, the
+ * kernel cannot operate.
+ */
+ if (!pkey)
+ return init_val ? -EINVAL : 0;
+
return __arch_set_user_pkey_access(tsk, pkey, init_val);
}
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index 0eafdf0..d6873b4 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -119,16 +119,21 @@ int pkey_initialize(void)
#else
os_reserved = 0;
#endif
+ /* Bits are in LE format. */
initial_allocation_mask = ~0x0;
+
+ /* register mask is in BE format */
pkey_amr_uamor_mask = ~0x0ul;
pkey_iamr_mask = ~0x0ul;
- /*
- * key 0, 1 are reserved.
- * key 0 is the default key, which allows read/write/execute.
- * key 1 is recommended not to be used. PowerISA(3.0) page 1015,
- * programming note.
- */
- for (i = 2; i < (pkeys_total - os_reserved); i++) {
+
+ for (i = 0; i < (pkeys_total - os_reserved); i++) {
+ /*
+ * key 1 is recommended not to be used.
+ * PowerISA(3.0) page 1015,
+ */
+ if (i == 1)
+ continue;
+
initial_allocation_mask &= ~(0x1 << i);
pkey_amr_uamor_mask &= ~(0x3ul << pkeyshift(i));
pkey_iamr_mask &= ~(0x1ul << pkeyshift(i));
@@ -142,7 +147,9 @@ void pkey_mm_init(struct mm_struct *mm)
{
if (static_branch_likely(&pkey_disabled))
return;
- mm_pkey_allocation_map(mm) = initial_allocation_mask;
+
+ /* allocate key-0 by default */
+ mm_pkey_allocation_map(mm) = initial_allocation_mask | 0x1;
/* -1 means unallocated or invalid */
mm->context.execute_only_pkey = -1;
}
@@ -407,9 +414,6 @@ static bool pkey_access_permitted(int pkey, bool write, bool execute)
int pkey_shift;
u64 amr;
- if (!pkey)
- return true;
-
if (!is_pkey_enabled(pkey))
return true;
--
1.7.1
^ permalink raw reply related
* [PATCH v2] powerpc: do not allow userspace to modify execute-only pkey
From: Ram Pai @ 2018-05-04 19:56 UTC (permalink / raw)
To: mpe
Cc: linuxppc-dev, benh, paulus, aneesh.kumar, bsingharora, hbabu,
mhocko, bauerman, linuxram, msuchanek, Ulrich.Weigand
When mprotect(....,PROT_EXEC) is called, the kernel allocates a
execute-only pkey and associates the pkey with the given address space.
The permission of this key should not be modifiable from userspace.
However a bug in the current implementation lets the permissions on the
key modifiable from userspace.
Whenever a key is allocated through mm_pkey_alloc(), the kernel programs
the UAMOR register to allow userspace to change permissions on the key.
This is fine for keys explicitly allocated through the
sys_pkey_alloc(). But for execute-only pkey, it must be disallowed.
Restructured the code to fix the bug.
cc: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
cc: Michael Ellermen <mpe@ellerman.id.au>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
History:
v2: Thiago noticed a bug -- __execute_only_pkey() will always fail
since it calls is_pkey_enabled() which always returns false
for execute_only key. is_pkey_enabled() returns false
because UAMOR bit for the execute_only key is and never be set.
Fixed it.
arch/powerpc/include/asm/pkeys.h | 24 ++++------------
arch/powerpc/mm/pkeys.c | 57 ++++++++++++++++++++++++++++++-------
2 files changed, 52 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 31a6976..3a9b82b 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -113,6 +113,8 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
extern void __arch_activate_pkey(int pkey);
extern void __arch_deactivate_pkey(int pkey);
+extern int __mm_pkey_alloc(struct mm_struct *mm);
+
/*
* Returns a positive, 5-bit key on success, or -1 on failure.
* Relies on the mmap_sem to protect against concurrency in mm_pkey_alloc() and
@@ -120,29 +122,14 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
*/
static inline int mm_pkey_alloc(struct mm_struct *mm)
{
- /*
- * Note: this is the one and only place we make sure that the pkey is
- * valid as far as the hardware is concerned. The rest of the kernel
- * trusts that only good, valid pkeys come out of here.
- */
- u32 all_pkeys_mask = (u32)(~(0x0));
int ret;
if (static_branch_likely(&pkey_disabled))
return -1;
+ ret = __mm_pkey_alloc(mm);
/*
- * Are we out of pkeys? We must handle this specially because ffz()
- * behavior is undefined if there are no zeros.
- */
- if (mm_pkey_allocation_map(mm) == all_pkeys_mask)
- return -1;
-
- ret = ffz((u32)mm_pkey_allocation_map(mm));
- __mm_pkey_allocated(mm, ret);
-
- /*
- * Enable the key in the hardware
+ * Enable userspace to modify the key permissions.
*/
if (ret > 0)
__arch_activate_pkey(ret);
@@ -158,7 +145,8 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
return -EINVAL;
/*
- * Disable the key in the hardware
+ * Reset the key and disable userspace
+ * from modifying the key permissions.
*/
__arch_deactivate_pkey(pkey);
__mm_pkey_free(mm, pkey);
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index d6873b4..e81d59e 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -190,6 +190,9 @@ static inline void write_uamor(u64 value)
mtspr(SPRN_UAMOR, value);
}
+/*
+ * return true if userspace can modify the pkey permissions.
+ */
static bool is_pkey_enabled(int pkey)
{
u64 uamor = read_uamor();
@@ -228,7 +231,10 @@ static void pkey_status_change(int pkey, bool enable)
init_amr(pkey, 0x0);
init_iamr(pkey, 0x0);
- /* Enable/disable key */
+ /*
+ * Enable/disable userspace to/from modifying the permissions
+ * on the key
+ */
old_uamor = read_uamor();
if (enable)
old_uamor |= (0x3ul << pkeyshift(pkey));
@@ -247,19 +253,35 @@ void __arch_deactivate_pkey(int pkey)
pkey_status_change(pkey, false);
}
-/*
- * Set the access rights in AMR IAMR and UAMOR registers for @pkey to that
- * specified in @init_val.
- */
-int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
+int __mm_pkey_alloc(struct mm_struct *mm)
+{
+ /*
+ * Note: this is the one and only place we make sure that the pkey is
+ * valid as far as the hardware is concerned. The rest of the kernel
+ * trusts that only good, valid pkeys come out of here.
+ */
+ u32 all_pkeys_mask = (u32)(~(0x0));
+ int ret;
+
+ /*
+ * Are we out of pkeys? We must handle this specially because ffz()
+ * behavior is undefined if there are no zeros.
+ */
+ if (mm_pkey_allocation_map(mm) == all_pkeys_mask)
+ return -1;
+
+ ret = ffz((u32)mm_pkey_allocation_map(mm));
+ __mm_pkey_allocated(mm, ret);
+
+ return ret;
+}
+
+static int set_user_pkey_access(struct task_struct *tsk, int pkey,
unsigned long init_val)
{
u64 new_amr_bits = 0x0ul;
u64 new_iamr_bits = 0x0ul;
- if (!is_pkey_enabled(pkey))
- return -EINVAL;
-
if (init_val & PKEY_DISABLE_EXECUTE) {
if (!pkey_execute_disable_supported)
return -EINVAL;
@@ -277,6 +299,19 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
return 0;
}
+/*
+ * Set the access rights in AMR IAMR and UAMOR registers for @pkey to that
+ * specified in @init_val.
+ */
+int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
+ unsigned long init_val)
+{
+ if (!is_pkey_enabled(pkey))
+ return -EINVAL;
+
+ return set_user_pkey_access(tsk, pkey, init_val);
+}
+
void thread_pkey_regs_save(struct thread_struct *thread)
{
if (static_branch_likely(&pkey_disabled))
@@ -336,7 +371,7 @@ int __execute_only_pkey(struct mm_struct *mm)
/* Do we need to assign a pkey for mm's execute-only maps? */
if (execute_only_pkey == -1) {
/* Go allocate one to use, which might fail */
- execute_only_pkey = mm_pkey_alloc(mm);
+ execute_only_pkey = __mm_pkey_alloc(mm);
if (execute_only_pkey < 0)
return -1;
need_to_set_mm_pkey = true;
@@ -355,7 +390,7 @@ int __execute_only_pkey(struct mm_struct *mm)
* Set up AMR so that it denies access for everything other than
* execution.
*/
- ret = __arch_set_user_pkey_access(current, execute_only_pkey,
+ ret = set_user_pkey_access(current, execute_only_pkey,
PKEY_DISABLE_ACCESS |
PKEY_DISABLE_WRITE);
/*
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v3] powerpc, pkey: make protection key 0 less special
From: Dave Hansen @ 2018-05-04 19:59 UTC (permalink / raw)
To: Ram Pai, mpe
Cc: linuxppc-dev, benh, paulus, aneesh.kumar, bsingharora, hbabu,
mhocko, bauerman, fweimer, msuchanek, Ulrich.Weigand,
Thomas Gleixner, Ingo Molnar, Andrew Morton
In-Reply-To: <1525461778-26265-1-git-send-email-linuxram@us.ibm.com>
On 05/04/2018 12:22 PM, Ram Pai wrote:
> @@ -407,9 +414,6 @@ static bool pkey_access_permitted(int pkey, bool write, bool execute)
> int pkey_shift;
> u64 amr;
>
> - if (!pkey)
> - return true;
> -
> if (!is_pkey_enabled(pkey))
> return true;
Looks fine to me. Obviously doesn't have any impact on x86 or the
generic code.
One question, though. Which other check makes up for this removed !pkey
check?
^ permalink raw reply
* [PATCH ] powerpc/pkeys: Detach execute_only key on !PROT_EXEC
From: Ram Pai @ 2018-05-04 20:01 UTC (permalink / raw)
To: mpe
Cc: linuxppc-dev, benh, paulus, aneesh.kumar, bsingharora, hbabu,
mhocko, bauerman, linuxram, msuchanek, Ulrich.Weigand,
dave.hansen, Shakeel Butt
Disassociate the exec_key from a VMA if the VMA permission is not
PROT_EXEC anymore. Otherwise the exec_only key continues to be
associated with the vma, causing unexpected behavior.
The problem was reported on x86 by Shakeel Butt,
which is also applicable on powerpc.
cc: Shakeel Butt <shakeelb@google.com>
Reported-by: Shakeel Butt <shakeelb@google.com>
Fixes 5586cf6 ("powerpc: introduce execute-only pkey")
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
arch/powerpc/mm/pkeys.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index e81d59e..fdeb9f5 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -425,9 +425,9 @@ int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot,
{
/*
* If the currently associated pkey is execute-only, but the requested
- * protection requires read or write, move it back to the default pkey.
+ * protection is not execute-only, move it back to the default pkey.
*/
- if (vma_is_pkey_exec_only(vma) && (prot & (PROT_READ | PROT_WRITE)))
+ if (vma_is_pkey_exec_only(vma) && (prot != PROT_EXEC))
return 0;
/*
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v3] powerpc, pkey: make protection key 0 less special
From: Ram Pai @ 2018-05-04 20:21 UTC (permalink / raw)
To: Dave Hansen
Cc: mpe, linuxppc-dev, benh, paulus, aneesh.kumar, bsingharora, hbabu,
mhocko, bauerman, fweimer, msuchanek, Ulrich.Weigand,
Thomas Gleixner, Ingo Molnar, Andrew Morton
In-Reply-To: <373dffe1-4752-5eb0-f97c-a06fe0e0fdb5@intel.com>
On Fri, May 04, 2018 at 12:59:27PM -0700, Dave Hansen wrote:
> On 05/04/2018 12:22 PM, Ram Pai wrote:
> > @@ -407,9 +414,6 @@ static bool pkey_access_permitted(int pkey, bool write, bool execute)
> > int pkey_shift;
> > u64 amr;
> >
> > - if (!pkey)
> > - return true;
> > -
> > if (!is_pkey_enabled(pkey))
> > return true;
>
> Looks fine to me. Obviously doesn't have any impact on x86 or the
> generic code.
>
> One question, though. Which other check makes up for this removed !pkey
> check?
is_pkey_enabled() does take care of it. we do not enable userspace to
change permissions on pkey-0. This information is tracked in
UAMOR register. is_pkey_enabled() refers to UAMOR to determine
if the given key is modifiable by userspace. since UAMOR has the bit
corresponding to key-0 set to 0, is_pkey_enabled(key-0) will return
false.
The deleted code above, would have done the same job without
referring UAMOR. However having special checks on pkey-0 makes
pkey-0 special. It defeats the purpose of this patch; which is to make
pkey-0 less special :).
--
Ram Pai
^ permalink raw reply
* Re: [PATCH v3] powerpc, pkey: make protection key 0 less special
From: Michal Suchánek @ 2018-05-04 21:26 UTC (permalink / raw)
To: Ram Pai
Cc: mpe, Ulrich.Weigand, bsingharora, Dave Hansen, benh, mhocko,
Ingo Molnar, Thomas Gleixner, Andrew Morton, aneesh.kumar,
bauerman, linuxppc-dev, fweimer, paulus, hbabu
In-Reply-To: <1525461778-26265-1-git-send-email-linuxram@us.ibm.com>
On Fri, 4 May 2018 12:22:58 -0700
"Ram Pai" <linuxram@us.ibm.com> wrote:
> Applications need the ability to associate an address-range with some
> key and latter revert to its initial default key. Pkey-0 comes close
> to providing this function but falls short, because the current
> implementation disallows applications to explicitly associate pkey-0
> to the address range.
>
> Lets make pkey-0 less special and treat it almost like any other key.
> Thus it can be explicitly associated with any address range, and can
> be freed. This gives the application more flexibility and power. The
> ability to free pkey-0 must be used responsibily, since pkey-0 is
> associated with almost all address-range by default.
>
> Even with this change pkey-0 continues to be slightly more special
> from the following point of view.
> (a) it is implicitly allocated.
> (b) it is the default key assigned to any address-range.
> (c) its permissions cannot be modified by userspace.
>
> NOTE: (c) is specific to powerpc only. pkey-0 is associated by default
> with all pages including kernel pages, and pkeys are also active in
> kernel mode. If any permission is denied on pkey-0, the kernel running
> in the context of the application will be unable to operate.
If it is not ok to change permissions of pkey 0 is it ok to free it?
Thanks
Michal
>
> Tested on powerpc.
>
> cc: Thomas Gleixner <tglx@linutronix.de>
> cc: Dave Hansen <dave.hansen@intel.com>
> cc: Michael Ellermen <mpe@ellerman.id.au>
> cc: Ingo Molnar <mingo@kernel.org>
> cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> ---
> History:
>
> v3: . Corrected a comment in arch_set_user_pkey_access().
> . Clarified the header, to capture the notion that
> pkey-0 permissions cannot be modified by userspace on
> powerpc. -- comment from Thiago
>
> v2: . mm_pkey_is_allocated() continued to treat pkey-0
> special. fixed it.
>
> arch/powerpc/include/asm/pkeys.h | 22 ++++++++++++++++++----
> arch/powerpc/mm/pkeys.c | 26 +++++++++++++++-----------
> 2 files changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pkeys.h
> b/arch/powerpc/include/asm/pkeys.h index 0409c80..31a6976 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -101,10 +101,14 @@ static inline u16 pte_to_pkey_bits(u64 pteflags)
>
> static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int
> pkey) {
> - /* A reserved key is never considered as 'explicitly
> allocated' */
> - return ((pkey < arch_max_pkey()) &&
> - !__mm_pkey_is_reserved(pkey) &&
> - __mm_pkey_is_allocated(mm, pkey));
> + if (pkey < 0 || pkey >= arch_max_pkey())
> + return false;
> +
> + /* Reserved keys are never allocated. */
> + if (__mm_pkey_is_reserved(pkey))
> + return false;
> +
> + return __mm_pkey_is_allocated(mm, pkey);
> }
>
> extern void __arch_activate_pkey(int pkey);
> @@ -200,6 +204,16 @@ static inline int
> arch_set_user_pkey_access(struct task_struct *tsk, int pkey, {
> if (static_branch_likely(&pkey_disabled))
> return -EINVAL;
> +
> + /*
> + * userspace should not change pkey-0 permissions.
> + * pkey-0 is associated with every page in the kernel.
> + * If userspace denies any permission on pkey-0, the
> + * kernel cannot operate.
> + */
> + if (!pkey)
> + return init_val ? -EINVAL : 0;
> +
> return __arch_set_user_pkey_access(tsk, pkey, init_val);
> }
>
> diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
> index 0eafdf0..d6873b4 100644
> --- a/arch/powerpc/mm/pkeys.c
> +++ b/arch/powerpc/mm/pkeys.c
> @@ -119,16 +119,21 @@ int pkey_initialize(void)
> #else
> os_reserved = 0;
> #endif
> + /* Bits are in LE format. */
> initial_allocation_mask = ~0x0;
> +
> + /* register mask is in BE format */
> pkey_amr_uamor_mask = ~0x0ul;
> pkey_iamr_mask = ~0x0ul;
> - /*
> - * key 0, 1 are reserved.
> - * key 0 is the default key, which allows read/write/execute.
> - * key 1 is recommended not to be used. PowerISA(3.0) page
> 1015,
> - * programming note.
> - */
> - for (i = 2; i < (pkeys_total - os_reserved); i++) {
> +
> + for (i = 0; i < (pkeys_total - os_reserved); i++) {
> + /*
> + * key 1 is recommended not to be used.
> + * PowerISA(3.0) page 1015,
> + */
> + if (i == 1)
> + continue;
> +
> initial_allocation_mask &= ~(0x1 << i);
> pkey_amr_uamor_mask &= ~(0x3ul << pkeyshift(i));
> pkey_iamr_mask &= ~(0x1ul << pkeyshift(i));
> @@ -142,7 +147,9 @@ void pkey_mm_init(struct mm_struct *mm)
> {
> if (static_branch_likely(&pkey_disabled))
> return;
> - mm_pkey_allocation_map(mm) = initial_allocation_mask;
> +
> + /* allocate key-0 by default */
> + mm_pkey_allocation_map(mm) = initial_allocation_mask | 0x1;
> /* -1 means unallocated or invalid */
> mm->context.execute_only_pkey = -1;
> }
> @@ -407,9 +414,6 @@ static bool pkey_access_permitted(int pkey, bool
> write, bool execute) int pkey_shift;
> u64 amr;
>
> - if (!pkey)
> - return true;
> -
> if (!is_pkey_enabled(pkey))
> return true;
>
^ permalink raw reply
* Re: [PATCH v3] powerpc, pkey: make protection key 0 less special
From: Dave Hansen @ 2018-05-04 21:31 UTC (permalink / raw)
To: Michal Suchánek, Ram Pai
Cc: mpe, Ulrich.Weigand, bsingharora, benh, mhocko, Ingo Molnar,
Thomas Gleixner, Andrew Morton, aneesh.kumar, bauerman,
linuxppc-dev, fweimer, paulus, hbabu
In-Reply-To: <20180504232647.3412f563@naga.suse.cz>
On 05/04/2018 02:26 PM, Michal Suchánek wrote:
> If it is not ok to change permissions of pkey 0 is it ok to free it?
It's pretty much never OK to free it on x86 or ppc. But, we're not
going to put code in to keep userspace from shooting itself in the foot,
at least on x86.
^ permalink raw reply
* Re: [PATCH v3] powerpc, pkey: make protection key 0 less special
From: Ram Pai @ 2018-05-04 21:45 UTC (permalink / raw)
To: Dave Hansen
Cc: Michal Suchánek, fweimer, Andrew Morton, mhocko,
Ulrich.Weigand, paulus, aneesh.kumar, bauerman, Thomas Gleixner,
linuxppc-dev, Ingo Molnar
In-Reply-To: <edd3d544-9fa8-5a79-4325-818ee5d7dfb0@intel.com>
On Fri, May 04, 2018 at 02:31:10PM -0700, Dave Hansen wrote:
> On 05/04/2018 02:26 PM, Michal Suchánek wrote:
> > If it is not ok to change permissions of pkey 0 is it ok to free it?
>
> It's pretty much never OK to free it on x86 or ppc. But, we're not
> going to put code in to keep userspace from shooting itself in the foot,
> at least on x86.
and on powerpc aswell.
--
Ram Pai
^ permalink raw reply
* [PATCH v13 0/3] mm, x86, powerpc: Enhancements to Memory Protection Keys.
From: Ram Pai @ 2018-05-04 21:59 UTC (permalink / raw)
To: mpe, mingo, akpm
Cc: linuxppc-dev, linux-mm, x86, linux-arch, linux-kernel,
dave.hansen, benh, paulus, khandual, aneesh.kumar, bsingharora,
hbabu, mhocko, bauerman, ebiederm, linuxram, corbet, arnd
This patch series provides arch-neutral enhancements to
enable memory-keys on new architecutes, and the corresponding
changes in x86 and powerpc specific code to support that.
a) Provides ability to support upto 32 keys. PowerPC
can handle 32 keys and hence needs this.
b) Arch-neutral code; and not the arch-specific code,
determines the format of the string, that displays the key
for each vma in smaps.
History:
-------
version 14:
(1) made VM_PKEY_BIT4 unusable on x86, #defined it to 0
-- comment by Dave Hansen
(2) due to some reason this patch series continue to
break some or the other build. The last series
passed everything but created a merge
conflict followed by build failure for
Michael Ellermen. :(
version v13:
(1) fixed a git bisect error. :(
version v12:
(1) fixed compilation errors seen with various x86
configs.
version v11:
(1) code that displays key in smaps is not any more
defined under CONFIG_ARCH_HAS_PKEYS.
- Comment by Eric W. Biederman and Michal Hocko
(2) merged two patches that implemented (1).
- comment by Michal Hocko
version prior to v11:
(1) used one additional bit from VM_HIGH_ARCH_*
to support 32 keys.
- Suggestion by Dave Hansen.
(2) powerpc specific changes to support memory keys.
Ram Pai (3):
mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS
is enabled
mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
mm, x86, powerpc: display pkey in smaps only if arch supports pkeys
arch/powerpc/include/asm/mmu_context.h | 5 -----
arch/powerpc/include/asm/pkeys.h | 2 ++
arch/x86/include/asm/mmu_context.h | 5 -----
arch/x86/include/asm/pkeys.h | 1 +
arch/x86/kernel/fpu/xstate.c | 5 +++++
arch/x86/kernel/setup.c | 8 --------
fs/proc/task_mmu.c | 16 +++++++++-------
include/linux/mm.h | 15 +++++++++++----
include/linux/pkeys.h | 7 ++++++-
9 files changed, 34 insertions(+), 30 deletions(-)
^ permalink raw reply
* [PATCH v13 1/3] mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled
From: Ram Pai @ 2018-05-04 21:59 UTC (permalink / raw)
To: mpe, mingo, akpm
Cc: linuxppc-dev, linux-mm, x86, linux-arch, linux-kernel,
dave.hansen, benh, paulus, khandual, aneesh.kumar, bsingharora,
hbabu, mhocko, bauerman, ebiederm, linuxram, corbet, arnd
In-Reply-To: <1525471183-21277-1-git-send-email-linuxram@us.ibm.com>
VM_PKEY_BITx are defined only if CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
is enabled. Powerpc also needs these bits. Hence lets define the
VM_PKEY_BITx bits for any architecture that enables
CONFIG_ARCH_HAS_PKEYS.
cc: Michael Ellermen <mpe@ellerman.id.au>
cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
cc: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/pkeys.h | 2 ++
fs/proc/task_mmu.c | 4 ++--
include/linux/mm.h | 9 +++++----
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 3a9b82b..425b181 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -26,6 +26,8 @@
# define VM_PKEY_BIT2 VM_HIGH_ARCH_2
# define VM_PKEY_BIT3 VM_HIGH_ARCH_3
# define VM_PKEY_BIT4 VM_HIGH_ARCH_4
+#elif !defined(VM_PKEY_BIT4)
+# define VM_PKEY_BIT4 VM_HIGH_ARCH_4
#endif
#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 65ae546..0c9e392 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -673,13 +673,13 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
[ilog2(VM_MERGEABLE)] = "mg",
[ilog2(VM_UFFD_MISSING)]= "um",
[ilog2(VM_UFFD_WP)] = "uw",
-#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
+#ifdef CONFIG_ARCH_HAS_PKEYS
/* These come out via ProtectionKey: */
[ilog2(VM_PKEY_BIT0)] = "",
[ilog2(VM_PKEY_BIT1)] = "",
[ilog2(VM_PKEY_BIT2)] = "",
[ilog2(VM_PKEY_BIT3)] = "",
-#endif
+#endif /* CONFIG_ARCH_HAS_PKEYS */
};
size_t i;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1ac1f06..c6a6f24 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -228,15 +228,16 @@ extern int overcommit_kbytes_handler(struct ctl_table *, int, void __user *,
#define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4)
#endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */
-#if defined(CONFIG_X86)
-# define VM_PAT VM_ARCH_1 /* PAT reserves whole VMA at once (x86) */
-#if defined (CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS)
+#ifdef CONFIG_ARCH_HAS_PKEYS
# define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
# define VM_PKEY_BIT0 VM_HIGH_ARCH_0 /* A protection key is a 4-bit value */
# define VM_PKEY_BIT1 VM_HIGH_ARCH_1
# define VM_PKEY_BIT2 VM_HIGH_ARCH_2
# define VM_PKEY_BIT3 VM_HIGH_ARCH_3
-#endif
+#endif /* CONFIG_ARCH_HAS_PKEYS */
+
+#if defined(CONFIG_X86)
+# define VM_PAT VM_ARCH_1 /* PAT reserves whole VMA at once (x86) */
#elif defined(CONFIG_PPC)
# define VM_SAO VM_ARCH_1 /* Strong Access Ordering (powerpc) */
#elif defined(CONFIG_PARISC)
--
1.7.1
^ permalink raw reply related
* [PATCH v13 3/3] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
From: Ram Pai @ 2018-05-04 21:59 UTC (permalink / raw)
To: mpe, mingo, akpm
Cc: linuxppc-dev, linux-mm, x86, linux-arch, linux-kernel,
dave.hansen, benh, paulus, khandual, aneesh.kumar, bsingharora,
hbabu, mhocko, bauerman, ebiederm, linuxram, corbet, arnd
In-Reply-To: <1525471183-21277-1-git-send-email-linuxram@us.ibm.com>
Only 4bits are allocated in the vma flags to hold 16 keys. This is
sufficient on x86. PowerPC supports 32 keys, which needs 5bits.
Allocate an additional bit.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Michael Ellermen <mpe@ellerman.id.au>
cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
cc: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
fs/proc/task_mmu.c | 1 +
include/linux/mm.h | 8 +++++++-
2 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 0c9e392..3ddddc7 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -679,6 +679,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
[ilog2(VM_PKEY_BIT1)] = "",
[ilog2(VM_PKEY_BIT2)] = "",
[ilog2(VM_PKEY_BIT3)] = "",
+ [ilog2(VM_PKEY_BIT4)] = "",
#endif /* CONFIG_ARCH_HAS_PKEYS */
};
size_t i;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index c6a6f24..cca67d1 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -230,10 +230,16 @@ extern int overcommit_kbytes_handler(struct ctl_table *, int, void __user *,
#ifdef CONFIG_ARCH_HAS_PKEYS
# define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
-# define VM_PKEY_BIT0 VM_HIGH_ARCH_0 /* A protection key is a 4-bit value */
+/* Protection key is a 4-bit value on x86 and 5-bit value on ppc64 */
+# define VM_PKEY_BIT0 VM_HIGH_ARCH_0
# define VM_PKEY_BIT1 VM_HIGH_ARCH_1
# define VM_PKEY_BIT2 VM_HIGH_ARCH_2
# define VM_PKEY_BIT3 VM_HIGH_ARCH_3
+#if defined(CONFIG_PPC)
+# define VM_PKEY_BIT4 VM_HIGH_ARCH_4
+#else
+# define VM_PKEY_BIT4 0
+#endif
#endif /* CONFIG_ARCH_HAS_PKEYS */
#if defined(CONFIG_X86)
--
1.7.1
^ permalink raw reply related
* [PATCH v11 3/3] mm, x86, powerpc: display pkey in smaps only if arch supports pkeys
From: Ram Pai @ 2018-05-04 21:59 UTC (permalink / raw)
To: mpe, mingo, akpm
Cc: linuxppc-dev, linux-mm, x86, linux-arch, linux-kernel,
dave.hansen, benh, paulus, khandual, aneesh.kumar, bsingharora,
hbabu, mhocko, bauerman, ebiederm, linuxram, corbet, arnd
In-Reply-To: <1525471183-21277-1-git-send-email-linuxram@us.ibm.com>
Currently the architecture specific code is expected to
display the protection keys in smap for a given vma.
This can lead to redundant code and possibly to divergent
formats in which the key gets displayed.
This patch changes the implementation. It displays the
pkey only if the architecture support pkeys, i.e
arch_pkeys_enabled() returns true. This patch
provides x86 implementation for arch_pkeys_enabled().
x86 arch_show_smap() function is not needed anymore.
Deleting it.
cc: Michael Ellermen <mpe@ellerman.id.au>
cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
cc: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
(fixed compilation errors for x86 configs)
Acked-by: Michal Hocko <mhocko@suse.com>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
arch/powerpc/include/asm/mmu_context.h | 5 -----
arch/x86/include/asm/mmu_context.h | 5 -----
arch/x86/include/asm/pkeys.h | 1 +
arch/x86/kernel/fpu/xstate.c | 5 +++++
arch/x86/kernel/setup.c | 8 --------
fs/proc/task_mmu.c | 11 ++++++-----
include/linux/pkeys.h | 7 ++++++-
7 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 1835ca1..896efa5 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -250,11 +250,6 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
#define thread_pkey_regs_restore(new_thread, old_thread)
#define thread_pkey_regs_init(thread)
-static inline int vma_pkey(struct vm_area_struct *vma)
-{
- return 0;
-}
-
static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
{
return 0x0UL;
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index 57e3785..3d748bd 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -296,11 +296,6 @@ static inline int vma_pkey(struct vm_area_struct *vma)
return (vma->vm_flags & vma_pkey_mask) >> VM_PKEY_SHIFT;
}
-#else
-static inline int vma_pkey(struct vm_area_struct *vma)
-{
- return 0;
-}
#endif
/*
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
index a0ba1ff..f6c287b 100644
--- a/arch/x86/include/asm/pkeys.h
+++ b/arch/x86/include/asm/pkeys.h
@@ -6,6 +6,7 @@
extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
unsigned long init_val);
+extern bool arch_pkeys_enabled(void);
/*
* Try to dedicate one of the protection keys to be used as an
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 87a57b7..4f566e9 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -945,6 +945,11 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
return 0;
}
+
+bool arch_pkeys_enabled(void)
+{
+ return boot_cpu_has(X86_FEATURE_OSPKE);
+}
#endif /* ! CONFIG_ARCH_HAS_PKEYS */
/*
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 6285697..960dbab 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1306,11 +1306,3 @@ static int __init register_kernel_offset_dumper(void)
return 0;
}
__initcall(register_kernel_offset_dumper);
-
-void arch_show_smap(struct seq_file *m, struct vm_area_struct *vma)
-{
- if (!boot_cpu_has(X86_FEATURE_OSPKE))
- return;
-
- seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
-}
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 3ddddc7..9ce0097 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -18,10 +18,12 @@
#include <linux/page_idle.h>
#include <linux/shmem_fs.h>
#include <linux/uaccess.h>
+#include <linux/pkeys.h>
#include <asm/elf.h>
#include <asm/tlb.h>
#include <asm/tlbflush.h>
+#include <asm/mmu_context.h>
#include "internal.h"
#define SEQ_PUT_DEC(str, val) \
@@ -728,12 +730,9 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long hmask,
}
#endif /* HUGETLB_PAGE */
-void __weak arch_show_smap(struct seq_file *m, struct vm_area_struct *vma)
-{
-}
-
#define SEQ_PUT_DEC(str, val) \
seq_put_decimal_ull_width(m, str, (val) >> 10, 8)
+
static int show_smap(struct seq_file *m, void *v, int is_pid)
{
struct proc_maps_private *priv = m->private;
@@ -836,9 +835,11 @@ static int show_smap(struct seq_file *m, void *v, int is_pid)
seq_puts(m, " kB\n");
}
if (!rollup_mode) {
- arch_show_smap(m, vma);
+ if (arch_pkeys_enabled())
+ seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
show_smap_vma_flags(m, vma);
}
+
m_cache_vma(m, vma);
return ret;
}
diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
index 0794ca7..49dff15 100644
--- a/include/linux/pkeys.h
+++ b/include/linux/pkeys.h
@@ -3,7 +3,6 @@
#define _LINUX_PKEYS_H
#include <linux/mm_types.h>
-#include <asm/mmu_context.h>
#ifdef CONFIG_ARCH_HAS_PKEYS
#include <asm/pkeys.h>
@@ -13,6 +12,7 @@
#define arch_override_mprotect_pkey(vma, prot, pkey) (0)
#define PKEY_DEDICATED_EXECUTE_ONLY 0
#define ARCH_VM_PKEY_FLAGS 0
+#define vma_pkey(vma) 0
static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
{
@@ -35,6 +35,11 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
return 0;
}
+static inline bool arch_pkeys_enabled(void)
+{
+ return false;
+}
+
static inline void copy_init_pkru_to_fpregs(void)
{
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 4/4] powerpc/xive: prepare all hcalls to support long busy delays
From: Benjamin Herrenschmidt @ 2018-05-04 22:29 UTC (permalink / raw)
To: Michael Ellerman, Cédric Le Goater, linuxppc-dev
In-Reply-To: <87d0yb8zye.fsf@concordia.ellerman.id.au>
On Fri, 2018-05-04 at 20:42 +1000, Michael Ellerman wrote:
> Cédric Le Goater <clg@kaod.org> writes:
>
> > This is not the case for the moment, but future releases of pHyp might
> > need to introduce some synchronisation routines under the hood which
> > would make the XIVE hcalls longer to complete.
> >
> > As this was done for H_INT_RESET, let's wrap the other hcalls in a
> > loop catching the H_LONG_BUSY_* codes.
>
> Are we sure it's safe to msleep() in all these paths?
Probably not. We can have the IRQ descriptor lock. We might need to
mdelay.
There's a Kconfig option (forgot which one) that will add checks for
attempts to sleep inside locks, you should run with that.
Cheers,
Ben.
>
> cheers
>
> > diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
> > index 7113f5d87952..97ea0a67a173 100644
> > --- a/arch/powerpc/sysdev/xive/spapr.c
> > +++ b/arch/powerpc/sysdev/xive/spapr.c
> > @@ -165,7 +165,10 @@ static long plpar_int_get_source_info(unsigned long flags,
> > unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
> > long rc;
> >
> > - rc = plpar_hcall(H_INT_GET_SOURCE_INFO, retbuf, flags, lisn);
> > + do {
> > + rc = plpar_hcall(H_INT_GET_SOURCE_INFO, retbuf, flags, lisn);
> > + } while (plpar_busy_delay(rc));
> > +
> > if (rc) {
> > pr_err("H_INT_GET_SOURCE_INFO lisn=%ld failed %ld\n", lisn, rc);
> > return rc;
> > @@ -198,8 +201,11 @@ static long plpar_int_set_source_config(unsigned long flags,
> > flags, lisn, target, prio, sw_irq);
> >
> >
> > - rc = plpar_hcall_norets(H_INT_SET_SOURCE_CONFIG, flags, lisn,
> > - target, prio, sw_irq);
> > + do {
> > + rc = plpar_hcall_norets(H_INT_SET_SOURCE_CONFIG, flags, lisn,
> > + target, prio, sw_irq);
> > + } while (plpar_busy_delay(rc));
> > +
> > if (rc) {
> > pr_err("H_INT_SET_SOURCE_CONFIG lisn=%ld target=%lx prio=%lx failed %ld\n",
> > lisn, target, prio, rc);
> > @@ -218,7 +224,11 @@ static long plpar_int_get_queue_info(unsigned long flags,
> > unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
> > long rc;
> >
> > - rc = plpar_hcall(H_INT_GET_QUEUE_INFO, retbuf, flags, target, priority);
> > + do {
> > + rc = plpar_hcall(H_INT_GET_QUEUE_INFO, retbuf, flags, target,
> > + priority);
> > + } while (plpar_busy_delay(rc));
> > +
> > if (rc) {
> > pr_err("H_INT_GET_QUEUE_INFO cpu=%ld prio=%ld failed %ld\n",
> > target, priority, rc);
> > @@ -247,8 +257,11 @@ static long plpar_int_set_queue_config(unsigned long flags,
> > pr_devel("H_INT_SET_QUEUE_CONFIG flags=%lx target=%lx priority=%lx qpage=%lx qsize=%lx\n",
> > flags, target, priority, qpage, qsize);
> >
> > - rc = plpar_hcall_norets(H_INT_SET_QUEUE_CONFIG, flags, target,
> > - priority, qpage, qsize);
> > + do {
> > + rc = plpar_hcall_norets(H_INT_SET_QUEUE_CONFIG, flags, target,
> > + priority, qpage, qsize);
> > + } while (plpar_busy_delay(rc));
> > +
> > if (rc) {
> > pr_err("H_INT_SET_QUEUE_CONFIG cpu=%ld prio=%ld qpage=%lx returned %ld\n",
> > target, priority, qpage, rc);
> > @@ -262,7 +275,10 @@ static long plpar_int_sync(unsigned long flags, unsigned long lisn)
> > {
> > long rc;
> >
> > - rc = plpar_hcall_norets(H_INT_SYNC, flags, lisn);
> > + do {
> > + rc = plpar_hcall_norets(H_INT_SYNC, flags, lisn);
> > + } while (plpar_busy_delay(rc));
> > +
> > if (rc) {
> > pr_err("H_INT_SYNC lisn=%ld returned %ld\n", lisn, rc);
> > return rc;
> > @@ -285,7 +301,11 @@ static long plpar_int_esb(unsigned long flags,
> > pr_devel("H_INT_ESB flags=%lx lisn=%lx offset=%lx in=%lx\n",
> > flags, lisn, offset, in_data);
> >
> > - rc = plpar_hcall(H_INT_ESB, retbuf, flags, lisn, offset, in_data);
> > + do {
> > + rc = plpar_hcall(H_INT_ESB, retbuf, flags, lisn, offset,
> > + in_data);
> > + } while (plpar_busy_delay(rc));
> > +
> > if (rc) {
> > pr_err("H_INT_ESB lisn=%ld offset=%ld returned %ld\n",
> > lisn, offset, rc);
> > --
> > 2.13.6
^ permalink raw reply
* Re: [PATCH v13 3/3] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
From: Dave Hansen @ 2018-05-04 22:57 UTC (permalink / raw)
To: Ram Pai, mpe, mingo, akpm
Cc: linuxppc-dev, linux-mm, x86, linux-arch, linux-kernel, benh,
paulus, khandual, aneesh.kumar, bsingharora, hbabu, mhocko,
bauerman, ebiederm, corbet, arnd
In-Reply-To: <1525471183-21277-3-git-send-email-linuxram@us.ibm.com>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 0c9e392..3ddddc7 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -679,6 +679,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
> [ilog2(VM_PKEY_BIT1)] = "",
> [ilog2(VM_PKEY_BIT2)] = "",
> [ilog2(VM_PKEY_BIT3)] = "",
> + [ilog2(VM_PKEY_BIT4)] = "",
> #endif /* CONFIG_ARCH_HAS_PKEYS */
...
> +#if defined(CONFIG_PPC)
> +# define VM_PKEY_BIT4 VM_HIGH_ARCH_4
> +#else
> +# define VM_PKEY_BIT4 0
> +#endif
> #endif /* CONFIG_ARCH_HAS_PKEYS */
That new line boils down to:
[ilog2(0)] = "",
on x86. It wasn't *obvious* to me that it is OK to do that. The other
possibly undefined bits (VM_SOFTDIRTY for instance) #ifdef themselves
out of this array.
I would just be a wee bit worried that this would overwrite the 0 entry
("??") with "".
^ permalink raw reply
* Re: [PATCH v13 3/3] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
From: Ram Pai @ 2018-05-05 1:12 UTC (permalink / raw)
To: Dave Hansen
Cc: mpe, mingo, akpm, linuxppc-dev, linux-mm, x86, linux-arch,
linux-kernel, benh, paulus, khandual, aneesh.kumar, bsingharora,
hbabu, mhocko, bauerman, ebiederm, corbet, arnd
In-Reply-To: <1e37895e-5a18-11c1-58f1-834f96dfd4d5@intel.com>
On Fri, May 04, 2018 at 03:57:33PM -0700, Dave Hansen wrote:
> > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> > index 0c9e392..3ddddc7 100644
> > --- a/fs/proc/task_mmu.c
> > +++ b/fs/proc/task_mmu.c
> > @@ -679,6 +679,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
> > [ilog2(VM_PKEY_BIT1)] = "",
> > [ilog2(VM_PKEY_BIT2)] = "",
> > [ilog2(VM_PKEY_BIT3)] = "",
> > + [ilog2(VM_PKEY_BIT4)] = "",
> > #endif /* CONFIG_ARCH_HAS_PKEYS */
> ...
> > +#if defined(CONFIG_PPC)
> > +# define VM_PKEY_BIT4 VM_HIGH_ARCH_4
> > +#else
> > +# define VM_PKEY_BIT4 0
> > +#endif
> > #endif /* CONFIG_ARCH_HAS_PKEYS */
>
> That new line boils down to:
>
> [ilog2(0)] = "",
>
> on x86. It wasn't *obvious* to me that it is OK to do that. The other
> possibly undefined bits (VM_SOFTDIRTY for instance) #ifdef themselves
> out of this array.
>
> I would just be a wee bit worried that this would overwrite the 0 entry
> ("??") with "".
Yes it would :-( and could potentially break anything that depends on
0th entry being "??"
Is the following fix acceptable?
#if VM_PKEY_BIT4
[ilog2(VM_PKEY_BIT4)] = "",
#endif
--
Ram Pai
^ permalink raw reply
* [PATCH] powerpc: cpm_gpio: Remove owner assignment from platform_driver
From: Fabio Estevam @ 2018-05-05 3:01 UTC (permalink / raw)
To: mpe; +Cc: benh, paulus, linuxppc-dev, Fabio Estevam
From: Fabio Estevam <fabio.estevam@nxp.com>
Structure platform_driver does not need to set the owner field, as this
will be populated by the driver core.
Generated by scripts/coccinelle/api/platform_no_drv_owner.cocci.
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
arch/powerpc/sysdev/cpm_gpio.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/sysdev/cpm_gpio.c b/arch/powerpc/sysdev/cpm_gpio.c
index 0badc90..0695d26 100644
--- a/arch/powerpc/sysdev/cpm_gpio.c
+++ b/arch/powerpc/sysdev/cpm_gpio.c
@@ -63,7 +63,6 @@ static struct platform_driver cpm_gpio_driver = {
.probe = cpm_gpio_probe,
.driver = {
.name = "cpm-gpio",
- .owner = THIS_MODULE,
.of_match_table = cpm_gpio_match,
},
};
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v13 3/3] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
From: Dave Hansen @ 2018-05-05 4:42 UTC (permalink / raw)
To: Ram Pai
Cc: mpe, mingo, akpm, linuxppc-dev, linux-mm, x86, linux-arch,
linux-kernel, benh, paulus, khandual, aneesh.kumar, bsingharora,
hbabu, mhocko, bauerman, ebiederm, corbet, arnd
In-Reply-To: <20180505011243.GB5617@ram.oc3035372033.ibm.com>
On 05/04/2018 06:12 PM, Ram Pai wrote:
>> That new line boils down to:
>>
>> [ilog2(0)] = "",
>>
>> on x86. It wasn't *obvious* to me that it is OK to do that. The other
>> possibly undefined bits (VM_SOFTDIRTY for instance) #ifdef themselves
>> out of this array.
>>
>> I would just be a wee bit worried that this would overwrite the 0 entry
>> ("??") with "".
> Yes it would :-( and could potentially break anything that depends on
> 0th entry being "??"
>
> Is the following fix acceptable?
>
> #if VM_PKEY_BIT4
> [ilog2(VM_PKEY_BIT4)] = "",
> #endif
Yep, I think that works for me.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox