Re: [PAE support] patch review

[Jean-Yves Migeon <jeanyves.migeon%free.fr@localhost>]

On 17.07.2010 15:52, Jean-Yves Migeon wrote:
> Hi lists,
> 
> You will find attached the patch for PAE support [1]; diff is against
> latest current. This one is for a last review before commit.
>[snip]
>
> ===== Limitations =====
> 
> - I could not test the patch with a system with more than 4GiB of RAM.
> 
> - I cannot promise anything with modules. The biggest concern being the
> paddr_t size change, mixing !PAE modules with a PAE kernel may lead to
> unpredictable results. So far I have been lucky, but I would recommend
> building a MONOLITHIC kernel; or rebuild modules with PAE enabled.
> 
> ===== Comments regarding the patch =====
> 
> - reworked locore.S.
> 
> - introduce i386_cpu_switch_pmap(), used to switch pmap for the curcpu.
> Due to the different handling of pmap mappings with PAE vs !PAE, Xen vs
> native, I hid the details within this function. This helps calling it
> from assembly, as some features, like BIOS calls, switch to pmap_kernel
> before mapping trampoline code in low memory.
> 
> WARNING: this function currently *breaks* the amd64 build (obviously,
> there is no i386_cpu_switch_pmap() for amd64).
> 
> ==> this part will be re-written with a cleaner abstraction, in sync
> with rmind and his uvmplock branch. But I would like you to review the
> rest of the patch.
>[snip]

I discussed it rapidly with Mindaugas via private mail, and my patch
does not interfere much with his branch.

I updated my patch [1].

1 - the i386_cpu_switch_pmap() becomes cpu_load_pmap(), is abstracted in
xen/x86/cpu.c and x86/x86/cpu.c (depending on the arch it is currently
running on). The call to this function in pmap_load() is surrounded with
uvm_emap_update/return.

2 - Like the ci_pae_l3_pdir{,pa} variables, xen_current_user_pgd becomes
an element of cpu_info (slowly paving the way for MP world).

There is another limitation I forgot to consider: libkvm does not
contain code that can handle the PAE memory mappings (see
lib/libkvm/kvm_i386.c:_kvm_kvatop()). pd_entry_t/pt_entry_t change size
(8 bytes long), and pl*_pi() macros do not have the same definition
(masks and shifts are different). The libkvm would have to be recompiled
with PAE enabled to handle a PAE kernel dump correctly.


With the updated version, I consider the PAE patch as finished. So,
unless someone objects, I will commit it on Friday evening (European time).

[1] http://www.netbsd.org/~jym/pae.diff

-- 
Jean-Yves Migeon
jeanyves.migeon%free.fr@localhost