RustCrypto / hashes

Collection of cryptographic hash functions written in pure Rust
1.87k stars 251 forks source link

To use avx2, both avx and avx2 must be checked. #386

Closed brocaar closed 2 years ago

brocaar commented 2 years ago

Please see the Intel note about detecting the avx2 extension:

image

https://www.intel.com/content/dam/develop/external/us/en/documents/36945

It states that both the support for AVX and AVX2 must be detected.


Some context:

I had a bug report that trying to login caused the application (ChirpStack) to panic. After a deep dive, it turned out that the host (Qemu CPU) does not support these instructions, but avx2_cpuid::get() returns true.

Then I noticed that the Intel docs state that both AVX and AVX2 must be checked. Running this application on this specific Qemu CPU returns:

Supports avx: false
Supports avx2: true
cpufeatures::new!(avx_cpuid, "avx");
cpufeatures::new!(avx2_cpuid, "avx2");

fn main() {
    println!("Supports avx: {}", avx_cpuid::get());
    println!("Supports avx2: {}", avx2_cpuid::get());
}

As reference, this is the content of /proc/cpuinfo:

processor       : 0
vendor_id       : AuthenticAMD
cpu family      : 6
model           : 13
model name      : QEMU Virtual CPU version (cpu64-rhel6)
stepping        : 3
microcode       : 0x1000065
cpu MHz         : 3393.624
cache size      : 512 KB
physical id     : 0
siblings        : 1
core id         : 0
cpu cores       : 1
apicid          : 0
initial apicid  : 0
fpu             : yes
fpu_exception   : yes
cpuid level     : 4
wp              : yes
flags           : fpu de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 syscall nx lm nopl cpuid tsc_known_freq pni cx16 hypervisor lahf_lm abm sse4a 3dnowprefetch vmmcall
bugs            : fxsave_leak sysret_ss_attrs null_seg spectre_v1 spectre_v2 spec_store_bypass
bogomips        : 6787.24
TLB size        : 1024 4K pages
clflush size    : 64
cache_alignment : 64
address sizes   : 48 bits physical, 48 bits virtual
power management:

processor       : 1
vendor_id       : AuthenticAMD
cpu family      : 6
model           : 13
model name      : QEMU Virtual CPU version (cpu64-rhel6)
stepping        : 3
microcode       : 0x1000065
cpu MHz         : 3393.624
cache size      : 512 KB
physical id     : 1
siblings        : 1
core id         : 0
cpu cores       : 1
apicid          : 1
initial apicid  : 1
fpu             : yes
fpu_exception   : yes
cpuid level     : 4
wp              : yes
flags           : fpu de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 syscall nx lm nopl cpuid tsc_known_freq pni cx16 hypervisor lahf_lm abm sse4a 3dnowprefetch vmmcall
bugs            : fxsave_leak sysret_ss_attrs null_seg spectre_v1 spectre_v2 spec_store_bypass
bogomips        : 6787.24
TLB size        : 1024 4K pages
clflush size    : 64
cache_alignment : 64
address sizes   : 48 bits physical, 48 bits virtual
power management:
brocaar commented 2 years ago

Hm, we have been following the Rust reference which assumes that enabled AVX2 implies AVX.

image

But isn't this to manually enable target features? I read this as: If you enable avx2, this will automatically enable avx (no need to enable both explicitly). It doesn't say anything about the detection of this CPU extension.

The Intel note is quite clear on this: "Application Software must identify that hardware supports AVX ...., after that it must also detect support for AVX2 ....".

Can you check what the following snippet prints on the CPU?

println!("avx: {}", is_x86_feature_detected!("avx"));
println!("avx2: {}", is_x86_feature_detected!("avx2"));

Output:

avx: false
avx2: false

The output of:

cpufeatures::new!(avx_cpuid, "avx");
cpufeatures::new!(avx2_cpuid, "avx2");

fn main() {
    println!("Supports avx: {}", avx_cpuid::get());
    println!("Supports avx2: {}", avx2_cpuid::get());
}
Supports avx: false
Supports avx2: true

The output of:

cpufeatures::new!(avx2_cpuid, "avx", "avx2");

fn main() {
    println!("Supports avx2: {}", avx2_cpuid::get());
}
Supports avx2: false
brocaar commented 2 years ago

I've updated the pull-request. After merging, would you be able to create a new release?

newpavlov commented 2 years ago

Hm, the is_x86_feature_detected! results are interesting. So it looks like the std code checks both AVX and AVX2 when we test existence of avx2.

@tarcieri Maybe it's better to modify cpufeatures to follow std?

tarcieri commented 2 years ago

Sure, sounds good

brocaar commented 2 years ago

The above PR solved this issue. Thanks! :+1: