[PATCH] openrisc: Fix build warning in cache.c

[PATCH] openrisc: Fix build warning in cache.c

[Sahil Siddiq]

Commit c5c6fd8be51207f0abd3 ("openrisc: Introduce new utility functions
to flush and invalidate caches") introduced new functions to flush or
invalidate a range of addresses. These functions make use of the mtspr
macro.

The kernel test robot reported an asm constraint-related warning and
error related to the usage of mtspr in these functions. This is because
the compiler is unable to verify that the constraints are not breached
by functions which call cache_loop_page().

Make cache_loop_page() inline so that the compiler can verify the
constraints of the operands used in mtspr.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202503311807.BZaUHY5L-lkp@intel.com/
Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>
---
Hi,

I noticed that the previous patches have already been merged in the
for-next branch of openrisc's repo. So, I thought I would send a separate
patch to fix this.

I managed to reproduce the warning and error using or1k-linux-gcc 13.3
following the instructions found here [1]. The issue is not with the usage
of unsigned int in place of unsigned short. I tried replacing int with
short but that didn't work either. Apart from this, I see there are no
issues with mfspr even though this uses unsigned long for the register
"ret" and for the immediate value "add".

Also, from the gcc manual [2]:

> The compiler cannot check whether the operands have data types that are
> reasonable for the instruction being executed.

I am not sure if the above is just for output operands or all operands.

In mtspr, __spr is constrained to be an immediate value. I noticed that
all calls to mtspr (except for the ones called via cache_loop_page())
have the value of __spr evaluated to a constant at compile time. However,
the compiler is unable to determine if the constraints of the operands
are violated when mtspr is called via cache_loop_page(). Making
cache_loop_page() __always_inline solves this problem since this results
in the value of __spr being evaluated before compilation is completed.

This warning/error can also be observed for mfspr by changing its
prototype to:

__attribute__((__noinline__))
static unsigned long mfspr(unsigned long add) {...}

The compiler did not throw any other warnings/errors on my machine.

Thanks,
Sahil

[1] https://download.01.org/0day-ci/archive/20250331/202503311807.BZaUHY5L-lkp@intel.com/reproduce
[2] https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Output-Operands

 arch/openrisc/mm/cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/openrisc/mm/cache.c b/arch/openrisc/mm/cache.c
index 7bdd07cfca60..0216d65e6f86 100644
--- a/arch/openrisc/mm/cache.c
+++ b/arch/openrisc/mm/cache.c
@@ -40,7 +40,7 @@ static __always_inline void cache_loop(unsigned long paddr, unsigned long end,
 	}
 }
 
-static void cache_loop_page(struct page *page, const unsigned int reg,
+static __always_inline void cache_loop_page(struct page *page, const unsigned int reg,
 					    const unsigned int cache_type)
 {
 	unsigned long paddr = page_to_pfn(page) << PAGE_SHIFT;
-- 
2.48.1

Re: [PATCH] openrisc: Fix build warning in cache.c

[Stafford Horne]

Hi Sahil,


On Wed, Apr 02, 2025 at 01:31:29AM +0530, Sahil Siddiq wrote:
> Commit c5c6fd8be51207f0abd3 ("openrisc: Introduce new utility functions
> to flush and invalidate caches") introduced new functions to flush or
> invalidate a range of addresses. These functions make use of the mtspr
> macro.
> 
> The kernel test robot reported an asm constraint-related warning and
> error related to the usage of mtspr in these functions. This is because
> the compiler is unable to verify that the constraints are not breached
> by functions which call cache_loop_page().
> 
> Make cache_loop_page() inline so that the compiler can verify the
> constraints of the operands used in mtspr.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202503311807.BZaUHY5L-lkp@intel.com/
> Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>
> ---
> Hi,
> 
> I noticed that the previous patches have already been merged in the
> for-next branch of openrisc's repo. So, I thought I would send a separate
> patch to fix this.
> 
> I managed to reproduce the warning and error using or1k-linux-gcc 13.3
> following the instructions found here [1]. The issue is not with the usage
> of unsigned int in place of unsigned short. I tried replacing int with
> short but that didn't work either. Apart from this, I see there are no
> issues with mfspr even though this uses unsigned long for the register
> "ret" and for the immediate value "add".
> 
> Also, from the gcc manual [2]:
> 
> > The compiler cannot check whether the operands have data types that are
> > reasonable for the instruction being executed.
> 
> I am not sure if the above is just for output operands or all operands.
> 
> In mtspr, __spr is constrained to be an immediate value. I noticed that
> all calls to mtspr (except for the ones called via cache_loop_page())
> have the value of __spr evaluated to a constant at compile time. However,
> the compiler is unable to determine if the constraints of the operands
> are violated when mtspr is called via cache_loop_page(). Making
> cache_loop_page() __always_inline solves this problem since this results
> in the value of __spr being evaluated before compilation is completed.
> 
> This warning/error can also be observed for mfspr by changing its
> prototype to:
> 
> __attribute__((__noinline__))
> static unsigned long mfspr(unsigned long add) {...}
> 
> The compiler did not throw any other warnings/errors on my machine.
> 
> Thanks,
> Sahil
> 
> [1] https://download.01.org/0day-ci/archive/20250331/202503311807.BZaUHY5L-lkp@intel.com/reproduce
> [2] https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Output-Operands

I will just take this fix and apply it to the series (git fixup) rather than
take this patch as is.  Also, as registers should be unsigned short, I think
we should change the type to that.

I will fixup patches in place.

-Stafford

>  arch/openrisc/mm/cache.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/openrisc/mm/cache.c b/arch/openrisc/mm/cache.c
> index 7bdd07cfca60..0216d65e6f86 100644
> --- a/arch/openrisc/mm/cache.c
> +++ b/arch/openrisc/mm/cache.c
> @@ -40,7 +40,7 @@ static __always_inline void cache_loop(unsigned long paddr, unsigned long end,
>  	}
>  }
>  
> -static void cache_loop_page(struct page *page, const unsigned int reg,
> +static __always_inline void cache_loop_page(struct page *page, const unsigned int reg,
>  					    const unsigned int cache_type)
>  {
>  	unsigned long paddr = page_to_pfn(page) << PAGE_SHIFT;
> -- 
> 2.48.1
> 

Re: [PATCH] openrisc: Fix build warning in cache.c

[Sahil Siddiq]

Hi Stafford,

On 4/3/25 7:36 PM, Stafford Horne wrote:
> [...]
> I will just take this fix and apply it to the series (git fixup) rather than
> take this patch as is.  Also, as registers should be unsigned short, I think
> we should change the type to that.
> 
> I will fixup patches in place.
> 

Sure thing. Will you be changing the register types as well in place with this
fix? Let me know if I should send v2 of this patch so the changes are together.

Also, should the types used in mfspr also be changed?

Thanks,
Sahil

Re: [PATCH] openrisc: Fix build warning in cache.c

[Stafford Horne]

On Fri, Apr 04, 2025 at 10:39:22AM +0530, Sahil Siddiq wrote:
> Hi Stafford,
> 
> On 4/3/25 7:36 PM, Stafford Horne wrote:
> > [...]
> > I will just take this fix and apply it to the series (git fixup) rather than
> > take this patch as is.  Also, as registers should be unsigned short, I think
> > we should change the type to that.
> > 
> > I will fixup patches in place.
> > 
> 
> Sure thing. Will you be changing the register types as well in place with this
> fix? Let me know if I should send v2 of this patch so the changes are together.

Hi Sahil,

Sorry for delay in getting back on this.  I was working on getting this patches
ready for upstreaming and noticed one thing:

    ---------------------------------------------------------------------------------------
    ./patches/or1k-20250418/0001-openrisc-Refactor-struct-cpuinfo_or1k-to-reduce-dupl.patch
    ---------------------------------------------------------------------------------------
    WARNING: From:/Signed-off-by: email address mismatch: 'From: Sahil Siddiq <icegambit91@gmail.com>' != 'Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>'

    total: 0 errors, 1 warnings, 102 lines checked

As you can see above the scripts/checkpatch.pl is failing with the warning
about your email and signed-off-by not matching.  You can see more about it
in the FROM_SIGN_OFF_MISMATCH section of the checkpatch[0] docs.

How would you like to resolve this?

-Stafford

[0] https://docs.kernel.org/dev-tools/checkpatch.html

Re: [PATCH] openrisc: Fix build warning in cache.c

[Sahil Siddiq]

Hi Stafford,

On 4/18/25 1:17 PM, Stafford Horne wrote:
> On Fri, Apr 04, 2025 at 10:39:22AM +0530, Sahil Siddiq wrote:
> [...]
> Hi Sahil,
> 
> Sorry for delay in getting back on this.

No worries :)

> I was working on getting this patches ready for upstreaming and noticed one
> thing:
> 
>      ---------------------------------------------------------------------------------------
>      ./patches/or1k-20250418/0001-openrisc-Refactor-struct-cpuinfo_or1k-to-reduce-dupl.patch
>      ---------------------------------------------------------------------------------------
>      WARNING: From:/Signed-off-by: email address mismatch: 'From: Sahil Siddiq <icegambit91@gmail.com>' != 'Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>'
> 
>      total: 0 errors, 1 warnings, 102 lines checked
> 
> As you can see above the scripts/checkpatch.pl is failing with the warning
> about your email and signed-off-by not matching.  You can see more about it
> in the FROM_SIGN_OFF_MISMATCH section of the checkpatch[0] docs.

Ok, this makes sense. I configured git-send-email to use gmail because
protonmail does not work with git-send-email without a paid account.

> How would you like to resolve this?

Is this a warning that cannot be ignored? I can:

1. submit the patch series with another email address that won't have issues
    with git-send-email, or
2. submit the patch series using protonmail's web client (which might not be
    the best option).

I would prefer not to use "icegambit91" in the signed-off-by tag.

What are your thoughts on this?

Thanks,
Sahil

Re: [PATCH] openrisc: Fix build warning in cache.c

[Stafford Horne]

On Fri, Apr 18, 2025 at 02:42:20PM +0530, Sahil Siddiq wrote:
> Hi Stafford,
> 
> On 4/18/25 1:17 PM, Stafford Horne wrote:
> > On Fri, Apr 04, 2025 at 10:39:22AM +0530, Sahil Siddiq wrote:
> > [...]
> > Hi Sahil,
> > 
> > Sorry for delay in getting back on this.
> 
> No worries :)
> 
> > I was working on getting this patches ready for upstreaming and noticed one
> > thing:
> > 
> >      ---------------------------------------------------------------------------------------
> >      ./patches/or1k-20250418/0001-openrisc-Refactor-struct-cpuinfo_or1k-to-reduce-dupl.patch
> >      ---------------------------------------------------------------------------------------
> >      WARNING: From:/Signed-off-by: email address mismatch: 'From: Sahil Siddiq <icegambit91@gmail.com>' != 'Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>'
> > 
> >      total: 0 errors, 1 warnings, 102 lines checked
> > 
> > As you can see above the scripts/checkpatch.pl is failing with the warning
> > about your email and signed-off-by not matching.  You can see more about it
> > in the FROM_SIGN_OFF_MISMATCH section of the checkpatch[0] docs.
> 
> Ok, this makes sense. I configured git-send-email to use gmail because
> protonmail does not work with git-send-email without a paid account.
> 
> > How would you like to resolve this?
> 
> Is this a warning that cannot be ignored? I can:
> 
> 1. submit the patch series with another email address that won't have issues
>    with git-send-email, or
> 2. submit the patch series using protonmail's web client (which might not be
>    the best option).
> 
> I would prefer not to use "icegambit91" in the signed-off-by tag.
> 
> What are your thoughts on this?

I could rewrite the from header to match sahilcdq@proton.me.

But if I do that you should find a way to get proton.me to work with
gi-send-email [0] for future commits.  It seems this can work using the
Protonmail Bridge[1], though this site also says proton.me is not so good for
using with git send email.

Maybe using another email in signed-off-by would be better.  The current
patches are on linux-next[2] where we can see the From/Signed-off-by mismatch.
For example:

    author	Sahil Siddiq <icegambit91@gmail.com>	2025-03-29 15:16:22 +0530
    committer	Stafford Horne <shorne@gmail.com>	2025-03-29 10:22:21 +0000
    commit	af83ece87a1ef5097434b7c3c1fc0e9e7b83b192 (patch)

    ...

    Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>
    Signed-off-by: Stafford Horne <shorne@gmail.com>

-Stafford

[0] https://git-send-email.io/#step-2
[1] https://proton.me/mail/bridge
[2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/arch/openrisc

Re: [PATCH] openrisc: Fix build warning in cache.c

[Sahil Siddiq]

Hi,

On 4/18/25 3:30 PM, Stafford Horne wrote:
> On Fri, Apr 18, 2025 at 02:42:20PM +0530, Sahil Siddiq wrote:
>> On 4/18/25 1:17 PM, Stafford Horne wrote:
>>> On Fri, Apr 04, 2025 at 10:39:22AM +0530, Sahil Siddiq wrote:
>>> [...]
>>> I was working on getting this patches ready for upstreaming and noticed one
>>> thing:
>>>
>>>       ---------------------------------------------------------------------------------------
>>>       ./patches/or1k-20250418/0001-openrisc-Refactor-struct-cpuinfo_or1k-to-reduce-dupl.patch
>>>       ---------------------------------------------------------------------------------------
>>>       WARNING: From:/Signed-off-by: email address mismatch: 'From: Sahil Siddiq <icegambit91@gmail.com>' != 'Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>'
>>>
>>>       total: 0 errors, 1 warnings, 102 lines checked
>>>
>>> As you can see above the scripts/checkpatch.pl is failing with the warning
>>> about your email and signed-off-by not matching.  You can see more about it
>>> in the FROM_SIGN_OFF_MISMATCH section of the checkpatch[0] docs.
>>
>> Ok, this makes sense. I configured git-send-email to use gmail because
>> protonmail does not work with git-send-email without a paid account.
>>
>>> How would you like to resolve this?
>>
>> Is this a warning that cannot be ignored? I can:
>>
>> 1. submit the patch series with another email address that won't have issues
>>     with git-send-email, or
>> 2. submit the patch series using protonmail's web client (which might not be
>>     the best option).
>>
>> I would prefer not to use "icegambit91" in the signed-off-by tag.
>>
>> What are your thoughts on this?
> 
> I could rewrite the from header to match sahilcdq@proton.me.
> 
> But if I do that you should find a way to get proton.me to work with
> gi-send-email [0] for future commits.  It seems this can work using the
> Protonmail Bridge[1], though this site also says proton.me is not so good for
> using with git send email.

