* [PATCH 0/2] coresight: next v4.16-rc5 @ 2018-03-13 17:24 Mathieu Poirier 2018-03-13 17:24 ` [PATCH 1/2] coresight: Use %px to print pcsr instead of %p Mathieu Poirier ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Mathieu Poirier @ 2018-03-13 17:24 UTC (permalink / raw) To: linux-arm-kernel Hi Greg, These two patches came in last week and are confined to Coresight. Given the late(ish) time in the cycle I would have normally waited for the next one but the changes are small and trivial. Please consider - if you think it's too late they'll simply go in later. Thanks, Mathieu Bo Yan (1): coresight: etm4x: Fix bit shifting Leo Yan (1): coresight: Use %px to print pcsr instead of %p drivers/hwtracing/coresight/coresight-cpu-debug.c | 2 +- drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] coresight: Use %px to print pcsr instead of %p 2018-03-13 17:24 [PATCH 0/2] coresight: next v4.16-rc5 Mathieu Poirier @ 2018-03-13 17:24 ` Mathieu Poirier 2018-04-05 6:26 ` Kees Cook 2018-03-13 17:24 ` [PATCH 2/2] coresight: etm4x: Fix bit shifting Mathieu Poirier 2018-03-14 16:59 ` [PATCH 0/2] coresight: next v4.16-rc5 Greg KH 2 siblings, 1 reply; 10+ messages in thread From: Mathieu Poirier @ 2018-03-13 17:24 UTC (permalink / raw) To: linux-arm-kernel From: Leo Yan <leo.yan@linaro.org> Commit ad67b74d2469 ("printk: hash addresses printed with %p") lets printk specifier %p to hash all addresses before printing, this was resulting in the high 32 bits of pcsr can only output zeros. So module cannot completely print pc value and it's pointless for debugging purpose. This patch fixes this by using %px to print pcsr instead. Cc: Mathieu Poirier <mathieu.poirier@linaro.org> Signed-off-by: Leo Yan <leo.yan@linaro.org> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> --- drivers/hwtracing/coresight/coresight-cpu-debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c index 6ea62c62ff27..9cdb3fbc8c1f 100644 --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c @@ -315,7 +315,7 @@ static void debug_dump_regs(struct debug_drvdata *drvdata) } pc = debug_adjust_pc(drvdata); - dev_emerg(dev, " EDPCSR: [<%p>] %pS\n", (void *)pc, (void *)pc); + dev_emerg(dev, " EDPCSR: [<%px>] %pS\n", (void *)pc, (void *)pc); if (drvdata->edcidsr_present) dev_emerg(dev, " EDCIDSR: %08x\n", drvdata->edcidsr); -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/2] coresight: Use %px to print pcsr instead of %p 2018-03-13 17:24 ` [PATCH 1/2] coresight: Use %px to print pcsr instead of %p Mathieu Poirier @ 2018-04-05 6:26 ` Kees Cook 2018-04-06 15:47 ` Mathieu Poirier 0 siblings, 1 reply; 10+ messages in thread From: Kees Cook @ 2018-04-05 6:26 UTC (permalink / raw) To: linux-arm-kernel On Tue, Mar 13, 2018 at 10:24 AM, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > From: Leo Yan <leo.yan@linaro.org> > > Commit ad67b74d2469 ("printk: hash addresses printed with %p") lets > printk specifier %p to hash all addresses before printing, this was > resulting in the high 32 bits of pcsr can only output zeros. So > module cannot completely print pc value and it's pointless for debugging > purpose. > > This patch fixes this by using %px to print pcsr instead. Why is %p (and now %px) needed here at all? %pS is already used, which should give useful debugging details, yes? -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] coresight: Use %px to print pcsr instead of %p 2018-04-05 6:26 ` Kees Cook @ 2018-04-06 15:47 ` Mathieu Poirier 2018-04-09 16:10 ` Leo Yan 0 siblings, 1 reply; 10+ messages in thread From: Mathieu Poirier @ 2018-04-06 15:47 UTC (permalink / raw) To: linux-arm-kernel On 5 April 2018 at 00:26, Kees Cook <keescook@chromium.org> wrote: > On Tue, Mar 13, 2018 at 10:24 AM, Mathieu Poirier > <mathieu.poirier@linaro.org> wrote: >> From: Leo Yan <leo.yan@linaro.org> >> >> Commit ad67b74d2469 ("printk: hash addresses printed with %p") lets >> printk specifier %p to hash all addresses before printing, this was >> resulting in the high 32 bits of pcsr can only output zeros. So >> module cannot completely print pc value and it's pointless for debugging >> purpose. >> >> This patch fixes this by using %px to print pcsr instead. > > Why is %p (and now %px) needed here at all? %pS is already used, which > should give useful debugging details, yes? Hey Leo, I've taken a second look at this and Kees has a point, %pS should be sufficient here. Please test again and see if %px is absolutely necessary. If you think that is the case please provide a snapshot of the corner case that makes the change mandatory. > > -Kees > > -- > Kees Cook > Pixel Security ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] coresight: Use %px to print pcsr instead of %p 2018-04-06 15:47 ` Mathieu Poirier @ 2018-04-09 16:10 ` Leo Yan 2018-04-09 17:54 ` Kees Cook 0 siblings, 1 reply; 10+ messages in thread From: Leo Yan @ 2018-04-09 16:10 UTC (permalink / raw) To: linux-arm-kernel Hi Mathieu, Kees, On Fri, Apr 06, 2018 at 09:47:31AM -0600, Mathieu Poirier wrote: > On 5 April 2018 at 00:26, Kees Cook <keescook@chromium.org> wrote: > > On Tue, Mar 13, 2018 at 10:24 AM, Mathieu Poirier > > <mathieu.poirier@linaro.org> wrote: > >> From: Leo Yan <leo.yan@linaro.org> > >> > >> Commit ad67b74d2469 ("printk: hash addresses printed with %p") lets > >> printk specifier %p to hash all addresses before printing, this was > >> resulting in the high 32 bits of pcsr can only output zeros. So > >> module cannot completely print pc value and it's pointless for debugging > >> purpose. > >> > >> This patch fixes this by using %px to print pcsr instead. > > > > Why is %p (and now %px) needed here at all? %pS is already used, which > > should give useful debugging details, yes? > > Hey Leo, > > I've taken a second look at this and Kees has a point, %pS should be > sufficient here. Please test again and see if %px is absolutely > necessary. If you think that is the case please provide a snapshot of > the corner case that makes the change mandatory. Sorry I missed replying for this patch due to my email filter and thanks for suggestion. I can think out one case which should use %px: if CPU is locked up in firmwares (e.g. NS-EL2 or ARM-TF in S-EL3), we can rely on %px to read back PC hex value; for this case the CPU PC address is out of kernel space. How about you guys think for this? Thanks, Leo Yan ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] coresight: Use %px to print pcsr instead of %p 2018-04-09 16:10 ` Leo Yan @ 2018-04-09 17:54 ` Kees Cook 2018-04-09 23:44 ` Leo Yan 0 siblings, 1 reply; 10+ messages in thread From: Kees Cook @ 2018-04-09 17:54 UTC (permalink / raw) To: linux-arm-kernel On Mon, Apr 9, 2018 at 9:10 AM, Leo Yan <leo.yan@linaro.org> wrote: > Hi Mathieu, Kees, > > On Fri, Apr 06, 2018 at 09:47:31AM -0600, Mathieu Poirier wrote: >> On 5 April 2018 at 00:26, Kees Cook <keescook@chromium.org> wrote: >> > On Tue, Mar 13, 2018 at 10:24 AM, Mathieu Poirier >> > <mathieu.poirier@linaro.org> wrote: >> >> From: Leo Yan <leo.yan@linaro.org> >> >> >> >> Commit ad67b74d2469 ("printk: hash addresses printed with %p") lets >> >> printk specifier %p to hash all addresses before printing, this was >> >> resulting in the high 32 bits of pcsr can only output zeros. So >> >> module cannot completely print pc value and it's pointless for debugging >> >> purpose. >> >> >> >> This patch fixes this by using %px to print pcsr instead. >> > >> > Why is %p (and now %px) needed here at all? %pS is already used, which >> > should give useful debugging details, yes? >> >> Hey Leo, >> >> I've taken a second look at this and Kees has a point, %pS should be >> sufficient here. Please test again and see if %px is absolutely >> necessary. If you think that is the case please provide a snapshot of >> the corner case that makes the change mandatory. > > Sorry I missed replying for this patch due to my email filter and > thanks for suggestion. > > I can think out one case which should use %px: if CPU is locked up in > firmwares (e.g. NS-EL2 or ARM-TF in S-EL3), we can rely on %px to read > back PC hex value; for this case the CPU PC address is out of kernel > space. > > How about you guys think for this? Wouldn't %pS still give you a meaningful offset? -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] coresight: Use %px to print pcsr instead of %p 2018-04-09 17:54 ` Kees Cook @ 2018-04-09 23:44 ` Leo Yan 0 siblings, 0 replies; 10+ messages in thread From: Leo Yan @ 2018-04-09 23:44 UTC (permalink / raw) To: linux-arm-kernel Hi Kees, On Mon, Apr 09, 2018 at 10:54:58AM -0700, Kees Cook wrote: > On Mon, Apr 9, 2018 at 9:10 AM, Leo Yan <leo.yan@linaro.org> wrote: > > Hi Mathieu, Kees, > > > > On Fri, Apr 06, 2018 at 09:47:31AM -0600, Mathieu Poirier wrote: > >> On 5 April 2018 at 00:26, Kees Cook <keescook@chromium.org> wrote: > >> > On Tue, Mar 13, 2018 at 10:24 AM, Mathieu Poirier > >> > <mathieu.poirier@linaro.org> wrote: > >> >> From: Leo Yan <leo.yan@linaro.org> > >> >> > >> >> Commit ad67b74d2469 ("printk: hash addresses printed with %p") lets > >> >> printk specifier %p to hash all addresses before printing, this was > >> >> resulting in the high 32 bits of pcsr can only output zeros. So > >> >> module cannot completely print pc value and it's pointless for debugging > >> >> purpose. > >> >> > >> >> This patch fixes this by using %px to print pcsr instead. > >> > > >> > Why is %p (and now %px) needed here at all? %pS is already used, which > >> > should give useful debugging details, yes? > >> > >> Hey Leo, > >> > >> I've taken a second look at this and Kees has a point, %pS should be > >> sufficient here. Please test again and see if %px is absolutely > >> necessary. If you think that is the case please provide a snapshot of > >> the corner case that makes the change mandatory. > > > > Sorry I missed replying for this patch due to my email filter and > > thanks for suggestion. > > > > I can think out one case which should use %px: if CPU is locked up in > > firmwares (e.g. NS-EL2 or ARM-TF in S-EL3), we can rely on %px to read > > back PC hex value; for this case the CPU PC address is out of kernel > > space. > > > > How about you guys think for this? > > Wouldn't %pS still give you a meaningful offset? My fault, I tested at my side %pS does output offset for PC value out of kernel space (e.g. when PC = 0x12345678). Thank you for the suggestions ;) Will send new patch for the fixing. Thanks, Leo Yan ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] coresight: etm4x: Fix bit shifting 2018-03-13 17:24 [PATCH 0/2] coresight: next v4.16-rc5 Mathieu Poirier 2018-03-13 17:24 ` [PATCH 1/2] coresight: Use %px to print pcsr instead of %p Mathieu Poirier @ 2018-03-13 17:24 ` Mathieu Poirier 2018-03-14 16:59 ` [PATCH 0/2] coresight: next v4.16-rc5 Greg KH 2 siblings, 0 replies; 10+ messages in thread From: Mathieu Poirier @ 2018-03-13 17:24 UTC (permalink / raw) To: linux-arm-kernel From: Bo Yan <byan@nvidia.com> ctxid_pid and vmid_val in config are of type u64. When an integer 0xFF is being left shifted more than 32 bits, the behavior is undefined. The fix is to specify 0xFF as an unsigned long. Detected by Coverity scan: CID 37650, 37651 (Bad bit shift operation) Signed-off-by: Bo Yan <byan@nvidia.com> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> --- drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c index 4e6eab53e34e..d21961710713 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -1780,7 +1780,7 @@ static ssize_t ctxid_masks_store(struct device *dev, */ for (j = 0; j < 8; j++) { if (maskbyte & 1) - config->ctxid_pid[i] &= ~(0xFF << (j * 8)); + config->ctxid_pid[i] &= ~(0xFFUL << (j * 8)); maskbyte >>= 1; } /* Select the next ctxid comparator mask value */ @@ -1963,7 +1963,7 @@ static ssize_t vmid_masks_store(struct device *dev, */ for (j = 0; j < 8; j++) { if (maskbyte & 1) - config->vmid_val[i] &= ~(0xFF << (j * 8)); + config->vmid_val[i] &= ~(0xFFUL << (j * 8)); maskbyte >>= 1; } /* Select the next vmid comparator mask value */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 0/2] coresight: next v4.16-rc5 2018-03-13 17:24 [PATCH 0/2] coresight: next v4.16-rc5 Mathieu Poirier 2018-03-13 17:24 ` [PATCH 1/2] coresight: Use %px to print pcsr instead of %p Mathieu Poirier 2018-03-13 17:24 ` [PATCH 2/2] coresight: etm4x: Fix bit shifting Mathieu Poirier @ 2018-03-14 16:59 ` Greg KH 2018-03-15 14:41 ` Mathieu Poirier 2 siblings, 1 reply; 10+ messages in thread From: Greg KH @ 2018-03-14 16:59 UTC (permalink / raw) To: linux-arm-kernel On Tue, Mar 13, 2018 at 11:24:29AM -0600, Mathieu Poirier wrote: > Hi Greg, > > These two patches came in last week and are confined to Coresight. Given > the late(ish) time in the cycle I would have normally waited for the > next one but the changes are small and trivial. > > Please consider - if you think it's too late they'll simply go in later. They aren't bugfixes, I'll queue them up now for 4.17-rc1. thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 0/2] coresight: next v4.16-rc5 2018-03-14 16:59 ` [PATCH 0/2] coresight: next v4.16-rc5 Greg KH @ 2018-03-15 14:41 ` Mathieu Poirier 0 siblings, 0 replies; 10+ messages in thread From: Mathieu Poirier @ 2018-03-15 14:41 UTC (permalink / raw) To: linux-arm-kernel On 14 March 2018 at 10:59, Greg KH <gregkh@linuxfoundation.org> wrote: > On Tue, Mar 13, 2018 at 11:24:29AM -0600, Mathieu Poirier wrote: >> Hi Greg, >> >> These two patches came in last week and are confined to Coresight. Given >> the late(ish) time in the cycle I would have normally waited for the >> next one but the changes are small and trivial. >> >> Please consider - if you think it's too late they'll simply go in later. > > They aren't bugfixes, I'll queue them up now for 4.17-rc1. Perfect - that's what I was hoping. When reading my initial email I realise it wasn't exactly clear. Thanks, Mathieu > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-04-09 23:44 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-13 17:24 [PATCH 0/2] coresight: next v4.16-rc5 Mathieu Poirier 2018-03-13 17:24 ` [PATCH 1/2] coresight: Use %px to print pcsr instead of %p Mathieu Poirier 2018-04-05 6:26 ` Kees Cook 2018-04-06 15:47 ` Mathieu Poirier 2018-04-09 16:10 ` Leo Yan 2018-04-09 17:54 ` Kees Cook 2018-04-09 23:44 ` Leo Yan 2018-03-13 17:24 ` [PATCH 2/2] coresight: etm4x: Fix bit shifting Mathieu Poirier 2018-03-14 16:59 ` [PATCH 0/2] coresight: next v4.16-rc5 Greg KH 2018-03-15 14:41 ` Mathieu Poirier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).