Date: Sun, 08 Mar 2015 22:00:34 -0700 From: Nathan Whitehorn <nwhitehorn@freebsd.org> To: Mark Millard <markmi@dsl-only.net>, FreeBSD PowerPC ML <freebsd-ppc@freebsd.org>, dumbbell@freebsd.org, ray@ddteam.net Subject: Re: 10.1-STABLE kern.vty=vt crashes PowerMac G5 extremely early in boot for 2560x1440 display... Message-ID: <54FD28F2.8050506@freebsd.org> In-Reply-To: <EEF037EC-CFF2-4D4D-BB1B-0A0750E09C1F@dsl-only.net> References: <8B7B5167-0C94-4BA3-9515-B4CB8A817BDB@dsl-only.net> <EEF037EC-CFF2-4D4D-BB1B-0A0750E09C1F@dsl-only.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Yes, I believe you are correct. That's an obvious bug that should be
fixed. I've CC'ed a couple of the people working on the vt system.
-Nathan
On 03/08/15 21:18, Mark Millard wrote:
> Nathan W. write:
>
>> Could you please try -CURRENT? vt itself at least works fine there, but
>> I'm not sure anyone has tested it with such a large display on any platform.
>> -Nathan
> I'll try. But it may take a bit to get a 11.0-CURRENT boot context established: I've got the G5's doing other things at the moment.
>
> Looking at the -r279514 11.0-CURRENT code (see below) there seems a VBF_STATIC flag for indicating a statically sized initial buffer. But the code does not seem to use the static sized limits involved: instead the code always seems to use the final sizes even before vtbuf_grow toggles the flag's status.
>
> In other words: I expect that this goes goes out of bounds and trashes memory it does not own while the static buffer is in use early on.
>
>
> #ifdef SC_HISTORY_SIZE
> #define VBF_DEFAULT_HISTORY_SIZE SC_HISTORY_SIZE
> #else
> #define VBF_DEFAULT_HISTORY_SIZE 500
> #endif
> ...
> #ifndef VT_FB_DEFAULT_WIDTH
> #define VT_FB_DEFAULT_WIDTH 2048
> #endif
> #ifndef VT_FB_DEFAULT_HEIGHT
> #define VT_FB_DEFAULT_HEIGHT 1200
> #endif
> ...
>
> #define _VTDEFH MAX(100, PIXEL_HEIGHT(VT_FB_DEFAULT_HEIGHT))
> #define _VTDEFW MAX(200, PIXEL_WIDTH(VT_FB_DEFAULT_WIDTH))
> ...
> static term_char_t vt_constextbuf[(_VTDEFW) * (VBF_DEFAULT_HISTORY_SIZE)];
> static term_char_t *vt_constextbufrows[VBF_DEFAULT_HISTORY_SIZE];
> static struct vt_window vt_conswindow = {
> .vw_number = VT_CONSWINDOW,
> .vw_flags = VWF_CONSOLE,
> .vw_buf = {
> .vb_buffer = &vt_constextbuf[0],
> .vb_rows = &vt_constextbufrows[0],
> .vb_history_size = VBF_DEFAULT_HISTORY_SIZE,
> .vb_curroffset = 0,
> .vb_roffset = 0,
> .vb_flags = VBF_STATIC,
> .vb_mark_start = {.tp_row = 0, .tp_col = 0,},
> .vb_mark_end = {.tp_row = 0, .tp_col = 0,},
> .vb_scr_size = {
> .tp_row = _VTDEFH,
> .tp_col = _VTDEFW,
> },
> },
> .vw_device = &vt_consdev,
> .vw_terminal = &vt_consterm,
> .vw_kbdmode = K_XLATE,
> .vw_grabbed = 0,
> };
>
> ...
>
> void
> vtbuf_init_early(struct vt_buf *vb)
> {
> term_rect_t rect;
>
> vb->vb_flags |= VBF_CURSOR;
> vb->vb_roffset = 0;
> vb->vb_curroffset = 0;
> vb->vb_mark_start.tp_row = 0;
> vb->vb_mark_start.tp_col = 0;
> vb->vb_mark_end.tp_row = 0;
> vb->vb_mark_end.tp_col = 0;
>
> vtbuf_init_rows(vb);
> rect.tr_begin.tp_row = rect.tr_begin.tp_col = 0;
> rect.tr_end.tp_col = vb->vb_scr_size.tp_col;
> rect.tr_end.tp_row = vb->vb_history_size;
> vtbuf_fill(vb, &rect, VTBUF_SPACE_CHAR(TERMINAL_NORM_ATTR));
> vtbuf_make_undirty(vb);
> if ((vb->vb_flags & VBF_MTX_INIT) == 0) {
> mtx_init(&vb->vb_lock, "vtbuf", NULL, MTX_SPIN);
> vb->vb_flags |= VBF_MTX_INIT;
> }
> }
> ...
> static void
> vtbuf_fill(struct vt_buf *vb, const term_rect_t *r, term_char_t c)
> {
> unsigned int pr, pc;
> term_char_t *row;
>
> for (pr = r->tr_begin.tp_row; pr < r->tr_end.tp_row; pr++) {
> row = vb->vb_rows[(vb->vb_curroffset + pr) %
> VTBUF_MAX_HEIGHT(vb)];
> for (pc = r->tr_begin.tp_col; pc < r->tr_end.tp_col; pc++) {
> row[pc] = c;
> }
> }
> }
> ...
> static void
> vtbuf_init_rows(struct vt_buf *vb)
> {
> int r;
>
> vb->vb_history_size = MAX(vb->vb_history_size, vb->vb_scr_size.tp_row);
>
> for (r = 0; r < vb->vb_history_size; r++)
> vb->vb_rows[r] = &vb->vb_buffer[r * vb->vb_scr_size.tp_col];
> }
> ...
> void
> vtbuf_init(struct vt_buf *vb, const term_pos_t *p)
> {
> int sz;
>
> vb->vb_scr_size = *p;
> vb->vb_history_size = VBF_DEFAULT_HISTORY_SIZE;
>
> if ((vb->vb_flags & VBF_STATIC) == 0) {
> sz = vb->vb_history_size * p->tp_col * sizeof(term_char_t);
> vb->vb_buffer = malloc(sz, M_VTBUF, M_WAITOK | M_ZERO);
>
> sz = vb->vb_history_size * sizeof(term_char_t *);
> vb->vb_rows = malloc(sz, M_VTBUF, M_WAITOK | M_ZERO);
> }
>
> vtbuf_init_early(vb);
> }
> ...
> void
> vtbuf_grow(struct vt_buf *vb, const term_pos_t *p, unsigned int history_size)
> {
> ...
> vb->vb_history_size = history_size;
> vb->vb_buffer = new;
> vb->vb_rows = rows;
> vb->vb_flags &= ~VBF_STATIC;
> vb->vb_scr_size = *p;
> vtbuf_init_rows(vb);
> ...
>
>
>
> ===
> Mark Millard
> markmi@dsl-only.net
>
> On 2015-Mar-8, at 03:22 AM, Mark Millard <markmi@dsl-only.net> wrote:
>
> Basic context:
>
> $ freebsd-version -ku; uname -a
> 10.1-STABLE
> 10.1-STABLE
> FreeBSD FBSDG5S0 10.1-STABLE FreeBSD 10.1-STABLE #0 r279507M: Fri Mar 6 23:08:59 PST 2015 root@FBSDG5S0:/usr/obj/usr/src/sys/GENERIC64vtsc powerpc
>
>
> The crash details (X11 is not part of the context, just console use):
>
> I tried using a 2560x1440 display with a PowerMac G5 but forgot to switch from kern.vty=vt to kern.vty=sc in /boot/loader.conf first.
>
> But the result was new since the last time I'd done such a thing (long ago)...
>
> It crashed so early that it returned to Apple's OpenFirmware.
>
> That allowed me to use .registers in OpenFirmware to find where it had crashed at.
>
> It turned out that the Interrupt Vector was 0x700 and SRR0 was 0x380. (Openfirmare does not put an exception handler at 0x380 (leaving zeros) so a 0x380 exception handler's attempted use leads to a double fault when OpenFirmware's handlers are all that are in place, the 2nd of the pair going to the 0x700 exception handler.)
>
> Luckily LR is preserved in this sequence and it ended up pointing to:
>
> vtbuf_init_early+0x78 (0x40787c was the address for the build)
>
> and that was the instruction after a bl to .vtbuf_fill.
>
> This was fully repeatable.
>
>
> As conformation of the size dependency: Switching to a smaller display booted fine (again fully repeatable).
>
>
> I'll note that kern.vty=sc handles the 2560x1440 display fine for that same PowerMac G5. sc uses most but not all of the width and it uses all of the height.
>
>
> Other context details:
>
> $ cd /usr/src
> $ svnlite info
> Path: .
> Working Copy Root Path: /usr/src
> URL: https://svn0.us-west.freebsd.org/base/stable/10
> Relative URL: ^/stable/10
> Repository Root: https://svn0.us-west.freebsd.org/base
> Repository UUID: ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
> Revision: 279507
> Node Kind: directory
> Schedule: normal
> Last Changed Author: ngie
> Last Changed Rev: 279507
> Last Changed Date: 2015-03-01 14:12:24 -0800 (Sun, 01 Mar 2015)
>
> $ svnlite st --no-ignore
> ? .snap
> ? restoresymtable
> M sys/ddb/db_main.c
> M sys/ddb/db_script.c
> I sys/powerpc/conf/GENERIC64vtsc
> I sys/powerpc/conf/GENERICvtsc
> M sys/powerpc/ofw/ofw_machdep.c
> M sys/powerpc/ofw/ofwcall64.S
> M sys/powerpc/powerpc/dump_machdep.c
>
> sys/powerpc/ofw/ofw_machdep.c has a PowerMac G5 specific change to avoid intermittent boot problems.
>
> sys/ddb/... and sys/powerpc/ofw/ofwcall64.S are just to help me get evidence if I do end up with another early-boot failure. DDB and GDB are listed in sys/powerpc/conf/GENERIC64vtsc for the same reason.
>
> sys/powerpc/powerpc/dump_machdep.c is from me forcing the DMA transfer size for dumps to be small enough not to be rejected as too large of a DMA request size.
>
> sys/powerpc/conf/GENERIC64vtsc turns off ps3 in order to turn on both vt and sc. It includes the standard GENERIC64.
>
> # more /etc/make.conf
> #CPP=clang-cpp
> #CC=clang
> #CXX=clang++
> WRKDIRPREFIX=/usr/obj/portswork
> WITH_DEBUG=
> MALLOC_PRODUCTION=
>
> # more /etc/src.conf
> #CPP=clang-cpp
> #CC=clang
> #CXX=clang++
> #CFLAGS+=-DELF_VERBOSE
> #WITH_DEBUG_FILES=
> #WITHOUT_CLANG
>
> ===
> Mark Millard
> markmi at dsl-only.net
>
>
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?54FD28F2.8050506>