Re: [edk2-devel] [PATCH v1 6/6] UefiPayloadPkg: LinuxBoot: use a text format for the configuration block.

[Cheng-Chieh Huang <chengchieh@google.com>]

[-- Attachment #1: Type: text/plain, Size: 15334 bytes --]

Here is the documentation:
https://github.com/osresearch/kexec-load/blob/main/README.md

On Thu, Jul 22, 2021 at 1:09 AM Marvin Häuser <mhaeuser@posteo.de> wrote:

>
>
> On 21.07.21 15:23, Cheng-Chieh Huang via groups.io wrote:
> > From: Trammell Hudson <hudson@trmm.net>
> >
> > This adds a text command line for the UefiPayloadPkg that uses
> > a textual magic number 'LnxBoot1' and a series of white-space
> > separated key=value[,value...] pairs for the parameters.
> >
> > The v1 binary configuration structure is used if it is present instead
> > for backwards compatability.
> >
> > Currently supported text command line arguments are are:
> >
> > * `serial=baud,base,width,type,hz,pci`
> > (only `baud` needs to be specified, the rest have reasonable
> > defaults)
> >
> > * `mem=start,end,type`
> > Types are based on `/sys/firmware/memmaps/*/types`:
> >      1 == "System RAM"
> >      3 == "ACPI Tables"
> >      4 == "ACPI Non-volatile Storage"
> >      5 == "Reserved"
> >
> > * `ACPI20=base` pointer to RDSP (from `/sys/firmware/efi/systab`)
> >
> > * `SMBIOS=base` pointer to the SMBIOS table (also from the EFI table)
> >
> > Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
> > ---
> >   UefiPayloadPkg/Include/Linuxboot.h             |  17 +-
> >   UefiPayloadPkg/Library/LbParseLib/LbParseLib.c | 252
> +++++++++++++++++---
> >   2 files changed, 230 insertions(+), 39 deletions(-)
> >
> > diff --git a/UefiPayloadPkg/Include/Linuxboot.h
> b/UefiPayloadPkg/Include/Linuxboot.h
> > index 34ca18069983..56b39b5a09ff 100644
> > --- a/UefiPayloadPkg/Include/Linuxboot.h
> > +++ b/UefiPayloadPkg/Include/Linuxboot.h
> > @@ -24,8 +24,7 @@ typedef struct MemoryMapEntryStruct {
> >     UINT32 Type;
> >   } MemoryMapEntry;
> >
> > -typedef struct UefiPayloadConfigStruct {
> > -  UINT64 Version;
> > +typedef struct {
> >     UINT64 AcpiBase;
> >     UINT64 AcpiSize;
> >     UINT64 SmbiosBase;
> > @@ -33,10 +32,22 @@ typedef struct UefiPayloadConfigStruct {
> >     SerialPortConfig SerialConfig;
> >     UINT32 NumMemoryMapEntries;
> >     MemoryMapEntry MemoryMapEntries[0];
> > +} UefiPayloadConfigV1;
> > +
> > +typedef struct UefiPayloadConfigStruct {
> > +  UINT64 Version;
> > +  union {
> > +    UefiPayloadConfigV1 v1;
> > +    struct {
> > +      char cmdline[0]; // up to 64 KB
> > +    } v2;
> > +  } config;
> >   } UefiPayloadConfig;
> >   #pragma pack()
> >
> > -#define UEFI_PAYLOAD_CONFIG_VERSION 1
> > +// magic version config is "LnxBoot1"
> > +#define UEFI_PAYLOAD_CONFIG_VERSION1 1
> > +#define UEFI_PAYLOAD_CONFIG_VERSION2 0x31746f6f42786e4cULL
> >
> >   #define LINUXBOOT_MEM_RAM 1
> >   #define LINUXBOOT_MEM_DEFAULT 2
> > diff --git a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> > index 34bfb6a1073f..5e68091cac91 100644
> > --- a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> > +++ b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> > @@ -1,13 +1,12 @@
> >   /** @file
> > -  This library will parse the linuxboot table in memory and extract
> those required
> > -  information.
> > +  This library will parse the linuxboot table in memory and extract
> those
> > +required information.
> >
> >     Copyright (c) 2021, the u-root Authors. All rights reserved.<BR>
> >     SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >   **/
> >
> > -
> >   #include <IndustryStandard/Acpi.h>
> >   #include <IndustryStandard/SmBios.h>
> >   #include <Library/BaseLib.h>
> > @@ -18,17 +17,42 @@
> >   #include <Library/PcdLib.h>
> >   #include <Linuxboot.h>
> >   #include <Uefi/UefiBaseType.h>
> > +#include <stdint.h>
> > +#include <stdlib.h>
> > +//#include <string.h>
> > +//#include <ctype.h>
> > +
> > +#define strncmp(a, b, n) AsciiStrnCmp((a), (b), (n))
> > +
> > +static uint64_t parse_int(const char* s, char** end) {
> > +  UINT64 x;
> > +
> > +  if (s[0] == '0' && s[1] == 'x')
> > +    AsciiStrHexToUint64S(s, end, &x);
> > +  else
> > +    AsciiStrDecimalToUint64S(s, end, &x);
> > +
> > +  return x;
> > +}
> > +
> > +static int isspace(const char c) { return c == ' ' || c == '\t' || c ==
> '\n'; }
> >
> >   // Retrieve UefiPayloadConfig from Linuxboot's uefiboot
> > -UefiPayloadConfig* GetUefiPayLoadConfig() {
> > -  UefiPayloadConfig* config =
> > +const UefiPayloadConfig* GetUefiPayLoadConfig() {
> > +  const UefiPayloadConfig* config =
> >         (UefiPayloadConfig*)(UINTN)(PcdGet32(PcdPayloadFdMemBase) -
> SIZE_64KB);
> > -  if (config->Version != UEFI_PAYLOAD_CONFIG_VERSION) {
> > -    DEBUG((DEBUG_ERROR, "Expect payload config version: %d, but get
> %d\n",
> > -           UEFI_PAYLOAD_CONFIG_VERSION, config->Version));
> > -    CpuDeadLoop ();
> > -  }
> > -  return config;
> > +
> > +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1 ||
> > +      config->Version == UEFI_PAYLOAD_CONFIG_VERSION2)
> > +    return config;
> > +
> > +  DEBUG((DEBUG_ERROR,
> > +         "Expect payload config version %016lx or %016lx, but get
> %016lx\n",
> > +         UEFI_PAYLOAD_CONFIG_VERSION1, UEFI_PAYLOAD_CONFIG_VERSION2,
> > +         config->Version));
> > +  CpuDeadLoop();
> > +  while (1)
> > +    ;
> >   }
> >
> >   // Align the address and add memory rang to MemInfoCallback
> > @@ -54,6 +78,57 @@ void AddMemoryRange(IN BL_MEM_INFO_CALLBACK
> MemInfoCallback, IN UINTN start,
> >     MemInfoCallback(&MemoryMap, NULL);
> >   }
> >
> > +const char* cmdline_next(const char* cmdline, const char** option) {
> > +  // at the end of the string, we're done
> > +  if (!cmdline || *cmdline == '\0') return NULL;
> > +
> > +  // skip any leading whitespace
> > +  while (isspace(*cmdline)) cmdline++;
> > +
> > +  // if we've hit the end of the string, we're done
> > +  if (*cmdline == '\0') return NULL;
> > +
> > +  *option = cmdline;
> > +
> > +  // find the end of this option or the string
> > +  while (!isspace(*cmdline) && *cmdline != '\0') cmdline++;
> > +
> > +  // cmdline points to the whitespace or end of string
> > +  return cmdline;
> > +}
> > +
> > +int cmdline_ints(const char* option, uint64_t args[], int max) {
> > +  // skip any leading text up to an '='
> > +  const char* s = option;
> > +  while (1) {
> > +    const char c = *s++;
> > +    if (c == '=') break;
> > +
> > +    if (c == '\0' || isspace(c)) {
> > +      s = option;
> > +      break;
> > +    }
> > +  }
> > +
> > +  for (int i = 0; i < max; i++) {
> > +    char* end;
> > +    args[i] = parse_int(s, &end);
> > +
> > +    // end of string or end of the option?
> > +    if (*end == '\0' || isspace(*end)) return i + 1;
> > +
> > +    // not separator? signal an error if we have consumed any ints,
> > +    // otherwise return 0 saying that none were found
> > +    if (*end != ',') return i == 0 ? 0 : -1;
> > +
> > +    // skip the , and get the next value
> > +    s = end + 1;
> > +  }
> > +
> > +  // too many values!
> > +  return -1;
> > +}
> > +
> >   /**
> >     Acquire the memory information from the linuxboot table in memory.
> >
> > @@ -67,20 +142,50 @@ void AddMemoryRange(IN BL_MEM_INFO_CALLBACK
> MemInfoCallback, IN UINTN start,
> >   RETURN_STATUS
> >   EFIAPI
> >   ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN VOID*
> Params) {
> > -  UefiPayloadConfig* config;
> > -  int i;
> > +  const UefiPayloadConfig* config = GetUefiPayLoadConfig();
> > +  if (!config) {
> > +    DEBUG(
> > +        (DEBUG_ERROR, "ParseMemoryInfo: Could not find UEFI Payload
> config\n"));
> > +    return RETURN_SUCCESS;
> > +  }
> > +
> > +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
> > +    const UefiPayloadConfigV1* config1 = &config->config.v1;
> > +    DEBUG(
> > +        (DEBUG_INFO, "MemoryMap #entries: %d\n",
> config1->NumMemoryMapEntries));
> > +
> > +    for (int i = 0; i < config1->NumMemoryMapEntries; i++) {
> > +      const MemoryMapEntry* entry = &config1->MemoryMapEntries[i];
> > +      DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n",
> entry->Start,
> > +             entry->End, entry->Type));
> > +      AddMemoryRange(MemInfoCallback, entry->Start, entry->End,
> entry->Type);
> > +    }
> > +  } else
> >
> > -  config = GetUefiPayLoadConfig();
> > +      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
> > +    const char* cmdline = config->config.v2.cmdline;
> > +    const char* option;
> > +    uint64_t args[3];
> >
> > -  DEBUG((DEBUG_INFO, "MemoryMap #entries: %d\n",
> config->NumMemoryMapEntries));
> > +    // look for the mem=start,end,type
> > +    while ((cmdline = cmdline_next(cmdline, &option))) {
> > +      if (strncmp(option, "mem=", 4) != 0) continue;
> >
> > -  MemoryMapEntry* entry = &config->MemoryMapEntries[0];
> > -  for (i = 0; i < config->NumMemoryMapEntries; i++) {
> > -    DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n",
> entry->Start,
> > -           entry->End, entry->Type));
> > -    AddMemoryRange(MemInfoCallback, entry->Start, entry->End,
> entry->Type);
> > -    entry++;
> > +      if (cmdline_ints(option, args, 3) != 3) {
> > +        DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
> > +        continue;
> > +      }
> > +
> > +      const uint64_t start = args[0];
> > +      const uint64_t end = args[1];
> > +      const uint64_t type = args[2];
> > +
> > +      DEBUG(
> > +          (DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", start, end,
> type));
> > +      AddMemoryRange(MemInfoCallback, start, end, type);
> > +    }
> >     }
> > +
> >     return RETURN_SUCCESS;
> >   }
> >
> > @@ -96,14 +201,52 @@ ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK
> MemInfoCallback, IN VOID* Params) {
> >   RETURN_STATUS
> >   EFIAPI
> >   ParseSystemTable(OUT SYSTEM_TABLE_INFO* SystemTableInfo) {
> > -  UefiPayloadConfig* config;
> > +  const UefiPayloadConfig* config = GetUefiPayLoadConfig();
> > +  if (!config) {
> > +    DEBUG((DEBUG_ERROR,
> > +           "ParseSystemTable: Could not find UEFI Payload config\n"));
> > +    return RETURN_SUCCESS;
> > +  }
> >
> > -  config = GetUefiPayLoadConfig();
> > -  SystemTableInfo->AcpiTableBase = config->AcpiBase;
> > -  SystemTableInfo->AcpiTableSize = config->AcpiSize;
> > +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
> > +    const UefiPayloadConfigV1* config1 = &config->config.v1;
> > +    SystemTableInfo->AcpiTableBase = config1->AcpiBase;
> > +    SystemTableInfo->AcpiTableSize = config1->AcpiSize;
> >
> > -  SystemTableInfo->SmbiosTableBase = config->SmbiosBase;
> > -  SystemTableInfo->SmbiosTableSize = config->SmbiosSize;
> > +    SystemTableInfo->SmbiosTableBase = config1->SmbiosBase;
> > +    SystemTableInfo->SmbiosTableSize = config1->SmbiosSize;
> > +  } else
> > +
> > +      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
> > +    const char* cmdline = config->config.v2.cmdline;
> > +    const char* option;
> > +    uint64_t args[2];
> > +
> > +    // look for the acpi config
> > +    while ((cmdline = cmdline_next(cmdline, &option))) {
> > +      if (strncmp(option, "ACPI20=", 7) == 0) {
> > +        const int count = cmdline_ints(option, args, 2);
> > +        if (count < 0) {
> > +          DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
> > +          continue;
> > +        }
> > +
> > +        if (count > 0) SystemTableInfo->AcpiTableBase = args[0];
> > +        if (count > 1) SystemTableInfo->AcpiTableSize = args[1];
> > +      }
> > +
> > +      if (strncmp(option, "SMBIOS=", 7) == 0) {
> > +        const int count = cmdline_ints(option, args, 2);
> > +        if (count < 0) {
> > +          DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
> > +          continue;
> > +        }
> > +
> > +        if (count > 0) SystemTableInfo->SmbiosTableBase = args[0];
> > +        if (count > 1) SystemTableInfo->SmbiosTableSize = args[1];
> > +      }
>
> Sorry, from only a quick peak, can this not leave {Acpi,Smbios}TableBase
> and {Acpi,Smbios}TableSize undefined entirely, while returning
> RETURN_SUCCESS?
> Even if not, it looks kind of odd a command-line argument can change the
> base address without changing the size, is there a documentation of this
> behaviour?
> What is the structure supposed to carry if no such arguments are given
> at all?
>
> Thanks for your time!
>
> Best regards,
> Marvin
>
> > +    }
> > +  }
> >
> >     return RETURN_SUCCESS;
> >   }
> > @@ -120,15 +263,52 @@ ParseSystemTable(OUT SYSTEM_TABLE_INFO*
> SystemTableInfo) {
> >   RETURN_STATUS
> >   EFIAPI
> >   ParseSerialInfo(OUT SERIAL_PORT_INFO* SerialPortInfo) {
> > -  UefiPayloadConfig* config;
> > -  config = GetUefiPayLoadConfig();
> > -
> > -  SerialPortInfo->BaseAddr = config->SerialConfig.BaseAddr;
> > -  SerialPortInfo->RegWidth = config->SerialConfig.RegWidth;
> > -  SerialPortInfo->Type = config->SerialConfig.Type;
> > -  SerialPortInfo->Baud = config->SerialConfig.Baud;
> > -  SerialPortInfo->InputHertz = config->SerialConfig.InputHertz;
> > -  SerialPortInfo->UartPciAddr = config->SerialConfig.UartPciAddr;
> > +  // fill in some reasonable defaults
> > +  SerialPortInfo->BaseAddr = 0x3f8;
> > +  SerialPortInfo->RegWidth = 1;
> > +  SerialPortInfo->Type = 1;  // uefi.SerialPortTypeIO
> > +  SerialPortInfo->Baud = 115200;
> > +  SerialPortInfo->InputHertz = 1843200;
> > +  SerialPortInfo->UartPciAddr = 0;
> > +
> > +  const UefiPayloadConfig* config = GetUefiPayLoadConfig();
> > +  if (!config) {
> > +    DEBUG((DEBUG_ERROR, "ParseSerialInfo: using default config\n"));
> > +    return RETURN_SUCCESS;
> > +  }
> > +
> > +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
> > +    const UefiPayloadConfigV1* config1 = &config->config.v1;
> > +    SerialPortInfo->BaseAddr = config1->SerialConfig.BaseAddr;
> > +    SerialPortInfo->RegWidth = config1->SerialConfig.RegWidth;
> > +    SerialPortInfo->Type = config1->SerialConfig.Type;
> > +    SerialPortInfo->Baud = config1->SerialConfig.Baud;
> > +    SerialPortInfo->InputHertz = config1->SerialConfig.InputHertz;
> > +    SerialPortInfo->UartPciAddr = config1->SerialConfig.UartPciAddr;
> > +  } else
> > +
> > +      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
> > +    const char* cmdline = config->config.v2.cmdline;
> > +    const char* option;
> > +    uint64_t args[6] = {};
> > +
> > +    while ((cmdline = cmdline_next(cmdline, &option))) {
> > +      if (strncmp(option, "serial=", 7) != 0) continue;
> > +
> > +      const int count = cmdline_ints(option, args, 6);
> > +      if (count < 0) {
> > +        DEBUG((DEBUG_ERROR, "Parse error: %a\n", option));
> > +        continue;
> > +      }
> > +
> > +      if (count > 0) SerialPortInfo->Baud = args[0];
> > +      if (count > 1) SerialPortInfo->BaseAddr = args[1];
> > +      if (count > 2) SerialPortInfo->RegWidth = args[2];
> > +      if (count > 3) SerialPortInfo->Type = args[3];
> > +      if (count > 4) SerialPortInfo->InputHertz = args[4];
> > +      if (count > 5) SerialPortInfo->UartPciAddr = args[5];
> > +    }
> > +  }
> >
> >     return RETURN_SUCCESS;
> >   }
>
>

[-- Attachment #2: Type: text/html, Size: 19253 bytes --]