Re: [PATCH v2 1/5] arm/xen: define xen_remap as ioremap_cached

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Tue, 4 Jun 2013, Russell King - ARM Linux wrote:
> On Tue, Jun 04, 2013 at 12:28:34PM +0100, Catalin Marinas wrote:
> > PROT_PTE_DEVICE and PROT_SECT_DEVICE above don't contain any memory type
> > information, just attributes/permission - present, young, dirty and XN:
> > 
> > #define PROT_PTE_DEVICE		L_PTE_PRESENT|L_PTE_YOUNG|L_PTE_DIRTY|L_PTE_XN
> > #define PROT_SECT_DEVICE	PMD_TYPE_SECT|PMD_SECT_AP_WRITE
> > 
> > The memory type is given by the L_PTE_MT_DEV_CACHED and PMD_SECT_WB
> > macros. Let's take prot_sect first as it's simpler. For MT_DEVICE_CACHED
> > we have:
> > 
> > .prot_sect = PMD_TYPE_SECT | PMD_SECT_AP_WRITE | PMD_SECT_WB
> > 
> > For MT_MEMORY we have:
> > 
> > .prot_sect = PMD_TYPE_SECT | PMD_SECT_AP_WRITE
> > 
> > The cache policy is added later to MT_MEMORY which is either WB or WBWA
> > (based on SMP, no particular reason as these are just processor hints;
> > for some historical reasons we enabled WBWA for ARM11MPCore but we could
> > leave it on all the time).
> 
> They may be reported to be just hints, but SMP, particularly ARM11MPCore,
> the SMP guys at ARM Ltd were very particular about _requiring_ and stating
> that it is required that WBWA must be used in the page tables for SMP,
> and not WB.  That suggests while the ARM ARM may say that they're hints,
> there's slightly more to it when it comes to SMP, and WBWA is a hard
> requirement there.
> 
> Indeed, that's something we enforce in the kernel because of the statements
> from the SMP development group.
> 
> > Similarly for prot_pte, present, young, dirty are the same.
> > 
> > Regarding the type, on ARMv7 (with or without LPAE) we use TEX remapping
> > and L_PTE_MT_DEVICE has the same index (3-bit TEX[0], C, B for NMRR/PRRR
> > or TEX[2:0] for MAIR0/MAIR1 registers) as Normal Cacheable Writeback
> > memory (there is no such thing as Device memory with cacheability
> > attributes, only Normal Cacheable memory).
> 
> You don't mean L_PTE_MT_DEVICE there. Thankfully, L_PTE_MT_DEVICE doesn't
> exist, so it's not that confusing.  What you mean is L_PTE_MT_DEV_CACHED,
> which _does_ map to "normal cacheable writeback memory" irrespective of
> the mappings which the kernel uses for RAM.
> 
> However, that mapping type (which is used for MT_DEVICE_CACHED) should
> not be used if you really do want normal memory.  And using MT_* definitions
> _not_ in asm/io.h with ioremap() is really... silly.  It's not something
> that is intended, nor is it something which I intend to be supportable
> into the future on ARM.  That's why the definitions are separate - see
> the comments in asm/io.h about "types 4 onwards are undefined for
> ioremap" - I'm not sure how more explicit to make that statement.  And
> as I _have_ made the statement, if I see people violating it, I won't
> care about their code if/when I decide to change the ioremap() behaviour.

Fair enough. In that case what do you suggest we should do? 
Given that ioremap_cached is already taken for "device cacheable memory", maybe we
could introduce ioremap_memory for "normal memory" on ARM and ARM64:

#define ioremap_memory(cookie,size) __arm_ioremap((cookie), (size), MT_MEMORY)

that would require us moving the MT_MEMORY definition to
arch/arm/include/asm/io.h though.

I am open to any suggestions.