Philippe,
Forgot to mention I to will be on vacation
the week after you and will return on the 14th. I am only in three
days next week but will try to put together a proposal for a common userspace STM
API.
Regards,
Doug
From: Deao, Douglas
Sent: Thursday, May 26, 2011 4:18
PM
To: 'Philippe Langlais'; Linus
Walleij; Lee Jones
Cc: Arvind Chauhan; Michael Hope;
linaro-dev@lists.linaro.org; pierre.peiffer@stericsson.com;
loic.pallardy@stericsson.com
Subject: RE: ST-E STM Driver
Review
Philippe,
So from your comments it seems to me there is nothing
controversial. Do you have spec for the trace framework you have coded and sent
in the patches?
Regards,
Doug
From: Philippe
Langlais [mailto:philippe.langlais@linaro.org]
Sent: Thursday, May 26, 2011 10:20
AM
To: Deao, Douglas; Linus Walleij;
Lee Jones
Cc: Arvind Chauhan; Michael Hope;
linaro-dev@lists.linaro.org; pierre.peiffer@stericsson.com;
loic.pallardy@stericsson.com
Subject: Re: ST-E STM Driver
Review
Hi Doug,
I initiate the work to build a hardware trace framework in the kernel, I'm not
started the study to
have a common userspace API for STM, thanks to this email we can start such a
work, but
it may be long (next week I'm in vacation till June 7th).
See my detailed response for all your interrogations and my thoughts about STE
STM implementation below:
On 25 May 2011 23:54, Deao,
Sorry it took a while to get back to you guys. I was visiting customers
last week. Most of my comments are just highlighting the differences between
TI's STM 1.0 driver and ST-E's STM 1.0 driver, but there are a few questions,
observations and suggestions. At the end I included some discussion on TI's
meta data and OST header requirements.
I have not had a chance to look at your actual implementation yet. Did you do
anything to abstract the actual HW transport ports and control registers from
the higher level driver functions?
Yes, partially I think through IOCTLs & debugfs (see our stm.h
userspace API)
I realize there is a lot here to work through so if you would rather schedule a conference call to talk through the differences I can do that. I would like to start work on a Linaro (Unified) STM Spec next week if I can get feedback from everybody over the next few days. I will be out of the office on 5/27 and 5/31.
I hope this email is enough.
I am especially interested in details of what you guys have in mind for a "common trace framework to receive STM drivers". If by framework you mean well defined APIs that are implemented for specific devices, then I think we are in agreement. What Michael and I have talked about is a common STM user mode experience across all Linaro supported devices, making Linux user mode code 100% portable between our devices.
For my point of view, the trace device framework must ease the
integration of new hardware trace drivers in the kernel
(not only STM MIPI) to present standard hooks in current trace infrastructure,
but it can cover a common STM userspace
API too.
ST-E STM Driver stm-trace.txt review:
1. Software Overview
In your "Software Overview" it states:
"The end of data packet is marked by a time stamp on latest byte(s) only."
I assume that user messages can be made up of any number of bytes, half-words, words or longs (what ever is most efficient) and you simply terminate the last element of the message with a time-stamp - right?
Yes, the message buffer can be mis aligned
In the TI STM implementation a message can be any number and combination of bytes, half-words, or word transfers terminated with a time-stamp on the last element. In addition to that we also add an OST header to a message. (See below for discussion on OST header).
In our case the OST header is added by the external capture
probe.
2. Lossless/Lossy modes.
TI only supports lossless mode for sw generated messages and is enforced in our hw implementation. Lossy mode is reserved for true hw messages.
For STE, hw messages are always lossy, but sw generated messages could
be configured in lossless (default) or lossy mode.
I did not notice that you documented a way to modify this through the debugfs API or IOCTLS.
I have 2 IOCTLs (STM_SET_MODE & STM_GET_MODE) and debugfs
(masters_modes) interfaces for that.
I am kind of thinking that may be ok since this is really a hw configuration choice in your case, but in the TI case the user does not get to make that choice.
OK
3. Channel Assignment
TI makes the assignment with mknod using the minor number to assign a fixed channel. This allows the user mode application to overload the channel usage for categorizing data (not my idea). I think we see the error of our ways here and will be ok with a dynamic channel allocation.
If we are agree with dynamic channel allocation then a common STM
userspace API is possible I think and perhaps common STM driver.
I am thinking that for each unique pid a channel should be assigned when the device is opened. I would guess you are keeping a channel table around and write() just checks the table for a pid assignment (no time to look at your implementation yet), if none is found the first free channel is used. If you moved this function back to open then you could do the IOCTL STM_GET_CHANNEL_NO anytime, not just after the first write.
The reason behind this behavior is for our current STM user lib which
open "/dev/stm" and alloc/free channels with
IOCTL and never use write operation (only mmap + direct write) and in this case
we don't want to loose a channel.
I think we can change this behavior. We can support multiple channels allocated
for one Process to avoid contention
in multi-threaded process.
In write how do you flag an error if you exhaust the number of available channels?
I return -ENOMEM.
4. Kernel API
TI does not support a Kernel API (yet). I can see that the Alloc/Free and File IO type functions are useful and should be standard.
Not sure what you mean by "lockless" trace functions?
Not protected against concurrent access, it's the responsibility
to the caller.
It looks like your "low level atomic trace functions for 1, 2, 4 or 8 bytes" is similar to TI's binary library functions (not supported by the TI STM Driver). This is what we use the OST header for, allowing our tool chain to differentiate between different message formats, rather than just assuming the data is a simple stream of bytes.
I'm not ready to add an OST header to each trace messages to
particularly for kernel function traces (ftrace has short messages 8 bytes size
with high throughput), I rather to dedicate a specific channel for this, OST
header has to be optional.
5. Debugfs APIs.
TI used a different approach. The tool-chain on the host provides all the transport setup through JTAG, so our driver does not support setting up the actual STM data export (number of pins and clock rate). In our case device transport parameters must match the host receiver's collection setup.
With your approach the user can change the clock rate and export pin width effectively at any time. Our tools actually go through a calibration process during initialization so any changes to the device's transport setup (clock rate, number of pins data exported on) would cause the TI tool chain a lot of grief.
There are some parameters we know we need to add (like master enables). This are currently also handled by the host tools. TI's STM module allows up to 4 SW masters to be enabled (with id masks that can be used to enable multiple masters from the same group) and 4 HW masters that can be enabled at the same time as the SW masters. If the user tries to enable more than the HW allows do you have a mechanism to flag an error?
We can't face the same problem with STE STM, our hardware support 6 SW
masters & 2 HW masters without restriction, all could be enabled at same
time (managed by a bit field register)
I don't have a lot of experience with debugfs but I am assuming it's primarily used for allowing scripts to configure a driver (like in your example) or extract information.
It's the current way to configure kernel tracing infrastructure (from
console, through scripts or user apps), kernel users are familiar with this
interface.
We may want to define a standard set of debugfs options whose implementation is vendor specific. But that raises some questions:
- How do we deal with options that don't make sense for a specific vendor?
Maybe just doing nothing is acceptable or do we want to provide a discovery mechanism?
Do nothing may be sufficient, we can add STM IP specific string
descriptor too, something like "vendor id+STM IP id+STP version"
- Would user scripts then also be vendor specific?
We should probably make an effort to avoid this. A discovery mechanism may allow user mode scripts to be generic.
Yes, but STM configuration scripts are device specific, to see.
6. Mapped Channels
I believe the TI hw transport channel mapping is compatible. In the TI case a channel is mapped into two spaces, the first half is for non-timestamp transfers and the second half is for time-stamped transfers. When we write a message (from a user mode write call for example) we simply write all the data except the last element through the non-timestamp port, and then the last element is written to the time-stamped port. So I think we could be compatible here.
I think so, we have similar channel memory mapping but, one STM channel
is accessed through this C union of 2 consecutive 64bits word (first for
non-timestamp, 2nd with timestamp)
/* One single channel mapping */
struct stm_channel {
union {
__u8 no_stamp8;
__u16 no_stamp16;
__u32 no_stamp32;
__u64 no_stamp64;
};
union {
__u8 stamp8;
__u16 stamp16;
__u32 stamp32;
__u64 stamp64;
};
};
With that said I am not sure about exposing all channels to a user mode library. You are relying on the library to use the convention of getting a free channel from the driver to make sure there are no conflicts. If the channel assignment is made when you open the device, you could conceivably map just the address space needed for the single channel, thus eliminating the need to get a free channel from the driver. In the TI case a single channel's transport mapping is 4K bytes, which matches the typical PAGE_SIZE. I realize not all hw implementations will match up with the PAGE_SIZE, which may be why you simply map all the channels back to user space.
It's our case, the 256 channels is mapped in the same 4K memory page.
Since free channels can become busy rapidly, maybe a better convention would be to simply use another device node if the user wants the library STM data to be transmitted on a different STM channel than the current process. This may be a case where providing a mechanism (see meta data discussion below) to allow channels to be named for the toolchain may be a good idea (provide task name and process id).
In our current implementation, a new open on the same
"/dev/stm" device allocate a new channel at first write.
7. 8-byte Writes
TI does not support 64-bit writes with our STM 1.0 module. We may need an IOCTL to get the largest transfer supported for the mmap case. For all other cases this should just be hidden in the device dependent code.
I think so
8. Kernel Internal Usage
I like the idea of having dedicated support in the driver for common kernel logging. Any ideas on how you would support kernel STM channel assignments without hard-coding?
But configurable through kernel configuration, the other way is to
dedicate a specific channel for this signalization like your channel 255, not
so easy to define.
We may need a mechanism to communicate the definition of each hard-coded channel to our tools.
to see
The following are TI specific:
9. Data protection
In SMP systems if the processor is switched a new master is generated (in some TI devices). So we protect the data with a mutex to guarantee a complete message is generated by the same master.
OK, it's not our case.