[PATCH v2 2/3] part: efi: add GPT PTE cache used with part_get_info_cached() API
Neil Armstrong
neil.armstrong at linaro.org
Tue Apr 8 16:52:26 CEST 2025
On 08/04/2025 12:57, Heinrich Schuchardt wrote:
> On 08.04.25 11:13, Neil Armstrong wrote:
>> Implement a simple cache for part_efi to be used by the newly
>> introduced part_get_info_cached() API.
>>
>> The cache simply stores a successfully scanned GPT PTE if called
>> from the part_get_info_cached() ops, and will return the cached
>> data if the blk_desc and lba offset matches, invalidating the
>> cache if not.
>>
>> The cache will only operate if called from the part_get_info_cached()
>> API, all the over calls will operate uncached, making sure we
>> can still update & re-scan the GPT partitions like before.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong at linaro.org>
>> ---
>> disk/part_efi.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 96 insertions(+), 18 deletions(-)
>>
>> diff --git a/disk/part_efi.c b/disk/part_efi.c
>> index 932d058c184ce6946b7142e7c2d9637b28e4661e..949968fcd30fc18766a023e73f43ca1381dcef89 100644
>> --- a/disk/part_efi.c
>> +++ b/disk/part_efi.c
>> @@ -55,12 +55,58 @@ static inline u32 efi_crc32(const void *buf, u32 len)
>> static int pmbr_part_valid(struct partition *part);
>> static int is_pmbr_valid(legacy_mbr * mbr);
>> static int is_gpt_valid(struct blk_desc *desc, u64 lba, gpt_header *pgpt_head,
>> - gpt_entry **pgpt_pte);
>> + gpt_entry **pgpt_pte, bool cache);
>> static gpt_entry *alloc_read_gpt_entries(struct blk_desc *desc,
>> gpt_header *pgpt_head);
>> static int is_pte_valid(gpt_entry * pte);
>> static int find_valid_gpt(struct blk_desc *desc, gpt_header *gpt_head,
>> - gpt_entry **pgpt_pte);
>> + gpt_entry **pgpt_pte, bool cache);
>> +
>
> Please, describe the structure and its members.
Ack
>
> https://docs.kernel.org/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation
>
>> +static struct gpt_pte_cache_data {
>> + struct blk_desc *desc;
>> + u64 lba;
>> + gpt_entry *gpt_pte;
>> + gpt_header gpt_head;
>> +} gpt_pte_cache;
>> +
>
> Please, describe all functions.
Ack
>
>> +static void clear_gpt_pte_cache(void)
>> +{
>> + if (gpt_pte_cache.desc) {
>> + if (gpt_pte_cache.gpt_pte)
>> + free(gpt_pte_cache.gpt_pte);
>> +
>> + memset(&gpt_pte_cache, 0, sizeof(gpt_pte_cache));
>> + }
>> +}
>> +
>> +static void cache_gpt_pte(struct blk_desc *desc, u64 lba,
>> + gpt_entry *gpt_pte, gpt_header *pgpt_head)
>> +{
>> + if (gpt_pte_cache.gpt_pte)
>> + free(gpt_pte_cache.gpt_pte);
>> +
>> + gpt_pte_cache.desc = desc;
>> + gpt_pte_cache.lba = lba;
>> + gpt_pte_cache.gpt_pte = gpt_pte;
>> + if (pgpt_head)
>> + memcpy(&gpt_pte_cache.gpt_head, pgpt_head, sizeof(gpt_header));
>> +}
>> +
>> +static gpt_entry *get_cached_gpt_pte(struct blk_desc *desc, u64 lba,
>> + gpt_header *pgpt_head)
>> +{
>> + if (gpt_pte_cache.desc && gpt_pte_cache.gpt_pte) {
>> + if (gpt_pte_cache.desc == desc &&
>> + gpt_pte_cache.lba == lba) {
>> + memcpy(pgpt_head, &gpt_pte_cache.gpt_head, sizeof(gpt_header));
>> + return gpt_pte_cache.gpt_pte;
>> + }
>> +
>> + clear_gpt_pte_cache();
>> + }
>> +
>> + return NULL;
>> +}
>>
>> static char *print_efiname(gpt_entry *pte)
>> {
>> @@ -205,7 +251,7 @@ int get_disk_guid(struct blk_desc *desc, char *guid)
>> unsigned char *guid_bin;
>>
>> /* This function validates AND fills in the GPT header and PTE */
>> - if (find_valid_gpt(desc, gpt_head, &gpt_pte) != 1)
>> + if (find_valid_gpt(desc, gpt_head, &gpt_pte, false) != 1)
>> return -EINVAL;
>>
>> guid_bin = gpt_head->disk_guid.b;
>> @@ -224,7 +270,7 @@ static void __maybe_unused part_print_efi(struct blk_desc *desc)
>> unsigned char *uuid;
>>
>> /* This function validates AND fills in the GPT header and PTE */
>> - if (find_valid_gpt(desc, gpt_head, &gpt_pte) != 1)
>> + if (find_valid_gpt(desc, gpt_head, &gpt_pte, false) != 1)
>> return;
>>
>> debug("%s: gpt-entry at %p\n", __func__, gpt_pte);
>> @@ -258,8 +304,13 @@ static void __maybe_unused part_print_efi(struct blk_desc *desc)
>> return;
>> }
>>
>> -static int __maybe_unused part_get_info_efi(struct blk_desc *desc, int part,
>> - struct disk_partition *info)
>> +static void __maybe_unused part_get_info_cache_free_efi(struct blk_desc *desc)
>> +{
>> + clear_gpt_pte_cache();
>> +}
>> +
>> +static int _part_get_info_efi(struct blk_desc *desc, int part,
>> + struct disk_partition *info, bool cache)
>> {
>> ALLOC_CACHE_ALIGN_BUFFER_PAD(gpt_header, gpt_head, 1, desc->blksz);
>> gpt_entry *gpt_pte = NULL;
>> @@ -271,13 +322,14 @@ static int __maybe_unused part_get_info_efi(struct blk_desc *desc, int part,
>> }
>>
>> /* This function validates AND fills in the GPT header and PTE */
>> - if (find_valid_gpt(desc, gpt_head, &gpt_pte) != 1)
>> + if (find_valid_gpt(desc, gpt_head, &gpt_pte, cache) != 1)
>> return -EINVAL;
>>
>> if (part > le32_to_cpu(gpt_head->num_partition_entries) ||
>> !is_pte_valid(&gpt_pte[part - 1])) {
>> log_debug("Invalid partition number %d\n", part);
>> - free(gpt_pte);
>> + if (!cache)
>> + free(gpt_pte);
>> return -EPERM;
>> }
>>
>> @@ -307,11 +359,23 @@ static int __maybe_unused part_get_info_efi(struct blk_desc *desc, int part,
>> log_debug("start 0x" LBAF ", size 0x" LBAF ", name %s\n", info->start,
>> info->size, info->name);
>>
>> - /* Remember to free pte */
>> - free(gpt_pte);
>> + if (!cache)
>> + free(gpt_pte);
>> return 0;
>> }
>>
>> +static int __maybe_unused part_get_info_cached_efi(struct blk_desc *desc, int part,
>> + struct disk_partition *info)
>> +{
>> + return _part_get_info_efi(desc, part, info, true);
>> +}
>> +
>> +static int __maybe_unused part_get_info_efi(struct blk_desc *desc, int part,
>> + struct disk_partition *info)
>> +{
>> + return _part_get_info_efi(desc, part, info, false);
>> +}
>> +
>> static int part_test_efi(struct blk_desc *desc)
>> {
>> ALLOC_CACHE_ALIGN_BUFFER_PAD(legacy_mbr, legacymbr, 1, desc->blksz);
>> @@ -689,7 +753,7 @@ int gpt_verify_headers(struct blk_desc *desc, gpt_header *gpt_head,
>> */
>> if (is_gpt_valid(desc,
>> GPT_PRIMARY_PARTITION_TABLE_LBA,
>> - gpt_head, gpt_pte) != 1) {
>> + gpt_head, gpt_pte, false) != 1) {
>> log_debug("Invalid GPT\n");
>> return -1;
>> }
>> @@ -706,7 +770,7 @@ int gpt_verify_headers(struct blk_desc *desc, gpt_header *gpt_head,
>> }
>>
>> if (is_gpt_valid(desc, (desc->lba - 1),
>> - gpt_head, gpt_pte) != 1) {
>> + gpt_head, gpt_pte, false) != 1) {
>> log_debug("Invalid Backup GPT\n");
>> return -1;
>> }
>> @@ -765,9 +829,9 @@ int gpt_repair_headers(struct blk_desc *desc)
>> int ret = -1;
>>
>> is_gpt1_valid = is_gpt_valid(desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
>> - gpt_h1, &gpt_e1);
>> + gpt_h1, &gpt_e1, false);
>> is_gpt2_valid = is_gpt_valid(desc, desc->lba - 1,
>> - gpt_h2, &gpt_e2);
>> + gpt_h2, &gpt_e2, false);
>>
>> if (is_gpt1_valid && is_gpt2_valid) {
>> ret = 0;
>> @@ -1023,12 +1087,13 @@ static int is_pmbr_valid(legacy_mbr *mbr)
>> * lba is the logical block address of the GPT header to test
>> * gpt is a GPT header ptr, filled on return.
>> * ptes is a PTEs ptr, filled on return.
>> + * cache is a bool, true to use the cached gpt_pte from previous call
>
> "use the cache" or "use cached GPT if available"?
>
> It remains unclear what will happen if cache=1 and no entry is found.
Ack, will clarify
>
> Best regards
>
> Heinrich
>
>> *
>> * Description: returns 1 if valid, 0 on error, 2 if ignored header
>> * If valid, returns pointers to PTEs.
>> */
>> static int is_gpt_valid(struct blk_desc *desc, u64 lba, gpt_header *pgpt_head,
>> - gpt_entry **pgpt_pte)
>> + gpt_entry **pgpt_pte, bool cache)
>> {
>> /* Confirm valid arguments prior to allocation. */
>> if (!desc || !pgpt_head) {
>> @@ -1036,6 +1101,12 @@ static int is_gpt_valid(struct blk_desc *desc, u64 lba, gpt_header *pgpt_head,
>> return 0;
>> }
>>
>> + if (cache) {
>> + *pgpt_pte = get_cached_gpt_pte(desc, lba, pgpt_head);
>> + if (*pgpt_pte)
>> + return 1;
>> + }
>> +
>> ALLOC_CACHE_ALIGN_BUFFER_PAD(legacy_mbr, mbr, 1, desc->blksz);
>>
>> /* Read MBR Header from device */
>> @@ -1081,6 +1152,9 @@ static int is_gpt_valid(struct blk_desc *desc, u64 lba, gpt_header *pgpt_head,
>> return 0;
>> }
>>
>> + if (cache)
>> + cache_gpt_pte(desc, lba, *pgpt_pte, pgpt_head);
>> +
>> /* We're done, all's well */
>> return 1;
>> }
>> @@ -1090,23 +1164,25 @@ static int is_gpt_valid(struct blk_desc *desc, u64 lba, gpt_header *pgpt_head,
>> *
>> * gpt is a GPT header ptr, filled on return.
>> * ptes is a PTEs ptr, filled on return.
>> + * cache if a bool, use cached GPT if available
>> *
>> * Description: returns 1 if found a valid gpt, 0 on error.
>> * If valid, returns pointers to PTEs.
>> */
>> static int find_valid_gpt(struct blk_desc *desc, gpt_header *gpt_head,
>> - gpt_entry **pgpt_pte)
>> + gpt_entry **pgpt_pte, bool cache)
>> {
>> int r;
>>
>> r = is_gpt_valid(desc, GPT_PRIMARY_PARTITION_TABLE_LBA, gpt_head,
>> - pgpt_pte);
>> + pgpt_pte, cache);
>>
>> if (r != 1) {
>> if (r != 2)
>> log_debug("Invalid GPT\n");
>>
>> - if (is_gpt_valid(desc, desc->lba - 1, gpt_head, pgpt_pte)
>> + if (is_gpt_valid(desc, desc->lba - 1, gpt_head, pgpt_pte,
>> + true)
>> != 1) {
>> log_debug("Invalid Backup GPT\n");
>> return 0;
>> @@ -1210,6 +1286,8 @@ U_BOOT_PART_TYPE(a_efi) = {
>> .name = "EFI",
>> .part_type = PART_TYPE_EFI,
>> .max_entries = GPT_ENTRY_NUMBERS,
>> + .get_info_cache_free = part_get_info_ptr(part_get_info_cache_free_efi),
>> + .get_info_cached = part_get_info_ptr(part_get_info_cached_efi),
>> .get_info = part_get_info_ptr(part_get_info_efi),
>> .print = part_print_ptr(part_print_efi),
>> .test = part_test_efi,
>>
>
More information about the U-Boot
mailing list