Re: [PATCH 1/2] x86/efi: Simplify efi_arch_cpu() a little

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Jul 22, 2024 at 11:18:37AM +0100, Andrew Cooper wrote:
> Make the "no extended leaves" case fatal and remove one level of indentation.
> Defer the max-leaf aquisition until it is first used.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Acked-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>

> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> CC: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> CC: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> CC: Gene Bright <gene@xxxxxxxxxxxxx>
> ---
>  xen/arch/x86/efi/efi-boot.h | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
> index f282358435f1..4e4be7174751 100644
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -738,29 +738,30 @@ static void __init efi_arch_handle_module(const struct 
> file *file,
>  
>  static void __init efi_arch_cpu(void)
>  {
> -    uint32_t eax = cpuid_eax(0x80000000U);
> +    uint32_t eax;
>      uint32_t *caps = boot_cpu_data.x86_capability;
>  
>      boot_tsc_stamp = rdtsc();
>  
>      caps[FEATURESET_1c] = cpuid_ecx(1);
>  
> -    if ( (eax >> 16) == 0x8000 && eax > 0x80000000U )
> -    {
> -        caps[FEATURESET_e1d] = cpuid_edx(0x80000001U);
> +    eax = cpuid_eax(0x80000000U);
> +    if ( (eax >> 16) != 0x8000 || eax < 0x80000000U )
> +        blexit(L"In 64bit mode, but no extended CPUID leaves?!?");
>  
> -        /*
> -         * This check purposefully doesn't use cpu_has_nx because
> -         * cpu_has_nx bypasses the boot_cpu_data read if Xen was compiled
> -         * with CONFIG_REQUIRE_NX
> -         */
> -        if ( IS_ENABLED(CONFIG_REQUIRE_NX) &&
> -             !boot_cpu_has(X86_FEATURE_NX) )
> -            blexit(L"This build of Xen requires NX support");
> +    caps[FEATURESET_e1d] = cpuid_edx(0x80000001U);
>  
> -        if ( cpu_has_nx )
> -            trampoline_efer |= EFER_NXE;
> -    }
> +    /*
> +     * This check purposefully doesn't use cpu_has_nx because
> +     * cpu_has_nx bypasses the boot_cpu_data read if Xen was compiled
> +     * with CONFIG_REQUIRE_NX
> +     */
> +    if ( IS_ENABLED(CONFIG_REQUIRE_NX) &&
> +         !boot_cpu_has(X86_FEATURE_NX) )
> +        blexit(L"This build of Xen requires NX support");
> +
> +    if ( cpu_has_nx )
> +        trampoline_efer |= EFER_NXE;
>  }
>  
>  static void __init efi_arch_blexit(void)
> -- 
> 2.39.2
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature