linux-kernel - Re: [PATCH 2/2] KVM: SVM: Do not use user return MSR support for virtualized TSC

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]

Message-ID: <380463c9-30ee-ce7e-32ab-0ed91825bdaa@amd.com>
Date:   Fri, 15 Sep 2023 10:08:40 -0500
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org, x86@...nel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Ingo Molnar <mingo@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Babu Moger <babu.moger@....com>
Subject: Re: [PATCH 2/2] KVM: SVM: Do not use user return MSR support for
 virtualized TSC_AUX

On 9/15/23 09:51, Sean Christopherson wrote:
> On Fri, Sep 15, 2023, Sean Christopherson wrote:
>> On Thu, Sep 14, 2023, Tom Lendacky wrote:
>>> When the TSC_AUX MSR is virtualized, the TSC_AUX value is swap type "B"
>>> within the VMSA. This means that the guest value is loaded on VMRUN and
>>> the host value is restored from the host save area on #VMEXIT.
>>>
>>> Since the value is restored on #VMEXIT, the KVM user return MSR support
>>> for TSC_AUX can be replaced by populating the host save area with current
>>> host value of TSC_AUX. This replaces two WRMSR instructions with a single
>>> RDMSR instruction.
>>>
>>> Signed-off-by: Tom Lendacky <thomas.lendacky@....com>
>>> ---
>>>   arch/x86/kvm/svm/sev.c | 14 +++++++++++++-
>>>   arch/x86/kvm/svm/svm.c | 26 ++++++++++++++++----------
>>>   arch/x86/kvm/svm/svm.h |  4 +++-
>>>   3 files changed, 32 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>>> index 565c9de87c6d..1bbaae2fed96 100644
>>> --- a/arch/x86/kvm/svm/sev.c
>>> +++ b/arch/x86/kvm/svm/sev.c
>>> @@ -2969,6 +2969,7 @@ static void sev_es_init_vmcb_after_set_cpuid(struct vcpu_svm *svm)
>>>   	if (boot_cpu_has(X86_FEATURE_V_TSC_AUX) &&
>>>   	    (guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP) ||
>>>   	     guest_cpuid_has(vcpu, X86_FEATURE_RDPID))) {
>>> +		svm->v_tsc_aux = true;
>>>   		set_msr_interception(vcpu, svm->msrpm, MSR_TSC_AUX, 1, 1);
>>>   		if (guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
>>>   			svm_clr_intercept(svm, INTERCEPT_RDTSCP);
>>> @@ -3071,8 +3072,10 @@ void sev_es_vcpu_reset(struct vcpu_svm *svm)
>>>   					    sev_enc_bit));
>>>   }
>>>   
>>> -void sev_es_prepare_switch_to_guest(struct sev_es_save_area *hostsa)
>>> +void sev_es_prepare_switch_to_guest(struct vcpu_svm *svm, struct sev_es_save_area *hostsa)
>>>   {
>>> +	u32 msr_hi;
>>> +
>>>   	/*
>>>   	 * All host state for SEV-ES guests is categorized into three swap types
>>>   	 * based on how it is handled by hardware during a world switch:
>>> @@ -3109,6 +3112,15 @@ void sev_es_prepare_switch_to_guest(struct sev_es_save_area *hostsa)
>>>   		hostsa->dr2_addr_mask = amd_get_dr_addr_mask(2);
>>>   		hostsa->dr3_addr_mask = amd_get_dr_addr_mask(3);
>>>   	}
>>> +
>>> +	/*
>>> +	 * If TSC_AUX virtualization is enabled, MSR_TSC_AUX is loaded but NOT
>>> +	 * saved by the CPU (Type-B). If TSC_AUX is not virtualized, the user
>>> +	 * return MSR support takes care of restoring MSR_TSC_AUX. This
>>> +	 * exchanges two WRMSRs for one RDMSR.
>>> +	 */
>>> +	if (svm->v_tsc_aux)
>>> +		rdmsr(MSR_TSC_AUX, hostsa->tsc_aux, msr_hi);
>>
>> IIUC, when V_TSC_AUX is supported, SEV-ES guests context switch MSR_TSC_AUX
>> regardless of what has been exposed to the guest.  So rather than condition the
>> hostsa->tsc_aux update on guest CPUID, just do it if V_TSC_AUX is supported.
>>
>> And then to avoid the RDMSR, which is presumably the motivation for checking
>> guest CPUID, grab the host value from user return framework.  The host values
>> are per-CPU, but constant after boot, so the only requirement is that KVM sets
>> up MSR_TSC_AUX in the user return framework.
> 
> Actually, duh.  The save area is also per-CPU, so just fill hostsa->tsc_aux in
> svm_hardware_setup() and then sev_es_prepare_switch_to_guest() never has to do
> anything.

Ah, right, because Linux never changes TSC_AUX post boot. Much simpler.

I'll rework based on the comments and send a v2 series.

Thanks,
Tom