Re: Armv7 panic on -current, rpi2 buildworld

On 2/20/23 14:32, Andrew Turner wrote:

>
>
>> On 20 Feb 2023, at 13:15, Kornel Dulęba <kd@FreeBSD.org> wrote:
>>
>>> Can you try with 24abb6b82102eec577eff9bd8dd7726e8cab89f4? There 
>>> were conditional branch instructions that may mean the function to 
>>> save the VFP state was not being run.
>>
>> I'm currently debugging this and applying 
>> 24abb6b82102eec577eff9bd8dd7726e8cab89f4 didn't quite help.
>> (I've tested it with dbl_and_ull_via_async that Mark shared in 
>> another thread.)
>> The root cause is located in vfp_save_state. It's called during the 
>> dump, right before the assert is triggered:
>>
>> 359         /*
>> 360          * savectx() will be called on panic with dumppcb as an 
>> argument,
>> 361          * dumppcb doesn't have pcb_vfpsaved set, so set it to save
>> 362          * the VFP registers.
>> 363          */
>> 364         if (pcb->pcb_vfpsaved == NULL)
>> 365                 pcb->pcb_vfpsaved = &pcb->pcb_vfpstate;
>>
>> Here pcb_vfpsaved == NULL, since the VFP has never been used in the 
>> kernel.
>> This triggers the KASSERT in get_vfpcontext, causing the panic.
>> Note that arm64 has very similar logic, so I wonder if a similar 
>> panic could be observed there.
>> Any thoughts?
>>
>
> It looks like cpu_copy_thread is missing setting pcb_vfpsaved so is 
> likely to be invalid.
>
> On arm64 we set the needed data in vfp_new_thread. This was added to 
> simplify adding SVE support, but could be reused on arm if it’s useful.
Thanks, that's it!
With some changes in cpu_copy_thread the assert is no longer triggered.
I'll do some more testing, cleanup and will post the patch to phabricator.
>
> Andrew
>