Re: [PATCH v5 0/3] hw/i386/acpi: Pre-compute the _PRT table

[Ricardo Ribalda]

Hi Mark



On Sun, 22 Sept 2024 at 13:57, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> On 21/08/2024 15:45, Igor Mammedov wrote:
>
> > On Wed, 14 Aug 2024 11:56:08 +0000
> > Ricardo Ribalda <ribalda@chromium.org> wrote:
> >
> >> Today for x86 the _PRT() table is computed in runtime.
> >> Under some configurations, computing the _PRT table can take more than
> >> 30 seconds and the ACPI timeout is violated.
> >>
> >> This patchset modifies _PRT() to return a pre-computed table.
> >
> > To be sure we are not breaking anything boot tested it with
> > rhel6.7/9.0, winxp, ws2022
>
> Hi Ricardo/Igor,
>
> Unfortunately it seems that this series (and in particular commit 99cb2c6c7b
> ("hw/i386/acpi-build: Return a pre-computed _PRT table") breaks my WinXP ISO 
> boot
> test case using the dc390/am53C974 SCSI device.
>
> Current master hangs when booting WinXP from a dc390 SCSI CD just after 
> loading the
> kernel device drivers and displaying "Setup is starting Windows" instead of
> proceeding to the "Welcome to Setup" screen.
>
> Note that there is a separate timeout issue caused by a bug in SeaBIOS which 
> is fixed
> in the SeaBIOS repository as commit 2424e4c0 ("esp-scsi: indicate acceptance 
> of
> MESSAGE IN phase data"). As the QEMU SeaBIOS binaries haven't yet been 
> updated to
> contain this fix, I've uploaded a pre-built bios.bin to
> https://www.ilande.co.uk/tmp/qemu/bios.bin to help reproduce the issue.
>
> Once the above file is downloaded the issue can be reproduced with the 
> command line
> below:
>
> ./qemu-system-x86_64 \
>    -m 1G \
>    -device dc390,id=scsi0 \
>    -device scsi-cd,drive=drive0,bus=scsi0.0,channel=0,scsi-id=6,lun=0 \
>    -drive file=winxp.iso,if=none,id=drive0 \
>    -vga cirrus \
>    -boot d \
>    -trace 'esp*' \
>    -bios bios.bin
>
> With the ESP tracing enabled it is possible to see that WinXP appears to get 
> stuck in
> a loop trying to send a SCSI command followed by a "Bus Reset". Reverting 
> 99cb2c6c7b
> allows the WinXP ISO to boot to the "Welcome to Setup" screen as before.
>
>
> ATB,
>
> Mark.

Thanks for the detailed report, and sorry for breaking your testcase.

I managed to reproduce locally.

Could you check if this patch fixes your issue and the rest of your testcases?
If so, I will prepare a proper patchset.

Thanks!

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4967aa7459..e7db51afba 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -741,7 +741,7 @@ static Aml *build_prt(bool is_pci0_prt)
     int pin;

     method = aml_method("_PRT", 0, AML_NOTSERIALIZED);
-    rt_pkg = aml_varpackage(nroutes);
+    rt_pkg = aml_package(nroutes);

     for (pin = 0; pin < nroutes; pin++) {
         Aml *pkg = aml_package(4);



>
> >> Changelog v3->v4 Thanks Richard:
> >> - Make link_name struct static
> >>
> >> Changelog v3->v4 Thanks Igor:
> >> - Add missing files to tests/qtest/bios-tables-test-allowed-diff.h
> >>
> >> Changelog v2->v3 Thanks Michael:
> >> - Code style
> >> - Add cover letter
> >>
> >> Ricardo Ribalda (3):
> >>    tests/acpi: pc: allow DSDT acpi table changes
> >>    hw/i386/acpi-build: Return a pre-computed _PRT table
> >>    tests/acpi: pc: update golden masters for DSDT
> >>
> >>   hw/i386/acpi-build.c                 | 120 +++++----------------------
> >>   tests/data/acpi/x86/pc/DSDT          | Bin 6830 -> 8527 bytes
> >>   tests/data/acpi/x86/pc/DSDT.acpierst | Bin 6741 -> 8438 bytes
> >>   tests/data/acpi/x86/pc/DSDT.acpihmat | Bin 8155 -> 9852 bytes
> >>   tests/data/acpi/x86/pc/DSDT.bridge   | Bin 13701 -> 15398 bytes
> >>   tests/data/acpi/x86/pc/DSDT.cphp     | Bin 7294 -> 8991 bytes
> >>   tests/data/acpi/x86/pc/DSDT.dimmpxm  | Bin 8484 -> 10181 bytes
> >>   tests/data/acpi/x86/pc/DSDT.hpbridge | Bin 6781 -> 8478 bytes
> >>   tests/data/acpi/x86/pc/DSDT.hpbrroot | Bin 3337 -> 5034 bytes
> >>   tests/data/acpi/x86/pc/DSDT.ipmikcs  | Bin 6902 -> 8599 bytes
> >>   tests/data/acpi/x86/pc/DSDT.memhp    | Bin 8189 -> 9886 bytes
> >>   tests/data/acpi/x86/pc/DSDT.nohpet   | Bin 6688 -> 8385 bytes
> >>   tests/data/acpi/x86/pc/DSDT.numamem  | Bin 6836 -> 8533 bytes
> >>   tests/data/acpi/x86/pc/DSDT.roothp   | Bin 10623 -> 12320 bytes
> >>   tests/data/acpi/x86/q35/DSDT.cxl     | Bin 9714 -> 13148 bytes
> >>   tests/data/acpi/x86/q35/DSDT.viot    | Bin 9464 -> 14615 bytes
> >>   16 files changed, 22 insertions(+), 98 deletions(-)
>


-- 
Ricardo Ribalda