From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
To: Yong-Xuan Wang <yongxuan.wang@sifive.com>
Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
rkanwal@rivosinc.com, anup@brainfault.org, atishp@atishpatra.org,
vincent.chen@sifive.com, greentime.hu@sifive.com,
frank.chang@sifive.com, jim.shu@sifive.com,
Palmer Dabbelt <palmer@dabbelt.com>,
Alistair Francis <alistair.francis@wdc.com>,
Bin Meng <bin.meng@windriver.com>,
Weiwei Li <liweiwei@iscas.ac.cn>,
Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
Subject: Re: [PATCH v3 2/6] target/riscv: support the AIA device emulation with KVM enabled
Date: Wed, 14 Jun 2023 06:13:47 -0300 [thread overview]
Message-ID: <be0fe4d5-428a-a58d-591d-136817ac98d4@ventanamicro.com> (raw)
In-Reply-To: <CAMWQL2gc4GEmEtHdhu64wYT9Smx88k6gQyLYu=cx5fBrQ0X=JA@mail.gmail.com>
Hi,
On 6/12/23 03:50, Yong-Xuan Wang wrote:
> Hi Daniel,
>
> I think this checking can be removed too. Would you send a patch to
> fix it? Or I can remove it in this patch.
Please go ahead and fix it on this patch. Thanks,
Daniel
>
> Regards,
> Yong-Xuan
>
> On Tue, Jun 6, 2023 at 2:45 AM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>>
>>
>>
>> On 5/26/23 03:25, 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 | 199 +++++++++++++++++++++++++-----------------------
>>> 1 file changed, 105 insertions(+), 94 deletions(-)
>>>
>>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>>> index 4e3efbee16..18b94888ab 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);
>>> + }
>>>
>>> /* 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);
>>>
>>> /* 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;
>>>
>>> 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,19 +1193,21 @@ 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,
>>> - (msimode) ? 0 : base_hartid,
>>> - (msimode) ? 0 : hart_count,
>>> - VIRT_IRQCHIP_NUM_SOURCES,
>>> - VIRT_IRQCHIP_NUM_PRIO_BITS,
>>> - msimode, true, NULL);
>>> -
>>> - if (aplic_m) {
>>> + 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);
>>> + }
>>> +
>>> + if (aplic_m || kvm_enabled()) {
>>
>> I am struggling to understand why we're checking for "aplic_m" here given that
>> riscv_aplic_create() will either returning something not NULL or error out via
>> &error_fatal.
>>
>> This behavior predates this patch though.
>>
>>
>>
>> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>>
>>> /* Per-socket S-level APLIC */
>>> - riscv_aplic_create(
>>> + 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,
>>> @@ -1206,7 +1217,7 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
>>> msimode, false, aplic_m);
>>> }
>>>
>>> - return aplic_m;
>>> + return kvm_enabled() ? aplic_s : aplic_m;
>>> }
>>>
>>> static void create_platform_bus(RISCVVirtState *s, DeviceState *irqchip)
next prev parent reply other threads:[~2023-06-14 9:14 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-26 6:25 [PATCH v3 0/6] Add RISC-V KVM AIA Support Yong-Xuan Wang
2023-05-26 6:25 ` [PATCH v3 1/6] update-linux-headers: sync-up header with Linux for KVM AIA support placeholder Yong-Xuan Wang
2023-05-26 6:25 ` [PATCH v3 2/6] target/riscv: support the AIA device emulation with KVM enabled Yong-Xuan Wang
2023-06-05 18:45 ` Daniel Henrique Barboza
2023-06-12 6:50 ` Yong-Xuan Wang
2023-06-14 9:13 ` Daniel Henrique Barboza [this message]
2023-05-26 6:25 ` [PATCH v3 3/6] target/riscv: check the in-kernel irqchip support Yong-Xuan Wang
2023-06-05 18:47 ` Daniel Henrique Barboza
2023-05-26 6:25 ` [PATCH v3 4/6] target/riscv: Create an KVM AIA irqchip Yong-Xuan Wang
2023-06-06 13:45 ` Daniel Henrique Barboza
2023-06-09 10:09 ` Yong-Xuan Wang
2023-05-26 6:25 ` [PATCH v3 5/6] target/riscv: update APLIC and IMSIC to support KVM AIA Yong-Xuan Wang
2023-06-06 13:47 ` Daniel Henrique Barboza
2023-05-26 6:25 ` [PATCH v3 6/6] target/riscv: select KVM AIA in riscv virt machine Yong-Xuan Wang
2023-06-06 13:48 ` Daniel Henrique Barboza
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=be0fe4d5-428a-a58d-591d-136817ac98d4@ventanamicro.com \
--to=dbarboza@ventanamicro.com \
--cc=alistair.francis@wdc.com \
--cc=anup@brainfault.org \
--cc=atishp@atishpatra.org \
--cc=bin.meng@windriver.com \
--cc=frank.chang@sifive.com \
--cc=greentime.hu@sifive.com \
--cc=jim.shu@sifive.com \
--cc=liweiwei@iscas.ac.cn \
--cc=palmer@dabbelt.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=rkanwal@rivosinc.com \
--cc=vincent.chen@sifive.com \
--cc=yongxuan.wang@sifive.com \
--cc=zhiwei_liu@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).