[Devel] sleep state parsing in acpica

[Devel] sleep state parsing in acpica

[Laszlo Ersek]

[-- Attachment #1: Type: text/plain, Size: 2966 bytes --]

Hello Robert, Jordan,

the S3 and S4 packages prepared by OVMF / edk2 commit [1] (svn rev
14003) look like

      Name (\_S3, Package (0x01)
      {
          0x00000001
      })
      Name (\_S4, Package (0x01)
      {
          0x00000002
      })

They are refused by the following ACPICA code section (please excuse the
wide listing):

repo: git://github.com/otcshare/acpica.git
file: source/components/hardware/hwxface.c

4fa895bc (Robert Moore 2008-11-19 13:15:10 -0800 635)     /*
4fa895bc (Robert Moore 2008-11-19 13:15:10 -0800 636)      * The package must have at least two elements. NOTE (March 2005): This
4fa895bc (Robert Moore 2008-11-19 13:15:10 -0800 637)      * goes against the current ACPI spec which defines this object as a
4fa895bc (Robert Moore 2008-11-19 13:15:10 -0800 638)      * package with one encoded DWORD element. However, existing practice
4fa895bc (Robert Moore 2008-11-19 13:15:10 -0800 639)      * by BIOS vendors seems to be to have 2 or more elements, at least
4fa895bc (Robert Moore 2008-11-19 13:15:10 -0800 640)      * one per sleep type (A/B).
4fa895bc (Robert Moore 2008-11-19 13:15:10 -0800 641)      */
4fa895bc (Robert Moore 2008-11-19 13:15:10 -0800 642)     else if (Info->ReturnObject->Package.Count < 2)
4fa895bc (Robert Moore 2008-11-19 13:15:10 -0800 643)     {
4fa895bc (Robert Moore 2008-11-19 13:15:10 -0800 644)         ACPI_ERROR ((AE_INFO,
4fa895bc (Robert Moore 2008-11-19 13:15:10 -0800 645)             "Sleep State return package does not have at least two elements"));
4fa895bc (Robert Moore 2008-11-19 13:15:10 -0800 646)         Status = AE_AML_NO_OPERAND;
4fa895bc (Robert Moore 2008-11-19 13:15:10 -0800 647)     }

I ran into the problem with the 20121217.16 Fedora 18 nightly build:

[    0.000000] Linux version 3.6.10-4.fc18.x86_64 (mockbuild@) (gcc version 4.7.2 20121109 (Red Hat 4.7.2-8) (GCC) ) #1 SMP Tue Dec 11 18:01:27 UTC 2012

[    0.064483] ACPI: Interpreter enabled
[    0.064483] ACPI: (supports S0ACPI Error: Sleep State return package does not have at least two elements (20120711/hwxface-511)
[    0.064483] ACPI Exception: AE_AML_NO_OPERAND, While evaluating SleepState [\_S3_], bad Sleep object ffff880119a33c18 type Package (20120711/hwxface-543)
[    0.064483] ACPI Error: Sleep State return package does not have at least two elements (20120711/hwxface-511)
[    0.064483] ACPI Exception: AE_AML_NO_OPERAND, While evaluating SleepState [\_S4_], bad Sleep object ffff880119a33cf0 type Package (20120711/hwxface-543)
[    0.064483]  S5)


Should I attempt a patch for ACPICA (see the attachment), or is it not
worth the hassle and I should just fixup the AML generation in OVMF
(split the DWord into Bytes)?

(Please keep me CC'd, I'm not subscribed to <devel(a)acpica.org>.)

Thanks
Laszlo

[1] http://tianocore.git.sourceforge.net/git/gitweb.cgi?p=tianocore/edk2;a=commitdiff;h=14430c55c8d0e9a8487c2c74155e63299632ef5e

[-- Attachment #2: 0001-AcpiGetSleepTypeData-accept-package-with-one-element.patch --]
[-- Type: text/plain, Size: 3727 bytes --]

>From 9d28cda4e7952b4818a72afdcfecc56ed687c680 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Thu, 20 Dec 2012 14:08:51 +0100
Subject: [PATCH] AcpiGetSleepTypeData: accept package with one element


Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 source/components/hardware/hwxface.c |   64 +++++++++++++++++++--------------
 1 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/source/components/hardware/hwxface.c b/source/components/hardware/hwxface.c
index ace8251..bcdc2ca 100644
--- a/source/components/hardware/hwxface.c
+++ b/source/components/hardware/hwxface.c
@@ -633,41 +633,51 @@ AcpiGetSleepTypeData (
     }
 
     /*
-     * The package must have at least two elements. NOTE (March 2005): This
-     * goes against the current ACPI spec which defines this object as a
-     * package with one encoded DWORD element. However, existing practice
-     * by BIOS vendors seems to be to have 2 or more elements, at least
-     * one per sleep type (A/B).
+     * The current ACPI spec (v 5.0) defines this object as a package with one
+     * encoded DWORD element.
+     *
+     * Existing practice by BIOS vendors seems to be to have 2 or more
+     * elements, at least one per sleep type (A/B).
      */
-    else if (Info->ReturnObject->Package.Count < 2)
-    {
-        ACPI_ERROR ((AE_INFO,
-            "Sleep State return package does not have at least two elements"));
-        Status = AE_AML_NO_OPERAND;
-    }
 
-    /* The first two elements must both be of type Integer */
-
-    else if (((Info->ReturnObject->Package.Elements[0])->Common.Type
-                != ACPI_TYPE_INTEGER) ||
-             ((Info->ReturnObject->Package.Elements[1])->Common.Type
-                != ACPI_TYPE_INTEGER))
+    else if (Info->ReturnObject->Package.Count == 0)
     {
         ACPI_ERROR ((AE_INFO,
-            "Sleep State return package elements are not both Integers "
-            "(%s, %s)",
-            AcpiUtGetObjectTypeName (Info->ReturnObject->Package.Elements[0]),
-            AcpiUtGetObjectTypeName (Info->ReturnObject->Package.Elements[1])));
-        Status = AE_AML_OPERAND_TYPE;
+            "Sleep State return package does not have elements"));
+        Status = AE_AML_NO_OPERAND;
     }
     else
     {
-        /* Valid _Sx_ package size, type, and value */
+        union acpi_operand_object **Elements;
+
+        Elements = Info->ReturnObject->Package.Elements;
 
-        *SleepTypeA = (UINT8)
-            (Info->ReturnObject->Package.Elements[0])->Integer.Value;
-        *SleepTypeB = (UINT8)
-            (Info->ReturnObject->Package.Elements[1])->Integer.Value;
+        if (Elements[0]->Common.Type != ACPI_TYPE_INTEGER)
+        {
+            ACPI_ERROR ((AE_INFO,
+                "1st element in Sleep State return package is not Integer "
+                "(%s)",
+                AcpiUtGetObjectTypeName (Elements[0])));
+            Status = AE_AML_OPERAND_TYPE;
+        }
+        else if (Info->ReturnObject->Package.Count == 1)
+        {
+            *SleepTypeA = (UINT8)  Elements[0]->Integer.Value;
+            *SleepTypeB = (UINT8) (Elements[0]->Integer.Value >> 8);
+        }
+        else if (Elements[1]->Common.Type != ACPI_TYPE_INTEGER)
+        {
+            ACPI_ERROR ((AE_INFO,
+                "2nd element in Sleep State return package is not Integer "
+                "(%s)",
+                AcpiUtGetObjectTypeName (Elements[1])));
+            Status = AE_AML_OPERAND_TYPE;
+        }
+        else
+        {
+            *SleepTypeA = (UINT8) Elements[0]->Integer.Value;
+            *SleepTypeB = (UINT8) Elements[1]->Integer.Value;
+        }
     }
 
     if (ACPI_FAILURE (Status))
-- 
1.7.1