Re: [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE

[Chuck Zmudzinski <brchuckz@xxxxxxx>]

On 1/3/2023 8:38 AM, Bernhard Beschow wrote:
>
>
> On Tue, Jan 3, 2023 at 2:17 PM Philippe Mathieu-Daudé <philmd@xxxxxxxxxx> 
> wrote:
>
>     Hi Chuck,
>
>     On 3/1/23 04:15, Chuck Zmudzinski wrote:
>     > On 1/2/23 4:34 PM, Bernhard Beschow wrote:
>     >> This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally 
> removes
>     >> it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen 
> in the PC
>     >> machine agnostic to the precise southbridge being used. 2/ will become
>     >> particularily interesting once PIIX4 becomes usable in the PC machine, 
> avoiding
>     >> the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>     >>
>     >> Testing done:
>     >> None, because I don't know how to conduct this properly :(
>     >>
>     >> Based-on: <20221221170003.2929-1-shentey@xxxxxxxxx>
>     >>            "[PATCH v4 00/30] Consolidate PIIX south bridges"
>
>     This series is based on a previous series:
>     
> https://lore.kernel.org/qemu-devel/20221221170003.2929-1-shentey@xxxxxxxxx/
>     (which itself also is).
>
>     >> Bernhard Beschow (6):
>     >>    include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
>     >>    hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
>     >>    hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
>     >>    hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
>     >>    hw/isa/piix: Resolve redundant k->config_write assignments
>     >>    hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>     >>
>     >>   hw/i386/pc_piix.c             | 34 ++++++++++++++++--
>     >>   hw/i386/xen/xen-hvm.c         |  9 +++--
>     >>   hw/isa/piix.c                 | 66 
> +----------------------------------
>     >
>     > This file does not exist on the Qemu master branch.
>     > But hw/isa/piix3.c and hw/isa/piix4.c do exist.
>     >
>     > I tried renaming it from piix.c to piix3.c in the patch, but
>     > the patch set still does not apply cleanly on my tree.
>     >
>     > Is this patch set re-based against something other than
>     > the current master Qemu branch?
>     >
>     > I have a system that is suitable for testing this patch set, but
>     > I need guidance on how to apply it to the Qemu source tree.
>
>     You can ask Bernhard to publish a branch with the full work,
>
>
> Hi Chuck,
>
> ... or just visit 
> https://patchew.org/QEMU/20230102213504.14646-1-shentey@xxxxxxxxx/ . There 
> you'll find a git tag with a complete history and all instructions!
>
> Thanks for giving my series a test ride!
>
> Best regards,
> Bernhard
>
>     or apply each series locally. I use the b4 tool for that:
>     https://b4.docs.kernel.org/en/latest/installing.html
>
>     i.e.:
>
>     $ git checkout -b shentey_work
>     $ b4 am 20221120150550.63059-1-shentey@xxxxxxxxx
>     $ git am
>     ./v2_20221120_shentey_decouple_intx_to_lnkx_routing_from_south_bridges.mbx
>     $ b4 am 20221221170003.2929-1-shentey@xxxxxxxxx
>     $ git am
>     
> ./v4_20221221_shentey_this_series_consolidates_the_implementations_of_the_piix3_and_piix4_south.mbx
>     $ b4 am 20230102213504.14646-1-shentey@xxxxxxxxx
>     $ git am ./20230102_shentey_resolve_type_piix3_xen_device.mbx
>
>     Now the branch 'shentey_work' contains all the patches and you can test.
>
>     Regards,
>
>     Phil.
>

Hi Phil and Bernard,

I tried applying these 3 patch series on top of the current qemu
master branch.

Unfortunately, I saw a regression, so I can't give a tested-by tag yet.

Here are the details of the testing I did so far:

Xen only needs one target, the i386-softmmu target which creates
the qemu-system-i386 binary that Xen uses for its device model.
That target compiled and linked with no problems with these 3
patch series applied on top of qemu master. I didn't try building
any other targets.

My tests used the xenfv machine type with the xen platform
pci device, which is ordinarily called a xen hvm guest with xen
paravirtualized network and block device drivers. It is based on the
i440fx machine type and so emulates piix3. I tested the xen
hvm guests with two different configurations as described below.

I tested both Linux and Windows guests, with mixed results. With the
current Qemu master (commit 222059a0fccf4 without the 3 patch
series applied), all tested guest configurations work normally for both
Linux and Windows guests.

With these 3 patch series applied on top of the qemu master branch,
which tries to consolidate piix3 and piix4 and resolve the xen piix3
device that my guests use, I unfortunately got a regression.

The regression occurred with a configuration that uses the qemu
bochs stdvga graphics device with a vnc display, and the qemu
usb-tablet device to emulate the mouse and keyboard. After applying
the 3 patch series, the emulated mouse is not working at all for Linux
guests. It works for Windows guests, but the mouse pointer in the
guest does not follow the mouse pointer in the vnc window as closely
as it does without the 3 patch series. So this is the bad news of a
regression introduced somewhere in these 3 patch series.

The good news is that by using guests in a configuration that does
not use the qemu usb-tablet device or the bochs stdvga device but
instead uses a real passed through usb3 controller with a real usb
mouse and a real usb keyboard connected, and also the real sound
card and vga device passed through and a 1920x1080 HDMI monitor,
there is no regression introduced by the 3 patch series and both Linux
and Windows guests in that configuration work perfectly.

My next test will be to test Bernhard's published git tag without
trying to merge the 3 patch series into master and see if that also
has the regression. I also will double check that I didn't make
any mistakes in merging the 3 patch series by creating the shentey_work
branch with b4 and git as Phil described and compare that to my
working tree.

I also will try testing only the first series, then the first series and the
second series, to try to determine in which of the 3 series the regression
is introduced.

Best regards,

Chuck