* Re: [PATCH v6 04/20] dma-pool: track decrypted atomic pools and select them via attrs
From: Petr Tesarik @ 2026-06-09 12:23 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm)
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
Mostafa Saleh, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
Michael Kelley
In-Reply-To: <20260604083959.1265923-5-aneesh.kumar@kernel.org>
On Thu, 4 Jun 2026 14:09:43 +0530
"Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> wrote:
> Teach the atomic DMA pool code to distinguish between encrypted and
> unencrypted pools, and make pool allocation select the matching pool based
> on DMA attributes.
>
> Introduce a dma_gen_pool wrapper that records whether a pool is
> unencrypted, initialize that state when the atomic pools are created, and
> use it when expanding and resizing the pools. Update dma_alloc_from_pool()
> to take attrs and skip pools whose encrypted state does not match
> DMA_ATTR_CC_SHARED. Update dma_free_from_pool() accordingly.
>
> Also pass DMA_ATTR_CC_SHARED from the swiotlb atomic allocation path so
> decrypted swiotlb allocations are taken from the correct atomic pool.
>
> Tested-by: Jiri Pirko <jiri@nvidia.com>
> Tested-by: Michael Kelley <mhklinux@outlook.com>
> Tested-by: Mostafa Saleh <smostafa@google.com>
> Reviewed-by: Mostafa Saleh <smostafa@google.com>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
FWIW this also looks good to me, but I don't think I'm the best person
to review changed to DMA generic pools.
Petr T
> ---
> drivers/iommu/dma-iommu.c | 2 +-
> include/linux/dma-map-ops.h | 2 +-
> kernel/dma/direct.c | 11 ++-
> kernel/dma/pool.c | 167 +++++++++++++++++++++++-------------
> kernel/dma/swiotlb.c | 7 +-
> 5 files changed, 123 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 54d96e847f16..c2595bee3d41 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -1673,7 +1673,7 @@ void *iommu_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
> if (IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
> !gfpflags_allow_blocking(gfp) && !coherent)
> page = dma_alloc_from_pool(dev, PAGE_ALIGN(size), &cpu_addr,
> - gfp, NULL);
> + gfp, attrs, NULL);
> else
> cpu_addr = iommu_dma_alloc_pages(dev, size, &page, gfp, attrs);
> if (!cpu_addr)
> diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
> index 6a1832a73cad..696b2c3a2305 100644
> --- a/include/linux/dma-map-ops.h
> +++ b/include/linux/dma-map-ops.h
> @@ -212,7 +212,7 @@ void *dma_common_pages_remap(struct page **pages, size_t size, pgprot_t prot,
> void dma_common_free_remap(void *cpu_addr, size_t size);
>
> struct page *dma_alloc_from_pool(struct device *dev, size_t size,
> - void **cpu_addr, gfp_t flags,
> + void **cpu_addr, gfp_t flags, unsigned long attrs,
> bool (*phys_addr_ok)(struct device *, phys_addr_t, size_t));
> bool dma_free_from_pool(struct device *dev, void *start, size_t size);
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 90dc5057a0c0..681f16a984ab 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -154,7 +154,7 @@ static bool dma_direct_use_pool(struct device *dev, gfp_t gfp)
> }
>
> static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
> - dma_addr_t *dma_handle, gfp_t gfp)
> + dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
> {
> struct page *page;
> u64 phys_limit;
> @@ -164,7 +164,8 @@ static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
> return NULL;
>
> gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
> - page = dma_alloc_from_pool(dev, size, &ret, gfp, dma_coherent_ok);
> + page = dma_alloc_from_pool(dev, size, &ret, gfp, attrs,
> + dma_coherent_ok);
> if (!page)
> return NULL;
> *dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
> @@ -253,7 +254,8 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> */
> if ((remap || (attrs & DMA_ATTR_CC_SHARED)) &&
> dma_direct_use_pool(dev, gfp))
> - return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
> + return dma_direct_alloc_from_pool(dev, size, dma_handle,
> + gfp, attrs);
>
> if (is_swiotlb_for_alloc(dev)) {
> page = dma_direct_alloc_swiotlb(dev, size);
> @@ -401,7 +403,8 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
> attrs |= DMA_ATTR_CC_SHARED;
>
> if ((attrs & DMA_ATTR_CC_SHARED) && dma_direct_use_pool(dev, gfp))
> - return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
> + return dma_direct_alloc_from_pool(dev, size, dma_handle,
> + gfp, attrs);
>
> if (is_swiotlb_for_alloc(dev)) {
> page = dma_direct_alloc_swiotlb(dev, size);
> diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
> index 2b2fbb709242..be78474a6c49 100644
> --- a/kernel/dma/pool.c
> +++ b/kernel/dma/pool.c
> @@ -12,12 +12,18 @@
> #include <linux/set_memory.h>
> #include <linux/slab.h>
> #include <linux/workqueue.h>
> +#include <linux/cc_platform.h>
>
> -static struct gen_pool *atomic_pool_dma __ro_after_init;
> +struct dma_gen_pool {
> + bool unencrypted;
> + struct gen_pool *pool;
> +};
> +
> +static struct dma_gen_pool atomic_pool_dma __ro_after_init;
> static unsigned long pool_size_dma;
> -static struct gen_pool *atomic_pool_dma32 __ro_after_init;
> +static struct dma_gen_pool atomic_pool_dma32 __ro_after_init;
> static unsigned long pool_size_dma32;
> -static struct gen_pool *atomic_pool_kernel __ro_after_init;
> +static struct dma_gen_pool atomic_pool_kernel __ro_after_init;
> static unsigned long pool_size_kernel;
>
> /* Size can be defined by the coherent_pool command line */
> @@ -76,11 +82,12 @@ static bool cma_in_zone(gfp_t gfp)
> return true;
> }
>
> -static int atomic_pool_expand(struct gen_pool *pool, size_t pool_size,
> +static int atomic_pool_expand(struct dma_gen_pool *dma_pool, size_t pool_size,
> gfp_t gfp)
> {
> unsigned int order;
> struct page *page = NULL;
> + bool leak_pages = false;
> void *addr;
> int ret = -ENOMEM;
>
> @@ -113,12 +120,17 @@ static int atomic_pool_expand(struct gen_pool *pool, size_t pool_size,
> * Memory in the atomic DMA pools must be unencrypted, the pools do not
> * shrink so no re-encryption occurs in dma_direct_free().
> */
> - ret = set_memory_decrypted((unsigned long)page_to_virt(page),
> - 1 << order);
> - if (ret)
> - goto remove_mapping;
> - ret = gen_pool_add_virt(pool, (unsigned long)addr, page_to_phys(page),
> - pool_size, NUMA_NO_NODE);
> + if (dma_pool->unencrypted) {
> + ret = set_memory_decrypted((unsigned long)page_to_virt(page),
> + 1 << order);
> + if (ret) {
> + leak_pages = true;
> + goto remove_mapping;
> + }
> + }
> +
> + ret = gen_pool_add_virt(dma_pool->pool, (unsigned long)addr,
> + page_to_phys(page), pool_size, NUMA_NO_NODE);
> if (ret)
> goto encrypt_mapping;
>
> @@ -126,62 +138,67 @@ static int atomic_pool_expand(struct gen_pool *pool, size_t pool_size,
> return 0;
>
> encrypt_mapping:
> - ret = set_memory_encrypted((unsigned long)page_to_virt(page),
> - 1 << order);
> - if (WARN_ON_ONCE(ret)) {
> - /* Decrypt succeeded but encrypt failed, purposely leak */
> - goto out;
> - }
> + if (dma_pool->unencrypted &&
> + set_memory_encrypted((unsigned long)page_to_virt(page), 1 << order))
> + leak_pages = true;
> +
> remove_mapping:
> #ifdef CONFIG_DMA_DIRECT_REMAP
> dma_common_free_remap(addr, pool_size);
> free_page:
> - __free_pages(page, order);
> + if (!leak_pages)
> + __free_pages(page, order);
> #endif
> out:
> return ret;
> }
>
> -static void atomic_pool_resize(struct gen_pool *pool, gfp_t gfp)
> +static void atomic_pool_resize(struct dma_gen_pool *dma_pool, gfp_t gfp)
> {
> - if (pool && gen_pool_avail(pool) < atomic_pool_size)
> - atomic_pool_expand(pool, gen_pool_size(pool), gfp);
> + if (dma_pool->pool && gen_pool_avail(dma_pool->pool) < atomic_pool_size)
> + atomic_pool_expand(dma_pool, gen_pool_size(dma_pool->pool), gfp);
> }
>
> static void atomic_pool_work_fn(struct work_struct *work)
> {
> if (IS_ENABLED(CONFIG_ZONE_DMA))
> - atomic_pool_resize(atomic_pool_dma,
> + atomic_pool_resize(&atomic_pool_dma,
> GFP_KERNEL | GFP_DMA);
> if (IS_ENABLED(CONFIG_ZONE_DMA32))
> - atomic_pool_resize(atomic_pool_dma32,
> + atomic_pool_resize(&atomic_pool_dma32,
> GFP_KERNEL | GFP_DMA32);
> - atomic_pool_resize(atomic_pool_kernel, GFP_KERNEL);
> + atomic_pool_resize(&atomic_pool_kernel, GFP_KERNEL);
> }
>
> -static __init struct gen_pool *__dma_atomic_pool_init(size_t pool_size,
> - gfp_t gfp)
> +static __init struct dma_gen_pool *__dma_atomic_pool_init(struct dma_gen_pool *dma_pool,
> + size_t pool_size, gfp_t gfp)
> {
> - struct gen_pool *pool;
> int ret;
>
> - pool = gen_pool_create(PAGE_SHIFT, NUMA_NO_NODE);
> - if (!pool)
> + dma_pool->pool = gen_pool_create(PAGE_SHIFT, NUMA_NO_NODE);
> + if (!dma_pool->pool)
> return NULL;
>
> - gen_pool_set_algo(pool, gen_pool_first_fit_order_align, NULL);
> + gen_pool_set_algo(dma_pool->pool, gen_pool_first_fit_order_align, NULL);
> +
> + /* if platform is using memory encryption atomic pools are by default decrypted. */
> + if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
> + dma_pool->unencrypted = true;
> + else
> + dma_pool->unencrypted = false;
>
> - ret = atomic_pool_expand(pool, pool_size, gfp);
> + ret = atomic_pool_expand(dma_pool, pool_size, gfp);
> if (ret) {
> - gen_pool_destroy(pool);
> + gen_pool_destroy(dma_pool->pool);
> + dma_pool->pool = NULL;
> pr_err("DMA: failed to allocate %zu KiB %pGg pool for atomic allocation\n",
> pool_size >> 10, &gfp);
> return NULL;
> }
>
> pr_info("DMA: preallocated %zu KiB %pGg pool for atomic allocations\n",
> - gen_pool_size(pool) >> 10, &gfp);
> - return pool;
> + gen_pool_size(dma_pool->pool) >> 10, &gfp);
> + return dma_pool;
> }
>
> #ifdef CONFIG_ZONE_DMA32
> @@ -207,21 +224,22 @@ static int __init dma_atomic_pool_init(void)
>
> /* All memory might be in the DMA zone(s) to begin with */
> if (has_managed_zone(ZONE_NORMAL)) {
> - atomic_pool_kernel = __dma_atomic_pool_init(atomic_pool_size,
> - GFP_KERNEL);
> - if (!atomic_pool_kernel)
> + __dma_atomic_pool_init(&atomic_pool_kernel, atomic_pool_size, GFP_KERNEL);
> + if (!atomic_pool_kernel.pool)
> ret = -ENOMEM;
> }
> +
> if (has_managed_dma()) {
> - atomic_pool_dma = __dma_atomic_pool_init(atomic_pool_size,
> - GFP_KERNEL | GFP_DMA);
> - if (!atomic_pool_dma)
> + __dma_atomic_pool_init(&atomic_pool_dma, atomic_pool_size,
> + GFP_KERNEL | GFP_DMA);
> + if (!atomic_pool_dma.pool)
> ret = -ENOMEM;
> }
> +
> if (has_managed_dma32) {
> - atomic_pool_dma32 = __dma_atomic_pool_init(atomic_pool_size,
> - GFP_KERNEL | GFP_DMA32);
> - if (!atomic_pool_dma32)
> + __dma_atomic_pool_init(&atomic_pool_dma32, atomic_pool_size,
> + GFP_KERNEL | GFP_DMA32);
> + if (!atomic_pool_dma32.pool)
> ret = -ENOMEM;
> }
>
> @@ -230,19 +248,44 @@ static int __init dma_atomic_pool_init(void)
> }
> postcore_initcall(dma_atomic_pool_init);
>
> -static inline struct gen_pool *dma_guess_pool(struct gen_pool *prev, gfp_t gfp)
> +static inline struct dma_gen_pool *__dma_guess_pool(struct dma_gen_pool *first,
> + struct dma_gen_pool *second, struct dma_gen_pool *third)
> {
> - if (prev == NULL) {
> + if (first->pool)
> + return first;
> + if (second && second->pool)
> + return second;
> + if (third && third->pool)
> + return third;
> + return NULL;
> +}
> +
> +static inline struct dma_gen_pool *dma_guess_pool(struct dma_gen_pool *prev,
> + gfp_t gfp)
> +{
> + if (!prev) {
> if (gfp & GFP_DMA)
> - return atomic_pool_dma ?: atomic_pool_dma32 ?: atomic_pool_kernel;
> + return __dma_guess_pool(&atomic_pool_dma,
> + &atomic_pool_dma32,
> + &atomic_pool_kernel);
> +
> if (gfp & GFP_DMA32)
> - return atomic_pool_dma32 ?: atomic_pool_dma ?: atomic_pool_kernel;
> - return atomic_pool_kernel ?: atomic_pool_dma32 ?: atomic_pool_dma;
> + return __dma_guess_pool(&atomic_pool_dma32,
> + &atomic_pool_dma,
> + &atomic_pool_kernel);
> +
> + return __dma_guess_pool(&atomic_pool_kernel,
> + &atomic_pool_dma32,
> + &atomic_pool_dma);
> }
> - if (prev == atomic_pool_kernel)
> - return atomic_pool_dma32 ? atomic_pool_dma32 : atomic_pool_dma;
> - if (prev == atomic_pool_dma32)
> - return atomic_pool_dma;
> +
> + if (prev == &atomic_pool_kernel)
> + return __dma_guess_pool(&atomic_pool_dma32,
> + &atomic_pool_dma, NULL);
> +
> + if (prev == &atomic_pool_dma32)
> + return __dma_guess_pool(&atomic_pool_dma, NULL, NULL);
> +
> return NULL;
> }
>
> @@ -272,16 +315,20 @@ static struct page *__dma_alloc_from_pool(struct device *dev, size_t size,
> }
>
> struct page *dma_alloc_from_pool(struct device *dev, size_t size,
> - void **cpu_addr, gfp_t gfp,
> + void **cpu_addr, gfp_t gfp, unsigned long attrs,
> bool (*phys_addr_ok)(struct device *, phys_addr_t, size_t))
> {
> - struct gen_pool *pool = NULL;
> + struct dma_gen_pool *dma_pool = NULL;
> struct page *page;
> bool pool_found = false;
>
> - while ((pool = dma_guess_pool(pool, gfp))) {
> + while ((dma_pool = dma_guess_pool(dma_pool, gfp))) {
> +
> + if (dma_pool->unencrypted != !!(attrs & DMA_ATTR_CC_SHARED))
> + continue;
> +
> pool_found = true;
> - page = __dma_alloc_from_pool(dev, size, pool, cpu_addr,
> + page = __dma_alloc_from_pool(dev, size, dma_pool->pool, cpu_addr,
> phys_addr_ok);
> if (page)
> return page;
> @@ -296,12 +343,14 @@ struct page *dma_alloc_from_pool(struct device *dev, size_t size,
>
> bool dma_free_from_pool(struct device *dev, void *start, size_t size)
> {
> - struct gen_pool *pool = NULL;
> + struct dma_gen_pool *dma_pool = NULL;
> +
> + while ((dma_pool = dma_guess_pool(dma_pool, 0))) {
>
> - while ((pool = dma_guess_pool(pool, 0))) {
> - if (!gen_pool_has_addr(pool, (unsigned long)start, size))
> + if (!gen_pool_has_addr(dma_pool->pool, (unsigned long)start, size))
> continue;
> - gen_pool_free(pool, (unsigned long)start, size);
> +
> + gen_pool_free(dma_pool->pool, (unsigned long)start, size);
> return true;
> }
>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index ac03a6856c2e..be4d418d92ac 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -612,6 +612,7 @@ static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
> u64 phys_limit, gfp_t gfp)
> {
> struct page *page;
> + unsigned long attrs = 0;
>
> /*
> * Allocate from the atomic pools if memory is encrypted and
> @@ -623,8 +624,12 @@ static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
> if (!IS_ENABLED(CONFIG_DMA_COHERENT_POOL))
> return NULL;
>
> + /* swiotlb considered decrypted by default */
> + if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
> + attrs = DMA_ATTR_CC_SHARED;
> +
> return dma_alloc_from_pool(dev, bytes, &vaddr, gfp,
> - dma_coherent_ok);
> + attrs, dma_coherent_ok);
> }
>
> gfp &= ~GFP_ZONEMASK;
^ permalink raw reply
* Re: [PATCH v6 05/20] dma: swiotlb: pass mapping attributes by reference
From: Petr Tesarik @ 2026-06-09 12:21 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm)
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
Mostafa Saleh, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Michael Kelley
In-Reply-To: <20260604083959.1265923-6-aneesh.kumar@kernel.org>
On Thu, 4 Jun 2026 14:09:44 +0530
"Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> wrote:
> Change swiotlb_tbl_map_single() to take the DMA mapping attributes by
> reference and update the direct callers accordingly.
>
> This is a preparatory change for a follow-up patch which updates the
> attributes based on the selected swiotlb pool. Keeping the signature change
> separate makes the follow-up patch easier to review.
>
> No functional change in this patch.
>
> Tested-by: Michael Kelley <mhklinux@outlook.com>
> Tested-by: Mostafa Saleh <smostafa@google.com>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
Reviewed-by: Petr Tesarik <ptesarik@suse.com>
Thanks
Petr T
> ---
> drivers/iommu/dma-iommu.c | 2 +-
> drivers/xen/swiotlb-xen.c | 2 +-
> include/linux/swiotlb.h | 2 +-
> kernel/dma/swiotlb.c | 6 +++---
> 4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index c2595bee3d41..725c7adb0a8d 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -1180,7 +1180,7 @@ static phys_addr_t iommu_dma_map_swiotlb(struct device *dev, phys_addr_t phys,
> trace_swiotlb_bounced(dev, phys, size);
>
> phys = swiotlb_tbl_map_single(dev, phys, size, iova_mask(iovad), dir,
> - attrs);
> + &attrs);
>
> /*
> * Untrusted devices should not see padding areas with random leftover
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 2cbf2b588f5b..8c4abe65cd49 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -243,7 +243,7 @@ static dma_addr_t xen_swiotlb_map_phys(struct device *dev, phys_addr_t phys,
> */
> trace_swiotlb_bounced(dev, dev_addr, size);
>
> - map = swiotlb_tbl_map_single(dev, phys, size, 0, dir, attrs);
> + map = swiotlb_tbl_map_single(dev, phys, size, 0, dir, &attrs);
> if (map == (phys_addr_t)DMA_MAPPING_ERROR)
> return DMA_MAPPING_ERROR;
>
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 133bb8ca9032..29187cec90d8 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -238,7 +238,7 @@ static inline phys_addr_t default_swiotlb_limit(void)
>
> phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t phys,
> size_t mapping_size, unsigned int alloc_aligned_mask,
> - enum dma_data_direction dir, unsigned long attrs);
> + enum dma_data_direction dir, unsigned long *attrs);
> dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
> size_t size, enum dma_data_direction dir, unsigned long attrs);
>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index be4d418d92ac..78ce05857c00 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -1391,7 +1391,7 @@ static unsigned long mem_used(struct io_tlb_mem *mem)
> */
> phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
> size_t mapping_size, unsigned int alloc_align_mask,
> - enum dma_data_direction dir, unsigned long attrs)
> + enum dma_data_direction dir, unsigned long *attrs)
> {
> struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
> unsigned int offset;
> @@ -1425,7 +1425,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
> size = ALIGN(mapping_size + offset, alloc_align_mask + 1);
> index = swiotlb_find_slots(dev, orig_addr, size, alloc_align_mask, &pool);
> if (index == -1) {
> - if (!(attrs & DMA_ATTR_NO_WARN))
> + if (!(*attrs & DMA_ATTR_NO_WARN))
> dev_warn_ratelimited(dev,
> "swiotlb buffer is full (sz: %zd bytes), total %lu (slots), used %lu (slots)\n",
> size, mem->nslabs, mem_used(mem));
> @@ -1604,7 +1604,7 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t paddr, size_t size,
>
> trace_swiotlb_bounced(dev, phys_to_dma(dev, paddr), size);
>
> - swiotlb_addr = swiotlb_tbl_map_single(dev, paddr, size, 0, dir, attrs);
> + swiotlb_addr = swiotlb_tbl_map_single(dev, paddr, size, 0, dir, &attrs);
> if (swiotlb_addr == (phys_addr_t)DMA_MAPPING_ERROR)
> return DMA_MAPPING_ERROR;
>
^ permalink raw reply
* Re: [PATCH v6 03/20] dma-direct: use DMA_ATTR_CC_SHARED in alloc/free paths
From: Petr Tesarik @ 2026-06-09 12:18 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm)
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
Mostafa Saleh, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
Michael Kelley
In-Reply-To: <20260604083959.1265923-4-aneesh.kumar@kernel.org>
On Thu, 4 Jun 2026 14:09:42 +0530
"Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> wrote:
> Propagate force_dma_unencrypted() into DMA_ATTR_CC_SHARED in the
> dma-direct allocation path and use the attribute to drive the related
> decisions.
>
> This updates dma_direct_alloc(), dma_direct_free(), and
> dma_direct_alloc_pages() to fold the forced unencrypted case into attrs.
>
> Tested-by: Jiri Pirko <jiri@nvidia.com>
> Tested-by: Michael Kelley <mhklinux@outlook.com>
> Tested-by: Mostafa Saleh <smostafa@google.com>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
Reviewed-by: Petr Tesarik <ptesarik@suse.com>
Petr T
> ---
> kernel/dma/direct.c | 53 +++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 44 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index a741c8a2ee66..90dc5057a0c0 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -193,16 +193,31 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
> {
> bool remap = false, set_uncached = false;
> - bool mark_mem_decrypt = true;
> + bool mark_mem_decrypt = false;
> struct page *page;
> void *ret;
>
> + /*
> + * DMA_ATTR_CC_SHARED is not a caller-visible dma_alloc_*()
> + * attribute. The direct allocator uses it internally after it has
> + * decided that the backing pages must be shared/decrypted, so the
> + * rest of the allocation path can consistently select DMA addresses,
> + * choose compatible pools and restore encryption on free.
> + */
> + if (attrs & DMA_ATTR_CC_SHARED)
> + return NULL;
> +
> + if (force_dma_unencrypted(dev)) {
> + attrs |= DMA_ATTR_CC_SHARED;
> + mark_mem_decrypt = true;
> + }
> +
> size = PAGE_ALIGN(size);
> if (attrs & DMA_ATTR_NO_WARN)
> gfp |= __GFP_NOWARN;
>
> - if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
> - !force_dma_unencrypted(dev) && !is_swiotlb_for_alloc(dev))
> + if (((attrs & (DMA_ATTR_NO_KERNEL_MAPPING | DMA_ATTR_CC_SHARED)) ==
> + DMA_ATTR_NO_KERNEL_MAPPING) && !is_swiotlb_for_alloc(dev))
> return dma_direct_alloc_no_mapping(dev, size, dma_handle, gfp);
>
> if (!dev_is_dma_coherent(dev)) {
> @@ -236,7 +251,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> * Remapping or decrypting memory may block, allocate the memory from
> * the atomic pools instead if we aren't allowed block.
> */
> - if ((remap || force_dma_unencrypted(dev)) &&
> + if ((remap || (attrs & DMA_ATTR_CC_SHARED)) &&
> dma_direct_use_pool(dev, gfp))
> return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>
> @@ -312,12 +327,24 @@ void dma_direct_free(struct device *dev, size_t size,
> void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
> {
> phys_addr_t phys;
> - bool mark_mem_encrypted = true;
> + bool mark_mem_encrypted = false;
> struct io_tlb_pool *swiotlb_pool;
> unsigned int page_order = get_order(size);
>
> - if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
> - !force_dma_unencrypted(dev) && !is_swiotlb_for_alloc(dev)) {
> + /* see dma_direct_alloc() for details */
> + WARN_ON(attrs & DMA_ATTR_CC_SHARED);
> +
> + /*
> + * if the device had requested for an unencrypted buffer,
> + * convert it to encrypted on free
> + */
> + if (force_dma_unencrypted(dev)) {
> + attrs |= DMA_ATTR_CC_SHARED;
> + mark_mem_encrypted = true;
> + }
> +
> + if (((attrs & (DMA_ATTR_NO_KERNEL_MAPPING | DMA_ATTR_CC_SHARED)) ==
> + DMA_ATTR_NO_KERNEL_MAPPING) && !is_swiotlb_for_alloc(dev)) {
> /* cpu_addr is a struct page cookie, not a kernel address */
> dma_free_contiguous(dev, cpu_addr, size);
> return;
> @@ -366,10 +393,14 @@ void dma_direct_free(struct device *dev, size_t size,
> struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
> dma_addr_t *dma_handle, enum dma_data_direction dir, gfp_t gfp)
> {
> + unsigned long attrs = 0;
> struct page *page;
> void *ret;
>
> - if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
> + if (force_dma_unencrypted(dev))
> + attrs |= DMA_ATTR_CC_SHARED;
> +
> + if ((attrs & DMA_ATTR_CC_SHARED) && dma_direct_use_pool(dev, gfp))
> return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>
> if (is_swiotlb_for_alloc(dev)) {
> @@ -403,7 +434,11 @@ void dma_direct_free_pages(struct device *dev, size_t size,
> phys_addr_t phys;
> void *vaddr = page_address(page);
> struct io_tlb_pool *swiotlb_pool;
> - bool mark_mem_encrypted = true;
> + /*
> + * if the device had requested for an unencrypted buffer,
> + * convert it to encrypted on free
> + */
> + bool mark_mem_encrypted = force_dma_unencrypted(dev);
>
> /* If cpu_addr is not from an atomic pool, dma_free_from_pool() fails */
> if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
^ permalink raw reply
* Re: [PATCH v6 02/20] dma-direct: swiotlb: handle swiotlb alloc/free outside __dma_direct_alloc_pages
From: Petr Tesarik @ 2026-06-09 12:15 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm)
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
Mostafa Saleh, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
Michael Kelley
In-Reply-To: <20260604083959.1265923-3-aneesh.kumar@kernel.org>
On Thu, 4 Jun 2026 14:09:41 +0530
"Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> wrote:
> Move swiotlb allocation out of __dma_direct_alloc_pages() and handle it in
> dma_direct_alloc() / dma_direct_alloc_pages().
>
> This is needed for follow-up changes that simplify the handling of
> memory encryption/decryption based on the DMA attribute flags.
>
> swiotlb backing pages are already mapped decrypted by
> swiotlb_update_mem_attributes() and rmem_swiotlb_device_init(), so
> dma-direct should not call dma_set_decrypted() on allocation nor
> dma_set_encrypted() on free for swiotlb-backed memory.
>
> Update alloc/free paths to detect swiotlb-backed pages and skip
> encrypt/decrypt transitions for those paths. Keep the existing highmem
> rejection in dma_direct_alloc_pages() for swiotlb allocations.
>
> Only for "restricted-dma-pool", we currently set `for_alloc = true`, while
> rmem_swiotlb_device_init() decrypts the whole pool up front. This pool is
> typically used together with "shared-dma-pool", where the shared region is
> accessed after remap/ioremap and the returned address is suitable for
> decrypted memory access. So existing code paths remain valid.
>
> Tested-by: Jiri Pirko <jiri@nvidia.com>
> Tested-by: Michael Kelley <mhklinux@outlook.com>
> Tested-by: Mostafa Saleh <smostafa@google.com>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
> include/linux/swiotlb.h | 6 ++++
> kernel/dma/direct.c | 71 ++++++++++++++++++++++++++++++-----------
> kernel/dma/swiotlb.c | 6 ++++
> 3 files changed, 65 insertions(+), 18 deletions(-)
>
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 3dae0f592063..133bb8ca9032 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -284,6 +284,8 @@ extern void swiotlb_print_info(void);
> #ifdef CONFIG_DMA_RESTRICTED_POOL
> struct page *swiotlb_alloc(struct device *dev, size_t size);
> bool swiotlb_free(struct device *dev, struct page *page, size_t size);
> +void swiotlb_free_from_pool(struct device *dev, phys_addr_t tlb_addr,
> + size_t size, struct io_tlb_pool *pool);
>
> static inline bool is_swiotlb_for_alloc(struct device *dev)
> {
> @@ -299,6 +301,10 @@ static inline bool swiotlb_free(struct device *dev, struct page *page,
> {
> return false;
> }
> +static inline void swiotlb_free_from_pool(struct device *dev, phys_addr_t tlb_addr,
> + size_t size, struct io_tlb_pool *pool)
> +{
> +}
> static inline bool is_swiotlb_for_alloc(struct device *dev)
> {
> return false;
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 583c5922bca2..a741c8a2ee66 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -96,14 +96,6 @@ static int dma_set_encrypted(struct device *dev, void *vaddr, size_t size)
> return ret;
> }
>
> -static void __dma_direct_free_pages(struct device *dev, struct page *page,
> - size_t size)
> -{
> - if (swiotlb_free(dev, page, size))
> - return;
> - dma_free_contiguous(dev, page, size);
> -}
> -
> static struct page *dma_direct_alloc_swiotlb(struct device *dev, size_t size)
> {
> struct page *page = swiotlb_alloc(dev, size);
> @@ -125,9 +117,6 @@ static struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
>
> WARN_ON_ONCE(!PAGE_ALIGNED(size));
>
> - if (is_swiotlb_for_alloc(dev))
> - return dma_direct_alloc_swiotlb(dev, size);
> -
> gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
> page = dma_alloc_contiguous(dev, size, gfp);
> if (page) {
> @@ -204,6 +193,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
> {
> bool remap = false, set_uncached = false;
> + bool mark_mem_decrypt = true;
> struct page *page;
> void *ret;
>
> @@ -250,11 +240,21 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> dma_direct_use_pool(dev, gfp))
> return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>
> + if (is_swiotlb_for_alloc(dev)) {
> + page = dma_direct_alloc_swiotlb(dev, size);
> + if (page) {
> + mark_mem_decrypt = false;
> + goto setup_page;
> + }
> + return NULL;
> + }
> +
> /* we always manually zero the memory once we are done */
> page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO, true);
> if (!page)
> return NULL;
>
> +setup_page:
> /*
> * dma_alloc_contiguous can return highmem pages depending on a
> * combination the cma= arguments and per-arch setup. These need to be
> @@ -281,7 +281,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> goto out_free_pages;
> } else {
> ret = page_address(page);
> - if (dma_set_decrypted(dev, ret, size))
> + if (mark_mem_decrypt && dma_set_decrypted(dev, ret, size))
> goto out_leak_pages;
> }
>
> @@ -298,10 +298,11 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> return ret;
>
> out_encrypt_pages:
> - if (dma_set_encrypted(dev, page_address(page), size))
> + if (mark_mem_decrypt && dma_set_encrypted(dev, page_address(page), size))
> return NULL;
> out_free_pages:
> - __dma_direct_free_pages(dev, page, size);
> + if (!swiotlb_free(dev, page, size))
> + dma_free_contiguous(dev, page, size);
> return NULL;
> out_leak_pages:
> return NULL;
> @@ -310,6 +311,9 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> void dma_direct_free(struct device *dev, size_t size,
> void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
> {
> + phys_addr_t phys;
> + bool mark_mem_encrypted = true;
> + struct io_tlb_pool *swiotlb_pool;
> unsigned int page_order = get_order(size);
>
> if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
> @@ -338,16 +342,25 @@ void dma_direct_free(struct device *dev, size_t size,
> dma_free_from_pool(dev, cpu_addr, PAGE_ALIGN(size)))
> return;
>
> + phys = dma_to_phys(dev, dma_addr);
> + swiotlb_pool = swiotlb_find_pool(dev, phys);
> + if (swiotlb_pool)
> + /* Swiotlb doesn't need a page attribute update on free */
> + mark_mem_encrypted = false;
> +
> if (is_vmalloc_addr(cpu_addr)) {
> vunmap(cpu_addr);
> } else {
> if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_CLEAR_UNCACHED))
> arch_dma_clear_uncached(cpu_addr, size);
> - if (dma_set_encrypted(dev, cpu_addr, size))
> + if (mark_mem_encrypted && dma_set_encrypted(dev, cpu_addr, size))
> return;
> }
>
> - __dma_direct_free_pages(dev, dma_direct_to_page(dev, dma_addr), size);
> + if (swiotlb_pool)
> + swiotlb_free_from_pool(dev, phys, size, swiotlb_pool);
> + else
> + dma_free_contiguous(dev, dma_direct_to_page(dev, dma_addr), size);
> }
>
> struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
> @@ -359,6 +372,15 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
> if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
> return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>
> + if (is_swiotlb_for_alloc(dev)) {
> + page = dma_direct_alloc_swiotlb(dev, size);
> + if (!page)
> + return NULL;
> +
> + ret = page_address(page);
> + goto setup_page;
> + }
> +
> page = __dma_direct_alloc_pages(dev, size, gfp, false);
> if (!page)
> return NULL;
> @@ -366,6 +388,7 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
> ret = page_address(page);
> if (dma_set_decrypted(dev, ret, size))
> goto out_leak_pages;
> +setup_page:
> memset(ret, 0, size);
> *dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
> return page;
> @@ -377,16 +400,28 @@ void dma_direct_free_pages(struct device *dev, size_t size,
> struct page *page, dma_addr_t dma_addr,
> enum dma_data_direction dir)
> {
> + phys_addr_t phys;
> void *vaddr = page_address(page);
> + struct io_tlb_pool *swiotlb_pool;
> + bool mark_mem_encrypted = true;
>
> /* If cpu_addr is not from an atomic pool, dma_free_from_pool() fails */
> if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
> dma_free_from_pool(dev, vaddr, size))
> return;
>
> - if (dma_set_encrypted(dev, vaddr, size))
> + phys = page_to_phys(page);
> + swiotlb_pool = swiotlb_find_pool(dev, phys);
> + if (swiotlb_pool)
> + mark_mem_encrypted = false;
> +
> + if (mark_mem_encrypted && dma_set_encrypted(dev, vaddr, size))
> return;
> - __dma_direct_free_pages(dev, page, size);
> +
> + if (swiotlb_pool)
> + swiotlb_free_from_pool(dev, phys, size, swiotlb_pool);
> + else
> + dma_free_contiguous(dev, page, size);
> }
>
> #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 1abd3e6146f4..ac03a6856c2e 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -1809,6 +1809,12 @@ bool swiotlb_free(struct device *dev, struct page *page, size_t size)
> return true;
> }
>
> +void swiotlb_free_from_pool(struct device *dev, phys_addr_t tlb_addr, size_t size,
> + struct io_tlb_pool *pool)
What's the reason to pass the buffer size if it's not used?
Other than that, this patch looks good to me.
Petr T
> +{
> + swiotlb_release_slots(dev, tlb_addr, pool);
> +}
> +
> static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
> struct device *dev)
> {
^ permalink raw reply
* Re: [PATCH v5 5/5] iommufd/vdevice: add TSM request ioctl
From: Alexey Kardashevskiy @ 2026-06-09 10:49 UTC (permalink / raw)
To: Dan Williams (nvidia), Aneesh Kumar K.V, linux-coco, iommu,
linux-kernel, kvm
Cc: Bjorn Helgaas, Jason Gunthorpe, Joerg Roedel, Jonathan Cameron,
Kevin Tian, Nicolin Chen, Samuel Ortiz, Steven Price,
Suzuki K Poulose, Will Deacon, Xu Yilun, Shameer Kolothum,
Paolo Bonzini, Tony Krowiak, Halil Pasic, Jason Herne,
Harald Freudenberger, Holger Dengler, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Alex Williamson, Matthew Rosato, Farhan Ali,
Eric Farman, linux-s390
In-Reply-To: <6a272cebec4af_4fa7810048@djbw-dev.notmuch>
On 9/6/26 06:58, Dan Williams (nvidia) wrote:
> Aneesh Kumar K.V wrote:
> [..]
>>> I think we can wait to move it to its own IOMMU operation unless/until
>>> there is a need to set RUN outside of an explicit guest request, right?
>>
>> Something like the below? (the diff against this series)
>>
>> I have not yet integrated this into the full CCA patchset for testing,
>> but I wanted to make sure we are aligned on the UAPI.
> [..]
>> -static bool iommufd_vdevice_tsm_req_scope_valid(u32 scope)
>> +static bool iommufd_vdevice_tsm_req_arch_valid(u32 tvm_arch)
>> {
>> - if (scope > IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_LAST)
>> + switch (tvm_arch) {
>> + case IOMMU_VDEVICE_TSM_TVM_ARCH_CCA:
>> + case IOMMU_VDEVICE_TSM_TVM_ARCH_SEV:
>> + case IOMMU_VDEVICE_TSM_TVM_ARCH_TDX:
>
> Makes sense for any command that needs tunneling. However, see below, what is
> that set, and do we need a IOMMU_VDEVICE_TSM_COMMON when architecture
> differentiation is not required?
I still do not follow why making these arches checks in runtime, should be caught at the build time (ARM vs x86 vs RiscV) or at the TSM modprobe (AMD vs Intel).
The scope becomes just IOMMU_VDEVICE_TSM_REQ_SCOPE_TUNNEL imho.
>
>> + return true;
>> + default:
>> return false;
>> + }
>> +}
>>
>> - switch (scope) {
>> - case IOMMU_VDEVICE_TSM_REQ_PCI_INFO:
>> - case IOMMU_VDEVICE_TSM_REQ_PCI_STATE_CHANGE:
>> - case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_READ:
>> - case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_WRITE:
>> +static bool iommufd_vdevice_tsm_req_op_valid(u32 op, u32 tvm_arch)
>> +{
>> + switch (op) {
>> + case TSM_REQ_READ_OBJECT:
>> + case TSM_REQ_REGEN_OBJECT:
>> + case TSM_REQ_OBJECT_INFO:
>
> The design goal of the netlink device-evidence interface is to be able
> to respond to all shapes of requests for evidence. So netlink caches
> objects that the hypercall handler can fill responses from.
>
> It eliminates a class of commands that need tunneling.
+1.
>
>> + case TSM_REQ_VALIDATE_MMIO:
>> + case TSM_REQ_SET_TDI_STATE:
>
> Are these potentially candidates for a IOMMU_VDEVICE_TSM_COMMON? The
> handler knows how to do the arch-specific response from the common
> iommufd result, or is there TSM-specific payload beyond @tsm_code for
> these.
These are not common to put in IOMMUFD - it is either TSM (for TDI states) or KVM (for MMIO validate) on AMD and other arches won't share much either, right?
> Make it the case that guest_req only needs non-common arch for
> operations that are implementation unique, or where the response payload
> exceeds what can be conveyed via @tsm_code.
>
>> return true;
>> + case TSM_REQ_SEV_ENABLE_DMA:
>> + case TSM_REQ_SEV_DISABLE_DMA:
>> + return tvm_arch == IOMMU_VDEVICE_TSM_TVM_ARCH_SEV;
> Right, this appears to be the only case where the command is
> implementation unique. The handler can only ask iommufd to take
> arch-specific action.
There are 2 arch-specific actions - one is calling TSM to execute actual guest request, the other one is notifying the host IOMMU driver about the device going secure. Like this:
https://github.com/AMDESE/linux-kvm/blob/tsm/drivers/iommu/iommufd/viommu.c#L603
I can tuck domain->ops->tsm_enable into my TSM but rather would not. Thanks,
--
Alexey
^ permalink raw reply
* Re: [PATCH v5 5/5] iommufd/vdevice: add TSM request ioctl
From: Aneesh Kumar K.V @ 2026-06-09 8:59 UTC (permalink / raw)
To: Dan Williams (nvidia), Dan Williams (nvidia),
Dan Williams (nvidia), Alexey Kardashevskiy, linux-coco, iommu,
linux-kernel, kvm
Cc: Bjorn Helgaas, Dan Williams, Jason Gunthorpe, Joerg Roedel,
Jonathan Cameron, Kevin Tian, Nicolin Chen, Samuel Ortiz,
Steven Price, Suzuki K Poulose, Will Deacon, Xu Yilun,
Shameer Kolothum, Paolo Bonzini, Tony Krowiak, Halil Pasic,
Jason Herne, Harald Freudenberger, Holger Dengler, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Alex Williamson, Matthew Rosato, Farhan Ali,
Eric Farman, linux-s390
In-Reply-To: <6a272cebec4af_4fa7810048@djbw-dev.notmuch>
"Dan Williams (nvidia)" <djbw@kernel.org> writes:
> Aneesh Kumar K.V wrote:
> [..]
>> > I think we can wait to move it to its own IOMMU operation unless/until
>> > there is a need to set RUN outside of an explicit guest request, right?
>>
>> Something like the below? (the diff against this series)
>>
>> I have not yet integrated this into the full CCA patchset for testing,
>> but I wanted to make sure we are aligned on the UAPI.
> [..]
>> -static bool iommufd_vdevice_tsm_req_scope_valid(u32 scope)
>> +static bool iommufd_vdevice_tsm_req_arch_valid(u32 tvm_arch)
>> {
>> - if (scope > IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_LAST)
>> + switch (tvm_arch) {
>> + case IOMMU_VDEVICE_TSM_TVM_ARCH_CCA:
>> + case IOMMU_VDEVICE_TSM_TVM_ARCH_SEV:
>> + case IOMMU_VDEVICE_TSM_TVM_ARCH_TDX:
>
> Makes sense for any command that needs tunneling. However, see below, what is
> that set, and do we need a IOMMU_VDEVICE_TSM_COMMON when architecture
> differentiation is not required?
>
>> + return true;
>> + default:
>> return false;
>> + }
>> +}
>>
>> - switch (scope) {
>> - case IOMMU_VDEVICE_TSM_REQ_PCI_INFO:
>> - case IOMMU_VDEVICE_TSM_REQ_PCI_STATE_CHANGE:
>> - case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_READ:
>> - case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_WRITE:
>> +static bool iommufd_vdevice_tsm_req_op_valid(u32 op, u32 tvm_arch)
>> +{
>> + switch (op) {
>> + case TSM_REQ_READ_OBJECT:
>> + case TSM_REQ_REGEN_OBJECT:
>> + case TSM_REQ_OBJECT_INFO:
>
> The design goal of the netlink device-evidence interface is to be able
> to respond to all shapes of requests for evidence. So netlink caches
> objects that the hypercall handler can fill responses from.
>
> It eliminates a class of commands that need tunneling.
>
Sure, I can drop this from the iommufd ioctl and use netlink to read and
regenerate the objects from the VMM.
Can I use netlink to find the cached object size? CCA supports
RHI_DA_OBJECT_SIZE, which can be used to query the object size.
If not should we have TSM_REQ_OBJECT_INFO?
>
>> + case TSM_REQ_VALIDATE_MMIO:
>> + case TSM_REQ_SET_TDI_STATE:
>
> Are these potentially candidates for a IOMMU_VDEVICE_TSM_COMMON? The
> handler knows how to do the arch-specific response from the common
> iommufd result, or is there TSM-specific payload beyond @tsm_code for
> these.
>
> Make it the case that guest_req only needs non-common arch for
> operations that are implementation unique, or where the response payload
> exceeds what can be conveyed via @tsm_code.
>
I am not sure I follow the IOMMU_VDEVICE_TSM_COMMON feedback above.
Earlier discussions around this concluded that we may want iommufd
to validate all input commands, rather than making the guest request
ioctl a passthrough interface.
If we make the ops IOMMU_VDEVICE_TSM_COMMON, we would still need to add
TSM_REQ_VALIDATE_MMIO and TSM_REQ_SET_TDI_STATE for the arch-specific
handler. Why not expose those to the generic iommufd layer, so that we
can add operation validation there and completely drop IOMMU_VDEVICE_TSM_COMMON?
>> return true;
>> + case TSM_REQ_SEV_ENABLE_DMA:
>> + case TSM_REQ_SEV_DISABLE_DMA:
>> + return tvm_arch == IOMMU_VDEVICE_TSM_TVM_ARCH_SEV;
>
> Right, this appears to be the only case where the command is
> implementation unique. The handler can only ask iommufd to take
> arch-specific action.
-aneesh
^ permalink raw reply
* Re: [PATCH v4 10/47] x86/tsc: Consolidate forcing of X86_FEATURE_TSC_KNOWN_FREQ for PV code
From: Thomas Gleixner @ 2026-06-09 7:48 UTC (permalink / raw)
To: Sean Christopherson, David Woodhouse
Cc: Paolo Bonzini, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov, Jan Kiszka,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
John Stultz, H. Peter Anvin, Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, kvm, linux-kernel, linux-coco, linux-hyperv,
virtualization, xen-devel, Tom Lendacky, Nikunj A Dadhania,
Michael Kelley
In-Reply-To: <aidEfvTMjLa2zt43@google.com>
On Mon, Jun 08 2026 at 15:38, Sean Christopherson wrote:
> On Sat, Jun 06, 2026, David Woodhouse wrote:
>> > Along with:
>> >
>> > if (!hypervisor_is_type(X86_HYPER_NATIVE)) {
>> > if (tsc_khz_early)
>> > pr_warn("Ignoring non-sensical tsc_early_khz command line argument\n");
>> >
>> > or something daft like that.
>
> Ya, I ended up in the same place once Sashiko pointed out that skipping the SNP/TDX
> setup was hazardous[*], and also once I realized that tsc_khz_early *complemented*
> the refinement instead of replacing it.
>
> This is what I have locally:
>
> if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
> known_tsc_khz = snp_secure_tsc_init();
> else if (boot_cpu_has(X86_FEATURE_TDX_GUEST))
> known_tsc_khz = tdx_tsc_init();
>
> /*
> * If the TSC frequency wasn't provided by trusted firmware, try to get
> * it from the hypervisor (which is untrusted when running as a CoCo guest).
> */
> if (!known_tsc_khz && x86_init.hyper.get_tsc_khz)
> known_tsc_khz = x86_init.hyper.get_tsc_khz();
>
> /*
> * Mark the TSC frequency as known if it was obtained from a hypervisor
> * or trusted firmware. Don't mark the frequency as known if the user
> * specified the frequency, as the user-provided frequency is intended
> * as a "starting point", not a known, guaranteed frequency.
> */
> if (known_tsc_khz && !tsc_early_khz)
> setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
If the frequenct is known via the above then you want to set the
KNOWN_FREQ feature bit unconditionally. SNP/TDX/hypervisor override the
command line argument as you print below.
> /*
> * Ignore the user-provided TSC frequency if the exact frequency was
> * obtained from trusted firmware or the hypervisor, as the user-
> * provided frequency is intended as a "starting point", not a known,
> * guaranteed frequency.
> */
> if (!known_tsc_khz)
> known_tsc_khz = tsc_early_khz;
> else if (tsc_early_khz)
> pr_err("Ignoring 'tsc_early_khz' in favor of firmware/hypervisor.\n");
>> All the nonsense about updating it every time we enter a CPU could just
>> go away completely.
>
> But to Thomas' point, why bother? For actual old hardware, kvmclock is what it
> is. For modern hardware, it's completely antiquated.
I agree, but we are not forced to make it a first class citizen to the
detriment of sane systems.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH v13 20/22] KVM: selftests: Implement MMIO WRITE for the TDX VM
From: Binbin Wu @ 2026-06-09 6:45 UTC (permalink / raw)
To: Lisa Wang
Cc: Andrew Jones, Ackerley Tng, Chao Gao, Chenyi Qiang, Dave Hansen,
Erdem Aktas, Ira Weiny, Isaku Yamahata, Kiryl Shutsemau,
linux-kselftest, Paolo Bonzini, Pratik R. Sampat, Reinette Chatre,
Rick Edgecombe, Roger Wang, Ryan Afranji, Sagi Shahar,
Sean Christopherson, Shuah Khan, Oliver Upton,
Jeremiah McReynolds, kvm, linux-coco, linux-kernel, x86
In-Reply-To: <20260521-tdx-selftests-v13-v13-20-6983ae4c3a4d@google.com>
On 5/22/2026 7:17 AM, Lisa Wang wrote:
> From: Erdem Aktas <erdemaktas@google.com>
>
> Implement the tdx_mmio_write() to allow TDX VMs to request MMIO
> emulation.
>
> Follow the Intel Guest-Hypervisor Communication Interface (GHCI) spec
> to the minimum extent that a spec-abiding TDX module will pass the
> request to KVM. Skip implementing the #VE handler as described in the
> GHCI spec so selftests will not take a dependency on having a working
^
Something was cut off?
>
> To perform emulated I/O, VMs use the TDG.VP.VMCALL instruction to
> request MMIO.
>
> Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> Co-developed-by: Sagi Shahar <sagis@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> Co-developed-by: Lisa Wang <wyihan@google.com>
> Signed-off-by: Lisa Wang <wyihan@google.com>
> ---
> tools/testing/selftests/kvm/Makefile.kvm | 1 +
> tools/testing/selftests/kvm/include/x86/tdx/tdx.h | 16 ++++++++++++
> tools/testing/selftests/kvm/lib/x86/tdx/tdx.c | 30 +++++++++++++++++++++++
> 3 files changed, 47 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
> index a651a876c522..489324cecf83 100644
> --- a/tools/testing/selftests/kvm/Makefile.kvm
> +++ b/tools/testing/selftests/kvm/Makefile.kvm
> @@ -33,6 +33,7 @@ LIBKVM_x86 += lib/x86/ucall.c
> LIBKVM_x86 += lib/x86/vmx.c
> LIBKVM_x86 += lib/x86/tdx/tdx_util.c
> LIBKVM_x86 += lib/x86/tdx/td_boot.S
> +LIBKVM_x86 += lib/x86/tdx/tdx.c
>
> LIBKVM_arm64 += lib/arm64/gic.c
> LIBKVM_arm64 += lib/arm64/gic_v3.c
> diff --git a/tools/testing/selftests/kvm/include/x86/tdx/tdx.h b/tools/testing/selftests/kvm/include/x86/tdx/tdx.h
> new file mode 100644
> index 000000000000..810ca7423c84
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/include/x86/tdx/tdx.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef SELFTESTS_TDX_TDX_H
> +#define SELFTESTS_TDX_TDX_H
Nit:
The headers in tools/testing/selftests/kvm use SELFTEST_KVM_XXX.
> +
> +#include <linux/types.h>
> +
> +enum mmio_size {
> + MMIO_SIZE_1B = 1,
> + MMIO_SIZE_2B = 2,
> + MMIO_SIZE_4B = 4,
> + MMIO_SIZE_8B = 8
> +};
> +
> +u64 tdx_mmio_write(u64 address, enum mmio_size size, u64 data_in);
> +
> +#endif // SELFTESTS_TDX_TDX_H
> diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx.c
> new file mode 100644
> index 000000000000..f19be79fe11f
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include "tdx/tdx.h"
> +
> +#define TDG_VP_VMCALL 0
> +#define TDG_VP_VMCALL_VE_REQUEST_MMIO 48
> +#define TDVMCALL_MMIO_WRITE 1
> +#define TDVMCALL_EXPOSE_REGS_MASK 0xFC00
> +
> +u64 tdx_mmio_write(u64 address, enum mmio_size size, u64 data_in)
> +{
> + register u64 r10_reg asm("r10") = TDG_VP_VMCALL;
I think this should just be 0 instead of TDG_VP_VMCALL, although
TDG_VP_VMCALL is also 0.
Per GHCI spec about R10:
: Set to 0 indicates that TDG.VP.VMCALL leaf used in R11 is defined
: in this specification.
: All other values 0x1 to 0xFFFFFFFFFFFFFFFF indicate TDG.VP.VMCALL
: is vendor-specific (both R10 and R11).
> + register u64 r11_reg asm("r11") = TDG_VP_VMCALL_VE_REQUEST_MMIO;
> + register u64 r12_reg asm("r12") = size;
> + register u64 r13_reg asm("r13") = TDVMCALL_MMIO_WRITE;
> + register u64 r14_reg asm("r14") = address;
> + register u64 r15_reg asm("r15") = data_in;
> + register u64 rax_reg asm("rax") = TDG_VP_VMCALL;
> + register u64 rcx_reg asm("rcx") = TDVMCALL_EXPOSE_REGS_MASK;
> +
> + asm volatile(
> + ".byte 0x66,0x0f,0x01,0xcc" /* tdcall */
> + : "+r" (r10_reg), "+r" (r11_reg)
> + : "r" (r12_reg), "r" (r13_reg), "r" (r14_reg), "r" (r15_reg),
> + "r" (rax_reg), "r" (rcx_reg)
> + : "cc", "memory"
> + );
> +
> + return r10_reg;
> +}
>
^ permalink raw reply
* Re: [PATCH v13 16/22] KVM: selftests: Load per-vCPU guest stack in TDX boot parameters
From: Binbin Wu @ 2026-06-09 5:37 UTC (permalink / raw)
To: Lisa Wang
Cc: Andrew Jones, Ackerley Tng, Chao Gao, Chenyi Qiang, Dave Hansen,
Erdem Aktas, Ira Weiny, Isaku Yamahata, Kiryl Shutsemau,
linux-kselftest, Paolo Bonzini, Pratik R. Sampat, Reinette Chatre,
Rick Edgecombe, Roger Wang, Ryan Afranji, Sagi Shahar,
Sean Christopherson, Shuah Khan, Oliver Upton,
Jeremiah McReynolds, kvm, linux-coco, linux-kernel, x86
In-Reply-To: <20260521-tdx-selftests-v13-v13-16-6983ae4c3a4d@google.com>
On 5/22/2026 7:16 AM, Lisa Wang wrote:
> From: Sagi Shahar <sagis@google.com>
>
> Allocate a guest stack for each vCPU and record the GVA in the TDX boot
> parameters region to allow proper vCPU initialization.
>
> Co-developed-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> Signed-off-by: Lisa Wang <wyihan@google.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
One nit below.
[...]>
> +void tdx_vcpu_load_boot_parameters(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
> +{
> + struct td_boot_parameters *params =
> + addr_gpa2hva(vm, TD_BOOT_PARAMETERS_GPA);
> + struct td_per_vcpu_parameters *vcpu_params =
> + ¶ms->per_vcpu[vcpu->id];
> +
> + vcpu_params->esp_gva = kvm_allocate_vcpu_stack(vm);
> +}
> +
> +
An extra empty line.
> static struct kvm_tdx_capabilities *tdx_read_capabilities(struct kvm_vm *vm)
> {
> struct kvm_tdx_capabilities *tdx_cap = NULL;
>
^ permalink raw reply
* Re: [PATCH v4 10/47] x86/tsc: Consolidate forcing of X86_FEATURE_TSC_KNOWN_FREQ for PV code
From: Sean Christopherson @ 2026-06-08 22:38 UTC (permalink / raw)
To: David Woodhouse
Cc: Thomas Gleixner, Paolo Bonzini, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, Kiryl Shutsemau, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz, H. Peter Anvin,
Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, kvm, linux-kernel, linux-coco, linux-hyperv,
virtualization, xen-devel, Tom Lendacky, Nikunj A Dadhania,
Michael Kelley
In-Reply-To: <eef867eae15e30d08482ba16a1a32159745b64a7.camel@infradead.org>
On Sat, Jun 06, 2026, David Woodhouse wrote:
> On Sat, 2026-06-06 at 12:34 +0200, Thomas Gleixner wrote:
> > On Fri, May 29 2026 at 07:43, Sean Christopherson wrote:
> >
> > > Now that all paravirt code that explicitly specifies the TSC frequency
> > > also sets X86_FEATURE_TSC_KNOWN_FREQ, replace all of the one-off code
> > > and simply set X86_FEATURE_TSC_KNOWN_FREQ if the TSC frequency is known.
> > >
> > > Do NOT force set TSC_KNOWN_FREQ if the "known" TSC frequency was provided
> > > by the user. Per commit bd35c77e32e4 ("x86/tsc: Add tsc_early_khz command
> > > line parameter"), one of the goals of the param is to allow the refined
> > > calibration work "to do meaningful error checking".
> > >
> > > Note, preferring the user-provided TSC frequency over the frequency from
> > > the hypervisor or trusted firmware, while simultaneously not treating the
> > > user-provided frequency as gospel, is obviously incongruous. Sweep the
> > > problem under the rug for now to avoid opening a big can of worms that
> > > likely doesn't have a great answer.
> >
> > There is a good answer I think.
> >
> > early_tsc_khz exists to cater for the overclocking crowd. On their
> > modded systems the firmware supplied TSC frequency (CPUID/MSR) is not
> > matching reality anymore. So they work around that by supplying a close
> > enough tsc_early_khz and then they let the refined calibration work
> > figure it out.
> >
> > Arguably that's only relevant for bare metal systems and what's worse is
> > that in virtual environments the refined calibration work can fail,
> > which renders the TSC unstable.
> >
> > So I'd rather say we change this logic to:
> >
> > if (!hypervisor_is_type(X86_HYPER_NATIVE)) {
> > tsc_khz = x86_init.....();
> > force(X86_FEATURE_TSC_KNOWN_FREQ);
> > } else if (tsc_khz_early) {
> > ....
> > } else {
> > ...
> > }
> >
> > Along with:
> >
> > if (!hypervisor_is_type(X86_HYPER_NATIVE)) {
> > if (tsc_khz_early)
> > pr_warn("Ignoring non-sensical tsc_early_khz command line argument\n");
> >
> > or something daft like that.
Ya, I ended up in the same place once Sashiko pointed out that skipping the SNP/TDX
setup was hazardous[*], and also once I realized that tsc_khz_early *complemented*
the refinement instead of replacing it.
This is what I have locally:
if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
known_tsc_khz = snp_secure_tsc_init();
else if (boot_cpu_has(X86_FEATURE_TDX_GUEST))
known_tsc_khz = tdx_tsc_init();
/*
* If the TSC frequency wasn't provided by trusted firmware, try to get
* it from the hypervisor (which is untrusted when running as a CoCo guest).
*/
if (!known_tsc_khz && x86_init.hyper.get_tsc_khz)
known_tsc_khz = x86_init.hyper.get_tsc_khz();
/*
* Mark the TSC frequency as known if it was obtained from a hypervisor
* or trusted firmware. Don't mark the frequency as known if the user
* specified the frequency, as the user-provided frequency is intended
* as a "starting point", not a known, guaranteed frequency.
*/
if (known_tsc_khz && !tsc_early_khz)
setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
/*
* Ignore the user-provided TSC frequency if the exact frequency was
* obtained from trusted firmware or the hypervisor, as the user-
* provided frequency is intended as a "starting point", not a known,
* guaranteed frequency.
*/
if (!known_tsc_khz)
known_tsc_khz = tsc_early_khz;
else if (tsc_early_khz)
pr_err("Ignoring 'tsc_early_khz' in favor of firmware/hypervisor.\n");
[*] https://lore.kernel.org/all/ahnF-FehodVd474X@google.com
> > The kernel has for various reasons always tried to cater for the needs
> > of users who are plagued by bonkers firmware, but we have to stop to
> > prioritize or treating equal ancient and modded out of spec hardware.
> >
> > TBH, I consider that whole KVM clock nonsense to fall into the modded
> > out of spec hardware realm. Do a reality check:
> >
> > How many production systems are out there still which run VMs on CPUs
> > with a broken TSC and the lack of VM TSC scaling?
> >
> > I'm not saying that we should not support the few remaining systems
> > anymore, but our tendency to pretend that we can keep all of this
> > nonsense working and at the same time making progress is just a fallacy.
FWIW, I have the exact same sentiments about kvmclock, but I'm also trying my
best not to break folks that are happily running on what is effectively flawed,
ancient "hardward".
> I don't know that we can take the KVM (and Xen) clock away from guests,
> but all of the *horrid* part about it is the way it attempts to cope
> with the possibility that the *host* timekeeping might flip away from
> TSC-based mode at any point in time. By the end of my outstanding
> cleanup series, that is the *only* thing the gtod_notifier remains for.
>
> If we can trust the hardware *and* the host kernel, then KVM could
> theoretically hardwire the kvmclock into 'master clock mode' where it
> basically just advertises the TSC→kvmclock relationship *once* to all
> CPUs and it never changes.
>
> All the nonsense about updating it every time we enter a CPU could just
> go away completely.
But to Thomas' point, why bother? For actual old hardware, kvmclock is what it
is. For modern hardware, it's completely antiquated.
^ permalink raw reply
* Re: [PATCH v5 5/5] iommufd/vdevice: add TSM request ioctl
From: Dan Williams (nvidia) @ 2026-06-08 20:58 UTC (permalink / raw)
To: Aneesh Kumar K.V, Dan Williams (nvidia), Dan Williams (nvidia),
Alexey Kardashevskiy, linux-coco, iommu, linux-kernel, kvm
Cc: Bjorn Helgaas, Dan Williams, Jason Gunthorpe, Joerg Roedel,
Jonathan Cameron, Kevin Tian, Nicolin Chen, Samuel Ortiz,
Steven Price, Suzuki K Poulose, Will Deacon, Xu Yilun,
Shameer Kolothum, Paolo Bonzini, Tony Krowiak, Halil Pasic,
Jason Herne, Harald Freudenberger, Holger Dengler, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Alex Williamson, Matthew Rosato, Farhan Ali,
Eric Farman, linux-s390
In-Reply-To: <yq5aik81sf22.fsf@kernel.org>
Aneesh Kumar K.V wrote:
[..]
> > I think we can wait to move it to its own IOMMU operation unless/until
> > there is a need to set RUN outside of an explicit guest request, right?
>
> Something like the below? (the diff against this series)
>
> I have not yet integrated this into the full CCA patchset for testing,
> but I wanted to make sure we are aligned on the UAPI.
[..]
> -static bool iommufd_vdevice_tsm_req_scope_valid(u32 scope)
> +static bool iommufd_vdevice_tsm_req_arch_valid(u32 tvm_arch)
> {
> - if (scope > IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_LAST)
> + switch (tvm_arch) {
> + case IOMMU_VDEVICE_TSM_TVM_ARCH_CCA:
> + case IOMMU_VDEVICE_TSM_TVM_ARCH_SEV:
> + case IOMMU_VDEVICE_TSM_TVM_ARCH_TDX:
Makes sense for any command that needs tunneling. However, see below, what is
that set, and do we need a IOMMU_VDEVICE_TSM_COMMON when architecture
differentiation is not required?
> + return true;
> + default:
> return false;
> + }
> +}
>
> - switch (scope) {
> - case IOMMU_VDEVICE_TSM_REQ_PCI_INFO:
> - case IOMMU_VDEVICE_TSM_REQ_PCI_STATE_CHANGE:
> - case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_READ:
> - case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_WRITE:
> +static bool iommufd_vdevice_tsm_req_op_valid(u32 op, u32 tvm_arch)
> +{
> + switch (op) {
> + case TSM_REQ_READ_OBJECT:
> + case TSM_REQ_REGEN_OBJECT:
> + case TSM_REQ_OBJECT_INFO:
The design goal of the netlink device-evidence interface is to be able
to respond to all shapes of requests for evidence. So netlink caches
objects that the hypercall handler can fill responses from.
It eliminates a class of commands that need tunneling.
> + case TSM_REQ_VALIDATE_MMIO:
> + case TSM_REQ_SET_TDI_STATE:
Are these potentially candidates for a IOMMU_VDEVICE_TSM_COMMON? The
handler knows how to do the arch-specific response from the common
iommufd result, or is there TSM-specific payload beyond @tsm_code for
these.
Make it the case that guest_req only needs non-common arch for
operations that are implementation unique, or where the response payload
exceeds what can be conveyed via @tsm_code.
> return true;
> + case TSM_REQ_SEV_ENABLE_DMA:
> + case TSM_REQ_SEV_DISABLE_DMA:
> + return tvm_arch == IOMMU_VDEVICE_TSM_TVM_ARCH_SEV;
Right, this appears to be the only case where the command is
implementation unique. The handler can only ask iommufd to take
arch-specific action.
^ permalink raw reply
* [PATCH v7 6/6] x86/sev: Add debugfs support for RMPOPT
From: Ashish Kalra @ 2026-06-08 18:57 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen,
darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <cover.1780903370.git.ashish.kalra@amd.com>
From: Ashish Kalra <ashish.kalra@amd.com>
Add a debugfs interface to report per-CPU RMPOPT status across all
system RAM.
To dump the per-CPU RMPOPT status for all system RAM:
/sys/kernel/debug/rmpopt# cat rmpopt-table
Memory @ 0GB: CPU(s): none
Memory @ 1GB: CPU(s): none
Memory @ 2GB: CPU(s): 0-1023
Memory @ 3GB: CPU(s): 0-1023
Memory @ 4GB: CPU(s): none
Memory @ 5GB: CPU(s): 0-1023
Memory @ 6GB: CPU(s): 0-1023
Memory @ 7GB: CPU(s): 0-1023
...
Memory @1025GB: CPU(s): 0-1023
Memory @1026GB: CPU(s): 0-1023
Memory @1027GB: CPU(s): 0-1023
Memory @1028GB: CPU(s): 0-1023
Memory @1029GB: CPU(s): 0-1023
Memory @1030GB: CPU(s): 0-1023
Memory @1031GB: CPU(s): 0-1023
Memory @1032GB: CPU(s): 0-1023
Memory @1033GB: CPU(s): 0-1023
Memory @1034GB: CPU(s): 0-1023
Memory @1035GB: CPU(s): 0-1023
Memory @1036GB: CPU(s): 0-1023
Memory @1037GB: CPU(s): 0-1023
Memory @1038GB: CPU(s): none
Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/virt/svm/sev.c | 128 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 128 insertions(+)
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index db2d4c1f5dd7..fe45a333df92 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -20,6 +20,8 @@
#include <linux/amd-iommu.h>
#include <linux/nospec.h>
#include <linux/workqueue.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
#include <asm/sev.h>
#include <asm/processor.h>
@@ -145,6 +147,15 @@ static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
static unsigned long snp_nr_leaked_pages;
+/* All users of rmpopt_report_cpumask must hold rmpopt_show_mutex. */
+static cpumask_t rmpopt_report_cpumask;
+static struct dentry *rmpopt_debugfs;
+static DEFINE_MUTEX(rmpopt_show_mutex);
+
+struct seq_paddr {
+ phys_addr_t next_seq_paddr;
+};
+
#undef pr_fmt
#define pr_fmt(fmt) "SEV-SNP: " fmt
@@ -587,6 +598,8 @@ static void rmpopt_cleanup(void)
cancel_delayed_work_sync(&rmpopt_delayed_work);
destroy_workqueue(rmpopt_wq);
+ debugfs_remove_recursive(rmpopt_debugfs);
+ rmpopt_debugfs = NULL;
cpus_read_lock();
@@ -635,6 +648,10 @@ static inline bool __rmpopt(u64 pa_start, u64 op_type)
: "a" (pa_start), "c" (op_type)
: "memory", "cc");
+ if (op_type == RMPOPT_FUNC_REPORT_STATUS)
+ assign_cpu(smp_processor_id(), &rmpopt_report_cpumask,
+ optimized);
+
return optimized;
}
@@ -654,6 +671,115 @@ static void rmpopt_smp(void *val)
rmpopt((u64)val);
}
+/*
+ * 'val' is a system physical address.
+ */
+static void rmpopt_report_status(void *val)
+{
+ u64 pa_start = ALIGN_DOWN((u64)val, SZ_1G);
+ u64 op_type = RMPOPT_FUNC_REPORT_STATUS;
+
+ __rmpopt(pa_start, op_type);
+}
+
+/*
+ * start() can be called multiple times if allocated buffer has overflowed
+ * and bigger buffer is allocated.
+ */
+static void *rmpopt_table_seq_start(struct seq_file *seq, loff_t *pos)
+{
+ phys_addr_t end_paddr = rmpopt_pa_end;
+ struct seq_paddr *p = seq->private;
+
+ if (*pos == 0) {
+ p->next_seq_paddr = rmpopt_pa_start;
+ if (p->next_seq_paddr >= end_paddr)
+ return NULL;
+ return &p->next_seq_paddr;
+ }
+
+ if (p->next_seq_paddr >= end_paddr)
+ return NULL;
+
+ return &p->next_seq_paddr;
+}
+
+static void *rmpopt_table_seq_next(struct seq_file *seq, void *v, loff_t *pos)
+{
+ phys_addr_t end_paddr = rmpopt_pa_end;
+ phys_addr_t *curr_paddr = v;
+
+ (*pos)++;
+ *curr_paddr += SZ_1G;
+ if (*curr_paddr >= end_paddr)
+ return NULL;
+
+ return curr_paddr;
+}
+
+static void rmpopt_table_seq_stop(struct seq_file *seq, void *v)
+{
+}
+
+static int rmpopt_table_seq_show(struct seq_file *seq, void *v)
+{
+ phys_addr_t *curr_paddr = v;
+
+ guard(mutex)(&rmpopt_show_mutex);
+
+ seq_printf(seq, "Memory @%3lluGB: ",
+ *curr_paddr >> (get_order(SZ_1G) + PAGE_SHIFT));
+
+ /*
+ * Query all online CPUs rather than just rmpopt_cpumask (primary
+ * threads only). The RMPOPT instruction only needs to run on one
+ * thread per core for the optimization to take effect, but debugfs
+ * reporting requires the RMPOPT status across all CPUs.
+ * Performance is not a concern for this diagnostic interface.
+ *
+ * This is safe because RMPOPT_BASE MSR is per-core and
+ * snp_prepare() ensures all CPUs are online when the MSR is
+ * programmed during snp_setup_rmpopt().
+ */
+ cpumask_clear(&rmpopt_report_cpumask);
+ on_each_cpu_mask(cpu_online_mask, rmpopt_report_status,
+ (void *)*curr_paddr, true);
+
+ if (cpumask_empty(&rmpopt_report_cpumask))
+ seq_puts(seq, "CPU(s): none\n");
+ else
+ seq_printf(seq, "CPU(s): %*pbl\n", cpumask_pr_args(&rmpopt_report_cpumask));
+
+ return 0;
+}
+
+static const struct seq_operations rmpopt_table_seq_ops = {
+ .start = rmpopt_table_seq_start,
+ .next = rmpopt_table_seq_next,
+ .stop = rmpopt_table_seq_stop,
+ .show = rmpopt_table_seq_show
+};
+
+static int rmpopt_table_open(struct inode *inode, struct file *file)
+{
+ return seq_open_private(file, &rmpopt_table_seq_ops, sizeof(struct seq_paddr));
+}
+
+static const struct file_operations rmpopt_table_fops = {
+ .open = rmpopt_table_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = seq_release_private,
+};
+
+static void rmpopt_debugfs_setup(void)
+{
+ rmpopt_debugfs = debugfs_create_dir("rmpopt", arch_debugfs_dir);
+
+ debugfs_create_file("rmpopt-table", 0400, rmpopt_debugfs,
+ NULL, &rmpopt_table_fops);
+}
+
/*
* RMPOPT optimizations skip RMP checks at 1GB granularity if this
* range of memory does not contain any SNP guest memory.
@@ -852,6 +978,8 @@ void snp_setup_rmpopt(void)
* optimizations on all physical memory.
*/
queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work, 0);
+
+ rmpopt_debugfs_setup();
}
EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
--
2.43.0
^ permalink raw reply related
* [PATCH v7 5/6] KVM: SEV: Perform RMP optimizations on SNP guest shutdown
From: Ashish Kalra @ 2026-06-08 18:57 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen,
darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <cover.1780903370.git.ashish.kalra@amd.com>
From: Ashish Kalra <ashish.kalra@amd.com>
Pages are converted from shared to private as SNP guests are launched.
This destroys exisiting RMPOPT optimizations in the regions where
pages are converted.
Conversely, guest pages are converted back to shared during SNP guest
termination and their region may become eligible for RMPOPT
optimization.
To take advantage of this, perform RMPOPT after guest termination.
Do it after a delay so that a single RMPOPT pass can be done if
multiple guests terminate in a short period of time.
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/kvm/svm/sev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index e107f368ed2d..29af6f6e603c 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3005,6 +3005,8 @@ void sev_vm_destroy(struct kvm *kvm)
*/
if (snp_decommission_context(kvm))
return;
+
+ snp_rmpopt_all_physmem();
} else {
sev_unbind_asid(kvm, sev->handle);
}
--
2.43.0
^ permalink raw reply related
* [PATCH v7 4/6] x86/sev: Add interface to re-enable RMP optimizations.
From: Ashish Kalra @ 2026-06-08 18:57 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen,
darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <cover.1780903370.git.ashish.kalra@amd.com>
From: Ashish Kalra <ashish.kalra@amd.com>
RMPOPT table is a per-CPU table which indicates if 1GB regions of
physical memory are entirely hypervisor-owned or not.
When performing host memory accesses in hypervisor mode as well as
non-SNP guest mode, the processor may consult the RMPOPT table to
potentially skip an RMP access and improve performance.
Events such as RMPUPDATE can clear RMP optimizations. Add an interface
to re-enable those optimizations.
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/include/asm/sev.h | 2 ++
arch/x86/virt/svm/sev.c | 15 +++++++++++++++
2 files changed, 17 insertions(+)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 0d662221615a..a11306f25336 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -662,6 +662,7 @@ static inline void snp_leak_pages(u64 pfn, unsigned int pages)
__snp_leak_pages(pfn, pages, true);
}
int snp_prepare(void);
+void snp_rmpopt_all_physmem(void);
void snp_setup_rmpopt(void);
void snp_clear_rmpopt_configured(void);
void snp_shutdown(void);
@@ -682,6 +683,7 @@ static inline void snp_leak_pages(u64 pfn, unsigned int npages) {}
static inline void kdump_sev_callback(void) { }
static inline void snp_fixup_e820_tables(void) {}
static inline int snp_prepare(void) { return -ENODEV; }
+static inline void snp_rmpopt_all_physmem(void) {}
static inline void snp_setup_rmpopt(void) {}
static inline void snp_clear_rmpopt_configured(void) {}
static inline void snp_shutdown(void) {}
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index b42788a66d40..db2d4c1f5dd7 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -760,6 +760,21 @@ static void rmpopt_work_handler(struct work_struct *work)
free_cpumask_var(follower_mask);
}
+void snp_rmpopt_all_physmem(void)
+{
+ if (!cpu_feature_enabled(X86_FEATURE_RMPOPT) || !rmpopt_configured)
+ return;
+
+ guard(mutex)(&rmpopt_wq_mutex);
+
+ if (!rmpopt_wq)
+ return;
+
+ queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work,
+ msecs_to_jiffies(RMPOPT_WORK_TIMEOUT));
+}
+EXPORT_SYMBOL_GPL(snp_rmpopt_all_physmem);
+
void snp_setup_rmpopt(void)
{
u64 rmpopt_base;
--
2.43.0
^ permalink raw reply related
* [PATCH v7 3/6] x86/sev: Add support to perform RMP optimizations asynchronously
From: Ashish Kalra @ 2026-06-08 18:56 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen,
darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <cover.1780903370.git.ashish.kalra@amd.com>
From: Ashish Kalra <ashish.kalra@amd.com>
When SEV-SNP is enabled, all writes to memory are checked to ensure
integrity of SNP guest memory. This imposes performance overhead on the
whole system.
RMPOPT is a new instruction that minimizes the performance overhead of
RMP checks on the hypervisor and on non-SNP guests by allowing RMP
checks to be skipped for 1GB regions of memory that are known not to
contain any SEV-SNP guest memory.
Add support for performing RMP optimizations asynchronously using a
dedicated workqueue.
Enable RMPOPT optimizations for up to 2TB of system RAM starting from
the lowest physical memory address aligned down to a 1GB boundary at
RMP initialization time. RMP checks can initially be skipped for 1GB
memory ranges that do not contain SEV-SNP guest memory (excluding
preassigned pages such as the RMP table and firmware pages). As SNP
guests are launched, RMPUPDATE will disable the corresponding RMPOPT
optimizations.
Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/virt/svm/sev.c | 208 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 205 insertions(+), 3 deletions(-)
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index 482008bb07e4..b42788a66d40 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -19,6 +19,7 @@
#include <linux/iommu.h>
#include <linux/amd-iommu.h>
#include <linux/nospec.h>
+#include <linux/workqueue.h>
#include <asm/sev.h>
#include <asm/processor.h>
@@ -125,9 +126,20 @@ static void *rmp_bookkeeping __ro_after_init;
static u64 probed_rmp_base, probed_rmp_size;
static cpumask_t rmpopt_cpumask;
-static phys_addr_t rmpopt_pa_start;
+static phys_addr_t rmpopt_pa_start, rmpopt_pa_end;
static bool rmpopt_configured;
+enum rmpopt_function {
+ RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS,
+ RMPOPT_FUNC_REPORT_STATUS
+};
+
+#define RMPOPT_WORK_TIMEOUT 10000
+
+static struct workqueue_struct *rmpopt_wq;
+static struct delayed_work rmpopt_delayed_work;
+static DEFINE_MUTEX(rmpopt_wq_mutex);
+
static LIST_HEAD(snp_leaked_pages_list);
static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
@@ -568,6 +580,14 @@ static void rmpopt_cleanup(void)
{
int cpu;
+ guard(mutex)(&rmpopt_wq_mutex);
+
+ if (!rmpopt_wq)
+ return;
+
+ cancel_delayed_work_sync(&rmpopt_delayed_work);
+ destroy_workqueue(rmpopt_wq);
+
cpus_read_lock();
for_each_cpu(cpu, &rmpopt_cpumask)
@@ -576,7 +596,8 @@ static void rmpopt_cleanup(void)
cpus_read_unlock();
cpumask_clear(&rmpopt_cpumask);
- rmpopt_pa_start = 0;
+ rmpopt_pa_start = rmpopt_pa_end = 0;
+ rmpopt_wq = NULL;
}
void snp_shutdown(void)
@@ -599,6 +620,146 @@ void snp_clear_rmpopt_configured(void)
rmpopt_configured = false;
}
+/*
+ * RMPOPT: F2 0F 01 FC
+ * Input: RAX = system physical address (1GB aligned)
+ * RCX = operation type
+ * Output: CF set if the range was optimized
+ */
+static inline bool __rmpopt(u64 pa_start, u64 op_type)
+{
+ bool optimized;
+
+ asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
+ : "=@ccc" (optimized)
+ : "a" (pa_start), "c" (op_type)
+ : "memory", "cc");
+
+ return optimized;
+}
+
+static void rmpopt(u64 pa)
+{
+ u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
+ u64 op_type = RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS;
+
+ __rmpopt(pa_start, op_type);
+}
+
+/*
+ * 'val' is a system physical address.
+ */
+static void rmpopt_smp(void *val)
+{
+ rmpopt((u64)val);
+}
+
+/*
+ * RMPOPT optimizations skip RMP checks at 1GB granularity if this
+ * range of memory does not contain any SNP guest memory.
+ */
+static void rmpopt_work_handler(struct work_struct *work)
+{
+ cpumask_var_t follower_mask;
+ phys_addr_t pa;
+ int this_cpu;
+
+ pr_info("Attempt RMP optimizations on physical address range @1GB alignment [0x%016llx - 0x%016llx]\n",
+ rmpopt_pa_start, rmpopt_pa_end);
+
+ if (!alloc_cpumask_var(&follower_mask, GFP_KERNEL))
+ return;
+
+ /*
+ * RMPOPT scans the RMP table, stores the result of the scan in the
+ * reserved processor memory. The RMP scan is the most expensive
+ * part. If a second RMPOPT occurs, it can skip the expensive scan
+ * if they can see a cached result in the reserved processor memory.
+ *
+ * Do RMPOPT on one CPU alone. Then, follow that up with RMPOPT
+ * on every other primary thread. Followers are "designed to"
+ * skip the scan if they see the "cached" scan results.
+ */
+ cpumask_copy(follower_mask, &rmpopt_cpumask);
+
+ /*
+ * Pin the worker to the current CPU for the leader loop so that
+ * this_cpu remains valid and the RMPOPT instruction executes on
+ * the correct CPU.
+ *
+ * Use migrate_disable() rather than get_cpu() to prevent
+ * migration while still allowing preemption.
+ */
+ migrate_disable();
+ this_cpu = smp_processor_id();
+
+ if (cpumask_test_cpu(this_cpu, follower_mask)) {
+ /*
+ * Current CPU is a primary thread in rmpopt_cpumask.
+ * Run leader locally and remove from follower mask.
+ */
+ cpumask_clear_cpu(this_cpu, follower_mask);
+
+ for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G) {
+ rmpopt(pa);
+ cond_resched();
+ }
+ } else if (cpumask_intersects(topology_sibling_cpumask(this_cpu),
+ follower_mask)) {
+ /*
+ * Current CPU is a sibling thread whose primary is in
+ * rmpopt_cpumask. RMPOPT_BASE MSR is per-core, so it
+ * is safe to run the leader locally. Remove the sibling's
+ * primary from the follower mask as this core is already
+ * covered by the leader.
+ */
+ cpumask_andnot(follower_mask, follower_mask,
+ topology_sibling_cpumask(this_cpu));
+
+ for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G) {
+ rmpopt(pa);
+ cond_resched();
+ }
+ } else {
+ /*
+ * Current CPU does not have RMPOPT_BASE MSR programmed.
+ * Pick an explicit leader from the cpumask to avoid #UD.
+ */
+ int leader_cpu = cpumask_first(follower_mask);
+
+ if (WARN_ON_ONCE(leader_cpu >= nr_cpu_ids)) {
+ migrate_enable();
+ goto out;
+ }
+
+ cpumask_clear_cpu(leader_cpu, follower_mask);
+
+ cpus_read_lock();
+ for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G) {
+ smp_call_function_single(leader_cpu, rmpopt_smp,
+ (void *)pa, true);
+ cond_resched();
+ }
+ cpus_read_unlock();
+ }
+
+ migrate_enable();
+
+ /* Followers: run RMPOPT on remaining cores */
+ cpus_read_lock();
+ for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G) {
+ on_each_cpu_mask(follower_mask, rmpopt_smp,
+ (void *)pa, true);
+
+ /* Give a chance for other threads to run */
+ cond_resched();
+ }
+ cpus_read_unlock();
+
+out:
+ free_cpumask_var(follower_mask);
+}
+
void snp_setup_rmpopt(void)
{
u64 rmpopt_base;
@@ -607,11 +768,35 @@ void snp_setup_rmpopt(void)
if (!cpu_feature_enabled(X86_FEATURE_RMPOPT) || !rmpopt_configured)
return;
+ guard(mutex)(&rmpopt_wq_mutex);
+
+ /*
+ * Guard against re-initialization. When SNP_SHUTDOWN_EX is issued
+ * with x86_snp_shutdown=0, snp_shutdown() is not called and
+ * rmpopt_cleanup() is skipped, but snp_initialized is still cleared.
+ * A subsequent __sev_snp_init_locked() would call snp_setup_rmpopt()
+ * again, leaking the existing workqueue, delayed work, debugfs
+ * entries, and cpumask state.
+ */
+ if (rmpopt_wq)
+ return;
+
+ /*
+ * Create an RMPOPT-specific workqueue to avoid scheduling
+ * RMPOPT workitem on the global system workqueue.
+ */
+ rmpopt_wq = alloc_workqueue("rmpopt_wq", WQ_UNBOUND, 1);
+ if (!rmpopt_wq) {
+ pr_err("Failed to allocate RMPOPT workqueue\n");
+ return;
+ }
+
cpus_read_lock();
/*
* The RMPOPT_BASE MSR is per-core, so only one thread per core needs
- * to set up the RMPOPT_BASE MSR.
+ * to set up the RMPOPT_BASE MSR. Likewise, only one thread per core
+ * needs to issue the RMPOPT instruction.
*
* Note: only online primary threads are included. If a core's
* primary thread is offline, that core is not covered. CPU hotplug
@@ -635,6 +820,23 @@ void snp_setup_rmpopt(void)
wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
cpus_read_unlock();
+
+ INIT_DELAYED_WORK(&rmpopt_delayed_work, rmpopt_work_handler);
+
+ rmpopt_pa_end = ALIGN(PFN_PHYS(max_pfn), SZ_1G);
+
+ /* Limit memory scanning to 2TB of RAM */
+ if ((rmpopt_pa_end - rmpopt_pa_start) > SZ_2T) {
+ pr_info("RMPOPT coverage limited to 2TB; memory above 0x%llx not optimized\n",
+ rmpopt_pa_start + SZ_2T);
+ rmpopt_pa_end = rmpopt_pa_start + SZ_2T;
+ }
+
+ /*
+ * Once all per-CPU RMPOPT tables have been configured, enable RMPOPT
+ * optimizations on all physical memory.
+ */
+ queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work, 0);
}
EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
--
2.43.0
^ permalink raw reply related
* [PATCH v7 2/6] x86/sev: Initialize RMPOPT configuration MSRs
From: Ashish Kalra @ 2026-06-08 18:56 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen,
darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <cover.1780903370.git.ashish.kalra@amd.com>
From: Ashish Kalra <ashish.kalra@amd.com>
The new RMPOPT instruction helps manage per-CPU RMP optimization
structures inside the CPU. It takes a 1GB-aligned physical address
and either returns the status of the optimizations or tries to enable
the optimizations.
Per-CPU RMPOPT tables support at most 2 TB of addressable memory for
RMP optimizations.
Initialize the per-CPU RMPOPT table base to the starting physical
address. This enables RMP optimization for up to 2 TB of system RAM on
all CPUs.
Additionally, add support to setup and enable RMPOPT once SNP is
enabled and initialized.
Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/coco/core.c | 3 ++
arch/x86/include/asm/msr-index.h | 3 ++
arch/x86/include/asm/sev.h | 4 ++
arch/x86/virt/svm/sev.c | 72 +++++++++++++++++++++++++++++++-
drivers/crypto/ccp/sev-dev.c | 3 ++
5 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
index 989ca9f72ba3..a8fc2ae50298 100644
--- a/arch/x86/coco/core.c
+++ b/arch/x86/coco/core.c
@@ -16,6 +16,7 @@
#include <asm/archrandom.h>
#include <asm/coco.h>
#include <asm/processor.h>
+#include <asm/sev.h>
enum cc_vendor cc_vendor __ro_after_init = CC_VENDOR_NONE;
SYM_PIC_ALIAS(cc_vendor);
@@ -172,6 +173,8 @@ static void amd_cc_platform_clear(enum cc_attr attr)
switch (attr) {
case CC_ATTR_HOST_SEV_SNP:
cc_flags.host_sev_snp = 0;
+ snp_clear_rmpopt_configured();
+ setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
break;
default:
break;
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 86554de9a3f5..28540744f1eb 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -761,6 +761,9 @@
#define MSR_AMD64_SEG_RMP_ENABLED_BIT 0
#define MSR_AMD64_SEG_RMP_ENABLED BIT_ULL(MSR_AMD64_SEG_RMP_ENABLED_BIT)
#define MSR_AMD64_RMP_SEGMENT_SHIFT(x) (((x) & GENMASK_ULL(13, 8)) >> 8)
+#define MSR_AMD64_RMPOPT_BASE 0xc0010139
+#define MSR_AMD64_RMPOPT_ENABLE_BIT 0
+#define MSR_AMD64_RMPOPT_ENABLE BIT_ULL(MSR_AMD64_RMPOPT_ENABLE_BIT)
#define MSR_SVSM_CAA 0xc001f000
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 594cfa19cbd4..0d662221615a 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -662,6 +662,8 @@ static inline void snp_leak_pages(u64 pfn, unsigned int pages)
__snp_leak_pages(pfn, pages, true);
}
int snp_prepare(void);
+void snp_setup_rmpopt(void);
+void snp_clear_rmpopt_configured(void);
void snp_shutdown(void);
#else
static inline bool snp_probe_rmptable_info(void) { return false; }
@@ -680,6 +682,8 @@ static inline void snp_leak_pages(u64 pfn, unsigned int npages) {}
static inline void kdump_sev_callback(void) { }
static inline void snp_fixup_e820_tables(void) {}
static inline int snp_prepare(void) { return -ENODEV; }
+static inline void snp_setup_rmpopt(void) {}
+static inline void snp_clear_rmpopt_configured(void) {}
static inline void snp_shutdown(void) {}
#endif
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index 8bcdce98f6dc..482008bb07e4 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -124,6 +124,10 @@ static void *rmp_bookkeeping __ro_after_init;
static u64 probed_rmp_base, probed_rmp_size;
+static cpumask_t rmpopt_cpumask;
+static phys_addr_t rmpopt_pa_start;
+static bool rmpopt_configured;
+
static LIST_HEAD(snp_leaked_pages_list);
static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
@@ -488,9 +492,14 @@ static bool __init setup_segmented_rmptable(void)
static bool __init setup_rmptable(void)
{
if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED) {
- if (!setup_segmented_rmptable())
+ if (!setup_segmented_rmptable()) {
+ setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
return false;
+ }
+ rmpopt_configured = true;
} else {
+ /* Note that Segmented RMP must be enabled to enable RMPOPT. */
+ setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
if (!setup_contiguous_rmptable())
return false;
}
@@ -555,6 +564,21 @@ int snp_prepare(void)
}
EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
+static void rmpopt_cleanup(void)
+{
+ int cpu;
+
+ cpus_read_lock();
+
+ for_each_cpu(cpu, &rmpopt_cpumask)
+ wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
+
+ cpus_read_unlock();
+
+ cpumask_clear(&rmpopt_cpumask);
+ rmpopt_pa_start = 0;
+}
+
void snp_shutdown(void)
{
u64 syscfg;
@@ -563,11 +587,57 @@ void snp_shutdown(void)
if (syscfg & MSR_AMD64_SYSCFG_SNP_EN)
return;
+ rmpopt_cleanup();
+
clear_rmp();
on_each_cpu(mfd_reconfigure, NULL, 1);
}
EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
+void snp_clear_rmpopt_configured(void)
+{
+ rmpopt_configured = false;
+}
+
+void snp_setup_rmpopt(void)
+{
+ u64 rmpopt_base;
+ int cpu;
+
+ if (!cpu_feature_enabled(X86_FEATURE_RMPOPT) || !rmpopt_configured)
+ return;
+
+ cpus_read_lock();
+
+ /*
+ * The RMPOPT_BASE MSR is per-core, so only one thread per core needs
+ * to set up the RMPOPT_BASE MSR.
+ *
+ * Note: only online primary threads are included. If a core's
+ * primary thread is offline, that core is not covered. CPU hotplug
+ * is not currently supported with SNP enabled.
+ */
+
+ for_each_online_cpu(cpu)
+ if (topology_is_primary_thread(cpu))
+ cpumask_set_cpu(cpu, &rmpopt_cpumask);
+
+ rmpopt_pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), SZ_1G);
+ rmpopt_base = rmpopt_pa_start | MSR_AMD64_RMPOPT_ENABLE;
+
+ /*
+ * Per-CPU RMPOPT tables support at most 2 TB of addressable memory
+ * for RMP optimizations. Initialize the per-CPU RMPOPT table base
+ * to the starting physical address to enable RMP optimizations for
+ * up to 2 TB of system RAM on all CPUs.
+ */
+ for_each_cpu(cpu, &rmpopt_cpumask)
+ wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
+
+ cpus_read_unlock();
+}
+EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
+
/*
* Do the necessary preparations which are verified by the firmware as
* described in the SNP_INIT_EX firmware command description in the SNP
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 78f98aee7a66..217b6b19802e 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1478,6 +1478,9 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
}
snp_hv_fixed_pages_state_update(sev, HV_FIXED);
+
+ snp_setup_rmpopt();
+
sev->snp_initialized = true;
dev_dbg(sev->dev, "SEV-SNP firmware initialized, SEV-TIO is %s\n",
data.tio_en ? "enabled" : "disabled");
--
2.43.0
^ permalink raw reply related
* [PATCH v7 1/6] x86/cpufeatures: Add X86_FEATURE_RMPOPT feature flag
From: Ashish Kalra @ 2026-06-08 18:56 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen,
darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <cover.1780903370.git.ashish.kalra@amd.com>
From: Ashish Kalra <ashish.kalra@amd.com>
Add a flag indicating whether RMPOPT instruction is supported.
RMPOPT is a new instruction that reduces the performance overhead of
RMP checks for the hypervisor and non-SNP guests by allowing those
checks to be skipped when 1-GB memory regions are known to contain no
SEV-SNP guest memory.
For more information on the RMPOPT instruction, see the AMD64 RMPOPT
technical documentation.
Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/include/asm/cpufeatures.h | 2 +-
arch/x86/kernel/cpu/scattered.c | 1 +
tools/arch/x86/include/asm/cpufeatures.h | 2 +-
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 1d506e5d6f46..794cc96b8493 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -76,7 +76,7 @@
#define X86_FEATURE_K8 ( 3*32+ 4) /* Opteron, Athlon64 */
#define X86_FEATURE_ZEN5 ( 3*32+ 5) /* CPU based on Zen5 microarchitecture */
#define X86_FEATURE_ZEN6 ( 3*32+ 6) /* CPU based on Zen6 microarchitecture */
-/* Free ( 3*32+ 7) */
+#define X86_FEATURE_RMPOPT ( 3*32+ 7) /* Support for AMD RMPOPT instruction */
#define X86_FEATURE_CONSTANT_TSC ( 3*32+ 8) /* "constant_tsc" TSC ticks at a constant rate */
#define X86_FEATURE_UP ( 3*32+ 9) /* "up" SMP kernel running on UP */
#define X86_FEATURE_ART ( 3*32+10) /* "art" Always running timer (ART) */
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 937129ce6a96..021c0bf22de2 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -67,6 +67,7 @@ static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_PERFMON_V2, CPUID_EAX, 0, 0x80000022, 0 },
{ X86_FEATURE_AMD_LBR_V2, CPUID_EAX, 1, 0x80000022, 0 },
{ X86_FEATURE_AMD_LBR_PMC_FREEZE, CPUID_EAX, 2, 0x80000022, 0 },
+ { X86_FEATURE_RMPOPT, CPUID_EDX, 0, 0x80000025, 0 },
{ X86_FEATURE_AMD_HTR_CORES, CPUID_EAX, 30, 0x80000026, 0 },
{ 0, 0, 0, 0, 0 }
};
diff --git a/tools/arch/x86/include/asm/cpufeatures.h b/tools/arch/x86/include/asm/cpufeatures.h
index 86d17b195e79..7ce681af1dd7 100644
--- a/tools/arch/x86/include/asm/cpufeatures.h
+++ b/tools/arch/x86/include/asm/cpufeatures.h
@@ -76,7 +76,7 @@
#define X86_FEATURE_K8 ( 3*32+ 4) /* Opteron, Athlon64 */
#define X86_FEATURE_ZEN5 ( 3*32+ 5) /* CPU based on Zen5 microarchitecture */
#define X86_FEATURE_ZEN6 ( 3*32+ 6) /* CPU based on Zen6 microarchitecture */
-/* Free ( 3*32+ 7) */
+#define X86_FEATURE_RMPOPT ( 3*32+ 7) /* Support for AMD RMPOPT instruction */
#define X86_FEATURE_CONSTANT_TSC ( 3*32+ 8) /* "constant_tsc" TSC ticks at a constant rate */
#define X86_FEATURE_UP ( 3*32+ 9) /* "up" SMP kernel running on UP */
#define X86_FEATURE_ART ( 3*32+10) /* "art" Always running timer (ART) */
--
2.43.0
^ permalink raw reply related
* [PATCH v7 0/6] Add RMPOPT support.
From: Ashish Kalra @ 2026-06-08 18:56 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen,
darwi, linux-kernel, linux-crypto, kvm, linux-coco
From: Ashish Kalra <ashish.kalra@amd.com>
In the SEV-SNP architecture, hypervisor and non-SNP guests are subject
to RMP checks on writes to provide integrity of SEV-SNP guest memory.
The RMPOPT architecture enables optimizations whereby the RMP checks
can be skipped if 1GB regions of memory are known to not contain any
SNP guest memory.
RMPOPT is a new instruction designed to minimize the performance
overhead of RMP checks for the hypervisor and non-SNP guests.
RMPOPT instruction currently supports two functions. In case of the
verify and report status function the CPU will read the RMP contents,
verify the entire 1GB region starting at the provided SPA is HV-owned.
For the entire 1GB region it checks that all RMP entries in this region
are HV-owned (i.e, not in assigned state) and then accordingly updates
the RMPOPT table to indicate if optimization has been enabled and
provide indication to software if the optimization was successful.
In case of report status function, the CPU returns the optimization
status for the 1GB region.
The RMPOPT table is managed by a combination of software and hardware.
Software uses the RMPOPT instruction to set bits in the table,
indicating that regions of memory are entirely HV-owned. Hardware
automatically clears bits in the RMPOPT table when RMP contents are
changed during RMPUPDATE instruction.
For more information on the RMPOPT instruction, see the AMD64 RMPOPT
technical documentation.
As SNP is enabled by default the hypervisor and non-SNP guests are
subject to RMP write checks to provide integrity of SNP guest memory.
This patch-series adds support to enable RMP optimizations for up to
2TB of system RAM across the system and allow RMPUPDATE to disable
those optimizations as SNP guests are launched.
Support for RAM larger than 2 TB will be added in follow-on series.
This series also introduces support to re-enable RMP optimizations
during SNP guest termination, after guest pages have been converted
back to shared.
RMP optimizations are performed asynchronously by queuing work on a
dedicated workqueue after a 10 second delay.
Delaying work allows batching of multiple SNP guest terminations.
Once 1GB hugetlb guest_memfd support is merged, support for
re-enabling RMPOPT optimizations during 1GB page cleanup will be added
in follow-on series.
Additionally add debugfs interface to report per-CPU RMPOPT status
across all system RAM.
v7:
- Sync tools/arch/x86/include/asm/cpufeatures.h to mirror the kernel
header for X86_FEATURE_RMPOPT.
- Fix commit title to use X86_FEATURE_RMPOPT to match the code
(was X86_FEATURE_AMD_RMPOPT).
- Add static bool rmpopt_configured, set only when segmented RMP setup
succeeds in setup_rmptable(). Check rmpopt_configured alongside
cpu_feature_enabled(X86_FEATURE_RMPOPT) in snp_setup_rmpopt() and
snp_rmpopt_all_physmem(), because setup_clear_cpu_cap() is unreliable
after alternatives are patched. Add snp_clear_rmpopt_configured()
called from amd_cc_platform_clear() when CC_ATTR_HOST_SEV_SNP is
cleared. Do not use __ro_after_init on rmpopt_configured since the
writer snp_clear_rmpopt_configured() is not __init.
- Add cond_resched() to all three leader loops in rmpopt_work_handler()
to prevent soft lockups on systems with up to 2TB of RAM.
- Add comment above __rmpopt() documenting the RMPOPT instruction
encoding (F2 0F 01 FC) and register interface (RAX = system physical
address input, RCX = operation type input, RFLAGS.CF = output).
Note: RMPOPT does not modify RAX unlike PVALIDATE/RMPUPDATE, so
the existing "a" (input-only) constraint is correct.
Sashiko AI code review identified several of the above issues.
v6:
- Drop wrmsrq_on_cpus() helper; use for_each_cpu() with wrmsrq_on_cpu()
instead, as RMPOPT_BASE MSR programming is not performance-critical.
- Rewrite rmpopt_work_handler() leader selection to use a local
follower_mask copy instead of modifying the global rmpopt_cpumask.
This eliminates the current_cpu_cleared tracking and the restore at
the end, and removes the need for synchronization comments about
transient cpumask inconsistency.
- Add three-way leader selection in rmpopt_work_handler():
1. Current CPU is a primary thread in cpumask: run leader locally.
2. Current CPU is a sibling thread whose primary is in cpumask:
run leader locally (RMPOPT_BASE MSR is per-core), remove the
primary from followers via cpumask_andnot(topology_sibling_cpumask).
3. Current CPU's core has no RMPOPT_BASE MSR programmed: pick an
explicit leader via cpumask_first() + smp_call_function_single()
to avoid #UD, with cpus_read_lock() around the IPI loop.
- Add WARN_ON_ONCE guard for empty cpumask in the explicit leader
fallback path, with migrate_enable() before goto out.
- Add .llseek = seq_lseek to rmpopt_table_fops for consistency with
other seq_file-based debugfs files and to support tools like "less".
- Change debugfs file permissions from 0444 to 0400 to restrict access
to root only.
- Add comment in rmpopt_table_seq_show() explaining why cpu_online_mask
is safe: RMPOPT_BASE MSR is per-core and snp_prepare() ensures all
CPUs are online when the MSR is programmed.
Sashiko AI code review identified several of the above issues.
v5:
- Introduce rmpopt_cleanup() to tear down workqueue, debugfs, cpumask,
and MSR state, called from snp_shutdown().
- Introduce rmpopt_wq_mutex to serialize snp_setup_rmpopt(),
snp_rmpopt_all_physmem(), and rmpopt_cleanup().
- Introduce rmpopt_show_mutex to serialize debugfs reporting of
rmpopt_report_cpumask.
- Move snp_rmpopt_all_physmem() call after SNP DECOMMISSION during
guest shutdown.
- Use migrate_disable()/migrate_enable() for CPU pinning in the
rmpopt_work_handler() leader loop to maintain CPU affinity without
disabling preemption for the entire RMPOPT scan.
- Add cpus_read_lock()/cpus_read_unlock() around the follower
on_each_cpu_mask() loop in rmpopt_work_handler().
- Guard snp_setup_rmpopt() against re-initialization when
SNP_SHUTDOWN_EX with x86_snp_shutdown=0 skips rmpopt_cleanup()
but clears snp_initialized, preventing workqueue and resource
leaks on repeated init/shutdown cycles.
- Replace setup_clear_cpu_cap() with pr_err() on alloc_workqueue()
failure in snp_setup_rmpopt(), as setup_clear_cpu_cap() cannot be
used after alternatives are patched; callers check rmpopt_wq != NULL
as the runtime guard instead.
- Add pr_info() when RMPOPT coverage is capped at 2TB.
- Add comments noting CPU hotplug is not supported with SNP enabled
and only online primary threads are covered by rmpopt_cpumask.
- Add comment in setup_rmptable() noting Segmented RMP must be
enabled to enable RMPOPT.
- Simplify cpumask setup loop to set if primary thread rather than
skip if not primary.
- Improve grammar and clarity in snp_setup_rmpopt() comments.
- Added Reviewed-by's.
Sashiko AI code review identified several of the above issues.
v4:
- Add new wrmsrq_on_cpus() helper to write same u64 value to a
per-CPU MSR across a cpumask without per-cpu struct allocation
overhead.
- Rename configure_and_enable_rmpopt() to snp_setup_rmpopt().
- Use wrmsrq_on_cpus() instead of wrmsrq_on_cpu() loop for
programming RMPOPT_BASE MSRs.
- Add setup_clear_cpu_cap(X86_FEATURE_RMPOPT) if segmented RMP
setup fails or workqueue allocation fails.
- Add X86_FEATURE_RMPOPT feature clear logic in amd_cc_platform_clear()
for CC_ATTR_HOST_SEV_SNP.
- All of the above allow checking for only X86_FEATURE_RMPOPT for both
RMPOPT setup/enable and RMP re-optimizations.
- Rename snp_perform_rmp_optimization() to snp_rmpopt_all_physmem().
- Split rmpopt() into rmpopt() and rmpopt_smp() for SMP callback use.
- Introduce separate rmpopt_report_cpumask for debugfs reporting,
distinct from rmpopt_cpumask used for primary thread tracking.
- Remove snp_perform_rmp_optimization() call from __sev_snp_init_locked()
and instead setup and enable RMPOPT after SNP is enabled and
initialized.
v3:
- Drop all RMPOPT kthread support and introduce adding custom and
dedicated workqueue to schedule delayed and asynchronous RMPOPT work.
- Drop the guest_memfd inode cleanup interface and add support to
re-enable RMP optimizations during guest shutdown using the
asynchronous and delayed workqueue interface.
- Introduce new __rmpopt() helper and rmpopt() and
rmpopt_report_status() wrappers on top which use rax and rcx
parameters to closely match RMPOPT specs.
- Use new optimized RMPOPT loop to issue RMPOPT instructions on all
system RAM upto 2TB and all CPUs, by optimizing each range on one CPU
first, then let other CPUs execute RMPOPT in parallel so they can skip
most work as the range has already been optimized.
- Also add support for running the optimized RMPOPT loop only on
one thread per core.
- Replace all PUD_SIZE references with SZ_1G to conform to 1GB regions
as specified by RMPOPT specifications and not be dependent on PUD_SIZE
which makes the RMPOPT patch-set independent of x86 page table sizes.
- Use wrmsrq_on_cpu() to program the RMPOPT_BASE MSR registers on
all CPUs that removes all ugly casting to use on_each_cpu_mask().
- Fix inline commits and patch commit messages
v2:
- Drop all NUMA and Socket configuration and enablement support and
enable RMPOPT support for up to 2TB of system RAM.
- Drop get_cpumask_of_primary_threads() and enable per-core RMPOPT
base MSRs and issue RMPOPT instruction on all CPUs.
- Drop the configfs interface to manually re-enable RMP optimizations.
- Add new guest_memfd cleanup interface to automatically re-enable
RMP optimizations during guest shutdown.
- Include references to the public RMPOPT documentation.
- Move debugfs directory for RMPOPT under architecuture specific
parent directory.
Ashish Kalra (6):
x86/cpufeatures: Add X86_FEATURE_RMPOPT feature flag
x86/sev: Initialize RMPOPT configuration MSRs
x86/sev: Add support to perform RMP optimizations asynchronously
x86/sev: Add interface to re-enable RMP optimizations.
KVM: SEV: Perform RMP optimizations on SNP guest shutdown
x86/sev: Add debugfs support for RMPOPT
arch/x86/coco/core.c | 3 +
arch/x86/include/asm/cpufeatures.h | 2 +-
arch/x86/include/asm/msr-index.h | 3 +
arch/x86/include/asm/sev.h | 6 +
arch/x86/kernel/cpu/scattered.c | 1 +
arch/x86/kvm/svm/sev.c | 2 +
arch/x86/virt/svm/sev.c | 417 ++++++++++++++++++++++-
drivers/crypto/ccp/sev-dev.c | 3 +
tools/arch/x86/include/asm/cpufeatures.h | 2 +-
9 files changed, 436 insertions(+), 3 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH 00/15] Enable TDX Module Extensions and DICE-based TDX Quoting
From: Adrian Hunter @ 2026-06-08 18:31 UTC (permalink / raw)
To: Xu Yilun, kas, djbw, rick.p.edgecombe, x86, peter.fang
Cc: linux-coco, linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan, xiaoyao.li
In-Reply-To: <20260522034128.3144354-1-yilun.xu@linux.intel.com>
On 22/05/2026 06:41, Xu Yilun wrote:
> This posting is just to collect initial review.
>
> Sean, Paolo, Dave please feel free to ignore for now. Sean, especially
> the x86 KVM stuff is only here as an example for the init code, and not
> ready for review.
>
> Kiryl and Dan, we are trying to get acks for the first 4 patches of the
> series so they can be serve as a settled base for all the other work
> that uses Extensions. Please review the first 4 patches and treat the
> later ones as an example for the Extensions initialization.
>
> == Why it's being posted ==
>
> The TDX Module is introducing a new concept called "TDX Module
> Extensions", and several upcoming features depend on them. The
> Extensions need some extra setup at TDX module init time, and the code
> to do this is expected to be somewhat generic.
>
> We want to get the basics of this TDX module extensions piece sorted so
> that all of the extension-based work can build on it. This series
> includes those basics, and an example usage called DICE-based TDX
> Quoting. Only the first 4 patches are about initializing the TDX module
> Extensions. I'd like some review on them. The later DICE patches are
> just included to serve as a usage example for the TDX module extension
> code.
>
> The first 4 patches will eventually need an ack by an x86 maintainer, so
> please review with that in mind.
>
> == Overview ==
>
> TDX Module introduces the "TDX Module Extensions" to support long
> running / hard-irq preemptible flows inside. This makes TDX Module
> capable of handling complex tasks through "Extension SEAMCALLs".
For me it would be easier to understand by starting higher level,
like:
"TDX Module Extensions enables optional but important TDX features
- such as DICE-based attestation quoting, TDX Connect, and live
migration - that require substantially more processing time than
core TDX operations, and also additional memory."
Also I would find it helpful to clarify how "TDX Module Extensions"
enhances interruptibility for Extension SEAMCALLs compared with
regular SEAMCALLs, since "hard-irq preemptible flows" had me
initially thinking along the wrong lines.
>
> TDX Module allows some add-on features to use the Extension. The first
> feature to use Extensions is DICE-based TDX Quoting [1]. DICE is an
> industry-standard, certificate-backed attestation framework that layers
> evidence through a chain of certificates.
>
> This series adds infrastructure to enable the Extensions and then
> implement DICE-based TDX Quoting.
>
> The Extensions consumes relatively large amount of memory (~50MB). So it
> is designed to be off by default. It must be enabled after basic TDX
> Module initialization and when add-on features require it. To enable
> the Extensions, host first adds extra memory to TDX Module via a
> SEAMCALL (TDH.EXT.MEM.ADD), then uses another SEAMCALL (TDH.EXT.INIT) to
> initialize Extensions, and then some add-on features, e.g. DICE, could
> use Extension SEAMCALLs for work. Note that host can never get the added
> memory back.
>
> Theoretically, the Extensions doesn't need to be enabled right after
> basic TDX initialization. It could be enabled right before the first
> Extension SEAMCALL is issued. That would save or postpone memory usage.
> But it isn't worth the complexity, the needs for the Extensions are vast
> but the savings are little for a typical TDX capable system (about
> 0.001% of memory). So the Linux decision is to just enable it along with
> the basic TDX.
>
> This series has 2 distinct parts:
>
> Patches 1-4: TDX Module Extensions enabling
> Patches 5-15: DICE-based TDX Quoting, primarily Peter's work.
>
> == Some history ==
>
> The TDX Module Extensions part was first posted along with TDX
> Connect [2]. Now this part is remarkably smaller because we've removed
> the generic tdx_page_array abstraction for HPA_LIST_INFO. TDX Module
> Extensions is the first user of HPA_LIST_INFO, and doesn't use it in a
> typical way (HPA_LIST_INFO can only hold at most 2MB memory). There
> isn't enough justification to make the abstraction in this series. A
> possible plan is to rebuild tdx_page_array iteratively when more use
> cases arise.
>
> == Misc ==
>
> This series is based on tip/x86/tdx [3], because we need a small
> being-merged patch [4] before our work.
>
>
> Link: https://cdrdv2.intel.com/v1/dl/getContent/874303 # [1]
> Link: https://lore.kernel.org/all/20260327160132.2946114-1-yilun.xu@linux.intel.com/ # [2]
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=x86/tdx # [3]
> Link: https://patch.msgid.link/20260402-fuller_tdx_kexec_support-v3-1-34438d7094bf@intel.com # [4]
>
>
> Peter Fang (10):
> x86/virt/tdx: Move tdx_tdr_pa() up in the file
> x86/virt/tdx: Initialize Quoting extension during bringup
> x86/virt/tdx: Prepare Quote buffer during extension bringup
> x86/virt/tdx: Add interface to check Quoting availability
> x86/virt/tdx: Add interface to generate a Quote
> x86/tdx: Move and rename Quote request structure
> KVM: TDX: Factor out userspace return path from tdx_get_quote()
> KVM: TDX: Add in-kernel Quote generation
> KVM: TDX: Support event-notify interrupts only with userspace quoting
> x86/virt/tdx: Enable TDX Quoting extension
>
> Xu Yilun (5):
> x86/virt/tdx: Read global metadata for TDX Module Extensions
> x86/virt/tdx: Add extra memory to TDX Module for Extensions
> x86/virt/tdx: Make TDX Module initialize Extensions
> x86/virt/tdx: Enable the Extensions right after basic TDX Module init
> x86/virt/tdx: Embed version info in SEAMCALL leaf function definitions
>
> Documentation/virt/kvm/api.rst | 8 +-
> arch/x86/include/asm/tdx.h | 34 ++
> arch/x86/include/asm/tdx_global_metadata.h | 11 +
> arch/x86/kvm/vmx/tdx.h | 6 +
> arch/x86/virt/vmx/tdx/tdx.h | 32 +-
> arch/x86/kvm/vmx/tdx.c | 176 ++++++++-
> arch/x86/virt/vmx/tdx/tdx.c | 387 +++++++++++++++++++-
> arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 27 ++
> drivers/virt/coco/tdx-guest/tdx-guest.c | 25 +-
> virt/kvm/kvm_main.c | 1 +
> 10 files changed, 655 insertions(+), 52 deletions(-)
>
>
> base-commit: 5209e5bfe5cab593476c3e7754e42c5e47ce36de
^ permalink raw reply
* Re: [PATCH v6 3/4] firmware: smccc: arm-cca-guest: Bind the TSM provider to an SMCCC device
From: Suzuki K Poulose @ 2026-06-08 16:27 UTC (permalink / raw)
To: Sudeep Holla, Aneesh Kumar K.V
Cc: linux-coco, linux-arm-kernel, linux-kernel, Catalin Marinas,
Greg KH, Jeremy Linton, Jonathan Cameron, Lorenzo Pieralisi,
Mark Rutland, Will Deacon, Steven Price
In-Reply-To: <20260608-hot-fascinating-tortoise-cccc61@sudeepholla>
On 08/06/2026 13:32, Sudeep Holla wrote:
> On Mon, Jun 08, 2026 at 04:56:29PM +0530, Aneesh Kumar K.V wrote:
>> Suzuki K Poulose <suzuki.poulose@arm.com> writes:
>>
>>> On 08/06/2026 09:19, Aneesh Kumar K.V wrote:
>>>> Sudeep Holla <sudeep.holla@kernel.org> writes:
>>>>
>>>>> On Thu, Jun 04, 2026 at 06:56:28PM +0530, Aneesh Kumar K.V wrote:
>>>>>> Sudeep Holla <sudeep.holla@kernel.org> writes:
>>>>>>
>>>>>> ...
>>
>> ...
>>
>>>>> I was trying to avoid conditional compilation altogether and hence the
>>>>> reason for keeping it as simple as possible. Also IS_ENABLED(CONFIG_ARM64)
>>>>> in above snippet must come as some condition to this generic probe.
>>>>>
>>>>> Adding any more logic or callback defeats the bus idea here if we need
>>>>> to rely/depend on multiple conditional compilation or callbacks IMO.
>>>>>
>>>>> Let's find see if it can work with what we are adding now and may add in
>>>>> near future and then decide.
>>>>>
>>>>
>>>> If we move all the conditional checks to the driver probe path, then I
>>>> think this can work. Something like the below:
>>>>
>>>> struct smccc_device_info {
>>>> u32 func_id;
>>>> bool requires_smc;
>>>> const char *device_name;
>>>> };
>>>>
>>>> static const struct smccc_device_info smccc_devices[] __initconst = {
>>>> {
>>>> .func_id = ARM_SMCCC_TRNG_VERSION,
>>>> .requires_smc = false,
>>>> .device_name = "arm-smccc-trng",
>>>> },
>>>>
>>>> {
>>>> .func_id = RSI_ABI_VERSION,
>>>
>>> Don't we need parameters passed to this (Requested Interface version for
>>> e.g.) ? See more below.
>>>
>>
>> The idea is that we only check whether the function ID is supported. All
>> other conditional logic should be handled in the driver probe path, as
>> demonstrated by the changes in drivers/char/hw_random/arm_smccc_trng.c.
>>
>
> +1. Yes, we just want to know whether the firmware is aware of that feature
> before creating the `smccc_device` for it. The device probe can then perform a
> more thorough, feature-specific check to determine whether the device/feature
> is usable.
>
> That is the main idea behind the approach I suggested. Please let me know if
> you still see any issues or think this may not work.
Ok, yea, I kind of forgot that we are boot strapping a driver based on
this "smccc device" and the device is only an indication of the firmware
service, the driver would decide if the service matches its expectation.
Thanks for the clarification, apologies for the noise.
Suzuki
>
^ permalink raw reply
* Re: [RFC PATCH] mm/vmalloc: add vmalloc_decrypted() and vzalloc_decrypted()
From: Catalin Marinas @ 2026-06-08 15:37 UTC (permalink / raw)
To: Kameron Carr
Cc: akpm, urezki, linux-mm, linux-kernel, rppt, mhklinux, linux-coco,
Suzuki K Poulose
In-Reply-To: <20260521205834.1012925-1-kameroncarr@linux.microsoft.com>
+ linux-coco, Suzuki (for the arm64 behaviour)
On Thu, May 21, 2026 at 01:58:34PM -0700, Kameron Carr wrote:
> In confidential computing environments (arm64 CCA, x86 SEV/TDX), guest
> memory is encrypted by default and must be explicitly transitioned to a
> decrypted/shared state for host-visible access. Calling
> set_memory_decrypted() on a vmalloc address is not supported, and not
> recommended as it would be inefficient to decrypt the pages after they
> have been mapped.
>
> Add vmalloc_decrypted() and vzalloc_decrypted() which decrypt pages on
> the linear map before creating the vmalloc mapping via vmap(), so
> physical pages are never mapped with conflicting encryption attributes
> across aliases. A new VM_DECRYPTED flag marks these allocations so that
> vfree() automatically re-encrypts pages before returning them to the
> page allocator.
>
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Link: https://lore.kernel.org/linux-arm-kernel/ZmNJdSxSz-sYpVgI@arm.com/
> Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
> ---
> include/linux/vmalloc.h | 7 ++
> mm/vmalloc.c | 163 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 170 insertions(+)
There are a few Sashiko comments worth reviewing:
https://sashiko.dev/#/patchset/20260521205834.1012925-1-kameroncarr@linux.microsoft.com
[...]
> +/*
> + * Re-encrypt the linear-map alias of all pages backing a VM_DECRYPTED area.
> + * Best-effort: on per-block failure the loop continues so as many pages as
> + * possible are returned to the encrypted state. Pages that fail to
> + * transition are left out of area->pages and leaked.
> + */
> +static int vm_pages_encrypt(struct vm_struct *area)
> +{
> + unsigned int nr = 1U << vm_area_page_order(area);
> + unsigned int i;
> + unsigned int leaked;
> + int ret = 0;
> +
> + for (i = 0; i < area->nr_pages; i += nr) {
> + int err = __vm_pages_enc_dec(area, i, nr, true);
> +
> + if (err && !ret)
> + ret = err;
> + }
> +
> + leaked = vm_compact_leaked_pages(area);
> + if (leaked)
> + pr_warn("vmalloc: re-encryption failed, leaked %u pages\n",
> + leaked);
> + return ret;
> +}
> +
> +/*
> + * Decrypt the linear-map alias of all pages backing a VM_DECRYPTED area.
> + * On failure, the already-decrypted prefix is rolled back to encrypted.
> + * Pages that fail either the initial decrypt or the rollback re-encrypt are
> + * left out of area->pages and leaked.
> + */
> +static int vm_pages_decrypt(struct vm_struct *area)
> +{
> + unsigned int nr = 1U << vm_area_page_order(area);
> + unsigned int i;
> + unsigned int leaked;
> + int ret = 0;
> +
> + for (i = 0; i < area->nr_pages; i += nr) {
> + ret = __vm_pages_enc_dec(area, i, nr, false);
> + if (ret)
> + goto rollback;
> + }
> + return 0;
> +
> +rollback:
> + while (i) {
> + i -= nr;
> + __vm_pages_enc_dec(area, i, nr, true);
> + }
> +
> + leaked = vm_compact_leaked_pages(area);
> + if (leaked)
> + pr_warn("vmalloc: decryption failed, leaked %u pages\n",
> + leaked);
> + return ret;
> +}
> +
> /**
> * vfree - Release memory allocated by vmalloc()
> * @addr: Memory base address
> @@ -3457,6 +3554,9 @@ void vfree(const void *addr)
> return;
> }
>
> + if (unlikely(vm->flags & VM_DECRYPTED))
> + vm_pages_encrypt(vm);
I think we still have the vmalloc aliases at this point as we lazily
reclaim them. We should call vm_unmap_aliases() before
vm_pages_encrypt(). It matches the x86 __set_memory_enc_pgtable() as
well with the explicit call to vm_unmap_aliases().
The vrealloc() path may have some issues as well but I haven't looked in
detail. Not sure it actually re-allocs decrypted pages. The simplest is
to reject vrealloc() for such vms until we have a use-case.
> +/**
> + * vzalloc_decrypted - allocate zeroed virtually contiguous decrypted memory
> + * @size: allocation size
> + *
> + * Like vmalloc_decrypted(), but the memory is set to zero.
> + *
> + * Return: pointer to the allocated memory or %NULL on error
> + */
> +void *vzalloc_decrypted_noprof(unsigned long size)
> +{
> + void *addr;
> +
> + addr = __vmalloc_node_range_noprof(size, 1, VMALLOC_START, VMALLOC_END,
> + GFP_KERNEL,
> + pgprot_decrypted(PAGE_KERNEL),
> + VM_DECRYPTED, NUMA_NO_NODE,
> + __builtin_return_address(0));
> + if (addr)
> + memset(addr, 0, size);
Talking to Suzuki, the small window between set_memory_decrypted() and
memset() potentially exposing stale data is safe, at least for Arm CCA
as the memory would be scrubbed (there are other places in the kernel
where we do something similar). I assume that's also the case for other
architectures, although not sure what pKVM does.
--
Catalin
^ permalink raw reply
* Re: [RFC PATCH 15/15] x86/virt/tdx: Enable TDX Quoting extension
From: Xu Yilun @ 2026-06-08 15:10 UTC (permalink / raw)
To: Kishen Maloor
Cc: kas, djbw, rick.p.edgecombe, x86, peter.fang, linux-coco,
linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan, xiaoyao.li
In-Reply-To: <caf5de99-bce3-4dc6-8757-cf2b4aaf85c9@intel.com>
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> > index 10aff23cd01f..524a14c01aa6 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.h
> > +++ b/arch/x86/virt/vmx/tdx/tdx.h
> > @@ -58,7 +58,8 @@
> > #define TDH_PHYMEM_CACHE_WB 40
> > #define TDH_PHYMEM_PAGE_WBINVD 41
> > #define TDH_VP_WR 43
> > -#define TDH_SYS_CONFIG 45
> > +#define TDH_SYS_CONFIG_V0 45
>
> Is it necessary to add _Vx macros when multiple versions can co-exist?
>
> Just wondering if it would be cleaner in the following way?
>
> - Leave the macros set at the current (non-deprecated) baseline version.
> - Select vX using SEAMCALL_LEAF_VER() in config_tdx_module() when a vX feature
> is enabled.
My perference is to have the most simple rule for the unversioned macro.
And leave all workarounds to _Vx macros, they will eventually been
removed or deprecated.
Anyway this patch is not expected to merge with this series, maybe we
will have a public release with V1 supported when it is to be merged,
then we don't have to make such a hard choice.
>
> u64 seamcall_fn = TDH_SYS_CONFIG;
> ...
> if (tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_QUOTE) {
> ...
> seamcall_fn = SEAMCALL_LEAF_VER(TDH_SYS_CONFIG, 1);
^ permalink raw reply
* Re: [PATCH v14 26/44] arm64: RMI: Allow populating initial contents
From: Steven Price @ 2026-06-08 13:53 UTC (permalink / raw)
To: Suzuki K Poulose, Gavin Shan, kvm, kvmarm
Cc: Catalin Marinas, Marc Zyngier, Will Deacon, James Morse,
Oliver Upton, Zenghui Yu, linux-arm-kernel, linux-kernel,
Joey Gouly, Alexandru Elisei, Christoffer Dall, Fuad Tabba,
linux-coco, Ganapatrao Kulkarni, Shanker Donthineni, Alper Gun,
Aneesh Kumar K . V, Emi Kisanuki, Vishal Annapurve, WeiLin.Chang,
Lorenzo.Pieralisi2
In-Reply-To: <ad45c64c-18ab-4d42-9813-0ac1704bb127@arm.com>
On 08/06/2026 10:41, Suzuki K Poulose wrote:
> On 08/06/2026 10:36, Steven Price wrote:
>> On 28/05/2026 06:30, Gavin Shan wrote:
>>> Hi Steve,
>>>
>>> On 5/13/26 11:17 PM, Steven Price wrote:
>>>> The VMM needs to populate the realm with some data before starting
>>>> (e.g.
>>>> a kernel and initrd). This is measured by the RMM and used as part of
>>>> the attestation later on.
>>>>
>>>> Signed-off-by: Steven Price <steven.price@arm.com>
>
> ...
>
>>>> diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
>>>> index a89873a5eb77..209087bcf399 100644
>>>> --- a/arch/arm64/kvm/rmi.c
>>>> +++ b/arch/arm64/kvm/rmi.c
>>>> @@ -486,6 +486,75 @@ void kvm_realm_unmap_range(struct kvm *kvm,
>>>> unsigned long start,
>>>> realm_unmap_private_range(kvm, start, end, may_block);
>>>> }
>>>> +static int realm_data_map_init(struct kvm *kvm, unsigned long ipa,
>>>> + kvm_pfn_t dst_pfn, kvm_pfn_t src_pfn,
>>>> + unsigned long flags)
>>>> +{
>>>> + struct realm *realm = &kvm->arch.realm;
>>>> + phys_addr_t rd = virt_to_phys(realm->rd);
>>>> + phys_addr_t dst_phys, src_phys;
>>>> + int ret;
>>>> +
>>>> + dst_phys = __pfn_to_phys(dst_pfn);
>>>> + src_phys = __pfn_to_phys(src_pfn);
>>>> +
>>>> + if (rmi_delegate_page(dst_phys))
>>>> + return -ENXIO;
>>>> +
>>>> + ret = rmi_rtt_data_map_init(rd, dst_phys, ipa, src_phys, flags);
>>>> + if (RMI_RETURN_STATUS(ret) == RMI_ERROR_RTT) {
>>>> + /* Create missing RTTs and retry */
>>>> + int level = RMI_RETURN_INDEX(ret);
>>>> +
>>>> + KVM_BUG_ON(level == KVM_PGTABLE_LAST_LEVEL, kvm);
>>>
>>> KVM_BUG_ON(level >= KVM_PGTABLE_LAST_LEVEL, kvm);
>>
>> Ack.
>>
>
> Thinking more about this, I guess a buggy VMM can trigger this
> by populating twice ? (level == KVM_PGTABLE_LAST_LEVEL). So, we should
> return the error back, than warning here and suppressing the error ?
Populating twice causes rmi_delegate_page() to be run twice on the same
page and the second one will then fail. So I don't think this is
possible (please correct me if I've missed something!)
Thanks,
Steve
^ permalink raw reply
* Re: [PATCH v14 29/44] arm64: RMI: Runtime faulting of memory
From: Suzuki K Poulose @ 2026-06-08 12:58 UTC (permalink / raw)
To: Steven Price, Gavin Shan, kvm, kvmarm
Cc: Catalin Marinas, Marc Zyngier, Will Deacon, James Morse,
Oliver Upton, Zenghui Yu, linux-arm-kernel, linux-kernel,
Joey Gouly, Alexandru Elisei, Christoffer Dall, Fuad Tabba,
linux-coco, Ganapatrao Kulkarni, Shanker Donthineni, Alper Gun,
Aneesh Kumar K . V, Emi Kisanuki, Vishal Annapurve, WeiLin.Chang,
Lorenzo.Pieralisi2
In-Reply-To: <21abf014-a68c-49b6-a113-4ec59c520a30@arm.com>
On 08/06/2026 11:56, Steven Price wrote:
> On 08/06/2026 10:30, Suzuki K Poulose wrote:
>> On 05/06/2026 07:23, Gavin Shan wrote:
>>> Hi Steve,
>>>
>>> On 5/13/26 11:17 PM, Steven Price wrote:
>>>> At runtime if the realm guest accesses memory which hasn't yet been
>>>> mapped then KVM needs to either populate the region or fault the guest.
>>>>
>>>> For memory in the lower (protected) region of IPA a fresh page is
>>>> provided to the RMM which will zero the contents. For memory in the
>>>> upper (shared) region of IPA, the memory from the memslot is mapped
>>>> into the realm VM non secure.
>>>>
>>>> Signed-off-by: Steven Price <steven.price@arm.com>
>>>> ---
>>>> Changes since v13:
>>>> * Numerous changes due to rebasing.
>>>> * Fix addr_range_desc() to encode the correct block size.
>>>> Changes since v12:
>>>> * Switch to RMM v2.0 range based APIs.
>>>> Changes since v11:
>>>> * Adapt to upstream changes.
>>>> Changes since v10:
>>>> * RME->RMI renaming.
>>>> * Adapt to upstream gmem changes.
>>>> Changes since v9:
>>>> * Fix call to kvm_stage2_unmap_range() in kvm_free_stage2_pgd() to set
>>>> may_block to avoid stall warnings.
>>>> * Minor coding style fixes.
>>>> Changes since v8:
>>>> * Propagate the may_block flag.
>>>> * Minor comments and coding style changes.
>>>> Changes since v7:
>>>> * Remove redundant WARN_ONs for realm_create_rtt_levels() - it will
>>>> internally WARN when necessary.
>>>> Changes since v6:
>>>> * Handle PAGE_SIZE being larger than RMM granule size.
>>>> * Some minor renaming following review comments.
>>>> Changes since v5:
>>>> * Reduce use of struct page in preparation for supporting the RMM
>>>> having a different page size to the host.
>>>> * Handle a race when delegating a page where another CPU has
>>>> faulted on
>>>> a the same page (and already delegated the physical page) but not
>>>> yet
>>>> mapped it. In this case simply return to the guest to either use the
>>>> mapping from the other CPU (or refault if the race is lost).
>>>> * The changes to populate_par_region() are moved into the previous
>>>> patch where they belong.
>>>> Changes since v4:
>>>> * Code cleanup following review feedback.
>>>> * Drop the PTE_SHARED bit when creating unprotected page table
>>>> entries.
>>>> This is now set by the RMM and the host has no control of it and the
>>>> spec requires the bit to be set to zero.
>>>> Changes since v2:
>>>> * Avoid leaking memory if failing to map it in the realm.
>>>> * Correctly mask RTT based on LPA2 flag (see rtt_get_phys()).
>>>> * Adapt to changes in previous patches.
>>>> ---
>>>> arch/arm64/include/asm/kvm_emulate.h | 8 ++
>>>> arch/arm64/include/asm/kvm_rmi.h | 12 ++
>>>> arch/arm64/kvm/mmu.c | 128 ++++++++++++++++----
>>>> arch/arm64/kvm/rmi.c | 173 +++++++++++++++++++++++++++
>>>> 4 files changed, 301 insertions(+), 20 deletions(-)
>>>>
>
> [...]
>
>>>> diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
>>>> index cae29fd3353c..761b38a4071c 100644
>>>> --- a/arch/arm64/kvm/rmi.c
>>>> +++ b/arch/arm64/kvm/rmi.c
>>>> @@ -597,6 +597,179 @@ static int realm_data_map_init(struct kvm *kvm,
>>>> unsigned long ipa,
>>>> return ret;
>>>> }
>>>> +static unsigned long addr_range_desc(unsigned long phys, unsigned
>>>> long size)
>>>> +{
>>>> + unsigned long out = 0;
>>>> +
>>>> + switch (size) {
>>>> + case P4D_SIZE:
>>>> + out = 3 | (1 << 2);
>>>> + break;
>>>> + case PUD_SIZE:
>>>> + out = 2 | (1 << 2);
>>>> + break;
>>>> + case PMD_SIZE:
>>>> + out = 1 | (1 << 2);
>>>> + break;
>>>> + case PAGE_SIZE:
>>>> + out = 0 | (1 << 2);
>>>> + break;
>>>> + default:
>>>> + /*
>>>> + * Only support mapping at the page level granulatity when
>>>> + * it's an unusual length. This should get us back onto a
>>>> larger
>>>> + * block size for the subsequent mappings.
>>>> + */
>>>> + out = 0 | ((MIN(size >> PAGE_SHIFT, PTRS_PER_PTE - 1)) << 2);
>>>> + break;
>>>> + }
>>>> +
>>>> + WARN_ON(phys & ~PAGE_MASK);
>>>> +
>>>> + out |= phys & PAGE_MASK;
>>>> +
>>>> + return out;
>>>> +}
>>>> +
>>>> +int realm_map_protected(struct kvm *kvm,
>>>> + unsigned long ipa,
>>>> + kvm_pfn_t pfn,
>>>> + unsigned long map_size,
>>>> + struct kvm_mmu_memory_cache *memcache)
>>>> +{
>>>> + struct realm *realm = &kvm->arch.realm;
>>>> + phys_addr_t phys = __pfn_to_phys(pfn);
>>>> + phys_addr_t base_phys = phys;
>>>> + phys_addr_t rd = virt_to_phys(realm->rd);
>>>> + unsigned long base_ipa = ipa;
>>>> + unsigned long ipa_top = ipa + map_size;
>>>> + int ret = 0;
>>>> +
>>>> + if (WARN_ON(!IS_ALIGNED(map_size, PAGE_SIZE) ||
>>>> + !IS_ALIGNED(ipa, map_size)))
>>>> + return -EINVAL;
>>>> +
>>>> + if (rmi_delegate_range(phys, map_size)) {
>>>> + /*
>>>> + * It's likely we raced with another VCPU on the same
>>>> + * fault. Assume the other VCPU has handled the fault
>>>> + * and return to the guest.
>>>> + */
>>>> + return 0;
>>>> + }
>>>> +
>>>> + while (ipa < ipa_top) {
>>>> + unsigned long flags = RMI_ADDR_TYPE_SINGLE;
>>>> + unsigned long range_desc = addr_range_desc(phys, ipa_top -
>>>> ipa);
>>>> + unsigned long out_top;
>>>> +
>>>> + ret = rmi_rtt_data_map(rd, ipa, ipa_top, flags, range_desc,
>>>> + &out_top);
>>>> +
>>>> + if (RMI_RETURN_STATUS(ret) == RMI_ERROR_RTT) {
>>>> + /* Create missing RTTs and retry */
>>>> + int level = RMI_RETURN_INDEX(ret);
>>>> +
>>>> + WARN_ON(level == KVM_PGTABLE_LAST_LEVEL);
>>>> + ret = realm_create_rtt_levels(realm, ipa, level,
>>>> + KVM_PGTABLE_LAST_LEVEL,
>>>> + memcache);
>>
>> Could we give the RMM a chance to make use of the Block mappings by
>> creating the Missing RTTs to the level that may work for the current
>> range_desc ? i.e., if the range_desc is a 2M block size, we could create
>> tables upto L2 in the first go and if the RMM still needs RTT, we could
>> go further down to the KVM_PGTABLE_LAST_LEVEL. I understand this is
>> kind of an optimisation, so may be we could defer it. (Same applies for
>> the non_secure map below).
>
> A simple change would be just to create one level at a time like this:
>
> diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
> index b79b96f7dffb..3f3ade1d3895 100644
> --- a/arch/arm64/kvm/rmi.c
> +++ b/arch/arm64/kvm/rmi.c
> @@ -767,15 +767,15 @@ static int realm_map_protected(struct kvm *kvm,
> /* Create missing RTTs and retry */
> int level = RMI_RETURN_INDEX(ret);
>
> - WARN_ON(level == KVM_PGTABLE_LAST_LEVEL);
> + if (WARN_ON(level >= KVM_PGTABLE_LAST_LEVEL))
> + goto err_undelegate;
> ret = realm_create_rtt_levels(realm, ipa, level,
> - KVM_PGTABLE_LAST_LEVEL,
> + level + 1,
> memcache);
> if (ret)
> goto err_undelegate;
>
> - ret = rmi_rtt_data_map(rd, ipa, ipa_top, flags,
> - range_desc, &out_top);
> + continue;
> }
That looks good to me.
Cheers
Suzuki
>
> if (WARN_ON(ret))
>
> Thanks,
> Steve
>
^ permalink raw reply
* Re: [PATCH v6 3/4] firmware: smccc: arm-cca-guest: Bind the TSM provider to an SMCCC device
From: Sudeep Holla @ 2026-06-08 12:32 UTC (permalink / raw)
To: Aneesh Kumar K.V, Suzuki K Poulose
Cc: linux-coco, Sudeep Holla, linux-arm-kernel, linux-kernel,
Catalin Marinas, Greg KH, Jeremy Linton, Jonathan Cameron,
Lorenzo Pieralisi, Mark Rutland, Will Deacon, Steven Price
In-Reply-To: <yq5aldcpz316.fsf@kernel.org>
On Mon, Jun 08, 2026 at 04:56:29PM +0530, Aneesh Kumar K.V wrote:
> Suzuki K Poulose <suzuki.poulose@arm.com> writes:
>
> > On 08/06/2026 09:19, Aneesh Kumar K.V wrote:
> >> Sudeep Holla <sudeep.holla@kernel.org> writes:
> >>
> >>> On Thu, Jun 04, 2026 at 06:56:28PM +0530, Aneesh Kumar K.V wrote:
> >>>> Sudeep Holla <sudeep.holla@kernel.org> writes:
> >>>>
> >>>> ...
>
> ...
>
> >>> I was trying to avoid conditional compilation altogether and hence the
> >>> reason for keeping it as simple as possible. Also IS_ENABLED(CONFIG_ARM64)
> >>> in above snippet must come as some condition to this generic probe.
> >>>
> >>> Adding any more logic or callback defeats the bus idea here if we need
> >>> to rely/depend on multiple conditional compilation or callbacks IMO.
> >>>
> >>> Let's find see if it can work with what we are adding now and may add in
> >>> near future and then decide.
> >>>
> >>
> >> If we move all the conditional checks to the driver probe path, then I
> >> think this can work. Something like the below:
> >>
> >> struct smccc_device_info {
> >> u32 func_id;
> >> bool requires_smc;
> >> const char *device_name;
> >> };
> >>
> >> static const struct smccc_device_info smccc_devices[] __initconst = {
> >> {
> >> .func_id = ARM_SMCCC_TRNG_VERSION,
> >> .requires_smc = false,
> >> .device_name = "arm-smccc-trng",
> >> },
> >>
> >> {
> >> .func_id = RSI_ABI_VERSION,
> >
> > Don't we need parameters passed to this (Requested Interface version for
> > e.g.) ? See more below.
> >
>
> The idea is that we only check whether the function ID is supported. All
> other conditional logic should be handled in the driver probe path, as
> demonstrated by the changes in drivers/char/hw_random/arm_smccc_trng.c.
>
+1. Yes, we just want to know whether the firmware is aware of that feature
before creating the `smccc_device` for it. The device probe can then perform a
more thorough, feature-specific check to determine whether the device/feature
is usable.
That is the main idea behind the approach I suggested. Please let me know if
you still see any issues or think this may not work.
--
Regards,
Sudeep
^ 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