* Re: [PATCH 2/3] powerpc: Fix incorrect stw{, ux, u, x} instructions in __set_pte_at
From: Segher Boessenkool @ 2020-10-19 20:14 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <1b26e1b8544ea46ad0da102d1367694cd23c222c.1603109522.git.christophe.leroy@csgroup.eu>
On Mon, Oct 19, 2020 at 12:12:47PM +0000, Christophe Leroy wrote:
> From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>
> The placeholder for instruction selection should use the second
> argument's operand, which is %1, not %0. This could generate incorrect
> assembly code if the instruction selection for argument %0 ever differs
> from argument %1.
"Instruction selection" isn't correct here... "if the memory addressing
of operand 0 is a different form from that of operand 1", perhaps?
The patch looks fine of course :-)
Acked-by: Segher Boessenkool <segher@kernel.crashing.org>
Segher
^ permalink raw reply
* Re: [PATCH 3/3] powerpc: Fix pre-update addressing in inline assembly
From: Segher Boessenkool @ 2020-10-19 20:24 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <fbcdb173cc42da62f00285dfef8c2f7d4960b5c7.1603109522.git.christophe.leroy@csgroup.eu>
On Mon, Oct 19, 2020 at 12:12:48PM +0000, Christophe Leroy wrote:
> In several places, inline assembly uses the "%Un" modifier
> to enable the use of instruction with pre-update addressing,
Calling this "pre-update" is misleading: the register is not updated
before the address is generated (or the memory access done!), and the
addressing is exactly the same as the "non-u" insn would use. It is
called an "update form" instruction, because (at the same time as doing
the memory access, logically anyway) it writes back the address used to
the base register.
> but the associated "<>" constraint is missing.
But that is just fine. Pointless, sure, but not a bug.
> Use UPD_CONSTR macro everywhere %Un modifier is used.
Eww. My poor stomach.
Have you verified that update form is *correct* in all these, and that
we even *want* this there?
Segher
^ permalink raw reply
* Re: [PATCH 1/2] ASoC: dt-bindings: fsl_spdif: Add new compatible string for i.MX8QM
From: Rob Herring @ 2020-10-19 21:14 UTC (permalink / raw)
To: Shengjiu Wang
Cc: devicetree, alsa-devel, timur, Xiubo.Lee, festevam, broonie,
lgirdwood, tiwai, nicoleotsuka, robh+dt, perex, linuxppc-dev,
linux-kernel
In-Reply-To: <1602739728-4433-1-git-send-email-shengjiu.wang@nxp.com>
On Thu, 15 Oct 2020 13:28:47 +0800, Shengjiu Wang wrote:
> Add new compatible string "fsl,imx8qm-spdif" for supporting spdif
> module on i.MX8QM.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
> Documentation/devicetree/bindings/sound/fsl,spdif.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH] powerpc/boot: move the .got section to after the .dynamic section
From: Fāng-ruì Sòng @ 2020-10-19 23:56 UTC (permalink / raw)
To: Bill Wendling; +Cc: Alan Modra, linuxppc-dev, Paul Mackerras
In-Reply-To: <20201017000151.150788-1-morbo@google.com>
On Fri, Oct 16, 2020 at 5:01 PM Bill Wendling <morbo@google.com> wrote:
>
> Both .dynamic and .got are RELRO sections and should be placed together,
> and LLD emits an error:
>
> ld.lld: error: section: .got is not contiguous with other relro sections
>
> Place them together to avoid this.
>
> Cc: Fangrui Song <maskray@google.com>
> Cc: Alan Modra <amodra@gmail.com>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> arch/powerpc/boot/zImage.lds.S | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/boot/zImage.lds.S b/arch/powerpc/boot/zImage.lds.S
> index a21f3a76e06f..d6f072865627 100644
> --- a/arch/powerpc/boot/zImage.lds.S
> +++ b/arch/powerpc/boot/zImage.lds.S
> @@ -34,6 +34,17 @@ SECTIONS
> __dynamic_start = .;
> *(.dynamic)
> }
> +
> +#ifdef CONFIG_PPC64_BOOT_WRAPPER
> + . = ALIGN(256);
> + .got :
> + {
> + __toc_start = .;
> + *(.got)
> + *(.toc)
> + }
> +#endif
> +
> .hash : { *(.hash) }
> .interp : { *(.interp) }
> .rela.dyn :
> @@ -76,16 +87,6 @@ SECTIONS
> _esm_blob_end = .;
> }
>
> -#ifdef CONFIG_PPC64_BOOT_WRAPPER
> - . = ALIGN(256);
> - .got :
> - {
> - __toc_start = .;
> - *(.got)
> - *(.toc)
> - }
> -#endif
The kernel does not require this but normally all read-only sections
precede SHF_WRITE sections.
.dynamic and .got have the SHF_WRITE flag and should be placed here.
Ideally, the order is: R RX RW(RELRO) RW(non-RELRO) (LLD order)
For comparison:
GNU ld -z separate-code order: R RX R RW(RELRO) RW(non-RELRO) (GNU
ld>=2.31 enables -z separate-code by default for Linux x86)
GNU ld -z noseparate-code order: RX RW(RELRO) RW(non-RELRO)
(AFAIK The only reason .dynamic is writable is due to DT_DEBUG (whose
purpose is questionable nowadays). mips .dynamic is read-only. LLD has
an option -z rodynamic to make .dynamic readonly)
> . = ALIGN(4096);
> .bss :
> {
> --
> 2.29.0.rc1.297.gfa9743e501-goog
>
^ permalink raw reply
* Re: [PATCH 3/8] powerpc: Mark functions called inside uaccess blocks w/ 'notrace'
From: Christopher M. Riedl @ 2020-10-20 1:59 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <30d9ccba-20a5-37b4-1629-d0f9e0da4c84@csgroup.eu>
On Fri Oct 16, 2020 at 4:02 AM CDT, Christophe Leroy wrote:
>
>
> Le 15/10/2020 à 17:01, Christopher M. Riedl a écrit :
> > Functions called between user_*_access_begin() and user_*_access_end()
> > should be either inlined or marked 'notrace' to prevent leaving
> > userspace access exposed. Mark any such functions relevant to signal
> > handling so that subsequent patches can call them inside uaccess blocks.
>
> Is it enough to mark it "notrace" ? I see that when I activate KASAN,
> there are still KASAN calls in
> those functions.
>
Maybe not enough after all :(
> In my series for 32 bits, I re-ordered stuff in order to do all those
> calls before doing the
> _access_begin(), can't you do the same on PPC64 ? (See
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/f6eac65781b4a57220477c8864bca2b57f29a5d5.1597770847.git.christophe.leroy@csgroup.eu/)
>
Yes, I will give this another shot in the next spin.
> Christophe
>
> >
> > Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
> > ---
> > arch/powerpc/kernel/process.c | 20 ++++++++++----------
> > arch/powerpc/mm/mem.c | 4 ++--
> > 2 files changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> > index ba2c987b8403..bf5d9654bd2c 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -84,7 +84,7 @@ extern unsigned long _get_SP(void);
> > */
> > bool tm_suspend_disabled __ro_after_init = false;
> >
> > -static void check_if_tm_restore_required(struct task_struct *tsk)
> > +static void notrace check_if_tm_restore_required(struct task_struct *tsk)
> > {
> > /*
> > * If we are saving the current thread's registers, and the
> > @@ -151,7 +151,7 @@ void notrace __msr_check_and_clear(unsigned long bits)
> > EXPORT_SYMBOL(__msr_check_and_clear);
> >
> > #ifdef CONFIG_PPC_FPU
> > -static void __giveup_fpu(struct task_struct *tsk)
> > +static void notrace __giveup_fpu(struct task_struct *tsk)
> > {
> > unsigned long msr;
> >
> > @@ -163,7 +163,7 @@ static void __giveup_fpu(struct task_struct *tsk)
> > tsk->thread.regs->msr = msr;
> > }
> >
> > -void giveup_fpu(struct task_struct *tsk)
> > +void notrace giveup_fpu(struct task_struct *tsk)
> > {
> > check_if_tm_restore_required(tsk);
> >
> > @@ -177,7 +177,7 @@ EXPORT_SYMBOL(giveup_fpu);
> > * Make sure the floating-point register state in the
> > * the thread_struct is up to date for task tsk.
> > */
> > -void flush_fp_to_thread(struct task_struct *tsk)
> > +void notrace flush_fp_to_thread(struct task_struct *tsk)
> > {
> > if (tsk->thread.regs) {
> > /*
> > @@ -234,7 +234,7 @@ static inline void __giveup_fpu(struct task_struct *tsk) { }
> > #endif /* CONFIG_PPC_FPU */
> >
> > #ifdef CONFIG_ALTIVEC
> > -static void __giveup_altivec(struct task_struct *tsk)
> > +static void notrace __giveup_altivec(struct task_struct *tsk)
> > {
> > unsigned long msr;
> >
> > @@ -246,7 +246,7 @@ static void __giveup_altivec(struct task_struct *tsk)
> > tsk->thread.regs->msr = msr;
> > }
> >
> > -void giveup_altivec(struct task_struct *tsk)
> > +void notrace giveup_altivec(struct task_struct *tsk)
> > {
> > check_if_tm_restore_required(tsk);
> >
> > @@ -285,7 +285,7 @@ EXPORT_SYMBOL(enable_kernel_altivec);
> > * Make sure the VMX/Altivec register state in the
> > * the thread_struct is up to date for task tsk.
> > */
> > -void flush_altivec_to_thread(struct task_struct *tsk)
> > +void notrace flush_altivec_to_thread(struct task_struct *tsk)
> > {
> > if (tsk->thread.regs) {
> > preempt_disable();
> > @@ -300,7 +300,7 @@ EXPORT_SYMBOL_GPL(flush_altivec_to_thread);
> > #endif /* CONFIG_ALTIVEC */
> >
> > #ifdef CONFIG_VSX
> > -static void __giveup_vsx(struct task_struct *tsk)
> > +static void notrace __giveup_vsx(struct task_struct *tsk)
> > {
> > unsigned long msr = tsk->thread.regs->msr;
> >
> > @@ -317,7 +317,7 @@ static void __giveup_vsx(struct task_struct *tsk)
> > __giveup_altivec(tsk);
> > }
> >
> > -static void giveup_vsx(struct task_struct *tsk)
> > +static void notrace giveup_vsx(struct task_struct *tsk)
> > {
> > check_if_tm_restore_required(tsk);
> >
> > @@ -352,7 +352,7 @@ void enable_kernel_vsx(void)
> > }
> > EXPORT_SYMBOL(enable_kernel_vsx);
> >
> > -void flush_vsx_to_thread(struct task_struct *tsk)
> > +void notrace flush_vsx_to_thread(struct task_struct *tsk)
> > {
> > if (tsk->thread.regs) {
> > preempt_disable();
> > diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> > index ddc32cc1b6cf..da2345a2abc6 100644
> > --- a/arch/powerpc/mm/mem.c
> > +++ b/arch/powerpc/mm/mem.c
> > @@ -378,7 +378,7 @@ static inline bool flush_coherent_icache(unsigned long addr)
> > * @start: the start address
> > * @stop: the stop address (exclusive)
> > */
> > -static void invalidate_icache_range(unsigned long start, unsigned long stop)
> > +static void notrace invalidate_icache_range(unsigned long start, unsigned long stop)
> > {
> > unsigned long shift = l1_icache_shift();
> > unsigned long bytes = l1_icache_bytes();
> > @@ -402,7 +402,7 @@ static void invalidate_icache_range(unsigned long start, unsigned long stop)
> > * @start: the start address
> > * @stop: the stop address (exclusive)
> > */
> > -void flush_icache_range(unsigned long start, unsigned long stop)
> > +void notrace flush_icache_range(unsigned long start, unsigned long stop)
> > {
> > if (flush_coherent_icache(start))
> > return;
> >
^ permalink raw reply
* [PATCH v3] soc: fsl: dpio: Get the cpumask through cpumask_of(cpu)
From: Yi Wang @ 2020-10-20 2:18 UTC (permalink / raw)
To: Roy.Pledge, leoyang.li
Cc: wang.yi59, jiang.xuexin, Hao Si, linux-kernel, Lin Chen,
xue.zhihong, linuxppc-dev, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 3858 bytes --]
From: Hao Si <si.hao@zte.com.cn>
The local variable 'cpumask_t mask' is in the stack memory, and its address
is assigned to 'desc->affinity' in 'irq_set_affinity_hint()'.
But the memory area where this variable is located is at risk of being
modified.
During LTP testing, the following error was generated:
Unable to handle kernel paging request at virtual address ffff000012e9b790
Mem abort info:
ESR = 0x96000007
Exception class = DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
Data abort info:
ISV = 0, ISS = 0x00000007
CM = 0, WnR = 0
swapper pgtable: 4k pages, 48-bit VAs, pgdp = 0000000075ac5e07
[ffff000012e9b790] pgd=00000027dbffe003, pud=00000027dbffd003,
pmd=00000027b6d61003, pte=0000000000000000
Internal error: Oops: 96000007 [#1] PREEMPT SMP
Modules linked in: xt_conntrack
Process read_all (pid: 20171, stack limit = 0x0000000044ea4095)
CPU: 14 PID: 20171 Comm: read_all Tainted: G B W
Hardware name: NXP Layerscape LX2160ARDB (DT)
pstate: 80000085 (Nzcv daIf -PAN -UAO)
pc : irq_affinity_hint_proc_show+0x54/0xb0
lr : irq_affinity_hint_proc_show+0x4c/0xb0
sp : ffff00001138bc10
x29: ffff00001138bc10 x28: 0000ffffd131d1e0
x27: 00000000007000c0 x26: ffff8025b9480dc0
x25: ffff8025b9480da8 x24: 00000000000003ff
x23: ffff8027334f8300 x22: ffff80272e97d000
x21: ffff80272e97d0b0 x20: ffff8025b9480d80
x19: ffff000009a49000 x18: 0000000000000000
x17: 0000000000000000 x16: 0000000000000000
x15: 0000000000000000 x14: 0000000000000000
x13: 0000000000000000 x12: 0000000000000040
x11: 0000000000000000 x10: ffff802735b79b88
x9 : 0000000000000000 x8 : 0000000000000000
x7 : ffff000009a49848 x6 : 0000000000000003
x5 : 0000000000000000 x4 : ffff000008157d6c
x3 : ffff00001138bc10 x2 : ffff000012e9b790
x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
irq_affinity_hint_proc_show+0x54/0xb0
seq_read+0x1b0/0x440
proc_reg_read+0x80/0xd8
__vfs_read+0x60/0x178
vfs_read+0x94/0x150
ksys_read+0x74/0xf0
__arm64_sys_read+0x24/0x30
el0_svc_common.constprop.0+0xd8/0x1a0
el0_svc_handler+0x34/0x88
el0_svc+0x10/0x14
Code: f9001bbf 943e0732 f94066c2 b4000062 (f9400041)
---[ end trace b495bdcb0b3b732b ]---
Kernel panic - not syncing: Fatal exception
SMP: stopping secondary CPUs
SMP: failed to stop secondary CPUs 0,2-4,6,8,11,13-15
Kernel Offset: disabled
CPU features: 0x0,21006008
Memory Limit: none
---[ end Kernel panic - not syncing: Fatal exception ]---
Fix it by using 'cpumask_of(cpu)' to get the cpumask.
Signed-off-by: Hao Si <si.hao@zte.com.cn>
Signed-off-by: Lin Chen <chen.lin5@zte.com.cn>
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
---
v3: Use cpumask_of(cpu) to get the pre-defined cpumask in the static
cpu_bit_bitmap array.
v2: Place 'cpumask_t mask' in the driver's private data and while at it,
rename it to cpu_mask.
drivers/soc/fsl/dpio/dpio-driver.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/soc/fsl/dpio/dpio-driver.c b/drivers/soc/fsl/dpio/dpio-driver.c
index 7b642c3..7f397b4 100644
--- a/drivers/soc/fsl/dpio/dpio-driver.c
+++ b/drivers/soc/fsl/dpio/dpio-driver.c
@@ -95,7 +95,6 @@ static int register_dpio_irq_handlers(struct fsl_mc_device *dpio_dev, int cpu)
{
int error;
struct fsl_mc_device_irq *irq;
- cpumask_t mask;
irq = dpio_dev->irqs[0];
error = devm_request_irq(&dpio_dev->dev,
@@ -112,9 +111,7 @@ static int register_dpio_irq_handlers(struct fsl_mc_device *dpio_dev, int cpu)
}
/* set the affinity hint */
- cpumask_clear(&mask);
- cpumask_set_cpu(cpu, &mask);
- if (irq_set_affinity_hint(irq->msi_desc->irq, &mask))
+ if (irq_set_affinity_hint(irq->msi_desc->irq, cpumask_of(cpu)))
dev_err(&dpio_dev->dev,
"irq_set_affinity failed irq %d cpu %d\n",
irq->msi_desc->irq, cpu);
--
2.15.2
^ permalink raw reply related
* Re: [PATCH 2/8] powerpc/signal: Add unsafe_copy_{vsx,fpr}_from_user()
From: Christopher M. Riedl @ 2020-10-20 2:01 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <5e15b794-e33e-a871-3296-df1154b7d408@csgroup.eu>
On Fri Oct 16, 2020 at 10:48 AM CDT, Christophe Leroy wrote:
>
>
> Le 15/10/2020 à 17:01, Christopher M. Riedl a écrit :
> > Reuse the "safe" implementation from signal.c except for calling
> > unsafe_copy_from_user() to copy into a local buffer. Unlike the
> > unsafe_copy_{vsx,fpr}_to_user() functions the "copy from" functions
> > cannot use unsafe_get_user() directly to bypass the local buffer since
> > doing so significantly reduces signal handling performance.
>
> Why can't the functions use unsafe_get_user(), why does it significantly
> reduces signal handling
> performance ? How much significant ? I would expect that not going
> through an intermediate memory
> area would be more efficient
>
Here is a comparison, 'unsafe-signal64-regs' avoids the intermediate buffer:
| | hash | radix |
| -------------------- | ------ | ------ |
| linuxppc/next | 289014 | 158408 |
| unsafe-signal64 | 298506 | 253053 |
| unsafe-signal64-regs | 254898 | 220831 |
I have not figured out the 'why' yet. As you mentioned in your series,
technically calling __copy_tofrom_user() is overkill for these
operations. The only obvious difference between unsafe_put_user() and
unsafe_get_user() is that we don't have asm-goto for the 'get' variant.
Instead we wrap with unsafe_op_wrap() which inserts a conditional and
then goto to the label.
Implemenations:
#define unsafe_copy_fpr_from_user(task, from, label) do { \
struct task_struct *__t = task; \
u64 __user *buf = (u64 __user *)from; \
int i; \
\
for (i = 0; i < ELF_NFPREG - 1; i++) \
unsafe_get_user(__t->thread.TS_FPR(i), &buf[i], label); \
unsafe_get_user(__t->thread.fp_state.fpscr, &buf[i], label); \
} while (0)
#define unsafe_copy_vsx_from_user(task, from, label) do { \
struct task_struct *__t = task; \
u64 __user *buf = (u64 __user *)from; \
int i; \
\
for (i = 0; i < ELF_NVSRHALFREG ; i++) \
unsafe_get_user(__t->thread.fp_state.fpr[i][TS_VSRLOWOFFSET], \
&buf[i], label); \
} while (0)
> Christophe
>
>
> >
> > Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
> > ---
> > arch/powerpc/kernel/signal.h | 33 +++++++++++++++++++++++++++++++++
> > 1 file changed, 33 insertions(+)
> >
> > diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
> > index 2559a681536e..e9aaeac0da37 100644
> > --- a/arch/powerpc/kernel/signal.h
> > +++ b/arch/powerpc/kernel/signal.h
> > @@ -53,6 +53,33 @@ unsigned long copy_ckfpr_from_user(struct task_struct *task, void __user *from);
> > &buf[i], label);\
> > } while (0)
> >
> > +#define unsafe_copy_fpr_from_user(task, from, label) do { \
> > + struct task_struct *__t = task; \
> > + u64 __user *__f = (u64 __user *)from; \
> > + u64 buf[ELF_NFPREG]; \
> > + int i; \
> > + \
> > + unsafe_copy_from_user(buf, __f, ELF_NFPREG * sizeof(double), \
> > + label); \
> > + for (i = 0; i < ELF_NFPREG - 1; i++) \
> > + __t->thread.TS_FPR(i) = buf[i]; \
> > + __t->thread.fp_state.fpscr = buf[i]; \
> > +} while (0)
> > +
> > +#define unsafe_copy_vsx_from_user(task, from, label) do { \
> > + struct task_struct *__t = task; \
> > + u64 __user *__f = (u64 __user *)from; \
> > + u64 buf[ELF_NVSRHALFREG]; \
> > + int i; \
> > + \
> > + unsafe_copy_from_user(buf, __f, \
> > + ELF_NVSRHALFREG * sizeof(double), \
> > + label); \
> > + for (i = 0; i < ELF_NVSRHALFREG ; i++) \
> > + __t->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = buf[i]; \
> > +} while (0)
> > +
> > +
> > #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> > #define unsafe_copy_ckfpr_to_user(to, task, label) do { \
> > struct task_struct *__t = task; \
> > @@ -80,6 +107,10 @@ unsigned long copy_ckfpr_from_user(struct task_struct *task, void __user *from);
> > unsafe_copy_to_user(to, (task)->thread.fp_state.fpr, \
> > ELF_NFPREG * sizeof(double), label)
> >
> > +#define unsafe_copy_fpr_from_user(task, from, label) \
> > + unsafe_copy_from_user((task)->thread.fp_state.fpr, from \
> > + ELF_NFPREG * sizeof(double), label)
> > +
> > static inline unsigned long
> > copy_fpr_to_user(void __user *to, struct task_struct *task)
> > {
> > @@ -115,6 +146,8 @@ copy_ckfpr_from_user(struct task_struct *task, void __user *from)
> > #else
> > #define unsafe_copy_fpr_to_user(to, task, label) do { } while (0)
> >
> > +#define unsafe_copy_fpr_from_user(task, from, label) do { } while (0)
> > +
> > static inline unsigned long
> > copy_fpr_to_user(void __user *to, struct task_struct *task)
> > {
> >
^ permalink raw reply
* Re: [PATCH 6/8] powerpc/signal64: Replace setup_trampoline() w/ unsafe_setup_trampoline()
From: Christopher M. Riedl @ 2020-10-20 2:42 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev; +Cc: Daniel Axtens
In-Reply-To: <c447ee06-a581-1f5f-42c7-814a1f570fe0@csgroup.eu>
On Fri Oct 16, 2020 at 10:56 AM CDT, Christophe Leroy wrote:
>
>
> Le 15/10/2020 à 17:01, Christopher M. Riedl a écrit :
> > From: Daniel Axtens <dja@axtens.net>
> >
> > Previously setup_trampoline() performed a costly KUAP switch on every
> > uaccess operation. These repeated uaccess switches cause a significant
> > drop in signal handling performance.
> >
> > Rewrite setup_trampoline() to assume that a userspace write access
> > window is open. Replace all uaccess functions with their 'unsafe'
> > versions to avoid the repeated uaccess switches.
> >
> > Signed-off-by: Daniel Axtens <dja@axtens.net>
> > Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
> > ---
> > arch/powerpc/kernel/signal_64.c | 32 +++++++++++++++++++-------------
> > 1 file changed, 19 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> > index bd92064e5576..6d4f7a5c4fbf 100644
> > --- a/arch/powerpc/kernel/signal_64.c
> > +++ b/arch/powerpc/kernel/signal_64.c
> > @@ -600,30 +600,33 @@ static long restore_tm_sigcontexts(struct task_struct *tsk,
> > /*
> > * Setup the trampoline code on the stack
> > */
> > -static long setup_trampoline(unsigned int syscall, unsigned int __user *tramp)
> > +#define unsafe_setup_trampoline(syscall, tramp, e) \
> > + unsafe_op_wrap(__unsafe_setup_trampoline(syscall, tramp), e)
> > +static long notrace __unsafe_setup_trampoline(unsigned int syscall,
> > + unsigned int __user *tramp)
> > {
> > int i;
> > - long err = 0;
> >
> > /* bctrl # call the handler */
> > - err |= __put_user(PPC_INST_BCTRL, &tramp[0]);
> > + unsafe_put_user(PPC_INST_BCTRL, &tramp[0], err);
> > /* addi r1, r1, __SIGNAL_FRAMESIZE # Pop the dummy stackframe */
> > - err |= __put_user(PPC_INST_ADDI | __PPC_RT(R1) | __PPC_RA(R1) |
> > - (__SIGNAL_FRAMESIZE & 0xffff), &tramp[1]);
> > + unsafe_put_user(PPC_INST_ADDI | __PPC_RT(R1) | __PPC_RA(R1) |
> > + (__SIGNAL_FRAMESIZE & 0xffff), &tramp[1], err);
> > /* li r0, __NR_[rt_]sigreturn| */
> > - err |= __put_user(PPC_INST_ADDI | (syscall & 0xffff), &tramp[2]);
> > + unsafe_put_user(PPC_INST_ADDI | (syscall & 0xffff), &tramp[2], err);
> > /* sc */
> > - err |= __put_user(PPC_INST_SC, &tramp[3]);
> > + unsafe_put_user(PPC_INST_SC, &tramp[3], err);
> >
> > /* Minimal traceback info */
> > for (i=TRAMP_TRACEBACK; i < TRAMP_SIZE ;i++)
> > - err |= __put_user(0, &tramp[i]);
> > + unsafe_put_user(0, &tramp[i], err);
> >
> > - if (!err)
> > - flush_icache_range((unsigned long) &tramp[0],
> > - (unsigned long) &tramp[TRAMP_SIZE]);
> > + flush_icache_range((unsigned long)&tramp[0],
> > + (unsigned long)&tramp[TRAMP_SIZE]);
>
> This flush should be done outside the user_write_access block.
>
Hmm, I suppose that means setup_trampoline() cannot be completely
"unsafe". I'll see if I can re-arrange the code which calls this
function to avoid an additional uaccess block instead and push the
start()/end() into setup_trampoline() directly.
> >
> > - return err;
> > + return 0;
> > +err:
> > + return 1;
> > }
> >
> > /*
> > @@ -888,7 +891,10 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
> > if (vdso64_rt_sigtramp && tsk->mm->context.vdso_base) {
> > regs->nip = tsk->mm->context.vdso_base + vdso64_rt_sigtramp;
> > } else {
> > - err |= setup_trampoline(__NR_rt_sigreturn, &frame->tramp[0]);
> > + if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
> > + return -EFAULT;
> > + err |= __unsafe_setup_trampoline(__NR_rt_sigreturn, &frame->tramp[0]);
> > + user_write_access_end();
> > if (err)
> > goto badframe;
> > regs->nip = (unsigned long) &frame->tramp[0];
> >
>
> Christophe
^ permalink raw reply
* Re: [PATCH 7/8] powerpc/signal64: Rewrite handle_rt_signal64() to minimise uaccess switches
From: Christopher M. Riedl @ 2020-10-20 2:44 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev; +Cc: Daniel Axtens
In-Reply-To: <1cedcf43-4594-5db1-d248-7c06a572aecc@csgroup.eu>
On Fri Oct 16, 2020 at 11:00 AM CDT, Christophe Leroy wrote:
>
>
> Le 15/10/2020 à 17:01, Christopher M. Riedl a écrit :
> > From: Daniel Axtens <dja@axtens.net>
> >
> > Add uaccess blocks and use the 'unsafe' versions of functions doing user
> > access where possible to reduce the number of times uaccess has to be
> > opened/closed.
> >
> > There is no 'unsafe' version of copy_siginfo_to_user, so move it
> > slightly to allow for a "longer" uaccess block.
> >
> > Signed-off-by: Daniel Axtens <dja@axtens.net>
> > Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
> > ---
> > arch/powerpc/kernel/signal_64.c | 54 ++++++++++++++++-----------------
> > 1 file changed, 27 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> > index 6d4f7a5c4fbf..3b97e3681a8f 100644
> > --- a/arch/powerpc/kernel/signal_64.c
> > +++ b/arch/powerpc/kernel/signal_64.c
> > @@ -843,46 +843,42 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
> > /* Save the thread's msr before get_tm_stackpointer() changes it */
> > unsigned long msr = regs->msr;
> > #endif
> > -
> > frame = get_sigframe(ksig, tsk, sizeof(*frame), 0);
> > - if (!access_ok(frame, sizeof(*frame)))
> > + if (!user_write_access_begin(frame, sizeof(*frame)))
> > goto badframe;
> >
> > - err |= __put_user(&frame->info, &frame->pinfo);
> > - err |= __put_user(&frame->uc, &frame->puc);
> > - err |= copy_siginfo_to_user(&frame->info, &ksig->info);
> > - if (err)
> > - goto badframe;
> > + unsafe_put_user(&frame->info, &frame->pinfo, badframe_block);
> > + unsafe_put_user(&frame->uc, &frame->puc, badframe_block);
> >
> > /* Create the ucontext. */
> > - err |= __put_user(0, &frame->uc.uc_flags);
> > - err |= __save_altstack(&frame->uc.uc_stack, regs->gpr[1]);
> > + unsafe_put_user(0, &frame->uc.uc_flags, badframe_block);
> > + unsafe_save_altstack(&frame->uc.uc_stack, regs->gpr[1], badframe_block);
> > +
> > #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> > if (MSR_TM_ACTIVE(msr)) {
> > /* The ucontext_t passed to userland points to the second
> > * ucontext_t (for transactional state) with its uc_link ptr.
> > */
> > - err |= __put_user(&frame->uc_transact, &frame->uc.uc_link);
> > + unsafe_put_user(&frame->uc_transact, &frame->uc.uc_link, badframe_block);
> > + user_write_access_end();
>
> Whaou. Doing this inside an #ifdef sequence is dirty.
> Can you reorganise code to avoid that and to avoid nesting #ifdef/#endif
> and the if/else as I did in
> signal32 ?
Hopefully yes - next spin!
>
> > err |= setup_tm_sigcontexts(&frame->uc.uc_mcontext,
> > &frame->uc_transact.uc_mcontext,
> > tsk, ksig->sig, NULL,
> > (unsigned long)ksig->ka.sa.sa_handler,
> > msr);
> > + if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
> > + goto badframe;
> > +
> > } else
> > #endif
> > {
> > - err |= __put_user(0, &frame->uc.uc_link);
> > -
> > - if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
> > - return -EFAULT;
> > - err |= __unsafe_setup_sigcontext(&frame->uc.uc_mcontext, tsk,
> > - ksig->sig, NULL,
> > - (unsigned long)ksig->ka.sa.sa_handler, 1);
> > - user_write_access_end();
> > + unsafe_put_user(0, &frame->uc.uc_link, badframe_block);
> > + unsafe_setup_sigcontext(&frame->uc.uc_mcontext, tsk, ksig->sig,
> > + NULL, (unsigned long)ksig->ka.sa.sa_handler,
> > + 1, badframe_block);
> > }
> > - err |= __copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set));
> > - if (err)
> > - goto badframe;
> > +
> > + unsafe_copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set), badframe_block);
> >
> > /* Make sure signal handler doesn't get spurious FP exceptions */
> > tsk->thread.fp_state.fpscr = 0;
> > @@ -891,15 +887,17 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
> > if (vdso64_rt_sigtramp && tsk->mm->context.vdso_base) {
> > regs->nip = tsk->mm->context.vdso_base + vdso64_rt_sigtramp;
> > } else {
> > - if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
> > - return -EFAULT;
> > - err |= __unsafe_setup_trampoline(__NR_rt_sigreturn, &frame->tramp[0]);
> > - user_write_access_end();
> > - if (err)
> > - goto badframe;
> > + unsafe_setup_trampoline(__NR_rt_sigreturn, &frame->tramp[0],
> > + badframe_block);
> > regs->nip = (unsigned long) &frame->tramp[0];
> > }
> >
> > + user_write_access_end();
> > +
> > + /* Save the siginfo outside of the unsafe block. */
> > + if (copy_siginfo_to_user(&frame->info, &ksig->info))
> > + goto badframe;
> > +
> > /* Allocate a dummy caller frame for the signal handler. */
> > newsp = ((unsigned long)frame) - __SIGNAL_FRAMESIZE;
> > err |= put_user(regs->gpr[1], (unsigned long __user *)newsp);
> > @@ -939,6 +937,8 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
> >
> > return 0;
> >
> > +badframe_block:
> > + user_write_access_end();
> > badframe:
> > signal_fault(current, regs, "handle_rt_signal64", frame);
> >
> >
>
>
> Christophe
^ permalink raw reply
* Re: [PATCH 8/8] powerpc/signal64: Rewrite rt_sigreturn() to minimise uaccess switches
From: Christopher M. Riedl @ 2020-10-20 2:45 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <bdc30b81-65bd-1cc0-b31a-61300e8c010a@csgroup.eu>
On Fri Oct 16, 2020 at 11:07 AM CDT, Christophe Leroy wrote:
>
>
> Le 15/10/2020 à 17:01, Christopher M. Riedl a écrit :
> > From: Daniel Axtens <dja@axtens.net>
> >
> > Add uaccess blocks and use the 'unsafe' versions of functions doing user
> > access where possible to reduce the number of times uaccess has to be
> > opened/closed.
> >
> > Signed-off-by: Daniel Axtens <dja@axtens.net>
> > Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
> > ---
> > arch/powerpc/kernel/signal_64.c | 23 +++++++++++++++--------
> > 1 file changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> > index 3b97e3681a8f..0f4ff7a5bfc1 100644
> > --- a/arch/powerpc/kernel/signal_64.c
> > +++ b/arch/powerpc/kernel/signal_64.c
> > @@ -779,18 +779,22 @@ SYSCALL_DEFINE0(rt_sigreturn)
> > */
> > regs->msr &= ~MSR_TS_MASK;
> >
> > - if (__get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR]))
> > + if (!user_read_access_begin(uc, sizeof(*uc)))
> > goto badframe;
> > +
> > + unsafe_get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR], badframe_block);
> > +
> > if (MSR_TM_ACTIVE(msr)) {
> > /* We recheckpoint on return. */
> > struct ucontext __user *uc_transact;
> >
> > /* Trying to start TM on non TM system */
> > if (!cpu_has_feature(CPU_FTR_TM))
> > - goto badframe;
> > + goto badframe_block;
> > +
> > + unsafe_get_user(uc_transact, &uc->uc_link, badframe_block);
> > + user_read_access_end();
>
> user_access_end() only in the if branch ?
>
> >
> > - if (__get_user(uc_transact, &uc->uc_link))
> > - goto badframe;
> > if (restore_tm_sigcontexts(current, &uc->uc_mcontext,
> > &uc_transact->uc_mcontext))
> > goto badframe;
> > @@ -810,12 +814,13 @@ SYSCALL_DEFINE0(rt_sigreturn)
> > * causing a TM bad thing.
> > */
> > current->thread.regs->msr &= ~MSR_TS_MASK;
> > +
> > +#ifndef CONFIG_PPC_TRANSACTIONAL_MEM
> > if (!user_read_access_begin(uc, sizeof(*uc)))
>
> The matching user_read_access_end() is not in the same #ifndef ? That's
> dirty and hard to follow.
> Can you re-organise the code to avoid all those nesting ?
Yes, thanks for pointing this out. I really wanted to avoid changing too
much of the logic inside these functions. But I suppose I ended up
creating a mess - I will fix this in the next spin.
>
> > - return -EFAULT;
> > - if (__unsafe_restore_sigcontext(current, NULL, 1, &uc->uc_mcontext)) {
> > - user_read_access_end();
> > goto badframe;
> > - }
> > +#endif
> > + unsafe_restore_sigcontext(current, NULL, 1, &uc->uc_mcontext,
> > + badframe_block);
> > user_read_access_end();
> > }
> >
> > @@ -825,6 +830,8 @@ SYSCALL_DEFINE0(rt_sigreturn)
> > set_thread_flag(TIF_RESTOREALL);
> > return 0;
> >
> > +badframe_block:
> > + user_read_access_end();
> > badframe:
> > signal_fault(current, regs, "rt_sigreturn", uc);
> >
> >
>
> Christophe
^ permalink raw reply
* Re: [PATCH 1/8] powerpc/uaccess: Add unsafe_copy_from_user
From: Christopher M. Riedl @ 2020-10-20 3:00 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <5dc712e9-4d08-b436-3137-d1ac100f2bfb@csgroup.eu>
On Fri Oct 16, 2020 at 10:17 AM CDT, Christophe Leroy wrote:
>
>
> Le 15/10/2020 à 17:01, Christopher M. Riedl a écrit :
> > Implement raw_copy_from_user_allowed() which assumes that userspace read
> > access is open. Use this new function to implement raw_copy_from_user().
> > Finally, wrap the new function to follow the usual "unsafe_" convention
> > of taking a label argument. The new raw_copy_from_user_allowed() calls
> > __copy_tofrom_user() internally, but this is still safe to call in user
> > access blocks formed with user_*_access_begin()/user_*_access_end()
> > since asm functions are not instrumented for tracing.
>
> Would objtool accept that if it was implemented on powerpc ?
>
> __copy_tofrom_user() is a function which is optimised for larger memory
> copies (using dcbz, etc ...)
> Do we need such an optimisation for unsafe_copy_from_user() ? Or can we
> do a simple loop as done for
> unsafe_copy_to_user() instead ?
I tried using a simple loop based on your unsafe_copy_to_user()
implementation. Similar to the copy_{vsx,fpr}_from_user() results there
is a hit to signal handling performance. The results with the loop are
in the 'unsafe-signal64-copy' column:
| | hash | radix |
| -------------------- | ------ | ------ |
| linuxppc/next | 289014 | 158408 |
| unsafe-signal64 | 298506 | 253053 |
| unsafe-signal64-copy | 197029 | 177002 |
Similar to the copy_{vsx,fpr}_from_user() patch I don't fully understand
why this performs so badly yet.
Implementation:
unsafe_copy_from_user(d, s, l, e) \
do { \
u8 *_dst = (u8 *)(d); \
const u8 __user *_src = (u8 __user*)(s); \
size_t _len = (l); \
int _i; \
\
for (_i = 0; _i < (_len & ~(sizeof(long) - 1)); _i += sizeof(long)) \
unsafe_get_user(*(long*)(_dst + _i), (long __user *)(_src + _i), e); \
if (IS_ENABLED(CONFIG_PPC64) && (_len & 4)) { \
unsafe_get_user(*(u32*)(_dst + _i), (u32 __user *)(_src + _i), e); \
_i += 4; \
} \
if (_len & 2) { \
unsafe_get_user(*(u16*)(_dst + _i), (u16 __user *)(_src + _i), e); \
_i += 2; \
} \
if (_len & 1) \
unsafe_get_user(*(u8*)(_dst + _i), (u8 __user *)(_src + _i), e); \
} while (0)
>
> Christophe
>
> >
> > Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
> > ---
> > arch/powerpc/include/asm/uaccess.h | 28 +++++++++++++++++++---------
> > 1 file changed, 19 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> > index 26781b044932..66940b4eb692 100644
> > --- a/arch/powerpc/include/asm/uaccess.h
> > +++ b/arch/powerpc/include/asm/uaccess.h
> > @@ -418,38 +418,45 @@ raw_copy_in_user(void __user *to, const void __user *from, unsigned long n)
> > }
> > #endif /* __powerpc64__ */
> >
> > -static inline unsigned long raw_copy_from_user(void *to,
> > - const void __user *from, unsigned long n)
> > +static inline unsigned long
> > +raw_copy_from_user_allowed(void *to, const void __user *from, unsigned long n)
> > {
> > - unsigned long ret;
> > if (__builtin_constant_p(n) && (n <= 8)) {
> > - ret = 1;
> > + unsigned long ret = 1;
> >
> > switch (n) {
> > case 1:
> > barrier_nospec();
> > - __get_user_size(*(u8 *)to, from, 1, ret);
> > + __get_user_size_allowed(*(u8 *)to, from, 1, ret);
> > break;
> > case 2:
> > barrier_nospec();
> > - __get_user_size(*(u16 *)to, from, 2, ret);
> > + __get_user_size_allowed(*(u16 *)to, from, 2, ret);
> > break;
> > case 4:
> > barrier_nospec();
> > - __get_user_size(*(u32 *)to, from, 4, ret);
> > + __get_user_size_allowed(*(u32 *)to, from, 4, ret);
> > break;
> > case 8:
> > barrier_nospec();
> > - __get_user_size(*(u64 *)to, from, 8, ret);
> > + __get_user_size_allowed(*(u64 *)to, from, 8, ret);
> > break;
> > }
> > if (ret == 0)
> > return 0;
> > }
> >
> > + return __copy_tofrom_user((__force void __user *)to, from, n);
> > +}
> > +
> > +static inline unsigned long
> > +raw_copy_from_user(void *to, const void __user *from, unsigned long n)
> > +{
> > + unsigned long ret;
> > +
> > barrier_nospec();
> > allow_read_from_user(from, n);
> > - ret = __copy_tofrom_user((__force void __user *)to, from, n);
> > + ret = raw_copy_from_user_allowed(to, from, n);
> > prevent_read_from_user(from, n);
> > return ret;
> > }
> > @@ -571,6 +578,9 @@ user_write_access_begin(const void __user *ptr, size_t len)
> > #define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
> > #define unsafe_put_user(x, p, e) __put_user_goto(x, p, e)
> >
> > +#define unsafe_copy_from_user(d, s, l, e) \
> > + unsafe_op_wrap(raw_copy_from_user_allowed(d, s, l), e)
> > +
> > #define unsafe_copy_to_user(d, s, l, e) \
> > do { \
> > u8 __user *_dst = (u8 __user *)(d); \
> >
^ permalink raw reply
* Re: [PATCH 6/8] powerpc/signal64: Replace setup_trampoline() w/ unsafe_setup_trampoline()
From: Christophe Leroy @ 2020-10-20 5:02 UTC (permalink / raw)
To: Christopher M. Riedl, linuxppc-dev; +Cc: Daniel Axtens
In-Reply-To: <C6HDEHRYN1HT.16V34IHMSGUK8@geist>
Le 20/10/2020 à 04:42, Christopher M. Riedl a écrit :
> On Fri Oct 16, 2020 at 10:56 AM CDT, Christophe Leroy wrote:
>>
>>
>> Le 15/10/2020 à 17:01, Christopher M. Riedl a écrit :
>>> From: Daniel Axtens <dja@axtens.net>
>>>
>>> Previously setup_trampoline() performed a costly KUAP switch on every
>>> uaccess operation. These repeated uaccess switches cause a significant
>>> drop in signal handling performance.
>>>
>>> Rewrite setup_trampoline() to assume that a userspace write access
>>> window is open. Replace all uaccess functions with their 'unsafe'
>>> versions to avoid the repeated uaccess switches.
>>>
>>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>>> Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
>>> ---
>>> arch/powerpc/kernel/signal_64.c | 32 +++++++++++++++++++-------------
>>> 1 file changed, 19 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
>>> index bd92064e5576..6d4f7a5c4fbf 100644
>>> --- a/arch/powerpc/kernel/signal_64.c
>>> +++ b/arch/powerpc/kernel/signal_64.c
>>> @@ -600,30 +600,33 @@ static long restore_tm_sigcontexts(struct task_struct *tsk,
>>> /*
>>> * Setup the trampoline code on the stack
>>> */
>>> -static long setup_trampoline(unsigned int syscall, unsigned int __user *tramp)
>>> +#define unsafe_setup_trampoline(syscall, tramp, e) \
>>> + unsafe_op_wrap(__unsafe_setup_trampoline(syscall, tramp), e)
>>> +static long notrace __unsafe_setup_trampoline(unsigned int syscall,
>>> + unsigned int __user *tramp)
>>> {
>>> int i;
>>> - long err = 0;
>>>
>>> /* bctrl # call the handler */
>>> - err |= __put_user(PPC_INST_BCTRL, &tramp[0]);
>>> + unsafe_put_user(PPC_INST_BCTRL, &tramp[0], err);
>>> /* addi r1, r1, __SIGNAL_FRAMESIZE # Pop the dummy stackframe */
>>> - err |= __put_user(PPC_INST_ADDI | __PPC_RT(R1) | __PPC_RA(R1) |
>>> - (__SIGNAL_FRAMESIZE & 0xffff), &tramp[1]);
>>> + unsafe_put_user(PPC_INST_ADDI | __PPC_RT(R1) | __PPC_RA(R1) |
>>> + (__SIGNAL_FRAMESIZE & 0xffff), &tramp[1], err);
>>> /* li r0, __NR_[rt_]sigreturn| */
>>> - err |= __put_user(PPC_INST_ADDI | (syscall & 0xffff), &tramp[2]);
>>> + unsafe_put_user(PPC_INST_ADDI | (syscall & 0xffff), &tramp[2], err);
>>> /* sc */
>>> - err |= __put_user(PPC_INST_SC, &tramp[3]);
>>> + unsafe_put_user(PPC_INST_SC, &tramp[3], err);
>>>
>>> /* Minimal traceback info */
>>> for (i=TRAMP_TRACEBACK; i < TRAMP_SIZE ;i++)
>>> - err |= __put_user(0, &tramp[i]);
>>> + unsafe_put_user(0, &tramp[i], err);
>>>
>>> - if (!err)
>>> - flush_icache_range((unsigned long) &tramp[0],
>>> - (unsigned long) &tramp[TRAMP_SIZE]);
>>> + flush_icache_range((unsigned long)&tramp[0],
>>> + (unsigned long)&tramp[TRAMP_SIZE]);
>>
>> This flush should be done outside the user_write_access block.
>>
>
> Hmm, I suppose that means setup_trampoline() cannot be completely
> "unsafe". I'll see if I can re-arrange the code which calls this
> function to avoid an additional uaccess block instead and push the
> start()/end() into setup_trampoline() directly.
I think we shouldn't put too much effort on setup_trampoline(). Nowadays 99.999% of applications use
the VDSO. Using the trampoline on stack requires to unmap the VDSO and remap the STACK RW. That's
really a corner case, I think it would be good enough to call it outside the main access begin/end
block, and let it do its own access_begin/end.
This corner functionnality can be tested using the sigreturn_vdso selftest in selftests/powerpc/signal/
Christophe
>
>>>
>>> - return err;
>>> + return 0;
>>> +err:
>>> + return 1;
>>> }
>>>
>>> /*
>>> @@ -888,7 +891,10 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
>>> if (vdso64_rt_sigtramp && tsk->mm->context.vdso_base) {
>>> regs->nip = tsk->mm->context.vdso_base + vdso64_rt_sigtramp;
>>> } else {
>>> - err |= setup_trampoline(__NR_rt_sigreturn, &frame->tramp[0]);
>>> + if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
>>> + return -EFAULT;
>>> + err |= __unsafe_setup_trampoline(__NR_rt_sigreturn, &frame->tramp[0]);
>>> + user_write_access_end();
>>> if (err)
>>> goto badframe;
>>> regs->nip = (unsigned long) &frame->tramp[0];
>>>
>>
>> Christophe
>
^ permalink raw reply
* [powerpc:fixes] BUILD SUCCESS 358ab796ce78ba271a6ff82834183ffb2cb68c4c
From: kernel test robot @ 2020-10-20 5:13 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes
branch HEAD: 358ab796ce78ba271a6ff82834183ffb2cb68c4c powerpc/powernv/dump: Handle multiple writes to ack attribute
elapsed time: 993m
configs tested: 223
configs skipped: 3
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
xtensa xip_kc705_defconfig
mips ar7_defconfig
arm mmp2_defconfig
powerpc ppc64e_defconfig
mips nlm_xlr_defconfig
nds32 alldefconfig
c6x evmc6457_defconfig
parisc generic-32bit_defconfig
arm am200epdkit_defconfig
powerpc redwood_defconfig
arc haps_hs_smp_defconfig
mips malta_kvm_defconfig
powerpc storcenter_defconfig
sh sh7710voipgw_defconfig
mips ip27_defconfig
s390 allyesconfig
arm rpc_defconfig
sh apsh4ad0a_defconfig
arc tb10x_defconfig
arm keystone_defconfig
mips malta_qemu_32r6_defconfig
m68k allmodconfig
mips decstation_64_defconfig
powerpc mpc837x_mds_defconfig
riscv nommu_virt_defconfig
m68k m5475evb_defconfig
powerpc g5_defconfig
csky alldefconfig
powerpc eiger_defconfig
powerpc tqm8540_defconfig
arm neponset_defconfig
powerpc mgcoge_defconfig
arm spitz_defconfig
xtensa audio_kc705_defconfig
sh sh7785lcr_32bit_defconfig
powerpc kmeter1_defconfig
nios2 defconfig
arm pxa_defconfig
arc vdk_hs38_smp_defconfig
arm pxa255-idp_defconfig
m68k bvme6000_defconfig
arm magician_defconfig
powerpc mpc5200_defconfig
arm pxa3xx_defconfig
sh landisk_defconfig
openrisc or1ksim_defconfig
arm nhk8815_defconfig
powerpc mpc834x_itx_defconfig
arm palmz72_defconfig
powerpc acadia_defconfig
arm collie_defconfig
nios2 3c120_defconfig
arm sama5_defconfig
sh migor_defconfig
powerpc makalu_defconfig
powerpc bluestone_defconfig
arm corgi_defconfig
powerpc ep88xc_defconfig
sh sh03_defconfig
powerpc tqm8xx_defconfig
sh alldefconfig
m68k sun3x_defconfig
powerpc holly_defconfig
mips lemote2f_defconfig
mips ip28_defconfig
sh kfr2r09-romimage_defconfig
arm lpd270_defconfig
sh se7751_defconfig
arm ep93xx_defconfig
arm integrator_defconfig
sh ap325rxa_defconfig
arm moxart_defconfig
powerpc pasemi_defconfig
um i386_defconfig
mips e55_defconfig
powerpc mpc832x_rdb_defconfig
mips cu1000-neo_defconfig
mips bcm47xx_defconfig
mips ip32_defconfig
sh se7780_defconfig
xtensa generic_kc705_defconfig
mips jazz_defconfig
arm multi_v5_defconfig
powerpc cell_defconfig
arm alldefconfig
mips tb0287_defconfig
arm hisi_defconfig
arm badge4_defconfig
h8300 alldefconfig
alpha alldefconfig
arm pcm027_defconfig
sh shmin_defconfig
arm sunxi_defconfig
sparc defconfig
arm realview_defconfig
mips tb0226_defconfig
sh shx3_defconfig
arm efm32_defconfig
powerpc amigaone_defconfig
powerpc powernv_defconfig
sh j2_defconfig
mips cavium_octeon_defconfig
arm omap2plus_defconfig
sh ecovec24-romimage_defconfig
arm cns3420vb_defconfig
arm s5pv210_defconfig
powerpc ps3_defconfig
sh rsk7269_defconfig
arm mvebu_v7_defconfig
m68k mvme147_defconfig
arc allyesconfig
sh se7724_defconfig
powerpc ppc40x_defconfig
powerpc mpc85xx_cds_defconfig
sh se7750_defconfig
sh kfr2r09_defconfig
powerpc mpc512x_defconfig
powerpc socrates_defconfig
c6x evmc6678_defconfig
powerpc skiroot_defconfig
arm zeus_defconfig
powerpc katmai_defconfig
mips maltaup_xpa_defconfig
arm tegra_defconfig
powerpc mpc866_ads_defconfig
mips nlm_xlp_defconfig
sh rsk7264_defconfig
sh magicpanelr2_defconfig
powerpc mpc8272_ads_defconfig
sh sdk7786_defconfig
arm oxnas_v6_defconfig
arc nsimosci_hs_defconfig
arm vt8500_v6_v7_defconfig
mips workpad_defconfig
mips loongson1b_defconfig
h8300 edosk2674_defconfig
powerpc currituck_defconfig
powerpc iss476-smp_defconfig
arm spear3xx_defconfig
arm colibri_pxa300_defconfig
powerpc ge_imp3a_defconfig
mips allmodconfig
mips mpc30x_defconfig
alpha defconfig
mips tb0219_defconfig
sh urquell_defconfig
nios2 10m50_defconfig
sparc64 defconfig
arm omap1_defconfig
mips vocore2_defconfig
sh edosk7705_defconfig
powerpc akebono_defconfig
mips bigsur_defconfig
sh dreamcast_defconfig
m68k m5407c3_defconfig
sh se7343_defconfig
arc nsimosci_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k defconfig
m68k allyesconfig
nds32 allnoconfig
c6x allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
i386 defconfig
mips allyesconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a004-20201019
x86_64 randconfig-a002-20201019
x86_64 randconfig-a006-20201019
x86_64 randconfig-a003-20201019
x86_64 randconfig-a005-20201019
x86_64 randconfig-a001-20201019
i386 randconfig-a006-20201019
i386 randconfig-a005-20201019
i386 randconfig-a001-20201019
i386 randconfig-a003-20201019
i386 randconfig-a004-20201019
i386 randconfig-a002-20201019
i386 randconfig-a015-20201019
i386 randconfig-a013-20201019
i386 randconfig-a016-20201019
i386 randconfig-a012-20201019
i386 randconfig-a011-20201019
i386 randconfig-a014-20201019
riscv nommu_k210_defconfig
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-a016-20201019
x86_64 randconfig-a015-20201019
x86_64 randconfig-a012-20201019
x86_64 randconfig-a013-20201019
x86_64 randconfig-a011-20201019
x86_64 randconfig-a014-20201019
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [powerpc:next-test] BUILD SUCCESS c9818c0abfb0c3500684bb2bc75981123d63134d
From: kernel test robot @ 2020-10-20 5:19 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test
branch HEAD: c9818c0abfb0c3500684bb2bc75981123d63134d powerpc/64s: Convert some cpu_setup() and cpu_restore() functions to C
elapsed time: 997m
configs tested: 209
configs skipped: 3
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
xtensa xip_kc705_defconfig
mips ar7_defconfig
arm mmp2_defconfig
powerpc ppc64e_defconfig
mips nlm_xlr_defconfig
nds32 alldefconfig
c6x evmc6457_defconfig
powerpc redwood_defconfig
parisc generic-32bit_defconfig
arm am200epdkit_defconfig
mips ip27_defconfig
s390 allyesconfig
arm rpc_defconfig
sh apsh4ad0a_defconfig
arc tb10x_defconfig
arm keystone_defconfig
mips malta_qemu_32r6_defconfig
m68k allmodconfig
mips decstation_64_defconfig
powerpc mpc837x_mds_defconfig
riscv nommu_virt_defconfig
m68k m5475evb_defconfig
powerpc g5_defconfig
csky alldefconfig
powerpc eiger_defconfig
powerpc tqm8540_defconfig
arm neponset_defconfig
powerpc mgcoge_defconfig
arm spitz_defconfig
xtensa audio_kc705_defconfig
sh sh7785lcr_32bit_defconfig
powerpc kmeter1_defconfig
nios2 defconfig
powerpc mpc5200_defconfig
arm pxa3xx_defconfig
sh landisk_defconfig
openrisc or1ksim_defconfig
arm nhk8815_defconfig
powerpc mpc834x_itx_defconfig
arm palmz72_defconfig
powerpc acadia_defconfig
arm collie_defconfig
nios2 3c120_defconfig
mips malta_kvm_defconfig
arm sama5_defconfig
sh migor_defconfig
powerpc makalu_defconfig
powerpc bluestone_defconfig
arm corgi_defconfig
powerpc ep88xc_defconfig
sh sh03_defconfig
sh sh7710voipgw_defconfig
powerpc tqm8xx_defconfig
sh alldefconfig
m68k sun3x_defconfig
powerpc holly_defconfig
mips lemote2f_defconfig
mips ip28_defconfig
sh kfr2r09-romimage_defconfig
arm lpd270_defconfig
sh se7751_defconfig
arm ep93xx_defconfig
arm integrator_defconfig
sh ap325rxa_defconfig
arm moxart_defconfig
powerpc pasemi_defconfig
um i386_defconfig
mips e55_defconfig
powerpc mpc832x_rdb_defconfig
mips cu1000-neo_defconfig
mips bcm47xx_defconfig
mips ip32_defconfig
sh se7780_defconfig
mips tb0287_defconfig
arm hisi_defconfig
arm badge4_defconfig
h8300 alldefconfig
alpha alldefconfig
arm pcm027_defconfig
sh shmin_defconfig
arm sunxi_defconfig
sparc defconfig
arm realview_defconfig
mips tb0226_defconfig
sh shx3_defconfig
arm efm32_defconfig
powerpc amigaone_defconfig
powerpc powernv_defconfig
sh j2_defconfig
mips cavium_octeon_defconfig
arm omap2plus_defconfig
sh ecovec24-romimage_defconfig
arm cns3420vb_defconfig
m68k bvme6000_defconfig
arm s5pv210_defconfig
powerpc ps3_defconfig
sh rsk7269_defconfig
arm mvebu_v7_defconfig
m68k mvme147_defconfig
arc allyesconfig
sh se7724_defconfig
powerpc ppc40x_defconfig
powerpc mpc85xx_cds_defconfig
sh se7750_defconfig
sh kfr2r09_defconfig
powerpc mpc512x_defconfig
powerpc socrates_defconfig
c6x evmc6678_defconfig
powerpc skiroot_defconfig
arm zeus_defconfig
powerpc katmai_defconfig
mips maltaup_xpa_defconfig
arm tegra_defconfig
powerpc mpc866_ads_defconfig
mips nlm_xlp_defconfig
sh rsk7264_defconfig
sh magicpanelr2_defconfig
powerpc mpc8272_ads_defconfig
sh sdk7786_defconfig
arm oxnas_v6_defconfig
arc nsimosci_hs_defconfig
arm vt8500_v6_v7_defconfig
mips workpad_defconfig
mips loongson1b_defconfig
h8300 edosk2674_defconfig
powerpc currituck_defconfig
powerpc iss476-smp_defconfig
arm spear3xx_defconfig
arm colibri_pxa300_defconfig
powerpc ge_imp3a_defconfig
mips allmodconfig
mips mpc30x_defconfig
alpha defconfig
mips tb0219_defconfig
sh urquell_defconfig
nios2 10m50_defconfig
sparc64 defconfig
arm omap1_defconfig
mips vocore2_defconfig
sh edosk7705_defconfig
m68k m5407c3_defconfig
sh se7343_defconfig
arc nsimosci_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k defconfig
m68k allyesconfig
nds32 allnoconfig
c6x allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
i386 defconfig
mips allyesconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a004-20201019
x86_64 randconfig-a002-20201019
x86_64 randconfig-a006-20201019
x86_64 randconfig-a003-20201019
x86_64 randconfig-a005-20201019
x86_64 randconfig-a001-20201019
i386 randconfig-a006-20201019
i386 randconfig-a005-20201019
i386 randconfig-a001-20201019
i386 randconfig-a003-20201019
i386 randconfig-a004-20201019
i386 randconfig-a002-20201019
i386 randconfig-a015-20201019
i386 randconfig-a013-20201019
i386 randconfig-a016-20201019
i386 randconfig-a012-20201019
i386 randconfig-a011-20201019
i386 randconfig-a014-20201019
riscv nommu_k210_defconfig
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-a016-20201019
x86_64 randconfig-a015-20201019
x86_64 randconfig-a012-20201019
x86_64 randconfig-a013-20201019
x86_64 randconfig-a011-20201019
x86_64 randconfig-a014-20201019
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [powerpc:merge] BUILD SUCCESS 96b5a60d059984a2f0eaef90e97f59ac4a76bff4
From: kernel test robot @ 2020-10-20 5:19 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge
branch HEAD: 96b5a60d059984a2f0eaef90e97f59ac4a76bff4 Automatic merge of 'fixes' into merge (2020-10-19 23:20)
elapsed time: 998m
configs tested: 176
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
xtensa xip_kc705_defconfig
mips ar7_defconfig
arm mmp2_defconfig
powerpc ppc64e_defconfig
mips nlm_xlr_defconfig
mips ip27_defconfig
s390 allyesconfig
arm rpc_defconfig
sh apsh4ad0a_defconfig
arc tb10x_defconfig
arm keystone_defconfig
arc axs103_smp_defconfig
powerpc socrates_defconfig
riscv alldefconfig
powerpc mpc7448_hpc2_defconfig
arm ixp4xx_defconfig
microblaze mmu_defconfig
powerpc mgcoge_defconfig
arm spitz_defconfig
xtensa audio_kc705_defconfig
sh sh7785lcr_32bit_defconfig
powerpc kmeter1_defconfig
powerpc mpc5200_defconfig
arm pxa3xx_defconfig
sh landisk_defconfig
openrisc or1ksim_defconfig
arm nhk8815_defconfig
powerpc mpc834x_itx_defconfig
arm am200epdkit_defconfig
arm palmz72_defconfig
powerpc acadia_defconfig
arm collie_defconfig
nios2 3c120_defconfig
mips malta_kvm_defconfig
arm sama5_defconfig
sh migor_defconfig
powerpc makalu_defconfig
powerpc bluestone_defconfig
arm corgi_defconfig
powerpc ep88xc_defconfig
sh sh03_defconfig
parisc generic-32bit_defconfig
sh sh7710voipgw_defconfig
powerpc tqm8xx_defconfig
sh alldefconfig
m68k sun3x_defconfig
powerpc holly_defconfig
mips lemote2f_defconfig
sh se7751_defconfig
arm ep93xx_defconfig
arm integrator_defconfig
c6x evmc6457_defconfig
powerpc mpc832x_rdb_defconfig
mips cu1000-neo_defconfig
mips bcm47xx_defconfig
mips ip32_defconfig
sh se7780_defconfig
mips tb0287_defconfig
arm hisi_defconfig
arm badge4_defconfig
h8300 alldefconfig
arm spear6xx_defconfig
arm tango4_defconfig
arm efm32_defconfig
powerpc amigaone_defconfig
powerpc powernv_defconfig
sh j2_defconfig
mips tb0226_defconfig
mips cavium_octeon_defconfig
arm omap2plus_defconfig
sh ecovec24-romimage_defconfig
arm cns3420vb_defconfig
m68k bvme6000_defconfig
arm s5pv210_defconfig
powerpc ps3_defconfig
arm mvebu_v7_defconfig
m68k mvme147_defconfig
sh rsk7269_defconfig
sh se7724_defconfig
sh shmin_defconfig
powerpc ppc40x_defconfig
powerpc mpc85xx_cds_defconfig
sh se7750_defconfig
alpha alldefconfig
sh kfr2r09_defconfig
powerpc mpc512x_defconfig
arm zeus_defconfig
powerpc katmai_defconfig
mips maltaup_xpa_defconfig
arm tegra_defconfig
sh rsk7264_defconfig
powerpc skiroot_defconfig
arm lpd270_defconfig
sh magicpanelr2_defconfig
arm vt8500_v6_v7_defconfig
mips workpad_defconfig
mips loongson1b_defconfig
powerpc currituck_defconfig
powerpc iss476-smp_defconfig
arm spear3xx_defconfig
m68k m5407c3_defconfig
sh se7343_defconfig
arc nsimosci_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
c6x allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a004-20201019
x86_64 randconfig-a002-20201019
x86_64 randconfig-a006-20201019
x86_64 randconfig-a003-20201019
x86_64 randconfig-a005-20201019
x86_64 randconfig-a001-20201019
i386 randconfig-a006-20201019
i386 randconfig-a005-20201019
i386 randconfig-a001-20201019
i386 randconfig-a003-20201019
i386 randconfig-a004-20201019
i386 randconfig-a002-20201019
i386 randconfig-a015-20201019
i386 randconfig-a013-20201019
i386 randconfig-a016-20201019
i386 randconfig-a012-20201019
i386 randconfig-a011-20201019
i386 randconfig-a014-20201019
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-a016-20201019
x86_64 randconfig-a015-20201019
x86_64 randconfig-a012-20201019
x86_64 randconfig-a013-20201019
x86_64 randconfig-a011-20201019
x86_64 randconfig-a014-20201019
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [PATCH 2/2] powerpc/watchpoint: Workaround P10 DD1 issue with VSX-32 byte instructions
From: Ravi Bangoria @ 2020-10-20 5:44 UTC (permalink / raw)
To: mpe
Cc: christophe.leroy, ravi.bangoria, mikey, jniethe5, npiggin, maddy,
paulus, naveen.n.rao, linuxppc-dev
In-Reply-To: <20201020054454.194343-1-ravi.bangoria@linux.ibm.com>
POWER10 DD1 has an issue where it generates watchpoint exceptions when it
shouldn't. The conditions where this occur are:
- octword op
- ending address of DAWR range is less than starting address of op
- those addresses need to be in the same or in two consecutive 512B
blocks
- 'op address + 64B' generates an address that has a carry into bit
52 (crosses 2K boundary)
Handle such spurious exception by considering them as extraneous and
emulating/single-steeping instruction without generating an event.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
Dependency: VSX-32 byte emulation support patches
https://lore.kernel.org/r/20201011050908.72173-1-ravi.bangoria@linux.ibm.com
arch/powerpc/kernel/hw_breakpoint.c | 69 ++++++++++++++++++++++++++++-
1 file changed, 67 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index f4e8f21046f5..4514745d27c3 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -499,6 +499,11 @@ static bool is_larx_stcx_instr(int type)
return type == LARX || type == STCX;
}
+static bool is_octword_vsx_instr(int type, int size)
+{
+ return ((type == LOAD_VSX || type == STORE_VSX) && size == 32);
+}
+
/*
* We've failed in reliably handling the hw-breakpoint. Unregister
* it and throw a warning message to let the user know about it.
@@ -549,6 +554,60 @@ static bool stepping_handler(struct pt_regs *regs, struct perf_event **bp,
return true;
}
+static void handle_p10dd1_spurious_exception(struct arch_hw_breakpoint **info,
+ int *hit, unsigned long ea)
+{
+ int i;
+ unsigned long hw_start_addr;
+ unsigned long hw_end_addr;
+
+ /*
+ * Handle spurious exception only when any bp_per_reg is set.
+ * Otherwise this might be created by xmon and not actually a
+ * spurious exception.
+ */
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!info[i])
+ continue;
+
+ hw_start_addr = ALIGN_DOWN(info[i]->address, HW_BREAKPOINT_SIZE);
+ hw_end_addr = ALIGN(info[i]->address + info[i]->len, HW_BREAKPOINT_SIZE);
+
+ /*
+ * Ending address of DAWR range is less than starting
+ * address of op.
+ */
+ if ((hw_end_addr - 1) >= ea)
+ continue;
+
+ /*
+ * Those addresses need to be in the same or in two
+ * consecutive 512B blocks;
+ */
+ if (((hw_end_addr - 1) >> 10) != (ea >> 10))
+ continue;
+
+ /*
+ * 'op address + 64B' generates an address that has a
+ * carry into bit 52 (crosses 2K boundary).
+ */
+ if ((ea & 0x800) == ((ea + 64) & 0x800))
+ continue;
+
+ break;
+ }
+
+ if (i == nr_wp_slots())
+ return;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (info[i]) {
+ hit[i] = 1;
+ info[i]->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
+ }
+ }
+}
+
int hw_breakpoint_handler(struct die_args *args)
{
bool err = false;
@@ -607,8 +666,14 @@ int hw_breakpoint_handler(struct die_args *args)
goto reset;
if (!nr_hit) {
- rc = NOTIFY_DONE;
- goto out;
+ if (cpu_has_feature(CPU_FTR_POWER10_DD1) &&
+ !IS_ENABLED(CONFIG_PPC_8xx) &&
+ is_octword_vsx_instr(type, size)) {
+ handle_p10dd1_spurious_exception(info, hit, ea);
+ } else {
+ rc = NOTIFY_DONE;
+ goto out;
+ }
}
/*
--
2.25.1
^ permalink raw reply related
* [PATCH 1/2] powerpc: Introduce POWER10_DD1 feature
From: Ravi Bangoria @ 2020-10-20 5:44 UTC (permalink / raw)
To: mpe
Cc: christophe.leroy, ravi.bangoria, mikey, jniethe5, npiggin, maddy,
paulus, naveen.n.rao, linuxppc-dev
POWER10_DD1 feature flag will be needed while adding
conditional code that applies only for Power10 DD1.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/cputable.h | 8 ++++++--
arch/powerpc/kernel/dt_cpu_ftrs.c | 3 +++
arch/powerpc/kernel/prom.c | 9 +++++++++
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 93bc70d4c9a1..d486f56c0d33 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -216,6 +216,7 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTR_P9_RADIX_PREFETCH_BUG LONG_ASM_CONST(0x0002000000000000)
#define CPU_FTR_ARCH_31 LONG_ASM_CONST(0x0004000000000000)
#define CPU_FTR_DAWR1 LONG_ASM_CONST(0x0008000000000000)
+#define CPU_FTR_POWER10_DD1 LONG_ASM_CONST(0x0010000000000000)
#ifndef __ASSEMBLY__
@@ -479,6 +480,7 @@ static inline void cpu_feature_keys_init(void) { }
CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | CPU_FTR_ARCH_31 | \
CPU_FTR_DAWR | CPU_FTR_DAWR1)
+#define CPU_FTRS_POWER10_DD1 (CPU_FTRS_POWER10 | CPU_FTR_POWER10_DD1)
#define CPU_FTRS_CELL (CPU_FTR_LWSYNC | \
CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
@@ -497,14 +499,16 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTRS_POSSIBLE \
(CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | CPU_FTRS_POWER8 | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_VSX_COMP | CPU_FTRS_POWER9 | \
- CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
+ CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 | \
+ CPU_FTRS_POWER10_DD1)
#else
#define CPU_FTRS_POSSIBLE \
(CPU_FTRS_PPC970 | CPU_FTRS_POWER5 | \
CPU_FTRS_POWER6 | CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | \
CPU_FTRS_POWER8 | CPU_FTRS_CELL | CPU_FTRS_PA6T | \
CPU_FTR_VSX_COMP | CPU_FTR_ALTIVEC_COMP | CPU_FTRS_POWER9 | \
- CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
+ CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 | \
+ CPU_FTRS_POWER10_DD1)
#endif /* CONFIG_CPU_LITTLE_ENDIAN */
#endif
#else
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 1098863e17ee..b2327f2967ff 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -811,6 +811,9 @@ static __init void cpufeatures_cpu_quirks(void)
}
update_tlbie_feature_flag(version);
+
+ if ((version & 0xffffffff) == 0x00800100)
+ cur_cpu_spec->cpu_features |= CPU_FTR_POWER10_DD1;
}
static void __init cpufeatures_setup_finished(void)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index c1545f22c077..c778c81284f7 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -305,6 +305,14 @@ static void __init check_cpu_feature_properties(unsigned long node)
}
}
+static void __init fixup_cpu_features(void)
+{
+ unsigned long version = mfspr(SPRN_PVR);
+
+ if ((version & 0xffffffff) == 0x00800100)
+ cur_cpu_spec->cpu_features |= CPU_FTR_POWER10_DD1;
+}
+
static int __init early_init_dt_scan_cpus(unsigned long node,
const char *uname, int depth,
void *data)
@@ -378,6 +386,7 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
check_cpu_feature_properties(node);
check_cpu_pa_features(node);
+ fixup_cpu_features();
}
identical_pvr_fixup(node);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 3/8] powerpc: Mark functions called inside uaccess blocks w/ 'notrace'
From: Michael Ellerman @ 2020-10-20 7:34 UTC (permalink / raw)
To: Peter Zijlstra, Christoph Hellwig
Cc: linuxppc-dev, linux-kernel, Christopher M. Riedl
In-Reply-To: <20201016094132.GI2611@hirez.programming.kicks-ass.net>
Peter Zijlstra <peterz@infradead.org> writes:
> On Fri, Oct 16, 2020 at 07:56:16AM +0100, Christoph Hellwig wrote:
>> On Thu, Oct 15, 2020 at 10:01:54AM -0500, Christopher M. Riedl wrote:
>> > Functions called between user_*_access_begin() and user_*_access_end()
>> > should be either inlined or marked 'notrace' to prevent leaving
>> > userspace access exposed. Mark any such functions relevant to signal
>> > handling so that subsequent patches can call them inside uaccess blocks.
>>
>> I don't think running this much code with uaccess enabled is a good
>> idea. Please refactor the code to reduce the criticial sections with
>> uaccess enabled.
>>
>> Btw, does powerpc already have the objtool validation that we don't
>> accidentally jump out of unsafe uaccess critical sections?
>
> It does not, there was some effort on that a while ago, but I suspect
> they're waiting for the ARM64 effort to land and build on that.
Right, we don't have objtool support.
We would definitely like objtool support at least for this uaccess
checking, I'm sure we have some escapes.
There was someone working on it in their own-time but last I heard that
was still WIP.
I didn't realise the ARM64 support was still not merged, so yeah having
that land first would probably simplify things, but we still need
someone who has time to work on it.
cheers
^ permalink raw reply
* [PATCH v2 1/3] powerpc/uaccess: Don't use "m<>" constraint with GCC 4.9
From: Christophe Leroy @ 2020-10-20 7:40 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
mathieu.desnoyers
Cc: linuxppc-dev, linux-kernel
GCC 4.9 sometimes fails to build with "m<>" constraint in
inline assembly.
CC lib/iov_iter.o
In file included from ./arch/powerpc/include/asm/cmpxchg.h:6:0,
from ./arch/powerpc/include/asm/atomic.h:11,
from ./include/linux/atomic.h:7,
from ./include/linux/crypto.h:15,
from ./include/crypto/hash.h:11,
from lib/iov_iter.c:2:
lib/iov_iter.c: In function 'iovec_from_user.part.30':
./arch/powerpc/include/asm/uaccess.h:287:2: error: 'asm' operand has impossible constraints
__asm__ __volatile__( \
^
./include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
^
./arch/powerpc/include/asm/uaccess.h:583:34: note: in expansion of macro 'unsafe_op_wrap'
#define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
^
./arch/powerpc/include/asm/uaccess.h:329:10: note: in expansion of macro '__get_user_asm'
case 4: __get_user_asm(x, (u32 __user *)ptr, retval, "lwz"); break; \
^
./arch/powerpc/include/asm/uaccess.h:363:3: note: in expansion of macro '__get_user_size_allowed'
__get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
^
./arch/powerpc/include/asm/uaccess.h:100:2: note: in expansion of macro '__get_user_nocheck'
__get_user_nocheck((x), (ptr), sizeof(*(ptr)), false)
^
./arch/powerpc/include/asm/uaccess.h:583:49: note: in expansion of macro '__get_user_allowed'
#define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
^
lib/iov_iter.c:1663:3: note: in expansion of macro 'unsafe_get_user'
unsafe_get_user(len, &uiov[i].iov_len, uaccess_end);
^
make[1]: *** [scripts/Makefile.build:283: lib/iov_iter.o] Error 1
Define a UPD_CONSTR macro that is "<>" by default and
only "" with GCC prior to GCC 5.
Fixes: fcf1f26895a4 ("powerpc/uaccess: Add pre-update addressing to __put_user_asm_goto()")
Fixes: 2f279eeb68b8 ("powerpc/uaccess: Add pre-update addressing to __get_user_asm() and __put_user_asm()")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Acked-by: Segher Boessenkool <segher@kernel.crashing.org>
---
v2: Moved UPD_CONSTR to asm-const.h to avoid circular inclusion issues with patch 3.
---
arch/powerpc/include/asm/asm-const.h | 13 +++++++++++++
arch/powerpc/include/asm/uaccess.h | 4 ++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/asm-const.h b/arch/powerpc/include/asm/asm-const.h
index 082c1538c562..0ce2368bd20f 100644
--- a/arch/powerpc/include/asm/asm-const.h
+++ b/arch/powerpc/include/asm/asm-const.h
@@ -11,4 +11,17 @@
# define __ASM_CONST(x) x##UL
# define ASM_CONST(x) __ASM_CONST(x)
#endif
+
+/*
+ * Inline assembly memory constraint
+ *
+ * GCC 4.9 doesn't properly handle pre update memory constraint "m<>"
+ *
+ */
+#if defined(GCC_VERSION) && GCC_VERSION < 50000
+#define UPD_CONSTR ""
+#else
+#define UPD_CONSTR "<>"
+#endif
+
#endif /* _ASM_POWERPC_ASM_CONST_H */
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 604d705f1bb8..8f27ea48fadb 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -223,7 +223,7 @@ do { \
"1: " op "%U1%X1 %0,%1 # put_user\n" \
EX_TABLE(1b, %l2) \
: \
- : "r" (x), "m<>" (*addr) \
+ : "r" (x), "m"UPD_CONSTR (*addr) \
: \
: label)
@@ -294,7 +294,7 @@ extern long __get_user_bad(void);
".previous\n" \
EX_TABLE(1b, 3b) \
: "=r" (err), "=r" (x) \
- : "m<>" (*addr), "i" (-EFAULT), "0" (err))
+ : "m"UPD_CONSTR (*addr), "i" (-EFAULT), "0" (err))
#ifdef __powerpc64__
#define __get_user_asm2(x, addr, err) \
--
2.25.0
^ permalink raw reply related
* [PATCH v2 2/3] powerpc: Fix incorrect stw{, ux, u, x} instructions in __set_pte_at
From: Christophe Leroy @ 2020-10-20 7:40 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
mathieu.desnoyers
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <212d3bc4a52ca71523759517bb9c61f7e477c46a.1603179582.git.christophe.leroy@csgroup.eu>
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
The placeholder for instruction selection should use the second
argument's operand, which is %1, not %0. This could generate incorrect
assembly code if the memory addressing of operand %0 is a different
form from that of operand %1.
Fixes: 9bf2b5cdc5fe ("powerpc: Fixes for CONFIG_PTE_64BIT for SMP support")
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Kumar Gala <galak@kernel.crashing.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
Cc: <stable@vger.kernel.org> # v2.6.28+
Acked-by: Segher Boessenkool <segher@kernel.crashing.org>
[chleroy: revised commit log iaw segher's comment]
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: Changed commit log.
---
arch/powerpc/include/asm/book3s/32/pgtable.h | 2 +-
arch/powerpc/include/asm/nohash/pgtable.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 36443cda8dcf..34f5ca391f0c 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -524,7 +524,7 @@ static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
__asm__ __volatile__("\
stw%U0%X0 %2,%0\n\
eieio\n\
- stw%U0%X0 %L2,%1"
+ stw%U1%X1 %L2,%1"
: "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
: "r" (pte) : "memory");
diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h
index 4b7c3472eab1..a00e4c1746d6 100644
--- a/arch/powerpc/include/asm/nohash/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/pgtable.h
@@ -199,7 +199,7 @@ static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
__asm__ __volatile__("\
stw%U0%X0 %2,%0\n\
eieio\n\
- stw%U0%X0 %L2,%1"
+ stw%U1%X1 %L2,%1"
: "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
: "r" (pte) : "memory");
return;
--
2.25.0
^ permalink raw reply related
* [PATCH v2 3/3] powerpc: Fix update form addressing in inline assembly
From: Christophe Leroy @ 2020-10-20 7:40 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
mathieu.desnoyers
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <212d3bc4a52ca71523759517bb9c61f7e477c46a.1603179582.git.christophe.leroy@csgroup.eu>
In several places, inline assembly uses the "%Un" modifier
to enable the use of instruction with update form addressing,
but the associated "<>" constraint is missing.
As mentioned in previous patch, this fails with gcc 4.9, so
"<>" can't be used directly.
Use UPD_CONSTR macro everywhere %Un modifier is used.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: Build failure (circular inclusion) fixed by change in patch 1
---
arch/powerpc/include/asm/atomic.h | 9 +++++----
arch/powerpc/include/asm/book3s/32/pgtable.h | 2 +-
arch/powerpc/include/asm/io.h | 4 ++--
arch/powerpc/include/asm/nohash/pgtable.h | 2 +-
arch/powerpc/kvm/powerpc.c | 4 ++--
5 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h
index 8a55eb8cc97b..61c6e8b200e8 100644
--- a/arch/powerpc/include/asm/atomic.h
+++ b/arch/powerpc/include/asm/atomic.h
@@ -10,6 +10,7 @@
#include <linux/types.h>
#include <asm/cmpxchg.h>
#include <asm/barrier.h>
+#include <asm/asm-const.h>
/*
* Since *_return_relaxed and {cmp}xchg_relaxed are implemented with
@@ -26,14 +27,14 @@ static __inline__ int atomic_read(const atomic_t *v)
{
int t;
- __asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter));
+ __asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m"UPD_CONSTR(v->counter));
return t;
}
static __inline__ void atomic_set(atomic_t *v, int i)
{
- __asm__ __volatile__("stw%U0%X0 %1,%0" : "=m"(v->counter) : "r"(i));
+ __asm__ __volatile__("stw%U0%X0 %1,%0" : "=m"UPD_CONSTR(v->counter) : "r"(i));
}
#define ATOMIC_OP(op, asm_op) \
@@ -316,14 +317,14 @@ static __inline__ s64 atomic64_read(const atomic64_t *v)
{
s64 t;
- __asm__ __volatile__("ld%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter));
+ __asm__ __volatile__("ld%U1%X1 %0,%1" : "=r"(t) : "m"UPD_CONSTR(v->counter));
return t;
}
static __inline__ void atomic64_set(atomic64_t *v, s64 i)
{
- __asm__ __volatile__("std%U0%X0 %1,%0" : "=m"(v->counter) : "r"(i));
+ __asm__ __volatile__("std%U0%X0 %1,%0" : "=m"UPD_CONSTR(v->counter) : "r"(i));
}
#define ATOMIC64_OP(op, asm_op) \
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 34f5ca391f0c..0e1b6e020cef 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -525,7 +525,7 @@ static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
stw%U0%X0 %2,%0\n\
eieio\n\
stw%U1%X1 %L2,%1"
- : "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
+ : "=m"UPD_CONSTR (*ptep), "=m"UPD_CONSTR (*((unsigned char *)ptep+4))
: "r" (pte) : "memory");
#else
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index 58635960403c..87964dfb838e 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -122,7 +122,7 @@ static inline u##size name(const volatile u##size __iomem *addr) \
{ \
u##size ret; \
__asm__ __volatile__("sync;"#insn"%U1%X1 %0,%1;twi 0,%0,0;isync"\
- : "=r" (ret) : "m" (*addr) : "memory"); \
+ : "=r" (ret) : "m"UPD_CONSTR (*addr) : "memory"); \
return ret; \
}
@@ -130,7 +130,7 @@ static inline u##size name(const volatile u##size __iomem *addr) \
static inline void name(volatile u##size __iomem *addr, u##size val) \
{ \
__asm__ __volatile__("sync;"#insn"%U0%X0 %1,%0" \
- : "=m" (*addr) : "r" (val) : "memory"); \
+ : "=m"UPD_CONSTR (*addr) : "r" (val) : "memory"); \
mmiowb_set_pending(); \
}
diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h
index a00e4c1746d6..55ef2112ed00 100644
--- a/arch/powerpc/include/asm/nohash/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/pgtable.h
@@ -200,7 +200,7 @@ static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
stw%U0%X0 %2,%0\n\
eieio\n\
stw%U1%X1 %L2,%1"
- : "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
+ : "=m"UPD_CONSTR (*ptep), "=m"UPD_CONSTR (*((unsigned char *)ptep+4))
: "r" (pte) : "memory");
return;
}
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 13999123b735..cf52d26f49cd 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -1087,7 +1087,7 @@ static inline u64 sp_to_dp(u32 fprs)
preempt_disable();
enable_kernel_fp();
- asm ("lfs%U1%X1 0,%1; stfd%U0%X0 0,%0" : "=m" (fprd) : "m" (fprs)
+ asm ("lfs%U1%X1 0,%1; stfd%U0%X0 0,%0" : "=m"UPD_CONSTR (fprd) : "m"UPD_CONSTR (fprs)
: "fr0");
preempt_enable();
return fprd;
@@ -1099,7 +1099,7 @@ static inline u32 dp_to_sp(u64 fprd)
preempt_disable();
enable_kernel_fp();
- asm ("lfd%U1%X1 0,%1; stfs%U0%X0 0,%0" : "=m" (fprs) : "m" (fprd)
+ asm ("lfd%U1%X1 0,%1; stfs%U0%X0 0,%0" : "=m"UPD_CONSTR (fprs) : "m"UPD_CONSTR (fprd)
: "fr0");
preempt_enable();
return fprs;
--
2.25.0
^ permalink raw reply related
* Re: [PATCH 3/3] powerpc: Fix pre-update addressing in inline assembly
From: Christophe Leroy @ 2020-10-20 7:44 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20201019202441.GU2672@gate.crashing.org>
Le 19/10/2020 à 22:24, Segher Boessenkool a écrit :
> On Mon, Oct 19, 2020 at 12:12:48PM +0000, Christophe Leroy wrote:
>> In several places, inline assembly uses the "%Un" modifier
>> to enable the use of instruction with pre-update addressing,
>
> Calling this "pre-update" is misleading: the register is not updated
> before the address is generated (or the memory access done!), and the
> addressing is exactly the same as the "non-u" insn would use. It is
> called an "update form" instruction, because (at the same time as doing
> the memory access, logically anyway) it writes back the address used to
> the base register.
>
>> but the associated "<>" constraint is missing.
>
> But that is just fine. Pointless, sure, but not a bug.
Most of those are from prehistoric code. So at some point in time it was effective. Then one day GCC
changed it's way and they became pointless. So, not a software bug, but still a regression at some
point.
>
>> Use UPD_CONSTR macro everywhere %Un modifier is used.
>
> Eww. My poor stomach.
There are not that many :)
>
> Have you verified that update form is *correct* in all these, and that
> we even *want* this there?
I can't see anything that would militate against it, do you ?
I guess if the elders have put %Us there, it was wanted.
Christophe
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/watchpoint: Workaround P10 DD1 issue with VSX-32 byte instructions
From: kernel test robot @ 2020-10-20 7:53 UTC (permalink / raw)
To: Ravi Bangoria, mpe
Cc: christophe.leroy, ravi.bangoria, mikey, kbuild-all, jniethe5,
npiggin, maddy, paulus, naveen.n.rao, linuxppc-dev
In-Reply-To: <20201020054454.194343-2-ravi.bangoria@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 3524 bytes --]
Hi Ravi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on powerpc/next]
[also build test WARNING on v5.9 next-20201016]
[cannot apply to mpe/next scottwood/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Ravi-Bangoria/powerpc-Introduce-POWER10_DD1-feature/20201020-134813
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allmodconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/a873a50e35b4c881b6bb53f48ae8ef7bb3e576eb
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ravi-Bangoria/powerpc-Introduce-POWER10_DD1-feature/20201020-134813
git checkout a873a50e35b4c881b6bb53f48ae8ef7bb3e576eb
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
arch/powerpc/kernel/hw_breakpoint.c: In function 'handle_p10dd1_spurious_exception':
>> arch/powerpc/kernel/hw_breakpoint.c:561:16: warning: variable 'hw_start_addr' set but not used [-Wunused-but-set-variable]
561 | unsigned long hw_start_addr;
| ^~~~~~~~~~~~~
vim +/hw_start_addr +561 arch/powerpc/kernel/hw_breakpoint.c
556
557 static void handle_p10dd1_spurious_exception(struct arch_hw_breakpoint **info,
558 int *hit, unsigned long ea)
559 {
560 int i;
> 561 unsigned long hw_start_addr;
562 unsigned long hw_end_addr;
563
564 /*
565 * Handle spurious exception only when any bp_per_reg is set.
566 * Otherwise this might be created by xmon and not actually a
567 * spurious exception.
568 */
569 for (i = 0; i < nr_wp_slots(); i++) {
570 if (!info[i])
571 continue;
572
573 hw_start_addr = ALIGN_DOWN(info[i]->address, HW_BREAKPOINT_SIZE);
574 hw_end_addr = ALIGN(info[i]->address + info[i]->len, HW_BREAKPOINT_SIZE);
575
576 /*
577 * Ending address of DAWR range is less than starting
578 * address of op.
579 */
580 if ((hw_end_addr - 1) >= ea)
581 continue;
582
583 /*
584 * Those addresses need to be in the same or in two
585 * consecutive 512B blocks;
586 */
587 if (((hw_end_addr - 1) >> 10) != (ea >> 10))
588 continue;
589
590 /*
591 * 'op address + 64B' generates an address that has a
592 * carry into bit 52 (crosses 2K boundary).
593 */
594 if ((ea & 0x800) == ((ea + 64) & 0x800))
595 continue;
596
597 break;
598 }
599
600 if (i == nr_wp_slots())
601 return;
602
603 for (i = 0; i < nr_wp_slots(); i++) {
604 if (info[i]) {
605 hit[i] = 1;
606 info[i]->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
607 }
608 }
609 }
610
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 70272 bytes --]
^ permalink raw reply
* RE: [PATCH 08/20] dt-bindings: usb: renesas-xhci: Refer to the usb-xhci.yaml file
From: Yoshihiro Shimoda @ 2020-10-20 4:42 UTC (permalink / raw)
To: Serge Semin, Mathias Nyman, Felipe Balbi, Greg Kroah-Hartman,
Rob Herring, Prabhakar Mahadev Lad
Cc: devicetree@vger.kernel.org, linux-snps-arc@lists.infradead.org,
linux-kernel@vger.kernel.org, Neil Armstrong, Kevin Hilman,
linux-usb@vger.kernel.org, linux-mips@vger.kernel.org,
Serge Semin, Bjorn Andersson, Manu Gautam, Andy Gross,
Pavel Parkhomenko, Alexey Malahov, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org, Roger Quadros
In-Reply-To: <20201014101402.18271-9-Sergey.Semin@baikalelectronics.ru>
Hi,
> From: Serge Semin, Sent: Wednesday, October 14, 2020 7:14 PM
>
> With minor peculiarities (like uploading some vendor-specific firmware)
> these are just Generic xHCI controllers fully compatible with its
> properties. Make sure the Renesas USB xHCI DT nodes are also validated
> against the Generic xHCI DT schema.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
Thank you for the patch!
Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Best regards,
Yoshihiro Shimoda
^ permalink raw reply
* [PATCH v3 01/16] dt-bindings: usb: usb-hcd: Convert generic USB properties to DT schema
From: Serge Semin @ 2020-10-20 11:20 UTC (permalink / raw)
To: Mathias Nyman, Felipe Balbi, Krzysztof Kozlowski,
Greg Kroah-Hartman, Rob Herring
Cc: devicetree, linux-snps-arc, linux-mips, Neil Armstrong,
Martin Blumenstingl, Kevin Hilman, Yoshihiro Shimoda, linux-usb,
linux-kernel, Lad Prabhakar, Serge Semin, Bjorn Andersson,
Serge Semin, Manu Gautam, Andy Gross, Pavel Parkhomenko,
Alexey Malahov, linuxppc-dev, Rob Herring, linux-arm-kernel,
Roger Quadros
In-Reply-To: <20201020112101.19077-1-Sergey.Semin@baikalelectronics.ru>
The generic USB HCD properties have been described in the legacy bindings
text file: Documentation/devicetree/bindings/usb/generic.txt . Let's
convert it' content into the USB HCD DT schema properties so all USB DT
nodes would be validated to have them properly utilized.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changelog v2:
- Discard '|' in all the new properties, since we don't need to preserve
the text formatting.
- Convert abbreviated form of the "maximum-speed" enum restriction into
the multi-lined version of the list.
- Drop quotes from around the string constants.
---
.../devicetree/bindings/usb/generic.txt | 57 ------------
.../devicetree/bindings/usb/usb-hcd.yaml | 88 +++++++++++++++++++
2 files changed, 88 insertions(+), 57 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/usb/generic.txt
diff --git a/Documentation/devicetree/bindings/usb/generic.txt b/Documentation/devicetree/bindings/usb/generic.txt
deleted file mode 100644
index ba472e7aefc9..000000000000
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ /dev/null
@@ -1,57 +0,0 @@
-Generic USB Properties
-
-Optional properties:
- - maximum-speed: tells USB controllers we want to work up to a certain
- speed. Valid arguments are "super-speed-plus",
- "super-speed", "high-speed", "full-speed" and
- "low-speed". In case this isn't passed via DT, USB
- controllers should default to their maximum HW
- capability.
- - dr_mode: tells Dual-Role USB controllers that we want to work on a
- particular mode. Valid arguments are "host",
- "peripheral" and "otg". In case this attribute isn't
- passed via DT, USB DRD controllers should default to
- OTG.
- - phy_type: tells USB controllers that we want to configure the core to support
- a UTMI+ PHY with an 8- or 16-bit interface if UTMI+ is
- selected. Valid arguments are "utmi" and "utmi_wide".
- In case this isn't passed via DT, USB controllers should
- default to HW capability.
- - otg-rev: tells usb driver the release number of the OTG and EH supplement
- with which the device and its descriptors are compliant,
- in binary-coded decimal (i.e. 2.0 is 0200H). This
- property is used if any real OTG features(HNP/SRP/ADP)
- is enabled, if ADP is required, otg-rev should be
- 0x0200 or above.
- - companion: phandle of a companion
- - hnp-disable: tells OTG controllers we want to disable OTG HNP, normally HNP
- is the basic function of real OTG except you want it
- to be a srp-capable only B device.
- - srp-disable: tells OTG controllers we want to disable OTG SRP, SRP is
- optional for OTG device.
- - adp-disable: tells OTG controllers we want to disable OTG ADP, ADP is
- optional for OTG device.
- - usb-role-switch: boolean, indicates that the device is capable of assigning
- the USB data role (USB host or USB device) for a given
- USB connector, such as Type-C, Type-B(micro).
- see connector/usb-connector.yaml.
- - role-switch-default-mode: indicating if usb-role-switch is enabled, the
- device default operation mode of controller while usb
- role is USB_ROLE_NONE. Valid arguments are "host" and
- "peripheral". Defaults to "peripheral" if not
- specified.
-
-
-This is an attribute to a USB controller such as:
-
-dwc3@4a030000 {
- compatible = "synopsys,dwc3";
- reg = <0x4a030000 0xcfff>;
- interrupts = <0 92 4>
- usb-phy = <&usb2_phy>, <&usb3,phy>;
- maximum-speed = "super-speed";
- dr_mode = "otg";
- phy_type = "utmi_wide";
- otg-rev = <0x0200>;
- adp-disable;
-};
diff --git a/Documentation/devicetree/bindings/usb/usb-hcd.yaml b/Documentation/devicetree/bindings/usb/usb-hcd.yaml
index 7263b7f2b510..ee7ea205c71d 100644
--- a/Documentation/devicetree/bindings/usb/usb-hcd.yaml
+++ b/Documentation/devicetree/bindings/usb/usb-hcd.yaml
@@ -22,9 +22,97 @@ properties:
description:
Name specifier for the USB PHY
+ maximum-speed:
+ description:
+ Tells USB controllers we want to work up to a certain speed. In case this
+ isn't passed via DT, USB controllers should default to their maximum HW
+ capability.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum:
+ - low-speed
+ - full-speed
+ - high-speed
+ - super-speed
+ - super-speed-plus
+
+ dr_mode:
+ description:
+ Tells Dual-Role USB controllers that we want to work on a particular
+ mode. In case this attribute isn't passed via DT, USB DRD controllers
+ should default to OTG.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [host, peripheral, otg]
+
+ phy_type:
+ description:
+ Tells USB controllers that we want to configure the core to support a
+ UTMI+ PHY with an 8- or 16-bit interface if UTMI+ is selected. In case
+ this isn't passed via DT, USB controllers should default to HW
+ capability.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [utmi, utmi_wide]
+
+ otg-rev:
+ description:
+ Tells usb driver the release number of the OTG and EH supplement with
+ which the device and its descriptors are compliant, in binary-coded
+ decimal (i.e. 2.0 is 0200H). This property is used if any real OTG
+ features (HNP/SRP/ADP) is enabled. If ADP is required, otg-rev should be
+ 0x0200 or above.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ companion:
+ description: Phandle of a companion device
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ hnp-disable:
+ description:
+ Tells OTG controllers we want to disable OTG HNP. Normally HNP is the
+ basic function of real OTG except you want it to be a srp-capable only B
+ device.
+ type: boolean
+
+ srp-disable:
+ description:
+ Tells OTG controllers we want to disable OTG SRP. SRP is optional for OTG
+ device.
+ type: boolean
+
+ adp-disable:
+ description:
+ Tells OTG controllers we want to disable OTG ADP. ADP is optional for OTG
+ device.
+ type: boolean
+
+ usb-role-switch:
+ description:
+ Indicates that the device is capable of assigning the USB data role
+ (USB host or USB device) for a given USB connector, such as Type-C,
+ Type-B(micro). See connector/usb-connector.yaml.
+
+ role-switch-default-mode:
+ description:
+ Indicates if usb-role-switch is enabled, the device default operation
+ mode of controller while usb role is USB_ROLE_NONE.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [host, peripheral]
+ default: peripheral
+
examples:
- |
usb {
phys = <&usb2_phy1>, <&usb3_phy1>;
phy-names = "usb";
};
+ - |
+ usb@4a030000 {
+ compatible = "snps,dwc3";
+ reg = <0x4a030000 0xcfff>;
+ interrupts = <0 92 4>;
+ usb-phy = <&usb2_phy>, <&usb3_phy>;
+ maximum-speed = "super-speed";
+ dr_mode = "otg";
+ phy_type = "utmi_wide";
+ otg-rev = <0x0200>;
+ adp-disable;
+ };
--
2.27.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox