Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 10 Mar 2015 14:52:55 +0200
From:      Aleksandr Rybalko <ray@ddteam.net>
To:        Nathan Whitehorn <nwhitehorn@freebsd.org>
Cc:        FreeBSD PowerPC ML <freebsd-ppc@freebsd.org>, dumbbell@freebsd.org, Mark Millard <markmi@dsl-only.net>
Subject:   Re: 10.1-STABLE kern.vty=vt crashes PowerMac G5 extremely early in boot for 2560x1440 display...
Message-ID:  <20150310145255.c59c23381173c310229c9d82@ddteam.net>
In-Reply-To: <54FD28F2.8050506@freebsd.org>
References:  <8B7B5167-0C94-4BA3-9515-B4CB8A817BDB@dsl-only.net> <EEF037EC-CFF2-4D4D-BB1B-0A0750E09C1F@dsl-only.net> <54FD28F2.8050506@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 08 Mar 2015 22:00:34 -0700
Nathan Whitehorn <nwhitehorn@freebsd.org> wrote:

> 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
> >
> >
> >
> 

Hi guys!

Mark, can you please try to add (or replace)
options      SC_HISTORY_SIZE=1000
to your kernel config?

Looks like, it is history size problem and we need to set minimum size
of history to at least (screen_height * 2) and check to not "grow"
under this size.

Thanks!

WBW
-- 
Aleksandr Rybalko <ray@ddteam.net>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150310145255.c59c23381173c310229c9d82>