diff mbox series

[3/4] efi_loader: preserve installer images in pmem

Message ID 20241025111411.165904-4-sughosh.ganu@linaro.org
State Superseded
Headers show
Series Add pmem node for preserving distro ISO's | expand

Commit Message

Sughosh Ganu Oct. 25, 2024, 11:14 a.m. UTC
From: Ilias Apalodimas <ilias.apalodimas@linaro.org>

One of the problems OS installers face, when running in EFI, is that
the mounted ISO after calling ExitBootServices goes away. For some
distros this is a problem since they rely on finding some core packages
before continuing the installation. Distros have works around this --
e.g Fedora has a special kernel command line parameter called
inst.stage2 [0].

ACPI has NFIT and NVDIMM support to provide ramdisks to the OS, but we
don't have anything in place for DTs. Linux and device trees have support
for persistent memory devices.

It's worth noting that for linux to instantiate the /dev/pmemX device,
the memory described in the pmem node has to be omitted from the EFI
memory map we hand over to the OS if ZONE_DEVICES and SPARSEMEM is
enabled. With those enabled the pmem driver ends up calling
devm_memremap_pages() instead of devm_memremap(). The latter works
whether the memory is omitted or marked as reserved, but mapping pages
only works if the memory is omitted.

On top of that, depending on how the kernel is configured, that memory
area must be page aligned or 2MB aligned. PowerPC is an exception here
and requires 16MB alignment, but since we don't have EFI support for
it, limit the alignment to 2MB.

Ensure that the ISO image is 2MB aligned and remove the region
occupied by the image from the EFI memory map.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
 lib/efi_loader/efi_bootmgr.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

Comments

Ilias Apalodimas Oct. 25, 2024, 11:16 a.m. UTC | #1
+CC Anton

