Emulate the TEST instruction with opcode F7 /0.
ClosedPublic
Actions

Authored by rgrimes on Jun 25 2019, 2:00 PM.

Details

Reviewers

jhb
markj
pmooney_pfmooney.com

Group Reviewers

bhyve

Commits

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

Summary

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

Test Plan

only compile tested

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jhb created this revision.Jun 25 2019, 2:00 PM

Herald added subscribers: bcran, rgrimes. · View Herald TranscriptJun 25 2019, 2:00 PM

Harbormaster completed remote builds in B25027: Diff 58996.Jun 25 2019, 2:00 PM

I am working with the original poster to get him a binary so he can test this fix.

markj added a subscriber: markj.Jun 25 2019, 3:07 PM

markj added inline comments.

sys/amd64/vmm/vmm_instruction_emul.c
472 ↗	(On Diff #58996)	Not sure what "struct __hack" is for.
1274 ↗	(On Diff #58996)	It should be 0xF7, based on the SDM.

I helped Jason Tubnor test this patch on 11.3RC2, we had to first merge r347065 to get this patch to apply, and then had to make the 0x7F to 0xF7 Markj points out to get it to work. OpenBSD now runs as a guest in 11.3RC2

sys/amd64/vmm/vmm_instruction_emul.c
472 ↗	(On Diff #58996)	Jason T removed these in his testing, not sure if that was right or not, but the patch does work without this.
1274 ↗	(On Diff #58996)	Confirmed Jason T could not get a patch to work until this was changed to 0xF7.

emaste added a subscriber: emaste.Jun 26 2019, 2:05 PM

Ping, since 11.3RC3 is back on the table I would like to see this pushed through along with r347065 which should of already been pushed through as it is 3 weeks past original MFC.

markj added inline comments.Jun 26 2019, 7:27 PM

sys/amd64/vmm/vmm_instruction_emul.c
472 ↗	(On Diff #58996)	I think I see now, it's so you can add a semicolon after GETANDFLAGS(...) without potentially triggering a compiler warning. Sorry for the noise.

@rgrimes would you help coordinate with re@?

Looks good, once the 7F->F7 fix is made.

sys/amd64/vmm/vmm_instruction_emul.c
472 ↗	(On Diff #58996)	It looks like it's there to appease the compiler when appending a `;` to the macro calls?

I was kinda waiting for jhb@ on this, as it is his patch, but given the urgency I am commandering this, and I'll push a new patch up for review. I would like to ask markj and pmooney to final review that and accept it before I commit to ^/head and push for an immediate MFC, and merge to releng. I'll also need to merge r347065 which is 6 weeks past MFC date.

rgrimes added inline comments.Jun 26 2019, 8:23 PM