| Message ID | 20230704050238.712223-1-rui.zhang@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | powercap: intel_rapl_tpmi: Fix a sparse warning | expand |
From: Rafael J. Wysocki > Sent: 11 July 2023 19:14 ... > On Tue, Jul 4, 2023 at 7:02 AM Zhang Rui <rui.zhang@intel.com> wrote: > > > > Fix a sparse warning in intel_rapl_tpmi driver. > > > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: warning: incorrect type in initializer > (different address spaces) > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: expected unsigned long long [usertype] > *tpmi_rapl_regs > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: got void [noderef] __iomem * > > > > Fixes: 9eef7f9da928 ("powercap: intel_rapl: Introduce RAPL TPMI interface driver") > > Reported-by: kernel test robot <lkp@intel.com> > > Closes: https://lore.kernel.org/oe-kbuild-all/202307031405.dy3druuy-lkp@intel.com/ > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > --- > > drivers/powercap/intel_rapl_tpmi.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/powercap/intel_rapl_tpmi.c b/drivers/powercap/intel_rapl_tpmi.c > > index 4f4f13ded225..05f664a096ab 100644 > > --- a/drivers/powercap/intel_rapl_tpmi.c > > +++ b/drivers/powercap/intel_rapl_tpmi.c > > @@ -138,7 +138,7 @@ static int parse_one_domain(struct tpmi_rapl_package *trp, u32 offset) > > enum tpmi_rapl_register reg_index; > > enum rapl_domain_reg_id reg_id; > > int tpmi_domain_size, tpmi_domain_flags; > > - u64 *tpmi_rapl_regs = trp->base + offset; > > + u64 *tpmi_rapl_regs = (u64 *)(trp->base + offset); > > u64 tpmi_domain_header = readq((void __iomem *)tpmi_rapl_regs); Isn't the correct fix to add __iomem to the definition of tpmi_rapi_regs and also remove the cast from the following readq() line? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Fri, Jul 14, 2023 at 3:34 PM David Laight <David.Laight@aculab.com> wrote: > > From: Rafael J. Wysocki > > Sent: 11 July 2023 19:14 > ... > > On Tue, Jul 4, 2023 at 7:02 AM Zhang Rui <rui.zhang@intel.com> wrote: > > > > > > Fix a sparse warning in intel_rapl_tpmi driver. > > > > > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: warning: incorrect type in initializer > > (different address spaces) > > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: expected unsigned long long [usertype] > > *tpmi_rapl_regs > > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: got void [noderef] __iomem * > > > > > > Fixes: 9eef7f9da928 ("powercap: intel_rapl: Introduce RAPL TPMI interface driver") > > > Reported-by: kernel test robot <lkp@intel.com> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202307031405.dy3druuy-lkp@intel.com/ > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > > --- > > > drivers/powercap/intel_rapl_tpmi.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/powercap/intel_rapl_tpmi.c b/drivers/powercap/intel_rapl_tpmi.c > > > index 4f4f13ded225..05f664a096ab 100644 > > > --- a/drivers/powercap/intel_rapl_tpmi.c > > > +++ b/drivers/powercap/intel_rapl_tpmi.c > > > @@ -138,7 +138,7 @@ static int parse_one_domain(struct tpmi_rapl_package *trp, u32 offset) > > > enum tpmi_rapl_register reg_index; > > > enum rapl_domain_reg_id reg_id; > > > int tpmi_domain_size, tpmi_domain_flags; > > > - u64 *tpmi_rapl_regs = trp->base + offset; > > > + u64 *tpmi_rapl_regs = (u64 *)(trp->base + offset); > > > u64 tpmi_domain_header = readq((void __iomem *)tpmi_rapl_regs); > > Isn't the correct fix to add __iomem to the definition of > tpmi_rapi_regs and also remove the cast from the following > readq() line? Good point. Rui, can you take another look at this, please?
Hi, David, Thanks for pointing this out, after digging into the problem further, I'm confused about how to address all the sparse warnings. In the RAPL framework, u64 regs[RAPL_DOMAIN_MAX][RAPL_DOMAIN_REG_MAX] is used to save the RAPL register addresses, because the register can either be a MMIO register or MSR. With sparse enabled, this becomes a problem for MMIO registers because we will get a warning anyway when saving an __iomem address to u64 regs[][], right? This problem is not new. The same warning is also there when compiling drivers/thermal/intel/int340x_thermal/processor_thermal_rapl.c, which also uses MMIO RAPL register. Any idea how to fix this? thanks, rui On Fri, 2023-07-14 at 16:39 +0200, Rafael J. Wysocki wrote: > On Fri, Jul 14, 2023 at 3:34 PM David Laight > <David.Laight@aculab.com> wrote: > > > > From: Rafael J. Wysocki > > > Sent: 11 July 2023 19:14 > > ... > > > On Tue, Jul 4, 2023 at 7:02 AM Zhang Rui <rui.zhang@intel.com> > > > wrote: > > > > > > > > Fix a sparse warning in intel_rapl_tpmi driver. > > > > > > > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: warning: > > > > incorrect type in initializer > > > (different address spaces) > > > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: > > > > expected unsigned long long [usertype] > > > *tpmi_rapl_regs > > > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: got > > > > void [noderef] __iomem * > > > > > > > > Fixes: 9eef7f9da928 ("powercap: intel_rapl: Introduce RAPL TPMI > > > > interface driver") > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > Closes: > > > > https://lore.kernel.org/oe-kbuild-all/202307031405.dy3druuy-lkp@intel.com/ > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > > > --- > > > > drivers/powercap/intel_rapl_tpmi.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/powercap/intel_rapl_tpmi.c > > > > b/drivers/powercap/intel_rapl_tpmi.c > > > > index 4f4f13ded225..05f664a096ab 100644 > > > > --- a/drivers/powercap/intel_rapl_tpmi.c > > > > +++ b/drivers/powercap/intel_rapl_tpmi.c > > > > @@ -138,7 +138,7 @@ static int parse_one_domain(struct > > > > tpmi_rapl_package *trp, u32 offset) > > > > enum tpmi_rapl_register reg_index; > > > > enum rapl_domain_reg_id reg_id; > > > > int tpmi_domain_size, tpmi_domain_flags; > > > > - u64 *tpmi_rapl_regs = trp->base + offset; > > > > + u64 *tpmi_rapl_regs = (u64 *)(trp->base + offset); > > > > u64 tpmi_domain_header = readq((void __iomem > > > > *)tpmi_rapl_regs); > > > > Isn't the correct fix to add __iomem to the definition of > > tpmi_rapi_regs and also remove the cast from the following > > readq() line? > > Good point. > > Rui, can you take another look at this, please?
From: Zhang, Rui > Sent: 17 July 2023 10:13 > > Hi, David, > > Thanks for pointing this out, after digging into the problem further, > I'm confused about how to address all the sparse warnings. > > In the RAPL framework, u64 regs[RAPL_DOMAIN_MAX][RAPL_DOMAIN_REG_MAX] > is used to save the RAPL register addresses, because the register can > either be a MMIO register or MSR. > > With sparse enabled, this becomes a problem for MMIO registers because > we will get a warning anyway when saving an __iomem address to u64 > regs[][], right? To avoid those issues (which seem different from the one here) you could use a union of the __iomem address and the MSR number. That ought to let you remove all the __iomem casts. David > > This problem is not new. The same warning is also there when compiling > drivers/thermal/intel/int340x_thermal/processor_thermal_rapl.c, which > also uses MMIO RAPL register. > > Any idea how to fix this? > > thanks, > rui > > On Fri, 2023-07-14 at 16:39 +0200, Rafael J. Wysocki wrote: > > On Fri, Jul 14, 2023 at 3:34 PM David Laight > > <David.Laight@aculab.com> wrote: > > > > > > From: Rafael J. Wysocki > > > > Sent: 11 July 2023 19:14 > > > ... > > > > On Tue, Jul 4, 2023 at 7:02 AM Zhang Rui <rui.zhang@intel.com> > > > > wrote: > > > > > > > > > > Fix a sparse warning in intel_rapl_tpmi driver. > > > > > > > > > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: warning: > > > > > incorrect type in initializer > > > > (different address spaces) > > > > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: > > > > > expected unsigned long long [usertype] > > > > *tpmi_rapl_regs > > > > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: got > > > > > void [noderef] __iomem * > > > > > > > > > > Fixes: 9eef7f9da928 ("powercap: intel_rapl: Introduce RAPL TPMI > > > > > interface driver") > > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > > Closes: > > > > > https://lore.kernel.org/oe-kbuild-all/202307031405.dy3druuy-lkp@intel.com/ > > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > > > > --- > > > > > drivers/powercap/intel_rapl_tpmi.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/powercap/intel_rapl_tpmi.c > > > > > b/drivers/powercap/intel_rapl_tpmi.c > > > > > index 4f4f13ded225..05f664a096ab 100644 > > > > > --- a/drivers/powercap/intel_rapl_tpmi.c > > > > > +++ b/drivers/powercap/intel_rapl_tpmi.c > > > > > @@ -138,7 +138,7 @@ static int parse_one_domain(struct > > > > > tpmi_rapl_package *trp, u32 offset) > > > > > enum tpmi_rapl_register reg_index; > > > > > enum rapl_domain_reg_id reg_id; > > > > > int tpmi_domain_size, tpmi_domain_flags; > > > > > - u64 *tpmi_rapl_regs = trp->base + offset; > > > > > + u64 *tpmi_rapl_regs = (u64 *)(trp->base + offset); > > > > > u64 tpmi_domain_header = readq((void __iomem > > > > > *)tpmi_rapl_regs); > > > > > > Isn't the correct fix to add __iomem to the definition of > > > tpmi_rapi_regs and also remove the cast from the following > > > readq() line? > > > > Good point. > > > > Rui, can you take another look at this, please? - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
diff --git a/drivers/powercap/intel_rapl_tpmi.c b/drivers/powercap/intel_rapl_tpmi.c index 4f4f13ded225..05f664a096ab 100644 --- a/drivers/powercap/intel_rapl_tpmi.c +++ b/drivers/powercap/intel_rapl_tpmi.c @@ -138,7 +138,7 @@ static int parse_one_domain(struct tpmi_rapl_package *trp, u32 offset) enum tpmi_rapl_register reg_index; enum rapl_domain_reg_id reg_id; int tpmi_domain_size, tpmi_domain_flags; - u64 *tpmi_rapl_regs = trp->base + offset; + u64 *tpmi_rapl_regs = (u64 *)(trp->base + offset); u64 tpmi_domain_header = readq((void __iomem *)tpmi_rapl_regs); /* Domain Parent bits are ignored for now */
Fix a sparse warning in intel_rapl_tpmi driver. ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: warning: incorrect type in initializer (different address spaces) ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: expected unsigned long long [usertype] *tpmi_rapl_regs ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: got void [noderef] __iomem * Fixes: 9eef7f9da928 ("powercap: intel_rapl: Introduce RAPL TPMI interface driver") Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202307031405.dy3druuy-lkp@intel.com/ Signed-off-by: Zhang Rui <rui.zhang@intel.com> --- drivers/powercap/intel_rapl_tpmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)