Re: [PATCH v2] KVM: SVM: improve the code readability for ASID management

[Sean Christopherson]

+Tom and Brijesh

On Mon, Aug 02, 2021, Mingwei Zhang wrote:
> KVM SEV code uses bitmaps to manage ASID states. ASID 0 was always skipped
> because it is never used by VM. Thus, in existing code, ASID value and its
> bitmap postion always has an 'offset-by-1' relationship.
> 
> Both SEV and SEV-ES shares the ASID space, thus KVM uses a dynamic range
> [min_asid, max_asid] to handle SEV and SEV-ES ASIDs separately.
> 
> Existing code mixes the usage of ASID value and its bitmap position by
> using the same variable called 'min_asid'.
> 
> Fix the min_asid usage: ensure that its usage is consistent with its name;
> allocate extra size for ASID 0 to ensure that each ASID has the same value
> with its bitmap position. Add comments on ASID bitmap allocation to clarify
> the size change.
> 
> v1 -> v2:
>  - change ASID bitmap size to incorporate ASID 0 [sean]
>  - remove the 'fixes' line in commit message. [sean/joerg]
> 
> Signed-off-by: Mingwei Zhang <mizhang@xxxxxxxxxx>
> Cc: Tom Lendacky <thomas.lendacky@xxxxxxx>
> Cc: Marc Orr <marcorr@xxxxxxxxxx>
> Cc: David Rientjes <rientjes@xxxxxxxxxx>
> Cc: Alper Gun <alpergun@xxxxxxxxxx>
> Cc: Dionna Glaze <dionnaglaze@xxxxxxxxxx>
> Cc: Sean Christopherson <seanjc@xxxxxxxxxx>
> Cc: Vipin Sharma <vipinsh@xxxxxxxxxx>
> Cc: Peter Gonda <pgonda@xxxxxxxxxx>
> Cc: Joerg Roedel <joro@xxxxxxxxxx>
> ---

...

> @@ -156,11 +157,11 @@ static int sev_asid_new(struct kvm_sev_info *sev)
>  		goto e_uncharge;
>  	}
>  
> -	__set_bit(pos, sev_asid_bitmap);
> +	__set_bit(asid, sev_asid_bitmap);


This patch missed sev_asid_free(). 

And on a very related topic, I'm pretty sure the VMCB+ASID invalidation logic
indexes sev_vmcbs incorrectly.  pre_sev_run() indexes sev_vmcbs by the ASID,
whereas sev_asid_free() indexes by ASID-1, i.e. on free KVM nullifies the wrong
sev_vmcb entry.  sev_cpu_init() allocates for max_sev_asid+1, so indexing by
ASID appears to be the intended behavior.  That code is also a good candidate for
conversion to nr_asids in this patch.

For the sev_vmcbs bug: