Re: [PATCH v1 04/26] x86/sev: Add the host SEV-SNP initialization support

[Jeremi Piotrowski]

On 08/01/2024 18:04, Borislav Petkov wrote:
> On Mon, Jan 08, 2024 at 05:49:01PM +0100, Jeremi Piotrowski wrote:
>> What I wrote: "allow for the kernel to allocate the rmptable".
> 
> What?!
> 
> "15.36.5 Hypervisor RMP Management
> 
> ...
> 
> Because the RMP is initialized by the AMD-SP to prevent direct access to
> the RMP, the hypervisor must use the RMPUPDATE instruction to alter the
> entries of the RMP. RMPUPDATE allows the hypervisor to alter the
> Guest_Physical_Address, Assigned, Page_Size, Immutable, and ASID fields
> of an RMP entry."
>> What you want is something that you should keep far and away from the
> upstream kernel.
>

Can we please not assume I am acting in bad faith. I am explicitly trying to
integrate nicely with AMD's KVM SNP host patches to cover an additional usecase
and get something upstreamable.

Let's separate RMP allocation from who (and how) maintains the entries.

"""
15.36.4 Initializing the RMP
..
Software must program RMP_BASE and RMP_END identically for each core in the
system and before enabling SEV-SNP globally.
"""

KVM expects UEFI to do this, Hyper-V does the allocation itself (on bare-metal).
Both are valid. Afaik it is the SNP_INIT command that hands over control of the
RMP from software to AMD-SP.

When it comes to "who and how maintains the rmp" - that is of course the AMD-SP
and hypervisor issues RMPUPDATE instructions. The paragraph you cite talks about
the physical RMP and AMD-SP - not virtualized SNP (aka "SNP-host VM"/nested SNP).
AMD specified an MSR-based RMPUPDATE for us for that usecase (15.36.19 SEV-SNP
Instruction Virtualization). The RMP inside the SNP-host VM is not related to
the physical RMP and is an entirely software based construct.

The RMP in nested SNP is only used for kernel bookkeeping and so its allocation
is optional. KVM could do without reading the RMP directly altogether (by tracking
the assigned bit somewhere) but that would be a design change and I'd rather see
the KVM SNP host patches merged in their current shape. Which is why the patch
I linked allocates a (shadow) RMP from the kernel.

I would very much appreciate if we would not prevent that usecase from working -
that's why I've been reviewing and testing multiple revisions of these patches
and providing feedback all along.