public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Isaku Yamahata <isaku.yamahata@linux.intel.com>
Cc: Jim Mattson <jmattson@google.com>,
	Maxim Levitsky <mlevitsk@redhat.com>,
	isaku.yamahata@intel.com,  kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, isaku.yamahata@gmail.com,
	 Paolo Bonzini <pbonzini@redhat.com>,
	erdemaktas@google.com,  Vishal Annapurve <vannapurve@google.com>
Subject: Re: [PATCH v2 1/3] KVM: x86: Make the hardcoded APIC bus frequency vm variable
Date: Wed, 20 Dec 2023 14:07:00 -0800	[thread overview]
Message-ID: <ZYNlhKCcOHgjTcFZ@google.com> (raw)
In-Reply-To: <20231219081104.GB2639779@ls.amr.corp.intel.com>

On Tue, Dec 19, 2023, Isaku Yamahata wrote:
> On Mon, Dec 18, 2023 at 07:53:45PM -0800, Jim Mattson <jmattson@google.com> wrote:
> > > There are several options to address this.
> > > 1. Make the KVM able to configure APIC bus frequency (This patch).
> > >    Pros: It resembles the existing hardware.  The recent Intel CPUs
> > >    adapts 25MHz.
> > >    Cons: Require the VMM to emulate the APIC timer at 25MHz.
> > > 2. Make the TDX architecture enumerate CPUID 0x15 to configurable
> > >    frequency or not enumerate it.
> > >    Pros: Any APIC bus frequency is allowed.
> > >    Cons: Deviation from the real hardware.

I don't buy this as a valid Con.  TDX is one gigantic deviation from real hardware,
and since TDX obviously can't guarantee the APIC timer is emulated at the correct
frequency, there can't possibly be any security benefits.  If this were truly a
Con that anyone cared about, we would have gotten patches to "fix" KVM a long time
ago.

If the TDX module wasn't effectively hardware-defined software, i.e. was actually
able to adapt at the speed of software, then fixing this in TDX would be a complete
no-brainer.

The KVM uAPI required to play nice is relatively minor, so I'm not totally opposed
to adding it.  But I totally agree with Jim that forcing KVM to change 13+ years
of behavior just because someone at Intel decided that 25MHz was a good number is
ridiculous.

> > > 3. Make the TDX guest kernel use 1GHz when it's running on KVM.
> > >    Cons: The kernel ignores CPUID leaf 0x15.
> > 
> > 4. Change CPUID.15H under TDX to report the crystal clock frequency as 1 GHz.
> > Pro: This has been the virtual APIC frequency for KVM guests for 13 years.
> > Pro: This requires changing only one hard-coded constant in TDX.
> > 
> > I see no compelling reason to complicate KVM with support for
> > configurable APIC frequencies, and I see no advantages to doing so.
> 
> Because TDX isn't specific to KVM, it should work with other VMM technologies.
> If we'd like to go for this route, the frequency would be configurable.  What
> frequency should be acceptable securely is obscure.  25MHz has long history with
> the real hardware.

  reply	other threads:[~2023-12-20 22:07 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-14  4:35 [PATCH v2 0/3] KVM: X86: Make bus clock frequency for vapic timer configurable isaku.yamahata
2023-11-14  4:35 ` [PATCH v2 1/3] KVM: x86: Make the hardcoded APIC bus frequency vm variable isaku.yamahata
2023-12-13 22:39   ` Maxim Levitsky
2023-12-13 23:10     ` Sean Christopherson
2023-12-13 23:18       ` Jim Mattson
2023-12-14  9:31       ` Maxim Levitsky
2023-12-14 16:41         ` Sean Christopherson
2023-12-19  1:40           ` Isaku Yamahata
2023-12-19  3:53             ` Jim Mattson
2023-12-19  7:56               ` Xiaoyao Li
2023-12-19  8:11               ` Isaku Yamahata
2023-12-20 22:07                 ` Sean Christopherson [this message]
2023-12-20 22:22                   ` Jim Mattson
2023-12-21  5:44                   ` Xiaoyao Li
2023-12-21 14:39                     ` Jim Mattson
2023-12-21 17:01             ` Maxim Levitsky
2023-11-14  4:35 ` [PATCH v2 2/3] KVM: X86: Add a capability to configure bus frequency for APIC timer isaku.yamahata
2023-12-13 22:40   ` Maxim Levitsky
2023-11-14  4:35 ` [PATCH v2 3/3] KVM: selftests: Add test case for x86 apic_bus_clock_frequency isaku.yamahata
2023-12-13 22:41   ` Maxim Levitsky

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=ZYNlhKCcOHgjTcFZ@google.com \
    --to=seanjc@google.com \
    --cc=erdemaktas@google.com \
    --cc=isaku.yamahata@gmail.com \
    --cc=isaku.yamahata@intel.com \
    --cc=isaku.yamahata@linux.intel.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mlevitsk@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=vannapurve@google.com \
    /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