[PATCH v2 0/2] preserve FDT blob and present as /sys/firmware/fdt

[PATCH v2 0/2] preserve FDT blob and present as /sys/firmware/fdt

[Ard Biesheuvel]

Meh, I am not necessarily a lot happier with this version, now split into a
generic and a arch specific part, but it addresses Grant's concerns with
respect to modification, without resorting to checksums.

Checksums only solve half of the problem, because /not/ presenting the
blob at all if the checksum is incorrect is not really an option imo.

Ard Biesheuvel (2):
  of/fdt: export fdt blob as /sys/firmware/fdt
  arm64: fdt: call preserve_fdt() before unflattening it

 arch/arm64/kernel/head.S  |  3 +++
 arch/arm64/kernel/setup.c |  1 +
 drivers/of/fdt.c          | 34 ++++++++++++++++++++++++++++++++++
 include/linux/of_fdt.h    |  2 ++
 4 files changed, 40 insertions(+)

-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt

[Ard Biesheuvel]

Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
that was passed to the kernel by the bootloader. This allows userland
applications such as kexec to access the raw binary. The blob needs to
be preserved as early as possible by calling preserve_fdt().

The fact that this node does not reside under /sys/firmware/device-tree
is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
communicate just the UEFI and ACPI entry points, but the FDT is never
unflattened and used to configure the system.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/of/fdt.c       | 34 ++++++++++++++++++++++++++++++++++
 include/linux/of_fdt.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d1ffca8b34ea..e9ee3d5f7ea4 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -22,6 +22,7 @@
 #include <linux/libfdt.h>
 #include <linux/debugfs.h>
 #include <linux/serial_core.h>
+#include <linux/sysfs.h>
 
 #include <asm/setup.h>  /* for COMMAND_LINE_SIZE */
 #include <asm/page.h>
@@ -1103,4 +1104,37 @@ static int __init of_flat_dt_debugfs_export_fdt(void)
 module_init(of_flat_dt_debugfs_export_fdt);
 #endif
 
+static u8 *raw_fdt_copy;
+
+void __init preserve_fdt(void)
+{
+	u32 fdt_size;
+
+	fdt_size = fdt_totalsize(initial_boot_params);
+	raw_fdt_copy = memcpy(__va(memblock_alloc(fdt_size, PAGE_SIZE)),
+			      initial_boot_params, fdt_size);
+}
+
+#ifdef CONFIG_SYSFS
+static ssize_t of_fdt_raw_read(struct file *filp, struct kobject *kobj,
+			       struct bin_attribute *bin_attr,
+			       char *buf, loff_t off, size_t count)
+{
+	memcpy(buf, raw_fdt_copy + off, count);
+	return count;
+}
+
+static int __init of_fdt_raw_init(void)
+{
+	static struct bin_attribute of_fdt_raw_attr =
+		__BIN_ATTR(fdt, S_IRUSR, of_fdt_raw_read, NULL, 0);
+
+	if (!raw_fdt_copy)
+		return 0;
+	of_fdt_raw_attr.size = fdt_totalsize(raw_fdt_copy);
+	return sysfs_create_bin_file(firmware_kobj, &of_fdt_raw_attr);
+}
+module_init(of_fdt_raw_init);
+#endif
+
 #endif /* CONFIG_OF_EARLY_FLATTREE */
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 0ff360d5b3b3..7672a26305a5 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -83,6 +83,7 @@ extern const void *of_flat_dt_match_machine(const void *default_match,
 /* Other Prototypes */
 extern void unflatten_device_tree(void);
 extern void unflatten_and_copy_device_tree(void);
+extern void preserve_fdt(void);
 extern void early_init_devtree(void *);
 extern void early_get_first_memblock_info(void *, phys_addr_t *);
 extern u64 fdt_translate_address(const void *blob, int node_offset);
@@ -92,6 +93,7 @@ static inline void early_init_fdt_scan_reserved_mem(void) {}
 static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
 static inline void unflatten_device_tree(void) {}
 static inline void unflatten_and_copy_device_tree(void) {}
+static inline void preserve_fdt(void) {}
 #endif /* CONFIG_OF_FLATTREE */
 
 #endif /* __ASSEMBLY__ */
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] arm64: fdt: call preserve_fdt() before unflattening it

[Ard Biesheuvel]

To support the new /sys/firmware/fdt entry that gives access to
the FDT blob as passed by the bootloader, call preserve_fdt()
in the early arch code to make a copy of it before the
unflattening code gets its hands on it.

Also, make the early mapping of the FDT read only so we are certain
it will not get clobbered by anything called by setup_machine_fdt(),
which handles the FDT blob even earlier.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm64/kernel/head.S  | 3 +++
 arch/arm64/kernel/setup.c | 1 +
 2 files changed, 4 insertions(+)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 0a6e4f924df8..df25dffcda9f 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -79,8 +79,10 @@
 
 #ifdef CONFIG_ARM64_64K_PAGES
 #define MM_MMUFLAGS	PTE_ATTRINDX(MT_NORMAL) | PTE_FLAGS
+#define MM_MMUFLAGS_RO	PTE_ATTRINDX(MT_NORMAL) | PTE_FLAGS | PTE_RDONLY
 #else
 #define MM_MMUFLAGS	PMD_ATTRINDX(MT_NORMAL) | PMD_FLAGS
+#define MM_MMUFLAGS_RO	PMD_ATTRINDX(MT_NORMAL) | PMD_FLAGS | PMD_SECT_RDONLY
 #endif
 
 /*
@@ -607,6 +609,7 @@ __create_page_tables:
 	 * Map the FDT blob (maximum 2MB; must be within 512MB of
 	 * PHYS_OFFSET).
 	 */
+	ldr	x7, =MM_MMUFLAGS_RO
 	mov	x3, x21				// FDT phys address
 	and	x3, x3, #~((1 << 21) - 1)	// 2MB aligned
 	mov	x6, #PAGE_OFFSET
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 2437196cc5d4..1b535cc42981 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -394,6 +394,7 @@ void __init setup_arch(char **cmdline_p)
 
 	efi_idmap_init();
 
+	preserve_fdt();
 	unflatten_device_tree();
 
 	psci_init();
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt

[Grant Likely]

On Mon, 10 Nov 2014 19:47:01 +0100
, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
 wrote:
> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
> that was passed to the kernel by the bootloader. This allows userland
> applications such as kexec to access the raw binary. The blob needs to
> be preserved as early as possible by calling preserve_fdt().
> 
> The fact that this node does not reside under /sys/firmware/device-tree
> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
> communicate just the UEFI and ACPI entry points, but the FDT is never
> unflattened and used to configure the system.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On further thought, nak. I want tree modifications to be treated as
bugs. Do a checksum CRC32 or check. It's simpler and it can be extended
to us an in-blob CRC when the file format gets upreved to include one.

g.

---

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index be16ce2ffd69..adda0a16934f 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -23,6 +23,7 @@ config OF_FLATTREE
 	bool
 	select DTC
 	select LIBFDT
+	select CRC
 
 config OF_EARLY_FLATTREE
 	bool
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 83a8e1154602..80db46a2eab9 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -10,6 +10,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/crc32.h>
 #include <linux/initrd.h>
 #include <linux/memblock.h>
 #include <linux/of.h>
@@ -420,6 +421,7 @@ int __initdata dt_root_addr_cells;
 int __initdata dt_root_size_cells;
 
 void *initial_boot_params;
+u32 initial_boot_params_crc;
 
 #ifdef CONFIG_OF_EARLY_FLATTREE
 
@@ -1003,6 +1005,9 @@ bool __init early_init_dt_verify(void *params)
 
 	/* Setup flat device-tree pointer */
 	initial_boot_params = params;
+	initial_boot_params_crc = crc32_be(0, params, fdt_totalsize(params));
+	pr_info("FDT CRC: %x\n", initial_boot_params_crc);
+
 	return true;
 }
 



> ---
>  drivers/of/fdt.c       | 34 ++++++++++++++++++++++++++++++++++
>  include/linux/of_fdt.h |  2 ++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index d1ffca8b34ea..e9ee3d5f7ea4 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -22,6 +22,7 @@
>  #include <linux/libfdt.h>
>  #include <linux/debugfs.h>
>  #include <linux/serial_core.h>
> +#include <linux/sysfs.h>
>  
>  #include <asm/setup.h>  /* for COMMAND_LINE_SIZE */
>  #include <asm/page.h>
> @@ -1103,4 +1104,37 @@ static int __init of_flat_dt_debugfs_export_fdt(void)
>  module_init(of_flat_dt_debugfs_export_fdt);
>  #endif
>  
> +static u8 *raw_fdt_copy;
> +
> +void __init preserve_fdt(void)
> +{
> +	u32 fdt_size;
> +
> +	fdt_size = fdt_totalsize(initial_boot_params);
> +	raw_fdt_copy = memcpy(__va(memblock_alloc(fdt_size, PAGE_SIZE)),
> +			      initial_boot_params, fdt_size);
> +}
> +
> +#ifdef CONFIG_SYSFS
> +static ssize_t of_fdt_raw_read(struct file *filp, struct kobject *kobj,
> +			       struct bin_attribute *bin_attr,
> +			       char *buf, loff_t off, size_t count)
> +{
> +	memcpy(buf, raw_fdt_copy + off, count);
> +	return count;
> +}
> +
> +static int __init of_fdt_raw_init(void)
> +{
> +	static struct bin_attribute of_fdt_raw_attr =
> +		__BIN_ATTR(fdt, S_IRUSR, of_fdt_raw_read, NULL, 0);
> +
> +	if (!raw_fdt_copy)
> +		return 0;
> +	of_fdt_raw_attr.size = fdt_totalsize(raw_fdt_copy);
> +	return sysfs_create_bin_file(firmware_kobj, &of_fdt_raw_attr);
> +}
> +module_init(of_fdt_raw_init);
> +#endif
> +
>  #endif /* CONFIG_OF_EARLY_FLATTREE */
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 0ff360d5b3b3..7672a26305a5 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -83,6 +83,7 @@ extern const void *of_flat_dt_match_machine(const void *default_match,
>  /* Other Prototypes */
>  extern void unflatten_device_tree(void);
>  extern void unflatten_and_copy_device_tree(void);
> +extern void preserve_fdt(void);
>  extern void early_init_devtree(void *);
>  extern void early_get_first_memblock_info(void *, phys_addr_t *);
>  extern u64 fdt_translate_address(const void *blob, int node_offset);
> @@ -92,6 +93,7 @@ static inline void early_init_fdt_scan_reserved_mem(void) {}
>  static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
>  static inline void unflatten_device_tree(void) {}
>  static inline void unflatten_and_copy_device_tree(void) {}
> +static inline void preserve_fdt(void) {}
>  #endif /* CONFIG_OF_FLATTREE */
>  
>  #endif /* __ASSEMBLY__ */
> -- 
> 1.8.3.2
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt

[Ard Biesheuvel]

On 11 November 2014 15:42, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Mon, 10 Nov 2014 19:47:01 +0100
> , Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>  wrote:
>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
>> that was passed to the kernel by the bootloader. This allows userland
>> applications such as kexec to access the raw binary. The blob needs to
>> be preserved as early as possible by calling preserve_fdt().
>>
>> The fact that this node does not reside under /sys/firmware/device-tree
>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
>> communicate just the UEFI and ACPI entry points, but the FDT is never
>> unflattened and used to configure the system.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> On further thought, nak. I want tree modifications to be treated as
> bugs. Do a checksum CRC32 or check. It's simpler and it can be extended
> to us an in-blob CRC when the file format gets upreved to include one.
>

Ah, right, I was under the impression that tree modifications were
expected and allowed.
In that case, yes, let me respin to calculate a CRC early on, and
WARN() if it gets corrupted.

-- 
Ard.


> g.
>
> ---
>
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index be16ce2ffd69..adda0a16934f 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -23,6 +23,7 @@ config OF_FLATTREE
>         bool
>         select DTC
>         select LIBFDT
> +       select CRC
>
>  config OF_EARLY_FLATTREE
>         bool
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 83a8e1154602..80db46a2eab9 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -10,6 +10,7 @@
>   */
>
>  #include <linux/kernel.h>
> +#include <linux/crc32.h>
>  #include <linux/initrd.h>
>  #include <linux/memblock.h>
>  #include <linux/of.h>
> @@ -420,6 +421,7 @@ int __initdata dt_root_addr_cells;
>  int __initdata dt_root_size_cells;
>
>  void *initial_boot_params;
> +u32 initial_boot_params_crc;
>
>  #ifdef CONFIG_OF_EARLY_FLATTREE
>
> @@ -1003,6 +1005,9 @@ bool __init early_init_dt_verify(void *params)
>
>         /* Setup flat device-tree pointer */
>         initial_boot_params = params;
> +       initial_boot_params_crc = crc32_be(0, params, fdt_totalsize(params));
> +       pr_info("FDT CRC: %x\n", initial_boot_params_crc);
> +
>         return true;
>  }
>
>
>
>
>> ---
>>  drivers/of/fdt.c       | 34 ++++++++++++++++++++++++++++++++++
>>  include/linux/of_fdt.h |  2 ++
>>  2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index d1ffca8b34ea..e9ee3d5f7ea4 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -22,6 +22,7 @@
>>  #include <linux/libfdt.h>
>>  #include <linux/debugfs.h>
>>  #include <linux/serial_core.h>
>> +#include <linux/sysfs.h>
>>
>>  #include <asm/setup.h>  /* for COMMAND_LINE_SIZE */
>>  #include <asm/page.h>
>> @@ -1103,4 +1104,37 @@ static int __init of_flat_dt_debugfs_export_fdt(void)
>>  module_init(of_flat_dt_debugfs_export_fdt);
>>  #endif
>>
>> +static u8 *raw_fdt_copy;
>> +
>> +void __init preserve_fdt(void)
>> +{
>> +     u32 fdt_size;
>> +
>> +     fdt_size = fdt_totalsize(initial_boot_params);
>> +     raw_fdt_copy = memcpy(__va(memblock_alloc(fdt_size, PAGE_SIZE)),
>> +                           initial_boot_params, fdt_size);
>> +}
>> +
>> +#ifdef CONFIG_SYSFS
>> +static ssize_t of_fdt_raw_read(struct file *filp, struct kobject *kobj,
>> +                            struct bin_attribute *bin_attr,
>> +                            char *buf, loff_t off, size_t count)
>> +{
>> +     memcpy(buf, raw_fdt_copy + off, count);
>> +     return count;
>> +}
>> +
>> +static int __init of_fdt_raw_init(void)
>> +{
>> +     static struct bin_attribute of_fdt_raw_attr =
>> +             __BIN_ATTR(fdt, S_IRUSR, of_fdt_raw_read, NULL, 0);
>> +
>> +     if (!raw_fdt_copy)
>> +             return 0;
>> +     of_fdt_raw_attr.size = fdt_totalsize(raw_fdt_copy);
>> +     return sysfs_create_bin_file(firmware_kobj, &of_fdt_raw_attr);
>> +}
>> +module_init(of_fdt_raw_init);
>> +#endif
>> +
>>  #endif /* CONFIG_OF_EARLY_FLATTREE */
>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>> index 0ff360d5b3b3..7672a26305a5 100644
>> --- a/include/linux/of_fdt.h
>> +++ b/include/linux/of_fdt.h
>> @@ -83,6 +83,7 @@ extern const void *of_flat_dt_match_machine(const void *default_match,
>>  /* Other Prototypes */
>>  extern void unflatten_device_tree(void);
>>  extern void unflatten_and_copy_device_tree(void);
>> +extern void preserve_fdt(void);
>>  extern void early_init_devtree(void *);
>>  extern void early_get_first_memblock_info(void *, phys_addr_t *);
>>  extern u64 fdt_translate_address(const void *blob, int node_offset);
>> @@ -92,6 +93,7 @@ static inline void early_init_fdt_scan_reserved_mem(void) {}
>>  static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
>>  static inline void unflatten_device_tree(void) {}
>>  static inline void unflatten_and_copy_device_tree(void) {}
>> +static inline void preserve_fdt(void) {}
>>  #endif /* CONFIG_OF_FLATTREE */
>>
>>  #endif /* __ASSEMBLY__ */
>> --
>> 1.8.3.2
>>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt

[Rob Herring]

On Tue, Nov 11, 2014 at 8:42 AM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Mon, 10 Nov 2014 19:47:01 +0100
> , Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>  wrote:
>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
>> that was passed to the kernel by the bootloader. This allows userland
>> applications such as kexec to access the raw binary. The blob needs to
>> be preserved as early as possible by calling preserve_fdt().
>>
>> The fact that this node does not reside under /sys/firmware/device-tree
>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
>> communicate just the UEFI and ACPI entry points, but the FDT is never
>> unflattened and used to configure the system.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> On further thought, nak. I want tree modifications to be treated as
> bugs. Do a checksum CRC32 or check. It's simpler and it can be extended
> to us an in-blob CRC when the file format gets upreved to include one.

Why? MIPS Octeon is full of them BTW. So the rule is any modifications
or fixups have to be on the unflattened tree?

For the CRC, we should add it into the entropy pool.

Rob

>
> g.
>
> ---
>
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index be16ce2ffd69..adda0a16934f 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -23,6 +23,7 @@ config OF_FLATTREE
>         bool
>         select DTC
>         select LIBFDT
> +       select CRC
>
>  config OF_EARLY_FLATTREE
>         bool
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 83a8e1154602..80db46a2eab9 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -10,6 +10,7 @@
>   */
>
>  #include <linux/kernel.h>
> +#include <linux/crc32.h>
>  #include <linux/initrd.h>
>  #include <linux/memblock.h>
>  #include <linux/of.h>
> @@ -420,6 +421,7 @@ int __initdata dt_root_addr_cells;
>  int __initdata dt_root_size_cells;
>
>  void *initial_boot_params;
> +u32 initial_boot_params_crc;
>
>  #ifdef CONFIG_OF_EARLY_FLATTREE
>
> @@ -1003,6 +1005,9 @@ bool __init early_init_dt_verify(void *params)
>
>         /* Setup flat device-tree pointer */
>         initial_boot_params = params;
> +       initial_boot_params_crc = crc32_be(0, params, fdt_totalsize(params));
> +       pr_info("FDT CRC: %x\n", initial_boot_params_crc);
> +
>         return true;
>  }
>
>
>
>
>> ---
>>  drivers/of/fdt.c       | 34 ++++++++++++++++++++++++++++++++++
>>  include/linux/of_fdt.h |  2 ++
>>  2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index d1ffca8b34ea..e9ee3d5f7ea4 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -22,6 +22,7 @@
>>  #include <linux/libfdt.h>
>>  #include <linux/debugfs.h>
>>  #include <linux/serial_core.h>
>> +#include <linux/sysfs.h>
>>
>>  #include <asm/setup.h>  /* for COMMAND_LINE_SIZE */
>>  #include <asm/page.h>
>> @@ -1103,4 +1104,37 @@ static int __init of_flat_dt_debugfs_export_fdt(void)
>>  module_init(of_flat_dt_debugfs_export_fdt);
>>  #endif
>>
>> +static u8 *raw_fdt_copy;
>> +
>> +void __init preserve_fdt(void)
>> +{
>> +     u32 fdt_size;
>> +
>> +     fdt_size = fdt_totalsize(initial_boot_params);
>> +     raw_fdt_copy = memcpy(__va(memblock_alloc(fdt_size, PAGE_SIZE)),
>> +                           initial_boot_params, fdt_size);
>> +}
>> +
>> +#ifdef CONFIG_SYSFS
>> +static ssize_t of_fdt_raw_read(struct file *filp, struct kobject *kobj,
>> +                            struct bin_attribute *bin_attr,
>> +                            char *buf, loff_t off, size_t count)
>> +{
>> +     memcpy(buf, raw_fdt_copy + off, count);
>> +     return count;
>> +}
>> +
>> +static int __init of_fdt_raw_init(void)
>> +{
>> +     static struct bin_attribute of_fdt_raw_attr =
>> +             __BIN_ATTR(fdt, S_IRUSR, of_fdt_raw_read, NULL, 0);
>> +
>> +     if (!raw_fdt_copy)
>> +             return 0;
>> +     of_fdt_raw_attr.size = fdt_totalsize(raw_fdt_copy);
>> +     return sysfs_create_bin_file(firmware_kobj, &of_fdt_raw_attr);
>> +}
>> +module_init(of_fdt_raw_init);
>> +#endif
>> +
>>  #endif /* CONFIG_OF_EARLY_FLATTREE */
>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>> index 0ff360d5b3b3..7672a26305a5 100644
>> --- a/include/linux/of_fdt.h
>> +++ b/include/linux/of_fdt.h
>> @@ -83,6 +83,7 @@ extern const void *of_flat_dt_match_machine(const void *default_match,
>>  /* Other Prototypes */
>>  extern void unflatten_device_tree(void);
>>  extern void unflatten_and_copy_device_tree(void);
>> +extern void preserve_fdt(void);
>>  extern void early_init_devtree(void *);
>>  extern void early_get_first_memblock_info(void *, phys_addr_t *);
>>  extern u64 fdt_translate_address(const void *blob, int node_offset);
>> @@ -92,6 +93,7 @@ static inline void early_init_fdt_scan_reserved_mem(void) {}
>>  static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
>>  static inline void unflatten_device_tree(void) {}
>>  static inline void unflatten_and_copy_device_tree(void) {}
>> +static inline void preserve_fdt(void) {}
>>  #endif /* CONFIG_OF_FLATTREE */
>>
>>  #endif /* __ASSEMBLY__ */
>> --
>> 1.8.3.2
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt

[Grant Likely]

On Tue, Nov 11, 2014 at 4:25 PM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Nov 11, 2014 at 8:42 AM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On Mon, 10 Nov 2014 19:47:01 +0100
>> , Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>  wrote:
>>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
>>> that was passed to the kernel by the bootloader. This allows userland
>>> applications such as kexec to access the raw binary. The blob needs to
>>> be preserved as early as possible by calling preserve_fdt().
>>>
>>> The fact that this node does not reside under /sys/firmware/device-tree
>>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
>>> communicate just the UEFI and ACPI entry points, but the FDT is never
>>> unflattened and used to configure the system.
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>
>> On further thought, nak. I want tree modifications to be treated as
>> bugs. Do a checksum CRC32 or check. It's simpler and it can be extended
>> to us an in-blob CRC when the file format gets upreved to include one.
>
> Why? MIPS Octeon is full of them BTW. So the rule is any modifications
> or fixups have to be on the unflattened tree?

That, or we require the fixups to explicitly clean up the CRC after
themselves. That will force them to be visible. Anything using the
fdt_*() write functions could potentially take care of it
automatically. That would make it immediately discoverable where
changes are being made in the tree.

g.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt

[Ard Biesheuvel]

On 11 November 2014 21:08, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Nov 11, 2014 at 4:25 PM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Tue, Nov 11, 2014 at 8:42 AM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>> On Mon, 10 Nov 2014 19:47:01 +0100
>>> , Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>  wrote:
>>>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
>>>> that was passed to the kernel by the bootloader. This allows userland
>>>> applications such as kexec to access the raw binary. The blob needs to
>>>> be preserved as early as possible by calling preserve_fdt().
>>>>
>>>> The fact that this node does not reside under /sys/firmware/device-tree
>>>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
>>>> communicate just the UEFI and ACPI entry points, but the FDT is never
>>>> unflattened and used to configure the system.
>>>>
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>
>>> On further thought, nak. I want tree modifications to be treated as
>>> bugs. Do a checksum CRC32 or check. It's simpler and it can be extended
>>> to us an in-blob CRC when the file format gets upreved to include one.
>>
>> Why? MIPS Octeon is full of them BTW. So the rule is any modifications
>> or fixups have to be on the unflattened tree?
>
> That, or we require the fixups to explicitly clean up the CRC after
> themselves. That will force them to be visible. Anything using the
> fdt_*() write functions could potentially take care of it
> automatically. That would make it immediately discoverable where
> changes are being made in the tree.
>

Doesn't that defeat the purpose of capturing the FDT as it was passed
by the bootloader? Those fixups will be reapplied by the kexec'ed
kernel anyway.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt

[Suzuki K. Poulose]

On 10/11/14 18:47, Ard Biesheuvel wrote:
> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
> that was passed to the kernel by the bootloader. This allows userland
> applications such as kexec to access the raw binary. The blob needs to
> be preserved as early as possible by calling preserve_fdt().
>
> The fact that this node does not reside under /sys/firmware/device-tree
> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
> communicate just the UEFI and ACPI entry points, but the FDT is never
> unflattened and used to configure the system.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>   drivers/of/fdt.c       | 34 ++++++++++++++++++++++++++++++++++
>   include/linux/of_fdt.h |  2 ++
>   2 files changed, 36 insertions(+)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index d1ffca8b34ea..e9ee3d5f7ea4 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -22,6 +22,7 @@
>   #include <linux/libfdt.h>
>   #include <linux/debugfs.h>
>   #include <linux/serial_core.h>
> +#include <linux/sysfs.h>
>
>   #include <asm/setup.h>  /* for COMMAND_LINE_SIZE */
>   #include <asm/page.h>
> @@ -1103,4 +1104,37 @@ static int __init of_flat_dt_debugfs_export_fdt(void)
>   module_init(of_flat_dt_debugfs_export_fdt);
>   #endif
>
> +static u8 *raw_fdt_copy;
> +
> +void __init preserve_fdt(void)
> +{
> +	u32 fdt_size;
> +
> +	fdt_size = fdt_totalsize(initial_boot_params);
> +	raw_fdt_copy = memcpy(__va(memblock_alloc(fdt_size, PAGE_SIZE)),
> +			      initial_boot_params, fdt_size);
> +}
> +
> +#ifdef CONFIG_SYSFS
> +static ssize_t of_fdt_raw_read(struct file *filp, struct kobject *kobj,
> +			       struct bin_attribute *bin_attr,
> +			       char *buf, loff_t off, size_t count)
> +{
> +	memcpy(buf, raw_fdt_copy + off, count);
Should we check for the off+count, not to exceed the fdt_size that we 
actually copied ?

Thanks
Suzuki


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt

[Grant Likely]

On Tue, Nov 11, 2014 at 8:34 PM, Ard Biesheuvel
<ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 11 November 2014 21:08, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On Tue, Nov 11, 2014 at 4:25 PM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On Tue, Nov 11, 2014 at 8:42 AM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>> On Mon, 10 Nov 2014 19:47:01 +0100
>>>> , Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>  wrote:
>>>>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
>>>>> that was passed to the kernel by the bootloader. This allows userland
>>>>> applications such as kexec to access the raw binary. The blob needs to
>>>>> be preserved as early as possible by calling preserve_fdt().
>>>>>
>>>>> The fact that this node does not reside under /sys/firmware/device-tree
>>>>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
>>>>> communicate just the UEFI and ACPI entry points, but the FDT is never
>>>>> unflattened and used to configure the system.
>>>>>
>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>
>>>> On further thought, nak. I want tree modifications to be treated as
>>>> bugs. Do a checksum CRC32 or check. It's simpler and it can be extended
>>>> to us an in-blob CRC when the file format gets upreved to include one.
>>>
>>> Why? MIPS Octeon is full of them BTW. So the rule is any modifications
>>> or fixups have to be on the unflattened tree?
>>
>> That, or we require the fixups to explicitly clean up the CRC after
>> themselves. That will force them to be visible. Anything using the
>> fdt_*() write functions could potentially take care of it
>> automatically. That would make it immediately discoverable where
>> changes are being made in the tree.
>>
>
> Doesn't that defeat the purpose of capturing the FDT as it was passed
> by the bootloader? Those fixups will be reapplied by the kexec'ed
> kernel anyway.

We're always going to be in a grey area here. There are some things
that should not be passed over. For example, a framebuffer that is
passed in from firmware will no longer be valid on kexec. Right now on
arch/arm, the exynos platform fixes up the DT because the memory node
is outright /corrupt/. In the MIPS case, they do a bunch of stuff in
the kernel to figure out the hardware as configured by firmware. I
don't agree with that approach, but I would be very surprised if the
full original DT has any validity after the early boot fixups.

Early boot fixups to the .dtb are always going to be oddball cases.

g.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt

[Ard Biesheuvel]

On 12 November 2014 12:45, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Nov 11, 2014 at 8:34 PM, Ard Biesheuvel
> <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On 11 November 2014 21:08, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>> On Tue, Nov 11, 2014 at 4:25 PM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> On Tue, Nov 11, 2014 at 8:42 AM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>>> On Mon, 10 Nov 2014 19:47:01 +0100
>>>>> , Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>>  wrote:
>>>>>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
>>>>>> that was passed to the kernel by the bootloader. This allows userland
>>>>>> applications such as kexec to access the raw binary. The blob needs to
>>>>>> be preserved as early as possible by calling preserve_fdt().
>>>>>>
>>>>>> The fact that this node does not reside under /sys/firmware/device-tree
>>>>>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
>>>>>> communicate just the UEFI and ACPI entry points, but the FDT is never
>>>>>> unflattened and used to configure the system.
>>>>>>
>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>>
>>>>> On further thought, nak. I want tree modifications to be treated as
>>>>> bugs. Do a checksum CRC32 or check. It's simpler and it can be extended
>>>>> to us an in-blob CRC when the file format gets upreved to include one.
>>>>
>>>> Why? MIPS Octeon is full of them BTW. So the rule is any modifications
>>>> or fixups have to be on the unflattened tree?
>>>
>>> That, or we require the fixups to explicitly clean up the CRC after
>>> themselves. That will force them to be visible. Anything using the
>>> fdt_*() write functions could potentially take care of it
>>> automatically. That would make it immediately discoverable where
>>> changes are being made in the tree.
>>>
>>
>> Doesn't that defeat the purpose of capturing the FDT as it was passed
>> by the bootloader? Those fixups will be reapplied by the kexec'ed
>> kernel anyway.
>
> We're always going to be in a grey area here. There are some things
> that should not be passed over. For example, a framebuffer that is
> passed in from firmware will no longer be valid on kexec. Right now on
> arch/arm, the exynos platform fixes up the DT because the memory node
> is outright /corrupt/. In the MIPS case, they do a bunch of stuff in
> the kernel to figure out the hardware as configured by firmware. I
> don't agree with that approach, but I would be very surprised if the
> full original DT has any validity after the early boot fixups.
>

Well, yes, but those fixups will presumably be done on every boot,
including the kexec ones.

> Early boot fixups to the .dtb are always going to be oddball cases.
>

OK, so generally speaking, whether the original DTB is more suitable
to be presented through /sys/firmware/fdt than the 'current' FDT
(i.e., whatever is in memory at the time the sysfs node is accessed)
is not obvious at all. So why not just do the latter?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt

[Grant Likely]

On Wed, Nov 12, 2014 at 11:51 AM, Ard Biesheuvel
<ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 12 November 2014 12:45, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On Tue, Nov 11, 2014 at 8:34 PM, Ard Biesheuvel
>> <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>> On 11 November 2014 21:08, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>> On Tue, Nov 11, 2014 at 4:25 PM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>> On Tue, Nov 11, 2014 at 8:42 AM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>>>> On Mon, 10 Nov 2014 19:47:01 +0100
>>>>>> , Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>>>  wrote:
>>>>>>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
>>>>>>> that was passed to the kernel by the bootloader. This allows userland
>>>>>>> applications such as kexec to access the raw binary. The blob needs to
>>>>>>> be preserved as early as possible by calling preserve_fdt().
>>>>>>>
>>>>>>> The fact that this node does not reside under /sys/firmware/device-tree
>>>>>>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
>>>>>>> communicate just the UEFI and ACPI entry points, but the FDT is never
>>>>>>> unflattened and used to configure the system.
>>>>>>>
>>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>>>
>>>>>> On further thought, nak. I want tree modifications to be treated as
>>>>>> bugs. Do a checksum CRC32 or check. It's simpler and it can be extended
>>>>>> to us an in-blob CRC when the file format gets upreved to include one.
>>>>>
>>>>> Why? MIPS Octeon is full of them BTW. So the rule is any modifications
>>>>> or fixups have to be on the unflattened tree?
>>>>
>>>> That, or we require the fixups to explicitly clean up the CRC after
>>>> themselves. That will force them to be visible. Anything using the
>>>> fdt_*() write functions could potentially take care of it
>>>> automatically. That would make it immediately discoverable where
>>>> changes are being made in the tree.
>>>>
>>>
>>> Doesn't that defeat the purpose of capturing the FDT as it was passed
>>> by the bootloader? Those fixups will be reapplied by the kexec'ed
>>> kernel anyway.
>>
>> We're always going to be in a grey area here. There are some things
>> that should not be passed over. For example, a framebuffer that is
>> passed in from firmware will no longer be valid on kexec. Right now on
>> arch/arm, the exynos platform fixes up the DT because the memory node
>> is outright /corrupt/. In the MIPS case, they do a bunch of stuff in
>> the kernel to figure out the hardware as configured by firmware. I
>> don't agree with that approach, but I would be very surprised if the
>> full original DT has any validity after the early boot fixups.
>>
>
> Well, yes, but those fixups will presumably be done on every boot,
> including the kexec ones.
>
>> Early boot fixups to the .dtb are always going to be oddball cases.
>>
>
> OK, so generally speaking, whether the original DTB is more suitable
> to be presented through /sys/firmware/fdt than the 'current' FDT
> (i.e., whatever is in memory at the time the sysfs node is accessed)
> is not obvious at all. So why not just do the latter?

We're now talking theoreticals. In the case you're working on
(ACPI+FDT), the FDT is generated by Grub or the EFI stub as an
intermediary format, and there isn't currently any code in your boot
path that modifies the FDT. You're arguing for a future situation
where the kernel makes a fixup that is not wanted in the second
kernel. I'm arguing that 1) I want to /know/ when that happens because
there are very few situations where it is a valid thing to do, and 2)
the likelyhood of that happening in the FDT+ACPI case is somewhere
between slim and none because you're working with an intermediary FDT,
not a real hardware description.

/If/ it ever becomes a problem we can revisit, until then I still
prefer the checksum method.

g.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt

[Ard Biesheuvel]

On 12 November 2014 13:01, Grant Likely <grant.likely@linaro.org> wrote:
> On Wed, Nov 12, 2014 at 11:51 AM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> On 12 November 2014 12:45, Grant Likely <grant.likely@linaro.org> wrote:
>>> On Tue, Nov 11, 2014 at 8:34 PM, Ard Biesheuvel
>>> <ard.biesheuvel@linaro.org> wrote:
>>>> On 11 November 2014 21:08, Grant Likely <grant.likely@linaro.org> wrote:
>>>>> On Tue, Nov 11, 2014 at 4:25 PM, Rob Herring <robherring2@gmail.com> wrote:
>>>>>> On Tue, Nov 11, 2014 at 8:42 AM, Grant Likely <grant.likely@linaro.org> wrote:
>>>>>>> On Mon, 10 Nov 2014 19:47:01 +0100
>>>>>>> , Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>>>>  wrote:
>>>>>>>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
>>>>>>>> that was passed to the kernel by the bootloader. This allows userland
>>>>>>>> applications such as kexec to access the raw binary. The blob needs to
>>>>>>>> be preserved as early as possible by calling preserve_fdt().
>>>>>>>>
>>>>>>>> The fact that this node does not reside under /sys/firmware/device-tree
>>>>>>>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
>>>>>>>> communicate just the UEFI and ACPI entry points, but the FDT is never
>>>>>>>> unflattened and used to configure the system.
>>>>>>>>
>>>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>>>>
>>>>>>> On further thought, nak. I want tree modifications to be treated as
>>>>>>> bugs. Do a checksum CRC32 or check. It's simpler and it can be extended
>>>>>>> to us an in-blob CRC when the file format gets upreved to include one.
>>>>>>
>>>>>> Why? MIPS Octeon is full of them BTW. So the rule is any modifications
>>>>>> or fixups have to be on the unflattened tree?
>>>>>
>>>>> That, or we require the fixups to explicitly clean up the CRC after
>>>>> themselves. That will force them to be visible. Anything using the
>>>>> fdt_*() write functions could potentially take care of it
>>>>> automatically. That would make it immediately discoverable where
>>>>> changes are being made in the tree.
>>>>>
>>>>
>>>> Doesn't that defeat the purpose of capturing the FDT as it was passed
>>>> by the bootloader? Those fixups will be reapplied by the kexec'ed
>>>> kernel anyway.
>>>
>>> We're always going to be in a grey area here. There are some things
>>> that should not be passed over. For example, a framebuffer that is
>>> passed in from firmware will no longer be valid on kexec. Right now on
>>> arch/arm, the exynos platform fixes up the DT because the memory node
>>> is outright /corrupt/. In the MIPS case, they do a bunch of stuff in
>>> the kernel to figure out the hardware as configured by firmware. I
>>> don't agree with that approach, but I would be very surprised if the
>>> full original DT has any validity after the early boot fixups.
>>>
>>
>> Well, yes, but those fixups will presumably be done on every boot,
>> including the kexec ones.
>>
>>> Early boot fixups to the .dtb are always going to be oddball cases.
>>>
>>
>> OK, so generally speaking, whether the original DTB is more suitable
>> to be presented through /sys/firmware/fdt than the 'current' FDT
>> (i.e., whatever is in memory at the time the sysfs node is accessed)
>> is not obvious at all. So why not just do the latter?
>
> We're now talking theoreticals. In the case you're working on
> (ACPI+FDT), the FDT is generated by Grub or the EFI stub as an
> intermediary format, and there isn't currently any code in your boot
> path that modifies the FDT. You're arguing for a future situation
> where the kernel makes a fixup that is not wanted in the second
> kernel. I'm arguing that 1) I want to /know/ when that happens because
> there are very few situations where it is a valid thing to do, and 2)
> the likelyhood of that happening in the FDT+ACPI case is somewhere
> between slim and none because you're working with an intermediary FDT,
> not a real hardware description.
>

But do you still want the warning? Or only if you access the sysfs
node? Or just a pr_warn() perhaps rather than a full blown WARN() ?
MIPS will start seeing the warning as well even if they don't care
about what is in /sys/firmware/fdt

-- 
Ard.

Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt

[Grant Likely]

On Wed, Nov 12, 2014 at 12:08 PM, Ard Biesheuvel
<ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 12 November 2014 13:01, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On Wed, Nov 12, 2014 at 11:51 AM, Ard Biesheuvel
>> <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>> On 12 November 2014 12:45, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>> On Tue, Nov 11, 2014 at 8:34 PM, Ard Biesheuvel
>>>> <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>>> On 11 November 2014 21:08, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>>>> On Tue, Nov 11, 2014 at 4:25 PM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>>> On Tue, Nov 11, 2014 at 8:42 AM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>>>>>> On Mon, 10 Nov 2014 19:47:01 +0100
>>>>>>>> , Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>>>>>  wrote:
>>>>>>>>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
>>>>>>>>> that was passed to the kernel by the bootloader. This allows userland
>>>>>>>>> applications such as kexec to access the raw binary. The blob needs to
>>>>>>>>> be preserved as early as possible by calling preserve_fdt().
>>>>>>>>>
>>>>>>>>> The fact that this node does not reside under /sys/firmware/device-tree
>>>>>>>>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
>>>>>>>>> communicate just the UEFI and ACPI entry points, but the FDT is never
>>>>>>>>> unflattened and used to configure the system.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>>>>>
>>>>>>>> On further thought, nak. I want tree modifications to be treated as
>>>>>>>> bugs. Do a checksum CRC32 or check. It's simpler and it can be extended
>>>>>>>> to us an in-blob CRC when the file format gets upreved to include one.
>>>>>>>
>>>>>>> Why? MIPS Octeon is full of them BTW. So the rule is any modifications
>>>>>>> or fixups have to be on the unflattened tree?
>>>>>>
>>>>>> That, or we require the fixups to explicitly clean up the CRC after
>>>>>> themselves. That will force them to be visible. Anything using the
>>>>>> fdt_*() write functions could potentially take care of it
>>>>>> automatically. That would make it immediately discoverable where
>>>>>> changes are being made in the tree.
>>>>>>
>>>>>
>>>>> Doesn't that defeat the purpose of capturing the FDT as it was passed
>>>>> by the bootloader? Those fixups will be reapplied by the kexec'ed
>>>>> kernel anyway.
>>>>
>>>> We're always going to be in a grey area here. There are some things
>>>> that should not be passed over. For example, a framebuffer that is
>>>> passed in from firmware will no longer be valid on kexec. Right now on
>>>> arch/arm, the exynos platform fixes up the DT because the memory node
>>>> is outright /corrupt/. In the MIPS case, they do a bunch of stuff in
>>>> the kernel to figure out the hardware as configured by firmware. I
>>>> don't agree with that approach, but I would be very surprised if the
>>>> full original DT has any validity after the early boot fixups.
>>>>
>>>
>>> Well, yes, but those fixups will presumably be done on every boot,
>>> including the kexec ones.
>>>
>>>> Early boot fixups to the .dtb are always going to be oddball cases.
>>>>
>>>
>>> OK, so generally speaking, whether the original DTB is more suitable
>>> to be presented through /sys/firmware/fdt than the 'current' FDT
>>> (i.e., whatever is in memory at the time the sysfs node is accessed)
>>> is not obvious at all. So why not just do the latter?
>>
>> We're now talking theoreticals. In the case you're working on
>> (ACPI+FDT), the FDT is generated by Grub or the EFI stub as an
>> intermediary format, and there isn't currently any code in your boot
>> path that modifies the FDT. You're arguing for a future situation
>> where the kernel makes a fixup that is not wanted in the second
>> kernel. I'm arguing that 1) I want to /know/ when that happens because
>> there are very few situations where it is a valid thing to do, and 2)
>> the likelyhood of that happening in the FDT+ACPI case is somewhere
>> between slim and none because you're working with an intermediary FDT,
>> not a real hardware description.
>>
>
> But do you still want the warning? Or only if you access the sysfs
> node? Or just a pr_warn() perhaps rather than a full blown WARN() ?
> MIPS will start seeing the warning as well even if they don't care
> about what is in /sys/firmware/fdt

pr_warn() and then don't export the file in /sys/firmware/fdt. I'm
okay with the check being done once at the end of initcalls.

g.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html