[arch-projects] [mkinitcpio] [PATCH] Add 'microcode' hooks
Weng Xuetian
wengxt at gmail.com
Mon Nov 3 15:00:41 UTC 2014
On Monday 03 November 2014 08:44:16, Dave Reisner Wrote：
> 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:
The intel-ucode you need is the old version, which put all intel-ucode file
into /usr/lib/firmware/intel-ucode, microcode_ctl can be used. I may check how
fedora package amd-ucode files.
The implementation idea comes from dracut, because I'm totally opposed to the
current intel-ucode.img way to do it. Well, if mkinitcpio must depends on how
arch (I'm not a arch user) packages intel-ucode file, it can be changed.
But somehow generating early_cpio is useful not only for amd/intel ucode
update, but also some acpi update.
>
> $ 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.
Well, personally I don't know which idea is better, but maybe merge all cpio
file together? It's also useful to check whether merged cpio file contains
correct files.
There are some way to do this:
1. Having no ability to list the early cpio.
- well, if call bsdcpio is what we want.
2. List them together in the same output
- do we need a header for each part? this might break compatibility for
people who use lsinitcpio in script
3. current way.
>
> > -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.
Nope, early_cpio can be also used in some other way. Though currently we only
support microcode, but we might also want to add acpi override:
http://git.kernel.org/cgit/boot/dracut/dracut.git/tree/dracut.sh#n1562
>
> > + _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...
Initially I'd like to use a separate directory in buildroot, but later found
that might not work well with existing functions so I tried to reuse the
functions but make a convention that early_root is used for separate thing.
There can be multiple files in different directory, I didn't make the
assumption that this feature can only be used for microcode update.
>
> > + 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...
Well, I use C here because there is some existing code to reuse, and I did
aware mkinitcpio has no C code, the implementation of this C is simple so we
can reimplement is with shell script.
>
> > @@ -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;
> > +}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.archlinux.org/pipermail/arch-projects/attachments/20141103/a1c1e54b/attachment-0001.bin>
More information about the arch-projects
mailing list