Skip to content

Commit

Permalink
Hardware revoker: correct wait for indicated epoch
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nwf committed Jan 13, 2025
1 parent 2105f28 commit 0a58f8f
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 7 deletions.
2 changes: 1 addition & 1 deletion sdk/core/allocator/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ namespace
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();
Expand Down
10 changes: 7 additions & 3 deletions sdk/include/platform/ibex/platform-hardware_revoker.hh
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,11 @@ namespace Ibex
}

/**
* 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)
Expand Down Expand Up @@ -177,7 +181,7 @@ namespace Ibex
// 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;
}
Expand All @@ -186,7 +190,7 @@ namespace Ibex
// 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;
}
Expand Down
4 changes: 1 addition & 3 deletions tests.extra/hardware_revoker_IRQs/top.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ void __cheri_compartment("top") entry()
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++)
Expand All @@ -50,7 +48,7 @@ void __cheri_compartment("top") entry()
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 {}, "
Expand Down

0 comments on commit 0a58f8f

Please sign in to comment.