Re: [PATCH] tracing: Expose CPU physical addresses (resource values) for PCI devices

3 messages, 2 authors, 2016-06-20 · open the first message on its own page

Re: [PATCH] tracing: Expose CPU physical addresses (resource values) for PCI devices

From: Steven Rostedt <rostedt@goodmis.org>
Date: 2016-06-17 21:59:10

Sorry for the late reply, this patch got pushed down in my INBOX.

Could I get someone from PPC to review this patch, just to be safe?

Thanks!

-- Steve



On Wed, 11 May 2016 14:06:57 -0500
Bjorn Helgaas [off-list ref] wrote:
quoted hunk
Previously, mmio_print_pcidev() put "user" addresses in the trace buffer.
On most architectures, these are the same as CPU physical addresses, but on
microblaze, mips, powerpc, and sparc, they may be something else, typically
a raw BAR value (a bus address as opposed to a CPU address).

Always expose the CPU physical address to avoid this arch-dependent
behavior.

This change should have no user-visible effect because this file currently
depends on CONFIG_HAVE_MMIOTRACE_SUPPORT, which is only defined for x86,
and pci_resource_to_user() is a no-op on x86.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 kernel/trace/trace_mmiotrace.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c
index 68f376c..cd7480d 100644
--- a/kernel/trace/trace_mmiotrace.c
+++ b/kernel/trace/trace_mmiotrace.c
@@ -68,19 +68,15 @@ static void mmio_print_pcidev(struct trace_seq *s, const struct pci_dev *dev)
 	trace_seq_printf(s, "PCIDEV %02x%02x %04x%04x %x",
 			 dev->bus->number, dev->devfn,
 			 dev->vendor, dev->device, dev->irq);
-	/*
-	 * XXX: is pci_resource_to_user() appropriate, since we are
-	 * supposed to interpret the __ioremap() phys_addr argument based on
-	 * these printed values?
-	 */
 	for (i = 0; i < 7; i++) {
-		pci_resource_to_user(dev, i, &dev->resource[i], &start, &end);
+		start = dev->resource[i].start;
 		trace_seq_printf(s, " %llx",
 			(unsigned long long)(start |
 			(dev->resource[i].flags & PCI_REGION_FLAG_MASK)));
 	}
 	for (i = 0; i < 7; i++) {
-		pci_resource_to_user(dev, i, &dev->resource[i], &start, &end);
+		start = dev->resource[i].start;
+		end = dev->resource[i].end;
 		trace_seq_printf(s, " %llx",
 			dev->resource[i].start < dev->resource[i].end ?
 			(unsigned long long)(end - start) + 1 : 0);

Re: [PATCH] tracing: Expose CPU physical addresses (resource values) for PCI devices

From: Benjamin Herrenschmidt <hidden>
Date: 2016-06-17 22:23:48

On Fri, 2016-06-17 at 17:59 -0400, Steven Rostedt wrote:
Sorry for the late reply, this patch got pushed down in my INBOX.

Could I get someone from PPC to review this patch, just to be safe?
The patch makes sense, I can try getting somebody onto porting
mmiotrace one of these days.

Cheers,
Ben.
Thanks!

-- Steve



On Wed, 11 May 2016 14:06:57 -0500
Bjorn Helgaas [off-list ref] wrote:
quoted
Previously, mmio_print_pcidev() put "user" addresses in the trace
buffer.
On most architectures, these are the same as CPU physical
addresses, but on
microblaze, mips, powerpc, and sparc, they may be something else,
typically
a raw BAR value (a bus address as opposed to a CPU address).

Always expose the CPU physical address to avoid this arch-dependent
behavior.

This change should have no user-visible effect because this file
currently
depends on CONFIG_HAVE_MMIOTRACE_SUPPORT, which is only defined for
x86,
and pci_resource_to_user() is a no-op on x86.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 kernel/trace/trace_mmiotrace.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/kernel/trace/trace_mmiotrace.c
b/kernel/trace/trace_mmiotrace.c
index 68f376c..cd7480d 100644
--- a/kernel/trace/trace_mmiotrace.c
+++ b/kernel/trace/trace_mmiotrace.c
@@ -68,19 +68,15 @@ static void mmio_print_pcidev(struct trace_seq
*s, const struct pci_dev *dev)
 	trace_seq_printf(s, "PCIDEV %02x%02x %04x%04x %x",
 			 dev->bus->number, dev->devfn,
 			 dev->vendor, dev->device, dev->irq);
-	/*
-	 * XXX: is pci_resource_to_user() appropriate, since we
are
-	 * supposed to interpret the __ioremap() phys_addr
argument based on
-	 * these printed values?
-	 */
 	for (i = 0; i < 7; i++) {
-		pci_resource_to_user(dev, i, &dev->resource[i],
&start, &end);
+		start = dev->resource[i].start;
 		trace_seq_printf(s, " %llx",
 			(unsigned long long)(start |
 			(dev->resource[i].flags &
PCI_REGION_FLAG_MASK)));
 	}
 	for (i = 0; i < 7; i++) {
-		pci_resource_to_user(dev, i, &dev->resource[i],
&start, &end);
+		start = dev->resource[i].start;
+		end = dev->resource[i].end;
 		trace_seq_printf(s, " %llx",
 			dev->resource[i].start < dev-
quoted
resource[i].end ?
 			(unsigned long long)(end - start) + 1 :
0);
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] tracing: Expose CPU physical addresses (resource values) for PCI devices

From: Steven Rostedt <rostedt@goodmis.org>
Date: 2016-06-20 13:58:35

On Sat, 18 Jun 2016 08:23:23 +1000
Benjamin Herrenschmidt [off-list ref] wrote:
On Fri, 2016-06-17 at 17:59 -0400, Steven Rostedt wrote:
quoted
Sorry for the late reply, this patch got pushed down in my INBOX.

Could I get someone from PPC to review this patch, just to be safe?  
The patch makes sense, I can try getting somebody onto porting
mmiotrace one of these days.
OK, thanks!

I'll pull this into my for-next repo then.

-- Steve
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help