| Message ID | 20210226065025.1254973-1-maddy@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V2,1/2] powerpc/perf: Infrastructure to support checking of attr.config* | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (626a6c3d2e20da80aaa710104f34ea6037b28b33) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 32 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Another drive-by review... just some minor nits, below... On Fri, Feb 26, 2021 at 12:20:24PM +0530, Madhavan Srinivasan wrote: > Introduce code to support the checking of attr.config* for > values which are reserved for a given platform. > Performance Monitoring Unit (PMU) configuration registers > have fields that are reserved and specific value to bit field I'd reword to "some specific values for bit fields are reserved". > as reserved. For ex., MMCRA[61:62] is Randome Sampling Mode (SM) s/Randome/Random/ This occurs here, and below, and in patch 2/2. > and value of 0b11 to this field is reserved. s/to/for/ > Writing a non-zero values in these fields or writing invalid > value to bit fields will have unknown behaviours. Suggest: Writing non-zero or invalid values in these fields will have unknown behaviors. (or "behaviours" ;-) PC > Patch adds a generic call-back function "check_attr_config" > in "struct power_pmu", to be called in event_init to > check for attr.config* values for a given platform. > "check_attr_config" is valid only for raw event type. > > Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com> > --- > Changelog v1: > -Fixed commit message and in-code comments > > arch/powerpc/include/asm/perf_event_server.h | 6 ++++++ > arch/powerpc/perf/core-book3s.c | 14 ++++++++++++++ > 2 files changed, 20 insertions(+) > > diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h > index 00e7e671bb4b..dde97d7d9253 100644 > --- a/arch/powerpc/include/asm/perf_event_server.h > +++ b/arch/powerpc/include/asm/perf_event_server.h > @@ -67,6 +67,12 @@ struct power_pmu { > * the pmu supports extended perf regs capability > */ > int capabilities; > + /* > + * Function to check event code for values which are > + * reserved. Function takes struct perf_event as input, > + * since event code could be spread in attr.config* > + */ > + int (*check_attr_config)(struct perf_event *ev); > }; > > /* > diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c > index 6817331e22ff..c6eeb4fdc5fd 100644 > --- a/arch/powerpc/perf/core-book3s.c > +++ b/arch/powerpc/perf/core-book3s.c > @@ -1958,6 +1958,20 @@ static int power_pmu_event_init(struct perf_event *event) > > if (ppmu->blacklist_ev && is_event_blacklisted(ev)) > return -EINVAL; > + /* > + * PMU config registers have fields that are > + * reserved and specific value to bit field as reserved. > + * For ex., MMCRA[61:62] is Randome Sampling Mode (SM) > + * and value of 0b11 to this field is reserved. > + * > + * This check is needed only for raw event type, > + * since tools like fuzzer use raw event type to > + * provide randomized event code values for test. > + * > + */ > + if (ppmu->check_attr_config && > + ppmu->check_attr_config(event)) > + return -EINVAL; > break; > default: > return -ENOENT; > -- > 2.26.2 >
On 2/26/21 7:33 PM, Paul A. Clarke wrote: > Another drive-by review... just some minor nits, below... > > On Fri, Feb 26, 2021 at 12:20:24PM +0530, Madhavan Srinivasan wrote: >> Introduce code to support the checking of attr.config* for >> values which are reserved for a given platform. >> Performance Monitoring Unit (PMU) configuration registers >> have fields that are reserved and specific value to bit field > I'd reword to "some specific values for bit fields are reserved". > >> as reserved. For ex., MMCRA[61:62] is Randome Sampling Mode (SM) > s/Randome/Random/ > This occurs here, and below, and in patch 2/2. > >> and value of 0b11 to this field is reserved. > s/to/for/ > >> Writing a non-zero values in these fields or writing invalid >> value to bit fields will have unknown behaviours. > Suggest: Writing non-zero or invalid values in these fields > will have unknown behaviors. (or "behaviours" ;-) > > PC Thanks for the review. Will fix it. Maddy > >> Patch adds a generic call-back function "check_attr_config" >> in "struct power_pmu", to be called in event_init to >> check for attr.config* values for a given platform. >> "check_attr_config" is valid only for raw event type. >> >> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com> >> --- >> Changelog v1: >> -Fixed commit message and in-code comments >> >> arch/powerpc/include/asm/perf_event_server.h | 6 ++++++ >> arch/powerpc/perf/core-book3s.c | 14 ++++++++++++++ >> 2 files changed, 20 insertions(+) >> >> diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h >> index 00e7e671bb4b..dde97d7d9253 100644 >> --- a/arch/powerpc/include/asm/perf_event_server.h >> +++ b/arch/powerpc/include/asm/perf_event_server.h >> @@ -67,6 +67,12 @@ struct power_pmu { >> * the pmu supports extended perf regs capability >> */ >> int capabilities; >> + /* >> + * Function to check event code for values which are >> + * reserved. Function takes struct perf_event as input, >> + * since event code could be spread in attr.config* >> + */ >> + int (*check_attr_config)(struct perf_event *ev); >> }; >> >> /* >> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c >> index 6817331e22ff..c6eeb4fdc5fd 100644 >> --- a/arch/powerpc/perf/core-book3s.c >> +++ b/arch/powerpc/perf/core-book3s.c >> @@ -1958,6 +1958,20 @@ static int power_pmu_event_init(struct perf_event *event) >> >> if (ppmu->blacklist_ev && is_event_blacklisted(ev)) >> return -EINVAL; >> + /* >> + * PMU config registers have fields that are >> + * reserved and specific value to bit field as reserved. >> + * For ex., MMCRA[61:62] is Randome Sampling Mode (SM) >> + * and value of 0b11 to this field is reserved. >> + * >> + * This check is needed only for raw event type, >> + * since tools like fuzzer use raw event type to >> + * provide randomized event code values for test. >> + * >> + */ >> + if (ppmu->check_attr_config && >> + ppmu->check_attr_config(event)) >> + return -EINVAL; >> break; >> default: >> return -ENOENT; >> -- >> 2.26.2 >>
diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h index 00e7e671bb4b..dde97d7d9253 100644 --- a/arch/powerpc/include/asm/perf_event_server.h +++ b/arch/powerpc/include/asm/perf_event_server.h @@ -67,6 +67,12 @@ struct power_pmu { * the pmu supports extended perf regs capability */ int capabilities; + /* + * Function to check event code for values which are + * reserved. Function takes struct perf_event as input, + * since event code could be spread in attr.config* + */ + int (*check_attr_config)(struct perf_event *ev); }; /* diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c index 6817331e22ff..c6eeb4fdc5fd 100644 --- a/arch/powerpc/perf/core-book3s.c +++ b/arch/powerpc/perf/core-book3s.c @@ -1958,6 +1958,20 @@ static int power_pmu_event_init(struct perf_event *event) if (ppmu->blacklist_ev && is_event_blacklisted(ev)) return -EINVAL; + /* + * PMU config registers have fields that are + * reserved and specific value to bit field as reserved. + * For ex., MMCRA[61:62] is Randome Sampling Mode (SM) + * and value of 0b11 to this field is reserved. + * + * This check is needed only for raw event type, + * since tools like fuzzer use raw event type to + * provide randomized event code values for test. + * + */ + if (ppmu->check_attr_config && + ppmu->check_attr_config(event)) + return -EINVAL; break; default: return -ENOENT;
Introduce code to support the checking of attr.config* for values which are reserved for a given platform. Performance Monitoring Unit (PMU) configuration registers have fields that are reserved and specific value to bit field as reserved. For ex., MMCRA[61:62] is Randome Sampling Mode (SM) and value of 0b11 to this field is reserved. Writing a non-zero values in these fields or writing invalid value to bit fields will have unknown behaviours. Patch adds a generic call-back function "check_attr_config" in "struct power_pmu", to be called in event_init to check for attr.config* values for a given platform. "check_attr_config" is valid only for raw event type. Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com> --- Changelog v1: -Fixed commit message and in-code comments arch/powerpc/include/asm/perf_event_server.h | 6 ++++++ arch/powerpc/perf/core-book3s.c | 14 ++++++++++++++ 2 files changed, 20 insertions(+)