| Message ID | 20201126110206.2118959-1-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kvm: x86/mmu: Fix get_mmio_spte() on CPUs supporting 5-level PT | expand |
On 26/11/20 12:02, Vitaly Kuznetsov wrote: > Commit 95fb5b0258b7 ("kvm: x86/mmu: Support MMIO in the TDP MMU") caused > the following WARNING on an Intel Ice Lake CPU: > > get_mmio_spte: detect reserved bits on spte, addr 0xb80a0, dump hierarchy: > ------ spte 0xb80a0 level 5. > ------ spte 0xfcd210107 level 4. > ------ spte 0x1004c40107 level 3. > ------ spte 0x1004c41107 level 2. > ------ spte 0x1db00000000b83b6 level 1. > WARNING: CPU: 109 PID: 10254 at arch/x86/kvm/mmu/mmu.c:3569 kvm_mmu_page_fault.cold.150+0x54/0x22f [kvm] > ... > Call Trace: > ? kvm_io_bus_get_first_dev+0x55/0x110 [kvm] > vcpu_enter_guest+0xaa1/0x16a0 [kvm] > ? vmx_get_cs_db_l_bits+0x17/0x30 [kvm_intel] > ? skip_emulated_instruction+0xaa/0x150 [kvm_intel] > kvm_arch_vcpu_ioctl_run+0xca/0x520 [kvm] > > The guest triggering this crashes. Note, this happens with the traditional > MMU and EPT enabled, not with the newly introduced TDP MMU. Turns out, > there was a subtle change in the above mentioned commit. Previously, > walk_shadow_page_get_mmio_spte() was setting 'root' to 'iterator.level' > which is returned by shadow_walk_init() and this equals to > 'vcpu->arch.mmu->shadow_root_level'. Now, get_mmio_spte() sets it to > 'int root = vcpu->arch.mmu->root_level'. > > The difference between 'root_level' and 'shadow_root_level' on CPUs > supporting 5-level page tables is that in some case we don't want to > use 5-level, in particular when 'cpuid_maxphyaddr(vcpu) <= 48' > kvm_mmu_get_tdp_level() returns '4'. In case upper layer is not used, > the corresponding SPTE will fail '__is_rsvd_bits_set()' check. > > Revert to using 'shadow_root_level'. > > Fixes: 95fb5b0258b7 ("kvm: x86/mmu: Support MMIO in the TDP MMU") > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > - The usual (for KVM MMU) caveat 'I may be missing everything' applies, > please review) > --- > arch/x86/kvm/mmu/mmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index 5bb1939b65d8..7a6ae9e90bd7 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -3517,7 +3517,7 @@ static bool get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) > { > u64 sptes[PT64_ROOT_MAX_LEVEL]; > struct rsvd_bits_validate *rsvd_check; > - int root = vcpu->arch.mmu->root_level; > + int root = vcpu->arch.mmu->shadow_root_level; > int leaf; > int level; > bool reserved = false; > Queued, thanks. Paolo
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 5bb1939b65d8..7a6ae9e90bd7 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -3517,7 +3517,7 @@ static bool get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) { u64 sptes[PT64_ROOT_MAX_LEVEL]; struct rsvd_bits_validate *rsvd_check; - int root = vcpu->arch.mmu->root_level; + int root = vcpu->arch.mmu->shadow_root_level; int leaf; int level; bool reserved = false;
Commit 95fb5b0258b7 ("kvm: x86/mmu: Support MMIO in the TDP MMU") caused the following WARNING on an Intel Ice Lake CPU: get_mmio_spte: detect reserved bits on spte, addr 0xb80a0, dump hierarchy: ------ spte 0xb80a0 level 5. ------ spte 0xfcd210107 level 4. ------ spte 0x1004c40107 level 3. ------ spte 0x1004c41107 level 2. ------ spte 0x1db00000000b83b6 level 1. WARNING: CPU: 109 PID: 10254 at arch/x86/kvm/mmu/mmu.c:3569 kvm_mmu_page_fault.cold.150+0x54/0x22f [kvm] ... Call Trace: ? kvm_io_bus_get_first_dev+0x55/0x110 [kvm] vcpu_enter_guest+0xaa1/0x16a0 [kvm] ? vmx_get_cs_db_l_bits+0x17/0x30 [kvm_intel] ? skip_emulated_instruction+0xaa/0x150 [kvm_intel] kvm_arch_vcpu_ioctl_run+0xca/0x520 [kvm] The guest triggering this crashes. Note, this happens with the traditional MMU and EPT enabled, not with the newly introduced TDP MMU. Turns out, there was a subtle change in the above mentioned commit. Previously, walk_shadow_page_get_mmio_spte() was setting 'root' to 'iterator.level' which is returned by shadow_walk_init() and this equals to 'vcpu->arch.mmu->shadow_root_level'. Now, get_mmio_spte() sets it to 'int root = vcpu->arch.mmu->root_level'. The difference between 'root_level' and 'shadow_root_level' on CPUs supporting 5-level page tables is that in some case we don't want to use 5-level, in particular when 'cpuid_maxphyaddr(vcpu) <= 48' kvm_mmu_get_tdp_level() returns '4'. In case upper layer is not used, the corresponding SPTE will fail '__is_rsvd_bits_set()' check. Revert to using 'shadow_root_level'. Fixes: 95fb5b0258b7 ("kvm: x86/mmu: Support MMIO in the TDP MMU") Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- - The usual (for KVM MMU) caveat 'I may be missing everything' applies, please review) --- arch/x86/kvm/mmu/mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)