On 07/02/17 21:19, Jeffrey Hugo wrote:
> From: Sameer Goel <sgoel@xxxxxxxxxxxxxx>
>
> In cases where a device tree is not provided (ie ACPI based system), an
> empty fdt is generated by efistub. #address-cells and #size-cells are not
> set in the empty fdt, so they default to 1 (4 byte wide). This can be an
> issue on 64-bit systems where values representing addresses, etc may be
> 8 bytes wide as the default value does not align with the general
> requirements for an empty DTB, and is fragile when passed to other agents
> as extra care is required to read the entire width of a value.
>
> This issue is observed on Qualcomm Technologies QDF24XX platforms when
> kexec-tools inserts 64-bit addresses into the "linux,elfcorehdr" and
> "linux,usable-memory-range" properties of the fdt. When the values are
> later consumed, they are truncated to 32-bit.
>
> Setting #address-cells and #size-cells to 2 at creation of the empty fdt
> resolves the observed issue, and makes the fdt less fragile.
Hang on, if this code is shared with 32-bit ARM, doesn't this just move
the problem around? If arm64 kexec is blindly punting reg properties
into a DT assuming *-cells == 2, then wouldn't ARM kexec likely be doing
the same relying on *-cells == 1?
Robin.
> Signed-off-by: Sameer Goel <sgoel@xxxxxxxxxxxxxx>
> Signed-off-by: Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx>
> ---
>
> [v2]
> -Add braces to an if when the corresponding else has braces
> -Remove print statements
> -Reword commit text
> -Removed gerrit tag
>
> drivers/firmware/efi/libstub/fdt.c | 28 ++++++++++++++++++++++++++--
> 1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> index 921dfa0..22ea73b 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -16,6 +16,22 @@
>
> #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;
> +
> + offset = fdt_path_offset(fdt, "/");
> + /* Set the #address-cells and #size-cells values for an empty tree */
> +
> + fdt_setprop_u32(fdt, offset, "#address-cells",
> + EFI_DT_ADDR_CELLS_DEFAULT);
> +
> + fdt_setprop_u32(fdt, offset, "#size-cells", EFI_DT_SIZE_CELLS_DEFAULT);
> +}
> +
> 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,
> @@ -42,10 +58,18 @@ static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
> }
> }
>
> - if (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;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
