[edk2,RFC] ArmPkg: add driver to add distro installer HTTP boot options

Message ID 20171021131049.23844-1-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • [edk2,RFC] ArmPkg: add driver to add distro installer HTTP boot options
Related show

Commit Message

Ard Biesheuvel Oct. 21, 2017, 1:10 p.m.
To make it easier for power users to provision a 'desktop' system [as
opposed to a VM or server] from scratch, introduce a driver that adds
boot options to the boot menu that can launch network installers over
HTTP straight off the Internet.

Currently, this only supports the 'mini.iso' style netboot installers
that Debian/Ubuntu provide: larger images that need to be mounted by
the installer when running under the Linux kernel are only supported
on ACPI systems with support for the NFIT table, and this was enabled
only recently (Linux v4.14) for arm64. For DT boot, there is currently
no way at all to expose ramdisks created by UEFI as block devices in
Linux.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---

Posted as an RFC because:
a) Where does this belong? Surely not in ArmPkg but I had to put it somewhere
b) Currently, the way the options are created results in them taking priority
   if no real boot options were set (i.e., for GRUB).
c) Is there a use case for downloading 300-500 MB installers off the Internet
   for a one shot installation? Or should we just stick to the mini.iso flavors.
d) Did I miss any distros we may care about?

 ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c   | 228 ++++++++++++++++++++
 ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf |  42 ++++
 2 files changed, 270 insertions(+)

-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Oct. 22, 2017, 2:40 p.m. | #1
On Sat, Oct 21, 2017 at 02:10:49PM +0100, Ard Biesheuvel wrote:
> To make it easier for power users to provision a 'desktop' system [as

> opposed to a VM or server] from scratch, introduce a driver that adds

> boot options to the boot menu that can launch network installers over

> HTTP straight off the Internet.

> 

> Currently, this only supports the 'mini.iso' style netboot installers

> that Debian/Ubuntu provide: larger images that need to be mounted by

> the installer when running under the Linux kernel are only supported

> on ACPI systems with support for the NFIT table, and this was enabled

> only recently (Linux v4.14) for arm64. For DT boot, there is currently

> no way at all to expose ramdisks created by UEFI as block devices in

> Linux.


This is good stuff.

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

> 

> Posted as an RFC because:

> a) Where does this belong? Surely not in ArmPkg but I had to put it somewhere


Not ArmPkg. EmbeddedPkg would also be wrong.

It feels to me like it shares a common thread with the ACPI/DT
switcher and the console preference widget.

> b) Currently, the way the options are created results in them taking priority

>    if no real boot options were set (i.e., for GRUB).

> c) Is there a use case for downloading 300-500 MB installers off the Internet

>    for a one shot installation? Or should we just stick to the mini.iso flavors.

> d) Did I miss any distros we may care about?


I think answers to all three of the above questions can end up being
somewhat platform-specific. But I also think figuring out how that may
best be implementented can wait.

I could see a case here for a new package of UI tweaks and utilities.
It would also not be unreasonable to add items like this under
MdeModulePkg/{Application|Library}.

/
    Leif

>  ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c   | 228 ++++++++++++++++++++

>  ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf |  42 ++++

>  2 files changed, 270 insertions(+)

> 

> diff --git a/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c b/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c

> new file mode 100644

> index 000000000000..7c5934935924

> --- /dev/null

> +++ b/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c

> @@ -0,0 +1,228 @@

> +/** @file

> + *

> + *  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.

> + *

> + *  This program and the accompanying materials are licensed and made available

> + *  under the terms and conditions of the BSD License which accompanies this

> + *  distribution.  The full text of the license may be found at

> + *  http://opensource.org/licenses/bsd-license.php

> + *

> + *  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,

> + *  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

> + */

> +

> +#include <Uefi.h>

> +#include <Library/BaseLib.h>

> +#include <Library/BaseMemoryLib.h>

> +#include <Library/DebugLib.h>

> +#include <Library/DevicePathLib.h>

> +#include <Library/MemoryAllocationLib.h>

> +#include <Library/UefiBootManagerLib.h>

> +#include <Library/UefiBootServicesTableLib.h>

> +#include <Library/UefiLib.h>

> +#include <Protocol/SimpleNetwork.h>

> +

> +typedef struct {

> +  CONST CHAR16  *Name;

> +  CONST CHAR8   *Uri;

> +} OS_INSTALLER_IMAGE;

> +

> +STATIC CONST OS_INSTALLER_IMAGE mOsInstallers[] = {

> +  { L"Install Debian Stretch over HTTP",

> +    "http://ftp.us.debian.org/debian/dists/stretch/main/installer-arm64/current/images/netboot/mini.iso" },

> +

> +  { L"Install Ubuntu 17.10 (Artful) over HTTP",

> +    "http://ports.ubuntu.com/ubuntu-ports/dists/artful/main/installer-arm64/current/images/netboot/mini.iso" },

> +

> +//

> +// The links below refer to 300-500 MB netboot images that need to be exposed to

> +// the OS via a ramdisk after the OS loader boots the installer from it.

> +// Currently, this requires ACPI/NFIT support, which was only enabled for arm64

> +// in Linux in version v4.14. For DT boot, there is currently no solution for

> +// this.

> +//

> +//  { L"Install openSUSE Tumbleweed over HTTP",

> +//    "http://download.opensuse.org/ports/aarch64/factory/iso/openSUSE-Tumbleweed-NET-aarch64-Current.iso" },

> +//

> +//  { L"Install Fedora Server 26 over HTTP",

> +//    "http://download.fedoraproject.org/pub/fedora-secondary/releases/26/Server/aarch64/iso/Fedora-Server-netinst-aarch64-26-1.5.iso" },

> +//

> +//  { L"Install Centos 7 over HTTP",

> +//    "http://mirror.centos.org/altarch/7/isos/aarch64/CentOS-7-aarch64-NetInstall.iso" }

> +//

> +};

> +

> +STATIC EFI_EVENT                  mRegisterProtocolEvent;

> +STATIC VOID                       *mRegistration;

> +

> +STATIC

> +EFI_STATUS

> +CreateOsInstallerBootOptions (

> +  IN  EFI_HANDLE    Handle

> +  )

> +{

> +  EFI_DEVICE_PATH_PROTOCOL        *ParentDevicePath;

> +  EFI_DEVICE_PATH_PROTOCOL        *TmpDevicePath;

> +  EFI_DEVICE_PATH_PROTOCOL        *NewDevicePath;

> +  EFI_STATUS                      Status;

> +  UINTN                           Idx;

> +  EFI_DEV_PATH                    *Node;

> +  UINTN                           Length;

> +  EFI_BOOT_MANAGER_LOAD_OPTION    NewOption;

> +  EFI_BOOT_MANAGER_LOAD_OPTION    *BootOptions;

> +  UINTN                           BootOptionCount;

> +  INTN                            OptionIndex;

> +

> +  BootOptions = EfiBootManagerGetLoadOptions (&BootOptionCount,

> +                  LoadOptionTypeBoot);

> +  ASSERT (BootOptions != NULL);

> +

> +  Status = gBS->HandleProtocol (Handle, &gEfiDevicePathProtocolGuid,

> +                  (VOID **)&ParentDevicePath);

> +  if (EFI_ERROR (Status)) {

> +    DEBUG ((DEBUG_WARN, "%a: gBS->HandleProtocol returned %r\n",

> +      __FUNCTION__, Status));

> +    return Status;

> +  }

> +

> +  for (Idx = 0; Idx < ARRAY_SIZE (mOsInstallers); Idx++) {

> +    Node = AllocateZeroPool (sizeof (IPv4_DEVICE_PATH));

> +    if (Node == NULL) {

> +      Status = EFI_OUT_OF_RESOURCES;

> +      goto FreeBootOptions;

> +    }

> +    Node->Ipv4.Header.Type    = MESSAGING_DEVICE_PATH;

> +    Node->Ipv4.Header.SubType = MSG_IPv4_DP;

> +    SetDevicePathNodeLength (Node, sizeof (IPv4_DEVICE_PATH));

> +

> +    TmpDevicePath = AppendDevicePathNode (ParentDevicePath,

> +                      (EFI_DEVICE_PATH_PROTOCOL*) Node);

> +    FreePool (Node);

> +    if (TmpDevicePath == NULL) {

> +      Status = EFI_OUT_OF_RESOURCES;

> +      goto FreeBootOptions;

> +    }

> +

> +    //

> +    // Update the URI node with the input boot file URI.

> +    //

> +    Length = sizeof (EFI_DEVICE_PATH_PROTOCOL) +

> +             AsciiStrSize (mOsInstallers[Idx].Uri);

> +    Node = AllocatePool (Length);

> +    if (Node == NULL) {

> +      Status = EFI_OUT_OF_RESOURCES;

> +      FreePool (TmpDevicePath);

> +      goto FreeBootOptions;

> +    }

> +    Node->DevPath.Type    = MESSAGING_DEVICE_PATH;

> +    Node->DevPath.SubType = MSG_URI_DP;

> +    SetDevicePathNodeLength (Node, Length);

> +    CopyMem ((UINT8*) Node + sizeof (EFI_DEVICE_PATH_PROTOCOL),

> +      mOsInstallers[Idx].Uri, AsciiStrSize (mOsInstallers[Idx].Uri));

> +    NewDevicePath = AppendDevicePathNode (TmpDevicePath,

> +                      (EFI_DEVICE_PATH_PROTOCOL*) Node);

> +    FreePool (Node);

> +    FreePool (TmpDevicePath);

> +    if (NewDevicePath == NULL) {

> +      Status = EFI_OUT_OF_RESOURCES;

> +      goto FreeBootOptions;

> +    }

> +

> +    //

> +    // Create a new load option.

> +    //

> +    Status = EfiBootManagerInitializeLoadOption (&NewOption,

> +               LoadOptionNumberUnassigned, LoadOptionTypeBoot,

> +               LOAD_OPTION_ACTIVE, (CHAR16 *)mOsInstallers[Idx].Name,

> +               NewDevicePath, NULL, 0);

> +    ASSERT_EFI_ERROR (Status);

> +

> +    OptionIndex = EfiBootManagerFindLoadOption (&NewOption, BootOptions,

> +                    BootOptionCount);

> +    if (OptionIndex == -1) {

> +      //

> +      // Add the new load option if it did not exist already

> +      //

> +      EfiBootManagerAddLoadOptionVariable (&NewOption, (UINTN) -1);

> +    }

> +    EfiBootManagerFreeLoadOption (&NewOption);

> +    FreePool (NewDevicePath);

> +  }

> +

> +FreeBootOptions:

> +

> +  EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);

> +  return Status;

> +}

> +

> +STATIC

> +BOOLEAN

> +MediaDisconnected (

> +  IN  EFI_HANDLE    Handle

> +  )

> +{

> +  EFI_SIMPLE_NETWORK_PROTOCOL   *Snp;

> +  EFI_STATUS                    Status;

> +

> +  Status = gBS->HandleProtocol (Handle, &gEfiSimpleNetworkProtocolGuid,

> +                  (VOID **)&Snp);

> +  if (EFI_ERROR (Status) || !Snp->Mode->MediaPresentSupported) {

> +    return FALSE;

> +  }

> +

> +  Snp->GetStatus (Snp, NULL, NULL);

> +

> +  return !Snp->Mode->MediaPresent;

> +}

> +

> +STATIC

> +VOID

> +OnRegisterProtocol (

> +  IN EFI_EVENT  Event,

> +  IN VOID       *Context

> +  )

> +{

> +  EFI_STATUS                      Status;

> +  UINTN                           HandleCount;

> +  EFI_HANDLE                      *HandleBuffer;

> +  UINTN                           Idx;

> +

> +  Status = gBS->LocateHandleBuffer (ByRegisterNotify,

> +                  &gEfiHttpServiceBindingProtocolGuid, mRegistration,

> +                  &HandleCount, &HandleBuffer);

> +  if (EFI_ERROR (Status)) {

> +    return;

> +  }

> +

> +  for (Idx = 0; Idx < HandleCount; Idx++) {

> +    if (MediaDisconnected (HandleBuffer[Idx])) {

> +      continue;

> +    }

> +

> +    CreateOsInstallerBootOptions (HandleBuffer[Idx]);

> +

> +    //

> +    // Create the options only a single time - we take care to only install

> +    // them for a network interface that has a link, and we should try not to

> +    // confuse the user by having 10 identical options when the system has 10

> +    // network interfaces.

> +    //

> +    gBS->CloseEvent (Event);

> +    break;

> +  }

> +  FreePool (HandleBuffer);

> +}

> +

> +EFI_STATUS

> +EFIAPI

> +OsInstallerMenuDxeEntryPoint (

> +  IN EFI_HANDLE        ImageHandle,

> +  IN EFI_SYSTEM_TABLE  *SystemTable

> +  )

> +{

> +  mRegisterProtocolEvent = EfiCreateProtocolNotifyEvent (

> +                             &gEfiHttpServiceBindingProtocolGuid, TPL_CALLBACK,

> +                             OnRegisterProtocol, NULL, &mRegistration);

> +

> +  return EFI_SUCCESS;

> +}

> diff --git a/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf b/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf

> new file mode 100644

> index 000000000000..9bc5a81a78a1

> --- /dev/null

> +++ b/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf

> @@ -0,0 +1,42 @@

> +#

> +#  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.

> +#

> +#  This program and the accompanying materials are licensed and made available

> +#  under the terms and conditions of the BSD License which accompanies this

> +#  distribution.  The full text of the license may be found at

> +#  http://opensource.org/licenses/bsd-license.php

> +#

> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,

> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

> +#

> +

> +[Defines]

> +  INF_VERSION           = 0x00010019

> +  BASE_NAME             = OsInstallerMenuDxe

> +  FILE_GUID             = 4def5019-3233-4925-98ef-db5c60b2aec4

> +  MODULE_TYPE           = UEFI_DRIVER

> +  VERSION_STRING        = 0.1

> +  ENTRY_POINT           = OsInstallerMenuDxeEntryPoint

> +

> +[Sources]

> +  OsInstallerMenuDxe.c

> +

> +[Packages]

> +  MdePkg/MdePkg.dec

> +  MdeModulePkg/MdeModulePkg.dec

> +

> +[LibraryClasses]

> +  BaseLib

> +  BaseMemoryLib

> +  DebugLib

> +  DevicePathLib

> +  MemoryAllocationLib

> +  UefiBootManagerLib

> +  UefiBootServicesTableLib

> +  UefiDriverEntryPoint

> +  UefiLib

> +

> +[Protocols]

> +  gEfiHttpServiceBindingProtocolGuid

> +  gEfiDevicePathProtocolGuid

> +  gEfiSimpleNetworkProtocolGuid

> -- 

> 2.11.0

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Oct. 22, 2017, 3:16 p.m. | #2
On 22 October 2017 at 15:40, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Sat, Oct 21, 2017 at 02:10:49PM +0100, Ard Biesheuvel wrote:

>> To make it easier for power users to provision a 'desktop' system [as

>> opposed to a VM or server] from scratch, introduce a driver that adds

>> boot options to the boot menu that can launch network installers over

>> HTTP straight off the Internet.

>>

>> Currently, this only supports the 'mini.iso' style netboot installers

>> that Debian/Ubuntu provide: larger images that need to be mounted by

>> the installer when running under the Linux kernel are only supported

>> on ACPI systems with support for the NFIT table, and this was enabled

>> only recently (Linux v4.14) for arm64. For DT boot, there is currently

>> no way at all to expose ramdisks created by UEFI as block devices in

>> Linux.

>

> This is good stuff.

>

>> Contributed-under: TianoCore Contribution Agreement 1.1

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> ---

>>

>> Posted as an RFC because:

>> a) Where does this belong? Surely not in ArmPkg but I had to put it somewhere

>

> Not ArmPkg. EmbeddedPkg would also be wrong.

>

> It feels to me like it shares a common thread with the ACPI/DT

> switcher and the console preference widget.

>


... which should live where? the ACPI/DT switcher currently lives in
EmbeddedPkg, and actually, the serial console widget actually does
have an 'embedded' nature, so it would not seem out of place there.
But that is assuming you think EmbeddedPkg has a reason to exist in
the first place, and be something other than 'stuff that the Intel
guys don't want to touch'

>> b) Currently, the way the options are created results in them taking priority

>>    if no real boot options were set (i.e., for GRUB).

>> c) Is there a use case for downloading 300-500 MB installers off the Internet

>>    for a one shot installation? Or should we just stick to the mini.iso flavors.

>> d) Did I miss any distros we may care about?

>

> I think answers to all three of the above questions can end up being

> somewhat platform-specific. But I also think figuring out how that may

> best be implementented can wait.

>


I solved b) by using the LOAD_OPTION_CATEGORY_APP flag: that way, it
does appear in the 'Boot Manager' menu, but it is not launched
automatically.

> I could see a case here for a new package of UI tweaks and utilities.

> It would also not be unreasonable to add items like this under

> MdeModulePkg/{Application|Library}.

>


How about ValueAddPkg? :-)

>

>>  ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c   | 228 ++++++++++++++++++++

>>  ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf |  42 ++++

>>  2 files changed, 270 insertions(+)

>>

>> diff --git a/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c b/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c

>> new file mode 100644

>> index 000000000000..7c5934935924

>> --- /dev/null

>> +++ b/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c

>> @@ -0,0 +1,228 @@

>> +/** @file

>> + *

>> + *  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.

>> + *

>> + *  This program and the accompanying materials are licensed and made available

>> + *  under the terms and conditions of the BSD License which accompanies this

>> + *  distribution.  The full text of the license may be found at

>> + *  http://opensource.org/licenses/bsd-license.php

>> + *

>> + *  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,

>> + *  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

>> + */

>> +

>> +#include <Uefi.h>

>> +#include <Library/BaseLib.h>

>> +#include <Library/BaseMemoryLib.h>

>> +#include <Library/DebugLib.h>

>> +#include <Library/DevicePathLib.h>

>> +#include <Library/MemoryAllocationLib.h>

>> +#include <Library/UefiBootManagerLib.h>

>> +#include <Library/UefiBootServicesTableLib.h>

>> +#include <Library/UefiLib.h>

>> +#include <Protocol/SimpleNetwork.h>

>> +

>> +typedef struct {

>> +  CONST CHAR16  *Name;

>> +  CONST CHAR8   *Uri;

>> +} OS_INSTALLER_IMAGE;

>> +

>> +STATIC CONST OS_INSTALLER_IMAGE mOsInstallers[] = {

>> +  { L"Install Debian Stretch over HTTP",

>> +    "http://ftp.us.debian.org/debian/dists/stretch/main/installer-arm64/current/images/netboot/mini.iso" },

>> +

>> +  { L"Install Ubuntu 17.10 (Artful) over HTTP",

>> +    "http://ports.ubuntu.com/ubuntu-ports/dists/artful/main/installer-arm64/current/images/netboot/mini.iso" },

>> +

>> +//

>> +// The links below refer to 300-500 MB netboot images that need to be exposed to

>> +// the OS via a ramdisk after the OS loader boots the installer from it.

>> +// Currently, this requires ACPI/NFIT support, which was only enabled for arm64

>> +// in Linux in version v4.14. For DT boot, there is currently no solution for

>> +// this.

>> +//

>> +//  { L"Install openSUSE Tumbleweed over HTTP",

>> +//    "http://download.opensuse.org/ports/aarch64/factory/iso/openSUSE-Tumbleweed-NET-aarch64-Current.iso" },

>> +//

>> +//  { L"Install Fedora Server 26 over HTTP",

>> +//    "http://download.fedoraproject.org/pub/fedora-secondary/releases/26/Server/aarch64/iso/Fedora-Server-netinst-aarch64-26-1.5.iso" },

>> +//

>> +//  { L"Install Centos 7 over HTTP",

>> +//    "http://mirror.centos.org/altarch/7/isos/aarch64/CentOS-7-aarch64-NetInstall.iso" }

>> +//

>> +};

>> +

>> +STATIC EFI_EVENT                  mRegisterProtocolEvent;

>> +STATIC VOID                       *mRegistration;

>> +

>> +STATIC

>> +EFI_STATUS

>> +CreateOsInstallerBootOptions (

>> +  IN  EFI_HANDLE    Handle

>> +  )

