Re: [PATCH] vmx/nested: Set the SGX feature flag only when hardware supported.

[zhaogezhang <18341265598@163.com>]

Thanks to all the maintainers for your timely feedback and correction. Much appreciated!


> 在 2026年4月1日，05:07，Huang, Kai <kai.huang@intel.com> 写道：
> 
> On Tue, 2026-03-31 at 09:30 -0700, Sean Christopherson wrote:
>>> On Tue, Mar 24, 2026, Kai Huang wrote:
>>> On Tue, 2026-03-24 at 11:27 +0800, 18341265598@163.com wrote:
>>>> From: "zhaoge.zhang" <zhangzg12@chinatelecom.cn>
>>>> 
>>>> If the hardware does not support the SGX feature and we set the
>>>> corresponding flag, when the L1 hypervisor enables the corresponding
>>>> feature in VMCS12, the VM entry will fail.
>>>> 
>>>> Reported-by: wei xiaoqiong <weixq1@chinatelecom.cn>
>>>> Signed-off-by: zhaoge.zhang <zhangzg12@chinatelecom.cn>
>>>> Reviewed-by: Huaitong Han <hanht2@chinatelecom.cn>
>>>> ---
>>>> arch/x86/kvm/vmx/nested.c |  2 +-
>>>> arch/x86/kvm/vmx/vmx.c    | 11 -----------
>>>> arch/x86/kvm/vmx/vmx.h    | 11 +++++++++++
>>>> 3 files changed, 12 insertions(+), 12 deletions(-)
>>>> 
>>>> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
>>>> index 937aeb4..396ac07 100644
>>>> --- a/arch/x86/kvm/vmx/nested.c
>>>> +++ b/arch/x86/kvm/vmx/nested.c
>>>> @@ -7278,7 +7278,7 @@ static void nested_vmx_setup_secondary_ctls(u32 ept_caps,
>>>>        msrs->secondary_ctls_high |=
>>>>            SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
>>>> 
>>>> -    if (enable_sgx)
>>>> +    if (enable_sgx && cpu_has_sgx())
>>>>        msrs->secondary_ctls_high |= SECONDARY_EXEC_ENCLS_EXITING;
>>>> }
>>> 
>>> Are you seeing the case where enable_sgx is true but cpu_has_sgx() reports
>>> false?
>>> 
>>> It's quite strange because during KVM load, if cpu_has_sgx() is false,
>>> enable_sgx is guaranteed to be false.
>>> 
>>> The only case that I can think of is after KVM loads somehow machine check
>>> happens, which "soft disables" SGX (CPUID reports SGX1 as 0).
>>> 
>>> Is this the case you are meeting?
>> 
>> It's a moot point (unless I'm getting -ENOCOFFEE, which is very possible at the
>> moment), because "enable_sgx" is cleared if ENCLS-exiting isn't supported.
>> 
>>    if (!cpu_has_vmx_encls_vmexit())
>>        enable_sgx = false;
>> 
>> Where cpu_has_vmx_encls_vmexit() is peeking at vmcs_config.cpu_based_2nd_exec_ctrl:
>> 
>>  static inline bool cpu_has_vmx_encls_vmexit(void)
>>  {
>>    return vmcs_config.cpu_based_2nd_exec_ctrl &
>>        SECONDARY_EXEC_ENCLS_EXITING;
>>  }
>> 
>> And thus already incorporates this code:
>> 
>>    if (!cpu_has_sgx())
>>        _cpu_based_2nd_exec_control &= ~SECONDARY_EXEC_ENCLS_EXITING;
>> 
>> 
>> I suppose the #MC could happen between setup_vmcs_config() and
>> nested_vmx_setup_ctls_msrs(), but at that point, KVM is hosed no matter what.
> 
> Exactly.  I think we can ignore this unless we can get more info from the
> author :-)