Messages in this thread | | | From | Mike Leach <> | Date | Wed, 19 Jan 2022 15:22:17 +0000 | Subject | Re: [PATCH 1/1] coresight: Fix TRCCONFIGR.QE sysfs interface |
| |
Hi James
On Fri, 14 Jan 2022 at 17:33, James Clark <james.clark@arm.com> wrote: > > It's impossible to program a valid value for TRCCONFIGR.QE > when TRCIDR0.QSUPP==0b10. In that case the following is true: > > Q element support is implemented, and only supports Q elements without > instruction counts. TRCCONFIGR.QE can only take the values 0b00 or 0b11. > > Currently the low bit of QSUPP is checked to see if the low bit of QE can > be written to, but as you can see when QSUPP==0b10 the low bit is cleared > making it impossible to ever write the only valid value of 0b11 to QE. > 0b10 would be written instead, which is a reserved QE value even for all > values of QSUPP. > > The fix is to allow writing the low bit of QE for any non zero value of > QSUPP. > > This change doesn't go any further to validate if the user supplied value > is valid, because none of the other parts this function do, but it does fix > the case where it was impossible to ever set a valid value. >
I concur that the input is not checked as valid, However all the other fields are single bit - with no invalid values - other than the cond field, which controls tracing of non-branch conditionals - architecturally unsupported for A profile in ETMv4/.
> Signed-off-by: James Clark <james.clark@arm.com> > --- > drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > index a0640fa5c55b..a99bb537ea23 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > @@ -368,7 +368,7 @@ static ssize_t mode_store(struct device *dev, > /* start by clearing QE bits */ > config->cfg &= ~(BIT(13) | BIT(14)); > /* if supported, Q elements with instruction counts are enabled */ > - if ((mode & BIT(0)) && (drvdata->q_support & BIT(0))) > + if ((mode & BIT(0)) && drvdata->q_support) > config->cfg |= BIT(13);
This can be trivially changed to
if ((mode) && drvdata->q_support) config->cfg |= BIT(13);
to ensure that any input mode 2'b01, 2'b10, 2'b11 results in output settings of 2'b01, 2'b11, 2'b11 respectively
With that
Reviewed-by: Mike Leach <mike.leach@linaro.org>
> /* > * if supported, Q elements with and without instruction > -- > 2.17.1 >
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
| |