From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yw1-f201.google.com (mail-yw1-f201.google.com [209.85.128.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 41F111514DC for ; Thu, 29 Aug 2024 20:45:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724964345; cv=none; b=cjmNxibX6n4BFu0kXA4vPew/n9yoy8U9wzT5aXb3WYtDxu0N+SPXjP9Us/mjLN/ndgQHXViB1AB2hXC43Lqo0ADx6bA5QGp/yX80LH7RUkIRusNhbs3qb2WtfocL6sRccFy3nBLLvJbASp8imcMDu2Esz4nfGx5oQyEpDq00axA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724964345; c=relaxed/simple; bh=4TZEqBx7eWCdocTsLXNrFut7vdn8ko9P9heYPOQxy4I=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=W3QwtkbTdFaXCFyE25+aon5IAfqixo1RAan+O5l0wPIAOU86mtu7RiLqEz5sQqtV2SvNKd3pYnvC321YOpo7L7duBZ1n67H6L8BQznentLgRabgpvQVgLyBdGTS+kRH5SwhUG3jj/3XYG7nTmSdLG+x0HANRjmqzdA/hI+YQuww= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=wkTXxSPP; arc=none smtp.client-ip=209.85.128.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="wkTXxSPP" Received: by mail-yw1-f201.google.com with SMTP id 00721157ae682-6b2249bf2d0so24606367b3.2 for ; Thu, 29 Aug 2024 13:45:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1724964343; x=1725569143; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=rF0JnnhJ+6UUAp3OAAeI13+/dbW5hEBVYSBB7nknrZM=; b=wkTXxSPPWuTdbJwzK+wH0pG6X9mTeI/WqPtDHCCXeI01rzQliZQBj3plWJFQBZwdBH Y+BeE0WPlkCK1MsUXjV2bL0YWQjkqMrJi61MM2rPPE7FNrPjLhYlTGkfY0FrIDqnO4CN ogbybt4LPnj25Q5jer/mXPk4fdVz2J5/oT0e0DtK2Oz+5OOMZp+JmdbQT8Zef8NctcZF TqXbGdZExySf46TDGHkCvFiVFGH/EmsDY92qIspI49nDkqM4vbgM2jcpNab8g30pEfSH wV6cbIvyXxuQAKAfUyEr7qMnANubqkXznw4Aoc5q1XY5nnzfJiYa/qRZhIe/ohnMy5WW Lxmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724964343; x=1725569143; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=rF0JnnhJ+6UUAp3OAAeI13+/dbW5hEBVYSBB7nknrZM=; b=CWqUO5dxNV7mi1T5y7D6yMgeyVfBoQ654BcCRO1H0U4cMjfbZx6c7a/kMtP54mPllp DylRc4sZK7ifUXAkXNJLAjyBbstaWRl5zSZvUSDSQpj4HQBiE3UO5WXBrOE50UzOZ9Be WKHsJcll4sF1krSAVm5jx7mrUUTvacB1bgpmD+IVm7JZ0Gb+hnD25F7zZwNy4got8kkt eGcGTyr/wDk7e+qaSWg+/bIUOoDDM86AJHBaE34lWKyQAeqsImH3wXJJhkpvW/4H3bz/ bB022baFlwvgOt2hsNC5jvDbf9oO4q6Q8p8VWS/jLTnHF/ZlamynMWfpViEoo5233fnj 52bw== X-Forwarded-Encrypted: i=1; AJvYcCXKyxP4Ii4kURj7yY5opKV+9mR5rPICpzLekNYG5Q781USrIiqOyddzq7KLUG5Q1OV5B3/U/+UsUWaxsyg=@vger.kernel.org X-Gm-Message-State: AOJu0Yy1PNmIcOduwtyd2eZkS4nPoO7FkRSEfXKPgrvyqQpS4cETqXT/ 7FBjxHyVlbhIIkbK8ge5vmUxJLQwd1CmVPzJfcpHvujsvARDAnM7slk8/NPYsTXxnvx2KKmuu6m 2xA== X-Google-Smtp-Source: AGHT+IGVP3HvwnzOqRNa87UbyRnTdSkHHKG6AQXQsUhHE0H0PT9PCKyy3tNykxqKPYyhE/+825Ajpugsias= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a81:d10b:0:b0:6ad:feb0:d010 with SMTP id 00721157ae682-6d2e94d0921mr569377b3.6.1724964342802; Thu, 29 Aug 2024 13:45:42 -0700 (PDT) Date: Thu, 29 Aug 2024 13:45:41 -0700 In-Reply-To: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: Message-ID: Subject: Re: [PATCH 3/6] KVM: x86: selftests: Set up AMD VM in pmu_counters_test From: Sean Christopherson To: Mingwei Zhang Cc: Colton Lewis , kvm@vger.kernel.org, ljr.kernel@gmail.com, jmattson@google.com, aaronlewis@google.com, pbonzini@redhat.com, shuah@kernel.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="us-ascii" On Wed, Aug 28, 2024, Mingwei Zhang wrote: > > >> +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. But then the test isn't verifying that KVM is honoring the architecture. I.e. backdooring information to the guest risks getting false passes because KVM incorrectly peeks at the same information, which shouldn't exist. > > >> + */ /* * Multi-line function comments should start on the line after the * opening slash-asterisk, like so. */ > > >> + 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. > > right. in the current code, you set up the counters properly according > to the hw capability. But the test can do more on a hw with perfmon > v2, right? Because it can test multiple combinations of setup for a > VM: say v1 + 4 counters and v1 + 6 counters etc. I am just following > the style of this selftest on Intel side, in which they do a similar > kind of enumeration of PMU version + PDCM capabilities. In each > configuration, it will invoke a VM and do the test. Ya. This is similar my comments on setting NUM_PER_CTR_CORE when the field shouldn't exist. One of the main goals of this test is to verify the KVM honors the architecture based on userspace's defined virtual CPU model, i.e. guest CPUID. That means testing all (or at least, within reason) possible combinations that can feasibly be supported by KVM given the underlying hardware. As written, this essentially just tests the maximal configuration that can be exposed to a guest, which isn't _that_ interesting because KVM tends to get plenty of coverage for such setups, e.g. by running "real" VMs.