Date: Mon, 20 Feb 2023 14:53:21 +0100 From: =?UTF-8?Q?Kornel_Dul=c4=99ba?= <kd@FreeBSD.org> To: Andrew Turner <andrew@fubar.geek.nz> Cc: Mark Millard <marklmi@yahoo.com>, bob prohaska <fbsd@www.zefox.net>, "freebsd-arm@freebsd.org" <freebsd-arm@freebsd.org> Subject: Re: Armv7 panic on -current, rpi2 buildworld Message-ID: <6aafafbd-590e-29c8-d98f-5d776ebff4bc@FreeBSD.org> In-Reply-To: <1572483E-D0CD-4384-BF73-A00FB3640F91@fubar.geek.nz> References: <20230215025741.GA32086@www.zefox.net> <CANCZdfou0s5Xz4_0pPdNSQnFH9qk9NAY=GyB7pBwnVNPvGS4Qw@mail.gmail.com> <A81B272C-8AE0-4DB8-A399-6862A13E4394@yahoo.com> <A137339C-683A-42B6-95CA-0EE5B4156562@yahoo.com> <CCE79CB7-BA79-4682-AC7C-4D5E8EC0A21A@fubar.geek.nz> <85ed9a05-e4ea-2811-1701-b8f2e982e3b1@FreeBSD.org> <1572483E-D0CD-4384-BF73-A00FB3640F91@fubar.geek.nz>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------XrbroFEUeTXKeUEnFtS0aYSO
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
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
>
--------------XrbroFEUeTXKeUEnFtS0aYSO
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>On 2/20/23 14:32, Andrew Turner wrote:<br>
</p>
<blockquote type="cite"
cite="mid:1572483E-D0CD-4384-BF73-A00FB3640F91@fubar.geek.nz">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<br>
<div><br>
<blockquote type="cite">
<div>On 20 Feb 2023, at 13:15, Kornel Dulęba
<a class="moz-txt-link-rfc2396E" href="mailto:kd@FreeBSD.org"><kd@FreeBSD.org></a> wrote:</div>
<br class="Apple-interchange-newline">
<div>
<meta http-equiv="Content-Type" content="text/html;
charset=UTF-8">
<div>
<blockquote type="cite"
cite="mid:CCE79CB7-BA79-4682-AC7C-4D5E8EC0A21A@fubar.geek.nz">
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
Can you try
with 24abb6b82102eec577eff9bd8dd7726e8cab89f4? There
were conditional branch instructions that may mean the
function to save the VFP state was not being run.</blockquote>
<p>I'm currently debugging this and applying
24abb6b82102eec577eff9bd8dd7726e8cab89f4 didn't quite
help.<br>
(I've tested it with dbl_and_ull_via_async that Mark
shared in another thread.)<br>
The root cause is located in vfp_save_state. It's called
during the dump, right before the assert is triggered:<br>
<br>
359 /*<br>
360 * savectx() will be called on panic with
dumppcb as an argument,<br>
361 * dumppcb doesn't have pcb_vfpsaved set, so
set it to save<br>
362 * the VFP registers.<br>
363 */<br>
364 if (pcb->pcb_vfpsaved == NULL)<br>
365 pcb->pcb_vfpsaved =
&pcb->pcb_vfpstate;<br>
<br>
Here pcb_vfpsaved == NULL, since the VFP has never been
used in the kernel.<br>
This triggers the KASSERT in get_vfpcontext, causing the
panic.<br>
Note that arm64 has very similar logic, so I wonder if a
similar panic could be observed there.<br>
Any thoughts?<br>
</p>
</div>
</div>
</blockquote>
<br>
</div>
<div>It looks like cpu_copy_thread is missing setting pcb_vfpsaved
so is likely to be invalid.</div>
<div><br>
</div>
<div>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.<br>
</div>
</blockquote>
<div>Thanks, that's it!<br>
With some changes in cpu_copy_thread the assert is no longer
triggered.<br>
I'll do some more testing, cleanup and will post the patch to
phabricator.<br>
</div>
<blockquote type="cite"
cite="mid:1572483E-D0CD-4384-BF73-A00FB3640F91@fubar.geek.nz">
<div><br>
</div>
<div>Andrew</div>
<br>
</blockquote>
</body>
</html>
--------------XrbroFEUeTXKeUEnFtS0aYSO--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6aafafbd-590e-29c8-d98f-5d776ebff4bc>