[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 2/6] target/riscv: support the AIA device emulation with K
From: |
Andrew Jones |
Subject: |
Re: [PATCH v4 2/6] target/riscv: support the AIA device emulation with KVM enabled |
Date: |
Tue, 4 Jul 2023 16:35:11 +0200 |
On Wed, Jun 21, 2023 at 02:54:52PM +0000, Yong-Xuan Wang wrote:
> Remove M mode AIA devices when using KVM acceleration
>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> Reviewed-by: Jim Shu <jim.shu@sifive.com>
> ---
> hw/riscv/virt.c | 207 +++++++++++++++++++++++++-----------------------
> 1 file changed, 108 insertions(+), 99 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 245c7b97b2..4a1d29a741 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -531,52 +531,54 @@ static void create_fdt_imsic(RISCVVirtState *s, const
> MemMapEntry *memmap,
> imsic_cells = g_new0(uint32_t, ms->smp.cpus * 2);
> imsic_regs = g_new0(uint32_t, socket_count * 4);
>
> - /* M-level IMSIC node */
> - for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
> - imsic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
> - imsic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_EXT);
> - }
> - imsic_max_hart_per_socket = 0;
> - for (socket = 0; socket < socket_count; socket++) {
> - imsic_addr = memmap[VIRT_IMSIC_M].base +
> - socket * VIRT_IMSIC_GROUP_MAX_SIZE;
> - imsic_size = IMSIC_HART_SIZE(0) * s->soc[socket].num_harts;
> - imsic_regs[socket * 4 + 0] = 0;
> - imsic_regs[socket * 4 + 1] = cpu_to_be32(imsic_addr);
> - imsic_regs[socket * 4 + 2] = 0;
> - imsic_regs[socket * 4 + 3] = cpu_to_be32(imsic_size);
> - if (imsic_max_hart_per_socket < s->soc[socket].num_harts) {
> - imsic_max_hart_per_socket = s->soc[socket].num_harts;
> + if (!kvm_enabled()) {
> + /* M-level IMSIC node */
> + for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
> + imsic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
> + imsic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_EXT);
> }
> - }
> - imsic_name = g_strdup_printf("/soc/imsics@%lx",
> - (unsigned long)memmap[VIRT_IMSIC_M].base);
> - qemu_fdt_add_subnode(ms->fdt, imsic_name);
> - qemu_fdt_setprop_string(ms->fdt, imsic_name, "compatible",
> - "riscv,imsics");
> - qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#interrupt-cells",
> - FDT_IMSIC_INT_CELLS);
> - qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller",
> - NULL, 0);
> - qemu_fdt_setprop(ms->fdt, imsic_name, "msi-controller",
> - NULL, 0);
> - qemu_fdt_setprop(ms->fdt, imsic_name, "interrupts-extended",
> - imsic_cells, ms->smp.cpus * sizeof(uint32_t) * 2);
> - qemu_fdt_setprop(ms->fdt, imsic_name, "reg", imsic_regs,
> - socket_count * sizeof(uint32_t) * 4);
> - qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,num-ids",
> - VIRT_IRQCHIP_NUM_MSIS);
> - if (socket_count > 1) {
> - qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,hart-index-bits",
> - imsic_num_bits(imsic_max_hart_per_socket));
> - qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,group-index-bits",
> - imsic_num_bits(socket_count));
> - qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,group-index-shift",
> - IMSIC_MMIO_GROUP_MIN_SHIFT);
> - }
> - qemu_fdt_setprop_cell(ms->fdt, imsic_name, "phandle", *msi_m_phandle);
> + imsic_max_hart_per_socket = 0;
> + for (socket = 0; socket < socket_count; socket++) {
> + imsic_addr = memmap[VIRT_IMSIC_M].base +
> + socket * VIRT_IMSIC_GROUP_MAX_SIZE;
> + imsic_size = IMSIC_HART_SIZE(0) * s->soc[socket].num_harts;
> + imsic_regs[socket * 4 + 0] = 0;
> + imsic_regs[socket * 4 + 1] = cpu_to_be32(imsic_addr);
> + imsic_regs[socket * 4 + 2] = 0;
> + imsic_regs[socket * 4 + 3] = cpu_to_be32(imsic_size);
> + if (imsic_max_hart_per_socket < s->soc[socket].num_harts) {
> + imsic_max_hart_per_socket = s->soc[socket].num_harts;
> + }
> + }
> + imsic_name = g_strdup_printf("/soc/imsics@%lx",
> + (unsigned long)memmap[VIRT_IMSIC_M].base);
> + qemu_fdt_add_subnode(ms->fdt, imsic_name);
> + qemu_fdt_setprop_string(ms->fdt, imsic_name, "compatible",
> + "riscv,imsics");
> + qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#interrupt-cells",
> + FDT_IMSIC_INT_CELLS);
> + qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller",
> + NULL, 0);
> + qemu_fdt_setprop(ms->fdt, imsic_name, "msi-controller",
> + NULL, 0);
> + qemu_fdt_setprop(ms->fdt, imsic_name, "interrupts-extended",
> + imsic_cells, ms->smp.cpus * sizeof(uint32_t) * 2);
> + qemu_fdt_setprop(ms->fdt, imsic_name, "reg", imsic_regs,
> + socket_count * sizeof(uint32_t) * 4);
> + qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,num-ids",
> + VIRT_IRQCHIP_NUM_MSIS);
> + if (socket_count > 1) {
> + qemu_fdt_setprop_cell(ms->fdt, imsic_name,
> "riscv,hart-index-bits",
> + imsic_num_bits(imsic_max_hart_per_socket));
> + qemu_fdt_setprop_cell(ms->fdt, imsic_name,
> "riscv,group-index-bits",
> + imsic_num_bits(socket_count));
> + qemu_fdt_setprop_cell(ms->fdt, imsic_name,
> + "riscv,group-index-shift", IMSIC_MMIO_GROUP_MIN_SHIFT);
> + }
> + qemu_fdt_setprop_cell(ms->fdt, imsic_name, "phandle",
> *msi_m_phandle);
>
> - g_free(imsic_name);
> + g_free(imsic_name);
> + }
The create_fdt_imsic() code for M-mode and S-mode is nearly identical. I
suggest creating a helper function where the differences have been
parameterized so we can write this something like
void create_fdt_imsic(...)
{
if (!kvm_enabled()) {
create_fdt_one_imsic(VIRT_IMSIC_M, 0, ...);
}
create_fdt_one_imsic(VIRT_IMSIC_S, imsic_num_bits(s->aia_guests + 1), ...);
}
>
> /* S-level IMSIC node */
> for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
> @@ -653,37 +655,40 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
> aplic_s_phandle = (*phandle)++;
> aplic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
>
> - /* M-level APLIC node */
> - for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
> - aplic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
> - aplic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_EXT);
> - }
> - aplic_addr = memmap[VIRT_APLIC_M].base +
> - (memmap[VIRT_APLIC_M].size * socket);
> - aplic_name = g_strdup_printf("/soc/aplic@%lx", aplic_addr);
> - qemu_fdt_add_subnode(ms->fdt, aplic_name);
> - qemu_fdt_setprop_string(ms->fdt, aplic_name, "compatible",
> "riscv,aplic");
> - qemu_fdt_setprop_cell(ms->fdt, aplic_name,
> - "#interrupt-cells", FDT_APLIC_INT_CELLS);
> - qemu_fdt_setprop(ms->fdt, aplic_name, "interrupt-controller", NULL, 0);
> - if (s->aia_type == VIRT_AIA_TYPE_APLIC) {
> - qemu_fdt_setprop(ms->fdt, aplic_name, "interrupts-extended",
> - aplic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 2);
> - } else {
> - qemu_fdt_setprop_cell(ms->fdt, aplic_name, "msi-parent",
> - msi_m_phandle);
> + if (!kvm_enabled()) {
> + /* M-level APLIC node */
> + for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
> + aplic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
> + aplic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_EXT);
> + }
> + aplic_addr = memmap[VIRT_APLIC_M].base +
> + (memmap[VIRT_APLIC_M].size * socket);
> + aplic_name = g_strdup_printf("/soc/aplic@%lx", aplic_addr);
> + qemu_fdt_add_subnode(ms->fdt, aplic_name);
> + qemu_fdt_setprop_string(ms->fdt, aplic_name,
> + "compatible", "riscv,aplic");
> + qemu_fdt_setprop_cell(ms->fdt, aplic_name,
> + "#interrupt-cells", FDT_APLIC_INT_CELLS);
> + qemu_fdt_setprop(ms->fdt, aplic_name, "interrupt-controller", NULL,
> 0);
> + if (s->aia_type == VIRT_AIA_TYPE_APLIC) {
> + qemu_fdt_setprop(ms->fdt, aplic_name, "interrupts-extended",
> + aplic_cells, s->soc[socket].num_harts * sizeof(uint32_t) *
> 2);
> + } else {
> + qemu_fdt_setprop_cell(ms->fdt, aplic_name, "msi-parent",
> + msi_m_phandle);
> + }
> + qemu_fdt_setprop_cells(ms->fdt, aplic_name, "reg",
> + 0x0, aplic_addr, 0x0, memmap[VIRT_APLIC_M].size);
> + qemu_fdt_setprop_cell(ms->fdt, aplic_name, "riscv,num-sources",
> + VIRT_IRQCHIP_NUM_SOURCES);
> + qemu_fdt_setprop_cell(ms->fdt, aplic_name, "riscv,children",
> + aplic_s_phandle);
> + qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegate",
> + aplic_s_phandle, 0x1, VIRT_IRQCHIP_NUM_SOURCES);
> + riscv_socket_fdt_write_id(ms, aplic_name, socket);
> + qemu_fdt_setprop_cell(ms->fdt, aplic_name, "phandle",
> aplic_m_phandle);
> + g_free(aplic_name);
> }
> - qemu_fdt_setprop_cells(ms->fdt, aplic_name, "reg",
> - 0x0, aplic_addr, 0x0, memmap[VIRT_APLIC_M].size);
> - qemu_fdt_setprop_cell(ms->fdt, aplic_name, "riscv,num-sources",
> - VIRT_IRQCHIP_NUM_SOURCES);
> - qemu_fdt_setprop_cell(ms->fdt, aplic_name, "riscv,children",
> - aplic_s_phandle);
> - qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegate",
> - aplic_s_phandle, 0x1, VIRT_IRQCHIP_NUM_SOURCES);
> - riscv_socket_fdt_write_id(ms, aplic_name, socket);
> - qemu_fdt_setprop_cell(ms->fdt, aplic_name, "phandle", aplic_m_phandle);
> - g_free(aplic_name);
I think it should be possible to parameterize an aplic helper as well.
>
> /* S-level APLIC node */
> for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
> @@ -1162,16 +1167,20 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType
> aia_type, int aia_guests,
> int i;
> hwaddr addr;
> uint32_t guest_bits;
> - DeviceState *aplic_m;
> + DeviceState *aplic_s = NULL;
> + DeviceState *aplic_m = NULL;
> bool msimode = (aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) ? true : false;
Not introduced with this patch, but we should drop the unnecessary () and
'? true : false' on this line.
>
> if (msimode) {
> - /* Per-socket M-level IMSICs */
> - addr = memmap[VIRT_IMSIC_M].base + socket *
> VIRT_IMSIC_GROUP_MAX_SIZE;
> - for (i = 0; i < hart_count; i++) {
> - riscv_imsic_create(addr + i * IMSIC_HART_SIZE(0),
> - base_hartid + i, true, 1,
> - VIRT_IRQCHIP_NUM_MSIS);
> + if (!kvm_enabled()) {
> + /* Per-socket M-level IMSICs */
> + addr = memmap[VIRT_IMSIC_M].base +
> + socket * VIRT_IMSIC_GROUP_MAX_SIZE;
> + for (i = 0; i < hart_count; i++) {
> + riscv_imsic_create(addr + i * IMSIC_HART_SIZE(0),
> + base_hartid + i, true, 1,
> + VIRT_IRQCHIP_NUM_MSIS);
> + }
> }
>
> /* Per-socket S-level IMSICs */
> @@ -1184,29 +1193,29 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType
> aia_type, int aia_guests,
> }
> }
>
> - /* 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,
> + 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, true, NULL);
> -
> - if (aplic_m) {
> - /* Per-socket S-level APLIC */
> - 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);
> - }
> + msimode, false, aplic_m);
>
> - return aplic_m;
> + return kvm_enabled() ? aplic_s : aplic_m;
> }
>
> static void create_platform_bus(RISCVVirtState *s, DeviceState *irqchip)
> --
> 2.17.1
>
>
Thanks,
drew
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v4 2/6] target/riscv: support the AIA device emulation with KVM enabled,
Andrew Jones <=