On Tue, Nov 29, 2022 at 03:49:06PM +0000, Michael Kelley (LINUX) wrote:

But it turns out that AMD really has two fairly different schemes:
the C-bit scheme and the vTOM scheme.

Except it doesn't:

"In the VMSA of an SNP-active guest, the VIRTUAL_TOM field designates
a 2MB aligned guest physical address called the virtual top of memory.
When bit 1 (vTOM) of SEV_FEATURES is set in the VMSA of an SNP-active
VM, the VIRTUAL_TOM..."

So SEV_FEATURES[1] is vTOM and it is part of SNP.

Why do you keep harping on this being something else is beyond me...

I already pointed you to the patch which adds this along with the other
SEV_FEATURES.

The details of these two AMD schemes are pretty different. vTOM is
*not* just a minor option on the C-bit scheme. It's an either/or -- a
guest VM is either doing the C-bit scheme or the vTOM scheme, not some
combination. Linux code in coco/core.c could choose to treat C-bit and
vTOM as two sub-schemes under CC_VENDOR_AMD, but that makes the code a
bit messy because we end up with "if" statements to figure out whether
to do things the C-bit way or the vTOM way.

Are you saying that that:

	if (cc_vendor == CC_VENDOR_AMD &&
	    sev_features & MSR_AMD64_SNP_VTOM_ENABLED)

is messy? Why?

We will have to support vTOM sooner or later.

Or we could model the two AMD schemes as two different vendors,
which is what I'm suggesting.  Doing so recognizes that the two schemes
are fairly disjoint, and it makes the code cleaner.

How is that any different from the above check?

You *need* some sort of a check to differentiate between the two anyway.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette