PIPT cache handling on s5pv210 chip

PIPT cache handling on s5pv210 chip

[Kyungmin Park]

Hi,

In s5pv210 code, there's some strange comments. Does anyone who know
why does it required or more fancy way?

Thank you,
Kyungmin Park


/*
 * flush_cache_vmap() is used when creating mappings (eg, via vmap,
 * vmalloc, ioremap etc) in kernel space for pages.  On non-VIPT
 * caches, since the direct-mappings of these pages may contain cached
 * data, we need to do a full cache flush to ensure that writebacks
 * don't corrupt data placed into these pages via the new mappings.
 */
static inline void flush_cache_vmap(unsigned long start, unsigned long end)
{
#if defined(CONFIG_ARCH_S5PV210)
/*
 * SAMSUNG SoC(S5PV210) has a L2 cache. L2 cache type is
 * VIPT type but L2 is PIPT type cache. If disabling L2 cache, this
 * code works well. But enabling L2 cache, there is data corruption
 * problem. In case of S5PV210, just call flush_cache_all() function
 * like as 2.6.28 kernel.
 */
        flush_cache_all();
#else
        if (!cache_is_vipt_nonaliasing())
                flush_cache_all();
        else
                /*
                 * set_pte_at() called from vmap_pte_range() does not
                 * have a DSB after cleaning the cache line.
                 */
                dsb();
#endif
}

static inline void flush_cache_vunmap(unsigned long start, unsigned long end)
{
#if defined(CONFIG_ARCH_S5PV210)
        flush_cache_all();
#else
        if (!cache_is_vipt_nonaliasing())
                flush_cache_all();
#endif
}

PIPT cache handling on s5pv210 chip

[Russell King - ARM Linux]

On Fri, May 14, 2010 at 12:49:22PM +0900, Kyungmin Park wrote:
> In s5pv210 code, there's some strange comments. Does anyone who know
> why does it required or more fancy way?

The comments look wrong and don't particularly match the code.  I'd
suggest ignoring this patch until someone reports a problem, and
please ensure that such a report is to LAKML.

PIPT cache handling on s5pv210 chip

[Kukjin Kim]

Kyungmin Park wrote:
> 
> Hi,
> 
> In s5pv210 code, there's some strange comments. Does anyone who know
> why does it required or more fancy way?
> 
> Thank you,
> Kyungmin Park
> 
> 
> /*
>  * flush_cache_vmap() is used when creating mappings (eg, via vmap,
>  * vmalloc, ioremap etc) in kernel space for pages.  On non-VIPT
>  * caches, since the direct-mappings of these pages may contain cached
>  * data, we need to do a full cache flush to ensure that writebacks
>  * don't corrupt data placed into these pages via the new mappings.
>  */
> static inline void flush_cache_vmap(unsigned long start, unsigned long end)
> {
> #if defined(CONFIG_ARCH_S5PV210)
> /*
>  * SAMSUNG SoC(S5PV210) has a L2 cache. L2 cache type is
>  * VIPT type but L2 is PIPT type cache. If disabling L2 cache, this
>  * code works well. But enabling L2 cache, there is data corruption
>  * problem. In case of S5PV210, just call flush_cache_all() function
>  * like as 2.6.28 kernel.
>  */
>         flush_cache_all();
> #else
>         if (!cache_is_vipt_nonaliasing())
>                 flush_cache_all();
>         else
>                 /*
>                  * set_pte_at() called from vmap_pte_range() does not
>                  * have a DSB after cleaning the cache line.
>                  */
>                 dsb();
> #endif
> }
> 
> static inline void flush_cache_vunmap(unsigned long start, unsigned long
end)
> {
> #if defined(CONFIG_ARCH_S5PV210)
>         flush_cache_all();
> #else
>         if (!cache_is_vipt_nonaliasing())
>                 flush_cache_all();
> #endif
> }
> 

Hi, 

In case of __vmalloc() with non-cacheable option, there may come some
malfunctions related to cache.
So to avoid this, it seems that the "if (!cache_is_vipt_nonaliasing())" has
been commented out like 28 version.

And the comments of the code looks wrong.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

PIPT cache handling on s5pv210 chip

[Russell King - ARM Linux]

On Fri, May 14, 2010 at 07:36:47PM +0900, Kukjin Kim wrote:
> In case of __vmalloc() with non-cacheable option, there may come some
> malfunctions related to cache.

Practically, you are not allowed on ARMv6 or ARMv7 to create mappings
which alias with differing memory types; that is 100% outlawed by the
architecture spec, and there is NO mitigation for this.

It means that it is _illegal_ to use __vmalloc() with anything but a
memory-like mapping protection.

There is mitigation for having different cacheability, but this is just
a work-around while OSes like Linux work to remove the creation of
aliases with differing cacheability attributes.  There's no guarantee
that later ARMv7 or future CPUs will continue to operate predictably.

The same applies for ioremap() being used on SDRAM - and those who look
at what's in my tree will notice that ioremap() has recently been made
to fail when used on SDRAM for this very reason.

Therefore, the above hack isn't going near mainline.

PIPT cache handling on s5pv210 chip

[Kukjin Kim]

Russell King wrote:
> 
> On Fri, May 14, 2010 at 07:36:47PM +0900, Kukjin Kim wrote:
> > In case of __vmalloc() with non-cacheable option, there may come some
> > malfunctions related to cache.
> 
> Practically, you are not allowed on ARMv6 or ARMv7 to create mappings
> which alias with differing memory types; that is 100% outlawed by the
> architecture spec, and there is NO mitigation for this.
> 
> It means that it is _illegal_ to use __vmalloc() with anything but a
> memory-like mapping protection.
> 
> There is mitigation for having different cacheability, but this is just
> a work-around while OSes like Linux work to remove the creation of
> aliases with differing cacheability attributes.  There's no guarantee
> that later ARMv7 or future CPUs will continue to operate predictably.
> 
> The same applies for ioremap() being used on SDRAM - and those who look
> at what's in my tree will notice that ioremap() has recently been made
> to fail when used on SDRAM for this very reason.

In case a driver starts the Memory_To_Peripheral DMA operation with the
memory area allotted by __vmalloc(,,no-cacheable), the driver can face some
problems caused by cache victims.

> Therefore, the above hack isn't going near mainline.

Nevertheless, I think so too.


Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

PIPT cache handling on s5pv210 chip

[Russell King - ARM Linux]

On Fri, May 14, 2010 at 08:34:14PM +0900, Kukjin Kim wrote:
> In case a driver starts the Memory_To_Peripheral DMA operation with the
> memory area allotted by __vmalloc(,,no-cacheable), the driver can face some
> problems caused by cache victims.

That's its own lookout - all the time that there's the kernel mapping
present, the cache can be loaded with data via that mapping - and being
PIPT, that means the data _could_ be hit by accesses via the __vmalloc
mapping.

As I've already explained, __vmalloc() with differing memory types on
ARMv6 and above is unpredictable, period.  The same is true for different
sharability settings for the same physical memory.  No amount of
discussing will change this; it's a fact of the hardware.  If you
create them, your system will have unpredictable behaviour.

With different cache attributes, it's unpredictable to the word of the
architecture spec, and eventually it will become unpredictable according
to the hardware as well.

So the message is: using __vmalloc() with _any_ differing attributes
from the main memory mapping is unpredictable and must not be used on
ARM.

PIPT cache handling on s5pv210 chip

[Ben Dooks]

On Fri, May 14, 2010 at 08:34:14PM +0900, Kukjin Kim wrote:
> Russell King wrote:
> > 
> > On Fri, May 14, 2010 at 07:36:47PM +0900, Kukjin Kim wrote:
> > > In case of __vmalloc() with non-cacheable option, there may come some
> > > malfunctions related to cache.
> > 
> > Practically, you are not allowed on ARMv6 or ARMv7 to create mappings
> > which alias with differing memory types; that is 100% outlawed by the
> > architecture spec, and there is NO mitigation for this.

Unfortunately, just because something is outlawed doesn't mean that people
will try and do it.
 
> > It means that it is _illegal_ to use __vmalloc() with anything but a
> > memory-like mapping protection.
> > 
> > There is mitigation for having different cacheability, but this is just
> > a work-around while OSes like Linux work to remove the creation of
> > aliases with differing cacheability attributes.  There's no guarantee
> > that later ARMv7 or future CPUs will continue to operate predictably.
> > 
> > The same applies for ioremap() being used on SDRAM - and those who look
> > at what's in my tree will notice that ioremap() has recently been made
> > to fail when used on SDRAM for this very reason.
> 
> In case a driver starts the Memory_To_Peripheral DMA operation with the
> memory area allotted by __vmalloc(,,no-cacheable), the driver can face some
> problems caused by cache victims.

This would seem to indicate either the driver needs to allocate the
memory differently, or that it is not following the correct procedure
to hande the memory area to the hardware or that there it is a problem
with how the L2 cache has been integrated into the kernel.

I'm not sure what kernel this is based on, or what drivers are being
affected by this as I suspect this is from one of the for-vendor branches
that tends to pop-up because something needs to be shipped ASAP.
 
> > Therefore, the above hack isn't going near mainline.
> 
> Nevertheless, I think so too.

This is the kind of thing that makes me want to go and find the
perpitrators and uise the ogg-cavetech-super-pointy-stick on them.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.