Re: [PATCH net-next 1/3] arm64: barrier: add DGH macros to control memory accesses merging

[Will Deacon]

On Tue, Jun 22, 2021 at 07:11:09PM +0800, Guangbin Huang wrote:
> From: Xiongfeng Wang <wangxiongfeng2@xxxxxxxxxx>
> 
> DGH prohibits merging memory accesses with Normal-NC or Device-GRE
> attributes before the hint instruction with any memory accesses
> appearing after the hint instruction. Provide macros to expose it to the
> arch code.

Hmm.

The architecture states:

  | DGH is a hint instruction. A DGH instruction is not expected to be
  | performance optimal to merge memory accesses with Normal Non-cacheable
  | or Device-GRE attributes appearing in program order before the hint
  | instruction with any memory accesses appearing after the hint instruction
  | into a single memory transaction on an interconnect.

which doesn't make a whole lot of sense to me, in all honesty.

> Signed-off-by: Xiongfeng Wang <wangxiongfeng2@xxxxxxxxxx>
> Signed-off-by: Cheng Jian <cj.chengjian@xxxxxxxxxx>
> Signed-off-by: Yufeng Mo <moyufeng@xxxxxxxxxx>
> ---
>  arch/arm64/include/asm/assembler.h | 7 +++++++
>  arch/arm64/include/asm/barrier.h   | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index 8418c1bd8f04..d723899328bd 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -90,6 +90,13 @@
>  	.endm
>  
>  /*
> + * Data gathering hint
> + */
> +	.macro	dgh
> +	hint	#6
> +	.endm
> +
> +/*
>   * RAS Error Synchronization barrier
>   */
>  	.macro  esb
> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> index 451e11e5fd23..02e1735706d2 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h
> @@ -22,6 +22,7 @@
>  #define dmb(opt)	asm volatile("dmb " #opt : : : "memory")
>  #define dsb(opt)	asm volatile("dsb " #opt : : : "memory")
>  
> +#define dgh()		asm volatile("hint #6" : : : "memory")

Although I'm fine with this in arm64, I don't think this is the interface
which drivers should be using. Instead, once we know what this instruction
is supposed to do, we should look at exposing it as part of the I/O barriers
and providing a NOP implementation for other architectures. That way,
drivers can use it without having to have the #ifdef CONFIG_ARM64 stuff that
you have in the later patches here.

Will