diff mbox series

[v2,1/2] part: detect EFI system partition

Message ID 20200405092818.6417-2-xypron.glpk@gmx.de
State Superseded
Headers show
Series efi_loader: identify EFI system partition | expand

Commit Message

Heinrich Schuchardt April 5, 2020, 9:28 a.m. UTC
Up to now for MBR and GPT partitions the info field 'bootable' was set to 1
if either the partition was an EFI system partition or the bootable flag
was set.

Turn info field 'bootable' into a bit mask with separate bits for bootable
and EFI system partition.

This will allow us to identify the EFI system partition in the UEFI
sub-system.

Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
---
v2:
	used BIT() macro to define bit mask
---
 disk/part_dos.c | 10 ++++++++--
 disk/part_efi.c | 12 ++++++++----
 include/part.h  | 11 ++++++++++-
 3 files changed, 26 insertions(+), 7 deletions(-)

--
2.25.1

Comments

AKASHI Takahiro April 14, 2020, 5:31 a.m. UTC | #1
Heinrich,

On Sun, Apr 05, 2020 at 11:28:17AM +0200, Heinrich Schuchardt wrote:
> Up to now for MBR and GPT partitions the info field 'bootable' was set to 1
> if either the partition was an EFI system partition or the bootable flag
> was set.
> 
> Turn info field 'bootable' into a bit mask with separate bits for bootable
> and EFI system partition.
> 
> This will allow us to identify the EFI system partition in the UEFI
> sub-system.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
> v2:
> 	used BIT() macro to define bit mask
> ---
>  disk/part_dos.c | 10 ++++++++--
>  disk/part_efi.c | 12 ++++++++----
>  include/part.h  | 11 ++++++++++-
>  3 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/disk/part_dos.c b/disk/part_dos.c
> index 83ff40d310..0ec7f1628e 100644
> --- a/disk/part_dos.c
> +++ b/disk/part_dos.c
> @@ -45,9 +45,15 @@ static inline int is_extended(int part_type)
>  	    part_type == 0x85);
>  }
> 
> -static inline int is_bootable(dos_partition_t *p)
> +static int is_bootable(dos_partition_t *p)
>  {
> -	return (p->sys_ind == 0xef) || (p->boot_ind == 0x80);
> +	int ret = 0;
> +
> +	if (p->sys_ind == 0xef)
> +		ret |= PART_EFI_SYSTEM_PARTITION;
> +	if (p->boot_ind == 0x80)
> +		ret |= PART_BOOTABLE;
> +	return ret;
>  }

The return value is no longer boolean, so the function's name
should be changed to avoid confusion. Say, get_bootable_flags()?
Then  another function, or inline function should be provided.

bool part_is_bootable(blk_desc *bdev)

for checking if the device is "bootable."

>  static void print_one_part(dos_partition_t *p, lbaint_t ext_part_sector,
> diff --git a/disk/part_efi.c b/disk/part_efi.c
> index b2e157d9c1..19f1f43f4e 100644
> --- a/disk/part_efi.c
> +++ b/disk/part_efi.c
> @@ -71,11 +71,15 @@ static char *print_efiname(gpt_entry *pte)
> 
>  static const efi_guid_t system_guid = PARTITION_SYSTEM_GUID;
> 
> -static inline int is_bootable(gpt_entry *p)
> +static int is_bootable(gpt_entry *p)
>  {
> -	return p->attributes.fields.legacy_bios_bootable ||
> -		!memcmp(&(p->partition_type_guid), &system_guid,
> -			sizeof(efi_guid_t));
> +	int ret = 0;
> +
> +	if (!memcmp(&p->partition_type_guid, &system_guid, sizeof(efi_guid_t)))
> +		ret |=  PART_EFI_SYSTEM_PARTITION;
> +	if (p->attributes.fields.legacy_bios_bootable)
> +		ret |=  PART_BOOTABLE;
> +	return ret;
>  }

ditto, and again

bool part_is_efi_system()

-Takahiro Akashi

>  static int validate_gpt_header(gpt_header *gpt_h, lbaint_t lba,
> diff --git a/include/part.h b/include/part.h
> index 0b5cf3d5e8..f3442ef85d 100644
> --- a/include/part.h
> +++ b/include/part.h
> @@ -51,13 +51,22 @@ struct block_drvr {
>  #define PART_TYPE_LEN 32
>  #define MAX_SEARCH_PARTITIONS 64
> 
> +#define PART_BOOTABLE			BIT(0)
> +#define PART_EFI_SYSTEM_PARTITION	BIT(1)
> +
>  typedef struct disk_partition {
>  	lbaint_t	start;	/* # of first block in partition	*/
>  	lbaint_t	size;	/* number of blocks in partition	*/
>  	ulong	blksz;		/* block size in bytes			*/
>  	uchar	name[PART_NAME_LEN];	/* partition name			*/
>  	uchar	type[PART_TYPE_LEN];	/* string type description		*/
> -	int	bootable;	/* Active/Bootable flag is set		*/
> +	/*
> +	 * The bootable is a bitmask with the following fields:
> +	 *
> +	 * PART_BOOTABLE		the MBR bootable flag is set
> +	 * PART_EFI_SYSTEM_PARTITION	the partition is an EFI system partition
> +	 */
> +	int	bootable;
>  #if CONFIG_IS_ENABLED(PARTITION_UUIDS)
>  	char	uuid[UUID_STR_LEN + 1];	/* filesystem UUID as string, if exists	*/
>  #endif
> --
> 2.25.1
>
Heinrich Schuchardt April 14, 2020, 6:01 a.m. UTC | #2
Am April 14, 2020 5:31:25 AM UTC schrieb AKASHI Takahiro <takahiro.akashi at linaro.org>:
>Heinrich,
>
>On Sun, Apr 05, 2020 at 11:28:17AM +0200, Heinrich Schuchardt wrote:
>> Up to now for MBR and GPT partitions the info field 'bootable' was
>set to 1
>> if either the partition was an EFI system partition or the bootable
>flag
>> was set.
>> 
>> Turn info field 'bootable' into a bit mask with separate bits for
>bootable
>> and EFI system partition.
>> 
>> This will allow us to identify the EFI system partition in the UEFI
>> sub-system.
>> 
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>> v2:
>> 	used BIT() macro to define bit mask
>> ---
>>  disk/part_dos.c | 10 ++++++++--
>>  disk/part_efi.c | 12 ++++++++----
>>  include/part.h  | 11 ++++++++++-
>>  3 files changed, 26 insertions(+), 7 deletions(-)
>> 
>> diff --git a/disk/part_dos.c b/disk/part_dos.c
>> index 83ff40d310..0ec7f1628e 100644
>> --- a/disk/part_dos.c
>> +++ b/disk/part_dos.c
>> @@ -45,9 +45,15 @@ static inline int is_extended(int part_type)
>>  	    part_type == 0x85);
>>  }
>> 
>> -static inline int is_bootable(dos_partition_t *p)
>> +static int is_bootable(dos_partition_t *p)
>>  {
>> -	return (p->sys_ind == 0xef) || (p->boot_ind == 0x80);
>> +	int ret = 0;
>> +
>> +	if (p->sys_ind == 0xef)
>> +		ret |= PART_EFI_SYSTEM_PARTITION;
>> +	if (p->boot_ind == 0x80)
>> +		ret |= PART_BOOTABLE;
>> +	return ret;
>>  }
>
>The return value is no longer boolean, so the function's name
>should be changed to avoid confusion. Say, get_bootable_flags()?
>Then  another function, or inline function should be provided.
>
>bool part_is_bootable(blk_desc *bdev)
>
>for checking if the device is "bootable."
>

Logical operators 'if', '!', '&&', '||' regard any non-zero value as true. So I see no benefit of such a second function.

Best regards

Heinrich

>>  static void print_one_part(dos_partition_t *p, lbaint_t
>ext_part_sector,
>> diff --git a/disk/part_efi.c b/disk/part_efi.c
>> index b2e157d9c1..19f1f43f4e 100644
>> --- a/disk/part_efi.c
>> +++ b/disk/part_efi.c
>> @@ -71,11 +71,15 @@ static char *print_efiname(gpt_entry *pte)
>> 
>>  static const efi_guid_t system_guid = PARTITION_SYSTEM_GUID;
>> 
>> -static inline int is_bootable(gpt_entry *p)
>> +static int is_bootable(gpt_entry *p)
>>  {
>> -	return p->attributes.fields.legacy_bios_bootable ||
>> -		!memcmp(&(p->partition_type_guid), &system_guid,
>> -			sizeof(efi_guid_t));
>> +	int ret = 0;
>> +
>> +	if (!memcmp(&p->partition_type_guid, &system_guid,
>sizeof(efi_guid_t)))
>> +		ret |=  PART_EFI_SYSTEM_PARTITION;
>> +	if (p->attributes.fields.legacy_bios_bootable)
>> +		ret |=  PART_BOOTABLE;
>> +	return ret;
>>  }
>
>ditto, and again
>
>bool part_is_efi_system()
>
>-Takahiro Akashi
>
>>  static int validate_gpt_header(gpt_header *gpt_h, lbaint_t lba,
>> diff --git a/include/part.h b/include/part.h
>> index 0b5cf3d5e8..f3442ef85d 100644
>> --- a/include/part.h
>> +++ b/include/part.h
>> @@ -51,13 +51,22 @@ struct block_drvr {
>>  #define PART_TYPE_LEN 32
>>  #define MAX_SEARCH_PARTITIONS 64
>> 
>> +#define PART_BOOTABLE			BIT(0)
>> +#define PART_EFI_SYSTEM_PARTITION	BIT(1)
>> +
>>  typedef struct disk_partition {
>>  	lbaint_t	start;	/* # of first block in partition	*/
>>  	lbaint_t	size;	/* number of blocks in partition	*/
>>  	ulong	blksz;		/* block size in bytes			*/
>>  	uchar	name[PART_NAME_LEN];	/* partition name			*/
>>  	uchar	type[PART_TYPE_LEN];	/* string type description		*/
>> -	int	bootable;	/* Active/Bootable flag is set		*/
>> +	/*
>> +	 * The bootable is a bitmask with the following fields:
>> +	 *
>> +	 * PART_BOOTABLE		the MBR bootable flag is set
>> +	 * PART_EFI_SYSTEM_PARTITION	the partition is an EFI system
>partition
>> +	 */
>> +	int	bootable;
>>  #if CONFIG_IS_ENABLED(PARTITION_UUIDS)
>>  	char	uuid[UUID_STR_LEN + 1];	/* filesystem UUID as string, if
>exists	*/
>>  #endif
>> --
>> 2.25.1
>>
diff mbox series

Patch

diff --git a/disk/part_dos.c b/disk/part_dos.c
index 83ff40d310..0ec7f1628e 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -45,9 +45,15 @@  static inline int is_extended(int part_type)
 	    part_type == 0x85);
 }

-static inline int is_bootable(dos_partition_t *p)
+static int is_bootable(dos_partition_t *p)
 {
-	return (p->sys_ind == 0xef) || (p->boot_ind == 0x80);
+	int ret = 0;
+
+	if (p->sys_ind == 0xef)
+		ret |= PART_EFI_SYSTEM_PARTITION;
+	if (p->boot_ind == 0x80)
+		ret |= PART_BOOTABLE;
+	return ret;
 }

 static void print_one_part(dos_partition_t *p, lbaint_t ext_part_sector,
diff --git a/disk/part_efi.c b/disk/part_efi.c
index b2e157d9c1..19f1f43f4e 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -71,11 +71,15 @@  static char *print_efiname(gpt_entry *pte)

 static const efi_guid_t system_guid = PARTITION_SYSTEM_GUID;

-static inline int is_bootable(gpt_entry *p)
+static int is_bootable(gpt_entry *p)
 {
-	return p->attributes.fields.legacy_bios_bootable ||
-		!memcmp(&(p->partition_type_guid), &system_guid,
-			sizeof(efi_guid_t));
+	int ret = 0;
+
+	if (!memcmp(&p->partition_type_guid, &system_guid, sizeof(efi_guid_t)))
+		ret |=  PART_EFI_SYSTEM_PARTITION;
+	if (p->attributes.fields.legacy_bios_bootable)
+		ret |=  PART_BOOTABLE;
+	return ret;
 }

 static int validate_gpt_header(gpt_header *gpt_h, lbaint_t lba,
diff --git a/include/part.h b/include/part.h
index 0b5cf3d5e8..f3442ef85d 100644
--- a/include/part.h
+++ b/include/part.h
@@ -51,13 +51,22 @@  struct block_drvr {
 #define PART_TYPE_LEN 32
 #define MAX_SEARCH_PARTITIONS 64

+#define PART_BOOTABLE			BIT(0)
+#define PART_EFI_SYSTEM_PARTITION	BIT(1)
+
 typedef struct disk_partition {
 	lbaint_t	start;	/* # of first block in partition	*/
 	lbaint_t	size;	/* number of blocks in partition	*/
 	ulong	blksz;		/* block size in bytes			*/
 	uchar	name[PART_NAME_LEN];	/* partition name			*/
 	uchar	type[PART_TYPE_LEN];	/* string type description		*/
-	int	bootable;	/* Active/Bootable flag is set		*/
+	/*
+	 * The bootable is a bitmask with the following fields:
+	 *
+	 * PART_BOOTABLE		the MBR bootable flag is set
+	 * PART_EFI_SYSTEM_PARTITION	the partition is an EFI system partition
+	 */
+	int	bootable;
 #if CONFIG_IS_ENABLED(PARTITION_UUIDS)
 	char	uuid[UUID_STR_LEN + 1];	/* filesystem UUID as string, if exists	*/
 #endif