Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Librem image shrink #1232

Merged
merged 2 commits into from
Dec 1, 2022
Merged

Librem image shrink #1232

merged 2 commits into from
Dec 1, 2022

Conversation

Unb0rn
Copy link
Contributor

@Unb0rn Unb0rn commented Oct 23, 2022

This should resolve #1229
Tried not to break anything for any Librem machine, however I am able to test it only on Librem 14

Still need some input on some options:

CONFIG_X86_IOPL_IOPERM - legacy. Is there something in heads still using it?
CONFIG_ZONE_DMA - are there any devices on librem machines still using it?
CONFIG_ISA_DMA_API - isn't it ancient?
CONFIG_RSEQ
CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS - do we need it? I think its only needed for some pretty old specific HW
CONFIG_SYN_COOKIES=y - the kernel is almost network-less. Is it some kind of dependency?
CONFIG_PRINTK=y - will make the kernel mute, but do we care?
CONFIG_BUG=y
CONFIG_MULTIUSER=y - the question is are we using multiuser system or everything runs as root now?

The format is also switched to full config instead of defconfig

tlaurion added a commit to tlaurion/heads that referenced this pull request Oct 24, 2022
…at for CircleCI librem builds + review/testing
tlaurion added a commit to tlaurion/heads that referenced this pull request Oct 24, 2022
…at for CircleCI librem builds + review/testing

- Put back @BLOB_DIR@
# CONFIG_SYSFS_DEPRECATED is not set
CONFIG_RELAY=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_INITRAMFS_SOURCE="../../../blobs/dev.cpio"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be CONFIG_INITRAMFS_SOURCE="@BLOB_DIR@/dev.cpio"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed this one. Remained from Purism's Pureboot config

@tlaurion
Copy link
Collaborator

@Unb0rn replied to your questions under #1229 (comment)

@JonathonHall-Purism
Copy link
Collaborator

Hi @Unb0rn, thanks again for the work on this!

Could you please split this up into a few commits so it is easier to review? I'd suggest one commit converting to full-config (with no other changes), and another commit making the config changes we want. Anything along those lines is more reviewable though, right now it is hard to see what exactly was changed (i.e. I think BTRFS was enabled here too, which isn't directly related).

You can replace the existing config file, and then we'll get CI builds I can help test. Right now this doesn't affect anything because the Librem boards still reference the existing config.

Regarding the specific configs in the description, I mentioned some of those in #1229 but didn't catch all of them:

CONFIG_X86_IOPL_IOPERM - Should be OK AFAIK, we don't need to run any binaries not specifically built for Heads on this kernel
CONFIG_ZONE_DMA - I'm not aware of anything requiring this, but I'm not sure, probably just need to test it.
CONFIG_ISA_DMA_API - Might be needed for LPC devices, which includes the TPM on L1UM IIRC.
CONFIG_RSEQ - Sounds removable just from some reading about it, but I don't think it adds much to bzImage either
CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS - I don't think this is needed
CONFIG_SYN_COOKIES=y - I don't think this is needed
CONFIG_PRINTK=y - Keep this please for users that might encounter unexpected issues, for testing, etc.
CONFIG_BUG=y - Keep this too please, as above, I think it is risky to silently ignore the conditions BUG tests for
CONFIG_MULTIUSER=y - I'm OK with removing this as long as it works, i.e. nothing we ship in Heads depends on the syscalls removed by this, we can try it

@Unb0rn
Copy link
Contributor Author

Unb0rn commented Oct 26, 2022

@JonathonHall-Purism yeah, you were right. Updated defconfig just to build and test everything, removed BTRFS and exFAT as they're unrelated.
Regarding low-power subsystem - Isn't it disabled in current defconfig? Other than that, left printk() and bug() in place
One more question to you and @tlaurion
Found some more "bloat" parameters:

CONFIG_IOMMU_SUPPORT
VIRTIO_MENU
VHOST_MENU
CONFIG_PTP_1588_CLOCK

Do we need any of it in our minimal image?

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 2, 2022

One more question to you and @tlaurion Found some more "bloat" parameters:

CONFIG_IOMMU_SUPPORT
VIRTIO_MENU
VHOST_MENU
CONFIG_PTP_1588_CLOCK

Do we need any of it in our minimal image?

@Unb0rn I do not think so. @JonathonHall-Purism ?

# under linux decompressed+patched directory and put it back in git tree
# to check changes with git difftool
# useful for development cycle of linux kernel version bumps.
linux.modifydefconfig:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, I had been doing this manually!

@JonathonHall-Purism
Copy link
Collaborator

Agree, I don't think we need any of those.

@Unb0rn
Copy link
Contributor Author

Unb0rn commented Nov 5, 2022

Well, it looks I started celebrating a bit too early)
I shou;d've paid more attention - in fact what I thought to be a success was a failure with flashrom. It looks like it tries to detect if it is being run as root, and as we don't have those syscalls in non-multiuser kernel it fails with "Not implemented" error. So I think I will make several more tests and push the fixed version later

@Unb0rn
Copy link
Contributor Author

Unb0rn commented Nov 5, 2022

It turned out to be even more interesting - it had nothing to do with multiuser support, however flashrom still uses iopl() or ioperm(), so had to re-enable it. Double checked to be working.
VHOST_MENU, VIRTIO_MENU and PTP_1588 are also disabled, and regarding IOMMU aren't we using it at all for isolation?

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 5, 2022

No IOMMU device isolation under Heads.

@tlaurion
Copy link
Collaborator

  • EXTFAT_FS support will definitely be a global improvement for users: Thumb drives and ssd drives generally comes formatted like that. (currently missing)
  • As noted under Linux x230 cleanup remix #1184, I notice a lot of things built as modules which are currently not instructed to be packed under initrd, resulting in useless build time (no impact on size of kernel per se, but unneeded if packed as modules and not instructed to be added under Heads per heads global Makefile condition for that config entree defined under board config)
    • This is the case for a lot of crypto stuff
    • AES as module? Suprised it is not braking things, for example with cryptesetup-reencrypt where cryptsetup is instructed to be compiled intoto and using kernel crypto backend?
  • IOMMU: No isolation from Heads as said. Though from what I recall, this is setuped there and was needed to have i915 working (xx20 and xx30 here, not Librem), but my human memory might be tricking me. Would have to check what happens when disabled.

@JonathonHall-Purism
Copy link
Collaborator

@tlaurion @Unb0rn I tested this thoroughly on L1UM-1X8C and did some basic testing on Mini v2. Looks great, no problems at all, and saves a little over 500 KB on the payload. Let's merge it 💯

Thanks for the patience while this progressed through my priority list too 🙇

@tlaurion
Copy link
Collaborator

@JonathonHall-Purism points addressed under #1232 (comment) to be dealt separately?

@JonathonHall-Purism
Copy link
Collaborator

Yeah, I think they should be addressed separately. This is a good improvement, it didn't introduce those issues, and I don't see any reason to hold it waiting for other improvements.

Double checking that everything else has tracking issues:

@tlaurion
Copy link
Collaborator

tlaurion commented Dec 1, 2022

OK!

Note that dynamic debugging kernel options are removed from kernel from this point!

Merging!

@tlaurion tlaurion merged commit 61f72f8 into linuxboot:master Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Question: Getting rid of unneeded kernel options on Librem 14
3 participants