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

David Woodhouse
2023-03-27T18:15:18Z

[-- Attachment #1: Type: text/plain, Size: 2235 bytes --]

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.

> 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.)

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]