Re: [PATCH 6/8] powerpc/perf: add support for the hv gpci (get performance counter info) interface
On 01/31/2014 09:58 PM, Michael Ellerman wrote:
On Thu, 2014-16-01 at 23:53:52 UTC, Cody P Schafer wrote:
This provides a basic link between perf and hv_gpci. Notably, it does
not yet support transactions and does not list any events (they can
still be manually composed).
What are the plans for listing?
I'm looking at extending the sysfs api for listing perf events. We can't
use the existing one as it doesn't let us parametrize the events by
anything, meaning we'd need to list an essentially duplicate event for
each cpu/core/chip and lpar (guest). The duplication in cpu/core/chip
comes from not using the typical cpu parameter to perf_event_open()
(which we don't do because it wouldn't make sense when the guest
monitored isn't us).
The manual compose is nice but pretty hairy to use in practice I would think.
diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
new file mode 100644
index 000..31d9d59
--- /dev/null
+++ b/arch/powerpc/perf/hv-gpci.c
@@ -0,0 +1,235 @@
+/*
+ * Hypervisor supplied "gpci" ("get performance counter info") performance
+ * counter support
+ *
+ * Author: Cody P Schafer
+ * Copyright 2014 IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+#define pr_fmt(fmt) "hv-gpci: " fmt
+
+#include
+#include
+#include
+#include
+#include
+#include
Needed?
asm/io.h is for virt_to_phys(). And yes, I need it.
+/* See arch/powerpc/include/asm/hv_gpci.h for details on the hcall interface */
+
+PMU_RANGE_ATTR(request, config, 0, 31); /* u32 */
+PMU_RANGE_ATTR(starting_index, config, 32, 63); /* u32 */
+PMU_RANGE_ATTR(secondary_index, config1, 0, 15); /* u16 */
+PMU_RANGE_ATTR(counter_info_version, config1, 16, 23); /* u8 */
+PMU_RANGE_ATTR(length, config1, 24, 31); /* u8, bytes of data (1-8) */
+PMU_RANGE_ATTR(offset, config1, 32, 63); /* u32, byte offset */
+
+static struct attribute *format_attr[] = {
+ &format_attr_request.attr,
+ &format_attr_starting_index.attr,
+ &format_attr_secondary_index.attr,
+ &format_attr_counter_info_version.attr,
+
Lonley blank line.
Which was seperating "real" attributes from those provided by the kernel
(gpci doesn't know about "offset" and "length"). I'll remove.
+ &format_attr_offset.attr,
+ &format_attr_length.attr,
+ NULL,
+};
+
+static struct attribute_group format_group = {
+ .name = "format",
+ .attrs = format_attr,
+};
+
+static const struct attribute_group *attr_groups[] = {
+ &format_group,
+ NULL,
+};
+
+static unsigned long single_gpci_request(u32 req, u32 starting_index,
+ u16 secondary_index, u8 version_in, u32 offset, u8 length,
+ u64 *value)
Passing the event and extracting the values in here would be neater IMHO.
Well, I'll at least add a wrapper that does that. The idea here was to
separate the perf specific logic from the gpci specific logic. And I'll
end up taking advantage of that separation when doing the interface
probing (mentioned way down near the end of this email).
+{
+ unsigned long ret;
+ size_t i;
+ u64 count;
+
+ struct {
+ struct hv_get_perf_counter_info_params params;
+ union {
+ union h_gpci_cvs data;
+ uint8_t bytes[sizeof(union h_gpci_cvs)];
+ };
+ } arg = {
+ .params = {
+ .counter_request = cpu_to_be32(req),
+ .starting_index = cpu_to_be32(starting_index),
+ .secondary_index = cpu_to_be16(secondary_index),
+ .counter_info_version_in = version_in,
+ }
+ };
+
+ ret = plpar_hcall_norets(H_GET_PERF_COUNTER_INFO,
+ virt_to_phys(&arg), sizeof(arg));
+ if (ret) {
+ pr_devel("hcall failed: 0x%lx\n", ret);
+ return ret;
+ }
+
+ /*
+* we verify offset and length are within the zeroed buffer at event
+* init.
+*/
+ count = 0;
+ for (i = offset; i < offset + length; i++)
+ count |= arg.bytes[i] << (i - offset);
+
+ *value = count;
+ return ret;
+}
+
+static u64 h_gpci_get_value(struct perf_event *event)
+{
+ u64 count;
+ unsigned long ret = single_gpci_request(event_get_request(event),
+ event_get_starting_index(event),
+ event_get_secondary_index(event),
+ event_get_counter_info_version(event),
+ event_get_offset(event),
+ event_get_length(event),
+
Re: [PATCH 6/8] powerpc/perf: add support for the hv gpci (get performance counter info) interface
On Thu, 2014-16-01 at 23:53:52 UTC, Cody P Schafer wrote:
> This provides a basic link between perf and hv_gpci. Notably, it does
> not yet support transactions and does not list any events (they can
> still be manually composed).
What are the plans for listing?
The manual compose is nice but pretty hairy to use in practice I would think.
> diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
> new file mode 100644
> index 000..31d9d59
> --- /dev/null
> +++ b/arch/powerpc/perf/hv-gpci.c
> @@ -0,0 +1,235 @@
> +/*
> + * Hypervisor supplied "gpci" ("get performance counter info") performance
> + * counter support
> + *
> + * Author: Cody P Schafer
> + * Copyright 2014 IBM Corporation.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +#define pr_fmt(fmt) "hv-gpci: " fmt
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
Needed?
> +/* See arch/powerpc/include/asm/hv_gpci.h for details on the hcall interface
> */
> +
> +PMU_RANGE_ATTR(request, config, 0, 31); /* u32 */
> +PMU_RANGE_ATTR(starting_index, config, 32, 63); /* u32 */
> +PMU_RANGE_ATTR(secondary_index, config1, 0, 15); /* u16 */
> +PMU_RANGE_ATTR(counter_info_version, config1, 16, 23); /* u8 */
> +PMU_RANGE_ATTR(length, config1, 24, 31); /* u8, bytes of data (1-8) */
> +PMU_RANGE_ATTR(offset, config1, 32, 63); /* u32, byte offset */
> +
> +static struct attribute *format_attr[] = {
> + &format_attr_request.attr,
> + &format_attr_starting_index.attr,
> + &format_attr_secondary_index.attr,
> + &format_attr_counter_info_version.attr,
> +
Lonley blank line.
> + &format_attr_offset.attr,
> + &format_attr_length.attr,
> + NULL,
> +};
> +
> +static struct attribute_group format_group = {
> + .name = "format",
> + .attrs = format_attr,
> +};
> +
> +static const struct attribute_group *attr_groups[] = {
> + &format_group,
> + NULL,
> +};
> +
> +static unsigned long single_gpci_request(u32 req, u32 starting_index,
> + u16 secondary_index, u8 version_in, u32 offset, u8 length,
> + u64 *value)
Passing the event and extracting the values in here would be neater IMHO.
> +{
> + unsigned long ret;
> + size_t i;
> + u64 count;
> +
> + struct {
> + struct hv_get_perf_counter_info_params params;
> + union {
> + union h_gpci_cvs data;
> + uint8_t bytes[sizeof(union h_gpci_cvs)];
> + };
> + } arg = {
> + .params = {
> + .counter_request = cpu_to_be32(req),
> + .starting_index = cpu_to_be32(starting_index),
> + .secondary_index = cpu_to_be16(secondary_index),
> + .counter_info_version_in = version_in,
> + }
> + };
> +
> + ret = plpar_hcall_norets(H_GET_PERF_COUNTER_INFO,
> + virt_to_phys(&arg), sizeof(arg));
> + if (ret) {
> + pr_devel("hcall failed: 0x%lx\n", ret);
> + return ret;
> + }
> +
> + /*
> + * we verify offset and length are within the zeroed buffer at event
> + * init.
> + */
> + count = 0;
> + for (i = offset; i < offset + length; i++)
> + count |= arg.bytes[i] << (i - offset);
> +
> + *value = count;
> + return ret;
> +}
> +
> +static u64 h_gpci_get_value(struct perf_event *event)
> +{
> + u64 count;
> + unsigned long ret = single_gpci_request(event_get_request(event),
> + event_get_starting_index(event),
> + event_get_secondary_index(event),
> + event_get_counter_info_version(event),
> + event_get_offset(event),
> + event_get_length(event),
> + &count);
> + if (ret)
> + return 0;
> + return count;
> +}
> +
> +static void h_gpci_event_update(struct perf_event *event)
> +{
> + s64 prev;
> + u64 now = h_gpci_get_value(event);
> + prev = local64_xchg(&event->hw.prev_count, now);
> + local64_add(now - prev, &event->count);
> +}
> +
> +static void h_gpci_event_start(struct perf_event *event, int flags)
> +{
> + local64_set(&event->hw.prev_count, h_gpci_get_value(event));
> + perf_swevent_start_hrtimer(event);
> +}
> +
> +static void h_gpci_event_stop(struct perf_event *event, int flags)
> +{
> + perf_swevent_cancel_hrtimer(event);
> + h_gpci_event_update(event);
> +}
> +
> +static int h_gpci_event_add(struct perf_event *event, int flags)
> +{
> + if (flags & PERF_EF_START)
> + h_gpci_event_start(event, flags);
> +
> +
