Re: [PATCH kvmtool v4 0/5] Add CFI flash emulation

["André Przywara" <andre.przywara@arm.com>]

On 24/04/2020 07:45, Ard Biesheuvel wrote:

Hi,

(adding Leif for EDK-2 discussion)

> On Thu, 23 Apr 2020 at 23:32, André Przywara <andre.przywara@arm.com> wrote:
>>
>> On 23/04/2020 21:43, Ard Biesheuvel wrote:

[ ... kvmtool series to add CFI flash emulation allowing EDK-2 to store
variables. Starting with this version (v4) the flash memory region is
presented as a read-only memslot to KVM, to allow direct guest accesses
as opposed to trap-and-emulate even read accesses to the array.]

>>
>>
>> Just curious: the images Sami gave me this morning did not show any
>> issues anymore (no no-syndrome fault, no alignment issues), even without
>> the mapping [1]. And even though I saw the 800k read traps, I didn't
>> notice any real performance difference (on a Juno). The PXE timeout was
>> definitely much more noticeable.
>>
>> So did you see any performance impact with this series?
>>
> 
> You normally don't PXE boot. There is an issue with the iSCSI driver
> as well, which causes a boot delay for some reason, so I disabled that
> in my build.
> 
> I definitely *feels* faster :-) But in any case, exposing the array
> mode as a r/o memslot is definitely the right way to deal with this.
> Even if Sami did find a workaround that masks the error, it is no
> guarantee that all accesses go through that library.

So I was wondering about this, maybe you can confirm or debunk this:
- Any memory given to the compiler (through a pointer) is assumed to be
"normal" memory: the compiler can re-arrange accesses, split them up or
collate them. Also unaligned accesses should be allowed - although I
guess most compilers would avoid them.
- This normally forbids to give a pointer to memory mapped as "device
memory" to the compiler, since this would violate all of the assumptions
above.
- If the device mapped as "device memory" is actually memory (SRAM,
ROM/flash, framebuffer), then most of the assumptions are met, except
the alignment requirement, which is bound to the mapping type, not the
actual device (ARMv8 ARM: Unaligned accesses to device memory always
trap, regardless of SCTLR.A)
- To accommodate the latter, GCC knows the option -malign-strict, to
avoid unaligned accesses. TF-A and U-Boot use this option, to run
without the MMU enabled.

Now if EDK-2 lets the compiler deal with the flash memory region
directly, I think this would still be prone to alignment faults. In fact
an earlier build I got from Sami faulted on exactly that, when I ran it,
even with the r/o memslot mapping in place.

So should EDK-2 add -malign-strict to be safe?
	or
Should EDK-2 add an additional or alternate mapping using a non-device
memory type (with all the mismatched attributes consequences)?
	or
Should EDK-2 only touch the flash region using MMIO accessors, and
forbid the compiler direct access to that region?

So does EDK-2 get away with this because the compiler typically avoids
unaligned accesses?

Cheers,
Andre

>>> [0] https://people.linaro.org/~ard.biesheuvel/KVMTOOL_EFI.fd
>>
>> Ah, nice, will this stay there for a while? I can't provide binaries,
>> but wanted others to be able to easily test this.
>>
> 
> Sure, I will leave it up until Linaro decides to take down my account.
> 

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm