linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc: warn on emulation of dcbz instruction
@ 2021-09-15 14:31 Christophe Leroy
  2021-09-16  7:15 ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2021-09-15 14:31 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Christophe Leroy, linux-kernel, linuxppc-dev, Stan Johnson, Finn Thain

dcbz instruction shouldn't be used on non-cached memory. Using
it on non-cached memory can result in alignment exception and
implies a heavy handling.

Instead of silentely emulating the instruction and resulting in high
performance degradation, warn whenever an alignment exception is
taken due to dcbz, so that the user is made aware that dcbz
instruction has been used unexpectedly.

Reported-by: Stan Johnson <userm57@yahoo.com>
Cc: Finn Thain <fthain@linux-m68k.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/align.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index bbb4181621dd..adc3a4a9c6e4 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -349,6 +349,7 @@ int fix_alignment(struct pt_regs *regs)
 		if (op.type != CACHEOP + DCBZ)
 			return -EINVAL;
 		PPC_WARN_ALIGNMENT(dcbz, regs);
+		WARN_ON_ONCE(1);
 		r = emulate_dcbz(op.ea, regs);
 	} else {
 		if (type == LARX || type == STCX)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc: warn on emulation of dcbz instruction
  2021-09-15 14:31 [PATCH] powerpc: warn on emulation of dcbz instruction Christophe Leroy
@ 2021-09-16  7:15 ` Benjamin Herrenschmidt
  2021-09-16  7:16   ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2021-09-16  7:15 UTC (permalink / raw)
  To: Christophe Leroy, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev, Stan Johnson, Finn Thain

On Wed, 2021-09-15 at 16:31 +0200, Christophe Leroy wrote:
> dcbz instruction shouldn't be used on non-cached memory. Using
> it on non-cached memory can result in alignment exception and
> implies a heavy handling.
> 
> Instead of silentely emulating the instruction and resulting in high
> performance degradation, warn whenever an alignment exception is
> taken due to dcbz, so that the user is made aware that dcbz
> instruction has been used unexpectedly.
> 
> Reported-by: Stan Johnson <userm57@yahoo.com>
> Cc: Finn Thain <fthain@linux-m68k.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>  arch/powerpc/kernel/align.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/powerpc/kernel/align.c
> b/arch/powerpc/kernel/align.c
> index bbb4181621dd..adc3a4a9c6e4 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -349,6 +349,7 @@ int fix_alignment(struct pt_regs *regs)
>  		if (op.type != CACHEOP + DCBZ)
>  			return -EINVAL;
>  		PPC_WARN_ALIGNMENT(dcbz, regs);
> +		WARN_ON_ONCE(1);

This is heavy handed ... It will be treated as an oops by various
things uselessly spit out a kernel backtrace. Isn't PPC_WARN_ALIGNMENT
enough ?

Ben.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc: warn on emulation of dcbz instruction
  2021-09-16  7:15 ` Benjamin Herrenschmidt
@ 2021-09-16  7:16   ` Benjamin Herrenschmidt
  2021-09-16  7:23     ` Christophe Leroy
  0 siblings, 1 reply; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2021-09-16  7:16 UTC (permalink / raw)
  To: Christophe Leroy, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev, Stan Johnson, Finn Thain

On Thu, 2021-09-16 at 17:15 +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2021-09-15 at 16:31 +0200, Christophe Leroy wrote:
> > dcbz instruction shouldn't be used on non-cached memory. Using
> > it on non-cached memory can result in alignment exception and
> > implies a heavy handling.
> > 
> > Instead of silentely emulating the instruction and resulting in
> > high
> > performance degradation, warn whenever an alignment exception is
> > taken due to dcbz, so that the user is made aware that dcbz
> > instruction has been used unexpectedly.
> > 
> > Reported-by: Stan Johnson <userm57@yahoo.com>
> > Cc: Finn Thain <fthain@linux-m68k.org>
> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > ---
> >  arch/powerpc/kernel/align.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/powerpc/kernel/align.c
> > b/arch/powerpc/kernel/align.c
> > index bbb4181621dd..adc3a4a9c6e4 100644
> > --- a/arch/powerpc/kernel/align.c
> > +++ b/arch/powerpc/kernel/align.c
> > @@ -349,6 +349,7 @@ int fix_alignment(struct pt_regs *regs)
> >  		if (op.type != CACHEOP + DCBZ)
> >  			return -EINVAL;
> >  		PPC_WARN_ALIGNMENT(dcbz, regs);
> > +		WARN_ON_ONCE(1);
> 
> This is heavy handed ... It will be treated as an oops by various
> things uselessly spit out a kernel backtrace. Isn't
> PPC_WARN_ALIGNMENT
> enough ?

Ah I saw your other one about fbdev...  Ok what about you do that in a
if (!user_mode(regs)) ?

Indeed the kernel should not do that.

Cheers,
Ben.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc: warn on emulation of dcbz instruction
  2021-09-16  7:16   ` Benjamin Herrenschmidt
@ 2021-09-16  7:23     ` Christophe Leroy
  2021-09-16 14:36       ` David Laight
  0 siblings, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2021-09-16  7:23 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev, Stan Johnson, Finn Thain



Le 16/09/2021 à 09:16, Benjamin Herrenschmidt a écrit :
> On Thu, 2021-09-16 at 17:15 +1000, Benjamin Herrenschmidt wrote:
>> On Wed, 2021-09-15 at 16:31 +0200, Christophe Leroy wrote:
>>> dcbz instruction shouldn't be used on non-cached memory. Using
>>> it on non-cached memory can result in alignment exception and
>>> implies a heavy handling.
>>>
>>> Instead of silentely emulating the instruction and resulting in
>>> high
>>> performance degradation, warn whenever an alignment exception is
>>> taken due to dcbz, so that the user is made aware that dcbz
>>> instruction has been used unexpectedly.
>>>
>>> Reported-by: Stan Johnson <userm57@yahoo.com>
>>> Cc: Finn Thain <fthain@linux-m68k.org>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>> ---
>>>   arch/powerpc/kernel/align.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/powerpc/kernel/align.c
>>> b/arch/powerpc/kernel/align.c
>>> index bbb4181621dd..adc3a4a9c6e4 100644
>>> --- a/arch/powerpc/kernel/align.c
>>> +++ b/arch/powerpc/kernel/align.c
>>> @@ -349,6 +349,7 @@ int fix_alignment(struct pt_regs *regs)
>>>   		if (op.type != CACHEOP + DCBZ)
>>>   			return -EINVAL;
>>>   		PPC_WARN_ALIGNMENT(dcbz, regs);
>>> +		WARN_ON_ONCE(1);
>>
>> This is heavy handed ... It will be treated as an oops by various
>> things uselessly spit out a kernel backtrace. Isn't
>> PPC_WARN_ALIGNMENT
>> enough ?


PPC_WARN_ALIGNMENT() only warns if explicitely activated, I want to 
catch uses on 'dcbz' on non-cached memory all the time as they are most 
often the result of using memset() instead of memset_io().

> 
> Ah I saw your other one about fbdev...  Ok what about you do that in a
> if (!user_mode(regs)) ?

Yes I can do WARN_ON_ONCE(!user_mode(regs)); instead.

> 
> Indeed the kernel should not do that.


Does userspace accesses non-cached memory directly ?

Christophe

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] powerpc: warn on emulation of dcbz instruction
  2021-09-16  7:23     ` Christophe Leroy
@ 2021-09-16 14:36       ` David Laight
  2021-09-17 12:33         ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 6+ messages in thread
From: David Laight @ 2021-09-16 14:36 UTC (permalink / raw)
  To: 'Christophe Leroy',
	Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev, Stan Johnson, Finn Thain

From: Christophe Leroy
> Sent: 16 September 2021 08:24
> 
> Le 16/09/2021 à 09:16, Benjamin Herrenschmidt a écrit :
> > On Thu, 2021-09-16 at 17:15 +1000, Benjamin Herrenschmidt wrote:
> >> On Wed, 2021-09-15 at 16:31 +0200, Christophe Leroy wrote:
> >>> dcbz instruction shouldn't be used on non-cached memory. Using
> >>> it on non-cached memory can result in alignment exception and
> >>> implies a heavy handling.
> >>>
> >>> Instead of silentely emulating the instruction and resulting in
> >>> high
> >>> performance degradation, warn whenever an alignment exception is
> >>> taken due to dcbz, so that the user is made aware that dcbz
> >>> instruction has been used unexpectedly.
> >>>
> >>> Reported-by: Stan Johnson <userm57@yahoo.com>
> >>> Cc: Finn Thain <fthain@linux-m68k.org>
> >>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> >>> ---
> >>>   arch/powerpc/kernel/align.c | 1 +
> >>>   1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/arch/powerpc/kernel/align.c
> >>> b/arch/powerpc/kernel/align.c
> >>> index bbb4181621dd..adc3a4a9c6e4 100644
> >>> --- a/arch/powerpc/kernel/align.c
> >>> +++ b/arch/powerpc/kernel/align.c
> >>> @@ -349,6 +349,7 @@ int fix_alignment(struct pt_regs *regs)
> >>>   		if (op.type != CACHEOP + DCBZ)
> >>>   			return -EINVAL;
> >>>   		PPC_WARN_ALIGNMENT(dcbz, regs);
> >>> +		WARN_ON_ONCE(1);
> >>
> >> This is heavy handed ... It will be treated as an oops by various
> >> things uselessly spit out a kernel backtrace. Isn't
> >> PPC_WARN_ALIGNMENT
> >> enough ?
> 
> 
> PPC_WARN_ALIGNMENT() only warns if explicitely activated, I want to
> catch uses on 'dcbz' on non-cached memory all the time as they are most
> often the result of using memset() instead of memset_io().
> 
> >
> > Ah I saw your other one about fbdev...  Ok what about you do that in a
> > if (!user_mode(regs)) ?
> 
> Yes I can do WARN_ON_ONCE(!user_mode(regs)); instead.
> 
> > Indeed the kernel should not do that.
> 
> Does userspace accesses non-cached memory directly ?

It probably can if a driver mmaps PCI space directly into user space.
That certainly works on x86-64.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc: warn on emulation of dcbz instruction
  2021-09-16 14:36       ` David Laight
@ 2021-09-17 12:33         ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2021-09-17 12:33 UTC (permalink / raw)
  To: David Laight, 'Christophe Leroy',
	Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev, Stan Johnson, Finn Thain

On Thu, 2021-09-16 at 14:36 +0000, David Laight wrote:
> > Does userspace accesses non-cached memory directly ?
> 
> 
> It probably can if a driver mmaps PCI space directly into user space.
> 
> That certainly works on x86-64.

The posterchild for that is Xorg

Cheers,
Ben.



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-09-17 12:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-15 14:31 [PATCH] powerpc: warn on emulation of dcbz instruction Christophe Leroy
2021-09-16  7:15 ` Benjamin Herrenschmidt
2021-09-16  7:16   ` Benjamin Herrenschmidt
2021-09-16  7:23     ` Christophe Leroy
2021-09-16 14:36       ` David Laight
2021-09-17 12:33         ` Benjamin Herrenschmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).