Hi,
On 2/12/20 8:41 AM, John Garry wrote:
On 12/02/2020 13:55, Sudeep Holla wrote:
On Wed, Feb 12, 2020 at 12:48:33PM +0000, John Garry wrote:
On 12/02/2020 11:59, Sudeep Holla wrote:
[...]
Hi Sudeep,
Yes, as mentioned above. We are not going to do extra work for lazy
firmware.
I don't think it's reasonable to just label this as lazy. The table
may just
not have the flag set unintentionally. FW and software guys make
mistakes,
like the mistakes in PPTT, itself.
We are not talking about flags, it's UID and it is pretty important if
there are more than one objects of same time.
I am talking about the Processor ID valid flag, which is specifically
related.
Linux also will be lazy on such platform and provide weird unique
numbers
like in the above case you have mentioned.
Personally I think that the kernel can be do better than provide
meaningless
values like this, since it knows the processor IDs and which physical
package they belong to.
This was discussed quite a lot, I can dig and point you to it. That's the
reason for choosing offset. We are *not going back* to this again. Fix
the
firmware before it gets copied for all future platforms and Linux has to
deal with that *forever*.
I would liked to have been made aware earlier of the oversight. Quite
often we only find problems when someone or something complains.
It is a strange API to provide offsets like this, and I did not realize
that they were actually being exposed to userspace.
If not, at least make the user know of potential deficiencies in the
table.
How ? What are your suggestions ? Does adding a warning or note that UID
is missing and offset is chosen help ?
I'd say so. I know now, but let's save others the potential hassle. And
having this debate again.
I am kind of fine with that.
How about something like this:
--- a/drivers/acpi/pptt.c
+++ b/drivers/acpi/pptt.c
@@ -515,6 +515,8 @@ static int topology_get_acpi_cpu_tag(struct
acpi_table_header *table,
if (level == 0 || cpu_node->flags & ACPI_PPTT_ACPI_PROCESSOR_ID_VALID)
return cpu_node->acpi_processor_id;
+ if (level == PPTT_ABORT_PACKAGE)
+ pr_warn_once("ACPI Processor ID valid not set for physical package
node, will use node table offset as substitute for UID\n");
To clarify my other email there, since I can't seem to type clearly..
Just note that find_acpi_cpu_topology_hetero_id() is also using a
PPTT_ABORT_PACKAGE termination.
return ACPI_PTR_DIFF(cpu_node, table);
}
pr_warn_once("PPTT table found, but unable to locate core %d
(%d)\n",
Thanks,
John
