Re: [Qemu-devel] [PATCH v3 03/15] target/arm/monitor: Introduce qmp_query_cpu_model_expansion

[Andrew Jones]

On Wed, Aug 07, 2019 at 08:22:07AM -0700, Richard Henderson wrote:
> On 8/6/19 5:21 AM, Andrew Jones wrote:
> > That's a reasonable suggestion. I do like having self-contained
> > validation, self-contained, but when cross-dependencies arise, then
> > it does make sense to have a master validation function, rather
> > than interconnecting too much. That said, for this series we only
> > enable the qmp query for aarch64, pmu, and sve* properties. aarch64
> > and pmu are independent, and thus self-contained...
> 
> Agreed.
> 
> > and I consider
> > all sve* properties one big entity, so their validation is also
> > self-contained. If we add vfp and neon, then indeed I was wrong
> > with my suggestion in the commit message. They would need a later
> > validation check. Should we just cross that bridge when we get there
> > though? Or would you like me to see how that would work within this
> > series?
> 
> While the sve* properties are handled by one function, they are not handled as
> "one big entity".  You examine then apply or diagnose the effects of sve384=on
> before you separately examine the effects of sve512=on.
> 
> I think it would be easiest to merely record facts while processing sve<N> and
> sve-max-vq, with no side effects.  Then in the common validation function see
> the required side effects and diagnose errors all at once.
>

I'm not sure. Of course I'd need to experiment with it to be sure, but
I'm reluctant to go through that exercise, because I believe that a
deferred validation will result in less specific errors messages. For
example, how would the validator know in which order the sve<N> properties
were provided? Which is necessary to complain that one length is not
allowed when another has already been disabled, or vice versa.

Also with deferred validation I think I'd need more vq states, at least
for when KVM is enabled. For example, if 384-bit vector lengths are not
supported on the KVM host, but the user does sve384=on, and all we do
is record that, then we've lost the KVM unsupported information and won't
find out until we attempt to enable it later at kvm vcpu init time. We'd
need a kvm-unsupported-user-enabled state to track that, which also means
we're not blindly recording user input in cpu_arm_set_sve_vq() anymore,
but are instead applying logic to decide which state we transition to.

Thanks,
drew