On 22 March 2013 13:53, Cohen, Eugene <[email protected]> wrote: > From the beginning I've been uncomfortable that ARM BDS has inherent > knowledge of how to boot Linux since this is not compatible with the UEFI > philosophy where BDS is OS agnostic and uses standardized NVRAM values (or > standardized boot application paths) to determine what to boot. This type of > stuff typically would go in an OS loader application, separate from BDS. If > a specialized Linux OS loader app wanted to add autoboot capability so > configuring boot arguments was not required that would be great, but the more > Linux specific stuff I see going into BDS the more I cringe as this is work > that would have to be un-done (or at least avoided) by someone wanted to > implement only standard UEFI boot mechanisms.
Sure, Linaro is in full agreement with you here. That's why we have roadmap cards and engineers in place to work on a more generic and compliant solution. In the meantime, all we have is the ARM BDS that we are patching (in more ways that one ;-)) and this is a patch to try to at least get it useful in the short term. With this context in mind, if you have any comments on it, they'd be appreciated. > > -----Original Message----- > From: [email protected] > [mailto:[email protected]] On Behalf Of Rony Nandy > Sent: Thursday, March 21, 2013 11:39 PM > To: Ryan Harkin > Cc: [email protected]; Shivamurthy Shastri > Subject: Re: [PATCH 2/2] Samsung/Arndale: Auto boot feature > > Hi Ryan, > I think you have a fair point but this patch is useful for new users as > they need not bother about configuring the device paths etc.Anyway,the user > can always interrupt the boot and set his own boot configuration in case he > doesen't want autoboot.This is just an add on feature.I concede this is a > problem > for LAVA. > > On 03/22/2013 01:45 AM, Ryan Harkin wrote: >> I'll Nack this one until I get some more opinions from the other UEFI >> people (LEG, Olivier Martin, etc...?) to see what they think. >> > > >> If we are to implement this type of feature, I'd like it to be done in >> a different kind of way. Perhaps adding a new boot type, eg, add >> BDS_LOADER_KERNEL_LINUX_AUTOBOOT to the ARM_BDS_LOADER_TYPE enum in >> ArmPlatformPkg/Bds/BdsInternal.h. > This method is worth considering.But,I am not sure if this solves the LAVA > issue. > > >> >> Then, if the user selects that option, they get a magic boot. >> >> I don't like the idea that I configure, say, to boot uImage from the >> SD card, partition 3, but this feature finds a zImage in partition 1 >> and boots it. >> >> >> On 19 March 2013 11:27, Shivamurthy Shastri <[email protected]> wrote: >>> The patch been modified as per the new changes in >>> ArmPlatformPkg/Bds/BootOption.c >>> >>> Signed-off-by: Shivamurthy Shastri <[email protected]> >>> Signed-off-by: Rony Nandy <[email protected]> >>> --- >>> ...-created-to-incorporate-auto-boot-feature.patch | 101 >>> ++++++++++++--------- >>> 1 file changed, 57 insertions(+), 44 deletions(-) >>> >>> diff --git >>> a/SamsungPlatformPkg/patches/0001-The-patch-created-to-incorporate-auto-boot-feature.patch >>> >>> b/SamsungPlatformPkg/patches/0001-The-patch-created-to-incorporate-auto-boot-feature.patch >>> index 91aea96..550f15c 100644 >>> --- >>> a/SamsungPlatformPkg/patches/0001-The-patch-created-to-incorporate-auto-boot-feature.patch >>> +++ >>> b/SamsungPlatformPkg/patches/0001-The-patch-created-to-incorporate-auto-boot-feature.patch >>> @@ -1,31 +1,30 @@ >>> -From 011bc2be4b3f2a5a5413511d7380b6fe3632d0f5 Mon Sep 17 00:00:00 2001 >>> +From 3c9dabb0f20f41c37e1e18c7fcc9a39ce8bfc51d Mon Sep 17 00:00:00 2001 >>> From: Shivamurthy Shastri <[email protected]> >>> -Date: Wed, 5 Dec 2012 14:44:57 +0530 >>> +Date: Fri, 22 Feb 2013 14:33:57 +0530 >>> Subject: [PATCH] The patch created to incorporate auto boot feature for >>> arndale board. >>> >>> Signed-off-by: Shivamurthy Shastri <[email protected]> >>> --- >>> - ArmPlatformPkg/Bds/BootOption.c | 71 >>> ++++++++++++++++++++++++++++++++++++++--- >>> - 1 file changed, 67 insertions(+), 4 deletions(-) >>> + ArmPlatformPkg/Bds/BootOption.c | 85 >>> ++++++++++++++++++++++++++++++++++++----- >>> + 1 file changed, 75 insertions(+), 10 deletions(-) >>> >>> diff --git a/ArmPlatformPkg/Bds/BootOption.c >>> b/ArmPlatformPkg/Bds/BootOption.c >>> -index 289d36a..468050d 100644 >>> +index 78a4230..f35d039 100644 >>> --- a/ArmPlatformPkg/Bds/BootOption.c >>> +++ b/ArmPlatformPkg/Bds/BootOption.c >>> -@@ -14,6 +14,11 @@ >>> +@@ -13,6 +13,10 @@ >>> + **/ >>> >>> #include "BdsInternal.h" >>> - >>> +#include <Library/DxeServicesTableLib.h> >>> +#include <Protocol/BlockIo.h> >>> +#include <Guid/DebugImageInfoTable.h> >>> +#include <Protocol/DevicePathToText.h> >>> -+ >>> + >>> extern EFI_HANDLE mImageHandle; >>> >>> - EFI_STATUS >>> -@@ -22,26 +27,61 @@ BootOptionStart ( >>> +@@ -22,48 +26,91 @@ BootOptionStart ( >>> ) >>> { >>> EFI_STATUS Status; >>> @@ -37,22 +36,22 @@ index 289d36a..468050d 100644 >>> EFI_DEVICE_PATH_PROTOCOL* FdtDevicePath; >>> - EFI_DEVICE_PATH_PROTOCOL* DefaultFdtDevicePath; >>> - UINTN FdtDevicePathSize; >>> +- UINTN CmdLineSize; >>> + //EFI_DEVICE_PATH_PROTOCOL* DefaultFdtDevicePath; >>> + //UINTN FdtDevicePathSize; >>> - UINTN CmdLineSize; >>> ++ //UINTN CmdLineSize; >>> UINTN InitrdSize; >>> EFI_DEVICE_PATH* Initrd; >>> +- UINTN FdtLocalSize; >>> ++ //UINTN FdtLocalSize; >>> UINT16 LoadOptionIndexSize; >>> ++ UINTN HandleCount; >>> ++ EFI_HANDLE* HandleBuffer; >>> ++ UINTN Index; >>> ++ EFI_DEVICE_PATH_TO_TEXT_PROTOCOL* DevicePathToTextProtocol; >>> ++ EFI_DEVICE_PATH_PROTOCOL* LoadImageDevicePath; >>> ++ EFI_DEVICE_PATH_PROTOCOL* FileSystemDevicePath; >>> >>> -+ UINTN HandleCount; >>> -+ EFI_HANDLE *HandleBuffer; >>> -+ UINTN Index; >>> -+ //CHAR16* String; >>> -+ //EFI_DEVICE_PATH_PROTOCOL* DevicePathProtocol; >>> -+ EFI_DEVICE_PATH_TO_TEXT_PROTOCOL* DevicePathToTextProtocol; >>> -+ EFI_DEVICE_PATH_PROTOCOL *LoadImageDevicePath; >>> -+ EFI_DEVICE_PATH_PROTOCOL *FileSystemDevicePath; >>> -+ >>> if (IS_ARM_BDS_BOOTENTRY (BootOption)) { >>> Status = EFI_UNSUPPORTED; >>> OptionalData = BootOption->OptionalData; >>> @@ -72,12 +71,6 @@ index 289d36a..468050d 100644 >>> + return Status; >>> + } >>> + >>> -+ /*for (Index = 0; Index < HandleCount; Index++) { >>> -+ Status = gBS->HandleProtocol (HandleBuffer[Index], >>> &gEfiDevicePathProtocolGuid, (VOID **)&DevicePathProtocol); >>> -+ String = >>> DevicePathToTextProtocol->ConvertDevicePathToText(DevicePathProtocol,TRUE,TRUE); >>> -+ Print (L"[0x%X] %s\n",(UINT32)HandleBuffer[Index], String); >>> -+ }*/ >>> -+ >>> + for (Index = 0; Index < HandleCount; Index++) { >>> + //Get the device path >>> + FileSystemDevicePath = >>> DevicePathFromHandle(HandleBuffer[Index]); >>> @@ -91,15 +84,30 @@ index 289d36a..468050d 100644 >>> >>> Status = BdsStartEfiApplication (mImageHandle, >>> BootOption->FilePathList, 0, NULL); >>> } else if (LoaderType == BDS_LOADER_KERNEL_LINUX_ATAG) { >>> -@@ -55,9 +95,18 @@ BootOptionStart ( >>> + LinuxArguments = &(OptionalData->Arguments.LinuxArguments); >>> +- CmdLineSize = ReadUnaligned16 ((CONST >>> UINT16*)&LinuxArguments->CmdLineSize); >>> ++ //CmdLineSize = ReadUnaligned16 ((CONST >>> UINT16*)&LinuxArguments->CmdLineSize); >>> + InitrdSize = ReadUnaligned16 ((CONST >>> UINT16*)&LinuxArguments->InitrdSize); >>> + >>> ++#if 0 >>> + if (InitrdSize > 0) { >>> + Initrd = GetAlignedDevicePath >>> ((EFI_DEVICE_PATH*)((UINTN)(LinuxArguments + 1) + CmdLineSize)); >>> + } else { >>> Initrd = NULL; >>> } >>> - >>> +- >>> ++#endif >>> ++ >>> + //Check if zImage file on SD-MMC. >>> + LoadImageDevicePath = FileDevicePath(HandleBuffer[Index], >>> L"uImage"); >>> ++ if (InitrdSize > 0) { >>> ++ Initrd = FileDevicePath(HandleBuffer[Index], L"uInitrd"); >>> ++ } else { >>> ++ Initrd = NULL; >>> ++ } >>> + Status = BdsBootLinuxAtag (LoadImageDevicePath, Initrd, >>> (CHAR8*)(LinuxArguments + 1)); >>> + if (EFI_ERROR(Status)) { >>> -+ continue; >>> ++ continue; >>> + } >>> + >>> +#if 0 >>> @@ -107,33 +115,38 @@ index 289d36a..468050d 100644 >>> Initrd, // Initrd >>> (CHAR8*)(LinuxArguments + 1)); // >>> CmdLine >>> +#endif >>> - } else if (LoaderType == BDS_LOADER_KERNEL_LINUX_FDT) { >>> + } else if ((LoaderType == BDS_LOADER_KERNEL_LINUX_GLOBAL_FDT) || >>> (LoaderType == BDS_LOADER_KERNEL_LINUX_LOCAL_FDT)) { >>> LinuxArguments = &(OptionalData->Arguments.LinuxArguments); >>> - CmdLineSize = ReadUnaligned16 ((CONST >>> UINT16*)&LinuxArguments->CmdLineSize); >>> -@@ -69,6 +118,7 @@ BootOptionStart ( >>> - Initrd = NULL; >>> - } >>> +- CmdLineSize = ReadUnaligned16 ((CONST >>> UINT16*)&LinuxArguments->CmdLineSize); >>> ++ //CmdLineSize = ReadUnaligned16 ((CONST >>> UINT16*)&LinuxArguments->CmdLineSize); >>> + InitrdSize = ReadUnaligned16 ((CONST >>> UINT16*)&LinuxArguments->InitrdSize); >>> >>> +#if 0 >>> - // Get the default FDT device path >>> - Status = gBS->LocateProtocol (&gEfiDevicePathFromTextProtocolGuid, >>> NULL, (VOID **)&EfiDevicePathFromTextProtocol); >>> - ASSERT_EFI_ERROR(Status); >>> -@@ -78,13 +128,26 @@ BootOptionStart ( >>> - FdtDevicePathSize = GetDevicePathSize (DefaultFdtDevicePath); >>> - Status = GetEnvironmentVariable ((CHAR16 *)L"Fdt", >>> DefaultFdtDevicePath, &FdtDevicePathSize, (VOID **)&FdtDevicePath); >>> - ASSERT_EFI_ERROR(Status); >>> + if (InitrdSize > 0) { >>> + Initrd = GetAlignedDevicePath >>> ((EFI_DEVICE_PATH*)((UINTN)(LinuxArguments + 1) + CmdLineSize)); >>> + } else { >>> +@@ -89,13 +136,31 @@ BootOptionStart ( >>> + Status = GetEnvironmentVariable ((CHAR16 *)L"Fdt", >>> DefaultFdtDevicePath, &FdtDevicePathSize, (VOID **)&FdtDevicePath); >>> + ASSERT_EFI_ERROR(Status); >>> + } >>> +- >>> +#endif >>> + >>> + //Check if zImage file on SD-MMC. >>> + LoadImageDevicePath = FileDevicePath(HandleBuffer[Index], >>> L"uImage"); >>> -+ FdtDevicePath = FileDevicePath(HandleBuffer[Index], >>> L"exynos5250-arndale.dtb"); >>> ++ FdtDevicePath = FileDevicePath(HandleBuffer[Index], L"board.dtb"); >>> ++ if (InitrdSize > 0) { >>> ++ Initrd = FileDevicePath(HandleBuffer[Index], L"uInitrd"); >>> ++ } else { >>> ++ Initrd = NULL; >>> ++ } >>> + Status = BdsBootLinuxFdt (LoadImageDevicePath, Initrd, >>> (CHAR8*)(LinuxArguments + 1), FdtDevicePath); >>> + FreePool (FdtDevicePath); >>> + if (EFI_ERROR(Status)) { >>> + continue; >>> + } >>> + } >>> - >>> ++ >>> +#if 0 >>> Status = BdsBootLinuxFdt (BootOption->FilePathList, >>> Initrd, // Initrd >>> @@ -146,5 +159,5 @@ index 289d36a..468050d 100644 >>> } else { >>> // Set BootCurrent variable >>> -- >>> -1.8.0 >>> +1.8.1.1 >>> >>> -- >>> 1.8.2 >>> >> . >> > > > _______________________________________________ > boot-architecture mailing list > [email protected] > http://lists.linaro.org/mailman/listinfo/boot-architecture _______________________________________________ boot-architecture mailing list [email protected] http://lists.linaro.org/mailman/listinfo/boot-architecture
