[PATCH 01/12] KVM: arm/arm64: vgic: Remove spurious call to kern_hyp_va

[Suzuki K Poulose]

On 04/12/17 14:13, Steve Capper wrote:
> In save_elrsr(.), we use the following technique to ascertain the
> address of the vgic global state:
> 	(kern_hyp_va(&kvm_vgic_global_state))->nr_lr
> 
> For arm, kern_hyp_va(va) == va, and this call effectively compiles out.
> 
> For arm64, this call can be spurious as the address of kvm_vgic_global_state
> will usually be determined by relative page/absolute page offset relocation
> at link time. As the function is idempotent, having the call for arm64 does
> not cause any problems.
> 
> Unfortunately, this is about to change for arm64 as we need to change
> the logic of kern_hyp_va to allow for kernel addresses that are outside
> the direct linear map.
> 
> This patch removes the call to kern_hyp_va, and ensures that correct
> HYP addresses are computed via relative page offset addressing on arm64.
> This is achieved by a custom accessor, hyp_address(.), which on arm is a
> simple reference operator.

minor nit: I somehow feel that there word "symbol" should be part of the name of
the macro, to make it implicit that it can only be used on a symbol and not any
generic variable.

> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index 08d3bb66c8b7..34a4ae906a97 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -25,6 +25,16 @@
>   
>   #define __hyp_text __section(.hyp.text) notrace
>   
> +#define hyp_address(symbol)				\
> +({							\
> +	typeof(&symbol) __ret;				\
> +	asm volatile(					\
> +	"adrp %[ptr], " #symbol	"\n"			\
> +	"add %[ptr], %[ptr], :lo12:" #symbol "\n"	\
> +	: [ptr] "=r"(__ret));				\
> +	__ret;						\
> +})
> +

> -	addr  = kern_hyp_va((kern_hyp_va(&kvm_vgic_global_state))->vcpu_base_va);
> +	addr  = kern_hyp_va(hyp_address(kvm_vgic_global_state)->vcpu_base_va);

e.g, Like here, why do we use hyp_address only for the kvm_vgic_global_state and not
the dereferenced value. Having a name, say, hyp_symbol_address() makes it clear.

Otherwise, looks good to me.

Suzuki