On 17/06/20 17:23, Andrew Jones wrote:
>>
>> Fix by kvm_arm_<FEATURE>_supported() functions take a AccelState
>> argument (already realized/valid at this point) instead of a
>> CPUState argument.
> I'd rather not do that. IMO, a CPU feature test should operate on CPU,
> not an "accelerator".
If it's a test that the feature is enabled (e.g. via -cpu) then I agree.
For something that ends up as a KVM_CHECK_EXTENSION or KVM_ENABLE_CAP on
the KVM fd, however, I think passing an AccelState is better.
kvm_arm_pmu_supported case is clearly the latter, even the error message
hints at that:
+ if (kvm_enabled() && !kvm_arm_pmu_supported(current_accel())) {
error_setg(errp, "'pmu' feature not supported by KVM on this host");
return;
}
but the same is true of kvm_arm_aarch32_supported and kvm_arm_sve_supported.
Applying the change to kvm_arm_pmu_supported as you suggest below would be
a bit of a bandaid because it would not have consistent prototypes. Sp
for Philippe's patch
Acked-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Thanks,
Paolo
> How that test is implemented is another story.
> If the CPUState isn't interesting, but it points to something that is,
> or there's another function that uses globals to get the job done, then
> fine, but the callers of a CPU feature test shouldn't need to know that.
>
> I think we should just revert d70c996df23f and then apply the same
> change to kvm_arm_pmu_supported() that other similar functions got
> with 4f7f589381d5.
