Add hal_core_alloc2() to fix a dining philosophers problem in hal_modexp2().

Uncoordinated attempts to allocate two modexpa7 cores leads to deadlock if
multiple clients try to do concurrent RSA signing operations.

The simplest solution (back off and retry) could theoretically lead to
resource starvation, but we haven't seen it in actual testing.

3 files changed, 49 insertions, 25 deletions

diff --git a/core.c b/core.c
index daa82fa..c604a15 100644
--- a/core.c
+++ b/core.c
@@ -4,7 +4,7 @@
  * This module contains code to probe the FPGA for its installed cores.
  *
  * Author: Paul Selkirk, Rob Austein
- * Copyright (c) 2015-2016, NORDUnet A/S All rights reserved.
+ * Copyright (c) 2015-2017, NORDUnet A/S All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -201,7 +201,7 @@ hal_core_t *hal_core_find(const char *name, hal_core_t *core)
   return NULL;
 }
 
-hal_error_t hal_core_alloc(const char *name, hal_core_t **pcore)
+static hal_error_t hal_core_alloc_no_wait(const char *name, hal_core_t **pcore)
 {
   /*
    * This used to allow name == NULL iff *core != NULL, but the
@@ -227,30 +227,52 @@ hal_error_t hal_core_alloc(const char *name, hal_core_t **pcore)
     *pcore = NULL;
   }
 
-  while (1) {
-    hal_critical_section_start();
-    for (core = hal_core_iterate(NULL); core != NULL; core = core->next) {
-      if (!name_matches(core, name))
-        continue;
-      if (core->busy) {
-        err = HAL_ERROR_CORE_BUSY;
-        continue;
-      }
-      err = HAL_OK;
-      *pcore = core;
-      core->busy = 1;
-      break;
+  hal_critical_section_start();
+  for (core = hal_core_find(name, NULL); core != NULL; core = hal_core_find(name, core)) {
+    if (core->busy) {
+      err = HAL_ERROR_CORE_BUSY;
+      continue;
     }
-    hal_critical_section_end();
-    if (err == HAL_ERROR_CORE_BUSY)
-      hal_task_yield();
-    else
-      break;
+    err = HAL_OK;
+    *pcore = core;
+    core->busy = 1;
+    break;
   }
+  hal_critical_section_end();
+
+  return err;
+}
+
+hal_error_t hal_core_alloc(const char *name, hal_core_t **pcore)
+{
+  hal_error_t err;
+
+  while ((err = hal_core_alloc_no_wait(name, pcore)) == HAL_ERROR_CORE_BUSY)
+    hal_task_yield();
 
   return err;
 }
 
+hal_error_t hal_core_alloc2(const char *name1, hal_core_t **pcore1,
+                            const char *name2, hal_core_t **pcore2)
+{
+  hal_error_t err;
+
+  while (1) {
+    if ((err = hal_core_alloc(name1, pcore1)) != HAL_OK)
+      return err;
+
+    if ((err = hal_core_alloc_no_wait(name2, pcore2)) == HAL_OK)
+      return HAL_OK;
+    
+    hal_core_free(*pcore1);
+    /* hal_core_free does a yield, so we don't need to do another one */
+
+    if (err != HAL_ERROR_CORE_BUSY)
+      return err;
+  }
+}
+
 void hal_core_free(hal_core_t *core)
 {
   if (core != NULL) {
diff --git a/hal.h b/hal.h
index 0030f3d..a614335 100644
--- a/hal.h
+++ b/hal.h
@@ -226,6 +226,8 @@ extern hal_addr_t hal_core_base(const hal_core_t *core);
 extern hal_core_t * hal_core_iterate(hal_core_t *core);
 extern void hal_core_reset_table(void);
 extern hal_error_t hal_core_alloc(const char *name, hal_core_t **core);
+extern hal_error_t hal_core_alloc2(const char *name1, hal_core_t **pcore1,
+                                   const char *name2, hal_core_t **pcore2);
 extern void hal_core_free(hal_core_t *core);
 extern void hal_critical_section_start(void);
 extern void hal_critical_section_end(void);
diff --git a/modexp.c b/modexp.c
index 7973258..37f5e0e 100644
--- a/modexp.c
+++ b/modexp.c
@@ -11,7 +11,7 @@
  * enough that this module is no longer needed, it will go away.
  *
  * Authors: Rob Austein
- * Copyright (c) 2015, NORDUnet A/S
+ * Copyright (c) 2015-2017, NORDUnet A/S
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -62,7 +62,7 @@ void hal_modexp_set_debug(const int onoff)
  * Get value of an ordinary register.
  */
 
-static hal_error_t inline get_register(const hal_core_t *core,
+static inline hal_error_t get_register(const hal_core_t *core,
                                        const hal_addr_t addr,
                                        uint32_t *value)
 {
@@ -84,7 +84,7 @@ static hal_error_t inline get_register(const hal_core_t *core,
  * Set value of an ordinary register.
  */
 
-static hal_error_t inline set_register(const hal_core_t *core,
+static inline hal_error_t set_register(const hal_core_t *core,
                                        const hal_addr_t addr,
                                        const uint32_t value)
 {
@@ -307,8 +307,8 @@ hal_error_t hal_modexp2(const int precalc, hal_modexp_arg_t *a1, hal_modexp_arg_
       (err = check_args(a2)) != HAL_OK)
     return err;
 
-  if ((err = hal_core_alloc(MODEXPA7_NAME, &a1->core)) == HAL_OK  &&
-      (err = hal_core_alloc(MODEXPA7_NAME, &a2->core)) == HAL_OK  &&
+  if ((err = hal_core_alloc2(MODEXPA7_NAME, &a1->core,
+                             MODEXPA7_NAME, &a2->core)) == HAL_OK  &&
       (err = setup_precalc(precalc, a1))               == HAL_OK  &&
       (err = setup_precalc(precalc, a2))               == HAL_OK  &&
       (!precalc ||