Re: [PATCH v16 8/8] x86/smpboot: Allow parallel bringup for SEV-ES

[Tom Lendacky]

On 3/27/23 13:14, David Woodhouse wrote:
> On Mon, 2023-03-27 at 19:47 +0200, Borislav Petkov wrote:
> > > Making sure that the stack protector is either disabled or properly
> > > set up, and disabling any instrumentation/profiling/debug crap that
> > > isn't initialized yet.
> >
> > Lemme dump brain of what Tom and I were talking about today so that it
> > is documented somewhere.
> >
> > * re: stack protector: I was thinking to mark this function
> >
> >   __attribute__((no_stack_protector))
> >
> > but gcc added the function attribute way later:
> >
> > ~/src/gcc/gcc.git> git tag --contains 346b302d09c1e6db56d9fe69048acb32fbb97845
> > basepoints/gcc-12
> > basepoints/gcc-13
> > releases/gcc-11.1.0
> > releases/gcc-11.2.0
> > releases/gcc-11.3.0
> > releases/gcc-12.1.0
> > releases/gcc-12.2.0
> >
> > which means, that function would have to live somewhere in a file which
> > has stack protector disabled. One possible place would be
> > arch/x86/mm/mem_encrypt_identity.c which is kinda related.
>
> Shouldn't the rest of head64.c have the stack protector disabled, for
> similar reasons?
>
> > * re: stack: in order to be able to call a C function that early, we'd
> > have to put the VA of the initial stack back into %rsp as we switch
> > pagetables a bit earlier in there (thx Tom).
>
> Hm, don't you have a stack at the point you added that call? I thought
> you did? It doesn't have to be *the* stack for the AP in question.
> Just "a" stack. And you have the lock on the real-mode one that you're
> using.

Unfortunately RSP has the identity mapped stack value and when the 
pagetable switch is performed the mapping to that stack is lost. It would 
need to be updated to the equivalent of __va(RSP) to get a stack that can 
be used without page faulting.

Thanks,
Tom

> > So by then, doing all that cargo-cult just in order to not have a bunch
> > of lines in asm doesn't sound all that great anymore.
> >
> > * The __head per-function attribute is easily solved by lifting the
> > __head define into a common header.
> >
> > So meh, dunno. I guess we can do the asm thing for now, until a cleaner
> > solution without too many warts presents itself.
>
> Hm, doesn't most of that just go away (or at least become "Already
> Broken; Someone Else's Problem™") if you just concede to put your new C
> function into head64.c along with a whole bunch of other existing
> CONFIG_AMD_MEM_ENCRYPT support?
>
> (We still have to fix it if it's Someone Else's Problem, of course.
> It's just that you don't have to count that complexity towards your own
> part.)