[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0a49bd9b-e4d8-42eb-854c-e8730b5a58b7@intel.com>
Date: Mon, 20 Oct 2025 14:10:12 +0800
From: Xiaoyao Li <xiaoyao.li@...el.com>
To: "Huang, Kai" <kai.huang@...el.com>, "seanjc@...gle.com"
<seanjc@...gle.com>
Cc: "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"pbonzini@...hat.com" <pbonzini@...hat.com>,
"Williams, Dan J" <dan.j.williams@...el.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
"binbin.wu@...ux.intel.com" <binbin.wu@...ux.intel.com>
Subject: Re: [PATCH v2 2/2] KVM: TDX: WARN if a SEAMCALL VM-Exit makes its way
out to KVM
On 10/18/2025 4:58 AM, Huang, Kai wrote:
> On Fri, 2025-10-17 at 10:25 -0700, Sean Christopherson wrote:
>> On Fri, Oct 17, 2025, Kai Huang wrote:
>>> On Thu, 2025-10-16 at 11:21 -0700, Sean Christopherson wrote:
>>>> WARN if KVM observes a SEAMCALL VM-Exit while running a TD guest, as the
>>>> TDX-Module is supposed to inject a #UD, per the "Unconditionally Blocked
>>>> Instructions" section of the TDX-Module base specification.
>>>>
>>>> Reported-by: Xiaoyao Li <xiaoyao.li@...el.com>
>>>> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
>>>> ---
>>>> arch/x86/kvm/vmx/tdx.c | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>>>> index 097304bf1e1d..ffcfe95f224f 100644
>>>> --- a/arch/x86/kvm/vmx/tdx.c
>>>> +++ b/arch/x86/kvm/vmx/tdx.c
>>>> @@ -2148,6 +2148,9 @@ int tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t fastpath)
>>>> * - If it's not an MSMI, no need to do anything here.
>>>> */
>>>> return 1;
>>>> + case EXIT_REASON_SEAMCALL:
>>>> + WARN_ON_ONCE(1);
>>>> + break;
>>>>
>>>
>>> While this exit should never happen from a TDX guest, I am wondering why
>>> we need to explicitly handle the SEAMCALL? E.g., per "Unconditionally
>>> Blocked Instructions" ENCLS/ENCLV are also listed, therefore
>>> EXIT_REASON_ELCLS/ENCLV should never come from a TDX guest either.
>>
>> Good point. SEAMCALL was obviously top of mind, I didn't think about all the
>> other exits that should be impossible.
>>
>> I haven't looked closely, at all, but I wonder if we can get away with this?
>>
>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>> index 097304bf1e1d..4c68444bd673 100644
>> --- a/arch/x86/kvm/vmx/tdx.c
>> +++ b/arch/x86/kvm/vmx/tdx.c
>> @@ -2149,6 +2149,8 @@ int tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t fastpath)
>> */
>> return 1;
>> default:
>> + /* All other known exits should be handled by the TDX-Module. */
>> + WARN_ON_ONCE(exit_reason.basic <= c);
>> break;
>> }
>
> Not 100% sure, but should be fine? Needs more second eyes here.
>
> E.g., when a new module feature makes another exit reason possible then
> presumably we need explicit opt-in to that feature.
>
> Don't quite follow 'exit_reason.basic <= c' part, though. Maybe we can
> just unconditional WARN_ON_ONCE()?
>
> Or we can do things similar to VMX:
>
> vcpu_unimpl(vcpu, "vmx: unexpected exit reason 0x%x\n",
> exit_reason.full);
>
> Or just get rid of this patch :-)
I agree to it.
WARN_ON_ONCE() seems not provide more information except that we can
identify quickly there is TDX module bug when it gets hit.
But it's not too hard to get these information from the
KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON with vp_enter_ret in data[0].
And if we add WARN_ON_ONCE() here, we need to evaluate if it needs to
update "EXIT_REASON_TDCALL" everytime a new EXIT reason is introduced.
e.g., currently the largest exit reason number defined in SDM is 79 (for
WRMSRLIST) and it is actually handled by TDX module and KVM cannot
receive it from a TD vcpu.
Powered by blists - more mailing lists