Get Memory info from ZOCL using xgq for Built in ps kernel #7564

saifuddin-xilinx · 2023-05-20T08:27:27Z

Problem solved by the commit

-- Updated the memory manager logic. Now DRM memory manager is initialized with 0 to MAX(uint64).
-- Memory manager will be initialized once with drm init and live till the driver active.
-- Memory allocation will be done based on the BANK information

-- Added a support to communicate the ZOCL memory range using XGQ configuration
-- Added a new query_mem command to exchange the memory info between ZOCL and XOCL
-- For PS kernel memory will be allocated with the configured range

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

-- N/A

How problem was solved, alternative solutions (if any) and why they were rejected

N/A

Risks (if any) associated the changes in the commit

N/A

What has been tested and how, request additional testing if necessary

Documentation impact (if any)

Signed-off-by: Saif <saifuddi@amd.com>

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

gbuildx · 2023-05-20T10:37:30Z

Build failed :(

dayeh-xilinx · 2023-05-21T08:22:10Z

retest this please.

gbuildx · 2023-05-21T11:16:21Z

Build failed :(

saifuddin-xilinx · 2023-05-21T16:02:40Z

retest this please

gbuildx · 2023-05-21T16:43:41Z

Build failed :(

dayeh-xilinx · 2023-05-21T17:56:03Z

retest this please.

gbuildx · 2023-05-21T20:14:48Z

Build failed :(

dayeh-xilinx · 2023-05-21T22:21:54Z

retest this please.

gbuildx · 2023-05-22T00:20:05Z

Build failed :(

gbuildx · 2023-05-22T07:05:13Z

Build Passed!

mamin506

Some question and minor changes.

mamin506 · 2023-05-22T18:29:47Z

src/runtime_src/core/edge/drm/zocl/zocl_ctrl_ert.c

+		r->h_mem_info = (u32)((zdev->host_mem & 0xFFFFFFFF00000000LL) >> 32);
+		break;
+
+	case XGQ_CMD_QUERY_MEM_SIZE:
+		r->l_mem_info = (u32)zdev->host_mem_len;
+		r->h_mem_info = (u32)((zdev->host_mem_len & 0xFFFFFFFF00000000LL) >> 32);


Value "0xFFFFFFFF00000000LL" shows up twice, right? Can we define it as a macro?

mamin506 · 2023-05-22T18:32:38Z

src/runtime_src/core/pcie/driver/linux/xocl/userpf/xocl_drm.c

-        drm_p->cma_bank_idx = -1;
-	/* Memory manager initialization is done in two phase. 
-	 * Phase 1 is done based on platform and Phase 2 is done 
-	 * based on XCLBIN. We have done phase 1 here.
-	 */
-        drm_p->xocl_drm_mm_done = true;
-


Can you explain why this code can be removed?

I have sent a mail with the details.

okay, looks like drm_p->xocl_drm_mm_done is not used anymore. Can we delete it in xocl_drm.h?

mamin506 · 2023-05-22T18:39:20Z

src/runtime_src/core/pcie/driver/linux/xocl/userpf/xocl_drm.c

 {
 	int err = 0;
 	int i = 0;
+	uint64_t mm_end_addr = 0xffffFFFFffffFFFF;


In Linux, you can use macro "U64_MAX"

mamin506 · 2023-05-22T18:41:03Z

src/runtime_src/core/pcie/driver/linux/xocl/userpf/xocl_drm.c

-
-	xocl_info(drm_p->ddev->dev, "drm_mm_init called for the available memory range"
-			" <%llx - %llx>", mm_start_addr, mm_end_addr);
+	xocl_info(drm_p->ddev->dev, "drm_mm_init called for the maximum memory range possible");


Why deleted <%llx - %llx>?

We are initializing the max range of memory. Hence, not specifically define any range

mamin506 · 2023-05-22T18:42:18Z

src/runtime_src/core/pcie/driver/linux/xocl/userpf/xocl_drm.c

@@ -1107,6 +1070,7 @@ static int xocl_cleanup_memory_manager(struct xocl_drm *drm_p)
        return 0;
 }

+#if 0


Is this code dead? Why comment it out but not delete it?

mamin506 · 2023-05-22T18:49:15Z

src/runtime_src/core/pcie/driver/linux/xocl/userpf/xocl_kds.c

+
+			ret = xocl_kds_xgq_query_mem(xdev, &XOCL_DRM(xdev)->ps_mem_data); 
+			if (ret) 
+	            userpf_info(xdev, "WARN ! Device doesn't configure for PS Kernel memory\n");


Indentation?

mamin506 · 2023-05-22T18:53:42Z

Hi @saifuddin-xilinx , is there any change in the mem topology section of xclbin?

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

gbuildx · 2023-05-24T14:06:56Z

Build failed :(

saifuddin-xilinx · 2023-05-24T15:40:56Z

retest this please

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

…/XRT into built_in_ps_kernel_support

gbuildx · 2023-05-24T18:10:27Z

Build failed :(

dayeh-xilinx · 2023-05-24T18:28:49Z

retest this please.

jeffli-xilinx · 2023-05-24T18:30:47Z

src/runtime_src/core/include/xgq_cmd_common.h

@@ -112,6 +112,7 @@ enum xgq_cmd_opcode {
 	XGQ_CMD_OP_DATA_INTEGRITY   	= 0x10e,
 	XGQ_CMD_OP_EXIT             	= 0x10f,
 	XGQ_CMD_OP_UNCFG_CU	        = 0x110,
+	XGQ_CMD_OP_QUERY_MEM	        = 0x111,


Please remember to update XGQ command spec page with this new command.
https://confluence.xilinx.com/pages/viewpage.action?pageId=302345261

Sure
Thanks

gbuildx · 2023-05-24T20:09:49Z

Build failed :(

dayeh-xilinx · 2023-05-24T23:39:21Z

retest this please.

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

gbuildx · 2023-05-25T07:18:46Z

Build failed :(

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

gbuildx · 2023-05-25T18:59:44Z

Build failed :(

saifuddin-xilinx · 2023-05-26T04:48:20Z

retest this please

gbuildx · 2023-05-26T07:32:35Z

Build failed :(

saifuddin-xilinx · 2023-05-29T09:32:56Z

retest this please

gbuildx · 2023-05-29T11:27:48Z

Build failed :(

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

gbuildx · 2023-05-30T15:37:45Z

Build Passed!

* Fixed xgq assignment issue for multislot Signed-off-by: Saif <saifuddi@amd.com> * Fixed allignment issues Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Cleanup memory manager for built in ps kernel support Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Get the ps kernel mem info from zocl through xgq * Fix issue to Get the ps kernel mem info from zocl through xgq Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Not to fail the driver is PS kernel memory is not configured * Fixed review comments Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Added memory management changes to this branch Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Delete unused variable Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Fixed one issue Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Remove the multislot fix from PS memory fix branch Signed-off-by: Saifuddin <saifuddi@xilinx.com> --------- Signed-off-by: Saif <saifuddi@amd.com> Signed-off-by: Saifuddin <saifuddi@xilinx.com> Co-authored-by: Saif <saifuddi@amd.com> (cherry picked from commit 47b38ea)

…7596) * Fixed xgq assignment issue for multislot Signed-off-by: Saif <saifuddi@amd.com> * Fixed allignment issues Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Cleanup memory manager for built in ps kernel support Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Get the ps kernel mem info from zocl through xgq * Fix issue to Get the ps kernel mem info from zocl through xgq Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Not to fail the driver is PS kernel memory is not configured * Fixed review comments Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Added memory management changes to this branch Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Delete unused variable Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Fixed one issue Signed-off-by: Saifuddin <saifuddi@xilinx.com> * Remove the multislot fix from PS memory fix branch Signed-off-by: Saifuddin <saifuddi@xilinx.com> --------- Signed-off-by: Saif <saifuddi@amd.com> Signed-off-by: Saifuddin <saifuddi@xilinx.com> Co-authored-by: Saif <saifuddi@amd.com> (cherry picked from commit 47b38ea) Co-authored-by: Saifuddin Kaijar <54270703+saifuddin-xilinx@users.noreply.github.com>

Saif and others added 5 commits May 18, 2023 12:10

Fixed xgq assignment issue for multislot

d08e310

Signed-off-by: Saif <saifuddi@amd.com>

Fixed allignment issues

c9028fb

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

Cleanup memory manager for built in ps kernel support

90d1bcb

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

Get the ps kernel mem info from zocl through xgq

900b45f

Fix issue to Get the ps kernel mem info from zocl through xgq

f51e443

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

saifuddin-xilinx requested review from maxzhen, chvamshi-xilinx, mamin506 and stsoe as code owners May 20, 2023 08:27

Not to fail the driver is PS kernel memory is not configured

ab6b5a7

mamin506 requested changes May 22, 2023

View reviewed changes

Fixed review comments

0b024db

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

saifuddin-xilinx requested a review from mamin506 May 24, 2023 15:35

saifuddin-xilinx added 2 commits May 24, 2023 21:25

Added memory management changes to this branch

e60a432

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

Merge branch 'multislot_fixes' of https://github.com/saifuddin-xilinx…

fcdc8e2

…/XRT into built_in_ps_kernel_support

saifuddin-xilinx requested a review from jeffli-xilinx May 24, 2023 17:06

mamin506 approved these changes May 24, 2023

View reviewed changes

jeffli-xilinx approved these changes May 24, 2023

View reviewed changes

Delete unused variable

17c1ee7

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

chvamshi-xilinx approved these changes May 25, 2023

View reviewed changes

Fixed one issue

a7d83c2

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

maxzhen approved these changes May 29, 2023

View reviewed changes

saifuddin-xilinx and others added 2 commits May 30, 2023 18:36

Merge branch 'Xilinx:master' into built_in_ps_kernel_support

b2aa40c

Remove the multislot fix from PS memory fix branch

a898460

Signed-off-by: Saifuddin <saifuddi@xilinx.com>

chvamshi-xilinx merged commit 47b38ea into Xilinx:master May 30, 2023

Get Memory info from ZOCL using xgq for Built in ps kernel #7564

Get Memory info from ZOCL using xgq for Built in ps kernel #7564

Conversation

saifuddin-xilinx commented May 20, 2023

Problem solved by the commit

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

How problem was solved, alternative solutions (if any) and why they were rejected

Risks (if any) associated the changes in the commit

What has been tested and how, request additional testing if necessary

Documentation impact (if any)

gbuildx commented May 20, 2023

dayeh-xilinx commented May 21, 2023

gbuildx commented May 21, 2023

saifuddin-xilinx commented May 21, 2023

gbuildx commented May 21, 2023

dayeh-xilinx commented May 21, 2023

gbuildx commented May 21, 2023

dayeh-xilinx commented May 21, 2023

gbuildx commented May 22, 2023

gbuildx commented May 22, 2023

mamin506 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mamin506 commented May 22, 2023

gbuildx commented May 24, 2023

saifuddin-xilinx commented May 24, 2023

gbuildx commented May 24, 2023

dayeh-xilinx commented May 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gbuildx commented May 24, 2023

dayeh-xilinx commented May 24, 2023

gbuildx commented May 25, 2023

gbuildx commented May 25, 2023

saifuddin-xilinx commented May 26, 2023

gbuildx commented May 26, 2023

saifuddin-xilinx commented May 29, 2023

gbuildx commented May 29, 2023

gbuildx commented May 30, 2023