From owner-freebsd-mips@FreeBSD.ORG Sat Jul 10 03:51:01 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 53291106564A; Sat, 10 Jul 2010 03:51:01 +0000 (UTC) (envelope-from c.jayachandran@gmail.com) Received: from mail-yx0-f182.google.com (mail-yx0-f182.google.com [209.85.213.182]) by mx1.freebsd.org (Postfix) with ESMTP id D8A968FC18; Sat, 10 Jul 2010 03:50:59 +0000 (UTC) Received: by yxn22 with SMTP id 22so630591yxn.13 for ; Fri, 09 Jul 2010 20:50:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=MWSpztiskCbPEp/HUTr5V/SKuq7SOgxi8u2PZjLS0fM=; b=lGMOltBzdiQEKBy9+1qvY0qMf7onXNWC40DYGAtu3hXFrXk5tpkZWlbLJoMgn8897s 2JnZa8ZyF9Q9AjOXJcDocLdhunOi6u5kox6vqf7gsN6T/ujXZR7jl06UYoWROj26CTxL 87s38F07WNWWdr/SJrPNLC6LVePJm4amnKb80= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=N7/mKsSmvA1rAN7jW+Mr++iPJaxFdBVEde9RVuxqmvjytAHxpdQR+UtS0vKbPXzt6b LdJE8GRpImyEgfTsGXyQeGjBLOUByXqwTCxPdycxeDl0IHjv5Zd+lrQzwgBJ6bQ6gAr4 cOzAVoBIL9xGKyRpXZ86Nv1O5FZvbkayrCrdU= MIME-Version: 1.0 Received: by 10.220.62.5 with SMTP id v5mr5557916vch.221.1278733858266; Fri, 09 Jul 2010 20:50:58 -0700 (PDT) Received: by 10.220.176.9 with HTTP; Fri, 9 Jul 2010 20:50:58 -0700 (PDT) In-Reply-To: <20100709.144631.1141490504242621103.imp@bsdimp.com> References: <20100708.021250.1099368555950605809.imp@bsdimp.com> <20100709.144631.1141490504242621103.imp@bsdimp.com> Date: Sat, 10 Jul 2010 09:20:58 +0530 Message-ID: From: "Jayachandran C." To: "M. Warner Losh" Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-mips@freebsd.org Subject: Re: Merging 64 bit changes to -HEAD - part 4 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: Sat, 10 Jul 2010 03:51:01 -0000 On Sat, Jul 10, 2010 at 2:16 AM, M. Warner Losh wrote: > In message: > =A0 =A0 =A0 =A0 =A0 =A0"Jayachandran C." write= s: > : On Thu, Jul 8, 2010 at 2:52 PM, Jayachandran C. > : wrote: > : > On Thu, Jul 8, 2010 at 1:42 PM, M. Warner Losh wrote= : > : >> In message: > : >> =A0 =A0 =A0 =A0 =A0 =A0"Jayachandran C." = writes: > : [...] > : >> : pmap-n64.patch > : >> : =A0The main n64 patch (from Juli's branch) > : >> : =A0 This still uses the old 2-level page tables. But this adds oth= er > : >> : pmap code to support n64. I have re-arranged some of =A0Juli's cod= e to > : >> : reduce #ifdefs. > : >> > : >> I think this could be done in smaller bites. =A0At least the > : >> MIPS_{PYHS,KSEGx}_TO_{KSEGx,PHYS} stuff can be done as a separate > : >> patch. > : [...] > : > Thanks for the review, I'll try to get the patches other than pmap-n6= 4 > : > in first (with mentor approval). I'll post a new split version of > : > pmap-n64 after that. > : > : Here are the pmap changes again, along with a ddb patch. > : > : kseg-def-to-cpu.h.patch > : Move KSEG address definitions from cpu.h to cpuregs.h with the other > : definitions, add some =A0XKPHYS related definitions. > > This looks good. > > : mips-pmap-intial-n64.patch (from Juli's branch) > : Initial changes to pmap.c for n64 support. When compiled for n64, pmap > : can use XKPHYS for a lot of its operations. The page tables are still > : 2-level. > > +#if !defined(__mips_n64) > + =A0 =A0 =A0 if (phys_avail[i - 1] >=3D MIPS_KSEG0_LARGEST_PHYS) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 memory_larger_than_512meg =3D 1; > +#endif > > We never set memory_larger_than_512meg then for __mips_n64. =A0Do we > need to ifdef both the declaration of this variable, as well as the > if (memory_larger_than_512mb) clauses later in the function? I thought I will prevent more ifdefs. Since the variable is zero, the compiler should optimize it out for n64 - but I have not actually looked at the generated code yet. I can add a comment to the variable if needed. > pmap_map should likely have a comment to the effect: > =A0* For memory that's directly mappable, we return the direct map > =A0* address. =A0For other addresses, we create a map. > > A similar comment is needed for pmap_kenter_temporary, pmap_zero_page, > pmap_zero_page_area, pmap_zero_page_idle, pmap_copy_page, pmap_mapdev, > pmap_unmapdev, pmap_kextract. =A0Hmmm, or maybe a general comment? Will look at this. > : mips-tlb64.patch (from Juli's branch) > : 64 bit TLB definitions. > > This looks good. > > : mips-ddb-64.patch (from Juli's branch) > : Minor fixups for ddb support. > > Can you explain the casting for all the calls to kbdpeek*? I had looked at cleaning it up (this is the reason I did not sent this patch earlier). But after looking at it, looked like we needed to revisit it, and just took the changes from Juli's tree. I can move all the casts to one line where we can define a temp variable of the needed type. > : This should get n64 up to mountroot - let me know you comments. > > Cool! > > thanks for your work. =A0Also, I've been providing reviews on many of > these commits, but I haven't seen a 'Reviewed by: imp' in the commit > messages. =A0I'd appreciate it if you could add then when I provide a > review. I had added reviewed by for 2 changesets for which you said OK. In the other changesets I thought your comments were more neutral so I was not sure if I could add the reviewed by. Will do this in further changes. Thanks JC.