coresight: next v4.16-rc5
cover coresight: next v4.16-rc5
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(-)
Signature · 1 line
2.7.4
PATCH 1/2 coresight: Use %px to print pcsr instead of %p
From: Leo Yan <[email protected]>
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.
--- drivers/hwtracing/coresight/coresight-cpu-debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hwtracing/coresight/coresight-cpu-debug.c +1 -1
@@ -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);
Signature · 1 line
2.7.4
PATCH 1/2 coresight: Use %px to print pcsr instead of %p
Quoted 9 lines · Mathieu Poirier
From: Leo Yan <[email protected]>
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
Signature · 2 lines
Kees Cook Pixel Security
PATCH 1/2 coresight: Use %px to print pcsr instead of %p
Quoted 10 lines · Kees Cook
Quoted 8 lines · Mathieu Poirier
From: Leo Yan <[email protected]>
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.
Quoted 4 lines
Quoted 1 line
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.
Quoted 6 lines
-Kees
--
Kees Cook
Pixel Security
PATCH 1/2 coresight: Use %px to print pcsr instead of %p
Hi Mathieu, Kees,
Quoted 16 lines · Mathieu Poirier
Quoted 10 lines · Kees Cook
Quoted 8 lines · Mathieu Poirier
From: Leo Yan <[email protected]>
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.Quoted 4 lines
Quoted 1 line
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?
Quoted 6 lines
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
PATCH 1/2 coresight: Use %px to print pcsr instead of %p
Quoted 29 lines · Leo Yan
Hi Mathieu, Kees,
Quoted 16 lines · Mathieu Poirier
Quoted 10 lines · Kees Cook
Quoted 8 lines · Mathieu Poirier
From: Leo Yan <[email protected]>
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.Quoted 4 lines
Quoted 1 line
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?
Quoted 6 lines
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.
Quoted 6 lines
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
Signature · 2 lines
Kees Cook Pixel Security
PATCH 1/2 coresight: Use %px to print pcsr instead of %p
Hi Kees,
Quoted 30 lines · Kees Cook
Quoted 29 lines · Leo Yan
Hi Mathieu, Kees,
Quoted 16 lines · Mathieu Poirier
Quoted 10 lines · Kees Cook
Quoted 8 lines · Mathieu Poirier
From: Leo Yan <[email protected]>
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.Quoted 4 lines
Quoted 1 line
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?
Quoted 6 lines
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.
Quoted 8 lines
Quoted 6 lines
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
PATCH 2/2 coresight: etm4x: Fix bit shifting
In reply to Mathieu PoirierFrom: Bo Yan <[email protected]>
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)
--- drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +2 -2
@@ -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 */
Signature · 1 line
2.7.4
cover coresight: next v4.16-rc5
In reply to Mathieu PoirierQuoted 7 lines · Mathieu Poirier
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
cover coresight: next v4.16-rc5
Quoted 10 lines · Greg KH
Quoted 7 lines · Mathieu Poirier
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
Quoted 4 lines
thanks,
greg k-h