linux-kernel - Re: [PATCH v2 00/25] TDX vCPU/VM creation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CABgObfZsF+1YGTQO_+uF+pBPm-i08BrEGCfTG8_o824776c=6Q@mail.gmail.com>
Date: Mon, 23 Dec 2024 17:25:40 +0100
From: Paolo Bonzini <pbonzini@...hat.com>
To: Rick Edgecombe <rick.p.edgecombe@...el.com>
Cc: seanjc@...gle.com, yan.y.zhao@...el.com, isaku.yamahata@...il.com, 
	kai.huang@...el.com, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	tony.lindgren@...ux.intel.com, xiaoyao.li@...el.com, 
	reinette.chatre@...el.com
Subject: Re: [PATCH v2 00/25] TDX vCPU/VM creation

On Wed, Oct 30, 2024 at 8:01 PM Rick Edgecombe
<rick.p.edgecombe@...el.com> wrote:
>
> Hi,
>
> Here is v2 of TDX VM/vCPU creation series. As discussed earlier, non-nits
> from v1[0] have been applied and it’s ready to hand off to Paolo. A few
> items remain that may be worth further discussion:
>  - Disable CET/PT in tdx_get_supported_xfam(), as these features haven’t
>    been been tested.
>  - The Retry loop around tdh_phymem_page_reclaim() in “KVM: TDX:
>    create/destroy VM structure” likely can be dropped.
>  - Drop support for TDX Module’s that don’t support
>    MD_FIELD_ID_FEATURES0_TOPOLOGY_ENUM. [1]
>  - Type-safety in to_vmx()/to_tdx(). [2]

To sum up:

removed:
04 replaced by add wrapper functions for SEAMCALLs subseries
06: not needed anymore, all logic for KeyID mgmt now in x86/virt/tdx
10: tdx_capabilities dropped, replaced mostly by 02
11: KVM_TDX_CAPABILITIES moved to patch 16
19: not needed anymore
20: was needed by patch 24
22: folded in other patches
24: left for later
25: left for later/for userspace

01/02:ok
03: need to change 32 to 128
04: ok
05/06/07/08/09/10: replaced with
https://lore.kernel.org/kvm/20241203010317.827803-2-rick.p.edgecombe@intel.com/
11: see the type safety comment above:
> The ugly part here is the type-unsafety of to_vmx/to_tdx.  We probably
> should add some "#pragma poison" of to_vmx/to_tdx: for example both can
> be poisoned in pmu_intel.c after the definition of
> vcpu_to_lbr_records(), while one of them can be poisoned in
> sgx.c/posted_intr.c/vmx.c/tdx.c.

12/13/14/15: ok
16/17: to review
18: not sure why the check against num_present_cpus() is needed?
19: ok
20: ok
21: ok

22: missing review comment from v1

> +     /* TDX only supports x2APIC, which requires an in-kernel local APIC. */
> +     if (!vcpu->arch.apic)
> +             return -EINVAL;

nit: Use kvm_apic_present()

23: ok

24: need to apply fix

-       if (sub_leaf & TDX_MD_UNREADABLE_LEAF_MASK ||
+       if (leaf & TDX_MD_UNREADABLE_LEAF_MASK ||

25: ok