Re: [PATCH v6 07/14] x86: Secure Launch kernel early boot stub

From: Simon Horman
Date: Fri May 05 2023 - 15:47:12 EST


On Fri, May 05, 2023 at 02:58:28PM -0400, Ross Philipson wrote:
> On 5/5/23 13:47, Simon Horman wrote:
> > On Thu, May 04, 2023 at 02:50:16PM +0000, Ross Philipson wrote:

...

> > > +asmlinkage __visible void sl_check_region(void *base, u32 size)
> > > +{
> > > + sl_check_pmr_coverage(base, size, false);
> > > +}
> >
> > I'm a nit unsure, what to do here, but clang-16 with W=1 says the following.
> >
> > arch/x86/boot/compressed/sl_main.c:533:27: warning: no previous prototype for function 'sl_main' [-Wmissing-prototypes]
> > asmlinkage __visible void sl_main(void *bootparams)
> > ^
> > arch/x86/boot/compressed/sl_main.c:533:22: note: declare 'static' if the function is not intended to be used outside of this translation unit
> > asmlinkage __visible void sl_main(void *bootparams)
> > ^
> > static
>
> Yea we will have to look into why this is. This function is only ever called
> from asm code so that might have something to do with this.

Thanks.

...

> > > diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
> > > index 01d19fc..74e3e7df 100644
> > > --- a/arch/x86/include/uapi/asm/bootparam.h
> > > +++ b/arch/x86/include/uapi/asm/bootparam.h
> > > @@ -26,6 +26,7 @@
> > > /* loadflags */
> > > #define LOADED_HIGH (1<<0)
> > > #define KASLR_FLAG (1<<1)
> > > +#define SLAUNCH_FLAG (1<<2)
> > > #define QUIET_FLAG (1<<5)
> > > #define KEEP_SEGMENTS (1<<6)
> > > #define CAN_USE_HEAP (1<<7)
> >
> > nit: please consider using BIT()
>
> I am a little reluctant to change something like this in an existing header.
> It seems a bit out of scope for the patch set.

Yes, sorry for the noise on this one.
I agree that what you have is the best approach here.