[Skiboot] [PATCH 4/5] core/nvram: add support for skiboot config strings

[Joel Stanley]

On Mon, Aug 8, 2016 at 12:28 PM, Oliver O'Halloran <oohall at gmail.com> wrote:
> This allows the ibm,skiboot partition to be used to store NUL terminated
> key=value OF configuration strings. These strings can be written using
> the nvram utility found in the linux powerpc-utils package. Currently
> the only use case for this is passing command line arguments to the
> boot kernel so only ASCII strings are supported. The 0xFF binary
> escaping mechanism for configuration strings is not supported.
>
> Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
> ---
>  core/nvram-format.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++----
>  include/nvram.h     |  2 ++
>  2 files changed, 64 insertions(+), 5 deletions(-)
>
> diff --git a/core/nvram-format.c b/core/nvram-format.c
> index be8d77f8394b..57d51842e360 100644
> --- a/core/nvram-format.c
> +++ b/core/nvram-format.c
> @@ -28,6 +28,8 @@ struct chrp_nvram_hdr {
>         char            name[12];
>  };
>
> +struct chrp_nvram_hdr *skiboot_part_hdr;
> +
>  #define NVRAM_SIG_FW_PRIV      0x51
>  #define NVRAM_SIG_SYSTEM       0x70
>  #define NVRAM_SIG_FREE         0x7f
> @@ -115,7 +117,8 @@ int nvram_check(void *nvram_image, const uint32_t nvram_size)
>  {
>         unsigned int offset = 0;
>         bool found_common = false;
> -       bool found_skiboot = false;
> +
> +       skiboot_part_hdr = NULL;
>
>         while (offset + sizeof(struct chrp_nvram_hdr) < nvram_size) {
>                 struct chrp_nvram_hdr *h = nvram_image + offset;
> @@ -138,7 +141,7 @@ int nvram_check(void *nvram_image, const uint32_t nvram_size)
>
>                 if (h->sig == NVRAM_SIG_FW_PRIV &&
>                     strcmp(h->name, NVRAM_NAME_FW_PRIV) == 0)
> -                       found_skiboot = true;
> +                       skiboot_part_hdr = h;
>
>                 offset += h->len << 4;
>                 if (offset > nvram_size) {
> @@ -151,9 +154,9 @@ int nvram_check(void *nvram_image, const uint32_t nvram_size)
>                 prerror("NVRAM: Common partition not found !\n");
>                 goto failed;
>         }
> -       if (!found_skiboot) {
> -               prerror("NVRAM: Skiboot private partition "
> -                       "not found !\n");
> +
> +       if (!skiboot_part_hdr) {
> +               prerror("NVRAM: Skiboot private partition not found !\n");
>                 goto failed;
>         } else {
>                 /*
> @@ -176,3 +179,57 @@ int nvram_check(void *nvram_image, const uint32_t nvram_size)
>   failed:
>         return -1;
>  }
> +
> +static const char *find_next_key(const char *start, const char *end)
> +{
> +       while (start < end) {
> +               if (*start == 0)
> +                       return start + 1;
> +
> +               start++;
> +       }
> +
> +       return end;
> +}
> +
> +/*
> + * nvram_query() - Returns the value associated with a key. The ibm,skiboot
> + * partition contains a set of key=value pairs separated by NUL terminators.
> + */
> +const void *nvram_query(const char *key, size_t *length)
> +{
> +       const char *part_end = (const void *) skiboot_part_hdr +

Why not const char *?

> +               skiboot_part_hdr->len * 16;
> +       const char *start = (void *)skiboot_part_hdr +

Same.

> +               sizeof(*skiboot_part_hdr);
> +       const char *end = find_next_key(start, part_end);
> +       int key_len = strlen(key);

Is there a maximum length for a key?

> +
> +       prlog(PR_TRACE, "start: %p %s\n", start, start);
> +       prlog(PR_TRACE, "end:   %p %s\n", end, end);

No NVRAM prefix?

> +
> +       while (start < end) {
> +               int remaining = part_end - start;
> +
> +               if (key_len + 1 > remaining)
> +                       return NULL;
> +
> +               if (strlen(start))

Maximum length?

> +                       prlog(PR_TRACE, "NVRAM: '%s' (%lu)\n",
> +                               start, strlen(start));

You're relying on strncmp returning false for the empty key to skip
the next bit?

Would a goto make your intention clearer?

> +
> +               if (!strncmp(key, start, key_len) && start[key_len] == '=') {
> +                       const char *value = &start[key_len + 1];
> +
> +                       if (length)
> +                               *length = end - value;
> +
> +                       return value;
> +               }
> +
> +               start = end;
> +               end = find_next_key(end, part_end);
> +       }
> +
> +       return NULL;
> +}
> diff --git a/include/nvram.h b/include/nvram.h
> index c90c5712a2f7..b4faa2e72dc8 100644
> --- a/include/nvram.h
> +++ b/include/nvram.h
> @@ -21,4 +21,6 @@ int nvram_format(void *nvram_image, uint32_t nvram_size);
>  int nvram_check(void *nvram_image, uint32_t nvram_size);
>  void nvram_reinit(void);
>
> +const void *nvram_query(const char *name, size_t *length);
> +
>  #endif /* __NVRAM_H */
> --
> 2.5.5
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot