On Wed, 2023-10-18 at 12:41 -0700, Sean Christopherson wrote:
> Revert KVM's made-up consistency check on SVM's TLB control. The APM says
> that unsupported encodings are reserved, but the APM doesn't state that
> VMRUN checks for a supported encoding. Unless something is called out
> in "Canonicalization and Consistency Checks" or listed as MBZ (Must Be
> Zero), AMD behavior is typically to let software shoot itself in the foot.
>
> This reverts commit 174a921b6975ef959dd82ee9e8844067a62e3ec1.
>
> Fixes: 174a921b6975 ("nSVM: Check for reserved encodings of TLB_CONTROL in nested VMCB")
> Reported-by: Stefan Sterz <s.sterz@xxxxxxxxxxx>
> Closes: https://lkml.kernel.org/r/b9915c9c-4cf6-051a-2d91-44cc6380f455%40proxmox.com
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
> arch/x86/kvm/svm/nested.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 3fea8c47679e..60891b9ce25f 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -247,18 +247,6 @@ static bool nested_svm_check_bitmap_pa(struct kvm_vcpu *vcpu, u64 pa, u32 size)
> kvm_vcpu_is_legal_gpa(vcpu, addr + size - 1);
> }
>
> -static bool nested_svm_check_tlb_ctl(struct kvm_vcpu *vcpu, u8 tlb_ctl)
> -{
> - /* Nested FLUSHBYASID is not supported yet. */
> - switch(tlb_ctl) {
> - case TLB_CONTROL_DO_NOTHING:
> - case TLB_CONTROL_FLUSH_ALL_ASID:
> - return true;
> - default:
> - return false;
> - }
> -}
> -
> static bool __nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
> struct vmcb_ctrl_area_cached *control)
> {
> @@ -278,9 +266,6 @@ static bool __nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
> IOPM_SIZE)))
> return false;
>
> - if (CC(!nested_svm_check_tlb_ctl(vcpu, control->tlb_ctl)))
> - return false;
> -
> if (CC((control->int_ctl & V_NMI_ENABLE_MASK) &&
> !vmcb12_is_intercept(control, INTERCEPT_NMI))) {
> return false;
Yes, after checking Jim's comment (*) on this I still agree that revert is OK.
KVM never passes through the tlb_ctl field (but does copy it to the cache),
thus there is no need to sanitize it.
https://www.spinics.net/lists/kvm/msg316072.html
Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
Best regards,
Maxim Levitsky
