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>