Re: [PATCH] MIPS: remove const from mips_io_port_base

[Paul Burton <paul.burton@mips.com>]

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
>