On Fri, Nov 27, 2020 at 04:38:18PM +0000, Peter Maydell wrote: > On Fri, 27 Nov 2020 at 16:26, Eduardo Habkost <ehabk...@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 <ehabk...@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
