Linux Confidential Computing Development
 help / color / mirror / Atom feed
* [PATCH v7 2/2] KVM: SEV: Add KVM_SEV_SNP_ENABLE_REQ_CERTS command
From: Michael Roth @ 2026-01-09 23:17 UTC (permalink / raw)
  To: kvm
  Cc: linux-coco, linux-kernel, pbonzini, seanjc, jroedel,
	thomas.lendacky, liam.merwick, huibo.wang
In-Reply-To: <20260109231732.1160759-1-michael.roth@amd.com>

Introduce a new command for KVM_MEMORY_ENCRYPT_OP ioctl that can be used
to enable fetching of endorsement key certificates from userspace via
the new KVM_EXIT_SNP_REQ_CERTS exit type. Also introduce a new
KVM_X86_SEV_SNP_REQ_CERTS KVM device attribute so that userspace can
query whether the kernel supports the new command/exit.

Suggested-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Liam Merwick <liam.merwick@oracle.com>
Tested-by: Liam Merwick <liam.merwick@oracle.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 .../virt/kvm/x86/amd-memory-encryption.rst    | 52 ++++++++++++++++++-
 arch/x86/include/uapi/asm/kvm.h               |  2 +
 arch/x86/kvm/svm/sev.c                        | 16 ++++++
 3 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/Documentation/virt/kvm/x86/amd-memory-encryption.rst b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
index 1ddb6a86ce7f..543b5e5dd8d4 100644
--- a/Documentation/virt/kvm/x86/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
@@ -572,6 +572,52 @@ Returns: 0 on success, -negative on error
 See SNP_LAUNCH_FINISH in the SEV-SNP specification [snp-fw-abi]_ for further
 details on the input parameters in ``struct kvm_sev_snp_launch_finish``.
 
+21. KVM_SEV_SNP_ENABLE_REQ_CERTS
+--------------------------------
+
+The KVM_SEV_SNP_ENABLE_REQ_CERTS command will configure KVM to exit to
+userspace with a ``KVM_EXIT_SNP_REQ_CERTS`` exit type as part of handling
+a guest attestation report, which will to allow userspace to provide a
+certificate corresponding to the endorsement key used by firmware to sign
+that attestation report.
+
+Returns: 0 on success, -negative on error
+
+NOTE: The endorsement key used by firmware may change as a result of
+management activities like updating SEV-SNP firmware or loading new
+endorsement keys, so some care should be taken to keep the returned
+certificate data in sync with the actual endorsement key in use by
+firmware at the time the attestation request is sent to SNP firmware. The
+recommended scheme to do this is to use file locking (e.g. via fcntl()'s
+F_OFD_SETLK) in the following manner:
+
+  - Prior to obtaining/providing certificate data as part of servicing an
+    exit type of ``KVM_EXIT_SNP_REQ_CERTS``, the VMM should obtain a
+    shared/read or exclusive/write lock on the certificate blob file before
+    reading it and returning it to KVM, and continue to hold the lock until
+    the attestation request is actually sent to firmware. To facilitate
+    this, the VMM can set the ``immediate_exit`` flag of kvm_run just after
+    supplying the certificate data, and just before resuming the vCPU.
+    This will ensure the vCPU will exit again to userspace with ``-EINTR``
+    after it finishes fetching the attestation request from firmware, at
+    which point the VMM can safely drop the file lock.
+
+  - Tools/libraries that perform updates to SNP firmware TCB values or
+    endorsement keys (e.g. via /dev/sev interfaces such as ``SNP_COMMIT``,
+    ``SNP_SET_CONFIG``, or ``SNP_VLEK_LOAD``, see
+    Documentation/virt/coco/sev-guest.rst for more details) in such a way
+    that the certificate blob needs to be updated, should similarly take an
+    exclusive lock on the certificate blob for the duration of any updates
+    to endorsement keys or the certificate blob contents to ensure that
+    VMMs using the above scheme will not return certificate blob data that
+    is out of sync with the endorsement key used by firmware at the time
+    the attestation request is actually issued.
+
+This scheme is recommended so that tools can use a fairly generic/natural
+approach to synchronizing firmware/certificate updates via file-locking,
+which should make it easier to maintain interoperability across
+tools/VMMs/vendors.
+
 Device attribute API
 ====================
 
@@ -579,11 +625,15 @@ Attributes of the SEV implementation can be retrieved through the
 ``KVM_HAS_DEVICE_ATTR`` and ``KVM_GET_DEVICE_ATTR`` ioctls on the ``/dev/kvm``
 device node, using group ``KVM_X86_GRP_SEV``.
 
-Currently only one attribute is implemented:
+The following attributes are currently implemented:
 
 * ``KVM_X86_SEV_VMSA_FEATURES``: return the set of all bits that
   are accepted in the ``vmsa_features`` of ``KVM_SEV_INIT2``.
 
+* ``KVM_X86_SEV_SNP_REQ_CERTS``: return a value of 1 if the kernel supports the
+  ``KVM_EXIT_SNP_REQ_CERTS`` exit, which allows for fetching endorsement key
+  certificates from userspace for each SNP attestation request the guest issues.
+
 Firmware Management
 ===================
 
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 7ceff6583652..b2c928c5965d 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -503,6 +503,7 @@ struct kvm_sync_regs {
 #define KVM_X86_GRP_SEV			1
 #  define KVM_X86_SEV_VMSA_FEATURES	0
 #  define KVM_X86_SNP_POLICY_BITS	1
+#  define KVM_X86_SEV_SNP_REQ_CERTS	2
 
 struct kvm_vmx_nested_state_data {
 	__u8 vmcs12[KVM_STATE_NESTED_VMX_VMCS_SIZE];
@@ -743,6 +744,7 @@ enum sev_cmd_id {
 	KVM_SEV_SNP_LAUNCH_START = 100,
 	KVM_SEV_SNP_LAUNCH_UPDATE,
 	KVM_SEV_SNP_LAUNCH_FINISH,
+	KVM_SEV_SNP_ENABLE_REQ_CERTS,
 
 	KVM_SEV_NR_MAX,
 };
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 2405c6fad95c..695463bc6c5b 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2161,6 +2161,9 @@ int sev_dev_get_attr(u32 group, u64 attr, u64 *val)
 		*val = snp_supported_policy_bits;
 		return 0;
 
+	case KVM_X86_SEV_SNP_REQ_CERTS:
+		*val = sev_snp_enabled ? 1 : 0;
+		return 0;
 	default:
 		return -ENXIO;
 	}
@@ -2577,6 +2580,16 @@ static int snp_launch_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+static int snp_enable_certs(struct kvm *kvm)
+{
+	if (kvm->created_vcpus || !sev_snp_guest(kvm))
+		return -EINVAL;
+
+	to_kvm_sev_info(kvm)->snp_certs_enabled = true;
+
+	return 0;
+}
+
 int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -2682,6 +2695,9 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_SNP_LAUNCH_FINISH:
 		r = snp_launch_finish(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_SNP_ENABLE_REQ_CERTS:
+		r = snp_enable_certs(kvm);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
-- 
2.25.1


^ permalink raw reply related

* [PATCH v7 1/2] KVM: Introduce KVM_EXIT_SNP_REQ_CERTS for SNP certificate-fetching
From: Michael Roth @ 2026-01-09 23:17 UTC (permalink / raw)
  To: kvm
  Cc: linux-coco, linux-kernel, pbonzini, seanjc, jroedel,
	thomas.lendacky, liam.merwick, huibo.wang, Dionna Glaze
In-Reply-To: <20260109231732.1160759-1-michael.roth@amd.com>

For SEV-SNP, the host can optionally provide a certificate table to the
guest when it issues an attestation request to firmware (see GHCB 2.0
specification regarding "SNP Extended Guest Requests"). This certificate
table can then be used to verify the endorsement key used by firmware to
sign the attestation report.

While it is possible for guests to obtain the certificates through other
means, handling it via the host provides more flexibility in being able
to keep the certificate data in sync with the endorsement key throughout
host-side operations that might resulting in the endorsement key
changing.

In the case of KVM, userspace will be responsible for fetching the
certificate table and keeping it in sync with any modifications to the
endorsement key by other userspace management tools. Define a new
KVM_EXIT_SNP_REQ_CERTS event where userspace is provided with the GPA of
the buffer the guest has provided as part of the attestation request so
that userspace can write the certificate data into it while relying on
filesystem-based locking to keep the certificates up-to-date relative to
the endorsement keys installed/utilized by firmware at the time the
certificates are fetched.

Reviewed-by: Liam Merwick <liam.merwick@oracle.com>
Tested-by: Liam Merwick <liam.merwick@oracle.com>
Tested-by: Dionna Glaze <dionnaglaze@google.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
[Melody: Update the documentation scheme about how file locking is
         expected to happen.]
Signed-off-by: Melody Wang <huibo.wang@amd.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 Documentation/virt/kvm/api.rst | 44 ++++++++++++++++++++++++
 arch/x86/kvm/svm/sev.c         | 62 ++++++++++++++++++++++++++++++----
 arch/x86/kvm/svm/svm.h         |  1 +
 include/uapi/linux/kvm.h       |  9 +++++
 4 files changed, 110 insertions(+), 6 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 01a3abef8abb..aa8b192179c2 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -7353,6 +7353,50 @@ Please note that the kernel is allowed to use the kvm_run structure as the
 primary storage for certain register types. Therefore, the kernel may use the
 values in kvm_run even if the corresponding bit in kvm_dirty_regs is not set.
 
+::
+
+		/* KVM_EXIT_SNP_REQ_CERTS */
+		struct kvm_exit_snp_req_certs {
+			__u64 gpa;
+			__u64 npages;
+			__u64 ret;
+		};
+
+KVM_EXIT_SNP_REQ_CERTS indicates an SEV-SNP guest with certificate-fetching
+enabled (see KVM_SEV_SNP_ENABLE_REQ_CERTS) has generated an Extended Guest
+Request NAE #VMGEXIT (SNP_GUEST_REQUEST) with message type MSG_REPORT_REQ,
+i.e. has requested an attestation report from firmware, and would like the
+certificate data corresponding the attestation report signature to be
+provided by the hypervisor as part of the request.
+
+To allow for userspace to provide the certificate, the 'gpa' and 'npages'
+are forwarded verbatim from the guest request (the RAX and RBX GHCB fields
+respectively).  'ret' is not an "output" from KVM, and is always '0' on
+exit.  KVM verifies the 'gpa' is 4KiB aligned prior to exiting to userspace,
+but otherwise the information from the guest isn't validated.
+
+Upon the next KVM_RUN, e.g. after userspace has serviced the request (or not),
+KVM will complete the #VMGEXIT, using the 'ret' field to determine whether to
+signal success or failure to the guest, and on failure, what reason code will
+be communicated via SW_EXITINFO2.  If 'ret' is set to an unsupported value (see
+the table below), KVM_RUN will fail with -EINVAL.  For a 'ret' of 'ENOSPC', KVM
+also consumes the 'npages' field, i.e. userspace can use the field to inform
+the guest of the number of pages needed to hold all the certificate data.
+
+The supported 'ret' values and their respective SW_EXITINFO2 encodings:
+
+  ======     =============================================================
+  0          0x0, i.e. success.  KVM will emit an SNP_GUEST_REQUEST command
+             to SNP firmware.
+  ENOSPC     0x0000000100000000, i.e. not enough guest pages to hold the
+             certificate table and certificate data.  KVM will also set the
+             RBX field in the GHBC to 'npages'.
+  EAGAIN     0x0000000200000000, i.e. the host is busy and the guest should
+             retry the request.
+  EIO        0xffffffff00000000, for all other errors (this return code is
+             a KVM-defined hypervisor value, as allowed by the GHCB)
+  ======     =============================================================
+
 
 .. _cap_enable:
 
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index f59c65abe3cf..2405c6fad95c 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -41,6 +41,16 @@
 
 #define GHCB_HV_FT_SUPPORTED	(GHCB_HV_FT_SNP | GHCB_HV_FT_SNP_AP_CREATION)
 
+/*
+ * The GHCB spec essentially states that all non-zero error codes other than
+ * those explicitly defined above should be treated as an error by the guest.
+ * Define a generic error to cover that case, and choose a value that is not
+ * likely to overlap with new explicit error codes should more be added to
+ * the GHCB spec later. KVM will use this to report generic errors when
+ * handling SNP guest requests.
+ */
+#define SNP_GUEST_VMM_ERR_GENERIC       (~0U)
+
 /* enable/disable SEV support */
 static bool sev_enabled = true;
 module_param_named(sev, sev_enabled, bool, 0444);
@@ -4155,6 +4165,36 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_
 	return ret;
 }
 
+static int snp_req_certs_err(struct vcpu_svm *svm, u32 vmm_error)
+{
+	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, SNP_GUEST_ERR(vmm_error, 0));
+
+	return 1; /* resume guest */
+}
+
+static int snp_complete_req_certs(struct kvm_vcpu *vcpu)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+	struct vmcb_control_area *control = &svm->vmcb->control;
+
+	switch (READ_ONCE(vcpu->run->snp_req_certs.ret)) {
+	case 0:
+		return snp_handle_guest_req(svm, control->exit_info_1,
+					    control->exit_info_2);
+	case ENOSPC:
+		vcpu->arch.regs[VCPU_REGS_RBX] = vcpu->run->snp_req_certs.npages;
+		return snp_req_certs_err(svm, SNP_GUEST_VMM_ERR_INVALID_LEN);
+	case EAGAIN:
+		return snp_req_certs_err(svm, SNP_GUEST_VMM_ERR_BUSY);
+	case EIO:
+		return snp_req_certs_err(svm, SNP_GUEST_VMM_ERR_GENERIC);
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
 static int snp_handle_ext_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_gpa)
 {
 	struct kvm *kvm = svm->vcpu.kvm;
@@ -4170,14 +4210,15 @@ static int snp_handle_ext_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t r
 	/*
 	 * As per GHCB spec, requests of type MSG_REPORT_REQ also allow for
 	 * additional certificate data to be provided alongside the attestation
-	 * report via the guest-provided data pages indicated by RAX/RBX. The
-	 * certificate data is optional and requires additional KVM enablement
-	 * to provide an interface for userspace to provide it, but KVM still
-	 * needs to be able to handle extended guest requests either way. So
-	 * provide a stub implementation that will always return an empty
-	 * certificate table in the guest-provided data pages.
+	 * report via the guest-provided data pages indicated by RAX/RBX. If
+	 * userspace enables KVM_EXIT_SNP_REQ_CERTS, then exit to userspace
+	 * to give userspace an opportunity to provide the certificate data
+	 * before issuing/completing the attestation request. Otherwise, return
+	 * an empty certificate table in the guest-provided data pages and
+	 * handle the attestation request immediately.
 	 */
 	if (msg_type == SNP_MSG_REPORT_REQ) {
+		struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
 		struct kvm_vcpu *vcpu = &svm->vcpu;
 		u64 data_npages;
 		gpa_t data_gpa;
@@ -4191,6 +4232,15 @@ static int snp_handle_ext_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t r
 		if (!PAGE_ALIGNED(data_gpa))
 			goto request_invalid;
 
+		if (sev->snp_certs_enabled) {
+			vcpu->run->exit_reason = KVM_EXIT_SNP_REQ_CERTS;
+			vcpu->run->snp_req_certs.gpa = data_gpa;
+			vcpu->run->snp_req_certs.npages = data_npages;
+			vcpu->run->snp_req_certs.ret = 0;
+			vcpu->arch.complete_userspace_io = snp_complete_req_certs;
+			return 0;
+		}
+
 		/*
 		 * As per GHCB spec (see "SNP Extended Guest Request"), the
 		 * certificate table is terminated by 24-bytes of zeroes.
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 01be93a53d07..eac1cbd3debe 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -115,6 +115,7 @@ struct kvm_sev_info {
 	void *guest_resp_buf;   /* Bounce buffer for SNP Guest Request output */
 	struct mutex guest_req_mutex; /* Must acquire before using bounce buffers */
 	cpumask_var_t have_run_cpus; /* CPUs that have done VMRUN for this VM. */
+	bool snp_certs_enabled;	/* SNP certificate-fetching support. */
 };
 
 struct kvm_svm {
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index dddb781b0507..8cd107cdcf0b 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -135,6 +135,12 @@ struct kvm_xen_exit {
 	} u;
 };
 
+struct kvm_exit_snp_req_certs {
+	__u64 gpa;
+	__u64 npages;
+	__u64 ret;
+};
+
 #define KVM_S390_GET_SKEYS_NONE   1
 #define KVM_S390_SKEYS_MAX        1048576
 
@@ -180,6 +186,7 @@ struct kvm_xen_exit {
 #define KVM_EXIT_MEMORY_FAULT     39
 #define KVM_EXIT_TDX              40
 #define KVM_EXIT_ARM_SEA          41
+#define KVM_EXIT_SNP_REQ_CERTS    42
 
 /* For KVM_EXIT_INTERNAL_ERROR */
 /* Emulate instruction failed. */
@@ -482,6 +489,8 @@ struct kvm_run {
 			__u64 gva;
 			__u64 gpa;
 		} arm_sea;
+		/* KVM_EXIT_SNP_REQ_CERTS */
+		struct kvm_exit_snp_req_certs snp_req_certs;
 		/* Fix the size of the union. */
 		char padding[256];
 	};
-- 
2.25.1


^ permalink raw reply related

* [PATCH v7 0/2] SEV-SNP: Add KVM support for SNP certificate fetching
From: Michael Roth @ 2026-01-09 23:17 UTC (permalink / raw)
  To: kvm
  Cc: linux-coco, linux-kernel, pbonzini, seanjc, jroedel,
	thomas.lendacky, liam.merwick, huibo.wang

This patchset is also available at:

  https://github.com/amdese/linux/commits/snp-certs-v7

and is based on top of kvm/next (0499add8efd7)


Overview
--------

The GHCB 2.0 specification defines 2 GHCB request types to allow SNP guests
to send encrypted messages/requests to firmware: SNP Guest Requests and SNP
Extended Guest Requests. These encrypted messages are used for things like
servicing attestation requests issued by the guest. Implementing support for
these is required to be fully GHCB-compliant.

For the most part, KVM only needs to handle forwarding these requests to
firmware (to be issued via the SNP_GUEST_REQUEST firmware command defined
in the SEV-SNP Firmware ABI), and then forwarding the encrypted response to
the guest.

However, in the case of SNP Extended Guest Requests, the host is also
able to provide the certificate data corresponding to the endorsement key
used by firmware to sign attestation report requests. This certificate data
is provided by userspace because:

  1) It allows for different keys/key types to be used for each particular
     guest with requiring any sort of KVM API to configure the certificate
     table in advance on a per-guest basis.

  2) It provides additional flexibility with how attestation requests might
     be handled during live migration where the certificate data for
     source/dest might be different.

  3) It allows all synchronization between certificates and firmware/signing
     key updates to be handled purely by userspace rather than requiring
     some in-kernel mechanism to facilitate it. [1]

