All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] efi: SPI NOR flash support
@ 2021-02-05  8:58 Michael Lawnick
  2021-02-05  9:02 ` [PATCH v2 2/2] efi: SPI NOR flash command line Michael Lawnick
  2021-02-11  7:36 ` [PATCH v2 1/2] efi: SPI NOR flash support Michael Lawnick
  0 siblings, 2 replies; 12+ messages in thread
From: Michael Lawnick @ 2021-02-05  8:58 UTC (permalink / raw)
  To: The development of GNU GRUB

Add EFI SPI NOR driver

Use UEFI interface for accessing SPI NOR flashes.
If supported the implementation of UEFI boot software abstracts
away all those ugly H/W details like SPI controller or protocol.
Provided functions:
grub_efi_spi_nor_
	init
	erase
	write
	read
	flash_size
	flash_id
	erase_block_size

This driver might be used for further abstraction to a common
(SPI) flash interface.

Signed-off-by: Michael Lawnick <michael.lawnick@nokia.com>
---
[Patch v2 1/2] : fix flaw in EFI header, wrong sequence of methods.
---
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 68b9e9f68..4d775e5f6 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -446,7 +446,7 @@ image = {
    i386_pc = boot/i386/pc/boot.S;

    cppflags = '-DHYBRID_BOOT=1';
-
+
    i386_pc_ldflags = '$(TARGET_IMG_LDFLAGS)';
    i386_pc_ldflags = '$(TARGET_IMG_BASE_LDOPT),0x7C00';

@@ -656,6 +656,12 @@ module = {
    enable = i386_multiboot;
  };

+module = {
+  name = efi_spi_nor;
+  common = bus/spi/efi_spi_nor.c;
+  enable = efi;
+};
+
  module = {
    name = nativedisk;
    common = commands/nativedisk.c;
diff --git a/grub-core/bus/spi/efi_spi_nor.c
b/grub-core/bus/spi/efi_spi_nor.c
new file mode 100644
index 000000000..0e073b436
--- /dev/null
+++ b/grub-core/bus/spi/efi_spi_nor.c
@@ -0,0 +1,298 @@
+/*  efi_spi_nor.c  - Give access to SPI NOR flash through UEFI interface.
+ *  Copyright 2021 Nokia
+ *  Licensed under the GNU General Public License v3.0 only
+ *  SPDX-License-Identifier: GPL-3.0-only
+ *
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2008  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <grub/command.h>
+#include <grub/misc.h>
+#include <grub/mm.h>
+#include <grub/types.h>
+#include <grub/efi/api.h>
+#include <grub/efi/efi.h>
+#include <grub/efi/efi_spi_nor.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+#define EFI_SPI_NOR_FLASH_PROTOCOL_GUID \
+	{ 0xb57ec3fe, 0xf833, 0x4ba6, \
+		{0x85, 0x78, 0x2a, 0x7d, 0x6a, 0x87, 0x44, 0x4b} \
+	}
+
+#define EFI_FLASHID_LEN 3
+
+struct efi_spi_nor_flash_protocol {
+	struct spi_nor	*spi_peripheral;
+	grub_efi_uint32_t	flash_size;
+	grub_efi_uint8_t	device_id[EFI_FLASHID_LEN];
+	grub_efi_uint32_t	erase_block_size;
+
+	grub_efi_status_t (* get_flash_id)(struct efi_spi_nor_flash_protocol
*this,
+					     grub_uint8_t *buffer);
+	grub_efi_status_t (* read_data)(struct efi_spi_nor_flash_protocol *this,
+					  grub_uint32_t offset, grub_uint32_t len, grub_uint8_t *data);
+	grub_efi_status_t (* lf_read_data)(struct efi_spi_nor_flash_protocol
*this,
+					     grub_uint32_t offset, grub_uint32_t len, grub_uint8_t *data);
+	grub_efi_status_t (* read_status)(struct efi_spi_nor_flash_protocol *this,
+					    grub_uint32_t num_bytes, grub_uint8_t *status);
+	grub_efi_status_t (* write_status)(struct efi_spi_nor_flash_protocol
*this,
+					     grub_uint32_t num_bytes, grub_uint8_t *status);
+	grub_efi_status_t (* write_data)(struct efi_spi_nor_flash_protocol *this,
+					   grub_uint32_t offset, grub_uint32_t len, grub_uint8_t *data);
+	grub_efi_status_t (* erase_blocks)(struct efi_spi_nor_flash_protocol
*this,
+					     grub_uint32_t offset, grub_uint32_t blk_count);
+};
+
+/* grub_efi_spi_nor_init - initialize access to SPI NOR flash device
+ *
+ * Search pool of SPI NOR flash devices known to underlying EFI bootware.
+ * Use <flash_id> and <instance> to filter out devices.
+ *
+ * IN: flash_id     - optional, pointer to max 3 bytes
(EFI_FLASHID_LEN) to match against
+ *                    SPI flash JEDEC ID, use NULL if no filtering.
+ * IN: num_id_bytes - number of bytes in flash_id. Maximum 3 bytes
+ *                    are used for comparison.
+ * IN: instance     - number of device occurances to skip
+ *
+ * returns : pointer to flash device or NULL on failure
+ */
+void *
+grub_efi_spi_nor_init(grub_uint8_t *flash_id, grub_uint32_t
num_id_bytes, grub_uint32_t instance)
+{
+	grub_efi_guid_t efi_guid_spi_nor_flash_protocol =
EFI_SPI_NOR_FLASH_PROTOCOL_GUID;
+	grub_efi_status_t ret;
+	grub_efi_uintn_t num_handles;
+	grub_efi_handle_t *handles;
+	grub_uint8_t found_id[EFI_FLASHID_LEN];
+	grub_uint32_t idx, match_cnt=0;
+	struct efi_spi_nor_flash_protocol *spi_nor;
+
+	handles = grub_efi_locate_handle(GRUB_EFI_BY_PROTOCOL,
+				&efi_guid_spi_nor_flash_protocol, 0,
+				&num_handles);
+	grub_boot_time("found %ld SPI NOR flash devices\n", num_handles);
+
+	if ((num_handles == 0) || (num_handles < instance + 1))
+		return NULL;
+
+	for (idx = 0; idx < num_handles; idx++) {
+		spi_nor = grub_efi_open_protocol(handles[idx],
+						&efi_guid_spi_nor_flash_protocol,
+						GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+		if (! spi_nor) {
+			grub_error(GRUB_ERR_UNKNOWN_DEVICE, "Failed to open device protocol\n");
+			grub_free(handles);
+			return NULL;
+		}
+
+		ret = spi_nor->get_flash_id(spi_nor, found_id);
+		if (ret != GRUB_EFI_SUCCESS) {
+			grub_error(GRUB_ERR_READ_ERROR, "Failed to read flash_id\n");
+			grub_free(handles);
+			return NULL;
+		}
+
+		/* if caller requests filtering by id */
+		if (flash_id != NULL) {
+			grub_uint32_t id;
+
+			if (num_id_bytes > EFI_FLASHID_LEN)
+				num_id_bytes = EFI_FLASHID_LEN;
+
+			for (id = 0; id < num_id_bytes; id++)
+				if (flash_id[id] != found_id[id])
+					break;
+
+			if (id != num_id_bytes)
+				continue;
+		}
+
+		if (match_cnt < instance) {
+			match_cnt++;
+			continue;
+		}
+
+		grub_boot_time("Found flash with ID 0x%02x 0x%02x 0x%02x\n",
+						found_id[0], found_id[1], found_id[2]);
+
+		grub_free(handles);
+		return spi_nor;
+	}
+
+	grub_free(handles);
+	return NULL;
+}
+
+/* grub_efi_spi_nor_flash_size - get memory size of SPI NOR flash device
+ *
+ * IN: efi_flash_dev - device identifier returned by grub_efi_spi_nor_init.
+ *
+ * returns : memory size of flash device or 0 on failure
+ */
+grub_uint32_t
+grub_efi_spi_nor_flash_size(void *efi_flash_dev)
+{
+	struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
+
+	if (spi_nor == NULL)
+		return 0;
+
+	return spi_nor->flash_size;
+}
+
+/* grub_efi_spi_nor_device_id - get three byte JEDEC ID of SPI NOR
flash device
+ *
+ * IN: efi_flash_dev - device identifier returned by grub_efi_spi_nor_init.
+ *
+ * returns : three byte JEDEC ID as a uint32 or 0 on failure
+ */
+grub_uint32_t
+grub_efi_spi_nor_device_id(void *efi_flash_dev)
+{
+	struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
+	grub_uint32_t devId = 0;
+	grub_efi_status_t ret;
+	grub_uint8_t device_id[3];
+	int i;
+
+	if (spi_nor == NULL)
+		return 0;
+
+	ret = spi_nor->get_flash_id(spi_nor, device_id);
+	if (ret != GRUB_EFI_SUCCESS)
+		return 0;
+
+	for(i=0; i<3;i++)
+		devId = (devId<<8) + device_id[i];
+
+	return devId;
+}
+
+/* grub_efi_spi_nor_erase_block_size - get erase block size of SPI NOR
flash device
+ *
+ * Parameters for calls to grub_efi_spi_nor_erase() need to be erase
block size
+ * aligned.
+ *
+ * IN: efi_flash_dev - device identifier returned by grub_efi_spi_nor_init.
+ *
+ * returns : size of erase block of flash device or 0 on failure
+ */
+grub_uint32_t
+grub_efi_spi_nor_erase_block_size(void *efi_flash_dev)
+{
+	struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
+
+	if (spi_nor == NULL)
+		return 0;
+
+	return spi_nor->erase_block_size;
+}
+
+/* grub_efi_spi_nor_read - read from SPI NOR flash device
+ *
+ * IN:  efi_flash_dev - device identifier returned by
grub_efi_spi_nor_init.
+ * OUT: buffer        - pointer to an preallocated buffer of min. bytes
size
+ * IN:  offset        - starting point where to start to read from
+ * IN:  bytes         - number of bytes to read from flash and write to
buffer
+ *
+ * returns : GRUB_EFI_SUCCESS if all ok
+ *           GRUB_EFI_INVALID_PARAMETER if an argument is bad
+ *
+ */
+grub_err_t
+grub_efi_spi_nor_read(void *efi_flash_dev, grub_uint8_t *buffer,
grub_uint32_t offset, grub_uint32_t bytes)
+{
+	struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
+	grub_efi_status_t ret;
+
+	if (spi_nor == NULL || buffer == NULL)
+		return GRUB_ERR_BAD_ARGUMENT;
+
+	ret = (spi_nor->read_data(spi_nor, offset, bytes, buffer));
+	if (ret != GRUB_EFI_SUCCESS)
+		return grub_error(GRUB_ERR_READ_ERROR, "Failed to read data @0x%x\n",
offset);
+
+	return GRUB_ERR_NONE;
+}
+
+/* grub_efi_spi_nor_write - write to SPI NOR flash device
+ *
+ * IN: efi_flash_dev - device identifier returned by grub_efi_spi_nor_init.
+ * IN: buffer        - pointer to buffer containig min. bytes size data
to write
+ * IN: offset        - starting point where to start to write to
+ * IN: bytes         - number of bytes to write to buffer
+ *
+ * returns : GRUB_EFI_SUCCESS if all ok
+ *           GRUB_EFI_INVALID_PARAMETER if an argument is bad
+ *
+ */
+grub_err_t
+grub_efi_spi_nor_write(void *efi_flash_dev, grub_uint8_t *buffer,
grub_uint32_t offset, grub_uint32_t bytes)
+{
+	struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
+	grub_efi_status_t ret;
+
+	if (spi_nor == NULL || buffer == NULL)
+		return GRUB_ERR_BAD_ARGUMENT;
+
+	ret = (spi_nor->write_data(spi_nor, offset, bytes, buffer));
+	if (ret != GRUB_EFI_SUCCESS)
+		return grub_error(GRUB_ERR_WRITE_ERROR, "Failed to write data
@0x%x\n", offset);
+
+	return GRUB_ERR_NONE;
+}
+
+/* grub_efi_spi_nor_erase - erase sectors on SPI NOR flash device
+ *
+ * Parameters offset and bytes need to be erase sector aligned, i.e.
+ * multiples of the size returned by function
grub_efi_spi_nor_erase_block_size()
+ *
+ * IN: efi_flash_dev - device identifier returned by grub_efi_spi_nor_init.
+ * IN: offset        - offset of first sector to erase
+ * IN: bytes         - length of region to erase
+ *
+ * returns : GRUB_EFI_SUCCESS if all ok
+ *           GRUB_EFI_INVALID_PARAMETER if an argument is bad
+ *
+ */
+grub_err_t
+grub_efi_spi_nor_erase(void *efi_flash_dev, grub_uint32_t offset,
grub_uint32_t bytes)
+{
+	struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
+	grub_efi_status_t ret;
+	grub_uint32_t sect_sz;
+	grub_uint32_t sect_mask;
+
+	if (spi_nor == NULL || bytes == 0)
+		return GRUB_ERR_BAD_ARGUMENT;
+
+	sect_sz = grub_efi_spi_nor_erase_block_size(spi_nor);
+	sect_mask = sect_sz - 1;
+	if ((offset & sect_mask) != 0)
+		return grub_error(GRUB_ERR_BAD_ARGUMENT, "SPI NOR erase offset not at
sector boundary");
+
+	if (((offset + bytes) & sect_mask) != 0)
+		return grub_error(GRUB_ERR_BAD_ARGUMENT, "SPI NOR erase end not at
sector boundary");
+
+	ret = spi_nor->erase_blocks(spi_nor, offset, (bytes-1) / sect_sz + 1);
+
+	if (ret != GRUB_EFI_SUCCESS)
+		return grub_error(GRUB_ERR_WRITE_ERROR, "SPI NOR erase operation
failed");
+
+	return GRUB_ERR_NONE;
+}
diff --git a/include/grub/efi/efi_spi_nor.h b/include/grub/efi/efi_spi_nor.h
new file mode 100644
index 000000000..d09f0c9b7
--- /dev/null
+++ b/include/grub/efi/efi_spi_nor.h
@@ -0,0 +1,37 @@
+/*  efi_spi_nor.h  - Give access to SPI NOR flash through UEFI interface.
+ *  Copyright 2021 Nokia
+ *  Licensed under the GNU General Public License v3.0 only
+ *  SPDX-License-Identifier: GPL-3.0-only
+ *
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2008  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef GRUB_EFI_SPI_NOR_HEADER
+#define GRUB_EFI_SPI_NOR_HEADER
+
+#include <grub/types.h>
+
+void *grub_efi_spi_nor_init(grub_uint8_t *flash_id, grub_uint32_t
num_id_bytes, grub_uint32_t instance);
+
+grub_uint32_t grub_efi_spi_nor_flash_size(void *efi_flash_dev);
+grub_uint32_t grub_efi_spi_nor_device_id(void *efi_flash_dev);
+grub_uint32_t grub_efi_spi_nor_erase_block_size(void *efi_flash_dev);
+
+grub_err_t grub_efi_spi_nor_read(void *efi_flash_dev, grub_uint8_t
*buffer, grub_uint32_t offset, grub_uint32_t bytes);
+grub_err_t grub_efi_spi_nor_write(void *efi_flash_dev, grub_uint8_t
*buffer, grub_uint32_t offset, grub_uint32_t bytes);
+grub_err_t grub_efi_spi_nor_erase(void *efi_flash_dev, grub_uint32_t
offset, grub_uint32_t bytes);
+
+#endif /*GRUB_EFISPINOR*/


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/2] efi: SPI NOR flash command line
  2021-02-05  8:58 [PATCH v2 1/2] efi: SPI NOR flash support Michael Lawnick
@ 2021-02-05  9:02 ` Michael Lawnick
  2021-02-12  9:20   ` Michael Lawnick
  2021-02-11  7:36 ` [PATCH v2 1/2] efi: SPI NOR flash support Michael Lawnick
  1 sibling, 1 reply; 12+ messages in thread
From: Michael Lawnick @ 2021-02-05  9:02 UTC (permalink / raw)
  To: The development of GNU GRUB

Add SPI NOR flash to command line
Based on patch '[PATCH 1/2] efi: SPI NOR flash support'
add command line functionality for interactive access
to SPI NOR flash.

Supported commands:
spi_nor
      init  - establish communication to a flash part
      read  - read from flash part to memory or print hexdump
      write - write to flash part from memory
      erase - erase some erase blocks

Signed-off-by: Michael Lawnick <michael.lawnick@nokia.com>
---
[Patch v2 2/2]: no change
---
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 4d775e5f6..403a5432f 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -659,6 +659,7 @@ module = {
  module = {
    name = efi_spi_nor;
    common = bus/spi/efi_spi_nor.c;
+  common = commands/efi/spinorcmd.c;
    enable = efi;
  };

diff --git a/grub-core/commands/efi/spinorcmd.c
b/grub-core/commands/efi/spinorcmd.c
new file mode 100644
index 000000000..c55a900aa
--- /dev/null
+++ b/grub-core/commands/efi/spinorcmd.c
@@ -0,0 +1,253 @@
+/*  spinorcmd.c  - Give access to SPI NOR flash on command line.
+ *  Copyright 2021 Nokia
+ *  Licensed under the GNU General Public License v3.0 only
+ *  SPDX-License-Identifier: GPL-3.0-only
+ *
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2008  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <grub/types.h>
+#include <grub/misc.h>
+#include <grub/mm.h>
+#include <grub/efi/api.h>
+#include <grub/efi/efi.h>
+#include <grub/efi/efi_spi_nor.h>
+#include <grub/command.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+static void *cli_spi_flash_device = NULL;
+
+static void
+usage (void)
+{
+	grub_printf("spi_nor - access SPI NOR flash through UEFI API\n");
+	grub_printf("Usage:\n");
+	grub_printf("  spi_nor init|read|write|format <args>\n");
+	grub_printf("      init [-id <id0 id1 id2>] [-cnt <count>]\n");
+	grub_printf("             to be called once before operation on a
device.\n");
+	grub_printf("             <id>    : optional up to 3 bytes flash
identifier\n");
+	grub_printf("                       to match against\n");
+	grub_printf("             <count> : use n-th occurance of device\n");
+	grub_printf("                       (can be combined with <id>)\n");
+	grub_printf("      read <offset> <bytes> [<addr>]\n");
+	grub_printf("             read and dump/save bytes\n");
+	grub_printf("      write <offset> <bytes> <addr>\n");
+	grub_printf("             write bytes from <addr> to flash\n");
+	grub_printf("      erase <offset> <bytes>\n");
+	grub_printf("             format area \n");
+	grub_printf("             (<offset> and <bytes> must be erase block
alligned)\n");
+	grub_printf("\n");
+}
+
+/*  get_init_args - return index and number of -id and/or -cnt arguments
+	handle 4 possible inputs:
+	- spi_nor init -id <id0> <id1> -cnt <count>
+	- spi_nor init -cnt <count> -id <id0>
+	- spi_nor init -cnt <count>
+	- spi_nor init -id <id0> <id1> <id2>
+*/
+static int get_init_args(int argc, char **args, int *id_idx, int
*id_cnt, int *cnt_idx)
+{
+	int opt_idx = 1;
+	*id_idx = 0;
+	*cnt_idx = 0;
+	*id_cnt = 0;
+
+	while (opt_idx < argc) {
+		if (!grub_strcmp(args[opt_idx],"-id")) {
+			opt_idx++;
+			*id_idx = opt_idx;
+
+			while ((opt_idx < argc)
+					&& (grub_strcmp(args[opt_idx], "-cnt"))
+					&& (*id_cnt < 3)) {
+				opt_idx++;
+				*id_cnt = *id_cnt + 1;
+			}
+
+			if (*id_cnt == 0)
+				return 1;
+		} else if (!grub_strcmp(args[opt_idx],"-cnt")) {
+			if (argc > opt_idx + 1) {
+				*cnt_idx = opt_idx + 1;
+				opt_idx += 2;
+			} else {
+				return 1;
+			}
+		} else {
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+static grub_err_t
+grub_cmd_spi_nor (grub_command_t cmd __attribute__ ((unused)),
+		    int argc __attribute__ ((unused)),
+		    char **args __attribute__ ((unused)))
+{
+	grub_err_t ret;
+	int cnt_idx, id_idx, id_cnt = 0;
+	grub_uint8_t *device_id = NULL, devId[3];
+	grub_uint32_t instance = 0;
+
+	if (argc < 1) {
+		grub_printf("Missing argument\n");
+		usage();
+		return GRUB_ERR_BAD_ARGUMENT;
+	}
+
+	if (!grub_strcmp("init", args[0])) {
+		if (argc != 1) {
+			if (get_init_args(argc, args, &id_idx, &id_cnt, &cnt_idx)) {
+				usage();
+				return GRUB_ERR_BAD_ARGUMENT;
+			} else {
+				if (id_idx != 0) {
+					for (int i=0; i<id_cnt; i++)
+						devId[i] = grub_strtoul(args[id_idx + i], NULL, 0);
+					device_id = devId;
+				}
+				if (cnt_idx != 0) {
+					instance = grub_strtoul(args[cnt_idx], NULL, 0);
+				}
+			}
+		}
+
+		cli_spi_flash_device = grub_efi_spi_nor_init(device_id, id_cnt,
instance);
+		if (cli_spi_flash_device == NULL) {
+			grub_printf("No SPI NOR flash found\n");
+			return GRUB_ERR_UNKNOWN_DEVICE;
+		}
+		grub_printf("Found dev %x, capacity 0x%x bytes, erase block size 0x%x\n",
+					grub_efi_spi_nor_device_id(cli_spi_flash_device),
+					grub_efi_spi_nor_flash_size(cli_spi_flash_device),
+					grub_efi_spi_nor_erase_block_size(cli_spi_flash_device));
+		return GRUB_ERR_NONE;
+	}
+
+	if (cli_spi_flash_device == NULL) {
+		grub_printf("No known device. Call 'init' first\n");
+		usage();
+		return GRUB_ERR_UNKNOWN_DEVICE;
+	}
+
+	if (!grub_strcmp("read", args[0])) {
+		grub_uint8_t *data;
+		grub_uint32_t offset, num_bytes, i, j;
+
+		if (argc < 3) {
+			grub_printf("Missing parameters\n");
+			usage();
+			return GRUB_ERR_BAD_ARGUMENT;
+		}
+
+		offset = grub_strtoul(args[1], NULL, 0);
+		num_bytes = grub_strtoul(args[2], NULL, 0);
+
+		if (argc == 4) {
+			data = (grub_uint8_t *)grub_strtoul(args[3], NULL, 0);
+			if (data == NULL) {
+				grub_printf("Bad memory pointer, 0 not supported.\n");
+				usage();
+				return GRUB_ERR_BAD_ARGUMENT;
+			}
+		} else {
+			data = grub_malloc(num_bytes);
+			if (data == NULL) {
+				grub_printf("Out of memory.\n");
+				usage();
+				return GRUB_ERR_OUT_OF_MEMORY;
+			}
+		}
+
+		ret = grub_efi_spi_nor_read(cli_spi_flash_device, data, offset,
num_bytes);
+		if (ret != GRUB_ERR_NONE)
+			return ret;
+
+		if (argc == 3) {
+			for (i=0; i<num_bytes; i+=16) {
+				grub_printf("0x%06x: ", i + offset);
+				for (j=0; (j<16) && (i+j<num_bytes); j++)
+					grub_printf("%02x ", data[i+j]);
+				grub_printf("\n");
+			}
+			grub_free(data);
+		}
+		return GRUB_ERR_NONE;
+	}
+
+	if (!grub_strcmp("write", args[0])) {
+		grub_uint8_t *data;
+		grub_uint32_t offset, num_bytes;
+
+		if (argc != 4) {
+			grub_printf("Wrong number of parameters\n");
+			usage();
+			return GRUB_ERR_BAD_ARGUMENT;
+		}
+
+		offset = grub_strtoul(args[1], NULL, 0);
+		num_bytes = grub_strtoul(args[2], NULL, 0);
+
+		data = (grub_uint8_t *)grub_strtoul(args[3], NULL, 0);
+		if (data == NULL) {
+			grub_printf("Bad memory pointer, 0 not supported.\n");
+			usage();
+			return GRUB_ERR_BAD_ARGUMENT;
+		}
+
+		ret = grub_efi_spi_nor_write(cli_spi_flash_device, data, offset,
num_bytes);
+
+		return ret;
+	}
+
+	if (!grub_strcmp("erase", args[0])) {
+		grub_uint32_t offset, num_bytes;
+
+		if (argc != 3) {
+			grub_printf("Wrong number of parameters\n");
+			usage();
+			return GRUB_ERR_BAD_ARGUMENT;
+		}
+
+		offset = grub_strtoul(args[1], NULL, 0);
+		num_bytes = grub_strtoul(args[2], NULL, 0);
+
+		ret = grub_efi_spi_nor_erase(cli_spi_flash_device, offset, num_bytes);
+
+		return ret;
+	}
+
+	grub_printf("Unknown command \"%s\"\n", args[1]);
+	usage();
+	return GRUB_ERR_BAD_ARGUMENT;
+}
+
+static grub_command_t cmd;
+
+GRUB_MOD_INIT(spinorcmd)
+{
+	cmd = grub_register_command("spi_nor", grub_cmd_spi_nor,
+			       "", "access SPI NOR flash");
+}
+
+GRUB_MOD_FINI(spinorcmd)
+{
+	grub_unregister_command(cmd);
+}



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] efi: SPI NOR flash support
  2021-02-05  8:58 [PATCH v2 1/2] efi: SPI NOR flash support Michael Lawnick
  2021-02-05  9:02 ` [PATCH v2 2/2] efi: SPI NOR flash command line Michael Lawnick
@ 2021-02-11  7:36 ` Michael Lawnick
  2021-02-11  7:36   ` Paul Menzel
  2021-02-11  8:51   ` Heinrich Schuchardt
  1 sibling, 2 replies; 12+ messages in thread
From: Michael Lawnick @ 2021-02-11  7:36 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Paul Menzel, xypron.glpk


Hi,

seven days of silence. In the end no interest for extending EFI support?

KR
Michael

Am 05.02.2021 um 09:58 schrieb Michael Lawnick:
> Add EFI SPI NOR driver
>
> Use UEFI interface for accessing SPI NOR flashes.
> If supported the implementation of UEFI boot software abstracts
> away all those ugly H/W details like SPI controller or protocol.
> Provided functions:
> grub_efi_spi_nor_
> 	init
> 	erase
> 	write
> 	read
> 	flash_size
> 	flash_id
> 	erase_block_size
>
> This driver might be used for further abstraction to a common
> (SPI) flash interface.
>
> Signed-off-by: Michael Lawnick <michael.lawnick@nokia.com>
> ---
> [Patch v2 1/2] : fix flaw in EFI header, wrong sequence of methods.
> ---
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 68b9e9f68..4d775e5f6 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -446,7 +446,7 @@ image = {
>      i386_pc = boot/i386/pc/boot.S;
>
>      cppflags = '-DHYBRID_BOOT=1';
> -
> +
>      i386_pc_ldflags = '$(TARGET_IMG_LDFLAGS)';
>      i386_pc_ldflags = '$(TARGET_IMG_BASE_LDOPT),0x7C00';
>
> @@ -656,6 +656,12 @@ module = {
>      enable = i386_multiboot;
>    };
>
> +module = {
> +  name = efi_spi_nor;
> +  common = bus/spi/efi_spi_nor.c;
> +  enable = efi;
> +};
> +
>    module = {
>      name = nativedisk;
>      common = commands/nativedisk.c;
> diff --git a/grub-core/bus/spi/efi_spi_nor.c
> b/grub-core/bus/spi/efi_spi_nor.c
> new file mode 100644
> index 000000000..0e073b436
> --- /dev/null
> +++ b/grub-core/bus/spi/efi_spi_nor.c
> @@ -0,0 +1,298 @@
> +/*  efi_spi_nor.c  - Give access to SPI NOR flash through UEFI interface.
> + *  Copyright 2021 Nokia
> + *  Licensed under the GNU General Public License v3.0 only
> + *  SPDX-License-Identifier: GPL-3.0-only
> + *
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2008  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <grub/command.h>
> +#include <grub/misc.h>
> +#include <grub/mm.h>
> +#include <grub/types.h>
> +#include <grub/efi/api.h>
> +#include <grub/efi/efi.h>
> +#include <grub/efi/efi_spi_nor.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +#define EFI_SPI_NOR_FLASH_PROTOCOL_GUID \
> +	{ 0xb57ec3fe, 0xf833, 0x4ba6, \
> +		{0x85, 0x78, 0x2a, 0x7d, 0x6a, 0x87, 0x44, 0x4b} \
> +	}
> +
> +#define EFI_FLASHID_LEN 3
> +
> +struct efi_spi_nor_flash_protocol {
> +	struct spi_nor	*spi_peripheral;
> +	grub_efi_uint32_t	flash_size;
> +	grub_efi_uint8_t	device_id[EFI_FLASHID_LEN];
> +	grub_efi_uint32_t	erase_block_size;
> +
> +	grub_efi_status_t (* get_flash_id)(struct efi_spi_nor_flash_protocol
> *this,
> +					     grub_uint8_t *buffer);
> +	grub_efi_status_t (* read_data)(struct efi_spi_nor_flash_protocol *this,
> +					  grub_uint32_t offset, grub_uint32_t len, grub_uint8_t *data);
> +	grub_efi_status_t (* lf_read_data)(struct efi_spi_nor_flash_protocol
> *this,
> +					     grub_uint32_t offset, grub_uint32_t len, grub_uint8_t *data);
> +	grub_efi_status_t (* read_status)(struct efi_spi_nor_flash_protocol *this,
> +					    grub_uint32_t num_bytes, grub_uint8_t *status);
> +	grub_efi_status_t (* write_status)(struct efi_spi_nor_flash_protocol
> *this,
> +					     grub_uint32_t num_bytes, grub_uint8_t *status);
> +	grub_efi_status_t (* write_data)(struct efi_spi_nor_flash_protocol *this,
> +					   grub_uint32_t offset, grub_uint32_t len, grub_uint8_t *data);
> +	grub_efi_status_t (* erase_blocks)(struct efi_spi_nor_flash_protocol
> *this,
> +					     grub_uint32_t offset, grub_uint32_t blk_count);
> +};
> +
> +/* grub_efi_spi_nor_init - initialize access to SPI NOR flash device
> + *
> + * Search pool of SPI NOR flash devices known to underlying EFI bootware.
> + * Use <flash_id> and <instance> to filter out devices.
> + *
> + * IN: flash_id     - optional, pointer to max 3 bytes
> (EFI_FLASHID_LEN) to match against
> + *                    SPI flash JEDEC ID, use NULL if no filtering.
> + * IN: num_id_bytes - number of bytes in flash_id. Maximum 3 bytes
> + *                    are used for comparison.
> + * IN: instance     - number of device occurances to skip
> + *
> + * returns : pointer to flash device or NULL on failure
> + */
> +void *
> +grub_efi_spi_nor_init(grub_uint8_t *flash_id, grub_uint32_t
> num_id_bytes, grub_uint32_t instance)
> +{
> +	grub_efi_guid_t efi_guid_spi_nor_flash_protocol =
> EFI_SPI_NOR_FLASH_PROTOCOL_GUID;
> +	grub_efi_status_t ret;
> +	grub_efi_uintn_t num_handles;
> +	grub_efi_handle_t *handles;
> +	grub_uint8_t found_id[EFI_FLASHID_LEN];
> +	grub_uint32_t idx, match_cnt=0;
> +	struct efi_spi_nor_flash_protocol *spi_nor;
> +
> +	handles = grub_efi_locate_handle(GRUB_EFI_BY_PROTOCOL,
> +				&efi_guid_spi_nor_flash_protocol, 0,
> +				&num_handles);
> +	grub_boot_time("found %ld SPI NOR flash devices\n", num_handles);
> +
> +	if ((num_handles == 0) || (num_handles < instance + 1))
> +		return NULL;
> +
> +	for (idx = 0; idx < num_handles; idx++) {
> +		spi_nor = grub_efi_open_protocol(handles[idx],
> +						&efi_guid_spi_nor_flash_protocol,
> +						GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> +		if (! spi_nor) {
> +			grub_error(GRUB_ERR_UNKNOWN_DEVICE, "Failed to open device protocol\n");
> +			grub_free(handles);
> +			return NULL;
> +		}
> +
> +		ret = spi_nor->get_flash_id(spi_nor, found_id);
> +		if (ret != GRUB_EFI_SUCCESS) {
> +			grub_error(GRUB_ERR_READ_ERROR, "Failed to read flash_id\n");
> +			grub_free(handles);
> +			return NULL;
> +		}
> +
> +		/* if caller requests filtering by id */
> +		if (flash_id != NULL) {
> +			grub_uint32_t id;
> +
> +			if (num_id_bytes > EFI_FLASHID_LEN)
> +				num_id_bytes = EFI_FLASHID_LEN;
> +
> +			for (id = 0; id < num_id_bytes; id++)
> +				if (flash_id[id] != found_id[id])
> +					break;
> +
> +			if (id != num_id_bytes)
> +				continue;
> +		}
> +
> +		if (match_cnt < instance) {
> +			match_cnt++;
> +			continue;
> +		}
> +
> +		grub_boot_time("Found flash with ID 0x%02x 0x%02x 0x%02x\n",
> +						found_id[0], found_id[1], found_id[2]);
> +
> +		grub_free(handles);
> +		return spi_nor;
> +	}
> +
> +	grub_free(handles);
> +	return NULL;
> +}
> +
> +/* grub_efi_spi_nor_flash_size - get memory size of SPI NOR flash device
> + *
> + * IN: efi_flash_dev - device identifier returned by grub_efi_spi_nor_init.
> + *
> + * returns : memory size of flash device or 0 on failure
> + */
> +grub_uint32_t
> +grub_efi_spi_nor_flash_size(void *efi_flash_dev)
> +{
> +	struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
> +
> +	if (spi_nor == NULL)
> +		return 0;
> +
> +	return spi_nor->flash_size;
> +}
> +
> +/* grub_efi_spi_nor_device_id - get three byte JEDEC ID of SPI NOR
> flash device
> + *
> + * IN: efi_flash_dev - device identifier returned by grub_efi_spi_nor_init.
> + *
> + * returns : three byte JEDEC ID as a uint32 or 0 on failure
> + */
> +grub_uint32_t
> +grub_efi_spi_nor_device_id(void *efi_flash_dev)
> +{
> +	struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
> +	grub_uint32_t devId = 0;
> +	grub_efi_status_t ret;
> +	grub_uint8_t device_id[3];
> +	int i;
> +
> +	if (spi_nor == NULL)
> +		return 0;
> +
> +	ret = spi_nor->get_flash_id(spi_nor, device_id);
> +	if (ret != GRUB_EFI_SUCCESS)
> +		return 0;
> +
> +	for(i=0; i<3;i++)
> +		devId = (devId<<8) + device_id[i];
> +
> +	return devId;
> +}
> +
> +/* grub_efi_spi_nor_erase_block_size - get erase block size of SPI NOR
> flash device
> + *
> + * Parameters for calls to grub_efi_spi_nor_erase() need to be erase
> block size
> + * aligned.
> + *
> + * IN: efi_flash_dev - device identifier returned by grub_efi_spi_nor_init.
> + *
> + * returns : size of erase block of flash device or 0 on failure
> + */
> +grub_uint32_t
> +grub_efi_spi_nor_erase_block_size(void *efi_flash_dev)
> +{
> +	struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
> +
> +	if (spi_nor == NULL)
> +		return 0;
> +
> +	return spi_nor->erase_block_size;
> +}
> +
> +/* grub_efi_spi_nor_read - read from SPI NOR flash device
> + *
> + * IN:  efi_flash_dev - device identifier returned by
> grub_efi_spi_nor_init.
> + * OUT: buffer        - pointer to an preallocated buffer of min. bytes
> size
> + * IN:  offset        - starting point where to start to read from
> + * IN:  bytes         - number of bytes to read from flash and write to
> buffer
> + *
> + * returns : GRUB_EFI_SUCCESS if all ok
> + *           GRUB_EFI_INVALID_PARAMETER if an argument is bad
> + *
> + */
> +grub_err_t
> +grub_efi_spi_nor_read(void *efi_flash_dev, grub_uint8_t *buffer,
> grub_uint32_t offset, grub_uint32_t bytes)
> +{
> +	struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
> +	grub_efi_status_t ret;
> +
> +	if (spi_nor == NULL || buffer == NULL)
> +		return GRUB_ERR_BAD_ARGUMENT;
> +
> +	ret = (spi_nor->read_data(spi_nor, offset, bytes, buffer));
> +	if (ret != GRUB_EFI_SUCCESS)
> +		return grub_error(GRUB_ERR_READ_ERROR, "Failed to read data @0x%x\n",
> offset);
> +
> +	return GRUB_ERR_NONE;
> +}
> +
> +/* grub_efi_spi_nor_write - write to SPI NOR flash device
> + *
> + * IN: efi_flash_dev - device identifier returned by grub_efi_spi_nor_init.
> + * IN: buffer        - pointer to buffer containig min. bytes size data
> to write
> + * IN: offset        - starting point where to start to write to
> + * IN: bytes         - number of bytes to write to buffer
> + *
> + * returns : GRUB_EFI_SUCCESS if all ok
> + *           GRUB_EFI_INVALID_PARAMETER if an argument is bad
> + *
> + */
> +grub_err_t
> +grub_efi_spi_nor_write(void *efi_flash_dev, grub_uint8_t *buffer,
> grub_uint32_t offset, grub_uint32_t bytes)
> +{
> +	struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
> +	grub_efi_status_t ret;
> +
> +	if (spi_nor == NULL || buffer == NULL)
> +		return GRUB_ERR_BAD_ARGUMENT;
> +
> +	ret = (spi_nor->write_data(spi_nor, offset, bytes, buffer));
> +	if (ret != GRUB_EFI_SUCCESS)
> +		return grub_error(GRUB_ERR_WRITE_ERROR, "Failed to write data
> @0x%x\n", offset);
> +
> +	return GRUB_ERR_NONE;
> +}
> +
> +/* grub_efi_spi_nor_erase - erase sectors on SPI NOR flash device
> + *
> + * Parameters offset and bytes need to be erase sector aligned, i.e.
> + * multiples of the size returned by function
> grub_efi_spi_nor_erase_block_size()
> + *
> + * IN: efi_flash_dev - device identifier returned by grub_efi_spi_nor_init.
> + * IN: offset        - offset of first sector to erase
> + * IN: bytes         - length of region to erase
> + *
> + * returns : GRUB_EFI_SUCCESS if all ok
> + *           GRUB_EFI_INVALID_PARAMETER if an argument is bad
> + *
> + */
> +grub_err_t
> +grub_efi_spi_nor_erase(void *efi_flash_dev, grub_uint32_t offset,
> grub_uint32_t bytes)
> +{
> +	struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
> +	grub_efi_status_t ret;
> +	grub_uint32_t sect_sz;
> +	grub_uint32_t sect_mask;
> +
> +	if (spi_nor == NULL || bytes == 0)
> +		return GRUB_ERR_BAD_ARGUMENT;
> +
> +	sect_sz = grub_efi_spi_nor_erase_block_size(spi_nor);
> +	sect_mask = sect_sz - 1;
> +	if ((offset & sect_mask) != 0)
> +		return grub_error(GRUB_ERR_BAD_ARGUMENT, "SPI NOR erase offset not at
> sector boundary");
> +
> +	if (((offset + bytes) & sect_mask) != 0)
> +		return grub_error(GRUB_ERR_BAD_ARGUMENT, "SPI NOR erase end not at
> sector boundary");
> +
> +	ret = spi_nor->erase_blocks(spi_nor, offset, (bytes-1) / sect_sz + 1);
> +
> +	if (ret != GRUB_EFI_SUCCESS)
> +		return grub_error(GRUB_ERR_WRITE_ERROR, "SPI NOR erase operation
> failed");
> +
> +	return GRUB_ERR_NONE;
> +}
> diff --git a/include/grub/efi/efi_spi_nor.h b/include/grub/efi/efi_spi_nor.h
> new file mode 100644
> index 000000000..d09f0c9b7
> --- /dev/null
> +++ b/include/grub/efi/efi_spi_nor.h
> @@ -0,0 +1,37 @@
> +/*  efi_spi_nor.h  - Give access to SPI NOR flash through UEFI interface.
> + *  Copyright 2021 Nokia
> + *  Licensed under the GNU General Public License v3.0 only
> + *  SPDX-License-Identifier: GPL-3.0-only
> + *
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2008  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef GRUB_EFI_SPI_NOR_HEADER
> +#define GRUB_EFI_SPI_NOR_HEADER
> +
> +#include <grub/types.h>
> +
> +void *grub_efi_spi_nor_init(grub_uint8_t *flash_id, grub_uint32_t
> num_id_bytes, grub_uint32_t instance);
> +
> +grub_uint32_t grub_efi_spi_nor_flash_size(void *efi_flash_dev);
> +grub_uint32_t grub_efi_spi_nor_device_id(void *efi_flash_dev);
> +grub_uint32_t grub_efi_spi_nor_erase_block_size(void *efi_flash_dev);
> +
> +grub_err_t grub_efi_spi_nor_read(void *efi_flash_dev, grub_uint8_t
> *buffer, grub_uint32_t offset, grub_uint32_t bytes);
> +grub_err_t grub_efi_spi_nor_write(void *efi_flash_dev, grub_uint8_t
> *buffer, grub_uint32_t offset, grub_uint32_t bytes);
> +grub_err_t grub_efi_spi_nor_erase(void *efi_flash_dev, grub_uint32_t
> offset, grub_uint32_t bytes);
> +
> +#endif /*GRUB_EFISPINOR*/
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Re: [PATCH v2 1/2] efi: SPI NOR flash support
  2021-02-11  7:36 ` [PATCH v2 1/2] efi: SPI NOR flash support Michael Lawnick
@ 2021-02-11  7:36   ` Paul Menzel
  2021-02-11  8:51   ` Heinrich Schuchardt
  1 sibling, 0 replies; 12+ messages in thread
From: Paul Menzel @ 2021-02-11  7:36 UTC (permalink / raw)
  To: The development of GNU GRUB

Sehr geehrte Damen und Herren,


vielen Dank für Ihre Nachricht, die ich nach dem 12. Februar 2021 lesen werde.


Freundliche Grüße

Paul Menzel


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] efi: SPI NOR flash support
  2021-02-11  7:36 ` [PATCH v2 1/2] efi: SPI NOR flash support Michael Lawnick
  2021-02-11  7:36   ` Paul Menzel
@ 2021-02-11  8:51   ` Heinrich Schuchardt
  2021-02-11 12:50     ` Michael Lawnick
  1 sibling, 1 reply; 12+ messages in thread
From: Heinrich Schuchardt @ 2021-02-11  8:51 UTC (permalink / raw)
  To: The development of GNU GRUB, Michael Lawnick, Michael Lawnick; +Cc: Paul Menzel

On 11.02.21 08:36, Michael Lawnick wrote:
>
> Hi,
>
> seven days of silence. In the end no interest for extending EFI support?
>
> KR
> Michael
>
> Am 05.02.2021 um 09:58 schrieb Michael Lawnick:
>> Add EFI SPI NOR driver
>>
>> Use UEFI interface for accessing SPI NOR flashes.
>> If supported the implementation of UEFI boot software abstracts
>> away all those ugly H/W details like SPI controller or protocol.
>> Provided functions:
>> grub_efi_spi_nor_
>>     init
>>     erase
>>     write
>>     read
>>     flash_size
>>     flash_id
>>     erase_block_size
>>
>> This driver might be used for further abstraction to a common
>> (SPI) flash interface.
>>

A commit message should describe what the patch is good for.

What is the use case for GRUB accessing SPI?

In your second patch you introduce a command to write and erase the SPI
flash. Hopefully the firmware has disabled writes.

GRUB writing to SPI would mean that a user program could introduce
malware into the firmware by adding said command to grub.cfg.

This would be a gross security issue. Hopefully the firmware has locked
the SPI flash before entering GRUB.

SPI flash updates should be effected via signed UEFI update capsules and
not via GRUB.

>> Signed-off-by: Michael Lawnick <michael.lawnick@nokia.com>
>> ---
>> [Patch v2 1/2] : fix flaw in EFI header, wrong sequence of methods.
>> ---
>> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
>> index 68b9e9f68..4d775e5f6 100644
>> --- a/grub-core/Makefile.core.def
>> +++ b/grub-core/Makefile.core.def
>> @@ -446,7 +446,7 @@ image = {
>>      i386_pc = boot/i386/pc/boot.S;
>>
>>      cppflags = '-DHYBRID_BOOT=1';
>> -
>> +
>>      i386_pc_ldflags = '$(TARGET_IMG_LDFLAGS)';
>>      i386_pc_ldflags = '$(TARGET_IMG_BASE_LDOPT),0x7C00';
>>
>> @@ -656,6 +656,12 @@ module = {
>>      enable = i386_multiboot;
>>    };
>>
>> +module = {
>> +  name = efi_spi_nor;
>> +  common = bus/spi/efi_spi_nor.c;
>> +  enable = efi;
>> +};
>> +
>>    module = {
>>      name = nativedisk;
>>      common = commands/nativedisk.c;
>> diff --git a/grub-core/bus/spi/efi_spi_nor.c
>> b/grub-core/bus/spi/efi_spi_nor.c
>> new file mode 100644
>> index 000000000..0e073b436
>> --- /dev/null
>> +++ b/grub-core/bus/spi/efi_spi_nor.c
>> @@ -0,0 +1,298 @@
>> +/*  efi_spi_nor.c  - Give access to SPI NOR flash through UEFI
>> interface.
>> + *  Copyright 2021 Nokia
>> + *  Licensed under the GNU General Public License v3.0 only
>> + *  SPDX-License-Identifier: GPL-3.0-only
>> + *
>> + *  GRUB  --  GRand Unified Bootloader
>> + *  Copyright (C) 2008  Free Software Foundation, Inc.

The FSF wrote part of this code in 2008?

>> + *
>> + *  GRUB is free software: you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation, either version 3 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  GRUB is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include <grub/command.h>
>> +#include <grub/misc.h>
>> +#include <grub/mm.h>
>> +#include <grub/types.h>
>> +#include <grub/efi/api.h>
>> +#include <grub/efi/efi.h>
>> +#include <grub/efi/efi_spi_nor.h>
>> +
>> +GRUB_MOD_LICENSE ("GPLv3+");
>> +
>> +#define EFI_SPI_NOR_FLASH_PROTOCOL_GUID \
>> +    { 0xb57ec3fe, 0xf833, 0x4ba6, \
>> +        {0x85, 0x78, 0x2a, 0x7d, 0x6a, 0x87, 0x44, 0x4b} \
>> +    }

This protocol is not defined in the UEFI spec 2.8B.

It is defined in the Platform Initialization (PI) Specification, Volume
1, Pre-EFI Initialization Core Interface.

So it seems it is not meant to be consumed by UEFI applications.

U-Boot adheres to the UEFI spec but has no PI phase. So don't expect the
protocol there.

Best regards

Heinrich

>> +
>> +#define EFI_FLASHID_LEN 3
>> +
>> +struct efi_spi_nor_flash_protocol {
>> +    struct spi_nor    *spi_peripheral;
>> +    grub_efi_uint32_t    flash_size;
>> +    grub_efi_uint8_t    device_id[EFI_FLASHID_LEN];
>> +    grub_efi_uint32_t    erase_block_size;
>> +
>> +    grub_efi_status_t (* get_flash_id)(struct efi_spi_nor_flash_protocol
>> *this,
>> +                         grub_uint8_t *buffer);
>> +    grub_efi_status_t (* read_data)(struct efi_spi_nor_flash_protocol
>> *this,
>> +                      grub_uint32_t offset, grub_uint32_t len,
>> grub_uint8_t *data);
>> +    grub_efi_status_t (* lf_read_data)(struct efi_spi_nor_flash_protocol
>> *this,
>> +                         grub_uint32_t offset, grub_uint32_t len,
>> grub_uint8_t *data);
>> +    grub_efi_status_t (* read_status)(struct
>> efi_spi_nor_flash_protocol *this,
>> +                        grub_uint32_t num_bytes, grub_uint8_t *status);
>> +    grub_efi_status_t (* write_status)(struct efi_spi_nor_flash_protocol
>> *this,
>> +                         grub_uint32_t num_bytes, grub_uint8_t *status);
>> +    grub_efi_status_t (* write_data)(struct
>> efi_spi_nor_flash_protocol *this,
>> +                       grub_uint32_t offset, grub_uint32_t len,
>> grub_uint8_t *data);
>> +    grub_efi_status_t (* erase_blocks)(struct efi_spi_nor_flash_protocol
>> *this,
>> +                         grub_uint32_t offset, grub_uint32_t blk_count);
>> +};
>> +
>> +/* grub_efi_spi_nor_init - initialize access to SPI NOR flash device
>> + *
>> + * Search pool of SPI NOR flash devices known to underlying EFI
>> bootware.
>> + * Use <flash_id> and <instance> to filter out devices.
>> + *
>> + * IN: flash_id     - optional, pointer to max 3 bytes
>> (EFI_FLASHID_LEN) to match against
>> + *                    SPI flash JEDEC ID, use NULL if no filtering.
>> + * IN: num_id_bytes - number of bytes in flash_id. Maximum 3 bytes
>> + *                    are used for comparison.
>> + * IN: instance     - number of device occurances to skip
>> + *
>> + * returns : pointer to flash device or NULL on failure
>> + */
>> +void *
>> +grub_efi_spi_nor_init(grub_uint8_t *flash_id, grub_uint32_t
>> num_id_bytes, grub_uint32_t instance)
>> +{
>> +    grub_efi_guid_t efi_guid_spi_nor_flash_protocol =
>> EFI_SPI_NOR_FLASH_PROTOCOL_GUID;
>> +    grub_efi_status_t ret;
>> +    grub_efi_uintn_t num_handles;
>> +    grub_efi_handle_t *handles;
>> +    grub_uint8_t found_id[EFI_FLASHID_LEN];
>> +    grub_uint32_t idx, match_cnt=0;
>> +    struct efi_spi_nor_flash_protocol *spi_nor;
>> +
>> +    handles = grub_efi_locate_handle(GRUB_EFI_BY_PROTOCOL,
>> +                &efi_guid_spi_nor_flash_protocol, 0,
>> +                &num_handles);
>> +    grub_boot_time("found %ld SPI NOR flash devices\n", num_handles);
>> +
>> +    if ((num_handles == 0) || (num_handles < instance + 1))
>> +        return NULL;
>> +
>> +    for (idx = 0; idx < num_handles; idx++) {
>> +        spi_nor = grub_efi_open_protocol(handles[idx],
>> +                        &efi_guid_spi_nor_flash_protocol,
>> +                        GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>> +        if (! spi_nor) {
>> +            grub_error(GRUB_ERR_UNKNOWN_DEVICE, "Failed to open
>> device protocol\n");
>> +            grub_free(handles);
>> +            return NULL;
>> +        }
>> +
>> +        ret = spi_nor->get_flash_id(spi_nor, found_id);
>> +        if (ret != GRUB_EFI_SUCCESS) {
>> +            grub_error(GRUB_ERR_READ_ERROR, "Failed to read
>> flash_id\n");
>> +            grub_free(handles);
>> +            return NULL;
>> +        }
>> +
>> +        /* if caller requests filtering by id */
>> +        if (flash_id != NULL) {
>> +            grub_uint32_t id;
>> +
>> +            if (num_id_bytes > EFI_FLASHID_LEN)
>> +                num_id_bytes = EFI_FLASHID_LEN;
>> +
>> +            for (id = 0; id < num_id_bytes; id++)
>> +                if (flash_id[id] != found_id[id])
>> +                    break;
>> +
>> +            if (id != num_id_bytes)
>> +                continue;
>> +        }
>> +
>> +        if (match_cnt < instance) {
>> +            match_cnt++;
>> +            continue;
>> +        }
>> +
>> +        grub_boot_time("Found flash with ID 0x%02x 0x%02x 0x%02x\n",
>> +                        found_id[0], found_id[1], found_id[2]);
>> +
>> +        grub_free(handles);
>> +        return spi_nor;
>> +    }
>> +
>> +    grub_free(handles);
>> +    return NULL;
>> +}
>> +
>> +/* grub_efi_spi_nor_flash_size - get memory size of SPI NOR flash device
>> + *
>> + * IN: efi_flash_dev - device identifier returned by
>> grub_efi_spi_nor_init.
>> + *
>> + * returns : memory size of flash device or 0 on failure
>> + */
>> +grub_uint32_t
>> +grub_efi_spi_nor_flash_size(void *efi_flash_dev)
>> +{
>> +    struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
>> +
>> +    if (spi_nor == NULL)
>> +        return 0;
>> +
>> +    return spi_nor->flash_size;
>> +}
>> +
>> +/* grub_efi_spi_nor_device_id - get three byte JEDEC ID of SPI NOR
>> flash device
>> + *
>> + * IN: efi_flash_dev - device identifier returned by
>> grub_efi_spi_nor_init.
>> + *
>> + * returns : three byte JEDEC ID as a uint32 or 0 on failure
>> + */
>> +grub_uint32_t
>> +grub_efi_spi_nor_device_id(void *efi_flash_dev)
>> +{
>> +    struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
>> +    grub_uint32_t devId = 0;
>> +    grub_efi_status_t ret;
>> +    grub_uint8_t device_id[3];
>> +    int i;
>> +
>> +    if (spi_nor == NULL)
>> +        return 0;
>> +
>> +    ret = spi_nor->get_flash_id(spi_nor, device_id);
>> +    if (ret != GRUB_EFI_SUCCESS)
>> +        return 0;
>> +
>> +    for(i=0; i<3;i++)
>> +        devId = (devId<<8) + device_id[i];
>> +
>> +    return devId;
>> +}
>> +
>> +/* grub_efi_spi_nor_erase_block_size - get erase block size of SPI NOR
>> flash device
>> + *
>> + * Parameters for calls to grub_efi_spi_nor_erase() need to be erase
>> block size
>> + * aligned.
>> + *
>> + * IN: efi_flash_dev - device identifier returned by
>> grub_efi_spi_nor_init.
>> + *
>> + * returns : size of erase block of flash device or 0 on failure
>> + */
>> +grub_uint32_t
>> +grub_efi_spi_nor_erase_block_size(void *efi_flash_dev)
>> +{
>> +    struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
>> +
>> +    if (spi_nor == NULL)
>> +        return 0;
>> +
>> +    return spi_nor->erase_block_size;
>> +}
>> +
>> +/* grub_efi_spi_nor_read - read from SPI NOR flash device
>> + *
>> + * IN:  efi_flash_dev - device identifier returned by
>> grub_efi_spi_nor_init.
>> + * OUT: buffer        - pointer to an preallocated buffer of min. bytes
>> size
>> + * IN:  offset        - starting point where to start to read from
>> + * IN:  bytes         - number of bytes to read from flash and write to
>> buffer
>> + *
>> + * returns : GRUB_EFI_SUCCESS if all ok
>> + *           GRUB_EFI_INVALID_PARAMETER if an argument is bad
>> + *
>> + */
>> +grub_err_t
>> +grub_efi_spi_nor_read(void *efi_flash_dev, grub_uint8_t *buffer,
>> grub_uint32_t offset, grub_uint32_t bytes)
>> +{
>> +    struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
>> +    grub_efi_status_t ret;
>> +
>> +    if (spi_nor == NULL || buffer == NULL)
>> +        return GRUB_ERR_BAD_ARGUMENT;
>> +
>> +    ret = (spi_nor->read_data(spi_nor, offset, bytes, buffer));
>> +    if (ret != GRUB_EFI_SUCCESS)
>> +        return grub_error(GRUB_ERR_READ_ERROR, "Failed to read data
>> @0x%x\n",
>> offset);
>> +
>> +    return GRUB_ERR_NONE;
>> +}
>> +
>> +/* grub_efi_spi_nor_write - write to SPI NOR flash device
>> + *
>> + * IN: efi_flash_dev - device identifier returned by
>> grub_efi_spi_nor_init.
>> + * IN: buffer        - pointer to buffer containig min. bytes size data
>> to write
>> + * IN: offset        - starting point where to start to write to
>> + * IN: bytes         - number of bytes to write to buffer
>> + *
>> + * returns : GRUB_EFI_SUCCESS if all ok
>> + *           GRUB_EFI_INVALID_PARAMETER if an argument is bad
>> + *
>> + */
>> +grub_err_t
>> +grub_efi_spi_nor_write(void *efi_flash_dev, grub_uint8_t *buffer,
>> grub_uint32_t offset, grub_uint32_t bytes)
>> +{
>> +    struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
>> +    grub_efi_status_t ret;
>> +
>> +    if (spi_nor == NULL || buffer == NULL)
>> +        return GRUB_ERR_BAD_ARGUMENT;
>> +
>> +    ret = (spi_nor->write_data(spi_nor, offset, bytes, buffer));
>> +    if (ret != GRUB_EFI_SUCCESS)
>> +        return grub_error(GRUB_ERR_WRITE_ERROR, "Failed to write data
>> @0x%x\n", offset);
>> +
>> +    return GRUB_ERR_NONE;
>> +}
>> +
>> +/* grub_efi_spi_nor_erase - erase sectors on SPI NOR flash device
>> + *
>> + * Parameters offset and bytes need to be erase sector aligned, i.e.
>> + * multiples of the size returned by function
>> grub_efi_spi_nor_erase_block_size()
>> + *
>> + * IN: efi_flash_dev - device identifier returned by
>> grub_efi_spi_nor_init.
>> + * IN: offset        - offset of first sector to erase
>> + * IN: bytes         - length of region to erase
>> + *
>> + * returns : GRUB_EFI_SUCCESS if all ok
>> + *           GRUB_EFI_INVALID_PARAMETER if an argument is bad
>> + *
>> + */
>> +grub_err_t
>> +grub_efi_spi_nor_erase(void *efi_flash_dev, grub_uint32_t offset,
>> grub_uint32_t bytes)
>> +{
>> +    struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
>> +    grub_efi_status_t ret;
>> +    grub_uint32_t sect_sz;
>> +    grub_uint32_t sect_mask;
>> +
>> +    if (spi_nor == NULL || bytes == 0)
>> +        return GRUB_ERR_BAD_ARGUMENT;
>> +
>> +    sect_sz = grub_efi_spi_nor_erase_block_size(spi_nor);
>> +    sect_mask = sect_sz - 1;
>> +    if ((offset & sect_mask) != 0)
>> +        return grub_error(GRUB_ERR_BAD_ARGUMENT, "SPI NOR erase
>> offset not at
>> sector boundary");
>> +
>> +    if (((offset + bytes) & sect_mask) != 0)
>> +        return grub_error(GRUB_ERR_BAD_ARGUMENT, "SPI NOR erase end
>> not at
>> sector boundary");
>> +
>> +    ret = spi_nor->erase_blocks(spi_nor, offset, (bytes-1) / sect_sz
>> + 1);
>> +
>> +    if (ret != GRUB_EFI_SUCCESS)
>> +        return grub_error(GRUB_ERR_WRITE_ERROR, "SPI NOR erase operation
>> failed");
>> +
>> +    return GRUB_ERR_NONE;
>> +}
>> diff --git a/include/grub/efi/efi_spi_nor.h
>> b/include/grub/efi/efi_spi_nor.h
>> new file mode 100644
>> index 000000000..d09f0c9b7
>> --- /dev/null
>> +++ b/include/grub/efi/efi_spi_nor.h
>> @@ -0,0 +1,37 @@
>> +/*  efi_spi_nor.h  - Give access to SPI NOR flash through UEFI
>> interface.
>> + *  Copyright 2021 Nokia
>> + *  Licensed under the GNU General Public License v3.0 only
>> + *  SPDX-License-Identifier: GPL-3.0-only
>> + *
>> + *  GRUB  --  GRand Unified Bootloader
>> + *  Copyright (C) 2008  Free Software Foundation, Inc.
>> + *
>> + *  GRUB is free software: you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation, either version 3 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  GRUB is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#ifndef GRUB_EFI_SPI_NOR_HEADER
>> +#define GRUB_EFI_SPI_NOR_HEADER
>> +
>> +#include <grub/types.h>
>> +
>> +void *grub_efi_spi_nor_init(grub_uint8_t *flash_id, grub_uint32_t
>> num_id_bytes, grub_uint32_t instance);
>> +
>> +grub_uint32_t grub_efi_spi_nor_flash_size(void *efi_flash_dev);
>> +grub_uint32_t grub_efi_spi_nor_device_id(void *efi_flash_dev);
>> +grub_uint32_t grub_efi_spi_nor_erase_block_size(void *efi_flash_dev);
>> +
>> +grub_err_t grub_efi_spi_nor_read(void *efi_flash_dev, grub_uint8_t
>> *buffer, grub_uint32_t offset, grub_uint32_t bytes);
>> +grub_err_t grub_efi_spi_nor_write(void *efi_flash_dev, grub_uint8_t
>> *buffer, grub_uint32_t offset, grub_uint32_t bytes);
>> +grub_err_t grub_efi_spi_nor_erase(void *efi_flash_dev, grub_uint32_t
>> offset, grub_uint32_t bytes);
>> +
>> +#endif /*GRUB_EFISPINOR*/
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] efi: SPI NOR flash support
  2021-02-11  8:51   ` Heinrich Schuchardt
@ 2021-02-11 12:50     ` Michael Lawnick
  2021-02-11 14:04       ` Heinrich Schuchardt
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Lawnick @ 2021-02-11 12:50 UTC (permalink / raw)
  To: Heinrich Schuchardt, The development of GNU GRUB; +Cc: Paul Menzel

Am 11.02.2021 um 09:51 schrieb Heinrich Schuchardt:
> On 11.02.21 08:36, Michael Lawnick wrote:
>>
>> Hi,
>>
>> seven days of silence. In the end no interest for extending EFI support?
>>
>> KR
>> Michael
>>
>> Am 05.02.2021 um 09:58 schrieb Michael Lawnick:
>>> Add EFI SPI NOR driver
>>>
>>> Use UEFI interface for accessing SPI NOR flashes.
>>> If supported the implementation of UEFI boot software abstracts
>>> away all those ugly H/W details like SPI controller or protocol.
>>> Provided functions:
>>> grub_efi_spi_nor_
>>>      init
>>>      erase
>>>      write
>>>      read
>>>      flash_size
>>>      flash_id
>>>      erase_block_size
>>>
>>> This driver might be used for further abstraction to a common
>>> (SPI) flash interface.
>>>
>
> A commit message should describe what the patch is good for.
>
> What is the use case for GRUB accessing SPI?

Many industrial systems use SPI flash as primary boot source. And most
times there are changeable parameters to be stored.

>
> In your second patch you introduce a command to write and erase the SPI
> flash. Hopefully the firmware has disabled writes.
>
> GRUB writing to SPI would mean that a user program could introduce
> malware into the firmware by adding said command to grub.cfg.
>
> This would be a gross security issue. Hopefully the firmware has locked
> the SPI flash before entering GRUB.
>
> SPI flash updates should be effected via signed UEFI update capsules and
> not via GRUB.

Hi,

write protection is system architecture issue. If sensitive sections
aren't protected S/W support for access won't change that. Latest in O/S
state the devices can be accessed.
On (many/some) x86 SoC I know it is BIOS which configures read/write
protection, on my current ARM based system it is a combination of
security level for complete boot device and H/W protection pin for
unchangeable section in data device.
In our company's area we do have H/W protection enabled on root of trust
part and verification on module chain.
Several boot parameters are stored in our SPI flash to configure the
systems for different use cases.

I have the impression that your view is coming from x86/desktop
direction. I am coming from embedded systems.

KR
Michael


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] efi: SPI NOR flash support
  2021-02-11 12:50     ` Michael Lawnick
@ 2021-02-11 14:04       ` Heinrich Schuchardt
  2021-02-11 16:01         ` Michael Lawnick
  0 siblings, 1 reply; 12+ messages in thread
From: Heinrich Schuchardt @ 2021-02-11 14:04 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Paul Menzel, Michael Lawnick

On 11.02.21 13:50, Michael Lawnick wrote:
> Am 11.02.2021 um 09:51 schrieb Heinrich Schuchardt:
>> On 11.02.21 08:36, Michael Lawnick wrote:
>>>
>>> Hi,
>>>
>>> seven days of silence. In the end no interest for extending EFI support?
>>>
>>> KR
>>> Michael
>>>
>>> Am 05.02.2021 um 09:58 schrieb Michael Lawnick:
>>>> Add EFI SPI NOR driver
>>>>
>>>> Use UEFI interface for accessing SPI NOR flashes.
>>>> If supported the implementation of UEFI boot software abstracts
>>>> away all those ugly H/W details like SPI controller or protocol.
>>>> Provided functions:
>>>> grub_efi_spi_nor_
>>>>      init
>>>>      erase
>>>>      write
>>>>      read
>>>>      flash_size
>>>>      flash_id
>>>>      erase_block_size
>>>>
>>>> This driver might be used for further abstraction to a common
>>>> (SPI) flash interface.
>>>>
>>
>> A commit message should describe what the patch is good for.
>>
>> What is the use case for GRUB accessing SPI?
>
> Many industrial systems use SPI flash as primary boot source. And most
> times there are changeable parameters to be stored.
>
>>
>> In your second patch you introduce a command to write and erase the SPI
>> flash. Hopefully the firmware has disabled writes.
>>
>> GRUB writing to SPI would mean that a user program could introduce
>> malware into the firmware by adding said command to grub.cfg.
>>
>> This would be a gross security issue. Hopefully the firmware has locked
>> the SPI flash before entering GRUB.
>>
>> SPI flash updates should be effected via signed UEFI update capsules and
>> not via GRUB.
>
> Hi,
>
> write protection is system architecture issue. If sensitive sections
> aren't protected S/W support for access won't change that. Latest in O/S
> state the devices can be accessed.
> On (many/some) x86 SoC I know it is BIOS which configures read/write
> protection, on my current ARM based system it is a combination of
> security level for complete boot device and H/W protection pin for
> unchangeable section in data device.
> In our company's area we do have H/W protection enabled on root of trust
> part and verification on module chain.
> Several boot parameters are stored in our SPI flash to configure the
> systems for different use cases.

I still do not understand why you need access in GRUB. You can read and
write the SPI flash in U-Boot on your embedded system.

Maybe you could provide a short example script showing how the new
command would fit into the GRUB boot flow.

Best regards

Heinrich

>
> I have the impression that your view is coming from x86/desktop
> direction. I am coming from embedded systems.
>
> KR
> Michael



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] efi: SPI NOR flash support
  2021-02-11 14:04       ` Heinrich Schuchardt
@ 2021-02-11 16:01         ` Michael Lawnick
  2021-02-11 17:16           ` Heinrich Schuchardt
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Lawnick @ 2021-02-11 16:01 UTC (permalink / raw)
  To: grub-devel, Heinrich Schuchardt

Am 11.02.2021 um 15:04 schrieb Heinrich Schuchardt:
> On 11.02.21 13:50, Michael Lawnick wrote:
>> Hi,
>> Several boot parameters are stored in our SPI flash to configure the
>> systems for different use cases.
>
> I still do not understand why you need access in GRUB. You can read and
> write the SPI flash in U-Boot on your embedded system.

In our latest system we get U-Boot from vendor. We try to change nothing
in it. To execute our boot management we let start GRUB as an EFI app.
The boot management needs configuration information which depends on
customer deployment scenario saved in SPI flash. It is our boot
management S/W which uses the EFI_SPI_NOR interface.

> Maybe you could provide a short example script showing how the new
> command would fit into the GRUB boot flow.

The spi_nor command is more intended as an example application and for
test, not intended for real use. I have no problem to deactivate it by
default (put it to separate config module) or even completely remove it.
My real point is the driver to abstract away the H/W and take EFI into
use if available.

KR
Michael



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] efi: SPI NOR flash support
  2021-02-11 16:01         ` Michael Lawnick
@ 2021-02-11 17:16           ` Heinrich Schuchardt
  2021-02-12  9:17             ` Michael Lawnick
  0 siblings, 1 reply; 12+ messages in thread
From: Heinrich Schuchardt @ 2021-02-11 17:16 UTC (permalink / raw)
  To: The development of GNU GRUB

On 11.02.21 17:01, Michael Lawnick wrote:
> Am 11.02.2021 um 15:04 schrieb Heinrich Schuchardt:
>> On 11.02.21 13:50, Michael Lawnick wrote:
>>> Hi,
>>> Several boot parameters are stored in our SPI flash to configure the
>>> systems for different use cases.
>>
>> I still do not understand why you need access in GRUB. You can read and
>> write the SPI flash in U-Boot on your embedded system.
>
> In our latest system we get U-Boot from vendor. We try to change nothing
> in it. To execute our boot management we let start GRUB as an EFI app.
> The boot management needs configuration information which depends on
> customer deployment scenario saved in SPI flash. It is our boot
> management S/W which uses the EFI_SPI_NOR interface.

You should consider upstreaming your board to mainline U-Boot.

>
>> Maybe you could provide a short example script showing how the new
>> command would fit into the GRUB boot flow.
>
> The spi_nor command is more intended as an example application and for
> test, not intended for real use. I have no problem to deactivate it by
> default (put it to separate config module) or even completely remove it.
> My real point is the driver to abstract away the H/W and take EFI into
> use if available.

U-Boot does not provide the EFI_SPI_NOR_FLASH_PROTOCOL. How will you
ever use the spi_nor command?

How about creating a UEFI application that can run

* in the UEFI shell
* directly from the UEFI boot manager
* from GRUB EFI

and upstreaming that utility to Tianocore EDK II which already has the
protocol that you want to consume?

Best regards

Heinrich


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] efi: SPI NOR flash support
  2021-02-11 17:16           ` Heinrich Schuchardt
@ 2021-02-12  9:17             ` Michael Lawnick
  0 siblings, 0 replies; 12+ messages in thread
From: Michael Lawnick @ 2021-02-12  9:17 UTC (permalink / raw)
  To: Heinrich Schuchardt, The development of GNU GRUB

Am 11.02.2021 um 18:16 schrieb Heinrich Schuchardt:
> On 11.02.21 17:01, Michael Lawnick wrote:
>> Am 11.02.2021 um 15:04 schrieb Heinrich Schuchardt:
>>> On 11.02.21 13:50, Michael Lawnick wrote:
>>>> Hi,
>>>> Several boot parameters are stored in our SPI flash to configure the
>>>> systems for different use cases.
>>>
>>> I still do not understand why you need access in GRUB. You can read and
>>> write the SPI flash in U-Boot on your embedded system.
>>
>> In our latest system we get U-Boot from vendor. We try to change nothing
>> in it. To execute our boot management we let start GRUB as an EFI app.
>> The boot management needs configuration information which depends on
>> customer deployment scenario saved in SPI flash. It is our boot
>> management S/W which uses the EFI_SPI_NOR interface.
>
> You should consider upstreaming your board to mainline U-Boot.

This is in responsibility of the SoC vendor and as far as I know part of
contract.

>>
>>> Maybe you could provide a short example script showing how the new
>>> command would fit into the GRUB boot flow.
>>
>> The spi_nor command is more intended as an example application and for
>> test, not intended for real use. I have no problem to deactivate it by
>> default (put it to separate config module) or even completely remove it.
>> My real point is the driver to abstract away the H/W and take EFI into
>> use if available.
>
> U-Boot does not provide the EFI_SPI_NOR_FLASH_PROTOCOL. How will you
> ever use the spi_nor command?

After upstreaming of current version it will be.

> How about creating a UEFI application that can run
>
> * in the UEFI shell
> * directly from the UEFI boot manager
> * from GRUB EFI
>
> and upstreaming that utility to Tianocore EDK II which already has the
> protocol that you want to consume?

Even Tianocore depends on support by underlying boot software.
No gain.

All in all you vote against adding more EFI support to make GRUB more
H/W independent?
Whats about EFI-PCI which exists only in form of *.h files?

KR
Michael



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] efi: SPI NOR flash command line
  2021-02-05  9:02 ` [PATCH v2 2/2] efi: SPI NOR flash command line Michael Lawnick
@ 2021-02-12  9:20   ` Michael Lawnick
  2021-02-16 13:28     ` Paul Menzel
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Lawnick @ 2021-02-12  9:20 UTC (permalink / raw)
  To: The development of GNU GRUB


Note: There is a bug in code below which will let this code fail on
BIOS-UEFI and requires a v3 as soon as basic discussion whether this
sort of functionality is accepted at all is done.
I don't want to disturb thread by a v3 thread.

KR
Michael

Am 05.02.2021 um 10:02 schrieb Michael Lawnick:
> Add SPI NOR flash to command line
> Based on patch '[PATCH 1/2] efi: SPI NOR flash support'
> add command line functionality for interactive access
> to SPI NOR flash.
>
> Supported commands:
> spi_nor
>        init  - establish communication to a flash part
>        read  - read from flash part to memory or print hexdump
>        write - write to flash part from memory
>        erase - erase some erase blocks
>
> Signed-off-by: Michael Lawnick <michael.lawnick@nokia.com>
> ---
> [Patch v2 2/2]: no change
> ---
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 4d775e5f6..403a5432f 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -659,6 +659,7 @@ module = {
>    module = {
>      name = efi_spi_nor;
>      common = bus/spi/efi_spi_nor.c;
> +  common = commands/efi/spinorcmd.c;
>      enable = efi;
>    };
>
> diff --git a/grub-core/commands/efi/spinorcmd.c
> b/grub-core/commands/efi/spinorcmd.c
> new file mode 100644
> index 000000000..c55a900aa
> --- /dev/null
> +++ b/grub-core/commands/efi/spinorcmd.c
> @@ -0,0 +1,253 @@
> +/*  spinorcmd.c  - Give access to SPI NOR flash on command line.
> + *  Copyright 2021 Nokia
> + *  Licensed under the GNU General Public License v3.0 only
> + *  SPDX-License-Identifier: GPL-3.0-only
> + *
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2008  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <grub/types.h>
> +#include <grub/misc.h>
> +#include <grub/mm.h>
> +#include <grub/efi/api.h>
> +#include <grub/efi/efi.h>
> +#include <grub/efi/efi_spi_nor.h>
> +#include <grub/command.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static void *cli_spi_flash_device = NULL;
> +
> +static void
> +usage (void)
> +{
> +	grub_printf("spi_nor - access SPI NOR flash through UEFI API\n");
> +	grub_printf("Usage:\n");
> +	grub_printf("  spi_nor init|read|write|format <args>\n");
> +	grub_printf("      init [-id <id0 id1 id2>] [-cnt <count>]\n");
> +	grub_printf("             to be called once before operation on a
> device.\n");
> +	grub_printf("             <id>    : optional up to 3 bytes flash
> identifier\n");
> +	grub_printf("                       to match against\n");
> +	grub_printf("             <count> : use n-th occurance of device\n");
> +	grub_printf("                       (can be combined with <id>)\n");
> +	grub_printf("      read <offset> <bytes> [<addr>]\n");
> +	grub_printf("             read and dump/save bytes\n");
> +	grub_printf("      write <offset> <bytes> <addr>\n");
> +	grub_printf("             write bytes from <addr> to flash\n");
> +	grub_printf("      erase <offset> <bytes>\n");
> +	grub_printf("             format area \n");
> +	grub_printf("             (<offset> and <bytes> must be erase block
> alligned)\n");
> +	grub_printf("\n");
> +}
> +
> +/*  get_init_args - return index and number of -id and/or -cnt arguments
> +	handle 4 possible inputs:
> +	- spi_nor init -id <id0> <id1> -cnt <count>
> +	- spi_nor init -cnt <count> -id <id0>
> +	- spi_nor init -cnt <count>
> +	- spi_nor init -id <id0> <id1> <id2>
> +*/
> +static int get_init_args(int argc, char **args, int *id_idx, int
> *id_cnt, int *cnt_idx)
> +{
> +	int opt_idx = 1;
> +	*id_idx = 0;
> +	*cnt_idx = 0;
> +	*id_cnt = 0;
> +
> +	while (opt_idx < argc) {
> +		if (!grub_strcmp(args[opt_idx],"-id")) {
> +			opt_idx++;
> +			*id_idx = opt_idx;
> +
> +			while ((opt_idx < argc)
> +					&& (grub_strcmp(args[opt_idx], "-cnt"))
> +					&& (*id_cnt < 3)) {
> +				opt_idx++;
> +				*id_cnt = *id_cnt + 1;
> +			}
> +
> +			if (*id_cnt == 0)
> +				return 1;
> +		} else if (!grub_strcmp(args[opt_idx],"-cnt")) {
> +			if (argc > opt_idx + 1) {
> +				*cnt_idx = opt_idx + 1;
> +				opt_idx += 2;
> +			} else {
> +				return 1;
> +			}
> +		} else {
> +			return 1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static grub_err_t
> +grub_cmd_spi_nor (grub_command_t cmd __attribute__ ((unused)),
> +		    int argc __attribute__ ((unused)),
> +		    char **args __attribute__ ((unused)))
> +{
> +	grub_err_t ret;
> +	int cnt_idx, id_idx, id_cnt = 0;
> +	grub_uint8_t *device_id = NULL, devId[3];
> +	grub_uint32_t instance = 0;
> +
> +	if (argc < 1) {
> +		grub_printf("Missing argument\n");
> +		usage();
> +		return GRUB_ERR_BAD_ARGUMENT;
> +	}
> +
> +	if (!grub_strcmp("init", args[0])) {
> +		if (argc != 1) {
> +			if (get_init_args(argc, args, &id_idx, &id_cnt, &cnt_idx)) {
> +				usage();
> +				return GRUB_ERR_BAD_ARGUMENT;
> +			} else {
> +				if (id_idx != 0) {
> +					for (int i=0; i<id_cnt; i++)
> +						devId[i] = grub_strtoul(args[id_idx + i], NULL, 0);
> +					device_id = devId;
> +				}
> +				if (cnt_idx != 0) {
> +					instance = grub_strtoul(args[cnt_idx], NULL, 0);
> +				}
> +			}
> +		}
> +
> +		cli_spi_flash_device = grub_efi_spi_nor_init(device_id, id_cnt,
> instance);
> +		if (cli_spi_flash_device == NULL) {
> +			grub_printf("No SPI NOR flash found\n");
> +			return GRUB_ERR_UNKNOWN_DEVICE;
> +		}
> +		grub_printf("Found dev %x, capacity 0x%x bytes, erase block size 0x%x\n",
> +					grub_efi_spi_nor_device_id(cli_spi_flash_device),
> +					grub_efi_spi_nor_flash_size(cli_spi_flash_device),
> +					grub_efi_spi_nor_erase_block_size(cli_spi_flash_device));
> +		return GRUB_ERR_NONE;
> +	}
> +
> +	if (cli_spi_flash_device == NULL) {
> +		grub_printf("No known device. Call 'init' first\n");
> +		usage();
> +		return GRUB_ERR_UNKNOWN_DEVICE;
> +	}
> +
> +	if (!grub_strcmp("read", args[0])) {
> +		grub_uint8_t *data;
> +		grub_uint32_t offset, num_bytes, i, j;
> +
> +		if (argc < 3) {
> +			grub_printf("Missing parameters\n");
> +			usage();
> +			return GRUB_ERR_BAD_ARGUMENT;
> +		}
> +
> +		offset = grub_strtoul(args[1], NULL, 0);
> +		num_bytes = grub_strtoul(args[2], NULL, 0);
> +
> +		if (argc == 4) {
> +			data = (grub_uint8_t *)grub_strtoul(args[3], NULL, 0);
> +			if (data == NULL) {
> +				grub_printf("Bad memory pointer, 0 not supported.\n");
> +				usage();
> +				return GRUB_ERR_BAD_ARGUMENT;
> +			}
> +		} else {
> +			data = grub_malloc(num_bytes);
> +			if (data == NULL) {
> +				grub_printf("Out of memory.\n");
> +				usage();
> +				return GRUB_ERR_OUT_OF_MEMORY;
> +			}
> +		}
> +
> +		ret = grub_efi_spi_nor_read(cli_spi_flash_device, data, offset,
> num_bytes);
> +		if (ret != GRUB_ERR_NONE)
> +			return ret;
> +
> +		if (argc == 3) {
> +			for (i=0; i<num_bytes; i+=16) {
> +				grub_printf("0x%06x: ", i + offset);
> +				for (j=0; (j<16) && (i+j<num_bytes); j++)
> +					grub_printf("%02x ", data[i+j]);
> +				grub_printf("\n");
> +			}
> +			grub_free(data);
> +		}
> +		return GRUB_ERR_NONE;
> +	}
> +
> +	if (!grub_strcmp("write", args[0])) {
> +		grub_uint8_t *data;
> +		grub_uint32_t offset, num_bytes;
> +
> +		if (argc != 4) {
> +			grub_printf("Wrong number of parameters\n");
> +			usage();
> +			return GRUB_ERR_BAD_ARGUMENT;
> +		}
> +
> +		offset = grub_strtoul(args[1], NULL, 0);
> +		num_bytes = grub_strtoul(args[2], NULL, 0);
> +
> +		data = (grub_uint8_t *)grub_strtoul(args[3], NULL, 0);
> +		if (data == NULL) {
> +			grub_printf("Bad memory pointer, 0 not supported.\n");
> +			usage();
> +			return GRUB_ERR_BAD_ARGUMENT;
> +		}
> +
> +		ret = grub_efi_spi_nor_write(cli_spi_flash_device, data, offset,
> num_bytes);
> +
> +		return ret;
> +	}
> +
> +	if (!grub_strcmp("erase", args[0])) {
> +		grub_uint32_t offset, num_bytes;
> +
> +		if (argc != 3) {
> +			grub_printf("Wrong number of parameters\n");
> +			usage();
> +			return GRUB_ERR_BAD_ARGUMENT;
> +		}
> +
> +		offset = grub_strtoul(args[1], NULL, 0);
> +		num_bytes = grub_strtoul(args[2], NULL, 0);
> +
> +		ret = grub_efi_spi_nor_erase(cli_spi_flash_device, offset, num_bytes);
> +
> +		return ret;
> +	}
> +
> +	grub_printf("Unknown command \"%s\"\n", args[1]);
> +	usage();
> +	return GRUB_ERR_BAD_ARGUMENT;
> +}
> +
> +static grub_command_t cmd;
> +
> +GRUB_MOD_INIT(spinorcmd)
> +{
> +	cmd = grub_register_command("spi_nor", grub_cmd_spi_nor,
> +			       "", "access SPI NOR flash");
> +}
> +
> +GRUB_MOD_FINI(spinorcmd)
> +{
> +	grub_unregister_command(cmd);
> +}
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] efi: SPI NOR flash command line
  2021-02-12  9:20   ` Michael Lawnick
@ 2021-02-16 13:28     ` Paul Menzel
  0 siblings, 0 replies; 12+ messages in thread
From: Paul Menzel @ 2021-02-16 13:28 UTC (permalink / raw)
  To: Michael Lawnick; +Cc: The development of GNU GRUB

Dear Michael,


Am 12.02.21 um 10:20 schrieb Michael Lawnick:

> Note: There is a bug in code below which will let this code fail on
> BIOS-UEFI and requires a v3 as soon as basic discussion whether this
> sort of functionality is accepted at all is done.
> I don't want to disturb thread by a v3 thread.

Thank you for preparing and upstreaming the patches.

I think, I read, that the maintainers should be added to Cc for patches, 
but as written, and as seen from the thread *RFC: Grub project 
management*, GRUB definitely has some resource problems. So, if you 
could send the v3 with the maintainers in Cc, that would be great.

[…]

> Am 05.02.2021 um 10:02 schrieb Michael Lawnick:
>> Add SPI NOR flash to command line
>> Based on patch '[PATCH 1/2] efi: SPI NOR flash support'
>> add command line functionality for interactive access
>> to SPI NOR flash.
>>
>> Supported commands:
>> spi_nor
>>        init  - establish communication to a flash part
>>        read  - read from flash part to memory or print hexdump
>>        write - write to flash part from memory
>>        erase - erase some erase blocks

Please name the system (board and firmware) you tested this with, and 
maybe give an example, how to test this. Is QEMU able to emulate a SPI 
NOR flash chip?

>> Signed-off-by: Michael Lawnick <michael.lawnick@nokia.com>
>> ---
>> [Patch v2 2/2]: no change
>> ---
>> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
>> index 4d775e5f6..403a5432f 100644
>> --- a/grub-core/Makefile.core.def
>> +++ b/grub-core/Makefile.core.def
>> @@ -659,6 +659,7 @@ module = {
>>    module = {
>>      name = efi_spi_nor;
>>      common = bus/spi/efi_spi_nor.c;
>> +  common = commands/efi/spinorcmd.c;
>>      enable = efi;
>>    };
>>
>> diff --git a/grub-core/commands/efi/spinorcmd.c
>> b/grub-core/commands/efi/spinorcmd.c
>> new file mode 100644
>> index 000000000..c55a900aa
>> --- /dev/null
>> +++ b/grub-core/commands/efi/spinorcmd.c
>> @@ -0,0 +1,253 @@
>> +/*  spinorcmd.c  - Give access to SPI NOR flash on command line.
>> + *  Copyright 2021 Nokia
>> + *  Licensed under the GNU General Public License v3.0 only
>> + *  SPDX-License-Identifier: GPL-3.0-only
>> + *
>> + *  GRUB  --  GRand Unified Bootloader
>> + *  Copyright (C) 2008  Free Software Foundation, Inc.
>> + *
>> + *  GRUB is free software: you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation, either version 3 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  GRUB is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include <grub/types.h>
>> +#include <grub/misc.h>
>> +#include <grub/mm.h>
>> +#include <grub/efi/api.h>
>> +#include <grub/efi/efi.h>
>> +#include <grub/efi/efi_spi_nor.h>
>> +#include <grub/command.h>
>> +
>> +GRUB_MOD_LICENSE ("GPLv3+");
>> +
>> +static void *cli_spi_flash_device = NULL;
>> +
>> +static void
>> +usage (void)
>> +{
>> +    grub_printf("spi_nor - access SPI NOR flash through UEFI API\n");
>> +    grub_printf("Usage:\n");
>> +    grub_printf("  spi_nor init|read|write|format <args>\n");
>> +    grub_printf("      init [-id <id0 id1 id2>] [-cnt <count>]\n");
>> +    grub_printf("             to be called once before operation on a device.\n");
>> +    grub_printf("             <id>    : optional up to 3 bytes flash identifier\n");
>> +    grub_printf("                       to match against\n");
>> +    grub_printf("             <count> : use n-th occurance of  device\n");
>> +    grub_printf("                       (can be combined with <id>)\n");
>> +    grub_printf("      read <offset> <bytes> [<addr>]\n");
>> +    grub_printf("             read and dump/save bytes\n");
>> +    grub_printf("      write <offset> <bytes> <addr>\n");
>> +    grub_printf("             write bytes from <addr> to flash\n");
>> +    grub_printf("      erase <offset> <bytes>\n");
>> +    grub_printf("             format area \n");
>> +    grub_printf("             (<offset> and <bytes> must be erase block alligned)\n");

aligned

>> +    grub_printf("\n");
>> +}
>> +
>> +/*  get_init_args - return index and number of -id and/or -cnt arguments
>> +    handle 4 possible inputs:
>> +    - spi_nor init -id <id0> <id1> -cnt <count>
>> +    - spi_nor init -cnt <count> -id <id0>
>> +    - spi_nor init -cnt <count>
>> +    - spi_nor init -id <id0> <id1> <id2>
>> +*/
>> +static int get_init_args(int argc, char **args, int *id_idx, int
>> *id_cnt, int *cnt_idx)
>> +{
>> +    int opt_idx = 1;
>> +    *id_idx = 0;
>> +    *cnt_idx = 0;
>> +    *id_cnt = 0;
>> +
>> +    while (opt_idx < argc) {
>> +        if (!grub_strcmp(args[opt_idx],"-id")) {

Please add a space after the comma.

>> +            opt_idx++;
>> +            *id_idx = opt_idx;
>> +
>> +            while ((opt_idx < argc)
>> +                    && (grub_strcmp(args[opt_idx], "-cnt"))
>> +                    && (*id_cnt < 3)) {
>> +                opt_idx++;
>> +                *id_cnt = *id_cnt + 1;
>> +            }
>> +
>> +            if (*id_cnt == 0)
>> +                return 1;
>> +        } else if (!grub_strcmp(args[opt_idx],"-cnt")) {
>> +            if (argc > opt_idx + 1) {
>> +                *cnt_idx = opt_idx + 1;
>> +                opt_idx += 2;
>> +            } else {
>> +                return 1;
>> +            }
>> +        } else {
>> +            return 1;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static grub_err_t
>> +grub_cmd_spi_nor (grub_command_t cmd __attribute__ ((unused)),
>> +            int argc __attribute__ ((unused)),
>> +            char **args __attribute__ ((unused)))
>> +{
>> +    grub_err_t ret;
>> +    int cnt_idx, id_idx, id_cnt = 0;
>> +    grub_uint8_t *device_id = NULL, devId[3];
>> +    grub_uint32_t instance = 0;
>> +
>> +    if (argc < 1) {
>> +        grub_printf("Missing argument\n");
>> +        usage();
>> +        return GRUB_ERR_BAD_ARGUMENT;
>> +    }
>> +
>> +    if (!grub_strcmp("init", args[0])) {
>> +        if (argc != 1) {
>> +            if (get_init_args(argc, args, &id_idx, &id_cnt, &cnt_idx)) {
>> +                usage();
>> +                return GRUB_ERR_BAD_ARGUMENT;
>> +            } else {
>> +                if (id_idx != 0) {
>> +                    for (int i=0; i<id_cnt; i++)
>> +                        devId[i] = grub_strtoul(args[id_idx + i], NULL, 0);
>> +                    device_id = devId;
>> +                }
>> +                if (cnt_idx != 0) {
>> +                    instance = grub_strtoul(args[cnt_idx], NULL, 0);
>> +                }
>> +            }
>> +        }
>> +
>> +        cli_spi_flash_device = grub_efi_spi_nor_init(device_id, id_cnt, instance);
>> +        if (cli_spi_flash_device == NULL) {
>> +            grub_printf("No SPI NOR flash found\n");
>> +            return GRUB_ERR_UNKNOWN_DEVICE;
>> +        }
>> +        grub_printf("Found dev %x, capacity 0x%x bytes, erase block size 0x%x\n",
>> +                    grub_efi_spi_nor_device_id(cli_spi_flash_device),
>> +                    grub_efi_spi_nor_flash_size(cli_spi_flash_device),
>> +                    grub_efi_spi_nor_erase_block_size(cli_spi_flash_device));
>> +        return GRUB_ERR_NONE;
>> +    }
>> +
>> +    if (cli_spi_flash_device == NULL) {
>> +        grub_printf("No known device. Call 'init' first\n");
>> +        usage();
>> +        return GRUB_ERR_UNKNOWN_DEVICE;
>> +    }
>> +
>> +    if (!grub_strcmp("read", args[0])) {
>> +        grub_uint8_t *data;
>> +        grub_uint32_t offset, num_bytes, i, j;
>> +
>> +        if (argc < 3) {
>> +            grub_printf("Missing parameters\n");
>> +            usage();
>> +            return GRUB_ERR_BAD_ARGUMENT;
>> +        }
>> +
>> +        offset = grub_strtoul(args[1], NULL, 0);
>> +        num_bytes = grub_strtoul(args[2], NULL, 0);
>> +
>> +        if (argc == 4) {
>> +            data = (grub_uint8_t *)grub_strtoul(args[3], NULL, 0);
>> +            if (data == NULL) {
>> +                grub_printf("Bad memory pointer, 0 not supported.\n");
>> +                usage();
>> +                return GRUB_ERR_BAD_ARGUMENT;
>> +            }
>> +        } else {
>> +            data = grub_malloc(num_bytes);
>> +            if (data == NULL) {
>> +                grub_printf("Out of memory.\n");
>> +                usage();
>> +                return GRUB_ERR_OUT_OF_MEMORY;
>> +            }
>> +        }
>> +
>> +        ret = grub_efi_spi_nor_read(cli_spi_flash_device, data, offset, num_bytes);
>> +        if (ret != GRUB_ERR_NONE)
>> +            return ret;
>> +
>> +        if (argc == 3) {
>> +            for (i=0; i<num_bytes; i+=16) {
>> +                grub_printf("0x%06x: ", i + offset);
>> +                for (j=0; (j<16) && (i+j<num_bytes); j++)
>> +                    grub_printf("%02x ", data[i+j]);
>> +                grub_printf("\n");
>> +            }
>> +            grub_free(data);
>> +        }
>> +        return GRUB_ERR_NONE;
>> +    }
>> +
>> +    if (!grub_strcmp("write", args[0])) {
>> +        grub_uint8_t *data;
>> +        grub_uint32_t offset, num_bytes;
>> +
>> +        if (argc != 4) {
>> +            grub_printf("Wrong number of parameters\n");
>> +            usage();
>> +            return GRUB_ERR_BAD_ARGUMENT;
>> +        }
>> +
>> +        offset = grub_strtoul(args[1], NULL, 0);
>> +        num_bytes = grub_strtoul(args[2], NULL, 0);
>> +
>> +        data = (grub_uint8_t *)grub_strtoul(args[3], NULL, 0);
>> +        if (data == NULL) {
>> +            grub_printf("Bad memory pointer, 0 not supported.\n");
>> +            usage();
>> +            return GRUB_ERR_BAD_ARGUMENT;
>> +        }
>> +
>> +        ret = grub_efi_spi_nor_write(cli_spi_flash_device, data, offset, num_bytes);
>> +
>> +        return ret;
>> +    }
>> +
>> +    if (!grub_strcmp("erase", args[0])) {
>> +        grub_uint32_t offset, num_bytes;
>> +
>> +        if (argc != 3) {
>> +            grub_printf("Wrong number of parameters\n");
>> +            usage();
>> +            return GRUB_ERR_BAD_ARGUMENT;
>> +        }
>> +
>> +        offset = grub_strtoul(args[1], NULL, 0);
>> +        num_bytes = grub_strtoul(args[2], NULL, 0);
>> +
>> +        ret = grub_efi_spi_nor_erase(cli_spi_flash_device, offset, num_bytes);
>> +
>> +        return ret;
>> +    }
>> +
>> +    grub_printf("Unknown command \"%s\"\n", args[1]);
>> +    usage();
>> +    return GRUB_ERR_BAD_ARGUMENT;
>> +}
>> +
>> +static grub_command_t cmd;
>> +
>> +GRUB_MOD_INIT(spinorcmd)
>> +{
>> +    cmd = grub_register_command("spi_nor", grub_cmd_spi_nor,
>> +                   "", "access SPI NOR flash");
>> +}
>> +
>> +GRUB_MOD_FINI(spinorcmd)
>> +{
>> +    grub_unregister_command(cmd);
>> +}

Sorry, I just to an superficial review, and the things commented are 
only nits (style issues).


Kind regards,

Paul


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-02-16 13:28 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-05  8:58 [PATCH v2 1/2] efi: SPI NOR flash support Michael Lawnick
2021-02-05  9:02 ` [PATCH v2 2/2] efi: SPI NOR flash command line Michael Lawnick
2021-02-12  9:20   ` Michael Lawnick
2021-02-16 13:28     ` Paul Menzel
2021-02-11  7:36 ` [PATCH v2 1/2] efi: SPI NOR flash support Michael Lawnick
2021-02-11  7:36   ` Paul Menzel
2021-02-11  8:51   ` Heinrich Schuchardt
2021-02-11 12:50     ` Michael Lawnick
2021-02-11 14:04       ` Heinrich Schuchardt
2021-02-11 16:01         ` Michael Lawnick
2021-02-11 17:16           ` Heinrich Schuchardt
2021-02-12  9:17             ` Michael Lawnick

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.