| Message ID | 20250120105045.1281262-1-sughosh.ganu@linaro.org |
|---|---|
| Headers | show |
| Series | Add pmem node for preserving distro ISO's | expand |
Heinrich, Tobias There's a slight problem that I forgot when commenting v2. Heinrich's idea of plugging this into blkmap is eventually the right thing to do. However, when I started coding this I only added the pmem memory as 'reserved' in the DT hoping that would work. Unfortunately, this depends on a kernel config option. I've managed to track down the problem here[0], but I haven't found time to test it properly and send it upstream. So for this feature to work reliably we *need* to remove the memory map we hand over to the OS. Since using EFI memmap function into the blkmap code makes no sense, can we perhaps merge v2 (or a variant of it), which only targets EFI, with an explanation of *why* while I try to sort out the kernel issue? [0] https://git.linaro.org/people/ilias.apalodimas/net-next.git/commit/?h=main Thanks /Ilias On Mon, 20 Jan 2025 at 12:51, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > When installing a distro via EFI HTTP boot some OS installers expect > the .iso image to be preserved and treat it as a "CDROM" to install > packages. > > This is problematic in EFI, since U-Boot mounts the image, starts the > installer, and eventually calls ExitBootServices. At that point the > image U-Boot mounted disappears. Some distros don't care and download > the missing packages from a web archive, while others halt the > installation complaining they can't find certain packages. > > If the firmware uses ACPI, this is supported by using NFIT which > provides NVDIMM ramdisks to the OS and preserves the image. > We don't have anything in place for Device Trees though. Since DT > supports persistent memory nodes (pmem) use those and preserve the > .iso for installers. > > The issue can be reproduced by attempting an EFI HTTP boot with Ubuntu > live server ISO, or a Rocky Linux ISO. The installation would fail > with the failure to locate certain packages. > > The earlier version had aligned the addition of pmem nodes with the > EFI HTTP boot feature. This has been changed, based on a review > comment from Heinrich to instead be done by looping through the memory > based blkmamp devices. > > Changes since V2: > * Fix a checkpatch error for putting a blank line after a function > * Use blkmap device based scanning for adding the pmem nodes > > > > Ilias Apalodimas (2): > efi_loader: add a function to remove memory from the EFI map > efi_loader: preserve installer images in pmem > > Masahisa Kojima (1): > fdt: add support for adding pmem nodes > > Sughosh Ganu (2): > blkmap: store type of blkmap device in corresponding structure > blkmap: add pmem nodes for blkmap mem mapping devices > > boot/fdt_support.c | 40 ++++++++++++- > boot/image-fdt.c | 9 +++ > cmd/blkmap.c | 16 ++++-- > drivers/block/blkmap.c | 90 +++-------------------------- > drivers/block/blkmap_helper.c | 47 +++++++++++++++- > include/blkmap.h | 103 +++++++++++++++++++++++++++++++++- > include/efi_loader.h | 11 ++-- > include/fdt_support.h | 13 +++++ > lib/efi_loader/efi_bootmgr.c | 22 ++++++-- > lib/efi_loader/efi_memory.c | 51 ++++++++++++----- > lib/lmb.c | 4 +- > test/dm/blkmap.c | 16 +++--- > 12 files changed, 302 insertions(+), 120 deletions(-) > > -- > 2.34.1 > >
On Fri, Jan 24, 2025 at 01:39:45PM +0200, Ilias Apalodimas wrote: > Heinrich, Tobias > > There's a slight problem that I forgot when commenting v2. > > Heinrich's idea of plugging this into blkmap is eventually the right > thing to do. > > However, when I started coding this I only added the pmem memory as > 'reserved' in the DT hoping that would work. > Unfortunately, this depends on a kernel config option. I've managed to > track down the problem here[0], but I haven't found time to test it > properly and send it upstream. > So for this feature to work reliably we *need* to remove the memory > map we hand over to the OS. > > Since using EFI memmap function into the blkmap code makes no sense, > can we perhaps merge v2 (or a variant of it), which only targets EFI, > with an explanation of *why* while I try to sort out the kernel issue? > > > [0] https://git.linaro.org/people/ilias.apalodimas/net-next.git/commit/?h=main I think this is a reasonable path forward, FWIW.
On fre, jan 24, 2025 at 13:39, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > Heinrich, Tobias > > There's a slight problem that I forgot when commenting v2. > > Heinrich's idea of plugging this into blkmap is eventually the right > thing to do. > > However, when I started coding this I only added the pmem memory as > 'reserved' in the DT hoping that would work. > Unfortunately, this depends on a kernel config option. I've managed to > track down the problem here[0], but I haven't found time to test it > properly and send it upstream. > So for this feature to work reliably we *need* to remove the memory > map we hand over to the OS. > > Since using EFI memmap function into the blkmap code makes no sense, > can we perhaps merge v2 (or a variant of it), which only targets EFI, > with an explanation of *why* while I try to sort out the kernel issue? I was not a part of the first two iterations of this series, but my view is basically this: Adding some flag to memory backed slices of block maps, that the fdt-fixup code can use to know whether a pmem node should be injected or not, is completely fine by me. What I am opposed to is adding restrictions on how block maps can be composed, i.e., limiting a block map to only contain either linear or memory mappings.
On Fri, 24 Jan 2025 at 17:10, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Heinrich, Tobias > > There's a slight problem that I forgot when commenting v2. > > Heinrich's idea of plugging this into blkmap is eventually the right > thing to do. > > However, when I started coding this I only added the pmem memory as > 'reserved' in the DT hoping that would work. > Unfortunately, this depends on a kernel config option. I've managed to > track down the problem here[0], but I haven't found time to test it > properly and send it upstream. > So for this feature to work reliably we *need* to remove the memory > map we hand over to the OS. > > Since using EFI memmap function into the blkmap code makes no sense, > can we perhaps merge v2 (or a variant of it), which only targets EFI, > with an explanation of *why* while I try to sort out the kernel issue? If it has been decided to go with the approach taken in V2, I will send an updated series which incorporates the other nits that Heinrich had on the patches. Those were handled in this version. -sughosh > > > [0] https://git.linaro.org/people/ilias.apalodimas/net-next.git/commit/?h=main > > Thanks > /Ilias > > On Mon, 20 Jan 2025 at 12:51, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > When installing a distro via EFI HTTP boot some OS installers expect > > the .iso image to be preserved and treat it as a "CDROM" to install > > packages. > > > > This is problematic in EFI, since U-Boot mounts the image, starts the > > installer, and eventually calls ExitBootServices. At that point the > > image U-Boot mounted disappears. Some distros don't care and download > > the missing packages from a web archive, while others halt the > > installation complaining they can't find certain packages. > > > > If the firmware uses ACPI, this is supported by using NFIT which > > provides NVDIMM ramdisks to the OS and preserves the image. > > We don't have anything in place for Device Trees though. Since DT > > supports persistent memory nodes (pmem) use those and preserve the > > .iso for installers. > > > > The issue can be reproduced by attempting an EFI HTTP boot with Ubuntu > > live server ISO, or a Rocky Linux ISO. The installation would fail > > with the failure to locate certain packages. > > > > The earlier version had aligned the addition of pmem nodes with the > > EFI HTTP boot feature. This has been changed, based on a review > > comment from Heinrich to instead be done by looping through the memory > > based blkmamp devices. > > > > Changes since V2: > > * Fix a checkpatch error for putting a blank line after a function > > * Use blkmap device based scanning for adding the pmem nodes > > > > > > > > Ilias Apalodimas (2): > > efi_loader: add a function to remove memory from the EFI map > > efi_loader: preserve installer images in pmem > > > > Masahisa Kojima (1): > > fdt: add support for adding pmem nodes > > > > Sughosh Ganu (2): > > blkmap: store type of blkmap device in corresponding structure > > blkmap: add pmem nodes for blkmap mem mapping devices > > > > boot/fdt_support.c | 40 ++++++++++++- > > boot/image-fdt.c | 9 +++ > > cmd/blkmap.c | 16 ++++-- > > drivers/block/blkmap.c | 90 +++-------------------------- > > drivers/block/blkmap_helper.c | 47 +++++++++++++++- > > include/blkmap.h | 103 +++++++++++++++++++++++++++++++++- > > include/efi_loader.h | 11 ++-- > > include/fdt_support.h | 13 +++++ > > lib/efi_loader/efi_bootmgr.c | 22 ++++++-- > > lib/efi_loader/efi_memory.c | 51 ++++++++++++----- > > lib/lmb.c | 4 +- > > test/dm/blkmap.c | 16 +++--- > > 12 files changed, 302 insertions(+), 120 deletions(-) > > > > -- > > 2.34.1 > > > >
Hi Sughosh, On Mon, 27 Jan 2025 at 08:47, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > On Fri, 24 Jan 2025 at 17:10, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > Heinrich, Tobias > > > > There's a slight problem that I forgot when commenting v2. > > > > Heinrich's idea of plugging this into blkmap is eventually the right > > thing to do. > > > > However, when I started coding this I only added the pmem memory as > > 'reserved' in the DT hoping that would work. > > Unfortunately, this depends on a kernel config option. I've managed to > > track down the problem here[0], but I haven't found time to test it > > properly and send it upstream. > > So for this feature to work reliably we *need* to remove the memory > > map we hand over to the OS. > > > > Since using EFI memmap function into the blkmap code makes no sense, > > can we perhaps merge v2 (or a variant of it), which only targets EFI, > > with an explanation of *why* while I try to sort out the kernel issue? > > If it has been decided to go with the approach taken in V2, I will > send an updated series which incorporates the other nits that Heinrich > had on the patches. Those were handled in this version. Yea I think that's the sane thing we can do for now, since this depends on being able to remove the memory for the map we present to the OS. Just don't limit it to http only. Cheers /Ilias > > -sughosh > > > > > > > [0] https://git.linaro.org/people/ilias.apalodimas/net-next.git/commit/?h=main > > > > Thanks > > /Ilias > > > > On Mon, 20 Jan 2025 at 12:51, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > > When installing a distro via EFI HTTP boot some OS installers expect > > > the .iso image to be preserved and treat it as a "CDROM" to install > > > packages. > > > > > > This is problematic in EFI, since U-Boot mounts the image, starts the > > > installer, and eventually calls ExitBootServices. At that point the > > > image U-Boot mounted disappears. Some distros don't care and download > > > the missing packages from a web archive, while others halt the > > > installation complaining they can't find certain packages. > > > > > > If the firmware uses ACPI, this is supported by using NFIT which > > > provides NVDIMM ramdisks to the OS and preserves the image. > > > We don't have anything in place for Device Trees though. Since DT > > > supports persistent memory nodes (pmem) use those and preserve the > > > .iso for installers. > > > > > > The issue can be reproduced by attempting an EFI HTTP boot with Ubuntu > > > live server ISO, or a Rocky Linux ISO. The installation would fail > > > with the failure to locate certain packages. > > > > > > The earlier version had aligned the addition of pmem nodes with the > > > EFI HTTP boot feature. This has been changed, based on a review > > > comment from Heinrich to instead be done by looping through the memory > > > based blkmamp devices. > > > > > > Changes since V2: > > > * Fix a checkpatch error for putting a blank line after a function > > > * Use blkmap device based scanning for adding the pmem nodes > > > > > > > > > > > > Ilias Apalodimas (2): > > > efi_loader: add a function to remove memory from the EFI map > > > efi_loader: preserve installer images in pmem > > > > > > Masahisa Kojima (1): > > > fdt: add support for adding pmem nodes > > > > > > Sughosh Ganu (2): > > > blkmap: store type of blkmap device in corresponding structure > > > blkmap: add pmem nodes for blkmap mem mapping devices > > > > > > boot/fdt_support.c | 40 ++++++++++++- > > > boot/image-fdt.c | 9 +++ > > > cmd/blkmap.c | 16 ++++-- > > > drivers/block/blkmap.c | 90 +++-------------------------- > > > drivers/block/blkmap_helper.c | 47 +++++++++++++++- > > > include/blkmap.h | 103 +++++++++++++++++++++++++++++++++- > > > include/efi_loader.h | 11 ++-- > > > include/fdt_support.h | 13 +++++ > > > lib/efi_loader/efi_bootmgr.c | 22 ++++++-- > > > lib/efi_loader/efi_memory.c | 51 ++++++++++++----- > > > lib/lmb.c | 4 +- > > > test/dm/blkmap.c | 16 +++--- > > > 12 files changed, 302 insertions(+), 120 deletions(-) > > > > > > -- > > > 2.34.1 > > > > > >