From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f201.google.com (mail-pf1-f201.google.com [209.85.210.201]) (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 7AEF51E511 for ; Tue, 17 Mar 2026 05:03:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773723787; cv=none; b=BgEtv4p5vi7V1jtg6d8ZmobXYk5zpJ9+++XUVpT4nTuR2CYaj2cO7XynLKjE8TvOBJs0gvlPO4vHxe08eV06PE6eMJPXKzS0cYvIyj0bwdKHKsvVlOhQLIkbKQps/mB9HpQB4aA1tsRPRnBL1Yl52Fd/Fh1jzO/rJCvLhbdcHn4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773723787; c=relaxed/simple; bh=VxkfTmjy+nzlP19AmZAdbPt/C91UYW0EpxC3UGEM620=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=lCrjR6OCXPI2+Db+3ZqjD+6sQnsRbPsajLK7r/e2XihO0WhkutG6Ibm6wxwIVNztrx4AQF6HwCLydUzaC/IPJEGrOseEe6cUxIjE1NO1+mQV+sXMJcPq0aYVxwUefJdzPcvRijJs4NpAeZ5x40wtlKTTlp4hi6xNK0wkt2WqVWU= 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=I2w38LdL; arc=none smtp.client-ip=209.85.210.201 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="I2w38LdL" Received: by mail-pf1-f201.google.com with SMTP id d2e1a72fcca58-829b7ed8964so5452630b3a.2 for ; Mon, 16 Mar 2026 22:03:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1773723786; x=1774328586; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:from:to:cc:subject:date:message-id :reply-to; bh=CFeh4+y5NNLjz0dbO14r7JLgb5QX3kZJnnchx+50iSQ=; b=I2w38LdLX/dGu7HWD3sZzGqls3R0LJr3Qnw8HkLdIhK6noz7SfkMSE3KmDWXhapTvC VkXOiaSn4UJODOSrPG6wqOyPIS16/lvM5wcHiiidcSEyVrDq6QzScwlqaPuZZMzc9xhM OGeFLtB7R5pEJLyfbvl7mLbKbGEgx0ySeKphWaqhuJg42HBfsh+oWETDzs6OZYldgFIv bVUwst2izwZMEpbPOA+3hcPUX9sxWKjN//wakcutM6MeXYmsxl64VzEMQYGXTBZgheW4 RTF9xoTNGH46v/clXHrPwJIihllOesw2CqwlbWoye8gtG4yvcULgEeJpJDPnVdkobx4c d76g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1773723786; x=1774328586; h=content-transfer-encoding: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=CFeh4+y5NNLjz0dbO14r7JLgb5QX3kZJnnchx+50iSQ=; b=FDrhropvz2Al9swUYL9YWmPPQkJwVFzHnMWlePDJ8SH7R+5FVSKWGPqithLTqvOK7K jTT06zr6dr6QE+L+jghgVMUN5G5o5GcWstYbX3vmsr9kz3FVfDZLMUqeE27OdY7YB9Vn NKxtXr1nIUHhJJfaeVoXJEb9f+g529BZgV7ES6GI1V8vmIrPjSvIEWJUAygVsG05wnmo G83PIT7rim+YCO9/n9RwyTA527/0KqDgAGVqf/TSAgqkdGI4tSopnYiJ2+Qk4O6/Zrtx KV+fNuqSlQIY8xD1ALZQzAtgddX1mGAx8pyXjeujzFvpNvHIqJ15WFZ8lDAVPJPUHVxu 7QLg== X-Forwarded-Encrypted: i=1; AJvYcCXJWF7WhyOko4O0g91C0++PP99SidOG8OKGm76TpeP07VMpDYMk4BYdgTnt29mOsBoDmhMW8uTqBhQnT2M=@vger.kernel.org X-Gm-Message-State: AOJu0Yzkgi3MJeIHcn/2zVpnOlovZto2yPiEsFM4VpTGl9MWSRAL7AmZ V9RMc5L+ZKCt9MqtddFHDq4zSlgxGnjsvPRKhqCCN0CnvlnN0nrp7fwDxI6Prrg/bEECtn0LMy6 g6qZInRWweC5qAJCCyZYRIWhIgQ== X-Received: from pfvb14.prod.google.com ([2002:a05:6a00:cce:b0:824:bb96:dae]) (user=joonwonkang job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6a00:b8e:b0:81f:4f47:c6d5 with SMTP id d2e1a72fcca58-82a1971daf1mr15205648b3a.25.1773723785549; Mon, 16 Mar 2026 22:03:05 -0700 (PDT) Date: Tue, 17 Mar 2026 05:03:02 +0000 In-Reply-To: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: X-Mailer: git-send-email 2.53.0.851.ga537e3e6e9-goog Message-ID: <20260317050303.937620-1-joonwonkang@google.com> Subject: Re: [PATCH] RFC: mailbox: Fix NULL message support in mbox_send_message() From: Joonwon Kang To: jassisinghbrar@gmail.com Cc: arnd@arndb.de, dianders@chromium.org, joonwonkang@google.com, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable > On Fri, Mar 13, 2026 at 5:12=E2=80=AFAM Joonwon Kang wrote: > > > > > 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 internal to the mailbox core and is never exposed to > > > controller drivers or clients. > > > > The following drivers are currently using ->active_req which now could = be > > assigned MBOX_NO_MSG. > > - drivers/mailbox/tegra-hsp.c > > - drivers/mailbox/mtk-vcp-mailbox.c > > > Good catch, Thanks. > mtk-vcp-mailbox.c is fine as is - it assumes only valid requests. This > patch will not introduce any change for it. > Yes, tegra-hsp.c should now track MBOX_NO_MSG instead of NULL. Single > line change. >=20 A more important aspect than single line change would be that you are creating a new API contract that the controller drivers who are to use ->active_req should now have new knowledge that the pointer value could be unconventional value such as -1. Without this additional knowledge, they may easily fail checking if the channel is empty. > > One of them is using ->active_req to wait until the channel is empty. I= n > > this case, strictly speaking, that controller driver should be aware of > > the sentinel value MBOX_NO_MSG, which means the sentinel value should b= e > > exposed to the controller. Or, if a future controller driver to come is= to > > use ->active_req for the same purpose for doorbell or non-doorbell, it > > should also be aware of the sentinel value anyway. > > > > However, I believe that it is not intuitive to the controller developer= s > > that a pointer value could be other value than a real memory address, > > NULL or error encoded value, which is -1(=3D=3D MBOX_NO_MSG). For this = reason, > > I think it will be better to change the type of ->active_req to give a > > better indication to the controller developers, e.g. to integer as in t= he > > original patch > > https://lore.kernel.org/all/20251126045926.2413532-1-joonwonkang@google= .com/. > > > > Or, we could change those drivers not to use ->active_req, hide > > ->active_req entirely in the mailbox core and keep this patch. > > > Yes, ideally controller drivers should not track active_req. But this > patch will cause least churn it seems. If we will take those two drivers as an exceptional and not recommended case that uses ->active_req directly, it will be fine not to change them except for tegra-hsp.c to check MBOX_NO_MSG. If that is the case and you are to keep this patch, please keep in mind to leave the original patch link in the commit message for better trackability of the discussion and solutions and for credit for the contribution of finding and analyzing this issue and proposing the first solution, which I believe is also important for future contributions to come. Thanks.