Index | Thread | Search

From:
Mike Larkin <mlarkin@nested.page>
Subject:
Re: fix machine memory on efiboot
To:
YASUOKA Masahiko <yasuoka@yasuoka.net>
Cc:
stu@spacehopper.org, iv209@yahoo.com, tech@openbsd.org
Date:
Sun, 14 Sep 2025 10:28:18 -0700

Download raw body.

Thread
On Mon, Sep 08, 2025 at 12:41:06PM +0900, YASUOKA Masahiko wrote:
> On Mon, 08 Sep 2025 12:28:28 +0900 (JST)
> YASUOKA Masahiko <yasuoka@openbsd.org> wrote:
> > Hi,
> >
> > # moved from bug@ to tech@
> >
> > On Sun, 7 Sep 2025 11:57:52 -0700
> > Mike Larkin <mlarkin@nested.page> wrote:
> >> On Fri, Sep 05, 2025 at 09:18:49AM +0100, Stuart Henderson wrote:
> >>> "machine mem" takes an optional argument to either restrict max memory, or
> >>> add/remove certain ranges - https://man.openbsd.org/amd64/boot#memory - you
> >>> could try excluding some of the memory ranges from the list you showed.
> >>>
> >>
> >> N.B. - machine mem to restrict memory doesn't work on EFI.
> >
> > I didn't aware of this problem.
> >
> > ok?
> >
> > Don't reset bios_memmap when exiting if it's modified by the operator.
>
> Let me tweak the diff.

Hi Yasuoka-san,

Please increase the version number in conf.c to reflect the change, and
if that is done, diff is ok mlarkin, thank you!

-ml

