[Qemu-devel] [PULL 08/15] i386/acpi-build: allow more than 255 elements in CPON

[Michael S. Tsirkin]

From: Laszlo Ersek <address@hidden>

The build_ssdt() function builds a number of AML objects that are related
to CPU hotplug, and whose IDs form a contiguous sequence of APIC IDs.
(APIC IDs are in fact discontiguous, but this is the traditional
interface: build a contiguous sequence from zero up that covers all
possible APIC IDs.) These objects are:

- a Processor() object for each VCPU,
- a NTFY method, with one branch for each VCPU,
- a CPON package with one element (hotplug status byte) for each VCPU.

The build_ssdt() function currently limits the *count* of processor
objects, and NTFY branches, and CPON elements, in 0xFF (see the assignment
to "acpi_cpus"). This allows for an inclusive APIC ID range of [0..254].
This is incorrect, because the highest APIC ID that we otherwise allow a
VCPU to take is 255.

In order to extend the maximum count to 256, and the traversed APIC ID
range correspondingly to [0..255]:
- the Processor() objects need no change,
- the NTFY method also needs no change,
- the CPON package must be updated, because it is defined with a
  DefPackage, and the number of elements in such a package can be at most
  255. We pick a DefVarPackage instead.

We replace the Op byte, and the encoding of the number of elements.
Compare:

DefPackage     := PackageOp    PkgLength NumElements    PackageElementList
DefVarPackage  := VarPackageOp PkgLength VarNumElements PackageElementList

PackageOp      := 0x12
VarPackageOp   := 0x13

NumElements    := ByteData
VarNumElements := TermArg => Integer

The build_append_int() function implements precisely the following TermArg
encodings (a subset of what the ACPI spec describes):

  TermArg             := DataObject
  DataObject          := ComputationalData
  ComputationalData   := ConstObj | ByteConst | WordConst | DWordConst

  directly encoded in the function, with build_append_byte():
    ConstObj          := ZeroOp | OneOp
      ZeroOp          := 0x00
      OneOp           := 0x01

  call to build_append_value(..., 1):
    ByteConst         := BytePrefix ByteData
      BytePrefix      := 0x0A
      ByteData        := 0x00 - 0xFF

  call to build_append_value(..., 2):
    WordConst         := WordPrefix WordData
      WordPrefix      := 0x0B
      WordData        := ByteData[0:7] ByteData[8:15]

  call to build_append_value(..., 4):
    DWordConst        := DWordPrefix DWordData
      DWordPrefix     := 0x0C
      DWordData       := WordData[0:15] WordData[16:31]

Signed-off-by: Laszlo Ersek <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Michael S. Tsirkin <address@hidden>
---
 hw/i386/acpi-build.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 41f3d8a..a5039d4 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1050,9 +1050,9 @@ build_ssdt(GArray *table_data, GArray *linker,
 
         {
             GArray *package = build_alloc_array();
-            uint8_t op = 0x12; /* PackageOp */
+            uint8_t op = 0x13; /* VarPackageOp */
 
-            build_append_byte(package, acpi_cpus); /* NumElements */
+            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);
-- 
MST