Re: [PAE support] patch review

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

On 20.07.2010 21:30, Manuel Bouyer wrote:
> On Tue, Jul 20, 2010 at 09:04:58PM +0200, Jean-Yves Migeon wrote:
>> [...]
>> 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.
> 
> I guess libkvm could handle both non-PAE and PAE with properly
> abstracted access functions. There would need to be a way to
> check the kernel type at run time though (a sysctl ?) 

I can't touch cpu_kcore_hdr_t without breaking the convention between
the dump and previous libkvm, so I guess this would have to be a sysctl.

I had a quick look; most of the problematic code "hides" in private
sections (within kvm_i386.c); however, there are two functions, namely
_kvm_kvatop() and _kvm_pa2off(), that except PA to be u_long, which is
incorrect for PAE (it should be unsigned long long).

1 - While these 2 functions remain private to kvm(3), supporting PAs
above 4GB would need a change of their prototypes, which will adversely
affect all other ports (as it touches code in kvm.c).

2 - For the rest (pl*_pi() and shifts/masks macros within kvm_kvatop()),
adding PAE namespace to these should be enough.

(2) is rather trivial for me, fixing (1) needs changes in private
prototypes. I don't think this will introduce any regression, but I
would prefer a confirmation first.

Other suggestions welcome.

Cheers,

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