Re: [PATCH 1/1] efi_loader: implement non-blocking file services

On 30.04.21 12:09, Michal Simek wrote:

Hi Heinrich,

pá 1. 1. 2021 v 19:29 odesílatel Heinrich Schuchardt xypron.glpk@gmx.de napsal:

...

Implement services OpenEx(), ReadEx(), WriteEx(), FlushEx() of the EFI_FILE_PROTOCOL.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

include/efi_api.h | 28 ++-- lib/efi_loader/efi_file.c | 317 ++++++++++++++++++++++++++++++++------ 2 files changed, 280 insertions(+), 65 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h index ecb43a0607..2b54ee02a2 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -1589,35 +1589,35 @@ struct efi_file_io_token {

struct efi_file_handle { u64 rev;

•   efi_status_t (EFIAPI *open)(struct efi_file_handle *file,
  

•   efi_status_t (EFIAPI *open)(struct efi_file_handle *this,
                    struct efi_file_handle **new_handle,
                    u16 *file_name, u64 open_mode, u64 attributes);
  

•   efi_status_t (EFIAPI *close)(struct efi_file_handle *file);
  

•   efi_status_t (EFIAPI *delete)(struct efi_file_handle *file);
  

•   efi_status_t (EFIAPI *read)(struct efi_file_handle *file,
  

•   efi_status_t (EFIAPI *close)(struct efi_file_handle *this);
  

•   efi_status_t (EFIAPI *delete)(struct efi_file_handle *this);
  

•   efi_status_t (EFIAPI *read)(struct efi_file_handle *this,
                    efi_uintn_t *buffer_size, void *buffer);
  

•   efi_status_t (EFIAPI *write)(struct efi_file_handle *file,
  

•   efi_status_t (EFIAPI *write)(struct efi_file_handle *this,
                    efi_uintn_t *buffer_size, void *buffer);
  

•   efi_status_t (EFIAPI *getpos)(struct efi_file_handle *file,
  

•   efi_status_t (EFIAPI *getpos)(struct efi_file_handle *this,
                                  u64 *pos);
  

•   efi_status_t (EFIAPI *setpos)(struct efi_file_handle *file,
  

•   efi_status_t (EFIAPI *setpos)(struct efi_file_handle *this,
                                  u64 pos);
  

•   efi_status_t (EFIAPI *getinfo)(struct efi_file_handle *file,
  

•   efi_status_t (EFIAPI *getinfo)(struct efi_file_handle *this,
                    const efi_guid_t *info_type, efi_uintn_t *buffer_size,
                    void *buffer);
  

•   efi_status_t (EFIAPI *setinfo)(struct efi_file_handle *file,
  

•   efi_status_t (EFIAPI *setinfo)(struct efi_file_handle *this,
                    const efi_guid_t *info_type, efi_uintn_t buffer_size,
                    void *buffer);
  

•   efi_status_t (EFIAPI *flush)(struct efi_file_handle *file);
  

•   efi_status_t (EFIAPI *open_ex)(struct efi_file_handle *file,
  

•   efi_status_t (EFIAPI *flush)(struct efi_file_handle *this);
  

•   efi_status_t (EFIAPI *open_ex)(struct efi_file_handle *this,
                    struct efi_file_handle **new_handle,
                    u16 *file_name, u64 open_mode, u64 attributes,
                    struct efi_file_io_token *token);
  

•   efi_status_t (EFIAPI *read_ex)(struct efi_file_handle *file,
  

•   efi_status_t (EFIAPI *read_ex)(struct efi_file_handle *this,
                    struct efi_file_io_token *token);
  

•   efi_status_t (EFIAPI *write_ex)(struct efi_file_handle *file,
  

•   efi_status_t (EFIAPI *write_ex)(struct efi_file_handle *this,
                    struct efi_file_io_token *token);
  

•   efi_status_t (EFIAPI *flush_ex)(struct efi_file_handle *file,
  

•   efi_status_t (EFIAPI *flush_ex)(struct efi_file_handle *this,
                    struct efi_file_io_token *token);
  

};

diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c index 72b7ec1e63..8ece8e71ee 100644 --- a/lib/efi_loader/efi_file.c +++ b/lib/efi_loader/efi_file.c @@ -246,18 +246,16 @@ error: return NULL; }

-static efi_status_t EFIAPI efi_file_open(struct efi_file_handle *file,

•           struct efi_file_handle **new_handle,
  

•           u16 *file_name, u64 open_mode, u64 attributes)
  

+static efi_status_t efi_file_open_int(struct efi_file_handle *this,

•                                 struct efi_file_handle **new_handle,
  

•                                 u16 *file_name, u64 open_mode,
  

•                                 u64 attributes)
  

{

•   struct file_handle *fh = to_fh(file);
  

•   struct file_handle *fh = to_fh(this);
    efi_status_t ret;
  
  

•   EFI_ENTRY("%p, %p, \"%ls\", %llx, %llu", file, new_handle,
  

•             file_name, open_mode, attributes);
  

•   /* Check parameters */
  

•   if (!file || !new_handle || !file_name) {
  

•   if (!this || !new_handle || !file_name) {
            ret = EFI_INVALID_PARAMETER;
            goto out;
    }
  

@@ -291,6 +289,75 @@ static efi_status_t EFIAPI efi_file_open(struct efi_file_handle *file, } else { ret = EFI_NOT_FOUND; } +out:

•   return ret;
  

+}

+/**

• • efi_file_open_()
• • This function implements the Open service of the File Protocol.
• • See the UEFI spec for details.
• • @this: EFI_FILE_PROTOCOL instance
• • @new_handle: on return pointer to file handle
• • @file_name: file name
• • @open_mode: mode to open the file (read, read/write, create/read/write)
• • @attributes: attributes for newly created file
• */

+static efi_status_t EFIAPI efi_file_open(struct efi_file_handle *this,

•                                    struct efi_file_handle **new_handle,
  

•                                    u16 *file_name, u64 open_mode,
  

•                                    u64 attributes)
  

+{

•   efi_status_t ret;
  

•   EFI_ENTRY("%p, %p, \"%ls\", %llx, %llu", this, new_handle,
  

•             file_name, open_mode, attributes);
  

•   ret = efi_file_open_int(this, new_handle, file_name, open_mode,
  

•                           attributes);
  

•   return EFI_EXIT(ret);
  

+}

+/**

• • efi_file_open_ex() - open file asynchronously
• • This function implements the OpenEx service of the File Protocol.
• • See the UEFI spec for details.
• • @this: EFI_FILE_PROTOCOL instance
• • @new_handle: on return pointer to file handle
• • @file_name: file name
• • @open_mode: mode to open the file (read, read/write, create/read/write)
• • @attributes: attributes for newly created file
• • @token: transaction token
• */

+static efi_status_t EFIAPI efi_file_open_ex(struct efi_file_handle *this,

•                                       struct efi_file_handle **new_handle,
  

•                                       u16 *file_name, u64 open_mode,
  

•                                       u64 attributes,
  

•                                       struct efi_file_io_token *token)
  

+{

•   efi_status_t ret;
  

•   EFI_ENTRY("%p, %p, \"%ls\", %llx, %llu, %p", this, new_handle,
  

•             file_name, open_mode, attributes, token);
  

•   if (!token) {
  

•           ret = EFI_INVALID_PARAMETER;
  

•           goto out;
  

•   }
  

•   ret = efi_file_open_int(this, new_handle, file_name, open_mode,
  

•                           attributes);
  

•   if (ret == EFI_SUCCESS && token->event) {
  

•           token->status = EFI_SUCCESS;
  

•           efi_signal_event(token->event);
  

•   }
  

out: return EFI_EXIT(ret); } @@ -441,19 +508,15 @@ static efi_status_t dir_read(struct file_handle *fh, u64 *buffer_size, return EFI_SUCCESS; }

-static efi_status_t EFIAPI efi_file_read(struct efi_file_handle *file,

•                                    efi_uintn_t *buffer_size, void *buffer)
  

+static efi_status_t efi_file_read_int(struct efi_file_handle *this,

•                                 efi_uintn_t *buffer_size, void *buffer)
  

{

•   struct file_handle *fh = to_fh(file);
  

•   struct file_handle *fh = to_fh(this);
    efi_status_t ret = EFI_SUCCESS;
    u64 bs;
  
  

•   EFI_ENTRY("%p, %p, %p", file, buffer_size, buffer);
  

•   if (!buffer_size) {
  

•           ret = EFI_INVALID_PARAMETER;
  

•           goto error;
  

•   }
  

•   if (!this || !buffer_size || !buffer)
  

•           return EFI_INVALID_PARAMETER;
  
    bs = *buffer_size;
    if (fh->isdir)
  

@@ -465,34 +528,77 @@ static efi_status_t EFIAPI efi_file_read(struct efi_file_handle *file, else *buffer_size = SIZE_MAX;

-error:

•   return ret;
  

+}

+/**

• • efi_file_read() - read file
• • This function implements the Read() service of the EFI_FILE_PROTOCOL.
• • See the Unified Extensible Firmware Interface (UEFI) specification for
• • details.
• • @this: file protocol instance
• • @buffer_size: number of bytes to read
• • @buffer: read buffer
• • Return: status code
• */

+static efi_status_t EFIAPI efi_file_read(struct efi_file_handle *this,

•                                    efi_uintn_t *buffer_size, void *buffer)
  

+{

•   efi_status_t ret;
  

•   EFI_ENTRY("%p, %p, %p", this, buffer_size, buffer);
  

•   ret = efi_file_read_int(this, buffer_size, buffer);
  

•   return EFI_EXIT(ret);
  

}

/**

• • efi_file_write() - write to file

• • efi_file_read_ex() - read file asynchonously

• • This function implements the Write() service of the EFI_FILE_PROTOCOL.

• • This function implements the ReadEx() service of the EFI_FILE_PROTOCOL.
  • See the Unified Extensible Firmware Interface (UEFI) specification for
  • details.

• • @file: file handle
• • @buffer_size: number of bytes to write
• • @buffer: buffer with the bytes to write

• • @this: file protocol instance
• • @token: transaction token
  • Return: status code
  */

-static efi_status_t EFIAPI efi_file_write(struct efi_file_handle *file,

•                                     efi_uintn_t *buffer_size,
  

•                                     void *buffer)
  

+static efi_status_t EFIAPI efi_file_read_ex(struct efi_file_handle *this,

•                                       struct efi_file_io_token *token)
  

{

•   struct file_handle *fh = to_fh(file);
  

•   efi_status_t ret;
  

•   EFI_ENTRY("%p, %p", this, token);
  

•   if (!token) {
  

•           ret = EFI_INVALID_PARAMETER;
  

•           goto out;
  

•   }
  

•   ret = efi_file_read_int(this, &token->buffer_size, token->buffer);
  

•   if (ret == EFI_SUCCESS && token->event) {
  

•           token->status = EFI_SUCCESS;
  

•           efi_signal_event(token->event);
  

•   }
  

+out:

•   return EFI_EXIT(ret);
  

+}

+static efi_status_t efi_file_write_int(struct efi_file_handle *this,

•                                  efi_uintn_t *buffer_size, void *buffer)
  

+{

•   struct file_handle *fh = to_fh(this);
    efi_status_t ret = EFI_SUCCESS;
    loff_t actwrite;
  
  

•   EFI_ENTRY("%p, %p, %p", file, buffer_size, buffer);
  

•   if (!file || !buffer_size || !buffer) {
  

•   if (!this || !buffer_size || !buffer) {
            ret = EFI_INVALID_PARAMETER;
            goto out;
    }
  

@@ -520,6 +626,67 @@ static efi_status_t EFIAPI efi_file_write(struct efi_file_handle *file, *buffer_size = actwrite; fh->offset += actwrite;

+out:

•   return ret;
  

+}

+/**

• • efi_file_write() - write to file
• • This function implements the Write() service of the EFI_FILE_PROTOCOL.
• • See the Unified Extensible Firmware Interface (UEFI) specification for
• • details.
• • @this: file protocol instance
• • @buffer_size: number of bytes to write
• • @buffer: buffer with the bytes to write
• • Return: status code
• */

+static efi_status_t EFIAPI efi_file_write(struct efi_file_handle *this,

•                                     efi_uintn_t *buffer_size,
  

•                                     void *buffer)
  

+{

•   efi_status_t ret;
  

•   EFI_ENTRY("%p, %p, %p", this, buffer_size, buffer);
  

•   ret = efi_file_write_int(this, buffer_size, buffer);
  

•   return EFI_EXIT(ret);
  

+}

+/**

• • efi_file_write_ex() - write to file
• • This function implements the WriteEx() service of the EFI_FILE_PROTOCOL.
• • See the Unified Extensible Firmware Interface (UEFI) specification for
• • details.
• • @this: file protocol instance
• • @token: transaction token
• • Return: status code
• */

+static efi_status_t EFIAPI efi_file_write_ex(struct efi_file_handle *this,

•                                        struct efi_file_io_token *token)
  

+{

•   efi_status_t ret;
  

•   EFI_ENTRY("%p, %p", this, token);
  

•   if (!token) {
  

•           ret = EFI_INVALID_PARAMETER;
  

•           goto out;
  

•   }
  

•   ret = efi_file_write_int(this, &token->buffer_size, token->buffer);
  

•   if (ret == EFI_SUCCESS && token->event) {
  

•           token->status = EFI_SUCCESS;
  

•           efi_signal_event(token->event);
  

•   }
  

out: return EFI_EXIT(ret); } @@ -761,36 +928,84 @@ out: return EFI_EXIT(ret); }

-static efi_status_t EFIAPI efi_file_flush(struct efi_file_handle *file) +/**

• • efi_file_flush_int() - flush file
• • This is the internal implementation of the Flush() and FlushEx() services of
• • the EFI_FILE_PROTOCOL.
• • @this: file protocol instance
• • Return: status code
• */

+static efi_status_t efi_file_flush_int(struct efi_file_handle *this) {

•   EFI_ENTRY("%p", file);
  

•   return EFI_EXIT(EFI_SUCCESS);
  

-}

•   struct file_handle *fh = to_fh(this);
  
  

-static efi_status_t EFIAPI efi_file_open_ex(struct efi_file_handle *file,

•                   struct efi_file_handle **new_handle,
  

•                   u16 *file_name, u64 open_mode, u64 attributes,
  

•                   struct efi_file_io_token *token)
  

-{

•   return EFI_UNSUPPORTED;
  

-}

•   if (!this)
  

•           return EFI_INVALID_PARAMETER;
  
  

-static efi_status_t EFIAPI efi_file_read_ex(struct efi_file_handle *file,

•                   struct efi_file_io_token *token)
  

-{

•   return EFI_UNSUPPORTED;
  

•   if (!(fh->open_mode & EFI_FILE_MODE_WRITE))
  

•           return EFI_ACCESS_DENIED;
  

•   /* TODO: flush for file position after end of file */
  

•   return EFI_SUCCESS;
  

}

-static efi_status_t EFIAPI efi_file_write_ex(struct efi_file_handle *file,

•                   struct efi_file_io_token *token)
  

+/**

• • efi_file_flush() - flush file
• • This function implements the Flush() service of the EFI_FILE_PROTOCOL.
• • See the Unified Extensible Firmware Interface (UEFI) specification for
• • details.
• • @this: file protocol instance
• • Return: status code
• */

+static efi_status_t EFIAPI efi_file_flush(struct efi_file_handle *this) {

•   return EFI_UNSUPPORTED;
  

•   efi_status_t ret;
  

•   EFI_ENTRY("%p", this);
  

•   ret = efi_file_flush_int(this);
  

•   return EFI_EXIT(ret);
  

}

-static efi_status_t EFIAPI efi_file_flush_ex(struct efi_file_handle *file,

•                   struct efi_file_io_token *token)
  

+/**

• • efi_file_flush_ex() - flush file
• • This function implements the FlushEx() service of the EFI_FILE_PROTOCOL.
• • See the Unified Extensible Firmware Interface (UEFI) specification for
• • details.
• • @this: file protocol instance
• • @token: transaction token
• • Return: status code
• */

+static efi_status_t EFIAPI efi_file_flush_ex(struct efi_file_handle *this,

•                                        struct efi_file_io_token *token)
  

{

•   return EFI_UNSUPPORTED;
  

•   efi_status_t ret;
  

•   EFI_ENTRY("%p, %p", this, token);
  

•   if (!token) {
  

•           ret = EFI_INVALID_PARAMETER;
  

•           goto out;
  

•   }
  

•   ret = efi_file_flush_int(this);
  

•   if (ret == EFI_SUCCESS && token->event) {
  

•           token->status = EFI_SUCCESS;
  

•           efi_signal_event(token->event);
  

•   }
  

+out:

•   return EFI_EXIT(ret);
  

}

static const struct efi_file_handle efi_file_handle_protocol = {

2.29.2

I have been trying to boot Fedora 35 Rawhide which booted fine with 2021.01 version but not with 2021.04 version. I have bisect it and this patch is causing that grub doesn't start automatically for this image. (I haven't checked others yet).

U-Boot 2021.01-00398-gdb12f518edb0 (Apr 30 2021 - 12:00:13 +0200)

Model: ZynqMP ZCU104 RevC Board: Xilinx ZynqMP DRAM: 2 GiB PMUFW: v1.1 EL Level: EL2 Chip ID: zu7e WDT: Started with servicing (60s timeout) NAND: 0 MiB MMC: mmc@ff170000: 0 Loading Environment from FAT... *** Warning - bad CRC, using default environment

In: serial Out: serial Err: serial Bootmode: LVL_SHFT_SD_MODE1 Reset reason: EXTERNAL Net: ZYNQ GEM: ff0e0000, mdio bus ff0e0000, phyaddr 12, interface rgmii-id eth0: ethernet@ff0e0000 Hit any key to stop autoboot: 0 switch to partitions #0, OK mmc0 is current device Scanning mmc 0:1... libfdt fdt_check_header(): FDT_ERR_BADMAGIC Scanning disk mmc@ff170000.blk... Found 4 disks Loading Boot0000 'Fedora' failed EFI boot manager: Cannot load any image Found EFI removable media binary efi/boot/bootaa64.efi 858216 bytes read in 79 ms (10.4 MiB/s) libfdt fdt_check_header(): FDT_ERR_BADMAGIC Booting /efi\boot\bootaa64.efi System BootOrder not found. Initializing defaults. Could not read \EFI: Invalid Parameter Error: could not find boot options: Invalid Parameter start_image() returned Invalid Parameter ## Application failed, r = 2 EFI LOAD FAILED: continuing... JTAG: Trying to boot script at 0x20000000 ## Executing script at 20000000 Wrong image format for "source" command JTAG: SCRIPT FAILED: continuing... switch to partitions #0, OK mmc0 is current device Scanning mmc 0:1... libfdt fdt_check_header(): FDT_ERR_BADMAGIC Loading Boot0000 'Fedora' failed EFI boot manager: Cannot load any image Found EFI removable media binary efi/boot/bootaa64.efi 858216 bytes read in 78 ms (10.5 MiB/s) libfdt fdt_check_header(): FDT_ERR_BADMAGIC Booting /efi\boot\bootaa64.efi System BootOrder not found. Initializing defaults. Could not read \EFI: Invalid Parameter Error: could not find boot options: Invalid Parameter start_image() returned Invalid Parameter ## Application failed, r = 2 EFI LOAD FAILED: continuing...

when this patch is reverted I am getting

eth0: ethernet@ff0e0000 Hit any key to stop autoboot: 0 switch to partitions #0, OK mmc0 is current device Scanning mmc 0:1... libfdt fdt_check_header(): FDT_ERR_BADMAGIC Scanning disk mmc@ff170000.blk... Found 4 disks Loading Boot0000 'Fedora' failed EFI boot manager: Cannot load any image Found EFI removable media binary efi/boot/bootaa64.efi 858216 bytes read in 80 ms (10.2 MiB/s) libfdt fdt_check_header(): FDT_ERR_BADMAGIC Booting /efi\boot\bootaa64.efi System BootOrder not found. Initializing defaults.

ethernet@ff0e0000 Waiting for PHY auto negotiation to complete. done

  Fedora (5.12.0-0.rc6.20210407git2d743660786e.185.fc35.aarch64)

35 (Rawhide Prerelease) UEFI Firmware Settings

...

Do you know what's the problem here? Peter: Have you ever seen this behavior?

The following patch is already queued for my next pull request:

[Patch V2] efi_loader: loosen buffer parameter check in efi_file_read_int https://patchwork.ozlabs.org/project/uboot/patch/20210428135401.22365-1-peng...

Does it solve you problem?

Best regards

Heinrich

Thanks, Michal