Re: [PATCH] Documentation: KVM: Describe guest CPUID.15H settings for in-kernel APIC

[Sean Christopherson <seanjc@xxxxxxxxxx>]

On Wed, Sep 21, 2022, Jim Mattson wrote:
> KVM_GET_SUPPORTED_CPUID must not populate guest CPUID.15H, because KVM
> has no way of knowing the base frequency of the local APIC emulated in
> userspace.
> 
> However, in reality, the in-kernel APIC emulation is in prevalent
> use. Document how KVM_GET_SUPPORTED_CPUID would populate CPUID.15H if
> the in-kernel APIC were the default.
> 
> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> ---
>  Documentation/virt/kvm/api.rst | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index abd7c32126ce..1e09ac9d48e9 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -1786,6 +1786,16 @@ support.  Instead it is reported via::
>  if that returns true and you use KVM_CREATE_IRQCHIP, or if you emulate the
>  feature in userspace, then you can enable the feature for KVM_SET_CPUID2.
>  
> +Similarly, CPUID leaf 0x15 always returns zeroes, because the core
> +crystal clock frequency must match the local APIC base frequency, and
> +the default configuration leaves the local APIC emulation to
> +userspace.
> +
> +If KVM_CREATE_IRQCHIP is used to enable the in-kernel local APIC
> +emulation, CPUID.15H:ECX can be set to 1000000000 (0x3b9aca00). For
> +the default guest TSC frequency, CPUID.15H:EBX can be set to tsc_khz
> +and CPUID.15H:ECX can be set to 1000000 (0xf4240).  The fraction can
> +be simplified if desired.

It would be helpful to explain what these numbers mean and where they come from.

That said, unlike 16H, I think we can actually "solve" this case, we just need a
way to force an in-kernel local APIC, i.e. add a module param or Kconfig.

I'm tempted to make it a Kconfig so that KVM can start moving toward removing
"support" for a userspace local APIC entirely, e.g. x2APIC is basically unusable
without an in-kernel APIC, so KVM is already quite far down that path anyways.

E.g.

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index ffdc28684cb7..b67135a53edf 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -1061,6 +1061,11 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
                                goto out;
                }
                break;
+#ifdef CONFIG_KVM_REQUIRE_IN_KERNEL_LOCAL_APIC
+       case 0x15:
+               <fill in magic values>
+               break;
+#endif
        /* Intel AMX TILE */
        case 0x1d:
                if (!kvm_cpu_cap_has(X86_FEATURE_AMX_TILE)) {
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index a5ac4a5a5179..5abaa09ec5aa 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -181,8 +181,10 @@ DECLARE_STATIC_KEY_FALSE(kvm_has_noapic_vcpu);
 
 static inline bool lapic_in_kernel(struct kvm_vcpu *vcpu)
 {
+#ifndef CONFIG_KVM_REQUIRE_IN_KERNEL_LOCAL_APIC
        if (static_branch_unlikely(&kvm_has_noapic_vcpu))
                return vcpu->arch.apic;
+#endif
        return true;
 }
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index eb412f9f03ea..8ea6f0175c1c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11763,6 +11763,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
        struct page *page;
        int r;
 
+       if (IS_ENABLED(CONFIG_KVM_REQUIRE_IN_KERNEL_LOCAL_APIC) &&
+           !irqchip_in_kernel(vcpu->kvm))
+               return -EINVAL;
+
        vcpu->arch.last_vmentry_cpu = -1;
        vcpu->arch.regs_avail = ~0;
        vcpu->arch.regs_dirty = ~0;