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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <877c6p8t35.fsf@redhat.com>
Date: Mon, 20 Jan 2025 15:20:46 +0100
From: Vitaly Kuznetsov <vkuznets@...hat.com>
To: Sean Christopherson <seanjc@...gle.com>, Paolo Bonzini
 <pbonzini@...hat.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org, Dongjie Zou
 <zoudongjie@...wei.com>
Subject: Re: [PATCH v2 4/4] KVM: selftests: Add CPUID tests for Hyper-V
 features that need in-kernel APIC

Sean Christopherson <seanjc@...gle.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@...hat.com>
> Signed-off-by: Sean Christopherson <seanjc@...gle.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@...hat.com>

-- 
Vitaly