On Mon, Feb 26, 2018 at 11:32:48PM +0200, Andy Shevchenko wrote:
> There is as far as we know no type field in the struct dev_header
> provided. Thus, assume that values of all properties are type of u32.

There is no type field, but the value can be something else than u32.

See here for sample properties:
https://gist.github.com/l1k/d58ff1e24976118d24f2bf550253a507

E.g. the "ThunderboltDROM" property is a raw 256 byte array which is
consumed by thunderbolt.ko.  It's up to the drivers to make sense of
the properties and assume a certain type.


> Deterministic type of property values is required to avoid union
> aliasing in the code.
> 
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> 
> - the union aliasing fix for built-in device properties will be sent during
> next cycle, though I can share through one of my temporary branches if needed

Sharing that might be helpful in understanding what you're trying to fix,
right now I'm clueless, sorry. :)

Thanks,

Lukas

> 
>  drivers/firmware/efi/apple-properties.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/firmware/efi/apple-properties.c 
> b/drivers/firmware/efi/apple-properties.c
> index b9602e0d7b50..a4847bcd26be 100644
> --- a/drivers/firmware/efi/apple-properties.c
> +++ b/drivers/firmware/efi/apple-properties.c
> @@ -40,8 +40,9 @@ struct dev_header {
>       u32 prop_count;
>       struct efi_dev_path path[0];
>       /*
> -      * followed by key/value pairs, each key and value preceded by u32 len,
> -      * len includes itself, value may be empty (in which case its len is 4)
> +      * Followed by key/value pairs, each key and value preceded by u32 len,
> +      * len includes itself, value may be empty (in which case its len is 4).
> +      * REVISIT: Non-empty values have one or more u32 items.
>        */
>  };
>  
> @@ -92,16 +93,18 @@ static void __init unmarshal_key_value_pairs(struct 
> dev_header *dev_header,
>               ucs2_as_utf8(key, ptr + sizeof(key_len),
>                            key_len - sizeof(key_len));
>  
> +             /* For now only 32-bit values are supported */
>               entry[i].name = key;
>               entry[i].length = val_len - sizeof(val_len);
>               entry[i].is_array = !!entry[i].length;
> -             entry[i].pointer.raw_data = ptr + key_len + sizeof(val_len);
> +             entry[i].pointer.u32_data = ptr + key_len + sizeof(val_len);
>  
>               if (dump_properties) {
>                       dev_info(dev, "property: %s\n", entry[i].name);
>                       print_hex_dump(KERN_INFO, pr_fmt(), DUMP_PREFIX_OFFSET,
> -                             16, 1, entry[i].pointer.raw_data,
> -                             entry[i].length, true);
> +                                    16, 4,
> +                                    entry[i].pointer.u32_data, 
> entry[i].length,
> +                                    true);
>               }
>  
>               ptr += key_len + val_len;
> -- 
> 2.16.1
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to