linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Improve KVM_SET_TSC_KHZ handling for CoCo VMs
@ 2025-07-13 22:20 Kai Huang
  2025-07-13 22:20 ` [PATCH v2 1/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created Kai Huang
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Kai Huang @ 2025-07-13 22:20 UTC (permalink / raw)
  To: seanjc, pbonzini
  Cc: kvm, thomas.lendacky, nikunj, bp, isaku.yamahata, xiaoyao.li,
	rick.p.edgecombe, chao.gao, linux-kernel

This series follows Sean's suggestions [1][2] to:

 - Reject vCPU scope KVM_SET_TSC_KHZ ioctl for TSC protected vCPU
 - Reject VM scope KVM_SET_TSC_KHZ ioctl when vCPUs have been created

.. in the discussion of SEV-SNP Secure TSC support series.

v1 -> v2:
 - Protect kvm->create_vcpus check using kvm->lock. - Chao.
 - Add documentation update to each patch.  -Nikunj.
 - Collect RB (Xiaoyao, Nikunj).
 - Switched the patch order to make documentation update easier.

 v1: https://lore.kernel.org/kvm/cover.1752038725.git.kai.huang@intel.com/

Hi Xiaoyao, Nikunj, I added your RB anyway, so let me know if you have
concern :-)

This series has been sanity tested with TDX guests using today's Qemu:

 - With this series Qemu can still run TDX guests successfully.
 - With some hack to the Qemu, both VM and vCPU scope KVM_SET_TSC_KHZ
   ioctls failed as expected.

Kai Huang (2):
  KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created
  KVM: x86: Reject KVM_SET_TSC_KHZ vCPU ioctl for TSC protected guest

 Documentation/virt/kvm/api.rst |  9 ++++++++-
 arch/x86/kvm/x86.c             | 13 ++++++++++---
 2 files changed, 18 insertions(+), 4 deletions(-)


base-commit: 6c7ecd725e503bf2ca69ff52c6cc48bb650b1f11
-- 
2.50.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created
  2025-07-13 22:20 [PATCH v2 0/2] Improve KVM_SET_TSC_KHZ handling for CoCo VMs Kai Huang
@ 2025-07-13 22:20 ` Kai Huang
  2025-07-14  5:43   ` Nikunj A. Dadhania
  2025-07-14  8:44   ` Chao Gao
  2025-07-13 22:20 ` [PATCH v2 2/2] KVM: x86: Reject KVM_SET_TSC_KHZ vCPU ioctl for TSC protected guest Kai Huang
  2025-07-15  0:23 ` [PATCH v2 0/2] Improve KVM_SET_TSC_KHZ handling for CoCo VMs Sean Christopherson
  2 siblings, 2 replies; 8+ messages in thread
From: Kai Huang @ 2025-07-13 22:20 UTC (permalink / raw)
  To: seanjc, pbonzini
  Cc: kvm, thomas.lendacky, nikunj, bp, isaku.yamahata, xiaoyao.li,
	rick.p.edgecombe, chao.gao, linux-kernel

Reject the KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created and
update the documentation to reflect it.

The VM scope KVM_SET_TSC_KHZ ioctl is used to set up the default TSC
frequency that all subsequently created vCPUs can use.  It is only
intended to be called before any vCPU is created.  Allowing it to be
called after that only results in confusion but nothing good.

Note this is an ABI change.  But currently in Qemu (the de facto
userspace VMM) only TDX uses this VM ioctl, and it is only called once
before creating any vCPU, therefore the risk of breaking userspace is
pretty low.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 Documentation/virt/kvm/api.rst | 2 +-
 arch/x86/kvm/x86.c             | 9 ++++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 43ed57e048a8..e343430ccb01 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -2006,7 +2006,7 @@ frequency is KHz.
 
 If the KVM_CAP_VM_TSC_CONTROL capability is advertised, this can also
 be used as a vm ioctl to set the initial tsc frequency of subsequently
-created vCPUs.
+created vCPUs. The vm ioctl must be called before any vCPU is created.
 
 4.56 KVM_GET_TSC_KHZ
 --------------------
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2806f7104295..4051c0cacb92 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7199,9 +7199,12 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
 		if (user_tsc_khz == 0)
 			user_tsc_khz = tsc_khz;
 
-		WRITE_ONCE(kvm->arch.default_tsc_khz, user_tsc_khz);
-		r = 0;
-
+		mutex_lock(&kvm->lock);
+		if (!kvm->created_vcpus) {
+			WRITE_ONCE(kvm->arch.default_tsc_khz, user_tsc_khz);
+			r = 0;
+		}
+		mutex_unlock(&kvm->lock);
 		goto out;
 	}
 	case KVM_GET_TSC_KHZ: {
-- 
2.50.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] KVM: x86: Reject KVM_SET_TSC_KHZ vCPU ioctl for TSC protected guest
  2025-07-13 22:20 [PATCH v2 0/2] Improve KVM_SET_TSC_KHZ handling for CoCo VMs Kai Huang
  2025-07-13 22:20 ` [PATCH v2 1/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created Kai Huang
@ 2025-07-13 22:20 ` Kai Huang
  2025-07-15  0:23 ` [PATCH v2 0/2] Improve KVM_SET_TSC_KHZ handling for CoCo VMs Sean Christopherson
  2 siblings, 0 replies; 8+ messages in thread
From: Kai Huang @ 2025-07-13 22:20 UTC (permalink / raw)
  To: seanjc, pbonzini
  Cc: kvm, thomas.lendacky, nikunj, bp, isaku.yamahata, xiaoyao.li,
	rick.p.edgecombe, chao.gao, linux-kernel

Reject KVM_SET_TSC_KHZ vCPU ioctl if guest's TSC is protected and not
changeable by KVM, and update the documentation to reflect it.

For such TSC protected guests, e.g. TDX guests, typically the TSC is
configured once at VM level before any vCPU are created and remains
unchanged during VM's lifetime.  KVM provides the KVM_SET_TSC_KHZ VM
scope ioctl to allow the userspace VMM to configure the TSC of such VM.
After that the userspace VMM is not supposed to call the KVM_SET_TSC_KHZ
vCPU scope ioctl anymore when creating the vCPU.

The de facto userspace VMM Qemu does this for TDX guests.  The upcoming
SEV-SNP guests with Secure TSC should follow.

Note this could be a break of ABI.  But for now only TDX guests are TSC
protected and only Qemu supports TDX, thus in practice this should not
break any existing userspace.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
---
 Documentation/virt/kvm/api.rst | 7 +++++++
 arch/x86/kvm/x86.c             | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index e343430ccb01..563878465a6a 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -2008,6 +2008,13 @@ If the KVM_CAP_VM_TSC_CONTROL capability is advertised, this can also
 be used as a vm ioctl to set the initial tsc frequency of subsequently
 created vCPUs. The vm ioctl must be called before any vCPU is created.
 
+For TSC protected Confidential Computing (CoCo) VMs where TSC frequency
+is configured once at VM scope and remains unchanged during VM's
+lifetime, the vm ioctl should be used to configure the TSC frequency
+and the vcpu ioctl is not supported.
+
+Example of such CoCo VMs: TDX guests.
+
 4.56 KVM_GET_TSC_KHZ
 --------------------
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 4051c0cacb92..26737bc4decb 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6186,6 +6186,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		u32 user_tsc_khz;
 
 		r = -EINVAL;
+
+		if (vcpu->arch.guest_tsc_protected)
+			goto out;
+
 		user_tsc_khz = (u32)arg;
 
 		if (kvm_caps.has_tsc_control &&
-- 
2.50.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created
  2025-07-13 22:20 ` [PATCH v2 1/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created Kai Huang
@ 2025-07-14  5:43   ` Nikunj A. Dadhania
  2025-07-14  8:44   ` Chao Gao
  1 sibling, 0 replies; 8+ messages in thread
From: Nikunj A. Dadhania @ 2025-07-14  5:43 UTC (permalink / raw)
  To: Kai Huang, seanjc, pbonzini
  Cc: kvm, thomas.lendacky, bp, isaku.yamahata, xiaoyao.li,
	rick.p.edgecombe, chao.gao, linux-kernel



On 7/14/2025 3:50 AM, Kai Huang wrote:
> Reject the KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created and
> update the documentation to reflect it.
> 
> The VM scope KVM_SET_TSC_KHZ ioctl is used to set up the default TSC
> frequency that all subsequently created vCPUs can use.  It is only
> intended to be called before any vCPU is created.  Allowing it to be
> called after that only results in confusion but nothing good.
> 
> Note this is an ABI change.  But currently in Qemu (the de facto
> userspace VMM) only TDX uses this VM ioctl, and it is only called once
> before creating any vCPU, therefore the risk of breaking userspace is
> pretty low.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

LTGM:

Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>

> ---
>  Documentation/virt/kvm/api.rst | 2 +-
>  arch/x86/kvm/x86.c             | 9 ++++++---
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 43ed57e048a8..e343430ccb01 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -2006,7 +2006,7 @@ frequency is KHz.
>  
>  If the KVM_CAP_VM_TSC_CONTROL capability is advertised, this can also
>  be used as a vm ioctl to set the initial tsc frequency of subsequently
> -created vCPUs.
> +created vCPUs. The vm ioctl must be called before any vCPU is created.
>  
>  4.56 KVM_GET_TSC_KHZ
>  --------------------
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2806f7104295..4051c0cacb92 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7199,9 +7199,12 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>  		if (user_tsc_khz == 0)
>  			user_tsc_khz = tsc_khz;
>  
> -		WRITE_ONCE(kvm->arch.default_tsc_khz, user_tsc_khz);
> -		r = 0;
> -
> +		mutex_lock(&kvm->lock);
> +		if (!kvm->created_vcpus) {
> +			WRITE_ONCE(kvm->arch.default_tsc_khz, user_tsc_khz);
> +			r = 0;
> +		}
> +		mutex_unlock(&kvm->lock);
>  		goto out;
>  	}
>  	case KVM_GET_TSC_KHZ: {


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created
  2025-07-13 22:20 ` [PATCH v2 1/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created Kai Huang
  2025-07-14  5:43   ` Nikunj A. Dadhania
@ 2025-07-14  8:44   ` Chao Gao
  1 sibling, 0 replies; 8+ messages in thread
From: Chao Gao @ 2025-07-14  8:44 UTC (permalink / raw)
  To: Kai Huang
  Cc: seanjc, pbonzini, kvm, thomas.lendacky, nikunj, bp,
	isaku.yamahata, xiaoyao.li, rick.p.edgecombe, linux-kernel

On Mon, Jul 14, 2025 at 10:20:19AM +1200, Kai Huang wrote:
>Reject the KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created and
>update the documentation to reflect it.
>
>The VM scope KVM_SET_TSC_KHZ ioctl is used to set up the default TSC
>frequency that all subsequently created vCPUs can use.  It is only
>intended to be called before any vCPU is created.  Allowing it to be
>called after that only results in confusion but nothing good.
>
>Note this is an ABI change.  But currently in Qemu (the de facto
>userspace VMM) only TDX uses this VM ioctl, and it is only called once
>before creating any vCPU, therefore the risk of breaking userspace is
>pretty low.
>
>Suggested-by: Sean Christopherson <seanjc@google.com>
>Signed-off-by: Kai Huang <kai.huang@intel.com>
>Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/2] Improve KVM_SET_TSC_KHZ handling for CoCo VMs
  2025-07-13 22:20 [PATCH v2 0/2] Improve KVM_SET_TSC_KHZ handling for CoCo VMs Kai Huang
  2025-07-13 22:20 ` [PATCH v2 1/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created Kai Huang
  2025-07-13 22:20 ` [PATCH v2 2/2] KVM: x86: Reject KVM_SET_TSC_KHZ vCPU ioctl for TSC protected guest Kai Huang
@ 2025-07-15  0:23 ` Sean Christopherson
  2025-07-15  0:54   ` Huang, Kai
  2 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2025-07-15  0:23 UTC (permalink / raw)
  To: Sean Christopherson, pbonzini, Kai Huang
  Cc: kvm, thomas.lendacky, nikunj, bp, isaku.yamahata, xiaoyao.li,
	rick.p.edgecombe, chao.gao, linux-kernel

On Mon, 14 Jul 2025 10:20:18 +1200, Kai Huang wrote:
> This series follows Sean's suggestions [1][2] to:
> 
>  - Reject vCPU scope KVM_SET_TSC_KHZ ioctl for TSC protected vCPU
>  - Reject VM scope KVM_SET_TSC_KHZ ioctl when vCPUs have been created
> 
> .. in the discussion of SEV-SNP Secure TSC support series.
> 
> [...]

Applied patch 2 to kvm-x86 fixes, with a tweaked changelog to call out that
TDX support hasn't yet been released, i.e. that there is no established ABI
to break.

Applied patch 1 to kvm-x86 misc, with tweaked documentation to not imply that
userspace "must" invoke the ioctl.  I think this is the last patch I'll throw
into misc for 6.17?  So in theory, if it breaks userspace, I can simply
truncate it from the pull request.

[1/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created
      https://github.com/kvm-x86/linux/commit/dcbe5a466c12
[2/2] KVM: x86: Reject KVM_SET_TSC_KHZ vCPU ioctl for TSC protected guest
      https://github.com/kvm-x86/linux/commit/e51cf184d90c

--
https://github.com/kvm-x86/linux/tree/next

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/2] Improve KVM_SET_TSC_KHZ handling for CoCo VMs
  2025-07-15  0:23 ` [PATCH v2 0/2] Improve KVM_SET_TSC_KHZ handling for CoCo VMs Sean Christopherson
@ 2025-07-15  0:54   ` Huang, Kai
  2025-07-15 14:07     ` Sean Christopherson
  0 siblings, 1 reply; 8+ messages in thread
From: Huang, Kai @ 2025-07-15  0:54 UTC (permalink / raw)
  To: pbonzini@redhat.com, seanjc@google.com
  Cc: Gao, Chao, Edgecombe, Rick P, bp@alien8.de, Li, Xiaoyao,
	nikunj@amd.com, thomas.lendacky@amd.com, kvm@vger.kernel.org,
	Yamahata, Isaku, linux-kernel@vger.kernel.org

On Mon, 2025-07-14 at 17:23 -0700, Sean Christopherson wrote:
> On Mon, 14 Jul 2025 10:20:18 +1200, Kai Huang wrote:
> > This series follows Sean's suggestions [1][2] to:
> > 
> >  - Reject vCPU scope KVM_SET_TSC_KHZ ioctl for TSC protected vCPU
> >  - Reject VM scope KVM_SET_TSC_KHZ ioctl when vCPUs have been created
> > 
> > .. in the discussion of SEV-SNP Secure TSC support series.
> > 
> > [...]
> 
> Applied patch 2 to kvm-x86 fixes, with a tweaked changelog to call out that
> TDX support hasn't yet been released, i.e. that there is no established ABI
> to break.
> 
> Applied patch 1 to kvm-x86 misc, with tweaked documentation to not imply that
> userspace "must" invoke the ioctl.  I think this is the last patch I'll throw
> into misc for 6.17?  So in theory, if it breaks userspace, I can simply
> truncate it from the pull request.

Thanks!

> 
> [1/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created
>       https://github.com/kvm-x86/linux/commit/dcbe5a466c12
> [2/2] KVM: x86: Reject KVM_SET_TSC_KHZ vCPU ioctl for TSC protected guest
>       https://github.com/kvm-x86/linux/commit/e51cf184d90c

Btw, in the second patch it seems you have:

  Fixes; adafea1 ("KVM: x86: Add infrastructure for secure TSC")

Shouldn't we follow the standard format, i.e.,

  Fixes: adafea110600 ("KVM: x86: Add infrastructure for secure TSC")


?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/2] Improve KVM_SET_TSC_KHZ handling for CoCo VMs
  2025-07-15  0:54   ` Huang, Kai
@ 2025-07-15 14:07     ` Sean Christopherson
  0 siblings, 0 replies; 8+ messages in thread
From: Sean Christopherson @ 2025-07-15 14:07 UTC (permalink / raw)
  To: Kai Huang
  Cc: pbonzini@redhat.com, Chao Gao, Rick P Edgecombe, bp@alien8.de,
	Xiaoyao Li, nikunj@amd.com, thomas.lendacky@amd.com,
	kvm@vger.kernel.org, Isaku Yamahata, linux-kernel@vger.kernel.org

On Tue, Jul 15, 2025, Kai Huang wrote:
> On Mon, 2025-07-14 at 17:23 -0700, Sean Christopherson wrote:
> > On Mon, 14 Jul 2025 10:20:18 +1200, Kai Huang wrote:
> > > This series follows Sean's suggestions [1][2] to:
> > > 
> > >  - Reject vCPU scope KVM_SET_TSC_KHZ ioctl for TSC protected vCPU
> > >  - Reject VM scope KVM_SET_TSC_KHZ ioctl when vCPUs have been created
> > > 
> > > .. in the discussion of SEV-SNP Secure TSC support series.
> > > 
> > > [...]
> > 
> > Applied patch 2 to kvm-x86 fixes, with a tweaked changelog to call out that
> > TDX support hasn't yet been released, i.e. that there is no established ABI
> > to break.
> > 
> > Applied patch 1 to kvm-x86 misc, with tweaked documentation to not imply that
> > userspace "must" invoke the ioctl.  I think this is the last patch I'll throw
> > into misc for 6.17?  So in theory, if it breaks userspace, I can simply
> > truncate it from the pull request.
> 
> Thanks!
> 
> > 
> > [1/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created
> >       https://github.com/kvm-x86/linux/commit/dcbe5a466c12
> > [2/2] KVM: x86: Reject KVM_SET_TSC_KHZ vCPU ioctl for TSC protected guest
> >       https://github.com/kvm-x86/linux/commit/e51cf184d90c
> 
> Btw, in the second patch it seems you have:
> 
>   Fixes; adafea1 ("KVM: x86: Add infrastructure for secure TSC")
> 
> Shouldn't we follow the standard format, i.e.,
> 
>   Fixes: adafea110600 ("KVM: x86: Add infrastructure for secure TSC")

Ugh, yes, the semi-colon is just a typo.  New hash:

  https://github.com/kvm-x86/linux/commit/b24bbb534c2d

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-07-15 14:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-13 22:20 [PATCH v2 0/2] Improve KVM_SET_TSC_KHZ handling for CoCo VMs Kai Huang
2025-07-13 22:20 ` [PATCH v2 1/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPUs have been created Kai Huang
2025-07-14  5:43   ` Nikunj A. Dadhania
2025-07-14  8:44   ` Chao Gao
2025-07-13 22:20 ` [PATCH v2 2/2] KVM: x86: Reject KVM_SET_TSC_KHZ vCPU ioctl for TSC protected guest Kai Huang
2025-07-15  0:23 ` [PATCH v2 0/2] Improve KVM_SET_TSC_KHZ handling for CoCo VMs Sean Christopherson
2025-07-15  0:54   ` Huang, Kai
2025-07-15 14:07     ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).