Re: [PATCH] ArmVirtPkg/ArmVirtQemu ARM: work around KVM limitations in LTO build

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 4 June 2018 at 17:18, Laszlo Ersek <lersek@redhat.com> 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 <ard.biesheuvel@linaro.org>
>> ---
>>  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.

> In general, I'm fine with the patch. According to [1] [2], this appears
> to be the right syntax for the goal.
>

Thanks,