Date: Wed, 7 Sep 2022 15:11:09 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: =?utf-8?Q?T=C4=B3l?= Coosemans <tijl@freebsd.org> Cc: Colin Percival <cperciva@tarsnap.com>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-branches@freebsd.org Subject: Re: git: 5d45f99a1ed8 - stable/13 - i386 doreti: Fix calculation of stack frame size Message-ID: <YxiKXcYh6w860Hlx@kib.kiev.ua> In-Reply-To: <20220907115604.6e5fbfeb@FreeBSD.org> References: <202209021506.282F6Jhx029847@gitrepo.freebsd.org> <01000182feec990a-943b1465-055b-4cb8-984c-225a4feabf7d-000000@email.amazonses.com> <20220906172225.1a4d1064@FreeBSD.org> <YxeA6Y4lD/5DXfcJ@kib.kiev.ua> <20220907115604.6e5fbfeb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Sep 07, 2022 at 11:56:04AM +0200, Tijl Coosemans wrote: > On Tue, 6 Sep 2022 20:18:33 +0300 Konstantin Belousov > <kostikbel@gmail.com> wrote: > > On Tue, Sep 06, 2022 at 05:22:25PM +0200, Tijl Coosemans wrote: > >> On Fri, 2 Sep 2022 15:58:14 +0000 Colin Percival <cperciva@tarsnap.com> > >> wrote: > >>> On 9/2/22 08:06, Tijl Coosemans wrote: > >>>> The branch stable/13 has been updated by tijl: > >>>> > >>>> URL: https://cgit.FreeBSD.org/src/commit/?id=5d45f99a1ed861618d5c4d5b4252d5eba001ad35 > >>>> > >>>> commit 5d45f99a1ed861618d5c4d5b4252d5eba001ad35 > >>>> Author: Tijl Coosemans <tijl@FreeBSD.org> > >>>> AuthorDate: 2022-09-02 14:16:35 +0000 > >>>> Commit: Tijl Coosemans <tijl@FreeBSD.org> > >>>> CommitDate: 2022-09-02 15:02:00 +0000 > >>>> > >>>> i386 doreti: Fix calculation of stack frame size > >>>> > >>>> Reviewed by: kib > >>>> Fixes: e8b2980e4a12 - i386 doreti: stop saving/restoring %ecx > >>>> > >>>> (cherry picked from commit cfdc649e455bc0d37d42c46b59360462e93b4300) > >>> I assume this MFC took place 23 minutes after the original commit because it > >>> was an urgent fix; to avoid the need for assumptions, please indicate the > >>> reason for an accelerated MFC in the commit message when you skip the normal > >>> 3 day waiting period. > >> > >> Will do. In this case it was a trivial fixup in a critical path. At the > >> time I also thought it fixed a panic I was seeing but this turned out to > >> be caused by another commit. > > > > It is not on critical path, at worst it affected vm86 mode only. > > That said, 'the other bug' was there before the series. > > Looking into this again I now believe your change was actually correct. > trampstk = (uintptr_t)tramp_stack_base + TRAMP_STACK_SZ - VM86_STACK_SPACE > So it already has VM86_STACK_SPACE subtracted. Can you double-check > this? The trampstk pointer has the space for additional segment registers values that are pushed by hardware (in C it is at the end of the struct trapframe). But the calculation for %ecx is the size we are going to copy from kstack to trampoline. So when we are returning to vm86 mode, we must account for additional segment registers by increasing the copy size. The trampstk definition you cited means that we do not overflow the destination by the copy. So I think that your fix was correct.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YxiKXcYh6w860Hlx>