* [PATCH] hw/intc: sifive_plic: Avoid overflowing the addr_config buffer
@ 2022-06-01 1:36 Alistair Francis
2022-06-01 3:11 ` limingwang (A)
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Alistair Francis @ 2022-06-01 1:36 UTC (permalink / raw)
To: jiangyifei, limingwang, qemu-riscv, qemu-devel
Cc: thuth, Bin Meng, alistair23, Alistair Francis, Palmer Dabbelt,
bmeng.cn, Alistair Francis
From: Alistair Francis <alistair.francis@wdc.com>
Since commit ad40be27 "target/riscv: Support start kernel directly by
KVM" we have been overflowing the addr_config on "M,MS..."
configurations, as reported https://gitlab.com/qemu-project/qemu/-/issues/1050.
This commit changes the loop in sifive_plic_create() from iterating over
the number of harts to just iterating over the addr_config. The
addr_config is based on the hart_config, and will contain interrup details
for all harts. This way we can't iterate past the end of addr_config.
Fixes: ad40be27084536 ("target/riscv: Support start kernel directly by KVM")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1050
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
hw/intc/sifive_plic.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
index eebbcf33d4..56d60e9ac9 100644
--- a/hw/intc/sifive_plic.c
+++ b/hw/intc/sifive_plic.c
@@ -431,7 +431,7 @@ DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
uint32_t context_stride, uint32_t aperture_size)
{
DeviceState *dev = qdev_new(TYPE_SIFIVE_PLIC);
- int i, j = 0;
+ int i;
SiFivePLICState *plic;
assert(enable_stride == (enable_stride & -enable_stride));
@@ -451,18 +451,17 @@ DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
plic = SIFIVE_PLIC(dev);
- for (i = 0; i < num_harts; i++) {
- CPUState *cpu = qemu_get_cpu(hartid_base + i);
- if (plic->addr_config[j].mode == PLICMode_M) {
- j++;
- qdev_connect_gpio_out(dev, num_harts + i,
+ for (i = 0; i < plic->num_addrs; i++) {
+ int cpu_num = plic->addr_config[i].hartid;
+ CPUState *cpu = qemu_get_cpu(hartid_base + cpu_num);
+
+ if (plic->addr_config[i].mode == PLICMode_M) {
+ qdev_connect_gpio_out(dev, num_harts + cpu_num,
qdev_get_gpio_in(DEVICE(cpu), IRQ_M_EXT));
}
-
- if (plic->addr_config[j].mode == PLICMode_S) {
- j++;
- qdev_connect_gpio_out(dev, i,
+ if (plic->addr_config[i].mode == PLICMode_S) {
+ qdev_connect_gpio_out(dev, cpu_num,
qdev_get_gpio_in(DEVICE(cpu), IRQ_S_EXT));
}
}
--
2.35.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] hw/intc: sifive_plic: Avoid overflowing the addr_config buffer
2022-06-01 1:36 [PATCH] hw/intc: sifive_plic: Avoid overflowing the addr_config buffer Alistair Francis
@ 2022-06-01 3:11 ` limingwang (A)
2022-06-01 13:58 ` Philippe Mathieu-Daudé
2022-06-02 1:05 ` Alistair Francis
2 siblings, 0 replies; 6+ messages in thread
From: limingwang (A) via @ 2022-06-01 3:11 UTC (permalink / raw)
To: Alistair Francis, Jiangyifei, qemu-riscv, qemu-devel
Cc: thuth, Bin Meng, alistair23, Alistair Francis, Palmer Dabbelt,
bmeng.cn, Alistair Francis
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Since commit ad40be27 "target/riscv: Support start kernel directly by KVM" we
> have been overflowing the addr_config on "M,MS..."
> configurations, as reported https://gitlab.com/qemu-project/qemu/-/issues/1050.
>
> This commit changes the loop in sifive_plic_create() from iterating over the number
> of harts to just iterating over the addr_config. The addr_config is based on the
> hart_config, and will contain interrup details for all harts. This way we can't iterate
> past the end of addr_config.
>
> Fixes: ad40be27084536 ("target/riscv: Support start kernel directly by KVM")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1050
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Mingwang Li <limingwang@huawei.com>
Mingwang
> ---
> hw/intc/sifive_plic.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c index
> eebbcf33d4..56d60e9ac9 100644
> --- a/hw/intc/sifive_plic.c
> +++ b/hw/intc/sifive_plic.c
> @@ -431,7 +431,7 @@ DeviceState *sifive_plic_create(hwaddr addr, char
> *hart_config,
> uint32_t context_stride, uint32_t aperture_size) {
> DeviceState *dev = qdev_new(TYPE_SIFIVE_PLIC);
> - int i, j = 0;
> + int i;
> SiFivePLICState *plic;
>
> assert(enable_stride == (enable_stride & -enable_stride)); @@ -451,18
> +451,17 @@ DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
> sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
>
> plic = SIFIVE_PLIC(dev);
> - for (i = 0; i < num_harts; i++) {
> - CPUState *cpu = qemu_get_cpu(hartid_base + i);
>
> - if (plic->addr_config[j].mode == PLICMode_M) {
> - j++;
> - qdev_connect_gpio_out(dev, num_harts + i,
> + for (i = 0; i < plic->num_addrs; i++) {
> + int cpu_num = plic->addr_config[i].hartid;
> + CPUState *cpu = qemu_get_cpu(hartid_base + cpu_num);
> +
> + if (plic->addr_config[i].mode == PLICMode_M) {
> + qdev_connect_gpio_out(dev, num_harts + cpu_num,
> qdev_get_gpio_in(DEVICE(cpu),
> IRQ_M_EXT));
> }
> -
> - if (plic->addr_config[j].mode == PLICMode_S) {
> - j++;
> - qdev_connect_gpio_out(dev, i,
> + if (plic->addr_config[i].mode == PLICMode_S) {
> + qdev_connect_gpio_out(dev, cpu_num,
> qdev_get_gpio_in(DEVICE(cpu),
> IRQ_S_EXT));
> }
> }
> --
> 2.35.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] hw/intc: sifive_plic: Avoid overflowing the addr_config buffer
@ 2022-06-01 3:11 ` limingwang (A)
0 siblings, 0 replies; 6+ messages in thread
From: limingwang (A) @ 2022-06-01 3:11 UTC (permalink / raw)
To: Alistair Francis, Jiangyifei, qemu-riscv, qemu-devel
Cc: thuth, Bin Meng, alistair23, Alistair Francis, Palmer Dabbelt,
bmeng.cn, Alistair Francis
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Since commit ad40be27 "target/riscv: Support start kernel directly by KVM" we
> have been overflowing the addr_config on "M,MS..."
> configurations, as reported https://gitlab.com/qemu-project/qemu/-/issues/1050.
>
> This commit changes the loop in sifive_plic_create() from iterating over the number
> of harts to just iterating over the addr_config. The addr_config is based on the
> hart_config, and will contain interrup details for all harts. This way we can't iterate
> past the end of addr_config.
>
> Fixes: ad40be27084536 ("target/riscv: Support start kernel directly by KVM")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1050
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Mingwang Li <limingwang@huawei.com>
Mingwang
> ---
> hw/intc/sifive_plic.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c index
> eebbcf33d4..56d60e9ac9 100644
> --- a/hw/intc/sifive_plic.c
> +++ b/hw/intc/sifive_plic.c
> @@ -431,7 +431,7 @@ DeviceState *sifive_plic_create(hwaddr addr, char
> *hart_config,
> uint32_t context_stride, uint32_t aperture_size) {
> DeviceState *dev = qdev_new(TYPE_SIFIVE_PLIC);
> - int i, j = 0;
> + int i;
> SiFivePLICState *plic;
>
> assert(enable_stride == (enable_stride & -enable_stride)); @@ -451,18
> +451,17 @@ DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
> sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
>
> plic = SIFIVE_PLIC(dev);
> - for (i = 0; i < num_harts; i++) {
> - CPUState *cpu = qemu_get_cpu(hartid_base + i);
>
> - if (plic->addr_config[j].mode == PLICMode_M) {
> - j++;
> - qdev_connect_gpio_out(dev, num_harts + i,
> + for (i = 0; i < plic->num_addrs; i++) {
> + int cpu_num = plic->addr_config[i].hartid;
> + CPUState *cpu = qemu_get_cpu(hartid_base + cpu_num);
> +
> + if (plic->addr_config[i].mode == PLICMode_M) {
> + qdev_connect_gpio_out(dev, num_harts + cpu_num,
> qdev_get_gpio_in(DEVICE(cpu),
> IRQ_M_EXT));
> }
> -
> - if (plic->addr_config[j].mode == PLICMode_S) {
> - j++;
> - qdev_connect_gpio_out(dev, i,
> + if (plic->addr_config[i].mode == PLICMode_S) {
> + qdev_connect_gpio_out(dev, cpu_num,
> qdev_get_gpio_in(DEVICE(cpu),
> IRQ_S_EXT));
> }
> }
> --
> 2.35.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/intc: sifive_plic: Avoid overflowing the addr_config buffer
2022-06-01 1:36 [PATCH] hw/intc: sifive_plic: Avoid overflowing the addr_config buffer Alistair Francis
@ 2022-06-01 13:58 ` Philippe Mathieu-Daudé
2022-06-01 13:58 ` Philippe Mathieu-Daudé
2022-06-02 1:05 ` Alistair Francis
2 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-06-01 13:58 UTC (permalink / raw)
To: Alistair Francis, jiangyifei, limingwang, qemu-riscv, qemu-devel
Cc: thuth, Bin Meng, alistair23, Alistair Francis, Palmer Dabbelt, bmeng.cn
On 1/6/22 03:36, Alistair Francis wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Since commit ad40be27 "target/riscv: Support start kernel directly by
> KVM" we have been overflowing the addr_config on "M,MS..."
> configurations, as reported https://gitlab.com/qemu-project/qemu/-/issues/1050.
>
> This commit changes the loop in sifive_plic_create() from iterating over
> the number of harts to just iterating over the addr_config. The
> addr_config is based on the hart_config, and will contain interrup details
> for all harts. This way we can't iterate past the end of addr_config.
>
> Fixes: ad40be27084536 ("target/riscv: Support start kernel directly by KVM")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1050
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> hw/intc/sifive_plic.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> index eebbcf33d4..56d60e9ac9 100644
> --- a/hw/intc/sifive_plic.c
> +++ b/hw/intc/sifive_plic.c
> @@ -431,7 +431,7 @@ DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
> uint32_t context_stride, uint32_t aperture_size)
> {
> DeviceState *dev = qdev_new(TYPE_SIFIVE_PLIC);
> - int i, j = 0;
> + int i;
> SiFivePLICState *plic;
>
> assert(enable_stride == (enable_stride & -enable_stride));
> @@ -451,18 +451,17 @@ DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
> sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
>
> plic = SIFIVE_PLIC(dev);
> - for (i = 0; i < num_harts; i++) {
> - CPUState *cpu = qemu_get_cpu(hartid_base + i);
>
> - if (plic->addr_config[j].mode == PLICMode_M) {
> - j++;
> - qdev_connect_gpio_out(dev, num_harts + i,
> + for (i = 0; i < plic->num_addrs; i++) {
> + int cpu_num = plic->addr_config[i].hartid;
> + CPUState *cpu = qemu_get_cpu(hartid_base + cpu_num);
> +
> + if (plic->addr_config[i].mode == PLICMode_M) {
> + qdev_connect_gpio_out(dev, num_harts + cpu_num,
> qdev_get_gpio_in(DEVICE(cpu), IRQ_M_EXT));
> }
> -
> - if (plic->addr_config[j].mode == PLICMode_S) {
> - j++;
> - qdev_connect_gpio_out(dev, i,
> + if (plic->addr_config[i].mode == PLICMode_S) {
> + qdev_connect_gpio_out(dev, cpu_num,
> qdev_get_gpio_in(DEVICE(cpu), IRQ_S_EXT));
> }
> }
The logic is much easier to follow now, thanks.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/intc: sifive_plic: Avoid overflowing the addr_config buffer
@ 2022-06-01 13:58 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-06-01 13:58 UTC (permalink / raw)
To: Alistair Francis, jiangyifei, limingwang, qemu-riscv, qemu-devel
Cc: thuth, Bin Meng, alistair23, Alistair Francis, Palmer Dabbelt, bmeng.cn
On 1/6/22 03:36, Alistair Francis wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Since commit ad40be27 "target/riscv: Support start kernel directly by
> KVM" we have been overflowing the addr_config on "M,MS..."
> configurations, as reported https://gitlab.com/qemu-project/qemu/-/issues/1050.
>
> This commit changes the loop in sifive_plic_create() from iterating over
> the number of harts to just iterating over the addr_config. The
> addr_config is based on the hart_config, and will contain interrup details
> for all harts. This way we can't iterate past the end of addr_config.
>
> Fixes: ad40be27084536 ("target/riscv: Support start kernel directly by KVM")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1050
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> hw/intc/sifive_plic.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> index eebbcf33d4..56d60e9ac9 100644
> --- a/hw/intc/sifive_plic.c
> +++ b/hw/intc/sifive_plic.c
> @@ -431,7 +431,7 @@ DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
> uint32_t context_stride, uint32_t aperture_size)
> {
> DeviceState *dev = qdev_new(TYPE_SIFIVE_PLIC);
> - int i, j = 0;
> + int i;
> SiFivePLICState *plic;
>
> assert(enable_stride == (enable_stride & -enable_stride));
> @@ -451,18 +451,17 @@ DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
> sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
>
> plic = SIFIVE_PLIC(dev);
> - for (i = 0; i < num_harts; i++) {
> - CPUState *cpu = qemu_get_cpu(hartid_base + i);
>
> - if (plic->addr_config[j].mode == PLICMode_M) {
> - j++;
> - qdev_connect_gpio_out(dev, num_harts + i,
> + for (i = 0; i < plic->num_addrs; i++) {
> + int cpu_num = plic->addr_config[i].hartid;
> + CPUState *cpu = qemu_get_cpu(hartid_base + cpu_num);
> +
> + if (plic->addr_config[i].mode == PLICMode_M) {
> + qdev_connect_gpio_out(dev, num_harts + cpu_num,
> qdev_get_gpio_in(DEVICE(cpu), IRQ_M_EXT));
> }
> -
> - if (plic->addr_config[j].mode == PLICMode_S) {
> - j++;
> - qdev_connect_gpio_out(dev, i,
> + if (plic->addr_config[i].mode == PLICMode_S) {
> + qdev_connect_gpio_out(dev, cpu_num,
> qdev_get_gpio_in(DEVICE(cpu), IRQ_S_EXT));
> }
> }
The logic is much easier to follow now, thanks.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/intc: sifive_plic: Avoid overflowing the addr_config buffer
2022-06-01 1:36 [PATCH] hw/intc: sifive_plic: Avoid overflowing the addr_config buffer Alistair Francis
2022-06-01 3:11 ` limingwang (A)
2022-06-01 13:58 ` Philippe Mathieu-Daudé
@ 2022-06-02 1:05 ` Alistair Francis
2 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2022-06-02 1:05 UTC (permalink / raw)
To: Alistair Francis
Cc: Jiangyifei, limingwang (A),
open list:RISC-V, qemu-devel@nongnu.org Developers, Thomas Huth,
Bin Meng, Alistair Francis, Palmer Dabbelt, Bin Meng
On Wed, Jun 1, 2022 at 11:36 AM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Since commit ad40be27 "target/riscv: Support start kernel directly by
> KVM" we have been overflowing the addr_config on "M,MS..."
> configurations, as reported https://gitlab.com/qemu-project/qemu/-/issues/1050.
>
> This commit changes the loop in sifive_plic_create() from iterating over
> the number of harts to just iterating over the addr_config. The
> addr_config is based on the hart_config, and will contain interrup details
> for all harts. This way we can't iterate past the end of addr_config.
>
> Fixes: ad40be27084536 ("target/riscv: Support start kernel directly by KVM")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1050
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> hw/intc/sifive_plic.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> index eebbcf33d4..56d60e9ac9 100644
> --- a/hw/intc/sifive_plic.c
> +++ b/hw/intc/sifive_plic.c
> @@ -431,7 +431,7 @@ DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
> uint32_t context_stride, uint32_t aperture_size)
> {
> DeviceState *dev = qdev_new(TYPE_SIFIVE_PLIC);
> - int i, j = 0;
> + int i;
> SiFivePLICState *plic;
>
> assert(enable_stride == (enable_stride & -enable_stride));
> @@ -451,18 +451,17 @@ DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
> sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
>
> plic = SIFIVE_PLIC(dev);
> - for (i = 0; i < num_harts; i++) {
> - CPUState *cpu = qemu_get_cpu(hartid_base + i);
>
> - if (plic->addr_config[j].mode == PLICMode_M) {
> - j++;
> - qdev_connect_gpio_out(dev, num_harts + i,
> + for (i = 0; i < plic->num_addrs; i++) {
> + int cpu_num = plic->addr_config[i].hartid;
> + CPUState *cpu = qemu_get_cpu(hartid_base + cpu_num);
> +
> + if (plic->addr_config[i].mode == PLICMode_M) {
> + qdev_connect_gpio_out(dev, num_harts + cpu_num,
> qdev_get_gpio_in(DEVICE(cpu), IRQ_M_EXT));
> }
> -
> - if (plic->addr_config[j].mode == PLICMode_S) {
> - j++;
> - qdev_connect_gpio_out(dev, i,
> + if (plic->addr_config[i].mode == PLICMode_S) {
> + qdev_connect_gpio_out(dev, cpu_num,
> qdev_get_gpio_in(DEVICE(cpu), IRQ_S_EXT));
> }
> }
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-06-02 1:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-01 1:36 [PATCH] hw/intc: sifive_plic: Avoid overflowing the addr_config buffer Alistair Francis
2022-06-01 3:11 ` limingwang (A) via
2022-06-01 3:11 ` limingwang (A)
2022-06-01 13:58 ` Philippe Mathieu-Daudé via
2022-06-01 13:58 ` Philippe Mathieu-Daudé
2022-06-02 1:05 ` Alistair Francis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.