@@ -27,6 +27,8 @@
#include "hv-24x7-catalog.h"
#include "hv-common.h"
+
+#if 0
static const char *domain_to_index_string(unsigned domain)
{
switch (domain) {
@@ -40,6 +42,7 @@ static const char *domain_to_index_string(unsigned domain)
return "UNKNOWN_DOMAIN_INDEX_STRING";
}
}
+#endif
static const char *event_domain_suffix(unsigned domain)
{
@@ -114,7 +117,8 @@ static bool catalog_entry_domain_is_valid(unsigned domain)
/* u3 0-6, one of HV_24X7_PERF_DOMAIN */
EVENT_DEFINE_RANGE_FORMAT(domain, config, 0, 3);
/* u16 */
-EVENT_DEFINE_RANGE_FORMAT(starting_index, config, 16, 31);
+EVENT_DEFINE_RANGE_FORMAT(core, config, 16, 31);
+EVENT_DEFINE_RANGE_FORMAT(vcpu, config, 16, 31);
/* u32, see "data_offset" */
EVENT_DEFINE_RANGE_FORMAT(offset, config, 32, 63);
/* u16 */
@@ -127,7 +131,8 @@ EVENT_DEFINE_RANGE(reserved3, config2, 0, 63);
static struct attribute *format_attrs[] = {
&format_attr_domain.attr,
&format_attr_offset.attr,
- &format_attr_starting_index.attr,
+ &format_attr_core.attr,
+ &format_attr_vcpu.attr,
&format_attr_lpar.attr,
NULL,
};
@@ -280,19 +285,23 @@ static unsigned core_domains[] = {
static char *event_fmt(struct hv_24x7_event_data *event, unsigned domain)
{
+ const char *sindex;
const char *lpar;
- if (is_physical_domain(domain))
+ if (is_physical_domain(domain)) {
lpar = "0x0";
- else
- lpar = "$sibling_guest_id";
+ sindex = "core";
+ } else {
+ lpar = "?";
+ sindex = "vcpu";
+ }
return kasprintf(GFP_KERNEL,
- "domain=0x%x,offset=0x%x,starting_index=%s,lpar=%s",
+ "domain=0x%x,offset=0x%x,%s=?,lpar=%s",
domain,
be16_to_cpu(event->event_counter_offs) +
be16_to_cpu(event->event_group_record_offs),
- domain_to_index_string(domain),
+ sindex,
lpar);
}
@@ -1061,9 +1070,17 @@ out:
static unsigned long event_24x7_request(struct perf_event *event, u64 *res,
bool success_expected)
{
+ u16 idx;
+ unsigned domain = event_get_domain(event);
+
+ if (is_physical_domain(domain))
+ idx = event_get_core(event);
+ else
+ idx = event_get_vcpu(event);
+
return single_24x7_request(event_get_domain(event),
event_get_offset(event),
- event_get_starting_index(event),
+ idx,
event_get_lpar(event),
res,
success_expected);
@@ -826,7 +826,7 @@ static char *format_alias(char *buf, int len, struct perf_pmu *pmu,
list_for_each_entry(term, &alias->terms, list)
if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
used += snprintf(buf + used, sub_non_neg(len, used),
- ",%s=$%s", term->config,
+ ",%s=%s", term->config,
term->val.str);
if (sub_non_neg(len, used) > 0) {
Jiri Olsa [jolsa@redhat.com] wrote: | anyway we could assign directly to the param term name as you do, | but I think we just need to mark the term as parametrized, like: | | in /sys/bus/event_source/devices/pmu/events/event_name you have: | param2=?,bar=1,param1=? I like the idea of just using a single ? for required parameters, but the problem I had with this approach can be seen with these two sysfs entries: $ cat HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE domain=0x2,offset=0xe0,starting_index=core,lpar=0x0 $ cat HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CORE domain=0x3,offset=0xe0,starting_index=vcpu,lpar=sibling_guest_id The parameter 'starting_index' refers to a core in one event and vcpu in another event. We were trying to give a hint as to what it refers to. Given that, 'starting_index' is not very intuitive, how about discarding starting_index and replacing with what it really means for the event and, use a simple '?' to indicate required parameter). $ cat HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE domain=0x2,offset=0xe0,core=?,lpar=0x0 $ cat HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CORE domain=0x3,offset=0xe0,vcpu=?,lpar=? perf list shows these as: hv_24x7/HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE,core=?/ hv_24x7/HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CHIP,vcpu=?,lpar=?/ command line would be -e hv_24x7/HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE,core=2/ or -e hv_24x7/HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CHIP,vcpu=2,lpar=7/ and would fail if a required parameter is missing. This would eliminate the need for new strings like 'sibling_guest_id' (or as Cody calls it monopolizing strings...) Following quick patch on top of the patchset shows the changes: