| Message ID | 1454487337-30184-4-git-send-email-zhang.chunyan@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
Hi, I think a quick clarification of the ARM hardware STM architecture may be of value here. The ARM hardware STM, when implemented as recommend in a hardware design, the master IDs are not under driver control, but have a hardwire association with source devices in the system. The master IDs are hardwired to individual cores and core security states, and there could be other IDs associated with hardware trace sources requiring output via the hardware STM. (an example of this is the ARM Juno development board). Therefore in multi-core systems many masters may be associated with a single software STM stream. A user-space application running on Core 1, may have a master ID of 0x11 in the STPv2 trace stream, but if this application is context switched and later continues running on Core 2, then master ID could change to 0x21. Masters identify a hardware source in this scheme, the precise values used will be implementation dependent. So the number of masters "available to the software" depends on your interpretation of the phrase. If you mean "master IDs on which software trace will appear" then that will be many - it depends on which core is running the application. However as described above, this is not predictable by any decoder, and the master set used may not be contiguous. If you mean "master IDs selectable/allocated by the driver" then that will be 0 - the driver has no control, and possibly no knowledge of which master is associated with the current execution context. (this is of course system dependent - it may well be possible to have some configuration database associating cores with hardware IDs, but this will be implementation and board support dependant). Therefore, there is a requirement to tell both the user-space STM client and decoder that not only is master ID management not needed - it is in fact not possible and the key to identify the software source for a given STM packet is the channel alone. In addition we have a nominal single Master ID "available to the software", to satisfy the requirements of the generic ST module API. So on Juno for example, while we do have 128 masters, each with 65536 channels, the master allocation is not visible to system software - each core sees a single master with 65536 channels. Therefore differentiation between software sources in the STM trace is by channel number allocations only. Best Regards Mike. ---------------------------------------------------------------- Mike Leach +44 (0)1254 893911 (Direct) Principal Engineer +44 (0)1254 893900 (Main) Arm Blackburn Design Centre +44 (0)1254 893901 (Fax) Belthorn House Walker Rd mailto:mike.leach@arm.com Guide Blackburn BB1 2QE ---------------------------------------------------------------- > -----Original Message----- > From: Alexander Shishkin [mailto:alexander.shishkin@linux.intel.com] > Sent: 05 February 2016 12:52 > To: Chunyan Zhang; mathieu.poirier@linaro.org > Cc: robh@kernel.org; broonie@kernel.org; pratikp@codeaurora.org; > nicolas.guion@st.com; corbet@lwn.net; Mark Rutland; Mike Leach; > tor@ti.com; Al Grant; zhang.lyra@gmail.com; linux-kernel@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-api@vger.kernel.org; linux- > doc@vger.kernel.org > Subject: Re: [PATCH V2 3/6] stm class: provision for statically assigned > masterIDs > > Chunyan Zhang <zhang.chunyan@linaro.org> writes: > > > From: Mathieu Poirier <mathieu.poirier@linaro.org> > > > > Some architecture like ARM assign masterIDs statically at the HW design > > phase, making masterID manipulation in the generic STM core irrelevant. > > > > This patch adds a new 'mstatic' flag to struct stm_data that tells the > > core that this specific STM device doesn't need explicit masterID > > management. > > So why do we need this patch? If your STM only has master 42 allocated > for software sources, simply set sw_start = 42, sw_end = 42 and you're > good to go, software will have exactly one channel to choose from. See > also the comment from <linux/stm.h>: > > * @sw_start: first STP master available to software > * @sw_end: last STP master available to software > > particularly the "available to software" part. Any other kinds of > masters the STM class code doesn't care about (yet). > > > In the core sw_start/end of masterID are set to '1', > > i.e there is only one masterID to deal with. > > This is also a completely arbitrary and unnecessary requirement. Again, > you can set both to 42 and it will still work. > > > Also this patch depends on [1], so that the number of masterID > > is '1' too. > > > > Finally the lower and upper bound for masterIDs as presented > > in ($SYSFS)/class/stm/XYZ.stm/masters and > > ($SYSFS)/../stp-policy/XYZ.stm.my_policy/some_device/masters > > are set to '-1'. That way users can't confuse them with > > architecture where masterID management is required (where any > > other value would be valid). > > Why is this a good idea? Having the actual master there will allow > software to know what it is and also tell the decoding side what it is > (assuming you have more than one source in the STP stream, it might not > be easy to figure out, unless you know it in advance). > > I don't see how one master statically assigned to software sources is > different from two masters or 32 masters. And I don't see any benefit of > hiding the master id from userspace. Am I missing something? > > Regards, > -- > Alex IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On 5 February 2016 at 05:52, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Chunyan Zhang <zhang.chunyan@linaro.org> writes: > >> From: Mathieu Poirier <mathieu.poirier@linaro.org> >> >> Some architecture like ARM assign masterIDs statically at the HW design >> phase, making masterID manipulation in the generic STM core irrelevant. >> >> This patch adds a new 'mstatic' flag to struct stm_data that tells the >> core that this specific STM device doesn't need explicit masterID >> management. > > So why do we need this patch? If your STM only has master 42 allocated > for software sources, simply set sw_start = 42, sw_end = 42 and you're > good to go, software will have exactly one channel to choose from. See > also the comment from <linux/stm.h>: On ARM each source, i.e entity capable of accessing STM channels, has a different master ID set in HW. We can't assume the IDs are contiguous and from a SW point of view there is no way to probe the values. > > * @sw_start: first STP master available to software > * @sw_end: last STP master available to software > > particularly the "available to software" part. Any other kinds of > masters the STM class code doesn't care about (yet). > >> In the core sw_start/end of masterID are set to '1', >> i.e there is only one masterID to deal with. > > This is also a completely arbitrary and unnecessary requirement. Again, > you can set both to 42 and it will still work. True - any value will do. The important thing to remember is that on ARM, there is only one masterID channel (from an STM core point of view). But we could also proceed differently, see below for more details. > >> Also this patch depends on [1], so that the number of masterID >> is '1' too. >> >> Finally the lower and upper bound for masterIDs as presented >> in ($SYSFS)/class/stm/XYZ.stm/masters and >> ($SYSFS)/../stp-policy/XYZ.stm.my_policy/some_device/masters >> are set to '-1'. That way users can't confuse them with >> architecture where masterID management is required (where any >> other value would be valid). > > Why is this a good idea? Having the actual master there will allow > software to know what it is and also tell the decoding side what it is > (assuming you have more than one source in the STP stream, it might not > be easy to figure out, unless you know it in advance). In the ARM world masterIDs are irrelevant so why bother with them? Writing a '-1' simply highlight this reality. Another way of approaching the problem would be to change sw_start/end to a bitfield that represent the valid masterIDs but in my opinion, it is a lot of code churn for information that isn't used outside of the decoder. Thanks, Mathieu > > I don't see how one master statically assigned to software sources is > different from two masters or 32 masters. And I don't see any benefit of > hiding the master id from userspace. Am I missing something? > > Regards, > -- > Alex
On 8 February 2016 at 06:26, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Mathieu Poirier <mathieu.poirier@linaro.org> writes: > >> On 5 February 2016 at 05:52, Alexander Shishkin >> <alexander.shishkin@linux.intel.com> wrote: >>> Chunyan Zhang <zhang.chunyan@linaro.org> writes: >>> >>>> From: Mathieu Poirier <mathieu.poirier@linaro.org> >>>> >>>> Some architecture like ARM assign masterIDs statically at the HW design >>>> phase, making masterID manipulation in the generic STM core irrelevant. >>>> >>>> This patch adds a new 'mstatic' flag to struct stm_data that tells the >>>> core that this specific STM device doesn't need explicit masterID >>>> management. >>> >>> So why do we need this patch? If your STM only has master 42 allocated >>> for software sources, simply set sw_start = 42, sw_end = 42 and you're >>> good to go, software will have exactly one channel to choose from. See >>> also the comment from <linux/stm.h>: >> >> On ARM each source, i.e entity capable of accessing STM channels, has >> a different master ID set in HW. We can't assume the IDs are >> contiguous and from a SW point of view there is no way to probe the >> values. > > Ok, it's the 'static' word that got me confused. From Mike's explanation > it seems to me that it's the antithesis of static; the master ID > assignment is so dynamic that it's not controllable by the software and > may or may not reflect core id, power state, phase of the moon, etc. Mike did write "master IDs are hardwired to individual cores and core security states", which make assignment for one platform very static. On the flip side those will change from one system to another. > >>>> In the core sw_start/end of masterID are set to '1', >>>> i.e there is only one masterID to deal with. >>> >>> This is also a completely arbitrary and unnecessary requirement. Again, >>> you can set both to 42 and it will still work. >> >> True - any value will do. The important thing to remember is that on >> ARM, there is only one masterID channel (from an STM core point of >> view). But we could also proceed differently, see below for more >> details. > > Well, we have the masters attribute with two numbers in it that define > the range of master IDs that the software can choose from. More > specifically to this situation: > > * the number of channel ID spaces available to the SW is > $end - $start + 1, that is, in your case, just 1; > * the number of master IDs for the SW to choose from is $end - $start; > * if $end==$start, their actual numeric value doesn't really matter, > either for the policy definition or for the actual writers. > > This $end==$start situation itself may be ambiguous and can be > interpreted either as having just one *static* master ID fixed for all > SW writers (what I assumed from your commit message) or as having a > floating master ID, which changes of its own accord and is not > controllable by software. Some clarification here. On ARM from a SW point of view $end == $start and that doesn't change (with regards to masterIDs) . The ambiguity comes from the fact that on other platforms where masterID configuration does change and is important, the condition $end == $start could also be valid. From configFS, how do we tell users when masterIDs are set in HW? That's what I wanted to highlight with the '-1'. Regardless of the solution we choose I think it is important that we inform, at the very least, users when masterIDs don't matter. We could also leave thing as is, kill this patch and document things thoroughly. With the former things are obvious. > > These two situations are really the same thing from the perspective of > the system under tracing. Also, both of these situations should already > work if the driver sets both sw_start and sw_end to the same > value. > > Regards, > -- > Alex
> Mike did write "master IDs are hardwired to individual cores and core security > states", which make assignment for one platform very static. > On the flip side those will change from one system to another. It depends on your perspective. From the perspective of a userspace process not pinned to a core, the master id will appear to vary dynamically and unpredictably as the thread migrates from one core to another. (That's actually useful if the decoder wants to know where the thread is running at any given point, as it can find that out for free, without the need to track migration events.) On the other hand if you are pinned (e.g. you're the kernel on a particular core, or you're a per-core worker thread in some thread pooling system) then you have a fixed master id, and then you can have one instance per core all using the same range of channel numbers, with the master id indicating the core - this saves on channel space compared to having to give each core its own range of channel space. Al IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On 8 February 2016 at 10:44, Al Grant <Al.Grant@arm.com> wrote: >> Mike did write "master IDs are hardwired to individual cores and core security >> states", which make assignment for one platform very static. >> On the flip side those will change from one system to another. > > It depends on your perspective. From the perspective of a userspace > process not pinned to a core, the master id will appear to vary > dynamically and unpredictably as the thread migrates from one > core to another. (That's actually useful if the decoder wants to know > where the thread is running at any given point, as it can find that out > for free, without the need to track migration events.) Right, that's the expected (and desired) behaviour. > > On the other hand if you are pinned (e.g. you're the kernel on a > particular core, or you're a per-core worker thread in some thread > pooling system) then you have a fixed master id, and then you can > have one instance per core all using the same range of channel > numbers, with the master id indicating the core - this saves on > channel space compared to having to give each core its own > range of channel space. From an STM core and driver point of view channel mapping works the same way - pinning of tasks is a kernel artefact. The problem of representing masterIDs in the STM core for an architecture with HW assigned masterIDs is still unresolved. > > Al > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On 12 February 2016 at 09:27, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Mathieu Poirier <mathieu.poirier@linaro.org> writes: > >> On 8 February 2016 at 06:26, Alexander Shishkin >> <alexander.shishkin@linux.intel.com> wrote: >>> This $end==$start situation itself may be ambiguous and can be >>> interpreted either as having just one *static* master ID fixed for all >>> SW writers (what I assumed from your commit message) or as having a >>> floating master ID, which changes of its own accord and is not >>> controllable by software. >> >> Some clarification here. >> >> On ARM from a SW point of view $end == $start and that doesn't change >> (with regards to masterIDs) . The ambiguity comes from the fact that >> on other platforms where masterID configuration does change and is >> important, the condition $end == $start could also be valid. > > Yes, that's what I was saying. The thing is, on the system-under-tracing > side these two situations are not very different from one > another. Master IDs are really just numbers without any semantics > attached to them in the sense that they are not covered by the mipi spec > or any other standard (to my knowledge). We are definitely on the same page here, just using slightly different terms. > > The difference is in the way we map channels to masters. One way is to > allocate a distinct set of channels for each master (the way Intel Trace > Hub does it); another way is to share the same set of channels between > multiple masters. We are in total agreement. > So we can describe this as "hardware implements the > same set of channels across multiple masters" or something along those > lines. I suggest "Shared channels"? In the end, that's really what it is... The outstanding issue is still how to represent these to different way of mapping things in the STM core. I suggested a flag, called "mstatic" (but that can be changed), and a representation of '-1' in for masterIDs sysFS. Whether we stick with that or not is irrelevant, I'd be fine with another mechanism. What I am keen on is that from sysFS users can quickly tell which heuristic is enacted on that specific architecture. > > Actually, in the latter scheme of things you can also have multiple > masters, at least theoretically. Say, you have masters [0..15], each > with distinct set of channels, but depending on hardware state these > masters actually end up as $state*16+$masterID in the STP stream. > > So we might also think about differentiating between the hardware > masters (0 though 15 in the above example) and STP masters. I'm not sure I get what you mean here. On ARM the masterIDs assigned in HW, which will depend on the state, will show up in the STP stream. But again, I might be missing your point. Thanks, Mathieu > > Regards, > -- > Alex
On 12 February 2016 at 13:33, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > On 12 February 2016 at 09:27, Alexander Shishkin > <alexander.shishkin@linux.intel.com> wrote: >> Mathieu Poirier <mathieu.poirier@linaro.org> writes: >> >>> On 8 February 2016 at 06:26, Alexander Shishkin >>> <alexander.shishkin@linux.intel.com> wrote: >>>> This $end==$start situation itself may be ambiguous and can be >>>> interpreted either as having just one *static* master ID fixed for all >>>> SW writers (what I assumed from your commit message) or as having a >>>> floating master ID, which changes of its own accord and is not >>>> controllable by software. >>> >>> Some clarification here. >>> >>> On ARM from a SW point of view $end == $start and that doesn't change >>> (with regards to masterIDs) . The ambiguity comes from the fact that >>> on other platforms where masterID configuration does change and is >>> important, the condition $end == $start could also be valid. >> >> Yes, that's what I was saying. The thing is, on the system-under-tracing >> side these two situations are not very different from one >> another. Master IDs are really just numbers without any semantics >> attached to them in the sense that they are not covered by the mipi spec >> or any other standard (to my knowledge). > > We are definitely on the same page here, just using slightly different terms. > >> >> The difference is in the way we map channels to masters. One way is to >> allocate a distinct set of channels for each master (the way Intel Trace >> Hub does it); another way is to share the same set of channels between >> multiple masters. > > We are in total agreement. > >> So we can describe this as "hardware implements the >> same set of channels across multiple masters" or something along those >> lines. > > I suggest "Shared channels"? In the end, that's really what it is... > > The outstanding issue is still how to represent these to different way > of mapping things in the STM core. I suggested a flag, called > "mstatic" (but that can be changed), and a representation of '-1' in > for masterIDs sysFS. Whether we stick with that or not is irrelevant, > I'd be fine with another mechanism. What I am keen on is that from > sysFS users can quickly tell which heuristic is enacted on that > specific architecture. Alex, How do you want to proceed with the above? Do you agree with my current proposal or can you think of a better way? Thanks, Mathieu > >> >> Actually, in the latter scheme of things you can also have multiple >> masters, at least theoretically. Say, you have masters [0..15], each >> with distinct set of channels, but depending on hardware state these >> masters actually end up as $state*16+$masterID in the STP stream. >> >> So we might also think about differentiating between the hardware >> masters (0 though 15 in the above example) and STP masters. > > I'm not sure I get what you mean here. On ARM the masterIDs assigned > in HW, which will depend on the state, will show up in the STP stream. > But again, I might be missing your point. > > Thanks, > Mathieu > >> >> Regards, >> -- >> Alex
diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c index 86bb4e3..cd3dc19 100644 --- a/drivers/hwtracing/stm/core.c +++ b/drivers/hwtracing/stm/core.c @@ -44,9 +44,15 @@ static ssize_t masters_show(struct device *dev, char *buf) { struct stm_device *stm = to_stm_device(dev); - int ret; + int ret, sw_start, sw_end; + + sw_start = stm->data->sw_start; + sw_end = stm->data->sw_end; - ret = sprintf(buf, "%u %u\n", stm->data->sw_start, stm->data->sw_end); + if (stm->data->mstatic) + sw_start = sw_end = STM_STATIC_MASTERID; + + ret = sprintf(buf, "%d %d\n", sw_start, sw_end); return ret; } @@ -629,7 +635,15 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data, if (!stm_data->packet || !stm_data->sw_nchannels) return -EINVAL; + /* + * MasterIDs are statically set in HW. As such the core is + * using a single master for interaction with this device. + */ + if (stm_data->mstatic) + stm_data->sw_start = stm_data->sw_end = 1; + nmasters = stm_data->sw_end - stm_data->sw_start; + stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL); if (!stm) return -ENOMEM; diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c index 691686e..7e70ca2 100644 --- a/drivers/hwtracing/stm/policy.c +++ b/drivers/hwtracing/stm/policy.c @@ -80,10 +80,17 @@ static ssize_t stp_policy_node_masters_show(struct config_item *item, char *page) { struct stp_policy_node *policy_node = to_stp_policy_node(item); + struct stm_device *stm = policy_node->policy->stm; + int first_master, last_master; ssize_t count; - count = sprintf(page, "%u %u\n", policy_node->first_master, - policy_node->last_master); + first_master = policy_node->first_master; + last_master = policy_node->last_master; + + if (stm && stm->data->mstatic) + first_master = last_master = STM_STATIC_MASTERID; + + count = sprintf(page, "%d %d\n", first_master, last_master); return count; } @@ -106,6 +113,13 @@ stp_policy_node_masters_store(struct config_item *item, const char *page, if (!stm) goto unlock; + /* + * masterIDs are statically allocated in HW, no point in trying to + * change their values. + */ + if (stm->data->mstatic) + goto unlock; + /* must be within [sw_start..sw_end], which is an inclusive range */ if (first > INT_MAX || last > INT_MAX || first > last || first < stm->data->sw_start || @@ -325,6 +339,7 @@ stp_policies_make(struct config_group *group, const char *name) * number of dot(s) are found in the <device_name>, the * first matched STM device name would be extracted. */ + for (p = devname; ; p++) { p = strchr(p, '.'); if (!p) { diff --git a/include/linux/stm.h b/include/linux/stm.h index f351d62..c9712a7 100644 --- a/include/linux/stm.h +++ b/include/linux/stm.h @@ -18,6 +18,11 @@ #include <linux/device.h> /** + * The masterIDs are statically set in hardware and can't be queried + */ +#define STM_STATIC_MASTERID -1 + +/** * enum stp_packet_type - STP packets that an STM driver sends */ enum stp_packet_type { @@ -46,6 +51,8 @@ struct stm_device; * struct stm_data - STM device description and callbacks * @name: device name * @stm: internal structure, only used by stm class code + * @mstatic: true if masterIDs are assigned in HW. If so @sw_start + * and @sw_end are set to '1' by the core. * @sw_start: first STP master available to software * @sw_end: last STP master available to software * @sw_nchannels: number of STP channels per master @@ -71,6 +78,7 @@ struct stm_device; struct stm_data { const char *name; struct stm_device *stm; + bool mstatic; unsigned int sw_start; unsigned int sw_end; unsigned int sw_nchannels;