linux-kernel - Re: [PATCH v2] KVM:SVM: Flush cache only on CPUs running SEV guest

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]

Message-ID: <fd2a75f0-4baa-4d2b-b350-4f38da32c670@amd.com>
Date: Wed, 13 Mar 2024 09:32:06 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Zheyun Shen <szy0127@...u.edu.cn>, Sean Christopherson
 <seanjc@...gle.com>, pbonzini <pbonzini@...hat.com>,
 tglx <tglx@...utronix.de>
Cc: kvm <kvm@...r.kernel.org>, linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] KVM:SVM: Flush cache only on CPUs running SEV guest

On 3/12/24 09:45, Tom Lendacky wrote:
> On 3/6/24 20:14, Zheyun Shen wrote:
>> On AMD CPUs without ensuring cache consistency, each memory page
>> reclamation in an SEV guest triggers a call to wbinvd_on_all_cpus(),
>> thereby affecting the performance of other programs on the host.
>>
>> Typically, an AMD server may have 128 cores or more, while the SEV guest
>> might only utilize 8 of these cores. Meanwhile, host can use qemu-affinity
>> to bind these 8 vCPUs to specific physical CPUs.
>>
>> Therefore, keeping a record of the physical core numbers each time a vCPU
>> runs can help avoid flushing the cache for all CPUs every time.
>>
>> Since the usage of sev_flush_asids() isn't tied to a single VM, we just
>> replace all wbinvd_on_all_cpus() with sev_do_wbinvd() except for that
>> in sev_flush_asids().
>>
>> Signed-off-by: Zheyun Shen <szy0127@...u.edu.cn>
> 
> I'm unable to launch my SEV or SEV-ES guests with this patch (haven't 
> tried an SEV-SNP guest, yet). Qemu segfaults at launch.
> 
> I'll try to dig into what is happening, but not sure when I'll be able to 
> do that at the moment.

Looks like it's the use of get_cpu() without an associated put_cpu() when 
setting the cpumask. I think what you really want to use is just the cpu 
parameter that is passed into pre_sev_run().


> 

>>   void sev_free_vcpu(struct kvm_vcpu *vcpu)
>> @@ -2648,6 +2666,7 @@ void pre_sev_run(struct vcpu_svm *svm, int cpu)
>>       sd->sev_vmcbs[asid] = svm->vmcb;
>>       svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ASID;
>>       vmcb_mark_dirty(svm->vmcb, VMCB_ASID);
>> +    cpumask_set_cpu(get_cpu(), sev_get_wbinvd_dirty_mask(svm->vcpu.kvm));

Just use 'cpu' here....  ^

When I do that I'm able to boot an SEV and SEV-ES guest.

I'll wait for the next version, in case there are other changes, before 
running through our CI for more thorough testing than just a single boot.

Thanks,
Tom

>>   }
>>   #define GHCB_SCRATCH_AREA_LIMIT        (16ULL * PAGE_SIZE)