Re: [PATCH v2 1/2] powerpc/perf: consolidate GPCI hcall structs into asm/hvcall.h
From: Tyrel Datwyler <tyreld@linux.ibm.com>
Date: 2020-08-05 22:39:43
On 7/27/20 11:46 AM, Scott Cheloha wrote:
quoted hunk ↗ jump to hunk
The H_GetPerformanceCounterInfo (GPCI) hypercall input/output structs are useful to modules outside of perf/, so move them into asm/hvcall.h to live alongside the other powerpc hypercall structs. Leave the perf-specific GPCI stuff in perf/hv-gpci.h. Signed-off-by: Scott Cheloha <redacted> --- arch/powerpc/include/asm/hvcall.h | 36 +++++++++++++++++++++++++++++++ arch/powerpc/perf/hv-gpci.c | 9 -------- arch/powerpc/perf/hv-gpci.h | 27 ----------------------- 3 files changed, 36 insertions(+), 36 deletions(-)diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h index e90c073e437e..c338480b4551 100644 --- a/arch/powerpc/include/asm/hvcall.h +++ b/arch/powerpc/include/asm/hvcall.h@@ -527,6 +527,42 @@ struct hv_guest_state { /* Latest version of hv_guest_state structure */ #define HV_GUEST_STATE_VERSION 1 +/* + * From the document "H_GetPerformanceCounterInfo Interface" v1.07 + * + * H_GET_PERF_COUNTER_INFO argument + */ +struct hv_get_perf_counter_info_params { + __be32 counter_request; /* I */ + __be32 starting_index; /* IO */ + __be16 secondary_index; /* IO */ + __be16 returned_values; /* O */ + __be32 detail_rc; /* O, only needed when called via *_norets() */ + + /* + * O, size each of counter_value element in bytes, only set for version + * >= 0x3 + */ + __be16 cv_element_size; + + /* I, 0 (zero) for versions < 0x3 */ + __u8 counter_info_version_in; + + /* O, 0 (zero) if version < 0x3. Must be set to 0 when making hcall */ + __u8 counter_info_version_out; + __u8 reserved[0xC]; + __u8 counter_value[]; +} __packed; + +#define HGPCI_REQ_BUFFER_SIZE 4096 +#define HGPCI_MAX_DATA_BYTES \ + (HGPCI_REQ_BUFFER_SIZE - sizeof(struct hv_get_perf_counter_info_params)) + +struct hv_gpci_request_buffer { + struct hv_get_perf_counter_info_params params; + uint8_t bytes[HGPCI_MAX_DATA_BYTES]; +} __packed; + #endif /* __ASSEMBLY__ */ #endif /* __KERNEL__ */ #endif /* _ASM_POWERPC_HVCALL_H */diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c index 6884d16ec19b..1667315b82e9 100644 --- a/arch/powerpc/perf/hv-gpci.c +++ b/arch/powerpc/perf/hv-gpci.c@@ -123,17 +123,8 @@ static const struct attribute_group *attr_groups[] = { NULL, }; -#define HGPCI_REQ_BUFFER_SIZE 4096 -#define HGPCI_MAX_DATA_BYTES \ - (HGPCI_REQ_BUFFER_SIZE - sizeof(struct hv_get_perf_counter_info_params)) - static DEFINE_PER_CPU(char, hv_gpci_reqb[HGPCI_REQ_BUFFER_SIZE]) __aligned(sizeof(uint64_t)); -struct hv_gpci_request_buffer { - struct hv_get_perf_counter_info_params params; - uint8_t bytes[HGPCI_MAX_DATA_BYTES]; -} __packed; - static unsigned long single_gpci_request(u32 req, u32 starting_index, u16 secondary_index, u8 version_in, u32 offset, u8 length, u64 *value)diff --git a/arch/powerpc/perf/hv-gpci.h b/arch/powerpc/perf/hv-gpci.h index a3053eda5dcc..4d108262bed7 100644 --- a/arch/powerpc/perf/hv-gpci.h +++ b/arch/powerpc/perf/hv-gpci.h@@ -2,33 +2,6 @@ #ifndef LINUX_POWERPC_PERF_HV_GPCI_H_ #define LINUX_POWERPC_PERF_HV_GPCI_H_ -#include <linux/types.h> - -/* From the document "H_GetPerformanceCounterInfo Interface" v1.07 */ - -/* H_GET_PERF_COUNTER_INFO argument */ -struct hv_get_perf_counter_info_params { - __be32 counter_request; /* I */ - __be32 starting_index; /* IO */ - __be16 secondary_index; /* IO */ - __be16 returned_values; /* O */ - __be32 detail_rc; /* O, only needed when called via *_norets() */ - - /* - * O, size each of counter_value element in bytes, only set for version - * >= 0x3 - */ - __be16 cv_element_size; - - /* I, 0 (zero) for versions < 0x3 */ - __u8 counter_info_version_in; - - /* O, 0 (zero) if version < 0x3. Must be set to 0 when making hcall */ - __u8 counter_info_version_out; - __u8 reserved[0xC]; - __u8 counter_value[]; -} __packed; -
Hmm, this pretty much guts this header which normally I'd be inclined to suggest moving the whole thing. The remainder of that header for context:
/* * counter info version => fw version/reference (spec version) *
* 8 => power8 (1.07)
* [7 is skipped by spec 1.07]
* 6 => TLBIE (1.07)
* 5 => v7r7m0.phyp (1.05)
* [4 skipped]
* 3 => v7r6m0.phyp (?)
* [1,2 skipped]
* 0 => v7r{2,3,4}m0.phyp (?)
*/
#define COUNTER_INFO_VERSION_CURRENT 0x8
/* capability mask masks. */
enum {
HV_GPCI_CM_GA = (1 << 7),
HV_GPCI_CM_EXPANDED = (1 << 6),
HV_GPCI_CM_LAB = (1 << 5)
};
#define REQUEST_FILE "../hv-gpci-requests.h"
#define NAME_LOWER hv_gpci
#define NAME_UPPER HV_GPCI
#include "req-gen/perf.h"
#undef REQUEST_FILE
#undef NAME_LOWER
#undef NAME_UPPER
The side effect of moving seems that we would have to drag "hv-gpci-requests.h"
along as well. So, maybe its best just moving the struct as you've done so it
can be used by code outside of perf.
-Tyrel