[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [UNIKRAFT PATCH 4/4] arch: boilerplate Arm support for thread-local storage
On 4/25/19 1:24 PM, Costin Lupu wrote:
Hi Florian,
On 4/24/19 10:27 AM, Florian Schmidt wrote:
Hi Costin,
On 4/16/19 3:22 PM, Costin Lupu wrote:
Hi Florian,
I'm not sure what is the expected outcome of this patch, however I
couldn't build on ARM64 because the following files are not included in
the Makefile:
The whole point of this patch was simply to make sure unikraft still
compiled on Arm, even if there was no usable functionality in some
parts. Apparently, I failed with that task. ;-)
However:
+LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) +=
$(UK_PLAT_COMMON_BASE)/thread.c|common
+LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) +=
$(UK_PLAT_COMMON_BASE)/sw_ctx.c|common
That doesn't seem right to me either, for two reasons: There is no
proper scheduling implemented yet for Arm, if I remember correctly; and
sw_ctx.c can't properly work on Arm because it includes x86-specific
code at this point in time. I think any build that enables uksched at
this point in time fails to build. Maybe that can be fixed by including
those two files, but I would expect everything to fail spectacularly as
soon as you run it, then.
The problem here is that sw_ctx.c should not contain any hardware
specific code. So the x86-specific code should be moved elsewhere and an
abstraction should be used instead.
The software context (sw_ctx) is an abstraction used in scheduling when
the hardware support isn't used, e.g. in cooperative scheduling. It's
the opposite of hardware context (hw_ctx) which involves using hardware
support for preemptive scheduling.
I agree with you here. I actually planned to remake sw_ctx.c
architecture-independent, but IIRC I agreed with Simon and Sharan to
postpone this until threading support for Arm is in. As it stands, you
can't properly use any kind of scheduling with Arm anyway, which is why
I didn't test that case that now fails to compile for you.
So yeah, the files in this patch are just there to allow compiling and
linking on Arm with the current feature set (scheduling disabled). Plus
some very preliminary untested implementation of tls_copy() for Arm64,
since I figured I'd do it now that I had read all the documentation
fresh in my mind.
Now, I lost track of the ARM changes and I'm not sure if this is WIP or
it should be fixed by these TLS series.
Please see my other comments inline.
On 4/15/19 1:03 PM, Florian Schmidt wrote:
Arm32 is ure boilerplate. Arm64 has some rough guesses about how the
A typo here: ure?
Ah, yes. It should have been "pure boilerplate".
layout should look like, but is untested.
Signed-off-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>
---
arch/arm/arm/include/uk/asm/thread.h | 60 ++++++++++++++++++++++
arch/arm/arm64/include/uk/asm/thread.h | 70 ++++++++++++++++++++++++++
plat/kvm/arm/link64.lds.S | 18 +++++++
plat/xen/arm/link32.lds | 18 +++++++
4 files changed, 166 insertions(+)
create mode 100644 arch/arm/arm/include/uk/asm/thread.h
create mode 100644 arch/arm/arm64/include/uk/asm/thread.h
diff --git a/arch/arm/arm/include/uk/asm/thread.h
b/arch/arm/arm/include/uk/asm/thread.h
new file mode 100644
index 00000000..7225209e
--- /dev/null
+++ b/arch/arm/arm/include/uk/asm/thread.h
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Florian Schmidt <florian.schmidt@xxxxxxxxx>
+ *
+ * Copyright (c) 2019, NEC Europe Ltd., NEC Corporation. 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 __UKARCH_THREAD_H__
+#error Do not include this header directly
+#endif
+
+#error Thread-local storage not implemented for arm32!
+
+extern char _tls_start[], _etdata[], _tls_end[];
+
+static inline unsigned long tls_area_size(void)
+{
+ return 0;
+}
+
+static inline unsigned int tls_area_align(void)
+{
+ return 1;
+}
+
+static inline void tls_copy(void *tls_area)
+{
+}
+
+static inline void *tls_pointer(void *tls_area)
+{
+ return NULL;
+}
diff --git a/arch/arm/arm64/include/uk/asm/thread.h
b/arch/arm/arm64/include/uk/asm/thread.h
new file mode 100644
index 00000000..2f5d9b98
--- /dev/null
+++ b/arch/arm/arm64/include/uk/asm/thread.h
Lets follow the same suggestions for this files as the ones mentioned
for the previous patches.
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Florian Schmidt <florian.schmidt@xxxxxxxxx>
+ *
+ * Copyright (c) 2019, NEC Europe Ltd., NEC Corporation. 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 __UKARCH_THREAD_H__
+#error Do not include this header directly
+#endif
+
+#warning Thread-local storage has not been tested on aarch64!
+
+extern char _tls_start[], _etdata[], _tls_end[];
+
+static inline unsigned long tls_area_size(void)
+{
+ /* aarch64 ABI adds 16 bytes of TCB at the beginning of the TLS
area,
+ * followed by the actual TLS data.
+ */
+ return _tls_end - _tls_start + 16;
+}
+
+static inline unsigned int tls_area_align(void)
+{
+ return 8;
+}
+
+static inline void tls_copy(void *tls_area)
+{
+ unsigned int tls_len = _tls_end - _tls_start;
+ unsigned int tls_data_len = _etdata - _tls_start;
+ unsigned int tls_bss_len = _tls_end - _etdata;
+
+ memset(tls_area, 0, 16);
+ memcpy(tls_area+16, _tls_start, tls_data_len);
+ memset(tls_area+tls_data_len+16, 0, tls_bss_len);
Again, lets stick to Unikraft coding style for the spacing of these 2
lines of code in order to avoid propagating such examples.
Agreed.
+}
+
+static inline void *tls_pointer(void *tls_area)
+{
+ return tls_area;
+}
diff --git a/plat/kvm/arm/link64.lds.S b/plat/kvm/arm/link64.lds.S
index 82328633..1ce22781 100644
--- a/plat/kvm/arm/link64.lds.S
+++ b/plat/kvm/arm/link64.lds.S
@@ -118,6 +118,24 @@ SECTIONS {
_ectors = .;
. = ALIGN(__PAGE_SIZE);
+ . = ALIGN(0x8);
+ _tls_start = .;
+ .tdata :
+ {
+ *(.tdata)
+ *(.tdata.*)
+ *(.gnu.linkonce.td.*)
+ }
+ _etdata = .;
+ .tbss :
+ {
+ *(.tbss)
+ *(.tbss.*)
+ *(.gnu.linkonce.tb.*)
+ . = ALIGN(0x8);
+ }
+ _tls_end = . + SIZEOF(.tbss);
+
Again these linker script changes may be refactored into a single linker
file which would be used by all architectures and platforms.
/* Read-write data that is initialized explicitly in code */
_data = .;
.data :
diff --git a/plat/xen/arm/link32.lds b/plat/xen/arm/link32.lds
index 2b8e42dd..e679ba6f 100644
--- a/plat/xen/arm/link32.lds
+++ b/plat/xen/arm/link32.lds
@@ -96,6 +96,24 @@ SECTIONS
. = ALIGN(4096);
_ectors = .;
+ . = ALIGN(0x8);
+ _tls_start = .;
+ .tdata :
+ {
+ *(.tdata)
+ *(.tdata.*)
+ *(.gnu.linkonce.td.*)
+ }
+ _etdata = .;
+ .tbss :
+ {
+ *(.tbss)
+ *(.tbss.*)
+ *(.gnu.linkonce.tb.*)
+ . = ALIGN(0x8);
+ }
+ _tls_end = . + SIZEOF(.tbss);
+
/* Data */
_data = .;
.data : {
--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel. +49 (0)6221 4342-265
Fax: +49 (0)6221 4342-155
e-mail: florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
