Re: [PATCH v1 3/4] KVM: MMU: Add 5 level EPT & Shadow page table support.

[Yu Zhang]

On 8/14/2017 10:13 PM, Paolo Bonzini wrote:
> On 14/08/2017 13:37, Yu Zhang wrote:
> > Thanks a lot for your comments, Paolo. :-)
> >
> >
> > On 8/14/2017 3:31 PM, Paolo Bonzini wrote:
> > > On 12/08/2017 15:35, Yu Zhang wrote:
> > > >    struct rsvd_bits_validate {
> > > > -    u64 rsvd_bits_mask[2][4];
> > > > +    u64 rsvd_bits_mask[2][5];
> > > >        u64 bad_mt_xwr;
> > > >    };
> > > Can you change this 4 to PT64_ROOT_MAX_LEVEL in patch 2?
> > Well, I had tried, but failed to find a neat approach to do so. The
> > difficulty I have met is that PT64_ROOT_MAX_LEVEL is defined together
> > with PT64_ROOT_4LEVEL/PT32E_ROOT_LEVEL/PT32_ROOT_LEVEL in mmu.h, yet
> > the rsvd_bits_validate structure is defined in kvm_host.h, which are
> > included in quite a lot .c files that do not include mmu.h or include
> > the mmu.h after kvm_host.h.
> >
> > I guess that's the reason why the magic number 4 instead of
> > PT64_ROOT_4LEVEL is used in current definition of rsvd_bits_vadlidate. :-)
> Yes, you're right.  I think the solution is to define
> PT64_ROOT_MAX_LEVEL in kvm_host.h.

Thanks, Paolo. How about we also move the definition of PT64_ROOT_4LEVEL/
PT32E_ROOT_LEVEL/PT32_ROOT_LEVEL from mmu.h to kvm_host.h? Then we
can define PT64_ROOT_MAX_LEVEL as PT64_ROOT_4LEVEL instead of 4 in 
kvm_host.h.

> > > > @@ -4444,7 +4457,7 @@ void kvm_init_shadow_ept_mmu(struct kvm_vcpu
> > > > *vcpu, bool execonly,
> > > >          MMU_WARN_ON(VALID_PAGE(context->root_hpa));
> > > >    -    context->shadow_root_level = kvm_x86_ops->get_tdp_level();
> > > > +    context->shadow_root_level = kvm_x86_ops->get_tdp_level(vcpu);
> > > >          context->nx = true;
> > > >        context->ept_ad = accessed_dirty;
> > > Below, there is:
> > >
> > >           context->root_level = context->shadow_root_level;
> > >
> > > this should be forced to PT64_ROOT_4LEVEL until there is support for
> > > nested EPT 5-level page tables.
> > So the context->shadow_root_level could be 5 or 4, and
> > context->root_level is always 4?
> That was my idea, but setting both to 4 should be fine too as you
> suggest below.
>
> > My understanding is that shadow ept level should be determined by
> > the width of ngpa, and that if L1 guest is not exposed with EPT5
> > feature, it shall only use 4 level ept for L2 guest, and the shadow
> > ept does not need a 5 level one. Is this understanding correct? And
> > how about we set both values to PT64_ROOT_4LEVEL for now?>
> > Besides, if we wanna support nested EPT5, what do you think we need to
> > do besides exposing the EPT5 feature to L1 guest?
> Nothing else, I think.

Thanks. I'll try to keep both values fixed to PT64_ROOT_4LEVEL then. :-)
For nested EPT5, we can enable it later(should be a quite simple patch, 
but need to
be verified in our simics environment, which I am not sure if nested 
scenario works).

B.R.
Yu