KVM: TDX: Don't report base TDVMCALLs

[PATCH] KVM: TDX: Don't report base TDVMCALLs

Posted by Xiaoyao Li 2 months, 3 weeks ago

Remove TDVMCALLINFO_GET_QUOTE from user_tdvmcallinfo_1_r11 reported to
userspace to align with the direction of the GHCI spec.

Recently, concern was raised about a gap in the GHCI spec that left
ambiguity in how to expose to the guest that only a subset of GHCI
TDVMCalls were supported. During the back and forth on the spec details[0],
<GetQuote> was moved from an individually enumerable TDVMCall, to one that
is part of the 'base spec', meaning it doesn't have a specific bit in the
<GetTDVMCallInfo> return values. Although the spec[1] is still in draft
form, the GetQoute part has been agreed by the major TDX VMMs.

Unfortunately the commits that were upstreamed still treat <GetQuote> as
individually enumerable. They set bit 0 in the user_tdvmcallinfo_1_r11
which is reported to userspace to tell supported optional TDVMCalls,
intending to say that <GetQuote> is supported.

So stop reporting <GetQute> in user_tdvmcallinfo_1_r11 to align with
the direction of the spec, and allow some future TDVMCall to use that bit.

[0] https://lore.kernel.org/all/aEmuKII8FGU4eQZz@google.com/
[1] https://cdrdv2.intel.com/v1/dl/getContent/858626

Fixes: 28224ef02b56 ("KVM: TDX: Report supported optional TDVMCALLs in TDX capabilities")
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 arch/x86/kvm/vmx/tdx.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index f31ccdeb905b..ea1261ca805f 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -173,7 +173,6 @@ static void td_init_cpuid_entry2(struct kvm_cpuid_entry2 *entry, unsigned char i
 	tdx_clear_unsupported_cpuid(entry);
 }
 
