[Buildroot] Queries/suggestions regarding patch series "grub2: add support for arm and aarch64"

[Buildroot] Queries/suggestions regarding patch series "grub2: add support for arm and aarch64"

[Sumit Garg]

Hello Erico,

Firstly, apologies for not posting these comments on your patch-set
[1] as I joined Buildroot ML yesterday only.

I was recently exploring to test EFI boot using Buildroot on qemu
aarch64 target. But currently GRUB support is missing for aarch64
target in Buildroot. So I came across your patch-set. I have tested
your patch-set on qemu/u-boot aarch64 target using efi. So I would say
its:

Tested-by: Sumit Garg <sumit.garg@linaro.org>

However, I will suggest you to use PARTUUID for root path in grub.cfg
as it will make it more generic and device independent as we could use
either virtio, sata, usb or mmc. So I would propose a similar change
as in [2].

Apart from that, how do you think about adding qemu/u-boot target for
aarch64 to Buildroot? As u-boot already have efi booting support using
bootefi.

[1] https://patchwork.ozlabs.org/project/buildroot/list/?submitter=46481

[2] Change:

diff --git a/board/aarch64-efi/genimage-efi.cfg
b/board/aarch64-efi/genimage-efi.cfg
index f93ab9d..4a38bfe 100644
--- a/board/aarch64-efi/genimage-efi.cfg
+++ b/board/aarch64-efi/genimage-efi.cfg
@@ -16,6 +16,7 @@ image efi-part.vfat {
 image disk.img {

   hdimage {
+    disk-signature = 0xDEEDBEEF
   }

   partition boot {
diff --git a/board/aarch64-efi/grub.cfg b/board/aarch64-efi/grub.cfg
index ab88da9..404670b 100644
--- a/board/aarch64-efi/grub.cfg
+++ b/board/aarch64-efi/grub.cfg
@@ -2,5 +2,5 @@ set default="0"
 set timeout="5"

 menuentry "Buildroot" {
-       linux /Image root=/dev/vda2 rootwait console=ttyAMA0
+       linux /Image root=PARTUUID=deedbeef-02 rootwait console=ttyAMA0
 }

Regards,
Sumit

[Buildroot] Queries/suggestions regarding patch series "grub2: add support for arm and aarch64"

[Sumit Garg]

Gentle reminder.

On Tue, 23 Oct 2018 at 11:28, Sumit Garg <sumit.garg@linaro.org> wrote:
>
> Hello Erico,
>
> Firstly, apologies for not posting these comments on your patch-set
> [1] as I joined Buildroot ML yesterday only.
>
> I was recently exploring to test EFI boot using Buildroot on qemu
> aarch64 target. But currently GRUB support is missing for aarch64
> target in Buildroot. So I came across your patch-set. I have tested
> your patch-set on qemu/u-boot aarch64 target using efi. So I would say
> its:
>
> Tested-by: Sumit Garg <sumit.garg@linaro.org>
>
> However, I will suggest you to use PARTUUID for root path in grub.cfg
> as it will make it more generic and device independent as we could use
> either virtio, sata, usb or mmc. So I would propose a similar change
> as in [2].
>
> Apart from that, how do you think about adding qemu/u-boot target for
> aarch64 to Buildroot? As u-boot already have efi booting support using
> bootefi.
>
> [1] https://patchwork.ozlabs.org/project/buildroot/list/?submitter=46481
>
> [2] Change:
>
> diff --git a/board/aarch64-efi/genimage-efi.cfg
> b/board/aarch64-efi/genimage-efi.cfg
> index f93ab9d..4a38bfe 100644
> --- a/board/aarch64-efi/genimage-efi.cfg
> +++ b/board/aarch64-efi/genimage-efi.cfg
> @@ -16,6 +16,7 @@ image efi-part.vfat {
>  image disk.img {
>
>    hdimage {
> +    disk-signature = 0xDEEDBEEF
>    }
>
>    partition boot {
> diff --git a/board/aarch64-efi/grub.cfg b/board/aarch64-efi/grub.cfg
> index ab88da9..404670b 100644
> --- a/board/aarch64-efi/grub.cfg
> +++ b/board/aarch64-efi/grub.cfg
> @@ -2,5 +2,5 @@ set default="0"
>  set timeout="5"
>
>  menuentry "Buildroot" {
> -       linux /Image root=/dev/vda2 rootwait console=ttyAMA0
> +       linux /Image root=PARTUUID=deedbeef-02 rootwait console=ttyAMA0
>  }
>
> Regards,
> Sumit

[Buildroot] Queries/suggestions regarding patch series "grub2: add support for arm and aarch64"

[Erico Nunes]

Hi Sumit,

On Tue, Oct 23, 2018 at 7:58 AM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> Hello Erico,
>
> Firstly, apologies for not posting these comments on your patch-set
> [1] as I joined Buildroot ML yesterday only.
>
> I was recently exploring to test EFI boot using Buildroot on qemu
> aarch64 target. But currently GRUB support is missing for aarch64
> target in Buildroot. So I came across your patch-set. I have tested
> your patch-set on qemu/u-boot aarch64 target using efi. So I would say
> its:
>
> Tested-by: Sumit Garg <sumit.garg@linaro.org>

Thanks for your testing and feedback.

Yes it is taking a while to have this patchset reviewed, but right now
there are well over 200 patches in the patchwork queue, so I think it
is understandable that it is taking some time. At this point I believe
it will only be considered after 2018.11 (for 2019.02) anyway.

> However, I will suggest you to use PARTUUID for root path in grub.cfg
> as it will make it more generic and device independent as we could use
> either virtio, sata, usb or mmc. So I would propose a similar change
> as in [2].
>
> Apart from that, how do you think about adding qemu/u-boot target for
> aarch64 to Buildroot? As u-boot already have efi booting support using
> bootefi.

Using PARTUUID seems to be an interesting change. I'd like to hear the
opinion of the other Buildroot developers before making this change in
this patchset though, since most other platforms in Buildroot are not
doing this.
If there is interest in it, maybe we can do this change for other
platforms too and in a separate patchset?
For example, then it might be also interesting to change it in the
pc_* platforms, as it may solve /dev/sda and /dev/vda issue for
testing them with qemu.

Cheers

Erico

[Buildroot] Queries/suggestions regarding patch series "grub2: add support for arm and aarch64"

[Carlos Santos]

> From: "Erico Nunes" <nunes.erico@gmail.com>
> To: "sumit garg" <sumit.garg@linaro.org>
> Cc: "buildroot" <buildroot@buildroot.org>
> Sent: Tuesday, October 30, 2018 4:23:27 PM
> Subject: Re: [Buildroot] Queries/suggestions regarding patch series "grub2: add support for arm and aarch64"

> Hi Sumit,
> 
> On Tue, Oct 23, 2018 at 7:58 AM Sumit Garg <sumit.garg@linaro.org> wrote:
>>
>> Hello Erico,
>>
>> Firstly, apologies for not posting these comments on your patch-set
>> [1] as I joined Buildroot ML yesterday only.
>>
>> I was recently exploring to test EFI boot using Buildroot on qemu
>> aarch64 target. But currently GRUB support is missing for aarch64
>> target in Buildroot. So I came across your patch-set. I have tested
>> your patch-set on qemu/u-boot aarch64 target using efi. So I would say
>> its:
>>
>> Tested-by: Sumit Garg <sumit.garg@linaro.org>
> 
> Thanks for your testing and feedback.
> 
> Yes it is taking a while to have this patchset reviewed, but right now
> there are well over 200 patches in the patchwork queue, so I think it
> is understandable that it is taking some time. At this point I believe
> it will only be considered after 2018.11 (for 2019.02) anyway.
> 
>> However, I will suggest you to use PARTUUID for root path in grub.cfg
>> as it will make it more generic and device independent as we could use
>> either virtio, sata, usb or mmc. So I would propose a similar change
>> as in [2].
>>
>> Apart from that, how do you think about adding qemu/u-boot target for
>> aarch64 to Buildroot? As u-boot already have efi booting support using
>> bootefi.
> 
> Using PARTUUID seems to be an interesting change. I'd like to hear the
> opinion of the other Buildroot developers before making this change in
> this patchset though, since most other platforms in Buildroot are not
> doing this.
> If there is interest in it, maybe we can do this change for other
> platforms too and in a separate patchset?
> For example, then it might be also interesting to change it in the
> pc_* platforms, as it may solve /dev/sda and /dev/vda issue for
> testing them with qemu.

PARTUUID is certainly an improvement. I used it here:

    https://patchwork.ozlabs.org/patch/976538/

-- 
Carlos Santos (Casantos) - DATACOM, P&D

[Buildroot] Queries/suggestions regarding patch series "grub2: add support for arm and aarch64"

[Thomas Petazzoni]

Hello,

On Tue, 30 Oct 2018 20:23:27 +0100, Erico Nunes wrote:

> Using PARTUUID seems to be an interesting change. I'd like to hear the
> opinion of the other Buildroot developers before making this change in
> this patchset though, since most other platforms in Buildroot are not
> doing this.

It is certainly a good idea to use PARTUUID when possible.

> If there is interest in it, maybe we can do this change for other
> platforms too and in a separate patchset?

In general yes. However, my understanding was that it works only with a
GPT partition table, because only the GPT partition table can embed a
unique identifier per partition, not the old MBR partition table. But
apparently this disk-signature thing allows to embed a disk-wide
identifier, not a per-partition one, so it works only when there's a
single partition ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] Queries/suggestions regarding patch series "grub2: add support for arm and aarch64"

[Sumit Garg]

On Fri, 2 Nov 2018 at 15:34, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> On Tue, 30 Oct 2018 20:23:27 +0100, Erico Nunes wrote:
>
> > Using PARTUUID seems to be an interesting change. I'd like to hear the
> > opinion of the other Buildroot developers before making this change in
> > this patchset though, since most other platforms in Buildroot are not
> > doing this.
>
> It is certainly a good idea to use PARTUUID when possible.
>
> > If there is interest in it, maybe we can do this change for other
> > platforms too and in a separate patchset?
>
> In general yes. However, my understanding was that it works only with a
> GPT partition table, because only the GPT partition table can embed a
> unique identifier per partition, not the old MBR partition table. But
> apparently this disk-signature thing allows to embed a disk-wide
> identifier, not a per-partition one, so it works only when there's a
> single partition ?
>

Yes disk-signature is disk-wide identifier. But it could work with
multiple partitions in following manner:

Like the change, I have proposed with disk-signature = 0xDEEDBEEF,
PARTUUID=deedbeef-02 refers to 2nd partition in disk with
corresponding signature.

AFAIK, I don't think partitioning format changes for a particular
device boot sequence if underlying disk changes (usb, sata, mmc etc.).
So "-02" string should remain a constant.

Regards,
Sumit

> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

[Buildroot] Queries/suggestions regarding patch series "grub2: add support for arm and aarch64"

[Thomas Petazzoni]

Hello Sumit,

On Fri, 2 Nov 2018 16:23:37 +0530, Sumit Garg wrote:

> Yes disk-signature is disk-wide identifier. But it could work with
> multiple partitions in following manner:
> 
> Like the change, I have proposed with disk-signature = 0xDEEDBEEF,
> PARTUUID=deedbeef-02 refers to 2nd partition in disk with
> corresponding signature.

Aaah, yes, I had entirely missed the -02 part in the root=PARTUUID=
statement. Indeed, it's a lot clearer now how this can work.

> AFAIK, I don't think partitioning format changes for a particular
> device boot sequence if underlying disk changes (usb, sata, mmc etc.).
> So "-02" string should remain a constant.

Yes, absolutely! Thanks for the additional explanation!

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] Queries/suggestions regarding patch series "grub2: add support for arm and aarch64"

[Erico Nunes]

On Fri, Nov 2, 2018 at 12:00 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Sumit,
>
> On Fri, 2 Nov 2018 16:23:37 +0530, Sumit Garg wrote:
>
> > Yes disk-signature is disk-wide identifier. But it could work with
> > multiple partitions in following manner:
> >
> > Like the change, I have proposed with disk-signature = 0xDEEDBEEF,
> > PARTUUID=deedbeef-02 refers to 2nd partition in disk with
> > corresponding signature.
>
> Aaah, yes, I had entirely missed the -02 part in the root=PARTUUID=
> statement. Indeed, it's a lot clearer now how this can work.
>
> > AFAIK, I don't think partitioning format changes for a particular
> > device boot sequence if underlying disk changes (usb, sata, mmc etc.).
> > So "-02" string should remain a constant.
>
> Yes, absolutely! Thanks for the additional explanation!

That looks nice indeed.

So I propose that if I need to respin the grub2 arm series, I will
incorporate the PARTUUID change. Otherwise, I can later send a
separate patchset changing it for aarch64_efi and the pc_* defconfigs.

Erico

[Buildroot] Queries/suggestions regarding patch series "grub2: add support for arm and aarch64"

[Carlos Santos]

> From: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> To: "Erico Nunes" <nunes.erico@gmail.com>
> Cc: "sumit garg" <sumit.garg@linaro.org>, "buildroot" <buildroot@buildroot.org>
> Sent: Sexta-feira, 2 de novembro de 2018 7:04:20
> Subject: Re: [Buildroot] Queries/suggestions regarding patch series "grub2: add support for arm and aarch64"

> Hello,
> 
> On Tue, 30 Oct 2018 20:23:27 +0100, Erico Nunes wrote:
> 
>> Using PARTUUID seems to be an interesting change. I'd like to hear the
>> opinion of the other Buildroot developers before making this change in
>> this patchset though, since most other platforms in Buildroot are not
>> doing this.
> 
> It is certainly a good idea to use PARTUUID when possible.
> 
>> If there is interest in it, maybe we can do this change for other
>> platforms too and in a separate patchset?
> 
> In general yes. However, my understanding was that it works only with a
> GPT partition table, because only the GPT partition table can embed a
> unique identifier per partition, not the old MBR partition table. But
> apparently this disk-signature thing allows to embed a disk-wide
> identifier, not a per-partition one, so it works only when there's a
> single partition ?

PARTUUID works with MBR partition tables too, even tough the ID is
not a true UUID. It's build from the 32-bit long disk ID plus the
partition number, e.g. "f000-baad-01", "f000-baad-02".

One can force the disk ID of a disk image to a known value with
something like this (requires host-util-linux):

    img_name="disk.img"
    disk_id="$("${HOST_DIR}/bin/uuidgen" -r | cut -d - -f 1)"
    printf 'x\ni\n0x%s\nr\nw\n' "${disk_id}" \
        | "${HOST_DIR}/sbin/fdisk" "${BINARIES_DIR}/${img_name}" \
        > /dev/null

-- 
Carlos Santos (Casantos) - DATACOM, P&D
?Marched towards the enemy, spear upright, armed with the certainty
that only the ignorant can have.? ? Epitaph of a volunteer