amd64/138220: [patch] FreeBSD/amd64 can't see all system memory

John Baldwin jhb at freebsd.org
Mon Sep 14 21:40:04 UTC 2009


The following reply was made to PR amd64/138220; it has been noted by GNATS.

From: John Baldwin <jhb at freebsd.org>
To: freebsd-amd64 at freebsd.org,
 "James R. Van Artsdalen" <james-freebsd-current at jrv.org>
Cc: FreeBSD-gnats-submit at freebsd.org
Subject: Re: amd64/138220: [patch] FreeBSD/amd64 can't see all system memory
Date: Mon, 14 Sep 2009 17:35:04 -0400

 Can you try this alternate patch instead?  I am leery of manipulating the SMAP
 directly since it resides in the kernel's module metadata which in theory
 should be read-only.  This patch changes the SMAP code to do an insertion sort
 into the physmap[] array for each new entry that is added.  I also changed the
 amd64 code to use a separate static routine to add each entry similar to i386
 which fixes the bug with overlapping regions that you noted while keeping the
 SMAP code identical between the two archs.
 
 --- //depot/vendor/freebsd/src/sys/amd64/amd64/machdep.c	2009/08/20 23:00:20
 +++ //depot/user/jhb/numa/sys/amd64/amd64/machdep.c	2009/09/14 19:37:10
 @@ -1192,6 +1192,77 @@
  
  u_int basemem;
  
 +static int
 +add_smap_entry(struct bios_smap *smap, vm_paddr_t *physmap, int *physmap_idxp)
 +{
 +	int i, insert_idx, physmap_idx;
 +
 +	physmap_idx = *physmap_idxp;
 +
 +	if (boothowto & RB_VERBOSE)
 +		printf("SMAP type=%02x base=%016lx len=%016lx\n",
 +		    smap->type, smap->base, smap->length);
 +
 +	if (smap->type != SMAP_TYPE_MEMORY)
 +		return (1);
 +
 +	if (smap->length == 0)
 +		return (0);
 +
 +	/*
 +	 * Find insertion point while checking for overlap.  Start off by
 +	 * assuming the new entry will be added to the end.
 +	 */
 +	insert_idx = physmap_idx + 2;
 +	for (i = 0; i <= physmap_idx; i += 2) {
 +		if (smap->base < physmap[i + 1]) {
 +			if (smap->base + smap->length <= physmap[i]) {
 +				insert_idx = i;
 +				break;
 +			}
 +			if (boothowto & RB_VERBOSE)
 +				printf(
 +	"Overlapping or non-monotonic memory region, ignoring second region\n");
 +			return (1);
 +		}
 +	}
 +
 +	/* See if we can prepend to the next entry. */
 +	if (insert_idx <= physmap_idx &&
 +	    smap->base + smap->length == physmap[insert_idx]) {
 +		physmap[insert_idx] = smap->base;
 +		return (1);
 +	}
 +
 +	/* See if we can append to the previous entry. */
 +	if (insert_idx > 0 && smap->base == physmap[insert_idx - 1]) {
 +		physmap[insert_idx - 1] += smap->length;
 +		return (1);
 +	}
 +
 +	physmap_idx += 2;
 +	*physmap_idxp = physmap_idx;
 +	if (physmap_idx == PHYSMAP_SIZE) {
 +		printf(
 +		"Too many segments in the physical address map, giving up\n");
 +		return (0);
 +	}
 +
 +	/*
 +	 * Move the last 'N' entries down to make room for the new
 +	 * entry if needed.
 +	 */
 +	for (i = physmap_idx - 2; i > insert_idx; i -= 2) {
 +		physmap[i] = physmap[i - 2];
 +		physmap[i + 1] = physmap[i - 1];
 +	}
 +
 +	/* Insert the new entry. */
 +	physmap[insert_idx] = smap->base;
 +	physmap[insert_idx + 1] = smap->base + smap->length;
 +	return (1);
 +}
 +
  /*
   * Populate the (physmap) array with base/bound pairs describing the
   * available physical memory in the system, then test this memory and
 @@ -1235,40 +1306,9 @@
  	smapsize = *((u_int32_t *)smapbase - 1);
  	smapend = (struct bios_smap *)((uintptr_t)smapbase + smapsize);
  
 -	for (smap = smapbase; smap < smapend; smap++) {
 -		if (boothowto & RB_VERBOSE)
 -			printf("SMAP type=%02x base=%016lx len=%016lx\n",
 -			    smap->type, smap->base, smap->length);
 -
 -		if (smap->type != SMAP_TYPE_MEMORY)
 -			continue;
 -
 -		if (smap->length == 0)
 -			continue;
 -
 -		for (i = 0; i <= physmap_idx; i += 2) {
 -			if (smap->base < physmap[i + 1]) {
 -				if (boothowto & RB_VERBOSE)
 -					printf(
 -	"Overlapping or non-monotonic memory region, ignoring second region\n");
 -				continue;
 -			}
 -		}
 -
 -		if (smap->base == physmap[physmap_idx + 1]) {
 -			physmap[physmap_idx + 1] += smap->length;
 -			continue;
 -		}
 -
 -		physmap_idx += 2;
 -		if (physmap_idx == PHYSMAP_SIZE) {
 -			printf(
 -		"Too many segments in the physical address map, giving up\n");
 +	for (smap = smapbase; smap < smapend; smap++)
 +		if (!add_smap_entry(smap, physmap, &physmap_idx))
  			break;
 -		}
 -		physmap[physmap_idx] = smap->base;
 -		physmap[physmap_idx + 1] = smap->base + smap->length;
 -	}
  
  	/*
  	 * Find the 'base memory' segment for SMP
 --- //depot/vendor/freebsd/src/sys/i386/i386/machdep.c	2009/09/04 14:55:14
 +++ //depot/user/jhb/numa/sys/i386/i386/machdep.c	2009/09/14 19:37:10
 @@ -1946,7 +1946,7 @@
  static int
  add_smap_entry(struct bios_smap *smap, vm_paddr_t *physmap, int *physmap_idxp)
  {
 -	int i, physmap_idx;
 +	int i, insert_idx, physmap_idx;
  
  	physmap_idx = *physmap_idxp;
  	
 @@ -1968,8 +1968,17 @@
  	}
  #endif
  
 +	/*
 +	 * Find insertion point while checking for overlap.  Start off by
 +	 * assuming the new entry will be added to the end.
 +	 */
 +	insert_idx = physmap_idx + 2;
  	for (i = 0; i <= physmap_idx; i += 2) {
  		if (smap->base < physmap[i + 1]) {
 +			if (smap->base + smap->length <= physmap[i]) {
 +				insert_idx = i;
 +				break;
 +			}
  			if (boothowto & RB_VERBOSE)
  				printf(
  	"Overlapping or non-monotonic memory region, ignoring second region\n");
 @@ -1977,8 +1986,16 @@
  		}
  	}
  
 -	if (smap->base == physmap[physmap_idx + 1]) {
 -		physmap[physmap_idx + 1] += smap->length;
 +	/* See if we can prepend to the next entry. */
 +	if (insert_idx <= physmap_idx &&
 +	    smap->base + smap->length == physmap[insert_idx]) {
 +		physmap[insert_idx] = smap->base;
 +		return (1);
 +	}
 +
 +	/* See if we can append to the previous entry. */
 +	if (insert_idx > 0 && smap->base == physmap[insert_idx - 1]) {
 +		physmap[insert_idx - 1] += smap->length;
  		return (1);
  	}
  
 @@ -1989,8 +2006,19 @@
  		"Too many segments in the physical address map, giving up\n");
  		return (0);
  	}
 -	physmap[physmap_idx] = smap->base;
 -	physmap[physmap_idx + 1] = smap->base + smap->length;
 +
 +	/*
 +	 * Move the last 'N' entries down to make room for the new
 +	 * entry if needed.
 +	 */
 +	for (i = physmap_idx - 2; i > insert_idx; i -= 2) {
 +		physmap[i] = physmap[i - 2];
 +		physmap[i + 1] = physmap[i - 1];
 +	}
 +
 +	/* Insert the new entry. */
 +	physmap[insert_idx] = smap->base;
 +	physmap[insert_idx + 1] = smap->base + smap->length;
  	return (1);
  }
  
 
 
 -- 
 John Baldwin


More information about the freebsd-amd64 mailing list