[PATCH-for-5.2 0/5] hw/arm: Fix various incorrect IRQ handling

Philippe Mathieu-Daudé posted 5 patches 3 years, 4 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20201107193403.436146-1-f4bug@amsat.org
include/hw/misc/stm32f2xx_syscfg.h |  2 --
hw/arm/armsse.c                    |  3 ++-
hw/arm/musicpal.c                  | 40 +++++++++++++++++++-----------
hw/arm/nseries.c                   | 11 --------
hw/arm/stm32f205_soc.c             |  1 -
hw/misc/stm32f2xx_syscfg.c         |  2 --
hw/arm/Kconfig                     |  1 +
7 files changed, 28 insertions(+), 32 deletions(-)
[PATCH-for-5.2 0/5] hw/arm: Fix various incorrect IRQ handling
Posted by Philippe Mathieu-Daudé 3 years, 4 months ago
This series is inspired by Peter's following patch:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg758178.html

I started to audit the IRQ uses and fixed the easy problems.

Unresolved ones:
- stellaris_init() connects different TYPE_STELLARIS_GPTM
  to the same ADC input (seems some weird kludge)
- platform_bus_link_device() uses sysbus_has_irq() to check
  if an device has IRQ mapped but it doesn't seem to work.

Anyway enough audit for the day.

Regards,

Phil.

Philippe Mathieu-Daudé (5):
  hw/arm/armsse: Correct expansion MPC interrupt lines
  hw/misc/stm32f2xx_syscfg: Remove extraneous IRQ
  hw/arm/nseries: Remove invalid/unnecessary n8x0_uart_setup()
  hw/arm/musicpal: Don't connect two qemu_irqs directly to the same
    input
  hw/arm/musicpal: Only use qdev_get_gpio_in() when necessary

 include/hw/misc/stm32f2xx_syscfg.h |  2 --
 hw/arm/armsse.c                    |  3 ++-
 hw/arm/musicpal.c                  | 40 +++++++++++++++++++-----------
 hw/arm/nseries.c                   | 11 --------
 hw/arm/stm32f205_soc.c             |  1 -
 hw/misc/stm32f2xx_syscfg.c         |  2 --
 hw/arm/Kconfig                     |  1 +
 7 files changed, 28 insertions(+), 32 deletions(-)

-- 
2.26.2

Re: [PATCH-for-5.2 0/5] hw/arm: Fix various incorrect IRQ handling
Posted by Peter Maydell 3 years, 4 months ago
On Sat, 7 Nov 2020 at 19:34, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> This series is inspired by Peter's following patch:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg758178.html
>
> I started to audit the IRQ uses and fixed the easy problems.
>
> Unresolved ones:
> - stellaris_init() connects different TYPE_STELLARIS_GPTM
>   to the same ADC input (seems some weird kludge)
> - platform_bus_link_device() uses sysbus_has_irq() to check
>   if an device has IRQ mapped but it doesn't seem to work.

Were you finding these by inspection, or did you add some
kind of assert or other check for double-irq-line-connections?

Applied to target-arm.next, thanks (deletion of omap_uart_attach()
can be a separate followup cleanup).

-- PMM

Re: [PATCH-for-5.2 0/5] hw/arm: Fix various incorrect IRQ handling
Posted by Philippe Mathieu-Daudé 3 years, 4 months ago
On 11/9/20 3:23 PM, Peter Maydell wrote:
> On Sat, 7 Nov 2020 at 19:34, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> This series is inspired by Peter's following patch:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg758178.html
>>
>> I started to audit the IRQ uses and fixed the easy problems.
>>
>> Unresolved ones:
>> - stellaris_init() connects different TYPE_STELLARIS_GPTM
>>   to the same ADC input (seems some weird kludge)
>> - platform_bus_link_device() uses sysbus_has_irq() to check
>>   if an device has IRQ mapped but it doesn't seem to work.
> 
> Were you finding these by inspection, or did you add some
> kind of assert or other check for double-irq-line-connections?

I was not sure adding a field to IRQState would be accepted,
so I added a trace event in qdev_connect_gpio_out_named():

 trace_qdev_connect_gpio_out(pin, n, name,
                             object_get_typename(OBJECT(dev)));

And lookup up when 'pin' pointer value appears more than 1 time.

For 6.0 I am wondering about replacing --enable-qom-cast-debug
by --maintainer-mode and add a field in IRQState guarded for
that mode. Maybe it is not too bad and we can use it normally.

> 
> Applied to target-arm.next, thanks (deletion of omap_uart_attach()
> can be a separate followup cleanup).

Yes, I have *some* OMAP patches for 6.0 ;)

> 
> -- PMM
>