Re: [Qemu-arm] [Qemu-devel] [PATCH v3 6/7] target-arm: Make page size a runtime setting

[Andrew Jones]

On Tue, Oct 11, 2016 at 06:08:18PM +0100, Peter Maydell wrote:
> Rather than defining TARGET_PAGE_BITS to always be 10,
> switch to using a value picked at runtime. This allows us
> to use 4K pages for modern ARM CPUs (and in particular all
> 64-bit CPUs) without having to drop support for the old
> ARMv5 CPUs which had 1K pages.
> 
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  target-arm/cpu.c | 24 ++++++++++++++++++++++++
>  target-arm/cpu.h |  9 +++++----
>  2 files changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 1b9540e..c94a324 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -576,6 +576,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>      ARMCPU *cpu = ARM_CPU(dev);
>      ARMCPUClass *acc = ARM_CPU_GET_CLASS(dev);
>      CPUARMState *env = &cpu->env;
> +    int pagebits;
>  
>      /* Some features automatically imply others: */
>      if (arm_feature(env, ARM_FEATURE_V8)) {
> @@ -631,6 +632,29 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>          set_feature(env, ARM_FEATURE_THUMB_DSP);
>      }
>  
> +    if (arm_feature(env, ARM_FEATURE_V7) &&
> +        !arm_feature(env, ARM_FEATURE_M) &&
> +        !arm_feature(env, ARM_FEATURE_MPU)) {
> +        /* v7VMSA drops support for the old ARMv5 tiny pages, so we
> +         * can use 4K pages.
> +         */
> +        pagebits = 12;
> +    } else {
> +        /* For CPUs which might have tiny 1K pages, or which have an
> +         * MPU and might have small region sizes, stick with 1K pages.
> +         */
> +        pagebits = 10;
> +    }
> +    if (!set_preferred_target_page_bits(pagebits)) {
> +        /* This can only ever happen for hotplugging a CPU, or if
> +         * the board code incorrectly creates a CPU which it has
> +         * promised via minimum_page_size that it will not.
> +         */
> +        error_setg(errp, "This CPU requires a smaller page size than the "
> +                   "system is using");

I'm not sure about this. IIUC, then with this it won't be possible to
create a board that sets up its cpus with the preferred target page bits
greater than the cpu's default set above, even when the cpu supports it.
For example, I may want a board that will only use AArch64 cpus with 64K
pages. In that case I'd like to set the minimum to 16 bits, but then that
would result in this error. I think we should only set the default when a
preference hasn't already been given. And, maybe we should also provide
a maximum to sanity check against? (side note: if we provide a maximum,
then we could use it in arch_dump.c for the dump info's block size,
which must be the maximum page size the cpu supports.)

Thanks,
drew

> +        return;
> +    }
> +
>      if (cpu->reset_hivecs) {
>              cpu->reset_sctlr |= (1 << 13);
>      }
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 76d824d..37d6eb3 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1766,10 +1766,11 @@ bool write_cpustate_to_list(ARMCPU *cpu);
>  #if defined(CONFIG_USER_ONLY)
>  #define TARGET_PAGE_BITS 12
>  #else
> -/* The ARM MMU allows 1k pages.  */
> -/* ??? Linux doesn't actually use these, and they're deprecated in recent
> -   architecture revisions.  Maybe a configure option to disable them.  */
> -#define TARGET_PAGE_BITS 10
> +/* ARMv7 and later CPUs have 4K pages minimum, but ARMv5 and v6
> + * have to support 1K tiny pages.
> + */
> +#define TARGET_PAGE_BITS_VARY
> +#define TARGET_PAGE_BITS_MIN 10
>  #endif
>  
>  #if defined(TARGET_AARCH64)
> -- 
> 2.7.4
> 
>