From 391eb78b3355547205c6a5fba25f21a6c2eb3745 Mon Sep 17 00:00:00 2001 From: Joshua Warren <816217+joshuaswarren@users.noreply.github.com> Date: Thu, 12 Nov 2026 17:08:14 -0600 Subject: [PATCH] soc: apple: mailbox: arm the T6050 fast receive poller with an atomic flag A kick that arrived while the fast poll tick was returning without a restart saw an active timer, started nothing, and was lost until the next jiffy poll or send. Swapping the hrtimer_active() check for !hrtimer_is_queued() does not fix this: the tick forwards its own timer, and hrtimer_forward() needs the timer not to be enqueued by anyone else. Add rx_fast_armed. A kick starts the timer only when it moves the flag from 0 to 1, and the tick keeps the flag while it restarts. When the tick finds the deadline passed it clears the flag and reads the deadline again, so a kick that saw the flag set is picked up instead of lost. The stop and quiesce paths use one helper that cancels the timer and clears the flag. Two KUnit cases cover the single arm and the lost-kick recheck. With the recheck removed the second case fails. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com> --- drivers/soc/apple/mailbox-g17-test.h | 101 +++++++++++++++++++++++++++ drivers/soc/apple/mailbox.c | 61 +++++++++++++--- 2 files changed, 151 insertions(+), 11 deletions(-) diff --git a/drivers/soc/apple/mailbox-g17-test.h b/drivers/soc/apple/mailbox-g17-test.h index 10690d1d27bf7..ade65adb4e583 100644 --- a/drivers/soc/apple/mailbox-g17-test.h +++ b/drivers/soc/apple/mailbox-g17-test.h @@ -93,10 +93,111 @@ static void apple_g17_rx_util_min_test(struct kunit *test) #endif } +struct apple_g17_fast_rx { + struct apple_mbox_priv priv; + struct completion ticked; +}; + +static enum hrtimer_restart apple_g17_fast_rx_tick(struct hrtimer *timer) +{ + struct apple_g17_fast_rx *f = container_of(timer, struct apple_g17_fast_rx, + priv.rx_fast_timer); + + complete(&f->ticked); + return HRTIMER_NORESTART; +} + +static struct apple_g17_fast_rx *apple_g17_fast_rx_new(struct kunit *test) +{ + struct apple_g17_fast_rx *f = kunit_kzalloc(test, sizeof(*f), GFP_KERNEL); + + KUNIT_ASSERT_NOT_NULL(test, f); + init_completion(&f->ticked); + hrtimer_setup(&f->priv.rx_fast_timer, apple_g17_fast_rx_tick, + CLOCK_MONOTONIC, HRTIMER_MODE_REL); + return f; +} + +/* Only the kick that takes rx_fast_armed from 0 to 1 starts the timer. */ +static void apple_g17_fast_rx_kick_arms_once_test(struct kunit *test) +{ + struct apple_g17_fast_rx *f = apple_g17_fast_rx_new(test); + ktime_t first; + + apple_mbox_fast_rx_kick(&f->priv); + KUNIT_EXPECT_EQ(test, atomic_read(&f->priv.rx_fast_armed), 1); + KUNIT_EXPECT_GT(test, wait_for_completion_timeout(&f->ticked, HZ), 0UL); + + /* The test tick does not restart, as the real tick does while armed. */ + reinit_completion(&f->ticked); + first = f->priv.rx_fast_until; + apple_mbox_fast_rx_kick(&f->priv); + KUNIT_EXPECT_EQ(test, wait_for_completion_timeout(&f->ticked, + msecs_to_jiffies(5)), 0UL); + KUNIT_EXPECT_TRUE(test, ktime_after(f->priv.rx_fast_until, first)); + + /* The real tick clears the flag on every NORESTART exit; model that. */ + atomic_set(&f->priv.rx_fast_armed, 0); + apple_mbox_fast_rx_kick(&f->priv); + KUNIT_EXPECT_GT(test, wait_for_completion_timeout(&f->ticked, HZ), 0UL); + apple_mbox_fast_rx_cancel(&f->priv); +} + +/* + * A kick that lands after the tick read its expired deadline but before the + * tick dropped the flag sees the flag set and starts nothing. The tick must + * then notice the new deadline instead of going idle with the kick lost. + */ +static void apple_g17_fast_rx_keeps_racing_kick_test(struct kunit *test) +{ + struct apple_g17_fast_rx *f = apple_g17_fast_rx_new(test); + ktime_t before; + + /* Deadline passed, no kick: the tick stops and the next kick re-arms. */ + atomic_set(&f->priv.rx_fast_armed, 1); + f->priv.rx_fast_until = 0; + KUNIT_EXPECT_FALSE(test, apple_mbox_fast_rx_keep_armed(&f->priv)); + KUNIT_EXPECT_EQ(test, atomic_read(&f->priv.rx_fast_armed), 0); + + /* The racing kick: the flag is still set, so the kick starts no timer. */ + atomic_set(&f->priv.rx_fast_armed, 1); + f->priv.rx_fast_until = 0; + before = ktime_get(); + apple_mbox_fast_rx_kick(&f->priv); + KUNIT_EXPECT_TRUE(test, ktime_after(f->priv.rx_fast_until, before)); + KUNIT_EXPECT_EQ(test, wait_for_completion_timeout(&f->ticked, 0), 0UL); + /* Pin the deadline live so a slow runner cannot expire the kick's own. */ + f->priv.rx_fast_until = ktime_add_ms(ktime_get(), 1000); + KUNIT_EXPECT_TRUE(test, apple_mbox_fast_rx_keep_armed(&f->priv)); + KUNIT_EXPECT_EQ(test, atomic_read(&f->priv.rx_fast_armed), 1); + apple_mbox_fast_rx_cancel(&f->priv); +} + +/* + * A kick that took the flag but has not called hrtimer_start() yet must keep + * it across a cancel: the timer is not queued, so nothing was cancelled. + */ +static void apple_g17_fast_rx_cancel_keeps_inflight_kick_test(struct kunit *test) +{ + struct apple_g17_fast_rx *f = apple_g17_fast_rx_new(test); + + atomic_set(&f->priv.rx_fast_armed, 1); + apple_mbox_fast_rx_cancel(&f->priv); + KUNIT_EXPECT_EQ(test, atomic_read(&f->priv.rx_fast_armed), 1); + + /* A queued timer is cancelled with its flag, so the next kick re-arms. */ + hrtimer_start(&f->priv.rx_fast_timer, ms_to_ktime(1000), HRTIMER_MODE_REL); + apple_mbox_fast_rx_cancel(&f->priv); + KUNIT_EXPECT_EQ(test, atomic_read(&f->priv.rx_fast_armed), 0); +} + static struct kunit_case apple_g17_stall_cases[] = { KUNIT_CASE(apple_g17_rx_flood_bounded_test), KUNIT_CASE(apple_g17_tx_stall_bounded_test), KUNIT_CASE(apple_g17_rx_util_min_test), + KUNIT_CASE(apple_g17_fast_rx_kick_arms_once_test), + KUNIT_CASE(apple_g17_fast_rx_keeps_racing_kick_test), + KUNIT_CASE(apple_g17_fast_rx_cancel_keeps_inflight_kick_test), { } }; static struct kunit_suite apple_g17_stall_suite = { diff --git a/drivers/soc/apple/mailbox.c b/drivers/soc/apple/mailbox.c index b73e7da0b2b59..754d5730ad281 100644 --- a/drivers/soc/apple/mailbox.c +++ b/drivers/soc/apple/mailbox.c @@ -283,6 +283,14 @@ struct apple_mbox_priv { /* Kick and fast tick write this lock-free; reads use READ_ONCE. */ ktime_t rx_fast_until; struct hrtimer rx_fast_timer; + /* + * 1 from the moment a kick takes responsibility for the timer until + * the tick returns without restarting it. The tick forwards its own + * timer only while it holds the flag, and kick starts the timer only + * when it takes the flag, so the two never enqueue the timer at once + * (hrtimer_forward() requires that). + */ + atomic_t rx_fast_armed; }; static struct apple_mbox_priv *apple_mbox_to_priv(struct apple_mbox *mbox) @@ -665,6 +673,20 @@ static struct apple_mbox_msg apple_mbox_read_msg(struct apple_mbox *mbox) static int apple_mbox_poll_locked(struct apple_mbox *mbox); +/* + * The tick found the deadline passed. Drop the flag, then look at the deadline + * again: a kick between the tick's read and this drop saw the flag set and did + * not start the timer. The full barriers of both xchg() calls pair up, so + * either that kick sees the flag clear and starts the timer, or this recheck + * sees its deadline. True: the tick holds the flag again and must restart. + */ +static bool apple_mbox_fast_rx_keep_armed(struct apple_mbox_priv *priv) +{ + atomic_xchg(&priv->rx_fast_armed, 0); + return ktime_before(ktime_get(), READ_ONCE(priv->rx_fast_until)) && + !atomic_xchg(&priv->rx_fast_armed, 1); +} + /* * Fast receive tick for rx_poll_only mailboxes (T6050): the same receive * step apple_mbox_v8_rx_poll_work() runs, under the same rx_lock, with @@ -681,8 +703,12 @@ static enum hrtimer_restart apple_mbox_fast_rx_poll(struct hrtimer *timer) ktime_t now; int consumed = 0; - if (!READ_ONCE(mbox->active) || READ_ONCE(mbox->removing)) - return HRTIMER_NORESTART; + if (!READ_ONCE(mbox->active) || READ_ONCE(mbox->removing)) { + /* A kick that raced the resume must not lose its poll. */ + if (!apple_mbox_fast_rx_keep_armed(priv)) + return HRTIMER_NORESTART; + goto restart; + } spin_lock_irqsave(&mbox->rx_lock, flags); if (likely(!READ_ONCE(mbox->removing))) @@ -694,10 +720,12 @@ static enum hrtimer_restart apple_mbox_fast_rx_poll(struct hrtimer *timer) /* Traffic arrived: keep polling as if a send just happened. */ WRITE_ONCE(priv->rx_fast_until, ktime_add_us(now, APPLE_MBOX_RX_FAST_GRACE_US)); - } else if (!ktime_before(now, READ_ONCE(priv->rx_fast_until))) { + } else if (!ktime_before(now, READ_ONCE(priv->rx_fast_until)) && + !apple_mbox_fast_rx_keep_armed(priv)) { return HRTIMER_NORESTART; } +restart: hrtimer_forward_now(timer, us_to_ktime(APPLE_MBOX_RX_FAST_POLL_US)); return HRTIMER_RESTART; } @@ -706,22 +734,33 @@ static enum hrtimer_restart apple_mbox_fast_rx_poll(struct hrtimer *timer) * tx_lock held. Lock-free on purpose: apple_mbox_poll_locked() invokes the * rx callback with rx_lock held, and a callback that sends would then take * tx_lock under rx_lock. Kick never touches rx_lock, so no ordering exists - * to invert. A kick that loses a race with the timer's final no-restart - * decision leaves the fast poller disarmed once; the next jiffy poll or - * send recovers it. + * to invert. Only the kick that moves rx_fast_armed from 0 to 1 starts the + * timer, so a kick never enqueues the timer while the tick forwards it. */ static void apple_mbox_fast_rx_kick(struct apple_mbox_priv *priv) { WRITE_ONCE(priv->rx_fast_until, ktime_add_us(ktime_get(), APPLE_MBOX_RX_FAST_GRACE_US)); - /* Publish the deadline before deciding to arm. */ - smp_mb(); - if (!hrtimer_active(&priv->rx_fast_timer)) + /* The full barrier in xchg() publishes the deadline before the arm. */ + if (!atomic_xchg(&priv->rx_fast_armed, 1)) hrtimer_start(&priv->rx_fast_timer, us_to_ktime(APPLE_MBOX_RX_FAST_POLL_US), HRTIMER_MODE_REL); } +/* + * After this the timer is neither queued nor running. The flag is cleared only + * when a queued timer was cancelled: a kick that already took the flag but has + * not called hrtimer_start() yet keeps it, and its timer starts afterwards. + * An idle timer with a stale flag cannot occur, because every tick exit that + * returns NORESTART clears the flag first. + */ +static void apple_mbox_fast_rx_cancel(struct apple_mbox_priv *priv) +{ + if (hrtimer_cancel(&priv->rx_fast_timer)) + atomic_set(&priv->rx_fast_armed, 0); +} + int apple_mbox_send(struct apple_mbox *mbox, const struct apple_mbox_msg msg, bool atomic) { @@ -1139,7 +1178,7 @@ static void apple_mbox_v8_rx_stop(struct apple_mbox *mbox) disable_irq(mbox->irq_recv_not_empty); /* Same cancel-and-drain guarantee for the fast poller as below. */ if (mbox->hw->rx_poll_only) - hrtimer_cancel(&priv->rx_fast_timer); + apple_mbox_fast_rx_cancel(priv); cancel_delayed_work_sync(&priv->rx_poll_work); } @@ -1328,7 +1367,7 @@ static void apple_mbox_quiesce(void *data) /* The v8 poller also stops on the removal fence; make sure it is gone. */ if (apple_mbox_has_rx_poller(mbox)) { if (mbox->hw->rx_poll_only) - hrtimer_cancel(&apple_mbox_to_priv(mbox)->rx_fast_timer); + apple_mbox_fast_rx_cancel(apple_mbox_to_priv(mbox)); cancel_delayed_work_sync(&apple_mbox_to_priv(mbox)->rx_poll_work); }