Re: [kvm-unit-tests PATCH 07/16] x86/pmu: Rename pmu_gp_counter_is_available() to pmu_arch_event_is_available()

From: Sean Christopherson <seanjc@google.com>
To: Dapeng Mi <dapeng1.mi@linux.intel.com>
Cc: "Andrew Jones" <andrew.jones@linux.dev>,
	"Janosch Frank" <frankja@linux.ibm.com>,
	"Claudio Imbrenda" <imbrenda@linux.ibm.com>,
	"Nico Böhr" <nrb@linux.ibm.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	kvm-riscv@lists.infradead.org, linux-s390@vger.kernel.org,
	kvm@vger.kernel.org
Subject: Re: [kvm-unit-tests PATCH 07/16] x86/pmu: Rename pmu_gp_counter_is_available() to pmu_arch_event_is_available()
Date: Tue, 10 Jun 2025 09:16:00 -0700	[thread overview]
Message-ID: <aEhaQITromUV7lIO@google.com> (raw)
In-Reply-To: <ffb5e853-dedc-45bb-acd8-c58ff2fc0b71@linux.intel.com>

On Tue, Jun 10, 2025, Dapeng Mi wrote:
> On 5/30/2025 6:19 AM, Sean Christopherson wrote:
> > @@ -51,7 +51,7 @@ void pmu_init(void)
> >  		}
> >  		pmu.gp_counter_width = PMC_DEFAULT_WIDTH;
> >  		pmu.gp_counter_mask_length = pmu.nr_gp_counters;
> > -		pmu.gp_counter_available = (1u << pmu.nr_gp_counters) - 1;
> > +		pmu.arch_event_available = (1u << pmu.nr_gp_counters) - 1;
> 
> "available architectural events" and "available GP counters" are two
> different things. I know this would be changed in later patch 09/16, but
> it's really confusing. Could we merge the later patch 09/16 into this patch?

Ya.  I was trying to not mix too many things in one patch, but looking at this
again, I 100% agree that squashing 7-9 into one patch is better overall.

> > @@ -463,7 +463,7 @@ static void check_counters_many(void)
> >  	int i, n;
> >  
> >  	for (i = 0, n = 0; n < pmu.nr_gp_counters; i++) {
> > -		if (!pmu_gp_counter_is_available(i))
> > +		if (!pmu_arch_event_is_available(i))
> >  			continue;
> 
> The intent of check_counters_many() is to verify all available GP and fixed
> counters can count correctly at the same time. So we should select another
> available event to verify the counter instead of skipping the counter if an
> event is not available.

Agreed, but I'm going to defer that for now, this series already wanders in too
many directions.  Definitely feel free to post a patch.

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv