Re: [PATCH v2 1/2] x86/sev: Put PSC struct on the stack in prep for unaccepted memory support

[Tom Lendacky]

On 8/12/22 09:33, Borislav Petkov wrote:
> On Fri, Aug 12, 2022 at 09:11:25AM -0500, Tom Lendacky wrote:
> > There was a whole discussion on this
>
> Pointer to it?

It starts here: 
https://lore.kernel.org/lkml/658c455c40e8950cb046dd885dd19dc1c52d060a.1659103274.git.thomas.lendacky@xxxxxxx/

> > and I would prefer to keep the ability to parallelize PSC without
> > locking.
>
> So smaller, on-stack PSC but lockless is still better than a bigger one
> but with synchronized accesses to it?
>
> > Well when we don't know which GHCB is in use, using that reserved area in
> > the GHCB doesn't help.
>
> What do you mean?
>
> The one which you read with
>
> 	data = this_cpu_read(runtime_data);

Memory acceptance is called before the per-CPU GHCBs have been allocated
and so you would be actually be using early boot GHCB. And that is decided
based on the #VC handler that is invoked - but in this case we're not
coming through the #VC handler to accept memory.

> in snp_register_per_cpu_ghcb() is the one you register.
>
> > Also, I don't want to update the GHCB specification for a single bit
> > that is only required because of the way Linux went about establishing
> > the GHCB usage.
>
> Linux?
>
> You mean, you did it this way: 885689e47dfa1499b756a07237eb645234d93cf9
>
> :-)

Well Joerg re-worked all that quite a bit. And with the SNP support, the
added requirement of registering the GHCB changed which GHCB could be
used. So even when the per-CPU GHCB is allocated, it can't be used until
it is registered, which depends on when the #VC handler is changed from
the boot #VC handler to the runtime #VC handler.

> "The runtime handler needs one GHCB per-CPU. Set them up and map them
> unencrypted."
>
> Why does that handler need one GHCB per CPU?

Each vCPU can be handling a #VC and you don't want to be serializing on a
single GHCB.

Thanks,
Tom

> As to the field, I was thinking along the lines of
>
> 	struct ghcb.vendor_flags
>
> field which each virt vendor can use however they like.
>
> It might be overkill but a random bool ain't pretty either. Especially
> if those things start getting added for all kinds of other things.
>
> If anything, you could make this a single u64 sev_flags which can at
> least collect all that gunk in one variable ... at least...
>
> Thx.