KVM: RISC-V: VCPU reset fixes

[PATCH 1/5] KVM: RISC-V: refactor vector state reset

Posted by Radim Krčmář 10 months, 1 week ago

Do not depend on the reset structures.

vector.datap is a kernel memory pointer that needs to be preserved as it
is not a part of the guest vector data.

Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>
---
 arch/riscv/include/asm/kvm_vcpu_vector.h |  6 ++----
 arch/riscv/kvm/vcpu.c                    |  5 ++++-
 arch/riscv/kvm/vcpu_vector.c             | 13 +++++++------
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/include/asm/kvm_vcpu_vector.h b/arch/riscv/include/asm/kvm_vcpu_vector.h
index 27f5bccdd8b0..57a798a4cb0d 100644
--- a/arch/riscv/include/asm/kvm_vcpu_vector.h
+++ b/arch/riscv/include/asm/kvm_vcpu_vector.h
@@ -33,8 +33,7 @@ void kvm_riscv_vcpu_guest_vector_restore(struct kvm_cpu_context *cntx,
 					 unsigned long *isa);
 void kvm_riscv_vcpu_host_vector_save(struct kvm_cpu_context *cntx);
 void kvm_riscv_vcpu_host_vector_restore(struct kvm_cpu_context *cntx);
-int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu,
-					struct kvm_cpu_context *cntx);
+int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu);
 void kvm_riscv_vcpu_free_vector_context(struct kvm_vcpu *vcpu);
 #else
 
@@ -62,8 +61,7 @@ static inline void kvm_riscv_vcpu_host_vector_restore(struct kvm_cpu_context *cn
 {
 }
 
-static inline int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu,
-						      struct kvm_cpu_context *cntx)
+static inline int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu)
 {
 	return 0;
 }
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index 60d684c76c58..2fb75288ecfe 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -57,6 +57,7 @@ static void kvm_riscv_reset_vcpu(struct kvm_vcpu *vcpu)
 	struct kvm_vcpu_csr *reset_csr = &vcpu->arch.guest_reset_csr;
 	struct kvm_cpu_context *cntx = &vcpu->arch.guest_context;
 	struct kvm_cpu_context *reset_cntx = &vcpu->arch.guest_reset_context;
+	void *vector_datap = cntx->vector.datap;
 	bool loaded;
 
 	/**
@@ -79,6 +80,8 @@ static void kvm_riscv_reset_vcpu(struct kvm_vcpu *vcpu)
 
 	kvm_riscv_vcpu_fp_reset(vcpu);
 
+	/* Restore datap as it's not a part of the guest context. */
+	cntx->vector.datap = vector_datap;
 	kvm_riscv_vcpu_vector_reset(vcpu);
 
 	kvm_riscv_vcpu_timer_reset(vcpu);
@@ -143,7 +146,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 	cntx->hstatus |= HSTATUS_SPV;
 	spin_unlock(&vcpu->arch.reset_cntx_lock);
 
-	if (kvm_riscv_vcpu_alloc_vector_context(vcpu, cntx))
+	if (kvm_riscv_vcpu_alloc_vector_context(vcpu))
 		return -ENOMEM;
 
 	/* By default, make CY, TM, and IR counters accessible in VU mode */
diff --git a/arch/riscv/kvm/vcpu_vector.c b/arch/riscv/kvm/vcpu_vector.c
index d92d1348045c..a5f88cb717f3 100644
--- a/arch/riscv/kvm/vcpu_vector.c
+++ b/arch/riscv/kvm/vcpu_vector.c
@@ -22,6 +22,9 @@ void kvm_riscv_vcpu_vector_reset(struct kvm_vcpu *vcpu)
 	struct kvm_cpu_context *cntx = &vcpu->arch.guest_context;
 
 	cntx->sstatus &= ~SR_VS;
+
+	cntx->vector.vlenb = riscv_v_vsize / 32;
+
 	if (riscv_isa_extension_available(isa, v)) {
 		cntx->sstatus |= SR_VS_INITIAL;
 		WARN_ON(!cntx->vector.datap);
@@ -70,13 +73,11 @@ void kvm_riscv_vcpu_host_vector_restore(struct kvm_cpu_context *cntx)
 		__kvm_riscv_vector_restore(cntx);
 }
 
-int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu,
-					struct kvm_cpu_context *cntx)
+int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu)
 {
-	cntx->vector.datap = kmalloc(riscv_v_vsize, GFP_KERNEL);
-	if (!cntx->vector.datap)
+	vcpu->arch.guest_context.vector.datap = kzalloc(riscv_v_vsize, GFP_KERNEL);
+	if (!vcpu->arch.guest_context.vector.datap)
 		return -ENOMEM;
-	cntx->vector.vlenb = riscv_v_vsize / 32;
 
 	vcpu->arch.host_context.vector.datap = kzalloc(riscv_v_vsize, GFP_KERNEL);
 	if (!vcpu->arch.host_context.vector.datap)
@@ -87,7 +88,7 @@ int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu,
 
 void kvm_riscv_vcpu_free_vector_context(struct kvm_vcpu *vcpu)
 {
-	kfree(vcpu->arch.guest_reset_context.vector.datap);
+	kfree(vcpu->arch.guest_context.vector.datap);
 	kfree(vcpu->arch.host_context.vector.datap);
 }
 #endif
-- 
2.48.1

Re: [PATCH 1/5] KVM: RISC-V: refactor vector state reset

Posted by Anup Patel 9 months ago

On Thu, Apr 3, 2025 at 5:02 PM Radim Krčmář <rkrcmar@ventanamicro.com> wrote:
>
> Do not depend on the reset structures.
>
> vector.datap is a kernel memory pointer that needs to be preserved as it
> is not a part of the guest vector data.
>
> Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>

Queued this patch for Linux-6.16

Thanks,
Anup

> ---
>  arch/riscv/include/asm/kvm_vcpu_vector.h |  6 ++----
>  arch/riscv/kvm/vcpu.c                    |  5 ++++-
>  arch/riscv/kvm/vcpu_vector.c             | 13 +++++++------
>  3 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/arch/riscv/include/asm/kvm_vcpu_vector.h b/arch/riscv/include/asm/kvm_vcpu_vector.h
> index 27f5bccdd8b0..57a798a4cb0d 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_vector.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_vector.h
> @@ -33,8 +33,7 @@ void kvm_riscv_vcpu_guest_vector_restore(struct kvm_cpu_context *cntx,
>                                          unsigned long *isa);
>  void kvm_riscv_vcpu_host_vector_save(struct kvm_cpu_context *cntx);
>  void kvm_riscv_vcpu_host_vector_restore(struct kvm_cpu_context *cntx);
> -int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu,
> -                                       struct kvm_cpu_context *cntx);
> +int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu);
>  void kvm_riscv_vcpu_free_vector_context(struct kvm_vcpu *vcpu);
>  #else
>
> @@ -62,8 +61,7 @@ static inline void kvm_riscv_vcpu_host_vector_restore(struct kvm_cpu_context *cn
>  {
>  }
>
> -static inline int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu,
> -                                                     struct kvm_cpu_context *cntx)
> +static inline int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu)
>  {
>         return 0;
>  }
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index 60d684c76c58..2fb75288ecfe 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -57,6 +57,7 @@ static void kvm_riscv_reset_vcpu(struct kvm_vcpu *vcpu)
>         struct kvm_vcpu_csr *reset_csr = &vcpu->arch.guest_reset_csr;
>         struct kvm_cpu_context *cntx = &vcpu->arch.guest_context;
>         struct kvm_cpu_context *reset_cntx = &vcpu->arch.guest_reset_context;
> +       void *vector_datap = cntx->vector.datap;
>         bool loaded;
>
>         /**
> @@ -79,6 +80,8 @@ static void kvm_riscv_reset_vcpu(struct kvm_vcpu *vcpu)
>
>         kvm_riscv_vcpu_fp_reset(vcpu);
>
> +       /* Restore datap as it's not a part of the guest context. */
> +       cntx->vector.datap = vector_datap;
>         kvm_riscv_vcpu_vector_reset(vcpu);
>
>         kvm_riscv_vcpu_timer_reset(vcpu);
> @@ -143,7 +146,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
>         cntx->hstatus |= HSTATUS_SPV;
>         spin_unlock(&vcpu->arch.reset_cntx_lock);
>
> -       if (kvm_riscv_vcpu_alloc_vector_context(vcpu, cntx))
> +       if (kvm_riscv_vcpu_alloc_vector_context(vcpu))
>                 return -ENOMEM;
>
>         /* By default, make CY, TM, and IR counters accessible in VU mode */
> diff --git a/arch/riscv/kvm/vcpu_vector.c b/arch/riscv/kvm/vcpu_vector.c
> index d92d1348045c..a5f88cb717f3 100644
> --- a/arch/riscv/kvm/vcpu_vector.c
> +++ b/arch/riscv/kvm/vcpu_vector.c
> @@ -22,6 +22,9 @@ void kvm_riscv_vcpu_vector_reset(struct kvm_vcpu *vcpu)
>         struct kvm_cpu_context *cntx = &vcpu->arch.guest_context;
>
>         cntx->sstatus &= ~SR_VS;
> +
> +       cntx->vector.vlenb = riscv_v_vsize / 32;
> +
>         if (riscv_isa_extension_available(isa, v)) {
>                 cntx->sstatus |= SR_VS_INITIAL;
>                 WARN_ON(!cntx->vector.datap);
> @@ -70,13 +73,11 @@ void kvm_riscv_vcpu_host_vector_restore(struct kvm_cpu_context *cntx)
>                 __kvm_riscv_vector_restore(cntx);
>  }
>
> -int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu,
> -                                       struct kvm_cpu_context *cntx)
> +int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu)
>  {
> -       cntx->vector.datap = kmalloc(riscv_v_vsize, GFP_KERNEL);
> -       if (!cntx->vector.datap)
> +       vcpu->arch.guest_context.vector.datap = kzalloc(riscv_v_vsize, GFP_KERNEL);
> +       if (!vcpu->arch.guest_context.vector.datap)
>                 return -ENOMEM;
> -       cntx->vector.vlenb = riscv_v_vsize / 32;
>
>         vcpu->arch.host_context.vector.datap = kzalloc(riscv_v_vsize, GFP_KERNEL);
>         if (!vcpu->arch.host_context.vector.datap)
> @@ -87,7 +88,7 @@ int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu,
>
>  void kvm_riscv_vcpu_free_vector_context(struct kvm_vcpu *vcpu)
>  {
> -       kfree(vcpu->arch.guest_reset_context.vector.datap);
> +       kfree(vcpu->arch.guest_context.vector.datap);
>         kfree(vcpu->arch.host_context.vector.datap);
>  }
>  #endif
> --
> 2.48.1
>

Re: [PATCH 1/5] KVM: RISC-V: refactor vector state reset

Posted by Andrew Jones 9 months, 2 weeks ago

On Thu, Apr 03, 2025 at 01:25:20PM +0200, Radim Krčmář wrote:
> Do not depend on the reset structures.
> 
> vector.datap is a kernel memory pointer that needs to be preserved as it
> is not a part of the guest vector data.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>
> ---
>  arch/riscv/include/asm/kvm_vcpu_vector.h |  6 ++----
>  arch/riscv/kvm/vcpu.c                    |  5 ++++-
>  arch/riscv/kvm/vcpu_vector.c             | 13 +++++++------
>  3 files changed, 13 insertions(+), 11 deletions(-)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 1/5] KVM: RISC-V: refactor vector state reset
[PATCH 2/5] KVM: RISC-V: refactor sbi reset request
[PATCH 3/5] KVM: RISC-V: remove unnecessary SBI reset state
[PATCH 4/5] KVM: RISC-V: reset VCPU state when becoming runnable
[PATCH 5/5] KVM: RISC-V: reset smstateen CSRs