On Mon, Jan 11, 2021 at 12:09:12PM +0000, Mike Leach wrote:
Hi Leo,
I think there is an issue here in that your modification assumes that all cpus in the system are of the same ETM type. The original routine allowed for differing ETM types, thus differing cpu ETM field lengths between ETMv4 / ETMv3, the field size was used after the relevant magic number for the cpu ETM was read.
You have replaced two different sizes - with a single calculated size.
I usually go through an entire patchset before looking at the comments people have made. In this case Mike and I are coming to the exact same conclusion.
I will look at Mike's patch on Monday.
Moving forwards we are seeing the newer FEAT_ETE protocol drivers appearing on the list, which will ultimately need a new metadata structure.
We have had discussions within ARM regarding the changing of the format to be more self describing - which should probably be opened out to the CS mailing list.
Regards
Mike
On Mon, 11 Jan 2021 at 07:29, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 1/9/21 7:44 AM, Leo Yan wrote:
The metadata array can be extended over time and the tool, if using the predefined macro (like CS_ETMV4_PRIV_MAX for ETMv4) as metadata array size to copy data, it can cause compatible issue within different versions of perf tool.
E.g. we recorded a data file with an old version tool, afterwards if use the new version perf tool to parse the file, since the metadata array has been extended and the macro CS_ETMV4_PRIV_MAX has been altered, if use it to parse the perf data with old format, this will lead to mismatch.
To maintain backward compatibility, this patch calculates per CPU metadata array size on the runtime, the calculation is based on the info stored in the data file so that it's reliable.
Signed-off-by: Leo Yan leo.yan@linaro.org
Looks good to me.
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK