[MODERATED] Re: [PATCH v2 0/8] PERFv2

On Thu, Feb 07, 2019 at 06:37:59AM -0800, speck for Andi Kleen wrote:

> > +static struct event_constraint *
> > +skl_get_event_constraints(struct cpu_hw_events *cpuc, int idx,
> > +			  struct perf_event *event)
> > +{
> > +	struct event_constraint *c;
> > +
> > +	c = hsw_get_event_constraints(cpuc, idx, event);
> > +
> > +	if (!force_rtm_abort) {
> > +		/*
> > +		 * Without TFA we must not use PMC3.
> > +		 */
> > +		c->idxmsk64 &= ~(1ULL << 3);
> 
> Doesn't that overwrite the global shared constraints constraints?
> Would need to make them non const at least, it will likely fault.

They are !const; we already update them at init time (for example we add
the GPRs to the fixed ones etc..).

> Okay I see you rely on it being guarded by the global, but for virtualization
> support will need the copy.
> 
> Would need a copy at least.

I've not thought abou virt yet; do we _have_ to expose the thing to
guests?

> Other than that there doesn't seem to be anything obviously wrong.
> 
> I will do some testing/checking and readd the opt-in/opt-out interfaces.

Right, so per using DEVICE_BOOL_ATTR() changing the value is racy like
hell. If we were to use an update function we could try and do
complicated crap. Not sure that's worth it though.