From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 8F5D181C8B for ; Thu, 1 Dec 2016 12:54:37 -0800 (PST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D7160624CB; Thu, 1 Dec 2016 20:54:36 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-138.phx2.redhat.com [10.3.116.138]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uB1KsYQM025693; Thu, 1 Dec 2016 15:54:35 -0500 To: Jordan Justen , Anthony PERARD , "Gao, Liming" , "Zhu, Yonghong" , Ard Biesheuvel References: <20161201152819.8341-1-anthony.perard@citrix.com> <53c67cb5-e947-8979-7738-288cc83f374b@redhat.com> <148062277652.26637.1503158876001098261@jljusten-ivb> Cc: edk2-devel@ml01.01.org From: Laszlo Ersek Message-ID: <914ffc38-90fe-4ded-4566-49f8e3112605@redhat.com> Date: Thu, 1 Dec 2016 21:54:34 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.0 MIME-Version: 1.0 In-Reply-To: <148062277652.26637.1503158876001098261@jljusten-ivb> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Thu, 01 Dec 2016 20:54:37 +0000 (UTC) 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:54:37 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 12/01/16 21:06, Jordan Justen wrote: > 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. Never tried the following, so I'm unsure if edk2 intends to support it explicitly, but what about binary-only library instances (the 2-clause BSDL allows that)? If the library class header doesn't state EFIAPI on the functions, the library vendor builds the library instance with Visual Studio, and the library user builds the client module with gcc (against the same library class header), calls will fail. (The way I imagine using binary-only library instances is that the library comes as a binary object with a matching INF file, in a separate subdirectory, and the user resolves the library class in his/her platform DSC to that INF file. Not sure about the exact [section names] in the library instance's INF file though.) Thanks! Laszlo > > -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(+) >>> >>