Date: Wed, 18 Aug 2010 17:41:50 -0700 From: Neel Natu <neelnatu@gmail.com> To: "Jayachandran C." <c.jayachandran@gmail.com> Cc: Alan Cox <alc@cs.rice.edu>, freebsd-mips@freebsd.org Subject: Re: [PATCH] Move from kseg0 to xkphys for 64 bit. Message-ID: <AANLkTi=DW_XKsJM4Thq14AbUY0njXvgxZ-SruEz%2BHQ4C@mail.gmail.com> In-Reply-To: <AANLkTinXWx7Xk8Wsq3GqTfCT5AEFGFZKkG2nGrY2i=UV@mail.gmail.com> References: <AANLkTinc2P7mO2qu%2BAiDtB=%2BoH3Winfc0AOAUxXS2XBh@mail.gmail.com> <AANLkTimdbxTJBMdRXgS9AGNKtLXmmBUuJmDGXGLGp46g@mail.gmail.com> <AANLkTinXWx7Xk8Wsq3GqTfCT5AEFGFZKkG2nGrY2i=UV@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi JC,
On Tue, Aug 17, 2010 at 10:22 PM, Jayachandran C.
<c.jayachandran@gmail.com> wrote:
> On Wed, Aug 18, 2010 at 7:35 AM, Neel Natu <neelnatu@gmail.com> wrote:
>> Hi JC,
>>
>> I have a few comments below.
>
> Thanks for the review!
>
>> Index: sys/mips/include/cpuregs.h
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>
>> +#define =A0 =A0 =A0 =A0MIPS_XKPHYS_LARGEST_PHYS =A0 =A0 =A0 =A00x100000=
00000
>> +#define =A0 =A0 =A0 =A0MIPS_XKPHYS_MASK =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
0x0ffffffffff
>>
>> Perhaps add a comment that this limit is less than that allowed by the
>> architecture because we can only address 40 bits with our 3 levels of
>> page tables.
>
> This is the physical address size - XLRs limit is 1TB of physical
> address (PABITS =3D 40), I guess the other processors will have a
> similar size limit. =A0I will see if I can get a better name that make
> it clear.
>
You are right. I must have spaced out - the number of levels in the
page table should only influence the number of usable bits in virtual
address space.
In any case, is there any reason not to make MIPS_XKPHYS_LARGEST_PHYS
the maximum allowed by the architecture? 1ULL << 59 if I remember
correctly.
>> Don't these macros need a 'ULL' type qualifier as well?
>
> This has to be ULL in 32 bit and UL in 64bit, I left it to the
> compiler for now (which is probably incorrect, I need to check if
> values will be promoted correctly in n32 if needed). =A0But probably
> using UINT64_C() from stdint.h is the better option? .
>
Wouldn't 'ULL' work equally well on o32 as well as n64? Not sure about
n32 though.
best
Neel
>> Index: sys/mips/mips/pmap.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>
>> Do you need to maintain 'valid1' in 'local_sysmap' anymore? It seems we
>> only check 'valid2' in pmap_lmem_unmap().
>>
>> =A0static __inline pd_entry_t *
>> =A0pmap_pdpe_to_pde(pd_entry_t *pdpe, vm_offset_t va)
>> =A0{
>> +
>> =A0 =A0 =A0 =A0return pdpe;
>> =A0}
>>
>> =A0static __inline
>> =A0pd_entry_t *pmap_pde(pmap_t pmap, vm_offset_t va)
>> =A0{
>> +
>> =A0 =A0 =A0 =A0return pmap_segmap(pmap, va);
>> =A0}
>>
>> Parentheses around the return value.
>
> I thought I had got all the style(9) fixes, looks like I missed a
> couple :( =A0 There are other places in the old code which needs this
> too, I think I will fix all the returns in pmap.c.
>
>> @@ -1032,7 +1080,11 @@
>> =A0pmap_grow_pte_page_cache()
>> =A0{
>>
>> +#ifdef __mips_n64
>> + =A0 =A0 =A0 vm_contig_grow_cache(3, 0, 0xffffffffffUL);
>> +#else
>> =A0 =A0 =A0 =A0vm_contig_grow_cache(3, 0, MIPS_KSEG0_LARGEST_PHYS);
>> +#endif
>> =A0}
>>
>> Why not use MIPS_XPHYS_LARGEST_PHYS in the __mips_n64 case instead of us=
ing
>> a literal value?
>
> Will make these changes.
>
>> best
>> Neel
>
>> On Mon, Aug 16, 2010 at 9:10 AM, Jayachandran C.
>> <c.jayachandran@gmail.com> wrote:
>>> I've attached the changes to move the 64bit port to use 64bit XKPHYS
>>> mapping of the physical memory instead of the current KSEG0. =A0With
>>> this changes the 64bit port will use just one freelist, and can
>>> allocate page table pages from anywhere in the memory.
>>>
>>> The changes are mainly to introduce macros like
>>> MIPS_PHYS_TO_DIRECT(pa), MIPS_DIRECT_TO_PHYS(), which will use KSEG0
>>> in 32 bit compilation and XKPHYS in 64 bit compilation. I also ended
>>> up changing the macro based PMAP_LMEM_MAP1(), PMAP_LMEM_MAP2(),
>>> PMAP_LMEM_UNMAP() to inline functions.
>>>
>>> I have also introduced a macro MIPS_DIRECT_MAPPABLE(pa), which will
>>> further reduce the cases in which we will need to have a special case
>>> for 64 bit compilation.
>>>
>>> Please let me know your comments.
>
> JC.
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTi=DW_XKsJM4Thq14AbUY0njXvgxZ-SruEz%2BHQ4C>