public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Jordan Justen <jordan.l.justen@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
	"Anthony PERARD" <anthony.perard@citrix.com>,
	"Gao, Liming" <liming.gao@intel.com>,
	"Zhu, Yonghong" <yonghong.zhu@intel.com>,
	"Ard Biesheuvel" <ard.biesheuvel@linaro.org>
Cc: edk2-devel@lists.01.org
Subject: Re: [PATCH 0/4] Fix runtime issue in XenBusDxe when compiled with GCC5
Date: Thu, 01 Dec 2016 12:06:16 -0800	[thread overview]
Message-ID: <148062277652.26637.1503158876001098261@jljusten-ivb> (raw)
In-Reply-To: <53c67cb5-e947-8979-7738-288cc83f374b@redhat.com>

On 2016-12-01 10:43:24, Laszlo Ersek wrote:
> 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?

I agree that this should be fixed.

But, if it works, I'm concerned that it would just be hiding a bug. My
understanding was that the EFIAPI on libraries was needed so that a
library implementation could be assembly based if desired. In this
case C is used for the implementation, so the calling conventions
should align.

-Jordan

> 
> > 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(+)
> > 
> 


  reply	other threads:[~2016-12-01 20:06 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-01 15:28 [PATCH 0/4] Fix runtime issue in XenBusDxe when compiled with GCC5 Anthony PERARD
2016-12-01 15:28 ` [PATCH 1/4] OvmfPkg/XenHypercallLib: Add EFIAPI Anthony PERARD
2016-12-01 15:28 ` [PATCH 2/4] OvmfPkg/XenBusDxe: Add EFIAPI to XenEventChannelNotify Anthony PERARD
2016-12-01 15:28 ` [PATCH 3/4] OvmfPkg/XenBusDxe: Add EFIAPI to XenStore functions Anthony PERARD
2016-12-01 15:28 ` [PATCH 4/4] OvmfPkg/XenBusDxe: Add EFIAPI to XenGrantTable{Grant, End}Access Anthony PERARD
2016-12-01 18:43 ` [PATCH 0/4] Fix runtime issue in XenBusDxe when compiled with GCC5 Laszlo Ersek
2016-12-01 20:06   ` Jordan Justen [this message]
2016-12-01 20:54     ` Laszlo Ersek
2016-12-02  0:58       ` Jordan Justen
2016-12-02  9:45         ` Laszlo Ersek
2016-12-02  4:36   ` Gao, Liming
2016-12-02 10:00     ` Laszlo Ersek
2016-12-02 16:02   ` Anthony PERARD
2016-12-02 19:26     ` Laszlo Ersek
2016-12-03 17:59       ` Laszlo Ersek
2016-12-05  2:55         ` Gao, Liming
2016-12-05 10:09           ` Laszlo Ersek
2017-02-21 16:39         ` Anthony PERARD
2017-02-21 17:07           ` Laszlo Ersek
2017-02-21 17:53             ` Anthony PERARD
2017-02-21 19:02               ` Laszlo Ersek
2017-02-21 19:08                 ` Rebecca Cran
2017-02-21 22:45                   ` Jordan Justen
2017-02-21 23:59                     ` Laszlo Ersek
2017-02-22 14:16                       ` Gao, Liming
2017-02-22  8:54                 ` Gao, Liming
2017-02-23 10:19                   ` Laszlo Ersek
2017-02-23 12:43                     ` Anthony PERARD
2017-02-23 13:00                     ` Gao, Liming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=148062277652.26637.1503158876001098261@jljusten-ivb \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox