Re: [PATCH v7 04/17] ARM64 / ACPI: Introduce early_param for "acpi" and pass acpi=force to enable ACPI
From: Hanjun Guo
Date: Tue Jan 20 2015 - 07:18:34 EST
On 2015å01æ20æ 19:10, Mark Rutland wrote:
On Tue, Jan 20, 2015 at 09:29:14AM +0000, Hanjun Guo wrote:
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@xxxxxxx>
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@xxxxxxx> wrote:
On Wed, Jan 14, 2015 at 03:04:52PM +0000, Hanjun Guo wrote:
From: Al Stone <al.stone@xxxxxxxxxx>
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@xxxxxxxxxx>
---
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.
I don't think we need to list potential use cases here, this can be
useful regardless of UEFI.
OK.
The other UEFI stub properties currently live under
Documentation/arm/uefi.txt. This should live with them.
OK, will update in next version.
+
+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.
I imagined this would be an empty property. It would only be present if
the stub generated the DTB, and has no value:
/chosen {
linux,uefi-stub-generated-dtb;
};
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@0") != 0))
+ return 0;
Do we ever generate chosen@0, and do we even accept that?
Sorry, I have limited knowledge about the history of DT, so I think you
meant that I just need to check strcmp(uname, "chosen") here, right?
+
+ 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;
+
This should create an empty property, and only when generated by the
stub.
OK. could you give me some guidance to use which API to create an
empty property? I try to find but failed.
Thanks for the review!
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/