linux-kernel - Re: [PATCH v3 64/75] x86/sev-es: Cache CPUID results for improved performance

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: <3ab38184-59fd-73f8-83ca-3f35c8e391eb@amd.com>
Date:   Wed, 6 May 2020 18:02:54 -0500
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Mike Stunes <mstunes@...are.com>, Joerg Roedel <joro@...tes.org>
Cc:     "x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Hellstrom <thellstrom@...are.com>,
        Jiri Slaby <jslaby@...e.cz>,
        Dan Williams <dan.j.williams@...el.com>,
        Juergen Gross <jgross@...e.com>,
        Kees Cook <keescook@...omium.org>,
        David Rientjes <rientjes@...gle.com>,
        Cfir Cohen <cfir@...gle.com>,
        Erdem Aktas <erdemaktas@...gle.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Joerg Roedel <jroedel@...e.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>
Subject: Re: [PATCH v3 64/75] x86/sev-es: Cache CPUID results for improved
 performance



On 5/6/20 1:08 PM, Mike Stunes wrote:
> 
> 
>> On Apr 28, 2020, at 8:17 AM, Joerg Roedel <joro@...tes.org> wrote:
>>
>> From: Mike Stunes <mstunes@...are.com>
>>
>> To avoid a future VMEXIT for a subsequent CPUID function, cache the
>> results returned by CPUID into an xarray.
>>
>> [tl: coding standard changes, register zero extension]
>>
>> Signed-off-by: Mike Stunes <mstunes@...are.com>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@....com>
>> [ jroedel@...e.de: - Wrapped cache handling into vc_handle_cpuid_cached()
>>                    - Used lower_32_bits() where applicable
>> 		   - Moved cache_index out of struct es_em_ctxt ]
>> Co-developed-by: Joerg Roedel <jroedel@...e.de>
>> Signed-off-by: Joerg Roedel <jroedel@...e.de>
>> ---
>> arch/x86/kernel/sev-es-shared.c |  12 ++--
>> arch/x86/kernel/sev-es.c        | 119 +++++++++++++++++++++++++++++++-
>> 2 files changed, 124 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/kernel/sev-es.c b/arch/x86/kernel/sev-es.c
>> index 03095bc7b563..0303834d4811 100644
>> --- a/arch/x86/kernel/sev-es.c
>> +++ b/arch/x86/kernel/sev-es.c
>> @@ -744,6 +758,91 @@ static enum es_result vc_handle_mmio(struct ghcb *ghcb,
>> 	return ret;
>> }
>>
>> +static unsigned long sev_es_get_cpuid_cache_index(struct es_em_ctxt *ctxt)
>> +{
>> +	unsigned long hi, lo;
>> +
>> +	/* Don't attempt to cache until the xarray is initialized */
>> +	if (!sev_es_cpuid_cache_initialized)
>> +		return ULONG_MAX;
>> +
>> +	lo = lower_32_bits(ctxt->regs->ax);
>> +
>> +	/*
>> +	 * CPUID 0x0000000d requires both RCX and XCR0, so it can't be
>> +	 * cached.
>> +	 */
>> +	if (lo == 0x0000000d)
>> +		return ULONG_MAX;
>> +
>> +	/*
>> +	 * Some callers of CPUID don't always set RCX to zero for CPUID
>> +	 * functions that don't require RCX, which can result in excessive
>> +	 * cached values, so RCX needs to be manually zeroed for use as part
>> +	 * of the cache index. Future CPUID values may need RCX, but since
>> +	 * they can't be known, they must not be cached.
>> +	 */
>> +	if (lo > 0x80000020)
>> +		return ULONG_MAX;
> 
> If the cache is shared across CPUs, do we also need to exclude function 0x1 because it contains the LAPIC ID? (Or is the cache per-CPU?)

It's currently not a per-CPU cache, but given what you pointed out, it 
should be if we're going to keep function 0x1 in there. The question will 
be how often is that CPUID issued, as that would determine if (not) 
caching it matters. Or even how often CPUID is issued and whether the 
xarray lock could be under contention if the cache is not per-CPU.

Thanks,
Tom

>