[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7b4d5df0-f554-2fc0-5c19-021f8eb3f6aa@amd.com>
Date: Wed, 2 Aug 2023 10:26:31 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Wu Zongyo <wuzongyo@...l.ustc.edu.cn>,
linux-kernel@...r.kernel.org, kvm@...r.kernel.org, x86@...nel.org,
linux-coco@...ts.linux.dev
Subject: Re: [Question] int3 instruction generates a #UD in SEV VM
On 8/2/23 10:04, Sean Christopherson wrote:
> On Wed, Aug 02, 2023, Tom Lendacky wrote:
>> On 8/2/23 09:25, Tom Lendacky wrote:
>>> On 8/2/23 09:01, Sean Christopherson wrote:
>>>>> You're right. The #UD is injected by KVM.
>>>>>
>>>>> The path I found is:
>>>>> svm_vcpu_run
>>>>> svm_complete_interrupts
>>>>> kvm_requeue_exception // vector = 3
>>>>> kvm_make_request
>>>>>
>>>>> vcpu_enter_guest
>>>>> kvm_check_and_inject_events
>>>>> svm_inject_exception
>>>>> svm_update_soft_interrupt_rip
>>>>> __svm_skip_emulated_instruction
>>>>> x86_emulate_instruction
>>>>> svm_can_emulate_instruction
>>>>> kvm_queue_exception(vcpu, UD_VECTOR)
>>>>>
>>>>> Does this mean a #PF intercept occur when the guest try to deliver a
>>>>> #BP through the IDT? But why?
>>>>
>>>> I doubt it's a #PF. A #NPF is much more likely, though it could be
>>>> something
>>>> else entirely, but I'm pretty sure that would require bugs in both
>>>> the host and
>>>> guest.
>>>>
>>>> What is the last exit recorded by trace_kvm_exit() before the #UD is
>>>> injected?
>>>
>>> I'm guessing it was a #NPF, too. Could it be related to the changes that
>>> went in around svm_update_soft_interrupt_rip()?
>>>
>>> 6ef88d6e36c2 ("KVM: SVM: Re-inject INT3/INTO instead of retrying the
>>> instruction")
>>
>> Sorry, that should have been:
>>
>> 7e5b5ef8dca3 ("KVM: SVM: Re-inject INTn instead of retrying the insn on "failure"")
>>
>>>
>>> Before this the !nrips check would prevent the call into
>>> svm_skip_emulated_instruction(). But now, there is a call to:
>>>
>>> svm_update_soft_interrupt_rip()
>>> __svm_skip_emulated_instruction()
>>> kvm_emulate_instruction()
>>> x86_emulate_instruction() (passed a NULL insn pointer)
>>> kvm_can_emulate_insn() (passed a NULL insn pointer)
>>> svm_can_emulate_instruction() (passed NULL insn pointer)
>>>
>>> Because it is an SEV guest, it ends up in the "if (unlikely(!insn))" path
>>> and injects the #UD.
>
> Yeah, my money is on that too. I believe this is the least awful solution:
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index d381ad424554..2eace114a934 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -385,6 +385,9 @@ static int __svm_skip_emulated_instruction(struct kvm_vcpu *vcpu,
> }
>
> if (!svm->next_rip) {
> + if (sev_guest(vcpu->kvm))
> + return 0;
> +
> if (unlikely(!commit_side_effects))
> old_rflags = svm->vmcb->save.rflags;
>
> I'll send a formal patch (with a comment) if that solves the problem.
>
>
> Side topic, KVM should require nrips for SEV and beyond, I don't see how SEV can
> possibly work if KVM doesn't utilize nrips. E.g. this
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 2eace114a934..43e500503d48 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -5111,9 +5111,11 @@ static __init int svm_hardware_setup(void)
>
> svm_adjust_mmio_mask();
>
> + nrips = nrips && boot_cpu_has(X86_FEATURE_NRIPS);
> +
> /*
> * Note, SEV setup consumes npt_enabled and enable_mmio_caching (which
> - * may be modified by svm_adjust_mmio_mask()).
> + * may be modified by svm_adjust_mmio_mask()), as well as nrips.
> */
> sev_hardware_setup();
You moved the setting of nrips up, I'm assuming you then want to add a
check in sev_hardware_setup() for nrips?
Thanks,
Tom
>
> @@ -5125,11 +5127,6 @@ static __init int svm_hardware_setup(void)
> goto err;
> }
>
> - if (nrips) {
> - if (!boot_cpu_has(X86_FEATURE_NRIPS))
> - nrips = false;
> - }
> -
> enable_apicv = avic = avic && avic_hardware_setup();
>
> if (!enable_apicv) {
>
Powered by blists - more mailing lists