RE: [Patch v3 07/14] x86/hyperv: Change vTOM handling to use standard coco mechanisms

[Michael Kelley (LINUX)]

From: Borislav Petkov <bp@xxxxxxxxx> Sent: Monday, November 21, 2022 7:03 AM
> 
> On Wed, Nov 16, 2022 at 10:41:30AM -0800, Michael Kelley wrote:
> ...
> 
> > @@ -108,6 +115,7 @@ u64 cc_mkenc(u64 val)
> >  	switch (vendor) {
> >  	case CC_VENDOR_AMD:
> >  		return val | cc_mask;
> > +	case CC_VENDOR_HYPERV:
> >  	case CC_VENDOR_INTEL:
> >  		return val & ~cc_mask;
> >  	default:
> > @@ -121,6 +129,7 @@ u64 cc_mkdec(u64 val)
> >  	switch (vendor) {
> >  	case CC_VENDOR_AMD:
> >  		return val & ~cc_mask;
> > +	case CC_VENDOR_HYPERV:
> >  	case CC_VENDOR_INTEL:
> >  		return val | cc_mask;
> >  	default:
> 
> Uuuh, this needs a BIG FAT COMMENT.
> 
> You're running on SNP and yet the enc/dec meaning is flipped. And that's
> because of vTOM.
> 
> What happens if you have other types of SNP-based VMs on HyperV? The
> isolation VMs thing? Or is that the same?
> 
> What happens when you do TDX guests with HyperV?
> 
> This becomes wrong then.
> 
> I think you need a more finer-grained check here in the sense of "is it
> a HyperV guest using a paravisor and vTOM is enabled" or so.
> 
> Otherwise, I like the removal of the HyperV-specific checks ofc.
> 

I think the core problem here is the naming and meaning of
CC_VENDOR_HYPERV. The name was created originally when the
Hyper-V vTOM handling code was a lot of special cases.   With the
changes in this patch series that make the vTOM functionality more
mainstream, the name would be better as CC_VENDOR_AMD_VTOM.
vTOM is part of the AMD SEV-SNP spec, and it's a different way of
doing the encryption from the "C-bit" based approach.  As much as
possible, I'm trying to not make it be Hyper-V specific, though currently
we have N=1 for hypervisors that offer the vTOM option, so it's a little
hard to generalize.

With the thinking oriented that way, a Linux guest on Hyper-V using
TDX will run with CC_VENDOR_INTEL.  A Linux guest on Hyper-V that
is fully enlightened to use the "C-bit" will run with CC_VENDOR_AMD.

Dexuan Cui just posted a patch set for initial TDX support on Hyper-V,
and I think that runs with CC_VENDOR_INTEL (Dexuan -- correct me if
I'm wrong about that -- I haven't reviewed your patches yet).  Tianyu Lan
has a patch set out for Hyper-V guests using the "C-bit".  That patch set
still uses CC_VENDOR_HYPERV.  Tianyu and I need to work through
whether his patch set can run with CC_VENDOR_AMD like everyone
else using the "C-bit" approach.

Yes, the polarity of the AMD vTOM bit matches the polarity of the
TDX GPA.SHARED bit, and is the opposite polarity of the AMD "C-bit".
I'll add a comment to that effect.

Anyway, that's where I think this should go. Does it make sense?
Other thoughts?

Michael