>> +{

>> +  EFI_DEVICE_PATH_PROTOCOL        *ParentDevicePath;

>> +  EFI_DEVICE_PATH_PROTOCOL        *TmpDevicePath;

>> +  EFI_DEVICE_PATH_PROTOCOL        *NewDevicePath;

>> +  EFI_STATUS                      Status;

>> +  UINTN                           Idx;

>> +  EFI_DEV_PATH                    *Node;

>> +  UINTN                           Length;

>> +  EFI_BOOT_MANAGER_LOAD_OPTION    NewOption;

>> +  EFI_BOOT_MANAGER_LOAD_OPTION    *BootOptions;

>> +  UINTN                           BootOptionCount;

>> +  INTN                            OptionIndex;

>> +

>> +  BootOptions = EfiBootManagerGetLoadOptions (&BootOptionCount,

>> +                  LoadOptionTypeBoot);

>> +  ASSERT (BootOptions != NULL);

>> +

>> +  Status = gBS->HandleProtocol (Handle, &gEfiDevicePathProtocolGuid,

>> +                  (VOID **)&ParentDevicePath);

>> +  if (EFI_ERROR (Status)) {

>> +    DEBUG ((DEBUG_WARN, "%a: gBS->HandleProtocol returned %r\n",

>> +      __FUNCTION__, Status));

>> +    return Status;

>> +  }

>> +

>> +  for (Idx = 0; Idx < ARRAY_SIZE (mOsInstallers); Idx++) {

>> +    Node = AllocateZeroPool (sizeof (IPv4_DEVICE_PATH));

>> +    if (Node == NULL) {

>> +      Status = EFI_OUT_OF_RESOURCES;

>> +      goto FreeBootOptions;

>> +    }

>> +    Node->Ipv4.Header.Type    = MESSAGING_DEVICE_PATH;

>> +    Node->Ipv4.Header.SubType = MSG_IPv4_DP;

>> +    SetDevicePathNodeLength (Node, sizeof (IPv4_DEVICE_PATH));

>> +

>> +    TmpDevicePath = AppendDevicePathNode (ParentDevicePath,

>> +                      (EFI_DEVICE_PATH_PROTOCOL*) Node);

>> +    FreePool (Node);

>> +    if (TmpDevicePath == NULL) {

>> +      Status = EFI_OUT_OF_RESOURCES;

>> +      goto FreeBootOptions;

>> +    }

>> +

>> +    //

>> +    // Update the URI node with the input boot file URI.

>> +    //

>> +    Length = sizeof (EFI_DEVICE_PATH_PROTOCOL) +

>> +             AsciiStrSize (mOsInstallers[Idx].Uri);

>> +    Node = AllocatePool (Length);

>> +    if (Node == NULL) {

>> +      Status = EFI_OUT_OF_RESOURCES;

>> +      FreePool (TmpDevicePath);

>> +      goto FreeBootOptions;

>> +    }

>> +    Node->DevPath.Type    = MESSAGING_DEVICE_PATH;

>> +    Node->DevPath.SubType = MSG_URI_DP;

>> +    SetDevicePathNodeLength (Node, Length);

>> +    CopyMem ((UINT8*) Node + sizeof (EFI_DEVICE_PATH_PROTOCOL),

>> +      mOsInstallers[Idx].Uri, AsciiStrSize (mOsInstallers[Idx].Uri));

>> +    NewDevicePath = AppendDevicePathNode (TmpDevicePath,

>> +                      (EFI_DEVICE_PATH_PROTOCOL*) Node);

>> +    FreePool (Node);

>> +    FreePool (TmpDevicePath);

>> +    if (NewDevicePath == NULL) {

>> +      Status = EFI_OUT_OF_RESOURCES;

>> +      goto FreeBootOptions;

>> +    }

>> +

>> +    //

>> +    // Create a new load option.

>> +    //

>> +    Status = EfiBootManagerInitializeLoadOption (&NewOption,

>> +               LoadOptionNumberUnassigned, LoadOptionTypeBoot,

>> +               LOAD_OPTION_ACTIVE, (CHAR16 *)mOsInstallers[Idx].Name,

>> +               NewDevicePath, NULL, 0);

>> +    ASSERT_EFI_ERROR (Status);

>> +

>> +    OptionIndex = EfiBootManagerFindLoadOption (&NewOption, BootOptions,

>> +                    BootOptionCount);

>> +    if (OptionIndex == -1) {

>> +      //

>> +      // Add the new load option if it did not exist already

>> +      //

>> +      EfiBootManagerAddLoadOptionVariable (&NewOption, (UINTN) -1);

>> +    }

>> +    EfiBootManagerFreeLoadOption (&NewOption);

>> +    FreePool (NewDevicePath);

>> +  }

>> +

>> +FreeBootOptions:

>> +

>> +  EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);

>> +  return Status;

>> +}

>> +

>> +STATIC

>> +BOOLEAN

>> +MediaDisconnected (

>> +  IN  EFI_HANDLE    Handle

>> +  )

>> +{

>> +  EFI_SIMPLE_NETWORK_PROTOCOL   *Snp;

>> +  EFI_STATUS                    Status;

>> +

>> +  Status = gBS->HandleProtocol (Handle, &gEfiSimpleNetworkProtocolGuid,

>> +                  (VOID **)&Snp);

>> +  if (EFI_ERROR (Status) || !Snp->Mode->MediaPresentSupported) {

>> +    return FALSE;

>> +  }

>> +

>> +  Snp->GetStatus (Snp, NULL, NULL);

>> +

>> +  return !Snp->Mode->MediaPresent;

>> +}

>> +

>> +STATIC

>> +VOID

>> +OnRegisterProtocol (

>> +  IN EFI_EVENT  Event,

>> +  IN VOID       *Context

>> +  )

>> +{

>> +  EFI_STATUS                      Status;

>> +  UINTN                           HandleCount;

>> +  EFI_HANDLE                      *HandleBuffer;

>> +  UINTN                           Idx;

>> +

>> +  Status = gBS->LocateHandleBuffer (ByRegisterNotify,

>> +                  &gEfiHttpServiceBindingProtocolGuid, mRegistration,

>> +                  &HandleCount, &HandleBuffer);

>> +  if (EFI_ERROR (Status)) {

>> +    return;

>> +  }

>> +

>> +  for (Idx = 0; Idx < HandleCount; Idx++) {

>> +    if (MediaDisconnected (HandleBuffer[Idx])) {

>> +      continue;

>> +    }

>> +

>> +    CreateOsInstallerBootOptions (HandleBuffer[Idx]);

>> +

>> +    //

>> +    // Create the options only a single time - we take care to only install

>> +    // them for a network interface that has a link, and we should try not to

>> +    // confuse the user by having 10 identical options when the system has 10

>> +    // network interfaces.

>> +    //

>> +    gBS->CloseEvent (Event);

>> +    break;

>> +  }

>> +  FreePool (HandleBuffer);

>> +}

>> +

>> +EFI_STATUS

>> +EFIAPI

>> +OsInstallerMenuDxeEntryPoint (

>> +  IN EFI_HANDLE        ImageHandle,

>> +  IN EFI_SYSTEM_TABLE  *SystemTable

>> +  )

>> +{

>> +  mRegisterProtocolEvent = EfiCreateProtocolNotifyEvent (

>> +                             &gEfiHttpServiceBindingProtocolGuid, TPL_CALLBACK,

>> +                             OnRegisterProtocol, NULL, &mRegistration);

>> +

>> +  return EFI_SUCCESS;

>> +}

>> diff --git a/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf b/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf

>> new file mode 100644

>> index 000000000000..9bc5a81a78a1

>> --- /dev/null

>> +++ b/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf

>> @@ -0,0 +1,42 @@

>> +#

>> +#  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.

>> +#

>> +#  This program and the accompanying materials are licensed and made available

>> +#  under the terms and conditions of the BSD License which accompanies this

>> +#  distribution.  The full text of the license may be found at

>> +#  http://opensource.org/licenses/bsd-license.php

>> +#

>> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,

>> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

>> +#

>> +

>> +[Defines]

>> +  INF_VERSION           = 0x00010019

>> +  BASE_NAME             = OsInstallerMenuDxe

>> +  FILE_GUID             = 4def5019-3233-4925-98ef-db5c60b2aec4

>> +  MODULE_TYPE           = UEFI_DRIVER

>> +  VERSION_STRING        = 0.1

>> +  ENTRY_POINT           = OsInstallerMenuDxeEntryPoint

>> +

>> +[Sources]

>> +  OsInstallerMenuDxe.c

>> +

>> +[Packages]

>> +  MdePkg/MdePkg.dec

>> +  MdeModulePkg/MdeModulePkg.dec

>> +

