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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.01.2020 20:34, Eslam Elnikety wrote:
> 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
>> same.)
>>
>>> 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.

I guess I see now what you mean, but I'm still unconvinced it needs
"fixing". After all - how is this different from "ucode=<N>" used
with the wrong number in a xen.gz boot? In fact I'm also unconvinced
the behavior of microcode_grab_module() to fall back as if
"ucode=scan" was specified, in case something bogus was found with
the specified number.

>>> --- 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.

Well, booting from EFI can be either via xen.efi, or via xen.gz's
efi_multiboot2() kind-of-entry-point. Yet the distinction discussed
is strictly between xen.efi and xen.gz.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel