On Thu, Oct 16, 2025 at 06:40:14PM +0200, Daniel Kiper wrote:
> On Thu, Oct 16, 2025 at 09:41:11AM +0800, Gary Lin wrote:
> > On Wed, Oct 15, 2025 at 04:42:11PM +0200, Daniel Kiper wrote:
> > > 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...
> > >
> > Will fix it in v2.
> >
> > > > +#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...
> > >
> > Setting the control registers wrongly may cause CPU execption and crash
> > the system immediately. For saftey, I followed Linux kernel to use
> > inline and memory barrier to ensure the order of the asm insn.
> 
> These functions are inlined because they are in a header file in the
> Linux kernel. And it looks reading/writing cr0 and cr4 does not require
> inlining contrary to, e.g., cr2 which are tagged with __always_inline.
> So, I would drop "inline" from all these functions.
> 
I see. Then I'm fine to drop all 'inline' from those functions.

> [...]
> 
> > BTW, is there any chance to get my PBKDF2 patches reviewed?
> > https://lore.kernel.org/grub-devel/[email protected]/
> >
> > It's a small patch set and reduces the decryption time hugely.
> >
> > Combining all optimization patches (PBKDF2, HW accel, and the memcpy()
> > patch in the Argon2 patch set), the decryption time of my testing PBKDF2
> > key slot reduces from 15 second to 1 second. It will be a huge
> > performance gain for 2.14.
> 
> I will take a look...
> 
Thanks!

Gary Lin

_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to