>> +[LibraryClasses]

>> +  BaseLib

>> +  BaseMemoryLib

>> +  DebugLib

>> +  DevicePathLib

>> +  MemoryAllocationLib

>> +  UefiBootManagerLib

>> +  UefiBootServicesTableLib

>> +  UefiDriverEntryPoint

>> +  UefiLib

>> +

>> +[Protocols]

>> +  gEfiHttpServiceBindingProtocolGuid

>> +  gEfiDevicePathProtocolGuid

>> +  gEfiSimpleNetworkProtocolGuid

>> --

>> 2.11.0

>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Oct. 24, 2017, 12:10 p.m. | #3
On 10/21/17 15:10, Ard Biesheuvel wrote:
> To make it easier for power users to provision a 'desktop' system [as

> opposed to a VM or server] from scratch, introduce a driver that adds

> boot options to the boot menu that can launch network installers over

> HTTP straight off the Internet.

> 

> Currently, this only supports the 'mini.iso' style netboot installers

> that Debian/Ubuntu provide: larger images that need to be mounted by

> the installer when running under the Linux kernel are only supported

> on ACPI systems with support for the NFIT table, and this was enabled

> only recently (Linux v4.14) for arm64. For DT boot, there is currently

> no way at all to expose ramdisks created by UEFI as block devices in

> Linux.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

> 

> Posted as an RFC because:

> a) Where does this belong? Surely not in ArmPkg but I had to put it somewhere

> b) Currently, the way the options are created results in them taking priority

>    if no real boot options were set (i.e., for GRUB).

> c) Is there a use case for downloading 300-500 MB installers off the Internet

>    for a one shot installation? Or should we just stick to the mini.iso flavors.

> d) Did I miss any distros we may care about?

> 

>  ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c   | 228 ++++++++++++++++++++

>  ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf |  42 ++++

>  2 files changed, 270 insertions(+)


https://github.com/tianocore/tianocore.github.io/wiki/HTTP-Boot

If the HTTP Boot stack is built into the firmware, it is possible to
navigate to:

  Device Manager
    Network Device List
      MAC:xx:xx:xx:xx:xx:xx
        HTTP Boot Configuration

On this dialog, we have
- "Input the description"
- "Internet protocol"
- "Boot URI"

The help text says, "A new Boot Option will be created according to this
Boot URI".

Therefore, I think the functionality already exists (for power users
that are willing to pick a NIC and to type a URL, anyway).

Is the goal of your patch to provide more convenience, or to provide the
base functionality?

Also, I think that creating baked-in boot options belongs with the
platform's PlatformBootManagerLib instance, not a separate UEFI_DRIVER
module. (That's where the UEFI shell boot option is created already, for
example.) Platform BDS is responsible for connecting NICs (none of them,
some of them, all of them -- and recursively or non-recursively), so if
you want to look at specific NIC-related protocol instances, you know
exactly when to look.

If the goal is really to save the user the typing of one URL, then I
suggest to implement this feature as a small, standalone library, under
edk2-platforms; with a sole API called CreateOsInstallerBootOptions().

Then, interested platforms may invoke CreateOsInstallerBootOptions() in
their PlatformBootManagerLib instances.

(This would be similar to:

  OvmfPkg/Include/Library/QemuBootOrderLib.h
  OvmfPkg/Library/QemuBootOrderLib/

and how we call SetBootOrderFromQemu() in ArmVirtPkg's and OvmfPkg's
PlatformBootManagerLib instances.)

Perhaps you can introduce the lib class, with a Null instance, in edk2
as well; and then call the new API in

  ArmPkg/Library/PlatformBootManagerLib

at the right spot. (I think adding the API / lib class to edk2 makes
sense, but I also think the lib instance with the actual URLs should
live under edk2-platforms, somewhere.)

Just my two cents.

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Oct. 24, 2017, 12:36 p.m. | #4
On Tue, Oct 24, 2017 at 02:10:44PM +0200, Laszlo Ersek wrote:
> On 10/21/17 15:10, Ard Biesheuvel wrote:

> > To make it easier for power users to provision a 'desktop' system [as

> > opposed to a VM or server] from scratch, introduce a driver that adds

> > boot options to the boot menu that can launch network installers over

> > HTTP straight off the Internet.

> > 

> > Currently, this only supports the 'mini.iso' style netboot installers

> > that Debian/Ubuntu provide: larger images that need to be mounted by

> > the installer when running under the Linux kernel are only supported

> > on ACPI systems with support for the NFIT table, and this was enabled

> > only recently (Linux v4.14) for arm64. For DT boot, there is currently

> > no way at all to expose ramdisks created by UEFI as block devices in

> > Linux.

> > 

> > Contributed-under: TianoCore Contribution Agreement 1.1

> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> > ---

> > 

> > Posted as an RFC because:

> > a) Where does this belong? Surely not in ArmPkg but I had to put it somewhere

> > b) Currently, the way the options are created results in them taking priority

> >    if no real boot options were set (i.e., for GRUB).

> > c) Is there a use case for downloading 300-500 MB installers off the Internet

> >    for a one shot installation? Or should we just stick to the mini.iso flavors.

> > d) Did I miss any distros we may care about?

> > 

> >  ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c   | 228 ++++++++++++++++++++

> >  ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf |  42 ++++

> >  2 files changed, 270 insertions(+)

> 

> https://github.com/tianocore/tianocore.github.io/wiki/HTTP-Boot

> 

> If the HTTP Boot stack is built into the firmware, it is possible to

> navigate to:

> 

>   Device Manager

>     Network Device List

>       MAC:xx:xx:xx:xx:xx:xx

>         HTTP Boot Configuration

> 

> On this dialog, we have

> - "Input the description"

> - "Internet protocol"

> - "Boot URI"

> 

> The help text says, "A new Boot Option will be created according to this

> Boot URI".

> 

> Therefore, I think the functionality already exists (for power users

> that are willing to pick a NIC and to type a URL, anyway).

> 

> Is the goal of your patch to provide more convenience, or to provide the

> base functionality?


Convenience. And given that I only noticed yesterday that the boot
fails on HTTP redirects, of somewhat restricted value.

Is that an official policy decision, or just a restriction of the
implementation?

> Also, I think that creating baked-in boot options belongs with the

> platform's PlatformBootManagerLib instance, not a separate UEFI_DRIVER

> module. (That's where the UEFI shell boot option is created already, for

> example.) Platform BDS is responsible for connecting NICs (none of them,

> some of them, all of them -- and recursively or non-recursively), so if

> you want to look at specific NIC-related protocol instances, you know

> exactly when to look.

> 

> If the goal is really to save the user the typing of one URL, then I

> suggest to implement this feature as a small, standalone library, under

> edk2-platforms; with a sole API called CreateOsInstallerBootOptions().

> 

> Then, interested platforms may invoke CreateOsInstallerBootOptions() in

> their PlatformBootManagerLib instances.

> 

> (This would be similar to:

> 

>   OvmfPkg/Include/Library/QemuBootOrderLib.h

>   OvmfPkg/Library/QemuBootOrderLib/

> 

> and how we call SetBootOrderFromQemu() in ArmVirtPkg's and OvmfPkg's

> PlatformBootManagerLib instances.)

> 

> Perhaps you can introduce the lib class, with a Null instance, in edk2

> as well; and then call the new API in

> 

>   ArmPkg/Library/PlatformBootManagerLib

>

> at the right spot. (I think adding the API / lib class to edk2 makes

> sense, but I also think the lib instance with the actual URLs should

> live under edk2-platforms, somewhere.)

> 

> Just my two cents.


These were exactly the kinds of things I meant about "how best to
implement" (without trying to answer them myself).

Regards,

Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Oct. 24, 2017, 1:32 p.m. | #5
On 10/24/17 14:36, Leif Lindholm wrote:
> On Tue, Oct 24, 2017 at 02:10:44PM +0200, Laszlo Ersek wrote:

>> On 10/21/17 15:10, Ard Biesheuvel wrote:

>>> To make it easier for power users to provision a 'desktop' system [as

>>> opposed to a VM or server] from scratch, introduce a driver that adds

>>> boot options to the boot menu that can launch network installers over

>>> HTTP straight off the Internet.

>>>

>>> Currently, this only supports the 'mini.iso' style netboot installers

>>> that Debian/Ubuntu provide: larger images that need to be mounted by

>>> the installer when running under the Linux kernel are only supported

>>> on ACPI systems with support for the NFIT table, and this was enabled

>>> only recently (Linux v4.14) for arm64. For DT boot, there is currently

>>> no way at all to expose ramdisks created by UEFI as block devices in

>>> Linux.

>>>

>>> Contributed-under: TianoCore Contribution Agreement 1.1

>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>>> ---

>>>

>>> Posted as an RFC because:

>>> a) Where does this belong? Surely not in ArmPkg but I had to put it somewhere

>>> b) Currently, the way the options are created results in them taking priority

>>>    if no real boot options were set (i.e., for GRUB).

>>> c) Is there a use case for downloading 300-500 MB installers off the Internet

>>>    for a one shot installation? Or should we just stick to the mini.iso flavors.

>>> d) Did I miss any distros we may care about?

>>>

>>>  ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c   | 228 ++++++++++++++++++++

>>>  ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf |  42 ++++

>>>  2 files changed, 270 insertions(+)

>>

>> https://github.com/tianocore/tianocore.github.io/wiki/HTTP-Boot

>>

>> If the HTTP Boot stack is built into the firmware, it is possible to

>> navigate to:

>>

>>   Device Manager

>>     Network Device List

>>       MAC:xx:xx:xx:xx:xx:xx

>>         HTTP Boot Configuration

>>

>> On this dialog, we have

>> - "Input the description"

>> - "Internet protocol"

>> - "Boot URI"

>>

>> The help text says, "A new Boot Option will be created according to this

>> Boot URI".

>>

>> Therefore, I think the functionality already exists (for power users

>> that are willing to pick a NIC and to type a URL, anyway).

>>

>> Is the goal of your patch to provide more convenience, or to provide the

>> base functionality?

> 

> Convenience. And given that I only noticed yesterday that the boot

> fails on HTTP redirects, of somewhat restricted value.

> 

> Is that an official policy decision, or just a restriction of the

> implementation?


Hm, I'm unsure; the following seems to imply that HttpBootDxe intends to handle redirects:

        if (HttpBootIsHttpRedirectStatusCode (HttpMessage->Data.Response->StatusCode)) {
          //
          // Server indicates the resource has been redirected to a different URL
          // according to the section 6.4 of RFC7231 and the RFC 7538.
          // Display the redirect information on the screen.
          //
          HttpHeader = HttpFindHeader (
                 HttpMessage->HeaderCount,
                 HttpMessage->Headers,
                 HTTP_HEADER_LOCATION
                 );
          if (HttpHeader != NULL) {
            Print (L"\n  HTTP ERROR: Resource Redirected.\n  New Location: %a\n", HttpHeader->FieldValue);
          }

Can you perhaps capture a packet trace and discuss it with Siyuan and Jiaxin? Perhaps the server returns a status code that is not handled by HttpBootDxe right now.

Thanks
Laszlo

> 

>> Also, I think that creating baked-in boot options belongs with the

>> platform's PlatformBootManagerLib instance, not a separate UEFI_DRIVER

>> module. (That's where the UEFI shell boot option is created already, for

>> example.) Platform BDS is responsible for connecting NICs (none of them,

>> some of them, all of them -- and recursively or non-recursively), so if

>> you want to look at specific NIC-related protocol instances, you know

>> exactly when to look.

>>

>> If the goal is really to save the user the typing of one URL, then I

>> suggest to implement this feature as a small, standalone library, under

>> edk2-platforms; with a sole API called CreateOsInstallerBootOptions().

>>

>> Then, interested platforms may invoke CreateOsInstallerBootOptions() in

>> their PlatformBootManagerLib instances.

>>

>> (This would be similar to:

>>

>>   OvmfPkg/Include/Library/QemuBootOrderLib.h

>>   OvmfPkg/Library/QemuBootOrderLib/

>>

>> and how we call SetBootOrderFromQemu() in ArmVirtPkg's and OvmfPkg's

>> PlatformBootManagerLib instances.)

>>

>> Perhaps you can introduce the lib class, with a Null instance, in edk2

>> as well; and then call the new API in

>>

>>   ArmPkg/Library/PlatformBootManagerLib

>>

>> at the right spot. (I think adding the API / lib class to edk2 makes

>> sense, but I also think the lib instance with the actual URLs should

>> live under edk2-platforms, somewhere.)

>>

>> Just my two cents.

> 

> These were exactly the kinds of things I meant about "how best to

> implement" (without trying to answer them myself).

> 

> Regards,

> 

> Leif

> _______________________________________________

> edk2-devel mailing list

> edk2-devel@lists.01.org

> https://lists.01.org/mailman/listinfo/edk2-devel

> 


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Oct. 24, 2017, 2:44 p.m. | #6
On Tue, Oct 24, 2017 at 03:32:51PM +0200, Laszlo Ersek wrote:
> > Convenience. And given that I only noticed yesterday that the boot

> > fails on HTTP redirects, of somewhat restricted value.

> > 

> > Is that an official policy decision, or just a restriction of the

> > implementation?

> 

> Hm, I'm unsure; the following seems to imply that HttpBootDxe intends to handle redirects:

> 

>         if (HttpBootIsHttpRedirectStatusCode (HttpMessage->Data.Response->StatusCode)) {

>           //

>           // Server indicates the resource has been redirected to a different URL

>           // according to the section 6.4 of RFC7231 and the RFC 7538.

>           // Display the redirect information on the screen.

>           //

>           HttpHeader = HttpFindHeader (

>                  HttpMessage->HeaderCount,

>                  HttpMessage->Headers,

>                  HTTP_HEADER_LOCATION

>                  );

>           if (HttpHeader != NULL) {

>             Print (L"\n  HTTP ERROR: Resource Redirected.\n  New Location: %a\n", HttpHeader->FieldValue);

>           }

> 

> Can you perhaps capture a packet trace and discuss it with Siyuan

> and Jiaxin? Perhaps the server returns a status code that is not

> handled by HttpBootDxe right now.


Oh, it's explicitly printed as a 302 (Found), detected by
HttpBootDxe/HttpBootSupport.c:HttpBootPrintErrorMessage().

See https://www.mail-archive.com/edk2-devel@lists.01.org/msg27819.html

But yes, I should have cc:d Siyuan and Jiaxin (and have now done so).
Can you comment?

Regards,

Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Fu, Siyuan Oct. 25, 2017, 2:18 a.m. | #7
Hi, Leif and Laszlo

The HTTP restriction is actually not supported now. If server replies a HTTP redirect response, the current HTTP boot driver will only print new URL address on the screen then abort the HTTP boot. It won't attempt to download the image from the new address.

BestRegards
Fu Siyuan


> -----Original Message-----

> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]

> Sent: Tuesday, October 24, 2017 10:44 PM

> To: Laszlo Ersek <lersek@redhat.com>

> Cc: edk2-devel@lists.01.org; daniel.thompson@linaro.org; Ard Biesheuvel

> <ard.biesheuvel@linaro.org>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin

> <jiaxin.wu@intel.com>

> Subject: Re: [edk2] [RFC PATCH] ArmPkg: add driver to add distro installer

> HTTP boot options

> 

> On Tue, Oct 24, 2017 at 03:32:51PM +0200, Laszlo Ersek wrote:

> > > Convenience. And given that I only noticed yesterday that the boot

> > > fails on HTTP redirects, of somewhat restricted value.

> > >

> > > Is that an official policy decision, or just a restriction of the

> > > implementation?

> >

> > Hm, I'm unsure; the following seems to imply that HttpBootDxe intends to

> handle redirects:

> >

> >         if (HttpBootIsHttpRedirectStatusCode (HttpMessage-

> >Data.Response->StatusCode)) {

> >           //

> >           // Server indicates the resource has been redirected to a

> different URL

> >           // according to the section 6.4 of RFC7231 and the RFC 7538.

> >           // Display the redirect information on the screen.

> >           //

> >           HttpHeader = HttpFindHeader (

> >                  HttpMessage->HeaderCount,

> >                  HttpMessage->Headers,

> >                  HTTP_HEADER_LOCATION

> >                  );

> >           if (HttpHeader != NULL) {

> >             Print (L"\n  HTTP ERROR: Resource Redirected.\n  New

> Location: %a\n", HttpHeader->FieldValue);

> >           }

> >

> > Can you perhaps capture a packet trace and discuss it with Siyuan

> > and Jiaxin? Perhaps the server returns a status code that is not

> > handled by HttpBootDxe right now.

> 

> Oh, it's explicitly printed as a 302 (Found), detected by

> HttpBootDxe/HttpBootSupport.c:HttpBootPrintErrorMessage().

> 

> See https://www.mail-archive.com/edk2-devel@lists.01.org/msg27819.html

> 

> But yes, I should have cc:d Siyuan and Jiaxin (and have now done so).

> Can you comment?

> 

> Regards,

> 

> Leif

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Oct. 25, 2017, 8:37 a.m. | #8
On Wed, Oct 25, 2017 at 02:18:38AM +0000, Fu, Siyuan wrote:
> Hi, Leif and Laszlo

> 

> The HTTP restriction is actually not supported now. If server

> replies a HTTP redirect response, the current HTTP boot driver will

> only print new URL address on the screen then abort the HTTP

> boot. It won't attempt to download the image from the new address.


Yes we noticed that :)
The question was if that was an intentional restriction, or just
something that has not been implemented yet.

Currently this is causing issues for example when attempting to
download an image hosted in some sort of CDN.

Best Regards,

Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Fu, Siyuan Oct. 25, 2017, 8:55 a.m. | #9
Hi, Leif

We don't have plan to support redirection now, largely because we haven't receive any requirement for this feature before.


BestRegards
Fu Siyuan


> -----Original Message-----

> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]

> Sent: Wednesday, October 25, 2017 4:38 PM

> To: Fu, Siyuan <siyuan.fu@intel.com>

> Cc: Laszlo Ersek <lersek@redhat.com>; edk2-devel@lists.01.org;

> daniel.thompson@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Wu,

> Jiaxin <jiaxin.wu@intel.com>

> Subject: Re: [edk2] [RFC PATCH] ArmPkg: add driver to add distro installer

> HTTP boot options

> 

> On Wed, Oct 25, 2017 at 02:18:38AM +0000, Fu, Siyuan wrote:

> > Hi, Leif and Laszlo

> >

> > The HTTP restriction is actually not supported now. If server

> > replies a HTTP redirect response, the current HTTP boot driver will

> > only print new URL address on the screen then abort the HTTP

> > boot. It won't attempt to download the image from the new address.

> 

> Yes we noticed that :)

