public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Leif Lindholm <leif.lindholm@linaro.org>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	Laszlo Ersek <lersek@redhat.com>,
	 "Gao, Liming" <liming.gao@intel.com>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access
Date: Tue, 5 Jun 2018 12:48:26 +0200	[thread overview]
Message-ID: <CAKv+Gu_fnQNnTA20Lem=+y2+Uu_uvuKUpx6pBRfivjyuhcBtFA@mail.gmail.com> (raw)
In-Reply-To: <20180605083953.xp6brdmduypx7ble@bivouac.eciton.net>

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


  reply	other threads:[~2018-06-05 10:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2018-06-05  8:42 ` Laszlo Ersek
2018-06-05  8:55 ` Bhupesh Sharma

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKv+Gu_fnQNnTA20Lem=+y2+Uu_uvuKUpx6pBRfivjyuhcBtFA@mail.gmail.com' \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox