lkml.org 
[lkml]   [2020]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 04/22] kvm: mmu: Allocate and free TDP MMU roots
Heads up, you may get this multiple times, our mail servers got "upgraded"
recently and are giving me troubles...

On Mon, Oct 12, 2020 at 03:59:35PM -0700, Ben Gardon wrote:
> On Tue, Sep 29, 2020 at 11:06 PM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> > > @@ -3691,7 +3690,13 @@ static int mmu_alloc_direct_roots(struct kvm_vcpu *vcpu)
> > > unsigned i;
> > >
> > > if (shadow_root_level >= PT64_ROOT_4LEVEL) {
> > > - root = mmu_alloc_root(vcpu, 0, 0, shadow_root_level, true);
> > > + if (vcpu->kvm->arch.tdp_mmu_enabled) {
> >
> > I believe this will break 32-bit NPT. Or at a minimum, look weird. It'd
> > be better to explicitly disable the TDP MMU on 32-bit KVM, then this becomes
> >
> > if (vcpu->kvm->arch.tdp_mmu_enabled) {
> >
> > } else if (shadow_root_level >= PT64_ROOT_4LEVEL) {
> >
> > } else {
> >
> > }
> >
>
> How does this break 32-bit NPT? I'm not sure I understand how we would
> get into a bad state here because I'm not familiar with the specifics
> of 32 bit NPT.

32-bit NPT will have a max TDP level of PT32E_ROOT_LEVEL (3), i.e. will
fail the "shadow_root_level >= PT64_ROOT_4LEVEL" check, and thus won't get
to the tdp_mmu_enabled check. That would likely break as some parts of KVM
would see tdp_mmu_enabled, but this root allocation would continue using
the legacy MMU.

It's somewhat of a moot point, because IIRC there are other things that will
break with 32-bit KVM, i.e. TDP MMU will be 64-bit only. But burying that
assumption/dependency in these flows is weird.

> > > + root = kvm_tdp_mmu_get_vcpu_root_hpa(vcpu);
> > > + } else {
> > > + root = mmu_alloc_root(vcpu, 0, 0, shadow_root_level,
> > > + true);
> > > + }

\
 
 \ /
  Last update: 2020-10-13 02:00    [W:0.210 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site