Re: [PATCH 8/8] hw/arm/virt: Disable highmem when on hypervisor.framework

[Peter Maydell]

On Thu, 26 Nov 2020 at 22:14, Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> On Thu, Nov 26, 2020 at 10:50:17PM +0100, Alexander Graf wrote:
> > The Apple M1 only supports up to 36 bits of physical address space. That
> > means we can not fit the 64bit MMIO BAR region into our address space.
> >
> > To fix this, let's not expose a 64bit MMIO BAR region when running on
> > Apple Silicon.
> >
> > I have not been able to find a way to enumerate that easily, so let's
> > just assume we always have that little PA space on hypervisor.framework
> > systems.
> >
> > Signed-off-by: Alexander Graf <agraf@csgraf.de>
> > ---
> >  hw/arm/virt.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 27dbeb549e..d74053ecd4 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -45,6 +45,7 @@
> >  #include "hw/display/ramfb.h"
> >  #include "net/net.h"
> >  #include "sysemu/device_tree.h"
> > +#include "sysemu/hvf.h"
> >  #include "sysemu/numa.h"
> >  #include "sysemu/runstate.h"
> >  #include "sysemu/sysemu.h"
> > @@ -1746,6 +1747,14 @@ static void machvirt_init(MachineState *machine)
> >      unsigned int smp_cpus = machine->smp.cpus;
> >      unsigned int max_cpus = machine->smp.max_cpus;
> >
> > +    /*
> > +     * On Hypervisor.framework capable systems, we only have 36 bits of PA
> > +     * space, which is not enough to fit a 64bit BAR space
> > +     */
> > +    if (hvf_enabled()) {
> > +        vms->highmem = false;
> > +    }
>
> Direct checks for *_enabled() are a pain to clean up later when
> we add support to new accelerators.  Can't this be implemented as
> (e.g.) a AccelClass::max_physical_address_bits field?

It's a property of the CPU (eg our emulated TCG CPUs may have
varying supported numbers of physical address bits). So the
virt board ought to look at the CPU, and the CPU should be
set up with the right information for all of KVM, TCG, HVF
(either a specific max_phys_addr_bits value or just ensure
its ID_AA64MMFR0_EL1.PARange is right, not sure which would
be easier/nicer).

thanks
-- PMM