* Re: [PATCH v14 26/44] arm64: RMI: Allow populating initial contents
From: Steven Price @ 2026-07-15 14:06 UTC (permalink / raw)
To: Kohei Enju
Cc: kvm, kvmarm, Catalin Marinas, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni,
Gavin Shan, Shanker Donthineni, Alper Gun, Aneesh Kumar K . V,
Emi Kisanuki, Vishal Annapurve, WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <alWzvLK1RRotK1LL@FCCLS0092175.localdomain>
On 14/07/2026 05:37, Kohei Enju wrote:
> On 05/13 14:17, 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>
>> ---
>> Changes since v13:
>> * Rename realm_create_protected_data_page() to realm_data_map_init().
>> Changes since v12:
>> * The ioctl now updates the structure with the amount populated rather
>> than returning this through the ioctl return code.
>> * Use the new RMM v2.0 range based RMI calls.
>> * Adapt to upstream changes in kvm_gmem_populate().
>> Changes since v11:
>> * The multiplex CAP is gone and there's a new ioctl which makes use of
>> the generic kvm_gmem_populate() functionality.
>> Changes since v7:
>> * Improve the error codes.
>> * Other minor changes from review.
>> Changes since v6:
>> * Handle host potentially having a larger page size than the RMM
>> granule.
>> * Drop historic "par" (protected address range) from
>> populate_par_region() - it doesn't exist within the current
>> architecture.
>> * Add a cond_resched() call in kvm_populate_realm().
>> Changes since v5:
>> * Refactor to use PFNs rather than tracking struct page in
>> realm_create_protected_data_page().
>> * Pull changes from a later patch (in the v5 series) for accessing
>> pages from a guest memfd.
>> * Do the populate in chunks to avoid holding locks for too long and
>> triggering RCU stall warnings.
>> ---
>> arch/arm64/include/asm/kvm_rmi.h | 4 ++
>> arch/arm64/kvm/Kconfig | 1 +
>> arch/arm64/kvm/arm.c | 13 ++++
>> arch/arm64/kvm/rmi.c | 106 +++++++++++++++++++++++++++++++
>> 4 files changed, 124 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/kvm_rmi.h b/arch/arm64/include/asm/kvm_rmi.h
>> index 007249a13dbc..a2b6bc412a22 100644
>> --- a/arch/arm64/include/asm/kvm_rmi.h
>> +++ b/arch/arm64/include/asm/kvm_rmi.h
>> @@ -88,6 +88,10 @@ int kvm_rec_enter(struct kvm_vcpu *vcpu);
>> int kvm_rec_pre_enter(struct kvm_vcpu *vcpu);
>> int handle_rec_exit(struct kvm_vcpu *vcpu, int rec_run_status);
>>
>> +struct kvm_arm_rmi_populate;
>> +
>> +int kvm_arm_rmi_populate(struct kvm *kvm,
>> + struct kvm_arm_rmi_populate *arg);
>> void kvm_realm_unmap_range(struct kvm *kvm,
>> unsigned long ipa,
>> unsigned long size,
>> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
>> index 4e16719fda22..d0cd011cf672 100644
>> --- a/arch/arm64/kvm/Kconfig
>> +++ b/arch/arm64/kvm/Kconfig
>> @@ -38,6 +38,7 @@ menuconfig KVM
>> select GUEST_PERF_EVENTS if PERF_EVENTS
>> select KVM_GUEST_MEMFD
>> select KVM_GENERIC_MEMORY_ATTRIBUTES
>> + select HAVE_KVM_ARCH_GMEM_POPULATE
>> help
>> Support hosting virtualized guest machines.
>>
>> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
>> index ed88a203b892..073ba9181da9 100644
>> --- a/arch/arm64/kvm/arm.c
>> +++ b/arch/arm64/kvm/arm.c
>> @@ -2131,6 +2131,19 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>> return -EFAULT;
>> return kvm_vm_ioctl_get_reg_writable_masks(kvm, &range);
>> }
>> + case KVM_ARM_RMI_POPULATE: {
>> + struct kvm_arm_rmi_populate req;
>> + int ret;
>> +
>> + if (!kvm_is_realm(kvm))
>> + return -ENXIO;
>> + if (copy_from_user(&req, argp, sizeof(req)))
>> + return -EFAULT;
>> + ret = kvm_arm_rmi_populate(kvm, &req);
>> + if (copy_to_user(argp, &req, sizeof(req)))
>> + return -EFAULT;
>> + return ret;
>> + }
>> default:
>> return -EINVAL;
>> }
>> 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);
>> +
>> + ret = realm_create_rtt_levels(realm, ipa, level,
>> + KVM_PGTABLE_LAST_LEVEL, NULL);
>> + if (!ret) {
>> + ret = rmi_rtt_data_map_init(rd, dst_phys, ipa, src_phys,
>> + flags);
>
> I think rmi_rtt_data_map_init() returns a raw RMI return value which is
> positive on failure. Shouldn't it be converted to -ENXIO or another
> Linux errno before returning it?
>
> Otherwise, the positive error value can propagate through
> kvm_gmem_populate() and populate_region() to kvm_arm_rmi_populate().
> There, it would be treated as the number of pages populated, even though
> the operation did not populate any pages.
Good spot.
>> + }
>> + }
>> +
>> + if (ret) {
>> + if (WARN_ON(rmi_undelegate_page(dst_phys))) {
>> + /* Undelegate failed, so we leak the page */
>> + get_page(pfn_to_page(dst_pfn));
>
> Since realm_create_rtt_levels() returns either 0 or a negative errno,
> how about preserving that error and converting any raw RMI erorr to
> -ENXIO here?
>
> return ret < 0 ? ret : -ENXIO;
Makes sense, although this needs to be:
return ret <= 0 ? ret : -ENXIO;
to handle the ret==0 case. I'll fix this in my next posting.
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int populate_region_cb(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
>> + struct page *src_page, void *opaque)
>> +{
>> + unsigned long data_flags = *(unsigned long *)opaque;
>> + phys_addr_t ipa = gfn_to_gpa(gfn);
>> +
>> + if (!src_page)
>> + return -EOPNOTSUPP;
>> +
>> + return realm_data_map_init(kvm, ipa, pfn, page_to_pfn(src_page),
>> + data_flags);
>> +}
>> +
>> +static long populate_region(struct kvm *kvm,
>> + gfn_t base_gfn,
>> + unsigned long pages,
>> + u64 uaddr,
>> + unsigned long data_flags)
>> +{
>> + long ret = 0;
>> +
>> + mutex_lock(&kvm->slots_lock);
>> + ret = kvm_gmem_populate(kvm, base_gfn, u64_to_user_ptr(uaddr), pages,
>> + populate_region_cb, &data_flags);
>> + mutex_unlock(&kvm->slots_lock);
>> +
>> + return ret;
>> +}
>> +
>> enum ripas_action {
>> RIPAS_INIT,
>> RIPAS_SET,
>> @@ -574,6 +643,43 @@ static int realm_ensure_created(struct kvm *kvm)
>> return -ENXIO;
>> }
>>
>> +int kvm_arm_rmi_populate(struct kvm *kvm,
>> + struct kvm_arm_rmi_populate *args)
>> +{
>> + unsigned long data_flags = 0;
>> + unsigned long ipa_start = args->base;
>> + unsigned long ipa_end = ipa_start + args->size;
>> + long pages_populated;
>> + int ret;
>> +
>> + if (args->reserved ||
>> + (args->flags & ~KVM_ARM_RMI_POPULATE_FLAGS_MEASURE) ||
>> + !IS_ALIGNED(ipa_start, PAGE_SIZE) ||
>> + !IS_ALIGNED(ipa_end, PAGE_SIZE) ||
>> + !IS_ALIGNED(args->source_uaddr, PAGE_SIZE))
>> + return -EINVAL;
>> +
>> + ret = realm_ensure_created(kvm);
>> + if (ret)
>> + return ret;
>
> If I understand correctly, concurrent KVM_ARM_RMI_POPULATE calls can
> race with each other in realm_ensure_created().
>
> realm_ensure_created() calls realm_create_rd() when kvm_realm_state() ==
> REALM_STATE_NONE. Since kvm->arch.config_lock is not held here, multiple
> threads could enter realm_create_rd() concurrently.
>
> KVM_ARM_RMI_POPULATE can also race with KVM_RUN through
> kvm_activate_realm(). kvm_activate_realm() takes kvm->arch.config_lock
> while checking the Realm state, calling realm_ensure_created(), and
> updating the state, whereas KVM_ARM_RMI_POPULATE does not.
>
> How about serializing Realm creation here as follows?
> scoped_guard(mutex, &kvm->arch.config_lock) {
> ret = realm_ensure_created(kvm);
> if (ret)
> return ret;
> }
>
> Does that make sense?
Yes this is a bug I'd already caught and is due to the fixed in my next
posting - similar to what you suggest the config_lock is now held when
realm_ensure_created() is called (and I've put a lockdep_assert_held()
in realm_ensure_created).
Thanks,
Steve
>> +
>> + if (args->flags & KVM_ARM_RMI_POPULATE_FLAGS_MEASURE)
>> + data_flags |= RMI_MEASURE_CONTENT;
>> +
>> + pages_populated = populate_region(kvm, gpa_to_gfn(ipa_start),
>> + args->size >> PAGE_SHIFT,
>> + args->source_uaddr, data_flags);
>> +
>> + if (pages_populated < 0)
>> + return pages_populated;
>> +
>> + args->size -= pages_populated << PAGE_SHIFT;
>> + args->source_uaddr += pages_populated << PAGE_SHIFT;
>> + args->base += pages_populated << PAGE_SHIFT;
>> +
>> + return 0;
>> +}
>> +
>> static void kvm_complete_ripas_change(struct kvm_vcpu *vcpu)
>> {
>> struct kvm *kvm = vcpu->kvm;
>> --
>> 2.43.0
>>
>>
^ permalink raw reply
* Re: [PATCH v8 6/6] coco: guest: arm64: Replace dummy CCA device with sysfs ABI
From: Suzuki K Poulose @ 2026-07-15 12:19 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm), linux-coco, linux-arm-kernel,
linux-kernel
Cc: Catalin Marinas, Greg KH, Jeremy Linton, Jonathan Cameron,
Lorenzo Pieralisi, Mark Rutland, Sudeep Holla, Will Deacon,
Steven Price, Andre Przywara
In-Reply-To: <20260707081351.1680209-7-aneesh.kumar@kernel.org>
On 07/07/2026 09:13, Aneesh Kumar K.V (Arm) wrote:
> The SMCCC firmware driver now creates the arm-smccc platform device and
> instantiates the CCA RSI auxiliary devices once the RSI ABI is discovered.
> The arm64-specific arm-cca-dev platform device stub is therefore no longer
> needed.
>
> However, userspace has used the arm-cca-dev platform device to detect Arm
> CCA Realm guests [1]. Removing it without a replacement would break that
> detection and would also leave userspace depending on kernel device-model
> details.
>
> Add /sys/firmware/cca/realm_guest as a stable, architecture-provided ABI
> for detecting whether the kernel is running as an Arm CCA Realm guest. The
> file returns 1 in Realm world and 0 otherwise, similar to the existing s390
> /sys/firmware/uv/prot_virt_guest interface for protected virtualization
> guests.
>
> Remove the dummy arm-cca-dev registration now that userspace has a
> dedicated CCA Realm guest indicator, and document the new ABI in
> Documentation/ABI/testing/sysfs-firmware-cca.
>
> [1] https://lore.kernel.org/all/4a7d84b2-2ec4-4773-a2d5-7b63d5c683cf@arm.com
>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>
^ permalink raw reply
* Re: [PATCH 00/15] Device Evidence and Trust for PCI Security Protocol (TDISP)
From: Alexey Kardashevskiy @ 2026-07-15 9:04 UTC (permalink / raw)
To: Jason Gunthorpe, Dan Williams (nvidia)
Cc: linux-coco, linux-pci, driver-core, ankita, Aaron Tomlin,
Alistair Francis, Aneesh Kumar K.V, Arnd Bergmann, Bjorn Helgaas,
Daniel Gomez, Danilo Krummrich, Dexuan Cui, Donald Hunter,
Greg Kroah-Hartman, Jakub Kicinski, Luis Chamberlain,
Lukas Wunner, Petr Pavlu, Rafael J. Wysocki, Robin Murphy,
Sami Tolvanen, Samuel Ortiz, Saravana Kannan, Will Deacon,
Xu Yilun
In-Reply-To: <20260709133601.GJ118978@ziepe.ca>
On 9/7/26 23:36, Jason Gunthorpe wrote:
> On Wed, Jul 08, 2026 at 07:45:09PM -0700, Dan Williams (nvidia) wrote:
>>> force_dma_unencrypted() does not *prevent* device access to private
>>> memory and provides no security properties on its own. It's only
>>> purpose is to inform the DMA API what the HW restrictions are for
>>> doing DMA.
>>
>> Right, to be clear, this mode's security properties come from never
>> asking the TSM to enable private DMA while the device is in RUN.
>
> Ok, that's a twist I hadn't thought about. I don't see a reason to
> support a driver probed with RUN but T=1 DMA disabled by the TSM.
afaik you cannot have RUN and T=0 DMA at the same time. You can though have C=0 (i.e. IOMMU <-> memory in the plain).
> Still, if we do this, I think having the TSM deal with it is probably
> better than making a cross product of the trust level, something like:
>
> echo only-shared-dma > $pdev/tsm/accept
>
> So maybe accept should be tweaked:
>
> echo full > $pdev/tsm/accept
>
> ? It doesn't matter much
I configure my hw to allow "accept" (== T=1 for DMA and MMIO) but still only allow unencrypted guest memory for DMA (set vTOM to 0 to say "all unencrypted) if the driver was loaded with "trust" other than "full" (and this series does not call the enable_dma() hook if not "full", Dan is changing it though) so the module parameter works...
>>>> echo 1 > $pdev/tsm/accept
>>>
>>> And now it is RUN. So I don't see the issue with enabling DMA at the
>>> same time as gonig to RUN? (though defering it to driver probe would
>>> be a very nice touch as well)
>>
>> It requires "accept" to consider the trust level. E.g. what does it mean
>> to do something like change requirements after accept?
>
>> echo full $pdev/trust
>> echo 1 > $pdev/tsm/accept
>> echo adversary > $pdev/trust
>
>> In that scenario this now adversary device may have been allowed to
>> operate without an enforcing vIOMMU, and needs to unlock the device to
>> correct that.
>
> If accept enables DMA then it is up to userspace to ensure all policy
> objectives are met before accepting, not after.
>
> I think the statement kernel makes to userspace is the moment it
> writes trust or accept the kernel is free to take action on it. ie
> don't write a trust or accept until you mean it.
>
>>>> There are also buses and paravisors that may know that private-DMA is
>>>> enabled for a device by construction. In that case it is also a "trusted
>>>> to access" signal, and not a "required to access" signal.
>>>
>>> In this case they wire force_dma_unencrypted()=false.
>>
>> Yeah, I just need a scheme where modular bus providers do not end up
>> compromising the private / unexported method of changing the flag that
>> force_dma_unencrypted() consumes.
>
> Yeah, the bus and TSM really have to control this property, it is like
> the other DMA attributes (eg dma seg boundary, etc)
>
>>> At least if we omit the double check it can be fairly easy to add in
>>> later if it really was needed for some time of use reason.
>>
>> As long as IOMMU presence can be enumerated prior to acceptance, T=1
>> always includes private DMA, and the T=1 status is tracked independent
>> of the trust level then yes, the cross product can be avoided.
>
> Ok. iommu has to be setup the moment the struct device is created, it
> can't add on later. I *think* the iommu related sysfs ordering is done
> properly before any uevents, but I haven't validated that.
>
>> 'struct device' grows some "request" policy for trust and T=1 that can
>> be changed by modules etc. 'struct device_private' grows the operational
>> trust level stable under device_lock() and a new flag to reflect T=1.
>
> I guess
> - The active trust level should be RO visible to the driver, iommu, etc
> It should be stable under a bound driver
>
> - The "dma require unencrypted" property needs to RO visible to the
> DMA API and stable under a bound driver. This would input where
> force_dma_unecrpyted() is in the flow [the name should align with
> all the other per-device DMA API specific properties like seg
> limit, boundary, mask, etc]
>
> - The requested trust policy should be internal to the driver core and
> be converted to the active trust level right before probe
>
> - We should have ways to enable/disable all DMA before/after probe,
"echo 1 > unlock" should do that (but also stops encrypted MMIO) or we want a finer knob?
> including both TSM and IOMMU approaches
> Broadly all busses get some way to convert the requested trust policy
> to the active trust level (eg by mixing in ACPI, etc, etc)
>
> All busses get some way to set the "dma require unencrypted" property
>
> IOMMU and drivers should be sensitive to the trust level
>
> TSM is sensitive to accept, not the trust level
This makes the module's "trust" parameter useless, right?
> Userspace must ensure any security policy is met before accepting or
> binding.
>
> ??
>
> Jason
--
Alexey
^ permalink raw reply
* Re: [PATCH v2 0/2] KVM: x86: gmem populate fix and cleanups
From: Sean Christopherson @ 2026-07-14 18:41 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Kiryl Shutsemau
Cc: Dave Hansen, Rick Edgecombe, kvm, x86, linux-coco, linux-kernel,
Sashiko Bot, Joerg Roedel, Yan Zhao, Ackerley Tng
In-Reply-To: <20260630213711.479692-1-seanjc@google.com>
On Tue, 30 Jun 2026 14:37:09 -0700, Sean Christopherson wrote:
> Fix a user-triggerable WARN due to KVM not pre-checking that userspace
> provided a source page for non-ZERO pages for SNP_LAUNCH_UPDATE, and then
> clean up the equivalent TDX code to also explicitly check the incoming
> source page *before* calling into guest_memfd, and to return -EINVAL, not
> -EOPNOTSUPP.
>
> v2:
> - Rewrite the SNP patch changelog.
> - Tweak the code to avoid checking KVM_SEV_SNP_PAGE_TYPE_ZERO twice.
> - Drop what is now effectively a sanity check in sev_gmem_post_populate(),
> so that we don't have to duplicate the logic when in-place conversion comes
> along.
> - Tack on the TDX change.
>
> [...]
Applied to kvm-x86 misc, thanks!
[1/2] KVM: SEV: Explicitly disallow NULL user address for SNP_LAUNCH_UPDATE
https://github.com/kvm-x86/linux/commit/2abe1ff20151
[2/2] KVM: TDX: Return EINVAL, not EOPNOTSUPP, for NULL INIT_MEM_REGION source
https://github.com/kvm-x86/linux/commit/eb606a243863
--
https://github.com/kvm-x86/linux/tree/next
^ permalink raw reply
* Re: [PATCH 2/2] virt: tdx-guest: Allocate Quote buffer dynamically
From: Kuppuswamy Sathyanarayanan @ 2026-07-14 16:22 UTC (permalink / raw)
To: Peter Fang
Cc: Dave Hansen, Kiryl Shutsemau, Rick Edgecombe, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, linux-kernel,
linux-coco, kvm
In-Reply-To: <20260714062502.GE3178326@pedri>
On 7/13/2026 11:25 PM, Peter Fang wrote:
> On Mon, Jul 13, 2026 at 10:27:46AM -0700, Kuppuswamy Sathyanarayanan wrote:
>> Hi Peter,
>>
>> On 6/12/2026 4:08 AM, Peter Fang wrote:
>>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>>
>>> The TDX attestation driver currently uses a fixed 128 KB Quote buffer
>>> shared with the host VMM. This may be too small for Quotes using schemes
>>> such as post-quantum cryptography (PQC), where certificate chains can
>>> increase the Quote size to several megabytes.
>>>
>>> Allocate the Quote buffer based on the size reported by the TDX module
>>> instead of always reserving a fixed-size buffer. This avoids wasting
>>> memory on platforms that do not require larger Quotes. Older platforms
>>> fall back to the default 128 KB buffer.
>>>
>>> Because the Quote buffer must be physically contiguous, its size is
>>> bound by the buddy allocator's maximum page order (4 MB), which should
>>> be sufficient for current attestation needs.
>>>
>>> struct tdx_quote_buf has a trailing flexible array, so use offsetof()
>>> instead of sizeof() to calculate the header size.
>>>
>>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>> Assisted-by: Claude:claude-opus-4-7
>>> Assisted-by: GitHub Copilot:gpt-5.4
>>> Signed-off-by: Peter Fang <peter.fang@intel.com>
>>> ---
>>
>> Looks good to me.
>>
>> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>
> Thanks Sathya!
>
>>
>>
>>> drivers/virt/coco/tdx-guest/tdx-guest.c | 52 ++++++++++++++++++-------
>>> 1 file changed, 38 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
>>> index a9ecc46df187..162fb47f3fae 100644
>>> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
>>> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
>>> @@ -163,7 +163,7 @@ static void tdx_mr_deinit(const struct attribute_group *mr_grp)
>>> * DICE-based attestation uses layered evidence that requires
>>> * larger Quote size (~100K).
>>> */
>>> -#define GET_QUOTE_BUF_SIZE SZ_128K
>>> +#define GET_QUOTE_DEFAULT_BUF_SIZE SZ_128K
>>>
>>> #define GET_QUOTE_CMD_VER 1
>>>
>>> @@ -171,7 +171,7 @@ static void tdx_mr_deinit(const struct attribute_group *mr_grp)
>>> #define GET_QUOTE_SUCCESS 0
>>> #define GET_QUOTE_IN_FLIGHT 0xffffffffffffffff
>>>
>>> -#define TDX_QUOTE_MAX_LEN (GET_QUOTE_BUF_SIZE - sizeof(struct tdx_quote_buf))
>>> +#define TDX_QUOTE_BUF_LEN(n) (offsetof(struct tdx_quote_buf, data) + (n))
>>>
>>> /* struct tdx_quote_buf: Format of Quote request buffer.
>>> * @version: Quote format version, filled by TD.
>>> @@ -192,8 +192,9 @@ struct tdx_quote_buf {
>>> u8 data[];
>>> };
>>>
>>> -/* Quote data buffer */
>>> +/* Quote data buffer and size */
>>> static void *quote_data;
>>> +static size_t quote_data_size;
>>>
>
> [ ... ]
>
>>>
>>> @@ -286,7 +310,7 @@ static int tdx_report_new_locked(struct tsm_report *report, void *data)
>>> if (desc->inblob_len != TDX_REPORTDATA_LEN)
>>> return -EINVAL;
>>>
>>> - memset(quote_data, 0, GET_QUOTE_BUF_SIZE);
>>> + memset(quote_data, 0, quote_data_size);
>>>
>>> /* Update Quote buffer header */
>>> quote_buf->version = GET_QUOTE_CMD_VER;
>>> @@ -297,7 +321,7 @@ static int tdx_report_new_locked(struct tsm_report *report, void *data)
>>> if (ret)
>>> return ret;
>>>
>>> - err = tdx_hcall_get_quote(quote_data, GET_QUOTE_BUF_SIZE);
>>> + err = tdx_hcall_get_quote(quote_data, quote_data_size);
>>> if (err) {
>>> pr_err("GetQuote hypercall failed, status:%llx\n", err);
>>> return -EIO;
>>> @@ -316,7 +340,7 @@ static int tdx_report_new_locked(struct tsm_report *report, void *data)
>>>
>>> out_len = READ_ONCE(quote_buf->out_len);
>>>
>>> - if (out_len > TDX_QUOTE_MAX_LEN)
>>> + if (TDX_QUOTE_BUF_LEN(out_len) > quote_data_size)
>>> return -EFBIG;
>>
>> Nit: I think this check will be more readable if you can rename
>> quote_data_size to quote_buf_size (since it holds total buffer
>> size).
>
> Hm. This pairs with the original "static void *quote_data". Or perhaps
> "quote_data_len"?
Yes, it pairs with quote_data buffer name. May be both should be renamed.
If it is too much trouble, just leave it.
>
>>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply
* SVSM Development Call July 15th, 2026
From: Stefano Garzarella @ 2026-07-14 16:14 UTC (permalink / raw)
To: coconut-svsm, linux-coco
In-Reply-To: <CAGxU2F5hP=7pA1rKRvq5sgr0t2y1YoUzYCmH8hzaCS58U4+Y3A@mail.gmail.com>
Hi,
Here is the call for agenda items for this weeks SVSM development
call. Please send any agenda items you have in mind as a reply to this
email or raise them in the meeting.
We will use the LF Zoom instance. Details of the meeting can be found
in our governance repository at:
https://github.com/coconut-svsm/governance
The link to the COCONUT-SVSM calendar is:
https://zoom-lfx.platform.linuxfoundation.org/meetings/coconut-svsm?view=week
The meeting will be recorded and the recording eventually published.
Regards,
Stefano
^ permalink raw reply
* RE: [PATCH v7 11/22] dma-pool: track decrypted atomic pools and select them via attrs
From: Michael Kelley @ 2026-07-14 15:46 UTC (permalink / raw)
To: Jason Gunthorpe, Aneesh Kumar K.V
Cc: iommu@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-coco@lists.linux.dev,
Robin Murphy, Marek Szyprowski, Will Deacon, Marc Zyngier,
Steven Price, Suzuki K Poulose, Catalin Marinas, Jiri Pirko,
Mostafa Saleh, Petr Tesarik, Alexey Kardashevskiy, Dan Williams,
Xu Yilun, linuxppc-dev@lists.ozlabs.org,
linux-s390@vger.kernel.org, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86@kernel.org, Jiri Pirko,
Michael Kelley
In-Reply-To: <20260714122504.GG3133966@ziepe.ca>
From: Jason Gunthorpe <jgg@ziepe.ca> Sent: Tuesday, July 14, 2026 5:25 AM
>
> On Tue, Jul 14, 2026 at 09:32:27AM +0530, Aneesh Kumar K.V wrote:
>
> > If we want to warn about such failures, we should add the warning
> > consistently across the code. We may also want to handle decrypt
> > failures by encrypting the page again to avoid leaking it.
> >
> > I will work on that as a tree-wide change in a separate patch.
>
> IMHO the WARN should be inside set_memory_encrypted(), and maybe it
> should be a BUG_ON...
There was a discussion about this a while back. The decision was
to embed the WARNs, and that callers should not output any error
messages upon failure.
The x86 implementation of set_memory_decrypted() and
set_memory_encrypted() has an embedded WARN_ON_ONCE()
that triggers on a failure. The arm64 realm encrypt/decrypt ops also
have an embedded WARN().
The pkvm encrypt/decrypt ops do not, so that's maybe where it should
be added.
Michael
^ permalink raw reply
* Re: [PATCH v7 11/22] dma-pool: track decrypted atomic pools and select them via attrs
From: Jason Gunthorpe @ 2026-07-14 12:26 UTC (permalink / raw)
To: Aneesh Kumar K.V
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, Mostafa Saleh,
Petr Tesarik, 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: <yq5aqzl6mc00.fsf@kernel.org>
On Tue, Jul 14, 2026 at 09:57:43AM +0530, Aneesh Kumar K.V wrote:
> >
> > But I don't view this as that important, the CC hypervisor is probably
> > going to use the S2 page table to force cachable on all system memory
> > so the non-cached pgprot is a NOP, but the extra vmap is wasteful and
> > it is confusing.. So maybe a little fixme is all that is needed here.
> >
>
> Something like?
Yeah
Jason
^ permalink raw reply
* Re: [PATCH v7 11/22] dma-pool: track decrypted atomic pools and select them via attrs
From: Jason Gunthorpe @ 2026-07-14 12:25 UTC (permalink / raw)
To: Aneesh Kumar K.V
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, Mostafa Saleh,
Petr Tesarik, 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: <yq5atsq2md64.fsf@kernel.org>
On Tue, Jul 14, 2026 at 09:32:27AM +0530, Aneesh Kumar K.V wrote:
> If we want to warn about such failures, we should add the warning
> consistently across the code. We may also want to handle decrypt
> failures by encrypting the page again to avoid leaking it.
>
> I will work on that as a tree-wide change in a separate patch.
IMHO the WARN should be inside set_memory_encrypted(), and maybe it
should be a BUG_ON...
"free" functions shouldn't fail, and returning the memory back to
normal is a "free" operation.
Jason
^ permalink raw reply
* Re: [PATCH v7 04/22] dma: free atomic pool pages by physical address
From: Jason Gunthorpe @ 2026-07-14 12:23 UTC (permalink / raw)
To: Aneesh Kumar K.V
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, Mostafa Saleh,
Petr Tesarik, 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: <yq5awluymdf6.fsf@kernel.org>
On Tue, Jul 14, 2026 at 09:27:01AM +0530, Aneesh Kumar K.V wrote:
> Jason Gunthorpe <jgg@ziepe.ca> writes:
>
> > On Wed, Jul 01, 2026 at 11:19:08AM +0530, Aneesh Kumar K.V (Arm) wrote:
> >> dma_direct_alloc_pages() may satisfy atomic allocations from the coherent
> >> atomic pools. The pool allocation is keyed by the virtual address stored in
> >> the gen_pool, but the pages API returns only the backing struct page.
> >>
> >> On architectures with CONFIG_DMA_DIRECT_REMAP, atomic pool chunks are added
> >> to the gen_pool using their remapped virtual address.
> >> dma_direct_free_pages() reconstructs a linear-map address with
> >> page_address(page) and passes that to dma_free_from_pool(). That address
> >> does not match the gen_pool virtual range, so the pool lookup can fail and
> >> the code can fall through to freeing a pool-owned page through the normal
> >> page allocator path.
> >>
> >> Add a page-based pool free helper that looks up the owning pool chunk by
> >> physical address, translates it back to the gen_pool virtual address, and
> >> frees that address to the pool. Use it from dma_direct_free_pages() while
> >> keeping the existing virtual-address helper for coherent allocation frees.
> >>
> >> 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/dma-map-ops.h | 1 +
> >> kernel/dma/direct.c | 4 +--
> >> kernel/dma/pool.c | 54 +++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 57 insertions(+), 2 deletions(-)
> >
> > This seems pretty suboptimal?
> >
> > If !CONFIG_DMA_DIRECT_REMAP then page_to_virt() was used to compute
> > the genpool's addr so dma_free_from_pool_page() can use the same
> > logic, which is how things must be working at all today
> >
> > The CONFIG_DMA_DIRECT_REMAP scenario does look broken, so I'm
> > surprised there isn't a Fixes line on this commit? I don't have an
> > opinion on the search, but since alloc_pages() is used there is 8
> > bytes in the struct page that could be used to store the remapped
> > vaddr to avoid the search if someday someone wants to improve
> > this. Maybe a small comment hinting that direction would be a nice
> > addition.
>
> Something like
>
> +/*
> + * FIXME!! We could avoid this by storing the remapped virtual address in
> + * struct page and using that for lookup.
> + */
> bool dma_free_from_pool_page(struct device *dev, struct page *page, size_t size)
Plus some if (!IS_ENABLED()) that does the direct lookup
I would just use : not !! :)
Jason
^ permalink raw reply
* Re: [PATCH v7 02/22] dma-pool: fix page leak in atomic_pool_expand() cleanup
From: Jason Gunthorpe @ 2026-07-14 12:22 UTC (permalink / raw)
To: Aneesh Kumar K.V
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, Mostafa Saleh,
Petr Tesarik, 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: <yq5azezumef6.fsf@kernel.org>
On Tue, Jul 14, 2026 at 09:05:25AM +0530, Aneesh Kumar K.V wrote:
> Jason Gunthorpe <jgg@ziepe.ca> writes:
>
> > On Wed, Jul 01, 2026 at 11:19:06AM +0530, Aneesh Kumar K.V (Arm) wrote:
> >> @@ -115,8 +116,10 @@ static int atomic_pool_expand(struct gen_pool *pool, size_t pool_size,
> >> */
> >> ret = set_memory_decrypted((unsigned long)page_to_virt(page),
> >> 1 << order);
> >> - if (ret)
> >> + if (ret) {
> >> + leak_pages = true;
> >> goto remove_mapping;
> >> + }
> >
> > Truely these _set_memory_decrypted() things are an insane API. So a if
> > it fails to decrypt it can be in any messy state?
> >
>
> Yes, we could possibly try to encrypt the page again and, if that
> succeeds, avoid leaking it. We might want to do that tree-wide in a
> separate patch.
IMHO it is a horrid API if failure leaves thing in an indeterminate
state. For something like this I don't see why the arch FW implementation
would be unable to restore things back to as they were on failure.
But whatever, everything about set_memory_xx is really bad it could
use a cleaning
Jason
^ permalink raw reply
* Re: [PATCH v2] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Xu Yilun @ 2026-07-14 11:18 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: x86, linux-kernel, rick.p.edgecombe, dave.hansen, dave.hansen,
yilun.xu, chao.gao, djbw, linux-coco, peter.fang, xiaoyao.li
In-Reply-To: <alTMhIWnRgrqatUK@thinkstation>
On Mon, Jul 13, 2026 at 12:38:02PM +0100, Kiryl Shutsemau wrote:
> On Mon, Jul 13, 2026 at 05:21:01PM +0800, Xu Yilun wrote:
> > > The commit message covers the mechanics, but is silent on compatibility
> > > with older TDX modules. That context matters for the design choice, so
> > > regardless of where the implementation discussion lands, I think it
> > > needs to be spelled out here.
> > >
> > > Who is responsible for picking a version the module supports?
> > >
> > > TDH.VP.INIT can hardcode version 1 only because KVM already refuses to
> > > enable TDX on modules without TOPOLOGY_ENUM.
> > >
> > > The TDH.SYS.UPDATE user from [1] is the opposite case: it has to pick
> > > version 0 or 1 at runtime depending on whether add-on features are
> > > configured, to keep working on modules that don't support them.
> > >
> > > The second point is the actual argument for a version field in struct
> > > tdx_module_args rather than encoding the version in the leaf defines:
> > > the version is not always a compile-time property of the call site.
> >
> > That's good point. I'll add the TDX module compatibility argument in
> > changelog:
> >
> > One concern is the compatibility with older TDX modules which don't
> > recognize the new SEAMCALLs. The kernel should decide which SEAMCALL
> > version to use at runtime, selecting the minimum version number for the
> > required functionality. It can't overwrite the function number with a
> > new value at compile time.
>
> There are two policies, not one: make the new version a hard
> requirement at init time as we did with TDH.VP.INIT, or fall back to
> version 0 at runtime like your TDH.SYS.UPDATE user does. Both are valid,
> and the changelog should not claim runtime selection is the rule.
Maybe we don't need 2 policies. This first policy is only helpful if we
don't keep version history at all, we always update to the newest (from
Linux POV) version and forget about old module compatibility.
Otherwise, we use the 2nd policy. Vx as the first version accepted by Linux,
Vy as the latest version accepted by Linux. We should maintain
module compatibility between [Vy, Vx]. TDH.VP.INIT has only V1
because the accepted version range is [V1, V1]. If there is a case in
future that requires TDH.VP.INIT v2, then it changes to [V1, V2].
TDH.SYS.UPDATE V0 is the first accepted version, so it will be [V0, V1].
I don't think we have to use different policies for each SEAMCALL,
1st policy is simple but I see problem - the newest module is not always
available. 2nd policy has better compatibility. Now I sort of like 2nd
policy - the runtime selection.
>
> BTW, we might eventually switch TDH.SYS.UPDATE to hardcoded v1 once we
> stop caring about older modules.
Maybe we don't have to. If we want compatibility, I don't see immediate
benifit to shrink the compatibility.
[...]
>
> > > Without TDH.SYS.UPDATE context, the patch seems pointless.
> >
> > mm.. It's true one target is to address the compatibility with old
> > module. But is the other concern valid?
> >
> > Another concern is the obscure usage of the 'fn' parameter for seamcall
> > wrappers. An existing caller for TDH.VP.INIT packs the version into the
> > 'fn' to match the low-level TDX ABI RAX layout. The RAX layout
> > interprets some bits differently, such as INTERRUPT_MODE, SEAMLDR flag,
> > which are not a good fit for the function number definition.
> >
> > Is this a blocker for you as an independent cleanup patch?
>
> Not a blocker. But respinning now seems premature: the shape of v3
> depends entirely on where the RAX composition discussion lands --
> args.version with FIELD_MODIFY(), a version macro in 'fn', or Dave's
> rax-in-the-struct. I'd wait for that to settle first.
Thanks for clarification.
^ permalink raw reply
* Re: [PATCH v7 00/22] dma-mapping: Track shared DMA state through direct, pool and swiotlb paths
From: Marek Szyprowski @ 2026-07-14 10:00 UTC (permalink / raw)
To: Aneesh Kumar K.V, Jason Gunthorpe
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Will Deacon, Marc Zyngier, Steven Price, Suzuki K Poulose,
Catalin Marinas, Jiri Pirko, Mostafa Saleh, Petr Tesarik,
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
In-Reply-To: <yq5aldbdnbj8.fsf@kernel.org>
On 14.07.2026 11:52, Aneesh Kumar K.V wrote:
> Marek Szyprowski <m.szyprowski@samsung.com> writes:
>> On 13.07.2026 21:43, Jason Gunthorpe wrote:
>>> On Tue, Jul 07, 2026 at 03:03:48PM +0200, Marek Szyprowski wrote:
>>>> I'm fine with merging on top of the topic branch and I assume that this
>>>> patchset is mature enough to give it a try in linux-next, but first I
>>>> would like to get a review or at least acks from others with good CC
>>>> knowledge or experience.
>>> I think it is good to go from a CC perspective, there are still some
>>> more items to fix up (like the MMIO) but I'd rather they be followups
>>> at this point.
>> Great. Aneesh, any chance You could send v8 with the remaining items
>> fixed till end of this week? I will be happy to push it to
>> dma-mapping-for-next for testing in linux-next asap.
>>
> Sure. You need this rebased on top of the pKVM changes, right?
> Otherwise, we'll end up with conflicts when you do the final pull
> request.
Ah, indeed. I forgot about the pKVM changes. If there is a stable
branch with those patches, please base v8 on top of it.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH v7 00/22] dma-mapping: Track shared DMA state through direct, pool and swiotlb paths
From: Aneesh Kumar K.V @ 2026-07-14 9:52 UTC (permalink / raw)
To: Marek Szyprowski, Jason Gunthorpe
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Will Deacon, Marc Zyngier, Steven Price, Suzuki K Poulose,
Catalin Marinas, Jiri Pirko, Mostafa Saleh, Petr Tesarik,
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
In-Reply-To: <cd5d0504-1592-4b48-89ea-1efe8aa57968@samsung.com>
Marek Szyprowski <m.szyprowski@samsung.com> writes:
> On 13.07.2026 21:43, Jason Gunthorpe wrote:
>> On Tue, Jul 07, 2026 at 03:03:48PM +0200, Marek Szyprowski wrote:
>>> I'm fine with merging on top of the topic branch and I assume that this
>>> patchset is mature enough to give it a try in linux-next, but first I
>>> would like to get a review or at least acks from others with good CC
>>> knowledge or experience.
>> I think it is good to go from a CC perspective, there are still some
>> more items to fix up (like the MMIO) but I'd rather they be followups
>> at this point.
>
> Great. Aneesh, any chance You could send v8 with the remaining items
> fixed till end of this week? I will be happy to push it to
> dma-mapping-for-next for testing in linux-next asap.
>
Sure. You need this rebased on top of the pKVM changes, right?
Otherwise, we'll end up with conflicts when you do the final pull
request.
-aneesh
^ permalink raw reply
* Re: [RFC PATCH 10/30] vfio/pci: Export vfio dma-buf specific info for importers
From: Fuad Tabba @ 2026-07-14 6:38 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Ackerley Tng, Xu Yilun, kvm, sumit.semwal, christian.koenig,
pbonzini, seanjc, alex.williamson, dan.j.williams, aik,
linux-coco, dri-devel, linux-media, linaro-mm-sig,
vivek.kasireddy, yilun.xu, linux-kernel, lukas, yan.y.zhao,
daniel.vetter, leon, baolu.lu, zhenzhong.duan, tao1.su, linux-pci,
zhiw, simona.vetter, shameerali.kolothum.thodi, aneesh.kumar,
iommu, kevin.tian
In-Reply-To: <20260713191727.GH674038@nvidia.com>
On Mon, 13 Jul 2026 at 20:17, Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Mon, Jul 13, 2026 at 08:08:14PM +0100, Fuad Tabba wrote:
> > On Sun, 12 Jul 2026 at 02:02, Ackerley Tng <ackerleytng@google.com> wrote:
> > >
> > > Jason Gunthorpe <jgg@nvidia.com> writes:
> > >
> > > > On Thu, May 29, 2025 at 01:34:53PM +0800, Xu Yilun wrote:
> > > >> Export vfio dma-buf specific info by attaching vfio_dma_buf_data in
> > > >> struct dma_buf::priv. Provide a helper vfio_dma_buf_get_data() for
> > > >> importers to fetch these data. Exporters identify VFIO dma-buf by
> > > >> successfully getting these data.
> > > >>
> > > >> VFIO dma-buf supports disabling host access to these exported MMIO
> > > >> regions when the device is converted to private. Exporters like KVM
> > > >> need to identify this type of dma-buf to decide if it is good to use.
> > > >> KVM only allows host unaccessible MMIO regions been mapped in private
> > > >> roots.
> > > >>
> > > >> Export struct kvm * handler attached to the vfio device. This
> > > >> allows KVM to do another sanity check. MMIO should only be assigned to
> > > >> a CoCo VM if its owner device is already assigned to the same VM.
> > > >
> > > > This doesn't seem right, it should be encapsulated into the standard
> > > > DMABUF API in some way.
> > > >
> > >
> > > I'd like to propose an alternative. I've been working on guest_memfd and
> > > new to the world of IO, please help me along! :)
> > >
> > > It seems like using dmabufs are used a little awkwardly here. IIUC
> > > dmabufs were originally meant to expose memory of one device to another
> > > device, mostly meant to share memory. Dmabufs do expose MMIO too, for
> > > device to device communications. Without virtualization, userspace MMIO
> > > would be done by mmap()-ing a VFIO fd and having the userspace program
> > > write to the userspace addresses.
> > >
> > > Before CoCo, device passthrough (MMIO) is mostly handled by mmap()-ing a
> > > VFIO fd and setting up the userspace address in a KVM memslot for the
> > > guest.
> > >
> > > With CoCo, is the problem we're solving that we want KVM to know what
> > > pfns to set up in stage 2 page tables, but not via userspace addresses?
> > >
> > > guest_memfd already does that for regular host memory, tracks the
> > > private/shared-ness of the memory, tracks which struct kvm the memory
> > > belongs to.
> > >
> > > guest_memfd functions as KVM's bridge to host memory. KVM already can
> > > ask guest_memfd for the pfn to map into stage 2 page tables, and already
> > > asks guest_memfd for the shared/private state of the memory. guest_memfd
> > > already also blocks the host from faulting guest private memory
> > > (mmap()-ing is always allowed).
> > >
> > >
> > > Instead of using dmabuf as the intermediary between the MMIO PFNs and
> > > KVM, why not use guest_memfd?
> > >
> > > What if we make guest_memfd accept a VFIO fd, or a dmabuf fd?
> >
> > This is interesting for pKVM too, provided it covers more than MMIO.
> >
> > We need guest_memfd to be backable by a dmabuf for ordinary guest memory, not
> > only for device MMIO. There is mobile hardware that doesn't tolerate scattered
> > private memory (DMA engines that can't gather, IOMMU page-table size
> > constraints), and a CMA-backed dmabuf heap is the practical way to get
> > contiguous memory at runtime.
>
> Why can't guestmemfd allocate directly from CMA? Allocating struct
> page memory through dmabuf just to put it back in a guestmemfd sounds
> very ugly to me.
Fair, and I think you're right. If guest_memfd can allocate from CMA directly
that covers what we need for contiguous guest memory, and it's cleaner than
routing it through a dmabuf. It also keeps the shared pages struct-page backed
and GUP-able, which the CMA heap's own mmap doesn't, since it sets VM_PFNMAP.
So going through a dmabuf for plain guest RAM would have cost us the thing we
need on the shared side anyway.
Consider the request to be for guest_memfd to be able to give us physically
contiguous memory. dmabuf was the mechanism I assumed, not the requirement.
> > HugeTLB doesn't help, it wants boot-time
> > reservation. Those pages are struct-page backed, so it's a different problem
> > from the non-struct-page MMIO case, and the shared parts still need to be
> > GUP-able.
>
> Isn't dmabuf pretty allergic to mmaping refcounted struct page backed
> memory since that wrecks its lifetime model?
Yes, and that's the same point. Agreed.
> > More important for the API shape: conversions have to work on subsets of such a
> > region, at page granularity. A pKVM guest doesn't know what backs its memory, so
> > it will issue share/unshare hypercalls over arbitrary ranges of whatever it was
> > given. If a dmabuf-backed guest_memfd can only be converted as a whole, we can't
> > use it for memory, and the guest can't be taught to care.
>
> More reasons not to involve DMABUF since guestmemfd already does all
> of this...
Also agreed for guest RAM.
Where I do still think a dmabuf is involved is the case where the buffer isn't
guest_memfd's to allocate: it already belongs to another exporter, and the guest
needs to see that same buffer. That's structurally what you're already handling
for device memory rather than a separate guest_memfd-over-dmabuf path, so I
don't think it argues for backing ordinary guest memory with a dmabuf.
> Jason
^ permalink raw reply
* Re: [PATCH 2/2] virt: tdx-guest: Allocate Quote buffer dynamically
From: Peter Fang @ 2026-07-14 6:25 UTC (permalink / raw)
To: Kuppuswamy Sathyanarayanan
Cc: Dave Hansen, Kiryl Shutsemau, Rick Edgecombe, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, linux-kernel,
linux-coco, kvm
In-Reply-To: <283d7cc7-e87e-4d4e-b1d9-ae19725e1606@linux.intel.com>
On Mon, Jul 13, 2026 at 10:27:46AM -0700, Kuppuswamy Sathyanarayanan wrote:
> Hi Peter,
>
> On 6/12/2026 4:08 AM, Peter Fang wrote:
> > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> >
> > The TDX attestation driver currently uses a fixed 128 KB Quote buffer
> > shared with the host VMM. This may be too small for Quotes using schemes
> > such as post-quantum cryptography (PQC), where certificate chains can
> > increase the Quote size to several megabytes.
> >
> > Allocate the Quote buffer based on the size reported by the TDX module
> > instead of always reserving a fixed-size buffer. This avoids wasting
> > memory on platforms that do not require larger Quotes. Older platforms
> > fall back to the default 128 KB buffer.
> >
> > Because the Quote buffer must be physically contiguous, its size is
> > bound by the buddy allocator's maximum page order (4 MB), which should
> > be sufficient for current attestation needs.
> >
> > struct tdx_quote_buf has a trailing flexible array, so use offsetof()
> > instead of sizeof() to calculate the header size.
> >
> > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> > Assisted-by: Claude:claude-opus-4-7
> > Assisted-by: GitHub Copilot:gpt-5.4
> > Signed-off-by: Peter Fang <peter.fang@intel.com>
> > ---
>
> Looks good to me.
>
> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Thanks Sathya!
>
>
> > drivers/virt/coco/tdx-guest/tdx-guest.c | 52 ++++++++++++++++++-------
> > 1 file changed, 38 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
> > index a9ecc46df187..162fb47f3fae 100644
> > --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> > +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
> > @@ -163,7 +163,7 @@ static void tdx_mr_deinit(const struct attribute_group *mr_grp)
> > * DICE-based attestation uses layered evidence that requires
> > * larger Quote size (~100K).
> > */
> > -#define GET_QUOTE_BUF_SIZE SZ_128K
> > +#define GET_QUOTE_DEFAULT_BUF_SIZE SZ_128K
> >
> > #define GET_QUOTE_CMD_VER 1
> >
> > @@ -171,7 +171,7 @@ static void tdx_mr_deinit(const struct attribute_group *mr_grp)
> > #define GET_QUOTE_SUCCESS 0
> > #define GET_QUOTE_IN_FLIGHT 0xffffffffffffffff
> >
> > -#define TDX_QUOTE_MAX_LEN (GET_QUOTE_BUF_SIZE - sizeof(struct tdx_quote_buf))
> > +#define TDX_QUOTE_BUF_LEN(n) (offsetof(struct tdx_quote_buf, data) + (n))
> >
> > /* struct tdx_quote_buf: Format of Quote request buffer.
> > * @version: Quote format version, filled by TD.
> > @@ -192,8 +192,9 @@ struct tdx_quote_buf {
> > u8 data[];
> > };
> >
> > -/* Quote data buffer */
> > +/* Quote data buffer and size */
> > static void *quote_data;
> > +static size_t quote_data_size;
> >
[ ... ]
> >
> > @@ -286,7 +310,7 @@ static int tdx_report_new_locked(struct tsm_report *report, void *data)
> > if (desc->inblob_len != TDX_REPORTDATA_LEN)
> > return -EINVAL;
> >
> > - memset(quote_data, 0, GET_QUOTE_BUF_SIZE);
> > + memset(quote_data, 0, quote_data_size);
> >
> > /* Update Quote buffer header */
> > quote_buf->version = GET_QUOTE_CMD_VER;
> > @@ -297,7 +321,7 @@ static int tdx_report_new_locked(struct tsm_report *report, void *data)
> > if (ret)
> > return ret;
> >
> > - err = tdx_hcall_get_quote(quote_data, GET_QUOTE_BUF_SIZE);
> > + err = tdx_hcall_get_quote(quote_data, quote_data_size);
> > if (err) {
> > pr_err("GetQuote hypercall failed, status:%llx\n", err);
> > return -EIO;
> > @@ -316,7 +340,7 @@ static int tdx_report_new_locked(struct tsm_report *report, void *data)
> >
> > out_len = READ_ONCE(quote_buf->out_len);
> >
> > - if (out_len > TDX_QUOTE_MAX_LEN)
> > + if (TDX_QUOTE_BUF_LEN(out_len) > quote_data_size)
> > return -EFBIG;
>
> Nit: I think this check will be more readable if you can rename
> quote_data_size to quote_buf_size (since it holds total buffer
> size).
Hm. This pairs with the original "static void *quote_data". Or perhaps
"quote_data_len"?
>
^ permalink raw reply
* Re: [PATCH 2/2] virt: tdx-guest: Allocate Quote buffer dynamically
From: Peter Fang @ 2026-07-14 6:18 UTC (permalink / raw)
To: Binbin Wu
Cc: Dave Hansen, Kiryl Shutsemau, Rick Edgecombe,
Kuppuswamy Sathyanarayanan, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin, linux-kernel, linux-coco,
kvm
In-Reply-To: <62069ec6-b586-446a-a696-6aa3356ac665@linux.intel.com>
On Fri, Jul 10, 2026 at 11:32:55AM +0800, Binbin Wu wrote:
> On 6/12/2026 7:08 PM, Peter Fang wrote:
> > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> >
> > The TDX attestation driver currently uses a fixed 128 KB Quote buffer
> > shared with the host VMM. This may be too small for Quotes using schemes
> > such as post-quantum cryptography (PQC), where certificate chains can
> > increase the Quote size to several megabytes.
>
> Nit:
> I think the description here should be more accurate.
> "Several megabytes" colloquially implies sizes that could easily exceed 4 MB.
> And it's a slight contradiction VS the 4MB limitation below.
Good point. I'll make this clearer.
>
> >
> > Allocate the Quote buffer based on the size reported by the TDX module
> > instead of always reserving a fixed-size buffer. This avoids wasting
> > memory on platforms that do not require larger Quotes. Older platforms
> > fall back to the default 128 KB buffer.
> >
> > Because the Quote buffer must be physically contiguous, its size is
> > bound by the buddy allocator's maximum page order (4 MB), which should
> > be sufficient for current attestation needs.
> >
> > struct tdx_quote_buf has a trailing flexible array, so use offsetof()
> > instead of sizeof() to calculate the header size.
> >
> > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> > Assisted-by: Claude:claude-opus-4-7
> > Assisted-by: GitHub Copilot:gpt-5.4
>
> Same issue as pointed by Kiryl in the first patch.
There have been discussions about this recently [1]. I think perhaps I
should drop the "Assisted-by" tags since I did not use AI to write the
actual code.
[1] https://lore.kernel.org/linux-coco/akxHbKbMyF7PZ0Av@google.com/
>
> > Signed-off-by: Peter Fang <peter.fang@intel.com>
>
> Also the use of struct_size_t() instead of offsetof().
Yep.
>
> Otherwise looks okay to me:
>
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Thanks for the review Binbin!
^ permalink raw reply
* Re: [PATCH v7 00/22] dma-mapping: Track shared DMA state through direct, pool and swiotlb paths
From: Marek Szyprowski @ 2026-07-14 6:06 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Aneesh Kumar K.V, iommu, linux-arm-kernel, linux-kernel,
linux-coco, Robin Murphy, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Mostafa Saleh,
Petr Tesarik, 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
In-Reply-To: <20260713194323.GC3133966@ziepe.ca>
On 13.07.2026 21:43, Jason Gunthorpe wrote:
> On Tue, Jul 07, 2026 at 03:03:48PM +0200, Marek Szyprowski wrote:
>> I'm fine with merging on top of the topic branch and I assume that this
>> patchset is mature enough to give it a try in linux-next, but first I
>> would like to get a review or at least acks from others with good CC
>> knowledge or experience.
> I think it is good to go from a CC perspective, there are still some
> more items to fix up (like the MMIO) but I'd rather they be followups
> at this point.
Great. Aneesh, any chance You could send v8 with the remaining items fixed till end of this week? I will be happy to push it to dma-mapping-for-next for testing in linux-next asap.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH 1/2] x86/tdx: Add helper to query maximum TD Quote size
From: Peter Fang @ 2026-07-14 5:56 UTC (permalink / raw)
To: Kuppuswamy Sathyanarayanan
Cc: Dave Hansen, Kiryl Shutsemau, Rick Edgecombe, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, linux-kernel,
linux-coco, kvm
In-Reply-To: <680a2be7-5a10-448d-8552-cb9ef64b6be7@linux.intel.com>
On Mon, Jul 13, 2026 at 09:01:38AM -0700, Kuppuswamy Sathyanarayanan wrote:
>
>
> On 6/12/2026 4:08 AM, Peter Fang wrote:
> > TDX attestation blob ("TD Quote") sizes can grow with newer
> > cryptographic schemes, so guests can no longer rely on a fixed-size
> > buffer for the Quote.
> >
> > Newer TDX modules report the maximum TD Quote size via a TD-scope
> > metadata field. Add a helper to query it instead of exposing tdg_vm_rd()
> > directly, as it can read arbitrary metadata fields.
> >
> > Thanks to Xu Yilun for suggesting this.
>
> I'm not sure what the original suggestion was. Perhaps a link to the discussion
> (or Xu Yilun's email) would be helpful for context.
It was an internal discussion. I can add that to the changelog.
>
> >
> > Assisted-by: Claude:claude-opus-4-7
> > Assisted-by: GitHub Copilot:gpt-5.4
> > Signed-off-by: Peter Fang <peter.fang@intel.com>
> > ---
>
> Looks good to me.
>
> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>
Thanks Sathya!
^ permalink raw reply
* Re: [PATCH 1/2] x86/tdx: Add helper to query maximum TD Quote size
From: Peter Fang @ 2026-07-14 5:54 UTC (permalink / raw)
To: Binbin Wu
Cc: Kiryl Shutsemau, Dave Hansen, Rick Edgecombe,
Kuppuswamy Sathyanarayanan, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin, linux-kernel, linux-coco,
kvm
In-Reply-To: <a95a286c-5fa9-47f7-8a9a-4ecf3aafdfee@linux.intel.com>
On Fri, Jul 10, 2026 at 10:38:40AM +0800, Binbin Wu wrote:
> On 6/12/2026 8:36 PM, Kiryl Shutsemau wrote:
> >>
> >> +/**
> >> + * tdx_get_max_quote_size() - Get the maximum TD Quote size
> >> + *
> >> + * Read the maximum size of a TD Quote from a 4-byte TD metadata field. The TDX
> >> + * guest driver uses it to size the buffer for Quote retrieval. Older TDX
> >> + * modules do not support this field and return an error.
> >> + *
> >> + * Return: Maximum Quote size in bytes on success, or 0 on failure.
> >> + */
> >> +u32 tdx_get_max_quote_size(void)
> >> +{
> >> + u64 val, ret;
> >> +
> >> + ret = tdg_vm_rd(TDCS_QUOTE_MAX_SIZE, &val);
> >> +
> >> + return ret ? 0 : (u32)val;
> >
> > Cast is redundant.
>
> I thought in Linux kernel, the explicit cast is often preferred?
> E.g. as documentation of the metadata field’s width here?
Hm... I asked AI to find some patterns in arch/x86, and looks
like the explicit cast is more common... Kiryl are you okay with me
keeping the cast?
>
^ permalink raw reply
* Re: [PATCH v7 20/22] dma: swiotlb: free dynamic pools from process context
From: Aneesh Kumar K.V @ 2026-07-14 5:03 UTC (permalink / raw)
To: Jason Gunthorpe
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, Mostafa Saleh,
Petr Tesarik, 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: <20260713194131.GZ3133966@ziepe.ca>
Jason Gunthorpe <jgg@ziepe.ca> writes:
> On Wed, Jul 01, 2026 at 11:19:24AM +0530, Aneesh Kumar K.V (Arm) wrote:
>
>> @@ -840,7 +843,7 @@ static void swiotlb_del_pool(struct device *dev, struct io_tlb_pool *pool)
>> list_del_rcu(&pool->node);
>> spin_unlock_irqrestore(&dev->dma_io_tlb_lock, flags);
>>
>> - call_rcu(&pool->rcu, swiotlb_dyn_free);
>> + swiotlb_schedule_dyn_free(pool);
>
> This knows if the pool has cc_shared memory in it since there is now a
> flag tracking that? So should the work queue version be conditional on
> that flag?
>
We could definitely do that. I was not sure whether it was worth it,
considering that it would add two paths for SWIOTLB dynamic free.
-aneesh
^ permalink raw reply
* Re: [PATCH v14 26/44] arm64: RMI: Allow populating initial contents
From: Kohei Enju @ 2026-07-14 4:37 UTC (permalink / raw)
To: Steven Price
Cc: kvm, kvmarm, Catalin Marinas, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni,
Gavin Shan, Shanker Donthineni, Alper Gun, Aneesh Kumar K . V,
Emi Kisanuki, Vishal Annapurve, WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <20260513131757.116630-27-steven.price@arm.com>
On 05/13 14:17, 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>
> ---
> Changes since v13:
> * Rename realm_create_protected_data_page() to realm_data_map_init().
> Changes since v12:
> * The ioctl now updates the structure with the amount populated rather
> than returning this through the ioctl return code.
> * Use the new RMM v2.0 range based RMI calls.
> * Adapt to upstream changes in kvm_gmem_populate().
> Changes since v11:
> * The multiplex CAP is gone and there's a new ioctl which makes use of
> the generic kvm_gmem_populate() functionality.
> Changes since v7:
> * Improve the error codes.
> * Other minor changes from review.
> Changes since v6:
> * Handle host potentially having a larger page size than the RMM
> granule.
> * Drop historic "par" (protected address range) from
> populate_par_region() - it doesn't exist within the current
> architecture.
> * Add a cond_resched() call in kvm_populate_realm().
> Changes since v5:
> * Refactor to use PFNs rather than tracking struct page in
> realm_create_protected_data_page().
> * Pull changes from a later patch (in the v5 series) for accessing
> pages from a guest memfd.
> * Do the populate in chunks to avoid holding locks for too long and
> triggering RCU stall warnings.
> ---
> arch/arm64/include/asm/kvm_rmi.h | 4 ++
> arch/arm64/kvm/Kconfig | 1 +
> arch/arm64/kvm/arm.c | 13 ++++
> arch/arm64/kvm/rmi.c | 106 +++++++++++++++++++++++++++++++
> 4 files changed, 124 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_rmi.h b/arch/arm64/include/asm/kvm_rmi.h
> index 007249a13dbc..a2b6bc412a22 100644
> --- a/arch/arm64/include/asm/kvm_rmi.h
> +++ b/arch/arm64/include/asm/kvm_rmi.h
> @@ -88,6 +88,10 @@ int kvm_rec_enter(struct kvm_vcpu *vcpu);
> int kvm_rec_pre_enter(struct kvm_vcpu *vcpu);
> int handle_rec_exit(struct kvm_vcpu *vcpu, int rec_run_status);
>
> +struct kvm_arm_rmi_populate;
> +
> +int kvm_arm_rmi_populate(struct kvm *kvm,
> + struct kvm_arm_rmi_populate *arg);
> void kvm_realm_unmap_range(struct kvm *kvm,
> unsigned long ipa,
> unsigned long size,
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index 4e16719fda22..d0cd011cf672 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -38,6 +38,7 @@ menuconfig KVM
> select GUEST_PERF_EVENTS if PERF_EVENTS
> select KVM_GUEST_MEMFD
> select KVM_GENERIC_MEMORY_ATTRIBUTES
> + select HAVE_KVM_ARCH_GMEM_POPULATE
> help
> Support hosting virtualized guest machines.
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index ed88a203b892..073ba9181da9 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -2131,6 +2131,19 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> return -EFAULT;
> return kvm_vm_ioctl_get_reg_writable_masks(kvm, &range);
> }
> + case KVM_ARM_RMI_POPULATE: {
> + struct kvm_arm_rmi_populate req;
> + int ret;
> +
> + if (!kvm_is_realm(kvm))
> + return -ENXIO;
> + if (copy_from_user(&req, argp, sizeof(req)))
> + return -EFAULT;
> + ret = kvm_arm_rmi_populate(kvm, &req);
> + if (copy_to_user(argp, &req, sizeof(req)))
> + return -EFAULT;
> + return ret;
> + }
> default:
> return -EINVAL;
> }
> 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);
> +
> + ret = realm_create_rtt_levels(realm, ipa, level,
> + KVM_PGTABLE_LAST_LEVEL, NULL);
> + if (!ret) {
> + ret = rmi_rtt_data_map_init(rd, dst_phys, ipa, src_phys,
> + flags);
I think rmi_rtt_data_map_init() returns a raw RMI return value which is
positive on failure. Shouldn't it be converted to -ENXIO or another
Linux errno before returning it?
Otherwise, the positive error value can propagate through
kvm_gmem_populate() and populate_region() to kvm_arm_rmi_populate().
There, it would be treated as the number of pages populated, even though
the operation did not populate any pages.
> + }
> + }
> +
> + if (ret) {
> + if (WARN_ON(rmi_undelegate_page(dst_phys))) {
> + /* Undelegate failed, so we leak the page */
> + get_page(pfn_to_page(dst_pfn));
Since realm_create_rtt_levels() returns either 0 or a negative errno,
how about preserving that error and converting any raw RMI erorr to
-ENXIO here?
return ret < 0 ? ret : -ENXIO;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static int populate_region_cb(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> + struct page *src_page, void *opaque)
> +{
> + unsigned long data_flags = *(unsigned long *)opaque;
> + phys_addr_t ipa = gfn_to_gpa(gfn);
> +
> + if (!src_page)
> + return -EOPNOTSUPP;
> +
> + return realm_data_map_init(kvm, ipa, pfn, page_to_pfn(src_page),
> + data_flags);
> +}
> +
> +static long populate_region(struct kvm *kvm,
> + gfn_t base_gfn,
> + unsigned long pages,
> + u64 uaddr,
> + unsigned long data_flags)
> +{
> + long ret = 0;
> +
> + mutex_lock(&kvm->slots_lock);
> + ret = kvm_gmem_populate(kvm, base_gfn, u64_to_user_ptr(uaddr), pages,
> + populate_region_cb, &data_flags);
> + mutex_unlock(&kvm->slots_lock);
> +
> + return ret;
> +}
> +
> enum ripas_action {
> RIPAS_INIT,
> RIPAS_SET,
> @@ -574,6 +643,43 @@ static int realm_ensure_created(struct kvm *kvm)
> return -ENXIO;
> }
>
> +int kvm_arm_rmi_populate(struct kvm *kvm,
> + struct kvm_arm_rmi_populate *args)
> +{
> + unsigned long data_flags = 0;
> + unsigned long ipa_start = args->base;
> + unsigned long ipa_end = ipa_start + args->size;
> + long pages_populated;
> + int ret;
> +
> + if (args->reserved ||
> + (args->flags & ~KVM_ARM_RMI_POPULATE_FLAGS_MEASURE) ||
> + !IS_ALIGNED(ipa_start, PAGE_SIZE) ||
> + !IS_ALIGNED(ipa_end, PAGE_SIZE) ||
> + !IS_ALIGNED(args->source_uaddr, PAGE_SIZE))
> + return -EINVAL;
> +
> + ret = realm_ensure_created(kvm);
> + if (ret)
> + return ret;
If I understand correctly, concurrent KVM_ARM_RMI_POPULATE calls can
race with each other in realm_ensure_created().
realm_ensure_created() calls realm_create_rd() when kvm_realm_state() ==
REALM_STATE_NONE. Since kvm->arch.config_lock is not held here, multiple
threads could enter realm_create_rd() concurrently.
KVM_ARM_RMI_POPULATE can also race with KVM_RUN through
kvm_activate_realm(). kvm_activate_realm() takes kvm->arch.config_lock
while checking the Realm state, calling realm_ensure_created(), and
updating the state, whereas KVM_ARM_RMI_POPULATE does not.
How about serializing Realm creation here as follows?
scoped_guard(mutex, &kvm->arch.config_lock) {
ret = realm_ensure_created(kvm);
if (ret)
return ret;
}
Does that make sense?
> +
> + if (args->flags & KVM_ARM_RMI_POPULATE_FLAGS_MEASURE)
> + data_flags |= RMI_MEASURE_CONTENT;
> +
> + pages_populated = populate_region(kvm, gpa_to_gfn(ipa_start),
> + args->size >> PAGE_SHIFT,
> + args->source_uaddr, data_flags);
> +
> + if (pages_populated < 0)
> + return pages_populated;
> +
> + args->size -= pages_populated << PAGE_SHIFT;
> + args->source_uaddr += pages_populated << PAGE_SHIFT;
> + args->base += pages_populated << PAGE_SHIFT;
> +
> + return 0;
> +}
> +
> static void kvm_complete_ripas_change(struct kvm_vcpu *vcpu)
> {
> struct kvm *kvm = vcpu->kvm;
> --
> 2.43.0
>
>
^ permalink raw reply
* Re: [PATCH v7 11/22] dma-pool: track decrypted atomic pools and select them via attrs
From: Aneesh Kumar K.V @ 2026-07-14 4:27 UTC (permalink / raw)
To: Jason Gunthorpe
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, Mostafa Saleh,
Petr Tesarik, 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: <20260713184810.GM3133966@ziepe.ca>
Jason Gunthorpe <jgg@ziepe.ca> writes:
> On Wed, Jul 01, 2026 at 11:19:15AM +0530, Aneesh Kumar K.V (Arm) wrote:
>> @@ -114,14 +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) {
>> - leak_pages = true;
>> - goto remove_mapping;
>> + if (dma_pool->cc_shared) {
>> + ret = set_memory_decrypted((unsigned long)page_to_virt(page),
>> + 1 << order);
>> + if (ret) {
>> + leak_pages = true;
>> + goto remove_mapping;
>> + }
>> }
>
> This makes the memory_decrypted conditional, but it doesn't change
> the lines a few above:
>
> addr = dma_common_contiguous_remap(page, pool_size,
> pgprot_decrypted(pgprot_dmacoherent(PAGE_KERNEL)),
> ^^^^^^^^^^^^
> __builtin_return_address(0));
> if (!addr)
> goto free_page;
>
> It is wrong to pass pgprot_decrypted() to the arch code if
> set_memory_decrypted() was not called.
>
> Also it looks at some point the nature of the atomic pool has become
> confused. Originally it was just to allocate atomic memory that had
> been vmap'd outside the atomic context (to set the non-coherent
> pgprot), so every caller was expecting non-cached memory.
>
> Then it was reused to also allocate CC shared memory outside the
> atomic context. That was fine for x86 that doesn't use DMA_REMAP but
> on ARM64 it now means all atomic pool CC memory is uncached? That
> doesn't seem to make any sense...
>
> I suppose along the lines of this patch the solution is to add a
> noncoherent property to the pool so we can select the correct
> combination:
>
> noncoherent !SHARED = vmap pgprot_noncached
> !noncoherent SHARED= vmap pgprot_decrypted + set_memory_decrypted
> noncoherent SHARED = (probably unrealistic in real systems)
> !noncoherent !SHARED = normal __dma_direct_alloc_pages()
>
> But I don't view this as that important, the CC hypervisor is probably
> going to use the S2 page table to force cachable on all system memory
> so the non-cached pgprot is a NOP, but the extra vmap is wasteful and
> it is confusing.. So maybe a little fixme is all that is needed here.
>
Something like?
modified kernel/dma/direct.c
@@ -260,6 +260,9 @@ 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.
+ * FIXME!! With CONFIG_DMA_DIRECT_REMAP, the pool is also mapped as
+ * DMA-coherent (non-cacheable). We may want to create a separate pool
+ * dedicated to CC_SHARED atomic allocations.
*/
if ((remap || (attrs & __DMA_ATTR_ALLOC_CC_SHARED)) &&
dma_direct_use_pool(dev, gfp)) {
modified kernel/dma/pool.c
@@ -88,6 +88,7 @@ static int atomic_pool_expand(struct dma_gen_pool *dma_pool, size_t pool_size,
unsigned int order;
struct page *page = NULL;
bool leak_pages = false;
+ pgprot_t prot;
void *addr;
int ret = -ENOMEM;
unsigned int min_encrypt_order = get_order(mem_cc_shared_granule_size());
@@ -110,8 +111,12 @@ static int atomic_pool_expand(struct dma_gen_pool *dma_pool, size_t pool_size,
arch_dma_prep_coherent(page, pool_size);
#ifdef CONFIG_DMA_DIRECT_REMAP
- addr = dma_common_contiguous_remap(page, pool_size,
- pgprot_decrypted(pgprot_dmacoherent(PAGE_KERNEL)),
+ if (dma_pool->cc_shared)
+ prot = pgprot_decrypted(pgprot_dmacoherent(PAGE_KERNEL));
+ else
+ prot = pgprot_dmacoherent(PAGE_KERNEL);
+
+ addr = dma_common_contiguous_remap(page, pool_size, prot,
__builtin_return_address(0));
if (!addr)
goto free_page;
^ permalink raw reply
* Re: [PATCH v7 11/22] dma-pool: track decrypted atomic pools and select them via attrs
From: Aneesh Kumar K.V @ 2026-07-14 4:02 UTC (permalink / raw)
To: Jason Gunthorpe
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, Mostafa Saleh,
Petr Tesarik, 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: <20260713175616.GJ3133966@ziepe.ca>
Jason Gunthorpe <jgg@ziepe.ca> writes:
> On Wed, Jul 01, 2026 at 11:19:15AM +0530, Aneesh Kumar K.V (Arm) wrote:
>> -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;
>> @@ -114,14 +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) {
>> - leak_pages = true;
>> - goto remove_mapping;
>> + if (dma_pool->cc_shared) {
>> + 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(pool, (unsigned long)addr, page_to_phys(page),
>> - pool_size, NUMA_NO_NODE);
>> +
>> + 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;
>>
>> @@ -129,12 +138,10 @@ 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 */
>> + if (dma_pool->cc_shared &&
>> + set_memory_encrypted((unsigned long)page_to_virt(page), 1 << order))
>> leak_pages = true;
>> - }
>> +
>
> Was it intentional to remove the WARN_ON and comment ?
That WARN_ON() was a bit inconsistent. Not all page leaks due to
encrypt/decrypt failures triggered a warning, so I removed it.
If we want to warn about such failures, we should add the warning
consistently across the code. We may also want to handle decrypt
failures by encrypting the page again to avoid leaking it.
I will work on that as a tree-wide change in a separate patch.
-aneesh
^ permalink raw reply
* Re: [PATCH v7 04/22] dma: free atomic pool pages by physical address
From: Aneesh Kumar K.V @ 2026-07-14 3:57 UTC (permalink / raw)
To: Jason Gunthorpe
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, Mostafa Saleh,
Petr Tesarik, 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: <20260713181059.GL3133966@ziepe.ca>
Jason Gunthorpe <jgg@ziepe.ca> writes:
> On Wed, Jul 01, 2026 at 11:19:08AM +0530, Aneesh Kumar K.V (Arm) wrote:
>> dma_direct_alloc_pages() may satisfy atomic allocations from the coherent
>> atomic pools. The pool allocation is keyed by the virtual address stored in
>> the gen_pool, but the pages API returns only the backing struct page.
>>
>> On architectures with CONFIG_DMA_DIRECT_REMAP, atomic pool chunks are added
>> to the gen_pool using their remapped virtual address.
>> dma_direct_free_pages() reconstructs a linear-map address with
>> page_address(page) and passes that to dma_free_from_pool(). That address
>> does not match the gen_pool virtual range, so the pool lookup can fail and
>> the code can fall through to freeing a pool-owned page through the normal
>> page allocator path.
>>
>> Add a page-based pool free helper that looks up the owning pool chunk by
>> physical address, translates it back to the gen_pool virtual address, and
>> frees that address to the pool. Use it from dma_direct_free_pages() while
>> keeping the existing virtual-address helper for coherent allocation frees.
>>
>> 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/dma-map-ops.h | 1 +
>> kernel/dma/direct.c | 4 +--
>> kernel/dma/pool.c | 54 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 57 insertions(+), 2 deletions(-)
>
> This seems pretty suboptimal?
>
> If !CONFIG_DMA_DIRECT_REMAP then page_to_virt() was used to compute
> the genpool's addr so dma_free_from_pool_page() can use the same
> logic, which is how things must be working at all today
>
> The CONFIG_DMA_DIRECT_REMAP scenario does look broken, so I'm
> surprised there isn't a Fixes line on this commit? I don't have an
> opinion on the search, but since alloc_pages() is used there is 8
> bytes in the struct page that could be used to store the remapped
> vaddr to avoid the search if someday someone wants to improve
> this. Maybe a small comment hinting that direction would be a nice
> addition.
>
> Jason
>
Something like
+/*
+ * FIXME!! We could avoid this by storing the remapped virtual address in
+ * struct page and using that for lookup.
+ */
bool dma_free_from_pool_page(struct device *dev, struct page *page, size_t size)
-aneesh
^ 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