* Re: [PATCH v2 08/21] coco/tdx-host: Implement FW_UPLOAD sysfs ABI for TDX Module updates
From: Chao Gao @ 2025-12-02 7:20 UTC (permalink / raw)
To: Binbin Wu
Cc: linux-coco, linux-kernel, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Kirill A. Shutemov
In-Reply-To: <41cdd3d5-c45b-4991-ace9-bef7cf9ed197@linux.intel.com>
On Mon, Nov 24, 2025 at 03:49:34PM +0800, Binbin Wu wrote:
>
>
>On 10/1/2025 10:52 AM, Chao Gao wrote:
>[...]
>> +static enum fw_upload_err tdx_fw_prepare(struct fw_upload *fwl,
>> + const u8 *data, u32 size)
>> +{
>> + struct tdx_fw_upload_status *status = fwl->dd_handle;
>> +
>> + if (status->cancel_request) {
>> + status->cancel_request = false;
>> + return FW_UPLOAD_ERR_CANCELED;
>> + }
>> +
>> + return FW_UPLOAD_ERR_NONE;
>> +}
>> +
>> +static enum fw_upload_err tdx_fw_write(struct fw_upload *fwl, const u8 *data,
>> + u32 offset, u32 size, u32 *written)
>> +{
>> + struct tdx_fw_upload_status *status = fwl->dd_handle;
>> +
>> + if (status->cancel_request) {
>> + status->cancel_request = false;
>> + return FW_UPLOAD_ERR_CANCELED;
>> + }
>
>Since the execution of the work is not protected by the mutex, if userspace
>requests cancellation after this point, after the TDX module update finished,
>it seems that the cancel value is left over and it could impact the next update?
Yes, I think this is a bug. A few other drivers just clear "cancel_request" in
the "prepare" phase, e.g., pd692x0_fw_prepare(), mpfs_auto_update_prepare(),
m10bmc_sec_prepare(). I will follow that approach.
^ permalink raw reply
* Re: [PATCH v2 11/21] x86/virt/seamldr: Allocate and populate a module update request
From: Chao Gao @ 2025-12-02 7:03 UTC (permalink / raw)
To: Binbin Wu
Cc: linux-coco, linux-kernel, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Kirill A. Shutemov, Dave Hansen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <a8073390-b952-43a1-8e63-18c2a10bb721@linux.intel.com>
On Thu, Nov 27, 2025 at 04:30:41PM +0800, Binbin Wu wrote:
>
>
>On 10/1/2025 10:52 AM, Chao Gao wrote:
>[...]
>> +
>> +/* Allocate and populate a seamldr_params */
>> +static struct seamldr_params *alloc_seamldr_params(const void *module, int module_size,
>> + const void *sig, int sig_size)
>> +{
>> + struct seamldr_params *params;
>> + const u8 *ptr;
>> + int i;
>> +
>> + BUILD_BUG_ON(sizeof(struct seamldr_params) != SZ_4K);
>> + if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
>> + return ERR_PTR(-EINVAL);
>> +
>> + if (!IS_ALIGNED(module_size, SZ_4K) || !IS_ALIGNED(sig_size, SZ_4K) ||
>> + !IS_ALIGNED((unsigned long)module, SZ_4K) ||
>> + !IS_ALIGNED((unsigned long)sig, SZ_4K))
>> + return ERR_PTR(-EINVAL);
>> +
>> + /* seamldr_params accepts one 4KB-page for sigstruct */
>> + if (sig_size != SZ_4K)
>According to the link [2] you provided above, it seems that the layout of
>tdx_blob as following:
>tdx_blob
>|- u16 version
>|- u16 checksum
>|- u32 offset_of_module --------------------------------------|
>|- u8 signature[8] |
>|- u32 len 8KB + (N * 4KB) |
>|- u32 resv1 |
>|- u64 resv2[509] |
>|- u8 data[] |
> |- _u64 sigstruct[256] //2KB sigstruct |
> |- _u64 reserved2[256] |
> |- _u64 reserved3[N*512] //4KB aligned, optional, N >=0 |
> |- _u8 module[] //<-----------------------------|
>
>If N is not 0 for reserved3, then the sig_size passed will not be 4KB.
The "reserved3[N*512]" is there for future extension.
The current P-SEAMLDR ABI only supports one 4KB page, so if a blob's sig_size
is larger, the kernel has to reject it. The P-SEAMLDR ABI should be extended
first, and then we can add kernel support accordingly.
>
>
>> + return ERR_PTR(-EINVAL);
>> +
>> + params = (struct seamldr_params *)get_zeroed_page(GFP_KERNEL);
>> + if (!params)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + params->scenario = SEAMLDR_SCENARIO_UPDATE;
>> + params->sigstruct_pa = (vmalloc_to_pfn(sig) << PAGE_SHIFT) +
>> + ((unsigned long)sig & ~PAGE_MASK);
>
>Since sig is 4KB aligned, is ((unsigned long)sig & ~PAGE_MASK) needed?
This is done intentionally. Otherwise, we would need to assume PAGE_SIZE is
4KB. Although this is true for x86, just in case it changes in the future and
subtly breaks this code, I use SZ_4K and apply PAGE_MASK here.
<snip>
>> +static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
>> +{
>> + const struct tdx_blob *blob = (const void *)data;
>> + int module_size, sig_size;
>> + const void *sig, *module;
>> +
>> + if (blob->version != 0x100) {
>> + pr_err("unsupported blob version: %u\n", blob->version);
>
>Based on the link [2], 0x100 stands for version 1.0, Using hexadecimal seems
>more readable.
Makes sense. Will do.
^ permalink raw reply
* Re: [PATCH kernel v3 3/4] iommu/amd: Report SEV-TIO support
From: Vasant Hegde @ 2025-12-02 4:57 UTC (permalink / raw)
To: Alexey Kardashevskiy, linux-kernel
Cc: linux-crypto, Tom Lendacky, John Allen, Herbert Xu,
David S. Miller, Ashish Kalra, Joerg Roedel,
Suravee Suthikulpanit, Will Deacon, Robin Murphy, Borislav Petkov,
Borislav Petkov (AMD), Dan Williams, Jason Gunthorpe,
Jerry Snitselaar, Gao Shiyuan, Sean Christopherson, Kim Phillips,
Nikunj A Dadhania, Michael Roth, Paolo Bonzini, iommu, x86,
linux-coco, Joerg Roedel
In-Reply-To: <20251202024449.542361-4-aik@amd.com>
On 12/2/2025 8:14 AM, Alexey Kardashevskiy wrote:
> The SEV-TIO switch in the AMD BIOS is reported to the OS via
> the IOMMU Extended Feature 2 register (EFR2), bit 1.
>
> Add helper to parse the bit and report the feature presence.
>
> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> Acked-by: Joerg Roedel <joerg.roedel@amd.com>
> Link: https://patch.msgid.link/20251121080629.444992-4-aik@amd.com
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
-Vasant
^ permalink raw reply
* [PATCH kernel v3 4/4] crypto/ccp: Implement SEV-TIO PCIe IDE (phase1)
From: Alexey Kardashevskiy @ 2025-12-02 2:44 UTC (permalink / raw)
To: linux-kernel
Cc: linux-crypto, Tom Lendacky, John Allen, Herbert Xu,
David S. Miller, Ashish Kalra, Joerg Roedel,
Suravee Suthikulpanit, Will Deacon, Robin Murphy, Borislav Petkov,
Borislav Petkov (AMD), Dan Williams, Jason Gunthorpe,
Jerry Snitselaar, Vasant Hegde, Gao Shiyuan, Sean Christopherson,
Kim Phillips, Nikunj A Dadhania, Michael Roth, Paolo Bonzini,
iommu, Alexey Kardashevskiy, x86, linux-coco
In-Reply-To: <20251202024449.542361-1-aik@amd.com>
Implement the SEV-TIO (Trusted I/O) firmware interface for PCIe TDISP
(Trust Domain In-Socket Protocol). This enables secure communication
between trusted domains and PCIe devices through the PSP (Platform
Security Processor).
The implementation includes:
- Device Security Manager (DSM) operations for establishing secure links
- SPDM (Security Protocol and Data Model) over DOE (Data Object Exchange)
- IDE (Integrity Data Encryption) stream management for secure PCIe
This module bridges the SEV firmware stack with the generic PCIe TSM
framework.
This is phase1 as described in Documentation/driver-api/pci/tsm.rst.
On AMD SEV, the AMD PSP firmware acts as TSM (manages the security/trust).
The CCP driver provides the interface to it and registers in the TSM
subsystem.
Detect the PSP support (reported via FEATURE_INFO + SNP_PLATFORM_STATUS)
and enable SEV-TIO in the SNP_INIT_EX call if the hardware supports TIO.
Implement SEV TIO PSP command wrappers in sev-dev-tio.c and store
the data in the SEV-TIO-specific structs.
Implement TSM hooks and IDE setup in sev-dev-tsm.c.
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
Changes:
v2:
* moved declarations from sev-dev-tio.h to sev-dev.h
* removed include "sev-dev-tio.h" from sev-dev.c to fight errors when TSM is disabled
* converted /** to /* as these are part of any external API and trigger unwanted kerneldoc warnings
* got rid of ifdefs
* "select PCI_TSM" moved under CRYPTO_DEV_SP_PSP
* open coded SNP_SEV_TIO_SUPPORTED
* renamed tio_present to tio_supp to match the flag name
* merged "crypto: ccp: Enable SEV-TIO feature in the PSP when supported" to this one
---
drivers/crypto/ccp/Kconfig | 1 +
drivers/crypto/ccp/Makefile | 4 +
drivers/crypto/ccp/sev-dev-tio.h | 123 +++
drivers/crypto/ccp/sev-dev.h | 9 +
include/linux/psp-sev.h | 11 +-
drivers/crypto/ccp/sev-dev-tio.c | 864 ++++++++++++++++++++
drivers/crypto/ccp/sev-dev-tsm.c | 405 +++++++++
drivers/crypto/ccp/sev-dev.c | 51 +-
8 files changed, 1465 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/ccp/Kconfig b/drivers/crypto/ccp/Kconfig
index f394e45e11ab..e2b127f0986b 100644
--- a/drivers/crypto/ccp/Kconfig
+++ b/drivers/crypto/ccp/Kconfig
@@ -39,6 +39,7 @@ config CRYPTO_DEV_SP_PSP
bool "Platform Security Processor (PSP) device"
default y
depends on CRYPTO_DEV_CCP_DD && X86_64 && AMD_IOMMU
+ select PCI_TSM
help
Provide support for the AMD Platform Security Processor (PSP).
The PSP is a dedicated processor that provides support for key
diff --git a/drivers/crypto/ccp/Makefile b/drivers/crypto/ccp/Makefile
index a9626b30044a..0424e08561ef 100644
--- a/drivers/crypto/ccp/Makefile
+++ b/drivers/crypto/ccp/Makefile
@@ -16,6 +16,10 @@ ccp-$(CONFIG_CRYPTO_DEV_SP_PSP) += psp-dev.o \
hsti.o \
sfs.o
+ifeq ($(CONFIG_PCI_TSM),y)
+ccp-$(CONFIG_CRYPTO_DEV_SP_PSP) += sev-dev-tsm.o sev-dev-tio.o
+endif
+
obj-$(CONFIG_CRYPTO_DEV_CCP_CRYPTO) += ccp-crypto.o
ccp-crypto-objs := ccp-crypto-main.o \
ccp-crypto-aes.o \
diff --git a/drivers/crypto/ccp/sev-dev-tio.h b/drivers/crypto/ccp/sev-dev-tio.h
new file mode 100644
index 000000000000..67512b3dbc53
--- /dev/null
+++ b/drivers/crypto/ccp/sev-dev-tio.h
@@ -0,0 +1,123 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __PSP_SEV_TIO_H__
+#define __PSP_SEV_TIO_H__
+
+#include <linux/pci-tsm.h>
+#include <linux/pci-ide.h>
+#include <linux/tsm.h>
+#include <uapi/linux/psp-sev.h>
+
+struct sla_addr_t {
+ union {
+ u64 sla;
+ struct {
+ u64 page_type :1,
+ page_size :1,
+ reserved1 :10,
+ pfn :40,
+ reserved2 :12;
+ };
+ };
+} __packed;
+
+#define SEV_TIO_MAX_COMMAND_LENGTH 128
+
+/* SPDM control structure for DOE */
+struct tsm_spdm {
+ unsigned long req_len;
+ void *req;
+ unsigned long rsp_len;
+ void *rsp;
+};
+
+/* Describes TIO device */
+struct tsm_dsm_tio {
+ u8 cert_slot;
+ struct sla_addr_t dev_ctx;
+ struct sla_addr_t req;
+ struct sla_addr_t resp;
+ struct sla_addr_t scratch;
+ struct sla_addr_t output;
+ size_t output_len;
+ size_t scratch_len;
+ struct tsm_spdm spdm;
+ struct sla_buffer_hdr *reqbuf; /* vmap'ed @req for DOE */
+ struct sla_buffer_hdr *respbuf; /* vmap'ed @resp for DOE */
+
+ int cmd;
+ int psp_ret;
+ u8 cmd_data[SEV_TIO_MAX_COMMAND_LENGTH];
+ void *data_pg; /* Data page for DEV_STATUS/TDI_STATUS/TDI_INFO/ASID_FENCE */
+
+#define TIO_IDE_MAX_TC 8
+ struct pci_ide *ide[TIO_IDE_MAX_TC];
+};
+
+/* Describes TSM structure for PF0 pointed by pci_dev->tsm */
+struct tio_dsm {
+ struct pci_tsm_pf0 tsm;
+ struct tsm_dsm_tio data;
+ struct sev_device *sev;
+};
+
+/* Data object IDs */
+#define SPDM_DOBJ_ID_NONE 0
+#define SPDM_DOBJ_ID_REQ 1
+#define SPDM_DOBJ_ID_RESP 2
+
+struct spdm_dobj_hdr {
+ u32 id; /* Data object type identifier */
+ u32 length; /* Length of the data object, INCLUDING THIS HEADER */
+ struct { /* Version of the data object structure */
+ u8 minor;
+ u8 major;
+ } version;
+} __packed;
+
+/**
+ * struct sev_tio_status - TIO_STATUS command's info_paddr buffer
+ *
+ * @length: Length of this structure in bytes
+ * @tio_en: Indicates that SNP_INIT_EX initialized the RMP for SEV-TIO
+ * @tio_init_done: Indicates TIO_INIT has been invoked
+ * @spdm_req_size_min: Minimum SPDM request buffer size in bytes
+ * @spdm_req_size_max: Maximum SPDM request buffer size in bytes
+ * @spdm_scratch_size_min: Minimum SPDM scratch buffer size in bytes
+ * @spdm_scratch_size_max: Maximum SPDM scratch buffer size in bytes
+ * @spdm_out_size_min: Minimum SPDM output buffer size in bytes
+ * @spdm_out_size_max: Maximum for the SPDM output buffer size in bytes
+ * @spdm_rsp_size_min: Minimum SPDM response buffer size in bytes
+ * @spdm_rsp_size_max: Maximum SPDM response buffer size in bytes
+ * @devctx_size: Size of a device context buffer in bytes
+ * @tdictx_size: Size of a TDI context buffer in bytes
+ * @tio_crypto_alg: TIO crypto algorithms supported
+ */
+struct sev_tio_status {
+ u32 length;
+ u32 tio_en :1,
+ tio_init_done :1,
+ reserved :30;
+ u32 spdm_req_size_min;
+ u32 spdm_req_size_max;
+ u32 spdm_scratch_size_min;
+ u32 spdm_scratch_size_max;
+ u32 spdm_out_size_min;
+ u32 spdm_out_size_max;
+ u32 spdm_rsp_size_min;
+ u32 spdm_rsp_size_max;
+ u32 devctx_size;
+ u32 tdictx_size;
+ u32 tio_crypto_alg;
+ u8 reserved2[12];
+} __packed;
+
+int sev_tio_init_locked(void *tio_status_page);
+int sev_tio_continue(struct tsm_dsm_tio *dev_data);
+
+int sev_tio_dev_create(struct tsm_dsm_tio *dev_data, u16 device_id, u16 root_port_id,
+ u8 segment_id);
+int sev_tio_dev_connect(struct tsm_dsm_tio *dev_data, u8 tc_mask, u8 ids[8], u8 cert_slot);
+int sev_tio_dev_disconnect(struct tsm_dsm_tio *dev_data, bool force);
+int sev_tio_dev_reclaim(struct tsm_dsm_tio *dev_data);
+
+#endif /* __PSP_SEV_TIO_H__ */
diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
index b9029506383f..b1cd556bbbf6 100644
--- a/drivers/crypto/ccp/sev-dev.h
+++ b/drivers/crypto/ccp/sev-dev.h
@@ -34,6 +34,8 @@ struct sev_misc_dev {
struct miscdevice misc;
};
+struct sev_tio_status;
+
struct sev_device {
struct device *dev;
struct psp_device *psp;
@@ -61,6 +63,9 @@ struct sev_device {
struct sev_user_data_snp_status snp_plat_status;
struct snp_feature_info snp_feat_info_0;
+
+ struct tsm_dev *tsmdev;
+ struct sev_tio_status *tio_status;
};
int sev_dev_init(struct psp_device *psp);
@@ -74,4 +79,8 @@ void sev_pci_exit(void);
struct page *snp_alloc_hv_fixed_pages(unsigned int num_2mb_pages);
void snp_free_hv_fixed_pages(struct page *page);
+void sev_tsm_init_locked(struct sev_device *sev, void *tio_status_page);
+void sev_tsm_uninit(struct sev_device *sev);
+int sev_tio_cmd_buffer_len(int cmd);
+
#endif /* __SEV_DEV_H */
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 34a25209f909..cce864dbf281 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -109,6 +109,13 @@ enum sev_cmd {
SEV_CMD_SNP_VLEK_LOAD = 0x0CD,
SEV_CMD_SNP_FEATURE_INFO = 0x0CE,
+ /* SEV-TIO commands */
+ SEV_CMD_TIO_STATUS = 0x0D0,
+ SEV_CMD_TIO_INIT = 0x0D1,
+ SEV_CMD_TIO_DEV_CREATE = 0x0D2,
+ SEV_CMD_TIO_DEV_RECLAIM = 0x0D3,
+ SEV_CMD_TIO_DEV_CONNECT = 0x0D4,
+ SEV_CMD_TIO_DEV_DISCONNECT = 0x0D5,
SEV_CMD_MAX,
};
@@ -750,7 +757,8 @@ struct sev_data_snp_init_ex {
u32 list_paddr_en:1;
u32 rapl_dis:1;
u32 ciphertext_hiding_en:1;
- u32 rsvd:28;
+ u32 tio_en:1;
+ u32 rsvd:27;
u32 rsvd1;
u64 list_paddr;
u16 max_snp_asid;
@@ -850,6 +858,7 @@ struct snp_feature_info {
} __packed;
#define SNP_CIPHER_TEXT_HIDING_SUPPORTED BIT(3)
+#define SNP_SEV_TIO_SUPPORTED BIT(1) /* EBX */
#ifdef CONFIG_CRYPTO_DEV_SP_PSP
diff --git a/drivers/crypto/ccp/sev-dev-tio.c b/drivers/crypto/ccp/sev-dev-tio.c
new file mode 100644
index 000000000000..9a98f98c20a7
--- /dev/null
+++ b/drivers/crypto/ccp/sev-dev-tio.c
@@ -0,0 +1,864 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+// Interface to PSP for CCP/SEV-TIO/SNP-VM
+
+#include <linux/pci.h>
+#include <linux/tsm.h>
+#include <linux/psp.h>
+#include <linux/vmalloc.h>
+#include <linux/bitfield.h>
+#include <linux/pci-doe.h>
+#include <asm/sev-common.h>
+#include <asm/sev.h>
+#include <asm/page.h>
+#include "sev-dev.h"
+#include "sev-dev-tio.h"
+
+#define to_tio_status(dev_data) \
+ (container_of((dev_data), struct tio_dsm, data)->sev->tio_status)
+
+#define SLA_PAGE_TYPE_DATA 0
+#define SLA_PAGE_TYPE_SCATTER 1
+#define SLA_PAGE_SIZE_4K 0
+#define SLA_PAGE_SIZE_2M 1
+#define SLA_SZ(s) ((s).page_size == SLA_PAGE_SIZE_2M ? SZ_2M : SZ_4K)
+#define SLA_SCATTER_LEN(s) (SLA_SZ(s) / sizeof(struct sla_addr_t))
+#define SLA_EOL ((struct sla_addr_t) { .pfn = ((1UL << 40) - 1) })
+#define SLA_NULL ((struct sla_addr_t) { 0 })
+#define IS_SLA_NULL(s) ((s).sla == SLA_NULL.sla)
+#define IS_SLA_EOL(s) ((s).sla == SLA_EOL.sla)
+
+static phys_addr_t sla_to_pa(struct sla_addr_t sla)
+{
+ u64 pfn = sla.pfn;
+ u64 pa = pfn << PAGE_SHIFT;
+
+ return pa;
+}
+
+static void *sla_to_va(struct sla_addr_t sla)
+{
+ void *va = __va(__sme_clr(sla_to_pa(sla)));
+
+ return va;
+}
+
+#define sla_to_pfn(sla) (__pa(sla_to_va(sla)) >> PAGE_SHIFT)
+#define sla_to_page(sla) virt_to_page(sla_to_va(sla))
+
+static struct sla_addr_t make_sla(struct page *pg, bool stp)
+{
+ u64 pa = __sme_set(page_to_phys(pg));
+ struct sla_addr_t ret = {
+ .pfn = pa >> PAGE_SHIFT,
+ .page_size = SLA_PAGE_SIZE_4K, /* Do not do SLA_PAGE_SIZE_2M ATM */
+ .page_type = stp ? SLA_PAGE_TYPE_SCATTER : SLA_PAGE_TYPE_DATA
+ };
+
+ return ret;
+}
+
+/* the BUFFER Structure */
+#define SLA_BUFFER_FLAG_ENCRYPTION BIT(0)
+
+/*
+ * struct sla_buffer_hdr - Scatter list address buffer header
+ *
+ * @capacity_sz: Total capacity of the buffer in bytes
+ * @payload_sz: Size of buffer payload in bytes, must be multiple of 32B
+ * @flags: Buffer flags (SLA_BUFFER_FLAG_ENCRYPTION: buffer is encrypted)
+ * @iv: Initialization vector used for encryption
+ * @authtag: Authentication tag for encrypted buffer
+ */
+struct sla_buffer_hdr {
+ u32 capacity_sz;
+ u32 payload_sz; /* The size of BUFFER_PAYLOAD in bytes. Must be multiple of 32B */
+ u32 flags;
+ u8 reserved1[4];
+ u8 iv[16]; /* IV used for the encryption of this buffer */
+ u8 authtag[16]; /* Authentication tag for this buffer */
+ u8 reserved2[16];
+} __packed;
+
+enum spdm_data_type_t {
+ DOBJ_DATA_TYPE_SPDM = 0x1,
+ DOBJ_DATA_TYPE_SECURE_SPDM = 0x2,
+};
+
+struct spdm_dobj_hdr_req {
+ struct spdm_dobj_hdr hdr; /* hdr.id == SPDM_DOBJ_ID_REQ */
+ u8 data_type; /* spdm_data_type_t */
+ u8 reserved2[5];
+} __packed;
+
+struct spdm_dobj_hdr_resp {
+ struct spdm_dobj_hdr hdr; /* hdr.id == SPDM_DOBJ_ID_RESP */
+ u8 data_type; /* spdm_data_type_t */
+ u8 reserved2[5];
+} __packed;
+
+/* Defined in sev-dev-tio.h so sev-dev-tsm.c can read types of blobs */
+struct spdm_dobj_hdr_cert;
+struct spdm_dobj_hdr_meas;
+struct spdm_dobj_hdr_report;
+
+/* Used in all SPDM-aware TIO commands */
+struct spdm_ctrl {
+ struct sla_addr_t req;
+ struct sla_addr_t resp;
+ struct sla_addr_t scratch;
+ struct sla_addr_t output;
+} __packed;
+
+static size_t sla_dobj_id_to_size(u8 id)
+{
+ size_t n;
+
+ BUILD_BUG_ON(sizeof(struct spdm_dobj_hdr_resp) != 0x10);
+ switch (id) {
+ case SPDM_DOBJ_ID_REQ:
+ n = sizeof(struct spdm_dobj_hdr_req);
+ break;
+ case SPDM_DOBJ_ID_RESP:
+ n = sizeof(struct spdm_dobj_hdr_resp);
+ break;
+ default:
+ WARN_ON(1);
+ n = 0;
+ break;
+ }
+
+ return n;
+}
+
+#define SPDM_DOBJ_HDR_SIZE(hdr) sla_dobj_id_to_size((hdr)->id)
+#define SPDM_DOBJ_DATA(hdr) ((u8 *)(hdr) + SPDM_DOBJ_HDR_SIZE(hdr))
+#define SPDM_DOBJ_LEN(hdr) ((hdr)->length - SPDM_DOBJ_HDR_SIZE(hdr))
+
+#define sla_to_dobj_resp_hdr(buf) ((struct spdm_dobj_hdr_resp *) \
+ sla_to_dobj_hdr_check((buf), SPDM_DOBJ_ID_RESP))
+#define sla_to_dobj_req_hdr(buf) ((struct spdm_dobj_hdr_req *) \
+ sla_to_dobj_hdr_check((buf), SPDM_DOBJ_ID_REQ))
+
+static struct spdm_dobj_hdr *sla_to_dobj_hdr(struct sla_buffer_hdr *buf)
+{
+ if (!buf)
+ return NULL;
+
+ return (struct spdm_dobj_hdr *) &buf[1];
+}
+
+static struct spdm_dobj_hdr *sla_to_dobj_hdr_check(struct sla_buffer_hdr *buf, u32 check_dobjid)
+{
+ struct spdm_dobj_hdr *hdr = sla_to_dobj_hdr(buf);
+
+ if (WARN_ON_ONCE(!hdr))
+ return NULL;
+
+ if (hdr->id != check_dobjid) {
+ pr_err("! ERROR: expected %d, found %d\n", check_dobjid, hdr->id);
+ return NULL;
+ }
+
+ return hdr;
+}
+
+static void *sla_to_data(struct sla_buffer_hdr *buf, u32 dobjid)
+{
+ struct spdm_dobj_hdr *hdr = sla_to_dobj_hdr(buf);
+
+ if (WARN_ON_ONCE(dobjid != SPDM_DOBJ_ID_REQ && dobjid != SPDM_DOBJ_ID_RESP))
+ return NULL;
+
+ if (!hdr)
+ return NULL;
+
+ return (u8 *) hdr + sla_dobj_id_to_size(dobjid);
+}
+
+/*
+ * struct sev_data_tio_status - SEV_CMD_TIO_STATUS command
+ *
+ * @length: Length of this command buffer in bytes
+ * @status_paddr: System physical address of the TIO_STATUS structure
+ */
+struct sev_data_tio_status {
+ u32 length;
+ u8 reserved[4];
+ u64 status_paddr;
+} __packed;
+
+/* TIO_INIT */
+struct sev_data_tio_init {
+ u32 length;
+ u8 reserved[12];
+} __packed;
+
+/*
+ * struct sev_data_tio_dev_create - TIO_DEV_CREATE command
+ *
+ * @length: Length in bytes of this command buffer
+ * @dev_ctx_sla: Scatter list address pointing to a buffer to be used as a device context buffer
+ * @device_id: PCIe Routing Identifier of the device to connect to
+ * @root_port_id: PCIe Routing Identifier of the root port of the device
+ * @segment_id: PCIe Segment Identifier of the device to connect to
+ */
+struct sev_data_tio_dev_create {
+ u32 length;
+ u8 reserved1[4];
+ struct sla_addr_t dev_ctx_sla;
+ u16 device_id;
+ u16 root_port_id;
+ u8 segment_id;
+ u8 reserved2[11];
+} __packed;
+
+/*
+ * struct sev_data_tio_dev_connect - TIO_DEV_CONNECT command
+ *
+ * @length: Length in bytes of this command buffer
+ * @spdm_ctrl: SPDM control structure defined in Section 5.1
+ * @dev_ctx_sla: Scatter list address of the device context buffer
+ * @tc_mask: Bitmask of the traffic classes to initialize for SEV-TIO usage.
+ * Setting the kth bit of the TC_MASK to 1 indicates that the traffic
+ * class k will be initialized
+ * @cert_slot: Slot number of the certificate requested for constructing the SPDM session
+ * @ide_stream_id: IDE stream IDs to be associated with this device.
+ * Valid only if corresponding bit in TC_MASK is set
+ */
+struct sev_data_tio_dev_connect {
+ u32 length;
+ u8 reserved1[4];
+ struct spdm_ctrl spdm_ctrl;
+ u8 reserved2[8];
+ struct sla_addr_t dev_ctx_sla;
+ u8 tc_mask;
+ u8 cert_slot;
+ u8 reserved3[6];
+ u8 ide_stream_id[8];
+ u8 reserved4[8];
+} __packed;
+
+/*
+ * struct sev_data_tio_dev_disconnect - TIO_DEV_DISCONNECT command
+ *
+ * @length: Length in bytes of this command buffer
+ * @flags: Command flags (TIO_DEV_DISCONNECT_FLAG_FORCE: force disconnect)
+ * @spdm_ctrl: SPDM control structure defined in Section 5.1
+ * @dev_ctx_sla: Scatter list address of the device context buffer
+ */
+#define TIO_DEV_DISCONNECT_FLAG_FORCE BIT(0)
+
+struct sev_data_tio_dev_disconnect {
+ u32 length;
+ u32 flags;
+ struct spdm_ctrl spdm_ctrl;
+ struct sla_addr_t dev_ctx_sla;
+} __packed;
+
+/*
+ * struct sev_data_tio_dev_meas - TIO_DEV_MEASUREMENTS command
+ *
+ * @length: Length in bytes of this command buffer
+ * @flags: Command flags (TIO_DEV_MEAS_FLAG_RAW_BITSTREAM: request raw measurements)
+ * @spdm_ctrl: SPDM control structure defined in Section 5.1
+ * @dev_ctx_sla: Scatter list address of the device context buffer
+ * @meas_nonce: Nonce for measurement freshness verification
+ */
+#define TIO_DEV_MEAS_FLAG_RAW_BITSTREAM BIT(0)
+
+struct sev_data_tio_dev_meas {
+ u32 length;
+ u32 flags;
+ struct spdm_ctrl spdm_ctrl;
+ struct sla_addr_t dev_ctx_sla;
+ u8 meas_nonce[32];
+} __packed;
+
+/*
+ * struct sev_data_tio_dev_certs - TIO_DEV_CERTIFICATES command
+ *
+ * @length: Length in bytes of this command buffer
+ * @spdm_ctrl: SPDM control structure defined in Section 5.1
+ * @dev_ctx_sla: Scatter list address of the device context buffer
+ */
+struct sev_data_tio_dev_certs {
+ u32 length;
+ u8 reserved[4];
+ struct spdm_ctrl spdm_ctrl;
+ struct sla_addr_t dev_ctx_sla;
+} __packed;
+
+/*
+ * struct sev_data_tio_dev_reclaim - TIO_DEV_RECLAIM command
+ *
+ * @length: Length in bytes of this command buffer
+ * @dev_ctx_sla: Scatter list address of the device context buffer
+ *
+ * This command reclaims resources associated with a device context.
+ */
+struct sev_data_tio_dev_reclaim {
+ u32 length;
+ u8 reserved[4];
+ struct sla_addr_t dev_ctx_sla;
+} __packed;
+
+static struct sla_buffer_hdr *sla_buffer_map(struct sla_addr_t sla)
+{
+ struct sla_buffer_hdr *buf;
+
+ BUILD_BUG_ON(sizeof(struct sla_buffer_hdr) != 0x40);
+ if (IS_SLA_NULL(sla))
+ return NULL;
+
+ if (sla.page_type == SLA_PAGE_TYPE_SCATTER) {
+ struct sla_addr_t *scatter = sla_to_va(sla);
+ unsigned int i, npages = 0;
+
+ for (i = 0; i < SLA_SCATTER_LEN(sla); ++i) {
+ if (WARN_ON_ONCE(SLA_SZ(scatter[i]) > SZ_4K))
+ return NULL;
+
+ if (WARN_ON_ONCE(scatter[i].page_type == SLA_PAGE_TYPE_SCATTER))
+ return NULL;
+
+ if (IS_SLA_EOL(scatter[i])) {
+ npages = i;
+ break;
+ }
+ }
+ if (WARN_ON_ONCE(!npages))
+ return NULL;
+
+ struct page **pp = kmalloc_array(npages, sizeof(pp[0]), GFP_KERNEL);
+
+ if (!pp)
+ return NULL;
+
+ for (i = 0; i < npages; ++i)
+ pp[i] = sla_to_page(scatter[i]);
+
+ buf = vm_map_ram(pp, npages, 0);
+ kfree(pp);
+ } else {
+ struct page *pg = sla_to_page(sla);
+
+ buf = vm_map_ram(&pg, 1, 0);
+ }
+
+ return buf;
+}
+
+static void sla_buffer_unmap(struct sla_addr_t sla, struct sla_buffer_hdr *buf)
+{
+ if (!buf)
+ return;
+
+ if (sla.page_type == SLA_PAGE_TYPE_SCATTER) {
+ struct sla_addr_t *scatter = sla_to_va(sla);
+ unsigned int i, npages = 0;
+
+ for (i = 0; i < SLA_SCATTER_LEN(sla); ++i) {
+ if (IS_SLA_EOL(scatter[i])) {
+ npages = i;
+ break;
+ }
+ }
+ if (!npages)
+ return;
+
+ vm_unmap_ram(buf, npages);
+ } else {
+ vm_unmap_ram(buf, 1);
+ }
+}
+
+static void dobj_response_init(struct sla_buffer_hdr *buf)
+{
+ struct spdm_dobj_hdr *dobj = sla_to_dobj_hdr(buf);
+
+ dobj->id = SPDM_DOBJ_ID_RESP;
+ dobj->version.major = 0x1;
+ dobj->version.minor = 0;
+ dobj->length = 0;
+ buf->payload_sz = sla_dobj_id_to_size(dobj->id) + dobj->length;
+}
+
+static void sla_free(struct sla_addr_t sla, size_t len, bool firmware_state)
+{
+ unsigned int npages = PAGE_ALIGN(len) >> PAGE_SHIFT;
+ struct sla_addr_t *scatter = NULL;
+ int ret = 0, i;
+
+ if (IS_SLA_NULL(sla))
+ return;
+
+ if (firmware_state) {
+ if (sla.page_type == SLA_PAGE_TYPE_SCATTER) {
+ scatter = sla_to_va(sla);
+
+ for (i = 0; i < npages; ++i) {
+ if (IS_SLA_EOL(scatter[i]))
+ break;
+
+ ret = snp_reclaim_pages(sla_to_pa(scatter[i]), 1, false);
+ if (ret)
+ break;
+ }
+ } else {
+ ret = snp_reclaim_pages(sla_to_pa(sla), 1, false);
+ }
+ }
+
+ if (WARN_ON(ret))
+ return;
+
+ if (scatter) {
+ for (i = 0; i < npages; ++i) {
+ if (IS_SLA_EOL(scatter[i]))
+ break;
+ free_page((unsigned long)sla_to_va(scatter[i]));
+ }
+ }
+
+ free_page((unsigned long)sla_to_va(sla));
+}
+
+static struct sla_addr_t sla_alloc(size_t len, bool firmware_state)
+{
+ unsigned long i, npages = PAGE_ALIGN(len) >> PAGE_SHIFT;
+ struct sla_addr_t *scatter = NULL;
+ struct sla_addr_t ret = SLA_NULL;
+ struct sla_buffer_hdr *buf;
+ struct page *pg;
+
+ if (npages == 0)
+ return ret;
+
+ if (WARN_ON_ONCE(npages > ((PAGE_SIZE / sizeof(struct sla_addr_t)) + 1)))
+ return ret;
+
+ BUILD_BUG_ON(PAGE_SIZE < SZ_4K);
+
+ if (npages > 1) {
+ pg = alloc_page(GFP_KERNEL | __GFP_ZERO);
+ if (!pg)
+ return SLA_NULL;
+
+ ret = make_sla(pg, true);
+ scatter = page_to_virt(pg);
+ for (i = 0; i < npages; ++i) {
+ pg = alloc_page(GFP_KERNEL | __GFP_ZERO);
+ if (!pg)
+ goto no_reclaim_exit;
+
+ scatter[i] = make_sla(pg, false);
+ }
+ scatter[i] = SLA_EOL;
+ } else {
+ pg = alloc_page(GFP_KERNEL | __GFP_ZERO);
+ if (!pg)
+ return SLA_NULL;
+
+ ret = make_sla(pg, false);
+ }
+
+ buf = sla_buffer_map(ret);
+ if (!buf)
+ goto no_reclaim_exit;
+
+ buf->capacity_sz = (npages << PAGE_SHIFT);
+ sla_buffer_unmap(ret, buf);
+
+ if (firmware_state) {
+ if (scatter) {
+ for (i = 0; i < npages; ++i) {
+ if (rmp_make_private(sla_to_pfn(scatter[i]), 0,
+ PG_LEVEL_4K, 0, true))
+ goto free_exit;
+ }
+ } else {
+ if (rmp_make_private(sla_to_pfn(ret), 0, PG_LEVEL_4K, 0, true))
+ goto no_reclaim_exit;
+ }
+ }
+
+ return ret;
+
+no_reclaim_exit:
+ firmware_state = false;
+free_exit:
+ sla_free(ret, len, firmware_state);
+ return SLA_NULL;
+}
+
+/* Expands a buffer, only firmware owned buffers allowed for now */
+static int sla_expand(struct sla_addr_t *sla, size_t *len)
+{
+ struct sla_buffer_hdr *oldbuf = sla_buffer_map(*sla), *newbuf;
+ struct sla_addr_t oldsla = *sla, newsla;
+ size_t oldlen = *len, newlen;
+
+ if (!oldbuf)
+ return -EFAULT;
+
+ newlen = oldbuf->capacity_sz;
+ if (oldbuf->capacity_sz == oldlen) {
+ /* This buffer does not require expansion, must be another buffer */
+ sla_buffer_unmap(oldsla, oldbuf);
+ return 1;
+ }
+
+ pr_notice("Expanding BUFFER from %ld to %ld bytes\n", oldlen, newlen);
+
+ newsla = sla_alloc(newlen, true);
+ if (IS_SLA_NULL(newsla))
+ return -ENOMEM;
+
+ newbuf = sla_buffer_map(newsla);
+ if (!newbuf) {
+ sla_free(newsla, newlen, true);
+ return -EFAULT;
+ }
+
+ memcpy(newbuf, oldbuf, oldlen);
+
+ sla_buffer_unmap(newsla, newbuf);
+ sla_free(oldsla, oldlen, true);
+ *sla = newsla;
+ *len = newlen;
+
+ return 0;
+}
+
+static int sev_tio_do_cmd(int cmd, void *data, size_t data_len, int *psp_ret,
+ struct tsm_dsm_tio *dev_data)
+{
+ int rc;
+
+ *psp_ret = 0;
+ rc = sev_do_cmd(cmd, data, psp_ret);
+
+ if (WARN_ON(!rc && *psp_ret == SEV_RET_SPDM_REQUEST))
+ return -EIO;
+
+ if (rc == 0 && *psp_ret == SEV_RET_EXPAND_BUFFER_LENGTH_REQUEST) {
+ int rc1, rc2;
+
+ rc1 = sla_expand(&dev_data->output, &dev_data->output_len);
+ if (rc1 < 0)
+ return rc1;
+
+ rc2 = sla_expand(&dev_data->scratch, &dev_data->scratch_len);
+ if (rc2 < 0)
+ return rc2;
+
+ if (!rc1 && !rc2)
+ /* Neither buffer requires expansion, this is wrong */
+ return -EFAULT;
+
+ *psp_ret = 0;
+ rc = sev_do_cmd(cmd, data, psp_ret);
+ }
+
+ if ((rc == 0 || rc == -EIO) && *psp_ret == SEV_RET_SPDM_REQUEST) {
+ struct spdm_dobj_hdr_resp *resp_hdr;
+ struct spdm_dobj_hdr_req *req_hdr;
+ struct sev_tio_status *tio_status = to_tio_status(dev_data);
+ size_t resp_len = tio_status->spdm_req_size_max -
+ (sla_dobj_id_to_size(SPDM_DOBJ_ID_RESP) + sizeof(struct sla_buffer_hdr));
+
+ if (!dev_data->cmd) {
+ if (WARN_ON_ONCE(!data_len || (data_len != *(u32 *) data)))
+ return -EINVAL;
+ if (WARN_ON(data_len > sizeof(dev_data->cmd_data)))
+ return -EFAULT;
+ memcpy(dev_data->cmd_data, data, data_len);
+ memset(&dev_data->cmd_data[data_len], 0xFF,
+ sizeof(dev_data->cmd_data) - data_len);
+ dev_data->cmd = cmd;
+ }
+
+ req_hdr = sla_to_dobj_req_hdr(dev_data->reqbuf);
+ resp_hdr = sla_to_dobj_resp_hdr(dev_data->respbuf);
+ switch (req_hdr->data_type) {
+ case DOBJ_DATA_TYPE_SPDM:
+ rc = PCI_DOE_FEATURE_CMA;
+ break;
+ case DOBJ_DATA_TYPE_SECURE_SPDM:
+ rc = PCI_DOE_FEATURE_SSESSION;
+ break;
+ default:
+ return -EINVAL;
+ }
+ resp_hdr->data_type = req_hdr->data_type;
+ dev_data->spdm.req_len = req_hdr->hdr.length -
+ sla_dobj_id_to_size(SPDM_DOBJ_ID_REQ);
+ dev_data->spdm.rsp_len = resp_len;
+ } else if (dev_data && dev_data->cmd) {
+ /* For either error or success just stop the bouncing */
+ memset(dev_data->cmd_data, 0, sizeof(dev_data->cmd_data));
+ dev_data->cmd = 0;
+ }
+
+ return rc;
+}
+
+int sev_tio_continue(struct tsm_dsm_tio *dev_data)
+{
+ struct spdm_dobj_hdr_resp *resp_hdr;
+ int ret;
+
+ if (!dev_data || !dev_data->cmd)
+ return -EINVAL;
+
+ resp_hdr = sla_to_dobj_resp_hdr(dev_data->respbuf);
+ resp_hdr->hdr.length = ALIGN(sla_dobj_id_to_size(SPDM_DOBJ_ID_RESP) +
+ dev_data->spdm.rsp_len, 32);
+ dev_data->respbuf->payload_sz = resp_hdr->hdr.length;
+
+ ret = sev_tio_do_cmd(dev_data->cmd, dev_data->cmd_data, 0,
+ &dev_data->psp_ret, dev_data);
+ if (ret)
+ return ret;
+
+ if (dev_data->psp_ret != SEV_RET_SUCCESS)
+ return -EINVAL;
+
+ return 0;
+}
+
+static void spdm_ctrl_init(struct spdm_ctrl *ctrl, struct tsm_dsm_tio *dev_data)
+{
+ ctrl->req = dev_data->req;
+ ctrl->resp = dev_data->resp;
+ ctrl->scratch = dev_data->scratch;
+ ctrl->output = dev_data->output;
+}
+
+static void spdm_ctrl_free(struct tsm_dsm_tio *dev_data)
+{
+ struct sev_tio_status *tio_status = to_tio_status(dev_data);
+ size_t len = tio_status->spdm_req_size_max -
+ (sla_dobj_id_to_size(SPDM_DOBJ_ID_RESP) +
+ sizeof(struct sla_buffer_hdr));
+ struct tsm_spdm *spdm = &dev_data->spdm;
+
+ sla_buffer_unmap(dev_data->resp, dev_data->respbuf);
+ sla_buffer_unmap(dev_data->req, dev_data->reqbuf);
+ spdm->rsp = NULL;
+ spdm->req = NULL;
+ sla_free(dev_data->req, len, true);
+ sla_free(dev_data->resp, len, false);
+ sla_free(dev_data->scratch, tio_status->spdm_scratch_size_max, true);
+
+ dev_data->req.sla = 0;
+ dev_data->resp.sla = 0;
+ dev_data->scratch.sla = 0;
+ dev_data->respbuf = NULL;
+ dev_data->reqbuf = NULL;
+ sla_free(dev_data->output, tio_status->spdm_out_size_max, true);
+}
+
+static int spdm_ctrl_alloc(struct tsm_dsm_tio *dev_data)
+{
+ struct sev_tio_status *tio_status = to_tio_status(dev_data);
+ struct tsm_spdm *spdm = &dev_data->spdm;
+ int ret;
+
+ dev_data->req = sla_alloc(tio_status->spdm_req_size_max, true);
+ dev_data->resp = sla_alloc(tio_status->spdm_req_size_max, false);
+ dev_data->scratch_len = tio_status->spdm_scratch_size_max;
+ dev_data->scratch = sla_alloc(dev_data->scratch_len, true);
+ dev_data->output_len = tio_status->spdm_out_size_max;
+ dev_data->output = sla_alloc(dev_data->output_len, true);
+
+ if (IS_SLA_NULL(dev_data->req) || IS_SLA_NULL(dev_data->resp) ||
+ IS_SLA_NULL(dev_data->scratch) || IS_SLA_NULL(dev_data->dev_ctx)) {
+ ret = -ENOMEM;
+ goto free_spdm_exit;
+ }
+
+ dev_data->reqbuf = sla_buffer_map(dev_data->req);
+ dev_data->respbuf = sla_buffer_map(dev_data->resp);
+ if (!dev_data->reqbuf || !dev_data->respbuf) {
+ ret = -EFAULT;
+ goto free_spdm_exit;
+ }
+
+ spdm->req = sla_to_data(dev_data->reqbuf, SPDM_DOBJ_ID_REQ);
+ spdm->rsp = sla_to_data(dev_data->respbuf, SPDM_DOBJ_ID_RESP);
+ if (!spdm->req || !spdm->rsp) {
+ ret = -EFAULT;
+ goto free_spdm_exit;
+ }
+
+ dobj_response_init(dev_data->respbuf);
+
+ return 0;
+
+free_spdm_exit:
+ spdm_ctrl_free(dev_data);
+ return ret;
+}
+
+int sev_tio_init_locked(void *tio_status_page)
+{
+ struct sev_tio_status *tio_status = tio_status_page;
+ struct sev_data_tio_status data_status = {
+ .length = sizeof(data_status),
+ };
+ int ret, psp_ret;
+
+ data_status.status_paddr = __psp_pa(tio_status_page);
+ ret = __sev_do_cmd_locked(SEV_CMD_TIO_STATUS, &data_status, &psp_ret);
+ if (ret)
+ return ret;
+
+ if (tio_status->length < offsetofend(struct sev_tio_status, tdictx_size) ||
+ tio_status->reserved)
+ return -EFAULT;
+
+ if (!tio_status->tio_en && !tio_status->tio_init_done)
+ return -ENOENT;
+
+ if (tio_status->tio_init_done)
+ return -EBUSY;
+
+ struct sev_data_tio_init ti = { .length = sizeof(ti) };
+
+ ret = __sev_do_cmd_locked(SEV_CMD_TIO_INIT, &ti, &psp_ret);
+ if (ret)
+ return ret;
+
+ ret = __sev_do_cmd_locked(SEV_CMD_TIO_STATUS, &data_status, &psp_ret);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+int sev_tio_dev_create(struct tsm_dsm_tio *dev_data, u16 device_id,
+ u16 root_port_id, u8 segment_id)
+{
+ struct sev_tio_status *tio_status = to_tio_status(dev_data);
+ struct sev_data_tio_dev_create create = {
+ .length = sizeof(create),
+ .device_id = device_id,
+ .root_port_id = root_port_id,
+ .segment_id = segment_id,
+ };
+ void *data_pg;
+ int ret;
+
+ dev_data->dev_ctx = sla_alloc(tio_status->devctx_size, true);
+ if (IS_SLA_NULL(dev_data->dev_ctx))
+ return -ENOMEM;
+
+ data_pg = snp_alloc_firmware_page(GFP_KERNEL_ACCOUNT);
+ if (!data_pg) {
+ ret = -ENOMEM;
+ goto free_ctx_exit;
+ }
+
+ create.dev_ctx_sla = dev_data->dev_ctx;
+ ret = sev_do_cmd(SEV_CMD_TIO_DEV_CREATE, &create, &dev_data->psp_ret);
+ if (ret)
+ goto free_data_pg_exit;
+
+ dev_data->data_pg = data_pg;
+
+ return 0;
+
+free_data_pg_exit:
+ snp_free_firmware_page(data_pg);
+free_ctx_exit:
+ sla_free(create.dev_ctx_sla, tio_status->devctx_size, true);
+ return ret;
+}
+
+int sev_tio_dev_reclaim(struct tsm_dsm_tio *dev_data)
+{
+ struct sev_tio_status *tio_status = to_tio_status(dev_data);
+ struct sev_data_tio_dev_reclaim r = {
+ .length = sizeof(r),
+ .dev_ctx_sla = dev_data->dev_ctx,
+ };
+ int ret;
+
+ if (dev_data->data_pg) {
+ snp_free_firmware_page(dev_data->data_pg);
+ dev_data->data_pg = NULL;
+ }
+
+ if (IS_SLA_NULL(dev_data->dev_ctx))
+ return 0;
+
+ ret = sev_do_cmd(SEV_CMD_TIO_DEV_RECLAIM, &r, &dev_data->psp_ret);
+
+ sla_free(dev_data->dev_ctx, tio_status->devctx_size, true);
+ dev_data->dev_ctx = SLA_NULL;
+
+ spdm_ctrl_free(dev_data);
+
+ return ret;
+}
+
+int sev_tio_dev_connect(struct tsm_dsm_tio *dev_data, u8 tc_mask, u8 ids[8], u8 cert_slot)
+{
+ struct sev_data_tio_dev_connect connect = {
+ .length = sizeof(connect),
+ .tc_mask = tc_mask,
+ .cert_slot = cert_slot,
+ .dev_ctx_sla = dev_data->dev_ctx,
+ .ide_stream_id = {
+ ids[0], ids[1], ids[2], ids[3],
+ ids[4], ids[5], ids[6], ids[7]
+ },
+ };
+ int ret;
+
+ if (WARN_ON(IS_SLA_NULL(dev_data->dev_ctx)))
+ return -EFAULT;
+ if (!(tc_mask & 1))
+ return -EINVAL;
+
+ ret = spdm_ctrl_alloc(dev_data);
+ if (ret)
+ return ret;
+
+ spdm_ctrl_init(&connect.spdm_ctrl, dev_data);
+
+ return sev_tio_do_cmd(SEV_CMD_TIO_DEV_CONNECT, &connect, sizeof(connect),
+ &dev_data->psp_ret, dev_data);
+}
+
+int sev_tio_dev_disconnect(struct tsm_dsm_tio *dev_data, bool force)
+{
+ struct sev_data_tio_dev_disconnect dc = {
+ .length = sizeof(dc),
+ .dev_ctx_sla = dev_data->dev_ctx,
+ .flags = force ? TIO_DEV_DISCONNECT_FLAG_FORCE : 0,
+ };
+
+ if (WARN_ON_ONCE(IS_SLA_NULL(dev_data->dev_ctx)))
+ return -EFAULT;
+
+ spdm_ctrl_init(&dc.spdm_ctrl, dev_data);
+
+ return sev_tio_do_cmd(SEV_CMD_TIO_DEV_DISCONNECT, &dc, sizeof(dc),
+ &dev_data->psp_ret, dev_data);
+}
+
+int sev_tio_cmd_buffer_len(int cmd)
+{
+ switch (cmd) {
+ case SEV_CMD_TIO_STATUS: return sizeof(struct sev_data_tio_status);
+ case SEV_CMD_TIO_INIT: return sizeof(struct sev_data_tio_init);
+ case SEV_CMD_TIO_DEV_CREATE: return sizeof(struct sev_data_tio_dev_create);
+ case SEV_CMD_TIO_DEV_RECLAIM: return sizeof(struct sev_data_tio_dev_reclaim);
+ case SEV_CMD_TIO_DEV_CONNECT: return sizeof(struct sev_data_tio_dev_connect);
+ case SEV_CMD_TIO_DEV_DISCONNECT: return sizeof(struct sev_data_tio_dev_disconnect);
+ default: return 0;
+ }
+}
diff --git a/drivers/crypto/ccp/sev-dev-tsm.c b/drivers/crypto/ccp/sev-dev-tsm.c
new file mode 100644
index 000000000000..ea29cd5d0ff9
--- /dev/null
+++ b/drivers/crypto/ccp/sev-dev-tsm.c
@@ -0,0 +1,405 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+// Interface to CCP/SEV-TIO for generic PCIe TDISP module
+
+#include <linux/pci.h>
+#include <linux/device.h>
+#include <linux/tsm.h>
+#include <linux/iommu.h>
+#include <linux/pci-doe.h>
+#include <linux/bitfield.h>
+#include <linux/module.h>
+
+#include <asm/sev-common.h>
+#include <asm/sev.h>
+
+#include "psp-dev.h"
+#include "sev-dev.h"
+#include "sev-dev-tio.h"
+
+MODULE_IMPORT_NS("PCI_IDE");
+
+#define TIO_DEFAULT_NR_IDE_STREAMS 1
+
+static uint nr_ide_streams = TIO_DEFAULT_NR_IDE_STREAMS;
+module_param_named(ide_nr, nr_ide_streams, uint, 0644);
+MODULE_PARM_DESC(ide_nr, "Set the maximum number of IDE streams per PHB");
+
+#define dev_to_sp(dev) ((struct sp_device *)dev_get_drvdata(dev))
+#define dev_to_psp(dev) ((struct psp_device *)(dev_to_sp(dev)->psp_data))
+#define dev_to_sev(dev) ((struct sev_device *)(dev_to_psp(dev)->sev_data))
+#define tsm_dev_to_sev(tsmdev) dev_to_sev((tsmdev)->dev.parent)
+
+#define pdev_to_tio_dsm(pdev) (container_of((pdev)->tsm, struct tio_dsm, tsm.base_tsm))
+
+static int sev_tio_spdm_cmd(struct tio_dsm *dsm, int ret)
+{
+ struct tsm_dsm_tio *dev_data = &dsm->data;
+ struct tsm_spdm *spdm = &dev_data->spdm;
+
+ /* Check the main command handler response before entering the loop */
+ if (ret == 0 && dev_data->psp_ret != SEV_RET_SUCCESS)
+ return -EINVAL;
+
+ if (ret <= 0)
+ return ret;
+
+ /* ret > 0 means "SPDM requested" */
+ while (ret == PCI_DOE_FEATURE_CMA || ret == PCI_DOE_FEATURE_SSESSION) {
+ ret = pci_doe(dsm->tsm.doe_mb, PCI_VENDOR_ID_PCI_SIG, ret,
+ spdm->req, spdm->req_len, spdm->rsp, spdm->rsp_len);
+ if (ret < 0)
+ break;
+
+ WARN_ON_ONCE(ret == 0); /* The response should never be empty */
+ spdm->rsp_len = ret;
+ ret = sev_tio_continue(dev_data);
+ }
+
+ return ret;
+}
+
+static int stream_enable(struct pci_ide *ide)
+{
+ struct pci_dev *rp = pcie_find_root_port(ide->pdev);
+ int ret;
+
+ ret = pci_ide_stream_enable(rp, ide);
+ if (ret)
+ return ret;
+
+ ret = pci_ide_stream_enable(ide->pdev, ide);
+ if (ret)
+ pci_ide_stream_disable(rp, ide);
+
+ return ret;
+}
+
+static int streams_enable(struct pci_ide **ide)
+{
+ int ret = 0;
+
+ for (int i = 0; i < TIO_IDE_MAX_TC; ++i) {
+ if (ide[i]) {
+ ret = stream_enable(ide[i]);
+ if (ret)
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static void stream_disable(struct pci_ide *ide)
+{
+ pci_ide_stream_disable(ide->pdev, ide);
+ pci_ide_stream_disable(pcie_find_root_port(ide->pdev), ide);
+}
+
+static void streams_disable(struct pci_ide **ide)
+{
+ for (int i = 0; i < TIO_IDE_MAX_TC; ++i)
+ if (ide[i])
+ stream_disable(ide[i]);
+}
+
+static void stream_setup(struct pci_ide *ide)
+{
+ struct pci_dev *rp = pcie_find_root_port(ide->pdev);
+
+ ide->partner[PCI_IDE_EP].rid_start = 0;
+ ide->partner[PCI_IDE_EP].rid_end = 0xffff;
+ ide->partner[PCI_IDE_RP].rid_start = 0;
+ ide->partner[PCI_IDE_RP].rid_end = 0xffff;
+
+ ide->pdev->ide_cfg = 0;
+ ide->pdev->ide_tee_limit = 1;
+ rp->ide_cfg = 1;
+ rp->ide_tee_limit = 0;
+
+ pci_warn(ide->pdev, "Forcing CFG/TEE for %s", pci_name(rp));
+ pci_ide_stream_setup(ide->pdev, ide);
+ pci_ide_stream_setup(rp, ide);
+}
+
+static u8 streams_setup(struct pci_ide **ide, u8 *ids)
+{
+ bool def = false;
+ u8 tc_mask = 0;
+ int i;
+
+ for (i = 0; i < TIO_IDE_MAX_TC; ++i) {
+ if (!ide[i]) {
+ ids[i] = 0xFF;
+ continue;
+ }
+
+ tc_mask |= BIT(i);
+ ids[i] = ide[i]->stream_id;
+
+ if (!def) {
+ struct pci_ide_partner *settings;
+
+ settings = pci_ide_to_settings(ide[i]->pdev, ide[i]);
+ settings->default_stream = 1;
+ def = true;
+ }
+
+ stream_setup(ide[i]);
+ }
+
+ return tc_mask;
+}
+
+static int streams_register(struct pci_ide **ide)
+{
+ int ret = 0, i;
+
+ for (i = 0; i < TIO_IDE_MAX_TC; ++i) {
+ if (ide[i]) {
+ ret = pci_ide_stream_register(ide[i]);
+ if (ret)
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static void streams_unregister(struct pci_ide **ide)
+{
+ for (int i = 0; i < TIO_IDE_MAX_TC; ++i)
+ if (ide[i])
+ pci_ide_stream_unregister(ide[i]);
+}
+
+static void stream_teardown(struct pci_ide *ide)
+{
+ pci_ide_stream_teardown(ide->pdev, ide);
+ pci_ide_stream_teardown(pcie_find_root_port(ide->pdev), ide);
+}
+
+static void streams_teardown(struct pci_ide **ide)
+{
+ for (int i = 0; i < TIO_IDE_MAX_TC; ++i) {
+ if (ide[i]) {
+ stream_teardown(ide[i]);
+ pci_ide_stream_free(ide[i]);
+ ide[i] = NULL;
+ }
+ }
+}
+
+static int stream_alloc(struct pci_dev *pdev, struct pci_ide **ide,
+ unsigned int tc)
+{
+ struct pci_dev *rp = pcie_find_root_port(pdev);
+ struct pci_ide *ide1;
+
+ if (ide[tc]) {
+ pci_err(pdev, "Stream for class=%d already registered", tc);
+ return -EBUSY;
+ }
+
+ /* FIXME: find a better way */
+ if (nr_ide_streams != TIO_DEFAULT_NR_IDE_STREAMS)
+ pci_notice(pdev, "Enable non-default %d streams", nr_ide_streams);
+ pci_ide_set_nr_streams(to_pci_host_bridge(rp->bus->bridge), nr_ide_streams);
+
+ ide1 = pci_ide_stream_alloc(pdev);
+ if (!ide1)
+ return -EFAULT;
+
+ /* Blindly assign streamid=0 to TC=0, and so on */
+ ide1->stream_id = tc;
+
+ ide[tc] = ide1;
+
+ return 0;
+}
+
+static struct pci_tsm *tio_pf0_probe(struct pci_dev *pdev, struct sev_device *sev)
+{
+ struct tio_dsm *dsm __free(kfree) = kzalloc(sizeof(*dsm), GFP_KERNEL);
+ int rc;
+
+ if (!dsm)
+ return NULL;
+
+ rc = pci_tsm_pf0_constructor(pdev, &dsm->tsm, sev->tsmdev);
+ if (rc)
+ return NULL;
+
+ pci_dbg(pdev, "TSM enabled\n");
+ dsm->sev = sev;
+ return &no_free_ptr(dsm)->tsm.base_tsm;
+}
+
+static struct pci_tsm *dsm_probe(struct tsm_dev *tsmdev, struct pci_dev *pdev)
+{
+ struct sev_device *sev = tsm_dev_to_sev(tsmdev);
+
+ if (is_pci_tsm_pf0(pdev))
+ return tio_pf0_probe(pdev, sev);
+ return 0;
+}
+
+static void dsm_remove(struct pci_tsm *tsm)
+{
+ struct pci_dev *pdev = tsm->pdev;
+
+ pci_dbg(pdev, "TSM disabled\n");
+
+ if (is_pci_tsm_pf0(pdev)) {
+ struct tio_dsm *dsm = container_of(tsm, struct tio_dsm, tsm.base_tsm);
+
+ pci_tsm_pf0_destructor(&dsm->tsm);
+ kfree(dsm);
+ }
+}
+
+static int dsm_create(struct tio_dsm *dsm)
+{
+ struct pci_dev *pdev = dsm->tsm.base_tsm.pdev;
+ u8 segment_id = pdev->bus ? pci_domain_nr(pdev->bus) : 0;
+ struct pci_dev *rootport = pcie_find_root_port(pdev);
+ u16 device_id = pci_dev_id(pdev);
+ u16 root_port_id;
+ u32 lnkcap = 0;
+
+ if (pci_read_config_dword(rootport, pci_pcie_cap(rootport) + PCI_EXP_LNKCAP,
+ &lnkcap))
+ return -ENODEV;
+
+ root_port_id = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
+
+ return sev_tio_dev_create(&dsm->data, device_id, root_port_id, segment_id);
+}
+
+static int dsm_connect(struct pci_dev *pdev)
+{
+ struct tio_dsm *dsm = pdev_to_tio_dsm(pdev);
+ struct tsm_dsm_tio *dev_data = &dsm->data;
+ u8 ids[TIO_IDE_MAX_TC];
+ u8 tc_mask;
+ int ret;
+
+ if (pci_find_doe_mailbox(pdev, PCI_VENDOR_ID_PCI_SIG,
+ PCI_DOE_FEATURE_SSESSION) != dsm->tsm.doe_mb) {
+ pci_err(pdev, "CMA DOE MB must support SSESSION\n");
+ return -EFAULT;
+ }
+
+ ret = stream_alloc(pdev, dev_data->ide, 0);
+ if (ret)
+ return ret;
+
+ ret = dsm_create(dsm);
+ if (ret)
+ goto ide_free_exit;
+
+ tc_mask = streams_setup(dev_data->ide, ids);
+
+ ret = sev_tio_dev_connect(dev_data, tc_mask, ids, dev_data->cert_slot);
+ ret = sev_tio_spdm_cmd(dsm, ret);
+ if (ret)
+ goto free_exit;
+
+ streams_enable(dev_data->ide);
+
+ ret = streams_register(dev_data->ide);
+ if (ret)
+ goto free_exit;
+
+ return 0;
+
+free_exit:
+ sev_tio_dev_reclaim(dev_data);
+
+ streams_disable(dev_data->ide);
+ide_free_exit:
+
+ streams_teardown(dev_data->ide);
+
+ return ret;
+}
+
+static void dsm_disconnect(struct pci_dev *pdev)
+{
+ bool force = SYSTEM_HALT <= system_state && system_state <= SYSTEM_RESTART;
+ struct tio_dsm *dsm = pdev_to_tio_dsm(pdev);
+ struct tsm_dsm_tio *dev_data = &dsm->data;
+ int ret;
+
+ ret = sev_tio_dev_disconnect(dev_data, force);
+ ret = sev_tio_spdm_cmd(dsm, ret);
+ if (ret && !force) {
+ ret = sev_tio_dev_disconnect(dev_data, true);
+ sev_tio_spdm_cmd(dsm, ret);
+ }
+
+ sev_tio_dev_reclaim(dev_data);
+
+ streams_disable(dev_data->ide);
+ streams_unregister(dev_data->ide);
+ streams_teardown(dev_data->ide);
+}
+
+static struct pci_tsm_ops sev_tsm_ops = {
+ .probe = dsm_probe,
+ .remove = dsm_remove,
+ .connect = dsm_connect,
+ .disconnect = dsm_disconnect,
+};
+
+void sev_tsm_init_locked(struct sev_device *sev, void *tio_status_page)
+{
+ struct sev_tio_status *t = kzalloc(sizeof(*t), GFP_KERNEL);
+ struct tsm_dev *tsmdev;
+ int ret;
+
+ WARN_ON(sev->tio_status);
+
+ if (!t)
+ return;
+
+ ret = sev_tio_init_locked(tio_status_page);
+ if (ret) {
+ pr_warn("SEV-TIO STATUS failed with %d\n", ret);
+ goto error_exit;
+ }
+
+ tsmdev = tsm_register(sev->dev, &sev_tsm_ops);
+ if (IS_ERR(tsmdev))
+ goto error_exit;
+
+ memcpy(t, tio_status_page, sizeof(*t));
+
+ pr_notice("SEV-TIO status: EN=%d INIT_DONE=%d rq=%d..%d rs=%d..%d "
+ "scr=%d..%d out=%d..%d dev=%d tdi=%d algos=%x\n",
+ t->tio_en, t->tio_init_done,
+ t->spdm_req_size_min, t->spdm_req_size_max,
+ t->spdm_rsp_size_min, t->spdm_rsp_size_max,
+ t->spdm_scratch_size_min, t->spdm_scratch_size_max,
+ t->spdm_out_size_min, t->spdm_out_size_max,
+ t->devctx_size, t->tdictx_size,
+ t->tio_crypto_alg);
+
+ sev->tsmdev = tsmdev;
+ sev->tio_status = t;
+
+ return;
+
+error_exit:
+ kfree(t);
+ pr_err("Failed to enable SEV-TIO: ret=%d en=%d initdone=%d SEV=%d\n",
+ ret, t->tio_en, t->tio_init_done, boot_cpu_has(X86_FEATURE_SEV));
+}
+
+void sev_tsm_uninit(struct sev_device *sev)
+{
+ if (sev->tsmdev)
+ tsm_unregister(sev->tsmdev);
+
+ sev->tsmdev = NULL;
+}
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 9e0c16b36f9c..d6095d1467b3 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -75,6 +75,10 @@ static bool psp_init_on_probe = true;
module_param(psp_init_on_probe, bool, 0444);
MODULE_PARM_DESC(psp_init_on_probe, " if true, the PSP will be initialized on module init. Else the PSP will be initialized on the first command requiring it");
+static bool sev_tio_enabled = IS_ENABLED(CONFIG_PCI_TSM);
+module_param_named(tio, sev_tio_enabled, bool, 0444);
+MODULE_PARM_DESC(tio, "Enables TIO in SNP_INIT_EX");
+
MODULE_FIRMWARE("amd/amd_sev_fam17h_model0xh.sbin"); /* 1st gen EPYC */
MODULE_FIRMWARE("amd/amd_sev_fam17h_model3xh.sbin"); /* 2nd gen EPYC */
MODULE_FIRMWARE("amd/amd_sev_fam19h_model0xh.sbin"); /* 3rd gen EPYC */
@@ -251,7 +255,7 @@ static int sev_cmd_buffer_len(int cmd)
case SEV_CMD_SNP_COMMIT: return sizeof(struct sev_data_snp_commit);
case SEV_CMD_SNP_FEATURE_INFO: return sizeof(struct sev_data_snp_feature_info);
case SEV_CMD_SNP_VLEK_LOAD: return sizeof(struct sev_user_data_snp_vlek_load);
- default: return 0;
+ default: return sev_tio_cmd_buffer_len(cmd);
}
return 0;
@@ -1434,6 +1438,19 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
data.init_rmp = 1;
data.list_paddr_en = 1;
data.list_paddr = __psp_pa(snp_range_list);
+
+ bool tio_supp = !!(sev->snp_feat_info_0.ebx & SNP_SEV_TIO_SUPPORTED);
+
+ data.tio_en = tio_supp && sev_tio_enabled && amd_iommu_sev_tio_supported();
+
+ /*
+ * When psp_init_on_probe is disabled, the userspace calling
+ * SEV ioctl can inadvertently shut down SNP and SEV-TIO causing
+ * unexpected state loss.
+ */
+ if (data.tio_en && !psp_init_on_probe)
+ dev_warn(sev->dev, "SEV-TIO as incompatible with psp_init_on_probe=0\n");
+
cmd = SEV_CMD_SNP_INIT_EX;
} else {
cmd = SEV_CMD_SNP_INIT;
@@ -1471,7 +1488,8 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
snp_hv_fixed_pages_state_update(sev, HV_FIXED);
sev->snp_initialized = true;
- dev_dbg(sev->dev, "SEV-SNP firmware initialized\n");
+ dev_dbg(sev->dev, "SEV-SNP firmware initialized, SEV-TIO is %s\n",
+ data.tio_en ? "enabled" : "disabled");
dev_info(sev->dev, "SEV-SNP API:%d.%d build:%d\n", sev->api_major,
sev->api_minor, sev->build);
@@ -1479,6 +1497,23 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
atomic_notifier_chain_register(&panic_notifier_list,
&snp_panic_notifier);
+ if (data.tio_en) {
+ /*
+ * This executes with the sev_cmd_mutex held so down the stack
+ * snp_reclaim_pages(locked=false) might be needed (which is extremely
+ * unlikely) but will cause a deadlock.
+ * Instead of exporting __snp_alloc_firmware_pages(), allocate a page
+ * for this one call here.
+ */
+ void *tio_status = page_address(__snp_alloc_firmware_pages(
+ GFP_KERNEL_ACCOUNT | __GFP_ZERO, 0, true));
+
+ if (tio_status) {
+ sev_tsm_init_locked(sev, tio_status);
+ __snp_free_firmware_pages(virt_to_page(tio_status), 0, true);
+ }
+ }
+
sev_es_tmr_size = SNP_TMR_SIZE;
return 0;
@@ -2758,8 +2793,20 @@ static void __sev_firmware_shutdown(struct sev_device *sev, bool panic)
static void sev_firmware_shutdown(struct sev_device *sev)
{
+ /*
+ * Calling without sev_cmd_mutex held as TSM will likely try disconnecting
+ * IDE and this ends up calling sev_do_cmd() which locks sev_cmd_mutex.
+ */
+ if (sev->tio_status)
+ sev_tsm_uninit(sev);
+
mutex_lock(&sev_cmd_mutex);
+
__sev_firmware_shutdown(sev, false);
+
+ kfree(sev->tio_status);
+ sev->tio_status = NULL;
+
mutex_unlock(&sev_cmd_mutex);
}
--
2.51.1
^ permalink raw reply related
* [PATCH kernel v3 3/4] iommu/amd: Report SEV-TIO support
From: Alexey Kardashevskiy @ 2025-12-02 2:44 UTC (permalink / raw)
To: linux-kernel
Cc: linux-crypto, Tom Lendacky, John Allen, Herbert Xu,
David S. Miller, Ashish Kalra, Joerg Roedel,
Suravee Suthikulpanit, Will Deacon, Robin Murphy, Borislav Petkov,
Borislav Petkov (AMD), Dan Williams, Jason Gunthorpe,
Jerry Snitselaar, Vasant Hegde, Gao Shiyuan, Sean Christopherson,
Kim Phillips, Nikunj A Dadhania, Michael Roth, Paolo Bonzini,
iommu, Alexey Kardashevskiy, x86, linux-coco, Joerg Roedel
In-Reply-To: <20251202024449.542361-1-aik@amd.com>
The SEV-TIO switch in the AMD BIOS is reported to the OS via
the IOMMU Extended Feature 2 register (EFR2), bit 1.
Add helper to parse the bit and report the feature presence.
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
Acked-by: Joerg Roedel <joerg.roedel@amd.com>
Link: https://patch.msgid.link/20251121080629.444992-4-aik@amd.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/iommu/amd/amd_iommu_types.h | 1 +
include/linux/amd-iommu.h | 2 ++
drivers/iommu/amd/init.c | 9 +++++++++
3 files changed, 12 insertions(+)
diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index a698a2e7ce2a..a2f72c53d3cc 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -107,6 +107,7 @@
/* Extended Feature 2 Bits */
+#define FEATURE_SEVSNPIO_SUP BIT_ULL(1)
#define FEATURE_SNPAVICSUP GENMASK_ULL(7, 5)
#define FEATURE_SNPAVICSUP_GAM(x) \
(FIELD_GET(FEATURE_SNPAVICSUP, x) == 0x1)
diff --git a/include/linux/amd-iommu.h b/include/linux/amd-iommu.h
index 8cced632ecd0..0f64f09d1f34 100644
--- a/include/linux/amd-iommu.h
+++ b/include/linux/amd-iommu.h
@@ -18,10 +18,12 @@ struct task_struct;
struct pci_dev;
extern void amd_iommu_detect(void);
+extern bool amd_iommu_sev_tio_supported(void);
#else /* CONFIG_AMD_IOMMU */
static inline void amd_iommu_detect(void) { }
+static inline bool amd_iommu_sev_tio_supported(void) { return false; }
#endif /* CONFIG_AMD_IOMMU */
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index f2991c11867c..ba95467ba492 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -2252,6 +2252,9 @@ static void print_iommu_info(void)
if (check_feature(FEATURE_SNP))
pr_cont(" SNP");
+ if (check_feature2(FEATURE_SEVSNPIO_SUP))
+ pr_cont(" SEV-TIO");
+
pr_cont("\n");
}
@@ -4015,4 +4018,10 @@ int amd_iommu_snp_disable(void)
return 0;
}
EXPORT_SYMBOL_GPL(amd_iommu_snp_disable);
+
+bool amd_iommu_sev_tio_supported(void)
+{
+ return check_feature2(FEATURE_SEVSNPIO_SUP);
+}
+EXPORT_SYMBOL_GPL(amd_iommu_sev_tio_supported);
#endif
--
2.51.1
^ permalink raw reply related
* [PATCH kernel v3 2/4] psp-sev: Assign numbers to all status codes and add new
From: Alexey Kardashevskiy @ 2025-12-02 2:44 UTC (permalink / raw)
To: linux-kernel
Cc: linux-crypto, Tom Lendacky, John Allen, Herbert Xu,
David S. Miller, Ashish Kalra, Joerg Roedel,
Suravee Suthikulpanit, Will Deacon, Robin Murphy, Borislav Petkov,
Borislav Petkov (AMD), Dan Williams, Jason Gunthorpe,
Jerry Snitselaar, Vasant Hegde, Gao Shiyuan, Sean Christopherson,
Kim Phillips, Nikunj A Dadhania, Michael Roth, Paolo Bonzini,
iommu, Alexey Kardashevskiy, x86, linux-coco
In-Reply-To: <20251202024449.542361-1-aik@amd.com>
Make the definitions explicit. Add some more new codes.
The following patches will be using SPDM_REQUEST and
EXPAND_BUFFER_LENGTH_REQUEST, others are useful for the PSP FW
diagnostics.
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
Link: https://patch.msgid.link/20251121080629.444992-3-aik@amd.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
include/uapi/linux/psp-sev.h | 66 ++++++++++++--------
1 file changed, 41 insertions(+), 25 deletions(-)
diff --git a/include/uapi/linux/psp-sev.h b/include/uapi/linux/psp-sev.h
index c2fd324623c4..2b5b042eb73b 100644
--- a/include/uapi/linux/psp-sev.h
+++ b/include/uapi/linux/psp-sev.h
@@ -47,32 +47,32 @@ typedef enum {
* with possible values from the specification.
*/
SEV_RET_NO_FW_CALL = -1,
- SEV_RET_SUCCESS = 0,
- SEV_RET_INVALID_PLATFORM_STATE,
- SEV_RET_INVALID_GUEST_STATE,
- SEV_RET_INAVLID_CONFIG,
+ SEV_RET_SUCCESS = 0,
+ SEV_RET_INVALID_PLATFORM_STATE = 0x0001,
+ SEV_RET_INVALID_GUEST_STATE = 0x0002,
+ SEV_RET_INAVLID_CONFIG = 0x0003,
SEV_RET_INVALID_CONFIG = SEV_RET_INAVLID_CONFIG,
- SEV_RET_INVALID_LEN,
- SEV_RET_ALREADY_OWNED,
- SEV_RET_INVALID_CERTIFICATE,
- SEV_RET_POLICY_FAILURE,
- SEV_RET_INACTIVE,
- SEV_RET_INVALID_ADDRESS,
- SEV_RET_BAD_SIGNATURE,
- SEV_RET_BAD_MEASUREMENT,
- SEV_RET_ASID_OWNED,
- SEV_RET_INVALID_ASID,
- SEV_RET_WBINVD_REQUIRED,
- SEV_RET_DFFLUSH_REQUIRED,
- SEV_RET_INVALID_GUEST,
- SEV_RET_INVALID_COMMAND,
- SEV_RET_ACTIVE,
- SEV_RET_HWSEV_RET_PLATFORM,
- SEV_RET_HWSEV_RET_UNSAFE,
- SEV_RET_UNSUPPORTED,
- SEV_RET_INVALID_PARAM,
- SEV_RET_RESOURCE_LIMIT,
- SEV_RET_SECURE_DATA_INVALID,
+ SEV_RET_INVALID_LEN = 0x0004,
+ SEV_RET_ALREADY_OWNED = 0x0005,
+ SEV_RET_INVALID_CERTIFICATE = 0x0006,
+ SEV_RET_POLICY_FAILURE = 0x0007,
+ SEV_RET_INACTIVE = 0x0008,
+ SEV_RET_INVALID_ADDRESS = 0x0009,
+ SEV_RET_BAD_SIGNATURE = 0x000A,
+ SEV_RET_BAD_MEASUREMENT = 0x000B,
+ SEV_RET_ASID_OWNED = 0x000C,
+ SEV_RET_INVALID_ASID = 0x000D,
+ SEV_RET_WBINVD_REQUIRED = 0x000E,
+ SEV_RET_DFFLUSH_REQUIRED = 0x000F,
+ SEV_RET_INVALID_GUEST = 0x0010,
+ SEV_RET_INVALID_COMMAND = 0x0011,
+ SEV_RET_ACTIVE = 0x0012,
+ SEV_RET_HWSEV_RET_PLATFORM = 0x0013,
+ SEV_RET_HWSEV_RET_UNSAFE = 0x0014,
+ SEV_RET_UNSUPPORTED = 0x0015,
+ SEV_RET_INVALID_PARAM = 0x0016,
+ SEV_RET_RESOURCE_LIMIT = 0x0017,
+ SEV_RET_SECURE_DATA_INVALID = 0x0018,
SEV_RET_INVALID_PAGE_SIZE = 0x0019,
SEV_RET_INVALID_PAGE_STATE = 0x001A,
SEV_RET_INVALID_MDATA_ENTRY = 0x001B,
@@ -87,6 +87,22 @@ typedef enum {
SEV_RET_RESTORE_REQUIRED = 0x0025,
SEV_RET_RMP_INITIALIZATION_FAILED = 0x0026,
SEV_RET_INVALID_KEY = 0x0027,
+ SEV_RET_SHUTDOWN_INCOMPLETE = 0x0028,
+ SEV_RET_INCORRECT_BUFFER_LENGTH = 0x0030,
+ SEV_RET_EXPAND_BUFFER_LENGTH_REQUEST = 0x0031,
+ SEV_RET_SPDM_REQUEST = 0x0032,
+ SEV_RET_SPDM_ERROR = 0x0033,
+ SEV_RET_SEV_STATUS_ERR_IN_DEV_CONN = 0x0035,
+ SEV_RET_SEV_STATUS_INVALID_DEV_CTX = 0x0036,
+ SEV_RET_SEV_STATUS_INVALID_TDI_CTX = 0x0037,
+ SEV_RET_SEV_STATUS_INVALID_TDI = 0x0038,
+ SEV_RET_SEV_STATUS_RECLAIM_REQUIRED = 0x0039,
+ SEV_RET_IN_USE = 0x003A,
+ SEV_RET_SEV_STATUS_INVALID_DEV_STATE = 0x003B,
+ SEV_RET_SEV_STATUS_INVALID_TDI_STATE = 0x003C,
+ SEV_RET_SEV_STATUS_DEV_CERT_CHANGED = 0x003D,
+ SEV_RET_SEV_STATUS_RESYNC_REQ = 0x003E,
+ SEV_RET_SEV_STATUS_RESPONSE_TOO_LARGE = 0x003F,
SEV_RET_MAX,
} sev_ret_code;
--
2.51.1
^ permalink raw reply related
* [PATCH kernel v3 1/4] ccp: Make snp_reclaim_pages and __sev_do_cmd_locked public
From: Alexey Kardashevskiy @ 2025-12-02 2:44 UTC (permalink / raw)
To: linux-kernel
Cc: linux-crypto, Tom Lendacky, John Allen, Herbert Xu,
David S. Miller, Ashish Kalra, Joerg Roedel,
Suravee Suthikulpanit, Will Deacon, Robin Murphy, Borislav Petkov,
Borislav Petkov (AMD), Dan Williams, Jason Gunthorpe,
Jerry Snitselaar, Vasant Hegde, Gao Shiyuan, Sean Christopherson,
Kim Phillips, Nikunj A Dadhania, Michael Roth, Paolo Bonzini,
iommu, Alexey Kardashevskiy, x86, linux-coco
In-Reply-To: <20251202024449.542361-1-aik@amd.com>
The snp_reclaim_pages() helper reclaims pages in the FW state. SEV-TIO
and the TMPM driver (a hardware engine which smashes IOMMU PDEs among
other things) will use to reclaim memory when cleaning up.
Share and export snp_reclaim_pages().
Most of the SEV-TIO code uses sev_do_cmd() which locks the sev_cmd_mutex
and already exported. But the SNP init code (which also sets up SEV-TIO)
executes under the sev_cmd_mutex lock so the SEV-TIO code has to use
the __sev_do_cmd_locked() helper. This one though does not need to be
exported/shared globally as SEV-TIO is a part of the CCP driver still.
Share __sev_do_cmd_locked() via the CCP internal header.
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
Link: https://patch.msgid.link/20251121080629.444992-2-aik@amd.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/crypto/ccp/sev-dev.h | 2 ++
include/linux/psp-sev.h | 6 ++++++
drivers/crypto/ccp/sev-dev.c | 11 +++--------
3 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
index ac03bd0848f7..b9029506383f 100644
--- a/drivers/crypto/ccp/sev-dev.h
+++ b/drivers/crypto/ccp/sev-dev.h
@@ -66,6 +66,8 @@ struct sev_device {
int sev_dev_init(struct psp_device *psp);
void sev_dev_destroy(struct psp_device *psp);
+int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret);
+
void sev_pci_init(void);
void sev_pci_exit(void);
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index e0dbcb4b4fd9..34a25209f909 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -992,6 +992,7 @@ int sev_do_cmd(int cmd, void *data, int *psp_ret);
void *psp_copy_user_blob(u64 uaddr, u32 len);
void *snp_alloc_firmware_page(gfp_t mask);
+int snp_reclaim_pages(unsigned long paddr, unsigned int npages, bool locked);
void snp_free_firmware_page(void *addr);
void sev_platform_shutdown(void);
bool sev_is_snp_ciphertext_hiding_supported(void);
@@ -1027,6 +1028,11 @@ static inline void *snp_alloc_firmware_page(gfp_t mask)
return NULL;
}
+static inline int snp_reclaim_pages(unsigned long paddr, unsigned int npages, bool locked)
+{
+ return -ENODEV;
+}
+
static inline void snp_free_firmware_page(void *addr) { }
static inline void sev_platform_shutdown(void) { }
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 0d13d47c164b..9e0c16b36f9c 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -387,13 +387,7 @@ static int sev_write_init_ex_file_if_required(int cmd_id)
return sev_write_init_ex_file();
}
-/*
- * snp_reclaim_pages() needs __sev_do_cmd_locked(), and __sev_do_cmd_locked()
- * needs snp_reclaim_pages(), so a forward declaration is needed.
- */
-static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret);
-
-static int snp_reclaim_pages(unsigned long paddr, unsigned int npages, bool locked)
+int snp_reclaim_pages(unsigned long paddr, unsigned int npages, bool locked)
{
int ret, err, i;
@@ -427,6 +421,7 @@ static int snp_reclaim_pages(unsigned long paddr, unsigned int npages, bool lock
snp_leak_pages(__phys_to_pfn(paddr), npages - i);
return ret;
}
+EXPORT_SYMBOL_GPL(snp_reclaim_pages);
static int rmp_mark_pages_firmware(unsigned long paddr, unsigned int npages, bool locked)
{
@@ -857,7 +852,7 @@ static int snp_reclaim_cmd_buf(int cmd, void *cmd_buf)
return 0;
}
-static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret)
+int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret)
{
struct cmd_buf_desc desc_list[CMD_BUF_DESC_MAX] = {0};
struct psp_device *psp = psp_master;
--
2.51.1
^ permalink raw reply related
* [PATCH kernel v3 0/4] PCI/TSM: Enabling core infrastructure on AMD SEV TIO
From: Alexey Kardashevskiy @ 2025-12-02 2:44 UTC (permalink / raw)
To: linux-kernel
Cc: linux-crypto, Tom Lendacky, John Allen, Herbert Xu,
David S. Miller, Ashish Kalra, Joerg Roedel,
Suravee Suthikulpanit, Will Deacon, Robin Murphy, Borislav Petkov,
Borislav Petkov (AMD), Dan Williams, Jason Gunthorpe,
Jerry Snitselaar, Vasant Hegde, Gao Shiyuan, Sean Christopherson,
Kim Phillips, Nikunj A Dadhania, Michael Roth, Paolo Bonzini,
iommu, Alexey Kardashevskiy, x86, linux-coco
Here are some patches to begin enabling SEV-TIO on AMD.
SEV-TIO allows guests to establish trust in a device that supports TEE
Device Interface Security Protocol (TDISP, defined in PCIe r6.0+) and
then interact with the device via private memory.
In order to streamline upstreaming process, a common TSM infrastructure
is being developed in collaboration with Intel+ARM+RiscV. There is
Documentation/driver-api/pci/tsm.rst with proposed phases:
1. IDE: encrypt PCI, host only
2. TDISP: lock + accept flow, host and guest, interface report
3. Enable secure MMIO + DMA: IOMMUFD, KVM changes
4. Device attestation: certificates, measurements
This is phase1 == IDE only.
SEV TIO spec:
https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58271.pdf
Acronyms:
TEE - Trusted Execution Environments, a concept of managing trust
between the host and devices
TSM - TEE Security Manager (TSM), an entity which ensures security on
the host
PSP - AMD platform secure processor (also "ASP", "AMD-SP"), acts as TSM
on AMD.
SEV TIO - the TIO protocol implemented by the PSP and used by the host
GHCB - guest/host communication block - a protocol for guest-to-host
communication via a shared page
TDISP - TEE Device Interface Security Protocol (PCIe).
Flow:
- Boot host OS, load CCP which registers itself as a TSM
- PCI TSM creates sysfs nodes under "tsm" subdirectory in for all
TDISP-capable devices
- Enable IDE via "echo tsm0 >
/sys/bus/pci/devices/0000:e1:00.0/tsm/connect"
- observe "secure" in stream states in "lspci" for the rootport and endpoint
This is pushed out to
https://github.com/AMDESE/linux-kvm/commits/tsm-staging
The full "WIP" trees and configs are here:
https://github.com/AMDESE/AMDSEV/blob/tsm/stable-commits
The previous conversation is here:
https://lore.kernel.org/r/20251121080629.444992-1-aik@amd.com
https://lore.kernel.org/r/20251111063819.4098701-1-aik@amd.com
https://lore.kernel.org/r/20250218111017.491719-1-aik@amd.com
This is based on sha1
f7ae6d4ec652 Dan Williams "PCI/TSM: Add 'dsm' and 'bound' attributes for dependent functions".
Please comment. Thanks.
Alexey Kardashevskiy (4):
ccp: Make snp_reclaim_pages and __sev_do_cmd_locked public
psp-sev: Assign numbers to all status codes and add new
iommu/amd: Report SEV-TIO support
crypto/ccp: Implement SEV-TIO PCIe IDE (phase1)
drivers/crypto/ccp/Kconfig | 1 +
drivers/crypto/ccp/Makefile | 4 +
drivers/crypto/ccp/sev-dev-tio.h | 123 +++
drivers/crypto/ccp/sev-dev.h | 11 +
drivers/iommu/amd/amd_iommu_types.h | 1 +
include/linux/amd-iommu.h | 2 +
include/linux/psp-sev.h | 17 +-
include/uapi/linux/psp-sev.h | 66 +-
drivers/crypto/ccp/sev-dev-tio.c | 864 ++++++++++++++++++++
drivers/crypto/ccp/sev-dev-tsm.c | 405 +++++++++
drivers/crypto/ccp/sev-dev.c | 62 +-
drivers/iommu/amd/init.c | 9 +
12 files changed, 1529 insertions(+), 36 deletions(-)
create mode 100644 drivers/crypto/ccp/sev-dev-tio.h
create mode 100644 drivers/crypto/ccp/sev-dev-tio.c
create mode 100644 drivers/crypto/ccp/sev-dev-tsm.c
--
2.51.1
^ permalink raw reply
* Re: [PATCH 1/3] KVM: guest_memfd: Remove preparation tracking
From: Michael Roth @ 2025-12-01 23:44 UTC (permalink / raw)
To: Yan Zhao
Cc: kvm, linux-coco, linux-mm, linux-kernel, thomas.lendacky,
pbonzini, seanjc, vbabka, ashish.kalra, liam.merwick, david,
vannapurve, ackerleytng, aik, ira.weiny
In-Reply-To: <aSUe1UfD3hXg2iMZ@yzhao56-desk.sh.intel.com>
On Tue, Nov 25, 2025 at 11:13:25AM +0800, Yan Zhao wrote:
> On Fri, Nov 21, 2025 at 06:43:14AM -0600, Michael Roth wrote:
> > On Thu, Nov 20, 2025 at 05:12:55PM +0800, Yan Zhao wrote:
> > > On Thu, Nov 13, 2025 at 05:07:57PM -0600, Michael Roth wrote:
> > > > @@ -797,19 +782,25 @@ 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)) {
> > > > + unsigned long i, nr_pages = folio_nr_pages(folio);
> > > > +
> > > > + for (i = 0; i < nr_pages; i++)
> > > > + clear_highpage(folio_page(folio, i));
> > > > + folio_mark_uptodate(folio);
> > > Here, the entire folio is cleared only when the folio is not marked uptodate.
> > > Then, please check my questions at the bottom
> >
> > Yes, in this patch at least where I tried to mirror the current logic. I
> > would not be surprised if we need to rework things for inplace/hugepage
> > support though, but decoupling 'preparation' from the uptodate flag is
> > the main goal here.
> Could you elaborate a little why the decoupling is needed if it's not for
> hugepage?
For instance, for in-place conversion:
1. initial allocation: clear, set uptodate, fault in as private
2. private->shared: call invalidate hook, fault in as shared
3. shared->private: call prep hook, fault in as private
Here, 2/3 need to track where the current state is shared/private in
order to make appropriate architecture-specific changes (e.g. RMP table
updates). But we want to allow for non-destructive in-place conversion,
where a page is 'uptodate', but not in the desired shared/private state.
So 'uptodate' becomes a separate piece of state, which is still
reasonable for gmem to track in the current 4K-only implementation, and
provides for a reasonable approach to upstreaming in-place conversion,
which isn't far off for either SNP or TDX.
For hugepages, we'll have other things to consider, but those things are
probably still somewhat far off, and so we shouldn't block steps toward
in-place conversion based on uncertainty around hugepages. I think it's
gotten enough attention at least that we know it *can* work, e.g. even
if we take the inefficient/easy route of zero'ing the whole folio on
initial access, setting it uptodate, and never doing anything with
uptodate again, it's still a usable implementation.
>
>
> > > > + }
> > > > +
> > > > + r = kvm_gmem_prepare_folio(kvm, slot, gfn, folio);
> > > >
> > > > folio_unlock(folio);
> > > >
> > > > @@ -852,7 +843,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)) {
> > > > @@ -860,19 +850,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, &max_order);
> > > > + folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &max_order);
> > > > if (IS_ERR(folio)) {
> > > > ret = PTR_ERR(folio);
> > > > break;
> > > > }
> > > >
> > > > - if (is_prepared) {
> > > > - folio_unlock(folio);
> > > > - folio_put(folio);
> > > > - ret = -EEXIST;
> > > > - break;
> > > > - }
> > > > -
> > > > folio_unlock(folio);
> > > > WARN_ON(!IS_ALIGNED(gfn, 1 << max_order) ||
> > > > (npages - i) < (1 << max_order));
> > > TDX could hit this warning easily when npages == 1, max_order == 9.
> >
> > Yes, this will need to change to handle that. I don't think I had to
> > change this for previous iterations of SNP hugepage support, but
> > there are definitely cases where a sub-2M range might get populated
> > even though it's backed by a 2M folio, so I'm not sure why I didn't
> > hit it there.
> >
> > But I'm taking Sean's cue on touching as little of the existing
> > hugepage logic as possible in this particular series so we can revisit
> > the remaining changes with some better context.
> Frankly, I don't understand why this patch 1 is required if we only want "moving
> GUP out of post_populate()" to work for 4KB folios.
Above I outlined one of the use-cases for in-place conversion.
During the 2 PUCK sessions prior to this RFC, Sean also mentioned some
potential that other deadlocks might exist in current code due to
how the locking is currently handled, and that we should consider this
as a general cleanup against current kvm/next, but I leave that to Sean
to elaborate on.
Personally I think this series makes sense against kvm/next regardless:
tracking preparation in gmem is basically already broken: everyone ignores
it except SNP, so it was never performing that duty as-designed. So we
are now simplying uptodate flag to no longer include this extra
state-tracking, and leaving it for architecture-specific tracking. I
can't see that be anything but beneficial to future gmem changes.
>
> > >
> > > > @@ -889,7 +872,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, max_order, opaque);
> > > > if (!ret)
> > > > - kvm_gmem_mark_prepared(folio);
> > > > + folio_mark_uptodate(folio);
> > > As also asked in [1], why is the entire folio marked as uptodate here? Why does
> > > kvm_gmem_get_pfn() clear all pages of a huge folio when the folio isn't marked
> > > uptodate?
> >
> > Quoting your example from[1] for more context:
> >
> > > I also have a question about this patch:
> > >
> > > Suppose there's a 2MB huge folio A, where
> > > A1 and A2 are 4KB pages belonging to folio A.
> > >
> > > (1) kvm_gmem_populate() invokes __kvm_gmem_get_pfn() and gets folio A.
> > > It adds page A1 and invokes folio_mark_uptodate() on folio A.
> >
> > In SNP hugepage patchset you responded to, it would only mark A1 as
> You mean code in
> https://github.com/amdese/linux/commits/snp-inplace-conversion-rfc1 ?
No, sorry, I got my references mixed up. The only publically-posted
version of SNP hugepage support is the THP series that does not involve
in-place conversion, and that's what I was referencing. It's there where
per-4K bitmap was added to track preparation, and in that series
page-clearing/preparation are still coupled to some degree so per-4K
tracking of page-clearing was still possible and that's how it was
handled:
https://github.com/AMDESE/linux/blob/snp-prepare-thp-rfc1/virt/kvm/guest_memfd.c#L992
but that can be considered an abandoned approach so I wouldn't spend
much time referencing that.
>
> > prepared/cleared. There was 4K-granularity tracking added to handle this.
> I don't find the code that marks only A1 as "prepared/cleared".
> Instead, I just found folio_mark_uptodate() is invoked by kvm_gmem_populate()
> to mark the entire folio A as uptodate.
>
> However, according to your statement below that "uptodate flag only tracks
> whether a folio has been cleared", I don't follow why and where the entire folio
> A would be cleared if kvm_gmem_populate() only adds page A1.
>
> > There was an odd subtlety in that series though: it was defaulting to the
> > folio_order() for the prep-tracking/post-populate, but it would then clamp
> > it down based on the max order possible according whether that particular
> > order was a homogenous range of KVM_MEMORY_ATTRIBUTE_PRIVATE. Which is not
> > a great way to handle things, and I don't remember if I'd actually intended
> > to implement it that way or not... that's probably why I never tripped over
> > the WARN_ON() above, now that I think of it.
> >
> > But neither of these these apply to any current plans for hugepage support
> > that I'm aware of, so probably not worth working through what that series
> > did and look at this from a fresh perspective.
> >
> > >
> > > (2) kvm_gmem_get_pfn() later faults in page A2.
> > > As folio A is uptodate, clear_highpage() is not invoked on page A2.
> > > kvm_gmem_prepare_folio() is invoked on the whole folio A.
> > >
> > > (2) could occur at least in TDX when only a part the 2MB page is added as guest
> > > initial memory.
> > >
> > > My questions:
> > > - Would (2) occur on SEV?
> > > - If it does, is the lack of clear_highpage() on A2 a problem ?
> > > - Is invoking gmem_prepare on page A1 a problem?
> >
> > Assuming this patch goes upstream in some form, we will now have the
> > following major differences versus previous code:
> >
> > 1) uptodate flag only tracks whether a folio has been cleared
> > 2) gmem always calls kvm_arch_gmem_prepare() via kvm_gmem_get_pfn() and
> > the architecture can handle it's own tracking at whatever granularity
> > it likes.
> 2) looks good to me.
>
> > My hope is that 1) can similarly be done in such a way that gmem does not
> > need to track things at sub-hugepage granularity and necessitate the need
> > for some new data structure/state/flag to track sub-page status.
> I actually don't understand what uptodate flag helps gmem to track.
> Why can't clear_highpage() be done inside arch specific code? TDX doesn't need
> this clearing after all.
It could. E.g. via the kernel-internal gmem flag that I mentioned in my
earlier reply, or some alternative.
In the context of this series, uptodate flag continues to instruct
kvm_gmem_get_pfn() that it doesn't not need to re-clear pages, because
a prior kvm_gmem_get_pfn() or kvm_gmem_populate() already initialized
the folio, and it is no longer tied to any notion of
preparedness-tracking.
What use uptodate will have in the context of hugepages: I'm not sure.
For non-in-place conversion, it's tempting to just let it continue to be
per-folio and require clearing the whole folio on initial access, but
it's not efficient. It may make sense to farm it out to
post-populate/prep hooks instead, as you're suggesting for TDX.
But then, for in-place conversion, you have to deal with pages initially
faulted in as shared. They might be split prior to initial access as a
private page, where we can't assume TDX will have scrubbed things. So in
that case it might still make sense to rely on it.
Definitely things that require some more thought. But having it inextricably
tied to preparedness just makes preparation tracking similarly more
complicated as it pulls it back into gmem when that does not seem to be
the direction any architectures other SNP have/want to go.
>
> > My understanding based on prior discussion in guest_memfd calls was that
> > it would be okay to go ahead and clear the entire folio at initial allocation
> > time, and basically never mess with it again. It was also my understanding
> That's where I don't follow in this patch.
> I don't see where the entire folio A is cleared if it's only partially mapped by
> kvm_gmem_populate(). kvm_gmem_get_pfn() won't clear folio A either due to
> kvm_gmem_populate() has set the uptodate flag.
>
> > that for TDX it might even be optimal to completely skip clearing the folio
> > if it is getting mapped into SecureEPT as a hugepage since the TDX module
> > would handle that, but that maybe conversely after private->shared there
> > would be some need to reclear... I'll try to find that discussion and
> > refresh. Vishal I believe suggested some flags to provide more control over
> > this behavior.
> >
> > >
> > > It's possible (at least for TDX) that a huge folio is only partially populated
> > > by kvm_gmem_populate(). Then kvm_gmem_get_pfn() faults in another part of the
> > > huge folio. For example, in TDX, GFN 0x81f belongs to the init memory region,
> > > while GFN 0x820 is faulted after TD is running. However, these two GFNs can
> > > belong to the same folio of order 9.
> >
> > Would the above scheme of clearing the entire folio up front and not re-clearing
> > at fault time work for this case?
> This case doesn't affect TDX, because TDX clearing private pages internally in
> SEAM APIs. So, as long as kvm_gmem_get_pfn() does not invoke clear_highpage()
> after making a folio private, it works fine for TDX.
>
> I was just trying to understand why SNP needs the clearing of entire folio in
> kvm_gmem_get_pfn() while I don't see how the entire folio is cleared when it's
> partially mapped in kvm_gmem_populate().
> Also, I'm wondering if it would be better if SNP could move the clearing of
> folio into something like kvm_arch_gmem_clear(), just as kvm_arch_gmem_prepare()
> which is always invoked by kvm_gmem_get_pfn() and the architecture can handle
> it's own tracking at whatever granularity.
Possibly, but I touched elsewhere on where in-place conversion might
trip up this approach. At least decoupling them allows for the prep side
of things to be moved to architecture-specific tracking. We can deal
with uptodate separately I think.
-Mike
>
>
> > > Note: the current code should not impact TDX. I'm just asking out of curiosity:)
> > >
> > > [1] https://lore.kernel.org/all/aQ3uj4BZL6uFQzrD@yzhao56-desk.sh.intel.com/
> > >
> > >
^ permalink raw reply
* Re: [PATCH v4 07/16] x86/virt/tdx: Add tdx_alloc/free_page() helpers
From: Edgecombe, Rick P @ 2025-12-01 22:39 UTC (permalink / raw)
To: 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,
linux-kernel@vger.kernel.org, tglx@linutronix.de, Yamahata, Isaku,
nik.borisov@suse.com, pbonzini@redhat.com, Annapurve, Vishal,
Gao, Chao, bp@alien8.de, x86@kernel.org
In-Reply-To: <dde56556-7611-4adf-9015-5bdf1a016786@suse.com>
On Thu, 2025-11-27 at 18:11 +0200, Nikolay Borisov wrote:
> > +/* Number PAMT pages to be provided to TDX module per 2M region of PA */
> > +static int tdx_dpamt_entry_pages(void)
> > +{
> > + if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> > + return 0;
> > +
> > + return tdx_sysinfo.tdmr.pamt_4k_entry_size * PTRS_PER_PTE /
> > PAGE_SIZE;
> > +}
>
> Isn't this guaranteed to return 2 always as per the ABI? Can't the
> allocation of the 2 pages be moved closer to where it's used - in
> tdh_phymem_pamt_add which will simplify things a bit?
Yea, it could be simpler if it was always guaranteed to be 2 pages. But it was
my understanding that it would not be a fixed size. Can you point to what docs
makes you think that?
Another option would be to ask TDX folks to make it fixed, and then require an
opt-in for it to be expanded later if needed. I would have to check on them on
the reasoning for it being dynamic sized. I'm not sure if it is *that*
complicated at this point though. Once there is more than one, the loops becomes
tempting. And if we loop over 2 we could easily loop over n.
^ permalink raw reply
* Re: [PATCH v4 07/16] x86/virt/tdx: Add tdx_alloc/free_page() helpers
From: Edgecombe, Rick P @ 2025-12-01 22:31 UTC (permalink / raw)
To: 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,
linux-kernel@vger.kernel.org, tglx@linutronix.de, Yamahata, Isaku,
nik.borisov@suse.com, pbonzini@redhat.com, Annapurve, Vishal,
Gao, Chao, bp@alien8.de, x86@kernel.org
Cc: kirill.shutemov@linux.intel.com
In-Reply-To: <21115e18-c68d-492d-9fd4-400452bd64c7@suse.com>
On Thu, 2025-11-27 at 14:29 +0200, Nikolay Borisov wrote:
> > While the TDX initialization code in arch/x86 uses pages with 2MB
> > alignment, KVM will need to hand 4KB pages for it to use. Under DPAMT,
> > these pages will need DPAMT backing 4KB backing.
>
> That paragraph is rather hard to parse. KVM will need to hand 4k pages
> to whom? The tdx init code? Also the last sentence with the 2 "backing"
> words is hard to parse. Does it say that the 4k pages that KVM need to
> pass must be backed by DPAMT pages i.e like a chicken and egg problem?
You're right it's confusing and also a slightly misleading typo, maybe this is
an improvement?
While the TDX module initialization code in arch/x86 only hands pages to the
TDX module with 2MB alignment, KVM will need to hand pages to the TDX module
at 4KB granularity. Under DPAMT, such pages will require performing the
TDH.PHYMEM.PAMT.ADD SEAMCALL to give a page pair the the TDX module to use
for PAMT tracking at the 4KB page size page.
Note: There is no chicken or egg problem, TDH.PHYMEM.PAMT.ADD handles it.
>
> >
> > Add tdx_alloc_page() and tdx_free_page() to handle both page allocation
> > and DPAMT installation. Make them behave like normal alloc/free functions
> > where allocation can fail in the case of no memory, but free (with any
> > necessary DPAMT release) always succeeds. Do this so they can support the
> > existing TDX flows that require cleanups to succeed. Also create
> > tdx_pamt_put()/tdx_pamt_get() to handle installing DPAMT 4KB backing for
> > pages that are already allocated (such as external page tables, or S-EPT
> > pages).
> >
>
> <snip>
>
>
> > +
> > +/* Serializes adding/removing PAMT memory */
> > +static DEFINE_SPINLOCK(pamt_lock);
> > +
> > +/* Bump PAMT refcount for the given page and allocate PAMT memory if needed
> > */
> > +int tdx_pamt_get(struct page *page)
> > +{
> > + u64 pamt_pa_array[MAX_TDX_ARG_SIZE(rdx)];
> > + atomic_t *pamt_refcount;
> > + u64 tdx_status;
> > + int ret;
> > +
> > + if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> > + return 0;
> > +
> > + ret = alloc_pamt_array(pamt_pa_array);
> > + if (ret)
> > + goto out_free;
> > +
> > + pamt_refcount = tdx_find_pamt_refcount(page_to_pfn(page));
> > +
> > + scoped_guard(spinlock, &pamt_lock) {
> > + /*
> > + * If the pamt page is already added (i.e. refcount >= 1),
> > + * then just increment the refcount.
> > + */
> > + if (atomic_read(pamt_refcount)) {
> > + atomic_inc(pamt_refcount);
> > + goto out_free;
> > + }
>
> Replace this pair of read/inc with a single call to atomic_inc_not_zero()
From the thread with Binbin on this patch, the atomics aren't really needed
until the optimization patch. So was thinking to actually use a simpler non-
atomic operations.
>
> > +
> > + /* Try to add the pamt page and take the refcount 0->1. */
> > +
> > + tdx_status = tdh_phymem_pamt_add(page, pamt_pa_array);
> > + if (!IS_TDX_SUCCESS(tdx_status)) {
> > + pr_err("TDH_PHYMEM_PAMT_ADD failed: %#llx\n",
> > tdx_status);
> > + goto out_free;
> > + }
> > +
> > + atomic_inc(pamt_refcount);
> > + }
> > +
> > + return ret;
> > +out_free:
> > + /*
> > + * pamt_pa_array is populated or zeroed up to
> > tdx_dpamt_entry_pages()
> > + * above. free_pamt_array() can handle either case.
> > + */
> > + free_pamt_array(pamt_pa_array);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(tdx_pamt_get);
> > +
> > +/*
> > + * Drop PAMT refcount for the given page and free PAMT memory if it is no
> > + * longer needed.
> > + */
> > +void tdx_pamt_put(struct page *page)
> > +{
> > + u64 pamt_pa_array[MAX_TDX_ARG_SIZE(rdx)];
> > + atomic_t *pamt_refcount;
> > + u64 tdx_status;
> > +
> > + if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> > + return;
> > +
> > + pamt_refcount = tdx_find_pamt_refcount(page_to_pfn(page));
> > +
> > + scoped_guard(spinlock, &pamt_lock) {
> > + /*
> > + * If the there are more than 1 references on the pamt
> > page,
> > + * don't remove it yet. Just decrement the refcount.
> > + */
> > + if (atomic_read(pamt_refcount) > 1) {
> > + atomic_dec(pamt_refcount);
> > + return;
> > + }
>
> nit: Could be replaced with : atomic_add_unless(pamt_refcount, -1, 1);
>
> Probably it would have been better to simply use atomic64_dec_and_test
> and if it returns true do the phymem_pamt_remove, but I suspect you
> can't do it because in case it fails you don't want to decrement the
> last refcount, though that could be remedied by an extra atomic_int in
> the failure path. I guess it might be worth simplifying since the extra
> inc will only be needed in exceptional cases (we don't expect failure ot
> be the usual path) and freeing is not a fast path.
The goal of this patch is to be as simple and obviously correct as possible.
Then the next patch "x86/virt/tdx: Optimize tdx_alloc/free_page() helpers"
should have the optimized versions. Do you have any similar suggestions on the
code after the next patch is applied?
^ permalink raw reply
* Re: [PATCH v4 06/16] x86/virt/tdx: Improve PAMT refcounts allocation for sparse memory
From: Edgecombe, Rick P @ 2025-12-01 22:14 UTC (permalink / raw)
To: kas@kernel.org
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Li, Xiaoyao, Hansen, Dave, Zhao, Yan Y, Wu, Binbin,
linux-kernel@vger.kernel.org, seanjc@google.com,
binbin.wu@linux.intel.com, pbonzini@redhat.com, mingo@redhat.com,
Yamahata, Isaku, tglx@linutronix.de, Annapurve, Vishal, Gao, Chao,
bp@alien8.de, x86@kernel.org
In-Reply-To: <irqkfods52iut7se552qo6b5o4qidtmghcdosdxmbytvpyphpi@ol5wuaoydaab>
On Thu, 2025-11-27 at 15:57 +0000, Kiryl Shutsemau wrote:
> > Yes, I don't see what that comment is referring to. But we do need it,
> > because hypothetically the refcount mapping could have failed halfway. So we
> > will have pte_none()s for the ranges that didn't get populated. I'll use:
> >
> > /* Refcount mapping could have failed part way, handle aborted mappings. */
>
> It is possible that we can have holes in physical address space between
> 0 and max_pfn. You need the check even outside of "failed halfway"
> scenario.
Err. right. Was thinking of for_each_mem_pfn_range() on the populate side.
pamt_refcount_depopulate() is just called with the whole refcount virtual
address range. I'll add both reasons to the comment.
^ permalink raw reply
* Re: [PATCH 3/3] KVM: guest_memfd: GUP source pages prior to populating guest memory
From: Michael Roth @ 2025-12-01 22:13 UTC (permalink / raw)
To: Yan Zhao
Cc: kvm, linux-coco, linux-mm, linux-kernel, thomas.lendacky,
pbonzini, seanjc, vbabka, ashish.kalra, liam.merwick, david,
vannapurve, ackerleytng, aik, ira.weiny
In-Reply-To: <aSQmAuxGK7+MUfRW@yzhao56-desk.sh.intel.com>
On Mon, Nov 24, 2025 at 05:31:46PM +0800, Yan Zhao wrote:
> On Fri, Nov 21, 2025 at 07:01:44AM -0600, Michael Roth wrote:
> > On Thu, Nov 20, 2025 at 05:11:48PM +0800, Yan Zhao wrote:
> > > On Thu, Nov 13, 2025 at 05:07:59PM -0600, Michael Roth wrote:
> > > > 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 filemap invalidate
> > > > lock while holding the mm->mmap_lock, leading to a potential ABBA
> > > > deadlock[1].
> > > >
> > > > 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.
> > > >
> > > > Suggested-by: Sean Christopherson <seanjc@google.com>
> > > > Signed-off-by: Michael Roth <michael.roth@amd.com>
> > > > ---
> > > > arch/x86/kvm/svm/sev.c | 40 ++++++++++++++++++++++++++------------
> > > > arch/x86/kvm/vmx/tdx.c | 21 +++++---------------
> > > > include/linux/kvm_host.h | 3 ++-
> > > > virt/kvm/guest_memfd.c | 42 ++++++++++++++++++++++++++++++++++------
> > > > 4 files changed, 71 insertions(+), 35 deletions(-)
> > > >
> > > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > > > index 0835c664fbfd..d0ac710697a2 100644
> > > > --- a/arch/x86/kvm/svm/sev.c
> > > > +++ b/arch/x86/kvm/svm/sev.c
> > > > @@ -2260,7 +2260,8 @@ struct sev_gmem_populate_args {
> > > > };
> > > >
> > > > static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pfn,
> > > > - void __user *src, int order, void *opaque)
> > > > + struct page **src_pages, loff_t src_offset,
> > > > + int order, void *opaque)
> > > > {
> > > > struct sev_gmem_populate_args *sev_populate_args = opaque;
> > > > struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
> > > > @@ -2268,7 +2269,7 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pf
> > > > int npages = (1 << order);
> > > > gfn_t gfn;
> > > >
> > > > - 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_pages))
> > > > return -EINVAL;
> > > >
> > > > for (gfn = gfn_start, i = 0; gfn < gfn_start + npages; gfn++, i++) {
> > > > @@ -2284,14 +2285,21 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pf
> > > > goto err;
> > > > }
> > > >
> > > > - if (src) {
> > > > - void *vaddr = kmap_local_pfn(pfn + i);
> > > > + if (src_pages) {
> > > > + void *src_vaddr = kmap_local_pfn(page_to_pfn(src_pages[i]));
> > > > + void *dst_vaddr = kmap_local_pfn(pfn + i);
> > > >
> > > > - if (copy_from_user(vaddr, src + i * PAGE_SIZE, PAGE_SIZE)) {
> > > > - ret = -EFAULT;
> > > > - goto err;
> > > > + memcpy(dst_vaddr, src_vaddr + src_offset, PAGE_SIZE - src_offset);
> > > > + kunmap_local(src_vaddr);
> > > > +
> > > > + if (src_offset) {
> > > > + src_vaddr = kmap_local_pfn(page_to_pfn(src_pages[i + 1]));
> > > > +
> > > > + memcpy(dst_vaddr + PAGE_SIZE - src_offset, src_vaddr, src_offset);
> > > > + kunmap_local(src_vaddr);
> > > IIUC, src_offset is the src's offset from the first page. e.g.,
> > > src could be 0x7fea82684100, with src_offset=0x100, while npages could be 512.
> > >
> > > Then it looks like the two memcpy() calls here only work when npages == 1 ?
> >
> > src_offset ends up being the offset into the pair of src pages that we
> > are using to fully populate a single dest page with each iteration. So
> > if we start at src_offset, read a page worth of data, then we are now at
> > src_offset in the next src page and the loop continues that way even if
> > npages > 1.
> >
> > If src_offset is 0 we never have to bother with straddling 2 src pages so
> > the 2nd memcpy is skipped on every iteration.
> >
> > That's the intent at least. Is there a flaw in the code/reasoning that I
> > missed?
> Oh, I got you. SNP expects a single src_offset applies for each src page.
>
> So if npages = 2, there're 4 memcpy() calls.
>
> src: |---------|---------|---------| (VA contiguous)
> ^ ^ ^
> | | |
> dst: |---------|---------| (PA contiguous)
>
>
> I previously incorrectly thought kvm_gmem_populate() should pass in src_offset
> as 0 for the 2nd src page.
>
> Would you consider checking if params.uaddr is PAGE_ALIGNED() in
> snp_launch_update() to simplify the design?
This was an option mentioned in the cover letter and during PUCK. I am
not opposed if that's the direction we decide, but I also don't think
it makes big difference since:
int (*kvm_gmem_populate_cb)(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
struct page **src_pages, loff_t src_offset,
int order, void *opaque);
basically reduces to Sean's originally proposed:
int (*kvm_gmem_populate_cb)(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
struct page *src_pages, int order,
void *opaque);
for any platform that enforces that the src is page-aligned, which
doesn't seem like a huge technical burden, IMO, despite me initially
thinking it would be gross when I brought this up during the PUCK call
that preceeding this posting.
>
> > >
> > > > }
> > > > - kunmap_local(vaddr);
> > > > +
> > > > + kunmap_local(dst_vaddr);
> > > > }
> > > >
> > > > ret = rmp_make_private(pfn + i, gfn << PAGE_SHIFT, PG_LEVEL_4K,
> > > > @@ -2331,12 +2339,20 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pf
> > > > if (!snp_page_reclaim(kvm, pfn + i) &&
> > > > 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 *src_vaddr = kmap_local_pfn(page_to_pfn(src_pages[i]));
> > > > + void *dst_vaddr = kmap_local_pfn(pfn + i);
> > > >
> > > > - if (copy_to_user(src + i * PAGE_SIZE, vaddr, PAGE_SIZE))
> > > > - pr_debug("Failed to write CPUID page back to userspace\n");
> > > > + memcpy(src_vaddr + src_offset, dst_vaddr, PAGE_SIZE - src_offset);
> > > > + kunmap_local(src_vaddr);
> > > > +
> > > > + if (src_offset) {
> > > > + src_vaddr = kmap_local_pfn(page_to_pfn(src_pages[i + 1]));
> > > > +
> > > > + memcpy(src_vaddr, dst_vaddr + PAGE_SIZE - src_offset, src_offset);
> > > > + kunmap_local(src_vaddr);
> > > > + }
> > > >
> > > > - kunmap_local(vaddr);
> > > > + kunmap_local(dst_vaddr);
> > > > }
> > > >
> > > > /* pfn + i is hypervisor-owned now, so skip below cleanup for it. */
> > > > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > > > index 57ed101a1181..dd5439ec1473 100644
> > > > --- a/arch/x86/kvm/vmx/tdx.c
> > > > +++ b/arch/x86/kvm/vmx/tdx.c
> > > > @@ -3115,37 +3115,26 @@ 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)
> > > > + struct page **src_pages, loff_t src_offset,
> > > > + int order, 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;
> > > >
> > > > - if (KVM_BUG_ON(!PAGE_ALIGNED(src), kvm))
> > > > + /* Source should be page-aligned, in which case src_offset will be 0. */
> > > > + if (KVM_BUG_ON(src_offset))
> > > if (KVM_BUG_ON(src_offset, kvm))
> > >
> > > > return -EINVAL;
> > > >
> > > > - /*
> > > > - * 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;
> > > > -
> > > > - kvm_tdx->page_add_src = src_page;
> > > > + kvm_tdx->page_add_src = src_pages[i];
> > > src_pages[0] ? i is not initialized.
> >
> > Sorry, I switched on TDX options for compile testing but I must have done a
> > sloppy job confirming it actually built. I'll re-test push these and squash
> > in the fixes in the github tree.
> >
> > >
> > > Should there also be a KVM_BUG_ON(order > 0, kvm) ?
> >
> > Seems reasonable, but I'm not sure this is the right patch. Maybe I
> > could squash it into the preceeding documentation patch so as to not
> > give the impression this patch changes those expectations in any way.
> I don't think it should be documented as a user requirement.
I didn't necessarily mean in the documentation, but mainly some patch
other than this. If we add that check here as part of this patch, we
give the impression that the order expectations are changing as a result
of the changes here, when in reality they are exactly the same as
before.
If not the documentation patch here, then I don't think it really fits
in this series at all and would be more of a standalone patch against
kvm/next.
The change here:
- if (KVM_BUG_ON(!PAGE_ALIGNED(src), kvm))
+ /* Source should be page-aligned, in which case src_offset will be 0. */
+ if (KVM_BUG_ON(src_offset))
made sense as part of this patch, because now that we are passing struct
page *src_pages, we can no longer infer alignment from 'src' field, and
instead need to infer it from src_offset being 0.
>
> However, we need to comment out that this assertion is due to that
> tdx_vcpu_init_mem_region() passes npages as 1 to kvm_gmem_populate().
You mean for the KVM_BUG_ON(order > 0, kvm) you're proposing to add?
Again, if feels awkward to address this as part of this series since it
is an existing/unchanged behavior and not really the intent of this
patchset.
>
> > >
> > > > 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 d93f75b05ae2..7e9d2403c61f 100644
> > > > --- a/include/linux/kvm_host.h
> > > > +++ b/include/linux/kvm_host.h
> > > > @@ -2581,7 +2581,8 @@ 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);
> > > > + struct page **src_pages, loff_t src_offset,
> > > > + int order, 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 9160379df378..e9ac3fd4fd8f 100644
> > > > --- a/virt/kvm/guest_memfd.c
> > > > +++ b/virt/kvm/guest_memfd.c
> > > > @@ -814,14 +814,17 @@ 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
> > > > +
> > > > +#define GMEM_GUP_NPAGES (1UL << PMD_ORDER)
> > > Limiting GMEM_GUP_NPAGES to PMD_ORDER may only work when the max_order of a huge
> > > folio is 2MB. What if the max_order returned from __kvm_gmem_get_pfn() is 1GB
> > > when src_pages[] can only hold up to 512 pages?
> >
> > This was necessarily chosen in prep for hugepages, but more about my
> > unease at letting userspace GUP arbitrarilly large ranges. PMD_ORDER
> > happens to align with 2MB hugepages while seeming like a reasonable
> > batching value so that's why I chose it.
> >
> > Even with 1GB support, I wasn't really planning to increase it. SNP
> > doesn't really make use of RMP sizes >2MB, and it sounds like TDX
> > handles promotion in a completely different path. So atm I'm leaning
> > toward just letting GMEM_GUP_NPAGES be the cap for the max page size we
> > support for kvm_gmem_populate() path and not bothering to change it
> > until a solid use-case arises.
> The problem is that with hugetlb-based guest_memfd, the folio itself could be
> of 1GB, though SNP and TDX can force mapping at only 4KB.
If TDX wants to unload handling of page-clearing to its per-page
post-populate callback and tie that its shared/private tracking that's
perfectly fine by me.
*How* TDX tells gmem it wants this different behavior is a topic for a
follow-up patchset, Vishal suggested kernel-internal flags to
kvm_gmem_create(), which seemed reasonable to me. In that case, uptodate
flag would probably just default to set and punt to post-populate/prep
hooks, because we absolutely *do not* want to have to re-introduce per-4K
tracking of this type of state within gmem, since getting rid of that sort
of tracking requirement within gmem is the entire motivation of this
series. And since, within this series, the uptodate flag and
prep-tracking both have the same 4K granularity, it seems unecessary to
address this here.
If you were to send a patchset on top of this (or even independently) that
introduces said kernel-internal gmem flag to offload uptodate-tracking to
post-populate/prep hooks, and utilize it to optimize the current 4K-only
TDX implementation by letting TDX module handle the initial
page-clearing, then I think that change/discussion can progress without
being blocked in any major way by this series.
But I don't think we need to flesh all that out here, so long as we are
aware of this as a future change/requirement and have reasonable
indication that it is compatible with this series.
>
> Then since max_order = folio_order(folio) (at least in the tree for [1]),
> WARN_ON() in kvm_gmem_populate() could still be hit:
>
> folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &max_order);
> WARN_ON(!IS_ALIGNED(gfn, 1 << max_order) ||
> (npages - i) < (1 << max_order));
Yes, in the SNP implementation of hugetlb I ended up removing this
warning, and in that case I also ended up forcing kvm_gmem_populate() to
be 4K-only:
https://github.com/AMDESE/linux/blob/snp-hugetlb-v2-wip0/virt/kvm/guest_memfd.c#L2372
but it makes a lot more sense to make those restrictions and changes in
the context of hugepage support, rather than this series which is trying
very hard to not do hugepage enablement, but simply keep what's partially
there intact while reworking other things that have proven to be
continued impediments to both in-place conversion and hugepage
enablement.
Also, there's talk now of enabling hugepages even without in-place
conversion for hugetlbfs, and that will likely be the same path we
follow for THP to remain in alignment. Rather than anticipating what all
these changes will mean WRT hugepage implementation/requirements, I
think it will be fruitful to remove some of the baggage that will
complicate that process/discussion like this patchset attempts.
-Mike
>
> TDX is even easier to hit this warning because it always passes npages as 1.
>
> [1] https://lore.kernel.org/all/cover.1747264138.git.ackerleytng@google.com
>
>
> > > Increasing GMEM_GUP_NPAGES to (1UL << PUD_ORDER) is probabaly not a good idea.
> > >
> > > Given both TDX/SNP map at 4KB granularity, why not just invoke post_populate()
> > > per 4KB while removing the max_order from post_populate() parameters, as done
> > > in Sean's sketch patch [1]?
> >
> > That's an option too, but SNP can make use of 2MB pages in the
> > post-populate callback so I don't want to shut the door on that option
> > just yet if it's not too much of a pain to work in. Given the guest BIOS
> > lives primarily in 1 or 2 of these 2MB regions the benefits might be
> > worthwhile, and SNP doesn't have a post-post-populate promotion path
> > like TDX (at least, not one that would help much for guest boot times)
> I see.
>
> So, what about below change?
>
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -878,11 +878,10 @@ 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),
> + while (!IS_ALIGNED(gfn, 1 << max_order) || (npages - i) < (1 << max_order) ||
> + !kvm_range_has_memory_attributes(kvm, gfn, gfn + (1 << max_order),
> KVM_MEMORY_ATTRIBUTE_PRIVATE,
> KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
> if (!max_order)
>
>
>
> >
> > >
> > > Then the WARN_ON() in kvm_gmem_populate() can be removed, which would be easily
> > > triggered by TDX when max_order > 0 && npages == 1:
> > >
> > > WARN_ON(!IS_ALIGNED(gfn, 1 << max_order) ||
> > > (npages - i) < (1 << max_order));
> > >
> > >
> > > [1] https://lore.kernel.org/all/aHEwT4X0RcfZzHlt@google.com/
> > >
> > > > 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;
> > > > -
> > > > + struct page **src_pages;
> > > > int ret = 0, max_order;
> > > > - long i;
> > > > + loff_t src_offset = 0;
> > > > + long i, src_npages;
> > > >
> > > > lockdep_assert_held(&kvm->slots_lock);
> > > >
> > > > @@ -836,9 +839,28 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
> > > > if (!file)
> > > > return -EFAULT;
> > > >
> > > > + npages = min_t(ulong, slot->npages - (start_gfn - slot->base_gfn), npages);
> > > > + npages = min_t(ulong, npages, GMEM_GUP_NPAGES);
> > > > +
> > > > + if (src) {
> > > > + src_npages = IS_ALIGNED((unsigned long)src, PAGE_SIZE) ? npages : npages + 1;
> > > > +
> > > > + src_pages = kmalloc_array(src_npages, sizeof(struct page *), GFP_KERNEL);
> > > > + if (!src_pages)
> > > > + return -ENOMEM;
> > > > +
> > > > + ret = get_user_pages_fast((unsigned long)src, src_npages, 0, src_pages);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + if (ret != src_npages)
> > > > + return -ENOMEM;
> > > > +
> > > > + src_offset = (loff_t)(src - PTR_ALIGN_DOWN(src, PAGE_SIZE));
> > > > + }
> > > > +
> > > > 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)) {
> > > > struct folio *folio;
> > > > gfn_t gfn = start_gfn + i;
> > > > @@ -869,8 +891,8 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
> > > > max_order--;
> > > > }
> > > >
> > > > - p = src ? src + i * PAGE_SIZE : NULL;
> > > > - ret = post_populate(kvm, gfn, pfn, p, max_order, opaque);
> > > > + ret = post_populate(kvm, gfn, pfn, src ? &src_pages[i] : NULL,
> > > > + src_offset, max_order, opaque);
> > > Why src_offset is not 0 starting from the 2nd page?
> > >
> > > > if (!ret)
> > > > folio_mark_uptodate(folio);
> > > >
> > > > @@ -882,6 +904,14 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
> > > >
> > > > filemap_invalidate_unlock(file->f_mapping);
> > > >
> > > > + if (src) {
> > > > + long j;
> > > > +
> > > > + for (j = 0; j < src_npages; j++)
> > > > + put_page(src_pages[j]);
> > > > + kfree(src_pages);
> > > > + }
> > > > +
> > > > return ret && !i ? ret : i;
> > > > }
> > > > EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_populate);
> > > > --
> > > > 2.25.1
> > > >
^ permalink raw reply
* Re: [PATCH 3/3] KVM: guest_memfd: GUP source pages prior to populating guest memory
From: Michael Roth @ 2025-12-01 21:03 UTC (permalink / raw)
To: Vishal Annapurve
Cc: Yan Zhao, kvm, linux-coco, linux-mm, linux-kernel,
thomas.lendacky, pbonzini, seanjc, vbabka, ashish.kalra,
liam.merwick, david, ackerleytng, aik, ira.weiny
In-Reply-To: <CAGtprH9_yo4P+oTaGzhHkC3gSdFPTYkvwHkwN66gQhQXX9fhRQ@mail.gmail.com>
On Sun, Nov 30, 2025 at 05:47:37PM -0800, Vishal Annapurve wrote:
> On Mon, Nov 24, 2025 at 1:34 AM Yan Zhao <yan.y.zhao@intel.com> wrote:
> >
> > > > > + if (src_offset) {
> > > > > + src_vaddr = kmap_local_pfn(page_to_pfn(src_pages[i + 1]));
> > > > > +
> > > > > + memcpy(dst_vaddr + PAGE_SIZE - src_offset, src_vaddr, src_offset);
> > > > > + kunmap_local(src_vaddr);
> > > > IIUC, src_offset is the src's offset from the first page. e.g.,
> > > > src could be 0x7fea82684100, with src_offset=0x100, while npages could be 512.
> > > >
> > > > Then it looks like the two memcpy() calls here only work when npages == 1 ?
> > >
> > > src_offset ends up being the offset into the pair of src pages that we
> > > are using to fully populate a single dest page with each iteration. So
> > > if we start at src_offset, read a page worth of data, then we are now at
> > > src_offset in the next src page and the loop continues that way even if
> > > npages > 1.
> > >
> > > If src_offset is 0 we never have to bother with straddling 2 src pages so
> > > the 2nd memcpy is skipped on every iteration.
> > >
> > > That's the intent at least. Is there a flaw in the code/reasoning that I
> > > missed?
> > Oh, I got you. SNP expects a single src_offset applies for each src page.
> >
> > So if npages = 2, there're 4 memcpy() calls.
> >
> > src: |---------|---------|---------| (VA contiguous)
> > ^ ^ ^
> > | | |
> > dst: |---------|---------| (PA contiguous)
> >
> >
> > I previously incorrectly thought kvm_gmem_populate() should pass in src_offset
> > as 0 for the 2nd src page.
> >
> > Would you consider checking if params.uaddr is PAGE_ALIGNED() in
> > snp_launch_update() to simplify the design?
> >
>
> IIUC, this ship has sailed, as asserting this would break existing
> userspace which can pass unaligned userspace buffers.
Actually, on the PUCK call before I sent this patchset Sean/Paolo seemed
to be okay with the prospect of enforcing that params.uaddr is
PAGE_ALIGNED(), since all *known* userspace implementations do use a
page-aligned params.uaddr and this would be highly unlikely to have any
serious fallout.
However, it was suggested that I post the RFC with non-page-aligned
handling intact so we can have some further discussion about it. That
would be one of the 3 approaches listed under (A) in the cover letter.
(Sean proposed another option that he might still advocate for, also
listed in the cover letter under (A), but wanted to see what this looked
like first).
Personally, I'm fine with forcing params.uaddr to. But there is still some
slight risk that some VMM out there flying under the radar will surface
this userspace breakage and that won't be fun to deal with.
IMO, if an implementation wants to enforce page alignment, they
can simply assert(src_offset == 0) in the post-populate callback and
just treat src_pages[0] as if it was the only src input, like what
was done in the tdx_post_populate() callback here. The overall changes
seemed trivial enough that I don't see it being a headache for platforms
that enforce that src pointer is PAGE-ALIGNED. And for platforms like
SNP that don't, it does not seem like a huge headache to straddle 2 src
pages for each PFN we're populating.
Maybe some better comments/documentation around kvm_gmem_populate()
would more effectively alleviate potential confusion from new users
of the proposed interface.
-Mike
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: David Hildenbrand (Red Hat) @ 2025-12-01 20:36 UTC (permalink / raw)
To: Borislav Petkov
Cc: Kiryl Shutsemau, Pratik R. Sampat, linux-mm, linux-coco,
linux-efi, x86, linux-kernel, tglx, mingo, dave.hansen, ardb,
akpm, osalvador, thomas.lendacky, michael.roth
In-Reply-To: <20251201202507.GFaS35o7WtLJOM0_jh@fat_crate.local>
On 12/1/25 21:25, Borislav Petkov wrote:
> On Mon, Dec 01, 2025 at 09:10:26PM +0100, David Hildenbrand (Red Hat) wrote:
>> Just to be clear, I don't think it exist and also I don't think that it
>> should exist.
>
> By that logic if it doesn't exist and someone sends a patch, I should simply
> ignore a review comment about that patch breaking some non-existent ABI and
> simply take it.
Well, we can always discuss and see if there is a way to not break a
specific use case, independent of any ABI stability guarantees.
>
> Well, it certainly works for me.
>
> Unless you folks come-a-runnin' later screaming it broke some use case of
> yours.
Heh, not me, but likely some of the CoCo folks regarding this specific
use case (kexec in a confidential VM).
> And then we're back to what I've been preaching on this thread from the
> very beginning: having a common agreement on what ABI Linux enforces.
Right. Maybe Kiryl knows more about this specific case as he brought up that
these structures are versioned.
--
Cheers
David
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Borislav Petkov @ 2025-12-01 20:25 UTC (permalink / raw)
To: David Hildenbrand (Red Hat)
Cc: Kiryl Shutsemau, Pratik R. Sampat, linux-mm, linux-coco,
linux-efi, x86, linux-kernel, tglx, mingo, dave.hansen, ardb,
akpm, osalvador, thomas.lendacky, michael.roth
In-Reply-To: <dcccdc4b-b7d7-47c4-b1b1-a6c70edb20fa@kernel.org>
On Mon, Dec 01, 2025 at 09:10:26PM +0100, David Hildenbrand (Red Hat) wrote:
> Just to be clear, I don't think it exist and also I don't think that it
> should exist.
By that logic if it doesn't exist and someone sends a patch, I should simply
ignore a review comment about that patch breaking some non-existent ABI and
simply take it.
Well, it certainly works for me.
Unless you folks come-a-runnin' later screaming it broke some use case of
yours. And then we're back to what I've been preaching on this thread from the
very beginning: having a common agreement on what ABI Linux enforces.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: David Hildenbrand (Red Hat) @ 2025-12-01 20:10 UTC (permalink / raw)
To: Borislav Petkov
Cc: Kiryl Shutsemau, Pratik R. Sampat, linux-mm, linux-coco,
linux-efi, x86, linux-kernel, tglx, mingo, dave.hansen, ardb,
akpm, osalvador, thomas.lendacky, michael.roth
In-Reply-To: <20251201191036.GEaS3oLBY8PEuE91Ap@fat_crate.local>
On 12/1/25 20:10, Borislav Petkov wrote:
> On Mon, Dec 01, 2025 at 07:32:38PM +0100, David Hildenbrand (Red Hat) wrote:
>> I think we are in agreement: from what I recall, this software contract used to be
>> rather simple and stable.
>
> Ok, please point me to the *explicit* document in our tree which says: "we
> won't break the kernel and support kexec with any kernel version"?
Just to be clear, I don't think it exist and also I don't think that it
should exist.
>
> Something ala Documentation/process/stable-api-nonsense.rst
>
> Which says things like:
>
> "Assuming that we had a stable kernel source interface for the kernel,
> a binary interface would naturally happen too, right? Wrong."
>
> Which I read as a "no" to the kexec question too.
>
> IOW, it is not about whether it works or not - it is about enforcing that.
Agreed.
--
Cheers
David
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Pratik R. Sampat @ 2025-12-01 19:35 UTC (permalink / raw)
To: David Hildenbrand (Red Hat), Kiryl Shutsemau
Cc: linux-mm, linux-coco, linux-efi, x86, linux-kernel, tglx, mingo,
bp, dave.hansen, ardb, akpm, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <2b82fb9f-338e-47e9-bd14-3bdb392dfcbf@kernel.org>
On 12/1/25 12:25 PM, David Hildenbrand (Red Hat) wrote:
> On 12/1/25 18:15, Pratik R. Sampat wrote:
>> Hi David,
>>
>> On 11/28/25 3:34 AM, David Hildenbrand (Red Hat) wrote:
>>> On 11/27/25 18:40, Kiryl Shutsemau wrote:
>>>> On Wed, Nov 26, 2025 at 04:27:29PM -0600, Pratik R. Sampat wrote:
>>>>>
>>>>>
>>>>> On 11/26/25 5:12 AM, Kiryl Shutsemau wrote:
>>>>>> On Tue, Nov 25, 2025 at 11:57:51AM -0600, Pratik R. Sampat wrote:
>>>>>>> The unaccepted memory structure currently only supports accepting memory
>>>>>>> present at boot time. The unaccepted table uses a fixed-size bitmap
>>>>>>> reserved in memblock based on the initial memory layout, preventing
>>>>>>> dynamic addition of memory ranges after boot. This causes guest
>>>>>>> termination when memory is hot-added in a secure virtual machine due to
>>>>>>> accessing pages that have not transitioned to private before use.
>>>>>>
>>>>>> How does the hot-pluggable memory look in EFI memory map? I thought
>>>>>> hot-pluggable ranges suppose to be declared thare. The cleanest solution
>>>>>> would be to have hot-pluggable and unaccepted indicated in EFI memory,
>>>>>> so we can size bitmap accordingly upfront.
>>>>>>
>>>>>
>>>>> I'm not quite sure if I fully understand. Do you mean to refer to the
>>>>> EFI_MEMORY_HOT_PLUGGABLE attribute that is used for cold plugged boot
>>>>> memory? If so, wouldn't it still be desirable to increase the size of
>>>>> the bitmap to what was marked as hotpluggable initially?
>>>>
>>>> I just don't understand how hotpluggable memory presented in EFI memory
>>>> map in presence of unaccepted memory. If not-yet-plugged memory marked
>>>> as unaccepted we can preallocate bitmap upfront and make unaccepted
>>>> memory transparent wrt hotplug.
>>>>
>>>> BTW, isn't virtio-mem a more attractive target to support than HW-style
>>>> hotplug?
>>>
>>> I would have thought so as well, such that we can just let virtio-mem take care of any acceptance before actually using hotplugged memory (exposing it to the buddy).
>>>
>>> Likely there is desire to support other hypervisors?
>>
>> That's true. We are certainly thinking about how the RAM discard manager
>> should look like with multiple states to allow guest_memfd and
>> virtio-mem to work together.
>>
>
> Right, there is the QEMU side of it as well.
>
>> Since both paths in Linux eventually converge around
>> add_memory_resource(), based on some light hacking in QEMU I could see
>> similar hotplug behavior for virtio-mem as well.
>
> For virtio-mem it would not be add_memory_resource().
>
> Whenever we would be plugging memory we would be accepting it, and when we would be unplugging memory we would unaccept it.
>
> That is, acceptance does not happen at add_memory_resource() time, but when virtio-mem asks the device to transition a device block from unplugged<->plugged.
Ah, I see. Thanks for clearing that up!
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Pratik R. Sampat @ 2025-12-01 19:35 UTC (permalink / raw)
To: David Hildenbrand (Red Hat), linux-mm, linux-coco, linux-efi, x86,
linux-kernel
Cc: tglx, mingo, bp, dave.hansen, kas, ardb, akpm, osalvador,
thomas.lendacky, michael.roth
In-Reply-To: <938a7948-7882-41a3-926b-3d2a8d07620d@kernel.org>
On 12/1/25 12:36 PM, David Hildenbrand (Red Hat) wrote:
> On 12/1/25 18:21, Pratik R. Sampat wrote:
>>
>>
>> On 11/28/25 3:32 AM, David Hildenbrand (Red Hat) wrote:
>>> On 11/25/25 18:57, Pratik R. Sampat wrote:
>>>> The unaccepted memory structure currently only supports accepting memory
>>>> present at boot time. The unaccepted table uses a fixed-size bitmap
>>>> reserved in memblock based on the initial memory layout, preventing
>>>> dynamic addition of memory ranges after boot. This causes guest
>>>> termination when memory is hot-added in a secure virtual machine due to
>>>> accessing pages that have not transitioned to private before use.
>>>>
>>>> Extend the unaccepted memory framework to handle hotplugged memory by
>>>> dynamically managing the unaccepted bitmap. Allocate a new bitmap when
>>>> hotplugged ranges exceed the reserved bitmap capacity and switch to
>>>> kernel-managed allocation.
>>>>
>>>> Hotplugged memory also follows the same acceptance policy using the
>>>> accept_memory=[eager|lazy] kernel parameter to accept memory either
>>>> up-front when added or before first use.
>>>>
>>>> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
>>>> ---
>>>> arch/x86/boot/compressed/efi.h | 1 +
>>>> .../firmware/efi/libstub/unaccepted_memory.c | 1 +
>>>> drivers/firmware/efi/unaccepted_memory.c | 83 +++++++++++++++++++
>>>> include/linux/efi.h | 1 +
>>>> include/linux/mm.h | 11 +++
>>>> mm/memory_hotplug.c | 7 ++
>>>> mm/page_alloc.c | 2 +
>>>> 7 files changed, 106 insertions(+)
>>>>
>>>> diff --git a/arch/x86/boot/compressed/efi.h b/arch/x86/boot/compressed/efi.h
>>>> index 4f7027f33def..a220a1966cae 100644
>>>> --- a/arch/x86/boot/compressed/efi.h
>>>> +++ b/arch/x86/boot/compressed/efi.h
>>>> @@ -102,6 +102,7 @@ struct efi_unaccepted_memory {
>>>> u32 unit_size;
>>>> u64 phys_base;
>>>> u64 size;
>>>> + bool mem_reserved;
>>>> unsigned long *bitmap;
>>>> };
>>>> diff --git a/drivers/firmware/efi/libstub/unaccepted_memory.c b/drivers/firmware/efi/libstub/unaccepted_memory.c
>>>> index c1370fc14555..b16bd61c12bf 100644
>>>> --- a/drivers/firmware/efi/libstub/unaccepted_memory.c
>>>> +++ b/drivers/firmware/efi/libstub/unaccepted_memory.c
>>>> @@ -83,6 +83,7 @@ efi_status_t allocate_unaccepted_bitmap(__u32 nr_desc,
>>>> unaccepted_table->unit_size = EFI_UNACCEPTED_UNIT_SIZE;
>>>> unaccepted_table->phys_base = unaccepted_start;
>>>> unaccepted_table->size = bitmap_size;
>>>> + unaccepted_table->mem_reserved = true;
>>>> memset(unaccepted_table->bitmap, 0, bitmap_size);
>>>> status = efi_bs_call(install_configuration_table,
>>>> diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
>>>> index 4479aad258f8..8537812346e2 100644
>>>> --- a/drivers/firmware/efi/unaccepted_memory.c
>>>> +++ b/drivers/firmware/efi/unaccepted_memory.c
>>>> @@ -218,6 +218,89 @@ bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size)
>>>> return ret;
>>>> }
>>>> +static int extend_unaccepted_bitmap(phys_addr_t mem_range_start,
>>>> + unsigned long mem_range_size)
>>>> +{
>>>> + struct efi_unaccepted_memory *unacc_tbl;
>>>> + unsigned long *old_bitmap, *new_bitmap;
>>>> + phys_addr_t start, end, mem_range_end;
>>>> + u64 phys_base, size, unit_size;
>>>> + unsigned long flags;
>>>> +
>>>> + unacc_tbl = efi_get_unaccepted_table();
>>>> + if (!unacc_tbl || !unacc_tbl->unit_size)
>>>> + return -EIO;
>>>> +
>>>> + unit_size = unacc_tbl->unit_size;
>>>> + phys_base = unacc_tbl->phys_base;
>>>> +
>>>> + mem_range_end = round_up(mem_range_start + mem_range_size, unit_size);
>>>> + size = DIV_ROUND_UP(mem_range_end - phys_base, unit_size * BITS_PER_BYTE);
>>>> +
>>>> + /* Translate to offsets from the beginning of the bitmap */
>>>> + start = mem_range_start - phys_base;
>>>> + end = mem_range_end - phys_base;
>>>> +
>>>> + old_bitmap = efi_get_unaccepted_bitmap();
>>>> + if (!old_bitmap)
>>>> + return -EIO;
>>>> +
>>>> + /* If the bitmap is already large enough, just set the bits */
>>>> + if (unacc_tbl->size >= size) {
>>>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>>>> + bitmap_set(old_bitmap, start / unit_size, (end - start) / unit_size);
>>>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>>>> +
>>>> + return 0;
>>>> + }
>>>> +
>>>> + /* Reserved memblocks cannot be extended so allocate a new bitmap */
>>>> + if (unacc_tbl->mem_reserved) {
>>>> + new_bitmap = kzalloc(size, GFP_KERNEL);
>>>> + if (!new_bitmap)
>>>> + return -ENOMEM;
>>>> +
>>>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>>>> + memcpy(new_bitmap, old_bitmap, unacc_tbl->size);
>>>> + unacc_tbl->mem_reserved = false;
>>>> + free_reserved_area(old_bitmap, old_bitmap + unacc_tbl->size, -1, NULL);
>>>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>>>> + } else {
>>>> + new_bitmap = krealloc(old_bitmap, size, GFP_KERNEL);
>>>> + if (!new_bitmap)
>>>> + return -ENOMEM;
>>>> +
>>>> + /* Zero the bitmap from the range it was extended from */
>>>> + memset(new_bitmap + unacc_tbl->size, 0, size - unacc_tbl->size);
>>>> + }
>>>> +
>>>> + bitmap_set(new_bitmap, start / unit_size, (end - start) / unit_size);
>>>> +
>>>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>>>> + unacc_tbl->size = size;
>>>> + unacc_tbl->bitmap = (unsigned long *)__pa(new_bitmap);
>>>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +int accept_hotplug_memory(phys_addr_t mem_range_start, unsigned long mem_range_size)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + if (!IS_ENABLED(CONFIG_UNACCEPTED_MEMORY))
>>>> + return 0;
>>>> +
>>>> + ret = extend_unaccepted_bitmap(mem_range_start, mem_range_size);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + if (!mm_lazy_accept_enabled())
>>>> + accept_memory(mem_range_start, mem_range_size);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> #ifdef CONFIG_PROC_VMCORE
>>>> static bool unaccepted_memory_vmcore_pfn_is_ram(struct vmcore_cb *cb,
>>>> unsigned long pfn)
>>>> diff --git a/include/linux/efi.h b/include/linux/efi.h
>>>> index a74b393c54d8..1021eb78388f 100644
>>>> --- a/include/linux/efi.h
>>>> +++ b/include/linux/efi.h
>>>> @@ -545,6 +545,7 @@ struct efi_unaccepted_memory {
>>>> u32 unit_size;
>>>> u64 phys_base;
>>>> u64 size;
>>>> + bool mem_reserved;
>>>> unsigned long *bitmap;
>>>> };
>>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>>>> index 1ae97a0b8ec7..bb43876e6c47 100644
>>>> --- a/include/linux/mm.h
>>>> +++ b/include/linux/mm.h
>>>> @@ -4077,6 +4077,9 @@ int set_anon_vma_name(unsigned long addr, unsigned long size,
>>>> bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size);
>>>> void accept_memory(phys_addr_t start, unsigned long size);
>>>> +int accept_hotplug_memory(phys_addr_t mem_range_start,
>>>> + unsigned long mem_range_size);
>>>> +bool mm_lazy_accept_enabled(void);
>>>> #else
>>>> @@ -4090,6 +4093,14 @@ static inline void accept_memory(phys_addr_t start, unsigned long size)
>>>> {
>>>> }
>>>> +static inline int accept_hotplug_memory(phys_addr_t mem_range_start,
>>>> + unsigned long mem_range_size)
>>>> +{
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static inline bool mm_lazy_accept_enabled(void) { return false; }
>>>> +
>>>> #endif
>>>> static inline bool pfn_is_unaccepted_memory(unsigned long pfn)
>>>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>>>> index 74318c787715..bf8086682b66 100644
>>>> --- a/mm/memory_hotplug.c
>>>> +++ b/mm/memory_hotplug.c
>>>> @@ -1581,6 +1581,13 @@ int add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
>>>> if (!strcmp(res->name, "System RAM"))
>>>> firmware_map_add_hotplug(start, start + size, "System RAM");
>>>> + ret = accept_hotplug_memory(start, size);
>>>
>>> What makes this special that we have to have "hotplug_memory" as part of the name?
>>>
>>> Staring at the helper itself, there isn't anything really hotplug specific happening in there except extending the bitmap, maybe?
>>>
>>
>> Right, we are extending the original bitmap and initializing a structure
>> to track state as well. I added the hotplug_memory keyword without
>> much thought, since I didn't see anyone else attempting to extend these
>> structures.
>>
>> That said, I agree the name is awkward. I could either come up with
>> something different, or we could eliminate the parent function
>> entirely and call extend_unaccepted_bitmap() + accept_memory() directly
>> from add_memory_resource(). Similarly, we could do the same to
>> s/unaccept_hotplug_memory/unaccept_memory too.
>
> BTW, can't we allocate the bitmap based on maximum memory in the system as indicated by e820 (which includes to-maybe-be-hotplugged-ranges) and not do this allocation during hotplug events?
>
> If you search for max_possible_pfn / max_pfn I think you should find what I mean.
>
> Then it would be a simple accept_memory().
>
Agreed, I think Kiryl was hinting at pre-allocated bitmaps as well.
Since, the overhead to do this upfront is fairly minimal, that should
certainly simplify things and have very little to no meddling with the
original EFI struct.
--Pratik
^ permalink raw reply
* Re: [PATCH 1/3] KVM: guest_memfd: Remove preparation tracking
From: Vishal Annapurve @ 2025-12-01 19:33 UTC (permalink / raw)
To: Yan Zhao
Cc: Michael Roth, kvm, linux-coco, linux-mm, linux-kernel,
thomas.lendacky, pbonzini, seanjc, vbabka, ashish.kalra,
liam.merwick, david, ackerleytng, aik, ira.weiny
In-Reply-To: <aS0ClozgeICZN/XX@yzhao56-desk.sh.intel.com>
On Sun, Nov 30, 2025 at 6:53 PM Yan Zhao <yan.y.zhao@intel.com> wrote:
>
> On Sun, Nov 30, 2025 at 05:35:41PM -0800, Vishal Annapurve wrote:
> > On Mon, Nov 24, 2025 at 7:15 PM Yan Zhao <yan.y.zhao@intel.com> wrote:
> > > > > > @@ -889,7 +872,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, max_order, opaque);
> > > > > > if (!ret)
> > > > > > - kvm_gmem_mark_prepared(folio);
> > > > > > + folio_mark_uptodate(folio);
> > > > > As also asked in [1], why is the entire folio marked as uptodate here? Why does
> > > > > kvm_gmem_get_pfn() clear all pages of a huge folio when the folio isn't marked
> > > > > uptodate?
> > > >
> > > > Quoting your example from[1] for more context:
> > > >
> > > > > I also have a question about this patch:
> > > > >
> > > > > Suppose there's a 2MB huge folio A, where
> > > > > A1 and A2 are 4KB pages belonging to folio A.
> > > > >
> > > > > (1) kvm_gmem_populate() invokes __kvm_gmem_get_pfn() and gets folio A.
> > > > > It adds page A1 and invokes folio_mark_uptodate() on folio A.
> > > >
> > > > In SNP hugepage patchset you responded to, it would only mark A1 as
> > > You mean code in
> > > https://github.com/amdese/linux/commits/snp-inplace-conversion-rfc1 ?
> > >
> > > > prepared/cleared. There was 4K-granularity tracking added to handle this.
> > > I don't find the code that marks only A1 as "prepared/cleared".
> > > Instead, I just found folio_mark_uptodate() is invoked by kvm_gmem_populate()
> > > to mark the entire folio A as uptodate.
> > >
> > > However, according to your statement below that "uptodate flag only tracks
> > > whether a folio has been cleared", I don't follow why and where the entire folio
> > > A would be cleared if kvm_gmem_populate() only adds page A1.
> >
> > I think kvm_gmem_populate() is currently only used by SNP and TDX
> > logic, I don't see an issue with marking the complete folio as
> > uptodate even if its partially updated by kvm_gmem_populate() paths as
> > the private memory will eventually get initialized anyways.
> Still using the above example,
> If only page A1 is passed to sev_gmem_post_populate(), will SNP initialize the
> entire folio A?
> - if yes, could you kindly point me to the code that does this? .
> - if sev_gmem_post_populate() only initializes page A1, after marking the
> complete folio A as uptodate in kvm_gmem_populate(), later faulting in page A2
> in kvm_gmem_get_pfn() will not clear page A2 by invoking clear_highpage(),
> since the entire folio A is uptodate. I don't understand why this is OK.
> Or what's the purpose of invoking clear_highpage() on other folios?
I think sev_gmem_post_populate() only initializes the ranges marked
for snp_launch_update(). Since the current code lacks a hugepage
provider, the kvm_gmem_populate() doesn't need to explicitly clear
anything for 4K backings during kvm_gmem_populate().
I see your point. Once a hugepage provider lands, kvm_gmem_populate()
can first invoke clear_highpage() or an equivalent API on a complete
huge folio before calling the architecture-specific post-populate hook
to keep the implementation consistent.
Subsequently, we need to figure out a way to avoid this clearing for
SNP/TDX/CCA private faults.
>
> Thanks
> Yan
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Borislav Petkov @ 2025-12-01 19:10 UTC (permalink / raw)
To: David Hildenbrand (Red Hat)
Cc: Kiryl Shutsemau, Pratik R. Sampat, linux-mm, linux-coco,
linux-efi, x86, linux-kernel, tglx, mingo, dave.hansen, ardb,
akpm, osalvador, thomas.lendacky, michael.roth
In-Reply-To: <052d7f47-edb6-4978-bc9a-c7eae469720f@kernel.org>
On Mon, Dec 01, 2025 at 07:32:38PM +0100, David Hildenbrand (Red Hat) wrote:
> I think we are in agreement: from what I recall, this software contract used to be
> rather simple and stable.
Ok, please point me to the *explicit* document in our tree which says: "we
won't break the kernel and support kexec with any kernel version"?
Something ala Documentation/process/stable-api-nonsense.rst
Which says things like:
"Assuming that we had a stable kernel source interface for the kernel,
a binary interface would naturally happen too, right? Wrong."
Which I read as a "no" to the kexec question too.
IOW, it is not about whether it works or not - it is about enforcing that.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: David Hildenbrand (Red Hat) @ 2025-12-01 18:36 UTC (permalink / raw)
To: Pratik R. Sampat, linux-mm, linux-coco, linux-efi, x86,
linux-kernel
Cc: tglx, mingo, bp, dave.hansen, kas, ardb, akpm, osalvador,
thomas.lendacky, michael.roth
In-Reply-To: <73a69c03-feda-4c56-9db1-30ec489066fb@amd.com>
On 12/1/25 18:21, Pratik R. Sampat wrote:
>
>
> On 11/28/25 3:32 AM, David Hildenbrand (Red Hat) wrote:
>> On 11/25/25 18:57, Pratik R. Sampat wrote:
>>> The unaccepted memory structure currently only supports accepting memory
>>> present at boot time. The unaccepted table uses a fixed-size bitmap
>>> reserved in memblock based on the initial memory layout, preventing
>>> dynamic addition of memory ranges after boot. This causes guest
>>> termination when memory is hot-added in a secure virtual machine due to
>>> accessing pages that have not transitioned to private before use.
>>>
>>> Extend the unaccepted memory framework to handle hotplugged memory by
>>> dynamically managing the unaccepted bitmap. Allocate a new bitmap when
>>> hotplugged ranges exceed the reserved bitmap capacity and switch to
>>> kernel-managed allocation.
>>>
>>> Hotplugged memory also follows the same acceptance policy using the
>>> accept_memory=[eager|lazy] kernel parameter to accept memory either
>>> up-front when added or before first use.
>>>
>>> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
>>> ---
>>> arch/x86/boot/compressed/efi.h | 1 +
>>> .../firmware/efi/libstub/unaccepted_memory.c | 1 +
>>> drivers/firmware/efi/unaccepted_memory.c | 83 +++++++++++++++++++
>>> include/linux/efi.h | 1 +
>>> include/linux/mm.h | 11 +++
>>> mm/memory_hotplug.c | 7 ++
>>> mm/page_alloc.c | 2 +
>>> 7 files changed, 106 insertions(+)
>>>
>>> diff --git a/arch/x86/boot/compressed/efi.h b/arch/x86/boot/compressed/efi.h
>>> index 4f7027f33def..a220a1966cae 100644
>>> --- a/arch/x86/boot/compressed/efi.h
>>> +++ b/arch/x86/boot/compressed/efi.h
>>> @@ -102,6 +102,7 @@ struct efi_unaccepted_memory {
>>> u32 unit_size;
>>> u64 phys_base;
>>> u64 size;
>>> + bool mem_reserved;
>>> unsigned long *bitmap;
>>> };
>>> diff --git a/drivers/firmware/efi/libstub/unaccepted_memory.c b/drivers/firmware/efi/libstub/unaccepted_memory.c
>>> index c1370fc14555..b16bd61c12bf 100644
>>> --- a/drivers/firmware/efi/libstub/unaccepted_memory.c
>>> +++ b/drivers/firmware/efi/libstub/unaccepted_memory.c
>>> @@ -83,6 +83,7 @@ efi_status_t allocate_unaccepted_bitmap(__u32 nr_desc,
>>> unaccepted_table->unit_size = EFI_UNACCEPTED_UNIT_SIZE;
>>> unaccepted_table->phys_base = unaccepted_start;
>>> unaccepted_table->size = bitmap_size;
>>> + unaccepted_table->mem_reserved = true;
>>> memset(unaccepted_table->bitmap, 0, bitmap_size);
>>> status = efi_bs_call(install_configuration_table,
>>> diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
>>> index 4479aad258f8..8537812346e2 100644
>>> --- a/drivers/firmware/efi/unaccepted_memory.c
>>> +++ b/drivers/firmware/efi/unaccepted_memory.c
>>> @@ -218,6 +218,89 @@ bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size)
>>> return ret;
>>> }
>>> +static int extend_unaccepted_bitmap(phys_addr_t mem_range_start,
>>> + unsigned long mem_range_size)
>>> +{
>>> + struct efi_unaccepted_memory *unacc_tbl;
>>> + unsigned long *old_bitmap, *new_bitmap;
>>> + phys_addr_t start, end, mem_range_end;
>>> + u64 phys_base, size, unit_size;
>>> + unsigned long flags;
>>> +
>>> + unacc_tbl = efi_get_unaccepted_table();
>>> + if (!unacc_tbl || !unacc_tbl->unit_size)
>>> + return -EIO;
>>> +
>>> + unit_size = unacc_tbl->unit_size;
>>> + phys_base = unacc_tbl->phys_base;
>>> +
>>> + mem_range_end = round_up(mem_range_start + mem_range_size, unit_size);
>>> + size = DIV_ROUND_UP(mem_range_end - phys_base, unit_size * BITS_PER_BYTE);
>>> +
>>> + /* Translate to offsets from the beginning of the bitmap */
>>> + start = mem_range_start - phys_base;
>>> + end = mem_range_end - phys_base;
>>> +
>>> + old_bitmap = efi_get_unaccepted_bitmap();
>>> + if (!old_bitmap)
>>> + return -EIO;
>>> +
>>> + /* If the bitmap is already large enough, just set the bits */
>>> + if (unacc_tbl->size >= size) {
>>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>>> + bitmap_set(old_bitmap, start / unit_size, (end - start) / unit_size);
>>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>>> +
>>> + return 0;
>>> + }
>>> +
>>> + /* Reserved memblocks cannot be extended so allocate a new bitmap */
>>> + if (unacc_tbl->mem_reserved) {
>>> + new_bitmap = kzalloc(size, GFP_KERNEL);
>>> + if (!new_bitmap)
>>> + return -ENOMEM;
>>> +
>>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>>> + memcpy(new_bitmap, old_bitmap, unacc_tbl->size);
>>> + unacc_tbl->mem_reserved = false;
>>> + free_reserved_area(old_bitmap, old_bitmap + unacc_tbl->size, -1, NULL);
>>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>>> + } else {
>>> + new_bitmap = krealloc(old_bitmap, size, GFP_KERNEL);
>>> + if (!new_bitmap)
>>> + return -ENOMEM;
>>> +
>>> + /* Zero the bitmap from the range it was extended from */
>>> + memset(new_bitmap + unacc_tbl->size, 0, size - unacc_tbl->size);
>>> + }
>>> +
>>> + bitmap_set(new_bitmap, start / unit_size, (end - start) / unit_size);
>>> +
>>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>>> + unacc_tbl->size = size;
>>> + unacc_tbl->bitmap = (unsigned long *)__pa(new_bitmap);
>>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +int accept_hotplug_memory(phys_addr_t mem_range_start, unsigned long mem_range_size)
>>> +{
>>> + int ret;
>>> +
>>> + if (!IS_ENABLED(CONFIG_UNACCEPTED_MEMORY))
>>> + return 0;
>>> +
>>> + ret = extend_unaccepted_bitmap(mem_range_start, mem_range_size);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (!mm_lazy_accept_enabled())
>>> + accept_memory(mem_range_start, mem_range_size);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> #ifdef CONFIG_PROC_VMCORE
>>> static bool unaccepted_memory_vmcore_pfn_is_ram(struct vmcore_cb *cb,
>>> unsigned long pfn)
>>> diff --git a/include/linux/efi.h b/include/linux/efi.h
>>> index a74b393c54d8..1021eb78388f 100644
>>> --- a/include/linux/efi.h
>>> +++ b/include/linux/efi.h
>>> @@ -545,6 +545,7 @@ struct efi_unaccepted_memory {
>>> u32 unit_size;
>>> u64 phys_base;
>>> u64 size;
>>> + bool mem_reserved;
>>> unsigned long *bitmap;
>>> };
>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>>> index 1ae97a0b8ec7..bb43876e6c47 100644
>>> --- a/include/linux/mm.h
>>> +++ b/include/linux/mm.h
>>> @@ -4077,6 +4077,9 @@ int set_anon_vma_name(unsigned long addr, unsigned long size,
>>> bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size);
>>> void accept_memory(phys_addr_t start, unsigned long size);
>>> +int accept_hotplug_memory(phys_addr_t mem_range_start,
>>> + unsigned long mem_range_size);
>>> +bool mm_lazy_accept_enabled(void);
>>> #else
>>> @@ -4090,6 +4093,14 @@ static inline void accept_memory(phys_addr_t start, unsigned long size)
>>> {
>>> }
>>> +static inline int accept_hotplug_memory(phys_addr_t mem_range_start,
>>> + unsigned long mem_range_size)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static inline bool mm_lazy_accept_enabled(void) { return false; }
>>> +
>>> #endif
>>> static inline bool pfn_is_unaccepted_memory(unsigned long pfn)
>>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>>> index 74318c787715..bf8086682b66 100644
>>> --- a/mm/memory_hotplug.c
>>> +++ b/mm/memory_hotplug.c
>>> @@ -1581,6 +1581,13 @@ int add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
>>> if (!strcmp(res->name, "System RAM"))
>>> firmware_map_add_hotplug(start, start + size, "System RAM");
>>> + ret = accept_hotplug_memory(start, size);
>>
>> What makes this special that we have to have "hotplug_memory" as part of the name?
>>
>> Staring at the helper itself, there isn't anything really hotplug specific happening in there except extending the bitmap, maybe?
>>
>
> Right, we are extending the original bitmap and initializing a structure
> to track state as well. I added the hotplug_memory keyword without
> much thought, since I didn't see anyone else attempting to extend these
> structures.
>
> That said, I agree the name is awkward. I could either come up with
> something different, or we could eliminate the parent function
> entirely and call extend_unaccepted_bitmap() + accept_memory() directly
> from add_memory_resource(). Similarly, we could do the same to
> s/unaccept_hotplug_memory/unaccept_memory too.
BTW, can't we allocate the bitmap based on maximum memory in the system
as indicated by e820 (which includes to-maybe-be-hotplugged-ranges) and
not do this allocation during hotplug events?
If you search for max_possible_pfn / max_pfn I think you should find
what I mean.
Then it would be a simple accept_memory().
--
Cheers
David
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: David Hildenbrand (Red Hat) @ 2025-12-01 18:32 UTC (permalink / raw)
To: Borislav Petkov
Cc: Kiryl Shutsemau, Pratik R. Sampat, linux-mm, linux-coco,
linux-efi, x86, linux-kernel, tglx, mingo, dave.hansen, ardb,
akpm, osalvador, thomas.lendacky, michael.roth
In-Reply-To: <20251201111201.GAaS14AX18qeHN20xf@fat_crate.local>
On 12/1/25 12:12, Borislav Petkov wrote:
> On Mon, Dec 01, 2025 at 10:18:37AM +0100, David Hildenbrand (Red Hat) wrote:
>> Makes sense to me, especially for confidential VMs where we pass such
>> kernel-managed data from the old to the new kernel.
>
> It shouldn't matter, right?
>
> I think the question is whether the kernel should agree to the software
> contract (/eyeroll) to keep the kernel ABI compatible wrt kexec.
>
> And I don't think we have agreed to that AFAIK.
I think we are in agreement: from what I recall, this software contract used to be
rather simple and stable.
Looking into the details, I guess it's all about
$ grep "define LINUX_EFI_" include/linux/efi.h
#define LINUX_EFI_CRASH_GUID EFI_GUID(0xcfc8fc79, 0xbe2e, 0x4ddc, 0x97, 0xf0, 0x9f, 0x98, 0xbf, 0xe2, 0x98, 0xa0)
#define LINUX_EFI_SCREEN_INFO_TABLE_GUID EFI_GUID(0xe03fc20a, 0x85dc, 0x406e, 0xb9, 0x0e, 0x4a, 0xb5, 0x02, 0x37, 0x1d, 0x95)
#define LINUX_EFI_ARM_CPU_STATE_TABLE_GUID EFI_GUID(0xef79e4aa, 0x3c3d, 0x4989, 0xb9, 0x02, 0x07, 0xa9, 0x43, 0xe5, 0x50, 0xd2)
#define LINUX_EFI_LOADER_ENTRY_GUID EFI_GUID(0x4a67b082, 0x0a4c, 0x41cf, 0xb6, 0xc7, 0x44, 0x0b, 0x29, 0xbb, 0x8c, 0x4f)
#define LINUX_EFI_RANDOM_SEED_TABLE_GUID EFI_GUID(0x1ce1e5bc, 0x7ceb, 0x42f2, 0x81, 0xe5, 0x8a, 0xad, 0xf1, 0x80, 0xf5, 0x7b)
#define LINUX_EFI_TPM_EVENT_LOG_GUID EFI_GUID(0xb7799cb0, 0xeca2, 0x4943, 0x96, 0x67, 0x1f, 0xae, 0x07, 0xb7, 0x47, 0xfa)
#define LINUX_EFI_MEMRESERVE_TABLE_GUID EFI_GUID(0x888eb0c6, 0x8ede, 0x4ff5, 0xa8, 0xf0, 0x9a, 0xee, 0x5c, 0xb9, 0x77, 0xc2)
#define LINUX_EFI_INITRD_MEDIA_GUID EFI_GUID(0x5568e427, 0x68fc, 0x4f3d, 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
#define LINUX_EFI_MOK_VARIABLE_TABLE_GUID EFI_GUID(0xc451ed2b, 0x9694, 0x45d3, 0xba, 0xba, 0xed, 0x9f, 0x89, 0x88, 0xa3, 0x89)
#define LINUX_EFI_COCO_SECRET_AREA_GUID EFI_GUID(0xadf956ad, 0xe98c, 0x484c, 0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47)
#define LINUX_EFI_BOOT_MEMMAP_GUID EFI_GUID(0x800f683f, 0xd08b, 0x423a, 0xa2, 0x93, 0x96, 0x5c, 0x3c, 0x6f, 0xe2, 0xb4)
#define LINUX_EFI_UNACCEPTED_MEM_TABLE_GUID EFI_GUID(0xd5d1de3c, 0x105c, 0x44f9, 0x9e, 0xa9, 0xbc, 0xef, 0x98, 0x12, 0x00, 0x31)
#define LINUX_EFI_LOADED_IMAGE_FIXED_GUID EFI_GUID(0xf5a37b6d, 0x3344, 0x42a5, 0xb6, 0xbb, 0x97, 0x86, 0x48, 0xc1, 0x89, 0x0a)
Likely the format of these section stayed unchanged over the years.
--
Cheers
David
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: David Hildenbrand (Red Hat) @ 2025-12-01 18:25 UTC (permalink / raw)
To: Pratik R. Sampat, Kiryl Shutsemau
Cc: linux-mm, linux-coco, linux-efi, x86, linux-kernel, tglx, mingo,
bp, dave.hansen, ardb, akpm, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <c8926ced-3ef3-4c11-9d04-00db388887c5@amd.com>
On 12/1/25 18:15, Pratik R. Sampat wrote:
> Hi David,
>
> On 11/28/25 3:34 AM, David Hildenbrand (Red Hat) wrote:
>> On 11/27/25 18:40, Kiryl Shutsemau wrote:
>>> On Wed, Nov 26, 2025 at 04:27:29PM -0600, Pratik R. Sampat wrote:
>>>>
>>>>
>>>> On 11/26/25 5:12 AM, Kiryl Shutsemau wrote:
>>>>> On Tue, Nov 25, 2025 at 11:57:51AM -0600, Pratik R. Sampat wrote:
>>>>>> The unaccepted memory structure currently only supports accepting memory
>>>>>> present at boot time. The unaccepted table uses a fixed-size bitmap
>>>>>> reserved in memblock based on the initial memory layout, preventing
>>>>>> dynamic addition of memory ranges after boot. This causes guest
>>>>>> termination when memory is hot-added in a secure virtual machine due to
>>>>>> accessing pages that have not transitioned to private before use.
>>>>>
>>>>> How does the hot-pluggable memory look in EFI memory map? I thought
>>>>> hot-pluggable ranges suppose to be declared thare. The cleanest solution
>>>>> would be to have hot-pluggable and unaccepted indicated in EFI memory,
>>>>> so we can size bitmap accordingly upfront.
>>>>>
>>>>
>>>> I'm not quite sure if I fully understand. Do you mean to refer to the
>>>> EFI_MEMORY_HOT_PLUGGABLE attribute that is used for cold plugged boot
>>>> memory? If so, wouldn't it still be desirable to increase the size of
>>>> the bitmap to what was marked as hotpluggable initially?
>>>
>>> I just don't understand how hotpluggable memory presented in EFI memory
>>> map in presence of unaccepted memory. If not-yet-plugged memory marked
>>> as unaccepted we can preallocate bitmap upfront and make unaccepted
>>> memory transparent wrt hotplug.
>>>
>>> BTW, isn't virtio-mem a more attractive target to support than HW-style
>>> hotplug?
>>
>> I would have thought so as well, such that we can just let virtio-mem take care of any acceptance before actually using hotplugged memory (exposing it to the buddy).
>>
>> Likely there is desire to support other hypervisors?
>
> That's true. We are certainly thinking about how the RAM discard manager
> should look like with multiple states to allow guest_memfd and
> virtio-mem to work together.
>
Right, there is the QEMU side of it as well.
> Since both paths in Linux eventually converge around
> add_memory_resource(), based on some light hacking in QEMU I could see
> similar hotplug behavior for virtio-mem as well.
For virtio-mem it would not be add_memory_resource().
Whenever we would be plugging memory we would be accepting it, and when
we would be unplugging memory we would unaccept it.
That is, acceptance does not happen at add_memory_resource() time, but
when virtio-mem asks the device to transition a device block from
unplugged<->plugged.
That also means that kexec is not a concern, because the device block
state will reflect whether memory was accepted or not.
So far the theory :)
So it will be very different to DIMM-based hotplug handling.
--
Cheers
David
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Pratik R. Sampat @ 2025-12-01 17:58 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: linux-mm, linux-coco, linux-efi, x86, linux-kernel, tglx, mingo,
bp, dave.hansen, ardb, akpm, david, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <maleev6ofzlnhi3rmqjawlllxkda4mrgwmb6msz5gz77klfrxl@adpqtakqrxrj>
On 12/1/25 11:48 AM, Kiryl Shutsemau wrote:
> On Mon, Dec 01, 2025 at 11:15:13AM -0600, Pratik R. Sampat wrote:
>>
>>
>> On 11/27/25 11:40 AM, Kiryl Shutsemau wrote:
>>> On Wed, Nov 26, 2025 at 04:27:29PM -0600, Pratik R. Sampat wrote:
>>>>
>>>>
>>>> On 11/26/25 5:12 AM, Kiryl Shutsemau wrote:
>>>>> On Tue, Nov 25, 2025 at 11:57:51AM -0600, Pratik R. Sampat wrote:
>>>>>> The unaccepted memory structure currently only supports accepting memory
>>>>>> present at boot time. The unaccepted table uses a fixed-size bitmap
>>>>>> reserved in memblock based on the initial memory layout, preventing
>>>>>> dynamic addition of memory ranges after boot. This causes guest
>>>>>> termination when memory is hot-added in a secure virtual machine due to
>>>>>> accessing pages that have not transitioned to private before use.
>>>>>
>>>>> How does the hot-pluggable memory look in EFI memory map? I thought
>>>>> hot-pluggable ranges suppose to be declared thare. The cleanest solution
>>>>> would be to have hot-pluggable and unaccepted indicated in EFI memory,
>>>>> so we can size bitmap accordingly upfront.
>>>>>
>>>>
>>>> I'm not quite sure if I fully understand. Do you mean to refer to the
>>>> EFI_MEMORY_HOT_PLUGGABLE attribute that is used for cold plugged boot
>>>> memory? If so, wouldn't it still be desirable to increase the size of
>>>> the bitmap to what was marked as hotpluggable initially?
>>>
>>> I just don't understand how hotpluggable memory presented in EFI memory
>>> map in presence of unaccepted memory. If not-yet-plugged memory marked
>>> as unaccepted we can preallocate bitmap upfront and make unaccepted
>>> memory transparent wrt hotplug.
>>
>> If memory that hasn't been plugged yet never gets plugged in or is only
>> partially plugged in, wouldn't we be wasting space by preallocating
>> the bitmap upfront? Or would that not be a concern in favor of
>> transparency?
>
> 4k per 64GiB of physical address space should be low enough to ignore, no?
>
> We can look into optimizing it out when it is an actual, not imaginary
> problem.
>
Sure, that's fair!
--Pratik
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox