Re: [kvm-unit-tests PATCH 3/4] x86/pmu: Test adaptive PEBS without any adaptive counters

[Sean Christopherson <seanjc@google.com>]

On Thu, Mar 07, 2024, Dapeng Mi wrote:
> On 3/7/2024 7:01 AM, Sean Christopherson wrote:
> > @@ -293,12 +293,9 @@ static void check_pebs_records(u64 bitmask, u64 pebs_data_cfg)
> >   	do {
> >   		pebs_rec = (struct pebs_basic *)cur_record;
> >   		pebs_record_size = pebs_rec->format_size >> RECORD_SIZE_OFFSET;
> > -		pebs_idx_match =
> > -			pebs_rec->applicable_counters & bitmask;
> > -		pebs_size_match =
> > -			pebs_record_size == get_adaptive_pebs_record_size(pebs_data_cfg);
> > -		data_cfg_match =
> > -			(pebs_rec->format_size & GENMASK_ULL(47, 0)) == pebs_data_cfg;
> > +		pebs_idx_match = pebs_rec->applicable_counters & bitmask;
> > +		pebs_size_match = pebs_record_size == get_pebs_record_size(pebs_data_cfg, use_adaptive);
> > +		data_cfg_match = (pebs_rec->format_size & GENMASK_ULL(47, 0)) == pebs_data_cfg;
> 
> Since there is already a macro RECORD_SIZE_OFFSET, we'd better use
> "RECORD_SIZE_OFFSET - 1" to replace the magic number 47.

Very belatedly, I disagree.  That the data configuration mask isn't derived from
record size.  The fact that the record size is the _only_ info that's excluded
is coincidental (sort of, that's not quite the right word), i.e. if we want to
use a #define, then we should add an explicit define, not abuse RECORD_SIZE_OFFSET.