On Mon, Jan 11, 2021, Tom Lendacky wrote:
> On 1/8/21 6:47 PM, Sean Christopherson wrote:
> > Replace calls to svm_sev_enabled() with direct checks on sev_enabled, or
> > in the case of svm_mem_enc_op, simply drop the call to svm_sev_enabled().
> > This effectively replaces checks against a valid max_sev_asid with checks
> > against sev_enabled. sev_enabled is forced off by sev_hardware_setup()
> > if max_sev_asid is invalid, all call sites are guaranteed to run after
> > sev_hardware_setup(), and all of the checks care about SEV being fully
> > enabled (as opposed to intentionally handling the scenario where
> > max_sev_asid is valid but SEV enabling fails due to OOM).
> >
> > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > ---
> > arch/x86/kvm/svm/sev.c | 6 +++---
> > arch/x86/kvm/svm/svm.h | 5 -----
> > 2 files changed, 3 insertions(+), 8 deletions(-)
> >
>
> With CONFIG_KVM=y, CONFIG_KVM_AMD=y and CONFIG_CRYPTO_DEV_CCP_DD=m, I get
> the following build warning:
...
> In function ‘bitmap_zero’,
> inlined from ‘__sev_recycle_asids’ at arch/x86/kvm/svm/sev.c:92:2,
> inlined from ‘sev_asid_new’ at arch/x86/kvm/svm/sev.c:113:16,
> inlined from ‘sev_guest_init’ at arch/x86/kvm/svm/sev.c:195:9:
> ./include/linux/bitmap.h:238:2: warning: argument 1 null where non-null expected [-Wnonnull]
> 238 | memset(dst, 0, len);
> | ^~~~~~~~~~~~~~~~~~~
Ah, because that config "silently" disables CONFIG_KVM_AMD_SEV. The warning
pops up because svm_sev_enabled() included !IS_ENABLED(CONFIG_KVM_AMD_SEV) and
that was enough for the compiler to understand that svm_mem_enc_op() was a nop.
That being said, unless I'm missing something, this is a false positive the
compiler's part, e.g. the warning occurs even if sev_enabled is false be default,
i.e. CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT=n.
Anyways, I'm leaning towards "fixing" this by defining sev_enabled and
sev_es_enabled to false if CONFIG_KVM_AMD_SEV=n. It'd be a worthwhile change to
condition the default values on CONFIG_KVM_AMD_SEV anyways, so it'd kill two
birds with one stone. Long term, I'm tempted to exporing conditioning all of
sev.c on CONFIG_KVM_AMD_SEV=y, but there are just enough functions exposed via
svm.h that make me think it wouldn't be worth the effort.
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 1b9174a49b65..7e14514dd083 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -28,12 +28,17 @@
#define __ex(x) __kvm_handle_fault_on_reboot(x)
/* enable/disable SEV support */
+#ifdef CONFIG_KVM_AMD_SEV
static bool sev_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
module_param_named(sev, sev_enabled, bool, 0444);
/* enable/disable SEV-ES support */
static bool sev_es_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
module_param_named(sev_es, sev_es_enabled, bool, 0444);
+#else
+#define sev_enabled false
+#define sev_es_enabled false
+#endif /* CONFIG_KVM_AMD_SEV */
static u8 sev_enc_bit;
static int sev_flush_asids(void);
@@ -1253,11 +1258,12 @@ void sev_vm_destroy(struct kvm *kvm)
void __init sev_hardware_setup(void)
{
+#ifdef CONFIG_KVM_AMD_SEV
unsigned int eax, ebx, ecx, edx;
bool sev_es_supported = false;
bool sev_supported = false;
- if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev_enabled)
+ if (!sev_enabled)
goto out;
/* Does the CPU support SEV? */
@@ -1310,6 +1316,7 @@ void __init sev_hardware_setup(void)
out:
sev_enabled = sev_supported;
sev_es_enabled = sev_es_supported;
+#endif
}
void sev_hardware_teardown(void)
