From: Bjorn Helgaas <helgaas@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>,
Ingo Molnar <mingo@redhat.com>,
Pekka Paalanen <ppaalanen@gmail.com>
Cc: linux-kernel@vger.kernel.org, Yinghai Lu <yinghai@kernel.org>
Subject: ftrace use of pci_resource_to_user()
Date: Wed, 4 May 2016 14:17:13 -0500 [thread overview]
Message-ID: <20160504191713.GA3117@localhost> (raw)
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.
Bjorn
next reply other threads:[~2016-05-04 19:17 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-04 19:17 Bjorn Helgaas [this message]
2016-05-04 19:34 ` ftrace use of pci_resource_to_user() Steven Rostedt
2016-05-06 10:16 ` Pekka Paalanen
2016-05-06 10:33 ` karol herbst
2016-05-11 19:04 ` Bjorn Helgaas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160504191713.GA3117@localhost \
--to=helgaas@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=ppaalanen@gmail.com \
--cc=rostedt@goodmis.org \
--cc=yinghai@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox