Am I reading the right patch? No need to replace AcpiEvaluateObject by AcpiEvaluateObjectTyped?
Thanks, -Aubrey Bill.Holler wrote: > Webrev has been uploaded: > > http://cr.opensolaris.org/~bholler/6819156/ > > Regards, > Bill > > > On 03/18/09 19:28, Bill Holler wrote: >> Hi tesla-dev, >> >> Please review this suggested fix for 6819156. >> If we store less cstates than in the _CST object, then re-allocate >> the ss_state buffer to the proper size to match the count. When the >> buffer is later freed in cpu_acpi_free_cstate_data() it is of the >> proper size to match the count. >> >> Regards, >> Bill >> >> -bash-3.2$ more 6819156.patch >> --- old/usr/src/uts/i86pc/os/cpupm/cpu_acpi.c Wed Mar 18 18:57:36 >> 2009 +++ new/usr/src/uts/i86pc/os/cpupm/cpu_acpi.c Wed Mar 18 >> 18:57:35 2009 @@ -688,6 +688,7 @@ ACPI_OBJECT *obj; >> ACPI_INTEGER cnt; >> cpu_acpi_cstate_t *cstate, *p; >> + size_t orig_size; >> int i, count; >> >> CPU_ACPI_OBJ_IS_NOT_CACHED(handle, CPU_ACPI_CST_CACHED); @@ >> -721,8 +722,8 @@ } >> >> CPU_ACPI_CSTATES_COUNT(handle) = (uint32_t)cnt; >> - CPU_ACPI_CSTATES(handle) = >> kmem_zalloc(CPU_ACPI_CSTATES_SIZE(cnt), >> - KM_SLEEP); >> + size = CPU_ACPI_CSTATES_SIZE(cnt); >> + CPU_ACPI_CSTATES(handle) = kmem_zalloc(size, KM_SLEEP); >> CPU_ACPI_BM_INFO(handle) = 0; >> cstate = (cpu_acpi_cstate_t *)CPU_ACPI_CSTATES(handle); >> p = cstate; @@ -775,6 +776,7 @@ >> if (count < 2) { >> cmn_err(CE_NOTE, "!cpu_acpi: _CST invalid count %d < >> 2", count); + >> kmem_free(CPU_ACPI_CSTATES(handle), size); >> AcpiOsFree(abuf.Pointer); return (-1); >> } >> @@ -782,12 +784,21 @@ >> if (cstate[0].cs_type != CPU_ACPI_C1) { >> cmn_err(CE_NOTE, "!cpu_acpi: _CST first element type >> not C1: " "%d", (int)cstate->cs_type); >> + kmem_free(CPU_ACPI_CSTATES(handle), size); >> AcpiOsFree(abuf.Pointer); >> return (-1); >> } >> >> - if (count != cnt) >> + if (count != cnt) { >> + void *orig = CPU_ACPI_CSTATES(handle); + >> CPU_ACPI_CSTATES_COUNT(handle) = (uint32_t)count; >> + CPU_ACPI_CSTATES(handle) = kmem_zalloc( >> + CPU_ACPI_CSTATES_SIZE(count), KM_SLEEP); >> + (void) memcpy(CPU_ACPI_CSTATES(handle), orig, >> + CPU_ACPI_CSTATES_SIZE(count)); >> + kmem_free(orig, size); >> + } >> >> AcpiOsFree(abuf.Pointer); >> CPU_ACPI_OBJ_IS_CACHED(handle, CPU_ACPI_CST_CACHED); >> >> >> On 03/18/09 19:04, Bill.Holler at Sun.COM wrote: >>> Sun Confidential: Internal only >>> >>> *Synopsis*: cpu_acpi_free_cstate_data can free the wrong size >>> >>> CrPrint: http://bt2ws.central.sun.com/CrPrint?id=6819156 >>> Monaco: http://monaco.sfbay.sun.com/detail.jsf?cr=6819156 >>> >>> CR 6819156 changed on Mar 19 2009 by bill.holler at sun.com >>> >>> === Field ============ === New Value ============= === Old Value >>> ============= >>> >>> Description New >>> Note >>> Evaluation New >>> Note Interest >>> List Michael.Corcoran at Sun.COM > >>> Introduced in Build >>> snv_110 > Introduced in >>> Release solaris_nevada >>> Status 6-Fix Understood >>> 1-Dispatched ====================== >>> =========================== =========================== >>> >>> *Change Request ID*: 6819156 >>> >>> *Synopsis*: cpu_acpi_free_cstate_data can free the wrong size >>> >>> Product: solaris >>> Category: kernel >>> Subcategory: sched >>> Type: Defect >>> Subtype: Functionality >>> Status: 6-Fix Understood >>> Substatus: Priority: 2-High >>> Introduced In Release: solaris_nevada >>> Introduced In Build: snv_110 >>> Responsible Manager: darrin.johnson at sun.com >>> Responsible Engineer: bill.holler at sun.com >>> Initial Evaluator: kernel-sched-ie at sun.com >>> Keywords: >>> === *Description* >>> ============================================================ >>> cpu_acpi_free_cstate_data() frees memory kmem_zalloc'ed in >>> cpu_acpi_cache_cst. The size to free is calculated with: >>> kmem_free(CPU_ACPI_CSTATES(handle), >>> CPU_ACPI_CSTATES_SIZE(CPU_ACPI_CSTATES_COUNT(handle))); >>> >>> Unfortunately the cstate count can be less than what was allocated >>> for. cpu_acpi_cache_cst() sets cstate count to be that provided by >>> _CST object: cnt = obj->Package.Elements[0].Integer.Value; >>> ... >>> CPU_ACPI_CSTATES_COUNT(handle) = (uint32_t)cnt; >>> CPU_ACPI_CSTATES(handle) = >>> kmem_zalloc(CPU_ACPI_CSTATES_SIZE(cnt), >>> KM_SLEEP); >>> The function then eliminates duplicate cstates. Each discarded >>> cstate causes the cstate count to be one less than what was >>> allocated for. >>> >>> >>> One possilble fix when cstate entries have been discarded in >>> cpu_acpi_cache_cst() is to: reallocate a smaller buffer of just the >>> right size to hold the csates as indicated in the cstate count. >>> >>> *** (#1 of 2): 2009-03-18 21:08:19 GMT+00:00 bill.holler at sun.com >>> >>> Here is an untested fix. re-allocate the ss_state buffer to the >>> corrected size: >>> >>> -bash-3.2$ more 6819156.patch >>> --- old/usr/src/uts/i86pc/os/cpupm/cpu_acpi.c Wed Mar 18 18:57:36 >>> 2009 +++ new/usr/src/uts/i86pc/os/cpupm/cpu_acpi.c Wed Mar 18 >>> 18:57:35 2009 @@ -688,6 +688,7 @@ ACPI_OBJECT *obj; >>> ACPI_INTEGER cnt; >>> cpu_acpi_cstate_t *cstate, *p; >>> + size_t orig_size; >>> int i, count; >>> >>> CPU_ACPI_OBJ_IS_NOT_CACHED(handle, CPU_ACPI_CST_CACHED); @@ >>> -721,8 +722,8 @@ } >>> >>> CPU_ACPI_CSTATES_COUNT(handle) = (uint32_t)cnt; >>> - CPU_ACPI_CSTATES(handle) = >>> kmem_zalloc(CPU_ACPI_CSTATES_SIZE(cnt), >>> - KM_SLEEP); >>> + size = CPU_ACPI_CSTATES_SIZE(cnt); >>> + CPU_ACPI_CSTATES(handle) = kmem_zalloc(size, KM_SLEEP); >>> CPU_ACPI_BM_INFO(handle) = 0; >>> cstate = (cpu_acpi_cstate_t *)CPU_ACPI_CSTATES(handle); >>> p = cstate; @@ -775,6 +776,7 @@ >>> if (count < 2) { >>> cmn_err(CE_NOTE, "!cpu_acpi: _CST invalid count %d >>> < 2", count); + >>> kmem_free(CPU_ACPI_CSTATES(handle), size); >>> AcpiOsFree(abuf.Pointer); return (-1); >>> } >>> @@ -782,12 +784,21 @@ >>> if (cstate[0].cs_type != CPU_ACPI_C1) { >>> cmn_err(CE_NOTE, "!cpu_acpi: _CST first element >>> type not C1: " "%d", (int)cstate->cs_type); >>> + kmem_free(CPU_ACPI_CSTATES(handle), size); >>> AcpiOsFree(abuf.Pointer); >>> return (-1); >>> } >>> >>> - if (count != cnt) >>> + if (count != cnt) { >>> + void *orig = CPU_ACPI_CSTATES(handle); + >>> CPU_ACPI_CSTATES_COUNT(handle) = (uint32_t)count; >>> + CPU_ACPI_CSTATES(handle) = kmem_zalloc( >>> + CPU_ACPI_CSTATES_SIZE(count), KM_SLEEP); >>> + (void) memcpy(CPU_ACPI_CSTATES(handle), orig, >>> + CPU_ACPI_CSTATES_SIZE(count)); >>> + kmem_free(orig, size); >>> + } >>> >>> AcpiOsFree(abuf.Pointer); >>> CPU_ACPI_OBJ_IS_CACHED(handle, CPU_ACPI_CST_CACHED); >>> >>> *** (#2 of 2): 2009-03-19 02:00:44 GMT+00:00 bill.holler at sun.com >>> >>> >>> === *Public Comments* >>> ======================================================== >>> >>> === *Comments* >>> =============================================================== >>> >>> === *Evaluation* >>> ============================================================= >>> See public Decription. >>> >>> *** (#1 of 1): 2009-03-19 02:00:44 GMT+00:00 bill.holler at sun.com >>> >>> >>> === *Suggested Fix* >>> ========================================================== >>> >>> === *Workaround* >>> ============================================================= >>> >>> === *Justification* >>> ========================================================== >>> Priority changed from [] to [2-High] >>> Machine can panic. >>> bill.holler at sun.com 2009-03-18 21:08:19 GMT >>> >>> *** (#1 of 1): 2009-03-18 21:08:19 GMT+00:00 bill.holler at sun.com >>> >>> >>> === *Additional Details* >>> ===================================================== >>> Targeted Release: Commit To Fix In Build: >>> Fixed In Build: Integrated In Build: Verified In >>> Build: See Also: 6819124 Duplicate of: Hooks: >>> Hook1: Hook2: Hook3: Hook4: >>> Hook5: Hook6: Interest List: Michael.Corcoran at Sun.COM, >>> tesla-dev at sun.com Program Management: Root Cause: Is a >>> Security Vulnerability?: No Fix Affects Documentation: No >>> Fix Affects Localization: No >>> Reported by: >>> === *History* >>> ================================================================ >>> Date Submitted: 2009-03-18 21:08:18 GMT+00:00 >>> Submitted By: bill.holler at sun.com >>> >>> Status Changed Date Updated Updated By >>> 6-Fix Understood 2009-03-19 02:00:44 GMT+00:00 >>> bill.holler at sun.com >>> >>> >>> === *Solution* >>> =============================================================== >>> >>> >>> === *Service Request* >>> ======================================================== >>> ID: 1-527843205 Customer: >>> Account Name: sun >>> Customer Contact: Customer Contact Role: >>> F-Functional Test Customer Contact Type: I-Internal (SMI) >>> Customer Impact: Critical Functionality: Secondary >>> Severity: 2 >>> Synopsis: Product Name: solaris >>> Product Release: solaris_nevada >>> Product Build: snv_110 >>> Operating System: snv_110 >>> Hardware: intel >>> Reference Number: Sun Contact: bill.holler at sun.com >>> Status: Open Source: BugTraq2 >>> Reproducible: Submitted By: bill.holler at sun.com >>> Submitted Date: 2009-03-18 21:08:19 GMT+00:00 >>> Description: >>> >>> === *Activity* >>> =============================================================== >>> >>> >>> === *Multiple Release (MR) Cluster* - 0 >>> ====================================== >>> >>> >>> === *Escalations* >>> ============================================================ >>> >>> >> _______________________________________________ >> tesla-dev mailing list >> tesla-dev at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/tesla-dev > > _______________________________________________ > tesla-dev mailing list > tesla-dev at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/tesla-dev
