Re: [PATCH v8 23/46] KVM: TDX: Make source page optional for KVM_TDX_INIT_MEM_REGION
From: Sean Christopherson <seanjc@google.com>
Date: 2026-06-30 13:27:08
Also in:
kvm, linux-coco, linux-doc, linux-kselftest, linux-mm, lkml
On Tue, Jun 30, 2026, Yan Zhao wrote:
On Tue, Jun 30, 2026 at 08:35:49AM +0800, Sean Christopherson wrote:quoted
Gah, I thought I had sent this out this morning, long before Ackerley's response. But I got distracted by a meeting and forgot to get back to this... *sigh* Sending what I already wrote, even though there's a lot of overlap with Ackerley's mail. On Mon, Jun 29, 2026, Yan Zhao wrote:quoted
On Fri, Jun 26, 2026 at 08:28:32AM -0700, Ackerley Tng wrote:quoted
Yan Zhao [off-list ref] writes:quoted
But if a user configures 0 uaddr as valid, writes to it, and then passes 0 as source_addr(not from gmem), I'm not sure if it's good for the kernel to silently treat 0 uaddr as an identifier for in-place copy from the private PFN in gmem.I'd say the original uAPI perhaps just didn't document 0 as an unsupported uaddr. Given that commit 2a62345b3052 already merged, uAPI was perhaps accidentally changed and no customer complained, I think we can move forward with 0 as an invalid src_address? I wouldn't think anyone relies on 0 intentionally being a valid address. I could document that, if it helps?What about just documenting that 0 is an unsupported uaddr which will be re-purposed as an indicator to use the target pfn as the source, regardless of whether gmem_in_place_conversion is true? i.e., if (!src_page) src_page = pfn_to_page(pfn);Because KVM can't generally use the target page as the source without in-place conversion, it's not supported today, and out-of-place conversion is being deprecated.By "out-of-place conversion", do you mean using per-VM memory attribute conversion?
Yep, I couldn't come up with a better description.
quoted
quoted
I don't get why the two scenarios should be treated differently: 1. gmem_in_place_conversion==true, shared memory is not from gmem 2. gmem_in_place_conversion==false, shared memory is not from gmem In both case, a 0 uaddr could be mapped to a valid page not from gmem.That's immaterial. KVM's ABI (that we're solidifying) is that an address of '0' for the source means NULL. The fact that userspace could have a valid mapping at virtual address '0' is irrelevant.So, I'm wondering if we can document that 0 uaddr could always mean using target PFN.
I would document it as saying "no source page", and then state that a source page is required if in-place conversion isn't enabled/supported/allowed.
i.e., for both scenarios 1 and 2, al long as 0 uaddr is specified, we always use target PFN as source for in-place add.quoted
Again, just because something is technically possible doesn't mean it needs to be supported by every piece of KVM's uAPI.quoted
So why not update the uAPI to handle both cases consistently? :)Because retroactively adding support for out-of-place conversion is pointless (requires a userspace update for a feature that's being deprecated), KVM can't generally support using the source for out-of-place conversion (it's effectively an obscure zero-page optimization), and IMO rejecting the out-of-place conversion scenario is valuable for KVM developers, e.g. to help newcomers understand what exactly is and isn't possible.Ok. You mean per-VM memory attribute is deprecating, and source page from !gmem backend is also deprecating, so we don't want to change uAPI for scenarios under gmem_in_place_conversion==false. Right?
Right.
quoted
Side topic, isn't TDX broken if target page has already been added to the TD? IIUC, kvm_tdp_mmu_map_private_pfn() will be a glorified nop due to the page already having a valid S-EPT mapping, and so KVM will incorrectly allow a doubleNot sure if my understand out-of-place conversion correctly. Given target PFNs and GFNs are not duplicated, what would cause double add? :)
I was working through what would happen if userspace did KVM_TDX_INIT_MEM_REGION on the same target page multiple times.
quoted
add. Ahhh, no, because KVM will return RET_PF_SPURIOUS and kvm_tdp_mmu_map_private_pfn() will then return -EIO.My asking was if we could document uaddr always means using target PFN, since TDX's in-place add does not rely on gmem in-place conversion.
Yeah, I was on a tangent, ignore everything from "Side topic" on.