On Thu, Jul 29, 2021, Yu Zhang wrote:
> On Wed, Jul 28, 2021 at 06:37:38PM +0000, Sean Christopherson wrote:
> > On Wed, Jul 28, 2021, Yu Zhang wrote:
> In the caller, force_tdp_unload was set to false for CR0/CR4/EFER changes. For SMM and
> cpuid updates, it is set to true.
>
> With this change, I can successfully boot a VM(and of course, number of unloadings is
> greatly reduced). But access test case in kvm-unit-test hangs, after CR4.SMEP is flipped.
> I'm trying to figure out why...
Hrm, I'll look into when I get around to making this into a proper patch.
Note, there's at least once bug, as is_root_usable() will compare the full role
against a root shadow page's modified role. A common helper to derive the page
role for a direct/TDP page from an existing mmu_role is likely the way to go, as
kvm_tdp_mmu_get_vcpu_root_hpa() would want the same functionality.
> > I'll put this on my todo list, I've been looking for an excuse to update the
> > cr0/cr4/efer flows anyways :-). If it works, the changes should be relatively
> > minor, if it works...
> >
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index a8cdfd8d45c4..700664fe163e 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -2077,8 +2077,20 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
> > role = vcpu->arch.mmu->mmu_role.base;
> > role.level = level;
> > role.direct = direct;
> > - if (role.direct)
> > + if (role.direct) {
> > role.gpte_is_8_bytes = true;
> > +
> > + /*
> > + * Guest PTE permissions do not impact SPTE permissions for
> > + * direct MMUs. Either there are no guest PTEs (CR0.PG=0) or
> > + * guest PTE permissions are enforced by the CPU (TDP enabled).
> > + */
> > + WARN_ON_ONCE(access != ACC_ALL);
> > + role.efer_nx = 0;
> > + role.cr0_wp = 0;
> > + role.smep_andnot_wp = 0;
> > + role.smap_andnot_wp = 0;
> > + }
>
> How about we do this in kvm_calc_mmu_role_common()? :-)
No, because the role in struct kvm_mmu does need the correct bits, even for TDP,
as the role is used to detect whether or not the context needs to be re-initialized,
e.g. it would get a false negative on a cr0_wp change, not go through
update_permission_bitmask(), and use the wrong page permissions when walking the
guest page tables.
