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@xxxxxxx> 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. >