-#define TDVMCALLINFO_GET_QUOTE				BIT(0)
 #define TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT	BIT(1)
 
 static int init_kvm_tdx_caps(const struct tdx_sys_info_td_conf *td_conf,
@@ -192,7 +191,6 @@ static int init_kvm_tdx_caps(const struct tdx_sys_info_td_conf *td_conf,
 	caps->cpuid.nent = td_conf->num_cpuid_config;
 
 	caps->user_tdvmcallinfo_1_r11 =
-		TDVMCALLINFO_GET_QUOTE |
 		TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT;
 
 	for (i = 0; i < td_conf->num_cpuid_config; i++)
-- 
2.43.0

Re: [PATCH] KVM: TDX: Don't report base TDVMCALLs

Posted by Vishal Annapurve 2 months, 3 weeks ago

On Wed, Jul 16, 2025 at 7:28 PM Xiaoyao Li <xiaoyao.li@intel.com> wrote:
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index f31ccdeb905b..ea1261ca805f 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -173,7 +173,6 @@ static void td_init_cpuid_entry2(struct kvm_cpuid_entry2 *entry, unsigned char i
>         tdx_clear_unsupported_cpuid(entry);
>  }
>
> -#define TDVMCALLINFO_GET_QUOTE                         BIT(0)
>  #define TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT      BIT(1)

I am struggling to find the patch that adds support for
TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT. Can you help point out the
series that adds this support?

Thanks,
Vishal

Re: [PATCH] KVM: TDX: Don't report base TDVMCALLs

Posted by Edgecombe, Rick P 2 months, 3 weeks ago

On Thu, 2025-07-17 at 05:25 -0700, Vishal Annapurve wrote:
> On Wed, Jul 16, 2025 at 7:28 PM Xiaoyao Li <xiaoyao.li@intel.com> wrote:
> > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > index f31ccdeb905b..ea1261ca805f 100644
> > --- a/arch/x86/kvm/vmx/tdx.c
> > +++ b/arch/x86/kvm/vmx/tdx.c
> > @@ -173,7 +173,6 @@ static void td_init_cpuid_entry2(struct kvm_cpuid_entry2 *entry, unsigned char i
> >          tdx_clear_unsupported_cpuid(entry);
> >   }
> > 
> > -#define TDVMCALLINFO_GET_QUOTE                         BIT(0)
> >   #define TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT      BIT(1)
> 
> I am struggling to find the patch that adds support for
> TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT. Can you help point out the
> series that adds this support?

This was the last version of the series posted:
https://lore.kernel.org/kvm/20250619180159.187358-1-pbonzini@redhat.com/#t

But it didn't have those changes. They got added before it was sent upstream.
Possibly in response to this discussion, but I'm not sure:
https://lore.kernel.org/kvm/e6fedf41180a98068c0b410b628f14cb85cad93f.camel@intel.com/

Re: [PATCH] KVM: TDX: Don't report base TDVMCALLs

Posted by Paolo Bonzini 2 months, 3 weeks ago

On 7/17/25 17:24, Edgecombe, Rick P wrote:
> On Thu, 2025-07-17 at 05:25 -0700, Vishal Annapurve wrote:
>> On Wed, Jul 16, 2025 at 7:28 PM Xiaoyao Li <xiaoyao.li@intel.com> wrote:
>>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>>> index f31ccdeb905b..ea1261ca805f 100644
>>> --- a/arch/x86/kvm/vmx/tdx.c
>>> +++ b/arch/x86/kvm/vmx/tdx.c
>>> @@ -173,7 +173,6 @@ static void td_init_cpuid_entry2(struct kvm_cpuid_entry2 *entry, unsigned char i
>>>           tdx_clear_unsupported_cpuid(entry);
>>>    }
>>>
>>> -#define TDVMCALLINFO_GET_QUOTE                         BIT(0)
>>>    #define TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT      BIT(1)
>>
>> I am struggling to find the patch that adds support for
>> TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT. Can you help point out the
>> series that adds this support?
> 
> This was the last version of the series posted:
> https://lore.kernel.org/kvm/20250619180159.187358-1-pbonzini@redhat.com/#t

Yep, I didn't send the patches for SetupEventNotifyInterrupt to the KVM 
mailing list.

I hadn't even noticed until now that I hadn't sent them because, when I 
came back from vacation, I found the matching QEMU patches in my inbox. 
So when git reported them to be in the next pull request I didn't think 
of double checking.  Of course the blame is mine---thanks Xiaoyao for 
writing the QEMU side.

They are:
- commit 28224ef02b56fceee2c161fe2a49a0bb197e44f5
   KVM: TDX: Report supported optional TDVMCALLs in TDX capabilities

- commit 4580dbef5ce0f95a4bd8ac2d007bc4fbf1539332
   KVM: TDX: Exit to userspace for SetupEventNotifyInterrupt

Sorry about that.

Paolo

Re: [PATCH] KVM: TDX: Don't report base TDVMCALLs

Posted by Binbin Wu 2 months, 3 weeks ago


On 7/17/2025 10:20 AM, Xiaoyao Li wrote:
> Remove TDVMCALLINFO_GET_QUOTE from user_tdvmcallinfo_1_r11 reported to
> userspace to align with the direction of the GHCI spec.
>
> Recently, concern was raised about a gap in the GHCI spec that left
> ambiguity in how to expose to the guest that only a subset of GHCI
> TDVMCalls were supported. During the back and forth on the spec details[0],
> <GetQuote> was moved from an individually enumerable TDVMCall, to one that
> is part of the 'base spec', meaning it doesn't have a specific bit in the

'GHCI base API' is more appropriate instead of 'base spec'

> <GetTDVMCallInfo> return values. Although the spec[1] is still in draft
> form, the GetQoute part has been agreed by the major TDX VMMs.
GetQoute  ->  <GetQuote>

typo and use <> to align with others.

>
> Unfortunately the commits that were upstreamed still treat <GetQuote> as
> individually enumerable. They set bit 0 in the user_tdvmcallinfo_1_r11
> which is reported to userspace to tell supported optional TDVMCalls,
> intending to say that <GetQuote> is supported.
>
> So stop reporting <GetQute> in user_tdvmcallinfo_1_r11 to align with

GetQute -> GetQuote

> the direction of the spec, and allow some future TDVMCall to use that bit.
>
> [0] https://lore.kernel.org/all/aEmuKII8FGU4eQZz@google.com/
> [1] https://cdrdv2.intel.com/v1/dl/getContent/858626
>
> Fixes: 28224ef02b56 ("KVM: TDX: Report supported optional TDVMCALLs in TDX capabilities")
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>

Nits: typos and wording suggested above.

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>

> ---
>   arch/x86/kvm/vmx/tdx.c | 2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index f31ccdeb905b..ea1261ca805f 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -173,7 +173,6 @@ static void td_init_cpuid_entry2(struct kvm_cpuid_entry2 *entry, unsigned char i
>   	tdx_clear_unsupported_cpuid(entry);
>   }
>   
> -#define TDVMCALLINFO_GET_QUOTE				BIT(0)
>   #define TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT	BIT(1)
>   
>   static int init_kvm_tdx_caps(const struct tdx_sys_info_td_conf *td_conf,
> @@ -192,7 +191,6 @@ static int init_kvm_tdx_caps(const struct tdx_sys_info_td_conf *td_conf,
>   	caps->cpuid.nent = td_conf->num_cpuid_config;
>   
>   	caps->user_tdvmcallinfo_1_r11 =
> -		TDVMCALLINFO_GET_QUOTE |
>   		TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT;
>   
>   	for (i = 0; i < td_conf->num_cpuid_config; i++)

Re: [PATCH] KVM: TDX: Don't report base TDVMCALLs

Posted by Xiaoyao Li 2 months, 3 weeks ago

On 7/17/2025 10:45 AM, Binbin Wu wrote:
> 
> 
> On 7/17/2025 10:20 AM, Xiaoyao Li wrote:
>> Remove TDVMCALLINFO_GET_QUOTE from user_tdvmcallinfo_1_r11 reported to
>> userspace to align with the direction of the GHCI spec.
>>
>> Recently, concern was raised about a gap in the GHCI spec that left
>> ambiguity in how to expose to the guest that only a subset of GHCI
>> TDVMCalls were supported. During the back and forth on the spec 
>> details[0],
>> <GetQuote> was moved from an individually enumerable TDVMCall, to one 
>> that
>> is part of the 'base spec', meaning it doesn't have a specific bit in the
> 
> 'GHCI base API' is more appropriate instead of 'base spec'
> 
>> <GetTDVMCallInfo> return values. Although the spec[1] is still in draft
>> form, the GetQoute part has been agreed by the major TDX VMMs.
> GetQoute  ->  <GetQuote>
> 
> typo and use <> to align with others.
> 
>>
>> Unfortunately the commits that were upstreamed still treat <GetQuote> as
>> individually enumerable. They set bit 0 in the user_tdvmcallinfo_1_r11
>> which is reported to userspace to tell supported optional TDVMCalls,
>> intending to say that <GetQuote> is supported.
>>
>> So stop reporting <GetQute> in user_tdvmcallinfo_1_r11 to align with
> 
> GetQute -> GetQuote
> 
>> the direction of the spec, and allow some future TDVMCall to use that 
>> bit.
>>
>> [0] https://lore.kernel.org/all/aEmuKII8FGU4eQZz@google.com/
>> [1] https://cdrdv2.intel.com/v1/dl/getContent/858626
>>
>> Fixes: 28224ef02b56 ("KVM: TDX: Report supported optional TDVMCALLs in 
>> TDX capabilities")
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> 
> Nits: typos and wording suggested above.

thanks for catching them!

I would like to leave them to Paolo to fix when he applies the patch 
instead of spinning a v2.

> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> 
>> ---
>>   arch/x86/kvm/vmx/tdx.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>> index f31ccdeb905b..ea1261ca805f 100644
>> --- a/arch/x86/kvm/vmx/tdx.c
>> +++ b/arch/x86/kvm/vmx/tdx.c
>> @@ -173,7 +173,6 @@ static void td_init_cpuid_entry2(struct 
>> kvm_cpuid_entry2 *entry, unsigned char i
>>       tdx_clear_unsupported_cpuid(entry);
>>   }
>> -#define TDVMCALLINFO_GET_QUOTE                BIT(0)
>>   #define TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT    BIT(1)
>>   static int init_kvm_tdx_caps(const struct tdx_sys_info_td_conf 
>> *td_conf,
>> @@ -192,7 +191,6 @@ static int init_kvm_tdx_caps(const struct 
>> tdx_sys_info_td_conf *td_conf,
>>       caps->cpuid.nent = td_conf->num_cpuid_config;
>>       caps->user_tdvmcallinfo_1_r11 =
>> -        TDVMCALLINFO_GET_QUOTE |
>>           TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT;
>>       for (i = 0; i < td_conf->num_cpuid_config; i++)
>