qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 0/7] QOM'ify PIIX southbridge creation


From: Mark Cave-Ayland
Subject: Re: [PATCH v3 0/7] QOM'ify PIIX southbridge creation
Date: Sun, 29 May 2022 11:06:07 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0

On 29/05/2022 10:46, Mark Cave-Ayland wrote:

On 28/05/2022 20:20, Bernhard Beschow wrote:

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 (7):
   include/hw/southbridge/piix: Aggregate all PIIX soughbridge type names
   hw/isa/piix4: Use object_initialize_child() for embedded struct
   hw/isa/piix{3,4}: Move pci_map_irq_fn's near pci_set_irq_fn's
   hw/isa/piix{3,4}: QOM'ify PCI device creation and wiring
   hw/isa/piix{3,4}: Factor out ISABus retrieval from create() functions
   hw/isa/piix4: QOM'ify PIIX4 PM creation
   hw/isa/piix{3,4}: Inline and remove create() functions

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

Hi Bernhard,

I've spotted a couple of small things, but once those are fixed this series looks good to me so I'm happy to give a:

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

Thanks for your patience with these series too: you've done some good work here, however patchsets like this can sometimes take a while to get reviewed because they both i) touch legacy code/APIs and ii) cut across multiple machines and maintainers. I'd really like to get this work, along with your RTC updates, merged soon as it is a great improvement.

One reason that you may not get many reviews is because you've not added the relevant maintainers on To/CC. Due to the large volume of emails on qemu-devel, quite a few maintainers will filter based upon their own email address so it is definitely worth adding them in.

Fortunately you can easily find the relevant maintainer email addresses by running "./scripts/get_maintainer.pl <path-to-git-patch-dir>" on your git format-patch directory. I'd recommend doing this, and also dropping qemu-trivial since I would say these patches are now beyond the trivial threshold.

Oh wait - I see now it's just the cover letter which is missing the additional maintainer addresses :) If you could add them into the cover letter for your next revision that would be great, since it gives context for maintainers to help with the review process.


ATB,

Mark.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]