hw/intc/allwinner-a10-pic.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
The Allwinner PIC model uses set_bit() and clear_bit() to update the
values in its irq_pending[] array when an interrupt arrives. However
it is using these functions wrongly: they work on an array of type
'long', and it is passing an array of type 'uint32_t'. Because the
code manually figures out the right array element, this works on
little-endian hosts and on 32-bit big-endian hosts, where bits 0..31
in a 'long' are in the same place as they are in a 'uint32_t'.
However it breaks on 64-bit big-endian hosts.
Remove the use of set_bit() and clear_bit() in favour of using
deposit32() on the array element. This fixes a bug where on
big-endian 64-bit hosts the guest kernel would hang early on in
bootup.
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/intc/allwinner-a10-pic.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/hw/intc/allwinner-a10-pic.c b/hw/intc/allwinner-a10-pic.c
index 8cca1248073..4875e68ba6a 100644
--- a/hw/intc/allwinner-a10-pic.c
+++ b/hw/intc/allwinner-a10-pic.c
@@ -49,12 +49,9 @@ static void aw_a10_pic_update(AwA10PICState *s)
static void aw_a10_pic_set_irq(void *opaque, int irq, int level)
{
AwA10PICState *s = opaque;
+ uint32_t *pending_reg = &s->irq_pending[irq / 32];
- if (level) {
- set_bit(irq % 32, (void *)&s->irq_pending[irq / 32]);
- } else {
- clear_bit(irq % 32, (void *)&s->irq_pending[irq / 32]);
- }
+ *pending_reg = deposit32(*pending_reg, irq % 32, 1, level);
aw_a10_pic_update(s);
}
--
2.34.1
On Mon, 24 Apr 2023 at 16:28, Peter Maydell <peter.maydell@linaro.org> wrote: > > The Allwinner PIC model uses set_bit() and clear_bit() to update the > values in its irq_pending[] array when an interrupt arrives. However > it is using these functions wrongly: they work on an array of type > 'long', and it is passing an array of type 'uint32_t'. Because the > code manually figures out the right array element, this works on > little-endian hosts and on 32-bit big-endian hosts, where bits 0..31 > in a 'long' are in the same place as they are in a 'uint32_t'. > However it breaks on 64-bit big-endian hosts. > > Remove the use of set_bit() and clear_bit() in favour of using > deposit32() on the array element. This fixes a bug where on > big-endian 64-bit hosts the guest kernel would hang early on in > bootup. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Applied to target-arm.next, thanks. -- PMM
On 24/4/23 17:28, Peter Maydell wrote: > The Allwinner PIC model uses set_bit() and clear_bit() to update the > values in its irq_pending[] array when an interrupt arrives. However > it is using these functions wrongly: they work on an array of type > 'long', and it is passing an array of type 'uint32_t'. Because the > code manually figures out the right array element, this works on > little-endian hosts and on 32-bit big-endian hosts, where bits 0..31 > in a 'long' are in the same place as they are in a 'uint32_t'. > However it breaks on 64-bit big-endian hosts. > > Remove the use of set_bit() and clear_bit() in favour of using > deposit32() on the array element. This fixes a bug where on > big-endian 64-bit hosts the guest kernel would hang early on in > bootup. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/intc/allwinner-a10-pic.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On 24/04/2023 17.28, Peter Maydell wrote: > The Allwinner PIC model uses set_bit() and clear_bit() to update the > values in its irq_pending[] array when an interrupt arrives. However > it is using these functions wrongly: they work on an array of type > 'long', and it is passing an array of type 'uint32_t'. Because the > code manually figures out the right array element, this works on > little-endian hosts and on 32-bit big-endian hosts, where bits 0..31 > in a 'long' are in the same place as they are in a 'uint32_t'. > However it breaks on 64-bit big-endian hosts. > > Remove the use of set_bit() and clear_bit() in favour of using > deposit32() on the array element. This fixes a bug where on > big-endian 64-bit hosts the guest kernel would hang early on in > bootup. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/intc/allwinner-a10-pic.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/hw/intc/allwinner-a10-pic.c b/hw/intc/allwinner-a10-pic.c > index 8cca1248073..4875e68ba6a 100644 > --- a/hw/intc/allwinner-a10-pic.c > +++ b/hw/intc/allwinner-a10-pic.c > @@ -49,12 +49,9 @@ static void aw_a10_pic_update(AwA10PICState *s) > static void aw_a10_pic_set_irq(void *opaque, int irq, int level) > { > AwA10PICState *s = opaque; > + uint32_t *pending_reg = &s->irq_pending[irq / 32]; > > - if (level) { > - set_bit(irq % 32, (void *)&s->irq_pending[irq / 32]); > - } else { > - clear_bit(irq % 32, (void *)&s->irq_pending[irq / 32]); > - } > + *pending_reg = deposit32(*pending_reg, irq % 32, 1, level); > aw_a10_pic_update(s); > } > Reviewed-by: Thomas Huth <thuth@redhat.com>
© 2016 - 2024 Red Hat, Inc.