On Fri, 25 Oct 2024 at 14:14, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> One of the problems OS installers face, when running in EFI, is that
> the mounted ISO after calling ExitBootServices goes away. For some
> distros this is a problem since they rely on finding some core packages
> before continuing the installation. Distros have works around this --
> e.g Fedora has a special kernel command line parameter called
> inst.stage2 [0].
>
> ACPI has NFIT and NVDIMM support to provide ramdisks to the OS, but we
> don't have anything in place for DTs. Linux and device trees have support
> for persistent memory devices.
>
> It's worth noting that for linux to instantiate the /dev/pmemX device,
> the memory described in the pmem node has to be omitted from the EFI
> memory map we hand over to the OS if ZONE_DEVICES and SPARSEMEM is
> enabled. With those enabled the pmem driver ends up calling
> devm_memremap_pages() instead of devm_memremap(). The latter works
> whether the memory is omitted or marked as reserved, but mapping pages
> only works if the memory is omitted.
>
> On top of that, depending on how the kernel is configured, that memory
> area must be page aligned or 2MB aligned. PowerPC is an exception here
> and requires 16MB alignment, but since we don't have EFI support for
> it, limit the alignment to 2MB.
>
> Ensure that the ISO image is 2MB aligned and remove the region
> occupied by the image from the EFI memory map.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>  lib/efi_loader/efi_bootmgr.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index a3aa2b8d1b..16f75555f6 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -18,6 +18,8 @@
>  #include <efi_loader.h>
>  #include <efi_variable.h>
>  #include <asm/unaligned.h>
> +#include <linux/kernel.h>
> +#include <linux/sizes.h>
>
>  static const struct efi_boot_services *bs;
>  static const struct efi_runtime_services *rs;
> @@ -358,13 +360,16 @@ static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size,
>         }
>
>         /*
> -        * TODO: expose the ramdisk to OS.
> -        * Need to pass the ramdisk information by the architecture-specific
> -        * methods such as 'pmem' device-tree node.
> +        * Linux supports 'pmem' which allows OS installers to find, reclaim
> +        * the mounted images and continue the installation since the contents
> +        * of the pmem region are treated as local media.
> +        *
> +        * The memory regions used for it needs to be carved out of the EFI
> +        * memory map.
>          */
> -       ret = efi_add_memory_map(addr, size, EFI_RESERVED_MEMORY_TYPE);
> +       ret = efi_remove_memory_map(addr, size, EFI_CONVENTIONAL_MEMORY);
>         if (ret != EFI_SUCCESS) {
> -               log_err("Memory reservation failed\n");
> +               log_err("Failed to reserve memory\n");
>                 goto err;
>         }
>
> @@ -486,6 +491,13 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp,
>                 ret = EFI_INVALID_PARAMETER;
>                 goto err;
>         }
> +       /*
> +        * Depending on the kernel configuration, pmem memory area must be page
> +        * aligned or 2MB aligned. PowerPC is an exception here and requires
> +        * 16MB alignment, but since we don't have EFI support for it, limit
> +        * the alignment to 2MB.
> +        */
> +       image_size = ALIGN(image_size, SZ_2M);
>
>         /*
>          * If the file extension is ".iso" or ".img", mount it and try to load
> --
> 2.34.1
>
Heinrich Schuchardt Nov. 11, 2024, 8 a.m. UTC | #2
On 10/25/24 13:14, Sughosh Ganu wrote:
> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> One of the problems OS installers face, when running in EFI, is that
> the mounted ISO after calling ExitBootServices goes away. For some
> distros this is a problem since they rely on finding some core packages
> before continuing the installation. Distros have works around this --
> e.g Fedora has a special kernel command line parameter called
> inst.stage2 [0].
>
> ACPI has NFIT and NVDIMM support to provide ramdisks to the OS, but we
> don't have anything in place for DTs. Linux and device trees have support
> for persistent memory devices.
>
> It's worth noting that for linux to instantiate the /dev/pmemX device,
> the memory described in the pmem node has to be omitted from the EFI
> memory map we hand over to the OS if ZONE_DEVICES and SPARSEMEM is
> enabled. With those enabled the pmem driver ends up calling
> devm_memremap_pages() instead of devm_memremap(). The latter works
> whether the memory is omitted or marked as reserved, but mapping pages
> only works if the memory is omitted.
>
> On top of that, depending on how the kernel is configured, that memory
> area must be page aligned or 2MB aligned. PowerPC is an exception here
> and requires 16MB alignment, but since we don't have EFI support for
> it, limit the alignment to 2MB.
>
> Ensure that the ISO image is 2MB aligned and remove the region
> occupied by the image from the EFI memory map.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>   lib/efi_loader/efi_bootmgr.c | 22 +++++++++++++++++-----
>   1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index a3aa2b8d1b..16f75555f6 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -18,6 +18,8 @@
>   #include <efi_loader.h>
>   #include <efi_variable.h>
>   #include <asm/unaligned.h>
> +#include <linux/kernel.h>
> +#include <linux/sizes.h>
>
>   static const struct efi_boot_services *bs;
>   static const struct efi_runtime_services *rs;
> @@ -358,13 +360,16 @@ static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size,
>   	}
>
>   	/*
> -	 * TODO: expose the ramdisk to OS.
> -	 * Need to pass the ramdisk information by the architecture-specific
> -	 * methods such as 'pmem' device-tree node.
> +	 * Linux supports 'pmem' which allows OS installers to find, reclaim
> +	 * the mounted images and continue the installation since the contents
> +	 * of the pmem region are treated as local media.
> +	 *
> +	 * The memory regions used for it needs to be carved out of the EFI
> +	 * memory map.
>   	 */
> -	ret = efi_add_memory_map(addr, size, EFI_RESERVED_MEMORY_TYPE);
> +	ret = efi_remove_memory_map(addr, size, EFI_CONVENTIONAL_MEMORY);
>   	if (ret != EFI_SUCCESS) {
> -		log_err("Memory reservation failed\n");
> +		log_err("Failed to reserve memory\n");
>   		goto err;
>   	}
>
> @@ -486,6 +491,13 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp,
>   		ret = EFI_INVALID_PARAMETER;
>   		goto err;
>   	}
> +	/*
> +	 * Depending on the kernel configuration, pmem memory area must be page
> +	 * aligned or 2MB aligned. PowerPC is an exception here and requires
> +	 * 16MB alignment, but since we don't have EFI support for it, limit
> +	 * the alignment to 2MB.
> +	 */
> +	image_size = ALIGN(image_size, SZ_2M);

The code regarding .iso and .img handling seems to be misplaced. Why
should we treat a file loaded from a block device differently to a file
loaded from the network?

Best regards

Heinrich

>
>   	/*
>   	 * If the file extension is ".iso" or ".img", mount it and try to load
Ilias Apalodimas Nov. 11, 2024, 10:46 a.m. UTC | #3
On Mon, 11 Nov 2024 at 10:05, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/25/24 13:14, Sughosh Ganu wrote:
> > From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> >
> > One of the problems OS installers face, when running in EFI, is that
> > the mounted ISO after calling ExitBootServices goes away. For some
> > distros this is a problem since they rely on finding some core packages
> > before continuing the installation. Distros have works around this --
> > e.g Fedora has a special kernel command line parameter called
> > inst.stage2 [0].
> >
> > ACPI has NFIT and NVDIMM support to provide ramdisks to the OS, but we
> > don't have anything in place for DTs. Linux and device trees have support
> > for persistent memory devices.
> >
> > It's worth noting that for linux to instantiate the /dev/pmemX device,
> > the memory described in the pmem node has to be omitted from the EFI
> > memory map we hand over to the OS if ZONE_DEVICES and SPARSEMEM is
> > enabled. With those enabled the pmem driver ends up calling
> > devm_memremap_pages() instead of devm_memremap(). The latter works
> > whether the memory is omitted or marked as reserved, but mapping pages
> > only works if the memory is omitted.
> >
> > On top of that, depending on how the kernel is configured, that memory
> > area must be page aligned or 2MB aligned. PowerPC is an exception here
> > and requires 16MB alignment, but since we don't have EFI support for
> > it, limit the alignment to 2MB.
> >
> > Ensure that the ISO image is 2MB aligned and remove the region
> > occupied by the image from the EFI memory map.
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> >   lib/efi_loader/efi_bootmgr.c | 22 +++++++++++++++++-----
> >   1 file changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> > index a3aa2b8d1b..16f75555f6 100644
> > --- a/lib/efi_loader/efi_bootmgr.c
> > +++ b/lib/efi_loader/efi_bootmgr.c
> > @@ -18,6 +18,8 @@
> >   #include <efi_loader.h>
> >   #include <efi_variable.h>
> >   #include <asm/unaligned.h>
> > +#include <linux/kernel.h>
> > +#include <linux/sizes.h>
> >
> >   static const struct efi_boot_services *bs;
> >   static const struct efi_runtime_services *rs;
> > @@ -358,13 +360,16 @@ static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size,
> >       }
> >
> >       /*
> > -      * TODO: expose the ramdisk to OS.
> > -      * Need to pass the ramdisk information by the architecture-specific
> > -      * methods such as 'pmem' device-tree node.
> > +      * Linux supports 'pmem' which allows OS installers to find, reclaim
> > +      * the mounted images and continue the installation since the contents
> > +      * of the pmem region are treated as local media.
> > +      *
> > +      * The memory regions used for it needs to be carved out of the EFI
> > +      * memory map.
> >        */
> > -     ret = efi_add_memory_map(addr, size, EFI_RESERVED_MEMORY_TYPE);
> > +     ret = efi_remove_memory_map(addr, size, EFI_CONVENTIONAL_MEMORY);
> >       if (ret != EFI_SUCCESS) {
> > -             log_err("Memory reservation failed\n");
> > +             log_err("Failed to reserve memory\n");
> >               goto err;
> >       }
> >
> > @@ -486,6 +491,13 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp,
> >               ret = EFI_INVALID_PARAMETER;
> >               goto err;
> >       }
> > +     /*
> > +      * Depending on the kernel configuration, pmem memory area must be page
> > +      * aligned or 2MB aligned. PowerPC is an exception here and requires
> > +      * 16MB alignment, but since we don't have EFI support for it, limit
> > +      * the alignment to 2MB.
> > +      */
> > +     image_size = ALIGN(image_size, SZ_2M);
>
> The code regarding .iso and .img handling seems to be misplaced. Why
> should we treat a file loaded from a block device differently to a file
> loaded from the network?

If the installer is located in a block device, the kernel will find
the image when it scans those devices during boot. It's only when we
have it on a memory area that we need to preserve it

Thanks
/Ilias
>
> Best regards
>
> Heinrich
>
> >
> >       /*
> >        * If the file extension is ".iso" or ".img", mount it and try to load
>
Heinrich Schuchardt Nov. 11, 2024, 11:23 a.m. UTC | #4
On 11/11/24 11:46, Ilias Apalodimas wrote:
> On Mon, 11 Nov 2024 at 10:05, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 10/25/24 13:14, Sughosh Ganu wrote:
>>> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>>>
>>> One of the problems OS installers face, when running in EFI, is that
>>> the mounted ISO after calling ExitBootServices goes away. For some
>>> distros this is a problem since they rely on finding some core packages
>>> before continuing the installation. Distros have works around this --
>>> e.g Fedora has a special kernel command line parameter called
>>> inst.stage2 [0].
>>>
>>> ACPI has NFIT and NVDIMM support to provide ramdisks to the OS, but we
>>> don't have anything in place for DTs. Linux and device trees have support
>>> for persistent memory devices.
>>>
>>> It's worth noting that for linux to instantiate the /dev/pmemX device,
>>> the memory described in the pmem node has to be omitted from the EFI
>>> memory map we hand over to the OS if ZONE_DEVICES and SPARSEMEM is
>>> enabled. With those enabled the pmem driver ends up calling
>>> devm_memremap_pages() instead of devm_memremap(). The latter works
>>> whether the memory is omitted or marked as reserved, but mapping pages
>>> only works if the memory is omitted.
>>>
>>> On top of that, depending on how the kernel is configured, that memory
>>> area must be page aligned or 2MB aligned. PowerPC is an exception here
>>> and requires 16MB alignment, but since we don't have EFI support for
>>> it, limit the alignment to 2MB.
>>>
>>> Ensure that the ISO image is 2MB aligned and remove the region
>>> occupied by the image from the EFI memory map.
>>>
>>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>>> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
>>> ---
>>>    lib/efi_loader/efi_bootmgr.c | 22 +++++++++++++++++-----
>>>    1 file changed, 17 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>>> index a3aa2b8d1b..16f75555f6 100644
>>> --- a/lib/efi_loader/efi_bootmgr.c
>>> +++ b/lib/efi_loader/efi_bootmgr.c
>>> @@ -18,6 +18,8 @@
>>>    #include <efi_loader.h>
>>>    #include <efi_variable.h>
>>>    #include <asm/unaligned.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/sizes.h>
>>>
>>>    static const struct efi_boot_services *bs;
>>>    static const struct efi_runtime_services *rs;
>>> @@ -358,13 +360,16 @@ static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size,
>>>        }
>>>
>>>        /*
>>> -      * TODO: expose the ramdisk to OS.
>>> -      * Need to pass the ramdisk information by the architecture-specific
>>> -      * methods such as 'pmem' device-tree node.
>>> +      * Linux supports 'pmem' which allows OS installers to find, reclaim
>>> +      * the mounted images and continue the installation since the contents
>>> +      * of the pmem region are treated as local media.
>>> +      *
>>> +      * The memory regions used for it needs to be carved out of the EFI
>>> +      * memory map.
>>>         */
>>> -     ret = efi_add_memory_map(addr, size, EFI_RESERVED_MEMORY_TYPE);
>>> +     ret = efi_remove_memory_map(addr, size, EFI_CONVENTIONAL_MEMORY);
>>>        if (ret != EFI_SUCCESS) {
>>> -             log_err("Memory reservation failed\n");
>>> +             log_err("Failed to reserve memory\n");
>>>                goto err;
>>>        }
>>>
>>> @@ -486,6 +491,13 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp,
>>>                ret = EFI_INVALID_PARAMETER;
>>>                goto err;
>>>        }
>>> +     /*
>>> +      * Depending on the kernel configuration, pmem memory area must be page
>>> +      * aligned or 2MB aligned. PowerPC is an exception here and requires
>>> +      * 16MB alignment, but since we don't have EFI support for it, limit
>>> +      * the alignment to 2MB.
>>> +      */
>>> +     image_size = ALIGN(image_size, SZ_2M);
>>
>> The code regarding .iso and .img handling seems to be misplaced. Why
>> should we treat a file loaded from a block device differently to a file
>> loaded from the network?
>
> If the installer is located in a block device, the kernel will find
> the image when it scans those devices during boot. It's only when we
> have it on a memory area that we need to preserve it

I meant we could have a block device with an '*.iso' file on it, which
we load into memory.

Simon has been working on keeping track of loaded images. Possibly
besides EFI binaries we can keep track of loaded ISOs in the same way in
future.

For the moment being let't use the patch as is.

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

>
> Thanks
> /Ilias
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>>        /*
>>>         * If the file extension is ".iso" or ".img", mount it and try to load
>>
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index a3aa2b8d1b..16f75555f6 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -18,6 +18,8 @@ 
 #include <efi_loader.h>
 #include <efi_variable.h>
 #include <asm/unaligned.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>
 
 static const struct efi_boot_services *bs;
 static const struct efi_runtime_services *rs;
@@ -358,13 +360,16 @@  static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size,
 	}
 
 	/*
-	 * TODO: expose the ramdisk to OS.
-	 * Need to pass the ramdisk information by the architecture-specific
-	 * methods such as 'pmem' device-tree node.
+	 * Linux supports 'pmem' which allows OS installers to find, reclaim
+	 * the mounted images and continue the installation since the contents
+	 * of the pmem region are treated as local media.
+	 *
+	 * The memory regions used for it needs to be carved out of the EFI
+	 * memory map.
 	 */
-	ret = efi_add_memory_map(addr, size, EFI_RESERVED_MEMORY_TYPE);
+	ret = efi_remove_memory_map(addr, size, EFI_CONVENTIONAL_MEMORY);
 	if (ret != EFI_SUCCESS) {
-		log_err("Memory reservation failed\n");
+		log_err("Failed to reserve memory\n");
 		goto err;
 	}
 
@@ -486,6 +491,13 @@  static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp,
 		ret = EFI_INVALID_PARAMETER;
 		goto err;
 	}
+	/*
+	 * Depending on the kernel configuration, pmem memory area must be page
+	 * aligned or 2MB aligned. PowerPC is an exception here and requires
+	 * 16MB alignment, but since we don't have EFI support for it, limit
+	 * the alignment to 2MB.
+	 */
+	image_size = ALIGN(image_size, SZ_2M);
 
 	/*
 	 * If the file extension is ".iso" or ".img", mount it and try to load