From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.126; helo=mga18.intel.com; envelope-from=dandan.bi@intel.com; receiver=edk2-devel@lists.01.org Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) (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 8B20A211A43CD for ; Tue, 8 Jan 2019 07:02:48 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Jan 2019 07:02:48 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,454,1539673200"; d="scan'208";a="132674698" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga002.fm.intel.com with ESMTP; 08 Jan 2019 07:02:47 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.408.0; Tue, 8 Jan 2019 07:02:47 -0800 Received: from shsmsx152.ccr.corp.intel.com ([169.254.6.44]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.160]) with mapi id 14.03.0415.000; Tue, 8 Jan 2019 23:02:45 +0800 From: "Bi, Dandan" To: 'Leif Lindholm' , Laszlo Ersek CC: "Ni, Ray" , "trini@konsulko.com" , AKASHI Takahiro , "Wu, Hao A" , Heinrich Schuchardt , "edk2-devel@lists.01.org" , Alexander Graf , "Gao, Liming" , "u-boot@lists.denx.de" , "robdclark@gmail.com" , "Kinney, Michael D" , "Zeng, Star" Thread-Topic: [edk2] [RESEND PATCH v2 2/6] efi_loader: Initial HII database protocols Thread-Index: AQHUppiJQEruak4VZUygYv9D0QKS86WjmqGAgAAOrwCAAFVnAIAAnVGAgADTEvA= Date: Tue, 8 Jan 2019 15:02:44 +0000 Message-ID: <3C0D5C461C9E904E8F62152F6274C0BB40B7D02A@SHSMSX152.ccr.corp.intel.com> References: <20181214101043.14067-1-takahiro.akashi@linaro.org> <20181214101043.14067-3-takahiro.akashi@linaro.org> <20181217011626.GC14562@linaro.org> <84b6f3fd-ed68-a541-7727-69e5392984e6@suse.de> <20181225083024.GC14405@linaro.org> <20190107140932.uefkly3a3jzlyjjf@bivouac.eciton.net> <7d6fbbff-ca48-588a-6082-bf8b95a7e829@redhat.com> <20190107192220.ugkcxfd3betvuypi@bivouac.eciton.net> <1d1c1e2f-193c-5e1f-f51a-b922b67eb428@redhat.com> <20190108095102.myetfzaancuzq7cx@bivouac.eciton.net> In-Reply-To: <20190108095102.myetfzaancuzq7cx@bivouac.eciton.net> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNjUwYzY3ZTItZDQ4ZS00N2JjLWFlNGQtZDM1MDFhZTY2NDdlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiU2w3Y1wvUHV5ZHZEclY2Q3g4QUNTSjZYVE4zdFZpcTlSQmhnVTc3TmxmSDVKNk5tY29VdCttMlRDd0RZbzdMdkoifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [RESEND PATCH v2 2/6] efi_loader: Initial HII database protocols X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Jan 2019 15:02:48 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Leif Lindholm > Sent: Tuesday, January 08, 2019 5:51 PM > To: Laszlo Ersek > Cc: Ni, Ray ; trini@konsulko.com; AKASHI Takahiro > ; Wu, Hao A ; Heinrich > Schuchardt ; edk2-devel@lists.01.org; Alexander > Graf ; Gao, Liming ; u- > boot@lists.denx.de; robdclark@gmail.com; Kinney, Michael D > ; Zeng, Star > Subject: Re: [edk2] [RESEND PATCH v2 2/6] efi_loader: Initial HII databas= e > protocols >=20 > MdePkg/MdeModulePkg maintainers - any comments? >=20 > On Tue, Jan 08, 2019 at 01:28:00AM +0100, Laszlo Ersek wrote: > > On 01/07/19 20:22, Leif Lindholm wrote: > > > On Mon, Jan 07, 2019 at 07:29:47PM +0100, Laszlo Ersek wrote: > > > > >> The UEFI spec (v2.7) explicitly requires EFI_GUID to be 64-bit > > >> aligned, unless specified otherwise. See in "Table 5. Common UEFI Da= ta > Types": > > >> > > >> EFI_GUID -- 128-bit buffer containing a unique identifier value. > > >> Unless otherwise specified, aligned on a 64-bit > > >> boundary. > > > > > > Indeed. > > > > > >> Whether edk2 satisfies that, and if so, how (by chance / by general > > >> build flags), I don't know. The code says, > > >> > > >> /// > > >> /// 128 bit buffer containing a unique identifier value. > > >> /// Unless otherwise specified, aligned on a 64 bit boundary. > > >> /// > > >> typedef struct { > > >> UINT32 Data1; > > >> UINT16 Data2; > > >> UINT16 Data3; > > >> UINT8 Data4[8]; > > >> } GUID; > > >> > > >> I think there may have been an expectation in > "MdePkg/Include/Base.h" > > >> that the supported compilers would automatically ensure the > > >> specified alignment, given the structure definition. > > > > > > But that would be expecting things not only not guaranteed by C, but > > > something there is no semantic information suggesting would be > > > useful for the compiler to do above. [...] > > > > Agreed. I'm not saying the edk2 code is right, just guessing why the > > code might look like it does. This would not be the first silent > > assumption, I think. > > > > Anyhow, I think it would be better to change the code than the spec. >=20 > Of course it would be better to change the code than the spec. >=20 > But as Ard points out off-thread, doing (as a hack, with gcc) >=20 > diff --git a/MdePkg/Include/Uefi/UefiBaseType.h > b/MdePkg/Include/Uefi/UefiBaseType.h > index 8c9d571eb1..75409f3460 100644 > --- a/MdePkg/Include/Uefi/UefiBaseType.h > +++ b/MdePkg/Include/Uefi/UefiBaseType.h > @@ -26,7 +26,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY KIND, EITHER EXPRESS OR IMPLIED. > /// > /// 128-bit buffer containing a unique identifier value. > /// > -typedef GUID EFI_GUID; > +typedef GUID EFI_GUID __attribute__((aligned (8))); > /// > /// Function return status for EFI API. > /// >=20 > breaks Linux boot on ARM (32-bit), since it inserts 32-bits of padding > between ConfigurationTable entries in the system table. So I don't see ho= w > that can realistically be fixed in the EDK2 codebase. >=20 > And with things like the EFI_HII_KEYBOARD_LAYOUT struct, if there has eve= r > been compatibility between EDK2 and commercial BIOSes, then that struct > has always been treated as packed (not just 32-bit aligned GUIDs), and th= e > spec just needs to reflect reality. If there hasn't, then indeed the code > change here would be trivial. The structure definitions in Include/Uefi/UefiInternalFormRepresentation.h = mainly describe the binary encoding of the different package types. And EFI= _HII_KEYBOARD_LAYOUT struct is for the Keyboard Layout Package.=20 Describing the *binary encoding* of the different package type, so I think= we should treat them as packed and it also should be the reason why they a= re packed now. Maybe we can reflect this more clear in Spec. >=20 > (Adding Liming as well, since we're now discussing MdePkg also.) >=20 > Yes, this discussion belongs on USWG (UEFI specification working group > mailing list), but I want to hear some comment from the package > maintainers first. >=20 > Either way, I see a bunch of new SCT tests coming up. >=20 > / > Leif > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel