Linux Confidential Computing Development
 help / color / mirror / Atom feed
* [RFC PATCH v1 1/2] KVM: arm64: CCA: Add support for configuring the Realm MEC policy
From: Kohei Enju @ 2026-07-24  9:40 UTC (permalink / raw)
  To: linux-arm-kernel, kvm, kvmarm, cgroups, linux-coco
  Cc: Paolo Bonzini, Marc Zyngier, Oliver Upton, Joey Gouly,
	Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
	Will Deacon, Tejun Heo, Johannes Weiner, Michal Koutný,
	Steven Price, Kohei Enju
In-Reply-To: <20260724094120.166536-1-enju.kohei@fujitsu.com>

Introduce the KVM_ARM_RMI_CONFIG ioctl to allow userspace to configure
Realm VM parameters before the Realm is created.

Currently, ARM_RMI_CFG_MEC_POLICY is the only supported configuration
item. Its value can be ARM_RMI_MEC_POLICY_SHARED or
ARM_RMI_MEC_POLICY_PRIVATE. Reject the private policy if the platform
does not support private MECs.

If userspace does not explicitly configure the MEC policy,
ARM_RMI_MEC_POLICY_SHARED is used by default.

Signed-off-by: Kohei Enju <enju.kohei@fujitsu.com>
---
 Documentation/virt/kvm/api.rst   | 29 +++++++++++++++++++++++++++++
 arch/arm64/include/asm/kvm_rmi.h |  4 ++++
 arch/arm64/kvm/arm.c             |  9 +++++++++
 arch/arm64/kvm/rmi.c             | 29 +++++++++++++++++++++++++++++
 drivers/firmware/arm_rmm/rmi.c   |  8 ++++++--
 include/linux/arm-rmi-cmds.h     |  1 +
 include/uapi/linux/kvm.h         | 17 +++++++++++++++++
 7 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 85bec9b4f021..e1c660e5eb34 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6690,6 +6690,35 @@ populated data is hashed and added to the guest's Realm Initial Measurement
 (RIM) stored by the RMM. This can then be retrieved by the guest (using the RSI
 interface) to present to an attestation server.
 
+4.147 KVM_ARM_RMI_CONFIG
+------------------------
+
+:Capability: KVM_CAP_ARM_RMI
+:Architectures: arm64
+:Type: vm ioctl
+:Parameters: struct kvm_arm_rmi_config (in)
+:Returns: 0 on success, < 0 on error
+
+::
+
+  struct kvm_arm_rmi_config {
+       __u32 cfg;
+       union {
+               /* cfg == ARM_RMI_CFG_MEC_POLICY */
+               __u8 mec_policy;
+
+               /* Fix the size of the union */
+               __u8 reserved[256];
+       };
+  };
+
+Configures parameters of a Realm VM before the Realm is created.
+
+Currently, `ARM_RMI_CFG_MEC_POLICY` is the only supported configuration item.
+`mec_policy` must be either `ARM_RMI_MEC_POLICY_SHARED` or
+`ARM_RMI_MEC_POLICY_PRIVATE`. `ARM_RMI_MEC_POLICY_PRIVATE` is not supported if
+no private MEC is available on the platform.
+
 .. _kvm_run:
 
 5. The kvm_run structure
diff --git a/arch/arm64/include/asm/kvm_rmi.h b/arch/arm64/include/asm/kvm_rmi.h
index 420e99fca07e..721033b132e1 100644
--- a/arch/arm64/include/asm/kvm_rmi.h
+++ b/arch/arm64/include/asm/kvm_rmi.h
@@ -60,6 +60,7 @@ enum realm_state {
  * @ia_bits: Number of valid Input Address bits in the IPA
  * @stage2_unmapped: The Realm stage-2 mappings have been removed
  * @rtts_destroyed: The non-root RTTs have been torn down
+ * @mec_policy: MEC policy for the Realm VM
  */
 struct realm {
 	void *rd;
@@ -76,6 +77,7 @@ struct realm {
 	unsigned int ia_bits;
 	bool stage2_unmapped;
 	bool rtts_destroyed;
+	unsigned int mec_policy;
 };
 
 /**
@@ -115,6 +117,8 @@ struct kvm_arm_rmi_populate;
 
 int kvm_arm_rmi_populate(struct kvm *kvm,
 			 struct kvm_arm_rmi_populate *arg);
+struct kvm_arm_rmi_config;
+int kvm_arm_rmi_config(struct kvm *kvm, struct kvm_arm_rmi_config *arg);
 void kvm_realm_unmap_range(struct kvm *kvm,
 			   unsigned long ipa,
 			   unsigned long size,
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 3862db30779f..17302ba2844c 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -2169,6 +2169,15 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
 			return -EFAULT;
 		return ret;
 	}
+	case KVM_ARM_RMI_CONFIG: {
+		struct kvm_arm_rmi_config cfg;
+
+		if (!kvm_is_realm(kvm))
+			return -ENXIO;
+		if (copy_from_user(&cfg, argp, sizeof(cfg)))
+			return -EFAULT;
+		return kvm_arm_rmi_config(kvm, &cfg);
+	}
 	default:
 		return -EINVAL;
 	}
diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
index 06cc85fb5092..629dea5e8c47 100644
--- a/arch/arm64/kvm/rmi.c
+++ b/arch/arm64/kvm/rmi.c
@@ -572,6 +572,9 @@ static int realm_create_rd(struct kvm *kvm)
 	if (kvm_lpa2_is_enabled())
 		params->flags0 |= RMI_REALM_PARAM_FLAG_LPA2;
 
+	params->flags0 |= FIELD_PREP(RMI_REALM_PARAM_FLAG_MEC_POLICY,
+				     realm->mec_policy);
+
 	r = realm_init_sve_param(kvm, params);
 	if (r)
 		goto out_undelegate_tables;
@@ -1155,6 +1158,30 @@ int kvm_arm_rmi_populate(struct kvm *kvm,
 	return ret;
 }
 
+int kvm_arm_rmi_config(struct kvm *kvm, struct kvm_arm_rmi_config *cfg)
+{
+	guard(mutex)(&kvm->arch.config_lock);
+
+	if (kvm_realm_state(kvm) != REALM_STATE_NONE)
+		return -EBUSY;
+
+	switch (cfg->cfg) {
+	case ARM_RMI_CFG_MEC_POLICY:
+		if (cfg->mec_policy != ARM_RMI_MEC_POLICY_SHARED &&
+		    cfg->mec_policy != ARM_RMI_MEC_POLICY_PRIVATE)
+			return -EINVAL;
+
+		if (cfg->mec_policy == ARM_RMI_MEC_POLICY_PRIVATE &&
+		    rmi_mec_count() == 0)
+			return -EOPNOTSUPP;
+
+		kvm->arch.realm.mec_policy = cfg->mec_policy;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
 static void kvm_complete_ripas_change(struct kvm_vcpu *vcpu)
 {
 	struct kvm *kvm = vcpu->kvm;
@@ -1474,6 +1501,8 @@ int kvm_init_realm(struct kvm *kvm)
 {
 	struct realm *realm = &kvm->arch.realm;
 
+	realm->mec_policy = ARM_RMI_MEC_POLICY_SHARED;
+
 	realm->params = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
 	realm->sro = kmalloc_obj(*realm->sro);
 	if (!realm->params || !realm->sro) {
diff --git a/drivers/firmware/arm_rmm/rmi.c b/drivers/firmware/arm_rmm/rmi.c
index d0c083bdf251..e9632c35e7db 100644
--- a/drivers/firmware/arm_rmm/rmi.c
+++ b/drivers/firmware/arm_rmm/rmi.c
@@ -14,8 +14,7 @@
 
 static bool arm64_rmi_is_available;
 
-/* Currently only the first 2 registers are used by Linux */
-#define RMI_FEAT_REG_COUNT	2
+#define RMI_FEAT_REG_COUNT	5
 static __ro_after_init unsigned long rmi_feat_reg_cache[RMI_FEAT_REG_COUNT];
 
 unsigned long rmi_feat_reg(unsigned long id)
@@ -27,6 +26,11 @@ unsigned long rmi_feat_reg(unsigned long id)
 }
 EXPORT_SYMBOL_GPL(rmi_feat_reg);
 
+u64 rmi_mec_count(void)
+{
+	return u64_get_bits(rmi_feat_reg(4), RMI_FEATURE_REGISTER_4_MEC_COUNT);
+}
+
 int rmi_delegate_range(phys_addr_t phys,
 		       unsigned long size,
 		       phys_addr_t *out_phys)
diff --git a/include/linux/arm-rmi-cmds.h b/include/linux/arm-rmi-cmds.h
index 138983ab4e3c..5236600421c6 100644
--- a/include/linux/arm-rmi-cmds.h
+++ b/include/linux/arm-rmi-cmds.h
@@ -28,6 +28,7 @@ struct rmi_sro_state {
 };
 
 unsigned long rmi_feat_reg(unsigned long id);
+u64 rmi_mec_count(void);
 
 int rmi_delegate_range(phys_addr_t phys, unsigned long size,
 		       phys_addr_t *out_phys);
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index adee3936d6ae..46e62327957c 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1704,4 +1704,21 @@ struct kvm_arm_rmi_populate {
 	__u32 reserved;
 };
 
+#define KVM_ARM_RMI_CONFIG     _IOW(KVMIO, 0xd8, struct kvm_arm_rmi_config)
+
+#define ARM_RMI_CFG_MEC_POLICY		0
+#define ARM_RMI_MEC_POLICY_SHARED	0
+#define ARM_RMI_MEC_POLICY_PRIVATE	1
+
+struct kvm_arm_rmi_config {
+	__u32 cfg;
+	union {
+		/* cfg == ARM_RMI_CFG_MEC_POLICY */
+		__u8 mec_policy;
+
+		/* Reserve space for future configuration payloads. */
+		__u8 reserved[256];
+	};
+};
+
 #endif /* __LINUX_KVM_H */
-- 
2.43.0


^ permalink raw reply related

* [RFC PATCH v1 2/2] cgroup/misc: Add support for Arm CCA MECIDs
From: Kohei Enju @ 2026-07-24  9:40 UTC (permalink / raw)
  To: linux-arm-kernel, kvm, kvmarm, cgroups, linux-coco
  Cc: Paolo Bonzini, Marc Zyngier, Oliver Upton, Joey Gouly,
	Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
	Will Deacon, Tejun Heo, Johannes Weiner, Michal Koutný,
	Steven Price, Kohei Enju
In-Reply-To: <20260724094120.166536-1-enju.kohei@fujitsu.com>

Arm CCA MECIDs are limited platform resources. Add a misc cgroup
resource to allow the system administrator to account for and limit
their use.

The cca_mec resource represents the number of Realm VMs using the
private MEC policy. Each such Realm consumes one MECID. Charge the
resource when the Realm is created, and uncharge it when the Realm is
successfully destroyed.

Realms using the shared MEC policy don't consume this resource.

Signed-off-by: Kohei Enju <enju.kohei@fujitsu.com>
---
 arch/arm64/include/asm/kvm_rmi.h |  2 ++
 arch/arm64/kvm/rmi.c             | 44 +++++++++++++++++++++++++++++++-
 drivers/firmware/arm_rmm/rmi.c   |  4 +++
 include/linux/misc_cgroup.h      |  4 +++
 kernel/cgroup/misc.c             |  4 +++
 5 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_rmi.h b/arch/arm64/include/asm/kvm_rmi.h
index 721033b132e1..182bdd22014f 100644
--- a/arch/arm64/include/asm/kvm_rmi.h
+++ b/arch/arm64/include/asm/kvm_rmi.h
@@ -61,6 +61,7 @@ enum realm_state {
  * @stage2_unmapped: The Realm stage-2 mappings have been removed
  * @rtts_destroyed: The non-root RTTs have been torn down
  * @mec_policy: MEC policy for the Realm VM
+ * @misc_cg: Misc cgroup charged while the Realm owns a private MECID
  */
 struct realm {
 	void *rd;
@@ -78,6 +79,7 @@ struct realm {
 	bool stage2_unmapped;
 	bool rtts_destroyed;
 	unsigned int mec_policy;
+	struct misc_cg *misc_cg;
 };
 
 /**
diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
index 629dea5e8c47..505fa80f635d 100644
--- a/arch/arm64/kvm/rmi.c
+++ b/arch/arm64/kvm/rmi.c
@@ -5,6 +5,7 @@
 
 #include <uapi/linux/psci.h>
 #include <linux/kvm_host.h>
+#include <linux/misc_cgroup.h>
 
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_mmu.h>
@@ -525,6 +526,36 @@ static int realm_init_sve_param(struct kvm *kvm, struct realm_params *params)
 	return 0;
 }
 
+static int realm_mec_charge(struct realm *realm)
+{
+	struct misc_cg *misc_cg;
+	int ret;
+
+	if (realm->mec_policy != ARM_RMI_MEC_POLICY_PRIVATE)
+		return 0;
+
+	misc_cg = get_current_misc_cg();
+
+	ret = misc_cg_try_charge(MISC_CG_RES_CCA_MEC, misc_cg, 1);
+	if (ret) {
+		put_misc_cg(misc_cg);
+		return ret;
+	}
+
+	realm->misc_cg = misc_cg;
+	return 0;
+}
+
+static void realm_mec_uncharge(struct realm *realm)
+{
+	if (!realm->misc_cg)
+		return;
+
+	misc_cg_uncharge(MISC_CG_RES_CCA_MEC, realm->misc_cg, 1);
+	put_misc_cg(realm->misc_cg);
+	realm->misc_cg = NULL;
+}
+
 static int realm_create_rd(struct kvm *kvm)
 {
 	struct realm *realm = &kvm->arch.realm;
@@ -579,11 +610,15 @@ static int realm_create_rd(struct kvm *kvm)
 	if (r)
 		goto out_undelegate_tables;
 
+	r = realm_mec_charge(realm);
+	if (r)
+		goto out_undelegate_tables;
+
 	params_phys = virt_to_phys(params);
 
 	if (rmi_realm_create(rd_phys, params_phys, realm->sro)) {
 		r = -ENXIO;
-		goto out_undelegate_tables;
+		goto out_uncharge_misc_cg;
 	}
 
 	realm->rd = rd;
@@ -594,6 +629,8 @@ static int realm_create_rd(struct kvm *kvm)
 
 	return 0;
 
+out_uncharge_misc_cg:
+	realm_mec_uncharge(realm);
 out_undelegate_tables:
 	if (WARN_ON(rmi_undelegate_range(kvm->arch.mmu.pgd_phys, top_delegated - kvm->arch.mmu.pgd_phys))) {
 		/* Leak the pages if they cannot be returned */
@@ -1478,10 +1515,15 @@ void kvm_destroy_realm(struct kvm *kvm)
 		if (WARN_ON(realm_destroy_rtts(kvm)))
 			return;
 
+		/*
+		 * Keep the misc cgroup charge if RMI_REALM_DESTROY fails, as
+		 * the corresponding MECID may still be allocated by the RMM.
+		 */
 		if (WARN_ON(rmi_realm_destroy(rd_phys, realm->sro)))
 			return;
 		free_delegated_page(rd_phys);
 		realm->rd = NULL;
+		realm_mec_uncharge(realm);
 	}
 
 	if (WARN_ON(rmi_undelegate_range(kvm->arch.mmu.pgd_phys,
diff --git a/drivers/firmware/arm_rmm/rmi.c b/drivers/firmware/arm_rmm/rmi.c
index e9632c35e7db..85a7e6686d2b 100644
--- a/drivers/firmware/arm_rmm/rmi.c
+++ b/drivers/firmware/arm_rmm/rmi.c
@@ -6,6 +6,7 @@
 #include <linux/cpufeature.h>
 #include <linux/memblock.h>
 #include <linux/arm-rmi-cmds.h>
+#include <linux/misc_cgroup.h>
 #include <linux/processor.h>
 #include <linux/slab.h>
 
@@ -714,6 +715,9 @@ static int __init arm64_init_rmi(void)
 	if (ret)
 		return ret;
 
+	WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_CCA_MEC,
+					  rmi_mec_count()));
+
 	arm64_rmi_is_available = true;
 	pr_info("RMI configured");
 
diff --git a/include/linux/misc_cgroup.h b/include/linux/misc_cgroup.h
index 0cb36a3ffc47..b542502dc52a 100644
--- a/include/linux/misc_cgroup.h
+++ b/include/linux/misc_cgroup.h
@@ -21,6 +21,10 @@ enum misc_res_type {
 #ifdef CONFIG_INTEL_TDX_HOST
 	/** @MISC_CG_RES_TDX: Intel TDX HKIDs resource */
 	MISC_CG_RES_TDX,
+#endif
+#ifdef CONFIG_ARM_RMM
+	/** @MISC_CG_RES_CCA_MEC: ARM64 CCA MECIDs resource */
+	MISC_CG_RES_CCA_MEC,
 #endif
 	/** @MISC_CG_RES_TYPES: count of enum misc_res_type constants */
 	MISC_CG_RES_TYPES
diff --git a/kernel/cgroup/misc.c b/kernel/cgroup/misc.c
index 4a9e2557141c..69259085115e 100644
--- a/kernel/cgroup/misc.c
+++ b/kernel/cgroup/misc.c
@@ -28,6 +28,10 @@ static const char *const misc_res_name[] = {
 	/* Intel TDX HKIDs resource */
 	"tdx",
 #endif
+#ifdef CONFIG_ARM_RMM
+	/* ARM64 CCA MECIDs resource */
+	"cca_mec",
+#endif
 };
 
 /* Root misc cgroup */
-- 
2.43.0


^ permalink raw reply related

* [RFC PATCH v1 0/2] KVM: arm64: CCA: Add MEC policy support for CCA Realms
From: Kohei Enju @ 2026-07-24  9:40 UTC (permalink / raw)
  To: linux-arm-kernel, kvm, kvmarm, cgroups, linux-coco
  Cc: Paolo Bonzini, Marc Zyngier, Oliver Upton, Joey Gouly,
	Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
	Will Deacon, Tejun Heo, Johannes Weiner, Michal Koutný,
	Steven Price, Kohei Enju

This series adds basic support for configuring the Memory Encryption
Context (MEC) policy for Arm CCA Realms.

Patch 1 introduces a new Realm VM ioctl, KVM_ARM_RMI_CONFIG, and uses it
to let userspace select the shared or private MEC policy before the
Realm is created. The default remains the shared MEC policy.

Patch 2 adds a misc cgroup resource for CCA MECIDs and charges one unit
for each Realm using the private MEC policy.

Feedback on the proposed KVM ABI and on using misc cgroup for MECID
accounting would be appreciated.

This series is based on Steven Price's CCA host series [0] [1]. I am
sending it as an RFC while the CCA host series is not yet upstreamed,
and plan to update it as needed to track future revisions of the host
series. Once the host series is merged, I plan to post a non-RFC
version.

[0] https://lore.kernel.org/all/20260715142841.80544-1-steven.price@arm.com/ 
[1] https://gitlab.arm.com/linux-arm/linux-cca  cca-host/v15

Kohei Enju (2):
  KVM: arm64: CCA: Add support for configuring the Realm MEC policy
  cgroup/misc: Add support for Arm CCA MECIDs

 Documentation/virt/kvm/api.rst   | 29 +++++++++++++
 arch/arm64/include/asm/kvm_rmi.h |  6 +++
 arch/arm64/kvm/arm.c             |  9 ++++
 arch/arm64/kvm/rmi.c             | 73 +++++++++++++++++++++++++++++++-
 drivers/firmware/arm_rmm/rmi.c   | 12 +++++-
 include/linux/arm-rmi-cmds.h     |  1 +
 include/linux/misc_cgroup.h      |  4 ++
 include/uapi/linux/kvm.h         | 17 ++++++++
 kernel/cgroup/misc.c             |  4 ++
 9 files changed, 152 insertions(+), 3 deletions(-)

-- 
2.43.0


^ permalink raw reply

* Re: [PATCH v2 03/17] x86/virt/tdx: Detect if the extensions initialization is required
From: Xiaoyao Li @ 2026-07-24  8:44 UTC (permalink / raw)
  To: Xu Yilun, Chao Gao
  Cc: x86, kvm, linux-coco, linux-kernel, djbw, kas, rick.p.edgecombe,
	yilun.xu, sohil.mehta, adrian.hunter, kishen.maloor,
	tony.lindgren, peter.fang, baolu.lu, zhenzhong.duan, dave.hansen,
	dave.hansen, seanjc
In-Reply-To: <akOkGqH6RQEs/RWN@yilunxu-OptiPlex-7050>

On 6/30/2026 7:10 PM, Xu Yilun wrote:
> On Mon, Jun 29, 2026 at 02:33:56PM +0800, Chao Gao wrote:
>>> +static __init int init_tdx_module_extensions(void)
>>> +{
>>> +	struct tdx_sys_info_ext sysinfo_ext;
>>> +	int ret;
>>> +
>>> +	if (!(tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_EXT))
>>> +		return 0;
>>> +
>>> +	ret = get_tdx_sys_info_ext(&sysinfo_ext);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	/* Skip if no feature requires TDX module extensions. */
>>> +	if (!sysinfo_ext.ext_required)
>>> +		return 0;
>>
>> What would happen if the kernel doesn't do this 'ext_required' check
>> and always does the extension initialization?
>>
>> If TDH.EXT.INIT returns success when no extension is configured, we
> 
> No, TDH.EXT.INIT fails when ext_required==false

What is the SEAMCALL return code in this case?

>> could drop this 'ext_required' from this series entirely.


^ permalink raw reply

* Re: [PATCH v2 03/17] x86/virt/tdx: Detect if the extensions initialization is required
From: Xiaoyao Li @ 2026-07-24  8:42 UTC (permalink / raw)
  To: Xu Yilun, x86, kvm, linux-coco, linux-kernel
  Cc: djbw, kas, rick.p.edgecombe, yilun.xu, sohil.mehta, adrian.hunter,
	kishen.maloor, tony.lindgren, peter.fang, baolu.lu,
	zhenzhong.duan, dave.hansen, dave.hansen, seanjc
In-Reply-To: <20260618081355.3253581-4-yilun.xu@linux.intel.com>

On 6/18/2026 4:13 PM, Xu Yilun wrote:
> TDX module extensions support extension SEAMCALLs that are preemptible
> and resumable, unlike normal SEAMCALLs that run to completion while
> monopolizing the CPU. 

This is not true. There are some normal (non-extension) SEAMCALLs also 
are preemptible and resumable. For example,

- TDH.PHYMEM.CACHE.WB
- TDH.SYS.DISABLE
- TDH.MEM.SEPT.ADD with version 1

(There might be more. I didn't check all.)

> This allows for higher-level API constructions,
> so better supports some add-on features that implement higher order
> security protocols.

I think we don't need to explain this? Just that some add-on features 
require the functionalities of TDX module extensions is enough?

> Add infrastructure to initialize TDX module extensions. Introduce the
> initial step of this process by detecting if the extensions are required
> by checking:
> 
>    1. If the extensions are supported via TDX_FEATURES0_EXT.
>    2. If any TDX add-on feature needs the extensions via a boolean
>       metadata field ext_required.
> 
> Currently all metadata fields are read at the very beginning of basic
> TDX initialization and stored in a global var. However, ext_required is
> only valid after the add-on feature configuration, making it
> incompatible with the existing metadata reading method.
> 
> To resolve this lifetime conflict, add a dedicated runtime metadata
> reading interface for the extensions, call it when the extensions
> initialization starts, and leave the field out of the global var. In
> this way, there is no confusion of when the metadata should be read.
> 
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
>   arch/x86/include/asm/tdx.h                  |  1 +
>   arch/x86/include/asm/tdx_global_metadata.h  |  4 ++++
>   arch/x86/virt/vmx/tdx/tdx.c                 | 25 +++++++++++++++++++++
>   arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 14 ++++++++++++
>   4 files changed, 44 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index e5a9cf656c07..5fbf89d5317c 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -35,6 +35,7 @@
>   /* Bit definitions of TDX_FEATURES0 metadata field */
>   #define TDX_FEATURES0_TD_PRESERVING	BIT_ULL(1)
>   #define TDX_FEATURES0_NO_RBP_MOD	BIT_ULL(18)
> +#define TDX_FEATURES0_EXT		BIT_ULL(39)
>   
>   #ifndef __ASSEMBLER__
>   
> diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
> index 41150d546589..83fc657a438e 100644
> --- a/arch/x86/include/asm/tdx_global_metadata.h
> +++ b/arch/x86/include/asm/tdx_global_metadata.h
> @@ -52,4 +52,8 @@ struct tdx_sys_info {
>   	struct tdx_sys_info_td_conf td_conf;
>   };
>   
> +struct tdx_sys_info_ext {
> +	bool ext_required;
> +};
> +
>   #endif
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 92305b5ea90d..6f3596f11d25 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1166,6 +1166,27 @@ static __init int init_tdmrs(struct tdmr_info_list *tdmr_list)
>   	return 0;
>   }
>   
> +static __init int init_tdx_module_extensions(void)
> +{
> +	struct tdx_sys_info_ext sysinfo_ext;
> +	int ret;
> +
> +	if (!(tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_EXT))
> +		return 0;
> +
> +	ret = get_tdx_sys_info_ext(&sysinfo_ext);
> +	if (ret)
> +		return ret;
> +
> +	/* Skip if no feature requires TDX module extensions. */
> +	if (!sysinfo_ext.ext_required)
> +		return 0;

There was the discussion around it in [1]. I agree with the point to 
make kernel code simple. But it requires change/clarification from TDX 
spec, right? So will TDX spec change?

If so, you would need to mention it somewhere.

[1] 
https://lore.kernel.org/all/9c00b87b7b69470ad1e7b1d2788414002b9a1c77.camel@intel.com/

> +	/* TODO: add the extensions enabling steps here */
> +
> +	return 0;
> +}
> +
>   static __init int init_tdx_module(void)
>   {
>   	int ret;
> @@ -1220,6 +1241,10 @@ static __init int init_tdx_module(void)
>   	if (ret)
>   		goto err_reset_pamts;
>   
> +	ret = init_tdx_module_extensions();
> +	if (ret)
> +		goto err_reset_pamts;
> +
>   	pr_info("%lu KB allocated for PAMT\n", tdmrs_count_pamt_kb(&tdx_tdmr_list));
>   
>   out_put_tdxmem:
> diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> index e49c300f23d4..b9e1c011a990 100644
> --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> @@ -131,3 +131,17 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
>   
>   	return ret;
>   }
> +
> +static __init int get_tdx_sys_info_ext(struct tdx_sys_info_ext *sysinfo_ext)
> +{
> +	int ret;
> +	u64 val;
> +
> +	ret = read_sys_metadata_field(0x3100000000000001, &val);
> +	if (ret)
> +		return ret;
> +
> +	sysinfo_ext->ext_required = val;
> +
> +	return 0;
> +}


^ permalink raw reply

* Re: [PATCH v2 02/17] x86/virt/tdx: Configure add-on features on TDX module init and update
From: Xiaoyao Li @ 2026-07-24  8:15 UTC (permalink / raw)
  To: Xu Yilun, x86, kvm, linux-coco, linux-kernel
  Cc: djbw, kas, rick.p.edgecombe, yilun.xu, sohil.mehta, adrian.hunter,
	kishen.maloor, tony.lindgren, peter.fang, baolu.lu,
	zhenzhong.duan, dave.hansen, dave.hansen, seanjc
In-Reply-To: <20260618081355.3253581-3-yilun.xu@linux.intel.com>

On 6/18/2026 4:13 PM, Xu Yilun wrote:
> In addition to basic TDX functionalities, TDX module provides add-on
> features that can be progressively enabled as the kernel supports them.

"add-on features" looks like a new term introduced by this sereis for 
TDX. So what is add-on features? all the features defined in 
TDX_FEATURE0/1? Or only the features needs to be explicitly enabled by 
R9 and R10 of TDH.SYS.CONFIG? ...

> The kernel should explicitly configure these features at boot or
> post-update initialization time. 

... So the answer is the latter. Then why only these bits are add-on 
features while the rest in TDX_FEATURES01 are not?

btw, s/should/needs/ is better?

> Configuring an add-on feature, such as
> TDX Quoting, that uses extension SEAMCALLs is the prerequisite for
> initializing TDX module extensions. 

Though the statement is right, it leads to the impression that the 
reason we are configuring the add-on feature is to initialize the TDX 
module extensions.

However, the truth is kenrel wants to enable/use an add-on feature and 
the add-on feature requires the functionalities provided by some TDX 
module extension. So kernel needs to enable the TDX module extensions.

> TDX Quoting is the target feature to
> enable but defer it for now until full kernel support is in place.
> 
> TDX module extends TDH.SYS.CONFIG and TDH.SYS.UPDATE with new bitmap
> input parameters to specify which add-on features to configure. The
> bitmap uses the same definitions as TDX_FEATURES0.
> 
> For runtime update, Linux applies a policy that no newer features should
> be added after update to avoid disrupting live TDX operations. To adhere
> to this, TDH.SYS.UPDATE must configure the same features as the
> TDH.SYS.CONFIG. Record the kernel required add-on feature bitmap in a
> global var so that both phases can use it.
> 
> TDX module advances the version of TDH.SYS.CONFIG and TDH.SYS.UPDATE for
> the change, so use the latest version (v1) for add-on feature enabling.
> But supporting existing modules which only support v0 is still necessary
> until they are deprecated. In fact, it is unlikely that TDH.SYS.CONFIG
> ever needs to change again and the code would stay in v1. So there is
> little value in worrying about deprecating v0 to save a couple lines of
> code in 5-7 years when these original TDX platforms sunset.
> 
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
>   arch/x86/virt/vmx/tdx/tdx.h |  6 ++++--
>   arch/x86/virt/vmx/tdx/tdx.c | 28 ++++++++++++++++++++++++++--
>   2 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index fbb520704662..a47e872480c7 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -58,9 +58,11 @@
>   #define TDH_PHYMEM_CACHE_WB		40
>   #define TDH_PHYMEM_PAGE_WBINVD		41
>   #define TDH_VP_WR			43
> -#define TDH_SYS_CONFIG			45
> +#define TDH_SYS_CONFIG_V0		45
> +#define TDH_SYS_CONFIG			SEAMCALL_LEAF_VER(TDH_SYS_CONFIG_V0, 1)
>   #define TDH_SYS_SHUTDOWN		52
> -#define TDH_SYS_UPDATE			53
> +#define TDH_SYS_UPDATE_V0		53
> +#define TDH_SYS_UPDATE			SEAMCALL_LEAF_VER(TDH_SYS_UPDATE_V0, 1)
>   #define TDH_SYS_DISABLE			69
>   
>   /* TDX page types */
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 2a03152796e6..92305b5ea90d 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -57,6 +57,7 @@ static struct tdx_module_state tdx_module_state;
>   static u32 tdx_global_keyid __ro_after_init;
>   static u32 tdx_guest_keyid_start __ro_after_init;
>   static u32 tdx_nr_guest_keyids __ro_after_init;
> +static u64 tdx_addon_feature0 __ro_after_init;
>   
>   static DEFINE_IDA(tdx_guest_keyid_pool);
>   
> @@ -1004,9 +1005,18 @@ static __init int construct_tdmrs(struct list_head *tmb_list,
>   	return ret;
>   }
>   
> +static __init void set_tdx_addon_features(void)
> +{
> +	/*
> +	 * To add DICE-based TDX Quoting feature bit in tdx_addon_feature0 when
> +	 * kernel is ready.
> +	 */
> +}
> +
>   static __init int config_tdx_module(struct tdmr_info_list *tdmr_list,
>   				    u64 global_keyid)
>   {
> +	u64 seamcall_fn = TDH_SYS_CONFIG_V0;
>   	struct tdx_module_args args = {};
>   	u64 *tdmr_pa_array;
>   	size_t array_sz;
> @@ -1032,7 +1042,15 @@ static __init int config_tdx_module(struct tdmr_info_list *tdmr_list,
>   	args.rcx = __pa(tdmr_pa_array);
>   	args.rdx = tdmr_list->nr_consumed_tdmrs;
>   	args.r8 = global_keyid;
> -	ret = seamcall_prerr(TDH_SYS_CONFIG, &args);
> +
> +	set_tdx_addon_features();

Maybe move the set_tdx_addon_features() out of config_tdx_module()? how 
about putting it after check_features(). config_tdx_module() looks like 
just a wrapper to invoke TDH.SYS.CONFIG, while the params of it are 
determined outside of it.

Just my feeling.

> +
> +	if (tdx_addon_feature0) {
> +		args.r9 = tdx_addon_feature0;
> +		seamcall_fn = TDH_SYS_CONFIG;
> +	}
> +
> +	ret = seamcall_prerr(seamcall_fn, &args);
>   
>   	/* Free the array as it is not required anymore. */
>   	kfree(tdmr_pa_array);
> @@ -1314,10 +1332,16 @@ int tdx_module_shutdown(void)
>   
>   int tdx_module_run_update(void)
>   {
> +	u64 seamcall_fn = TDH_SYS_UPDATE_V0;
>   	struct tdx_module_args args = {};
>   	int ret;
>   
> -	ret = seamcall_prerr(TDH_SYS_UPDATE, &args);
> +	if (tdx_addon_feature0) {
> +		args.r9 = tdx_addon_feature0;
> +		seamcall_fn = TDH_SYS_UPDATE;
> +	}
> +
> +	ret = seamcall_prerr(seamcall_fn, &args);
>   	if (ret)
>   		return ret;
>   


^ permalink raw reply

* Re: [PATCH v15 12/37] KVM: arm64: CCA: Support the VGIC in realms
From: Kohei Enju @ 2026-07-24  5:40 UTC (permalink / raw)
  To: Steven Price
  Cc: kvm, kvmarm, Catalin Marinas, Marc Zyngier, Will Deacon,
	James Morse, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
	linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
	Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni,
	Gavin Shan, Shanker Donthineni, Alper Gun, Aneesh Kumar K . V,
	Emi Kisanuki, Vishal Annapurve, WeiLin.Chang, Lorenzo Pieralisi
In-Reply-To: <1cd3327c-1eab-4022-9ab1-625ca8641e81@arm.com>

On 07/23 15:46, Steven Price wrote:
> On 23/07/2026 07:56, Kohei Enju wrote:
> > On 07/22 14:31, Steven Price wrote:
> >> On 22/07/2026 09:27, Kohei Enju wrote:
> >>> On 07/15 15:28, Steven Price wrote:
> >>>> The RMM provides emulation of a VGIC to the realm guest. With RMM v2.0
> >>>> the registers are passed in the system registers so this works similar
> >>>> to a normal guest, but kvm_arch_vcpu_put() need reordering to early out,
> >>>> and realm guests don't support GICv2 even if the host does.
> >>>>
> >>>> Signed-off-by: Steven Price <steven.price@arm.com>
> >>>
> >>> Hi Steven,
> >>
> >> Hi Kohei,
> >>
> >>> I've been testing this series and found that when the host CPU doesn't have
> >>> ARM64_HAS_ICH_HCR_EL2_TDIR this series doesn't work as expected.
> >>
> >> Thanks for testing!
> >>
> >>> Since commit 2a28810cbb8b ("KVM: arm64: GICv3: Detect and work around the lack
> >>> of ICV_DIR_EL1 trapping"), when the host CPU doesn't support this feature, KVM
> >>> traps all GIC sysreg accesses in the common group. However, currently trap
> >>> handlers for ICC_{PMR,RPR,CTLR}_EL1 registers are missing [0]. So when Realm
> >>> guests try to access those registers, KVM traps them but just emits the warning
> >>> shown in [1], and Realm guests fail to boot.
> >>>
> >>> As far as I can tell, the CCA requirements don't require the
> >>> ARM64_HAS_ICH_HCR_EL2_TDIR feature. If that's the case, this seems to be a
> >>> problem. Is there any workaround for this issue, or should we implement trap
> >>> handlers for those registers?
> >>
> >> As Marc has already said in his reply, I'm really surprised you have a
> >> CPU which implements CCA but doesn't have ARM64_HAS_ICH_HCR_EL2_TDIR.
> >> Can you give some more details about the platform you are testing on?
> > 
> > Thank you for taking a look, Steve.
> > 
> > Unfortunately, due to the company's policy, I can't share any details
> > about the platform I'm currently testing on. When the time is right,
> > I'll be happy to do so.
> > 
> >>
> >> Ultimately there are two options here - either don't support CCA (so
> >> detect the lack of ARM64_HAS_ICH_HCR_EL2_TDIR and bail out early), or
> >> plumb in the trap handlers - as commit 2a28810cbb8b points out this
> >> isn't something we really want to support if we can avoid it.
> > 
> > Yes, the former makes perfect sense if there would be no systems that
> > supports CCA without TDIR. However, if such systems do exist, I'd be
> > interested in exploring the latter approach.
> > 
> >>
> >> Hence I'm interested to know where this sits between "hacked up test
> >> system" and "production hardware". E.g. if this is an emulator it might
> >> be possible to just enable the CPU feature.
> > 
> > Again, I can't share that right now, but I'd like to share more when the
> > time comes.
> 
> Given Marc's response for now I'm not going to add support for this. 
> "When the time comes" we can revisit whether it makes sense to have 
> upstream support and what that might look like.

I understand. Thank you for clarifying the current stance.

> 
> However, I did ask an AI tool to have a go at implementing this, and it 
> came up with the (very lightly tested) patch below. That might at least 
> give you something you can test. It doesn't meet Marc's request that it
> reuses the existing implementation, and it took the AI a few goes at
> getting something that "works" so I suspect it might have bugs - I
> haven't reviewed the code myself.

Thanks for the patch. That's very helpful, and I'll look into it.

Thanks,
Kohei

> 
> Thanks,
> Steve
> 
> ----8<----
> From 7aaaa5089c05f19bf00df2b651fa4bd7d738384c Mon Sep 17 00:00:00 2001
> From: Steven Price <steven.price@arm.com>
> Date: Thu, 23 Jul 2026 11:49:36 +0100
> Subject: [PATCH] KVM: arm64: CCA: Handle GICv3 CPU interface traps for realms
> 
> On CPUs that lack ICH_HCR_EL2.TDIR, KVM works around the missing DIR
> trap by setting ICH_HCR_EL2.TC and trapping the GICv3 common CPU
> interface registers. Normal guests handle these traps in hyp via the
> VGIC CPU interface emulation, but realm guests exit through the RMM and
> reach KVM's host-side sysreg emulation instead.
> 
> That path only handles ICC_DIR_EL1 and treats the rest of the trapped
> common group as undefined. A realm guest can therefore take an
> unexpected UNDEF when accessing registers such as ICC_PMR_EL1 or
> ICC_CTLR_EL1 on systems that require the TC workaround.
> 
> Handle the GICv3 common CPU interface registers directly from the realm
> sysreg exit path. Reads are satisfied from the saved VGIC state and
> writes update the VGIC shadow state. For PMR and CTLR writes, also
> restore the VMCR/APR state to the live GIC CPU interface before
> re-entering the REC, matching the effect a direct guest sysreg write
> would have had.
> 
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
>  arch/arm64/kvm/rmi-exit.c | 119 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 119 insertions(+)
> 
> diff --git a/arch/arm64/kvm/rmi-exit.c b/arch/arm64/kvm/rmi-exit.c
> index 78d9189fd5ca..0fad8e68d920 100644
> --- a/arch/arm64/kvm/rmi-exit.c
> +++ b/arch/arm64/kvm/rmi-exit.c
> @@ -3,17 +3,25 @@
>   * Copyright (C) 2023-2026 ARM Ltd.
>   */
>  
> +#include <linux/bitfield.h>
> +#include <linux/irqchip/arm-gic-v3.h>
>  #include <linux/kvm_host.h>
>  #include <kvm/arm_hypercalls.h>
>  #include <kvm/arm_psci.h>
>  
>  #include <linux/arm-smccc-rmi.h>
>  #include <asm/kvm_emulate.h>
> +#include <asm/kvm_hyp.h>
>  #include <asm/kvm_rmi.h>
>  #include <asm/kvm_mmu.h>
> +#include <asm/sysreg.h>
> +
> +#include "vgic/vgic.h"
>  
>  typedef int (*exit_handler_fn)(struct kvm_vcpu *vcpu);
>  
> +#define GICV3_IDLE_PRIORITY	0xff
> +
>  static int rec_exit_reason_notimpl(struct kvm_vcpu *vcpu)
>  {
>  	vcpu_err(vcpu, "Unhandled exit reason from realm (ESR: %#llx)\n",
> @@ -48,6 +56,114 @@ static int rec_exit_sync_iabt(struct kvm_vcpu *vcpu)
>  	return -ENXIO;
>  }
>  
> +static int rec_get_gicv3_bpr_min(void)
> +{
> +	return 8 - (FIELD_GET(ICH_VTR_EL2_PREbits,
> +			      kvm_vgic_global_state.ich_vtr_el2) + 1);
> +}
> +
> +static u8 rec_get_gicv3_active_priority(struct kvm_vcpu *vcpu)
> +{
> +	struct vgic_v3_cpu_if *cpuif = &vcpu->arch.vgic_cpu.vgic_v3;
> +	u8 prio = 0;
> +
> +	for (int i = 0; i <= vgic_v3_max_apr_idx(vcpu); i++) {
> +		u32 val = cpuif->vgic_ap0r[i] | cpuif->vgic_ap1r[i];
> +
> +		if (!val) {
> +			prio += 32;
> +			continue;
> +		}
> +
> +		return (prio + __ffs(val)) << rec_get_gicv3_bpr_min();
> +	}
> +
> +	return GICV3_IDLE_PRIORITY;
> +}
> +
> +static void rec_restore_gicv3_vmcr_aprs(struct kvm_vcpu *vcpu)
> +{
> +	preempt_disable();
> +	kvm_call_hyp(__vgic_v3_restore_vmcr_aprs,
> +		     &vcpu->arch.vgic_cpu.vgic_v3);
> +	preempt_enable();
> +}
> +
> +/*
> + * Realm exits are handled after the RMM has returned to the host, so use the
> + * saved VGIC shadow state instead of the hyp-side live sysreg helpers.
> + */
> +static bool rec_exit_gicv3_sys_reg(struct kvm_vcpu *vcpu, bool is_write)
> +{
> +	struct realm_rec *rec = &vcpu->arch.rec;
> +	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> +	struct vgic_vmcr vmcr;
> +	unsigned long esr = kvm_vcpu_get_esr(vcpu);
> +	int rt = kvm_vcpu_sys_get_rt(vcpu);
> +	u32 sysreg = esr_sys64_to_sysreg(esr);
> +	u64 val;
> +
> +	if (!kvm_has_gicv3(vcpu->kvm))
> +		return false;
> +
> +	vgic_get_vmcr(vcpu, &vmcr);
> +
> +	switch (sysreg) {
> +	case SYS_ICC_DIR_EL1:
> +		if (!is_write)
> +			return false;
> +
> +		vgic_v3_deactivate(vcpu, rec->run->exit.gprs[rt]);
> +		return true;
> +	case SYS_ICC_PMR_EL1:
> +		if (is_write) {
> +			vmcr.pmr = FIELD_GET(ICC_PMR_EL1_MASK,
> +					     rec->run->exit.gprs[rt]);
> +			vgic_set_vmcr(vcpu, &vmcr);
> +			rec_restore_gicv3_vmcr_aprs(vcpu);
> +		} else {
> +			rec->run->enter.gprs[rt] = FIELD_PREP(ICC_PMR_EL1_MASK,
> +							      vmcr.pmr);
> +		}
> +
> +		return true;
> +	case SYS_ICC_CTLR_EL1:
> +		if (is_write) {
> +			val = rec->run->exit.gprs[rt];
> +			vmcr.cbpr = FIELD_GET(ICC_CTLR_EL1_CBPR_MASK, val);
> +			vmcr.eoim = FIELD_GET(ICC_CTLR_EL1_EOImode_MASK, val);
> +			vgic_set_vmcr(vcpu, &vmcr);
> +			rec_restore_gicv3_vmcr_aprs(vcpu);
> +		} else {
> +			val  = FIELD_PREP(ICC_CTLR_EL1_PRI_BITS_MASK,
> +					  vgic_cpu->num_pri_bits - 1);
> +			val |= FIELD_PREP(ICC_CTLR_EL1_ID_BITS_MASK,
> +					  vgic_cpu->num_id_bits);
> +			val |= FIELD_PREP(ICC_CTLR_EL1_SEIS_MASK,
> +					  FIELD_GET(ICH_VTR_EL2_SEIS,
> +						    kvm_vgic_global_state.ich_vtr_el2));
> +			val |= FIELD_PREP(ICC_CTLR_EL1_A3V_MASK,
> +					  FIELD_GET(ICH_VTR_EL2_A3V,
> +						    kvm_vgic_global_state.ich_vtr_el2));
> +			val |= FIELD_PREP(ICC_CTLR_EL1_CBPR_MASK,
> +					  vmcr.cbpr);
> +			val |= FIELD_PREP(ICC_CTLR_EL1_EOImode_MASK,
> +					  vmcr.eoim);
> +			rec->run->enter.gprs[rt] = val;
> +		}
> +
> +		return true;
> +	case SYS_ICC_RPR_EL1:
> +		if (is_write)
> +			return false;
> +
> +		rec->run->enter.gprs[rt] = rec_get_gicv3_active_priority(vcpu);
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  static int rec_exit_sys_reg(struct kvm_vcpu *vcpu)
>  {
>  	struct realm_rec *rec = &vcpu->arch.rec;
> @@ -59,6 +175,9 @@ static int rec_exit_sys_reg(struct kvm_vcpu *vcpu)
>  	if (is_write)
>  		vcpu_set_reg(vcpu, rt, rec->run->exit.gprs[rt]);
>  
> +	if (rec_exit_gicv3_sys_reg(vcpu, is_write))
> +		return 1;
> +
>  	ret = kvm_handle_sys_reg(vcpu);
>  	if (!is_write)
>  		rec->run->enter.gprs[rt] = vcpu_get_reg(vcpu, rt);
> -- 
> 2.43.0
> 
> 
> 

^ permalink raw reply

* Re: [PATCH v3] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Xu Yilun @ 2026-07-24  3:14 UTC (permalink / raw)
  To: Edgecombe, Rick P
  Cc: linux-kernel@vger.kernel.org, x86@kernel.org, Gao, Chao,
	Xu, Yilun, Hansen, Dave, dave.hansen@linux.intel.com,
	kas@kernel.org, Li, Xiaoyao, djbw@kernel.org,
	linux-coco@lists.linux.dev, Fang, Peter
In-Reply-To: <d3d33a22f57883f937f28919b96a5fdccdc080af.camel@intel.com>

> >  static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> >  						  struct tdx_module_args *args)
> >  {
> > +	/*
> > +	 * fn contains base leaf number for TDX module calls and P-SEAMLDR
> > +	 * calls. Other fields in SEAMCALL leaf like leaf ABI version number
> > +	 * are in struct tdx_module_args.
> > +	 */
> > +	BUILD_BUG_ON(fn & ~(SEAMCALL_LEAF_MASK | SEAMCALL_SEAMLDR_MASK));
> > +
> 
> I guess this works because the __always_inline is enough for the compiler to
> trace fn to a compile time constant up the callchain. It seems somewhat compiler
> sensitive though. Did you convince yourself somehow that some compiler will not
> complain?

I see the previous talk [1][2], that existing __seamcall*() rely on the
direct call, though there is no guarantee. We accept the assumption
until there is compiler issue. So I think this doesn't add more problem.

[1] https://lore.kernel.org/lkml/20250605145914.GW39944@noisy.programming.kicks-ass.net/
[2] 0b3bc018e86a ("x86/virt/tdx: Avoid indirect calls to TDX assembly function")

^ permalink raw reply

* Re: [PATCH v10 3/6] x86/sev: Disable CPU hotplug while SNP is active
From: Kalra, Ashish @ 2026-07-23 22:19 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: tglx, mingo, dave.hansen, x86, hpa, seanjc, peterz,
	thomas.lendacky, herbert, davem, ardb, pbonzini, aik,
	Michael.Roth, KPrateek.Nayak, Tycho.Andersen, Nathan.Fontenot,
	ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
	pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
	linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <b1a5e1c9-481d-4c70-a058-de4718618891@amd.com>


On 7/23/2026 3:39 PM, Kalra, Ashish wrote:
> 
> On 7/23/2026 3:00 PM, Borislav Petkov wrote:
>> On Thu, Jul 23, 2026 at 02:44:47PM -0500, Kalra, Ashish wrote:
>>> Actually, i will have to remove cpus_read_lock()/unlock() across the whole
>>> patch series and also in the original code.
>>
>> You need to do that in the same patch which adds the hotplug disable so that
>> there's no hole in hotplug handling when bisecting.
>>
>> Make sure to explain why you're doing that in the commit message.
>>
>> Thx.
>>
> 
> This is the list of the consolidated changes:
> 
> Patch 2:  "Initialize RMPOPT configuration MSRs":
>   - Renamed rmpopt_cleanup() to snp_cleanup_rmpopt() to match snp_setup_rmpopt().
>   - Simplified the 2 TB RMPOPT_BASE comment.
>   - Replaced the misleading primary-thread comment with "…only one thread per core needs to set up the RMPOPT_BASE MSR. All
>   primary threads are online, otherwise SNP would not have been enabled."
> 
>  
> Patch 3:  "Disable CPU hotplug while SNP is active":
>   - Used your commit message.
>   - Since CPU hotplug is disabled while SNP is active, the online mask is stable, so I dropped the redundant
>   cpus_read_lock()/cpus_read_unlock() in snp_prepare() and the cpus_read_lock() in snp_setup_rmpopt()/snp_cleanup_rmpopt() —
>   all in this same patch so there's no hole in hotplug handling when bisecting, and explained why in the commit message.
>   - Dropped the superfluous "SnpEn was never set" comment and rewrote the snp_shutdown() re-enable comment.
> 
> The RMPOPT scan in rmpopt_work_handler() (added in 4/6) is likewise lock-free — it runs only while SNP is active, i.e.
> after the hotplug-disable patch, so it never needed cpus_read_lock() in the first place.
> 

Actually for v11, I am reordering the patches so "Disable CPU hotplug" comes before "Initialize RMPOPT configuration MSRs." 

snp_prepare()'s cpus_read_lock() is then dropped in the hotplug-disable patch itself (bisect stays clean, explained in the commit
message), and snp_setup_rmpopt()/snp_cleanup_rmpopt() — introduced afterwards — never take it in the first place, same as
rmpopt_work_handler(). So the removal is implied by ordering rather than an add-then-remove.

Also folded in: renamed rmpopt_cleanup() to snp_cleanup_rmpopt(), simplified the RMPOPT_BASE comment and the primary-thread
comment and used your reworded hotplug commit message.

Thanks,  
Ashish

^ permalink raw reply

* Re: [PATCH v10 3/6] x86/sev: Disable CPU hotplug while SNP is active
From: Kalra, Ashish @ 2026-07-23 20:39 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: tglx, mingo, dave.hansen, x86, hpa, seanjc, peterz,
	thomas.lendacky, herbert, davem, ardb, pbonzini, aik,
	Michael.Roth, KPrateek.Nayak, Tycho.Andersen, Nathan.Fontenot,
	ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
	pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
	linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <20260723200003.GEamJyw2W1nMPAUdHc@fat_crate.local>


On 7/23/2026 3:00 PM, Borislav Petkov wrote:
> On Thu, Jul 23, 2026 at 02:44:47PM -0500, Kalra, Ashish wrote:
>> Actually, i will have to remove cpus_read_lock()/unlock() across the whole
>> patch series and also in the original code.
> 
> You need to do that in the same patch which adds the hotplug disable so that
> there's no hole in hotplug handling when bisecting.
> 
> Make sure to explain why you're doing that in the commit message.
> 
> Thx.
> 

This is the list of the consolidated changes:

Patch 2:  "Initialize RMPOPT configuration MSRs":
  - Renamed rmpopt_cleanup() to snp_cleanup_rmpopt() to match snp_setup_rmpopt().
  - Simplified the 2 TB RMPOPT_BASE comment.
  - Replaced the misleading primary-thread comment with "…only one thread per core needs to set up the RMPOPT_BASE MSR. All
  primary threads are online, otherwise SNP would not have been enabled."

 
Patch 3:  "Disable CPU hotplug while SNP is active":
  - Used your commit message.
  - Since CPU hotplug is disabled while SNP is active, the online mask is stable, so I dropped the redundant
  cpus_read_lock()/cpus_read_unlock() in snp_prepare() and the cpus_read_lock() in snp_setup_rmpopt()/snp_cleanup_rmpopt() —
  all in this same patch so there's no hole in hotplug handling when bisecting, and explained why in the commit message.
  - Dropped the superfluous "SnpEn was never set" comment and rewrote the snp_shutdown() re-enable comment.

The RMPOPT scan in rmpopt_work_handler() (added in 4/6) is likewise lock-free — it runs only while SNP is active, i.e.
after the hotplug-disable patch, so it never needed cpus_read_lock() in the first place.

Thanks,
Ashish

^ permalink raw reply

* Re: [PATCH v10 3/6] x86/sev: Disable CPU hotplug while SNP is active
From: Borislav Petkov @ 2026-07-23 20:00 UTC (permalink / raw)
  To: Kalra, Ashish
  Cc: tglx, mingo, dave.hansen, x86, hpa, seanjc, peterz,
	thomas.lendacky, herbert, davem, ardb, pbonzini, aik,
	Michael.Roth, KPrateek.Nayak, Tycho.Andersen, Nathan.Fontenot,
	ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
	pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
	linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <404433af-b908-4849-a8b6-1c1ee9d33d8e@amd.com>

On Thu, Jul 23, 2026 at 02:44:47PM -0500, Kalra, Ashish wrote:
> Actually, i will have to remove cpus_read_lock()/unlock() across the whole
> patch series and also in the original code.

You need to do that in the same patch which adds the hotplug disable so that
there's no hole in hotplug handling when bisecting.

Make sure to explain why you're doing that in the commit message.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply

* Re: [PATCH v10 3/6] x86/sev: Disable CPU hotplug while SNP is active
From: Kalra, Ashish @ 2026-07-23 19:44 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: tglx, mingo, dave.hansen, x86, hpa, seanjc, peterz,
	thomas.lendacky, herbert, davem, ardb, pbonzini, aik,
	Michael.Roth, KPrateek.Nayak, Tycho.Andersen, Nathan.Fontenot,
	ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
	pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
	linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <20260723185323.GAamJjI2HoM1gZmExw@fat_crate.local>


On 7/23/2026 1:53 PM, Borislav Petkov wrote:
> On Tue, Jun 30, 2026 at 06:11:03PM +0000, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> While SNP is active, every memory write is checked against the RMP to
>> protect SEV-SNP guest memory.  A core performs these RMP checks only once
>> ...
> 
> Use this commit message for your next revision. The idea is to split it into
> smaller paragraphs for easier parsing, do simple formulations and not talk
> about future patches because git history is not always linear:
> 
> "While SNP is active, every memory write is checked against the RMP to
> protect SEV-SNP guest memory.  A core performs these RMP checks only once SNP
> has been initialized via SNP_INIT and the SNP-enable bit in SYSCFG is set on
> that core; the firmware requires the SNP-enable bit to be set on every present
> CPU before SNP initialization.
> 
> A core that is not SNP-enabled and not SNP-initialized performs no RMP checks
> at all, so there is no valid configuration with SNP active and any CPU exempt
> from RMP checks.
> 
> The firmware determines which CPUs are present from the processor and the
> BIOS/UEFI configuration (e.g. SMT disabled in the BIOS) and enumerates them at
> SNP init; it is not aware of the OS bringing CPUs online or offline
> afterwards.
> 
> SNP_INIT fails unless SnpEn is set on all CPUs, so a CPU that is offline when
> SNP_INIT is issued, does not have SnpEn set, SNP_INIT fails, and there can be
> no SNP guest memory.  OS CPU hotplug can thus diverge from the firmware's
> expectations and break SNP.
> 
> Tie CPU hotplug to the SNP-enable bit: disable it in snp_prepare() before
> SNP is enabled, and re-enable it in snp_shutdown() once the firmware has
> disabled SNP.
> 
> If snp_prepare() fails before enabling SNP it re-enables hotplug itself; once
> SNP is enabled hotplug stays disabled, including across a failed SNP_INIT and
> across the legacy SNP_SHUTDOWN_EX path, both of which leave SNP enabled.
> 
> A kexec target that boots with SNP already enabled, disables hotplug once in
> snp_rmptable_init(), since snp_prepare() bails when SNP is already enabled."
> 

Sure.

>>
>> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> ---
>>  arch/x86/virt/svm/sev.c | 31 +++++++++++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index dab6e1c290bc..04a58ac4339c 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
>> @@ -535,6 +535,15 @@ int snp_prepare(void)
>>  
>>  	clear_rmp();
>>  
>> +	/*
>> +	 * Disable CPU hotplug before enabling SNP, so no CPU can come online
>> +	 * without SnpEn while SNP is enabled; it is re-enabled in snp_shutdown()
>> +	 * once SNP is disabled.  Must be before cpus_read_lock():
>> +	 * cpu_hotplug_disable() takes cpu_add_remove_lock, which nests above
>> +	 * cpu_hotplug_lock.
>> +	 */
>> +	cpu_hotplug_disable();
>> +
>>  	cpus_read_lock();
> 
> What's the point of grabbing the hotplug lock if you just disabled hotplug?
> 
>>  	if (!cpumask_equal(cpu_online_mask, cpu_present_mask)) {
>> @@ -560,6 +569,10 @@ int snp_prepare(void)
>>  unlock:
>>  	cpus_read_unlock();
> 
> ditto.
> 

This is part of the base code, but i will have to modify it now that hotplug is explicitly disabled.

Actually, i will have to remove cpus_read_lock()/unlock() across the whole patch series and also in
the original code.

Thanks,
Ashish

^ permalink raw reply

* Re: [PATCH v10 3/6] x86/sev: Disable CPU hotplug while SNP is active
From: Borislav Petkov @ 2026-07-23 18:53 UTC (permalink / raw)
  To: Ashish Kalra
  Cc: tglx, mingo, dave.hansen, x86, hpa, seanjc, peterz,
	thomas.lendacky, herbert, davem, ardb, pbonzini, aik,
	Michael.Roth, KPrateek.Nayak, Tycho.Andersen, Nathan.Fontenot,
	ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
	pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
	linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <205a5259f9fd353dc0ca6b00565c8175a96768c7.1782841284.git.ashish.kalra@amd.com>

On Tue, Jun 30, 2026 at 06:11:03PM +0000, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> While SNP is active, every memory write is checked against the RMP to
> protect SEV-SNP guest memory.  A core performs these RMP checks only once
> ...

Use this commit message for your next revision. The idea is to split it into
smaller paragraphs for easier parsing, do simple formulations and not talk
about future patches because git history is not always linear:

"While SNP is active, every memory write is checked against the RMP to
protect SEV-SNP guest memory.  A core performs these RMP checks only once SNP
has been initialized via SNP_INIT and the SNP-enable bit in SYSCFG is set on
that core; the firmware requires the SNP-enable bit to be set on every present
CPU before SNP initialization.

A core that is not SNP-enabled and not SNP-initialized performs no RMP checks
at all, so there is no valid configuration with SNP active and any CPU exempt
from RMP checks.

The firmware determines which CPUs are present from the processor and the
BIOS/UEFI configuration (e.g. SMT disabled in the BIOS) and enumerates them at
SNP init; it is not aware of the OS bringing CPUs online or offline
afterwards.

SNP_INIT fails unless SnpEn is set on all CPUs, so a CPU that is offline when
SNP_INIT is issued, does not have SnpEn set, SNP_INIT fails, and there can be
no SNP guest memory.  OS CPU hotplug can thus diverge from the firmware's
expectations and break SNP.

Tie CPU hotplug to the SNP-enable bit: disable it in snp_prepare() before
SNP is enabled, and re-enable it in snp_shutdown() once the firmware has
disabled SNP.

If snp_prepare() fails before enabling SNP it re-enables hotplug itself; once
SNP is enabled hotplug stays disabled, including across a failed SNP_INIT and
across the legacy SNP_SHUTDOWN_EX path, both of which leave SNP enabled.

A kexec target that boots with SNP already enabled, disables hotplug once in
snp_rmptable_init(), since snp_prepare() bails when SNP is already enabled."

> 
> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  arch/x86/virt/svm/sev.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index dab6e1c290bc..04a58ac4339c 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -535,6 +535,15 @@ int snp_prepare(void)
>  
>  	clear_rmp();
>  
> +	/*
> +	 * Disable CPU hotplug before enabling SNP, so no CPU can come online
> +	 * without SnpEn while SNP is enabled; it is re-enabled in snp_shutdown()
> +	 * once SNP is disabled.  Must be before cpus_read_lock():
> +	 * cpu_hotplug_disable() takes cpu_add_remove_lock, which nests above
> +	 * cpu_hotplug_lock.
> +	 */
> +	cpu_hotplug_disable();
> +
>  	cpus_read_lock();

What's the point of grabbing the hotplug lock if you just disabled hotplug?

>  	if (!cpumask_equal(cpu_online_mask, cpu_present_mask)) {
> @@ -560,6 +569,10 @@ int snp_prepare(void)
>  unlock:
>  	cpus_read_unlock();

ditto.

>  
> +	/* Re-enable CPU hotplug; SnpEn was never set. */


^ Superfluous comment.

> +	if (ret)
> +		cpu_hotplug_enable();
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
> @@ -587,6 +600,13 @@ void snp_shutdown(void)
>  
>  	rmpopt_cleanup();
>  
> +	/*
> +	 * Re-enable CPU hotplug now that the firmware has disabled SNP; CPU
> +	 * hotplug is not re-enabled for a legacy SNP shutdown.  After
> +	 * rmpopt_cleanup() so RMPOPT_BASE is cleared with hotplug still disabled.
> +	 */

I can't parse that comment.

> +	cpu_hotplug_enable();
> +
>  	clear_rmp();
>  	on_each_cpu(mfd_reconfigure, NULL, 1);
>  }

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply

* Re: [PATCH v10 2/6] x86/sev: Initialize RMPOPT configuration MSRs
From: Borislav Petkov @ 2026-07-23 18:32 UTC (permalink / raw)
  To: Kalra, Ashish
  Cc: K Prateek Nayak, tglx, mingo, dave.hansen, x86, hpa, seanjc,
	peterz, thomas.lendacky, herbert, davem, ardb, pbonzini, aik,
	Michael.Roth, Tycho.Andersen, Nathan.Fontenot, ackerleytng,
	jackyli, pgonda, rientjes, jacobhxu, xin, pawan.kumar.gupta,
	babu.moger, dyoung, nikunj, john.allen, darwi, linux-kernel,
	linux-crypto, kvm, linux-coco
In-Reply-To: <5460b946-b511-4060-8951-264d17a2b21b@amd.com>

On Thu, Jul 23, 2026 at 01:50:48AM -0500, Kalra, Ashish wrote:
> > What happens if you boot with a subset of cores, the boot flow enables RMPOPT
> > and then you online the rest?
> > 
> > Have we tried that?
> > 
>  It can't happen here. snp_prepare() does the following, in order:

Then the comment in the code is greatly misleading:

        /*
         * The RMPOPT_BASE MSR is per-core, so only one thread per core needs
         * to set up the RMPOPT_BASE MSR.
         * 
         * Note: only online primary threads are included.  If a core's
         * primary thread is offline, that core is not covered.  CPU hotplug
         * is not currently supported with SNP enabled.
         */     
        scoped_guard(cpus_read_lock) {
                for_each_online_cpu(cpu)
                        if (topology_is_primary_thread(cpu))

I'd expect to read there:

        /*
         * The RMPOPT_BASE MSR is per-core, so only one thread per core needs
         * to set up the RMPOPT_BASE MSR. All primary threads are online,
	 * otherwise SNP would not have been enabled.
         */

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply

* Re: [PATCH v3] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Edgecombe, Rick P @ 2026-07-23 18:13 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org, yilun.xu@linux.intel.com,
	x86@kernel.org
  Cc: Gao, Chao, Xu, Yilun, Hansen, Dave, dave.hansen@linux.intel.com,
	kas@kernel.org, Li, Xiaoyao, djbw@kernel.org,
	linux-coco@lists.linux.dev, Fang, Peter
In-Reply-To: <20260722084634.131020-1-yilun.xu@linux.intel.com>

On Wed, 2026-07-22 at 16:46 +0800, Xu Yilun wrote:
> SEAMCALL invokes TDX module functions using a function number and
> parameters. To extend the functionality of existing SEAMCALLs while
> keeping backward compatibility, TDX adds more numbered SEAMCALLs of the
> same family. This is just like syscalls, except that TDX defines a
> specific function number encoding pattern: a base function number and a
> version together encode the full function number.
> 
> One concern is the compatibility with older TDX modules which don't
> recognize the new SEAMCALLs. The kernel should decide which SEAMCALL
> version to use at runtime, selecting the minimum version number for the
> required functionality. It can't overwrite the function number with a
> new value at compile time.
> 
> Another concern is the obscure usage of the 'fn' parameter for SEAMCALL
> wrappers. An existing caller for TDH.VP.INIT packs the version into the
> 'fn' to match the low-level TDX ABI RAX layout. The RAX layout
> interprets some bits differently, such as INTERRUPT_MODE, SEAMLDR flag,
> which are not a good fit for the function number definition.
> 
> Refactor the SEAMCALL wrappers to allow for version selection. Add a
> version field in struct tdx_module_args [1], so that most existing
> callers get a default "version == 0" behavior without code churn, while
> callers requiring extended functionalities can specify the version
> descriptively. Enforce the 'fn' parameter as the base function number.
> Encode the base function number and tdx_module_args.version in RAX
> before invoking the SEAMCALL instruction in assembly code.
> 
> Link: https://lore.kernel.org/kvm/4f4b0f29-424b-45ed-8cfd-c77da2ea390f@intel.com/ # [1]
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> The main concern in v2 is: should we make 'fn' exactly match the RAX
> layout of TDX module ABI?
> 
> Kiryl's point is that SEAMCALL wrappers are the representation of the
> ABI functions, so make the 'fn' match the RAX and struct tdx_module_args
> match the other CPU registers, compose base leaf number & version into
> 'fn'. This saves callers from digging into low-level code to understand
> what happens.
> 
> Dave's point is that the RAX layout is logically unfriendly, we want
> SEAMCALL wrappers to be the logically sound software APIs to the rest of
> the system. So 'fn' only contains the base leaf number, version and
> other RAX fields are in struct tdx_module_args.
> 
> While both options have merits, I chose the latter from a
> software-centric perspective. I sent v3 to invite more discussion.

Yea, I think it's good enough. Let's keep the option to revisit it if needed,
when we get to shrinking the layers of seamcall wrappers. One question below...

> 
> Change in v3:
> - Move the RAX encoding in assembly code.
> - Compile-time check that fn only contain base leaf number bits
> - Add the full description of SEAMCALL leaf according to TDX module SPEC
> - Fix the description of struct tdx_module_args.
> 
> v2: https://lore.kernel.org/all/20260708170330.83850-1-yilun.xu@linux.intel.com/
> - Drop the C wrapper __seamcall_encode_fn()
> - Rewrite the first paragraph of the changelog
> - Make change log concise
> - Add a link to the thread where this was suggested
> - Move alternative schemes description below the separator
> 
> v1: https://lore.kernel.org/all/20260702144614.59464-1-yilun.xu@linux.intel.com/
> ---
>  arch/x86/include/asm/shared/tdx.h         | 10 +++++++---
>  arch/x86/virt/vmx/tdx/seamcall_internal.h | 19 +++++++++++++++++++
>  arch/x86/virt/vmx/tdx/tdx.h               |  8 --------
>  arch/x86/virt/vmx/tdx/tdxcall.S           | 10 ++++++++--
>  arch/x86/kernel/asm-offsets.c             |  1 +
>  arch/x86/virt/vmx/tdx/tdx.c               |  5 +++--
>  6 files changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> index f20e91d7ac35..67490c330d91 100644
> --- a/arch/x86/include/asm/shared/tdx.h
> +++ b/arch/x86/include/asm/shared/tdx.h
> @@ -122,9 +122,11 @@
>  #include <linux/compiler_attributes.h>
>  
>  /*
> - * Used in __tdcall*() to gather the input/output registers' values of the
> - * TDCALL instruction when requesting services from the TDX module. This is a
> - * software only structure and not part of the TDX module/VMM ABI
> + * Used in __tdcall*() to gather the input/output parameters of the TDCALL
> + * instruction when requesting services from the TDX module. 'version' is
> + * encoded in rax along with the Leaf number. Others are raw input/output
> + * registers' values. This is a software only structure and not part of the TDX
> + * module/VMM ABI.
>   */
>  struct tdx_module_args {
>  	/* callee-clobbered */
> @@ -143,6 +145,8 @@ struct tdx_module_args {
>  	u64 rbx;
>  	u64 rdi;
>  	u64 rsi;
> +	/* Leaf ABI version, encoded in rax */
> +	u8  version;
>  };
>  
>  /* Used to communicate with the TDX module */
> diff --git a/arch/x86/virt/vmx/tdx/seamcall_internal.h b/arch/x86/virt/vmx/tdx/seamcall_internal.h
> index be5f446467df..c586ddad072f 100644
> --- a/arch/x86/virt/vmx/tdx/seamcall_internal.h
> +++ b/arch/x86/virt/vmx/tdx/seamcall_internal.h
> @@ -11,6 +11,7 @@
>  #ifndef _X86_VIRT_SEAMCALL_INTERNAL_H
>  #define _X86_VIRT_SEAMCALL_INTERNAL_H
>  
> +#include <linux/bitfield.h>
>  #include <linux/printk.h>
>  #include <linux/types.h>
>  #include <asm/archrandom.h>
> @@ -23,9 +24,27 @@ u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args);
>  
>  typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args);
>  
> +/*
> + * SEAMCALL leaf:
> + *
> + * Bit 15:0	Leaf number
> + * Bit 23:16	Leaf ABI version number
> + * Bit 24	Pending interrupts detection mode
> + * Bit 63	1 for P-SEAMLDR leaf, 0 for TDX module leaf
> + */
> +#define SEAMCALL_LEAF_MASK		GENMASK_U64(15, 0)
> +#define SEAMCALL_SEAMLDR_MASK		BIT_U64(63)
> +
>  static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
>  						  struct tdx_module_args *args)
>  {
> +	/*
> +	 * fn contains base leaf number for TDX module calls and P-SEAMLDR
> +	 * calls. Other fields in SEAMCALL leaf like leaf ABI version number
> +	 * are in struct tdx_module_args.
> +	 */
> +	BUILD_BUG_ON(fn & ~(SEAMCALL_LEAF_MASK | SEAMCALL_SEAMLDR_MASK));
> +

I guess this works because the __always_inline is enough for the compiler to
trace fn to a compile time constant up the callchain. It seems somewhat compiler
sensitive though. Did you convince yourself somehow that some compiler will not
complain?

>  	lockdep_assert_preemption_disabled();
>  
>  	/*
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index bdfd0e1e337a..63e3acfb5d0c 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -50,14 +50,6 @@
>  #define TDH_SYS_UPDATE			53
>  #define TDH_SYS_DISABLE			69
>  
> -/*
> - * SEAMCALL leaf:
> - *
> - * Bit 15:0	Leaf number
> - * Bit 23:16	Version number
> - */
> -#define TDX_VERSION_SHIFT		16
> -
>  /* TDX page types */
>  #define	PT_NDA		0x0
>  #define	PT_RSVD		0x1
> diff --git a/arch/x86/virt/vmx/tdx/tdxcall.S b/arch/x86/virt/vmx/tdx/tdxcall.S
> index 016a2a1ec1d6..a01542c02c36 100644
> --- a/arch/x86/virt/vmx/tdx/tdxcall.S
> +++ b/arch/x86/virt/vmx/tdx/tdxcall.S
> @@ -45,8 +45,14 @@
>  .macro TDX_MODULE_CALL host:req ret=0 saved=0
>  	FRAME_BEGIN
>  
> -	/* Move Leaf ID to RAX */
> -	mov %rdi, %rax
> +	/* Leaf ABI version -> RAX[23:16]. Zero rest of RAX. */
> +	movzbl	TDX_MODULE_version(%rsi), %eax
> +	shl	$16, %eax
> +	/*
> +	 * Combine base leaf number arg and leaf ABI version into RAX, they
> +	 * don't overlap.
> +	 */
> +	or	%rdi, %rax
>  
>  	/* Move other input regs from 'struct tdx_module_args' */
>  	movq	TDX_MODULE_rcx(%rsi), %rcx
> diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
> index 081816888f7a..b3c00ff4d819 100644
> --- a/arch/x86/kernel/asm-offsets.c
> +++ b/arch/x86/kernel/asm-offsets.c
> @@ -95,6 +95,7 @@ static void __used common(void)
>  	OFFSET(TDX_MODULE_rbx, tdx_module_args, rbx);
>  	OFFSET(TDX_MODULE_rdi, tdx_module_args, rdi);
>  	OFFSET(TDX_MODULE_rsi, tdx_module_args, rsi);
> +	OFFSET(TDX_MODULE_version, tdx_module_args, version);
>  
>  	BLANK();
>  	OFFSET(BP_scratch, boot_params, scratch);
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 42df8ea464c4..7a89e29b118c 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1910,10 +1910,11 @@ u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid)
>  		.rcx = vp->tdvpr_pa,
>  		.rdx = initial_rcx,
>  		.r8 = x2apicid,
> +		/* apicid requires version == 1. */
> +		.version = 1,
>  	};
>  
> -	/* apicid requires version == 1. */
> -	return seamcall(TDH_VP_INIT | (1ULL << TDX_VERSION_SHIFT), &args);
> +	return seamcall(TDH_VP_INIT, &args);
>  }
>  EXPORT_SYMBOL_FOR_KVM(tdh_vp_init);
>  


^ permalink raw reply

* Re: [PATCH v7 08/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory
From: Sean Christopherson @ 2026-07-23 15:34 UTC (permalink / raw)
  To: Yan Zhao
  Cc: Rick P Edgecombe, sashiko-reviews@lists.linux.dev,
	kvm@vger.kernel.org, linux-coco@lists.linux.dev, Kai Huang,
	Dave Hansen, tony.lindgren@linux.intel.com, Binbin Wu,
	kas@kernel.org, mingo@redhat.com, linux-kernel@vger.kernel.org,
	pbonzini@redhat.com, nik.borisov@suse.com,
	linux-doc@vger.kernel.org, hpa@zytor.com, tglx@kernel.org,
	Vishal Annapurve, bp@alien8.de, Chao Gao, x86@kernel.org
In-Reply-To: <amG2rkuE5WqldZxM@yzhao56-desk.sh.intel.com>

On Thu, Jul 23, 2026, Yan Zhao wrote:
> On Wed, Jul 22, 2026 at 08:12:20AM -0700, Sean Christopherson wrote:
> > On Mon, Jul 20, 2026, Rick P Edgecombe wrote:
> > > On Sat, 2026-07-18 at 06:10 +0000, sashiko-bot@kernel.org wrote:
> > > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > > > - [High] Infinite kernel loop in `kvm_tdp_mmu_map_private_pfn` due to permanent PAMT cache depletion on transient TDX module contention.
> > > > --
> > > 
> > > Our internal Sashiko found this too. It's a false positive as a real bug.
> > > 
> > > Today kvm_tdp_mmu_map_private_pfn() is only called tdx_gmem_post_populate()
> > > during TD setup. It holds the heavyweight tdx_vm_state_guard which grabs vm-
> > > >lock, kvm->slots_lock, and all vcpu->mutex. So there should be no contention
> > > possible.
> > > 
> > > Any potential confusion is not new either, because a similar thing could happen
> > > with the external page tables.
> > > 
> > > But Yan and I were discussing that it would be a good cleanup to fix this anyway
> > > because the reason it is not a functional issue is not clear from the code. For
> > > improved readability (and quieter sashiko reports) the topup can happen inside
> > > the retry loop. Either by moving the retry loop or moving the topup.
> > 
> > Hmm, yeah, I think I agree.  Super duper technically, that's a fix for an existing
> > flaw.  Because very, very, VERY theoretically, the cache of page table pages could
> > be exhausted.  E.g. if some other task managed to free non-leaf page tables while
> > mmu_lock was dropped, thus forcing kvm_tdp_mmu_map_private_pfn() to allocate from
> > its cache over and over.  In practice, that's likely impossible thanks to holding
> > slots_lock, but given that (a) retry should be rare and (b) kvm_mmu_topup_memory_cache()
> > is basically free if no work needs to be done, I don't see any reason to do topup
> > outside of the retry loop.
> > 
> > The other thing we should address is the call to kvm_mmu_reload().  Like cache
> > exhaustion, it *should* be impossible for the root to be invalidated/obsoleted,
> > thanks to holding slots lock.  But as evidenced by the rash of recent shadow MMU
> > bugs, we don't always get things perfect, and lack of defense-in-depth can be
> > *extremely* painful.
> > 
> > I don't think I want to just move kvm_mmu_reload() into the loop, because KVM
> > should provide stronger guarantees with respect to the validity of the loop,
> > versus the population of the caches.  I.e. I want to WARN if the root becomes
> > obsolete after the initial reload.  And more importantly, KVM really should
> > check the validity of the root after acquiring mmu_lock.
> > 
> > We can't simply call is_page_fault_stale(), because mmu_invalidate_retry_gfn()
> > is inherently fuzzy, i.e. could get false positives, even though the pfn provided
> > by guest_memfd is guaranteed to be valid.  E.g. if shared gfns surrounding the
> > to-be-mapped gfn are concurrently invalidated.
> Past you said "No" to checking is_page_fault_stale() [*] :)
> [*] https://lore.kernel.org/all/aPken0s-0MfdSd5o@google.com/

And my reasoning there still stands: there will be false positives, and avoiding
constant false positives requries a weird mmu_seq snapshot.  To be very clear, I
still find the code to be gross, but unfortunately, the onslaught of recent bugs
in scenarios we _thought_ were impossible has made it abundantly clear that, at
least when it's not completely insane, KVM needs to effectively "fail close"
when the impossible happens.  I.e. take action to ensure a bad assumption in KVM
can't be abused to esclate into a DoS or UAF.

> > My only hesitation with manually checking KVM_REQ_MMU_FREE_OBSOLETE_ROOTS is that
> > if more checks/functionality were added to is_page_fault_stale() in the future,
> > then we could end up missing kvm_tdp_mmu_map_private_pfn() and introduce a bug.
> > 
> > Maybe we can have it both ways?  WARN if roots are unexpectedly made obsolete,
> > but fully check is_page_fault_stale() and gracefully handle an obsolete root
> > instead of effectively terminating the guest.
> > 
> > 	do {
> > 		if (signal_pending(current))
> > 			return -EINTR;
> > 
> > 		if (kvm_test_request(KVM_REQ_VM_DEAD, vcpu))
> > 			return -EIO;
> > 
> > 		r = kvm_mmu_reload(vcpu);
> > 		if (r)
> > 			return r;
> > 
> > 		r = mmu_topup_memory_caches(vcpu, false);
> > 		if (r)
> > 			return r;
> > 
> > 		cond_resched();
> > 	
> > 		guard(read_lock)(&kvm->mmu_lock);
> > 
> > 		WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));
> > 
> > 		/*
> > 		 * Snapshot the invalidation sequence counter after acquiring
> > 		 * mmu_lock, as guest_memfd guarantees the validity of the pfn,
> > 		 * i.e. any concurrent invalidations are guaranteed to be
> > 		 * irrelevant.
> > 		 */
> > 		fault.mmu_seq = vcpu->kvm->mmu_invalidate_seq;
> > 		if (is_page_fault_stale(vcpu, &fault))
> > 			continue;
> > 
> > 		r = kvm_tdp_mmu_map(vcpu, &fault);
> > 	} while (r == RET_PF_RETRY);
> > 
> BTW, since kvm_tdp_mmu_map_private_pfn() is currently solely invoked by TDX
> during the TD built phase, and was introduced to avoid redundant
> kvm_gmem_get_pfn() calls in the gmem population path, are there any foreseeable
> future users of kvm_tdp_mmu_map_private_pfn()?

Nope, not that I know of.

> If not, could we simply drop the RETRY loop, given that the locks in the TDX
> path already guarantee that a RETRY error will never occur?"

We could, but I don't think that buys us much, because we still need the
is_page_fault_stale() check, or an equivalent.  At that point, removing the
retry loop is probably a net negative, because it could be the difference between
a race resulting in a WARN but an otherwise usable VM, and an unintentional guest
DoS.

^ permalink raw reply

* Re: [PATCH v15 12/37] KVM: arm64: CCA: Support the VGIC in realms
From: Steven Price @ 2026-07-23 14:46 UTC (permalink / raw)
  To: Kohei Enju
  Cc: kvm, kvmarm, Catalin Marinas, Marc Zyngier, Will Deacon,
	James Morse, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
	linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
	Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni,
	Gavin Shan, Shanker Donthineni, Alper Gun, Aneesh Kumar K . V,
	Emi Kisanuki, Vishal Annapurve, WeiLin.Chang, Lorenzo Pieralisi
In-Reply-To: <amG3KHE4Tdd_Nvuz@FCCLS0092175.localdomain>

On 23/07/2026 07:56, Kohei Enju wrote:
> On 07/22 14:31, Steven Price wrote:
>> On 22/07/2026 09:27, Kohei Enju wrote:
>>> On 07/15 15:28, Steven Price wrote:
>>>> The RMM provides emulation of a VGIC to the realm guest. With RMM v2.0
>>>> the registers are passed in the system registers so this works similar
>>>> to a normal guest, but kvm_arch_vcpu_put() need reordering to early out,
>>>> and realm guests don't support GICv2 even if the host does.
>>>>
>>>> Signed-off-by: Steven Price <steven.price@arm.com>
>>>
>>> Hi Steven,
>>
>> Hi Kohei,
>>
>>> I've been testing this series and found that when the host CPU doesn't have
>>> ARM64_HAS_ICH_HCR_EL2_TDIR this series doesn't work as expected.
>>
>> Thanks for testing!
>>
>>> Since commit 2a28810cbb8b ("KVM: arm64: GICv3: Detect and work around the lack
>>> of ICV_DIR_EL1 trapping"), when the host CPU doesn't support this feature, KVM
>>> traps all GIC sysreg accesses in the common group. However, currently trap
>>> handlers for ICC_{PMR,RPR,CTLR}_EL1 registers are missing [0]. So when Realm
>>> guests try to access those registers, KVM traps them but just emits the warning
>>> shown in [1], and Realm guests fail to boot.
>>>
>>> As far as I can tell, the CCA requirements don't require the
>>> ARM64_HAS_ICH_HCR_EL2_TDIR feature. If that's the case, this seems to be a
>>> problem. Is there any workaround for this issue, or should we implement trap
>>> handlers for those registers?
>>
>> As Marc has already said in his reply, I'm really surprised you have a
>> CPU which implements CCA but doesn't have ARM64_HAS_ICH_HCR_EL2_TDIR.
>> Can you give some more details about the platform you are testing on?
> 
> Thank you for taking a look, Steve.
> 
> Unfortunately, due to the company's policy, I can't share any details
> about the platform I'm currently testing on. When the time is right,
> I'll be happy to do so.
> 
>>
>> Ultimately there are two options here - either don't support CCA (so
>> detect the lack of ARM64_HAS_ICH_HCR_EL2_TDIR and bail out early), or
>> plumb in the trap handlers - as commit 2a28810cbb8b points out this
>> isn't something we really want to support if we can avoid it.
> 
> Yes, the former makes perfect sense if there would be no systems that
> supports CCA without TDIR. However, if such systems do exist, I'd be
> interested in exploring the latter approach.
> 
>>
>> Hence I'm interested to know where this sits between "hacked up test
>> system" and "production hardware". E.g. if this is an emulator it might
>> be possible to just enable the CPU feature.
> 
> Again, I can't share that right now, but I'd like to share more when the
> time comes.

Given Marc's response for now I'm not going to add support for this. 
"When the time comes" we can revisit whether it makes sense to have 
upstream support and what that might look like.

However, I did ask an AI tool to have a go at implementing this, and it 
came up with the (very lightly tested) patch below. That might at least 
give you something you can test. It doesn't meet Marc's request that it
reuses the existing implementation, and it took the AI a few goes at
getting something that "works" so I suspect it might have bugs - I
haven't reviewed the code myself.

Thanks,
Steve

----8<----
From 7aaaa5089c05f19bf00df2b651fa4bd7d738384c Mon Sep 17 00:00:00 2001
From: Steven Price <steven.price@arm.com>
Date: Thu, 23 Jul 2026 11:49:36 +0100
Subject: [PATCH] KVM: arm64: CCA: Handle GICv3 CPU interface traps for realms

On CPUs that lack ICH_HCR_EL2.TDIR, KVM works around the missing DIR
trap by setting ICH_HCR_EL2.TC and trapping the GICv3 common CPU
interface registers. Normal guests handle these traps in hyp via the
VGIC CPU interface emulation, but realm guests exit through the RMM and
reach KVM's host-side sysreg emulation instead.

That path only handles ICC_DIR_EL1 and treats the rest of the trapped
common group as undefined. A realm guest can therefore take an
unexpected UNDEF when accessing registers such as ICC_PMR_EL1 or
ICC_CTLR_EL1 on systems that require the TC workaround.

Handle the GICv3 common CPU interface registers directly from the realm
sysreg exit path. Reads are satisfied from the saved VGIC state and
writes update the VGIC shadow state. For PMR and CTLR writes, also
restore the VMCR/APR state to the live GIC CPU interface before
re-entering the REC, matching the effect a direct guest sysreg write
would have had.

Signed-off-by: Steven Price <steven.price@arm.com>
---
 arch/arm64/kvm/rmi-exit.c | 119 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/arch/arm64/kvm/rmi-exit.c b/arch/arm64/kvm/rmi-exit.c
index 78d9189fd5ca..0fad8e68d920 100644
--- a/arch/arm64/kvm/rmi-exit.c
+++ b/arch/arm64/kvm/rmi-exit.c
@@ -3,17 +3,25 @@
  * Copyright (C) 2023-2026 ARM Ltd.
  */
 
+#include <linux/bitfield.h>
+#include <linux/irqchip/arm-gic-v3.h>
 #include <linux/kvm_host.h>
 #include <kvm/arm_hypercalls.h>
 #include <kvm/arm_psci.h>
 
 #include <linux/arm-smccc-rmi.h>
 #include <asm/kvm_emulate.h>
+#include <asm/kvm_hyp.h>
 #include <asm/kvm_rmi.h>
 #include <asm/kvm_mmu.h>
+#include <asm/sysreg.h>
+
+#include "vgic/vgic.h"
 
 typedef int (*exit_handler_fn)(struct kvm_vcpu *vcpu);
 
+#define GICV3_IDLE_PRIORITY	0xff
+
 static int rec_exit_reason_notimpl(struct kvm_vcpu *vcpu)
 {
 	vcpu_err(vcpu, "Unhandled exit reason from realm (ESR: %#llx)\n",
@@ -48,6 +56,114 @@ static int rec_exit_sync_iabt(struct kvm_vcpu *vcpu)
 	return -ENXIO;
 }
 
+static int rec_get_gicv3_bpr_min(void)
+{
+	return 8 - (FIELD_GET(ICH_VTR_EL2_PREbits,
+			      kvm_vgic_global_state.ich_vtr_el2) + 1);
+}
+
+static u8 rec_get_gicv3_active_priority(struct kvm_vcpu *vcpu)
+{
+	struct vgic_v3_cpu_if *cpuif = &vcpu->arch.vgic_cpu.vgic_v3;
+	u8 prio = 0;
+
+	for (int i = 0; i <= vgic_v3_max_apr_idx(vcpu); i++) {
+		u32 val = cpuif->vgic_ap0r[i] | cpuif->vgic_ap1r[i];
+
+		if (!val) {
+			prio += 32;
+			continue;
+		}
+
+		return (prio + __ffs(val)) << rec_get_gicv3_bpr_min();
+	}
+
+	return GICV3_IDLE_PRIORITY;
+}
+
+static void rec_restore_gicv3_vmcr_aprs(struct kvm_vcpu *vcpu)
+{
+	preempt_disable();
+	kvm_call_hyp(__vgic_v3_restore_vmcr_aprs,
+		     &vcpu->arch.vgic_cpu.vgic_v3);
+	preempt_enable();
+}
+
+/*
+ * Realm exits are handled after the RMM has returned to the host, so use the
+ * saved VGIC shadow state instead of the hyp-side live sysreg helpers.
+ */
+static bool rec_exit_gicv3_sys_reg(struct kvm_vcpu *vcpu, bool is_write)
+{
+	struct realm_rec *rec = &vcpu->arch.rec;
+	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+	struct vgic_vmcr vmcr;
+	unsigned long esr = kvm_vcpu_get_esr(vcpu);
+	int rt = kvm_vcpu_sys_get_rt(vcpu);
+	u32 sysreg = esr_sys64_to_sysreg(esr);
+	u64 val;
+
+	if (!kvm_has_gicv3(vcpu->kvm))
+		return false;
+
+	vgic_get_vmcr(vcpu, &vmcr);
+
+	switch (sysreg) {
+	case SYS_ICC_DIR_EL1:
+		if (!is_write)
+			return false;
+
+		vgic_v3_deactivate(vcpu, rec->run->exit.gprs[rt]);
+		return true;
+	case SYS_ICC_PMR_EL1:
+		if (is_write) {
+			vmcr.pmr = FIELD_GET(ICC_PMR_EL1_MASK,
+					     rec->run->exit.gprs[rt]);
+			vgic_set_vmcr(vcpu, &vmcr);
+			rec_restore_gicv3_vmcr_aprs(vcpu);
+		} else {
+			rec->run->enter.gprs[rt] = FIELD_PREP(ICC_PMR_EL1_MASK,
+							      vmcr.pmr);
+		}
+
+		return true;
+	case SYS_ICC_CTLR_EL1:
+		if (is_write) {
+			val = rec->run->exit.gprs[rt];
+			vmcr.cbpr = FIELD_GET(ICC_CTLR_EL1_CBPR_MASK, val);
+			vmcr.eoim = FIELD_GET(ICC_CTLR_EL1_EOImode_MASK, val);
+			vgic_set_vmcr(vcpu, &vmcr);
+			rec_restore_gicv3_vmcr_aprs(vcpu);
+		} else {
+			val  = FIELD_PREP(ICC_CTLR_EL1_PRI_BITS_MASK,
+					  vgic_cpu->num_pri_bits - 1);
+			val |= FIELD_PREP(ICC_CTLR_EL1_ID_BITS_MASK,
+					  vgic_cpu->num_id_bits);
+			val |= FIELD_PREP(ICC_CTLR_EL1_SEIS_MASK,
+					  FIELD_GET(ICH_VTR_EL2_SEIS,
+						    kvm_vgic_global_state.ich_vtr_el2));
+			val |= FIELD_PREP(ICC_CTLR_EL1_A3V_MASK,
+					  FIELD_GET(ICH_VTR_EL2_A3V,
+						    kvm_vgic_global_state.ich_vtr_el2));
+			val |= FIELD_PREP(ICC_CTLR_EL1_CBPR_MASK,
+					  vmcr.cbpr);
+			val |= FIELD_PREP(ICC_CTLR_EL1_EOImode_MASK,
+					  vmcr.eoim);
+			rec->run->enter.gprs[rt] = val;
+		}
+
+		return true;
+	case SYS_ICC_RPR_EL1:
+		if (is_write)
+			return false;
+
+		rec->run->enter.gprs[rt] = rec_get_gicv3_active_priority(vcpu);
+		return true;
+	default:
+		return false;
+	}
+}
+
 static int rec_exit_sys_reg(struct kvm_vcpu *vcpu)
 {
 	struct realm_rec *rec = &vcpu->arch.rec;
@@ -59,6 +175,9 @@ static int rec_exit_sys_reg(struct kvm_vcpu *vcpu)
 	if (is_write)
 		vcpu_set_reg(vcpu, rt, rec->run->exit.gprs[rt]);
 
+	if (rec_exit_gicv3_sys_reg(vcpu, is_write))
+		return 1;
+
 	ret = kvm_handle_sys_reg(vcpu);
 	if (!is_write)
 		rec->run->enter.gprs[rt] = vcpu_get_reg(vcpu, rt);
-- 
2.43.0



^ permalink raw reply related

* Re: [PATCH v10 2/6] x86/sev: Initialize RMPOPT configuration MSRs
From: K Prateek Nayak @ 2026-07-23 14:29 UTC (permalink / raw)
  To: Kalra, Ashish, Borislav Petkov
  Cc: tglx, mingo, dave.hansen, x86, hpa, seanjc, peterz,
	thomas.lendacky, herbert, davem, ardb, pbonzini, aik,
	Michael.Roth, Tycho.Andersen, Nathan.Fontenot, ackerleytng,
	jackyli, pgonda, rientjes, jacobhxu, xin, pawan.kumar.gupta,
	babu.moger, dyoung, nikunj, john.allen, darwi, linux-kernel,
	linux-crypto, kvm, linux-coco
In-Reply-To: <0b471f38-8c96-4d6b-869a-44a3684ebdd7@amd.com>

Hello Ashish,

On 7/23/2026 6:54 PM, Kalra, Ashish wrote:
> 
> On 7/23/2026 1:50 AM, Kalra, Ashish wrote:
>> Hello Boris,
>>
>> On 7/23/2026 12:14 AM, Borislav Petkov wrote:
>>> On Thu, Jul 23, 2026 at 10:23:56AM +0530, K Prateek Nayak wrote:
>>>> The offline cores will remain offline since hotplug is disabled. RMPOPT
>>>> is simply a performance optimization for RMP checks and leaving the
>>>> offline cores (that will never exit idle) unoptimized should be
>>>> acceptable.
>>>
>>> What happens if you boot with a subset of cores, the boot flow enables RMPOPT
>>> and then you online the rest?
>>>
>>> Have we tried that?
>>>
>>
>>  It can't happen here. snp_prepare() does the following, in order:
>>
>>   1. cpu_hotplug_disable() — freeze the CPU set.
>>   2. fail SNP init (-EOPNOTSUPP) unless every present CPU is online (cpumask_equal(cpu_online_mask, cpu_present_mask).
>>   3. enable SNP.
>>
>>   snp_setup_rmpopt() runs only after snp_prepare() succeeds, so all present CPUs are online and stay online (hotplug is
>>   disabled). No core's primary thread can be offline at this point.
>>
>>   (The kexec path is the one exception — snp_prepare() returns early there — which I'm handling in the other thread.)
>>
>>   Thanks,
>>   Ashish
>>
> 
> Additionally for kexec considerations: on RMPOPT-capable platforms kexec clears SnpEn (full SNP_SHUTDOWN_EX, x86_snp_shutdown=1),
> so snp_prepare() re-runs its cpumask_equal() all-online check on the kexec boot exactly as on a normal boot.
> 
> So no kexec-specific case where RMPOPT gets set up with a partial CPU set.

Thank you for clarifying! Much appreciated _/\_

-- 
Thanks and Regards,
Prateek


^ permalink raw reply

* Re: [PATCH v10 2/6] x86/sev: Initialize RMPOPT configuration MSRs
From: Kalra, Ashish @ 2026-07-23 13:24 UTC (permalink / raw)
  To: Borislav Petkov, K Prateek Nayak
  Cc: tglx, mingo, dave.hansen, x86, hpa, seanjc, peterz,
	thomas.lendacky, herbert, davem, ardb, pbonzini, aik,
	Michael.Roth, Tycho.Andersen, Nathan.Fontenot, ackerleytng,
	jackyli, pgonda, rientjes, jacobhxu, xin, pawan.kumar.gupta,
	babu.moger, dyoung, nikunj, john.allen, darwi, linux-kernel,
	linux-crypto, kvm, linux-coco
In-Reply-To: <5460b946-b511-4060-8951-264d17a2b21b@amd.com>


On 7/23/2026 1:50 AM, Kalra, Ashish wrote:
> Hello Boris,
> 
> On 7/23/2026 12:14 AM, Borislav Petkov wrote:
>> On Thu, Jul 23, 2026 at 10:23:56AM +0530, K Prateek Nayak wrote:
>>> The offline cores will remain offline since hotplug is disabled. RMPOPT
>>> is simply a performance optimization for RMP checks and leaving the
>>> offline cores (that will never exit idle) unoptimized should be
>>> acceptable.
>>
>> What happens if you boot with a subset of cores, the boot flow enables RMPOPT
>> and then you online the rest?
>>
>> Have we tried that?
>>
> 
>  It can't happen here. snp_prepare() does the following, in order:
> 
>   1. cpu_hotplug_disable() — freeze the CPU set.
>   2. fail SNP init (-EOPNOTSUPP) unless every present CPU is online (cpumask_equal(cpu_online_mask, cpu_present_mask).
>   3. enable SNP.
> 
>   snp_setup_rmpopt() runs only after snp_prepare() succeeds, so all present CPUs are online and stay online (hotplug is
>   disabled). No core's primary thread can be offline at this point.
> 
>   (The kexec path is the one exception — snp_prepare() returns early there — which I'm handling in the other thread.)
> 
>   Thanks,
>   Ashish
> 

Additionally for kexec considerations: on RMPOPT-capable platforms kexec clears SnpEn (full SNP_SHUTDOWN_EX, x86_snp_shutdown=1),
so snp_prepare() re-runs its cpumask_equal() all-online check on the kexec boot exactly as on a normal boot.

So no kexec-specific case where RMPOPT gets set up with a partial CPU set.

Thanks,
Ashish

^ permalink raw reply

* Re: [PATCH v15 12/37] KVM: arm64: CCA: Support the VGIC in realms
From: Kohei Enju @ 2026-07-23  9:02 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Steven Price, kvm, kvmarm, Catalin Marinas, Will Deacon,
	James Morse, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
	linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
	Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni,
	Gavin Shan, Shanker Donthineni, Alper Gun, Aneesh Kumar K . V,
	Emi Kisanuki, Vishal Annapurve, WeiLin.Chang, Lorenzo Pieralisi
In-Reply-To: <868q72dt8o.wl-maz@kernel.org>

On 07/23 09:07, Marc Zyngier wrote:
> On Thu, 23 Jul 2026 07:38:16 +0100,
> Kohei Enju <enju.kohei@fujitsu.com> wrote:
> > 
> > On 07/22 10:27, Marc Zyngier wrote:
> > > >
> > > > As far as I can tell, the CCA requirements don't require the
> > > > ARM64_HAS_ICH_HCR_EL2_TDIR feature. If that's the case, this seems to be a
> > > > problem. Is there any workaround for this issue, or should we implement trap
> > > > handlers for those registers?
> > > 
> > > No. Either you support TDIR, like any modern CPU, or you don't run
> > > CCA. I'm not adding yet another level of emulation for this.
> > 
> > However, I'm still not aware of any requirement in the CCA architecture,
> > or in Armv8-A/Armv9-A, that mandates TDIR support.
> 
> The GICv3 architecture has deprecated the lack of TDIR support since
> the very first release of the specification in 2015, because it was
> quickly identified as an architecture bug. CCA mandates GICv3 support
> as written in the spec, and relying on deprecated features 11 years
> later is not acceptable.
> 
> By any definition, this is buggy hardware, and I don't feel a strong
> urge to support it.
> 
> > 
> > When you say "you don't run CCA", is that because the architecture
> > requires it,
> 
> See above.
> 
> > or because that's the current KVM policy? If it's the
> > latter, we'd be interested in adding the required emulation to make it
> > work.
> 
> And I say no to this. Enough. This isn't a "policy*. This is a hard
> red line. "Do Not Cross".
> 
> We already have the most complicated interrupt architecture ever, and
> I have zero desire to add even more complexity to it. Because I'm the
> idiot who ends-up maintaining this horror, and not you.
> 
> So no, no more emulation code. CCA in KVM won't handle systems that do
> not support TDIR, unless you rewrite the CCA support so that it can
> use the *existing* emulation code without any change.

Understood. Thanks for the clarification.

Thanks,
Kohei

> 
> Thanks,
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.
> 

^ permalink raw reply

* Re: [PATCH v7 11/11] x86/virt/tdx: Optimize tdx_pamt_get/put()
From: Nikolay Borisov @ 2026-07-23  9:00 UTC (permalink / raw)
  To: Rick Edgecombe, bp, dave.hansen, hpa, kas, kvm, linux-coco,
	linux-doc, linux-kernel, mingo, pbonzini, seanjc, tglx,
	vannapurve, x86, chao.gao, yan.y.zhao, kai.huang, tony.lindgren,
	binbin.wu
In-Reply-To: <20260718014500.2231262-12-rick.p.edgecombe@intel.com>



On 7/18/26 04:45, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> 
> The Dynamic PAMT get/put helpers use a global spinlock to serialize all
> refcount updates and SEAMCALL invocations. This gives correct behavior for
> concurrent callers, but leads to contention. It is especially bad from the
> KVM side, which is designed to allow faulting in EPT under a shared lock.
> With the global spinlock, not only is the lock an exclusive one, but it is
> for all TDs instead of just a single one.
> 
> But taking the global lock each time is actually unnecessary. Only the 0->1
> and 1->0 refcount transitions actually need the lock (to pair with
> SEAMCALLs that actually add and remove with the Dynamic PAMT pages). The
> common case of incrementing or decrementing a non-zero refcount can be
> done locklessly.
> 
> So create a fast and slow path. Check the refcount outside the lock and
> only take it for the slow path (0->1 and 1->0 transitions).
> 
> On the put side make the refcount adjustment and lock taking atomic so if
> a 'get' happens between them, it doesn't cause the Dynamic PAMT to be
> freed incorrectly. On the get side there is no technique for doing the
> refcount adjustment and lock atomically, so check the refcount again
> inside the lock.
> 
> AI was used under supervision to collect/apply feedback, review code and
> workshop logs. It assisted in identifying/evaluating the stale
> conditionals for the races resolved from the atomic_dec_and_lock() change.
> Separate from atomic_dec_and_lock() fallout, it suggested to change
> atomic_inc() to atomic_set(pamt_refcount, 1) in the put error path for the
> sake of being more precise, which Kiryl had also suggested in the past.
> The model also suggested updated comments following the
> atomic_dec_and_lock() change based on some directed prompting. The
> comments were subsequently edited or further prompted for fine tuning.
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Reviewed-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>

Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>

^ permalink raw reply

* Re: [PATCH v14 03/22] KVM: selftests: Initialize the TDX VM
From: Xiaoyao Li @ 2026-07-23  8:44 UTC (permalink / raw)
  To: Lisa Wang, Andrew Jones, Ackerley Tng, Binbin Wu, Chao Gao,
	Chenyi Qiang, Dave Hansen, Erdem Aktas, Kiryl Shutsemau,
	linux-kselftest, Paolo Bonzini, Pratik R. Sampat, Reinette Chatre,
	Rick Edgecombe, Roger Wang, Ryan Afranji, Sagi Shahar,
	Sean Christopherson, Shuah Khan, Oliver Upton
  Cc: Jeremiah McReynolds, kvm, linux-coco, linux-kernel, x86
In-Reply-To: <20260722-tdx-selftests-v14-3-15ad654a50db@google.com>

On 7/23/2026 7:13 AM, Lisa Wang wrote:
> From: Sagi Shahar <sagis@google.com>
> 
> Add tdx_init_vm() to handle the mandatory VM-level initialization
> sequence required for Intel TDX.
> 
> For TDX, the guest's CPUID configuration must be "sealed" during
> KVM_TDX_INIT_VM before any vCPUs are created. This is necessary because
> the TDX hardware directly virtualizes CPUID and includes the
> configuration in the guest's initial security measurement.
> 
> The helper calculates the required CPUID values by filtering the host-
> supported bits (kvm_get_supported_cpuid) against the "directly
> configurable" bits reported by KVM_TDX_CAPABILITIES, ensuring
> compliance with the strict requirements of the TDH.MNG.INIT SEAMCALL.

<snip>

> +/*
> + * TDX ioctls
> + * Use underscores to avoid collisions with struct member names.
> + */
> +#define __tdx_vm_ioctl(vm, cmd, _flags, arg)				\

sev uses the name __vm_sev_ioctl, I think we need to keep them consistent.

> +({									\
> +	u64 r;								\
> +									\
> +	union {								\
> +		struct kvm_tdx_cmd c;					\
> +		unsigned long raw;					\
> +	} tdx_cmd = { .c = {						\
> +		.id = (cmd),						\
> +		.flags = (u32)(_flags),					\
> +		.data = (u64)(arg),					\
> +	} };								\
> +									\
> +	r = __vm_ioctl(vm, KVM_MEMORY_ENCRYPT_OP, &tdx_cmd.raw);	\
> +	r ?: tdx_cmd.c.hw_error;					\

I know it takes the same handling from __vm_sev_ioctl(). But I think the 
handling for hw_error is not correct, at least for TDX (I didn't check 
for SEV).

the hw_error is the additional info, to tell the SEAMCALL return code, 
when the IOCTL fails. KVM requires hw_error to be in the input, and KVM 
puts the SEAMCALL return code into hw_error when the IOCTL fails due to 
SEAMCALL failure. That means, when r == 0, the hw_error is always 0.

I think we need to provide hw_error along with r to the caller so that 
caller can print them together.

> +})
> +
> +#define tdx_vm_ioctl(vm, cmd, flags, arg)				\
> +({									\
> +	u64 ret = __tdx_vm_ioctl(vm, cmd, flags, arg);			\
> +									\
> +	if (ret) {							\
> +		TEST_ASSERT(!ret,					\
> +			    "%s failed, rc: 0x%llx errno: %i (%s)",	\
> +			    #cmd, (unsigned long long)ret,		\
> +			    errno, strerror(errno));			\

The if() looks silly. Why add it? And why change it from 
__TEST_ASSERT_VM_VCPU_IOCTL() in the v13?

Considering the suggestion of hw_error above, I think we need to 
introduce the TEST_ASSERT_TDX_VM_VCPU_IOCTL() which accepts additional 
hw_error?

<snip>
> diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> new file mode 100644
> index 000000000000..e1ffb67a106c
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> @@ -0,0 +1,120 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include "processor.h"
> +#include "tdx/tdx_util.h"
> +
> +static struct kvm_tdx_capabilities *tdx_read_capabilities(struct kvm_vm *vm)

make it const, is better.

<snip>

> +
> +void tdx_init_vm(struct kvm_vm *vm, u64 attributes)
> +{
> +	struct kvm_tdx_init_vm *init_vm;
> +	const struct kvm_cpuid2 *tmp;
> +	struct kvm_cpuid2 *cpuid;
> +
> +	tmp = kvm_get_supported_cpuid();
> +
> +	cpuid = allocate_kvm_cpuid2(tmp->nent);
> +	memcpy(cpuid, tmp, kvm_cpuid2_size(tmp->nent));
> +	tdx_filter_cpuid(vm, cpuid);
> +
> +	init_vm = calloc(1, sizeof(*init_vm) +
> +			 sizeof(init_vm->cpuid.entries[0]) * cpuid->nent);
> +	TEST_ASSERT(init_vm, "init_vm allocation failed");
> +
> +	memcpy(&init_vm->cpuid, cpuid, kvm_cpuid2_size(cpuid->nent));
> +	free(cpuid);
> +
> +	init_vm->attributes = attributes;

Besides CPUID, it only allows attributes to be configure but leave XFAM 
as 0. I think the changelog needs to explain why we need to configure 
attributes.

The rest of the patch looks good to me.

> +
> +	tdx_vm_ioctl(vm, KVM_TDX_INIT_VM, 0, init_vm);
> +
> +	free(init_vm);
> +}
> 


^ permalink raw reply

* Re: [PATCH v2 2/2] virt: tdx-guest: Allocate Quote buffer dynamically
From: Peter Fang @ 2026-07-23  8:19 UTC (permalink / raw)
  To: Kiryl Shutsemau
  Cc: Edgecombe, Rick P, Hansen, Dave, kvm@vger.kernel.org,
	bp@alien8.de, Li, Xiaoyao, x86@kernel.org,
	sathyanarayanan.kuppuswamy@linux.intel.com, mingo@redhat.com,
	dave.hansen@linux.intel.com, tglx@kernel.org,
	linux-coco@lists.linux.dev, hpa@zytor.com,
	linux-kernel@vger.kernel.org, binbin.wu@linux.intel.com
In-Reply-To: <amCv00f5Q6QlztyY@thinkstation>

On Wed, Jul 22, 2026 at 01:00:35PM +0100, Kiryl Shutsemau wrote:
> On Tue, Jul 21, 2026 at 04:43:57PM +0000, Edgecombe, Rick P wrote:
> > BUT, is this number coming from the untrusted host or the TDX module? I cant
> > find TDCS_QUOTE_MAX_SIZE in the docs.
> 
> I cannot find it too. Peter, any pointers where it is documented
> publicly?

Gosh sorry I missed this. Turns out it was not fully published due to
some internal error. Yikes.

This metadata field is documented in "Intel TDX Module Extension for
Quoting", Section "TD-Scope Metadata" [1]. But the encoding of this
field got missed in the June 2026 release of "Intel TDX Module ABI
Definitions". I just escalated this. I'll come back and give an update
once it's fixed... Apologies for the oversight.

[1] https://cdrdv2.intel.com/v1/dl/getContent/874303

> 
> > If it is coming from the TDX module then
> > we are not talking about security considerations at all. It's just a functional
> > "do you want to know if things are failing". Not sure why this one would be
> > special in that regard. If we think it will fail so often that we don't want a
> > warning, then we probably need another solution.
> 
> Silent makes no sense to me. We are talking about attestation. If
> attestation fails the TD is functionally dead: it cannot obtain secrets.

Yep. I'll remove __GFP_NOWARN in the next version. Thanks.

> 
> -- 
>   Kiryl Shutsemau / Kirill A. Shutemov

^ permalink raw reply

* Re: [PATCH v15 12/37] KVM: arm64: CCA: Support the VGIC in realms
From: Marc Zyngier @ 2026-07-23  8:07 UTC (permalink / raw)
  To: Kohei Enju
  Cc: Steven Price, kvm, kvmarm, Catalin Marinas, Will Deacon,
	James Morse, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
	linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
	Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni,
	Gavin Shan, Shanker Donthineni, Alper Gun, Aneesh Kumar K . V,
	Emi Kisanuki, Vishal Annapurve, WeiLin.Chang, Lorenzo Pieralisi
In-Reply-To: <amGwgAfHWX2P9pZx@FCCLS0092175.localdomain>

On Thu, 23 Jul 2026 07:38:16 +0100,
Kohei Enju <enju.kohei@fujitsu.com> wrote:
> 
> On 07/22 10:27, Marc Zyngier wrote:
> > >
> > > As far as I can tell, the CCA requirements don't require the
> > > ARM64_HAS_ICH_HCR_EL2_TDIR feature. If that's the case, this seems to be a
> > > problem. Is there any workaround for this issue, or should we implement trap
> > > handlers for those registers?
> > 
> > No. Either you support TDIR, like any modern CPU, or you don't run
> > CCA. I'm not adding yet another level of emulation for this.
> 
> However, I'm still not aware of any requirement in the CCA architecture,
> or in Armv8-A/Armv9-A, that mandates TDIR support.

The GICv3 architecture has deprecated the lack of TDIR support since
the very first release of the specification in 2015, because it was
quickly identified as an architecture bug. CCA mandates GICv3 support
as written in the spec, and relying on deprecated features 11 years
later is not acceptable.

By any definition, this is buggy hardware, and I don't feel a strong
urge to support it.

> 
> When you say "you don't run CCA", is that because the architecture
> requires it,

See above.

> or because that's the current KVM policy? If it's the
> latter, we'd be interested in adding the required emulation to make it
> work.

And I say no to this. Enough. This isn't a "policy*. This is a hard
red line. "Do Not Cross".

We already have the most complicated interrupt architecture ever, and
I have zero desire to add even more complexity to it. Because I'm the
idiot who ends-up maintaining this horror, and not you.

So no, no more emulation code. CCA in KVM won't handle systems that do
not support TDIR, unless you rewrite the CCA support so that it can
use the *existing* emulation code without any change.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply

* Re: [PATCH v10 2/6] x86/sev: Initialize RMPOPT configuration MSRs
From: Kalra, Ashish @ 2026-07-23  8:00 UTC (permalink / raw)
  To: K Prateek Nayak, Borislav Petkov
  Cc: tglx, mingo, dave.hansen, x86, hpa, seanjc, peterz,
	thomas.lendacky, herbert, davem, ardb, pbonzini, aik,
	Michael.Roth, Tycho.Andersen, Nathan.Fontenot, ackerleytng,
	jackyli, pgonda, rientjes, jacobhxu, xin, pawan.kumar.gupta,
	babu.moger, dyoung, nikunj, john.allen, darwi, linux-kernel,
	linux-crypto, kvm, linux-coco
In-Reply-To: <535c2a43-47b9-45ac-be9f-d0c53f9f01cc@amd.com>


On 7/23/2026 2:16 AM, K Prateek Nayak wrote:
> Hello Ashish,
> 
> On 7/23/2026 12:33 PM, Kalra, Ashish wrote:
>> Hello Prateek,
>>
>> On 7/23/2026 12:58 AM, K Prateek Nayak wrote:
>>> Hello Boris,
>>>
>>> On 7/23/2026 10:44 AM, Borislav Petkov wrote:
>>>> On Thu, Jul 23, 2026 at 10:23:56AM +0530, K Prateek Nayak wrote:
>>>>> The offline cores will remain offline since hotplug is disabled. RMPOPT
>>>>> is simply a performance optimization for RMP checks and leaving the
>>>>> offline cores (that will never exit idle) unoptimized should be
>>>>> acceptable.
>>>>
>>>> What happens if you boot with a subset of cores, the boot flow enables RMPOPT
>>>> and then you online the rest?
>>>
>>> RMPOPT happens at __sev_snp_init_locked() for all online cores. Until
>>> then, SnpEn is still 0 and we don't need RMPOPT because RMP checks
>>> haven't been enabled yet.
>>>
>>>>
>>>> Have we tried that?
>>>
>>> That said, Ashish, should snp_rmptable_init() do a
>>> snp_rmpopt_all_physmem() (or something equivalent) when it finds SNP_EN
>>> set in MSR_AMD64_SYSCFG after a kexec?
>>>
>>
>> It already happens, just later in the sequence. On kexec __sev_snp_init_locked() still runs: snp_prepare() returns early
>> (SnpEn set), SNP_INIT re-initializes the firmware context, and then snp_setup_rmpopt() is called.
> 
> But that only happens when the first SNP guest is created right? Until
> then RMP checks are enforced but no confidential guest is running and
> the CPUs are paying price for those checks.

Typically, on RMPOPT-capable platforms SnpEn isn't inherited across kexec. 

As i mentioned earlier (on v9 thread), kexec with SNP active currently requires
a full SNP shutdown before the kexec. SNP_SHUTDOWN_EX (and the IOMMU SNP shutdown it performs)
fail if there are any active SNP guests or assigned ASIDs, so a working kexec has to terminate
all SNP guests and run a full shutdown first (via systemctl kexec). 

Now, on any platform that supports the x86 SNP shutdown, that issues SNP_SHUTDOWN_EX with x86_snp_shutdown=1, 
the firmware clears SnpEn before the kexec. The kexec kernel boots with SnpEn clear, exactly like a normal boot. (The
only case SnpEn survives kexec is the legacy shutdown path, and those platforms don't support RMPOPT anyway.)

Thanks,
Ashish

> 
>> On the fresh kexec kernel, rmpopt_wq is NULL, so snp_setup_rmpopt() does the full setup — programs the per-CPU RMPOPT_BASE MSRs and
>> queues the initial all-physmem optimization pass. So RMPOPT is re-applied on kexec through the normal path.
>>
>> Doing it in snp_rmptable_init() would be too early: the per-CPU RMPOPT_BASE MSRs aren't programmed until
>> snp_setup_rmpopt(), so a pass there would have no base configured.
> 
> Ack! Which is why I mentioned something equivalent ;-)
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox