[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [PATCH 09/40] arm64: add the basic helpers for arm64
Hi Shijie,
On 03/11/17 03:11, Huang Shijie wrote:
This patch adds the basic helpers in header os.h for arm32 and arm64:
This is slightly untrue. arm32 helpers already exists. You only move
them in separate headers. But they should really be moved in a separate
patch.
0.) mb/rmb/wmb
1.) local_irq_disable/local_irq_enable
2.) local_irq_save/local_irq_restore/local_save_flags
3.) __ffs
This patch refers to Chen Baozi's patch:
"Initial codes for arm64"
Change-Id: Id9025f016968a815029dfba7ee275d9f757974f0
Jira: ENTOS-247
Signed-off-by: Huang Shijie <shijie.huang@xxxxxxx>
---
include/arm/arm32/os.h | 29 ++++++++++++++++++++++
include/arm/arm64/os.h | 35 +++++++++++++++++++++++++++
include/arm/os.h | 65 ++++++--------------------------------------------
3 files changed, 71 insertions(+), 58 deletions(-)
create mode 100644 include/arm/arm32/os.h
create mode 100644 include/arm/arm64/os.h
diff --git a/include/arm/arm32/os.h b/include/arm/arm32/os.h
new file mode 100644
index 0000000..6da604f
--- /dev/null
+++ b/include/arm/arm32/os.h
@@ -0,0 +1,29 @@
+#ifndef _ARM32_OS_H
+#define _ARM32_OS_H
+
+static inline void local_irq_disable(void) {
+ __asm__ __volatile__("cpsid i":::"memory");
Does the __ is really needed? Same for everywhere in that patch.
+}
+
+static inline void local_irq_enable(void) {
+ __asm__ __volatile__("cpsie i":::"memory");
+}
+
+#define local_irq_save(x) { \
+ __asm__ __volatile__("mrs %0, cpsr;cpsid i":"=r"(x)::"memory"); \
+}
+
+#define local_irq_restore(x) { \
+ __asm__ __volatile__("msr cpsr_c, %0"::"r"(x):"memory"); \
+}
+
+#define local_save_flags(x) { \
+ __asm__ __volatile__("mrs %0, cpsr":"=r"(x)::"memory"); \
+}
+
+/* We probably only need "dmb" here, but we'll start by being paranoid. */
+#define mb() __asm__("dsb":::"memory");
+#define rmb() __asm__("dsb":::"memory");
+#define wmb() __asm__("dsb":::"memory");
+
+#endif
Let's try to follow good practice on new files. E.g:
#endif /* _ARM32_OS_H */
/*
* Local variables:
* mode: C
* c-file-style: "BSD"
* c-basic-offset: 4
* tab-width: 4
* indent-tabs-mode: nil
* End:
*/
diff --git a/include/arm/arm64/os.h b/include/arm/arm64/os.h
new file mode 100644
index 0000000..cb2ab14
--- /dev/null
+++ b/include/arm/arm64/os.h
@@ -0,0 +1,35 @@
+#ifndef _ARM64_OS_H_
+#define _ARM64_OS_H_
+
+static inline void local_irq_disable(void)
+{
+ __asm__ __volatile__("msr daifset, #2": : :"memory");
+}
+
+static inline void local_irq_enable(void)
+{
+ __asm__ __volatile__("msr daifclr, #2": : :"memory");
+}
+
+#define local_irq_save(x) { \
+ __asm__ __volatile__("mrs %0, daif; msr daifset, #2":"=r"(x)::"memory"); \
Can you try to be consistent. Either you put space between : or you don't.
Personally, I would prefer the ::: version.
+}
+
+#define local_irq_restore(x) { \
+ __asm__ __volatile__("msr daif, %0": :"r"(x):"memory"); \
+}
+
+#define local_save_flags(x) { \
+ __asm__ __volatile__("mrs %0, daif":"=r"(x): :"memory"); \
+}
+
+#define isb() __asm__ __volatile("isb" : : : "memory")
+
+#define dmb(opt) __asm__ __volatile("dmb " #opt : : : "memory")
+#define dsb(opt) __asm__ __volatile("dsb " #opt : : : "memory")
isb, dmb, dsb are the same mnemonic on Arm32 and Arm64. Please move them
in common header.
+
+#define mb() dmb(sy) /* Full system memory barrier all */
+#define wmb() dmb(st) /* Full system memory barrier store */
+#define rmb() dmb(ld) /* Full system memory barrier load */
Can you explain the rationale behind using dmb here and not dsb?
Note that I will ask the same, if you move back to dsb.
+
+#endif
diff --git a/include/arm/os.h b/include/arm/os.h
index 6a1cc37..37fb007 100644
--- a/include/arm/os.h
+++ b/include/arm/os.h
@@ -22,27 +22,11 @@ extern void *device_tree;
extern shared_info_t *HYPERVISOR_shared_info;
-// disable interrupts
-static inline void local_irq_disable(void) {
- __asm__ __volatile__("cpsid i":::"memory");
-}
-
-// enable interrupts
-static inline void local_irq_enable(void) {
- __asm__ __volatile__("cpsie i":::"memory");
-}
-
-#define local_irq_save(x) { \
- __asm__ __volatile__("mrs %0, cpsr;cpsid i":"=r"(x)::"memory"); \
-}
-
-#define local_irq_restore(x) { \
- __asm__ __volatile__("msr cpsr_c, %0"::"r"(x):"memory"); \
-}
-
-#define local_save_flags(x) { \
- __asm__ __volatile__("mrs %0, cpsr":"=r"(x)::"memory"); \
-}
+#if defined(__arm__)
+#include <arm32/os.h>
+#elif defined(__aarch64__)
+#include <arm64/os.h>
+#endif
static inline int irqs_disabled(void) {
int x;
@@ -50,14 +34,7 @@ static inline int irqs_disabled(void) {
return x & 0x80;
}
-/* We probably only need "dmb" here, but we'll start by being paranoid. */
-#define mb() __asm__("dsb":::"memory");
-#define rmb() __asm__("dsb":::"memory");
-#define wmb() __asm__("dsb":::"memory");
-
-/************************** arm *******************************/
#ifdef __INSIDE_MINIOS__
-#if defined (__arm__)
#define xchg(ptr,v) __atomic_exchange_n(ptr, v, __ATOMIC_SEQ_CST)
/**
@@ -121,39 +98,11 @@ static __inline__ void clear_bit(int nr, volatile unsigned
long *addr)
test_and_clear_bit(nr, addr);
}
-/**
- * __ffs - find first (lowest) set bit in word.
- * @word: The word to search
- *
- * Undefined if no bit exists, so code should check against 0 first.
- */
Why did you drop the comment?
-static __inline__ unsigned long __ffs(unsigned long word)
+static inline unsigned long __ffs(unsigned long word)
Why the prototype has been changed?
{
- int clz;
-
- /* xxxxx10000 = word
- * xxxxx01111 = word - 1
- * 0000011111 = word ^ (word - 1)
- * 4 = 31 - clz(word ^ (word - 1))
- */
-
- __asm__ (
- "sub r0, %[word], #1\n"
- "eor r0, r0, %[word]\n"
- "clz %[clz], r0\n":
- /* Outputs: */
- [clz] "=r"(clz):
- /* Inputs: */
- [word] "r"(word):
- /* Clobbers: */
- "r0");
-
- return 31 - clz;
+ return __builtin_ctzl(word);
Are you sure __builtin_ctzl is defined everywhere? Furthermore, this
does not belong to this patch.
}
-#else /* ifdef __arm__ */
-#error "Unsupported architecture"
-#endif
#endif /* ifdef __INSIDE_MINIOS */
/********************* common arm32 and arm64 ****************************/
Cheers,
--
Julien Grall
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/minios-devel
|
