From: Russell King - ARM Linux on
On Mon, Jul 19, 2010 at 10:55:15AM -0700, Michael Bohan wrote:
>
> On 7/16/2010 12:58 AM, Russell King - ARM Linux wrote:
>
>> As the patch has been out for RFC since early April on the linux-arm-kernel
>> mailing list (Subject: [RFC] Prohibit ioremap() on kernel managed RAM),
>> and no comments have come back from Qualcomm folk.
>
> Would it be unreasonable to allow a map request to succeed if the
> requested attributes matched that of the preexisting mapping?

What would be the point of creating such a mapping?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: FUJITA Tomonori on
On Mon, 19 Jul 2010 09:22:13 +0100
Russell King - ARM Linux <linux(a)arm.linux.org.uk> wrote:

> > If I want to share the buffer with another device I have to
> > make a copy of the entire thing then fix up the virtual mappings for
> > the other device I'm sharing with.
>
> This is something the DMA API doesn't do - probably because there hasn't
> been a requirement for it.
>
> One of the issues for drivers is that by separating the mapped scatterlist
> from the input buffer scatterlist, it creates something else for them to
> allocate, which causes an additional failure point - and as all users sit
> well with the current API, there's little reason to change especially
> given the number of drivers which would need to be updated.

Agreed. There was the discussion about separating 'dma_addr and dma_len' from
scatterlist struct but I don't think that it's worth doing so.


> I'm just proving that it's not as hard as you seem to be making out.

Agreed again.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Russell King - ARM Linux on
On Tue, Jul 20, 2010 at 01:45:17PM -0700, Zach Pfeffer wrote:
> As you mention in your patch the things that can't conflict are memory
> type (strongly- ordered/device/normal), cache policy
> (cacheable/non-cacheable, copy- back/write-through), and coherency
> realm (non-shareable/inner- shareable/outer-shareable). You can
> conflict in allocation preferences (write-allocate/write-no-allocate),
> as those are just "hints".

What you refer to as "hints" I refer to as cache policy - practically
on ARM they're all tied up into the same set of bits.

> You can also conflict in access permissions which can and do conflict
> (which are what multiple mappings are all about...some buffer can get
> some access, while others get different access).

Access permissions don't conflict between mappings - each mapping has
unique access permissions.

> The VCM API allows the same memory to be mapped as long as it makes
> sense and allows those attributes that can change to be specified. It
> could be the alternative, globally applicable approach, your looking
> for and request in your patch.

I very much doubt it - there's virtually no call for creating an
additional mapping of existing kernel memory with different permissions.
The only time kernel memory gets remapped is with vmalloc(), where we
want to create a virtually contiguous mapping from a collection of
(possibly) non-contiguous pages. Such allocations are always created
with R/W permissions.

There are some cases where the vmalloc APIs are used to create mappings
with different memory properties, but as already covered, this has become
illegal with ARMv6 and v7 architectures.

So no, VCM doesn't help because there's nothing that could be solved here.
Creating read-only mappings is pointless, and creating mappings with
different memory type, sharability or cache attributes is illegal.

> Without the VCM API (or something like it) there will just be a bunch
> of duplicated code that's basically doing ioremap. This code will
> probably fail to configure its mappings correctly, in which case your
> patch is a bad idea because it'll spawn bugs all over the place
> instead of at a know location. We could instead change ioremap to
> match the attributes of System RAM if that's what its mapping.

And as I say, what is the point of creating another identical mapping to
the one we already have?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Russell King - ARM Linux on
On Tue, Jul 20, 2010 at 03:02:34PM -0700, stepanm(a)codeaurora.org wrote:
> Russell-
>
> If a driver wants to allow a device to access memory (and cache coherency
> is off/not present for device addesses), the driver needs to remap that
> memory as non-cacheable.

If that memory is not part of the kernel's managed memory, then that's
fine. But if it _is_ part of the kernel's managed memory, then it is
not permitted by the ARM architecture specification to allow maps of
the memory with differing [memory type, sharability, cache] attributes.

Basically, if a driver wants to create these kinds of mappings, then
they should expect the system to become unreliable and unpredictable.
That's not something any sane person should be aiming to do.

> Suppose there exists a chunk of
> physically-contiguous memory (say, memory reserved for device use) that
> happened to be already mapped into the kernel as normal memory (cacheable,
> etc). One way to remap this memory is to use ioremap (and then never touch
> the original virtual mapping, which would now have conflicting
> attributes).

This doesn't work, and is unpredictable on ARMv6 and ARMv7. Not touching
the original mapping is _not_ _sufficient_ to guarantee that the mapping
is not used. (We've seen problems on OMAP as a result of this.)

Any mapping which exists can be speculatively prefetched by such CPUs
at any time, which can lead it to be read into the cache. Then, your
different attributes for your "other" mapping can cause problems if you
hit one of these cache lines - and then you can have (possibly silent)
data corruption.

> I feel as if there should be a better way to remap memory for
> device access, either by altering the attributes on the original mapping,
> or removing the original mapping and creating a new one with attributes
> set to non-cacheable.

This is difficult to achieve without remapping kernel memory using L2
page tables, so we can unmap pages on 4K page granularity. That's
going to increase TLB overhead and result in lower system performance
as there'll be a greater number of MMU misses.

However, one obvious case would be to use highmem-only pages for
remapping - but you then have to ensure that those pages are never
kmapped in any way, because those mappings will fall into the same
unpredictable category that we're already trying to avoid. This
may be possible, but you'll have to ensure that most of the system
RAM is in highmem - which poses other problems (eg, if lowmem gets
low.)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Shilimkar, Santosh on
> -----Original Message-----
> From: linux-arm-kernel-bounces(a)lists.infradead.org [mailto:linux-arm-
> kernel-bounces(a)lists.infradead.org] On Behalf Of Russell King - ARM Linux
> Sent: Wednesday, July 21, 2010 4:00 AM
> To: stepanm(a)codeaurora.org
> Cc: linux-arch(a)vger.kernel.org; dwalker(a)codeaurora.org; mel(a)csn.ul.ie;
> linux-arm-msm(a)vger.kernel.org; linux-kernel(a)vger.kernel.org; FUJITA
> Tomonori; linux-mm(a)kvack.org; andi(a)firstfloor.org; Zach Pfeffer; Michael
> Bohan; Tim HRM; linux-omap(a)vger.kernel.org; linux-arm-
> kernel(a)lists.infradead.org; ebiederm(a)xmission.com
> Subject: Re: [RFC 1/3 v3] mm: iommu: An API to unify IOMMU, CPU and device
> memory management
>
> On Tue, Jul 20, 2010 at 03:02:34PM -0700, stepanm(a)codeaurora.org wrote:
> > Russell-
> >
> > If a driver wants to allow a device to access memory (and cache
> coherency
> > is off/not present for device addesses), the driver needs to remap that
> > memory as non-cacheable.
>
> If that memory is not part of the kernel's managed memory, then that's
> fine. But if it _is_ part of the kernel's managed memory, then it is
> not permitted by the ARM architecture specification to allow maps of
> the memory with differing [memory type, sharability, cache] attributes.
>
> Basically, if a driver wants to create these kinds of mappings, then
> they should expect the system to become unreliable and unpredictable.
> That's not something any sane person should be aiming to do.
>
> > Suppose there exists a chunk of
> > physically-contiguous memory (say, memory reserved for device use) that
> > happened to be already mapped into the kernel as normal memory
> (cacheable,
> > etc). One way to remap this memory is to use ioremap (and then never
> touch
> > the original virtual mapping, which would now have conflicting
> > attributes).
>
> This doesn't work, and is unpredictable on ARMv6 and ARMv7. Not touching
> the original mapping is _not_ _sufficient_ to guarantee that the mapping
> is not used. (We've seen problems on OMAP as a result of this.)
>
> Any mapping which exists can be speculatively prefetched by such CPUs
> at any time, which can lead it to be read into the cache. Then, your
> different attributes for your "other" mapping can cause problems if you
> hit one of these cache lines - and then you can have (possibly silent)
> data corruption.
>
> > I feel as if there should be a better way to remap memory for
> > device access, either by altering the attributes on the original
> mapping,
> > or removing the original mapping and creating a new one with attributes
> > set to non-cacheable.
>
> This is difficult to achieve without remapping kernel memory using L2
> page tables, so we can unmap pages on 4K page granularity. That's
> going to increase TLB overhead and result in lower system performance
> as there'll be a greater number of MMU misses.
>
> However, one obvious case would be to use highmem-only pages for
> remapping - but you then have to ensure that those pages are never
> kmapped in any way, because those mappings will fall into the same
> unpredictable category that we're already trying to avoid. This
> may be possible, but you'll have to ensure that most of the system
> RAM is in highmem - which poses other problems (eg, if lowmem gets
> low.)
>
Why can't we consider an option of removing the old mappings when
we need to create new ones with different attributes as suggested
by Catalin on similar thread previously. This will avoid the duplicate
mapping with different attributes issue on newer ARMs.

Is this something can't be worked out?

Regards,
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/