diff mbox series

[PULL,31/35] hw/intc/allwinner-a10-pic: Don't use set_bit()/clear_bit()

Message ID 20230502121459.2422303-32-peter.maydell@linaro.org
State Accepted
Commit 2c5fa0778c3b4307f9f3af7f27886c46d129c62f
Headers show
Series [PULL,01/35] target/arm: Move cortex sysregs into a separate file | expand

Commit Message

Peter Maydell May 2, 2023, 12:14 p.m. UTC 
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>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20230424152833.1334136-1-peter.maydell@linaro.org
---
 hw/intc/allwinner-a10-pic.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Guenter Roeck June 3, 2023, 3:03 p.m. UTC | #1 
Hi,

On Tue, May 02, 2023 at 01:14:55PM +0100, 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>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-id: 20230424152833.1334136-1-peter.maydell@linaro.org

In v8.0.2, the cubieboard emulation running Linux crashes during reboot
with a hung task error. Tested with mainline Linux (v6.4-rc4-78-g929ed21dfdb6)
and with v5.15.114. Host is AMD Ryzen 5900X.

Requesting system reboot
[   61.927460] INFO: task kworker/0:1:13 blocked for more than 30 seconds.
[   61.927896]       Not tainted 5.15.115-rc2-00038-g31e35d9f1b8d #1
[   61.928144] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[   61.928419] task:kworker/0:1     state:D stack:    0 pid:   13 ppid:     2 flags:0x00000000
[   61.928972] Workqueue: events_freezable mmc_rescan
[   61.929739] [<c13734f0>] (__schedule) from [<c1373c98>] (schedule+0x80/0x15c)
[   61.930041] [<c1373c98>] (schedule) from [<c137ad64>] (schedule_timeout+0xd4/0x12c)
[   61.930270] [<c137ad64>] (schedule_timeout) from [<c137477c>] (do_wait_for_common+0xa0/0x154)
[   61.930523] [<c137477c>] (do_wait_for_common) from [<c1374870>] (wait_for_completion+0x40/0x4c)
[   61.930764] [<c1374870>] (wait_for_completion) from [<c1044cd0>] (mmc_wait_for_req_done+0x6c/0x90)
[   61.931012] [<c1044cd0>] (mmc_wait_for_req_done) from [<c1044e34>] (mmc_wait_for_cmd+0x70/0xa8)
[   61.931252] [<c1044e34>] (mmc_wait_for_cmd) from [<c10512a0>] (sdio_reset+0x58/0x124)
[   61.931478] [<c10512a0>] (sdio_reset) from [<c1046328>] (mmc_rescan+0x294/0x30c)
[   61.931692] [<c1046328>] (mmc_rescan) from [<c036be10>] (process_one_work+0x28c/0x720)
[   61.931924] [<c036be10>] (process_one_work) from [<c036c308>] (worker_thread+0x64/0x53c)
[   61.932153] [<c036c308>] (worker_thread) from [<c03753e0>] (kthread+0x15c/0x180)
[   61.932365] [<c03753e0>] (kthread) from [<c030015c>] (ret_from_fork+0x14/0x38)
[   61.932628] Exception stack(0xc31ddfb0 to 0xc31ddff8)

This was not seen with v8.0.0. Bisect points to this patch. Reverting it
fixes the problem.

Bisect log is attached.

Guenter

