Hi Wang,
Thanks for the patchset - overall it looks good, just a few comments
inline below.
On Sun, Feb 24, 2019 at 03:13:53PM +0800, Wang Xuerui wrote:
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index a84c24d894aa..b0068a1e1e33 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -2158,12 +2158,27 @@ config KVM_GUEST_TIMER_FREQ
> emulation when determining guest CPU Frequency. Instead, the guest's
> timer frequency is specified directly.
>
> +choice
> + prompt "Virtual address size"
> + default MIPS_VA_BITS_DEFAULT
This whole choice ought to depend on 64BIT - it will have no effect for
MIPS32 kernels.
The prompt might be more accurate as "Maximum virtual address size".
> +
> +config MIPS_VA_BITS_DEFAULT
Can you rename this MIPS_VA_BITS_40?
> + bool "40 bits or less virtual memory"
> + help
> + This is the default setting on MIPS platforms since antiquity,
> + which gives 40 bits or less of virtual address space, depending on
> + the CPU.
> +
> + If unsure, say Y.
> +
Isn't it always 40 bits? The comment above the definition of TASK_SIZE64
in asm/processor.h suggests everything back to the R4000 can do 1TB (ie.
40 bit) VAs, and TACK_SIZE64 is unconditionally 1TB.
> config MIPS_VA_BITS_48
> bool "48 bits virtual memory"
> depends on 64BIT
With the choice depending on 64BIT you can then drop the dependency
here.
> + select MIPS_LARGE_VA
> help
> Support a maximum at least 48 bits of application virtual
> - memory. Default is 40 bits or less, depending on the CPU.
> + memory.
> +
I don't think there's any need to describe the default again here so
would drop this change.
> For page sizes 16k and above, this option results in a small
> memory overhead for page tables. For 4k page size, a fourth
> level of page tables is added which imposes both a memory
> @@ -2171,6 +2186,11 @@ config MIPS_VA_BITS_48
>
> If unsure, say N.
>
> +endchoice
> +
> +config MIPS_LARGE_VA
> + bool
> +
> choice
> prompt "Kernel page size"
> default PAGE_SIZE_4KB
> @@ -2187,7 +2207,7 @@ config PAGE_SIZE_4KB
> config PAGE_SIZE_8KB
> bool "8kB"
> depends on CPU_R8000 || CPU_CAVIUM_OCTEON
> - depends on !MIPS_VA_BITS_48
> + depends on !MIPS_LARGE_VA
I think it would be cleaner to introduce the MIPS_VA_BITS entry you add
in the next patch here instead, and make this:
depends on (MIPS_VA_BITS <= 40) if 64BIT
Or maybe even give MIPS_VA_BITS values for 32BIT kernels so you can drop
the "if 64BIT" part, eg:
default 30 if 32BIT && KVM_GUEST
default 31 if 32BIT
With that & similar changes to other uses of MIPS_LARGE_VA we won't need
MIPS_LARGE_VA at all, and dependencies will be more explicit about the
range of VA sizes they actually care about.
Thanks,
Paul