Right. Also, protonmail bridge requires a paid proton account.

> Maybe using another email in signed-off-by would be better.  The current
> patches are on linux-next[2] where we can see the From/Signed-off-by mismatch.
> For example:
> 
>      author	Sahil Siddiq <icegambit91@gmail.com>	2025-03-29 15:16:22 +0530
>      committer	Stafford Horne <shorne@gmail.com>	2025-03-29 10:22:21 +0000
>      commit	af83ece87a1ef5097434b7c3c1fc0e9e7b83b192 (patch)
> 
>      ...
> 
>      Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>
>      Signed-off-by: Stafford Horne <shorne@gmail.com>
> 

I agree. I have another email id (sahilcdq0@gmail.com). I can configure git
to use this and this can also be used in the signed-off-by tag. So, this id
shouldn't cause an issue.

Let me know if I should send the series using this email id.

Thanks,
Sahil

Re: [PATCH] openrisc: Fix build warning in cache.c

[Stafford Horne]

On Fri, Apr 18, 2025 at 06:04:29PM +0530, Sahil Siddiq wrote:
> Hi,
> 
> On 4/18/25 3:30 PM, Stafford Horne wrote:
> > On Fri, Apr 18, 2025 at 02:42:20PM +0530, Sahil Siddiq wrote:
> > > On 4/18/25 1:17 PM, Stafford Horne wrote:
> > > > On Fri, Apr 04, 2025 at 10:39:22AM +0530, Sahil Siddiq wrote:
> > > > [...]
> > > > I was working on getting this patches ready for upstreaming and noticed one
> > > > thing:
> > > > 
> > > >       ---------------------------------------------------------------------------------------
> > > >       ./patches/or1k-20250418/0001-openrisc-Refactor-struct-cpuinfo_or1k-to-reduce-dupl.patch
> > > >       ---------------------------------------------------------------------------------------
> > > >       WARNING: From:/Signed-off-by: email address mismatch: 'From: Sahil Siddiq <icegambit91@gmail.com>' != 'Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>'
> > > > 
> > > >       total: 0 errors, 1 warnings, 102 lines checked
> > > > 
> > > > As you can see above the scripts/checkpatch.pl is failing with the warning
> > > > about your email and signed-off-by not matching.  You can see more about it
> > > > in the FROM_SIGN_OFF_MISMATCH section of the checkpatch[0] docs.
> > > 
> > > Ok, this makes sense. I configured git-send-email to use gmail because
> > > protonmail does not work with git-send-email without a paid account.
> > > 
> > > > How would you like to resolve this?
> > > 
> > > Is this a warning that cannot be ignored? I can:
> > > 
> > > 1. submit the patch series with another email address that won't have issues
> > >     with git-send-email, or
> > > 2. submit the patch series using protonmail's web client (which might not be
> > >     the best option).
> > > 
> > > I would prefer not to use "icegambit91" in the signed-off-by tag.
> > > 
> > > What are your thoughts on this?
> > 
> > I could rewrite the from header to match sahilcdq@proton.me.
> > 
> > But if I do that you should find a way to get proton.me to work with
> > gi-send-email [0] for future commits.  It seems this can work using the
> > Protonmail Bridge[1], though this site also says proton.me is not so good for
> > using with git send email.
> 
> Right. Also, protonmail bridge requires a paid proton account.
> 
> > Maybe using another email in signed-off-by would be better.  The current
> > patches are on linux-next[2] where we can see the From/Signed-off-by mismatch.
> > For example:
> > 
> >      author	Sahil Siddiq <icegambit91@gmail.com>	2025-03-29 15:16:22 +0530
> >      committer	Stafford Horne <shorne@gmail.com>	2025-03-29 10:22:21 +0000
> >      commit	af83ece87a1ef5097434b7c3c1fc0e9e7b83b192 (patch)
> > 
> >      ...
> > 
> >      Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>
> >      Signed-off-by: Stafford Horne <shorne@gmail.com>
> > 
> 
> I agree. I have another email id (sahilcdq0@gmail.com). I can configure git
> to use this and this can also be used in the signed-off-by tag. So, this id
> shouldn't cause an issue.
> 
> Let me know if I should send the series using this email id.

