Re: [RFC 07/19] KVM: x86/mmu: Factor wrprot for nested PML out of make_spte

[Paolo Bonzini]

On 11/18/21 18:43, Ben Gardon wrote:
> > Aha!  The dependency on @vcpu can be avoided without having to take a flag from
> > the caller.  The shadow page has everything we need.  The check is really "is this
> > a page for L2 EPT".  The kvm_x86_ops.cpu_dirty_log_size gets us the EPT part, and
> > kvm_mmu_page.guest_mode gets us the L2 part.
>
> Haha that's way cleaner than what I was doing! Seems like an obvious
> solution in retrospect. I'll include this in the next version of the
> series I send out unless Paolo beats me and just merges it directly.
> Happy to give this my reviewed-by.

Yeah, I am including the early cleanup parts because it makes no sense
to hold off on them; and Sean's patch qualifies as well.

I can't blame you for not remembering role.guest_mode.  Jim added it for
a decidedly niche reason:

    commit 1313cc2bd8f6568dd8801feef446afbe43e6d313
    Author: Jim Mattson <jmattson@xxxxxxxxxx>
    Date:   Wed May 9 17:02:04 2018 -0400

    kvm: mmu: Add guest_mode to kvm_mmu_page_role
    
    L1 and L2 need to have disjoint mappings, so that L1's APIC access
    page (under VMX) can be omitted from L2's mappings.
    
    Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
    Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
    Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>

though it's actually gotten a lot more important than just that:

    commit 992edeaefed682511bd173dabd2f54b1ce5387df
    Author: Liran Alon <liran.alon@xxxxxxxxxx>
    Date:   Wed Nov 20 14:24:52 2019 +0200

    KVM: nVMX: Assume TLB entries of L1 and L2 are tagged differently if L0 use EPT
    
    Since commit 1313cc2bd8f6 ("kvm: mmu: Add guest_mode to kvm_mmu_page_role"),
    guest_mode was added to mmu-role and therefore if L0 use EPT, it will
    always run L1 and L2 with different EPTP. i.e. EPTP01!=EPTP02.
    
    Because TLB entries are tagged with EP4TA, KVM can assume
    TLB entries populated while running L2 are tagged differently
    than TLB entries populated while running L1.

Paolo