powercap: intel_rapl_tpmi: Fix a sparse warning

[PATCH] powercap: intel_rapl_tpmi: Fix a sparse warning

Posted by Zhang Rui 2 years, 7 months ago

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);
 
 	/* Domain Parent bits are ignored for now */
-- 
2.34.1

Re: [PATCH] powercap: intel_rapl_tpmi: Fix a sparse warning

Posted by Rafael J. Wysocki 2 years, 7 months ago

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);
>
>         /* Domain Parent bits are ignored for now */
> --

Applied as 6.5-rc material, thanks!

RE: [PATCH] powercap: intel_rapl_tpmi: Fix a sparse warning

Posted by David Laight 2 years, 6 months ago

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)

Re: [PATCH] powercap: intel_rapl_tpmi: Fix a sparse warning

Posted by Rafael J. Wysocki 2 years, 6 months ago

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?

Re: [PATCH] powercap: intel_rapl_tpmi: Fix a sparse warning

Posted by Zhang, Rui 2 years, 6 months ago

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?

RE: [PATCH] powercap: intel_rapl_tpmi: Fix a sparse warning

Posted by David Laight 2 years, 6 months ago

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)