[PATCH v7 04/17] ARM64 / ACPI: Introduce early_param for "acpi" and pass acpi=force to enable ACPI

Hanjun Guo
2015-01-20T09:29:14Z

On 2015?01?20? 02:01, Mark Rutland wrote:
> On Mon, Jan 19, 2015 at 05:52:33PM +0000, Catalin Marinas wrote:
>> On Mon, Jan 19, 2015 at 04:59:47PM +0000, Jon Masters wrote:
>>> On 01/19/2015 10:13 AM, Grant Likely wrote:
>>>> On Mon, 19 Jan 2015 13:51:45 +0000
>>>> , Catalin Marinas <catalin.marinas@arm.com>
>>>> wrote:
>>>>> On Mon, Jan 19, 2015 at 11:55:32AM +0000, Ard Biesheuvel wrote:
>>>>>> On 19 January 2015 at 11:42, Catalin Marinas <catalin.marinas@arm.com> wrote:
>>>>>>> On Wed, Jan 14, 2015 at 03:04:52PM +0000, Hanjun Guo wrote:
>>>>>>>> From: Al Stone <al.stone@linaro.org>
>>>>>>>>
>>>>>>>> Introduce one early parameters "off" and "force" for "acpi", acpi=off
>>>>>>>> will be the default behavior for ARM64, so introduce acpi=force to
>>>>>>>> enable ACPI on ARM64.
>>>>>>>>
>>>>>>>> Disable ACPI before early parameters parsed, and enable it to pass
>>>>>>>> "acpi=force" if people want use ACPI on ARM64. This ensures DT be
>>>>>>>> the prefer one if ACPI table and DT both are provided at this moment.
>>>>>>> [...]
>>>>>>>> --- a/arch/arm64/kernel/setup.c
>>>>>>>> +++ b/arch/arm64/kernel/setup.c
>>>>>>>> @@ -62,6 +62,7 @@
>>>>>>>> #include <asm/memblock.h>
>>>>>>>> #include <asm/psci.h>
>>>>>>>> #include <asm/efi.h>
>>>>>>>> +#include <asm/acpi.h>
>>>>>>>>
>>>>>>>> unsigned int processor_id;
>>>>>>>> EXPORT_SYMBOL(processor_id);
>>>>>>>> @@ -388,6 +389,8 @@ void __init setup_arch(char **cmdline_p)
>>>>>>>> early_fixmap_init();
>>>>>>>> early_ioremap_init();
>>>>>>>>
>>>>>>>> + disable_acpi();
>>>>>>>> +
>>>>>>>> parse_early_param();
>>>>>>>>
>>>>>>>> /*
>>>>>>>
>>>>>>> Did we get to any conclusion here? DT being the preferred one is fine
>>>>>>> when both DT and ACPI are present but do we still want the kernel to
>>>>>>> ignore ACPI altogether if DT is not present? It's a bit harder to detect
>>>>>>> the presence of DT at this point since the EFI_STUB added one already. I
>>>>>>> guess we could move the "acpi=force" argument passing to EFI_STUB if no
>>>>>>> DT is present at boot.
>>>>>>
>>>>>> Since the EFI stub populates the /chosen node in DT, I would prefer
>>>>>> for it to add a property there to indicate whether it created the DT
>>>>>> from scratch rather than adding ACPI specific stuff in there (even if
>>>>>> it is just a string to concatenate)
>>>>>
>>>>> This works for me. So we could pass "acpi=force" in EFI stub if it
>>>>> created the DT from scratch *and* ACPI tables are present (can it detect
>>>>> the latter? And maybe it could print something if none are available).
>>>>> If that works, the actual kernel can assume that ACPI needs to be
>>>>> explicitly enabled via acpi=force, irrespective of how much information
>>>>> it has in DT.
>>>>
>>>> Ditto for me. I think this is a fine solution. And, yes, the stub can
>>>> easily detect the presence of ACPI by looking in the UEFI config table.
>>>
>>> I get the point behind doing this, but could we not have it pass in a
>>> different parameter than =force? Perhaps something new? I'd like to
>>> separate out the case that it was enabled automatically vs explicitly
>>> forced on by a user wanting to use ACPI on a system with both tables.
>>
>> Ard had a point, so we should probably not pass acpi=force from EFI stub
>> (especially since a user may explicitly pass acpi=off irrespective of DT
>> presence). Some other property in the chosen node? It's not even an ABI
>> since that's a contract between EFI stub and the rest of the kernel, so
>> an in-kernel only interface.
>
> Not strictly true once kexec is in place. Then it becomes a stub ->
> kernel -> kernel -> kernel -> ... interface, alnog with the rest of the
> properties the stub puts in the DTB.
>
> Having something like /chosen/linux,uefi-stub-generated-dtb sounds sane
> regardless.

How about the patch (just RFC, maybe it is horrible :) ) below:

When system supporting both DT and ACPI but firmware providing
no dtb, we can use this linux,uefi-stub-generated-dtb property
to let kernel know that we can try ACPI configuration data.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
Documentation/devicetree/bindings/chosen.txt | 19 ++++++++++++++++
arch/arm64/kernel/setup.c | 34
+++++++++++++++++++++++++++-
drivers/firmware/efi/libstub/fdt.c | 6 +++++
3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/chosen.txt
b/Documentation/devicetree/bindings/chosen.txt
index ed838f4..18776b9 100644
--- a/Documentation/devicetree/bindings/chosen.txt
+++ b/Documentation/devicetree/bindings/chosen.txt
@@ -44,3 +44,22 @@ Implementation note: Linux will look for the property
"linux,stdout-path" or
on PowerPC "stdout" if "stdout-path" is not found. However, the
"linux,stdout-path" and "stdout" properties are deprecated. New platforms
should only use the "stdout-path" property.
+
+
+linux,uefi-stub-generated-dtb property
+--------------------------------------
+
+UEFI stub will generate this property in the chosen node to let linux
kernel
+know that there is no DTB provided by firmware.
+
+There is a use case for system supporting both DT and ACPI, when firmware
+doesn't provide DT, we can try ACPI configration data to boot the system.
+
+Usage:
+
+linux,uefi-stub-generated-dtb = "true" means that it is true that the dtb
+is generated by uefi stub
+
+or
+
+linux,uefi-stub-generated-dtb = "false" is the reverse.
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 54e39e3..8268c7b 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -371,6 +371,31 @@ static void __init request_standard_resources(void)
}
}

+int __init dt_scan_chosen(unsigned long node, const char *uname,
+ int depth, void *data)
+{
+ const char *p;
+
+ if (depth != 1 || !data ||
+ (strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen at 0") != 0))
+ return 0;
+
+ p = of_get_flat_dt_prop(node, "linux,uefi-stub-generated-dtb", NULL);
+ if (!p && !strcmp(p, "true"))
+ *data = true;
+
+ return 1;
+}
+
+static bool __init is_uefi_stub_generated_dtb(void)
+{
+ bool flag = false;
+
+ of_scan_flat_dt(dt_scan_chosen, &flag);
+
+ return flag;
+}
+
u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID };

void __init setup_arch(char **cmdline_p)
@@ -389,7 +414,14 @@ void __init setup_arch(char **cmdline_p)
early_fixmap_init();
early_ioremap_init();

- disable_acpi();
+ /*
+ * If no dtb provided by firmware, enable ACPI
+ * and try to boot with ACPI configuration data
+ */
+ if (is_uefi_stub_generated_dtb())
+ enable_acpi();
+ else
+ disable_acpi();

parse_early_param();

diff --git a/drivers/firmware/efi/libstub/fdt.c
b/drivers/firmware/efi/libstub/fdt.c
index c846a96..9e2084b 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -154,6 +154,12 @@ efi_status_t update_fdt(efi_system_table_t
*sys_table, void *orig_fdt,
if (status)
goto fdt_set_fail;

+ /* Add a property to show the dtb is generated by uefi stub or not */
+ status = fdt_setprop_string(fdt, node, "linux,uefi-stub-generated-dtb",
+ orig_fdt ? "false" : "true");
+ if (status)
+ goto fdt_set_fail;
+
return EFI_SUCCESS;