[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCHv5 18/46] plat/kvm: Add Arm64 basic entry code



On 10.09.2018 10:57, Wei Chen (Arm Technology China) wrote:

Hi Julien,


-----Original Message-----
From: Julien Grall <julien.grall@xxxxxxx>
Sent: 2018年9月7日 22:58
To: Wei Chen (Arm Technology China) <Wei.Chen@xxxxxxx>; minios-
devel@xxxxxxxxxxxxxxxxxxxx; simon.kuenzer@xxxxxxxxx
Cc: Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
Subject: Re: [Minios-devel] [UNIKRAFT PATCHv5 18/46] plat/kvm: Add Arm64 basic
entry code

Hi,

On 08/10/2018 08:08 AM, Wei Chen wrote:

From: Wei Chen <Wei.Chen@xxxxxxx>

QEMU/KVM can boot an Arm64 elf image without multiboot. In this case,
we can plage _libkvmplat_entry to entry64.S directly as the vCPU
reset entry. In this basic entry code, we just initialize the boot
stack and prepare jumping to _libkvmplat_start.

Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
---
   arch/arm/Compiler.uk                |  4 ++
   arch/arm/Makefile.uk                |  4 ++
   plat/common/include/arm/arm64/asm.h | 77 +++++++++++++++++++++++++++++
   plat/common/include/arm/asm.h       | 43 ++++++++++++++++
   plat/common/include/asm.h           | 43 ++++++++++++++++
   plat/kvm/Config.uk                  |  2 +-
   plat/kvm/Makefile.uk                | 14 ++++++
   plat/kvm/arm/entry64.S              | 74 +++++++++++++++++++++++++++
   plat/kvm/arm/setup.c                | 26 ++++++++++
   9 files changed, 286 insertions(+), 1 deletion(-)
   create mode 100644 plat/common/include/arm/arm64/asm.h
   create mode 100644 plat/common/include/arm/asm.h
   create mode 100644 plat/common/include/asm.h
   create mode 100644 plat/kvm/arm/entry64.S
   create mode 100644 plat/kvm/arm/setup.c

diff --git a/arch/arm/Compiler.uk b/arch/arm/Compiler.uk
index 147f662..3fd0f3f 100644
--- a/arch/arm/Compiler.uk
+++ b/arch/arm/Compiler.uk
@@ -1,3 +1,7 @@
   ifeq ($(CONFIG_UK_ARCH),arm)
        include $(CONFIG_UK_BASE)/arch/arm/arm/Compiler.uk
   endif
+
+ifeq ($(CONFIG_UK_ARCH),arm64)
+       include $(CONFIG_UK_BASE)/arch/arm/arm64/Compiler.uk
+endif
diff --git a/arch/arm/Makefile.uk b/arch/arm/Makefile.uk
index e59b2a3..11d81fb 100644
--- a/arch/arm/Makefile.uk
+++ b/arch/arm/Makefile.uk
@@ -2,3 +2,7 @@
   ifeq ($(CONFIG_UK_ARCH),arm)
        include $(CONFIG_UK_BASE)/arch/arm/arm/Makefile.uk
   endif
+
+ifeq ($(CONFIG_UK_ARCH),arm64)
+       include $(CONFIG_UK_BASE)/arch/arm/arm64/Makefile.uk
+endif
diff --git a/plat/common/include/arm/arm64/asm.h

b/plat/common/include/arm/arm64/asm.h

new file mode 100644
index 0000000..ce361b1
--- /dev/null
+++ b/plat/common/include/arm/arm64/asm.h
@@ -0,0 +1,77 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Wei Chen <wei.chen@xxxxxxx>
+ *
+ * Copyright (c) 2018, Arm Ltd. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS

IS"

+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,

THE

+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR

PURPOSE

+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS

BE

+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF

THE

+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+#ifndef __PLAT_CMN_ARM_ARM64_ASM_H__
+#define __PLAT_CMN_ARM_ARM64_ASM_H__
+
+#ifndef _BITUL


