* [PATCH] kvm/sev: make SEV/SEV-ES asids configurable @ 2023-10-10 10:04 ` José Pekkarinen 0 siblings, 0 replies; 14+ messages in thread From: José Pekkarinen @ 2023-10-10 10:04 UTC (permalink / raw) To: seanjc, pbonzini, skhan Cc: dave.hansen, kvm, x86, linux-kernel, mingo, bp, hpa, tglx, linux-kernel-mentees There are bioses that doesn't allow to configure the number of asids allocated for SEV/SEV-ES, for those cases, the default behaviour allocates all the asids for SEV, leaving no room for SEV-ES to have some fun. If the user request SEV-ES to be enabled, it will find the kernel just run out of resources and ignored user request. This following patch will address this issue by making the number of asids for SEV/SEV-ES configurable over kernel module parameters. Signed-off-by: José Pekkarinen <jose.pekkarinen@foxhound.fi> --- arch/x86/kvm/svm/sev.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 07756b7348ae..68a63b42d16a 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -51,9 +51,18 @@ static bool sev_enabled = true; module_param_named(sev, sev_enabled, bool, 0444); +/* nr of asids requested for SEV */ +static unsigned int requested_sev_asids; +module_param_named(sev_asids, requested_sev_asids, uint, 0444); + /* enable/disable SEV-ES support */ static bool sev_es_enabled = true; module_param_named(sev_es, sev_es_enabled, bool, 0444); + +/* nr of asids requested for SEV-ES */ +static unsigned int requested_sev_es_asids; +module_param_named(sev_es_asids, requested_sev_asids, uint, 0444); + #else #define sev_enabled false #define sev_es_enabled false @@ -2194,6 +2203,11 @@ void __init sev_hardware_setup(void) if (!max_sev_asid) goto out; + if (requested_sev_asids + requested_sev_es_asids > max_sev_asid) { + pr_info("SEV asids requested more than available: %u ASIDs\n", max_sev_asid); + goto out; + } + /* Minimum ASID value that should be used for SEV guest */ min_sev_asid = edx; sev_me_mask = 1UL << (ebx & 0x3f); @@ -2215,7 +2229,8 @@ void __init sev_hardware_setup(void) goto out; } - sev_asid_count = max_sev_asid - min_sev_asid + 1; + sev_asid_count = (requested_sev_asids) ? max_sev_asid - min_sev_asid + 1 : + requested_sev_asids; WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_SEV, sev_asid_count)); sev_supported = true; @@ -2237,10 +2252,11 @@ void __init sev_hardware_setup(void) goto out; /* Has the system been allocated ASIDs for SEV-ES? */ - if (min_sev_asid == 1) + if (max_sev_asid - sev_asid_count <= 1) goto out; - sev_es_asid_count = min_sev_asid - 1; + sev_es_asid_count = (requested_sev_es_asids) ? min_sev_asid - 1 : + requested_sev_es_asids; WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_SEV_ES, sev_es_asid_count)); sev_es_supported = true; @@ -2248,11 +2264,13 @@ void __init sev_hardware_setup(void) if (boot_cpu_has(X86_FEATURE_SEV)) pr_info("SEV %s (ASIDs %u - %u)\n", sev_supported ? "enabled" : "disabled", - min_sev_asid, max_sev_asid); + min_sev_asid, sev_asid_count); if (boot_cpu_has(X86_FEATURE_SEV_ES)) pr_info("SEV-ES %s (ASIDs %u - %u)\n", sev_es_supported ? "enabled" : "disabled", - min_sev_asid > 1 ? 1 : 0, min_sev_asid - 1); + sev_asid_count, + (requested_sev_es_asids) ? 0 : + sev_asid_count + sev_es_asid_count); sev_enabled = sev_supported; sev_es_enabled = sev_es_supported; -- 2.41.0 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] kvm/sev: make SEV/SEV-ES asids configurable @ 2023-10-10 10:04 ` José Pekkarinen 0 siblings, 0 replies; 14+ messages in thread From: José Pekkarinen @ 2023-10-10 10:04 UTC (permalink / raw) To: seanjc, pbonzini, skhan Cc: José Pekkarinen, tglx, mingo, bp, dave.hansen, x86, hpa, kvm, linux-kernel, linux-kernel-mentees There are bioses that doesn't allow to configure the number of asids allocated for SEV/SEV-ES, for those cases, the default behaviour allocates all the asids for SEV, leaving no room for SEV-ES to have some fun. If the user request SEV-ES to be enabled, it will find the kernel just run out of resources and ignored user request. This following patch will address this issue by making the number of asids for SEV/SEV-ES configurable over kernel module parameters. Signed-off-by: José Pekkarinen <jose.pekkarinen@foxhound.fi> --- arch/x86/kvm/svm/sev.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 07756b7348ae..68a63b42d16a 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -51,9 +51,18 @@ static bool sev_enabled = true; module_param_named(sev, sev_enabled, bool, 0444); +/* nr of asids requested for SEV */ +static unsigned int requested_sev_asids; +module_param_named(sev_asids, requested_sev_asids, uint, 0444); + /* enable/disable SEV-ES support */ static bool sev_es_enabled = true; module_param_named(sev_es, sev_es_enabled, bool, 0444); + +/* nr of asids requested for SEV-ES */ +static unsigned int requested_sev_es_asids; +module_param_named(sev_es_asids, requested_sev_asids, uint, 0444); + #else #define sev_enabled false #define sev_es_enabled false @@ -2194,6 +2203,11 @@ void __init sev_hardware_setup(void) if (!max_sev_asid) goto out; + if (requested_sev_asids + requested_sev_es_asids > max_sev_asid) { + pr_info("SEV asids requested more than available: %u ASIDs\n", max_sev_asid); + goto out; + } + /* Minimum ASID value that should be used for SEV guest */ min_sev_asid = edx; sev_me_mask = 1UL << (ebx & 0x3f); @@ -2215,7 +2229,8 @@ void __init sev_hardware_setup(void) goto out; } - sev_asid_count = max_sev_asid - min_sev_asid + 1; + sev_asid_count = (requested_sev_asids) ? max_sev_asid - min_sev_asid + 1 : + requested_sev_asids; WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_SEV, sev_asid_count)); sev_supported = true; @@ -2237,10 +2252,11 @@ void __init sev_hardware_setup(void) goto out; /* Has the system been allocated ASIDs for SEV-ES? */ - if (min_sev_asid == 1) + if (max_sev_asid - sev_asid_count <= 1) goto out; - sev_es_asid_count = min_sev_asid - 1; + sev_es_asid_count = (requested_sev_es_asids) ? min_sev_asid - 1 : + requested_sev_es_asids; WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_SEV_ES, sev_es_asid_count)); sev_es_supported = true; @@ -2248,11 +2264,13 @@ void __init sev_hardware_setup(void) if (boot_cpu_has(X86_FEATURE_SEV)) pr_info("SEV %s (ASIDs %u - %u)\n", sev_supported ? "enabled" : "disabled", - min_sev_asid, max_sev_asid); + min_sev_asid, sev_asid_count); if (boot_cpu_has(X86_FEATURE_SEV_ES)) pr_info("SEV-ES %s (ASIDs %u - %u)\n", sev_es_supported ? "enabled" : "disabled", - min_sev_asid > 1 ? 1 : 0, min_sev_asid - 1); + sev_asid_count, + (requested_sev_es_asids) ? 0 : + sev_asid_count + sev_es_asid_count); sev_enabled = sev_supported; sev_es_enabled = sev_es_supported; -- 2.41.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] kvm/sev: make SEV/SEV-ES asids configurable 2023-10-10 10:04 ` José Pekkarinen @ 2023-10-10 11:35 ` Greg KH -1 siblings, 0 replies; 14+ messages in thread From: Greg KH @ 2023-10-10 11:35 UTC (permalink / raw) To: José Pekkarinen Cc: x86, hpa, kvm, seanjc, dave.hansen, linux-kernel, mingo, bp, pbonzini, tglx, linux-kernel-mentees On Tue, Oct 10, 2023 at 01:04:39PM +0300, José Pekkarinen wrote: > There are bioses that doesn't allow to configure the > number of asids allocated for SEV/SEV-ES, for those > cases, the default behaviour allocates all the asids > for SEV, leaving no room for SEV-ES to have some fun. "fun"? Also, please use the full 72 columns for your changelog. > If the user request SEV-ES to be enabled, it will > find the kernel just run out of resources and ignored > user request. This following patch will address this > issue by making the number of asids for SEV/SEV-ES > configurable over kernel module parameters. > > Signed-off-by: José Pekkarinen <jose.pekkarinen@foxhound.fi> > --- > arch/x86/kvm/svm/sev.c | 28 +++++++++++++++++++++++----- > 1 file changed, 23 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index 07756b7348ae..68a63b42d16a 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -51,9 +51,18 @@ > static bool sev_enabled = true; > module_param_named(sev, sev_enabled, bool, 0444); > > +/* nr of asids requested for SEV */ > +static unsigned int requested_sev_asids; > +module_param_named(sev_asids, requested_sev_asids, uint, 0444); > + > /* enable/disable SEV-ES support */ > static bool sev_es_enabled = true; > module_param_named(sev_es, sev_es_enabled, bool, 0444); > + > +/* nr of asids requested for SEV-ES */ > +static unsigned int requested_sev_es_asids; > +module_param_named(sev_es_asids, requested_sev_asids, uint, 0444); Why more module parameters? Why can't this "just work" properly without forcing a user to make manual changes? This isn't the 1990's anymore. > + > #else > #define sev_enabled false > #define sev_es_enabled false > @@ -2194,6 +2203,11 @@ void __init sev_hardware_setup(void) > if (!max_sev_asid) > goto out; > > + if (requested_sev_asids + requested_sev_es_asids > max_sev_asid) { > + pr_info("SEV asids requested more than available: %u ASIDs\n", max_sev_asid); Why isn't this an error? thanks, greg k-h _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kvm/sev: make SEV/SEV-ES asids configurable @ 2023-10-10 11:35 ` Greg KH 0 siblings, 0 replies; 14+ messages in thread From: Greg KH @ 2023-10-10 11:35 UTC (permalink / raw) To: José Pekkarinen Cc: seanjc, pbonzini, skhan, dave.hansen, kvm, x86, linux-kernel, mingo, bp, hpa, tglx, linux-kernel-mentees On Tue, Oct 10, 2023 at 01:04:39PM +0300, José Pekkarinen wrote: > There are bioses that doesn't allow to configure the > number of asids allocated for SEV/SEV-ES, for those > cases, the default behaviour allocates all the asids > for SEV, leaving no room for SEV-ES to have some fun. "fun"? Also, please use the full 72 columns for your changelog. > If the user request SEV-ES to be enabled, it will > find the kernel just run out of resources and ignored > user request. This following patch will address this > issue by making the number of asids for SEV/SEV-ES > configurable over kernel module parameters. > > Signed-off-by: José Pekkarinen <jose.pekkarinen@foxhound.fi> > --- > arch/x86/kvm/svm/sev.c | 28 +++++++++++++++++++++++----- > 1 file changed, 23 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index 07756b7348ae..68a63b42d16a 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -51,9 +51,18 @@ > static bool sev_enabled = true; > module_param_named(sev, sev_enabled, bool, 0444); > > +/* nr of asids requested for SEV */ > +static unsigned int requested_sev_asids; > +module_param_named(sev_asids, requested_sev_asids, uint, 0444); > + > /* enable/disable SEV-ES support */ > static bool sev_es_enabled = true; > module_param_named(sev_es, sev_es_enabled, bool, 0444); > + > +/* nr of asids requested for SEV-ES */ > +static unsigned int requested_sev_es_asids; > +module_param_named(sev_es_asids, requested_sev_asids, uint, 0444); Why more module parameters? Why can't this "just work" properly without forcing a user to make manual changes? This isn't the 1990's anymore. > + > #else > #define sev_enabled false > #define sev_es_enabled false > @@ -2194,6 +2203,11 @@ void __init sev_hardware_setup(void) > if (!max_sev_asid) > goto out; > > + if (requested_sev_asids + requested_sev_es_asids > max_sev_asid) { > + pr_info("SEV asids requested more than available: %u ASIDs\n", max_sev_asid); Why isn't this an error? thanks, greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kvm/sev: make SEV/SEV-ES asids configurable 2023-10-10 11:35 ` Greg KH @ 2023-10-10 11:40 ` Paolo Bonzini -1 siblings, 0 replies; 14+ messages in thread From: Paolo Bonzini @ 2023-10-10 11:40 UTC (permalink / raw) To: Greg KH, José Pekkarinen Cc: x86, hpa, kvm, seanjc, dave.hansen, linux-kernel, mingo, bp, tglx, linux-kernel-mentees On 10/10/23 13:35, Greg KH wrote: > On Tue, Oct 10, 2023 at 01:04:39PM +0300, José Pekkarinen wrote: >> There are bioses that doesn't allow to configure the >> number of asids allocated for SEV/SEV-ES, for those >> cases, the default behaviour allocates all the asids >> for SEV, leaving no room for SEV-ES to have some fun. In addition to what Greg pointed out (and there are many more cases that have to be checked for errors, including possible overflows), why is it correct to just ignore what's in CPUID? Paolo > "fun"? > > Also, please use the full 72 columns for your changelog. > >> If the user request SEV-ES to be enabled, it will >> find the kernel just run out of resources and ignored >> user request. This following patch will address this >> issue by making the number of asids for SEV/SEV-ES >> configurable over kernel module parameters. >> >> Signed-off-by: José Pekkarinen <jose.pekkarinen@foxhound.fi> >> --- >> arch/x86/kvm/svm/sev.c | 28 +++++++++++++++++++++++----- >> 1 file changed, 23 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c >> index 07756b7348ae..68a63b42d16a 100644 >> --- a/arch/x86/kvm/svm/sev.c >> +++ b/arch/x86/kvm/svm/sev.c >> @@ -51,9 +51,18 @@ >> static bool sev_enabled = true; >> module_param_named(sev, sev_enabled, bool, 0444); >> >> +/* nr of asids requested for SEV */ >> +static unsigned int requested_sev_asids; >> +module_param_named(sev_asids, requested_sev_asids, uint, 0444); >> + >> /* enable/disable SEV-ES support */ >> static bool sev_es_enabled = true; >> module_param_named(sev_es, sev_es_enabled, bool, 0444); >> + >> +/* nr of asids requested for SEV-ES */ >> +static unsigned int requested_sev_es_asids; >> +module_param_named(sev_es_asids, requested_sev_asids, uint, 0444); > > Why more module parameters? Why can't this "just work" properly without > forcing a user to make manual changes? This isn't the 1990's anymore. > > >> + >> #else >> #define sev_enabled false >> #define sev_es_enabled false >> @@ -2194,6 +2203,11 @@ void __init sev_hardware_setup(void) >> if (!max_sev_asid) >> goto out; >> >> + if (requested_sev_asids + requested_sev_es_asids > max_sev_asid) { >> + pr_info("SEV asids requested more than available: %u ASIDs\n", max_sev_asid); > > Why isn't this an error? > > thanks, > > greg k-h > _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kvm/sev: make SEV/SEV-ES asids configurable @ 2023-10-10 11:40 ` Paolo Bonzini 0 siblings, 0 replies; 14+ messages in thread From: Paolo Bonzini @ 2023-10-10 11:40 UTC (permalink / raw) To: Greg KH, José Pekkarinen Cc: seanjc, skhan, dave.hansen, kvm, x86, linux-kernel, mingo, bp, hpa, tglx, linux-kernel-mentees On 10/10/23 13:35, Greg KH wrote: > On Tue, Oct 10, 2023 at 01:04:39PM +0300, José Pekkarinen wrote: >> There are bioses that doesn't allow to configure the >> number of asids allocated for SEV/SEV-ES, for those >> cases, the default behaviour allocates all the asids >> for SEV, leaving no room for SEV-ES to have some fun. In addition to what Greg pointed out (and there are many more cases that have to be checked for errors, including possible overflows), why is it correct to just ignore what's in CPUID? Paolo > "fun"? > > Also, please use the full 72 columns for your changelog. > >> If the user request SEV-ES to be enabled, it will >> find the kernel just run out of resources and ignored >> user request. This following patch will address this >> issue by making the number of asids for SEV/SEV-ES >> configurable over kernel module parameters. >> >> Signed-off-by: José Pekkarinen <jose.pekkarinen@foxhound.fi> >> --- >> arch/x86/kvm/svm/sev.c | 28 +++++++++++++++++++++++----- >> 1 file changed, 23 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c >> index 07756b7348ae..68a63b42d16a 100644 >> --- a/arch/x86/kvm/svm/sev.c >> +++ b/arch/x86/kvm/svm/sev.c >> @@ -51,9 +51,18 @@ >> static bool sev_enabled = true; >> module_param_named(sev, sev_enabled, bool, 0444); >> >> +/* nr of asids requested for SEV */ >> +static unsigned int requested_sev_asids; >> +module_param_named(sev_asids, requested_sev_asids, uint, 0444); >> + >> /* enable/disable SEV-ES support */ >> static bool sev_es_enabled = true; >> module_param_named(sev_es, sev_es_enabled, bool, 0444); >> + >> +/* nr of asids requested for SEV-ES */ >> +static unsigned int requested_sev_es_asids; >> +module_param_named(sev_es_asids, requested_sev_asids, uint, 0444); > > Why more module parameters? Why can't this "just work" properly without > forcing a user to make manual changes? This isn't the 1990's anymore. > > >> + >> #else >> #define sev_enabled false >> #define sev_es_enabled false >> @@ -2194,6 +2203,11 @@ void __init sev_hardware_setup(void) >> if (!max_sev_asid) >> goto out; >> >> + if (requested_sev_asids + requested_sev_es_asids > max_sev_asid) { >> + pr_info("SEV asids requested more than available: %u ASIDs\n", max_sev_asid); > > Why isn't this an error? > > thanks, > > greg k-h > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kvm/sev: make SEV/SEV-ES asids configurable 2023-10-10 11:40 ` Paolo Bonzini @ 2023-10-10 12:20 ` José Pekkarinen -1 siblings, 0 replies; 14+ messages in thread From: José Pekkarinen @ 2023-10-10 12:20 UTC (permalink / raw) To: Paolo Bonzini Cc: x86, hpa, kvm, dave.hansen, linux-kernel, mingo, bp, tglx, seanjc, linux-kernel-mentees On 2023-10-10 14:40, Paolo Bonzini wrote: > On 10/10/23 13:35, Greg KH wrote: >> On Tue, Oct 10, 2023 at 01:04:39PM +0300, José Pekkarinen wrote: >>> There are bioses that doesn't allow to configure the >>> number of asids allocated for SEV/SEV-ES, for those >>> cases, the default behaviour allocates all the asids >>> for SEV, leaving no room for SEV-ES to have some fun. > > In addition to what Greg pointed out (and there are many more cases > that have to be checked for errors, including possible overflows), why > is it correct to just ignore what's in CPUID? I'll check if I can use cpuid info to improve this for v2, I just noticed my cpu advertises sev_es but the kernel doesn't activate it despite of the kvm_amd parameter being set, so I tried to fix it this way, but any comments or ideas for improvement are welcomed. Thanks! José. _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kvm/sev: make SEV/SEV-ES asids configurable @ 2023-10-10 12:20 ` José Pekkarinen 0 siblings, 0 replies; 14+ messages in thread From: José Pekkarinen @ 2023-10-10 12:20 UTC (permalink / raw) To: Paolo Bonzini Cc: Greg KH, seanjc, skhan, dave.hansen, kvm, x86, linux-kernel, mingo, bp, hpa, tglx, linux-kernel-mentees On 2023-10-10 14:40, Paolo Bonzini wrote: > On 10/10/23 13:35, Greg KH wrote: >> On Tue, Oct 10, 2023 at 01:04:39PM +0300, José Pekkarinen wrote: >>> There are bioses that doesn't allow to configure the >>> number of asids allocated for SEV/SEV-ES, for those >>> cases, the default behaviour allocates all the asids >>> for SEV, leaving no room for SEV-ES to have some fun. > > In addition to what Greg pointed out (and there are many more cases > that have to be checked for errors, including possible overflows), why > is it correct to just ignore what's in CPUID? I'll check if I can use cpuid info to improve this for v2, I just noticed my cpu advertises sev_es but the kernel doesn't activate it despite of the kvm_amd parameter being set, so I tried to fix it this way, but any comments or ideas for improvement are welcomed. Thanks! José. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kvm/sev: make SEV/SEV-ES asids configurable 2023-10-10 11:35 ` Greg KH @ 2023-10-10 12:01 ` José Pekkarinen -1 siblings, 0 replies; 14+ messages in thread From: José Pekkarinen @ 2023-10-10 12:01 UTC (permalink / raw) To: Greg KH Cc: x86, hpa, kvm, seanjc, dave.hansen, linux-kernel, mingo, bp, pbonzini, tglx, linux-kernel-mentees On 2023-10-10 14:35, Greg KH wrote: > On Tue, Oct 10, 2023 at 01:04:39PM +0300, José Pekkarinen wrote: >> There are bioses that doesn't allow to configure the >> number of asids allocated for SEV/SEV-ES, for those >> cases, the default behaviour allocates all the asids >> for SEV, leaving no room for SEV-ES to have some fun. > > "fun"? > > Also, please use the full 72 columns for your changelog. > Alright. >> If the user request SEV-ES to be enabled, it will >> find the kernel just run out of resources and ignored >> user request. This following patch will address this >> issue by making the number of asids for SEV/SEV-ES >> configurable over kernel module parameters. >> >> Signed-off-by: José Pekkarinen <jose.pekkarinen@foxhound.fi> >> --- >> arch/x86/kvm/svm/sev.c | 28 +++++++++++++++++++++++----- >> 1 file changed, 23 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c >> index 07756b7348ae..68a63b42d16a 100644 >> --- a/arch/x86/kvm/svm/sev.c >> +++ b/arch/x86/kvm/svm/sev.c >> @@ -51,9 +51,18 @@ >> static bool sev_enabled = true; >> module_param_named(sev, sev_enabled, bool, 0444); >> >> +/* nr of asids requested for SEV */ >> +static unsigned int requested_sev_asids; >> +module_param_named(sev_asids, requested_sev_asids, uint, 0444); >> + >> /* enable/disable SEV-ES support */ >> static bool sev_es_enabled = true; >> module_param_named(sev_es, sev_es_enabled, bool, 0444); >> + >> +/* nr of asids requested for SEV-ES */ >> +static unsigned int requested_sev_es_asids; >> +module_param_named(sev_es_asids, requested_sev_asids, uint, 0444); > > Why more module parameters? Why can't this "just work" properly > without > forcing a user to make manual changes? This isn't the 1990's anymore. I could think of setting both cgroup caps to the maximum number of asids and then check the code in the module to make sure anytime a sev/sev_es asid is reserved both cgroups get updated to reflect the remaining asids, or even better, just use only one cgroup to keep track of them. That way the parameters become redundant, would any of these ideas work for you? Do you, or anybody else, have better ideas or preferences in this topic? >> + >> #else >> #define sev_enabled false >> #define sev_es_enabled false >> @@ -2194,6 +2203,11 @@ void __init sev_hardware_setup(void) >> if (!max_sev_asid) >> goto out; >> >> + if (requested_sev_asids + requested_sev_es_asids > max_sev_asid) { >> + pr_info("SEV asids requested more than available: %u ASIDs\n", >> max_sev_asid); > > Why isn't this an error? Good question, I'll address it in v2. Thanks! José. _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kvm/sev: make SEV/SEV-ES asids configurable @ 2023-10-10 12:01 ` José Pekkarinen 0 siblings, 0 replies; 14+ messages in thread From: José Pekkarinen @ 2023-10-10 12:01 UTC (permalink / raw) To: Greg KH Cc: seanjc, pbonzini, skhan, dave.hansen, kvm, x86, linux-kernel, mingo, bp, hpa, tglx, linux-kernel-mentees On 2023-10-10 14:35, Greg KH wrote: > On Tue, Oct 10, 2023 at 01:04:39PM +0300, José Pekkarinen wrote: >> There are bioses that doesn't allow to configure the >> number of asids allocated for SEV/SEV-ES, for those >> cases, the default behaviour allocates all the asids >> for SEV, leaving no room for SEV-ES to have some fun. > > "fun"? > > Also, please use the full 72 columns for your changelog. > Alright. >> If the user request SEV-ES to be enabled, it will >> find the kernel just run out of resources and ignored >> user request. This following patch will address this >> issue by making the number of asids for SEV/SEV-ES >> configurable over kernel module parameters. >> >> Signed-off-by: José Pekkarinen <jose.pekkarinen@foxhound.fi> >> --- >> arch/x86/kvm/svm/sev.c | 28 +++++++++++++++++++++++----- >> 1 file changed, 23 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c >> index 07756b7348ae..68a63b42d16a 100644 >> --- a/arch/x86/kvm/svm/sev.c >> +++ b/arch/x86/kvm/svm/sev.c >> @@ -51,9 +51,18 @@ >> static bool sev_enabled = true; >> module_param_named(sev, sev_enabled, bool, 0444); >> >> +/* nr of asids requested for SEV */ >> +static unsigned int requested_sev_asids; >> +module_param_named(sev_asids, requested_sev_asids, uint, 0444); >> + >> /* enable/disable SEV-ES support */ >> static bool sev_es_enabled = true; >> module_param_named(sev_es, sev_es_enabled, bool, 0444); >> + >> +/* nr of asids requested for SEV-ES */ >> +static unsigned int requested_sev_es_asids; >> +module_param_named(sev_es_asids, requested_sev_asids, uint, 0444); > > Why more module parameters? Why can't this "just work" properly > without > forcing a user to make manual changes? This isn't the 1990's anymore. I could think of setting both cgroup caps to the maximum number of asids and then check the code in the module to make sure anytime a sev/sev_es asid is reserved both cgroups get updated to reflect the remaining asids, or even better, just use only one cgroup to keep track of them. That way the parameters become redundant, would any of these ideas work for you? Do you, or anybody else, have better ideas or preferences in this topic? >> + >> #else >> #define sev_enabled false >> #define sev_es_enabled false >> @@ -2194,6 +2203,11 @@ void __init sev_hardware_setup(void) >> if (!max_sev_asid) >> goto out; >> >> + if (requested_sev_asids + requested_sev_es_asids > max_sev_asid) { >> + pr_info("SEV asids requested more than available: %u ASIDs\n", >> max_sev_asid); > > Why isn't this an error? Good question, I'll address it in v2. Thanks! José. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kvm/sev: make SEV/SEV-ES asids configurable 2023-10-10 10:04 ` José Pekkarinen @ 2023-10-10 16:49 ` Peter Gonda -1 siblings, 0 replies; 14+ messages in thread From: Peter Gonda via Linux-kernel-mentees @ 2023-10-10 16:49 UTC (permalink / raw) To: José Pekkarinen Cc: x86, hpa, kvm, seanjc, dave.hansen, linux-kernel, mingo, bp, pbonzini, tglx, linux-kernel-mentees On Tue, Oct 10, 2023 at 5:22 AM José Pekkarinen <jose.pekkarinen@foxhound.fi> wrote: > > There are bioses that doesn't allow to configure the > number of asids allocated for SEV/SEV-ES, for those > cases, the default behaviour allocates all the asids > for SEV, leaving no room for SEV-ES to have some fun. > If the user request SEV-ES to be enabled, it will > find the kernel just run out of resources and ignored > user request. This following patch will address this > issue by making the number of asids for SEV/SEV-ES > configurable over kernel module parameters. > All this patch does is introduce an error case right? Because if the BIOS hasn't actually configured those SEV-ES asids and KVM tries to use an SEV as an SEV-ES asid commands to the ASP will fail, right? What happens when you try to create an SEV-ES VM with this patch, when the BIOS hasn't allocated any SEV-ES asids? _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kvm/sev: make SEV/SEV-ES asids configurable @ 2023-10-10 16:49 ` Peter Gonda 0 siblings, 0 replies; 14+ messages in thread From: Peter Gonda @ 2023-10-10 16:49 UTC (permalink / raw) To: José Pekkarinen Cc: seanjc, pbonzini, skhan, tglx, mingo, bp, dave.hansen, x86, hpa, kvm, linux-kernel, linux-kernel-mentees On Tue, Oct 10, 2023 at 5:22 AM José Pekkarinen <jose.pekkarinen@foxhound.fi> wrote: > > There are bioses that doesn't allow to configure the > number of asids allocated for SEV/SEV-ES, for those > cases, the default behaviour allocates all the asids > for SEV, leaving no room for SEV-ES to have some fun. > If the user request SEV-ES to be enabled, it will > find the kernel just run out of resources and ignored > user request. This following patch will address this > issue by making the number of asids for SEV/SEV-ES > configurable over kernel module parameters. > All this patch does is introduce an error case right? Because if the BIOS hasn't actually configured those SEV-ES asids and KVM tries to use an SEV as an SEV-ES asid commands to the ASP will fail, right? What happens when you try to create an SEV-ES VM with this patch, when the BIOS hasn't allocated any SEV-ES asids? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kvm/sev: make SEV/SEV-ES asids configurable 2023-10-10 16:49 ` Peter Gonda @ 2023-10-10 18:03 ` José Pekkarinen -1 siblings, 0 replies; 14+ messages in thread From: José Pekkarinen @ 2023-10-10 18:03 UTC (permalink / raw) To: Peter Gonda Cc: x86, hpa, kvm, seanjc, dave.hansen, linux-kernel, mingo, bp, pbonzini, tglx, linux-kernel-mentees On 2023-10-10 19:49, Peter Gonda wrote: > On Tue, Oct 10, 2023 at 5:22 AM José Pekkarinen > <jose.pekkarinen@foxhound.fi> wrote: >> >> There are bioses that doesn't allow to configure the >> number of asids allocated for SEV/SEV-ES, for those >> cases, the default behaviour allocates all the asids >> for SEV, leaving no room for SEV-ES to have some fun. >> If the user request SEV-ES to be enabled, it will >> find the kernel just run out of resources and ignored >> user request. This following patch will address this >> issue by making the number of asids for SEV/SEV-ES >> configurable over kernel module parameters. >> > > All this patch does is introduce an error case right? Because if the > BIOS hasn't actually configured those SEV-ES asids and KVM tries to > use an SEV as an SEV-ES asid commands to the ASP will fail, right? > > What happens when you try to create an SEV-ES VM with this patch, when > the BIOS hasn't allocated any SEV-ES asids? It still doesn't enable SEV-ES since the cpu exposes min_sev_asids as 1, and there is a check to bail out in the hardware setup function, so definitely this is not fixing anything. I may not being understanding something here though, since my BIOS doesn't seem to have any options nor hints about SEV-ES, so I'm not quite sure it really does something to provide the functionality. For the records it is a Supermicro H11SSL-NC. Thanks! José. _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kvm/sev: make SEV/SEV-ES asids configurable @ 2023-10-10 18:03 ` José Pekkarinen 0 siblings, 0 replies; 14+ messages in thread From: José Pekkarinen @ 2023-10-10 18:03 UTC (permalink / raw) To: Peter Gonda Cc: seanjc, pbonzini, skhan, tglx, mingo, bp, dave.hansen, x86, hpa, kvm, linux-kernel, linux-kernel-mentees On 2023-10-10 19:49, Peter Gonda wrote: > On Tue, Oct 10, 2023 at 5:22 AM José Pekkarinen > <jose.pekkarinen@foxhound.fi> wrote: >> >> There are bioses that doesn't allow to configure the >> number of asids allocated for SEV/SEV-ES, for those >> cases, the default behaviour allocates all the asids >> for SEV, leaving no room for SEV-ES to have some fun. >> If the user request SEV-ES to be enabled, it will >> find the kernel just run out of resources and ignored >> user request. This following patch will address this >> issue by making the number of asids for SEV/SEV-ES >> configurable over kernel module parameters. >> > > All this patch does is introduce an error case right? Because if the > BIOS hasn't actually configured those SEV-ES asids and KVM tries to > use an SEV as an SEV-ES asid commands to the ASP will fail, right? > > What happens when you try to create an SEV-ES VM with this patch, when > the BIOS hasn't allocated any SEV-ES asids? It still doesn't enable SEV-ES since the cpu exposes min_sev_asids as 1, and there is a check to bail out in the hardware setup function, so definitely this is not fixing anything. I may not being understanding something here though, since my BIOS doesn't seem to have any options nor hints about SEV-ES, so I'm not quite sure it really does something to provide the functionality. For the records it is a Supermicro H11SSL-NC. Thanks! José. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-10-10 18:11 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-10 10:04 [PATCH] kvm/sev: make SEV/SEV-ES asids configurable José Pekkarinen 2023-10-10 10:04 ` José Pekkarinen 2023-10-10 11:35 ` Greg KH 2023-10-10 11:35 ` Greg KH 2023-10-10 11:40 ` Paolo Bonzini 2023-10-10 11:40 ` Paolo Bonzini 2023-10-10 12:20 ` José Pekkarinen 2023-10-10 12:20 ` José Pekkarinen 2023-10-10 12:01 ` José Pekkarinen 2023-10-10 12:01 ` José Pekkarinen 2023-10-10 16:49 ` Peter Gonda via Linux-kernel-mentees 2023-10-10 16:49 ` Peter Gonda 2023-10-10 18:03 ` José Pekkarinen 2023-10-10 18:03 ` José Pekkarinen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.