---
# bad: [f7f686b61cf7ee142c9264d2e04ac2c6a96d37f8] Update version for 8.0.2 release
# good: [c1eb2ddf0f8075faddc5f7c3d39feae3e8e9d6b4] Update version for v8.0.0 release
git bisect start 'v8.0.2' 'v8.0.0'
# bad: [21b54a683d14c0c6f9af35536d9059c60b7449ca] s390x/pv: Fix spurious warning with asynchronous teardown
git bisect bad 21b54a683d14c0c6f9af35536d9059c60b7449ca
# bad: [4dc5df865c482c6e8894964c7f300fa556c3b78e] softfloat: Fix the incorrect computation in float32_exp2
git bisect bad 4dc5df865c482c6e8894964c7f300fa556c3b78e
# good: [f0c5a780292bd405bbce818b63757313cafcf262] target/arm: Initialize debug capabilities only once
git bisect good f0c5a780292bd405bbce818b63757313cafcf262
# bad: [af08c70ef5204fedb2b974fbecaf65e1b6cc0a2f] hw/intc/allwinner-a10-pic: Don't use set_bit()/clear_bit()
git bisect bad af08c70ef5204fedb2b974fbecaf65e1b6cc0a2f
# good: [168f193c5be54fc9a6d725dbb9974c0d2815792a] hw/arm/boot: Make write_bootloader() public as arm_write_bootloader()
git bisect good 168f193c5be54fc9a6d725dbb9974c0d2815792a
# good: [975f12aa528d6cab5cc41efebaf05d7eb7296d94] hw/arm/raspi: Use arm_write_bootloader() to write boot code
git bisect good 975f12aa528d6cab5cc41efebaf05d7eb7296d94
# first bad commit: [af08c70ef5204fedb2b974fbecaf65e1b6cc0a2f] hw/intc/allwinner-a10-pic: Don't use set_bit()/clear_bit()
Michael Tokarev June 3, 2023, 5:46 p.m. UTC | #2 
03.06.2023 18:03, Guenter Roeck wrote:
> Hi,
> 
> On Tue, May 02, 2023 at 01:14:55PM +0100, 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>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Message-id: 20230424152833.1334136-1-peter.maydell@linaro.org
> 
> In v8.0.2, the cubieboard emulation running Linux crashes during reboot
> with a hung task error. Tested with mainline Linux (v6.4-rc4-78-g929ed21dfdb6)
> and with v5.15.114. Host is AMD Ryzen 5900X.
> 
> Requesting system reboot
> [   61.927460] INFO: task kworker/0:1:13 blocked for more than 30 seconds.
> [   61.927896]       Not tainted 5.15.115-rc2-00038-g31e35d9f1b8d #1
> [   61.928144] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [   61.928419] task:kworker/0:1     state:D stack:    0 pid:   13 ppid:     2 flags:0x00000000
> [   61.928972] Workqueue: events_freezable mmc_rescan
> [   61.929739] [<c13734f0>] (__schedule) from [<c1373c98>] (schedule+0x80/0x15c)
> [   61.930041] [<c1373c98>] (schedule) from [<c137ad64>] (schedule_timeout+0xd4/0x12c)
> [   61.930270] [<c137ad64>] (schedule_timeout) from [<c137477c>] (do_wait_for_common+0xa0/0x154)
> [   61.930523] [<c137477c>] (do_wait_for_common) from [<c1374870>] (wait_for_completion+0x40/0x4c)
> [   61.930764] [<c1374870>] (wait_for_completion) from [<c1044cd0>] (mmc_wait_for_req_done+0x6c/0x90)
> [   61.931012] [<c1044cd0>] (mmc_wait_for_req_done) from [<c1044e34>] (mmc_wait_for_cmd+0x70/0xa8)
> [   61.931252] [<c1044e34>] (mmc_wait_for_cmd) from [<c10512a0>] (sdio_reset+0x58/0x124)
> [   61.931478] [<c10512a0>] (sdio_reset) from [<c1046328>] (mmc_rescan+0x294/0x30c)
> [   61.931692] [<c1046328>] (mmc_rescan) from [<c036be10>] (process_one_work+0x28c/0x720)
> [   61.931924] [<c036be10>] (process_one_work) from [<c036c308>] (worker_thread+0x64/0x53c)
> [   61.932153] [<c036c308>] (worker_thread) from [<c03753e0>] (kthread+0x15c/0x180)
> [   61.932365] [<c03753e0>] (kthread) from [<c030015c>] (ret_from_fork+0x14/0x38)
> [   61.932628] Exception stack(0xc31ddfb0 to 0xc31ddff8)
> 
> This was not seen with v8.0.0. Bisect points to this patch. Reverting it
> fixes the problem.

Does this happen on master too, or just on stable-8.0 ?

Thanks,

