diff mbox series

x86: asm/io.h: Harden virt_to_phys/isa_virt_to_bus prototypes

Message ID 168146523405.6279.3632248068235163346.stgit@skinsburskii.localdomain
State New
Headers
Series x86: asm/io.h: Harden virt_to_phys/isa_virt_to_bus prototypes |

Commit Message

Stanislav Kinsburskii April 14, 2023, 9:40 a.m. UTC 
  From: Stanislav Kinsburskii <stanislav.kinsburskii@gmail.com>

These two helper functions - virt_to_phys and isa_virt_to_bus - don't need the
address pointer to be mutable.

In the same time expecting it to be mutable leads to the following build
warning for constant pointers:

  warning: passing argument 1 of ‘virt_to_phys’ discards ‘const’ qualifier from pointer target type

Signed-off-by: Stanislav Kinsburskii <stanislav.kinsburskii@gmail.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: Borislav Petkov <bp@alien8.de>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: x86@kernel.org
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Geert Uytterhoeven <geert@linux-m68k.org>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Chris Down <chris@chrisdown.name>
CC: Helge Deller <deller@gmx.de>
CC: Omar Sandoval <osandov@fb.com>
CC: linux-kernel@vger.kernel.org
---
 arch/x86/include/asm/io.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Stanislav Kinsburskii April 15, 2023, 3:23 a.m. UTC | #1 
On Sat, Apr 22, 2023 at 11:02:33AM +0200, Arnd Bergmann wrote:
> On Fri, Apr 14, 2023, at 11:40, Stanislav Kinsburskii wrote:
> > From: Stanislav Kinsburskii <stanislav.kinsburskii@gmail.com>
> >
> > These two helper functions - virt_to_phys and isa_virt_to_bus - don't need the
> > address pointer to be mutable.
> >
> > In the same time expecting it to be mutable leads to the following build
> > warning for constant pointers:
> >
> >   warning: passing argument 1 of ‘virt_to_phys’ discards ‘const’ 
> > qualifier from pointer target type
> >
> 
> The change looks fine, and this is clearly useful for spreading
> more 'const' annotations, but I have two concerns:
> 
> - I'd really like this to be done consistently across architectures,
>   so at least the asm-generic/io.h version should get the same
>   annotation, or ideally all of them in one patch.
> 

Sure, let me come up with the a series to cover other architectures.

> - I would not describe this change itself as "hardening", as it
>   also does the opposite, when you have a pointer that is actually
>   "const" but converting it through virt_to_phys() and back
>   through phys_to_virt() ends up losing the annotation.
>

Indeen, I didn't think about this way.
Let me then just state what the change does (i.e. "Make virt_to_phys to
allow unmutable pointers"), unless you have a better option to advice.

Thanks,
Stanislav

>      Arnd
Arnd Bergmann April 22, 2023, 9:02 a.m. UTC | #2 
On Fri, Apr 14, 2023, at 11:40, Stanislav Kinsburskii wrote:
> From: Stanislav Kinsburskii <stanislav.kinsburskii@gmail.com>
>
> These two helper functions - virt_to_phys and isa_virt_to_bus - don't need the
> address pointer to be mutable.
>
> In the same time expecting it to be mutable leads to the following build
> warning for constant pointers:
>
>   warning: passing argument 1 of ‘virt_to_phys’ discards ‘const’ 
> qualifier from pointer target type
>

The change looks fine, and this is clearly useful for spreading
more 'const' annotations, but I have two concerns:

- I'd really like this to be done consistently across architectures,
  so at least the asm-generic/io.h version should get the same
  annotation, or ideally all of them in one patch.

- I would not describe this change itself as "hardening", as it
  also does the opposite, when you have a pointer that is actually
  "const" but converting it through virt_to_phys() and back
  through phys_to_virt() ends up losing the annotation.

     Arnd
diff mbox series

Patch

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index e9025640f634..0e6f5b48f517 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -128,7 +128,7 @@  extern int valid_mmap_phys_addr_range(unsigned long pfn, size_t size);
  *	this function
  */
 
-static inline phys_addr_t virt_to_phys(volatile void *address)
+static inline phys_addr_t virt_to_phys(const volatile void *address)
 {
 	return __pa(address);
 }
@@ -163,7 +163,7 @@  static inline void *phys_to_virt(phys_addr_t address)
  * However, we truncate the address to unsigned int to avoid undesirable
  * promotions in legacy drivers.
  */
-static inline unsigned int isa_virt_to_bus(volatile void *address)
+static inline unsigned int isa_virt_to_bus(const volatile void *address)
 {
 	return (unsigned int)virt_to_phys(address);
 }