linux-kernel - Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases

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]

Date:	Mon, 22 Jun 2015 18:10:02 +0200
From:	Christoph Hellwig <hch@....de>
To:	Dan Williams <dan.j.williams@...el.com>
Cc:	arnd@...db.de, mingo@...hat.com, bp@...en8.de, hpa@...or.com,
	tglx@...utronix.de, ross.zwisler@...ux.intel.com,
	akpm@...ux-foundation.org, jgross@...e.com, x86@...nel.org,
	toshi.kani@...com, linux-nvdimm@...ts.01.org,
	benh@...nel.crashing.org, mcgrof@...e.com, konrad.wilk@...cle.com,
	linux-kernel@...r.kernel.org, stefan.bader@...onical.com,
	luto@...capital.net, linux-mm@...ck.org, geert@...ux-m68k.org,
	ralf@...ux-mips.org, hmh@....eng.br, mpe@...erman.id.au,
	tj@...nel.org, paulus@...ba.org, hch@....de
Subject: Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases

On Mon, Jun 22, 2015 at 04:24:27AM -0400, Dan Williams wrote:
> Some archs define the first parameter to ioremap() as unsigned long,
> while the balance define it as resource_size_t.  Unify on
> resource_size_t to enable passing ioremap function pointers.  Also, some
> archs use function-like macros for defining ioremap aliases, but
> asm-generic/io.h expects object-like macros, unify on the latter.
> 
> Move all handling of ioremap aliasing (i.e. ioremap_wt => ioremap) to
> include/linux/io.h.  Add a check to include/linux/io.h to warn at
> compile time if an arch violates expectations.
> 
> Kill ARCH_HAS_IOREMAP_WC and ARCH_HAS_IOREMAP_WT in favor of just
> testing for ioremap_wc, and ioremap_wt being defined.  This arrangement
> allows drivers to know when ioremap_<foo> are being re-directed to plain
> ioremap.
> 
> Reported-by: kbuild test robot <fengguang.wu@...el.com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>

Hmm, this is quite a bit of churn, and doesn't make the interface lot
more obvious.

I guess it's enough to get the pmem related bits going, but I'd really
prefer defining the ioremap* prototype in linux/io.h and requiring
and out of line implementation in the architectures, it's not like
it's a fast path.  And to avoid the ifdef mess make it something like:

void __iomem *ioremap_flags(resource_size_t offset, unsigned long size,
			unsigned long prot_val, unsigned flags);

static inline void __iomem *ioremap(resource_size_t offset, unsigned long size)
{
	return ioremap_flags(offset, size, 0, 0);
}

static inline void __iomem *ioremap_prot(resource_size_t offset,
		unsigned long size, unsigned long prot_val)
{
	return ioremap_flags(offset, size, prot_val, 0);
}

static inline void __iomem *ioremap_nocache(resource_size_t offset,
		unsigned long size)
{
	return ioremap_flags(offset, size, 0, IOREMAP_NOCACHE);
}

static inline void __iomem *ioremap_cache(resource_size_t offset,
		unsigned long size)
{
	return ioremap_flags(offset, size, 0, IOREMAP_CACHE);
}

static inline void __iomem *ioremap_uc(resource_size_t offset,
		unsigned long size)
{
	return ioremap_flags(offset, size, 0, IOREMAP_UC);
}

static inline void __iomem *ioremap_wc(resource_size_t offset,
		unsigned long size)
{
	return ioremap_flags(offset, size, 0, IOREMAP_WC);
}

static inline void __iomem *ioremap_wt(resource_size_t offset,
		unsigned long size)
{
	return ioremap_flags(offset, size, 0, IOREMAP_WT);
}

With all wrappers but ioremap() itself deprecated in the long run.

Besides following the one API one prototype guideline this gives
us one proper entry point for all the variants.  Additionally
it can reject non-supported caching modes at run time, e.g. because
different hardware may or may not support it.  Additionally it
avoids the need for all these HAVE_IOREMAP_FOO defines, which need
constant updating.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/