Messages in this thread | | | From | Noah Goldstein <> | Date | Mon, 27 Sep 2021 13:02:34 -0500 | Subject | Re: [PATCH v1] x86/fpu: Remove opmask state from avx512_timestamp check |
| |
On Mon, Sep 20, 2021 at 12:40 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > 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 > Ping
(sorry if this has shown up multiple times for anyone, was accidentally including HTML earlier)
| |