Communications among developers and users of Linux on embedded PowerPC boards

Tiejun Chen | 10 Jul 2012 09:59

[PATCH 1/1] powerpc: add "memory" attribute for mfmsr()

Add "memory" attribute in inline assembly language as a compiler
barrier to make sure 4.6.x GCC don't reorder mfmsr().

Signed-off-by: Tiejun Chen <tiejun.chen <at> windriver.com>
---
 arch/powerpc/include/asm/reg.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 559da19..578e5a0 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
 <at>  <at>  -1016,7 +1016,8  <at>  <at> 
 /* Macros for setting and retrieving special purpose registers */
 #ifndef __ASSEMBLY__
 #define mfmsr()		({unsigned long rval; \
-			asm volatile("mfmsr %0" : "=r" (rval)); rval;})
+			asm volatile("mfmsr %0" : "=r" (rval) : \
+						: "memory"); rval;})
 #ifdef CONFIG_PPC_BOOK3S_64
 #define __mtmsrd(v, l)	asm volatile("mtmsrd %0," __stringify(l) \
 				     : : "r" (v) : "memory")
--

-- 
1.5.6

Benjamin Herrenschmidt | 10 Jul 2012 10:19

Re: [PATCH 1/1] powerpc: add "memory" attribute for mfmsr()

On Tue, 2012-07-10 at 15:59 +0800, Tiejun Chen wrote:
> Add "memory" attribute in inline assembly language as a compiler
> barrier to make sure 4.6.x GCC don't reorder mfmsr().

Out of curiosity, did you see a case where it was re-ordered
improperly ?

Cheers,
Ben.

> Signed-off-by: Tiejun Chen <tiejun.chen <at> windriver.com>
> ---
>  arch/powerpc/include/asm/reg.h |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 559da19..578e5a0 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
>  <at>  <at>  -1016,7 +1016,8  <at>  <at> 
>  /* Macros for setting and retrieving special purpose registers */
>  #ifndef __ASSEMBLY__
>  #define mfmsr()		({unsigned long rval; \
> -			asm volatile("mfmsr %0" : "=r" (rval)); rval;})
> +			asm volatile("mfmsr %0" : "=r" (rval) : \
> +						: "memory"); rval;})
>  #ifdef CONFIG_PPC_BOOK3S_64
>  #define __mtmsrd(v, l)	asm volatile("mtmsrd %0," __stringify(l) \
>  				     : : "r" (v) : "memory")

(Continue reading)

tiejun.chen | 10 Jul 2012 10:22

Re: [PATCH 1/1] powerpc: add "memory" attribute for mfmsr()

On 07/10/2012 04:19 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2012-07-10 at 15:59 +0800, Tiejun Chen wrote:
>> Add "memory" attribute in inline assembly language as a compiler
>> barrier to make sure 4.6.x GCC don't reorder mfmsr().
> 
> Out of curiosity, did you see a case where it was re-ordered
> improperly ?

Yes.

In my scenario, there's one simple wrapper from local_irq_save().
------
uint32_t XX_DisableAllIntr(void)
{
    unsigned long flags;

    local_irq_save(flags);

    return (uint32_t)flags;
}

When CONFIG_TRACE_IRQFLAGS_SUPPORT is enabled, kernel would expand
local_irq_save(). Firstly, please refer to the follows:

#define local_irq_save(flags)               \
    do {                        \
        raw_local_irq_save(flags);      \
        trace_hardirqs_off();           \
    } while (0)

(Continue reading)

tiejun.chen | 10 Jul 2012 10:27

Re: [PATCH 1/1] powerpc: add "memory" attribute for mfmsr()

On 07/10/2012 04:22 PM, tiejun.chen wrote:
> On 07/10/2012 04:19 PM, Benjamin Herrenschmidt wrote:
>> On Tue, 2012-07-10 at 15:59 +0800, Tiejun Chen wrote:
>>> Add "memory" attribute in inline assembly language as a compiler
>>> barrier to make sure 4.6.x GCC don't reorder mfmsr().
>>
>> Out of curiosity, did you see a case where it was re-ordered
>> improperly ?
> 
> Yes.
> 
> In my scenario, there's one simple wrapper from local_irq_save().
> ------
> uint32_t XX_DisableAllIntr(void)
> {
>     unsigned long flags;
> 
>     local_irq_save(flags);
> 
>     return (uint32_t)flags;
> }
> 
> When CONFIG_TRACE_IRQFLAGS_SUPPORT is enabled, kernel would expand
> local_irq_save(). Firstly, please refer to the follows:
> 
> #define local_irq_save(flags)               \
>     do {                        \
>         raw_local_irq_save(flags);      \
>         trace_hardirqs_off();           \
>     } while (0)

(Continue reading)

tiejun.chen | 10 Jul 2012 10:27

Re: [PATCH 1/1] powerpc: add "memory" attribute for mfmsr()

On 07/10/2012 04:22 PM, tiejun.chen wrote:
> On 07/10/2012 04:19 PM, Benjamin Herrenschmidt wrote:
>> On Tue, 2012-07-10 at 15:59 +0800, Tiejun Chen wrote:
>>> Add "memory" attribute in inline assembly language as a compiler
>>> barrier to make sure 4.6.x GCC don't reorder mfmsr().
>>
>> Out of curiosity, did you see a case where it was re-ordered
>> improperly ?
> 
> Yes.
> 
> In my scenario, there's one simple wrapper from local_irq_save().
> ------
> uint32_t XX_DisableAllIntr(void)
> {
>     unsigned long flags;
> 
>     local_irq_save(flags);
> 
>     return (uint32_t)flags;
> }
> 
> When CONFIG_TRACE_IRQFLAGS_SUPPORT is enabled, kernel would expand
> local_irq_save(). Firstly, please refer to the follows:
> 
> #define local_irq_save(flags)               \
>     do {                        \
>         raw_local_irq_save(flags);      \
>         trace_hardirqs_off();           \
>     } while (0)

(Continue reading)

tiejun.chen | 10 Jul 2012 10:22

Re: [PATCH 1/1] powerpc: add "memory" attribute for mfmsr()

On 07/10/2012 04:19 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2012-07-10 at 15:59 +0800, Tiejun Chen wrote:
>> Add "memory" attribute in inline assembly language as a compiler
>> barrier to make sure 4.6.x GCC don't reorder mfmsr().
> 
> Out of curiosity, did you see a case where it was re-ordered
> improperly ?

Yes.

In my scenario, there's one simple wrapper from local_irq_save().
------
uint32_t XX_DisableAllIntr(void)
{
    unsigned long flags;

    local_irq_save(flags);

    return (uint32_t)flags;
}

When CONFIG_TRACE_IRQFLAGS_SUPPORT is enabled, kernel would expand
local_irq_save(). Firstly, please refer to the follows:

#define local_irq_save(flags)               \
    do {                        \
        raw_local_irq_save(flags);      \
        trace_hardirqs_off();           \
    } while (0)

(Continue reading)

Benjamin Herrenschmidt | 10 Jul 2012 10:19

Re: [PATCH 1/1] powerpc: add "memory" attribute for mfmsr()

On Tue, 2012-07-10 at 15:59 +0800, Tiejun Chen wrote:
> Add "memory" attribute in inline assembly language as a compiler
> barrier to make sure 4.6.x GCC don't reorder mfmsr().

Out of curiosity, did you see a case where it was re-ordered
improperly ?

Cheers,
Ben.

> Signed-off-by: Tiejun Chen <tiejun.chen <at> windriver.com>
> ---
>  arch/powerpc/include/asm/reg.h |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 559da19..578e5a0 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
>  <at>  <at>  -1016,7 +1016,8  <at>  <at> 
>  /* Macros for setting and retrieving special purpose registers */
>  #ifndef __ASSEMBLY__
>  #define mfmsr()		({unsigned long rval; \
> -			asm volatile("mfmsr %0" : "=r" (rval)); rval;})
> +			asm volatile("mfmsr %0" : "=r" (rval) : \
> +						: "memory"); rval;})
>  #ifdef CONFIG_PPC_BOOK3S_64
>  #define __mtmsrd(v, l)	asm volatile("mtmsrd %0," __stringify(l) \
>  				     : : "r" (v) : "memory")

(Continue reading)

Return

Return to gmane.linux.ports.ppc.embedded.

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane