W dniu 7.12.2023 o 11:26, Xiong Yining pisze:

DeviceTree created by QEMU contain the information of memory, We can parsed this information and pass them to EDK2 via SMC call.

Signed-off-by: Xiong Yining xiongyining1480@phytium.com.cn Signed-off-by: Chen Baozi chenbaozi@phytium.com.cn Change-Id: I054b6a26fd38ace9898d221eabc70fea09e45f25

For multiple memory nodes I use those qemu arguments:

-smp 4,sockets=4,maxcpus=4 -m 4G,slots=2,maxmem=5G -object memory-backend-ram,size=3G,id=m0 -object memory-backend-ram,size=1G,id=m1 -numa node,nodeid=0,cpus=0-1,memdev=m0 -numa node,nodeid=1,memdev=m1,initiator=0 -numa node,nodeid=2,cpus=2-3

("--numa" argument to my "boot-sbsa-ref.sh" script [1])

1. https://github.com/hrw/sbsa-ref-status/blob/main/boot-sbsa-ref.sh

This gives such memory info in DT:

memory@100c0000000 { numa-node-id = <0x01>; reg = <0x100 0xc0000000 0x00 0x40000000>; device_type = "memory"; };

memory@10000000000 { numa-node-id = <0x00>; reg = <0x100 0x00 0x00 0xc0000000>; device_type = "memory"; };

---

plat/qemu/qemu_sbsa/sbsa_sip_svc.c | 84 +++++++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/plat/qemu/qemu_sbsa/sbsa_sip_svc.c b/plat/qemu/qemu_sbsa/sbsa_sip_svc.c index 05ebec472..9657599ae 100644 --- a/plat/qemu/qemu_sbsa/sbsa_sip_svc.c +++ b/plat/qemu/qemu_sbsa/sbsa_sip_svc.c @@ -10,6 +10,7 @@ #include <common/runtime_svc.h> #include <libfdt.h> #include <smccc_helpers.h> +#include <lib/xlat_tables/xlat_tables_v2.h> /* default platform version is 0.0 */ static int platform_version_major; @@ -19,6 +20,7 @@ static int platform_version_minor; #define SMC64_FUNCTION (SMC_FASTCALL | 0x40000000) #define SIP_FUNCTION (SMC64_FUNCTION | 0x02000000) #define SIP_FUNCTION_ID(n) (SIP_FUNCTION | (n)) +#define MAXNODE 128

Less tabs here please.

/*

• We do not use SMCCC_ARCH_SOC_ID here because qemu_sbsa is virtual platform

@@ -28,13 +30,81 @@ static int platform_version_minor; #define SIP_SVC_VERSION SIP_FUNCTION_ID(1) #define SIP_SVC_GET_GIC SIP_FUNCTION_ID(100) #define SIP_SVC_GET_GIC_ITS SIP_FUNCTION_ID(101) +#define SIP_SVC_GET_MEMORY SIP_FUNCTION_ID(300) +#define SIP_SVC_GET_MEMORYNODE_COUNT SIP_FUNCTION_ID(301)

Please make GET_MEMORY_NODE_COUNT to be first and GET_MEMORY_NODE to be second. You call them in that order so why not keeping it here too?

static uint64_t gic_its_addr; +typedef struct{

• uint32_t nodeid;
• uint64_t addr_base;
• uint64_t addr_size;

+} memory_node;

+typedef struct{

• uint32_t num_memnode;
• memory_node memnode[MAXNODE];

+} memory_info;

+static memory_info sbsa_mem_info;

• void sbsa_set_gic_bases(const uintptr_t gicd_base, const uintptr_t gicr_base); uintptr_t sbsa_get_gicd(void); uintptr_t sbsa_get_gicr(void);

+void read_memoryinfo_from_dt(void *dtb) +{

• const fdt32_t *prop;
• int32_t prev, node;
• const char *type;
• int len;
• uint32_t i;

Can it be a name rather than one letter? It is not Fortran.

• uint32_t higher_value, lower_value;
• uint64_t cur_base, cur_size;
• i = 0;
• for (prev = 0;; prev = node) {

for (prev = 0, i = 0;; prev = node) {

• node = fdt_next_node(dtb, prev, NULL);
  

• if (node < 0) {
  

• 	break;
  

• }
  

Can you add comment with DT info we parse here? Like it is with GIC info.

• type = fdt_getprop(dtb, node, "device_type", &len);
  

• if (type && strncmp(type, "memory", len) == 0) {
  

• 	prop = fdt_getprop(dtb, node, "numa-node-id", NULL);
  

• 	if (prop) {
  

• 		sbsa_mem_info.memnode[i].nodeid = fdt32_to_cpu(*prop);
  

• 	}
  

• 	/*
  

• 	 * Get the 'reg' property of this node and
  

• 	 * assume two 8 bytes for base and size.
  

• 	 */
  

• 	prop = fdt_getprop(dtb, node, "reg", &len);
  

• 	if (prop != 0 && len == (2 * sizeof(int64_t))) {
  

• 		higher_value = fdt32_to_cpu(*prop);
  

• 		lower_value = fdt32_to_cpu(*(prop + 1));
  

• 		cur_base = (uint64_t)(lower_value | ((uint64_t)higher_value) << 32);
  

• 		sbsa_mem_info.memnode[i].addr_base = cur_base;
  

• 		higher_value = fdt32_to_cpu(*(prop + 2));
  

• 		lower_value = fdt32_to_cpu(*(prop + 3));
  

• 		cur_size = (uint64_t)(lower_value | ((uint64_t)higher_value) << 32);
  

• 		sbsa_mem_info.memnode[i].addr_size = cur_size;
  

• 		INFO("System RAM@%d: 0x%lx - 0x%lx\n",
  

• 					sbsa_mem_info.memnode[i].nodeid,
  

• 					sbsa_mem_info.memnode[i].addr_base,
  

• 					sbsa_mem_info.memnode[i].addr_base +
  

• 					sbsa_mem_info.memnode[i].addr_size);
  

"sbsa_mem_info.memnode[i].addr_size - 1" here:

INFO: System RAM@1: 0x100c0000000 - 0x10100000000

INFO: System RAM@0: 0x10000000000 - 0x100c0000000

This should rather be:

INFO: System RAM@1: 0x100c0000000 - 0x100ffffffff INFO: System RAM@0: 0x10000000000 - 0x100bfffffff

Also: less tabs here too:

INFO("System RAM@%d: 0x%lx - 0x%lx\n", sbsa_mem_info.memnode[i].nodeid, sbsa_mem_info.memnode[i].addr_base, sbsa_mem_info.memnode[i].addr_base + sbsa_mem_info.memnode[i].addr_size - 1);

• 	}
  

• 	i++;
  

• }
  

• }
• sbsa_mem_info.num_memnode = i;

+}

• void read_platform_config_from_dt(void *dtb) { int node;

@@ -91,6 +161,8 @@ void read_platform_config_from_dt(void *dtb) return; } INFO("GICI base = 0x%lx\n", gic_its_addr);

• read_memoryinfo_from_dt(dtb); }

void read_platform_version(void *dtb) @@ -144,7 +216,7 @@ uintptr_t sbsa_sip_smc_handler(uint32_t smc_fid, u_register_t flags) { uint32_t ns;

• uint64_t index;

We have MAXNODE = 128 above so we can go with uint32_t type.

/* Determine which security state this SMC originated from */ ns = is_caller_non_secure(flags); if (!ns) { @@ -163,6 +235,16 @@ uintptr_t sbsa_sip_smc_handler(uint32_t smc_fid, case SIP_SVC_GET_GIC_ITS: SMC_RET2(handle, NULL, gic_its_addr);

• case SIP_SVC_GET_MEMORY:

• index = x1;
  

• SMC_RET4(handle, NULL,
  

• 				sbsa_mem_info.memnode[index].nodeid,
  

• 				sbsa_mem_info.memnode[index].addr_base,
  

• 				sbsa_mem_info.memnode[index].addr_size);
  

• case SIP_SVC_GET_MEMORYNODE_COUNT:

• SMC_RET2(handle, NULL, sbsa_mem_info.num_memnode);
  

• default: ERROR("%s: unhandled SMC (0x%x) (function id: %d)\n", __func__, smc_fid, smc_fid - SIP_FUNCTION);