Re: [PATCH 3/3] rfc: perf: cs_etm: Detect pid in VMID for kernel running at EL2

Suzuki K Poulose
2020-11-13T09:49:10Z

Hi Leo

On 11/13/20 12:11 AM, Leo Yan wrote:
> On Wed, Nov 11, 2020 at 11:40:03AM +0000, Suzuki Kuruppassery Poulose wrote:
>> On 11/11/20 11:03 AM, Al Grant wrote:
>
> [...]
>
>>> But if you must do it heuristically at decode time based on what
>>> you see in packets, then it would be safer to say that if you see both
>>> VMID and CONTEXIDR, the TID will be in VMID.
>>
>> This may not be entirely correct, unless we correlate the VMID_OPT. Because,
>> when we add Virtualization support (vmid == actual vmid).
>>
>> So we should do:
>> if (VMID && VMIDOPT) {
>> tid = pid;
>> } else if (CID) {
>>
>> }
>
> Rather than heuristic method, I think we can use metadata to store
> "pid" entry in the function cs_etm_get_metadata(), so that in the
> record phase, the "pid" is stored into perf data file.

True, that is a good idea. That makes it future proof.

>
> When decode the trace data, we can retrieve "pid" value from the
> metadata, and can base on the "pid" value to make decision for using
> context_id or VMID.

>
> The metadata can be accessed by referring
> cs_etm_queue::cs_etm_auxtrace::metadata; but the file
> util/cs-etm-decoder/cs-etm-decoder.c cannot directly access the
> metadata structure due to "cs_etm_queue" and "cs_etm_auxtrace" both
> are defined in util/cs-etm.c. It's better to introduce a helper
> function in util/cs-etm.c to retrieve "pid" value, like:
>
> u64 cs_etm__etmq_get_pid_type(struct cs_etm_queue *etmq);

I am not an expert in that side, how it interacts with the OpenCSD.
thus left this patch as an RFC.

I appreciate your feedback and thoughts. If you feel like you could
cleanup and implement this, please feel free to do so. Otherwise, I
might try it out whenever I have some spare cycles to understand
this side of the code (and this might be delayed).

Cheers
Suzuki

>
> Thanks,
> Leo