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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 12 Aug 2022 09:51:41 -0500
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "Kirill A. Shutemov" <kirill@...temov.name>,
        "H. Peter Anvin" <hpa@...or.com>,
        Michael Roth <michael.roth@....com>,
        Joerg Roedel <jroedel@...e.de>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v2 1/2] x86/sev: Put PSC struct on the stack in prep for
 unaccepted memory support

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@amd.com/

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