On Tue, Apr 27, 2010 at 04:35:06PM +0300, Avi Kivity wrote: > On 04/27/2010 04:20 PM, Joerg Roedel wrote: > >Hmm, difficult since both mmu's are active in the npt-npt case. The > >arch.mmu struct contains mostly the l1 paging state initialized for > >shadow paging and different set_cr3/get_cr3/inject_page_fault functions. > >This keeps the changes to the mmu small and optimize for the common case > >(a nested npt fault). > > Well, it reduces the changes to the mmu, but it makes a 'struct > kvm_mmu' incoherent since its meaning depends on whether it is > nested or not. For someone reading the code, it is hard to see when > to use ->nested_mmu or ->mmu. > > Perhaps have > > struct kvm_mmu base_mmu; > struct kvm_mmu nested_mmu; > struct kvm_mmu *mmu; > > You could have one patch that mindlessly changes mmu. to mmu->. The > impact of the patchset increases, but I think the result is more > readable. > > It will be a pain adapting the patchset, but easier than updating > mmu.txt to reflect the current situation. Okay, I finally read most of mmu.txt :) I agree that the implemented scheme for using arch.mmu and arch.nested_mmu is non-obvious. The most complicated thing is that .gva_to_gpa functions are exchanged between mmu and nested_mmu. This means that nested_mmu.gva_to_gpa basically walks a page table in a mode described by arch.mmu while mmu.gva_to_gpa walks the full two-dimensional page table. But I also don't yet see how a *mmu pointer would help here. From my current understanding of the idea the only place to use it would be the init_kvm_mmu path. But maybe we could also do this to make things more clear: * Rename nested_mmu to l2_mmu and use it to save the current paging mode of the l2 guest * Do not switch the the .gva_to_gpa function pointers between mmu contexts and provide a wrapper function for all callers of arch.mmu.gva_to_gpa which uses the right one for the current context. The arch.nested flag is the indicator for which one to use. Btw. the purpose of the nested-flag is to prevent to reset arch.mmu when the l2 guest changes his paging mode and we could remove this flag with a pointer-solution. Currently its a bit unclear when to use mmu or nested_mmu. With a pointer it would be unclear to the code reader when to use the pointer and when to select the mmu_contexts directly. Joerg -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
