kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Leif Lindholm]

Hi there,

(all this done on current HEAD: 66234fee9c)

I was looking through the definition of the aarch64 "max" cpu, and
noticed it invokes aarch64_a57_initfn as a template, followed by
overriding some feature and ID fields to enable additional
functionality.

I then noticed it does not override kvm_target, which hence remains
set as QEMU_KVM_ARM_TARGET_CORTEX_A57. Of course, this only happens on
the else side of the if (kvm_enabled()) branch, so doesn't affect it.

However, while looking at this, I noticed aarch64_a72_initfn doesn't
initialise kvm_target at all.

Then I looked at the definition of QEMU_KVM_ARM_TARGET_CORTEX_A57,
found there was also a KVM_ARM_TARGET_CORTEX_A57, and then I noticed
there exists a KVM_ARM_TARGET_CORTEX_A57 (in
linux-headers/asm-arm64/kvm.h), and *then* I noticed there exists a
KVM_ARM_TARGET_GENERIC_V8 definition there as well - plus a comment
saying "please don't add any more targets unless you really need to".
Then I noticed there isn't a corresponding
QEMU_KVM_ARM_TARGET_GENERIC_V8 in target/arm/kvm-consts.h.

So, then I decided to actually test things, and found that
(with -enable-kvm):
- on Cortex-A53 hardware
  - "max" kvm_target gets initialized to 4 (KVM_ARM_TARGET_CORTEX_A53)
    by kvm_arm_get_host_cpu_features (as returned from the kernel for
    vm_arm_create_scratch_host_vcpu)
  - cortex-A72 fails to start with "KVM is not supported for this guest
    CPU type"
  (fair enough, it's later than A53)
- on Cortex-A72 hardware
  - "max" kvm_target gets initialized to 5 (KVM_ARM_TARGET_GENERIC_V8)
    by kvm_arm_get_host_cpu_features
  - "cortex-A72" fails to start (umm...)

However ... if I haven't managed to confuse myself somewhere in here
(which is completely possible), would it be OK if I submitted a set of
patches that:
- add a QEMU_KVM_ARM_TARGET_GENERIC_V8 to match the kernel one
- set kvm_target for Cortex-A72 to QEMU_KVM_ARM_TARGET_GENERIC_V8
  - alternatively drop the explicit settings for A57/A53
- drop the call from aarch64_max_initfn to aarch64_a57_initfn, and
  copy the relevant bits into the former for the !kvm case

?

/
    Leif

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Peter Maydell]

[added kvm-arm to the cc list; the kernel folks tend to hang out
there, not on qemu-devel, so KVM related questions are usually
worth raising there as well.]

On Thu, 4 Jun 2020 at 13:55, Leif Lindholm <leif@nuviainc.com> wrote:
> However, while looking at this, I noticed aarch64_a72_initfn doesn't
> initialise kvm_target at all.

Yep. The kernel doesn't support "give me a CPU that looks like
a Cortex-A72".

> So, then I decided to actually test things, and found that
> (with -enable-kvm):
> - on Cortex-A53 hardware
>   - "max" kvm_target gets initialized to 4 (KVM_ARM_TARGET_CORTEX_A53)
>     by kvm_arm_get_host_cpu_features (as returned from the kernel for
>     vm_arm_create_scratch_host_vcpu)
>   - cortex-A72 fails to start with "KVM is not supported for this guest
>     CPU type"
>   (fair enough, it's later than A53)

Untested, but I assume that -cpu cortex-a53 works on the A53...

> - on Cortex-A72 hardware
>   - "max" kvm_target gets initialized to 5 (KVM_ARM_TARGET_GENERIC_V8)
>     by kvm_arm_get_host_cpu_features
>   - "cortex-A72" fails to start (umm...)

...and fails on the A72 host.

> However ... if I haven't managed to confuse myself somewhere in here
> (which is completely possible), would it be OK if I submitted a set of
> patches that:
> - add a QEMU_KVM_ARM_TARGET_GENERIC_V8 to match the kernel one
> - set kvm_target for Cortex-A72 to QEMU_KVM_ARM_TARGET_GENERIC_V8

This would be wrong -- it would mean that you could tell QEMU "give
me a guest CPU that's a Cortex-A72" and it would not error on
non-A72 hardware but not actually give a guest CPU that looks
like a Cortex-A72.

 * If what you want is "give me something that works" then that's
   -cpu host or -cpu max.

 * If what you want is "give me something that's always this kind of
   CPU regardless of the host hardware" then that's a lot of kernel
   dev work nobody's been enthusiastic enough to undertake yet
   (notably the "what do we do about CPU errata workarounds" question
   would need to be solved...)

 * If what you want is "allow me to say '-cpu cortex-a72' and have
   it work on an A72 host and not anywhere else" then I guess we could
   implement that on the QEMU side by querying the MIDR and checking
   whether it was what we expected.

>   - alternatively drop the explicit settings for A57/A53

These explicit settings are correct, because for these CPUs
the kernel does have a "give me what I want in particular"
setting (which it will fail on the wrong h/w), and also as
back-compat for older kernels that predate the GENERIC_V8
define and only recognize the explicit "give me an A53" value.

> - drop the call from aarch64_max_initfn to aarch64_a57_initfn, and
>   copy the relevant bits into the former for the !kvm case

Not sure why you care about this -- it's an implementation
detail of the TCG handling of the 'max' CPU. There's an argument
for disentangling TCG 'max' so it's not literally implemented
as "a57 plus newer stuff", granted.

thanks
-- PMM

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Peter Maydell]

[added kvm-arm to the cc list; the kernel folks tend to hang out
there, not on qemu-devel, so KVM related questions are usually
worth raising there as well.]

