Re: [PATCH 10/25] KVM: TDX: Initialize KVM supported capabilities when module setup

Xiaoyao Li
2024-10-10T08:25:37Z

On 9/11/2024 7:04 PM, Tony Lindgren wrote:
> On Tue, Sep 10, 2024 at 07:15:12PM +0200, Paolo Bonzini wrote:
>> On 8/29/24 06:51, Tony Lindgren wrote:
>>>> nit: Since there are other similarly named functions that come later how
>>>> about rename this to init_kvm_tdx_caps, so that it's clear that the
>>>> functions that are executed ones are prefixed with "init_" and those that
>>>> will be executed on every TDV boot up can be named prefixed with "setup_"
>>> We can call setup_kvm_tdx_caps() from from tdx_get_kvm_supported_cpuid(),
>>> and drop the struct kvm_tdx_caps. So then the setup_kvm_tdx_caps() should
>>> be OK.
>>
>> I don't understand this suggestion since tdx_get_capabilities() also needs
>> kvm_tdx_caps. I think the code is okay as it is with just the rename that
>> Nik suggested (there are already some setup_*() functions in KVM but for
>> example setup_vmcs_config() is called from hardware_setup()).
>
> Oh sorry for the confusion, looks like I pasted the function names wrong
> way around above and left out where setup_kvm_tdx_caps() can be called
> from.
>
> I meant only tdx_get_capabilities() needs to call setup_kvm_tdx_caps().
> And setup_kvm_tdx_caps() calls tdx_get_kvm_supported_cpuid().
>
> The data in kvm_tdx_caps is only needed for tdx_get_capabilities(). It can
> be generated from the data already in td_conf.
>
> At least that's what it looks like to me, but maybe I'm missing something.

kvm_tdx_caps is setup in __tdx_bringup() because it also serves the
purpose to validate the KVM's capabilities against the specific TDX
module. If KVM and TDX module are incompatible, it needs to fail the
bring up of TDX in KVM. It's too late to validate it when
KVM_TDX_CAPABILITIES issued. E.g., if the TDX module reports some
fixed-1 attribute bit while KVM isn't aware of, in such case KVM needs
to set enable_tdx to 0 to reflect that TDX cannot be enabled/brought up.

> Regards,
>
> Tony