svn commit: r277643 - in head/sys: arm/arm dev/mem i386/i386 mips/mips sparc64/sparc64

[Ian Lepore]

On Sat, 2015-01-24 at 12:33 -0600, Alan Cox wrote:
> On 01/24/2015 09:42, Konstantin Belousov wrote:
> > On Sat, Jan 24, 2015 at 07:56:37AM -0700, Ian Lepore wrote:
> >> On Sat, 2015-01-24 at 12:51 +0000, Konstantin Belousov wrote:
> >>> Author: kib
> >>> Date: Sat Jan 24 12:51:15 2015
> >>> New Revision: 277643
> >>> URL: https://svnweb.freebsd.org/changeset/base/277643
> >>>
> >>> Log:
> >>>   Remove Giant from /dev/mem and /dev/kmem.  It is definitely not needed
> >>>   for i386, and from the code inspection, nothing in the
> >>>   arm/mips/sparc64 implementations depends on it.
> >>>   
> >> I'm not sure I agree with that.  On arm the memrw() implementation uses
> >> a single statically-allocated page of kva space into which it maps each
> >> physical page in turn in the main loop.  What prevents preemption or
> >> multicore access to /dev/mem from trying to use that single page for
> >> multiple operations at once?
> > I see, thank you for noting this.
> >
> > But, I do not think that Giant is a solution for the problem. uiomove()
> > call accesses userspace, which may fault and cause sleep. If the
> > thread sleeps, the Giant is automatically dropped, so there is no real
> > protection.
> >
> > I think dump exclusive sx around whole memrw() should be enough.
> >
> > I can revert the commit for now, or I can leave it as is while
> > writing the patch with sx and waiting for somebody review.  What
> > would you prefer ?
> >
> > P.S. mips uses uiomove_fromphys(), avoiding transient mapping,
> > and sparc allocates KVA when needed.
> >
> >
> 
> While we're here, it's worth noting that the arm version of /dev/mem is
> not functionally equivalent to that of amd64 or i386.  Arm disallows
> access to non-DRAM addresses through /dev/mem.

That's true for the read/write interface, but not for mmap().  In fact,
we have users insisting that mmap() on /dev/mem should provide userland
access to memory-mapped devices, and we have ARM architecture rules that
say you can't map the same physical address multiple times with
different attributes, such as being Device memory in the kernel and
Strongly Ordered when mapped into userland.  But if the memory isn't
mapped S-O for userland, they have no hope of usefully accessing the
devices (because they don't have access to cache and buffer
maintenance).

Even "normal" memory has a variety of attributes that make the temporary
mappings done in memrw() a bit iffy, although I'm not sure we're doing
anything right now that could lead to trouble.  Trouble lurks though if
we ever start using some of the more subtle features of the arm memory
architecture, such as turning off the sharable attribute on pages that
are inherently per-cpu to avoid the overhead of hardware cache coherence
when we know only one core can access the pages.

-- Ian