Date: Fri, 09 Jul 2010 14:46:31 -0600 (MDT) From: "M. Warner Losh" <imp@bsdimp.com> To: c.jayachandran@gmail.com Cc: jmallett@FreeBSD.org, freebsd-mips@FreeBSD.org Subject: Re: Merging 64 bit changes to -HEAD - part 4 Message-ID: <20100709.144631.1141490504242621103.imp@bsdimp.com> In-Reply-To: <AANLkTik8fBlxMyZ3AZETF_7DvlSpPsx2hozIWinRy0U0@mail.gmail.com> References: <20100708.021250.1099368555950605809.imp@bsdimp.com> <AANLkTilFZ4zAKqGcDKie6og5Nhrh9PU_Ta3uDoq7yexA@mail.gmail.com> <AANLkTik8fBlxMyZ3AZETF_7DvlSpPsx2hozIWinRy0U0@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <AANLkTik8fBlxMyZ3AZETF_7DvlSpPsx2hozIWinRy0U0@mail.gmail.com>
"Jayachandran C." <c.jayachandran@gmail.com> writes:
: On Thu, Jul 8, 2010 at 2:52 PM, Jayachandran C.
: <c.jayachandran@gmail.com> wrote:
: > On Thu, Jul 8, 2010 at 1:42 PM, M. Warner Losh <imp@bsdimp.com> wrote:
: >> In message: <AANLkTikSVi27V2UICgLvKd8Bk7v6tuGty9YX6-C6-21H@mail.gmail.com>
: >> "Jayachandran C." <c.jayachandran@gmail.com> writes:
: [...]
: >> : pmap-n64.patch
: >> : The main n64 patch (from Juli's branch)
: >> : This still uses the old 2-level page tables. But this adds other
: >> : pmap code to support n64. I have re-arranged some of Juli's code to
: >> : reduce #ifdefs.
: >>
: >> I think this could be done in smaller bites. At 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-n64
: > 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 XKPHYS 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)
+ if (phys_avail[i - 1] >= MIPS_KSEG0_LARGEST_PHYS)
+ memory_larger_than_512meg = 1;
+#endif
We never set memory_larger_than_512meg then for __mips_n64. Do we
need to ifdef both the declaration of this variable, as well as the
if (memory_larger_than_512mb) clauses later in the function?
pmap_map should likely have a comment to the effect:
* For memory that's directly mappable, we return the direct map
* address. For 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. Hmmm, or maybe a general comment?
: 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*?
: This should get n64 up to mountroot - let me know you comments.
Cool!
thanks for your work. Also, I've been providing reviews on many of
these commits, but I haven't seen a 'Reviewed by: imp' in the commit
messages. I'd appreciate it if you could add then when I provide a
review.
Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100709.144631.1141490504242621103.imp>