acpi: make SSDT 1.0 spec compliant when possible

The ACPI specification says:

The ASL compiler can emit two different AML opcodes for a Package
declaration, either PackageOp or VarPackageOp. For small, fixed-length
packages, the PackageOp is used and this opcode is compatible with ACPI
1.0. A VarPackageOp will be emitted if any of the following conditions
are true:
. The NumElements argument is a TermArg that can only be resolved at
runtime.
. At compile time, NumElements resolves to a constant that is larger than
255.
. The PackageList contains more than 255 initializer elements.
Note: The ability to create variable-sized packages was first introduced
in ACPI 2.0. ACPI 1.0 only allowed fixed-size packages with up to 255 elements.

So the spec seems to say a fixed value up to 255 must always
be used with PackageOp and not VarPackageOp, and some guests
(windows up to win2k8) seem to interpret it like this.

Let's do just this, choosing the encoding depending on
the number of elements.

Fixes 9bcc80cd71892df42605e0c097d85c0237ff45d1
(i386/acpi-build: allow more than 255 elements in CPON).

https://bugs.launchpad.net/bugs/1297651

Reported-by: Robert Hu <robert.hu@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

1 files changed, 14 insertions, 2 deletions

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index f1054dd..7597517 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1055,9 +1055,21 @@ build_ssdt(GArray *table_data, GArray *linker,
 
         {
             GArray *package = build_alloc_array();
-            uint8_t op = 0x13; /* VarPackageOp */
+            uint8_t op;
+
+            /*
+             * Note: The ability to create variable-sized packages was first introduced in ACPI 2.0. ACPI 1.0 only
+             * allowed fixed-size packages with up to 255 elements.
+             * Windows guests up to win2k8 fail when VarPackageOp is used.
+             */
+            if (acpi_cpus <= 255) {
+                op = 0x12; /* PackageOp */
+                build_append_byte(package, acpi_cpus); /* NumElements */
+            } else {
+                op = 0x13; /* VarPackageOp */
+                build_append_int(package, acpi_cpus); /* VarNumElements */
+            }
 
-            build_append_int(package, acpi_cpus); /* VarNumElements */
             for (i = 0; i < acpi_cpus; i++) {
                 uint8_t b = test_bit(i, cpu->found_cpus) ? 0x01 : 0x00;
                 build_append_byte(package, b);