lkml.org 
[lkml]   [2022]   [Apr]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 05/11] KVM: SVM: Re-inject INT3/INTO instead of retrying the instruction
On 23.04.2022 04:14, Sean Christopherson wrote:
> Re-inject INT3/INTO instead of retrying the instruction if the CPU
> encountered an intercepted exception while vectoring the software
> exception, e.g. if vectoring INT3 encounters a #PF and KVM is using
> shadow paging. Retrying the instruction is architecturally wrong, e.g.
> will result in a spurious #DB if there's a code breakpoint on the INT3/O,
> and lack of re-injection also breaks nested virtualization, e.g. if L1
> injects a software exception and vectoring the injected exception
> encounters an exception that is intercepted by L0 but not L1.
>
> Due to, ahem, deficiencies in the SVM architecture, acquiring the next
> RIP may require flowing through the emulator even if NRIPS is supported,
> as the CPU clears next_rip if the VM-Exit is due to an exception other
> than "exceptions caused by the INT3, INTO, and BOUND instructions". To
> deal with this, "skip" the instruction to calculate next_rip (if it's
> not already known), and then unwind the RIP write and any side effects
> (RFLAGS updates).
>
> Save the computed next_rip and use it to re-stuff next_rip if injection
> doesn't complete. This allows KVM to do the right thing if next_rip was
> known prior to injection, e.g. if L1 injects a soft event into L2, and
> there is no backing INTn instruction, e.g. if L1 is injecting an
> arbitrary event.
>
> Note, it's impossible to guarantee architectural correctness given SVM's
> architectural flaws. E.g. if the guest executes INTn (no KVM injection),
> an exit occurs while vectoring the INTn, and the guest modifies the code
> stream while the exit is being handled, KVM will compute the incorrect
> next_rip due to "skipping" the wrong instruction. A future enhancement
> to make this less awful would be for KVM to detect that the decoded
> instruction is not the correct INTn and drop the to-be-injected soft
> event (retrying is a lesser evil compared to shoving the wrong RIP on the
> exception stack).
>
> Reported-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/svm/nested.c | 28 +++++++-
> arch/x86/kvm/svm/svm.c | 140 +++++++++++++++++++++++++++-----------
> arch/x86/kvm/svm/svm.h | 6 +-
> 3 files changed, 130 insertions(+), 44 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 461c5f247801..0163238aa198 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -609,6 +609,21 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
> }
> }
>
> +static inline bool is_evtinj_soft(u32 evtinj)
> +{
> + u32 type = evtinj & SVM_EVTINJ_TYPE_MASK;
> + u8 vector = evtinj & SVM_EVTINJ_VEC_MASK;
> +
> + if (!(evtinj & SVM_EVTINJ_VALID))
> + return false;
> +
> + /*
> + * Intentionally return false for SOFT events, SVM doesn't yet support
> + * re-injecting soft interrupts.
> + */
> + return type == SVM_EVTINJ_TYPE_EXEPT && kvm_exception_is_soft(vector);
> +}
> +
> static void nested_vmcb02_prepare_control(struct vcpu_svm *svm,
> unsigned long vmcb12_rip)
> {
> @@ -677,6 +692,16 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm,
> else if (boot_cpu_has(X86_FEATURE_NRIPS))
> vmcb02->control.next_rip = vmcb12_rip;
>
> + if (is_evtinj_soft(vmcb02->control.event_inj)) {
> + svm->soft_int_injected = true;
> + svm->soft_int_csbase = svm->vmcb->save.cs.base;
> + svm->soft_int_old_rip = vmcb12_rip;
> + if (svm->nrips_enabled)
> + svm->soft_int_next_rip = svm->nested.ctl.next_rip;
> + else
> + svm->soft_int_next_rip = vmcb12_rip;

The above branch can be actually reduced to simply
"svm->soft_int_next_rip = vmcb02->control.next_rip" if NRIPS is required
for (at least) nSVM.

Thanks,
Maciej

\
 
 \ /
  Last update: 2022-04-26 01:01    [W:0.224 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site