On 06/04/18 17:30, Ard Biesheuvel wrote: > On 4 June 2018 at 17:18, Laszlo Ersek <[email protected]> wrote: >> On 06/04/18 16:50, Ard Biesheuvel wrote: >>> KVM on ARM refuses to decode load/store instructions used to perform >>> I/O to emulated devices, and instead relies on the exception syndrome >>> information to describe the operand register, access size, etc. >>> This is only possible for instructions that have a single input/output >>> register (as opposed to ones that increment the offset register, or >>> load/store pair instructions, etc). Otherwise, QEMU crashes with the >>> following error >>> >>> error: kvm run failed Function not implemented >>> R00=01010101 R01=00000008 R02=00000048 R03=08000820 >>> R04=00000120 R05=7faaa0e0 R06=7faaa0dc R07=7faaa0e8 >>> R08=7faaa0ec R09=7faaa088 R10=000000ff R11=00000080 >>> R12=ff000000 R13=7fccfe08 R14=7faa835f R15=7faa887c >>> PSR=800001f3 N--- T svc32 >>> QEMU: Terminated >>> >>> and KVM produces a warning such as the following in the kernel log >>> >>> kvm [17646]: load/store instruction decoding not implemented >>> >>> GCC with LTO enabled will emit such instructions for Mmio[Read|Write] >>> invocations performed in a loop, so we need to disable LTO for the >>> IoLib library to ensure that the emitted instructions are suitable for >>> emulated I/O under KVM >>> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Ard Biesheuvel <[email protected]> >>> --- >>> ArmVirtPkg/ArmVirtQemu.dsc | 18 ++++++++++++++++++ >>> 1 file changed, 18 insertions(+) >>> >>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc >>> index d74feb709cd1..e6e3d82d6ca9 100644 >>> --- a/ArmVirtPkg/ArmVirtQemu.dsc >>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc >>> @@ -414,3 +414,21 @@ [Components.AARCH64] >>> <LibraryClasses> >>> NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf >>> } >>> + >>> +[Components.ARM] >>> + MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf { >>> + <BuildOptions> >>> + // >>> + // KVM on ARM refuses to decode load/store instructions used to >>> perform >>> + // I/O to emulated devices, and instead relies on the exception >>> syndrome >>> + // information to describe the operand register, access size, etc. >>> + // This is only possible for instructions that have a single >>> input/output >>> + // register (as opposed to ones that increment the offset register, >>> or >>> + // load/store pair instructions, etc). >>> + // GCC with LTO enabled will emit such instructions for >>> Mmio[Read|Write] >>> + // invocations performed in a loop, so we need to disable LTO for >>> this >>> + // library to ensure that the emitted instructions are suitable for >>> + // emulated I/O under KVM >>> + // >>> + GCC:*_GCC5_ARM_CC_FLAGS = -fno-lto >>> + } >>> >> >> Heh :) See <https://bugzilla.redhat.com/show_bug.cgi?id=1576593>. >> >> - Is there perhaps a finer-grained GCC option for this? (This is a >> rhetorical question; I know you must have checked.) >> > > Unfortunately not. That is why this is a workaround rather than a fix. > >> - Is this only with gcc-8? >> > > I don't think so. Any GCC/GNU-ld combo that supports LTO is > susceptible to this afaik. Even worse, I don't think this is limited > to 32-bit ARM either, even if we've never managed to hit it. > >> - Should we do the same for the ArmVirtXen and ArmVirtQemuKernel >> platforms? In turn, patch "ArmVirt.dsc.inc" instead? (BTW I have no clue >> about Xen's emulation of the instructions at hand.) >> > > This is a fix I would like to apply with moderation, so that we get a > feeling for which platforms need it and which don't. > > ArmVirtQemuKernel is primarily used in TCG mode, as far as I am aware, > by the OP-TEE guys for instance, who use secure world emulation. > > Xen doesn't really use I/O emulation on ARM (everything is > paravirtualized) so I don't think it is affected.
Can you please work some of the above answers (as you see fit) into the commit message, when you push the patch? Reviewed-by: Laszlo Ersek <[email protected]> Thanks! Laszlo >> In general, I'm fine with the patch. According to [1] [2], this appears >> to be the right syntax for the goal. >> > > Thanks, > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
