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(-)
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. 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(-) -- 2.46.0.76.ge559c4bf1a-goog
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 > 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(-) >
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.
>> 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(-)
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
On 22/09/2024 14:11, Ricardo Ribalda wrote:
> 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);
Hi Ricardo,
I can confirm that the above patch fixes my particular test case here - thanks so
much for the quick reply!
ATB,
Mark.
>>>> 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(-)
© 2016 - 2024 Red Hat, Inc.