Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src/sys/arch

On Tue, 8 Nov 2011 08:49:22 +0530, Cherry G. Mathew wrote:
Please keep ci_pae_l3_pdir to a uint32_t and back out its paddr_t type.

Unless you can convince me that your code will do the right thing(tm), the PDP for PAE should to be allocated below the 4GiB physical boundary; unless mistaken, the uvm_km_alloc(kernel_map, ...) does not enforce that. Besides, %cr3 is still a 32 bits entity under PAE. Trying to stash a 64 bits paddr_t
in %cr3 is a really bad idea.

See comments

This doesn't hold true for Xen. See:

Xen does the <4GB translation for the VM by maintaining its own
shadow. I just took the easier route ( after initially using the x86
pmap < 4GB cr3 code ), but if you're thinking of a unified a
xen/non-xen implementation we'll have to re-think this one.

Okay; then all users of cr3 have to be documented (_especially_ the native x86 code), as there at least two places where there's implicit uint64_t => uint32_t truncation: lcr3() and pcb_cr3. A comment above these should be enough. Same goes for having paddr_t for L3 PD in struct cpu_info.

- are you sure that these have to be
"defined(PAE) || defined(__x86_64__)" ?

- please use "for (i = 0; i < PTP_LEVELS - 1; i++ ) { ... }". I will have to identify these blocks later when GENERIC will include both PAE and !PAE
pmaps. PTP_LEVELS makes it a lot easier.

ok, will watchout for it - do you want to do this one yourself ?

Please do :)

Jean-Yves Migeon

Home | Main Index | Thread Index | Old Index