To support fetching certificate data from userspace, a new KVM
KVM_EXIT_SNP_REQ_CERTS exit type is used to fetch the data similarly to
KVM_EXIT_MMIO/etc, with an associate KVM capability to detect/enable the
exits depending on whether userspace has been configured to provide
certificate data.

[1] https://lore.kernel.org/kvm/ZS614OSoritrE1d2@google.com/


Testing
-------

For testing this via QEMU, use the following tree:

  https://github.com/amdese/qemu/commits/snp-certs-rfc3-wip1

A basic command-line invocation for SNP with certificate data supplied
would be:

 qemu-system-x86_64 -smp 32,maxcpus=255 -cpu EPYC-Milan-v2
  -machine q35,confidential-guest-support=sev0,memory-backend=ram1
  -object memory-backend-memfd,id=ram1,size=4G,share=true,reserve=false
  -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,id-auth=,certs-filename=/home/mroth/cert.blob
  -bios OVMF.fd

Something like the following simple example can be used to simulate an
exclusive lock being held on the certificate by management tools performing an
update:

  #include <stdlib.h>
  #include <stdio.h>
  #define __USE_GNU
  #include <fcntl.h>
  #include <unistd.h>
  #include <errno.h>
  #include <stdbool.h>
  #include <sys/types.h>
  #include <sys/stat.h>
  
  int main(int argc, void **argv)
  {
      int ret, fd, i = 0;
      char *path = argv[1];
  
      struct flock fl = {
          .l_whence = SEEK_SET,
          .l_start = 0,
          .l_len = 0,
          .l_type = F_WRLCK
      };
  
      fd = open(path, O_RDWR);
      ret = fcntl(fd, F_OFD_SETLK, &fl);
      if (ret) {
          printf("error locking file, ret %d errno %d\n", ret, errno);
          return ret;
      }
  
      while (true) {
          i++;
          printf("now holding lock (%d seconds elapsed)...\n", i);
          usleep(1000 * 1000);
      }
  
      return 0;
  }

The format of the certificate blob is defined in the GHCB 2.0 specification,
but if it's not being parsed on the guest-side then random data will suffice
for testing the KVM bits.

Any feedback/review is appreciated.

Thanks!

-Mike

Changes since v6:

 * Incorporate documentation/comment suggestions from Sean, along with
   additional clarity/grammar fixups.
 * Don't define SNP_GUEST_VMM_ERR_GENERIC for general use within kernel,
   instead limit it to a KVM-specific choice of value in lieu of any
   formally-defined guest message return code for generic/undefined errors.
 * switch struct kvm_exit_snp_req_certs to using a 'gpa' argument instead
   of 'gfn' (Sean)
 * rebase to kvm/next, re-test, and collect R-b/T-b's

Changes since v5:

 * Drop KVM capability in favor of a KVM device attribute to advertise
   support to userspace, and introduce a new KVM_SEV_SNP_ENABLE_REQ_CERTS
   command to switch it on (Sean)
 * Only allow certificate-fetching to be enabled prior to starting any
   vCPUs to avoid races/unexpected behavior (Sean)
 * Drop unecessary cast in SNP_GUEST_VMM_ERR_GENERIC definition (Sean)
 * Add checks to enforce that userspace only uses EIO to indicate generic
   errors when fetching certificates to ensure that other error codes remain
   usable for other/future error conditions (Joerg, Sean)
 * Clean up setting of GHCB error codes via a small helper routine (Sean)
 * Use READ_ONCE() when checking userspace's return value in struct kvm_run
   (Sean)
 * Use u64 instead of u32 for npages/ret fields in the kvm_run struct (Sean)
 * Use a switch statement to handle individual error codes reported by
   userspace (Sean)
 * Move 'snp_certs_enabled' flag from struct kvm_arch to kvm_sev_info (Sean)
 * Documentation fix-ups (Sean)
 * Rebase to latest kvm/next

Changes since v4:

 * Minor documentation updates to make the implementation notes less
   specific to QEMU.
 * Collected Reviewed-by/Tested-by from v3 since there have been no
   functional changes since then and only minor documentation updates.
 * Rebased/re-tested on top of latest kvm/next (d3d0b8dfe060)

Changes since v3:

 * This version updates the documentation scheme about how file locking is
   expected to happen.

Changes since v2:

 * As per discussion during PUCK, drop all the KVM_EXIT_COCO infrastructure
   since there are enough differences with TDX's quote generation to make
   unifying the 2 exits over-complicated for userspace, and the code-sharing
   we stand to gain from placing everything under the KVM_EXIT_COCO_*
   umbrella are of questionable benefit.
 * Update/simplify documentation as per the above.
 * Rebase/re-test on top of latest kvm-coco-queue

Changes since v1:

 * Drop subtype-specific error codes. Instead use standard error codes like
   ENOSPC/etc. and let KVM determine whether a particular error requires
   special handling for a particular KVM_EXIT_COCO subtype. (Sean)
 * Introduce special handling for EAGAIN for KVM_EXIT_COCO_REQ_CERTS such
   that the guest can be instructed to retry if userspace is temporarily unable
   to immediately lock/provide the certificate data. (Sean)
 * Move the 'ret' field of struct kvm_exit_coco to the top-level so all
   sub-types can propagate error codes the same way.
 * Add more clarifying details in KVM documentation about the suggested
   file-locking scheme to avoid races between certificate requests and updates
   to SNP firmware that might modify the endorsement key corresponding to the
   certificate data.

Changes since splitting this off from v15 SNP KVM patchset:

 * Address clang-reported warnings regarding uninitialized variables 
 * Address a memory leak of the request/response buffer pages, and refactor
   the code based on Sean's suggestions:
   https://lore.kernel.org/kvm/ZktbBRLXeOp9X6aH@google.com/
 * Fix SNP Extended Guest Request handling to only attempt to fetch
   certificates if handling MSG_REQ_REPORT (attestation) message types
 * Drop KVM_EXIT_VMGEXIT and introduce KVM_EXIT_COCO events instead
 * Refactor patch layout for easier handling/review


----------------------------------------------------------------
Michael Roth (2):
      KVM: Introduce KVM_EXIT_SNP_REQ_CERTS for SNP certificate-fetching
      KVM: SEV: Add KVM_SEV_SNP_ENABLE_REQ_CERTS command

 Documentation/virt/kvm/api.rst                     | 44 ++++++++++++
 .../virt/kvm/x86/amd-memory-encryption.rst         | 52 ++++++++++++++-
 arch/x86/include/uapi/asm/kvm.h                    |  2 +
 arch/x86/kvm/svm/sev.c                             | 78 ++++++++++++++++++++--
 arch/x86/kvm/svm/svm.h                             |  1 +
 include/uapi/linux/kvm.h                           |  9 +++
 6 files changed, 179 insertions(+), 7 deletions(-)




^ permalink raw reply

* Re: [PATCH v2 2/2] x86/virt/tdx: Print TDX module version during init
From: Edgecombe, Rick P @ 2026-01-09 20:00 UTC (permalink / raw)
  To: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Verma, Vishal L,
	linux-kernel@vger.kernel.org
  Cc: Gao, Chao, bp@alien8.de, Huang, Kai, kas@kernel.org,
	dave.hansen@linux.intel.com, mingo@redhat.com, Williams, Dan J,
	tglx@linutronix.de, hpa@zytor.com, x86@kernel.org
In-Reply-To: <20260109-tdx_print_module_version-v2-2-e10e4ca5b450@intel.com>

On Fri, 2026-01-09 at 12:14 -0700, Vishal Verma wrote:
> It is useful to print the TDX module version in dmesg logs. This is
> currently the only way to determine the module version from the host. It
> also creates a record for any future problems being investigated. This
> was also requested in [1].
> 
> Include the version in the log messages during init, e.g.:
> 
>   virt/tdx: TDX module version: 1.5.24
>   virt/tdx: 1034220 KB allocated for PAMT
>   virt/tdx: module initialized
> 
> Print the version in get_tdx_sys_info(), right after the version
> metadata is read, which makes it available even if there are subsequent
> initialization failures.
> 
> Based on a patch by Kai Huang <kai.huang@intel.com> [2]
> 
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> Reviewed-by: Chao Gao <chao.gao@intel.com>
> Cc: Chao Gao <chao.gao@intel.com>
> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Cc: Kai Huang <kai.huang@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Link: https://lore.kernel.org/all/CAGtprH8eXwi-TcH2+-Fo5YdbEwGmgLBh9ggcDvd6N=bsKEJ_WQ@mail.gmail.com/ # [1]
> Link: https://lore.kernel.org/all/6b5553756f56a8e3222bfc36d0bdb3e5192137b7.1731318868.git.kai.huang@intel.com # [2]
> ---

Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

^ permalink raw reply

