Re: [PATCH v7 036/102] KVM: x86/mmu: Allow non-zero value for non-present SPTE

Kai Huang
2022-07-20T03:15:53Z

> --- a/arch/x86/kvm/mmu/spte.c
> +++ b/arch/x86/kvm/mmu/spte.c
> @@ -36,6 +36,9 @@ u64 __read_mostly shadow_present_mask;
> u64 __read_mostly shadow_me_value;
> u64 __read_mostly shadow_me_mask;
> u64 __read_mostly shadow_acc_track_mask;
> +#ifdef CONFIG_X86_64
> +u64 __read_mostly shadow_nonpresent_value;
> +#endif

Is this ever used?

>
> u64 __read_mostly shadow_nonpresent_or_rsvd_mask;
> u64 __read_mostly shadow_nonpresent_or_rsvd_lower_gfn_mask;
> @@ -360,7 +363,7 @@ void kvm_mmu_set_mmio_spte_mask(u64 mmio_value, u64 mmio_mask, u64 access_mask)
> * not set any RWX bits.
> */
> if (WARN_ON((mmio_value & mmio_mask) != mmio_value) ||
> - WARN_ON(mmio_value && (REMOVED_SPTE & mmio_mask) == mmio_value))
> + WARN_ON(mmio_value && (__REMOVED_SPTE & mmio_mask) == mmio_value))
> mmio_value = 0;

This chunk doesn't look right, or necessary. We need mmio_mask/mmio_value which
causes EPT violation but with "suppress #VE" bit clear.

So, actually, we want to make sure SHADOW_NONPRESENT_VALUE is *NOT* in mmio_mask
and mmio_value. Using (REMOVED_SPTE & mmio_mask) == mmio_value can actually
ensure SHADOW_NONPRESENT_VALUE is never set in MMIO spte, correct? So I think
using REMOVED_SPTE is fine.

Or maybe additionally adding a explicit check is even better:

if (WARN_ON(mmio_mask & SHADOW_NONPRESENT_VALUE))
mmio_value = 0;

But this change maybe should be in another patch which deals setting up per-VM
mmio_mask/mmio_value anyway. This patch, instead, focuses on allowing non-zero
value for non-present SPTE.