* [PATCH 0/4] riscv: AIA: refinement for KVM acceleration
@ 2025-02-17 8:19 Yong-Xuan Wang
2025-02-17 8:19 ` [PATCH 1/4] hw/riscv/virt: KVM AIA refinement Yong-Xuan Wang
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Yong-Xuan Wang @ 2025-02-17 8:19 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: greentime.hu, vincent.chen, frank.chang, jim.shu, Yong-Xuan Wang
Reorder the code to reduce the conditional checking and remove
unnecessary resource setting when using in-kernl AIA irqchip.
Yong-Xuan Wang (4):
hw/riscv/virt: KVM AIA refinement
hw/intc/imsic: refine the IMSIC realize
hw/intc/aplic: refine the APLIC realize
hw/intc/aplic: refine kvm_msicfgaddr
hw/intc/riscv_aplic.c | 73 ++++++++++++++++++++-------------------
hw/intc/riscv_imsic.c | 47 +++++++++++++------------
hw/riscv/virt.c | 79 ++++++++++++++++++++-----------------------
3 files changed, 102 insertions(+), 97 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/4] hw/riscv/virt: KVM AIA refinement
2025-02-17 8:19 [PATCH 0/4] riscv: AIA: refinement for KVM acceleration Yong-Xuan Wang
@ 2025-02-17 8:19 ` Yong-Xuan Wang
2025-02-17 19:24 ` Daniel Henrique Barboza
2025-02-17 8:19 ` [PATCH 2/4] hw/intc/imsic: refine the IMSIC realize Yong-Xuan Wang
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Yong-Xuan Wang @ 2025-02-17 8:19 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: greentime.hu, vincent.chen, frank.chang, jim.shu, Yong-Xuan Wang,
Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei
KVM AIA is only needed to be set when the virt machine use the AIA MSI.
So we can move the KVM AIA configuration into virt_create_aia() to reduce
the condition checking.
Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
hw/riscv/virt.c | 79 +++++++++++++++++++++++--------------------------
1 file changed, 37 insertions(+), 42 deletions(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index dae46f4733cd..a52117ef71ee 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -58,14 +58,6 @@
#include "qapi/qapi-visit-common.h"
#include "hw/virtio/virtio-iommu.h"
-/* KVM AIA only supports APLIC MSI. APLIC Wired is always emulated by QEMU. */
-static bool virt_use_kvm_aia_aplic_imsic(RISCVVirtAIAType aia_type)
-{
- bool msimode = aia_type == VIRT_AIA_TYPE_APLIC_IMSIC;
-
- return riscv_is_kvm_aia_aplic_imsic(msimode);
-}
-
static bool virt_use_emulated_aplic(RISCVVirtAIAType aia_type)
{
bool msimode = aia_type == VIRT_AIA_TYPE_APLIC_IMSIC;
@@ -1298,10 +1290,12 @@ static DeviceState *virt_create_plic(const MemMapEntry *memmap, int socket,
return ret;
}
-static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
+static DeviceState *virt_create_aia(RISCVVirtState *s,
const MemMapEntry *memmap, int socket,
int base_hartid, int hart_count)
{
+ RISCVVirtAIAType aia_type = s->aia_type;
+ int aia_guests = s->aia_guests;
int i;
hwaddr addr = 0;
uint32_t guest_bits;
@@ -1309,6 +1303,28 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
DeviceState *aplic_m = NULL;
bool msimode = aia_type == VIRT_AIA_TYPE_APLIC_IMSIC;
+ if (!kvm_enabled()) {
+ /* Per-socket M-level APLIC */
+ aplic_m = riscv_aplic_create(memmap[VIRT_APLIC_M].base +
+ socket * memmap[VIRT_APLIC_M].size,
+ memmap[VIRT_APLIC_M].size,
+ (msimode) ? 0 : base_hartid,
+ (msimode) ? 0 : hart_count,
+ VIRT_IRQCHIP_NUM_SOURCES,
+ VIRT_IRQCHIP_NUM_PRIO_BITS,
+ msimode, true, NULL);
+ }
+
+ /* Per-socket S-level APLIC */
+ aplic_s = riscv_aplic_create(memmap[VIRT_APLIC_S].base +
+ socket * memmap[VIRT_APLIC_S].size,
+ memmap[VIRT_APLIC_S].size,
+ (msimode) ? 0 : base_hartid,
+ (msimode) ? 0 : hart_count,
+ VIRT_IRQCHIP_NUM_SOURCES,
+ VIRT_IRQCHIP_NUM_PRIO_BITS,
+ msimode, false, aplic_m);
+
if (msimode) {
if (!kvm_enabled()) {
/* Per-socket M-level IMSICs */
@@ -1329,32 +1345,20 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
base_hartid + i, false, 1 + aia_guests,
VIRT_IRQCHIP_NUM_MSIS);
}
- }
- if (!kvm_enabled()) {
- /* Per-socket M-level APLIC */
- aplic_m = riscv_aplic_create(memmap[VIRT_APLIC_M].base +
- socket * memmap[VIRT_APLIC_M].size,
- memmap[VIRT_APLIC_M].size,
- (msimode) ? 0 : base_hartid,
- (msimode) ? 0 : hart_count,
- VIRT_IRQCHIP_NUM_SOURCES,
- VIRT_IRQCHIP_NUM_PRIO_BITS,
- msimode, true, NULL);
- }
- /* Per-socket S-level APLIC */
- aplic_s = riscv_aplic_create(memmap[VIRT_APLIC_S].base +
- socket * memmap[VIRT_APLIC_S].size,
- memmap[VIRT_APLIC_S].size,
- (msimode) ? 0 : base_hartid,
- (msimode) ? 0 : hart_count,
+ if (kvm_irqchip_in_kernel()) {
+ kvm_riscv_aia_create(MACHINE(s), IMSIC_MMIO_GROUP_MIN_SHIFT,
VIRT_IRQCHIP_NUM_SOURCES,
- VIRT_IRQCHIP_NUM_PRIO_BITS,
- msimode, false, aplic_m);
+ VIRT_IRQCHIP_NUM_MSIS,
+ memmap[VIRT_APLIC_S].base,
+ memmap[VIRT_IMSIC_S].base,
+ aia_guests);
+ }
- if (kvm_enabled() && msimode) {
- riscv_aplic_set_kvm_msicfgaddr(RISCV_APLIC(aplic_s), addr);
+ if (kvm_enabled()) {
+ riscv_aplic_set_kvm_msicfgaddr(RISCV_APLIC(aplic_s), addr);
+ }
}
return kvm_enabled() ? aplic_s : aplic_m;
@@ -1621,9 +1625,8 @@ static void virt_machine_init(MachineState *machine)
s->irqchip[i] = virt_create_plic(memmap, i,
base_hartid, hart_count);
} else {
- s->irqchip[i] = virt_create_aia(s->aia_type, s->aia_guests,
- memmap, i, base_hartid,
- hart_count);
+ s->irqchip[i] = virt_create_aia(s, memmap, i,
+ base_hartid, hart_count);
}
/* Try to use different IRQCHIP instance based device type */
@@ -1641,14 +1644,6 @@ static void virt_machine_init(MachineState *machine)
}
}
- if (kvm_enabled() && virt_use_kvm_aia_aplic_imsic(s->aia_type)) {
- kvm_riscv_aia_create(machine, IMSIC_MMIO_GROUP_MIN_SHIFT,
- VIRT_IRQCHIP_NUM_SOURCES, VIRT_IRQCHIP_NUM_MSIS,
- memmap[VIRT_APLIC_S].base,
- memmap[VIRT_IMSIC_S].base,
- s->aia_guests);
- }
-
if (riscv_is_32bit(&s->soc[0])) {
#if HOST_LONG_BITS == 64
/* limit RAM size in a 32-bit system */
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] hw/intc/imsic: refine the IMSIC realize
2025-02-17 8:19 [PATCH 0/4] riscv: AIA: refinement for KVM acceleration Yong-Xuan Wang
2025-02-17 8:19 ` [PATCH 1/4] hw/riscv/virt: KVM AIA refinement Yong-Xuan Wang
@ 2025-02-17 8:19 ` Yong-Xuan Wang
2025-02-17 8:19 ` [PATCH 3/4] hw/intc/aplic: refine the APLIC realize Yong-Xuan Wang
2025-02-17 8:19 ` [PATCH 4/4] hw/intc/aplic: refine kvm_msicfgaddr Yong-Xuan Wang
3 siblings, 0 replies; 7+ messages in thread
From: Yong-Xuan Wang @ 2025-02-17 8:19 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: greentime.hu, vincent.chen, frank.chang, jim.shu, Yong-Xuan Wang,
Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei
When the IMSIC is emulated in the kernel, the GPIO output lines to CPUs
and aia_ireg_rmw_fn setting can be remove. In this case the IMSIC
trigger CPU interrupts by KVM APIs, and the RMW of IREG is handled in
kernel.
This patch also move the code that claim the CPU interrupts to the
beginning of IMSIC realization. This can avoid the unnecessary resource
allocation before checking failed.
Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
hw/intc/riscv_imsic.c | 47 ++++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/hw/intc/riscv_imsic.c b/hw/intc/riscv_imsic.c
index dc8162c0a7c9..241b12fef09f 100644
--- a/hw/intc/riscv_imsic.c
+++ b/hw/intc/riscv_imsic.c
@@ -349,7 +349,19 @@ static void riscv_imsic_realize(DeviceState *dev, Error **errp)
CPUState *cpu = cpu_by_arch_id(imsic->hartid);
CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
+ /* Claim the CPU interrupt to be triggered by this IMSIC */
+ if (riscv_cpu_claim_interrupts(rcpu,
+ (imsic->mmode) ? MIP_MEIP : MIP_SEIP) < 0) {
+ error_setg(errp, "%s already claimed",
+ (imsic->mmode) ? "MEIP" : "SEIP");
+ return;
+ }
+
if (!kvm_irqchip_in_kernel()) {
+ /* Create output IRQ lines */
+ imsic->external_irqs = g_malloc(sizeof(qemu_irq) * imsic->num_pages);
+ qdev_init_gpio_out(dev, imsic->external_irqs, imsic->num_pages);
+
imsic->num_eistate = imsic->num_pages * imsic->num_irqs;
imsic->eidelivery = g_new0(uint32_t, imsic->num_pages);
imsic->eithreshold = g_new0(uint32_t, imsic->num_pages);
@@ -361,18 +373,6 @@ static void riscv_imsic_realize(DeviceState *dev, Error **errp)
IMSIC_MMIO_SIZE(imsic->num_pages));
sysbus_init_mmio(SYS_BUS_DEVICE(dev), &imsic->mmio);
- /* Claim the CPU interrupt to be triggered by this IMSIC */
- if (riscv_cpu_claim_interrupts(rcpu,
- (imsic->mmode) ? MIP_MEIP : MIP_SEIP) < 0) {
- error_setg(errp, "%s already claimed",
- (imsic->mmode) ? "MEIP" : "SEIP");
- return;
- }
-
- /* Create output IRQ lines */
- imsic->external_irqs = g_malloc(sizeof(qemu_irq) * imsic->num_pages);
- qdev_init_gpio_out(dev, imsic->external_irqs, imsic->num_pages);
-
/* Force select AIA feature and setup CSR read-modify-write callback */
if (env) {
if (!imsic->mmode) {
@@ -381,8 +381,11 @@ static void riscv_imsic_realize(DeviceState *dev, Error **errp)
} else {
rcpu->cfg.ext_smaia = true;
}
- riscv_cpu_set_aia_ireg_rmw_fn(env, (imsic->mmode) ? PRV_M : PRV_S,
- riscv_imsic_rmw, imsic);
+
+ if (!kvm_irqchip_in_kernel()) {
+ riscv_cpu_set_aia_ireg_rmw_fn(env, (imsic->mmode) ? PRV_M : PRV_S,
+ riscv_imsic_rmw, imsic);
+ }
}
msi_nonbroken = true;
@@ -464,15 +467,17 @@ DeviceState *riscv_imsic_create(hwaddr addr, uint32_t hartid, bool mmode,
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
- for (i = 0; i < num_pages; i++) {
- if (!i) {
- qdev_connect_gpio_out_named(dev, NULL, i,
- qdev_get_gpio_in(DEVICE(cpu),
+ if (!kvm_irqchip_in_kernel()) {
+ for (i = 0; i < num_pages; i++) {
+ if (!i) {
+ qdev_connect_gpio_out_named(dev, NULL, i,
+ qdev_get_gpio_in(DEVICE(cpu),
(mmode) ? IRQ_M_EXT : IRQ_S_EXT));
- } else {
- qdev_connect_gpio_out_named(dev, NULL, i,
- qdev_get_gpio_in(DEVICE(cpu),
+ } else {
+ qdev_connect_gpio_out_named(dev, NULL, i,
+ qdev_get_gpio_in(DEVICE(cpu),
IRQ_LOCAL_MAX + i - 1));
+ }
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/4] hw/intc/aplic: refine the APLIC realize
2025-02-17 8:19 [PATCH 0/4] riscv: AIA: refinement for KVM acceleration Yong-Xuan Wang
2025-02-17 8:19 ` [PATCH 1/4] hw/riscv/virt: KVM AIA refinement Yong-Xuan Wang
2025-02-17 8:19 ` [PATCH 2/4] hw/intc/imsic: refine the IMSIC realize Yong-Xuan Wang
@ 2025-02-17 8:19 ` Yong-Xuan Wang
2025-02-17 8:19 ` [PATCH 4/4] hw/intc/aplic: refine kvm_msicfgaddr Yong-Xuan Wang
3 siblings, 0 replies; 7+ messages in thread
From: Yong-Xuan Wang @ 2025-02-17 8:19 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: greentime.hu, vincent.chen, frank.chang, jim.shu, Yong-Xuan Wang,
Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei
When the APLIC is emulated in the kernel, the GPIO output lines to CPUs
can be remove. In this case the APLIC trigger CPU interrupts by KVM APIs.
This patch also move the code that claim the CPU interrupts to the
beginning of APLIC realization. This can avoid the unnecessary resource
allocation before checking failed.
Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
hw/intc/riscv_aplic.c | 49 +++++++++++++++++++++++--------------------
1 file changed, 26 insertions(+), 23 deletions(-)
diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index 0974c6a5db39..e5714267c096 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -893,6 +893,26 @@ static void riscv_aplic_realize(DeviceState *dev, Error **errp)
RISCVAPLICState *aplic = RISCV_APLIC(dev);
if (riscv_use_emulated_aplic(aplic->msimode)) {
+ /* Create output IRQ lines for non-MSI mode */
+ if (!aplic->msimode) {
+ /* Claim the CPU interrupt to be triggered by this APLIC */
+ for (i = 0; i < aplic->num_harts; i++) {
+ RISCVCPU *cpu;
+
+ cpu = RISCV_CPU(cpu_by_arch_id(aplic->hartid_base + i));
+ if (riscv_cpu_claim_interrupts(cpu,
+ (aplic->mmode) ? MIP_MEIP : MIP_SEIP) < 0) {
+ error_report("%s already claimed",
+ (aplic->mmode) ? "MEIP" : "SEIP");
+ exit(1);
+ }
+ }
+
+ aplic->external_irqs = g_malloc(sizeof(qemu_irq) *
+ aplic->num_harts);
+ qdev_init_gpio_out(dev, aplic->external_irqs, aplic->num_harts);
+ }
+
aplic->bitfield_words = (aplic->num_irqs + 31) >> 5;
aplic->sourcecfg = g_new0(uint32_t, aplic->num_irqs);
aplic->state = g_new0(uint32_t, aplic->num_irqs);
@@ -927,23 +947,6 @@ static void riscv_aplic_realize(DeviceState *dev, Error **errp)
}
}
- /* Create output IRQ lines for non-MSI mode */
- if (!aplic->msimode) {
- aplic->external_irqs = g_malloc(sizeof(qemu_irq) * aplic->num_harts);
- qdev_init_gpio_out(dev, aplic->external_irqs, aplic->num_harts);
-
- /* Claim the CPU interrupt to be triggered by this APLIC */
- for (i = 0; i < aplic->num_harts; i++) {
- RISCVCPU *cpu = RISCV_CPU(cpu_by_arch_id(aplic->hartid_base + i));
- if (riscv_cpu_claim_interrupts(cpu,
- (aplic->mmode) ? MIP_MEIP : MIP_SEIP) < 0) {
- error_report("%s already claimed",
- (aplic->mmode) ? "MEIP" : "SEIP");
- exit(1);
- }
- }
- }
-
msi_nonbroken = true;
}
@@ -1067,15 +1070,15 @@ DeviceState *riscv_aplic_create(hwaddr addr, hwaddr size,
if (riscv_use_emulated_aplic(msimode)) {
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
- }
- if (!msimode) {
- for (i = 0; i < num_harts; i++) {
- CPUState *cpu = cpu_by_arch_id(hartid_base + i);
+ if (!msimode) {
+ for (i = 0; i < num_harts; i++) {
+ CPUState *cpu = cpu_by_arch_id(hartid_base + i);
- qdev_connect_gpio_out_named(dev, NULL, i,
- qdev_get_gpio_in(DEVICE(cpu),
+ qdev_connect_gpio_out_named(dev, NULL, i,
+ qdev_get_gpio_in(DEVICE(cpu),
(mmode) ? IRQ_M_EXT : IRQ_S_EXT));
+ }
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/4] hw/intc/aplic: refine kvm_msicfgaddr
2025-02-17 8:19 [PATCH 0/4] riscv: AIA: refinement for KVM acceleration Yong-Xuan Wang
` (2 preceding siblings ...)
2025-02-17 8:19 ` [PATCH 3/4] hw/intc/aplic: refine the APLIC realize Yong-Xuan Wang
@ 2025-02-17 8:19 ` Yong-Xuan Wang
3 siblings, 0 replies; 7+ messages in thread
From: Yong-Xuan Wang @ 2025-02-17 8:19 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: greentime.hu, vincent.chen, frank.chang, jim.shu, Yong-Xuan Wang,
Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei
Let kvm_msicfgaddr use the same format with mmsicfgaddr and smsicfgaddr.
Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
hw/intc/riscv_aplic.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index e5714267c096..5964cde7e09e 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -181,8 +181,10 @@ void riscv_aplic_set_kvm_msicfgaddr(RISCVAPLICState *aplic, hwaddr addr)
{
#ifdef CONFIG_KVM
if (riscv_use_emulated_aplic(aplic->msimode)) {
+ addr >>= APLIC_xMSICFGADDR_PPN_SHIFT;
aplic->kvm_msicfgaddr = extract64(addr, 0, 32);
- aplic->kvm_msicfgaddrH = extract64(addr, 32, 32);
+ aplic->kvm_msicfgaddrH = extract64(addr, 32, 32) &
+ APLIC_xMSICFGADDRH_VALID_MASK;
}
#endif
}
@@ -403,12 +405,17 @@ static void riscv_aplic_msi_send(RISCVAPLICState *aplic,
}
}
- if (aplic->mmode) {
- msicfgaddr = aplic_m->mmsicfgaddr;
- msicfgaddrH = aplic_m->mmsicfgaddrH;
+ if (aplic->kvm_splitmode) {
+ msicfgaddr = aplic->kvm_msicfgaddr;
+ msicfgaddrH = ((uint64_t)aplic->kvm_msicfgaddrH << 32);
} else {
- msicfgaddr = aplic_m->smsicfgaddr;
- msicfgaddrH = aplic_m->smsicfgaddrH;
+ if (aplic->mmode) {
+ msicfgaddr = aplic_m->mmsicfgaddr;
+ msicfgaddrH = aplic_m->mmsicfgaddrH;
+ } else {
+ msicfgaddr = aplic_m->smsicfgaddr;
+ msicfgaddrH = aplic_m->smsicfgaddrH;
+ }
}
lhxs = (msicfgaddrH >> APLIC_xMSICFGADDRH_LHXS_SHIFT) &
@@ -431,11 +438,6 @@ static void riscv_aplic_msi_send(RISCVAPLICState *aplic,
addr |= (uint64_t)(guest_idx & APLIC_xMSICFGADDR_PPN_HART(lhxs));
addr <<= APLIC_xMSICFGADDR_PPN_SHIFT;
- if (aplic->kvm_splitmode) {
- addr |= aplic->kvm_msicfgaddr;
- addr |= ((uint64_t)aplic->kvm_msicfgaddrH << 32);
- }
-
address_space_stl_le(&address_space_memory, addr,
eiid, MEMTXATTRS_UNSPECIFIED, &result);
if (result != MEMTX_OK) {
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4] hw/riscv/virt: KVM AIA refinement
2025-02-17 8:19 ` [PATCH 1/4] hw/riscv/virt: KVM AIA refinement Yong-Xuan Wang
@ 2025-02-17 19:24 ` Daniel Henrique Barboza
2025-02-19 11:25 ` Yong-Xuan Wang
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Henrique Barboza @ 2025-02-17 19:24 UTC (permalink / raw)
To: Yong-Xuan Wang, qemu-devel, qemu-riscv
Cc: greentime.hu, vincent.chen, frank.chang, jim.shu, Palmer Dabbelt,
Alistair Francis, Weiwei Li, Liu Zhiwei
On 2/17/25 5:19 AM, Yong-Xuan Wang wrote:
> KVM AIA is only needed to be set when the virt machine use the AIA MSI.
> So we can move the KVM AIA configuration into virt_create_aia() to reduce
> the condition checking.
>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
Unfortunately this doesn't work.
The reason is that kvm_riscv_aia_create(), as it is now, is called only once
during virt_machine_init() and it's already handling initialization for each socket:
for (socket = 0; socket < socket_count; socket++) {
socket_imsic_base = imsic_base + socket * (1U << group_shift);
hart_count = riscv_socket_hart_count(machine, socket);
base_hart = riscv_socket_first_hartid(machine, socket);
if (max_hart_per_socket < hart_count) {
max_hart_per_socket = hart_count;
}
for (i = 0; i < hart_count; i++) {
imsic_addr = socket_imsic_base + i * IMSIC_HART_SIZE(guest_bits);
ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_ADDR,
KVM_DEV_RISCV_AIA_ADDR_IMSIC(i + base_hart),
&imsic_addr, true, NULL);
if (ret < 0) {
error_report("KVM AIA: failed to set the IMSIC address for hart %d", i);
exit(1);
}
}
}
After this change, kvm_riscv_aia_create() is being called once for each socket since it's
now being called inside virt_create_aia(). And this will cause errors when running qemu-kvm
with more than one socket:
./qemu-system-riscv64 \
-machine virt,accel=kvm,aia=aplic-imsic -m 2G \
-object memory-backend-ram,size=1G,id=m0 \
-object memory-backend-ram,size=1G,id=m1 \
-smp 2,sockets=2,cores=1,threads=1 \
-numa node,memdev=m0,cpus=0,nodeid=0 \
-numa node,memdev=m1,cpus=1,nodeid=1 \
(...)
qemu-system-riscv64: KVM AIA: failed to set the IMSIC address for hart 0
To make this patch work we would need changes in kvm_riscv_aia_create() to handle just the
current socket. The loop I mentioned above is one place, and there's another place where
we set group_bits and group_shift if socket_count > 1.
To be honest I'm not sure if all these extra required changes are worth the simplification
this patch is proposing.
Thanks,
Daniel
> hw/riscv/virt.c | 79 +++++++++++++++++++++++--------------------------
> 1 file changed, 37 insertions(+), 42 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index dae46f4733cd..a52117ef71ee 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -58,14 +58,6 @@
> #include "qapi/qapi-visit-common.h"
> #include "hw/virtio/virtio-iommu.h"
>
> -/* KVM AIA only supports APLIC MSI. APLIC Wired is always emulated by QEMU. */
> -static bool virt_use_kvm_aia_aplic_imsic(RISCVVirtAIAType aia_type)
> -{
> - bool msimode = aia_type == VIRT_AIA_TYPE_APLIC_IMSIC;
> -
> - return riscv_is_kvm_aia_aplic_imsic(msimode);
> -}
> -
> static bool virt_use_emulated_aplic(RISCVVirtAIAType aia_type)
> {
> bool msimode = aia_type == VIRT_AIA_TYPE_APLIC_IMSIC;
> @@ -1298,10 +1290,12 @@ static DeviceState *virt_create_plic(const MemMapEntry *memmap, int socket,
> return ret;
> }
>
> -static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
> +static DeviceState *virt_create_aia(RISCVVirtState *s,
> const MemMapEntry *memmap, int socket,
> int base_hartid, int hart_count)
> {
> + RISCVVirtAIAType aia_type = s->aia_type;
> + int aia_guests = s->aia_guests;
> int i;
> hwaddr addr = 0;
> uint32_t guest_bits;
> @@ -1309,6 +1303,28 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
> DeviceState *aplic_m = NULL;
> bool msimode = aia_type == VIRT_AIA_TYPE_APLIC_IMSIC;
>
> + if (!kvm_enabled()) {
> + /* Per-socket M-level APLIC */
> + aplic_m = riscv_aplic_create(memmap[VIRT_APLIC_M].base +
> + socket * memmap[VIRT_APLIC_M].size,
> + memmap[VIRT_APLIC_M].size,
> + (msimode) ? 0 : base_hartid,
> + (msimode) ? 0 : hart_count,
> + VIRT_IRQCHIP_NUM_SOURCES,
> + VIRT_IRQCHIP_NUM_PRIO_BITS,
> + msimode, true, NULL);
> + }
> +
> + /* Per-socket S-level APLIC */
> + aplic_s = riscv_aplic_create(memmap[VIRT_APLIC_S].base +
> + socket * memmap[VIRT_APLIC_S].size,
> + memmap[VIRT_APLIC_S].size,
> + (msimode) ? 0 : base_hartid,
> + (msimode) ? 0 : hart_count,
> + VIRT_IRQCHIP_NUM_SOURCES,
> + VIRT_IRQCHIP_NUM_PRIO_BITS,
> + msimode, false, aplic_m);
> +
> if (msimode) {
> if (!kvm_enabled()) {
> /* Per-socket M-level IMSICs */
> @@ -1329,32 +1345,20 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
> base_hartid + i, false, 1 + aia_guests,
> VIRT_IRQCHIP_NUM_MSIS);
> }
> - }
>
> - if (!kvm_enabled()) {
> - /* Per-socket M-level APLIC */
> - aplic_m = riscv_aplic_create(memmap[VIRT_APLIC_M].base +
> - socket * memmap[VIRT_APLIC_M].size,
> - memmap[VIRT_APLIC_M].size,
> - (msimode) ? 0 : base_hartid,
> - (msimode) ? 0 : hart_count,
> - VIRT_IRQCHIP_NUM_SOURCES,
> - VIRT_IRQCHIP_NUM_PRIO_BITS,
> - msimode, true, NULL);
> - }
>
> - /* Per-socket S-level APLIC */
> - aplic_s = riscv_aplic_create(memmap[VIRT_APLIC_S].base +
> - socket * memmap[VIRT_APLIC_S].size,
> - memmap[VIRT_APLIC_S].size,
> - (msimode) ? 0 : base_hartid,
> - (msimode) ? 0 : hart_count,
> + if (kvm_irqchip_in_kernel()) {
> + kvm_riscv_aia_create(MACHINE(s), IMSIC_MMIO_GROUP_MIN_SHIFT,
> VIRT_IRQCHIP_NUM_SOURCES,
> - VIRT_IRQCHIP_NUM_PRIO_BITS,
> - msimode, false, aplic_m);
> + VIRT_IRQCHIP_NUM_MSIS,
> + memmap[VIRT_APLIC_S].base,
> + memmap[VIRT_IMSIC_S].base,
> + aia_guests);
> + }
>
> - if (kvm_enabled() && msimode) {
> - riscv_aplic_set_kvm_msicfgaddr(RISCV_APLIC(aplic_s), addr);
> + if (kvm_enabled()) {
> + riscv_aplic_set_kvm_msicfgaddr(RISCV_APLIC(aplic_s), addr);
> + }
> }
>
> return kvm_enabled() ? aplic_s : aplic_m;
> @@ -1621,9 +1625,8 @@ static void virt_machine_init(MachineState *machine)
> s->irqchip[i] = virt_create_plic(memmap, i,
> base_hartid, hart_count);
> } else {
> - s->irqchip[i] = virt_create_aia(s->aia_type, s->aia_guests,
> - memmap, i, base_hartid,
> - hart_count);
> + s->irqchip[i] = virt_create_aia(s, memmap, i,
> + base_hartid, hart_count);
> }
>
> /* Try to use different IRQCHIP instance based device type */
> @@ -1641,14 +1644,6 @@ static void virt_machine_init(MachineState *machine)
> }
> }
>
> - if (kvm_enabled() && virt_use_kvm_aia_aplic_imsic(s->aia_type)) {
> - kvm_riscv_aia_create(machine, IMSIC_MMIO_GROUP_MIN_SHIFT,
> - VIRT_IRQCHIP_NUM_SOURCES, VIRT_IRQCHIP_NUM_MSIS,
> - memmap[VIRT_APLIC_S].base,
> - memmap[VIRT_IMSIC_S].base,
> - s->aia_guests);
> - }
> -
> if (riscv_is_32bit(&s->soc[0])) {
> #if HOST_LONG_BITS == 64
> /* limit RAM size in a 32-bit system */
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4] hw/riscv/virt: KVM AIA refinement
2025-02-17 19:24 ` Daniel Henrique Barboza
@ 2025-02-19 11:25 ` Yong-Xuan Wang
0 siblings, 0 replies; 7+ messages in thread
From: Yong-Xuan Wang @ 2025-02-19 11:25 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, greentime.hu, vincent.chen, frank.chang,
jim.shu, Palmer Dabbelt, Alistair Francis, Weiwei Li, Liu Zhiwei
Hi Daniel,
On Tue, Feb 18, 2025 at 3:24 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
>
>
> On 2/17/25 5:19 AM, Yong-Xuan Wang wrote:
> > KVM AIA is only needed to be set when the virt machine use the AIA MSI.
> > So we can move the KVM AIA configuration into virt_create_aia() to reduce
> > the condition checking.
> >
> > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> > ---
>
> Unfortunately this doesn't work.
>
> The reason is that kvm_riscv_aia_create(), as it is now, is called only once
> during virt_machine_init() and it's already handling initialization for each socket:
>
>
> for (socket = 0; socket < socket_count; socket++) {
> socket_imsic_base = imsic_base + socket * (1U << group_shift);
> hart_count = riscv_socket_hart_count(machine, socket);
> base_hart = riscv_socket_first_hartid(machine, socket);
>
> if (max_hart_per_socket < hart_count) {
> max_hart_per_socket = hart_count;
> }
>
> for (i = 0; i < hart_count; i++) {
> imsic_addr = socket_imsic_base + i * IMSIC_HART_SIZE(guest_bits);
> ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_ADDR,
> KVM_DEV_RISCV_AIA_ADDR_IMSIC(i + base_hart),
> &imsic_addr, true, NULL);
> if (ret < 0) {
> error_report("KVM AIA: failed to set the IMSIC address for hart %d", i);
> exit(1);
> }
> }
> }
>
> After this change, kvm_riscv_aia_create() is being called once for each socket since it's
> now being called inside virt_create_aia(). And this will cause errors when running qemu-kvm
> with more than one socket:
>
> ./qemu-system-riscv64 \
> -machine virt,accel=kvm,aia=aplic-imsic -m 2G \
> -object memory-backend-ram,size=1G,id=m0 \
> -object memory-backend-ram,size=1G,id=m1 \
> -smp 2,sockets=2,cores=1,threads=1 \
> -numa node,memdev=m0,cpus=0,nodeid=0 \
> -numa node,memdev=m1,cpus=1,nodeid=1 \
> (...)
> qemu-system-riscv64: KVM AIA: failed to set the IMSIC address for hart 0
>
Oh I forgot to test the NUMA config. Sorry.
>
> To make this patch work we would need changes in kvm_riscv_aia_create() to handle just the
> current socket. The loop I mentioned above is one place, and there's another place where
> we set group_bits and group_shift if socket_count > 1.
>
Also we need to find a place to initialize the in-kernel AIA after
setting up all the IMSICs among sockets. This would make things more
complicated. I will remove this patch in the next version. Thank you!
Regards,
Yong-Xuan
> To be honest I'm not sure if all these extra required changes are worth the simplification
> this patch is proposing.
>
>
> Thanks,
>
> Daniel
>
>
>
>
>
> > hw/riscv/virt.c | 79 +++++++++++++++++++++++--------------------------
> > 1 file changed, 37 insertions(+), 42 deletions(-)
> >
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > index dae46f4733cd..a52117ef71ee 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -58,14 +58,6 @@
> > #include "qapi/qapi-visit-common.h"
> > #include "hw/virtio/virtio-iommu.h"
> >
> > -/* KVM AIA only supports APLIC MSI. APLIC Wired is always emulated by QEMU. */
> > -static bool virt_use_kvm_aia_aplic_imsic(RISCVVirtAIAType aia_type)
> > -{
> > - bool msimode = aia_type == VIRT_AIA_TYPE_APLIC_IMSIC;
> > -
> > - return riscv_is_kvm_aia_aplic_imsic(msimode);
> > -}
> > -
> > static bool virt_use_emulated_aplic(RISCVVirtAIAType aia_type)
> > {
> > bool msimode = aia_type == VIRT_AIA_TYPE_APLIC_IMSIC;
> > @@ -1298,10 +1290,12 @@ static DeviceState *virt_create_plic(const MemMapEntry *memmap, int socket,
> > return ret;
> > }
> >
> > -static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
> > +static DeviceState *virt_create_aia(RISCVVirtState *s,
> > const MemMapEntry *memmap, int socket,
> > int base_hartid, int hart_count)
> > {
> > + RISCVVirtAIAType aia_type = s->aia_type;
> > + int aia_guests = s->aia_guests;
> > int i;
> > hwaddr addr = 0;
> > uint32_t guest_bits;
> > @@ -1309,6 +1303,28 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
> > DeviceState *aplic_m = NULL;
> > bool msimode = aia_type == VIRT_AIA_TYPE_APLIC_IMSIC;
> >
> > + if (!kvm_enabled()) {
> > + /* Per-socket M-level APLIC */
> > + aplic_m = riscv_aplic_create(memmap[VIRT_APLIC_M].base +
> > + socket * memmap[VIRT_APLIC_M].size,
> > + memmap[VIRT_APLIC_M].size,
> > + (msimode) ? 0 : base_hartid,
> > + (msimode) ? 0 : hart_count,
> > + VIRT_IRQCHIP_NUM_SOURCES,
> > + VIRT_IRQCHIP_NUM_PRIO_BITS,
> > + msimode, true, NULL);
> > + }
> > +
> > + /* Per-socket S-level APLIC */
> > + aplic_s = riscv_aplic_create(memmap[VIRT_APLIC_S].base +
> > + socket * memmap[VIRT_APLIC_S].size,
> > + memmap[VIRT_APLIC_S].size,
> > + (msimode) ? 0 : base_hartid,
> > + (msimode) ? 0 : hart_count,
> > + VIRT_IRQCHIP_NUM_SOURCES,
> > + VIRT_IRQCHIP_NUM_PRIO_BITS,
> > + msimode, false, aplic_m);
> > +
> > if (msimode) {
> > if (!kvm_enabled()) {
> > /* Per-socket M-level IMSICs */
> > @@ -1329,32 +1345,20 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
> > base_hartid + i, false, 1 + aia_guests,
> > VIRT_IRQCHIP_NUM_MSIS);
> > }
> > - }
> >
> > - if (!kvm_enabled()) {
> > - /* Per-socket M-level APLIC */
> > - aplic_m = riscv_aplic_create(memmap[VIRT_APLIC_M].base +
> > - socket * memmap[VIRT_APLIC_M].size,
> > - memmap[VIRT_APLIC_M].size,
> > - (msimode) ? 0 : base_hartid,
> > - (msimode) ? 0 : hart_count,
> > - VIRT_IRQCHIP_NUM_SOURCES,
> > - VIRT_IRQCHIP_NUM_PRIO_BITS,
> > - msimode, true, NULL);
> > - }
> >
> > - /* Per-socket S-level APLIC */
> > - aplic_s = riscv_aplic_create(memmap[VIRT_APLIC_S].base +
> > - socket * memmap[VIRT_APLIC_S].size,
> > - memmap[VIRT_APLIC_S].size,
> > - (msimode) ? 0 : base_hartid,
> > - (msimode) ? 0 : hart_count,
> > + if (kvm_irqchip_in_kernel()) {
> > + kvm_riscv_aia_create(MACHINE(s), IMSIC_MMIO_GROUP_MIN_SHIFT,
> > VIRT_IRQCHIP_NUM_SOURCES,
> > - VIRT_IRQCHIP_NUM_PRIO_BITS,
> > - msimode, false, aplic_m);
> > + VIRT_IRQCHIP_NUM_MSIS,
> > + memmap[VIRT_APLIC_S].base,
> > + memmap[VIRT_IMSIC_S].base,
> > + aia_guests);
> > + }
> >
> > - if (kvm_enabled() && msimode) {
> > - riscv_aplic_set_kvm_msicfgaddr(RISCV_APLIC(aplic_s), addr);
> > + if (kvm_enabled()) {
> > + riscv_aplic_set_kvm_msicfgaddr(RISCV_APLIC(aplic_s), addr);
> > + }
> > }
> >
> > return kvm_enabled() ? aplic_s : aplic_m;
> > @@ -1621,9 +1625,8 @@ static void virt_machine_init(MachineState *machine)
> > s->irqchip[i] = virt_create_plic(memmap, i,
> > base_hartid, hart_count);
> > } else {
> > - s->irqchip[i] = virt_create_aia(s->aia_type, s->aia_guests,
> > - memmap, i, base_hartid,
> > - hart_count);
> > + s->irqchip[i] = virt_create_aia(s, memmap, i,
> > + base_hartid, hart_count);
> > }
> >
> > /* Try to use different IRQCHIP instance based device type */
> > @@ -1641,14 +1644,6 @@ static void virt_machine_init(MachineState *machine)
> > }
> > }
> >
> > - if (kvm_enabled() && virt_use_kvm_aia_aplic_imsic(s->aia_type)) {
> > - kvm_riscv_aia_create(machine, IMSIC_MMIO_GROUP_MIN_SHIFT,
> > - VIRT_IRQCHIP_NUM_SOURCES, VIRT_IRQCHIP_NUM_MSIS,
> > - memmap[VIRT_APLIC_S].base,
> > - memmap[VIRT_IMSIC_S].base,
> > - s->aia_guests);
> > - }
> > -
> > if (riscv_is_32bit(&s->soc[0])) {
> > #if HOST_LONG_BITS == 64
> > /* limit RAM size in a 32-bit system */
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-02-19 11:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-17 8:19 [PATCH 0/4] riscv: AIA: refinement for KVM acceleration Yong-Xuan Wang
2025-02-17 8:19 ` [PATCH 1/4] hw/riscv/virt: KVM AIA refinement Yong-Xuan Wang
2025-02-17 19:24 ` Daniel Henrique Barboza
2025-02-19 11:25 ` Yong-Xuan Wang
2025-02-17 8:19 ` [PATCH 2/4] hw/intc/imsic: refine the IMSIC realize Yong-Xuan Wang
2025-02-17 8:19 ` [PATCH 3/4] hw/intc/aplic: refine the APLIC realize Yong-Xuan Wang
2025-02-17 8:19 ` [PATCH 4/4] hw/intc/aplic: refine kvm_msicfgaddr Yong-Xuan Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).