* [PATCH v2 2/2] x86/virt/tdx: Print TDX module version during init
From: Vishal Verma @ 2026-01-09 19:14 UTC (permalink / raw)
  To: linux-kernel, linux-coco, kvm
  Cc: x86, Chao Gao, Dan Williams, Kai Huang, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin,
	Kiryl Shutsemau, Rick Edgecombe, Vishal Verma
In-Reply-To: <20260109-tdx_print_module_version-v2-0-e10e4ca5b450@intel.com>

It is useful to print the TDX module version in dmesg logs. This is
currently the only way to determine the module version from the host. It
also creates a record for any future problems being investigated. This
was also requested in [1].

Include the version in the log messages during init, e.g.:

  virt/tdx: TDX module version: 1.5.24
  virt/tdx: 1034220 KB allocated for PAMT
  virt/tdx: module initialized

Print the version in get_tdx_sys_info(), right after the version
metadata is read, which makes it available even if there are subsequent
initialization failures.

Based on a patch by Kai Huang <kai.huang@intel.com> [2]

Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
Reviewed-by: Chao Gao <chao.gao@intel.com>
Cc: Chao Gao <chao.gao@intel.com>
Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: Kai Huang <kai.huang@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Link: https://lore.kernel.org/all/CAGtprH8eXwi-TcH2+-Fo5YdbEwGmgLBh9ggcDvd6N=bsKEJ_WQ@mail.gmail.com/ # [1]
Link: https://lore.kernel.org/all/6b5553756f56a8e3222bfc36d0bdb3e5192137b7.1731318868.git.kai.huang@intel.com # [2]
---
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 0454124803f3..4c9917a9c2c3 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -105,6 +105,12 @@ static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 	int ret = 0;
 
 	ret = ret ?: get_tdx_sys_info_version(&sysinfo->version);
+
+	pr_info("Module version: %u.%u.%02u\n",
+		sysinfo->version.major_version,
+		sysinfo->version.minor_version,
+		sysinfo->version.update_version);
+
 	ret = ret ?: get_tdx_sys_info_features(&sysinfo->features);
 	ret = ret ?: get_tdx_sys_info_tdmr(&sysinfo->tdmr);
 	ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);

-- 
2.52.0


^ permalink raw reply related

* [PATCH v2 1/2] x86/virt/tdx: Retrieve TDX module version
From: Vishal Verma @ 2026-01-09 19:14 UTC (permalink / raw)
  To: linux-kernel, linux-coco, kvm
  Cc: x86, Chao Gao, Dan Williams, Kai Huang, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin,
	Kiryl Shutsemau, Rick Edgecombe, Vishal Verma
In-Reply-To: <20260109-tdx_print_module_version-v2-0-e10e4ca5b450@intel.com>

From: Chao Gao <chao.gao@intel.com>

Each TDX module has several bits of metadata about which specific TDX
module it is. The primary bit of info is the version, which has an x.y.z
format. These represent the major version, minor version, and update
version respectively. Knowing the running TDX Module version is valuable
for bug reporting and debugging. Note that the module does expose other
pieces of version-related metadata, such as build number and date. Those
aren't retrieved for now, that can be added if needed in the future.

Retrieve the TDX Module version using the existing metadata reading
interface. Later changes will expose this information. The metadata
reading interfaces have existed for quite some time, so this will work
with older versions of the TDX module as well - i.e. this isn't a new
interface.

As a side note, the global metadata reading code was originally set up
to be auto-generated from a JSON definition [1]. However, later [2] this
was found to be unsustainable, and the autogeneration approach was
dropped in favor of just manually adding fields as needed (e.g. as in
this patch).

Signed-off-by: Chao Gao <chao.gao@intel.com>
Link: https://lore.kernel.org/kvm/CABgObfYXUxqQV_FoxKjC8U3t5DnyM45nz5DpTxYZv2x_uFK_Kw@mail.gmail.com/ # [1]
Link: https://lore.kernel.org/all/1e7bcbad-eb26-44b7-97ca-88ab53467212@intel.com/ # [2]
Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: Kai Huang <kai.huang@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Kiryl Shutsemau <kas@kernel.org>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 arch/x86/include/asm/tdx_global_metadata.h  |  7 +++++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 16 ++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
index 060a2ad744bff..40689c8dc67eb 100644
--- a/arch/x86/include/asm/tdx_global_metadata.h
+++ b/arch/x86/include/asm/tdx_global_metadata.h
@@ -5,6 +5,12 @@
 
 #include <linux/types.h>
 
+struct tdx_sys_info_version {
+	u16 minor_version;
+	u16 major_version;
+	u16 update_version;
+};
+
 struct tdx_sys_info_features {
 	u64 tdx_features0;
 };
@@ -35,6 +41,7 @@ struct tdx_sys_info_td_conf {
 };
 
 struct tdx_sys_info {
+	struct tdx_sys_info_version version;
 	struct tdx_sys_info_features features;
 	struct tdx_sys_info_tdmr tdmr;
 	struct tdx_sys_info_td_ctrl td_ctrl;
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 13ad2663488b1..0454124803f36 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -7,6 +7,21 @@
  * Include this file to other C file instead.
  */
 
+static int get_tdx_sys_info_version(struct tdx_sys_info_version *sysinfo_version)
+{
+	int ret = 0;
+	u64 val;
+
+	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000003, &val)))
+		sysinfo_version->minor_version = val;
+	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000004, &val)))
+		sysinfo_version->major_version = val;
+	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000005, &val)))
+		sysinfo_version->update_version = val;
+
+	return ret;
+}
+
 static int get_tdx_sys_info_features(struct tdx_sys_info_features *sysinfo_features)
 {
 	int ret = 0;
@@ -89,6 +104,7 @@ static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 {
 	int ret = 0;
 
+	ret = ret ?: get_tdx_sys_info_version(&sysinfo->version);
 	ret = ret ?: get_tdx_sys_info_features(&sysinfo->features);
 	ret = ret ?: get_tdx_sys_info_tdmr(&sysinfo->tdmr);
 	ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);

-- 
2.52.0


^ permalink raw reply related

* [PATCH v2 0/2] x86/virt/tdx: Print TDX module version to dmesg
From: Vishal Verma @ 2026-01-09 19:14 UTC (permalink / raw)
  To: linux-kernel, linux-coco, kvm
  Cc: x86, Chao Gao, Dan Williams, Kai Huang, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin,
	Kiryl Shutsemau, Rick Edgecombe, Vishal Verma

=== Problem & Solution ===

Currently, there is neither an ABI, nor any other way to determine from
the host system, what version of the TDX module is running. A sysfs ABI
for this has been proposed in [1], but it may need additional discussion.

Many/most TDX developers already carry patches like this in their
development branches. It can be tricky to know which TDX module is
actually loaded on a system, and so this functionality has been needed
regularly for development and processing bug reports. Hence, it is
prudent to break out the patches to retrieve and print the TDX module
version, as those parts are very straightforward, and get some level of
debugability and traceability for TDX host systems.

=== Dependencies ===

None. This is based on v6.19-rc4, and applies cleanly to tip.git.

=== Patch details ===

Patch 1 is a prerequisite that adds the infrastructure to retrieve the
TDX module version from its global metadata. This was originally posted in [2].

Patch 2 is based on a patch from Kai Huang [3], and prints the version to
dmesg during init.

=== Testing ===

This has passed the usual suite of tests, including successful 0day
builds, KVM Unit tests, KVM selftests, a TD creation smoke test, and
selected KVM tests from the Avocado test suite.

[1]: https://lore.kernel.org/all/20260105074350.98564-1-chao.gao@intel.com/
[2]: https://lore.kernel.org/all/20260105074350.98564-2-chao.gao@intel.com/
[3]: https://lore.kernel.org/all/57eaa1b17429315f8b5207774307f3c1dd40cf37.1730118186.git.kai.huang@intel.com/

Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
Changes in v2:
- Collect review tags (Kiryl, Rick)
- Reword commit messages for clarity (Rick)
- Move the version print get_tdx_sys_info() (Kiryl, Dave)
- Link to v1: https://patch.msgid.link/20260107-tdx_print_module_version-v1-0-822baa56762d@intel.com

---
Chao Gao (1):
      x86/virt/tdx: Retrieve TDX module version

Vishal Verma (1):
      x86/virt/tdx: Print TDX module version during init

 arch/x86/include/asm/tdx_global_metadata.h  |  7 +++++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 22 ++++++++++++++++++++++
 2 files changed, 29 insertions(+)
---
base-commit: 9ace4753a5202b02191d54e9fdf7f9e3d02b85eb
change-id: 20260107-tdx_print_module_version-e4ca7edc2022

Best regards,
--  
Vishal Verma <vishal.l.verma@intel.com>


^ permalink raw reply

* Re: [PATCH v4 04/16] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
From: Edgecombe, Rick P @ 2026-01-09 16:05 UTC (permalink / raw)
  To: yilun.xu@linux.intel.com
  Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
	Li, Xiaoyao, Hansen, Dave, Zhao, Yan Y, Wu, Binbin,
	kas@kernel.org, seanjc@google.com, mingo@redhat.com,
	pbonzini@redhat.com, tglx@linutronix.de, Yamahata, Isaku,
	linux-kernel@vger.kernel.org, kirill.shutemov@linux.intel.com,
	Annapurve, Vishal, Gao, Chao, bp@alien8.de, x86@kernel.org
In-Reply-To: <aWBlcCUvybAYWed8@yilunxu-OptiPlex-7050>

On Fri, 2026-01-09 at 10:18 +0800, Xu Yilun wrote:
> On the other hand, the cost of a newly designed firmware interface
> for an already online functionality is not low, especially when you
> want backward compatibility to old TDX Module. The worst case is we
> keep both sets of the code...

I think TDX module changes are something to consider long term. We
already discussed not overhauling the metadata reading again ahead of
the current work, so I don't think there is anything else to discuss
here.

^ permalink raw reply

* Re: [PATCH] dma-direct: swiotlb: Skip encryption toggles for swiotlb allocations
From: Aneesh Kumar K.V @ 2026-01-09  2:51 UTC (permalink / raw)
  To: Robin Murphy, iommu, linux-kernel, linux-coco
  Cc: Marek Szyprowski, steven.price, Suzuki K Poulose
In-Reply-To: <cddcaeff-9652-4685-ba6c-0fda79bd8680@arm.com>

Robin Murphy <robin.murphy@arm.com> writes:

> On 2026-01-02 3:54 pm, Aneesh Kumar K.V (Arm) wrote:
>> Swiotlb backing pages are already mapped decrypted via
>> swiotlb_update_mem_attributes(), so dma-direct does not need to call
>> set_memory_decrypted() during allocation or re-encrypt the memory on
>> free.
>> 
>> Handle swiotlb-backed buffers explicitly: obtain the DMA address and
>> zero the linear mapping for lowmem pages, and bypass the decrypt/encrypt
>> transitions when allocating/freeing from the swiotlb pool (detected via
>> swiotlb_find_pool()).
>
> swiotlb_update_mem_attributes() only applies to the default SWIOTLB 
> buffer, while the dma_direct_alloc_swiotlb() path is only for private 
> restricted pools (because the whole point is that restricted DMA devices 
> cannot use the regular allocator/default pools). There is no redundancy 
> here AFAICS.
>

But rmem_swiotlb_device_init() is also marking the entire pool decrypted

	set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
			     rmem->size >> PAGE_SHIFT);

-aneesh

>
> Thanks,
> Robin.
>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>>   kernel/dma/direct.c | 56 +++++++++++++++++++++++++++++++++++++--------
>>   1 file changed, 46 insertions(+), 10 deletions(-)
>> 
>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index faf1e41afde8..c4ef4457bd74 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -104,15 +104,27 @@ static void __dma_direct_free_pages(struct device *dev, struct page *page,
>>   	dma_free_contiguous(dev, page, size);
>>   }
>>   
>> -static struct page *dma_direct_alloc_swiotlb(struct device *dev, size_t size)
>> +static struct page *dma_direct_alloc_swiotlb(struct device *dev, size_t size,
>> +					     dma_addr_t *dma_handle)
>>   {
>> -	struct page *page = swiotlb_alloc(dev, size);
>> +	void *lm_addr;
>> +	struct page *page;
>> +
>> +	page = swiotlb_alloc(dev, size);
>> +	if (!page)
>> +		return NULL;
>>   
>> -	if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
>> +	if (!dma_coherent_ok(dev, page_to_phys(page), size)) {
>>   		swiotlb_free(dev, page, size);
>>   		return NULL;
>>   	}
>> +	/* If HighMem let caller take care of creating a mapping */
>> +	if (PageHighMem(page))
>> +		return page;
>>   
>> +	lm_addr = page_address(page);
>> +	memset(lm_addr, 0, size);
>> +	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
>>   	return page;
>>   }
>>   
>> @@ -125,9 +137,6 @@ static struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
>>   
>>   	WARN_ON_ONCE(!PAGE_ALIGNED(size));
>>   
>> -	if (is_swiotlb_for_alloc(dev))
>> -		return dma_direct_alloc_swiotlb(dev, size);
>> -
>>   	gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
>>   	page = dma_alloc_contiguous(dev, size, gfp);
>>   	if (page) {
>> @@ -204,6 +213,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>>   		dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
>>   {
>>   	bool remap = false, set_uncached = false;
>> +	bool mark_mem_decrypt = true;
>>   	bool allow_highmem = true;
>>   	struct page *page;
>>   	void *ret;
>> @@ -251,6 +261,14 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>>   	    dma_direct_use_pool(dev, gfp))
>>   		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>>   
>> +	if (is_swiotlb_for_alloc(dev)) {
>> +		page = dma_direct_alloc_swiotlb(dev, size, dma_handle);
>> +		if (page) {
>> +			mark_mem_decrypt = false;
>> +			goto setup_page;
>> +		}
>> +		return NULL;
>> +	}
>>   
>>   	if (force_dma_unencrypted(dev))
>>   		/*
>> @@ -266,6 +284,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>>   	if (!page)
>>   		return NULL;
>>   
>> +setup_page:
>>   	/*
>>   	 * dma_alloc_contiguous can return highmem pages depending on a
>>   	 * combination the cma= arguments and per-arch setup.  These need to be
>> @@ -295,7 +314,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>>   		ret = page_address(page);
>>   	}
>>   
>> -	if (force_dma_unencrypted(dev)) {
>> +	if (mark_mem_decrypt && force_dma_unencrypted(dev)) {
>>   		void *lm_addr;
>>   
>>   		lm_addr = page_address(page);
>> @@ -316,7 +335,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>>   	return ret;
>>   
>>   out_encrypt_pages:
>> -	if (dma_set_encrypted(dev, page_address(page), size))
>> +	if (mark_mem_decrypt && dma_set_encrypted(dev, page_address(page), size))
>>   		return NULL;
>>   out_free_pages:
>>   	__dma_direct_free_pages(dev, page, size);
>> @@ -328,6 +347,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>>   void dma_direct_free(struct device *dev, size_t size,
>>   		void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
>>   {
>> +	bool mark_mem_encrypted = true;
>>   	unsigned int page_order = get_order(size);
>>   
>>   	if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
>> @@ -356,6 +376,9 @@ void dma_direct_free(struct device *dev, size_t size,
>>   	    dma_free_from_pool(dev, cpu_addr, PAGE_ALIGN(size)))
>>   		return;
>>   
>> +	if (swiotlb_find_pool(dev, dma_to_phys(dev, dma_addr)))
>> +		mark_mem_encrypted = false;
>> +
>>   	if (is_vmalloc_addr(cpu_addr)) {
>>   		vunmap(cpu_addr);
>>   	} else {
>> @@ -363,7 +386,7 @@ void dma_direct_free(struct device *dev, size_t size,
>>   			arch_dma_clear_uncached(cpu_addr, size);
>>   	}
>>   
>> -	if (force_dma_unencrypted(dev)) {
>> +	if (mark_mem_encrypted && force_dma_unencrypted(dev)) {
>>   		void *lm_addr;
>>   
>>   		lm_addr = phys_to_virt(dma_to_phys(dev, dma_addr));
>> @@ -385,6 +408,15 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
>>   	if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
>>   		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>>   
>> +	if (is_swiotlb_for_alloc(dev)) {
>> +		page = dma_direct_alloc_swiotlb(dev, size, dma_handle);
>> +		if (page && PageHighMem(page)) {
>> +			swiotlb_free(dev, page, size);
>> +			return NULL;
>> +		}
>> +		return page;
>> +	}
>> +
>>   	page = __dma_direct_alloc_pages(dev, size, gfp, false);
>>   	if (!page)
>>   		return NULL;
>> @@ -404,13 +436,17 @@ void dma_direct_free_pages(struct device *dev, size_t size,
>>   		enum dma_data_direction dir)
>>   {
>>   	void *vaddr = page_address(page);
>> +	bool mark_mem_encrypted = true;
>>   
>>   	/* If cpu_addr is not from an atomic pool, dma_free_from_pool() fails */
>>   	if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
>>   	    dma_free_from_pool(dev, vaddr, size))
>>   		return;
>>   
>> -	if (dma_set_encrypted(dev, vaddr, size))
>> +	if (swiotlb_find_pool(dev, page_to_phys(page)))
>> +		mark_mem_encrypted = false;
>> +
>> +	if (mark_mem_encrypted && dma_set_encrypted(dev, vaddr, size))
>>   		return;
>>   	__dma_direct_free_pages(dev, page, size);
>>   }

^ permalink raw reply

* Re: [PATCH v4 04/16] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
From: Xu Yilun @ 2026-01-09  2:18 UTC (permalink / raw)
  To: Edgecombe, Rick P
  Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
	Li, Xiaoyao, Hansen, Dave, Zhao, Yan Y, Wu, Binbin,
	kas@kernel.org, seanjc@google.com, mingo@redhat.com,
	pbonzini@redhat.com, tglx@linutronix.de, Yamahata, Isaku,
	linux-kernel@vger.kernel.org, kirill.shutemov@linux.intel.com,
	Annapurve, Vishal, Gao, Chao, bp@alien8.de, x86@kernel.org
In-Reply-To: <b4af0f9795d69fdc1f6599032335a2103c2fe29a.camel@intel.com>

On Thu, Jan 08, 2026 at 04:52:10PM +0000, Edgecombe, Rick P wrote:
> On Thu, 2026-01-08 at 20:53 +0800, Xu Yilun wrote:
> > I actually don't understand why a RDALL seamcall could eliminate
> > the check "if (some_optional_feature_exists) read_it;". IIUC, The
> > check
> > exists because kernel doesn't trust TDX Module so kernel wants to
> > verify
> > the correctness/consistency of the data, otherwise we could accept
> > whatever TDX Module tells us, do the below for each field:
> > 
> >   static int read_sys_metadata_field(u64 field_id, u64 *data)
> >   {
> > 	...
> > 	ret = seamcall(TDH_SYS_RD, &args);
> > 	if (ret == TDX_SUCCESS) {
> > 		*data = args.r8;
> > 		return 0;
> > 	}
> > 
> > 	/* The field doesn't exist */
> > 	if (ret == TDX_METADATA_FIELD_ID_INCORRECT) {
> > 		*data = 0;
> > 		return 0;
> > 	}
> > 
> > 	...
> > 
> > 	/* Real reading error */
> > 	return -EFAULT;
> >   }
> > 
> > The trustness doesn't change no matter how kernel retrieves these
> > data,
> > by a series of RD or a RDALL.
> 
> Having it be field specific behavior (like the diff I posted) means we
> don't need to worry about TDX module bugs where some field read fails
> and we don't catch it.

We still need this specific behavior when we bulk read. Can you accept
a successfully returned blob with TDX_FEATURES0_DYNAMIC_PAMT set but
pamt_page_bitmap_entry_bits field missing? I assume no, so we still
need:

	if (blob->tdx_feature0 & TDX_FEATURES0_DYNAMIC_PAMT)
		check blob->pamt_page_bitmap_entry_bits validity;

BTW: ret == TDX_METADATA_FIELD_ID_INCORRECT is not something bad, TDX
Module is effectively telling us the field doesn't exist.

> 
> By RDALL, I mean a simpler way to bulk read the metadata. So for future

I understand. But as I mentioned that has nothing to do with the
optional feature checking.

> looking changes, let's think about what we need and not try to find yet
> more clever ways to code around the current interface. The amount of

I don't think so, cause I don't see much benifit bulk reading could
bring to us. Bulk reading basically retrieves an _unverified_ blob from
TDX Module. I believe it could also been achieved by a simple iteration
of existing single reads. The major work, data verification, is still
there.

On the other hand, the cost of a newly designed firmware interface for
an already online functionality is not low, especially when you want
backward compatibility to old TDX Module. The worst case is we keep both
sets of the code...

> code and discussion on TDX metadata reading is just too high. Please go
> back and look at the earlier threads if you haven't yet.

I've read most of the threads before my first posting on this topic.
Most of the discussion/effort focus on the effective data verification.
But I haven't found any decisive evidence that bulk reading could
contribute on it.

^ permalink raw reply

* Re: [PATCH] coco/guest: Remove unneeded selection of CRYPTO
From: Eric Biggers @ 2026-01-09  2:26 UTC (permalink / raw)
  To: Dan Williams, linux-coco
In-Reply-To: <20251204055512.494013-1-ebiggers@kernel.org>

On Wed, Dec 03, 2025 at 09:55:12PM -0800, Eric Biggers wrote:
> All that's needed here is CRYPTO_HASH_INFO.  It used to be the case that
> CRYPTO_HASH_INFO was visible only when CRYPTO, but that was fixed by
> commit aacb37f597d0 ("lib/crypto: hash_info: Move hash_info.c into
> lib/crypto/").  Now CRYPTO_HASH_INFO can be selected directly.
> 
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
>  drivers/virt/coco/guest/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/virt/coco/guest/Kconfig b/drivers/virt/coco/guest/Kconfig
> index 3d5e1d05bf34..da570dc4bd48 100644
> --- a/drivers/virt/coco/guest/Kconfig
> +++ b/drivers/virt/coco/guest/Kconfig
> @@ -11,7 +11,6 @@ config TSM_REPORTS
>  	tristate
>  
>  config TSM_MEASUREMENTS
>  	select TSM_GUEST
>  	select CRYPTO_HASH_INFO
> -	select CRYPTO
>  	bool
> 

Any interest in applying this patch?

- Eric

^ permalink raw reply

* [PATCH v3 6/6] KVM: guest_memfd: GUP source pages prior to populating guest memory
From: Michael Roth @ 2026-01-08 21:46 UTC (permalink / raw)
  To: kvm
  Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
	seanjc, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
	ackerleytng, aik, ira.weiny, yan.y.zhao, pankaj.gupta, Kai Huang
In-Reply-To: <20260108214622.1084057-1-michael.roth@amd.com>

Currently the post-populate callbacks handle copying source pages into
private GPA ranges backed by guest_memfd, where kvm_gmem_populate()
acquires the filemap invalidate lock, then calls a post-populate
callback which may issue a get_user_pages() on the source pages prior to
copying them into the private GPA (e.g. TDX).

This will not be compatible with in-place conversion, where the
userspace page fault path will attempt to acquire the filemap invalidate
lock while holding the mm->mmap_lock, leading to a potential ABBA
deadlock.

Address this by hoisting the GUP above the filemap invalidate lock so
that these page faults path can be taken early, prior to acquiring the
filemap invalidate lock.

It's not currently clear whether this issue is reachable with the
current implementation of guest_memfd, which doesn't support in-place
conversion, however it does provide a consistent mechanism to provide
stable source/target PFNs to callbacks rather than punting to
vendor-specific code, which allows for more commonality across
architectures, which may be worthwhile even without in-place conversion.

As part of this change, also begin enforcing that the 'src' argument to
kvm_gmem_populate() must be page-aligned, as this greatly reduces the
complexity around how the post-populate callbacks are implemented, and
since no current in-tree users support using a non-page-aligned 'src'
argument.

Suggested-by: Sean Christopherson <seanjc@google.com>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Co-developed-by: Vishal Annapurve <vannapurve@google.com>
Signed-off-by: Vishal Annapurve <vannapurve@google.com>
Tested-by: Vishal Annapurve <vannapurve@google.com>
Tested-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 arch/x86/kvm/svm/sev.c   | 33 ++++++++--------
 arch/x86/kvm/vmx/tdx.c   | 16 ++------
 include/linux/kvm_host.h |  4 +-
 virt/kvm/guest_memfd.c   | 84 +++++++++++++++++++++++++++-------------
 4 files changed, 79 insertions(+), 58 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index b4409bc652d1..0ab7c89262fb 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2268,7 +2268,7 @@ struct sev_gmem_populate_args {
 };
 
 static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
-				  void __user *src, void *opaque)
+				  struct page *src_page, void *opaque)
 {
 	struct sev_gmem_populate_args *sev_populate_args = opaque;
 	struct sev_data_snp_launch_update fw_args = {0};
@@ -2277,7 +2277,7 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
 	int level;
 	int ret;
 
-	if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src))
+	if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src_page))
 		return -EINVAL;
 
 	ret = snp_lookup_rmpentry((u64)pfn, &assigned, &level);
@@ -2288,15 +2288,14 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
 		goto out;
 	}
 
-	if (src) {
-		void *vaddr = kmap_local_pfn(pfn);
+	if (src_page) {
+		void *src_vaddr = kmap_local_page(src_page);
+		void *dst_vaddr = kmap_local_pfn(pfn);
 
-		if (copy_from_user(vaddr, src, PAGE_SIZE)) {
-			kunmap_local(vaddr);
-			ret = -EFAULT;
-			goto out;
-		}
-		kunmap_local(vaddr);
+		memcpy(dst_vaddr, src_vaddr, PAGE_SIZE);
+
+		kunmap_local(src_vaddr);
+		kunmap_local(dst_vaddr);
 	}
 
 	ret = rmp_make_private(pfn, gfn << PAGE_SHIFT, PG_LEVEL_4K,
@@ -2326,17 +2325,19 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
 	if (ret && !snp_page_reclaim(kvm, pfn) &&
 	    sev_populate_args->type == KVM_SEV_SNP_PAGE_TYPE_CPUID &&
 	    sev_populate_args->fw_error == SEV_RET_INVALID_PARAM) {
-		void *vaddr = kmap_local_pfn(pfn);
+		void *src_vaddr = kmap_local_page(src_page);
+		void *dst_vaddr = kmap_local_pfn(pfn);
 
-		if (copy_to_user(src, vaddr, PAGE_SIZE))
-			pr_debug("Failed to write CPUID page back to userspace\n");
+		memcpy(src_vaddr, dst_vaddr, PAGE_SIZE);
 
-		kunmap_local(vaddr);
+		kunmap_local(src_vaddr);
+		kunmap_local(dst_vaddr);
 	}
 
 out:
-	pr_debug("%s: exiting with return code %d (fw_error %d)\n",
-		 __func__, ret, sev_populate_args->fw_error);
+	if (ret)
+		pr_debug("%s: error updating GFN %llx, return code %d (fw_error %d)\n",
+			 __func__, gfn, ret, sev_populate_args->fw_error);
 	return ret;
 }
 
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 4fb042ce8ed1..5df9d32d2058 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -3118,34 +3118,24 @@ struct tdx_gmem_post_populate_arg {
 };
 
 static int tdx_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
-				  void __user *src, void *_arg)
+				  struct page *src_page, void *_arg)
 {
 	struct tdx_gmem_post_populate_arg *arg = _arg;
 	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
 	u64 err, entry, level_state;
 	gpa_t gpa = gfn_to_gpa(gfn);
-	struct page *src_page;
 	int ret, i;
 
 	if (KVM_BUG_ON(kvm_tdx->page_add_src, kvm))
 		return -EIO;
 
-	/*
-	 * Get the source page if it has been faulted in. Return failure if the
-	 * source page has been swapped out or unmapped in primary memory.
-	 */
-	ret = get_user_pages_fast((unsigned long)src, 1, 0, &src_page);
-	if (ret < 0)
-		return ret;
-	if (ret != 1)
-		return -ENOMEM;
+	if (!src_page)
+		return -EOPNOTSUPP;
 
 	kvm_tdx->page_add_src = src_page;
 	ret = kvm_tdp_mmu_map_private_pfn(arg->vcpu, gfn, pfn);
 	kvm_tdx->page_add_src = NULL;
 
-	put_page(src_page);
-
 	if (ret || !(arg->flags & KVM_TDX_MEASURE_MEMORY_REGION))
 		return ret;
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 1d0cee72e560..49c0cfe24fd8 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2566,7 +2566,7 @@ int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_ord
  * @gfn: starting GFN to be populated
  * @src: userspace-provided buffer containing data to copy into GFN range
  *       (passed to @post_populate, and incremented on each iteration
- *       if not NULL)
+ *       if not NULL). Must be page-aligned.
  * @npages: number of pages to copy from userspace-buffer
  * @post_populate: callback to issue for each gmem page that backs the GPA
  *                 range
