From owner-freebsd-amd64@FreeBSD.ORG Mon Sep 14 21:35:13 2009 Return-Path: Delivered-To: freebsd-amd64@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8A8BA1065697; Mon, 14 Sep 2009 21:35:13 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 4C6B08FC14; Mon, 14 Sep 2009 21:35:13 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id EE5D346B35; Mon, 14 Sep 2009 17:35:12 -0400 (EDT) Received: from jhbbsd.hudson-trading.com (unknown [209.249.190.8]) by bigwig.baldwin.cx (Postfix) with ESMTPA id EDF038A01B; Mon, 14 Sep 2009 17:35:11 -0400 (EDT) From: John Baldwin To: freebsd-amd64@freebsd.org, "James R. Van Artsdalen" Date: Mon, 14 Sep 2009 17:35:04 -0400 User-Agent: KMail/1.9.7 References: <200908262239.n7QMdeHV045328@bigtex.housenet.jrv> In-Reply-To: <200908262239.n7QMdeHV045328@bigtex.housenet.jrv> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200909141735.05018.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Mon, 14 Sep 2009 17:35:11 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00,RDNS_NONE autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: FreeBSD-gnats-submit@freebsd.org Subject: Re: amd64/138220: [patch] FreeBSD/amd64 can't see all system memory X-BeenThere: freebsd-amd64@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting FreeBSD to the AMD64 platform List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Sep 2009 21:35:13 -0000 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