Skip to content

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

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

Closed
wants to merge 2 commits into from

Conversation

brocaar
Copy link

@brocaar brocaar commented Aug 16, 2022

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:

Copy link
Member

@newpavlov newpavlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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"));

Either way, I think it's worth to merge this fix, although in a slightly different form.

@brocaar
Copy link
Author

brocaar commented Aug 16, 2022

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
Copy link
Author

brocaar commented Aug 16, 2022

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

@newpavlov
Copy link
Member

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
Copy link
Member

Sure, sounds good

@brocaar
Copy link
Author

brocaar commented Aug 18, 2022

The above PR solved this issue. Thanks! 👍

@brocaar brocaar closed this Aug 18, 2022
brocaar added a commit to chirpstack/chirpstack that referenced this pull request Aug 18, 2022
In some scenarios, this check returned true while the avx2 extension was
not available (e.g. QEMU emulated CPU).

See for more details: RustCrypto/hashes#386.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants