1. Patchew
  2. QEMU
  3. View series

[PATCH v3 00/18] hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()

Philippe Mathieu-Daudé posted 18 patches 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230302224058.43315-1-philmd@linaro.org
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, "Hervé Poussineau" <hpoussin@reactos.org>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, John Snow <jsnow@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>
hw/audio/adlib.c              |  4 +--
hw/display/qxl.c              |  7 ++--
hw/display/vga.c              |  9 +++--
hw/dma/i82374.c               |  7 ++--
hw/i386/pc_piix.c             | 13 +++++---
hw/ide/ioport.c               | 15 +++++++--
hw/ide/isa.c                  |  2 +-
hw/ide/piix.c                 | 54 +++++++++++++++++++++++-------
hw/isa/isa-bus.c              | 36 ++++++++------------
hw/isa/piix3.c                | 63 +++++++++++++++--------------------
hw/isa/piix4.c                | 12 ++++---
hw/mips/malta.c               |  2 +-
hw/watchdog/wdt_ib700.c       |  4 +--
include/exec/ioport.h         | 15 +++++----
include/hw/ide/internal.h     |  3 +-
include/hw/ide/isa.h          |  3 ++
include/hw/ide/piix.h         |  4 +++
include/hw/isa/isa.h          |  3 +-
include/hw/southbridge/piix.h | 14 ++++----
softmmu/ioport.c              | 48 +++++++++++++++++++-------
softmmu/qdev-monitor.c        |  3 ++
21 files changed, 190 insertions(+), 131 deletions(-)
[PATCH v3 00/18] hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()
Posted by Philippe Mathieu-Daudé 1 year, 1 month ago 
Since v2: rebased

I'm posting this series as it to not block Bernhard's PIIX
cleanup work. I don't have code change planned, but eventually
reword / improve commit descriptions.

Tested commit after commit to be sure it is bisectable. Sadly
this was before Zoltan & Thomas report a problem with commit
bb98e0f59c ("hw/isa/vt82c686: Remove intermediate IRQ forwarder").

Background thread:
https://lore.kernel.org/qemu-devel/5095dffc-309b-6c72-d255-8cdaa6fd3d52@ilande.co.uk/

Philippe Mathieu-Daudé (18):
  hw/ide/piix: Expose output IRQ as properties for late object
    population
  hw/ide/piix: Allow using PIIX3-IDE as standalone PCI function
  hw/i386/pc_piix: Wire PIIX3 IDE ouput IRQs to ISA bus IRQs 14/15
  hw/isa/piix4: Wire PIIX4 IDE ouput IRQs to ISA bus IRQs 14/15
  hw/ide: Rename ISA specific ide_init_ioport -> ide_bus_init_ioport_isa
  hw/ide/piix: Ensure IDE output IRQs are wired at realization
  hw/isa: Deprecate isa_get_irq() in favor of isa_bus_get_irq()
  hw/ide: Introduce generic ide_init_ioport()
  hw/ide/piix: Use generic ide_bus_init_ioport()
  hw/isa: Ensure isa_register_portio_list() do not get NULL ISA device
  hw/isa: Simplify isa_address_space[_io]()
  hw/isa: Reduce 'isabus' singleton scope to isa_bus_new()
  exec/ioport: Factor portio_list_register_flush_coalesced() out
  exec/ioport: Factor portio_list_register() out
  hw/southbridge/piix: Use OBJECT_DECLARE_SIMPLE_TYPE() macro
  hw/isa/piix: Batch register QOM types using DEFINE_TYPES() macro
  hw/isa/piix: Unify QOM type name of PIIX ISA function
  hw/isa/piix: Unify PIIX-ISA QOM type names using qdev aliases

 hw/audio/adlib.c              |  4 +--
 hw/display/qxl.c              |  7 ++--
 hw/display/vga.c              |  9 +++--
 hw/dma/i82374.c               |  7 ++--
 hw/i386/pc_piix.c             | 13 +++++---
 hw/ide/ioport.c               | 15 +++++++--
 hw/ide/isa.c                  |  2 +-
 hw/ide/piix.c                 | 54 +++++++++++++++++++++++-------
 hw/isa/isa-bus.c              | 36 ++++++++------------
 hw/isa/piix3.c                | 63 +++++++++++++++--------------------
 hw/isa/piix4.c                | 12 ++++---
 hw/mips/malta.c               |  2 +-
 hw/watchdog/wdt_ib700.c       |  4 +--
 include/exec/ioport.h         | 15 +++++----
 include/hw/ide/internal.h     |  3 +-
 include/hw/ide/isa.h          |  3 ++
 include/hw/ide/piix.h         |  4 +++
 include/hw/isa/isa.h          |  3 +-
 include/hw/southbridge/piix.h | 14 ++++----
 softmmu/ioport.c              | 48 +++++++++++++++++++-------
 softmmu/qdev-monitor.c        |  3 ++
 21 files changed, 190 insertions(+), 131 deletions(-)

