From owner-freebsd-mips@FreeBSD.ORG Thu Dec 2 18:11:59 2010 Return-Path: Delivered-To: freebsd-mips@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3BC7A1065693 for ; Thu, 2 Dec 2010 18:11:59 +0000 (UTC) (envelope-from alc@rice.edu) Received: from mh5.mail.rice.edu (mh5.mail.rice.edu [128.42.199.32]) by mx1.freebsd.org (Postfix) with ESMTP id 085DE8FC12 for ; Thu, 2 Dec 2010 18:11:58 +0000 (UTC) Received: from mh5.mail.rice.edu (localhost.localdomain [127.0.0.1]) by mh5.mail.rice.edu (Postfix) with ESMTP id 8783728F755; Thu, 2 Dec 2010 11:54:53 -0600 (CST) X-Virus-Scanned: by amavis-2.6.4 at mh5.mail.rice.edu, auth channel Received: from mh5.mail.rice.edu ([127.0.0.1]) by mh5.mail.rice.edu (mh5.mail.rice.edu [127.0.0.1]) (amavis, port 10026) with ESMTP id wCb9BIFNUXrc; Thu, 2 Dec 2010 11:54:53 -0600 (CST) Received: from adsl-216-63-78-18.dsl.hstntx.swbell.net (adsl-216-63-78-18.dsl.hstntx.swbell.net [216.63.78.18]) (using TLSv1 with cipher RC4-MD5 (128/128 bits)) (No client certificate requested) (Authenticated sender: alc) by mh5.mail.rice.edu (Postfix) with ESMTPSA id 0771128F767; Thu, 2 Dec 2010 11:54:52 -0600 (CST) Message-ID: <4CF7DD6C.6020303@rice.edu> Date: Thu, 02 Dec 2010 11:54:52 -0600 From: Alan Cox User-Agent: Thunderbird 2.0.0.24 (X11/20100725) MIME-Version: 1.0 To: Warner Losh References: <4CF5E518.20508@rice.edu> <4CF71F79.3020500@bsdimp.com> In-Reply-To: <4CF71F79.3020500@bsdimp.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Alan Cox , freebsd-mips@freebsd.org Subject: Re: vm_page_startup() X-BeenThere: freebsd-mips@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting FreeBSD to MIPS List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Dec 2010 18:11:59 -0000 Warner Losh wrote: > On 11/30/2010 23:03, Alan Cox wrote: >> Given this bit of code in the MIPS pmap: >> >> vm_offset_t >> pmap_map(vm_offset_t *virt, vm_offset_t start, vm_offset_t end, int >> prot) >> { >> vm_offset_t va, sva; >> >> if (MIPS_DIRECT_MAPPABLE(end)) >> return (MIPS_PHYS_TO_DIRECT(start)); > All mips architectures have at least a small direct map... vm_page_startup() places the data structures that it allocates and initializes at the upper end of the largest contiguous chunk of physical memory. So, machines with minimal physical memory will wind up using "direct mappable" memory and obviously 64-bit machines will do so as well. It is the machines "in the middle", i.e., with more than 512MB but not 64-bit, that will wind up not using the direct map here. Doesn't the above test have an off-by-error, specifically, shouldn't it be "MIPS_DIRECT_MAPPABLE(end - 1)"? > >> The following in vm_page_startup() can apply to MIPS (as well as amd64): >> >> #ifdef __amd64__ >> /* >> * pmap_map on amd64 comes out of the direct-map, not kvm like i386, >> * so the pages must be tracked for a crashdump to include this data. >> * This includes the vm_page_array and the early UMA bootstrap pages. >> */ >> for (pa = new_end; pa < phys_avail[biggestone + 1]; pa += PAGE_SIZE) >> dump_add_page(pa); >> #endif >> >> Even if a particular MIPS-based machine didn't wind up using the >> direct map for the pmap_map calls in vm_page_startup() there would be >> no harm in having called dump_add_page(). On the other hand, if you >> don't call dump_add_page() when the direct map is used, you're crash >> dump will be missing some kernel data structures. > So would you suggest having: > > #if defined(__mips__) || defined(__amd64__) > > or > > #if __VM_HAS_DIRECT_MAP > > and adding that define to pmap.h for mips and amd64? I'm leaning to > the latter, since I don't like #ifdef arch in vm/* code on general > principles, but you're the maintainer... > I can live with the former, since it's not really adding a new #ifdef, just updating an existing one. (For the upcoming 8.2-RELEASE, you might just want to do this.) As for the latter, the "right" solution is slightly more complicated. There exist architectures, e.g., ia64 and sparc64, that have direct maps but don't implement minidumps. So, there really needs to be two #define's, one for each of those features, and then the #if would be: #if defined(__VM_HAS_DIRECT_MAP) && defined ("implements minidumps") Alan