@@ -2581,7 +2581,7 @@ int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_ord
  * Returns the number of pages that were populated.
  */
 typedef int (*kvm_gmem_populate_cb)(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
-				    void __user *src, void *opaque);
+				    struct page *page, void *opaque);
 
 long kvm_gmem_populate(struct kvm *kvm, gfn_t gfn, void __user *src, long npages,
 		       kvm_gmem_populate_cb post_populate, void *opaque);
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 8b1248f42aae..18ae59b92257 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -820,12 +820,48 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_get_pfn);
 
 #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_POPULATE
+
+static long __kvm_gmem_populate(struct kvm *kvm, struct kvm_memory_slot *slot,
+				struct file *file, gfn_t gfn, struct page *src_page,
+				kvm_gmem_populate_cb post_populate, void *opaque)
+{
+	pgoff_t index = kvm_gmem_get_index(slot, gfn);
+	struct folio *folio;
+	kvm_pfn_t pfn;
+	int ret;
+
+	filemap_invalidate_lock(file->f_mapping);
+
+	folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, NULL);
+	if (IS_ERR(folio)) {
+		ret = PTR_ERR(folio);
+		goto out_unlock;
+	}
+
+	folio_unlock(folio);
+
+	if (!kvm_range_has_memory_attributes(kvm, gfn, gfn + 1,
+					     KVM_MEMORY_ATTRIBUTE_PRIVATE,
+					     KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
+		ret = -EINVAL;
+		goto out_put_folio;
+	}
+
+	ret = post_populate(kvm, gfn, pfn, src_page, opaque);
+	if (!ret)
+		folio_mark_uptodate(folio);
+
+out_put_folio:
+	folio_put(folio);
+out_unlock:
+	filemap_invalidate_unlock(file->f_mapping);
+	return ret;
+}
+
 long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long npages,
 		       kvm_gmem_populate_cb post_populate, void *opaque)
 {
 	struct kvm_memory_slot *slot;
-	void __user *p;
-
 	int ret = 0;
 	long i;
 
@@ -834,6 +870,9 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
 	if (WARN_ON_ONCE(npages <= 0))
 		return -EINVAL;
 
+	if (WARN_ON_ONCE(!PAGE_ALIGNED(src)))
+		return -EINVAL;
+
 	slot = gfn_to_memslot(kvm, start_gfn);
 	if (!kvm_slot_has_gmem(slot))
 		return -EINVAL;
@@ -842,47 +881,38 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
 	if (!file)
 		return -EFAULT;
 
-	filemap_invalidate_lock(file->f_mapping);
-
 	npages = min_t(ulong, slot->npages - (start_gfn - slot->base_gfn), npages);
 	for (i = 0; i < npages; i++) {
-		struct folio *folio;
-		gfn_t gfn = start_gfn + i;
-		pgoff_t index = kvm_gmem_get_index(slot, gfn);
-		kvm_pfn_t pfn;
+		struct page *src_page = NULL;
+		void __user *p;
 
 		if (signal_pending(current)) {
 			ret = -EINTR;
 			break;
 		}
 
-		folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, NULL);
-		if (IS_ERR(folio)) {
-			ret = PTR_ERR(folio);
-			break;
-		}
+		p = src ? src + i * PAGE_SIZE : NULL;
 
-		folio_unlock(folio);
+		if (p) {
+			ret = get_user_pages_fast((unsigned long)p, 1, 0, &src_page);
+			if (ret < 0)
+				break;
+			if (ret != 1) {
+				ret = -ENOMEM;
+				break;
+			}
+		}
 
-		ret = -EINVAL;
-		if (!kvm_range_has_memory_attributes(kvm, gfn, gfn + 1,
-						     KVM_MEMORY_ATTRIBUTE_PRIVATE,
-						     KVM_MEMORY_ATTRIBUTE_PRIVATE))
-			goto put_folio_and_exit;
+		ret = __kvm_gmem_populate(kvm, slot, file, start_gfn + i, src_page,
+					  post_populate, opaque);
 
-		p = src ? src + i * PAGE_SIZE : NULL;
-		ret = post_populate(kvm, gfn, pfn, p, opaque);
-		if (!ret)
-			folio_mark_uptodate(folio);
+		if (src_page)
+			put_page(src_page);
 
-put_folio_and_exit:
-		folio_put(folio);
 		if (ret)
 			break;
 	}
 
-	filemap_invalidate_unlock(file->f_mapping);
-
 	return ret && !i ? ret : i;
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_populate);
-- 
2.25.1


^ permalink raw reply related

* [PATCH v3 5/6] KVM: TDX: Document alignment requirements for KVM_TDX_INIT_MEM_REGION
From: Michael Roth @ 2026-01-08 21:46 UTC (permalink / raw)
  To: kvm
  Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
	seanjc, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
	ackerleytng, aik, ira.weiny, yan.y.zhao, pankaj.gupta, Kai Huang
In-Reply-To: <20260108214622.1084057-1-michael.roth@amd.com>

Since it was never possible to use a non-PAGE_SIZE-aligned @source_addr,
go ahead and document this as a requirement. This is in preparation for
enforcing page-aligned @source_addr for all architectures in
guest_memfd.

Reviewed-by: Vishal Annapurve <vannapurve@google.com>
Tested-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 Documentation/virt/kvm/x86/intel-tdx.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/virt/kvm/x86/intel-tdx.rst b/Documentation/virt/kvm/x86/intel-tdx.rst
index 5efac62c92c7..6a222e9d0954 100644
--- a/Documentation/virt/kvm/x86/intel-tdx.rst
+++ b/Documentation/virt/kvm/x86/intel-tdx.rst
@@ -156,7 +156,7 @@ KVM_TDX_INIT_MEM_REGION
 :Returns: 0 on success, <0 on error
 
 Initialize @nr_pages TDX guest private memory starting from @gpa with userspace
-provided data from @source_addr.
+provided data from @source_addr. @source_addr must be PAGE_SIZE-aligned.
 
 Note, before calling this sub command, memory attribute of the range
 [gpa, gpa + nr_pages] needs to be private.  Userspace can use
-- 
2.25.1


^ permalink raw reply related

* [PATCH v3 4/6] KVM: SEV: Document/enforce page-alignment for KVM_SEV_SNP_LAUNCH_UPDATE
From: Michael Roth @ 2026-01-08 21:46 UTC (permalink / raw)
  To: kvm
  Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
	seanjc, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
	ackerleytng, aik, ira.weiny, yan.y.zhao, pankaj.gupta, Kai Huang
In-Reply-To: <20260108214622.1084057-1-michael.roth@amd.com>

In the past, KVM_SEV_SNP_LAUNCH_UPDATE accepted a non-page-aligned
'uaddr' parameter to copy data from, but continuing to support this with
new functionality like in-place conversion and hugepages in the pipeline
has proven to be more trouble than it is worth, since there are no known
users that have been identified who use a non-page-aligned 'uaddr'
parameter.

Rather than locking guest_memfd into continuing to support this, go
ahead and document page-alignment as a requirement and begin enforcing
this in the handling function.

Reviewed-by: Vishal Annapurve <vannapurve@google.com>
Tested-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 Documentation/virt/kvm/x86/amd-memory-encryption.rst | 2 +-
 arch/x86/kvm/svm/sev.c                               | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/virt/kvm/x86/amd-memory-encryption.rst b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
index 1ddb6a86ce7f..5a88d0197cb3 100644
--- a/Documentation/virt/kvm/x86/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
@@ -523,7 +523,7 @@ Returns: 0 on success, < 0 on error, -EAGAIN if caller should retry
 
         struct kvm_sev_snp_launch_update {
                 __u64 gfn_start;        /* Guest page number to load/encrypt data into. */
-                __u64 uaddr;            /* Userspace address of data to be loaded/encrypted. */
+                __u64 uaddr;            /* 4k-aligned address of data to be loaded/encrypted. */
                 __u64 len;              /* 4k-aligned length in bytes to copy into guest memory.*/
                 __u8 type;              /* The type of the guest pages being initialized. */
                 __u8 pad0;
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index a70bd3f19e29..b4409bc652d1 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2367,6 +2367,11 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	     params.type != KVM_SEV_SNP_PAGE_TYPE_CPUID))
 		return -EINVAL;
 
+	src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL : u64_to_user_ptr(params.uaddr);
+
+	if (!PAGE_ALIGNED(src))
+		return -EINVAL;
+
 	npages = params.len / PAGE_SIZE;
 
 	/*
@@ -2398,7 +2403,6 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
 
 	sev_populate_args.sev_fd = argp->sev_fd;
 	sev_populate_args.type = params.type;
-	src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL : u64_to_user_ptr(params.uaddr);
 
 	count = kvm_gmem_populate(kvm, params.gfn_start, src, npages,
 				  sev_gmem_post_populate, &sev_populate_args);
-- 
2.25.1


^ permalink raw reply related

* [PATCH v3 3/6] KVM: guest_memfd: Remove preparation tracking
From: Michael Roth @ 2026-01-08 21:46 UTC (permalink / raw)
  To: kvm
  Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
	seanjc, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
	ackerleytng, aik, ira.weiny, yan.y.zhao, pankaj.gupta, Kai Huang
In-Reply-To: <20260108214622.1084057-1-michael.roth@amd.com>

guest_memfd currently uses the folio uptodate flag to track:

  1) whether or not a page has been cleared before initial usage
  2) whether or not the architecture hooks have been issued to put the
     page in a private state as defined by the architecture

In practice, 2) is only actually being tracked for SEV-SNP VMs, and
there do not seem to be any plans/reasons that would suggest this will
change in the future, so this additional tracking/complexity is not
really providing any general benefit to guest_memfd users. Future plans
around in-place conversion and hugepage support, where the per-folio
uptodate flag is planned to be used purely to track the initial clearing
of folios, whereas conversion operations could trigger multiple
transitions between 'prepared' and 'unprepared' and thus need separate
tracking, will make the burden of tracking this information within
guest_memfd even more complex, since preparation generally happens
during fault time, on the "read-side" of any global locks that might
protect state tracked by guest_memfd, and so may require more complex
locking schemes to allow for concurrent handling of page faults for
multiple vCPUs where the "preparedness" state tracked by guest_memfd
might need to be updated as part of handling the fault.

Instead of keeping this current/future complexity within guest_memfd for
what is essentially just SEV-SNP, just drop the tracking for 2) and have
the arch-specific preparation hooks get triggered unconditionally on
every fault so the arch-specific hooks can check the preparation state
directly and decide whether or not a folio still needs additional
preparation. In the case of SEV-SNP, the preparation state is already
checked again via the preparation hooks to avoid double-preparation, so
nothing extra needs to be done to update the handling of things there.

Reviewed-by: Vishal Annapurve <vannapurve@google.com>
Tested-by: Vishal Annapurve <vannapurve@google.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@amd.com>
Tested-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 virt/kvm/guest_memfd.c | 44 ++++++++++++------------------------------
 1 file changed, 12 insertions(+), 32 deletions(-)

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 9dafa44838fe..8b1248f42aae 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -76,11 +76,6 @@ static int __kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slo
 	return 0;
 }
 
-static inline void kvm_gmem_mark_prepared(struct folio *folio)
-{
-	folio_mark_uptodate(folio);
-}
-
 /*
  * Process @folio, which contains @gfn, so that the guest can use it.
  * The folio must be locked and the gfn must be contained in @slot.
@@ -90,13 +85,7 @@ static inline void kvm_gmem_mark_prepared(struct folio *folio)
 static int kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot,
 				  gfn_t gfn, struct folio *folio)
 {
-	unsigned long nr_pages, i;
 	pgoff_t index;
-	int r;
-
-	nr_pages = folio_nr_pages(folio);
-	for (i = 0; i < nr_pages; i++)
-		clear_highpage(folio_page(folio, i));
 
 	/*
 	 * Preparing huge folios should always be safe, since it should
@@ -114,11 +103,8 @@ static int kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot,
 	WARN_ON(!IS_ALIGNED(slot->gmem.pgoff, folio_nr_pages(folio)));
 	index = kvm_gmem_get_index(slot, gfn);
 	index = ALIGN_DOWN(index, folio_nr_pages(folio));
-	r = __kvm_gmem_prepare_folio(kvm, slot, index, folio);
-	if (!r)
-		kvm_gmem_mark_prepared(folio);
 
-	return r;
+	return __kvm_gmem_prepare_folio(kvm, slot, index, folio);
 }
 
 /*
@@ -429,7 +415,7 @@ static vm_fault_t kvm_gmem_fault_user_mapping(struct vm_fault *vmf)
 
 	if (!folio_test_uptodate(folio)) {
 		clear_highpage(folio_page(folio, 0));
-		kvm_gmem_mark_prepared(folio);
+		folio_mark_uptodate(folio);
 	}
 
 	vmf->page = folio_file_page(folio, vmf->pgoff);
@@ -766,7 +752,7 @@ void kvm_gmem_unbind(struct kvm_memory_slot *slot)
 static struct folio *__kvm_gmem_get_pfn(struct file *file,
 					struct kvm_memory_slot *slot,
 					pgoff_t index, kvm_pfn_t *pfn,
-					bool *is_prepared, int *max_order)
+					int *max_order)
 {
 	struct file *slot_file = READ_ONCE(slot->gmem.file);
 	struct gmem_file *f = file->private_data;
@@ -796,7 +782,6 @@ static struct folio *__kvm_gmem_get_pfn(struct file *file,
 	if (max_order)
 		*max_order = 0;
 
-	*is_prepared = folio_test_uptodate(folio);
 	return folio;
 }
 
@@ -806,19 +791,22 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
 {
 	pgoff_t index = kvm_gmem_get_index(slot, gfn);
 	struct folio *folio;
-	bool is_prepared = false;
 	int r = 0;
 
 	CLASS(gmem_get_file, file)(slot);
 	if (!file)
 		return -EFAULT;
 
-	folio = __kvm_gmem_get_pfn(file, slot, index, pfn, &is_prepared, max_order);
+	folio = __kvm_gmem_get_pfn(file, slot, index, pfn, max_order);
 	if (IS_ERR(folio))
 		return PTR_ERR(folio);
 
-	if (!is_prepared)
-		r = kvm_gmem_prepare_folio(kvm, slot, gfn, folio);
+	if (!folio_test_uptodate(folio)) {
+		clear_highpage(folio_page(folio, 0));
+		folio_mark_uptodate(folio);
+	}
+
+	r = kvm_gmem_prepare_folio(kvm, slot, gfn, folio);
 
 	folio_unlock(folio);
 
@@ -861,7 +849,6 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
 		struct folio *folio;
 		gfn_t gfn = start_gfn + i;
 		pgoff_t index = kvm_gmem_get_index(slot, gfn);
-		bool is_prepared = false;
 		kvm_pfn_t pfn;
 
 		if (signal_pending(current)) {
@@ -869,19 +856,12 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
 			break;
 		}
 
-		folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &is_prepared, NULL);
+		folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, NULL);
 		if (IS_ERR(folio)) {
 			ret = PTR_ERR(folio);
 			break;
 		}
 
-		if (is_prepared) {
-			folio_unlock(folio);
-			folio_put(folio);
-			ret = -EEXIST;
-			break;
-		}
-
 		folio_unlock(folio);
 
 		ret = -EINVAL;
@@ -893,7 +873,7 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
 		p = src ? src + i * PAGE_SIZE : NULL;
 		ret = post_populate(kvm, gfn, pfn, p, opaque);
 		if (!ret)
-			kvm_gmem_mark_prepared(folio);
+			folio_mark_uptodate(folio);
 
 put_folio_and_exit:
 		folio_put(folio);
-- 
2.25.1


^ permalink raw reply related

* [PATCH v3 2/6] KVM: guest_memfd: Remove partial hugepage handling from kvm_gmem_populate()
From: Michael Roth @ 2026-01-08 21:46 UTC (permalink / raw)
  To: kvm
  Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
	seanjc, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
	ackerleytng, aik, ira.weiny, yan.y.zhao, pankaj.gupta, Kai Huang
In-Reply-To: <20260108214622.1084057-1-michael.roth@amd.com>

kvm_gmem_populate(), and the associated post-populate callbacks, have
some limited support for dealing with guests backed by hugepages by
passing the order information along to each post-populate callback and
iterating through the pages passed to kvm_gmem_populate() in
hugepage-chunks.

However, guest_memfd doesn't yet support hugepages, and in most cases
additional changes in the kvm_gmem_populate() path would also be needed
to actually allow for this functionality.

This makes the existing code unecessarily complex, and makes changes
difficult to work through upstream due to theoretical impacts on
hugepage support that can't be considered properly without an actual
hugepage implementation to reference. So for now, remove what's there
so changes for things like in-place conversion can be
implemented/reviewed more efficiently.

Suggested-by: Vishal Annapurve <vannapurve@google.com>
Co-developed-by: Vishal Annapurve <vannapurve@google.com>
Signed-off-by: Vishal Annapurve <vannapurve@google.com>
Tested-by: Vishal Annapurve <vannapurve@google.com>
Tested-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 arch/x86/kvm/svm/sev.c   | 94 ++++++++++++++++------------------------
 arch/x86/kvm/vmx/tdx.c   |  2 +-
 include/linux/kvm_host.h |  2 +-
 virt/kvm/guest_memfd.c   | 30 +++++++------
 4 files changed, 56 insertions(+), 72 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 261d9ef8631b..a70bd3f19e29 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2267,67 +2267,53 @@ struct sev_gmem_populate_args {
 	int fw_error;
 };
 
-static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pfn,
-				  void __user *src, int order, void *opaque)
+static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
+				  void __user *src, void *opaque)
 {
 	struct sev_gmem_populate_args *sev_populate_args = opaque;
+	struct sev_data_snp_launch_update fw_args = {0};
 	struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
-	int n_private = 0, ret, i;
-	int npages = (1 << order);
-	gfn_t gfn;
+	bool assigned = false;
+	int level;
+	int ret;
 
 	if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src))
 		return -EINVAL;
 
-	for (gfn = gfn_start, i = 0; gfn < gfn_start + npages; gfn++, i++) {
-		struct sev_data_snp_launch_update fw_args = {0};
-		bool assigned = false;
-		int level;
-
-		ret = snp_lookup_rmpentry((u64)pfn + i, &assigned, &level);
-		if (ret || assigned) {
-			pr_debug("%s: Failed to ensure GFN 0x%llx RMP entry is initial shared state, ret: %d assigned: %d\n",
-				 __func__, gfn, ret, assigned);
-			ret = ret ? -EINVAL : -EEXIST;
-			goto err;
-		}
+	ret = snp_lookup_rmpentry((u64)pfn, &assigned, &level);
+	if (ret || assigned) {
+		pr_debug("%s: Failed to ensure GFN 0x%llx RMP entry is initial shared state, ret: %d assigned: %d\n",
+			 __func__, gfn, ret, assigned);
+		ret = ret ? -EINVAL : -EEXIST;
+		goto out;
+	}
 
-		if (src) {
-			void *vaddr = kmap_local_pfn(pfn + i);
+	if (src) {
+		void *vaddr = kmap_local_pfn(pfn);
 
-			if (copy_from_user(vaddr, src + i * PAGE_SIZE, PAGE_SIZE)) {
-				kunmap_local(vaddr);
-				ret = -EFAULT;
-				goto err;
-			}
+		if (copy_from_user(vaddr, src, PAGE_SIZE)) {
 			kunmap_local(vaddr);
+			ret = -EFAULT;
+			goto out;
 		}
-
-		ret = rmp_make_private(pfn + i, gfn << PAGE_SHIFT, PG_LEVEL_4K,
-				       sev_get_asid(kvm), true);
-		if (ret)
-			goto err;
-
-		n_private++;
-
-		fw_args.gctx_paddr = __psp_pa(sev->snp_context);
-		fw_args.address = __sme_set(pfn_to_hpa(pfn + i));
-		fw_args.page_size = PG_LEVEL_TO_RMP(PG_LEVEL_4K);
-		fw_args.page_type = sev_populate_args->type;
-
-		ret = __sev_issue_cmd(sev_populate_args->sev_fd, SEV_CMD_SNP_LAUNCH_UPDATE,
-				      &fw_args, &sev_populate_args->fw_error);
-		if (ret)
-			goto fw_err;
+		kunmap_local(vaddr);
 	}
 
-	return 0;
+	ret = rmp_make_private(pfn, gfn << PAGE_SHIFT, PG_LEVEL_4K,
+			       sev_get_asid(kvm), true);
+	if (ret)
+		goto out;
+
+	fw_args.gctx_paddr = __psp_pa(sev->snp_context);
+	fw_args.address = __sme_set(pfn_to_hpa(pfn));
+	fw_args.page_size = PG_LEVEL_TO_RMP(PG_LEVEL_4K);
+	fw_args.page_type = sev_populate_args->type;
 
-fw_err:
+	ret = __sev_issue_cmd(sev_populate_args->sev_fd, SEV_CMD_SNP_LAUNCH_UPDATE,
+			      &fw_args, &sev_populate_args->fw_error);
 	/*
 	 * If the firmware command failed handle the reclaim and cleanup of that
-	 * PFN specially vs. prior pages which can be cleaned up below without
-	 * needing to reclaim in advance.
+	 * PFN before reporting an error.
 	 *
 	 * Additionally, when invalid CPUID function entries are detected,
 	 * firmware writes the expected values into the page and leaves it
@@ -2337,26 +2323,20 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pf
 	 * information to provide information on which CPUID leaves/fields
 	 * failed CPUID validation.
 	 */
-	if (!snp_page_reclaim(kvm, pfn + i) &&
+	if (ret && !snp_page_reclaim(kvm, pfn) &&
 	    sev_populate_args->type == KVM_SEV_SNP_PAGE_TYPE_CPUID &&
 	    sev_populate_args->fw_error == SEV_RET_INVALID_PARAM) {
-		void *vaddr = kmap_local_pfn(pfn + i);
+		void *vaddr = kmap_local_pfn(pfn);
 
-		if (copy_to_user(src + i * PAGE_SIZE, vaddr, PAGE_SIZE))
+		if (copy_to_user(src, vaddr, PAGE_SIZE))
 			pr_debug("Failed to write CPUID page back to userspace\n");
 
 		kunmap_local(vaddr);
 	}
 
-	/* pfn + i is hypervisor-owned now, so skip below cleanup for it. */
-	n_private--;
-
-err:
-	pr_debug("%s: exiting with error ret %d (fw_error %d), restoring %d gmem PFNs to shared.\n",
-		 __func__, ret, sev_populate_args->fw_error, n_private);
-	for (i = 0; i < n_private; i++)
-		kvm_rmp_make_shared(kvm, pfn + i, PG_LEVEL_4K);
-
+out:
+	pr_debug("%s: exiting with return code %d (fw_error %d)\n",
+		 __func__, ret, sev_populate_args->fw_error);
 	return ret;
 }
 
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 2d7a4d52ccfb..4fb042ce8ed1 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -3118,7 +3118,7 @@ struct tdx_gmem_post_populate_arg {
 };
 
 static int tdx_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
-				  void __user *src, int order, void *_arg)
+				  void __user *src, void *_arg)
 {
 	struct tdx_gmem_post_populate_arg *arg = _arg;
 	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index d93f75b05ae2..1d0cee72e560 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2581,7 +2581,7 @@ int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_ord
  * Returns the number of pages that were populated.
  */
 typedef int (*kvm_gmem_populate_cb)(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
-				    void __user *src, int order, void *opaque);
+				    void __user *src, void *opaque);
 
 long kvm_gmem_populate(struct kvm *kvm, gfn_t gfn, void __user *src, long npages,
 		       kvm_gmem_populate_cb post_populate, void *opaque);
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index fdaea3422c30..9dafa44838fe 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -151,6 +151,15 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
 					 mapping_gfp_mask(inode->i_mapping), policy);
 	mpol_cond_put(policy);
 
+	/*
+	 * External interfaces like kvm_gmem_get_pfn() support dealing
+	 * with hugepages to a degree, but internally, guest_memfd currently
+	 * assumes that all folios are order-0 and handling would need
+	 * to be updated for anything otherwise (e.g. page-clearing
+	 * operations).
+	 */
+	WARN_ON_ONCE(folio_order(folio));
+
 	return folio;
 }
 
@@ -829,7 +838,7 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
 	struct kvm_memory_slot *slot;
 	void __user *p;
 
-	int ret = 0, max_order;
+	int ret = 0;
 	long i;
 
 	lockdep_assert_held(&kvm->slots_lock);
@@ -848,7 +857,7 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
 	filemap_invalidate_lock(file->f_mapping);
 
 	npages = min_t(ulong, slot->npages - (start_gfn - slot->base_gfn), npages);
-	for (i = 0; i < npages; i += (1 << max_order)) {
+	for (i = 0; i < npages; i++) {
 		struct folio *folio;
 		gfn_t gfn = start_gfn + i;
 		pgoff_t index = kvm_gmem_get_index(slot, gfn);
@@ -860,7 +869,7 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
 			break;
 		}
 
-		folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &is_prepared, &max_order);
+		folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &is_prepared, NULL);
 		if (IS_ERR(folio)) {
 			ret = PTR_ERR(folio);
 			break;
@@ -874,20 +883,15 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
 		}
 
 		folio_unlock(folio);
-		WARN_ON(!IS_ALIGNED(gfn, 1 << max_order) ||
-			(npages - i) < (1 << max_order));
 
 		ret = -EINVAL;
