All CoreSight compliant components have an implementation defined number of 0 to 8 claim tag bits in the claim tag registers.
These are used to claim the CoreSight resources by system agents.
ARM recommends implementions have 4 claim tag registers.
The CoreSight drivers implement a 2 claim tag bit protocol to allow self hosted and external debug agents to manage access to the hardware.
However, if there are less than 2 claim tags available the protocol incorrectly returns an error on device claim, as no checks are made.
If insufficient claim tags are present in a component then the protocol must return success on claim / disclaim to allow components to be used normally.
Changes read the CLAIMSET bits to establish the number of available tags, and adjust the claim returns accordingly.
Applies to coresignt/next
Mike Leach (1): coresight: fix issue where coresight component has no claimtags
drivers/hwtracing/coresight/coresight-core.c | 26 ++++++++++++++++++++ drivers/hwtracing/coresight/coresight-priv.h | 8 ++++++ 2 files changed, 34 insertions(+)
Coresight components have 0 to 8 claim tag bits. ARM recommends 4 and the implemented claim tag protocol uses two of these.
If a component has insufficient claim tags then the protocol incorrectly returns an error when attempting to claim a component.
Fix by reading CLAIMSET to establish then actual number of claim tags and return success when attempting to claim a component were there are insufficient tags to implement the protocol.
Signed-off-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/coresight-core.c | 26 ++++++++++++++++++++ drivers/hwtracing/coresight/coresight-priv.h | 8 ++++++ 2 files changed, 34 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 3267192f0c1c..ce4eade8a1e3 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -131,6 +131,22 @@ coresight_find_out_connection(struct coresight_device *csdev, return ERR_PTR(-ENODEV); }
+/* + * Reading CLIAMSET returns a bitfield representing the number of claim tags + * implemented from bit 0 to bit nTag-1, valid bits set to 1. + * + * Claim protocol requires 2 bits so test for highest bit required, + * bit 1 - CORESIGHT_CLAIM_SELF_HOSTED_BIT + * + * return true if sufficient claim tags implemented for protocol + */ +static bool coresight_claim_tags_implemented_unlocked(struct csdev_access *csa) +{ + u32 claim_bits_impl = FIELD_GET(CORESIGHT_CLAIM_BITS_MAX_MASK, + csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMSET)); + return ((claim_bits_impl & CORESIGHT_CLAIM_SELF_HOSTED_BIT) != 0); +} + static u32 coresight_read_claim_tags_unlocked(struct coresight_device *csdev) { return FIELD_GET(CORESIGHT_CLAIM_MASK, @@ -156,6 +172,9 @@ EXPORT_SYMBOL_GPL(coresight_clear_self_claim_tag);
void coresight_clear_self_claim_tag_unlocked(struct csdev_access *csa) { + if (!coresight_claim_tags_implemented_unlocked(csa)) + return; + csdev_access_relaxed_write32(csa, CORESIGHT_CLAIM_SELF_HOSTED, CORESIGHT_CLAIMCLR); isb(); @@ -181,6 +200,10 @@ int coresight_claim_device_unlocked(struct coresight_device *csdev) return -EINVAL;
csa = &csdev->access; + + if (!coresight_claim_tags_implemented_unlocked(csa)) + return 0; + tag = coresight_read_claim_tags_unlocked(csdev);
switch (tag) { @@ -237,6 +260,9 @@ void coresight_disclaim_device_unlocked(struct coresight_device *csdev) if (WARN_ON(!csdev)) return;
+ if (!coresight_claim_tags_implemented_unlocked(&csdev->access)) + return; + if (coresight_read_claim_tags_unlocked(csdev) == CORESIGHT_CLAIM_SELF_HOSTED) coresight_clear_self_claim_tag_unlocked(&csdev->access); else diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 33e22b1ba043..f93cbc9bb36a 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -41,6 +41,14 @@ extern const struct device_type coresight_dev_type[]; #define CORESIGHT_CLAIM_SELF_HOSTED 2 #define CORESIGHT_CLAIM_INVALID 3
+/* + * 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) + #define TIMEOUT_US 100 #define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb)
On Wed, Oct 22, 2025 at 12:45:20AM +0100, Mike Leach wrote:
[...]
+/*
- Reading CLIAMSET returns a bitfield representing the number of claim tags
- implemented from bit 0 to bit nTag-1, valid bits set to 1.
- Claim protocol requires 2 bits so test for highest bit required,
- bit 1 - CORESIGHT_CLAIM_SELF_HOSTED_BIT
- return true if sufficient claim tags implemented for protocol
- */
+static bool coresight_claim_tags_implemented_unlocked(struct csdev_access *csa) +{
- u32 claim_bits_impl = FIELD_GET(CORESIGHT_CLAIM_BITS_MAX_MASK,
csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMSET));- return ((claim_bits_impl & CORESIGHT_CLAIM_SELF_HOSTED_BIT) != 0);
+}
The logic looks good to me.
Can we improve a bit with a cached flag to avoid every time checking the CLAIMSET, we can read it only once at init time instead.
- We can add a new flag ("bool claim_impl" in the struct csdev_access), by default the field will be zero.
- 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.
- Afterwards, simply check the "claim_impl" flag.
[...]
+/*
- 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 ?
CORESIGHT_CLAIM_MASK CORESIGHT_CLAIM_SELF_HOSTED
I don't mind if extend CORESIGHT_CLAIM_MASK to 8 bits and even remove CORESIGHT_CLAIM_INVALID (as it cannot cover other invalid values). Or we can defer the extension as needed later.
Thanks, Leo
Hi Leo
On Wed, 22 Oct 2025 at 10:30, Leo Yan leo.yan@arm.com wrote:
On Wed, Oct 22, 2025 at 12:45:20AM +0100, Mike Leach wrote:
[...]
+/*
- Reading CLIAMSET returns a bitfield representing the number of claim tags
- implemented from bit 0 to bit nTag-1, valid bits set to 1.
- Claim protocol requires 2 bits so test for highest bit required,
- bit 1 - CORESIGHT_CLAIM_SELF_HOSTED_BIT
- return true if sufficient claim tags implemented for protocol
- */
+static bool coresight_claim_tags_implemented_unlocked(struct csdev_access *csa) +{
u32 claim_bits_impl = FIELD_GET(CORESIGHT_CLAIM_BITS_MAX_MASK,csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMSET));return ((claim_bits_impl & CORESIGHT_CLAIM_SELF_HOSTED_BIT) != 0);+}
The logic looks good to me.
Can we improve a bit with a cached flag to avoid every time checking the CLAIMSET, we can read it only once at init time instead.
When I considered how often we use the claim tags - compared to all the other programming of devices, a single extra read did not seem excessive.
- 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.
- 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. Certainly with the new check for number of claim tags it is safe for all devices, and there would appear to be no reason to do it as early as it appears in some of the drivers. However that would be a larger change.
- Afterwards, simply check the "claim_impl" flag.
[...]
+/*
- 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.
CORESIGHT_CLAIM_MASK CORESIGHT_CLAIM_SELF_HOSTED
I don't mind if extend CORESIGHT_CLAIM_MASK to 8 bits and even remove CORESIGHT_CLAIM_INVALID (as it cannot cover other invalid values). Or we can defer the extension as needed later.
The invalid value is used to as a value comparison to check for a race condition with both protocol bits set.
Thanks
Mike
Thanks, Leo
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).
[...]
+/*
- 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 ?
Thanks, Leo
Hi Leo
On Wed, 22 Oct 2025 at 15:04, Leo Yan leo.yan@arm.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
Hi Leo
On Thu, 23 Oct 2025 at 11:18, Mike Leach mike.leach@linaro.org wrote:
Hi Leo
On Wed, 22 Oct 2025 at 15:04, Leo Yan leo.yan@arm.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
-
Leo Yan -
Mike Leach