-- 
2.38.1
Re: [PATCH v3 00/18] hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()
Posted by David Woodhouse 1 year, 1 month ago 

On 2 March 2023 22:40:40 GMT, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>Since v2: rebased
>
>I'm posting this series as it to not block Bernhard's PIIX
>cleanup work. I don't have code change planned, but eventually
>reword / improve commit descriptions.
>
>Tested commit after commit to be sure it is bisectable. Sadly
>this was before Zoltan & Thomas report a problem with commit
>bb98e0f59c ("hw/isa/vt82c686: Remove intermediate IRQ forwarder").

However much I stare at the partial revert which fixes it, I just cannot believe that the change could make any difference at all. There's got to be something weird going on there.

I was going to ask if the level mode for the PIT made any difference, but this is the output IRQ from the PIT to the CPU itself so I don't see how it would.

Would like to see a report with tracing from pic_update_irq, the CPU interrupt "handler" and the intermediate IRQ handler. With the intermediate present and without it. To compare the two.
Re: [PATCH v3 00/18] hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()
Posted by BALATON Zoltan 1 year, 1 month ago 
On Fri, 3 Mar 2023, David Woodhouse wrote:
> Would like to see a report with tracing from pic_update_irq, the CPU 
> interrupt "handler" and the intermediate IRQ handler. With the 
> intermediate present and without it. To compare the two.

Here it is witout revert when it hangs after printing:

0.536| Memory used before SYS_Init: 9MB
0.606|
0.606|
0.606| ABox 1.30 (2.7.2018) © 1999-2022 by Ralph Schmidt, Emmanuel Lesueur, Teemu Suikki, Harry Sintonen
1.257| PCI ATA/ATAPI Driver@2: PIO Mode 4
1.257| PCI ATA/ATAPI Driver@2: UDMA Mode 5

U pic_update_irq master 1 imr 248 irr 7 padd 0
   pic_update_irq master 1 imr 248 irr 7 padd 0
D pic_update_irq master 1 imr 248 irr 7 padd 0
   pic_update_irq master 1 imr 248 irr 7 padd 0
M pic_update_irq master 1 imr 248 irr 7 padd 0
   pic_update_irq master 1 imr 248 irr 7 padd 0
A pic_update_irq master 1 imr 248 irr 7 padd 0
   pic_update_irq master 1 imr 248 irr 7 padd 0
   pic_update_irq master 1 imr 248 irr 7 padd 0
   pic_update_irq master 1 imr 248 irr 7 padd 0
M pic_update_irq master 1 imr 248 irr 7 padd 0
   pic_update_irq master 1 imr 248 irr 7 padd 0
o pic_update_irq master 1 imr 248 irr 7 padd 0
   pic_update_irq master 1 imr 248 irr 7 padd 0
d pic_update_irq master 1 imr 248 irr 7 padd 0
   pic_update_irq master 1 imr 248 irr 7 padd 0
e pic_update_irq master 1 imr 248 irr 7 padd 0
   pic_update_irq master 1 imr 248 irr 7 padd 0
   pic_update_irq master 1 imr 248 irr 7 padd 0
   pic_update_irq master 1 imr 248 irr 7 padd 0
5 pic_update_irq master 1 imr 248 irr 7 padd 0
   pic_update_irq master 1 imr 248 irr 7 padd 0

pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0

pic_update_irq master 0 imr 45 irr 16 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 16 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 16 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 16 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 16 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 16 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 16 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 16 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 16 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 16 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 16 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 16 padd 0
[more of the above repeating some more then]
pic_update_irq master 0 imr 45 irr 16 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 16 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 144 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 144 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 0 imr 45 irr 144 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0
pic_update_irq master 1 imr 248 irr 7 padd 0

and there seems to be no more interrupts
with the revert when it boots this should print:

1.513| PCI ATA/ATAPI Driver@2: PIO Mode 4
1.514| PCI ATA/ATAPI Driver@2: UDMA Mode 5
1.517| ide.device@2: QEMU     QEMU DVD-ROM     <CDROM>
1.523| ide.device@2:  CDRom <CD001>,<MORPHOS > found, bootable
1.525| ide.device@2:  Mount <CD0>
1.526| ide.device@2:  Partition <CD0> DosType 0x43444653 BootPri 127

and the trace including the irq forwarder func in vt82c686 isa:

M via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
o via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
d via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
e via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
5 via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0

  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0

  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
pic_update_irq master 0 imr 45 irr 128 padd 0
pic_update_irq master 1 imr 248 irr 4 padd 1

  via_isa_request_i8259_irq: irq=0 level=1
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
pic_update_irq master 0 imr 45 irr 128 padd 0
pic_update_irq master 1 imr 248 irr 4 padd 1
  via_isa_request_i8259_irq: irq=0 level=1
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
pic_update_irq master 0 imr 45 irr 128 padd 0
pic_update_irq master 1 imr 248 irr 4 padd 1
  via_isa_request_i8259_irq: irq=0 level=1
pic_update_irq master 0 imr 45 irr 128 padd 0
pic_update_irq master 1 imr 248 irr 4 padd 1
  via_isa_request_i8259_irq: irq=0 level=1
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
1 via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
. via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
2 via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
8 via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
2 via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
| via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
i via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
d via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
e via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
. via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
d via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
e via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
v via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
i via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
c via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
e via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
@ via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
2 via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
: via_isa_request_i8259_irq: irq=0 level=0
[...]
< via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
C via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
D via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
R via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
O via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
M via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0
> via_isa_request_i8259_irq: irq=0 level=0
   via_isa_request_i8259_irq: irq=0 level=0

  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0

  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
pic_update_irq master 0 imr 45 irr 128 padd 0
pic_update_irq master 1 imr 248 irr 4 padd 1
  via_isa_request_i8259_irq: irq=0 level=1
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
pic_update_irq master 0 imr 45 irr 128 padd 0
pic_update_irq master 1 imr 248 irr 4 padd 1
  via_isa_request_i8259_irq: irq=0 level=1
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
pic_update_irq master 0 imr 45 irr 128 padd 0
pic_update_irq master 1 imr 248 irr 4 padd 1
  via_isa_request_i8259_irq: irq=0 level=1
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0
pic_update_irq master 0 imr 45 irr 128 padd 0
pic_update_irq master 1 imr 248 irr 4 padd 1
  via_isa_request_i8259_irq: irq=0 level=1
pic_update_irq master 0 imr 45 irr 128 padd 0
pic_update_irq master 1 imr 248 irr 4 padd 1
  via_isa_request_i8259_irq: irq=0 level=1
  via_isa_request_i8259_irq: irq=0 level=0
  via_isa_request_i8259_irq: irq=0 level=0

and so on. Neither of the above had your ltim patch applied yet.

Regards,
BALATON Zoltan
Re: [PATCH v3 00/18] hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()
Posted by BALATON Zoltan 1 year, 1 month ago 
On Fri, 3 Mar 2023, David Woodhouse wrote:
> On 2 March 2023 22:40:40 GMT, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>> Since v2: rebased
>>
>> I'm posting this series as it to not block Bernhard's PIIX
>> cleanup work. I don't have code change planned, but eventually
>> reword / improve commit descriptions.
>>
>> Tested commit after commit to be sure it is bisectable. Sadly
>> this was before Zoltan & Thomas report a problem with commit
>> bb98e0f59c ("hw/isa/vt82c686: Remove intermediate IRQ forwarder").
>
> However much I stare at the partial revert which fixes it, I just cannot 
> believe that the change could make any difference at all. There's got to 
> be something weird going on there.
>
> I was going to ask if the level mode for the PIT made any difference, 
> but this is the output IRQ from the PIT to the CPU itself so I don't see 
> how it would.

This is independent of the ltim patch and I've found this even before 
you've sent that patch as this is in my v5 series which you've replied to. 
I've found this problem before when I've first written this model back in 
2019 and did not understand why it's needed but as now shown also with the 
prep machine there's some other problem somewhere that makes this 
necessary. As the way we had before works for the last few years reverting 
it is the safest bet now but we can try to find out and clean up 
eventually.

> Would like to see a report with tracing from pic_update_irq, the CPU 
> interrupt "handler" and the intermediate IRQ handler. With the 
> intermediate present and without it. To compare the two.

I'll try to collect such trace when I'll have time but if you want to 
experiment debian 8.11.0 netinstall cd should boot either with -kernel or 
with the -bios pegasos2.rom (type boot cd install/pegasos at the ok prompt 
in that case but the rom needs to be extracted from an updater as it's not 
freely distributable). I think Thomas Huth also reproduced the same with 
prep or 40p firmware after a similar change on that machine now.

In any case it's unrelated to level sensitive mode which is only needed by 
MorphOS on pegasos2 to fix simultaneous interrupts e.g.with sound and USB 
or PCI cards which all share IRQ9 on that machine. Other guests don't even 
enable that ltim bit so it should not affect anything else.

Regards,
BALATON Zoltan
Re: [PATCH v3 00/18] hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()
Posted by Mark Cave-Ayland 1 year, 1 month ago 
On 03/03/2023 06:58, David Woodhouse wrote:

> On 2 March 2023 22:40:40 GMT, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>> Since v2: rebased
>>
>> I'm posting this series as it to not block Bernhard's PIIX
>> cleanup work. I don't have code change planned, but eventually
>> reword / improve commit descriptions.
>>
>> Tested commit after commit to be sure it is bisectable. Sadly
>> this was before Zoltan & Thomas report a problem with commit
>> bb98e0f59c ("hw/isa/vt82c686: Remove intermediate IRQ forwarder").
> 
> However much I stare at the partial revert which fixes it, I just cannot believe that the change could make any difference at all. There's got to be something weird going on there.
> 
> I was going to ask if the level mode for the PIT made any difference, but this is the output IRQ from the PIT to the CPU itself so I don't see how it would.
> 
> Would like to see a report with tracing from pic_update_irq, the CPU interrupt "handler" and the intermediate IRQ handler. With the intermediate present and without it. To compare the two.

I suspect it's related to the removal of the allocation of the qemu_irq: qdev gpios 
work by adding a child IRQ object to the device, so it could be possible that 
something in the gpio internals isn't being updated correctly when the value is 
overwritten directly.

Is the problem picked up when running a binary built with --enable-sanitizers? That's 
normally quite good at detecting this kind of issue.


ATB,

Mark.
Re: [PATCH v3 00/18] hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()
Posted by Bernhard Beschow 1 year, 1 month ago 
Am 3. März 2023 07:46:31 UTC schrieb Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk>:
>On 03/03/2023 06:58, David Woodhouse wrote:
>
>> On 2 March 2023 22:40:40 GMT, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>>> Since v2: rebased
>>>
>>> I'm posting this series as it to not block Bernhard's PIIX
>>> cleanup work. I don't have code change planned, but eventually
>>> reword / improve commit descriptions.
>>>
>>> Tested commit after commit to be sure it is bisectable. Sadly
>>> this was before Zoltan & Thomas report a problem with commit
>>> bb98e0f59c ("hw/isa/vt82c686: Remove intermediate IRQ forwarder").
>>
>> However much I stare at the partial revert which fixes it, I just cannot believe that the change could make any difference at all. There's got to be something weird going on there.
>>
>> I was going to ask if the level mode for the PIT made any difference, but this is the output IRQ from the PIT to the CPU itself so I don't see how it would.
>>
>> Would like to see a report with tracing from pic_update_irq, the CPU interrupt "handler" and the intermediate IRQ handler. With the intermediate present and without it. To compare the two.
>
>I suspect it's related to the removal of the allocation of the qemu_irq: qdev gpios work by adding a child IRQ object to the device, so it could be possible that something in the gpio internals isn't being updated correctly when the value is overwritten directly.

I've just sent a series fixing the issue.

The problem was that cpu_intr gets populated by
qdev_connect_gpio_out() in board code which happens after via's
realize method has been executed. So in via's realize method cpu_intr
is still NULL which causes a NULL qemu_irq to be passed to the i8259.

One way to fix this is to move qdev_connect_gpio_out() in board code
between pci_new_multifunction() and pci_realize_and_unref().

By having an intermediate IRQ handler the problem didn't appear since
the (non-NULL) qemu_irq holding the intermediate handler is passed to
the i8259. The intermediate handler delays reading cpu_intr to
runtime, so initializing it after realize() is no problem. The price,
however, is that an indirection occurs at runtime every time cpu_intr
is triggered.

BTW, the PIC proxy in my PIIX consolidation series attempted to solve
the same problem: The ISABus IRQs need to be already populated in
piix-ide's realize method, otherwise NULL qemu_irqs are used. As long
as piix-ide is realized in board code, separately from the piix south
bridge, the ISABus IRQs can be populated in between. However, once
piix-ide is realized in the south bridge, the ISA IRQs must be
populated before the south bridge's realize(). The PIC proxy solved
this by introducing intermediate ISA IRQs while the latest incarnation
of the PIIX consolidation series uses the same approach as described
above.

Best regards,
Bernhard
>
>Is the problem picked up when running a binary built with --enable-sanitizers? That's normally quite good at detecting this kind of issue.
>
>
>ATB,
>
>Mark.
Re: [PATCH v3 00/18] hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()
Posted by Philippe Mathieu-Daudé 1 year, 1 month ago 
On 3/3/23 08:46, Mark Cave-Ayland wrote:
> On 03/03/2023 06:58, David Woodhouse wrote:
> 
>> On 2 March 2023 22:40:40 GMT, "Philippe Mathieu-Daudé" 
>> <philmd@linaro.org> wrote:
>>> Since v2: rebased
>>>
>>> I'm posting this series as it to not block Bernhard's PIIX
>>> cleanup work. I don't have code change planned, but eventually
>>> reword / improve commit descriptions.
>>>
>>> Tested commit after commit to be sure it is bisectable. Sadly
>>> this was before Zoltan & Thomas report a problem with commit
>>> bb98e0f59c ("hw/isa/vt82c686: Remove intermediate IRQ forwarder").
>>
>> However much I stare at the partial revert which fixes it, I just 
>> cannot believe that the change could make any difference at all. 
>> There's got to be something weird going on there.
>>
>> I was going to ask if the level mode for the PIT made any difference, 
>> but this is the output IRQ from the PIT to the CPU itself so I don't 
>> see how it would.
>>
>> Would like to see a report with tracing from pic_update_irq, the CPU 
>> interrupt "handler" and the intermediate IRQ handler. With the 
>> intermediate present and without it. To compare the two.
> 
> I suspect it's related to the removal of the allocation of the qemu_irq: 
> qdev gpios work by adding a child IRQ object to the device, so it could 
> be possible that something in the gpio internals isn't being updated 
> correctly when the value is overwritten directly.
> 
> Is the problem picked up when running a binary built with 
> --enable-sanitizers? That's normally quite good at detecting this kind 
> of issue.

No ASan warning. However I see (before/after bb98e0f59c):

qemu-system-ppc: pc87312: unsupported device reconfiguration (0f 11 00)
qemu-system-ppc: pc87312: unsupported device reconfiguration (0f 11 84)
qemu-system-ppc: pc87312: unsupported device reconfiguration (09 01 84)
Re: [PATCH v3 00/18] hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()
Posted by BALATON Zoltan 1 year, 1 month ago 
On Fri, 3 Mar 2023, Philippe Mathieu-Daudé wrote:
> On 3/3/23 08:46, Mark Cave-Ayland wrote:
>> On 03/03/2023 06:58, David Woodhouse wrote:
>> 
>>> On 2 March 2023 22:40:40 GMT, "Philippe Mathieu-Daudé" <philmd@linaro.org> 
>>> wrote:
>>>> Since v2: rebased
>>>> 
>>>> I'm posting this series as it to not block Bernhard's PIIX
>>>> cleanup work. I don't have code change planned, but eventually
>>>> reword / improve commit descriptions.
>>>> 
>>>> Tested commit after commit to be sure it is bisectable. Sadly
>>>> this was before Zoltan & Thomas report a problem with commit
>>>> bb98e0f59c ("hw/isa/vt82c686: Remove intermediate IRQ forwarder").
>>> 
>>> However much I stare at the partial revert which fixes it, I just cannot 
>>> believe that the change could make any difference at all. There's got to 
>>> be something weird going on there.
>>> 
>>> I was going to ask if the level mode for the PIT made any difference, but 
>>> this is the output IRQ from the PIT to the CPU itself so I don't see how 
>>> it would.
>>> 
>>> Would like to see a report with tracing from pic_update_irq, the CPU 
>>> interrupt "handler" and the intermediate IRQ handler. With the 
>>> intermediate present and without it. To compare the two.
>> 
>> I suspect it's related to the removal of the allocation of the qemu_irq: 
>> qdev gpios work by adding a child IRQ object to the device, so it could be 
>> possible that something in the gpio internals isn't being updated correctly 
>> when the value is overwritten directly.
>> 
>> Is the problem picked up when running a binary built with 
>> --enable-sanitizers? That's normally quite good at detecting this kind of 
>> issue.
>
> No ASan warning. However I see (before/after bb98e0f59c):
>
> qemu-system-ppc: pc87312: unsupported device reconfiguration (0f 11 00)
> qemu-system-ppc: pc87312: unsupported device reconfiguration (0f 11 84)
> qemu-system-ppc: pc87312: unsupported device reconfiguration (09 01 84)

This does not seem related at all especially if you also see it before 
because we have the same problem in vt82c686 and this error above rather 
looks liek it should be a qemu_log_mask(LOG_UNIMP) as that's all that 
function seems to do where this is printed so looks like it's just 
unimplemented functionality.

Regards,
BALATON Zoltan
Re: [PATCH v3 00/18] hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()
Posted by Michael S. Tsirkin 1 year, 1 month ago 
On Thu, Mar 02, 2023 at 11:40:40PM +0100, Philippe Mathieu-Daudé wrote:
> Since v2: rebased
> 
> I'm posting this series as it to not block Bernhard's PIIX
> cleanup work. I don't have code change planned, but eventually
> reword / improve commit descriptions.
> 
> Tested commit after commit to be sure it is bisectable. Sadly
> this was before Zoltan & Thomas report a problem with commit
> bb98e0f59c ("hw/isa/vt82c686: Remove intermediate IRQ forwarder").
> 
> Background thread:
> https://lore.kernel.org/qemu-devel/5095dffc-309b-6c72-d255-8cdaa6fd3d52@ilande.co.uk/


Acked-by: Michael S. Tsirkin <mst@redhat.com>

who's merging this you?

I am unsure about interdependencies between all these patchsets at this
point.

> Philippe Mathieu-Daudé (18):
>   hw/ide/piix: Expose output IRQ as properties for late object
>     population
>   hw/ide/piix: Allow using PIIX3-IDE as standalone PCI function
>   hw/i386/pc_piix: Wire PIIX3 IDE ouput IRQs to ISA bus IRQs 14/15
>   hw/isa/piix4: Wire PIIX4 IDE ouput IRQs to ISA bus IRQs 14/15
>   hw/ide: Rename ISA specific ide_init_ioport -> ide_bus_init_ioport_isa
>   hw/ide/piix: Ensure IDE output IRQs are wired at realization
>   hw/isa: Deprecate isa_get_irq() in favor of isa_bus_get_irq()
>   hw/ide: Introduce generic ide_init_ioport()
>   hw/ide/piix: Use generic ide_bus_init_ioport()
>   hw/isa: Ensure isa_register_portio_list() do not get NULL ISA device
>   hw/isa: Simplify isa_address_space[_io]()
>   hw/isa: Reduce 'isabus' singleton scope to isa_bus_new()
>   exec/ioport: Factor portio_list_register_flush_coalesced() out
>   exec/ioport: Factor portio_list_register() out
>   hw/southbridge/piix: Use OBJECT_DECLARE_SIMPLE_TYPE() macro
>   hw/isa/piix: Batch register QOM types using DEFINE_TYPES() macro
>   hw/isa/piix: Unify QOM type name of PIIX ISA function
>   hw/isa/piix: Unify PIIX-ISA QOM type names using qdev aliases
> 
>  hw/audio/adlib.c              |  4 +--
>  hw/display/qxl.c              |  7 ++--
>  hw/display/vga.c              |  9 +++--
>  hw/dma/i82374.c               |  7 ++--
>  hw/i386/pc_piix.c             | 13 +++++---
>  hw/ide/ioport.c               | 15 +++++++--
>  hw/ide/isa.c                  |  2 +-
>  hw/ide/piix.c                 | 54 +++++++++++++++++++++++-------
>  hw/isa/isa-bus.c              | 36 ++++++++------------
>  hw/isa/piix3.c                | 63 +++++++++++++++--------------------
>  hw/isa/piix4.c                | 12 ++++---
>  hw/mips/malta.c               |  2 +-
>  hw/watchdog/wdt_ib700.c       |  4 +--
>  include/exec/ioport.h         | 15 +++++----
>  include/hw/ide/internal.h     |  3 +-
>  include/hw/ide/isa.h          |  3 ++
>  include/hw/ide/piix.h         |  4 +++
>  include/hw/isa/isa.h          |  3 +-
>  include/hw/southbridge/piix.h | 14 ++++----
>  softmmu/ioport.c              | 48 +++++++++++++++++++-------
>  softmmu/qdev-monitor.c        |  3 ++
>  21 files changed, 190 insertions(+), 131 deletions(-)
> 
> -- 
> 2.38.1
> 
> 
>
Re: [PATCH v3 00/18] hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()
Posted by Bernhard Beschow 1 year ago 

Am 2. März 2023 22:40:40 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>Since v2: rebased
>
>I'm posting this series as it to not block Bernhard's PIIX
>cleanup work. I don't have code change planned, but eventually
>reword / improve commit descriptions.
>
>Tested commit after commit to be sure it is bisectable. Sadly
>this was before Zoltan & Thomas report a problem with commit
>bb98e0f59c ("hw/isa/vt82c686: Remove intermediate IRQ forwarder").
>
>Background thread:
>https://lore.kernel.org/qemu-devel/5095dffc-309b-6c72-d255-8cdaa6fd3d52@ilande.co.uk/

Hi,

I've just sent yet another proposal which might make some renamings done in this series appear unneccessary.

Best regards,
Bernhard

>
>Philippe Mathieu-Daudé (18):
>  hw/ide/piix: Expose output IRQ as properties for late object
>    population
>  hw/ide/piix: Allow using PIIX3-IDE as standalone PCI function
>  hw/i386/pc_piix: Wire PIIX3 IDE ouput IRQs to ISA bus IRQs 14/15
>  hw/isa/piix4: Wire PIIX4 IDE ouput IRQs to ISA bus IRQs 14/15
>  hw/ide: Rename ISA specific ide_init_ioport -> ide_bus_init_ioport_isa
>  hw/ide/piix: Ensure IDE output IRQs are wired at realization
>  hw/isa: Deprecate isa_get_irq() in favor of isa_bus_get_irq()
>  hw/ide: Introduce generic ide_init_ioport()
>  hw/ide/piix: Use generic ide_bus_init_ioport()
>  hw/isa: Ensure isa_register_portio_list() do not get NULL ISA device
>  hw/isa: Simplify isa_address_space[_io]()
>  hw/isa: Reduce 'isabus' singleton scope to isa_bus_new()
>  exec/ioport: Factor portio_list_register_flush_coalesced() out
>  exec/ioport: Factor portio_list_register() out
>  hw/southbridge/piix: Use OBJECT_DECLARE_SIMPLE_TYPE() macro
>  hw/isa/piix: Batch register QOM types using DEFINE_TYPES() macro
>  hw/isa/piix: Unify QOM type name of PIIX ISA function
>  hw/isa/piix: Unify PIIX-ISA QOM type names using qdev aliases
>
> hw/audio/adlib.c              |  4 +--
> hw/display/qxl.c              |  7 ++--
> hw/display/vga.c              |  9 +++--
> hw/dma/i82374.c               |  7 ++--
> hw/i386/pc_piix.c             | 13 +++++---
> hw/ide/ioport.c               | 15 +++++++--
> hw/ide/isa.c                  |  2 +-
> hw/ide/piix.c                 | 54 +++++++++++++++++++++++-------
> hw/isa/isa-bus.c              | 36 ++++++++------------
> hw/isa/piix3.c                | 63 +++++++++++++++--------------------
> hw/isa/piix4.c                | 12 ++++---
> hw/mips/malta.c               |  2 +-
> hw/watchdog/wdt_ib700.c       |  4 +--
> include/exec/ioport.h         | 15 +++++----
> include/hw/ide/internal.h     |  3 +-
> include/hw/ide/isa.h          |  3 ++
> include/hw/ide/piix.h         |  4 +++
> include/hw/isa/isa.h          |  3 +-
> include/hw/southbridge/piix.h | 14 ++++----
> softmmu/ioport.c              | 48 +++++++++++++++++++-------
> softmmu/qdev-monitor.c        |  3 ++
> 21 files changed, 190 insertions(+), 131 deletions(-)
>
Re: [PATCH v3 00/18] hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()
Posted by Michael S. Tsirkin 1 year ago 
On Thu, Mar 02, 2023 at 11:40:40PM +0100, Philippe Mathieu-Daudé wrote:
> Since v2: rebased
> 
> I'm posting this series as it to not block Bernhard's PIIX
> cleanup work. I don't have code change planned, but eventually
> reword / improve commit descriptions.

> Tested commit after commit to be sure it is bisectable. Sadly
> this was before Zoltan & Thomas report a problem with commit
> bb98e0f59c ("hw/isa/vt82c686: Remove intermediate IRQ forwarder").

I'm not sure what this implies, or how do you want to
resolve the conflicts with Bernhard's work.

did my best to review, series:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


> 
> Background thread:
> https://lore.kernel.org/qemu-devel/5095dffc-309b-6c72-d255-8cdaa6fd3d52@ilande.co.uk/
> 
> Philippe Mathieu-Daudé (18):
>   hw/ide/piix: Expose output IRQ as properties for late object
>     population
>   hw/ide/piix: Allow using PIIX3-IDE as standalone PCI function
>   hw/i386/pc_piix: Wire PIIX3 IDE ouput IRQs to ISA bus IRQs 14/15
>   hw/isa/piix4: Wire PIIX4 IDE ouput IRQs to ISA bus IRQs 14/15
>   hw/ide: Rename ISA specific ide_init_ioport -> ide_bus_init_ioport_isa
>   hw/ide/piix: Ensure IDE output IRQs are wired at realization
>   hw/isa: Deprecate isa_get_irq() in favor of isa_bus_get_irq()
>   hw/ide: Introduce generic ide_init_ioport()
>   hw/ide/piix: Use generic ide_bus_init_ioport()
>   hw/isa: Ensure isa_register_portio_list() do not get NULL ISA device
>   hw/isa: Simplify isa_address_space[_io]()
>   hw/isa: Reduce 'isabus' singleton scope to isa_bus_new()
>   exec/ioport: Factor portio_list_register_flush_coalesced() out
>   exec/ioport: Factor portio_list_register() out
>   hw/southbridge/piix: Use OBJECT_DECLARE_SIMPLE_TYPE() macro
>   hw/isa/piix: Batch register QOM types using DEFINE_TYPES() macro
>   hw/isa/piix: Unify QOM type name of PIIX ISA function
>   hw/isa/piix: Unify PIIX-ISA QOM type names using qdev aliases
> 
>  hw/audio/adlib.c              |  4 +--
>  hw/display/qxl.c              |  7 ++--
>  hw/display/vga.c              |  9 +++--
>  hw/dma/i82374.c               |  7 ++--
>  hw/i386/pc_piix.c             | 13 +++++---
>  hw/ide/ioport.c               | 15 +++++++--
>  hw/ide/isa.c                  |  2 +-
>  hw/ide/piix.c                 | 54 +++++++++++++++++++++++-------
>  hw/isa/isa-bus.c              | 36 ++++++++------------
>  hw/isa/piix3.c                | 63 +++++++++++++++--------------------
>  hw/isa/piix4.c                | 12 ++++---
>  hw/mips/malta.c               |  2 +-
>  hw/watchdog/wdt_ib700.c       |  4 +--
>  include/exec/ioport.h         | 15 +++++----
>  include/hw/ide/internal.h     |  3 +-
>  include/hw/ide/isa.h          |  3 ++
>  include/hw/ide/piix.h         |  4 +++
>  include/hw/isa/isa.h          |  3 +-
>  include/hw/southbridge/piix.h | 14 ++++----
>  softmmu/ioport.c              | 48 +++++++++++++++++++-------
>  softmmu/qdev-monitor.c        |  3 ++
>  21 files changed, 190 insertions(+), 131 deletions(-)
> 
> -- 
> 2.38.1
> 
> 
>
Re: [PATCH v3 00/18] hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()
Posted by Mark Cave-Ayland 12 months ago 
On 21/04/2023 09:25, Michael S. Tsirkin wrote:

