[PATCH v2 1/3] efi_loader: Add SPI I/O protocol support
Heinrich Schuchardt
xypron.glpk at gmx.de
Sat Aug 13 21:20:46 CEST 2022
On 8/4/22 12:34, Paul Barker wrote:
> This addition allows UEFI applications running under u-boot to access
> peripherals on SPI busses. It is based on the UEFI Platform
> Initialization (PI) Specification, Version 1.7 Errata A (April 2020).
> Only the core functionality required to discover SPI peripherals and
> communicate with them is currently implemented. Other functionality such
> as the legacy SPI controller interface and the ability to update the SPI
> peripheral object associated with a particular SPI I/O protocol object
> is currently unimplemented.
>
> Since there are no open source implementations of this protocol to use
> as an example, educated guesses/hacks have been made in cases where the
> UEFI PI specification is unclear and these are documented in comments.
>
> This implementation has been tested on the SanCloud BBE Lite and allowed
> a UEFI test application to successfully communicate with a Micron
> Authenta flash device connected via the SPI bus. It has also been tested
> with the sandbox target using the included efi_selftest case.
The commit message should describe which protocols are actually implemented.
I still have no clue why any PI protocol should be implemented U-Boot.
You neither descibe it here nor in the cover-letter.
I would expect capsule updates to be used to update the SPI flash.
>
> Signed-off-by: Paul Barker <paul.barker at sancloud.com>
> ---
> MAINTAINERS | 7 +
> arch/sandbox/dts/test.dts | 8 +
> include/efi_loader.h | 4 +
> include/efi_spi_protocol.h | 158 +++++
> lib/efi_loader/Kconfig | 8 +
> lib/efi_loader/Makefile | 1 +
> lib/efi_loader/efi_setup.c | 6 +
> lib/efi_loader/efi_spi_protocol.c | 614 +++++++++++++++++++
> lib/efi_selftest/Makefile | 1 +
> lib/efi_selftest/efi_selftest_spi_protocol.c | 237 +++++++
> 10 files changed, 1044 insertions(+)
> create mode 100644 include/efi_spi_protocol.h
> create mode 100644 lib/efi_loader/efi_spi_protocol.c
> create mode 100644 lib/efi_selftest/efi_selftest_spi_protocol.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4d1930f76e44..1b5d9c37576b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -856,6 +856,13 @@ F: tools/efivar.py
> F: tools/file2include.c
> F: tools/mkeficapsule.c
>
> +EFI SPI SUPPORT
> +M: Paul Barker <paul.barker at sancloud.com>
> +S: Maintained
> +F: include/efi_spi_protocol.h
> +F: lib/efi_loader/efi_spi_protocol.c
> +F: lib/efi_selftest/efi_selftest_spi_protocol.c
> +
> EFI VARIABLES VIA OP-TEE
> M: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> S: Maintained
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index d1a8cc7bfb7e..5ac59140b748 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -1185,6 +1185,10 @@
> compatible = "spansion,m25p16", "jedec,spi-nor";
> spi-max-frequency = <40000000>;
> sandbox,filename = "spi.bin";
> +
> + uefi-spi-vendor = "spansion";
> + uefi-spi-part-number = "mt25p16";
> + uefi-spi-io-guid = [5deb81b8 92ad 484a 8fdd fa75a8e4c6b8];
> };
> spi.bin at 1 {
> reg = <1>;
> @@ -1193,6 +1197,10 @@
> sandbox,filename = "spi.bin";
> spi-cpol;
> spi-cpha;
> +
> + uefi-spi-vendor = "spansion";
Does the sandbox provide anything that is related to vendor spansion?
Or could the vendor be "sandbox"?
> + uefi-spi-part-number = "mt25p16";
Does the sandbox really emulate a
"Micron M25P16 Serial Flash Embedded Memory"?
> + uefi-spi-io-guid = [cd9eb3b6 1f2b 43a6 b8d7 3192d7cf7270];
This is a byte-string. The byte order in the array differs from the low
endian string representation used in the UEFI specification. To avoid
confusion, please, add a space after each byte and add a comment with
the string representation.
> };
> };
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 3a63a1f75fdf..2eb96b08205b 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -537,6 +537,10 @@ efi_status_t efi_rng_register(void);
> efi_status_t efi_tcg2_register(void);
> /* Called by efi_init_obj_list() to install RISCV_EFI_BOOT_PROTOCOL */
> efi_status_t efi_riscv_register(void);
> +/* Called by efi_init_obj_list() to install EFI_SPI_CONFIGURATION_PROTOCOL &
> + * EFI_SPI_IO_PROTOCOL
> + */
> +efi_status_t efi_spi_protocol_register(void);
> /* Called by efi_init_obj_list() to do initial measurement */
> efi_status_t efi_tcg2_do_initial_measurement(void);
> /* measure the pe-coff image, extend PCR and add Event Log */
> diff --git a/include/efi_spi_protocol.h b/include/efi_spi_protocol.h
> new file mode 100644
> index 000000000000..1a4456bd9349
> --- /dev/null
> +++ b/include/efi_spi_protocol.h
> @@ -0,0 +1,158 @@
> +/* SPDX-License-Identifier: BSD-2-Clause-Patent */
> +/*
> + * Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
Is this really a copy from EDK II and not based on the specification?
Please, add a reference to the
"UEFI Platform Initialization (PI) Specification".
and a description what this include is about.
> + */
> +
> +#ifndef _EFI_SPI_PROTOCOL_H
> +#define _EFI_SPI_PROTOCOL_H
> +
> +#include <efi.h>
> +#include <efi_api.h>
> +
> +#define EFI_SPI_CONFIGURATION_GUID \
> + EFI_GUID(0x85a6d3e6, 0xb65b, 0x4afc, \
> + 0xb3, 0x8f, 0xc6, 0xd5, 0x4a, 0xf6, 0xdd, 0xc8)
Please, add a comment that this GUID is defined in the "UEFI Platform
Initialization (PI) Specification".
Please, add the GUID to list_guid[]in lib/uuid.c .
> +
> +struct efi_spi_peripheral;
> +
> +struct efi_spi_part {
> + efi_string_t vendor;
> + efi_string_t part_number;
Why did you remove "const" used in the specifications?
> + u32 min_clock_hz;
> + u32 max_clock_hz;
> + bool chip_select_polarity;
> +};
> +
> +struct efi_spi_bus {
> + efi_string_t friendly_name;
> + struct efi_spi_peripheral *peripheral_list;
> + struct efi_device_path *controller_path;
> +
> + efi_status_t
> + (EFIAPI * clock)(const struct efi_spi_peripheral *spi_peripheral,
> + u32 *clock_hz);
> +
> + void *clock_parameter;
> +};
> +
> +struct efi_spi_peripheral {
> + struct efi_spi_peripheral *next_spi_peripheral;
> + efi_string_t friendly_name;
> + efi_guid_t *spi_peripheral_driver_guid;
> + struct efi_spi_part *spi_part;
> + u32 max_clock_hz;
> + bool clock_polarity;
> + bool clock_phase;
> + u32 attributes;
> + void *configuration_data;
> + struct efi_spi_bus *spi_bus;
> +
> + efi_status_t
> + (EFIAPI * chip_select)(const struct efi_spi_peripheral *spi_peripheral,
> + bool pin_value);
> +
> + void *chip_select_parameter;
> +};
> +
> +struct efi_spi_configuration_protocol {
> + u32 bus_count;
> + struct efi_spi_bus **bus_list;
> +};
> +
> +#define EFI_LEGACY_SPI_CONTROLLER_GUID \
> + EFI_GUID(0x39136fc7, 0x1a11, 0x49de, \
> + 0xbf, 0x35, 0x0e, 0x78, 0xdd, 0xb5, 0x24, 0xfc)
The commit message clear states that the legacy protocol will not be
implemented. Why should we implement any *legacy*?
> +
> +struct efi_legacy_spi_controller_protocol;
> +
> +struct efi_legacy_spi_controller_protocol {
> + u32 maximum_offset;
> + u32 maximum_range_bytes;
> + u32 range_register_count;
> +
> + efi_status_t
> + (EFIAPI * erase_block_opcode)(const struct efi_legacy_spi_controller_protocol *this,
> + u8 erase_block_opcode);
> +
> + efi_status_t
> + (EFIAPI * write_status_prefix)(const struct efi_legacy_spi_controller_protocol *this,
> + u8 write_status_prefix);
> +
> + efi_status_t
> + (EFIAPI * bios_base_address)(const struct efi_legacy_spi_controller_protocol *this,
> + u32 bios_base_address);
> +
> + efi_status_t
> + (EFIAPI * clear_spi_protect)(const struct efi_legacy_spi_controller_protocol *this);
> +
> + bool
> + (EFIAPI * is_range_protected)(const struct efi_legacy_spi_controller_protocol *this,
> + u32 bios_address,
> + u32 blocks_to_protect);
> +
> + efi_status_t
> + (EFIAPI * protect_next_range)(const struct efi_legacy_spi_controller_protocol *this,
> + u32 bios_address,
> + u32 blocks_to_protect);
> +
> + efi_status_t
> + (EFIAPI * lock_controller)(const struct efi_legacy_spi_controller_protocol *this);
> +};
> +
> +struct efi_spi_io_protocol;
> +
> +enum efi_spi_transaction_type {
> + SPI_TRANSACTION_FULL_DUPLEX,
> + SPI_TRANSACTION_WRITE_ONLY,
> + SPI_TRANSACTION_READ_ONLY,
> + SPI_TRANSACTION_WRITE_THEN_READ
> +};
> +
> +struct efi_spi_bus_transaction {
> + struct efi_spi_peripheral *spi_peripheral;
> + enum efi_spi_transaction_type transaction_type;
> + bool debug_transaction;
> + u32 bus_width;
> + u32 frame_size;
> + u32 write_bytes;
> + u8 *write_buffer;
> + u32 read_bytes;
> + u8 *read_buffer;
> +};
> +
> +struct efi_spi_io_protocol {
> + struct efi_spi_peripheral *spi_peripheral;
> + struct efi_spi_peripheral *original_spi_peripheral;
> + u32 frame_size_support_mask;
> + u32 maximum_transfer_bytes;
> + u32 attributes;
> + struct efi_legacy_spi_controller_protocol *legacy_spi_protocol;
> +
> + efi_status_t
> + (EFIAPI * transaction)(const struct efi_spi_io_protocol *this,
> + enum efi_spi_transaction_type transaction_type,
> + bool debug_transaction,
> + u32 clock_hz,
> + u32 bus_width,
> + u32 frame_size,
> + u32 write_bytes,
> + u8 *write_buffer,
> + u32 read_bytes,
> + u8 *read_buffer);
> +
> + efi_status_t
> + (EFIAPI * update_spi_peripheral)(struct efi_spi_io_protocol *this,
> + struct efi_spi_peripheral *spi_peripheral);
> +};
> +
> +struct efi_spi_peripheral_priv {
> + struct efi_spi_peripheral peripheral;
> + struct efi_spi_part part;
> + struct efi_spi_io_protocol io_protocol;
> + efi_handle_t handle;
> + struct spi_slave *target;
> +};
> +
> +efi_status_t efi_spi_protocol_register(void);
> +
> +#endif
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index e3f2402d0e8e..c5899a6f6e52 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -394,4 +394,12 @@ config EFI_RISCV_BOOT_PROTOCOL
> replace the transfer via the device-tree. The latter is not
> possible on systems using ACPI.
>
> +config EFI_SPI_PROTOCOL
> + bool "EFI SPI protocol support"
> + depends on DM_SPI
> + help
> + Provide implementations of EFI_SPI_CONFIGURATION_PROTOCOL and
> + EFI_SPI_IO_PROTOCOL to allow UEFI applications to access devices
> + connected via SPI bus.
> +
> endif
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index f54c244c3268..9fa0d27927b6 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -74,6 +74,7 @@ obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
> obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
> obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_tcg2.o
> obj-$(CONFIG_EFI_RISCV_BOOT_PROTOCOL) += efi_riscv.o
> +obj-$(CONFIG_EFI_SPI_PROTOCOL) += efi_spi_protocol.o
> obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
> obj-$(CONFIG_EFI_SIGNATURE_SUPPORT) += efi_signature.o
>
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 492ecf4cb15c..ef1ee9862b72 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -295,6 +295,12 @@ efi_status_t efi_init_obj_list(void)
> goto out;
> }
>
> + if (IS_ENABLED(CONFIG_EFI_SPI_PROTOCOL)) {
> + ret = efi_spi_protocol_register();
> + if (ret != EFI_SUCCESS)
> + goto out;
> + }
> +
> /* Secure boot */
> ret = efi_init_secure_boot();
> if (ret != EFI_SUCCESS)
> diff --git a/lib/efi_loader/efi_spi_protocol.c b/lib/efi_loader/efi_spi_protocol.c
> new file mode 100644
> index 000000000000..c491741cbbf1
> --- /dev/null
> +++ b/lib/efi_loader/efi_spi_protocol.c
> @@ -0,0 +1,614 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2022 Micron Technology, Inc.
> + */
> +
> +#define LOG_CATEGORY LOGC_EFI
> +
> +#include <common.h>
> +#include <dm/device.h>
> +#include <dm/device-internal.h>
> +#include <dm/read.h>
> +#include <efi.h>
> +#include <efi_loader.h>
> +#include <efi_spi_protocol.h>
> +#include <malloc.h>
> +#include <spi.h>
> +
> +static efi_string_t convert_efi_string(const char *str)
> +{
> + efi_string_t str_16, tmp;
> + size_t sz_8, sz_16;
> +
> + sz_8 = strlen(str);
> + sz_16 = utf8_utf16_strlen(str);
> + str_16 = calloc(sz_16 + 1, 2);
> + if (!str_16)
> + return NULL;
> +
> + tmp = str_16;
> + utf8_utf16_strcpy(&tmp, str);
> +
> + return str_16;
> +}
> +
> +static void dump_buffer(const char *msg, u32 length, u8 *buffer)
> +{
> + u32 i;
> + EFI_PRINT("%s %d bytes:", msg, length);
> + for (i = 0; i < length; i++)
> + EFI_PRINT(" %02x", buffer[i]);
> + EFI_PRINT("\n");
> +}
> +
> +static efi_status_t EFIAPI
> +efi_spi_bus_clock(const struct efi_spi_peripheral *spi_peripheral,
> + u32 *clock_hz)
> +{
> + EFI_ENTRY("%p, %p", spi_peripheral, clock_hz);
> + return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +static efi_status_t EFIAPI
> +efi_spi_peripheral_chip_select(const struct efi_spi_peripheral *spi_peripheral,
> + bool pin_value)
> +{
> + EFI_ENTRY("%p, %d", spi_peripheral, pin_value);
> + return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +static efi_status_t EFIAPI
> +legacy_erase_block_opcode(const struct efi_legacy_spi_controller_protocol *this,
> + u8 erase_block_opcode)
> +{
> + EFI_ENTRY("%p, %u", this, erase_block_opcode);
> + return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +static efi_status_t EFIAPI
> +legacy_write_status_prefix(const struct efi_legacy_spi_controller_protocol *this,
> + u8 write_status_prefix)
> +{
> + EFI_ENTRY("%p, %u", this, write_status_prefix);
> + return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +static efi_status_t EFIAPI
> +legacy_bios_base_address(const struct efi_legacy_spi_controller_protocol *this,
> + u32 bios_base_address)
> +{
> + EFI_ENTRY("%p, %u", this, bios_base_address);
> + return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +static efi_status_t EFIAPI
> +legacy_clear_spi_protect(const struct efi_legacy_spi_controller_protocol *this)
> +{
> + EFI_ENTRY("%p", this);
> + return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +static bool EFIAPI
> +legacy_is_range_protected(const struct efi_legacy_spi_controller_protocol *this,
> + u32 bios_address,
> + u32 blocks_to_protect)
> +{
> + EFI_ENTRY("%p, %u, %u", this, bios_address, blocks_to_protect);
> + return EFI_EXIT(false);
> +}
> +
> +static efi_status_t EFIAPI
> +legacy_protect_next_range(const struct efi_legacy_spi_controller_protocol *this,
> + u32 bios_address,
> + u32 blocks_to_protect)
> +{
> + EFI_ENTRY("%p, %u, %u", this, bios_address, blocks_to_protect);
> + return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +static efi_status_t EFIAPI
> +legacy_lock_controller(const struct efi_legacy_spi_controller_protocol *this)
> +{
> + EFI_ENTRY("%p", this);
> + return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +static efi_status_t EFIAPI
> +efi_spi_io_update_spi_peripheral(struct efi_spi_io_protocol *this,
> + struct efi_spi_peripheral *spi_peripheral)
> +{
> + EFI_ENTRY("%p, %p", this, spi_peripheral);
> + return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +static efi_status_t EFIAPI
> +efi_spi_io_transaction(const struct efi_spi_io_protocol *this,
> + enum efi_spi_transaction_type transaction_type,
> + bool debug_transaction,
> + u32 clock_hz,
> + u32 bus_width,
> + u32 frame_size,
> + u32 write_bytes,
> + u8 *write_buffer,
> + u32 read_bytes,
> + u8 *read_buffer)
> +{
> + struct spi_slave *target;
> + efi_status_t status = EFI_SUCCESS;
> + int r;
> +
> + /* We ignore the bus_width and frame_size arguments to this function as the
> + * appropriate bus configuration for the connected device will be performed
> + * during spi_claim_bus().
> + */
> +
> + /* TODO: Print transaction details if debug_transaction is true. */
> +
> + EFI_ENTRY("%p, %u, %u, %u, %u, %u, %u, %p, %u, %p",
> + this, transaction_type, debug_transaction,
> + clock_hz, bus_width, frame_size,
> + write_bytes, write_buffer, read_bytes, read_buffer);
> +
> + if (!this)
> + return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
> + target = container_of(this, struct efi_spi_peripheral_priv, io_protocol)->target;
> +
> + if (clock_hz > this->spi_peripheral->max_clock_hz)
> + return EFI_EXIT(EFI_UNSUPPORTED);
> +
> + r = spi_claim_bus(target);
> + if (r != 0)
> + return EFI_EXIT(EFI_DEVICE_ERROR);
> + EFI_PRINT("SPI IO: Bus claimed\n");
> +
> + if (clock_hz) {
> + EFI_PRINT("SPI IO: Setting clock rate to %u Hz\n", clock_hz);
> + spi_get_ops(target->dev->parent)->set_speed(target->dev->parent, clock_hz);
> + } else {
> + EFI_PRINT("SPI IO: Using default clock rate\n");
> + }
> +
> + switch (transaction_type) {
> + case SPI_TRANSACTION_FULL_DUPLEX:
> + EFI_PRINT("SPI IO: Full-duplex\n");
> + if (write_bytes != read_bytes || !write_bytes || !write_buffer || !read_buffer) {
> + status = EFI_INVALID_PARAMETER;
> + break;
> + }
> + if (debug_transaction)
> + dump_buffer("SPI IO: write", write_bytes, write_buffer);
> + r = spi_xfer(target, 8 * write_bytes,
> + write_buffer, read_buffer, SPI_XFER_ONCE);
> + EFI_PRINT("SPI IO: xfer returned %d\n", r);
> + if (debug_transaction)
> + dump_buffer("SPI IO: read", read_bytes, read_buffer);
> + status = (r == 0) ? EFI_SUCCESS : EFI_DEVICE_ERROR;
> + break;
> + case SPI_TRANSACTION_READ_ONLY:
> + EFI_PRINT("SPI IO: Read-only\n");
> + if (!read_bytes || !read_buffer) {
> + status = EFI_INVALID_PARAMETER;
> + break;
> + }
> + r = spi_xfer(target, 8 * read_bytes,
> + NULL, read_buffer, SPI_XFER_ONCE);
> + EFI_PRINT("SPI IO: xfer returned %d\n", r);
> + if (debug_transaction)
> + dump_buffer("SPI IO: read", read_bytes, read_buffer);
> + status = (r == 0) ? EFI_SUCCESS : EFI_DEVICE_ERROR;
> + break;
> + case SPI_TRANSACTION_WRITE_ONLY:
> + EFI_PRINT("SPI IO: Write-only\n");
> + if (!write_bytes || !write_buffer) {
> + status = EFI_INVALID_PARAMETER;
> + break;
> + }
> + if (debug_transaction)
> + dump_buffer("SPI IO: write", write_bytes, write_buffer);
> + r = spi_xfer(target, 8 * write_bytes,
> + write_buffer, NULL, SPI_XFER_ONCE);
> + EFI_PRINT("SPI IO: xfer returned %d\n", r);
> + status = (r == 0) ? EFI_SUCCESS : EFI_DEVICE_ERROR;
> + break;
> + case SPI_TRANSACTION_WRITE_THEN_READ:
> + EFI_PRINT("SPI IO: Write-then-read\n");
> + if (!write_bytes || !write_buffer || !read_bytes || !read_buffer) {
> + status = EFI_INVALID_PARAMETER;
> + break;
> + }
> + if (debug_transaction)
> + dump_buffer("SPI IO: write", write_bytes, write_buffer);
> + r = spi_xfer(target, 8 * write_bytes,
> + write_buffer, NULL, SPI_XFER_BEGIN);
> + EFI_PRINT("SPI IO: xfer [1/2] returned %d\n", r);
> + if (r != 0) {
> + status = EFI_DEVICE_ERROR;
> + break;
> + }
> + r = spi_xfer(target, 8 * read_bytes,
> + NULL, read_buffer, SPI_XFER_END);
> + EFI_PRINT("SPI IO: xfer [2/2] returned %d\n", r);
> + if (debug_transaction)
> + dump_buffer("SPI IO: read", read_bytes, read_buffer);
> + status = (r == 0) ? EFI_SUCCESS : EFI_DEVICE_ERROR;
> + break;
> + default:
> + status = EFI_INVALID_PARAMETER;
> + break;
> + }
> +
> + spi_release_bus(target);
> + EFI_PRINT("SPI IO: Released bus\n");
> + return EFI_EXIT(status);
> +}
> +
> +static struct efi_device_path null_device_path = {
> + .type = DEVICE_PATH_TYPE_END,
> + .sub_type = DEVICE_PATH_SUB_TYPE_END,
> + .length = 4
> +};
> +
> +static struct efi_legacy_spi_controller_protocol
> +dummy_legacy_spi_controller_protocol = {
> + .maximum_offset = 0,
> + .maximum_range_bytes = 0,
> + .range_register_count = 0,
> + .erase_block_opcode = legacy_erase_block_opcode,
> + .write_status_prefix = legacy_write_status_prefix,
> + .bios_base_address = legacy_bios_base_address,
> + .clear_spi_protect = legacy_clear_spi_protect,
> + .is_range_protected = legacy_is_range_protected,
> + .protect_next_range = legacy_protect_next_range,
> + .lock_controller = legacy_lock_controller
> +};
In the commit message you said that the legacy protocol will not be
implemented.
Why would a *legacy* be implemented?
> +
> +static efi_guid_t efi_spi_configuration_guid = EFI_SPI_CONFIGURATION_GUID;
> +
> +static void destroy_efi_spi_peripheral(struct efi_spi_peripheral *peripheral)
> +{
> + struct efi_spi_peripheral_priv *priv =
> + container_of(peripheral,
> + struct efi_spi_peripheral_priv,
> + peripheral);
> + free(priv->peripheral.friendly_name);
> + free(priv->part.vendor);
> + free(priv->part.part_number);
> + efi_delete_handle(priv->handle);
> + free(priv);
> +}
> +
> +static void destroy_efi_spi_bus(struct efi_spi_bus *bus)
> +{
> + struct efi_spi_peripheral *peripheral = bus->peripheral_list;
> +
> + while (peripheral) {
> + struct efi_spi_peripheral *next =
> + peripheral->next_spi_peripheral;
> + destroy_efi_spi_peripheral(peripheral);
> + peripheral = next;
> + }
> + free(bus->friendly_name);
> + free(bus);
> +}
> +
> +static efi_status_t efi_spi_new_handle(const efi_guid_t *guid, void *proto)
> +{
> + efi_status_t status;
> + efi_handle_t handle;
> +
> + status = efi_create_handle(&handle);
> + if (status != EFI_SUCCESS) {
> + printf("Failed to create EFI handle\n");
> + goto fail_1;
> + }
> +
> + status = efi_add_protocol(handle, guid, proto);
> + if (status != EFI_SUCCESS) {
> + printf("Failed to add protocol\n");
> + goto fail_2;
> + }
> +
> + return EFI_SUCCESS;
> +
> +fail_2:
> + efi_delete_handle(handle);
> +fail_1:
> + return status;
> +}
> +
> +static void
> +efi_spi_init_part(struct efi_spi_part *part,
> + struct spi_slave *target,
> + efi_string_t vendor_utf16,
> + efi_string_t part_number_utf16
> +)
> +{
> + part->vendor = vendor_utf16;
> + part->part_number = part_number_utf16;
> + part->min_clock_hz = 0;
> + part->max_clock_hz = target->max_hz;
> + part->chip_select_polarity =
> + (target->mode & SPI_CS_HIGH) ? true : false;
> +}
> +
> +static void
> +efi_spi_init_peripheral(struct efi_spi_peripheral *peripheral,
> + struct efi_spi_part *part,
> + struct efi_spi_bus *bus,
> + struct spi_slave *target,
> + efi_guid_t *guid,
> + efi_string_t name_utf16
> +)
> +{
> + peripheral->friendly_name = name_utf16;
> + peripheral->spi_part = part;
> + peripheral->next_spi_peripheral = NULL;
> + peripheral->spi_peripheral_driver_guid = guid;
> + peripheral->max_clock_hz = target->max_hz;
> + peripheral->clock_polarity = (target->mode & SPI_CPOL) ? true : false;
> + peripheral->clock_phase = (target->mode & SPI_CPHA) ? true : false;
> + peripheral->attributes = 0;
> + peripheral->configuration_data = NULL;
> + peripheral->spi_bus = bus;
> + peripheral->chip_select = efi_spi_peripheral_chip_select;
> + peripheral->chip_select_parameter = NULL;
> +}
> +
> +static void
> +efi_spi_append_peripheral(struct efi_spi_peripheral *peripheral,
> + struct efi_spi_bus *bus
> +)
> +{
> + if (bus->peripheral_list) {
> + struct efi_spi_peripheral *tmp = bus->peripheral_list;
> +
> + while (tmp->next_spi_peripheral)
> + tmp = tmp->next_spi_peripheral;
> +
> + tmp->next_spi_peripheral = peripheral;
> + } else {
> + bus->peripheral_list = peripheral;
> + }
> +}
> +
> +static void
> +efi_spi_init_io_protocol(struct efi_spi_io_protocol *io_protocol,
> + struct efi_spi_peripheral *peripheral,
> + struct spi_slave *target
> +)
> +{
> + u32 max_read, max_write;
> +
> + io_protocol->spi_peripheral = peripheral;
> + io_protocol->original_spi_peripheral = peripheral;
> + io_protocol->legacy_spi_protocol = &dummy_legacy_spi_controller_protocol;
> + io_protocol->transaction = efi_spi_io_transaction;
> + io_protocol->update_spi_peripheral = efi_spi_io_update_spi_peripheral;
> +
> + /* This is a bit of a hack. The EFI data structures do not allow us to
> + * represent a frame size greater than 32 bits.
> + */
> + if (target->wordlen <= 32)
> + io_protocol->frame_size_support_mask =
> + 1 << (target->wordlen - 1);
> + else
> + io_protocol->frame_size_support_mask = 0;
> +
> + /* Again, this is a bit of a hack. The EFI data structures only allow
> + * for a single maximum transfer size whereas the u-boot spi_slave
> + * structure records maximum read transfer size and maximum write
> + * transfer size separately. So we need to take the minimum of these two
> + * values.
> + *
> + * In u-boot, a value of zero for these fields means there is no limit
> + * on the transfer size. However in the UEFI PI spec a value of zero is
> + * invalid so we'll use 0xFFFFFFFF as a substitute unlimited value.
> + */
> + max_write = target->max_write_size ? target->max_write_size : 0xFFFFFFFF;
> + max_read = target->max_read_size ? target->max_read_size : 0xFFFFFFFF;
> + io_protocol->maximum_transfer_bytes = (max_read > max_write) ? max_write : max_read;
> +
> + /* Hack++. Leave attributes set to zero since the flags listed in the
> + * UEFI PI spec have no defined numerical values and so cannot be used.
> + */
> + io_protocol->attributes = 0;
> +}
> +
> +static efi_status_t
> +export_spi_peripheral(struct efi_spi_bus *bus, struct udevice *dev)
> +{
> + efi_string_t name_utf16, vendor_utf16, part_number_utf16;
> + struct efi_spi_peripheral_priv *priv;
> + efi_status_t status;
> + struct udevice *dev_bus = dev->parent;
> + struct spi_slave *target;
> + const char *name = dev_read_name(dev);
> + const char *vendor = dev_read_string(dev, "uefi-spi-vendor");
> + const char *part_number = dev_read_string(dev, "uefi-spi-part-number");
> + efi_guid_t *guid =
> + (efi_guid_t *)dev_read_u8_array_ptr(dev, "uefi-spi-io-guid", 16);
> +
> + if (device_get_uclass_id(dev) == UCLASS_SPI_EMUL) {
> + debug("Skipping emulated SPI peripheral %s\n", name);
> + goto fail_1;
> + }
> +
> + if (!vendor || !part_number || !guid) {
> + debug("Skipping SPI peripheral %s\n", name);
> + status = EFI_UNSUPPORTED;
> + goto fail_1;
> + }
> +
> + if (!device_active(dev)) {
> + int ret = device_probe(dev);
> + if (ret) {
> + debug("Skipping SPI peripheral %s, probe failed\n", name);
> + goto fail_1;
> + }
> + }
> +
> + target = dev_get_parent_priv(dev);
> + if (!target) {
> + debug("Skipping uninitialized SPI peripheral %s\n", name);
> + status = EFI_UNSUPPORTED;
> + goto fail_1;
> + }
> +
> + debug("Registering SPI dev %d:%d, name %s\n",
> + dev_bus->seq_, spi_chip_select(dev), name);
> +
> + priv = calloc(1, sizeof(*priv));
> + if (!priv) {
> + status = EFI_OUT_OF_RESOURCES;
> + goto fail_1;
> + }
> +
> + vendor_utf16 = convert_efi_string(vendor);
> + if (!vendor_utf16) {
> + status = EFI_OUT_OF_RESOURCES;
> + goto fail_2;
> + }
> +
> + part_number_utf16 = convert_efi_string(part_number);
> + if (!part_number_utf16) {
> + status = EFI_OUT_OF_RESOURCES;
> + goto fail_3;
> + }
> +
> + name_utf16 = convert_efi_string(name);
> + if (!name_utf16) {
> + status = EFI_OUT_OF_RESOURCES;
> + goto fail_4;
> + }
> +
> + priv->target = target;
> +
> + efi_spi_init_part(&priv->part, target, vendor_utf16, part_number_utf16);
> +
> + efi_spi_init_peripheral(&priv->peripheral, &priv->part,
> + bus, target, guid, name_utf16);
> +
> + efi_spi_append_peripheral(&priv->peripheral, bus);
> +
> + efi_spi_init_io_protocol(&priv->io_protocol, &priv->peripheral, target);
> +
> + status = efi_spi_new_handle(guid, &priv->io_protocol);
> + if (status != EFI_SUCCESS)
> + goto fail_5;
> +
> + printf("Added EFI_SPI_IO_PROTOCOL for %s with guid "
> + "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x\n",
This information does not interest any user. Please, use log_debug().
> + name,
> + guid->b[0], guid->b[1], guid->b[2], guid->b[3],
> + guid->b[4], guid->b[5], guid->b[6], guid->b[7],
> + guid->b[8], guid->b[9], guid->b[10], guid->b[11],
> + guid->b[12], guid->b[13], guid->b[14], guid->b[15]);
> + return EFI_SUCCESS;
> +
> +fail_5:
> + free(name_utf16);
> +fail_4:
> + free(part_number_utf16);
> +fail_3:
> + free(vendor_utf16);
> +fail_2:
> + free(priv);
> +fail_1:
> + return status;
> +}
> +
> +static struct efi_spi_bus *export_spi_bus(int i)
> +{
> + struct efi_spi_bus *bus;
> + struct udevice *dev, *child;
> + const char *name;
> + int r;
> +
> + r = uclass_get_device(UCLASS_SPI, i, &dev);
> + if (r < 0) {
> + printf("Failed to get SPI bus %d\n", i);
> + goto fail_1;
> + }
> +
> + name = dev_read_name(dev);
> + debug("Registering SPI bus %d, name %s\n", i, name);
> +
> + bus = calloc(1, sizeof(*bus));
> + if (!bus)
> + goto fail_1;
> +
> + bus->friendly_name = convert_efi_string(name);
> + if (!bus->friendly_name)
> + goto fail_2;
> +
> + bus->peripheral_list = NULL;
> + bus->clock = efi_spi_bus_clock;
> + bus->clock_parameter = NULL;
> +
> + /* For the purposes of the current implementation, we do not need to expose
> + * the hardware device path to users of the SPI I/O protocol.
> + */
> + bus->controller_path = &null_device_path;
> +
> + device_foreach_child(child, dev) {
> + efi_status_t status = export_spi_peripheral(bus, child);
> +
> + if (status == EFI_OUT_OF_RESOURCES)
> + goto fail_3;
> + }
> +
> + return bus;
> +
> +fail_3:
> + destroy_efi_spi_bus(bus);
> +fail_2:
> + free(bus);
> +fail_1:
> + return NULL;
> +}
> +
> +efi_status_t efi_spi_protocol_register(void)
> +{
> + efi_status_t status;
> + struct efi_spi_configuration_protocol *proto;
> + uint i;
> +
> + printf("Registering EFI_SPI_CONFIGURATION_PROTOCOL\n");
> +
> + proto = calloc(1, sizeof(*proto));
> + if (!proto) {
> + status = EFI_OUT_OF_RESOURCES;
> + goto fail_1;
> + }
> +
> + proto->bus_count = uclass_id_count(UCLASS_SPI);
> + proto->bus_list = calloc(proto->bus_count, sizeof(*proto->bus_list));
> + if (!proto->bus_list) {
> + status = EFI_OUT_OF_RESOURCES;
> + goto fail_2;
> + }
> +
> + for (i = 0; i < proto->bus_count; i++) {
> + proto->bus_list[i] = export_spi_bus(i);
> + if (!proto->bus_list[i])
> + goto fail_3;
> + }
> +
> + status = efi_spi_new_handle(&efi_spi_configuration_guid, proto);
> + if (status != EFI_SUCCESS)
> + goto fail_3;
> +
> + return EFI_SUCCESS;
> +
> +fail_3:
> + for (i = 0; i < proto->bus_count; i++) {
> + if (proto->bus_list[i])
> + destroy_efi_spi_bus(proto->bus_list[i]);
> + }
> + free(proto->bus_list);
> +fail_2:
> + free(proto);
> +fail_1:
> + return status;
> +}
> diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
> index 33536c9ec021..c9b6e4fbb452 100644
> --- a/lib/efi_selftest/Makefile
> +++ b/lib/efi_selftest/Makefile
> @@ -62,6 +62,7 @@ obj-$(CONFIG_EFI_LOADER_HII) += efi_selftest_hii.o
> obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_selftest_rng.o
> obj-$(CONFIG_EFI_GET_TIME) += efi_selftest_rtc.o
> obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_selftest_tcg2.o
> +obj-$(CONFIG_EFI_SPI_PROTOCOL) += efi_selftest_spi_protocol.o
>
> ifeq ($(CONFIG_GENERATE_ACPI_TABLE),)
> obj-y += efi_selftest_fdt.o
> diff --git a/lib/efi_selftest/efi_selftest_spi_protocol.c b/lib/efi_selftest/efi_selftest_spi_protocol.c
> new file mode 100644
> index 000000000000..2a3723d3931f
> --- /dev/null
> +++ b/lib/efi_selftest/efi_selftest_spi_protocol.c
> @@ -0,0 +1,237 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2022 Micron Technology, Inc.
> + */
> +
> +#include <efi_selftest.h>
> +#include <efi_spi_protocol.h>
> +
> +static struct efi_boot_services *boottime;
> +static efi_guid_t efi_spi_configuration_guid = EFI_SPI_CONFIGURATION_GUID;
> +
> +static int setup(const efi_handle_t img_handle,
> + const struct efi_system_table *systable)
> +{
> + boottime = systable->boottime;
> + return EFI_ST_SUCCESS;
> +}
> +
> +static int test_peripheral(struct efi_spi_peripheral *p, struct efi_spi_bus *bus)
> +{
> + struct efi_spi_io_protocol *io_protocol;
> + u8 req[5], resp[5];
> + efi_status_t ret;
> +
> + if (!p->friendly_name) {
> + efi_st_error("SPI peripheral lacks a friendly name\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!p->spi_peripheral_driver_guid) {
> + efi_st_error("SPI peripheral lacks driver GUID\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!p->spi_part) {
> + efi_st_error("SPI peripheral lacks SPI part definition\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!p->max_clock_hz) {
> + efi_st_error("SPI peripheral has a max clock rate of zero\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!p->spi_bus) {
> + efi_st_error("SPI peripheral lack pointer to SPI bus\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (p->spi_bus != bus) {
> + efi_st_error("SPI peripheral spi_bus pointer points to the wrong bus\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!p->chip_select) {
> + efi_st_error("SPI peripheral lacks chip_select function\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!p->spi_part->vendor) {
> + efi_st_error("SPI part lacks vendor string\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!p->spi_part->part_number) {
> + efi_st_error("SPI part lacks part number string\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (p->spi_part->min_clock_hz > p->spi_part->max_clock_hz) {
> + efi_st_error("SPI part min clock rate is greater than max clock rate\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (p->spi_part->max_clock_hz != p->max_clock_hz) {
> + efi_st_error("SPI part max clock rate does not match peripheral max clock rate\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + ret = boottime->locate_protocol(p->spi_peripheral_driver_guid,
> + NULL, (void **)&io_protocol);
> + if (ret != EFI_SUCCESS) {
> + efi_st_error("SPI IO protocol not available\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (io_protocol->spi_peripheral != p) {
> + efi_st_error("SPI IO protocol spi_peripheral pointer points to the wrong peripheral\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (io_protocol->original_spi_peripheral != p) {
> + efi_st_error("SPI IO protocol original_spi_peripheral pointer points to the wrong peripheral\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!io_protocol->maximum_transfer_bytes) {
> + efi_st_error("SPI IO protocol has zero maximum transfer size\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!io_protocol->legacy_spi_protocol) {
> + efi_st_error("SPI IO protocol lacks legacy SPI protocol\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!io_protocol->transaction) {
> + efi_st_error("SPI IO protocol lacks transaction function\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!io_protocol->update_spi_peripheral) {
> + efi_st_error("SPI IO protocol lacks update_spi_peripheral function\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!io_protocol->legacy_spi_protocol->erase_block_opcode) {
> + efi_st_error("SPI legacy controller protocol lacks erase_block_opcode function\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!io_protocol->legacy_spi_protocol->write_status_prefix) {
> + efi_st_error("SPI legacy controller protocol lacks write_status_prefix function\n");
> + return EFI_ST_FAILURE;
> + }
I would have expected that you actually call each of the functions.
> +
> + if (!io_protocol->legacy_spi_protocol->bios_base_address) {
> + efi_st_error("SPI legacy controller protocol lacks bios_base_address function\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!io_protocol->legacy_spi_protocol->clear_spi_protect) {
> + efi_st_error("SPI legacy controller protocol lacks clear_spi_protect function\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!io_protocol->legacy_spi_protocol->is_range_protected) {
> + efi_st_error("SPI legacy controller protocol lacks is_range_protected function\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!io_protocol->legacy_spi_protocol->protect_next_range) {
> + efi_st_error("SPI legacy controller protocol lacks protect_next_range function\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!io_protocol->legacy_spi_protocol->lock_controller) {
> + efi_st_error("SPI legacy controller protocol lacks lock_controller function\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + req[0] = 0x9f;
> + ret = io_protocol->transaction(io_protocol,
> + SPI_TRANSACTION_FULL_DUPLEX,
> + false, 0, 1, 8,
> + sizeof(req), req,
> + sizeof(resp), resp);
> + if (ret != EFI_SUCCESS) {
> + efi_st_error("SPI transaction failed\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if ((resp[0] != 0xff) || (resp[1] != 0x20) || (resp[2] != 0x20) || (resp[3] != 0x15)) {
> + efi_st_error("Incorrect response from sandbox SPI flash emulator\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + return EFI_ST_SUCCESS;
> +}
> +
> +static int test_bus(struct efi_spi_bus *bus)
> +{
> + struct efi_spi_peripheral *p;
> + int status;
> +
> + if (!bus->friendly_name) {
> + efi_st_error("SPI bus lacks a friendly name\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!bus->peripheral_list) {
> + efi_st_error("SPI bus has zero peripherals\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!bus->clock) {
> + efi_st_error("SPI bus lacks clock function\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + for (p = bus->peripheral_list; p; p = p->next_spi_peripheral) {
> + status = test_peripheral(p, bus);
> + if (status) {
> + efi_st_error("Failed testing SPI peripheral\n");
> + return status;
> + }
> + }
> +
> + return EFI_ST_SUCCESS;
> +}
> +
> +static int execute(void)
> +{
> + struct efi_spi_configuration_protocol *spi;
> + efi_status_t ret;
> + int status;
> + u32 i;
> +
> + ret = boottime->locate_protocol(&efi_spi_configuration_guid,
> + NULL, (void **)&spi);
> + if (ret != EFI_SUCCESS) {
> + efi_st_error("SPI configuration protocol not available\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + if (!spi->bus_count) {
> + efi_st_error("SPI configuration protocol has zero busses\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + for (i = 0; i < spi->bus_count; i++) {
> + status = test_bus(spi->bus_list[i]);
> + if (status) {
> + efi_st_error("Failed testing SPI bus %d\n", i);
> + return status;
> + }
> + }
This test does not test anything changing any state of SPI flash. Why is
this patch needed if that is all that is usable?
Best regards
Heinrich
> +
> + return EFI_ST_SUCCESS;
> +}
> +
> +EFI_UNIT_TEST(spi_protocol) = {
> + .name = "SPI protocol",
> + .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
> + .setup = setup,
> + .execute = execute,
> +};
More information about the U-Boot
mailing list