Date: Mon, 14 Sep 2009 17:35:04 -0400 From: John Baldwin <jhb@freebsd.org> To: freebsd-amd64@freebsd.org, "James R. Van Artsdalen" <james-freebsd-current@jrv.org> Cc: FreeBSD-gnats-submit@freebsd.org Subject: Re: amd64/138220: [patch] FreeBSD/amd64 can't see all system memory Message-ID: <200909141735.05018.jhb@freebsd.org> In-Reply-To: <200908262239.n7QMdeHV045328@bigtex.housenet.jrv> References: <200908262239.n7QMdeHV045328@bigtex.housenet.jrv>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200909141735.05018.jhb>