<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<br>
<div class="moz-cite-prefix">Le 18/05/2022 à 09:42, Abhishek Singh
Tomar a écrit :<br>
</div>
<blockquote type="cite"
cite="mid:20220518074304.42497-2-abhishek@linux.ibm.com">
<pre class="moz-quote-pre" wrap="">The patch bypasses hardware-dependent implementation for
PLDM test. This patch provides API that will bypass the
need to exchange messages with BMC over the LPC bus whereas
messages are handled by the same program for self test.
It also contains common functions and declarations that
are used to implement the PLDM self test.
Signed-off-by: Abhishek Singh Tomar <a class="moz-txt-link-rfc2396E" href="mailto:abhishek@linux.ibm.com"><abhishek@linux.ibm.com></a>
---
core/pldm/test/common/test-pldm-common.c | 190 +++++++++++++++++++++++
1 file changed, 190 insertions(+)
create mode 100644 core/pldm/test/common/test-pldm-common.c
</pre>
</blockquote>
<br>
<font size="2">why did you create a subfolder "common" ?</font><br>
<br>
<blockquote type="cite"
cite="mid:20220518074304.42497-2-abhishek@linux.ibm.com">
<pre class="moz-quote-pre" wrap="">
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..4acbfd35
--- /dev/null
+++ b/core/pldm/test/common/test-pldm-common.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
+// Copyright 2022 IBM Corp.
+
+#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"
+
+
+
+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);
+</pre>
</blockquote>
<br>
<font size="2">I am not so fan about the global functions. So,
rather than declaring<br>
these functions in each test file, we could instead have a common
list, <br>
each test would record its type and the functions to call.</font><br>
<br>
<blockquote type="cite"
cite="mid:20220518074304.42497-2-abhishek@linux.ibm.com">
<pre class="moz-quote-pre" wrap="">
+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;
+}
+
+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 OPAL_SUCCESS;
+ return OPAL_PARAMETER;
+}
+
+int ast_mctp_message_tx(uint8_t eid, uint8_t *msg, int len)
+{
+ int ret;</pre>
</blockquote>
<blockquote type="cite"
cite="mid:20220518074304.42497-2-abhishek@linux.ibm.com">
<pre class="moz-quote-pre" wrap="">
+ 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;
+
+ /* 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 != OPAL_SUCCESS)
+ return ret;
+ }
+</pre>
</blockquote>
<br>
<font size="2">You could simplify the code:<br>
<br>
request = ((struct pldm_msg
*)pldm_received_msg)->hdr.request;<br>
<br>
switch (request) {<br>
case PLDM_RESPONSE:<br>
return pldm_test_verify_response(pldm_received_msg,
len-1);<br>
break;<br>
<br>
case PLDM_REQUEST:<br>
....<br>
<br>
</font>
<blockquote type="cite"
cite="mid:20220518074304.42497-2-abhishek@linux.ibm.com">
<pre class="moz-quote-pre" wrap="">
+ /* 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);</pre>
</blockquote>
<br>
<font size="2">The name of the function is not very explicit.</font><br>
<font size="2">pldm_test_reply_request() seems to build a response
to the Skiboot request. <br>
The response is sent afterwards.</font><br>
<br>
<blockquote type="cite"
cite="mid:20220518074304.42497-2-abhishek@linux.ibm.com">
<pre class="moz-quote-pre" wrap="">
+ if (ret != OPAL_SUCCESS)
+ return ret;
+</pre>
</blockquote>
<br>
blanck is not necessary.<br>
<br>
<blockquote type="cite"
cite="mid:20220518074304.42497-2-abhishek@linux.ibm.com">
<pre class="moz-quote-pre" wrap="">
+
+ /*
+ * If response length > 0 then response back
+ * else if response length == 0 then no need to response
+ */
+ if (response_len > 0) {</pre>
</blockquote>
<font size="2"><br>
Why do you need to test response_len ? <br>
If response_len is equal to 0 or <0, so pldm_test_reply_request
will return an error.<br>
<br>
</font>
<blockquote type="cite"
cite="mid:20220518074304.42497-2-abhishek@linux.ibm.com">
<pre class="moz-quote-pre" wrap="">
+ vmsg = malloc(response_len+1);
+ /* TYPE: PLDM = 0x01 (000_0001b) as per MCTP - DSP0240 */
+ vmsg[0] = 1;
+ memcpy(vmsg + 1, response_msg, response_len);
+ free(response_msg);
+ pldm_rx_message(BMC_EID, 0, 0, NULL, vmsg, response_len+1);
+ free(vmsg);
+ }
+ }
+
+ return OPAL_SUCCESS;
+}
+
+void ast_mctp_exit(void)
+{
+}
+
+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 OPAL_SUCCESS;
+
+}
+
+
+void unlock(struct lock *l)
+{
+ assert(l->lock_val);
+ l->lock_val = 0;
+}
+
+void prd_occ_reset(uint32_t proc)
+{
+ (void)proc;
+}
+
</pre>
</blockquote>
<br>
</body>
</html>