* [PATCH 1/1] target/riscv/kvm.c: remove group setting of KVM AIA if the machine only has 1 socket
2023-12-18 9:05 [PATCH 1/1] hw/riscv/virt.c: fix the interrupts-extended property format of PLIC Yong-Xuan Wang
@ 2023-12-18 9:05 ` Yong-Xuan Wang
2023-12-18 21:42 ` Daniel Henrique Barboza
2023-12-18 21:20 ` [PATCH 1/1] hw/riscv/virt.c: fix the interrupts-extended property format of PLIC Daniel Henrique Barboza
2024-01-04 0:58 ` Alistair Francis
2 siblings, 1 reply; 5+ messages in thread
From: Yong-Xuan Wang @ 2023-12-18 9:05 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: greentime.hu, vincent.chen, frank.chang, jim.shu, Yong-Xuan Wang,
Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Andrew Jones,
Philippe Mathieu-Daudé
The emulated AIA within the Linux kernel restores the HART index
of the IMSICs according to the configured AIA settings. During
this process, the group setting is used only when the machine
partitions harts into groups. It's unnecessary to set the group
configuration if the machine has only one socket, as its address
space might not contain the group shift.
Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
Reviewed-by: Jim Shu <jim.shu@sifive.com>
---
target/riscv/kvm/kvm-cpu.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 62a1e51f0a2e..6494597157b8 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1387,21 +1387,24 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
exit(1);
}
- socket_bits = find_last_bit(&socket_count, BITS_PER_LONG) + 1;
- ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
- KVM_DEV_RISCV_AIA_CONFIG_GROUP_BITS,
- &socket_bits, true, NULL);
- if (ret < 0) {
- error_report("KVM AIA: failed to set group_bits");
- exit(1);
- }
- ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
- KVM_DEV_RISCV_AIA_CONFIG_GROUP_SHIFT,
- &group_shift, true, NULL);
- if (ret < 0) {
- error_report("KVM AIA: failed to set group_shift");
- exit(1);
+ if (socket_count > 1) {
+ socket_bits = find_last_bit(&socket_count, BITS_PER_LONG) + 1;
+ ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
+ KVM_DEV_RISCV_AIA_CONFIG_GROUP_BITS,
+ &socket_bits, true, NULL);
+ if (ret < 0) {
+ error_report("KVM AIA: failed to set group_bits");
+ exit(1);
+ }
+
+ ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
+ KVM_DEV_RISCV_AIA_CONFIG_GROUP_SHIFT,
+ &group_shift, true, NULL);
+ if (ret < 0) {
+ error_report("KVM AIA: failed to set group_shift");
+ exit(1);
+ }
}
guest_bits = guest_num == 0 ? 0 :
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] target/riscv/kvm.c: remove group setting of KVM AIA if the machine only has 1 socket
2023-12-18 9:05 ` [PATCH 1/1] target/riscv/kvm.c: remove group setting of KVM AIA if the machine only has 1 socket Yong-Xuan Wang
@ 2023-12-18 21:42 ` Daniel Henrique Barboza
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Henrique Barboza @ 2023-12-18 21:42 UTC (permalink / raw)
To: Yong-Xuan Wang, qemu-devel, qemu-riscv
Cc: greentime.hu, vincent.chen, frank.chang, jim.shu, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei, Andrew Jones,
Philippe Mathieu-Daudé
On 12/18/23 06:05, Yong-Xuan Wang wrote:
> The emulated AIA within the Linux kernel restores the HART index
> of the IMSICs according to the configured AIA settings. During
> this process, the group setting is used only when the machine
> partitions harts into groups. It's unnecessary to set the group
> configuration if the machine has only one socket, as its address
> space might not contain the group shift.
>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> Reviewed-by: Jim Shu <jim.shu@sifive.com>
> ---
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> target/riscv/kvm/kvm-cpu.c | 31 +++++++++++++++++--------------
> 1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 62a1e51f0a2e..6494597157b8 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1387,21 +1387,24 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
> exit(1);
> }
>
> - socket_bits = find_last_bit(&socket_count, BITS_PER_LONG) + 1;
> - ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
> - KVM_DEV_RISCV_AIA_CONFIG_GROUP_BITS,
> - &socket_bits, true, NULL);
> - if (ret < 0) {
> - error_report("KVM AIA: failed to set group_bits");
> - exit(1);
> - }
>
> - ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
> - KVM_DEV_RISCV_AIA_CONFIG_GROUP_SHIFT,
> - &group_shift, true, NULL);
> - if (ret < 0) {
> - error_report("KVM AIA: failed to set group_shift");
> - exit(1);
> + if (socket_count > 1) {
> + socket_bits = find_last_bit(&socket_count, BITS_PER_LONG) + 1;
> + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
> + KVM_DEV_RISCV_AIA_CONFIG_GROUP_BITS,
> + &socket_bits, true, NULL);
> + if (ret < 0) {
> + error_report("KVM AIA: failed to set group_bits");
> + exit(1);
> + }
> +
> + ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
> + KVM_DEV_RISCV_AIA_CONFIG_GROUP_SHIFT,
> + &group_shift, true, NULL);
> + if (ret < 0) {
> + error_report("KVM AIA: failed to set group_shift");
> + exit(1);
> + }
> }
>
> guest_bits = guest_num == 0 ? 0 :
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] hw/riscv/virt.c: fix the interrupts-extended property format of PLIC
2023-12-18 9:05 [PATCH 1/1] hw/riscv/virt.c: fix the interrupts-extended property format of PLIC Yong-Xuan Wang
2023-12-18 9:05 ` [PATCH 1/1] target/riscv/kvm.c: remove group setting of KVM AIA if the machine only has 1 socket Yong-Xuan Wang
@ 2023-12-18 21:20 ` Daniel Henrique Barboza
2024-01-04 0:58 ` Alistair Francis
2 siblings, 0 replies; 5+ messages in thread
From: Daniel Henrique Barboza @ 2023-12-18 21:20 UTC (permalink / raw)
To: Yong-Xuan Wang, qemu-devel, qemu-riscv
Cc: greentime.hu, vincent.chen, frank.chang, jim.shu, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei
On 12/18/23 06:05, Yong-Xuan Wang wrote:
> The interrupts-extended property of PLIC only has 2 * hart number
> fields when KVM enabled, copy 4 * hart number fields to fdt will
> expose some uninitialized value.
>
> In this patch, I also refactor the code about the setting of
> interrupts-extended property of PLIC for improved readability.
>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> Reviewed-by: Jim Shu <jim.shu@sifive.com>
> ---
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> hw/riscv/virt.c | 47 +++++++++++++++++++++++++++--------------------
> 1 file changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index d2eac2415619..e42baf82cab6 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -460,24 +460,6 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
> "sifive,plic-1.0.0", "riscv,plic0"
> };
>
> - if (kvm_enabled()) {
> - plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
> - } else {
> - plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 4);
> - }
> -
> - for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
> - if (kvm_enabled()) {
> - plic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
> - plic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_EXT);
> - } else {
> - plic_cells[cpu * 4 + 0] = cpu_to_be32(intc_phandles[cpu]);
> - plic_cells[cpu * 4 + 1] = cpu_to_be32(IRQ_M_EXT);
> - plic_cells[cpu * 4 + 2] = cpu_to_be32(intc_phandles[cpu]);
> - plic_cells[cpu * 4 + 3] = cpu_to_be32(IRQ_S_EXT);
> - }
> - }
> -
> plic_phandles[socket] = (*phandle)++;
> plic_addr = memmap[VIRT_PLIC].base + (memmap[VIRT_PLIC].size * socket);
> plic_name = g_strdup_printf("/soc/plic@%lx", plic_addr);
> @@ -490,8 +472,33 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
> (char **)&plic_compat,
> ARRAY_SIZE(plic_compat));
> qemu_fdt_setprop(ms->fdt, plic_name, "interrupt-controller", NULL, 0);
> - qemu_fdt_setprop(ms->fdt, plic_name, "interrupts-extended",
> - plic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
> +
> + if (kvm_enabled()) {
> + plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
> +
> + for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
> + plic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
> + plic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_EXT);
> + }
> +
> + qemu_fdt_setprop(ms->fdt, plic_name, "interrupts-extended",
> + plic_cells,
> + s->soc[socket].num_harts * sizeof(uint32_t) * 2);
> + } else {
> + plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 4);
> +
> + for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
> + plic_cells[cpu * 4 + 0] = cpu_to_be32(intc_phandles[cpu]);
> + plic_cells[cpu * 4 + 1] = cpu_to_be32(IRQ_M_EXT);
> + plic_cells[cpu * 4 + 2] = cpu_to_be32(intc_phandles[cpu]);
> + plic_cells[cpu * 4 + 3] = cpu_to_be32(IRQ_S_EXT);
> + }
> +
> + qemu_fdt_setprop(ms->fdt, plic_name, "interrupts-extended",
> + plic_cells,
> + s->soc[socket].num_harts * sizeof(uint32_t) * 4);
> + }
> +
> qemu_fdt_setprop_cells(ms->fdt, plic_name, "reg",
> 0x0, plic_addr, 0x0, memmap[VIRT_PLIC].size);
> qemu_fdt_setprop_cell(ms->fdt, plic_name, "riscv,ndev",
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] hw/riscv/virt.c: fix the interrupts-extended property format of PLIC
2023-12-18 9:05 [PATCH 1/1] hw/riscv/virt.c: fix the interrupts-extended property format of PLIC Yong-Xuan Wang
2023-12-18 9:05 ` [PATCH 1/1] target/riscv/kvm.c: remove group setting of KVM AIA if the machine only has 1 socket Yong-Xuan Wang
2023-12-18 21:20 ` [PATCH 1/1] hw/riscv/virt.c: fix the interrupts-extended property format of PLIC Daniel Henrique Barboza
@ 2024-01-04 0:58 ` Alistair Francis
2 siblings, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2024-01-04 0:58 UTC (permalink / raw)
To: Yong-Xuan Wang
Cc: qemu-devel, qemu-riscv, greentime.hu, vincent.chen, frank.chang,
jim.shu, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei
On Mon, Dec 18, 2023 at 7:07 PM Yong-Xuan Wang <yongxuan.wang@sifive.com> wrote:
>
> The interrupts-extended property of PLIC only has 2 * hart number
> fields when KVM enabled, copy 4 * hart number fields to fdt will
> expose some uninitialized value.
>
> In this patch, I also refactor the code about the setting of
> interrupts-extended property of PLIC for improved readability.
>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> Reviewed-by: Jim Shu <jim.shu@sifive.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> hw/riscv/virt.c | 47 +++++++++++++++++++++++++++--------------------
> 1 file changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index d2eac2415619..e42baf82cab6 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -460,24 +460,6 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
> "sifive,plic-1.0.0", "riscv,plic0"
> };
>
> - if (kvm_enabled()) {
> - plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
> - } else {
> - plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 4);
> - }
> -
> - for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
> - if (kvm_enabled()) {
> - plic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
> - plic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_EXT);
> - } else {
> - plic_cells[cpu * 4 + 0] = cpu_to_be32(intc_phandles[cpu]);
> - plic_cells[cpu * 4 + 1] = cpu_to_be32(IRQ_M_EXT);
> - plic_cells[cpu * 4 + 2] = cpu_to_be32(intc_phandles[cpu]);
> - plic_cells[cpu * 4 + 3] = cpu_to_be32(IRQ_S_EXT);
> - }
> - }
> -
> plic_phandles[socket] = (*phandle)++;
> plic_addr = memmap[VIRT_PLIC].base + (memmap[VIRT_PLIC].size * socket);
> plic_name = g_strdup_printf("/soc/plic@%lx", plic_addr);
> @@ -490,8 +472,33 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
> (char **)&plic_compat,
> ARRAY_SIZE(plic_compat));
> qemu_fdt_setprop(ms->fdt, plic_name, "interrupt-controller", NULL, 0);
> - qemu_fdt_setprop(ms->fdt, plic_name, "interrupts-extended",
> - plic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
> +
> + if (kvm_enabled()) {
> + plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
> +
> + for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
> + plic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
> + plic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_EXT);
> + }
> +
> + qemu_fdt_setprop(ms->fdt, plic_name, "interrupts-extended",
> + plic_cells,
> + s->soc[socket].num_harts * sizeof(uint32_t) * 2);
> + } else {
> + plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 4);
> +
> + for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
> + plic_cells[cpu * 4 + 0] = cpu_to_be32(intc_phandles[cpu]);
> + plic_cells[cpu * 4 + 1] = cpu_to_be32(IRQ_M_EXT);
> + plic_cells[cpu * 4 + 2] = cpu_to_be32(intc_phandles[cpu]);
> + plic_cells[cpu * 4 + 3] = cpu_to_be32(IRQ_S_EXT);
> + }
> +
> + qemu_fdt_setprop(ms->fdt, plic_name, "interrupts-extended",
> + plic_cells,
> + s->soc[socket].num_harts * sizeof(uint32_t) * 4);
> + }
> +
> qemu_fdt_setprop_cells(ms->fdt, plic_name, "reg",
> 0x0, plic_addr, 0x0, memmap[VIRT_PLIC].size);
> qemu_fdt_setprop_cell(ms->fdt, plic_name, "riscv,ndev",
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread