Bug 238794

Summary: LAPIC register test failure with OpenBSD 6.5-snapshot guest
Product: Base System Reporter: Jason Tubnor <jason>
Component: bhyveAssignee: freebsd-virtualization mailing list <virtualization>
Status: Closed FIXED    
Severity: Affects Some People CC: emaste, jhb, markj, rgrimes
Priority: --- Keywords: needs-patch, needs-qa
Version: 11.3-RELEASEFlags: koobs: mfc-stable11?
koobs: mfc-stable12?
Hardware: amd64   
OS: Any   
URL: https://reviews.FreeBSD.org/D20755

Description Jason Tubnor 2019-06-25 03:26:32 UTC
OpenBSD has just imported LLVM 8.0 into -current and the way the compiler builds code to test the LAPIC register has changed (though still valid according to Intel documentation).

guenther@ from the OpenBSD project provided some insights based on information provided.  Below is an extract from the email thread:

> Below is output from bhyve.log:
> 
> rdmsr to register 0xc80 on vcpu 0
> Failed to emulate instruction [0xf7 0x04 0x25 0x00 0xd3 0xd1 0x81 0x00 0x10
> 0x00 0x00 0x74 0x08 0xf3 0x90] at 0xffffffff817648f0

According to objdump -d, that's:
 f7 04 25 00 d3 d1 81    testl  $0x1000,0xffffffff81d1d300
 00 10 00 00
 74 08                   je     <forward some>
 f3 90                   pause

That's testing the LAPIC ICRLO, a memory-mapped register.  Previously, the 
compiler generated code like this:

 8b 0c 25 00 63 cf 81    mov    0xffffffff81cf6300,%ecx
 f7 c1 00 10 00 00       test   $0x1000,%ecx
 74 09                   je     <forward some>
 f3 90                   pause  

where it loaded the LAPIC register into %ecx and then tested that value; 
now it combines them and does a direct test.  Congrats, that's legal 
according to Intel (reportedly, Windows will use SSE(!) instructions to 
read LAPIC registers), so this seems like a bug in Bhyve.

The full thread can be found here:

https://marc.info/?l=openbsd-bugs&m=156142900518812&w=2
Comment 1 John Baldwin freebsd_committer freebsd_triage 2019-06-25 14:01:48 UTC
Please try the patch at the phab URL.  I have only compile-tested it, but it should emulate the TEST instruction clang is using in this case.
Comment 2 Jason Tubnor 2019-06-26 02:31:24 UTC
D19975 needed to be applied before D20755.

Both patched and built correctly.

Issue still remains below is the output of bhyve.log:


rdmsr to register 0xc80 on vcpu 0
vm_run error -1, errno 22
Comment 3 Mark Johnston freebsd_committer 2019-06-26 03:06:04 UTC
(In reply to Jason Tubnor from comment #2)
In the line of D20755 consisting of "case 0x7F:", could you try replacing 0x7F with 0xF7 and retest?
Comment 4 Jason Tubnor 2019-06-26 03:43:59 UTC
As per comments by markj@ in https://reviews.freebsd.org/D20755

I modified the D20755 patch - switch 7F to F7 and removed struct __hack (the latter is unrelated and worked if it was there or not).

Compiled successfully and OpenBSD -current now boots correctly.

Please advise when a modified version (with the above changes) hits phabricator and I will download, patch and test.

Additional test cases:

- Ensure Windows Server 2016 still boots and operates correctly (in progress)
- Patch applies to FreeBSD 12.0
  - Test OpenBSD -current
  - Test Windows Server 2016
Comment 5 Jason Tubnor 2019-06-26 03:54:59 UTC
Confirmed D20755 works with Windows Server 2016 guests.

Host:

FreeBSD 11.3-RC2 #0 r349252: Fri Jun 21 04:48:05 UTC 2019     root@releng2.nyi.freebsd.org:/usr/obj/usr/src/sys/GENERIC
Comment 6 Jason Tubnor 2019-06-26 06:29:34 UTC
FreeBSD 12.0-RELEASE-p6 - Confirmed D20755 works (after modifications as per the discussion above).

Host:

# uname -a
FreeBSD testhost 12.0-RELEASE-p6 FreeBSD 12.0-RELEASE-p6 GENERIC  amd64

# freebsd-version
12.0-RELEASE-p3

Guests tests:

Windows Server 2016 - OK
OpenBSD 6.5-current - OK
FreeBSD 12.0-p6 - OK
Comment 7 commit-hook freebsd_committer 2019-06-26 21:20:41 UTC
A commit references this bug:

Author: rgrimes
Date: Wed Jun 26 21:19:44 UTC 2019
New revision: 349441
URL: https://svnweb.freebsd.org/changeset/base/349441

Log:
  Emulate the "TEST r/m{16,32,64}, imm{16,32,32}" instructions (opcode F7H).

  This adds emulation for:
  	test r/m16, imm16
  	test r/m32, imm32
  	test r/m64, imm32 sign-extended to 64

  OpenBSD guests compiled with clang 8.0.0 use TEST directly against a
  Local APIC register instead of separate read via MOV followed by a
  TEST against the register.

  PR:		238794
  Submitted by:	jhb
  Reported by:	Jason Tubnor jason@tubnor.net
  Tested by:	Jason Tubnor jason@tubnor.net
  Reviewed by:	markj, Patrick Mooney patrick.mooney@joyent.com
  MFC after:	3 days
  Differential Revision:	https://reviews.freebsd.org/D20755

Changes:
  head/sys/amd64/vmm/vmm_instruction_emul.c
Comment 8 commit-hook freebsd_committer 2019-07-07 17:30:40 UTC
A commit references this bug:

Author: markj
Date: Sun Jul  7 17:30:23 UTC 2019
New revision: 349808
URL: https://svnweb.freebsd.org/changeset/base/349808

Log:
  MFC r349441 (by rgrimes):
  Emulate the "TEST r/m{16,32,64}, imm{16,32,32}" instructions (opcode F7H).

  PR:	238794

Changes:
_U  stable/12/
  stable/12/sys/amd64/vmm/vmm_instruction_emul.c
Comment 9 commit-hook freebsd_committer 2019-07-07 17:31:44 UTC
A commit references this bug:

Author: markj
Date: Sun Jul  7 17:31:13 UTC 2019
New revision: 349809
URL: https://svnweb.freebsd.org/changeset/base/349809

Log:
  MFC r349441 (by rgrimes):
  Emulate the "TEST r/m{16,32,64}, imm{16,32,32}" instructions (opcode F7H).

  PR:	238794

Changes:
_U  stable/11/
  stable/11/sys/amd64/vmm/vmm_instruction_emul.c
Comment 10 Ed Maste freebsd_committer 2019-08-13 18:00:26 UTC
Planned for release as EN-19:16.bhyve