All of lore.kernel.org
 help / color / mirror / Atom feed
From: catalin.marinas@arm.com (Catalin Marinas)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 1/5] arm/xen: define xen_remap as ioremap_cached
Date: Tue, 4 Jun 2013 12:28:34 +0100	[thread overview]
Message-ID: <20130604112834.GA3234@MacBook-Pro.local> (raw)
In-Reply-To: <1370337650.24512.88.camel@zakaz.uk.xensource.com>

On Tue, Jun 04, 2013 at 10:20:50AM +0100, Ian Campbell wrote:
> On Mon, 2013-06-03 at 16:33 +0100, Stefano Stabellini wrote:
> > Define xen_remap as ioremap_cache (MT_MEMORY and MT_DEVICE_CACHED end up
> > having the same AttrIndx encoding).
> 
> The entries in static struct mem_type mem_types[] look entirely
> different to me.  What am I missing?
> 	[MT_DEVICE_CACHED] = {	  /* ioremap_cached */
> 		.prot_pte	= PROT_PTE_DEVICE | L_PTE_MT_DEV_CACHED,
> 		.prot_l1	= PMD_TYPE_TABLE,
> 		.prot_sect	= PROT_SECT_DEVICE | PMD_SECT_WB,
> 		.domain		= DOMAIN_IO,
> 	},
> 	[MT_MEMORY] = {
> 		.prot_pte  = L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY,
> 		.prot_l1   = PMD_TYPE_TABLE,
> 		.prot_sect = PMD_TYPE_SECT | PMD_SECT_AP_WRITE,
> 		.domain    = DOMAIN_KERNEL,
> 	},
> 
> I can see in pgtable-3level.h how L_PTE_MT_DEV_CACHED and
> L_PTE_MT_WRITEBACK are the same but not where the MT_WRITEBACK comes
> from for MT_MEMORY. Things are less clear in pgtable-2level.h, where one
> is 0x3 and the other is 0xb. I can see that the entries are the same in
> armv6_mt_table but how that would apply to a v7 processor?

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).

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).

We have XN in addition for MT_DEVICE_CACHED to prevent speculative
instruction fetches. However, you still get speculative D-cache line
fills since the memory is Normal Cacheable.

> Anyhow, even if I'm prepared to believe that MT_MEMORY and
> MT_DEVICE_CACHED end up being the same thing (which TBH I am) it seems
> that the level of abstraction involved makes us vulnerable to future
> changes subtly breaking things for us. What about:
> 
>         /* Device shared memory */
>         #define ioremap_shm(cookie,size)		__arm_ioremap((cookie), (size), MT_MEMORY)

For my understanding, what is Xen doing with such mapping? I would
rather make ioremap_cached() use MT_MEMORY on ARMv6/v7 (or make it
closer to that, I'm not sure about the implications on ARMv5 and earlier
but for newer architectures I don't see the point in pretending to have
Cacheable Device memory). That's however for Russell to comment.

-- 
Catalin

WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com>
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Will Deacon <Will.Deacon@arm.com>,
	"konrad.wilk@oracle.com" <konrad.wilk@oracle.com>
Subject: Re: [PATCH v2 1/5] arm/xen: define xen_remap as ioremap_cached
Date: Tue, 4 Jun 2013 12:28:34 +0100	[thread overview]
Message-ID: <20130604112834.GA3234@MacBook-Pro.local> (raw)
In-Reply-To: <1370337650.24512.88.camel@zakaz.uk.xensource.com>

On Tue, Jun 04, 2013 at 10:20:50AM +0100, Ian Campbell wrote:
> On Mon, 2013-06-03 at 16:33 +0100, Stefano Stabellini wrote:
> > Define xen_remap as ioremap_cache (MT_MEMORY and MT_DEVICE_CACHED end up
> > having the same AttrIndx encoding).
> 
> The entries in static struct mem_type mem_types[] look entirely
> different to me.  What am I missing?
> 	[MT_DEVICE_CACHED] = {	  /* ioremap_cached */
> 		.prot_pte	= PROT_PTE_DEVICE | L_PTE_MT_DEV_CACHED,
> 		.prot_l1	= PMD_TYPE_TABLE,
> 		.prot_sect	= PROT_SECT_DEVICE | PMD_SECT_WB,
> 		.domain		= DOMAIN_IO,
> 	},
> 	[MT_MEMORY] = {
> 		.prot_pte  = L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY,
> 		.prot_l1   = PMD_TYPE_TABLE,
> 		.prot_sect = PMD_TYPE_SECT | PMD_SECT_AP_WRITE,
> 		.domain    = DOMAIN_KERNEL,
> 	},
> 
> I can see in pgtable-3level.h how L_PTE_MT_DEV_CACHED and
> L_PTE_MT_WRITEBACK are the same but not where the MT_WRITEBACK comes
> from for MT_MEMORY. Things are less clear in pgtable-2level.h, where one
> is 0x3 and the other is 0xb. I can see that the entries are the same in
> armv6_mt_table but how that would apply to a v7 processor?

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).

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).