> The question was if that was an intentional restriction, or just

> something that has not been implemented yet.

> 

> Currently this is causing issues for example when attempting to

> download an image hosted in some sort of CDN.

> 

> Best Regards,

> 

> Leif

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c b/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c
new file mode 100644
index 000000000000..7c5934935924
--- /dev/null
+++ b/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.c
@@ -0,0 +1,228 @@ 
+/** @file
+ *
+ *  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.
+ *
+ *  This program and the accompanying materials are licensed and made available
+ *  under the terms and conditions of the BSD License which accompanies this
+ *  distribution.  The full text of the license may be found at
+ *  http://opensource.org/licenses/bsd-license.php
+ *
+ *  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+ */
+
+#include <Uefi.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootManagerLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+#include <Protocol/SimpleNetwork.h>
+
+typedef struct {
+  CONST CHAR16  *Name;
+  CONST CHAR8   *Uri;
+} OS_INSTALLER_IMAGE;
+
+STATIC CONST OS_INSTALLER_IMAGE mOsInstallers[] = {
+  { L"Install Debian Stretch over HTTP",
+    "http://ftp.us.debian.org/debian/dists/stretch/main/installer-arm64/current/images/netboot/mini.iso" },
+
+  { L"Install Ubuntu 17.10 (Artful) over HTTP",
+    "http://ports.ubuntu.com/ubuntu-ports/dists/artful/main/installer-arm64/current/images/netboot/mini.iso" },
+
+//
+// The links below refer to 300-500 MB netboot images that need to be exposed to
+// the OS via a ramdisk after the OS loader boots the installer from it.
+// Currently, this requires ACPI/NFIT support, which was only enabled for arm64
+// in Linux in version v4.14. For DT boot, there is currently no solution for
+// this.
+//
+//  { L"Install openSUSE Tumbleweed over HTTP",
+//    "http://download.opensuse.org/ports/aarch64/factory/iso/openSUSE-Tumbleweed-NET-aarch64-Current.iso" },
+//
+//  { L"Install Fedora Server 26 over HTTP",
+//    "http://download.fedoraproject.org/pub/fedora-secondary/releases/26/Server/aarch64/iso/Fedora-Server-netinst-aarch64-26-1.5.iso" },
+//
+//  { L"Install Centos 7 over HTTP",
+//    "http://mirror.centos.org/altarch/7/isos/aarch64/CentOS-7-aarch64-NetInstall.iso" }
+//
+};
+
+STATIC EFI_EVENT                  mRegisterProtocolEvent;
+STATIC VOID                       *mRegistration;
+
+STATIC
+EFI_STATUS
+CreateOsInstallerBootOptions (
+  IN  EFI_HANDLE    Handle
+  )
+{
+  EFI_DEVICE_PATH_PROTOCOL        *ParentDevicePath;
+  EFI_DEVICE_PATH_PROTOCOL        *TmpDevicePath;
+  EFI_DEVICE_PATH_PROTOCOL        *NewDevicePath;
+  EFI_STATUS                      Status;
+  UINTN                           Idx;
+  EFI_DEV_PATH                    *Node;
+  UINTN                           Length;
+  EFI_BOOT_MANAGER_LOAD_OPTION    NewOption;
+  EFI_BOOT_MANAGER_LOAD_OPTION    *BootOptions;
+  UINTN                           BootOptionCount;
+  INTN                            OptionIndex;
+
+  BootOptions = EfiBootManagerGetLoadOptions (&BootOptionCount,
+                  LoadOptionTypeBoot);
+  ASSERT (BootOptions != NULL);
+
+  Status = gBS->HandleProtocol (Handle, &gEfiDevicePathProtocolGuid,
+                  (VOID **)&ParentDevicePath);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_WARN, "%a: gBS->HandleProtocol returned %r\n",
+      __FUNCTION__, Status));
+    return Status;
+  }
+
+  for (Idx = 0; Idx < ARRAY_SIZE (mOsInstallers); Idx++) {
+    Node = AllocateZeroPool (sizeof (IPv4_DEVICE_PATH));
+    if (Node == NULL) {
+      Status = EFI_OUT_OF_RESOURCES;
+      goto FreeBootOptions;
+    }
+    Node->Ipv4.Header.Type    = MESSAGING_DEVICE_PATH;
+    Node->Ipv4.Header.SubType = MSG_IPv4_DP;
+    SetDevicePathNodeLength (Node, sizeof (IPv4_DEVICE_PATH));
+
+    TmpDevicePath = AppendDevicePathNode (ParentDevicePath,
+                      (EFI_DEVICE_PATH_PROTOCOL*) Node);
+    FreePool (Node);
+    if (TmpDevicePath == NULL) {
+      Status = EFI_OUT_OF_RESOURCES;
+      goto FreeBootOptions;
+    }
+
+    //
+    // Update the URI node with the input boot file URI.
+    //
+    Length = sizeof (EFI_DEVICE_PATH_PROTOCOL) +
+             AsciiStrSize (mOsInstallers[Idx].Uri);
+    Node = AllocatePool (Length);
+    if (Node == NULL) {
+      Status = EFI_OUT_OF_RESOURCES;
+      FreePool (TmpDevicePath);
+      goto FreeBootOptions;
+    }
+    Node->DevPath.Type    = MESSAGING_DEVICE_PATH;
+    Node->DevPath.SubType = MSG_URI_DP;
+    SetDevicePathNodeLength (Node, Length);
+    CopyMem ((UINT8*) Node + sizeof (EFI_DEVICE_PATH_PROTOCOL),
+      mOsInstallers[Idx].Uri, AsciiStrSize (mOsInstallers[Idx].Uri));
+    NewDevicePath = AppendDevicePathNode (TmpDevicePath,
+                      (EFI_DEVICE_PATH_PROTOCOL*) Node);
+    FreePool (Node);
+    FreePool (TmpDevicePath);
+    if (NewDevicePath == NULL) {
+      Status = EFI_OUT_OF_RESOURCES;
+      goto FreeBootOptions;
+    }
+
+    //
+    // Create a new load option.
+    //
+    Status = EfiBootManagerInitializeLoadOption (&NewOption,
+               LoadOptionNumberUnassigned, LoadOptionTypeBoot,
+               LOAD_OPTION_ACTIVE, (CHAR16 *)mOsInstallers[Idx].Name,
+               NewDevicePath, NULL, 0);
+    ASSERT_EFI_ERROR (Status);
+
+    OptionIndex = EfiBootManagerFindLoadOption (&NewOption, BootOptions,
+                    BootOptionCount);
+    if (OptionIndex == -1) {
+      //
+      // Add the new load option if it did not exist already
+      //
+      EfiBootManagerAddLoadOptionVariable (&NewOption, (UINTN) -1);
+    }
+    EfiBootManagerFreeLoadOption (&NewOption);
+    FreePool (NewDevicePath);
+  }
+
+FreeBootOptions:
+
+  EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
+  return Status;
+}
+
+STATIC
+BOOLEAN
+MediaDisconnected (
+  IN  EFI_HANDLE    Handle
+  )
+{
+  EFI_SIMPLE_NETWORK_PROTOCOL   *Snp;
+  EFI_STATUS                    Status;
+
+  Status = gBS->HandleProtocol (Handle, &gEfiSimpleNetworkProtocolGuid,
+                  (VOID **)&Snp);
+  if (EFI_ERROR (Status) || !Snp->Mode->MediaPresentSupported) {
+    return FALSE;
+  }
+
+  Snp->GetStatus (Snp, NULL, NULL);
+
+  return !Snp->Mode->MediaPresent;
+}
+
+STATIC
+VOID
+OnRegisterProtocol (
+  IN EFI_EVENT  Event,
+  IN VOID       *Context
+  )
+{
+  EFI_STATUS                      Status;
+  UINTN                           HandleCount;
+  EFI_HANDLE                      *HandleBuffer;
+  UINTN                           Idx;
+
+  Status = gBS->LocateHandleBuffer (ByRegisterNotify,
+                  &gEfiHttpServiceBindingProtocolGuid, mRegistration,
+                  &HandleCount, &HandleBuffer);
+  if (EFI_ERROR (Status)) {
+    return;
+  }
+
+  for (Idx = 0; Idx < HandleCount; Idx++) {
+    if (MediaDisconnected (HandleBuffer[Idx])) {
+      continue;
+    }
+
+    CreateOsInstallerBootOptions (HandleBuffer[Idx]);
+
+    //
+    // Create the options only a single time - we take care to only install
+    // them for a network interface that has a link, and we should try not to
+    // confuse the user by having 10 identical options when the system has 10
+    // network interfaces.
+    //
+    gBS->CloseEvent (Event);
+    break;
+  }
+  FreePool (HandleBuffer);
+}
+
+EFI_STATUS
+EFIAPI
+OsInstallerMenuDxeEntryPoint (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  mRegisterProtocolEvent = EfiCreateProtocolNotifyEvent (
+                             &gEfiHttpServiceBindingProtocolGuid, TPL_CALLBACK,
+                             OnRegisterProtocol, NULL, &mRegistration);
+
+  return EFI_SUCCESS;
+}
diff --git a/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf b/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf
new file mode 100644
index 000000000000..9bc5a81a78a1
--- /dev/null
+++ b/ArmPkg/Drivers/OsInstallerMenuDxe/OsInstallerMenuDxe.inf
@@ -0,0 +1,42 @@ 
+#
+#  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.
+#
+#  This program and the accompanying materials are licensed and made available
+#  under the terms and conditions of the BSD License which accompanies this
+#  distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+
+[Defines]
+  INF_VERSION           = 0x00010019
+  BASE_NAME             = OsInstallerMenuDxe
+  FILE_GUID             = 4def5019-3233-4925-98ef-db5c60b2aec4
+  MODULE_TYPE           = UEFI_DRIVER
+  VERSION_STRING        = 0.1
+  ENTRY_POINT           = OsInstallerMenuDxeEntryPoint
+
+[Sources]
+  OsInstallerMenuDxe.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  DevicePathLib
+  MemoryAllocationLib
+  UefiBootManagerLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  UefiLib
+
+[Protocols]
+  gEfiHttpServiceBindingProtocolGuid
+  gEfiDevicePathProtocolGuid
+  gEfiSimpleNetworkProtocolGuid