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; +}