* [RFC PATCH 1/6] block: partitions: efi: Move efi.h header to include/linux/gpt.h
2023-12-11 15:12 [RFC PATCH 0/6] Add GPT parser to MTD layer Romain Gantois
@ 2023-12-11 15:12 ` Romain Gantois
2023-12-11 15:12 ` [RFC PATCH 2/6] block: partitions: efi: Fix some style issues Romain Gantois
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Romain Gantois @ 2023-12-11 15:12 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jens Axboe,
Davidlohr Bueso, Ard Biesheuvel
Cc: Romain Gantois, Thomas Petazzoni, Herve Codina, linux-mtd,
devicetree, linux-kernel, linux-block, linux-efi
The GPT parser located in the block layer defines all of the GPT-specific
data structures and logic that are necessary to parse this kind of
partition table. However, it is also specifically tailored for block
devices.
Assuming that different GPT parsers will be implemented in other kernel
subsystems, it is desirable to create a common set of GPT struct
definitions, macros and helpers, so as to limit code reuse between parsers.
As a first step towards this common codebase, this commit moves the efi.h
header, that contains GPT-specific definitions, to include/linux/gpt.h.
There is no functional change.
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
MAINTAINERS | 3 ++-
block/partitions/efi.c | 2 +-
block/partitions/msdos.c | 2 +-
block/partitions/efi.h => include/linux/gpt.h | 0
4 files changed, 4 insertions(+), 3 deletions(-)
rename block/partitions/efi.h => include/linux/gpt.h (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 97f51d5ec1cf..22e37e2ea1ae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9233,7 +9233,8 @@ GUID PARTITION TABLE (GPT)
M: Davidlohr Bueso <dave@stgolabs.net>
L: linux-efi@vger.kernel.org
S: Maintained
-F: block/partitions/efi.*
+F: block/partitions/efi.c
+F: include/linux/gpt.h
HABANALABS PCI DRIVER
M: Oded Gabbay <ogabbay@kernel.org>
diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index 5e9be13a56a8..db50c3f2bab3 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -87,8 +87,8 @@
#include <linux/ctype.h>
#include <linux/math64.h>
#include <linux/slab.h>
+#include <linux/gpt.h>
#include "check.h"
-#include "efi.h"
/* This allows a kernel command line option 'gpt' to override
* the test for invalid PMBR. Not __initdata because reloading
diff --git a/block/partitions/msdos.c b/block/partitions/msdos.c
index b5d5c229cc3b..d0376cf27448 100644
--- a/block/partitions/msdos.c
+++ b/block/partitions/msdos.c
@@ -27,9 +27,9 @@
*/
#include <linux/msdos_fs.h>
#include <linux/msdos_partition.h>
+#include <linux/gpt.h>
#include "check.h"
-#include "efi.h"
/*
* Many architectures don't like unaligned accesses, while
diff --git a/block/partitions/efi.h b/include/linux/gpt.h
similarity index 100%
rename from block/partitions/efi.h
rename to include/linux/gpt.h
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [RFC PATCH 2/6] block: partitions: efi: Fix some style issues
2023-12-11 15:12 [RFC PATCH 0/6] Add GPT parser to MTD layer Romain Gantois
2023-12-11 15:12 ` [RFC PATCH 1/6] block: partitions: efi: Move efi.h header to include/linux/gpt.h Romain Gantois
@ 2023-12-11 15:12 ` Romain Gantois
2023-12-12 0:57 ` Davidlohr Bueso
2023-12-11 15:12 ` [RFC PATCH 3/6] block: partitions: efi: Separate out GPT-specific code Romain Gantois
` (4 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Romain Gantois @ 2023-12-11 15:12 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jens Axboe,
Davidlohr Bueso, Ard Biesheuvel
Cc: Romain Gantois, Thomas Petazzoni, Herve Codina, linux-mtd,
devicetree, linux-kernel, linux-block, linux-efi
The block layer EFI code is quite old and does not perfectly match the
current kernel coding style. Fix some indentation and trailing whitespace
issues in efi.c.
There is no functional change.
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
block/partitions/efi.c | 181 ++++++++++++++++++++---------------------
include/linux/gpt.h | 35 ++++----
2 files changed, 107 insertions(+), 109 deletions(-)
diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index db50c3f2bab3..bac514a62d61 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -23,7 +23,7 @@
* - Ported to 2.5.7-pre1 and 2.5.7-dj2
* - Applied patch to avoid fault in alternate header handling
* - cleaned up find_valid_gpt
- * - On-disk structure and copy in memory is *always* LE now -
+ * - On-disk structure and copy in memory is *always* LE now -
* swab fields as needed
* - remove print_gpt_header()
* - only use first max_p partition entries, to keep the kernel minor number
@@ -40,7 +40,7 @@
* - moved le_efi_guid_to_cpus() back into this file. GPT is the only
* thing that keeps EFI GUIDs on disk.
* - Changed gpt structure names and members to be simpler and more Linux-like.
- *
+ *
* Wed Oct 17 2001 Matt Domsch <Matt_Domsch@dell.com>
* - Removed CONFIG_DEVFS_VOLUMES_UUID code entirely per Martin Wilck
*
@@ -65,7 +65,7 @@
*
* Wed Jun 6 2001 Martin Wilck <Martin.Wilck@Fujitsu-Siemens.com>
* - added devfs volume UUID support (/dev/volumes/uuids) for
- * mounting file systems by the partition GUID.
+ * mounting file systems by the partition GUID.
*
* Tue Dec 5 2000 Matt Domsch <Matt_Domsch@dell.com>
* - Moved crc32() to linux/lib, added efi_crc32().
@@ -103,14 +103,13 @@ force_gpt_fn(char *str)
}
__setup("gpt", force_gpt_fn);
-
/**
* efi_crc32() - EFI version of crc32 function
* @buf: buffer to calculate crc32 of
* @len: length of buf
*
* Description: Returns EFI-style CRC32 value for @buf
- *
+ *
* This function uses the little endian Ethernet polynomial
* but seeds the function with ~0, and xor's with ~0 at the end.
* Note, the EFI Specification, v1.02, has a reference to
@@ -125,7 +124,7 @@ efi_crc32(const void *buf, unsigned long len)
/**
* last_lba(): return number of last logical block of device
* @disk: block device
- *
+ *
* Description: Returns last LBA value on success, 0 on error.
* This is stored (by sd and ide-geometry) in
* the part[0] entry for this disk, and is the number of
@@ -194,9 +193,9 @@ static int is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors)
goto done;
check_hybrid:
for (i = 0; i < 4; i++)
- if ((mbr->partition_record[i].os_type !=
- EFI_PMBR_OSTYPE_EFI_GPT) &&
- (mbr->partition_record[i].os_type != 0x00))
+ if (mbr->partition_record[i].os_type !=
+ EFI_PMBR_OSTYPE_EFI_GPT &&
+ mbr->partition_record[i].os_type != 0x00)
ret = GPT_MBR_HYBRID;
/*
@@ -213,7 +212,7 @@ static int is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors)
*/
if (ret == GPT_MBR_PROTECTIVE) {
sz = le32_to_cpu(mbr->partition_record[part].size_in_lba);
- if (sz != (uint32_t) total_sectors - 1 && sz != 0xFFFFFFFF)
+ if (sz != (uint32_t)total_sectors - 1 && sz != 0xFFFFFFFF)
pr_debug("GPT: mbr size in lba (%u) different than whole disk (%u).\n",
sz, min_t(uint32_t,
total_sectors - 1, 0xFFFFFFFF));
@@ -235,17 +234,19 @@ static int is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors)
static size_t read_lba(struct parsed_partitions *state,
u64 lba, u8 *buffer, size_t count)
{
- size_t totalreadcount = 0;
sector_t n = lba *
(queue_logical_block_size(state->disk->queue) / 512);
+ size_t totalreadcount = 0;
+ unsigned char *data;
+ Sector sect;
+ int copied;
if (!buffer || lba > last_lba(state->disk))
- return 0;
+ return 0;
while (count) {
- int copied = 512;
- Sector sect;
- unsigned char *data = read_part_sector(state, n++, §);
+ copied = 512;
+ data = read_part_sector(state, n++, §);
if (!data)
break;
if (copied > count)
@@ -253,7 +254,7 @@ static size_t read_lba(struct parsed_partitions *state,
memcpy(buffer, data, copied);
put_dev_sector(sect);
buffer += copied;
- totalreadcount +=copied;
+ totalreadcount += copied;
count -= copied;
}
return totalreadcount;
@@ -263,7 +264,7 @@ static size_t read_lba(struct parsed_partitions *state,
* alloc_read_gpt_entries(): reads partition entries from disk
* @state: disk parsed partitions
* @gpt: GPT header
- *
+ *
* Description: Returns ptes on success, NULL on error.
* Allocates space for PTEs based on information found in @gpt.
* Notes: remember to free pte when you're done!
@@ -271,14 +272,14 @@ static size_t read_lba(struct parsed_partitions *state,
static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state,
gpt_header *gpt)
{
- size_t count;
gpt_entry *pte;
+ size_t count;
if (!gpt)
return NULL;
count = (size_t)le32_to_cpu(gpt->num_partition_entries) *
- le32_to_cpu(gpt->sizeof_partition_entry);
+ le32_to_cpu(gpt->sizeof_partition_entry);
if (!count)
return NULL;
pte = kmalloc(count, GFP_KERNEL);
@@ -286,9 +287,9 @@ static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state,
return NULL;
if (read_lba(state, le64_to_cpu(gpt->partition_entry_lba),
- (u8 *) pte, count) < count) {
+ (u8 *)pte, count) < count) {
kfree(pte);
- pte=NULL;
+ pte = NULL;
return NULL;
}
return pte;
@@ -298,7 +299,7 @@ static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state,
* alloc_read_gpt_header(): Allocates GPT header, reads into it from disk
* @state: disk parsed partitions
* @lba: the Logical Block Address of the partition table
- *
+ *
* Description: returns GPT header on success, NULL on error. Allocates
* and fills a GPT header starting at @ from @state->disk.
* Note: remember to free gpt when finished with it.
@@ -306,16 +307,16 @@ static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state,
static gpt_header *alloc_read_gpt_header(struct parsed_partitions *state,
u64 lba)
{
+ unsigned int ssz = queue_logical_block_size(state->disk->queue);
gpt_header *gpt;
- unsigned ssz = queue_logical_block_size(state->disk->queue);
gpt = kmalloc(ssz, GFP_KERNEL);
if (!gpt)
return NULL;
- if (read_lba(state, lba, (u8 *) gpt, ssz) < ssz) {
+ if (read_lba(state, lba, (u8 *)gpt, ssz) < ssz) {
kfree(gpt);
- gpt=NULL;
+ gpt = NULL;
return NULL;
}
@@ -486,31 +487,31 @@ compare_gpts(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
if (le64_to_cpu(pgpt->my_lba) != le64_to_cpu(agpt->alternate_lba)) {
pr_warn("GPT:Primary header LBA != Alt. header alternate_lba\n");
pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(pgpt->my_lba),
- (unsigned long long)le64_to_cpu(agpt->alternate_lba));
+ (unsigned long long)le64_to_cpu(pgpt->my_lba),
+ (unsigned long long)le64_to_cpu(agpt->alternate_lba));
error_found++;
}
if (le64_to_cpu(pgpt->alternate_lba) != le64_to_cpu(agpt->my_lba)) {
pr_warn("GPT:Primary header alternate_lba != Alt. header my_lba\n");
pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(pgpt->alternate_lba),
- (unsigned long long)le64_to_cpu(agpt->my_lba));
+ (unsigned long long)le64_to_cpu(pgpt->alternate_lba),
+ (unsigned long long)le64_to_cpu(agpt->my_lba));
error_found++;
}
if (le64_to_cpu(pgpt->first_usable_lba) !=
- le64_to_cpu(agpt->first_usable_lba)) {
+ le64_to_cpu(agpt->first_usable_lba)) {
pr_warn("GPT:first_usable_lbas don't match.\n");
pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(pgpt->first_usable_lba),
- (unsigned long long)le64_to_cpu(agpt->first_usable_lba));
+ (unsigned long long)le64_to_cpu(pgpt->first_usable_lba),
+ (unsigned long long)le64_to_cpu(agpt->first_usable_lba));
error_found++;
}
if (le64_to_cpu(pgpt->last_usable_lba) !=
- le64_to_cpu(agpt->last_usable_lba)) {
+ le64_to_cpu(agpt->last_usable_lba)) {
pr_warn("GPT:last_usable_lbas don't match.\n");
pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(pgpt->last_usable_lba),
- (unsigned long long)le64_to_cpu(agpt->last_usable_lba));
+ (unsigned long long)le64_to_cpu(pgpt->last_usable_lba),
+ (unsigned long long)le64_to_cpu(agpt->last_usable_lba));
error_found++;
}
if (efi_guidcmp(pgpt->disk_guid, agpt->disk_guid)) {
@@ -518,27 +519,24 @@ compare_gpts(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
error_found++;
}
if (le32_to_cpu(pgpt->num_partition_entries) !=
- le32_to_cpu(agpt->num_partition_entries)) {
- pr_warn("GPT:num_partition_entries don't match: "
- "0x%x != 0x%x\n",
- le32_to_cpu(pgpt->num_partition_entries),
- le32_to_cpu(agpt->num_partition_entries));
+ le32_to_cpu(agpt->num_partition_entries)) {
+ pr_warn("GPT:num_partition_entries don't match: 0x%x != 0x%x\n",
+ le32_to_cpu(pgpt->num_partition_entries),
+ le32_to_cpu(agpt->num_partition_entries));
error_found++;
}
if (le32_to_cpu(pgpt->sizeof_partition_entry) !=
- le32_to_cpu(agpt->sizeof_partition_entry)) {
- pr_warn("GPT:sizeof_partition_entry values don't match: "
- "0x%x != 0x%x\n",
- le32_to_cpu(pgpt->sizeof_partition_entry),
- le32_to_cpu(agpt->sizeof_partition_entry));
+ le32_to_cpu(agpt->sizeof_partition_entry)) {
+ pr_warn("GPT:sizeof_partition_entry values don't match: 0x%x != 0x%x\n",
+ le32_to_cpu(pgpt->sizeof_partition_entry),
+ le32_to_cpu(agpt->sizeof_partition_entry));
error_found++;
}
if (le32_to_cpu(pgpt->partition_entry_array_crc32) !=
- le32_to_cpu(agpt->partition_entry_array_crc32)) {
- pr_warn("GPT:partition_entry_array_crc32 values don't match: "
- "0x%x != 0x%x\n",
- le32_to_cpu(pgpt->partition_entry_array_crc32),
- le32_to_cpu(agpt->partition_entry_array_crc32));
+ le32_to_cpu(agpt->partition_entry_array_crc32)) {
+ pr_warn("GPT:partition_entry_array_crc32 values don't match: 0x%x != 0x%x\n",
+ le32_to_cpu(pgpt->partition_entry_array_crc32),
+ le32_to_cpu(agpt->partition_entry_array_crc32));
error_found++;
}
if (le64_to_cpu(pgpt->alternate_lba) != lastlba) {
@@ -581,20 +579,22 @@ compare_gpts(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
gpt_entry **ptes)
{
+ sector_t total_sectors = get_capacity(state->disk);
int good_pgpt = 0, good_agpt = 0, good_pmbr = 0;
- gpt_header *pgpt = NULL, *agpt = NULL;
+ const struct block_device_operations *fops;
gpt_entry *pptes = NULL, *aptes = NULL;
- legacy_mbr *legacymbr;
+ gpt_header *pgpt = NULL, *agpt = NULL;
struct gendisk *disk = state->disk;
- const struct block_device_operations *fops = disk->fops;
- sector_t total_sectors = get_capacity(state->disk);
+ legacy_mbr *legacymbr;
u64 lastlba;
+ fops = disk->fops;
+
if (!ptes)
return 0;
lastlba = last_lba(state->disk);
- if (!force_gpt) {
+ if (!force_gpt) {
/* This will be added to the EFI Spec. per Intel after v1.02. */
legacymbr = kzalloc(sizeof(*legacymbr), GFP_KERNEL);
if (!legacymbr)
@@ -609,17 +609,17 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
pr_debug("Device has a %s MBR\n",
good_pmbr == GPT_MBR_PROTECTIVE ?
- "protective" : "hybrid");
+ "protective" : "hybrid");
}
good_pgpt = is_gpt_valid(state, GPT_PRIMARY_PARTITION_TABLE_LBA,
&pgpt, &pptes);
- if (good_pgpt)
+ if (good_pgpt)
good_agpt = is_gpt_valid(state,
le64_to_cpu(pgpt->alternate_lba),
&agpt, &aptes);
- if (!good_agpt && force_gpt)
- good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
+ if (!good_agpt && force_gpt)
+ good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
if (!good_agpt && force_gpt && fops->alternative_gpt_sector) {
sector_t agpt_sector;
@@ -631,39 +631,38 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
&agpt, &aptes);
}
- /* The obviously unsuccessful case */
- if (!good_pgpt && !good_agpt)
- goto fail;
+ /* The obviously unsuccessful case */
+ if (!good_pgpt && !good_agpt)
+ goto fail;
compare_gpts(pgpt, agpt, lastlba);
- /* The good cases */
- if (good_pgpt) {
- *gpt = pgpt;
- *ptes = pptes;
- kfree(agpt);
- kfree(aptes);
+ /* The good cases */
+ if (good_pgpt) {
+ *gpt = pgpt;
+ *ptes = pptes;
+ kfree(agpt);
+ kfree(aptes);
if (!good_agpt)
- pr_warn("Alternate GPT is invalid, using primary GPT.\n");
- return 1;
- }
- else if (good_agpt) {
- *gpt = agpt;
- *ptes = aptes;
- kfree(pgpt);
- kfree(pptes);
+ pr_warn("Alternate GPT is invalid, using primary GPT.\n");
+ return 1;
+ } else if (good_agpt) {
+ *gpt = agpt;
+ *ptes = aptes;
+ kfree(pgpt);
+ kfree(pptes);
pr_warn("Primary GPT is invalid, using alternate GPT.\n");
- return 1;
- }
+ return 1;
+ }
- fail:
- kfree(pgpt);
- kfree(agpt);
- kfree(pptes);
- kfree(aptes);
- *gpt = NULL;
- *ptes = NULL;
- return 0;
+fail:
+ kfree(pgpt);
+ kfree(agpt);
+ kfree(pptes);
+ kfree(aptes);
+ *gpt = NULL;
+ *ptes = NULL;
+ return 0;
}
/**
@@ -712,10 +711,10 @@ static void utf16_le_to_7bit(const __le16 *in, unsigned int size, u8 *out)
*/
int efi_partition(struct parsed_partitions *state)
{
+ unsigned int ssz = queue_logical_block_size(state->disk->queue) / 512;
gpt_header *gpt = NULL;
gpt_entry *ptes = NULL;
u32 i;
- unsigned ssz = queue_logical_block_size(state->disk->queue) / 512;
if (!find_valid_gpt(state, &gpt, &ptes) || !gpt || !ptes) {
kfree(gpt);
@@ -725,17 +724,17 @@ int efi_partition(struct parsed_partitions *state)
pr_debug("GUID Partition Table is valid! Yea!\n");
- for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < state->limit-1; i++) {
+ for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < state->limit - 1; i++) {
struct partition_meta_info *info;
- unsigned label_max;
+ unsigned int label_max;
u64 start = le64_to_cpu(ptes[i].starting_lba);
u64 size = le64_to_cpu(ptes[i].ending_lba) -
- le64_to_cpu(ptes[i].starting_lba) + 1ULL;
+ le64_to_cpu(ptes[i].starting_lba) + 1ULL;
if (!is_pte_valid(&ptes[i], last_lba(state->disk)))
continue;
- put_partition(state, i+1, start * ssz, size * ssz);
+ put_partition(state, i + 1, start * ssz, size * ssz);
/* If this is a RAID volume, tell md */
if (!efi_guidcmp(ptes[i].partition_type_guid, PARTITION_LINUX_RAID_GUID))
diff --git a/include/linux/gpt.h b/include/linux/gpt.h
index 84b9f36b9e47..633be6bc826c 100644
--- a/include/linux/gpt.h
+++ b/include/linux/gpt.h
@@ -4,7 +4,7 @@
* Per Intel EFI Specification v1.02
* http://developer.intel.com/technology/efi/efi.htm
*
- * By Matt Domsch <Matt_Domsch@dell.com> Fri Sep 22 22:15:56 CDT 2000
+ * By Matt Domsch <Matt_Domsch@dell.com> Fri Sep 22 22:15:56 CDT 2000
* Copyright 2000,2001 Dell Inc.
************************************************************/
@@ -31,26 +31,26 @@
#define GPT_PRIMARY_PARTITION_TABLE_LBA 1
#define PARTITION_SYSTEM_GUID \
- EFI_GUID( 0xC12A7328, 0xF81F, 0x11d2, \
- 0xBA, 0x4B, 0x00, 0xA0, 0xC9, 0x3E, 0xC9, 0x3B)
+ EFI_GUID(0xC12A7328, 0xF81F, 0x11d2, \
+ 0xBA, 0x4B, 0x00, 0xA0, 0xC9, 0x3E, 0xC9, 0x3B)
#define LEGACY_MBR_PARTITION_GUID \
- EFI_GUID( 0x024DEE41, 0x33E7, 0x11d3, \
- 0x9D, 0x69, 0x00, 0x08, 0xC7, 0x81, 0xF3, 0x9F)
+ EFI_GUID(0x024DEE41, 0x33E7, 0x11d3, \
+ 0x9D, 0x69, 0x00, 0x08, 0xC7, 0x81, 0xF3, 0x9F)
#define PARTITION_MSFT_RESERVED_GUID \
- EFI_GUID( 0xE3C9E316, 0x0B5C, 0x4DB8, \
- 0x81, 0x7D, 0xF9, 0x2D, 0xF0, 0x02, 0x15, 0xAE)
+ EFI_GUID(0xE3C9E316, 0x0B5C, 0x4DB8, \
+ 0x81, 0x7D, 0xF9, 0x2D, 0xF0, 0x02, 0x15, 0xAE)
#define PARTITION_BASIC_DATA_GUID \
- EFI_GUID( 0xEBD0A0A2, 0xB9E5, 0x4433, \
- 0x87, 0xC0, 0x68, 0xB6, 0xB7, 0x26, 0x99, 0xC7)
+ EFI_GUID(0xEBD0A0A2, 0xB9E5, 0x4433, \
+ 0x87, 0xC0, 0x68, 0xB6, 0xB7, 0x26, 0x99, 0xC7)
#define PARTITION_LINUX_RAID_GUID \
- EFI_GUID( 0xa19d880f, 0x05fc, 0x4d3b, \
- 0xa0, 0x06, 0x74, 0x3f, 0x0f, 0x84, 0x91, 0x1e)
+ EFI_GUID(0xa19d880f, 0x05fc, 0x4d3b, \
+ 0xa0, 0x06, 0x74, 0x3f, 0x0f, 0x84, 0x91, 0x1e)
#define PARTITION_LINUX_SWAP_GUID \
- EFI_GUID( 0x0657fd6d, 0xa4ab, 0x43c4, \
- 0x84, 0xe5, 0x09, 0x33, 0xc8, 0x4b, 0x4f, 0x4f)
+ EFI_GUID(0x0657fd6d, 0xa4ab, 0x43c4, \
+ 0x84, 0xe5, 0x09, 0x33, 0xc8, 0x4b, 0x4f, 0x4f)
#define PARTITION_LINUX_LVM_GUID \
- EFI_GUID( 0xe6d6d379, 0xf507, 0x44c2, \
- 0xa2, 0x3c, 0x23, 0x8f, 0x2a, 0x3d, 0xf9, 0x28)
+ EFI_GUID(0xe6d6d379, 0xf507, 0x44c2, \
+ 0xa2, 0x3c, 0x23, 0x8f, 0x2a, 0x3d, 0xf9, 0x28)
typedef struct _gpt_header {
__le64 signature;
@@ -78,7 +78,7 @@ typedef struct _gpt_header {
typedef struct _gpt_entry_attributes {
u64 required_to_function:1;
u64 reserved:47;
- u64 type_guid_specific:16;
+ u64 type_guid_specific:16;
} __packed gpt_entry_attributes;
typedef struct _gpt_entry {
@@ -87,7 +87,7 @@ typedef struct _gpt_entry {
__le64 starting_lba;
__le64 ending_lba;
gpt_entry_attributes attributes;
- __le16 partition_name[72/sizeof(__le16)];
+ __le16 partition_name[72 / sizeof(__le16)];
} __packed gpt_entry;
typedef struct _gpt_mbr_record {
@@ -103,7 +103,6 @@ typedef struct _gpt_mbr_record {
__le32 size_in_lba; /* used by EFI - size of pt in LBA */
} __packed gpt_mbr_record;
-
typedef struct _legacy_mbr {
u8 boot_code[440];
__le32 unique_mbr_signature;
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [RFC PATCH 2/6] block: partitions: efi: Fix some style issues
2023-12-11 15:12 ` [RFC PATCH 2/6] block: partitions: efi: Fix some style issues Romain Gantois
@ 2023-12-12 0:57 ` Davidlohr Bueso
0 siblings, 0 replies; 11+ messages in thread
From: Davidlohr Bueso @ 2023-12-12 0:57 UTC (permalink / raw)
To: Romain Gantois
Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jens Axboe,
Ard Biesheuvel, Thomas Petazzoni, Herve Codina, linux-mtd,
devicetree, linux-kernel, linux-block, linux-efi
On Mon, 11 Dec 2023, Romain Gantois wrote:
>The block layer EFI code is quite old and does not perfectly match the
>current kernel coding style. Fix some indentation and trailing whitespace
>issues in efi.c.
I agree that the styling can use some love. However, a few comments below
where such changes are really unnecessary.
...
>@@ -213,7 +212,7 @@ static int is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors)
> */
> if (ret == GPT_MBR_PROTECTIVE) {
> sz = le32_to_cpu(mbr->partition_record[part].size_in_lba);
>- if (sz != (uint32_t) total_sectors - 1 && sz != 0xFFFFFFFF)
>+ if (sz != (uint32_t)total_sectors - 1 && sz != 0xFFFFFFFF)
Like here.
> pr_debug("GPT: mbr size in lba (%u) different than whole disk (%u).\n",
> sz, min_t(uint32_t,
> total_sectors - 1, 0xFFFFFFFF));
>@@ -235,17 +234,19 @@ static int is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors)
> static size_t read_lba(struct parsed_partitions *state,
> u64 lba, u8 *buffer, size_t count)
> {
>- size_t totalreadcount = 0;
> sector_t n = lba *
> (queue_logical_block_size(state->disk->queue) / 512);
>+ size_t totalreadcount = 0;
>+ unsigned char *data;
>+ Sector sect;
>+ int copied;
>
> if (!buffer || lba > last_lba(state->disk))
>- return 0;
>+ return 0;
>
> while (count) {
>- int copied = 512;
>- Sector sect;
>- unsigned char *data = read_part_sector(state, n++, §);
>+ copied = 512;
>+ data = read_part_sector(state, n++, §);
ditto
> if (!data)
> break;
> if (copied > count)
>@@ -253,7 +254,7 @@ static size_t read_lba(struct parsed_partitions *state,
> memcpy(buffer, data, copied);
> put_dev_sector(sect);
> buffer += copied;
>- totalreadcount +=copied;
>+ totalreadcount += copied;
> count -= copied;
> }
> return totalreadcount;
>@@ -263,7 +264,7 @@ static size_t read_lba(struct parsed_partitions *state,
> * alloc_read_gpt_entries(): reads partition entries from disk
> * @state: disk parsed partitions
> * @gpt: GPT header
>- *
>+ *
> * Description: Returns ptes on success, NULL on error.
> * Allocates space for PTEs based on information found in @gpt.
> * Notes: remember to free pte when you're done!
>@@ -271,14 +272,14 @@ static size_t read_lba(struct parsed_partitions *state,
> static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state,
> gpt_header *gpt)
> {
>- size_t count;
> gpt_entry *pte;
>+ size_t count;
ditto
>
> if (!gpt)
> return NULL;
>
> count = (size_t)le32_to_cpu(gpt->num_partition_entries) *
>- le32_to_cpu(gpt->sizeof_partition_entry);
>+ le32_to_cpu(gpt->sizeof_partition_entry);
> if (!count)
> return NULL;
> pte = kmalloc(count, GFP_KERNEL);
>@@ -286,9 +287,9 @@ static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state,
> return NULL;
>
> if (read_lba(state, le64_to_cpu(gpt->partition_entry_lba),
>- (u8 *) pte, count) < count) {
>+ (u8 *)pte, count) < count) {
> kfree(pte);
>- pte=NULL;
>+ pte = NULL;
> return NULL;
> }
> return pte;
>@@ -298,7 +299,7 @@ static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state,
> * alloc_read_gpt_header(): Allocates GPT header, reads into it from disk
> * @state: disk parsed partitions
> * @lba: the Logical Block Address of the partition table
>- *
>+ *
> * Description: returns GPT header on success, NULL on error. Allocates
> * and fills a GPT header starting at @ from @state->disk.
> * Note: remember to free gpt when finished with it.
>@@ -306,16 +307,16 @@ static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state,
> static gpt_header *alloc_read_gpt_header(struct parsed_partitions *state,
> u64 lba)
> {
>+ unsigned int ssz = queue_logical_block_size(state->disk->queue);
> gpt_header *gpt;
>- unsigned ssz = queue_logical_block_size(state->disk->queue);
ditto
>
> gpt = kmalloc(ssz, GFP_KERNEL);
> if (!gpt)
> return NULL;
>
>- if (read_lba(state, lba, (u8 *) gpt, ssz) < ssz) {
>+ if (read_lba(state, lba, (u8 *)gpt, ssz) < ssz) {
ditto
> kfree(gpt);
>- gpt=NULL;
>+ gpt = NULL;
> return NULL;
> }
>
>@@ -486,31 +487,31 @@ compare_gpts(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
> if (le64_to_cpu(pgpt->my_lba) != le64_to_cpu(agpt->alternate_lba)) {
> pr_warn("GPT:Primary header LBA != Alt. header alternate_lba\n");
> pr_warn("GPT:%lld != %lld\n",
>- (unsigned long long)le64_to_cpu(pgpt->my_lba),
>- (unsigned long long)le64_to_cpu(agpt->alternate_lba));
>+ (unsigned long long)le64_to_cpu(pgpt->my_lba),
>+ (unsigned long long)le64_to_cpu(agpt->alternate_lba));
> error_found++;
> }
> if (le64_to_cpu(pgpt->alternate_lba) != le64_to_cpu(agpt->my_lba)) {
> pr_warn("GPT:Primary header alternate_lba != Alt. header my_lba\n");
> pr_warn("GPT:%lld != %lld\n",
>- (unsigned long long)le64_to_cpu(pgpt->alternate_lba),
>- (unsigned long long)le64_to_cpu(agpt->my_lba));
>+ (unsigned long long)le64_to_cpu(pgpt->alternate_lba),
>+ (unsigned long long)le64_to_cpu(agpt->my_lba));
> error_found++;
> }
> if (le64_to_cpu(pgpt->first_usable_lba) !=
>- le64_to_cpu(agpt->first_usable_lba)) {
>+ le64_to_cpu(agpt->first_usable_lba)) {
> pr_warn("GPT:first_usable_lbas don't match.\n");
> pr_warn("GPT:%lld != %lld\n",
>- (unsigned long long)le64_to_cpu(pgpt->first_usable_lba),
>- (unsigned long long)le64_to_cpu(agpt->first_usable_lba));
>+ (unsigned long long)le64_to_cpu(pgpt->first_usable_lba),
>+ (unsigned long long)le64_to_cpu(agpt->first_usable_lba));
> error_found++;
> }
> if (le64_to_cpu(pgpt->last_usable_lba) !=
>- le64_to_cpu(agpt->last_usable_lba)) {
>+ le64_to_cpu(agpt->last_usable_lba)) {
> pr_warn("GPT:last_usable_lbas don't match.\n");
> pr_warn("GPT:%lld != %lld\n",
>- (unsigned long long)le64_to_cpu(pgpt->last_usable_lba),
>- (unsigned long long)le64_to_cpu(agpt->last_usable_lba));
>+ (unsigned long long)le64_to_cpu(pgpt->last_usable_lba),
>+ (unsigned long long)le64_to_cpu(agpt->last_usable_lba));
> error_found++;
> }
> if (efi_guidcmp(pgpt->disk_guid, agpt->disk_guid)) {
>@@ -518,27 +519,24 @@ compare_gpts(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
> error_found++;
> }
> if (le32_to_cpu(pgpt->num_partition_entries) !=
>- le32_to_cpu(agpt->num_partition_entries)) {
>- pr_warn("GPT:num_partition_entries don't match: "
>- "0x%x != 0x%x\n",
>- le32_to_cpu(pgpt->num_partition_entries),
>- le32_to_cpu(agpt->num_partition_entries));
>+ le32_to_cpu(agpt->num_partition_entries)) {
>+ pr_warn("GPT:num_partition_entries don't match: 0x%x != 0x%x\n",
>+ le32_to_cpu(pgpt->num_partition_entries),
>+ le32_to_cpu(agpt->num_partition_entries));
> error_found++;
> }
> if (le32_to_cpu(pgpt->sizeof_partition_entry) !=
>- le32_to_cpu(agpt->sizeof_partition_entry)) {
>- pr_warn("GPT:sizeof_partition_entry values don't match: "
>- "0x%x != 0x%x\n",
>- le32_to_cpu(pgpt->sizeof_partition_entry),
>- le32_to_cpu(agpt->sizeof_partition_entry));
>+ le32_to_cpu(agpt->sizeof_partition_entry)) {
>+ pr_warn("GPT:sizeof_partition_entry values don't match: 0x%x != 0x%x\n",
>+ le32_to_cpu(pgpt->sizeof_partition_entry),
>+ le32_to_cpu(agpt->sizeof_partition_entry));
> error_found++;
> }
> if (le32_to_cpu(pgpt->partition_entry_array_crc32) !=
>- le32_to_cpu(agpt->partition_entry_array_crc32)) {
>- pr_warn("GPT:partition_entry_array_crc32 values don't match: "
>- "0x%x != 0x%x\n",
>- le32_to_cpu(pgpt->partition_entry_array_crc32),
>- le32_to_cpu(agpt->partition_entry_array_crc32));
>+ le32_to_cpu(agpt->partition_entry_array_crc32)) {
>+ pr_warn("GPT:partition_entry_array_crc32 values don't match: 0x%x != 0x%x\n",
>+ le32_to_cpu(pgpt->partition_entry_array_crc32),
>+ le32_to_cpu(agpt->partition_entry_array_crc32));
> error_found++;
> }
> if (le64_to_cpu(pgpt->alternate_lba) != lastlba) {
>@@ -581,20 +579,22 @@ compare_gpts(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
> static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
> gpt_entry **ptes)
> {
>+ sector_t total_sectors = get_capacity(state->disk);
> int good_pgpt = 0, good_agpt = 0, good_pmbr = 0;
>- gpt_header *pgpt = NULL, *agpt = NULL;
>+ const struct block_device_operations *fops;
> gpt_entry *pptes = NULL, *aptes = NULL;
>- legacy_mbr *legacymbr;
>+ gpt_header *pgpt = NULL, *agpt = NULL;
> struct gendisk *disk = state->disk;
>- const struct block_device_operations *fops = disk->fops;
>- sector_t total_sectors = get_capacity(state->disk);
>+ legacy_mbr *legacymbr;
> u64 lastlba;
>
>+ fops = disk->fops;
ditto
>+
> if (!ptes)
> return 0;
>
> lastlba = last_lba(state->disk);
>- if (!force_gpt) {
>+ if (!force_gpt) {
> /* This will be added to the EFI Spec. per Intel after v1.02. */
> legacymbr = kzalloc(sizeof(*legacymbr), GFP_KERNEL);
> if (!legacymbr)
>@@ -609,17 +609,17 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
>
> pr_debug("Device has a %s MBR\n",
> good_pmbr == GPT_MBR_PROTECTIVE ?
>- "protective" : "hybrid");
>+ "protective" : "hybrid");
> }
>
> good_pgpt = is_gpt_valid(state, GPT_PRIMARY_PARTITION_TABLE_LBA,
> &pgpt, &pptes);
>- if (good_pgpt)
>+ if (good_pgpt)
> good_agpt = is_gpt_valid(state,
> le64_to_cpu(pgpt->alternate_lba),
> &agpt, &aptes);
>- if (!good_agpt && force_gpt)
>- good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
>+ if (!good_agpt && force_gpt)
>+ good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
>
> if (!good_agpt && force_gpt && fops->alternative_gpt_sector) {
> sector_t agpt_sector;
>@@ -631,39 +631,38 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
> &agpt, &aptes);
> }
>
>- /* The obviously unsuccessful case */
>- if (!good_pgpt && !good_agpt)
>- goto fail;
>+ /* The obviously unsuccessful case */
>+ if (!good_pgpt && !good_agpt)
>+ goto fail;
>
> compare_gpts(pgpt, agpt, lastlba);
>
>- /* The good cases */
>- if (good_pgpt) {
>- *gpt = pgpt;
>- *ptes = pptes;
>- kfree(agpt);
>- kfree(aptes);
>+ /* The good cases */
>+ if (good_pgpt) {
>+ *gpt = pgpt;
>+ *ptes = pptes;
>+ kfree(agpt);
>+ kfree(aptes);
> if (!good_agpt)
>- pr_warn("Alternate GPT is invalid, using primary GPT.\n");
>- return 1;
>- }
>- else if (good_agpt) {
>- *gpt = agpt;
>- *ptes = aptes;
>- kfree(pgpt);
>- kfree(pptes);
>+ pr_warn("Alternate GPT is invalid, using primary GPT.\n");
>+ return 1;
>+ } else if (good_agpt) {
>+ *gpt = agpt;
>+ *ptes = aptes;
>+ kfree(pgpt);
>+ kfree(pptes);
> pr_warn("Primary GPT is invalid, using alternate GPT.\n");
>- return 1;
>- }
>+ return 1;
>+ }
>
>- fail:
>- kfree(pgpt);
>- kfree(agpt);
>- kfree(pptes);
>- kfree(aptes);
>- *gpt = NULL;
>- *ptes = NULL;
>- return 0;
>+fail:
>+ kfree(pgpt);
>+ kfree(agpt);
>+ kfree(pptes);
>+ kfree(aptes);
>+ *gpt = NULL;
>+ *ptes = NULL;
>+ return 0;
> }
>
> /**
>@@ -712,10 +711,10 @@ static void utf16_le_to_7bit(const __le16 *in, unsigned int size, u8 *out)
> */
> int efi_partition(struct parsed_partitions *state)
> {
>+ unsigned int ssz = queue_logical_block_size(state->disk->queue) / 512;
> gpt_header *gpt = NULL;
> gpt_entry *ptes = NULL;
> u32 i;
>- unsigned ssz = queue_logical_block_size(state->disk->queue) / 512;
ditto
>
> if (!find_valid_gpt(state, &gpt, &ptes) || !gpt || !ptes) {
> kfree(gpt);
>@@ -725,17 +724,17 @@ int efi_partition(struct parsed_partitions *state)
>
> pr_debug("GUID Partition Table is valid! Yea!\n");
>
>- for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < state->limit-1; i++) {
>+ for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < state->limit - 1; i++) {
ditto
> struct partition_meta_info *info;
>- unsigned label_max;
>+ unsigned int label_max;
> u64 start = le64_to_cpu(ptes[i].starting_lba);
> u64 size = le64_to_cpu(ptes[i].ending_lba) -
>- le64_to_cpu(ptes[i].starting_lba) + 1ULL;
>+ le64_to_cpu(ptes[i].starting_lba) + 1ULL;
>
> if (!is_pte_valid(&ptes[i], last_lba(state->disk)))
> continue;
>
>- put_partition(state, i+1, start * ssz, size * ssz);
>+ put_partition(state, i + 1, start * ssz, size * ssz);
>
> /* If this is a RAID volume, tell md */
> if (!efi_guidcmp(ptes[i].partition_type_guid, PARTITION_LINUX_RAID_GUID))
>diff --git a/include/linux/gpt.h b/include/linux/gpt.h
>index 84b9f36b9e47..633be6bc826c 100644
>--- a/include/linux/gpt.h
>+++ b/include/linux/gpt.h
>@@ -4,7 +4,7 @@
> * Per Intel EFI Specification v1.02
> * http://developer.intel.com/technology/efi/efi.htm
> *
>- * By Matt Domsch <Matt_Domsch@dell.com> Fri Sep 22 22:15:56 CDT 2000
>+ * By Matt Domsch <Matt_Domsch@dell.com> Fri Sep 22 22:15:56 CDT 2000
> * Copyright 2000,2001 Dell Inc.
> ************************************************************/
>
>@@ -31,26 +31,26 @@
> #define GPT_PRIMARY_PARTITION_TABLE_LBA 1
>
> #define PARTITION_SYSTEM_GUID \
>- EFI_GUID( 0xC12A7328, 0xF81F, 0x11d2, \
>- 0xBA, 0x4B, 0x00, 0xA0, 0xC9, 0x3E, 0xC9, 0x3B)
>+ EFI_GUID(0xC12A7328, 0xF81F, 0x11d2, \
>+ 0xBA, 0x4B, 0x00, 0xA0, 0xC9, 0x3E, 0xC9, 0x3B)
> #define LEGACY_MBR_PARTITION_GUID \
>- EFI_GUID( 0x024DEE41, 0x33E7, 0x11d3, \
>- 0x9D, 0x69, 0x00, 0x08, 0xC7, 0x81, 0xF3, 0x9F)
>+ EFI_GUID(0x024DEE41, 0x33E7, 0x11d3, \
>+ 0x9D, 0x69, 0x00, 0x08, 0xC7, 0x81, 0xF3, 0x9F)
> #define PARTITION_MSFT_RESERVED_GUID \
>- EFI_GUID( 0xE3C9E316, 0x0B5C, 0x4DB8, \
>- 0x81, 0x7D, 0xF9, 0x2D, 0xF0, 0x02, 0x15, 0xAE)
>+ EFI_GUID(0xE3C9E316, 0x0B5C, 0x4DB8, \
>+ 0x81, 0x7D, 0xF9, 0x2D, 0xF0, 0x02, 0x15, 0xAE)
> #define PARTITION_BASIC_DATA_GUID \
>- EFI_GUID( 0xEBD0A0A2, 0xB9E5, 0x4433, \
>- 0x87, 0xC0, 0x68, 0xB6, 0xB7, 0x26, 0x99, 0xC7)
>+ EFI_GUID(0xEBD0A0A2, 0xB9E5, 0x4433, \
>+ 0x87, 0xC0, 0x68, 0xB6, 0xB7, 0x26, 0x99, 0xC7)
> #define PARTITION_LINUX_RAID_GUID \
>- EFI_GUID( 0xa19d880f, 0x05fc, 0x4d3b, \
>- 0xa0, 0x06, 0x74, 0x3f, 0x0f, 0x84, 0x91, 0x1e)
>+ EFI_GUID(0xa19d880f, 0x05fc, 0x4d3b, \
>+ 0xa0, 0x06, 0x74, 0x3f, 0x0f, 0x84, 0x91, 0x1e)
> #define PARTITION_LINUX_SWAP_GUID \
>- EFI_GUID( 0x0657fd6d, 0xa4ab, 0x43c4, \
>- 0x84, 0xe5, 0x09, 0x33, 0xc8, 0x4b, 0x4f, 0x4f)
>+ EFI_GUID(0x0657fd6d, 0xa4ab, 0x43c4, \
>+ 0x84, 0xe5, 0x09, 0x33, 0xc8, 0x4b, 0x4f, 0x4f)
> #define PARTITION_LINUX_LVM_GUID \
>- EFI_GUID( 0xe6d6d379, 0xf507, 0x44c2, \
>- 0xa2, 0x3c, 0x23, 0x8f, 0x2a, 0x3d, 0xf9, 0x28)
>+ EFI_GUID(0xe6d6d379, 0xf507, 0x44c2, \
>+ 0xa2, 0x3c, 0x23, 0x8f, 0x2a, 0x3d, 0xf9, 0x28)
>
> typedef struct _gpt_header {
> __le64 signature;
>@@ -78,7 +78,7 @@ typedef struct _gpt_header {
> typedef struct _gpt_entry_attributes {
> u64 required_to_function:1;
> u64 reserved:47;
>- u64 type_guid_specific:16;
>+ u64 type_guid_specific:16;
> } __packed gpt_entry_attributes;
>
> typedef struct _gpt_entry {
>@@ -87,7 +87,7 @@ typedef struct _gpt_entry {
> __le64 starting_lba;
> __le64 ending_lba;
> gpt_entry_attributes attributes;
>- __le16 partition_name[72/sizeof(__le16)];
>+ __le16 partition_name[72 / sizeof(__le16)];
> } __packed gpt_entry;
>
> typedef struct _gpt_mbr_record {
>@@ -103,7 +103,6 @@ typedef struct _gpt_mbr_record {
> __le32 size_in_lba; /* used by EFI - size of pt in LBA */
> } __packed gpt_mbr_record;
>
>-
> typedef struct _legacy_mbr {
> u8 boot_code[440];
> __le32 unique_mbr_signature;
>--
>2.43.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH 3/6] block: partitions: efi: Separate out GPT-specific code
2023-12-11 15:12 [RFC PATCH 0/6] Add GPT parser to MTD layer Romain Gantois
2023-12-11 15:12 ` [RFC PATCH 1/6] block: partitions: efi: Move efi.h header to include/linux/gpt.h Romain Gantois
2023-12-11 15:12 ` [RFC PATCH 2/6] block: partitions: efi: Fix some style issues Romain Gantois
@ 2023-12-11 15:12 ` Romain Gantois
2023-12-11 15:12 ` [RFC PATCH 4/6] block: partitions: efi: Move GPT-specific code to a new library Romain Gantois
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Romain Gantois @ 2023-12-11 15:12 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jens Axboe,
Davidlohr Bueso, Ard Biesheuvel
Cc: Romain Gantois, Thomas Petazzoni, Herve Codina, linux-mtd,
devicetree, linux-kernel, linux-block, linux-efi
The current GPT parser implemented in the block layer mixes
blockdev-specific code with GPT concepts. Separate out device-agnostic GPT
functions from the rest of the parser, in preparation for the creation of a
new generic purpose GPT parser library.
This mostly implies renaming functions and changing argument types. The
only significant change is the new gpt_validate_header function, which has
been separated out from the is_gpt_valid function.
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
block/partitions/efi.c | 199 ++++++++++++++++++++++-------------------
include/linux/gpt.h | 37 +++++++-
2 files changed, 143 insertions(+), 93 deletions(-)
diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index bac514a62d61..3630ebf4b997 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -151,7 +151,7 @@ static inline int pmbr_part_valid(gpt_mbr_record *part)
}
/**
- * is_pmbr_valid(): test Protective MBR for validity
+ * gpt_is_pmbr_valid(): test Protective MBR for validity
* @mbr: pointer to a legacy mbr structure
* @total_sectors: amount of sectors in the device
*
@@ -168,7 +168,7 @@ static inline int pmbr_part_valid(gpt_mbr_record *part)
* Returns 0 upon invalid MBR, or GPT_MBR_PROTECTIVE or
* GPT_MBR_HYBRID depending on the device layout.
*/
-static int is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors)
+int gpt_is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors)
{
uint32_t sz = 0;
int i, part = 0, ret = 0; /* invalid by default */
@@ -324,166 +324,183 @@ static gpt_header *alloc_read_gpt_header(struct parsed_partitions *state,
}
/**
- * is_gpt_valid() - tests one GPT header and PTEs for validity
- * @state: disk parsed partitions
- * @lba: logical block address of the GPT header to test
- * @gpt: GPT header ptr, filled on return.
- * @ptes: PTEs ptr, filled on return.
+ * gpt_validate_header() - tests one GPT header for validity
+ * @gpt: header to check
+ * @lba: logical block address of the GPT header to test
+ * @lba_size: logical block size of the partitioned device
+ * @lastlba: last logical block on the partitioned device
*
- * Description: returns 1 if valid, 0 on error.
- * If valid, returns pointers to newly allocated GPT header and PTEs.
+ * Returns 0 if validation was successful.
*/
-static int is_gpt_valid(struct parsed_partitions *state, u64 lba,
- gpt_header **gpt, gpt_entry **ptes)
+int gpt_validate_header(gpt_header *gpt, u64 lba, unsigned int lba_size,
+ u64 lastlba)
{
u32 crc, origcrc;
- u64 lastlba, pt_size;
-
- if (!ptes)
- return 0;
- if (!(*gpt = alloc_read_gpt_header(state, lba)))
- return 0;
+ u64 pt_size;
/* Check the GUID Partition Table signature */
- if (le64_to_cpu((*gpt)->signature) != GPT_HEADER_SIGNATURE) {
- pr_debug("GUID Partition Table Header signature is wrong:"
- "%lld != %lld\n",
- (unsigned long long)le64_to_cpu((*gpt)->signature),
+ if (le64_to_cpu(gpt->signature) != GPT_HEADER_SIGNATURE) {
+ pr_debug("GUID Partition Table Header signature is wrong: %lld != %lld\n",
+ (unsigned long long)le64_to_cpu(gpt->signature),
(unsigned long long)GPT_HEADER_SIGNATURE);
- goto fail;
+ return -EINVAL;
}
/* Check the GUID Partition Table header size is too big */
- if (le32_to_cpu((*gpt)->header_size) >
- queue_logical_block_size(state->disk->queue)) {
+ if (le32_to_cpu(gpt->header_size) > lba_size) {
pr_debug("GUID Partition Table Header size is too large: %u > %u\n",
- le32_to_cpu((*gpt)->header_size),
- queue_logical_block_size(state->disk->queue));
- goto fail;
+ le32_to_cpu(gpt->header_size), lba_size);
+ return -EINVAL;
}
/* Check the GUID Partition Table header size is too small */
- if (le32_to_cpu((*gpt)->header_size) < sizeof(gpt_header)) {
+ if (le32_to_cpu(gpt->header_size) < sizeof(gpt_header)) {
pr_debug("GUID Partition Table Header size is too small: %u < %zu\n",
- le32_to_cpu((*gpt)->header_size),
- sizeof(gpt_header));
- goto fail;
+ le32_to_cpu(gpt->header_size),
+ sizeof(gpt_header));
+ return -EINVAL;
}
/* Check the GUID Partition Table CRC */
- origcrc = le32_to_cpu((*gpt)->header_crc32);
- (*gpt)->header_crc32 = 0;
- crc = efi_crc32((const unsigned char *) (*gpt), le32_to_cpu((*gpt)->header_size));
+ origcrc = le32_to_cpu(gpt->header_crc32);
+ gpt->header_crc32 = 0;
+ crc = efi_crc32((const unsigned char *)gpt, le32_to_cpu(gpt->header_size));
if (crc != origcrc) {
pr_debug("GUID Partition Table Header CRC is wrong: %x != %x\n",
crc, origcrc);
- goto fail;
+ return -EINVAL;
}
- (*gpt)->header_crc32 = cpu_to_le32(origcrc);
+ gpt->header_crc32 = cpu_to_le32(origcrc);
/* Check that the my_lba entry points to the LBA that contains
- * the GUID Partition Table */
- if (le64_to_cpu((*gpt)->my_lba) != lba) {
+ * the GUID Partition Table
+ */
+ if (le64_to_cpu(gpt->my_lba) != lba) {
pr_debug("GPT my_lba incorrect: %lld != %lld\n",
- (unsigned long long)le64_to_cpu((*gpt)->my_lba),
+ (unsigned long long)le64_to_cpu(gpt->my_lba),
(unsigned long long)lba);
- goto fail;
+ return -EINVAL;
}
/* Check the first_usable_lba and last_usable_lba are
* within the disk.
*/
- lastlba = last_lba(state->disk);
- if (le64_to_cpu((*gpt)->first_usable_lba) > lastlba) {
+ if (le64_to_cpu(gpt->first_usable_lba) > lastlba) {
pr_debug("GPT: first_usable_lba incorrect: %lld > %lld\n",
- (unsigned long long)le64_to_cpu((*gpt)->first_usable_lba),
+ (unsigned long long)le64_to_cpu(gpt->first_usable_lba),
(unsigned long long)lastlba);
- goto fail;
+ return -EINVAL;
}
- if (le64_to_cpu((*gpt)->last_usable_lba) > lastlba) {
+ if (le64_to_cpu(gpt->last_usable_lba) > lastlba) {
pr_debug("GPT: last_usable_lba incorrect: %lld > %lld\n",
- (unsigned long long)le64_to_cpu((*gpt)->last_usable_lba),
+ (unsigned long long)le64_to_cpu(gpt->last_usable_lba),
(unsigned long long)lastlba);
- goto fail;
+ return -EINVAL;
}
- if (le64_to_cpu((*gpt)->last_usable_lba) < le64_to_cpu((*gpt)->first_usable_lba)) {
+ if (le64_to_cpu(gpt->last_usable_lba) < le64_to_cpu(gpt->first_usable_lba)) {
pr_debug("GPT: last_usable_lba incorrect: %lld > %lld\n",
- (unsigned long long)le64_to_cpu((*gpt)->last_usable_lba),
- (unsigned long long)le64_to_cpu((*gpt)->first_usable_lba));
- goto fail;
+ (unsigned long long)le64_to_cpu(gpt->last_usable_lba),
+ (unsigned long long)le64_to_cpu(gpt->first_usable_lba));
+ return -EINVAL;
}
+
/* Check that sizeof_partition_entry has the correct value */
- if (le32_to_cpu((*gpt)->sizeof_partition_entry) != sizeof(gpt_entry)) {
+ if (le32_to_cpu(gpt->sizeof_partition_entry) != sizeof(gpt_entry)) {
pr_debug("GUID Partition Entry Size check failed.\n");
- goto fail;
+ return -EINVAL;
}
/* Sanity check partition table size */
- pt_size = (u64)le32_to_cpu((*gpt)->num_partition_entries) *
- le32_to_cpu((*gpt)->sizeof_partition_entry);
+ pt_size = (u64)get_pt_size(gpt);
if (pt_size > KMALLOC_MAX_SIZE) {
pr_debug("GUID Partition Table is too large: %llu > %lu bytes\n",
(unsigned long long)pt_size, KMALLOC_MAX_SIZE);
- goto fail;
+ return -EINVAL;
}
- if (!(*ptes = alloc_read_gpt_entries(state, *gpt)))
- goto fail;
+ return 0;
+}
- /* Check the GUID Partition Entry Array CRC */
- crc = efi_crc32((const unsigned char *) (*ptes), pt_size);
+/* Check the GUID Partition Entry Array CRC */
+int gpt_check_pte_array_crc(gpt_header *gpt, gpt_entry *ptes)
+{
+ u32 crc;
- if (crc != le32_to_cpu((*gpt)->partition_entry_array_crc32)) {
+ crc = efi_crc32((const unsigned char *)ptes, get_pt_size(gpt));
+ if (crc != le32_to_cpu(gpt->partition_entry_array_crc32)) {
pr_debug("GUID Partition Entry Array CRC check failed.\n");
- goto fail_ptes;
+ return -EINVAL;
}
- /* We're done, all's well */
- return 1;
-
- fail_ptes:
- kfree(*ptes);
- *ptes = NULL;
- fail:
- kfree(*gpt);
- *gpt = NULL;
return 0;
}
/**
- * is_pte_valid() - tests one PTE for validity
- * @pte:pte to check
- * @lastlba: last lba of the disk
+ * is_gpt_valid() - tests one GPT header and PTEs for validity
+ * @state: disk parsed partitions
+ * @lba: logical block address of the GPT header to test
+ * @gpt: GPT header ptr, filled on return.
+ * @ptes: PTEs ptr, filled on return.
*
* Description: returns 1 if valid, 0 on error.
+ * If valid, returns pointers to newly allocated GPT header and PTEs.
*/
-static inline int
-is_pte_valid(const gpt_entry *pte, const u64 lastlba)
+static int is_gpt_valid(struct parsed_partitions *state, u64 lba,
+ gpt_header **gpt, gpt_entry **ptes)
{
- if ((!efi_guidcmp(pte->partition_type_guid, NULL_GUID)) ||
- le64_to_cpu(pte->starting_lba) > lastlba ||
- le64_to_cpu(pte->ending_lba) > lastlba)
+ u64 lastlba;
+
+ if (!ptes)
return 0;
+
+ *gpt = alloc_read_gpt_header(state, lba);
+ if (!(*gpt))
+ return 0;
+
+ lastlba = last_lba(state->disk);
+ if (gpt_validate_header(*gpt, lba,
+ queue_logical_block_size(state->disk->queue),
+ lastlba))
+ goto fail;
+
+ *ptes = alloc_read_gpt_entries(state, *gpt);
+ if (!(*ptes))
+ goto fail;
+
+ if (gpt_check_pte_array_crc(*gpt, *ptes))
+ goto fail_ptes;
+
+ /* We're done, all's well */
return 1;
+
+fail_ptes:
+ kfree(*ptes);
+ *ptes = NULL;
+fail:
+ kfree(*gpt);
+ *gpt = NULL;
+ return 0;
}
/**
- * compare_gpts() - Search disk for valid GPT headers and PTEs
+ * gpt_compare_alt() - Compares the Primary and Alternate GPT headers
* @pgpt: primary GPT header
* @agpt: alternate GPT header
* @lastlba: last LBA number
*
- * Description: Returns nothing. Sanity checks pgpt and agpt fields
- * and prints warnings on discrepancies.
- *
+ * Description: Sanity checks pgpt and agpt fields and prints warnings
+ * on discrepancies. Returns error count. GPT parsers can choose to
+ * ignore this or not.
+ *
*/
-static void
-compare_gpts(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
+int gpt_compare_alt(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
{
int error_found = 0;
+
if (!pgpt || !agpt)
- return;
+ return -EINVAL;
+
if (le64_to_cpu(pgpt->my_lba) != le64_to_cpu(agpt->alternate_lba)) {
pr_warn("GPT:Primary header LBA != Alt. header alternate_lba\n");
pr_warn("GPT:%lld != %lld\n",
@@ -557,7 +574,7 @@ compare_gpts(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
if (error_found)
pr_warn("GPT: Use GNU Parted to correct GPT errors.\n");
- return;
+ return error_found;
}
/**
@@ -601,7 +618,7 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
goto fail;
read_lba(state, 0, (u8 *)legacymbr, sizeof(*legacymbr));
- good_pmbr = is_pmbr_valid(legacymbr, total_sectors);
+ good_pmbr = gpt_is_pmbr_valid(legacymbr, total_sectors);
kfree(legacymbr);
if (!good_pmbr)
@@ -635,7 +652,7 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
if (!good_pgpt && !good_agpt)
goto fail;
- compare_gpts(pgpt, agpt, lastlba);
+ gpt_compare_alt(pgpt, agpt, lastlba);
/* The good cases */
if (good_pgpt) {
@@ -674,7 +691,7 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
* Description: Converts @size UTF16-LE symbols from @in string to 7-bit
* ASCII characters and stores them to @out. Adds trailing zero to @out array.
*/
-static void utf16_le_to_7bit(const __le16 *in, unsigned int size, u8 *out)
+void utf16_le_to_7bit(const __le16 *in, unsigned int size, u8 *out)
{
unsigned int i = 0;
@@ -731,7 +748,7 @@ int efi_partition(struct parsed_partitions *state)
u64 size = le64_to_cpu(ptes[i].ending_lba) -
le64_to_cpu(ptes[i].starting_lba) + 1ULL;
- if (!is_pte_valid(&ptes[i], last_lba(state->disk)))
+ if (!gpt_is_pte_valid(&ptes[i], last_lba(state->disk)))
continue;
put_partition(state, i + 1, start * ssz, size * ssz);
diff --git a/include/linux/gpt.h b/include/linux/gpt.h
index 633be6bc826c..f7f5892fe256 100644
--- a/include/linux/gpt.h
+++ b/include/linux/gpt.h
@@ -8,8 +8,8 @@
* Copyright 2000,2001 Dell Inc.
************************************************************/
-#ifndef FS_PART_EFI_H_INCLUDED
-#define FS_PART_EFI_H_INCLUDED
+#ifndef _GPT_H
+#define _GPT_H
#include <linux/types.h>
#include <linux/fs.h>
@@ -111,4 +111,37 @@ typedef struct _legacy_mbr {
__le16 signature;
} __packed legacy_mbr;
+// Helpers for validating GPT metadata
+int gpt_is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors);
+int gpt_validate_header(gpt_header *gpt, u64 lba, unsigned int lba_size,
+ u64 lastlba);
+int gpt_check_pte_array_crc(gpt_header *gpt, gpt_entry *ptes);
+int gpt_compare_alt(gpt_header *pgpt, gpt_header *agpt, u64 lastlba);
+
+/**
+ * is_pte_valid() - tests one PTE for validity
+ * @pte:pte to check
+ * @lastlba: last lba of the disk
+ *
+ * returns 1 if valid, 0 on error.
+ */
+ static inline bool
+gpt_is_pte_valid(const gpt_entry *pte, const u64 lastlba)
+{
+ if ((!efi_guidcmp(pte->partition_type_guid, NULL_GUID)) ||
+ le64_to_cpu(pte->starting_lba) > lastlba ||
+ le64_to_cpu(pte->ending_lba) > lastlba)
+ return 0;
+ return 1;
+}
+
+// Returns size in bytes of PTE array
+static inline int get_pt_size(gpt_header *gpt)
+{
+ return le32_to_cpu(gpt->num_partition_entries)
+ * le32_to_cpu(gpt->sizeof_partition_entry);
+}
+
+void utf16_le_to_7bit(const __le16 *in, unsigned int size, u8 *out);
+
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [RFC PATCH 4/6] block: partitions: efi: Move GPT-specific code to a new library
2023-12-11 15:12 [RFC PATCH 0/6] Add GPT parser to MTD layer Romain Gantois
` (2 preceding siblings ...)
2023-12-11 15:12 ` [RFC PATCH 3/6] block: partitions: efi: Separate out GPT-specific code Romain Gantois
@ 2023-12-11 15:12 ` Romain Gantois
2023-12-11 15:12 ` [RFC PATCH 5/6] drivers: mtd: introduce GPT parser for NOR flash devices Romain Gantois
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Romain Gantois @ 2023-12-11 15:12 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jens Axboe,
Davidlohr Bueso, Ard Biesheuvel
Cc: Romain Gantois, Thomas Petazzoni, Herve Codina, linux-mtd,
devicetree, linux-kernel, linux-block, linux-efi
The GPT parser in the block layer contains much logic that is not specific
to block devices. Move all of this logic to a new generic GPT library so
that future GPT parsers can make use of it.
lib/gpt.c is designed as a stateless library. It mainly contains helpers
that validate GPT metadata.
The efi_crc32 function is moved out of the block layer GPT parser and into
the generic efi header.
There is no functional change.
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
MAINTAINERS | 1 +
block/partitions/Kconfig | 2 +-
block/partitions/efi.c | 336 --------------------------------------
include/linux/efi.h | 18 +++
lib/Kconfig | 3 +
lib/Makefile | 3 +
lib/gpt.c | 342 +++++++++++++++++++++++++++++++++++++++
7 files changed, 368 insertions(+), 337 deletions(-)
create mode 100644 lib/gpt.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 22e37e2ea1ae..c01abee48b75 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9235,6 +9235,7 @@ L: linux-efi@vger.kernel.org
S: Maintained
F: block/partitions/efi.c
F: include/linux/gpt.h
+F: lib/gpt.c
HABANALABS PCI DRIVER
M: Oded Gabbay <ogabbay@kernel.org>
diff --git a/block/partitions/Kconfig b/block/partitions/Kconfig
index 7aff4eb81c60..c2b2618213ba 100644
--- a/block/partitions/Kconfig
+++ b/block/partitions/Kconfig
@@ -250,7 +250,7 @@ config KARMA_PARTITION
config EFI_PARTITION
bool "EFI GUID Partition support" if PARTITION_ADVANCED
default y
- select CRC32
+ select GENERIC_LIB_GPT
help
Say Y here if you would like to use hard disks under Linux which
were partitioned using EFI GPT.
diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index 3630ebf4b997..58bcd2cbcdf8 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -83,7 +83,6 @@
*
************************************************************/
#include <linux/kernel.h>
-#include <linux/crc32.h>
#include <linux/ctype.h>
#include <linux/math64.h>
#include <linux/slab.h>
@@ -103,24 +102,6 @@ force_gpt_fn(char *str)
}
__setup("gpt", force_gpt_fn);
-/**
- * efi_crc32() - EFI version of crc32 function
- * @buf: buffer to calculate crc32 of
- * @len: length of buf
- *
- * Description: Returns EFI-style CRC32 value for @buf
- *
- * This function uses the little endian Ethernet polynomial
- * but seeds the function with ~0, and xor's with ~0 at the end.
- * Note, the EFI Specification, v1.02, has a reference to
- * Dr. Dobbs Journal, May 1994 (actually it's in May 1992).
- */
-static inline u32
-efi_crc32(const void *buf, unsigned long len)
-{
- return (crc32(~0L, buf, len) ^ ~0L);
-}
-
/**
* last_lba(): return number of last logical block of device
* @disk: block device
@@ -136,91 +117,6 @@ static u64 last_lba(struct gendisk *disk)
queue_logical_block_size(disk->queue)) - 1ULL;
}
-static inline int pmbr_part_valid(gpt_mbr_record *part)
-{
- if (part->os_type != EFI_PMBR_OSTYPE_EFI_GPT)
- goto invalid;
-
- /* set to 0x00000001 (i.e., the LBA of the GPT Partition Header) */
- if (le32_to_cpu(part->starting_lba) != GPT_PRIMARY_PARTITION_TABLE_LBA)
- goto invalid;
-
- return GPT_MBR_PROTECTIVE;
-invalid:
- return 0;
-}
-
-/**
- * gpt_is_pmbr_valid(): test Protective MBR for validity
- * @mbr: pointer to a legacy mbr structure
- * @total_sectors: amount of sectors in the device
- *
- * Description: Checks for a valid protective or hybrid
- * master boot record (MBR). The validity of a pMBR depends
- * on all of the following properties:
- * 1) MSDOS signature is in the last two bytes of the MBR
- * 2) One partition of type 0xEE is found
- *
- * In addition, a hybrid MBR will have up to three additional
- * primary partitions, which point to the same space that's
- * marked out by up to three GPT partitions.
- *
- * Returns 0 upon invalid MBR, or GPT_MBR_PROTECTIVE or
- * GPT_MBR_HYBRID depending on the device layout.
- */
-int gpt_is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors)
-{
- uint32_t sz = 0;
- int i, part = 0, ret = 0; /* invalid by default */
-
- if (!mbr || le16_to_cpu(mbr->signature) != MSDOS_MBR_SIGNATURE)
- goto done;
-
- for (i = 0; i < 4; i++) {
- ret = pmbr_part_valid(&mbr->partition_record[i]);
- if (ret == GPT_MBR_PROTECTIVE) {
- part = i;
- /*
- * Ok, we at least know that there's a protective MBR,
- * now check if there are other partition types for
- * hybrid MBR.
- */
- goto check_hybrid;
- }
- }
-
- if (ret != GPT_MBR_PROTECTIVE)
- goto done;
-check_hybrid:
- for (i = 0; i < 4; i++)
- if (mbr->partition_record[i].os_type !=
- EFI_PMBR_OSTYPE_EFI_GPT &&
- mbr->partition_record[i].os_type != 0x00)
- ret = GPT_MBR_HYBRID;
-
- /*
- * Protective MBRs take up the lesser of the whole disk
- * or 2 TiB (32bit LBA), ignoring the rest of the disk.
- * Some partitioning programs, nonetheless, choose to set
- * the size to the maximum 32-bit limitation, disregarding
- * the disk size.
- *
- * Hybrid MBRs do not necessarily comply with this.
- *
- * Consider a bad value here to be a warning to support dd'ing
- * an image from a smaller disk to a larger disk.
- */
- if (ret == GPT_MBR_PROTECTIVE) {
- sz = le32_to_cpu(mbr->partition_record[part].size_in_lba);
- if (sz != (uint32_t)total_sectors - 1 && sz != 0xFFFFFFFF)
- pr_debug("GPT: mbr size in lba (%u) different than whole disk (%u).\n",
- sz, min_t(uint32_t,
- total_sectors - 1, 0xFFFFFFFF));
- }
-done:
- return ret;
-}
-
/**
* read_lba(): Read bytes from disk, starting at given LBA
* @state: disk parsed partitions
@@ -323,119 +219,6 @@ static gpt_header *alloc_read_gpt_header(struct parsed_partitions *state,
return gpt;
}
-/**
- * gpt_validate_header() - tests one GPT header for validity
- * @gpt: header to check
- * @lba: logical block address of the GPT header to test
- * @lba_size: logical block size of the partitioned device
- * @lastlba: last logical block on the partitioned device
- *
- * Returns 0 if validation was successful.
- */
-int gpt_validate_header(gpt_header *gpt, u64 lba, unsigned int lba_size,
- u64 lastlba)
-{
- u32 crc, origcrc;
- u64 pt_size;
-
- /* Check the GUID Partition Table signature */
- if (le64_to_cpu(gpt->signature) != GPT_HEADER_SIGNATURE) {
- pr_debug("GUID Partition Table Header signature is wrong: %lld != %lld\n",
- (unsigned long long)le64_to_cpu(gpt->signature),
- (unsigned long long)GPT_HEADER_SIGNATURE);
- return -EINVAL;
- }
-
- /* Check the GUID Partition Table header size is too big */
- if (le32_to_cpu(gpt->header_size) > lba_size) {
- pr_debug("GUID Partition Table Header size is too large: %u > %u\n",
- le32_to_cpu(gpt->header_size), lba_size);
- return -EINVAL;
- }
-
- /* Check the GUID Partition Table header size is too small */
- if (le32_to_cpu(gpt->header_size) < sizeof(gpt_header)) {
- pr_debug("GUID Partition Table Header size is too small: %u < %zu\n",
- le32_to_cpu(gpt->header_size),
- sizeof(gpt_header));
- return -EINVAL;
- }
-
- /* Check the GUID Partition Table CRC */
- origcrc = le32_to_cpu(gpt->header_crc32);
- gpt->header_crc32 = 0;
- crc = efi_crc32((const unsigned char *)gpt, le32_to_cpu(gpt->header_size));
-
- if (crc != origcrc) {
- pr_debug("GUID Partition Table Header CRC is wrong: %x != %x\n",
- crc, origcrc);
- return -EINVAL;
- }
- gpt->header_crc32 = cpu_to_le32(origcrc);
-
- /* Check that the my_lba entry points to the LBA that contains
- * the GUID Partition Table
- */
- if (le64_to_cpu(gpt->my_lba) != lba) {
- pr_debug("GPT my_lba incorrect: %lld != %lld\n",
- (unsigned long long)le64_to_cpu(gpt->my_lba),
- (unsigned long long)lba);
- return -EINVAL;
- }
-
- /* Check the first_usable_lba and last_usable_lba are
- * within the disk.
- */
- if (le64_to_cpu(gpt->first_usable_lba) > lastlba) {
- pr_debug("GPT: first_usable_lba incorrect: %lld > %lld\n",
- (unsigned long long)le64_to_cpu(gpt->first_usable_lba),
- (unsigned long long)lastlba);
- return -EINVAL;
- }
- if (le64_to_cpu(gpt->last_usable_lba) > lastlba) {
- pr_debug("GPT: last_usable_lba incorrect: %lld > %lld\n",
- (unsigned long long)le64_to_cpu(gpt->last_usable_lba),
- (unsigned long long)lastlba);
- return -EINVAL;
- }
- if (le64_to_cpu(gpt->last_usable_lba) < le64_to_cpu(gpt->first_usable_lba)) {
- pr_debug("GPT: last_usable_lba incorrect: %lld > %lld\n",
- (unsigned long long)le64_to_cpu(gpt->last_usable_lba),
- (unsigned long long)le64_to_cpu(gpt->first_usable_lba));
- return -EINVAL;
- }
-
- /* Check that sizeof_partition_entry has the correct value */
- if (le32_to_cpu(gpt->sizeof_partition_entry) != sizeof(gpt_entry)) {
- pr_debug("GUID Partition Entry Size check failed.\n");
- return -EINVAL;
- }
-
- /* Sanity check partition table size */
- pt_size = (u64)get_pt_size(gpt);
- if (pt_size > KMALLOC_MAX_SIZE) {
- pr_debug("GUID Partition Table is too large: %llu > %lu bytes\n",
- (unsigned long long)pt_size, KMALLOC_MAX_SIZE);
- return -EINVAL;
- }
-
- return 0;
-}
-
-/* Check the GUID Partition Entry Array CRC */
-int gpt_check_pte_array_crc(gpt_header *gpt, gpt_entry *ptes)
-{
- u32 crc;
-
- crc = efi_crc32((const unsigned char *)ptes, get_pt_size(gpt));
- if (crc != le32_to_cpu(gpt->partition_entry_array_crc32)) {
- pr_debug("GUID Partition Entry Array CRC check failed.\n");
- return -EINVAL;
- }
-
- return 0;
-}
-
/**
* is_gpt_valid() - tests one GPT header and PTEs for validity
* @state: disk parsed partitions
@@ -483,100 +266,6 @@ static int is_gpt_valid(struct parsed_partitions *state, u64 lba,
return 0;
}
-/**
- * gpt_compare_alt() - Compares the Primary and Alternate GPT headers
- * @pgpt: primary GPT header
- * @agpt: alternate GPT header
- * @lastlba: last LBA number
- *
- * Description: Sanity checks pgpt and agpt fields and prints warnings
- * on discrepancies. Returns error count. GPT parsers can choose to
- * ignore this or not.
- *
- */
-int gpt_compare_alt(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
-{
- int error_found = 0;
-
- if (!pgpt || !agpt)
- return -EINVAL;
-
- if (le64_to_cpu(pgpt->my_lba) != le64_to_cpu(agpt->alternate_lba)) {
- pr_warn("GPT:Primary header LBA != Alt. header alternate_lba\n");
- pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(pgpt->my_lba),
- (unsigned long long)le64_to_cpu(agpt->alternate_lba));
- error_found++;
- }
- if (le64_to_cpu(pgpt->alternate_lba) != le64_to_cpu(agpt->my_lba)) {
- pr_warn("GPT:Primary header alternate_lba != Alt. header my_lba\n");
- pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(pgpt->alternate_lba),
- (unsigned long long)le64_to_cpu(agpt->my_lba));
- error_found++;
- }
- if (le64_to_cpu(pgpt->first_usable_lba) !=
- le64_to_cpu(agpt->first_usable_lba)) {
- pr_warn("GPT:first_usable_lbas don't match.\n");
- pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(pgpt->first_usable_lba),
- (unsigned long long)le64_to_cpu(agpt->first_usable_lba));
- error_found++;
- }
- if (le64_to_cpu(pgpt->last_usable_lba) !=
- le64_to_cpu(agpt->last_usable_lba)) {
- pr_warn("GPT:last_usable_lbas don't match.\n");
- pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(pgpt->last_usable_lba),
- (unsigned long long)le64_to_cpu(agpt->last_usable_lba));
- error_found++;
- }
- if (efi_guidcmp(pgpt->disk_guid, agpt->disk_guid)) {
- pr_warn("GPT:disk_guids don't match.\n");
- error_found++;
- }
- if (le32_to_cpu(pgpt->num_partition_entries) !=
- le32_to_cpu(agpt->num_partition_entries)) {
- pr_warn("GPT:num_partition_entries don't match: 0x%x != 0x%x\n",
- le32_to_cpu(pgpt->num_partition_entries),
- le32_to_cpu(agpt->num_partition_entries));
- error_found++;
- }
- if (le32_to_cpu(pgpt->sizeof_partition_entry) !=
- le32_to_cpu(agpt->sizeof_partition_entry)) {
- pr_warn("GPT:sizeof_partition_entry values don't match: 0x%x != 0x%x\n",
- le32_to_cpu(pgpt->sizeof_partition_entry),
- le32_to_cpu(agpt->sizeof_partition_entry));
- error_found++;
- }
- if (le32_to_cpu(pgpt->partition_entry_array_crc32) !=
- le32_to_cpu(agpt->partition_entry_array_crc32)) {
- pr_warn("GPT:partition_entry_array_crc32 values don't match: 0x%x != 0x%x\n",
- le32_to_cpu(pgpt->partition_entry_array_crc32),
- le32_to_cpu(agpt->partition_entry_array_crc32));
- error_found++;
- }
- if (le64_to_cpu(pgpt->alternate_lba) != lastlba) {
- pr_warn("GPT:Primary header thinks Alt. header is not at the end of the disk.\n");
- pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(pgpt->alternate_lba),
- (unsigned long long)lastlba);
- error_found++;
- }
-
- if (le64_to_cpu(agpt->my_lba) != lastlba) {
- pr_warn("GPT:Alternate GPT header not at the end of the disk.\n");
- pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(agpt->my_lba),
- (unsigned long long)lastlba);
- error_found++;
- }
-
- if (error_found)
- pr_warn("GPT: Use GNU Parted to correct GPT errors.\n");
- return error_found;
-}
-
/**
* find_valid_gpt() - Search disk for valid GPT headers and PTEs
* @state: disk parsed partitions
@@ -682,31 +371,6 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
return 0;
}
-/**
- * utf16_le_to_7bit(): Naively converts a UTF-16LE string to 7-bit ASCII characters
- * @in: input UTF-16LE string
- * @size: size of the input string
- * @out: output string ptr, should be capable to store @size+1 characters
- *
- * Description: Converts @size UTF16-LE symbols from @in string to 7-bit
- * ASCII characters and stores them to @out. Adds trailing zero to @out array.
- */
-void utf16_le_to_7bit(const __le16 *in, unsigned int size, u8 *out)
-{
- unsigned int i = 0;
-
- out[size] = 0;
-
- while (i < size) {
- u8 c = le16_to_cpu(in[i]) & 0xff;
-
- if (c && !isprint(c))
- c = '!';
- out[i] = c;
- i++;
- }
-}
-
/**
* efi_partition - scan for GPT partitions
* @state: disk parsed partitions
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 9cc5bf32f6f2..c4b818f77de7 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -24,6 +24,7 @@
#include <linux/range.h>
#include <linux/reboot.h>
#include <linux/uuid.h>
+#include <linux/crc32.h>
#include <asm/page.h>
@@ -1348,4 +1349,21 @@ bool efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table)
umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n);
+/**
+ * efi_crc32() - EFI version of crc32 function
+ * @buf: buffer to calculate crc32 of
+ * @len: length of buf
+ *
+ * Description: Returns EFI-style CRC32 value for @buf
+ *
+ * This function uses the little endian Ethernet polynomial
+ * but seeds the function with ~0, and xor's with ~0 at the end.
+ * Note, the EFI Specification, v1.02, has a reference to
+ * Dr. Dobbs Journal, May 1994 (actually it's in May 1992).
+ */
+static inline u32 efi_crc32(const void *buf, unsigned long len)
+{
+ return (crc32(~0L, buf, len) ^ ~0L);
+}
+
#endif /* _LINUX_EFI_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index 3ea1c830efab..de776911944e 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -748,6 +748,9 @@ config GENERIC_LIB_ASHLDI3
config GENERIC_LIB_ASHRDI3
bool
+config GENERIC_LIB_GPT
+ bool
+
config GENERIC_LIB_LSHRDI3
bool
diff --git a/lib/Makefile b/lib/Makefile
index 6b09731d8e61..fba8fae70efa 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -444,3 +444,6 @@ $(obj)/$(TEST_FORTIFY_LOG): $(addprefix $(obj)/, $(TEST_FORTIFY_LOGS)) FORCE
ifeq ($(CONFIG_FORTIFY_SOURCE),y)
$(obj)/string.o: $(obj)/$(TEST_FORTIFY_LOG)
endif
+
+# GPT parsing routines
+obj-$(CONFIG_GENERIC_LIB_GPT) += gpt.o
diff --git a/lib/gpt.c b/lib/gpt.c
new file mode 100644
index 000000000000..16303634f4fb
--- /dev/null
+++ b/lib/gpt.c
@@ -0,0 +1,342 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* EFI GUID Partition Table handling
+ *
+ * http://www.uefi.org/specs/
+ * http://www.intel.com/technology/efi/
+ *
+ * efi.[ch] by Matt Domsch <Matt_Domsch@dell.com>
+ * Copyright 2000,2001,2002,2004 Dell Inc.
+ *
+ * This code was previously in block/partitions/efi.c
+ * and was moved in /lib so that other kernel subsystems
+ * could use it as a common GPT parsing library.
+ *
+ * This library should be stateless and not make any
+ * assumptions about the type of device the GPT data
+ * came from.
+ *
+ */
+
+#include <linux/gpt.h>
+#include <linux/efi.h>
+
+static inline int pmbr_part_valid(gpt_mbr_record *part)
+{
+ if (part->os_type != EFI_PMBR_OSTYPE_EFI_GPT)
+ goto invalid;
+
+ /* set to 0x00000001 (i.e., the LBA of the GPT Partition Header) */
+ if (le32_to_cpu(part->starting_lba) != GPT_PRIMARY_PARTITION_TABLE_LBA)
+ goto invalid;
+
+ return GPT_MBR_PROTECTIVE;
+invalid:
+ return 0;
+}
+
+/**
+ * gpt_is_pmbr_valid(): test Protective MBR for validity
+ * @mbr: pointer to a legacy mbr structure
+ * @total_sectors: amount of sectors in the device
+ *
+ * Description: Checks for a valid protective or hybrid
+ * master boot record (MBR). The validity of a pMBR depends
+ * on all of the following properties:
+ * 1) MSDOS signature is in the last two bytes of the MBR
+ * 2) One partition of type 0xEE is found
+ *
+ * In addition, a hybrid MBR will have up to three additional
+ * primary partitions, which point to the same space that's
+ * marked out by up to three GPT partitions.
+ *
+ * Returns 0 upon invalid MBR, or GPT_MBR_PROTECTIVE or
+ * GPT_MBR_HYBRID depending on the device layout.
+ */
+int gpt_is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors)
+{
+ int i, part = 0, ret = 0; /* invalid by default */
+ uint32_t sz = 0;
+
+ if (!mbr || le16_to_cpu(mbr->signature) != MSDOS_MBR_SIGNATURE)
+ goto done;
+
+ for (i = 0; i < 4; i++) {
+ ret = pmbr_part_valid(&mbr->partition_record[i]);
+ if (ret == GPT_MBR_PROTECTIVE) {
+ part = i;
+ /*
+ * Ok, we at least know that there's a protective MBR,
+ * now check if there are other partition types for
+ * hybrid MBR.
+ */
+ goto check_hybrid;
+ }
+ }
+
+ if (ret != GPT_MBR_PROTECTIVE)
+ goto done;
+check_hybrid:
+ for (i = 0; i < 4; i++)
+ if (mbr->partition_record[i].os_type != EFI_PMBR_OSTYPE_EFI_GPT &&
+ mbr->partition_record[i].os_type != 0x00)
+ ret = GPT_MBR_HYBRID;
+
+ /*
+ * Protective MBRs take up the lesser of the whole disk
+ * or 2 TiB (32bit LBA), ignoring the rest of the disk.
+ * Some partitioning programs, nonetheless, choose to set
+ * the size to the maximum 32-bit limitation, disregarding
+ * the disk size.
+ *
+ * Hybrid MBRs do not necessarily comply with this.
+ *
+ * Consider a bad value here to be a warning to support dd'ing
+ * an image from a smaller disk to a larger disk.
+ */
+ if (ret == GPT_MBR_PROTECTIVE) {
+ sz = le32_to_cpu(mbr->partition_record[part].size_in_lba);
+ if (sz != (uint32_t)total_sectors - 1 && sz != 0xFFFFFFFF)
+ pr_debug("GPT: mbr size in lba (%u) different than whole disk (%u).\n",
+ sz, min_t(uint32_t,
+ total_sectors - 1, 0xFFFFFFFF));
+ }
+done:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(gpt_is_pmbr_valid);
+
+/**
+ * gpt_validate_header() - tests one GPT header for validity
+ * @gpt: header to check
+ * @lba: logical block address of the GPT header to test
+ * @lba_size: logical block size of the partitioned device
+ * @lastlba: last logical block on the partitioned device
+ *
+ * Returns 0 if validation was successful.
+ */
+int gpt_validate_header(gpt_header *gpt, u64 lba, unsigned int lba_size,
+ u64 lastlba)
+{
+ u32 crc, origcrc;
+ u64 pt_size;
+
+ /* Check the GUID Partition Table signature */
+ if (le64_to_cpu(gpt->signature) != GPT_HEADER_SIGNATURE) {
+ pr_debug("GUID Partition Table Header signature is wrong: %lld != %lld\n",
+ (unsigned long long)le64_to_cpu(gpt->signature),
+ (unsigned long long)GPT_HEADER_SIGNATURE);
+ return -EINVAL;
+ }
+
+ /* Check the GUID Partition Table header size is too big */
+ if (le32_to_cpu(gpt->header_size) > lba_size) {
+ pr_debug("GUID Partition Table Header size is too large: %u > %u\n",
+ le32_to_cpu(gpt->header_size), lba_size);
+ return -EINVAL;
+ }
+
+ /* Check the GUID Partition Table header size is too small */
+ if (le32_to_cpu(gpt->header_size) < sizeof(gpt_header)) {
+ pr_debug("GUID Partition Table Header size is too small: %u < %zu\n",
+ le32_to_cpu(gpt->header_size),
+ sizeof(gpt_header));
+ return -EINVAL;
+ }
+
+ /* Check the GUID Partition Table CRC */
+ origcrc = le32_to_cpu(gpt->header_crc32);
+ gpt->header_crc32 = 0;
+ crc = efi_crc32((const unsigned char *)gpt, le32_to_cpu(gpt->header_size));
+
+ if (crc != origcrc) {
+ pr_debug("GUID Partition Table Header CRC is wrong: %x != %x\n",
+ crc, origcrc);
+ return -EINVAL;
+ }
+ gpt->header_crc32 = cpu_to_le32(origcrc);
+
+ /* Check that the my_lba entry points to the LBA that contains
+ * the GUID Partition Table
+ */
+ if (le64_to_cpu(gpt->my_lba) != lba) {
+ pr_debug("GPT my_lba incorrect: %lld != %lld\n",
+ (unsigned long long)le64_to_cpu(gpt->my_lba),
+ (unsigned long long)lba);
+ return -EINVAL;
+ }
+
+ /* Check the first_usable_lba and last_usable_lba are
+ * within the disk.
+ */
+ if (le64_to_cpu(gpt->first_usable_lba) > lastlba) {
+ pr_debug("GPT: first_usable_lba incorrect: %lld > %lld\n",
+ (unsigned long long)le64_to_cpu(gpt->first_usable_lba),
+ (unsigned long long)lastlba);
+ return -EINVAL;
+ }
+ if (le64_to_cpu(gpt->last_usable_lba) > lastlba) {
+ pr_debug("GPT: last_usable_lba incorrect: %lld > %lld\n",
+ (unsigned long long)le64_to_cpu(gpt->last_usable_lba),
+ (unsigned long long)lastlba);
+ return -EINVAL;
+ }
+ if (le64_to_cpu(gpt->last_usable_lba) < le64_to_cpu(gpt->first_usable_lba)) {
+ pr_debug("GPT: last_usable_lba incorrect: %lld > %lld\n",
+ (unsigned long long)le64_to_cpu(gpt->last_usable_lba),
+ (unsigned long long)le64_to_cpu(gpt->first_usable_lba));
+ return -EINVAL;
+ }
+
+ /* Check that sizeof_partition_entry has the correct value */
+ if (le32_to_cpu(gpt->sizeof_partition_entry) != sizeof(gpt_entry)) {
+ pr_debug("GUID Partition Entry Size check failed.\n");
+ return -EINVAL;
+ }
+
+ /* Sanity check partition table size */
+ pt_size = (u64)get_pt_size(gpt);
+ if (pt_size > KMALLOC_MAX_SIZE) {
+ pr_debug("GUID Partition Table is too large: %llu > %lu bytes\n",
+ (unsigned long long)pt_size, KMALLOC_MAX_SIZE);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(gpt_validate_header);
+
+/* Check the GUID Partition Entry Array CRC */
+int gpt_check_pte_array_crc(gpt_header *gpt, gpt_entry *ptes)
+{
+ u32 crc;
+
+ crc = efi_crc32((const unsigned char *)ptes, get_pt_size(gpt));
+ if (crc != le32_to_cpu(gpt->partition_entry_array_crc32)) {
+ pr_debug("GUID Partition Entry Array CRC check failed.\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(gpt_check_pte_array_crc);
+
+/**
+ * gpt_compare_alt() - Compares the Primary and Alternate GPT headers
+ * @pgpt: primary GPT header
+ * @agpt: alternate GPT header
+ * @lastlba: last LBA number
+ *
+ * Description: Sanity checks pgpt and agpt fields and prints warnings
+ * on discrepancies. Returns error count. GPT parsers can choose to
+ * ignore this or not.
+ *
+ */
+int gpt_compare_alt(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
+{
+ int error_found = 0;
+
+ if (!pgpt || !agpt)
+ return -EINVAL;
+
+ if (le64_to_cpu(pgpt->my_lba) != le64_to_cpu(agpt->alternate_lba)) {
+ pr_warn("GPT:Primary header LBA != Alt. header alternate_lba\n");
+ pr_warn("GPT:%lld != %lld\n",
+ (unsigned long long)le64_to_cpu(pgpt->my_lba),
+ (unsigned long long)le64_to_cpu(agpt->alternate_lba));
+ error_found++;
+ }
+ if (le64_to_cpu(pgpt->alternate_lba) != le64_to_cpu(agpt->my_lba)) {
+ pr_warn("GPT:Primary header alternate_lba != Alt. header my_lba\n");
+ pr_warn("GPT:%lld != %lld\n",
+ (unsigned long long)le64_to_cpu(pgpt->alternate_lba),
+ (unsigned long long)le64_to_cpu(agpt->my_lba));
+ error_found++;
+ }
+ if (le64_to_cpu(pgpt->first_usable_lba) !=
+ le64_to_cpu(agpt->first_usable_lba)) {
+ pr_warn("GPT:first_usable_lbas don't match.\n");
+ pr_warn("GPT:%lld != %lld\n",
+ (unsigned long long)le64_to_cpu(pgpt->first_usable_lba),
+ (unsigned long long)le64_to_cpu(agpt->first_usable_lba));
+ error_found++;
+ }
+ if (le64_to_cpu(pgpt->last_usable_lba) !=
+ le64_to_cpu(agpt->last_usable_lba)) {
+ pr_warn("GPT:last_usable_lbas don't match.\n");
+ pr_warn("GPT:%lld != %lld\n",
+ (unsigned long long)le64_to_cpu(pgpt->last_usable_lba),
+ (unsigned long long)le64_to_cpu(agpt->last_usable_lba));
+ error_found++;
+ }
+ if (efi_guidcmp(pgpt->disk_guid, agpt->disk_guid)) {
+ pr_warn("GPT:disk_guids don't match.\n");
+ error_found++;
+ }
+ if (le32_to_cpu(pgpt->num_partition_entries) !=
+ le32_to_cpu(agpt->num_partition_entries)) {
+ pr_warn("GPT:num_partition_entries don't match: 0x%x != 0x%x\n",
+ le32_to_cpu(pgpt->num_partition_entries),
+ le32_to_cpu(agpt->num_partition_entries));
+ error_found++;
+ }
+ if (le32_to_cpu(pgpt->sizeof_partition_entry) !=
+ le32_to_cpu(agpt->sizeof_partition_entry)) {
+ pr_warn("GPT:sizeof_partition_entry values don't match: 0x%x != 0x%x\n",
+ le32_to_cpu(pgpt->sizeof_partition_entry),
+ le32_to_cpu(agpt->sizeof_partition_entry));
+ error_found++;
+ }
+ if (le32_to_cpu(pgpt->partition_entry_array_crc32) !=
+ le32_to_cpu(agpt->partition_entry_array_crc32)) {
+ pr_warn("GPT:partition_entry_array_crc32 values don't match: 0x%x != 0x%x\n",
+ le32_to_cpu(pgpt->partition_entry_array_crc32),
+ le32_to_cpu(agpt->partition_entry_array_crc32));
+ error_found++;
+ }
+ if (le64_to_cpu(pgpt->alternate_lba) != lastlba) {
+ pr_warn("GPT:Primary header thinks Alt. header is not at the end of the disk.\n");
+ pr_warn("GPT:%lld != %lld\n",
+ (unsigned long long)le64_to_cpu(pgpt->alternate_lba),
+ (unsigned long long)lastlba);
+ error_found++;
+ }
+
+ if (le64_to_cpu(agpt->my_lba) != lastlba) {
+ pr_warn("GPT:Alternate GPT header not at the end of the disk.\n");
+ pr_warn("GPT:%lld != %lld\n",
+ (unsigned long long)le64_to_cpu(agpt->my_lba),
+ (unsigned long long)lastlba);
+ error_found++;
+ }
+
+ if (error_found)
+ pr_warn("GPT: Use GNU Parted to correct GPT errors.\n");
+ return error_found;
+}
+EXPORT_SYMBOL_GPL(gpt_compare_alt);
+
+/**
+ * utf16_le_to_7bit(): Naively converts a UTF-16LE string to 7-bit ASCII characters
+ * @in: input UTF-16LE string
+ * @size: size of the input string
+ * @out: output string ptr, should be capable to store @size+1 characters
+ *
+ * Description: Converts @size UTF16-LE symbols from @in string to 7-bit
+ * ASCII characters and stores them to @out. Adds trailing zero to @out array.
+ */
+void utf16_le_to_7bit(const __le16 *in, unsigned int size, u8 *out)
+{
+ unsigned int i = 0;
+
+ out[size] = 0;
+
+ while (i < size) {
+ u8 c = le16_to_cpu(in[i]) & 0xff;
+
+ if (c && !isprint(c))
+ c = '!';
+ out[i] = c;
+ i++;
+ }
+}
+EXPORT_SYMBOL_GPL(utf16_le_to_7bit);
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [RFC PATCH 5/6] drivers: mtd: introduce GPT parser for NOR flash devices
2023-12-11 15:12 [RFC PATCH 0/6] Add GPT parser to MTD layer Romain Gantois
` (3 preceding siblings ...)
2023-12-11 15:12 ` [RFC PATCH 4/6] block: partitions: efi: Move GPT-specific code to a new library Romain Gantois
@ 2023-12-11 15:12 ` Romain Gantois
2023-12-11 15:12 ` [RFC PATCH 6/6] dt-bindings: mtd: add GPT partition bindings Romain Gantois
2023-12-12 0:43 ` [RFC PATCH 0/6] Add GPT parser to MTD layer Davidlohr Bueso
6 siblings, 0 replies; 11+ messages in thread
From: Romain Gantois @ 2023-12-11 15:12 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jens Axboe,
Davidlohr Bueso, Ard Biesheuvel
Cc: Romain Gantois, Thomas Petazzoni, Herve Codina, linux-mtd,
devicetree, linux-kernel, linux-block, linux-efi
MTD devices can be partitioned using a fixed partitioning scheme defined in
the relevant device tree node. Dynamic partitioning is also possible, using
MTD parsers. They are capable of parsing partition tables on the device at
boot time.
Add support for GPT parsing at the MTD layer. The parser logic makes use of
a common set of GPT helpers and typedefs that have previously been
extracted from the block layer parser.
The main issue with GPT partitioning are the userspace tools which are
already capable of writing a partition table. Not using them would require
re-inventing the wheel, but on the other side they are block-oriented tools
with absolutely no knowledge of the specificities of MTD flashes. Tools
such as parted or fdisk act upon block devices and not mtd devices, so we
need a block abstraction for writing the table. Thus, the only possible
solution so far is the use of mtdblock in-between, which raises two main
concerns: the lack of bad-block handling and wear-leveling. In order to
avoid risky situations, the parser will only parse NORs.
Example usage:
> echo "
device: /dev/mtdblock0
unit: sectors
first-lba: 40
last-lba: 32734
sector-size: 512
start=40, size=8
start=48, size=80
start=128, size=32600
" | sfdisk /dev/mtdblock0
After rebooting:
> lsmtd
DEVICE MAJ:MIN NAME TYPE SIZE
mtd0 90:0 spi0.0 nor 16M
mtd1 90:2 nor 4K
mtd2 90:4 nor 40K
mtd3 90:6 nor 15.9M
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
drivers/mtd/parsers/Kconfig | 10 ++
drivers/mtd/parsers/Makefile | 1 +
drivers/mtd/parsers/gpt.c | 222 +++++++++++++++++++++++++++++++++++
3 files changed, 233 insertions(+)
create mode 100644 drivers/mtd/parsers/gpt.c
diff --git a/drivers/mtd/parsers/Kconfig b/drivers/mtd/parsers/Kconfig
index da03ab6efe04..3f34220c6fc3 100644
--- a/drivers/mtd/parsers/Kconfig
+++ b/drivers/mtd/parsers/Kconfig
@@ -118,6 +118,16 @@ config MTD_AFS_PARTS
for your particular device. It won't happen automatically. The
'physmap' map driver (CONFIG_MTD_PHYSMAP) does this, for example.
+config MTD_GPT_PARTS
+ tristate "GPT partition table parsing"
+ select GENERIC_LIB_GPT
+ help
+ This allow specifying MTD partitions on a NOR Flash device using
+ a fixed-location GPT partition table. No wear-leveling is done by
+ this parser, and it cannot recover from bad blocks in critical GPT
+ metadata locations. For these reasons, NAND Flash devices cannot
+ make use of this partition parser.
+
config MTD_PARSER_TPLINK_SAFELOADER
tristate "TP-Link Safeloader partitions parser"
depends on MTD && (ARCH_BCM_5301X || ATH79 || SOC_MT7620 || SOC_MT7621 || COMPILE_TEST)
diff --git a/drivers/mtd/parsers/Makefile b/drivers/mtd/parsers/Makefile
index 9b00c62b837a..eb31585f2dd1 100644
--- a/drivers/mtd/parsers/Makefile
+++ b/drivers/mtd/parsers/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_MTD_SERCOMM_PARTS) += scpart.o
obj-$(CONFIG_MTD_SHARPSL_PARTS) += sharpslpart.o
obj-$(CONFIG_MTD_REDBOOT_PARTS) += redboot.o
obj-$(CONFIG_MTD_QCOMSMEM_PARTS) += qcomsmempart.o
+obj-$(CONFIG_MTD_GPT_PARTS) += gpt.o
diff --git a/drivers/mtd/parsers/gpt.c b/drivers/mtd/parsers/gpt.c
new file mode 100644
index 000000000000..c2fa9545b4b5
--- /dev/null
+++ b/drivers/mtd/parsers/gpt.c
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * MTD parser for GPT partition tables.
+ *
+ * This parser supports GPT partition tables located at fixed standard sectors.
+ * Partitioning a raw flash device in this manner prevents wear-leveling and bad
+ * block handling at the whole-device level. Note that bad blocks on critical
+ * GPT sectors will completely break your partition table! Because of this, use
+ * of this parser is restricted to NOR flash devices, which are less susceptible
+ * to bad blocks than NAND flash devices.
+ *
+ * http://www.uefi.org/specs/
+ *
+ * Acronyms:
+ * PTE: Partition Table Entry
+ * LBA: Logical Block Address
+ *
+ * Copyright © 2023 Bootlin
+ *
+ * Author: Romain Gantois <romain.gantois@bootlin.com>
+ *
+ */
+
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/mtd.h>
+#include <linux/minmax.h>
+#include <linux/kernel.h>
+#include <linux/gpt.h>
+
+/*
+ * We assume that the GPT partition was written through an mtdblock device. This
+ * would make the LBA size 512.
+ */
+#define MTD_GPT_LBA_SIZE 512
+
+/*
+ * This value is pretty much arbitrary, it's in the range of typical MTD parser
+ * caps. Creating too many partitions on a raw Flash device is a bad idea
+ * anyway.
+ */
+#define MTD_GPT_MAX_PARTS 32
+
+#define mtd_gpt_lba_to_offset(x) ((x) * MTD_GPT_LBA_SIZE)
+#define mtd_gpt_lba_to_size(x) ((size_t)(x) * MTD_GPT_LBA_SIZE)
+
+#define MTD_GPT_PARTNAME_SIZE sizeof(((gpt_entry *)0)->partition_name)
+
+static int mtd_gpt_read_header(struct mtd_info *mtd, gpt_header *gpt, int lba, u64 last_lba)
+{
+ loff_t read_offset = mtd_gpt_lba_to_offset(lba);
+ size_t retlen;
+ int ret;
+
+ dev_dbg(&mtd->dev, "reading GPT header at offset %lli\n", read_offset);
+
+ ret = mtd_read(mtd, read_offset, MTD_GPT_LBA_SIZE, &retlen, (u_char *)gpt);
+ if (ret) {
+ dev_err(&mtd->dev, "failed to read GPT header\n");
+ return -EIO;
+ }
+
+ if (retlen < MTD_GPT_LBA_SIZE) {
+ dev_err(&mtd->dev, "truncated read 0x%zx expected 0x%x\n",
+ retlen, MTD_GPT_LBA_SIZE);
+ return -EIO;
+ }
+
+ return gpt_validate_header(gpt, lba, MTD_GPT_LBA_SIZE, last_lba);
+}
+
+static int mtd_gpt_parse_partitions(struct mtd_info *mtd,
+ const struct mtd_partition **pparts,
+ struct mtd_part_parser_data *data)
+{
+ int ret = 0, valid_parts = 0, part_no;
+ u64 last_lba = div_u64(mtd->size, MTD_GPT_LBA_SIZE) - 1ULL;
+ struct mtd_partition *parts, *part;
+ size_t pte_read_size, retlen;
+ char *name_buf, *cur_name;
+ loff_t pte_read_offset;
+ gpt_header *gpt, *agpt;
+ gpt_entry *ptes, *pte;
+ unsigned int nr_parts;
+
+ if (mtd_type_is_nand(mtd)) {
+ /* NAND flash devices are too susceptible to bad blocks */
+ dev_err(&mtd->dev, "GPT parsing is forbidden on NAND devices!");
+ return -EPERM;
+ }
+
+ gpt = kzalloc(MTD_GPT_LBA_SIZE, GFP_KERNEL);
+ if (!gpt)
+ return -ENOMEM;
+
+ agpt = kzalloc(MTD_GPT_LBA_SIZE, GFP_KERNEL);
+ if (!agpt)
+ return -ENOMEM;
+
+ ret = mtd_gpt_read_header(mtd, gpt, GPT_PRIMARY_PARTITION_TABLE_LBA, last_lba);
+ if (ret) {
+ dev_warn(&mtd->dev,
+ "invalid primary GPT header: error %d, trying alternate", ret);
+ ret = mtd_gpt_read_header(mtd, gpt, le64_to_cpu(gpt->alternate_lba), last_lba);
+ if (ret) {
+ dev_err(&mtd->dev, "invalid alternate GPT header: error %d\n", ret);
+ goto free_gpt;
+ }
+ } else {
+ /* Check alternate header and warn if it doesn't match primary header */
+ gpt_compare_alt(gpt, agpt, last_lba);
+ }
+
+ /* This should contain the PTE array */
+ pte_read_offset = mtd_gpt_lba_to_offset(le64_to_cpu(gpt->partition_entry_lba));
+ pte_read_size = get_pt_size(gpt);
+ ptes = kzalloc(pte_read_size, GFP_KERNEL);
+ if (!ptes) {
+ ret = -ENOMEM;
+ goto free_gpt;
+ }
+
+ dev_dbg(&mtd->dev, "reading PTE array offset %lli size 0x%zx\n",
+ pte_read_offset, pte_read_size);
+
+ ret = mtd_read(mtd, pte_read_offset, pte_read_size, &retlen, (u_char *)ptes);
+ if (ret)
+ goto free_ptes;
+
+ if (retlen < pte_read_size) {
+ ret = -EIO;
+ goto free_ptes;
+ }
+
+ ret = gpt_check_pte_array_crc(gpt, ptes);
+ if (ret) {
+ dev_err(&mtd->dev,
+ "CRC check failed for GPT Partition Table Entry array! error %d\n", ret);
+ goto free_ptes;
+ }
+
+ nr_parts = le32_to_cpu(gpt->num_partition_entries);
+ parts = kcalloc(min(nr_parts, MTD_GPT_MAX_PARTS),
+ sizeof(*parts),
+ GFP_KERNEL);
+ if (!parts) {
+ ret = -ENOMEM;
+ goto free_ptes;
+ }
+
+ name_buf = kcalloc(min(nr_parts, MTD_GPT_MAX_PARTS), (MTD_GPT_PARTNAME_SIZE + 1),
+ GFP_KERNEL);
+ if (!name_buf) {
+ ret = -ENOMEM;
+ goto free_parts;
+ }
+
+ for (part_no = 0; part_no < nr_parts && valid_parts < MTD_GPT_MAX_PARTS; part_no++) {
+ pte = &ptes[part_no];
+ part = &parts[valid_parts];
+ cur_name = &name_buf[valid_parts * (MTD_GPT_PARTNAME_SIZE + 1)];
+
+ if (!gpt_is_pte_valid(pte, last_lba)) {
+ dev_warn(&mtd->dev, "skipping invalid partition entry %d!\n", part_no);
+ continue;
+ }
+
+ part->offset = mtd_gpt_lba_to_offset(le64_to_cpu(pte->starting_lba));
+ part->size = mtd_gpt_lba_to_size(le64_to_cpu(pte->ending_lba) -
+ le64_to_cpu(pte->starting_lba) + 1ULL);
+ part->name = cur_name;
+
+ /* part->name is const so we can't pass it directly */
+ utf16_le_to_7bit(pte->partition_name,
+ MTD_GPT_PARTNAME_SIZE / sizeof(__le16),
+ cur_name);
+ valid_parts++;
+ }
+
+ if (valid_parts == MTD_GPT_MAX_PARTS)
+ dev_warn(&mtd->dev,
+ "reached maximum allowed number of MTD partitions %d\n",
+ MTD_GPT_MAX_PARTS);
+
+ *pparts = parts;
+ kfree(ptes);
+ kfree(gpt);
+ return valid_parts;
+
+free_parts:
+ kfree(parts);
+free_ptes:
+ kfree(ptes);
+free_gpt:
+ kfree(agpt);
+ kfree(gpt);
+
+ return ret;
+}
+
+static void mtd_gpt_cleanup_partitions(const struct mtd_partition *pparts, int nr_parts)
+{
+ kfree(pparts->name);
+ kfree(pparts);
+}
+
+static const struct of_device_id mtd_gpt_of_match_table[] = {
+ { .compatible = "gpt" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mtd_gpt_of_match_table);
+
+static struct mtd_part_parser mtd_gpt_parser = {
+ .parse_fn = mtd_gpt_parse_partitions,
+ .cleanup = mtd_gpt_cleanup_partitions,
+ .name = "GPT",
+ .of_match_table = mtd_gpt_of_match_table,
+};
+module_mtd_part_parser(mtd_gpt_parser);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Romain Gantois <romain.gantois@bootlin.com>");
+MODULE_DESCRIPTION("MTD parser for GPT partition tables");
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [RFC PATCH 6/6] dt-bindings: mtd: add GPT partition bindings
2023-12-11 15:12 [RFC PATCH 0/6] Add GPT parser to MTD layer Romain Gantois
` (4 preceding siblings ...)
2023-12-11 15:12 ` [RFC PATCH 5/6] drivers: mtd: introduce GPT parser for NOR flash devices Romain Gantois
@ 2023-12-11 15:12 ` Romain Gantois
2023-12-11 16:21 ` Rob Herring
2023-12-12 0:43 ` [RFC PATCH 0/6] Add GPT parser to MTD layer Davidlohr Bueso
6 siblings, 1 reply; 11+ messages in thread
From: Romain Gantois @ 2023-12-11 15:12 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jens Axboe,
Davidlohr Bueso, Ard Biesheuvel
Cc: Romain Gantois, Thomas Petazzoni, Herve Codina, linux-mtd,
devicetree, linux-kernel, linux-block, linux-efi
Allow parsing GPT layouts on MTD devices.
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
.../bindings/mtd/partitions/gpt.yaml | 41 +++++++++++++++++++
.../bindings/mtd/partitions/partitions.yaml | 1 +
2 files changed, 42 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/partitions/gpt.yaml
diff --git a/Documentation/devicetree/bindings/mtd/partitions/gpt.yaml b/Documentation/devicetree/bindings/mtd/partitions/gpt.yaml
new file mode 100644
index 000000000000..3c538562e3e5
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/partitions/gpt.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/partitions/gpt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GUID Partition Table (GPT)
+
+description: The GPT format is commonly used on block devices to describe a
+partitioning scheme. It mainly consists of a Legacy or Protective MBR for
+backwards compatibility, a primary GPT header with an array of Partition Table
+Entries, and a backup header with a backup array of PTEs. This partition table
+format can be used on MTD devices, specifically NOR flash devices, since NAND
+flashes are susceptible to bad blocks which could easily corrupt the GPT layout.
+Logical Block Addresses (LBAs) are defined to target 512-byte blocks.
+
+maintainers:
+ - Romain Gantois <romain.gantois@bootlin.com>
+
+select: false
+
+properties:
+ compatible:
+ const: gpt
+
+ '#address-cells': false
+
+ '#size-cells': false
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ flash@0 {
+ partitions {
+ compatible = "gpt";
+ };
+ };
diff --git a/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
index 1dda2c80747b..f2b1565d5d0a 100644
--- a/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
+++ b/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
@@ -18,6 +18,7 @@ oneOf:
- $ref: brcm,bcm4908-partitions.yaml
- $ref: brcm,bcm947xx-cfe-partitions.yaml
- $ref: fixed-partitions.yaml
+ - $ref: gpt.yaml
- $ref: linksys,ns-partitions.yaml
- $ref: qcom,smem-part.yaml
- $ref: redboot-fis.yaml
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [RFC PATCH 6/6] dt-bindings: mtd: add GPT partition bindings
2023-12-11 15:12 ` [RFC PATCH 6/6] dt-bindings: mtd: add GPT partition bindings Romain Gantois
@ 2023-12-11 16:21 ` Rob Herring
0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2023-12-11 16:21 UTC (permalink / raw)
To: Romain Gantois
Cc: Vignesh Raghavendra, Davidlohr Bueso, linux-mtd, devicetree,
linux-kernel, Thomas Petazzoni, Rob Herring, Ard Biesheuvel,
linux-block, Miquel Raynal, Herve Codina, Jens Axboe,
Krzysztof Kozlowski, Conor Dooley, linux-efi, Richard Weinberger
On Mon, 11 Dec 2023 16:12:42 +0100, Romain Gantois wrote:
> Allow parsing GPT layouts on MTD devices.
>
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
> ---
> .../bindings/mtd/partitions/gpt.yaml | 41 +++++++++++++++++++
> .../bindings/mtd/partitions/partitions.yaml | 1 +
> 2 files changed, 42 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mtd/partitions/gpt.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/mtd/partitions/gpt.yaml:11:1: [error] syntax error: could not find expected ':' (syntax)
dtschema/dtc warnings/errors:
make[2]: *** Deleting file 'Documentation/devicetree/bindings/mtd/partitions/gpt.example.dts'
Documentation/devicetree/bindings/mtd/partitions/gpt.yaml:11:1: could not find expected ':'
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/mtd/partitions/gpt.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml:
while scanning a simple key
in "<unicode string>", line 10, column 1
could not find expected ':'
in "<unicode string>", line 11, column 1
./Documentation/devicetree/bindings/mtd/partitions/gpt.yaml:11:1: could not find expected ':'
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/partitions/gpt.yaml: ignoring, error parsing file
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1424: dt_binding_check] Error 2
make: *** [Makefile:234: __sub-make] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231211151244.289349-7-romain.gantois@bootlin.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 0/6] Add GPT parser to MTD layer
2023-12-11 15:12 [RFC PATCH 0/6] Add GPT parser to MTD layer Romain Gantois
` (5 preceding siblings ...)
2023-12-11 15:12 ` [RFC PATCH 6/6] dt-bindings: mtd: add GPT partition bindings Romain Gantois
@ 2023-12-12 0:43 ` Davidlohr Bueso
2023-12-14 10:34 ` Miquel Raynal
6 siblings, 1 reply; 11+ messages in thread
From: Davidlohr Bueso @ 2023-12-12 0:43 UTC (permalink / raw)
To: Romain Gantois
Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jens Axboe,
Ard Biesheuvel, Thomas Petazzoni, Herve Codina, linux-mtd,
devicetree, linux-kernel, linux-block, linux-efi
On Mon, 11 Dec 2023, Romain Gantois wrote:
>Hello everyone,
>
>MTD devices were historically partitioned using fixed partitions schemes
>defined in the kernel device tree or on the cmdline. More recently, a bunch
>of dynamic parsers have been introduced, allowing partitioning information
>to be stored in-band. However, unlike disks, parsers for MTD devices do not
>support runtime discovery of the partition format. This format is instead
>named in the device-tree using a compatible string.
>
>The GUID Partition Table is one of the most common ways of partitioning a
>block device. As of now, there is no support in the MTD layer for parsing
>GPT tables. Indeed, use cases for layouts like GPT on raw Flash devices are
>rare, and for good reason since these partitioning schemes are sensitive to
>bad blocks in strategic locations such as LBA 2. Moreover, they do not
>allow proper wear-leveling to be performed on the full span of the device.
>
>However, allowing GPT to be used on MTD devices can be practical in some
>cases. In the context of an A/B OTA upgrade that can act on either NOR of
>eMMC devices, having the same partition table format for both kinds of
>devices can simplify the task of the update software.
>
>This series adds a fully working MTD GPT parser to the kernel. Use of the
>parser is restricted to NOR flash devices, since NAND flashes are too
>susceptible to bad blocks. To ensure coherence and code-reuse between
>subsystems, I've factored device-agnostic code from the block layer GPT
>parser and moved it to a new generic library in lib/gpt.c. No functional
>change is intended in the block layer parser.
>
>I understand that this can seem like a strange feature for MTD devices, but
>with the restriction to NOR devices, the partition table can be fairly
>reliable. Moreover, this addition fits nicely into the MTD parser model.
>Please tell me what you think.
I am not a fan of this. The usecase seems very hacky and ad-hoc to justify
decoupling from the block layer, not to mention move complexity out of
userspace and into the kernel (new parser) for something that is already
being done/worked around. Also, what other user would consume this new gpt
lib abstraction in the future? I don't think it is worth it.
Thanks,
Davidlohr
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC PATCH 0/6] Add GPT parser to MTD layer
2023-12-12 0:43 ` [RFC PATCH 0/6] Add GPT parser to MTD layer Davidlohr Bueso
@ 2023-12-14 10:34 ` Miquel Raynal
0 siblings, 0 replies; 11+ messages in thread
From: Miquel Raynal @ 2023-12-14 10:34 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: Romain Gantois, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jens Axboe,
Ard Biesheuvel, Thomas Petazzoni, Herve Codina, linux-mtd,
devicetree, linux-kernel, linux-block, linux-efi
Hi Davidlohr,
dave@stgolabs.net wrote on Mon, 11 Dec 2023 16:43:58 -0800:
> On Mon, 11 Dec 2023, Romain Gantois wrote:
>
> >Hello everyone,
> >
> >MTD devices were historically partitioned using fixed partitions schemes
> >defined in the kernel device tree or on the cmdline. More recently, a bunch
> >of dynamic parsers have been introduced, allowing partitioning information
> >to be stored in-band. However, unlike disks, parsers for MTD devices do not
> >support runtime discovery of the partition format. This format is instead
> >named in the device-tree using a compatible string.
> >
> >The GUID Partition Table is one of the most common ways of partitioning a
> >block device. As of now, there is no support in the MTD layer for parsing
> >GPT tables. Indeed, use cases for layouts like GPT on raw Flash devices are
> >rare, and for good reason since these partitioning schemes are sensitive to
> >bad blocks in strategic locations such as LBA 2. Moreover, they do not
> >allow proper wear-leveling to be performed on the full span of the device.
> >
> >However, allowing GPT to be used on MTD devices can be practical in some
> >cases. In the context of an A/B OTA upgrade that can act on either NOR of
> >eMMC devices, having the same partition table format for both kinds of
> >devices can simplify the task of the update software.
> >
> >This series adds a fully working MTD GPT parser to the kernel. Use of the
> >parser is restricted to NOR flash devices, since NAND flashes are too
> >susceptible to bad blocks. To ensure coherence and code-reuse between
> >subsystems, I've factored device-agnostic code from the block layer GPT
> >parser and moved it to a new generic library in lib/gpt.c. No functional
> >change is intended in the block layer parser.
> >
> >I understand that this can seem like a strange feature for MTD devices, but
> >with the restriction to NOR devices, the partition table can be fairly
> >reliable. Moreover, this addition fits nicely into the MTD parser model.
> >Please tell me what you think.
>
> I am not a fan of this. The usecase seems very hacky and ad-hoc to justify
> decoupling from the block layer,
The use case indeed is a bit ad-hoc, as it is an OTA tool which makes
it painful to handle two separate types of partitioning between blocks
and mtd devices, so being able to parse a GPT on an mtd device would
help a lot.
> not to mention move complexity out of
> userspace and into the kernel (new parser) for something that is already
> being done/worked around.
This is the part I don't fully agree with. There is no added
complexity, the parser exists and is kept untouched (apart from the
cosmetic changes). For a long time mtd partitioning information was
kept out of the storage (through fixed-partitions) but it's been quite
some time since the need for more flexible approaches arised, so we do
have "dynamic" partition parsers already and the one proposed by Romain
looks very straightforward and is thus not a problem to me. It
basically just extends the list of partition tables mtd devices know
about with a very common and popular format.
To be honest I do not have a strong opinion on whether this should be
merged or not but my reluctance is more about the mix of styles between
'block' and 'mtd'. People shall not treat them similarly for a number
of reasons, and this parser is an obvious step towards a more common
handling, knowing that it's been exclusively used on blocks for
decades.
> Also, what other user would consume this new gpt
> lib abstraction in the future? I don't think it is worth it.
Well, again I don't feel like this is a problem, sharing code between
two parties is already a win and the choice for a lib sounds rational
to me. The question being, shall we do it/do we want to do it.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 11+ messages in thread