Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 27 Mar 2012 18:13:44 -0400
From:      Jung-uk Kim <jkim@FreeBSD.org>
To:        freebsd-current@FreeBSD.org
Cc:        Poul-Henning Kamp <phk@phk.freebsd.dk>, Sevan / Venture37 <venture37@gmail.com>, Andreas Tobler <andreast@freebsd.org>, Lars Engels <lars.engels@0x20.net>
Subject:   [PATCH] ACPI object refcount fix
Message-ID:  <201203271813.47740.jkim@FreeBSD.org>

next in thread | raw e-mail | index | archive | help

--Boundary-00=_bujcPbsD9s4SZ4f
Content-Type: text/plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

The upstream maintainer just e-mailed me a possible fix for the 
problem and it looks very promising.  Please try the attached patch.  
Note the patch reverts r233555 and applies the fix.  This patch is 
also available from here:

http://people.freebsd.org/~jkim/acpi_refcnt.diff

Thanks!

Jung-uk Kim

--Boundary-00=_bujcPbsD9s4SZ4f
Content-Type: text/plain;
  charset="iso-8859-1";
  name="acpi_refcnt.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="acpi_refcnt.diff"

Index: sys/contrib/dev/acpica/include/aclocal.h
===================================================================
--- sys/contrib/dev/acpica/include/aclocal.h	(revision 233578)
+++ sys/contrib/dev/acpica/include/aclocal.h	(working copy)
@@ -424,6 +424,7 @@ typedef struct acpi_predefined_data
 /* Defines for Flags field above */
 
 #define ACPI_OBJECT_REPAIRED    1
+#define ACPI_OBJECT_WRAPPED     2
 
 
 /*
Index: sys/contrib/dev/acpica/include/acnamesp.h
===================================================================
--- sys/contrib/dev/acpica/include/acnamesp.h	(revision 233578)
+++ sys/contrib/dev/acpica/include/acnamesp.h	(working copy)
@@ -368,8 +368,9 @@ AcpiNsRepairObject (
     ACPI_OPERAND_OBJECT     **ReturnObjectPtr);
 
 ACPI_STATUS
-AcpiNsRepairPackageList (
+AcpiNsWrapWithPackage (
     ACPI_PREDEFINED_DATA    *Data,
+    ACPI_OPERAND_OBJECT     *OriginalObject,
     ACPI_OPERAND_OBJECT     **ObjDescPtr);
 
 ACPI_STATUS
Index: sys/contrib/dev/acpica/components/namespace/nsrepair.c
===================================================================
--- sys/contrib/dev/acpica/components/namespace/nsrepair.c	(revision 233578)
+++ sys/contrib/dev/acpica/components/namespace/nsrepair.c	(working copy)
@@ -74,11 +74,10 @@
  * Buffer  -> String
  * Buffer  -> Package of Integers
  * Package -> Package of one Package
+ * An incorrect standalone object is wrapped with required outer package
  *
  * Additional possible repairs:
- *
  * Required package elements that are NULL replaced by Integer/String/Buffer
- * Incorrect standalone package wrapped with required outer package
  *
  ******************************************************************************/
 
@@ -100,12 +99,7 @@ AcpiNsConvertToBuffer (
     ACPI_OPERAND_OBJECT     *OriginalObject,
     ACPI_OPERAND_OBJECT     **ReturnObject);
 
-static ACPI_STATUS
-AcpiNsConvertToPackage (
-    ACPI_OPERAND_OBJECT     *OriginalObject,
-    ACPI_OPERAND_OBJECT     **ReturnObject);
 
-
 /*******************************************************************************
  *
  * FUNCTION:    AcpiNsRepairObject
@@ -172,10 +166,24 @@ AcpiNsRepairObject (
     }
     if (ExpectedBtypes & ACPI_RTYPE_PACKAGE)
     {
-        Status = AcpiNsConvertToPackage (ReturnObject, &NewObject);
+        /*
+         * A package is expected. We will wrap the existing object with a
+         * new package object. It is often the case that if a variable-length
+         * package is required, but there is only a single object needed, the
+         * BIOS will return that object instead of wrapping it with a Package
+         * object. Note: after the wrapping, the package will be validated
+         * for correct contents (expected object type or types).
+         */
+        Status = AcpiNsWrapWithPackage (Data, ReturnObject, &NewObject);
         if (ACPI_SUCCESS (Status))
         {
-            goto ObjectRepaired;
+            /*
+             * The original object just had its reference count
+             * incremented for being inserted into the new package.
+             */
+            *ReturnObjectPtr = NewObject;       /* New Package object */
+            Data->Flags |= ACPI_OBJECT_REPAIRED;
+            return (AE_OK);
         }
     }
 
@@ -188,24 +196,30 @@ ObjectRepaired:
 
     /* Object was successfully repaired */
 
-    /*
-     * If the original object is a package element, we need to:
-     * 1. Set the reference count of the new object to match the
-     *    reference count of the old object.
-     * 2. Decrement the reference count of the original object.
-     */
     if (PackageIndex != ACPI_NOT_PACKAGE_ELEMENT)
     {
-        NewObject->Common.ReferenceCount =
-            ReturnObject->Common.ReferenceCount;
+        /*
+         * The original object is a package element. We need to
+         * decrement the reference count of the original object,
+         * for removing it from the package.
+         *
+         * However, if the original object was just wrapped with a
+         * package object as part of the repair, we don't need to
+         * change the reference count.
+         */
+        if (!(Data->Flags & ACPI_OBJECT_WRAPPED))
+        {
+            NewObject->Common.ReferenceCount =
+                ReturnObject->Common.ReferenceCount;
 
-        if (ReturnObject->Common.ReferenceCount > 1)
-        {
-            ReturnObject->Common.ReferenceCount--;
+            if (ReturnObject->Common.ReferenceCount > 1)
+            {
+                ReturnObject->Common.ReferenceCount--;
+            }
         }
 
         ACPI_DEBUG_PRINT ((ACPI_DB_REPAIR,
-            "%s: Converted %s to expected %s at index %u\n",
+            "%s: Converted %s to expected %s at Package index %u\n",
             Data->Pathname, AcpiUtGetObjectTypeName (ReturnObject),
             AcpiUtGetObjectTypeName (NewObject), PackageIndex));
     }
@@ -498,71 +512,6 @@ AcpiNsConvertToBuffer (
 
 /*******************************************************************************
  *
- * FUNCTION:    AcpiNsConvertToPackage
- *
- * PARAMETERS:  OriginalObject      - Object to be converted
- *              ReturnObject        - Where the new converted object is returned
- *
- * RETURN:      Status. AE_OK if conversion was successful.
- *
- * DESCRIPTION: Attempt to convert a Buffer object to a Package. Each byte of
- *              the buffer is converted to a single integer package element.
- *
- ******************************************************************************/
-
-static ACPI_STATUS
-AcpiNsConvertToPackage (
-    ACPI_OPERAND_OBJECT     *OriginalObject,
-    ACPI_OPERAND_OBJECT     **ReturnObject)
-{
-    ACPI_OPERAND_OBJECT     *NewObject;
-    ACPI_OPERAND_OBJECT     **Elements;
-    UINT32                  Length;
-    UINT8                   *Buffer;
-
-
-    switch (OriginalObject->Common.Type)
-    {
-    case ACPI_TYPE_BUFFER:
-
-        /* Buffer-to-Package conversion */
-
-        Length = OriginalObject->Buffer.Length;
-        NewObject = AcpiUtCreatePackageObject (Length);
-        if (!NewObject)
-        {
-            return (AE_NO_MEMORY);
-        }
-
-        /* Convert each buffer byte to an integer package element */
-
-        Elements = NewObject->Package.Elements;
-        Buffer = OriginalObject->Buffer.Pointer;
-
-        while (Length--)
-        {
-            *Elements = AcpiUtCreateIntegerObject ((UINT64) *Buffer);
-            if (!*Elements)
-            {
-                AcpiUtRemoveReference (NewObject);
-                return (AE_NO_MEMORY);
-            }
-            Elements++;
-            Buffer++;
-        }
-        break;
-
-    default:
-        return (AE_AML_OPERAND_TYPE);
-    }
-
-    *ReturnObject = NewObject;
-    return (AE_OK);
-}
-
-
-/*******************************************************************************
- *
  * FUNCTION:    AcpiNsRepairNullElement
  *
  * PARAMETERS:  Data                - Pointer to validation data structure
@@ -745,42 +694,43 @@ AcpiNsRemoveNullElements (
 
 /*******************************************************************************
  *
- * FUNCTION:    AcpiNsRepairPackageList
+ * FUNCTION:    AcpiNsWrapWithPackage
  *
  * PARAMETERS:  Data                - Pointer to validation data structure
- *              ObjDescPtr          - Pointer to the object to repair. The new
- *                                    package object is returned here,
- *                                    overwriting the old object.
+ *              OriginalObject      - Pointer to the object to repair.
+ *              ObjDescPtr          - The new package object is returned here
  *
  * RETURN:      Status, new object in *ObjDescPtr
  *
- * DESCRIPTION: Repair a common problem with objects that are defined to return
- *              a variable-length Package of Packages. If the variable-length
- *              is one, some BIOS code mistakenly simply declares a single
- *              Package instead of a Package with one sub-Package. This
- *              function attempts to repair this error by wrapping a Package
- *              object around the original Package, creating the correct
- *              Package with one sub-Package.
+ * DESCRIPTION: Repair a common problem with objects that are defined to
+ *              return a variable-length Package of sub-objects. If there is
+ *              only one sub-object, some BIOS code mistakenly simply declares
+ *              the single object instead of a Package with one sub-object.
+ *              This function attempts to repair this error by wrapping a
+ *              Package object around the original object, creating the
+ *              correct and expected Package with one sub-object.
  *
  *              Names that can be repaired in this manner include:
- *              _ALR, _CSD, _HPX, _MLS, _PRT, _PSS, _TRT, TSS
+ *              _ALR, _CSD, _HPX, _MLS, _PLD, _PRT, _PSS, _TRT, _TSS,
+ *              _BCL, _DOD, _FIX, _Sx
  *
  ******************************************************************************/
 
 ACPI_STATUS
-AcpiNsRepairPackageList (
+AcpiNsWrapWithPackage (
     ACPI_PREDEFINED_DATA    *Data,
+    ACPI_OPERAND_OBJECT     *OriginalObject,
     ACPI_OPERAND_OBJECT     **ObjDescPtr)
 {
     ACPI_OPERAND_OBJECT     *PkgObjDesc;
 
 
-    ACPI_FUNCTION_NAME (NsRepairPackageList);
+    ACPI_FUNCTION_NAME (NsWrapWithPackage);
 
 
     /*
      * Create the new outer package and populate it. The new package will
-     * have a single element, the lone subpackage.
+     * have a single element, the lone sub-object.
      */
     PkgObjDesc = AcpiUtCreatePackageObject (1);
     if (!PkgObjDesc)
@@ -788,15 +738,15 @@ ACPI_STATUS
         return (AE_NO_MEMORY);
     }
 
-    PkgObjDesc->Package.Elements[0] = *ObjDescPtr;
+    PkgObjDesc->Package.Elements[0] = OriginalObject;
 
+    ACPI_DEBUG_PRINT ((ACPI_DB_REPAIR,
+        "%s: Wrapped %s with expected Package object\n",
+        Data->Pathname, AcpiUtGetObjectTypeName (OriginalObject)));
+
     /* Return the new object in the object pointer */
 
     *ObjDescPtr = PkgObjDesc;
-    Data->Flags |= ACPI_OBJECT_REPAIRED;
-
-    ACPI_DEBUG_PRINT ((ACPI_DB_REPAIR,
-        "%s: Repaired incorrectly formed Package\n", Data->Pathname));
-
+    Data->Flags |= ACPI_OBJECT_REPAIRED | ACPI_OBJECT_WRAPPED;
     return (AE_OK);
 }
Index: sys/contrib/dev/acpica/components/namespace/nspredef.c
===================================================================
--- sys/contrib/dev/acpica/components/namespace/nspredef.c	(revision 233578)
+++ sys/contrib/dev/acpica/components/namespace/nspredef.c	(working copy)
@@ -681,7 +681,7 @@ AcpiNsCheckPackage (
         {
             /* Create the new outer package and populate it */
 
-            Status = AcpiNsRepairPackageList (Data, ReturnObjectPtr);
+            Status = AcpiNsWrapWithPackage (Data, *Elements, ReturnObjectPtr);
             if (ACPI_FAILURE (Status))
             {
                 return (Status);

--Boundary-00=_bujcPbsD9s4SZ4f--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203271813.47740.jkim>