Messages in this thread Patch in this message | | | Subject | Re: [RFC PATCH v2] coresight: etm4x: Modify core-commit of cpu to avoid the overflow of HiSilicon ETM | From | Qi Liu <> | Date | Tue, 1 Sep 2020 09:57:33 +0800 |
| |
Hi Mathieu,
Thanks for your review.
On 2020/9/1 6:13, Mathieu Poirier wrote: > Following Al's comment I have the following recommendations... > > On Wed, Aug 19, 2020 at 04:06:37PM +0800, Qi Liu wrote: >> When too much trace information is generated on-chip, the ETM will >> overflow, and cause data loss. This is a common phenomenon on ETM >> devices. >> >> But sometimes we do not want to lose performance trace data, so we >> suppress the speed of instructions sent from CPU core to ETM to >> avoid the overflow of ETM. >> >> Signed-off-by: Qi Liu <liuqi115@huawei.com> >> --- >> >> Changes since v1: >> - ETM on HiSilicon Hip09 platform supports backpressure, so does >> not need to modify core commit. >> >> drivers/hwtracing/coresight/coresight-etm4x.c | 43 +++++++++++++++++++++++++++ >> 1 file changed, 43 insertions(+) >> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c >> index 7797a57..7641f89 100644 >> --- a/drivers/hwtracing/coresight/coresight-etm4x.c >> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c >> @@ -43,6 +43,10 @@ MODULE_PARM_DESC(boot_enable, "Enable tracing on boot"); >> #define PARAM_PM_SAVE_NEVER 1 /* never save any state */ >> #define PARAM_PM_SAVE_SELF_HOSTED 2 /* save self-hosted state only */ >> >> +#define CORE_COMMIT_CLEAR 0x3000 >> +#define CORE_COMMIT_SHIFT 12 >> +#define HISI_ETM_AMBA_ID_V1 0x000b6d01 > Do you have a name for the SoC that can be added so that other HiSilicon SoC can > be added? I suggest something like: > > #define HISI_NAME_CORE_COMMIT_CLEAR > #define HISI_NAME_CORE_COMMIT_SHIFT > #define HISI_NAME_ETM_ID Will fix this next version. > Moreover I don't see an entry for 0x000b6d01 in the amba id table - is your > devices upstream? Needless to day that is mandatory in order to move forward > with this work. A patch has been applied to add this ETM id and here is the link: https://lore.kernel.org/linux-arm-kernel/20200813210000.GO3393195@xps15/
>> + >> static int pm_save_enable = PARAM_PM_SAVE_FIRMWARE; >> module_param(pm_save_enable, int, 0444); >> MODULE_PARM_DESC(pm_save_enable, >> @@ -104,11 +108,40 @@ struct etm4_enable_arg { >> int rc; >> }; >> >> +static void etm4_cpu_actlr1_cfg(void *info) >> +{ >> + struct etm4_enable_arg *arg = (struct etm4_enable_arg *)info; >> + u64 val; >> + >> + asm volatile("mrs %0,s3_1_c15_c2_5" : "=r"(val)); >> + val &= ~CORE_COMMIT_CLEAR; >> + val |= arg->rc << CORE_COMMIT_SHIFT; >> + asm volatile("msr s3_1_c15_c2_5,%0" : : "r"(val)); >> +} >> + >> +static void etm4_config_core_commit(int cpu, int val) >> +{ >> + struct etm4_enable_arg arg = {0}; >> + >> + arg.rc = val; >> + smp_call_function_single(cpu, etm4_cpu_actlr1_cfg, &arg, 1); >> +} >> + >> static int etm4_enable_hw(struct etmv4_drvdata *drvdata) >> { >> int i, rc; >> + struct amba_device *adev; >> struct etmv4_config *config = &drvdata->config; >> struct device *etm_dev = &drvdata->csdev->dev; >> + struct device *dev = drvdata->csdev->dev.parent; >> + >> + adev = container_of(dev, struct amba_device, dev); >> + /* >> + * If ETM device is HiSilicon ETM device, reduce the >> + * core-commit to avoid ETM overflow. >> + */ >> + if (adev->periphid == HISI_ETM_AMBA_ID_V1) >> + etm4_config_core_commit(drvdata->cpu, 1); > I susggest to add a function like etm4_enable_arch_specific() and do the above > in there. The same goes for the disable path. > > Thanks, > Mathieu Thanks, I'll fix this next version : )
Qi > >> CS_UNLOCK(drvdata->base); >> >> @@ -472,10 +505,20 @@ static void etm4_disable_hw(void *info) >> { >> u32 control; >> struct etmv4_drvdata *drvdata = info; >> + struct device *dev = drvdata->csdev->dev.parent; >> struct etmv4_config *config = &drvdata->config; >> struct device *etm_dev = &drvdata->csdev->dev; >> + struct amba_device *adev; >> int i; >> >> + adev = container_of(dev, struct amba_device, dev); >> + /* >> + * If ETM device is HiSilicon ETM device, resume the >> + * core-commit after ETM trace is complete. >> + */ >> + if (adev->periphid == HISI_ETM_AMBA_ID_V1) >> + etm4_config_core_commit(drvdata->cpu, 0); >> + >> CS_UNLOCK(drvdata->base); >> >> if (!drvdata->skip_power_up) { >> -- >> 2.8.1 >> > . >
| |