* [PATCH] selftests: kvm/x86: test if it checks all the bits in the LBR_FMT bit-field
@ 2022-08-04 7:38 Like Xu
2022-08-04 23:26 ` Sean Christopherson
0 siblings, 1 reply; 3+ messages in thread
From: Like Xu @ 2022-08-04 7:38 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel
From: Like Xu <likexu@tencent.com>
User space only enable guest LBR feature when the exactly supported
LBR format value is initialized to the MSR_IA32_PERF_CAPABILITIES.
The input is also invalid if only partially supported bits are set.
Note for PEBS feature, the PEBS_FORMAT bit field is the primary concern,
thus if the PEBS_FORMAT input is empty, the other bits check about PEBS
(like PEBS_TRAP or ARCH_REG) will be ignored.
Signed-off-by: Like Xu <likexu@tencent.com>
---
tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
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 6ec901dab61e..98483947f921 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
@@ -13,6 +13,7 @@
#define _GNU_SOURCE /* for program_invocation_short_name */
#include <sys/ioctl.h>
+#include <linux/bitmap.h>
#include "kvm_util.h"
#include "vmx.h"
@@ -56,7 +57,7 @@ int main(int argc, char *argv[])
const struct kvm_cpuid_entry2 *entry_a_0;
struct kvm_vm *vm;
struct kvm_vcpu *vcpu;
- int ret;
+ int ret, bit;
union cpuid10_eax eax;
union perf_capabilities host_cap;
@@ -97,6 +98,12 @@ int main(int argc, char *argv[])
ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, 0x30);
TEST_ASSERT(ret == 0, "Bad PERF_CAPABILITIES didn't fail.");
+ /* testcase 4, reject LBR_FMT if only partially supported bits are set */
+ for_each_set_bit(bit, (unsigned long *)&host_cap.capabilities, 6) {
+ ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, BIT_ULL(bit));
+ TEST_ASSERT(ret == 0, "Bad PERF_CAPABILITIES didn't fail.");
+ }
+
printf("Completed perf capability tests.\n");
kvm_vm_free(vm);
}
--
2.37.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] selftests: kvm/x86: test if it checks all the bits in the LBR_FMT bit-field
2022-08-04 7:38 [PATCH] selftests: kvm/x86: test if it checks all the bits in the LBR_FMT bit-field Like Xu
@ 2022-08-04 23:26 ` Sean Christopherson
2022-08-05 6:10 ` Like Xu
0 siblings, 1 reply; 3+ messages in thread
From: Sean Christopherson @ 2022-08-04 23:26 UTC (permalink / raw)
To: Like Xu; +Cc: Paolo Bonzini, kvm, linux-kernel
On Thu, Aug 04, 2022, Like Xu wrote:
> From: Like Xu <likexu@tencent.com>
>
> User space only enable guest LBR feature when the exactly supported
> LBR format value is initialized to the MSR_IA32_PERF_CAPABILITIES.
> The input is also invalid if only partially supported bits are set.
>
> Note for PEBS feature, the PEBS_FORMAT bit field is the primary concern,
> thus if the PEBS_FORMAT input is empty, the other bits check about PEBS
> (like PEBS_TRAP or ARCH_REG) will be ignored.
>
> Signed-off-by: Like Xu <likexu@tencent.com>
> ---
> tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> 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 6ec901dab61e..98483947f921 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
> @@ -13,6 +13,7 @@
>
> #define _GNU_SOURCE /* for program_invocation_short_name */
> #include <sys/ioctl.h>
> +#include <linux/bitmap.h>
>
> #include "kvm_util.h"
> #include "vmx.h"
> @@ -56,7 +57,7 @@ int main(int argc, char *argv[])
> const struct kvm_cpuid_entry2 *entry_a_0;
> struct kvm_vm *vm;
> struct kvm_vcpu *vcpu;
> - int ret;
> + int ret, bit;
> union cpuid10_eax eax;
> union perf_capabilities host_cap;
>
> @@ -97,6 +98,12 @@ int main(int argc, char *argv[])
> ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, 0x30);
> TEST_ASSERT(ret == 0, "Bad PERF_CAPABILITIES didn't fail.");
>
> + /* testcase 4, reject LBR_FMT if only partially supported bits are set */
> + for_each_set_bit(bit, (unsigned long *)&host_cap.capabilities, 6) {
> + ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, BIT_ULL(bit));
> + TEST_ASSERT(ret == 0, "Bad PERF_CAPABILITIES didn't fail.");
> + }
Rather than add another testcase and target only a subset of possible values, what
about replacing (fixing IMO) testcase #3 with fully exhaustive approach?
---
From: Sean Christopherson <seanjc@google.com>
Date: Thu, 4 Aug 2022 12:18:15 -0700
Subject: [PATCH] KVM: selftests: Test all possible "invalid"
PERF_CAPABILITIES.LBR_FMT vals
Test all possible input values to verify that KVM rejects all values
except the exact host value. Due to the LBR format affecting the core
functionality of LBRs, KVM can't emulate "other" formats, so even though
there are a variety of legal values, KVM should reject anything but an
exact host match.
Suggested-by: Like Xu <like.xu.linux@gmail.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../selftests/kvm/x86_64/vmx_pmu_caps_test.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 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 6ec901dab61e..069589c52f41 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
@@ -59,6 +59,7 @@ int main(int argc, char *argv[])
int ret;
union cpuid10_eax eax;
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);
@@ -91,11 +92,17 @@ int main(int argc, char *argv[])
vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, host_cap.lbr_format);
ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), (u64)host_cap.lbr_format);
- /* testcase 3, check invalid LBR format is rejected */
- /* Note, on Arch LBR capable platforms, LBR_FMT in perf capability msr is 0x3f,
- * to avoid the failure, use a true invalid format 0x30 for the test. */
- ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, 0x30);
- TEST_ASSERT(ret == 0, "Bad PERF_CAPABILITIES didn't fail.");
+ /*
+ * Testcase 3, check that an "invalid" LBR format is rejected. Only an
+ * exact match of the host's format (and 0/disabled) is allowed.
+ */
+ for (val = 1; val <= PMU_CAP_LBR_FMT; val++) {
+ if (val == (host_cap.capabilities & PMU_CAP_LBR_FMT))
+ continue;
+
+ ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
+ TEST_ASSERT(!ret, "Bad LBR FMT = 0x%lx didn't fail", val);
+ }
printf("Completed perf capability tests.\n");
kvm_vm_free(vm);
base-commit: 93472b79715378a2386598d6632c654a2223267b
--
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] selftests: kvm/x86: test if it checks all the bits in the LBR_FMT bit-field
2022-08-04 23:26 ` Sean Christopherson
@ 2022-08-05 6:10 ` Like Xu
0 siblings, 0 replies; 3+ messages in thread
From: Like Xu @ 2022-08-05 6:10 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, kvm, linux-kernel
On 5/8/2022 7:26 am, Sean Christopherson wrote:
> On Thu, Aug 04, 2022, Like Xu wrote:
>> From: Like Xu <likexu@tencent.com>
>>
>> User space only enable guest LBR feature when the exactly supported
>> LBR format value is initialized to the MSR_IA32_PERF_CAPABILITIES.
>> The input is also invalid if only partially supported bits are set.
>>
>> Note for PEBS feature, the PEBS_FORMAT bit field is the primary concern,
>> thus if the PEBS_FORMAT input is empty, the other bits check about PEBS
>> (like PEBS_TRAP or ARCH_REG) will be ignored.
>>
>> Signed-off-by: Like Xu <likexu@tencent.com>
>> ---
>> tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> 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 6ec901dab61e..98483947f921 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
>> @@ -13,6 +13,7 @@
>>
>> #define _GNU_SOURCE /* for program_invocation_short_name */
>> #include <sys/ioctl.h>
>> +#include <linux/bitmap.h>
>>
>> #include "kvm_util.h"
>> #include "vmx.h"
>> @@ -56,7 +57,7 @@ int main(int argc, char *argv[])
>> const struct kvm_cpuid_entry2 *entry_a_0;
>> struct kvm_vm *vm;
>> struct kvm_vcpu *vcpu;
>> - int ret;
>> + int ret, bit;
>> union cpuid10_eax eax;
>> union perf_capabilities host_cap;
>>
>> @@ -97,6 +98,12 @@ int main(int argc, char *argv[])
>> ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, 0x30);
>> TEST_ASSERT(ret == 0, "Bad PERF_CAPABILITIES didn't fail.");
>>
>> + /* testcase 4, reject LBR_FMT if only partially supported bits are set */
>> + for_each_set_bit(bit, (unsigned long *)&host_cap.capabilities, 6) {
>> + ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, BIT_ULL(bit));
>> + TEST_ASSERT(ret == 0, "Bad PERF_CAPABILITIES didn't fail.");
>> + }
>
> Rather than add another testcase and target only a subset of possible values, what
> about replacing (fixing IMO) testcase #3 with fully exhaustive approach?
Yeah, much better.
>
> ---
> From: Sean Christopherson <seanjc@google.com>
> Date: Thu, 4 Aug 2022 12:18:15 -0700
> Subject: [PATCH] KVM: selftests: Test all possible "invalid"
> PERF_CAPABILITIES.LBR_FMT vals
>
> Test all possible input values to verify that KVM rejects all values
> except the exact host value. Due to the LBR format affecting the core
> functionality of LBRs, KVM can't emulate "other" formats, so even though
> there are a variety of legal values, KVM should reject anything but an
> exact host match.
>
> Suggested-by: Like Xu <like.xu.linux@gmail.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> .../selftests/kvm/x86_64/vmx_pmu_caps_test.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 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 6ec901dab61e..069589c52f41 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
> @@ -59,6 +59,7 @@ int main(int argc, char *argv[])
> int ret;
> union cpuid10_eax eax;
> 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);
> @@ -91,11 +92,17 @@ int main(int argc, char *argv[])
> vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, host_cap.lbr_format);
> ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), (u64)host_cap.lbr_format);
>
> - /* testcase 3, check invalid LBR format is rejected */
> - /* Note, on Arch LBR capable platforms, LBR_FMT in perf capability msr is 0x3f,
> - * to avoid the failure, use a true invalid format 0x30 for the test. */
> - ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, 0x30);
> - TEST_ASSERT(ret == 0, "Bad PERF_CAPABILITIES didn't fail.");
> + /*
> + * Testcase 3, check that an "invalid" LBR format is rejected. Only an
> + * exact match of the host's format (and 0/disabled) is allowed.
> + */
> + for (val = 1; val <= PMU_CAP_LBR_FMT; val++) {
> + if (val == (host_cap.capabilities & PMU_CAP_LBR_FMT))
Emm,
if (val == host_cap.lbr_format)
to save a minor cost of logical operation.
> + continue;
> +
> + ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
> + TEST_ASSERT(!ret, "Bad LBR FMT = 0x%lx didn't fail", val);
> + }
>
> printf("Completed perf capability tests.\n");
> kvm_vm_free(vm);
>
> base-commit: 93472b79715378a2386598d6632c654a2223267b
> --
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-05 6:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-04 7:38 [PATCH] selftests: kvm/x86: test if it checks all the bits in the LBR_FMT bit-field Like Xu
2022-08-04 23:26 ` Sean Christopherson
2022-08-05 6:10 ` Like Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).