Re: [PATCH v2 18/21] arm64: KVM: Introduce hyp_alternate_value helper

[Marc Zyngier]

On 01/02/16 14:41, Christoffer Dall wrote:
> On Mon, Jan 25, 2016 at 03:53:52PM +0000, Marc Zyngier wrote:
>> We already have hyp_alternate_select() to define a function pointer
>> that gets changed by a kernel feature or workaround.
>>
>> It would be useful to have a similar feature that resolves in a
>> direct value, without requiring a function call. For this purpose,
>> introduce hyp_alternate_value(), which returns one of two values
>> depending on the state of the alternative.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>> ---
>>  arch/arm64/kvm/hyp/hyp.h | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm64/kvm/hyp/hyp.h b/arch/arm64/kvm/hyp/hyp.h
>> index 44eaff7..dc75fdb 100644
>> --- a/arch/arm64/kvm/hyp/hyp.h
>> +++ b/arch/arm64/kvm/hyp/hyp.h
>> @@ -144,6 +144,17 @@ typeof(orig) * __hyp_text fname(void)					\
>>  	return val;							\
>>  }
>>  
>> +#define hyp_alternate_value(fname, orig, alt, cond)			\
>> +typeof(orig) __hyp_text fname(void)					\
>> +{									\
>> +	typeof(alt) val = orig;						\
>> +	asm volatile(ALTERNATIVE("nop		\n",			\
>> +				 "mov	%0, %1	\n",			\
>> +				 cond)					\
>> +		     : "+r" (val) : "r" ((typeof(orig))alt));		\
>> +	return val;							\
>> +}
>> +
>>  void __vgic_v2_save_state(struct kvm_vcpu *vcpu);
>>  void __vgic_v2_restore_state(struct kvm_vcpu *vcpu);
>>  
>> -- 
>> 2.1.4
>>
> I'm really not convinced that this is more readable than simply defining
> a function where needed.  Perhaps the thing that needs a definition is
> the "asm volatile(ALTERNATIVE(...))" part?  I also don't see why any of
> this is specific to KVM or Hyp ?

I can easily factor out the whole asm volatile part. What I'm trying to
avoid is an additional function call, but maybe we shouldn't need to
worry about the overhead on page faults altogether?

I'll drop it for now, and we can reconsider it later.

	M.
-- 
Jazz is not dead. It just smells funny...