> -----Original Message----- > From: Qemu-riscv > [mailto:qemu-riscv-bounces+jiangyifei=huawei.com@xxxxxxxxxx] On Behalf Of > Anup Patel > Sent: Friday, April 30, 2021 12:54 PM > To: Jiangyifei <jiangyifei@xxxxxxxxxx> > Cc: Bin Meng <bin.meng@xxxxxxxxxxxxx>; open list:RISC-V > <qemu-riscv@xxxxxxxxxx>; Sagar Karandikar <sagark@xxxxxxxxxxxxxxxxx>; > KVM General <kvm@xxxxxxxxxxxxxxx>; libvir-list@xxxxxxxxxx; Bastian > Koppelmann <kbastian@xxxxxxxxxxxxxxxxxxxxx>; Anup Patel > <anup.patel@xxxxxxx>; QEMU Developers <qemu-devel@xxxxxxxxxx>; > yinyipeng <yinyipeng1@xxxxxxxxxx>; Palmer Dabbelt <palmer@xxxxxxxxxxx>; > kvm-riscv@xxxxxxxxxxxxxxxxxxx; Alistair Francis <Alistair.Francis@xxxxxxx>; > Fanliang (EulerOS) <fanliang@xxxxxxxxxx>; Wubin (H) > <wu.wubin@xxxxxxxxxx>; Zhanghailiang <zhang.zhanghailiang@xxxxxxxxxx> > Subject: Re: [PATCH RFC v5 07/12] hw/riscv: PLIC update external interrupt by > KVM when kvm enabled > > On Mon, Apr 12, 2021 at 12:24 PM Yifei Jiang <jiangyifei@xxxxxxxxxx> wrote: > > > > Only support supervisor external interrupt currently. > > > > Signed-off-by: Yifei Jiang <jiangyifei@xxxxxxxxxx> > > Signed-off-by: Yipeng Yin <yinyipeng1@xxxxxxxxxx> > > --- > > hw/intc/sifive_plic.c | 29 ++++++++++++++++++++--------- > > target/riscv/kvm-stub.c | 5 +++++ > > target/riscv/kvm.c | 20 ++++++++++++++++++++ > > target/riscv/kvm_riscv.h | 1 + > > 4 files changed, 46 insertions(+), 9 deletions(-) > > > > diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c index > > 97a1a27a9a..2746eb7a05 100644 > > --- a/hw/intc/sifive_plic.c > > +++ b/hw/intc/sifive_plic.c > > @@ -31,6 +31,8 @@ > > #include "target/riscv/cpu.h" > > #include "sysemu/sysemu.h" > > #include "migration/vmstate.h" > > +#include "sysemu/kvm.h" > > +#include "kvm_riscv.h" > > > > #define RISCV_DEBUG_PLIC 0 > > > > @@ -147,15 +149,24 @@ static void sifive_plic_update(SiFivePLICState *plic) > > continue; > > } > > int level = sifive_plic_irqs_pending(plic, addrid); > > - switch (mode) { > > - case PLICMode_M: > > - riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MEIP, > BOOL_TO_MASK(level)); > > - break; > > - case PLICMode_S: > > - riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_SEIP, > BOOL_TO_MASK(level)); > > - break; > > - default: > > - break; > > + if (kvm_enabled()) { > > + if (mode == PLICMode_M) { > > + continue; > > + } > > + kvm_riscv_set_irq(RISCV_CPU(cpu), IRQ_S_EXT, level); > > + } else { > > + switch (mode) { > > + case PLICMode_M: > > + riscv_cpu_update_mip(RISCV_CPU(cpu), > > + MIP_MEIP, > BOOL_TO_MASK(level)); > > + break; > > + case PLICMode_S: > > + riscv_cpu_update_mip(RISCV_CPU(cpu), > > + MIP_SEIP, > BOOL_TO_MASK(level)); > > + break; > > + default: > > + break; > > + } > > I am not comfortable with this patch. > > This way we will endup calling kvm_riscv_set_irq() from various places in > hw/intc and hw/riscv. > > I suggest to extend riscv_cpu_update_mip() such that when kvm is enabled > riscv_cpu_update_mip() will: > 1) Consider only MIP_SEIP bit in "mask" parameter and all other > bits in "mask" parameter will be ignored probably with warning > 2) When the MIP_SEIP bit is set in "mask" call kvm_riscv_set_irq() to change > the IRQ state in the KVM module. > > Regards, > Anup > Yes, but riscv_cpu_update_mip() in target/riscv/cpu_helper.c is used to TCG. So it is not appropriate to adapt for KVM. We will move riscv_cpu_update_mip() to target/riscv/cpu.c and modify it according your advice. Regards, Yifei > > } > > } > > > > diff --git a/target/riscv/kvm-stub.c b/target/riscv/kvm-stub.c index > > 39b96fe3f4..4e8fc31a21 100644 > > --- a/target/riscv/kvm-stub.c > > +++ b/target/riscv/kvm-stub.c > > @@ -23,3 +23,8 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu) { > > abort(); > > } > > + > > +void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level) { > > + abort(); > > +} > > diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c index > > 79c931acb4..da63535812 100644 > > --- a/target/riscv/kvm.c > > +++ b/target/riscv/kvm.c > > @@ -453,6 +453,26 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu) > > env->gpr[11] = cpu->env.fdt_addr; /* a1 */ > > } > > > > +void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level) { > > + int ret; > > + unsigned virq = level ? KVM_INTERRUPT_SET : > KVM_INTERRUPT_UNSET; > > + > > + if (irq != IRQ_S_EXT) { > > + return; > > + } > > + > > + if (!kvm_enabled()) { > > + return; > > + } > > + > > + ret = kvm_vcpu_ioctl(CPU(cpu), KVM_INTERRUPT, &virq); > > + if (ret < 0) { > > + perror("Set irq failed"); > > + abort(); > > + } > > +} > > + > > bool kvm_arch_cpu_check_are_resettable(void) > > { > > return true; > > diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h index > > f38c82bf59..ed281bdce0 100644 > > --- a/target/riscv/kvm_riscv.h > > +++ b/target/riscv/kvm_riscv.h > > @@ -20,5 +20,6 @@ > > #define QEMU_KVM_RISCV_H > > > > void kvm_riscv_reset_vcpu(RISCVCPU *cpu); > > +void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level); > > > > #endif > > -- > > 2.19.1 > > > > > > -- > > kvm-riscv mailing list > > kvm-riscv@xxxxxxxxxxxxxxxxxxx > > http://lists.infradead.org/mailman/listinfo/kvm-riscv