On Thu, 4 Jun 2020 at 13:55, Leif Lindholm <leif@nuviainc.com> wrote:
> However, while looking at this, I noticed aarch64_a72_initfn doesn't
> initialise kvm_target at all.

Yep. The kernel doesn't support "give me a CPU that looks like
a Cortex-A72".

> So, then I decided to actually test things, and found that
> (with -enable-kvm):
> - on Cortex-A53 hardware
>   - "max" kvm_target gets initialized to 4 (KVM_ARM_TARGET_CORTEX_A53)
>     by kvm_arm_get_host_cpu_features (as returned from the kernel for
>     vm_arm_create_scratch_host_vcpu)
>   - cortex-A72 fails to start with "KVM is not supported for this guest
>     CPU type"
>   (fair enough, it's later than A53)

Untested, but I assume that -cpu cortex-a53 works on the A53...

> - on Cortex-A72 hardware
>   - "max" kvm_target gets initialized to 5 (KVM_ARM_TARGET_GENERIC_V8)
>     by kvm_arm_get_host_cpu_features
>   - "cortex-A72" fails to start (umm...)

...and fails on the A72 host.

> However ... if I haven't managed to confuse myself somewhere in here
> (which is completely possible), would it be OK if I submitted a set of
> patches that:
> - add a QEMU_KVM_ARM_TARGET_GENERIC_V8 to match the kernel one
> - set kvm_target for Cortex-A72 to QEMU_KVM_ARM_TARGET_GENERIC_V8

This would be wrong -- it would mean that you could tell QEMU "give
me a guest CPU that's a Cortex-A72" and it would not error on
non-A72 hardware but not actually give a guest CPU that looks
like a Cortex-A72.

 * If what you want is "give me something that works" then that's
   -cpu host or -cpu max.

 * If what you want is "give me something that's always this kind of
   CPU regardless of the host hardware" then that's a lot of kernel
   dev work nobody's been enthusiastic enough to undertake yet
   (notably the "what do we do about CPU errata workarounds" question
   would need to be solved...)

 * If what you want is "allow me to say '-cpu cortex-a72' and have
   it work on an A72 host and not anywhere else" then I guess we could
   implement that on the QEMU side by querying the MIDR and checking
   whether it was what we expected.

>   - alternatively drop the explicit settings for A57/A53

These explicit settings are correct, because for these CPUs
the kernel does have a "give me what I want in particular"
setting (which it will fail on the wrong h/w), and also as
back-compat for older kernels that predate the GENERIC_V8
define and only recognize the explicit "give me an A53" value.

> - drop the call from aarch64_max_initfn to aarch64_a57_initfn, and
>   copy the relevant bits into the former for the !kvm case

Not sure why you care about this -- it's an implementation
detail of the TCG handling of the 'max' CPU. There's an argument
for disentangling TCG 'max' so it's not literally implemented
as "a57 plus newer stuff", granted.

thanks
-- PMM
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Andrew Jones]

On Thu, Jun 04, 2020 at 01:55:44PM +0100, Leif Lindholm wrote:
> Hi there,
> 
> (all this done on current HEAD: 66234fee9c)
> 
> I was looking through the definition of the aarch64 "max" cpu, and
> noticed it invokes aarch64_a57_initfn as a template, followed by
> overriding some feature and ID fields to enable additional
> functionality.
> 
> I then noticed it does not override kvm_target, which hence remains
> set as QEMU_KVM_ARM_TARGET_CORTEX_A57. Of course, this only happens on
> the else side of the if (kvm_enabled()) branch, so doesn't affect it.
> 
> However, while looking at this, I noticed aarch64_a72_initfn doesn't
> initialise kvm_target at all.
> 
> Then I looked at the definition of QEMU_KVM_ARM_TARGET_CORTEX_A57,
> found there was also a KVM_ARM_TARGET_CORTEX_A57, and then I noticed
> there exists a KVM_ARM_TARGET_CORTEX_A57 (in
> linux-headers/asm-arm64/kvm.h), and *then* I noticed there exists a
> KVM_ARM_TARGET_GENERIC_V8 definition there as well - plus a comment
> saying "please don't add any more targets unless you really need to".
> Then I noticed there isn't a corresponding
> QEMU_KVM_ARM_TARGET_GENERIC_V8 in target/arm/kvm-consts.h.
> 
> So, then I decided to actually test things, and found that
> (with -enable-kvm):
> - on Cortex-A53 hardware
>   - "max" kvm_target gets initialized to 4 (KVM_ARM_TARGET_CORTEX_A53)
>     by kvm_arm_get_host_cpu_features (as returned from the kernel for
>     vm_arm_create_scratch_host_vcpu)
>   - cortex-A72 fails to start with "KVM is not supported for this guest
>     CPU type"
>   (fair enough, it's later than A53)
> - on Cortex-A72 hardware
>   - "max" kvm_target gets initialized to 5 (KVM_ARM_TARGET_GENERIC_V8)
>     by kvm_arm_get_host_cpu_features
>   - "cortex-A72" fails to start (umm...)
> 
> However ... if I haven't managed to confuse myself somewhere in here
> (which is completely possible), would it be OK if I submitted a set of
> patches that:
> - add a QEMU_KVM_ARM_TARGET_GENERIC_V8 to match the kernel one
> - set kvm_target for Cortex-A72 to QEMU_KVM_ARM_TARGET_GENERIC_V8

I'd rather not do that. AArch64 KVM doesn't support CPU models, so we
shouldn't try to fake it by allowing '-cpu cortex-a72' to work, even when
using it on a host with Cortex-A72 CPUs. I'd rather all AArch64 KVM users
use 'host' or 'max' (which is the same as 'host' when used with KVM) until
CPU models truly work.

>   - alternatively drop the explicit settings for A57/A53

The A57 and A53 KVM targets would have to be exactly the same as the
KVM generic target in order to do that. And, even then we can't remove
them from KVM, as they're API, so it doesn't help much to change them
in QEMU.

> - drop the call from aarch64_max_initfn to aarch64_a57_initfn, and
>   copy the relevant bits into the former for the !kvm case

I don't have a strong preference here, but if the naming is what's
troublesome, then I'd think we're better off creating something like
an aarch64_aXX_initfn() function and then calling it from both a57
and max, and anywhere else it fits.

Thanks,
drew

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Andrew Jones]

On Thu, Jun 04, 2020 at 02:10:08PM +0100, Peter Maydell wrote:
> [added kvm-arm to the cc list; the kernel folks tend to hang out
> there, not on qemu-devel, so KVM related questions are usually
> worth raising there as well.]
> 
> On Thu, 4 Jun 2020 at 13:55, Leif Lindholm <leif@nuviainc.com> wrote:
> > However, while looking at this, I noticed aarch64_a72_initfn doesn't
> > initialise kvm_target at all.
> 
> Yep. The kernel doesn't support "give me a CPU that looks like
> a Cortex-A72".
> 
> > So, then I decided to actually test things, and found that
> > (with -enable-kvm):
> > - on Cortex-A53 hardware
> >   - "max" kvm_target gets initialized to 4 (KVM_ARM_TARGET_CORTEX_A53)
> >     by kvm_arm_get_host_cpu_features (as returned from the kernel for
> >     vm_arm_create_scratch_host_vcpu)
> >   - cortex-A72 fails to start with "KVM is not supported for this guest
> >     CPU type"
> >   (fair enough, it's later than A53)
> 
> Untested, but I assume that -cpu cortex-a53 works on the A53...
> 
> > - on Cortex-A72 hardware
> >   - "max" kvm_target gets initialized to 5 (KVM_ARM_TARGET_GENERIC_V8)
> >     by kvm_arm_get_host_cpu_features
> >   - "cortex-A72" fails to start (umm...)
> 
> ...and fails on the A72 host.
> 
> > However ... if I haven't managed to confuse myself somewhere in here
> > (which is completely possible), would it be OK if I submitted a set of
> > patches that:
> > - add a QEMU_KVM_ARM_TARGET_GENERIC_V8 to match the kernel one
> > - set kvm_target for Cortex-A72 to QEMU_KVM_ARM_TARGET_GENERIC_V8
> 
> This would be wrong -- it would mean that you could tell QEMU "give
> me a guest CPU that's a Cortex-A72" and it would not error on
> non-A72 hardware but not actually give a guest CPU that looks
> like a Cortex-A72.
> 
>  * If what you want is "give me something that works" then that's
>    -cpu host or -cpu max.
> 
>  * If what you want is "give me something that's always this kind of
>    CPU regardless of the host hardware" then that's a lot of kernel
>    dev work nobody's been enthusiastic enough to undertake yet
>    (notably the "what do we do about CPU errata workarounds" question
>    would need to be solved...)
> 
>  * If what you want is "allow me to say '-cpu cortex-a72' and have
>    it work on an A72 host and not anywhere else" then I guess we could
>    implement that on the QEMU side by querying the MIDR and checking
>    whether it was what we expected.

It would match what we have for A53 and A57, so in that case it sounds
reasonable. OTOH, it may cause headaches when we want to finally say that
CPU models work, because it won't be clear to a user that just using
'-cpu cortex-a72' will really work, or just work on an A72 hosts like
before. We'll have that problem with A53 and A57 too, but maybe we
shouldn't add any more.

> 
> >   - alternatively drop the explicit settings for A57/A53
> 
> These explicit settings are correct, because for these CPUs
> the kernel does have a "give me what I want in particular"
> setting (which it will fail on the wrong h/w), and also as
> back-compat for older kernels that predate the GENERIC_V8
> define and only recognize the explicit "give me an A53" value.

Actually, I think the failing for the wrong hardware is about all these
older targets do. I didn't look real closely, but I think all targets
produce the same result for the guest, which is to pass through the host
ID registers.

> 
> > - drop the call from aarch64_max_initfn to aarch64_a57_initfn, and
> >   copy the relevant bits into the former for the !kvm case
> 
> Not sure why you care about this -- it's an implementation
> detail of the TCG handling of the 'max' CPU. There's an argument
> for disentangling TCG 'max' so it's not literally implemented
> as "a57 plus newer stuff", granted.
> 
> thanks
> -- PMM
>

Thanks,
drew

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Andrew Jones]

On Thu, Jun 04, 2020 at 02:10:08PM +0100, Peter Maydell wrote:
> [added kvm-arm to the cc list; the kernel folks tend to hang out
> there, not on qemu-devel, so KVM related questions are usually
> worth raising there as well.]
> 
> On Thu, 4 Jun 2020 at 13:55, Leif Lindholm <leif@nuviainc.com> wrote:
> > However, while looking at this, I noticed aarch64_a72_initfn doesn't
> > initialise kvm_target at all.
> 
> Yep. The kernel doesn't support "give me a CPU that looks like
> a Cortex-A72".
> 
> > So, then I decided to actually test things, and found that
> > (with -enable-kvm):
> > - on Cortex-A53 hardware
> >   - "max" kvm_target gets initialized to 4 (KVM_ARM_TARGET_CORTEX_A53)
> >     by kvm_arm_get_host_cpu_features (as returned from the kernel for
> >     vm_arm_create_scratch_host_vcpu)
> >   - cortex-A72 fails to start with "KVM is not supported for this guest
> >     CPU type"
> >   (fair enough, it's later than A53)
> 
> Untested, but I assume that -cpu cortex-a53 works on the A53...
> 
> > - on Cortex-A72 hardware
> >   - "max" kvm_target gets initialized to 5 (KVM_ARM_TARGET_GENERIC_V8)
> >     by kvm_arm_get_host_cpu_features
> >   - "cortex-A72" fails to start (umm...)
> 
> ...and fails on the A72 host.
> 
> > However ... if I haven't managed to confuse myself somewhere in here
> > (which is completely possible), would it be OK if I submitted a set of
> > patches that:
> > - add a QEMU_KVM_ARM_TARGET_GENERIC_V8 to match the kernel one
> > - set kvm_target for Cortex-A72 to QEMU_KVM_ARM_TARGET_GENERIC_V8
> 
> This would be wrong -- it would mean that you could tell QEMU "give
> me a guest CPU that's a Cortex-A72" and it would not error on
> non-A72 hardware but not actually give a guest CPU that looks
> like a Cortex-A72.
> 
>  * If what you want is "give me something that works" then that's
>    -cpu host or -cpu max.
> 
>  * If what you want is "give me something that's always this kind of
>    CPU regardless of the host hardware" then that's a lot of kernel
>    dev work nobody's been enthusiastic enough to undertake yet
>    (notably the "what do we do about CPU errata workarounds" question
>    would need to be solved...)
> 
>  * If what you want is "allow me to say '-cpu cortex-a72' and have
>    it work on an A72 host and not anywhere else" then I guess we could
>    implement that on the QEMU side by querying the MIDR and checking
>    whether it was what we expected.

It would match what we have for A53 and A57, so in that case it sounds
reasonable. OTOH, it may cause headaches when we want to finally say that
CPU models work, because it won't be clear to a user that just using
'-cpu cortex-a72' will really work, or just work on an A72 hosts like
before. We'll have that problem with A53 and A57 too, but maybe we
shouldn't add any more.

> 
> >   - alternatively drop the explicit settings for A57/A53
> 
> These explicit settings are correct, because for these CPUs
> the kernel does have a "give me what I want in particular"
> setting (which it will fail on the wrong h/w), and also as
> back-compat for older kernels that predate the GENERIC_V8
> define and only recognize the explicit "give me an A53" value.

Actually, I think the failing for the wrong hardware is about all these
older targets do. I didn't look real closely, but I think all targets
produce the same result for the guest, which is to pass through the host
ID registers.

> 
> > - drop the call from aarch64_max_initfn to aarch64_a57_initfn, and
> >   copy the relevant bits into the former for the !kvm case
> 
> Not sure why you care about this -- it's an implementation
> detail of the TCG handling of the 'max' CPU. There's an argument
> for disentangling TCG 'max' so it's not literally implemented
> as "a57 plus newer stuff", granted.
> 
> thanks
> -- PMM
>

Thanks,
drew

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Peter Maydell]

On Thu, 4 Jun 2020 at 14:32, Andrew Jones <drjones@redhat.com> wrote:
> On Thu, Jun 04, 2020 at 02:10:08PM +0100, Peter Maydell wrote:
> > These explicit settings are correct, because for these CPUs
> > the kernel does have a "give me what I want in particular"
> > setting (which it will fail on the wrong h/w), and also as
> > back-compat for older kernels that predate the GENERIC_V8
> > define and only recognize the explicit "give me an A53" value.
>
> Actually, I think the failing for the wrong hardware is about all these
> older targets do. I didn't look real closely, but I think all targets
> produce the same result for the guest, which is to pass through the host
> ID registers.

Yes; it's just that originally "specify CPU exactly" was the
only interface, and there wasn't a GENERIC_V8 at all.
I actually suspect that current QEMU will no longer work
on a kernel that's so old that it lacks the GENERIC_V8 and
PREFERRED_TARGET support[*], but we don't have an explicit "we need
at least host kernel version X" requirement that we track, so
it's hard to say for certain. (If we cared enough to test we
could likely delete a bit of back-compat handling code in QEMU.)

[*] in particular I have a feeling that recent changes to the
GIC handling code in the virt board implicitly dropped handling
for ancient kernels

thanks
-- PMM

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Peter Maydell]

On Thu, 4 Jun 2020 at 14:32, Andrew Jones <drjones@redhat.com> wrote:
> On Thu, Jun 04, 2020 at 02:10:08PM +0100, Peter Maydell wrote:
> > These explicit settings are correct, because for these CPUs
> > the kernel does have a "give me what I want in particular"
> > setting (which it will fail on the wrong h/w), and also as
> > back-compat for older kernels that predate the GENERIC_V8
> > define and only recognize the explicit "give me an A53" value.
>
> Actually, I think the failing for the wrong hardware is about all these
> older targets do. I didn't look real closely, but I think all targets
> produce the same result for the guest, which is to pass through the host
> ID registers.

Yes; it's just that originally "specify CPU exactly" was the
only interface, and there wasn't a GENERIC_V8 at all.
I actually suspect that current QEMU will no longer work
on a kernel that's so old that it lacks the GENERIC_V8 and
PREFERRED_TARGET support[*], but we don't have an explicit "we need
at least host kernel version X" requirement that we track, so
it's hard to say for certain. (If we cared enough to test we
could likely delete a bit of back-compat handling code in QEMU.)

[*] in particular I have a feeling that recent changes to the
GIC handling code in the virt board implicitly dropped handling
for ancient kernels

thanks
-- PMM
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Leif Lindholm]

