On Tue, Apr 01, 2025 at 09:01:16AM -0400, Xiaoyao Li wrote:
> Date: Tue, 1 Apr 2025 09:01:16 -0400
> From: Xiaoyao Li <xiaoyao...@intel.com>
> Subject: [PATCH v8 06/55] i386/tdx: Introduce is_tdx_vm() helper and cache
> tdx_guest object
> X-Mailer: git-send-email 2.34.1
>
> It will need special handling for TDX VMs all around the QEMU.
> Introduce is_tdx_vm() helper to query if it's a TDX VM.
>
> Cache tdx_guest object thus no need to cast from ms->cgs every time.
>
> Signed-off-by: Xiaoyao Li <xiaoyao...@intel.com>
> Acked-by: Gerd Hoffmann <kra...@redhat.com>
> Reviewed-by: Isaku Yamahata <isaku.yamah...@intel.com>
> ---
> changes in v3:
> - replace object_dynamic_cast with TDX_GUEST();
> ---
> target/i386/kvm/tdx.c | 15 ++++++++++++++-
> target/i386/kvm/tdx.h | 10 ++++++++++
> 2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index c67be5e618e2..16f67e18ae78 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -18,8 +18,16 @@
> #include "kvm_i386.h"
> #include "tdx.h"
>
> +static TdxGuest *tdx_guest;
> +
> static struct kvm_tdx_capabilities *tdx_caps;
>
> +/* Valid after
> kvm_arch_init()->confidential_guest_kvm_init()->tdx_kvm_init() */
> +bool is_tdx_vm(void)
> +{
> + return !!tdx_guest;
> +}
> +
> enum tdx_ioctl_level {
> TDX_VM_IOCTL,
> TDX_VCPU_IOCTL,
> @@ -117,15 +125,20 @@ static int get_tdx_capabilities(Error **errp)
>
> static int tdx_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
> {
> + TdxGuest *tdx = TDX_GUEST(cgs);
> int r = 0;
>
> kvm_mark_guest_state_protected();
>
> if (!tdx_caps) {
> r = get_tdx_capabilities(errp);
> + if (r) {
> + return r;
> + }
> }
>
> - return r;
> + tdx_guest = tdx;
> + return 0;
> }
>
> static int tdx_kvm_type(X86ConfidentialGuest *cg)
> diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h
> index f3b725336161..de8ae9196163 100644
> --- a/target/i386/kvm/tdx.h
> +++ b/target/i386/kvm/tdx.h
> @@ -3,6 +3,10 @@
> #ifndef QEMU_I386_TDX_H
> #define QEMU_I386_TDX_H
>
> +#ifndef CONFIG_USER_ONLY
> +#include CONFIG_DEVICES /* CONFIG_TDX */
> +#endif
> +
> #include "confidential-guest.h"
>
> #define TYPE_TDX_GUEST "tdx-guest"
> @@ -18,4 +22,10 @@ typedef struct TdxGuest {
> uint64_t attributes; /* TD attributes */
> } TdxGuest;
>
> +#ifdef CONFIG_TDX
> +bool is_tdx_vm(void);
> +#else
> +#define is_tdx_vm() 0
> +#endif /* CONFIG_TDX */
> +
a little nit: could we rename it as "tdx_enabled"?
Then the cases like these would be neater?
if (sev_enabled() || is_tdx_vm()) {
...
}
if (sev_enabled()) {
...
} else if (is_tdx_vm()) {
...
}
Otherwise,
Reviewed-by: Zhao Liu <zhao1....@intel.com>
Thanks,
Zhao
