Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x64: Handle Intel JCC Erratum #8475

Open
wants to merge 3 commits into
base: master
from

Conversation

@MerryMage
Copy link
Member

MerryMage commented Nov 17, 2019

Also see relevant white paper.

This implementation adds NOP padding before all affected JCC/JMP/CALL/RET instructions.

Please note: This implementation will split macro-op fusable op-JCC pairs. An improved implementation would add padding before such pairs, instead of in between these instructions.

The Intel CPU microcode migitations for SKX102 cause jump instructions and
macrofused jump instructions across a 32-byte boundary to not be cached in the
decoded icache.

Information about which CPUs are affected can be found in an Intel white paper,
document number 341810-001.
@MerryMage MerryMage force-pushed the MerryMage:jcc_erratum branch from 5b47bba to 3022c02 Nov 17, 2019
@BhaaLseN

This comment has been minimized.

Copy link
Member

BhaaLseN commented Nov 17, 2019

Would you mind adding a TL;DR comment somewhere, briefly describing what the problem is there (and how to address it)? Next to the AddJccErratumPadding method is probably a good place.

@MerryMage MerryMage force-pushed the MerryMage:jcc_erratum branch from 3022c02 to 4583f9f Nov 17, 2019
@MerryMage

This comment has been minimized.

Copy link
Member Author

MerryMage commented Nov 17, 2019

Added a comment.

@MerryMage MerryMage force-pushed the MerryMage:jcc_erratum branch from 4583f9f to 403f886 Nov 19, 2019
@MerryMage

This comment has been minimized.

Copy link
Member Author

MerryMage commented Nov 19, 2019

Added handling of macrofused instruction pairs.

Todo:

  • Audit all J_CC locations to ensure no invalid assumed fusions occur.
Source/Core/Common/x64Emitter.cpp Outdated Show resolved Hide resolved
Source/Core/Common/x64Emitter.h Outdated Show resolved Hide resolved
@@ -81,6 +81,7 @@ void Jit64AsmRoutineManager::Generate()
ResetStack(*this);

SUB(32, PPCSTATE(downcount), R(RSCRATCH2));
NOP(); // Prevent op-Jcc fusion.

This comment has been minimized.

Copy link
@phire

phire Nov 20, 2019

Member

Why are we preventing op-Jcc fusion? I though the emitter now handles this.

This comment has been minimized.

Copy link
@MerryMage

MerryMage Nov 20, 2019

Author Member

Here, the later JLE in the dispatcher will be detected as fusable with the SUB, and thus the SUB will be memmove-d. However, this invalidates the code location in the dispatcher variable.

An alternative way of preventing this is to modify GetCodePtr as so:

const u8* XEmitter::GetCodePtr() const
{
  jcc_fusion_type = JccFusionType::None;
  return code;
}

i.e. prevent the possibility of memmove-ing code across such locations

This comment has been minimized.

Copy link
@phire

phire Nov 20, 2019

Member

It's fine, but would be nice if the comment elaborated.

This comment has been minimized.

Copy link
@MerryMage

MerryMage Nov 21, 2019

Author Member

Done.

@MayImilae

This comment has been minimized.

Copy link
Contributor

MayImilae commented Nov 20, 2019

@JMC47 please do thorough performance testing! We'll need a lot of data for when this hits a Progress Report.

@MerryMage MerryMage force-pushed the MerryMage:jcc_erratum branch from 403f886 to 42c0316 Nov 21, 2019
@MerryMage MerryMage changed the title [RFC] x64: Handle Intel JCC Erratum x64: Handle Intel JCC Erratum Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
You can’t perform that action at this time.