> On Thu, Mar 02, 2023 at 11:40:40PM +0100, Philippe Mathieu-Daudé wrote:
>> Since v2: rebased
>>
>> I'm posting this series as it to not block Bernhard's PIIX
>> cleanup work. I don't have code change planned, but eventually
>> reword / improve commit descriptions.
> 
>> Tested commit after commit to be sure it is bisectable. Sadly
>> this was before Zoltan & Thomas report a problem with commit
>> bb98e0f59c ("hw/isa/vt82c686: Remove intermediate IRQ forwarder").
> 
> I'm not sure what this implies, or how do you want to
> resolve the conflicts with Bernhard's work.
> 
> did my best to review, series:
> 
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Phil: I revisited this series after our discussion earlier in the week re: device 
IRQs, and I realised that there is some overlap with this series, Bernhard's latest 
series, and some of my local patches for switching between legacy/native PCI IDE IRQ 
routing.

I've now gone through both series and replied where I think we should now be going 
with this, but I can see that the final series will likely pull from all 3 sources.

How do you think we should best move forward from here? Once you've replied to the 
various comments I can potentially pull everything together into an updated series if 
that helps?

>> Background thread:
>> https://lore.kernel.org/qemu-devel/5095dffc-309b-6c72-d255-8cdaa6fd3d52@ilande.co.uk/
>>
>> Philippe Mathieu-Daudé (18):
>>    hw/ide/piix: Expose output IRQ as properties for late object
>>      population
>>    hw/ide/piix: Allow using PIIX3-IDE as standalone PCI function
>>    hw/i386/pc_piix: Wire PIIX3 IDE ouput IRQs to ISA bus IRQs 14/15
>>    hw/isa/piix4: Wire PIIX4 IDE ouput IRQs to ISA bus IRQs 14/15
>>    hw/ide: Rename ISA specific ide_init_ioport -> ide_bus_init_ioport_isa
>>    hw/ide/piix: Ensure IDE output IRQs are wired at realization
>>    hw/isa: Deprecate isa_get_irq() in favor of isa_bus_get_irq()
>>    hw/ide: Introduce generic ide_init_ioport()
>>    hw/ide/piix: Use generic ide_bus_init_ioport()
>>    hw/isa: Ensure isa_register_portio_list() do not get NULL ISA device
>>    hw/isa: Simplify isa_address_space[_io]()
>>    hw/isa: Reduce 'isabus' singleton scope to isa_bus_new()
>>    exec/ioport: Factor portio_list_register_flush_coalesced() out
>>    exec/ioport: Factor portio_list_register() out
>>    hw/southbridge/piix: Use OBJECT_DECLARE_SIMPLE_TYPE() macro
>>    hw/isa/piix: Batch register QOM types using DEFINE_TYPES() macro
>>    hw/isa/piix: Unify QOM type name of PIIX ISA function
>>    hw/isa/piix: Unify PIIX-ISA QOM type names using qdev aliases
>>
>>   hw/audio/adlib.c              |  4 +--
>>   hw/display/qxl.c              |  7 ++--
>>   hw/display/vga.c              |  9 +++--
>>   hw/dma/i82374.c               |  7 ++--
>>   hw/i386/pc_piix.c             | 13 +++++---
>>   hw/ide/ioport.c               | 15 +++++++--
>>   hw/ide/isa.c                  |  2 +-
>>   hw/ide/piix.c                 | 54 +++++++++++++++++++++++-------
>>   hw/isa/isa-bus.c              | 36 ++++++++------------
>>   hw/isa/piix3.c                | 63 +++++++++++++++--------------------
>>   hw/isa/piix4.c                | 12 ++++---
>>   hw/mips/malta.c               |  2 +-
>>   hw/watchdog/wdt_ib700.c       |  4 +--
>>   include/exec/ioport.h         | 15 +++++----
>>   include/hw/ide/internal.h     |  3 +-
>>   include/hw/ide/isa.h          |  3 ++
>>   include/hw/ide/piix.h         |  4 +++
>>   include/hw/isa/isa.h          |  3 +-
>>   include/hw/southbridge/piix.h | 14 ++++----
>>   softmmu/ioport.c              | 48 +++++++++++++++++++-------
>>   softmmu/qdev-monitor.c        |  3 ++
>>   21 files changed, 190 insertions(+), 131 deletions(-)
>>
>> -- 
>> 2.38.1


ATB,

Mark.

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.