Re: [PATCH v2 4/4] KVM: selftests: Add CPUID tests for Hyper-V features that need in-kernel APIC

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Sean Christopherson <seanjc@google.com> writes:

> Add testcases to x86's Hyper-V CPUID test to verify that KVM advertises
> support for features that require an in-kernel local APIC appropriately,
> i.e. that KVM hides support from the vCPU-scoped ioctl if the VM doesn't
> have an in-kernel local APIC.
>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> index 3188749ec6e1..8f26130dc30d 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> @@ -43,6 +43,7 @@ static bool smt_possible(void)
>  
>  static void test_hv_cpuid(struct kvm_vcpu *vcpu, bool evmcs_expected)
>  {
> +	const bool has_irqchip = !vcpu || vcpu->vm->has_irqchip;
>  	const struct kvm_cpuid2 *hv_cpuid_entries;
>  	int i;
>  	int nent_expected = 10;
> @@ -85,12 +86,19 @@ static void test_hv_cpuid(struct kvm_vcpu *vcpu, bool evmcs_expected)
>  				    entry->eax, evmcs_expected
>  				);
>  			break;
> +		case 0x40000003:
> +			TEST_ASSERT(has_irqchip || !(entry->edx & BIT(19)),
> +				    "Synthetic Timers should require in-kernel APIC");

Nitpick: BIT(19) of CPUID.0x40000003(EDX) advertises 'direct' mode
for Synthetic timers and that's what we have paired with
lapic_in_kernel() check. Thus, we may want to be a bit more specific and
say

"Direct Synthetic timers should require in-kernel APIC"
(personally, I'd prefer "Synthetic timers in 'direct' mode" name but
that's not how TLFS calls them)

or something similar. 

(feel free to address this small rant of mine upon commit or just ignore)

> +			break;
>  		case 0x40000004:
>  			test_val = entry->eax & (1UL << 18);
>  
>  			TEST_ASSERT(!!test_val == !smt_possible(),
>  				    "NoNonArchitecturalCoreSharing bit"
>  				    " doesn't reflect SMT setting");
> +
> +			TEST_ASSERT(has_irqchip || !(entry->eax & BIT(10)),
> +				    "Cluster IPI (i.e. SEND_IPI) should require in-kernel APIC");
>  			break;
>  		case 0x4000000A:
>  			TEST_ASSERT(entry->eax & (1UL << 19),
> @@ -145,9 +153,14 @@ int main(int argc, char *argv[])
>  
>  	TEST_REQUIRE(kvm_has_cap(KVM_CAP_HYPERV_CPUID));
>  
> -	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
> +	/* Test the vCPU ioctl without an in-kernel local APIC. */
> +	vm = vm_create_barebones();
> +	vcpu = __vm_vcpu_add(vm, 0);
> +	test_hv_cpuid(vcpu, false);
> +	kvm_vm_free(vm);
>  
>  	/* Test vCPU ioctl version */
> +	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
>  	test_hv_cpuid_e2big(vm, vcpu);
>  	test_hv_cpuid(vcpu, false);

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>

-- 
Vitaly