On Tue, Jun 30, 2026, Yan Zhao wrote: > 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?
Yep, I couldn't come up with a better description. > > > 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. > > > 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? 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? :) I was working through what would happen if userspace did KVM_TDX_INIT_MEM_REGION on the same target page multiple times. > > > 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.
