[PATCH] disk: dos: Add all options for EFI System Partitions

[PATCH] disk: dos: Add all options for EFI System Partitions

[Peter Robinson]

The EFI spec states that the ESP can be any of FAT12/16/32 but for
compatibility doesn't necssarily require the partition to be the
EFI partition table ID of 0xef. A number of arm devices will not
find their firmware on a FAT partition with an ID of 0xef so also
allow the original FAT12/16/32 partition IDs as they are also
permissable for an ESP.

Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
---
 disk/part_dos.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/disk/part_dos.c b/disk/part_dos.c
index 567ead7511d..303eb1d13ee 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -40,6 +40,12 @@ static int get_bootable(dos_partition_t *p)
 {
 	int ret = 0;
 
+	if (p->sys_ind == 0x1)
+		ret |= PART_EFI_SYSTEM_PARTITION;
+	if (p->sys_ind == 0x6)
+		ret |= PART_EFI_SYSTEM_PARTITION;
+	if (p->sys_ind == 0x0b)
+		ret |= PART_EFI_SYSTEM_PARTITION;
 	if (p->sys_ind == 0xef)
 		ret |= PART_EFI_SYSTEM_PARTITION;
 	if (p->boot_ind == 0x80)
-- 
2.43.1

Re: [PATCH] disk: dos: Add all options for EFI System Partitions

[Mark Kettenis]

> From: Peter Robinson <pbrobinson@gmail.com>
> Date: Mon, 19 Feb 2024 09:12:15 +0000
> 
> The EFI spec states that the ESP can be any of FAT12/16/32 but for
> compatibility doesn't necssarily require the partition to be the
> EFI partition table ID of 0xef. A number of arm devices will not
> find their firmware on a FAT partition with an ID of 0xef so also
> allow the original FAT12/16/32 partition IDs as they are also
> permissable for an ESP.

Hi Peter,

Any reason not to include 0x0c as well?  That is what we use on
OpenBSD/armv7 and OpenBSD/arm64.  And as far as I know all UEFI
implementations (on arm64 at least) boot from such a partition.

(And yes, we use that partition type because we want to have a
bootable image that works on the various Raspberry Pi models).

That said, what problem does this fix?  And what happens if we have
both a 0xea and a 0x01/0x06/0x0b/0x0c partition?  In that case U-Boot
should probably prefer the 0xea over the others as the ESP.

Oh, and while your're at it, the hex constants are a bit inconsistent
(0x1/0x6 vs. 0x0b).

Cheers,

Mark

> Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
> ---
>  disk/part_dos.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/disk/part_dos.c b/disk/part_dos.c
> index 567ead7511d..303eb1d13ee 100644
> --- a/disk/part_dos.c
> +++ b/disk/part_dos.c
> @@ -40,6 +40,12 @@ static int get_bootable(dos_partition_t *p)
>  {
>  	int ret = 0;
>  
> +	if (p->sys_ind == 0x1)
> +		ret |= PART_EFI_SYSTEM_PARTITION;
> +	if (p->sys_ind == 0x6)
> +		ret |= PART_EFI_SYSTEM_PARTITION;
> +	if (p->sys_ind == 0x0b)
> +		ret |= PART_EFI_SYSTEM_PARTITION;
>  	if (p->sys_ind == 0xef)
>  		ret |= PART_EFI_SYSTEM_PARTITION;
>  	if (p->boot_ind == 0x80)
> -- 
> 2.43.1
> 
> 

Re: [PATCH] disk: dos: Add all options for EFI System Partitions

[Peter Robinson]

On Mon, 19 Feb 2024 at 10:24, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> > From: Peter Robinson <pbrobinson@gmail.com>
> > Date: Mon, 19 Feb 2024 09:12:15 +0000
> >
> > The EFI spec states that the ESP can be any of FAT12/16/32 but for
> > compatibility doesn't necssarily require the partition to be the
> > EFI partition table ID of 0xef. A number of arm devices will not
> > find their firmware on a FAT partition with an ID of 0xef so also
> > allow the original FAT12/16/32 partition IDs as they are also
> > permissable for an ESP.
>
> Hi Peter,
>
> Any reason not to include 0x0c as well?  That is what we use on
> OpenBSD/armv7 and OpenBSD/arm64.  And as far as I know all UEFI
> implementations (on arm64 at least) boot from such a partition.

I wasn't 100% the support with LBA so I erred with caution, but no
reason I can't add it.

> (And yes, we use that partition type because we want to have a
> bootable image that works on the various Raspberry Pi models).

Yes, that is the same reason for us, plus a few other random other Arm
devices that won't recognise EF as VFAT and won't boot.

> That said, what problem does this fix?  And what happens if we have
> both a 0xea and a 0x01/0x06/0x0b/0x0c partition?  In that case U-Boot
> should probably prefer the 0xea over the others as the ESP.

The reason is because the support to write EFI vars on ESP, and yes I
realise it's got security issues but for most boards it's the least of
their problem, as the support won't do that without the flag and you
get a bunch of these on boot:

No EFI system partition
Failed to persist EFI variables

As for multiple partitions UEFI should handle that and I believe the
EFI var support has logic around which partition it chooses.

> Oh, and while your're at it, the hex constants are a bit inconsistent
> (0x1/0x6 vs. 0x0b).

Will fix with v2 when I add 0x0c, I'll await other feedback for a bit.

Thanks,
Peter

> Cheers,
>
> Mark
>
> > Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
> > ---
> >  disk/part_dos.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/disk/part_dos.c b/disk/part_dos.c
> > index 567ead7511d..303eb1d13ee 100644
> > --- a/disk/part_dos.c
> > +++ b/disk/part_dos.c
> > @@ -40,6 +40,12 @@ static int get_bootable(dos_partition_t *p)
> >  {
> >       int ret = 0;
> >
> > +     if (p->sys_ind == 0x1)
> > +             ret |= PART_EFI_SYSTEM_PARTITION;
> > +     if (p->sys_ind == 0x6)
> > +             ret |= PART_EFI_SYSTEM_PARTITION;
> > +     if (p->sys_ind == 0x0b)
> > +             ret |= PART_EFI_SYSTEM_PARTITION;
> >       if (p->sys_ind == 0xef)
> >               ret |= PART_EFI_SYSTEM_PARTITION;
> >       if (p->boot_ind == 0x80)
> > --
> > 2.43.1
> >
> >

Re: [PATCH] disk: dos: Add all options for EFI System Partitions

[Ilias Apalodimas]

Hi all,

On Mon, 19 Feb 2024 at 12:53, Peter Robinson <pbrobinson@gmail.com> wrote:
>
> On Mon, 19 Feb 2024 at 10:24, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >
> > > From: Peter Robinson <pbrobinson@gmail.com>
> > > Date: Mon, 19 Feb 2024 09:12:15 +0000
> > >
> > > The EFI spec states that the ESP can be any of FAT12/16/32 but for
> > > compatibility doesn't necssarily require the partition to be the
> > > EFI partition table ID of 0xef. A number of arm devices will not
> > > find their firmware on a FAT partition with an ID of 0xef so also
> > > allow the original FAT12/16/32 partition IDs as they are also
> > > permissable for an ESP.
> >
> > Hi Peter,
> >
> > Any reason not to include 0x0c as well?  That is what we use on
> > OpenBSD/armv7 and OpenBSD/arm64.  And as far as I know all UEFI
> > implementations (on arm64 at least) boot from such a partition.
>
> I wasn't 100% the support with LBA so I erred with caution, but no
> reason I can't add it.
>
> > (And yes, we use that partition type because we want to have a
> > bootable image that works on the various Raspberry Pi models).
>
> Yes, that is the same reason for us, plus a few other random other Arm
> devices that won't recognise EF as VFAT and won't boot.
>
> > That said, what problem does this fix?  And what happens if we have
> > both a 0xea and a 0x01/0x06/0x0b/0x0c partition?  In that case U-Boot
> > should probably prefer the 0xea over the others as the ESP.
>
> The reason is because the support to write EFI vars on ESP, and yes I
> realise it's got security issues but for most boards it's the least of
> their problem, as the support won't do that without the flag and you
> get a bunch of these on boot:
>
> No EFI system partition
> Failed to persist EFI variables
>
> As for multiple partitions UEFI should handle that and I believe the
> EFI var support has logic around which partition it chooses.

For now we 'just' select the first ESP partition we scan while adding
disks on the EFI subsystem. But this patch doesn't change that logic.

That being said, I am pretty sure Mark is right here and this is going
to be a problem in the future (but it's still orthogonal to this
patch).

>
> > Oh, and while your're at it, the hex constants are a bit inconsistent
> > (0x1/0x6 vs. 0x0b).
>
> Will fix with v2 when I add 0x0c, I'll await other feedback for a bit.
>
> Thanks,
> Peter
>
> > Cheers,
> >
> > Mark
> >
> > > Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
> > > ---
> > >  disk/part_dos.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/disk/part_dos.c b/disk/part_dos.c
> > > index 567ead7511d..303eb1d13ee 100644
> > > --- a/disk/part_dos.c
> > > +++ b/disk/part_dos.c
> > > @@ -40,6 +40,12 @@ static int get_bootable(dos_partition_t *p)
> > >  {
> > >       int ret = 0;
> > >
> > > +     if (p->sys_ind == 0x1)
> > > +             ret |= PART_EFI_SYSTEM_PARTITION;
> > > +     if (p->sys_ind == 0x6)
> > > +             ret |= PART_EFI_SYSTEM_PARTITION;
> > > +     if (p->sys_ind == 0x0b)
> > > +             ret |= PART_EFI_SYSTEM_PARTITION;
> > >       if (p->sys_ind == 0xef)
> > >               ret |= PART_EFI_SYSTEM_PARTITION;


Can switch the many ifs to a switch statement? Going to make adding
more IDs easier.

Thanks
/Ilias
> > >       if (p->boot_ind == 0x80)
> > > --
> > > 2.43.1
> > >
> > >