| Message ID | 20201218003139.2167891-5-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86/mmu: Bug fixes and cleanups in get_mmio_spte() | expand |
Sean Christopherson <seanjc@google.com> writes: > Check only the terminal leaf for a "!PRESENT || MMIO" SPTE when looking > for reserved bits on valid, non-MMIO SPTEs. The get_walk() helpers > terminate their walks if a not-present or MMIO SPTE is encountered, i.e. > the non-terminal SPTEs have already been verified to be regular SPTEs. > This eliminates an extra check-and-branch in a relatively hot loop. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/mmu/mmu.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index 4798a4472066..769855f5f0a1 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -3511,7 +3511,7 @@ static int get_walk(struct kvm_vcpu *vcpu, u64 addr, u64 *sptes, int *root_level > return leaf; > } > > -/* return true if reserved bit is detected on spte. */ > +/* return true if reserved bit(s) are detected on a valid, non-MMIO SPTE. */ > static bool get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) > { > u64 sptes[PT64_ROOT_MAX_LEVEL + 1]; > @@ -3534,11 +3534,20 @@ static bool get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) > return reserved; > } > > + *sptep = sptes[leaf]; > + > + /* > + * Skip reserved bits checks on the terminal leaf if it's not a valid > + * SPTE. Note, this also (intentionally) skips MMIO SPTEs, which, by > + * design, always have reserved bits set. The purpose of the checks is > + * to detect reserved bits on non-MMIO SPTEs. i.e. buggy SPTEs. > + */ > + if (!is_shadow_present_pte(sptes[leaf])) > + leaf++; > + > rsvd_check = &vcpu->arch.mmu->shadow_zero_check; > > - for (level = root; level >= leaf; level--) { > - if (!is_shadow_present_pte(sptes[level])) > - break; > + for (level = root; level >= leaf; level--) > /* > * Use a bitwise-OR instead of a logical-OR to aggregate the > * reserved bit and EPT's invalid memtype/XWR checks to avoid > @@ -3546,7 +3555,6 @@ static bool get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) > */ > reserved |= __is_bad_mt_xwr(rsvd_check, sptes[level]) | > __is_rsvd_bits_set(rsvd_check, sptes[level], level); > - } > > if (reserved) { > pr_err("%s: detect reserved bits on spte, addr 0x%llx, dump hierarchy:\n", > @@ -3556,8 +3564,6 @@ static bool get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) > sptes[level], level); > } > > - *sptep = sptes[leaf]; > - > return reserved; > } FWIW (as I got a bit lost in tdp-mmu code when checking that is_shadow_present_pte() is always performed) Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 4798a4472066..769855f5f0a1 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -3511,7 +3511,7 @@ static int get_walk(struct kvm_vcpu *vcpu, u64 addr, u64 *sptes, int *root_level return leaf; } -/* return true if reserved bit is detected on spte. */ +/* return true if reserved bit(s) are detected on a valid, non-MMIO SPTE. */ static bool get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) { u64 sptes[PT64_ROOT_MAX_LEVEL + 1]; @@ -3534,11 +3534,20 @@ static bool get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) return reserved; } + *sptep = sptes[leaf]; + + /* + * Skip reserved bits checks on the terminal leaf if it's not a valid + * SPTE. Note, this also (intentionally) skips MMIO SPTEs, which, by + * design, always have reserved bits set. The purpose of the checks is + * to detect reserved bits on non-MMIO SPTEs. i.e. buggy SPTEs. + */ + if (!is_shadow_present_pte(sptes[leaf])) + leaf++; + rsvd_check = &vcpu->arch.mmu->shadow_zero_check; - for (level = root; level >= leaf; level--) { - if (!is_shadow_present_pte(sptes[level])) - break; + for (level = root; level >= leaf; level--) /* * Use a bitwise-OR instead of a logical-OR to aggregate the * reserved bit and EPT's invalid memtype/XWR checks to avoid @@ -3546,7 +3555,6 @@ static bool get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) */ reserved |= __is_bad_mt_xwr(rsvd_check, sptes[level]) | __is_rsvd_bits_set(rsvd_check, sptes[level], level); - } if (reserved) { pr_err("%s: detect reserved bits on spte, addr 0x%llx, dump hierarchy:\n", @@ -3556,8 +3564,6 @@ static bool get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) sptes[level], level); } - *sptep = sptes[leaf]; - return reserved; }
Check only the terminal leaf for a "!PRESENT || MMIO" SPTE when looking for reserved bits on valid, non-MMIO SPTEs. The get_walk() helpers terminate their walks if a not-present or MMIO SPTE is encountered, i.e. the non-terminal SPTEs have already been verified to be regular SPTEs. This eliminates an extra check-and-branch in a relatively hot loop. Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/mmu/mmu.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)