>
> > Index: sys/arch/amd64/stand/efiboot/efiboot.c
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
> > diff -u -p -r1.43 efiboot.c
> > --- sys/arch/amd64/stand/efiboot/efiboot.c	27 Aug 2025 09:08:12 -0000	1.43
> > +++ sys/arch/amd64/stand/efiboot/efiboot.c	8 Sep 2025 03:23:45 -0000
> (snip)
> > @@ -350,14 +346,37 @@ efi_memprobe_internal(void)
> >  	status = BS->GetMemoryMap(&siz, NULL, &mapkey, &mmsiz, &mmver);
> >  	if (status != EFI_BUFFER_TOO_SMALL)
> >  		panic("cannot get the size of memory map");
> > -	mm0 = alloc(siz);
> > -	status = BS->GetMemoryMap(&siz, mm0, &mapkey, &mmsiz, &mmver);
> > +	mm = alloc(siz);
> > +	status = BS->GetMemoryMap(&siz, mm, &mapkey, &mmsiz, &mmver);
> >  	if (status != EFI_SUCCESS)
> >  		panic("cannot get the memory map");
> > -	n = siz / mmsiz;
> > +
> >  	mmap_key = mapkey;
> >
> > -	for (i = 0, mm = mm0; i < n; i++, mm = NextMemoryDescriptor(mm, mmsiz)){
> > +	bios_efiinfo.mmap_desc_ver = mmver;
> > +	bios_efiinfo.mmap_desc_size = mmsiz;
> > +	bios_efiinfo.mmap_size = siz;
> > +	bios_efiinfo.mmap_start = (uintptr_t)mm;
> > +
> > +	if (bios_memmap_modified)
> > +		return (0);
> > +
> > +	cnvmem = extmem = 0;
> > +	bios_memmap[0].type = BIOS_MAP_END;
> > +	return (efi_update_bios_memmap());
> > +}
> > +
>
> I'd like to move the 2 lines before calling efi_update_bios_memmap()
> into  efi_update_bios_memmap() is better.
>
> Index: sys/arch/amd64/stand/efiboot/cmd_i386.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/cmd_i386.c,v
> diff -u -p -r1.3 cmd_i386.c
> --- sys/arch/amd64/stand/efiboot/cmd_i386.c	27 Aug 2025 09:08:12 -0000	1.3
> +++ sys/arch/amd64/stand/efiboot/cmd_i386.c	8 Sep 2025 03:37:01 -0000
> @@ -95,6 +95,7 @@ Xmemory(void)
>  		for (i = 1; i < cmd.argc; i++) {
>  			char *p;
>  			long long addr, size;
> +			extern int bios_memmap_modified;
>
>  			p = cmd.argv[i];
>
> @@ -145,6 +146,7 @@ Xmemory(void)
>  					printf("bad OP\n");
>  					return 0;
>  				}
> +				bios_memmap_modified = 1;
>  			}
>  		}
>  	}
> Index: sys/arch/amd64/stand/efiboot/efiboot.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
> diff -u -p -r1.43 efiboot.c
> --- sys/arch/amd64/stand/efiboot/efiboot.c	27 Aug 2025 09:08:12 -0000	1.43
> +++ sys/arch/amd64/stand/efiboot/efiboot.c	8 Sep 2025 03:37:01 -0000
> @@ -60,6 +60,7 @@ int	 efi_device_path_depth(EFI_DEVICE_PA
>  int	 efi_device_path_ncmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *, int);
>  static void	 efi_heap_init(void);
>  static int	 efi_memprobe_internal(void);
> +static int	 efi_update_bios_memmap(void);
>  static void	 efi_video_init(void);
>  static void	 efi_video_reset(void);
>  static EFI_STATUS
> @@ -283,6 +284,7 @@ efi_device_path_ncmp(EFI_DEVICE_PATH *dp
>   ***********************************************************************/
>  bios_memmap_t		 bios_memmap[128];
>  bios_efiinfo_t		 bios_efiinfo;
> +int			 bios_memmap_modified = 0;
>
>  static void
>  efi_heap_init(void)
> @@ -335,13 +337,7 @@ efi_memprobe_internal(void)
>  	EFI_STATUS		 status;
>  	UINTN			 mapkey, mmsiz, siz;
>  	UINT32			 mmver;
> -	EFI_MEMORY_DESCRIPTOR	*mm0, *mm;
> -	int			 i, n;
> -	bios_memmap_t		*bm, bm0;
> -	int			 error = 0;
> -
> -	cnvmem = extmem = 0;
> -	bios_memmap[0].type = BIOS_MAP_END;
> +	EFI_MEMORY_DESCRIPTOR	*mm;
>
>  	if (bios_efiinfo.mmap_start != 0)
>  		free((void *)bios_efiinfo.mmap_start, bios_efiinfo.mmap_size);
> @@ -350,14 +346,37 @@ efi_memprobe_internal(void)
>  	status = BS->GetMemoryMap(&siz, NULL, &mapkey, &mmsiz, &mmver);
>  	if (status != EFI_BUFFER_TOO_SMALL)
>  		panic("cannot get the size of memory map");
> -	mm0 = alloc(siz);
> -	status = BS->GetMemoryMap(&siz, mm0, &mapkey, &mmsiz, &mmver);
> +	mm = alloc(siz);
> +	status = BS->GetMemoryMap(&siz, mm, &mapkey, &mmsiz, &mmver);
>  	if (status != EFI_SUCCESS)
>  		panic("cannot get the memory map");
> -	n = siz / mmsiz;
> +
>  	mmap_key = mapkey;
>
> -	for (i = 0, mm = mm0; i < n; i++, mm = NextMemoryDescriptor(mm, mmsiz)){
> +	bios_efiinfo.mmap_desc_ver = mmver;
> +	bios_efiinfo.mmap_desc_size = mmsiz;
> +	bios_efiinfo.mmap_size = siz;
> +	bios_efiinfo.mmap_start = (uintptr_t)mm;
> +
> +	if (bios_memmap_modified)
> +		return (0);
> +
> +	return (efi_update_bios_memmap());
> +}
> +
> +static int
> +efi_update_bios_memmap(void)
> +{
> +	int			 i, n, error = 0;
> +	bios_memmap_t		*bm, bm0;
> +	EFI_MEMORY_DESCRIPTOR	*mm;
> +
> +	cnvmem = extmem = 0;
> +	bios_memmap[0].type = BIOS_MAP_END;
> +	n = bios_efiinfo.mmap_size / bios_efiinfo.mmap_desc_size;
> +	for (i = 0, mm = (EFI_MEMORY_DESCRIPTOR *)bios_efiinfo.mmap_start;
> +	    i < n;
> +	    i++, mm = NextMemoryDescriptor(mm, bios_efiinfo.mmap_desc_size)) {
>  		bm0.type = BIOS_MAP_END;
>  		bm0.addr = mm->PhysicalStart;
>  		bm0.size = mm->NumberOfPages * EFI_PAGE_SIZE;
> @@ -415,12 +434,7 @@ efi_memprobe_internal(void)
>  			extmem += bm->size / 1024;
>  	}
>
> -	bios_efiinfo.mmap_desc_ver = mmver;
> -	bios_efiinfo.mmap_desc_size = mmsiz;
> -	bios_efiinfo.mmap_size = siz;
> -	bios_efiinfo.mmap_start = (uintptr_t)mm0;
> -
> -	return error;
> +	return (error);
>  }
>
>  /***********************************************************************
>