linux-kernel - Re: [PATCH 01/24] PCI: Add iobusn

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: <20120210125150.3353237c@jbarnes-desktop>
Date:	Fri, 10 Feb 2012 12:51:50 -0800
From:	Jesse Barnes <jbarnes@...tuousgeek.org>
To:	Yinghai Lu <yinghai@...nel.org>
Cc:	Bjorn Helgaas <bhelgaas@...gle.com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Tony Luck <tony.luck@...el.com>,
	Dominik Brodowski <linux@...inikbrodowski.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Greg Kroah-Hartman <gregkh@...e.de>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org
Subject: Re: [PATCH 01/24] PCI: Add iobusn_resource

On Mon, 6 Feb 2012 10:55:14 -0800
Yinghai Lu <yinghai@...nel.org> wrote:

> On Mon, Feb 6, 2012 at 10:48 AM, Bjorn Helgaas <bhelgaas@...gle.com> wrote:
> >> +struct resource iobusn_resource = {
> >> +       .name   = "PCI busn",
> >> +       .start  = 0,
> >> +       .end    = 0xffffff,
> >> +       .flags  = IORESOURCE_BUS,
> >> +};
> >
> > I'm not sure this should be global.  iomem_resource and
> > ioport_resource *are* really global, because they refer to processor
> > address space that is the same for everybody.  But PCI bus numbers are
> > specific to PCI.  Some machines don't have PCI at all, and there are
> > different bus architectures to which this doesn't apply.
> 
> that does not hurt them.

Yes but it's superfluous and confusing if you're porting to a new arch
or looking at changes in generic code that may affect you.

> > The 0-0xffffff range is misleading because it includes both the domain
> > and the bus number, and it's meaningless to allocate ranges that cross
> > domain boundaries.  For example, [bus 0x0000f0-0x000120] includes bus
> > numbers from domain 0000 and domain 0001, which doesn't make any sense
> > because a bus can only be in one domain.
> 
> allocation code will make sure it will be cross the boundary for domain.

But that means everyone reading it will do a double take, have to dig
into the implementation, and only then say "ah yeah ok it looks
correct" rather than it being obvious from the fact that the resource
is tracked on a per-domain basis.

> > I think it would make more sense to keep this bus number resource in a
> > per-host bridge structure.  Then we wouldn't need to include the
> > domain number at all because the host bridge determines the domain.
> 
> not sure. insert the all busn_res of all peer root buses into one
> global iobusn_resource
> looks more simple.

In what sense?  Simpler in the sense of your current implementation,
but not simpler at all to someone just reading the code...

-- 
Jesse Barnes, Intel Open Source Technology Center

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)