[Skiboot] [PATCH v2 1/9] core/pldm/test : pldm self test common

[Abhishek SIngh Tomar]

On Tue, May 10, 2022 at 03:48:40PM +0200, Christophe Lombard wrote:
> 
> Le 07/05/2022 à 08:35, Abhishek Singh Tomar a écrit :
> > The patch contain common file to be used for PLDM self test.
> > It bypass hardware dependent code for PLDM implementation.
> > 
> > Signed-off-by: Abhishek Singh Tomar<abhishek at linux.ibm.com>
> > ---
> >   core/pldm/test/common/test_pldm-common.c | 187 +++++++++++++++++++++++
> >   1 file changed, 187 insertions(+)
> >   create mode 100644 core/pldm/test/common/test_pldm-common.c
> > 
> > diff --git a/core/pldm/test/common/test_pldm-common.c b/core/pldm/test/common/test_pldm-common.c
> > new file mode 100644
> > index 00000000..33671150
> > --- /dev/null
> > +++ b/core/pldm/test/common/test_pldm-common.c
> > @@ -0,0 +1,187 @@
> > +// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
> > +/*
> > + * Copyright 2013-2019 IBM Corp.
> > + */
> > +
> 
> 2022
ok
> 
> > +
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <stdarg.h>
> > +#include <stdbool.h>
> > +#include <types.h>
> > +#include <stdint.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <time.h>
> > +#include <timer.h>
> > +#include <ccan/list/list.h>
> > +#include <ccan/short_types/short_types.h>
> > +
> > +#define __LITTLE_ENDIAN_BITFIELD
> > +#define __TEST__
> > +#define __SKIBOOT__
> > +#define zalloc(bytes) calloc((bytes), 1)
> > +static inline unsigned long mftb(void);
> > +#include <timebase.h>
> > +#include <op-panel.h>
> > +#include <include/platform.h>
> > +#include "../../pldm.h"
> > +#include <include/pldm.h>
> > +#include <ast.h>
> > +#ifdef ARRAY_SIZE
> > +#undef ARRAY_SIZE
> > +#endif
> > +
> > +
> > +#include <pldm/libpldm/bios_table.h>
> > +#include <pldm/libpldm/bios_table.c>
> > +#undef pr_fmt
> > +#include "../../pldm-bios-requests.c"
> > +#include <pldm/libpldm/base.c>
> > +#include <pldm/libpldm/pdr.c>
> > +#include <pldm/libpldm/bios.c>
> > +#include <pldm/libpldm/platform.c>
> > +#include <pldm/libpldm/utils.c>
> > +#include <pldm/ibm/libpldm/file_io.c>
> > +#include <pldm/libpldm/fru.c>
> > +#include "../../pldm-file-io-requests.c"
> > +#include "../../pldm-requester.c"
> > +#include "../../pldm-common.c"
> > +#include "../../pldm-responder.c"
> > +#include "../../pldm-base-requests.c"
> > +#include "../../pldm-watchdog.c"
> > +#include "../../pldm-fru-requests.c"
> > +#include "../../pldm-platform-requests.c"
> > +#include "../../../device.c"
> > +
> > +
> 
> any reason on include all these files ?
It seems that one file is dependent on other
> 
> > +
> > +char __rodata_start[1], __rodata_end[1];
> > +unsigned long tb_hz = 512000000;
> > +struct dt_node *dt_root;
> > +struct debug_descriptor debug_descriptor;
> > +struct platform platform;
> > +
> > +int pldm_test_reply_request(void *request_msg, size_t request_len,
> > +		void **response_msg, size_t *response_len);
> > +int pldm_test_verify_response(void *response_msg, size_t response_len);
> > +
> 
> Rather than declaring these functions in each test file, we could instead
> have a
> common list, each test would record its type and the functions to call.
> 
> > +void time_wait_ms(unsigned long ms)
> > +{
> > +	usleep(ms * 1000);
> > +}
> > +void init_timer(struct timer *t, timer_func_t expiry, void *data)
> > +{
> > +	t->link.next = t->link.prev = NULL;
> > +	t->target = 0;
> > +	t->expiry = expiry;
> > +	t->user_data = data;
> > +	t->running = NULL;
> > +}
> > +uint64_t schedule_timer(struct timer *t, uint64_t how_long)
> > +{
> > +	if (t != NULL)
> > +		return how_long;
> > +	return 0;
> > +}
> > +void cancel_timer(struct timer *t)
> > +{
> > +	t->link.next = t->link.prev = NULL;
> > +}
> > +
> > +static inline unsigned long mftb(void)
> > +{
> > +	unsigned long clk;
> > +
> > +	clk = clock();
> > +	return clk;
> > +}
> 
> why do you declare these previous fonctions ?
In self-test, multiple functions definitions are required but their
definitions are not visible to that test as their files are not included
in the test as they are hardware-dependent or other functions in their
files are hardware-dependent so the approach I have seen being followed
in self-test is writing an equivalent function that has no hardware
dependency.
> 
> > +
> > +int ast_mctp_init(void (*fn)(uint8_t src_eid, bool tag_owner, uint8_t msg_tag, void *data,
> > +			void *msg, size_t len))
> > +{
> > +	if (fn != NULL)
> > +		return PLDM_SUCCESS;
> > +	return PLDM_ERROR_INVALID_DATA;
> > +}
> > +
> > +int ast_mctp_message_tx(uint8_t eid, uint8_t *msg, int len)
> > +{
> > +	int ret;
> > +	uint8_t *pldm_received_msg = msg+1;
> > +	void *response_msg;
> > +	char *vmsg;
> > +	size_t response_len;
> > +
> > +	/* TEST eid is BMC_ID */
> > +	if (eid != BMC_EID)
> > +		return OPAL_PARAMETER;
> > +
> 
> You mix return code OPAL and PLDM
ok
> 
> > +	/* TEST Message TYPE: PLDM = 0x01 (000_0001b) as per MCTP - DSP0240 */
> > +	if (msg[0] != 0x01) {
> > +		printf("TEST : %s : request MCTP message type not set for PLDM\n", __func__);
> > +		return OPAL_PARAMETER;
> > +	}
> > +
> > +	if (((struct pldm_msg *)pldm_received_msg)->hdr.request == PLDM_RESPONSE) {
> > +		ret = pldm_test_verify_response(pldm_received_msg, len-1);
> > +		if (ret != PLDM_SUCCESS)
> > +			return ret;
> > +	}
> > +
> > +	/* Reply to requests */
> > +	else if (((struct pldm_msg *)pldm_received_msg)->hdr.request == PLDM_REQUEST) {
> > +		ret = pldm_test_reply_request(pldm_received_msg, len-1,
> > +				&response_msg, &response_len);
> > +		if (ret != PLDM_SUCCESS)
> > +			return ret;
> > +		vmsg = malloc(response_len+1);
> > +		/*  TYPE: PLDM = 0x01 (000_0001b) as per MCTP - DSP0240 */
> > +		vmsg[0] = 0x01;
> > +
> > +		memcpy(vmsg + 1, response_msg, response_len);
> > +
> > +		pldm_rx_message(BMC_EID, 0, 0, NULL, vmsg, response_len+1);
> 
> vmsg is not freed.
ok
> 
> > +	}
> > +
> > +	return PLDM_SUCCESS;
> > +}
> > +
> > +void ast_mctp_exit(void)
> > +{
> > +	return;
> > +}
> > +
> > +void lock_caller(struct lock *l, const char *caller)
> > +{
> > +	(void)caller;
> > +	assert(!l->lock_val);
> > +	l->lock_val++;
> > +}
> > +
> > +int _opal_queue_msg(enum opal_msg_type msg_type, void *data,
> > +		void (*consumed)(void *data, int status),
> > +		size_t params_size, const void *params)
> > +{
> > +	(void)msg_type;
> > +	if (data != NULL || consumed != NULL)
> > +		return OPAL_PARAMETER;
> > +	if (params != NULL && params_size > 0)
> > +		return OPAL_PARAMETER;
> > +	return PLDM_SUCCESS;
> > +
> > +}
> > +
> > +
> > +void unlock(struct lock *l)
> > +{
> > +	assert(l->lock_val);
> > +	l->lock_val = 0;
> > +}
> > +
> > +void prd_occ_reset(uint32_t proc)
> > +{
> > +	(void)proc;
> > +}
> > +