[gnutls-devel] GnuTLS | x86:add detection of instruction set on Zhaoxin CPU (!1335)

[Read-only notification of GnuTLS library development activities]

Merge request https://gitlab.com/gnutls/gnutls/-/merge_requests/1335 was reviewed by Daiki Ueno

--
  
Daiki Ueno started a new discussion on lib/accelerated/x86/sha-padlock.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1335#note_416789266

>  
> -const gnutls_crypto_digest_st _gnutls_sha_padlock_nano = {
> +const gnutls_crypto_digest_st _gnutls_sha_padlock_enhance = {

Although this is a matter of taste, I would give those variants a self-describing names, something like:
- `_gnutls_sha_padlock` → `_gnutls_sha_padlock_oneshot`
- `_gnutls_sha_padlock_nano` → `_gnutls_sha_padlock`

--
  
Daiki Ueno started a new discussion on lib/accelerated/x86/x86-common.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1335#note_416789270

> -	     memcmp(&d, "aurH", 4) == 0 && memcmp(&c, "auls", 4) == 0)) {
> +	     memcmp(&d, "aurH", 4) == 0 &&
> +         memcmp(&c, "auls", 4) == 0) ||

Would be nice to add comments saying which CPU model this code is detecting.

--
  
Daiki Ueno started a new discussion on lib/accelerated/x86/x86-common.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1335#note_416789273

> -	if (capabilities == 0)
> +	if (capabilities == 0){
> +        if(!read_cpuid_vals(_gnutls_x86_cpuid_s))

The indentation looks strange.

--
  
Daiki Ueno started a new discussion on lib/accelerated/x86/x86-common.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1335#note_416789275

> +{
> +	unsigned int a,b,c,d;
> +	a = b = c = d = 0;

I'd remove this initialization, as we don't have it in other functions calling `__get_cpuid` and I assume all arguments are set upon a successful call.

--
  
Daiki Ueno started a new discussion on lib/accelerated/x86/x86-common.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1335#note_416789282

> +
> +	unsigned int family = ((a >> 8) & 0x0F);
> +	unsigned int model = ((a >> 4) & 0x0F) + ((a >> 12) & 0xF0);

Afaik we still support pre-C99 compilers; please move the variable declaration to the top.

--
  
Daiki Ueno started a new discussion on lib/accelerated/x86/x86-common.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1335#note_416789283

>  	unsigned a,b,c,t;
>  
> -	memset(_gnutls_x86_cpuid_s, 0, sizeof(_gnutls_x86_cpuid_s));

Why is this `memset` no longer necessary?

--
  
Daiki Ueno started a new discussion on lib/accelerated/x86/x86-common.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1335#note_416789290

> +#define PADLOCK (1<<20)
> +#define PADLOCK_PHE (1<<21)
> +#define PADLOCK_PHE_SHA512 (1<<22)

I'm not quite sure about the relationship between VIA and Zhaoxin, but is there any specific (strong) reason behind removing "VIA" term?


-- 
Reply to this email directly or view it on GitLab: https://gitlab.com/gnutls/gnutls/-/merge_requests/1335
You're receiving this email because of your account on gitlab.com.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.gnupg.org/pipermail/gnutls-devel/attachments/20200922/12ac4671/attachment-0001.html>