| 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
| ||
|
Message-ID: <4475637c-4d34-4037-8f43-8b8280617554@linux.intel.com>
Date: Thu, 17 Jul 2025 09:27:46 +0800
From: Binbin Wu <binbin.wu@...ux.intel.com>
To: Xiaoyao Li <xiaoyao.li@...el.com>
Cc: "Kirill A. Shutemov" <kas@...nel.org>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Sean Christopherson <seanjc@...gle.com>, Paolo Bonzini
<pbonzini@...hat.com>, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
"H. Peter Anvin" <hpa@...or.com>, linux-coco@...ts.linux.dev,
kvm@...r.kernel.org, linux-kernel@...r.kernel.org, x86@...nel.org,
Rick Edgecombe <rick.p.edgecombe@...el.com>, Kai Huang
<kai.huang@...el.com>, yan.y.zhao@...el.com, reinette.chatre@...el.com,
adrian.hunter@...el.com, tony.lindgren@...el.com
Subject: Re: [PATCH v3 3/4] x86/tdx: Rename TDX_ATTR_* to TDX_TD_ATTR_*
On 7/15/2025 5:13 PM, Xiaoyao Li wrote:
> The macros TDX_ATTR_* and DEF_TDX_ATTR_* are related to TD attributes,
> which are TD-scope attributes. Naming them as TDX_ATTR_* can be somewhat
> confusing and might mislead people into thinking they are TDX global
> things.
It seems that tdx_attributes is limited to hold td attributes.
For the same reason, is it better to rename tdx_attributes to tdx_td_attributes?
Otherwise,
Reviewed-by: Binbin Wu <binbin.wu@...ux.intel.com>
> Rename TDX_ATTR_* to TDX_TD_ATTR_* to explicitly clarify they are
> TD-scope things.
>
> Suggested-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@...el.com>
> ---
> arch/x86/coco/tdx/debug.c | 26 ++++++++--------
> arch/x86/coco/tdx/tdx.c | 8 ++---
> arch/x86/include/asm/shared/tdx.h | 50 +++++++++++++++----------------
> arch/x86/kvm/vmx/tdx.c | 4 +--
> 4 files changed, 44 insertions(+), 44 deletions(-)
>
> diff --git a/arch/x86/coco/tdx/debug.c b/arch/x86/coco/tdx/debug.c
> index 28990c2ab0a1..8e477db4ce0a 100644
> --- a/arch/x86/coco/tdx/debug.c
> +++ b/arch/x86/coco/tdx/debug.c
> @@ -7,21 +7,21 @@
> #include <linux/printk.h>
> #include <asm/tdx.h>
>
> -#define DEF_TDX_ATTR_NAME(_name) [TDX_ATTR_##_name##_BIT] = __stringify(_name)
> +#define DEF_TDX_TD_ATTR_NAME(_name) [TDX_TD_ATTR_##_name##_BIT] = __stringify(_name)
>
> static __initdata const char *tdx_attributes[] = {
> - DEF_TDX_ATTR_NAME(DEBUG),
> - DEF_TDX_ATTR_NAME(HGS_PLUS_PROF),
> - DEF_TDX_ATTR_NAME(PERF_PROF),
> - DEF_TDX_ATTR_NAME(PMT_PROF),
> - DEF_TDX_ATTR_NAME(ICSSD),
> - DEF_TDX_ATTR_NAME(LASS),
> - DEF_TDX_ATTR_NAME(SEPT_VE_DISABLE),
> - DEF_TDX_ATTR_NAME(MIGRATABLE),
> - DEF_TDX_ATTR_NAME(PKS),
> - DEF_TDX_ATTR_NAME(KL),
> - DEF_TDX_ATTR_NAME(TPA),
> - DEF_TDX_ATTR_NAME(PERFMON),
> + DEF_TDX_TD_ATTR_NAME(DEBUG),
> + DEF_TDX_TD_ATTR_NAME(HGS_PLUS_PROF),
> + DEF_TDX_TD_ATTR_NAME(PERF_PROF),
> + DEF_TDX_TD_ATTR_NAME(PMT_PROF),
> + DEF_TDX_TD_ATTR_NAME(ICSSD),
> + DEF_TDX_TD_ATTR_NAME(LASS),
> + DEF_TDX_TD_ATTR_NAME(SEPT_VE_DISABLE),
> + DEF_TDX_TD_ATTR_NAME(MIGRATABLE),
> + DEF_TDX_TD_ATTR_NAME(PKS),
> + DEF_TDX_TD_ATTR_NAME(KL),
> + DEF_TDX_TD_ATTR_NAME(TPA),
> + DEF_TDX_TD_ATTR_NAME(PERFMON),
> };
>
> #define DEF_TD_CTLS_NAME(_name) [TD_CTLS_##_name##_BIT] = __stringify(_name)
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 7b2833705d47..186915a17c50 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -238,14 +238,14 @@ static void __noreturn tdx_panic(const char *msg)
> *
> * TDX 1.0 does not allow the guest to disable SEPT #VE on its own. The VMM
> * controls if the guest will receive such #VE with TD attribute
> - * TDX_ATTR_SEPT_VE_DISABLE.
> + * TDX_TD_ATTR_SEPT_VE_DISABLE.
> *
> * Newer TDX modules allow the guest to control if it wants to receive SEPT
> * violation #VEs.
> *
> * Check if the feature is available and disable SEPT #VE if possible.
> *
> - * If the TD is allowed to disable/enable SEPT #VEs, the TDX_ATTR_SEPT_VE_DISABLE
> + * If the TD is allowed to disable/enable SEPT #VEs, the TDX_TD_ATTR_SEPT_VE_DISABLE
> * attribute is no longer reliable. It reflects the initial state of the
> * control for the TD, but it will not be updated if someone (e.g. bootloader)
> * changes it before the kernel starts. Kernel must check TDCS_TD_CTLS bit to
> @@ -254,14 +254,14 @@ static void __noreturn tdx_panic(const char *msg)
> static void disable_sept_ve(u64 td_attr)
> {
> const char *msg = "TD misconfiguration: SEPT #VE has to be disabled";
> - bool debug = td_attr & TDX_ATTR_DEBUG;
> + bool debug = td_attr & TDX_TD_ATTR_DEBUG;
> u64 config, controls;
>
> /* Is this TD allowed to disable SEPT #VE */
> tdg_vm_rd(TDCS_CONFIG_FLAGS, &config);
> if (!(config & TDCS_CONFIG_FLEXIBLE_PENDING_VE)) {
> /* No SEPT #VE controls for the guest: check the attribute */
> - if (td_attr & TDX_ATTR_SEPT_VE_DISABLE)
> + if (td_attr & TDX_TD_ATTR_SEPT_VE_DISABLE)
> return;
>
> /* Relax SEPT_VE_DISABLE check for debug TD for backtraces */
> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> index 11f3cf30b1ac..049638e3da74 100644
> --- a/arch/x86/include/asm/shared/tdx.h
> +++ b/arch/x86/include/asm/shared/tdx.h
> @@ -20,31 +20,31 @@
> #define TDG_VM_RD 7
> #define TDG_VM_WR 8
>
> -/* TDX attributes */
> -#define TDX_ATTR_DEBUG_BIT 0
> -#define TDX_ATTR_DEBUG BIT_ULL(TDX_ATTR_DEBUG_BIT)
> -#define TDX_ATTR_HGS_PLUS_PROF_BIT 4
> -#define TDX_ATTR_HGS_PLUS_PROF BIT_ULL(TDX_ATTR_HGS_PLUS_PROF_BIT)
> -#define TDX_ATTR_PERF_PROF_BIT 5
> -#define TDX_ATTR_PERF_PROF BIT_ULL(TDX_ATTR_PERF_PROF_BIT)
> -#define TDX_ATTR_PMT_PROF_BIT 6
> -#define TDX_ATTR_PMT_PROF BIT_ULL(TDX_ATTR_PMT_PROF_BIT)
> -#define TDX_ATTR_ICSSD_BIT 16
> -#define TDX_ATTR_ICSSD BIT_ULL(TDX_ATTR_ICSSD_BIT)
> -#define TDX_ATTR_LASS_BIT 27
> -#define TDX_ATTR_LASS BIT_ULL(TDX_ATTR_LASS_BIT)
> -#define TDX_ATTR_SEPT_VE_DISABLE_BIT 28
> -#define TDX_ATTR_SEPT_VE_DISABLE BIT_ULL(TDX_ATTR_SEPT_VE_DISABLE_BIT)
> -#define TDX_ATTR_MIGRATABLE_BIT 29
> -#define TDX_ATTR_MIGRATABLE BIT_ULL(TDX_ATTR_MIGRATABLE_BIT)
> -#define TDX_ATTR_PKS_BIT 30
> -#define TDX_ATTR_PKS BIT_ULL(TDX_ATTR_PKS_BIT)
> -#define TDX_ATTR_KL_BIT 31
> -#define TDX_ATTR_KL BIT_ULL(TDX_ATTR_KL_BIT)
> -#define TDX_ATTR_TPA_BIT 62
> -#define TDX_ATTR_TPA BIT_ULL(TDX_ATTR_TPA_BIT)
> -#define TDX_ATTR_PERFMON_BIT 63
> -#define TDX_ATTR_PERFMON BIT_ULL(TDX_ATTR_PERFMON_BIT)
> +/* TDX TD attributes */
> +#define TDX_TD_ATTR_DEBUG_BIT 0
> +#define TDX_TD_ATTR_DEBUG BIT_ULL(TDX_TD_ATTR_DEBUG_BIT)
> +#define TDX_TD_ATTR_HGS_PLUS_PROF_BIT 4
> +#define TDX_TD_ATTR_HGS_PLUS_PROF BIT_ULL(TDX_TD_ATTR_HGS_PLUS_PROF_BIT)
> +#define TDX_TD_ATTR_PERF_PROF_BIT 5
> +#define TDX_TD_ATTR_PERF_PROF BIT_ULL(TDX_TD_ATTR_PERF_PROF_BIT)
> +#define TDX_TD_ATTR_PMT_PROF_BIT 6
> +#define TDX_TD_ATTR_PMT_PROF BIT_ULL(TDX_TD_ATTR_PMT_PROF_BIT)
> +#define TDX_TD_ATTR_ICSSD_BIT 16
> +#define TDX_TD_ATTR_ICSSD BIT_ULL(TDX_TD_ATTR_ICSSD_BIT)
> +#define TDX_TD_ATTR_LASS_BIT 27
> +#define TDX_TD_ATTR_LASS BIT_ULL(TDX_TD_ATTR_LASS_BIT)
> +#define TDX_TD_ATTR_SEPT_VE_DISABLE_BIT 28
> +#define TDX_TD_ATTR_SEPT_VE_DISABLE BIT_ULL(TDX_TD_ATTR_SEPT_VE_DISABLE_BIT)
> +#define TDX_TD_ATTR_MIGRATABLE_BIT 29
> +#define TDX_TD_ATTR_MIGRATABLE BIT_ULL(TDX_TD_ATTR_MIGRATABLE_BIT)
> +#define TDX_TD_ATTR_PKS_BIT 30
> +#define TDX_TD_ATTR_PKS BIT_ULL(TDX_TD_ATTR_PKS_BIT)
> +#define TDX_TD_ATTR_KL_BIT 31
> +#define TDX_TD_ATTR_KL BIT_ULL(TDX_TD_ATTR_KL_BIT)
> +#define TDX_TD_ATTR_TPA_BIT 62
> +#define TDX_TD_ATTR_TPA BIT_ULL(TDX_TD_ATTR_TPA_BIT)
> +#define TDX_TD_ATTR_PERFMON_BIT 63
> +#define TDX_TD_ATTR_PERFMON BIT_ULL(TDX_TD_ATTR_PERFMON_BIT)
>
> /* TDX TD-Scope Metadata. To be used by TDG.VM.WR and TDG.VM.RD */
> #define TDCS_CONFIG_FLAGS 0x1110000300000016
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index efb7d589b672..c539c2e6109f 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -62,7 +62,7 @@ void tdh_vp_wr_failed(struct vcpu_tdx *tdx, char *uclass, char *op, u32 field,
> pr_err("TDH_VP_WR[%s.0x%x]%s0x%llx failed: 0x%llx\n", uclass, field, op, val, err);
> }
>
> -#define KVM_SUPPORTED_TD_ATTRS (TDX_ATTR_SEPT_VE_DISABLE)
> +#define KVM_SUPPORTED_TD_ATTRS (TDX_TD_ATTR_SEPT_VE_DISABLE)
>
> static __always_inline struct kvm_tdx *to_kvm_tdx(struct kvm *kvm)
> {
> @@ -700,7 +700,7 @@ int tdx_vcpu_create(struct kvm_vcpu *vcpu)
> vcpu->arch.l1_tsc_scaling_ratio = kvm_tdx->tsc_multiplier;
>
> vcpu->arch.guest_state_protected =
> - !(to_kvm_tdx(vcpu->kvm)->attributes & TDX_ATTR_DEBUG);
> + !(to_kvm_tdx(vcpu->kvm)->attributes & TDX_TD_ATTR_DEBUG);
>
> if ((kvm_tdx->xfam & XFEATURE_MASK_XTILE) == XFEATURE_MASK_XTILE)
> vcpu->arch.xfd_no_write_intercept = true;
Powered by blists - more mailing lists