Linux Confidential Computing Development
 help / color / mirror / Atom feed
* RE: [PATCH v2 20/31] x86/virt/tdx: Add a helper to loop on TDX_INTERRUPTED_RESUMABLE
From: Ackerley Tng @ 2026-07-04 14:45 UTC (permalink / raw)
  To: Tian, Kevin, Xu Yilun, linux-coco@lists.linux.dev,
	linux-pci@vger.kernel.org, Williams, Dan J, x86@kernel.org
  Cc: Gao, Chao, Jiang, Dave, baolu.lu@linux.intel.com, Xu, Yilun,
	Duan, Zhenzhong, kvm@vger.kernel.org, Edgecombe, Rick P,
	dave.hansen@linux.intel.com, kas@kernel.org, Li, Xiaoyao,
	Verma, Vishal L, linux-kernel@vger.kernel.org
In-Reply-To: <BN9PR11MB52767E6C22CDF7DAD9D5FF1E8C582@BN9PR11MB5276.namprd11.prod.outlook.com>

"Tian, Kevin" <kevin.tian@intel.com> writes:

>> From: Xu Yilun <yilun.xu@linux.intel.com>
>> Sent: Saturday, March 28, 2026 12:01 AM
>>
>> +static u64 __maybe_unused __seamcall_ir_resched(sc_func_t sc_func, u64

How about __seamcall_retry_interrupted, to emphasize that this function
retries a certain kind of seamcall?

I think resched is part of the implementation, of what's done while
retrying, which might not need to be part of the name.

No strong opinion though. Thanks for considering.

>> fn,
>> +						struct tdx_module_args *args)
>> +{
>
> 'ir' sounds redundant with the trailing 'resched'?
>
> not big deal, just a bit confusing when seeing it in IOMMU side where
> 'ir' also refers to 'interrupt remapping' and is frequently used in
> irq_remapping.c... :)

^ permalink raw reply

* Re: [PATCH v2 20/31] x86/virt/tdx: Add a helper to loop on TDX_INTERRUPTED_RESUMABLE
From: Ackerley Tng @ 2026-07-04 14:42 UTC (permalink / raw)
  To: Xu Yilun, linux-coco, linux-pci, dan.j.williams, x86
  Cc: chao.gao, dave.jiang, baolu.lu, yilun.xu, zhenzhong.duan, kvm,
	rick.p.edgecombe, dave.hansen, kas, xiaoyao.li, vishal.l.verma,
	linux-kernel
In-Reply-To: <20260327160132.2946114-21-yilun.xu@linux.intel.com>

Xu Yilun <yilun.xu@linux.intel.com> writes:

> Add a helper to handle SEAMCALL return code TDX_INTERRUPTED_RESUMABLE.
>
> SEAMCALL returns TDX_INTERRUPTED_RESUMABLE to avoid stalling host for
> long time. After host has handled the interrupt, it calls the
> interrupted SEAMCALL again and TDX Module continues to execute. TDX
> Module made progress in this case and would eventually finish. An
> infinite loop in host should be safe.
>
> The helper is for SEAMCALL wrappers which output information by using
> seamcall_ret() or seamcall_saved_ret(). The 2 functions overwrite input
> arguments by outputs but much SEAMCALLs expect the same inputs to

Did you mean most instead of much?

> resume.
>
> The helper is not for special cases where the SEAMCALL expects modified
> inputs to resume. The helper is also not for SEAMCALLs with no output,
> do {...} while (r == TDX_INTERRUPTED_RESUMABLE) just works.
>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
>  arch/x86/virt/vmx/tdx/tdx.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index cd0948794b6c..294f36048c03 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -2084,6 +2084,29 @@ static inline u64 tdx_tdr_pa(struct tdx_td *td)
>  	return page_to_phys(td->tdr_page);
>  }
>
> +static u64 __maybe_unused __seamcall_ir_resched(sc_func_t sc_func, u64 fn,
> +						struct tdx_module_args *args)
> +{
> +	struct tdx_module_args _args;
> +	u64 r;
> +
> +	while (1) {
> +		_args = *(args);

Is this copying meant to ensure that every retry has the same input
args? Perhaps a comment should be added so it's obvious that every retry
will have the same input, and outputs from each retry are dropped.

> +		r = sc_retry(sc_func, fn, &_args);
> +		if (r != TDX_INTERRUPTED_RESUMABLE)
> +			break;
> +
> +		cond_resched();
> +	}
> +
> +	*args = _args;
> +
> +	return r;
> +}
> +
> +#define seamcall_ret_ir_resched(fn, args)	\
> +	__seamcall_ir_resched(__seamcall_ret, fn, args)
> +
>  noinstr u64 tdh_vp_enter(struct tdx_vp *td, struct tdx_module_args *args)
>  {
>  	args->rcx = td->tdvpr_pa;
> --
> 2.25.1

^ permalink raw reply

* Re: [PATCH v2 15/31] coco/tdx-host: Introduce a "tdx_host" device
From: Ackerley Tng @ 2026-07-04 14:26 UTC (permalink / raw)
  To: Xu Yilun, linux-coco, linux-pci, dan.j.williams, x86
  Cc: chao.gao, dave.jiang, baolu.lu, yilun.xu, zhenzhong.duan, kvm,
	rick.p.edgecombe, dave.hansen, kas, xiaoyao.li, vishal.l.verma,
	linux-kernel
In-Reply-To: <20260327160132.2946114-16-yilun.xu@linux.intel.com>

Xu Yilun <yilun.xu@linux.intel.com> writes:

> From: Chao Gao <chao.gao@intel.com>
>
> TDX depends on a platform firmware module that is invoked via instructions
> similar to vmenter (i.e. enter into a new privileged "root-mode" context to
> manage private memory and private device mechanisms). It is a software
> construct that depends on the CPU vmxon state to enable invocation of
> TDX module ABIs. Unlike other Trusted Execution Environment (TEE) platform
> implementations that employ a firmware module running on a PCI device with
> an MMIO mailbox for communication, TDX has no hardware device to point to
> as the TEE Secure Manager (TSM).
>

I'm trying to get up to speed on Confidential IO. Thank you for this
summary!

s/TEE Secure Manager (TSM)/TEE Security Manager/

> Create a virtual device not only to align with other implementations but
> also to make it easier to
>
>  - expose metadata (e.g., TDX module version, seamldr version etc) to
>    the userspace as device attributes
>
>  - implement firmware uploader APIs which are tied to a device. This is
>    needed to support TDX module runtime updates
>
>  - enable TDX Connect which will share a common infrastructure with other
>    platform implementations. In the TDX Connect context, every
>    architecture has a TSM, represented by a PCIe or virtual device. The
>    new "tdx_host" device will serve the TSM role.
>
> A faux device is used for TDX because the TDX module is singular within
> the system and lacks associated platform resources. Using a faux device
> eliminates the need to create a stub bus.
>
> The call to tdx_get_sysinfo() ensures that the TDX module is ready to
> provide services.
>
> Note that AMD has a PCI device for the PSP for SEV and ARM CCA will
> likely have a faux device [1].
>
> Co-developed-by: Xu Yilun <yilun.xu@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
> Reviewed-by: Kai Huang <kai.huang@intel.com>
> Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> Link: https://lore.kernel.org/all/2025073035-bulginess-rematch-b92e@gregkh/ # [1]
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
>
> [...snip...]
>

^ permalink raw reply

* Re: [PATCH v2 16/17] KVM: TDX: Add in-kernel Quote generation
From: Peter Fang @ 2026-07-04  5:43 UTC (permalink / raw)
  To: Edgecombe, Rick P
  Cc: seanjc@google.com, djbw@kernel.org, kvm@vger.kernel.org,
	linux-coco@lists.linux.dev, Li, Xiaoyao, Hansen, Dave,
	dave.hansen@linux.intel.com, baolu.lu@linux.intel.com,
	Hunter, Adrian, kas@kernel.org, tony.lindgren@linux.intel.com,
	Xu, Yilun, linux-kernel@vger.kernel.org, Mehta, Sohil,
	Duan, Zhenzhong, Maloor, Kishen, yilun.xu@linux.intel.com,
	x86@kernel.org
In-Reply-To: <9df36c49e6be69dd9eece71f70a404a84b1563ab.camel@intel.com>

On Wed, Jul 01, 2026 at 11:45:53AM -0700, Edgecombe, Rick P wrote:
> On Wed, 2026-07-01 at 10:25 -0700, Sean Christopherson wrote:
> > > > That is a good question. The answer is partly historical reasons, but I
> > > > think the pros/cons don’t really move the needle too much.
> > > > 
> > > > The main benefit of doing it with the host in the loop is that the guest
> > > > side TDVMCALL quoting interface can stay the same. There is also a wrinkle
> > > > in that there is a limited HW resource involved in the quoting,
> > 
> > What is this magical resource?
> 
> It's a HW crypto thing. I'll let Peter explain more.

It's called S3M (Secured Startup Services). There was once a public
document about it, but it was removed for some reason. It basically
provides a number of boot and security services through dedicated HW.
Attestation is one of them. This is part of our plan to develop new
attestation schemes without using SGX.

There was a lot of internal debate about how this HW should be managed.
We decided to let the TDX module handle it and sort of hide it from the
host kernel and only expose it as SEAMCALLs. It however impacted how the
quoting SEAMCALLs look, so apologies for not calling out these factors.

> 
> > 
> > > > so we want to do these operations one at a time. Having a mutex on the
> > > > host is the KISS way of accomplishing some level of fairness for DOS
> > > > prevention.
> > 
> > At the risk of unintentionally causing effecitvely DoS by introducing a
> > system-wide lock.
> 
> The DoS that people are interested in trying to prevent is a TD getting starved
> of quotes forever. So the lock is supposed to give some eventual fairness.
> Eventually the guest gets its quote even if it has to wait.
> 
> If instead contention throws a BUSY code to the guest, the guest spins trying.
> That wouldn't have the guarantees.
> 
> So I think the two options are: have some lock in the TDX module, or have a lock
> of some sort in the host. I think actually a waiting lock in the TDX module is
> possible. But I think there are tradeoffs besides where the extra code is. If
> it's in Linux we get a lot more control, lockdep, etc.
> 
> BTW the "historical reasons" part I mentioned involves a past effort to create
> configurable host controlled policies around managing these resources. So part
> of the simplification is doing a simple eventually fair global lock instead of
> something more complicated.

Yeah my bad for not including a lot more background. There were several
design aspects that influenced this implementation. I'll do a better job
of explaining them in the next version, but basically:

  1. Performance requirements: For this first implementation, the
     priority is to not starve a legit guest from getting a quote, even
     when there's a malicious guest DoSing the host. A mutex is sort of
     a "fair enough queue" for this problem. A guest's authenticity can
     only be verified after the quote is generated.

  2. HW limitation: The TDX module implements different attestation
     schemes, some of which use the S3M during TDH.QUOTE.GET. This
     limits the amount of concurrency this SEAMCALL supports and is also
     part of why we chose a mutex for TDH.QUOTE.GET. (The shared buffer
     in core TDX doesn't seem great, so I will improve it.)

  3. Host policies: Preserving the GetQuote GHCI also allows the host to
     participate in the quoting process, e.g. add host-controlled
     policies or fetch host-side quote information. It's a bit similar
     to SEV-SNP's KVM_EXIT_SNP_REQ_CERTS [1]. This series does not
     implement host policies though.


Ultimately, there are ways to push more of this into the module, and the
current design is kind of a tradeoff between the complexities of the
work needed in the guest, the host, and the TDX module. Like Rick said,
there were past versions that required way more work in the host and the
guest, such as managing the S3M, new GHCI calls, etc.

[1] Commit fa9893fadbc2 ("KVM: Introduce KVM_EXIT_SNP_REQ_CERTS for SNP
certificate-fetching"

> 

^ permalink raw reply

* Re: [PATCH v14 29/44] arm64: RMI: Runtime faulting of memory
From: Gavin Shan @ 2026-07-03 23:36 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Suzuki K Poulose, Steven Price, kvm, kvmarm, Catalin Marinas,
	Marc Zyngier, Will Deacon, James Morse, Oliver Upton, Zenghui Yu,
	linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
	Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni,
	Shanker Donthineni, Alper Gun, Aneesh Kumar K . V, Emi Kisanuki,
	Vishal Annapurve, WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <akUc/ScJN0GlM+9G@lpieralisi>

On 7/1/26 11:58 PM, Lorenzo Pieralisi wrote:
> On Sun, Jun 28, 2026 at 08:33:01PM +1000, Gavin Shan wrote:
>> On 6/27/26 2:44 AM, Lorenzo Pieralisi wrote:
>>> On Fri, Jun 26, 2026 at 09:43:03PM +1000, Gavin Shan wrote:
>>>> On 6/26/26 6:47 PM, Suzuki K Poulose wrote:
>>>>> On 26/06/2026 08:43, Gavin Shan wrote:
>>>>>> On 6/26/26 1:58 AM, Suzuki K Poulose wrote:
>>>>>>> On 25/06/2026 14:53, Gavin Shan wrote:
>>>>>>>> On 6/6/26 12:35 AM, Lorenzo Pieralisi wrote:
>>>>>>>>> On Fri, Jun 05, 2026 at 06:11:11PM +1000, Gavin Shan wrote:
>>>>>>>>>> On 6/5/26 5:28 PM, Lorenzo Pieralisi wrote:
>>>>>>>>>>> On Fri, Jun 05, 2026 at 04:23:15PM +1000, Gavin Shan wrote:
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>>>>>
>>>>>>>>>> I tried to rebase Jean's latest QEMU series [1] to upstream QEMU, and found
>>>>>>>>>> that memory slots backed by THP are broken. With THP disabled on the host and
>>>>>>>>>> other fixes (mentioned in my prevous replies) applied on the top of this (v14)
>>>>>>>>>> series, I'm able to boot a realm guest with rebased QEMU series [2], plus more
>>>>>>>>>> fxies on the top.
>>>>>>>>>>
>>>>>>>>>> [1] https://git.codelinaro.org/linaro/dcap/qemu.git  (branch: cca/ latest)
>>>>>>>>>> [2] https://git.qemu.org/git/qemu.git                (branch: cca/ gavin)
>>>>>>>>>>
>>>>>>>>>> Lorenzo, You may be saying there is someone making QEMU to support ARM/CCA?
>>>>>>>>>
>>>>>>>>> Mathieu and I are working on that yes and with Steven/Suzuki to fix the THP
>>>>>>>>> issues you pointed out above.
>>>>>>>>>
>>>>>>>>>> If so, I'm not sure if there is a QEMU repository for me to try?
>>>>>>>>>
>>>>>>>>> We should be able to submit patches by end of June - we shall let you know
>>>>>>>>> whether we can make something available earlier.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Not sure if there are other known issues in this series. It seems the stage2
>>>>>>>> page fault handling on the shared space isn't working well. In my test, the
>>>>>>>> vring (struct vring_desc) of virtio-net-pci is updated by the guest, and the
>>>>>>>> data isn't seen by QEMU, I'm suspecting if the host-page-frame-number is properly
>>>>>>>> resolved in the s2 page fault handler for shared (unprotected) space.
>>>>>>>>
>>>>>>>> - I rebased Jean's latest qemu branch to the upstream qemu;
>>>>>>>>
>>>>>>>> - On the host, which is emulated by qemu/tcg, the THP (transparent huge page) is
>>>>>>>>      disabled.
>>>>>>>>
>>>>>>>> - On the guest, I can see the virtio vring (struct vring_desc) is updated. The
>>>>>>>>      S1 page-table entry looks correct because the corresponding physical address
>>>>>>>>      0x10046880000 is a sane shared (unprotected) space address.
>>>>>>>>
>>>>>>>>      [   52.094143] software IO TLB: Memory encryption is active and system is using DMA bounce buffers
>>>>>>>>      [   52.289746] virtqueue_add_desc_split: desc[0]@0xffff000006880000, [00000100b983f000  00000640  0002  0001]
>>>>>>>>      [   52.432150] PTE 0x00e8010046880707 at address 0xffff000006880000
>>>>>>>>
>>>>>>>> - On the host, the s2 page-table-entry is unmapped due to attribute transition (private -> shared).
>>>>>>>>      A subsequent S2 page fault is raised against the adress and the s2 page-table-entry is built.
>>>>>>>>
>>>>>>>>      [  109.259077] ====> realm_unmap_shared_range: tracked_unprot_addr=0x10046880000
>>>>>>>>      [  109.260249] realm_unmap_shared_range: unmapped shared range at 0x10046880000
>>>>>>>>      [  109.317786] realm_unmap_shared_range: unmapped shared range at 0x10046880000
>>>>>>>>      [  109.629939] ====> kvm_handle_guest_abort: fault_ipa=0x10046880000, esr=0x92000007
>>>>>>>>      [  109.630245] realm_map_non_secure: ipa=0x10046880000, pfn=0xb8b59, size=0x1000, prot=0xf
>>>>>>>>      [  109.630331] realm_map_non_secure: ipa=0x10046880000, ipa_top=0x10046881000, flags=0x1e0001, range_desc=0xb8b59004
>>>>>>>
>>>>>>> Are you able to correlate the order of the transitions and the Guest
>>>>>>> access with RMM log ? We haven't seen this from our end. We are aware
>>>>>>> of permission fault issues with Unprotected IPA when backing the memslot
>>>>>>> with MAP_PRIVATE areas. But this looks different.
>>>>>>>
>>>>>>> Lorenzo, have you run into this ?
>>>>>>>
>>>>>>
>>>>>> It's hard to correlate the order since the logs are collected from two separate
>>>>>> consoles. For the write permission, I add code to the host where the permission
>>>>>> is always added for all s2 page faults in the shared space. Otherwise, qemu can
>>>>>> be killed by -EFAULT or similar error.
>>>>>
>>>>> This is the problem. We can't add WRITE permission by default. I believe
>>>>> you may have MAP_PRIVATE mapping and it has to be mapped as READ only
>>>>> and on a permission fault, we replace it with a writable page. By
>>>>> overriding the WRITE permission, you let the guest write to a page
>>>>> that may not be seen by the VMM.
>>>>>
>>>>> We identified this as a bug in the KVM driver in this series (reported
>>>>> by Lorenzo) and there is a corresponding tf-RMM change that is required
>>>>> to get this working. So, please could you wait until the next series
>>>>> when this will be addressed ? Or you could switch to using MAP_SHARED
>>>>> for the "shared" memory in the memslot.
>>>>>
>>>>
>>>> Exactly. the syntax for MAP_PRIVATE is broken if the write permission is
>>>> enforced for a read fault in the shared space. In my case, the host page can
>>>> be the zero page and eventually multiple s2 page-table entries (for multiple
>>>> unprotected or shared pages) point to the zero page. It's why clearing the
>>>> 3rd queue (Ctrl queue) also clears the first queue (Rx queue) in my case.
>>>>
>>>> Yes, this issue can be avoid by using a shared memory backend in qemu, something
>>>> like below. With this, I'm able to see virtio-net-pci starts to work...
>>>>
>>>>       -object memory-backend-ram,id=mem0,size=2G,share=yes
>>>
>>> Yes, as Suzuki said that's what we have been fixing. QEmu patches
>>> will be on the mailing lists very shortly - the KVM/tf-RMM fixes
>>> to make MAP_PRIVATE work will be included in the next posting.
>>>
>>> Feel free to drop your QEmu command line so that I can give it
>>> a shot and check whether the fixes solve the problem you hit
>>> (I think so because that's precisely the kind of issue I got
>>> into when I started debugging THP/MAP_PRIVATE but it is better
>>> to check).
>>>
>>
>> The virtio-net-pci doesn't work with the following command lines. The guest
>> kernel image is built from upstream kernel (v7.1.rc7).
>>
>>      qemu-system-aarch64 -enable-kvm -object rme-guest,id=rme0,             \
>>      -machine virt,gic-version=3,confidential-guest-support=rme0            \
>>      -cpu host,pmu=off                                                      \
>>      -smp maxcpus=2,cpus=2,sockets=1,clusters=1,cores=1,threads=2           \
>>      -m 2G -object memory-backend-ram,id=mem0,size=2G                       \
>>      -numa node,nodeid=0,cpus=0-1,memdev=mem0                               \
>>      -serial mon:stdio -monitor none -nographic -nodefaults                 \
>>      -kernel /mnt/linux/arch/arm64/boot/Image                               \
>>      -initrd /mnt/buildroot/output/images/rootfs.cpio.xz                    \
>>      -append earlycon=pl011,mmio,0x10009000000                              \
>>      -device pcie-root-port,bus=pcie.0,chassis=1,id=pcie.1                  \
>>      -device pcie-root-port,bus=pcie.0,chassis=2,id=pcie.2                  \
>>      -device pcie-root-port,bus=pcie.0,chassis=3,id=pcie.3                  \
>>      -device pcie-root-port,bus=pcie.0,chassis=4,id=pcie.4                  \
>>      -netdev tap,id=tap1,vhost=on,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown  \
>>      -device virtio-net-pci,bus=pcie.2,netdev=tap1,mac=b8:3f:d2:1d:3e:c0
> 
> Tested with private RAM backend, THP left enabled (and additional KVM and
> RMM patches that will be included in v15) looks like the net interface
> comes up cleanly.
> 
> Probably it is best to sync up on the upcoming v15 posting that I am
> testing in the background to make sure you don't spend more time chasing
> it.
> 

Thanks for your confirmation. For every respin of the arm/cca host series,
I try to give it a test to make sure nothing gets broken in my coverage.
For this specific revision (v14), more time was actually spent to rebase
Jean's qemu (branch: cca/latest) to upstream so that I can do the tests.

Lets wait for a while to see what we will have in v15 and the qemu series
you're going to post, and I will give it another try.

One unrelated question is 64KB granule size, it's not supported by upstream
TF-RMM "main" or "topics/rmm-v2.0-poc_2" branch, not sure about the plan
when this will be supported in TF-RMM. Since 64KB isn't supported by TF-RMM,
I was unable to test 64KB page size.

Upstream EDK2 doesn't support realm guest yet even some of related commits
have been merged. So I still use linaro's one at "https://git.codelinaro.org
/linaro/dcap/edk2  (branch: cca/latest).
  
Thanks,
Gavin


> Thanks,
> Lorenzo
> 
>>
>> The virtio-net-pci starts to work with the shareable memory-backend.
>>
>>      -object memory-backend-ram,id=mem0,size=2G,share=yes
>>
>> Note that THP is disabled on my host.
>>
>>      root@host:~# cat /sys/kernel/mm/transparent_hugepage/enabled
>>      always madvise [never]
> 


^ permalink raw reply

* Re: [PATCH v6 03/20] dma-direct: use DMA_ATTR_CC_SHARED in alloc/free paths
From: Jason Gunthorpe @ 2026-07-03 11:46 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Aneesh Kumar K.V (Arm), iommu, linux-arm-kernel, linux-kernel,
	linux-coco, Robin Murphy, Marek Szyprowski, Will Deacon,
	Marc Zyngier, Steven Price, Suzuki K Poulose, Catalin Marinas,
	Jiri Pirko, Mostafa Saleh, Petr Tesarik, Dan Williams, Xu Yilun,
	linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
	Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
	Michael Kelley, Cheloha, Scott
In-Reply-To: <58bb92a9-c5ea-47a3-b19a-934573f15305@amd.com>

On Fri, Jul 03, 2026 at 08:15:49PM +1000, Alexey Kardashevskiy wrote:
> On 3/7/26 00:47, Jason Gunthorpe wrote:
> > On Thu, Jul 02, 2026 at 10:25:16AM +1000, Alexey Kardashevskiy wrote:
> > 
> > > > > not externally available so I'll have to trick the DMA layer into
> > > > > using SWIOTLB (which is still all shared, right?) as I specifically
> > > > > want to skip page conversions. Setting low DMA mask won't guarantee
> > > > > that the DMA layer won't allocate a page outside of SWIOTLB and
> > > > > convert it. Manually do
> > > > 
> > > > Why so particular?
> > > 
> > > aahhh I missed "pre-". I need a way to get pre-shared pages for my
> > > sev-guest activities.
> > 
> > It sounds to me like you don't, what you are worried about is
> > optimizing the dma_alloc_coherent flow to avoid fragmentation and all
> > CC architetures require this optimization.
> 
> It is not about fragmentation, it is about unwanted page state
> changes when few pages are shared for a very short time.

That's really basically the same thing. The responsiblity for
optimizing the shared/private conversion lies with the DMA API not the
callers.

> When I needed these pages, I could: get a private page, convert,
> (possibly trigger RMP and IO PDE smashing if that 4K is backed with
> 2M), do the VM<->HV communication, convert back to private (and
> possibly unsmash the page).

IMHO it would be better to use something like dma_pool to amortize the
coherent allocations instead of trying to hack with swiotlb.

Jason

^ permalink raw reply

* Re: [PATCH v6 03/20] dma-direct: use DMA_ATTR_CC_SHARED in alloc/free paths
From: Alexey Kardashevskiy @ 2026-07-03 10:15 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Aneesh Kumar K.V (Arm), iommu, linux-arm-kernel, linux-kernel,
	linux-coco, Robin Murphy, Marek Szyprowski, Will Deacon,
	Marc Zyngier, Steven Price, Suzuki K Poulose, Catalin Marinas,
	Jiri Pirko, Mostafa Saleh, Petr Tesarik, Dan Williams, Xu Yilun,
	linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
	Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
	Michael Kelley, Cheloha, Scott
In-Reply-To: <20260702144729.GP7525@ziepe.ca>



On 3/7/26 00:47, Jason Gunthorpe wrote:
> On Thu, Jul 02, 2026 at 10:25:16AM +1000, Alexey Kardashevskiy wrote:
> 
>>>> not externally available so I'll have to trick the DMA layer into
>>>> using SWIOTLB (which is still all shared, right?) as I specifically
>>>> want to skip page conversions. Setting low DMA mask won't guarantee
>>>> that the DMA layer won't allocate a page outside of SWIOTLB and
>>>> convert it. Manually do
>>>
>>> Why so particular?
>>
>> aahhh I missed "pre-". I need a way to get pre-shared pages for my
>> sev-guest activities.
> 
> It sounds to me like you don't, what you are worried about is
> optimizing the dma_alloc_coherent flow to avoid fragmentation and all
> CC architetures require this optimization.

It is not about fragmentation, it is about unwanted page state changes when few pages are shared for a very short time.

When I needed these pages, I could: get a private page, convert, (possibly trigger RMP and IO PDE smashing if that 4K is backed with 2M), do the VM<->HV communication, convert back to private (and possibly unsmash the page).

Or ask swiotlb for these already shared pages, and skip all of the above.

How would defragmentation help here? It is two or zero conversions.

> So I'm strongly against you trying to open code something in your
> driver. Use the DMA API as-is. Propose general DMA API improvements to
> reduce the fragmentation problem. It is a real problem.
> 
> Jason

-- 
Alexey


^ permalink raw reply

* Re: [PATCH v6 02/11] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
From: Chao Gao @ 2026-07-03  8:26 UTC (permalink / raw)
  To: Rick Edgecombe
  Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
	linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
	vannapurve, x86, yan.y.zhao, kai.huang, Kirill A. Shutemov,
	Binbin Wu
In-Reply-To: <20260526023515.288829-3-rick.p.edgecombe@intel.com>

On Mon, May 25, 2026 at 07:35:06PM -0700, Rick Edgecombe wrote:
>From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
>The TDX Physical Address Metadata Table (PAMT) holds data about the
>physical memory used by TDX, and must be allocated by the kernel during
>TDX module initialization.
>
>The exact size of the required PAMT memory is determined by the TDX module
>and may vary between TDX module versions. Currently it is approximately
>0.4% of the system memory. This is a significant commitment, especially if
>it is not known upfront whether the machine will run any TDX guests.
>
>Each memory region that the TDX module might use needs three separate PAMT
>allocations. One for each supported page size (1GB, 2MB, 4KB). The
>TDX module supports a new feature designed to reduce PAMT overhead called
>Dynamic PAMT. At a high level, Dynamic PAMT still has the 1GB and 2MB
>levels allocated on TDX module initialization, but the 4KB level is
>allocated dynamically during runtime.
>
>However, in the details, Dynamic PAMT still needs some smaller per 4KB
>page scoped data (currently it is 1 bit per page). The TDX module exposes
>the number of bits as a separate piece of metadata than the 4KB static
>allocation for regular PAMT. Although the size is enumerated differently,
>it is handed to the TDX module in the same way the 4KB page size PAMT
>allocation is for regular, non-dynamic PAMT.
>
>Begin to implement Dynamic PAMT in the kernel by reading the bits-per-page
>needed for Dynamic PAMT. Calculate the size needed for the bitmap,
>and use it instead of the 4KB size determined for normal PAMT, in the case
>of Dynamic PAMT.
>
>Unlike the existing metadata reading code, this code is not generated by a
>script. So adjust the comment to be more generic. Also, start to adopt a
>more normal kernel code style without the tenary statements and if
>conditionals assignments that the auto generated code has.
>
>Assisted-by: Sashiko:claude-opus-4-6
>Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
>Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

Reviewed-by: Chao Gao <chao.gao@intel.com>

^ permalink raw reply

* Re: [RFC PATCH 15/15] x86/virt/tdx: Enable TDX Quoting extension
From: Nikolay Borisov @ 2026-07-03  7:57 UTC (permalink / raw)
  To: Xu Yilun, kas, djbw, rick.p.edgecombe, x86, peter.fang
  Cc: linux-coco, linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
	zhenzhong.duan, xiaoyao.li
In-Reply-To: <20260522034128.3144354-16-yilun.xu@linux.intel.com>



On 5/22/26 06:41, Xu Yilun wrote:
> From: Peter Fang <peter.fang@intel.com>
> 
> Enable the TDX Quoting feature via TDH.SYS.CONFIG when supported by the
> TDX module.
> 
> The TDX Quoting extension generates TDX attestation Quotes via a
> SEAMCALL, without using a discrete Quoting engine.
"Quoting engine" is a new term, introduced here, better say "without 
involvement from the VMM"

<snip>

^ permalink raw reply

* Re: [PATCH v6 01/11] x86/virt/tdx: Simplify tdmr_get_pamt_sz()
From: Chao Gao @ 2026-07-03  5:48 UTC (permalink / raw)
  To: Rick Edgecombe
  Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
	linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
	vannapurve, x86, yan.y.zhao, kai.huang, Binbin Wu
In-Reply-To: <20260526023515.288829-2-rick.p.edgecombe@intel.com>

On Mon, May 25, 2026 at 07:35:05PM -0700, Rick Edgecombe wrote:
>Since the loop that iterates over it is gone, further simplify the code by
>dropping the array of intermediate size and base storage. Just store the
>values to their final locations.

>Accept the small complication of having
>to clear tdmr->pamt_4k_base in the error path, so that tdmr_do_pamt_func()
>will not try to operate on the TDMR struct when attempting to free it.

The clearing of tdmr->pamt_4k_base was dropped, so this section is a bit
stale. Apart from this nit,

Reviewed-by: Chao Gao <chao.gao@intel.com>

^ permalink raw reply

* Re: [PATCH v6 11/11] Documentation/x86: Add documentation for TDX's Dynamic PAMT
From: Binbin Wu @ 2026-07-03  4:54 UTC (permalink / raw)
  To: Rick Edgecombe
  Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
	linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
	vannapurve, x86, chao.gao, yan.y.zhao, kai.huang,
	Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-12-rick.p.edgecombe@intel.com>

On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> 
> Expand TDX documentation to include information on the Dynamic PAMT
> feature.
> 
> The new section explains PAMT support in the TDX module and how Dynamic
> PAMT affects the kernel memory use.
> 
> Assisted-by: Sashiko:claude-opus-4-6 GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>

One nit below.

> ---
> v6:
>  - Add missing word (Binbin)
>  - Use "::" instead of ":"
>  - Make format of dmesg example accurate
> 
> v3:
>  - Trim down docs to be about things that user cares about, instead
>    of development history and other details like this.
> ---
>  Documentation/arch/x86/tdx.rst | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/arch/x86/tdx.rst b/Documentation/arch/x86/tdx.rst
> index ff6b110291bc6..ce026a88b6f78 100644
> --- a/Documentation/arch/x86/tdx.rst
> +++ b/Documentation/arch/x86/tdx.rst
> @@ -73,6 +73,28 @@ initialize::
>  
>    [..] virt/tdx: TDX-Module initialization failed ...
>  
> +Dynamic PAMT
> +------------
> +
> +PAMT

Nit:
It seems that there is no spelling out of the acronym in the source code or the documentation.
Maybe it's a good place to give the full version. E.g.

Physical Address Metadata Table (PAMT) is ...

> is memory that the TDX module needs to keep data about each page
> +(think like struct page). It needs to be handed to the TDX module for its
> +exclusive use. For normal PAMT, this is installed when the TDX module
> +is first loaded and comes to about 0.4% of system memory.
> +
> +Dynamic PAMT is a TDX feature that allows VMM to allocate part of the
> +PAMT as needed (the parts for tracking 4KB size pages). The other page
> +sizes (1GB and 2MB) are still allocated statically at the time of
> +TDX module initialization. This reduces the amount of memory that TDX
> +uses while TDs are not in use.
> +
> +When Dynamic PAMT is in use, dmesg shows it like::
> +
> +  [..] virt/tdx: Enable Dynamic PAMT
> +  [..] virt/tdx: 10092 KB allocated for PAMT
> +  [..] virt/tdx: TDX-Module initialized
> +
> +Dynamic PAMT is enabled automatically if supported.
> +
>  TDX Interaction to Other Kernel Components
>  ------------------------------------------
>  


^ permalink raw reply

* Re: [PATCH v6 10/11] x86/virt/tdx: Enable Dynamic PAMT
From: Binbin Wu @ 2026-07-03  4:35 UTC (permalink / raw)
  To: Rick Edgecombe
  Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
	linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
	vannapurve, x86, chao.gao, yan.y.zhao, kai.huang,
	Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-11-rick.p.edgecombe@intel.com>

On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> 
> The Physical Address Metadata Table (PAMT) holds TDX metadata for
> physical memory and must be allocated by the kernel during TDX module
> initialization. Dynamic PAMT is a TDX module feature that can reduce this
> memory use by allocating part of the PAMT dynamically.
> 
> All pieces are in place to Enable Dynamic PAMT if it is supported.
> Determine if the TDX module supports it by checking the 'features0' bit
> exposed by the TDX module.
> 
> The TDX module also exposes information about whether the *system* (and
> not the module) supports Dynamic PAMT.
> 
> The TDX module documentation describes how PAMT works internally. To allow
> the last level to be dynamically allocated, it uses a 3 level tree
> structure, not unlike page tables. Like page tables, it has a maximum
> address space that it can cover. This address space can be covered in 48
> bits. If the host physical address space is higher than this, than the
                                                                  ^
                                                                then


> TDX module can't guarantee the tree will be able to cover the TDX memory.
> 
> The TDX module exposes this system support via metadata stating the
> minimum number of HKIDs that need to be available in order for Dynamic
> PAMT to be usable. The reasoning appears to be that more HKIDs can shrink
> the "real" addressable physical address bits enough to make the 48 bit
> Dynamic PAMT limit workable on high physical address width HW. However,
> the docs also clearly explain the 48 bit limit and how this fits into the
> Dymamic PAMT tree constraints.
    ^
  Dynamic


> 
> The handy x86_phys_bits value is already read and adjusted for keyid bits.
> So just compare that against 48 instead of reading more metadata and
> burdening the code with the more tenuous connection to minimum HKID bits.
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
It looks like whether to check 48 bit physical address width limitation is
still open.

The rest LGTM.




^ permalink raw reply

* Re: [PATCH v6 09/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory
From: Binbin Wu @ 2026-07-03  3:15 UTC (permalink / raw)
  To: Rick Edgecombe
  Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
	linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
	vannapurve, x86, chao.gao, yan.y.zhao, kai.huang,
	Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-10-rick.p.edgecombe@intel.com>

On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> 
> Add Dynamic PAMT support to KVM's S-EPT MMU by "getting" a PAMT page when
> adding guest memory (PAGE.ADD or PAGE.AUG), and "putting" the page when
> removing guest memory (PAGE.REMOVE).
> 
> To access the per-vCPU PAMT caches without plumbing @vcpu throughout the
> TDP MMU, begrudgingly use kvm_get_running_vcpu() to get the vCPU, and bug
> the VM if KVM attempts to set an S-EPT leaf without an active vCPU.  KVM
> only supports creating _new_ mappings in page (pre)fault paths, all of
> which require an active vCPU.
> 
> The PAMT memory holds metadata for TDX-protected memory. With Dynamic
> PAMT, PAMT_4K is allocated on demand. The kernel supplies the TDX module
> with a few pages that cover 2M of host physical memory.
> 
> Releases are balanced via tdx_pamt_put(): every control-page free goes
> through tdx_free_control_page(), and guest data pages are put directly on
> the successful tdh_mem_page_remove() path and in the
> tdx_mem_page_add/aug() error path.
> 
> Assisted-by: Sashiko:claude-opus-4-6 GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>

One nit below.

[...]

> @@ -1669,16 +1683,29 @@ static struct page *tdx_spte_to_sept_pt(struct kvm *kvm, gfn_t gfn,
>  static int tdx_sept_map_nonleaf_spte(struct kvm *kvm, gfn_t gfn,
>  				     enum pg_level level, u64 new_spte)
>  {
> +	struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
> +	struct vcpu_tdx *tdx = to_tdx(vcpu);

Nit:
Is it better to move this after checking vcpu is not NULL?
Although tdx is not dereferenced in between, if vcpu is NULL,
it means container_of() does arithmetic to a NULL pointer.


>  	gpa_t gpa = gfn_to_gpa(gfn);
>  	u64 err, entry, level_state;
>  	struct page *sept_pt;
> +	int ret;
> +
> +	if (KVM_BUG_ON(!vcpu, kvm))
> +		return -EIO;
>  
>  	sept_pt = tdx_spte_to_sept_pt(kvm, gfn, new_spte, level);
>  	if (!sept_pt)
>  		return -EIO;
>  
> +	ret = tdx_pamt_get(page_to_pfn(sept_pt), &tdx->pamt_cache);
> +	if (ret)
> +		return ret;
> +
>  	err = tdh_mem_sept_add(&to_kvm_tdx(kvm)->td, gpa, level, sept_pt,
>  			       &entry, &level_state);
> +	if (err)
> +		tdx_pamt_put(page_to_pfn(sept_pt));
> +
>  	if (unlikely(tdx_operand_busy(err)))
>  		return -EBUSY;
>  
> @@ -1691,8 +1718,14 @@ static int tdx_sept_map_nonleaf_spte(struct kvm *kvm, gfn_t gfn,
>  static int tdx_sept_map_leaf_spte(struct kvm *kvm, gfn_t gfn, enum pg_level level,
>  				  u64 new_spte)
>  {
> +	struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
>  	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);

Ditto 

>  	kvm_pfn_t pfn = spte_to_pfn(new_spte);
> +	struct vcpu_tdx *tdx = to_tdx(vcpu);
> +	int ret;
> +
> +	if (KVM_BUG_ON(!vcpu, kvm))
> +		return -EIO;
>  
[...]

^ permalink raw reply

* Re: [PATCH] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Xiaoyao Li @ 2026-07-03  0:00 UTC (permalink / raw)
  To: Xu Yilun, x86, linux-kernel
  Cc: kas, rick.p.edgecombe, dave.hansen, dave.hansen, yilun.xu,
	chao.gao, djbw, linux-coco, peter.fang
In-Reply-To: <20260702144614.59464-1-yilun.xu@linux.intel.com>

On 7/2/2026 10:46 PM, Xu Yilun wrote:
> +/*
> + * SEAMCALL leaf:
> + *
> + * Bit 15:0	Leaf number
> + * Bit 23:16	Version number
> + */
> +#define SEAMCALL_VERSION_MASK		GENMASK_U64(23, 16)
> +
> +static __always_inline u64 __seamcall_encode_fn(sc_func_t func, u64 fn,
> +						struct tdx_module_args *args)
> +{
> +	FIELD_MODIFY(SEAMCALL_VERSION_MASK, &fn, args->version);
> +
> +	return func(fn, args);
> +}
> +
>   static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
>   						  struct tdx_module_args *args)
>   {
> @@ -39,7 +56,7 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
>   	 */
>   	this_cpu_write(cache_state_incoherent, true);
>   
> -	return func(fn, args);
> +	return __seamcall_encode_fn(func, fn, args);
>   }

Can we drop the new wrapper? I don't see why we need it. The wrapper 
makes the code harder to read.



^ permalink raw reply

* RE: [PATCH v5 47/51] x86/paravirt: Don't use a PV sched_clock in CoCo guests with trusted TSC
From: Michael Kelley @ 2026-07-02 17:48 UTC (permalink / raw)
  To: Sean Christopherson, Jonathan Corbet, Paolo Bonzini,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	x86@kernel.org, Kiryl Shutsemau, Rick Edgecombe, K. Y. Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
	Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
	Juergen Gross, Daniel Lezcano, John Stultz
  Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
	Broadcom internal kernel review list, Boris Ostrovsky,
	Stephen Boyd, linux-doc@vger.kernel.org, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-coco@lists.linux.dev,
	linux-hyperv@vger.kernel.org, virtualization@lists.linux.dev,
	xen-devel@lists.xenproject.org, Tom Lendacky, Nikunj A Dadhania,
	David Woodhouse, David Woodhouse, Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-48-seanjc@google.com>

From: Sean Christopherson <seanjc@google.com> Sent: Wednesday, July 1, 2026 12:32 PM
> 
> Silently ignore attempts to switch to a paravirt sched_clock when running
> as a CoCo guest with trusted TSC.  In hand-wavy theory, a misbehaving
> hypervisor could attack the guest by manipulating the PV clock to affect
> guest scheduling in some weird and/or predictable way.  More importantly,
> reading TSC on such platforms is faster than any PV clock, and sched_clock
> is all about speed.
> 
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kernel/tsc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 012321fed5e5..a146fc7b5e74 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -283,6 +283,15 @@ bool using_native_sched_clock(void)
>  int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
>  				      void (*save)(void), void (*restore)(void))
>  {
> +	/*
> +	 * Don't replace TSC with a PV clock when running as a CoCo guest and
> +	 * the TSC is secure/trusted; PV clocks are emulated by the hypervisor,
> +	 * which isn't in the guest's TCB.
> +	 */
> +	if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC) ||
> +	    boot_cpu_has(X86_FEATURE_TDX_GUEST))
> +		return -EPERM;

Do a pr_warn() in the error case? Your commit message says to
do the ignore silently, but I wonder if that's a good idea. At least
for Hyper-V, the error case shouldn't happen.

Michael

> +
>  	if (!stable)
>  		clear_sched_clock_stable();
> 
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
> 


^ permalink raw reply

* RE: [PATCH v5 36/51] x86/paravirt: Pass sched_clock save/restore helpers during registration
From: Michael Kelley @ 2026-07-02 17:48 UTC (permalink / raw)
  To: Sean Christopherson, Jonathan Corbet, Paolo Bonzini,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	x86@kernel.org, Kiryl Shutsemau, Rick Edgecombe, K. Y. Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
	Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
	Juergen Gross, Daniel Lezcano, John Stultz
  Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
	Broadcom internal kernel review list, Boris Ostrovsky,
	Stephen Boyd, linux-doc@vger.kernel.org, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-coco@lists.linux.dev,
	linux-hyperv@vger.kernel.org, virtualization@lists.linux.dev,
	xen-devel@lists.xenproject.org, Tom Lendacky, Nikunj A Dadhania,
	David Woodhouse, David Woodhouse, Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-37-seanjc@google.com>

From: Sean Christopherson <seanjc@google.com> Sent: Wednesday, July 1, 2026 12:32 PM
> 
> Pass in a PV clock's save/restore helpers when configuring sched_clock
> instead of relying on each PV clock to manually set the save/restore hooks.
> In addition to bringing sanity to the code, this will allow gracefully
> "rejecting" a PV sched_clock, e.g. when running as a CoCo guest that has
> access to a "secure" TSC.
> 
> No functional change intended.
> 
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

For the Hyper-V changes,

Reviewed-by: Michael Kelley <mhklinux@outlook.com>

> ---
>  arch/x86/include/asm/timer.h       | 9 ++++++---
>  arch/x86/kernel/cpu/vmware.c       | 8 +++-----
>  arch/x86/kernel/kvmclock.c         | 6 +++---
>  arch/x86/kernel/tsc.c              | 5 ++++-
>  arch/x86/xen/time.c                | 5 ++---
>  drivers/clocksource/hyperv_timer.c | 6 ++----
>  6 files changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/x86/include/asm/timer.h b/arch/x86/include/asm/timer.h
> index fe41d40a9ae6..e97cd1ae03d1 100644
> --- a/arch/x86/include/asm/timer.h
> +++ b/arch/x86/include/asm/timer.h
> @@ -14,11 +14,14 @@ extern int no_timer_check;
>  extern bool using_native_sched_clock(void);
> 
>  #ifdef CONFIG_PARAVIRT
> -void __paravirt_set_sched_clock(u64 (*func)(void), bool stable);
> +void __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
> +				void (*save)(void), void (*restore)(void));
> 
> -static inline void paravirt_set_sched_clock(u64 (*func)(void))
> +static inline void paravirt_set_sched_clock(u64 (*func)(void),
> +					    void (*save)(void),
> +					    void (*restore)(void))
>  {
> -	__paravirt_set_sched_clock(func, true);
> +	__paravirt_set_sched_clock(func, true, save, restore);
>  }
>  #endif
> 
> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> index 5c1ccaf4a25e..232255279a6e 100644
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ b/arch/x86/kernel/cpu/vmware.c
> @@ -347,11 +347,9 @@ static void __init vmware_paravirt_ops_setup(void)
> 
>  	vmware_cyc2ns_setup();
> 
> -	if (vmw_sched_clock) {
> -		paravirt_set_sched_clock(vmware_sched_clock);
> -		x86_platform.save_sched_clock_state = x86_init_noop;
> -		x86_platform.restore_sched_clock_state = x86_init_noop;
> -	}
> +	if (vmw_sched_clock)
> +		paravirt_set_sched_clock(vmware_sched_clock,
> +					 x86_init_noop, x86_init_noop);
> 
>  	if (vmware_is_stealclock_available()) {
>  		has_steal_clock = true;
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index 07e875738c39..5b9955343199 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -158,7 +158,9 @@ static void kvm_restore_sched_clock_state(void)
>  static inline void kvm_sched_clock_init(bool stable)
>  {
>  	kvm_sched_clock_offset = kvm_clock_read();
> -	__paravirt_set_sched_clock(kvm_sched_clock_read, stable);
> +	__paravirt_set_sched_clock(kvm_sched_clock_read, stable,
> +				   kvm_save_sched_clock_state,
> +				   kvm_restore_sched_clock_state);
> 
>  	pr_info("kvm-clock: using sched offset of %llu cycles",
>  		kvm_sched_clock_offset);
> @@ -367,8 +369,6 @@ void __init kvmclock_init(bool prefer_tsc)
>  #ifdef CONFIG_SMP
>  	x86_cpuinit.early_percpu_clock_init = kvm_setup_secondary_clock;
>  #endif
> -	x86_platform.save_sched_clock_state = kvm_save_sched_clock_state;
> -	x86_platform.restore_sched_clock_state = kvm_restore_sched_clock_state;
>  	kvm_get_preset_lpj();
> 
>  	/*
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 7473dcab4775..83353d643150 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -280,12 +280,15 @@ bool using_native_sched_clock(void)
>  	return static_call_query(pv_sched_clock) == native_sched_clock;
>  }
> 
> -void __paravirt_set_sched_clock(u64 (*func)(void), bool stable)
> +void __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
> +				void (*save)(void), void (*restore)(void))
>  {
>  	if (!stable)
>  		clear_sched_clock_stable();
> 
>  	static_call_update(pv_sched_clock, func);
> +	x86_platform.save_sched_clock_state = save;
> +	x86_platform.restore_sched_clock_state = restore;
>  }
>  #else
>  u64 sched_clock_noinstr(void) __attribute__((alias("native_sched_clock")));
> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> index 477441752f40..8cd8bfaf1320 100644
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -566,13 +566,12 @@ static void __init xen_init_time_common(void)
>  {
>  	xen_sched_clock_offset = xen_clocksource_read();
>  	static_call_update(pv_steal_clock, xen_steal_clock);
> -	paravirt_set_sched_clock(xen_sched_clock);
> +
>  	/*
>  	 * Xen has paravirtualized suspend/resume and so doesn't use the common
>  	 * x86 sched_clock save/restore hooks.
>  	 */
> -	x86_platform.save_sched_clock_state = x86_init_noop;
> -	x86_platform.restore_sched_clock_state = x86_init_noop;
> +	paravirt_set_sched_clock(xen_sched_clock, x86_init_noop, x86_init_noop);
> 
>  	x86_init.hyper.get_tsc_khz = xen_tsc_khz;
>  	x86_platform.get_wallclock = xen_get_wallclock;
> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> index 220668207d19..8ee7a9de0f4f 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -570,10 +570,8 @@ static void hv_restore_sched_clock_state(void)
>  static __always_inline void hv_setup_sched_clock(void *sched_clock)
>  {
>  	/* We're on x86/x64 *and* using PV ops */
> -	paravirt_set_sched_clock(sched_clock);
> -
> -	x86_platform.save_sched_clock_state = hv_save_sched_clock_state;
> -	x86_platform.restore_sched_clock_state = hv_restore_sched_clock_state;
> +	paravirt_set_sched_clock(sched_clock, hv_save_sched_clock_state,
> +				 hv_restore_sched_clock_state);
>  }
>  #else /* !CONFIG_GENERIC_SCHED_CLOCK && !CONFIG_PARAVIRT */
>  static __always_inline void hv_setup_sched_clock(void *sched_clock) {}
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
> 


^ permalink raw reply

* RE: [PATCH v5 14/51] x86/tsc: Consolidate forcing of X86_FEATURE_TSC_KNOWN_FREQ for PV code
From: Michael Kelley @ 2026-07-02 17:47 UTC (permalink / raw)
  To: Sean Christopherson, Jonathan Corbet, Paolo Bonzini,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	x86@kernel.org, Kiryl Shutsemau, Rick Edgecombe, K. Y. Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
	Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
	Juergen Gross, Daniel Lezcano, John Stultz
  Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
	Broadcom internal kernel review list, Boris Ostrovsky,
	Stephen Boyd, linux-doc@vger.kernel.org, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-coco@lists.linux.dev,
	linux-hyperv@vger.kernel.org, virtualization@lists.linux.dev,
	xen-devel@lists.xenproject.org, Tom Lendacky, Nikunj A Dadhania,
	David Woodhouse, David Woodhouse, Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-15-seanjc@google.com>

From: Sean Christopherson <seanjc@google.com> Sent: Wednesday, July 1, 2026 12:32 PM
> 
> Now that all paravirt code that explicitly specifies the TSC frequency
> also sets X86_FEATURE_TSC_KNOWN_FREQ, replace all of the one-off code
> and simply set X86_FEATURE_TSC_KNOWN_FREQ if the TSC frequency is known.
> 
> Do NOT force set TSC_KNOWN_FREQ if the "known" TSC frequency was provided
> by the user.  Per commit bd35c77e32e4 ("x86/tsc: Add tsc_early_khz command
> line parameter"), one of the goals of the param is to allow the refined
> calibration work "to do meaningful error checking".
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

For the Hyper-V changes,

Reviewed-by: Michael Kelley <mhklinux@outlook.com>

> ---
>  arch/x86/coco/sev/core.c       |  1 -
>  arch/x86/coco/tdx/tdx.c        |  1 -
>  arch/x86/kernel/cpu/acrn.c     |  1 -
>  arch/x86/kernel/cpu/mshyperv.c |  1 -
>  arch/x86/kernel/cpu/vmware.c   |  2 --
>  arch/x86/kernel/jailhouse.c    |  1 -
>  arch/x86/kernel/kvmclock.c     |  1 -
>  arch/x86/kernel/tsc.c          | 13 ++++++++++---
>  arch/x86/xen/time.c            |  1 -
>  9 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index bc5ae9ef74da..72313b36b6f5 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -2027,7 +2027,6 @@ unsigned int __init snp_secure_tsc_init(void)
> 
>  	secrets = (__force struct snp_secrets_page *)mem;
> 
> -	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
>  	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> 
>  	rdmsrq(MSR_AMD64_GUEST_TSC_FREQ, tsc_freq_mhz);
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index ae2d35f2ef33..94682aca188b 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -1205,7 +1205,6 @@ unsigned int __init tdx_tsc_init(void)
> 
>  	/* TSC is the only reliable clock in TDX guest */
>  	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> -	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
> 
>  	return info.crystal_khz * info.numerator / info.denominator;
>  }
> diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
> index 3818f6ae0629..dc71a6fdd461 100644
> --- a/arch/x86/kernel/cpu/acrn.c
> +++ b/arch/x86/kernel/cpu/acrn.c
> @@ -40,7 +40,6 @@ static void __init acrn_init_platform(void)
>  	if (acrn_tsc_khz_cpuid) {
>  		x86_init.hyper.get_tsc_khz = acrn_get_tsc_khz;
>  		x86_init.hyper.get_cpu_khz = acrn_get_tsc_khz;
> -		setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
>  	}
>  }
> 
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index f9bc1c2d8c93..e03c69a4db33 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -575,7 +575,6 @@ static void __init ms_hyperv_init_platform(void)
>  	    ms_hyperv.misc_features & HV_FEATURE_FREQUENCY_MSRS_AVAILABLE) {
>  		x86_init.hyper.get_tsc_khz = hv_get_tsc_khz;
>  		x86_init.hyper.get_cpu_khz = hv_get_tsc_khz;
> -		setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
>  	}
> 
>  	if (ms_hyperv.priv_high & HV_ISOLATION) {
> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> index 3cb473cae462..0a3bd90576d4 100644
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ b/arch/x86/kernel/cpu/vmware.c
> @@ -390,8 +390,6 @@ static void __init vmware_set_capabilities(void)
>  {
>  	setup_force_cpu_cap(X86_FEATURE_CONSTANT_TSC);
>  	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> -	if (vmware_tsc_khz)
> -		setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
>  	if (vmware_hypercall_mode == CPUID_VMWARE_FEATURES_ECX_VMCALL)
>  		setup_force_cpu_cap(X86_FEATURE_VMCALL);
>  	else if (vmware_hypercall_mode == CPUID_VMWARE_FEATURES_ECX_VMMCALL)
> diff --git a/arch/x86/kernel/jailhouse.c b/arch/x86/kernel/jailhouse.c
> index e24c05ab4fae..ff173052cdce 100644
> --- a/arch/x86/kernel/jailhouse.c
> +++ b/arch/x86/kernel/jailhouse.c
> @@ -255,7 +255,6 @@ static void __init jailhouse_init_platform(void)
>  	pr_debug("Jailhouse: PM-Timer IO Port: %#x\n", pmtmr_ioport);
> 
>  	precalibrated_tsc_khz = setup_data.v1.tsc_khz;
> -	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
> 
>  	pci_probe = 0;
> 
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index 4f8299303a19..35a879d33e9e 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -138,7 +138,6 @@ static inline void kvm_sched_clock_init(bool stable)
>   */
>  static unsigned int __init kvm_get_tsc_khz(void)
>  {
> -	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
>  	return pvclock_tsc_khz(this_cpu_pvti());
>  }
> 
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 1dca9464b41c..676910292af7 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -1541,11 +1541,18 @@ void __init tsc_early_init(void)
>  	if (!known_tsc_khz && x86_init.hyper.get_tsc_khz)
>  		known_tsc_khz = x86_init.hyper.get_tsc_khz();
> 
> +	/*
> +	 * Mark the TSC frequency as known if it was obtained from a hypervisor
> +	 * or trusted firmware.
> +	 */
> +	if (known_tsc_khz)
> +		setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
> +
>  	/*
>  	 * Ignore the user-provided TSC frequency if the exact frequency was
> -	 * obtained from trusted firmware or the hypervisor, as the user-
> -	 * provided frequency is intended as a "starting point", not a known,
> -	 * guaranteed frequency.
> +	 * obtained from trusted firmware or the hypervisor, and don't mark the
> +	 * frequency as known, as the user-provided frequency is intended as a
> +	 * "starting point", not a known, guaranteed frequency
>  	 */
>  	if (!known_tsc_khz)
>  		known_tsc_khz = tsc_early_khz;
> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> index 1adb44fdddb2..487ad838c441 100644
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -43,7 +43,6 @@ static unsigned int __init xen_tsc_khz(void)
>  	struct pvclock_vcpu_time_info *info =
>  		&HYPERVISOR_shared_info->vcpu_info[0].time;
> 
> -	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
>  	return pvclock_tsc_khz(info);
>  }
> 
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
> 


^ permalink raw reply

* RE: [PATCH v5 11/51] x86/tsc: Add dedicated hypervisor hooks for getting known TSC/CPU frequencies
From: Michael Kelley @ 2026-07-02 17:47 UTC (permalink / raw)
  To: Sean Christopherson, Jonathan Corbet, Paolo Bonzini,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	x86@kernel.org, Kiryl Shutsemau, Rick Edgecombe, K. Y. Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
	Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
	Juergen Gross, Daniel Lezcano, John Stultz
  Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
	Broadcom internal kernel review list, Boris Ostrovsky,
	Stephen Boyd, linux-doc@vger.kernel.org, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-coco@lists.linux.dev,
	linux-hyperv@vger.kernel.org, virtualization@lists.linux.dev,
	xen-devel@lists.xenproject.org, Tom Lendacky, Nikunj A Dadhania,
	David Woodhouse, David Woodhouse, Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-12-seanjc@google.com>

From: Sean Christopherson <seanjc@google.com> Sent: Wednesday, July 1, 2026 12:32 PM
> 
> Add dedicated hypervisor hooks for getting known TSC/CPU frequencies
> instead of overriding seemingly generic platform hooks, and explicitly
> priotize hypervisor-provided frequencies over native methods, but do NOT

s/priotize/prioritize/

> clobber the frequency obtained from trusted firmware.  While shuffling the
> hooks around is arguably "six of one, half dozen of the other", scoping
> them to x86_hyper_init makes their purpose more obvious, and allows for
> explicitly defining the priority of sources (as is done here).
> 
> As is already done when trusted firmware provides the TSC frequency, ignore

Word "ignore" is duplicated.

> ignore tsc_early_khz if the exact TSC frequency was obtained from the
> hypervisor, as attempting to refine the TSC frequency when running in a VM
> is all but guaranteed to cause problems sooner or later due to the
> calibration sources being emulated devices in the vast majority of setups.
> 
> Cc: David Woodhouse <dwmw2@infradead.org>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

For the Hyper-V changes,

Reviewed-by: Michael Kelley <mhklinux@outlook.com> 

> ---
>  .../admin-guide/kernel-parameters.txt         |  3 +-
>  arch/x86/include/asm/acrn.h                   |  5 ----
>  arch/x86/include/asm/x86_init.h               |  4 +++
>  arch/x86/kernel/cpu/acrn.c                    | 10 +++++--
>  arch/x86/kernel/cpu/mshyperv.c                |  6 ++--
>  arch/x86/kernel/cpu/vmware.c                  |  8 ++---
>  arch/x86/kernel/jailhouse.c                   |  6 ++--
>  arch/x86/kernel/kvmclock.c                    |  6 ++--
>  arch/x86/kernel/tsc.c                         | 29 ++++++++++++++-----
>  arch/x86/xen/time.c                           |  4 +--
>  10 files changed, 50 insertions(+), 31 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-
> guide/kernel-parameters.txt
> index 490e6aa72fc2..a387bb2c47e2 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -7948,7 +7948,8 @@ Kernel parameters
> 
>  			Note, tsc_early_khz is ignored if the TSC frequency is
>  			provided by trusted firmware when running as an SNP or
> -			TDX guest.
> +			TDX guest, or when the hypervisor provides the exact
> +			frequency via a paravirtual interface.
> 
>  	tsx=		[X86] Control Transactional Synchronization
>  			Extensions (TSX) feature in Intel processors that
> diff --git a/arch/x86/include/asm/acrn.h b/arch/x86/include/asm/acrn.h
> index db42b477c41d..a892179c61c6 100644
> --- a/arch/x86/include/asm/acrn.h
> +++ b/arch/x86/include/asm/acrn.h
> @@ -32,11 +32,6 @@ static inline u32 acrn_cpuid_base(void)
>  	return 0;
>  }
> 
> -static inline unsigned long acrn_get_tsc_khz(void)
> -{
> -	return cpuid_eax(ACRN_CPUID_TIMING_INFO);
> -}
> -
>  /*
>   * Hypercalls for ACRN
>   *
> diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
> index 953d3199408a..0c89bf40f507 100644
> --- a/arch/x86/include/asm/x86_init.h
> +++ b/arch/x86/include/asm/x86_init.h
> @@ -123,6 +123,8 @@ struct x86_init_pci {
>   * @msi_ext_dest_id:		MSI supports 15-bit APIC IDs
>   * @init_mem_mapping:		setup early mappings during init_mem_mapping()
>   * @init_after_bootmem:		guest init after boot allocator is finished
> + * @get_tsc_khz:		get the TSC frequency (returns 0 if frequency is unknown)
> + * @get_cpu_khz:		get the CPU frequency (returns 0 if frequency is unknown)
>   */
>  struct x86_hyper_init {
>  	void (*init_platform)(void);
> @@ -131,6 +133,8 @@ struct x86_hyper_init {
>  	bool (*msi_ext_dest_id)(void);
>  	void (*init_mem_mapping)(void);
>  	void (*init_after_bootmem)(void);
> +	unsigned int (*get_tsc_khz)(void);
> +	unsigned int (*get_cpu_khz)(void);
>  };
> 
>  /**
> diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
> index dc119af83524..ad8f2da8003b 100644
> --- a/arch/x86/kernel/cpu/acrn.c
> +++ b/arch/x86/kernel/cpu/acrn.c
> @@ -24,13 +24,15 @@ static u32 __init acrn_detect(void)
>  	return acrn_cpuid_base();
>  }
> 
> +static unsigned int __init acrn_get_tsc_khz(void)
> +{
> +	return cpuid_eax(ACRN_CPUID_TIMING_INFO);
> +}
> +
>  static void __init acrn_init_platform(void)
>  {
>  	/* Install system interrupt handler for ACRN hypervisor callback */
>  	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_acrn_hv_callback);
> -
> -	x86_platform.calibrate_tsc = acrn_get_tsc_khz;
> -	x86_platform.calibrate_cpu = acrn_get_tsc_khz;
>  }
> 
>  static bool acrn_x2apic_available(void)
> @@ -78,4 +80,6 @@ const __initconst struct hypervisor_x86 x86_hyper_acrn = {
>  	.type			= X86_HYPER_ACRN,
>  	.init.init_platform     = acrn_init_platform,
>  	.init.x2apic_available  = acrn_x2apic_available,
> +	.init.get_tsc_khz	= acrn_get_tsc_khz,
> +	.init.get_cpu_khz	= acrn_get_tsc_khz,
>  };
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 87beecec76f0..f9bc1c2d8c93 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -395,7 +395,7 @@ static int hv_nmi_unknown(unsigned int val, struct pt_regs *regs)
>  }
>  #endif
> 
> -static unsigned long hv_get_tsc_khz(void)
> +static unsigned int __init hv_get_tsc_khz(void)
>  {
>  	unsigned long freq;
> 
> @@ -573,8 +573,8 @@ static void __init ms_hyperv_init_platform(void)
> 
>  	if (ms_hyperv.features & HV_ACCESS_FREQUENCY_MSRS &&
>  	    ms_hyperv.misc_features & HV_FEATURE_FREQUENCY_MSRS_AVAILABLE) {
> -		x86_platform.calibrate_tsc = hv_get_tsc_khz;
> -		x86_platform.calibrate_cpu = hv_get_tsc_khz;
> +		x86_init.hyper.get_tsc_khz = hv_get_tsc_khz;
> +		x86_init.hyper.get_cpu_khz = hv_get_tsc_khz;
>  		setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
>  	}
> 
> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> index 13b97265c535..3cb473cae462 100644
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ b/arch/x86/kernel/cpu/vmware.c
> @@ -64,7 +64,7 @@ struct vmware_steal_time {
>  	u64 reserved[7];
>  };
> 
> -static unsigned long vmware_tsc_khz __ro_after_init;
> +static unsigned long vmware_tsc_khz __initdata;
>  static u8 vmware_hypercall_mode     __ro_after_init;
> 
>  unsigned long vmware_hypercall_slow(unsigned long cmd,
> @@ -137,7 +137,7 @@ static inline int __vmware_platform(void)
>  	return eax != UINT_MAX && ebx == VMWARE_HYPERVISOR_MAGIC;
>  }
> 
> -static unsigned long vmware_get_tsc_khz(void)
> +static unsigned int __init vmware_get_tsc_khz(void)
>  {
>  	return vmware_tsc_khz;
>  }
> @@ -419,8 +419,8 @@ static void __init vmware_platform_setup(void)
>  		}
> 
>  		vmware_tsc_khz = tsc_khz;
> -		x86_platform.calibrate_tsc = vmware_get_tsc_khz;
> -		x86_platform.calibrate_cpu = vmware_get_tsc_khz;
> +		x86_init.hyper.get_tsc_khz = vmware_get_tsc_khz;
> +		x86_init.hyper.get_cpu_khz = vmware_get_tsc_khz;
> 
>  		/* Skip lapic calibration since we know the bus frequency. */
>  		apic_set_timer_period_hz(ecx, "VMware hypervisor");
> diff --git a/arch/x86/kernel/jailhouse.c b/arch/x86/kernel/jailhouse.c
> index f2d4ef89c085..e24c05ab4fae 100644
> --- a/arch/x86/kernel/jailhouse.c
> +++ b/arch/x86/kernel/jailhouse.c
> @@ -68,7 +68,7 @@ static void __init jailhouse_timer_init(void)
>  	apic_set_timer_period_khz(setup_data.v1.apic_khz, "Jailhouse hypervisor");
>  }
> 
> -static unsigned long jailhouse_get_tsc(void)
> +static unsigned int __init jailhouse_get_tsc(void)
>  {
>  	return precalibrated_tsc_khz;
>  }
> @@ -210,8 +210,6 @@ static void __init jailhouse_init_platform(void)
>  	x86_init.mpparse.parse_smp_cfg		= jailhouse_parse_smp_config;
>  	x86_init.pci.arch_init			= jailhouse_pci_arch_init;
> 
> -	x86_platform.calibrate_cpu		= jailhouse_get_tsc;
> -	x86_platform.calibrate_tsc		= jailhouse_get_tsc;
>  	x86_platform.get_wallclock		= jailhouse_get_wallclock;
>  	x86_platform.legacy.rtc			= 0;
>  	x86_platform.legacy.warm_reset		= 0;
> @@ -293,5 +291,7 @@ const struct hypervisor_x86 x86_hyper_jailhouse __refconst = {
>  	.detect			= jailhouse_detect,
>  	.init.init_platform	= jailhouse_init_platform,
>  	.init.x2apic_available	= jailhouse_x2apic_available,
> +	.init.get_tsc_khz	= jailhouse_get_tsc,
> +	.init.get_cpu_khz	= jailhouse_get_tsc,
>  	.ignore_nopv		= true,
>  };
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index cb3d0ca1fa22..4f8299303a19 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -136,7 +136,7 @@ static inline void kvm_sched_clock_init(bool stable)
>   * poll of guests can be running and trouble each other. So we preset
>   * lpj here
>   */
> -static unsigned long kvm_get_tsc_khz(void)
> +static unsigned int __init kvm_get_tsc_khz(void)
>  {
>  	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
>  	return pvclock_tsc_khz(this_cpu_pvti());
> @@ -343,8 +343,8 @@ void __init kvmclock_init(void)
>  	flags = pvclock_read_flags(&hv_clock_boot[0].pvti);
>  	kvm_sched_clock_init(flags & PVCLOCK_TSC_STABLE_BIT);
> 
> -	x86_platform.calibrate_tsc = kvm_get_tsc_khz;
> -	x86_platform.calibrate_cpu = kvm_get_tsc_khz;
> +	x86_init.hyper.get_tsc_khz = kvm_get_tsc_khz;
> +	x86_init.hyper.get_cpu_khz = kvm_get_tsc_khz;
>  	x86_platform.get_wallclock = kvm_get_wallclock;
>  	x86_platform.set_wallclock = kvm_set_wallclock;
>  #ifdef CONFIG_X86_LOCAL_APIC
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 86384a83a5f6..1dca9464b41c 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -1451,13 +1451,17 @@ static int __init init_tsc_clocksource(void)
>  device_initcall(init_tsc_clocksource);
> 
>  static bool __init determine_cpu_tsc_frequencies(bool early,
> +						 unsigned int known_cpu_khz,
>  						 unsigned int known_tsc_khz)
>  {
>  	/* Make sure that cpu and tsc are not already calibrated */
>  	WARN_ON(cpu_khz || tsc_khz);
> 
>  	if (early) {
> -		cpu_khz = x86_platform.calibrate_cpu();
> +		if (known_cpu_khz)
> +			cpu_khz = known_cpu_khz;
> +		else
> +			cpu_khz = x86_platform.calibrate_cpu();
>  		if (known_tsc_khz)
>  			tsc_khz = known_tsc_khz;
>  		else
> @@ -1514,7 +1518,7 @@ static void __init tsc_enable_sched_clock(void)
> 
>  void __init tsc_early_init(void)
>  {
> -	unsigned int known_tsc_khz = 0;
> +	unsigned int known_cpu_khz = 0, known_tsc_khz = 0;
> 
>  	if (!boot_cpu_has(X86_FEATURE_TSC))
>  		return;
> @@ -1522,22 +1526,33 @@ void __init tsc_early_init(void)
>  	if (is_early_uv_system())
>  		return;
> 
> +	if (x86_init.hyper.get_cpu_khz)
> +		known_cpu_khz = x86_init.hyper.get_cpu_khz();
> +
>  	if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
>  		known_tsc_khz = snp_secure_tsc_init();
>  	else if (boot_cpu_has(X86_FEATURE_TDX_GUEST))
>  		known_tsc_khz = tdx_tsc_init();
> 
> +	/*
> +	 * If the TSC frequency wasn't provided by trusted firmware, try to get
> +	 * it from the hypervisor (which is untrusted when running as a CoCo guest).
> +	 */
> +	if (!known_tsc_khz && x86_init.hyper.get_tsc_khz)
> +		known_tsc_khz = x86_init.hyper.get_tsc_khz();
> +
>  	/*
>  	 * Ignore the user-provided TSC frequency if the exact frequency was
> -	 * obtained from trusted firmware, as the user-provided frequency is
> -	 * intended as a "starting point", not a known, guaranteed frequency.
> +	 * obtained from trusted firmware or the hypervisor, as the user-
> +	 * provided frequency is intended as a "starting point", not a known,
> +	 * guaranteed frequency.
>  	 */
>  	if (!known_tsc_khz)
>  		known_tsc_khz = tsc_early_khz;
>  	else if (tsc_early_khz)
> -		pr_err("Ignoring 'tsc_early_khz' in favor of trusted firmware.\n");
> +		pr_err("Ignoring 'tsc_early_khz' in favor of firmware/hypervisor.\n");
> 
> -	if (!determine_cpu_tsc_frequencies(true, known_tsc_khz))
> +	if (!determine_cpu_tsc_frequencies(true, known_cpu_khz, known_tsc_khz))
>  		return;
>  	tsc_enable_sched_clock();
>  }
> @@ -1558,7 +1573,7 @@ void __init tsc_init(void)
> 
>  	if (!tsc_khz) {
>  		/* We failed to determine frequencies earlier, try again */
> -		if (!determine_cpu_tsc_frequencies(false, 0)) {
> +		if (!determine_cpu_tsc_frequencies(false, 0, 0)) {
>  			mark_tsc_unstable("could not calculate TSC khz");
>  			setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE_TIMER);
>  			return;
> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> index d62c14334b35..1adb44fdddb2 100644
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -38,7 +38,7 @@
>  static u64 xen_sched_clock_offset __read_mostly;
> 
>  /* Get the TSC speed from Xen */
> -static unsigned long xen_tsc_khz(void)
> +static unsigned int __init xen_tsc_khz(void)
>  {
>  	struct pvclock_vcpu_time_info *info =
>  		&HYPERVISOR_shared_info->vcpu_info[0].time;
> @@ -569,7 +569,7 @@ static void __init xen_init_time_common(void)
>  	static_call_update(pv_steal_clock, xen_steal_clock);
>  	paravirt_set_sched_clock(xen_sched_clock);
> 
> -	x86_platform.calibrate_tsc = xen_tsc_khz;
> +	x86_init.hyper.get_tsc_khz = xen_tsc_khz;
>  	x86_platform.get_wallclock = xen_get_wallclock;
>  }
> 
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
> 


^ permalink raw reply

* RE: [PATCH v5 01/51] x86/apic: Provide helpers to set local APIC timer period in hz and khz
From: Michael Kelley @ 2026-07-02 17:47 UTC (permalink / raw)
  To: Sean Christopherson, Jonathan Corbet, Paolo Bonzini,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	x86@kernel.org, Kiryl Shutsemau, Rick Edgecombe, K. Y. Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
	Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
	Juergen Gross, Daniel Lezcano, John Stultz
  Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
	Broadcom internal kernel review list, Boris Ostrovsky,
	Stephen Boyd, linux-doc@vger.kernel.org, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-coco@lists.linux.dev,
	linux-hyperv@vger.kernel.org, virtualization@lists.linux.dev,
	xen-devel@lists.xenproject.org, Tom Lendacky, Nikunj A Dadhania,
	David Woodhouse, David Woodhouse, Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-2-seanjc@google.com>

From: Sean Christopherson <seanjc@google.com> Sent: Wednesday, July 1, 2026 12:31 PM
> 
> Add and use APIs to set the local APIC timer period instead of open coding
> the subtle HZ math in a all external callers, and make lapic_timer_period

Spurious word "a".

> local to apic.c.  Provide APIs to specify the frequency in both hertz and
> kilohertz so that Hyper-V and VMware code aren't forced to lose precision.
> 
> Opportunistically use mul_u64_u32_div() to harden against the possibility
> that the period in Khz is greater than 4294967, i.e. if the APIC timer runs
> at ~4.29 GHz.  As pointed out by Sashiko, 4294968 * 1000 == 0x1_000002c0,
> and thus a Khz period of 4294968 would silently overflow the 32-bit
> unsigned integer used by most callers.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/apic.h    |  3 ++-
>  arch/x86/kernel/apic/apic.c    | 12 +++++++++++-
>  arch/x86/kernel/cpu/mshyperv.c |  5 +----
>  arch/x86/kernel/cpu/vmware.c   |  4 +---
>  arch/x86/kernel/jailhouse.c    |  2 +-
>  arch/x86/kernel/tsc.c          |  2 +-
>  arch/x86/kernel/tsc_msr.c      |  2 +-
>  7 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> index 9cd493d467d4..cd84a94688a2 100644
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -63,7 +63,6 @@ extern int apic_verbosity;
>  extern int local_apic_timer_c2_ok;
> 
>  extern bool apic_is_disabled;
> -extern unsigned int lapic_timer_period;
> 
>  extern enum apic_intr_mode_id apic_intr_mode;
>  enum apic_intr_mode_id {
> @@ -138,6 +137,8 @@ void register_lapic_address(unsigned long address);
>  extern void setup_boot_APIC_clock(void);
>  extern void setup_secondary_APIC_clock(void);
>  extern void lapic_update_tsc_freq(void);
> +extern void apic_set_timer_period_hz(u64 period_hz, const char *source);
> +extern void apic_set_timer_period_khz(u64 period_khz, const char *source);
> 
>  #ifdef CONFIG_X86_64
>  static inline bool apic_force_enable(unsigned long addr)
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index aa1e19979aa8..8d3d930576fd 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -176,7 +176,7 @@ static struct resource lapic_resource = {
>  };
> 
>  /* Measured in ticks per HZ. */
> -unsigned int lapic_timer_period = 0;
> +static unsigned int lapic_timer_period;
> 
>  static void apic_pm_activate(void);
> 
> @@ -796,6 +796,16 @@ bool __init apic_needs_pit(void)
>  	return lapic_timer_period == 0;
>  }
> 
> +void apic_set_timer_period_khz(u64 period_khz, const char *source)
> +{
> +	lapic_timer_period = mul_u64_u32_div(period_khz, 1000, HZ);
> +}
> +
> +void apic_set_timer_period_hz(u64 period_hz, const char *source)
> +{
> +	lapic_timer_period = div_u64(period_hz, HZ);
> +}

A string "source" argument is passed in, but not used. Is there an
envisioned future use? Also, this function doesn't output a pr_info()
message like the existing Hyper-V and VMware code does. I don't
know that the message is all that useful, though I do remember
one case where I was debugging some clock/timer issue when I
looked at it. 

Michael

> +
>  static int __init calibrate_APIC_clock(void)
>  {
>  	struct clock_event_device *levt = this_cpu_ptr(&lapic_events);
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 185d4f677ec0..87beecec76f0 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -646,10 +646,7 @@ static void __init ms_hyperv_init_platform(void)
>  		u64	hv_lapic_frequency;
> 
>  		rdmsrq(HV_X64_MSR_APIC_FREQUENCY, hv_lapic_frequency);
> -		hv_lapic_frequency = div_u64(hv_lapic_frequency, HZ);
> -		lapic_timer_period = hv_lapic_frequency;
> -		pr_info("Hyper-V: LAPIC Timer Frequency: %#x\n",
> -			lapic_timer_period);
> +		apic_set_timer_period_hz(hv_lapic_frequency, "Hyper-V hypervisor");
>  	}
> 
>  	register_nmi_handler(NMI_UNKNOWN, hv_nmi_unknown, NMI_FLAG_FIRST,
> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> index 34b73573b108..36f779dd311d 100644
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ b/arch/x86/kernel/cpu/vmware.c
> @@ -424,9 +424,7 @@ static void __init vmware_platform_setup(void)
> 
>  #ifdef CONFIG_X86_LOCAL_APIC
>  		/* Skip lapic calibration since we know the bus frequency. */
> -		lapic_timer_period = ecx / HZ;
> -		pr_info("Host bus clock speed read from hypervisor : %u Hz\n",
> -			ecx);
> +		apic_set_timer_period_hz(ecx, "VMware hypervisor");
>  #endif
>  	} else {
>  		pr_warn("Failed to get TSC freq from the hypervisor\n");
> diff --git a/arch/x86/kernel/jailhouse.c b/arch/x86/kernel/jailhouse.c
> index f58ce9220e0f..f2d4ef89c085 100644
> --- a/arch/x86/kernel/jailhouse.c
> +++ b/arch/x86/kernel/jailhouse.c
> @@ -65,7 +65,7 @@ static void jailhouse_get_wallclock(struct timespec64 *now)
> 
>  static void __init jailhouse_timer_init(void)
>  {
> -	lapic_timer_period = setup_data.v1.apic_khz * (1000 / HZ);
> +	apic_set_timer_period_khz(setup_data.v1.apic_khz, "Jailhouse hypervisor");
>  }
> 
>  static unsigned long jailhouse_get_tsc(void)
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index ce10ae4b298b..f9ecc9256863 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -717,7 +717,7 @@ unsigned long native_calibrate_tsc(void)
>  	 * lapic_timer_period here to avoid having to calibrate the APIC
>  	 * timer later.
>  	 */
> -	lapic_timer_period = crystal_khz * 1000 / HZ;
> +	apic_set_timer_period_khz(crystal_khz, "CPUID 0x15/0x16");
>  #endif
> 
>  	return crystal_khz * ebx_numerator / eax_denominator;
> diff --git a/arch/x86/kernel/tsc_msr.c b/arch/x86/kernel/tsc_msr.c
> index 48e6cc1cb017..7e990871e041 100644
> --- a/arch/x86/kernel/tsc_msr.c
> +++ b/arch/x86/kernel/tsc_msr.c
> @@ -211,7 +211,7 @@ unsigned long cpu_khz_from_msr(void)
>  		pr_err("Error MSR_FSB_FREQ index %d is unknown\n", index);
> 
>  #ifdef CONFIG_X86_LOCAL_APIC
> -	lapic_timer_period = (freq * 1000) / HZ;
> +	apic_set_timer_period_khz(freq, "MSR_FSB_FREQ");
>  #endif
> 
>  	/*
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
> 


^ permalink raw reply

* Re: [RFC PATCH 09/15] x86/virt/tdx: Add interface to generate a Quote
From: Xu Yilun @ 2026-07-02 15:58 UTC (permalink / raw)
  To: Nikolay Borisov
  Cc: kas, djbw, rick.p.edgecombe, x86, peter.fang, linux-coco,
	linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
	zhenzhong.duan, xiaoyao.li
In-Reply-To: <a8441214-1560-498d-ba01-0807502d0e28@suse.com>

> > +
> > +	do {
> > +		r = seamcall_ret(TDH_QUOTE_GET, &args);
> > +	} while (r == TDX_INTERRUPTED_RESUMABLE);
> 
> 
> nit: This pattern seems to repeat a lot, might be worth it to consider
> introducing a wrapper similar to existing sc_retry?

We didn't make the wrapper because TDX_INTERRUPTED_RESUMABLE can't be
handled in a unified way for all SEAMCALLs, I see 2 conventions:

1. Host should set a resume=1 parameter on retry, such as
   TDH_PHYMEM_CACHE_WB, and several migration SEAMCALLs,
   TDH.EXPORT.MEM, TDH.EXPORT.STATE.XXX

2. Host just refills the previous args (may have been modified by seamcall
   output) on retry. such as TDH_EXT_INIT, TDH_EXT_MEM_ADD,
   TDH_QUOTE_INIT, TDH_QUOTE_GET...

Making a helper that can't cover all interrupted resumable case may
cause more confusing. BTW, there are already many seamcall wrappers, we
need to carefully evaluate if it is worth adding new wrappers, for now I
don't think so. 

^ permalink raw reply

* Re: [PATCH v5 2/3] x86/insn-eval: Add insn_assign_reg() helper
From: Kiryl Shutsemau @ 2026-07-02 15:30 UTC (permalink / raw)
  To: David Laight, Sean Christopherson
  Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	Paolo Bonzini, Kuppuswamy Sathyanarayanan, Kai Huang, Xiaoyao Li,
	Rick Edgecombe, Binbin Wu, Andi Kleen, Dan Williams,
	Borys Tsyrulnikov, kvm, linux-coco, linux-kernel, stable
In-Reply-To: <20260701180033.6e9c07aa@pumpkin>

On Wed, Jul 01, 2026 at 06:00:33PM +0100, David Laight wrote:
> On Wed, 1 Jul 2026 07:59:05 -0700
> Sean Christopherson <seanjc@google.com> wrote:
> 
> > On Wed, Jul 01, 2026, Kiryl Shutsemau wrote:
> > > From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
> > > 
> > > KVM's instruction emulator has a small helper, assign_register(), that
> > > writes a value into a sub-register with x86 partial-register-write
> > > semantics: 1- and 2-byte writes leave the upper bits of the destination
> > > untouched, 4-byte writes zero-extend to 64 bits, 8-byte writes overwrite
> > > the full register.
> > > 
> > > The TDX guest #VE handler needs the same logic for port I/O emulation
> > > to get 32-bit zero-extension right.  Rather than copy-pasting the
> > > helper, lift it to <asm/insn-eval.h> as insn_assign_reg() so both can
> > > use it.
> > > 
> > > Add <asm/insn.h> to the header's includes so it builds standalone in
> > > callers that have not pulled it in transitively.
> > > 
> > > No functional change.
> > > 
> > > Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> > > Cc: stable@vger.kernel.org # prerequisite for the following 32-bit port I/O zero-extension fix
> > > ---
> > >  arch/x86/include/asm/insn-eval.h | 30 ++++++++++++++++++++++++++++++
> > >  arch/x86/kvm/emulate.c           | 26 ++++----------------------
> > >  2 files changed, 34 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
> > > index 4733e9064ee5..0c87759816d3 100644
> > > --- a/arch/x86/include/asm/insn-eval.h
> > > +++ b/arch/x86/include/asm/insn-eval.h
> > > @@ -9,6 +9,7 @@
> > >  #include <linux/compiler.h>
> > >  #include <linux/bug.h>
> > >  #include <linux/err.h>
> > > +#include <asm/insn.h>
> > >  #include <asm/ptrace.h>
> > >  
> > >  #define INSN_CODE_SEG_ADDR_SZ(params) ((params >> 4) & 0xf)
> > > @@ -46,4 +47,33 @@ enum insn_mmio_type insn_decode_mmio(struct insn *insn, int *bytes);
> > >  
> > >  bool insn_is_nop(struct insn *insn);
> > >  
> > > +/*
> > > + * Write @val into *@reg with x86 partial-register-write semantics: a 1-
> > > + * or 2-byte write leaves the upper bits of the destination untouched; a
> > > + * 4-byte write zero-extends to 64 bits (matching IN[BWL], MOV[BWL]  
> > 
> > The placement of the "(matching IN[BWL], MOV[BWL] etc.)" blurb is confusing.  I
> > *think* you're trying to say this behavior matches that of MOVB, MOVW, and MOVL
> > instruction mnemonics, but the blurb is buried in the snippet that specifically
> > describes the 4-byte write behavior.
> > 
> > FWIW, I think giving examples does more harm than good, because the behavior isn't
> > instruction specific, it's architectural behavior that applies to all writes to
> > GPRs, as defined in "3.4.1.1 General-Purpose Registers in 64-Bit Mode".  E.g. for
> > a MOV instruction that sign-extends a 32-bit immediate to a 64-bit registers, it's
> > not that the instruction is exempt from the normal GPR semenatics, it's that the
> > instruction performs a 64-bit write to the destination even though the source is
> > only 32 bits.
> > 
> > And the B/W/L terminology isn't architectural, it's AT&T syntax.

Agreed.  Dropped the IN[BWL]/MOV[BWL] examples and reworded the comment
to describe architectural GPR-write behaviour with a pointer to the SDM
section instead.  I also spelled out that @bytes is the width of the
write, not a property of the instruction, to cover the sign-extending
MOV case you raised.

> > E.g. trying
> > to encode "movl" with NASM yields "error: instruction expected, found `movl dword'".
> > Yes, the kernel uses AT&T syntax for assembly, but I think this helper should very
> > explicitly document that it's emulating architectural behavior.
> > 
> > > + * etc.); an 8-byte write overwrites the full register.
> > > + *
> > > + * @reg need not be 8-byte aligned: KVM's instruction emulator points
> > > + * into the middle of a register slot to address the high-byte
>                  ^ it isn't really the 'middle'.

Reworded to "offsets the pointer by one byte".

> > > + * registers (AH, CH, DH, BH).  Use narrow stores for the sub-word
> > > + * cases so that the access width matches @bytes.
> > > + */
> > > +static inline void insn_assign_reg(unsigned long *reg, u64 val, int bytes)
> > > +{
> > > +	switch (bytes) {
> > > +	case 1:
> > > +		*(u8 *)reg = (u8)val;
> > > +		break;
> > > +	case 2:
> > > +		*(u16 *)reg = (u16)val;
> > > +		break;
> > > +	case 4:
> > > +		*reg = (u32)val;  
> > 
> > IMO, it's worth keeping a short comment here, because even with the explanation
> > above, I suspect most people will think the code is buggy.  E.g.
> > 
> > 		/* As above, zero-extend 4-byte writes on 64-bit CPUs. */
> > 		*reg = (u32)val;

Added on the 4-byte case, slightly reworded.

> Or be even more specific and use '& 0xffffffff' rather than a cast.
> Particularly since the casts of the RHS in the byte/short cases aren't
> needed at all.

I'd rather keep the body exactly as KVM has it today.  This is now a
straight move + rename with no functional change, and the v4 attempt to
rewrite it with arithmetic is precisely what introduced the AH/CH/DH/BH
clobber Sashiko flagged.  Tidying the casts turns it back into a rewrite
and diverges from the form KVM has shipped for years.  Feel free to
submit a separate cleanup on top if you feel strongly.

Updated patch below; I'll fold it into v6.

-- >8 --
Subject: [PATCH] x86/insn-eval: Move assign_register() out of KVM as insn_assign_reg()

KVM's instruction emulator has a small helper, assign_register(), that
writes a value into a register following the x86 rules for writes to
general-purpose registers: an 8- or 16-bit write leaves the rest of the
register untouched, a 32-bit write zero-extends the result to 64 bits,
and a 64-bit write replaces the whole register.

The TDX guest #VE handler needs the same logic for port I/O emulation
to get 32-bit zero-extension right.  Rather than add a third copy of
the same switch, move the helper verbatim to <asm/insn-eval.h>, rename
it to insn_assign_reg(), and route KVM's callers through it.

Add <asm/insn.h> to the header's includes so it builds standalone in
callers that have not pulled it in transitively.

No functional change.

Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Cc: stable@vger.kernel.org # prerequisite for the following 32-bit port I/O zero-extension fix
---
 arch/x86/include/asm/insn-eval.h | 36 ++++++++++++++++++++++++++++++++
 arch/x86/kvm/emulate.c           | 26 ++++-------------------
 2 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
index 4733e9064ee5..ae05647a0afb 100644
--- a/arch/x86/include/asm/insn-eval.h
+++ b/arch/x86/include/asm/insn-eval.h
@@ -9,6 +9,7 @@
 #include <linux/compiler.h>
 #include <linux/bug.h>
 #include <linux/err.h>
+#include <asm/insn.h>
 #include <asm/ptrace.h>

 #define INSN_CODE_SEG_ADDR_SZ(params) ((params >> 4) & 0xf)
@@ -46,4 +47,39 @@ enum insn_mmio_type insn_decode_mmio(struct insn *insn, int *bytes);

 bool insn_is_nop(struct insn *insn);

+/*
+ * Write @val into *@reg following the x86 rules for writes to
+ * general-purpose registers (Intel SDM Vol. 1, "General-Purpose
+ * Registers in 64-Bit Mode"): an 8- or 16-bit write leaves the rest of
+ * the register untouched, a 32-bit write zero-extends the result into
+ * the upper 32 bits, and a 64-bit write replaces the whole register.
+ *
+ * @bytes is the width of the write, not a property of the instruction:
+ * an instruction that, say, sign-extends a 32-bit immediate into a
+ * 64-bit register does a 64-bit write here.
+ *
+ * @reg need not be 8-byte aligned: KVM's instruction emulator offsets
+ * the pointer by one byte to address the high-byte registers (AH, CH,
+ * DH, BH).  Use narrow stores for the sub-word cases so the access
+ * width matches @bytes and the adjacent bytes are left alone.
+ */
+static inline void insn_assign_reg(unsigned long *reg, u64 val, int bytes)
+{
+	switch (bytes) {
+	case 1:
+		*(u8 *)reg = (u8)val;
+		break;
+	case 2:
+		*(u16 *)reg = (u16)val;
+		break;
+	case 4:
+		/* A 32-bit write zero-extends into the upper 32 bits. */
+		*reg = (u32)val;
+		break;
+	case 8:
+		*reg = val;
+		break;
+	}
+}
+
 #endif /* _ASM_X86_INSN_EVAL_H */
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index b566ab5c7515..c6dcb5ac48af 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -24,6 +24,7 @@
 #include "kvm_emulate.h"
 #include <linux/stringify.h>
 #include <asm/debugreg.h>
+#include <asm/insn-eval.h>
 #include <asm/nospec-branch.h>
 #include <asm/ibt.h>
 #include <asm/text-patching.h>
@@ -439,25 +440,6 @@ static void assign_masked(ulong *dest, ulong src, ulong mask)
 	*dest = (*dest & ~mask) | (src & mask);
 }

-static void assign_register(unsigned long *reg, u64 val, int bytes)
-{
-	/* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */
-	switch (bytes) {
-	case 1:
-		*(u8 *)reg = (u8)val;
-		break;
-	case 2:
-		*(u16 *)reg = (u16)val;
-		break;
-	case 4:
-		*reg = (u32)val;
-		break;	/* 64b: zero-extend */
-	case 8:
-		*reg = val;
-		break;
-	}
-}
-
 static inline unsigned long ad_mask(struct x86_emulate_ctxt *ctxt)
 {
 	return (1UL << (ctxt->ad_bytes << 3)) - 1;
@@ -505,7 +487,7 @@ register_address_increment(struct x86_emulate_ctxt *ctxt, int reg, int inc)
 {
 	ulong *preg = reg_rmw(ctxt, reg);

-	assign_register(preg, *preg + inc, ctxt->ad_bytes);
+	insn_assign_reg(preg, *preg + inc, ctxt->ad_bytes);
 }

 static void rsp_increment(struct x86_emulate_ctxt *ctxt, int inc)
@@ -1767,7 +1749,7 @@ static int load_segment_descriptor(struct x86_emulate_ctxt *ctxt,

 static void write_register_operand(struct operand *op)
 {
-	return assign_register(op->addr.reg, op->val, op->bytes);
+	return insn_assign_reg(op->addr.reg, op->val, op->bytes);
 }

 static int writeback(struct x86_emulate_ctxt *ctxt, struct operand *op)
@@ -2008,7 +1990,7 @@ static int em_popa(struct x86_emulate_ctxt *ctxt)
 		rc = emulate_pop(ctxt, &val, ctxt->op_bytes);
 		if (rc != X86EMUL_CONTINUE)
 			break;
-		assign_register(reg_rmw(ctxt, reg), val, ctxt->op_bytes);
+		insn_assign_reg(reg_rmw(ctxt, reg), val, ctxt->op_bytes);
 		--reg;
 	}
 	return rc;

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

^ permalink raw reply related

* Re: [RFC PATCH 12/15] KVM: TDX: Add in-kernel Quote generation
From: Nikolay Borisov @ 2026-07-02 15:26 UTC (permalink / raw)
  To: Xu Yilun, kas, djbw, rick.p.edgecombe, x86, peter.fang
  Cc: linux-coco, linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
	zhenzhong.duan, xiaoyao.li
In-Reply-To: <20260522034128.3144354-13-yilun.xu@linux.intel.com>



On 5/22/26 06:41, Xu Yilun wrote:
> From: Peter Fang <peter.fang@intel.com>
> 
> Provide an in-kernel path for TDX Quote generation when handling
> TDG.VP.VMCALL<GetQuote>, without requiring an exit to userspace.
> 
> Use the core TDX API when the TDX Quoting extension is available. For
> simplicity, each KVM guest checks for availability only once during
> initialization. KVM does not handle Quoting service disruptions.

I think calling this in-kernel quote generation vs user-spcae quote 
generation is misleading. Rather the distinction is : quote via the vmm 
or a quote generated by the tdx module/tdx extension. I think this 
should be the level at which the distinction is made.


i.e tdx_get_quote_vmm vs tdx_get_quote_ext or some along those lines.


<snip>

^ permalink raw reply

* Re: [PATCH v6 00/20] dma-mapping: Use DMA_ATTR_CC_SHARED through direct, pool and swiotlb paths
From: Jason Gunthorpe @ 2026-07-02 15:19 UTC (permalink / raw)
  To: Aneesh Kumar K.V
  Cc: Alexey Kardashevskiy, Catalin Marinas, iommu, linux-arm-kernel,
	linux-kernel, linux-coco, Robin Murphy, Marek Szyprowski,
	Will Deacon, Marc Zyngier, Steven Price, Suzuki K Poulose,
	Jiri Pirko, Mostafa Saleh, Petr Tesarik, Dan Williams, Xu Yilun,
	linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
	Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <yq5abjcro17x.fsf@kernel.org>

On Wed, Jul 01, 2026 at 08:39:06AM +0530, Aneesh Kumar K.V wrote:

> This is the only code path where we need to special-case host memory
> encryption. For this reason, I have avoided renaming
> io_tlb_mem::cc_shared to io_tlb_mem::unencrypted. I can send a v7 with
> the above and we can review the changes based on that?

I'm OK with this

Jason

^ permalink raw reply

* Re: [PATCH v6 03/20] dma-direct: use DMA_ATTR_CC_SHARED in alloc/free paths
From: Jason Gunthorpe @ 2026-07-02 14:47 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Aneesh Kumar K.V (Arm), iommu, linux-arm-kernel, linux-kernel,
	linux-coco, Robin Murphy, Marek Szyprowski, Will Deacon,
	Marc Zyngier, Steven Price, Suzuki K Poulose, Catalin Marinas,
	Jiri Pirko, Mostafa Saleh, Petr Tesarik, Dan Williams, Xu Yilun,
	linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
	Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
	Michael Kelley, Cheloha, Scott
In-Reply-To: <98800d21-18f6-44d6-9ba0-da9b77567d85@amd.com>

On Thu, Jul 02, 2026 at 10:25:16AM +1000, Alexey Kardashevskiy wrote:

> > > not externally available so I'll have to trick the DMA layer into
> > > using SWIOTLB (which is still all shared, right?) as I specifically
> > > want to skip page conversions. Setting low DMA mask won't guarantee
> > > that the DMA layer won't allocate a page outside of SWIOTLB and
> > > convert it. Manually do
> > 
> > Why so particular?
> 
> aahhh I missed "pre-". I need a way to get pre-shared pages for my
> sev-guest activities.

It sounds to me like you don't, what you are worried about is
optimizing the dma_alloc_coherent flow to avoid fragmentation and all
CC architetures require this optimization.

So I'm strongly against you trying to open code something in your
driver. Use the DMA API as-is. Propose general DMA API improvements to
reduce the fragmentation problem. It is a real problem.

Jason

^ permalink raw reply

* [PATCH] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Xu Yilun @ 2026-07-02 14:46 UTC (permalink / raw)
  To: x86, linux-kernel
  Cc: kas, rick.p.edgecombe, dave.hansen, dave.hansen, yilun.xu,
	yilun.xu, chao.gao, djbw, linux-coco, peter.fang

TDX uses the SEAMCALL instruction to invoke various TDX module
functions. Just like the syscall, a SEAMCALL specifies the operation
using a function number and parameters. Moreover, TDX also uses SEAMCALL
versions to extend the functionalities of existing SEAMCALLs while
keeping backward compatibility. Unlike syscall versions that assign
brand new numbers, TDX segments the function number into a basic
function number field and a version field. Together, they encode the new
function number.

An existing SEAMCALL (TDH.VP.INIT) helper is already using the version
field. However, having the caller pack the version into the function
number open-codes the ABI layout, making the SEAMCALL helper definition
obscure and error prone.

Add a version field in struct tdx_module_args, so that most existing
SEAMCALL helpers get a default "version == 0" behavior without code
churn, while callers requiring extended functionalities can specify the
version descriptively. As an internal implementation detail, encode the
tdx_module_args.version in the function number before calling into
assembly code.

Two alternative schemes were considered:

1. Define versioned macros like TDH_VP_INIT_V0, TDH_VP_INIT_V1, etc.
   However, this breaks naming consistency unless all existing stable
   function macros are changed to TDH_XXX_V0.

2. Add an explicit 'version' parameter to the base seamcall() API. This
   unnecessarily forces all stable SEAMCALL helpers to pass a
   meaningless '0' argument. Additionally, the magic '0' or '1' values
   at caller sites are not descriptive.

Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
---
 arch/x86/include/asm/shared/tdx.h         |  2 ++
 arch/x86/virt/vmx/tdx/seamcall_internal.h | 19 ++++++++++++++++++-
 arch/x86/virt/vmx/tdx/tdx.h               |  8 --------
 arch/x86/virt/vmx/tdx/tdx.c               |  5 +++--
 4 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
index f20e91d7ac35..b9aac2de233a 100644
--- a/arch/x86/include/asm/shared/tdx.h
+++ b/arch/x86/include/asm/shared/tdx.h
@@ -143,6 +143,8 @@ struct tdx_module_args {
 	u64 rbx;
 	u64 rdi;
 	u64 rsi;
+	/* ABI version, encoded in rax */
+	u8  version;
 };
 
 /* Used to communicate with the TDX module */
diff --git a/arch/x86/virt/vmx/tdx/seamcall_internal.h b/arch/x86/virt/vmx/tdx/seamcall_internal.h
index be5f446467df..7002e41cddad 100644
--- a/arch/x86/virt/vmx/tdx/seamcall_internal.h
+++ b/arch/x86/virt/vmx/tdx/seamcall_internal.h
@@ -11,6 +11,7 @@
 #ifndef _X86_VIRT_SEAMCALL_INTERNAL_H
 #define _X86_VIRT_SEAMCALL_INTERNAL_H
 
+#include <linux/bitfield.h>
 #include <linux/printk.h>
 #include <linux/types.h>
 #include <asm/archrandom.h>
@@ -23,6 +24,22 @@ u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args);
 
 typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args);
 
+/*
+ * SEAMCALL leaf:
+ *
+ * Bit 15:0	Leaf number
+ * Bit 23:16	Version number
+ */
+#define SEAMCALL_VERSION_MASK		GENMASK_U64(23, 16)
+
+static __always_inline u64 __seamcall_encode_fn(sc_func_t func, u64 fn,
+						struct tdx_module_args *args)
+{
+	FIELD_MODIFY(SEAMCALL_VERSION_MASK, &fn, args->version);
+
+	return func(fn, args);
+}
+
 static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
 						  struct tdx_module_args *args)
 {
@@ -39,7 +56,7 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
 	 */
 	this_cpu_write(cache_state_incoherent, true);
 
-	return func(fn, args);
+	return __seamcall_encode_fn(func, fn, args);
 }
 
 static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index bdfd0e1e337a..63e3acfb5d0c 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -50,14 +50,6 @@
 #define TDH_SYS_UPDATE			53
 #define TDH_SYS_DISABLE			69
 
-/*
- * SEAMCALL leaf:
- *
- * Bit 15:0	Leaf number
- * Bit 23:16	Version number
- */
-#define TDX_VERSION_SHIFT		16
-
 /* TDX page types */
 #define	PT_NDA		0x0
 #define	PT_RSVD		0x1
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 42df8ea464c4..7a89e29b118c 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1910,10 +1910,11 @@ u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid)
 		.rcx = vp->tdvpr_pa,
 		.rdx = initial_rcx,
 		.r8 = x2apicid,
+		/* apicid requires version == 1. */
+		.version = 1,
 	};
 
-	/* apicid requires version == 1. */
-	return seamcall(TDH_VP_INIT | (1ULL << TDX_VERSION_SHIFT), &args);
+	return seamcall(TDH_VP_INIT, &args);
 }
 EXPORT_SYMBOL_FOR_KVM(tdh_vp_init);
 
-- 
2.25.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox