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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 4 Aug 2021 12:53:07 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Sean Christopherson <seanjc@...gle.com>,
        Mingwei Zhang <mizhang@...gle.com>
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Tom Lendacky <thomas.lendacky@....com>,
        Marc Orr <marcorr@...gle.com>,
        David Rientjes <rientjes@...gle.com>,
        Alper Gun <alpergun@...gle.com>,
        Dionna Glaze <dionnaglaze@...gle.com>,
        Vipin Sharma <vipinsh@...gle.com>,
        Peter Gonda <pgonda@...gle.com>,
        Brijesh Singh <brijesh.singh@....com>
Subject: Re: [PATCH v2] KVM: SVM: improve the code readability for ASID
 management

On 03/08/21 18:50, Sean Christopherson wrote:
> 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.

It's also missing this (off by one for SEV guests, pointless extra work for
SEV-ES):

index da5b9515a47b..7fbce342eec4 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -147,7 +147,7 @@ static int sev_asid_new(struct kvm_sev_info *sev)
  	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_sev_asid, min_asid);
+	asid = find_next_zero_bit(sev_asid_bitmap, max_asid + 1, min_asid);
  	if (asid > max_asid) {
  		if (retry && __sev_recycle_asids(min_asid, max_asid)) {
  			retry = false;


Queued both Mingwei's patch and yours, thanks (to 5.14-rc in order to avoid conflicts).

Paolo