|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 4/4] x86/microcode: Support builtin CPU microcode
On 18.12.19 13:42, Jan Beulich wrote: On 18.12.2019 02:32, Eslam Elnikety wrote:+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Xen can bundle microcode updates within its image. This support is conditional +on the build configuration BUILTIN_UCODE being enabled. Builtin microcode is +useful to ensure that, by default, a minimum microcode patch level will be +applied to the underlying CPU. + +To use microcode updates available on the build system as builtin, +use BUILTIN_UCODE_DIR to refer to the directory containing the firmware updates +and specify the individual microcode patches via either BUILTIN_UCODE_AMD or +BUILTIN_UCODE_INTEL for AMD microcode or INTEL microcode, respectively. For +instance, the configuration below is suitable for a build system which has a +``/lib/firmware/`` directory which, in turn, includes the individual microcode +patches ``amd-ucode/microcode_amd_fam15h.bin``, ``intel-ucode/06-3a-09``, and +``intel-ucode/06-2f-02``. + + CONFIG_BUILTIN_UCODE=y + CONFIG_BUILTIN_UCODE_DIR="/lib/firmware/" + CONFIG_BUILTIN_UCODE_AMD="amd-ucode/microcode_amd_fam15h.bin" + CONFIG_BUILTIN_UCODE_INTEL="intel-ucode/06-3a-09 intel-ucode/06-2f-02"Rather than a blank as separator, the more conventional one on Unix and alike would be : I think. Of course ideally there wouldn't be any restriction at all on the characters usable here for file names. It would be great if there is a particular convention. The blank separator is aligned with Linux way of doing builtin microcode.
Any particular preference between the v1 vs v2 approach?
Maybe I am missing something. Are you asking if we can safely skip the bootstrap_map(NULL)? (Quoting your response on PATCH v2 2/4 "And of course we really want these mappings to be gone") If yes, then I think you want to get away without #ifdef here, by having a suitably placed #define ucode_builtin false somewhere up the file. Agreed. That will make the code snippet more readable indeed. --- /dev/null +++ b/xen/arch/x86/microcode/Makefile @@ -0,0 +1,46 @@ +# Copyright (C) 2019 Amazon.com, Inc. or its affiliates. +# Author: Eslam Elnikety <elnikety@xxxxxxxxxx> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 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 General Public License for more details. + +# Remove quotes and excess spaces from configuration strings +UCODE_DIR=$(strip $(subst $\",,$(CONFIG_BUILTIN_UCODE_DIR))) +UCODE_AMD=$(strip $(subst $\",,$(CONFIG_BUILTIN_UCODE_AMD))) +UCODE_INTEL=$(strip $(subst $\",,$(CONFIG_BUILTIN_UCODE_INTEL))) + +# AMD and INTEL microcode blobs. Use 'wildcard' to filter for existing blobs. +amd-blobs := $(wildcard $(addprefix $(UCODE_DIR),$(UCODE_AMD))) +intel-blobs := $(wildcard $(addprefix $(UCODE_DIR),$(UCODE_INTEL))) + +ifneq ($(amd-blobs),) +obj-y += ucode_amd.o +endif + +ifneq ($(intel-blobs),) +obj-y += ucode_intel.o +endif + +ifeq ($(amd-blobs)$(intel-blobs),) +obj-y += ucode_dummy.o +endif + +ucode_amd.o: Makefile $(amd-blobs) + cat $(amd-blobs) > $@.bin + $(OBJCOPY) -I binary -O elf64-x86-64 -B i386:x86-64 --rename-section .data=.builtin_amd_ucode,alloc,load,readonly,data,contents $@.bin $@ + rm -f $@.bin + +ucode_intel.o: Makefile $(intel-blobs) + cat $(intel-blobs) > $@.bin + $(OBJCOPY) -I binary -O elf64-x86-64 -B i386:x86-64 --rename-section .data=.builtin_intel_ucode,alloc,load,readonly,data,contents $@.bin $@ + rm -f $@.binThis can be had with a pattern rule (with the vendor being the stem) and hence without duplication, I think. Also - is simply concatenating the blobs reliable enough? There's no build time diagnostic that the result would actually be understood at runtime. Concatenation is reliable (as long as the individual microcode blobs are not malformed, and in that case the builtin is not making matters worse compared to presenting the malformed update via <integer> | scan). +ucode_dummy.o: Makefile + $(CC) $(CFLAGS) -c -x c /dev/null -o $@;Since the commit message doesn't explain why this is needed, I have to ask (I guess we somewhere have a dependency on $(obj-y) not being empty). Your guess is correct. All sub-directories of xen/arch/x86 are expected to produce built_in.o. If there are not amd nor intel microcode blobs, there will be no build dependencies and the build fails preparing the built_in.o _If_ it is needed, I don't see why you need ifeq() around its use. In fact you could have obj-y := ucode-dummy.o right at the top of the file. Furthermore I don't really understand why you need this in the first place. While cat won't do what you want with an empty argument list, can't you simply prepend / append /dev/null? To make sure we are on the same page. You are suggesting using "/dev/null" in case there are no amd/intel ucode to generate the ucode_amd/intel.o? If so, objcopy does not allow using /dev/null as input (complains about empty binary). (I agree with your other inline suggestions that I have omitted. Those I will address in v3). Thanks, Eslam _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |