From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8F66A81C98 for ; Thu, 1 Dec 2016 12:06:17 -0800 (PST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP; 01 Dec 2016 12:06:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,726,1477983600"; d="scan'208";a="37927359" Received: from awillke-mobl1.amr.corp.intel.com (HELO localhost) ([10.252.136.252]) by fmsmga005.fm.intel.com with ESMTP; 01 Dec 2016 12:06:16 -0800 MIME-Version: 1.0 To: Laszlo Ersek , "Anthony PERARD" , "Gao, Liming" , "Zhu, Yonghong" , "Ard Biesheuvel" Message-ID: <148062277652.26637.1503158876001098261@jljusten-ivb> From: Jordan Justen In-Reply-To: <53c67cb5-e947-8979-7738-288cc83f374b@redhat.com> Cc: edk2-devel@lists.01.org References: <20161201152819.8341-1-anthony.perard@citrix.com> <53c67cb5-e947-8979-7738-288cc83f374b@redhat.com> User-Agent: alot/0.3.7 Date: Thu, 01 Dec 2016 12:06:16 -0800 Subject: Re: [PATCH 0/4] Fix runtime issue in XenBusDxe when compiled with GCC5 X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Dec 2016 20:06:17 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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 - 00000= 000 !!!! > > RIP - 000000001F26AF6B, CS - 0000000000000038, RFLAGS - 0000000000010= 202 > > 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/Xen= BusDxe/DEBUG/XenBusDxe.dll (ImageBase=3D000000001F266000, EntryPoint=3D0000= 00001F2669D5) !!!! > > = > > 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 !=3D 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(+) > > = >=20