diff mbox series

[RFC,1/2] ACPI/PPTT: Add acpi_pptt_get_package_info() API

Message ID 1580210059-199540-2-git-send-email-john.garry@huawei.com (mailing list archive)
State RFC, archived
Headers show
Series Add basic generic ACPI soc driver | expand

Commit Message

John Garry Jan. 28, 2020, 11:14 a.m. UTC
The ACPI PPTT ID structure (see 6.2 spec, section 5.2.29.3) allows the
vendor to provide an identifier (or vendor specific part number) for a
particular processor hierarchy node structure. That may be a processor
identifier for a processor node, or some chip identifier for a processor
package node.

In some circumstances it can be useful to learn the SoC package identifiers
in the system. An example is in [0], where the userspace perf tool needs
to know the SoC identifier for certain per-SoC event matching. So for this
purpose, add an API to get ID structure members for a processor package
node index, which may be used by some driver to expose this info to
userspace.

The ID structure table has a number of fields, which are left open to
interpretation per implementation. However the spec does provide reference
examples of how the fields could be used. As such, just provide the
table fields directly in the API, which the caller may interpret (probably
as per spec example).

https://lore.kernel.org/linux-arm-kernel/1579876505-113251-6-git-send-email-john.garry@huawei.com/

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/acpi/pptt.c  | 81 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/acpi.h | 13 +++++++
 2 files changed, 94 insertions(+)

Comments

Sudeep Holla Jan. 28, 2020, 12:34 p.m. UTC | #1
On Tue, Jan 28, 2020 at 07:14:18PM +0800, John Garry wrote:
> The ACPI PPTT ID structure (see 6.2 spec, section 5.2.29.3) allows the
> vendor to provide an identifier (or vendor specific part number) for a
> particular processor hierarchy node structure. That may be a processor
> identifier for a processor node, or some chip identifier for a processor
> package node.
>

Unfortunately, there were plans to deprecate this in favour of the new
SOC_ID SMCCC API[1]. I am not sure if you or anyone in your company have
access to UEFI ASWG mantis where you can look for the ECR for the PPTT
Type 2 deprecation. I understand it's not ideal, but we need to converge,
please take a look at both before further discussion.

I personally would not prefer to add the support when I know it is getting
deprecated. I am not sure on kernel community policy on the same.


[...]

>
> The ID structure table has a number of fields, which are left open to
> interpretation per implementation. However the spec does provide reference
> examples of how the fields could be used. As such, just provide the
> table fields directly in the API, which the caller may interpret (probably
> as per spec example).
>

The "open for interpretation" part is why it's not being favoured anymore
by silicon vendors as OEM/ODMs can override the same.

> https://lore.kernel.org/linux-arm-kernel/1579876505-113251-6-git-send-email-john.garry@huawei.com/
>
Ah, there's already quite a lot of dependency built for this feature :(

--
Regards,
Sudeep

[1] https://developer.arm.com/docs/den0028/c
John Garry Jan. 28, 2020, 2:04 p.m. UTC | #2
On 28/01/2020 12:34, Sudeep Holla wrote:

Hi Sudeep,

> On Tue, Jan 28, 2020 at 07:14:18PM +0800, John Garry wrote:
>> The ACPI PPTT ID structure (see 6.2 spec, section 5.2.29.3) allows the
>> vendor to provide an identifier (or vendor specific part number) for a
>> particular processor hierarchy node structure. That may be a processor
>> identifier for a processor node, or some chip identifier for a processor
>> package node.
>>
> 
> Unfortunately, there were plans to deprecate this in favour of the new
> SOC_ID SMCCC API[1]. I am not sure if you or anyone in your company have
> access to UEFI ASWG mantis where you can look for the ECR for the PPTT
> Type 2 deprecation.

I wasn't aware and I can't get access...

Personally I would rather PPTT ID structure have a fixed field 
definition in future spec versions, rather than deprecate.

 From checking here, nobody has even used it (properly) for processor 
package nodes:
https://github.com/tianocore/edk2-platforms/tree/master/Platform

  I understand it's not ideal, but we need to converge,
> please take a look at both before further discussion.

I can only check the SMCCC extension which you pointed me at.

> 
> I personally would not prefer to add the support when I know it is getting
> deprecated. I am not sure on kernel community policy on the same.

So I need a generic solution for this, as my userspace tool requires a 
generic solution.

> 
> 
> [...]
> 
>>
>> The ID structure table has a number of fields, which are left open to
>> interpretation per implementation. However the spec does provide reference
>> examples of how the fields could be used. As such, just provide the
>> table fields directly in the API, which the caller may interpret (probably
>> as per spec example).
>>
> 
> The "open for interpretation" part is why it's not being favoured anymore
> by silicon vendors as OEM/ODMs can override the same.
> 
>> https://lore.kernel.org/linux-arm-kernel/1579876505-113251-6-git-send-email-john.garry@huawei.com/
>>
> Ah, there's already quite a lot of dependency built for this feature :(

Not really. It's only an RFC ATM, and my requirement is a sysfs file to 
read the SoC id(s) (under ACPI FW). So I would still expect to be able 
to support this from the SMCCC extension method.

> 
> --
> Regards,
> Sudeep
> 
> [1] https://developer.arm.com/docs/den0028/c
> .
> 

Cheers,
John
Sudeep Holla Jan. 28, 2020, 2:54 p.m. UTC | #3
On Tue, Jan 28, 2020 at 02:04:19PM +0000, John Garry wrote:
> On 28/01/2020 12:34, Sudeep Holla wrote:
>
> Hi Sudeep,
>
> > On Tue, Jan 28, 2020 at 07:14:18PM +0800, John Garry wrote:
> > > The ACPI PPTT ID structure (see 6.2 spec, section 5.2.29.3) allows the
> > > vendor to provide an identifier (or vendor specific part number) for a
> > > particular processor hierarchy node structure. That may be a processor
> > > identifier for a processor node, or some chip identifier for a processor
> > > package node.
> > >
> >
> > Unfortunately, there were plans to deprecate this in favour of the new
> > SOC_ID SMCCC API[1]. I am not sure if you or anyone in your company have
> > access to UEFI ASWG mantis where you can look for the ECR for the PPTT
> > Type 2 deprecation.
>
> I wasn't aware and I can't get access...
>

I can understand, it is not well published/advertised.

> Personally I would rather PPTT ID structure have a fixed field definition in
> future spec versions, rather than deprecate.
>
> From checking here, nobody has even used it (properly) for processor package
> nodes:
> https://github.com/tianocore/edk2-platforms/tree/master/Platform
>

Yes, that was one of the things we looked at when we started with SOC_ID
SMCCC API and proposal to deprecate PPTT Type 2 table.

> > I understand it's not ideal, but we need to converge,
> > please take a look at both before further discussion.
>
> I can only check the SMCCC extension which you pointed me at.
>

Sure, that will at-least give you what SMCCC SOC_ID API looks like.

> >
> > I personally would not prefer to add the support when I know it is getting
> > deprecated. I am not sure on kernel community policy on the same.
>
> So I need a generic solution for this, as my userspace tool requires a
> generic solution.
>

Yes I agree on the generic solution and the soc driver you have proposed
in the patch. No objections there, just the source of the information
needs to be changed. Instead of ACPI PPTT Type 2 table, it needs to be
SOC_ID SMCCC v1.2 API

> >
> >
> > [...]
> >
> > >
> > > The ID structure table has a number of fields, which are left open to
> > > interpretation per implementation. However the spec does provide reference
> > > examples of how the fields could be used. As such, just provide the
> > > table fields directly in the API, which the caller may interpret (probably
> > > as per spec example).
> > >
> >
> > The "open for interpretation" part is why it's not being favoured anymore
> > by silicon vendors as OEM/ODMs can override the same.
> >
> > > https://lore.kernel.org/linux-arm-kernel/1579876505-113251-6-git-send-email-john.garry@huawei.com/
> > >
> > Ah, there's already quite a lot of dependency built for this feature :(
>
> Not really. It's only an RFC ATM, and my requirement is a sysfs file to read
> the SoC id(s) (under ACPI FW). So I would still expect to be able to support
> this from the SMCCC extension method.
>

As mentioned above, yes the driver would remain almost same for SMCCC
SOC_ID support too. The main point was: do we need to add support to
PPTT Type 2 entry when we know there is proposal to deprecate it. I
would at-least wait to see progress on that until I would add this to
the kernel.

--
Regards,
Sudeep
John Garry Jan. 29, 2020, 11:03 a.m. UTC | #4
On 28/01/2020 14:54, Sudeep Holla wrote:
>>>> https://lore.kernel.org/linux-arm-kernel/1579876505-113251-6-git-send-email-john.garry@huawei.com/
>>>>
>>> Ah, there's already quite a lot of dependency built for this feature:(
>> Not really. It's only an RFC ATM, and my requirement is a sysfs file to read
>> the SoC id(s) (under ACPI FW). So I would still expect to be able to support
>> this from the SMCCC extension method.
>>

Hi Sudeep,

> As mentioned above, yes the driver would remain almost same for SMCCC
> SOC_ID support too. The main point was: do we need to add support to
> PPTT Type 2 entry when we know there is proposal to deprecate it. I
> would at-least wait to see progress on that until I would add this to
> the kernel.

Not having support in the kernel for a feature which may be officially 
deprecated, i.e public knowledge, in future (maybe 1 year) seems harsh. 
Especially when there is no final released alternative.

So do you know if other OSes use it?

Thanks
John
Sudeep Holla Jan. 30, 2020, 11:23 a.m. UTC | #5
On Tue, Jan 28, 2020 at 12:34:15PM +0000, Sudeep Holla wrote:
> On Tue, Jan 28, 2020 at 07:14:18PM +0800, John Garry wrote:
> > The ACPI PPTT ID structure (see 6.2 spec, section 5.2.29.3) allows the
> > vendor to provide an identifier (or vendor specific part number) for a
> > particular processor hierarchy node structure. That may be a processor
> > identifier for a processor node, or some chip identifier for a processor
> > package node.
> >
>
> Unfortunately, there were plans to deprecate this in favour of the new
> SOC_ID SMCCC API[1]. I am not sure if you or anyone in your company have
> access to UEFI ASWG mantis where you can look for the ECR for the PPTT
> Type 2 deprecation. I understand it's not ideal, but we need to converge,
> please take a look at both before further discussion.
>
> I personally would not prefer to add the support when I know it is getting
> deprecated. I am not sure on kernel community policy on the same.
>

OK, the details on the proposal to deprecate can be now found in UEFI
bugzilla [1]

--
Regards,
Sudeep

[1] https://bugzilla.tianocore.org/show_bug.cgi?id=2492
John Garry Jan. 30, 2020, 4:12 p.m. UTC | #6
On 30/01/2020 11:23, Sudeep Holla wrote:
>> I personally would not prefer to add the support when I know it is getting
>> deprecated. I am not sure on kernel community policy on the same.
>>
> OK, the details on the proposal to deprecate can be now found in UEFI
> bugzilla [1]
> 

Wouldn't it be a better approach to propose deprecating the field when 
there is a readily available alternative, i.e. not a spec from a 
different body in beta stage?

To me, this new SMC support will take an appreciable amount of time to 
be implemented in FW by SiPs when actually released. And if it requires 
an ATF upgrade - which I guess it does - then that's a big job.

Thanks,
John

> --
> Regards,
> Sudeep
> 
> [1]https://bugzilla.tianocore.org/show_bug.cgi?id=2492
> .
Sudeep Holla Jan. 30, 2020, 5:41 p.m. UTC | #7
On Thu, Jan 30, 2020 at 04:12:20PM +0000, John Garry wrote:
> On 30/01/2020 11:23, Sudeep Holla wrote:
> > > I personally would not prefer to add the support when I know it is getting
> > > deprecated. I am not sure on kernel community policy on the same.
> > >
> > OK, the details on the proposal to deprecate can be now found in UEFI
> > bugzilla [1]
> >
>
> Wouldn't it be a better approach to propose deprecating the field when there
> is a readily available alternative, i.e. not a spec from a different body in
> beta stage?
>

Understandable and valid concerns. It would be helpful if you raise it in
the UEFI bugzilla. Your concerns will get lost if you just raise here.

> To me, this new SMC support will take an appreciable amount of time to be
> implemented in FW by SiPs when actually released. And if it requires an ATF
> upgrade - which I guess it does - then that's a big job.
>

Again I do understand, please raise it with the SMCCC specification contact
as listed in the link I shared.

--
Regards,
Sudeep
John Garry Jan. 31, 2020, 10:58 a.m. UTC | #8
On 30/01/2020 17:41, Sudeep Holla wrote:
> On Thu, Jan 30, 2020 at 04:12:20PM +0000, John Garry wrote:
>> On 30/01/2020 11:23, Sudeep Holla wrote:
>>>> I personally would not prefer to add the support when I know it is getting
>>>> deprecated. I am not sure on kernel community policy on the same.
>>>>
>>> OK, the details on the proposal to deprecate can be now found in UEFI
>>> bugzilla [1]
>>>
>>
>> Wouldn't it be a better approach to propose deprecating the field when there
>> is a readily available alternative, i.e. not a spec from a different body in
>> beta stage?
>>
> 
> Understandable and valid concerns. It would be helpful if you raise it in
> the UEFI bugzilla. Your concerns will get lost if you just raise here.

ok, thanks. I can do that if and when I can get an account...

> 
>> To me, this new SMC support will take an appreciable amount of time to be
>> implemented in FW by SiPs when actually released. And if it requires an ATF
>> upgrade - which I guess it does - then that's a big job.
>>
> 
> Again I do understand, please raise it with the SMCCC specification contact
> as listed in the link I shared.
> 

I will talk with my FW guys first when they return from CNY.

Cheers,
john
diff mbox series

Patch

diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
index f31544d3656e..ea4ed6300d0b 100644
--- a/drivers/acpi/pptt.c
+++ b/drivers/acpi/pptt.c
@@ -760,3 +760,84 @@  int find_acpi_cpu_topology_hetero_id(unsigned int cpu)
 	return find_acpi_cpu_topology_tag(cpu, PPTT_ABORT_PACKAGE,
 					  ACPI_PPTT_ACPI_IDENTICAL);
 }
+
+/**
+ * acpi_pptt_get_package_info() - Get processor package information
+ * @index: Index into processor package
+ * @info: Pointer to structure to fill in processor package info
+ *
+ * For a particular processor package index, fill in the acpi_pptt_package_info
+ * structure.
+ *
+ * Return: -ENOENT if the PPTT or processor package index doesn't exist,
+ *	   -EINVAL for invalid arguments, 0 for success.
+ */
+int acpi_pptt_get_package_info(int index, struct acpi_pptt_package_info *info)
+{
+	struct acpi_subtable_header *entry;
+	struct acpi_table_header *table;
+	unsigned long table_end;
+	acpi_status status;
+	int ret, count = 0;
+
+	if (!info)
+		return -EINVAL;
+
+	status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
+	if (ACPI_FAILURE(status)) {
+		acpi_pptt_warn_missing();
+		return -ENOENT;
+	}
+
+	table_end = (unsigned long)table + table->length;
+	entry = ACPI_ADD_PTR(struct acpi_subtable_header, table,
+			     sizeof(struct acpi_table_pptt));
+
+	ret = -ENOENT;
+	while (entry) {
+		struct acpi_pptt_processor *cpu_node;
+
+		cpu_node = (struct acpi_pptt_processor *)entry;
+
+		if (entry->type == ACPI_PPTT_TYPE_PROCESSOR &&
+		    cpu_node->flags & ACPI_PPTT_PHYSICAL_PACKAGE) {
+			int cnt = cpu_node->number_of_priv_resources;
+			int i;
+
+			for (i = 0; i < cnt; i++) {
+				struct acpi_subtable_header *r;
+
+				r = acpi_get_pptt_resource(table, cpu_node, i);
+
+				if (r->type == ACPI_PPTT_TYPE_ID &&
+				    count == index) {
+					struct acpi_pptt_id *id;
+
+					id = (struct acpi_pptt_id *)r;
+					info->LEVEL_2_ID =
+						le64_to_cpu(id->level2_id);
+					info->vendor_id =
+						le32_to_cpu(id->vendor_id);
+
+					ret = 0;
+					goto out;
+				}
+
+				if (r->type == ACPI_PPTT_TYPE_ID)
+					count++;
+			}
+		}
+
+		entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry,
+				     entry->length);
+		if ((unsigned long)entry >= table_end)
+			break;
+	}
+
+out:
+	acpi_put_table(table);
+
+	return ret;
+
+}
+EXPORT_SYMBOL_GPL(acpi_pptt_get_package_info);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 0f37a7d5fa77..0a911a298731 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1268,13 +1268,26 @@  static inline int lpit_read_residency_count_address(u64 *address)
 }
 #endif
 
+struct acpi_pptt_package_info {
+	u64 LEVEL_2_ID;
+	u32 vendor_id;
+};
+
 #ifdef CONFIG_ACPI_PPTT
 int acpi_pptt_cpu_is_thread(unsigned int cpu);
 int find_acpi_cpu_topology(unsigned int cpu, int level);
 int find_acpi_cpu_topology_package(unsigned int cpu);
 int find_acpi_cpu_topology_hetero_id(unsigned int cpu);
 int find_acpi_cpu_cache_topology(unsigned int cpu, int level);
+int acpi_pptt_get_package_info(int index, struct acpi_pptt_package_info *info);
 #else
+static inline int acpi_pptt_get_package_info(int index,
+					     struct acpi_pptt_package_info *info);
+{
+	return -EINVAL;
+
+}
+
 static inline int acpi_pptt_cpu_is_thread(unsigned int cpu)
 {
 	return -EINVAL;