[Skiboot] [PATCH] hdata: Make sure reserved node name starts with "ibm, "

[Oliver]

On Sat, Jun 23, 2018 at 4:23 PM, Vasant Hegde
<hegdevasant at linux.vnet.ibm.com> wrote:
> HDAT does not use consistent label format for reserved memory label.
> Few starts with "ibm," while few other starts with component name.
>
> Lets make sure all the reserved node name starts with "ibm,".
>
> Reported-by: Pridhiviraj Paidipeddi <ppaidipe at linux.vnet.ibm.com>
> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
> ---
> @Stewart,
>   This fixes https://github.com/open-power/boston-openpower/issues/675
>
> -Vasant
>
>  hdata/memory.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/hdata/memory.c b/hdata/memory.c
> index ecce881d1..c0c30066e 100644
> --- a/hdata/memory.c
> +++ b/hdata/memory.c
> @@ -27,6 +27,8 @@
>  #include "spira.h"
>  #include "hdata.h"

> +#define FW_RESERVE_NODE_PREFIX "ibm,"
> +

Is this really better than just using the string literal?

>  struct HDIF_ram_area_id {
>         __be16 id;
>  #define RAM_AREA_INSTALLED     0x8000
> @@ -595,7 +597,7 @@ static void get_hb_reserved_mem(struct HDIF_common_hdr *ms_vpd)
>         const struct msvpd_hb_reserved_mem *hb_resv_mem;
>         u64 start_addr, end_addr, label_size;
>         struct dt_node *node;
> -       int count, i;
> +       int count, i, label_offset;
>         char *label;
>
>         /*
> @@ -643,11 +645,19 @@ static void get_hb_reserved_mem(struct HDIF_common_hdr *ms_vpd)
>                 if (label_size > 64)
>                         label_size = 64;
>
> -               label = malloc(label_size+1);
> +               /* Accommodate "ibm," */
> +               label_offset = 0;
> +               label = malloc(label_size + strlen(FW_RESERVE_NODE_PREFIX) + 1);

Come to think of it we could probably eliminate the malloc()ed label
in favour of a stack allocated array. The HDAT labels are only 64
bytes and that's not much more than a bare stack frame.

>                 assert(label);
>
> -               memcpy(label, hb_resv_mem->label, label_size);
> -               label[label_size] = '\0';
> +               /* Add "ibm," to reserved node name */
> +               if (strncasecmp(hb_resv_mem->label, "ibm", 3)) {
> +                       memcpy(label, FW_RESERVE_NODE_PREFIX,
> +                              strlen(FW_RESERVE_NODE_PREFIX));
> +                       label_offset = strlen(FW_RESERVE_NODE_PREFIX);

Using strncat() or even snprintf() would probably be clearer

> +               }
> +               memcpy(label + label_offset, hb_resv_mem->label, label_size);
> +               label[label_size + label_offset] = '\0';

I'd put the name canonicalization into add_hb_reserve_node() rather
than here. Opal-prd uses the ibm,prd-label property to find reserved
regions that are exported by hostboot so the label needs to be left
unchanged here.

>                 /* Unnamed reservations are always broken. Ignore them. */
>                 if (strlen(label) == 0) {
> --
> 2.14.3
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot