On 29/05/2022 14:02, Bernhard Beschow wrote:
On Sun, May 29, 2022 at 12:06 PM Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk
<mailto:mark.cave-ayl...@ilande.co.uk>> wrote:
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
<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
<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-ayl...@ilande.co.uk
<mailto:mark.cave-ayl...@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 <http://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.
Hi Mark,
Thanks for your great work you put into reviews and the useful insights! It seems to
me that the time it takes for patches to be queued depends on the subsystem - some
are faster, some are slower...
I've automated my setup as described in [1]. However, it doesn't seem to work for the
cover letter which I'd expect to be sent to the union of all reviewers of all
patches. Any idea how to fix this?
Best regards,
Bernhard
[1]
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#cc-the-relevant-maintainer
<https://www.qemu.org/docs/master/devel/submitting-a-patch.html#cc-the-relevant-maintainer>
Good question. I tend to do "git format-patch -o /tmp/foo --cover-letter" to generate
the series, fill in the cover letter, and then use "git send-email /tmp/foo" to send
out the emails (entering in the results of get_maintainer.pl by hand). I'm not sure
why the cover letter isn't being generated correctly in your case I'm afraid.
ATB,
Mark.
