Re: [PATCH] MdePkg/BaseIoLibIntrinsicArmVirt ARM: avoid double word loads and stores

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 7/11/18 14:38, Ard Biesheuvel wrote:
> On 7 November 2018 at 14:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> BaseIoLibIntrinsicArmVirt was created to prevent LTO from merging
>> accesses to MMIO regions, resulting in instructions with multiple
>> output registers that KVM on ARM cannot emulate (since the exception
>> syndrome information that KVM relies on can only describe a single
>> output register)
>>
>> However, using double word loads on ARM amounts to the same thing,
>> and so code that relies on doing 64-bit MMIO to regions that are
>> emulated under KVM (such as the GICv3 TYPER register) will still
>> suffer from the original issue.
>>
>> So replace ldrd and strd with equivalent two instruction sequences.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> Please consider this patch with the hunk below appended
> 
>> ---
>>   MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.S | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.S b/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.S
>> index 3ad22bd5706d..0d802d6928d6 100644
>> --- a/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.S
>> +++ b/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.S
>> @@ -125,7 +125,8 @@ ASM_PFX(MmioWrite32Internal):
>>   //  @return The value read.
>>   //
>>   ASM_PFX(MmioRead64Internal):
>> -  ldrd    r0, r1, [r0]
>> +  ldr     r1, [r0, #4]
>> +  ldr     r0, [r0]

Ard remembered me UEFI is little-endian only :)
Thus this code is safe.

With the hunk appended:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>>     dmb
>>     bx      lr
>>
>> @@ -141,5 +142,6 @@ ASM_PFX(MmioRead64Internal):
>>   //
>>   ASM_PFX(MmioWrite64Internal):
>>     dmb     st
>> -  strd    r2, r3, [r0]
>> +  str     r2, [r0]
>> +  str     r3, [r0, #4]
>>     bx      lr
> 
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm
> b/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm
> index e1a3d68a430c..deba8c1f0c59 100644
> --- a/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm
> @@ -127,7 +127,8 @@ MmioWrite32Internal
>   ;  @return The value read.
>   ;
>   MmioRead64Internal
> -  ldrd    r0, r1, [r0]
> +  ldr     r1, [r0, #4]
> +  ldr     r0, [r0]
>     dmb
>     bx      lr
> 
> @@ -143,7 +144,8 @@ MmioRead64Internal
>   ;
>   MmioWrite64Internal
>     dmb     st
> -  strd    r2, r3, [r0]
> +  str     r2, [r0]
> +  str     r3, [r0, #4]
>     bx      lr
> 
>     END
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>