[PATCH] MIPS: remove const from mips_io_port_base

[PATCH] MIPS: remove const from mips_io_port_base

[Hauke Mehrtens]

This variable is changed by some platform init code. When LTO is used
gcc assumes that the variable never changes and inlines the constant
instead of checking this variable which is wrong.

This fixes a runtime boot problem when LTO is activated.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 arch/mips/include/asm/io.h | 2 +-
 arch/mips/kernel/setup.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
index a7d0b836f2f7..f28f8cd44dd3 100644
--- a/arch/mips/include/asm/io.h
+++ b/arch/mips/include/asm/io.h
@@ -60,7 +60,7 @@
  * instruction, so the lower 16 bits must be zero.  Should be true on
  * on any sane architecture; generic code does not use this assumption.
  */
-extern const unsigned long mips_io_port_base;
+extern unsigned long mips_io_port_base;
 
 /*
  * Gcc will generate code to load the value of mips_io_port_base after each
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 2c96c0c68116..153460c531a9 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -75,7 +75,7 @@ static char __initdata builtin_cmdline[COMMAND_LINE_SIZE] = CONFIG_CMDLINE;
  * mips_io_port_base is the begin of the address space to which x86 style
  * I/O ports are mapped.
  */
-const unsigned long mips_io_port_base = -1;
+unsigned long mips_io_port_base = -1;
 EXPORT_SYMBOL(mips_io_port_base);
 
 static struct resource code_resource = { .name = "Kernel code", };
-- 
2.11.0

Re: [PATCH] MIPS: remove const from mips_io_port_base

[Paul Burton]

Hi Hauke,

On Sat, Jun 16, 2018 at 05:47:45PM +0200, Hauke Mehrtens wrote:
> This variable is changed by some platform init code. When LTO is used
> gcc assumes that the variable never changes and inlines the constant
> instead of checking this variable which is wrong.
> 
> This fixes a runtime boot problem when LTO is activated.
> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  arch/mips/include/asm/io.h | 2 +-
>  arch/mips/kernel/setup.c   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

This does cause GCC 8.1.0 to emit extra loads, adding 652 bytes to a
32r2el_defconfig build from current mips-next. I'd expect kernels for
some of the older supported machines might show a larger gain as they're
more likely to use inX/outX. Combined with the fact that LTO support
isn't in mainline yet anyway, this makes me hesitant to apply this one.

Although it is tempting from the standpoint that set_io_port_base() is
an ugly hack & this would allow its removal... I wonder if we could
clean that up in the same patch though, since it's a natural consequence
of removing const & might make it attractive enough to tolerate the code
size increase.

Even better might be if we could use a fixed virtual address for the I/O
port base, for example by using fixmap. That should give even better
code generation but would of course have TLB overhead & be more
complicated to implement.

Thanks,
    Paul

> diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
> index a7d0b836f2f7..f28f8cd44dd3 100644
> --- a/arch/mips/include/asm/io.h
> +++ b/arch/mips/include/asm/io.h
> @@ -60,7 +60,7 @@
>   * instruction, so the lower 16 bits must be zero.  Should be true on
>   * on any sane architecture; generic code does not use this assumption.
>   */
> -extern const unsigned long mips_io_port_base;
> +extern unsigned long mips_io_port_base;
>  
>  /*
>   * Gcc will generate code to load the value of mips_io_port_base after each
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index 2c96c0c68116..153460c531a9 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -75,7 +75,7 @@ static char __initdata builtin_cmdline[COMMAND_LINE_SIZE] = CONFIG_CMDLINE;
>   * mips_io_port_base is the begin of the address space to which x86 style
>   * I/O ports are mapped.
>   */
> -const unsigned long mips_io_port_base = -1;
> +unsigned long mips_io_port_base = -1;
>  EXPORT_SYMBOL(mips_io_port_base);
>  
>  static struct resource code_resource = { .name = "Kernel code", };
> -- 
> 2.11.0
>