| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0jizZELraQsS0gP0ALL60wyhT2F-7no5p5KKmg7Spi6uw@mail.gmail.com>
Date: Thu, 21 Jul 2022 11:54:05 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Sudeep Holla <sudeep.holla@....com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Dietmar Eggemann <dietmar.eggemann@....com>,
Pierre Gondois <pierre.gondois@....com>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
linux-riscv <linux-riscv@...ts.infradead.org>,
Guenter Roeck <linux@...ck-us.net>,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
Conor Dooley <conor.dooley@...rochip.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Geert Uytterhoeven <geert+renesas@...der.be>,
Ionela Voinescu <ionela.voinescu@....com>
Subject: Re: [PATCH v3 2/3] ACPI: PPTT: Leave the table mapped for the runtime usage
On Wed, Jul 20, 2022 at 2:56 PM Sudeep Holla <sudeep.holla@....com> wrote:
>
> Currently, everytime an information needs to be fetched from the PPTT,
> the table is mapped via acpi_get_table() and unmapped after the use via
> acpi_put_table() which is fine. However we do this at runtime especially
> when the CPU is hotplugged out and plugged in back since we re-populate
> the cache topology and other information.
>
> However, with the support to fetch LLC information from the PPTT in the
> cpuhotplug path which is executed in the atomic context, it is preferred
> to avoid mapping and unmapping of the PPTT for every single use as the
> acpi_get_table() might sleep waiting for a mutex.
>
> In order to avoid the same, the table is needs to just mapped once on
> the boot CPU and is never unmapped allowing it to be used at runtime
> with out the hassle of mapping and unmapping the table.
>
> Reported-by: Guenter Roeck <linux@...ck-us.net>
> Cc: Rafael J. Wysocki <rafael@...nel.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@....com>
>
> --
>
> Hi Rafael,
>
> Sorry to bother you again on this PPTT changes. Guenter reported an issue
> with lockdep enabled in -next that include my cacheinfo/arch_topology changes
> to utilise LLC from PPTT in the CPU hotplug path.
>
> Please ack the change once you are happy so that I can get it merged with
> other fixes via Greg's tree.
OK
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
> index dd3222a15c9c..c91342dcbcd6 100644
> --- a/drivers/acpi/pptt.c
> +++ b/drivers/acpi/pptt.c
> @@ -533,21 +533,37 @@ static int topology_get_acpi_cpu_tag(struct acpi_table_header *table,
> return -ENOENT;
> }
>
> +
> +static struct acpi_table_header *acpi_get_pptt(void)
> +{
> + static struct acpi_table_header *pptt;
> + acpi_status status;
> +
> + /*
> + * PPTT will be used at runtime on every CPU hotplug in path, so we
> + * don't need to call acpi_put_table() to release the table mapping.
> + */
> + if (!pptt) {
> + status = acpi_get_table(ACPI_SIG_PPTT, 0, &pptt);
> + if (ACPI_FAILURE(status))
> + acpi_pptt_warn_missing();
> + }
> +
> + return pptt;
> +}
> +
> static int find_acpi_cpu_topology_tag(unsigned int cpu, int level, int flag)
> {
> struct acpi_table_header *table;
> - acpi_status status;
> int retval;
>
> - status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
> - if (ACPI_FAILURE(status)) {
> - acpi_pptt_warn_missing();
> + table = acpi_get_pptt();
> + if (!table)
> return -ENOENT;
> - }
> +
> retval = topology_get_acpi_cpu_tag(table, cpu, level, flag);
> pr_debug("Topology Setup ACPI CPU %d, level %d ret = %d\n",
> cpu, level, retval);
> - acpi_put_table(table);
>
> return retval;
> }
> @@ -568,16 +584,13 @@ static int find_acpi_cpu_topology_tag(unsigned int cpu, int level, int flag)
> static int check_acpi_cpu_flag(unsigned int cpu, int rev, u32 flag)
> {
> struct acpi_table_header *table;
> - acpi_status status;
> u32 acpi_cpu_id = get_acpi_id_for_cpu(cpu);
> struct acpi_pptt_processor *cpu_node = NULL;
> int ret = -ENOENT;
>
> - status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
> - if (ACPI_FAILURE(status)) {
> - acpi_pptt_warn_missing();
> - return ret;
> - }
> + table = acpi_get_pptt();
> + if (!table)
> + return -ENOENT;
>
> if (table->revision >= rev)
> cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
> @@ -585,8 +598,6 @@ static int check_acpi_cpu_flag(unsigned int cpu, int rev, u32 flag)
> if (cpu_node)
> ret = (cpu_node->flags & flag) != 0;
>
> - acpi_put_table(table);
> -
> return ret;
> }
>
> @@ -605,18 +616,15 @@ 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;
> +
> + table = acpi_get_pptt();
> + if (!table)
> + return -ENOENT;
>
> 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)) {
> - acpi_pptt_warn_missing();
> - } else {
> - number_of_levels = acpi_find_cache_levels(table, acpi_cpu_id);
> - acpi_put_table(table);
> - }
> + number_of_levels = acpi_find_cache_levels(table, acpi_cpu_id);
> pr_debug("Cache Setup find last level level=%d\n", number_of_levels);
>
> return number_of_levels;
> @@ -638,20 +646,16 @@ int acpi_find_last_cache_level(unsigned int cpu)
> 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)) {
> - acpi_pptt_warn_missing();
> + table = acpi_get_pptt();
> + if (!table)
> return -ENOENT;
> - }
> +
> + pr_debug("Cache Setup ACPI CPU %d\n", cpu);
>
> cache_setup_acpi_cpu(table, cpu);
> - acpi_put_table(table);
>
> - return status;
> + return 0;
> }
>
> /**
> @@ -730,50 +734,38 @@ int find_acpi_cpu_topology_package(unsigned int cpu)
> int find_acpi_cpu_topology_cluster(unsigned int cpu)
> {
> struct acpi_table_header *table;
> - acpi_status status;
> struct acpi_pptt_processor *cpu_node, *cluster_node;
> u32 acpi_cpu_id;
> int retval;
> int is_thread;
>
> - status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
> - if (ACPI_FAILURE(status)) {
> - acpi_pptt_warn_missing();
> + table = acpi_get_pptt();
> + if (!table)
> return -ENOENT;
> - }
>
> acpi_cpu_id = get_acpi_id_for_cpu(cpu);
> cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
> - if (cpu_node == NULL || !cpu_node->parent) {
> - retval = -ENOENT;
> - goto put_table;
> - }
> + if (!cpu_node || !cpu_node->parent)
> + return -ENOENT;
>
> is_thread = cpu_node->flags & ACPI_PPTT_ACPI_PROCESSOR_IS_THREAD;
> cluster_node = fetch_pptt_node(table, cpu_node->parent);
> - if (cluster_node == NULL) {
> - retval = -ENOENT;
> - goto put_table;
> - }
> + if (!cluster_node)
> + return -ENOENT;
> +
> if (is_thread) {
> - if (!cluster_node->parent) {
> - retval = -ENOENT;
> - goto put_table;
> - }
> + if (!cluster_node->parent)
> + return -ENOENT;
> +
> cluster_node = fetch_pptt_node(table, cluster_node->parent);
> - if (cluster_node == NULL) {
> - retval = -ENOENT;
> - goto put_table;
> - }
> + if (!cluster_node)
> + return -ENOENT;
> }
> if (cluster_node->flags & ACPI_PPTT_ACPI_PROCESSOR_ID_VALID)
> retval = cluster_node->acpi_processor_id;
> else
> retval = ACPI_PTR_DIFF(cluster_node, table);
>
> -put_table:
> - acpi_put_table(table);
> -
> return retval;
> }
>
>
> --
> b4 0.10.0-dev-54fef
Powered by blists - more mailing lists