[Skiboot] [PATCH V2 10/15] core/pldm: Decode the GetFRURecordTableMetadata request
Christophe Lombard
clombard at linux.ibm.com
Wed May 17 00:13:41 AEST 2023
Le 26/04/2023 à 17:00, Frederic Barrat a écrit :
>
>
> On 29/04/2022 11:47, Christophe Lombard wrote:
>> The GetFRURecordTableMetadata command is used to get the FRU Record
>> Table metadata information that includes the FRU Record major version,
>> the FRU Record minor version, the size of the largest FRU Record data,
>> total length of the FRU Record Table, total number of FRU Record Data
>> structures, and the integrity checksum on the FRU Record Table data.
>>
>> Add an "IBM, skiboot" FRU Record product requested by the BMC.
>>
>> Signed-off-by: Christophe Lombard <clombard at linux.vnet.ibm.com>
>> ---
>> core/pldm/pldm-fru-requests.c | 53 ++++++++++++++++++++++
>> core/pldm/pldm-responder.c | 85 +++++++++++++++++++++++++++++++++++
>> core/pldm/pldm.h | 3 ++
>> 3 files changed, 141 insertions(+)
>>
>> diff --git a/core/pldm/pldm-fru-requests.c
>> b/core/pldm/pldm-fru-requests.c
>> index eda8f939..6da1fb55 100644
>> --- a/core/pldm/pldm-fru-requests.c
>> +++ b/core/pldm/pldm-fru-requests.c
>> @@ -11,6 +11,9 @@
>> #include <pldm/libpldm/fru.h>
>> #include "pldm.h"
>> +static void *fru_table;
>> +static size_t fru_table_length;
>> +
>> static int get_fru_record_by_option_req(uint16_t fru_table_handle,
>> uint16_t record_set_identifier,
>> uint8_t record_type,
>> @@ -146,3 +149,53 @@ int pldm_fru_dt_add_bmc_version(void)
>> return OPAL_SUCCESS;
>> }
>> +
>> +#define RECORD_SET_ID 100
>> +
>> +void pldm_fru_add_record(uint32_t *table_length,
>> + uint16_t *total_record_set_identifiers,
>> + uint16_t *total_table_records)
>> +{
>> + struct pldm_fru_record_data_format *record;
>> + struct pldm_fru_record_tlv *fru_tlv;
>> + size_t fru_table_size, record_size;
>> + char fru_product[] = "IBM, skiboot";
>> +
>> + if (fru_table) {
>> + *table_length = fru_table_length;
>> + *total_record_set_identifiers = 1;
>> + *total_table_records = 1;
>> + return;
>> + }
>> +
>> + /* allocate fru table */
>> + fru_table_size = sizeof(struct pldm_fru_record_data_format) +
>> + sizeof(struct pldm_fru_record_tlv) +
>> + strlen(fru_product);
>> + fru_table = malloc(fru_table_size);
>
>
> The newer version of this patch has a zalloc(), which is good, but we
> still don't check for the return value.
>
>
>> +
>> + /* fill fru record data */
>> + record = (struct pldm_fru_record_data_format *)fru_table;
>> + record->record_set_id = htole16(RECORD_SET_ID);
>> + record->record_type = PLDM_FRU_RECORD_TYPE_GENERAL;
>> + record->num_fru_fields = 1;
>> + record->encoding_type = PLDM_FRU_ENCODING_ASCII;
>> +
>> + /* to start, set the size as the start of the TLV structs */
>> + record_size = offsetof(struct pldm_fru_record_data_format, tlvs);
>> +
>> + /* TLVs data */
>> + fru_tlv = (struct pldm_fru_record_tlv *)(fru_table + record_size);
>> + fru_tlv->type = PLDM_FRU_FIELD_TYPE_OTHER;
>> + fru_tlv->length = strlen(fru_product);
>> + memcpy(fru_tlv->value, fru_product, fru_tlv->length);
>> +
>> + /* increment record_size by total size of this TLV */
>> + record_size += (offsetof(struct pldm_fru_record_tlv, value) +
>> fru_tlv->length);
>> +
>> + *table_length = record_size;
>> + *total_record_set_identifiers = 1;
>> + *total_table_records = 1;
>> +
>> + fru_table_length = *table_length;
>> +}
>> diff --git a/core/pldm/pldm-responder.c b/core/pldm/pldm-responder.c
>> index e8773beb..f11317c5 100644
>> --- a/core/pldm/pldm-responder.c
>> +++ b/core/pldm/pldm-responder.c
>> @@ -11,6 +11,7 @@
>> #include <opal-msg.h>
>> #include <debug_descriptor.h>
>> #include <pldm/ibm/libpldm/platform_oem_ibm.h>
>> +#include <pldm/libpldm/fru.h>
>> #include <pldm/libpldm/platform.h>
>> #include <pldm/libpldm/state_set.h>
>> #include <pldm/libpldm/utils.h>
>> @@ -902,6 +903,86 @@ static struct pldm_cmd pldm_platform_get_pdr = {
>> .handler = platform_get_pdr_handle,
>> };
>> +/*
>> + * PLDM Fru commands support
>> + */
>> +static struct pldm_type pldm_fru_type = {
>> + .name = "fru",
>> + .pldm_type_id = PLDM_FRU,
>> +};
>> +
>> +/* currently we support version 1.0 of fru table */
>> +#define SUPPORTED_FRU_VERSION_MAJOR 1
>> +#define SUPPORTED_FRU_VERSION_MINOR 0
>> +
>> +/* Used by the metadata request handler for the value of
>> + * FRUTableMaximumSize
>> + * 0 means SetFRURecordTable command is not supported (see DSP 0257
>> + * v1.0.0 Table 9)
>> + */
>> +#define FRU_TABLE_MAX_SIZE_UNSUPPORTED 0
>> +
>> +/*
>> + * GetFRURecordTableMetadata (0X01)
>> + * The GetFRURecordTableMetadata command is used to get the FRU Record
>> + * Table metadata information that includes the FRU Record major
>> + * version, the FRU Record minor version, the size of the largest FRU
>> + * Record data, total length of the FRU Record Table, total number of
>> + * FRU Record Data structures, and the integrity checksum on the FRU
>> + * Record Table data.
>> + */
>> +static int fru_get_record_table_metadata_handler(const struct
>> pldm_rx_data *req)
>> +{
>> + char response_msg[PKT_SIZE(struct
>> pldm_get_fru_record_table_metadata_resp)];
>> + uint16_t total_record_set_identifiers, total_table_records;
>> + uint32_t fru_table_length;
>> + int rc;
>> +
>> + /*
>> + * GetFRURecordTableMetadataGetFRURecordTableMetadata requests
>> + * don't have any payload, so no need to decode them
>> + */
>> +
>> + /* add specific fru record */
>> + pldm_fru_add_record(&fru_table_length,
>> + &total_record_set_identifiers,
>> + &total_table_records);
>
>
> Somehow, the name of that function threw me off. It seems we're adding
> a record but all the parameters are "out" parameters, so it is
> surprising at first. Now, I understand what we're doing, but maybe
> rename the function to pldm_fru_create_local_record_table() ?
>
Correct. Thanks
> Fred
>
>
>> +
>> + /* create a PLDM response message for GetFRURecordTableMetadata */
>> + rc = encode_get_fru_record_table_metadata_resp(
>> + req->hdrinf.instance,
>> + PLDM_SUCCESS,
>> + SUPPORTED_FRU_VERSION_MAJOR,
>> + SUPPORTED_FRU_VERSION_MINOR,
>> + FRU_TABLE_MAX_SIZE_UNSUPPORTED,
>> + fru_table_length,
>> + total_record_set_identifiers,
>> + total_table_records,
>> + 0, // checksum, not calculated
>> + (struct pldm_msg *)response_msg);
>> + if (rc != PLDM_SUCCESS) {
>> + prlog(PR_ERR, "Encode GetFRURecordTableMetadata Error, rc:
>> %d\n", rc);
>> + pldm_cc_resp(req, req->hdrinf.pldm_type,
>> + req->hdrinf.command, PLDM_ERROR);
>> + return OPAL_PARAMETER;
>> + }
>> +
>> + /* send PLDM message over MCTP */
>> + rc = pldm_send(req->source_eid, response_msg,
>> sizeof(response_msg));
>> + if (rc) {
>> + prlog(PR_ERR, "Failed to send GetFRURecordTableMetadata
>> response, rc = %d\n", rc);
>> + return OPAL_HARDWARE;
>> + }
>> +
>> + return OPAL_SUCCESS;
>> +}
>> +
>> +static struct pldm_cmd pldm_fru_get_record_table_metadata = {
>> + .name = "PLDM_GET_FRU_RECORD_TABLE_METADATA",
>> + .pldm_cmd_id = PLDM_GET_FRU_RECORD_TABLE_METADATA,
>> + .handler = fru_get_record_table_metadata_handler,
>> +};
>> +
>> int pldm_rx_handle_request(struct pldm_rx_data *rx)
>> {
>> const struct pldm_type *t;
>> @@ -950,5 +1031,9 @@ int pldm_mctp_responder_init(void)
>> pldm_add_cmd(&pldm_platform_type,
>> &pldm_platform_set_state_effecter_states);
>> pldm_add_cmd(&pldm_platform_type, &pldm_platform_get_pdr);
>> + /* Register platform commands we'll respond to - DSP0257 */
>> + pldm_add_type(&pldm_fru_type);
>> + pldm_add_cmd(&pldm_fru_type, &pldm_fru_get_record_table_metadata);
>> +
>> return OPAL_SUCCESS;
>> }
>> diff --git a/core/pldm/pldm.h b/core/pldm/pldm.h
>> index 8ec72b77..d0ceb546 100644
>> --- a/core/pldm/pldm.h
>> +++ b/core/pldm/pldm.h
>> @@ -88,6 +88,9 @@ int pldm_fru_get_record_by_option(uint16_t
>> fru_table_handle,
>> uint8_t record_type,
>> uint8_t field_type,
>> struct variable_field *fru_structure_data);
>> +void pldm_fru_add_record(uint32_t *fru_table_length,
>> + uint16_t *total_record_set_identifiers,
>> + uint16_t *total_table_records);
>> int pldm_bios_find_lid_by_attr_name(const char *name, char **lid);
>> int pldm_bios_get_lids_id(char **lid_ids_string);
More information about the Skiboot
mailing list