linux-kernel - Re: [PATCH v4 1/4] lib: Introduce atomic MMIO modify

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: <20130824231526.GS6617@n2100.arm.linux.org.uk>
Date:	Sun, 25 Aug 2013 00:15:26 +0100
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>
Cc:	richard -rw- weinberger <richard.weinberger@...il.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Gregory Clement <gregory.clement@...e-electrons.com>,
	Lior Amsalem <alior@...vell.com>,
	Baruch Siach <baruch@...s.co.il>,
	Will Deacon <will.deacon@....com>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	Catalin Marinas <catalin.marinas@....com>
Subject: Re: [PATCH v4 1/4] lib: Introduce atomic MMIO modify

On Sat, Aug 24, 2013 at 04:58:59PM -0300, Ezequiel Garcia wrote:
> On Sat, Aug 24, 2013 at 08:27:10PM +0200, richard -rw- weinberger wrote:
> > On Sat, Aug 24, 2013 at 5:35 PM, Ezequiel Garcia
> > <ezequiel.garcia@...e-electrons.com> wrote:
> > > Some platforms have MMIO regions that are shared across orthogonal
> > > subsystems. This commit implements a possible solution for the
> > > thread-safe access of such regions through a spinlock-protected API.
> > >
> > > Concurrent access is protected with a single spinlock for the
> > > entire MMIO address space. While this protects shared-registers,
> > > it also serializes access to unrelated/unshared registers.
> > >
> > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>
> > > ---
> > >  include/linux/io.h |  5 +++++
> > >  lib/Makefile       |  2 +-
> > >  lib/atomicio.c     | 27 +++++++++++++++++++++++++++
> > >  3 files changed, 33 insertions(+), 1 deletion(-)
> > >  create mode 100644 lib/atomicio.c
> > >
> > > diff --git a/include/linux/io.h b/include/linux/io.h
> > > index f4f42fa..c331dcb 100644
> > > --- a/include/linux/io.h
> > > +++ b/include/linux/io.h
> > > @@ -101,4 +101,9 @@ static inline void arch_phys_wc_del(int handle)
> > >  #define arch_phys_wc_add arch_phys_wc_add
> > >  #endif
> > >
> > > +#ifndef __HAVE_ARCH_ATOMIC_IO_MODIFY
> > > +/* Atomic MMIO-wide IO modify */
> > > +extern void atomic_io_modify(void __iomem *reg, u32 mask, u32 set);
> > > +#endif
> > > +
> > >  #endif /* _LINUX_IO_H */
> > > diff --git a/lib/Makefile b/lib/Makefile
> > > index 7baccfd..695d6e2 100644
> > > --- a/lib/Makefile
> > > +++ b/lib/Makefile
> > > @@ -13,7 +13,7 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
> > >          sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
> > >          proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \
> > >          is_single_threaded.o plist.o decompress.o kobject_uevent.o \
> > > -        earlycpio.o percpu-refcount.o
> > > +        earlycpio.o percpu-refcount.o atomicio.o
> > >
> > >  obj-$(CONFIG_ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS) += usercopy.o
> > >  lib-$(CONFIG_MMU) += ioremap.o
> > > diff --git a/lib/atomicio.c b/lib/atomicio.c
> > > new file mode 100644
> > > index 0000000..1750f9d
> > > --- /dev/null
> > > +++ b/lib/atomicio.c
> > > @@ -0,0 +1,27 @@
> > > +#include <linux/io.h>
> > > +#include <linux/spinlock.h>
> > > +
> > > +#ifndef __HAVE_ARCH_ATOMIC_IO_MODIFY
> > > +/*
> > > + * Generic atomic MMIO modify.
> > > + *
> > > + * Allows thread-safe access to registers shared by unrelated subsystems.
> > > + * The access is protected by a single MMIO-wide lock.
> > > + *
> > > + * Optimized variants can be implemented on a per-architecture basis.
> > > + */
> > > +static DEFINE_RAW_SPINLOCK(__io_lock);
> > > +void atomic_io_modify(void __iomem *reg, u32 mask, u32 set)
> > > +{
> > > +       unsigned long flags;
> > > +       u32 value;
> > > +
> > > +       raw_spin_lock_irqsave(&__io_lock, flags);
> > > +       value = readl(reg) & ~mask;
> > > +       value |= (set & mask);
> > > +       writel(value, reg);
> > > +       raw_spin_unlock_irqrestore(&__io_lock, flags);
> > > +
> > > +}
> > > +EXPORT_SYMBOL(atomic_io_modify);
> > 
> > Why not the default case EXPORT_SYMBOL_GPL()?
> > 
> 
> Because I copy-pasted the export from some other lib/.. :-)
> 
> Mind explaining me the difference, why you say _GPL it's the default,
> and why EXPORT_SYMBOL is more frequently used in lib/ ?

This is actually a decision solely for the author of the code which is
being created: it's a statement about the _use_ of the symbol.

Do you wish to permit your code to be used by modules with non-GPL
compatible licenses - such as closed source modules?  If so, then use
EXPORT_SYMBOL().

If you wish your code to only be used by GPL compatible modules, then
use EXPORT_SYMBOL_GPL().

Things get a little murkey if you call code which has been exported with
EXPORT_SYMBOL_GPL() and you wish to mark your new function with
EXPORT_SYMBOL(), because you're effectively bypassing acess restrictions
to taht code put in place by the original code author (you're not in this
case, but I'm including this statement for completeness.)
--
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/