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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJ9a7VggKoYdELcWTXxOg6-78TK2NAEE9uNo6cgpmx0x=0cwsQ@mail.gmail.com>
Date: Thu, 23 Oct 2025 11:18:50 +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 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.





> [...]
>
> > > > +/*
> > > > + * 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