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@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxx>
> > Reviewed-by: Jim Shu <jim.shu@xxxxxxxxxx>
> > ---
> > 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
