Re: [PATCH 01/26] ArmLib: remove ArmReplaceLiveTranslationEntry() implementation

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 10 August 2016 at 18:56, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, Aug 10, 2016 at 05:17:37PM +0200, Ard Biesheuvel wrote:
>> The function ArmReplaceLiveTranslationEntry() has been moved to
>> ArmMmuLib, so remove the old implementation from ArmLib.
>
> Could you add a statement to this commit message that you're also
> fixing up the export attributes for the ArmMmuLib version to prepare
> for the new macros?
>

Actually, it is not quite that. The ArmMmuLib implementation was never
actually accessible, so the ArmLib version was still being used. That
is why it needs to be fixed in the same patch, or we'll end up with
either two or no implementations, causing breakage. I will add some
more explanation to the commit log

> If you do:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S           | 60 --------------------
>>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S |  4 ++
>>  2 files changed, 4 insertions(+), 60 deletions(-)
>>
>> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
>> index 9441f47e30ba..5cef98fd42a0 100644
>> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
>> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
>> @@ -488,64 +488,4 @@ ASM_PFX(ArmReadCurrentEL):
>>    mrs   x0, CurrentEL
>>    ret
>>
>> -
>> -  .macro __replace_entry, el
>> -
>> -  // disable the MMU
>> -  mrs   x8, sctlr_el\el
>> -  bic   x9, x8, #CTRL_M_BIT
>> -  msr   sctlr_el\el, x9
>> -  isb
>> -
>> -  // write updated entry
>> -  str   x1, [x0]
>> -
>> -  // invalidate again to get rid of stale clean cachelines that may
>> -  // have been filled speculatively since the last invalidate
>> -  dmb   sy
>> -  dc    ivac, x0
>> -
>> -  // flush the TLBs
>> -  .if   \el == 1
>> -  tlbi  vmalle1
>> -  .else
>> -  tlbi  alle\el
>> -  .endif
>> -  dsb   sy
>> -
>> -  // re-enable the MMU
>> -  msr   sctlr_el\el, x8
>> -  isb
>> -  .endm
>> -
>> -//VOID
>> -//ArmReplaceLiveTranslationEntry (
>> -//  IN  UINT64  *Entry,
>> -//  IN  UINT64  Value
>> -//  )
>> -ASM_PFX(ArmReplaceLiveTranslationEntry):
>> -
>> -  // disable interrupts
>> -  mrs   x2, daif
>> -  msr   daifset, #0xf
>> -  isb
>> -
>> -  // clean and invalidate first so that we don't clobber
>> -  // adjacent entries that are dirty in the caches
>> -  dc    civac, x0
>> -  dsb   ish
>> -
>> -  EL1_OR_EL2_OR_EL3(x3)
>> -1:__replace_entry 1
>> -  b     4f
>> -2:__replace_entry 2
>> -  b     4f
>> -3:__replace_entry 3
>> -
>> -4:msr   daif, x2
>> -  ret
>> -
>> -ASM_PFX(ArmReplaceLiveTranslationEntrySize):
>> -  .long   . - ArmReplaceLiveTranslationEntry
>> -
>>  ASM_FUNCTION_REMOVE_IF_UNREFERENCED
>> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
>> index 7c5d205d940b..3834da7bfedd 100644
>> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
>> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
>> @@ -14,6 +14,8 @@
>>
>>  #include <AsmMacroIoLibV8.h>
>>
>> +GCC_ASM_EXPORT(ArmReplaceLiveTranslationEntry)
>> +
>>    .set CTRL_M_BIT,      (1 << 0)
>>
>>    .macro __replace_entry, el
>> @@ -72,5 +74,7 @@ ASM_PFX(ArmReplaceLiveTranslationEntry):
>>  4:msr   daif, x2
>>    ret
>>
>> +ASM_GLOBAL ASM_PFX(ArmReplaceLiveTranslationEntrySize)
>> +
>>  ASM_PFX(ArmReplaceLiveTranslationEntrySize):
>>    .long   . - ArmReplaceLiveTranslationEntry
>> --
>> 2.7.4
>>