-		while (!kvm_range_has_memory_attributes(kvm, gfn, gfn + (1 << max_order),
-							KVM_MEMORY_ATTRIBUTE_PRIVATE,
-							KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
-			if (!max_order)
-				goto put_folio_and_exit;
-			max_order--;
-		}
+		if (!kvm_range_has_memory_attributes(kvm, gfn, gfn + 1,
+						     KVM_MEMORY_ATTRIBUTE_PRIVATE,
+						     KVM_MEMORY_ATTRIBUTE_PRIVATE))
+			goto put_folio_and_exit;
 
 		p = src ? src + i * PAGE_SIZE : NULL;
-		ret = post_populate(kvm, gfn, pfn, p, max_order, opaque);
+		ret = post_populate(kvm, gfn, pfn, p, opaque);
 		if (!ret)
 			kvm_gmem_mark_prepared(folio);
 
-- 
2.25.1


^ permalink raw reply related

* [PATCH v3 1/6] KVM: SVM: Fix a missing kunmap_local() in sev_gmem_post_populate()
From: Michael Roth @ 2026-01-08 21:46 UTC (permalink / raw)
  To: kvm
  Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
	seanjc, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
	ackerleytng, aik, ira.weiny, yan.y.zhao, pankaj.gupta
In-Reply-To: <20260108214622.1084057-1-michael.roth@amd.com>

From: Yan Zhao <yan.y.zhao@intel.com>

sev_gmem_post_populate() needs to unmap the target vaddr after
copy_from_user() to the vaddr fails.

Fixes: dee5a47cc7a4 ("KVM: SEV: Add KVM_SEV_SNP_LAUNCH_UPDATE command")
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 arch/x86/kvm/svm/sev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index f59c65abe3cf..261d9ef8631b 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2296,6 +2296,7 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pf
 			void *vaddr = kmap_local_pfn(pfn + i);
 
 			if (copy_from_user(vaddr, src + i * PAGE_SIZE, PAGE_SIZE)) {
+				kunmap_local(vaddr);
 				ret = -EFAULT;
 				goto err;
 			}
-- 
2.25.1


^ permalink raw reply related

* [PATCH v3 0/6] KVM: guest_memfd: Rework preparation/population flows in prep for in-place conversion
From: Michael Roth @ 2026-01-08 21:46 UTC (permalink / raw)
  To: kvm
  Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
	seanjc, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
	ackerleytng, aik, ira.weiny, yan.y.zhao, pankaj.gupta

This patchset is also available at:

  https://github.com/AMDESE/linux/tree/gmem-populate-rework-v3

and is based on top of kvm/next (0499add8efd7)


Overview
--------

Yan previously posted a series[1] that reworked kvm_gmem_populate() to deal
with potential locking issues that might arise once in-place conversion
support[2] is added for guest_memfd. To quote Yan's original summary of the
issues:

  (1)
  In Michael's series "KVM: gmem: 2MB THP support and preparedness tracking
  changes" [4], kvm_gmem_get_pfn() was modified to rely on the filemap
  invalidation lock for protecting its preparedness tracking. Similarly, the
  in-place conversion version of guest_memfd series by Ackerly also requires
  kvm_gmem_get_pfn() to acquire filemap invalidation lock [5].
  
  kvm_gmem_get_pfn
      filemap_invalidate_lock_shared(file_inode(file)->i_mapping);
  
  However, since kvm_gmem_get_pfn() is called by kvm_tdp_map_page(), which is
  in turn invoked within kvm_gmem_populate() in TDX, a deadlock occurs on the
  filemap invalidation lock.
  
  (2)
  Moreover, in step 2, get_user_pages_fast() may acquire mm->mmap_lock,
  resulting in the following lock sequence in tdx_vcpu_init_mem_region():
  - filemap invalidation lock --> mm->mmap_lock
  
  However, in future code, the shared filemap invalidation lock will be held
  in kvm_gmem_fault_shared() (see [6]), leading to the lock sequence:
  - mm->mmap_lock --> filemap invalidation lock
  
  This creates an AB-BA deadlock issue.

Sean has since then addressed (1) with his series[3] that avoids relying on
calling kvm_gmem_get_pfn() within the TDX post-populate callback to re-fetch
the PFN that was passed to it.

This series aims to address (2), which is still outstanding, and does so based
heavily on Sean's suggested approach[4] of hoisting the get_user_pages_fast()
out of the TDX post-populate callback so that it can be called prior to taking
the filemap invalidate lock so that the ABBA deadlock is no longer possible.
As preperation for this change, all the partial enablement for hugepages in
the kvm_gmem_populate() path is stripped out so that it can be better
considered once hugepage support is actually in place and code/design can be
kept simpler in the meantime.

It additionally removes 'preparation' tracking from guest_memfd, which would
similarly complicate locking considerations in the context of in-place
conversion (and even moreso in the context of hugepage support). This has
been discussed during both the guest_memfd calls and PUCK calls, and so far
no strong objections have been given, so hopefully that particular change
isn't too controversial.


Some items worth noting/discussing
----------------------------------

(A) While one of the aims of this rework is to implement things such that
    a separate source address can still be passed to kvm_gmem_populate()
    even though the gmem pages can be populated in-place from userspace
    beforehand, issues still arise if the source address itself has the
    KVM_MEMORY_ATTRIBUTE_PRIVATE attribute set, e.g. if source/target
    addresses are the same page. One line of reasoning would be to
    conclude that KVM_MEMORY_ATTRIBUTE_PRIVATE implies that it cannot
    be used as the source of a GUP/copy_from_user(), and thus cases like
    source==target are naturally disallowed. Thus userspace has no choice
    but to populate pages in-place *prior* to setting the
    KVM_MEMORY_ATTRIBUTE_PRIVATE attribute (as kvm_gmem_populate()
    requires), and passing in NULL for the source such that the GUP can
    be skipped (otherwise, it will trigger the shared memory fault path,
    which will then SIGBUS because it will see that it is faulting in
    pages for which KVM_MEMORY_ATTRIBUTE_PRIVATE is set).

    While workable, this would at the very least involve documentation
    updates to KVM_TDX_INIT_MEM_REGION/KVM_SEV_SNP_LAUNCH_UPDATE to cover
    these soon-to-be-possible scenarios. Ira posted a patch separately
    that demonstrates how a NULL source could be safely handled within
    the TDX post-populate callback[5].


Changes since v2
----------------

- add a separate pre-patch from Yan to fix a missing kunmap_local() in SNP
  post-populate callback prior to refactoring the code rather than silently
  fixing as part of the refactoring (Yan)
- add a check that to error if src_page is NULL in TDX post-populate
  callback (Yan)
- use kmap_local_page() instead of kmap_local_pfn() when a struct page is
  already available (Kai)
- fixup commit msg for PATCH #6 (Kai)
- collect various Reviewed-by/Tested-by's and rebase to latest kvm/next


Changes since RFC v1
--------------------

- add a prep patch to remove partial hugepage enablement in
  kvm_gmem_populate() to simplify things until a hugepage implementation
  can actually make use of it (Yan, Ira, Vishal, Sean)
- begin retroactively enforcing that source pages must be page-aligned
  so that kvm_gmem_populate() callbacks can be simplified. add a patch
  to update SNP user-facing documentation to mention this.
- drop handling for GUP'ing multiple pages before issuing callbacks.
  This will only be needed for potentially for hugepages, and it must
  simpler to handle per-page in the meantime. (Yan, Vishal)
- make sure TDX actually builds (Ira, Yan)


Thanks,

Mike


[1] https://lore.kernel.org/kvm/20250703062641.3247-1-yan.y.zhao@intel.com/
[2] https://lore.kernel.org/kvm/cover.1760731772.git.ackerleytng@google.com/
[3] https://lore.kernel.org/kvm/20251030200951.3402865-1-seanjc@google.com/
[4] https://lore.kernel.org/kvm/aHEwT4X0RcfZzHlt@google.com/
[5] https://lore.kernel.org/kvm/20251105-tdx-init-in-place-v1-1-1196b67d0423@intel.com/


----------------------------------------------------------------
Michael Roth (5):
      KVM: guest_memfd: Remove partial hugepage handling from kvm_gmem_populate()
      KVM: guest_memfd: Remove preparation tracking
      KVM: SEV: Document/enforce page-alignment for KVM_SEV_SNP_LAUNCH_UPDATE
      KVM: TDX: Document alignment requirements for KVM_TDX_INIT_MEM_REGION
      KVM: guest_memfd: GUP source pages prior to populating guest memory

Yan Zhao (1):
      KVM: SVM: Fix a missing kunmap_local() in sev_gmem_post_populate()

 .../virt/kvm/x86/amd-memory-encryption.rst         |   2 +-
 Documentation/virt/kvm/x86/intel-tdx.rst           |   2 +-
 arch/x86/kvm/svm/sev.c                             | 108 +++++++---------
 arch/x86/kvm/vmx/tdx.c                             |  16 +--
 include/linux/kvm_host.h                           |   4 +-
 virt/kvm/guest_memfd.c                             | 140 +++++++++++----------
 6 files changed, 131 insertions(+), 141 deletions(-)



^ permalink raw reply

* Re: [PATCH v2 2/5] KVM: guest_memfd: Remove preparation tracking
From: Michael Roth @ 2026-01-08 21:38 UTC (permalink / raw)
  To: Huang, Kai
  Cc: kvm@vger.kernel.org, david@redhat.com, liam.merwick@oracle.com,
	seanjc@google.com, aik@amd.com, linux-mm@kvack.org,
	Annapurve, Vishal, linux-kernel@vger.kernel.org,
	thomas.lendacky@amd.com, vbabka@suse.cz, ashish.kalra@amd.com,
	linux-coco@lists.linux.dev, Weiny, Ira, pbonzini@redhat.com,
	ackerleytng@google.com, Zhao, Yan Y
In-Reply-To: <35e79cadf079622588ddb9fac0ccc985751dd81b.camel@intel.com>

On Thu, Dec 18, 2025 at 10:53:14PM +0000, Huang, Kai wrote:
> 
> >  /*
> >   * Process @folio, which contains @gfn, so that the guest can use it.
> >   * The folio must be locked and the gfn must be contained in @slot.
> > @@ -90,13 +85,7 @@ static inline void kvm_gmem_mark_prepared(struct folio *folio)
> >  static int kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot,
> >  				  gfn_t gfn, struct folio *folio)
> >  {
> > -	unsigned long nr_pages, i;
> >  	pgoff_t index;
> > -	int r;
> > -
> > -	nr_pages = folio_nr_pages(folio);
> > -	for (i = 0; i < nr_pages; i++)
> > -		clear_highpage(folio_page(folio, i));
> > 
> 
> Here the entire folio is cleared, but ...
> 
> [...]
> 
> > -	folio = __kvm_gmem_get_pfn(file, slot, index, pfn, &is_prepared, max_order);
> > +	folio = __kvm_gmem_get_pfn(file, slot, index, pfn, max_order);
> >  	if (IS_ERR(folio))
> >  		return PTR_ERR(folio);
> >  
> > -	if (!is_prepared)
> > -		r = kvm_gmem_prepare_folio(kvm, slot, gfn, folio);
> > +	if (!folio_test_uptodate(folio)) {
> > +		clear_highpage(folio_page(folio, 0));
> > +		folio_mark_uptodate(folio);
> > +	}
> 
> ... here only the first page is cleared.
> 
> I understand currently there's no huge folio coming out of gmem now, but
> since both __kvm_gmem_get_pfn() and kvm_gmem_get_pfn() still have
> @max_level as output, it's kinda inconsistent here.
> 
> I also see kvm_gmem_fault_user_mapping() only clears the first page too,
> but I think that already has assumption that folio can never be huge
> currently?
> 
> Given this, and the fact that the first patch of this series has
> introduced 
> 
> 	WARN_ON_ONCE(folio_order(folio));
> 
> in kvm_gmem_get_folio(), I think it's fine to only clear the first page,
> but for the sake of consistency, perhaps we should just remove @max_order
> from __kvm_gmem_get_pfn() and kvm_gmem_get_pfn()?
> 
> Then we can handle huge folio logic when that comes to play.

Sean had mentioned during the PUCK prior to this that he was okay with
stripping out traces of hugepage support from kvm_gmem_populate() path,
since it's bringing about unecessary complexity for a use-case we'll
potentially never support. We will however eventually support hugepages
outside the kvm_gmem_populate() path, and the bits and pieces of the
API that plumb those details into KVM MMU code are more useful to keep
around since there's existing hugepage support in KVM MMU that make it
clearer where/when we'll need it. So I'm not sure we gain much from the
churn of stripping it out. However, if as part of wiring up hugepage
support those interfaces prove insufficient, then I wouldn't be opposed
to similarly adding pre-patches to strip it out for a cleaner base
implementation, but I don't really think that need to be part fo this
series which is focused more on the population path rather than fault
handling at run-time, so I've left things as-is for v3 for now.

> 
> Btw:
> 
> I actually looked into the RFC v1 discussion but the code there actually
> does a loop to clear all pages in the folio.  There were some other

The thinking there was to try to not actively break the hugepage-related
bits that were already in place, but since we decided to implement
hugepage-related stuff for kvm_gmem_populate() as a clean follow-up
implementation, there's no need to consider the hugepage case and loop
through the page.

> discussions about AFAICT they were more related to issues regarding to 
> "mark entire folio as uptodate while only one page is processed in
> post_populate()".
> 
> Btw2:
> 
> There was also discussion that clearing page isn't required for TDX.  To
> that end, maybe we can remove clearing page from gmem common code but to
> SEV code, e.g., as part of "folio preparation"?

I think we are considering this approach for TDX and there was some
discussion of having gmem-internal flags to select for this type of
handling, but I think that would make more sense as part of TDX-specific
enablement of in-place conversion. I'm planning to post the SNP-specific
enablement of in-place conversion patches on top of this series, so
maybe we can consider this in response to that series or as part of the
TDX-specific enablement.

-Mike

^ permalink raw reply

* Re: [PATCH 2/2] x86/virt/tdx: Print TDX module version during init
From: Dave Hansen @ 2026-01-08 20:49 UTC (permalink / raw)
  To: Verma, Vishal L, kas@kernel.org
  Cc: Gao, Chao, Edgecombe, Rick P, dave.hansen@linux.intel.com,
	Huang, Kai, x86@kernel.org, bp@alien8.de,
	linux-kernel@vger.kernel.org, Williams, Dan J, tglx@linutronix.de,
	kvm@vger.kernel.org, linux-coco@lists.linux.dev, hpa@zytor.com,
	mingo@redhat.com
In-Reply-To: <261b253ff5bcf593adbddbc34f7a5b4befaa4c21.camel@intel.com>

On 1/8/26 10:39, Verma, Vishal L wrote:
>         ret = ret ?: get_tdx_sys_info_version(&sysinfo->version);
> +
> +       pr_info("Module version: %u.%u.%02u\n",
> +               sysinfo->version.major_version,
> +               sysinfo->version.minor_version,
> +               sysinfo->version.update_version);

This is wonky, but it's also fine.

If we can't even get the module version, we have pretty big problems on
our hands Seeing "Module version: 0.0.00" is a nice indication. It'll
almost certainly be followed by a bunch of other nasty messages, so one
wonky message before them will be a drop in the bucket.

^ permalink raw reply

* Re: [PATCH 2/2] x86/virt/tdx: Print TDX module version during init
From: Verma, Vishal L @ 2026-01-08 20:38 UTC (permalink / raw)
  To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
	linux-kernel@vger.kernel.org, Edgecombe, Rick P
  Cc: Gao, Chao, x86@kernel.org, Huang, Kai, kas@kernel.org,
	dave.hansen@linux.intel.com, mingo@redhat.com, tglx@linutronix.de,
	Williams, Dan J, hpa@zytor.com, bp@alien8.de
In-Reply-To: <d939c09969a30300ed1faa86361f956809831fa5.camel@intel.com>

On Thu, 2026-01-08 at 20:24 +0000, Edgecombe, Rick P wrote:
> On Wed, 2026-01-07 at 17:31 -0700, Vishal Verma wrote:
> > It is useful to print the TDX module version in dmesg logs. This allows
> > for a quick spot check for whether the correct/expected TDX module is
> > being loaded, and also creates a record for any future problems being
> > investigated.
> > 
> 
> It is more then a spot check, it's the only way to know which version is loaded.

I'll update to:

   It is useful to print the TDX module version in dmesg logs. This is
   currently the only way to determine the module version from the host. It
   also creates a record for...

> 
> >  This was also requested in [1].
> > 
> > Include the version in the log messages during init, e.g.:
> > 
> >   virt/tdx: TDX module version: 1.5.24
> >   virt/tdx: 1034220 KB allocated for PAMT
> >   virt/tdx: module initialized
> > 
> > ..followed by remaining TDX initialization messages (or errors).
> 
> The TDX initialization errors would be before "module initialized", right?

Yep, I think this whole line can just be removed to avoid confusion.


^ permalink raw reply

* Re: [PATCH 2/2] x86/virt/tdx: Print TDX module version during init
From: Verma, Vishal L @ 2026-01-08 20:30 UTC (permalink / raw)
  To: kas@kernel.org, Edgecombe, Rick P
  Cc: Gao, Chao, mingo@redhat.com, Huang, Kai, x86@kernel.org,
	hpa@zytor.com, linux-kernel@vger.kernel.org, Williams, Dan J,
	bp@alien8.de, kvm@vger.kernel.org, tglx@linutronix.de,
	linux-coco@lists.linux.dev, dave.hansen@linux.intel.com
In-Reply-To: <aaeda226c7e2d0d81e5b0f767475330e6d9d8bce.camel@intel.com>

On Thu, 2026-01-08 at 20:20 +0000, Edgecombe, Rick P wrote:
> On Thu, 2026-01-08 at 18:39 +0000, Verma, Vishal L wrote:
> > > It can be useful to dump version information, even if get_tdx_sys_info()
> > > fails. Version info is likely to be valid on failure.
> > 
> > Good point, maybe something like this to print it as soon as it is
> > retrieved?
> > 
> > ---3<---
> > 
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > index fba00ddc11f1..5ce4ebe99774 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > @@ -1084,11 +1084,6 @@ static int init_tdx_module(void)
> >         if (ret)
> >                 return ret;
> >  
> > -       pr_info("Module version: %u.%u.%02u\n",
> > -               tdx_sysinfo.version.major_version,
> > -               tdx_sysinfo.version.minor_version,
> > -               tdx_sysinfo.version.update_version);
> > -
> >         /* Check whether the kernel can support this module */
> >         ret = check_features(&tdx_sysinfo);
> >         if (ret)
> > diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> > b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> > index 0454124803f3..4c9917a9c2c3 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> > @@ -105,6 +105,12 @@ static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
> >         int ret = 0;
> >  
> >         ret = ret ?: get_tdx_sys_info_version(&sysinfo->version);
> > +
> > +       pr_info("Module version: %u.%u.%02u\n",
> > +               sysinfo->version.major_version,
> > +               sysinfo->version.minor_version,
> > +               sysinfo->version.update_version);
> > +
> >         ret = ret ?: get_tdx_sys_info_features(&sysinfo->features);
> >         ret = ret ?: get_tdx_sys_info_tdmr(&sysinfo->tdmr);
> >         ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
> 
> It's awkward because it doesn't check if get_tdx_sys_info_version() fails, even
> the though the rest of the code handles this case. I'd just leave it. Let's keep
> this as simple as possible, because anything here will be a huge upgrade.


I considered gating it on 'ret', but making it unconditional also
provides us an indirect hint as to which field failed to retrieve.

Do you mean leave it as in stick to printing only after
get_tdx_sys_info()?

^ permalink raw reply

* Re: [PATCH 2/2] x86/virt/tdx: Print TDX module version during init
From: Edgecombe, Rick P @ 2026-01-08 20:24 UTC (permalink / raw)
  To: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Verma, Vishal L,
	linux-kernel@vger.kernel.org
  Cc: Gao, Chao, bp@alien8.de, Huang, Kai, kas@kernel.org,
	dave.hansen@linux.intel.com, mingo@redhat.com, Williams, Dan J,
	tglx@linutronix.de, hpa@zytor.com, x86@kernel.org
In-Reply-To: <20260107-tdx_print_module_version-v1-2-822baa56762d@intel.com>

On Wed, 2026-01-07 at 17:31 -0700, Vishal Verma wrote:
> It is useful to print the TDX module version in dmesg logs. This allows
> for a quick spot check for whether the correct/expected TDX module is
> being loaded, and also creates a record for any future problems being
> investigated.
> 

It is more then a spot check, it's the only way to know which version is loaded.

>  This was also requested in [1].
> 
> Include the version in the log messages during init, e.g.:
> 
>   virt/tdx: TDX module version: 1.5.24
>   virt/tdx: 1034220 KB allocated for PAMT
>   virt/tdx: module initialized
> 
> ..followed by remaining TDX initialization messages (or errors).

The TDX initialization errors would be before "module initialized", right?

> 
> Print the version early in init_tdx_module(), right after the global
> metadata is read, which makes it available even if there are subsequent
> initialization failures.
> 
> Based on a patch by Kai Huang <kai.huang@intel.com> [2]
> 
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> Reviewed-by: Chao Gao <chao.gao@intel.com>
> Cc: Chao Gao <chao.gao@intel.com>
> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Cc: Kai Huang <kai.huang@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Link: https://lore.kernel.org/all/CAGtprH8eXwi-TcH2+-Fo5YdbEwGmgLBh9ggcDvd6N=bsKEJ_WQ@mail.gmail.com/ # [1]
> Link: https://lore.kernel.org/all/6b5553756f56a8e3222bfc36d0bdb3e5192137b7.1731318868.git.kai.huang@intel.com # [2]
> ---
>  arch/x86/virt/vmx/tdx/tdx.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 5ce4ebe99774..fba00ddc11f1 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1084,6 +1084,11 @@ static int init_tdx_module(void)
>  	if (ret)
>  		return ret;
>  
> +	pr_info("Module version: %u.%u.%02u\n",
> +		tdx_sysinfo.version.major_version,
> +		tdx_sysinfo.version.minor_version,
> +		tdx_sysinfo.version.update_version);
> +
>  	/* Check whether the kernel can support this module */
>  	ret = check_features(&tdx_sysinfo);
>  	if (ret)
> 


^ permalink raw reply

* Re: [PATCH 2/2] x86/virt/tdx: Print TDX module version during init
From: Edgecombe, Rick P @ 2026-01-08 20:20 UTC (permalink / raw)
  To: Verma, Vishal L, kas@kernel.org
  Cc: Gao, Chao, Huang, Kai, bp@alien8.de, hpa@zytor.com,
	x86@kernel.org, linux-kernel@vger.kernel.org, Williams, Dan J,
	dave.hansen@linux.intel.com, kvm@vger.kernel.org,
	tglx@linutronix.de, linux-coco@lists.linux.dev, mingo@redhat.com
In-Reply-To: <261b253ff5bcf593adbddbc34f7a5b4befaa4c21.camel@intel.com>

On Thu, 2026-01-08 at 18:39 +0000, Verma, Vishal L wrote:
> > It can be useful to dump version information, even if get_tdx_sys_info()
> > fails. Version info is likely to be valid on failure.
> 
> Good point, maybe something like this to print it as soon as it is
> retrieved?
> 
> ---3<---
> 
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index fba00ddc11f1..5ce4ebe99774 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1084,11 +1084,6 @@ static int init_tdx_module(void)
>         if (ret)
>                 return ret;
>  
> -       pr_info("Module version: %u.%u.%02u\n",
> -               tdx_sysinfo.version.major_version,
> -               tdx_sysinfo.version.minor_version,
> -               tdx_sysinfo.version.update_version);
> -
>         /* Check whether the kernel can support this module */
>         ret = check_features(&tdx_sysinfo);
>         if (ret)
> diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> index 0454124803f3..4c9917a9c2c3 100644
> --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> @@ -105,6 +105,12 @@ static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
>         int ret = 0;
>  
>         ret = ret ?: get_tdx_sys_info_version(&sysinfo->version);
> +
> +       pr_info("Module version: %u.%u.%02u\n",
> +               sysinfo->version.major_version,
> +               sysinfo->version.minor_version,
> +               sysinfo->version.update_version);
> +
>         ret = ret ?: get_tdx_sys_info_features(&sysinfo->features);
>         ret = ret ?: get_tdx_sys_info_tdmr(&sysinfo->tdmr);
>         ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);

