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

[Andi Kleen <ak@linux.intel.com>]

> A few questions:
> 
>  - How will this 'feature' interact with MSR_CORE_PERF_GLOBAL_CTRL ?

It's completely independent of GLOBAL_CTRL on the software level.
Internally it uses it, but it shouldn't be software visible.

>  - Is having GLOBAL_CTRL.3 0 and not using RTM instructions enough
>    guarantee to avoid PMC3 from being clobbered?

With no use of RTM clobbering should be fairly unlikely,
but in theory it could happen if RETPOLINE is not used
(due to an indirect branch mistakenenly executing something
that looks like XBEGIN). If we assume that RETPOLINE 
is used I believe it shouldn't happen in the kernel at least.
Even without RETPOLINE such a case should be fairly unlikely,
but cannot be 100% ruled out.

> Esp. that latter question, because both your patches and the below seems
> to rely on that; and if the answer is yet, the below can be further
> simplified.

I don't see how I rely on that?

In fact I avoided some simple optimizations to not rely on it, but only
ever use counter 3 after the MSR write.

> 
> So what (aside from being _completely_ untested and not doing virt crap)
> is wrong with the below? It sure as heck is simpler.


> +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.

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. 


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.

-Andi