[PATCH 2/3] KVM: arm64: Access CNTHCTL_EL2 bit fields correctly on VHE systems

[Suzuki K Poulose]

On 16/01/17 14:11, Marc Zyngier wrote:
> On 16/01/17 13:30, Marc Zyngier wrote:
>> On 13/01/17 14:56, Suzuki K Poulose wrote:
>>> On 13/01/17 13:30, Marc Zyngier wrote:
>>>> [+ Suzuki, who wrote the whole cpus_have_const_cap thing]
>>>>
>>
>> [...]
>>
>>>> But maybe we should have have some stronger guarantees that we'll
>>>> always get things inlined, and that the "const" side is enforced:
>>>
>>> Agreed.
>>>
>>>>
>>>> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
>>>> index b4989df..4710469 100644
>>>> --- a/arch/arm64/include/asm/cpufeature.h
>>>> +++ b/arch/arm64/include/asm/cpufeature.h
>>>> @@ -105,10 +105,11 @@ static inline bool cpu_have_feature(unsigned int num)
>>>>  }
>>>>
>>>>  /* System capability check for constant caps */
>>>> -static inline bool cpus_have_const_cap(int num)
>>>> +static __always_inline bool cpus_have_const_cap(int num)
>>>
>>> I think we should have the above change and make it inline always.
>>>
>>>>  {
>>>> -	if (num >= ARM64_NCAPS)
>>>> -		return false;
>>>> +	BUILD_BUG_ON(!__builtin_constant_p(num));
>>>
>>> This is not needed, as the compilation would fail if num is not a constant with
>>> static key code.
>
> I also just checked this, and it doesn't fail if the compiler doesn't
> directly supports jump labels (we then fallback to the static key being
> a standard memory access).

Ah, I missed that part of the story. Sorry about that. Please go ahead with the
changes. I had a similar check in my first version and was dropped later with a
similar review comment. We hadn't considered older tool chain.


Suzuki