Re: [PATCH v2] Makefile: Allow to set file systems modules for default_payload.elf

[Paul Menzel]

Dear Daniel,


On 07.03.19 20:37, Daniel Kiper wrote:
> On Thu, Mar 07, 2019 at 12:16:06PM +0100, Paul Menzel wrote:
> > Date: Wed Mar 6 08:14:28 2019 +0100
> >
> > By default all file system modules are added to the GRUB coreboot
> > payload `default_payload.elf`. This makes the image quite big,
> > especially as often not all modules are needed.
> >
> > Introduce the variable `FS_PAYLOAD_MODULES`, which can be used to
> > explicitly set file systems modules to be added.
> >
> >      $ make default_payload.elf
> >      test -f default_payload.elf && rm default_payload.elf || true
> >      pkgdatadir=. ./grub-mkstandalone --grub-mkimage=./grub-mkimage -O 
> > i386-coreboot -o default_payload.elf --modules='ahci pata ehci uhci ohci 
> > usb_keyboard usbms part_msdos ext2 fat at_keyboard part_gpt usbserial_usbdebug 
> > cbfs' --install-modules='ls linux search configfile normal cbtime cbls memrw 
> > iorw minicmd lsmmap lspci halt reboot hexdump pcidump regexp setpci lsacpi 
> > chain test serial multiboot cbmemc linux16 gzio echo help syslinuxcfg xnu affs 
> > afs bfs btrfs cbfs cpio cpio_be exfat ext2 f2fs fat hfs hfsplus iso9660 jfs 
> > minix minix2 minix2_be minix3 minix3_be minix_be newc nilfs2 ntfs odc procfs 
> > reiserfs romfs sfs squash4 tar udf ufs1 ufs1_be ufs2 xfs zfs password_pbkdf2 ' 
> > --fonts= --themes= --locales= -d grub-core/ /boot/grub/grub.cfg=./coreboot.cfg
> >      $ ls -l default_payload.elf
> >      -rw-rw---- 1 joey joey 1199568 Mar  6 13:58 default_payload.elf
> >
> >      $ make default_payload.elf FS_PAYLOAD_MODULES="" # ext2 already in 
> > `--modules`
> >      test -f default_payload.elf && rm default_payload.elf || true
> >      pkgdatadir=. ./grub-mkstandalone --grub-mkimage=./grub-mkimage -O 
> > i386-coreboot -o default_payload.elf --modules='ahci pata ehci uhci ohci 
> > usb_keyboard usbms part_msdos ext2 fat at_keyboard part_gpt usbserial_usbdebug 
> > cbfs' --install-modules='ls linux search configfile normal cbtime cbls memrw 
> > iorw minicmd lsmmap lspci halt reboot hexdump pcidump regexp setpci lsacpi 
> > chain test serial multiboot cbmemc linux16 gzio echo help syslinuxcfg xnu  
> > password_pbkdf2 ' --fonts= --themes= --locales= -d grub-core/ 
> > /boot/grub/grub.cfg=./coreboot.cfg
> >      $ ls -l default_payload.elf
> >      -rw-rw---- 1 joey joey 832976 Mar  7 12:13 default_payload.elf
> >
> > So, the resulting payload size is around 370 kB smaller. (Adding it to
> > the CBFS, it will be compressed, so the effective size difference will
> > be smaller.)
> >
> > Signed-off-by: Paul Menzel <address@hidden>
> > ---
> >   Makefile.am | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 94296a37e..1f4bb9b8c 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -422,9 +422,10 @@ BOOTCHECK_TIMEOUT=180
> >   bootcheck: $(BOOTCHECKS)
> >
> >   if COND_i386_coreboot
> > +FS_PAYLOAD_MODULES ?= $(shell cat grub-core/fs.lst)
> >   default_payload.elf: grub-mkstandalone grub-mkimage FORCE
> >         test -f $@ && rm $@ || true
> > -       pkgdatadir=. ./grub-mkstandalone --grub-mkimage=./grub-mkimage -O 
> > i386-coreboot -o $@ --modules='ahci pata ehci uhci ohci usb_keyboard usbms 
> > part_msdos ext2 fat at_keyboard part_gpt usbserial_usbdebug cbfs' 
> > --install-modules='ls linux search configfile normal cbtime cbls memrw iorw 
> > minicmd lsmmap lspci halt reboot hexdump pcidump regexp setpci lsacpi chain 
> > test serial multiboot cbmemc linux16 gzio echo help syslinuxcfg xnu $(shell cat 
> > grub-core/fs.lst) password_pbkdf2 $(EXTRA_PAYLOAD_MODULES)' --fonts= --themes= 
> > --locales= -d grub-core/ /boot/grub/grub.cfg=$(srcdir)/coreboot.cfg
>
> If you do that why not go further and drop...
>
> > +       pkgdatadir=. ./grub-mkstandalone --grub-mkimage=./grub-mkimage
> > -O i386-coreboot -o $@ --modules='ahci pata ehci uhci ohci
> > usb_keyboard usbms part_msdos ext2 fat at_keyboard part_gpt
>
> ... ext2, fat...

They are added to the “core image”, so that one has a chance to load 
modules or other files like payloads from these common file systems. I 
suppose, it’s safer to have these included by default.

> > usbserial_usbdebug cbfs' --install-modules='ls linux search configfile
>
> ... and cbfs from here? Does anything depend on these filesystems?

Yes, at least the configuration file `etc/grub.cfg` is stored in CBFS. 
The file is read by the GRUB payload by default. So the module *cbfs* is 
essential for the GRUB payload.

> > normal cbtime cbls memrw iorw minicmd lsmmap lspci halt reboot hexdump
> > pcidump regexp setpci lsacpi chain test serial multiboot cbmemc
> > linux16 gzio echo help syslinuxcfg xnu $(FS_PAYLOAD_MODULES)
> > password_pbkdf2 $(EXTRA_PAYLOAD_MODULES)' --fonts= --themes=
> > --locales= -d grub-core/ /boot/grub/grub.cfg=$(srcdir)/coreboot.cfg


Kind regards,

Paul