From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org 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 5167821123B73 for ; Tue, 8 Jan 2019 03:55:58 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5D0D3A0E5A; Tue, 8 Jan 2019 11:55:57 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-197.rdu2.redhat.com [10.10.120.197]) by smtp.corp.redhat.com (Postfix) with ESMTP id BCEFA5D756; Tue, 8 Jan 2019 11:55:53 +0000 (UTC) To: Leif Lindholm Cc: AKASHI Takahiro , Alexander Graf , Heinrich Schuchardt , trini@konsulko.com, robdclark@gmail.com, u-boot@lists.denx.de, edk2-devel@lists.01.org, Jian J Wang , Hao Wu , Ruiyu Ni , Star Zeng , Andrew Fish , Michael D Kinney , Ard Biesheuvel , Liming Gao 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> From: Laszlo Ersek Message-ID: <65b16f57-8d34-87ee-2fcc-8312d333f308@redhat.com> Date: Tue, 8 Jan 2019 12:55:52 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190108095102.myetfzaancuzq7cx@bivouac.eciton.net> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Tue, 08 Jan 2019 11:55:58 +0000 (UTC) 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 11:55:59 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 01/08/19 10:51, Leif Lindholm wrote: > MdePkg/MdeModulePkg maintainers - any comments? > > 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 Data 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. > > Of course it would be better to change the code than the spec. > > But as Ard points out off-thread, doing (as a hack, with gcc) > > 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. > /// > > breaks Linux boot on ARM (32-bit), since it inserts 32-bits of padding > between ConfigurationTable entries in the system table. ( More precisely, it adds padding to EFI_CONFIGURATION_TABLE after "VendorGuid" or after "VendorTable". Padding may not be added at the beginning of structures, and may not be added anywhere to arrays. The practical effect is the same, so this is just a side comment about C. ) Thanks Laszlo