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

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

[Ard Biesheuvel]

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
+  }
-- 
2.17.0

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

[Laszlo Ersek]

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.)

- Is this only with gcc-8?

- 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.)

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

Thanks!
Laszlo

[1]
https://edk2-docs.gitbooks.io/edk-ii-dsc-specification/content/2_dsc_overview/24_[buildoptions]_section.html#table-8-edk-ii-buildoptions-variable-descriptions

[2]
https://edk2-docs.gitbooks.io/edk-ii-dsc-specification/content/2_dsc_overview/211_[components]_section_processing.html#211-components-section-processing

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

[Ard Biesheuvel]

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,

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

[Laszlo Ersek]

On 06/04/18 17:30, Ard Biesheuvel wrote:
> 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.

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 <lersek@redhat.com>

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,
>