[Skiboot] [PATCH V2 15/21] core/pldm: Find lid attribute from bios tables

Abhishek SIngh Tomar abhishek at linux.ibm.com
Wed Mar 16 20:09:41 AEDT 2022


Hello Christophe

In function find_lid_by_attr_name(). I doubt the scenario the code will fail if two attribute names
are such that one attribute is substring of other.

Taking example of input variable to function:
str  : "ATTR_AB=21,ATTR_A=43,ATTR_X=45
name : "ATTR_A"
expected output is 43 but output received will be 21

> + * parse a string, format:
> + * <ATTR_a>=<lid_id_1>,<ATTR_b>=<lid_id_2>
> + */
> +static int find_lid_by_attr_name(char *str, const char *name,
> +				 char **lid)
> +{
> +	char *p;
> +
> +	for (p = strtok(str, ","); p != NULL; p = strtok(NULL, ",")) {
> +		if (!strncmp(p, name, strlen(name))) {
> +			p += strlen(name) + 1;
> +			*lid = strdup(p);
> +			return OPAL_SUCCESS;
> +		}
> +	}
> +	prlog(PR_ERR, "%s - lid not found, attr name: %s\n",
> +		      __func__, name);
> +
> +	return OPAL_PARAMETER;
> +}

Though we may not have such scenario in our case of atribute name but sometime this become 
reason of failure in parsing string. I suggest to match(compare) length of attribute name in string 
with name input variable string length.

Regards
Abhishek Singh Tomar

