Re: [PATCH] kvm/sev: remove redundant MISC_CG_RES_SEV_ES

[Paolo Bonzini]

On 10/10/23 19:49, José Pekkarinen wrote:
> SEV-ES is an extra encrypted state that shares common resources
> with SEV. Using an extra CG for its purpose doesn't seem to
> provide much value. This patch will clean up the control group
> along with multiple checks that become redundant with it.
>
> The patch will also remove a redundant logic on sev initialization
> that produces SEV-ES to be disabled, while supported by the cpu
> and requested by the user through the sev_es parameter.

In what sense is it shared?  The SEV ASIDs and the SEV-ES ASIDs are 
separate (and in both cases limited) resources, and therefore they have 
separate cgroups.

Paolo

> Signed-off-by: José Pekkarinen<jose.pekkarinen@xxxxxxxxxxx>
> ---
>   arch/x86/kvm/svm/sev.c      | 18 +++---------------
>   include/linux/misc_cgroup.h |  2 --
>   2 files changed, 3 insertions(+), 17 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 07756b7348ae..8a06d92187cf 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -37,13 +37,9 @@
>    * this file are not used but this file still gets compiled into the KVM AMD
>    * module.
>    *
> - * We will not have MISC_CG_RES_SEV and MISC_CG_RES_SEV_ES entries in the enum
> - * misc_res_type {} defined in linux/misc_cgroup.h.
> - *
>    * Below macros allow compilation to succeed.
>    */
>   #define MISC_CG_RES_SEV MISC_CG_RES_TYPES
> -#define MISC_CG_RES_SEV_ES MISC_CG_RES_TYPES
>   #endif
>   
>   #ifdef CONFIG_KVM_AMD_SEV
> @@ -125,13 +121,13 @@ static bool __sev_recycle_asids(int min_asid, int max_asid)
>   
>   static int sev_misc_cg_try_charge(struct kvm_sev_info *sev)
>   {
> -	enum misc_res_type type = sev->es_active ? MISC_CG_RES_SEV_ES : MISC_CG_RES_SEV;
> +	enum misc_res_type type = MISC_CG_RES_SEV;
>   	return misc_cg_try_charge(type, sev->misc_cg, 1);
>   }
>   
>   static void sev_misc_cg_uncharge(struct kvm_sev_info *sev)
>   {
> -	enum misc_res_type type = sev->es_active ? MISC_CG_RES_SEV_ES : MISC_CG_RES_SEV;
> +	enum misc_res_type type = MISC_CG_RES_SEV;
>   	misc_cg_uncharge(type, sev->misc_cg, 1);
>   }
>   
> @@ -2167,7 +2163,7 @@ void __init sev_set_cpu_caps(void)
>   void __init sev_hardware_setup(void)
>   {
>   #ifdef CONFIG_KVM_AMD_SEV
> -	unsigned int eax, ebx, ecx, edx, sev_asid_count, sev_es_asid_count;
> +	unsigned int eax, ebx, ecx, edx, sev_asid_count;
>   	bool sev_es_supported = false;
>   	bool sev_supported = false;
>   
> @@ -2236,14 +2232,7 @@ void __init sev_hardware_setup(void)
>   	if (!boot_cpu_has(X86_FEATURE_SEV_ES))
>   		goto out;
>   
> -	/* Has the system been allocated ASIDs for SEV-ES? */
> -	if (min_sev_asid == 1)
> -		goto out;
> -
> -	sev_es_asid_count = min_sev_asid - 1;
> -	WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_SEV_ES, sev_es_asid_count));
>   	sev_es_supported = true;
> -
>   out:
>   	if (boot_cpu_has(X86_FEATURE_SEV))
>   		pr_info("SEV %s (ASIDs %u - %u)\n",
> @@ -2271,7 +2260,6 @@ void sev_hardware_unsetup(void)
>   	bitmap_free(sev_reclaim_asid_bitmap);
>   
>   	misc_cg_set_capacity(MISC_CG_RES_SEV, 0);
> -	misc_cg_set_capacity(MISC_CG_RES_SEV_ES, 0);
>   }
>   
>   int sev_cpu_init(struct svm_cpu_data *sd)
> diff --git a/include/linux/misc_cgroup.h b/include/linux/misc_cgroup.h
> index c238207d1615..23d3cd153f60 100644
> --- a/include/linux/misc_cgroup.h
> +++ b/include/linux/misc_cgroup.h
> @@ -15,8 +15,6 @@ enum misc_res_type {
>   #ifdef CONFIG_KVM_AMD_SEV
>   	/* AMD SEV ASIDs resource */
>   	MISC_CG_RES_SEV,
> -	/* AMD SEV-ES ASIDs resource */
> -	MISC_CG_RES_SEV_ES,
>   #endif
>   	MISC_CG_RES_TYPES
>   };
> --