Re: [PATCH] efi/libstub/arm*: Set default address and size cells values for an empty dtb

Ard Biesheuvel
2017-02-07T18:12:32Z

Hi Jeffrey,

(adding Mark)

On 7 February 2017 at 17:59, Jeffrey Hugo <jhugo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> From: Sameer Goel <sgoel-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>
> In cases where a device tree is not provided (ie ACPI based system), an
> empty fdt is generated by efistub. Sets the address and size cell values
> in a generated fdt to support 64 bit addressing.
>
> This enables kexec/kdump on Qualcomm Technologies QDF24XX platforms as those
> utilities will read the address/size values from the fdt, and such values
> may exceed the range provided by the 32 bit default.
>

As far as I know, those properties are explicitly associated with the
'reg' properties of subordinate nodes. So which nodes are we talking
about here? Are we producing an incorrect DT by not setting these? Or
is this simply a convenience to work around bugs in the tooling?

> Change-Id: Ie7f3637e375bd6631c6bda1f7b3c9003765ff4a5

Please drop these IDs when you post

> Signed-off-by: Sameer Goel <sgoel-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Jeffrey Hugo <jhugo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> drivers/firmware/efi/libstub/fdt.c | 38 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> index 921dfa0..def5c9c 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -16,6 +16,34 @@
>
> #include "efistub.h"
>
> +#define EFI_DT_ADDR_CELLS_DEFAULT 2
> +#define EFI_DT_SIZE_CELLS_DEFAULT 2
> +
> +static void fdt_update_cell_size(efi_system_table_t *sys_table, void *fdt)
> +{
> + int offset;
> + int status;
> +
> + offset = fdt_path_offset(fdt, "/");
> + /* Set the #address-cells and #size-cells values for an empty tree */
> +
> + status = fdt_setprop_u32(fdt, offset, "#address-cells",
> + EFI_DT_ADDR_CELLS_DEFAULT);
> + if (status) {
> + pr_efi(sys_table,
> + "Failed to set #address-cells for empty dtb\n");
> + return;
> + }
> +
> + status = fdt_setprop_u32(fdt, offset, "#size-cells",
> + EFI_DT_SIZE_CELLS_DEFAULT);
> + if (status) {
> + pr_efi(sys_table,
> + "Failed to set #size-cells for empty dtb\n");
> + return;
> + }
> +}
> +
> static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
> unsigned long orig_fdt_size,
> void *fdt, int new_fdt_size, char *cmdline_ptr,
> @@ -44,8 +72,16 @@ static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
>
> if (orig_fdt)
> status = fdt_open_into(orig_fdt, fdt, new_fdt_size);
> - else
> + else {
> status = fdt_create_empty_tree(fdt, new_fdt_size);
> + if (status == 0) {
> + /*
> + * Any failure from the following function is non
> + * critical
> + */
> + fdt_update_cell_size(sys_table, fdt);
> + }
> + }
>
> if (status != 0)
> goto fdt_set_fail;
> --
> Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
> Qualcomm Technologies, Inc. is a member of the
> Code Aurora Forum, a Linux Foundation Collaborative Project.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html