[PATCH v3 4/6] target/riscv: Create an KVM AIA irqchip

Yong-Xuan Wang posted 6 patches 1 year, 4 months ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liweiwei@iscas.ac.cn>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, "Michael S. Tsirkin" <mst@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[PATCH v3 4/6] target/riscv: Create an KVM AIA irqchip
Posted by Yong-Xuan Wang 1 year, 4 months ago
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);
+    }
+
+    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
-- 
2.17.1
Re: [PATCH v3 4/6] target/riscv: Create an KVM AIA irqchip
Posted by Daniel Henrique Barboza 1 year, 4 months ago

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
Re: [PATCH v3 4/6] target/riscv: Create an KVM AIA irqchip
Posted by Yong-Xuan Wang 1 year, 4 months ago
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