Bug 191564 - invalid memory access in UEFI efifb vt(4) driver
Summary: invalid memory access in UEFI efifb vt(4) driver
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: amd64 Any
: --- Affects Some People
Assignee: Marcel Moolenaar
URL:
Keywords: uefi, vt
Depends on:
Blocks:
 
Reported: 2014-07-02 17:52 UTC by Ed Maste
Modified: 2015-09-01 17:50 UTC (History)
7 users (show)

See Also:


Attachments
screenshot of UEFI vt(4) boot (10.53 KB, image/png)
2014-07-02 17:52 UTC, Ed Maste
no flags Details
Proposed fix (19.43 KB, patch)
2015-08-10 21:34 UTC, Marcel Moolenaar
no flags Details | Diff
efi screenshot (315.66 KB, image/jpeg)
2015-08-11 23:17 UTC, Brad Davis
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Maste freebsd_committer 2014-07-02 17:52:34 UTC
Created attachment 144346 [details]
screenshot of UEFI vt(4) boot

The UEFI loader uses UEFI's console services for I/O; the vt_efifb driver takes over for vt(4)'s framebuffer when the kernel boots, but does not clear the screen first, so some of the previous output remains on screen until it is scrolled off.

See the attached screenshot - the loader's output finished with "Start @  0xffff...." at the bottom, and the kernel's output started with "GDB: no debug ports present" at the top.
Comment 1 Aleksandr Rybalko freebsd_committer 2014-07-11 14:40:40 UTC
Hi!

Looks it is not first time :)

vt_efifb driver 3 times try to clear screen :)

1.
        /* blank full size */
        len = info->fb_size / 4;
        for (i = 0; i < len; i++) {
                ((uint32_t *)info->fb_vbase)[i] = 0;
        }

2. vt_fb_blank(vd, TC_BLACK); inside of vt_fb_init called from vt_efifb_init
3. vt_fb_blank(vd, TC_BLACK); inside of vt_efifb_init

If cases #2 and #3 depend on many things, but case #1 is explicit enough, just put zeros to fb_vbase until < fb_size.

Looks like it is still related to wrong info.
Comment 2 Ed Maste freebsd_committer 2014-07-12 14:37:49 UTC
> Looks like it is still related to wrong info.

