Myron,
I think you could ignore this person... I've spent more than 2 weeks
explaining
him my changes to EC, but he insisted on quality of his "analysis",
until was proven
to be completely wrong by test run on his own hardware.
If you have any questions about why he is so vocal, ask Len -- he is the
manager of
Yakui...
Regards,
Alex.
Zhao Yakui wrote:
On Thu, 2008-10-23 at 10:11 -0600, Myron Stowe wrote:
On Thu, 2008-10-23 at 17:32 +0800, Zhao Yakui wrote:
On Wed, 2008-10-22 at 13:10 -0600, Myron Stowe wrote:
Len, Alexey:
The following three item patch series fixes an issue with the introduction
of > 256 processor declaration support: "Allow processor to be declared with
the Device() instead of Processor()" (git SHA 11bf04c4).
The root issue is in the lsapic mapping logic of drivers/acpi/processor_core.c.
Currently, the logic tries both types of matches irregardless of declaration
type and relies on one failing. According to the spec - lsapic mapping is
dependent on how the processor object was declared: CPUs declared using the
"Processor" statement should use the Local SAPIC's 'processor_id', and CPUs
declared using the "Device" statement should use the 'uid'.
Reference: Section 8.4 Declaring Processors; Section 5.2.11.13 Local SAPIC
Structure. "Advanced Configuration and Power Interface Specification",
Revision 3.0b, October 2006.
[PATCH 1/3] disambiguates the processor declaration type that is currently
conflated so that subsequent logic can behave based explicitly on the
declaration's type. I expect the disambiguation this patch introduces will
also be advantageous when extending the > 256 processor support for x86 via
x2APIC.
In the patch the new HID("ACPI_CPU") for processor is introduced. Is it
required?
If there exists the _UID object under the scope of Processor, it won't
work well.
I'm not sure I understand your concern. If you are concerned about the
case of a "Processor" type declaration that has a _UID object within
it's scope then this patch will handle such correctly (by the way - some
of our past platforms do have this combination). This is basically what
[PATCH 1/3] addresses - setting the underlying infrastructure so that
subsequent mapping related logic - [PATCH2/3] - can explicitly
disambiguate between "Processor" declarations and "Device" declarations
and use the correct component corresponding to the declaration type
used.
In most cases there is no _UID object under the scope of Processor
namespace. And in such cases it is unnecessary to add the new
HID("ACPI_CPU") for the processor.
But if there exists the _UID object under the scope of processor
namespace , IMO this patch is not appropriate. The ACPI_ID returned by
_UID object should have higher priority than the ACPI_ID defined in the
processor definition. In such case OS should match the ACPI_ID obtained
from the _UID with slapic table to get the correct processor ID.
Based on the above analysis IMO it is unnecessary to add the new
HID("ACPI_CPU") for the processor.
Thanks.
If I misunderstood your question, or I am understanding your question
but you don't see how [PATCH 1/3] solves your concern then please ask
again.
According to the spec the returned value of the _UID object for
processor/device can be numeric value or a string. If it is a numeric
value, we should match it with the ACPI_UID field of slapic table to get
the APIC id. If it is string, we should match it with the ACPI_UID
string field of slapic table get the APIC ID.
Correct - thus my "warning" so to speak in the last paragraph of the
original posting. I was going to tackle this and it turned out to get
pretty messy fast. Fortunately the error path printk in
acpi_processor_get_info() when a "Device" declaration with a _UID that
is *not* an integer is encountered will trigger and indicate exactly
where the issue is at some future date when a vendor utilizes this
combination.
Thanks for your reply!
Myron
[PATCH 2/3] addresses the root issue as stated above. With respect to this
patch I'm ambivalent about the 'printk' introduced in "map_lsapic_id()" -
perhaps it should be removed?
[PATCH 3/3] is non-functional white space/spelling fixes in the related code.
While the specific fix is ia64 focused the underlying code affected is common
to both x86 and ia64. I have tested on the following platform/namespace
combinations:
ia64 with "Processor" type namespace processor declarations,
ia64 with "Device" type namespace processor declarations,
x86 with "Processor" type namespace processor declarations.
Note that this patch series does *not* handle "Device" type processor
declarations that contain a string type _UID object under the processor
device's scope (I am currently not aware of any platforms that have such to
test against).
All comments welcome.
Myron
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
