[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v1 2/4] x86/microcode: Improve parsing for ucode=

Thanks for getting the other patches in the series onto master, Jan.

This is the only patch out of this series that did not make it through, so I keeping my comments here.

On 23.01.20 11:26, Jan Beulich wrote:
On 22.01.2020 23:30, Eslam Elnikety wrote:
Decouple the microcode indexing mechanism when using GRUB to that
when using EFI. This allows us to avoid the "unspecified effect" of
using `<integer>` when booting via EFI.

Personally I'd prefer us to continue call this unspecified. It
simply shouldn't be used. People shouldn't rely on it getting
ignored. (Iirc, despite this being v1, you had previously
submitted a patch to this effect, with me replaying about the

With that, Xen can explicitly ignore that option when using EFI.

Don't we do so already, by way of the "if ( !ucode_mod_forced )"
you delete?

Not quite. If cfg.efi does not specify "ucode=<filename>", then a `ucode=<integer>` from the commandline gets parsed, resulting in the "unspecified" behaviour. Here, the ucode_mod_idx will hold the <integer> which will be used as index into the modules prepared in whatever order the efi/boot.c defines.

In any case, let me know (and others too) if, later on, you may want to favor the explicitly ignore (opposed to unspecified) semantic and I will be happy to send another revision of this patch while addressing your comment below.

--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2147,9 +2147,9 @@ for updating CPU micrcode. When negative, counting starts 
at the end of
  the modules in the GrUB entry (so with the blob commonly being last,
  one could specify `ucode=-1`). Note that the value of zero is not valid
  here (entry zero, i.e. the first module, is always the Dom0 kernel
-image). Note further that use of this option has an unspecified effect
-when used with xen.efi (there the concept of modules doesn't exist, and
-the blob gets specified via the `ucode=<filename>` config file/section
+image). This option should be used only with legacy boot, as it is explicitly
+ignored in EFI boot. When booting via EFI, the microcode update blob for
+early loading can be specified via the `ucode=<filename>` config file/section
  entry; see [EFI configuration file description](efi.html)).

Just like in patch 1, please distinguish "EFI boot" in general from
"use of xen.efi" (relevant here of course only if indeed we went
with this patch).


You have mentioned this very same remark on the other patch too. My understanding is that "EFI boot" may be ambiguous between (a) we are actually booting from xen.efi or (b) a system with EFI support (and the latter may still be falling onto grub for booting). Let me know if that's not actually what your remark is about.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.