Hi Sumit,
On 2026-09-28T06:40:33, Sumit Singh Dhami <[email protected]> wrote:
> disk: part_efi: support writing full 64-bit GPT partition attributes
>
> Currently gpt_fill_pte() always zeroes the GPT partition entry's
> 64-bit attributes field, and only ever sets the legacy BIOS bootable
> bit.
>
> Add a new CONFIG_PARTITION_ATTR option that, when enabled, adds new
> fields to struct disk_partition along with their accessors to support
> writing full 64-bit attributes.
>
> Signed-off-by: Sumit Singh Dhami <[email protected]>
>
> disk/Kconfig | 8 ++++++++
> disk/part_efi.c | 9 +++++++--
> include/part.h | 30 ++++++++++++++++++++++++++++++
> 3 files changed, 45 insertions(+), 2 deletions(-)
> diff --git a/include/part.h b/include/part.h
> @@ -76,6 +76,10 @@ struct disk_partition {
> +#if CONFIG_IS_ENABLED(PARTITION_ATTR)
> + u64 gpt_attr; /* raw 64-bit GPT partition attributes, if
> attrs= was given */
> + bool gpt_attr_valid; /* true if attrs= was present in the partitions
> list */
> +#endif
I don't think gpt_attr_valid is needed. Every caller of gpt_restore()
zero-initialises the partition array (calloc() in set_gpt_info(),
designated initialisers in the tests), so when attrs is not given
gpt_attr is 0 and the memset() in gpt_fill_pte() gives the same result
as assigning it. Patch 5 also uses disk_partition_gpt_attr() without
checking the flag. Please can you drop the bool and the _valid
accessor, so the write path becomes a single assignment?
Also the comments refer to attrs= which is 'gpt write' syntax; this
struct is used by all the partition drivers, so please describe the
field in terms of the GPT entry.
> diff --git a/disk/part_efi.c b/disk/part_efi.c
> @@ -531,8 +531,13 @@ int gpt_fill_pte(struct blk_desc *desc,
> + if (CONFIG_IS_ENABLED(PARTITION_ATTR) &&
> + disk_partition_gpt_attr_valid(&partitions[i]))
> + gpt_e[i].attributes.raw =
> + disk_partition_gpt_attr(&partitions[i]);
The CONFIG_IS_ENABLED() check is redundant since the accessor already
returns false when the option is disabled. Also raw is a little-endian
on-disk value, so this should use cpu_to_le64() (and le64_to_cpu() on
the read side in patch 3), even though the existing bitfield code
doesn't get this right either.
Regards,
Simon
