Index | Thread | Search

From:
Jeremie Courreges-Anglas <jca@wxcvbn.org>
Subject:
efiboot "machine memory" support for riscv64
To:
tech@openbsd.org
Date:
Sun, 4 Aug 2024 22:23:00 +0200

Download raw body.

Thread
The code below works fine on my home machine as I am able to print the
memory map and mask out chunks of memory.  mach mem =XXXM isn't very
useful because only 22MB of memory are available in the lowest
addresses on that machine.

The implementation is cheap:
- it doesn't manipulate the efi memory map from efiboot, instead it
  passes hints to the kernel which already has the memreg_remove()
  code.
- it doesn't add support for '+' ie forcefully adding memory to the
  memory map.  That looks like a hack for a problem I don't have,
  something that *should* be fixed by providing a proper dtb.

Does this look worth having?  ok?


PS: implementing a barebone memtest directly in efiboot seems the
least ugly way to provide the user with the physicall addresses of
possibly bad RAM cells (on amd64 I would use memtest86+).  Before I
start hacking on this, does this sound reasonable, or maybe someone
has a better idea?



Index: stand/efiboot/efiboot.c
===================================================================
RCS file: /cvs/src/sys/arch/riscv64/stand/efiboot/efiboot.c,v diff -u
-p -r1.11 efiboot.c --- stand/efiboot/efiboot.c 23 Jun 2024 13:11:51
-0000 1.11 +++ stand/efiboot/efiboot.c 4 Aug 2024 19:46:13 -0000 @@
-495,6 +495,45 @@ efi_dma_constraint(void) dma_constraint,
sizeof(dma_constraint)); }
 
+struct fdt_reg	reserved_memory[64];
+unsigned int	nreserved;
+
+static void
+fdt_add_reserved_memory(long long addr, long long size)
+{
+	if (nreserved == nitems(reserved_memory)) {
+		printf("too many reserved mappings\n");
+		return;
+	}
+
+	reserved_memory[nreserved].addr = addr;
+	reserved_memory[nreserved].size = size;
+	nreserved++;
+}
+
+static void
+fdt_reset_reserved_memory(void)
+{
+	memset(reserved_memory, 0, sizeof(reserved_memory));
+	nreserved = 0;
+}
+
+static void
+fdt_print_reserved_memory(void)
+{
+	int		 i;
+
+	if (nreserved == 0)
+		return;
+
+	printf("Reserved memory ranges:\n");
+	for (i = 0; i < nreserved; i++) {
+		printf("          pa 0x%llx size 0x%llx\n",
+		    reserved_memory[i].addr,
+		    reserved_memory[i].size);
+	}
+}
+
 char *bootmac = NULL;
 
 void *
