[PATCH v2 0/5] Introduce Xen support to ARM64

[PATCH v2 0/5] Introduce Xen support to ARM64

[Stefano Stabellini]

Hi all,
this patch series introduces Xen support to arch/arm64.

As you can see from the following patches, there is very little arm64
specific code here, basically only one assembly file
(arch/arm64/xen/hypercall.S).

Everything else is common with Xen for ARMv7, in particular the code
under arch/arm/xen (enlighten.c and the stubs in grant-table.c) and most
of the header files under arch/arm/include/asm/xen (hypercall.h,
hypervisor.h, interface.h, page.h) can be reused as-is.


Stefano Stabellini (5):
      arm/xen: define xen_remap as ioremap_cached
      arm64/xen: introduce asm/xen header files on arm64
      arm64/xen: implement ioremap_cached on arm64
      arm64/xen: use XEN_IO_PROTO_ABI_ARM on ARM64
      arm64/xen: introduce CONFIG_XEN and hypercall.S on ARM64

 arch/arm/include/asm/xen/page.h         |    3 +-
 arch/arm64/Kconfig                      |   10 +++
 arch/arm64/Makefile                     |    1 +
 arch/arm64/include/asm/hypervisor.h     |    6 ++
 arch/arm64/include/asm/io.h             |    2 +
 arch/arm64/include/asm/sync_bitops.h    |   26 +++++++++
 arch/arm64/include/asm/xen/events.h     |   21 +++++++
 arch/arm64/include/asm/xen/hypercall.h  |    1 +
 arch/arm64/include/asm/xen/hypervisor.h |    1 +
 arch/arm64/include/asm/xen/interface.h  |    1 +
 arch/arm64/include/asm/xen/page.h       |    1 +
 arch/arm64/xen/Makefile                 |    2 +
 arch/arm64/xen/hypercall.S              |   94 +++++++++++++++++++++++++++++++
 include/xen/interface/io/protocols.h    |    2 +-
 14 files changed, 168 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm64/include/asm/hypervisor.h
 create mode 100644 arch/arm64/include/asm/sync_bitops.h
 create mode 100644 arch/arm64/include/asm/xen/events.h
 create mode 100644 arch/arm64/include/asm/xen/hypercall.h
 create mode 100644 arch/arm64/include/asm/xen/hypervisor.h
 create mode 100644 arch/arm64/include/asm/xen/interface.h
 create mode 100644 arch/arm64/include/asm/xen/page.h
 create mode 100644 arch/arm64/xen/Makefile
 create mode 100644 arch/arm64/xen/hypercall.S


git://git.kernel.org/pub/scm/linux/kernel/git/sstabellini/xen.git xen-arm64-2

- Stefano

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

[Stefano Stabellini]

Define xen_remap as ioremap_cache (MT_MEMORY and MT_DEVICE_CACHED end up
having the same AttrIndx encoding).

Remove include asm/mach/map.h, not unneeded.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/arm/include/asm/xen/page.h |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
index 30cdacb..359a7b5 100644
--- a/arch/arm/include/asm/xen/page.h
+++ b/arch/arm/include/asm/xen/page.h
@@ -1,7 +1,6 @@
 #ifndef _ASM_ARM_XEN_PAGE_H
 #define _ASM_ARM_XEN_PAGE_H
 
-#include <asm/mach/map.h>
 #include <asm/page.h>
 #include <asm/pgtable.h>
 
@@ -88,6 +87,6 @@ static inline bool set_phys_to_machine(unsigned long pfn, unsigned long mfn)
 	return __set_phys_to_machine(pfn, mfn);
 }
 
-#define xen_remap(cookie, size) __arm_ioremap((cookie), (size), MT_MEMORY);
+#define xen_remap(cookie, size) ioremap_cached((cookie), (size));
 
 #endif /* _ASM_ARM_XEN_PAGE_H */
-- 
1.7.2.5

[PATCH v2 2/5] arm64/xen: introduce asm/xen header files on arm64

[Stefano Stabellini]

asm/xen/hypercall.h, asm/xen/hypervisor.h, asm/xen/interface.h and
asm/xen/page.h are indentical so to avoid code duplication we are just
including the original arm header files here.

asm/xen/events.h is slightly different, so introduce a different file
here (use xchg to implement xchg_xen_ulong and pass regs->pstate to
raw_irqs_disabled_flags).

Also introduce asm/hypervisor.h and asm/sync_bitops.h.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Change in v2:
- add all the required headers in this patch, by including the original
  arch/arm headers when possible to avoid code duplication;
- use xchg to implement xchg_xen_ulong.
---
 arch/arm64/include/asm/hypervisor.h     |    6 ++++++
 arch/arm64/include/asm/sync_bitops.h    |   26 ++++++++++++++++++++++++++
 arch/arm64/include/asm/xen/events.h     |   21 +++++++++++++++++++++
 arch/arm64/include/asm/xen/hypercall.h  |    1 +
 arch/arm64/include/asm/xen/hypervisor.h |    1 +
 arch/arm64/include/asm/xen/interface.h  |    1 +
 arch/arm64/include/asm/xen/page.h       |    1 +
 7 files changed, 57 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm64/include/asm/hypervisor.h
 create mode 100644 arch/arm64/include/asm/sync_bitops.h
 create mode 100644 arch/arm64/include/asm/xen/events.h
 create mode 100644 arch/arm64/include/asm/xen/hypercall.h
 create mode 100644 arch/arm64/include/asm/xen/hypervisor.h
 create mode 100644 arch/arm64/include/asm/xen/interface.h
 create mode 100644 arch/arm64/include/asm/xen/page.h

diff --git a/arch/arm64/include/asm/hypervisor.h b/arch/arm64/include/asm/hypervisor.h
new file mode 100644
index 0000000..d2c7904
--- /dev/null
+++ b/arch/arm64/include/asm/hypervisor.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_ARM64_HYPERVISOR_H
+#define _ASM_ARM64_HYPERVISOR_H
+
+#include <asm/xen/hypervisor.h>
+
+#endif
diff --git a/arch/arm64/include/asm/sync_bitops.h b/arch/arm64/include/asm/sync_bitops.h
new file mode 100644
index 0000000..8da0bf4
--- /dev/null
+++ b/arch/arm64/include/asm/sync_bitops.h
@@ -0,0 +1,26 @@
+#ifndef __ASM_SYNC_BITOPS_H__
+#define __ASM_SYNC_BITOPS_H__
+
+#include <asm/bitops.h>
+#include <asm/cmpxchg.h>
+
+/* sync_bitops functions are equivalent to the SMP implementation of the
+ * original functions, independently from CONFIG_SMP being defined.
+ *
+ * We need them because _set_bit etc are not SMP safe if !CONFIG_SMP. But
+ * under Xen you might be communicating with a completely external entity
+ * who might be on another CPU (e.g. two uniprocessor guests communicating
+ * via event channels and grant tables). So we need a variant of the bit
+ * ops which are SMP safe even on a UP kernel.
+ */
+
+#define sync_set_bit(nr, p)            set_bit(nr, p)
+#define sync_clear_bit(nr, p)          clear_bit(nr, p)
+#define sync_change_bit(nr, p)         change_bit(nr, p)
+#define sync_test_and_set_bit(nr, p)   test_and_set_bit(nr, p)
+#define sync_test_and_clear_bit(nr, p) test_and_clear_bit(nr, p)
+#define sync_test_and_change_bit(nr, p)        test_and_change_bit(nr, p)
+#define sync_test_bit(nr, addr)                test_bit(nr, addr)
+#define sync_cmpxchg                   cmpxchg
+
+#endif
diff --git a/arch/arm64/include/asm/xen/events.h b/arch/arm64/include/asm/xen/events.h
new file mode 100644
index 0000000..8655321
--- /dev/null
+++ b/arch/arm64/include/asm/xen/events.h
@@ -0,0 +1,21 @@
+#ifndef _ASM_ARM64_XEN_EVENTS_H
+#define _ASM_ARM64_XEN_EVENTS_H
+
+#include <asm/ptrace.h>
+#include <asm/atomic.h>
+
+enum ipi_vector {
+	XEN_PLACEHOLDER_VECTOR,
+
+	/* Xen IPIs go here */
+	XEN_NR_IPIS,
+};
+
+static inline int xen_irqs_disabled(struct pt_regs *regs)
+{
+	return raw_irqs_disabled_flags((unsigned long) regs->pstate);
+}
+
+#define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
+
+#endif /* _ASM_ARM64_XEN_EVENTS_H */
diff --git a/arch/arm64/include/asm/xen/hypercall.h b/arch/arm64/include/asm/xen/hypercall.h
new file mode 100644
index 0000000..74b0c42
--- /dev/null
+++ b/arch/arm64/include/asm/xen/hypercall.h
@@ -0,0 +1 @@
+#include <../../arm/include/asm/xen/hypercall.h>
diff --git a/arch/arm64/include/asm/xen/hypervisor.h b/arch/arm64/include/asm/xen/hypervisor.h
new file mode 100644
index 0000000..f263da8
--- /dev/null
+++ b/arch/arm64/include/asm/xen/hypervisor.h
@@ -0,0 +1 @@
+#include <../../arm/include/asm/xen/hypervisor.h>
diff --git a/arch/arm64/include/asm/xen/interface.h b/arch/arm64/include/asm/xen/interface.h
new file mode 100644
index 0000000..44457ae
--- /dev/null
+++ b/arch/arm64/include/asm/xen/interface.h
@@ -0,0 +1 @@
+#include <../../arm/include/asm/xen/interface.h>
diff --git a/arch/arm64/include/asm/xen/page.h b/arch/arm64/include/asm/xen/page.h
new file mode 100644
index 0000000..bed87ec
--- /dev/null
+++ b/arch/arm64/include/asm/xen/page.h
@@ -0,0 +1 @@
+#include <../../arm/include/asm/xen/page.h>
-- 
1.7.2.5

[PATCH v2 3/5] arm64/xen: implement ioremap_cached on arm64

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/arm64/include/asm/io.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index 2e12258..1d12f89 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -228,10 +228,12 @@ extern void __iounmap(volatile void __iomem *addr);
 #define PROT_DEFAULT		(PTE_TYPE_PAGE | PTE_AF | PTE_DIRTY)
 #define PROT_DEVICE_nGnRE	(PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_DEVICE_nGnRE))
 #define PROT_NORMAL_NC		(PROT_DEFAULT | PTE_ATTRINDX(MT_NORMAL_NC))
+#define PROT_NORMAL		(PROT_DEFAULT | PTE_ATTRINDX(MT_NORMAL))
 
 #define ioremap(addr, size)		__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
 #define ioremap_nocache(addr, size)	__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
 #define ioremap_wc(addr, size)		__ioremap((addr), (size), __pgprot(PROT_NORMAL_NC))
+#define ioremap_cached(addr, size)	__ioremap((addr), (size), __pgprot(PROT_NORMAL))
 #define iounmap				__iounmap
 
 #define PROT_SECT_DEFAULT	(PMD_TYPE_SECT | PMD_SECT_AF)
-- 
1.7.2.5

[PATCH v2 4/5] arm64/xen: use XEN_IO_PROTO_ABI_ARM on ARM64

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 include/xen/interface/io/protocols.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/xen/interface/io/protocols.h b/include/xen/interface/io/protocols.h
index 0eafaf2..056744b 100644
--- a/include/xen/interface/io/protocols.h
+++ b/include/xen/interface/io/protocols.h
@@ -15,7 +15,7 @@
 # define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_IA64
 #elif defined(__powerpc64__)
 # define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_POWERPC64
-#elif defined(__arm__)
+#elif defined(__arm__) || defined(__aarch64__)
 # define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_ARM
 #else
 # error arch fixup needed here
-- 
1.7.2.5

[PATCH v2 5/5] arm64/xen: introduce CONFIG_XEN and hypercall.S on ARM64

[Stefano Stabellini]

Introduce CONFIG_XEN and the implementation of hypercall.S (that is
the only ARMv8 specific code in Xen support for ARM).

Compile enlighten.c and grant_table.c from arch/arm.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Changes in v2:
- remove depends on !GENERIC_ATOMIC64;
- compile enlighten.c and grant_table.c from arch/arm directly;
- fix the privcmd implementation: according to the aarch64 procedure
call ABI the first 7 parameters are passed on registers so we don't need
to push/pop anything.
---
 arch/arm64/Kconfig         |   10 +++++
 arch/arm64/Makefile        |    1 +
 arch/arm64/xen/Makefile    |    2 +
 arch/arm64/xen/hypercall.S |   94 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 107 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm64/xen/Makefile
 create mode 100644 arch/arm64/xen/hypercall.S

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 56b3f6d..b5d6ada 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -182,6 +182,16 @@ config HW_PERF_EVENTS
 
 source "mm/Kconfig"
 
+config XEN_DOM0
+	def_bool y
+	depends on XEN
+
+config XEN
+	bool "Xen guest support on ARM64 (EXPERIMENTAL)"
+	depends on ARM64 && OF
+	help
+	  Say Y if you want to run Linux in a Virtual Machine on Xen on ARM64.
+
 endmenu
 
 menu "Boot options"
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index c95c5cb..79dd13d 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -37,6 +37,7 @@ TEXT_OFFSET := 0x00080000
 export	TEXT_OFFSET GZFLAGS
 
 core-y		+= arch/arm64/kernel/ arch/arm64/mm/
+core-$(CONFIG_XEN)		+= arch/arm64/xen/
 libs-y		:= arch/arm64/lib/ $(libs-y)
 libs-y		+= $(LIBGCC)
 
diff --git a/arch/arm64/xen/Makefile b/arch/arm64/xen/Makefile
new file mode 100644
index 0000000..be24040
--- /dev/null
+++ b/arch/arm64/xen/Makefile
@@ -0,0 +1,2 @@
+xen-arm-y	+= $(addprefix ../../arm/xen/, enlighten.o grant-table.o)
+obj-y		:= xen-arm.o hypercall.o
diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
new file mode 100644
index 0000000..8e362cc
--- /dev/null
+++ b/arch/arm64/xen/hypercall.S
@@ -0,0 +1,94 @@
+/******************************************************************************
+ * hypercall.S
+ *
+ * Xen hypercall wrappers
+ *
+ * Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Citrix, 2012
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/*
+ * The Xen hypercall calling convention is very similar to the ARM AEBI
+ * procedure calling convention: the first paramter is passed in x0, the
+ * second in x1, the third in x2 and the fourth in x3. Considering that
+ * Xen hypercalls have 5 arguments at most, the fifth paramter is passed
+ * in rx, differently from the procedure calling convention of using the
+ * stack for that case.
+ *
+ * The hypercall number is passed in x16.
+ *
+ * The return value is in x0.
+ *
+ * The hvc ISS is required to be 0xEA1, that is the Xen specific ARM
+ * hypercall tag.
+ *
+ * Parameter structs passed to hypercalls are laid out according to
+ * the ARM EABI standard.
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+#include <xen/interface/xen.h>
+
+
+#define XEN_IMM 0xEA1
+
+#define HYPERCALL_SIMPLE(hypercall)		\
+ENTRY(HYPERVISOR_##hypercall)			\
+	mov x16, #__HYPERVISOR_##hypercall;	\
+	hvc XEN_IMM;						\
+	ret;								\
+ENDPROC(HYPERVISOR_##hypercall)
+
+#define HYPERCALL0 HYPERCALL_SIMPLE
+#define HYPERCALL1 HYPERCALL_SIMPLE
+#define HYPERCALL2 HYPERCALL_SIMPLE
+#define HYPERCALL3 HYPERCALL_SIMPLE
+#define HYPERCALL4 HYPERCALL_SIMPLE
+#define HYPERCALL5 HYPERCALL_SIMPLE
+
+                .text
+
+HYPERCALL2(xen_version);
+HYPERCALL3(console_io);
+HYPERCALL3(grant_table_op);
+HYPERCALL2(sched_op);
+HYPERCALL2(event_channel_op);
+HYPERCALL2(hvm_op);
+HYPERCALL2(memory_op);
+HYPERCALL2(physdev_op);
+HYPERCALL3(vcpu_op);
+
+ENTRY(privcmd_call)
+	mov x16, x0
+	mov x0, x1
+	mov x1, x2
+	mov x2, x3
+	mov x3, x4
+	mov x4, x5
+	hvc XEN_IMM
+	ret
+ENDPROC(privcmd_call);
-- 
1.7.2.5

Re: [PATCH v2 5/5] arm64/xen: introduce CONFIG_XEN and hypercall.S on ARM64

[Catalin Marinas]

On Mon, Jun 03, 2013 at 04:33:44PM +0100, Stefano Stabellini wrote:
> --- /dev/null
> +++ b/arch/arm64/xen/hypercall.S
...
> +/*
> + * The Xen hypercall calling convention is very similar to the ARM AEBI
> + * procedure calling convention: the first paramter is passed in x0, the
> + * second in x1, the third in x2 and the fourth in x3. Considering that
> + * Xen hypercalls have 5 arguments at most, the fifth paramter is passed
> + * in rx, differently from the procedure calling convention of using the
> + * stack for that case.

You may want to use the AArch64 ABI here where parameters are passed in
x0-x7, return in x0.

BTW, I think it's missing a MAINTAINERS file update.

Otherwise, the series looks good. How do you plan to upstream this?

-- 
Catalin

Re: [Xen-devel] [PATCH v2 5/5] arm64/xen: introduce CONFIG_XEN and hypercall.S on ARM64

[Ian Campbell]

On Mon, 2013-06-03 at 17:25 +0100, Catalin Marinas wrote:
> On Mon, Jun 03, 2013 at 04:33:44PM +0100, Stefano Stabellini wrote:
> > --- /dev/null
> > +++ b/arch/arm64/xen/hypercall.S
> ...
> > +/*
> > + * The Xen hypercall calling convention is very similar to the ARM AEBI
> > + * procedure calling convention: the first paramter is passed in x0, the
> > + * second in x1, the third in x2 and the fourth in x3. Considering that
> > + * Xen hypercalls have 5 arguments at most, the fifth paramter is passed

                                                          parameter

> > + * in rx, differently from the procedure calling convention of using the

          ^x4 not rx

> > + * stack for that case.
> 
> You may want to use the AArch64 ABI here where parameters are passed in
> x0-x7, return in x0.

I think that is actually what we are doing (up to 5 arguments), the
"Considering that ..." bit at the end is a left over of the 32 bit
version I think.

> 
> BTW, I think it's missing a MAINTAINERS file update.
> 
> Otherwise, the series looks good. How do you plan to upstream this?
> 

Re: [Xen-devel] [PATCH v2 5/5] arm64/xen: introduce CONFIG_XEN and hypercall.S on ARM64

[Catalin Marinas]

On Mon, Jun 03, 2013 at 05:37:35PM +0100, Ian Campbell wrote:
> On Mon, 2013-06-03 at 17:25 +0100, Catalin Marinas wrote:
> > On Mon, Jun 03, 2013 at 04:33:44PM +0100, Stefano Stabellini wrote:
> > > --- /dev/null
> > > +++ b/arch/arm64/xen/hypercall.S
> > ...
> > > +/*
> > > + * The Xen hypercall calling convention is very similar to the ARM AEBI
> > > + * procedure calling convention: the first paramter is passed in x0, the
> > > + * second in x1, the third in x2 and the fourth in x3. Considering that
> > > + * Xen hypercalls have 5 arguments at most, the fifth paramter is passed
> 
>                                                           parameter
> 
> > > + * in rx, differently from the procedure calling convention of using the
> 
>           ^x4 not rx
> 
> > > + * stack for that case.
> > 
> > You may want to use the AArch64 ABI here where parameters are passed in
> > x0-x7, return in x0.
> 
> I think that is actually what we are doing (up to 5 arguments), the
> "Considering that ..." bit at the end is a left over of the 32 bit
> version I think.

Yes, only the comment needs updating.

-- 
Catalin

Re: [PATCH v2 5/5] arm64/xen: introduce CONFIG_XEN and hypercall.S on ARM64

[Stefano Stabellini]

On Mon, 3 Jun 2013, Catalin Marinas wrote:
> On Mon, Jun 03, 2013 at 04:33:44PM +0100, Stefano Stabellini wrote:
> > --- /dev/null
> > +++ b/arch/arm64/xen/hypercall.S
> ...
> > +/*
> > + * The Xen hypercall calling convention is very similar to the ARM AEBI
> > + * procedure calling convention: the first paramter is passed in x0, the
> > + * second in x1, the third in x2 and the fourth in x3. Considering that
> > + * Xen hypercalls have 5 arguments at most, the fifth paramter is passed
> > + * in rx, differently from the procedure calling convention of using the
> > + * stack for that case.
> 
> You may want to use the AArch64 ABI here where parameters are passed in
> x0-x7, return in x0.

We already do this, we just need to update the comment.


> BTW, I think it's missing a MAINTAINERS file update.

Yep, I'll make those changes.


> Otherwise, the series looks good. How do you plan to upstream this?

Good question.
Traditionally Xen stuff goes to Linus directly via one of the Xen trees
(Konrad's or mine), but since this is the very first Xen patch series
for arm64 if you prefer I can submit a pull request to you instead.
Just let me know, I am happy either way.

In any case, are you OK with sending it upstream during the next merge
window?

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

[Ian Campbell]

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?

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)

Ian.

> Remove include asm/mach/map.h, not unneeded.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  arch/arm/include/asm/xen/page.h |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
> index 30cdacb..359a7b5 100644
> --- a/arch/arm/include/asm/xen/page.h
> +++ b/arch/arm/include/asm/xen/page.h
> @@ -1,7 +1,6 @@
>  #ifndef _ASM_ARM_XEN_PAGE_H
>  #define _ASM_ARM_XEN_PAGE_H
>  
> -#include <asm/mach/map.h>
>  #include <asm/page.h>
>  #include <asm/pgtable.h>
>  
> @@ -88,6 +87,6 @@ static inline bool set_phys_to_machine(unsigned long pfn, unsigned long mfn)
>  	return __set_phys_to_machine(pfn, mfn);
>  }
>  
> -#define xen_remap(cookie, size) __arm_ioremap((cookie), (size), MT_MEMORY);
> +#define xen_remap(cookie, size) ioremap_cached((cookie), (size));
>  
>  #endif /* _ASM_ARM_XEN_PAGE_H */

Re: [PATCH v2 2/5] arm64/xen: introduce asm/xen header files on arm64

[Ian Campbell]

On Mon, 2013-06-03 at 16:33 +0100, Stefano Stabellini wrote:
> asm/xen/hypercall.h, asm/xen/hypervisor.h, asm/xen/interface.h and
> asm/xen/page.h are indentical so to avoid code duplication we are just

"identical"

> including the original arm header files here.
> 
> asm/xen/events.h is slightly different, so introduce a different file
> here (use xchg to implement xchg_xen_ulong and pass regs->pstate to
> raw_irqs_disabled_flags).
> 
> Also introduce asm/hypervisor.h and asm/sync_bitops.h.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[...]
> +/* sync_bitops functions are equivalent to the SMP implementation of the
> + * original functions, independently from CONFIG_SMP being defined.
> + *
> + * We need them because _set_bit etc are not SMP safe if !CONFIG_SMP. But
> + * under Xen you might be communicating with a completely external entity
> + * who might be on another CPU (e.g. two uniprocessor guests communicating
> + * via event channels and grant tables). So we need a variant of the bit
> + * ops which are SMP safe even on a UP kernel.
> + */
> +
> +#define sync_set_bit(nr, p)            set_bit(nr, p)

Oh, arm64 seems to have optimised bitops now, which means I can ditch
the skanky versions in Xen in favour of these. Nice.

