* 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
* Re: [PATCH] KVM: TDX: Fix APIC MSR ranges in tdx_has_emulated_msr()
From: Edgecombe, Rick P @ 2026-04-04 0:11 UTC (permalink / raw)
To: seanjc@google.com
Cc: kvm@vger.kernel.org, Hansen, Dave, Yamahata, Isaku,
dmaluka@chromium.org, x86@kernel.org, kas@kernel.org,
bp@alien8.de, linux-kernel@vger.kernel.org, mingo@redhat.com,
dave.hansen@linux.intel.com, binbin.wu@linux.intel.com,
linux-coco@lists.linux.dev, hpa@zytor.com, tglx@kernel.org,
pbonzini@redhat.com
In-Reply-To: <adBIPdBA-aJWcUrY@google.com>
On Fri, 2026-04-03 at 16:07 -0700, Sean Christopherson wrote:
> > 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.
Oh, sorry.
> IIUC, what's happening here is that the TDX-Module is emulating x2APIC stuff.
I'll stick to this language. The tdx docs call them differently of course.
"native" there is about #VE or not. So I can talk about it with respect to VE or
!VE.
>
> > 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.
Yea, on both accounts. So where we are at with this is, starting to reject
changes that build on the pattern. We haven't gone so far as to ask for a
feature to notify the host of the guest opt-ins. But I wouldn't say we have a
grand design in mind either. If you have any clarity, please feel free to drop a
quotable.
>
> 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.
Like your diff? Expose any MSRs that might be emulated in the TDX paradigm. But
don't expose all MSRs that KVM supports.
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Paolo Bonzini @ 2026-04-04 5:16 UTC (permalink / raw)
To: Chang S. Bae
Cc: Sean Christopherson, Kiryl Shutsemau, kvm, x86, linux-coco,
linux-kernel
In-Reply-To: <aea176c8-35de-4042-bf98-e42ce05f93fb@intel.com>
[-- Attachment #1: Type: text/plain, Size: 951 bytes --]
On Sat, Apr 4, 2026 at 12:05 AM Chang S. Bae <chang.seok.bae@intel.com> wrote:
>
> 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.
You're right about fast paths... so something like the attached patch.
It is not too bad to translate into assembly, where it could use
alternatives (in the same way as
RESTORE_GUEST_SPEC_CTRL/RESTORE_GUEST_SPEC_CTRL_BODY) in place of
static_cpu_has(). Maybe it's best to bite the bullet and do it
already...
Paolo
[-- Attachment #2: apx.patch --]
[-- Type: text/x-patch, Size: 6071 bytes --]
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 959fcc01ee0f..9a1766037b6f 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -887,6 +887,7 @@ struct kvm_vcpu_arch {
struct fpu_guest guest_fpu;
u64 xcr0;
+ u64 early_xcr0;
u64 guest_supported_xcr0;
u64 ia32_xss;
u64 guest_supported_xss;
@@ -2101,6 +2102,20 @@ void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end);
int load_pdptrs(struct kvm_vcpu *vcpu, unsigned long cr3);
+void __kvm_load_guest_apx(struct kvm_vcpu *vcpu);
+static inline void kvm_load_guest_apx(struct kvm_vcpu *vcpu)
+{
+ if (static_cpu_has(X86_FEATURE_APX))
+ __kvm_load_guest_apx(vcpu);
+}
+
+void __kvm_save_guest_apx(struct kvm_vcpu *vcpu);
+static inline void kvm_save_guest_apx(struct kvm_vcpu *vcpu)
+{
+ if (static_cpu_has(X86_FEATURE_APX))
+ __kvm_save_guest_apx(vcpu);
+}
+
int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
const void *val, int bytes);
diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
index 657f5f743ed9..e44cfed94160 100644
--- a/arch/x86/kvm/reverse_cpuid.h
+++ b/arch/x86/kvm/reverse_cpuid.h
@@ -31,6 +31,7 @@
/* Intel-defined sub-features, CPUID level 0x00000007:1 (EDX) */
#define X86_FEATURE_AVX_VNNI_INT8 KVM_X86_FEATURE(CPUID_7_1_EDX, 4)
#define X86_FEATURE_AVX_NE_CONVERT KVM_X86_FEATURE(CPUID_7_1_EDX, 5)
+#define KVM_X86_FEATURE_APX KVM_X86_FEATURE(CPUID_7_1_EDX, 7)
#define X86_FEATURE_AMX_COMPLEX KVM_X86_FEATURE(CPUID_7_1_EDX, 8)
#define X86_FEATURE_AVX_VNNI_INT16 KVM_X86_FEATURE(CPUID_7_1_EDX, 10)
#define X86_FEATURE_PREFETCHITI KVM_X86_FEATURE(CPUID_7_1_EDX, 14)
@@ -151,6 +152,7 @@ static __always_inline u32 __feature_translate(int x86_feature)
KVM_X86_TRANSLATE_FEATURE(TSA_SQ_NO);
KVM_X86_TRANSLATE_FEATURE(TSA_L1_NO);
KVM_X86_TRANSLATE_FEATURE(MSR_IMM);
+ KVM_X86_TRANSLATE_FEATURE(APX);
default:
return x86_feature;
}
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e6477affac9a..c0a8143f274c 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4359,6 +4359,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
vcpu->arch.host_debugctl != svm->vmcb->save.dbgctl)
update_debugctlmsr(svm->vmcb->save.dbgctl);
+ kvm_load_guest_apx(vcpu);
kvm_wait_lapic_expire(vcpu);
/*
@@ -4381,6 +4382,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
vcpu->arch.regs[VCPU_REGS_RSP] = svm->vmcb->save.rsp;
vcpu->arch.regs[VCPU_REGS_RIP] = svm->vmcb->save.rip;
}
+ kvm_save_guest_apx(vcpu);
vcpu->arch.regs_dirty = 0;
if (unlikely(svm->vmcb->control.exit_code == SVM_EXIT_NMI))
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 8b24e682535b..c4c0da9281c1 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7693,10 +7693,12 @@ fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
else if (force_immediate_exit)
smp_send_reschedule(vcpu->cpu);
+ kvm_load_guest_apx(vcpu);
kvm_wait_lapic_expire(vcpu);
/* The actual VMENTER/EXIT is in the .noinstr.text section. */
vmx_vcpu_enter_exit(vcpu, __vmx_vcpu_run_flags(vmx));
+ kvm_save_guest_apx(vcpu);
/* All fields are clean at this point */
if (kvm_is_using_evmcs()) {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0757b93e528d..69abfdd946dd 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1220,9 +1220,13 @@ static void kvm_load_xfeatures(struct kvm_vcpu *vcpu, bool load_guest)
if (!kvm_is_cr4_bit_set(vcpu, X86_CR4_OSXSAVE))
return;
- if (vcpu->arch.xcr0 != kvm_host.xcr0)
+ /*
+ * Do not load the definitive XCR0 yet; vcpu->arch.early_xcr0 keeps
+ * APX enabled so that the kernel can move to and from r16...r31.
+ */
+ if (vcpu->arch.early_xcr0 != kvm_host.xcr0)
xsetbv(XCR_XFEATURE_ENABLED_MASK,
- load_guest ? vcpu->arch.xcr0 : kvm_host.xcr0);
+ load_guest ? vcpu->arch.early_xcr0 : kvm_host.xcr0);
if (guest_cpu_cap_has(vcpu, X86_FEATURE_XSAVES) &&
vcpu->arch.ia32_xss != kvm_host.xss)
@@ -1302,6 +1302,11 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
vcpu->arch.xcr0 = xcr0;
+ /* APX is needed to save/restore registers for fast path WRMSR. */
+ vcpu->arch.early_xcr0 = xcr0;
+ if (guest_cpu_cap_has(vcpu, X86_FEATURE_APX))
+ vcpu->arch.early_xcr0 |= kvm_host.xcr0 & XFEATURE_MASK_APX;
+
if ((xcr0 ^ old_xcr0) & XFEATURE_MASK_EXTEND)
vcpu->arch.cpuid_dynamic_bits_dirty = true;
return 0;
@@ -11056,6 +11061,49 @@ static void kvm_vcpu_reload_apic_access_page(struct kvm_vcpu *vcpu)
kvm_x86_call(set_apic_access_page_addr)(vcpu);
}
+/*
+ * Assuming the kernel does not use APX for now. When
+ * the kernel starts using APX this needs to move into
+ * assembly, and KVM_GET/SET_XSAVE needs to fill in
+ * EGPRs from vcpu->arch.regs.
+ */
+void __kvm_load_guest_apx(struct kvm_vcpu *vcpu)
+{
+ if (vcpu->arch.early_xcr0 != vcpu->arch.xcr0)
+ xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
+
+ if (!(vcpu->arch.xcr0 & XFEATURE_MASK_APX))
+ return;
+
+ WARN_ON_ONCE(!irqs_disabled());
+
+ asm("mov %[r16], %%r16\n"
+ "mov %[r17], %%r17\n" // ...
+ : : [r16] "m" (vcpu->arch.regs[16]),
+ [r17] "m" (vcpu->arch.regs[17]));
+}
+
+/*
+ * Assuming the kernel does not use APX for now. When
+ * the kernel starts using APX this needs to move into
+ * assembly and zero out APX registers for the host.
+ */
+void __kvm_save_guest_apx(struct kvm_vcpu *vcpu)
+{
+ if (vcpu->arch.early_xcr0 != vcpu->arch.xcr0)
+ xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.early_xcr0);
+
+ if (!(vcpu->arch.xcr0 & XFEATURE_MASK_APX))
+ return;
+
+ WARN_ON_ONCE(!irqs_disabled());
+
+ asm("mov %%r16, %[r16]\n"
+ "mov %%r17, %[r17]\n" // ...
+ : : [r16] "m" (vcpu->arch.regs[16]),
+ [r17] "m" (vcpu->arch.regs[17]));
+}
+
/*
* Called within kvm->srcu read side.
* Returns 1 to let vcpu_run() continue the guest execution loop without
^ permalink raw reply related
* Re: [PATCH 2/2] x86/virt/tdx: Use PFN directly for unmapping guest private memory
From: Paolo Bonzini @ 2026-04-04 6:39 UTC (permalink / raw)
To: Yan Zhao, Xiaoyao Li
Cc: seanjc, dave.hansen, tglx, mingo, bp, kas, x86, linux-kernel, kvm,
linux-coco, kai.huang, rick.p.edgecombe, yilun.xu, vannapurve,
ackerleytng, sagis, binbin.wu, isaku.yamahata
In-Reply-To: <abu6MVdXar3MmTdF@yzhao56-desk.sh.intel.com>
On 3/19/26 09:56, Yan Zhao wrote:
> On Thu, Mar 19, 2026 at 04:56:10PM +0800, Xiaoyao Li wrote:
>> So why not considering option 2?
>>
>> 2. keep tdx_quirk_reset_page() as-is for the cases of
>> tdx_reclaim_page() and tdx_reclaim_td_control_pages() that have the
>> struct page. But only change tdx_sept_remove_private_spte() to use
>> tdx_quirk_reset_paddr() directly.
>>
>> It will need export tdx_quirk_reset_paddr() for KVM. I think it will be OK?
> I don't think it's necessary. But if we have to export an extra API, IMHO,
> tdx_quirk_reset_pfn() is better than tdx_quirk_reset_paddr(). Otherwise,
> why not only expose tdx_quirk_reset_paddr()?
That works for me, it seems the cleanest.
Paolo
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Sean Christopherson @ 2026-04-06 15:28 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Chang S. Bae, Kiryl Shutsemau, kvm, x86, linux-coco, linux-kernel,
Andrew Cooper
In-Reply-To: <CABgObfY05NU8DS82jkwpF89_p1nR7VJ30HBq_xaMg_u+-j=0Cw@mail.gmail.com>
+Andrew
On Sat, Apr 04, 2026, Paolo Bonzini wrote:
> On Sat, Apr 4, 2026 at 12:05 AM Chang S. Bae <chang.seok.bae@intel.com> wrote:
> >
> > 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.
>
> You're right about fast paths...
Ya, potential fastpath usage is why I wanted to just context switch around
entry/exit.
> so something like the attached patch.
> It is not too bad to translate into assembly, where it could use
> alternatives (in the same way as
> RESTORE_GUEST_SPEC_CTRL/RESTORE_GUEST_SPEC_CTRL_BODY) in place of
> static_cpu_has(). Maybe it's best to bite the bullet and do it
> already...
My strong vote is to context switch in assembly, but _conditionally_ context
switch R16-R31. All of this started from Andrew's comment:
: You can't unconditionally use PUSH2/POP2 in the VMExit, because at that
: point in time it's the guest's XCR0 in context. If the guest has APX
: disabled, PUSH2 in the VMExit path will #UD.
:
: You either need two VMExit handlers, one APX and one non-APX and choose
: based on the guest XCR0 value, or you need a branch prior to regaining
: speculative safety, or you need to save/restore XCR0 as the first
: action. It's horrible any way you look at it.
But that second paragraph isn't quite correct, at least not for KVM. Specifically,
"need a branch prior to regaining speculative safety" isn't correct, as that holds
true if and only if "regaining speculative safety" requires executing code that
might access R16-R31. If we massage __vmx_vcpu_run() to restore SPEC_CTRL in
assembly, same as __svm_vcpu_run(), then __{svm,vmx}_vcpu_run() can simply context
switch R16-R31 if and only if APX is enabled in XCR0.
KVM always intercepts XCR0 writes (when XCR0 isn't context switched by "harware",
i.e. ignoring SEV-ES+ and TDX guests), and IIUC all access to R16-R31 is gated on
XCR0.APX=1. So unless I'm missing something (or hardware is flawed and lets the
guest speculative consume R16-R31, which would be sad), it's perfectly safe to
run the guest with host state in R16-R31.
That would avoid pointlessly context switching 16 registers when APX is not being
used by the guest, and would avoid having to write XCR0 in the fastpath.
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 959fcc01ee0f..9a1766037b6f 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -887,6 +887,7 @@ struct kvm_vcpu_arch {
> struct fpu_guest guest_fpu;
>
> u64 xcr0;
> + u64 early_xcr0;
...
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0757b93e528d..69abfdd946dd 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1220,9 +1220,13 @@ static void kvm_load_xfeatures(struct kvm_vcpu *vcpu, bool load_guest)
> if (!kvm_is_cr4_bit_set(vcpu, X86_CR4_OSXSAVE))
> return;
>
> - if (vcpu->arch.xcr0 != kvm_host.xcr0)
> + /*
> + * Do not load the definitive XCR0 yet; vcpu->arch.early_xcr0 keeps
> + * APX enabled so that the kernel can move to and from r16...r31.
> + */
> + if (vcpu->arch.early_xcr0 != kvm_host.xcr0)
> xsetbv(XCR_XFEATURE_ENABLED_MASK,
> - load_guest ? vcpu->arch.xcr0 : kvm_host.xcr0);
> + load_guest ? vcpu->arch.early_xcr0 : kvm_host.xcr0);
Even _if_ we want to play XCR0 games, tracking early_xcr0 is unnecessary. This
can be:
/*
* XCR0 is context switched around VM-Enter/VM-Exit if APX is enabled
* in the host but not in the guest.
*/
if (vcpu->arch.xcr0 != kvm_host.xcr0 &&
(!cpu_feature_enabled(X86_FEATURE_APX) ||
vcpu->arch.xcr0 & XFEATURE_MASK_APX))
xsetbv(XCR_XFEATURE_ENABLED_MASK,
load_guest ? vcpu->arch.xcr0 : kvm_host.xcr0);
And then __kvm_load_guest_apx()
<context switch R16-R31>
if (cpu_feature_enabled(X86_FEATURE_APX) &&
!(vcpu->arch.xcr0 & & XFEATURE_MASK_APX))
xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
And __kvm_save_guest_apx() would reverse the order of __kvm_load_guest_apx().
> @@ -11056,6 +11061,49 @@ static void kvm_vcpu_reload_apic_access_page(struct kvm_vcpu *vcpu)
> kvm_x86_call(set_apic_access_page_addr)(vcpu);
> }
>
> +/*
> + * Assuming the kernel does not use APX for now. When
> + * the kernel starts using APX this needs to move into
> + * assembly, and KVM_GET/SET_XSAVE needs to fill in
> + * EGPRs from vcpu->arch.regs.
> + */
> +void __kvm_load_guest_apx(struct kvm_vcpu *vcpu)
> +{
> + if (vcpu->arch.early_xcr0 != vcpu->arch.xcr0)
> + xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
This is wrong. The "real" xcr0 needs to be loaded *after* accessing R16+.
> + if (!(vcpu->arch.xcr0 & XFEATURE_MASK_APX))
> + return;
> +
> + WARN_ON_ONCE(!irqs_disabled());
> +
> + asm("mov %[r16], %%r16\n"
> + "mov %[r17], %%r17\n" // ...
> + : : [r16] "m" (vcpu->arch.regs[16]),
> + [r17] "m" (vcpu->arch.regs[17]));
> +}
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Sean Christopherson @ 2026-04-06 15:40 UTC (permalink / raw)
To: Dave Hansen
Cc: Paolo Bonzini, Kiryl Shutsemau, kvm, x86, linux-coco,
linux-kernel, Chang S . Bae
In-Reply-To: <fb9e61cc-20df-47cd-b324-1dd8d5985372@intel.com>
On Fri, Apr 03, 2026, Dave Hansen wrote:
> 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.
Ooof, one nasty wrinkle to prepare for is an NMI that arrives after VM-Exit on
Intel CPUs. Unless Intel extends VMX to context switch XCR0 at VM-Entry/VM-Exit,
and/or provides GIF-like functionality (which would be awesome!), it will be
possible for an NMI to be taken with the guest's XCR0 loaded, i.e. with XCR0.APX=0
even when APX is fully enabled in the host.
> 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] dma-buf: heaps: system: document system_cc_shared heap
From: T.J. Mercier @ 2026-04-06 20:20 UTC (permalink / raw)
To: Jiri Pirko
Cc: dri-devel, linaro-mm-sig, iommu, linux-media, sumit.semwal,
benjamin.gaignard, Brian.Starkey, jstultz, christian.koenig,
m.szyprowski, robin.murphy, jgg, leon, sean.anderson, ptesarik,
catalin.marinas, aneesh.kumar, suzuki.poulose, steven.price,
thomas.lendacky, john.allen, ashish.kalra, suravee.suthikulpanit,
linux-coco
In-Reply-To: <20260402141103.598495-1-jiri@resnulli.us>
On Thu, Apr 2, 2026 at 7:11 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> From: Jiri Pirko <jiri@nvidia.com>
>
> Document the system_cc_shared dma-buf heap that was introduced
> recently. Describe its purpose, availability conditions and
> relation to confidential computing VMs.
>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
> Documentation/userspace-api/dma-buf-heaps.rst | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst
> index 05445c83b79a..591732393e7d 100644
> --- a/Documentation/userspace-api/dma-buf-heaps.rst
> +++ b/Documentation/userspace-api/dma-buf-heaps.rst
> @@ -16,6 +16,14 @@ following heaps:
>
> - The ``system`` heap allocates virtually contiguous, cacheable, buffers.
>
> + - The ``system_cc_shared`` heap allocates virtually contiguous, cacheable,
> + buffers using shared (decrypted) memory. It is only present on
> + confidential computing (CoCo) VMs where memory encryption is active
> + (e.g., AMD SEV, Intel TDX). The allocated pages have the encryption
> + bit cleared, making them accessible for device DMA without TDISP
> + support. On non-CoCo VMs configurations, this heap is
"non-CoCo VM configurations"
> + not registered.
Doesn't seem like you need to wrap this line.
with that: Reviewed-by: T.J.Mercier <tjmercier@google.com>
> +
> - The ``default_cma_region`` heap allocates physically contiguous,
> cacheable, buffers. Only present if a CMA region is present. Such a
> region is usually created either through the kernel commandline
Each paragraph starting with '-' confused me for a second there. Those
aren't part of the diff. :)
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Paolo Bonzini @ 2026-04-06 21:41 UTC (permalink / raw)
To: Sean Christopherson
Cc: Chang S. Bae, Kiryl Shutsemau, kvm, the arch/x86 maintainers,
linux-coco, Kernel Mailing List, Linux, Andrew Cooper
In-Reply-To: <adPRA4ZhnvbaXSn0@google.com>
Il lun 6 apr 2026, 17:28 Sean Christopherson <seanjc@google.com> ha scritto:
> > You're right about fast paths...
>
> Ya, potential fastpath usage is why I wanted to just context switch around
> entry/exit.
>
> > so something like the attached patch.
> > It is not too bad to translate into assembly, where it could use
> > alternatives (in the same way as
> > RESTORE_GUEST_SPEC_CTRL/RESTORE_GUEST_SPEC_CTRL_BODY) in place of
> > static_cpu_has(). Maybe it's best to bite the bullet and do it
> > already...
>
> My strong vote is to context switch in assembly, but _conditionally_ context
> switch R16-R31.
>
> But that second paragraph isn't quite correct, at least not for KVM. Specifically,
> "need a branch prior to regaining speculative safety" isn't correct, as that holds
> true if and only if "regaining speculative safety" requires executing code that
> might access R16-R31. If we massage __vmx_vcpu_run() to restore SPEC_CTRL in
> assembly, same as __svm_vcpu_run(), then __{svm,vmx}_vcpu_run() can simply context
> switch R16-R31 if and only if APX is enabled in XCR0.
I might even have patches for that lying around (the SPEC_CTRL part).
> KVM always intercepts XCR0 writes (when XCR0 isn't context switched by "hardware",
> i.e. ignoring SEV-ES+ and TDX guests), and IIUC all access to R16-R31 is gated on
> XCR0.APX=1
Right, fortunately.
> . So unless I'm missing something (or hardware is flawed and lets the
> guest speculative consume R16-R31, which would be sad), it's perfectly safe to
> run the guest with host state in R16-R31.
>
> That would avoid pointlessly context switching 16 registers when APX is not being
> used by the guest, and would avoid having to write XCR0 in the fastpath.
For now yes, but once/if the kernel starts using the registers there's
no way out of writing XCR0 for APX-disabled guests in the fast path.
If we ignore that, we can keep guest XCR0 all the time for now, and
that would be:
- move SPEC_CTRL to assembly
- not changing XCR0 handling at all
- use XCR0 in addition to just static_cpu_has(X86_FEATURE_APX) to make
r16-r31 swap conditional
> > - if (vcpu->arch.xcr0 != kvm_host.xcr0)
> > + /*
> > + * Do not load the definitive XCR0 yet; vcpu->arch.early_xcr0 keeps
> > + * APX enabled so that the kernel can move to and from r16...r31.
> > + */
> > + if (vcpu->arch.early_xcr0 != kvm_host.xcr0)
> > xsetbv(XCR_XFEATURE_ENABLED_MASK,
> > - load_guest ? vcpu->arch.xcr0 : kvm_host.xcr0);
> > + load_guest ? vcpu->arch.early_xcr0 : kvm_host.xcr0);
>
> Even _if_ we want to play XCR0 games,
(which depends on whether we want to be ready for kernel usage of APX, right?)
> tracking early_xcr0 is unnecessary. This can be:
>
> /*
> * XCR0 is context switched around VM-Enter/VM-Exit if APX is enabled
> * in the host but not in the guest.
> */
> if (vcpu->arch.xcr0 != kvm_host.xcr0 &&
> (!cpu_feature_enabled(X86_FEATURE_APX) ||
> vcpu->arch.xcr0 & XFEATURE_MASK_APX))
> xsetbv(XCR_XFEATURE_ENABLED_MASK,
> load_guest ? vcpu->arch.xcr0 : kvm_host.xcr0);
This is a bit more complex however, because in the end early_xcr0 is
precomputing the same conditions and optimizations. For example...
> > +void __kvm_load_guest_apx(struct kvm_vcpu *vcpu)
> > +{
> > + if (vcpu->arch.early_xcr0 != vcpu->arch.xcr0)
> > + xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
>
> This is wrong. The "real" xcr0 needs to be loaded *after* accessing R16+.
... this is actually the same optimization you mention above: the real
xcr0 only needs to be loaded if APX is off in the guest, and in that
case you don't need to load r16-r31. So you can load xcr0 first, and
then any components (for now only APX) that need to be swapped.
> > + if (!(vcpu->arch.xcr0 & XFEATURE_MASK_APX))
> > + return;
... Because the loads are conditional on APX being enabled in the real xcr0.
Paolo
> > +
> > + WARN_ON_ONCE(!irqs_disabled());
> > +
> > + asm("mov %[r16], %%r16\n"
> > + "mov %[r17], %%r17\n" // ...
> > + : : [r16] "m" (vcpu->arch.regs[16]),
> > + [r17] "m" (vcpu->arch.regs[17]));
> > +}
>
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Sean Christopherson @ 2026-04-06 22:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Chang S. Bae, Kiryl Shutsemau, kvm, the arch/x86 maintainers,
linux-coco, Kernel Mailing List, Linux, Andrew Cooper
In-Reply-To: <CABgObfbLm3FR4f_nv5EyYJx4jwfeBaVgTLhr7P++hmhCP98e3Q@mail.gmail.com>
On Mon, Apr 06, 2026, Paolo Bonzini wrote:
> Il lun 6 apr 2026, 17:28 Sean Christopherson <seanjc@google.com> ha scritto:
> > > You're right about fast paths...
> >
> > Ya, potential fastpath usage is why I wanted to just context switch around
> > entry/exit.
> >
> > > so something like the attached patch.
> > > It is not too bad to translate into assembly, where it could use
> > > alternatives (in the same way as
> > > RESTORE_GUEST_SPEC_CTRL/RESTORE_GUEST_SPEC_CTRL_BODY) in place of
> > > static_cpu_has(). Maybe it's best to bite the bullet and do it
> > > already...
> >
> > My strong vote is to context switch in assembly, but _conditionally_ context
> > switch R16-R31.
> >
> > But that second paragraph isn't quite correct, at least not for KVM. Specifically,
> > "need a branch prior to regaining speculative safety" isn't correct, as that holds
> > true if and only if "regaining speculative safety" requires executing code that
> > might access R16-R31. If we massage __vmx_vcpu_run() to restore SPEC_CTRL in
> > assembly, same as __svm_vcpu_run(), then __{svm,vmx}_vcpu_run() can simply context
> > switch R16-R31 if and only if APX is enabled in XCR0.
>
> I might even have patches for that lying around (the SPEC_CTRL part).
>
> > KVM always intercepts XCR0 writes (when XCR0 isn't context switched by "hardware",
> > i.e. ignoring SEV-ES+ and TDX guests), and IIUC all access to R16-R31 is gated on
> > XCR0.APX=1
>
> Right, fortunately.
>
> > . So unless I'm missing something (or hardware is flawed and lets the
> > guest speculative consume R16-R31, which would be sad), it's perfectly safe to
> > run the guest with host state in R16-R31.
> >
> > That would avoid pointlessly context switching 16 registers when APX is not being
> > used by the guest, and would avoid having to write XCR0 in the fastpath.
>
> For now yes, but once/if the kernel starts using the registers there's
> no way out of writing XCR0 for APX-disabled guests in the fast path.
Why's that? So long as KVM uses vcpu->arch.regs[R16-R31] as the source of truth
when emulating anything, there's no danger of taking a #UD in the host due to
accessing R16-R31 with XCR0.APX=0. There's not even any danger of consuming stale
guest state, e.g. in case KVM screws up accesses R16-R31 instead of generating #UD,
as the value in regs[] will still be the guest's last written value.
If we wanted be paranoid, we could add sanity checks to ensure R16-R31 don't show
up in hardware-provided informational fields, but to some extent that's orthogonal
to how KVM maintains guest values.
> If we ignore that, we can keep guest XCR0 all the time for now, and
> that would be:
> - move SPEC_CTRL to assembly
> - not changing XCR0 handling at all
> - use XCR0 in addition to just static_cpu_has(X86_FEATURE_APX) to make
> r16-r31 swap conditional
>
> > > - if (vcpu->arch.xcr0 != kvm_host.xcr0)
> > > + /*
> > > + * Do not load the definitive XCR0 yet; vcpu->arch.early_xcr0 keeps
> > > + * APX enabled so that the kernel can move to and from r16...r31.
> > > + */
> > > + if (vcpu->arch.early_xcr0 != kvm_host.xcr0)
> > > xsetbv(XCR_XFEATURE_ENABLED_MASK,
> > > - load_guest ? vcpu->arch.xcr0 : kvm_host.xcr0);
> > > + load_guest ? vcpu->arch.early_xcr0 : kvm_host.xcr0);
> >
> > Even _if_ we want to play XCR0 games,
>
> (which depends on whether we want to be ready for kernel usage of APX, right?)
No?
^ permalink raw reply
* Re: [PATCH v2 09/19] PCI/TSM: Support creating encrypted MMIO descriptors via TDISP Report
From: Alexey Kardashevskiy @ 2026-04-06 22:08 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: <20260403140858.GJ310919@nvidia.com>
On 4/4/26 01:08, Jason Gunthorpe wrote:
> 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?
This is what I am trying to clarify - if all ranges must be reported (as some think this is what the PCIe spec says), then no, not anywhere.
pcie r7, Table 11-16 TDI Report Structure, MMIO_RANGE:
"Each MMIO Range of the TDI is reported with the MMIO reporting offset added."
--
Alexey
^ permalink raw reply
* Re: [PATCH v2 09/19] PCI/TSM: Support creating encrypted MMIO descriptors via TDISP Report
From: Jason Gunthorpe @ 2026-04-06 22:21 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: <70912675-0737-4ebf-8ba0-ab9a2e493bbe@amd.com>
On Tue, Apr 07, 2026 at 08:08:51AM +1000, Alexey Kardashevskiy wrote:
>
>
> On 4/4/26 01:08, Jason Gunthorpe wrote:
> > 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?
>
> This is what I am trying to clarify - if all ranges must be reported
> (as some think this is what the PCIe spec says), then no, not
> anywhere.
>
> pcie r7, Table 11-16 TDI Report Structure, MMIO_RANGE:
>
> "Each MMIO Range of the TDI is reported with the MMIO reporting offset added."
I think the argument was something like it didn't have to report
non-secure ranges? But I don't know, it was hashed out in some thread
for ARM and then I know our folks looked at it and nobody pushed back
to insist that every single byte of the BAR had to be covered by a
reported range.
I wouldn't take the sentance you quoted as confirmation, you need a
sentance that says every single byte of the BAR is covered by a single
reported range.
Jason
^ 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