| Message ID | 20220429094609.72733-2-clombard@linux.vnet.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Implement virtual flash content using PLDM | expand |
On 29/04/2022 11:46, Christophe Lombard wrote: > This patch parses the "hb_lid_ids" string from bios tables and complete > the global list of lid files. Each entry in the list contains the name, > the id, the length of the lid file and the virtual address start access. > This virtual address is used for for PNOR Resource Provider operations. > 16 MB of VMM address are reserved space per section. > > Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com> > --- > core/pldm/Makefile.inc | 2 +- > core/pldm/pldm-lid-files.c | 170 +++++++++++++++++++++++++++++++++++++ > include/pldm.h | 7 ++ > 3 files changed, 178 insertions(+), 1 deletion(-) > create mode 100644 core/pldm/pldm-lid-files.c > > diff --git a/core/pldm/Makefile.inc b/core/pldm/Makefile.inc > index d3affaa0..506bf5d7 100644 > --- a/core/pldm/Makefile.inc > +++ b/core/pldm/Makefile.inc > @@ -13,7 +13,7 @@ CFLAGS_$(PLDM_DIR)/pldm-bios-requests.o = -Wno-strict-prototypes > PLDM_OBJS = pldm-common.o pldm-responder.o pldm-requester.o > PLDM_OBJS += pldm-base-requests.o pldm-platform-requests.o > PLDM_OBJS += pldm-bios-requests.o pldm-fru-requests.o > -PLDM_OBJS += pldm-file-io-requests.o > +PLDM_OBJS += pldm-file-io-requests.o pldm-lid-files.o > > PLDM = $(PLDM_DIR)/built-in.a > $(PLDM): $(PLDM_OBJS:%=$(PLDM_DIR)/%) > diff --git a/core/pldm/pldm-lid-files.c b/core/pldm/pldm-lid-files.c > new file mode 100644 > index 00000000..11e2f9e2 > --- /dev/null > +++ b/core/pldm/pldm-lid-files.c > @@ -0,0 +1,170 @@ > +// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later > +// Copyright 2022 IBM Corp. > + > +#define pr_fmt(fmt) "PLDM: " fmt > + > +#include <endian.h> > +#include <lock.h> > +#include <opal-api.h> > +#include <libflash/errors.h> > +#include <libflash/ffs.h> > +#include "pldm.h" > + > +/* > + * This struct is used to map a PNOR sections. > + * The content is deriving from the hb_lid_ids PLDM BIOS Attribute. > + */ > +struct pldm_lid { > + struct list_node list; > + uint32_t start; > + uint32_t handle; > + uint32_t length; > + char name[PART_NAME_MAX + 1]; > + char id[PART_NAME_MAX + 1]; > +}; > + > +static LIST_HEAD(lid_files); > + > +#define MEGABYTE (1024*1024) > + > +/* > + * When using PLDM for PNOR Resource Provider operations, > + * reserve 16 MB of VMM address space per section. > + * Note that all of this space may not actually be used by each section. > + */ > +#define VMM_SIZE_RESERVED_PER_SECTION (16 * MEGABYTE) > + > +/* > + * Print the attributes of lid files. > + */ > +static void print_lid_files_attr(void) > +{ > + struct pldm_lid *lid = NULL; > + > + list_for_each(&lid_files, lid, list) > + prlog(PR_NOTICE, "name: %s, id: %s, handle: %d, length: 0x%x, start: 0x%x\n", > + lid->name, lid->id, lid->handle, lid->length, lid->start); > + > +} > + > +/* > + * Return the number of lid files. > + */ > +static uint32_t get_lids_count(void) > +{ > + struct pldm_lid *lid = NULL; > + uint32_t count = 0; > + > + list_for_each(&lid_files, lid, list) > + count++; > + > + return count; > +} > + > +/* > + * parse the "hb_lid_ids" string > + * <ATTR_a>=<lid_id_1>,<ATTR_b>=<lid_id_2> > + */ > +static int parse_hb_lid_ids_string(char *str) > +{ > + struct pldm_lid *lid; > + const char *pp = "="; > + char *attr, *attr_end; > + int rc, count = 1; > + char *lid_id; > + > + for (char *p = strtok(str, ","); p != NULL; p = strtok(NULL, ",")) { > + lid = zalloc(sizeof(struct pldm_lid)); > + if (!lid) { > + prlog(PR_ERR, "Error allocating pldm_lid structure\n"); > + rc = OPAL_RESOURCE; > + goto err; > + } > + > + /* parse the string <attr>=<lid_id> */ > + attr = p; > + while (*pp != *p) > + p++; Same comment as in a previous patch: we should protect ourselves against malformed expressions in case the string doesn't have a "=" in it. > + > + attr_end = p; > + lid_id = ++p; > + *attr_end = '\0'; > + > + strcpy(lid->name, attr); > + strcpy(lid->id, lid_id); > + > + /* reserve 16 MB of VMM address space per section */ > + lid->start = VMM_SIZE_RESERVED_PER_SECTION * count; I was wondering by we start with count = 1, i.e the very first section address it not 0. I found the answer in the next patch (I think :) ), as we want to emulate a flash rom and we reserve address 0 for the header. A comment could be useful. > + > + /* handle and length */ > + rc = pldm_find_file_handle_by_lid_id(lid->id, > + &lid->handle, > + &lid->length); > + if ((rc) && (rc != OPAL_PARAMETER)) > + goto err; This is where I think it would make sense to check and err out if the lid length is bigger than the VMM_SIZE_RESERVED_PER_SECTION space we reserve. > + > + /* add new member in the global list */ > + list_add_tail(&lid_files, &lid->list); > + > + count++; > + } > + > + return OPAL_SUCCESS; > + > +err: > + /* free all lid entries */ > + list_for_each(&lid_files, lid, list) > + free(lid); > + > + return rc; > +} > + > +/* > + * Parse the "hb_lid_ids" string from bios tables and complete > + * the global list of lid files. > + */ > +static int lid_ids_to_vaddr_mapping(void) > +{ > + char *lid_ids_string; lid_ids_string must be initialized to NULL because of "goto out" Fred
Le 14/04/2023 à 16:04, Frederic Barrat a écrit : > > > On 29/04/2022 11:46, Christophe Lombard wrote: >> This patch parses the "hb_lid_ids" string from bios tables and complete >> the global list of lid files. Each entry in the list contains the name, >> the id, the length of the lid file and the virtual address start access. >> This virtual address is used for for PNOR Resource Provider operations. >> 16 MB of VMM address are reserved space per section. >> >> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com> >> --- >> core/pldm/Makefile.inc | 2 +- >> core/pldm/pldm-lid-files.c | 170 +++++++++++++++++++++++++++++++++++++ >> include/pldm.h | 7 ++ >> 3 files changed, 178 insertions(+), 1 deletion(-) >> create mode 100644 core/pldm/pldm-lid-files.c >> >> diff --git a/core/pldm/Makefile.inc b/core/pldm/Makefile.inc >> index d3affaa0..506bf5d7 100644 >> --- a/core/pldm/Makefile.inc >> +++ b/core/pldm/Makefile.inc >> @@ -13,7 +13,7 @@ CFLAGS_$(PLDM_DIR)/pldm-bios-requests.o = >> -Wno-strict-prototypes >> PLDM_OBJS = pldm-common.o pldm-responder.o pldm-requester.o >> PLDM_OBJS += pldm-base-requests.o pldm-platform-requests.o >> PLDM_OBJS += pldm-bios-requests.o pldm-fru-requests.o >> -PLDM_OBJS += pldm-file-io-requests.o >> +PLDM_OBJS += pldm-file-io-requests.o pldm-lid-files.o >> PLDM = $(PLDM_DIR)/built-in.a >> $(PLDM): $(PLDM_OBJS:%=$(PLDM_DIR)/%) >> diff --git a/core/pldm/pldm-lid-files.c b/core/pldm/pldm-lid-files.c >> new file mode 100644 >> index 00000000..11e2f9e2 >> --- /dev/null >> +++ b/core/pldm/pldm-lid-files.c >> @@ -0,0 +1,170 @@ >> +// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later >> +// Copyright 2022 IBM Corp. >> + >> +#define pr_fmt(fmt) "PLDM: " fmt >> + >> +#include <endian.h> >> +#include <lock.h> >> +#include <opal-api.h> >> +#include <libflash/errors.h> >> +#include <libflash/ffs.h> >> +#include "pldm.h" >> + >> +/* >> + * This struct is used to map a PNOR sections. >> + * The content is deriving from the hb_lid_ids PLDM BIOS Attribute. >> + */ >> +struct pldm_lid { >> + struct list_node list; >> + uint32_t start; >> + uint32_t handle; >> + uint32_t length; >> + char name[PART_NAME_MAX + 1]; >> + char id[PART_NAME_MAX + 1]; >> +}; >> + >> +static LIST_HEAD(lid_files); >> + >> +#define MEGABYTE (1024*1024) >> + >> +/* >> + * When using PLDM for PNOR Resource Provider operations, >> + * reserve 16 MB of VMM address space per section. >> + * Note that all of this space may not actually be used by each >> section. >> + */ >> +#define VMM_SIZE_RESERVED_PER_SECTION (16 * MEGABYTE) >> + >> +/* >> + * Print the attributes of lid files. >> + */ >> +static void print_lid_files_attr(void) >> +{ >> + struct pldm_lid *lid = NULL; >> + >> + list_for_each(&lid_files, lid, list) >> + prlog(PR_NOTICE, "name: %s, id: %s, handle: %d, length: >> 0x%x, start: 0x%x\n", >> + lid->name, lid->id, lid->handle, lid->length, >> lid->start); >> + >> +} >> + >> +/* >> + * Return the number of lid files. >> + */ >> +static uint32_t get_lids_count(void) >> +{ >> + struct pldm_lid *lid = NULL; >> + uint32_t count = 0; >> + >> + list_for_each(&lid_files, lid, list) >> + count++; >> + >> + return count; >> +} >> + >> +/* >> + * parse the "hb_lid_ids" string >> + * <ATTR_a>=<lid_id_1>,<ATTR_b>=<lid_id_2> >> + */ >> +static int parse_hb_lid_ids_string(char *str) >> +{ >> + struct pldm_lid *lid; >> + const char *pp = "="; >> + char *attr, *attr_end; >> + int rc, count = 1; >> + char *lid_id; >> + >> + for (char *p = strtok(str, ","); p != NULL; p = strtok(NULL, >> ",")) { >> + lid = zalloc(sizeof(struct pldm_lid)); >> + if (!lid) { >> + prlog(PR_ERR, "Error allocating pldm_lid structure\n"); >> + rc = OPAL_RESOURCE; >> + goto err; >> + } >> + >> + /* parse the string <attr>=<lid_id> */ >> + attr = p; >> + while (*pp != *p) >> + p++; > > > Same comment as in a previous patch: we should protect ourselves > against malformed expressions in case the string doesn't have a "=" in > it. Normally it should never, but you never know. Thanks. > > >> + >> + attr_end = p; >> + lid_id = ++p; >> + *attr_end = '\0'; >> + >> + strcpy(lid->name, attr); >> + strcpy(lid->id, lid_id); >> + >> + /* reserve 16 MB of VMM address space per section */ >> + lid->start = VMM_SIZE_RESERVED_PER_SECTION * count; > > > I was wondering by we start with count = 1, i.e the very first section > address it not 0. I found the answer in the next patch (I think :) ), > as we want to emulate a flash rom and we reserve address 0 for the > header. A comment could be useful. > > >> + >> + /* handle and length */ >> + rc = pldm_find_file_handle_by_lid_id(lid->id, >> + &lid->handle, >> + &lid->length); >> + if ((rc) && (rc != OPAL_PARAMETER)) >> + goto err; > > > This is where I think it would make sense to check and err out if the > lid length is bigger than the VMM_SIZE_RESERVED_PER_SECTION space we > reserve. Correct. > > >> + >> + /* add new member in the global list */ >> + list_add_tail(&lid_files, &lid->list); >> + >> + count++; >> + } >> + >> + return OPAL_SUCCESS; >> + >> +err: >> + /* free all lid entries */ >> + list_for_each(&lid_files, lid, list) >> + free(lid); >> + >> + return rc; >> +} >> + >> +/* >> + * Parse the "hb_lid_ids" string from bios tables and complete >> + * the global list of lid files. >> + */ >> +static int lid_ids_to_vaddr_mapping(void) >> +{ >> + char *lid_ids_string; > > > lid_ids_string must be initialized to NULL because of "goto out" Correct. Thanks. > > Fred >
diff --git a/core/pldm/Makefile.inc b/core/pldm/Makefile.inc index d3affaa0..506bf5d7 100644 --- a/core/pldm/Makefile.inc +++ b/core/pldm/Makefile.inc @@ -13,7 +13,7 @@ CFLAGS_$(PLDM_DIR)/pldm-bios-requests.o = -Wno-strict-prototypes PLDM_OBJS = pldm-common.o pldm-responder.o pldm-requester.o PLDM_OBJS += pldm-base-requests.o pldm-platform-requests.o PLDM_OBJS += pldm-bios-requests.o pldm-fru-requests.o -PLDM_OBJS += pldm-file-io-requests.o +PLDM_OBJS += pldm-file-io-requests.o pldm-lid-files.o PLDM = $(PLDM_DIR)/built-in.a $(PLDM): $(PLDM_OBJS:%=$(PLDM_DIR)/%) diff --git a/core/pldm/pldm-lid-files.c b/core/pldm/pldm-lid-files.c new file mode 100644 index 00000000..11e2f9e2 --- /dev/null +++ b/core/pldm/pldm-lid-files.c @@ -0,0 +1,170 @@ +// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later +// Copyright 2022 IBM Corp. + +#define pr_fmt(fmt) "PLDM: " fmt + +#include <endian.h> +#include <lock.h> +#include <opal-api.h> +#include <libflash/errors.h> +#include <libflash/ffs.h> +#include "pldm.h" + +/* + * This struct is used to map a PNOR sections. + * The content is deriving from the hb_lid_ids PLDM BIOS Attribute. + */ +struct pldm_lid { + struct list_node list; + uint32_t start; + uint32_t handle; + uint32_t length; + char name[PART_NAME_MAX + 1]; + char id[PART_NAME_MAX + 1]; +}; + +static LIST_HEAD(lid_files); + +#define MEGABYTE (1024*1024) + +/* + * When using PLDM for PNOR Resource Provider operations, + * reserve 16 MB of VMM address space per section. + * Note that all of this space may not actually be used by each section. + */ +#define VMM_SIZE_RESERVED_PER_SECTION (16 * MEGABYTE) + +/* + * Print the attributes of lid files. + */ +static void print_lid_files_attr(void) +{ + struct pldm_lid *lid = NULL; + + list_for_each(&lid_files, lid, list) + prlog(PR_NOTICE, "name: %s, id: %s, handle: %d, length: 0x%x, start: 0x%x\n", + lid->name, lid->id, lid->handle, lid->length, lid->start); + +} + +/* + * Return the number of lid files. + */ +static uint32_t get_lids_count(void) +{ + struct pldm_lid *lid = NULL; + uint32_t count = 0; + + list_for_each(&lid_files, lid, list) + count++; + + return count; +} + +/* + * parse the "hb_lid_ids" string + * <ATTR_a>=<lid_id_1>,<ATTR_b>=<lid_id_2> + */ +static int parse_hb_lid_ids_string(char *str) +{ + struct pldm_lid *lid; + const char *pp = "="; + char *attr, *attr_end; + int rc, count = 1; + char *lid_id; + + for (char *p = strtok(str, ","); p != NULL; p = strtok(NULL, ",")) { + lid = zalloc(sizeof(struct pldm_lid)); + if (!lid) { + prlog(PR_ERR, "Error allocating pldm_lid structure\n"); + rc = OPAL_RESOURCE; + goto err; + } + + /* parse the string <attr>=<lid_id> */ + attr = p; + while (*pp != *p) + p++; + + attr_end = p; + lid_id = ++p; + *attr_end = '\0'; + + strcpy(lid->name, attr); + strcpy(lid->id, lid_id); + + /* reserve 16 MB of VMM address space per section */ + lid->start = VMM_SIZE_RESERVED_PER_SECTION * count; + + /* handle and length */ + rc = pldm_find_file_handle_by_lid_id(lid->id, + &lid->handle, + &lid->length); + if ((rc) && (rc != OPAL_PARAMETER)) + goto err; + + /* add new member in the global list */ + list_add_tail(&lid_files, &lid->list); + + count++; + } + + return OPAL_SUCCESS; + +err: + /* free all lid entries */ + list_for_each(&lid_files, lid, list) + free(lid); + + return rc; +} + +/* + * Parse the "hb_lid_ids" string from bios tables and complete + * the global list of lid files. + */ +static int lid_ids_to_vaddr_mapping(void) +{ + char *lid_ids_string; + int rc; + + /* get lid ids string from bios tables */ + rc = pldm_bios_get_lids_id(&lid_ids_string); + if (rc) + goto out; + + /* parse the "hb_lid_ids" string */ + rc = parse_hb_lid_ids_string(lid_ids_string); + +out: + if (lid_ids_string) + free(lid_ids_string); + + return rc; +} + +int pldm_lid_files_init(struct blocklevel_device **bl) +{ + uint32_t lid_files_count; + int rc; + + if (!bl) + return FLASH_ERR_PARM_ERROR; + + *bl = NULL; + + /* convert lid ids data to pnor structure */ + rc = lid_ids_to_vaddr_mapping(); + if (rc) + goto err; + + lid_files_count = get_lids_count(); + + prlog(PR_NOTICE, "Number of lid files: %d\n", lid_files_count); + print_lid_files_attr(); + + return OPAL_SUCCESS; + +err: + return rc; +} diff --git a/include/pldm.h b/include/pldm.h index c3a549e5..9735261d 100644 --- a/include/pldm.h +++ b/include/pldm.h @@ -5,6 +5,8 @@ #ifndef __PLDM_H__ #define __PLDM_H__ +#include <skiboot.h> + /** * PLDM over MCTP initialization */ @@ -30,4 +32,9 @@ int pldm_platform_restart(void); */ int pldm_fru_dt_add_bmc_version(void); +/** + * Convert lid ids data to pnor structure + */ +int pldm_lid_files_init(struct blocklevel_device **bl); + #endif /* __PLDM_H__ */
This patch parses the "hb_lid_ids" string from bios tables and complete the global list of lid files. Each entry in the list contains the name, the id, the length of the lid file and the virtual address start access. This virtual address is used for for PNOR Resource Provider operations. 16 MB of VMM address are reserved space per section. Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com> --- core/pldm/Makefile.inc | 2 +- core/pldm/pldm-lid-files.c | 170 +++++++++++++++++++++++++++++++++++++ include/pldm.h | 7 ++ 3 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 core/pldm/pldm-lid-files.c