On Tue, Jun 30, 2026 at 08:35:49AM +0800, Sean Christopherson wrote: > 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: > > On Fri, Jun 26, 2026 at 08:28:32AM -0700, Ackerley Tng wrote: > > > Yan Zhao <[email protected]> writes: > > > > 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?
> > 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.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. > Again, just because something is technically possible doesn't mean it needs to > be supported by every piece of KVM's uAPI. > > > 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? > 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 > double Not sure if my understand out-of-place conversion correctly. Given target PFNs and GFNs are not duplicated, what would cause double add? :) > 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.
