Date: Tue, 5 Jun 2018 10:01:46 -0500 From: Mahmoud Al-Qudsi <mqudsi@neosmart.net> To: Bruce Evans <brde@optusnet.com.au> Cc: kib@freebsd.org, bugs@freebsd.org Subject: Re: [Bug 228755] libvgl under syscons causes system reboot (via SDL 1.2) Message-ID: <CACcTrKfNhhWEjXsnV--uyNd3xYD2T5a9x4tE7qrKT=ztaau83w@mail.gmail.com> In-Reply-To: <20180605213242.K3458@besplex.bde.org> References: <bug-228755-227@https.bugs.freebsd.org/bugzilla/> <20180605175705.Q2604@besplex.bde.org> <20180605213242.K3458@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Jun 5, 2018 at 7:32 AM, Bruce Evans <brde@optusnet.com.au> wrote:
> This is the driver getting the check wrong for all frame buffer sizes that
> are not a multiple of PAGE_SIZE. The frame buffer size is not a multiple
> of PAGE SIZE mainly for the interesting geometries 800x600x8 and
> 1920x1080x8. I use this fix:
>
> Index: vesa.c
> ===================================================================
> --- vesa.c (revision 334595)
> +++ vesa.c (working copy)
> @@ -1642,7 +1642,7 @@
> (adp->va_info.vi_flags & V_INFO_LINEAR) != 0) {
> /* va_window_size == va_buffer_size/vi_planes */
> /* XXX: is this correct? */
> - if (offset > adp->va_window_size - PAGE_SIZE)
> + if (offset > trunc_page(adp->va_window_size))
> return (-1);
> *paddr = adp->va_info.vi_buffer + offset;
> #ifdef VM_MEMATTR_WRITE_COMBINING
>
> After fixing this, even 1920x1080x8 works.
I submitted a (different) patch [0] that we've been using in production since
2014 for that particular code multiple times, most recently in January of 2018
[1]. It's not a libvgl-specific issue and honestly deserves to be fixed
already. With it, we've been able to use sc as a pretty much bulletproof X
target, with the caveat that it's insanely slow at higher (native) resolutions.
This particular issue is actually already tracked in kern/162373, the same
patch was submitted there by Mikhail Kupchik way back in November of 2011.
Since you seem to know what you're doing - is the comparison with
`trunc_page(adp->va_window_size)` here the way to go or the alternative
approach with `offset >= adp->va_info.vi_buffer_size` range checking instead?
Mahmoud Al-Qudsi
NeoSmart Technologies
[0]: https://github.com/neosmart/freebsd/commit/b3af2d1ed4a63fe1a5d771ed3a052242250098d7
[1]: http://freebsd.1045724.x6.nabble.com/PATCH-validated-against-50k-i686-machines-for-vmparam-h-and-vesa-c-td6235962.html
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACcTrKfNhhWEjXsnV--uyNd3xYD2T5a9x4tE7qrKT=ztaau83w>