On Thursday, September 13, 2007 12:36 AM, Mark.Haywood at Sun.COM wrote:
> Li, Aubrey wrote:
>> On Wednesday, September 12, 2007 5:48 PM, Mark.Haywood at Sun.COM wrote:
>>
>>
>>> Hi Aubrey,
>>>
>>> See comments at bottom ...
>>>
>>> Li, Aubrey wrote:
>>>
>>>> On Tuesday, September 11, 2007 12:39 AM, Mark.Haywood at Sun.COM
>>>> wrote:
>>>>
>>>>
>>>>> Li, Aubrey wrote:
>>>>>
>>>>>> On Friday, September 07, 2007 8:54 PM, Mark.Haywood at Sun.COM
>>>>>> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Hi Aubrey,
>>>>>>>
>>>>>>> See comments inline ...
>>>>>>>
>>>>>>> Li, Aubrey wrote:
>>>>>>>
>>> >>>>> ----------
>>>
>>>>>>>> In addition, acpica building cpu map function assumes there is
>>>>>>>> only one processor type object under _PR_ object. That causes
>>>>>>>> the processor handler is always mapped to the last Processor
>>>>>>>> Type object. This is not correct on case 2) machine, it will
>>>>>>>> cause to give a wrong handler to evaluate _PDC object. That's
>>>>>>>> another reason that _PSS not found. Here is the patch(it's
>>>>>>>> attached as well in case of whitespace coding style issue),
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>> I'll have to look at this one some more. I don't understand you
>>>>>>> explanation and I don't understand the proposed fix. Are you
>>>>>>> sure that you didn't mean
>>>>>>>
>>>>>>> if (cpu_map[cpu_id]->obj != NULL)
>>>>>>> cpu_map[cpu_id]->obj = obj;
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> Two kinds of processor configuration machines in my hand.
>>>>>>
>>>>>> 1) _PR_ has only one child CPU0, which is ok.
>>>>>> CPU0 is _PDC's parent, and it will be mapped to the cpu acpi
>>>>>> handler. So when passed the handler to evaluate _PDC, it will be
>>>>>> sucessful.
>>>>>>
>>>>>> Level = 1, parent = \___ child = _PR_
>>>>>> Level = 2, parent = _PR_ child = CPU0
>>>>>> Level = 3, parent = CPU0 child = _PPC
>>>>>> Level = 3, parent = CPU0 child = _PCT
>>>>>> Level = 3, parent = CPU0 child = _PSS
>>>>>> Level = 3, parent = CPU0 child = SPSS
>>>>>> Level = 3, parent = CPU0 child = NPSS
>>>>>> Level = 3, parent = CPU0 child = _PDC
>>>>>>
>>>>>> 2) _PR_ has two child: P001 and CPU1. and the last object CPU1 is
>>>>>> always mapped to be the cpu acpi handler.
>>>>>> However, P001 which is the parent of _PDC, should be the cpu
>>>>>> acpi handler. We should always use the first one. So when
>>>>>> **cpu_map[cpu_id]->obj == NULL **, we can assign **obj** to it.
>>>>>> Otherwise the first matched handler will be overwrite by the last
>>>>>> one. And the wrong parent parameter will be passed to evaluate
>>>>>> _PDC. So we run into failure.
>>>>>>
>>>>>> Level = 1, parent = \___ child = _PR_
>>>>>> Level = 2, parent = _PR_ child = P001
>>>>>> Level = 3, parent = P001 child = HI0_
>>>>>> Level = 3, parent = P001 child = _PDC
>>>>>> Level = 3, parent = P001 child = _OSC
>>>>>> Level = 3, parent = P001 child = _CST
>>>>>> Level = 3, parent = P001 child = _PPC
>>>>>> Level = 3, parent = P001 child = _PCT
>>>>>> Level = 3, parent = P001 child = _PSD
>>>>>> Level = 3, parent = P001 child = _PSS
>>>>>> Level = 3, parent = P001 child = SPSS
>>>>>> Level = 3, parent = P001 child = NPSS
>>>>>> Level = 2, parent = _PR_ child = CPU1
>>>>>>
> ------------------------------------------------------------------
>>>>>>
>>>>>> Did you see the same problem on Toshiba Tecra M5?
>>>>>>
>>>>>>
>>>>> This is interesting and is not something that I've seen
>>>>> before. But then
>>>>> again, given the previously existing bug with _PDC
>>>>> negotiation, we might
>>>>> not have noticed this problem. As far as I know, folks with
>>>>> Toshiba Tecra M5s are not seeing this problem.
>>>>>
>>>>>
>>>> This problem is not depend on the previously existing bug.
>>>> The current driver doesn't log _PDC failure, so we didn't notice
>>>> this problem. I can get _PSS object on that machine by iasl and
>>>> acpidump, but the driver still warns me it's not found.
>>>> So I dig into it and found _PDC evaluating failure.
>>>> So I suggest we put _PDC evaluating failure into log, on the boot
>>>> screen is not necessary.
>>>>
>>>>
>>>>> What machine had a BIOS that gave you the (_PR) Processor Tree
>>>>> object in case 2 above?
>>>>>
>>>> I'll check if the platform is release, ;-)
>>>>
>>>>
>>>>> That seems wrong to me. Or at least it seems to
>>>>> me that we
>>>>> (I) need to have a better understanding of it. I'd not have
>>>>> thought that it was possible for the ACPI Processor NameSpace to
>>>>> define two separate Processor objects for the same processor. For
>>>>> example, is it possible to have the following tree structure?
>>>>>
>>>>> Level = 1, parent = \___ child = _PR_
>>>>> Level = 2, parent = _PR_ child = CPU1
>>>>> Level = 2, parent = _PR_ child = P001
>>>>> Level = 3, parent = P001 child = HI0_
>>>>> Level = 3, parent = P001 child = _PDC
>>>>> Level = 3, parent = P001 child = _OSC
>>>>> Level = 3, parent = P001 child = _CST
>>>>> Level = 3, parent = P001 child = _PPC
>>>>> Level = 3, parent = P001 child = _PCT
>>>>> Level = 3, parent = P001 child = _PSD
>>>>> Level = 3, parent = P001 child = _PSS
>>>>> Level = 3, parent = P001 child = SPSS
>>>>> Level = 3, parent = P001 child = NPSS
>>>>>
>>>>>
>>>>> If so, then the proposed fix above would not work.
>>>>>
>>>>>
>>>> Your change of that table is so good that we don't have to change
>>>> the current code behavior. But that's the table the BIOS give out.
>>>> And the driver have to be based on it. I didn't found the
>>>> specification for it so far. At least, the current method of
>>>> building cpu acpi map doesn't cover all kinds of machine, which
>>>> need to be fixed.
>>>> I'll continue to investigate and let you know when I get something.
>>>>
>>> I ran into a system today with the following iasl dump:
>>>
>>> Scope (_PR)
>>> {
>>> Processor (P001, 0x01, 0x00000810, 0x06) {} Alias
>>> (P001, CPU1) Processor (P002, 0x02, 0x00000000, 0x00) {}
>>> Alias (P002, CPU2) Processor (P003, 0x03, 0x00000000, 0x00)
>>> {} Alias (P003, CPU3) Processor (P004, 0x04,
>>> 0x00000000, 0x00) {} Alias (P004, CPU4) }
>>>
>>> I've never see an "Alias" defined before. I'm wondering if
>>> that is what
>>> you are running into. If so, then I think the fix might be to ignore
>>> aliases when doing the mapping. Can you check to see if that's what
>>> you're running into? If so, then we'll have to figure out how to
>>> identify an "Alias".
>>>
>> Yes, I saw an "Alias" on my system. From the objects "P001" and
>> "CPU1", they have the same type and flags.
>> We have to find another way to identify this "Alias".
>> Can we simply not add these "Alias" object into the namespace when
>> we load the whole table?
>>
>>
>>> BTW, On this particular system, there really wasn't a _PSS.
>>>
>>>
>>
>> _PSS may be in another table which needs to be loaded by evaluating
>> _PDC. Can you give me the ssdt table on your system to check?
>>
> No, I already considered this. There isn't a _PDC method
> defined either.
> Additionally, when I read the IA32_MISC_ENABLE_MSR (MSR 0x1A0), it
> does not have IA32_MISC_ENABLE_EST (bit 16) enabled. I
> asked the owner to make sure that SpeedStep was enabled in the BIOS
> setup and he
> said he didn't see any way to enable/disable it. I think the BIOS for
> that system is not yet cooked.
>
So on this kind of machine, the driver will return at checking
IA32_MISC_ENABLE_MSR,
And we will not get _PSS not found warning. So, we are fine at these
machines, right?
What we should care about is the weybridge platform I'm using,
evaluating _PDC will load processor configuration table,
but with duplicate processor objects, and ACPICA is confused with them.
Thanks,
-Aubrey
