From owner-svn-src-all@FreeBSD.ORG Thu Oct 28 20:51:57 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 945FA1065670; Thu, 28 Oct 2010 20:51:57 +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 505D78FC20; Thu, 28 Oct 2010 20:51:57 +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 CAAF746B03; Thu, 28 Oct 2010 16:51:56 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 8CA5A8A01D; Thu, 28 Oct 2010 16:51:55 -0400 (EDT) From: John Baldwin To: Attilio Rao Date: Thu, 28 Oct 2010 16:38:13 -0400 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100819; KDE/4.4.5; amd64; ; ) References: <201010281631.o9SGVdtZ014923@svn.freebsd.org> <201010281411.40423.jhb@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201010281638.14043.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Thu, 28 Oct 2010 16:51:55 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.96.3 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-1.9 required=4.2 tests=BAYES_00 autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on bigwig.baldwin.cx Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r214457 - in head/sys: amd64/amd64 conf i386/i386 x86/x86 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Oct 2010 20:51:57 -0000 On Thursday, October 28, 2010 4:28:12 pm Attilio Rao wrote: > 2010/10/28 John Baldwin : > > On Thursday, October 28, 2010 1:21:34 pm Attilio Rao wrote: > >> 2010/10/28 John Baldwin : > >> > On Thursday, October 28, 2010 12:31:39 pm Attilio Rao wrote: > >> >> Author: attilio > >> >> Date: Thu Oct 28 16:31:39 2010 > >> >> New Revision: 214457 > >> >> URL: http://svn.freebsd.org/changeset/base/214457 > >> >> > >> >> Log: > >> >> Merge nexus.c from amd64 and i386 to x86 subtree. > >> >> > >> >> Sponsored by: Sandvine Incorporated > >> >> Tested by: gianni > >> >> > >> > > >> > It would be better to merge these two routines. The loader now passes the > >> > smap to i386 kernels as well, so ram_attach() should probably be changed to > >> > try the amd64 approach first and if that fails fall back to using the > >> > phys_avail[] array instead. > >> > >> What do you think about this patch?: > >> Index: nexus.c > >> =================================================================== > >> --- nexus.c (revision 214457) > >> +++ nexus.c (working copy) > >> @@ -52,9 +52,7 @@ > >> #include > >> #include > >> #include > >> -#ifdef __amd64__ > >> #include > >> -#endif > >> #include > >> #include > >> #include > >> @@ -67,12 +65,10 @@ > >> #include > >> #include > >> > >> -#ifdef __amd64__ > >> #include > >> -#include > >> -#endif > >> #include > >> #include > >> +#include > >> > >> #ifdef DEV_APIC > >> #include "pcib_if.h" > >> @@ -89,11 +85,13 @@ > >> #include > >> > >> #ifdef __amd64__ > >> -#define RMAN_BUS_SPACE_IO AMD64_BUS_SPACE_IO > >> -#define RMAN_BUS_SPACE_MEM AMD64_BUS_SPACE_MEM > >> +#define X86_BUS_SPACE_IO AMD64_BUS_SPACE_IO > >> +#define X86_BUS_SPACE_MEM AMD64_BUS_SPACE_MEM > >> +#define ELF_KERN_STR "elf64 kernel" > >> #else > >> -#define RMAN_BUS_SPACE_IO I386_BUS_SPACE_IO > >> -#define RMAN_BUS_SPACE_MEM I386_BUS_SPACE_MEM > >> +#define X86_BUS_SPACE_IO I386_BUS_SPACE_IO > >> +#define X86_BUS_SPACE_MEM I386_BUS_SPACE_MEM > >> +#define ELF_KERN_STR "elf32 kernel" > >> #endif > >> @@ -701,16 +699,11 @@ > >> panic("ram_attach: resource %d failed to attach", rid); > >> rid++; > >> } > >> - return (0); > >> -} > >> -#else > >> -static int > >> -ram_attach(device_t dev) > >> -{ > >> - struct resource *res; > >> - vm_paddr_t *p; > >> - int error, i, rid; > >> > >> + /* If at least one smap attached, return. */ > >> + if (rid != 0) > >> + return (0); > >> + > > > > Perhaps this instead: > > > > /* If we found an SMAP, return. */ > > if (smapbase != NULL) > > return (0); > > No, I don't think this check is right, smapbase will always be != NULL > (otherwise the code panics). Oh, that needs to be fixed then. It can be NULL on i386 with an old loader (or on a really old machine without an SMAP). The amd64 nexus code could assume it would never be NULL, but i386 cannot. It should probably more closely match what i386 does during the memory probe which is: kmdp = search("elf kernel"); if (kmdp == NULL) kmdp = search("elfXX kernel"); if (kmdp != NULL) smapbase = preload_search(...); else smapbase = NULL; if (smapbase != NULL) { for (smap = ...) { } return (0); } /* fall through to old i386 code using phys_avail[] */ -- John Baldwin