Play nice with systems where SEV and SEV-ES are enabled, but all ASIDs
have been carved out for SEV-eS, i.e. where actually running SEV guests
is impossible.
v4:
- Convert all ASID usage to unsigned integers.
- Clean up sev_asid_new() so that it doesn't needlessly overload its
return value.
- Split out the -EBUSY=>-EINVAL change to a separate patch.
v3: https://lore.kernel.org/all/[email protected]
Ashish Kalra (1):
KVM: SVM: Add support for allowing zero SEV ASIDs
Sean Christopherson (3):
KVM: SVM: Set sev->asid in sev_asid_new() instead of overloading the
return
KVM: SVM: Use unsigned integers when dealing with ASIDs
KVM: SVM: Return -EINVAL instead of -EBUSY on attempt to re-init
SEV/SEV-ES
arch/x86/kvm/svm/sev.c | 58 +++++++++++++++++++++++++-----------------
arch/x86/kvm/trace.h | 10 ++++----
2 files changed, 39 insertions(+), 29 deletions(-)
base-commit: 41bccc98fb7931d63d03f326a746ac4d429c1dd3
--
2.43.0.429.g432eaa2c6b-goog
Return -EINVAL instead of -EBUSY if userspace attempts KVM_SEV{,ES}_INIT
on a VM that already has SEV active. Returning -EBUSY is nonsencial as
it's impossible to deactivate SEV without destroying the VM, i.e. the VM
isn't "busy" in any sane sense of the word, and the odds of any userspace
wanting exactly -EBUSY on a userspace bug are minuscule.
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/svm/sev.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 38e40fbc7ea0..cb19b57e1031 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -259,9 +259,8 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
if (kvm->created_vcpus)
return -EINVAL;
- ret = -EBUSY;
if (unlikely(sev->active))
- return ret;
+ return -EINVAL;
sev->active = true;
sev->es_active = argp->id == KVM_SEV_ES_INIT;
--
2.43.0.429.g432eaa2c6b-goog
Explicitly set sev->asid in sev_asid_new() when a new ASID is successfully
allocated, and return '0' to indicate success instead of overloading the
return value to multiplex the ASID with error codes. There is exactly one
caller of sev_asid_new(), and sev_asid_free() already consumes sev->asid,
i.e. returning the ASID isn't necessary for flexibility, nor does it
provide symmetry between related APIs.
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/svm/sev.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index f760106c31f8..7c000088bca6 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -179,7 +179,8 @@ static int sev_asid_new(struct kvm_sev_info *sev)
mutex_unlock(&sev_bitmap_lock);
- return asid;
+ sev->asid = asid;
+ return 0;
e_uncharge:
sev_misc_cg_uncharge(sev);
put_misc_cg(sev->misc_cg);
@@ -246,7 +247,7 @@ static void sev_unbind_asid(struct kvm *kvm, unsigned int handle)
static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
{
struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
- int asid, ret;
+ int ret;
if (kvm->created_vcpus)
return -EINVAL;
@@ -257,10 +258,9 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
sev->active = true;
sev->es_active = argp->id == KVM_SEV_ES_INIT;
- asid = sev_asid_new(sev);
- if (asid < 0)
+ ret = sev_asid_new(sev);
+ if (ret)
goto e_no_asid;
- sev->asid = asid;
ret = sev_platform_init(&argp->error);
if (ret)
--
2.43.0.429.g432eaa2c6b-goog
From: Ashish Kalra <[email protected]>
Some BIOSes allow the end user to set the minimum SEV ASID value
(CPUID 0x8000001F_EDX) to be greater than the maximum number of
encrypted guests, or maximum SEV ASID value (CPUID 0x8000001F_ECX)
in order to dedicate all the SEV ASIDs to SEV-ES or SEV-SNP.
The SEV support, as coded, does not handle the case where the minimum
SEV ASID value can be greater than the maximum SEV ASID value.
As a result, the following confusing message is issued:
[ 30.715724] kvm_amd: SEV enabled (ASIDs 1007 - 1006)
Fix the support to properly handle this case.
Fixes: 916391a2d1dc ("KVM: SVM: Add support for SEV-ES capability in KVM")
Suggested-by: Sean Christopherson <[email protected]>
Signed-off-by: Ashish Kalra <[email protected]>
Cc: [email protected]
Acked-by: Tom Lendacky <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/svm/sev.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 04c4c14473fd..38e40fbc7ea0 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -144,10 +144,21 @@ static void sev_misc_cg_uncharge(struct kvm_sev_info *sev)
static int sev_asid_new(struct kvm_sev_info *sev)
{
- unsigned int asid, min_asid, max_asid;
+ /*
+ * SEV-enabled guests must use asid from min_sev_asid to max_sev_asid.
+ * SEV-ES-enabled guest can use from 1 to min_sev_asid - 1.
+ * Note: min ASID can end up larger than the max if basic SEV support is
+ * effectively disabled by disallowing use of ASIDs for SEV guests.
+ */
+ unsigned int min_asid = sev->es_active ? 1 : min_sev_asid;
+ unsigned int max_asid = sev->es_active ? min_sev_asid - 1 : max_sev_asid;
+ unsigned int asid;
bool retry = true;
int ret;
+ if (min_asid > max_asid)
+ return -ENOTTY;
+
WARN_ON(sev->misc_cg);
sev->misc_cg = get_current_misc_cg();
ret = sev_misc_cg_try_charge(sev);
@@ -159,12 +170,6 @@ static int sev_asid_new(struct kvm_sev_info *sev)
mutex_lock(&sev_bitmap_lock);
- /*
- * SEV-enabled guests must use asid from min_sev_asid to max_sev_asid.
- * SEV-ES-enabled guest can use from 1 to min_sev_asid - 1.
- */
- min_asid = sev->es_active ? 1 : min_sev_asid;
- max_asid = sev->es_active ? min_sev_asid - 1 : max_sev_asid;
again:
asid = find_next_zero_bit(sev_asid_bitmap, max_asid + 1, min_asid);
if (asid > max_asid) {
@@ -2234,8 +2239,10 @@ void __init sev_hardware_setup(void)
goto out;
}
- sev_asid_count = max_sev_asid - min_sev_asid + 1;
- WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_SEV, sev_asid_count));
+ if (min_sev_asid <= max_sev_asid) {
+ sev_asid_count = max_sev_asid - min_sev_asid + 1;
+ WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_SEV, sev_asid_count));
+ }
sev_supported = true;
/* SEV-ES support requested? */
@@ -2266,7 +2273,9 @@ void __init sev_hardware_setup(void)
out:
if (boot_cpu_has(X86_FEATURE_SEV))
pr_info("SEV %s (ASIDs %u - %u)\n",
- sev_supported ? "enabled" : "disabled",
+ sev_supported ? min_sev_asid <= max_sev_asid ? "enabled" :
+ "unusable" :
+ "disabled",
min_sev_asid, max_sev_asid);
if (boot_cpu_has(X86_FEATURE_SEV_ES))
pr_info("SEV-ES %s (ASIDs %u - %u)\n",
--
2.43.0.429.g432eaa2c6b-goog
On 1/31/24 17:56, Sean Christopherson wrote:
> Explicitly set sev->asid in sev_asid_new() when a new ASID is successfully
> allocated, and return '0' to indicate success instead of overloading the
> return value to multiplex the ASID with error codes. There is exactly one
> caller of sev_asid_new(), and sev_asid_free() already consumes sev->asid,
> i.e. returning the ASID isn't necessary for flexibility, nor does it
> provide symmetry between related APIs.
>
> Signed-off-by: Sean Christopherson <[email protected]>
Reviewed-by: Tom Lendacky <[email protected]>
> ---
> arch/x86/kvm/svm/sev.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index f760106c31f8..7c000088bca6 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -179,7 +179,8 @@ static int sev_asid_new(struct kvm_sev_info *sev)
>
> mutex_unlock(&sev_bitmap_lock);
>
> - return asid;
> + sev->asid = asid;
> + return 0;
> e_uncharge:
> sev_misc_cg_uncharge(sev);
> put_misc_cg(sev->misc_cg);
> @@ -246,7 +247,7 @@ static void sev_unbind_asid(struct kvm *kvm, unsigned int handle)
> static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
> {
> struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> - int asid, ret;
> + int ret;
>
> if (kvm->created_vcpus)
> return -EINVAL;
> @@ -257,10 +258,9 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
>
> sev->active = true;
> sev->es_active = argp->id == KVM_SEV_ES_INIT;
> - asid = sev_asid_new(sev);
> - if (asid < 0)
> + ret = sev_asid_new(sev);
> + if (ret)
> goto e_no_asid;
> - sev->asid = asid;
>
> ret = sev_platform_init(&argp->error);
> if (ret)
On 1/31/24 17:56, Sean Christopherson wrote:
> Return -EINVAL instead of -EBUSY if userspace attempts KVM_SEV{,ES}_INIT
> on a VM that already has SEV active. Returning -EBUSY is nonsencial as
> it's impossible to deactivate SEV without destroying the VM, i.e. the VM
> isn't "busy" in any sane sense of the word, and the odds of any userspace
> wanting exactly -EBUSY on a userspace bug are minuscule.
>
> Signed-off-by: Sean Christopherson <[email protected]>
Reviewed-by: Tom Lendacky <[email protected]>
> ---
> arch/x86/kvm/svm/sev.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 38e40fbc7ea0..cb19b57e1031 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -259,9 +259,8 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
> if (kvm->created_vcpus)
> return -EINVAL;
>
> - ret = -EBUSY;
> if (unlikely(sev->active))
> - return ret;
> + return -EINVAL;
>
> sev->active = true;
> sev->es_active = argp->id == KVM_SEV_ES_INIT;
On Wed, 31 Jan 2024 15:56:05 -0800, Sean Christopherson wrote:
> Play nice with systems where SEV and SEV-ES are enabled, but all ASIDs
> have been carved out for SEV-eS, i.e. where actually running SEV guests
> is impossible.
>
> v4:
> - Convert all ASID usage to unsigned integers.
> - Clean up sev_asid_new() so that it doesn't needlessly overload its
> return value.
> - Split out the -EBUSY=>-EINVAL change to a separate patch.
>
> [...]
Applied to kvm-x86 svm, thanks!
[1/4] KVM: SVM: Set sev->asid in sev_asid_new() instead of overloading the return
https://github.com/kvm-x86/linux/commit/cc4ce37bed85
[2/4] KVM: SVM: Use unsigned integers when dealing with ASIDs
https://github.com/kvm-x86/linux/commit/466eec4a22a7
[3/4] KVM: SVM: Add support for allowing zero SEV ASIDs
https://github.com/kvm-x86/linux/commit/0aa6b90ef9d7
[4/4] KVM: SVM: Return -EINVAL instead of -EBUSY on attempt to re-init SEV/SEV-ES
https://github.com/kvm-x86/linux/commit/fdd58834d132
--
https://github.com/kvm-x86/linux/tree/next