| Message ID | 20210622175739.3610207-17-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86/mmu: Bug fixes and summer cleaning | expand |
On 22/06/21 19:57, Sean Christopherson wrote: > Drop the smep_andnot_wp role check from the "uses NX" calculation now > that all non-nested shadow MMUs treat NX as used via the !TDP check. > > The shadow MMU for nested NPT, which shares the helper, does not need to > deal with SMEP (or WP) as NPT walks are always "user" accesses and WP is > explicitly noted as being ignored: > > Table walks for guest page tables are always treated as user writes at > the nested page table level. > > A table walk for the guest page itself is always treated as a user > access at the nested page table level > > The host hCR0.WP bit is ignored under nested paging. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/mmu/mmu.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index 96c16a6e0044..ca7680d1ea24 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -4223,8 +4223,7 @@ reset_shadow_zero_bits_mask(struct kvm_vcpu *vcpu, struct kvm_mmu *context) > * NX can be used by any non-nested shadow MMU to avoid having to reset > * MMU contexts. Note, KVM forces EFER.NX=1 when TDP is disabled. > */ > - bool uses_nx = context->nx || !tdp_enabled || > - context->mmu_role.base.smep_andnot_wp; > + bool uses_nx = context->nx || !tdp_enabled; > struct rsvd_bits_validate *shadow_zero_check; > int i; > > Good idea, but why not squash it into patch 2? Paolo
On Wed, Jun 23, 2021, Paolo Bonzini wrote: > On 22/06/21 19:57, Sean Christopherson wrote: > > Drop the smep_andnot_wp role check from the "uses NX" calculation now > > that all non-nested shadow MMUs treat NX as used via the !TDP check. > > > > The shadow MMU for nested NPT, which shares the helper, does not need to > > deal with SMEP (or WP) as NPT walks are always "user" accesses and WP is > > explicitly noted as being ignored: > > > > Table walks for guest page tables are always treated as user writes at > > the nested page table level. > > > > A table walk for the guest page itself is always treated as a user > > access at the nested page table level > > > > The host hCR0.WP bit is ignored under nested paging. > > > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > --- > > arch/x86/kvm/mmu/mmu.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > > index 96c16a6e0044..ca7680d1ea24 100644 > > --- a/arch/x86/kvm/mmu/mmu.c > > +++ b/arch/x86/kvm/mmu/mmu.c > > @@ -4223,8 +4223,7 @@ reset_shadow_zero_bits_mask(struct kvm_vcpu *vcpu, struct kvm_mmu *context) > > * NX can be used by any non-nested shadow MMU to avoid having to reset > > * MMU contexts. Note, KVM forces EFER.NX=1 when TDP is disabled. > > */ > > - bool uses_nx = context->nx || !tdp_enabled || > > - context->mmu_role.base.smep_andnot_wp; > > + bool uses_nx = context->nx || !tdp_enabled; > > struct rsvd_bits_validate *shadow_zero_check; > > int i; > > > > Good idea, but why not squash it into patch 2? Because that patch is marked for stable and dropping the smep_andnot_wp is not necessary to fix the bug. At worst, the too-liberal uses_nx will suppress the WARN in handle_mmio_page_fault() because this is for checking KVM's SPTEs, not the guest's SPTEs, i.e. KVM won't miss a guest reserved NX #PF. That said, I'm not at all opposed to squashing this. I have a feeling I originally split the patches because I wasn't super confident about either change, and never revisited them.
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 96c16a6e0044..ca7680d1ea24 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -4223,8 +4223,7 @@ reset_shadow_zero_bits_mask(struct kvm_vcpu *vcpu, struct kvm_mmu *context) * NX can be used by any non-nested shadow MMU to avoid having to reset * MMU contexts. Note, KVM forces EFER.NX=1 when TDP is disabled. */ - bool uses_nx = context->nx || !tdp_enabled || - context->mmu_role.base.smep_andnot_wp; + bool uses_nx = context->nx || !tdp_enabled; struct rsvd_bits_validate *shadow_zero_check; int i;
Drop the smep_andnot_wp role check from the "uses NX" calculation now that all non-nested shadow MMUs treat NX as used via the !TDP check. The shadow MMU for nested NPT, which shares the helper, does not need to deal with SMEP (or WP) as NPT walks are always "user" accesses and WP is explicitly noted as being ignored: Table walks for guest page tables are always treated as user writes at the nested page table level. A table walk for the guest page itself is always treated as a user access at the nested page table level The host hCR0.WP bit is ignored under nested paging. Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/mmu/mmu.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)