linux-kernel - Re: [PATCH v2 08/21] KVM: selftests: Split PMU caps sub-tests to avoid writing MSR after KVM

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: <85b3d348-2e4a-43aa-0131-27e9fc375cf9@gmail.com>
Date:   Tue, 14 Feb 2023 15:27:30 +0800
From:   Like Xu <like.xu.linux@...il.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Xiaoyao Li <xiaoyao.li@...el.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH v2 08/21] KVM: selftests: Split PMU caps sub-tests to
 avoid writing MSR after KVM_RUN

Nit, could this patch be applied before the relevant KVM modifications [1] so that
a CI bot doesn't generate an error report before this selftests patch is applied ?

[1] KVM: x86: Disallow writes to immutable feature MSRs after KVM_RUN

On 10/2/2023 8:31 am, Sean Christopherson wrote:
> Split the PERF_CAPABILITIES subtests into two parts so that the LBR format
> testcases don't execute after KVM_RUN.  Now that KVM disallows changing
> PERF_CAPABILITIES after KVM_RUN (same as guest CPUID), attempting to set
> the MSR after KVM_RUN will yield false positives and/or false negatives
> depending on what the test is trying to do.
> 
> Land the LBR format test in a more generic "immutable features" test in
> anticipation of expanding its scope to other immutable features.
> 
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
>   .../selftests/kvm/x86_64/vmx_pmu_caps_test.c  | 51 +++++++++++--------
>   1 file changed, 31 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c b/tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c
> index c280ba1e6572..ac08c0fdd84d 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c
> @@ -41,24 +41,10 @@ static void guest_code(void)
>   	wrmsr(MSR_IA32_PERF_CAPABILITIES, PMU_CAP_LBR_FMT);
>   }
>   
> -int main(int argc, char *argv[])
> +static void test_fungible_perf_capabilities(union perf_capabilities host_cap)
>   {
> -	struct kvm_vm *vm;
>   	struct kvm_vcpu *vcpu;
> -	int ret;
> -	union perf_capabilities host_cap;
> -	uint64_t val;
> -
> -	host_cap.capabilities = kvm_get_feature_msr(MSR_IA32_PERF_CAPABILITIES);
> -	host_cap.capabilities &= (PMU_CAP_FW_WRITES | PMU_CAP_LBR_FMT);
> -
> -	/* Create VM */
> -	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
> -
> -	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_PDCM));
> -
> -	TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
> -	TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
> +	struct kvm_vm *vm = vm_create_with_one_vcpu(&vcpu, guest_code);
>   
>   	/* testcase 1, set capabilities when we have PDCM bit */
>   	vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, PMU_CAP_FW_WRITES);
> @@ -70,7 +56,16 @@ int main(int argc, char *argv[])
>   	vcpu_run(vcpu);
>   	ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), PMU_CAP_FW_WRITES);
>   
> -	/* testcase 2, check valid LBR formats are accepted */
> +	kvm_vm_free(vm);
> +}
> +
> +static void test_immutable_perf_capabilities(union perf_capabilities host_cap)
> +{
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm = vm_create_with_one_vcpu(&vcpu, NULL);
> +	uint64_t val;
> +	int ret;
> +
>   	vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, 0);
>   	ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), 0);
>   
> @@ -78,8 +73,8 @@ int main(int argc, char *argv[])
>   	ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), (u64)host_cap.lbr_format);
>   
>   	/*
> -	 * Testcase 3, check that an "invalid" LBR format is rejected.  Only an
> -	 * exact match of the host's format (and 0/disabled) is allowed.
> +	 * KVM only supports the host's native LBR format, as well as '0' (to
> +	 * disable LBR support).  Verify KVM rejects all other LBR formats.
>   	 */
>   	for (val = 1; val <= PMU_CAP_LBR_FMT; val++) {
>   		if (val == (host_cap.capabilities & PMU_CAP_LBR_FMT))
> @@ -88,7 +83,23 @@ int main(int argc, char *argv[])
>   		ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
>   		TEST_ASSERT(!ret, "Bad LBR FMT = 0x%lx didn't fail", val);
>   	}
> +	kvm_vm_free(vm);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	union perf_capabilities host_cap;
> +
> +	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_PDCM));
> +
> +	TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
> +	TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
> +
> +	host_cap.capabilities = kvm_get_feature_msr(MSR_IA32_PERF_CAPABILITIES);
> +	host_cap.capabilities &= (PMU_CAP_FW_WRITES | PMU_CAP_LBR_FMT);
> +
> +	test_fungible_perf_capabilities(host_cap);
> +	test_immutable_perf_capabilities(host_cap);
>   
>   	printf("Completed perf capability tests.\n");
> -	kvm_vm_free(vm);
>   }