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
