linux-kernel - Re: [PATCH v3 2/2] coresight: core: Disable helpers for devices that fail to enable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CALJ9ZPPdqrAJkV7CHJBLGKoqQm5hmR7s_4_qRJ=gWyTvxJtL-g@mail.gmail.com>
Date: Fri, 25 Apr 2025 14:02:10 -0700
From: Yabin Cui <yabinc@...gle.com>
To: Mike Leach <mike.leach@...aro.org>
Cc: James Clark <james.clark@...aro.org>, coresight@...ts.linaro.org, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	Suzuki K Poulose <suzuki.poulose@....com>, Leo Yan <leo.yan@....com>, 
	Jie Gan <quic_jiegan@...cinc.com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>
Subject: Re: [PATCH v3 2/2] coresight: core: Disable helpers for devices that
 fail to enable

On Fri, Apr 25, 2025 at 8:25 AM Mike Leach <mike.leach@...aro.org> wrote:
>
> On Tue, 15 Apr 2025 at 14:51, James Clark <james.clark@...aro.org> wrote:
> >
> >
> >
> > On 08/04/2025 8:59 pm, Yabin Cui wrote:
> > > When enabling a SINK or LINK type coresight device fails, the
> > > associated helpers should be disabled.
> > >
> > > Signed-off-by: Yabin Cui <yabinc@...gle.com>
> > > Suggested-by: Suzuki K Poulose <suzuki.poulose@....com>
> > > ---
> > >   drivers/hwtracing/coresight/coresight-core.c | 9 +++++++--
> > >   1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> > > index fb43ef6a3b1f..a56ba9087538 100644
> > > --- a/drivers/hwtracing/coresight/coresight-core.c
> > > +++ b/drivers/hwtracing/coresight/coresight-core.c
> > > @@ -486,8 +486,10 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
> > >                        * that need disabling. Disabling the path here
> > >                        * would mean we could disrupt an existing session.
> > >                        */
> > > -                     if (ret)
> > > +                     if (ret) {
> > > +                             coresight_disable_helpers(csdev);
> >
> > Hi Yabin,
> >
> > Unfortunately coresight_disable_helpers() takes a path pointer now so
> > this needs to be updated.
> >
> > I tested with that change made and it works ok.
> >
> > >                               goto out;
> > > +                     }
> > >                       break;
> > >               case CORESIGHT_DEV_TYPE_SOURCE:
> > >                       /* sources are enabled from either sysFS or Perf */
> > > @@ -496,10 +498,13 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
> > >                       parent = list_prev_entry(nd, link)->csdev;
> > >                       child = list_next_entry(nd, link)->csdev;
> > >                       ret = coresight_enable_link(csdev, parent, child, source);
> > > -                     if (ret)
> > > +                     if (ret) {
> > > +                             coresight_disable_helpers(csdev);
> > >                               goto err;
> > > +                     }
> > >                       break;
> > >               default:
> > > +                     coresight_disable_helpers(csdev);
> >
> > Minor nit, you could collapse these last two into "goto
> > err_disable_helpers" and add another label before err: that disables
> > helpers before falling through to err:.
> >
> > Other than that:
> >
> > Reviewed-by: James Clark <james.clark@...aro.org>
> >
> > >                       goto err;
> > >               }
> > >       }
> >
>
> Subject to James' comments -
>
> Reviewed-by: Mike Leach <mike.leach@...aro.org>
>

Hi Mike, I have uploaded the v4 patch as suggested by James, in
"[PATCH v4 2/2] coresight: core: Disable helpers for devices that fail
to enable".
Could you help review the v4 patch? In that patch, Leo suggested consolidating
error handling. But you expressed concern about it when reviewing the v2 patch.


>
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK