efi_loader: add back optional efi_handler::open()

PATCH New Rob Clark 2 participants 6 messages Jul 24, 2017 – Jul 28, 2017

Share & tools

Rob Clark

author

2017-07-24 14:38 UTC #0 lore raw

PATCH 1/1 efi_loader: add back optional efi_handler::open()

In some cases it is useful to defer creation of the protocol interface object. So add back an optional ->open() hook that is used if protcol_interface is NULL.

I've slightly simplified the fxn ptr signature to remove unneeded args, and so compiler will complain if patches that used the "old way" are, and which do not need this extra complexity, are rebased.

Signed-off-by Rob Clark <[email protected]>

---
 include/efi_loader.h          | 9 ++++++++-
 lib/efi_loader/efi_boottime.c | 7 +++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

include/efi_loader.h +8 -1

index 90411cdbab..3da0477fdc 100644
@@ -41,10 +41,17 @@ extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
 /*
  * When the UEFI payload wants to open a protocol on an object to get its
  * interface (usually a struct with callback functions), this struct maps the
- * protocol GUID to the respective protocol interface */
+ * protocol GUID to the respective protocol interface.
+ *
+ * The optional ->open() fxn can be used for cases where the protocol
+ * interface is constructed on-demand, and is called if protocol_interface
+ * is NULL.
+ */
 struct efi_handler {
 	const efi_guid_t *guid;
 	void *protocol_interface;
+	efi_status_t (EFIAPI *open)(void *handle, efi_guid_t *protocol,
+			void **protocol_interface);
 };
 
 /*

lib/efi_loader/efi_boottime.c +7 -0

index 7d45c18ff7..21f41866d0 100644
@@ -1063,6 +1063,13 @@ static efi_status_t EFIAPI efi_open_protocol(
 			if (!guidcmp(hprotocol, protocol)) {
 				if (attributes !=
 				    EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
+					if (!handler->protocol_interface) {
+						r = handler->open(efiobj->handle,
+								protocol,
+								&handler->protocol_interface);
+						if (r != EFI_SUCCESS)
+							goto out;
+					}
 					*protocol_interface =
 						handler->protocol_interface;
 				}

Signature · 1 line

2.13.0

Rob Clark

author

2017-07-24 14:39 UTC #1 lore raw

PATCH 1/2 efi_loader: add helper macro to construct protocol objects

There are a bunch of protocols which should be exposed by GUID but are not. Add a helper macro to create an efi_object, to avoid much typing.

Note that using the pointer for efiobj->handle is semi-arbitrary. We just need a unique value to match the efiobj supporting the protocol with the handle that LocateHandle() returns..

See LibLocateProtocol() in gnu-efi. It does LocateHandle() to find all the handles, and then loops over them calling HandleProtocol() with the GUID of the protocol it is trying to find.

Signed-off-by Rob Clark <[email protected]>

---
 include/efi_loader.h | 8 ++++++++
 1 file changed, 8 insertions(+)

include/efi_loader.h +8 -0

index 27c741ba0c..51fbf23864 100644
@@ -63,6 +63,14 @@ struct efi_object {
 	void *handle;
 };
 
+#define EFI_PROTOCOL_OBJECT(_guid, _protocol) (struct efi_object){	\
+	.protocols = {{							\
+		.guid = &(_guid),	 				\
+		.protocol_interface = (void *)(_protocol), 		\
+	}},								\
+	.handle = (void *)(_protocol),					\
+}
+
 /**
  * struct efi_event
  *

Signature · 1 line

2.13.0

Alexander Graf

Applied

2017-07-28 22:27 UTC #2 lore raw

PATCH 1/2 efi_loader: add helper macro to construct protocol objects

Quoted 7 lines

There are a bunch of protocols which should be exposed by GUID but are not. Add a helper macro to create an efi_object, to avoid much typing.
Note that using the pointer for efiobj->handle is semi-arbitrary. We just need a unique value to match the efiobj supporting the protocol with the handle that LocateHandle() returns..

Quoted 5 lines

See LibLocateProtocol() in gnu-efi. It does LocateHandle() to find all the handles, and then loops over them calling HandleProtocol() with the GUID of the protocol it is trying to find.
Signed-off-by Rob Clark <[email protected]>

Thanks, applied to efi-next

Alex

Rob Clark

author

2017-07-24 14:39 UTC #3 lore raw

PATCH 2/2 efi_loader: expose protocols via GUID

In reply to Rob Clark

shim.efi (or rather gnu-efi's LibLocateProtocol() which shim.efi uses) resolves protocols via efi_locate_handle() so the console protocols need to be added to the efi object list.

Signed-off-by Rob Clark <[email protected]>

---
 include/efi_api.h            |  9 +++++++++
 include/efi_loader.h         |  1 +
 lib/efi_loader/efi_console.c | 15 ++++++++++++++-
 3 files changed, 24 insertions(+), 1 deletion(-)

include/efi_api.h +9 -0

index 2c443080b6..d8fecb6980 100644
@@ -393,6 +393,11 @@ struct simple_text_output_mode {
 	bool cursor_visible;
 };
 
+
+#define EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL_GUID \
+	EFI_GUID(0x387477c2, 0x69c7, 0x11d2, \
+		 0x8e, 0x39, 0x0, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
+
 struct efi_simple_text_output_protocol {
 	void *reset;
 	efi_status_t (EFIAPI *output_string)(

@@ -427,6 +432,10 @@ struct efi_input_key {
 	s16 unicode_char;
 };
 
+#define EFI_SIMPLE_TEXT_INPUT_PROTOCOL_GUID \
+	EFI_GUID(0x387477c1, 0x69c7, 0x11d2, \
+		 0x8e, 0x39, 0x0, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
+
 struct efi_simple_input_interface {
 	efi_status_t(EFIAPI *reset)(struct efi_simple_input_interface *this,
 			bool ExtendedVerification);

include/efi_loader.h +1 -0

index 51fbf23864..48eeb4cdd8 100644
@@ -109,6 +109,7 @@ int efi_net_register(void **handle);
 /* Called by bootefi to make SMBIOS tables available */
 void efi_smbios_register(void);
 
+
 /* Called by networking code to memorize the dhcp ack package */
 void efi_net_set_dhcp_ack(void *pkt, int len);

lib/efi_loader/efi_console.c +14 -1

index dbe98ac08b..ba0a8b90cd 100644
@@ -426,7 +426,6 @@ struct efi_simple_input_interface efi_con_in = {
 	.read_key_stroke = efi_cin_read_key_stroke,
 	.wait_for_key = NULL,
 };
-
 static struct efi_event *console_timer_event;
 
 static void efi_key_notify(struct efi_event *event, void *context)

@@ -441,10 +440,24 @@ static void efi_console_timer_notify(struct efi_event *event, void *context)
 	EFI_EXIT(EFI_SUCCESS);
 }
 
+
+static struct efi_object efi_console_control_obj =
+	EFI_PROTOCOL_OBJECT(efi_guid_console_control, &efi_console_control);
+static struct efi_object efi_console_output_obj =
+	EFI_PROTOCOL_OBJECT(EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL_GUID, &efi_con_out);
+static struct efi_object efi_console_input_obj =
+	EFI_PROTOCOL_OBJECT(EFI_SIMPLE_TEXT_INPUT_PROTOCOL_GUID, &efi_con_in);
+
 /* This gets called from do_bootefi_exec(). */
 int efi_console_register(void)
 {
 	efi_status_t r;
+
+	/* Hook up to the device list */
+	list_add_tail(&efi_console_control_obj.link, &efi_obj_list);
+	list_add_tail(&efi_console_output_obj.link, &efi_obj_list);
+	list_add_tail(&efi_console_input_obj.link, &efi_obj_list);
+
 	r = efi_create_event(EVT_NOTIFY_WAIT, TPL_CALLBACK,
 			     efi_key_notify, NULL, &efi_con_in.wait_for_key);
 	if (r != EFI_SUCCESS) {

Signature · 1 line

2.13.0

Alexander Graf

Applied

2017-07-28 22:25 UTC #4 lore raw

PATCH 2/2 efi_loader: expose protocols via GUID

Quoted 5 lines

shim.efi (or rather gnu-efi's LibLocateProtocol() which shim.efi uses) resolves protocols via efi_locate_handle() so the console protocols need to be added to the efi object list.
Signed-off-by Rob Clark <[email protected]>

Thanks, applied to efi-next

Alex

Alexander Graf

2017-07-25 08:52 UTC #5 lore raw

PATCH 1/1 efi_loader: add back optional efi_handler::open()

In reply to Rob Clark

Quoted 42 lines · Rob Clark

In some cases it is useful to defer creation of the protocol interface object. So add back an optional ->open() hook that is used if protcol_interface is NULL.
I've slightly simplified the fxn ptr signature to remove unneeded args, and so compiler will complain if patches that used the "old way" are, and which do not need this extra complexity, are rebased.
Signed-off-by Rob Clark <[email protected]>
---
  include/efi_loader.h          | 9 ++++++++-
  lib/efi_loader/efi_boottime.c | 7 +++++++
  2 files changed, 15 insertions(+), 1 deletion(-)
include/efi_loader.h +8 -1
index 90411cdbab..3da0477fdc 100644
@@ -41,10 +41,17 @@ extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
  /*
   * When the UEFI payload wants to open a protocol on an object to get its
   * interface (usually a struct with callback functions), this struct maps the
- * protocol GUID to the respective protocol interface */
+ * protocol GUID to the respective protocol interface.
+ *
+ * The optional ->open() fxn can be used for cases where the protocol
+ * interface is constructed on-demand, and is called if protocol_interface
+ * is NULL.
+ */
  struct efi_handler {
  	const efi_guid_t *guid;
  	void *protocol_interface;
+	efi_status_t (EFIAPI *open)(void *handle, efi_guid_t *protocol,
+			void **protocol_interface);
  };
 
  /*
lib/efi_loader/efi_boottime.c +0 -0
index 7d45c18ff7..21f41866d0 100644

lib/efi_loader/efi_boottime.c static efi_status_t EFIAPI efi_open_protocol( · line 1069

Quoted 7 lines

lib/efi_loader/efi_boottime.c +4 -0

@@ -1063,6 +1063,13 @@ static efi_status_t EFIAPI efi_open_protocol(
  			if (!guidcmp(hprotocol, protocol)) {
  				if (attributes !=
  				    EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
+					if (!handler->protocol_interface) {
+						r = handler->open(efiobj->handle,
+								protocol,
+								&handler->protocol_interface);

With this patch you basically allow deferred setting of handler->protocol_interface. I think that's a reasonable thing to do, but we have to be careful not to break UEFI assumptions.

For example efi_locate_protocol reads handler->protocol_interface, so that needs adjustment I suppose.

Alex