Re: [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access

[Laszlo Ersek <lersek@redhat.com>]

On 06/05/18 09:30, Ard Biesheuvel wrote:
> Even though MMIO shares the address space with ordinary memory, the
> accesses involved are *not* ordinary memory accesses, and so it is
> a bad idea to let the compiler generate them using pointer dereferences.

I think I slightly disagree with the wording here. Using
volatile-qualified pointers (implying compiler barriers), possibly
combined with CPU barriers (wrapped into C functions), seems valid in
both practice and on the ISO C level. "volatile" does not guarantee
atomicity, but an IoLib instance can still assume that, if it is tied to
a specific toolchain, and the toolchain guarantees atomicity. Anyway:

> 
> Instead, introduce a set of internal accessors implemented in assembler,
> and call those from the various Mmio[Read|Write]XX () implementations.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Open coding the MMIO accesses is a good idea in general, but it also works
> around an issue that affects EDK2 running under KVM on ARM or AARCH64, where
> LTO code generation results in MMIO accesses involving instructions that cannot
> be emulated by the hypervisor.

edk2 code (and developers!) are generally conscious about this issue,
and call the IoLib APIs carefully. Thus, I believe that, with this
patch, we can address the problem in edk2 "for good" -- we can trust
that all such accesses will go through a few "choke points", and we can
add the work-arounds to the "choke points". (This is exactly what I
believe we aren't certain about with regard to the Linux kernel.)

Therefore I welcome the patch! I just think the commit message should
name the KVM workaround as the primary reason. The C code is not wrong
(knowing the compiler), and the generated assembly is also not wrong on
the bare metal.

I don't insist though that you update the commit message; I prefer
working code to documentation that's polished to my taste. :)

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thank you!
Laszlo

> 
>  MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S    | 164 ++++++++++++++++++
>  MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.S        | 164 ++++++++++++++++++
>  MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.asm      | 165 ++++++++++++++++++
>  MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf |   9 +-
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c             |  36 ++--
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.h             | 179 ++++++++++++++++++++
>  6 files changed, 692 insertions(+), 25 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S
> new file mode 100644
> index 000000000000..ac96df602f7a
> --- /dev/null
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S
> @@ -0,0 +1,164 @@
> +#
> +#  Copyright (c) 2014-2018, Linaro Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials are licensed and made available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +#
> +
> +.text
> +.align 3
> +
> +GCC_ASM_EXPORT(MmioRead8Internal)
> +GCC_ASM_EXPORT(MmioWrite8Internal)
> +GCC_ASM_EXPORT(MmioRead16Internal)
> +GCC_ASM_EXPORT(MmioWrite16Internal)
> +GCC_ASM_EXPORT(MmioRead32Internal)
> +GCC_ASM_EXPORT(MmioWrite32Internal)
> +GCC_ASM_EXPORT(MmioRead64Internal)
> +GCC_ASM_EXPORT(MmioWrite64Internal)
> +
> +//
> +//  Reads an 8-bit MMIO register.
> +//
> +//  Reads the 8-bit MMIO register specified by Address. The 8-bit read value is
> +//  returned. This function must guarantee that all MMIO read and write
> +//  operations are serialized.
> +//
> +//  If 8-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to read.
> +//
> +//  @return The value read.
> +//
> +ASM_PFX(MmioRead8Internal):
> +  ldrb    w0, [x0]
> +  dsb     ld
> +  ret
> +
> +//
> +//  Writes an 8-bit MMIO register.
> +//
> +//  Writes the 8-bit MMIO register specified by Address with the value specified
> +//  by Value and returns Value. This function must guarantee that all MMIO read
> +//  and write operations are serialized.
> +//
> +//  If 8-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to write.
> +//  @param  Value   The value to write to the MMIO register.
> +//
> +ASM_PFX(MmioWrite8Internal):
> +  dsb     st
> +  strb    w1, [x0]
> +  ret
> +
> +//
> +//  Reads a 16-bit MMIO register.
> +//
> +//  Reads the 16-bit MMIO register specified by Address. The 16-bit read value is
> +//  returned. This function must guarantee that all MMIO read and write
> +//  operations are serialized.
> +//
> +//  If 16-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to read.
> +//
> +//  @return The value read.
> +//
> +ASM_PFX(MmioRead16Internal):
> +  ldrh    w0, [x0]
> +  dsb     ld
> +  ret
> +
> +//
> +//  Writes a 16-bit MMIO register.
> +//
> +//  Writes the 16-bit MMIO register specified by Address with the value specified
> +//  by Value and returns Value. This function must guarantee that all MMIO read
> +//  and write operations are serialized.
> +//
> +//  If 16-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to write.
> +//  @param  Value   The value to write to the MMIO register.
> +//
> +ASM_PFX(MmioWrite16Internal):
> +  dsb     st
> +  strh    w1, [x0]
> +  ret
> +
> +//
> +//  Reads a 32-bit MMIO register.
> +//
> +//  Reads the 32-bit MMIO register specified by Address. The 32-bit read value is
> +//  returned. This function must guarantee that all MMIO read and write
> +//  operations are serialized.
> +//
> +//  If 32-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to read.
> +//
> +//  @return The value read.
> +//
> +ASM_PFX(MmioRead32Internal):
> +  ldr     w0, [x0]
> +  dsb     ld
> +  ret
> +
> +//
> +//  Writes a 32-bit MMIO register.
> +//
> +//  Writes the 32-bit MMIO register specified by Address with the value specified
> +//  by Value and returns Value. This function must guarantee that all MMIO read
> +//  and write operations are serialized.
> +//
> +//  If 32-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to write.
> +//  @param  Value   The value to write to the MMIO register.
> +//
> +ASM_PFX(MmioWrite32Internal):
> +  dsb     st
> +  str     w1, [x0]
> +  ret
> +
> +//
> +//  Reads a 64-bit MMIO register.
> +//
> +//  Reads the 64-bit MMIO register specified by Address. The 64-bit read value is
> +//  returned. This function must guarantee that all MMIO read and write
> +//  operations are serialized.
> +//
> +//  If 64-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to read.
> +//
> +//  @return The value read.
> +//
> +ASM_PFX(MmioRead64Internal):
> +  ldr     x0, [x0]
> +  dsb     ld
> +  ret
> +
> +//
> +//  Writes a 64-bit MMIO register.
> +//
> +//  Writes the 64-bit MMIO register specified by Address with the value specified
> +//  by Value and returns Value. This function must guarantee that all MMIO read
> +//  and write operations are serialized.
> +//
> +//  If 64-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to write.
> +//  @param  Value   The value to write to the MMIO register.
> +//
> +ASM_PFX(MmioWrite64Internal):
> +  dsb     st
> +  str     x1, [x0]
> +  ret
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.S b/MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.S
> new file mode 100644
> index 000000000000..09791951b2fd
> --- /dev/null
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.S
> @@ -0,0 +1,164 @@
> +#
> +#  Copyright (c) 2014-2018, Linaro Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials are licensed and made available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +#
> +
> +.text
> +.align 3
> +
> +GCC_ASM_EXPORT(MmioRead8Internal)
> +GCC_ASM_EXPORT(MmioWrite8Internal)
> +GCC_ASM_EXPORT(MmioRead16Internal)
> +GCC_ASM_EXPORT(MmioWrite16Internal)
> +GCC_ASM_EXPORT(MmioRead32Internal)
> +GCC_ASM_EXPORT(MmioWrite32Internal)
> +GCC_ASM_EXPORT(MmioRead64Internal)
> +GCC_ASM_EXPORT(MmioWrite64Internal)
> +
> +//
> +//  Reads an 8-bit MMIO register.
> +//
> +//  Reads the 8-bit MMIO register specified by Address. The 8-bit read value is
> +//  returned. This function must guarantee that all MMIO read and write
> +//  operations are serialized.
> +//
> +//  If 8-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to read.
> +//
> +//  @return The value read.
> +//
> +ASM_PFX(MmioRead8Internal):
> +  ldrb    r0, [r0]
> +  dsb
> +  bx      lr
> +
> +//
> +//  Writes an 8-bit MMIO register.
> +//
> +//  Writes the 8-bit MMIO register specified by Address with the value specified
> +//  by Value and returns Value. This function must guarantee that all MMIO read
> +//  and write operations are serialized.
> +//
> +//  If 8-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to write.
> +//  @param  Value   The value to write to the MMIO register.
> +//
> +ASM_PFX(MmioWrite8Internal):
> +  dsb     st
> +  strb    r1, [r0]
> +  bx      lr
> +
> +//
> +//  Reads a 16-bit MMIO register.
> +//
> +//  Reads the 16-bit MMIO register specified by Address. The 16-bit read value is
> +//  returned. This function must guarantee that all MMIO read and write
> +//  operations are serialized.
> +//
> +//  If 16-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to read.
> +//
> +//  @return The value read.
> +//
> +ASM_PFX(MmioRead16Internal):
> +  ldrh    r0, [r0]
> +  dsb
> +  bx      lr
> +
> +//
> +//  Writes a 16-bit MMIO register.
> +//
> +//  Writes the 16-bit MMIO register specified by Address with the value specified
> +//  by Value and returns Value. This function must guarantee that all MMIO read
> +//  and write operations are serialized.
> +//
> +//  If 16-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to write.
> +//  @param  Value   The value to write to the MMIO register.
> +//
> +ASM_PFX(MmioWrite16Internal):
> +  dsb     st
> +  strh    r1, [r0]
> +  bx      lr
> +
> +//
> +//  Reads a 32-bit MMIO register.
> +//
> +//  Reads the 32-bit MMIO register specified by Address. The 32-bit read value is
> +//  returned. This function must guarantee that all MMIO read and write
> +//  operations are serialized.
> +//
> +//  If 32-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to read.
> +//
> +//  @return The value read.
> +//
> +ASM_PFX(MmioRead32Internal):
> +  ldr     r0, [r0]
> +  dsb
> +  bx      lr
> +
> +//
> +//  Writes a 32-bit MMIO register.
> +//
> +//  Writes the 32-bit MMIO register specified by Address with the value specified
> +//  by Value and returns Value. This function must guarantee that all MMIO read
> +//  and write operations are serialized.
> +//
> +//  If 32-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to write.
> +//  @param  Value   The value to write to the MMIO register.
> +//
> +ASM_PFX(MmioWrite32Internal):
> +  dsb     st
> +  str     r1, [r0]
> +  bx      lr
> +
> +//
> +//  Reads a 64-bit MMIO register.
> +//
> +//  Reads the 64-bit MMIO register specified by Address. The 64-bit read value is
> +//  returned. This function must guarantee that all MMIO read and write
> +//  operations are serialized.
> +//
> +//  If 64-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to read.
> +//
> +//  @return The value read.
> +//
> +ASM_PFX(MmioRead64Internal):
> +  ldrd    r0, r1, [r0]
> +  dsb
> +  bx      lr
> +
> +//
> +//  Writes a 64-bit MMIO register.
> +//
> +//  Writes the 64-bit MMIO register specified by Address with the value specified
> +//  by Value and returns Value. This function must guarantee that all MMIO read
> +//  and write operations are serialized.
> +//
> +//  If 64-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to write.
> +//  @param  Value   The value to write to the MMIO register.
> +//
> +ASM_PFX(MmioWrite64Internal):
> +  dsb     st
> +  strd    r2, r3, [r0]
> +  bx      lr
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.asm b/MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.asm
> new file mode 100644
> index 000000000000..59a92962cb21
> --- /dev/null
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.asm
> @@ -0,0 +1,165 @@
> +;
> +;  Copyright (c) 2014-2018, Linaro Limited. All rights reserved.
> +;
> +;  This program and the accompanying materials are licensed and made available
> +;  under the terms and conditions of the BSD License which accompanies this
> +;  distribution.  The full text of the license may be found at
> +;  http:;opensource.org/licenses/bsd-license.php
> +;
> +;  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +;  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +;
> +
> +
> +AREA IoLibMmio, CODE, READONLY
> +
> +EXPORT MmioRead8Internal
> +EXPORT MmioWrite8Internal
> +EXPORT MmioRead16Internal
> +EXPORT MmioWrite16Internal
> +EXPORT MmioRead32Internal
> +EXPORT MmioWrite32Internal
> +EXPORT MmioRead64Internal
> +EXPORT MmioWrite64Internal
> +
> +;
> +;  Reads an 8-bit MMIO register.
> +;
> +;  Reads the 8-bit MMIO register specified by Address. The 8-bit read value is
> +;  returned. This function must guarantee that all MMIO read and write
> +;  operations are serialized.
> +;
> +;  If 8-bit MMIO register operations are not supported, then ASSERT().
> +;
> +;  @param  Address The MMIO register to read.
> +;
> +;  @return The value read.
> +;
> +MmioRead8Internal
> +  ldrb    r0, [r0]
> +  dsb
> +  bx      lr
> +
> +;
> +;  Writes an 8-bit MMIO register.
> +;
> +;  Writes the 8-bit MMIO register specified by Address with the value specified
> +;  by Value and returns Value. This function must guarantee that all MMIO read
> +;  and write operations are serialized.
> +;
> +;  If 8-bit MMIO register operations are not supported, then ASSERT().
> +;
> +;  @param  Address The MMIO register to write.
> +;  @param  Value   The value to write to the MMIO register.
> +;
> +MmioWrite8Internal
> +  dsb     st
> +  strb    r1, [r0]
> +  bx      lr
> +
> +;
> +;  Reads a 16-bit MMIO register.
> +;
> +;  Reads the 16-bit MMIO register specified by Address. The 16-bit read value is
> +;  returned. This function must guarantee that all MMIO read and write
> +;  operations are serialized.
> +;
> +;  If 16-bit MMIO register operations are not supported, then ASSERT().
> +;
> +;  @param  Address The MMIO register to read.
> +;
> +;  @return The value read.
> +;
> +MmioRead16Internal
> +  ldrh    r0, [r0]
> +  dsb
> +  bx      lr
> +
> +;
> +;  Writes a 16-bit MMIO register.
> +;
> +;  Writes the 16-bit MMIO register specified by Address with the value specified
> +;  by Value and returns Value. This function must guarantee that all MMIO read
> +;  and write operations are serialized.
> +;
> +;  If 16-bit MMIO register operations are not supported, then ASSERT().
> +;
> +;  @param  Address The MMIO register to write.
> +;  @param  Value   The value to write to the MMIO register.
> +;
> +MmioWrite16Internal
> +  dsb     st
> +  strh    r1, [r0]
> +  bx      lr
> +
> +;
> +;  Reads a 32-bit MMIO register.
> +;
> +;  Reads the 32-bit MMIO register specified by Address. The 32-bit read value is
> +;  returned. This function must guarantee that all MMIO read and write
> +;  operations are serialized.
> +;
> +;  If 32-bit MMIO register operations are not supported, then ASSERT().
> +;
> +;  @param  Address The MMIO register to read.
> +;
> +;  @return The value read.
> +;
> +MmioRead32Internal
> +  ldr     r0, [r0]
> +  dsb
> +  bx      lr
> +
> +;
> +;  Writes a 32-bit MMIO register.
> +;
> +;  Writes the 32-bit MMIO register specified by Address with the value specified
> +;  by Value and returns Value. This function must guarantee that all MMIO read
> +;  and write operations are serialized.
> +;
> +;  If 32-bit MMIO register operations are not supported, then ASSERT().
> +;
> +;  @param  Address The MMIO register to write.
> +;  @param  Value   The value to write to the MMIO register.
> +;
> +MmioWrite32Internal
> +  dsb     st
> +  str     r1, [r0]
> +  bx      lr
> +
> +;
> +;  Reads a 64-bit MMIO register.
> +;
> +;  Reads the 64-bit MMIO register specified by Address. The 64-bit read value is
> +;  returned. This function must guarantee that all MMIO read and write
> +;  operations are serialized.
> +;
> +;  If 64-bit MMIO register operations are not supported, then ASSERT().
> +;
> +;  @param  Address The MMIO register to read.
> +;
> +;  @return The value read.
> +;
> +MmioRead64Internal
> +  ldrd    r0, r1, [r0]
> +  dsb
> +  bx      lr
> +
> +;
> +;  Writes a 64-bit MMIO register.
> +;
> +;  Writes the 64-bit MMIO register specified by Address with the value specified
> +;  by Value and returns Value. This function must guarantee that all MMIO read
> +;  and write operations are serialized.
> +;
> +;  If 64-bit MMIO register operations are not supported, then ASSERT().
> +;
> +;  @param  Address The MMIO register to write.
> +;  @param  Value   The value to write to the MMIO register.
> +;
> +MmioWrite64Internal
> +  dsb     st
> +  strd    r2, r3, [r0]
> +  bx      lr
> +
> +  END
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> index 8844b1ce4c2b..22813098e97a 100644
> --- a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> @@ -61,11 +61,16 @@ [Sources.EBC]
>  [Sources.IPF]
>    IoLibIpf.c
>  
> -[Sources.ARM]
> +[Sources.ARM, Sources.AARCH64]
>    IoLibArm.c
> +  IoLibArm.h
> +
> +[Sources.ARM]
> +  Arm/IoLibMmio.S   | GCC
> +  Arm/IoLibMmio.asm | RVCT
>  
>  [Sources.AARCH64]
> -  IoLibArm.c
> +  AArch64/IoLibMmio.S
>  
>  [Packages]
>    MdePkg/MdePkg.dec
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c b/MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c
> index 5ce12ca56ae2..449bc5ebbf85 100644
> --- a/MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c
> @@ -20,6 +20,7 @@
>  // Include common header file for this module.
>  //
>  #include "BaseIoLibIntrinsicInternal.h"
> +#include "IoLibArm.h"
>  
>  /**
>    Reads an 8-bit I/O port.
> @@ -411,10 +412,7 @@ MmioRead8 (
>    IN      UINTN                     Address
>    )
>  {
> -  UINT8                             Value;
> -
> -  Value = *(volatile UINT8*)Address;
> -  return Value;
> +  return MmioRead8Internal (Address);
>  }
>  
>  /**
> @@ -437,7 +435,7 @@ MmioWrite8 (
>    IN      UINT8                     Value
>    )
>  {
> -  *(volatile UINT8*)Address = Value;
> +  MmioWrite8Internal (Address, Value);
>    return Value;
>  }
>  
> @@ -461,11 +459,7 @@ MmioRead16 (
>    IN      UINTN                     Address
>    )
>  {
> -  UINT16                            Value;
> -
> -  ASSERT ((Address & 1) == 0);
> -  Value = *(volatile UINT16*)Address;
> -  return Value;
> +  return MmioRead16Internal (Address);
>  }
>  
>  /**
> @@ -489,7 +483,8 @@ MmioWrite16 (
>    )
>  {
>    ASSERT ((Address & 1) == 0);
> -  *(volatile UINT16*)Address = Value;
> +
> +  MmioWrite16Internal (Address, Value);
>    return Value;
>  }
>  
> @@ -513,11 +508,7 @@ MmioRead32 (
>    IN      UINTN                     Address
>    )
>  {
> -  UINT32                            Value;
> -
> -  ASSERT ((Address & 3) == 0);
> -  Value = *(volatile UINT32*)Address;
> -  return Value;
> +  return MmioRead32Internal (Address);
>  }
>  
>  /**
> @@ -541,7 +532,8 @@ MmioWrite32 (
>    )
>  {
>    ASSERT ((Address & 3) == 0);
> -  *(volatile UINT32*)Address = Value;
> +
> +  MmioWrite32Internal (Address, Value);
>    return Value;
>  }
>  
> @@ -565,11 +557,9 @@ MmioRead64 (
>    IN      UINTN                     Address
>    )
>  {
> -  UINT64                            Value;
> -
>    ASSERT ((Address & 7) == 0);
> -  Value = *(volatile UINT64*)Address;
> -  return Value;
> +
> +  return MmioRead64Internal (Address);
>  }
>  
>  /**
> @@ -593,7 +583,7 @@ MmioWrite64 (
>    )
>  {
>    ASSERT ((Address & 7) == 0);
> -  *(volatile UINT64*)Address = Value;
> +
> +  MmioWrite64Internal (Address, Value);
>    return Value;
>  }
> -
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.h b/MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.h
> new file mode 100644
> index 000000000000..6c63d0b5545b
> --- /dev/null
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.h
> @@ -0,0 +1,179 @@
> +/** @file
> +  Internal MMIO routines implemented in ARM assembler
> +
> +  Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution. The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php.
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#ifndef __BASEIOLIBINTRINSIC_IOLIBARM_H_
> +#define __BASEIOLIBINTRINSIC_IOLIBARM_H_
> +
> +/**
> +  Reads an 8-bit MMIO register.
> +
> +  Reads the 8-bit MMIO register specified by Address. The 8-bit read value is
> +  returned. This function must guarantee that all MMIO read and write
> +  operations are serialized.
> +
> +  If 8-bit MMIO register operations are not supported, then ASSERT().
> +
> +  @param  Address The MMIO register to read.
> +
> +  @return The value read.
> +
> +**/
> +UINT8
> +EFIAPI
> +MmioRead8Internal (
> +  IN      UINTN                     Address
> +  );
> +
> +/**
> +  Writes an 8-bit MMIO register.
> +
> +  Writes the 8-bit MMIO register specified by Address with the value specified
> +  by Value. This function must guarantee that all MMIO read and write operations
> +  are serialized.
> +
> +  If 8-bit MMIO register operations are not supported, then ASSERT().
> +
> +  @param  Address The MMIO register to write.
> +  @param  Value   The value to write to the MMIO register.
> +
> +**/
> +VOID
> +EFIAPI
> +MmioWrite8Internal (
> +  IN      UINTN                     Address,
> +  IN      UINT8                     Value
> +  );
> +
> +/**
> +  Reads a 16-bit MMIO register.
> +
> +  Reads the 16-bit MMIO register specified by Address. The 16-bit read value is
> +  returned. This function must guarantee that all MMIO read and write
> +  operations are serialized.
> +
> +  If 16-bit MMIO register operations are not supported, then ASSERT().
> +
> +  @param  Address The MMIO register to read.
> +
> +  @return The value read.
> +
> +**/
> +UINT16
> +EFIAPI
> +MmioRead16Internal (
> +  IN      UINTN                     Address
> +  );
> +
> +/**
> +  Writes a 16-bit MMIO register.
> +
> +  Writes the 16-bit MMIO register specified by Address with the value specified
> +  by Value. This function must guarantee that all MMIO read and write operations
> +  are serialized.
> +
> +  If 16-bit MMIO register operations are not supported, then ASSERT().
> +
> +  @param  Address The MMIO register to write.
> +  @param  Value   The value to write to the MMIO register.
> +
> +**/
> +VOID
> +EFIAPI
> +MmioWrite16Internal (
> +  IN      UINTN                     Address,
> +  IN      UINT16                    Value
> +  );
> +
> +/**
> +  Reads a 32-bit MMIO register.
> +
> +  Reads the 32-bit MMIO register specified by Address. The 32-bit read value is
> +  returned. This function must guarantee that all MMIO read and write
> +  operations are serialized.
> +
> +  If 32-bit MMIO register operations are not supported, then ASSERT().
> +
> +  @param  Address The MMIO register to read.
> +
> +  @return The value read.
> +
> +**/
> +UINT32
> +EFIAPI
> +MmioRead32Internal (
> +  IN      UINTN                     Address
> +  );
> +
> +/**
> +  Writes a 32-bit MMIO register.
> +
> +  Writes the 32-bit MMIO register specified by Address with the value specified
> +  by Value. This function must guarantee that all MMIO read and write operations
> +  are serialized.
> +
> +  If 32-bit MMIO register operations are not supported, then ASSERT().
> +
> +  @param  Address The MMIO register to write.
> +  @param  Value   The value to write to the MMIO register.
> +
> +**/
> +VOID
> +EFIAPI
> +MmioWrite32Internal (
> +  IN      UINTN                     Address,
> +  IN      UINT32                    Value
> +  );
> +
> +/**
> +  Reads a 64-bit MMIO register.
> +
> +  Reads the 64-bit MMIO register specified by Address. The 64-bit read value is
> +  returned. This function must guarantee that all MMIO read and write
> +  operations are serialized.
> +
> +  If 64-bit MMIO register operations are not supported, then ASSERT().
> +
> +  @param  Address The MMIO register to read.
> +
> +  @return The value read.
> +
> +**/
> +UINT64
> +EFIAPI
> +MmioRead64Internal (
> +  IN      UINTN                     Address
> +  );
> +
> +/**
> +  Writes a 64-bit MMIO register.
> +
> +  Writes the 64-bit MMIO register specified by Address with the value specified
> +  by Value. This function must guarantee that all MMIO read and write operations
> +  are serialized.
> +
> +  If 64-bit MMIO register operations are not supported, then ASSERT().
> +
> +  @param  Address The MMIO register to write.
> +  @param  Value   The value to write to the MMIO register.
> +
> +**/
> +VOID
> +EFIAPI
> +MmioWrite64Internal (
> +  IN      UINTN                     Address,
> +  IN      UINT64                    Value
> +  );
> +
> +#endif
>