linux-kernel - Re: [PATCH] powercap: intel_rapl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 17 Jul 2023 09:13:08 +0000
From:   "Zhang, Rui" <rui.zhang@...el.com>
To:     "rafael@...nel.org" <rafael@...nel.org>,
        "David.Laight@...lab.com" <David.Laight@...lab.com>
CC:     "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
        "daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        lkp <lkp@...el.com>
Subject: Re: [PATCH] powercap: intel_rapl_tpmi: Fix a sparse warning

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@...lab.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@...el.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@...el.com>
> > > > Closes:
> > > > https://lore.kernel.org/oe-kbuild-all/202307031405.dy3druuy-lkp@intel.com/
> > > > Signed-off-by: Zhang Rui <rui.zhang@...el.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?