* Re: [PATCH v8 13/30] powerpc: Add a probe_user_read_inst() function
From: Jordan Niethe @ 2020-05-13 23:51 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, Alistair Popple, Nicholas Piggin, Balamuruhan S,
naveen.n.rao, linuxppc-dev, Daniel Axtens
In-Reply-To: <87blmsatet.fsf@mpe.ellerman.id.au>
On Wed, May 13, 2020 at 10:52 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Jordan Niethe <jniethe5@gmail.com> writes:
> > diff --git a/arch/powerpc/lib/inst.c b/arch/powerpc/lib/inst.c
> > new file mode 100644
> > index 000000000000..eaf786afad2b
> > --- /dev/null
> > +++ b/arch/powerpc/lib/inst.c
> > @@ -0,0 +1,18 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright 2020, IBM Corporation.
> > + */
> > +
> > +#include <linux/uaccess.h>
> > +#include <asm/inst.h>
> > +
> > +int probe_user_read_inst(struct ppc_inst *inst,
> > + struct ppc_inst *nip)
> > +{
> > + unsigned int val;
> > + int err;
> > +
> > + err = probe_user_read(&val, nip, sizeof(val));
> > + *inst = ppc_inst(val);
>
> We shouldn't be storing to *inst if the read failed?
Good point.
>
> I changed it to:
>
> + if (!err)
> + *inst = ppc_inst(val);
> +
>
> Similarly for probe_kernel_read_inst().
Thanks.
>
> cheers
^ permalink raw reply
* Re: [PATCH v8 16/30] powerpc: Define and use __get_user_instr{, inatomic}()
From: Jordan Niethe @ 2020-05-13 23:54 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, Alistair Popple, Nicholas Piggin, Balamuruhan S,
naveen.n.rao, linuxppc-dev, Daniel Axtens
In-Reply-To: <878shvc40x.fsf@mpe.ellerman.id.au>
On Thu, May 14, 2020 at 12:17 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Jordan Niethe <jniethe5@gmail.com> writes:
> > Define specific __get_user_instr() and __get_user_instr_inatomic()
> > macros for reading instructions from user space.
>
> At least for fix_alignment() we could be coming from the kernel, not
> sure about the other cases.
>
> I can tweak the change log.
>
> > diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> > index 2f500debae21..c0a35e4586a5 100644
> > --- a/arch/powerpc/include/asm/uaccess.h
> > +++ b/arch/powerpc/include/asm/uaccess.h
> > @@ -105,6 +105,11 @@ static inline int __access_ok(unsigned long addr, unsigned long size,
> > #define __put_user_inatomic(x, ptr) \
> > __put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
> >
> > +#define __get_user_instr(x, ptr) \
> > + __get_user_nocheck((x).val, (u32 *)(ptr), sizeof(u32), true)
> > +
> > +#define __get_user_instr_inatomic(x, ptr) \
> > + __get_user_nosleep((x).val, (u32 *)(ptr), sizeof(u32))
>
> I'm not super keen on adding new __ versions, which lack the access_ok()
> check, but I guess we have to.
>
> > diff --git a/arch/powerpc/kernel/vecemu.c b/arch/powerpc/kernel/vecemu.c
> > index 3dd70eeb10c5..60ed5aea8d4e 100644
> > --- a/arch/powerpc/kernel/vecemu.c
> > +++ b/arch/powerpc/kernel/vecemu.c
> > @@ -266,7 +266,7 @@ int emulate_altivec(struct pt_regs *regs)
> > unsigned int va, vb, vc, vd;
> > vector128 *vrs;
> >
> > - if (get_user(instr.val, (unsigned int __user *)regs->nip))
> > + if (__get_user_instr(instr, (void __user *)regs->nip))
> > return -EFAULT;
>
> That drops the access_ok() check, which is not OK, at least without a
> reasonable justification.
>
> Given it's regs->nip I guess it should be safe, but it should still be
> called out. Or preferably switched to __get_user() in a precursor patch.
Or should I add a get_user_instr() that includes the check?
>
> cheers
^ permalink raw reply
* [PATCH v3 0/2] Implement reentrant rtas call
From: Leonardo Bras @ 2020-05-14 1:12 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Allison Randal, Greg Kroah-Hartman, Thomas Gleixner,
Nicholas Piggin, Leonardo Bras, Nathan Lynch, Gautham R. Shenoy,
Nadav Amit
Cc: linuxppc-dev, linux-kernel
Patch 2 implement rtas_call_reentrant() for reentrant rtas-calls:
"ibm,int-on", "ibm,int-off",ibm,get-xive" and "ibm,set-xive",
according to LoPAPR Version 1.1 (March 24, 2016).
For that, it's necessary that every call uses a different
rtas buffer (rtas_args). Paul Mackerras suggested using the PACA
structure for creating a per-cpu buffer for these calls.
Patch 1 was necessary to make PACA have a 'struct rtas_args' member.
Reentrant rtas calls can be useful to avoid deadlocks in crashing,
where rtas-calls are needed, but some other thread crashed holding
the rtas.lock.
This is a backtrace of a deadlock from a kdump testing environment:
#0 arch_spin_lock
#1 lock_rtas ()
#2 rtas_call (token=8204, nargs=1, nret=1, outputs=0x0)
#3 ics_rtas_mask_real_irq (hw_irq=4100)
#4 machine_kexec_mask_interrupts
#5 default_machine_crash_shutdown
#6 machine_crash_shutdown
#7 __crash_kexec
#8 crash_kexec
#9 oops_end
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
---
Changes since v2:
- Fixed build failure from ppc64e, by including spinlock_types.h on
rtas-types.h
- Improved commit messages
Changes since v1:
- Moved buffer from stack to PACA (as suggested by Paul Mackerras)
- Added missing output bits
- Improve documentation following kernel-doc format (as suggested by
Nathan Lynch)
Leonardo Bras (2):
powerpc/rtas: Move type/struct definitions from rtas.h into
rtas-types.h
powerpc/rtas: Implement reentrant rtas call
arch/powerpc/include/asm/paca.h | 2 +
arch/powerpc/include/asm/rtas-types.h | 126 ++++++++++++++++++++++++++
arch/powerpc/include/asm/rtas.h | 119 +-----------------------
arch/powerpc/kernel/rtas.c | 42 +++++++++
arch/powerpc/sysdev/xics/ics-rtas.c | 22 ++---
5 files changed, 183 insertions(+), 128 deletions(-)
create mode 100644 arch/powerpc/include/asm/rtas-types.h
--
2.25.4
^ permalink raw reply
* [PATCH v3 1/2] powerpc/rtas: Move type/struct definitions from rtas.h into rtas-types.h
From: Leonardo Bras @ 2020-05-14 1:12 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Allison Randal, Greg Kroah-Hartman, Thomas Gleixner,
Nicholas Piggin, Leonardo Bras, Nathan Lynch, Gautham R. Shenoy,
Nadav Amit
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200514011245.127174-1-leobras.c@gmail.com>
In order to get any rtas* struct into other headers, including rtas.h
may cause a lot of errors, regarding include dependency needed for
inline functions.
Create rtas-types.h and move there all type/struct definitions
from rtas.h, then include rtas-types.h into rtas.h.
Also, as suggested by checkpath.pl, replace uint8_t for u8, and keep
the same type pattern for the whole file, as they are the same
according to powerpc/boot/types.h.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
---
arch/powerpc/include/asm/rtas-types.h | 126 ++++++++++++++++++++++++++
arch/powerpc/include/asm/rtas.h | 118 +-----------------------
2 files changed, 127 insertions(+), 117 deletions(-)
create mode 100644 arch/powerpc/include/asm/rtas-types.h
diff --git a/arch/powerpc/include/asm/rtas-types.h b/arch/powerpc/include/asm/rtas-types.h
new file mode 100644
index 000000000000..87354e28f160
--- /dev/null
+++ b/arch/powerpc/include/asm/rtas-types.h
@@ -0,0 +1,126 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _POWERPC_RTAS_TYPES_H
+#define _POWERPC_RTAS_TYPES_H
+#ifdef __KERNEL__
+
+#include <linux/spinlock_types.h>
+
+typedef __be32 rtas_arg_t;
+
+struct rtas_args {
+ __be32 token;
+ __be32 nargs;
+ __be32 nret;
+ rtas_arg_t args[16];
+ rtas_arg_t *rets; /* Pointer to return values in args[]. */
+};
+
+struct rtas_t {
+ unsigned long entry; /* physical address pointer */
+ unsigned long base; /* physical address pointer */
+ unsigned long size;
+ arch_spinlock_t lock;
+ struct rtas_args args;
+ struct device_node *dev; /* virtual address pointer */
+};
+
+struct rtas_suspend_me_data {
+ atomic_t working; /* number of cpus accessing this struct */
+ atomic_t done;
+ int token; /* ibm,suspend-me */
+ atomic_t error;
+ struct completion *complete; /* wait on this until working == 0 */
+};
+
+struct rtas_error_log {
+ /* Byte 0 */
+ u8 byte0; /* Architectural version */
+
+ /* Byte 1 */
+ u8 byte1;
+ /* XXXXXXXX
+ * XXX 3: Severity level of error
+ * XX 2: Degree of recovery
+ * X 1: Extended log present?
+ * XX 2: Reserved
+ */
+
+ /* Byte 2 */
+ u8 byte2;
+ /* XXXXXXXX
+ * XXXX 4: Initiator of event
+ * XXXX 4: Target of failed operation
+ */
+ u8 byte3; /* General event or error*/
+ __be32 extended_log_length; /* length in bytes */
+ unsigned char buffer[1]; /* Start of extended log */
+ /* Variable length. */
+};
+
+/* RTAS general extended event log, Version 6. The extended log starts
+ * from "buffer" field of struct rtas_error_log defined above.
+ */
+struct rtas_ext_event_log_v6 {
+ /* Byte 0 */
+ u8 byte0;
+ /* XXXXXXXX
+ * X 1: Log valid
+ * X 1: Unrecoverable error
+ * X 1: Recoverable (correctable or successfully retried)
+ * X 1: Bypassed unrecoverable error (degraded operation)
+ * X 1: Predictive error
+ * X 1: "New" log (always 1 for data returned from RTAS)
+ * X 1: Big Endian
+ * X 1: Reserved
+ */
+
+ /* Byte 1 */
+ u8 byte1; /* reserved */
+
+ /* Byte 2 */
+ u8 byte2;
+ /* XXXXXXXX
+ * X 1: Set to 1 (indicating log is in PowerPC format)
+ * XXX 3: Reserved
+ * XXXX 4: Log format used for bytes 12-2047
+ */
+
+ /* Byte 3 */
+ u8 byte3; /* reserved */
+ /* Byte 4-11 */
+ u8 reserved[8]; /* reserved */
+ /* Byte 12-15 */
+ __be32 company_id; /* Company ID of the company */
+ /* that defines the format for */
+ /* the vendor specific log type */
+ /* Byte 16-end of log */
+ u8 vendor_log[1]; /* Start of vendor specific log */
+ /* Variable length. */
+};
+
+/* Vendor specific Platform Event Log Format, Version 6, section header */
+struct pseries_errorlog {
+ __be16 id; /* 0x00 2-byte ASCII section ID */
+ __be16 length; /* 0x02 Section length in bytes */
+ u8 version; /* 0x04 Section version */
+ u8 subtype; /* 0x05 Section subtype */
+ __be16 creator_component; /* 0x06 Creator component ID */
+ u8 data[]; /* 0x08 Start of section data */
+};
+
+/* RTAS pseries hotplug errorlog section */
+struct pseries_hp_errorlog {
+ u8 resource;
+ u8 action;
+ u8 id_type;
+ u8 reserved;
+ union {
+ __be32 drc_index;
+ __be32 drc_count;
+ struct { __be32 count, index; } ic;
+ char drc_name[1];
+ } _drc_u;
+};
+
+#endif /* __KERNEL__ */
+#endif /* _POWERPC_RTAS_TYPES_H */
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 3c1887351c71..c35c5350b7e4 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -5,6 +5,7 @@
#include <linux/spinlock.h>
#include <asm/page.h>
+#include <asm/rtas-types.h>
#include <linux/time.h>
#include <linux/cpumask.h>
@@ -42,33 +43,6 @@
*
*/
-typedef __be32 rtas_arg_t;
-
-struct rtas_args {
- __be32 token;
- __be32 nargs;
- __be32 nret;
- rtas_arg_t args[16];
- rtas_arg_t *rets; /* Pointer to return values in args[]. */
-};
-
-struct rtas_t {
- unsigned long entry; /* physical address pointer */
- unsigned long base; /* physical address pointer */
- unsigned long size;
- arch_spinlock_t lock;
- struct rtas_args args;
- struct device_node *dev; /* virtual address pointer */
-};
-
-struct rtas_suspend_me_data {
- atomic_t working; /* number of cpus accessing this struct */
- atomic_t done;
- int token; /* ibm,suspend-me */
- atomic_t error;
- struct completion *complete; /* wait on this until working == 0 */
-};
-
/* RTAS event classes */
#define RTAS_INTERNAL_ERROR 0x80000000 /* set bit 0 */
#define RTAS_EPOW_WARNING 0x40000000 /* set bit 1 */
@@ -148,31 +122,6 @@ struct rtas_suspend_me_data {
/* RTAS check-exception vector offset */
#define RTAS_VECTOR_EXTERNAL_INTERRUPT 0x500
-struct rtas_error_log {
- /* Byte 0 */
- uint8_t byte0; /* Architectural version */
-
- /* Byte 1 */
- uint8_t byte1;
- /* XXXXXXXX
- * XXX 3: Severity level of error
- * XX 2: Degree of recovery
- * X 1: Extended log present?
- * XX 2: Reserved
- */
-
- /* Byte 2 */
- uint8_t byte2;
- /* XXXXXXXX
- * XXXX 4: Initiator of event
- * XXXX 4: Target of failed operation
- */
- uint8_t byte3; /* General event or error*/
- __be32 extended_log_length; /* length in bytes */
- unsigned char buffer[1]; /* Start of extended log */
- /* Variable length. */
-};
-
static inline uint8_t rtas_error_severity(const struct rtas_error_log *elog)
{
return (elog->byte1 & 0xE0) >> 5;
@@ -212,47 +161,6 @@ uint32_t rtas_error_extended_log_length(const struct rtas_error_log *elog)
#define RTAS_V6EXT_COMPANY_ID_IBM (('I' << 24) | ('B' << 16) | ('M' << 8))
-/* RTAS general extended event log, Version 6. The extended log starts
- * from "buffer" field of struct rtas_error_log defined above.
- */
-struct rtas_ext_event_log_v6 {
- /* Byte 0 */
- uint8_t byte0;
- /* XXXXXXXX
- * X 1: Log valid
- * X 1: Unrecoverable error
- * X 1: Recoverable (correctable or successfully retried)
- * X 1: Bypassed unrecoverable error (degraded operation)
- * X 1: Predictive error
- * X 1: "New" log (always 1 for data returned from RTAS)
- * X 1: Big Endian
- * X 1: Reserved
- */
-
- /* Byte 1 */
- uint8_t byte1; /* reserved */
-
- /* Byte 2 */
- uint8_t byte2;
- /* XXXXXXXX
- * X 1: Set to 1 (indicating log is in PowerPC format)
- * XXX 3: Reserved
- * XXXX 4: Log format used for bytes 12-2047
- */
-
- /* Byte 3 */
- uint8_t byte3; /* reserved */
- /* Byte 4-11 */
- uint8_t reserved[8]; /* reserved */
- /* Byte 12-15 */
- __be32 company_id; /* Company ID of the company */
- /* that defines the format for */
- /* the vendor specific log type */
- /* Byte 16-end of log */
- uint8_t vendor_log[1]; /* Start of vendor specific log */
- /* Variable length. */
-};
-
static
inline uint8_t rtas_ext_event_log_format(struct rtas_ext_event_log_v6 *ext_log)
{
@@ -287,16 +195,6 @@ inline uint32_t rtas_ext_event_company_id(struct rtas_ext_event_log_v6 *ext_log)
#define PSERIES_ELOG_SECT_ID_HOTPLUG (('H' << 8) | 'P')
#define PSERIES_ELOG_SECT_ID_MCE (('M' << 8) | 'C')
-/* Vendor specific Platform Event Log Format, Version 6, section header */
-struct pseries_errorlog {
- __be16 id; /* 0x00 2-byte ASCII section ID */
- __be16 length; /* 0x02 Section length in bytes */
- uint8_t version; /* 0x04 Section version */
- uint8_t subtype; /* 0x05 Section subtype */
- __be16 creator_component; /* 0x06 Creator component ID */
- uint8_t data[]; /* 0x08 Start of section data */
-};
-
static
inline uint16_t pseries_errorlog_id(struct pseries_errorlog *sect)
{
@@ -309,20 +207,6 @@ inline uint16_t pseries_errorlog_length(struct pseries_errorlog *sect)
return be16_to_cpu(sect->length);
}
-/* RTAS pseries hotplug errorlog section */
-struct pseries_hp_errorlog {
- u8 resource;
- u8 action;
- u8 id_type;
- u8 reserved;
- union {
- __be32 drc_index;
- __be32 drc_count;
- struct { __be32 count, index; } ic;
- char drc_name[1];
- } _drc_u;
-};
-
#define PSERIES_HP_ELOG_RESOURCE_CPU 1
#define PSERIES_HP_ELOG_RESOURCE_MEM 2
#define PSERIES_HP_ELOG_RESOURCE_SLOT 3
--
2.25.4
^ permalink raw reply related
* [PATCH v3 2/2] powerpc/rtas: Implement reentrant rtas call
From: Leonardo Bras @ 2020-05-14 1:12 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Allison Randal, Greg Kroah-Hartman, Thomas Gleixner,
Nicholas Piggin, Leonardo Bras, Nathan Lynch, Gautham R. Shenoy,
Nadav Amit
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200514011245.127174-1-leobras.c@gmail.com>
Implement rtas_call_reentrant() for reentrant rtas-calls:
"ibm,int-on", "ibm,int-off",ibm,get-xive" and "ibm,set-xive".
On LoPAPR Version 1.1 (March 24, 2016), from 7.3.10.1 to 7.3.10.4,
items 2 and 3 say:
2 - For the PowerPC External Interrupt option: The * call must be
reentrant to the number of processors on the platform.
3 - For the PowerPC External Interrupt option: The * argument call
buffer for each simultaneous call must be physically unique.
So, these rtas-calls can be called in a lockless way, if using
a different buffer for each cpu doing such rtas call.
For this, it was suggested to add the buffer (struct rtas_args)
in the PACA struct, so each cpu can have it's own buffer.
Reentrant rtas calls are useful to avoid deadlocks in crashing,
where rtas-calls are needed, but some other thread crashed holding
the rtas.lock.
This is a backtrace of a deadlock from a kdump testing environment:
#0 arch_spin_lock
#1 lock_rtas ()
#2 rtas_call (token=8204, nargs=1, nret=1, outputs=0x0)
#3 ics_rtas_mask_real_irq (hw_irq=4100)
#4 machine_kexec_mask_interrupts
#5 default_machine_crash_shutdown
#6 machine_crash_shutdown
#7 __crash_kexec
#8 crash_kexec
#9 oops_end
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
---
arch/powerpc/include/asm/paca.h | 2 ++
arch/powerpc/include/asm/rtas.h | 1 +
arch/powerpc/kernel/rtas.c | 42 +++++++++++++++++++++++++++++
arch/powerpc/sysdev/xics/ics-rtas.c | 22 +++++++--------
4 files changed, 56 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index e3cc9eb9204d..5a76ba50b40f 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -29,6 +29,7 @@
#include <asm/hmi.h>
#include <asm/cpuidle.h>
#include <asm/atomic.h>
+#include <asm/rtas-types.h>
#include <asm-generic/mmiowb_types.h>
@@ -270,6 +271,7 @@ struct paca_struct {
#ifdef CONFIG_MMIOWB
struct mmiowb_state mmiowb_state;
#endif
+ struct rtas_args reentrant_args;
} ____cacheline_aligned;
extern void copy_mm_to_paca(struct mm_struct *mm);
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index c35c5350b7e4..fa7509c85881 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -236,6 +236,7 @@ extern struct rtas_t rtas;
extern int rtas_token(const char *service);
extern int rtas_service_present(const char *service);
extern int rtas_call(int token, int, int, int *, ...);
+int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...);
void rtas_call_unlocked(struct rtas_args *args, int token, int nargs,
int nret, ...);
extern void __noreturn rtas_restart(char *cmd);
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index c5fa251b8950..d426b5c4856c 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -41,6 +41,7 @@
#include <asm/time.h>
#include <asm/mmu.h>
#include <asm/topology.h>
+#include <asm/paca.h>
/* This is here deliberately so it's only used in this file */
void enter_rtas(unsigned long);
@@ -483,6 +484,47 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
}
EXPORT_SYMBOL(rtas_call);
+/**
+ * rtas_call_reentrant() - Used for reentrant rtas calls
+ * @token: Token for desired reentrant RTAS call
+ * @nargs: Number of Input Parameters
+ * @nret: Number of Output Parameters
+ * @outputs: Array of outputs
+ * @...: Inputs for desired RTAS call
+ *
+ * According to LoPAR documentation, only "ibm,int-on", "ibm,int-off",
+ * "ibm,get-xive" and "ibm,set-xive" are currently reentrant.
+ * Reentrant calls need their own rtas_args buffer, so not using rtas.args, but
+ * PACA one instead.
+ *
+ * Return: -1 on error,
+ * First output value of RTAS call if (nret > 0),
+ * 0 otherwise,
+ */
+
+int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...)
+{
+ va_list list;
+ struct rtas_args *args;
+ int i;
+
+ if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
+ return -1;
+
+ /* We use the per-cpu (PACA) rtas args buffer */
+ args = &local_paca->reentrant_args;
+
+ va_start(list, outputs);
+ va_rtas_call_unlocked(args, token, nargs, nret, list);
+ va_end(list);
+
+ if (nret > 1 && outputs)
+ for (i = 0; i < nret - 1; ++i)
+ outputs[i] = be32_to_cpu(args->rets[i + 1]);
+
+ return (nret > 0) ? be32_to_cpu(args->rets[0]) : 0;
+}
+
/* For RTAS_BUSY (-2), delay for 1 millisecond. For an extended busy status
* code of 990n, perform the hinted delay of 10^n (last digit) milliseconds.
*/
diff --git a/arch/powerpc/sysdev/xics/ics-rtas.c b/arch/powerpc/sysdev/xics/ics-rtas.c
index 6aabc74688a6..4cf18000f07c 100644
--- a/arch/powerpc/sysdev/xics/ics-rtas.c
+++ b/arch/powerpc/sysdev/xics/ics-rtas.c
@@ -50,8 +50,8 @@ static void ics_rtas_unmask_irq(struct irq_data *d)
server = xics_get_irq_server(d->irq, irq_data_get_affinity_mask(d), 0);
- call_status = rtas_call(ibm_set_xive, 3, 1, NULL, hw_irq, server,
- DEFAULT_PRIORITY);
+ call_status = rtas_call_reentrant(ibm_set_xive, 3, 1, NULL, hw_irq,
+ server, DEFAULT_PRIORITY);
if (call_status != 0) {
printk(KERN_ERR
"%s: ibm_set_xive irq %u server %x returned %d\n",
@@ -60,7 +60,7 @@ static void ics_rtas_unmask_irq(struct irq_data *d)
}
/* Now unmask the interrupt (often a no-op) */
- call_status = rtas_call(ibm_int_on, 1, 1, NULL, hw_irq);
+ call_status = rtas_call_reentrant(ibm_int_on, 1, 1, NULL, hw_irq);
if (call_status != 0) {
printk(KERN_ERR "%s: ibm_int_on irq=%u returned %d\n",
__func__, hw_irq, call_status);
@@ -91,7 +91,7 @@ static void ics_rtas_mask_real_irq(unsigned int hw_irq)
if (hw_irq == XICS_IPI)
return;
- call_status = rtas_call(ibm_int_off, 1, 1, NULL, hw_irq);
+ call_status = rtas_call_reentrant(ibm_int_off, 1, 1, NULL, hw_irq);
if (call_status != 0) {
printk(KERN_ERR "%s: ibm_int_off irq=%u returned %d\n",
__func__, hw_irq, call_status);
@@ -99,8 +99,8 @@ static void ics_rtas_mask_real_irq(unsigned int hw_irq)
}
/* Have to set XIVE to 0xff to be able to remove a slot */
- call_status = rtas_call(ibm_set_xive, 3, 1, NULL, hw_irq,
- xics_default_server, 0xff);
+ call_status = rtas_call_reentrant(ibm_set_xive, 3, 1, NULL, hw_irq,
+ xics_default_server, 0xff);
if (call_status != 0) {
printk(KERN_ERR "%s: ibm_set_xive(0xff) irq=%u returned %d\n",
__func__, hw_irq, call_status);
@@ -131,7 +131,7 @@ static int ics_rtas_set_affinity(struct irq_data *d,
if (hw_irq == XICS_IPI || hw_irq == XICS_IRQ_SPURIOUS)
return -1;
- status = rtas_call(ibm_get_xive, 1, 3, xics_status, hw_irq);
+ status = rtas_call_reentrant(ibm_get_xive, 1, 3, xics_status, hw_irq);
if (status) {
printk(KERN_ERR "%s: ibm,get-xive irq=%u returns %d\n",
@@ -146,8 +146,8 @@ static int ics_rtas_set_affinity(struct irq_data *d,
return -1;
}
- status = rtas_call(ibm_set_xive, 3, 1, NULL,
- hw_irq, irq_server, xics_status[1]);
+ status = rtas_call_reentrant(ibm_set_xive, 3, 1, NULL,
+ hw_irq, irq_server, xics_status[1]);
if (status) {
printk(KERN_ERR "%s: ibm,set-xive irq=%u returns %d\n",
@@ -179,7 +179,7 @@ static int ics_rtas_map(struct ics *ics, unsigned int virq)
return -EINVAL;
/* Check if RTAS knows about this interrupt */
- rc = rtas_call(ibm_get_xive, 1, 3, status, hw_irq);
+ rc = rtas_call_reentrant(ibm_get_xive, 1, 3, status, hw_irq);
if (rc)
return -ENXIO;
@@ -198,7 +198,7 @@ static long ics_rtas_get_server(struct ics *ics, unsigned long vec)
{
int rc, status[2];
- rc = rtas_call(ibm_get_xive, 1, 3, status, vec);
+ rc = rtas_call_reentrant(ibm_get_xive, 1, 3, status, vec);
if (rc)
return -1;
return status[0];
--
2.25.4
^ permalink raw reply related
* Re: [PATCH v8 23/30] powerpc: Add prefixed instructions to instruction data type
From: Jordan Niethe @ 2020-05-14 1:40 UTC (permalink / raw)
To: linuxppc-dev
Cc: Christophe Leroy, Alistair Popple, Nicholas Piggin, Balamuruhan S,
naveen.n.rao, Daniel Axtens
In-Reply-To: <20200506034050.24806-24-jniethe5@gmail.com>
Hi mpe,
Relating to your message on [PATCH v8 16/30] powerpc: Define and use
__get_user_instr{,inatomic}() - could you please take this.
diff --git a/arch/powerpc/include/asm/uaccess.h
b/arch/powerpc/include/asm/uaccess.h
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -106,6 +106,24 @@ static inline int __access_ok(unsigned long addr,
unsigned long size,
__put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
#ifdef __powerpc64__
+#define get_user_instr(x, ptr) \
+({ \
+ long __gui_ret = 0; \
+ unsigned long __gui_ptr = (unsigned long)ptr; \
+ struct ppc_inst __gui_inst; \
+ unsigned int prefix, suffix; \
+ __gui_ret = get_user(prefix, (unsigned int __user *)__gui_ptr); \
+ if (!__gui_ret && (prefix >> 26) == OP_PREFIX) { \
+ __gui_ret = get_user(suffix, \
+ (unsigned int __user *)__gui_ptr + 1); \
+ __gui_inst = ppc_inst_prefix(prefix, suffix); \
+ } else { \
+ __gui_inst = ppc_inst(prefix); \
+ } \
+ (x) = __gui_inst; \
+ __gui_ret; \
+})
+
#define __get_user_instr(x, ptr) \
({ \
long __gui_ret = 0; \
@@ -142,6 +160,8 @@ static inline int __access_ok(unsigned long addr,
unsigned long size,
__gui_ret; \
})
#else
+#define get_user_instr(x, ptr) \
+ get_user((x).val, (u32 *)(ptr))
#define __get_user_instr(x, ptr) \
__get_user_nocheck((x).val, (u32 *)(ptr), sizeof(u32), true)
#define __get_user_instr_inatomic(x, ptr) \
^ permalink raw reply
* Re: [PATCH v8 16/30] powerpc: Define and use __get_user_instr{, inatomic}()
From: Jordan Niethe @ 2020-05-14 1:43 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, Alistair Popple, Nicholas Piggin, Balamuruhan S,
naveen.n.rao, linuxppc-dev, Daniel Axtens
In-Reply-To: <CACzsE9od2GFXBiy5imy_dGutx7POOnCx7+k-Ynx+UMcNzyTsTw@mail.gmail.com>
Hi mpe, could you please take this.
arch/powerpc/include/asm/uaccess.h | 3 +++
arch/powerpc/kernel/vecemu.c | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/uaccess.h
b/arch/powerpc/include/asm/uaccess.h
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -105,6 +105,9 @@ static inline int __access_ok(unsigned long addr,
unsigned long size,
#define __put_user_inatomic(x, ptr) \
__put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
+#define get_user_instr(x, ptr) \
+ get_user((x).val, (u32 *)(ptr))
+
#define __get_user_instr(x, ptr) \
__get_user_nocheck((x).val, (u32 *)(ptr), sizeof(u32), true)
diff --git a/arch/powerpc/kernel/vecemu.c b/arch/powerpc/kernel/vecemu.c
index 60ed5aea8d4e..ae632569446f 100644
--- a/arch/powerpc/kernel/vecemu.c
+++ b/arch/powerpc/kernel/vecemu.c
@@ -266,7 +266,7 @@ int emulate_altivec(struct pt_regs *regs)
unsigned int va, vb, vc, vd;
vector128 *vrs;
- if (__get_user_instr(instr, (void __user *)regs->nip))
+ if (get_user_instr(instr, (void __user *)regs->nip))
return -EFAULT;
word = ppc_inst_val(instr);
^ permalink raw reply related
* Re: [PATCH 06/11] powerpc/xmon: constify sysrq_key_op
From: Michael Ellerman @ 2020-05-14 3:44 UTC (permalink / raw)
To: Emil Velikov, dri-devel
Cc: Greg Kroah-Hartman, Jiri Slaby, emil.l.velikov, linux-kernel,
Paul Mackerras, linuxppc-dev
In-Reply-To: <20200513214351.2138580-6-emil.l.velikov@gmail.com>
Emil Velikov <emil.l.velikov@gmail.com> writes:
> With earlier commits, the API no longer discards the const-ness of the
> sysrq_key_op. As such we can add the notation.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
> Please keep me in the CC list, as I'm not subscribed to the list.
>
> IMHO it would be better if this gets merged this via the tty tree.
Fine by me.
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 7af840c0fc93..0d8ca5c9f131 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -3842,7 +3842,7 @@ static void sysrq_handle_xmon(int key)
> xmon_init(0);
> }
>
> -static struct sysrq_key_op sysrq_xmon_op = {
> +static const struct sysrq_key_op sysrq_xmon_op = {
> .handler = sysrq_handle_xmon,
> .help_msg = "xmon(x)",
> .action_msg = "Entering xmon",
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH v8 00/30] Initial Prefixed Instruction support
From: Christophe Leroy @ 2020-05-14 5:31 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, npiggin, bala24, naveen.n.rao, dja
In-Reply-To: <20200506034050.24806-1-jniethe5@gmail.com>
Le 06/05/2020 à 05:40, Jordan Niethe a écrit :
> A future revision of the ISA will introduce prefixed instructions. A
> prefixed instruction is composed of a 4-byte prefix followed by a
> 4-byte suffix.
>
> All prefixes have the major opcode 1. A prefix will never be a valid
> word instruction. A suffix may be an existing word instruction or a
> new instruction.
>
> This series enables prefixed instructions and extends the instruction
> emulation to support them. Then the places where prefixed instructions
> might need to be emulated are updated.
>
> v8 incorporates feedback from Alistair Popple and Balamuruhan Suriyakumar.
> The major changes:
> - Fix some style issues
> - Fix __patch_instruction() on big endian
> - Reintroduce v3's forbidding breakpoints on second word of prefix
> instructions for kprobes and xmon. Missed this when changing to
> using a data type.
> - Use the data type in some places that were missed.
Checkpatch seems to report the following warnings for pmac32_defconfig,
are they harmless ?
+arch/powerpc/kernel/align.c:307:13: warning: cast removes address space
'<asn:1>' of expression
+arch/powerpc/kernel/align.c:307:13: warning: cast removes address space
'<asn:1>' of expression
+arch/powerpc/kernel/align.c:307:13: warning: cast removes address space
'<asn:1>' of expression
+arch/powerpc/kernel/align.c:307:13: warning: cast removes address space
'<asn:1>' of expression
+arch/powerpc/kernel/align.c:307:13: warning: cast removes address space
'<asn:1>' of expression
+arch/powerpc/kernel/align.c:307:13: warning: incorrect type in argument
1 (different address spaces) expected void const volatile [noderef]
<asn:1> * got unsigned int [usertype] *
+arch/powerpc/kernel/align.c:307:13: warning: incorrect type in
initializer (different address spaces) expected unsigned int [noderef]
<asn:1> *__gu_addr got unsigned int [usertype] *
+arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: cast removes
address space '<asn:1>' of expression
+arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: cast removes
address space '<asn:1>' of expression
+arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: cast removes
address space '<asn:1>' of expression
+arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: cast removes
address space '<asn:1>' of expression
+arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: cast removes
address space '<asn:1>' of expression
-arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: incorrect type in
argument 1 (different address spaces) expected void const volatile
[noderef] <asn:1> * got unsigned int *
+arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: incorrect type in
argument 1 (different address spaces) expected void const volatile
[noderef] <asn:1> * got unsigned int [usertype] *
-arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: incorrect type in
initializer (different address spaces) expected unsigned int [noderef]
<asn:1> *__gu_addr got unsigned int *
+arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: incorrect type in
initializer (different address spaces) expected unsigned int [noderef]
<asn:1> *__gu_addr got unsigned int [usertype] *
+arch/powerpc/kernel/vecemu.c:269:13: warning: cast removes address
space '<asn:1>' of expression
+arch/powerpc/kernel/vecemu.c:269:13: warning: cast removes address
space '<asn:1>' of expression
+arch/powerpc/kernel/vecemu.c:269:13: warning: cast removes address
space '<asn:1>' of expression
+arch/powerpc/kernel/vecemu.c:269:13: warning: cast removes address
space '<asn:1>' of expression
+arch/powerpc/kernel/vecemu.c:269:13: warning: cast removes address
space '<asn:1>' of expression
+arch/powerpc/kernel/vecemu.c:269:13: warning: incorrect type in
argument 1 (different address spaces) expected void const volatile
[noderef] <asn:1> * got unsigned int [usertype] *
+arch/powerpc/kernel/vecemu.c:269:13: warning: incorrect type in
initializer (different address spaces) expected unsigned int [noderef]
<asn:1> *__gu_addr got unsigned int [usertype] *
+arch/powerpc/lib/inst.c:55:37: warning: incorrect type in argument 2
(different address spaces) expected void const [noderef] <asn:1> *src
got struct ppc_inst *nip
+arch/powerpc/mm/fault.c:284:59: warning: incorrect type in argument 2
(different address spaces) expected struct ppc_inst *nip got struct
ppc_inst [noderef] <asn:1> *
Christophe
>
> v7 fixes compilation issues for some configs reported by Alistair
> Popple.
>
> v6 is based on feedback from Balamuruhan Suriyakumar, Alistair Popple,
> Christophe Leroy and Segher Boessenkool.
> The major changes:
> - Use the instruction type in more places that had been missed before
> - Fix issues with ppc32
> - Introduce new self tests for code patching and feature fixups
>
> v5 is based on feedback from Nick Piggins, Michael Ellerman, Balamuruhan
> Suriyakumar and Alistair Popple.
> The major changes:
> - The ppc instruction type is now a struct
> - Series now just based on next
> - ppc_inst_masked() dropped
> - Space for xmon breakpoints allocated in an assembly file
> - "Add prefixed instructions to instruction data type" patch seperated in
> to smaller patches
> - Calling convention for create_branch() is changed
> - Some places which had not been updated to use the data type are now updated
>
> v4 is based on feedback from Nick Piggins, Christophe Leroy and Daniel Axtens.
> The major changes:
> - Move xmon breakpoints from data section to text section
> - Introduce a data type for instructions on powerpc
>
> v3 is based on feedback from Christophe Leroy. The major changes:
> - Completely replacing store_inst() with patch_instruction() in
> xmon
> - Improve implementation of mread_instr() to not use mread().
> - Base the series on top of
> https://patchwork.ozlabs.org/patch/1232619/ as this will effect
> kprobes.
> - Some renaming and simplification of conditionals.
>
> v2 incorporates feedback from Daniel Axtens and and Balamuruhan
> S. The major changes are:
> - Squashing together all commits about SRR1 bits
> - Squashing all commits for supporting prefixed load stores
> - Changing abbreviated references to sufx/prfx -> suffix/prefix
> - Introducing macros for returning the length of an instruction
> - Removing sign extension flag from pstd/pld in sstep.c
> - Dropping patch "powerpc/fault: Use analyse_instr() to check for
> store with updates to sp" from the series, it did not really fit
> with prefixed enablement in the first place and as reported by Greg
> Kurz did not work correctly.
>
>
> Alistair Popple (1):
> powerpc: Enable Prefixed Instructions
>
> Jordan Niethe (29):
> powerpc/xmon: Remove store_inst() for patch_instruction()
> powerpc/xmon: Move breakpoint instructions to own array
> powerpc/xmon: Move breakpoints to text section
> powerpc/xmon: Use bitwise calculations in_breakpoint_table()
> powerpc: Change calling convention for create_branch() et. al.
> powerpc: Use a macro for creating instructions from u32s
> powerpc: Use an accessor for instructions
> powerpc: Use a function for getting the instruction op code
> powerpc: Use a function for byte swapping instructions
> powerpc: Introduce functions for instruction equality
> powerpc: Use a datatype for instructions
> powerpc: Use a function for reading instructions
> powerpc: Add a probe_user_read_inst() function
> powerpc: Add a probe_kernel_read_inst() function
> powerpc/kprobes: Use patch_instruction()
> powerpc: Define and use __get_user_instr{,inatomic}()
> powerpc: Introduce a function for reporting instruction length
> powerpc/xmon: Use a function for reading instructions
> powerpc/xmon: Move insertion of breakpoint for xol'ing
> powerpc: Make test_translate_branch() independent of instruction
> length
> powerpc: Define new SRR1 bits for a future ISA version
> powerpc: Add prefixed instructions to instruction data type
> powerpc: Test prefixed code patching
> powerpc: Test prefixed instructions in feature fixups
> powerpc/xmon: Don't allow breakpoints on suffixes
> powerpc/kprobes: Don't allow breakpoints on suffixes
> powerpc: Support prefixed instructions in alignment handler
> powerpc sstep: Add support for prefixed load/stores
> powerpc sstep: Add support for prefixed fixed-point arithmetic
>
> arch/powerpc/include/asm/code-patching.h | 37 +-
> arch/powerpc/include/asm/inst.h | 107 +++++
> arch/powerpc/include/asm/kprobes.h | 2 +-
> arch/powerpc/include/asm/ppc-opcode.h | 3 +
> arch/powerpc/include/asm/reg.h | 7 +-
> arch/powerpc/include/asm/sstep.h | 15 +-
> arch/powerpc/include/asm/uaccess.h | 43 ++
> arch/powerpc/include/asm/uprobes.h | 7 +-
> arch/powerpc/kernel/align.c | 13 +-
> arch/powerpc/kernel/asm-offsets.c | 8 +
> arch/powerpc/kernel/crash_dump.c | 7 +-
> arch/powerpc/kernel/epapr_paravirt.c | 7 +-
> arch/powerpc/kernel/hw_breakpoint.c | 5 +-
> arch/powerpc/kernel/jump_label.c | 5 +-
> arch/powerpc/kernel/kgdb.c | 9 +-
> arch/powerpc/kernel/kprobes.c | 37 +-
> arch/powerpc/kernel/mce_power.c | 5 +-
> arch/powerpc/kernel/module_64.c | 3 +-
> arch/powerpc/kernel/optprobes.c | 102 +++--
> arch/powerpc/kernel/optprobes_head.S | 3 +
> arch/powerpc/kernel/security.c | 12 +-
> arch/powerpc/kernel/setup_32.c | 8 +-
> arch/powerpc/kernel/trace/ftrace.c | 168 ++++----
> arch/powerpc/kernel/traps.c | 20 +-
> arch/powerpc/kernel/uprobes.c | 5 +-
> arch/powerpc/kernel/vecemu.c | 20 +-
> arch/powerpc/kvm/book3s_hv_nested.c | 2 +-
> arch/powerpc/kvm/book3s_hv_rm_mmu.c | 2 +-
> arch/powerpc/kvm/emulate_loadstore.c | 2 +-
> arch/powerpc/lib/Makefile | 2 +-
> arch/powerpc/lib/code-patching.c | 319 +++++++++------
> arch/powerpc/lib/feature-fixups-test.S | 69 ++++
> arch/powerpc/lib/feature-fixups.c | 160 ++++++--
> arch/powerpc/lib/inst.c | 70 ++++
> arch/powerpc/lib/sstep.c | 459 +++++++++++++++-------
> arch/powerpc/lib/test_code-patching.S | 20 +
> arch/powerpc/lib/test_emulate_step.c | 56 +--
> arch/powerpc/mm/fault.c | 15 +-
> arch/powerpc/mm/nohash/8xx.c | 5 +-
> arch/powerpc/perf/8xx-pmu.c | 9 +-
> arch/powerpc/perf/core-book3s.c | 4 +-
> arch/powerpc/platforms/86xx/mpc86xx_smp.c | 5 +-
> arch/powerpc/platforms/powermac/smp.c | 5 +-
> arch/powerpc/xmon/Makefile | 2 +-
> arch/powerpc/xmon/xmon.c | 122 ++++--
> arch/powerpc/xmon/xmon_bpts.S | 11 +
> arch/powerpc/xmon/xmon_bpts.h | 14 +
> 47 files changed, 1409 insertions(+), 602 deletions(-)
> create mode 100644 arch/powerpc/include/asm/inst.h
> create mode 100644 arch/powerpc/lib/inst.c
> create mode 100644 arch/powerpc/lib/test_code-patching.S
> create mode 100644 arch/powerpc/xmon/xmon_bpts.S
> create mode 100644 arch/powerpc/xmon/xmon_bpts.h
>
^ permalink raw reply
* Re: [PATCH v8 13/30] powerpc: Add a probe_user_read_inst() function
From: Christophe Leroy @ 2020-05-14 5:46 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev
Cc: christophe.leroy, alistair, npiggin, bala24, naveen.n.rao, dja
In-Reply-To: <20200506034050.24806-14-jniethe5@gmail.com>
Le 06/05/2020 à 05:40, Jordan Niethe a écrit :
> Introduce a probe_user_read_inst() function to use in cases where
> probe_user_read() is used for getting an instruction. This will be more
> useful for prefixed instructions.
>
> Reviewed-by: Alistair Popple <alistair@popple.id.au>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v6: - New to series
> ---
> arch/powerpc/include/asm/inst.h | 3 +++
> arch/powerpc/lib/Makefile | 2 +-
> arch/powerpc/lib/inst.c | 18 ++++++++++++++++++
> arch/powerpc/mm/fault.c | 2 +-
> 4 files changed, 23 insertions(+), 2 deletions(-)
> create mode 100644 arch/powerpc/lib/inst.c
>
> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> index 552e953bf04f..3e9a58420151 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -37,4 +37,7 @@ static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
> return ppc_inst_val(x) == ppc_inst_val(y);
> }
>
> +int probe_user_read_inst(struct ppc_inst *inst,
> + struct ppc_inst *nip);
> +
> #endif /* _ASM_INST_H */
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index b8de3be10eb4..546591848219 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -16,7 +16,7 @@ CFLAGS_code-patching.o += -DDISABLE_BRANCH_PROFILING
> CFLAGS_feature-fixups.o += -DDISABLE_BRANCH_PROFILING
> endif
>
> -obj-y += alloc.o code-patching.o feature-fixups.o pmem.o
> +obj-y += alloc.o code-patching.o feature-fixups.o pmem.o inst.o
>
> ifndef CONFIG_KASAN
> obj-y += string.o memcmp_$(BITS).o
> diff --git a/arch/powerpc/lib/inst.c b/arch/powerpc/lib/inst.c
> new file mode 100644
> index 000000000000..eaf786afad2b
> --- /dev/null
> +++ b/arch/powerpc/lib/inst.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2020, IBM Corporation.
> + */
> +
> +#include <linux/uaccess.h>
> +#include <asm/inst.h>
> +
> +int probe_user_read_inst(struct ppc_inst *inst,
> + struct ppc_inst *nip)
> +{
> + unsigned int val;
> + int err;
> +
> + err = probe_user_read(&val, nip, sizeof(val));
> + *inst = ppc_inst(val);
> + return err;
> +}
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index 4a50f125ec18..f3a943eae305 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -281,7 +281,7 @@ static bool bad_stack_expansion(struct pt_regs *regs, unsigned long address,
> access_ok(nip, sizeof(*nip))) {
> struct ppc_inst inst;
>
> - if (!probe_user_read(&inst, nip, sizeof(inst)))
> + if (!probe_user_read_inst(&inst, (struct ppc_inst __user *)nip))
Shouldn't 'nip' become de 'struct ppc_inst __user *' instead of casting ?
> return !store_updates_sp(inst);
> *must_retry = true;
> }
>
Christophe
^ permalink raw reply
* Re: [PATCH v8 23/30] powerpc: Add prefixed instructions to instruction data type
From: Christophe Leroy @ 2020-05-14 6:11 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev
Cc: christophe.leroy, alistair, npiggin, bala24, naveen.n.rao, dja
In-Reply-To: <20200506034050.24806-24-jniethe5@gmail.com>
Le 06/05/2020 à 05:40, Jordan Niethe a écrit :
> For powerpc64, redefine the ppc_inst type so both word and prefixed
> instructions can be represented. On powerpc32 the type will remain the
> same. Update places which had assumed instructions to be 4 bytes long.
>
> Reviewed-by: Alistair Popple <alistair@popple.id.au>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4: New to series
> v5: - Distinguish normal instructions from prefixed instructions with a
> 0xff marker for the suffix.
> - __patch_instruction() using std for prefixed instructions
> v6: - Return false instead of 0 in ppc_inst_prefixed()
> - Fix up types for ppc32 so it compiles
> - remove ppc_inst_write()
> - __patching_instruction(): move flush out of condition
> v8: - style
> - Define and use OP_PREFIX instead of '1' (back from v3)
> - __patch_instruction() fix for big endian
> ---
> arch/powerpc/include/asm/inst.h | 69 ++++++++++++++++++++++++---
> arch/powerpc/include/asm/kprobes.h | 2 +-
> arch/powerpc/include/asm/ppc-opcode.h | 3 ++
> arch/powerpc/include/asm/uaccess.h | 40 +++++++++++++++-
> arch/powerpc/include/asm/uprobes.h | 2 +-
> arch/powerpc/kernel/crash_dump.c | 2 +-
> arch/powerpc/kernel/optprobes.c | 42 ++++++++--------
> arch/powerpc/kernel/optprobes_head.S | 3 ++
> arch/powerpc/lib/code-patching.c | 19 ++++++--
> arch/powerpc/lib/feature-fixups.c | 5 +-
> arch/powerpc/lib/inst.c | 41 ++++++++++++++++
> arch/powerpc/lib/sstep.c | 4 +-
> arch/powerpc/xmon/xmon.c | 4 +-
> arch/powerpc/xmon/xmon_bpts.S | 2 +
> 14 files changed, 200 insertions(+), 38 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> index 2f3c9d5bcf7c..7868b80b610e 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -2,29 +2,79 @@
> #ifndef _ASM_INST_H
> #define _ASM_INST_H
>
> +#include <asm/ppc-opcode.h>
> /*
> * Instruction data type for POWER
> */
>
> struct ppc_inst {
> u32 val;
> +#ifdef __powerpc64__
CONFIG_PPC64 should be used instead. This is also reported by checkpatch.
> + u32 suffix;
> +#endif /* __powerpc64__ */
> } __packed;
>
> -#define ppc_inst(x) ((struct ppc_inst){ .val = x })
> -
> static inline u32 ppc_inst_val(struct ppc_inst x)
> {
> return x.val;
> }
>
> -static inline int ppc_inst_len(struct ppc_inst x)
> +static inline int ppc_inst_primary_opcode(struct ppc_inst x)
> {
> - return sizeof(struct ppc_inst);
> + return ppc_inst_val(x) >> 26;
What about using get_op() from asm/disassemble.h instead of hardcodiing ?
> }
>
> -static inline int ppc_inst_primary_opcode(struct ppc_inst x)
> +#ifdef __powerpc64__
Use CONFIG_PPC64
> +#define ppc_inst(x) ((struct ppc_inst){ .val = (x), .suffix = 0xff })
> +
> +#define ppc_inst_prefix(x, y) ((struct ppc_inst){ .val = (x), .suffix = (y) })
> +
> +static inline u32 ppc_inst_suffix(struct ppc_inst x)
> {
> - return ppc_inst_val(x) >> 26;
> + return x.suffix;
> +}
> +
> +static inline bool ppc_inst_prefixed(struct ppc_inst x)
> +{
> + return (ppc_inst_primary_opcode(x) == 1) && ppc_inst_suffix(x) != 0xff;
> +}
> +
> +static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
> +{
> + return ppc_inst_prefix(swab32(ppc_inst_val(x)),
> + swab32(ppc_inst_suffix(x)));
> +}
> +
> +static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> +{
> + u32 val, suffix;
> +
> + val = *(u32 *)ptr;
> + if ((val >> 26) == 1) {
Don't hardcode, use ppc_inst_primary_opcode() and compare it to OP_PREFIX
Or use get_op() from asm/disassemble.h
> + suffix = *((u32 *)ptr + 1);
> + return ppc_inst_prefix(val, suffix);
> + } else {
> + return ppc_inst(val);
> + }
> +}
> +
> +static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
> +{
> + return *(u64 *)&x == *(u64 *)&y;
> +}
> +
> +#else
> +
> +#define ppc_inst(x) ((struct ppc_inst){ .val = x })
> +
> +static inline bool ppc_inst_prefixed(struct ppc_inst x)
> +{
> + return false;
> +}
> +
> +static inline u32 ppc_inst_suffix(struct ppc_inst x)
> +{
> + return 0;
> }
>
> static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
> @@ -42,6 +92,13 @@ static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
> return ppc_inst_val(x) == ppc_inst_val(y);
> }
>
> +#endif /* __powerpc64__ */
> +
> +static inline int ppc_inst_len(struct ppc_inst x)
> +{
> + return (ppc_inst_prefixed(x)) ? 8 : 4;
> +}
> +
> int probe_user_read_inst(struct ppc_inst *inst,
> struct ppc_inst *nip);
> int probe_kernel_read_inst(struct ppc_inst *inst,
> diff --git a/arch/powerpc/include/asm/kprobes.h b/arch/powerpc/include/asm/kprobes.h
> index 66b3f2983b22..4fc0e15e23a5 100644
> --- a/arch/powerpc/include/asm/kprobes.h
> +++ b/arch/powerpc/include/asm/kprobes.h
> @@ -43,7 +43,7 @@ extern kprobe_opcode_t optprobe_template_ret[];
> extern kprobe_opcode_t optprobe_template_end[];
>
> /* Fixed instruction size for powerpc */
> -#define MAX_INSN_SIZE 1
> +#define MAX_INSN_SIZE 2
> #define MAX_OPTIMIZED_LENGTH sizeof(kprobe_opcode_t) /* 4 bytes */
> #define MAX_OPTINSN_SIZE (optprobe_template_end - optprobe_template_entry)
> #define RELATIVEJUMP_SIZE sizeof(kprobe_opcode_t) /* 4 bytes */
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index c1df75edde44..2a39c716c343 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -158,6 +158,9 @@
> /* VMX Vector Store Instructions */
> #define OP_31_XOP_STVX 231
>
> +/* Prefixed Instructions */
> +#define OP_PREFIX 1
> +
> #define OP_31 31
> #define OP_LWZ 32
> #define OP_STFS 52
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index c0a35e4586a5..217897927926 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -105,11 +105,49 @@ static inline int __access_ok(unsigned long addr, unsigned long size,
> #define __put_user_inatomic(x, ptr) \
> __put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
>
> +#ifdef __powerpc64__
Replace by CONFIG_PPC64
> +#define __get_user_instr(x, ptr) \
> +({ \
> + long __gui_ret = 0; \
> + unsigned long __gui_ptr = (unsigned long)ptr; \
> + struct ppc_inst __gui_inst; \
> + unsigned int prefix, suffix; \
> + __gui_ret = __get_user(prefix, (unsigned int __user *)__gui_ptr); \
__get_user() can be costly especially with KUAP. I think you should
perform a 64 bits read and fallback on a 32 bits read only if the 64
bits read failed.
> + if (!__gui_ret && (prefix >> 26) == OP_PREFIX) { \
What about using get_op() from asm/disassemble.h instead of hardcodiing ?
> + __gui_ret = __get_user(suffix, \
> + (unsigned int __user *)__gui_ptr + 1); \
> + __gui_inst = ppc_inst_prefix(prefix, suffix); \
> + } else { \
> + __gui_inst = ppc_inst(prefix); \
> + } \
> + (x) = __gui_inst; \
> + __gui_ret; \
> +})
> +
> +#define __get_user_instr_inatomic(x, ptr) \
> +({ \
> + long __gui_ret = 0; \
> + unsigned long __gui_ptr = (unsigned long)ptr; \
> + struct ppc_inst __gui_inst; \
> + unsigned int prefix, suffix; \
> + __gui_ret = __get_user_inatomic(prefix, (unsigned int __user *)__gui_ptr); \
Same
> + if (!__gui_ret && (prefix >> 26) == OP_PREFIX) { \
> + __gui_ret = __get_user_inatomic(suffix, \
> + (unsigned int __user *)__gui_ptr + 1); \
> + __gui_inst = ppc_inst_prefix(prefix, suffix); \
> + } else { \
> + __gui_inst = ppc_inst(prefix); \
> + } \
> + (x) = __gui_inst; \
> + __gui_ret; \
> +})
> +#else
> #define __get_user_instr(x, ptr) \
> __get_user_nocheck((x).val, (u32 *)(ptr), sizeof(u32), true)
> -
> #define __get_user_instr_inatomic(x, ptr) \
> __get_user_nosleep((x).val, (u32 *)(ptr), sizeof(u32))
> +#endif
> +
> extern long __put_user_bad(void);
>
> /*
> diff --git a/arch/powerpc/include/asm/uprobes.h b/arch/powerpc/include/asm/uprobes.h
> index 7e3b329ba2d3..5bf65f5d44a9 100644
> --- a/arch/powerpc/include/asm/uprobes.h
> +++ b/arch/powerpc/include/asm/uprobes.h
> @@ -15,7 +15,7 @@
>
> typedef ppc_opcode_t uprobe_opcode_t;
>
> -#define MAX_UINSN_BYTES 4
> +#define MAX_UINSN_BYTES 8
> #define UPROBE_XOL_SLOT_BYTES (MAX_UINSN_BYTES)
>
> /* The following alias is needed for reference from arch-agnostic code */
> diff --git a/arch/powerpc/kernel/crash_dump.c b/arch/powerpc/kernel/crash_dump.c
> index 72bafb47e757..735e89337398 100644
> --- a/arch/powerpc/kernel/crash_dump.c
> +++ b/arch/powerpc/kernel/crash_dump.c
> @@ -46,7 +46,7 @@ static void __init create_trampoline(unsigned long addr)
> * two instructions it doesn't require any registers.
> */
> patch_instruction(p, ppc_inst(PPC_INST_NOP));
> - patch_branch(++p, addr + PHYSICAL_START, 0);
> + patch_branch((void *)p + 4, addr + PHYSICAL_START, 0);
> }
>
> void __init setup_kdump_trampoline(void)
> diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
> index 52c1ab3f85aa..a8e66603d12b 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -162,43 +162,43 @@ void patch_imm32_load_insns(unsigned int val, kprobe_opcode_t *addr)
>
> /*
> * Generate instructions to load provided immediate 64-bit value
> - * to register 'r3' and patch these instructions at 'addr'.
> + * to register 'reg' and patch these instructions at 'addr'.
> */
> -void patch_imm64_load_insns(unsigned long val, kprobe_opcode_t *addr)
> +void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
I think this change should go in a separate patch.
> {
> - /* lis r3,(op)@highest */
> + /* lis reg,(op)@highest */
> patch_instruction((struct ppc_inst *)addr,
> - ppc_inst(PPC_INST_ADDIS | ___PPC_RT(3) |
> + ppc_inst(PPC_INST_ADDIS | ___PPC_RT(reg) |
> ((val >> 48) & 0xffff)));
> addr++;
>
> - /* ori r3,r3,(op)@higher */
> + /* ori reg,reg,(op)@higher */
> patch_instruction((struct ppc_inst *)addr,
> - ppc_inst(PPC_INST_ORI | ___PPC_RA(3) |
> - ___PPC_RS(3) | ((val >> 32) & 0xffff)));
> + ppc_inst(PPC_INST_ORI | ___PPC_RA(reg) |
> + ___PPC_RS(reg) | ((val >> 32) & 0xffff)));
> addr++;
>
> - /* rldicr r3,r3,32,31 */
> + /* rldicr reg,reg,32,31 */
> patch_instruction((struct ppc_inst *)addr,
> - ppc_inst(PPC_INST_RLDICR | ___PPC_RA(3) |
> - ___PPC_RS(3) | __PPC_SH64(32) | __PPC_ME64(31)));
> + ppc_inst(PPC_INST_RLDICR | ___PPC_RA(reg) |
> + ___PPC_RS(reg) | __PPC_SH64(32) | __PPC_ME64(31)));
> addr++;
>
> - /* oris r3,r3,(op)@h */
> + /* oris reg,reg,(op)@h */
> patch_instruction((struct ppc_inst *)addr,
> - ppc_inst(PPC_INST_ORIS | ___PPC_RA(3) |
> - ___PPC_RS(3) | ((val >> 16) & 0xffff)));
> + ppc_inst(PPC_INST_ORIS | ___PPC_RA(reg) |
> + ___PPC_RS(reg) | ((val >> 16) & 0xffff)));
> addr++;
>
> - /* ori r3,r3,(op)@l */
> + /* ori reg,reg,(op)@l */
> patch_instruction((struct ppc_inst *)addr,
> - ppc_inst(PPC_INST_ORI | ___PPC_RA(3) |
> - ___PPC_RS(3) | (val & 0xffff)));
> + ppc_inst(PPC_INST_ORI | ___PPC_RA(reg) |
> + ___PPC_RS(reg) | (val & 0xffff)));
> }
>
> int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
> {
> - struct ppc_inst branch_op_callback, branch_emulate_step;
> + struct ppc_inst branch_op_callback, branch_emulate_step, temp;
> kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
> long b_offset;
> unsigned long nip, size;
> @@ -249,7 +249,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
> * Fixup the template with instructions to:
> * 1. load the address of the actual probepoint
> */
> - patch_imm64_load_insns((unsigned long)op, buff + TMPL_OP_IDX);
> + patch_imm64_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
>
> /*
> * 2. branch to optimized_callback() and emulate_step()
> @@ -282,7 +282,11 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
> /*
> * 3. load instruction to be emulated into relevant register, and
> */
> - patch_imm32_load_insns(*p->ainsn.insn, buff + TMPL_INSN_IDX);
> + temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
> + patch_imm64_load_insns(ppc_inst_val(temp) |
> + ((u64)ppc_inst_suffix(temp) << 32),
> + 4,
So now we are also using r4 ? Any explanation somewhere on the way it
works ? This change seems unrelated to this patch, nothing in the
description about it. Can we suddenly use a new register without problem ?
> + buff + TMPL_INSN_IDX);
What's the point with splitting this line in 4 lines ? Can't it fit in 2
lines ?
>
> /*
> * 4. branch back from trampoline
> diff --git a/arch/powerpc/kernel/optprobes_head.S b/arch/powerpc/kernel/optprobes_head.S
> index cf383520843f..ff8ba4d3824d 100644
> --- a/arch/powerpc/kernel/optprobes_head.S
> +++ b/arch/powerpc/kernel/optprobes_head.S
> @@ -94,6 +94,9 @@ optprobe_template_insn:
> /* 2, Pass instruction to be emulated in r4 */
> nop
> nop
> + nop
> + nop
> + nop
>
> .global optprobe_template_call_emulate
> optprobe_template_call_emulate:
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index d946f7d6bb32..58b67b62d5d3 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -24,13 +24,24 @@ static int __patch_instruction(struct ppc_inst *exec_addr, struct ppc_inst instr
> {
> int err = 0;
>
> - __put_user_asm(ppc_inst_val(instr), patch_addr, err, "stw");
> - if (err)
> - return err;
> + if (!ppc_inst_prefixed(instr)) {
> + __put_user_asm(ppc_inst_val(instr), patch_addr, err, "stw");
> + if (err)
> + return err;
This test should remain outside of the if/else, it doesn't need to be
duplicated.
> + } else {
> +#ifdef CONFIG_CPU_LITTLE_ENDIAN
> + __put_user_asm((u64)ppc_inst_suffix(instr) << 32 |
> + ppc_inst_val(instr), patch_addr, err, "std");
> +#else
> + __put_user_asm((u64)ppc_inst_val(instr) << 32 |
> + ppc_inst_suffix(instr), patch_addr, err, "std");
> +#endif /* CONFIG_CPU_LITTLE_ENDIAN */
> + if (err)
> + return err;
> + }
>
> asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
> "r" (exec_addr));
> -
Why remove the blank line ?
> return 0;
> }
>
> diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
> index 2bd2b752de4f..a8238eff3a31 100644
> --- a/arch/powerpc/lib/feature-fixups.c
> +++ b/arch/powerpc/lib/feature-fixups.c
> @@ -84,12 +84,13 @@ static int patch_feature_section(unsigned long value, struct fixup_entry *fcur)
> src = alt_start;
> dest = start;
>
> - for (; src < alt_end; src++, dest++) {
> + for (; src < alt_end; src = (void *)src + ppc_inst_len(ppc_inst_read(src)),
> + (dest = (void *)dest + ppc_inst_len(ppc_inst_read(dest)))) {
Can we do this outside the for() for readability ?
> if (patch_alt_instruction(src, dest, alt_start, alt_end))
> return 1;
> }
>
> - for (; dest < end; dest++)
> + for (; dest < end; dest = (void *)dest + ppc_inst_len(ppc_inst(PPC_INST_NOP)))
> raw_patch_instruction(dest, ppc_inst(PPC_INST_NOP));
>
> return 0;
> diff --git a/arch/powerpc/lib/inst.c b/arch/powerpc/lib/inst.c
> index 08dedd927268..eb6f9ee28ac6 100644
> --- a/arch/powerpc/lib/inst.c
> +++ b/arch/powerpc/lib/inst.c
> @@ -3,9 +3,49 @@
> * Copyright 2020, IBM Corporation.
> */
>
> +#include <asm/ppc-opcode.h>
> #include <linux/uaccess.h>
> #include <asm/inst.h>
>
> +#ifdef __powerpc64__
> +int probe_user_read_inst(struct ppc_inst *inst,
> + struct ppc_inst *nip)
> +{
> + unsigned int val, suffix;
> + int err;
> +
> + err = probe_user_read(&val, nip, sizeof(val));
A user read is costly with KUAP. Can we do a 64 bits read and perform a
32 bits read only when 64 bits read fails ?
> + if (err)
> + return err;
> + if ((val >> 26) == OP_PREFIX) {
What about using get_op() from asm/disassemble.h instead of hardcodiing ?
> + err = probe_user_read(&suffix, (void *)nip + 4,
4 or sizeof(unsigned int) ? Why use both in the same line ?
> + sizeof(unsigned int));
> + *inst = ppc_inst_prefix(val, suffix);
> + } else {
> + *inst = ppc_inst(val);
> + }
> + return err;
> +}
> +
> +int probe_kernel_read_inst(struct ppc_inst *inst,
> + struct ppc_inst *src)
> +{
> + unsigned int val, suffix;
> + int err;
> +
> + err = probe_kernel_read(&val, src, sizeof(val));
> + if (err)
> + return err;
> + if ((val >> 26) == OP_PREFIX) {
> + err = probe_kernel_read(&suffix, (void *)src + 4,
> + sizeof(unsigned int));
> + *inst = ppc_inst_prefix(val, suffix);
> + } else {
> + *inst = ppc_inst(val);
> + }
> + return err;
> +}
> +#else
> int probe_user_read_inst(struct ppc_inst *inst,
> struct ppc_inst *nip)
> {
> @@ -27,3 +67,4 @@ int probe_kernel_read_inst(struct ppc_inst *inst,
> *inst = ppc_inst(val);
> return err;
> }
> +#endif /* __powerpc64__ */
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index 95a56bb1ba3f..ecd756c346fd 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -1169,10 +1169,12 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
> unsigned long int imm;
> unsigned long int val, val2;
> unsigned int mb, me, sh;
> - unsigned int word;
> + unsigned int word, suffix;
> long ival;
>
> word = ppc_inst_val(instr);
> + suffix = ppc_inst_suffix(instr);
> +
> op->type = COMPUTE;
>
> opcode = ppc_inst_primary_opcode(instr);
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 4d6980d51456..647b3829c4eb 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -758,8 +758,8 @@ static int xmon_bpt(struct pt_regs *regs)
>
> /* Are we at the trap at bp->instr[1] for some bp? */
> bp = in_breakpoint_table(regs->nip, &offset);
> - if (bp != NULL && offset == 4) {
> - regs->nip = bp->address + 4;
> + if (bp != NULL && (offset == 4 || offset == 8)) {
> + regs->nip = bp->address + offset;
> atomic_dec(&bp->ref_count);
> return 1;
> }
> diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
> index f3ad0ab50854..69726814cd27 100644
> --- a/arch/powerpc/xmon/xmon_bpts.S
> +++ b/arch/powerpc/xmon/xmon_bpts.S
> @@ -4,6 +4,8 @@
> #include <asm/asm-offsets.h>
> #include "xmon_bpts.h"
>
> +/* Prefixed instructions can not cross 64 byte boundaries */
> +.align 6
> .global bpt_table
> bpt_table:
> .space NBPTS * BPT_SIZE
>
Christophe
^ permalink raw reply
* Re: [PATCH v8 28/30] powerpc: Support prefixed instructions in alignment handler
From: Christophe Leroy @ 2020-05-14 6:14 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev
Cc: christophe.leroy, alistair, npiggin, bala24, naveen.n.rao, dja
In-Reply-To: <20200506034050.24806-29-jniethe5@gmail.com>
Le 06/05/2020 à 05:40, Jordan Niethe a écrit :
> If a prefixed instruction results in an alignment exception, the
> SRR1_PREFIXED bit is set. The handler attempts to emulate the
> responsible instruction and then increment the NIP past it. Use
> SRR1_PREFIXED to determine by how much the NIP should be incremented.
>
> Prefixed instructions are not permitted to cross 64-byte boundaries. If
> they do the alignment interrupt is invoked with SRR1 BOUNDARY bit set.
> If this occurs send a SIGBUS to the offending process if in user mode.
> If in kernel mode call bad_page_fault().
Shouldn't this patch go before patch 23 ?
Christophe
>
> Reviewed-by: Alistair Popple <alistair@popple.id.au>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v2: - Move __get_user_instr() and __get_user_instr_inatomic() to this
> commit (previously in "powerpc sstep: Prepare to support prefixed
> instructions").
> - Rename sufx to suffix
> - Use a macro for calculating instruction length
> v3: Move __get_user_{instr(), instr_inatomic()} up with the other
> get_user definitions and remove nested if.
> v4: Rolled into "Add prefixed instructions to instruction data type"
> v5: Only one definition of inst_length()
> ---
> arch/powerpc/kernel/traps.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 493a3fa0ac1a..105242cc2f28 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -583,6 +583,8 @@ static inline int check_io_access(struct pt_regs *regs)
> #define REASON_ILLEGAL (ESR_PIL | ESR_PUO)
> #define REASON_PRIVILEGED ESR_PPR
> #define REASON_TRAP ESR_PTR
> +#define REASON_PREFIXED 0
> +#define REASON_BOUNDARY 0
>
> /* single-step stuff */
> #define single_stepping(regs) (current->thread.debug.dbcr0 & DBCR0_IC)
> @@ -597,12 +599,16 @@ static inline int check_io_access(struct pt_regs *regs)
> #define REASON_ILLEGAL SRR1_PROGILL
> #define REASON_PRIVILEGED SRR1_PROGPRIV
> #define REASON_TRAP SRR1_PROGTRAP
> +#define REASON_PREFIXED SRR1_PREFIXED
> +#define REASON_BOUNDARY SRR1_BOUNDARY
>
> #define single_stepping(regs) ((regs)->msr & MSR_SE)
> #define clear_single_step(regs) ((regs)->msr &= ~MSR_SE)
> #define clear_br_trace(regs) ((regs)->msr &= ~MSR_BE)
> #endif
>
> +#define inst_length(reason) (((reason) & REASON_PREFIXED) ? 8 : 4)
> +
> #if defined(CONFIG_E500)
> int machine_check_e500mc(struct pt_regs *regs)
> {
> @@ -1593,11 +1599,20 @@ void alignment_exception(struct pt_regs *regs)
> {
> enum ctx_state prev_state = exception_enter();
> int sig, code, fixed = 0;
> + unsigned long reason;
>
> /* We restore the interrupt state now */
> if (!arch_irq_disabled_regs(regs))
> local_irq_enable();
>
> + reason = get_reason(regs);
> +
> + if (reason & REASON_BOUNDARY) {
> + sig = SIGBUS;
> + code = BUS_ADRALN;
> + goto bad;
> + }
> +
> if (tm_abort_check(regs, TM_CAUSE_ALIGNMENT | TM_CAUSE_PERSISTENT))
> goto bail;
>
> @@ -1606,7 +1621,8 @@ void alignment_exception(struct pt_regs *regs)
> fixed = fix_alignment(regs);
>
> if (fixed == 1) {
> - regs->nip += 4; /* skip over emulated instruction */
> + /* skip over emulated instruction */
> + regs->nip += inst_length(reason);
> emulate_single_step(regs);
> goto bail;
> }
> @@ -1619,6 +1635,7 @@ void alignment_exception(struct pt_regs *regs)
> sig = SIGBUS;
> code = BUS_ADRALN;
> }
> +bad:
> if (user_mode(regs))
> _exception(sig, regs, code, regs->dar);
> else
>
^ permalink raw reply
* Re: [PATCH v8 29/30] powerpc sstep: Add support for prefixed load/stores
From: Christophe Leroy @ 2020-05-14 6:15 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, npiggin, bala24, naveen.n.rao, dja
In-Reply-To: <20200506034050.24806-30-jniethe5@gmail.com>
Shouldn't this patch go before patch 23 ?
Christophe
Le 06/05/2020 à 05:40, Jordan Niethe a écrit :
> This adds emulation support for the following prefixed integer
> load/stores:
> * Prefixed Load Byte and Zero (plbz)
> * Prefixed Load Halfword and Zero (plhz)
> * Prefixed Load Halfword Algebraic (plha)
> * Prefixed Load Word and Zero (plwz)
> * Prefixed Load Word Algebraic (plwa)
> * Prefixed Load Doubleword (pld)
> * Prefixed Store Byte (pstb)
> * Prefixed Store Halfword (psth)
> * Prefixed Store Word (pstw)
> * Prefixed Store Doubleword (pstd)
> * Prefixed Load Quadword (plq)
> * Prefixed Store Quadword (pstq)
>
> the follow prefixed floating-point load/stores:
> * Prefixed Load Floating-Point Single (plfs)
> * Prefixed Load Floating-Point Double (plfd)
> * Prefixed Store Floating-Point Single (pstfs)
> * Prefixed Store Floating-Point Double (pstfd)
>
> and for the following prefixed VSX load/stores:
> * Prefixed Load VSX Scalar Doubleword (plxsd)
> * Prefixed Load VSX Scalar Single-Precision (plxssp)
> * Prefixed Load VSX Vector [0|1] (plxv, plxv0, plxv1)
> * Prefixed Store VSX Scalar Doubleword (pstxsd)
> * Prefixed Store VSX Scalar Single-Precision (pstxssp)
> * Prefixed Store VSX Vector [0|1] (pstxv, pstxv0, pstxv1)
>
> Reviewed-by: Balamuruhan S <bala24@linux.ibm.com>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v2: - Combine all load/store patches
> - Fix the name of Type 01 instructions
> - Remove sign extension flag from pstd/pld
> - Rename sufx -> suffix
> v3: - Move prefixed loads and stores into the switch statement
> v6: - Compile on ppc32
> - Add back in + GETLENGTH(op->type)
> v8: Use fallthrough; keyword
> ---
> arch/powerpc/include/asm/sstep.h | 4 +
> arch/powerpc/lib/sstep.c | 163 ++++++++++++++++++++++++++++++-
> 2 files changed, 165 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/sstep.h b/arch/powerpc/include/asm/sstep.h
> index c3ce903ac488..9b200a5f8794 100644
> --- a/arch/powerpc/include/asm/sstep.h
> +++ b/arch/powerpc/include/asm/sstep.h
> @@ -90,11 +90,15 @@ enum instruction_type {
> #define VSX_LDLEFT 4 /* load VSX register from left */
> #define VSX_CHECK_VEC 8 /* check MSR_VEC not MSR_VSX for reg >= 32 */
>
> +/* Prefixed flag, ORed in with type */
> +#define PREFIXED 0x800
> +
> /* Size field in type word */
> #define SIZE(n) ((n) << 12)
> #define GETSIZE(w) ((w) >> 12)
>
> #define GETTYPE(t) ((t) & INSTR_TYPE_MASK)
> +#define GETLENGTH(t) (((t) & PREFIXED) ? 8 : 4)
>
> #define MKOP(t, f, s) ((t) | (f) | SIZE(s))
>
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index ecd756c346fd..6794a7672ad5 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -187,6 +187,44 @@ static nokprobe_inline unsigned long xform_ea(unsigned int instr,
> return ea;
> }
>
> +/*
> + * Calculate effective address for a MLS:D-form / 8LS:D-form
> + * prefixed instruction
> + */
> +static nokprobe_inline unsigned long mlsd_8lsd_ea(unsigned int instr,
> + unsigned int suffix,
> + const struct pt_regs *regs)
> +{
> + int ra, prefix_r;
> + unsigned int dd;
> + unsigned long ea, d0, d1, d;
> +
> + prefix_r = instr & (1ul << 20);
> + ra = (suffix >> 16) & 0x1f;
> +
> + d0 = instr & 0x3ffff;
> + d1 = suffix & 0xffff;
> + d = (d0 << 16) | d1;
> +
> + /*
> + * sign extend a 34 bit number
> + */
> + dd = (unsigned int)(d >> 2);
> + ea = (signed int)dd;
> + ea = (ea << 2) | (d & 0x3);
> +
> + if (!prefix_r && ra)
> + ea += regs->gpr[ra];
> + else if (!prefix_r && !ra)
> + ; /* Leave ea as is */
> + else if (prefix_r && !ra)
> + ea += regs->nip;
> + else if (prefix_r && ra)
> + ; /* Invalid form. Should already be checked for by caller! */
> +
> + return ea;
> +}
> +
> /*
> * Return the largest power of 2, not greater than sizeof(unsigned long),
> * such that x is a multiple of it.
> @@ -1166,6 +1204,9 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
> struct ppc_inst instr)
> {
> unsigned int opcode, ra, rb, rc, rd, spr, u;
> +#ifdef __powerpc64__
> + unsigned int suffixopcode, prefixtype, prefix_r;
> +#endif
> unsigned long int imm;
> unsigned long int val, val2;
> unsigned int mb, me, sh;
> @@ -2652,6 +2693,124 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
> break;
> }
> break;
> + case 1: /* Prefixed instructions */
> + prefix_r = word & (1ul << 20);
> + ra = (suffix >> 16) & 0x1f;
> + op->update_reg = ra;
> + rd = (suffix >> 21) & 0x1f;
> + op->reg = rd;
> + op->val = regs->gpr[rd];
> +
> + suffixopcode = suffix >> 26;
> + prefixtype = (word >> 24) & 0x3;
> + switch (prefixtype) {
> + case 0: /* Type 00 Eight-Byte Load/Store */
> + if (prefix_r && ra)
> + break;
> + op->ea = mlsd_8lsd_ea(word, suffix, regs);
> + switch (suffixopcode) {
> + case 41: /* plwa */
> + op->type = MKOP(LOAD, PREFIXED | SIGNEXT, 4);
> + break;
> + case 42: /* plxsd */
> + op->reg = rd + 32;
> + op->type = MKOP(LOAD_VSX, PREFIXED, 8);
> + op->element_size = 8;
> + op->vsx_flags = VSX_CHECK_VEC;
> + break;
> + case 43: /* plxssp */
> + op->reg = rd + 32;
> + op->type = MKOP(LOAD_VSX, PREFIXED, 4);
> + op->element_size = 8;
> + op->vsx_flags = VSX_FPCONV | VSX_CHECK_VEC;
> + break;
> + case 46: /* pstxsd */
> + op->reg = rd + 32;
> + op->type = MKOP(STORE_VSX, PREFIXED, 8);
> + op->element_size = 8;
> + op->vsx_flags = VSX_CHECK_VEC;
> + break;
> + case 47: /* pstxssp */
> + op->reg = rd + 32;
> + op->type = MKOP(STORE_VSX, PREFIXED, 4);
> + op->element_size = 8;
> + op->vsx_flags = VSX_FPCONV | VSX_CHECK_VEC;
> + break;
> + case 51: /* plxv1 */
> + op->reg += 32;
> + fallthrough;
> + case 50: /* plxv0 */
> + op->type = MKOP(LOAD_VSX, PREFIXED, 16);
> + op->element_size = 16;
> + op->vsx_flags = VSX_CHECK_VEC;
> + break;
> + case 55: /* pstxv1 */
> + op->reg = rd + 32;
> + fallthrough;
> + case 54: /* pstxv0 */
> + op->type = MKOP(STORE_VSX, PREFIXED, 16);
> + op->element_size = 16;
> + op->vsx_flags = VSX_CHECK_VEC;
> + break;
> + case 56: /* plq */
> + op->type = MKOP(LOAD, PREFIXED, 16);
> + break;
> + case 57: /* pld */
> + op->type = MKOP(LOAD, PREFIXED, 8);
> + break;
> + case 60: /* stq */
> + op->type = MKOP(STORE, PREFIXED, 16);
> + break;
> + case 61: /* pstd */
> + op->type = MKOP(STORE, PREFIXED, 8);
> + break;
> + }
> + break;
> + case 1: /* Type 01 Eight-Byte Register-to-Register */
> + break;
> + case 2: /* Type 10 Modified Load/Store */
> + if (prefix_r && ra)
> + break;
> + op->ea = mlsd_8lsd_ea(word, suffix, regs);
> + switch (suffixopcode) {
> + case 32: /* plwz */
> + op->type = MKOP(LOAD, PREFIXED, 4);
> + break;
> + case 34: /* plbz */
> + op->type = MKOP(LOAD, PREFIXED, 1);
> + break;
> + case 36: /* pstw */
> + op->type = MKOP(STORE, PREFIXED, 4);
> + break;
> + case 38: /* pstb */
> + op->type = MKOP(STORE, PREFIXED, 1);
> + break;
> + case 40: /* plhz */
> + op->type = MKOP(LOAD, PREFIXED, 2);
> + break;
> + case 42: /* plha */
> + op->type = MKOP(LOAD, PREFIXED | SIGNEXT, 2);
> + break;
> + case 44: /* psth */
> + op->type = MKOP(STORE, PREFIXED, 2);
> + break;
> + case 48: /* plfs */
> + op->type = MKOP(LOAD_FP, PREFIXED | FPCONV, 4);
> + break;
> + case 50: /* plfd */
> + op->type = MKOP(LOAD_FP, PREFIXED, 8);
> + break;
> + case 52: /* pstfs */
> + op->type = MKOP(STORE_FP, PREFIXED | FPCONV, 4);
> + break;
> + case 54: /* pstfd */
> + op->type = MKOP(STORE_FP, PREFIXED, 8);
> + break;
> + }
> + break;
> + case 3: /* Type 11 Modified Register-to-Register */
> + break;
> + }
> #endif /* __powerpc64__ */
>
> }
> @@ -2760,7 +2919,7 @@ void emulate_update_regs(struct pt_regs *regs, struct instruction_op *op)
> {
> unsigned long next_pc;
>
> - next_pc = truncate_if_32bit(regs->msr, regs->nip + 4);
> + next_pc = truncate_if_32bit(regs->msr, regs->nip + GETLENGTH(op->type));
> switch (GETTYPE(op->type)) {
> case COMPUTE:
> if (op->type & SETREG)
> @@ -3205,7 +3364,7 @@ int emulate_step(struct pt_regs *regs, struct ppc_inst instr)
> return 0;
>
> instr_done:
> - regs->nip = truncate_if_32bit(regs->msr, regs->nip + 4);
> + regs->nip = truncate_if_32bit(regs->msr, regs->nip + GETLENGTH(op.type));
> return 1;
> }
> NOKPROBE_SYMBOL(emulate_step);
>
^ permalink raw reply
* Re: [PATCH v8 30/30] powerpc sstep: Add support for prefixed fixed-point arithmetic
From: Christophe Leroy @ 2020-05-14 6:15 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, npiggin, bala24, naveen.n.rao, dja
In-Reply-To: <20200506034050.24806-31-jniethe5@gmail.com>
Le 06/05/2020 à 05:40, Jordan Niethe a écrit :
> This adds emulation support for the following prefixed Fixed-Point
> Arithmetic instructions:
> * Prefixed Add Immediate (paddi)
Shouldn't this patch go before patch 23 ?
Christophe
>
> Reviewed-by: Balamuruhan S <bala24@linux.ibm.com>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v3: Since we moved the prefixed loads/stores into the load/store switch
> statement it no longer makes sense to have paddi in there, so move it
> out.
> ---
> arch/powerpc/lib/sstep.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index 6794a7672ad5..964fe7bbfce3 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -1337,6 +1337,26 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
>
> switch (opcode) {
> #ifdef __powerpc64__
> + case 1:
> + prefix_r = word & (1ul << 20);
> + ra = (suffix >> 16) & 0x1f;
> + rd = (suffix >> 21) & 0x1f;
> + op->reg = rd;
> + op->val = regs->gpr[rd];
> + suffixopcode = suffix >> 26;
> + prefixtype = (word >> 24) & 0x3;
> + switch (prefixtype) {
> + case 2:
> + if (prefix_r && ra)
> + return 0;
> + switch (suffixopcode) {
> + case 14: /* paddi */
> + op->type = COMPUTE | PREFIXED;
> + op->val = mlsd_8lsd_ea(word, suffix, regs);
> + goto compute_done;
> + }
> + }
> + break;
> case 2: /* tdi */
> if (rd & trap_compare(regs->gpr[ra], (short) word))
> goto trap;
>
^ permalink raw reply
* Re: powerpc/mpc85xx: Add Cyrus P5040 device tree source
From: Scott Wood @ 2020-05-14 6:42 UTC (permalink / raw)
To: Darren Stevens; +Cc: linuxppc-dev, chzigotzky
In-Reply-To: <4fb03d6874.55cfc4a1@auth.smtp.1and1.co.uk>
On Wed, 2020-05-13 at 23:02 +0100, Darren Stevens wrote:
> Hello Scott
>
> On 08/05/2020, Scott Wood wrote:
> > On Thu, 2020-05-07 at 22:30 +0100, Darren Stevens wrote:
> > >
> > > +/include/ "p5040si-pre.dtsi"
> > > +
> > > +/ {
> > > + model = "varisys,CYRUS5040";
> > > + compatible = "varisys,CYRUS";
> >
> > Is this board 100% compatible with the Cyrus P5020 board, down to every
> > last
> > quirk, except for the SoC plugged into it? If not, they shouldn't have
> > the
> > same compatible. If they are, then couldn't everything in this file but
> > the
> > SoC include be moved to a dtsi shared with cyrus_p5020.dts?
>
> It's not 100% compatible, the mdio ports map to different fman ports, but
> both as are 'corenet generic' boards, I added varisys,CYRUS so it would be
> detected in corenet_generic.c - support for the 5020 was added by Andy
> Flemming, I've just tried to copy what he did.
>
> I can add another entry to the table, but do we realy want a separate entry
> in the table for every supported board rather than using the device tree for
> similar boards?
A separate compatible for each board is generally what we've done, as it
allows for the possibility of board-specific quirks. At least it's just a
table entry; back in the day it used to be a separate file. :-P
That said, if you're pretty sure that all potentially relevant differences are
described elsewhere in the device tree, I wouldn't mind too much if it
becomes:
compatible = "varisys,CYRUS5040", "varisys,CYRUS";
-Scott
^ permalink raw reply
* Re: [PATCH RFC 1/4] powerpc/radix: Fix compilation for radix with CONFIG_SMP=n
From: Joel Stanley @ 2020-05-14 8:26 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, Michael Neuling, Benjamin Herrenschmidt
In-Reply-To: <1589010661.v7yharjogg.astroid@bobo.none>
On Sat, 9 May 2020 at 07:52, Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Excerpts from Paul Mackerras's message of May 9, 2020 3:02 pm:
> > This fixes the compile errors we currently get with CONFIG_SMP=n and
> > CONFIG_PPC_RADIX_MMU=y.
>
> Did I already fix this, or does it keep getting broken?! :(
>
> Anyway fine by me if it's required.
You're right, your fix was merged in 5.7-rc1.
Cheers,
Joel
^ permalink raw reply
* Re: [PATCH v8 00/30] Initial Prefixed Instruction support
From: Jordan Niethe @ 2020-05-14 10:33 UTC (permalink / raw)
To: Christophe Leroy
Cc: Alistair Popple, Nicholas Piggin, Balamuruhan S, naveen.n.rao,
linuxppc-dev, Daniel Axtens
In-Reply-To: <d5d59817-6e90-5643-6405-2b2794348684@csgroup.eu>
On Thu, May 14, 2020 at 3:31 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 06/05/2020 à 05:40, Jordan Niethe a écrit :
> > A future revision of the ISA will introduce prefixed instructions. A
> > prefixed instruction is composed of a 4-byte prefix followed by a
> > 4-byte suffix.
> >
> > All prefixes have the major opcode 1. A prefix will never be a valid
> > word instruction. A suffix may be an existing word instruction or a
> > new instruction.
> >
> > This series enables prefixed instructions and extends the instruction
> > emulation to support them. Then the places where prefixed instructions
> > might need to be emulated are updated.
> >
> > v8 incorporates feedback from Alistair Popple and Balamuruhan Suriyakumar.
> > The major changes:
> > - Fix some style issues
> > - Fix __patch_instruction() on big endian
> > - Reintroduce v3's forbidding breakpoints on second word of prefix
> > instructions for kprobes and xmon. Missed this when changing to
> > using a data type.
> > - Use the data type in some places that were missed.
>
> Checkpatch seems to report the following warnings for pmac32_defconfig,
> are they harmless ?
>
> +arch/powerpc/kernel/align.c:307:13: warning: cast removes address space
> '<asn:1>' of expression
> +arch/powerpc/kernel/align.c:307:13: warning: cast removes address space
> '<asn:1>' of expression
> +arch/powerpc/kernel/align.c:307:13: warning: cast removes address space
> '<asn:1>' of expression
> +arch/powerpc/kernel/align.c:307:13: warning: cast removes address space
> '<asn:1>' of expression
> +arch/powerpc/kernel/align.c:307:13: warning: cast removes address space
> '<asn:1>' of expression
> +arch/powerpc/kernel/align.c:307:13: warning: incorrect type in argument
> 1 (different address spaces) expected void const volatile [noderef]
> <asn:1> * got unsigned int [usertype] *
> +arch/powerpc/kernel/align.c:307:13: warning: incorrect type in
> initializer (different address spaces) expected unsigned int [noderef]
> <asn:1> *__gu_addr got unsigned int [usertype] *
> +arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: cast removes
> address space '<asn:1>' of expression
> +arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: cast removes
> address space '<asn:1>' of expression
> +arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: cast removes
> address space '<asn:1>' of expression
> +arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: cast removes
> address space '<asn:1>' of expression
> +arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: cast removes
> address space '<asn:1>' of expression
> -arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: incorrect type in
> argument 1 (different address spaces) expected void const volatile
> [noderef] <asn:1> * got unsigned int *
> +arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: incorrect type in
> argument 1 (different address spaces) expected void const volatile
> [noderef] <asn:1> * got unsigned int [usertype] *
> -arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: incorrect type in
> initializer (different address spaces) expected unsigned int [noderef]
> <asn:1> *__gu_addr got unsigned int *
> +arch/powerpc/kernel/hw_breakpoint.c:XX:13: warning: incorrect type in
> initializer (different address spaces) expected unsigned int [noderef]
> <asn:1> *__gu_addr got unsigned int [usertype] *
> +arch/powerpc/kernel/vecemu.c:269:13: warning: cast removes address
> space '<asn:1>' of expression
> +arch/powerpc/kernel/vecemu.c:269:13: warning: cast removes address
> space '<asn:1>' of expression
> +arch/powerpc/kernel/vecemu.c:269:13: warning: cast removes address
> space '<asn:1>' of expression
> +arch/powerpc/kernel/vecemu.c:269:13: warning: cast removes address
> space '<asn:1>' of expression
> +arch/powerpc/kernel/vecemu.c:269:13: warning: cast removes address
> space '<asn:1>' of expression
> +arch/powerpc/kernel/vecemu.c:269:13: warning: incorrect type in
> argument 1 (different address spaces) expected void const volatile
> [noderef] <asn:1> * got unsigned int [usertype] *
> +arch/powerpc/kernel/vecemu.c:269:13: warning: incorrect type in
> initializer (different address spaces) expected unsigned int [noderef]
> <asn:1> *__gu_addr got unsigned int [usertype] *
> +arch/powerpc/lib/inst.c:55:37: warning: incorrect type in argument 2
> (different address spaces) expected void const [noderef] <asn:1> *src
> got struct ppc_inst *nip
> +arch/powerpc/mm/fault.c:284:59: warning: incorrect type in argument 2
> (different address spaces) expected struct ppc_inst *nip got struct
> ppc_inst [noderef] <asn:1> *
Thanks, I was missing some __user.
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -106,10 +106,10 @@ static inline int __access_ok(unsigned long
addr, unsigned long size,
__put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
#define __get_user_instr(x, ptr) \
- __get_user_nocheck((x).val, (u32 *)(ptr), sizeof(u32), true)
+ __get_user_nocheck((x).val, (u32 __user *)(ptr), sizeof(u32), true)
#define __get_user_instr_inatomic(x, ptr) \
- __get_user_nosleep((x).val, (u32 *)(ptr), sizeof(u32))
+ __get_user_nosleep((x).val, (u32 __user *)(ptr), sizeof(u32))
extern long __put_user_bad(void);
/*
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -38,6 +38,6 @@ static inline bool ppc_inst_equal(struct ppc_inst x,
struct ppc_inst y)
}
int probe_user_read_inst(struct ppc_inst *inst,
- struct ppc_inst *nip);
+ struct ppc_inst __user *nip);
#endif /* _ASM_INST_H */
diff --git a/arch/powerpc/lib/inst.c b/arch/powerpc/lib/inst.c
index eaf786afad2b..c15611086d26 100644
--- a/arch/powerpc/lib/inst.c
+++ b/arch/powerpc/lib/inst.c
@@ -7,7 +7,7 @@
#include <asm/inst.h>
int probe_user_read_inst(struct ppc_inst *inst,
- struct ppc_inst *nip)
+ struct ppc_inst __user *nip)
{
unsigned int val;
int err;
>
> Christophe
>
> >
> > v7 fixes compilation issues for some configs reported by Alistair
> > Popple.
> >
> > v6 is based on feedback from Balamuruhan Suriyakumar, Alistair Popple,
> > Christophe Leroy and Segher Boessenkool.
> > The major changes:
> > - Use the instruction type in more places that had been missed before
> > - Fix issues with ppc32
> > - Introduce new self tests for code patching and feature fixups
> >
> > v5 is based on feedback from Nick Piggins, Michael Ellerman, Balamuruhan
> > Suriyakumar and Alistair Popple.
> > The major changes:
> > - The ppc instruction type is now a struct
> > - Series now just based on next
> > - ppc_inst_masked() dropped
> > - Space for xmon breakpoints allocated in an assembly file
> > - "Add prefixed instructions to instruction data type" patch seperated in
> > to smaller patches
> > - Calling convention for create_branch() is changed
> > - Some places which had not been updated to use the data type are now updated
> >
> > v4 is based on feedback from Nick Piggins, Christophe Leroy and Daniel Axtens.
> > The major changes:
> > - Move xmon breakpoints from data section to text section
> > - Introduce a data type for instructions on powerpc
> >
> > v3 is based on feedback from Christophe Leroy. The major changes:
> > - Completely replacing store_inst() with patch_instruction() in
> > xmon
> > - Improve implementation of mread_instr() to not use mread().
> > - Base the series on top of
> > https://patchwork.ozlabs.org/patch/1232619/ as this will effect
> > kprobes.
> > - Some renaming and simplification of conditionals.
> >
> > v2 incorporates feedback from Daniel Axtens and and Balamuruhan
> > S. The major changes are:
> > - Squashing together all commits about SRR1 bits
> > - Squashing all commits for supporting prefixed load stores
> > - Changing abbreviated references to sufx/prfx -> suffix/prefix
> > - Introducing macros for returning the length of an instruction
> > - Removing sign extension flag from pstd/pld in sstep.c
> > - Dropping patch "powerpc/fault: Use analyse_instr() to check for
> > store with updates to sp" from the series, it did not really fit
> > with prefixed enablement in the first place and as reported by Greg
> > Kurz did not work correctly.
> >
> >
> > Alistair Popple (1):
> > powerpc: Enable Prefixed Instructions
> >
> > Jordan Niethe (29):
> > powerpc/xmon: Remove store_inst() for patch_instruction()
> > powerpc/xmon: Move breakpoint instructions to own array
> > powerpc/xmon: Move breakpoints to text section
> > powerpc/xmon: Use bitwise calculations in_breakpoint_table()
> > powerpc: Change calling convention for create_branch() et. al.
> > powerpc: Use a macro for creating instructions from u32s
> > powerpc: Use an accessor for instructions
> > powerpc: Use a function for getting the instruction op code
> > powerpc: Use a function for byte swapping instructions
> > powerpc: Introduce functions for instruction equality
> > powerpc: Use a datatype for instructions
> > powerpc: Use a function for reading instructions
> > powerpc: Add a probe_user_read_inst() function
> > powerpc: Add a probe_kernel_read_inst() function
> > powerpc/kprobes: Use patch_instruction()
> > powerpc: Define and use __get_user_instr{,inatomic}()
> > powerpc: Introduce a function for reporting instruction length
> > powerpc/xmon: Use a function for reading instructions
> > powerpc/xmon: Move insertion of breakpoint for xol'ing
> > powerpc: Make test_translate_branch() independent of instruction
> > length
> > powerpc: Define new SRR1 bits for a future ISA version
> > powerpc: Add prefixed instructions to instruction data type
> > powerpc: Test prefixed code patching
> > powerpc: Test prefixed instructions in feature fixups
> > powerpc/xmon: Don't allow breakpoints on suffixes
> > powerpc/kprobes: Don't allow breakpoints on suffixes
> > powerpc: Support prefixed instructions in alignment handler
> > powerpc sstep: Add support for prefixed load/stores
> > powerpc sstep: Add support for prefixed fixed-point arithmetic
> >
> > arch/powerpc/include/asm/code-patching.h | 37 +-
> > arch/powerpc/include/asm/inst.h | 107 +++++
> > arch/powerpc/include/asm/kprobes.h | 2 +-
> > arch/powerpc/include/asm/ppc-opcode.h | 3 +
> > arch/powerpc/include/asm/reg.h | 7 +-
> > arch/powerpc/include/asm/sstep.h | 15 +-
> > arch/powerpc/include/asm/uaccess.h | 43 ++
> > arch/powerpc/include/asm/uprobes.h | 7 +-
> > arch/powerpc/kernel/align.c | 13 +-
> > arch/powerpc/kernel/asm-offsets.c | 8 +
> > arch/powerpc/kernel/crash_dump.c | 7 +-
> > arch/powerpc/kernel/epapr_paravirt.c | 7 +-
> > arch/powerpc/kernel/hw_breakpoint.c | 5 +-
> > arch/powerpc/kernel/jump_label.c | 5 +-
> > arch/powerpc/kernel/kgdb.c | 9 +-
> > arch/powerpc/kernel/kprobes.c | 37 +-
> > arch/powerpc/kernel/mce_power.c | 5 +-
> > arch/powerpc/kernel/module_64.c | 3 +-
> > arch/powerpc/kernel/optprobes.c | 102 +++--
> > arch/powerpc/kernel/optprobes_head.S | 3 +
> > arch/powerpc/kernel/security.c | 12 +-
> > arch/powerpc/kernel/setup_32.c | 8 +-
> > arch/powerpc/kernel/trace/ftrace.c | 168 ++++----
> > arch/powerpc/kernel/traps.c | 20 +-
> > arch/powerpc/kernel/uprobes.c | 5 +-
> > arch/powerpc/kernel/vecemu.c | 20 +-
> > arch/powerpc/kvm/book3s_hv_nested.c | 2 +-
> > arch/powerpc/kvm/book3s_hv_rm_mmu.c | 2 +-
> > arch/powerpc/kvm/emulate_loadstore.c | 2 +-
> > arch/powerpc/lib/Makefile | 2 +-
> > arch/powerpc/lib/code-patching.c | 319 +++++++++------
> > arch/powerpc/lib/feature-fixups-test.S | 69 ++++
> > arch/powerpc/lib/feature-fixups.c | 160 ++++++--
> > arch/powerpc/lib/inst.c | 70 ++++
> > arch/powerpc/lib/sstep.c | 459 +++++++++++++++-------
> > arch/powerpc/lib/test_code-patching.S | 20 +
> > arch/powerpc/lib/test_emulate_step.c | 56 +--
> > arch/powerpc/mm/fault.c | 15 +-
> > arch/powerpc/mm/nohash/8xx.c | 5 +-
> > arch/powerpc/perf/8xx-pmu.c | 9 +-
> > arch/powerpc/perf/core-book3s.c | 4 +-
> > arch/powerpc/platforms/86xx/mpc86xx_smp.c | 5 +-
> > arch/powerpc/platforms/powermac/smp.c | 5 +-
> > arch/powerpc/xmon/Makefile | 2 +-
> > arch/powerpc/xmon/xmon.c | 122 ++++--
> > arch/powerpc/xmon/xmon_bpts.S | 11 +
> > arch/powerpc/xmon/xmon_bpts.h | 14 +
> > 47 files changed, 1409 insertions(+), 602 deletions(-)
> > create mode 100644 arch/powerpc/include/asm/inst.h
> > create mode 100644 arch/powerpc/lib/inst.c
> > create mode 100644 arch/powerpc/lib/test_code-patching.S
> > create mode 100644 arch/powerpc/xmon/xmon_bpts.S
> > create mode 100644 arch/powerpc/xmon/xmon_bpts.h
> >
^ permalink raw reply related
* [PATCH v6 00/16] powerpc/watchpoint: Preparation for more than one watchpoint
From: Ravi Bangoria @ 2020-05-14 11:17 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, apopple, ravi.bangoria, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
So far, powerpc Book3S code has been written with an assumption of
only one watchpoint. But Power10[1] is introducing second watchpoint
register (DAWR). Even though this patchset does not enable 2nd DAWR,
it makes the infrastructure ready so that enabling 2nd DAWR should
just be a matter of changing count.
Existing functionality works fine with the patchset. I've tested it
with perf, ptrace(gdb), xmon. All hw-breakpoint selftests are passing
as well. And I've build tested for 8xx and 'AMCC 44x, 46x or 47x'.
Note: kvm or PowerVM guest is not enabled yet.
v5: https://lore.kernel.org/linuxppc-dev/20200511025911.212827-1-ravi.bangoria@linux.ibm.com
v5->v6:
- Rebased to powerpc/next-test which has prefix-instruction support
- Adopt 'struct ppc_inst' in hw-breakpoint code as well
[1]: https://www-355.ibm.com/systems/power/openpower/
Ravi Bangoria (16):
powerpc/watchpoint: Rename current DAWR macros
powerpc/watchpoint: Add SPRN macros for second DAWR
powerpc/watchpoint: Introduce function to get nr watchpoints
dynamically
powerpc/watchpoint/ptrace: Return actual num of available watchpoints
powerpc/watchpoint: Provide DAWR number to set_dawr
powerpc/watchpoint: Provide DAWR number to __set_breakpoint
powerpc/watchpoint: Get watchpoint count dynamically while disabling
them
powerpc/watchpoint: Disable all available watchpoints when
!dawr_force_enable
powerpc/watchpoint: Convert thread_struct->hw_brk to an array
powerpc/watchpoint: Use loop for thread_struct->ptrace_bps
powerpc/watchpoint: Introduce is_ptrace_bp() function
powerpc/watchpoint: Use builtin ALIGN*() macros
powerpc/watchpoint: Prepare handler to handle more than one
watcnhpoint
powerpc/watchpoint: Don't allow concurrent perf and ptrace events
powerpc/watchpoint/xmon: Don't allow breakpoint overwriting
powerpc/watchpoint/xmon: Support 2nd DAWR
arch/powerpc/include/asm/cputable.h | 6 +-
arch/powerpc/include/asm/debug.h | 2 +-
arch/powerpc/include/asm/hw_breakpoint.h | 32 +-
arch/powerpc/include/asm/processor.h | 6 +-
arch/powerpc/include/asm/reg.h | 6 +-
arch/powerpc/include/asm/sstep.h | 2 +
arch/powerpc/kernel/dawr.c | 23 +-
arch/powerpc/kernel/hw_breakpoint.c | 642 ++++++++++++++++++----
arch/powerpc/kernel/process.c | 85 +--
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 72 ++-
arch/powerpc/kernel/ptrace/ptrace32.c | 4 +-
arch/powerpc/kernel/signal.c | 13 +-
arch/powerpc/kvm/book3s_hv.c | 12 +-
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 18 +-
arch/powerpc/xmon/xmon.c | 99 +++-
kernel/events/hw_breakpoint.c | 16 +
16 files changed, 811 insertions(+), 227 deletions(-)
--
2.26.2
^ permalink raw reply
* [PATCH v6 01/16] powerpc/watchpoint: Rename current DAWR macros
From: Ravi Bangoria @ 2020-05-14 11:17 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, apopple, ravi.bangoria, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200514111741.97993-1-ravi.bangoria@linux.ibm.com>
Power10 is introducing second DAWR. Use real register names from ISA
for current macros:
s/SPRN_DAWR/SPRN_DAWR0/
s/SPRN_DAWRX/SPRN_DAWRX0/
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Reviewed-by: Michael Neuling <mikey@neuling.org>
---
arch/powerpc/include/asm/reg.h | 4 ++--
arch/powerpc/kernel/dawr.c | 4 ++--
arch/powerpc/kvm/book3s_hv.c | 12 ++++++------
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 18 +++++++++---------
arch/powerpc/xmon/xmon.c | 2 +-
5 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index f95eb8f97756..60a21b6b2057 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -283,14 +283,14 @@
#define CTRL_CT1 0x40000000 /* thread 1 */
#define CTRL_TE 0x00c00000 /* thread enable */
#define CTRL_RUNLATCH 0x1
-#define SPRN_DAWR 0xB4
+#define SPRN_DAWR0 0xB4
#define SPRN_RPR 0xBA /* Relative Priority Register */
#define SPRN_CIABR 0xBB
#define CIABR_PRIV 0x3
#define CIABR_PRIV_USER 1
#define CIABR_PRIV_SUPER 2
#define CIABR_PRIV_HYPER 3
-#define SPRN_DAWRX 0xBC
+#define SPRN_DAWRX0 0xBC
#define DAWRX_USER __MASK(0)
#define DAWRX_KERNEL __MASK(1)
#define DAWRX_HYP __MASK(2)
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index cc14aa6c4a1b..e91b613bf137 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -39,8 +39,8 @@ int set_dawr(struct arch_hw_breakpoint *brk)
if (ppc_md.set_dawr)
return ppc_md.set_dawr(dawr, dawrx);
- mtspr(SPRN_DAWR, dawr);
- mtspr(SPRN_DAWRX, dawrx);
+ mtspr(SPRN_DAWR0, dawr);
+ mtspr(SPRN_DAWRX0, dawrx);
return 0;
}
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 93493f0cbfe8..db07199f0977 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3392,8 +3392,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
int trap;
unsigned long host_hfscr = mfspr(SPRN_HFSCR);
unsigned long host_ciabr = mfspr(SPRN_CIABR);
- unsigned long host_dawr = mfspr(SPRN_DAWR);
- unsigned long host_dawrx = mfspr(SPRN_DAWRX);
+ unsigned long host_dawr = mfspr(SPRN_DAWR0);
+ unsigned long host_dawrx = mfspr(SPRN_DAWRX0);
unsigned long host_psscr = mfspr(SPRN_PSSCR);
unsigned long host_pidr = mfspr(SPRN_PID);
@@ -3422,8 +3422,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
mtspr(SPRN_SPURR, vcpu->arch.spurr);
if (dawr_enabled()) {
- mtspr(SPRN_DAWR, vcpu->arch.dawr);
- mtspr(SPRN_DAWRX, vcpu->arch.dawrx);
+ mtspr(SPRN_DAWR0, vcpu->arch.dawr);
+ mtspr(SPRN_DAWRX0, vcpu->arch.dawrx);
}
mtspr(SPRN_CIABR, vcpu->arch.ciabr);
mtspr(SPRN_IC, vcpu->arch.ic);
@@ -3475,8 +3475,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
(local_paca->kvm_hstate.fake_suspend << PSSCR_FAKE_SUSPEND_LG));
mtspr(SPRN_HFSCR, host_hfscr);
mtspr(SPRN_CIABR, host_ciabr);
- mtspr(SPRN_DAWR, host_dawr);
- mtspr(SPRN_DAWRX, host_dawrx);
+ mtspr(SPRN_DAWR0, host_dawr);
+ mtspr(SPRN_DAWRX0, host_dawrx);
mtspr(SPRN_PID, host_pidr);
/*
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 780a499c7114..70de3325d0e9 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -707,8 +707,8 @@ BEGIN_FTR_SECTION
END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
BEGIN_FTR_SECTION
mfspr r5, SPRN_CIABR
- mfspr r6, SPRN_DAWR
- mfspr r7, SPRN_DAWRX
+ mfspr r6, SPRN_DAWR0
+ mfspr r7, SPRN_DAWRX0
mfspr r8, SPRN_IAMR
std r5, STACK_SLOT_CIABR(r1)
std r6, STACK_SLOT_DAWR(r1)
@@ -803,8 +803,8 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
beq 1f
ld r5, VCPU_DAWR(r4)
ld r6, VCPU_DAWRX(r4)
- mtspr SPRN_DAWR, r5
- mtspr SPRN_DAWRX, r6
+ mtspr SPRN_DAWR0, r5
+ mtspr SPRN_DAWRX0, r6
1:
ld r7, VCPU_CIABR(r4)
ld r8, VCPU_TAR(r4)
@@ -1766,8 +1766,8 @@ BEGIN_FTR_SECTION
* If the DAWR doesn't work, it's ok to write these here as
* this value should always be zero
*/
- mtspr SPRN_DAWR, r6
- mtspr SPRN_DAWRX, r7
+ mtspr SPRN_DAWR0, r6
+ mtspr SPRN_DAWRX0, r7
END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
BEGIN_FTR_SECTION
ld r5, STACK_SLOT_TID(r1)
@@ -2577,8 +2577,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
mfmsr r6
andi. r6, r6, MSR_DR /* in real mode? */
bne 4f
- mtspr SPRN_DAWR, r4
- mtspr SPRN_DAWRX, r5
+ mtspr SPRN_DAWR0, r4
+ mtspr SPRN_DAWRX0, r5
4: li r3, 0
blr
@@ -3329,7 +3329,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
mtspr SPRN_AMR, r0
mtspr SPRN_IAMR, r0
mtspr SPRN_CIABR, r0
- mtspr SPRN_DAWRX, r0
+ mtspr SPRN_DAWRX0, r0
BEGIN_MMU_FTR_SECTION
b 4f
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index d1a79f9e0566..effb10c2e32f 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1956,7 +1956,7 @@ static void dump_207_sprs(void)
printf("hfscr = %.16lx dhdes = %.16lx rpr = %.16lx\n",
mfspr(SPRN_HFSCR), mfspr(SPRN_DHDES), mfspr(SPRN_RPR));
printf("dawr = %.16lx dawrx = %.16lx ciabr = %.16lx\n",
- mfspr(SPRN_DAWR), mfspr(SPRN_DAWRX), mfspr(SPRN_CIABR));
+ mfspr(SPRN_DAWR0), mfspr(SPRN_DAWRX0), mfspr(SPRN_CIABR));
#endif
}
--
2.26.2
^ permalink raw reply related
* [PATCH v6 02/16] powerpc/watchpoint: Add SPRN macros for second DAWR
From: Ravi Bangoria @ 2020-05-14 11:17 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, apopple, ravi.bangoria, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200514111741.97993-1-ravi.bangoria@linux.ibm.com>
Power10 is introducing second DAWR. Add SPRN_ macros for the same.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Reviewed-by: Michael Neuling <mikey@neuling.org>
---
arch/powerpc/include/asm/reg.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 60a21b6b2057..054f8a71d686 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -284,6 +284,7 @@
#define CTRL_TE 0x00c00000 /* thread enable */
#define CTRL_RUNLATCH 0x1
#define SPRN_DAWR0 0xB4
+#define SPRN_DAWR1 0xB5
#define SPRN_RPR 0xBA /* Relative Priority Register */
#define SPRN_CIABR 0xBB
#define CIABR_PRIV 0x3
@@ -291,6 +292,7 @@
#define CIABR_PRIV_SUPER 2
#define CIABR_PRIV_HYPER 3
#define SPRN_DAWRX0 0xBC
+#define SPRN_DAWRX1 0xBD
#define DAWRX_USER __MASK(0)
#define DAWRX_KERNEL __MASK(1)
#define DAWRX_HYP __MASK(2)
--
2.26.2
^ permalink raw reply related
* [PATCH v6 03/16] powerpc/watchpoint: Introduce function to get nr watchpoints dynamically
From: Ravi Bangoria @ 2020-05-14 11:17 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, apopple, ravi.bangoria, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200514111741.97993-1-ravi.bangoria@linux.ibm.com>
So far we had only one watchpoint, so we have hardcoded HBP_NUM to 1.
But Power10 is introducing 2nd DAWR and thus kernel should be able to
dynamically find actual number of watchpoints supported by hw it's
running on. Introduce function for the same. Also convert HBP_NUM macro
to HBP_NUM_MAX, which will now represent maximum number of watchpoints
supported by Powerpc.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Reviewed-by: Michael Neuling <mikey@neuling.org>
---
arch/powerpc/include/asm/cputable.h | 6 +++++-
arch/powerpc/include/asm/hw_breakpoint.h | 5 +++++
arch/powerpc/include/asm/processor.h | 2 +-
arch/powerpc/kernel/hw_breakpoint.c | 2 +-
4 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 40a4d3c6fd99..c67b94f3334c 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -614,7 +614,11 @@ enum {
};
#endif /* __powerpc64__ */
-#define HBP_NUM 1
+/*
+ * Maximum number of hw breakpoint supported on powerpc. Number of
+ * breakpoints supported by actual hw might be less than this.
+ */
+#define HBP_NUM_MAX 1
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index f2f8d8aa8e3b..518b41eef924 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -43,6 +43,11 @@ struct arch_hw_breakpoint {
#define DABR_MAX_LEN 8
#define DAWR_MAX_LEN 512
+static inline int nr_wp_slots(void)
+{
+ return HBP_NUM_MAX;
+}
+
#ifdef CONFIG_HAVE_HW_BREAKPOINT
#include <linux/kdebug.h>
#include <asm/reg.h>
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 5ab202055d5a..f209c5703ee2 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -180,7 +180,7 @@ struct thread_struct {
int fpexc_mode; /* floating-point exception mode */
unsigned int align_ctl; /* alignment handling control */
#ifdef CONFIG_HAVE_HW_BREAKPOINT
- struct perf_event *ptrace_bps[HBP_NUM];
+ struct perf_event *ptrace_bps[HBP_NUM_MAX];
/*
* Helps identify source of single-step exception and subsequent
* hw-breakpoint enablement
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 423603c92c0f..01f07d91df70 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -39,7 +39,7 @@ static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
int hw_breakpoint_slots(int type)
{
if (type == TYPE_DATA)
- return HBP_NUM;
+ return nr_wp_slots();
return 0; /* no instruction breakpoints available */
}
--
2.26.2
^ permalink raw reply related
* [PATCH v6 04/16] powerpc/watchpoint/ptrace: Return actual num of available watchpoints
From: Ravi Bangoria @ 2020-05-14 11:17 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, apopple, ravi.bangoria, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200514111741.97993-1-ravi.bangoria@linux.ibm.com>
User can ask for num of available watchpoints(dbginfo.num_data_bps)
using ptrace(PPC_PTRACE_GETHWDBGINFO). Return actual number of
available watchpoints on the machine rather than hardcoded 1.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Reviewed-by: Michael Neuling <mikey@neuling.org>
---
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index f87e7c5c3bf3..12962302d6a4 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -44,7 +44,7 @@ void ppc_gethwdinfo(struct ppc_debug_info *dbginfo)
dbginfo->version = 1;
dbginfo->num_instruction_bps = 0;
if (ppc_breakpoint_available())
- dbginfo->num_data_bps = 1;
+ dbginfo->num_data_bps = nr_wp_slots();
else
dbginfo->num_data_bps = 0;
dbginfo->num_condition_regs = 0;
--
2.26.2
^ permalink raw reply related
* [PATCH v6 05/16] powerpc/watchpoint: Provide DAWR number to set_dawr
From: Ravi Bangoria @ 2020-05-14 11:17 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, apopple, ravi.bangoria, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200514111741.97993-1-ravi.bangoria@linux.ibm.com>
Introduce new parameter 'nr' to set_dawr() which indicates which DAWR
should be programed.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Reviewed-by: Michael Neuling <mikey@neuling.org>
---
arch/powerpc/include/asm/hw_breakpoint.h | 4 ++--
arch/powerpc/kernel/dawr.c | 15 ++++++++++-----
arch/powerpc/kernel/process.c | 2 +-
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index 518b41eef924..5b3b02834e0b 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -104,10 +104,10 @@ static inline bool dawr_enabled(void)
{
return dawr_force_enable;
}
-int set_dawr(struct arch_hw_breakpoint *brk);
+int set_dawr(int nr, struct arch_hw_breakpoint *brk);
#else
static inline bool dawr_enabled(void) { return false; }
-static inline int set_dawr(struct arch_hw_breakpoint *brk) { return -1; }
+static inline int set_dawr(int nr, struct arch_hw_breakpoint *brk) { return -1; }
#endif
#endif /* __KERNEL__ */
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index e91b613bf137..8114ad3a8574 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -16,7 +16,7 @@
bool dawr_force_enable;
EXPORT_SYMBOL_GPL(dawr_force_enable);
-int set_dawr(struct arch_hw_breakpoint *brk)
+int set_dawr(int nr, struct arch_hw_breakpoint *brk)
{
unsigned long dawr, dawrx, mrd;
@@ -39,15 +39,20 @@ int set_dawr(struct arch_hw_breakpoint *brk)
if (ppc_md.set_dawr)
return ppc_md.set_dawr(dawr, dawrx);
- mtspr(SPRN_DAWR0, dawr);
- mtspr(SPRN_DAWRX0, dawrx);
+ if (nr == 0) {
+ mtspr(SPRN_DAWR0, dawr);
+ mtspr(SPRN_DAWRX0, dawrx);
+ } else {
+ mtspr(SPRN_DAWR1, dawr);
+ mtspr(SPRN_DAWRX1, dawrx);
+ }
return 0;
}
static void set_dawr_cb(void *info)
{
- set_dawr(info);
+ set_dawr(0, info);
}
static ssize_t dawr_write_file_bool(struct file *file,
@@ -60,7 +65,7 @@ static ssize_t dawr_write_file_bool(struct file *file,
/* Send error to user if they hypervisor won't allow us to write DAWR */
if (!dawr_force_enable &&
firmware_has_feature(FW_FEATURE_LPAR) &&
- set_dawr(&null_brk) != H_SUCCESS)
+ set_dawr(0, &null_brk) != H_SUCCESS)
return -ENODEV;
rc = debugfs_write_file_bool(file, user_buf, count, ppos);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index db766252238f..dc161b0adc82 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -806,7 +806,7 @@ void __set_breakpoint(struct arch_hw_breakpoint *brk)
if (dawr_enabled())
// Power8 or later
- set_dawr(brk);
+ set_dawr(0, brk);
else if (IS_ENABLED(CONFIG_PPC_8xx))
set_breakpoint_8xx(brk);
else if (!cpu_has_feature(CPU_FTR_ARCH_207S))
--
2.26.2
^ permalink raw reply related
* [PATCH v6 06/16] powerpc/watchpoint: Provide DAWR number to __set_breakpoint
From: Ravi Bangoria @ 2020-05-14 11:17 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, apopple, ravi.bangoria, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200514111741.97993-1-ravi.bangoria@linux.ibm.com>
Introduce new parameter 'nr' to __set_breakpoint() which indicates
which DAWR should be programed. Also convert current_brk variable
to an array.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Reviewed-by: Michael Neuling <mikey@neuling.org>
---
arch/powerpc/include/asm/debug.h | 2 +-
arch/powerpc/include/asm/hw_breakpoint.h | 2 +-
arch/powerpc/kernel/hw_breakpoint.c | 8 ++++----
arch/powerpc/kernel/process.c | 14 +++++++-------
arch/powerpc/kernel/signal.c | 2 +-
arch/powerpc/xmon/xmon.c | 2 +-
6 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
index 7756026b95ca..ec57daf87f40 100644
--- a/arch/powerpc/include/asm/debug.h
+++ b/arch/powerpc/include/asm/debug.h
@@ -45,7 +45,7 @@ static inline int debugger_break_match(struct pt_regs *regs) { return 0; }
static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
#endif
-void __set_breakpoint(struct arch_hw_breakpoint *brk);
+void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk);
bool ppc_breakpoint_available(void);
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
extern void do_send_trap(struct pt_regs *regs, unsigned long address,
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index 5b3b02834e0b..1120c7d9db58 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -85,7 +85,7 @@ static inline void hw_breakpoint_disable(void)
brk.len = 0;
brk.hw_len = 0;
if (ppc_breakpoint_available())
- __set_breakpoint(&brk);
+ __set_breakpoint(0, &brk);
}
extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
int hw_breakpoint_handler(struct die_args *args);
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 01f07d91df70..f5472402c06d 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -64,7 +64,7 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
* If so, DABR will be populated in single_step_dabr_instruction().
*/
if (current->thread.last_hit_ubp != bp)
- __set_breakpoint(info);
+ __set_breakpoint(0, info);
return 0;
}
@@ -222,7 +222,7 @@ void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs)
info = counter_arch_bp(tsk->thread.last_hit_ubp);
regs->msr &= ~MSR_SE;
- __set_breakpoint(info);
+ __set_breakpoint(0, info);
tsk->thread.last_hit_ubp = NULL;
}
@@ -347,7 +347,7 @@ int hw_breakpoint_handler(struct die_args *args)
if (!(info->type & HW_BRK_TYPE_EXTRANEOUS_IRQ))
perf_bp_event(bp, regs);
- __set_breakpoint(info);
+ __set_breakpoint(0, info);
out:
rcu_read_unlock();
return rc;
@@ -380,7 +380,7 @@ static int single_step_dabr_instruction(struct die_args *args)
if (!(info->type & HW_BRK_TYPE_EXTRANEOUS_IRQ))
perf_bp_event(bp, regs);
- __set_breakpoint(info);
+ __set_breakpoint(0, info);
current->thread.last_hit_ubp = NULL;
/*
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index dc161b0adc82..f303aea61794 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -637,7 +637,7 @@ void do_break (struct pt_regs *regs, unsigned long address,
}
#endif /* CONFIG_PPC_ADV_DEBUG_REGS */
-static DEFINE_PER_CPU(struct arch_hw_breakpoint, current_brk);
+static DEFINE_PER_CPU(struct arch_hw_breakpoint, current_brk[HBP_NUM_MAX]);
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
/*
@@ -714,7 +714,7 @@ EXPORT_SYMBOL_GPL(switch_booke_debug_regs);
static void set_breakpoint(struct arch_hw_breakpoint *brk)
{
preempt_disable();
- __set_breakpoint(brk);
+ __set_breakpoint(0, brk);
preempt_enable();
}
@@ -800,13 +800,13 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk)
return 0;
}
-void __set_breakpoint(struct arch_hw_breakpoint *brk)
+void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk)
{
- memcpy(this_cpu_ptr(¤t_brk), brk, sizeof(*brk));
+ memcpy(this_cpu_ptr(¤t_brk[nr]), brk, sizeof(*brk));
if (dawr_enabled())
// Power8 or later
- set_dawr(0, brk);
+ set_dawr(nr, brk);
else if (IS_ENABLED(CONFIG_PPC_8xx))
set_breakpoint_8xx(brk);
else if (!cpu_has_feature(CPU_FTR_ARCH_207S))
@@ -1174,8 +1174,8 @@ struct task_struct *__switch_to(struct task_struct *prev,
* schedule DABR
*/
#ifndef CONFIG_HAVE_HW_BREAKPOINT
- if (unlikely(!hw_brk_match(this_cpu_ptr(¤t_brk), &new->thread.hw_brk)))
- __set_breakpoint(&new->thread.hw_brk);
+ if (unlikely(!hw_brk_match(this_cpu_ptr(¤t_brk[0]), &new->thread.hw_brk)))
+ __set_breakpoint(0, &new->thread.hw_brk);
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
#endif
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index a46c3fdb6853..8e29138a344a 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -272,7 +272,7 @@ static void do_signal(struct task_struct *tsk)
* triggered inside the kernel.
*/
if (tsk->thread.hw_brk.address && tsk->thread.hw_brk.type)
- __set_breakpoint(&tsk->thread.hw_brk);
+ __set_breakpoint(0, &tsk->thread.hw_brk);
#endif
/* Re-enable the breakpoints for the signal stack */
thread_change_pc(tsk, tsk->thread.regs);
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index effb10c2e32f..30b3e3d99c0d 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -954,7 +954,7 @@ static void insert_cpu_bpts(void)
brk.address = dabr.address;
brk.type = (dabr.enabled & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL;
brk.len = DABR_MAX_LEN;
- __set_breakpoint(&brk);
+ __set_breakpoint(0, &brk);
}
if (iabr)
--
2.26.2
^ permalink raw reply related
* [PATCH v6 07/16] powerpc/watchpoint: Get watchpoint count dynamically while disabling them
From: Ravi Bangoria @ 2020-05-14 11:17 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, apopple, ravi.bangoria, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200514111741.97993-1-ravi.bangoria@linux.ibm.com>
Instead of disabling only one watchpoint, get num of available
watchpoints dynamically and disable all of them.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Reviewed-by: Michael Neuling <mikey@neuling.org>
---
arch/powerpc/include/asm/hw_breakpoint.h | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index 1120c7d9db58..d472b2eb757e 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -78,14 +78,14 @@ extern void ptrace_triggered(struct perf_event *bp,
struct perf_sample_data *data, struct pt_regs *regs);
static inline void hw_breakpoint_disable(void)
{
- struct arch_hw_breakpoint brk;
-
- brk.address = 0;
- brk.type = 0;
- brk.len = 0;
- brk.hw_len = 0;
- if (ppc_breakpoint_available())
- __set_breakpoint(0, &brk);
+ int i;
+ struct arch_hw_breakpoint null_brk = {0};
+
+ if (!ppc_breakpoint_available())
+ return;
+
+ for (i = 0; i < nr_wp_slots(); i++)
+ __set_breakpoint(i, &null_brk);
}
extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
int hw_breakpoint_handler(struct die_args *args);
--
2.26.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox