linux-kernel - Re: [PATCH 1/1] coresight: fix issue where coresight component has no claimtags

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: <CAJ9a7VgOPgvzRmGg_a_W_LgFNVXBoxBH62XZar53s0W5c=4pvg@mail.gmail.com>
Date: Thu, 23 Oct 2025 12:03:07 +0100
From: Mike Leach <mike.leach@...aro.org>
To: Leo Yan <leo.yan@....com>
Cc: coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, suzuki.poulose@....com, james.clark@...aro.org
Subject: Re: [PATCH 1/1] coresight: fix issue where coresight component has no claimtags

Hi Leo

On Thu, 23 Oct 2025 at 11:18, Mike Leach <mike.leach@...aro.org> wrote:
>
> Hi Leo
>
> On Wed, 22 Oct 2025 at 15:04, Leo Yan <leo.yan@....com> wrote:
> >
> > Hi Mike,
> >
> > On Wed, Oct 22, 2025 at 01:35:46PM +0100, Mike Leach wrote:
> >
> > [...]
> >
> > > > - We can add a new flag ("bool claim_impl" in the struct csdev_access),
> > > >   by default the field will be zero.
> > > >
> > >
> > > I considered a bool - but the correct place for this would be
> > > coresight_device - where we keep all the information about the
> > > hardware features.
> >
> > Maybe coresight_device is suitable. But this might require to update
> > furthermore for claim function arguments (e.g., need to pass "csdev"
> > rather than "csdev_access").
> >
> > > > - The drivers support claim tags call coresight_clear_self_claim_tag()
> > > >   in probe (see __catu_probe() as an example), we can call a new
> > > >   function coresight_init_claim_tag() to replace it, this function sets
> > > >   "claim_impl" properly and clear the tag if supported.
> > > >
> > >
> > > I considered moving this initialisation to the common coresight code
> > > where we create the coresight_device.
> >
> > Seems to me, this is dangerous. If a module is not CoreSight compliant
> > and claim registers are absent, when access we will get unknown values
> > or even cause serious result (external abort or bus lockup).
> >
>
> Currently all our drivers assume claim registers are compliant - so
> this makes no functional difference to the current situation.
>
> If the hardware is non-compliant, and is bound to the current drivers,
> then there will be an access to the registers that could cause the
> same issues.
>
> There is little sense in having a call in individual drivers to init
> claim tags which are regarded as common management structure.
> Any coresight component with no claim tags must still either have
> registers indicating no claim tags or as per the specification
> unimplemented register locations have to be  RAZ/WI - which amounts to
> the same thing.
>
>

Having re-visited this - the cpu bound ETMs use a smp call before
creating the coresight_device so it would be impractical to move this
into the common code or use csdev parameters to make the checks.

The only time we claim  / disclaim is during enable / disable device
when we are writing many 10s of registers anyway. It does not seem too
much of  a performance hit to add one additional read per enable

Regards

Mike

>
>
>
> > [...]
> >
> > > > > +/*
> > > > > + * Coresight specification defines a maximum of 8 claim tag bits.
> > > > > + * The precise number is implementation defined, and may be obtained by
> > > > > + * reading the CLAIMSET register.
> > > > > + */
> > > > > +#define CORESIGHT_CLAIM_BITS_MAX_MASK        GENMASK(7, 0)
> > > > > +#define CORESIGHT_CLAIM_SELF_HOSTED_BIT      BIT(1)
> > > >
> > > > Now we only care about the self-host bit. Can reuse existed macros ?
> > > >
> > >
> > > I feel that drivers should be written to match the specification - the
> > > macros above are a correct mask value per specification and the
> > > correct bitfield comparison for the MSBit of the protocol.
> > >
> > > The ones below are a protocol masks field, and a specific protocol
> > > value that is used in value rather than bit comparisons in the
> > > claimtag code. I felt it clearer to differentiate between the uses
> > > when reading the code.
> >
> > How about change CORESIGHT_CLAIM_EXTERNAL and
> > CORESIGHT_CLAIM_SELF_HOSTED to MSBit ?
> >
>
> The checking code for the protocol does a value compare on the pair of
> bits, not individual bit checks.
>
> Regards
>
>
> Mike
>
>
> > Thanks,
> > Leo
>
>
>
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK



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