On Tue, Jun 30, 2026 at 09:27:06PM +0800, Sean Christopherson wrote: > 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. Ok.
> > 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. Oh. To have KVM_TDX_INIT_MEM_REGION on the same target page multiple times, the user needs to invoke KVM_TDX_INIT_MEM_REGION on the same GPA multiple times. In that case, yes, kvm_tdp_mmu_map_private_pfn() will return -EIO. > > > > > 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. >