Yes, please do this.

-Stafford

Re: [PATCH] openrisc: Fix build warning in cache.c

[Sahil Siddiq]

Hi,

On 4/19/25 10:42 AM, Stafford Horne wrote:
> On Fri, Apr 18, 2025 at 06:04:29PM +0530, Sahil Siddiq wrote:
>> On 4/18/25 3:30 PM, Stafford Horne wrote:
>>> On Fri, Apr 18, 2025 at 02:42:20PM +0530, Sahil Siddiq wrote:
>>> [...]
>>> Maybe using another email in signed-off-by would be better.  The current
>>> patches are on linux-next[2] where we can see the From/Signed-off-by mismatch.
>>> For example:
>>>
>>>       author	Sahil Siddiq <icegambit91@gmail.com>	2025-03-29 15:16:22 +0530
>>>       committer	Stafford Horne <shorne@gmail.com>	2025-03-29 10:22:21 +0000
>>>       commit	af83ece87a1ef5097434b7c3c1fc0e9e7b83b192 (patch)
>>>
>>>       ...
>>>
>>>       Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>
>>>       Signed-off-by: Stafford Horne <shorne@gmail.com>
>>>
>>
>> I agree. I have another email id (sahilcdq0@gmail.com). I can configure git
>> to use this and this can also be used in the signed-off-by tag. So, this id
>> shouldn't cause an issue.
>>
>> Let me know if I should send the series using this email id.
> 
> Yes, please do this.

I have sent a new patch series. The email ids should match now. I have also
addressed the build warning that had appeared earlier so those changes won't
have to be made manually using "git fixup".

Thanks,
Sahil