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

[Maxim Levitsky]

On Mon, 2023-12-18 at 17:40 -0800, Isaku Yamahata 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.

Looks great!

In theory this gives me an idea that KVM could parse the guest CPUID leaf
0x15 and deduce the frequency from it automatically instead of a new capability,
but I understand that this is (also in theory) not backward compatible assuming
that some hypervisors already expose this leaf for some reason,
thus a new capability will be needed anyway.

Thus I have no more complaints, and thanks for addressing my feedback!

Best regards,
	Maxim Levitsky

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