Re: [Qemu-arm] [PATCH v3 15/22] target/arm: Add ARM_FEATURE_V7VE for v7 Virtualization Extensions

[Peter Maydell]

On 1 June 2018 at 16:34, Aaron Lindsay <address@hidden> wrote:
> On Jun 01 09:57, Peter Maydell wrote:
>> No; V7VE always implies ARM_DIV. (ARM_DIV doesn't imply V7VE,
>> though, which is why it is a separate feature bit.)
>
> Okay, then I'm confused about some of the preexisting logic in
> kvm_arm_get_host_cpu_features. The preexisting code in that function
> sets ARM_DIV and THUMB_DIV based on the appropriate bits in ID_ISAR0. If
> we already knew that
>>    (by definition a host CPU which supports KVM has v7VE.)
> and that all V7VE CPUs have ARM_DIV, why did the code there bother
> checking ID_ISAR0 to begin with?

Good question. I suspect I'd just forgotten that at the point
when I wrote that KVM code.

>> switch (extract32(id_isar0, 24, 4)) {
>> case 1:
>>     set_feature(&features, ARM_FEATURE_THUMB_DIV);
>>     break;
>> case 2:
>>     set_feature(&features, ARM_FEATURE_ARM_DIV);
>>     set_feature(&features, ARM_FEATURE_THUMB_DIV);
>>     break;
>> default:
>>     break;
>> }
>
> Should this switch/case be removed entirely?

Yes, I think so (and also the id_isar0 variable and the
idregs[] entry which arranges to initialize it). We should
just set the THUMB_DIV and ARM_DIV features explicitly.

thanks
-- PMM