[Skiboot] [PATCH v2 5/9] core/pldm/test : pldm BIOS Specification Self test

Christophe Lombard clombard at linux.vnet.ibm.com
Wed May 11 23:41:15 AEST 2022



Le 07/05/2022 à 08:35, Abhishek Singh Tomar a écrit :
> The patch contains self test for PLDM Bios Specification implementation
>
> Signed-off-by: Abhishek Singh Tomar<abhishek at linux.ibm.com>
> ---
>   core/pldm/pldm-lid-files.c               |  16 +-
>   core/pldm/test/Makefile.check            |   1 +
>   core/pldm/test/common/test_pldm-common.c |   1 +
>   core/pldm/test/test_pldm-bios.c          | 259 +++++++++++++++++++++++
>   4 files changed, 273 insertions(+), 4 deletions(-)
>   create mode 100644 core/pldm/test/test_pldm-bios.c
>
> diff --git a/core/pldm/pldm-lid-files.c b/core/pldm/pldm-lid-files.c
> index e8122268..2d8685d2 100644
> --- a/core/pldm/pldm-lid-files.c
> +++ b/core/pldm/pldm-lid-files.c
> @@ -261,8 +261,11 @@ static int lid_files_read(struct blocklevel_device *bl __unused,
>   	/* LPC is only 32bit */
>   	if (pos > UINT_MAX || len > UINT_MAX)
>   		return FLASH_ERR_PARM_ERROR;
> -
> +#ifndef __TEST__
>   	prlog(PR_TRACE, "lid files read at 0x%llx for 0x%llx\n", pos, len);
> +#else
> +	prlog(PR_TRACE, "lid files read at 0x%lx for 0x%lx\n", pos, len);
> +#endif
>
>   	if (pos == 0) {
>   		/* return a 'fake' header flash */
> @@ -290,9 +293,11 @@ static int lid_files_write(struct blocklevel_device *bl __unused,
>   	/* LPC is only 32bit */
>   	if (pos > UINT_MAX || len > UINT_MAX)
>   		return FLASH_ERR_PARM_ERROR;
> -
> +#ifndef __TEST__
>   	prlog(PR_TRACE, "lid files write at 0x%llx for 0x%llx\n", pos, len);
> -
> +#else
> +	prlog(PR_TRACE, "lid files write at 0x%lx for 0x%lx\n", pos, len);
> +#endif
>   	/* convert offset to lid id */
>   	lid = vaddr_to_lid_id(pos);
>   	if (!lid)
> @@ -306,8 +311,11 @@ static int lid_files_write(struct blocklevel_device *bl __unused,
>   static int lid_files_erase(struct blocklevel_device *bl __unused,
>   			   uint64_t pos, uint64_t len)
>   {
> -
> +#ifndef __TEST__
>   	prlog(PR_TRACE, "lid files erase at 0x%llx for 0x%llx\n", pos, len);
> +#else
> +	prlog(PR_TRACE, "lid files erase at 0x%lx for 0x%lx\n", pos, len);
> +#endif
>   	return OPAL_UNSUPPORTED;
>   }

Same remark as previously about this update.

> diff --git a/core/pldm/test/Makefile.check b/core/pldm/test/Makefile.check
> index 42d60993..ce43dea0 100644
> --- a/core/pldm/test/Makefile.check
> +++ b/core/pldm/test/Makefile.check
> @@ -1,5 +1,6 @@
>   # -*-Makefile-*-
>   PLDM_TEST := core/pldm/test/test_pldm-fileio \
> +	     core/pldm/test/test_pldm-bios \
>
>   LCOV_EXCLUDE += $(PLDM_TEST:%=%.c)
>
> diff --git a/core/pldm/test/common/test_pldm-common.c b/core/pldm/test/common/test_pldm-common.c
> index 33671150..0333937c 100644
> --- a/core/pldm/test/common/test_pldm-common.c
> +++ b/core/pldm/test/common/test_pldm-common.c
> @@ -52,6 +52,7 @@ static inline unsigned long mftb(void);
>   #include "../../pldm-watchdog.c"
>   #include "../../pldm-fru-requests.c"
>   #include "../../pldm-platform-requests.c"
> +#include "../../pldm-lid-files.c"
>   #include "../../../device.c"
>
>
> diff --git a/core/pldm/test/test_pldm-bios.c b/core/pldm/test/test_pldm-bios.c
> new file mode 100644
> index 00000000..2e6330ac
> --- /dev/null
> +++ b/core/pldm/test/test_pldm-bios.c
> @@ -0,0 +1,259 @@
> +// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
> +/*
> + * Copyright 2013-2019 IBM Corp.
> + */
> +
> +
> +#include "common/test_pldm-common.c"
> +
> +
> +#define TEST_BIOS_STRING "hb_lid_ids"
> +#define TEST_BIOS_STRING_HANDLE 60
> +#define TEST_ATTR_HANDLE 0
> +#define TEST_ATTR_STRING_MIN_LEN 0
> +#define TEST_ATTR_STRING_MAX_LEN 0
> +#define TEST_ATTR_STRING_DEFAULT_LEN 0
> +#define TEST_ATTR_STRING_DEFAULT ""
> +#define TEST_VALUE_TABLE_CURRENT_STR "ATTR_PERM=81e00663,ATTR_TMP=81e00664,NVRAM=81e0066b"
> +#define TEST_VALID_ATTR_NAME "ATTR_TMP"
> +
> +
> +/*
> + * This function tries to duplicate BMC functionality for Pldm self test
> + * It will handle pldm response message
> + * For now we don't have any response
> + */
> +int pldm_test_verify_response(void *response_msg, size_t response_len)
> +{
> +	if (response_len > 0 || response_msg != NULL)
> +		return OPAL_PARAMETER;
> +
> +	return OPAL_PARAMETER;
> +
> +}
> +
> +
> +int pldm_test_reply_request_bios(void *request_msg, size_t request_len,
> +		void **response_msg, size_t *response_len);
> +
> +/*
> + * This function tries to duplicate BMC functionality for Pldm self test
> + * It generate bios table for self test based on input parameter tabletype
> + */
> +uint32_t test_table_entry_generate(uint8_t **bios_table, uint8_t tableType)
> +{
> +	uint32_t bios_table_length = 0;
> +	int pad_len = 0;
> +	uint32_t checksum = 0;
> +
> +	switch (tableType) {
> +
> +	case PLDM_BIOS_STRING_TABLE:
> +		bios_table_length = sizeof(struct pldm_bios_string_table_entry)
> +			+ strlen(TEST_BIOS_STRING) - 1;
> +
> +		/* calculate padding length */
> +		if (bios_table_length % 4)
> +			pad_len = 4 - (bios_table_length % 4);
> +		else
> +			pad_len = 0;
> +		bios_table_length += sizeof(uint32_t) + pad_len;
> +
> +		*bios_table = malloc(bios_table_length);
> +		memset(*bios_table, 0, bios_table_length);
> +		struct pldm_bios_string_table_entry *string_entry =
> +			(struct pldm_bios_string_table_entry *)(*bios_table);

You mix declaration and code.

> +		string_entry->string_handle = htole16(TEST_BIOS_STRING_HANDLE);
> +		string_entry->string_length = htole16(strlen(TEST_BIOS_STRING));
> +		memcpy(string_entry->name, TEST_BIOS_STRING, string_entry->string_length);
> +		break;
> +
> +	case PLDM_BIOS_ATTR_TABLE:
> +		struct pldm_bios_table_attr_entry_string_info info;
> +
> +		bios_table_length = sizeof(struct pldm_bios_attr_table_entry)
> +			+ sizeof(struct attr_table_string_entry_fields)
> +			+ strlen(TEST_ATTR_STRING_DEFAULT);
> +
> +		/* calculate padding length */
> +		if (bios_table_length % 4)
> +			pad_len = 4 - (bios_table_length % 4);
> +		else
> +			pad_len = 0;
> +		bios_table_length += sizeof(uint32_t) + pad_len;
> +
> +		*bios_table = malloc(bios_table_length);
> +		memset(*bios_table, 0, bios_table_length);
> +
> +		info.name_handle = TEST_BIOS_STRING_HANDLE;
> +		info.read_only = 0;
> +		info.string_type = PLDM_BIOS_STRING;
> +		info.min_length = TEST_ATTR_STRING_MIN_LEN;
> +		info.max_length = TEST_ATTR_STRING_MAX_LEN;
> +		info.def_length = TEST_ATTR_STRING_DEFAULT_LEN;
> +		info.def_string = malloc(strlen(TEST_ATTR_STRING_DEFAULT));
> +		memcpy((uint8_t *)info.def_string, TEST_ATTR_STRING_DEFAULT,
> +				strlen(TEST_ATTR_STRING_DEFAULT));
> +		pldm_bios_table_attr_entry_string_encode(*bios_table, bios_table_length, &info);
> +		free((uint8_t *)info.def_string);
> +		break;
> +
> +	case PLDM_BIOS_ATTR_VAL_TABLE:
> +		bios_table_length = sizeof(struct pldm_bios_attr_val_table_entry)
> +			+ sizeof(uint16_t) + sizeof(TEST_VALUE_TABLE_CURRENT_STR) - 1;
> +
> +		/* calculate padding length */
> +		if (bios_table_length % 4)
> +			pad_len = 4 - (bios_table_length % 4);
> +		else
> +			pad_len = 0;
> +		bios_table_length += sizeof(uint32_t) + pad_len;
> +
> +		*bios_table = malloc(bios_table_length);
> +		memset(*bios_table, 0, bios_table_length);
> +
> +		pldm_bios_table_attr_value_entry_encode_string(*bios_table, bios_table_length,
> +				TEST_ATTR_HANDLE, PLDM_BIOS_STRING,
> +				sizeof(TEST_VALUE_TABLE_CURRENT_STR),
> +				TEST_VALUE_TABLE_CURRENT_STR);
> +		break;
> +	default:
> +		printf("PLDM_TEST: INvalid Table type");
> +		return OPAL_PARAMETER;
> +
> +	}
> +
> +	/* Add padding data */
> +	memset(*bios_table + bios_table_length - sizeof(uint32_t) - pad_len, 0, pad_len);
> +
> +
> +	checksum = htole32(pldm_crc32(*bios_table, bios_table_length - sizeof(uint32_t)
> +				- pad_len));
> +	memcpy(*bios_table + bios_table_length - sizeof(uint32_t), (void *)&checksum,
> +			sizeof(uint32_t));
> +
> +	return bios_table_length;

It would be better, I think, to add a new parameter for 
bios_table_length and the function
returns a rc to make the whole thing more cohesive.

> +
> +}
> +
> +/*
> + * This function tries to duplicate BMC functionality for Pldm self test
> + * It will only handle PLDM_BIOS type request
> + * As bios test will have only pldm request of type = PLDM_BIOS
> + */
> +int pldm_test_reply_request(void *request_msg, size_t request_len,
> +		void **response_msg, size_t *response_len)
> +{
> +	switch (((struct pldm_msg *)request_msg)->hdr.type) {
> +	case PLDM_BIOS:
> +		return pldm_test_reply_request_bios(request_msg, request_len,
> +				response_msg, response_len);
> +
> +	default:
> +		printf("PLDM_TEST : Not equal to PLDM_BIOS\n");

or printf("PLDM_TEST : hdr type (%...) not supported\n", (struct 
pldm_msg *)request_msg)->hdr.type);

> +		return OPAL_PARAMETER;
> +	}
> +
> +}
> +
> +/*
> + * This function tries to duplicate BMC functionality for Pldm self test
> + * it tries to handle PLDM_REQUEST for PLDM_BIOS and reply with appropriate
> + * PLDM_RESPONSE message
> + */
> +int pldm_test_reply_request_bios(void *request_msg, size_t request_len,
> +		void **response_msg, size_t *response_len)
> +{
> +	int ret;
> +	uint32_t transfer_handle;
> +	uint8_t transfer_op_flag, table_type;
> +	uint8_t *bios_table;
> +	size_t payload_length;
> +
> +
> +
> +	/*
> +	 * check if command send is PLDM_GET_BIOS_TABLE then only
> +	 * reply response message and return PLDM_SUCCESS
> +	 * else return error
> +	 */
> +	if (((struct pldm_msg *)request_msg)->hdr.command == PLDM_GET_BIOS_TABLE) {
> +		payload_length = request_len - sizeof(struct pldm_msg_hdr);
> +		ret = decode_get_bios_table_req(request_msg, payload_length, &transfer_handle,
> +				&transfer_op_flag, &table_type);
> +		if (ret != PLDM_SUCCESS)
> +			return ret;
> +
> +		/*  generate the table to reply request on behalf on BMC for PLDM self test */
> +		ret = test_table_entry_generate(&bios_table, table_type);
> +		if (ret < PLDM_SUCCESS)
> +			return ret;
> +		payload_length = ret + sizeof(struct pldm_get_bios_table_resp) - 1;
> +
> +		*response_len = sizeof(struct pldm_msg_hdr)
> +			+ payload_length - 1;
> +		*response_msg = malloc(*response_len);
> +
> +		ret = encode_get_bios_table_resp(((struct pldm_msg *)request_msg)->hdr.instance_id,
> +				PLDM_SUCCESS, PLDM_GET_NEXTPART, PLDM_START_AND_END,
> +				bios_table, payload_length, *response_msg);
> +		free(bios_table);
> +		if (ret != PLDM_SUCCESS)
> +			return ret;
> +
> +		return OPAL_SUCCESS;
> +	} else
> +		return OPAL_PARAMETER;
> +
> +	return OPAL_SUCCESS;
> +}
> +
> +
> +int main(void)
> +{
> +	size_t ret;
> +	char *lid;
> +	char name[] = "Error";
> +	struct blocklevel_device *bl;
> +
> +	/*
> +	 * Attempt to call pldm_bios_find_lid_by_attr_name()
> +	 * before pldm_bios_init() return error OPAL_HARDWARE
> +	 */
> +	ret = pldm_bios_find_lid_by_attr_name(name, &lid);
> +	if (ret  != OPAL_HARDWARE)

You could put an error message.

> +		return ret;
> +
> +       /* Init Pldm bios */
> +	ret = pldm_bios_init();
> +	if (ret  != PLDM_SUCCESS)
> +		return ret;
> +
> +
> +       /*
> +	* Attempt to call pldm_bios_find_lid_by_attr_name()
> +	* when name argument not present return error OPAL_PARAMETER
> +	*/
> +	ret = pldm_bios_find_lid_by_attr_name(TEST_VALID_ATTR_NAME, &lid);
> +	if (ret  != PLDM_SUCCESS)

pldm_bios_find_lid_by_attr_name() only returns OPAL_ return code.

> +		return ret;
> +
> +
> +	/*
> +	 * Attempt to call pldm_bios_find_lid_by_attr_name()
> +	 * when name argument  present return PLDM_SUCCESS
> +	 */
> +	ret = pldm_bios_find_lid_by_attr_name(TEST_VALID_ATTR_NAME, &lid);
> +	if (ret  != PLDM_SUCCESS)

same remark here.

> +		return ret;
> +
> +
> +	/* Init pldm_lid_files_init */
> +	ret = pldm_lid_files_init(&bl);
> +	if (ret  != PLDM_SUCCESS)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/skiboot/attachments/20220511/ee6f74c2/attachment.htm>


More information about the Skiboot mailing list