diff mbox

[v7,05/13] ACPI/PPTT: Add Processor Properties Topology Table parsing

Message ID 20180228220619.6992-6-jeremy.linton@arm.com (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Jeremy Linton Feb. 28, 2018, 10:06 p.m. UTC
ACPI 6.2 adds a new table, which describes how processing units
are related to each other in tree like fashion. Caches are
also sprinkled throughout the tree and describe the properties
of the caches in relation to other caches and processing units.

Add the code to parse the cache hierarchy and report the total
number of levels of cache for a given core using
acpi_find_last_cache_level() as well as fill out the individual
cores cache information with cache_setup_acpi() once the
cpu_cacheinfo structure has been populated by the arch specific
code.

An additional patch later in the set adds the ability to report
peers in the topology using find_acpi_cpu_topology()
to report a unique ID for each processing unit at a given level
in the tree. These unique id's can then be used to match related
processing units which exist as threads, COD (clusters
on die), within a given package, etc.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 drivers/acpi/pptt.c | 488 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 488 insertions(+)
 create mode 100644 drivers/acpi/pptt.c

Comments

Sudeep Holla March 6, 2018, 5:39 p.m. UTC | #1
On 28/02/18 22:06, Jeremy Linton wrote:
> ACPI 6.2 adds a new table, which describes how processing units
> are related to each other in tree like fashion. Caches are
> also sprinkled throughout the tree and describe the properties
> of the caches in relation to other caches and processing units.
> 
> Add the code to parse the cache hierarchy and report the total
> number of levels of cache for a given core using
> acpi_find_last_cache_level() as well as fill out the individual
> cores cache information with cache_setup_acpi() once the
> cpu_cacheinfo structure has been populated by the arch specific
> code.
> 
> An additional patch later in the set adds the ability to report
> peers in the topology using find_acpi_cpu_topology()
> to report a unique ID for each processing unit at a given level
> in the tree. These unique id's can then be used to match related
> processing units which exist as threads, COD (clusters
> on die), within a given package, etc.
> 
> 
The more I look at the ACPI table parsing code, I get more questions
every time :). So I just skimmed through it this time. Not sure why
ACPI_PTR_* is not used elsewhere in drivers/acpi/tables.c

Anyways for cacheinfo part of this file:

Acked-by: Sudeep Holla <sudeep.holla@arm.com>
Ard Biesheuvel March 8, 2018, 4:39 p.m. UTC | #2
On 28 February 2018 at 22:06, Jeremy Linton <jeremy.linton@arm.com> wrote:
> ACPI 6.2 adds a new table, which describes how processing units
> are related to each other in tree like fashion. Caches are
> also sprinkled throughout the tree and describe the properties
> of the caches in relation to other caches and processing units.
>
> Add the code to parse the cache hierarchy and report the total
> number of levels of cache for a given core using
> acpi_find_last_cache_level() as well as fill out the individual
> cores cache information with cache_setup_acpi() once the
> cpu_cacheinfo structure has been populated by the arch specific
> code.
>
> An additional patch later in the set adds the ability to report
> peers in the topology using find_acpi_cpu_topology()
> to report a unique ID for each processing unit at a given level
> in the tree. These unique id's can then be used to match related
> processing units which exist as threads, COD (clusters
> on die), within a given package, etc.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  drivers/acpi/pptt.c | 488 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 488 insertions(+)
>  create mode 100644 drivers/acpi/pptt.c
>
> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
> new file mode 100644
> index 000000000000..883e4318c6cd
> --- /dev/null
> +++ b/drivers/acpi/pptt.c
...
> +/* total number of attributes checked by the properties code */
> +#define PPTT_CHECKED_ATTRIBUTES 4
> +
> +/*
> + * The ACPI spec implies that the fields in the cache structures are used to
> + * extend and correct the information probed from the hardware. Lets only
> + * set fields that we determine are VALID.
> + */
> +static void update_cache_properties(struct cacheinfo *this_leaf,
> +                                   struct acpi_pptt_cache *found_cache,
> +                                   struct acpi_pptt_processor *cpu_node)
> +{
> +       int valid_flags = 0;
> +
> +       if (found_cache->flags & ACPI_PPTT_SIZE_PROPERTY_VALID) {
> +               this_leaf->size = found_cache->size;
> +               valid_flags++;
> +       }
> +       if (found_cache->flags & ACPI_PPTT_LINE_SIZE_VALID) {
> +               this_leaf->coherency_line_size = found_cache->line_size;
> +               valid_flags++;
> +       }
> +       if (found_cache->flags & ACPI_PPTT_NUMBER_OF_SETS_VALID) {
> +               this_leaf->number_of_sets = found_cache->number_of_sets;
> +               valid_flags++;
> +       }
> +       if (found_cache->flags & ACPI_PPTT_ASSOCIATIVITY_VALID) {
> +               this_leaf->ways_of_associativity = found_cache->associativity;
> +               valid_flags++;
> +       }
> +       if (found_cache->flags & ACPI_PPTT_WRITE_POLICY_VALID) {
> +               switch (found_cache->attributes & ACPI_PPTT_MASK_WRITE_POLICY) {
> +               case ACPI_PPTT_CACHE_POLICY_WT:
> +                       this_leaf->attributes = CACHE_WRITE_THROUGH;
> +                       break;
> +               case ACPI_PPTT_CACHE_POLICY_WB:
> +                       this_leaf->attributes = CACHE_WRITE_BACK;
> +                       break;
> +               }
> +       }
> +       if (found_cache->flags & ACPI_PPTT_ALLOCATION_TYPE_VALID) {
> +               switch (found_cache->attributes & ACPI_PPTT_MASK_ALLOCATION_TYPE) {
> +               case ACPI_PPTT_CACHE_READ_ALLOCATE:
> +                       this_leaf->attributes |= CACHE_READ_ALLOCATE;
> +                       break;
> +               case ACPI_PPTT_CACHE_WRITE_ALLOCATE:
> +                       this_leaf->attributes |= CACHE_WRITE_ALLOCATE;
> +                       break;
> +               case ACPI_PPTT_CACHE_RW_ALLOCATE:
> +               case ACPI_PPTT_CACHE_RW_ALLOCATE_ALT:
> +                       this_leaf->attributes |=
> +                               CACHE_READ_ALLOCATE | CACHE_WRITE_ALLOCATE;
> +                       break;
> +               }
> +       }
> +       /*
> +        * If the above flags are valid, and the cache type is NOCACHE
> +        * update the cache type as well.
> +        */
> +       if ((this_leaf->type == CACHE_TYPE_NOCACHE) &&
> +           (valid_flags == PPTT_CHECKED_ATTRIBUTES))
> +               this_leaf->type = CACHE_TYPE_UNIFIED;

Why do we need the associativity and #sets attributes to be valid in
order to set the cache type?

I see how size and line size are rather fundamental properties, but
for a system cache, the geometry doesn't really matter.

> +}
> +
> +/*
> + * Update the kernel cache information for each level of cache
> + * associated with the given acpi cpu.
> + */
> +static void cache_setup_acpi_cpu(struct acpi_table_header *table,
> +                                unsigned int cpu)
> +{
> +       struct acpi_pptt_cache *found_cache;
> +       struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
> +       u32 acpi_cpu_id = get_acpi_id_for_cpu(cpu);
> +       struct cacheinfo *this_leaf;
> +       unsigned int index = 0;
> +       struct acpi_pptt_processor *cpu_node = NULL;
> +
> +       while (index < get_cpu_cacheinfo(cpu)->num_leaves) {
> +               this_leaf = this_cpu_ci->info_list + index;
> +               found_cache = acpi_find_cache_node(table, acpi_cpu_id,
> +                                                  this_leaf->type,
> +                                                  this_leaf->level,
> +                                                  &cpu_node);
> +               pr_debug("found = %p %p\n", found_cache, cpu_node);
> +               if (found_cache)
> +                       update_cache_properties(this_leaf,
> +                                               found_cache,
> +                                               cpu_node);
> +
> +               index++;
> +       }
> +}
> +
> +/**
> + * acpi_find_last_cache_level() - Determines the number of cache levels for a PE
> + * @cpu: Kernel logical cpu number
> + *
> + * Given a logical cpu number, returns the number of levels of cache represented
> + * in the PPTT. Errors caused by lack of a PPTT table, or otherwise, return 0
> + * indicating we didn't find any cache levels.
> + *
> + * Return: Cache levels visible to this core.
> + */
> +int acpi_find_last_cache_level(unsigned int cpu)
> +{
> +       u32 acpi_cpu_id;
> +       struct acpi_table_header *table;
> +       int number_of_levels = 0;
> +       acpi_status status;
> +
> +       pr_debug("Cache Setup find last level cpu=%d\n", cpu);
> +
> +       acpi_cpu_id = get_acpi_id_for_cpu(cpu);
> +       status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
> +       if (ACPI_FAILURE(status)) {
> +               pr_err_once("No PPTT table found, cache topology may be inaccurate\n");
> +       } else {
> +               number_of_levels = acpi_find_cache_levels(table, acpi_cpu_id);
> +               acpi_put_table(table);
> +       }
> +       pr_debug("Cache Setup find last level level=%d\n", number_of_levels);
> +
> +       return number_of_levels;
> +}
> +
> +/**
> + * cache_setup_acpi() - Override CPU cache topology with data from the PPTT
> + * @cpu: Kernel logical cpu number
> + *
> + * Updates the global cache info provided by cpu_get_cacheinfo()
> + * when there are valid properties in the acpi_pptt_cache nodes. A
> + * successful parse may not result in any updates if none of the
> + * cache levels have any valid flags set.  Futher, a unique value is
> + * associated with each known CPU cache entry. This unique value
> + * can be used to determine whether caches are shared between cpus.
> + *
> + * Return: -ENOENT on failure to find table, or 0 on success
> + */
> +int cache_setup_acpi(unsigned int cpu)
> +{
> +       struct acpi_table_header *table;
> +       acpi_status status;
> +
> +       pr_debug("Cache Setup ACPI cpu %d\n", cpu);
> +
> +       status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
> +       if (ACPI_FAILURE(status)) {
> +               pr_err_once("No PPTT table found, cache topology may be inaccurate\n");
> +               return -ENOENT;
> +       }
> +
> +       cache_setup_acpi_cpu(table, cpu);
> +       acpi_put_table(table);
> +
> +       return status;
> +}
> --
> 2.13.6
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeremy Linton March 8, 2018, 7:52 p.m. UTC | #3
Hi,

On 03/08/2018 10:39 AM, Ard Biesheuvel wrote:
> On 28 February 2018 at 22:06, Jeremy Linton <jeremy.linton@arm.com> wrote:
>> ACPI 6.2 adds a new table, which describes how processing units
>> are related to each other in tree like fashion. Caches are
>> also sprinkled throughout the tree and describe the properties
>> of the caches in relation to other caches and processing units.
>>
>> Add the code to parse the cache hierarchy and report the total
>> number of levels of cache for a given core using
>> acpi_find_last_cache_level() as well as fill out the individual
>> cores cache information with cache_setup_acpi() once the
>> cpu_cacheinfo structure has been populated by the arch specific
>> code.
>>
>> An additional patch later in the set adds the ability to report
>> peers in the topology using find_acpi_cpu_topology()
>> to report a unique ID for each processing unit at a given level
>> in the tree. These unique id's can then be used to match related
>> processing units which exist as threads, COD (clusters
>> on die), within a given package, etc.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>>   drivers/acpi/pptt.c | 488 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 488 insertions(+)
>>   create mode 100644 drivers/acpi/pptt.c
>>
>> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
>> new file mode 100644
>> index 000000000000..883e4318c6cd
>> --- /dev/null
>> +++ b/drivers/acpi/pptt.c
> ...
>> +/* total number of attributes checked by the properties code */
>> +#define PPTT_CHECKED_ATTRIBUTES 4
>> +
>> +/*
>> + * The ACPI spec implies that the fields in the cache structures are used to
>> + * extend and correct the information probed from the hardware. Lets only
>> + * set fields that we determine are VALID.
>> + */
>> +static void update_cache_properties(struct cacheinfo *this_leaf,
>> +                                   struct acpi_pptt_cache *found_cache,
>> +                                   struct acpi_pptt_processor *cpu_node)
>> +{
>> +       int valid_flags = 0;
>> +
>> +       if (found_cache->flags & ACPI_PPTT_SIZE_PROPERTY_VALID) {
>> +               this_leaf->size = found_cache->size;
>> +               valid_flags++;
>> +       }
>> +       if (found_cache->flags & ACPI_PPTT_LINE_SIZE_VALID) {
>> +               this_leaf->coherency_line_size = found_cache->line_size;
>> +               valid_flags++;
>> +       }
>> +       if (found_cache->flags & ACPI_PPTT_NUMBER_OF_SETS_VALID) {
>> +               this_leaf->number_of_sets = found_cache->number_of_sets;
>> +               valid_flags++;
>> +       }
>> +       if (found_cache->flags & ACPI_PPTT_ASSOCIATIVITY_VALID) {
>> +               this_leaf->ways_of_associativity = found_cache->associativity;
>> +               valid_flags++;
>> +       }
>> +       if (found_cache->flags & ACPI_PPTT_WRITE_POLICY_VALID) {
>> +               switch (found_cache->attributes & ACPI_PPTT_MASK_WRITE_POLICY) {
>> +               case ACPI_PPTT_CACHE_POLICY_WT:
>> +                       this_leaf->attributes = CACHE_WRITE_THROUGH;
>> +                       break;
>> +               case ACPI_PPTT_CACHE_POLICY_WB:
>> +                       this_leaf->attributes = CACHE_WRITE_BACK;
>> +                       break;
>> +               }
>> +       }
>> +       if (found_cache->flags & ACPI_PPTT_ALLOCATION_TYPE_VALID) {
>> +               switch (found_cache->attributes & ACPI_PPTT_MASK_ALLOCATION_TYPE) {
>> +               case ACPI_PPTT_CACHE_READ_ALLOCATE:
>> +                       this_leaf->attributes |= CACHE_READ_ALLOCATE;
>> +                       break;
>> +               case ACPI_PPTT_CACHE_WRITE_ALLOCATE:
>> +                       this_leaf->attributes |= CACHE_WRITE_ALLOCATE;
>> +                       break;
>> +               case ACPI_PPTT_CACHE_RW_ALLOCATE:
>> +               case ACPI_PPTT_CACHE_RW_ALLOCATE_ALT:
>> +                       this_leaf->attributes |=
>> +                               CACHE_READ_ALLOCATE | CACHE_WRITE_ALLOCATE;
>> +                       break;
>> +               }
>> +       }
>> +       /*
>> +        * If the above flags are valid, and the cache type is NOCACHE
>> +        * update the cache type as well.
>> +        */
>> +       if ((this_leaf->type == CACHE_TYPE_NOCACHE) &&
>> +           (valid_flags == PPTT_CHECKED_ATTRIBUTES))
>> +               this_leaf->type = CACHE_TYPE_UNIFIED;
> 
> Why do we need the associativity and #sets attributes to be valid in
> order to set the cache type?

This happened a couple revisions ago because its better to force people 
to completely populate the attributes in the tables (particularly for 
nodes we are generating, which is what the _NOCACHE check above is 
detecting) and leave half of them undefined and therefor exported to 
user-space in a way which makes the properties unreliable across machines.


> 
> I see how size and line size are rather fundamental properties, but
> for a system cache, the geometry doesn't really matter.

Originally all of them were required (to avoid having the discussion 
about which ones were "important"), but Sudeep was of the opinion that 
these were the "important" ones, I can see his point, and no one else 
spoke up... So, those are the ones that are required.


> 
>> +}
>> +
>> +/*
>> + * Update the kernel cache information for each level of cache
>> + * associated with the given acpi cpu.
>> + */
>> +static void cache_setup_acpi_cpu(struct acpi_table_header *table,
>> +                                unsigned int cpu)
>> +{
>> +       struct acpi_pptt_cache *found_cache;
>> +       struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
>> +       u32 acpi_cpu_id = get_acpi_id_for_cpu(cpu);
>> +       struct cacheinfo *this_leaf;
>> +       unsigned int index = 0;
>> +       struct acpi_pptt_processor *cpu_node = NULL;
>> +
>> +       while (index < get_cpu_cacheinfo(cpu)->num_leaves) {
>> +               this_leaf = this_cpu_ci->info_list + index;
>> +               found_cache = acpi_find_cache_node(table, acpi_cpu_id,
>> +                                                  this_leaf->type,
>> +                                                  this_leaf->level,
>> +                                                  &cpu_node);
>> +               pr_debug("found = %p %p\n", found_cache, cpu_node);
>> +               if (found_cache)
>> +                       update_cache_properties(this_leaf,
>> +                                               found_cache,
>> +                                               cpu_node);
>> +
>> +               index++;
>> +       }
>> +}
>> +
>> +/**
>> + * acpi_find_last_cache_level() - Determines the number of cache levels for a PE
>> + * @cpu: Kernel logical cpu number
>> + *
>> + * Given a logical cpu number, returns the number of levels of cache represented
>> + * in the PPTT. Errors caused by lack of a PPTT table, or otherwise, return 0
>> + * indicating we didn't find any cache levels.
>> + *
>> + * Return: Cache levels visible to this core.
>> + */
>> +int acpi_find_last_cache_level(unsigned int cpu)
>> +{
>> +       u32 acpi_cpu_id;
>> +       struct acpi_table_header *table;
>> +       int number_of_levels = 0;
>> +       acpi_status status;
>> +
>> +       pr_debug("Cache Setup find last level cpu=%d\n", cpu);
>> +
>> +       acpi_cpu_id = get_acpi_id_for_cpu(cpu);
>> +       status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
>> +       if (ACPI_FAILURE(status)) {
>> +               pr_err_once("No PPTT table found, cache topology may be inaccurate\n");
>> +       } else {
>> +               number_of_levels = acpi_find_cache_levels(table, acpi_cpu_id);
>> +               acpi_put_table(table);
>> +       }
>> +       pr_debug("Cache Setup find last level level=%d\n", number_of_levels);
>> +
>> +       return number_of_levels;
>> +}
>> +
>> +/**
>> + * cache_setup_acpi() - Override CPU cache topology with data from the PPTT
>> + * @cpu: Kernel logical cpu number
>> + *
>> + * Updates the global cache info provided by cpu_get_cacheinfo()
>> + * when there are valid properties in the acpi_pptt_cache nodes. A
>> + * successful parse may not result in any updates if none of the
>> + * cache levels have any valid flags set.  Futher, a unique value is
>> + * associated with each known CPU cache entry. This unique value
>> + * can be used to determine whether caches are shared between cpus.
>> + *
>> + * Return: -ENOENT on failure to find table, or 0 on success
>> + */
>> +int cache_setup_acpi(unsigned int cpu)
>> +{
>> +       struct acpi_table_header *table;
>> +       acpi_status status;
>> +
>> +       pr_debug("Cache Setup ACPI cpu %d\n", cpu);
>> +
>> +       status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
>> +       if (ACPI_FAILURE(status)) {
>> +               pr_err_once("No PPTT table found, cache topology may be inaccurate\n");
>> +               return -ENOENT;
>> +       }
>> +
>> +       cache_setup_acpi_cpu(table, cpu);
>> +       acpi_put_table(table);
>> +
>> +       return status;
>> +}
>> --
>> 2.13.6
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki March 19, 2018, 10:46 a.m. UTC | #4
On Wednesday, February 28, 2018 11:06:11 PM CET Jeremy Linton wrote:
> ACPI 6.2 adds a new table, which describes how processing units
> are related to each other in tree like fashion. Caches are
> also sprinkled throughout the tree and describe the properties
> of the caches in relation to other caches and processing units.
> 
> Add the code to parse the cache hierarchy and report the total
> number of levels of cache for a given core using
> acpi_find_last_cache_level() as well as fill out the individual
> cores cache information with cache_setup_acpi() once the
> cpu_cacheinfo structure has been populated by the arch specific
> code.
> 
> An additional patch later in the set adds the ability to report
> peers in the topology using find_acpi_cpu_topology()
> to report a unique ID for each processing unit at a given level
> in the tree. These unique id's can then be used to match related
> processing units which exist as threads, COD (clusters
> on die), within a given package, etc.
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

A couple of cosmetic comments.

> ---
>  drivers/acpi/pptt.c | 488 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 488 insertions(+)
>  create mode 100644 drivers/acpi/pptt.c
> 
> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
> new file mode 100644
> index 000000000000..883e4318c6cd
> --- /dev/null
> +++ b/drivers/acpi/pptt.c
> @@ -0,0 +1,488 @@

Use an SPDX license ID here and then you don't need the license boilerplate
below.

> +/*
> + * Copyright (C) 2018, ARM
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * This file implements parsing of Processor Properties Topology Table (PPTT)
> + * which is optionally used to describe the processor and cache topology.
> + * Due to the relative pointers used throughout the table, this doesn't
> + * leverage the existing subtable parsing in the kernel.
> + *
> + * The PPTT structure is an inverted tree, with each node potentially
> + * holding one or two inverted tree data structures describing
> + * the caches available at that level. Each cache structure optionally
> + * contains properties describing the cache at a given level which can be
> + * used to override hardware probed values.
> + */
> +#define pr_fmt(fmt) "ACPI PPTT: " fmt
> +
> +#include <linux/acpi.h>
> +#include <linux/cacheinfo.h>
> +#include <acpi/processor.h>
> +
> +/*
> + * Given the PPTT table, find and verify that the subtable entry
> + * is located within the table
> + */

If you add a comment above a function, make it a kerneldoc.  That
never hurts, but really helps sometimes.

> +static struct acpi_subtable_header *fetch_pptt_subtable(

Don't break the line here, it can be broken after the first arg.

That's OK even if it is more than 80 chars long.

> +	struct acpi_table_header *table_hdr, u32 pptt_ref)
> +{
> +	struct acpi_subtable_header *entry;
> +
> +	/* there isn't a subtable at reference 0 */
> +	if (pptt_ref < sizeof(struct acpi_subtable_header))
> +		return NULL;
> +
> +	if (pptt_ref + sizeof(struct acpi_subtable_header) > table_hdr->length)
> +		return NULL;
> +
> +	entry = ACPI_ADD_PTR(struct acpi_subtable_header, table_hdr, pptt_ref);
> +
> +	if (pptt_ref + entry->length > table_hdr->length)
> +		return NULL;
> +
> +	return entry;
> +}
> +
> +static struct acpi_pptt_processor *fetch_pptt_node(
> +	struct acpi_table_header *table_hdr, u32 pptt_ref)
> +{
> +	return (struct acpi_pptt_processor *)fetch_pptt_subtable(table_hdr,
> +								 pptt_ref);

You don't need to break this line too.

> +}
> +
> +static struct acpi_pptt_cache *fetch_pptt_cache(
> +	struct acpi_table_header *table_hdr, u32 pptt_ref)
> +{
> +	return (struct acpi_pptt_cache *)fetch_pptt_subtable(table_hdr,
> +							     pptt_ref);

And here.

> +}
> +
> +static struct acpi_subtable_header *acpi_get_pptt_resource(
> +	struct acpi_table_header *table_hdr,
> +	struct acpi_pptt_processor *node, int resource)
> +{
> +	u32 *ref;
> +
> +	if (resource >= node->number_of_priv_resources)
> +		return NULL;
> +
> +	ref = ACPI_ADD_PTR(u32, node, sizeof(struct acpi_pptt_processor));
> +	ref += resource;
> +
> +	return fetch_pptt_subtable(table_hdr, *ref);
> +}
> +
> +/*
> + * Match the type passed and special case the TYPE_UNIFIED so that
> + * it match both ACPI_PPTT_CACHE_TYPE_UNIFIED(_ALT) types.
> + */
> +static inline bool acpi_pptt_match_type(int table_type, int type)
> +{
> +	return (((table_type & ACPI_PPTT_MASK_CACHE_TYPE) == type) ||
> +		(table_type & ACPI_PPTT_CACHE_TYPE_UNIFIED & type));
> +}
> +
> +/*
> + * Attempt to find a given cache level, while counting the max number
> + * of cache levels for the cache node.
> + *
> + * Given a pptt resource, verify that it is a cache node, then walk
> + * down each level of caches, counting how many levels are found
> + * as well as checking the cache type (icache, dcache, unified). If a
> + * level & type match, then we set found, and continue the search.
> + * Once the entire cache branch has been walked return its max
> + * depth.
> + */
> +static int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
> +				int local_level,
> +				struct acpi_subtable_header *res,
> +				struct acpi_pptt_cache **found,
> +				int level, int type)
> +{
> +	struct acpi_pptt_cache *cache;
> +
> +	if (res->type != ACPI_PPTT_TYPE_CACHE)
> +		return 0;
> +
> +	cache = (struct acpi_pptt_cache *) res;
> +	while (cache) {
> +		local_level++;
> +
> +		if ((local_level == level) &&
> +		    (cache->flags & ACPI_PPTT_CACHE_TYPE_VALID) &&
> +		    acpi_pptt_match_type(cache->attributes, type)) {
> +			if ((*found != NULL) && (cache != *found))

Inner parens are not necessary (and above and in some other places too).

> +				pr_err("Found duplicate cache level/type unable to determine uniqueness\n");

This is not an error, rather a statement that something is inconsistent in the
ACPI table, so please consider using a different log level here.

[cut]

I guess I could post similar comments for the other general ACPI patches in
this series, so please address them in there too if applicable.

Thanks!

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeremy Linton March 20, 2018, 1:25 p.m. UTC | #5
Hi,

Thanks for taking a look at this.

On 03/19/2018 05:46 AM, Rafael J. Wysocki wrote:
> On Wednesday, February 28, 2018 11:06:11 PM CET Jeremy Linton wrote:
>> ACPI 6.2 adds a new table, which describes how processing units
>> are related to each other in tree like fashion. Caches are
>> also sprinkled throughout the tree and describe the properties
>> of the caches in relation to other caches and processing units.
>>
>> Add the code to parse the cache hierarchy and report the total
>> number of levels of cache for a given core using
>> acpi_find_last_cache_level() as well as fill out the individual
>> cores cache information with cache_setup_acpi() once the
>> cpu_cacheinfo structure has been populated by the arch specific
>> code.
>>
>> An additional patch later in the set adds the ability to report
>> peers in the topology using find_acpi_cpu_topology()
>> to report a unique ID for each processing unit at a given level
>> in the tree. These unique id's can then be used to match related
>> processing units which exist as threads, COD (clusters
>> on die), within a given package, etc.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> 
> A couple of cosmetic comments.
> 
>> ---
>>   drivers/acpi/pptt.c | 488 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 488 insertions(+)
>>   create mode 100644 drivers/acpi/pptt.c
>>
>> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
>> new file mode 100644
>> index 000000000000..883e4318c6cd
>> --- /dev/null
>> +++ b/drivers/acpi/pptt.c
>> @@ -0,0 +1,488 @@
> 
> Use an SPDX license ID here and then you don't need the license boilerplate
> below.

Sure, good point!

> 
>> +/*
>> + * Copyright (C) 2018, ARM
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * This file implements parsing of Processor Properties Topology Table (PPTT)
>> + * which is optionally used to describe the processor and cache topology.
>> + * Due to the relative pointers used throughout the table, this doesn't
>> + * leverage the existing subtable parsing in the kernel.
>> + *
>> + * The PPTT structure is an inverted tree, with each node potentially
>> + * holding one or two inverted tree data structures describing
>> + * the caches available at that level. Each cache structure optionally
>> + * contains properties describing the cache at a given level which can be
>> + * used to override hardware probed values.
>> + */
>> +#define pr_fmt(fmt) "ACPI PPTT: " fmt
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/cacheinfo.h>
>> +#include <acpi/processor.h>
>> +
>> +/*
>> + * Given the PPTT table, find and verify that the subtable entry
>> + * is located within the table
>> + */
> 
> If you add a comment above a function, make it a kerneldoc.  That
> never hurts, but really helps sometimes.

Sure, I did that for the visible symbols. I will convert the internal 
ones as well.

> 
>> +static struct acpi_subtable_header *fetch_pptt_subtable(
> 
> Don't break the line here, it can be broken after the first arg.
> 
> That's OK even if it is more than 80 chars long.
> 
>> +	struct acpi_table_header *table_hdr, u32 pptt_ref)
>> +{
>> +	struct acpi_subtable_header *entry;
>> +
>> +	/* there isn't a subtable at reference 0 */
>> +	if (pptt_ref < sizeof(struct acpi_subtable_header))
>> +		return NULL;
>> +
>> +	if (pptt_ref + sizeof(struct acpi_subtable_header) > table_hdr->length)
>> +		return NULL;
>> +
>> +	entry = ACPI_ADD_PTR(struct acpi_subtable_header, table_hdr, pptt_ref);
>> +
>> +	if (pptt_ref + entry->length > table_hdr->length)
>> +		return NULL;
>> +
>> +	return entry;
>> +}
>> +
>> +static struct acpi_pptt_processor *fetch_pptt_node(
>> +	struct acpi_table_header *table_hdr, u32 pptt_ref)
>> +{
>> +	return (struct acpi_pptt_processor *)fetch_pptt_subtable(table_hdr,
>> +								 pptt_ref);
> 
> You don't need to break this line too.

Sure.

> 
>> +}
>> +
>> +static struct acpi_pptt_cache *fetch_pptt_cache(
>> +	struct acpi_table_header *table_hdr, u32 pptt_ref)
>> +{
>> +	return (struct acpi_pptt_cache *)fetch_pptt_subtable(table_hdr,
>> +							     pptt_ref);
> 
> And here.
> 
>> +}
>> +
>> +static struct acpi_subtable_header *acpi_get_pptt_resource(
>> +	struct acpi_table_header *table_hdr,
>> +	struct acpi_pptt_processor *node, int resource)
>> +{
>> +	u32 *ref;
>> +
>> +	if (resource >= node->number_of_priv_resources)
>> +		return NULL;
>> +
>> +	ref = ACPI_ADD_PTR(u32, node, sizeof(struct acpi_pptt_processor));
>> +	ref += resource;
>> +
>> +	return fetch_pptt_subtable(table_hdr, *ref);
>> +}
>> +
>> +/*
>> + * Match the type passed and special case the TYPE_UNIFIED so that
>> + * it match both ACPI_PPTT_CACHE_TYPE_UNIFIED(_ALT) types.
>> + */
>> +static inline bool acpi_pptt_match_type(int table_type, int type)
>> +{
>> +	return (((table_type & ACPI_PPTT_MASK_CACHE_TYPE) == type) ||
>> +		(table_type & ACPI_PPTT_CACHE_TYPE_UNIFIED & type));
>> +}
>> +
>> +/*
>> + * Attempt to find a given cache level, while counting the max number
>> + * of cache levels for the cache node.
>> + *
>> + * Given a pptt resource, verify that it is a cache node, then walk
>> + * down each level of caches, counting how many levels are found
>> + * as well as checking the cache type (icache, dcache, unified). If a
>> + * level & type match, then we set found, and continue the search.
>> + * Once the entire cache branch has been walked return its max
>> + * depth.
>> + */
>> +static int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
>> +				int local_level,
>> +				struct acpi_subtable_header *res,
>> +				struct acpi_pptt_cache **found,
>> +				int level, int type)
>> +{
>> +	struct acpi_pptt_cache *cache;
>> +
>> +	if (res->type != ACPI_PPTT_TYPE_CACHE)
>> +		return 0;
>> +
>> +	cache = (struct acpi_pptt_cache *) res;
>> +	while (cache) {
>> +		local_level++;
>> +
>> +		if ((local_level == level) &&
>> +		    (cache->flags & ACPI_PPTT_CACHE_TYPE_VALID) &&
>> +		    acpi_pptt_match_type(cache->attributes, type)) {
>> +			if ((*found != NULL) && (cache != *found))
> 
> Inner parens are not necessary (and above and in some other places too).
Sure.

> 
>> +				pr_err("Found duplicate cache level/type unable to determine uniqueness\n");
> 
> This is not an error, rather a statement that something is inconsistent in the
> ACPI table, so please consider using a different log level here.

pr_warn seems to be the other common choice in the acpi directory for 
table errors, I will will use that.


> 
> [cut]
> 
> I guess I could post similar comments for the other general ACPI patches in
> this series, so please address them in there too if applicable.
> 

Ok, I will look over them again.

Thanks,


> Thanks!
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
new file mode 100644
index 000000000000..883e4318c6cd
--- /dev/null
+++ b/drivers/acpi/pptt.c
@@ -0,0 +1,488 @@ 
+/*
+ * Copyright (C) 2018, ARM
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * This file implements parsing of Processor Properties Topology Table (PPTT)
+ * which is optionally used to describe the processor and cache topology.
+ * Due to the relative pointers used throughout the table, this doesn't
+ * leverage the existing subtable parsing in the kernel.
+ *
+ * The PPTT structure is an inverted tree, with each node potentially
+ * holding one or two inverted tree data structures describing
+ * the caches available at that level. Each cache structure optionally
+ * contains properties describing the cache at a given level which can be
+ * used to override hardware probed values.
+ */
+#define pr_fmt(fmt) "ACPI PPTT: " fmt
+
+#include <linux/acpi.h>
+#include <linux/cacheinfo.h>
+#include <acpi/processor.h>
+
+/*
+ * Given the PPTT table, find and verify that the subtable entry
+ * is located within the table
+ */
+static struct acpi_subtable_header *fetch_pptt_subtable(
+	struct acpi_table_header *table_hdr, u32 pptt_ref)
+{
+	struct acpi_subtable_header *entry;
+
+	/* there isn't a subtable at reference 0 */
+	if (pptt_ref < sizeof(struct acpi_subtable_header))
+		return NULL;
+
+	if (pptt_ref + sizeof(struct acpi_subtable_header) > table_hdr->length)
+		return NULL;
+
+	entry = ACPI_ADD_PTR(struct acpi_subtable_header, table_hdr, pptt_ref);
+
+	if (pptt_ref + entry->length > table_hdr->length)
+		return NULL;
+
+	return entry;
+}
+
+static struct acpi_pptt_processor *fetch_pptt_node(
+	struct acpi_table_header *table_hdr, u32 pptt_ref)
+{
+	return (struct acpi_pptt_processor *)fetch_pptt_subtable(table_hdr,
+								 pptt_ref);
+}
+
+static struct acpi_pptt_cache *fetch_pptt_cache(
+	struct acpi_table_header *table_hdr, u32 pptt_ref)
+{
+	return (struct acpi_pptt_cache *)fetch_pptt_subtable(table_hdr,
+							     pptt_ref);
+}
+
+static struct acpi_subtable_header *acpi_get_pptt_resource(
+	struct acpi_table_header *table_hdr,
+	struct acpi_pptt_processor *node, int resource)
+{
+	u32 *ref;
+
+	if (resource >= node->number_of_priv_resources)
+		return NULL;
+
+	ref = ACPI_ADD_PTR(u32, node, sizeof(struct acpi_pptt_processor));
+	ref += resource;
+
+	return fetch_pptt_subtable(table_hdr, *ref);
+}
+
+/*
+ * Match the type passed and special case the TYPE_UNIFIED so that
+ * it match both ACPI_PPTT_CACHE_TYPE_UNIFIED(_ALT) types.
+ */
+static inline bool acpi_pptt_match_type(int table_type, int type)
+{
+	return (((table_type & ACPI_PPTT_MASK_CACHE_TYPE) == type) ||
+		(table_type & ACPI_PPTT_CACHE_TYPE_UNIFIED & type));
+}
+
+/*
+ * Attempt to find a given cache level, while counting the max number
+ * of cache levels for the cache node.
+ *
+ * Given a pptt resource, verify that it is a cache node, then walk
+ * down each level of caches, counting how many levels are found
+ * as well as checking the cache type (icache, dcache, unified). If a
+ * level & type match, then we set found, and continue the search.
+ * Once the entire cache branch has been walked return its max
+ * depth.
+ */
+static int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
+				int local_level,
+				struct acpi_subtable_header *res,
+				struct acpi_pptt_cache **found,
+				int level, int type)
+{
+	struct acpi_pptt_cache *cache;
+
+	if (res->type != ACPI_PPTT_TYPE_CACHE)
+		return 0;
+
+	cache = (struct acpi_pptt_cache *) res;
+	while (cache) {
+		local_level++;
+
+		if ((local_level == level) &&
+		    (cache->flags & ACPI_PPTT_CACHE_TYPE_VALID) &&
+		    acpi_pptt_match_type(cache->attributes, type)) {
+			if ((*found != NULL) && (cache != *found))
+				pr_err("Found duplicate cache level/type unable to determine uniqueness\n");
+
+			pr_debug("Found cache @ level %d\n", level);
+			*found = cache;
+			/*
+			 * continue looking at this node's resource list
+			 * to verify that we don't find a duplicate
+			 * cache node.
+			 */
+		}
+		cache = fetch_pptt_cache(table_hdr, cache->next_level_of_cache);
+	}
+	return local_level;
+}
+
+/*
+ * Given a CPU node look for cache levels that exist at this level, and then
+ * for each cache node, count how many levels exist below (logically above) it.
+ * If a level and type are specified, and we find that level/type, abort
+ * processing and return the acpi_pptt_cache structure.
+ */
+static struct acpi_pptt_cache *acpi_find_cache_level(
+	struct acpi_table_header *table_hdr,
+	struct acpi_pptt_processor *cpu_node,
+	int *starting_level, int level, int type)
+{
+	struct acpi_subtable_header *res;
+	int number_of_levels = *starting_level;
+	int resource = 0;
+	struct acpi_pptt_cache *ret = NULL;
+	int local_level;
+
+	/* walk down from processor node */
+	while ((res = acpi_get_pptt_resource(table_hdr, cpu_node, resource))) {
+		resource++;
+
+		local_level = acpi_pptt_walk_cache(table_hdr, *starting_level,
+						   res, &ret, level, type);
+		/*
+		 * we are looking for the max depth. Since its potentially
+		 * possible for a given node to have resources with differing
+		 * depths verify that the depth we have found is the largest.
+		 */
+		if (number_of_levels < local_level)
+			number_of_levels = local_level;
+	}
+	if (number_of_levels > *starting_level)
+		*starting_level = number_of_levels;
+
+	return ret;
+}
+
+/*
+ * Given a processor node containing a processing unit, walk into it and count
+ * how many levels exist solely for it, and then walk up each level until we hit
+ * the root node (ignore the package level because it may be possible to have
+ * caches that exist across packages). Count the number of cache levels that
+ * exist at each level on the way up.
+ */
+static int acpi_process_node(struct acpi_table_header *table_hdr,
+			     struct acpi_pptt_processor *cpu_node)
+{
+	int total_levels = 0;
+
+	do {
+		acpi_find_cache_level(table_hdr, cpu_node, &total_levels, 0, 0);
+		cpu_node = fetch_pptt_node(table_hdr, cpu_node->parent);
+	} while (cpu_node);
+
+	return total_levels;
+}
+
+/*
+ * Determine if the *node parameter is a leaf node by iterating the
+ * PPTT table, looking for nodes which reference it.
+ * Return 0 if we find a node referencing the passed node,
+ * or 1 if we don't.
+ */
+static int acpi_pptt_leaf_node(struct acpi_table_header *table_hdr,
+			       struct acpi_pptt_processor *node)
+{
+	struct acpi_subtable_header *entry;
+	unsigned long table_end;
+	u32 node_entry;
+	struct acpi_pptt_processor *cpu_node;
+	u32 proc_sz;
+
+	table_end = (unsigned long)table_hdr + table_hdr->length;
+	node_entry = ACPI_PTR_DIFF(node, table_hdr);
+	entry = ACPI_ADD_PTR(struct acpi_subtable_header, table_hdr,
+			     sizeof(struct acpi_table_pptt));
+	proc_sz = sizeof(struct acpi_pptt_processor *);
+
+	while (((unsigned long)entry + proc_sz) < table_end) {
+		cpu_node = (struct acpi_pptt_processor *)entry;
+		if ((entry->type == ACPI_PPTT_TYPE_PROCESSOR) &&
+		    (cpu_node->parent == node_entry))
+			return 0;
+		entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry,
+				     entry->length);
+	}
+	return 1;
+}
+
+/*
+ * Find the subtable entry describing the provided processor.
+ * This is done by iterating the PPTT table looking for processor nodes
+ * which have an acpi_processor_id that matches the acpi_cpu_id parameter
+ * passed into the function. If we find a node that matches this criteria
+ * we verify that its a leaf node in the topology rather than depending
+ * on the valid flag, which doesn't need to be set for leaf nodes.
+ */
+static struct acpi_pptt_processor *acpi_find_processor_node(
+	struct acpi_table_header *table_hdr,
+	u32 acpi_cpu_id)
+{
+	struct acpi_subtable_header *entry;
+	unsigned long table_end;
+	struct acpi_pptt_processor *cpu_node;
+	u32 proc_sz;
+
+	table_end = (unsigned long)table_hdr + table_hdr->length;
+	entry = ACPI_ADD_PTR(struct acpi_subtable_header, table_hdr,
+			     sizeof(struct acpi_table_pptt));
+	proc_sz = sizeof(struct acpi_pptt_processor *);
+
+	/* find the processor structure associated with this cpuid */
+	while (((unsigned long)entry + proc_sz) < table_end) {
+		cpu_node = (struct acpi_pptt_processor *)entry;
+
+		if (entry->length == 0) {
+			pr_err("Invalid zero length subtable\n");
+			break;
+		}
+		if ((entry->type == ACPI_PPTT_TYPE_PROCESSOR) &&
+		    (acpi_cpu_id == cpu_node->acpi_processor_id) &&
+		     acpi_pptt_leaf_node(table_hdr, cpu_node)) {
+			return (struct acpi_pptt_processor *)entry;
+		}
+
+		entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry,
+				     entry->length);
+	}
+
+	return NULL;
+}
+
+static int acpi_find_cache_levels(struct acpi_table_header *table_hdr,
+				  u32 acpi_cpu_id)
+{
+	int number_of_levels = 0;
+	struct acpi_pptt_processor *cpu;
+
+	cpu = acpi_find_processor_node(table_hdr, acpi_cpu_id);
+	if (cpu)
+		number_of_levels = acpi_process_node(table_hdr, cpu);
+
+	return number_of_levels;
+}
+
+/* Convert the linux cache_type to a ACPI PPTT cache type value */
+static u8 acpi_cache_type(enum cache_type type)
+{
+	switch (type) {
+	case CACHE_TYPE_DATA:
+		pr_debug("Looking for data cache\n");
+		return ACPI_PPTT_CACHE_TYPE_DATA;
+	case CACHE_TYPE_INST:
+		pr_debug("Looking for instruction cache\n");
+		return ACPI_PPTT_CACHE_TYPE_INSTR;
+	default:
+	case CACHE_TYPE_UNIFIED:
+		pr_debug("Looking for unified cache\n");
+		/*
+		 * It is important that ACPI_PPTT_CACHE_TYPE_UNIFIED
+		 * contains the bit pattern that will match both
+		 * ACPI unified bit patterns because we use it later
+		 * to match both cases.
+		 */
+		return ACPI_PPTT_CACHE_TYPE_UNIFIED;
+	}
+}
+
+/* find the ACPI node describing the cache type/level for the given CPU */
+static struct acpi_pptt_cache *acpi_find_cache_node(
+	struct acpi_table_header *table_hdr, u32 acpi_cpu_id,
+	enum cache_type type, unsigned int level,
+	struct acpi_pptt_processor **node)
+{
+	int total_levels = 0;
+	struct acpi_pptt_cache *found = NULL;
+	struct acpi_pptt_processor *cpu_node;
+	u8 acpi_type = acpi_cache_type(type);
+
+	pr_debug("Looking for CPU %d's level %d cache type %d\n",
+		 acpi_cpu_id, level, acpi_type);
+
+	cpu_node = acpi_find_processor_node(table_hdr, acpi_cpu_id);
+
+	while ((cpu_node) && (!found)) {
+		found = acpi_find_cache_level(table_hdr, cpu_node,
+					      &total_levels, level, acpi_type);
+		*node = cpu_node;
+		cpu_node = fetch_pptt_node(table_hdr, cpu_node->parent);
+	}
+
+	return found;
+}
+
+/* total number of attributes checked by the properties code */
+#define PPTT_CHECKED_ATTRIBUTES 4
+
+/*
+ * The ACPI spec implies that the fields in the cache structures are used to
+ * extend and correct the information probed from the hardware. Lets only
+ * set fields that we determine are VALID.
+ */
+static void update_cache_properties(struct cacheinfo *this_leaf,
+				    struct acpi_pptt_cache *found_cache,
+				    struct acpi_pptt_processor *cpu_node)
+{
+	int valid_flags = 0;
+
+	if (found_cache->flags & ACPI_PPTT_SIZE_PROPERTY_VALID) {
+		this_leaf->size = found_cache->size;
+		valid_flags++;
+	}
+	if (found_cache->flags & ACPI_PPTT_LINE_SIZE_VALID) {
+		this_leaf->coherency_line_size = found_cache->line_size;
+		valid_flags++;
+	}
+	if (found_cache->flags & ACPI_PPTT_NUMBER_OF_SETS_VALID) {
+		this_leaf->number_of_sets = found_cache->number_of_sets;
+		valid_flags++;
+	}
+	if (found_cache->flags & ACPI_PPTT_ASSOCIATIVITY_VALID) {
+		this_leaf->ways_of_associativity = found_cache->associativity;
+		valid_flags++;
+	}
+	if (found_cache->flags & ACPI_PPTT_WRITE_POLICY_VALID) {
+		switch (found_cache->attributes & ACPI_PPTT_MASK_WRITE_POLICY) {
+		case ACPI_PPTT_CACHE_POLICY_WT:
+			this_leaf->attributes = CACHE_WRITE_THROUGH;
+			break;
+		case ACPI_PPTT_CACHE_POLICY_WB:
+			this_leaf->attributes = CACHE_WRITE_BACK;
+			break;
+		}
+	}
+	if (found_cache->flags & ACPI_PPTT_ALLOCATION_TYPE_VALID) {
+		switch (found_cache->attributes & ACPI_PPTT_MASK_ALLOCATION_TYPE) {
+		case ACPI_PPTT_CACHE_READ_ALLOCATE:
+			this_leaf->attributes |= CACHE_READ_ALLOCATE;
+			break;
+		case ACPI_PPTT_CACHE_WRITE_ALLOCATE:
+			this_leaf->attributes |= CACHE_WRITE_ALLOCATE;
+			break;
+		case ACPI_PPTT_CACHE_RW_ALLOCATE:
+		case ACPI_PPTT_CACHE_RW_ALLOCATE_ALT:
+			this_leaf->attributes |=
+				CACHE_READ_ALLOCATE | CACHE_WRITE_ALLOCATE;
+			break;
+		}
+	}
+	/*
+	 * If the above flags are valid, and the cache type is NOCACHE
+	 * update the cache type as well.
+	 */
+	if ((this_leaf->type == CACHE_TYPE_NOCACHE) &&
+	    (valid_flags == PPTT_CHECKED_ATTRIBUTES))
+		this_leaf->type = CACHE_TYPE_UNIFIED;
+}
+
+/*
+ * Update the kernel cache information for each level of cache
+ * associated with the given acpi cpu.
+ */
+static void cache_setup_acpi_cpu(struct acpi_table_header *table,
+				 unsigned int cpu)
+{
+	struct acpi_pptt_cache *found_cache;
+	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
+	u32 acpi_cpu_id = get_acpi_id_for_cpu(cpu);
+	struct cacheinfo *this_leaf;
+	unsigned int index = 0;
+	struct acpi_pptt_processor *cpu_node = NULL;
+
+	while (index < get_cpu_cacheinfo(cpu)->num_leaves) {
+		this_leaf = this_cpu_ci->info_list + index;
+		found_cache = acpi_find_cache_node(table, acpi_cpu_id,
+						   this_leaf->type,
+						   this_leaf->level,
+						   &cpu_node);
+		pr_debug("found = %p %p\n", found_cache, cpu_node);
+		if (found_cache)
+			update_cache_properties(this_leaf,
+						found_cache,
+						cpu_node);
+
+		index++;
+	}
+}
+
+/**
+ * acpi_find_last_cache_level() - Determines the number of cache levels for a PE
+ * @cpu: Kernel logical cpu number
+ *
+ * Given a logical cpu number, returns the number of levels of cache represented
+ * in the PPTT. Errors caused by lack of a PPTT table, or otherwise, return 0
+ * indicating we didn't find any cache levels.
+ *
+ * Return: Cache levels visible to this core.
+ */
+int acpi_find_last_cache_level(unsigned int cpu)
+{
+	u32 acpi_cpu_id;
+	struct acpi_table_header *table;
+	int number_of_levels = 0;
+	acpi_status status;
+
+	pr_debug("Cache Setup find last level cpu=%d\n", cpu);
+
+	acpi_cpu_id = get_acpi_id_for_cpu(cpu);
+	status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
+	if (ACPI_FAILURE(status)) {
+		pr_err_once("No PPTT table found, cache topology may be inaccurate\n");
+	} else {
+		number_of_levels = acpi_find_cache_levels(table, acpi_cpu_id);
+		acpi_put_table(table);
+	}
+	pr_debug("Cache Setup find last level level=%d\n", number_of_levels);
+
+	return number_of_levels;
+}
+
+/**
+ * cache_setup_acpi() - Override CPU cache topology with data from the PPTT
+ * @cpu: Kernel logical cpu number
+ *
+ * Updates the global cache info provided by cpu_get_cacheinfo()
+ * when there are valid properties in the acpi_pptt_cache nodes. A
+ * successful parse may not result in any updates if none of the
+ * cache levels have any valid flags set.  Futher, a unique value is
+ * associated with each known CPU cache entry. This unique value
+ * can be used to determine whether caches are shared between cpus.
+ *
+ * Return: -ENOENT on failure to find table, or 0 on success
+ */
+int cache_setup_acpi(unsigned int cpu)
+{
+	struct acpi_table_header *table;
+	acpi_status status;
+
+	pr_debug("Cache Setup ACPI cpu %d\n", cpu);
+
+	status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
+	if (ACPI_FAILURE(status)) {
+		pr_err_once("No PPTT table found, cache topology may be inaccurate\n");
+		return -ENOENT;
+	}
+
+	cache_setup_acpi_cpu(table, cpu);
+	acpi_put_table(table);
+
+	return status;
+}