From owner-freebsd-current@FreeBSD.ORG Tue Mar 27 22:13:56 2012 Return-Path: Delivered-To: freebsd-current@FreeBSD.org Received: from [127.0.0.1] (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by hub.freebsd.org (Postfix) with ESMTP id 1C715106566C; Tue, 27 Mar 2012 22:13:55 +0000 (UTC) (envelope-from jkim@FreeBSD.org) From: Jung-uk Kim To: freebsd-current@FreeBSD.org Date: Tue, 27 Mar 2012 18:13:44 -0400 User-Agent: KMail/1.6.2 MIME-Version: 1.0 Content-Disposition: inline Content-Type: Multipart/Mixed; boundary="Boundary-00=_bujcPbsD9s4SZ4f" Message-Id: <201203271813.47740.jkim@FreeBSD.org> Cc: Poul-Henning Kamp , Sevan / Venture37 , Andreas Tobler , Lars Engels Subject: [PATCH] ACPI object refcount fix X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Mar 2012 22:13:56 -0000 --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--