Hardware revoker: correct wait for indicated epoch
The commentary says that we wait for the indicated (even) epoch to have
finished, but in practice we were merely waiting for the next one to
begin. Because has_revocation_finished_for_epoch() had the intended
semantics and is the actual function used to gate release from
quarantine, wait_for_completion()'s incorrect waiting was merely
ineffecient, making malloc_internal spin a bit in the "Quarantine has
enough memory to satisfy allocation" case.
diff --git a/sdk/core/allocator/main.cc b/sdk/core/allocator/main.cc
index c05ab9c..492ebf2 100644
--- a/sdk/core/allocator/main.cc
+++ b/sdk/core/allocator/main.cc
@@ -219,7 +219,7 @@
requires(!Revocation::SupportsInterruptNotification<T>)
{
// Yield while until a revocation pass has finished.
- while (!revoker.has_revocation_finished_for_epoch<true>(epoch))
+ while (!revoker.has_revocation_finished_for_epoch(epoch))
{
// Release the lock before sleeping
g.unlock();
diff --git a/sdk/include/platform/ibex/platform-hardware_revoker.hh b/sdk/include/platform/ibex/platform-hardware_revoker.hh
index c039cce..965fa05 100644
--- a/sdk/include/platform/ibex/platform-hardware_revoker.hh
+++ b/sdk/include/platform/ibex/platform-hardware_revoker.hh
@@ -121,7 +121,11 @@
}
/**
- * Queries whether the specified revocation epoch has finished.
+ * Queries whether the specified revocation epoch has finished, or,
+ * if `AllowPartial` is true, that it has (at least) started.
+ *
+ * `epoch` must be even, as memory leaves quarantine only when
+ * revocation is not in progress.
*/
template<bool AllowPartial = false>
uint32_t has_revocation_finished_for_epoch(uint32_t epoch)
@@ -177,7 +181,7 @@
// futex word with respect to the read of the revocation epoch.
__c11_atomic_signal_fence(__ATOMIC_SEQ_CST);
// If the requested epoch has finished, return success.
- if (has_revocation_finished_for_epoch<true>(epoch))
+ if (has_revocation_finished_for_epoch(epoch))
{
return true;
}
@@ -186,7 +190,7 @@
// There is a possible race: if the revocation pass finished
// before we requested the interrupt, we won't get the
// interrupt. Check again before we wait.
- if (has_revocation_finished_for_epoch<true>(epoch))
+ if (has_revocation_finished_for_epoch(epoch))
{
return true;
}
diff --git a/tests.extra/hardware_revoker_IRQs/top.cc b/tests.extra/hardware_revoker_IRQs/top.cc
index 9cb7d67..7347934 100644
--- a/tests.extra/hardware_revoker_IRQs/top.cc
+++ b/tests.extra/hardware_revoker_IRQs/top.cc
@@ -40,8 +40,6 @@
uint32_t epoch = r.system_epoch_get();
Debug::log("At startup, revocation epoch is {}; waiting...", epoch);
- // Just in case a revocation is somehow active...
- epoch &= ~1;
r.system_bg_revoker_kick();
for (int i = 0; i < 10; i++)
@@ -50,7 +48,7 @@
uint32_t newepoch;
Timeout t{50};
- res = r.wait_for_completion(&t, (epoch & ~1) + 2);
+ res = r.wait_for_completion(&t, epoch & ~1);
newepoch = r.system_epoch_get();
Debug::log("After wait: for {}, result {}, epoch now is {}, "
