[v3,6/7] target-arm: Make page size a runtime setting

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 <peter.maydell@linaro.org>
---
 target-arm/cpu.c | 24 ++++++++++++++++++++++++
 target-arm/cpu.h |  9 +++++----
 2 files changed, 29 insertions(+), 4 deletions(-)

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 <peter.maydell@linaro.org>
> ---
>  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
> 
>

On 12 October 2016 at 14:33, Andrew Jones <drjones@redhat.com> wrote:
> 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 <peter.maydell@linaro.org>

>> +    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.)

If we had a CPU which supported only 64K pages then we would
make this if-ladder set pagebits appropriately. But we don't
have any of those, so it's a bit moot. If the CPU can be
configured by the guest to use 4K pages then the board
must not have set the preferred-page-size to something
larger, or the guest will fall over when it tries it.
That's what this check is protecting against.

The CPU's maximum page size isn't very interesting for this
patchset, because we only care about the minimum (which is
what the TLB needs to use). If the board code was somehow
buggy and requested a page size greater than the CPU's
maximum, then it will also be greater than the CPU's
minimum, and be caught by this error message.

thanks
-- PMM

On Wed, Oct 12, 2016 at 02:40:30PM +0100, Peter Maydell wrote:
> On 12 October 2016 at 14:33, Andrew Jones <drjones@redhat.com> wrote:
> > 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 <peter.maydell@linaro.org>
> 
> >> +    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.)
> 
> If we had a CPU which supported only 64K pages then we would
> make this if-ladder set pagebits appropriately. But we don't
> have any of those, so it's a bit moot. If the CPU can be
> configured by the guest to use 4K pages then the board
> must not have set the preferred-page-size to something
> larger, or the guest will fall over when it tries it.
> That's what this check is protecting against.

Fair enough and understood, but there is a case where we can know
what page size to use. When booting with -kernel we can probe the
kernel image's header (see Documentation/arm64/booting.txt) If the
magic number matches, then we can check the header flags to see
what the kernel page size is.

An API like the one in this series would be useful for setting
the page size to match the guest kernel's, but only if the API
doesn't override the selection.

> 
> The CPU's maximum page size isn't very interesting for this
> patchset, because we only care about the minimum (which is
> what the TLB needs to use). If the board code was somehow
> buggy and requested a page size greater than the CPU's
> maximum, then it will also be greater than the CPU's
> minimum, and be caught by this error message.

Agreed we don't need a maximum as it is now. I think we may
if we allow preferences to be greater than the minimum though.

Thanks,
drew