All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Linus Walleij <linus.walleij@linaro.org>,
	Nicholas Piggin <npiggin@gmail.com>,
	Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org
Subject: Re: [PATCH] powerpc: Make virt_to_pfn() a static inline
Date: Mon, 14 Aug 2023 22:37:07 +1000	[thread overview]
Message-ID: <87a5uter64.fsf@mail.lhotse> (raw)
In-Reply-To: <20230809-virt-to-phys-powerpc-v1-1-12e912a7d439@linaro.org>

Linus Walleij <linus.walleij@linaro.org> writes:
> 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.
...
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index f2b6bf5687d0..9ee4b6d4a82a 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -9,6 +9,7 @@
>  #ifndef __ASSEMBLY__
>  #include <linux/types.h>
>  #include <linux/kernel.h>
> +#include <linux/bug.h>
>  #else
>  #include <asm/types.h>
>  #endif
> @@ -119,16 +120,6 @@ extern long long virt_phys_offset;
>  #define ARCH_PFN_OFFSET		((unsigned long)(MEMORY_START >> PAGE_SHIFT))
>  #endif
>  
> -#define virt_to_pfn(kaddr)	(__pa(kaddr) >> PAGE_SHIFT)
> -#define virt_to_page(kaddr)	pfn_to_page(virt_to_pfn(kaddr))
> -#define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
> -
> -#define virt_addr_valid(vaddr)	({					\
> -	unsigned long _addr = (unsigned long)vaddr;			\
> -	_addr >= PAGE_OFFSET && _addr < (unsigned long)high_memory &&	\
> -	pfn_valid(virt_to_pfn(_addr));					\
> -})
> -
>  /*
>   * On Book-E parts we need __va to parse the device tree and we can't
>   * determine MEMORY_START until then.  However we can determine PHYSICAL_START
> @@ -233,6 +224,25 @@ extern long long virt_phys_offset;
>  #endif
>  #endif
>  
> +#ifndef __ASSEMBLY__
> +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)
> +{
> +	return (const void *)(((unsigned long)__va(pfn)) << PAGE_SHIFT);

Any reason to do it this way rather than:

+       return __va(pfn << PAGE_SHIFT);

Seems to be equivalent and much cleaner?

cheers

WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au>
To: Linus Walleij <linus.walleij@linaro.org>,
	Nicholas Piggin <npiggin@gmail.com>,
	Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org, Linus Walleij <linus.walleij@linaro.org>
Subject: Re: [PATCH] powerpc: Make virt_to_pfn() a static inline
Date: Mon, 14 Aug 2023 22:37:07 +1000	[thread overview]
Message-ID: <87a5uter64.fsf@mail.lhotse> (raw)
In-Reply-To: <20230809-virt-to-phys-powerpc-v1-1-12e912a7d439@linaro.org>

Linus Walleij <linus.walleij@linaro.org> writes:
> 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.
...
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index f2b6bf5687d0..9ee4b6d4a82a 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -9,6 +9,7 @@
>  #ifndef __ASSEMBLY__
>  #include <linux/types.h>
>  #include <linux/kernel.h>
> +#include <linux/bug.h>
>  #else
>  #include <asm/types.h>
>  #endif
> @@ -119,16 +120,6 @@ extern long long virt_phys_offset;
>  #define ARCH_PFN_OFFSET		((unsigned long)(MEMORY_START >> PAGE_SHIFT))
>  #endif
>  
> -#define virt_to_pfn(kaddr)	(__pa(kaddr) >> PAGE_SHIFT)
> -#define virt_to_page(kaddr)	pfn_to_page(virt_to_pfn(kaddr))
> -#define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
> -
> -#define virt_addr_valid(vaddr)	({					\
> -	unsigned long _addr = (unsigned long)vaddr;			\
> -	_addr >= PAGE_OFFSET && _addr < (unsigned long)high_memory &&	\
> -	pfn_valid(virt_to_pfn(_addr));					\
> -})
> -
>  /*
>   * On Book-E parts we need __va to parse the device tree and we can't
>   * determine MEMORY_START until then.  However we can determine PHYSICAL_START
> @@ -233,6 +224,25 @@ extern long long virt_phys_offset;
>  #endif
>  #endif
>  
> +#ifndef __ASSEMBLY__
> +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)
> +{
> +	return (const void *)(((unsigned long)__va(pfn)) << PAGE_SHIFT);

Any reason to do it this way rather than:

+       return __va(pfn << PAGE_SHIFT);

Seems to be equivalent and much cleaner?

cheers

  reply	other threads:[~2023-08-14 12:38 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-09  8:07 [PATCH] powerpc: Make virt_to_pfn() a static inline Linus Walleij
2023-08-09  8:07 ` Linus Walleij
2023-08-14 12:37 ` Michael Ellerman [this message]
2023-08-14 12:37   ` Michael Ellerman
2023-08-14 14:29   ` Christophe Leroy
2023-08-15  7:21     ` Michael Ellerman
2023-08-14 18:22   ` Linus Walleij
2023-08-14 18:22     ` Linus Walleij
2023-08-15  7:30 ` Michael Ellerman
2023-08-15  7:30   ` Michael Ellerman
2023-08-15  7:42   ` Linus Walleij
2023-08-15  7:42     ` Linus Walleij
2023-08-15 11:45     ` Michael Ellerman
2023-08-15 11:45       ` Michael Ellerman
2023-08-23 11:55 ` Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a5uter64.fsf@mail.lhotse \
    --to=mpe@ellerman.id.au \
    --cc=christophe.leroy@csgroup.eu \
    --cc=kvm@vger.kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=npiggin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.