| Message ID | 20231130204343.503076-1-sudeep.holla@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] firmware: arm_scmi: Fix frequency truncation by promoting multiplier to u64 | expand |
On Thu, Nov 30, 2023 at 08:43:42PM +0000, Sudeep Holla wrote: > Fix the frequency truncation for all values equal to or greater 4GHz by > updating the multiplier 'mult_factor' to u64 type. It is also possible > that the multiplier itself can be greater than or equal to 2^32. So we need > to also fix the equation computing the value of the multiplier. > > Fixes: a9e3fbfaa0ff ("firmware: arm_scmi: add initial support for performance protocol") > Reported-by: Sibi Sankar <quic_sibis@quicinc.com> > Closes: https://lore.kernel.org/all/20231129065748.19871-3-quic_sibis@quicinc.com/ > Cc: Cristian Marussi <cristian.marussi@arm.com> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > drivers/firmware/arm_scmi/perf.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > index 81dd5c5e5533..8ce449922e55 100644 > --- a/drivers/firmware/arm_scmi/perf.c > +++ b/drivers/firmware/arm_scmi/perf.c > @@ -152,7 +152,7 @@ struct perf_dom_info { > u32 opp_count; > u32 sustained_freq_khz; > u32 sustained_perf_level; > - u32 mult_factor; > + u64 mult_factor; I have now changed this to unsigned long instead of u64 to fix the 32-bit build failure[1].
On Fri, Dec 01, 2023 at 02:39:35PM +0000, Sudeep Holla wrote: > On Thu, Nov 30, 2023 at 08:43:42PM +0000, Sudeep Holla wrote: > > Fix the frequency truncation for all values equal to or greater 4GHz by > > updating the multiplier 'mult_factor' to u64 type. It is also possible > > that the multiplier itself can be greater than or equal to 2^32. So we need > > to also fix the equation computing the value of the multiplier. > > > > Fixes: a9e3fbfaa0ff ("firmware: arm_scmi: add initial support for performance protocol") > > Reported-by: Sibi Sankar <quic_sibis@quicinc.com> > > Closes: https://lore.kernel.org/all/20231129065748.19871-3-quic_sibis@quicinc.com/ > > Cc: Cristian Marussi <cristian.marussi@arm.com> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > --- > > drivers/firmware/arm_scmi/perf.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > > index 81dd5c5e5533..8ce449922e55 100644 > > --- a/drivers/firmware/arm_scmi/perf.c > > +++ b/drivers/firmware/arm_scmi/perf.c > > @@ -152,7 +152,7 @@ struct perf_dom_info { > > u32 opp_count; > > u32 sustained_freq_khz; > > u32 sustained_perf_level; > > - u32 mult_factor; > > + u64 mult_factor; > > I have now changed this to unsigned long instead of u64 to fix the 32-bit > build failure[1]. Right, I was caught a few times too by this kind of failures on v7 :D ... but this 32bit issue makes me wonder what to do in such a case... ...I mean, on 32bit if the calculated freq oveflows, there is just nothing we can do on v7 without overcomplicating the code...but I suppose it is unplausible to have such high freq on a v7... as a palliative I can only think of some sort of overflow check (only on v7) that could trigger a warning ... but it is hardly worth the effort probably.. Thanks, Cristian
On Fri, Dec 01, 2023 at 04:17:56PM +0000, Cristian Marussi wrote: > On Fri, Dec 01, 2023 at 02:39:35PM +0000, Sudeep Holla wrote: > > On Thu, Nov 30, 2023 at 08:43:42PM +0000, Sudeep Holla wrote: > > > Fix the frequency truncation for all values equal to or greater 4GHz by > > > updating the multiplier 'mult_factor' to u64 type. It is also possible > > > that the multiplier itself can be greater than or equal to 2^32. So we need > > > to also fix the equation computing the value of the multiplier. > > > > > > Fixes: a9e3fbfaa0ff ("firmware: arm_scmi: add initial support for performance protocol") > > > Reported-by: Sibi Sankar <quic_sibis@quicinc.com> > > > Closes: https://lore.kernel.org/all/20231129065748.19871-3-quic_sibis@quicinc.com/ > > > Cc: Cristian Marussi <cristian.marussi@arm.com> > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > > --- > > > drivers/firmware/arm_scmi/perf.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > > > index 81dd5c5e5533..8ce449922e55 100644 > > > --- a/drivers/firmware/arm_scmi/perf.c > > > +++ b/drivers/firmware/arm_scmi/perf.c > > > @@ -152,7 +152,7 @@ struct perf_dom_info { > > > u32 opp_count; > > > u32 sustained_freq_khz; > > > u32 sustained_perf_level; > > > - u32 mult_factor; > > > + u64 mult_factor; > > > > I have now changed this to unsigned long instead of u64 to fix the 32-bit > > build failure[1]. > > Right, I was caught a few times too by this kind of failures on v7 :D >
On Thu, 30 Nov 2023 20:43:42 +0000, Sudeep Holla wrote: > Fix the frequency truncation for all values equal to or greater 4GHz by > updating the multiplier 'mult_factor' to u64 type. It is also possible > that the multiplier itself can be greater than or equal to 2^32. So we need > to also fix the equation computing the value of the multiplier. > Applied to sudeep.holla/linux (for-next/scmi/fixes), thanks! [1/2] firmware: arm_scmi: Fix frequency truncation by promoting multiplier to u64 (Applied changing u64 to unsigned long to fix armv7 build) https://git.kernel.org/sudeep.holla/c/8e3c98d9187e [2/2] firmware: arm_scmi: Fix possible frequency truncation when using level indexing mode https://git.kernel.org/sudeep.holla/c/77f5032e94f2 -- Regards, Sudeep
diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index 81dd5c5e5533..8ce449922e55 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -152,7 +152,7 @@ struct perf_dom_info { u32 opp_count; u32 sustained_freq_khz; u32 sustained_perf_level; - u32 mult_factor; + u64 mult_factor; struct scmi_perf_domain_info info; struct scmi_opp opp[MAX_OPPS]; struct scmi_fc_info *fc_info; @@ -273,8 +273,8 @@ scmi_perf_domain_attributes_get(const struct scmi_protocol_handle *ph, dom_info->mult_factor = 1000; else dom_info->mult_factor = - (dom_info->sustained_freq_khz * 1000) / - dom_info->sustained_perf_level; + (dom_info->sustained_freq_khz * 1000UL) + / dom_info->sustained_perf_level; strscpy(dom_info->info.name, attr->name, SCMI_SHORT_NAME_MAX_SIZE); }
Fix the frequency truncation for all values equal to or greater 4GHz by updating the multiplier 'mult_factor' to u64 type. It is also possible that the multiplier itself can be greater than or equal to 2^32. So we need to also fix the equation computing the value of the multiplier. Fixes: a9e3fbfaa0ff ("firmware: arm_scmi: add initial support for performance protocol") Reported-by: Sibi Sankar <quic_sibis@quicinc.com> Closes: https://lore.kernel.org/all/20231129065748.19871-3-quic_sibis@quicinc.com/ Cc: Cristian Marussi <cristian.marussi@arm.com> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/arm_scmi/perf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- 2.43.0