From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::d41; helo=mail-io1-xd41.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io1-xd41.google.com (mail-io1-xd41.google.com [IPv6:2607:f8b0:4864:20::d41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D3F5A211982E0 for ; Tue, 11 Dec 2018 04:15:49 -0800 (PST) Received: by mail-io1-xd41.google.com with SMTP id l14so11587288ioj.5 for ; Tue, 11 Dec 2018 04:15:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ram7VdArb6JAjBK2RF+Q4BABKk4+/KlUj/cf8u8n14Y=; b=QQBwGPTyGkOGtdBDO6Ss8T0Vi/LOU6gia91uIoAULgnFuggw4HBNZIjTJGteHVwdNl IGgkwUf0B5zRGtFyc0v87Nt883h4OG5PeaxMizhaMZ3z787dnM+gCVWnKCeCEEANYpqC VodUXPZ8MlH7shPPQ2fVh67Ln2gsKsBEV9v2o= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ram7VdArb6JAjBK2RF+Q4BABKk4+/KlUj/cf8u8n14Y=; b=FjjraAVGDi93zzzG4QYbnV0yA8es+rUgm8+RB/z75kkJP76CFtchPGViSQtVK2DsF8 3mGlfXYa3GYl5WEm0bSHdgu43rL23gG7/aVLDJyY2gdcIfiLVTCeJ8/YzO0I4tbPkpmi /lFBtQkqN9ofjhjLkKSeKwk2nh3vkBG++nMR2iO/g+q+C4WR3iDiI9y/QpEpXZjaVhgd ZEQY+rqippHEo3y/haXiB2Xa301L15Exy7sWpywtSimzsnWIPfeDkMq/BHQGMiTaKgPl k2+fAhtwZcooxyrSWvj5R71elXlpPNSb86BOo5xG9KEA3kdiehjICVei/r/aqVVOGrTi 9ogg== X-Gm-Message-State: AA+aEWaNW7ehdqh0KBr3/fZuxI7LoZ/gpQXwswAWwpgHrY35sJL8KKZn u4+E/2BCrQB7Dlo1bKKwI/fHfauv4ggPwSb6O9GywQ== X-Google-Smtp-Source: AFSGD/XHk98i+8LnwpBa0YS/Hz+dFQZy863M/YZ9CaCW/92Nts2E3cSLBENtRX0HJirqlgDyuR0ebO1HUf7kME9L8jg= X-Received: by 2002:a5e:cb42:: with SMTP id h2mr13687587iok.60.1544530547762; Tue, 11 Dec 2018 04:15:47 -0800 (PST) MIME-Version: 1.0 References: <20181208102431.9185-1-ard.biesheuvel@linaro.org> In-Reply-To: From: Ard Biesheuvel Date: Tue, 11 Dec 2018 13:15:36 +0100 Message-ID: To: "Wu, Hao A" Cc: "edk2-devel@lists.01.org" , Jian J Wang , Ruiyu Ni , "Gao, Liming" , Leif Lindholm Subject: Re: [PATCH] MdeModulePkg/FileExplorerLib: avoid packed struct for program data 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, 11 Dec 2018 12:15:50 -0000 Content-Type: text/plain; charset="UTF-8" On Tue, 11 Dec 2018 at 02:48, Wu, Hao A wrote: > > > -----Original Message----- > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > Sent: Saturday, December 08, 2018 6:25 PM > > To: edk2-devel@lists.01.org > > Cc: Wang, Jian J; Wu, Hao A; Ni, Ruiyu; Ard Biesheuvel > > Subject: [PATCH] MdeModulePkg/FileExplorerLib: avoid packed struct for > > program data > > > > Struct packing is only necessary for data structures whose in-memory > > representation is covered by the PI or UEFI specs, and may deviate > > from the ordinary C rules for alignment. > > > > So in case of FileExplorerLib, this applies to the device path struct > > only, and other structures used to carry program data should not be > > packed, or we may end up with alignment faults on architectures such > > as ARM, which don't permit load/store double or multiple instructions > > to access memory locations that are not 32-bit aligned. > > > > E.g., the following call in FileExplorerLibConstructor() > > > > InitializeListHead (&gFileExplorerPrivate.FsOptionMenu->Head); > > > > which is emitted as follows for 32-bit ARM/Thumb2 by Clang-5.0 > > > > 3de0: b510 push {r4, lr} > > 3de2: 4604 mov r4, r0 > > ... > > 3de8: e9c4 4400 strd r4, r4, [r4] > > 3dec: bd10 pop {r4, pc} > > > > will perform a double-word store on the first argument, passed in > > register r0, assuming that the pointer type of the argument is > > enough to guarantee that the value is suitably aligned. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel > > --- > > MdeModulePkg/Library/FileExplorerLib/FileExplorer.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/MdeModulePkg/Library/FileExplorerLib/FileExplorer.h > > b/MdeModulePkg/Library/FileExplorerLib/FileExplorer.h > > index bf1450dbd581..603185abe4b1 100644 > > --- a/MdeModulePkg/Library/FileExplorerLib/FileExplorer.h > > +++ b/MdeModulePkg/Library/FileExplorerLib/FileExplorer.h > > @@ -51,6 +51,8 @@ typedef struct { > > EFI_DEVICE_PATH_PROTOCOL End; > > } HII_VENDOR_DEVICE_PATH; > > > > +#pragma pack() > > + > > typedef struct { > > EFI_HANDLE DeviceHandle; > > EFI_DEVICE_PATH_PROTOCOL *DevicePath; > > @@ -100,8 +102,6 @@ typedef struct { > > > > #define FILE_EXPLORER_PRIVATE_FROM_THIS(a) CR (a, > > FILE_EXPLORER_CALLBACK_DATA, FeConfigAccess, > > FILE_EXPLORER_CALLBACK_DATA_SIGNATURE) > > > > -#pragma pack() > > - > > Reviewed-by: Hao Wu > > Best Regards, > Hao Wu > Thanks all Pushed as f0574a194c1e..765fb87c2b70