Re: [PATCH v1 03/11] MdePkg: Base Memory Lib instance using MMIO

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 5/14/20 11:22 AM, Ard Biesheuvel wrote:
> Hi Sami,
> 
> On 5/14/20 10:40 AM, Sami Mujawar wrote:
>> Some device drivers perform copy operations on
>> device memory, e.g. device drivers for a Flash
>> device. On some architectures unaligned access
>> to device memory regions is not permitted. To
>> add to this if the device is virtualised then
>> there are further restrictions on the type of
>> load/store operations that can be performed
>> on the device memory regions, e.g. on AARCH64,
>> Pre/Post index or LDP operations cannot be
>> used, as a trap to EL2 does not provide the
>> syndrome information to the hypervisor.
>>
> 
> We are conflating two different things here:
> - the use of unaligned accesses to read from device memory
> - the use of load/store instructions that have multiple output 
> registers, making it very difficult to emulate them in a virtual machine 
> context.
> 
> I though that for the kvmtool port, we addressed the second issue by 
> using a read-only memslot for the NOR flash when it is in array mode? 
> IIRC, the issue only affects reads, as all writes to NOR flash device 
> memory are already done using the proper MMIO accessors. If those MMIO 
> accessors compile to something that does not work under emulation, you 
> should be use the special IoLib implementation that exists for this use 
> case:
> 
>    MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
> 
> So that leaves the issue of unaligned accesses to device memory. This 
> should not happen unless you are using BaseMemoryLibOptDxe for AARCH64, 
> which deliberately uses overlapping loads and stores for performance. 
> Any BASE library is built with -mstrict-align, and so any other C code 
> implementation of BaseMemoryLib should work fine for reading for a 
> read-only memslot mapped using device attributes.
> 
> So in summary, I don't think we need this library.
> 

BTW I think the AlignedCopyMem() introduction predates the changes to 
use -mstrict-align for BASE libraries, and to use the ordinary 
BaseMemoryLib implementation for DXE_RUNTIME_DRIVER modules on AArch64.