perf: Implement Nehalem uncore pmu

Changelogs of v3:

- Allocate uncore data with kmalloc_node, like AMD NB stuff. (Peter Zijlstra)

- per-task uncore event is not allowed. Simply set pmu::task_ctx_nr = perf_invalid_context. (Peter Zijlstra)

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +25 -0

+
+static int
+uncore_perf_event_set_period(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	s64 left = local64_read(&hwc->period_left);
+	s64 period = hwc->sample_period;
+	u64 max_period = (1ULL << UNCORE_CNTVAL_BITS) - 1;
+	int ret = 0, idx = hwc->idx;
+
+	/*
+	 * If we are way outside a reasonable range then just skip forward:
+	 */
+	if (unlikely(left <= -period)) {
+		left = period;
+		local64_set(&hwc->period_left, left);
+		hwc->last_period = period;
+		ret = 1;
+	}
+
+	if (unlikely(left <= 0)) {
+		left += period;
+		local64_set(&hwc->period_left, left);
+		hwc->last_period = period;
+		ret = 1;

Quoted 25 lines · Lin Ming

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +25 -0

+
+static int
+uncore_perf_event_set_period(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	s64 left = local64_read(&hwc->period_left);
+	s64 period = hwc->sample_period;
+	u64 max_period = (1ULL << UNCORE_CNTVAL_BITS) - 1;
+	int ret = 0, idx = hwc->idx;
+
+	/*
+	 * If we are way outside a reasonable range then just skip forward:
+	 */
+	if (unlikely(left <= -period)) {
+		left = period;
+		local64_set(&hwc->period_left, left);
+		hwc->last_period = period;
+		ret = 1;
+	}
+
+	if (unlikely(left <= 0)) {
+		left += period;
+		local64_set(&hwc->period_left, left);
+		hwc->last_period = period;
+		ret = 1;

arch/x86/kernel/cpu/perf_event_intel_uncore.c line 143

Quoted 10 lines

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +10 -0

+	}
+
+	if (left > max_period)
+		left = max_period;
+
+	/*
+	 * The hw event starts counting from this event offset,
+	 * mark it to be able to extra future deltas:
+	 */
+	local64_set(&hwc->prev_count, (u64)-left);

All uncore pmu interrupts from a socket are routed to one of the four cores, so local64_set seems not correct here.

But hwc->prev_count is defined as local64_t, any idea how to set it correctly?

Or is it OK if local64_set is always executed in the same cpu?

Quoted 5 lines · Lin Ming

+       /* Check CPUID signatures: 06_1AH, 06_1EH, 06_1FH */
+       family = boot_cpu_data.x86;
+       model = boot_cpu_data.x86_model;
+       if (family != 6 || (model != 0x1A && model != 0x1E && model != 0x1F))
+               return;

So that's 26, 30 and 31? Curiously arch/x86/kernel/cpu/perf_event_intel.c does have 31.

Quoted 9 lines · Peter Zijlstra

arch/x86/kernel/cpu/perf_event_intel_uncore.c line 534

Quoted 5 lines · Lin Ming

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +5 -0

+       /* Check CPUID signatures: 06_1AH, 06_1EH, 06_1FH */
+       family = boot_cpu_data.x86;
+       model = boot_cpu_data.x86_model;
+       if (family != 6 || (model != 0x1A && model != 0x1E && model != 0x1F))
+               return;

So that's 26, 30 and 31? Curiously arch/x86/kernel/cpu/perf_event_intel.c does have 31.

It is also missing model 44 (0x2c).

Quoted 8 lines · Stephane Eranian

Quoted 7 lines · Peter Zijlstra

Quoted 5 lines · Lin Ming

+       /* Check CPUID signatures: 06_1AH, 06_1EH, 06_1FH */
+       family = boot_cpu_data.x86;
+       model = boot_cpu_data.x86_model;
+       if (family != 6 || (model != 0x1A && model != 0x1E && model != 0x1F))
+               return;

Quoted 2 lines

Quoted 2 lines

So that's 26, 30 and 31? Curiously arch/x86/kernel/cpu/perf_event_intel.c does have 31.

That was clearly meant to say: doesn't.. Does Intel have an exhaustive model list somewhere?

Quoted 1 line

It is also missing model 44 (0x2c).

Right.. but if the westmere uncore is the same, then its also missing 37.

The -EX chips have a different uncore, right?

Hi,

So I have tested this patch a bit on WSM and as I expected there are issues with sampling.

When HT is on, both siblings CPUs get the interrupt. The HW does not allow you to only point interrupts to a single HT thread (CPU).

I did verify that indeed both threads get the interrupt and that you have a race condition. Both sibling CPUs stop uncore, get the status. They may get the same overflow status. Both will pass the uncore->active_mask because it's shared among siblings cores. Thus, you have a race for the whole interrupt handler execution.

You need some serialization in there. But the patch does not address this. The problem is different from the back-to-back interrupt issue that Don worked on.
The per-cpu marked/handled trick cannot work to avoid this problem.

You cannot simply say "the lowest indexed" CPU of a sibling pair handles the interrupt
because you don't know if this in an uncore intr, core interrupt or something else. You
need to check. That means each HT thread needs to check uncore ovfl_status. IF the
status is zero, then return. Otherwise, you need to do a 2nd level check before you can
execute the handler. You need to know if the sibling CPU has already "consumed" that
interrupt.

I think you need some sort of generation counter per physical core and per HT thread.
On interrupt, you could do something along the line of:

      if (mycpu->intr_count == mysibling->intr_count) {
          then mycpu->intr_count++
          execute intr_handler()
      } else {
          mycpu->intr_count++
          return;
      }

Of course, the above needs some atomicity and ad locking (but I don't think you can
use locks in NMI context).

This makes me wonder if vectoring uncore to NMI is really needed, given you cannot
correlated to an IP, incl. a kernel IP. If we were to vector to a dedicated (lower prio)
vector, then we could use the trick of saying the lowest indexed CPU in a pair handles the interrupt (because we would already know this is an uncore interrupt).
This would be much simpler. Price: not samples in kernel's critical section. But those
are useless anyway with uncore events.

- uncore_get_status().
PERF_GLOBAL_STATUS contains more than overflow
status, bit 61-63 need to be masked off.

- uncore_pmu_cpu_prepare()
I don't understand the x86_max_cores < 2 test. If you run your NHM/WSM is single core mode, you still have uncore to deal with thus, you need cpuc->intel_uncore initialized, don't you?

- I assume that the reason the uncore->refcnt management is not using atomic ops because the whole CPU hotplug is serialized, right?

Hi,

So I have tested this patch a bit on WSM and as I expected there are issues with sampling.

When HT is on, both siblings CPUs get the interrupt. The HW does not allow you to only point interrupts to a single HT thread (CPU).

I did verify that indeed both threads get the interrupt and that you have a race condition. Both sibling CPUs stop uncore, get the status. They may get the same overflow status. Both will pass the uncore->active_mask because it's shared among siblings cores. Thus, you have a race for the whole interrupt handler execution.

You need some serialization in there. But the patch does not address this. The problem is different from the back-to-back interrupt issue that Don worked on.
The per-cpu marked/handled trick cannot work to avoid this problem.

You cannot simply say "the lowest indexed" CPU of a sibling pair handles the interrupt
because you don't know if this in an uncore intr, core interrupt or something else. You
need to check. That means each HT thread needs to check uncore ovfl_status. IF the
status is zero, then return. Otherwise, you need to do a 2nd level check before you can
execute the handler. You need to know if the sibling CPU has already "consumed" that
interrupt.

I think you need some sort of generation counter per physical core and per HT thread.
On interrupt, you could do something along the line of:

      if (mycpu->intr_count == mysibling->intr_count) {
          then mycpu->intr_count++
          execute intr_handler()
      } else {
          mycpu->intr_count++
          return;
      }

Of course, the above needs some atomicity and ad locking

(but I don't think you can use locks in NMI context).

This makes me wonder if vectoring uncore to NMI is really needed, given you cannot
correlated to an IP, incl. a kernel IP. If we were to vector to a dedicated (lower prio)
vector, then we could use the trick of saying the lowest indexed CPU in a pair handles the interrupt (because we would already know this is an uncore interrupt).
This would be much simpler. Price: not samples in kernel's critical section. But those
are useless anyway with uncore events.

Quoted 7 lines · Stephane Eranian

Hi,

So I have tested this patch a bit on WSM and as I expected there are issues with sampling.

When HT is on, both siblings CPUs get the interrupt. The HW does not allow you to only point interrupts to a single HT thread (CPU).

Egads, how ugly :/

Quoted 26 lines

I did verify that indeed both threads get the interrupt and that you have a race condition. Both sibling CPUs stop uncore, get the status. They may get the same overflow status. Both will pass the uncore->active_mask because it's shared among siblings cores. Thus, you have a race for the whole interrupt handler execution.

You need some serialization in there. But the patch does not address this. The problem is different from the back-to-back interrupt issue that Don worked on.
The per-cpu marked/handled trick cannot work to avoid this problem.

You cannot simply say "the lowest indexed" CPU of a sibling pair handles the interrupt
because you don't know if this in an uncore intr, core interrupt or something else. You
need to check. That means each HT thread needs to check uncore ovfl_status. IF the
status is zero, then return. Otherwise, you need to do a 2nd level check before you can
execute the handler. You need to know if the sibling CPU has already "consumed" that
interrupt.

I think you need some sort of generation counter per physical core and per HT thread.
On interrupt, you could do something along the line of:

Quoted 8 lines

      if (mycpu->intr_count == mysibling->intr_count) {
          then mycpu->intr_count++
          execute intr_handler()
      } else {
          mycpu->intr_count++
          return;
      }

Of course, the above needs some atomicity and ad locking

Does that guarantee that the same sibling handles all interrupts? Since a lot of the infrastructure uses local*_t we're not good with cross-cpu stuff.

Damn what a mess.. we need to serialize enough for both cpus to at least see the overflow bit.. maybe something like:

struct intel_percore {
   ...
   atomic_t uncore_barrier;
};

void uncore_barrier(void)

{
	struct intel_percore *percore = this_cpu_ptr(cpu_hw_events)->percore;
	int armed;

	armed = atomic_cmpxchg(&percore->uncore_barrier, 0, 1) == 0;
	if (armed) {
		/* we armed, it, now wait for completion */
		while (atomic_read(&percore->uncore_barrier))
			cpu_relax();
	} else {
		/* our sibling must have, decrement it */
		if (atomic_cmpxchg(&percore->uncore_barrier, 1, 0) != 1)
			BUG();
	}
}

Then have something like:

handle_uncore_interrupt()

{
	u64 overflow = rdmsrl(MSR_UNCORE_PERF_GLOBAL_OVF_STATUS);
	int cpu;

	if (!overflow)
		return 0; /* not our interrupt to handle */

	uncore_barrier(); /* wait so our sibling will also observe the overflow */

	cpu = smp_processor_id();
	if (cpu != cpumask_first(topology_thread_cpumask(cpu)))

		return 1; /* our sibling will handle it, eat the NMI */

	/* OK, we've got an overflow and we're the first CPU in the thread mask */

	... do fancy stuff ...

	return 1; /* we handled it, eat the NMI */
}

Quoted 3 lines · Peter Zijlstra

Quoted 2 lines · Stephane Eranian

Hi,

Quoted 8 lines

Quoted 5 lines

So I have tested this patch a bit on WSM and as I expected there are issues with sampling.

When HT is on, both siblings CPUs get the interrupt. The HW does not allow you to only point interrupts to a single HT thread (CPU).

Egads, how ugly :/

That would of course need to also grow some smarts to detect if there is only 1 sibling online.

CC'ed Cyrill as P4 might have something similar.

Quoted 7 lines · Stephane Eranian

Hi,

So I have tested this patch a bit on WSM and as I expected there are issues with sampling.

When HT is on, both siblings CPUs get the interrupt. The HW does not allow you to only point interrupts to a single HT thread (CPU).

Egads, how ugly :/

Quoted 23 lines

I did verify that indeed both threads get the interrupt and that you have a race condition. Both sibling CPUs stop uncore, get the status. They may get the same overflow status. Both will pass the uncore->active_mask because it's shared among siblings cores. Thus,  you have a race for the whole interrupt handler execution.

You need some serialization in there. But the patch does not address this. The problem is different from the back-to-back interrupt issue that Don worked on.
The per-cpu marked/handled trick cannot work to avoid this problem.

You cannot simply say "the lowest indexed" CPU of a sibling pair handles the interrupt
because you don't know if this in an uncore intr, core interrupt or something else. You
need to check. That means each HT thread needs to check uncore ovfl_status. IF the
status is zero, then return. Otherwise, you need to do a 2nd level check before you can
execute the handler. You need to know if the sibling CPU has already "consumed" that
interrupt.

Quoted 11 lines

I think you need some sort of generation counter per physical core and per HT thread.
On interrupt, you could do something along the line of:
      if (mycpu->intr_count == mysibling->intr_count) { then mycpu->intr_count++
          execute intr_handler()
      } else {
          mycpu->intr_count++
          return;
      }
Of course, the above needs some atomicity and ad locking

Does that guarantee that the same sibling handles all interrupts? Since a lot of the infrastructure uses local*_t we're not good with cross-cpu stuff.

Damn what a mess.. we need to serialize enough for both cpus to at least see the overflow bit.. maybe something like:

struct intel_percore {
  ...
  atomic_t uncore_barrier;
};

void uncore_barrier(void)
{
       struct intel_percore *percore = this_cpu_ptr(cpu_hw_events)->percore; int armed;

       armed = atomic_cmpxchg(&percore->uncore_barrier, 0, 1) == 0; if (armed) {
               /* we armed, it, now wait for completion */ while (atomic_read(&percore->uncore_barrier)) cpu_relax();
       } else {
               /* our sibling must have, decrement it */
               if (atomic_cmpxchg(&percore->uncore_barrier, 1, 0) != 1) BUG();
       }
}

Then have something like:

handle_uncore_interrupt()
{
       u64 overflow = rdmsrl(MSR_UNCORE_PERF_GLOBAL_OVF_STATUS); int cpu;

       if (!overflow)
               return 0; /* not our interrupt to handle */

       uncore_barrier(); /* wait so our sibling will also observe the overflow */

       cpu = smp_processor_id();
       if (cpu != cpumask_first(topology_thread_cpumask(cpu))) return 1; /* our sibling will handle it, eat the NMI */

       /* OK, we've got an overflow and we're the first CPU in the thread mask */

       ... do fancy stuff ...

       return 1; /* we handled it, eat the NMI */
}

Quoted 1 line

(but I don't think you can use locks in NMI context).

You can, as long as they're never used from !NMI, its icky, but it works.

Quoted 10 lines

This makes me wonder if vectoring uncore to NMI is really needed, given you cannot
correlated to an IP, incl. a kernel IP. If we were to vector to a dedicated (lower prio)
vector, then we could use the trick of saying the lowest indexed CPU in a pair handles the interrupt (because we would already know this is an uncore interrupt).
This would be much simpler. Price: not samples in kernel's critical section. But those
are useless anyway with uncore events.

But the uncore uses the same PMI line, right? You cannot point the uncore to another vector. /me goes find the docs -- ok, its too early in the morning to clearly parse that...

Besides, people asked for the sampling thing didn't they (also we need it to fold the count to avoid overflow on 48bit). Also the PAPI people even want per-task uncore counters because that's the only thing PAPI can do.

Hi,

Ok, so I have an explanation for what we are seeing. In fact, what bothered me in all
of this is that I did not recall ever running into this problem of double-interrupt with HT
when I implemented the perfmon support for uncore sampling. The reason is in fact
real simple.

You are getting interrupts on both threads because you have enabled uncore PMU on all CPUs, in uncore_cpu_starting():

You need to do that only on ONE of the two siblings. In fact, you want that only ONCE
per socket. You can do this on the first CPU to use the uncore to add something a bit
more dynamic and to make sure you have some control over where the overhead is applied.
Once you do that, only one CPU/socket will get the interrupt and all will be fine.

Quoted 125 lines · Stephane Eranian

Quoted 7 lines

Hi,

So I have tested this patch a bit on WSM and as I expected there are issues with sampling.

When HT is on, both siblings CPUs get the interrupt. The HW does not allow you to only point interrupts to a single HT thread (CPU).

Egads, how ugly :/

Quoted 26 lines

I did verify that indeed both threads get the interrupt and that you have a race condition. Both sibling CPUs stop uncore, get the status. They may get the same overflow status. Both will pass the uncore->active_mask because it's shared among siblings cores. Thus, you have a race for the whole interrupt handler execution.

You need some serialization in there. But the patch does not address this. The problem is different from the back-to-back interrupt issue that Don worked on.
The per-cpu marked/handled trick cannot work to avoid this problem.

You cannot simply say "the lowest indexed" CPU of a sibling pair handles the interrupt
because you don't know if this in an uncore intr, core interrupt or something else. You
need to check. That means each HT thread needs to check uncore ovfl_status. IF the
status is zero, then return. Otherwise, you need to do a 2nd level check before you can
execute the handler. You need to know if the sibling CPU has already "consumed" that
interrupt.

I think you need some sort of generation counter per physical core and per HT thread.
On interrupt, you could do something along the line of:

Quoted 8 lines

      if (mycpu->intr_count == mysibling->intr_count) {
          then mycpu->intr_count++
          execute intr_handler()
      } else {
          mycpu->intr_count++
          return;
      }

Of course, the above needs some atomicity and ad locking

Does that guarantee that the same sibling handles all interrupts? Since a lot of the infrastructure uses local*_t we're not good with cross-cpu stuff.

Damn what a mess.. we need to serialize enough for both cpus to at least see the overflow bit.. maybe something like:

struct intel_percore {
...
atomic_t uncore_barrier;
};

void uncore_barrier(void)

{
       struct intel_percore *percore = this_cpu_ptr(cpu_hw_events)->percore;
       int armed;

       armed = atomic_cmpxchg(&percore->uncore_barrier, 0, 1) == 0;
       if (armed) {
               /* we armed, it, now wait for completion */
               while (atomic_read(&percore->uncore_barrier))
                       cpu_relax();
       } else {
               /* our sibling must have, decrement it */
               if (atomic_cmpxchg(&percore->uncore_barrier, 1, 0) != 1)
                       BUG();
       }
}

Then have something like:

handle_uncore_interrupt()

{
       u64 overflow = rdmsrl(MSR_UNCORE_PERF_GLOBAL_OVF_STATUS);
       int cpu;

       if (!overflow)
               return 0; /* not our interrupt to handle */

       uncore_barrier(); /* wait so our sibling will also observe the overflow */

       cpu = smp_processor_id();
       if (cpu != cpumask_first(topology_thread_cpumask(cpu)))
               return 1; /* our sibling will handle it, eat the NMI */

       /* OK, we've got an overflow and we're the first CPU in the thread mask */

       ... do fancy stuff ...

       return 1; /* we handled it, eat the NMI */
}

Quoted 1 line

(but I don't think you can use locks in NMI context).

You can, as long as they're never used from !NMI, its icky, but it works.

Quoted 10 lines

This makes me wonder if vectoring uncore to NMI is really needed, given you cannot
correlated to an IP, incl. a kernel IP. If we were to vector to a dedicated (lower prio)
vector, then we could use the trick of saying the lowest indexed CPU in a pair handles the interrupt (because we would already know this is an uncore interrupt).
This would be much simpler. Price: not samples in kernel's critical section. But those
are useless anyway with uncore events.

But the uncore uses the same PMI line, right? You cannot point the uncore to another vector. /me goes find the docs -- ok, its too early in the morning to clearly parse that...

Besides, people asked for the sampling thing didn't they (also we need it to fold the count to avoid overflow on 48bit). Also the PAPI people even want per-task uncore counters because that's the only thing PAPI can do.

Quoted 10 lines · Stephane Eranian

Hi,

Ok, so I have an explanation for what we are seeing. In fact, what bothered me in all
of this is that I did not recall ever running into this problem of double-interrupt with HT
when I implemented the perfmon support for uncore sampling. The reason is in fact
real simple.

Quoted 4 lines

You are getting interrupts on both threads because you have enabled uncore PMU on all CPUs, in uncore_cpu_starting():
+       rdmsrl(MSR_IA32_DEBUGCTLMSR, val);
+       wrmsrl(MSR_IA32_DEBUGCTLMSR, val | DEBUGCTLMSR_ENABLE_UNCORE_PMI);

Yeah! Thanks for the catch.

Quoted 9 lines

You need to do that only on ONE of the two siblings. In fact, you want that only ONCE
per socket. You can do this on the first CPU to  use the uncore to add something a bit
more dynamic and to make sure you have some control over where the overhead is applied.
Once you do that, only one CPU/socket will get the interrupt and all will be fine.

I'll update the patches.

Thanks,
Lin Ming

Quoted 129 lines · Peter Zijlstra

Quoted 125 lines · Stephane Eranian

Quoted 7 lines

Hi,

So I have tested this patch a bit on WSM and as I expected there are issues with sampling.

When HT is on, both siblings CPUs get the interrupt. The HW does not allow you to only point interrupts to a single HT thread (CPU).

Egads, how ugly :/

Quoted 23 lines

I did verify that indeed both threads get the interrupt and that you have a race condition. Both sibling CPUs stop uncore, get the status. They may get the same overflow status. Both will pass the uncore->active_mask because it's shared among siblings cores. Thus,  you have a race for the whole interrupt handler execution.

You need some serialization in there. But the patch does not address this. The problem is different from the back-to-back interrupt issue that Don worked on.
The per-cpu marked/handled trick cannot work to avoid this problem.

You cannot simply say "the lowest indexed" CPU of a sibling pair handles the interrupt
because you don't know if this in an uncore intr, core interrupt or something else. You
need to check. That means each HT thread needs to check uncore ovfl_status. IF the
status is zero, then return. Otherwise, you need to do a 2nd level check before you can
execute the handler. You need to know if the sibling CPU has already "consumed" that
interrupt.

Quoted 11 lines

I think you need some sort of generation counter per physical core and per HT thread.
On interrupt, you could do something along the line of:
      if (mycpu->intr_count == mysibling->intr_count) { then mycpu->intr_count++
          execute intr_handler()
      } else {
          mycpu->intr_count++
          return;
      }
Of course, the above needs some atomicity and ad locking

Does that guarantee that the same sibling handles all interrupts? Since a lot of the infrastructure uses local*_t we're not good with cross-cpu stuff.

Damn what a mess.. we need to serialize enough for both cpus to at least see the overflow bit.. maybe something like:

struct intel_percore {
  ...
  atomic_t uncore_barrier;
};

void uncore_barrier(void)
{
       struct intel_percore *percore = this_cpu_ptr(cpu_hw_events)->percore; int armed;

       armed = atomic_cmpxchg(&percore->uncore_barrier, 0, 1) == 0; if (armed) {
               /* we armed, it, now wait for completion */ while (atomic_read(&percore->uncore_barrier)) cpu_relax();
       } else {
               /* our sibling must have, decrement it */
               if (atomic_cmpxchg(&percore->uncore_barrier, 1, 0) != 1) BUG();
       }
}

Then have something like:

handle_uncore_interrupt()
{
       u64 overflow = rdmsrl(MSR_UNCORE_PERF_GLOBAL_OVF_STATUS); int cpu;

       if (!overflow)
               return 0; /* not our interrupt to handle */

       uncore_barrier(); /* wait so our sibling will also observe the overflow */

       cpu = smp_processor_id();
       if (cpu != cpumask_first(topology_thread_cpumask(cpu))) return 1; /* our sibling will handle it, eat the NMI */

       /* OK, we've got an overflow and we're the first CPU in the thread mask */

       ... do fancy stuff ...

       return 1; /* we handled it, eat the NMI */
}

Quoted 1 line

(but I don't think you can use locks in NMI context).

You can, as long as they're never used from !NMI, its icky, but it works.

Quoted 10 lines

This makes me wonder if vectoring uncore to NMI is really needed, given you cannot
correlated to an IP, incl. a kernel IP. If we were to vector to a dedicated (lower prio)
vector, then we could use the trick of saying the lowest indexed CPU in a pair handles the interrupt (because we would already know this is an uncore interrupt).
This would be much simpler. Price: not samples in kernel's critical section. But those
are useless anyway with uncore events.

But the uncore uses the same PMI line, right? You cannot point the uncore to another vector. /me goes find the docs -- ok, its too early in the morning to clearly parse that...

Besides, people asked for the sampling thing didn't they (also we need it to fold the count to avoid overflow on 48bit). Also the PAPI people even want per-task uncore counters because that's the only thing PAPI can do.

If think there you want something like:

if (cpu == cpumask_first(topology_core_siblings(cpu)) { rdmsrl(MSR_IA32_DEBUGCTLMSR, val);
    wrmsrl(MSR_IA32_DEBUGCTLMSR, val | DEBUGCTLMSR_ENABLE_UNCORE_PMI);
}

Quoted 6 lines

If think there you want something like:

if (cpu == cpumask_first(topology_core_siblings(cpu)) { rdmsrl(MSR_IA32_DEBUGCTLMSR, val); wrmsrl(MSR_IA32_DEBUGCTLMSR, val | DEBUGCTLMSR_ENABLE_UNCORE_PMI); }

You have to be careful with hotplug. The code would need to be rerun on hotplug. Otherwise the interrupt may end up on a downed CPU.

arch/x86/kernel/cpu/perf_event_intel_uncore.c line 534

Quoted 5 lines · Lin Ming

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +5 -0

+       /* Check CPUID signatures: 06_1AH, 06_1EH, 06_1FH */
+       family = boot_cpu_data.x86;
+       model = boot_cpu_data.x86_model;
+       if (family != 6 || (model != 0x1A && model != 0x1E && model != 0x1F))
+               return; 

So that's 26, 30 and 31? Curiously arch/x86/kernel/cpu/perf_event_intel.c does have 31.

Also, could you match the style from perf_event_intel.c?

Quoted 4 lines · Lin Ming

+++ b/arch/x86/kernel/cpu/perf_event.c

(quoted diff) +0 -0

@@ -1362,6 +1357,7 @@ int __init init_hw_perf_events(void)
 

arch/x86/kernel/cpu/perf_event.c int __init init_hw_perf_events(void) · line 1363

Quoted 6 lines

arch/x86/kernel/cpu/perf_event.c +1 -0

        switch (boot_cpu_data.x86_vendor) {
        case X86_VENDOR_INTEL:
+               init_uncore_pmu();
                err = intel_pmu_init();
                break;
        case X86_VENDOR_AMD: 

Since you're otherwise fully separated from the regular cpu driver, could you also grow your own early_initcall() ?

+
+       if (uncore->n_events == 1) {
+               nmi_core = topology_core_id(raw_smp_processor_id());
+               uncore->nmi_core = nmi_core;
+               uncore_pmu_enable_all(nmi_core);
+       }

Lin,

Quoted 7 lines

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +7 -0

+static void uncore_pmu_enable_all(int nmi_core)
+{
+       u64 ctrl;
+
+       ctrl = ((1 << UNCORE_NUM_GENERAL_COUNTERS) - 1) | MSR_UNCORE_PERF_GLOBAL_CTRL_EN_FC0;
+
+       /* Route all interrupts to the first core that accesses uncore */

arch/x86/kernel/cpu/perf_event_intel_uncore.c line 53

Quoted 4 lines

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +4 -0

+       ctrl |= 1ULL << (48 + nmi_core);
+
+       wrmsrl(MSR_UNCORE_PERF_GLOBAL_CTRL, ctrl);
+}

Are you sure nmi_core is always between 0-3 on a 4-core system and 0-5 on a 6-core system?
In other words, is that what topology_core_id(raw_smp_processor_id()) returns?

Note that, unfortunately, I have not seen documentation that says on 6-core system
UNC_GLOBAL_CTRL has 6 interrupt target bits, but it would make sense.

Otherwise, you will get a kernel panic when you wrmsr UNC_GLOBAL_CTRL.

Quoted 6 lines

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +6 -0

+
+       if (uncore->n_events == 1) {
+               nmi_core = topology_core_id(raw_smp_processor_id());
+               uncore->nmi_core = nmi_core;
+               uncore_pmu_enable_all(nmi_core);
+       }

Quoted 1 line · Stephane Eranian

Lin,

Hi, Stephane,

Sorry for late response, I'm just back from vacation.

Quoted 9 lines · Lin Ming

Quoted 7 lines

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +7 -0

+static void uncore_pmu_enable_all(int nmi_core)
+{
+       u64 ctrl;
+
+       ctrl = ((1 << UNCORE_NUM_GENERAL_COUNTERS) - 1) | MSR_UNCORE_PERF_GLOBAL_CTRL_EN_FC0;
+
+       /* Route all interrupts to the first core that accesses uncore */

Quoted 8 lines

arch/x86/kernel/cpu/perf_event_intel_uncore.c line 53

Quoted 4 lines

+       ctrl |= 1ULL << (48 + nmi_core);

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +3 -0

+
+       wrmsrl(MSR_UNCORE_PERF_GLOBAL_CTRL, ctrl);
+}

Are you sure nmi_core is always between 0-3 on a 4-core system and 0-5 on a 6-core system?
In other words, is that what topology_core_id(raw_smp_processor_id()) returns?

I just have a look at a 6-core system, the core id is not 0-5

$ cat /proc/cpuinfo |grep "core id"
core id         : 0
core id         : 1
core id         : 2
core id         : 8
core id         : 9
core id         : 10

So we'd better route all the interrupts to the first core of the socket.

Thanks for the catch.
Lin Ming

Quoted 14 lines

Note that, unfortunately, I have not seen documentation that says on 6-core system
UNC_GLOBAL_CTRL has 6 interrupt target bits, but it would make sense.

Otherwise, you will get a kernel panic when you wrmsr UNC_GLOBAL_CTRL.

Quoted 6 lines

+
+       if (uncore->n_events == 1) {
+               nmi_core = topology_core_id(raw_smp_processor_id());
+               uncore->nmi_core = nmi_core;
+               uncore_pmu_enable_all(nmi_core);
+       }

Quoted 35 lines · Lin Ming

Quoted 1 line · Stephane Eranian

Lin,

Hi, Stephane,

Sorry for late response, I'm just back from vacation.

Quoted 9 lines · Lin Ming

Quoted 7 lines

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +7 -0

+static void uncore_pmu_enable_all(int nmi_core)
+{
+       u64 ctrl;
+
+       ctrl = ((1 << UNCORE_NUM_GENERAL_COUNTERS) - 1) | MSR_UNCORE_PERF_GLOBAL_CTRL_EN_FC0;
+
+       /* Route all interrupts to the first core that accesses uncore */

Quoted 8 lines

arch/x86/kernel/cpu/perf_event_intel_uncore.c line 53

Quoted 4 lines

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +4 -0

+       ctrl |= 1ULL << (48 + nmi_core);
+
+       wrmsrl(MSR_UNCORE_PERF_GLOBAL_CTRL, ctrl);
+}

Are you sure nmi_core is always between 0-3 on a 4-core system and 0-5 on a 6-core system?
In other words, is that what topology_core_id(raw_smp_processor_id()) returns?

I just have a look at a 6-core system, the core id is not 0-5

$ cat /proc/cpuinfo |grep "core id"
core id         : 0
core id         : 1
core id         : 2
core id         : 8
core id         : 9
core id         : 10

Quoted 2 lines

So we'd better route all the interrupts to the first core of the socket.

I recently realized the issue with 0,1,2,8,9,10. At the time I wrote the perfmon support for uncore, those systems did not exist. Sparse APIC id is a major pain for uncore PMU interrupt routine given the way UNC_GLOBAL_CTRL works.

Unfortunately, routing to core 0 (core_cpu_id=0) won't be enough in the presence of HOTPLUG CPU. Imagine I disable the first three 3 cores. Now you the cpu you have to play with are 8,9,10. You need to remap to a number between 0-5.

Quoted 22 lines

Thanks for the catch.
Lin Ming

Quoted 14 lines

Note that, unfortunately, I have not seen documentation that says on 6-core system
UNC_GLOBAL_CTRL has 6 interrupt target bits, but it would make sense.

Otherwise, you will get a kernel panic when you wrmsr UNC_GLOBAL_CTRL.

Quoted 6 lines

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +6 -0

+
+       if (uncore->n_events == 1) {
+               nmi_core = topology_core_id(raw_smp_processor_id());
+               uncore->nmi_core = nmi_core;
+               uncore_pmu_enable_all(nmi_core);
+       }

Quoted 38 lines · Stephane Eranian

Quoted 35 lines · Lin Ming

Quoted 1 line · Stephane Eranian

Lin,

Hi, Stephane,

Sorry for late response, I'm just back from vacation.

Quoted 9 lines · Lin Ming

Quoted 7 lines

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +7 -0

+static void uncore_pmu_enable_all(int nmi_core)
+{
+       u64 ctrl;
+
+       ctrl = ((1 << UNCORE_NUM_GENERAL_COUNTERS) - 1) | MSR_UNCORE_PERF_GLOBAL_CTRL_EN_FC0;
+
+       /* Route all interrupts to the first core that accesses uncore */

Quoted 8 lines

arch/x86/kernel/cpu/perf_event_intel_uncore.c line 53

Quoted 4 lines

+       ctrl |= 1ULL << (48 + nmi_core);

arch/x86/kernel/cpu/perf_event_intel_uncore.c new file +3 -0

+
+       wrmsrl(MSR_UNCORE_PERF_GLOBAL_CTRL, ctrl);
+}

Are you sure nmi_core is always between 0-3 on a 4-core system and 0-5 on a 6-core system?
In other words, is that what topology_core_id(raw_smp_processor_id()) returns?

I just have a look at a 6-core system, the core id is not 0-5

$ cat /proc/cpuinfo |grep "core id"
core id         : 0
core id         : 1
core id         : 2
core id         : 8
core id         : 9
core id         : 10

Quoted 2 lines

So we'd better route all the interrupts to the first core of the socket.

Quoted 8 lines

I recently realized the issue with 0,1,2,8,9,10. At the time I wrote the perfmon support for uncore, those systems did not exist. Sparse APIC id is a major pain for uncore PMU interrupt routine given the way UNC_GLOBAL_CTRL works.

Unfortunately, routing to core 0 (core_cpu_id=0) won't be enough in the presence of HOTPLUG CPU. Imagine I disable the first three 3 cores. Now you the cpu you have to play with are 8,9,10. You need to remap to a number between 0-5.

Good idea.

And I have confirmed that there are 6 interrupt target bits on my 6-core Westmere machine, although documentation does not say this.

u64 val=0x3FULL << 48;
wrmsrl(MSR_UNCORE_PERF_GLOBAL_CTRL, val);

Thanks,
Lin Ming

Quoted 22 lines

Quoted 22 lines

Thanks for the catch.
Lin Ming

Quoted 14 lines

Note that, unfortunately, I have not seen documentation that says on 6-core system
UNC_GLOBAL_CTRL has 6 interrupt target bits, but it would make sense.

Otherwise, you will get a kernel panic when you wrmsr UNC_GLOBAL_CTRL.

Quoted 6 lines

+
+       if (uncore->n_events == 1) {
+               nmi_core = topology_core_id(raw_smp_processor_id());
+               uncore->nmi_core = nmi_core;
+               uncore_pmu_enable_all(nmi_core);
+       }