* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Sean Christopherson @ 2026-04-02 20:47 UTC (permalink / raw)
To: Dave Hansen
Cc: Yan Zhao, pbonzini, dave.hansen, tglx, mingo, bp, kas, x86,
linux-kernel, kvm, linux-coco, kai.huang, rick.p.edgecombe,
yilun.xu, vannapurve, ackerleytng, sagis, binbin.wu, xiaoyao.li,
isaku.yamahata
In-Reply-To: <a14531ab-f069-41f9-8c5c-9fe6f28a9454@intel.com>
On Thu, Mar 19, 2026, Dave Hansen wrote:
> On 3/18/26 17:57, Yan Zhao wrote:
> > Remove the completely unnecessary assumption that memory mapped into a TDX
> > guest is backed by refcounted struct page memory. From KVM's point of view,
> > TDH_MEM_PAGE_ADD and TDH_MEM_PAGE_AUG are glorified writes to PTEs, so they
> > have no business placing requirements on how KVM and guest_memfd manage
> > memory.
>
> I think this goes a bit too far.
>
> It's one thing to say that it's more convenient for KVM to stick with
> pfns because it's what KVM uses now. Or, that the goals of using 'struct
> page' can be accomplished other ways. It's quite another to say what
> other bits of the codebase have "business" doing.
>
> Sean, can we tone this down a _bit_ to help guide folks in the future?
I strongly disagree on this one. IMO, super low level APIs have no business
placing unnecessary requirements on callers. Requiring that the target memory
be convertible? A-ok because that's an actual requirement of the architecture.
Requiring or assuming anything about "struct page" or folios? Not ok.
This isn't a convenience thing, it's a core tenent of KVM guest memory managment.
KVM's MMUs work with PFNs, full stop. A PFN might have been acquired via GUP and
thus a refcounted struct page, but there is a hard boundary in KVM between getting
the page via GUP and installing the PFN into KVM's MMU.
KVM didn't always have a hard boundary, and it took us literally years to undo
the resulting messes. And the TDX hugepage support that was posted that pulled
information from "struct page" and/or its folio re-introduced the exact type of
flawed assumptions that we spent years purging from KVM.
So yeah, what I wrote was a strongly worded statement, but that was 100% intentional,
because I want to be crystal clear that requiring KVM to pass a struct page is a
complete non-starter for me.
> > Rip out the misguided struct page assumptions/constraints and instead have
>
> Could we maybe tone down the editorializing a bit, please? Folks can
> have honest disagreements about this stuff while not being "misguided".
FWIW, I'm not trying to say the intent or people's viewpoints were misguided, I'm
saying the code itself is misguided. AFAICT, the "struct page" stuff was added
to try to harden the TDX implementation, e.g. to guard against effective UAF of
memory that was assigned to a TD. But my viewpoint is that requiring a struct
page made the overall implemenation _less_ robust, and thus the code is misguided
because its justfication/reasoning was flawed.
> > the two SEAMCALL wrapper APIs take PFN directly. This ensures that for
> > future huge page support in S-EPT, the kernel doesn't pick up even worse
> > assumptions like "a hugepage must be contained in a single folio".
>
> I don't really understand what this is saying.
>
> Is the concern that KVM might want to set up page tables for memory that
> differ from how it was allocated? I'm a bit worried that this assumes
> something about folios that doesn't always hold.
Heh, the concern is that taking a page/folio in the SEAMCALL wrappers will lead
to assumptions that don't always hold. Specifically, the TDX hugepage support[*]
was building up assumptions that KVM would never attempt to install a hugepage
that didn't fit into a single folio:
+ if (start_idx + npages > folio_nr_pages(folio))
+ return TDX_OPERAND_INVALID;
[*] https://lore.kernel.org/all/20250807094132.4453-1-yan.y.zhao@intel.com
> I think the hugetlbfs gigantic support uses folios in at least a few
> spots today.
Yes, and the in-progress guest_memfd+HugeTLB work will also use folios. The
potential hiccup with the above folio_nr_pages() assumption is that KVM may want
to shatter folios to 4KiB granularity for tracking purposes, but still map
hugepage when memory is known to be physically contiguous.
That's where a lot of this is coming from. Taking a "struct page" is a bad
enough assumption on its own (that all TDX private memory is backed by struct page),
but even worse it's a slippery slope to even more bad assumptions (e.g. about how
guest_memfd internally manages its folios).
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Dave Hansen @ 2026-04-02 21:09 UTC (permalink / raw)
To: Sean Christopherson
Cc: Yan Zhao, pbonzini, dave.hansen, tglx, mingo, bp, kas, x86,
linux-kernel, kvm, linux-coco, kai.huang, rick.p.edgecombe,
yilun.xu, vannapurve, ackerleytng, sagis, binbin.wu, xiaoyao.li,
isaku.yamahata
In-Reply-To: <ac7V0g2q2hN3dU5u@google.com>
On 4/2/26 13:47, Sean Christopherson wrote:
> On Thu, Mar 19, 2026, Dave Hansen wrote:
>> On 3/18/26 17:57, Yan Zhao wrote:
>>> Remove the completely unnecessary assumption that memory mapped into a TDX
>>> guest is backed by refcounted struct page memory. From KVM's point of view,
>>> TDH_MEM_PAGE_ADD and TDH_MEM_PAGE_AUG are glorified writes to PTEs, so they
>>> have no business placing requirements on how KVM and guest_memfd manage
>>> memory.
>>
>> I think this goes a bit too far.
>>
>> It's one thing to say that it's more convenient for KVM to stick with
>> pfns because it's what KVM uses now. Or, that the goals of using 'struct
>> page' can be accomplished other ways. It's quite another to say what
>> other bits of the codebase have "business" doing.
>>
>> Sean, can we tone this down a _bit_ to help guide folks in the future?
>
> I strongly disagree on this one.
I think I understand the motivation now. All I'm saying is that instead
of something like:
Remove the completely unnecessary assumption that memory mapped
into a TDX guest is backed by refcounted struct page memory.
I'd rather see something along the lines of
KVM's MMUs work with PFNs. This is very much an intentional
design choice. It ensures that the KVM MMUs remains flexible
and are not too tied to the regular CPU MMUs and the kernel code
around them.
Using 'struct page' for TDX memory is not a good fit anywhere
near the KVM MMU code.
Would you disagree strongly with that kind of rewording?
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Sean Christopherson @ 2026-04-02 22:11 UTC (permalink / raw)
To: Dave Hansen
Cc: Yan Zhao, pbonzini, dave.hansen, tglx, mingo, bp, kas, x86,
linux-kernel, kvm, linux-coco, kai.huang, rick.p.edgecombe,
yilun.xu, vannapurve, ackerleytng, sagis, binbin.wu, xiaoyao.li,
isaku.yamahata
In-Reply-To: <33adf49d-4937-413e-a594-830c11b2bed0@intel.com>
On Thu, Apr 02, 2026, Dave Hansen wrote:
> On 4/2/26 13:47, Sean Christopherson wrote:
> > On Thu, Mar 19, 2026, Dave Hansen wrote:
> >> On 3/18/26 17:57, Yan Zhao wrote:
> >>> Remove the completely unnecessary assumption that memory mapped into a TDX
> >>> guest is backed by refcounted struct page memory. From KVM's point of view,
> >>> TDH_MEM_PAGE_ADD and TDH_MEM_PAGE_AUG are glorified writes to PTEs, so they
> >>> have no business placing requirements on how KVM and guest_memfd manage
> >>> memory.
> >>
> >> I think this goes a bit too far.
> >>
> >> It's one thing to say that it's more convenient for KVM to stick with
> >> pfns because it's what KVM uses now. Or, that the goals of using 'struct
> >> page' can be accomplished other ways. It's quite another to say what
> >> other bits of the codebase have "business" doing.
> >>
> >> Sean, can we tone this down a _bit_ to help guide folks in the future?
> >
> > I strongly disagree on this one.
>
> I think I understand the motivation now. All I'm saying is that instead
> of something like:
>
> Remove the completely unnecessary assumption that memory mapped
> into a TDX guest is backed by refcounted struct page memory.
>
> I'd rather see something along the lines of
>
> KVM's MMUs work with PFNs. This is very much an intentional
> design choice. It ensures that the KVM MMUs remains flexible
> and are not too tied to the regular CPU MMUs and the kernel code
> around them.
>
> Using 'struct page' for TDX memory is not a good fit anywhere
> near the KVM MMU code.
>
> Would you disagree strongly with that kind of rewording?
Not at all, works for me.
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Sean Christopherson @ 2026-04-02 23:07 UTC (permalink / raw)
To: Chang S. Bae
Cc: Paolo Bonzini, Kiryl Shutsemau, kvm, x86, linux-coco,
linux-kernel
In-Reply-To: <3051c067-048e-4388-8c22-1e275d8d3b5a@intel.com>
On Wed, Mar 25, 2026, Chang S. Bae wrote:
> On 3/12/2026 10:47 AM, Sean Christopherson wrote:
> > On Thu, Mar 12, 2026, Chang S. Bae wrote:
> > >
> > > However, that is sort of what-if scenarios at best. The host kernel still
> > > manages EGPR context switching through XSAVE. Saving EGPRs into regs[] would
> > > introduce an oddity to synchronize between two buffers: regs[] and
> > > gfpu->fpstate, which looks like unnecessary complexity.
>
> No, this looks ugly.
Sorry, you lost me. What looks ugly?
> If guest EGPR state is saved in vcpu->arch.regs[], the APX area there isn't
> necessary:
>
> When the KVM API exposes state in XSAVE format, the frontend can handle this
> separately. Alongside uABI <-> guest fpstate copy functions, new copy
> functions may deal with the state between uABI <-> VCPU cache.
>
> Further, one could think of exclusion as such:
>
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index 76153dfb58c9..5404f9399eea 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -794,9 +794,10 @@ static u64 __init guest_default_mask(void)
> {
> /*
> * Exclude dynamic features, which require userspace opt-in even
> - * for KVM guests.
> + * for KVM guests, and APX as extended general-purpose register
> + * states are saved in the KVM cache separately.
> */
> - return ~(u64)XFEATURE_MASK_USER_DYNAMIC;
> + return ~((u64)XFEATURE_MASK_USER_DYNAMIC | XFEATURE_MASK_APX);
> }
>
> But this default bitmask feeds into the permission bits:
>
> fpu->guest_perm.__state_perm = guest_default_cfg.features;
> fpu->guest_perm.__state_size = guest_default_cfg.size;
>
> This policy looks clear and sensible: permission is granted only if space is
> reserved to save the state. If there is a strong desire to save memory, I
> think it should go through a more thorough review to revisit this policy.
And I'm lost again.
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Sean Christopherson @ 2026-04-02 23:19 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kiryl Shutsemau, kvm, x86, linux-coco, linux-kernel,
Chang S . Bae
In-Reply-To: <7ec084f8-812e-42f2-8470-e416fa7ee848@redhat.com>
On Wed, Mar 11, 2026, Paolo Bonzini wrote:
> On 3/11/26 01:33, Sean Christopherson wrote:
> > Clean up KVM's register tracking and storage in preparation for landing APX,
> > which expands the maximum number of GPRs from 16 to 32.
> >
> > This is kinda sorta an RFC, as there are some very opinionated changes. I.e.
> > if you dislike something, please speak up.
> >
> > My thought is to treat R16-R31 as much like other GPRs as possible (though
> > maybe we don't need to expand regs[] as sketched out in the last patch?).
>
> The cleanups in patches 1-4 are nice.
>
> For APX specifically, in abstract it's nice to treat R16-R31 as much as
> possible as regular GPRs. On the other hand, the extra 16 regs[] entries
> would be more or less unused, the ugly switch statements wouldn't go away.
Hmm, yeah, but only if XSAVE is the source of truth for guest R16-R31.
Do we know what the compiler and/or kernel rules for using R16-R31 will be?
E.g. if C code is allowed to use R16-R31 at will, then KVM will either need to
swap R16-R31 in assembly, or annotate a pile of functions as "no_egpr" or
whatever.
At that point, my vote would be to use regs[] to track R16-R31 for KVM's purposes.
IIUC, we could largely ignore XSAVE state at runtime and just ensure R16-R31 are
copied to/from userspace as needed, same as we do for PKRU.
If R16-R31 aren't generally available for C code, then how exactly is APX going
to be used?
Understanding the usage rules for R16-R31 seems fundamental to figuring what to
do in KVM...
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Ackerley Tng @ 2026-04-02 23:23 UTC (permalink / raw)
To: Yan Zhao, seanjc, pbonzini, dave.hansen
Cc: tglx, mingo, bp, kas, x86, linux-kernel, kvm, linux-coco,
kai.huang, rick.p.edgecombe, yilun.xu, vannapurve, sagis,
binbin.wu, xiaoyao.li, isaku.yamahata
In-Reply-To: <20260319005703.8983-1-yan.y.zhao@intel.com>
Yan Zhao <yan.y.zhao@intel.com> writes:
>
> [...snip...]
>
> -u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page *source, u64 *ext_err1, u64 *ext_err2)
> +u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *source,
> + u64 *ext_err1, u64 *ext_err2)
> {
> struct tdx_module_args args = {
> .rcx = gpa,
> .rdx = tdx_tdr_pa(td),
> - .r8 = page_to_phys(page),
> + .r8 = PFN_PHYS(pfn),
> .r9 = page_to_phys(source),
Perhaps in some future patch, are we considering also passing pfn
instead of struct page for source? Would we also update
kvm_tdx->page_add_src to be a kvm_pfn_t?
> };
> u64 ret;
>
> - tdx_clflush_page(page);
> + tdx_clflush_pfn(pfn);
> ret = seamcall_ret(TDH_MEM_PAGE_ADD, &args);
>
> *ext_err1 = args.rcx;
>
> [...snip...]
>
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Sean Christopherson @ 2026-04-02 23:35 UTC (permalink / raw)
To: Ackerley Tng
Cc: Yan Zhao, pbonzini, dave.hansen, tglx, mingo, bp, kas, x86,
linux-kernel, kvm, linux-coco, kai.huang, rick.p.edgecombe,
yilun.xu, vannapurve, sagis, binbin.wu, xiaoyao.li,
isaku.yamahata
In-Reply-To: <CAEvNRgGknoP=QscA-efLB0LSS03XttsdN3v0KBVROAXTknAakw@mail.gmail.com>
On Thu, Apr 02, 2026, Ackerley Tng wrote:
> Yan Zhao <yan.y.zhao@intel.com> writes:
>
> >
> > [...snip...]
> >
> > -u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page *source, u64 *ext_err1, u64 *ext_err2)
> > +u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *source,
> > + u64 *ext_err1, u64 *ext_err2)
> > {
> > struct tdx_module_args args = {
> > .rcx = gpa,
> > .rdx = tdx_tdr_pa(td),
> > - .r8 = page_to_phys(page),
> > + .r8 = PFN_PHYS(pfn),
> > .r9 = page_to_phys(source),
>
> Perhaps in some future patch, are we considering also passing pfn instead of
> struct page for source? Would we also update kvm_tdx->page_add_src to be a
> kvm_pfn_t?
Probably?
I assume you're asking in the context of in-place conversion, where KVM will
allow a single guest_memfd page to be both the source and the dest?
Right now, KVM requires the source page to be a GUP'able page, specifically so
that KVM can obtain a reference and ensure the page isn't freed until KVM is done
with it. If/when the source and dest are one and the same, then I don't think
we'd want to GUP the page (and there would be no need to since this would all run
while holding gmem's filemap_invalidate_lock()), at which point, yeah, passing a
"struct page" doesn't make much sense, and passing kvm_pfn_t or u64 or whatever
seems like the obvious choice.
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Edgecombe, Rick P @ 2026-04-02 23:36 UTC (permalink / raw)
To: ackerleytng@google.com, pbonzini@redhat.com, seanjc@google.com,
Zhao, Yan Y, dave.hansen@linux.intel.com
Cc: sagis@google.com, Yamahata, Isaku, x86@kernel.org, kas@kernel.org,
yilun.xu@linux.intel.com, bp@alien8.de, mingo@redhat.com,
linux-kernel@vger.kernel.org, Huang, Kai, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, Li, Xiaoyao, tglx@kernel.org,
binbin.wu@linux.intel.com, Annapurve, Vishal
In-Reply-To: <CAEvNRgGknoP=QscA-efLB0LSS03XttsdN3v0KBVROAXTknAakw@mail.gmail.com>
On Thu, 2026-04-02 at 16:23 -0700, Ackerley Tng wrote:
> Yan Zhao <yan.y.zhao@intel.com> writes:
>
> >
> > [...snip...]
> >
> > -u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page *source, u64 *ext_err1, u64 *ext_err2)
> > +u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *source,
> > + u64 *ext_err1, u64 *ext_err2)
> > {
> > struct tdx_module_args args = {
> > .rcx = gpa,
> > .rdx = tdx_tdr_pa(td),
> > - .r8 = page_to_phys(page),
> > + .r8 = PFN_PHYS(pfn),
> > .r9 = page_to_phys(source),
>
> Perhaps in some future patch, are we considering also passing pfn
> instead of struct page for source? Would we also update
> kvm_tdx->page_add_src to be a kvm_pfn_t?
Can you remind me, with the new API we were going to do an in-place add right?
Then I'd wonder if we could maybe change tdh_mem_page_add() to only have a
single pfn arg. The passing of ->src_page is kind of awkward already.
Like Ira was playing around with here:
https://lore.kernel.org/kvm/20251105-tdx-init-in-place-v1-1-1196b67d0423@intel.com/
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Sean Christopherson @ 2026-04-02 23:46 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: ackerleytng@google.com, pbonzini@redhat.com, Yan Y Zhao,
dave.hansen@linux.intel.com, sagis@google.com, Isaku Yamahata,
x86@kernel.org, kas@kernel.org, yilun.xu@linux.intel.com,
bp@alien8.de, mingo@redhat.com, linux-kernel@vger.kernel.org,
Kai Huang, kvm@vger.kernel.org, linux-coco@lists.linux.dev,
Xiaoyao Li, tglx@kernel.org, binbin.wu@linux.intel.com,
Vishal Annapurve
In-Reply-To: <4f0a5d5f7b743e3872547e14d02b894cd9f60550.camel@intel.com>
On Thu, Apr 02, 2026, Rick P Edgecombe wrote:
> On Thu, 2026-04-02 at 16:23 -0700, Ackerley Tng wrote:
> > Yan Zhao <yan.y.zhao@intel.com> writes:
> >
> > >
> > > [...snip...]
> > >
> > > -u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page *source, u64 *ext_err1, u64 *ext_err2)
> > > +u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *source,
> > > + u64 *ext_err1, u64 *ext_err2)
> > > {
> > > struct tdx_module_args args = {
> > > .rcx = gpa,
> > > .rdx = tdx_tdr_pa(td),
> > > - .r8 = page_to_phys(page),
> > > + .r8 = PFN_PHYS(pfn),
> > > .r9 = page_to_phys(source),
> >
> > Perhaps in some future patch, are we considering also passing pfn
> > instead of struct page for source? Would we also update
> > kvm_tdx->page_add_src to be a kvm_pfn_t?
>
> Can you remind me, with the new API we were going to do an in-place add right?
> Then I'd wonder if we could maybe change tdh_mem_page_add() to only have a
> single pfn arg.
No. In-place ADD will be supported, but it won't be mandatory. Practically
speaking, we can't make it mandatory unless we're willing to completely rip out
support for per-VM attributes (or at least, per-VM PRIVATE tracking). I suppose
we could require in-place ADD when using per-gmem attributes, but I don't see
the point given that TDH_MEM_PAGE_ADD itself takes a source and dest.
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Edgecombe, Rick P @ 2026-04-02 23:53 UTC (permalink / raw)
To: seanjc@google.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Li, Xiaoyao, Zhao, Yan Y, dave.hansen@linux.intel.com,
linux-kernel@vger.kernel.org, kas@kernel.org, mingo@redhat.com,
pbonzini@redhat.com, binbin.wu@linux.intel.com, Yamahata, Isaku,
ackerleytng@google.com, sagis@google.com, tglx@kernel.org,
bp@alien8.de, Annapurve, Vishal, yilun.xu@linux.intel.com,
x86@kernel.org
In-Reply-To: <ac7_uQc5t0uU3a1h@google.com>
On Thu, 2026-04-02 at 16:46 -0700, Sean Christopherson wrote:
> > Can you remind me, with the new API we were going to do an in-place add
> > right?
> > Then I'd wonder if we could maybe change tdh_mem_page_add() to only have a
> > single pfn arg.
>
> No. In-place ADD will be supported, but it won't be mandatory. Practically
> speaking, we can't make it mandatory unless we're willing to completely rip
> out support for per-VM attributes (or at least, per-VM PRIVATE tracking). I
> suppose we could require in-place ADD when using per-gmem attributes, but I
> don't see the point given that TDH_MEM_PAGE_ADD itself takes a source and
> dest.
Thanks. It might still be cleaner to copy clear text from the GUPed page to the
destination page and let the tdh_mem_page_add() call in the map path do it in-
place? Especially if we want to support in-place add as an option, it would make
the code more uniform.
But it sounds like we don't need to decide now.
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Chang S. Bae @ 2026-04-03 0:05 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Kiryl Shutsemau, kvm, x86, linux-coco,
linux-kernel
In-Reply-To: <ac72z1cfXnpUmkWv@google.com>
[-- Attachment #1: Type: text/plain, Size: 2641 bytes --]
On 4/2/2026 4:07 PM, Sean Christopherson wrote:
> On Wed, Mar 25, 2026, Chang S. Bae wrote:
>> On 3/12/2026 10:47 AM, Sean Christopherson wrote:
>>> On Thu, Mar 12, 2026, Chang S. Bae wrote:
>>>>
>>>> However, that is sort of what-if scenarios at best. The host kernel still
>>>> manages EGPR context switching through XSAVE. Saving EGPRs into regs[] would
>>>> introduce an oddity to synchronize between two buffers: regs[] and
>>>> gfpu->fpstate, which looks like unnecessary complexity.
>>
>> No, this looks ugly.
>
> Sorry, you lost me. What looks ugly?
Oh, this is against my comment above. Keeping regs[] <-> guest fpstate
in sync will be unnecessarily complex without clear usage (continues below).
>> If guest EGPR state is saved in vcpu->arch.regs[], the APX area there isn't
>> necessary:
>>
>> When the KVM API exposes state in XSAVE format, the frontend can handle this
>> separately. Alongside uABI <-> guest fpstate copy functions, new copy
>> functions may deal with the state between uABI <-> VCPU cache.
>>
>> Further, one could think of exclusion as such:
>>
>> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
>> index 76153dfb58c9..5404f9399eea 100644
>> --- a/arch/x86/kernel/fpu/xstate.c
>> +++ b/arch/x86/kernel/fpu/xstate.c
>> @@ -794,9 +794,10 @@ static u64 __init guest_default_mask(void)
>> {
>> /*
>> * Exclude dynamic features, which require userspace opt-in even
>> - * for KVM guests.
>> + * for KVM guests, and APX as extended general-purpose register
>> + * states are saved in the KVM cache separately.
>> */
>> - return ~(u64)XFEATURE_MASK_USER_DYNAMIC;
>> + return ~((u64)XFEATURE_MASK_USER_DYNAMIC | XFEATURE_MASK_APX);
>> }
>>
>> But this default bitmask feeds into the permission bits:
>>
>> fpu->guest_perm.__state_perm = guest_default_cfg.features;
>> fpu->guest_perm.__state_size = guest_default_cfg.size;
>>
>> This policy looks clear and sensible: permission is granted only if space is
>> reserved to save the state. If there is a strong desire to save memory, I
>> think it should go through a more thorough review to revisit this policy.
>
> And I'm lost again.
Here I made myself pursuing the approach saving/restoring EGPRs via
regs[] on VM entry/exit. Then a couple of follow-up questions:
1. What about APX area in guest fpstate?
2. How to support the state for KVM ABI?
It surely departs from the "XSAVE - the single source of truth" model.
Then,
Leave the APX area in guest fpstate unused.
Copying APX state directly between regs[] and uABI to preserve XSAVE-
based ABI like in the attached diff.
That's all I'm saying.
[-- Attachment #2: kvmapi-apx.diff --]
[-- Type: text/plain, Size: 4470 bytes --]
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index fffbf087937d..b3ab2ac827e6 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -59,6 +59,16 @@ void __init kvm_init_xstate_sizes(void)
}
}
+u32 xstate_size(unsigned int xfeature)
+{
+ return xstate_sizes[xfeature].eax;
+}
+
+u32 xstate_offset(unsigned int xfeature)
+{
+ return xstate_sizes[xfeature].ebx;
+}
+
u32 xstate_required_size(u64 xstate_bv, bool compacted)
{
u32 ret = XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET;
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index 039b8e6f40ba..5ace99dd152b 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -64,6 +64,8 @@ bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
void __init kvm_init_xstate_sizes(void);
u32 xstate_required_size(u64 xstate_bv, bool compacted);
+u32 xstate_size(unsigned int xfeature);
+u32 xstate_offset(unsigned int xfeature);
int cpuid_query_maxphyaddr(struct kvm_vcpu *vcpu);
int cpuid_query_maxguestphyaddr(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c1e1b3030786..1f064a32b8b7 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -108,6 +108,12 @@ EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_host);
#define emul_to_vcpu(ctxt) \
((struct kvm_vcpu *)(ctxt)->vcpu)
+#ifdef CONFIG_KVM_APX
+#define VCPU_EGPRS_PTR(vcpu) &(vcpu)->arch.regs[VCPU_REGS_R16]
+#else
+#define VCPU_EGPRS_PTR(vcpu) NULL
+#endif
+
/* EFER defaults:
* - enable syscall per default because its emulated by KVM
* - enable LME and LMA per default on 64 bit KVM
@@ -5804,10 +5810,33 @@ static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu,
return 0;
}
+static void kvm_copy_vcpu_regs_to_uabi(struct kvm_vcpu *vcpu, struct kvm_xsave *uabi_xsave)
+{
+ union fpregs_state *xstate = (union fpregs_state *)uabi_xsave->region;
+ void *uabi_apx = (void*)uabi_xsave->region + xstate_offset(XFEATURE_APX);
+ void *vcpu_egprs = VCPU_EGPRS_PTR(vcpu);
+
+ if (!vcpu_egprs)
+ return;
-static int kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu,
- u8 *state, unsigned int size)
+ memcpy(uabi_apx, vcpu_egprs, xstate_size(XFEATURE_APX));
+ xstate->xsave.header.xfeatures |= XFEATURE_MASK_APX;
+}
+
+static void kvm_copy_uabi_to_vcpu_regs(struct kvm_vcpu *vcpu, struct kvm_xsave *uabi_xsave)
{
+ union fpregs_state *xstate = (union fpregs_state *)uabi_xsave->region;
+ void *uabi_apx = (void*)uabi_xsave->region + xstate_offset(XFEATURE_APX);
+ void *vcpu_egprs = VCPU_EGPRS_PTR(vcpu);
+
+ if (vcpu_egprs && xstate->xsave.header.xfeatures & XFEATURE_MASK_APX)
+ memcpy(vcpu_egprs, uabi_apx, xstate_size(XFEATURE_APX));
+}
+
+static int kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu, struct kvm_xsave *guest_xsave,
+ unsigned int size)
+{
+
/*
* Only copy state for features that are enabled for the guest. The
* state itself isn't problematic, but setting bits in the header for
@@ -5826,15 +5855,23 @@ static int kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu,
if (fpstate_is_confidential(&vcpu->arch.guest_fpu))
return vcpu->kvm->arch.has_protected_state ? -EINVAL : 0;
- fpu_copy_guest_fpstate_to_uabi(&vcpu->arch.guest_fpu, state, size,
+ /*
+ * The generic XSAVE copy function zeros out areas not present in
+ * guest fpstate. Those not in fpstate but in somewhere else,
+ * like EGPRs, should be copied after this.
+ */
+ fpu_copy_guest_fpstate_to_uabi(&vcpu->arch.guest_fpu, guest_xsave->region, size,
supported_xcr0, vcpu->arch.pkru);
+
+ kvm_copy_vcpu_regs_to_uabi(vcpu, guest_xsave);
+
return 0;
}
static int kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu,
struct kvm_xsave *guest_xsave)
{
- return kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region,
+ return kvm_vcpu_ioctl_x86_get_xsave2(vcpu, guest_xsave,
sizeof(guest_xsave->region));
}
@@ -5853,6 +5890,8 @@ static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,
*/
xstate->xsave.header.xfeatures &= ~vcpu->arch.guest_fpu.fpstate->xfd;
+ kvm_copy_uabi_to_vcpu_regs(vcpu, guest_xsave);
+
return fpu_copy_uabi_to_guest_fpstate(&vcpu->arch.guest_fpu,
guest_xsave->region,
kvm_caps.supported_xcr0,
@@ -6464,7 +6503,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
if (!u.xsave)
break;
- r = kvm_vcpu_ioctl_x86_get_xsave2(vcpu, u.buffer, size);
+ r = kvm_vcpu_ioctl_x86_get_xsave2(vcpu, u.xsave, size);
if (r < 0)
break;
--
2.51.0
^ permalink raw reply related
* RE: [PATCH 2/2] x86/tdx: Accept hotplugged memory before online
From: Reshetova, Elena @ 2026-04-03 10:37 UTC (permalink / raw)
To: Edgecombe, Rick P, pbonzini@redhat.com, prsampat@amd.com
Cc: bp@alien8.de, marcandre.lureau@redhat.com, kas@kernel.org,
x86@kernel.org, linux-kernel@vger.kernel.org, mingo@redhat.com,
dave.hansen@linux.intel.com, Qiang, Chenyi, tglx@kernel.org,
hpa@zytor.com, kvm@vger.kernel.org, linux-coco@lists.linux.dev
In-Reply-To: <9b1290bbc1283a2f35bdcd177d27ae7aea89151c.camel@intel.com>
> On Thu, 2026-04-02 at 08:18 +0000, Reshetova, Elena wrote:
> > > Oh, I was just wondering if we could just zero the page on accept
> > > failure for the case of already accepted. Handle the issue
> > > internally
> > > and actually go back to something like patch 1. Will it work for
> > > SNP?
> >
> > I don't know about SNP, but if you are proposing to zero the page on
> > double acceptance, this is not great from security pov.
>
> Accept does zero the memory already. So the guest side operation is
> doing an operation that says "make this memory usable in an known state
> of zeros". And the operation complies. What is the difference?
The difference is that you do it in a re-accept case.
>
> > It creates a
> > predictable behaviour primitive for the host to zero any data inside
> > the confidential guest and it can be misused (think of zeroing out a
> > page containing a cryptographic key).
>
> If the host can trigger an accept somehow in the guest (via something
> like this or other issue), then the host can also remove, then AUG the
> page from the S-EPT. This will result in a normal accept which also
> zeros the page.
Yes, that's why the guest currently does not allow accepting a page that
has already been accepted.
>
> So the part about whether a triggered accept succeeds or returns an
> already accepted error is already under the control of the host. I.e.,
> if we don't have the zeroing behavior, the host can already cause the
> page to get zeroed. So I don't think anything is regressed. Both come
> down to how careful the guest is about what it accepts.
Yes, and my point is that we should not allow guest to freely double
accepting ever.
For any use case that requires releasing memory and accepting it back,
it should be explicit action by the guest to track that memory has been
"released" (under correct and safe conditions) and then it is ok to accept
it back (even if it doesnt mean physically accepting it) and in this case it
is ok (and even strongly desired) to zero the page to simulate the normal
accept behaviour.
^ permalink raw reply
* Re: [PATCH kernel 4/9] dma/swiotlb: Stop forcing SWIOTLB for TDISP devices
From: Alexey Kardashevskiy @ 2026-04-03 12:40 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: dan.j.williams, Robin Murphy, x86, linux-kernel, kvm, linux-pci,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Sean Christopherson, Paolo Bonzini,
Andy Lutomirski, Peter Zijlstra, Bjorn Helgaas, Marek Szyprowski,
Andrew Morton, Catalin Marinas, Michael Ellerman, Mike Rapoport,
Tom Lendacky, Ard Biesheuvel, Neeraj Upadhyay, Ashish Kalra,
Stefano Garzarella, Melody Wang, Seongman Lee, Joerg Roedel,
Nikunj A Dadhania, Michael Roth, Suravee Suthikulpanit,
Andi Kleen, Kuppuswamy Sathyanarayanan, Tony Luck,
David Woodhouse, Greg Kroah-Hartman, Denis Efremov, Geliang Tang,
Piotr Gregor, Michael S. Tsirkin, Alex Williamson, Arnd Bergmann,
Jesse Barnes, Jacob Pan, Yinghai Lu, Kevin Brodsky,
Jonathan Cameron, Aneesh Kumar K.V (Arm), Xu Yilun, Herbert Xu,
Kim Phillips, Konrad Rzeszutek Wilk, Stefano Stabellini,
Claire Chang, linux-coco, iommu
In-Reply-To: <20260304124316.GL972761@nvidia.com>
On 4/3/26 23:43, Jason Gunthorpe wrote:
> On Wed, Mar 04, 2026 at 05:45:31PM +1100, Alexey Kardashevskiy wrote:
>
>>> I suspect AMD needs to use their vTOM feature to allow shared memory
>>> to remain available to TDISP RUN with a high/low address split.
>>
>> I could probably do something about it bit I wonder what is the real
>> live use case which requires leaking SME mask, have a live example
>> which I could try recreating?
>
> We need shared memory allocated through a DMABUF heap:
>
> https://lore.kernel.org/all/20260223095136.225277-1-jiri@resnulli.us/
>
> To work with all PCI devices in the system, TDISP or not.
>
> Without this the ability for a TDISP device to ingest (encrypted) data
> requires all kinds of memcpy..
>
> So the DMA API should see the DMA_ATTR_CC_DECRYPTED and setup the
> correct dma_dddr_t either by choosing the shared alias for the TDISP
> device's vTOM, or setting the C bit in a vIOMMU S1.
Something like that?
https://github.com/AMDESE/linux-kvm/commit/266a41a1ea746557eb63debce886ce2c98820667
With some little hacks I can make this tree do TDISP DMA to private or shared (swiotlb) memory by steering via this vTOM thing. Thanks,
>
> Jason
--
Alexey
^ permalink raw reply
* Re: [PATCH v2 09/19] PCI/TSM: Support creating encrypted MMIO descriptors via TDISP Report
From: Alexey Kardashevskiy @ 2026-04-03 12:41 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Xu Yilun, Aneesh Kumar K.V, Dan Williams, linux-coco, linux-pci,
gregkh, bhelgaas, alistair23, lukas, Arnd Bergmann
In-Reply-To: <20260330114902.GA310919@nvidia.com>
On 30/3/26 22:49, Jason Gunthorpe wrote:
> On Mon, Mar 30, 2026 at 04:47:44PM +1100, Alexey Kardashevskiy wrote:
>
>> What do I miss? Thanks,
>
> You can't tell where things start so there is no way to relate the
> offsets to something the kernel can understand.
Reported ranges have BAR indexes and start addresses (with the reported MMIO offset added), and the first reported range starts at the first 4K of that BAR. And these ranges are in sorted order. Enough to calculate an offset of a 2nd/3rd/... range of that BAR, and this is what I am after.
--
Alexey
^ permalink raw reply
* Re: [PATCH v2 09/19] PCI/TSM: Support creating encrypted MMIO descriptors via TDISP Report
From: Jason Gunthorpe @ 2026-04-03 14:08 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Xu Yilun, Aneesh Kumar K.V, Dan Williams, linux-coco, linux-pci,
gregkh, bhelgaas, alistair23, lukas, Arnd Bergmann
In-Reply-To: <46de1d41-a660-460c-a49f-ae2d2ad7afe1@amd.com>
On Fri, Apr 03, 2026 at 11:41:25PM +1100, Alexey Kardashevskiy wrote:
>
>
> On 30/3/26 22:49, Jason Gunthorpe wrote:
> > On Mon, Mar 30, 2026 at 04:47:44PM +1100, Alexey Kardashevskiy wrote:
> >
> > > What do I miss? Thanks,
> >
> > You can't tell where things start so there is no way to relate the
> > offsets to something the kernel can understand.
>
> Reported ranges have BAR indexes and start addresses (with the
> reported MMIO offset added), and the first reported range starts at
> the first 4K of that BAR.
I was told this is not the case, the first reported range can start
anywhere in the BAR?
Jason
^ permalink raw reply
* Re: [PATCH] x86/tdx: Use pg_level in TDX APIs, not the TDX-Module's 0-based level
From: Sean Christopherson @ 2026-04-03 15:22 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Yan Zhao, Vishal Annapurve,
Ackerley Tng
In-Reply-To: <20260120203937.1447592-1-seanjc@google.com>
On Tue, Jan 20, 2026, Sean Christopherson wrote:
> Rework the TDX APIs to take the kernel's 1-based pg_level enum, not the
> TDX-Module's 0-based level. The APIs are _kernel_ APIs, not TDX-Module
> APIs, and the kernel (and KVM) uses "enum pg_level" literally everywhere.
>
> Using "enum pg_level" eliminates ambiguity when looking at the APIs (it's
> NOT clear that "int level" refers to the TDX-Module's level), and will
> allow for using existing helpers like page_level_size() when support for
> hugepages is added to the S-EPT APIs.
>
> No functional change intended.
>
> Cc: Kai Huang <kai.huang@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Cc: Yan Zhao <yan.y.zhao@intel.com>
> Cc: Vishal Annapurve <vannapurve@google.com>
> Cc: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
This still applies cleanly, any chance this can be picked up for 7.1 to avoid
prototype conflicts in ongoing work?
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Paolo Bonzini @ 2026-04-03 16:03 UTC (permalink / raw)
To: Sean Christopherson
Cc: Kiryl Shutsemau, kvm, x86, linux-coco, linux-kernel,
Chang S . Bae
In-Reply-To: <ac75iuf9gHIhvm9u@google.com>
On 4/3/26 01:19, Sean Christopherson wrote:
> On Wed, Mar 11, 2026, Paolo Bonzini wrote:
>> On 3/11/26 01:33, Sean Christopherson wrote:
>>> Clean up KVM's register tracking and storage in preparation for landing APX,
>>> which expands the maximum number of GPRs from 16 to 32.
>>>
>>> This is kinda sorta an RFC, as there are some very opinionated changes. I.e.
>>> if you dislike something, please speak up.
>>>
>>> My thought is to treat R16-R31 as much like other GPRs as possible (though
>>> maybe we don't need to expand regs[] as sketched out in the last patch?).
>>
>> The cleanups in patches 1-4 are nice.
>>
>> For APX specifically, in abstract it's nice to treat R16-R31 as much as
>> possible as regular GPRs. On the other hand, the extra 16 regs[] entries
>> would be more or less unused, the ugly switch statements wouldn't go away.
>
> Hmm, yeah, but only if XSAVE is the source of truth for guest R16-R31.
>
> Do we know what the compiler and/or kernel rules for using R16-R31 will be?
On the compiler side, they are enabling APX only with new enough -march,
or with -march=native if the host has APX. This, by the way, implies
CONFIG_X86_NATIVE_CPU is currently hosed on Diamond Rapids and newer
machines.
From what I remember of Chang Seok Bae's presentation at Plumbers last
year, right now there's no plan to have the kernel use APX, except
possibly through the usual kernel_fpu_begin/end.
> E.g. if C code is allowed to use R16-R31 at will, then KVM will either need to
> swap R16-R31 in assembly, or annotate a pile of functions as "no_egpr" or
> whatever.
__attribute__((__target__("no-apxf")), yeah. Perhaps it could be done
(kernel-wide) for all noinstr functions, but I think we agree that it's
not a great idea overall.
> At that point, my vote would be to use regs[] to track R16-R31 for KVM's purposes.
> IIUC, we could largely ignore XSAVE state at runtime and just ensure R16-R31 are
> copied to/from userspace as needed, same as we do for PKRU.
If it can be done efficiently when APX is not available, I suppose
that's fine. The KVM code is nicer for sure.
But until the kernel starts using APX, I would do the save/restore near
kvm_load_xfeatures(), because __vmx_vcpu_run()/__svm_vcpu_run() would
have to check whether xcr0.apx is set or not. This is much simpler:
// runs with host xcr0, so it can assume it includes APX
// alternatively it could be a static_call(), to only invoke
// the function if at least one guest enables APX in xcr0
if (static_cpu_has(X86_FEATURE_APX))
kvm_apx_save(vcpu);
kvm_load_xfeatures(vcpu, true);
...
kvm_load_xfeatures(vcpu, false);
// same as above
if (static_cpu_has(X86_FEATURE_APX))
kvm_apx_restore(vcpu);
Writing kvm_apx_save() and kvm_apx_restore() already now in a .S file is
fine but I don't care too much.
If the kernel starts using APX we would have to do the xcr0 changes in
two steps, one in kvm_load_xfeatures() and one (finalizing the APX bit)
right around the assembly code for world switching. That xcr0 update
would have to be in kvm_apx_save/restore, which would have to 1) be
rewritten in assembly 2) be called from within
__vmx_vcpu_run/__svm_vcpu_run.
But it would be premature to do now the two-step save/restore of xcr0.
> If R16-R31 aren't generally available for C code, then how exactly is APX going
> to be used?
Only in userspace, at least for now.
Paolo
> Understanding the usage rules for R16-R31 seems fundamental to figuring what to
> do in KVM...
>
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Dave Hansen @ 2026-04-03 16:07 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Kiryl Shutsemau, kvm, x86, linux-coco, linux-kernel,
Chang S . Bae
In-Reply-To: <ac75iuf9gHIhvm9u@google.com>
On 4/2/26 16:19, Sean Christopherson wrote:
> Do we know what the compiler and/or kernel rules for using R16-R31 will be?
> E.g. if C code is allowed to use R16-R31 at will, then KVM will either need to
> swap R16-R31 in assembly, or annotate a pile of functions as "no_egpr" or
> whatever.
My _assumption_ is that the speedup from using the new GPRs as GPRs in
the kernel is going to be enough for us to support it. This is even
though those kernel binaries won't run on old hardware.
If I'm right, then we're going to have to handle the new GPRs just like
the existing ones and save them on kernel entry before we hit C code.
I'm not sure I want to be messing with XSAVE there. XSAVE requires
munging a header which means even if we used XSAVE we'd need to XSAVE
and then copy things over to pt_regs (assuming we continue using pt_regs).
That doesn't seem like loads of fun because we'll also need to copy out
to the XSAVE UABI spots, like PKRU times 32.
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Fix APIC MSR ranges in tdx_has_emulated_msr()
From: Sean Christopherson @ 2026-04-03 16:30 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: dmaluka@chromium.org, kvm@vger.kernel.org, pbonzini@redhat.com,
Dave Hansen, binbin.wu@linux.intel.com, Isaku Yamahata,
bp@alien8.de, x86@kernel.org, kas@kernel.org, hpa@zytor.com,
linux-kernel@vger.kernel.org, mingo@redhat.com,
dave.hansen@linux.intel.com, tglx@kernel.org,
linux-coco@lists.linux.dev
In-Reply-To: <f67179d7050ae28ac261bb6227b1c045c10579c9.camel@intel.com>
On Thu, Mar 19, 2026, Rick P Edgecombe wrote:
> On Thu, 2026-03-19 at 15:40 +0800, Binbin Wu wrote:
> > tdx_has_emulated_msr() is used by KVM to decide whether to emulate a MSR access from the
> > TDVMCALL or just return the error code.
> >
> > During an off-list discussion, Rick noted that #VE reduction could change the behavior of
> > accessing an MSR (e.g., from #VE to #GP or to be virtualized by the TDX module) without
> > KVM knowing.Because KVM lacks the full context to perfectly decide if an MSR should be
> > emulated, the question was raised: Can we just delete tdx_has_emulated_msr() entirely?
> >
> > However, these native type x2apic MSRs are a special case. Since the TDX module owns the
> > APICv page, KVM cannot emulate these MSRs. If we remove tdx_has_emulated_msr(), a guest
> > directly issuing TDVMCALLs for these native type x2apic MSRs will trigger a silent failure,
> > even though this is the guest's fault.
> >
> > It comes down to a tradeoff. Should we prioritize code simplicity by dropping the function,
> > or keep it to explicitly catch this misbehaving guest corner case?
>
> I think from KVM's perspective it doesn't want to help the guest behave
> correctly.
Uh, yes KVM does does. KVM is responsible for emulating the APIC timer, isn't it?
> So we can ignore that I think. But it does really care to not define
> any specific guest ABI that it has to maintain. So tdx_has_emulated_msr() has
> some value there. And even more, it wants to not allow the guest to hurt the
> host.
>
> On the latter point, another problem with deleting tdx_has_emulated_msr() is the
> current code path skips the checks done in the other MSR paths. So we would need
> to call some appropriate higher up MSR helper to protect the host? And that
> wades into the CPUID bit consistency issues.
>
> So maybe... could we do a more limited version of the deletion where we allow
> all the APIC MSRs through? We'd have to check that it won't cause problems.
What? No. KVM can't get actually read/write most (all?) MSRs, allowing access
is far worse than returning an error, as for all intents and purposes KVM will
silently drop writes, and return garbage on reads.
> Failing that, we should maybe just explicitly list the ones TDX supports rather
> than the current way we define the APIC ones. As you mention below, it's not
> correct in other ways too so it could be more robust.
No? Don't we just want to allow access to MSRs that aren't accelerated? What
the TDX-Module supports is largely irrelevant, I think.
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 1e47c194af53..28e87630870b 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -2116,23 +2116,26 @@ bool tdx_has_emulated_msr(u32 index)
case MSR_IA32_MC0_CTL2 ... MSR_IA32_MCx_CTL2(KVM_MAX_MCE_BANKS) - 1:
/* MSR_IA32_MCx_{CTL, STATUS, ADDR, MISC, CTL2} */
case MSR_KVM_POLL_CONTROL:
+ /*
+ * x2APIC registers that are virtualized by the CPU can't be
+ * emulated, KVM doesn't have access to the virtual APIC page.
+ */
+ case X2APIC_MSR(APIC_ID):
+ case X2APIC_MSR(APIC_LVR):
+ case X2APIC_MSR(APIC_LDR):
+ case X2APIC_MSR(APIC_SPIV):
+ case X2APIC_MSR(APIC_ESR):
+ case X2APIC_MSR(APIC_ICR):
+ case X2APIC_MSR(APIC_LVTT):
+ case X2APIC_MSR(APIC_LVTTHMR):
+ case X2APIC_MSR(APIC_LVTPC):
+ case X2APIC_MSR(APIC_LVT0):
+ case X2APIC_MSR(APIC_LVT1):
+ case X2APIC_MSR(APIC_LVTERR):
+ case X2APIC_MSR(APIC_TMICT):
+ case X2APIC_MSR(APIC_TMCCT):
+ case X2APIC_MSR(APIC_TDCR):
return true;
- case APIC_BASE_MSR ... APIC_BASE_MSR + 0xff:
- /*
- * x2APIC registers that are virtualized by the CPU can't be
- * emulated, KVM doesn't have access to the virtual APIC page.
- */
- switch (index) {
- case X2APIC_MSR(APIC_TASKPRI):
- case X2APIC_MSR(APIC_PROCPRI):
- case X2APIC_MSR(APIC_EOI):
- case X2APIC_MSR(APIC_ISR) ... X2APIC_MSR(APIC_ISR + APIC_ISR_NR):
- case X2APIC_MSR(APIC_TMR) ... X2APIC_MSR(APIC_TMR + APIC_ISR_NR):
- case X2APIC_MSR(APIC_IRR) ... X2APIC_MSR(APIC_IRR + APIC_ISR_NR):
- return false;
- default:
- return true;
- }
default:
return false;
}
^ permalink raw reply related
* Re: [PATCH] KVM: TDX: Fix APIC MSR ranges in tdx_has_emulated_msr()
From: Edgecombe, Rick P @ 2026-04-03 18:40 UTC (permalink / raw)
To: seanjc@google.com
Cc: tglx@kernel.org, Hansen, Dave, dave.hansen@linux.intel.com,
dmaluka@chromium.org, x86@kernel.org, binbin.wu@linux.intel.com,
bp@alien8.de, kas@kernel.org, linux-kernel@vger.kernel.org,
hpa@zytor.com, kvm@vger.kernel.org, pbonzini@redhat.com,
Yamahata, Isaku, mingo@redhat.com, linux-coco@lists.linux.dev
In-Reply-To: <ac_rNT69TdocrMYc@google.com>
On Fri, 2026-04-03 at 09:30 -0700, Sean Christopherson wrote:
> On Thu, Mar 19, 2026, Rick P Edgecombe wrote:
> > On Thu, 2026-03-19 at 15:40 +0800, Binbin Wu wrote:
> > > tdx_has_emulated_msr() is used by KVM to decide whether to emulate a MSR access from the
> > > TDVMCALL or just return the error code.
> > >
> > > During an off-list discussion, Rick noted that #VE reduction could change the behavior of
> > > accessing an MSR (e.g., from #VE to #GP or to be virtualized by the TDX module) without
> > > KVM knowing.Because KVM lacks the full context to perfectly decide if an MSR should be
> > > emulated, the question was raised: Can we just delete tdx_has_emulated_msr() entirely?
> > >
> > > However, these native type x2apic MSRs are a special case. Since the TDX module owns the
> > > APICv page, KVM cannot emulate these MSRs. If we remove tdx_has_emulated_msr(), a guest
> > > directly issuing TDVMCALLs for these native type x2apic MSRs will trigger a silent failure,
> > > even though this is the guest's fault.
> > >
> > > It comes down to a tradeoff. Should we prioritize code simplicity by dropping the function,
> > > or keep it to explicitly catch this misbehaving guest corner case?
> >
> > I think from KVM's perspective it doesn't want to help the guest behave
> > correctly.
>
> Uh, yes KVM does does. KVM is responsible for emulating the APIC timer, isn't it?
Yea totally. We need to emulate the interface accurately. But we are kind of
making up the contract after the fact. If the guest performs the wrong type of
MSR write, should we make the contract that the VMM should help it catch it's
mistake?
>
> > So we can ignore that I think. But it does really care to not define
> > any specific guest ABI that it has to maintain. So tdx_has_emulated_msr() has
> > some value there. And even more, it wants to not allow the guest to hurt the
> > host.
> >
> > On the latter point, another problem with deleting tdx_has_emulated_msr() is the
> > current code path skips the checks done in the other MSR paths. So we would need
> > to call some appropriate higher up MSR helper to protect the host? And that
> > wades into the CPUID bit consistency issues.
> >
> > So maybe... could we do a more limited version of the deletion where we allow
> > all the APIC MSRs through? We'd have to check that it won't cause problems.
>
> What? No. KVM can't get actually read/write most (all?) MSRs, allowing access
> is far worse than returning an error, as for all intents and purposes KVM will
> silently drop writes, and return garbage on reads.
>
> > Failing that, we should maybe just explicitly list the ones TDX supports rather
> > than the current way we define the APIC ones. As you mention below, it's not
> > correct in other ways too so it could be more robust.
>
> No? Don't we just want to allow access to MSRs that aren't accelerated? What
> the TDX-Module supports is largely irrelevant, I think.
Not sure if I might be missing the point here. As above, we don't have enough
info to know which MSRs are accelerated. If the guest enabled #VE reduction, it
changes which ones are accelerated and the VMM is not notified. I think the
below is a sane limitation, but doesn't lets KVM perfectly notify the guest when
it screws up.
So the line would be to block MSRs that can never be emulated.
BTW, I've been treating this secret contract change as an arch mistake to at
least not build on. It's a whole subject though... Let me know if you are
interested in the details.
>
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 1e47c194af53..28e87630870b 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -2116,23 +2116,26 @@ bool tdx_has_emulated_msr(u32 index)
> case MSR_IA32_MC0_CTL2 ... MSR_IA32_MCx_CTL2(KVM_MAX_MCE_BANKS) - 1:
> /* MSR_IA32_MCx_{CTL, STATUS, ADDR, MISC, CTL2} */
> case MSR_KVM_POLL_CONTROL:
> + /*
> + * x2APIC registers that are virtualized by the CPU can't be
> + * emulated, KVM doesn't have access to the virtual APIC page.
> + */
> + case X2APIC_MSR(APIC_ID):
> + case X2APIC_MSR(APIC_LVR):
> + case X2APIC_MSR(APIC_LDR):
> + case X2APIC_MSR(APIC_SPIV):
> + case X2APIC_MSR(APIC_ESR):
> + case X2APIC_MSR(APIC_ICR):
> + case X2APIC_MSR(APIC_LVTT):
> + case X2APIC_MSR(APIC_LVTTHMR):
> + case X2APIC_MSR(APIC_LVTPC):
> + case X2APIC_MSR(APIC_LVT0):
> + case X2APIC_MSR(APIC_LVT1):
> + case X2APIC_MSR(APIC_LVTERR):
> + case X2APIC_MSR(APIC_TMICT):
> + case X2APIC_MSR(APIC_TMCCT):
> + case X2APIC_MSR(APIC_TDCR):
> return true;
> - case APIC_BASE_MSR ... APIC_BASE_MSR + 0xff:
> - /*
> - * x2APIC registers that are virtualized by the CPU can't be
> - * emulated, KVM doesn't have access to the virtual APIC page.
> - */
> - switch (index) {
> - case X2APIC_MSR(APIC_TASKPRI):
> - case X2APIC_MSR(APIC_PROCPRI):
> - case X2APIC_MSR(APIC_EOI):
> - case X2APIC_MSR(APIC_ISR) ... X2APIC_MSR(APIC_ISR + APIC_ISR_NR):
> - case X2APIC_MSR(APIC_TMR) ... X2APIC_MSR(APIC_TMR + APIC_ISR_NR):
> - case X2APIC_MSR(APIC_IRR) ... X2APIC_MSR(APIC_IRR + APIC_ISR_NR):
> - return false;
> - default:
> - return true;
> - }
> default:
> return false;
> }
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Fix APIC MSR ranges in tdx_has_emulated_msr()
From: Sean Christopherson @ 2026-04-03 19:07 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: tglx@kernel.org, Dave Hansen, dave.hansen@linux.intel.com,
dmaluka@chromium.org, x86@kernel.org, binbin.wu@linux.intel.com,
bp@alien8.de, kas@kernel.org, linux-kernel@vger.kernel.org,
hpa@zytor.com, kvm@vger.kernel.org, pbonzini@redhat.com,
Isaku Yamahata, mingo@redhat.com, linux-coco@lists.linux.dev
In-Reply-To: <401db97a254b356ad8539a5d637d68ee826179a5.camel@intel.com>
On Fri, Apr 03, 2026, Rick P Edgecombe wrote:
> On Fri, 2026-04-03 at 09:30 -0700, Sean Christopherson wrote:
> > > > It comes down to a tradeoff. Should we prioritize code simplicity by dropping the function,
> > > > or keep it to explicitly catch this misbehaving guest corner case?
> > >
> > > I think from KVM's perspective it doesn't want to help the guest behave
> > > correctly.
> >
> > Uh, yes KVM does does. KVM is responsible for emulating the APIC timer, isn't it?
>
> Yea totally. We need to emulate the interface accurately. But we are kind of
> making up the contract after the fact. If the guest performs the wrong type of
> MSR write, should we make the contract that the VMM should help it catch it's
> mistake?
>
> >
> > > So we can ignore that I think. But it does really care to not define
> > > any specific guest ABI that it has to maintain. So tdx_has_emulated_msr() has
> > > some value there. And even more, it wants to not allow the guest to hurt the
> > > host.
> > >
> > > On the latter point, another problem with deleting tdx_has_emulated_msr() is the
> > > current code path skips the checks done in the other MSR paths. So we would need
> > > to call some appropriate higher up MSR helper to protect the host? And that
> > > wades into the CPUID bit consistency issues.
> > >
> > > So maybe... could we do a more limited version of the deletion where we allow
> > > all the APIC MSRs through? We'd have to check that it won't cause problems.
> >
> > What? No. KVM can't get actually read/write most (all?) MSRs, allowing access
> > is far worse than returning an error, as for all intents and purposes KVM will
> > silently drop writes, and return garbage on reads.
> >
> > > Failing that, we should maybe just explicitly list the ones TDX supports rather
> > > than the current way we define the APIC ones. As you mention below, it's not
> > > correct in other ways too so it could be more robust.
> >
> > No? Don't we just want to allow access to MSRs that aren't accelerated? What
> > the TDX-Module supports is largely irrelevant, I think.
>
> Not sure if I might be missing the point here. As above, we don't have enough
> info to know which MSRs are accelerated. If the guest enabled #VE reduction, it
> changes which ones are accelerated and the VMM is not notified.
What does the "accleration" in that case? Or does it reduce which ones are
accelerated?
> I think the below is a sane limitation, but doesn't lets KVM perfectly notify
> the guest when it screws up.
>
> So the line would be to block MSRs that can never be emulated.
>
> BTW, I've been treating this secret contract change as an arch mistake to at
> least not build on. It's a whole subject though... Let me know if you are
> interested in the details.
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Fix APIC MSR ranges in tdx_has_emulated_msr()
From: Edgecombe, Rick P @ 2026-04-03 19:30 UTC (permalink / raw)
To: seanjc@google.com
Cc: linux-coco@lists.linux.dev, Hansen, Dave, dmaluka@chromium.org,
bp@alien8.de, kas@kernel.org, x86@kernel.org,
binbin.wu@linux.intel.com, linux-kernel@vger.kernel.org,
dave.hansen@linux.intel.com, tglx@kernel.org, kvm@vger.kernel.org,
hpa@zytor.com, pbonzini@redhat.com, mingo@redhat.com,
Yamahata, Isaku
In-Reply-To: <adAQCXI-7Yj0wcKA@google.com>
On Fri, 2026-04-03 at 12:07 -0700, Sean Christopherson wrote:
> > > No? Don't we just want to allow access to MSRs that aren't accelerated?
> > > What the TDX-Module supports is largely irrelevant, I think.
> >
> > Not sure if I might be missing the point here. As above, we don't have
> > enough info to know which MSRs are accelerated. If the guest enabled #VE
> > reduction, it changes which ones are accelerated and the VMM is not
> > notified.
>
> What does the "accleration" in that case? Or does it reduce which ones are
> accelerated?
I mean ones where wrmsr is handled by the TDX module instead of generating a #VE
that gets morphed into TDVMCALL by the guest. Actually usually called "native",
but I just reused your "accelerated" term from the mail.
So... "Reduced #VE" (also called "VE reduction") reduces which things cause a
#VE. The guest opts into it and the TDX module starts behaving differently. It's
kind of grab bag of changes including changing CPUID behavior, which is another
wrinkle. It was intended to fixup guest side TDX arch issues.
^ permalink raw reply
* Re: [PATCH 2/2] x86/tdx: Accept hotplugged memory before online
From: Edgecombe, Rick P @ 2026-04-03 19:41 UTC (permalink / raw)
To: Reshetova, Elena, pbonzini@redhat.com, prsampat@amd.com,
Duan, Zhenzhong
Cc: x86@kernel.org, marcandre.lureau@redhat.com, kas@kernel.org,
dave.hansen@linux.intel.com, linux-kernel@vger.kernel.org,
mingo@redhat.com, bp@alien8.de, Qiang, Chenyi, tglx@kernel.org,
hpa@zytor.com, kvm@vger.kernel.org, linux-coco@lists.linux.dev
In-Reply-To: <IA1PR11MB94952B22F6EB648ED7C1AE3CE75EA@IA1PR11MB9495.namprd11.prod.outlook.com>
On Fri, 2026-04-03 at 10:37 +0000, Reshetova, Elena wrote:
> > > > So the part about whether a triggered accept succeeds or returns an
> > > > already accepted error is already under the control of the host. > >
> > > > I.e., if we don't have the zeroing behavior, the host can already > >
> > > > cause the page to get zeroed. So I don't think anything is > >
> > > > regressed. Both come down to how careful the guest is about what it > >
> > > > accepts.
> >
> > Yes, and my point is that we should not allow guest to freely double
> > accepting ever.
> > For any use case that requires releasing memory and accepting it > back, it
> > should be explicit action by the guest to track that memory > has been
> > "released" (under correct and safe conditions) and then it > is ok to accept
> > it back (even if it doesnt mean physically accepting > it) and in this case
> > it is ok (and even strongly desired) to zero the > page to simulate the
> > normal accept behaviour.
Hmm, it doesn't seem like you engaged with my point. Or at least I'm not
following what is exposed?
So I'm going to assume you agree that this procedure would not open up any
specific new capabilities for the host that don't exist today. And instead you
are just saying that the guest should have infrastructure to not double accept
memory in the first place.
But the problem here is not that the guest losing track of the accept state
actually. It is that the guest relies on the host to actually zap the S-EPT
before re-plugging memory at the same physical address space. So the guest is
tracking that the memory is released correctly. Better tracking will not help.
It relies on host behavior to not hit a double accept.
TDX connect will use this "unaccept" seamcall, so I asked Zhenzhong (Cced) how
much of what we need for that solution will just get added for TDX connect
anyway. It seems like we should make sure the same solution will work for both
SNP and TDX and keep the options open at this stage.
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Chang S. Bae @ 2026-04-03 22:05 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson
Cc: Kiryl Shutsemau, kvm, x86, linux-coco, linux-kernel
In-Reply-To: <88e9d7f0-35b8-4559-9f4d-c7daf1af6012@redhat.com>
On 4/3/2026 9:03 AM, Paolo Bonzini wrote:
>
> But until the kernel starts using APX, I would do the save/restore near
> kvm_load_xfeatures(), because __vmx_vcpu_run()/__svm_vcpu_run() would
> have to check whether xcr0.apx is set or not.
Right, I'd much prefer this. Then, it requires to audit whether any
fast-path handler could access EGPRs.
But there are cases with the new {RD|WR}MSR (MSR_IMM) instructions that
appear to access GPRs. Because of this, the EGPR saving/restoring needs
to happen earlier.
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Fix APIC MSR ranges in tdx_has_emulated_msr()
From: Sean Christopherson @ 2026-04-03 23:07 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: linux-coco@lists.linux.dev, Dave Hansen, dmaluka@chromium.org,
bp@alien8.de, kas@kernel.org, x86@kernel.org,
binbin.wu@linux.intel.com, linux-kernel@vger.kernel.org,
dave.hansen@linux.intel.com, tglx@kernel.org, kvm@vger.kernel.org,
hpa@zytor.com, pbonzini@redhat.com, mingo@redhat.com,
Isaku Yamahata
In-Reply-To: <ec581571ab4141ed77e6979b3e03773e301a62c1.camel@intel.com>
On Fri, Apr 03, 2026, Rick P Edgecombe wrote:
> On Fri, 2026-04-03 at 12:07 -0700, Sean Christopherson wrote:
> > > > No? Don't we just want to allow access to MSRs that aren't accelerated?
> > > > What the TDX-Module supports is largely irrelevant, I think.
> > >
> > > Not sure if I might be missing the point here. As above, we don't have
> > > enough info to know which MSRs are accelerated. If the guest enabled #VE
> > > reduction, it changes which ones are accelerated and the VMM is not
> > > notified.
> >
> > What does the "accleration" in that case? Or does it reduce which ones are
> > accelerated?
>
> I mean ones where wrmsr is handled by the TDX module instead of generating a #VE
> that gets morphed into TDVMCALL by the guest. Actually usually called "native",
> but I just reused your "accelerated" term from the mail.
It's neither. Precision matters here, otherwise I can't follow along. Accelerated
means the CPU virtualizes it without software involvement. Native would mean the
guest has direct access to bare metal hardware. IIUC, what's happening here is
that the TDX-Module is emulating x2APIC stuff.
> So... "Reduced #VE" (also called "VE reduction") reduces which things cause a
> #VE. The guest opts into it and the TDX module starts behaving differently. It's
> kind of grab bag of changes including changing CPUID behavior, which is another
> wrinkle. It was intended to fixup guest side TDX arch issues.
And KVM has no visilibity into which mode the guest has selected? That's awful.
If KVM has no visiblity, then I don't see an option other than for KVM to advertise
and emulate what it can at all times, and it becomes the guest's responsibility
to not screw up. I guess it's not really any different from not trying to use
MMIO accesses after switching to x2APIC mode.
^ 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