| Message ID | 20160511190657.5898.4248.stgit@bhelgaas-glaptop2.roam.corp.google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
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 <bhelgaas@google.com> wrote: > 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); -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <bhelgaas@google.com> wrote: > > > 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); > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 18 Jun 2016 08:23:23 +1000 Benjamin Herrenschmidt <benh@au1.ibm.com> wrote: > 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. > OK, thanks! I'll pull this into my for-next repo then. -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html