From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f202.google.com (mail-pl1-f202.google.com [209.85.214.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7B8A229D267 for ; Mon, 23 Mar 2026 05:14:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.202 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774242842; cv=none; b=IrzAtaLwpxvj3T8mTJSEQDTf2ujm24ZMzxWJGo3RZ9SFHHCsr3TDuFqCj/h7KabrX+YfWqM9NxBDv7BGGUWBawSDm5Oz977nvckpOL2mz2HXzrjBpumpumP1OyBbfIdZYe58iACCUD0tC6ridgx1PeFMMClgIpm8ScilUgpsLJE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774242842; c=relaxed/simple; bh=z6Zcgc+MXbi/QN7rQlvUnN2XBI1eboxvkMdP4BVwfr8=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=h5+X/1eGhjn5hMVylkq1QsOKezcNr9SiCjUi3nFsL45ezITqpv1HFugZX1ujR9oTh3iVcPLVhnCQWHClYLtp4hsf+GTAHcu4r+RSQgfLLRwJsxWyUfeO+eqkjt5dFRoUobTY5tXH3pdi14qSnaKNjsoJavJ7qGF8LldZDod2GoA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--joonwonkang.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=jASpxxYf; arc=none smtp.client-ip=209.85.214.202 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--joonwonkang.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="jASpxxYf" Received: by mail-pl1-f202.google.com with SMTP id d9443c01a7336-2b05a3c2421so46142145ad.1 for ; Sun, 22 Mar 2026 22:14:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1774242841; x=1774847641; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=J6K2uMlwdA0abghu8MqcrivTleBIcvizA9okA+4Axro=; b=jASpxxYfFk6r4L7Bz1+MgIAB3upjqnlHURnED2UvAsXuGpFdU/A6Ya7Yw2benaHXRq PHjqbWEBJb/ElpZYRfQlyQbh/G4UP9fApX98VyOmxIREE6/BYFfc0LIIH2tY0emfCGpO nbNdWwsocuYU9mGqrxI9MzdRviXOs6JfWFdjcUvXBvHbtnRa+5vHs3GmlECmgsDVsVBv 9aSXrJiF2/4UYeRuJ7aoPutaeqSvgN24GUVjhjrQtSQHYdOmuWJjphFEMbUwQ5Bfjcc7 9JvQ6+V/7fKP2EW+p7Ycr674amde0eE0Jfx+yEzZdW2yvVqUEJZ5piAtYqTK9SQJNnz2 HeWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774242841; x=1774847641; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=J6K2uMlwdA0abghu8MqcrivTleBIcvizA9okA+4Axro=; b=qeXCWxSv0U/ZzeX3Li1zkB5t0xTF+CwHnmhyAV1W+/0jVX6O691hgpRjmBx7EHcYHQ s9wBG7sxG51opPZV4fOUN2NGQFDSZfiGoREWynhPFF9txXOQ5S17jNoi203iVxQ6ZPbW 4okURort27iEEbhYtJn90H43Fjd+6OhYVIiN2o45dXtlQ6HuMr3YlX6cPJMMCxV+1Zks p121xktquJs+8cT5CGJU0FlXFOOqwS2IDMhpsWYYN3IKqxVlpnbYCB8BaAILOQ/ecrg+ OVYJbq2D65myRaG6obQ1I5j+1yX0mGEbbd/uzHcBgt4lLQPMIInZo9VQF+sv5+R+gfWy z8qw== X-Forwarded-Encrypted: i=1; AJvYcCUpc78lTVCgD0vBDubxqiB1VSuSBe8vK9RACvH1K/TlxSLbzfqNQzzl2Zmcd+I1lQ/O80RnMcViNjbaF68=@vger.kernel.org X-Gm-Message-State: AOJu0Yw1pHY1ZOKZ/3QsQ3kkGHrrSyWxvCa9KdhS2u7lxtqo9DE6Te/a RojChRmXbFlCoGzh5mKUn9Y+BhA8K8vJuAam5GR+UOM4sGGYzshC93A846toG5GfX0UJP0ko3I0 Ccd/xY8NTjkThLIzNaCJFw6loSA== X-Received: from plha16.prod.google.com ([2002:a17:902:ecd0:b0:2b0:5538:b55d]) (user=joonwonkang job=prod-delivery.src-stubby-dispatcher) by 2002:a17:902:e5ce:b0:2b0:59c4:e9dc with SMTP id d9443c01a7336-2b08271d4ecmr101423305ad.22.1774242840657; Sun, 22 Mar 2026 22:14:00 -0700 (PDT) Date: Mon, 23 Mar 2026 05:13:59 +0000 In-Reply-To: <20260322171752.608486-1-jassisinghbrar@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20260322171752.608486-1-jassisinghbrar@gmail.com> X-Mailer: git-send-email 2.53.0.959.g497ff81fa9-goog Message-ID: <20260323051359.3167665-1-joonwonkang@google.com> Subject: Re: [PATCH] mailbox: Fix NULL message support in mbox_send_message() From: Joonwon Kang To: jassisinghbrar@gmail.com Cc: andersson@kernel.org, dianders@chromium.org, joonwonkang@google.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, maz@kernel.org, shawn.guo@linaro.org, stable@vger.kernel.org, tglx@kernel.org, akpm@linux-foundation.org Content-Type: text/plain; charset="UTF-8" > The active_req field serves double duty as both the "is a TX in > flight" flag (NULL means idle) and the storage for the in-flight > message pointer. When a client sends NULL via mbox_send_message(), > active_req is set to NULL, which the framework misinterprets as > "no active request". This breaks the TX state machine by: > > - tx_tick() short-circuits on (!mssg), skipping the tx_done > callback and the tx_complete completion > - txdone_hrtimer() skips the channel entirely since active_req > is NULL, so poll-based TX-done detection never fires. > > Fix this by introducing a MBOX_NO_MSG sentinel value that means > "no active request," freeing NULL to be valid message data. The > sentinel is defined in the subsystem-internal mailbox.h so that > controller drivers within drivers/mailbox/ can reference it, but > it is not exposed to clients outside the subsystem. It sounds that it allows future controller drivers also to refer to the new sentinel pointer value. > > Fifteen in-tree callers send NULL (doorbell-style IPCs on Qualcomm, > Tegra, TI, Xilinx, i.MX, SCMI, and PCC platforms). All were > audited for regression: > > - Most already work around the bug via knows_txdone=true with a > manual mbox_client_txdone() call, making the framework's > tracking irrelevant. These are unaffected. > > - Poll-based callers (Xilinx zynqmp/r5) are strictly better off: > the poll timer now correctly detects NULL-active channels > instead of silently skipping them. > > - irq-qcom-mpm.c was a pre-existing bug -- the only Qualcomm > caller that omitted the knows_txdone + mbox_client_txdone() > pattern. Fixed in a companion commit ("irqchip/qcom-mpm: Fix > missing mailbox TX done acknowledgment"). > > - No caller sets both a tx_done callback and sends NULL, nor > combines tx_block=true with NULL sends, so the newly reachable > callback/completion paths are never exercised. > > Also update tegra-hsp's flush callback, which directly inspects > active_req to wait for the channel to drain: the old "!= NULL" > check becomes "!= MBOX_NO_MSG", otherwise flush spins until > timeout since the sentinel is non-NULL. > > The only tradeoff is that 'MBOX_NO_MSG' can not be used as a message > by clients. The other, but I guess more important, tradeoff is that future controller driver developers should now know that the pointer value of `->active->req` could be -1(== MBOX_NO_MSG) other than conventional pointer value(memory address, NULL, or error-encoded pointer value). Although I am still not sure if this is better than changing the type from pointer to integer index to make the intention clearer, could you add API doc for `->active_req` that it may become MBOX_NO_MSG when no active request exists? Otherwise, it is likely that the future driver developers just use NULL to check the channel emptiness and also are not aware that this pointer could be assigned the unconventional value -1. Just like this mis-use case of `->active_req` was not caught in tegra-hsp.c in the first attempt of this patch, it could be hard the same way for controller driver developers to avoid/prevent the mis-use without proper API doc. Thanks.