[arch-projects] [mkinitcpio] [PATCH] Add 'microcode' hooks
Dave Reisner
d at falconindy.com
Mon Nov 3 13:44:16 UTC 2014
On Sun, Nov 02, 2014 at 02:18:00AM -0500, Weng Xuetian wrote:
> Add early microcode support with early cpio. All files added under
> /early_root will be created as a separate cpio prepend to original one.
> Add a early_cpio file to early cpio to detect whether cpio contains
> early cpio, thus we can have lsinitcpio work normally like old one.
> ---
Hi, I'm not opposed to this idea, but this patch has a long way to go
before it's in mergeable shape.
> .gitignore | 1 +
> Makefile | 9 +++-
> install/microcode | 24 +++++++++++
> lsinitcpio | 35 +++++++++++++---
> mkinitcpio | 13 +++++-
> skipcpio.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 198 insertions(+), 7 deletions(-)
> create mode 100644 install/microcode
> create mode 100644 skipcpio.c
>
> diff --git a/.gitignore b/.gitignore
> index a814a0f..1e501f0 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -4,3 +4,4 @@ lsinitcpio.1
> *~
> *.bak
> mkinitcpio-*.tar.gz
> +skipcpio
> diff --git a/Makefile b/Makefile
> index 5af0eb2..a3a3591 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -20,7 +20,10 @@ DIRS = \
> /usr/lib/systemd/system/shutdown.target.wants \
> /usr/lib/tmpfiles.d
>
> -all: doc
> +all: doc skipcpio
> +
> +skipcpio: skipcpio.c
> + $(CC) skipcpio.c -o skipcpio
Get rid of this compile command. You just want the dependency without
the build rule. Make's default rules for generating binaries from .c are
better than yours, as they'll add in CFLAGS and LDFLAGS from the
environment.
>
> MANPAGES = \
> man/mkinitcpio.8 \
> @@ -40,6 +43,7 @@ install: all
> < mkinitcpio > $(DESTDIR)/usr/bin/mkinitcpio
>
> sed -e 's|\(^_f_functions\)=.*|\1=/usr/lib/initcpio/functions|' \
> + -e 's|\(^_f_skipcpio\)=.*|\1=/usr/lib/initcpio/skipcpio|' \
> -e 's|%VERSION%|$(VERSION)|g' \
> < lsinitcpio > $(DESTDIR)/usr/bin/lsinitcpio
>
> @@ -67,6 +71,8 @@ install: all
> ln -s mkinitcpio $(DESTDIR)/usr/share/bash-completion/completions/lsinitcpio
> install -m644 shell/zsh-completion $(DESTDIR)/usr/share/zsh/site-functions/_mkinitcpio
>
> + install -m755 skipcpio $(DESTDIR)/usr/lib/initcpio/skipcpio
> +
> doc: $(MANPAGES)
> man/%: man/%.txt Makefile
> a2x -d manpage \
> @@ -76,6 +82,7 @@ man/%: man/%.txt Makefile
>
> clean:
> $(RM) mkinitcpio-${VERSION}.tar.gz $(MANPAGES)
> + $(RM) skipcpio
>
> dist: doc
> echo $(VERSION) > VERSION
> diff --git a/install/microcode b/install/microcode
> new file mode 100644
> index 0000000..4dfd69e
> --- /dev/null
> +++ b/install/microcode
> @@ -0,0 +1,24 @@
> +#!/bin/bash
> +
> +build() {
> + ucode_dir=(amd-ucode intel-ucode)
> + ucode_dest=(AuthenticAMD.bin GenuineIntel.bin)
> +
> + for idx in 0 1; do
> + fw=${ucode_dir[$idx]}
> + for fwpath in "${_d_firmware[@]}"; do
> + if [[ -d $fwpath/$fw ]]; then
> + add_dir '/early_root/kernel/x86/microcode'
> + cat $fwpath/$fw/* > $BUILDROOT/early_root/kernel/x86/microcode/${ucode_dest[$idx]}
I don't understand this loop. It adds both AMD and Intel firmware to the
image? Concatenates multiple files? Where do these files even come from?
I've found:
$ pkgfile -vr amd-ucode/
core/linux-firmware 20141009.0e5f637-1 /usr/lib/firmware/amd-ucode/microcode_amd.bin
core/linux-firmware 20141009.0e5f637-1 /usr/lib/firmware/amd-ucode/microcode_amd.bin.asc
core/linux-firmware 20141009.0e5f637-1 /usr/lib/firmware/amd-ucode/microcode_amd_fam15h.bin
core/linux-firmware 20141009.0e5f637-1 /usr/lib/firmware/amd-ucode/microcode_amd_fam15h.bin.asc
We really need all of this? Even the GPG signatures? What about Intel?
$ pkgfile -vr intel-ucode/
extra/intel-ucode 20140913-1 /usr/share/licenses/intel-ucode/LICENSE
Doesn't look too promising... I think you've missed a step somewhere.
> + fi
> + done
> + done
> +}
> +
> +help() {
> + cat <<HELPEOF
> +This hook generate early ucode update
> +HELPEOF
> +}
> +
> +# vim: set ft=sh ts=4 sw=4 et:
> diff --git a/lsinitcpio b/lsinitcpio
> index 9567b79..902bfe2 100755
> --- a/lsinitcpio
> +++ b/lsinitcpio
> @@ -6,8 +6,10 @@
> shopt -s extglob
>
> _list='--list'
> -_optcolor=1 _optverbose=
> +_optcolor=1 _optverbose= _optnoearly=1
> _f_functions=functions
> +_f_skipcpio=./skipcpio
> +_tmp_files=()
>
> usage() {
> cat<<USAGE
> @@ -21,6 +23,7 @@ usage: ${0##*/} [action] [options] <initramfs>
> -x, --extract extract image to disk
>
> Options:
> + -e, --early display early cpio if possible
> -h, --help display this help
> -n, --nocolor disable colorized output
> -V, --version display version information
> @@ -131,7 +134,8 @@ analyze_image() {
> local kernver ratio columns=$(tput cols) image=$1
>
> workdir=$(mktemp -d --tmpdir lsinitcpio.XXXXXX)
> - trap 'rm -rf "$workdir"' EXIT
> + _tmp_files+=("$workdir")
> + trap 'rm -rf "${_tmp_files[@]}"' EXIT
>
> # fallback in case tput failed us
> columns=${columns:-80}
> @@ -174,7 +178,7 @@ analyze_image() {
> explicitmod=($MODULES)
>
> # print results
> - imagename=$_image
> + imagename=$_origimage
> [[ -L $_image ]] && imagename+=" -> $(readlink -e "$_image")"
> msg 'Image: %s %s' "$imagename"
> [[ $version ]] && msg 'Created with mkinitcpio %s' "$version"
> @@ -225,10 +229,18 @@ analyze_image() {
> printf ' %s\n' $CLEANUPHOOKS
> printf '\n'
> fi
> +
> + msg 'Contains early CPIO:'
> + if (( _hasearly )); then
> + printf ' yes\n'
> + else
> + printf ' no\n'
> + fi
> + printf '\n'
> }
>
> -_opt_short='achlnVvx'
> -_opt_long=('analyze' 'config' 'help' 'list' 'nocolor' 'version' 'verbose' 'extract')
> +_opt_short='achlenVvx'
> +_opt_long=('analyze' 'config' 'help' 'list' 'early' 'nocolor' 'version' 'verbose' 'extract')
>
> parseopts "$_opt_short" "${_opt_long[@]}" -- "$@" || exit
> set -- "${OPTRET[@]}"
> @@ -245,6 +257,8 @@ while :; do
> exit 0 ;;
> -l|--list)
> _optlistcontents=1 ;;
> + -e|--early)
> + _optnoearly=0 ;;
This flag is really confusing. The name of the flag is "early", but
passing it seems to turn detection of the early initramfs image OFF
(contrary to the description given by the usage). Why can't we avoid the
flag and just do the right thing all the time? No one's going to care
about extracting or listing the early firmware. If they really do, they
can just use bsdtar/bsdcpio directly on the image.
> -n|--nocolor)
> _optcolor=0 ;;
> -V|--version)
> @@ -262,6 +276,8 @@ while :; do
> done
>
> _image=$1
> +_origimage=$1
> +_hasearly=0
>
> if [[ -t 1 ]] && (( _optcolor )); then
> try_enable_color
> @@ -281,6 +297,15 @@ esac
> # read compression type
> _compress=$(detect_filetype "$_image") || exit
Because you leave this alone, an initramfs with firmware prepended will
now always show up as uncompressed, regardless of any compression used
in the real image.
>
> +if (( _optnoearly )) && decomp "$_image" | bsdcpio -i --quiet --list | grep -q '^early_cpio'; then
This would be just as effective if you checked the very firsy entry in
"$_image" for "/kernel/x86/microcode/...". As the first file in an
initramfs, this will fail to extract properly (as the parent paths don't
exist). So, there's a high probability that we're finding some early
firmware. Bonus: no need for the ugly-ass flag file. Additional bonus:
since you'll have the path (after matching it), you can grab the name of
the firmware and display it in -a output.
> + _hasearly=1
> + _tmp_image=$(mktemp --tmpdir lsinitcpio.XXXXXX)
> + _tmp_files+=("$_tmp_image")
This is no "temporary" image. This is the *real* initramfs.
> + trap 'rm -rf "${_tmp_files[@]}"' EXIT
You also set this trap in analyze_image. It's a very generic thing that
you're doing -- just move it to a common path and call it once.
> + $_f_skipcpio $_image > $_tmp_image
Needs more quoting...
> + _image=$_tmp_image
> +fi
> +
> if (( _optanalyze )); then
> analyze_image "$_image"
> elif (( _optshowconfig )); then
> diff --git a/mkinitcpio b/mkinitcpio
> index 01fe8ba..cfe5747 100755
> --- a/mkinitcpio
> +++ b/mkinitcpio
> @@ -221,10 +221,21 @@ build_image() {
> cpio_opts+=('-R' '0:0')
> fi
>
> + echo -n > "$out"
If your goal is to truncate "$out", then all you need to do is:
>"$out"
> + if [[ -d "$BUILDROOT/early_root" ]]; then
> + pushd "$BUILDROOT/early_root" >/dev/null
> + touch "$BUILDROOT/early_root/early_cpio"
> + find . -mindepth 1 -printf "%P\0" |
> + LANG=C bsdcpio "${cpio_opts[@]}" >> "$out"
No error checking?
> + popd >/dev/null
> +
> + rm -rf "$BUILDROOT/early_root"
Removing this directory now makes debugging artificially difficult. Do
we really need to invoke bsdcpio here? It's a single uncompressed file.
We could just write the CPIO header ourselves as a preamble with
printf...
> + fi
> +
> pushd "$BUILDROOT" >/dev/null
> find . -print0 |
> LANG=C bsdcpio "${cpio_opts[@]}" |
> - $compress $COMPRESSION_OPTIONS > "$out"
> + $compress $COMPRESSION_OPTIONS >> "$out"
> pipesave=("${PIPESTATUS[@]}") # save immediately
> popd >/dev/null
>
> diff --git a/skipcpio.c b/skipcpio.c
> new file mode 100644
> index 0000000..445d7f6
> --- /dev/null
> +++ b/skipcpio.c
I really dislike the idea of adding architecture specific code to
mkinitcpio. I've resisted it several times. I need a really good reason
to add someone else's unruly C to mkinitcpio...
> @@ -0,0 +1,123 @@
> +/* dracut-install.c -- install files and executables
> +
> + Copyright (C) 2012 Harald Hoyer
> + Copyright (C) 2012 Red Hat, Inc. All rights reserved.
> +
> + This program is free software: you can redistribute it and/or modify
> + under the terms of the GNU Lesser General Public License as published by
> + the Free Software Foundation; either version 2.1 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful, but
> + WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public License
> + along with this program; If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +#define PROGRAM_VERSION_STRING "1"
> +
> +#ifndef _GNU_SOURCE
> +#define _GNU_SOURCE
> +#endif
> +
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#define CPIO_END "TRAILER!!!"
> +#define CPIO_ENDLEN (sizeof(CPIO_END)-1)
> +
> +static char buf[CPIO_ENDLEN * 2 + 1];
> +
> +int main(int argc, char **argv)
> +{
> + FILE *f;
> + size_t s;
> +
> + if (argc != 2) {
> + fprintf(stderr, "Usage: %s <file>\n", argv[0]);
> + exit(1);
> + }
> +
> + f = fopen(argv[1], "r");
> +
> + if (f == NULL) {
> + fprintf(stderr, "Cannot open file '%s'\n", argv[1]);
> + exit(1);
> + }
> +
> + s = fread(buf, 6, 1, f);
> + if (s <= 0) {
> + fprintf(stderr, "Read error from file '%s'\n", argv[1]);
> + fclose(f);
> + exit(1);
> + }
> + fseek(f, 0, SEEK_SET);
> +
> + /* check, if this is a cpio archive */
> + if ((buf[0] == 0x71 && buf[1] == 0xc7)
> + || (buf[0] == '0' && buf[1] == '7' && buf[2] == '0' && buf[3] == '7' && buf[4] == '0' && buf[5] == '1')) {
> + long pos = 0;
> +
> + /* Search for CPIO_END */
> + do {
> + char *h;
> + fseek(f, pos, SEEK_SET);
> + buf[sizeof(buf) - 1] = 0;
> + s = fread(buf, CPIO_ENDLEN, 2, f);
> + if (s <= 0)
> + break;
> +
> + h = strstr(buf, CPIO_END);
> + if (h) {
> + pos = (h - buf) + pos + CPIO_ENDLEN;
> + fseek(f, pos, SEEK_SET);
> + break;
> + }
> + pos += CPIO_ENDLEN;
> + } while (!feof(f));
> +
> + if (feof(f)) {
> + /* CPIO_END not found, just cat the whole file */
> + fseek(f, 0, SEEK_SET);
> + } else {
> + /* skip zeros */
> + while (!feof(f)) {
> + size_t i;
> +
> + buf[sizeof(buf) - 1] = 0;
> + s = fread(buf, 1, sizeof(buf) - 1, f);
> + if (s <= 0)
> + break;
> +
> + for (i = 0; (i < s) && (buf[i] == 0); i++) ;
> +
> + if (buf[i] != 0) {
> + pos += i;
> + fseek(f, pos, SEEK_SET);
> + break;
> + }
> +
> + pos += s;
> + }
> + }
> + }
> + /* cat out the rest */
> + while (!feof(f)) {
> + s = fread(buf, 1, sizeof(buf), f);
> + if (s <= 0)
> + break;
> +
> + s = fwrite(buf, 1, s, stdout);
> + if (s <= 0)
> + break;
> + }
> + fclose(f);
> +
> + return EXIT_SUCCESS;
> +}
> --
> 2.0.2
More information about the arch-projects
mailing list