[Bug 228755] libvgl under syscons causes system reboot (via SDL 1.2)

[Bruce Evans]

On Wed, 6 Jun 2018, Bruce Evans wrote:

> On Tue, 5 Jun 2018, Mahmoud Al-Qudsi wrote:
>
>> On Tue, Jun 5, 2018 at 7:32 AM, Bruce Evans <brde at optusnet.com.au> wrote:
>>> ...
>>> 1920x1080x8.  I use this fix:
>>> ...
>>> -               if (offset > adp->va_window_size - PAGE_SIZE)
>>> +               if (offset > trunc_page(adp->va_window_size))
>> ...
>> 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?
>
> I think the tests are equivalent under the assumptions that the caller has
> rounded down 'offset' to a page boundary and that the hardware frame buffer
> size is a multiple of the page size.  The trunc_page() test is clearer but
> still cyptic.  The range being checked is from offset to offset + PAGE_SIZE 
> -1
> under the simplifying assumpting that offset has been rounded.  It is the
> end of this range, not the start, that should be compared with the rounded-uo
> buffer size.  But after rounding, '<' means at least 1 page smaller, not just
> at least 1 byte smaller, so we can take the shortcut of replacing
> offset + PAGE_SIZE - 1 by that less about PAGE_SIZE, but it is still not
> obvious that this doesn't have an off-by-1-byte error.

It is my fix that has an off-by-1 error.  If fails to reduce the buffer
size to the offset of the last page in the buffer in the usual case where
the buffer size is a multiple of the page size.  trunc_page(buffer_size - 1)
does that.

Checking the start offsets (reduced to page boundaries) is less clear than
checking the end offsets, but is more robust because in theory an offset
may be for the last page so rounding up to a page boundary overflows to 0.
vm code has to worry about this more than most code, so it usually rounds
down to page boundaries.

I see a bug in the size calculation for kernel-level mappings of the frame
buffer for direct-mapped modes (that is, mainly modes with 15, 16, 24 or
32 planes of colors).  vgl doesn't support these modes (that is one reason
it is slow.  Which modes do you use, and which ones panic?

Bruce