On 11/22/19 3:24 PM, Jim Mattson wrote:
On Fri, Nov 22, 2019 at 1:22 PM Brijesh Singh <brijesh.singh@xxxxxxx> wrote:
Ah, I missed the fact that we don't need to pass the SevES
bit to the guest because guest actually does not need it.
It just needs the SevBit to make decision whether its
safe to call the RDMSR for SEV_STATUS. The SEV_STATUS
MSR will give information which SEV feature is enabled.
Why does it have to be safe to read the SEV_STATUS MSR? We read
nonexistent MSRs all the time.
The MSR access happens very early in the boot, IIRC calling this MSR on
non AMD platform may result in #GP. If OS is not ready to handle the
#GP so early then we will have problem.
thanks
On 11/22/19 1:52 PM, Peter Gonda wrote:
I am not sure that the SevEs CPUID bit has the same problem as the Sev
bit. It seems the reason the Sev bit was to be passed to the guest was
to prevent the guest from reading the SEV MSR if it did not exist. If
the guest is running with SevEs it must be also running with Sev. So
the guest can safely read the SevStatus MSR to check the SevEsEnabled
bit because the Sev CPUID bit will be set.
If I look at the AMD patches for ES. I see just that,
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAMDESE%2Flinux%2Fcommit%2Fc19d84b803caf8e3130b1498868d0fcafc755da7&data=02%7C01%7Cbrijesh.singh%40amd.com%7C86545e99d62e4f8e8eb508d76f92720c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637100547082927245&sdata=5YsknSUmboS95T0OfLWvJ%2BcOQQk5sIllGfshNqf0j6Y%3D&reserved=0,
it doesn't look for the SevEs CPUID bit.
} else {
/* For SEV, check the SEV MSR */
msr = __rdmsr(MSR_AMD64_SEV);
if (!(msr & MSR_AMD64_SEV_ENABLED))
return;
/* SEV state cannot be controlled by a command line option */
sme_me_mask = me_mask;
sme_me_status |= SEV_ACTIVE;
physical_mask &= ~sme_me_mask;
+
+ if (!(msr & MSR_AMD64_SEV_ES_ENABLED))
+ return;
+
+ sme_me_status |= SEV_ES_ACTIVE;
return;
}
}
On Fri, Nov 22, 2019 at 9:18 AM Jim Mattson <jmattson@xxxxxxxxxx> wrote:
Does SEV-ES indicate that SEV-ES guests are supported, or that the
current (v)CPU is running with SEV-ES enabled, or both?
On Fri, Nov 22, 2019 at 5:01 AM Brijesh Singh <brijesh.singh@xxxxxxx> wrote:
On 11/21/19 2:33 PM, Peter Gonda wrote:
Only pass through guest relevant CPUID information: Cbit location and
SEV bit. The kernel does not support nested SEV guests so the other data
in this CPUID leaf is unneeded by the guest.
Suggested-by: Jim Mattson <jmattson@xxxxxxxxxx>
Signed-off-by: Peter Gonda <pgonda@xxxxxxxxxx>
Reviewed-by: Jim Mattson <jmattson@xxxxxxxxxx>
---
arch/x86/kvm/cpuid.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 946fa9cb9dd6..6439fb1dbe76 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -780,8 +780,14 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
break;
/* Support memory encryption cpuid if host supports it */
case 0x8000001F:
- if (!boot_cpu_has(X86_FEATURE_SEV))
+ if (boot_cpu_has(X86_FEATURE_SEV)) {
+ /* Expose only SEV bit and CBit location */
+ entry->eax &= F(SEV);
I know SEV-ES patches are not accepted yet, but can I ask to pass the
SEV-ES bit in eax?
+ entry->ebx &= GENMASK(5, 0);
+ entry->edx = entry->ecx = 0;
+ } else {
entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
+ }
break;
/*Add support for Centaur's CPUID instruction*/
case 0xC0000000:
