[PATCH v2 00/41] pxe: Support read_all() for extlinux and PXE

[Simon Glass]

Hi Tom,

On Tue, 3 Dec 2024 at 17:37, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Tom,
>
> On Tue, 3 Dec 2024 at 17:12, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Tue, Dec 03, 2024 at 05:03:26PM -0700, Simon Glass wrote:
> > > Hi,
> > >
> > > On Tue, 3 Dec 2024 at 16:46, Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > This series implements read_all() so that it is possible to read all the
> > > > files relating to a bootflow, make adjustments and then boot.
> > > >
> > > > Unfortunately quite a few things stand in the way, so this series
> > > > finishes off several pending items: zboot without CONFIG_CMDLINE,
> > > > required support in pxe_utils and the differing code in the extlinux and
> > > > PXE bootmeths. There is very little new code, but quite a lot of
> > > > refactoring.
> > > >
> > > > The bootm, booti and bootz commands have all been refactored previously,
> > > > so that they can operate without needing CONFIG_CMDLINE to be enabled.
> > > > At the, time, zboot was left alone, since it is x86-specific and a bit
> > > > more trouble.
> > > >
> > > > This series adds a programatic API for zboot and uses the forthcoming
> > > > bootstd 'image list' to collect information from an extlinux file. It is
> > > > therefore possible to parse the file, load the resulting images and then
> > > > examine/adjust the resulting bootflow, before booting.
> > > >
> > > > The addition of options to extlinux resulted in the PXE and extlinux
> > > > bootmeth having slightly different features. This is tidied up in this
> > > > series, with common functions for both. This allows the same features
> > > > (loading images as a separate step) to be provided for PXE.
> > > >
> > > > Changes in v2:
> > > > - Split out strlcpy() change into new patch
> > > > - Move strlcpy() change into an earlier patch
> > > > - Fix 'initaddr' typo'
> > > > - Rebase on earlier change
> > > > - Add new patch to simplify the code reading fdtcontroladdr and fdt_addr
> > > >
> > > > Simon Glass (41):
> > > >   x86: Make do_zboot_states() static
> > > >   x86: Rename zboot_run() to zboot_run_args()
> > > >   x86: Drop duplicate definition of zimage_dump()
> > > >   x86: Move x86 zboot state into struct bootm_info
> > > >   x86: Rename state to bmi
> > > >   x86: Move the bootm state for zimage into cmd/
> > > >   bootstd: Correct display of kernel version
> > > >   x86: Drop the unnecessary base_ptr argument to zboot_dump()
> > > >   boot: Use strlcpy() in label_boot()
> > > >   boot: Split pxe label_boot() into two parts
> > > >   boot: Split pxe label_run_boot() into two parts
> > > >   boot: Pass just the FDT argument to label_process_fdt()
> > > >   bootm: Allow building bootm.c without CONFIG_SYS_BOOTM_LEN
> > > >   boot: pxe: Use bootm_...() functions where possible
> > > >   pxe_utils: Simplify default fdt in label_run_boot()
> > > >   boot: pxe: Refactor label_run_boot() to avoid cmdline
> > > >   net: Keep the bootstage functions together
> > > >   net: Tidy up the comments to parse_args()
> > > >   net: Simplify parse_args()
> > > >   net: Return the load address from parse_args()
> > > >   net: Return the address and size from parse_addr_size()
> > > >   net: Return the size from parse_args()
> > > >   net: Refactor part of netboot_common() into a function
> > > >   net: Drop #ifdef in parse_args()
> > > >   net: Provide a function to run network operations
> > > >   boot: Avoid using the cmdline in bootmeth_pxe and pxe cmd
> > > >   pxe: Drop the cmdline parameter
> > > >   pxe: Record the bootflow in the PXE context
> > > >   pxe: Allow skipping the boot
> > > >   test: Update bootm test to restore silent_console
> > > >   bootmeth_extlinux: Move extlinux_info into plat
> > > >   bootmeth_extlinux: Move pxe_context into plat
> > > >   bootmeth: Refactor to put options in a common file
> > > >   bootmeth_pxe: Implement the fallback option
> > > >   bootmeth_pxe: Drop the driver-private data
> > > >   bootmeth_extlinux: Move boot code into common file
> > > >   bootstd: Update extlinux and pxe to allow boot interruption
> > > >   pxe: Collect the FDT in the bootflow
> > > >   pxe: Deal with a missing FDT in the bootflow
> > > >   pxe_utils: Refactor to separate reading from booting
> > > >   bootmeth_extlinux: Refactor extlinux to split boot
> > > >
> > > >  arch/x86/include/asm/zimage.h |  57 +---
> > > >  arch/x86/lib/zimage.c         | 102 ++++---
> > > >  boot/Makefile                 |   6 +-
> > > >  boot/bootm.c                  |   8 +-
> > > >  boot/bootmeth_cros.c          |   6 +-
> > > >  boot/bootmeth_extlinux.c      |  97 +------
> > > >  boot/bootmeth_pxe.c           |  48 ++--
> > > >  boot/ext_pxe_common.c         | 134 +++++++++
> > > >  boot/pxe_utils.c              | 507 +++++++++++++++++++++-------------
> > > >  cmd/bootflow.c                |   8 +-
> > > >  cmd/net.c                     |  92 +++---
> > > >  cmd/pxe.c                     |  30 +-
> > > >  cmd/sysboot.c                 |   4 +-
> > > >  cmd/x86/zboot.c               |  33 ++-
> > > >  include/bootm.h               |  63 ++++-
> > > >  include/bootmeth.h            |   8 +-
> > > >  include/extlinux.h            |  52 +++-
> > > >  include/net-common.h          |  30 ++
> > > >  include/pxe_utils.h           |  56 +++-
> > > >  net/net.c                     |  44 +++
> > > >  test/boot/bootflow.c          |  71 +++++
> > > >  test/boot/bootm.c             |   7 +
> > > >  22 files changed, 957 insertions(+), 506 deletions(-)
> > > >  create mode 100644 boot/ext_pxe_common.c
> > > >
> > > > --
> > > > 2.34.1
> > > >
> > >
> > > Unfortunately it seems there is a bug in here somewhere, in that it
> > > prepends the extlinux file's path to any files which are read, at
> > > least with pxe. So I am going to need to take another look...
> > >
> > > We don't have sandbox tests for this case, unfortunately. At some
> > > point we should try to add some.
> >
> > Does the pxelinux test not catch the problem? If not, that would be
> > where to start expanding perhaps.
>
> Right...it needs a little more work, but for now I'll have to fix the
> bug, as it's already 41 patches. I might be able to add a smaller
> test, will see.

It turned out to be a boolean which needs a different value between
extlinux and PXE. I didn't notice it at first. So I'll send this
series once it passes CI

Regards,
Simon