| 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
| ||
|
Message-ID: <gsnth6b49srd.fsf@coltonlewis-kvm.c.googlers.com>
Date: Wed, 28 Aug 2024 22:39:34 +0000
From: Colton Lewis <coltonlewis@...gle.com>
To: Mingwei Zhang <mizhang@...gle.com>
Cc: kvm@...r.kernel.org, ljr.kernel@...il.com, jmattson@...gle.com,
aaronlewis@...gle.com, seanjc@...gle.com, pbonzini@...hat.com,
shuah@...nel.org, linux-kselftest@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/6] KVM: x86: selftests: Set up AMD VM in pmu_counters_test
Hi Mingwei
Mingwei Zhang <mizhang@...gle.com> writes:
> On Tue, Aug 13, 2024, Colton Lewis wrote:
>> Branch in main() depending on if the CPU is Intel or AMD. They are
>> subject to vastly different requirements because the AMD PMU lacks
>> many properties defined by the Intel PMU including the entire CPUID
>> 0xa function where Intel stores all the PMU properties. AMD lacks this
>> as well as any consistent notion of PMU versions as Intel does. Every
>> feature is a separate flag and they aren't the same features as Intel.
>> Set up a VM for testing core AMD counters and ensure proper CPUID
>> features are set.
>> Signed-off-by: Colton Lewis <coltonlewis@...gle.com>
>> ---
>> .../selftests/kvm/x86_64/pmu_counters_test.c | 80 ++++++++++++++++---
>> 1 file changed, 68 insertions(+), 12 deletions(-)
>> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
>> b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
>> index 0e305e43a93b..a11df073331a 100644
>> --- a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
>> +++ b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
>> @@ -33,7 +33,7 @@
>> static uint8_t kvm_pmu_version;
>> static bool kvm_has_perf_caps;
>> -static struct kvm_vm *pmu_vm_create_with_one_vcpu(struct kvm_vcpu
>> **vcpu,
>> +static struct kvm_vm *intel_pmu_vm_create(struct kvm_vcpu **vcpu,
>> void *guest_code,
>> uint8_t pmu_version,
>> uint64_t perf_capabilities)
>> @@ -303,7 +303,7 @@ static void test_arch_events(uint8_t pmu_version,
>> uint64_t perf_capabilities,
>> if (!pmu_version)
>> return;
>> - vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_arch_events,
>> + vm = intel_pmu_vm_create(&vcpu, guest_test_arch_events,
>> pmu_version, perf_capabilities);
>> vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH,
>> @@ -463,7 +463,7 @@ static void test_gp_counters(uint8_t pmu_version,
>> uint64_t perf_capabilities,
>> struct kvm_vcpu *vcpu;
>> struct kvm_vm *vm;
>> - vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_gp_counters,
>> + vm = intel_pmu_vm_create(&vcpu, guest_test_gp_counters,
>> pmu_version, perf_capabilities);
>> vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_NR_GP_COUNTERS,
>> @@ -530,7 +530,7 @@ static void test_fixed_counters(uint8_t pmu_version,
>> uint64_t perf_capabilities,
>> struct kvm_vcpu *vcpu;
>> struct kvm_vm *vm;
>> - vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_fixed_counters,
>> + vm = intel_pmu_vm_create(&vcpu, guest_test_fixed_counters,
>> pmu_version, perf_capabilities);
>> vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_FIXED_COUNTERS_BITMASK,
>> @@ -627,18 +627,74 @@ static void test_intel_counters(void)
>> }
>> }
>> -int main(int argc, char *argv[])
>> +static uint8_t nr_core_counters(void)
>> {
>> - TEST_REQUIRE(kvm_is_pmu_enabled());
>> + const uint8_t nr_counters =
>> kvm_cpu_property(X86_PROPERTY_NUM_PERF_CTR_CORE);
>> + const bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
>> + /* The default numbers promised if the property is 0 */
>> + const uint8_t amd_nr_core_ext_counters = 6;
>> + const uint8_t amd_nr_core_counters = 4;
>> +
>> + if (nr_counters != 0)
>> + return nr_counters;
>> +
>> + if (core_ext)
>> + return amd_nr_core_ext_counters;
>> +
>> + return amd_nr_core_counters;
>> +}
>> +
>> +static void guest_test_core_counters(void)
>> +{
>> + GUEST_DONE();
>> +}
>> - TEST_REQUIRE(host_cpu_is_intel);
>> - TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
>> - TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
>> +static void test_core_counters(void)
>> +{
>> + uint8_t nr_counters = nr_core_counters();
>> + bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
>> + bool perf_mon_v2 = kvm_cpu_has(X86_FEATURE_PERF_MON_V2);
>> + struct kvm_vcpu *vcpu;
>> + struct kvm_vm *vm;
>> - kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
>> - kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
>> + vm = vm_create_with_one_vcpu(&vcpu, guest_test_core_counters);
>> - test_intel_counters();
>> + /* This property may not be there in older underlying CPUs,
>> + * but it simplifies the test code for it to be set
>> + * unconditionally.
>> + */
>> + vcpu_set_cpuid_property(vcpu, X86_PROPERTY_NUM_PERF_CTR_CORE,
>> nr_counters);
>> + if (core_ext)
>> + vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_CTR_EXT_CORE);
>> + if (perf_mon_v2)
>> + vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_MON_V2);
> hmm, I think this might not be enough. So, when the baremetal machine
> supports Perfmon v2, this code is just testing v2. But we should be able
> to test anything below v2, ie., v1, v1 without core_ext. So, three
> cases need to be tested here: v1 with 4 counters; v1 with core_ext (6
> counters); v2.
> If, the machine running this selftest does not support v2 but it does
> support core extension, then we fall back to test v1 with 4 counters and
> v1 with 6 counters.
This should cover all cases the way I wrote it. I detect the number of
counters in nr_core_counters(). That tells me if I am dealing with 4 or
6 and then I set the cpuid property based on that so I can read that
number in later code instead of duplicating the logic.
I could always inline nr_core_counters() to make this more obvious since
it is only called in this function. It was one of the first bits of code
I wrote working on this series and I assumed I would need to call it a
bunch before I decided I could just set the cpuid property after calling
it once.
>> +
>> + pr_info("Testing core counters: CoreExt = %u, PerfMonV2 = %u,
>> NumCounters = %u\n",
>> + core_ext, perf_mon_v2, nr_counters);
>> + run_vcpu(vcpu);
>> +
>> + kvm_vm_free(vm);
>> +}
>> +
>> +static void test_amd_counters(void)
>> +{
>> + test_core_counters();
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + TEST_REQUIRE(kvm_is_pmu_enabled());
>> +
>> + if (host_cpu_is_intel) {
>> + TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
>> + TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
>> + kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
>> + kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
>> + test_intel_counters();
>> + } else if (host_cpu_is_amd) {
>> + /* AMD CPUs don't have the same properties to look at. */
>> + test_amd_counters();
>> + }
>> return 0;
>> }
>> --
>> 2.46.0.76.ge559c4bf1a-goog
Powered by blists - more mailing lists