linux-kernel - Re: [PATCH] drivers: pci: convert generic host controller to DT host bridge creation API

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: <201408201323.52404.arnd@arndb.de>
Date:	Wed, 20 Aug 2014 13:23:52 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Will Deacon <will.deacon@....com>
Cc:	Liviu Dudau <Liviu.Dudau@....com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Catalin Marinas <Catalin.Marinas@....com>,
	Jingoo Han <jg1.han@...sung.com>,
	Kukjin Kim <kgene.kim@...sung.com>,
	Suravee Suthikulanit <suravee.suthikulpanit@....com>,
	"linux-pci" <linux-pci@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	LAKML <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] drivers: pci: convert generic host controller to DT host bridge creation API

On Tuesday 19 August 2014, Will Deacon wrote:
> On Tue, Aug 12, 2014 at 05:41:35PM +0100, Liviu Dudau wrote:

> > +     if (!res_valid) {
> > +             dev_err(dev, "non-prefetchable memory resource required\n");
> > +             return -EINVAL;
> > +     }

I don't see why this part should be in the host controller driver. It's really
a sanity check that could apply anywhere, so could we just move it into the
common code, or alternatively drop it?

> > +     if (iores) {
> > +             if (!PAGE_ALIGNED(io_cpuaddr))
> > +                     return -EINVAL;
> 
> Why is this alignment check not in the core code?

This confused me too. I have to look back at the core code, but I assume it's
either aligned already based on the way this number gets created, or it
can have an offset within the page that is the same as the offset within the
physical address of iores and that hould be handled internally by
pci_remap_iospace().

> Probably a question for
> somebody like Arnd, but do we need to deal with multiple IO resources?
> Currently we'll just silently take the last one that we found, which doesn't
> sound ideal.

I can't think of a case where you'd actually have multiple IO resources,
but I agree we should either treat that as an error or handle it right.
My guess is that handling it is actually easier.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/