[Skiboot] [PATCH V2 15/21] core/pldm: Find lid attribute from bios tables
Christophe Lombard
clombard at linux.vnet.ibm.com
Fri Mar 18 03:38:53 AEDT 2022
Le 16/03/2022 à 10:09, Abhishek SIngh Tomar a écrit :
> 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
Thanks for this review.
I am going to correct this issue.
>> + * 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,
>> + ¤tString);
>> + *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