| Message ID | 20190904161245.111924-13-anup.patel@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM RISC-V Support | expand |
On 04.09.19 18:15, Anup Patel wrote: > We get illegal instruction trap whenever Guest/VM executes WFI > instruction. > > This patch handles WFI trap by blocking the trapped VCPU using > kvm_vcpu_block() API. The blocked VCPU will be automatically > resumed whenever a VCPU interrupt is injected from user-space > or from in-kernel IRQCHIP emulation. > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > Acked-by: Paolo Bonzini <pbonzini@redhat.com> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/riscv/kvm/vcpu_exit.c | 72 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 72 insertions(+) > > diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c > index d75a6c35b6c7..39469f67b241 100644 > --- a/arch/riscv/kvm/vcpu_exit.c > +++ b/arch/riscv/kvm/vcpu_exit.c > @@ -12,6 +12,13 @@ > #include <linux/kvm_host.h> > #include <asm/csr.h> > > +#define INSN_OPCODE_MASK 0x007c > +#define INSN_OPCODE_SHIFT 2 > +#define INSN_OPCODE_SYSTEM 28 > + > +#define INSN_MASK_WFI 0xffffff00 > +#define INSN_MATCH_WFI 0x10500000 > + > #define INSN_MATCH_LB 0x3 > #define INSN_MASK_LB 0x707f > #define INSN_MATCH_LH 0x1003 > @@ -112,6 +119,67 @@ > (s32)(((insn) >> 7) & 0x1f)) > #define MASK_FUNCT3 0x7000 > > +static int truly_illegal_insn(struct kvm_vcpu *vcpu, > + struct kvm_run *run, > + ulong insn) > +{ > + /* Redirect trap to Guest VCPU */ > + kvm_riscv_vcpu_trap_redirect(vcpu, EXC_INST_ILLEGAL, insn); > + > + return 1; > +} > + > +static int system_opcode_insn(struct kvm_vcpu *vcpu, > + struct kvm_run *run, > + ulong insn) > +{ > + if ((insn & INSN_MASK_WFI) == INSN_MATCH_WFI) { > + vcpu->stat.wfi_exit_stat++; > + if (!kvm_arch_vcpu_runnable(vcpu)) { > + srcu_read_unlock(&vcpu->kvm->srcu, vcpu->arch.srcu_idx); > + kvm_vcpu_block(vcpu); > + vcpu->arch.srcu_idx = srcu_read_lock(&vcpu->kvm->srcu); > + kvm_clear_request(KVM_REQ_UNHALT, vcpu); > + } > + vcpu->arch.guest_context.sepc += INSN_LEN(insn); > + return 1; > + } > + > + return truly_illegal_insn(vcpu, run, insn); > +} > + > +static int illegal_inst_fault(struct kvm_vcpu *vcpu, struct kvm_run *run, > + unsigned long insn) > +{ > + unsigned long ut_scause = 0; > + struct kvm_cpu_context *ct; > + > + if (unlikely((insn & 3) != 3)) { What do the low 2 bits mean here? Maybe you can use a define instead? Alex Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
On Mon, Sep 23, 2019 at 12:24 PM Alexander Graf <graf@amazon.com> wrote: > > > > On 04.09.19 18:15, Anup Patel wrote: > > We get illegal instruction trap whenever Guest/VM executes WFI > > instruction. > > > > This patch handles WFI trap by blocking the trapped VCPU using > > kvm_vcpu_block() API. The blocked VCPU will be automatically > > resumed whenever a VCPU interrupt is injected from user-space > > or from in-kernel IRQCHIP emulation. > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > Acked-by: Paolo Bonzini <pbonzini@redhat.com> > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > > --- > > arch/riscv/kvm/vcpu_exit.c | 72 ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 72 insertions(+) > > > > diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c > > index d75a6c35b6c7..39469f67b241 100644 > > --- a/arch/riscv/kvm/vcpu_exit.c > > +++ b/arch/riscv/kvm/vcpu_exit.c > > @@ -12,6 +12,13 @@ > > #include <linux/kvm_host.h> > > #include <asm/csr.h> > > > > +#define INSN_OPCODE_MASK 0x007c > > +#define INSN_OPCODE_SHIFT 2 > > +#define INSN_OPCODE_SYSTEM 28 > > + > > +#define INSN_MASK_WFI 0xffffff00 > > +#define INSN_MATCH_WFI 0x10500000 > > + > > #define INSN_MATCH_LB 0x3 > > #define INSN_MASK_LB 0x707f > > #define INSN_MATCH_LH 0x1003 > > @@ -112,6 +119,67 @@ > > (s32)(((insn) >> 7) & 0x1f)) > > #define MASK_FUNCT3 0x7000 > > > > +static int truly_illegal_insn(struct kvm_vcpu *vcpu, > > + struct kvm_run *run, > > + ulong insn) > > +{ > > + /* Redirect trap to Guest VCPU */ > > + kvm_riscv_vcpu_trap_redirect(vcpu, EXC_INST_ILLEGAL, insn); > > + > > + return 1; > > +} > > + > > +static int system_opcode_insn(struct kvm_vcpu *vcpu, > > + struct kvm_run *run, > > + ulong insn) > > +{ > > + if ((insn & INSN_MASK_WFI) == INSN_MATCH_WFI) { > > + vcpu->stat.wfi_exit_stat++; > > + if (!kvm_arch_vcpu_runnable(vcpu)) { > > + srcu_read_unlock(&vcpu->kvm->srcu, vcpu->arch.srcu_idx); > > + kvm_vcpu_block(vcpu); > > + vcpu->arch.srcu_idx = srcu_read_lock(&vcpu->kvm->srcu); > > + kvm_clear_request(KVM_REQ_UNHALT, vcpu); > > + } > > + vcpu->arch.guest_context.sepc += INSN_LEN(insn); > > + return 1; > > + } > > + > > + return truly_illegal_insn(vcpu, run, insn); > > +} > > + > > +static int illegal_inst_fault(struct kvm_vcpu *vcpu, struct kvm_run *run, > > + unsigned long insn) > > +{ > > + unsigned long ut_scause = 0; > > + struct kvm_cpu_context *ct; > > + > > + if (unlikely((insn & 3) != 3)) { > > What do the low 2 bits mean here? Maybe you can use a define instead? These bits are for instruction length (16bit or 32bit). I will add appropriate defines for these bits. Regards, Anup > > > Alex > > > > > Amazon Development Center Germany GmbH > Krausenstr. 38 > 10117 Berlin > Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich > Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B > Sitz: Berlin > Ust-ID: DE 289 237 879 > >
diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c index d75a6c35b6c7..39469f67b241 100644 --- a/arch/riscv/kvm/vcpu_exit.c +++ b/arch/riscv/kvm/vcpu_exit.c @@ -12,6 +12,13 @@ #include <linux/kvm_host.h> #include <asm/csr.h> +#define INSN_OPCODE_MASK 0x007c +#define INSN_OPCODE_SHIFT 2 +#define INSN_OPCODE_SYSTEM 28 + +#define INSN_MASK_WFI 0xffffff00 +#define INSN_MATCH_WFI 0x10500000 + #define INSN_MATCH_LB 0x3 #define INSN_MASK_LB 0x707f #define INSN_MATCH_LH 0x1003 @@ -112,6 +119,67 @@ (s32)(((insn) >> 7) & 0x1f)) #define MASK_FUNCT3 0x7000 +static int truly_illegal_insn(struct kvm_vcpu *vcpu, + struct kvm_run *run, + ulong insn) +{ + /* Redirect trap to Guest VCPU */ + kvm_riscv_vcpu_trap_redirect(vcpu, EXC_INST_ILLEGAL, insn); + + return 1; +} + +static int system_opcode_insn(struct kvm_vcpu *vcpu, + struct kvm_run *run, + ulong insn) +{ + if ((insn & INSN_MASK_WFI) == INSN_MATCH_WFI) { + vcpu->stat.wfi_exit_stat++; + if (!kvm_arch_vcpu_runnable(vcpu)) { + srcu_read_unlock(&vcpu->kvm->srcu, vcpu->arch.srcu_idx); + kvm_vcpu_block(vcpu); + vcpu->arch.srcu_idx = srcu_read_lock(&vcpu->kvm->srcu); + kvm_clear_request(KVM_REQ_UNHALT, vcpu); + } + vcpu->arch.guest_context.sepc += INSN_LEN(insn); + return 1; + } + + return truly_illegal_insn(vcpu, run, insn); +} + +static int illegal_inst_fault(struct kvm_vcpu *vcpu, struct kvm_run *run, + unsigned long insn) +{ + unsigned long ut_scause = 0; + struct kvm_cpu_context *ct; + + if (unlikely((insn & 3) != 3)) { + if (insn == 0) { + ct = &vcpu->arch.guest_context; + insn = kvm_riscv_vcpu_unpriv_read(vcpu, true, + ct->sepc, + &ut_scause); + if (ut_scause) { + if (ut_scause == EXC_LOAD_PAGE_FAULT) + ut_scause = EXC_INST_PAGE_FAULT; + kvm_riscv_vcpu_trap_redirect(vcpu, ut_scause, + ct->sepc); + return 1; + } + } + if ((insn & 3) != 3) + return truly_illegal_insn(vcpu, run, insn); + } + + switch ((insn & INSN_OPCODE_MASK) >> INSN_OPCODE_SHIFT) { + case INSN_OPCODE_SYSTEM: + return system_opcode_insn(vcpu, run, insn); + default: + return truly_illegal_insn(vcpu, run, insn); + } +} + static int emulate_load(struct kvm_vcpu *vcpu, struct kvm_run *run, unsigned long fault_addr) { @@ -515,6 +583,10 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run, ret = -EFAULT; run->exit_reason = KVM_EXIT_UNKNOWN; switch (scause) { + case EXC_INST_ILLEGAL: + if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV) + ret = illegal_inst_fault(vcpu, run, stval); + break; case EXC_INST_PAGE_FAULT: case EXC_LOAD_PAGE_FAULT: case EXC_STORE_PAGE_FAULT: