lkml.org 
[lkml]   [2016]   [May]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: ftrace use of pci_resource_to_user()
On Wed, 4 May 2016 14:17:13 -0500
Bjorn Helgaas <helgaas@kernel.org> wrote:

> 138295373ccf ("ftrace: mmiotrace update, #2") added this use of
> pci_resource_to_user():
>
> +static int mmio_print_pcidev(struct trace_seq *s, const struct pci_dev *dev)
> +{
> ...
> + /*
> + * 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);
> + ret += trace_seq_printf(s, " %llx",
> + (unsigned long long)(start |
> + (dev->resource[i].flags & PCI_REGION_FLAG_MASK)));
> + }
>
> I think it was a mistake to use pci_resource_to_user() here because it
> adds unnecessary arch dependencies in whatever consumes this output.
>
> On most arches, pci_resource_to_user() is a no-op and the result is
> normal resource addresses, i.e., CPU physical addresses that match
> things in /proc/iomem and /sys/devices/pci.../resource.
>
> On microblaze, mips, powerpc, and sparc, the result of
> pci_resource_to_user() is something else, usually a PCI bus address (a
> raw BAR value). These values are only useful for using mmap on
> files like /proc/bus/pci/....
>
> I don't know what, if anything, consumes this output. If things parse
> it, we shouldn't break them. But those things likely would need
> special cases for microblaze, mips, powerpc, and sparc.
>
> If it's only for human consumption, I think we should consider
> removing the use of pci_resource_to_user() and printing
> dev->resource[i].start instead.

Currently this code requires the arch to define HAVE_MMIOTRACE_SUPPORT,
and so far as I can tell, only x86 does this.

-- Steve

\
 
 \ /
  Last update: 2016-05-04 21:41    [W:0.045 / U:0.168 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site