On Fri, Mar 04, 2022 at 02:11:48PM +0100, Christophe Lombard wrote:
> This specification defines the data structures and messages for
> communicating BIOS settings, BIOS attributes, boot configurations, and
> boot order settings.
> 
> Use the GetBIOSTable command to get STRING, Attribute and Attribute values
> tables from the BIOS.
> 
> The contents of these tables are needed to read/write the desired lid
> files located on the BMC.
> 
> Signed-off-by: Christophe Lombard <clombard at linux.vnet.ibm.com>
> ---
>  core/pldm/pldm-bios-requests.c | 138 +++++++++++++++++++++++++++++++++
>  core/pldm/pldm.h               |   2 +
>  2 files changed, 140 insertions(+)
> 
> diff --git a/core/pldm/pldm-bios-requests.c b/core/pldm/pldm-bios-requests.c
> index 0e2f9a25..80ceb471 100644
> --- a/core/pldm/pldm-bios-requests.c
> +++ b/core/pldm/pldm-bios-requests.c
> @@ -84,6 +84,144 @@ static void bios_init_complete(bool success)
>  	bios_ready = true;
>  }
>  
> +/*
> + * parse a string, format:
> + * <ATTR_a>=<lid_id_1>,<ATTR_b>=<lid_id_2>
> + */
> +static int find_lid_by_attr_name(char *str, const char *name,
> +				 char **lid)
> +{
> +	char *p;
> +
> +	for (p = strtok(str, ","); p != NULL; p = strtok(NULL, ",")) {
> +		if (!strncmp(p, name, strlen(name))) {
> +			p += strlen(name) + 1;
> +			*lid = strdup(p);
> +			return OPAL_SUCCESS;
> +		}
> +	}
> +	prlog(PR_ERR, "%s - lid not found, attr name: %s\n",
> +		      __func__, name);
> +
> +	return OPAL_PARAMETER;
> +}
> +
> +/*
> + * Retrieve attribut value from string.
> + *
> + * Ex:   string value = "hb_lid_ids"
> + *  From STRING table (with string value = "hb_lid_ids") -> string_handle = 60
> + *  From ATTR table (with attr name handle = 60)         -> attr handle = 8
> + *  From ATTR VAL table (with attr handle = 8)
> + *    -> CurrentString = ATTR_PERM=81e00663,ATTR_TMP=81e00664, ...
> + *                       NVRAM=81e0066b,...,pnor.toc=NA"
> + */
> +static int find_bios_table_attr_values_by_string(const char *str,
> +						 char **current_string)
> +{
> +	const struct pldm_bios_string_table_entry *string_entry;
> +	const struct pldm_bios_attr_table_entry *attr_entry;
> +	const struct pldm_bios_attr_val_table_entry *val_entry;
> +	uint16_t string_handle, attr_handle;
> +	struct variable_field currentString;
> +
> +	if ((!bios_string_table) || (!bios_attr_table))
> +		return OPAL_PARAMETER;
> +
> +	/* decode string table */
> +	string_entry = pldm_bios_table_string_find_by_string(
> +			bios_string_table, bios_string_length, str);
> +	if (!string_entry) {
> +		prlog(PR_ERR, "String table entry not found, str: %s\n",
> +			      str);
> +		return OPAL_PARAMETER;
> +	}
> +
> +	/* get the string handle for the entry */
> +	string_handle = pldm_bios_table_string_entry_decode_handle(string_entry);
> +	prlog(PR_TRACE, "%s - string_handle: %d\n", __func__, string_handle);
> +
> +	/* decode attribute table */
> +	attr_entry = pldm_bios_table_attr_find_by_string_handle(
> +		      bios_attr_table, bios_attr_length, string_handle);
> +	if (!attr_entry) {
> +		prlog(PR_ERR, "Attribute table entry not found, string_handle: %d\n",
> +			      string_handle);
> +		return OPAL_PARAMETER;
> +	}
> +
> +	/* get the attribute handle from the attribute table entry */
> +	attr_handle = pldm_bios_table_attr_entry_decode_attribute_handle(attr_entry);
> +	prlog(PR_TRACE, "%s - attr_handle: %d\n", __func__, attr_handle);
> +
> +	/* decode attribute value table */
> +	val_entry = pldm_bios_table_attr_value_find_by_handle(
> +			bios_val_table, bios_val_length, attr_handle);
> +
> +	if (!val_entry) {
> +		prlog(PR_ERR, "Attribute val table entry not found, attr_handle: %d\n",
> +			      attr_handle);
> +		return OPAL_PARAMETER;
> +	}
> +
> +	/* get Current String Itself */
> +	pldm_bios_table_attr_value_entry_string_decode_string(
> +			val_entry,
> +			&currentString);
> +	*current_string = strdup(currentString.ptr);
> +
> +	return OPAL_SUCCESS;
> +}
> +
> +#define HB_LID_IDS "hb_lid_ids"
> +
> +/*
> + * Find lid attribute from bios tables
> + */
> +int pldm_bios_find_lid_by_attr_name(const char *name, char **lid)
> +{
> +	char *hb_lid_ids_string = NULL;
> +	int rc = OPAL_SUCCESS;
> +
> +	if (!bios_ready)
> +		return OPAL_HARDWARE;
> +
> +	/* find current attribut string */
> +	rc = find_bios_table_attr_values_by_string(HB_LID_IDS, &hb_lid_ids_string);
> +	if (rc)
> +		goto err;
> +
> +	/* find lid attribut from current string */
> +	rc = find_lid_by_attr_name(hb_lid_ids_string, name, lid);
> +	if (rc)
> +		goto err;
> +
> +	free(hb_lid_ids_string);
> +
> +	prlog(PR_DEBUG, "%s - lid: %s, attr name: %s\n",
> +			__func__, *lid, name);
> +
> +	return OPAL_SUCCESS;
> +
> +err:
> +	if (hb_lid_ids_string)
> +		free(hb_lid_ids_string);
> +
> +	return rc;
> +}
> +
> +/*
> + * Get lid ids string from bios tables
> + */
> +int pldm_bios_get_lids_id(char **lid_ids_string)
> +{
> +	if (!bios_ready)
> +		return OPAL_HARDWARE;
> +
> +	/* find current attribut string */
> +	return find_bios_table_attr_values_by_string(HB_LID_IDS, lid_ids_string);
> +}
> +
>  /*
>   * Send/receive a PLDM GetBIOSTable request message
>   */
> diff --git a/core/pldm/pldm.h b/core/pldm/pldm.h
> index d90ff84c..4f3d262a 100644
> --- a/core/pldm/pldm.h
> +++ b/core/pldm/pldm.h
> @@ -51,6 +51,8 @@ int pldm_rx_handle_request(struct pldm_rx_data *rx);
>  int pldm_mctp_responder_init(void);
>  
>  /* Requester support */
> +int pldm_bios_find_lid_by_attr_name(const char *name, char **lid);
> +int pldm_bios_get_lids_id(char **lid_ids_string);
>  int pldm_bios_init(void);
>  
>  void pldm_platform_exit(void);
> -- 
> 2.35.1
> 
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot


More information about the Skiboot mailing list