[U-Boot] [PATCH 0/4] efi_loader: fix EFI_BLOCK_IO_PROTOCOL

[U-Boot] [PATCH 0/4] efi_loader: fix EFI_BLOCK_IO_PROTOCOL

[Heinrich Schuchardt]

* provide parameter checks for ReadBlocks and WriteBlocks
* correct return code for Reset
* indented debug output
* add EFI_BLOCK_IO_PROTOCOL and other protocols to HTML documentation

Heinrich Schuchardt (4):
  efi_loader: parameter checks BLOCK_IO_PROTOCOL
  efi_loader: use EFI_PRINT() instead of debug()
  efi_loader: EFI_BLOCK_IO_PROTOCOL.Reset()
  doc: UEFI API documentation

 doc/api/efi.rst              | 33 ++++++++++++++++++++
 lib/efi_loader/efi_console.c | 11 ++++---
 lib/efi_loader/efi_disk.c    | 60 +++++++++++++++++++++++++++++++++---
 lib/efi_loader/efi_gop.c     |  2 +-
 lib/efi_loader/efi_net.c     |  9 ++++--
 5 files changed, 101 insertions(+), 14 deletions(-)

--
2.23.0.rc1

[U-Boot] [PATCH 1/4] efi_loader: parameter checks BLOCK_IO_PROTOCOL

[Heinrich Schuchardt]

Check parameters of ReadBlocks() and WriteBlocks().

If the buffer size is not a multiple of the block size, we have to return
EFI_BAD_BUFFER_SIZE.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_disk.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 7a6b06821a..0bf436ac66 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -74,7 +74,7 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,

 	/* We only support full block access */
 	if (buffer_size & (blksz - 1))
-		return EFI_DEVICE_ERROR;
+		return EFI_BAD_BUFFER_SIZE;

 	if (direction == EFI_DISK_READ)
 		n = blk_dread(desc, lba, blocks, buffer);
@@ -99,6 +99,20 @@ static efi_status_t EFIAPI efi_disk_read_blocks(struct efi_block_io *this,
 	void *real_buffer = buffer;
 	efi_status_t r;

+	if (!this)
+		return EFI_INVALID_PARAMETER;
+	/* TODO: check for media changes */
+	if (media_id != this->media->media_id)
+		return EFI_MEDIA_CHANGED;
+	if (!this->media->media_present)
+		return EFI_NO_MEDIA;
+	/* media->io_align is a power of 2 */
+	if ((uintptr_t)buffer & (this->media->io_align - 1))
+		return EFI_INVALID_PARAMETER;
+	if (lba * this->media->block_size + buffer_size >
+	    this->media->last_block * this->media->block_size)
+		return EFI_INVALID_PARAMETER;
+
 #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
 	if (buffer_size > EFI_LOADER_BOUNCE_BUFFER_SIZE) {
 		r = efi_disk_read_blocks(this, media_id, lba,
@@ -134,6 +148,22 @@ static efi_status_t EFIAPI efi_disk_write_blocks(struct efi_block_io *this,
 	void *real_buffer = buffer;
 	efi_status_t r;

+	if (!this)
+		return EFI_INVALID_PARAMETER;
+	if (this->media->read_only)
+		return EFI_WRITE_PROTECTED;
+	/* TODO: check for media changes */
+	if (media_id != this->media->media_id)
+		return EFI_MEDIA_CHANGED;
+	if (!this->media->media_present)
+		return EFI_NO_MEDIA;
+	/* media->io_align is a power of 2 */
+	if ((uintptr_t)buffer & (this->media->io_align - 1))
+		return EFI_INVALID_PARAMETER;
+	if (lba * this->media->block_size + buffer_size >
+	    this->media->last_block * this->media->block_size)
+		return EFI_INVALID_PARAMETER;
+
 #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
 	if (buffer_size > EFI_LOADER_BOUNCE_BUFFER_SIZE) {
 		r = efi_disk_write_blocks(this, media_id, lba,
@@ -288,6 +318,11 @@ static efi_status_t efi_disk_add_dev(
 	/* Fill in EFI IO Media info (for read/write callbacks) */
 	diskobj->media.removable_media = desc->removable;
 	diskobj->media.media_present = 1;
+	/*
+	 * MediaID is just an arbitrary counter.
+	 * We have to change it if the medium is removed or changed.
+	 */
+	diskobj->media.media_id = 1;
 	diskobj->media.block_size = desc->blksz;
 	diskobj->media.io_align = desc->blksz;
 	diskobj->media.last_block = desc->lba - offset;
--
2.23.0.rc1

[U-Boot] [PATCH 2/4] efi_loader: use EFI_PRINT() instead of debug()

[Heinrich Schuchardt]

EFI_PRINT() offers indention of debug messages. Adjust the debug messages
of the BLOCK_IO_PROTOCOL.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_disk.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 0bf436ac66..4d4937c8ba 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -69,8 +69,8 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
 	blocks = buffer_size / blksz;
 	lba += diskobj->offset;

-	debug("EFI: %s:%d blocks=%x lba=%llx blksz=%x dir=%d\n", __func__,
-	      __LINE__, blocks, lba, blksz, direction);
+	EFI_PRINT("blocks=%x lba=%llx blksz=%x dir=%d\n",
+		  blocks, lba, blksz, direction);

 	/* We only support full block access */
 	if (buffer_size & (blksz - 1))
@@ -84,7 +84,7 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
 	/* We don't do interrupts, so check for timers cooperatively */
 	efi_timer_check();

-	debug("EFI: %s:%d n=%lx blocks=%x\n", __func__, __LINE__, n, blocks);
+	EFI_PRINT("n=%lx blocks=%x\n", n, blocks);

 	if (n != blocks)
 		return EFI_DEVICE_ERROR;
--
2.23.0.rc1

[U-Boot] [PATCH 3/4] efi_loader: EFI_BLOCK_IO_PROTOCOL.Reset()

[Heinrich Schuchardt]

We cannot do anything in EFI_BLOCK_IO_PROTOCOL.Reset() but this does not
justify to return an error.

Let EFI_BLOCK_IO_PROTOCOL.Reset() return EFI_SUCCESS.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_disk.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 4d4937c8ba..9007a5f77f 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -41,11 +41,26 @@ struct efi_disk_obj {
 	struct blk_desc *desc;
 };

+/**
+ * efi_disk_reset() - reset block device
+ *
+ * This function implements the Reset service of the EFI_BLOCK_IO_PROTOCOL.
+ *
+ * As U-Boot's block devices do not have a reset function simply return
+ * EFI_SUCCESS.
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * @this:			pointer to the BLOCK_IO_PROTOCOL
+ * @extended_verification:	extended verification
+ * Return:			status code
+ */
 static efi_status_t EFIAPI efi_disk_reset(struct efi_block_io *this,
 			char extended_verification)
 {
 	EFI_ENTRY("%p, %x", this, extended_verification);
-	return EFI_EXIT(EFI_DEVICE_ERROR);
+	return EFI_EXIT(EFI_SUCCESS);
 }

 enum efi_disk_direction {
--
2.23.0.rc1

[U-Boot] [PATCH 4/4] doc: UEFI API documentation

[Heinrich Schuchardt]

Add some more files to the UEFI API documentation.

Correct some Sphinx comments.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 doc/api/efi.rst              | 33 +++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_console.c | 11 ++++++-----
 lib/efi_loader/efi_gop.c     |  2 +-
 lib/efi_loader/efi_net.c     |  9 ++++++---
 4 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/doc/api/efi.rst b/doc/api/efi.rst
index 39e2dbae0b..2ca344932e 100644
--- a/doc/api/efi.rst
+++ b/doc/api/efi.rst
@@ -103,3 +103,36 @@ Block device driver

 .. kernel-doc:: lib/efi_driver/efi_block_device.c
    :internal:
+
+Protocols
+---------
+
+Block IO protocol
+~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: lib/efi_loader/efi_disk.c
+   :internal:
+
+File protocol
+~~~~~~~~~~~~~
+
+.. kernel-doc:: lib/efi_loader/efi_file.c
+   :internal:
+
+Graphical output protocol
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: lib/efi_loader/efi_gop.c
+   :internal:
+
+Network protocols
+~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: lib/efi_loader/efi_net.c
+   :internal:
+
+Text IO protocols
+~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: lib/efi_loader/efi_console.c
+   :internal:
diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
index 5109017796..a55e4b33ec 100644
--- a/lib/efi_loader/efi_console.c
+++ b/lib/efi_loader/efi_console.c
@@ -219,9 +219,9 @@ static bool cout_mode_matches(struct cout_mode *mode, int rows, int cols)
 /**
  * query_console_serial() - query console size
  *
- * @rows	pointer to return number of rows
- * @columns	pointer to return number of columns
- * Returns	0 on success
+ * @rows:	pointer to return number of rows
+ * @cols:	pointer to return number of columns
+ * Returns:	0 on success
  */
 static int query_console_serial(int *rows, int *cols)
 {
@@ -464,7 +464,7 @@ struct efi_simple_text_output_protocol efi_con_out = {
  * struct efi_cin_notify_function - registered console input notify function
  *
  * @link:	link to list
- * @data:	key to notify
+ * @key:	key to notify
  * @function:	function to call
  */
 struct efi_cin_notify_function {
@@ -482,6 +482,7 @@ static LIST_HEAD(cin_notify_functions);
  * set_shift_mask() - set shift mask
  *
  * @mod:	Xterm shift mask
+ * @key_state:  receives the state of the shift, alt, control, and logo keys
  */
 void set_shift_mask(int mod, struct efi_key_state *key_state)
 {
@@ -504,7 +505,7 @@ void set_shift_mask(int mod, struct efi_key_state *key_state)
  *
  * This gets called when we have already parsed CSI.
  *
- * @modifiers:  bit mask (shift, alt, ctrl)
+ * @key_state:  receives the state of the shift, alt, control, and logo keys
  * @return:	the unmodified code
  */
 static int analyze_modifiers(struct efi_key_state *key_state)
diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
index cad509bfea..1511e3bdb4 100644
--- a/lib/efi_loader/efi_gop.c
+++ b/lib/efi_loader/efi_gop.c
@@ -319,7 +319,7 @@ static efi_status_t gop_blt_vid_to_buf(struct efi_gop *this,
  * details.
  *
  * @this:		the graphical output protocol
- * @model_number:	the mode to be set
+ * @mode_number:	the mode to be set
  * Return:		status code
  */
 static efi_status_t EFIAPI gop_set_mode(struct efi_gop *this, u32 mode_number)
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index cff6332a68..82d2595847 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -399,7 +399,7 @@ out:
  * Protocol. See the UEFI spec for details.
  *
  * @this:		the instance of the Simple Network Protocol
- * @readwrite:		true for read, false for write
+ * @read_write:		true for read, false for write
  * @offset:		offset in NVRAM
  * @buffer_size:	size of buffer
  * @buffer:		buffer
@@ -639,6 +639,9 @@ out:
  * efi_net_set_dhcp_ack() - take note of a selected DHCP IP address
  *
  * This function is called by dhcp_handler().
+ *
+ * @pkt:	packet received by dhcp_handler()
+ * @len:	length of the packet received
  */
 void efi_net_set_dhcp_ack(void *pkt, int len)
 {
@@ -668,8 +671,8 @@ static void efi_net_push(void *pkt, int len)
  *
  * This notification function is called in every timer cycle.
  *
- * @event	the event for which this notification function is registered
- * @context	event context - not used in this function
+ * @event:	the event for which this notification function is registered
+ * @context:	event context - not used in this function
  */
 static void EFIAPI efi_network_timer_notify(struct efi_event *event,
 					    void *context)
--
2.23.0.rc1