[PATCH 0/2] KVM: arm64: Dynamic IPA cosmetic fixups

[PATCH 0/2] KVM: arm64: Dynamic IPA cosmetic fixups

[Marc Zyngier]

Here's a couple of tiny cleanups I'd like to queue on top of Suzuki's
series. They are purely cosmetic, and just aim at making the code
slightly more readable.

If nobody shouts, I'll stack them on top.

Thanks,

	M.

Marc Zyngier (2):
  KVM: arm/arm64: Rename kvm_arm_config_vm to kvm_arm_setup_stage2
  KVM: arm64: Drop __cpu_init_stage2 on the VHE path

 arch/arm/include/asm/kvm_host.h   |  6 +++++-
 arch/arm64/include/asm/kvm_arm.h  |  2 +-
 arch/arm64/include/asm/kvm_host.h |  2 +-
 arch/arm64/kvm/reset.c            |  2 +-
 virt/kvm/arm/arm.c                | 12 +++---------
 5 files changed, 11 insertions(+), 13 deletions(-)

-- 
2.19.0

[PATCH 1/2] KVM: arm/arm64: Rename kvm_arm_config_vm to kvm_arm_setup_stage2

[Marc Zyngier]

VM tends to be a very overloaded term in KVM, so let's keep it
to describe the virtual machine. For the virtual memory setup,
let's use the "stage2" suffix.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/kvm_host.h   | 6 +++++-
 arch/arm64/include/asm/kvm_arm.h  | 2 +-
 arch/arm64/include/asm/kvm_host.h | 2 +-
 arch/arm64/kvm/reset.c            | 2 +-
 virt/kvm/arm/arm.c                | 2 +-
 5 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 72d46418e1ef..b45af481ccf7 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -354,8 +354,12 @@ static inline void kvm_vcpu_put_sysregs(struct kvm_vcpu *vcpu) {}
 struct kvm *kvm_arch_alloc_vm(void);
 void kvm_arch_free_vm(struct kvm *kvm);
 
-static inline int kvm_arm_config_vm(struct kvm *kvm, unsigned long type)
+static inline int kvm_arm_setup_stage2(struct kvm *kvm, unsigned long type)
 {
+	/*
+	 * On 32bit ARM, VMs get a static 40bit IPA stage2 setup,
+	 * so any non-zero value used as type is illegal.
+	 */
 	if (type)
 		return -EINVAL;
 	return 0;
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index f1330284720d..6e324d1f1231 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -133,7 +133,7 @@
  * 40 bits wide (T0SZ = 24).  Systems with a PARange smaller than 40 bits are
  * not known to exist and will break with this configuration.
  *
- * The VTCR_EL2 is configured per VM and is initialised in kvm_arm_config_vm().
+ * The VTCR_EL2 is configured per VM and is initialised in kvm_arm_setup_stage2().
  *
  * Note that when using 4K pages, we concatenate two first level page tables
  * together. With 16K pages, we concatenate 16 first level page tables.
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index f008f8866b2a..376a5b695467 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -511,6 +511,6 @@ void kvm_set_ipa_limit(void);
 struct kvm *kvm_arch_alloc_vm(void);
 void kvm_arch_free_vm(struct kvm *kvm);
 
-int kvm_arm_config_vm(struct kvm *kvm, unsigned long type);
+int kvm_arm_setup_stage2(struct kvm *kvm, unsigned long type);
 
 #endif /* __ARM64_KVM_HOST_H__ */
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 95f28d5950e0..aa806d582552 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -189,7 +189,7 @@ void kvm_set_ipa_limit(void)
  * all CPUs, as it is safe to run with or without the feature and
  * the bit is RES0 on CPUs that don't support it.
  */
-int kvm_arm_config_vm(struct kvm *kvm, unsigned long type)
+int kvm_arm_setup_stage2(struct kvm *kvm, unsigned long type)
 {
 	u64 vtcr = VTCR_EL2_FLAGS;
 	u32 parange, phys_shift;
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 631f9a3ad99a..91c464c9cd21 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -120,7 +120,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 {
 	int ret, cpu;
 
-	ret = kvm_arm_config_vm(kvm, type);
+	ret = kvm_arm_setup_stage2(kvm, type);
 	if (ret)
 		return ret;
 
-- 
2.19.0

Re: [PATCH 1/2] KVM: arm/arm64: Rename kvm_arm_config_vm to kvm_arm_setup_stage2

[Suzuki K Poulose]

On 10/01/2018 02:06 PM, Marc Zyngier wrote:
> VM tends to be a very overloaded term in KVM, so let's keep it
> to describe the virtual machine. For the virtual memory setup,
> let's use the "stage2" suffix.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

The intention was that the function would serve as the backend check
for the vm_type argument. That said, I am fine with the changes, FWIW:

Reviewed-by : Suzuki K Poulose <suzuki.poulose@arm.com>

Re: [PATCH 1/2] KVM: arm/arm64: Rename kvm_arm_config_vm to kvm_arm_setup_stage2

[Auger Eric]

Hi Marc,

On 10/1/18 3:06 PM, Marc Zyngier wrote:
> VM tends to be a very overloaded term in KVM, so let's keep it
> to describe the virtual machine. For the virtual memory setup,
> let's use the "stage2" suffix.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  arch/arm/include/asm/kvm_host.h   | 6 +++++-
>  arch/arm64/include/asm/kvm_arm.h  | 2 +-
>  arch/arm64/include/asm/kvm_host.h | 2 +-
>  arch/arm64/kvm/reset.c            | 2 +-
>  virt/kvm/arm/arm.c                | 2 +-
>  5 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 72d46418e1ef..b45af481ccf7 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -354,8 +354,12 @@ static inline void kvm_vcpu_put_sysregs(struct kvm_vcpu *vcpu) {}
>  struct kvm *kvm_arch_alloc_vm(void);
>  void kvm_arch_free_vm(struct kvm *kvm);
>  
> -static inline int kvm_arm_config_vm(struct kvm *kvm, unsigned long type)
> +static inline int kvm_arm_setup_stage2(struct kvm *kvm, unsigned long type)
>  {
> +	/*
> +	 * On 32bit ARM, VMs get a static 40bit IPA stage2 setup,
> +	 * so any non-zero value used as type is illegal.
> +	 */
>  	if (type)
>  		return -EINVAL;
>  	return 0;
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index f1330284720d..6e324d1f1231 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -133,7 +133,7 @@
>   * 40 bits wide (T0SZ = 24).  Systems with a PARange smaller than 40 bits are
>   * not known to exist and will break with this configuration.
>   *
> - * The VTCR_EL2 is configured per VM and is initialised in kvm_arm_config_vm().
> + * The VTCR_EL2 is configured per VM and is initialised in kvm_arm_setup_stage2().
>   *
>   * Note that when using 4K pages, we concatenate two first level page tables
>   * together. With 16K pages, we concatenate 16 first level page tables.
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index f008f8866b2a..376a5b695467 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -511,6 +511,6 @@ void kvm_set_ipa_limit(void);
>  struct kvm *kvm_arch_alloc_vm(void);
>  void kvm_arch_free_vm(struct kvm *kvm);
>  
> -int kvm_arm_config_vm(struct kvm *kvm, unsigned long type);
> +int kvm_arm_setup_stage2(struct kvm *kvm, unsigned long type);
>  
>  #endif /* __ARM64_KVM_HOST_H__ */
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 95f28d5950e0..aa806d582552 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -189,7 +189,7 @@ void kvm_set_ipa_limit(void)
>   * all CPUs, as it is safe to run with or without the feature and
>   * the bit is RES0 on CPUs that don't support it.
>   */
> -int kvm_arm_config_vm(struct kvm *kvm, unsigned long type)
> +int kvm_arm_setup_stage2(struct kvm *kvm, unsigned long type)
>  {
>  	u64 vtcr = VTCR_EL2_FLAGS;
>  	u32 parange, phys_shift;
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 631f9a3ad99a..91c464c9cd21 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -120,7 +120,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>  {
>  	int ret, cpu;
>  
> -	ret = kvm_arm_config_vm(kvm, type);
> +	ret = kvm_arm_setup_stage2(kvm, type);
>  	if (ret)
>  		return ret;
>  
> 

[PATCH 2/2] KVM: arm64: Drop __cpu_init_stage2 on the VHE path

[Marc Zyngier]

__cpu_init_stage2 doesn't do anything anymore on arm64, and is
totally non-sensical if running VHE (as VHE is 64bit only).

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 virt/kvm/arm/arm.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 91c464c9cd21..4ce99bb223bc 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1310,16 +1310,10 @@ static void cpu_hyp_reinit(void)
 {
 	cpu_hyp_reset();
 
-	if (is_kernel_in_hyp_mode()) {
-		/*
-		 * __cpu_init_stage2() is safe to call even if the PM
-		 * event was cancelled before the CPU was reset.
-		 */
-		__cpu_init_stage2();
+	if (is_kernel_in_hyp_mode())
 		kvm_timer_init_vhe();
-	} else {
+	else
 		cpu_init_hyp_mode(NULL);
-	}
 
 	if (vgic_present)
 		kvm_vgic_init_cpu_hardware();
-- 
2.19.0

Re: [PATCH 2/2] KVM: arm64: Drop __cpu_init_stage2 on the VHE path

[Suzuki K Poulose]

Marc,

On 10/01/2018 02:06 PM, Marc Zyngier wrote:
> __cpu_init_stage2 doesn't do anything anymore on arm64, and is
> totally non-sensical if running VHE (as VHE is 64bit only).
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   virt/kvm/arm/arm.c | 10 ++--------
>   1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 91c464c9cd21..4ce99bb223bc 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1310,16 +1310,10 @@ static void cpu_hyp_reinit(void)
>   {
>   	cpu_hyp_reset();
>   
> -	if (is_kernel_in_hyp_mode()) {
> -		/*
> -		 * __cpu_init_stage2() is safe to call even if the PM
> -		 * event was cancelled before the CPU was reset.
> -		 */
> -		__cpu_init_stage2();
> +	if (is_kernel_in_hyp_mode())
>   		kvm_timer_init_vhe();

The patch looks good to me. However, if we ever add something there in
the future, this caller site could be lost. Is it worth a comment ?

Besides,

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Re: [PATCH 2/2] KVM: arm64: Drop __cpu_init_stage2 on the VHE path

[Auger Eric]

Hi Marc,

On 10/1/18 3:06 PM, Marc Zyngier wrote:
> __cpu_init_stage2 doesn't do anything anymore on arm64, and is
> totally non-sensical if running VHE (as VHE is 64bit only).
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  virt/kvm/arm/arm.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 91c464c9cd21..4ce99bb223bc 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1310,16 +1310,10 @@ static void cpu_hyp_reinit(void)
>  {
>  	cpu_hyp_reset();
>  
> -	if (is_kernel_in_hyp_mode()) {
> -		/*
> -		 * __cpu_init_stage2() is safe to call even if the PM
> -		 * event was cancelled before the CPU was reset.
> -		 */
> -		__cpu_init_stage2();
> +	if (is_kernel_in_hyp_mode())
>  		kvm_timer_init_vhe();
> -	} else {
> +	else
>  		cpu_init_hyp_mode(NULL);
> -	}
>  
>  	if (vgic_present)
>  		kvm_vgic_init_cpu_hardware();
>