[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zmsl6WtT40-u4Pkn@google.com>
Date: Thu, 13 Jun 2024 10:01:29 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Xin Li <xin3.li@...el.com>
Cc: linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
linux-doc@...r.kernel.org, linux-kselftest@...r.kernel.org,
pbonzini@...hat.com, corbet@....net, tglx@...utronix.de, mingo@...hat.com,
bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
shuah@...nel.org, vkuznets@...hat.com, peterz@...radead.org,
ravi.v.shankar@...el.com, xin@...or.com
Subject: Re: [PATCH v2 13/25] KVM: VMX: Handle VMX nested exception for FRED
On Wed, Feb 07, 2024, Xin Li wrote:
> @@ -725,8 +733,28 @@ static void kvm_multiple_exception(struct kvm_vcpu *vcpu,
> vcpu->arch.exception.injected = false;
> vcpu->arch.exception.pending = false;
>
> + /*
> + * A #DF is NOT a nested event per its definition, however per
> + * FRED spec 5.0 Appendix B, its delivery determines the new
> + * stack level as is done for events occurring when CPL = 0.
> + */
Similar to my comments about CR2, this is mostly noise. Unless I'm missing a
patch, KVM is not responsible for emulating the stack level stuff, and so there
is zero reason to mention it, because it's not relevant to the KVM code. Simply
say that #DF is defined to not be a nested exception.
/* #DF is NOT a nested event, per its definition. */
> + vcpu->arch.exception.nested = false;
> +
> kvm_queue_exception_e(vcpu, DF_VECTOR, 0);
> } else {
> + /*
> + * FRED spec 5.0 Appendix B: delivery of a nested exception
> + * determines the new stack level as is done for events
> + * occurring when CPL = 0.
> + *
> + * IOW, FRED event delivery of an event encountered in ring 3
> + * normally uses stack level 0 unconditionally. However, if
> + * the event is an exception nested on any earlier event,
> + * delivery of the nested exception will consult the FRED MSR
> + * IA32_FRED_STKLVLS to determine which stack level to use.
> + */
And drop this entirely. The above does not help the reader understand _why_ KVM
sets nested=true for FRED. E.g. there's no CPL check here.
IMO, this code is entirely self-explanatory; KVM is quite obviously handling a
back-to-back exceptions, and it doesn't take a big mental leap to grok that FRED
tracks that information by describing the second exception as "nested".
> + vcpu->arch.exception.nested = kvm_is_fred_enabled(vcpu);
> +
> /* replace previous exception with a new one in a hope
> that instruction re-execution will regenerate lost
> exception */
Powered by blists - more mailing lists