[PATCH] microblaze: Make virt_to_pfn() a static inline

[PATCH] microblaze: Make virt_to_pfn() a static inline

[Linus Walleij]

Making virt_to_pfn() a static inline taking a strongly typed
(const void *) makes the contract of a passing a pointer of that
type to the function explicit and exposes any misuse of the
macro virt_to_pfn() acting polymorphic and accepting many types
such as (void *), (unitptr_t) or (unsigned long) as arguments
without warnings.

Move the function down in the file so __pa() exists in our
scope, and it compiles. This in turn requires moving __pa()
as it depends on __virt_to_phys() that was below. (Lazy macro
evaluation conflicts with strict function ordering.)

Make a symmetric change to pfn_to_virt() so we have type
checking both ways.

Due to this the <asm/page.h> file being included into some
assembly files, some further inclusion guards are needed
to make sure assembly keeps compiling.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/microblaze/include/asm/page.h | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/arch/microblaze/include/asm/page.h b/arch/microblaze/include/asm/page.h
index 337f23eabc71..86a4ce07c192 100644
--- a/arch/microblaze/include/asm/page.h
+++ b/arch/microblaze/include/asm/page.h
@@ -99,9 +99,6 @@ extern int page_is_ram(unsigned long pfn);
 # define phys_to_pfn(phys)	(PFN_DOWN(phys))
 # define pfn_to_phys(pfn)	(PFN_PHYS(pfn))
 
-# define virt_to_pfn(vaddr)	(phys_to_pfn((__pa(vaddr))))
-# define pfn_to_virt(pfn)	__va(pfn_to_phys((pfn)))
-
 #  define virt_to_page(kaddr)	(pfn_to_page(__pa(kaddr) >> PAGE_SHIFT))
 #  define page_to_virt(page)   __va(page_to_pfn(page) << PAGE_SHIFT)
 #  define page_to_phys(page)     (page_to_pfn(page) << PAGE_SHIFT)
@@ -109,11 +106,6 @@ extern int page_is_ram(unsigned long pfn);
 #  define ARCH_PFN_OFFSET	(memory_start >> PAGE_SHIFT)
 # endif /* __ASSEMBLY__ */
 
-#define	virt_addr_valid(vaddr)	(pfn_valid(virt_to_pfn(vaddr)))
-
-# define __pa(x)	__virt_to_phys((unsigned long)(x))
-# define __va(x)	((void *)__phys_to_virt((unsigned long)(x)))
-
 /* Convert between virtual and physical address for MMU. */
 /* Handle MicroBlaze processor with virtual memory. */
 #define __virt_to_phys(addr) \
@@ -125,6 +117,25 @@ extern int page_is_ram(unsigned long pfn);
 #define tovirt(rd, rs) \
 	addik rd, rs, (CONFIG_KERNEL_START - CONFIG_KERNEL_BASE_ADDR)
 
+#ifndef __ASSEMBLY__
+
+# define __pa(x)	__virt_to_phys((unsigned long)(x))
+# define __va(x)	((void *)__phys_to_virt((unsigned long)(x)))
+
+static inline unsigned long virt_to_pfn(const void *vaddr)
+{
+	return phys_to_pfn(__pa(vaddr));
+}
+
+static inline const void *pfn_to_virt(unsigned long pfn)
+{
+	return __va(pfn_to_phys((pfn)));
+}
+
+#define	virt_addr_valid(vaddr)	(pfn_valid(virt_to_pfn(vaddr)))
+
+#endif /* __ASSEMBLY__ */
+
 #define TOPHYS(addr)  __virt_to_phys(addr)
 
 #endif /* __KERNEL__ */

---
base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
change-id: 20230808-virt-to-phys-microblaze-4afdf2bcf96c

Best regards,
-- 
Linus Walleij <linus.walleij@linaro.org>

Re: [PATCH] microblaze: Make virt_to_pfn() a static inline

[Michal Simek]

On 8/8/23 10:40, Linus Walleij wrote:
> Making virt_to_pfn() a static inline taking a strongly typed
> (const void *) makes the contract of a passing a pointer of that
> type to the function explicit and exposes any misuse of the
> macro virt_to_pfn() acting polymorphic and accepting many types
> such as (void *), (unitptr_t) or (unsigned long) as arguments
> without warnings.
> 
> Move the function down in the file so __pa() exists in our
> scope, and it compiles. This in turn requires moving __pa()
> as it depends on __virt_to_phys() that was below. (Lazy macro
> evaluation conflicts with strict function ordering.)
> 
> Make a symmetric change to pfn_to_virt() so we have type
> checking both ways.
> 
> Due to this the <asm/page.h> file being included into some
> assembly files, some further inclusion guards are needed
> to make sure assembly keeps compiling.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>   arch/microblaze/include/asm/page.h | 27 +++++++++++++++++++--------
>   1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/microblaze/include/asm/page.h b/arch/microblaze/include/asm/page.h
> index 337f23eabc71..86a4ce07c192 100644
> --- a/arch/microblaze/include/asm/page.h
> +++ b/arch/microblaze/include/asm/page.h
> @@ -99,9 +99,6 @@ extern int page_is_ram(unsigned long pfn);
>   # define phys_to_pfn(phys)	(PFN_DOWN(phys))
>   # define pfn_to_phys(pfn)	(PFN_PHYS(pfn))
>   
> -# define virt_to_pfn(vaddr)	(phys_to_pfn((__pa(vaddr))))
> -# define pfn_to_virt(pfn)	__va(pfn_to_phys((pfn)))
> -
>   #  define virt_to_page(kaddr)	(pfn_to_page(__pa(kaddr) >> PAGE_SHIFT))
>   #  define page_to_virt(page)   __va(page_to_pfn(page) << PAGE_SHIFT)
>   #  define page_to_phys(page)     (page_to_pfn(page) << PAGE_SHIFT)
> @@ -109,11 +106,6 @@ extern int page_is_ram(unsigned long pfn);
>   #  define ARCH_PFN_OFFSET	(memory_start >> PAGE_SHIFT)
>   # endif /* __ASSEMBLY__ */
>   
> -#define	virt_addr_valid(vaddr)	(pfn_valid(virt_to_pfn(vaddr)))
> -
> -# define __pa(x)	__virt_to_phys((unsigned long)(x))
> -# define __va(x)	((void *)__phys_to_virt((unsigned long)(x)))
> -
>   /* Convert between virtual and physical address for MMU. */
>   /* Handle MicroBlaze processor with virtual memory. */
>   #define __virt_to_phys(addr) \
> @@ -125,6 +117,25 @@ extern int page_is_ram(unsigned long pfn);
>   #define tovirt(rd, rs) \
>   	addik rd, rs, (CONFIG_KERNEL_START - CONFIG_KERNEL_BASE_ADDR)
>   
> +#ifndef __ASSEMBLY__
> +
> +# define __pa(x)	__virt_to_phys((unsigned long)(x))
> +# define __va(x)	((void *)__phys_to_virt((unsigned long)(x)))
> +
> +static inline unsigned long virt_to_pfn(const void *vaddr)
> +{
> +	return phys_to_pfn(__pa(vaddr));
> +}
> +
> +static inline const void *pfn_to_virt(unsigned long pfn)
> +{
> +	return __va(pfn_to_phys((pfn)));
> +}
> +
> +#define	virt_addr_valid(vaddr)	(pfn_valid(virt_to_pfn(vaddr)))
> +
> +#endif /* __ASSEMBLY__ */
> +
>   #define TOPHYS(addr)  __virt_to_phys(addr)
>   
>   #endif /* __KERNEL__ */
> 
> ---
> base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
> change-id: 20230808-virt-to-phys-microblaze-4afdf2bcf96c
> 
> Best regards,

Applied.
M

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP/Versal ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal/Versal NET SoCs
TF-A maintainer - Xilinx ZynqMP/Versal/Versal NET SoCs

Re: [PATCH] microblaze: Make virt_to_pfn() a static inline

[Linus Walleij]

On Tue, Aug 8, 2023 at 10:40 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> +static inline const void *pfn_to_virt(unsigned long pfn)

I think maybe I should not have put const there. It means the
pointer shouldn't be modified and I think there is code that
does that, or cast to some other pointer which is not const:ed
and then use things like ++ on the resulting type.

If you run into compile warnings you can just delete the const,
or I can send a follow-up patch.

Yours,
Linus Walleij

Re: [PATCH] microblaze: Make virt_to_pfn() a static inline

[Michal Simek]

On 8/23/23 10:07, Linus Walleij wrote:
> On Tue, Aug 8, 2023 at 10:40 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> 
>> +static inline const void *pfn_to_virt(unsigned long pfn)
> 
> I think maybe I should not have put const there. It means the
> pointer shouldn't be modified and I think there is code that
> does that, or cast to some other pointer which is not const:ed
> and then use things like ++ on the resulting type.
> 
> If you run into compile warnings you can just delete the const,
> or I can send a follow-up patch.

Thanks for letting me knoe.

I compiled it by my default toolchain and didn't see any issue.
It should be tmr in linux-next and more bots will run over it.
But if you want to fix it please send the follow up patch on the top of this one 
and I will take it.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP/Versal ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal/Versal NET SoCs
TF-A maintainer - Xilinx ZynqMP/Versal/Versal NET SoCs