linux-kernel - Re: [PATCH RFC 5/7] kvm: x86: Revise CPUID.D.1.EBX for alignment rule

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: <YKwagHvhqiY1rrAI@google.com>
Date:   Mon, 24 May 2021 21:28:32 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Jing Liu <jing2.liu@...ux.intel.com>
Cc:     pbonzini@...hat.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, jing2.liu@...el.com
Subject: Re: [PATCH RFC 5/7] kvm: x86: Revise CPUID.D.1.EBX for alignment rule

On Sun, Feb 07, 2021, Jing Liu wrote:
> CPUID.0xD.1.EBX[1] is set if, when the compacted format of an XSAVE
> area is used, this extended state component located on the next
> 64-byte boundary following the preceding state component (otherwise,
> it is located immediately following the preceding state component).
> 
> AMX tileconfig and tiledata are the first to use 64B alignment.
> Revise the runtime cpuid modification for this rule.
> 
> Signed-off-by: Jing Liu <jing2.liu@...ux.intel.com>
> ---
>  arch/x86/kvm/cpuid.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 04a73c395c71..ee1fac0a865e 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -35,12 +35,17 @@ static u32 xstate_required_size(u64 xstate_bv, bool compacted)
>  {
>  	int feature_bit = 0;
>  	u32 ret = XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET;
> +	bool is_aligned = false;
>  
>  	xstate_bv &= XFEATURE_MASK_EXTEND;
>  	while (xstate_bv) {
>  		if (xstate_bv & 0x1) {
>  		        u32 eax, ebx, ecx, edx, offset;
>  		        cpuid_count(0xD, feature_bit, &eax, &ebx, &ecx, &edx);
> +			/* ECX[2]: 64B alignment in compacted form */
> +			is_aligned = !!(ecx & 2);
> +			if (is_aligned && compacted)

I'd forego the local is_aligned, and also check "compacted" first so that the
uncompacted variant isn't required to evaluated ecx.

And the real reason I am responding... can you post this as a standalone patch?
I stumbled across the "aligned" flag when reading through the SDM for a completely
unrelated reason, and also discovered that the flag has been documented since
2016.  While AMX may be the first to "officially" utilize the alignment flag,
the flag itself is architectural and not strictly limited to AMX.

> +				ret = ALIGN(ret, 64);
>  			offset = compacted ? ret : ebx;
>  			ret = max(ret, offset + eax);
>  		}
> -- 
> 2.18.4
>