Re: [PATCH v6 06/13] x86/hyperv: Change vTOM handling to use standard coco mechanisms
From: Borislav Petkov <bp@alien8.de>
Date: 2023-03-20 11:23:41
Also in:
linux-arch, linux-iommu, linux-pci, lkml, netdev
On Wed, Mar 08, 2023 at 06:40:07PM -0800, Michael Kelley wrote:
quoted hunk ↗ jump to hunk
diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c index 49b44f8..d1c3306 100644 --- a/arch/x86/coco/core.c +++ b/arch/x86/coco/core.c@@ -88,8 +106,6 @@ bool cc_platform_has(enum cc_attr attr) return amd_cc_platform_has(attr); case CC_VENDOR_INTEL: return intel_cc_platform_has(attr); - case CC_VENDOR_HYPERV: - return hyperv_cc_platform_has(attr); default: return false; }@@ -103,11 +119,14 @@ u64 cc_mkenc(u64 val) * encryption status of the page. * * - for AMD, bit *set* means the page is encrypted - * - for Intel *clear* means encrypted. + * - for AMD with vTOM and for Intel, *clear* means encrypted */ switch (vendor) { case CC_VENDOR_AMD: - return val | cc_mask; + if (sev_status & MSR_AMD64_SNP_VTOM) + return val & ~cc_mask;
This is silly. It should simply be: if (sev_status & MSR_AMD64_SNP_VTOM) return val;
quoted hunk ↗ jump to hunk
+ else + return val | cc_mask; case CC_VENDOR_INTEL: return val & ~cc_mask; default:@@ -120,7 +139,10 @@ u64 cc_mkdec(u64 val) /* See comment in cc_mkenc() */ switch (vendor) { case CC_VENDOR_AMD: - return val & ~cc_mask; + if (sev_status & MSR_AMD64_SNP_VTOM) + return val | cc_mask;
So if you set the C-bit, that doesn't make it decrypted on AMD. cc_mask on VTOM is 0 so why even bother? Same as the above.
+ else + return val & ~cc_mask; case CC_VENDOR_INTEL: return val | cc_mask; default:
...
+void __init hv_vtom_init(void)
+{
+ /*
+ * By design, a VM using vTOM doesn't see the SEV setting,
+ * so SEV initialization is bypassed and sev_status isn't set.
+ * Set it here to indicate a vTOM VM.
+ */This looks like a hack. The SEV status MSR cannot be intercepted so the guest should see vTOM. How are you running vTOM without setting it even up?!
+ sev_status = MSR_AMD64_SNP_VTOM; + cc_set_vendor(CC_VENDOR_AMD); + cc_set_mask(ms_hyperv.shared_gpa_boundary); + physical_mask &= ms_hyperv.shared_gpa_boundary - 1; + + x86_platform.hyper.is_private_mmio = hv_is_private_mmio; + x86_platform.guest.enc_cache_flush_required = hv_vtom_cache_flush_required; + x86_platform.guest.enc_tlb_flush_required = hv_vtom_tlb_flush_required; + x86_platform.guest.enc_status_change_finish = hv_vtom_set_host_visibility; +} + +#endif /* CONFIG_AMD_MEM_ENCRYPT */ + /* * hv_map_memory - map memory to extra space in the AMD SEV-SNP Isolation VM. */
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette