linux-kernel - Re: [PATCH V3 12/18] coresight: tmc: dump system memory content only when needed

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]

Date:	Mon, 25 Apr 2016 08:38:29 -0600
From:	Mathieu Poirier <mathieu.poirier@...aro.org>
To:	Suzuki K Poulose <Suzuki.Poulose@....com>
Cc:	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V3 12/18] coresight: tmc: dump system memory content only
 when needed

On 25 April 2016 at 05:16, Suzuki K Poulose <Suzuki.Poulose@....com> wrote:
> On 22/04/16 18:14, Mathieu Poirier wrote:
>>
>> Calling tmc_etf/etr_dump_hw() is required only when operating from
>> sysFS.  When working from Perf, the system memory is harvested
>> from the AUX trace API.
>>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier@...aro.org>
>> ---
>>   drivers/hwtracing/coresight/coresight-tmc-etf.c | 7 ++++++-
>>   drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 ++++++-
>>   2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> index d090a9745c73..25fad93b68d4 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> @@ -74,7 +74,12 @@ static void tmc_etb_disable_hw(struct tmc_drvdata
>> *drvdata)
>>         CS_UNLOCK(drvdata->base);
>>
>>         tmc_flush_and_stop(drvdata);
>> -       tmc_etb_dump_hw(drvdata);
>> +       /*
>> +        * When operating in sysFS mode the content of the buffer needs to
>> be
>> +        * read before the TMC is disabled.
>> +        */
>> +       if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
>> +               tmc_etb_dump_hw(drvdata);
>>         tmc_disable_hw(drvdata);
>>
>>         CS_LOCK(drvdata->base);
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> index 8bbbf3ab1387..4b000f4003a2 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -76,7 +76,12 @@ static void tmc_etr_disable_hw(struct tmc_drvdata
>> *drvdata)
>>         CS_UNLOCK(drvdata->base);
>>
>>         tmc_flush_and_stop(drvdata);
>> -       tmc_etr_dump_hw(drvdata);
>> +       /*
>> +        * When operating in sysFS mode the content of the buffer needs to
>> be
>> +        * read before the TMC is disabled.
>> +        */
>> +       if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
>> +               tmc_etr_dump_hw(drvdata);
>>         tmc_disable_hw(drvdata);
>
>
> Nit: It would be cleaner if tmc_etX_disable_hw accepted a bool parameter to
> decide
> whether to dump the hw data or not. That way, the callers can make the
> decision,
> leaving the disable_hw code unaware of the mode checks.

My goal in pushing the functionality to tmc_etX_disable_hw() was to
especially keep the the higher layers unaware of sink buffer
management specifics.  There is two ways to look at this and we simply
landed on different sides of the fence.  I'm not strongly opinionated
on this, I'll modify if you're really keen on this.

Thanks,
Mathieu

>
> Suzuki