Re: [PATCH] h8300: Make irq disable and enable a compiler barrier

[Steven Rostedt]

Ping!

(Warning 2: is this architecture still maintained?)

-- Steve

On Fri, 18 Sep 2020 15:25:07 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> From: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>
> 
> In a conversation on #linux-rt, it was asked if the irq disabling and
> enabling functions were a compiler barrier, and reads wont happen outside
> these functions from where they were in the call.
> 
> I stated that they most definitely need to be, otherwise we would have bugs
> all over the place if not. And just to confirm, I looked at the
> implementation on x86 which had "memory" as a constraint to the asm, and
> that is a compiler barrier. As that was just one arch, I looked at others,
> and all the other archs do the same but one. H8300 seems to fail here. And
> that is most definitely a bug.
> 
> Add the compiler barrier to the enabling and disabling of interrupts on
> H8300. As this is a really critical bug, I'm starting to wonder how much
> this arch is really used. Let's see how quickly this patch gets added to
> the arch, and if it is not applied or responded to within a month, I think
> we can safely state that this arch is not maintained.
> 
> Reported-by: Vlastimil Babka <vbabka@xxxxxxx>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>
> ---
> diff --git a/arch/h8300/include/asm/irqflags.h b/arch/h8300/include/asm/irqflags.h
> index 48756b7f405e..477cb9d7785f 100644
> --- a/arch/h8300/include/asm/irqflags.h
> +++ b/arch/h8300/include/asm/irqflags.h
> @@ -15,12 +15,12 @@ static inline h8300flags arch_local_save_flags(void)
>  
>  static inline void arch_local_irq_disable(void)
>  {
> -	__asm__ volatile ("orc  #0xc0,ccr");
> +	__asm__ volatile ("orc  #0xc0,ccr" : : : "memory");
>  }
>  
>  static inline void arch_local_irq_enable(void)
>  {
> -	__asm__ volatile ("andc #0x3f,ccr");
> +	__asm__ volatile ("andc #0x3f,ccr" : : : "memory");
>  }
>  
>  static inline h8300flags arch_local_irq_save(void)
> @@ -28,13 +28,13 @@ static inline h8300flags arch_local_irq_save(void)
>  	h8300flags flags;
>  
>  	__asm__ volatile ("stc ccr,%w0\n\t"
> -		      "orc  #0xc0,ccr" : "=r" (flags));
> +		      "orc  #0xc0,ccr" : "=r" (flags) : : "memory");
>  	return flags;
>  }
>  
>  static inline void arch_local_irq_restore(h8300flags flags)
>  {
> -	__asm__ volatile ("ldc %w0,ccr" : : "r" (flags) : "cc");
> +	__asm__ volatile ("ldc %w0,ccr" : : "r" (flags) : "memory", "cc");
>  }
>  
>  static inline int arch_irqs_disabled_flags(unsigned long flags)
> @@ -55,13 +55,13 @@ static inline h8300flags arch_local_save_flags(void)
>  
>  static inline void arch_local_irq_disable(void)
>  {
> -	__asm__ volatile ("orc #0x80,ccr\n\t");
> +	__asm__ volatile ("orc #0x80,ccr\n\t" : : : "memory");
>  }
>  
>  static inline void arch_local_irq_enable(void)
>  {
>  	__asm__ volatile ("andc #0x7f,ccr\n\t"
> -		      "andc #0xf0,exr\n\t");
> +		      "andc #0xf0,exr\n\t" : : : "memory");
>  }
>  
>  static inline h8300flags arch_local_irq_save(void)
> @@ -71,7 +71,7 @@ static inline h8300flags arch_local_irq_save(void)
>  	__asm__ volatile ("stc ccr,%w0\n\t"
>  		      "stc exr,%x0\n\t"
>  		      "orc  #0x80,ccr\n\t"
> -		      : "=r" (flags));
> +		      : "=r" (flags) : : "memory");
>  	return flags;
>  }
>  
> @@ -79,7 +79,7 @@ static inline void arch_local_irq_restore(h8300flags flags)
>  {
>  	__asm__ volatile ("ldc %w0,ccr\n\t"
>  		      "ldc %x0,exr"
> -		      : : "r" (flags) : "cc");
> +		      : : "r" (flags) : "memory", "cc");
>  }
>  
>  static inline int arch_irqs_disabled_flags(h8300flags flags)