Message ID | 20230526062509.31682-5-yongxuan.wang@sifive.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Add RISC-V KVM AIA Support | expand |
On 5/26/23 03:25, Yong-Xuan Wang wrote: > implement a function to create an KVM AIA chip > > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com> > Reviewed-by: Jim Shu <jim.shu@sifive.com> > --- > target/riscv/kvm.c | 83 ++++++++++++++++++++++++++++++++++++++++ > target/riscv/kvm_riscv.h | 3 ++ > 2 files changed, 86 insertions(+) > > diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c > index eb469e8ca5..ead121154f 100644 > --- a/target/riscv/kvm.c > +++ b/target/riscv/kvm.c > @@ -34,6 +34,7 @@ > #include "exec/address-spaces.h" > #include "hw/boards.h" > #include "hw/irq.h" > +#include "hw/intc/riscv_imsic.h" > #include "qemu/log.h" > #include "hw/loader.h" > #include "kvm_riscv.h" > @@ -548,3 +549,85 @@ bool kvm_arch_cpu_check_are_resettable(void) > void kvm_arch_accel_class_init(ObjectClass *oc) > { > } > + > +void kvm_riscv_aia_create(DeviceState *aplic_s, bool msimode, int socket, > + uint64_t aia_irq_num, uint64_t hart_count, > + uint64_t aplic_base, uint64_t imsic_base) > +{ > + int ret; > + int aia_fd = -1; > + uint64_t aia_mode; > + uint64_t aia_nr_ids; > + uint64_t aia_hart_bits = find_last_bit(&hart_count, BITS_PER_LONG) + 1; > + > + if (!msimode) { > + error_report("Currently KVM AIA only supports aplic_imsic mode"); > + exit(1); > + } > + > + aia_fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_RISCV_AIA, false); > + > + if (aia_fd < 0) { > + error_report("Unable to create in-kernel irqchip"); > + exit(1); > + } > + > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG, > + KVM_DEV_RISCV_AIA_CONFIG_MODE, > + &aia_mode, false, NULL); > + > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG, > + KVM_DEV_RISCV_AIA_CONFIG_IDS, > + &aia_nr_ids, false, NULL); > + > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG, > + KVM_DEV_RISCV_AIA_CONFIG_SRCS, > + &aia_irq_num, true, NULL); > + if (ret < 0) { > + error_report("KVM AIA: fail to set number input irq lines"); > + exit(1); > + } I see that you didn't check 'ret' for the first 2 calls of kvm_device_access(). Is it intentional? Since you're setting customized error messages for every step I think it's worth also handling the case where we fail to set aia_mode and aia_nr_ids. Thanks, Daniel > + > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG, > + KVM_DEV_RISCV_AIA_CONFIG_HART_BITS, > + &aia_hart_bits, true, NULL); > + if (ret < 0) { > + error_report("KVM AIA: fail to set number of harts"); > + exit(1); > + } > + > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_ADDR, > + KVM_DEV_RISCV_AIA_ADDR_APLIC, > + &aplic_base, true, NULL); > + if (ret < 0) { > + error_report("KVM AIA: fail to set the base address of APLIC"); > + exit(1); > + } > + > + for (int i = 0; i < hart_count; i++) { > + uint64_t imsic_addr = imsic_base + i * IMSIC_HART_SIZE(0); > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_ADDR, > + KVM_DEV_RISCV_AIA_ADDR_IMSIC(i), > + &imsic_addr, true, NULL); > + if (ret < 0) { > + error_report("KVM AIA: fail to set the base address of IMSICs"); > + exit(1); > + } > + } > + > + if (kvm_has_gsi_routing()) { > + for (uint64_t idx = 0; idx < aia_irq_num + 1; ++idx) { > + kvm_irqchip_add_irq_route(kvm_state, idx, socket, idx); > + } > + kvm_gsi_routing_allowed = true; > + kvm_irqchip_commit_routes(kvm_state); > + } > + > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CTRL, > + KVM_DEV_RISCV_AIA_CTRL_INIT, > + NULL, true, NULL); > + if (ret < 0) { > + error_report("KVM AIA: initialized fail"); > + exit(1); > + } > +} > diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h > index 606968a4b7..6067adff51 100644 > --- a/target/riscv/kvm_riscv.h > +++ b/target/riscv/kvm_riscv.h > @@ -21,6 +21,9 @@ > > void kvm_riscv_reset_vcpu(RISCVCPU *cpu); > void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level); > +void kvm_riscv_aia_create(DeviceState *aplic_s, bool msimode, int socket, > + uint64_t aia_irq_num, uint64_t hart_count, > + uint64_t aplic_base, uint64_t imsic_base); > > #define KVM_DEV_RISCV_AIA_GRP_CONFIG 0 > #define KVM_DEV_RISCV_AIA_CONFIG_MODE 0
Hi Daniel, Thanks for your suggestions! I'll fix it in patch v4. Regards, Yong-Xuan On Tue, Jun 6, 2023 at 9:45 PM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > > > On 5/26/23 03:25, Yong-Xuan Wang wrote: > > implement a function to create an KVM AIA chip > > > > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com> > > Reviewed-by: Jim Shu <jim.shu@sifive.com> > > --- > > target/riscv/kvm.c | 83 ++++++++++++++++++++++++++++++++++++++++ > > target/riscv/kvm_riscv.h | 3 ++ > > 2 files changed, 86 insertions(+) > > > > diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c > > index eb469e8ca5..ead121154f 100644 > > --- a/target/riscv/kvm.c > > +++ b/target/riscv/kvm.c > > @@ -34,6 +34,7 @@ > > #include "exec/address-spaces.h" > > #include "hw/boards.h" > > #include "hw/irq.h" > > +#include "hw/intc/riscv_imsic.h" > > #include "qemu/log.h" > > #include "hw/loader.h" > > #include "kvm_riscv.h" > > @@ -548,3 +549,85 @@ bool kvm_arch_cpu_check_are_resettable(void) > > void kvm_arch_accel_class_init(ObjectClass *oc) > > { > > } > > + > > +void kvm_riscv_aia_create(DeviceState *aplic_s, bool msimode, int socket, > > + uint64_t aia_irq_num, uint64_t hart_count, > > + uint64_t aplic_base, uint64_t imsic_base) > > +{ > > + int ret; > > + int aia_fd = -1; > > + uint64_t aia_mode; > > + uint64_t aia_nr_ids; > > + uint64_t aia_hart_bits = find_last_bit(&hart_count, BITS_PER_LONG) + 1; > > + > > + if (!msimode) { > > + error_report("Currently KVM AIA only supports aplic_imsic mode"); > > + exit(1); > > + } > > + > > + aia_fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_RISCV_AIA, false); > > + > > + if (aia_fd < 0) { > > + error_report("Unable to create in-kernel irqchip"); > > + exit(1); > > + } > > + > > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG, > > + KVM_DEV_RISCV_AIA_CONFIG_MODE, > > + &aia_mode, false, NULL); > > + > > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG, > > + KVM_DEV_RISCV_AIA_CONFIG_IDS, > > + &aia_nr_ids, false, NULL); > > + > > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG, > > + KVM_DEV_RISCV_AIA_CONFIG_SRCS, > > + &aia_irq_num, true, NULL); > > + if (ret < 0) { > > + error_report("KVM AIA: fail to set number input irq lines"); > > + exit(1); > > + } > > I see that you didn't check 'ret' for the first 2 calls of kvm_device_access(). > Is it intentional? > > Since you're setting customized error messages for every step I think it's worth > also handling the case where we fail to set aia_mode and aia_nr_ids. > > > Thanks, > > > Daniel > > > > + > > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG, > > + KVM_DEV_RISCV_AIA_CONFIG_HART_BITS, > > + &aia_hart_bits, true, NULL); > > + if (ret < 0) { > > + error_report("KVM AIA: fail to set number of harts"); > > + exit(1); > > + } > > + > > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_ADDR, > > + KVM_DEV_RISCV_AIA_ADDR_APLIC, > > + &aplic_base, true, NULL); > > + if (ret < 0) { > > + error_report("KVM AIA: fail to set the base address of APLIC"); > > + exit(1); > > + } > > + > > + for (int i = 0; i < hart_count; i++) { > > + uint64_t imsic_addr = imsic_base + i * IMSIC_HART_SIZE(0); > > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_ADDR, > > + KVM_DEV_RISCV_AIA_ADDR_IMSIC(i), > > + &imsic_addr, true, NULL); > > + if (ret < 0) { > > + error_report("KVM AIA: fail to set the base address of IMSICs"); > > + exit(1); > > + } > > + } > > + > > + if (kvm_has_gsi_routing()) { > > + for (uint64_t idx = 0; idx < aia_irq_num + 1; ++idx) { > > + kvm_irqchip_add_irq_route(kvm_state, idx, socket, idx); > > + } > > + kvm_gsi_routing_allowed = true; > > + kvm_irqchip_commit_routes(kvm_state); > > + } > > + > > + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CTRL, > > + KVM_DEV_RISCV_AIA_CTRL_INIT, > > + NULL, true, NULL); > > + if (ret < 0) { > > + error_report("KVM AIA: initialized fail"); > > + exit(1); > > + } > > +} > > diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h > > index 606968a4b7..6067adff51 100644 > > --- a/target/riscv/kvm_riscv.h > > +++ b/target/riscv/kvm_riscv.h > > @@ -21,6 +21,9 @@ > > > > void kvm_riscv_reset_vcpu(RISCVCPU *cpu); > > void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level); > > +void kvm_riscv_aia_create(DeviceState *aplic_s, bool msimode, int socket, > > + uint64_t aia_irq_num, uint64_t hart_count, > > + uint64_t aplic_base, uint64_t imsic_base); > > > > #define KVM_DEV_RISCV_AIA_GRP_CONFIG 0 > > #define KVM_DEV_RISCV_AIA_CONFIG_MODE 0
diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c index eb469e8ca5..ead121154f 100644 --- a/target/riscv/kvm.c +++ b/target/riscv/kvm.c @@ -34,6 +34,7 @@ #include "exec/address-spaces.h" #include "hw/boards.h" #include "hw/irq.h" +#include "hw/intc/riscv_imsic.h" #include "qemu/log.h" #include "hw/loader.h" #include "kvm_riscv.h" @@ -548,3 +549,85 @@ bool kvm_arch_cpu_check_are_resettable(void) void kvm_arch_accel_class_init(ObjectClass *oc) { } + +void kvm_riscv_aia_create(DeviceState *aplic_s, bool msimode, int socket, + uint64_t aia_irq_num, uint64_t hart_count, + uint64_t aplic_base, uint64_t imsic_base) +{ + int ret; + int aia_fd = -1; + uint64_t aia_mode; + uint64_t aia_nr_ids; + uint64_t aia_hart_bits = find_last_bit(&hart_count, BITS_PER_LONG) + 1; + + if (!msimode) { + error_report("Currently KVM AIA only supports aplic_imsic mode"); + exit(1); + } + + aia_fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_RISCV_AIA, false); + + if (aia_fd < 0) { + error_report("Unable to create in-kernel irqchip"); + exit(1); + } + + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG, + KVM_DEV_RISCV_AIA_CONFIG_MODE, + &aia_mode, false, NULL); + + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG, + KVM_DEV_RISCV_AIA_CONFIG_IDS, + &aia_nr_ids, false, NULL); + + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG, + KVM_DEV_RISCV_AIA_CONFIG_SRCS, + &aia_irq_num, true, NULL); + if (ret < 0) { + error_report("KVM AIA: fail to set number input irq lines"); + exit(1); + } + + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG, + KVM_DEV_RISCV_AIA_CONFIG_HART_BITS, + &aia_hart_bits, true, NULL); + if (ret < 0) { + error_report("KVM AIA: fail to set number of harts"); + exit(1); + } + + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_ADDR, + KVM_DEV_RISCV_AIA_ADDR_APLIC, + &aplic_base, true, NULL); + if (ret < 0) { + error_report("KVM AIA: fail to set the base address of APLIC"); + exit(1); + } + + for (int i = 0; i < hart_count; i++) { + uint64_t imsic_addr = imsic_base + i * IMSIC_HART_SIZE(0); + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_ADDR, + KVM_DEV_RISCV_AIA_ADDR_IMSIC(i), + &imsic_addr, true, NULL); + if (ret < 0) { + error_report("KVM AIA: fail to set the base address of IMSICs"); + exit(1); + } + } + + if (kvm_has_gsi_routing()) { + for (uint64_t idx = 0; idx < aia_irq_num + 1; ++idx) { + kvm_irqchip_add_irq_route(kvm_state, idx, socket, idx); + } + kvm_gsi_routing_allowed = true; + kvm_irqchip_commit_routes(kvm_state); + } + + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CTRL, + KVM_DEV_RISCV_AIA_CTRL_INIT, + NULL, true, NULL); + if (ret < 0) { + error_report("KVM AIA: initialized fail"); + exit(1); + } +} diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h index 606968a4b7..6067adff51 100644 --- a/target/riscv/kvm_riscv.h +++ b/target/riscv/kvm_riscv.h @@ -21,6 +21,9 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu); void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level); +void kvm_riscv_aia_create(DeviceState *aplic_s, bool msimode, int socket, + uint64_t aia_irq_num, uint64_t hart_count, + uint64_t aplic_base, uint64_t imsic_base); #define KVM_DEV_RISCV_AIA_GRP_CONFIG 0 #define KVM_DEV_RISCV_AIA_CONFIG_MODE 0