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

[Andrew Jones]

On Wed, Oct 12, 2016 at 02:40:30PM +0100, Peter Maydell wrote:
> On 12 October 2016 at 14:33, Andrew Jones <address@hidden> 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 <address@hidden>
> 
> >> +    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