Re: [PULL 09/11] target/arm/kvm: host cpu: Add support for sve<N> properties

From: Andrew Jones <drjones@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Richard Henderson <richard.henderson@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [PULL 09/11] target/arm/kvm: host cpu: Add support for sve<N> properties
Date: Fri, 4 Feb 2022 12:26:19 +0100	[thread overview]
Message-ID: <20220204112619.ouocuiwbzlv4n2gs@gator> (raw)
In-Reply-To: <CAFEAcA9_3DNozRsH8+iXbs2Z4-ar=Eki3ENvZocSmfbp+g13qQ@mail.gmail.com>

On Thu, Feb 03, 2022 at 05:40:20PM +0000, Peter Maydell wrote:
> On Thu, 3 Feb 2022 at 17:36, Andrew Jones <drjones@redhat.com> wrote:
> >
> > On Thu, Feb 03, 2022 at 04:46:21PM +0000, Peter Maydell wrote:
> > > Was this intentional?
> >
> > No, darn. I don't know how many times I rebased that series and was always
> > careful to ensure sve-max-vq was left in the non-kvm part of the above
> > condition. I guess the final rebase finally got me...
> >
> > >
> > > I'd like to fix up the weird divergence between -cpu host and
> > > -cpu max, either by moving sve-max-vq into aarch64_add_sve_properties()
> > > so it's present on both, or by changing the aarch64_max_initfn() so
> > > it only adds the property when using TCG.
> >
> > The later, please. sve-max-vq won't work for any of the machines that
> > support SVE that I know of, so I think it's a bad idea for KVM.
> >
> > >
> > > (I think also this code may get the '-cpu max,aarch64=off' case wrong,
> > > as it doesn't guard the calls to add the sve and pauth properties
> > > with the "if aarch64" feature check.)
> >
> > Yes, but these property dependencies may need to be checked at property
> > finalize time. That means that the properties may get added, but then
> > they will error out if the user tried to enable them. Otherwise, they'll
> > be disabled and the QMP query will inform the user that they cannot be
> > enabled.
> 
> Does 'max' need to do anything different from what we're doing
> already in arm_host_initfn() for 'host' ?
>

I don't think so. I think we want max+kvm == host and max+kvm to support
a strict subset of the properties that max+tcg supports. We could try to
provide all of max+tcg's properties to max+kvm, and then let them error
out when they bump into KVM limitations, but when we know they will likely
never work (e.g. sve-max-vq), then I think I still prefer just removing
them.

> (My proposal for
> fixing this stuff is basically to make aarch64_max_initfn()
> start with "if (kvm or hvf) { call arm_host_initfn(); return }".)

That sounds good to me. We'll get max+kvm == host that way for sure,
and I hope that host supports a strict subset of max's properties
already.

Thanks,
drew