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

[Eduardo Habkost]

On Fri, Nov 27, 2020 at 04:38:18PM +0000, Peter Maydell wrote:
> On Fri, 27 Nov 2020 at 16:26, Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > On Thu, Nov 26, 2020 at 10:29:01PM +0000, Peter Maydell wrote:
> > > On Thu, 26 Nov 2020 at 22:14, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > > 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).
> >
> > Agreed.
> >
> > My suggestion would still apply to the CPU code that will pick
> > the address size; ideally, accel-specific behaviour should be
> > represented as meaningful fields in AccelClass (either data or
> > virtual methods) instead of direct *_enabled() checks.
> 
> Having looked a bit more closely at some of the relevant target/arm
> code, I think the best approach is going to be that in virt.c
> we just check the PARange ID register field (probably via
> a convenience function that does the conversion of that to
> a nice number-of-bits return value; we might even have one
> already). KVM and TCG both already set that ID register field
> in the CPU struct correctly in their existing
> implicitly-accelerator-specific code; HVF needs to do the same.

Do you know how the implicitly-accelerator-specific code is
implemented?  PARange is in id_aa64mmfr0, correct?  I don't see
any accel-specific code for initializing id_aa64mmfr0.

-- 
Eduardo