diff mbox

[v3,5/6] arm64/xen: introduce CONFIG_XEN and hypercall.S on ARM64

Message ID 1370434530-22428-5-git-send-email-stefano.stabellini@eu.citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Stefano Stabellini June 5, 2013, 12:15 p.m. UTC
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.

Changes in v3:
- update comment to reflect the actual hypercall calling convention.
---
 arch/arm64/Kconfig         |   10 +++++
 arch/arm64/Makefile        |    1 +
 arch/arm64/xen/Makefile    |    2 +
 arch/arm64/xen/hypercall.S |   92 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 105 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm64/xen/Makefile
 create mode 100644 arch/arm64/xen/hypercall.S

Comments

Ian Campbell June 5, 2013, 12:23 p.m. UTC | #1
On Wed, 2013-06-05 at 13:15 +0100, Stefano Stabellini wrote:
> 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>

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

> [....]
> +#define HYPERCALL_SIMPLE(hypercall)		\
> +ENTRY(HYPERVISOR_##hypercall)			\
> +	mov x16, #__HYPERVISOR_##hypercall;	\
> +	hvc XEN_IMM;						\
> +	ret;								\

Not sure if this is corruption due to whitespace/email etc but it'd be
nice to align those \'s.

Ian.
Stefano Stabellini June 5, 2013, 12:38 p.m. UTC | #2
On Wed, 5 Jun 2013, Ian Campbell wrote:
> On Wed, 2013-06-05 at 13:15 +0100, Stefano Stabellini wrote:
> > 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>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> > [....]
> > +#define HYPERCALL_SIMPLE(hypercall)		\
> > +ENTRY(HYPERVISOR_##hypercall)			\
> > +	mov x16, #__HYPERVISOR_##hypercall;	\
> > +	hvc XEN_IMM;						\
> > +	ret;								\
> 
> Not sure if this is corruption due to whitespace/email etc but it'd be
> nice to align those \'s.

They are aligned with ts=4, misaligned with ts=8 (ts is vim terminology
for "how many spaces a tab equals to") :-/

Given that according to Linux coding style tabs are 8 characters, I'll
fix the indentation.
Arnd Bergmann June 5, 2013, 12:44 p.m. UTC | #3
On Wednesday 05 June 2013 13:15:29 Stefano Stabellini wrote:
> 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

I think it would be nicer to redirect the entire directory, not just
the enlighten.o and grant-table.o files. You could do in arch/arm64/Makefile:

core-(CONFIG_XEN) += arch/arm/xen/

That leaves a small difference in hypercall.o, which I think you can
handle with an #ifdef.

I believe the reason why KVM does the more elaborate variant is that
they want to be able to build their code as a loadable module that
also includes code from virt/kvm, which you don't need.

	Arnd
Will Deacon June 5, 2013, 12:50 p.m. UTC | #4
On Wed, Jun 05, 2013 at 01:44:55PM +0100, Arnd Bergmann wrote:
> On Wednesday 05 June 2013 13:15:29 Stefano Stabellini wrote:
> > 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
> 
> I think it would be nicer to redirect the entire directory, not just
> the enlighten.o and grant-table.o files. You could do in arch/arm64/Makefile:
> 
> core-(CONFIG_XEN) += arch/arm/xen/
> 
> That leaves a small difference in hypercall.o, which I think you can
> handle with an #ifdef.
> 
> I believe the reason why KVM does the more elaborate variant is that
> they want to be able to build their code as a loadable module that
> also includes code from virt/kvm, which you don't need.

I thought we scrapped the idea of KVM as a loadable module on ARM, mainly
due to the complexities with retrospective initialisation of HYP mode/EL2?

Will
Stefano Stabellini June 5, 2013, 1:04 p.m. UTC | #5
On Wed, 5 Jun 2013, Arnd Bergmann wrote:
> On Wednesday 05 June 2013 13:15:29 Stefano Stabellini wrote:
> > 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
> 
> I think it would be nicer to redirect the entire directory, not just
> the enlighten.o and grant-table.o files. You could do in arch/arm64/Makefile:
> 
> core-(CONFIG_XEN) += arch/arm/xen/
> 
> That leaves a small difference in hypercall.o, which I think you can
> handle with an #ifdef.

That is nicer.

The implementation of hypercall.S is significantly different between
arm32 and arm64, so hypercall.S is going to be harder to read.
However hypercall.S is just one file and it's not going to grow much,
while we might have many more C source files in common between the two
architecures, so I think that your approach is going to be cleaner in
the long term.

I'll make the change.
Christopher Covington June 5, 2013, 2:43 p.m. UTC | #6
Hi Will,

On 06/05/2013 08:50 AM, Will Deacon wrote:
> On Wed, Jun 05, 2013 at 01:44:55PM +0100, Arnd Bergmann wrote:
>> On Wednesday 05 June 2013 13:15:29 Stefano Stabellini wrote:
>>> 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
>>
>> I think it would be nicer to redirect the entire directory, not just
>> the enlighten.o and grant-table.o files. You could do in arch/arm64/Makefile:
>>
>> core-(CONFIG_XEN) += arch/arm/xen/
>>
>> That leaves a small difference in hypercall.o, which I think you can
>> handle with an #ifdef.
>>
>> I believe the reason why KVM does the more elaborate variant is that
>> they want to be able to build their code as a loadable module that
>> also includes code from virt/kvm, which you don't need.
> 
> I thought we scrapped the idea of KVM as a loadable module on ARM, mainly
> due to the complexities with retrospective initialisation of HYP mode/EL2?

What if Hyp/EL2 support were dubbed regular kernel code and the rest of KVM
was made loadable?

Christopher
Catalin Marinas June 6, 2013, 2:42 p.m. UTC | #7
Hi Arnd,

On Wed, Jun 05, 2013 at 01:44:55PM +0100, Arnd Bergmann wrote:
> On Wednesday 05 June 2013 13:15:29 Stefano Stabellini wrote:
> > 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
> 
> I think it would be nicer to redirect the entire directory, not just
> the enlighten.o and grant-table.o files. You could do in arch/arm64/Makefile:
> 
> core-(CONFIG_XEN) += arch/arm/xen/
> 
> That leaves a small difference in hypercall.o, which I think you can
> handle with an #ifdef.

Sorry, I missed this part. I want to keep the AArch64 assembly under
arm64, even if it is small.
Stefano Stabellini June 6, 2013, 4:19 p.m. UTC | #8
On Thu, 6 Jun 2013, Catalin Marinas wrote:
> Hi Arnd,
> 
> On Wed, Jun 05, 2013 at 01:44:55PM +0100, Arnd Bergmann wrote:
> > On Wednesday 05 June 2013 13:15:29 Stefano Stabellini wrote:
> > > 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
> > 
> > I think it would be nicer to redirect the entire directory, not just
> > the enlighten.o and grant-table.o files. You could do in arch/arm64/Makefile:
> > 
> > core-(CONFIG_XEN) += arch/arm/xen/
> > 
> > That leaves a small difference in hypercall.o, which I think you can
> > handle with an #ifdef.
> 
> Sorry, I missed this part. I want to keep the AArch64 assembly under
> arm64, even if it is small.

That's fine, I don't have a strong opinion on this, I am happy either way.

In this case the v3 of the series is the one we want.
I'll submit a pull request to you for it during the next couple of days.
diff mbox

Patch

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..71d1a49
--- /dev/null
+++ b/arch/arm64/xen/hypercall.S
@@ -0,0 +1,92 @@ 
+/******************************************************************************
+ * 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 procedure
+ * call standard for the ARM 64-bit architecture: the first parameter is
+ * passed in x0, the second in x1, the third in x2, the fourth in x3 and
+ * the fifth in x4.
+ *
+ * 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 64-bit 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);