/mjt
Guenter Roeck June 3, 2023, 6:06 p.m. UTC | #3 
On 6/3/23 10:46, Michael Tokarev wrote:
> 03.06.2023 18:03, Guenter Roeck wrote:
>> Hi,
>>
>> On Tue, May 02, 2023 at 01:14:55PM +0100, 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>
>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Message-id: 20230424152833.1334136-1-peter.maydell@linaro.org
>>
>> In v8.0.2, the cubieboard emulation running Linux crashes during reboot
>> with a hung task error. Tested with mainline Linux (v6.4-rc4-78-g929ed21dfdb6)
>> and with v5.15.114. Host is AMD Ryzen 5900X.
>>
>> Requesting system reboot
>> [   61.927460] INFO: task kworker/0:1:13 blocked for more than 30 seconds.
>> [   61.927896]       Not tainted 5.15.115-rc2-00038-g31e35d9f1b8d #1
>> [   61.928144] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> [   61.928419] task:kworker/0:1     state:D stack:    0 pid:   13 ppid:     2 flags:0x00000000
>> [   61.928972] Workqueue: events_freezable mmc_rescan
>> [   61.929739] [<c13734f0>] (__schedule) from [<c1373c98>] (schedule+0x80/0x15c)
>> [   61.930041] [<c1373c98>] (schedule) from [<c137ad64>] (schedule_timeout+0xd4/0x12c)
>> [   61.930270] [<c137ad64>] (schedule_timeout) from [<c137477c>] (do_wait_for_common+0xa0/0x154)
>> [   61.930523] [<c137477c>] (do_wait_for_common) from [<c1374870>] (wait_for_completion+0x40/0x4c)
>> [   61.930764] [<c1374870>] (wait_for_completion) from [<c1044cd0>] (mmc_wait_for_req_done+0x6c/0x90)
>> [   61.931012] [<c1044cd0>] (mmc_wait_for_req_done) from [<c1044e34>] (mmc_wait_for_cmd+0x70/0xa8)
>> [   61.931252] [<c1044e34>] (mmc_wait_for_cmd) from [<c10512a0>] (sdio_reset+0x58/0x124)
>> [   61.931478] [<c10512a0>] (sdio_reset) from [<c1046328>] (mmc_rescan+0x294/0x30c)
>> [   61.931692] [<c1046328>] (mmc_rescan) from [<c036be10>] (process_one_work+0x28c/0x720)
>> [   61.931924] [<c036be10>] (process_one_work) from [<c036c308>] (worker_thread+0x64/0x53c)
>> [   61.932153] [<c036c308>] (worker_thread) from [<c03753e0>] (kthread+0x15c/0x180)
>> [   61.932365] [<c03753e0>] (kthread) from [<c030015c>] (ret_from_fork+0x14/0x38)
>> [   61.932628] Exception stack(0xc31ddfb0 to 0xc31ddff8)
>>
>> This was not seen with v8.0.0. Bisect points to this patch. Reverting it
>> fixes the problem.
> 
> Does this happen on master too, or just on stable-8.0 ?
> 

It does. Tested with v8.0.0-1542-g848a6caa88.

Here is my command line in case you want to give it a try:

qemu-system-arm -M cubieboard -kernel arch/arm/boot/zImage -no-reboot \
     -initrd rootfs-armv5.cpio -m 512 \
     --append "panic=-1 rdinit=/sbin/init earlycon=uart8250,mmio32,0x1c28000,115200n8 console=ttyS0" \
     -dtb arch/arm/boot/dts/sun4i-a10-cubieboard.dtb -nographic \
     -monitor null -serial stdio

initrd is https://github.com/groeck/linux-build-test/blob/master/rootfs/arm-v7/rootfs-armv5.cpio.gz

This is with multi_v7_defconfig with some debug options added. If necessary
I'll be happy to provide the exact configuration.

Guenter
Peter Maydell June 5, 2023, 9:40 a.m. UTC | #4 
On Sat, 3 Jun 2023 at 19:06, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 6/3/23 10:46, Michael Tokarev wrote:
> > 03.06.2023 18:03, Guenter Roeck wrote:
> >> Hi,
> >>
> >> On Tue, May 02, 2023 at 01:14:55PM +0100, 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>
> >>> Reviewed-by: Thomas Huth <thuth@redhat.com>
> >>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >>> Message-id: 20230424152833.1334136-1-peter.maydell@linaro.org
> >>
> >> In v8.0.2, the cubieboard emulation running Linux crashes during reboot
> >> with a hung task error. Tested with mainline Linux (v6.4-rc4-78-g929ed21dfdb6)
> >> and with v5.15.114. Host is AMD Ryzen 5900X.
> >>
> >> Requesting system reboot
> >> [   61.927460] INFO: task kworker/0:1:13 blocked for more than 30 seconds.
> >> [   61.927896]       Not tainted 5.15.115-rc2-00038-g31e35d9f1b8d #1
> >> [   61.928144] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> >> [   61.928419] task:kworker/0:1     state:D stack:    0 pid:   13 ppid:     2 flags:0x00000000
> >> [   61.928972] Workqueue: events_freezable mmc_rescan
> >> [   61.929739] [<c13734f0>] (__schedule) from [<c1373c98>] (schedule+0x80/0x15c)
> >> [   61.930041] [<c1373c98>] (schedule) from [<c137ad64>] (schedule_timeout+0xd4/0x12c)
> >> [   61.930270] [<c137ad64>] (schedule_timeout) from [<c137477c>] (do_wait_for_common+0xa0/0x154)
> >> [   61.930523] [<c137477c>] (do_wait_for_common) from [<c1374870>] (wait_for_completion+0x40/0x4c)
> >> [   61.930764] [<c1374870>] (wait_for_completion) from [<c1044cd0>] (mmc_wait_for_req_done+0x6c/0x90)
> >> [   61.931012] [<c1044cd0>] (mmc_wait_for_req_done) from [<c1044e34>] (mmc_wait_for_cmd+0x70/0xa8)
> >> [   61.931252] [<c1044e34>] (mmc_wait_for_cmd) from [<c10512a0>] (sdio_reset+0x58/0x124)
> >> [   61.931478] [<c10512a0>] (sdio_reset) from [<c1046328>] (mmc_rescan+0x294/0x30c)
> >> [   61.931692] [<c1046328>] (mmc_rescan) from [<c036be10>] (process_one_work+0x28c/0x720)
> >> [   61.931924] [<c036be10>] (process_one_work) from [<c036c308>] (worker_thread+0x64/0x53c)
> >> [   61.932153] [<c036c308>] (worker_thread) from [<c03753e0>] (kthread+0x15c/0x180)
> >> [   61.932365] [<c03753e0>] (kthread) from [<c030015c>] (ret_from_fork+0x14/0x38)
> >> [   61.932628] Exception stack(0xc31ddfb0 to 0xc31ddff8)
> >>
> >> This was not seen with v8.0.0. Bisect points to this patch. Reverting it
> >> fixes the problem.
> >
> > Does this happen on master too, or just on stable-8.0 ?
> >
>
> It does. Tested with v8.0.0-1542-g848a6caa88.
>
> Here is my command line in case you want to give it a try:
>
> qemu-system-arm -M cubieboard -kernel arch/arm/boot/zImage -no-reboot \
>      -initrd rootfs-armv5.cpio -m 512 \
>      --append "panic=-1 rdinit=/sbin/init earlycon=uart8250,mmio32,0x1c28000,115200n8 console=ttyS0" \
>      -dtb arch/arm/boot/dts/sun4i-a10-cubieboard.dtb -nographic \
>      -monitor null -serial stdio
>
> initrd is https://github.com/groeck/linux-build-test/blob/master/rootfs/arm-v7/rootfs-armv5.cpio.gz
>
> This is with multi_v7_defconfig with some debug options added. If necessary
> I'll be happy to provide the exact configuration.

If you can provide a link to the zImage and the dtb to reproduce
as well, that would be helpful.

