Apple device properties

arch/x86/boot/compressed/eboot.c +9 -0

index cc69e37..ff01637 100644
@@ -537,6 +537,63 @@ static void setup_efi_pci(struct boot_params *params)
 	efi_call_early(free_pool, pci_handle);
 }
 
+static void retrieve_apple_device_properties(struct boot_params *params)
+{
+	efi_guid_t guid = APPLE_PROPERTIES_PROTOCOL_GUID;
+	struct setup_data *data, *new;
+	efi_status_t status;
+	void *properties;
+	u32 size = 0;
+
+	status = efi_call_early(locate_protocol, &guid, NULL, &properties);

arch/x86/boot/compressed/eboot.c +5 -0

+	efi_early->call(efi_is_64bit() ?
+			((apple_properties_protocol_64 *)properties)->get_all :
+			((apple_properties_protocol_32 *)properties)->get_all,
+			properties, NULL, &size);
+	if (!size)

arch/x86/boot/compressed/eboot.c +19 -0

+		if (status == EFI_BUFFER_TOO_SMALL)
+			efi_call_early(free_pool, new);
+	} while (status == EFI_BUFFER_TOO_SMALL);
+
+	new->type = SETUP_APPLE_PROPERTIES;
+	new->len  = size;
+	new->next = 0;
+
+	data = (struct setup_data *)(unsigned long)params->hdr.setup_data;
+	if (!data)
+		params->hdr.setup_data = (unsigned long)new;
+	else {
+		while (data->next)
+			data = (struct setup_data *)(unsigned long)data->next;
+		data->next = (unsigned long)new;
+	}
+}
+
 static efi_status_t
 setup_uga32(void **uga_handle, unsigned long size, u32 *width, u32 *height)
 {

@@ -1068,6 +1125,7 @@ static efi_status_t exit_boot(struct boot_params *boot_params,
 struct boot_params *efi_main(struct efi_config *c,
 			     struct boot_params *boot_params)
 {
+	efi_char16_t const apple[] = { 'A', 'p', 'p', 'l', 'e', 0 };
 	struct desc_ptr *gdt = NULL;
 	efi_loaded_image_t *image;
 	struct setup_header *hdr = &boot_params->hdr;

@@ -1098,6 +1156,11 @@ struct boot_params *efi_main(struct efi_config *c,
 

arch/x86/include/uapi/asm/bootparam.h +1 -0

index c18ce67..b10bf31 100644
@@ -7,6 +7,7 @@
 #define SETUP_DTB			2
 #define SETUP_PCI			3
 #define SETUP_EFI			4
+#define SETUP_APPLE_PROPERTIES		5
 
 /* ram_size flags */
 #define RAMDISK_IMAGE_START_MASK	0x07FF

drivers/firmware/efi/Kconfig +13 -0

index 893fda4..2e78b0b 100644
@@ -129,6 +129,19 @@ config EFI_TEST
 	  Say Y here to enable the runtime services support via /dev/efi_test.
 	  If unsure, say N.
 
+config APPLE_PROPERTIES
+	bool "Apple Device Properties"
+	depends on EFI_STUB && X86
+	select EFI_DEV_PATH_PARSER
+	select UCS2_STRING
+	help
+	  Retrieve properties from EFI on Apple Macs and assign them to
+	  devices, allowing for improved support of Apple hardware.
+	  Properties that would otherwise be missing include the
+	  Thunderbolt Device ROM and GPU configuration data.
+
+	  If unsure, say Y if you have a Mac.  Otherwise N.
+
 endmenu
 
 config UEFI_CPER

drivers/firmware/efi/Makefile +1 -0

index 3e91ae3..ad67342 100644
@@ -22,6 +22,7 @@ obj-$(CONFIG_EFI_FAKE_MEMMAP)		+= fake_mem.o
 obj-$(CONFIG_EFI_BOOTLOADER_CONTROL)	+= efibc.o
 obj-$(CONFIG_EFI_TEST)			+= test/
 obj-$(CONFIG_EFI_DEV_PATH_PARSER)	+= dev-path-parser.o
+obj-$(CONFIG_APPLE_PROPERTIES)		+= apple-properties.o
 
 arm-obj-$(CONFIG_EFI)			:= arm-init.o arm-runtime.o
 obj-$(CONFIG_ARM)			+= $(arm-obj-y)

drivers/firmware/efi/apple-properties.c new file +74 -0

new file mode 100644
index 0000000..3b4d6a2
@@ -0,0 +1,239 @@
+/*
+ * apple-properties.c - EFI device properties on Macs
+ * Copyright (C) 2016 Lukas Wunner <lukas-JFq808J9C/[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License (version 2) as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define pr_fmt(fmt) "apple-properties: " fmt
+
+#include <linux/bootmem.h>
+#include <linux/dmi.h>
+#include <linux/efi.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+#include <linux/ucs2_string.h>
+#include <asm/setup.h>
+
+static bool dump_properties __initdata;
+
+static int __init dump_properties_enable(char *arg)
+{
+	dump_properties = true;
+	return 0;
+}
+
+__setup("dump_apple_properties", dump_properties_enable);
+
+struct dev_header {
+	u32 len;
+	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)
+	 */
+};
+
+struct properties_header {
+	u32 len;
+	u32 version;
+	u32 dev_count;
+	struct dev_header dev_header[0];
+};
+
+static u8 one __initdata = 1;
+
+static void __init unmarshal_key_value_pairs(struct dev_header *dev_header,
+					     struct device *dev, void *ptr,
+					     struct property_entry entry[])
+{
+	int i;
+
+	for (i = 0; i < dev_header->prop_count; i++) {
+		int remaining = dev_header->len - (ptr - (void *)dev_header);
+		u32 key_len, val_len;
+		char *key;
+
+		if (sizeof(key_len) > remaining)
+			break;
+		key_len = *(typeof(key_len) *)ptr;
+		if (key_len + sizeof(val_len) > remaining ||
+		    key_len < sizeof(key_len) + sizeof(efi_char16_t) ||
+		    *(efi_char16_t *)(ptr + sizeof(key_len)) == 0) {
+			dev_err(dev, "invalid property name len at %#zx\n",

drivers/firmware/efi/apple-properties.c new file +20 -0

+		ucs2_as_utf8(key, ptr + sizeof(key_len),
+			     key_len - sizeof(key_len));
+
+		entry[i].name = key;
+		entry[i].is_array = true;
+		entry[i].length = val_len - sizeof(val_len);
+		entry[i].pointer.raw_data = ptr + key_len + sizeof(val_len);
+		if (!entry[i].length) {
+			/* driver core doesn't accept empty properties */
+			entry[i].length = 1;
+			entry[i].pointer.raw_data = &one;
+		}
+
+		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);
+		}
+

drivers/firmware/efi/apple-properties.c new file +12 -0

+}
+
+static int __init unmarshal_devices(struct properties_header *properties)
+{
+	size_t offset = offsetof(struct properties_header, dev_header[0]);
+
+	while (offset + sizeof(struct dev_header) < properties->len) {
+		struct dev_header *dev_header = (void *)properties + offset;
+		struct property_entry *entry = NULL;
+		struct device *dev;
+		size_t len;
+		int ret, i;

drivers/firmware/efi/apple-properties.c new file +14 -0

+		dev = efi_get_device_by_path((struct efi_dev_path **)&ptr, &len);
+		if (IS_ERR(dev)) {
+			pr_err("device path parse error %ld at %#zx:\n",
+			       PTR_ERR(dev), ptr - (void *)dev_header);
+			print_hex_dump(KERN_ERR, pr_fmt(), DUMP_PREFIX_OFFSET,
+			       16, 1, dev_header, dev_header->len, true);
+			dev = NULL;
+			goto skip_device;
+		}
+
+		entry = kcalloc(dev_header->prop_count + 1, sizeof(*entry),
+				GFP_KERNEL);
+		if (!entry) {
+			dev_err(dev, "cannot allocate properties\n");

drivers/firmware/efi/apple-properties.c new file +30 -0

+		for (i = 0; entry[i].name; i++)
+			kfree(entry[i].name);
+
+skip_device:
+		kfree(entry);
+		put_device(dev);
+		offset += dev_header->len;
+	}
+
+	return 0;
+}
+
+static int __init map_properties(void)
+{
+	struct properties_header *properties;
+	struct setup_data *data;
+	u32 data_len;
+	u64 pa_data;
+	int ret;
+
+	if (!dmi_match(DMI_SYS_VENDOR, "Apple Inc.") &&
+	    !dmi_match(DMI_SYS_VENDOR, "Apple Computer, Inc."))
+		return 0;
+
+	pa_data = boot_params.hdr.setup_data;
+	while (pa_data) {
+		data = ioremap(pa_data, sizeof(*data));
+		if (!data) {
+			pr_err("cannot map setup_data header\n");
+			return -ENOMEM;

drivers/firmware/efi/apple-properties.c new file +14 -0

+		data = ioremap(pa_data, sizeof(*data) + data_len);
+		if (!data) {
+			pr_err("cannot map setup_data payload\n");
+			return -ENOMEM;
+		}
+
+		properties = (struct properties_header *)data->data;
+		if (properties->version != 1) {
+			pr_err("unsupported version:\n");
+			print_hex_dump(KERN_ERR, pr_fmt(), DUMP_PREFIX_OFFSET,
+			       16, 1, properties, data_len, true);
+			ret = -ENOTSUPP;
+		} else if (properties->len != data_len) {
+			pr_err("length mismatch, expected %u\n", data_len);

drivers/firmware/efi/apple-properties.c new file +6 -0

+		return ret;
+	}
+	return 0;
+}
+
+fs_initcall(map_properties);

include/linux/efi.h +6 -0

index 2617672..88f343f 100644
@@ -443,6 +443,22 @@ typedef struct {
 #define EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO_16 0x20000
 #define EFI_PCI_IO_ATTRIBUTE_VGA_IO_16 0x40000
 
+typedef struct {
+	u32 version;
+	u32 get;
+	u32 set;
+	u32 del;
+	u32 get_all;

include/linux/efi.h +1 -0

 /*
  * Types and defines for EFI ResetSystem
  */

@@ -592,6 +608,7 @@ void efi_native_runtime_setup(void);
 #define EFI_RNG_ALGORITHM_RAW			EFI_GUID(0xe43176d7, 0xb6e8, 0x4827,  0xb7, 0x84, 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61)
 #define EFI_MEMORY_ATTRIBUTES_TABLE_GUID	EFI_GUID(0xdcfa911d, 0x26eb, 0x469f,  0xa2, 0x20, 0x38, 0xb7, 0xdc, 0x46, 0x12, 0x20)
 #define EFI_CONSOLE_OUT_DEVICE_GUID		EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4,  0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
+#define APPLE_PROPERTIES_PROTOCOL_GUID		EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb,  0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0)
 
 /*
  * This GUID is used to pass to the kernel proper the struct screen_info

Signature · 1 line

2.10.1

arch/x86/boot/compressed/eboot.c static void setup_efi_pci(struct boot_params *params) · line 558

Quoted 7 lines · Lukas Wunner

arch/x86/boot/compressed/eboot.c +7 -0

+	if ((efi_is_64bit() ?
+			((apple_properties_protocol_64 *)properties)->version :
+			((apple_properties_protocol_32 *)properties)->version)
+								   != 0x10000) {
+		efi_printk(sys_table, "Unsupported properties proto version\n");
+		return;
+	}

It's a minor point, but please don't write the 32/64 code like this. Either use two separate functions, like __file_size32() and __file_size64(), or if that's two much code duplication stash the version field and get_all pointer in stack-local variables at the start of the function.

arch/x86/boot/compressed/eboot.c struct boot_params *efi_main(struct efi_config *c, · line 1163

Quoted 5 lines

arch/x86/boot/compressed/eboot.c +5 -0

+	if (!memcmp((void *)sys_table->fw_vendor, apple, sizeof(apple))) {
+		if (IS_ENABLED(CONFIG_APPLE_PROPERTIES))
+			retrieve_apple_device_properties(boot_params);
+	}
+

Could you split this off into a setup_apple_properties() function to mirror setup_efi_pci()?

drivers/firmware/efi/apple-properties.c line 90

Quoted 6 lines

drivers/firmware/efi/apple-properties.c new file +6 -0

+
+		key = kzalloc((key_len - sizeof(key_len)) * 4 + 1, GFP_KERNEL);
+		if (!key) {
+			dev_err(dev, "cannot allocate property name\n");
+			break;
+		}

Could you add a comment to document the kzalloc() size argument above?

drivers/firmware/efi/apple-properties.c line 120

Quoted 7 lines

drivers/firmware/efi/apple-properties.c new file +7 -0

+	if (i != dev_header->prop_count) {
+		dev_err(dev, "got %d device properties, expected %u\n", i,
+			dev_header->prop_count);
+		print_hex_dump(KERN_ERR, pr_fmt(), DUMP_PREFIX_OFFSET,
+			16, 1, dev_header, dev_header->len, true);
+	} else
+		dev_info(dev, "assigning %d device properties\n", i);

Mismatched braces are frowned upon in the CodingStyle guide. Please use braces for both the 'if' and 'else'. Alternatively, return early for one of the conditions (and save a level of indentation too).

Newline here please.

And a newline here.

Please sprinkle a few newlines to make this more readable.

Newline please.

Newline.

Newline.

Newline.

Ideally, I'd like to send the v4.10 material to tip by the end of the week.

Macs with Thunderbolt 1 do not have a unit-specific DROM: The DROM is empty with uid 0x1000000000000. (Apple started factory-burning a unit- specific DROM with Thunderbolt 2.)

Instead, the NHI EFI driver supplies a DROM in a device property. Use it if available. It's only available when booting with the efistub. If it's not available, silently fall back to our hardcoded DROM.

The size of the DROM is always 256 bytes. The number is hardcoded into the NHI EFI driver. This commit can deal with an arbitrary size however, just in case they ever change that.

Background information: The EFI firmware volume contains ROM files for the NHI, GMUX and several other chips as well as key material. This strategy allows Apple to deploy ROM or key updates by simply publishing an EFI firmware update on their website. Drivers do not access those files directly but rather through a file server via EFI protocol AC5E4829-A8FD-440B-AF33-9FFE013B12D8. Files are identified by GUID, the NHI DROM has 339370BD-CFC6-4454-8EF7-704653120818.

The NHI EFI driver amends that file with a unit-specific uid. The uid has 64 bit but its entropy is much lower: 24 bit represent the model, 24 bit are taken from a serial number, 16 bit are fixed. The NHI EFI driver obtains the serial number via the DataHub protocol, copies it into the DROM, calculates the CRC and submits the result as a device property.

A modification is needed in the resume code where we currently read the uid of all switches in the hierarchy to detect plug events that occurred during sleep. On Thunderbolt 1 root switches this will now lead to a mismatch between the uid of the empty DROM and the EFI DROM. Exempt the root switch from this check: It's built in, so the uid should never change. However we continue to *read* the uid of the root switch, this seems like a good way to test its reachability after resume.

Cc Pedro Vilaça <[email protected]>

Cc Andreas Noever <[email protected]>

Tested-by Lukas Wunner <lukas-JFq808J9C/[email protected]> [MacBookPro9,1]

Tested-by Pierre Moreau <[email protected]> [MacBookPro11,3]

Signed-off-by Lukas Wunner <lukas-JFq808J9C/[email protected]>

---
 drivers/thunderbolt/Kconfig  |  1 +
 drivers/thunderbolt/eeprom.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/switch.c |  2 +-
 3 files changed, 44 insertions(+), 1 deletion(-)

drivers/thunderbolt/Kconfig +1 -0

index c121acc..0056df7 100644
@@ -1,6 +1,7 @@
 menuconfig THUNDERBOLT
        tristate "Thunderbolt support for Apple devices"
        depends on PCI
+       select APPLE_PROPERTIES
        select CRC32
        help
          Cactus Ridge Thunderbolt Controller driver

drivers/thunderbolt/eeprom.c +20 -0

index 2b9602c..785a171 100644
@@ -5,6 +5,7 @@
  */
 
 #include <linux/crc32.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include "tb.h"
 

@@ -360,6 +361,39 @@ static int tb_drom_parse_entries(struct tb_switch *sw)
 }
 
 /**
+ * tb_drom_copy_efi - copy drom supplied by EFI to sw->drom if present
+ */
+static int tb_drom_copy_efi(struct tb_switch *sw, u16 *size)
+{
+       struct device *dev = &sw->tb->nhi->pdev->dev;
+       int len, res;
+
+       len = device_property_read_u8_array(dev, "ThunderboltDROM", NULL, 0);
+       if (len < 0 || len < sizeof(struct tb_drom_header))
+               return -EINVAL;
+
+       sw->drom = kmalloc(len, GFP_KERNEL);
+       if (!sw->drom)
+               return -ENOMEM;
+
+       res = device_property_read_u8_array(dev, "ThunderboltDROM", sw->drom,
+                                                                       len);
+       if (res)
+               goto err;

drivers/thunderbolt/eeprom.c +12 -0

+       return -EINVAL;
+}
+
+/**
  * tb_drom_read - copy drom to sw->drom and parse it
  */
 int tb_drom_read(struct tb_switch *sw)

@@ -374,6 +408,13 @@ int tb_drom_read(struct tb_switch *sw)
 
        if (tb_route(sw) == 0) {
                /*
+                * Apple's NHI EFI driver supplies a DROM for the root switch
+                * in a device property. Use it if available.
+                */
+               if (tb_drom_copy_efi(sw, &size) == 0)
+                       goto parse;
+
+               /*
                 * The root switch contains only a dummy drom (header only,
                 * no entries). Hardcode the configuration here.
                 */

@@ -418,6 +459,7 @@ int tb_drom_read(struct tb_switch *sw)
        if (res)
                goto err;
 
+parse:
        header = (void *) sw->drom;
 
        if (header->data_len + TB_DROM_DATA_START != size) {

drivers/thunderbolt/switch.c +1 -2

index 9840fde..c6f30b1 100644
@@ -460,7 +460,7 @@ int tb_switch_resume(struct tb_switch *sw)
                tb_sw_warn(sw, "uid read failed\n");
                return err;
        }
-       if (sw->uid != uid) {
+       if (sw != sw->tb->root_switch && sw->uid != uid) {
                tb_sw_info(sw,
                        "changed while suspended (uid %#llx -> %#llx)\n",
                        sw->uid, uid);
--

2.10.1

drivers/thunderbolt/eeprom.c static int tb_drom_parse_entries(struct tb_switch *sw) · line 392

Quoted 2 lines · Lukas Wunner

drivers/thunderbolt/eeprom.c +2 -0

+err:
+       kfree(sw->drom);

Please also set sw->drom to NULL here. Otherwise sw->drom will be freed again in the error path of tb_switch_alloc or in tb_switch_free.

btw: any success in decoding the path crumbs property?

Retrieve device properties from EFI on Macs before ExitBootServices is called and assign them to devices (patch [2/3]). The devices that properties pertain to are encoded as EFI Device Paths, so add a parser for these (patch [1/3]). As a first use case, amend the Thunderbolt driver to take advantage of the Device ROM supplied by EFI (patch [3/3]).

Changes since v2:

- Renamed get_device_by_efi_path() to efi_get_device_by_path() in patch [1/3]. (Matt Fleming)

- Support multiple instance device paths in patch [1/3]. (Peter Jones)

- Validate the size specified in the properties payload in patch [2/3].

- Other than that, just some minor polishing and rebasing.

Link to v2:
http://www.spinics.net/lists/linux-efi/msg09498.html

Browseable on GitHub:
https://github.com/l1k/linux/commits/apple_properties_v3

Quoted 15 lines · Lukas Wunner

Retrieve device properties from EFI on Macs before ExitBootServices is called and assign them to devices (patch [2/3]). The devices that properties pertain to are encoded as EFI Device Paths, so add a parser for these (patch [1/3]). As a first use case, amend the Thunderbolt driver to take advantage of the Device ROM supplied by EFI (patch [3/3]).

Changes since v2:

- Renamed get_device_by_efi_path() to efi_get_device_by_path() in patch [1/3]. (Matt Fleming)

- Support multiple instance device paths in patch [1/3]. (Peter Jones)

- Validate the size specified in the properties payload in patch [2/3].

Quoted 7 lines

- Other than that, just some minor polishing and rebasing.

Link to v2:
http://www.spinics.net/lists/linux-efi/msg09498.html

Browseable on GitHub:
https://github.com/l1k/linux/commits/apple_properties_v3

Apart from the minor comments I made, I think this is now ready for merging.

Andreas, once Lukas spins a v4 of this series, how would you like to handle the merging since there are both EFI and thunderbolt changes?

If you'd like me to take this through the EFI tree, that's fine, but I'll need an ACK for the thunderbolt changes (PATCH 3).

Hi Matt,

PATCH 3 is acked by me (with one minor comment). I'm fine with pushing this series through the EFI tree (I'm not expecting many changes on the tb side, so the should be no conflicts).