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

[Simon Horman <horms@kernel.org>]

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.