> On 22 Nov 2018, at 17:30, Laszlo Ersek <[email protected]> wrote: > >> On 11/22/18 14:08, Ard Biesheuvel wrote: >>> On Thu, 22 Nov 2018 at 12:37, Ard Biesheuvel <[email protected]> >>> wrote: >>> >>> On Thu, 22 Nov 2018 at 12:19, chandni cherukuri >>> <[email protected]> wrote: >>>> >>>> On Thu, Nov 22, 2018 at 1:20 PM Ard Biesheuvel >>>> <[email protected]> wrote: >>>>> >>>>>> On Thu, 22 Nov 2018 at 05:01, Thomas Abraham <[email protected]> >>>>>> wrote: >>>>>> >>>>>> Hi Ard, >>>>>> >>>>>> On Thu, Nov 22, 2018 at 3:46 AM Ard Biesheuvel >>>>>> <[email protected]> wrote: >>>>>>> >>>>>>>> On Wed, 21 Nov 2018 at 14:48, Thomas Abraham <[email protected]> >>>>>>>> wrote: >>>>>>>> >>>>>>>> Hi Ard, >>>>>>>> >>>>>>>> On Wed, Nov 21, 2018 at 5:31 PM Ard Biesheuvel >>>>>>>> <[email protected]> wrote: >>>>>>>>> >>>>>>>>> Align edk2-platform with upcoming changes to EDK2 to get rid of >>>>>>>>> per-bank >>>>>>>>> NOR flash GUIDs. >>>>>>>>> >>>>>>>>> Ard Biesheuvel (3): >>>>>>>>> Platform/ARM: replace hardcoded VenHW() device paths referring to NOR >>>>>>>>> flash >>>>>>>>> Silicon/SynQuacer: drop per-bank NOR flash GUIDs >>>>>>>>> Platform/ARM: drop per-bank NOR flash GUIDs >>>>>>>>> >>>>>>>>> Platform/ARM/JunoPkg/ArmJuno.dec | 2 +- >>>>>>>>> Platform/ARM/JunoPkg/ArmJuno.dsc | 2 +- >>>>>>>>> .../JunoPkg/Library/NorFlashJunoLib/NorFlashJuno.c | 2 -- >>>>>>>>> .../ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.c | 2 -- >>>>>>>>> Platform/ARM/SgiPkg/SgiPlatform.dsc | 2 +- >>>>>>>>> Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc | 2 +- >>>>>>>>> Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 2 +- >>>>>>>>> .../NorFlashArmVExpressLib/NorFlashArmVExpress.c | 4 ---- >>>>>>>>> .../SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c | 14 +++++++------- >>>>>>>>> .../SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h | 3 +++ >>>>>>>>> .../NorFlashSynQuacerLib/NorFlashSynQuacer.c | 6 ------ >>>>>>>>> 11 files changed, 15 insertions(+), 26 deletions(-) >>>>>>>> >>>>>>>> Tested this patch series and "[PATCH v2 0/5] ArmPlatformPkg, >>>>>>>> ArmVirtPkg: discover NOR flash banks from DTB" patch series on the >>>>>>>> Juno board. With these patches applied, the boot fails on Juno board >>>>>>>> with the following messages. I have not yet tried to debug the issue >>>>>>>> but wanted to let you know this. >>>>>>>> >>>>>>>> [...] >>>>>>>> Loading driver at 0x000F830C000 EntryPoint=0x000F831B2AC IScsiDxe.efi >>>>>>>> add-symbol-file >>>>>>>> /home/thopan01/devel/juno/uefi/uefi/edk2/Build/ArmJuno/DEBUG_GCC5/AARCH64/MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe/DEBUG/Udp4Dxe.dll >>>>>>>> 0xF8300000 >>>>>>>> Loading driver at 0x000F82FF000 EntryPoint=0x000F8306DF0 Udp4Dxe.efi >>>>>>>> add-symbol-file >>>>>>>> /home/thopan01/devel/juno/uefi/uefi/edk2/Build/ArmJuno/DEBUG_GCC5/AARCH64/Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe/DEBUG/FdtPlatformDxe.dll >>>>>>>> 0xF82EE000 >>>>>>>> Loading driver at 0x000F82ED000 EntryPoint=0x000F82F76EC >>>>>>>> FdtPlatformDxe.efi >>>>>>>> Found image: fip in block 5. >>>>>>>> Found image: norkern in block 20. >>>>>>>> Found image: ramdisk.img in block 116. >>>>>>>> Found image: hdlcdclk in block 151. >>>>>>>> Found image: selftest in block 152. >>>>>>>> Found image: board.dtb in block 156. >>>>>>>> Found image: scp_bl1 in block 249. >>>>>>>> Found image: bl1 in block 251. >>>>>>>> Found image: startup.nsh in block 252. >>>>>>>> ASSERT [BootMonFs] >>>>>>>> /home/thopan01/devel/juno/uefi/uefi/edk2/MdePkg/Library/BaseLib/String.c(173): >>>>>>>> ((UINTN) String & 0x00000001) == 0 >>>>>>>> >> ... >>> Could you please share the backtrace and the results of >>> >>> nm -n >>> Build/ArmJuno/DEBUG_GCC5/AARCH64/Platform/ARM/Drivers/BootMonFs/BootMonFs/DEBUG/BootMonFs.dll >>> >>> for the failing case? >>> >> >> It seems like BootMonFsOpenFile() is being called from FdtClientDxe > > I think you mean "FdtPlatformDxe". >
Yes. Yay for multitasking :-) >> with a misaligned CHAR16* argument for Filename. > > Yup, that's not an infrequent bug. > > Device paths are packed. Once we introduce a UINT8 field to a device > path node, the rest of the nodes in the same device path will shift by 1 > byte. If there is a CHAR16 array field in one of those "later" device > path nodes, such as "FILEPATH_DEVICE_PATH.PathName", then it might go > from naturally aligned to mis-aligned, or vice versa. Therefore, passing > such CHAR16 fields to string functions in BaseLib, that is, right out of > the containing device path node, is never portable. If it happens to > work, that is by chance only. > > For example: > > https://bugzilla.tianocore.org/show_bug.cgi?id=1003 > https://bugzilla.tianocore.org/show_bug.cgi?id=1008 > > I suggest grepping "FdtPlatformDxe" (or, well, rather "BootMonFs"!) for > "PathName", and wherever it is passed to StrLen() -- because > MdePkg/Library/BaseLib/String.c(173) is in StrLen() --, make sure the > pathname is first re-aligned naturally, temporarily. For example, with > AllocateCopyPool(). > > gBS->AllocatePool() guarantees 8-byte alignment per spec. While > AllocatePool() / AllocateCopyPool() from MemoryAllocationLib don't > explicitly guarantee the same "on the tin", I think that's not by > design, but by omission. FWIW, for TianoCore BZs 1003 and 1008 above, we > assumed AllocateCopyPool() would ensure 8-byte alignment as well. > I tracked this down to BdsLib (a remnant of the illustrious ARM bds), which is a bit of a mess. I’ll have some patches out shortly. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

