From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f177.google.com (mail-pf1-f177.google.com [209.85.210.177]) (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 D97A12E6CC6 for ; Fri, 21 Nov 2025 15:37:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.177 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763739437; cv=none; b=i3RCN5GblC+seIPKGSs5kDtlXT0LclwKvoTsxMZReqwdsAlPhQtjigrYKs/dWCDLxpAY/q3LBC7ZxSvt+EA6RP4+kTWIjLnJ8pSGvoCOWQZGe21d//v/14aSuaUgMWTvm3wcHIiFIqQxZ5AGyxBXkgmsinyWlAqvhP6Rb/u0l2k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763739437; c=relaxed/simple; bh=9SEYQ6o3otkAfW+rZ8gGxm5y9lr5TZX3GX9hh60Fq54=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Gy9MHV02w5xUoynBn8tIsQXNS2xYf80y/p+Bu2JSqwtbJdjD3H8OeBH70Lv5rE/FXMIm3BW+Rl1s2rFVknejiGKxoYQTXGZgBkp5xSNJjvM5Lhg+p0o8XDX1qRVvZoKSSMBhSasAVdGcfLoYRC9F17i1139FLvhEH5rqdqdFkYY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=dihh+YYZ; arc=none smtp.client-ip=209.85.210.177 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="dihh+YYZ" Received: by mail-pf1-f177.google.com with SMTP id d2e1a72fcca58-7bb710d1d1dso3209765b3a.1 for ; Fri, 21 Nov 2025 07:37:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1763739434; x=1764344234; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=wn1kSEu9OwZ97Yg1s+a7dWNpqwCIOmyY5CJUhlq1Tr8=; b=dihh+YYZkSJnkgAmmAONoQ9feKxp3fbL3JtWqaX6ZNBMX6baBGynrm0x39UrRrrs3W sSpyDF3wAQGKWzAgr8M05L2pk466rbBQSPcIFNJ0wCBMVbz7fAb0dH65Ibb0vRWt+kUn RlK7N7VexdFt4DvjS6+7VKho6Ryot88uc3iNXqdU3a1+VkXMYrGBA3H85Z+XSwIUG3N3 IxXV41nIhzcaYHTu0g1jk4Lp0V4QQ7PNBTY90mRnjLswysOEtT3IZd1rOmtVTw0juf4P o6n2/fvZrMvEbp5Nb13KG9Ak7wWfi1W5v1WBboK51UwUuRiHTfSrItatxeZLnKGQqdN5 4Vcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763739434; x=1764344234; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=wn1kSEu9OwZ97Yg1s+a7dWNpqwCIOmyY5CJUhlq1Tr8=; b=LQMiq4+FbJibI7ZtYNcVX3zRroiHie8LaNz1akW451HbCIqqq1pH2/+4twAlaczHbo AStc8EIeoO6xpJeOIT4yeOxl5cNIF1lWwyrDSHit2Vh+t6XayzHIuiihPDOL8ghUm4S8 e6Swv+1pbwwwNawrSNtAuNniwetV/rsh1mhIqyxY7s/3uql/l8sNbt23HyEWaeTUvP6r LVfFIFYBZ7UZ5DnyzIiZFEMg5DW94/wxtCNr7QE+ua5IhjKQJa3qZ6+B+yI99Ka+rFd1 i7Tcq1ED1sVt6cAHHBjumGHOVIi3YqdeV9XqIoSkbeLurRzspyX3Oe9y49nZyJmgOibY jx6w== X-Forwarded-Encrypted: i=1; AJvYcCUZlPtHYWt70X66ZOgUtnFJUrHGzAAruLJ+pgb1Q8y4iSH1o6lcAWTjRd1zS9ehhClMVTZbRYTtpKHoOjo=@vger.kernel.org X-Gm-Message-State: AOJu0YwI4k82qgyYxBCW71aFYaPKh1frJyNvZA5ZTkfbzKknc5hC9kjX t9OYP3ixFHA4FrNSe3Pfsk4okTfQKTjp03BYzWbQiuUR+Tp1N2sTear3U9A8QLPqpeL7CY4hX8m mlC9dnO0= X-Gm-Gg: ASbGnctiGw+16Yfk08ZuJxv23aNR2Z8SfXZo7HCw7vnviz2PIBVzX/l9OI1erNEUVCm ljflN0LfdIbY1r0Z91BHhifwzUujJZu2X24ZiBuBChGw+y+0QsNPhLtip2wSqCgc8T07bvAgpeT q2zEAHolRJ1xcMcPBb4CnfiuYB/9fUhFZczMlVlNVOB24cvSA5ofQ3lXxdq/DVcQtFgTdeKQPnA VMz0tyzoaqBtxOJjRJ4enD8ZmdWGGNSqsBBh7GtBs2OQjXFVgZ/9GYANJNt00v7ZrK2QpDXwppe fEfPxsDR0DO6Ei2iauJpKiT8Zm3A8+MskJb9oXWdZrz1cr7ebGY+6tBfZCk3fs6fmv8/nPXBSoT T3hVP0mbdepp0YnQhhLHdYoEbjRfw4iLHxShIiG2/WkiYkXeMwWsN8sF+a3Nx1mW4qPtBG/UlWJ qT2titSfObTgkr9g== X-Google-Smtp-Source: AGHT+IHf0JXGqxWOmSObNRkoxYuE/072gCT9gskWkAHGDfr+qbbCbb7muBRs8XMkmwuAj5teff3rew== X-Received: by 2002:a05:6a20:728b:b0:358:dc7d:a2be with SMTP id adf61e73a8af0-3614ebab4admr3929217637.17.1763739433864; Fri, 21 Nov 2025 07:37:13 -0800 (PST) Received: from p14s ([2604:3d09:148c:c800:4d45:74bb:af4e:251c]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-7c3f024b7d2sm6326077b3a.40.2025.11.21.07.37.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Nov 2025 07:37:13 -0800 (PST) Date: Fri, 21 Nov 2025 08:37:10 -0700 From: Mathieu Poirier To: Tanmay Shah Cc: andersson@kernel.org, linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 3/3] remoteproc: xlnx: add crash detection mechanism Message-ID: References: <20251113154403.2454319-1-tanmay.shah@amd.com> <20251113154403.2454319-4-tanmay.shah@amd.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20251113154403.2454319-4-tanmay.shah@amd.com> On Thu, Nov 13, 2025 at 07:44:04AM -0800, Tanmay Shah wrote: > Remote processor will report the crash reason via the resource table > and notify the host via kick. The host checks this crash reason on > every kick notification from the remote and report to the core > framework. Then the rproc core framework will start the recovery > process. Please substitute the word "kick" for "mailbox notification". I also have to assume "core framework" and "rproc core framework" are the same. Pick one and stick with it. > > Signed-off-by: Tanmay Shah > --- > > Changes in v2: > - clear attach recovery boot flag during detach and stop ops > > drivers/remoteproc/xlnx_r5_remoteproc.c | 56 +++++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > > diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c > index 8677b732ad14..5d04e8c0dc52 100644 > --- a/drivers/remoteproc/xlnx_r5_remoteproc.c > +++ b/drivers/remoteproc/xlnx_r5_remoteproc.c > @@ -108,6 +108,10 @@ struct rsc_tbl_data { > const uintptr_t rsc_tbl; > } __packed; > > +enum fw_vendor_rsc { > + FW_RSC_VENDOR_CRASH_REASON = RSC_VENDOR_START, > +}; > + > /* > * Hardcoded TCM bank values. This will stay in driver to maintain backward > * compatibility with device-tree that does not have TCM information. > @@ -127,9 +131,21 @@ static const struct mem_bank_data zynqmp_tcm_banks_lockstep[] = { > {0xffe30000UL, 0x30000, 0x10000UL, PD_R5_1_BTCM, "btcm1"}, > }; > > +/** > + * struct xlnx_rproc_crash_report - resource to know crash status and reason > + * > + * @crash_state: if true, the rproc is notifying crash, time to recover > + * @crash_reason: reason of crash > + */ > +struct xlnx_rproc_crash_report { > + u32 crash_state; > + u32 crash_reason; > +} __packed; > + > /** > * struct zynqmp_r5_core - remoteproc core's internal data > * > + * @crash_report: rproc crash state and reason > * @rsc_tbl_va: resource table virtual address > * @sram: Array of sram memories assigned to this core > * @num_sram: number of sram for this core > @@ -143,6 +159,7 @@ static const struct mem_bank_data zynqmp_tcm_banks_lockstep[] = { > * @ipi: pointer to mailbox information > */ > struct zynqmp_r5_core { > + struct xlnx_rproc_crash_report *crash_report; > void __iomem *rsc_tbl_va; > struct zynqmp_sram_bank *sram; > int num_sram; > @@ -227,10 +244,14 @@ static void handle_event_notified(struct work_struct *work) > static void zynqmp_r5_mb_rx_cb(struct mbox_client *cl, void *msg) > { > struct zynqmp_ipi_message *ipi_msg, *buf_msg; > + struct zynqmp_r5_core *r5_core; > + struct rproc *rproc; > struct mbox_info *ipi; > size_t len; > > ipi = container_of(cl, struct mbox_info, mbox_cl); > + r5_core = ipi->r5_core; > + rproc = r5_core->rproc; > > /* copy data from ipi buffer to r5_core */ > ipi_msg = (struct zynqmp_ipi_message *)msg; > @@ -244,6 +265,13 @@ static void zynqmp_r5_mb_rx_cb(struct mbox_client *cl, void *msg) > buf_msg->len = len; > memcpy(buf_msg->data, ipi_msg->data, len); > > + /* Check for crash only if rproc crash is expected */ > + if (rproc->state == RPROC_ATTACHED || rproc->state == RPROC_RUNNING) { > + if (r5_core->crash_report->crash_state) > + rproc_report_crash(rproc, > + r5_core->crash_report->crash_reason); At this stage ->crash_state indicates that a crash occured, but how is it reset once the crash has been handle? How do we make sure the next mailbox notification won't trigger another crash report? > + } > + > /* received and processed interrupt ack */ > if (mbox_send_message(ipi->rx_chan, NULL) < 0) > dev_err(cl->dev, "ack failed to mbox rx_chan\n"); > @@ -397,6 +425,7 @@ static int zynqmp_r5_rproc_start(struct rproc *rproc) > if (ret) > dev_err(r5_core->dev, > "failed to start RPU = 0x%x\n", r5_core->pm_domain_id); > + Spurious change > return ret; > } > > @@ -438,6 +467,8 @@ static int zynqmp_r5_rproc_stop(struct rproc *rproc) > if (ret) > dev_err(r5_core->dev, "core force power down failed\n"); > > + test_and_clear_bit(RPROC_FEAT_ATTACH_ON_RECOVERY, rproc->features); > + > return ret; > } > > @@ -874,6 +905,8 @@ static int zynqmp_r5_get_rsc_table_va(struct zynqmp_r5_core *r5_core) > > static int zynqmp_r5_attach(struct rproc *rproc) > { > + rproc_set_feature(rproc, RPROC_FEAT_ATTACH_ON_RECOVERY); > + Why can't this be set in probe() and left alone from thereon? > dev_dbg(&rproc->dev, "rproc %d attached\n", rproc->index); > > return 0; > @@ -888,6 +921,8 @@ static int zynqmp_r5_detach(struct rproc *rproc) > */ > zynqmp_r5_rproc_kick(rproc, 0); > > + clear_bit(RPROC_FEAT_ATTACH_ON_RECOVERY, rproc->features); > + I'm not sure why this needs to be done, same comment for zynqmp_r5_rproc_stop(). > return 0; > } > > @@ -896,6 +931,26 @@ static void zynqmp_r5_coredump(struct rproc *rproc) > (void)rproc; > } > > +static int zynqmp_r5_handle_crash_rsc(struct rproc *rproc, void *rsc, > + int offset, int avail) > +{ > + struct zynqmp_r5_core *r5_core = rproc->priv; > + > + r5_core->crash_report = > + (struct xlnx_rproc_crash_report *)(r5_core->rsc_tbl_va + offset); > + This function is so simple that I would fold it in zynqmp_r5_handle_rsc() below. Thanks, Mathieu > + return RSC_HANDLED; > +} > + > +static int zynqmp_r5_handle_rsc(struct rproc *rproc, u32 rsc_type, void *rsc, > + int offset, int avail) > +{ > + if (rsc_type == FW_RSC_VENDOR_CRASH_REASON) > + return zynqmp_r5_handle_crash_rsc(rproc, rsc, offset, avail); > + > + return RSC_IGNORED; > +} > + > static const struct rproc_ops zynqmp_r5_rproc_ops = { > .prepare = zynqmp_r5_rproc_prepare, > .unprepare = zynqmp_r5_rproc_unprepare, > @@ -911,6 +966,7 @@ static const struct rproc_ops zynqmp_r5_rproc_ops = { > .attach = zynqmp_r5_attach, > .detach = zynqmp_r5_detach, > .coredump = zynqmp_r5_coredump, > + .handle_rsc = zynqmp_r5_handle_rsc, > }; > > /** > -- > 2.34.1 >