We have XN in addition for MT_DEVICE_CACHED to prevent speculative
instruction fetches. However, you still get speculative D-cache line
fills since the memory is Normal Cacheable.

> Anyhow, even if I'm prepared to believe that MT_MEMORY and
> MT_DEVICE_CACHED end up being the same thing (which TBH I am) it seems
> that the level of abstraction involved makes us vulnerable to future
> changes subtly breaking things for us. What about:
> 
>         /* Device shared memory */
>         #define ioremap_shm(cookie,size)		__arm_ioremap((cookie), (size), MT_MEMORY)

For my understanding, what is Xen doing with such mapping? I would
rather make ioremap_cached() use MT_MEMORY on ARMv6/v7 (or make it
closer to that, I'm not sure about the implications on ARMv5 and earlier
but for newer architectures I don't see the point in pretending to have
Cacheable Device memory). That's however for Russell to comment.

-- 
Catalin

  reply	other threads:[~2013-06-04 11:28 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-03 15:32 [PATCH v2 0/5] Introduce Xen support to ARM64 Stefano Stabellini
2013-06-03 15:32 ` Stefano Stabellini
2013-06-03 15:32 ` Stefano Stabellini
2013-06-03 15:33 ` [PATCH v2 1/5] arm/xen: define xen_remap as ioremap_cached Stefano Stabellini
2013-06-03 15:33   ` Stefano Stabellini
2013-06-03 15:33   ` Stefano Stabellini
2013-06-04  9:20   ` Ian Campbell
2013-06-04  9:20     ` Ian Campbell
2013-06-04  9:20     ` Ian Campbell
2013-06-04 11:28     ` Catalin Marinas [this message]
2013-06-04 11:28       ` Catalin Marinas
2013-06-04 11:35       ` Stefano Stabellini
2013-06-04 11:35         ` Stefano Stabellini
2013-06-04 13:58       ` Russell King - ARM Linux
2013-06-04 13:58         ` Russell King - ARM Linux
2013-06-04 14:14         ` Stefano Stabellini
2013-06-04 14:14           ` Stefano Stabellini
2013-06-04 16:26         ` Catalin Marinas
2013-06-04 16:26           ` Catalin Marinas
2013-06-04 16:26           ` Catalin Marinas
2013-06-03 15:33 ` [PATCH v2 2/5] arm64/xen: introduce asm/xen header files on arm64 Stefano Stabellini
2013-06-03 15:33   ` Stefano Stabellini
2013-06-03 15:33   ` Stefano Stabellini
2013-06-04  9:24   ` Ian Campbell
2013-06-04  9:24     ` Ian Campbell
2013-06-04  9:24     ` Ian Campbell
2013-06-03 15:33 ` [PATCH v2 3/5] arm64/xen: implement ioremap_cached " Stefano Stabellini
2013-06-03 15:33   ` Stefano Stabellini
2013-06-03 15:33   ` Stefano Stabellini
2013-06-03 15:33 ` [PATCH v2 4/5] arm64/xen: use XEN_IO_PROTO_ABI_ARM on ARM64 Stefano Stabellini
2013-06-03 15:33   ` Stefano Stabellini
2013-06-03 15:33   ` Stefano Stabellini
2013-06-04  9:25   ` Ian Campbell
2013-06-04  9:25     ` Ian Campbell
2013-06-04  9:25     ` Ian Campbell
2013-06-03 15:33 ` [PATCH v2 5/5] arm64/xen: introduce CONFIG_XEN and hypercall.S " Stefano Stabellini
2013-06-03 15:33   ` Stefano Stabellini
2013-06-03 15:33   ` Stefano Stabellini
2013-06-03 16:25   ` Catalin Marinas
2013-06-03 16:25     ` Catalin Marinas
2013-06-03 16:25     ` Catalin Marinas
2013-06-03 16:37     ` [Xen-devel] " Ian Campbell
2013-06-03 16:37       ` Ian Campbell
2013-06-03 16:43       ` Catalin Marinas
2013-06-03 16:43         ` Catalin Marinas
2013-06-03 16:43         ` Catalin Marinas
2013-06-03 16:51     ` Stefano Stabellini
2013-06-03 16:51       ` Stefano Stabellini
2013-06-05 13:21       ` Catalin Marinas
2013-06-05 13:21         ` Catalin Marinas
2013-06-04 14:47     ` Konrad Rzeszutek Wilk
2013-06-04 14:47       ` Konrad Rzeszutek Wilk
2013-06-04 16:27       ` Catalin Marinas
2013-06-04 16:27         ` Catalin Marinas
2013-06-04 16:27         ` Catalin Marinas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130604112834.GA3234@MacBook-Pro.local \
    --to=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.