Re: [PATCH 2/2] s390/fpu: Fix kmsan in fpu_vstl function

Aleksei Nikiforov
2025-11-07T15:34:46Z

On 11/7/25 14:32, Alexander Potapenko wrote:
> On Fri, Nov 7, 2025 at 11:49 AM Heiko Carstens <hca@linux.ibm.com> wrote:
>>
>> On Fri, Nov 07, 2025 at 11:26:50AM +0100, Alexander Potapenko wrote:
>>> On Thu, Nov 6, 2025 at 5:09 PM Aleksei Nikiforov
>>> <aleksei.nikiforov@linux.ibm.com> wrote:
>>>> @@ -409,6 +410,7 @@ static __always_inline void fpu_vstl(u8 v1, u32 index, const void *vxr)
>>>> : [vxr] "=R" (*(u8 *)vxr)
>>>> : [index] "d" (index), [v1] "I" (v1)
>>>> : "memory", "1");
>>>> + instrument_write_after(vxr, size);
>>>> }
>>>
>>> Wouldn't it be easier to just call kmsan_unpoison_memory() here directly?
>>
>> I guess that's your call. Looks like we have already a couple of
>> kmsan_unpoison_memory() behind inline assemblies.
>>
>> So I guess we should either continue using kmsan_unpoison_memory()
>> directly, or convert all of them to such a new helper. Both works of
>> course. What do you prefer?
>
> Upon reflection, I think adding instrument_write_after() is not the best idea.
> For tools like KASAN and KCSAN, every write has the same semantics,
> and the instrumentation just notifies the tool that the write
> occurred.
> For KMSAN, however, writes may affect metadata differently, requiring
> us to either poison or unpoison the destination.
> In certain special cases, like instrument_get_user() or
> instrument_copy_from_user() the semantics are always fixed, but this
> is not true for arbitrary writes.
>
> We could make the new annotation's name more verbose, but it will just
> become a synonym of kmsan_unpoison_memory().
> So I suggest sticking with kmsan_unpoison_memory() for now.
>
>

I'll rework changes with that suggestion. Thank you.