thanks
-- PMM
Guenter Roeck June 5, 2023, 1:35 p.m. UTC | #5 
On 6/5/23 02:40, Peter Maydell wrote:
> On Sat, 3 Jun 2023 at 19:06, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 6/3/23 10:46, Michael Tokarev wrote:
>>> 03.06.2023 18:03, Guenter Roeck wrote:
>>>> Hi,
>>>>
>>>> On Tue, May 02, 2023 at 01:14:55PM +0100, 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>
>>>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> Message-id: 20230424152833.1334136-1-peter.maydell@linaro.org
>>>>
>>>> In v8.0.2, the cubieboard emulation running Linux crashes during reboot
>>>> with a hung task error. Tested with mainline Linux (v6.4-rc4-78-g929ed21dfdb6)
>>>> and with v5.15.114. Host is AMD Ryzen 5900X.
>>>>
>>>> Requesting system reboot
>>>> [   61.927460] INFO: task kworker/0:1:13 blocked for more than 30 seconds.
>>>> [   61.927896]       Not tainted 5.15.115-rc2-00038-g31e35d9f1b8d #1
>>>> [   61.928144] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>>>> [   61.928419] task:kworker/0:1     state:D stack:    0 pid:   13 ppid:     2 flags:0x00000000
>>>> [   61.928972] Workqueue: events_freezable mmc_rescan
>>>> [   61.929739] [<c13734f0>] (__schedule) from [<c1373c98>] (schedule+0x80/0x15c)
>>>> [   61.930041] [<c1373c98>] (schedule) from [<c137ad64>] (schedule_timeout+0xd4/0x12c)
>>>> [   61.930270] [<c137ad64>] (schedule_timeout) from [<c137477c>] (do_wait_for_common+0xa0/0x154)
>>>> [   61.930523] [<c137477c>] (do_wait_for_common) from [<c1374870>] (wait_for_completion+0x40/0x4c)
>>>> [   61.930764] [<c1374870>] (wait_for_completion) from [<c1044cd0>] (mmc_wait_for_req_done+0x6c/0x90)
>>>> [   61.931012] [<c1044cd0>] (mmc_wait_for_req_done) from [<c1044e34>] (mmc_wait_for_cmd+0x70/0xa8)
>>>> [   61.931252] [<c1044e34>] (mmc_wait_for_cmd) from [<c10512a0>] (sdio_reset+0x58/0x124)
>>>> [   61.931478] [<c10512a0>] (sdio_reset) from [<c1046328>] (mmc_rescan+0x294/0x30c)
>>>> [   61.931692] [<c1046328>] (mmc_rescan) from [<c036be10>] (process_one_work+0x28c/0x720)
>>>> [   61.931924] [<c036be10>] (process_one_work) from [<c036c308>] (worker_thread+0x64/0x53c)
>>>> [   61.932153] [<c036c308>] (worker_thread) from [<c03753e0>] (kthread+0x15c/0x180)
>>>> [   61.932365] [<c03753e0>] (kthread) from [<c030015c>] (ret_from_fork+0x14/0x38)
>>>> [   61.932628] Exception stack(0xc31ddfb0 to 0xc31ddff8)
>>>>
>>>> This was not seen with v8.0.0. Bisect points to this patch. Reverting it
>>>> fixes the problem.
>>>
>>> Does this happen on master too, or just on stable-8.0 ?
>>>
>>
>> It does. Tested with v8.0.0-1542-g848a6caa88.
>>
>> Here is my command line in case you want to give it a try:
>>
>> qemu-system-arm -M cubieboard -kernel arch/arm/boot/zImage -no-reboot \
>>       -initrd rootfs-armv5.cpio -m 512 \
>>       --append "panic=-1 rdinit=/sbin/init earlycon=uart8250,mmio32,0x1c28000,115200n8 console=ttyS0" \
>>       -dtb arch/arm/boot/dts/sun4i-a10-cubieboard.dtb -nographic \
>>       -monitor null -serial stdio
>>
>> initrd is https://github.com/groeck/linux-build-test/blob/master/rootfs/arm-v7/rootfs-armv5.cpio.gz
>>
>> This is with multi_v7_defconfig with some debug options added. If necessary
>> I'll be happy to provide the exact configuration.
> 
> If you can provide a link to the zImage and the dtb to reproduce
> as well, that would be helpful.
> 

Please see http://server.roeck-us.net/qemu/arm-v7/.

There are also compiled versions of qemu v8.0.0 and v8.0.2 as well as scripts
to run the test. Note that the initrd will auto-reboot. The cubieboard emulation
does not support board reset, so the test will normally end with

Requesting system reboot
[   22.020700] reboot: Restarting system

In the failure case, the second line is not seen, and there will be a hung task
crash about 30 seconds after the reboot request.

Requesting system reboot
[   61.960821] INFO: task kworker/0:2:67 blocked for more than 30 seconds.
[   61.961406]       Tainted: G                 N 6.4.0-rc5 #1

Hope this helps,
Guenter
Peter Maydell June 6, 2023, 10:33 a.m. UTC | #6 
On Mon, 5 Jun 2023 at 14:35, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 6/5/23 02:40, Peter Maydell wrote:
> > If you can provide a link to the zImage and the dtb to reproduce
> > as well, that would be helpful.
>
>
> Please see http://server.roeck-us.net/qemu/arm-v7/.

Thanks. I've identified the cause of the regression; will send
a patch shortly.

-- PMM
diff mbox series

Patch

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);
 }