Re: [PATCH 2/2] exec: Ensure variable page size is only used with TARGET_PAGE_BITS_VARY

[Richard Henderson]

On 11/4/20 1:35 AM, Philippe Mathieu-Daudé wrote:
> If TARGET_PAGE_BITS_VARY is not supported, machines should not
> intent to modify the target page size.
> As set_preferred_target_page_bits() is supposed to return 'false'
> on failure (documented in "qemu-common.h"), return false to
> indicate failure if this ever happens.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/qemu-common.h | 4 +++-
>  exec-vary.c           | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index fda7dc6ca77..3ea616d4567 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -70,7 +70,9 @@ void cpu_exec_step_atomic(CPUState *cpu);
>   * size may be smaller than any given CPU's preference).
>   * Returns true on success, false on failure (which can only happen
>   * if this is called after the system has already finalized its
> - * choice of page size and the requested page size is smaller than that).
> + * choice of page size and the requested page size is smaller than
> + * that). Only target supporting variable page size should set a
> + * preferred target page size.
>   */
>  bool set_preferred_target_page_bits(int bits);
>  
> diff --git a/exec-vary.c b/exec-vary.c
> index ff905f2a8fb..4b0b7f193af 100644
> --- a/exec-vary.c
> +++ b/exec-vary.c
> @@ -86,8 +86,10 @@ bool set_preferred_target_page_bits(int bits)
>          }
>          init_target_page.bits = bits;
>      }
> -#endif
>      return true;
> +#else
> +    return false;
> +#endif

I guess the only generic caller is the one in softmmu/vl.c, which is protected
by machine_class->minimum_page_bits != 0.  So this affects exactly one machine
outside arm -- fuloong2e.

Certainly we should not allow an impossible setting.  I wonder if it is worth
doing return bits == TARGET_PAGE_BITS?

Either way,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~