On 7/13/24 23:16, Roman Stratiienko wrote:
Hello Heinrich,

Thank you for your comments.

Just a few thoughts about this patch.

1. I was also sending this patch as a noninvasive, semi-cosmetic
change. Relying on hardcoded values is usually not a good practice.
2. Shifting of primary GPT works in u-boot and Linux kernel with minor
tweaks. See patches [1] and [2] for more context. I'm not going to
upstream them.
3. Accepting this patch upstream helps me to reduce the dependency
chain and potential conflicts during the rebase.

Hello Roman,

could you, please, answer my questions below.

Best regards

Heinrich


BR, Roman.

[1]: 
https://github.com/GloDroidCommunity/amlogic-series/blob/8cc5ee7df1358092f77f43aa0c9661883473e5af/patches-aosp/glodroid/bootloader/u-boot/0005-GLODROID-Shift-GPT-position-to-support-Amlogic-platf.patch
[2]: 
https://github.com/GloDroidCommunity/amlogic-series/blob/8cc5ee7df1358092f77f43aa0c9661883473e5af/patches-aosp/glodroid/kernel/common-android14-6.1-lts/0001-GLODROID-Allow-placing-primary-GPT-to-2MiB-offset-to.patch

пн, 10 черв. 2024 р. о 15:22 Heinrich Schuchardt <xypron.g...@gmx.de> пише:

On 19.05.24 17:35, Roman Stratiienko wrote:
Use GPT_PRIMARY_PARTITION_TABLE_LBA as the only source for a primary GPT
location instead of hardcoding it every time.

Sometimes, we need to shift the primary GPT location.
Having the location in the single place simplifies such shifting.

In gdisk there is an expert command "j - move the main partition table".
This corresponds to CONFIG_EFI_PARTITION_ENTRIES_OFF. It does not move
the GPT primary header but only the partition entries. So sector 1 is
still occupied.

If I read you correctly, CONFIG_EFI_PARTITION_ENTRIES_OFF is not doing
what you need. Instead of only moving the partition entries of the
primary partition table, you want to move the complete primary GPT table.

Somehow the operating system must find the primary GPT table.

When writing the partition table shouldn't the offset be reflected in
the protective MBR fields StartingCHS and StartingLBA? See Table 5.4
"Protective MBR Partition Record protecting the entire disk*" of the
UEFI 2.10 specification.

Does Linux support this? For testing remove the backup partition table!

GPT_PRIMARY_PARTITION_TABLE_LBA should be made configurable in this case.

Best regards

Heinrich


Signed-off-by: Roman Stratiienko <r.stratiie...@gmail.com>
---
   disk/part_efi.c | 10 +++++-----
   1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/disk/part_efi.c b/disk/part_efi.c
index 4ce9243ef2..b55e251ea1 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -387,7 +387,7 @@ int write_gpt_table(struct blk_desc *desc, gpt_header 
*gpt_h, gpt_entry *gpt_e)
       gpt_h->header_crc32 = cpu_to_le32(calc_crc32);

       /* Write the First GPT to the block right after the Legacy MBR */
-     if (blk_dwrite(desc, 1, 1, gpt_h) != 1)
+     if (blk_dwrite(desc, (lbaint_t)le64_to_cpu(gpt_h->my_lba), 1, gpt_h) != 1)
               goto err;

       if (blk_dwrite(desc, le64_to_cpu(gpt_h->partition_entry_lba),
@@ -534,7 +534,7 @@ int gpt_fill_pte(struct blk_desc *desc,

   static uint32_t partition_entries_offset(struct blk_desc *desc)
   {
-     uint32_t offset_blks = 2;
+     uint32_t offset_blks = GPT_PRIMARY_PARTITION_TABLE_LBA + 1;
       uint32_t __maybe_unused offset_bytes;
       int __maybe_unused config_offset;

@@ -572,8 +572,8 @@ static uint32_t partition_entries_offset(struct blk_desc 
*desc)
        * The earliest LBA this can be at is LBA#2 (i.e. right behind
        * the (protective) MBR and the GPT header.
        */
-     if (offset_blks < 2)
-             offset_blks = 2;
+     if (offset_blks < (GPT_PRIMARY_PARTITION_TABLE_LBA + 1))
+             offset_blks = GPT_PRIMARY_PARTITION_TABLE_LBA + 1;

       return offset_blks;
   }
@@ -584,7 +584,7 @@ int gpt_fill_header(struct blk_desc *desc, gpt_header 
*gpt_h, char *str_guid,
       gpt_h->signature = cpu_to_le64(GPT_HEADER_SIGNATURE_UBOOT);
       gpt_h->revision = cpu_to_le32(GPT_HEADER_REVISION_V1);
       gpt_h->header_size = cpu_to_le32(sizeof(gpt_header));
-     gpt_h->my_lba = cpu_to_le64(1);
+     gpt_h->my_lba = cpu_to_le64(GPT_PRIMARY_PARTITION_TABLE_LBA);
       gpt_h->alternate_lba = cpu_to_le64(desc->lba - 1);
       gpt_h->last_usable_lba = cpu_to_le64(desc->lba - 34);
       gpt_h->partition_entry_lba =


Reply via email to