Ian.

Re: [PATCH v2 4/5] arm64/xen: use XEN_IO_PROTO_ABI_ARM on ARM64

[Ian Campbell]

On Mon, 2013-06-03 at 16:33 +0100, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  include/xen/interface/io/protocols.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/xen/interface/io/protocols.h b/include/xen/interface/io/protocols.h
> index 0eafaf2..056744b 100644
> --- a/include/xen/interface/io/protocols.h
> +++ b/include/xen/interface/io/protocols.h
> @@ -15,7 +15,7 @@
>  # define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_IA64
>  #elif defined(__powerpc64__)
>  # define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_POWERPC64
> -#elif defined(__arm__)
> +#elif defined(__arm__) || defined(__aarch64__)
>  # define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_ARM
>  #else
>  # error arch fixup needed here

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

[Catalin Marinas]

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

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

[Stefano Stabellini]

On Tue, 4 Jun 2013, Catalin Marinas wrote:
> 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.

Xen guests share these pages with one another and place a lockless ring
buffer on it for bidirectional communication. So the page that is being
ioremapped actually belongs to another guest and it's RAM.

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

[Russell King - ARM Linux]

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.

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

[Stefano Stabellini]

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.

Re: [PATCH v2 5/5] arm64/xen: introduce CONFIG_XEN and hypercall.S on ARM64

[Konrad Rzeszutek Wilk]

> BTW, I think it's missing a MAINTAINERS file update.

I presume you mean that Stefano should have the MAINTAINERS file updated to include
him being the maintainer of arch/arm64/xen* ?

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

[Catalin Marinas]

On Tue, Jun 04, 2013 at 02:58:57PM +0100, 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.

It may have been a hard requirement in the early days (FPGA?), I don't
fully remember. The ARM11MPCore TRM for r1p0 states "Inner Write-Back No
Allocate on Write behaves as Write-back Write-Allocate".

Anyway, my comment above was for leaving it on all the time (v6 and v7).

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

Yes, that's what I meant.

> However, that mapping type (which is used for MT_DEVICE_CACHED) should
> not be used if you really do want normal memory. 

Well, you want some range mapped as cacheable, so on ARM
MT_DEVICE_CACHED gives you normal memory. I'm not sure in the Xen
context where this memory comes from, it looks like some physical
address not under kernel control. There are other ways to map this but
ioremap_cached() seems right for this situation (x86 has a similar
definition for xen_remap).

BTW, it looks like 3 architectures call this 'ioremap_cached' while
other 3 (or 4) call it 'ioremap_cache'. Not a standard API.

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

That's a fair point.

-- 
Catalin

Re: [PATCH v2 5/5] arm64/xen: introduce CONFIG_XEN and hypercall.S on ARM64

[Catalin Marinas]

On Tue, Jun 04, 2013 at 03:47:41PM +0100, Konrad Rzeszutek Wilk wrote:
> > BTW, I think it's missing a MAINTAINERS file update.
> 
> I presume you mean that Stefano should have the MAINTAINERS file updated to include
> him being the maintainer of arch/arm64/xen* ?

Yes (or whoever will maintain Xen for arm64).

-- 
Catalin

Re: [PATCH v2 5/5] arm64/xen: introduce CONFIG_XEN and hypercall.S on ARM64

[Catalin Marinas]

On Mon, Jun 03, 2013 at 05:51:53PM +0100, Stefano Stabellini wrote:
> On Mon, 3 Jun 2013, Catalin Marinas wrote:
> > Otherwise, the series looks good. How do you plan to upstream this?
> 
> Good question.
> Traditionally Xen stuff goes to Linus directly via one of the Xen trees
> (Konrad's or mine), but since this is the very first Xen patch series
> for arm64 if you prefer I can submit a pull request to you instead.
> Just let me know, I am happy either way.

I can take the first series and subsequent merges can go via the Xen
tree (KVM has a similar approach).

> In any case, are you OK with sending it upstream during the next merge
> window?

Yes, if I get a pull request in time ;)

-- 
Catalin