Re: [PATCH 0/4] Fix runtime issue in XenBusDxe when compiled with GCC5

[Laszlo Ersek <lersek@redhat.com>]

On 12/01/16 16:28, Anthony PERARD wrote:
> Hi,
> 
> That might be only with the Xen part of OVMF but now that the GCC5
> toolchains is used with my gcc (6.2.1 20160830, Arch Linux), OVMF fail
> to boot in Xen guests.
> 
> Here is the result:
> !!!! X64 Exception Type - 06(#UD - Invalid Opcode)  CPU Apic ID - 00000000 !!!!
> RIP  - 000000001F26AF6B, CS  - 0000000000000038, RFLAGS - 0000000000010202
> RAX  - 0000000000000001, RCX - 000000001F26AF51, RDX - 0000000000000004
> RBX  - 0000000000000000, RSP - 000000001F43C510, RBP - 000000001E583D18
> RSI  - 0000000000000003, RDI - 0000000000000001
> R8   - 0000000000000000, R9  - 0000000000000000, R10 - 000000001E58DB98
> R11  - 0000000000000002, R12 - 000000001E58D898, R13 - 0000000000000000
> R14  - 000000001E58D8A0, R15 - 000000001F26D001
> DS   - 0000000000000030, ES  - 0000000000000030, FS  - 0000000000000030
> GS   - 0000000000000030, SS  - 0000000000000030
> CR0  - 0000000080000033, CR2 - 0000000000000000, CR3 - 000000001F3DB000
> CR4  - 0000000000000668, CR8 - 0000000000000000
> DR0  - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
> DR3  - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
> GDTR - 000000001F3C9A98 0000000000000047, LDTR - 0000000000000000
> IDTR - 000000001EB0A018 0000000000000FFF,   TR - 0000000000000000
> FXSAVE_STATE - 000000001F43C170
> !!!! Find PE image ./Build/OvmfX64/DEBUG_GCC5/X64/OvmfPkg/XenBusDxe/XenBusDxe/DEBUG/XenBusDxe.dll (ImageBase=000000001F266000, EntryPoint=000000001F2669D5) !!!!
> 
> Removing the gcc option -flto in only the XenBusDxe module makes OVMF
> boot.
> 
> While trying to debug that, I've added some debug prints (in this module
> and in XenPvBlkDxe), and the exception could change and become a "page
> fault" instead, or even an assert failure in the PrintLib, that was the
> ASSERT(Buffer != NULL) at I think
> MdePkg/Library/BasePrintLib/PrintLibInternal.c:366
> 
> Adding EFIAPI to internal functions in XenBusDxe makes things work
> again.  My guest is that gcc would bypass (optimise) an exported
> functions and call directly an internal one but without reordering the
> arguments (EFIAPI vs nothing).
> 
> Does that make sense?

Thank you for the investigation. It is strange that only the Xen modules
are affected, I'm unsure what that's the case.

Either way, it seems to be a gcc-6 bug, or an edk2 toolchain bug. You
should *not* need EFIAPI for functions with external linkage if the
calls to them straddle only files in the same module. I'm suspecting
gcc-6 (we've received no such reports with gcc-5). Maybe we need a GCC6
toolchain as well, for turning off some new features in gcc-6?

Jordan, Liming, Yonghong, Ard -- any ideas?

Anthony: while we all figure this out, please consider building OVMF
with the "-b NOOPT" switch. Support for the NOOPT build target has
recently been added to the GCC Ia32/X64 toolchains in BaseTools, and to
the OVMF DSC files as well. The build targets correspond to:

RELEASE -- compiler optimization enabled; DEBUG, ASSERT, and similar
           DebugLib features compiled out
DEBUG   -- compiler optimization enabled; DebugLib features preserved
NOOPT   -- compiler optimization disabled; DebugLib features preserved

(Note that for ArmVirtPkg and the GCC AARCH64 toolchains in BaseTools,
there is no NOOPT, and DEBUG means actually NOOPT -- if I remember
correctly. Ard will correct me if I'm wrong :))

If "-b NOOPT" works for you, I'd prefer that as a temporary solution
(until the root cause is found and addressed) to the XenBusDxe patches.

Hrpmf, wait a second, I do see something interesting: in this series you
*are* modifying APIs declared in a library class header (namely
"OvmfPkg/Include/Library/XenHypercallLib.h"). Such functions (public
libraries) *are* required to specify EFIAPI.

What happens if you apply patch #1 only?

Thanks!
Laszlo

> Anthony PERARD (4):
>   OvmfPkg/XenHypercallLib: Add EFIAPI
>   OvmfPkg/XenBusDxe: Add EFIAPI to XenEventChannelNotify
>   OvmfPkg/XenBusDxe: Add EFIAPI to XenStore functions
>   OvmfPkg/XenBusDxe: Add EFIAPI to XenGrantTable{Grant,End}Access
> 
>  OvmfPkg/Include/Library/XenHypercallLib.h      |  3 +++
>  OvmfPkg/Library/XenHypercallLib/XenHypercall.c |  3 +++
>  OvmfPkg/XenBusDxe/EventChannel.c               |  1 +
>  OvmfPkg/XenBusDxe/EventChannel.h               |  1 +
>  OvmfPkg/XenBusDxe/GrantTable.c                 |  2 ++
>  OvmfPkg/XenBusDxe/XenStore.c                   | 13 +++++++++++++
>  OvmfPkg/XenBusDxe/XenStore.h                   | 10 ++++++++++
>  7 files changed, 33 insertions(+)
>