linux-kernel - Re: [PATCH v2 08/25] arch: introduce memremap()

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: <20150727231755.GV30479@wotan.suse.de>
Date:	Tue, 28 Jul 2015 01:17:55 +0200
From:	"Luis R. Rodriguez" <mcgrof@...e.com>
To:	Dan Williams <dan.j.williams@...el.com>
Cc:	tglx@...utronix.de, mingo@...nel.org, hpa@...or.com,
	linux-arch@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
	linux-nvdimm@...ts.01.org, linux-kernel@...r.kernel.org,
	rmk+kernel@....linux.org.uk, hch@....de,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 08/25] arch: introduce memremap()

On Fri, Jul 24, 2015 at 10:38:42PM -0400, Dan Williams wrote:
> Existing users of ioremap_cache() are mapping memory that is known in
> advance to not have i/o side effects.  These users are forced to cast
> away the __iomem annotation, or otherwise neglect to fix the sparse
> errors thrown when dereferencing pointers to this memory.  Provide
> memremap() as a non __iomem annotated ioremap_*() in the case when
> ioremap is otherwise a pointer to memory.

Ok so a special use case.

> Outside of ioremap() and
> ioremap_nocache(), the expectation is that most calls to
> ioremap_<type>() are seeking memory-like semantics (e.g.  speculative
> reads, and prefetching permitted).  These callsites can be moved to
> memremap() over time.

Such memory-like smantics are not well defined yet and this has caused
issues over expectations over a slew of APIs. As you note above
your own defined 'semantics' so far for memremap are just that there are
no i/o side effects, when the mapped memory is just a pointer to memory,
as such I do not think its fair to set the excpectations that we'll
switch all other ioremap_*() callers to the same memremap() API. Now,
it may be a good idea to use something similar, ie, to pass flags,
but setting the expectations outright to move to memremap() without having
any agreement having been made over semantics seems uncalled for at this
point in time, specially when you are noting that the expectations for
both sets of calls are different.

So perhaps:

"
Outside of ioremap() and ioremap_nocache(), all other ioremap_<type>()
variant calls are seeking memory-like semantics (e.g.  speculative
reads, and prefetching permitted) and all calls expecations currently
differ depending on architecture. Once and if we get agreement on such
semantics we may be able to move such ioremap_*() variant calls to
a similar API where the semantics required are clearly spelled out
and well defined and passed as arguments.
"

> diff --git a/kernel/memremap.c b/kernel/memremap.c
> new file mode 100644
> index 000000000000..ba206fd11785
> --- /dev/null
> +++ b/kernel/memremap.c
> @@ -0,0 +1,82 @@
> +/*
> + * Copyright(c) 2015 Intel Corporation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */
> +#include <linux/types.h>
> +#include <linux/io.h>
> +#include <linux/mm.h>
> +
> +#ifndef ioremap_cache
> +/* temporary while we convert existing ioremap_cache users to memremap */
> +__weak void __iomem *ioremap_cache(resource_size_t offset, unsigned long size)
> +{
> +	return ioremap(offset, size);
> +}
> +#endif
> +
> +/*
> + * memremap() is "ioremap" for cases where it is known that the resource
> + * being mapped does not have i/o side effects and the __iomem
> + * annotation is not applicable.
> + */
> +void *memremap(resource_size_t offset, size_t size, unsigned long flags)
> +{
> +	int is_ram = region_is_ram(offset, size);
> +	void *addr = NULL;
> +
> +	if (is_ram < 0) {
> +		WARN_ONCE(1, "memremap attempted on mixed range %pa size: %zu\n",
> +				&offset, size);
> +		return NULL;
> +	}
> +
> +	/* Try all mapping types requested until one returns non-NULL */
> +	if (flags & MEMREMAP_CACHE) {
> +		flags &= ~MEMREMAP_CACHE;
> +		/*
> +		 * MEMREMAP_CACHE is special in that it can be satisifed
> +		 * from the direct map.  Some archs depend on the
> +		 * capability of memremap() to autodetect cases where
> +		 * the requested range is potentially in "System RAM"
> +		 */
> +		if (is_ram)
> +			addr = __va(offset);

The no MMU case seems to match this, can that be switch to this?

  Luis
--
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/