It's awkward because it doesn't check if get_tdx_sys_info_version() fails, even
the though the rest of the code handles this case. I'd just leave it. Let's keep
this as simple as possible, because anything here will be a huge upgrade.

^ permalink raw reply

* Re: [PATCH 1/2] x86/virt/tdx: Retrieve TDX module version
From: Edgecombe, Rick P @ 2026-01-08 20:18 UTC (permalink / raw)
  To: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Verma, Vishal L,
	linux-kernel@vger.kernel.org
  Cc: Gao, Chao, bp@alien8.de, Huang, Kai, kas@kernel.org,
	dave.hansen@linux.intel.com, mingo@redhat.com, Williams, Dan J,
	tglx@linutronix.de, hpa@zytor.com, x86@kernel.org
In-Reply-To: <20260107-tdx_print_module_version-v1-1-822baa56762d@intel.com>

On Wed, 2026-01-07 at 17:31 -0700, Vishal Verma wrote:
> From: Chao Gao <chao.gao@intel.com>
> 
> Each TDX module has several bits of metadata about which specific TDX
> module it is. 
> 


> The primary bit of info is the version, which has an x.y.z
> format, where x represents the major version, y the minor version, and z
> the update version.
> 

A bit of a run-on sentence.

>  Knowing the running TDX Module version is valuable
> for bug reporting and debugging. Note that the module does expose other
> pieces of version-related metadata, such as build number and date. Those
> aren't retrieved for now, that can be added if needed in the future.
> 
> Retrieve the TDX Module version using the existing metadata reading
> interface. Later changes will expose this information. The metadata
> reading interfaces have existed for quite some time, so this will work
> with older versions of the TDX module as well - i.e. this isn't a new
> interface.
> 
> As a side note, the global metadata reading code was originally set up
> to be auto-generated from a JSON definition [1]. However, later [2] this
> was found to be unsustainable, and the autogeneration approach was
> dropped in favor of just manually adding fields as needed (e.g. as in
> this patch).
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Cc: Kai Huang <kai.huang@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Link: https://lore.kernel.org/kvm/CABgObfYXUxqQV_FoxKjC8U3t5DnyM45nz5DpTxYZv2x_uFK_Kw@mail.gmail.com/ # [1]
> Link: https://lore.kernel.org/all/1e7bcbad-eb26-44b7-97ca-88ab53467212@intel.com/ # [2]
> ---
>  arch/x86/include/asm/tdx_global_metadata.h  |  7 +++++++
>  arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 16 ++++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
> index 060a2ad744bff..40689c8dc67eb 100644
> --- a/arch/x86/include/asm/tdx_global_metadata.h
> +++ b/arch/x86/include/asm/tdx_global_metadata.h
> @@ -5,6 +5,12 @@
>  
>  #include <linux/types.h>
>  
> +struct tdx_sys_info_version {
> +	u16 minor_version;
> +	u16 major_version;
> +	u16 update_version;
> +};
> +
>  struct tdx_sys_info_features {
>  	u64 tdx_features0;
>  };
> @@ -35,6 +41,7 @@ struct tdx_sys_info_td_conf {
>  };
>  
>  struct tdx_sys_info {
> +	struct tdx_sys_info_version version;
>  	struct tdx_sys_info_features features;
>  	struct tdx_sys_info_tdmr tdmr;
>  	struct tdx_sys_info_td_ctrl td_ctrl;
> diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> index 13ad2663488b1..0454124803f36 100644
> --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> @@ -7,6 +7,21 @@
>   * Include this file to other C file instead.
>   */
>  
> +static int get_tdx_sys_info_version(struct tdx_sys_info_version *sysinfo_version)
> +{
> +	int ret = 0;
> +	u64 val;
> +
> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000003, &val)))
> +		sysinfo_version->minor_version = val;
> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000004, &val)))
> +		sysinfo_version->major_version = val;
> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000005, &val)))
> +		sysinfo_version->update_version = val;
> +
> +	return ret;
> +}
> +
>  static int get_tdx_sys_info_features(struct tdx_sys_info_features *sysinfo_features)
>  {
>  	int ret = 0;
> @@ -89,6 +104,7 @@ static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
>  {
>  	int ret = 0;
>  
> +	ret = ret ?: get_tdx_sys_info_version(&sysinfo->version);
>  	ret = ret ?: get_tdx_sys_info_features(&sysinfo->features);
>  	ret = ret ?: get_tdx_sys_info_tdmr(&sysinfo->tdmr);
>  	ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
> 

Code looks good to me.

Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

^ 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