[PATCH] kvm: x86/mmu: Fix get_mmio_spte() on CPUs supporting 5-level PT

[PATCH] kvm: x86/mmu: Fix get_mmio_spte() on CPUs supporting 5-level PT

[Vitaly Kuznetsov]

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;
-- 
2.26.2

Re: [PATCH] kvm: x86/mmu: Fix get_mmio_spte() on CPUs supporting 5-level PT

[Paolo Bonzini]

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