Re: [PATCH v1 3/8] KVM: x86: Allocate performance counter for PEBS event

[Peter Zijlstra]

On Wed, Oct 30, 2019 at 04:06:36AM +0000, Kang, Luwei wrote:
> > >  static void pmc_reprogram_counter(struct kvm_pmc *pmc, u32 type,
> > >  				  unsigned config, bool exclude_user,
> > >  				  bool exclude_kernel, bool intr,
> > > -				  bool in_tx, bool in_tx_cp)
> > > +				  bool in_tx, bool in_tx_cp, bool pebs)
> > >  {
> > >  	struct perf_event *event;
> > >  	struct perf_event_attr attr = {
> > > @@ -111,9 +111,12 @@ static void pmc_reprogram_counter(struct kvm_pmc *pmc, u32 type,
> > >  		.exclude_user = exclude_user,
> > >  		.exclude_kernel = exclude_kernel,
> > >  		.config = config,
> > > +		.precise_ip = pebs ? 1 : 0,
> > > +		.aux_output = pebs ? 1 : 0,
> > 
> > srsly?
> 
> Hi Peter,
>     Thanks for review. For aux_output, I think it should be set 1 when the guest wants to enabled PEBS by Intel PT.
>      For precise_ip, it is the precise level in perf and set by perf command line in KVM guest, this may not reflect the accurate value (can be 0~3) here. Here set to 1 is used to allocate a counter for PEBS event and set the MSR_IA32_PEBS_ENABLE register. For PMU virtualization, KVM will trap the guest's write operation to PMU registers and allocate/free event counter from host if a counter enable/disable in guest. We can't always deduce the exact parameter of perf command line from the value of the guest writers to the register.

Please, teach your MUA to wrap on 78 chars.

The thing I really fell over is the gratuitous 'bool ? 1 : 0'. But yes,
the aux_output without a group leader is dead wrong. We'll go fix
perf_event_create_kernel_counter() to refuse that.