2023-11-06 13:44:39

by Linus Walleij

[permalink] [raw]
Subject: [PATCH] powerpc: Fix signature of pfn_to_kaddr()

There is a const in the returned value from pfn_to_kaddr()
but there are consumers that want to modify the result
and the generic function pfn_to_virt() in <asm-generic/page.h>
does allow this, so let's relax this requirement and do not
make the returned value const.

Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Fixes: 58b6fed89ab0 ("powerpc: Make virt_to_pfn() a static inline")
Signed-off-by: Linus Walleij <[email protected]>
---
The remaining warnings from the test robot appear a bit bogus.
If someone knows what to do about them, please help. The warnings
often properly uncovers problems that have been around forever
due to these functions being disguised as macros.
---
arch/powerpc/include/asm/page.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index e5fcc79b5bfb..5243e48dc13a 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -230,7 +230,7 @@ static inline unsigned long virt_to_pfn(const void *kaddr)
return __pa(kaddr) >> PAGE_SHIFT;
}

-static inline const void *pfn_to_kaddr(unsigned long pfn)
+static inline void *pfn_to_kaddr(unsigned long pfn)
{
return __va(pfn << PAGE_SHIFT);
}

---
base-commit: d2f51b3516dade79269ff45eae2a7668ae711b25
change-id: 20231106-virt-to-pfn-fix-ppc-d91de47c6017

Best regards,
--
Linus Walleij <[email protected]>


2023-11-07 05:58:14

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH] powerpc: Fix signature of pfn_to_kaddr()

Linus Walleij <[email protected]> writes:
> There is a const in the returned value from pfn_to_kaddr()
> but there are consumers that want to modify the result
> and the generic function pfn_to_virt() in <asm-generic/page.h>
> does allow this, so let's relax this requirement and do not
> make the returned value const.
>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

I'm struggling to connect the removal of const with those bug reports.
It looks like all those warnings are about 0xc000000000000000 being
outside the range of unsigned long when building 32-bit.

Is it the right bug report link?

The current signature of:

static inline const void *pfn_to_kaddr(unsigned long pfn) ...

seems OK to me.

It allows code like:

const void *p = pfn_to_kaddr(pfn);
p++;

But errors for:

const void *p = pfn_to_kaddr(pfn);
unsigned long *q = p;
*q = 0;

error: initialization discards ‘const’ qualifier from pointer target type


Having said that it looks like almost every caller of pfn_to_kaddr()
casts the result to unsigned long, so possibly that would be the better
return type in terms of the actual usage. Although that would conflict
with __va() which returns void * :/

cheers

2023-11-07 08:07:09

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] powerpc: Fix signature of pfn_to_kaddr()

On Tue, Nov 7, 2023 at 6:57 AM Michael Ellerman <[email protected]> wrote:

> I'm struggling to connect the removal of const with those bug reports.
> It looks like all those warnings are about 0xc000000000000000 being
> outside the range of unsigned long when building 32-bit.

Aha right. I wonder what actually causes that.

> Is it the right bug report link?

Yeah I'm just bad at understanding these reports.

> The current signature of:
>
> static inline const void *pfn_to_kaddr(unsigned long pfn) ...
>
> seems OK to me.

OK then, drop this patch.

Yours,
Linus Walleij