linux-kernel - Re: [PATCH] KVM: SVM: Remove TSS reloading code after VMEXIT

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 18 May 2023 10:45:34 -0700
From:   Sean Christopherson <seanjc@...gle.com>
To:     Mingwei Zhang <mizhang@...gle.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Venkatesh Srinivas <venkateshs@...gle.com>,
        Jim Mattson <jmattson@...gle.com>
Subject: Re: [PATCH] KVM: SVM: Remove TSS reloading code after VMEXIT

On Thu, May 18, 2023, Mingwei Zhang wrote:
> Remove TSS reloading code after VMEXIT since upstream KVM after [1] has
> already been using VMLOAD to load host segment state (including TSS).
> Therefore, reload_tss() becomes redundant. Because of that, also remove the
> relevant data field tss_desc in svm_cpu_data as well as its data structure
> definition.
> 
> [1] commit e79b91bb3c91 ("KVM: SVM: use vmsave/vmload for saving/restoring additionalhost state")

This should be

Fixes: e79b91bb3c91 ("KVM: SVM: use vmsave/vmload for saving/restoring additional host state")

to make it clear that the code could have, and should have, been removed by that
commit.

Can you also explain what happens with the TSS busy bit?  I'm staring at a comically
long internal discussion about this patch, I would likely to capture the important
bits in the changelog.  Doesn't have to be super verbose, e.g. just an explanation
that makes it abundantly clear reload_tss() is fully redundant.

> Reported-by: Venkatesh Srinivas <venkateshs@...gle.com>
> Suggested-by: Jim Mattson <jmattson@...gle.com>
> Tested-by: Mingwei Zhang <mizhang@...gle.com>

Heh, you wrote the code and sent the patch, so it darn well better be tested :-)
There are scenarios where a Tested-by for the _original_ author is warranted, e.g.
if someone else tweaked and reposted the patch.  But in this case, there's no need.