On 1/11/21 4:42 AM, Vitaly Kuznetsov wrote:
Sean Christopherson <seanjc@xxxxxxxxxx> 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@xxxxxxxxxx> --- 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.hindex 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);
