lkml.org 
[lkml]   [2022]   [Feb]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH] ACPI / amba: Skip creating amba device when associated cpu is not online
From
Date
Hi Suzuki,


在 2022/1/28 19:00, Suzuki K Poulose 写道:
> Hi Xiang
>
> On 07/01/2022 08:41, chenxiang wrote:
>> From: Xiang Chen <chenxiang66@hisilicon.com>
>>
>> If not up all the cpus with command line "maxcpus=x", system will be
>> blocked.
>> We find that some amba devices such as ETM devices, are associated with
>> special cpus, and if the cpu is not up, the register of associated
>> device
>> is not allowed to access. BIOS reports all the ETM device nodes and a
>> amba device is created for every ETM device, so even if one cpu is
>> not up,
>> the amba device will still be created for the associated device, and
>> also
>> the register of device (pid and cid) will be accessed when adding amba
>> device which will cause the issue.
>> To fix it, skip creating amba device if it is associated with a cpu
>> which
>> is not online.
>
> I understand the issue. We do not have an issue at least on DT based
> platforms with a similar environment (Juno). The key is the power
> management for the components.
>
> There are two separate issues at play here :
>
> 1) Power management with ACPI. I believe there is a solution in progress
> to address this.
>
> 2) The ETM is in the same power domain as that of the CPU and normal
> device power management may not work without the CPU being online.
>
> 3) Additionally we have other issue of supporting system instructions
> with ACPI, which do not appear on the AMBA bus.
>
> Considering all of these, the ideal solution is to :
>
> 1) Implement power management for ACPI, which is anyway in progress
> (at least for SCMI based systems)
> 2) Move the ETM driver away from the AMBA framework. That would make
> the CPU online problem and the (3) above easier to solve.
> Anshuman is going to look into this.
>
> In the meantime, we could have this temporary fix in and solve it
> forever by moving away from the AMBA.

Ok, thanks.
Looking forward to the ideal soluation and i am glad to test it after
implemented.

>
>>
>> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
>> ---
>> drivers/acpi/acpi_amba.c | 36 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 36 insertions(+)
>>
>> diff --git a/drivers/acpi/acpi_amba.c b/drivers/acpi/acpi_amba.c
>> index ab8a4e0191b1..2369198f734b 100644
>> --- a/drivers/acpi/acpi_amba.c
>> +++ b/drivers/acpi/acpi_amba.c
>> @@ -16,6 +16,7 @@
>> #include <linux/ioport.h>
>> #include <linux/kernel.h>
>> #include <linux/module.h>
>> +#include <acpi/processor.h>
>> #include "internal.h"
>> @@ -45,6 +46,35 @@ static void amba_register_dummy_clk(void)
>> clk_register_clkdev(amba_dummy_clk, "apb_pclk", NULL);
>> }
>> +static int acpi_handle_to_cpuid(acpi_handle handle)
>> +{
>> + int cpu = -1;
>> + struct acpi_processor *pr;
>> +
>> + for_each_possible_cpu(cpu) {
>> + pr = per_cpu(processors, cpu);
>> + if (pr && pr->handle == handle)
>> + break;
>> + }
>> +
>> + return cpu;
>> +}
>> +
>
> Please could we reuse the function in coresight-platform.c ?
> i.e, move it to a generic location and share it, rather than
> duplicating it ?
>
>
>> +static int acpi_dev_get_cpu(struct acpi_device *adev)
>> +{
>> + acpi_handle cpu_handle;
>> + acpi_status status;
>> + int cpu;
>> +
>> + status = acpi_get_parent(adev->handle, &cpu_handle);
>> + if (ACPI_FAILURE(status))
>> + return -1;
>> + cpu = acpi_handle_to_cpuid(cpu_handle);
>> + if (cpu >= nr_cpu_ids)
>> + return -1;
>> + return cpu;
>> +}
>> +
>> static int amba_handler_attach(struct acpi_device *adev,
>> const struct acpi_device_id *id)
>> {
>> @@ -54,11 +84,17 @@ static int amba_handler_attach(struct acpi_device
>> *adev,
>> bool address_found = false;
>> int irq_no = 0;
>> int ret;
>> + int cpu;
>> /* If the ACPI node already has a physical device attached,
>> skip it. */
>> if (adev->physical_node_count)
>> return 0;
>> + /* If the cpu associated with the device is not online, skip
>> it. */
>> + cpu = acpi_dev_get_cpu(adev);
>> + if (cpu >= 0 && !cpu_online(cpu))
>> + return 0;
>> +
>
> Except for the comment above, the patch looks good to me.
>
> Suzuki
> .
>

\
 
 \ /
  Last update: 2022-02-09 08:24    [W:0.119 / U:1.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site