| Message ID | 20230125142056.18356-19-andy.chiu@sifive.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | riscv: Add vector ISA support | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 13 and now 13 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/alphanumeric_selects | success | Out of order selects before the patch: 57 and now 57 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 2 this patch: 2 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 33 lines checked |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
On Wed, Jan 25, 2023 at 7:53 PM Andy Chiu <andy.chiu@sifive.com> wrote: > > Running below m-mode, an illegal instruction trap where m-mode could not > handle would be redirected back to s-mode. However, kvm running in hs-mode > terminates the vs-mode software when it receive such exception code. > Instead, it should redirect the trap back to vs-mode, and let vs-mode trap > handler decide the next step. > > Besides, hs-mode should run transparently to vs-mode. So terminating > guest OS breaks assumption for the kernel running in vs-mode. > > We use first-use trap to enable Vector for user space processes. This > means that the user process running in u- or vu- mode will take an > illegal instruction trap for the first time using V. Then the s- or vs- > mode kernel would allocate V for the process. Thus, we must redirect the > trap back to vs-mode in order to get the first-use trap working for guest > OSes here. In general, it is a good strategy to always redirect illegal instruction traps to VS-mode. > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > --- > arch/riscv/kvm/vcpu_exit.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c > index c9f741ab26f5..2a02cb750892 100644 > --- a/arch/riscv/kvm/vcpu_exit.c > +++ b/arch/riscv/kvm/vcpu_exit.c > @@ -162,6 +162,16 @@ void kvm_riscv_vcpu_trap_redirect(struct kvm_vcpu *vcpu, > vcpu->arch.guest_context.sepc = csr_read(CSR_VSTVEC); > } > > +static int vcpu_trap_redirect_vs(struct kvm_vcpu *vcpu, > + struct kvm_cpu_trap *trap) > +{ > + /* set up trap handler and trap info when it gets back to vs */ > + kvm_riscv_vcpu_trap_redirect(vcpu, trap); > + /* return to s-mode by setting vcpu's SPP */ > + vcpu->arch.guest_context.sstatus |= SR_SPP; Setting sstatus.SPP needs to be done in kvm_riscv_vcpu_trap_redirect() because for guest all traps are always taken by VS-mode. > + return 1; > +} > + > /* > * Return > 0 to return to guest, < 0 on error, 0 (and set exit_reason) on > * proper exit to userspace. > @@ -179,6 +189,10 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run, > ret = -EFAULT; > run->exit_reason = KVM_EXIT_UNKNOWN; > switch (trap->scause) { > + case EXC_INST_ILLEGAL: > + if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV) > + ret = vcpu_trap_redirect_vs(vcpu, trap); > + break; > case EXC_VIRTUAL_INST_FAULT: > if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV) > ret = kvm_riscv_vcpu_virtual_insn(vcpu, run, trap); > @@ -206,6 +220,7 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run, > vcpu->arch.guest_context.hstatus); > kvm_err("SCAUSE=0x%lx STVAL=0x%lx HTVAL=0x%lx HTINST=0x%lx\n", > trap->scause, trap->stval, trap->htval, trap->htinst); > + asm volatile ("ebreak\n\t"); This is not a related change. > } > > return ret; > -- > 2.17.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv Overall, this patch can be accepted independent of this series due to its usefulness. I send a v2 of this patch separately. Regards, Anup
On Fri, Jan 27, 2023 at 7:28 PM Anup Patel <apatel@ventanamicro.com> wrote: > > On Wed, Jan 25, 2023 at 7:53 PM Andy Chiu <andy.chiu@sifive.com> wrote: > > > > Running below m-mode, an illegal instruction trap where m-mode could not > > handle would be redirected back to s-mode. However, kvm running in hs-mode > > terminates the vs-mode software when it receive such exception code. > > Instead, it should redirect the trap back to vs-mode, and let vs-mode trap > > handler decide the next step. > > > > Besides, hs-mode should run transparently to vs-mode. So terminating > > guest OS breaks assumption for the kernel running in vs-mode. > > > > We use first-use trap to enable Vector for user space processes. This > > means that the user process running in u- or vu- mode will take an > > illegal instruction trap for the first time using V. Then the s- or vs- > > mode kernel would allocate V for the process. Thus, we must redirect the > > trap back to vs-mode in order to get the first-use trap working for guest > > OSes here. > > In general, it is a good strategy to always redirect illegal instruction > traps to VS-mode. > > > > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > > --- > > arch/riscv/kvm/vcpu_exit.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c > > index c9f741ab26f5..2a02cb750892 100644 > > --- a/arch/riscv/kvm/vcpu_exit.c > > +++ b/arch/riscv/kvm/vcpu_exit.c > > @@ -162,6 +162,16 @@ void kvm_riscv_vcpu_trap_redirect(struct kvm_vcpu *vcpu, > > vcpu->arch.guest_context.sepc = csr_read(CSR_VSTVEC); > > } > > > > +static int vcpu_trap_redirect_vs(struct kvm_vcpu *vcpu, > > + struct kvm_cpu_trap *trap) > > +{ > > + /* set up trap handler and trap info when it gets back to vs */ > > + kvm_riscv_vcpu_trap_redirect(vcpu, trap); > > + /* return to s-mode by setting vcpu's SPP */ > > + vcpu->arch.guest_context.sstatus |= SR_SPP; > > Setting sstatus.SPP needs to be done in kvm_riscv_vcpu_trap_redirect() > because for guest all traps are always taken by VS-mode. NIce. Sorry that I didn't dig much into the kvm part so I thought it was left to VU-mode on purpose. > > > + return 1; > > +} > > + > > /* > > * Return > 0 to return to guest, < 0 on error, 0 (and set exit_reason) on > > * proper exit to userspace. > > @@ -179,6 +189,10 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run, > > ret = -EFAULT; > > run->exit_reason = KVM_EXIT_UNKNOWN; > > switch (trap->scause) { > > + case EXC_INST_ILLEGAL: > > + if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV) > > + ret = vcpu_trap_redirect_vs(vcpu, trap); > > + break; > > case EXC_VIRTUAL_INST_FAULT: > > if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV) > > ret = kvm_riscv_vcpu_virtual_insn(vcpu, run, trap); > > @@ -206,6 +220,7 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run, > > vcpu->arch.guest_context.hstatus); > > kvm_err("SCAUSE=0x%lx STVAL=0x%lx HTVAL=0x%lx HTINST=0x%lx\n", > > trap->scause, trap->stval, trap->htval, trap->htinst); > > + asm volatile ("ebreak\n\t"); > > This is not a related change. > Oops, that was a mistake. > > } > > > > return ret; > > -- > > 2.17.1 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > Overall, this patch can be accepted independent of this series due > to its usefulness. > > I send a v2 of this patch separately. Thank you Anup. I will leave this patch there for the following revision of the vector patches. Cheers, Andy
diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c index c9f741ab26f5..2a02cb750892 100644 --- a/arch/riscv/kvm/vcpu_exit.c +++ b/arch/riscv/kvm/vcpu_exit.c @@ -162,6 +162,16 @@ void kvm_riscv_vcpu_trap_redirect(struct kvm_vcpu *vcpu, vcpu->arch.guest_context.sepc = csr_read(CSR_VSTVEC); } +static int vcpu_trap_redirect_vs(struct kvm_vcpu *vcpu, + struct kvm_cpu_trap *trap) +{ + /* set up trap handler and trap info when it gets back to vs */ + kvm_riscv_vcpu_trap_redirect(vcpu, trap); + /* return to s-mode by setting vcpu's SPP */ + vcpu->arch.guest_context.sstatus |= SR_SPP; + return 1; +} + /* * Return > 0 to return to guest, < 0 on error, 0 (and set exit_reason) on * proper exit to userspace. @@ -179,6 +189,10 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run, ret = -EFAULT; run->exit_reason = KVM_EXIT_UNKNOWN; switch (trap->scause) { + case EXC_INST_ILLEGAL: + if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV) + ret = vcpu_trap_redirect_vs(vcpu, trap); + break; case EXC_VIRTUAL_INST_FAULT: if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV) ret = kvm_riscv_vcpu_virtual_insn(vcpu, run, trap); @@ -206,6 +220,7 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run, vcpu->arch.guest_context.hstatus); kvm_err("SCAUSE=0x%lx STVAL=0x%lx HTVAL=0x%lx HTINST=0x%lx\n", trap->scause, trap->stval, trap->htval, trap->htinst); + asm volatile ("ebreak\n\t"); } return ret;
Running below m-mode, an illegal instruction trap where m-mode could not handle would be redirected back to s-mode. However, kvm running in hs-mode terminates the vs-mode software when it receive such exception code. Instead, it should redirect the trap back to vs-mode, and let vs-mode trap handler decide the next step. Besides, hs-mode should run transparently to vs-mode. So terminating guest OS breaks assumption for the kernel running in vs-mode. We use first-use trap to enable Vector for user space processes. This means that the user process running in u- or vu- mode will take an illegal instruction trap for the first time using V. Then the s- or vs- mode kernel would allocate V for the process. Thus, we must redirect the trap back to vs-mode in order to get the first-use trap working for guest OSes here. Signed-off-by: Andy Chiu <andy.chiu@sifive.com> --- arch/riscv/kvm/vcpu_exit.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)