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

[Richard Henderson]

On 8/8/19 1:50 AM, Andrew Jones wrote:
> 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.

My point is that we would not *need* to know in which order the properties are
provided, and do not care.  Indeed, removing this ordering malarkey is a
feature not a bug.

The error message would simply state, e.g., that sve256 may not be disabled
while sve512 is enabled.  The error message does not need to reference the
order in which they appeared.

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

Or perhaps, less vq states.  You do not need to compute kvm states early.  You
can wait to collect those until validation time and keep them in their own
local bitmap.

bool validate_sve_properties(...)
{
    // Populate uninitialized bits in sve_vq_map from sve_max_vq.
    // Populate uninitialized bits in sve_vq_map from on bits in sve_vq_map.
    // All remaining uninitialized bits are set to off.
    // Reset sve_max_vq to the maximum bit set in sve_vq_map, plus 1.
    // Diagnose off bits in sve_vq_map from on bits in sve_vq_map.

    if (kvm_enabled()) {
        DECLARE_BITMAP(test, ARM_MAX_VQ);
        kvm_arm_sve_get_vls(CPU(cpu), test);
        if (!bitmap_equal(test, s->sve_vq_map, s->sve_max_vq))  {
            bitmap_xor(test, test, s->sve_vq_map, s->sve_max_vq);
            // Diagnose the differences in TEST:
            // test[i] & s->sve_vq_map[i] -> i is unsupported by kvm
            // test[i] & !s->sve_vq_map[i] -> i is required by kvm
        }
    }
}

If you prefer not to experiment with this, then I will.


r~