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.

Sign up for GitHub

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

Jump to bottom

Add support for Loongarch #190

Merged
merged 1 commit into from
Feb 4, 2024
Merged

Add support for Loongarch #190

merged 1 commit into from
Feb 4, 2024

Conversation

fangyaling
Copy link
Contributor

No description provided.

@hannesm
Copy link
Member

hannesm commented Jan 30, 2024

any chance there's a CI system that can check these conditional code paths, to avoid regressions in the future?

@fangyaling
Copy link
Contributor Author

fangyaling commented Feb 1, 2024 •
edited
Loading

It would be good if we could add something to the CI, but now the ocaml patch not merged, only built in local.
Now the code in the patch is refer kernel module.
The ref code link in kernel is https://lore.kernel.org/lkml/20220226110338.77547-20-chenhuacai@loongson.cn/
`
static __always_inline u64 __arch_get_hw_counter(s32 clock_mode,
const struct vdso_data *vd)
{
unsigned int count;

__asm__ __volatile__(
"	rdtime.d %0, $zero\n"
: "=r" (count));

return count;

}
`

hannesm
hannesm reviewed Feb 3, 2024
View reviewed changes
src/native/entropy_cpu_stubs.c Outdated
@@ -143,6 +143,14 @@ static inline unsigned long get_count(void) {
}
#endif

#if defined (__loongarch__)
Copy link
Member

Choose a reason for hiding this comment

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

here it is __loongarch__

hannesm
hannesm reviewed Feb 3, 2024
View reviewed changes
src/native/mirage_crypto.h Outdated
@@ -47,7 +47,7 @@ extern struct _mc_cpu_features mc_detected_cpu_features;

#endif /* __mc_ACCELERATE__ */

#if defined (__x86_64__) || defined (__aarch64__) || defined (__powerpc64__) || (64 == __riscv_xlen) || defined (__s390x__) || (defined (__mips__) && _MIPS_SIM==_ABI64)
#if defined (__x86_64__) || defined (__aarch64__) || defined (__powerpc64__) || (64 == __riscv_xlen) || defined (__s390x__) || (defined (__mips__) && _MIPS_SIM==_ABI64) || defined(__loongarch64)
Copy link
Member

Choose a reason for hiding this comment

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

and here __loongarch64 (thus no trailing __).

would you mind to elaborate which defines are used and why? are all __loongarch__ systems 64 bit? should the code above in entropy_cpu_stubs.c be guarded by the same define?

@fangyaling
Copy link
Contributor Author

fangyaling commented Feb 4, 2024 •
edited
Loading

@hannesm Thanks for your review. And We use __loongarch_lp64 and __loongarch64 (no trailing ) for this case. The __loongarch_lp64 is recommended.

@hannesm hannesm merged commit a63f181 into mirage:main Feb 4, 2024
3 of 4 checks passed
@hannesm
Copy link
Member

hannesm commented Feb 4, 2024

Thanks.

@hannesm hannesm mentioned this pull request Feb 26, 2024
Merged
hannesm added a commit to hannesm/opam-repository that referenced this pull request Feb 26, 2024
@hannesm
[new release] mirage-crypto (8 packages) (0.11.3)
7d3dab1 
CHANGES:

* mirage-crypto, mirage-crypto-rng{,lwt,mirage}: support CL.EXE compiler
  (mirage/mirage-crypto#137 @jonahbeckford) - mirage-crypto-pk not yet due to gmp dependency,
  mirage-crypto-ec doesn't pass testsuite
* mirage-crypto-ec: use simpler square root for ed25519 - saving 3
  multiplications and 2 squarings, details
  https://mailarchive.ietf.org/arch/msg/cfrg/qlKpMBqxXZYmDpXXIx6LO3Oznv4/
  (mirage/mirage-crypto#196 @hannesm)
* mirage-crypto-ec: use sliding window method with pre-computed calues of
  multiples of the generator point for NIST curves, speedup around 4x for P-256
  sign (mirage/mirage-crypto#191 @Firobe, review @palainp @hannesm)
* mirage-crypto-ec: documentation: warn about power timing analysis on `k` in
  Dsa.sign (mirage/mirage-crypto#195 @hannesm, as proposed by @edwintorok)
* mirage-crypto-ec: replace internal Cstruct.t by string (speedup up to 2.5x)
  (mirage/mirage-crypto#146 @dinosaure @hannesm @reynir, review @Firobe @palainp @hannesm @reynir)
* bench/speed: add EC (ECDSA & EdDSA generate/sign/verify, ECDH secret/share)
  operations (mirage/mirage-crypto#192 @hannesm)
* mirage-crypto-rng: use rdtime instead of rdcycle on RISC-V (rdcycle is
  privileged since Linux kernel 6.6) (mirage/mirage-crypto#194 @AdrianBunk, review by @edwintorok)
* mirage-crypto-rng: support Loongarch (mirage/mirage-crypto#190 @fangyaling, review @loongson-zn)
* mirage-crypto-rng: support NetBSD (mirage/mirage-crypto#189 @drchrispinnock)
* mirage-crypto-rng: allocate less in Fortuna when feeding (mirage/mirage-crypto#188 @hannesm,
  reported by @palainp)
* mirage-crypto-ec: avoid mirage-crypto-pk and asn1-combinators test dependency
  (instead, craft our own asn.1 decoder -- mirage/mirage-crypto#200 @hannesm)

### Performance differences between v0.11.2 and v0.11.3 and OpenSSL

The overall result is promising: P-256 sign operation improved 9.4 times, but
is still a 4.9 times slower than OpenSSL.

Numbers in operations per second (apart from speedup, which is a factor
v0.11.3 / v0.11.2), gathered on a Intel i7-5600U CPU 2.60GHz using FreeBSD 14.0,
OCaml 4.14.1, and OpenSSL 3.0.12.

#### P224

| op     | v0.11.2 | v0.11.3 | speedup | OpenSSL |
|--------|---------|---------|---------|---------|
| gen    | 1160    | 20609   |    17.8 |         |
| sign   | 931     | 8169    |     8.8 | 21319   |
| verify | 328     | 1606    |     4.9 | 10719   |
| dh-sec | 1011    | 12595   |    12.5 |         |
| dh-kex | 992     | 2021    |     2.0 | 16691   |

#### P256

| op     | v0.11.2 | v0.11.3 | speedup | OpenSSL |
|--------|---------|---------|---------|---------|
| gen    | 990     | 19365   |    19.6 |         |
| sign   | 792     | 7436    |     9.4 | 36182   |
| verify | 303     | 1488    |     4.9 | 13383   |
| dh-sec | 875     | 11508   |    13.2 |         |
| dh-kex | 895     | 1861    |     2.1 | 17742   |

#### P384

| op     | v0.11.2 | v0.11.3 | speedup | OpenSSL |
|--------|---------|---------|---------|---------|
| gen    | 474     | 6703    |    14.1 |         |
| sign   | 349     | 3061    |     8.8 | 900     |
| verify | 147     | 544     |     3.7 | 1062    |
| dh-sec | 378     | 4405    |    11.7 |         |
| dh-kex | 433     | 673     |     1.6 | 973     |

#### P521

| op     | v0.11.2 | v0.11.3 | speedup | OpenSSL |
|--------|---------|---------|---------|---------|
| gen    | 185     | 1996    |    10.8 |         |
| sign   | 137     | 438     |     3.2 | 2737    |
| verify | 66      | 211     |     3.2 | 1354    |
| dh-sec | 180     | 1535    |     8.5 |         |
| dh-kex | 201     | 268     |     1.3 | 2207    |

#### 25519

| op     | v0.11.2 | v0.11.3 | speedup | OpenSSL |
|--------|---------|---------|---------|---------|
| gen    | 23271   | 22345   |     1.0 |         |
| sign   | 11228   | 10985   |     1.0 | 21794   |
| verify | 8149    | 8029    |     1.0 | 7729    |
| dh-sec | 14075   | 13968   |     1.0 |         |
| dh-kex | 13487   | 14079   |     1.0 | 24824   |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hannesm hannesm hannesm left review comments

@loongson-zn loongson-zn loongson-zn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fangyaling @hannesm @loongson-zn