diff mbox series

[03/16] KVM: arm64: Avoid remapping the SVE state in the hyp stage-1

Message ID 20211013155831.943476-4-qperret@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: arm64: Implement unshare hypercall for pkvm | expand

Commit Message

Quentin Perret Oct. 13, 2021, 3:58 p.m. UTC
We currently map the SVE state into the hypervisor stage-1 on VCPU_RUN,
when the vCPU thread's PID has changed. However, this only needs to be
done during the first VCPU_RUN as the SVE state doesn't depend on
thread-specific data, so move the create_hyp_mapping() call to
kvm_vcpu_first_run_init().

Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Quentin Perret <qperret@google.com>
---
 arch/arm64/kvm/arm.c    | 12 ++++++++++++
 arch/arm64/kvm/fpsimd.c | 11 -----------
 2 files changed, 12 insertions(+), 11 deletions(-)

Comments

Marc Zyngier Oct. 16, 2021, 11:04 a.m. UTC | #1
On Wed, 13 Oct 2021 16:58:18 +0100,
Quentin Perret <qperret@google.com> wrote:
> 
> We currently map the SVE state into the hypervisor stage-1 on VCPU_RUN,
> when the vCPU thread's PID has changed. However, this only needs to be
> done during the first VCPU_RUN as the SVE state doesn't depend on
> thread-specific data, so move the create_hyp_mapping() call to
> kvm_vcpu_first_run_init().
> 
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Quentin Perret <qperret@google.com>
> ---
>  arch/arm64/kvm/arm.c    | 12 ++++++++++++
>  arch/arm64/kvm/fpsimd.c | 11 -----------
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index fe102cd2e518..c33d8c073820 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -618,6 +618,18 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
>  	if (ret)
>  		return ret;
>  
> +	if (vcpu->arch.sve_state) {
> +		void *sve_end;
> +
> +		sve_end = vcpu->arch.sve_state + vcpu_sve_state_size(vcpu);
> +
> +		ret = create_hyp_mappings(vcpu->arch.sve_state, sve_end,
> +					  PAGE_HYP);
> +		if (ret)
> +			return ret;
> +	}
> +
> +
>  	ret = kvm_arm_pmu_v3_enable(vcpu);
>  
>  	return ret;
> diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> index 5621020b28de..62c0d78da7be 100644
> --- a/arch/arm64/kvm/fpsimd.c
> +++ b/arch/arm64/kvm/fpsimd.c
> @@ -43,17 +43,6 @@ int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
>  	if (ret)
>  		goto error;
>  
> -	if (vcpu->arch.sve_state) {
> -		void *sve_end;
> -
> -		sve_end = vcpu->arch.sve_state + vcpu_sve_state_size(vcpu);
> -
> -		ret = create_hyp_mappings(vcpu->arch.sve_state, sve_end,
> -					  PAGE_HYP);
> -		if (ret)
> -			goto error;
> -	}
> -
>  	vcpu->arch.host_thread_info = kern_hyp_va(ti);
>  	vcpu->arch.host_fpsimd_state = kern_hyp_va(fpsimd);
>  error:

I actually ended-up implementing a similar patch as part of my 'first
run' series[1], though I moved the mapping to the point where we
finalise the vcpu as that's where the allocation takes place.

Do you see any potential issue with that approach?

Thanks,

	M.

[1] https://lore.kernel.org/r/20211015090822.2994920-2-maz@kernel.org
Quentin Perret Oct. 18, 2021, 10:36 a.m. UTC | #2
On Saturday 16 Oct 2021 at 12:04:15 (+0100), Marc Zyngier wrote:
> I actually ended-up implementing a similar patch as part of my 'first
> run' series[1], though I moved the mapping to the point where we
> finalise the vcpu as that's where the allocation takes place.
> 
> Do you see any potential issue with that approach?

Nope, and in fact I think your patch should allow to simplify a bit
kvm_arm_vcpu_destroy() for me, as checking arch.sve_state != NULL would
now be sufficient to know if the sve state has been shared -- checking
arch.has_run_once is no longer needed.

I'll base v2 on this.

Thanks!
Quentin
diff mbox series

Patch

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index fe102cd2e518..c33d8c073820 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -618,6 +618,18 @@  static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
 	if (ret)
 		return ret;
 
+	if (vcpu->arch.sve_state) {
+		void *sve_end;
+
+		sve_end = vcpu->arch.sve_state + vcpu_sve_state_size(vcpu);
+
+		ret = create_hyp_mappings(vcpu->arch.sve_state, sve_end,
+					  PAGE_HYP);
+		if (ret)
+			return ret;
+	}
+
+
 	ret = kvm_arm_pmu_v3_enable(vcpu);
 
 	return ret;
diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
index 5621020b28de..62c0d78da7be 100644
--- a/arch/arm64/kvm/fpsimd.c
+++ b/arch/arm64/kvm/fpsimd.c
@@ -43,17 +43,6 @@  int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
 	if (ret)
 		goto error;
 
-	if (vcpu->arch.sve_state) {
-		void *sve_end;
-
-		sve_end = vcpu->arch.sve_state + vcpu_sve_state_size(vcpu);
-
-		ret = create_hyp_mappings(vcpu->arch.sve_state, sve_end,
-					  PAGE_HYP);
-		if (ret)
-			goto error;
-	}
-
 	vcpu->arch.host_thread_info = kern_hyp_va(ti);
 	vcpu->arch.host_fpsimd_state = kern_hyp_va(fpsimd);
 error: