public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Rob Clark <robdclark@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] efi_loader: add back optional efi_handler::open()
Date: Tue, 25 Jul 2017 08:21:35 -0400	[thread overview]
Message-ID: <20170725122135.27292-1-robdclark@gmail.com> (raw)

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 <robdclark@gmail.com>
---
v2: also handle this properly in locate_protocol as agraf pointed out

Note that I might be misunderstanding what the EFI application expects
when looking up device-path on a file handle.. if this is only expected
to be the device path up to the partition object (and not including the
file-path part of the path), then maybe ->open() isn't *as* important
as I thought it would be.  OTOH I think it would be cleaner to use for
loaded_image_info_obj and bootefi_device_obj.. currently I have to
patch in the real boot device-path in efi_set_bootdev(), which is
messy and fragile.

I don't fully understand Heinrich's concerns, especially since the
EFI_OPEN_PROTOCOL_INFORMATION_ENTRY stuff looks like it only cares
about already opened protocols, which should already have their open()
called.  (And either way, since open() is optional maybe it is enough
to just not use it for any efiobj's where it would be problematic?)

So tl;dr, if this is really going to be a problem for some things that
Heinrich has got in the pipeline, maybe (pending clarification of a
file handle's device-path) we can get by without this.  But if it is
not a problem, then it certainly seems useful in a few places.

 include/efi_loader.h          |  9 ++++++++-
 lib/efi_loader/efi_boottime.c | 30 ++++++++++++++++++++++++------
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 48eeb4cdd8..a9888afb04 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -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, const efi_guid_t *protocol,
+			void **protocol_interface);
 };
 
 /*
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 7d45c18ff7..80d9024a53 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -903,6 +903,23 @@ out:
 	return EFI_EXIT(r);
 }
 
+static efi_status_t open_protocol(struct efi_object *efiobj,
+				  struct efi_handler *handler,
+				  void **protocol_interface)
+{
+	efi_status_t ret = EFI_SUCCESS;
+
+	if (!handler->protocol_interface) {
+		ret = handler->open(efiobj->handle,
+				handler->guid,
+				&handler->protocol_interface);
+	}
+	*protocol_interface =
+		handler->protocol_interface;
+
+	return ret;
+}
+
 static efi_status_t EFIAPI efi_locate_protocol(efi_guid_t *protocol,
 					       void *registration,
 					       void **protocol_interface)
@@ -925,9 +942,10 @@ static efi_status_t EFIAPI efi_locate_protocol(efi_guid_t *protocol,
 			if (!handler->guid)
 				continue;
 			if (!guidcmp(handler->guid, protocol)) {
-				*protocol_interface =
-					handler->protocol_interface;
-				return EFI_EXIT(EFI_SUCCESS);
+				efi_status_t ret;
+				ret = open_protocol(efiobj, handler,
+						    protocol_interface);
+				return EFI_EXIT(ret);
 			}
 		}
 	}
@@ -1061,12 +1079,12 @@ static efi_status_t EFIAPI efi_open_protocol(
 			if (!hprotocol)
 				continue;
 			if (!guidcmp(hprotocol, protocol)) {
+				r = EFI_SUCCESS;
 				if (attributes !=
 				    EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
-					*protocol_interface =
-						handler->protocol_interface;
+					r = open_protocol(efiobj, handler,
+							  protocol_interface);
 				}
-				r = EFI_SUCCESS;
 				goto out;
 			}
 		}
-- 
2.13.0

             reply	other threads:[~2017-07-25 12:21 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-25 12:21 Rob Clark [this message]
2017-07-25 16:39 ` [U-Boot] [PATCH] efi_loader: add back optional efi_handler::open() Heinrich Schuchardt
2017-07-25 17:20   ` Rob Clark
  -- strict thread matches above, loose matches on Subject: below --
2017-07-24 14:38 Rob Clark
2017-07-25  8:52 ` Alexander Graf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170725122135.27292-1-robdclark@gmail.com \
    --to=robdclark@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox