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

Allow going around PDEP catastrophe on AMD processors #786

Closed
damageboy opened this issue Dec 12, 2019 · 3 comments
Closed

Allow going around PDEP catastrophe on AMD processors #786

damageboy opened this issue Dec 12, 2019 · 3 comments

Comments

@damageboy
Copy link
Contributor

At the request of @danmosemsft I'm opening this issue...

There has been a recent and very unfortunate discovery about PDEP on AMD processors.
This comes from a very respected pair of performance gurus, namely the person running the famous uops.info website.

Discussion:
https://twitter.com/trav_downs/status/1203425075896225792
Repro:
https://twitter.com/uops_info/status/1202950721211162626

Screenshot (twitter being twitter and all):
image

To summarize: While AMD processors officially support the PDEP opcode, it is really implemented in microcode via a loop and might execute anywhere between 18-289 cycles.

This might also have potential security implications for code that performs PDEP as part of cryptographic functions that might require/rely on constant time execution (Though I'm less of an expert, maybe people like @blowdart can provide some feedback about how real this is).

In my opinion, the only proper way to handle such erratic CPU behavior (Unfortunately it's really not the last, nor the first time this sort of thing happens) is to provide HW information to developers while also making sure that the JIT can and will treat this information as a constant when it comes to code-generation.

I've already opened a separate issue for exposing these HW level constants in a more general way, but this PDEP mess also deserves some attention in its own.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Dec 12, 2019
@damageboy
Copy link
Contributor Author

Looks like @InstLatx64 followed up on this with more concrete timing per mask:

https://twitter.com/InstLatX64/status/1209095219087585281?s=20

image

@GrabYourPitchforks
Copy link
Member

Related: #2251

@tannergooding tannergooding removed the untriaged New issue has not been triaged by the area owner label Feb 10, 2020
@tannergooding
Copy link
Member

@damageboy, would you be fine with closing this given that we've updated the codebase to not use PDEP/PEXT and we are tracking exposing CPUID as its own intrinsic via #785?

@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants