Page MenuHomeFreeBSD

Emulate the TEST instruction with opcode F7 /0.
ClosedPublic

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

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jhb created this revision.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?

rgrimes commandeered this revision.Jun 26 2019, 8:03 PM
rgrimes added a reviewer: jhb.

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
sys/amd64/vmm/vmm_instruction_emul.c
1279 ↗(On Diff #58996)

These 3 7F's also should be F7

rgrimes updated this revision to Diff 59079.Jun 26 2019, 8:26 PM

Fix 7F to F7 in 4 places

rgrimes marked 4 inline comments as done.

Finally review please

This revision is now accepted and ready to land.Jun 26 2019, 8:41 PM
markj accepted this revision as: markj.Jun 26 2019, 8:44 PM