On Mon, Sep 23, 2019 at 08:31:38AM +0800, Abner Chang wrote: > Support RISC-V cache related functions. > > Signed-off-by: Abner Chang <abner.ch...@hpe.com>
What is the purpose of the .c file? Currently all I see it doing it printing some messages before calling into assembly, and forcing Hungarian notation on the filenames. There is no value in providing runtime notifications that synchronization is not required - this is a library, we wil learn at build time if our code is impacted by lack of some primitive. Can you drop the .c file, rename the .S file Synchronization.S, and renaming the functions in the .S: InternalSyncCompareExchange32 InternalSyncCompareExchange64 InternalSyncIncrement InternalSyncDecrement U500 still builds fine after this change. / Leif > --- > .../BaseSynchronizationLib.inf | 6 + > .../RiscV64/Synchronization.c | 183 > +++++++++++++++++++++ > .../RiscV64/SynchronizationAsm.S | 78 +++++++++ > 3 files changed, 267 insertions(+) > create mode 100644 > MdePkg/Library/BaseSynchronizationLib/RiscV64/Synchronization.c > create mode 100644 > MdePkg/Library/BaseSynchronizationLib/RiscV64/SynchronizationAsm.S > > diff --git a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > index 446bc19..c16ef9d 100755 > --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > @@ -3,6 +3,7 @@ > # > # Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR> > # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR> > +# Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development LP. All > rights reserved.<BR> > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -78,6 +79,11 @@ > AArch64/Synchronization.S | GCC > AArch64/Synchronization.asm | MSFT > > +[Sources.RISCV64] > + Synchronization.c > + RiscV64/Synchronization.c | GCC > + RiscV64/SynchronizationAsm.S > + > [Packages] > MdePkg/MdePkg.dec > > diff --git a/MdePkg/Library/BaseSynchronizationLib/RiscV64/Synchronization.c > b/MdePkg/Library/BaseSynchronizationLib/RiscV64/Synchronization.c > new file mode 100644 > index 0000000..e210b74 > --- /dev/null > +++ b/MdePkg/Library/BaseSynchronizationLib/RiscV64/Synchronization.c > @@ -0,0 +1,183 @@ > +/** @file > + Implementation of synchronization functions on RISC-V > + > + Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development LP. All > rights reserved.<BR> > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > +**/ > + > +#include <Library/DebugLib.h> > + > +UINT32 > +SyncCompareExchange32 ( > + IN volatile UINT32 *Value, > + IN UINT32 CompareValue, > + IN UINT32 ExchangeValue > +); > + > +UINT64 > +SyncCompareExchange64 ( > + IN volatile UINT64 *Value, > + IN UINT64 CompareValue, > + IN UINT64 ExchangeValue > +); > + > +UINT32 > +SyncSyncIncrement32 ( > + IN volatile UINT32 *Value > + ); > + > +UINT32 > +SyncSyncDecrement32 ( > + IN volatile UINT32 *Value > + ); > + > +/** > + Performs an atomic compare exchange operation on a 16-bit > + unsigned integer. > + > + Performs an atomic compare exchange operation on the 16-bit > + unsigned integer specified by Value. If Value is equal to > + CompareValue, then Value is set to ExchangeValue and > + CompareValue is returned. If Value is not equal to > + CompareValue, then Value is returned. The compare exchange > + operation must be performed using MP safe mechanisms. > + > + @param Value A pointer to the 16-bit value for the > + compare exchange operation. > + @param CompareValue 16-bit value used in compare operation. > + @param ExchangeValue 16-bit value used in exchange operation. > + > + @return The original *Value before exchange. > + > +**/ > +UINT16 > +EFIAPI > +InternalSyncCompareExchange16 ( > + IN volatile UINT16 *Value, > + IN UINT16 CompareValue, > + IN UINT16 ExchangeValue > + ) > +{ > + DEBUG((DEBUG_ERROR, "%a:RISC-V does not support 16-bit AMO operation\n", > __FUNCTION__)); > + ASSERT (FALSE); > + return 0; > +} > + > +/** > + Performs an atomic compare exchange operation on a 32-bit > + unsigned integer. > + > + Performs an atomic compare exchange operation on the 32-bit > + unsigned integer specified by Value. If Value is equal to > + CompareValue, then Value is set to ExchangeValue and > + CompareValue is returned. If Value is not equal to > + CompareValue, then Value is returned. The compare exchange > + operation must be performed using MP safe mechanisms. > + > + @param Value A pointer to the 32-bit value for the > + compare exchange operation. > + @param CompareValue 32-bit value used in compare operation. > + @param ExchangeValue 32-bit value used in exchange operation. > + > + @return The original *Value before exchange. > + > +**/ > +UINT32 > +EFIAPI > +InternalSyncCompareExchange32 ( > + IN volatile UINT32 *Value, > + IN UINT32 CompareValue, > + IN UINT32 ExchangeValue > + ) > +{ > + > + if (((UINTN)Value % sizeof (UINT32)) != 0) { > + DEBUG((DEBUG_ERROR, "%a:Value pointer must aligned at natural > address.\n", __FUNCTION__)); > + ASSERT (FALSE); > + } > + return SyncCompareExchange32(Value, CompareValue, ExchangeValue); > +} > + > +/** > + Performs an atomic compare exchange operation on a 64-bit unsigned integer. > + > + Performs an atomic compare exchange operation on the 64-bit unsigned > integer specified > + by Value. If Value is equal to CompareValue, then Value is set to > ExchangeValue and > + CompareValue is returned. If Value is not equal to CompareValue, then > Value is returned. > + The compare exchange operation must be performed using MP safe mechanisms. > + > + @param Value A pointer to the 64-bit value for the compare > exchange > + operation. > + @param CompareValue 64-bit value used in compare operation. > + @param ExchangeValue 64-bit value used in exchange operation. > + > + @return The original *Value before exchange. > + > +**/ > +UINT64 > +EFIAPI > +InternalSyncCompareExchange64 ( > + IN volatile UINT64 *Value, > + IN UINT64 CompareValue, > + IN UINT64 ExchangeValue > + ) > +{ > + if (((UINTN)Value % sizeof (UINT64)) != 0) { > + DEBUG((DEBUG_ERROR, "%a:Value pointer must aligned at natural > address.\n", __FUNCTION__)); > + ASSERT (FALSE); > + } > + return SyncCompareExchange64 (Value, CompareValue, ExchangeValue); > +} > + > +/** > + Performs an atomic increment of an 32-bit unsigned integer. > + > + Performs an atomic increment of the 32-bit unsigned integer specified by > + Value and returns the incremented value. The increment operation must be > + performed using MP safe mechanisms. The state of the return value is not > + guaranteed to be MP safe. > + > + @param Value A pointer to the 32-bit value to increment. > + > + @return The incremented value. > + > +**/ > +UINT32 > +EFIAPI > +InternalSyncIncrement ( > + IN volatile UINT32 *Value > + ) > +{ > + if (((UINTN)Value % sizeof (UINT32)) != 0) { > + DEBUG((DEBUG_ERROR, "%a:Value pointer must aligned at natural > address.\n", __FUNCTION__)); > + ASSERT (FALSE); > + } > + return SyncSyncIncrement32 (Value); > +} > + > +/** > + Performs an atomic decrement of an 32-bit unsigned integer. > + > + Performs an atomic decrement of the 32-bit unsigned integer specified by > + Value and returns the decrement value. The decrement operation must be > + performed using MP safe mechanisms. The state of the return value is not > + guaranteed to be MP safe. > + > + @param Value A pointer to the 32-bit value to decrement. > + > + @return The decrement value. > + > +**/ > +UINT32 > +EFIAPI > +InternalSyncDecrement ( > + IN volatile UINT32 *Value > + ) > +{ > + if (((UINTN)Value % sizeof (UINT32)) != 0) { > + DEBUG((DEBUG_ERROR, "%a:Value pointer must aligned at natural > address.\n", __FUNCTION__)); > + ASSERT (FALSE); > + } > + return SyncSyncDecrement32 (Value); > +} > diff --git > a/MdePkg/Library/BaseSynchronizationLib/RiscV64/SynchronizationAsm.S > b/MdePkg/Library/BaseSynchronizationLib/RiscV64/SynchronizationAsm.S > new file mode 100644 > index 0000000..943e274 > --- /dev/null > +++ b/MdePkg/Library/BaseSynchronizationLib/RiscV64/SynchronizationAsm.S > @@ -0,0 +1,78 @@ > +//------------------------------------------------------------------------------ > +// > +// RISC-V synchronization functions. > +// > +// Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development LP. All > rights reserved.<BR> > +// > +// SPDX-License-Identifier: BSD-2-Clause-Patent > +// > +//------------------------------------------------------------------------------ > +#include <Base.h> > + > +.data > + > +.text > +.align 3 > + > +.global ASM_PFX(SyncCompareExchange32) > +.global ASM_PFX(SyncCompareExchange64) > +.global ASM_PFX(SyncSyncIncrement32) > +.global ASM_PFX(SyncSyncDecrement32) > + > +// > +// ompare and xchange a 32-bit value. > +// > +// @param a0 : Pointer to 32-bit value. > +// @param a1 : Compare value. > +// @param a2 : Exchange value. > +// > +ASM_PFX (SyncCompareExchange32): > + lr.w a3, (a0) // Load the value from a0 and make > + // the reservation of address. > + bne a3, a1, exit > + sc.w a3, a2, (a0) // Write the value back to the address. > + mv a3, a1 > +exit: > + mv a0, a3 > + ret > + > +.global ASM_PFX(SyncCompareExchange64) > + > +// > +// Compare and xchange a 64-bit value. > +// > +// @param a0 : Pointer to 64-bit value. > +// @param a1 : Compare value. > +// @param a2 : Exchange value. > +// > +ASM_PFX (SyncCompareExchange64): > + lr.d a3, (a0) // Load the value from a0 and make > + // the reservation of address. > + bne a3, a1, exit > + sc.d a3, a2, (a0) // Write the value back to the address. > + mv a3, a1 > +exit2: > + mv a0, a3 > + ret > + > +// > +// Performs an atomic increment of an 32-bit unsigned integer. > +// > +// @param a0 : Pointer to 32-bit value. > +// > +ASM_PFX (SyncSyncIncrement32): > + li a1, 1 > + amoadd.w a2, a1, (a0) > + mv a0, a2 > + ret > + > +// > +// Performs an atomic decrement of an 32-bit unsigned integer. > +// > +// @param a0 : Pointer to 32-bit value. > +// > +ASM_PFX (SyncSyncDecrement32): > + li a1, -1 > + amoadd.w a2, a1, (a0) > + mv a0, a2 > + ret > -- > 2.7.4 > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48159): https://edk2.groups.io/g/devel/message/48159 Mute This Topic: https://groups.io/mt/34258207/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-