Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Leif Lindholm]

Hmm ... I managed to accidentally mark this one as read.

Anyway, I spent the Friday writing the RFC I just sent out instead.

On Thu, Jun 04, 2020 at 19:43:06 +0100, Peter Maydell wrote:
> On Thu, 4 Jun 2020 at 17:26, Leif Lindholm <leif@nuviainc.com> wrote:
> >
> > On Thu, Jun 04, 2020 at 17:09:30 +0100, Peter Maydell wrote:
> > > On Thu, 4 Jun 2020 at 17:03, Leif Lindholm <leif@nuviainc.com> wrote:
> > > > But there's also things like:
> > > > - a57_initfn explicitly setting kvm_target, then only being called
> > > >   from max_initfn for !kvm_enabled()
> > >
> > > Expected -- a KVM 'max' is nothing to do with a TCG 'max':
> > >  * for KVM, -cpu max means "same as -cpu host"
> > >  * for TCG, -cpu max means "start with an A57, then add in all the
> > >    extra architectural features that have been added since then".
> >
> > Sure. But why are we setting the kvm_target at all for the
> > !kvm_enabled() case?
> 
> Because it happens to be set in the a57 initfn, and it's harmless
> if we're not using TCG. I feel like some of your take on this set of
> functions comes from thinking of max_initfn as in some sense the
> 'primary' function here, when it's the other way around : a57_initfn
> is the standard kind of CPU initfn (behaving in and written the same
> way as a53_initfn and a72_initfn), whereas max_initfn is an odd
> special case which happens for convenience-of-implementation
> to piggyback on the a57 implementation.

Well, my take is more looking at a part of the codebase that I was not
previously familiar with, seeing things that seemed redundant, but not
being confident enough to dismiss them outright so having to spend
time investigating (and asking silly questions on list).

> > > kvm_target being set by a57_initfn is specifically for the case
> > > where a KVM user is using "-cpu cortex-a57".
> > >
> > > > - a57_initfn setting cpu->dtb_compatible to "arm,cortex-a57"
> > >
> > > What else would it set it to?
> >
> > Hmm, I had been hoping there was a generic v8a one - kvm64.c kind of
> > got my hopes up with "arm,arm-v8".
> 
> Ah, that's the other way around -- yes, for 'max' we should be using a
> more generic value, not accidentally using 'cortex-a57'. (Linux doesn't
> in practice care IIRC, which is why this bug hasn't been noticed.) But
> for an actual cortex-a57 model we should report as arm,cortex-a57.

Well, I think any OS that tries to be generically bootable *has* to
ignore that value, although it could be useful for informational
purposes.

> https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/cpus.yaml
> is the official list of permitted strings, incidentally.

My feeling is none of the values there are appropriate (arm,armv8
indicates ARM ltd, but not aarch64 support). I made something up for
the RFC set. We could always send a patch adding some qemu, or
generic, target.

> > Not entirely unrelated question:
> > Would you take added field definitions in target/arm/cpu.h for
> > features not yet emulated in QEMU but defined in released versions of
> > ARM ARM?
> 
> Yes in theory (you'll see that we have a bunch of field definitions already
> which we don't yet implement, because we tend to define all the fields
> for an ID register at the point where we want to access one field), but
> on the other hand there's usually no need to actually use them unless
> we're adding the emulation for the feature, so the question is: what would
> you do with them if you added them?

Reduce the delta between internal development and upstream.

This is not a suggestion NUVIA has a vast army of qemu developers
ready to implement all features supported by ARMv8.whatever.
But sometimes all we need is an instruction trace, and for that we
need to make sure any selection sequences pick the optimal ones we do
intend to implement.

Best Regards,

Leif