LKML Archive on lore.kernel.org
 help / Atom feed
From: Martin Mares <mj@ucw.cz>
To: Matthew Wilcox <willy@infradead.org>
Cc: "Logan Gunthorpe" <logang@deltatee.com>,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-doc@vger.kernel.org, "Stephen Bates" <sbates@raithlin.com>,
	"Christoph Hellwig" <hch@lst.de>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	"Marc Zyngier" <marc.zyngier@arm.com>,
	"Kai-Heng Feng" <kai.heng.feng@canonical.com>,
	"Frederic Weisbecker" <frederic@kernel.org>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"Jérôme Glisse" <jglisse@redhat.com>,
	"Benjamin Herrenschmidt" <benh@kernel.crashing.org>,
	"Alex Williamson" <alex.williamson@redhat.com>,
	"Christian König" <christian.koenig@amd.com>
Subject: Re: lspci: Display path to device
Date: Fri, 10 Aug 2018 11:35:01 +0200
Message-ID: <mj+md-20180810.091549.51451.nikam@ucw.cz> (raw)
In-Reply-To: <20180717203900.GA1771@bombadil.infradead.org>

Hello!

> I don't like telling the user to grovel around lspci -t by hand.  It's
> not many lines of code to add a new -P option to lspci to show the path
> to each device instead of bus:dev.fn

I like the feature, but I have a couple of minor objections to the
implementation:

> -static char options[] = "nvbxs:d:ti:mgp:qkMDQ" GENERIC_OPTIONS ;
> +static char options[] = "nvbxs:d:tPi:mgp:qkMDQ" GENERIC_OPTIONS ;

Please update the help_msg, too.

> +static void
> +show_slot_path(struct pci_dev *p)
> +{
> +  struct pci_dev *d = NULL;
> +
> +  if (opt_path && p->bus)
> +    {
> +      for (d = p->access->devices; d; d = d->next) {
> +	if (d->hdrtype == -1)
> +	  d->hdrtype = pci_read_byte(d, PCI_HEADER_TYPE) & 0x7f;

Please do not modify the hdrtype field, it is private to libpci.

Instead, always read the PCI_HEADER_TYPE register. It will be fast since
lspci caches configuration space accesses.

> +	if (d->hdrtype != PCI_HEADER_TYPE_BRIDGE &&
> +	    d->hdrtype != PCI_HEADER_TYPE_CARDBUS)
> +	  continue;
> +	if (pci_read_byte(d, PCI_SECONDARY_BUS) > p->bus)
> +	  continue;
> +	if (pci_read_byte(d, PCI_SUBORDINATE_BUS) < p->bus)
> +	  continue;

Beware, cardbus bridges use different registers for secondary and subordinate bus.

... the more I think about it, the more I am convinced that we want to
re-use the bus topology builder from ls-tree.c. I will give it a try,
stay tuned.

				Martin

  parent reply index

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-17 17:02 [PATCH v7 0/4] Add parameter for disabling ACS redirection for P2P Logan Gunthorpe
2018-07-17 17:02 ` [PATCH v7 1/4] PCI: Make specifying PCI devices in kernel parameters reusable Logan Gunthorpe
2018-07-17 17:02 ` [PATCH v7 2/4] PCI: Allow specifying devices using a base bus and path of devfns Logan Gunthorpe
2018-07-17 17:02 ` [PATCH v7 3/4] PCI: Introduce disable_acs_redir quirk Logan Gunthorpe
2018-07-17 17:48   ` Alex Williamson
2018-07-17 17:02 ` [PATCH v7 4/4] PCI: Introduce the disable_acs_redir parameter Logan Gunthorpe
2018-07-17 17:48   ` Alex Williamson
2018-07-17 17:49     ` Logan Gunthorpe
2018-07-17 17:57       ` Stephen  Bates
2018-07-17 20:39 ` lspci: Display path to device Matthew Wilcox
2018-07-17 20:51   ` Logan Gunthorpe
2018-07-17 21:00   ` Bjorn Helgaas
2018-07-17 21:46     ` Matthew Wilcox
2018-08-09 23:48   ` Matthew Wilcox
2018-08-10  9:35   ` Martin Mares [this message]
2018-08-10 10:30     ` Martin Mares
2018-08-10 14:56       ` Matthew Wilcox
2018-08-12  9:28         ` Martin Mares
2018-08-12 10:31           ` Matthew Wilcox
2018-08-12 10:51             ` Martin Mares
2018-08-13 15:55             ` Logan Gunthorpe

Reply instructions:

You may reply publically 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=mj+md-20180810.091549.51451.nikam@ucw.cz \
    --to=mj@ucw.cz \
    --cc=alex.williamson@redhat.com \
    --cc=benh@kernel.crashing.org \
    --cc=bhelgaas@google.com \
    --cc=christian.koenig@amd.com \
    --cc=corbet@lwn.net \
    --cc=dan.j.williams@intel.com \
    --cc=frederic@kernel.org \
    --cc=hch@lst.de \
    --cc=jglisse@redhat.com \
    --cc=kai.heng.feng@canonical.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=logang@deltatee.com \
    --cc=marc.zyngier@arm.com \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=sbates@raithlin.com \
    --cc=tglx@linutronix.de \
    --cc=willy@infradead.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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox