Re: [PATCH v5 04/27] arm64: daifflags: Use irqflags functions for daifflags

[Catalin Marinas]

On Tue, Aug 28, 2018 at 04:51:14PM +0100, Julien Thierry wrote:
> Some of the work done in daifflags save/restore is already provided
> by irqflags functions. Daifflags should always be a superset of irqflags
> (it handles irq status + status of other flags). Modifying behaviour of
> irqflags should alter the behaviour of daifflags.
> 
> Use irqflags_save/restore functions for the corresponding daifflags
> operation.
> 
> Signed-off-by: Julien Thierry 
> Cc: Catalin Marinas 
> Cc: Will Deacon 
> Cc: James Morse 

Queued for 4.20. Thanks.

-- 
Catalin

Re: [PATCH v5 04/27] arm64: daifflags: Use irqflags functions for daifflags

[Catalin Marinas]

On Tue, Aug 28, 2018 at 04:51:14PM +0100, Julien Thierry wrote:
> Some of the work done in daifflags save/restore is already provided
> by irqflags functions. Daifflags should always be a superset of irqflags
> (it handles irq status + status of other flags). Modifying behaviour of
> irqflags should alter the behaviour of daifflags.
> 
> Use irqflags_save/restore functions for the corresponding daifflags
> operation.
> 
> Signed-off-by: Julien Thierry 
> Cc: Catalin Marinas 
> Cc: Will Deacon 
> Cc: James Morse 

Queued for 4.20. Thanks.

-- 
Catalin

Re: [PATCH v5 04/27] arm64: daifflags: Use irqflags functions for daifflags

[James Morse]

Hi Julien,

On 28/08/18 16:51, Julien Thierry wrote:
> Some of the work done in daifflags save/restore is already provided
> by irqflags functions. Daifflags should always be a superset of irqflags
> (it handles irq status + status of other flags). Modifying behaviour of
> irqflags should alter the behaviour of daifflags.
> 
> Use irqflags_save/restore functions for the corresponding daifflags
> operation.

> diff --git a/arch/arm64/include/asm/daifflags.h 
> b/arch/arm64/include/asm/daifflags.h
> index 22e4c83..8d91f22 100644
> --- a/arch/arm64/include/asm/daifflags.h
> +++ b/arch/arm64/include/asm/daifflags.h
> @@ -36,11 +36,8 @@ static inline unsigned long local_daif_save(void)
>  {
>   unsigned long flags;
>  
> - asm volatile(
> - "mrs%0, daif// local_daif_save\n"
> - : "=r" (flags)
> - :
> - : "memory");
> + flags = arch_local_save_flags();

I clearly should have done this from the beginning!
(I thought there was some header-loop that prevented it, but I can't reproduce
that now!)


>   local_daif_mask();
>  
>   return flags;
> @@ -60,11 +57,9 @@ static inline void local_daif_restore(unsigned long flags)
>  {
>   if (!arch_irqs_disabled_flags(flags))
>   trace_hardirqs_on();
> - asm volatile(
> - "msrdaif, %0// local_daif_restore"
> - :
> - : "r" (flags)
> - : "memory");
> +
> + arch_local_irq_restore(flags);

And I thought this only messed with the I bit, which it clearly doesn't.

Thanks for fixing these!

Reviewed-by: James Morse 

Re: [PATCH v5 04/27] arm64: daifflags: Use irqflags functions for daifflags

[James Morse]

Hi Julien,

On 28/08/18 16:51, Julien Thierry wrote:
> Some of the work done in daifflags save/restore is already provided
> by irqflags functions. Daifflags should always be a superset of irqflags
> (it handles irq status + status of other flags). Modifying behaviour of
> irqflags should alter the behaviour of daifflags.
> 
> Use irqflags_save/restore functions for the corresponding daifflags
> operation.

> diff --git a/arch/arm64/include/asm/daifflags.h 
> b/arch/arm64/include/asm/daifflags.h
> index 22e4c83..8d91f22 100644
> --- a/arch/arm64/include/asm/daifflags.h
> +++ b/arch/arm64/include/asm/daifflags.h
> @@ -36,11 +36,8 @@ static inline unsigned long local_daif_save(void)
>  {
>   unsigned long flags;
>  
> - asm volatile(
> - "mrs%0, daif// local_daif_save\n"
> - : "=r" (flags)
> - :
> - : "memory");
> + flags = arch_local_save_flags();

I clearly should have done this from the beginning!
(I thought there was some header-loop that prevented it, but I can't reproduce
that now!)


>   local_daif_mask();
>  
>   return flags;
> @@ -60,11 +57,9 @@ static inline void local_daif_restore(unsigned long flags)
>  {
>   if (!arch_irqs_disabled_flags(flags))
>   trace_hardirqs_on();
> - asm volatile(
> - "msrdaif, %0// local_daif_restore"
> - :
> - : "r" (flags)
> - : "memory");
> +
> + arch_local_irq_restore(flags);

And I thought this only messed with the I bit, which it clearly doesn't.

Thanks for fixing these!

Reviewed-by: James Morse 

[PATCH v5 04/27] arm64: daifflags: Use irqflags functions for daifflags

[Julien Thierry]

Some of the work done in daifflags save/restore is already provided
by irqflags functions. Daifflags should always be a superset of irqflags
(it handles irq status + status of other flags). Modifying behaviour of
irqflags should alter the behaviour of daifflags.

Use irqflags_save/restore functions for the corresponding daifflags
operation.

Signed-off-by: Julien Thierry 
Cc: Catalin Marinas 
Cc: Will Deacon 
Cc: James Morse 
---
 arch/arm64/include/asm/daifflags.h | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/daifflags.h 
b/arch/arm64/include/asm/daifflags.h
index 22e4c83..8d91f22 100644
--- a/arch/arm64/include/asm/daifflags.h
+++ b/arch/arm64/include/asm/daifflags.h
@@ -36,11 +36,8 @@ static inline unsigned long local_daif_save(void)
 {
unsigned long flags;
 
-   asm volatile(
-   "mrs%0, daif// local_daif_save\n"
-   : "=r" (flags)
-   :
-   : "memory");
+   flags = arch_local_save_flags();
+
local_daif_mask();
 
return flags;
@@ -60,11 +57,9 @@ static inline void local_daif_restore(unsigned long flags)
 {
if (!arch_irqs_disabled_flags(flags))
trace_hardirqs_on();
-   asm volatile(
-   "msrdaif, %0// local_daif_restore"
-   :
-   : "r" (flags)
-   : "memory");
+
+   arch_local_irq_restore(flags);
+
if (arch_irqs_disabled_flags(flags))
trace_hardirqs_off();
 }
-- 
1.9.1

[PATCH v5 04/27] arm64: daifflags: Use irqflags functions for daifflags

[Julien Thierry]

Some of the work done in daifflags save/restore is already provided
by irqflags functions. Daifflags should always be a superset of irqflags
(it handles irq status + status of other flags). Modifying behaviour of
irqflags should alter the behaviour of daifflags.

Use irqflags_save/restore functions for the corresponding daifflags
operation.

Signed-off-by: Julien Thierry 
Cc: Catalin Marinas 
Cc: Will Deacon 
Cc: James Morse 
---
 arch/arm64/include/asm/daifflags.h | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/daifflags.h 
b/arch/arm64/include/asm/daifflags.h
index 22e4c83..8d91f22 100644
--- a/arch/arm64/include/asm/daifflags.h
+++ b/arch/arm64/include/asm/daifflags.h
@@ -36,11 +36,8 @@ static inline unsigned long local_daif_save(void)
 {
unsigned long flags;
 
-   asm volatile(
-   "mrs%0, daif// local_daif_save\n"
-   : "=r" (flags)
-   :
-   : "memory");
+   flags = arch_local_save_flags();
+
local_daif_mask();
 
return flags;
@@ -60,11 +57,9 @@ static inline void local_daif_restore(unsigned long flags)
 {
if (!arch_irqs_disabled_flags(flags))
trace_hardirqs_on();
-   asm volatile(
-   "msrdaif, %0// local_daif_restore"
-   :
-   : "r" (flags)
-   : "memory");
+
+   arch_local_irq_restore(flags);
+
if (arch_irqs_disabled_flags(flags))
trace_hardirqs_off();
 }
-- 
1.9.1