Dana.Myers wrote:
> Li, Aubrey wrote:
>> Dana.Myers wrote:
>>
>>
>>> Li, Aubrey wrote:
>>>
>>>> Liu, Jiang wrote:
>>>>
>>>>
>>>>
>>>>> Hi all.
>>>>> I have analyzed relative code with Aubrey, and could
>>>>> only give one
>>>>> possible explanation for the phenomenon. It may be caused by buggy
>>>>> ACPI BIOS implementation which returns non Package object on _CST
>>>>> evaluation, and current cpupm driver dosen't check whether the
>>>>> returned object is a package object before access it. So if a
>>>>> Integer object is returned, obj->Package.Elements will be NULL,
>>>>> then #GP happens.
>>>>> Otherwise I couldn't
>>>>> explain why "cnt = obj->Package.Elements[0].Integer.Value;" cause
>>>>> #GP. Thanks!
>>>>>
>>>>>
>>>> Right, we need to use AcpiEvaluateObjectTyped to call _CST with
>>>> ACPI_TYPE_PACKAGE if we run into this case.
>>>>
>>>>
>>> Agreed; that occurred to me this morning, that we're not
>>> checking the type of the returned object before accessing it,
>>> or using AcpiEvaluateObjectTyped(). So, yes, this is a necessary
>>> change, after we understand what it actually broken here.
>>>
>>>> But who knows how BIOS implements _CST, I'm waiting for the DSL
>>>> file.
>>>>
>>>>
>>> So, now that I've had a chance to look at the ACPI BIOS from an M10,
>>> the issue is more interesting.
>>>
>>> There's no _CST object in the disassembly; the Toshiba BIOS uses the
>>> ASL 'Load' operator to dynamically load objects as part of the
>>> per-CPU _OSC object evaluation, which is where, I'm certain, the
>>> _CST objects are coming from.
>>>
>>> This makes inspecting the relevant BIOS code a little more
>>> difficult. I suggest that we first insert a kernel printf to tell
>>> us what type of object is returned from AcpiEvaluateObject(), to
>>> research this.
>>>
>>
>> Did you receive the tar.gz file I sent?
>>
>> _CST is already there in cs0/cs1.dsl. the dynamical table can be dump
>> out by acpidump. anyway, _CST call YCST eventually, which will
>> return the package we need.
>>
>>
> Actually, will YCST return the value we need? YCST is a series of:
>
> if (....) {
> return (Package())
> }
>
> blocks. It appears to me that there might be nothing returned by
> YCST, which is what we're apparently seeing based on investigating
> the return value from AcpiEvaluateObject(). buffer Length == 0.
>
> Dana
>
>>
>> This is weird because as I pointed out, the element ptr is just
>> pointing an offset of the buffer we passed. If the kernel really
>> panic on this line, It's centernly not a ACPI CA bug. Whatever ACPI
>> CA does can't change the address. Does this make sense?
>>
> The case in which ACPI CA *could* be at fault (and I don't think it
> is now that we've had a chance to look into it) is that a smaller
> object
> is returned.
> We don't pass a buffer - it's allocated. It *could* be allocated
> smaller than expected, or not allocated at all - which is what is
> happening here.
Don't we pass the buffer when we build the object by
AcpiUtCopyIobjectToEobject()?
Yes, the buffer is allocated early, and the size depends on returned object
size.
>
> #1. AcpiEvaluateObjectTyped() will catch this error case and
> return an
> ACPI error.
Yeah, we need to change to use this interface instead.
> #2. Why is YCST apparently not returning a valid Package object?
>> But I doubt the panic place is really here, can the bug reporter
>> send the panic stack here?
Thanks,
-Aubrey