qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] ACPI: i386: bump MADT to revision 3
@ 2023-05-15 20:33 Eric DeVolder
  2023-05-15 20:33 ` [PATCH 1/3] ACPI: bios-tables-test.c step 2 (allowed-diff entries) Eric DeVolder
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Eric DeVolder @ 2023-05-15 20:33 UTC (permalink / raw)
  To: qemu-devel, shannon.zhaosl, mst, imammedo, ani, peter.maydell,
	qemu-arm, marcel.apfelbaum, pbonzini, richard.henderson, eduardo
  Cc: boris.ostrovsky, miguel.luis, eric.devolder

The following Linux kernel change broke CPU hotplug for MADT revision
less than 5.

 e2869bd7af60 ("x86/acpi/boot: Do not register processors that cannot be onlined for x2APIC")

Discussion on this topic can be located here:

 https://lore.kernel.org/linux-acpi/20230327191026.3454-1-eric.devolder@oracle.com/T/#t

which resulted in the following fixes in Linux 6.3-rc5:

 a74fabfbd1b7: ("x86/ACPI/boot: Use FADT version to check support for online capable")
 fed8d8773b8e: ("x86/acpi/boot: Correct acpi_is_processor_usable() check")

However, as part of the investigation into resolving this breakage, I
learned that i386 QEMU reports revision 1, while technically it
generates revision 3.

Below is a table summarizing the changes to the MADT. This information
gleamed from the ACPI specs on uefi.org.

ACPI    MADT    What
Version Version
1.0             MADT not present
2.0     1       Section 5.2.10.4
3.0     2       Section 5.2.11.4
                 5.2.11.13 Local SAPIC Structure added two new fields:
                  ACPI Processor UID Value
                  ACPI Processor UID String
                 5.2.10.14 Platform Interrupt Sources Structure:
                  Reserved changed to Platform Interrupt Sources Flags
3.0b    2       Section 5.2.11.4
                 Added a section describing guidelines for the ordering of
                 processors in the MADT to support proper boot processor
                 and multi-threaded logical processor operation.
4.0     3       Section 5.2.12
                 Adds Processor Local x2APIC structure type 9
                 Adds Local x2APIC NMI structure type 0xA
5.0     3       Section 5.2.12
6.0     3       Section 5.2.12
6.0a    4       Section 5.2.12
                 Adds ARM GIC structure types 0xB-0xF
6.2a    45      Section 5.2.12   <--- version 45, is indeed accurate!
6.2b    5       Section 5.2.12
                 GIC ITS last Reserved offset changed to 16 from 20 (typo)
6.3     5       Section 5.2.12
                 Adds Local APIC Flags Online Capable!
                 Adds GICC SPE Overflow Interrupt field
6.4     5       Section 5.2.12
                 Adds Multiprocessor Wakeup Structure type 0x10
                 (change notes says structure previously misplaced?)
6.5     5       Section 5.2.12

For the MADT revision change 1 -> 2, the spec has a change to the
SAPIC structure. In general, QEMU does not generate/support SAPIC.
So the QEMU i386 MADT revision can safely be moved to 2.

For the MADT revision change 2 -> 3, the spec adds Local x2APIC
structures. QEMU has long supported x2apic ACPI structures.
So the QEMU i386 MADT revision can safely be moved to 3.

So, set the MADT revision to 3.

Regards,
Eric
---
Eric DeVolder (3):
  ACPI: bios-tables-test.c step 2 (allowed-diff entries)
  ACPI: i386: bump to MADT to revision 3
  ACPI: bios-tables-test.c step 5 (update expected table binaries)

 hw/i386/acpi-common.c                         |   2 +-
 tests/data/acpi/microvm/APIC                  | Bin 70 -> 70 bytes
 tests/data/acpi/microvm/APIC.ioapic2          | Bin 82 -> 82 bytes
 tests/data/acpi/microvm/APIC.pcie             | Bin 110 -> 110 bytes
 tests/data/acpi/pc/APIC                       | Bin 120 -> 120 bytes
 tests/data/acpi/pc/APIC.acpihmat              | Bin 128 -> 128 bytes
 tests/data/acpi/pc/APIC.cphp                  | Bin 160 -> 160 bytes
 tests/data/acpi/pc/APIC.dimmpxm               | Bin 144 -> 144 bytes
 tests/data/acpi/q35/APIC                      | Bin 120 -> 120 bytes
 tests/data/acpi/q35/APIC.acpihmat             | Bin 128 -> 128 bytes
 tests/data/acpi/q35/APIC.acpihmat-noinitiator | Bin 144 -> 144 bytes
 tests/data/acpi/q35/APIC.core-count2          | Bin 2478 -> 2478 bytes
 tests/data/acpi/q35/APIC.cphp                 | Bin 160 -> 160 bytes
 tests/data/acpi/q35/APIC.dimmpxm              | Bin 144 -> 144 bytes
 tests/data/acpi/q35/APIC.xapic                | Bin 2686 -> 2686 bytes
 15 files changed, 1 insertion(+), 1 deletion(-)

-- 
2.31.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] ACPI: bios-tables-test.c step 2 (allowed-diff entries)
  2023-05-15 20:33 [PATCH 0/3] ACPI: i386: bump MADT to revision 3 Eric DeVolder
@ 2023-05-15 20:33 ` Eric DeVolder
  2023-05-16  7:25   ` Ani Sinha
  2023-05-15 20:33 ` [PATCH 2/3] ACPI: i386: bump to MADT to revision 3 Eric DeVolder
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Eric DeVolder @ 2023-05-15 20:33 UTC (permalink / raw)
  To: qemu-devel, shannon.zhaosl, mst, imammedo, ani, peter.maydell,
	qemu-arm, marcel.apfelbaum, pbonzini, richard.henderson, eduardo
  Cc: boris.ostrovsky, miguel.luis, eric.devolder

Following the guidelines in tests/qtest/bios-tables-test.c,
set up bios-tables-test-allowed-diff.h to exclude the
imminent changes to the APIC tables, per step 2.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..1e5e354ecf 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,5 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/pc/APIC",
+"tests/data/acpi/q35/APIC",
+"tests/data/acpi/microvm/APIC",
+"tests/data/acpi/virt/APIC",
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] ACPI: i386: bump to MADT to revision 3
  2023-05-15 20:33 [PATCH 0/3] ACPI: i386: bump MADT to revision 3 Eric DeVolder
  2023-05-15 20:33 ` [PATCH 1/3] ACPI: bios-tables-test.c step 2 (allowed-diff entries) Eric DeVolder
@ 2023-05-15 20:33 ` Eric DeVolder
  2023-05-16  7:25   ` Ani Sinha
  2023-05-16 12:31   ` Igor Mammedov
  2023-05-15 20:33 ` [PATCH 3/3] ACPI: bios-tables-test.c step 5 (update expected table binaries) Eric DeVolder
  2023-05-16  7:39 ` [PATCH 0/3] ACPI: i386: bump MADT to revision 3 Ani Sinha
  3 siblings, 2 replies; 12+ messages in thread
From: Eric DeVolder @ 2023-05-15 20:33 UTC (permalink / raw)
  To: qemu-devel, shannon.zhaosl, mst, imammedo, ani, peter.maydell,
	qemu-arm, marcel.apfelbaum, pbonzini, richard.henderson, eduardo
  Cc: boris.ostrovsky, miguel.luis, eric.devolder

Currently i386 QEMU generates MADT revision 3, and reports
MADT revision 1. Set .revision to 3 to match reality.

Link: https://lore.kernel.org/linux-acpi/20230327191026.3454-1-eric.devolder@ora
cle.com/T/#t
Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 hw/i386/acpi-common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
index 52e5c1439a..8a0932fe84 100644
--- a/hw/i386/acpi-common.c
+++ b/hw/i386/acpi-common.c
@@ -102,7 +102,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
     MachineClass *mc = MACHINE_GET_CLASS(x86ms);
     const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
     AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
-    AcpiTable table = { .sig = "APIC", .rev = 1, .oem_id = oem_id,
+    AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = oem_id,
                         .oem_table_id = oem_table_id };
 
     acpi_table_begin(&table, table_data);
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] ACPI: bios-tables-test.c step 5 (update expected table binaries)
  2023-05-15 20:33 [PATCH 0/3] ACPI: i386: bump MADT to revision 3 Eric DeVolder
  2023-05-15 20:33 ` [PATCH 1/3] ACPI: bios-tables-test.c step 2 (allowed-diff entries) Eric DeVolder
  2023-05-15 20:33 ` [PATCH 2/3] ACPI: i386: bump to MADT to revision 3 Eric DeVolder
@ 2023-05-15 20:33 ` Eric DeVolder
  2023-05-16  7:28   ` Ani Sinha
  2023-05-16  7:39 ` [PATCH 0/3] ACPI: i386: bump MADT to revision 3 Ani Sinha
  3 siblings, 1 reply; 12+ messages in thread
From: Eric DeVolder @ 2023-05-15 20:33 UTC (permalink / raw)
  To: qemu-devel, shannon.zhaosl, mst, imammedo, ani, peter.maydell,
	qemu-arm, marcel.apfelbaum, pbonzini, richard.henderson, eduardo
  Cc: boris.ostrovsky, miguel.luis, eric.devolder

Following the guidelines in tests/qtest/bios-tables-test.c, this
is step 5 and 6.

The MADT/APIC table diffs show (for pc, q35 and microvm) bumping
revision from 1 to 3 (and checksum changing accordingly):

 Using expected file 'tests/data/acpi/pc/DSDT'
 acpi-test: Warning! APIC binary file mismatch. Actual [aml:/tmp/aml-9UKC51], Expected [aml:tests/data/acpi/pc/APIC].
 See source file tests/qtest/bios-tables-test.c for instructions on how to update expected files.
 acpi-test: Warning! APIC mismatch. Actual [asl:/tmp/asl-CFKC51.dsl, aml:/tmp/aml-9UKC51], Expected [asl:/tmp/asl-TMFC51.dsl, aml:tests/data/acpi/pc/APIC].
 --- /tmp/asl-TMFC51.dsl	2023-05-15 14:15:26.599183824 -0400
 +++ /tmp/asl-CFKC51.dsl	2023-05-15 14:15:26.598183818 -0400
 @@ -1,32 +1,32 @@
  /*
   * Intel ACPI Component Architecture
   * AML/ASL+ Disassembler version 20230331 (64-bit version)
   * Copyright (c) 2000 - 2023 Intel Corporation
   *
 - * Disassembly of tests/data/acpi/pc/APIC, Mon May 15 14:15:26 2023
 + * Disassembly of /tmp/aml-9UKC51, Mon May 15 14:15:26 2023
   *
   * ACPI Data Table [APIC]
   *
   * Format: [HexOffset DecimalOffset ByteLength]  FieldName : FieldValue (in hex)
   */

  [000h 0000 004h]                   Signature : "APIC"    [Multiple APIC Description Table (MADT)]
  [004h 0004 004h]                Table Length : 00000078
 -[008h 0008 001h]                    Revision : 01
 -[009h 0009 001h]                    Checksum : 8A
 +[008h 0008 001h]                    Revision : 03
 +[009h 0009 001h]                    Checksum : 88
  [00Ah 0010 006h]                      Oem ID : "BOCHS "
  [010h 0016 008h]                Oem Table ID : "BXPC    "
  [018h 0024 004h]                Oem Revision : 00000001
  [01Ch 0028 004h]             Asl Compiler ID : "BXPC"
  [020h 0032 004h]       Asl Compiler Revision : 00000001

  [024h 0036 004h]          Local Apic Address : FEE00000
  [028h 0040 004h]       Flags (decoded below) : 00000001
                           PC-AT Compatibility : 1

  [02Ch 0044 001h]               Subtable Type : 00 [Processor Local APIC]
  [02Dh 0045 001h]                      Length : 08
  [02Eh 0046 001h]                Processor ID : 00
  [02Fh 0047 001h]               Local Apic ID : 00
  [030h 0048 004h]       Flags (decoded below) : 00000001
                             Processor Enabled : 1
 @@ -81,24 +81,24 @@
  [06Bh 0107 001h]                      Source : 0B
  [06Ch 0108 004h]                   Interrupt : 0000000B
  [070h 0112 002h]       Flags (decoded below) : 000D
                                      Polarity : 1
                                  Trigger Mode : 3

  [072h 0114 001h]               Subtable Type : 04 [Local APIC NMI]
  [073h 0115 001h]                      Length : 06
  [074h 0116 001h]                Processor ID : FF
  [075h 0117 002h]       Flags (decoded below) : 0000
                                      Polarity : 0
                                  Trigger Mode : 0
  [077h 0119 001h]        Interrupt Input LINT : 01

  Raw Table Data: Length 120 (0x78)

 -    0000: 41 50 49 43 78 00 00 00 01 8A 42 4F 43 48 53 20  // APICx.....BOCHS
 +    0000: 41 50 49 43 78 00 00 00 03 88 42 4F 43 48 53 20  // APICx.....BOCHS
      0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43  // BXPC    ....BXPC
      0020: 01 00 00 00 00 00 E0 FE 01 00 00 00 00 08 00 00  // ................
      0030: 01 00 00 00 01 0C 00 00 00 00 C0 FE 00 00 00 00  // ................
      0040: 02 0A 00 00 02 00 00 00 00 00 02 0A 00 05 05 00  // ................
      0050: 00 00 0D 00 02 0A 00 09 09 00 00 00 0D 00 02 0A  // ................
      0060: 00 0A 0A 00 00 00 0D 00 02 0A 00 0B 0B 00 00 00  // ................
      0070: 0D 00 04 06 FF 00 00 01                          // ........

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 tests/data/acpi/microvm/APIC                  | Bin 70 -> 70 bytes
 tests/data/acpi/microvm/APIC.ioapic2          | Bin 82 -> 82 bytes
 tests/data/acpi/microvm/APIC.pcie             | Bin 110 -> 110 bytes
 tests/data/acpi/pc/APIC                       | Bin 120 -> 120 bytes
 tests/data/acpi/pc/APIC.acpihmat              | Bin 128 -> 128 bytes
 tests/data/acpi/pc/APIC.cphp                  | Bin 160 -> 160 bytes
 tests/data/acpi/pc/APIC.dimmpxm               | Bin 144 -> 144 bytes
 tests/data/acpi/q35/APIC                      | Bin 120 -> 120 bytes
 tests/data/acpi/q35/APIC.acpihmat             | Bin 128 -> 128 bytes
 tests/data/acpi/q35/APIC.acpihmat-noinitiator | Bin 144 -> 144 bytes
 tests/data/acpi/q35/APIC.core-count2          | Bin 2478 -> 2478 bytes
 tests/data/acpi/q35/APIC.cphp                 | Bin 160 -> 160 bytes
 tests/data/acpi/q35/APIC.dimmpxm              | Bin 144 -> 144 bytes
 tests/data/acpi/q35/APIC.xapic                | Bin 2686 -> 2686 bytes
 tests/qtest/bios-tables-test-allowed-diff.h   |   4 ----
 15 files changed, 4 deletions(-)

diff --git a/tests/data/acpi/microvm/APIC b/tests/data/acpi/microvm/APIC
index 68dbd44a7e35a356083f086df60f70e424c4249f..672764e711d80402890902ba9ded10915770e84c 100644
GIT binary patch
delta 16
XcmZ>B<8ln}barE4U|=qq$Ylcn95e$)

delta 16
XcmZ>B<8ln}barE4U|=kn$Ylcn95Mq&

diff --git a/tests/data/acpi/microvm/APIC.ioapic2 b/tests/data/acpi/microvm/APIC.ioapic2
index 3063c52cd3e9bbed29c06031b375900f4a49b9e0..6f24fdb12ce3f1c13df7ff835e475d8023e20d4a 100644
GIT binary patch
delta 16
XcmWFv;&Ke|bPi%*U|?>X$mIb59$W*3

delta 16
XcmWFv;&Ke|bPi%*U|?*X$mIb59$Ev1

diff --git a/tests/data/acpi/microvm/APIC.pcie b/tests/data/acpi/microvm/APIC.pcie
index 4e8f6ed8d6a866429fc17aecdeafc3fb5ef65fa3..2239ca76a607fb1ff9d392298e2bd6461bba7ecf 100644
GIT binary patch
delta 16
Xcmd1H<8ln}bk1X7U|_DA$dv*BBD@3c

delta 16
Xcmd1H<8ln}bk1X7U|_77$dv*BBDw?a

diff --git a/tests/data/acpi/pc/APIC b/tests/data/acpi/pc/APIC
index 208331db53b7dd5c6205cce0e95427636b86dd64..868a3432f0295257393e45b75483ef4bec455d74 100644
GIT binary patch
delta 16
Xcmb=Z;BpM`bgp1vU|{Z;$dv~GB#s0m

delta 16
Xcmb=Z;BpM`bgp1vU|{T;$dv~GB#Z<k

diff --git a/tests/data/acpi/pc/APIC.acpihmat b/tests/data/acpi/pc/APIC.acpihmat
index 812c4603f2701494f6bb761570323158a20d4043..125d1ff0871f772bc8cfe3e2afbff70edf221291 100644
GIT binary patch
delta 18
ZcmZo*Y+&Sa4DfVrU|?WiE}h6#1^_241Tz2t

delta 18
ZcmZo*Y+&Sa4DfVrU|?WiET70#1^_221Tz2t

diff --git a/tests/data/acpi/pc/APIC.cphp b/tests/data/acpi/pc/APIC.cphp
index 65cc4f4a9aa2676140a6525cdac1e838274b1e07..a2c2a24e5e3cf143b57a8932f78eeda6d7b8bbdb 100644
GIT binary patch
delta 18
ZcmZ3$xPXz%F~HM#0RsaAv)DwgX#guQ1XKV3

delta 18
ZcmZ3$xPXz%F~HM#0RsaAqr^n6X#guO1XKV3

diff --git a/tests/data/acpi/pc/APIC.dimmpxm b/tests/data/acpi/pc/APIC.dimmpxm
index d904d4a70ddecbb79a83a267af8e26f925e9f4c6..9b5922bc72db1fe64819a3970d6ca95543da799e 100644
GIT binary patch
delta 18
ZcmbQhIDwJNF~HM#0s{jBv*$#vHUKF+1V;b>

delta 18
ZcmbQhIDwJNF~HM#0s{jBqxVFvHUKF)1V;b>

diff --git a/tests/data/acpi/q35/APIC b/tests/data/acpi/q35/APIC
index 208331db53b7dd5c6205cce0e95427636b86dd64..868a3432f0295257393e45b75483ef4bec455d74 100644
GIT binary patch
delta 16
Xcmb=Z;BpM`bgp1vU|{Z;$dv~GB#s0m

delta 16
Xcmb=Z;BpM`bgp1vU|{T;$dv~GB#Z<k

diff --git a/tests/data/acpi/q35/APIC.acpihmat b/tests/data/acpi/q35/APIC.acpihmat
index 812c4603f2701494f6bb761570323158a20d4043..125d1ff0871f772bc8cfe3e2afbff70edf221291 100644
GIT binary patch
delta 18
ZcmZo*Y+&Sa4DfVrU|?WiE}h6#1^_241Tz2t

delta 18
ZcmZo*Y+&Sa4DfVrU|?WiET70#1^_221Tz2t

diff --git a/tests/data/acpi/q35/APIC.acpihmat-noinitiator b/tests/data/acpi/q35/APIC.acpihmat-noinitiator
index d904d4a70ddecbb79a83a267af8e26f925e9f4c6..9b5922bc72db1fe64819a3970d6ca95543da799e 100644
GIT binary patch
delta 18
ZcmbQhIDwJNF~HM#0s{jBv*$#vHUKF+1V;b>

delta 18
ZcmbQhIDwJNF~HM#0s{jBqxVFvHUKF)1V;b>

diff --git a/tests/data/acpi/q35/APIC.core-count2 b/tests/data/acpi/q35/APIC.core-count2
index a255082ef5bc39f0d92d3e372b91f09dd6d0d9a1..f5da2eb1e8a93d961b39f665f2e8b02acf1aeb3c 100644
GIT binary patch
delta 19
acmZ1{yiS<QF~HM#9VY_=^SO;&OE>{I`URQ*

delta 19
acmZ1{yiS<QF~HM#9VY_=<Ase}OE>{I_yw8(

diff --git a/tests/data/acpi/q35/APIC.cphp b/tests/data/acpi/q35/APIC.cphp
index 65cc4f4a9aa2676140a6525cdac1e838274b1e07..a2c2a24e5e3cf143b57a8932f78eeda6d7b8bbdb 100644
GIT binary patch
delta 18
ZcmZ3$xPXz%F~HM#0RsaAv)DwgX#guQ1XKV3

delta 18
ZcmZ3$xPXz%F~HM#0RsaAqr^n6X#guO1XKV3

diff --git a/tests/data/acpi/q35/APIC.dimmpxm b/tests/data/acpi/q35/APIC.dimmpxm
index d904d4a70ddecbb79a83a267af8e26f925e9f4c6..9b5922bc72db1fe64819a3970d6ca95543da799e 100644
GIT binary patch
delta 18
ZcmbQhIDwJNF~HM#0s{jBv*$#vHUKF+1V;b>

delta 18
ZcmbQhIDwJNF~HM#0s{jBqxVFvHUKF)1V;b>

diff --git a/tests/data/acpi/q35/APIC.xapic b/tests/data/acpi/q35/APIC.xapic
index c1969c35aa12b61d25e0134bbb8d2187ba42d663..83bd28325af9d6d7619015a9701866b8f3f1d754 100644
GIT binary patch
delta 19
acmew-@=t`zF~HNgj*EeTS#2X%2^Ro9-UT)Q

delta 19
acmew-@=t`zF~HNgj*EeTQDY-l2^Ro9+yyoO

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 1e5e354ecf..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,5 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/pc/APIC",
-"tests/data/acpi/q35/APIC",
-"tests/data/acpi/microvm/APIC",
-"tests/data/acpi/virt/APIC",
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] ACPI: bios-tables-test.c step 2 (allowed-diff entries)
  2023-05-15 20:33 ` [PATCH 1/3] ACPI: bios-tables-test.c step 2 (allowed-diff entries) Eric DeVolder
@ 2023-05-16  7:25   ` Ani Sinha
  0 siblings, 0 replies; 12+ messages in thread
From: Ani Sinha @ 2023-05-16  7:25 UTC (permalink / raw)
  To: Eric DeVolder
  Cc: qemu-devel, shannon.zhaosl, mst, imammedo, peter.maydell,
	qemu-arm, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	boris.ostrovsky, miguel.luis

On Tue, May 16, 2023 at 2:03 AM Eric DeVolder <eric.devolder@oracle.com> wrote:
>
> Following the guidelines in tests/qtest/bios-tables-test.c,
> set up bios-tables-test-allowed-diff.h to exclude the
> imminent changes to the APIC tables, per step 2.
>
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>

Acked-by: Ani Sinha <anisinha@redhat.com>

> ---
>  tests/qtest/bios-tables-test-allowed-diff.h | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8b..1e5e354ecf 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,5 @@
>  /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/pc/APIC",
> +"tests/data/acpi/q35/APIC",
> +"tests/data/acpi/microvm/APIC",
> +"tests/data/acpi/virt/APIC",
> --
> 2.31.1
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] ACPI: i386: bump to MADT to revision 3
  2023-05-15 20:33 ` [PATCH 2/3] ACPI: i386: bump to MADT to revision 3 Eric DeVolder
@ 2023-05-16  7:25   ` Ani Sinha
  2023-05-16 12:31   ` Igor Mammedov
  1 sibling, 0 replies; 12+ messages in thread
From: Ani Sinha @ 2023-05-16  7:25 UTC (permalink / raw)
  To: Eric DeVolder
  Cc: qemu-devel, shannon.zhaosl, mst, imammedo, peter.maydell,
	qemu-arm, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	boris.ostrovsky, miguel.luis

On Tue, May 16, 2023 at 2:04 AM Eric DeVolder <eric.devolder@oracle.com> wrote:
>
> Currently i386 QEMU generates MADT revision 3, and reports
> MADT revision 1. Set .revision to 3 to match reality.
>
> Link: https://lore.kernel.org/linux-acpi/20230327191026.3454-1-eric.devolder@ora
> cle.com/T/#t
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>

Reviewed-by: Ani Sinha <anisinha@redhat.com>

> ---
>  hw/i386/acpi-common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> index 52e5c1439a..8a0932fe84 100644
> --- a/hw/i386/acpi-common.c
> +++ b/hw/i386/acpi-common.c
> @@ -102,7 +102,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
>      MachineClass *mc = MACHINE_GET_CLASS(x86ms);
>      const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
>      AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
> -    AcpiTable table = { .sig = "APIC", .rev = 1, .oem_id = oem_id,
> +    AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = oem_id,
>                          .oem_table_id = oem_table_id };
>
>      acpi_table_begin(&table, table_data);
> --
> 2.31.1
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] ACPI: bios-tables-test.c step 5 (update expected table binaries)
  2023-05-15 20:33 ` [PATCH 3/3] ACPI: bios-tables-test.c step 5 (update expected table binaries) Eric DeVolder
@ 2023-05-16  7:28   ` Ani Sinha
  0 siblings, 0 replies; 12+ messages in thread
From: Ani Sinha @ 2023-05-16  7:28 UTC (permalink / raw)
  To: Eric DeVolder
  Cc: qemu-devel, shannon.zhaosl, mst, imammedo, peter.maydell,
	qemu-arm, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	boris.ostrovsky, miguel.luis

On Tue, May 16, 2023 at 2:03 AM Eric DeVolder <eric.devolder@oracle.com> wrote:
>
> Following the guidelines in tests/qtest/bios-tables-test.c, this
> is step 5 and 6.
>
> The MADT/APIC table diffs show (for pc, q35 and microvm) bumping
> revision from 1 to 3 (and checksum changing accordingly):
>
>  Using expected file 'tests/data/acpi/pc/DSDT'
>  acpi-test: Warning! APIC binary file mismatch. Actual [aml:/tmp/aml-9UKC51], Expected [aml:tests/data/acpi/pc/APIC].
>  See source file tests/qtest/bios-tables-test.c for instructions on how to update expected files.
>  acpi-test: Warning! APIC mismatch. Actual [asl:/tmp/asl-CFKC51.dsl, aml:/tmp/aml-9UKC51], Expected [asl:/tmp/asl-TMFC51.dsl, aml:tests/data/acpi/pc/APIC].
>  --- /tmp/asl-TMFC51.dsl        2023-05-15 14:15:26.599183824 -0400
>  +++ /tmp/asl-CFKC51.dsl        2023-05-15 14:15:26.598183818 -0400
>  @@ -1,32 +1,32 @@
>   /*
>    * Intel ACPI Component Architecture
>    * AML/ASL+ Disassembler version 20230331 (64-bit version)
>    * Copyright (c) 2000 - 2023 Intel Corporation
>    *
>  - * Disassembly of tests/data/acpi/pc/APIC, Mon May 15 14:15:26 2023
>  + * Disassembly of /tmp/aml-9UKC51, Mon May 15 14:15:26 2023
>    *
>    * ACPI Data Table [APIC]
>    *
>    * Format: [HexOffset DecimalOffset ByteLength]  FieldName : FieldValue (in hex)
>    */
>
>   [000h 0000 004h]                   Signature : "APIC"    [Multiple APIC Description Table (MADT)]
>   [004h 0004 004h]                Table Length : 00000078
>  -[008h 0008 001h]                    Revision : 01
>  -[009h 0009 001h]                    Checksum : 8A
>  +[008h 0008 001h]                    Revision : 03
>  +[009h 0009 001h]                    Checksum : 88
>   [00Ah 0010 006h]                      Oem ID : "BOCHS "
>   [010h 0016 008h]                Oem Table ID : "BXPC    "
>   [018h 0024 004h]                Oem Revision : 00000001
>   [01Ch 0028 004h]             Asl Compiler ID : "BXPC"
>   [020h 0032 004h]       Asl Compiler Revision : 00000001
>
>   [024h 0036 004h]          Local Apic Address : FEE00000
>   [028h 0040 004h]       Flags (decoded below) : 00000001
>                            PC-AT Compatibility : 1
>
>   [02Ch 0044 001h]               Subtable Type : 00 [Processor Local APIC]
>   [02Dh 0045 001h]                      Length : 08
>   [02Eh 0046 001h]                Processor ID : 00
>   [02Fh 0047 001h]               Local Apic ID : 00
>   [030h 0048 004h]       Flags (decoded below) : 00000001
>                              Processor Enabled : 1
>  @@ -81,24 +81,24 @@
>   [06Bh 0107 001h]                      Source : 0B
>   [06Ch 0108 004h]                   Interrupt : 0000000B
>   [070h 0112 002h]       Flags (decoded below) : 000D
>                                       Polarity : 1
>                                   Trigger Mode : 3
>
>   [072h 0114 001h]               Subtable Type : 04 [Local APIC NMI]
>   [073h 0115 001h]                      Length : 06
>   [074h 0116 001h]                Processor ID : FF
>   [075h 0117 002h]       Flags (decoded below) : 0000
>                                       Polarity : 0
>                                   Trigger Mode : 0
>   [077h 0119 001h]        Interrupt Input LINT : 01
>
>   Raw Table Data: Length 120 (0x78)
>
>  -    0000: 41 50 49 43 78 00 00 00 01 8A 42 4F 43 48 53 20  // APICx.....BOCHS
>  +    0000: 41 50 49 43 78 00 00 00 03 88 42 4F 43 48 53 20  // APICx.....BOCHS
>       0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43  // BXPC    ....BXPC
>       0020: 01 00 00 00 00 00 E0 FE 01 00 00 00 00 08 00 00  // ................
>       0030: 01 00 00 00 01 0C 00 00 00 00 C0 FE 00 00 00 00  // ................
>       0040: 02 0A 00 00 02 00 00 00 00 00 02 0A 00 05 05 00  // ................
>       0050: 00 00 0D 00 02 0A 00 09 09 00 00 00 0D 00 02 0A  // ................
>       0060: 00 0A 0A 00 00 00 0D 00 02 0A 00 0B 0B 00 00 00  // ................
>       0070: 0D 00 04 06 FF 00 00 01                          // ........
>
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>

Acked-by: Ani Sinha <anisinha@redhat.com>

> ---
>  tests/data/acpi/microvm/APIC                  | Bin 70 -> 70 bytes
>  tests/data/acpi/microvm/APIC.ioapic2          | Bin 82 -> 82 bytes
>  tests/data/acpi/microvm/APIC.pcie             | Bin 110 -> 110 bytes
>  tests/data/acpi/pc/APIC                       | Bin 120 -> 120 bytes
>  tests/data/acpi/pc/APIC.acpihmat              | Bin 128 -> 128 bytes
>  tests/data/acpi/pc/APIC.cphp                  | Bin 160 -> 160 bytes
>  tests/data/acpi/pc/APIC.dimmpxm               | Bin 144 -> 144 bytes
>  tests/data/acpi/q35/APIC                      | Bin 120 -> 120 bytes
>  tests/data/acpi/q35/APIC.acpihmat             | Bin 128 -> 128 bytes
>  tests/data/acpi/q35/APIC.acpihmat-noinitiator | Bin 144 -> 144 bytes
>  tests/data/acpi/q35/APIC.core-count2          | Bin 2478 -> 2478 bytes
>  tests/data/acpi/q35/APIC.cphp                 | Bin 160 -> 160 bytes
>  tests/data/acpi/q35/APIC.dimmpxm              | Bin 144 -> 144 bytes
>  tests/data/acpi/q35/APIC.xapic                | Bin 2686 -> 2686 bytes
>  tests/qtest/bios-tables-test-allowed-diff.h   |   4 ----
>  15 files changed, 4 deletions(-)
>
> diff --git a/tests/data/acpi/microvm/APIC b/tests/data/acpi/microvm/APIC
> index 68dbd44a7e35a356083f086df60f70e424c4249f..672764e711d80402890902ba9ded10915770e84c 100644
> GIT binary patch
> delta 16
> XcmZ>B<8ln}barE4U|=qq$Ylcn95e$)
>
> delta 16
> XcmZ>B<8ln}barE4U|=kn$Ylcn95Mq&
>
> diff --git a/tests/data/acpi/microvm/APIC.ioapic2 b/tests/data/acpi/microvm/APIC.ioapic2
> index 3063c52cd3e9bbed29c06031b375900f4a49b9e0..6f24fdb12ce3f1c13df7ff835e475d8023e20d4a 100644
> GIT binary patch
> delta 16
> XcmWFv;&Ke|bPi%*U|?>X$mIb59$W*3
>
> delta 16
> XcmWFv;&Ke|bPi%*U|?*X$mIb59$Ev1
>
> diff --git a/tests/data/acpi/microvm/APIC.pcie b/tests/data/acpi/microvm/APIC.pcie
> index 4e8f6ed8d6a866429fc17aecdeafc3fb5ef65fa3..2239ca76a607fb1ff9d392298e2bd6461bba7ecf 100644
> GIT binary patch
> delta 16
> Xcmd1H<8ln}bk1X7U|_DA$dv*BBD@3c
>
> delta 16
> Xcmd1H<8ln}bk1X7U|_77$dv*BBDw?a
>
> diff --git a/tests/data/acpi/pc/APIC b/tests/data/acpi/pc/APIC
> index 208331db53b7dd5c6205cce0e95427636b86dd64..868a3432f0295257393e45b75483ef4bec455d74 100644
> GIT binary patch
> delta 16
> Xcmb=Z;BpM`bgp1vU|{Z;$dv~GB#s0m
>
> delta 16
> Xcmb=Z;BpM`bgp1vU|{T;$dv~GB#Z<k
>
> diff --git a/tests/data/acpi/pc/APIC.acpihmat b/tests/data/acpi/pc/APIC.acpihmat
> index 812c4603f2701494f6bb761570323158a20d4043..125d1ff0871f772bc8cfe3e2afbff70edf221291 100644
> GIT binary patch
> delta 18
> ZcmZo*Y+&Sa4DfVrU|?WiE}h6#1^_241Tz2t
>
> delta 18
> ZcmZo*Y+&Sa4DfVrU|?WiET70#1^_221Tz2t
>
> diff --git a/tests/data/acpi/pc/APIC.cphp b/tests/data/acpi/pc/APIC.cphp
> index 65cc4f4a9aa2676140a6525cdac1e838274b1e07..a2c2a24e5e3cf143b57a8932f78eeda6d7b8bbdb 100644
> GIT binary patch
> delta 18
> ZcmZ3$xPXz%F~HM#0RsaAv)DwgX#guQ1XKV3
>
> delta 18
> ZcmZ3$xPXz%F~HM#0RsaAqr^n6X#guO1XKV3
>
> diff --git a/tests/data/acpi/pc/APIC.dimmpxm b/tests/data/acpi/pc/APIC.dimmpxm
> index d904d4a70ddecbb79a83a267af8e26f925e9f4c6..9b5922bc72db1fe64819a3970d6ca95543da799e 100644
> GIT binary patch
> delta 18
> ZcmbQhIDwJNF~HM#0s{jBv*$#vHUKF+1V;b>
>
> delta 18
> ZcmbQhIDwJNF~HM#0s{jBqxVFvHUKF)1V;b>
>
> diff --git a/tests/data/acpi/q35/APIC b/tests/data/acpi/q35/APIC
> index 208331db53b7dd5c6205cce0e95427636b86dd64..868a3432f0295257393e45b75483ef4bec455d74 100644
> GIT binary patch
> delta 16
> Xcmb=Z;BpM`bgp1vU|{Z;$dv~GB#s0m
>
> delta 16
> Xcmb=Z;BpM`bgp1vU|{T;$dv~GB#Z<k
>
> diff --git a/tests/data/acpi/q35/APIC.acpihmat b/tests/data/acpi/q35/APIC.acpihmat
> index 812c4603f2701494f6bb761570323158a20d4043..125d1ff0871f772bc8cfe3e2afbff70edf221291 100644
> GIT binary patch
> delta 18
> ZcmZo*Y+&Sa4DfVrU|?WiE}h6#1^_241Tz2t
>
> delta 18
> ZcmZo*Y+&Sa4DfVrU|?WiET70#1^_221Tz2t
>
> diff --git a/tests/data/acpi/q35/APIC.acpihmat-noinitiator b/tests/data/acpi/q35/APIC.acpihmat-noinitiator
> index d904d4a70ddecbb79a83a267af8e26f925e9f4c6..9b5922bc72db1fe64819a3970d6ca95543da799e 100644
> GIT binary patch
> delta 18
> ZcmbQhIDwJNF~HM#0s{jBv*$#vHUKF+1V;b>
>
> delta 18
> ZcmbQhIDwJNF~HM#0s{jBqxVFvHUKF)1V;b>
>
> diff --git a/tests/data/acpi/q35/APIC.core-count2 b/tests/data/acpi/q35/APIC.core-count2
> index a255082ef5bc39f0d92d3e372b91f09dd6d0d9a1..f5da2eb1e8a93d961b39f665f2e8b02acf1aeb3c 100644
> GIT binary patch
> delta 19
> acmZ1{yiS<QF~HM#9VY_=^SO;&OE>{I`URQ*
>
> delta 19
> acmZ1{yiS<QF~HM#9VY_=<Ase}OE>{I_yw8(
>
> diff --git a/tests/data/acpi/q35/APIC.cphp b/tests/data/acpi/q35/APIC.cphp
> index 65cc4f4a9aa2676140a6525cdac1e838274b1e07..a2c2a24e5e3cf143b57a8932f78eeda6d7b8bbdb 100644
> GIT binary patch
> delta 18
> ZcmZ3$xPXz%F~HM#0RsaAv)DwgX#guQ1XKV3
>
> delta 18
> ZcmZ3$xPXz%F~HM#0RsaAqr^n6X#guO1XKV3
>
> diff --git a/tests/data/acpi/q35/APIC.dimmpxm b/tests/data/acpi/q35/APIC.dimmpxm
> index d904d4a70ddecbb79a83a267af8e26f925e9f4c6..9b5922bc72db1fe64819a3970d6ca95543da799e 100644
> GIT binary patch
> delta 18
> ZcmbQhIDwJNF~HM#0s{jBv*$#vHUKF+1V;b>
>
> delta 18
> ZcmbQhIDwJNF~HM#0s{jBqxVFvHUKF)1V;b>
>
> diff --git a/tests/data/acpi/q35/APIC.xapic b/tests/data/acpi/q35/APIC.xapic
> index c1969c35aa12b61d25e0134bbb8d2187ba42d663..83bd28325af9d6d7619015a9701866b8f3f1d754 100644
> GIT binary patch
> delta 19
> acmew-@=t`zF~HNgj*EeTS#2X%2^Ro9-UT)Q
>
> delta 19
> acmew-@=t`zF~HNgj*EeTQDY-l2^Ro9+yyoO
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index 1e5e354ecf..dfb8523c8b 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1,5 +1 @@
>  /* List of comma-separated changed AML files to ignore */
> -"tests/data/acpi/pc/APIC",
> -"tests/data/acpi/q35/APIC",
> -"tests/data/acpi/microvm/APIC",
> -"tests/data/acpi/virt/APIC",
> --
> 2.31.1
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] ACPI: i386: bump MADT to revision 3
  2023-05-15 20:33 [PATCH 0/3] ACPI: i386: bump MADT to revision 3 Eric DeVolder
                   ` (2 preceding siblings ...)
  2023-05-15 20:33 ` [PATCH 3/3] ACPI: bios-tables-test.c step 5 (update expected table binaries) Eric DeVolder
@ 2023-05-16  7:39 ` Ani Sinha
  3 siblings, 0 replies; 12+ messages in thread
From: Ani Sinha @ 2023-05-16  7:39 UTC (permalink / raw)
  To: Eric DeVolder
  Cc: qemu-devel, shannon.zhaosl, mst, imammedo, peter.maydell,
	qemu-arm, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	boris.ostrovsky, miguel.luis

On Tue, May 16, 2023 at 2:03 AM Eric DeVolder <eric.devolder@oracle.com> wrote:
>
> The following Linux kernel change broke CPU hotplug for MADT revision
> less than 5.
>
>  e2869bd7af60 ("x86/acpi/boot: Do not register processors that cannot be onlined for x2APIC")
>
> Discussion on this topic can be located here:
>
>  https://lore.kernel.org/linux-acpi/20230327191026.3454-1-eric.devolder@oracle.com/T/#t
>
> which resulted in the following fixes in Linux 6.3-rc5:
>
>  a74fabfbd1b7: ("x86/ACPI/boot: Use FADT version to check support for online capable")
>  fed8d8773b8e: ("x86/acpi/boot: Correct acpi_is_processor_usable() check")
>
> However, as part of the investigation into resolving this breakage, I
> learned that i386 QEMU reports revision 1, while technically it
> generates revision 3.

Since this series was sent to my personal email and not my redhat
email, I suspect you have not rebased your patchset. Maybe you should
do that in case it generates any conflicts etc.

>
> Below is a table summarizing the changes to the MADT. This information
> gleamed from the ACPI specs on uefi.org.
>
> ACPI    MADT    What
> Version Version
> 1.0             MADT not present
> 2.0     1       Section 5.2.10.4
> 3.0     2       Section 5.2.11.4
>                  5.2.11.13 Local SAPIC Structure added two new fields:
>                   ACPI Processor UID Value
>                   ACPI Processor UID String
>                  5.2.10.14 Platform Interrupt Sources Structure:
>                   Reserved changed to Platform Interrupt Sources Flags
> 3.0b    2       Section 5.2.11.4
>                  Added a section describing guidelines for the ordering of
>                  processors in the MADT to support proper boot processor
>                  and multi-threaded logical processor operation.
> 4.0     3       Section 5.2.12
>                  Adds Processor Local x2APIC structure type 9
>                  Adds Local x2APIC NMI structure type 0xA
> 5.0     3       Section 5.2.12
> 6.0     3       Section 5.2.12
> 6.0a    4       Section 5.2.12
>                  Adds ARM GIC structure types 0xB-0xF
> 6.2a    45      Section 5.2.12   <--- version 45, is indeed accurate!
> 6.2b    5       Section 5.2.12
>                  GIC ITS last Reserved offset changed to 16 from 20 (typo)
> 6.3     5       Section 5.2.12
>                  Adds Local APIC Flags Online Capable!
>                  Adds GICC SPE Overflow Interrupt field
> 6.4     5       Section 5.2.12
>                  Adds Multiprocessor Wakeup Structure type 0x10
>                  (change notes says structure previously misplaced?)
> 6.5     5       Section 5.2.12
>
> For the MADT revision change 1 -> 2, the spec has a change to the
> SAPIC structure. In general, QEMU does not generate/support SAPIC.
> So the QEMU i386 MADT revision can safely be moved to 2.
>
> For the MADT revision change 2 -> 3, the spec adds Local x2APIC
> structures. QEMU has long supported x2apic ACPI structures.
> So the QEMU i386 MADT revision can safely be moved to 3.
>
> So, set the MADT revision to 3.
>
> Regards,
> Eric
> ---
> Eric DeVolder (3):
>   ACPI: bios-tables-test.c step 2 (allowed-diff entries)
>   ACPI: i386: bump to MADT to revision 3
>   ACPI: bios-tables-test.c step 5 (update expected table binaries)
>
>  hw/i386/acpi-common.c                         |   2 +-
>  tests/data/acpi/microvm/APIC                  | Bin 70 -> 70 bytes
>  tests/data/acpi/microvm/APIC.ioapic2          | Bin 82 -> 82 bytes
>  tests/data/acpi/microvm/APIC.pcie             | Bin 110 -> 110 bytes
>  tests/data/acpi/pc/APIC                       | Bin 120 -> 120 bytes
>  tests/data/acpi/pc/APIC.acpihmat              | Bin 128 -> 128 bytes
>  tests/data/acpi/pc/APIC.cphp                  | Bin 160 -> 160 bytes
>  tests/data/acpi/pc/APIC.dimmpxm               | Bin 144 -> 144 bytes
>  tests/data/acpi/q35/APIC                      | Bin 120 -> 120 bytes
>  tests/data/acpi/q35/APIC.acpihmat             | Bin 128 -> 128 bytes
>  tests/data/acpi/q35/APIC.acpihmat-noinitiator | Bin 144 -> 144 bytes
>  tests/data/acpi/q35/APIC.core-count2          | Bin 2478 -> 2478 bytes
>  tests/data/acpi/q35/APIC.cphp                 | Bin 160 -> 160 bytes
>  tests/data/acpi/q35/APIC.dimmpxm              | Bin 144 -> 144 bytes
>  tests/data/acpi/q35/APIC.xapic                | Bin 2686 -> 2686 bytes
>  15 files changed, 1 insertion(+), 1 deletion(-)
>
> --
> 2.31.1
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] ACPI: i386: bump to MADT to revision 3
  2023-05-15 20:33 ` [PATCH 2/3] ACPI: i386: bump to MADT to revision 3 Eric DeVolder
  2023-05-16  7:25   ` Ani Sinha
@ 2023-05-16 12:31   ` Igor Mammedov
  2023-05-16 12:51     ` Ani Sinha
  1 sibling, 1 reply; 12+ messages in thread
From: Igor Mammedov @ 2023-05-16 12:31 UTC (permalink / raw)
  To: Eric DeVolder
  Cc: qemu-devel, shannon.zhaosl, mst, ani, peter.maydell, qemu-arm,
	marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	boris.ostrovsky, miguel.luis

On Mon, 15 May 2023 16:33:10 -0400
Eric DeVolder <eric.devolder@oracle.com> wrote:

> Currently i386 QEMU generates MADT revision 3, and reports
> MADT revision 1. Set .revision to 3 to match reality.
> 
> Link: https://lore.kernel.org/linux-acpi/20230327191026.3454-1-eric.devolder@ora
> cle.com/T/#t
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  hw/i386/acpi-common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> index 52e5c1439a..8a0932fe84 100644
> --- a/hw/i386/acpi-common.c
> +++ b/hw/i386/acpi-common.c
> @@ -102,7 +102,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
>      MachineClass *mc = MACHINE_GET_CLASS(x86ms);
>      const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
>      AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
> -    AcpiTable table = { .sig = "APIC", .rev = 1, .oem_id = oem_id,
> +    AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = oem_id,
>                          .oem_table_id = oem_table_id };
>  
>      acpi_table_begin(&table, table_data);

make check fails for me at this point
(my guess is that not all APIC tables are whitelisted)



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] ACPI: i386: bump to MADT to revision 3
  2023-05-16 12:31   ` Igor Mammedov
@ 2023-05-16 12:51     ` Ani Sinha
  2023-05-16 21:22       ` Eric DeVolder
  0 siblings, 1 reply; 12+ messages in thread
From: Ani Sinha @ 2023-05-16 12:51 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Eric DeVolder, qemu-devel, shannon.zhaosl, mst, peter.maydell,
	qemu-arm, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	boris.ostrovsky, miguel.luis

On Tue, May 16, 2023 at 6:01 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Mon, 15 May 2023 16:33:10 -0400
> Eric DeVolder <eric.devolder@oracle.com> wrote:
>
> > Currently i386 QEMU generates MADT revision 3, and reports
> > MADT revision 1. Set .revision to 3 to match reality.
> >
> > Link: https://lore.kernel.org/linux-acpi/20230327191026.3454-1-eric.devolder@ora
> > cle.com/T/#t
> > Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> > ---
> >  hw/i386/acpi-common.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> > index 52e5c1439a..8a0932fe84 100644
> > --- a/hw/i386/acpi-common.c
> > +++ b/hw/i386/acpi-common.c
> > @@ -102,7 +102,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> >      MachineClass *mc = MACHINE_GET_CLASS(x86ms);
> >      const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
> >      AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
> > -    AcpiTable table = { .sig = "APIC", .rev = 1, .oem_id = oem_id,
> > +    AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = oem_id,
> >                          .oem_table_id = oem_table_id };
> >
> >      acpi_table_begin(&table, table_data);
>
> make check fails for me at this point
> (my guess is that not all APIC tables are whitelisted)

I think the patchset needs to be rebased and the blobs regenerated.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] ACPI: i386: bump to MADT to revision 3
  2023-05-16 12:51     ` Ani Sinha
@ 2023-05-16 21:22       ` Eric DeVolder
  2023-05-16 21:28         ` Michael S. Tsirkin
  0 siblings, 1 reply; 12+ messages in thread
From: Eric DeVolder @ 2023-05-16 21:22 UTC (permalink / raw)
  To: Ani Sinha, Igor Mammedov
  Cc: qemu-devel, shannon.zhaosl, mst, peter.maydell, qemu-arm,
	marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	boris.ostrovsky, miguel.luis



On 5/16/23 07:51, Ani Sinha wrote:
> On Tue, May 16, 2023 at 6:01 PM Igor Mammedov <imammedo@redhat.com> wrote:
>>
>> On Mon, 15 May 2023 16:33:10 -0400
>> Eric DeVolder <eric.devolder@oracle.com> wrote:
>>
>>> Currently i386 QEMU generates MADT revision 3, and reports
>>> MADT revision 1. Set .revision to 3 to match reality.
>>>
>>> Link: https://lore.kernel.org/linux-acpi/20230327191026.3454-1-eric.devolder@ora
>>> cle.com/T/#t
>>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>>> ---
>>>   hw/i386/acpi-common.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
>>> index 52e5c1439a..8a0932fe84 100644
>>> --- a/hw/i386/acpi-common.c
>>> +++ b/hw/i386/acpi-common.c
>>> @@ -102,7 +102,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
>>>       MachineClass *mc = MACHINE_GET_CLASS(x86ms);
>>>       const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
>>>       AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
>>> -    AcpiTable table = { .sig = "APIC", .rev = 1, .oem_id = oem_id,
>>> +    AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = oem_id,
>>>                           .oem_table_id = oem_table_id };
>>>
>>>       acpi_table_begin(&table, table_data);
>>
>> make check fails for me at this point
>> (my guess is that not all APIC tables are whitelisted)
> 
> I think the patchset needs to be rebased and the blobs regenerated.

So I've been trying to overcome this today and not having much luck.

When I run "make check V=2", I see at the end:

Summary of Failures:

  45/786 qemu:qtest+qtest-i386 / qtest-i386/bios-tables-test
  68/786 qemu:qtest+qtest-x86_64 / qtest-x86_64/bios-tables-test

If I go look at 45/786, for example, I see:

Looking for expected file 'tests/data/acpi/pc/FACP'
Using expected file 'tests/data/acpi/pc/FACP'
Looking for expected file 'tests/data/acpi/pc/APIC'
Using expected file 'tests/data/acpi/pc/APIC'
Looking for expected file 'tests/data/acpi/pc/HPET'
Using expected file 'tests/data/acpi/pc/HPET'
Looking for expected file 'tests/data/acpi/pc/WAET'
Using expected file 'tests/data/acpi/pc/WAET'
Looking for expected file 'tests/data/acpi/pc/FACS'
Using expected file 'tests/data/acpi/pc/FACS'
Looking for expected file 'tests/data/acpi/pc/DSDT'
Using expected file 'tests/data/acpi/pc/DSDT'
acpi-test: Warning! APIC binary file mismatch. Actual [aml:/tmp/aml-R4D741], Expected 
[aml:tests/data/acpi/pc/APIC].
See source file tests/qtest/bios-tables-test.c for instructions on how to update expected files.
acpi-test: Warning! APIC mismatch. Actual [asl:/tmp/asl-GVD741.dsl, aml:/tmp/aml-R4D741], Expected 
[asl:/tmp/asl-1F9641.dsl, aml:tests/data/acpi/pc/APIC].
--- /tmp/asl-1F9641.dsl 2023-05-16 15:18:31.292579156 -0400
+++ /tmp/asl-GVD741.dsl 2023-05-16 15:18:31.291579149 -0400
@@ -1,32 +1,32 @@
  /*
   * Intel ACPI Component Architecture
   * AML/ASL+ Disassembler version 20230331 (64-bit version)
   * Copyright (c) 2000 - 2023 Intel Corporation
   *
- * Disassembly of tests/data/acpi/pc/APIC, Tue May 16 15:18:31 2023
+ * Disassembly of /tmp/aml-R4D741, Tue May 16 15:18:31 2023
   *
   * ACPI Data Table [APIC]
   *
   * Format: [HexOffset DecimalOffset ByteLength]  FieldName : FieldValue (in hex)
   */

  [000h 0000 004h]                   Signature : "APIC"    [Multiple APIC Description Table (MADT)]
  [004h 0004 004h]                Table Length : 00000078
-[008h 0008 001h]                    Revision : 01
-[009h 0009 001h]                    Checksum : 8A
+[008h 0008 001h]                    Revision : 03
+[009h 0009 001h]                    Checksum : 88
  [00Ah 0010 006h]                      Oem ID : "BOCHS "
  [010h 0016 008h]                Oem Table ID : "BXPC    "
  [018h 0024 004h]                Oem Revision : 00000001
  [01Ch 0028 004h]             Asl Compiler ID : "BXPC"
  [020h 0032 004h]       Asl Compiler Revision : 00000001
[...]

And the q35 looks very very similar.

It suggests that I need to list tests/data/acpi/pc/APIC, which I have done
in bios-tables-test-allowed-diff.h:

/* List of comma-separated changed AML files to ignore */
"tests/data/acpi/pc/APIC",
"tests/data/acpi/q35/APIC",
"tests/data/acpi/microvm/APIC",
"tests/data/acpi/virt/APIC",

But as I looked closer at the files that changed in the last step
of the previous post, there are a bunch of them:

  tests/data/acpi/microvm/APIC                  | Bin 70 -> 70 bytes
  tests/data/acpi/microvm/APIC.ioapic2          | Bin 82 -> 82 bytes
  tests/data/acpi/microvm/APIC.pcie             | Bin 110 -> 110 bytes
  tests/data/acpi/pc/APIC                       | Bin 120 -> 120 bytes
  tests/data/acpi/pc/APIC.acpihmat              | Bin 128 -> 128 bytes
  tests/data/acpi/pc/APIC.cphp                  | Bin 160 -> 160 bytes
  tests/data/acpi/pc/APIC.dimmpxm               | Bin 144 -> 144 bytes
  tests/data/acpi/q35/APIC                      | Bin 120 -> 120 bytes
  tests/data/acpi/q35/APIC.acpihmat             | Bin 128 -> 128 bytes
  tests/data/acpi/q35/APIC.acpihmat-noinitiator | Bin 144 -> 144 bytes
  tests/data/acpi/q35/APIC.core-count2          | Bin 2478 -> 2478 bytes
  tests/data/acpi/q35/APIC.cphp                 | Bin 160 -> 160 bytes
  tests/data/acpi/q35/APIC.dimmpxm              | Bin 144 -> 144 bytes
  tests/data/acpi/q35/APIC.xapic                | Bin 2686 -> 2686 bytes

Should all of these files be listed in allowed-diff.h?
And I would also need to provide the diff for each in the patch
containing this last step, correct?

Thanks,
eric



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] ACPI: i386: bump to MADT to revision 3
  2023-05-16 21:22       ` Eric DeVolder
@ 2023-05-16 21:28         ` Michael S. Tsirkin
  0 siblings, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2023-05-16 21:28 UTC (permalink / raw)
  To: Eric DeVolder
  Cc: Ani Sinha, Igor Mammedov, qemu-devel, shannon.zhaosl,
	peter.maydell, qemu-arm, marcel.apfelbaum, pbonzini,
	richard.henderson, eduardo, boris.ostrovsky, miguel.luis

On Tue, May 16, 2023 at 04:22:58PM -0500, Eric DeVolder wrote:
> 
> 
> On 5/16/23 07:51, Ani Sinha wrote:
> > On Tue, May 16, 2023 at 6:01 PM Igor Mammedov <imammedo@redhat.com> wrote:
> > > 
> > > On Mon, 15 May 2023 16:33:10 -0400
> > > Eric DeVolder <eric.devolder@oracle.com> wrote:
> > > 
> > > > Currently i386 QEMU generates MADT revision 3, and reports
> > > > MADT revision 1. Set .revision to 3 to match reality.
> > > > 
> > > > Link: https://lore.kernel.org/linux-acpi/20230327191026.3454-1-eric.devolder@ora
> > > > cle.com/T/#t
> > > > Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> > > > ---
> > > >   hw/i386/acpi-common.c | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> > > > index 52e5c1439a..8a0932fe84 100644
> > > > --- a/hw/i386/acpi-common.c
> > > > +++ b/hw/i386/acpi-common.c
> > > > @@ -102,7 +102,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> > > >       MachineClass *mc = MACHINE_GET_CLASS(x86ms);
> > > >       const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
> > > >       AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
> > > > -    AcpiTable table = { .sig = "APIC", .rev = 1, .oem_id = oem_id,
> > > > +    AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = oem_id,
> > > >                           .oem_table_id = oem_table_id };
> > > > 
> > > >       acpi_table_begin(&table, table_data);
> > > 
> > > make check fails for me at this point
> > > (my guess is that not all APIC tables are whitelisted)
> > 
> > I think the patchset needs to be rebased and the blobs regenerated.
> 
> So I've been trying to overcome this today and not having much luck.
> 
> When I run "make check V=2", I see at the end:
> 
> Summary of Failures:
> 
>  45/786 qemu:qtest+qtest-i386 / qtest-i386/bios-tables-test
>  68/786 qemu:qtest+qtest-x86_64 / qtest-x86_64/bios-tables-test
> 
> If I go look at 45/786, for example, I see:
> 
> Looking for expected file 'tests/data/acpi/pc/FACP'
> Using expected file 'tests/data/acpi/pc/FACP'
> Looking for expected file 'tests/data/acpi/pc/APIC'
> Using expected file 'tests/data/acpi/pc/APIC'
> Looking for expected file 'tests/data/acpi/pc/HPET'
> Using expected file 'tests/data/acpi/pc/HPET'
> Looking for expected file 'tests/data/acpi/pc/WAET'
> Using expected file 'tests/data/acpi/pc/WAET'
> Looking for expected file 'tests/data/acpi/pc/FACS'
> Using expected file 'tests/data/acpi/pc/FACS'
> Looking for expected file 'tests/data/acpi/pc/DSDT'
> Using expected file 'tests/data/acpi/pc/DSDT'
> acpi-test: Warning! APIC binary file mismatch. Actual [aml:/tmp/aml-R4D741],
> Expected [aml:tests/data/acpi/pc/APIC].
> See source file tests/qtest/bios-tables-test.c for instructions on how to update expected files.
> acpi-test: Warning! APIC mismatch. Actual [asl:/tmp/asl-GVD741.dsl,
> aml:/tmp/aml-R4D741], Expected [asl:/tmp/asl-1F9641.dsl,
> aml:tests/data/acpi/pc/APIC].
> --- /tmp/asl-1F9641.dsl 2023-05-16 15:18:31.292579156 -0400
> +++ /tmp/asl-GVD741.dsl 2023-05-16 15:18:31.291579149 -0400
> @@ -1,32 +1,32 @@
>  /*
>   * Intel ACPI Component Architecture
>   * AML/ASL+ Disassembler version 20230331 (64-bit version)
>   * Copyright (c) 2000 - 2023 Intel Corporation
>   *
> - * Disassembly of tests/data/acpi/pc/APIC, Tue May 16 15:18:31 2023
> + * Disassembly of /tmp/aml-R4D741, Tue May 16 15:18:31 2023
>   *
>   * ACPI Data Table [APIC]
>   *
>   * Format: [HexOffset DecimalOffset ByteLength]  FieldName : FieldValue (in hex)
>   */
> 
>  [000h 0000 004h]                   Signature : "APIC"    [Multiple APIC Description Table (MADT)]
>  [004h 0004 004h]                Table Length : 00000078
> -[008h 0008 001h]                    Revision : 01
> -[009h 0009 001h]                    Checksum : 8A
> +[008h 0008 001h]                    Revision : 03
> +[009h 0009 001h]                    Checksum : 88
>  [00Ah 0010 006h]                      Oem ID : "BOCHS "
>  [010h 0016 008h]                Oem Table ID : "BXPC    "
>  [018h 0024 004h]                Oem Revision : 00000001
>  [01Ch 0028 004h]             Asl Compiler ID : "BXPC"
>  [020h 0032 004h]       Asl Compiler Revision : 00000001
> [...]
> 
> And the q35 looks very very similar.
> 
> It suggests that I need to list tests/data/acpi/pc/APIC, which I have done
> in bios-tables-test-allowed-diff.h:
> 
> /* List of comma-separated changed AML files to ignore */
> "tests/data/acpi/pc/APIC",
> "tests/data/acpi/q35/APIC",
> "tests/data/acpi/microvm/APIC",
> "tests/data/acpi/virt/APIC",
> 
> But as I looked closer at the files that changed in the last step
> of the previous post, there are a bunch of them:
> 
>  tests/data/acpi/microvm/APIC                  | Bin 70 -> 70 bytes
>  tests/data/acpi/microvm/APIC.ioapic2          | Bin 82 -> 82 bytes
>  tests/data/acpi/microvm/APIC.pcie             | Bin 110 -> 110 bytes
>  tests/data/acpi/pc/APIC                       | Bin 120 -> 120 bytes
>  tests/data/acpi/pc/APIC.acpihmat              | Bin 128 -> 128 bytes
>  tests/data/acpi/pc/APIC.cphp                  | Bin 160 -> 160 bytes
>  tests/data/acpi/pc/APIC.dimmpxm               | Bin 144 -> 144 bytes
>  tests/data/acpi/q35/APIC                      | Bin 120 -> 120 bytes
>  tests/data/acpi/q35/APIC.acpihmat             | Bin 128 -> 128 bytes
>  tests/data/acpi/q35/APIC.acpihmat-noinitiator | Bin 144 -> 144 bytes
>  tests/data/acpi/q35/APIC.core-count2          | Bin 2478 -> 2478 bytes
>  tests/data/acpi/q35/APIC.cphp                 | Bin 160 -> 160 bytes
>  tests/data/acpi/q35/APIC.dimmpxm              | Bin 144 -> 144 bytes
>  tests/data/acpi/q35/APIC.xapic                | Bin 2686 -> 2686 bytes
> 
> Should all of these files be listed in allowed-diff.h?

unfortunately, yes.

> And I would also need to provide the diff for each in the patch
> containing this last step, correct?
> 
> Thanks,
> eric

if you mean commit log, if the diff is the same it is
enough to list it once and mention it is the same.

-- 
MST



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-05-16 21:28 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-15 20:33 [PATCH 0/3] ACPI: i386: bump MADT to revision 3 Eric DeVolder
2023-05-15 20:33 ` [PATCH 1/3] ACPI: bios-tables-test.c step 2 (allowed-diff entries) Eric DeVolder
2023-05-16  7:25   ` Ani Sinha
2023-05-15 20:33 ` [PATCH 2/3] ACPI: i386: bump to MADT to revision 3 Eric DeVolder
2023-05-16  7:25   ` Ani Sinha
2023-05-16 12:31   ` Igor Mammedov
2023-05-16 12:51     ` Ani Sinha
2023-05-16 21:22       ` Eric DeVolder
2023-05-16 21:28         ` Michael S. Tsirkin
2023-05-15 20:33 ` [PATCH 3/3] ACPI: bios-tables-test.c step 5 (update expected table binaries) Eric DeVolder
2023-05-16  7:28   ` Ani Sinha
2023-05-16  7:39 ` [PATCH 0/3] ACPI: i386: bump MADT to revision 3 Ani Sinha

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).