RE: [RFC PATCH v1 2/6] x86/sev: Add support for NestedVirtSnpMsr

[Michael Kelley (LINUX)]

From: Jeremi Piotrowski <jpiotrowski@xxxxxxxxxxxxxxxxxxx> Sent: Monday, January 30, 2023 7:26 AM
> 
> On Sat, Jan 28, 2023 at 07:48:27PM +0000, Michael Kelley (LINUX) wrote:
> > From: Jeremi Piotrowski <jpiotrowski@xxxxxxxxxxxxxxxxxxx> Sent: Monday, January
> 23, 2023 8:51 AM
> > >
> > > The rmpupdate and psmash instructions, which are used in AMD's SEV-SNP
> > > to update the RMP (Reverse Map) table, can't be trapped. For nested
> > > scenarios, AMD defined MSR versions of these instructions which can be
> >
> > s/can be/must be/  ??
> >
> 
> yes indeed
> 
> > > emulated by the top-level hypervisor. One instance where these MSRs are
> >
> > And by "top-level", I think you are referring the hypervisor running at L1, right?
> > Using the L0/L1/L2 terminology would probably help make the description
> > more precise.
> 
> These instructions are called by the L1 hypervisor and are emulated by the L0
> hypervisor which controls the actual rmp table. I'll rephrase the commit
> message to make that clearer.
> 

[snip]

> > > +
> > > +static u64 virt_psmash(u64 paddr)
> > > +{
> > > +	int ret;
> > > +
> > > +	asm volatile(
> > > +		"wrmsr\n\t"
> > > +		: "=a"(ret)
> > > +		: "a"(paddr), "c"(MSR_AMD64_VIRT_PSMASH)
> > > +		: "memory", "cc"
> > > +	);
> > > +	return ret;
> > > +}
> >
> > From checking the AMD spec, I can see that the above use
> > of wrmsr is non-conventional.  Could you capture the basics
> > of the usage paradigm in a comment?  I.e., the expected
> > inputs and outputs, and the core assumption that the
> > MSR isn't implemented in hardware, but must trap
> > to the hypervisor.
> 
> ok, how does this sound:
> 
> /*
>  * This version of rmpupdate is not implemented in hardware but always
>  * traps to L0 hypervisor. It doesn't follow usual wrmsr conventions.
>  * Inputs:
>  *   rax: 4KB aligned GPA
>  *   rdx: bytes 7:0 of new rmp entry
>  *   r8:  bytes 15:8 of new rmp entry
>  * Outputs:
>  *   rax: rmpupdate return code
>  */
> 
> and similar for psmash.
> 

Yes, that works for me.

Michael