On Tue, Sep 09, 2025 at 04:09:37PM +0800, Gary Lin via Grub-devel wrote:
> Implement the necessary functions to dynamically enable SSE and AVX
> on x86_64 EFI systems when the hardware is capable.
>
> Signed-off-by: Gary Lin <[email protected]>
> ---
> grub-core/Makefile.core.def | 1 +
> grub-core/lib/hwfeatures-gcry.c | 9 +
> grub-core/lib/x86_64/efi/hwfeatures-gcry.c | 244 +++++++++++++++++++++
> include/grub/x86_64/cpuid.h | 1 +
> include/grub/x86_64/efi/hwfeatures-gcry.h | 25 +++
> 5 files changed, 280 insertions(+)
> create mode 100644 grub-core/lib/x86_64/efi/hwfeatures-gcry.c
> create mode 100644 include/grub/x86_64/cpuid.h
> create mode 100644 include/grub/x86_64/efi/hwfeatures-gcry.h
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 9b8360c98..df7fc5326 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1686,6 +1686,7 @@ module = {
> name = crypto;
> common = lib/crypto.c;
> common = lib/hwfeatures-gcry.c;
> + x86_64_efi = lib/x86_64/efi/hwfeatures-gcry.c;
>
> extra_dist = lib/libgcrypt-grub/cipher/crypto.lst;
> };
> diff --git a/grub-core/lib/hwfeatures-gcry.c b/grub-core/lib/hwfeatures-gcry.c
> index 652e67c43..acfa7be33 100644
> --- a/grub-core/lib/hwfeatures-gcry.c
> +++ b/grub-core/lib/hwfeatures-gcry.c
> @@ -19,6 +19,9 @@
> #include <grub/dl.h>
>
> #include <grub/hwfeatures-gcry.h>
> +#if defined (__x86_64__) && defined (GRUB_MACHINE_EFI)
> +#include <grub/x86_64/efi/hwfeatures-gcry.h>
> +#endif
>
> GRUB_MOD_LICENSE ("GPLv3+");
>
> @@ -33,11 +36,17 @@ grub_gcry_hwf_enabled (void)
> void
> grub_enable_gcry_hwf (void)
> {
> +#if defined (__x86_64__) && defined (GRUB_MACHINE_EFI)
> + grub_enable_gcry_hwf_x86_64_efi ();
> +#endif
> __gcry_use_hwf = true;
> }
>
> void
> grub_reset_gcry_hwf (void)
> {
> +#if defined (__x86_64__) && defined (GRUB_MACHINE_EFI)
> + grub_reset_gcry_hwf_x86_64_efi ();
> +#endif
> __gcry_use_hwf = false;
> }
> diff --git a/grub-core/lib/x86_64/efi/hwfeatures-gcry.c
> b/grub-core/lib/x86_64/efi/hwfeatures-gcry.c
> new file mode 100644
> index 000000000..8e66c1d13
> --- /dev/null
> +++ b/grub-core/lib/x86_64/efi/hwfeatures-gcry.c
> @@ -0,0 +1,244 @@
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2025 Free Software Foundation, Inc.
> + *
> + * GRUB 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 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <grub/types.h>
> +#include <grub/x86_64/efi/hwfeatures-gcry.h>
> +#include <grub/x86_64/cpuid.h>
> +#include <grub/misc.h>
> +
> +/*
> + * Older versions of GCC may reorder the inline asm, which can lead to
> + * unexpected behavior when reading the Control Registers. The __FORCE_ORDER
> + * macro is used to prevent this.
> + *
> + * Ref:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=aa5cacdc29d76a005cbbee018a47faa6e724dd2d
> + */
> +#define __FORCE_ORDER "m"(*(unsigned int *)0x1000UL)
Missing space before 0x1000UL...
> +#define HW_FEATURE_X86_64_SSE (1 << 0)
> +#define HW_FEATURE_X86_64_AVX (1 << 1)
> +
> +static grub_uint32_t hw_features = 0;
> +static grub_uint64_t old_cr0, old_cr4, old_xcr0;
> +
> +static inline grub_uint64_t
I am not entirely sure why you force inlining here and there. I think
mostly it is not needed and compiler should be free to do (almost) any
optimization which is needed...
> +read_cr0 (void)
> +{
> + grub_uint64_t val;
Missing empty line here...
> + asm volatile ("mov %%cr0, %0" : "=r" (val) : __FORCE_ORDER);
> + return val;
> +}
> +
> +static inline grub_uint64_t
> +read_cr4 (void)
> +{
> + grub_uint64_t val;
Ditto...
> + asm volatile ("mov %%cr4,%0" : "=r" (val) : __FORCE_ORDER);
> + return val;
> +}
Anyway, in general I am OK with the patch set except these minor issues
mentioned above. If you fix them you can add my RB to all patches.
Daniel
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
