Messages in this thread | | | Date | Wed, 4 May 2016 15:34:17 -0400 | From | Steven Rostedt <> | Subject | Re: 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
| |