Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: "Huang, Kai" <kai.huang@intel.com>
To: "pbonzini@redhat.com" <pbonzini@redhat.com>,
	"Miao, Jun" <jun.miao@intel.com>,
	"seanjc@google.com" <seanjc@google.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"tglx@kernel.org" <tglx@kernel.org>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] KVM: x86: Block TSC multiplier writes for protected guest TSC
Date: Tue, 12 May 2026 01:12:36 +0000	[thread overview]
Message-ID: <33cc37f03fe011180b90c66787d5ee5e7456471a.camel@intel.com> (raw)
In-Reply-To: <20260512111830.1295437-1-jun.miao@intel.com>

On Tue, 2026-05-12 at 07:18 -0400, Jun Miao wrote:
> Commit d5a4e408e69b ("KVM: x86: Add infrastructure for secure TSC")

The hash isn't correct.  It should be adafea110600 AFAICT.

> added the guest_tsc_protected flag to prevent KVM from changing the
> TSC offset/multiplier of vCPUs whose TSC is managed by a confidential
> computing module (e.g. TDX, SEV-SNP Secure TSC).  However only the TSC
> offset write path was guarded; kvm_vcpu_write_tsc_multiplier() was
> left unprotected.
> 
> As a result, userspace can still change the TSC scaling ratio of a
> TDX vCPU via the KVM_SET_TSC_KHZ ioctl path:
> 
>   KVM_SET_TSC_KHZ
>     -> kvm_arch_vcpu_ioctl()
>     -> kvm_set_tsc_khz()
>     -> set_tsc_khz()
>     -> kvm_vcpu_write_tsc_multiplier()   <-- not guarded

Commit b24bbb534c2d ("KVM: x86: Reject KVM_SET_TSC_KHZ vCPU ioctl for TSC
protected guest") already added code to bail out early in kvm_arch_vcpu_ioctl()
for TSC protected vCPU.

> 
> and similarly during kvm_arch_vcpu_create() -> kvm_set_tsc_khz()
> which can reset the multiplier to default_tsc_scaling_ratio.

This is more like initialization of KVM's internal vCPU data structure based on
the default TSC (which is configured to the TD before creating any vCPU [*]), so
I think it is OK from functionality perspective.  Note vt_write_tsc_multiplier()
simply does nothing for TD so it's safe.  But since it is called after vcpu-
>arch.guest_tsc_protected is set, I agree it's kinda not nice.

[*] TDX's flow is: 1) userspace uses KVM_SET_TSC_KHZ vm-ioctl to configure the
default TSC; 2) userspace invokes KVM_TDX_INIT_VM, which configures the TD's TSC
using the default TSC; 3) userspace creates vCPUs for the TD, in which step vcpu
is marked as TSC protected.

After 3), both KVM_SET_TSC_KHZ vm-ioctl and vcpu-ioctl will fail to change the
TSC.

Theoretically, AFAICT there's a chance that if 1) gets called again with a
different TSC "after 2), but before 3)", KVM will have a wrong default TSC from
what is configured the the TD.  But no sane userspace should ever do that.

> 
> Make kvm_vcpu_write_tsc_multiplier() symmetric with
> kvm_vcpu_write_tsc_offset() by skipping the update when
> guest_tsc_protected is set.  This single chokepoint covers all
> existing callers (set_tsc_khz() in both ioctl and vCPU create paths).

Fine with making kvm_vcpu_write_tsc_multiplier() symmetric to TSC offset,
though.

> 
> Fixes: d5a4e408e69b ("KVM: x86: Add infrastructure for secure TSC")
> Signed-off-by: Jun Miao <jun.miao@intel.com>
> ---
>  arch/x86/kvm/x86.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0a1b63c63d1a..e935fe33d9c2 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2736,6 +2736,9 @@ static void kvm_vcpu_write_tsc_offset(struct kvm_vcpu *vcpu, u64 l1_offset)
>  
>  static void kvm_vcpu_write_tsc_multiplier(struct kvm_vcpu *vcpu, u64 l1_multiplier)
>  {
> +	if (vcpu->arch.guest_tsc_protected)
> +		return;
> +
>  	vcpu->arch.l1_tsc_scaling_ratio = l1_multiplier;
>  
>  	/* Userspace is changing the multiplier while L2 is active */

  reply	other threads:[~2026-05-12  1:12 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-12 11:18 [PATCH] KVM: x86: Block TSC multiplier writes for protected guest TSC Jun Miao
2026-05-12  1:12 ` Huang, Kai [this message]
2026-05-12  1:29   ` Miao, Jun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=33cc37f03fe011180b90c66787d5ee5e7456471a.camel@intel.com \
    --to=kai.huang@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=jun.miao@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=tglx@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox