[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCHv5 15/46] plat/include: Define address offsets of boot stack and pagetable



On 07.09.2018 07:14, Wei Chen (Arm Technology China) wrote:
Hi Simon,

-----Original Message-----
From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
Sent: 2018年9月6日 22:53
To: Wei Chen (Arm Technology China) <Wei.Chen@xxxxxxx>; minios-
devel@xxxxxxxxxxxxxxxxxxxx
Cc: Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
Subject: Re: [Minios-devel] [UNIKRAFT PATCHv5 15/46] plat/include: Define
address offsets of boot stack and pagetable

Hey Wei,

These defines for the memory layout are specific for KVM for now, right?
Wouldn't it then make sense to place this files to plat/kvm/include?
Or do you know if this is going to be the same for Xen?


I want to use the same memory layout for KVM and Xen. I know that current
Code for Xen platform is ported from mini-os. But once, when code for
Arm/KVM becomes stable, I want reuse most of the code for these two
platforms.

Okay, sounds reasonable. Could you add this as one sentence in the commit message to eplain why you decided to place this to common/?


On 10.08.2018 09:08, Wei Chen wrote:
From: Wei Chen <Wei.Chen@xxxxxxx>

If we place the boot stack and pagetable in BSS section. These
areas are not easy to be reused after changing to new stack.
So, in Arm64, we want to place the pagetable and boot stack
after the end of image. In this case, once we change to new
stack or we have new pagetable, these two areas can be reclaimed
very easy.

Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
---
   plat/common/include/arm/arm64/mm.h | 86 ++++++++++++++++++++++++++++++
   plat/common/include/arm/mm.h       | 44 +++++++++++++++
   plat/common/include/mm.h           | 44 +++++++++++++++
   3 files changed, 174 insertions(+)
   create mode 100644 plat/common/include/arm/arm64/mm.h
   create mode 100644 plat/common/include/arm/mm.h
   create mode 100644 plat/common/include/mm.h

diff --git a/plat/common/include/arm/arm64/mm.h
b/plat/common/include/arm/arm64/mm.h
new file mode 100644
index 0000000..3c1db27
--- /dev/null
+++ b/plat/common/include/arm/arm64/mm.h
@@ -0,0 +1,86 @@
+/* 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 __CPU_ARM_64_MM_H__
+#define __CPU_ARM_64_MM_H__
+
+/*
+ * We will place the pagetable and boot stack after image area,
+ * So we define the address offset of pagetable and boot stack
+ * here.
+ */

This is a general comment, right? For now it looks like it would explain
the following block. Could you add an empty line afterwards?


Yes, I will add an empty line.

+#define PAGE_SIZE  __PAGE_SIZE
+#define PAGE_SHIFT __PAGE_SHIFT
+#define STACK_SIZE __STACK_SIZE

Do you require these renamings or could you use the ukarch __* variants
directly?


It's just a habit, I think I can use ukarch __* variants directly, if this
is uniform style of Unikraft : )


It is actually just a little detail. I would take your patch with or without the change. I just do not like this renaming of the macros because it looks like a source for confusion later.

+
+/*
+ * Each entry in L0_TABLE can link to a L1_TABLE which supports 512GiB
+ * memory mapping. One 4K page can provide 512 entries. In this case,
+ * one page for L0_TABLE is enough for current stage.
+ */
+#define L0_TABLE_OFFSET 0
+#define L0_TABLE_SIZE   PAGE_SIZE
+
+/*
+ * Each entry in L1_TABLE can map to a 1GiB memory or link to a
+ * L2_TABLE which supports 1GiB memory mapping. One 4K page can provide
+ * 512 entries. We need at least 2 pages to support 1TB memory space
+ * for platforms like KVM QEMU virtual machine.
+ */
+#define L1_TABLE_OFFSET (L0_TABLE_OFFSET + L0_TABLE_SIZE)
+#define L1_TABLE_SIZE   (PAGE_SIZE * 2)
+
+/*
+ * Each entry in L2_TABLE can map to a 2MiB block memory or link to a
+ * L3_TABLE which supports 2MiB memory mapping. We need a L3_TABLE to
+ * cover image area for us to manager different sections attributes.
+ * So, we need one page for L2_TABLE to provide 511 enties for 2MiB
+ * block mapping and 1 entry for L3_TABLE link.
+ */
+#define L2_TABLE_OFFSET (L1_TABLE_OFFSET + L1_TABLE_SIZE)
+#define L2_TABLE_SIZE   PAGE_SIZE
+
+/*
+ * As Unikraft image's size is very tiny, from tens to hundreds kilo
+ * bytes. So one page for L3_TABLE is enough for us to manage section
+ * attributes of image.
+ */
+#define L3_TABLE_OFFSET (L2_TABLE_OFFSET + L2_TABLE_SIZE)
+#define L3_TABLE_SIZE   PAGE_SIZE
+
+/* Total memory size that will be used by pagetable */
+#define PAGE_TABLE_SIZE (L0_TABLE_SIZE + L1_TABLE_SIZE + \
+                               L2_TABLE_SIZE + L3_TABLE_SIZE)
+
+#endif /* __CPU_ARM_64_MM_H__ */
diff --git a/plat/common/include/arm/mm.h b/plat/common/include/arm/mm.h
new file mode 100644
index 0000000..0f5db19
--- /dev/null
+++ b/plat/common/include/arm/mm.h
@@ -0,0 +1,44 @@
+/* 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_MM_H__
+#define __PLAT_CMN_ARM_MM_H__
+
+#if defined(__ARM_64__)
+#include "arm64/mm.h"
+#else
+#error "Add cpu_defs.h for current architecture."

I think it should be called "mm.h"


Yes, it's a typo, I will fix it.

+#endif
+
+#endif /* __PLAT_CMN_ARM_MM_H__ */
diff --git a/plat/common/include/mm.h b/plat/common/include/mm.h
new file mode 100644
index 0000000..c3a37f5
--- /dev/null
+++ b/plat/common/include/mm.h
@@ -0,0 +1,44 @@
+/* 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_MM_H__
+#define __PLAT_CMN_MM_H__
+
+#if defined(__ARM_64__)
+#include <arm/mm.h>
+#else
+#error "Add cpu.h for current architecture."

Should also be called mm.h, probably.


Yes.

+#endif
+
+#endif /* __PLAT_CMN_MM_H__ */


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

 


Rackspace

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