@@ -536,6 +575,29 @@ efi_makebootargs(char *bootargs, int how
 	if (node == NULL)
 		fdt_node_add_node(fdt_find_node("/"), "chosen", &node);
 
+	if (nreserved > 0) {
+		void		*child;
+		unsigned int	 i;
+
+		node = fdt_find_node("/reserved-memory");
+		if (node == NULL) {
+			fdt_node_add_node(fdt_find_node("/"), "reserved-memory",
+			    &node);
+		}
+
+		for (i = 0; i < nreserved; i++) {
+			char childname[sizeof("openbsd,reserved-memoryXXXX")];
+			struct fdt_reg reg;
+
+			snprintf(childname, sizeof(childname),
+			    "openbsd,reserved-memory%u", i);
+			fdt_node_add_node(node, childname, &child);
+			reg.addr = htobe64(reserved_memory[i].addr);
+			reg.size = htobe64(reserved_memory[i].size);
+			fdt_node_add_property(child, "reg", &reg, sizeof(reg));
+		}
+	}
+
 	node = fdt_find_node("/chosen");
 	hartid = efi_get_boot_hart_id();
 	if (hartid >= 0) {
@@ -973,6 +1035,23 @@ efi_memprobe_find(UINTN pages, UINTN ali
 	return EFI_OUT_OF_RESOURCES;
 }
 
+static void
+efi_dumpmem(void)
+{
+	EFI_MEMORY_DESCRIPTOR	*mm;
+	int			 i;
+
+	efi_memprobe_internal();	/* sync the current map */
+
+	for (i = 0, mm = mmap; i < mmap_ndesc;
+	    i++, mm = NextMemoryDescriptor(mm, mmap_descsiz)) {
+		printf("type 0x%x pa 0x%llx va 0x%llx pages 0x%llx attr 0x%llx\n",
+		    mm->Type, mm->PhysicalStart,
+		    mm->VirtualStart, mm->NumberOfPages,
+		    mm->Attribute);
+	}
+}
+
 void *
 efi_fdt(void)
 {
@@ -1060,11 +1139,13 @@ retry:
 
 int Xdtb_efi(void);
 int Xexit_efi(void);
+int Xmemory_efi(void);
 int Xpoweroff_efi(void);
 
 const struct cmd_table cmd_machine[] = {
 	{ "dtb",	CMDT_CMD, Xdtb_efi },
 	{ "exit",	CMDT_CMD, Xexit_efi },
+	{ "memory",	CMDT_CMD, Xmemory_efi },
 	{ "poweroff",	CMDT_CMD, Xpoweroff_efi },
 	{ NULL, 0 }
 };
@@ -1092,6 +1173,78 @@ Xexit_efi(void)
 	for (;;)
 		continue;
 	return (0);
+}
+
+int
+Xmemory_efi(void)
+{
+	if (cmd.argc >= 2) {
+		int i;
+		/* parse the memory specs */
+
+		for (i = 1; i < cmd.argc; i++) {
+			char *p;
+			long long addr, size;
+
+			p = cmd.argv[i];
+
+			if (strcmp(p, "reset") == 0) {
+				fdt_reset_reserved_memory();
+				continue;
+			}
+
+			size = strtoll(p + 1, &p, 0);
+			/* Size the size */
+			switch (*p) {
+				case 'G':
+				case 'g':
+					size *= 1024;
+				case 'M':
+				case 'm':
+					size *= 1024;
+				case 'K':
+				case 'k':
+					size *= 1024;
+					p++;
+			}
+
+			/* Handle (possibly non-existent) address part */
+			switch (*p) {
+				case '@':
+					addr = strtoll(p + 1, NULL, 0);
+					break;
+
+				/* Adjust address if we don't need it */
+				default:
+					if (cmd.argv[i][0] == '=')
+						addr = -1;
+					else
+						addr = 0;
+			}
+
+			if (addr == 0 || size == 0) {
+				printf("bad language\n");
+				return 0;
+			} else {
+				switch (cmd.argv[i][0]) {
+				case '-':
+					fdt_add_reserved_memory(addr, size);
+					break;
+				case '=':
+					fdt_add_reserved_memory(size, -1L);
+					break;
+				default :
+					printf("bad OP\n");
+					return 0;
+				}
+			}
+		}
+	}
+
+	efi_dumpmem();
+	fdt_print_reserved_memory();
+
+	return 0;
 }
 
 int
Index: stand/efiboot/fdt.h
===================================================================
RCS file: /cvs/src/sys/arch/riscv64/stand/efiboot/fdt.h,v
diff -u -p -r1.1 fdt.h
--- stand/efiboot/fdt.h	28 Apr 2021 19:01:00 -0000	1.1
+++ stand/efiboot/fdt.h	4 Aug 2024 19:46:13 -0000
@@ -40,6 +40,11 @@ struct fdt {
 	int		struct_size;
 };
 
+struct fdt_reg {
+	uint64_t	addr;
+	uint64_t	size;
+};
+
 #define FDT_MAGIC	0xd00dfeed
 #define FDT_NODE_BEGIN	0x01
 #define FDT_NODE_END	0x02

-- 
jca