linux-kernel - Re: [PATCH 2/3] powerpc ioremap

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] [day] [month] [year] [list]

Message-Id: <20080429153634.1f7ceffb.akpm@linux-foundation.org>
Date:	Tue, 29 Apr 2008 15:36:34 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	benh@...nel.crashing.org
Cc:	riel@...hat.com, linux-kernel@...r.kernel.org, ajackson@...hat.com,
	airlied@...hat.com, paulus@...ba.org
Subject: Re: [PATCH 2/3] powerpc ioremap_prot

On Wed, 30 Apr 2008 08:07:53 +1000
Benjamin Herrenschmidt <benh@...nel.crashing.org> wrote:

> 
> On Tue, 2008-04-29 at 11:17 -0700, Andrew Morton wrote:
> > 
> > Given that x86 implements ioremap_prot() as a regular C function, it
> > would
> > be nicer to require that all architectures do that.  Especially as
> > macros
> > suck.
> > 
> > Your powerpc implementation of ioremap_prot() has a different
> > signature
> > from the x86 one: `phys_addr_t address' versus `resource_size_t
> > phys_addr'.
> > Can that be improved?
> 
> Well, we already had ioremap_flags() which is the same thing, that's why
> I made it just a #define :-)
> 
> But I'm pondering removing our ioremap_flags completely in favor of
> ioremap_prot. This was just a patch to "make it work" so Rik could move
> on with his core patch (btw. Rik, you got the SOBs in the wrong order on
> that one).
> 
> Regarding the difference, well, it has to do with us historically using
> that phys_addr_t type for ioremap. I can try to look into changing that
> but it will take a bit more effort.

It does seem pretty bad to create the same-named function in two
architectures, only with sometimes-different argument types.

A minimal fix would be to make powerpc's implementation be an out-of-line C
function which takes a resource_size_t and which calls ioremap_flags()?

> > >  static inline pte_t pte_mkspecial(pte_t pte) {
> > >       return pte; }
> > > +static inline unsigned long pte_pgprot(pte_t pte) {
> > > +     return __pgprot(pte_val(pte)) & PAGE_PROT_BITS; }
> > 
> > ick.  \n's are cheap.
> 
> Yeah well, just adapted to the style of the other ones around it :-)

I'm not a big believer in making new code match broken old code.

> Those things have been there forever, I think we can even blame
> pre-paulus maintainership here !

I blame Rusty.

> I'll change them all in one go in a different patch if you want.

whatever ;)
--
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/