On Thu, Jun 04, 2020 at 14:10:08 +0100, Peter Maydell wrote:
> [added kvm-arm to the cc list; the kernel folks tend to hang out
> there, not on qemu-devel, so KVM related questions are usually
> worth raising there as well.]
> 
> On Thu, 4 Jun 2020 at 13:55, Leif Lindholm <leif@nuviainc.com> wrote:
> > However, while looking at this, I noticed aarch64_a72_initfn doesn't
> > initialise kvm_target at all.
> 
> Yep. The kernel doesn't support "give me a CPU that looks like
> a Cortex-A72".
> 
> > So, then I decided to actually test things, and found that
> > (with -enable-kvm):
> > - on Cortex-A53 hardware
> >   - "max" kvm_target gets initialized to 4 (KVM_ARM_TARGET_CORTEX_A53)
> >     by kvm_arm_get_host_cpu_features (as returned from the kernel for
> >     vm_arm_create_scratch_host_vcpu)
> >   - cortex-A72 fails to start with "KVM is not supported for this guest
> >     CPU type"
> >   (fair enough, it's later than A53)
> 
> Untested, but I assume that -cpu cortex-a53 works on the A53...

Yes.

> > - on Cortex-A72 hardware
> >   - "max" kvm_target gets initialized to 5 (KVM_ARM_TARGET_GENERIC_V8)
> >     by kvm_arm_get_host_cpu_features
> >   - "cortex-A72" fails to start (umm...)
> 
> ...and fails on the A72 host.

From an explicit software test. If I initialize kvm_target to
KVM_ARM_TARGET_GENERIC_V8, I can certainly run EDK2.

> > However ... if I haven't managed to confuse myself somewhere in here
> > (which is completely possible), would it be OK if I submitted a set of
> > patches that:
> > - add a QEMU_KVM_ARM_TARGET_GENERIC_V8 to match the kernel one
> > - set kvm_target for Cortex-A72 to QEMU_KVM_ARM_TARGET_GENERIC_V8
> 
> This would be wrong -- it would mean that you could tell QEMU "give
> me a guest CPU that's a Cortex-A72" and it would not error on
> non-A72 hardware but not actually give a guest CPU that looks
> like a Cortex-A72.
> 
>  * If what you want is "give me something that works" then that's
>    -cpu host or -cpu max.

That's what I thought until I saw the manual A57/A53 setting of
kvm_target.

>  * If what you want is "give me something that's always this kind of
>    CPU regardless of the host hardware" then that's a lot of kernel
>    dev work nobody's been enthusiastic enough to undertake yet
>    (notably the "what do we do about CPU errata workarounds" question
>    would need to be solved...)
> 
>  * If what you want is "allow me to say '-cpu cortex-a72' and have
>    it work on an A72 host and not anywhere else" then I guess we could
>    implement that on the QEMU side by querying the MIDR and checking
>    whether it was what we expected.

I don't really.

> >   - alternatively drop the explicit settings for A57/A53
> 
> These explicit settings are correct, because for these CPUs
> the kernel does have a "give me what I want in particular"
> setting (which it will fail on the wrong h/w), and also as
> back-compat for older kernels that predate the GENERIC_V8
> define and only recognize the explicit "give me an A53" value.

Right. But then I got somewhat confused also by how
https://git.qemu.org/?p=qemu.git;a=blob;f=target/arm/kvm64.c#l494
doesn't explicitly list KVM_ARM_TARGET_CORTEX_A53.

> > - drop the call from aarch64_max_initfn to aarch64_a57_initfn, and
> >   copy the relevant bits into the former for the !kvm case
> 
> Not sure why you care about this -- it's an implementation
> detail of the TCG handling of the 'max' CPU. There's an argument
> for disentangling TCG 'max' so it's not literally implemented
> as "a57 plus newer stuff", granted.

So ... the reason I care is because I'm adding a new cpu in my local
branch, figured cpu64.c was a good starting point, and then followed a
long string of repeating the questions "why?" and "why not?" while
trying to understand why things were set up the way they are.

And I ended up in a state where it looks like we do some things for
A57 that we don't do for A53, and we do even fewer things for A72, but
then we do (end up doing) all of the A57 bits again for max (for TCG
only), and then overwriting them.

Then I tried to build some sort of consistent working model in my head
and send some questions off to the list rather than try to send out
patches straight away as I figured the likelihood was high I had
missed or misunderstood something.

I guess what I'm really asking is if there is some legacy in here
that can be cleaned up to make the expected behaviour for a new CPU
more clear from looking at available code? And if there are specific
legacy things that need to be kept around for compatibility that
should not be implemented by new CPUs, if they could have some nice
warnings attached.

Regards,

Leif

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Leif Lindholm]

On Thu, Jun 04, 2020 at 14:10:08 +0100, Peter Maydell wrote:
> [added kvm-arm to the cc list; the kernel folks tend to hang out
> there, not on qemu-devel, so KVM related questions are usually
> worth raising there as well.]
> 
> On Thu, 4 Jun 2020 at 13:55, Leif Lindholm <leif@nuviainc.com> wrote:
> > However, while looking at this, I noticed aarch64_a72_initfn doesn't
> > initialise kvm_target at all.
> 
> Yep. The kernel doesn't support "give me a CPU that looks like
> a Cortex-A72".
> 
> > So, then I decided to actually test things, and found that
> > (with -enable-kvm):
> > - on Cortex-A53 hardware
> >   - "max" kvm_target gets initialized to 4 (KVM_ARM_TARGET_CORTEX_A53)
> >     by kvm_arm_get_host_cpu_features (as returned from the kernel for
> >     vm_arm_create_scratch_host_vcpu)
> >   - cortex-A72 fails to start with "KVM is not supported for this guest
> >     CPU type"
> >   (fair enough, it's later than A53)
> 
> Untested, but I assume that -cpu cortex-a53 works on the A53...

Yes.

> > - on Cortex-A72 hardware
> >   - "max" kvm_target gets initialized to 5 (KVM_ARM_TARGET_GENERIC_V8)
> >     by kvm_arm_get_host_cpu_features
> >   - "cortex-A72" fails to start (umm...)
> 
> ...and fails on the A72 host.

From an explicit software test. If I initialize kvm_target to
KVM_ARM_TARGET_GENERIC_V8, I can certainly run EDK2.

> > However ... if I haven't managed to confuse myself somewhere in here
> > (which is completely possible), would it be OK if I submitted a set of
> > patches that:
> > - add a QEMU_KVM_ARM_TARGET_GENERIC_V8 to match the kernel one
> > - set kvm_target for Cortex-A72 to QEMU_KVM_ARM_TARGET_GENERIC_V8
> 
> This would be wrong -- it would mean that you could tell QEMU "give
> me a guest CPU that's a Cortex-A72" and it would not error on
> non-A72 hardware but not actually give a guest CPU that looks
> like a Cortex-A72.
> 
>  * If what you want is "give me something that works" then that's
>    -cpu host or -cpu max.

That's what I thought until I saw the manual A57/A53 setting of
kvm_target.

>  * If what you want is "give me something that's always this kind of
>    CPU regardless of the host hardware" then that's a lot of kernel
>    dev work nobody's been enthusiastic enough to undertake yet
>    (notably the "what do we do about CPU errata workarounds" question
>    would need to be solved...)
> 
>  * If what you want is "allow me to say '-cpu cortex-a72' and have
>    it work on an A72 host and not anywhere else" then I guess we could
>    implement that on the QEMU side by querying the MIDR and checking
>    whether it was what we expected.

I don't really.

> >   - alternatively drop the explicit settings for A57/A53
> 
> These explicit settings are correct, because for these CPUs
> the kernel does have a "give me what I want in particular"
> setting (which it will fail on the wrong h/w), and also as
> back-compat for older kernels that predate the GENERIC_V8
> define and only recognize the explicit "give me an A53" value.

Right. But then I got somewhat confused also by how
https://git.qemu.org/?p=qemu.git;a=blob;f=target/arm/kvm64.c#l494
doesn't explicitly list KVM_ARM_TARGET_CORTEX_A53.

> > - drop the call from aarch64_max_initfn to aarch64_a57_initfn, and
> >   copy the relevant bits into the former for the !kvm case
> 
> Not sure why you care about this -- it's an implementation
> detail of the TCG handling of the 'max' CPU. There's an argument
> for disentangling TCG 'max' so it's not literally implemented
> as "a57 plus newer stuff", granted.

So ... the reason I care is because I'm adding a new cpu in my local
branch, figured cpu64.c was a good starting point, and then followed a
long string of repeating the questions "why?" and "why not?" while
trying to understand why things were set up the way they are.

And I ended up in a state where it looks like we do some things for
A57 that we don't do for A53, and we do even fewer things for A72, but
then we do (end up doing) all of the A57 bits again for max (for TCG
only), and then overwriting them.

Then I tried to build some sort of consistent working model in my head
and send some questions off to the list rather than try to send out
patches straight away as I figured the likelihood was high I had
missed or misunderstood something.

I guess what I'm really asking is if there is some legacy in here
that can be cleaned up to make the expected behaviour for a new CPU
more clear from looking at available code? And if there are specific
legacy things that need to be kept around for compatibility that
should not be implemented by new CPUs, if they could have some nice
warnings attached.

Regards,

Leif
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Peter Maydell]

On Thu, 4 Jun 2020 at 16:38, Leif Lindholm <leif@nuviainc.com> wrote:
> On Thu, Jun 04, 2020 at 14:10:08 +0100, Peter Maydell wrote:
> > On Thu, 4 Jun 2020 at 13:55, Leif Lindholm <leif@nuviainc.com> wrote:
> > > So, then I decided to actually test things, and found that
> > > (with -enable-kvm):
> > > - on Cortex-A53 hardware
> > >   - "max" kvm_target gets initialized to 4 (KVM_ARM_TARGET_CORTEX_A53)
> > >     by kvm_arm_get_host_cpu_features (as returned from the kernel for
> > >     vm_arm_create_scratch_host_vcpu)
> > >   - cortex-A72 fails to start with "KVM is not supported for this guest
> > >     CPU type"
> > >   (fair enough, it's later than A53)
> >
> > Untested, but I assume that -cpu cortex-a53 works on the A53...
>
> Yes.
>
> > > - on Cortex-A72 hardware
> > >   - "max" kvm_target gets initialized to 5 (KVM_ARM_TARGET_GENERIC_V8)
> > >     by kvm_arm_get_host_cpu_features
> > >   - "cortex-A72" fails to start (umm...)
> >
> > ...and fails on the A72 host.
>
> From an explicit software test. If I initialize kvm_target to
> KVM_ARM_TARGET_GENERIC_V8, I can certainly run EDK2.

That would be asking for a -cpu max, though, not for an A53.

> > > However ... if I haven't managed to confuse myself somewhere in here
> > > (which is completely possible), would it be OK if I submitted a set of
> > > patches that:
> > > - add a QEMU_KVM_ARM_TARGET_GENERIC_V8 to match the kernel one
> > > - set kvm_target for Cortex-A72 to QEMU_KVM_ARM_TARGET_GENERIC_V8
> >
> > This would be wrong -- it would mean that you could tell QEMU "give
> > me a guest CPU that's a Cortex-A72" and it would not error on
> > non-A72 hardware but not actually give a guest CPU that looks
> > like a Cortex-A72.
> >
> >  * If what you want is "give me something that works" then that's
> >    -cpu host or -cpu max.
>
> That's what I thought until I saw the manual A57/A53 setting of
> kvm_target.

> > >   - alternatively drop the explicit settings for A57/A53
> >
> > These explicit settings are correct, because for these CPUs
> > the kernel does have a "give me what I want in particular"
> > setting (which it will fail on the wrong h/w), and also as
> > back-compat for older kernels that predate the GENERIC_V8
> > define and only recognize the explicit "give me an A53" value.
>
> Right. But then I got somewhat confused also by how
> https://git.qemu.org/?p=qemu.git;a=blob;f=target/arm/kvm64.c#l494
> doesn't explicitly list KVM_ARM_TARGET_CORTEX_A53.

That list is supposed to contain "all CPUs which might be
present on a host kernel which doesn't support the
PREFERRED_TARGET ioctl". That ioctl went in in Linux kernel
commit 42c4e0c77ac91, and in a kernel source tree
"git show 42c4e0c77ac91:arch/arm64/include/uapi/asm/kvm.h"
tells us that indeed at that point the only 3 CPUs supported
were AEM_V8, FOUNDATION_V8 and CORTEX_A57. Once the host
kernel supported the PREFERRED_TARGET ioctl, we could query
it to ask "what kind of CPU are you?" in that function rather
than having to guess.

> So ... the reason I care is because I'm adding a new cpu in my local
> branch, figured cpu64.c was a good starting point, and then followed a
> long string of repeating the questions "why?" and "why not?" while
> trying to understand why things were set up the way they are.
>
> And I ended up in a state where it looks like we do some things for
> A57 that we don't do for A53, and we do even fewer things for A72, but
> then we do (end up doing) all of the A57 bits again for max (for TCG
> only), and then overwriting them.

(we don't overwrite them, we augment them.)

> Then I tried to build some sort of consistent working model in my head
> and send some questions off to the list rather than try to send out
> patches straight away as I figured the likelihood was high I had
> missed or misunderstood something.
>
> I guess what I'm really asking is if there is some legacy in here
> that can be cleaned up to make the expected behaviour for a new CPU
> more clear from looking at available code? And if there are specific
> legacy things that need to be kept around for compatibility that
> should not be implemented by new CPUs, if they could have some nice
> warnings attached.

At this point I'd take another step backwards and ask
"why are you trying to add a new CPU?". Are you after a
TCG emulation of it, or are you trying to use KVM? If you're
using KVM, generally QEMU is set up so you don't need to tell
it about new CPU types at all. Mostly the cpu definitions for
specific CPUs in cpu64.c are there because we want them for TCG.

thanks
-- PMM

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Peter Maydell]

On Thu, 4 Jun 2020 at 16:38, Leif Lindholm <leif@nuviainc.com> wrote:
> On Thu, Jun 04, 2020 at 14:10:08 +0100, Peter Maydell wrote:
> > On Thu, 4 Jun 2020 at 13:55, Leif Lindholm <leif@nuviainc.com> wrote:
> > > So, then I decided to actually test things, and found that
> > > (with -enable-kvm):
> > > - on Cortex-A53 hardware
> > >   - "max" kvm_target gets initialized to 4 (KVM_ARM_TARGET_CORTEX_A53)
> > >     by kvm_arm_get_host_cpu_features (as returned from the kernel for
> > >     vm_arm_create_scratch_host_vcpu)
> > >   - cortex-A72 fails to start with "KVM is not supported for this guest
> > >     CPU type"
> > >   (fair enough, it's later than A53)
> >
> > Untested, but I assume that -cpu cortex-a53 works on the A53...
>
> Yes.
>
> > > - on Cortex-A72 hardware
> > >   - "max" kvm_target gets initialized to 5 (KVM_ARM_TARGET_GENERIC_V8)
> > >     by kvm_arm_get_host_cpu_features
> > >   - "cortex-A72" fails to start (umm...)
> >
> > ...and fails on the A72 host.
>
> From an explicit software test. If I initialize kvm_target to
> KVM_ARM_TARGET_GENERIC_V8, I can certainly run EDK2.

That would be asking for a -cpu max, though, not for an A53.

> > > However ... if I haven't managed to confuse myself somewhere in here
> > > (which is completely possible), would it be OK if I submitted a set of
> > > patches that:
> > > - add a QEMU_KVM_ARM_TARGET_GENERIC_V8 to match the kernel one
> > > - set kvm_target for Cortex-A72 to QEMU_KVM_ARM_TARGET_GENERIC_V8
> >
> > This would be wrong -- it would mean that you could tell QEMU "give
> > me a guest CPU that's a Cortex-A72" and it would not error on
> > non-A72 hardware but not actually give a guest CPU that looks
> > like a Cortex-A72.
> >
> >  * If what you want is "give me something that works" then that's
> >    -cpu host or -cpu max.
>
> That's what I thought until I saw the manual A57/A53 setting of
> kvm_target.

> > >   - alternatively drop the explicit settings for A57/A53
> >
> > These explicit settings are correct, because for these CPUs
> > the kernel does have a "give me what I want in particular"
> > setting (which it will fail on the wrong h/w), and also as
> > back-compat for older kernels that predate the GENERIC_V8
> > define and only recognize the explicit "give me an A53" value.
>
> Right. But then I got somewhat confused also by how
> https://git.qemu.org/?p=qemu.git;a=blob;f=target/arm/kvm64.c#l494
> doesn't explicitly list KVM_ARM_TARGET_CORTEX_A53.

That list is supposed to contain "all CPUs which might be
present on a host kernel which doesn't support the
PREFERRED_TARGET ioctl". That ioctl went in in Linux kernel
commit 42c4e0c77ac91, and in a kernel source tree
"git show 42c4e0c77ac91:arch/arm64/include/uapi/asm/kvm.h"
tells us that indeed at that point the only 3 CPUs supported
were AEM_V8, FOUNDATION_V8 and CORTEX_A57. Once the host
kernel supported the PREFERRED_TARGET ioctl, we could query
it to ask "what kind of CPU are you?" in that function rather
than having to guess.

> So ... the reason I care is because I'm adding a new cpu in my local
> branch, figured cpu64.c was a good starting point, and then followed a
> long string of repeating the questions "why?" and "why not?" while
> trying to understand why things were set up the way they are.
>
> And I ended up in a state where it looks like we do some things for
> A57 that we don't do for A53, and we do even fewer things for A72, but
> then we do (end up doing) all of the A57 bits again for max (for TCG
> only), and then overwriting them.

(we don't overwrite them, we augment them.)

> Then I tried to build some sort of consistent working model in my head
> and send some questions off to the list rather than try to send out
> patches straight away as I figured the likelihood was high I had
> missed or misunderstood something.
>
> I guess what I'm really asking is if there is some legacy in here
> that can be cleaned up to make the expected behaviour for a new CPU
> more clear from looking at available code? And if there are specific
> legacy things that need to be kept around for compatibility that
> should not be implemented by new CPUs, if they could have some nice
> warnings attached.

At this point I'd take another step backwards and ask
"why are you trying to add a new CPU?". Are you after a
TCG emulation of it, or are you trying to use KVM? If you're
using KVM, generally QEMU is set up so you don't need to tell
it about new CPU types at all. Mostly the cpu definitions for
specific CPUs in cpu64.c are there because we want them for TCG.

thanks
-- PMM
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Leif Lindholm]

On Thu, Jun 04, 2020 at 15:18:02 +0200, Andrew Jones wrote:
> > - drop the call from aarch64_max_initfn to aarch64_a57_initfn, and
> >   copy the relevant bits into the former for the !kvm case
> 
> I don't have a strong preference here, but if the naming is what's
> troublesome, then I'd think we're better off creating something like
> an aarch64_aXX_initfn() function and then calling it from both a57
> and max, and anywhere else it fits.

Well, the naming isn't the only issue, although there looks like a
certain amount of duplication could be deleted from a57/a53/a72 and
also be used for max if there was a common initfn.

But there's also things like:
- a57_initfn explicitly setting kvm_target, then only being called
  from max_initfn for !kvm_enabled()
- a57_initfn setting cpu->dtb_compatible to "arm,cortex-a57"
- a57 initfn setting cpu->midr, max_initfn overwriting parts of it

Best Regards,

Leif

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Peter Maydell]

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".

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?

> - a57 initfn setting cpu->midr, max_initfn overwriting parts of it

Also expected, TCG's -cpu max is "A57 with lots of extras".

The way we create a TCG -cpu max is a bit odd, as the code was
originally written in a situation where A57 was the most advanced
TCG CPU we had and there were no extra architectural features
supported by our CPU emulation. Today we have an A72 model as
well and a lot of extra architectural features, so the "code
borrowed" to "extras added" ratio looks a bit unbalanced.
Cleaning it up would not be a bad idea.

thanks
-- PMM

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Leif Lindholm]

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?

> 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". Still, for "max", would it not be
useful to update it to the track the most architecturally advanced cpu
supported? At this point "arm,cortex-a72".

> > - a57 initfn setting cpu->midr, max_initfn overwriting parts of it
> 
> Also expected, TCG's -cpu max is "A57 with lots of extras".

Maybe I'm being too rigid in my thinking here, but it does kind of jar
to see code call a function called aarch64_a57_initfn to have it write
a value where it then throws away the only thing in the value that
made it a57.

> The way we create a TCG -cpu max is a bit odd, as the code was
> originally written in a situation where A57 was the most advanced
> TCG CPU we had and there were no extra architectural features
> supported by our CPU emulation. Today we have an A72 model as
> well and a lot of extra architectural features, so the "code
> borrowed" to "extras added" ratio looks a bit unbalanced.
> Cleaning it up would not be a bad idea.

I might start by doing that bit. It might make a lot of the above
niggles simply disappear.

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?

Regards,

Leif

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Peter Maydell]

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.

> > 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.

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

> > > - a57 initfn setting cpu->midr, max_initfn overwriting parts of it
> >
> > Also expected, TCG's -cpu max is "A57 with lots of extras".
>
> Maybe I'm being too rigid in my thinking here, but it does kind of jar
> to see code call a function called aarch64_a57_initfn to have it write
> a value where it then throws away the only thing in the value that
> made it a57.

The things that make it an A57 are all the feature flag and ID
register values (which is is what tells the TCG implementation
how the CPU should behave). The MIDR value is the least interesting
bit of the a57_initfn in some ways: it doesn't change the QEMU
emulation's behaviour and mostly guests don't care what the value
is except for purposes of installing errata workarounds that don't
matter for QEMU: your average guest would work just as well with
any MIDR...

> 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?

thanks
-- PMM

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

Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Peter Maydell]

On Mon, 8 Jun 2020 at 13:02, Leif Lindholm <leif@nuviainc.com> wrote:
> On Thu, Jun 04, 2020 at 19:43:06 +0100, Peter Maydell wrote:
> > 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.

arm,armv8 is the generic "for software models" target, which
sounds appropriate enough to me. Anything consuming a dtb
presumably already knows whether it's in AArch64 state. If
you needed to be able to determine that from the device tree
then I think you'd be better off defining a property for it
rather than having some kind of lookup table of "these compat
string values imply 64-bit and these others do not".

However, I've just noticed that when the kernel added this to the
CPU binding list (apparently in passing during the conversion
to yaml) in commit 672951cbd1b70a9ede6f9c6eba4ed6b726d32b03
in 2018, it documented "arm,armv8", whereas the string QEMU uses
when KVM is being used is "arm,arm-v8" with a hyphen (or
"arm,arm-v7" for 32-bit KVM), which is what we've used since
2013. So I guess we need to also add the with-a-hyphen version
to the kernel binding documentation.

thanks
-- PMM