Why do you need this #ifndef?



I copied it from ukvm code. It seems I just need #ifndef _BITUL
for following small section:
#define _BITUL(x)   (_AC(1,UL) << (x))
#define _BITULL(x)  (_AC(1,ULL) << (x))
There was recently a new header upstreamed to Unikraft that give you definitions of this sort: include/uk/bitops.h 
You can those instead.




+
+#ifdef __ASSEMBLY__
+
+/* Linkage for ARM */
+#define __ALIGN .align 2
+#define __ALIGN_STR ".align 2"
+
+#define ALIGN __ALIGN
+#define ALIGN_STR __ALIGN_STR
+
+#define ENTRY(name)    \
+.globl name;           \
+ALIGN;                 \
+name:
+
+#define GLOBAL(name)   \
+.globl name;           \
+name:
+
+#define END(name)      \
+.size name, .-name
+
+#define ENDPROC(name)  \
+.type name, %function; \
+END(name)
+
+#define _AC(X,Y)    X
+#define _AT(T,X)    X
+
+#else
+#define __AC(X,Y)   (X##Y)
+#define _AC(X,Y)    __AC(X,Y)
+#define _AT(T,X)    ((T)(X))
+#endif
+
+#define _BITUL(x)   (_AC(1,UL) << (x))
+#define _BITULL(x)  (_AC(1,ULL) << (x))


Most of this code is not arm specific. Can this be defined in a common
header?


Maybe you want to put these definitions somewhere in /include/uk/





I was not sure how can this change affect the x86 code, so I placed
it for arm only. But as you mentioned, I will try.


+
+#endif
+#endif /* __PLAT_CMN_ARM_ARM64_ASM_H__ */
diff --git a/plat/common/include/arm/asm.h b/plat/common/include/arm/asm.h
new file mode 100644
index 0000000..ad07520
--- /dev/null
+++ b/plat/common/include/arm/asm.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Wei Chen <wei.chen@xxxxxxx>
+ *
+ * Copyright (c) 2018, Arm Ltd. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS

IS"

+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,

THE

+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR

PURPOSE

+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS

BE

+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF

THE

+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+#ifndef __PLAT_CMN_ARM_ASM_H__
+#define __PLAT_CMN_ARM_ASM_H__
+
+#if defined(__ARM_64__)
+#include "arm64/asm.h"
+#else
+#error "Add asm.h for current architecture."
+#endif
+
+#endif /* __PLAT_CMN_ARM_ASM_H__ */
diff --git a/plat/common/include/asm.h b/plat/common/include/asm.h
new file mode 100644
index 0000000..26576d5
--- /dev/null
+++ b/plat/common/include/asm.h


I don't think this header is useful because asm.h is only called from
arm specific code.



What did you mean by? I need a header for:
#define ENTRY(name)     \
.globl name;            \
ALIGN;                  \
name:

I don't want to define them in every file.


+.text
+ENTRY(_libkvmplat_entry)
+       /* Boot stack is placed after pagetable area temporarily */
+       ldr x26, =_end
+       add x26, x26, #PAGE_TABLE_SIZE
+       add x27, x26, #BOOT_STACK_SIZE
+
+       /*
+        * Clean the boot stack. As _end, PAGE_TABLE_SIZE and BOOT_STACK_SIZE
+        * are page_size alignment, the boot stack can be 64-bytes alignment
+        * too. Execute 4 stp consecutively without boundary check would be
+        * safe here.
+        */
+1:
+       stp xzr, xzr, [x26], #16
+       stp xzr, xzr, [x26], #16
+       stp xzr, xzr, [x26], #16
+       stp xzr, xzr, [x26], #16
+       cmp x26, x27
+       b.lo 1b


If you move them to BSS, then you don't have to care about initializing
the stack and page-table.



I have replied in previous patch.


Cheers,

--
Julien Grall


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.

Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.