Jeremy Fitzhardinge wrote:

Zachary Amsden wrote:

quoted

I though about it, but it gets really ugly.  You need wrappers for 
all the MMU ops in pvops generic code, which means either another 
layer of wrappers or a bunch of CONFIG_DEBUG_PARAVIRT

Oh, yes, more wrappers please!  We could do it at the paravirt_ops 
level: set up your pv_ops, then call paravirt_debug_mmuops(), which 
would save away your ops and replace them with wrappers.  That basic 
structure would lend itself to all kinds of paravirt-level debugging 
tools.

It would be a bit more elegant if we had mmu_ops.  Maybe we should do 
that splitup before 64bit?

zamsden strongly agrees jsgf.

quoted

only things that are easy to break because they also depend on PAE 
vs. non-PAE.

Hm, would they?  Would they need to inspect the content of the pte_t 
(etc), or just look at the struct page for the page being updated?  
(pmd operations being a bit more awkward, of course.)

No, but there are different sets of operations and different sets of 
validation rules (for example, multiple PGDs per page for PAE, depending 
on pgd allocation size).

Well, I have to say that keeping the hypervisor hints in sync with the 
actual kernel-level page state worries me, so any debug tools which 
could help there would be good.  This seems like it should be the 
right place to do it, but I can't say I've thought about it in any 
detail.

Yes, these things are subtle and easy to break.  In fact, still broken 
(although in a different way) - check out this naked write to a pte:

arch/i386/mm/init.c:            pte->pte_high &= ~(1 << (_PAGE_BIT_NX - 
32));
arch/i386/mm/init.c:            pte->pte_high |= 1 << (_PAGE_BIT_NX - 32);

Not only has page typing created a dependence on the guest accurately 
representing page state to the hypervisor, it also requires the guest to 
use proper APIs for access to those pages.  That is much harder to 
enforce - perhaps static checking can get some of the way there.

The page type debugging is certainly a start in a good direction.

Of course, if it *is* helpful to the page hinting patches, then it 
suggests that it's a facility with wider scope than pv-ops.

I certainly think it's not wide enough to justify a new struct page 
field, but perhaps a debug-only field is commonly useful; those pg bits 
are getting quite crammed.

Zach