Indeed, although I can't figure out where new/correct info comes from.  We clearly do get correct info, text is correctly output later on.
Comment 3 Dave Cottlehuber freebsd_committer 2014-07-28 09:29:55 UTC
FWIW I see similar issues during install on apple macbook pro (amd64, early 2011, intel + amd radeon graphics cards). Anything I can help with?
Comment 4 Ed Maste freebsd_committer 2014-10-06 16:22:04 UTC
Still present at r272323+df7f946 with efifb (this is after the recent large set of vt(4) fixes).
Comment 5 Ed Maste freebsd_committer 2014-11-17 22:21:58 UTC
Perhaps related to efifb's use of DMAP before it's available?
i.e. see https://svnweb.freebsd.org/base?view=revision&revision=274382
Comment 6 Marcel Moolenaar freebsd_committer 2015-08-10 21:34:45 UTC
Created attachment 159740 [details]
Proposed fix
Comment 7 Ed Maste freebsd_committer 2015-08-11 14:24:42 UTC
The proposed fix address the issue here.
Comment 8 Glen Barber freebsd_committer 2015-08-11 17:35:22 UTC
The proposed patch allows my laptop (ThinkPad T540p) to boot UEFI with efifb.
Comment 9 Dave Cottlehuber freebsd_committer 2015-08-11 17:46:17 UTC
I'm keen to try this but I've not built my own -CURRENT release directly. Is it possible somebody can make a .iso or .img available?
Comment 10 Ed Maste freebsd_committer 2015-08-11 21:32:30 UTC
(In reply to Dave Cottlehuber from comment #9)
> I'm keen to try this but I've not built my own -CURRENT release directly. Is it > possible somebody can make a .iso or .img available?

I have a test image available and will leave it here for the next week or so:
http://people.freebsd.org/~emaste/emaste-staging.img.xz
(sha256 9b0f407bbbb6cdc1e580f282f885dd1f7e66c5f17d6ea6e2afd8b69c1f7406f4)

You should be able to dd it to a USB stick and then test booting with it.

Note that this is from my work tree so has a few other changes as well. Also, this is very likely not the final fix but will confirm the problem and fix.
Comment 11 Sydney Meyer 2015-08-11 21:57:46 UTC
This image boots fine on my Mac Mini 3,1 (2009).
Comment 12 Brad Davis freebsd_committer 2015-08-11 23:17:21 UTC
Created attachment 159789 [details]
efi screenshot

This is a screenshot of what I see when I try to EFI boot FreeBSD on a MacBook Pro 6,2 with this patch.
Comment 13 Marcel Moolenaar freebsd_committer 2015-08-11 23:34:11 UTC
(In reply to Brad Davis from comment #12)

The photo shows that your loader was used and thus that it's unclear whether Ed's images was booted or not, unless of course Ed's image has a loader built by brd@hive.den.so14k.com...

I think clarification and/or confirmation is in order.
Comment 14 Brad Davis freebsd_committer 2015-08-12 00:05:24 UTC
(In reply to Marcel Moolenaar from comment #13)

Ed had me set an IP address in rc.conf and the machine does boot, just no video.

That is from a kernel w/ the patch and an older loader that I built.
Comment 15 Baptiste Daroussin freebsd_committer 2015-08-12 06:18:50 UTC
I can confirm that allows me to boot in EFI mode! Thank you very much.

Tested on Lenovo S20-30!
Comment 16 Dave Cottlehuber freebsd_committer 2015-08-12 13:17:38 UTC
(In reply to Ed Maste from comment #10)
- no change for me unfortunately
- similar output as Brad's https://bugs.freebsd.org/bugzilla/attachment.cgi?id=159789
I can grab a shot if that's useful. thanks Ed for posting the image!
Comment 17 Dave Cottlehuber freebsd_committer 2015-08-12 13:19:04 UTC
(In reply to Dave Cottlehuber from comment #16)
urrgh.

iMac 27" 2013/2014 model, same box as reported in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=193745
Comment 18 commit-hook freebsd_committer 2015-08-12 15:26:51 UTC
A commit references this bug:

Author: marcel
Date: Wed Aug 12 15:26:35 UTC 2015
New revision: 286667
URL: https://svnweb.freebsd.org/changeset/base/286667

Log:
  Better support memory mapped console devices, such as VGA and EFI
  frame buffers and memory mapped UARTs.

  1.  Delay calling cninit() until after pmap_bootstrap(). This makes
      sure we have PMAP initialized enough to add translations. Keep
      kdb_init() after cninit() so that we have console when we need
      to break into the debugger on boot.
  2.  Unfortunately, the ATPIC code had be moved as well so as to
      avoid a spurious trap #30. The reason for which is not known
      at this time.
  3.  In pmap_mapdev_attr(), when we need to map a device prior to the
      VM system being initialized, use virtual_avail as the KVA to map
      the device at. In particular, avoid using the direct map on amd64
      because we can't demote by virtue of not being able to allocate
      yet. Keep track of the translation.
      Re-use the translation after the VM has been initialized to not
      waste KVA and to satisfy the assumption in uart(4) that the handle
      returned for the low-level console is the same as later returned
      when the device is probed and attached.
  4.  In pmap_unmapdev() remove the mapping from the table when called
      pre-init. Otherwise keep the mapping. During bus probe and attach
      device resources are mapped and unmapped multiple times, which
      would have us destroy the mapping used by the low-level console.
  5.  In pmap_init(), set pmap_initialized to signal that we're not
      pre-init anymore. On amd64, bring the direct map in sync with the
      translations created at that time.
  6.  Implement bus_space_map() and bus_space_unmap() for real: when
      the tag corresponds to memory space, call the corresponding
      pmap_mapdev() and pmap_unmapdev() functions to construct and
      actual handle.
  7.  In efifb.c and vt_vga.c, remove the crutches and hacks and simply
      call pmap_mapdev_attr() or bus_space_map() as desired.

  Notes:
  1.  uart(4) already used bus_space_map() during low-level console
      setup but since serial ports have traditionally been I/O port
      based, the lack of a proper implementation for said function
      was not a problem. It has always supported memory mapped UARTs
      for low-level consoles by setting hw.uart.console accordingly.
  2.  The use of the direct map on amd64 without setting caching
      attributes has been a bigger problem than previously thought.
      This change has the fortunate (and unexpected) side-effect of
      fixing various EFI frame buffer problems (though not all).

  PR: 191564, 194952

  Special thanks to:
  1.  XipLink, Inc -- generously donated an Intel Bay Trail E3800
      based eval board (ADLE3800PC).
  2.  The FreeBSD Foundation, in particular emaste@ -- for UEFI
      support in general and testing.
  3.  Everyone who tested the proposed for PR 191564.
  4.  jhb@ and kib@ for being a soundboard and applying a clue bat
      if so needed.

Changes:
  head/sys/amd64/amd64/machdep.c
  head/sys/amd64/amd64/pmap.c
  head/sys/conf/files.amd64
  head/sys/conf/files.i386
  head/sys/dev/vt/hw/efifb/efifb.c
  head/sys/dev/vt/hw/vga/vt_vga.c
  head/sys/i386/i386/machdep.c
  head/sys/i386/i386/pmap.c
  head/sys/x86/include/bus.h
  head/sys/x86/x86/bus_machdep.c
Comment 19 Marcel Moolenaar freebsd_committer 2015-08-12 16:06:57 UTC
This is believed to be fixed for everybody in FreeBSD -current as of today (revision 286667). Please test at your earliest convenience.
Comment 20 Sydney Meyer 2015-08-16 19:38:20 UTC
I have created a USB memstick from r286712 and it's booting fine on the following devices:

Mac Mini 3,1 (Late 2009) - Intel Core2Duo P8700 & NVidia 9400M.
Macbook Pro 8,2 (Early 2011) - Intel Core i7 2720QM & Intel HD 3000 + AMD Radeon HD 6750M.
Macbook Pro 11,1 (Mid 2014) - Intel Core i5 4278U + Intel Iris 5100.

Thanks for working on this.
Comment 21 Sydney Meyer 2015-08-17 17:09:21 UTC
Also a Macbook Air 6,2 (Early 2014) - Intel Core i5 4260U & Intel HD 5000 boots this image alright.

If someone would like to give this a spin, until the next -current snapshots are out:

http://188.226.176.105/f11-r286712-uefi-usb.img.xz
(sha256 fa1d4ed4af09d0d92707049864b7e2412eba6c6818bb724f81070c22d0101b61)
Comment 22 commit-hook freebsd_committer 2015-08-25 14:40:14 UTC
A commit references this bug:

Author: marcel
Date: Tue Aug 25 14:39:44 UTC 2015
New revision: 287126
URL: https://svnweb.freebsd.org/changeset/base/287126

Log:
  MFC r286667 & r286723

  Better support memory mapped console devices, such as VGA and EFI
  frame buffers and memory mapped UARTs.

  PR:		191564, 194952, 202276

Changes:
_U  stable/10/
  stable/10/sys/amd64/amd64/machdep.c
  stable/10/sys/amd64/amd64/pmap.c
  stable/10/sys/conf/files.amd64
  stable/10/sys/conf/files.i386
  stable/10/sys/dev/vt/hw/efifb/efifb.c
  stable/10/sys/dev/vt/hw/vga/vt_vga.c
  stable/10/sys/dev/vt/hw/vga/vt_vga_reg.h
  stable/10/sys/i386/i386/machdep.c
  stable/10/sys/i386/i386/pmap.c
  stable/10/sys/x86/include/bus.h
  stable/10/sys/x86/x86/bus_machdep.c
Comment 23 Brad Davis freebsd_committer 2015-08-25 14:47:05 UTC
It also fixes my MacBook Pro 6,2. Thanks Marcel!