linux-kernel - Re: ftrace use of pci_resource_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160504153417.62d657c6@gandalf.local.home>
Date:	Wed, 4 May 2016 15:34:17 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Bjorn Helgaas <helgaas@...nel.org>
Cc:	Ingo Molnar <mingo@...hat.com>,
	Pekka Paalanen <ppaalanen@...il.com>,
	linux-kernel@...r.kernel.org, Yinghai Lu <yinghai@...nel.org>
Subject: Re: ftrace use of pci_resource_to_user()

On Wed, 4 May 2016 14:17:13 -0500
Bjorn Helgaas <helgaas@...nel.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