public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access
@ 2018-06-05  7:30 Ard Biesheuvel
  2018-06-05  8:39 ` Leif Lindholm
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2018-06-05  7:30 UTC (permalink / raw)
  To: edk2-devel
  Cc: lersek, leif.lindholm, liming.gao, michael.d.kinney,
	Ard Biesheuvel

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.

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.

 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
-- 
2.17.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access
  2018-06-05  7:30 [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access Ard Biesheuvel
@ 2018-06-05  8:39 ` Leif Lindholm
  2018-06-05 10:48   ` Ard Biesheuvel
  2018-06-05  8:42 ` Laszlo Ersek
  2018-06-05  8:55 ` Bhupesh Sharma
  2 siblings, 1 reply; 5+ messages in thread
From: Leif Lindholm @ 2018-06-05  8:39 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, lersek, liming.gao, michael.d.kinney

On Tue, Jun 05, 2018 at 09:30:04AM +0200, 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.
> 
> 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.

I think there are arguments for and against this solution instead of
disabling LTO.

My main argument against this version is that it makes a
re-unification of the (needlessly) different pure C implementations
more tedious/questionable.

It _is_ possible that the introduction of LTO makes the described
semantics of the functions currently incorrect for ARM*, at which
point the above reservation is irrelevant.

However, the DSBs seem like a bit of a sledge hammer - all that's
being mandated is serialization? Why would DMB not suffice?
And if we make that change ... I would sort of prefer to see the
barriers added as a separate patch, with a clear commit message -
because that is not just part of the refactoring.

/
    Leif

>  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
> -- 
> 2.17.0
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access
  2018-06-05  7:30 [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access Ard Biesheuvel
  2018-06-05  8:39 ` Leif Lindholm
@ 2018-06-05  8:42 ` Laszlo Ersek
  2018-06-05  8:55 ` Bhupesh Sharma
  2 siblings, 0 replies; 5+ messages in thread
From: Laszlo Ersek @ 2018-06-05  8:42 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel; +Cc: leif.lindholm, liming.gao, michael.d.kinney

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
> 



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access
  2018-06-05  7:30 [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access Ard Biesheuvel
  2018-06-05  8:39 ` Leif Lindholm
  2018-06-05  8:42 ` Laszlo Ersek
@ 2018-06-05  8:55 ` Bhupesh Sharma
  2 siblings, 0 replies; 5+ messages in thread
From: Bhupesh Sharma @ 2018-06-05  8:55 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel, michael.d.kinney, Laszlo Ersek, liming.gao,
	Leif Lindholm

On Tue, Jun 5, 2018 at 1:00 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> 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.
>
> 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.
>
>  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

I am generally worried about sprinkling of the 'dsb' that the patch
here (and below) does, without adding comments.

Please note that with current (and possible future) ARM core erratas
and SoC specific erratas (from Si vendors) assumptions about the
serialization sequence can be tricky at times in terms of MMIO
accesses and dsb usages.

I would suggest that we document the 'dsb' usage here and _warn_ users
to add to/fix the sequence in case they are working on a 'broken' Si
with known erratas (which they normally get around with by fixing and
using inhouse versions of gcc which fix the sequencing issue for them
while generating the relevant code).

Regards,
Bhupesh


> +  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
> --
> 2.17.0
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access
  2018-06-05  8:39 ` Leif Lindholm
@ 2018-06-05 10:48   ` Ard Biesheuvel
  0 siblings, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2018-06-05 10:48 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: edk2-devel@lists.01.org, Laszlo Ersek, Gao, Liming,
	Kinney, Michael D

On 5 June 2018 at 10:39, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Jun 05, 2018 at 09:30:04AM +0200, 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.
>>
>> 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.
>
> I think there are arguments for and against this solution instead of
> disabling LTO.
>
> My main argument against this version is that it makes a
> re-unification of the (needlessly) different pure C implementations
> more tedious/questionable.
>
> It _is_ possible that the introduction of LTO makes the described
> semantics of the functions currently incorrect for ARM*, at which
> point the above reservation is irrelevant.
>
> However, the DSBs seem like a bit of a sledge hammer - all that's
> being mandated is serialization? Why would DMB not suffice?
> And if we make that change ... I would sort of prefer to see the
> barriers added as a separate patch, with a clear commit message -
> because that is not just part of the refactoring.
>

Points taken.

Perhaps it is more appropriate to create a special ArmVirtPkg IoLib
implementation for the time being, and defer changes like this one
until we have more data points to consider.


>>  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
>> --
>> 2.17.0
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-06-05 10:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-05  7:30 [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access Ard Biesheuvel
2018-06-05  8:39 ` Leif Lindholm
2018-06-05 10:48   ` Ard Biesheuvel
2018-06-05  8:42 ` Laszlo Ersek
2018-06-05  8:55 ` Bhupesh Sharma

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox