Re: [PATCH v3 01/13] x86/fpu/xstate: Avoid getting xstate address of init_fpstate if fpstate contains the component

From: Thomas Gleixner
Date: Wed Feb 22 2023 - 03:38:09 EST


On Tue, Feb 21 2023 at 19:05, Chang S. Bae wrote:
> On 2/21/2023 8:36 AM, Mingwei Zhang wrote:
>> @@ -1151,10 +1152,11 @@ void __copy_xstate_to_uabi_buf(struct membuf to, struct fpstate *fpstate,
>> pkru.pkru = pkru_val;
>> membuf_write(&to, &pkru, sizeof(pkru));
>> } else {
>> - copy_feature(header.xfeatures & BIT_ULL(i), &to,
>> - __raw_xsave_addr(xsave, i),
>> - __raw_xsave_addr(xinit, i),
>> - xstate_sizes[i]);
>> + xsave_addr = (header.xfeatures & BIT_ULL(i)) ?
>> + __raw_xsave_addr(xsave, i) :
>> + __raw_xsave_addr(xinit, i);
>> +
>> + membuf_write(&to, xsave_addr, xstate_sizes[i]);
>> }
>> /*
>> * Keep track of the last copied state in the non-compacted
>
> So this hunk is under for_each_extended_xfeature(i, mask) -- it skips
> the copy routine if mask[i] == 0; instead, it fills zeros.
>
> We have this [1]:
>
> if (fpu_state_size_dynamic())
> mask &= (header.xfeatures | xinit->header.xcomp_bv);
>
> If header.xfeatures[18] = 0 then mask[18] = 0 because
> xinit->header.xcomp_bv[18] = 0. Then, it won't hit that code. So, I'm
> confused about the problem that you described here.

Read the suggested changelog I wrote in my reply to Mingwei.

TLDR:

xsave.header.xfeatures[18] = 1
xinit.header.xfeatures[18] = 0
-> mask[18] = 1
-> __raw_xsave_addr(xsave, 18) <- Success
-> __raw_xsave_addr(xinit, 18) <- WARN

Thanks,

tglx