Re: [PATCH v2 1/3] KVM: x86: Make the hardcoded APIC bus frequency vm variable

[Xiaoyao Li]

On 12/19/2023 11:53 AM, Jim Mattson wrote:
> On Mon, Dec 18, 2023 at 5:40 PM Isaku Yamahata
> <isaku.yamahata@xxxxxxxxxxxxxxx> wrote:
> > On Thu, Dec 14, 2023 at 08:41:43AM -0800,
> > Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> >
> > > On Thu, Dec 14, 2023, Maxim Levitsky wrote:
> > > > On Wed, 2023-12-13 at 15:10 -0800, Sean Christopherson wrote:
> > > > > Upstream KVM's non-TDX behavior is fine, because KVM doesn't advertise support
> > > > > for CPUID 0x15, i.e. doesn't announce to host userspace that it's safe to expose
> > > > > CPUID 0x15 to the guest.  Because TDX makes exposing CPUID 0x15 mandatory, KVM
> > > > > needs to be taught to correctly emulate the guest's APIC bus frequency, a.k.a.
> > > > > the TDX guest core crystal frequency of 25Mhz.
> > > >
> > > > I assume that TDX doesn't allow to change the CPUID 0x15 leaf.
> > >
> > > Correct.  I meant to call that out below, but left my sentence half-finished.  It
> > > was supposed to say:
> > >
> > >    I halfheartedly floated the idea of "fixing" the TDX module/architecture to either
> > >    use 1Ghz as the base frequency or to allow configuring the base frequency
> > >    advertised to the guest.
> > >
> > > > > I halfheartedly floated the idea of "fixing" the TDX module/architecture to either
> > > > > use 1Ghz as the base frequency (off list), but it definitely isn't a hill worth
> > > > > dying on since the KVM changes are relatively simple.
> > > > >
> > > > > https://lore.kernel.org/all/ZSnIKQ4bUavAtBz6@xxxxxxxxxx
> > > >
> > > > Best regards,
> > > >      Maxim Levitsky
> >
> > The followings are the updated version of the commit message.
> >
> >
> > KVM: x86: Make the hardcoded APIC bus frequency VM variable
> >
> > The TDX architecture hard-codes the APIC bus frequency to 25MHz in the
> > CPUID leaf 0x15.  The
> > TDX mandates it to be exposed and doesn't allow the VMM to override
> > its value.  The KVM APIC timer emulation hard-codes the frequency to
> > 1GHz.  It doesn't unconditionally enumerate it to the guest unless the
> > user space VMM sets the CPUID leaf 0x15 by KVM_SET_CPUID.
> >
> > If the CPUID leaf 0x15 is enumerated, the guest kernel uses it as the
> > APIC bus frequency.  If not, the guest kernel measures the frequency
> > based on other known timers like the ACPI timer or the legacy PIT.
> > The TDX guest kernel gets timer interrupt more times by 1GHz / 25MHz.
> >
> > To ensure that the guest doesn't have a conflicting view of the APIC
> > bus frequency, allow the userspace to tell KVM to use the same
> > frequency that TDX mandates instead of the default 1Ghz.
> >
> > 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.
> > 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.

This will have an impact on TSC frequency. Core crystal clock frequency 
is also used to calculate TSC frequency.

> 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.

I'm wondering what's the attitude of KVM community to provide support 
CPUID leaf 0x15? Even KVM decides to never advertise CPUID 0x15 in 
GET_SUPPORTED_CPUID, hard-coded APIC frequency puts additional 
limitation when userspace want to emualte CPUID 0x15