On Thu, Aug 31, 2017 at 02:26:22PM +0100, Suzuki K Poulose wrote:

On 31/08/17 12:35, Jan Glauber wrote:

quoted

On Wed, Aug 30, 2017 at 11:03:00AM +0100, Suzuki K Poulose wrote:

quoted

On 29/08/17 14:12, Jan Glauber wrote:

[...]

quoted

quoted

quoted

+/* LMC events */
+#define LMC_EVENT_IFB_CNT		0x1d0
+#define LMC_EVENT_OPS_CNT		0x1d8
+#define LMC_EVENT_DCLK_CNT		0x1e0
+#define LMC_EVENT_BANK_CONFLICT1	0x360
+#define LMC_EVENT_BANK_CONFLICT2	0x368
+
+#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
+	&((struct perf_pmu_events_attr[]) {						\
+		{									\
+			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
+			_id,								\
+			"lmc_event=" __stringify(_id),					\
+		}									\
+	})[0].attr.attr
+
+/* map counter numbers to register offsets */
+static int lmc_events[] = {
+	LMC_EVENT_IFB_CNT,
+	LMC_EVENT_OPS_CNT,
+	LMC_EVENT_DCLK_CNT,
+	LMC_EVENT_BANK_CONFLICT1,
+	LMC_EVENT_BANK_CONFLICT2,
+};
+
+static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
+			   lmc_events[hwc->config]);
+}
+

Is there any reason why we can't use the LMC event code directly
here, avoiding the mapping altogether ?

I wanted to avoid exposing the raw numbers (0x1d0 - 0x368) here.

Thats the primarily the reason why we expose the "aliases" in events/.
The other problem with adding another layer of mapping is, you are preventing
someone from actually mapping the raw code used by the perf tool (which is now
a mapping index) to the real raw code used by the hardware unless they have
the kernel source handy. If you choose to expose the raw numbers, like *all*
the other PMUs, the user can map it by looking up the manual.

So what would that do to the config bits? Currently they are:
PMU_FORMAT_ATTR(lmc_event, "config:0-2");

Should I have config:0-9 then? Wouldn't that be confusing as there are
only 5 events?

Also I need to be very careful as we need to prevent a user from
accessing anything else then the counters. I can do that with the
event_valid callback though.

thanks,
Jan

Cheers
Suzuki