Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Noah Goldstein <goldstein.w.n@gmail.com>
Cc:     goldstein.w.n@gmail.com, tglx@linutronix.de, mingo@redhat.com,
        bp@alien8.de, x86@kernel.org, hpa@zytor.com, luto@kernel.org,
        linux-kernel@vger.kernel.org
Subject: [PATCH v1] x86/fpu: Remove opmask state from avx512_timestamp check
Date:   Mon, 20 Sep 2021 00:39:53 -0500
Message-Id: <20210920053951.4093668-1-goldstein.w.n@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
To:     unlisted-recipients:; (no To-header on input)
Precedence: bulk

This commit creates a new mask, XFEATURE_MASK_ZMM, to test against
xfeatures for conditionally updating the axx512_timestamp.

Based on the comments, the avx512 state is meant to track when the
state would cause frequencey throttling. The opmasks (k0-k7) do not
cause frequency throttling, so they don't make sense to include.

The current implementation, as well as the old, still does have a
false positive on ymm16-ymm31 and xmm16-31 because
XFEATURE_MASK_Hi16_ZMM includes them.

Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
---
Issue is reproducible with the following code on x86_64:

```
	.global	_start
	.text
_start:
	korq	%k0, %k0, %k0

loop:
	jmp	loop


	movl	$60, %eax
	xorl	%edi, %edi
	syscall
```

Pretending run as executable named "foo":

$> cat /proc/$(pidof foo)/arch_status


This should yield -1 as no frequency changing AVX512 instructions
where used but instead tracks the process.

Note there still is a false positive with ymm16-ymm31 and xmm16-xmm31
but since there is no state to distinguish between there use and
zmm16-31 that seems inevitable.

    
 arch/x86/include/asm/fpu/types.h | 2 ++
 arch/x86/kernel/fpu/core.c       | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index f5a38a5f3ae1..a4816fa7d541 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -141,6 +141,8 @@ enum xfeature {
 #define XFEATURE_MASK_AVX512		(XFEATURE_MASK_OPMASK \
 					 | XFEATURE_MASK_ZMM_Hi256 \
 					 | XFEATURE_MASK_Hi16_ZMM)
+#define XFEATURE_MASK_ZMM		(XFEATURE_MASK_ZMM_Hi256 \
+					 | XFEATURE_MASK_Hi16_ZMM)
 
 #define FIRST_EXTENDED_XFEATURE	XFEATURE_YMM
 
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 7ada7bd03a32..342620a2e8ef 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -104,8 +104,10 @@ void save_fpregs_to_fpstate(struct fpu *fpu)
 		/*
 		 * AVX512 state is tracked here because its use is
 		 * known to slow the max clock speed of the core.
+		 * Note: This has a false positive on Hi16 ymm and
+		 * xmm registers.
 		 */
-		if (fpu->state.xsave.header.xfeatures & XFEATURE_MASK_AVX512)
+		if (fpu->state.xsave.header.xfeatures & XFEATURE_MASK_ZMM)
 			fpu->avx512_timestamp = jiffies;
 		return;
 	}
-- 
2.25.1