[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/18 09:57, Wei Chen (Arm Technology China) wrote:

Hi Julien,


Hi Wei,


-----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))
Why would someone else override _BITUL? It feels like that the arch one should be the one that is used by default.
But, I can't see any caller for them in Unikraft. So, it is probably better to drop them. 


+
+#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.
This is not what I meant. The file common/include/asm.h is just a re-direction for arm/asm.h. Can't you directly include arm/asm.h from Arm? 

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