Messages in this thread | | | Subject | Re: [PATCH 03/13] KVM: SVM: Move SEV module params/variables to sev.c | From | Tom Lendacky <> | Date | Mon, 11 Jan 2021 09:36:29 -0600 |
| |
On 1/11/21 4:42 AM, Vitaly Kuznetsov wrote: > Sean Christopherson <seanjc@google.com> writes: > >> Unconditionally invoke sev_hardware_setup() when configuring SVM and >> handle clearing the module params/variable 'sev' and 'sev_es' in >> sev_hardware_setup(). This allows making said variables static within >> sev.c and reduces the odds of a collision with guest code, e.g. the guest >> side of things has already laid claim to 'sev_enabled'. >> >> Signed-off-by: Sean Christopherson <seanjc@google.com> >> --- >> arch/x86/kvm/svm/sev.c | 11 +++++++++++ >> arch/x86/kvm/svm/svm.c | 15 +-------------- >> arch/x86/kvm/svm/svm.h | 2 -- >> 3 files changed, 12 insertions(+), 16 deletions(-) >> >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c >> index 0eeb6e1b803d..8ba93b8fa435 100644 >> --- a/arch/x86/kvm/svm/sev.c >> +++ b/arch/x86/kvm/svm/sev.c >> @@ -27,6 +27,14 @@ >> >> #define __ex(x) __kvm_handle_fault_on_reboot(x) >> >> +/* enable/disable SEV support */ >> +static int sev = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT); >> +module_param(sev, int, 0444); >> + >> +/* enable/disable SEV-ES support */ >> +static int sev_es = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT); >> +module_param(sev_es, int, 0444); > > Two stupid questions (and not really related to your patch) for > self-eduacation if I may: > > 1) Why do we rely on CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT (which > sound like it control the guest side of things) to set defaults here?
I thought it was a review comment, but I'm not able to find it now.
Brijesh probably remembers better than me.
> > 2) It appears to be possible to do 'modprobe kvm_amd sev=0 sev_es=1' and > this looks like a bogus configuration, should we make an effort to > validate the correctness upon module load?
This will still result in an overall sev=0 sev_es=0. Is the question just about issuing a message based on the initial values specified?
Thanks, Tom
> >> + >> static u8 sev_enc_bit; >> static int sev_flush_asids(void); >> static DECLARE_RWSEM(sev_deactivate_lock); >> @@ -1249,6 +1257,9 @@ void __init sev_hardware_setup(void) >> bool sev_es_supported = false; >> bool sev_supported = false; >> >> + if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev) >> + goto out; >> + >> /* Does the CPU support SEV? */ >> if (!boot_cpu_has(X86_FEATURE_SEV)) >> goto out; >> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c >> index ccf52c5531fb..f89f702b2a58 100644 >> --- a/arch/x86/kvm/svm/svm.c >> +++ b/arch/x86/kvm/svm/svm.c >> @@ -189,14 +189,6 @@ module_param(vls, int, 0444); >> static int vgif = true; >> module_param(vgif, int, 0444); >> >> -/* enable/disable SEV support */ >> -int sev = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT); >> -module_param(sev, int, 0444); >> - >> -/* enable/disable SEV-ES support */ >> -int sev_es = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT); >> -module_param(sev_es, int, 0444); >> - >> bool __read_mostly dump_invalid_vmcb; >> module_param(dump_invalid_vmcb, bool, 0644); >> >> @@ -976,12 +968,7 @@ static __init int svm_hardware_setup(void) >> kvm_enable_efer_bits(EFER_SVME | EFER_LMSLE); >> } >> >> - if (IS_ENABLED(CONFIG_KVM_AMD_SEV) && sev) { >> - sev_hardware_setup(); >> - } else { >> - sev = false; >> - sev_es = false; >> - } >> + sev_hardware_setup(); >> >> svm_adjust_mmio_mask(); >> >> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h >> index 0fe874ae5498..8e169835f52a 100644 >> --- a/arch/x86/kvm/svm/svm.h >> +++ b/arch/x86/kvm/svm/svm.h >> @@ -408,8 +408,6 @@ static inline bool gif_set(struct vcpu_svm *svm) >> #define MSR_CR3_LONG_MBZ_MASK 0xfff0000000000000U >> #define MSR_INVALID 0xffffffffU >> >> -extern int sev; >> -extern int sev_es; >> extern bool dump_invalid_vmcb; >> >> u32 svm_msrpm_offset(u32 msr); >
| |