Merging 64 bit changes to -HEAD - part 4

[Jayachandran C.]

On Sat, Jul 10, 2010 at 2:16 AM, M. Warner Losh <imp at bsdimp.com> wrote:
> In message: <AANLkTik8fBlxMyZ3AZETF_7DvlSpPsx2hozIWinRy0U0 at mail.gmail.com>
>            "Jayachandran C." <c.jayachandran at gmail.com> writes:
> : On Thu, Jul 8, 2010 at 2:52 PM, Jayachandran C.
> : <c.jayachandran at gmail.com> wrote:
> : > On Thu, Jul 8, 2010 at 1:42 PM, M. Warner Losh <imp at bsdimp.com> wrote:
> : >> In message: <AANLkTikSVi27V2UICgLvKd8Bk7v6tuGty9YX6-C6-21H at mail.gmail.com>
> : >>            "Jayachandran C." <c.jayachandran at 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?

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:
>  * 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?

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.  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.

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.