mbox series

[v4,00/11] QOM'ify PIIX southbridge creation

Message ID 20220603185045.143789-1-shentey@gmail.com (mailing list archive)
Headers show
Series QOM'ify PIIX southbridge creation | expand

Message

Bernhard Beschow June 3, 2022, 6:50 p.m. UTC
v4:
* Rebase onto https://patchew.org/QEMU/20220530112718.26582-1-philippe.mathieu.daude@gmail.com/
* Cosmetics (fix typo, omit "include") (Mark, Philippe)
* Split piix3 and piix4 (Philippe)
* s/Found-by/Reported-by/ (Philippe)
* Don't alias smbus (Mark)

v3:
* Rebase onto 'hw/acpi/piix4: remove legacy piix4_pm_init() function' (Mark) [1]
* Use embedded structs for touched PCI devices (Mark)
* Fix piix4's rtc embedded struct to be initialized by
  object_initialize_child() (Peter) [2]

Testing done:

1)
`make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
Result: All pass.

2)
* `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
* `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
  debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`

In both cases the system booted successfully and it was possible to shut down
the system using the `poweroff` command.

v2:
* Preserve `DeviceState *` as return value of piix4_create() (Mark)
* Aggregate all type name movements into first commit (Mark)
* Have piix4 southbridge rather than malta board instantiate piix4 pm (me)

Testing done:

1)
`make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
Result: All pass.

2)
Modify pci_piix3_realize() to start with
    error_setg(errp, "This is a test");
Then start `qemu-system-x86_64 -M pc -m 1G -accel kvm -cpu host -cdrom
archlinux-2022.05.01-x86_64.iso`.
Result: qemu-system-x86_64 aborts with: "This is a test"

v1:
The piix3 and piix4 southbridge devices still rely on create() functions which
are deprecated. This series resolves these functions piece by piece to
modernize the code.

Both devices are modified in lockstep where possible to provide more context.

Testing done:
* `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
* `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
  debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`

In both cases the system booted successfully and it was possible to shut down
the system using the `poweroff` command.

[1] https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg05686.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg01128.html

Bernhard Beschow (11):
  hw/southbridge/piix: Aggregate all PIIX southbridge type names
  hw/isa/piix4: Use object_initialize_child() for embedded struct
  hw/isa/piix4: Move pci_map_irq_fn' near pci_set_irq_fn
  hw/isa/piix4: QOM'ify PCI device creation and wiring
  hw/isa/piix4: Factor out ISABus retrieval from piix4_create()
  hw/isa/piix4: QOM'ify PIIX4 PM creation
  hw/isa/piix4: Inline and remove piix4_create()
  hw/isa/piix3: Move pci_map_irq_fn near pci_set_irq_fn
  hw/isa/piix3: QOM'ify PCI device creation and wiring
  hw/isa/piix3: Factor out ISABus retrieval from piix3_create()
  hw/isa/piix3: Inline and remove piix3_create()

 hw/i386/pc_piix.c             |   7 +-
 hw/isa/piix3.c                |  98 +++++++++++++++-------------
 hw/isa/piix4.c                | 119 +++++++++++++++++-----------------
 hw/mips/malta.c               |   9 ++-
 include/hw/isa/isa.h          |   2 -
 include/hw/southbridge/piix.h |   6 +-
 6 files changed, 128 insertions(+), 113 deletions(-)

Comments

Mark Cave-Ayland June 6, 2022, 10:38 a.m. UTC | #1
On 03/06/2022 19:50, Bernhard Beschow wrote:

> v4:
> * Rebase onto https://patchew.org/QEMU/20220530112718.26582-1-philippe.mathieu.daude@gmail.com/
> * Cosmetics (fix typo, omit "include") (Mark, Philippe)
> * Split piix3 and piix4 (Philippe)
> * s/Found-by/Reported-by/ (Philippe)
> * Don't alias smbus (Mark)
> 
> v3:
> * Rebase onto 'hw/acpi/piix4: remove legacy piix4_pm_init() function' (Mark) [1]
> * Use embedded structs for touched PCI devices (Mark)
> * Fix piix4's rtc embedded struct to be initialized by
>    object_initialize_child() (Peter) [2]
> 
> Testing done:
> 
> 1)
> `make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
> Result: All pass.
> 
> 2)
> * `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
> * `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
>    debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`
> 
> In both cases the system booted successfully and it was possible to shut down
> the system using the `poweroff` command.
> 
> v2:
> * Preserve `DeviceState *` as return value of piix4_create() (Mark)
> * Aggregate all type name movements into first commit (Mark)
> * Have piix4 southbridge rather than malta board instantiate piix4 pm (me)
> 
> Testing done:
> 
> 1)
> `make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
> Result: All pass.
> 
> 2)
> Modify pci_piix3_realize() to start with
>      error_setg(errp, "This is a test");
> Then start `qemu-system-x86_64 -M pc -m 1G -accel kvm -cpu host -cdrom
> archlinux-2022.05.01-x86_64.iso`.
> Result: qemu-system-x86_64 aborts with: "This is a test"
> 
> v1:
> The piix3 and piix4 southbridge devices still rely on create() functions which
> are deprecated. This series resolves these functions piece by piece to
> modernize the code.
> 
> Both devices are modified in lockstep where possible to provide more context.
> 
> Testing done:
> * `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
> * `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
>    debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`
> 
> In both cases the system booted successfully and it was possible to shut down
> the system using the `poweroff` command.
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg05686.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg01128.html
> 
> Bernhard Beschow (11):
>    hw/southbridge/piix: Aggregate all PIIX southbridge type names
>    hw/isa/piix4: Use object_initialize_child() for embedded struct
>    hw/isa/piix4: Move pci_map_irq_fn' near pci_set_irq_fn
>    hw/isa/piix4: QOM'ify PCI device creation and wiring
>    hw/isa/piix4: Factor out ISABus retrieval from piix4_create()
>    hw/isa/piix4: QOM'ify PIIX4 PM creation
>    hw/isa/piix4: Inline and remove piix4_create()
>    hw/isa/piix3: Move pci_map_irq_fn near pci_set_irq_fn
>    hw/isa/piix3: QOM'ify PCI device creation and wiring
>    hw/isa/piix3: Factor out ISABus retrieval from piix3_create()
>    hw/isa/piix3: Inline and remove piix3_create()
> 
>   hw/i386/pc_piix.c             |   7 +-
>   hw/isa/piix3.c                |  98 +++++++++++++++-------------
>   hw/isa/piix4.c                | 119 +++++++++++++++++-----------------
>   hw/mips/malta.c               |   9 ++-
>   include/hw/isa/isa.h          |   2 -
>   include/hw/southbridge/piix.h |   6 +-
>   6 files changed, 128 insertions(+), 113 deletions(-)

I've had quick look over this and it seems fine to me, so for the entire series:

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.
Philippe Mathieu-Daudé June 11, 2022, 9:47 a.m. UTC | #2
On 3/6/22 20:50, Bernhard Beschow wrote:

> Bernhard Beschow (11):
>    hw/southbridge/piix: Aggregate all PIIX southbridge type names
>    hw/isa/piix4: Use object_initialize_child() for embedded struct
>    hw/isa/piix4: Move pci_map_irq_fn' near pci_set_irq_fn
>    hw/isa/piix4: QOM'ify PCI device creation and wiring
>    hw/isa/piix4: Factor out ISABus retrieval from piix4_create()
>    hw/isa/piix4: QOM'ify PIIX4 PM creation
>    hw/isa/piix4: Inline and remove piix4_create()
>    hw/isa/piix3: Move pci_map_irq_fn near pci_set_irq_fn
>    hw/isa/piix3: QOM'ify PCI device creation and wiring
>    hw/isa/piix3: Factor out ISABus retrieval from piix3_create()
>    hw/isa/piix3: Inline and remove piix3_create()

Series queued via mips-next, thanks!