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. > 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%7Cfe5a46e348a5464ea52b08d76f85909b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637100491764446005&sdata=R6RrRO0TpcfM7uzpBbsGhVp47bA%2BVoz624IBQif%2BxjA%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:
