Arm64 irq1 #13520

lipengfei28 · 2024-09-18T02:55:53Z

Summary

arm64: simply the vectors

Impact

Testing

lipengfei28 · 2024-09-18T02:56:21Z

1. save FPU regs enter exception and retore exit exception every time
2. simp the vectors
3. save sp_el0 enter exception and restore sp_el0 exit exception every time

arch/arm64/src/common/arm64_syscall.c

Summary The original implement for exception handler is very simple and haven't framework for breakpoint/watchpoint routine or brk instruction. I refine the fatal handler and add framework for debug handler to register or unregister. this is a prepare for watchpoint/breakpoint implement Signed-off-by: qinwei1 <[email protected]>

Signed-off-by: ligd <[email protected]>

github-actions · 2024-09-23T08:13:52Z

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

arch/arm64/src/common/arm64_arch.h

Summary Add a macro to obtain current execute level Signed-off-by: qinwei1 <[email protected]>

Signed-off-by: ligd <[email protected]>

Signed-off-by: lipengfei28 <[email protected]>

github-actions · 2024-09-23T13:18:23Z

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

pussuw · 2024-09-23T18:50:52Z

arch/arm64/src/common/arm64_vectors.S

-
-    bne    exc_handle
-
-#ifdef CONFIG_LIB_SYSCALL


Why exactly was this part changed??? Now you destroy the incoming system call parameter registers x0-x6, and then re-read them from the context wasting massive amounts of CPU time??

WHY WAS THIS CHANGED? I spent days making this optimized system call path to work, please restore this functionality as it was.

pussuw · 2024-09-23T18:52:25Z

arch/arm64/src/common/arm64_syscall.c

+           * cannot yet handle nested system calls.
+           */
+
+          regs[REG_X0] = dispatch_syscall(regs[REG_X0], regs[REG_X1],


This is completely unnecessary, the registers x0-x6 are already correct when coming from userspace. There is NO REASON TO READ THEM FROM THE CONTEXT AREA. What I had done is much more optimized than what this does.

Another thing this change does; the system call now runs with interrupts disabled, affecting interrupt service latency.

@lipengfei28 please take a look.

jlaitine · 2024-09-23T21:27:37Z

After this PR got merged, nuttx is not compiling any more. I am getting this:

common/arm64_fatal.c:84:30: error: initialized field overwritten [-Werror=override-init]
84 | [ESR_ELX_EC_UNKNOWN] = "Unknown/Uncategorized",
| ^~~~~~~~~~~~~~~~~~~~~~~
common/arm64_fatal.c:84:30: note: (near initialization for 'g_esr_class_str[0]')
common/arm64_fatal.c:85:30: error: initialized field overwritten [-Werror=override-init]
85 | [ESR_ELX_EC_WFX] = "WFI/WFE",
| ^~~~~~~~~
common/arm64_fatal.c:85:30: note: (near initialization for 'g_esr_class_str[1]')
common/arm64_fatal.c:86:30: error: initialized field overwritten [-Werror=override-init]
86 | [ESR_ELX_EC_CP15_32] = "CP15 MCR/MRC",
| ^~~~~~~~~~~~~~
common/arm64_fatal.c:86:30: note: (near initialization for 'g_esr_class_str[3]')
common/arm64_fatal.c:87:30: error: initialized field overwritten [-Werror=override-init]
87 | [ESR_ELX_EC_CP15_64] = "CP15 MCRR/MRRC",
| ^~~~~~~~~~~~~~~~
common/arm64_fatal.c:87:30: note: (near initialization for 'g_esr_class_str[4]')
common/arm64_fatal.c:88:30: error: initialized field overwritten [-Werror=override-init]
88 | [ESR_ELX_EC_CP14_MR] = "CP14 MCR/MRC",
| ^~~~~~~~~~~~~~
common/arm64_fatal.c:88:30: note: (near initialization for 'g_esr_class_str[5]')
common/arm64_fatal.c:89:30: error: initialized field overwritten [-Werror=override-init]
89 | [ESR_ELX_EC_CP14_LS] = "CP14 LDC/STC",
| ^~~~~~~~~~~~~~
common/arm64_fatal.c:89:30: note: (near initialization for 'g_esr_class_str[6]')
common/arm64_fatal.c:90:30: error: initialized field overwritten [-Werror=override-init]
90 | [ESR_ELX_EC_FP_ASIMD] = "ASIMD",

lipengfei28 · 2024-09-24T02:42:24Z

commit 8288fe4 (HEAD -> master, apache/master)
Author: chao an [email protected]
Date: Mon Sep 23 08:59:15 2024 +0800

Revert "toolchain/ghs: Fix CONFIG_SCHED_CRITMONITOR_MAXTIME_XXX "zero used for undefined preprocessing identifier" warnings"

move private define to public

This reverts commit 236678d730220138416724d2cab25d6e1bd2ce9d.

Signed-off-by: chao an <[email protected]>

commit a5b85fc
Author: wangjianyu3 [email protected]
Date: Mon Sep 23 18:44:17 2024 +0800

misc/rpmsgdev: Fix invalid pointer error when there are more than one remotes

Test: (see tests/testcases/rpmsgdev for details)
  # 1. Register dummy device
  testdev -d 0 -r "/dev/ttyGNSS0"
  # 2. Call rpmsgdev_export() to export the device to remote
  testdev -d 2 -c "droid" -l "/dev/ttyGNSS0"

Log:
  [ap] arm_busfault: PANIC!!! Bus Fault:
  [ap] arm_busfault:        IRQ: 5 regs: 0x3c434e44
  [ap] arm_busfault:        BASEPRI: 00000000 PRIMASK: 00000000 IPSR: 00000005 CONTROL: 00000004
  [ap] arm_busfault:        CFSR: 00008200 HFSR: 00000000 DFSR: 00000000 BFAR: 00000000 AFSR: 00040000
  [ap] arm_busfault: Bus Fault Reason:
  [ap] arm_busfault:        Precise data bus error
  [ap] dump_assert_info: Current Version: NuttX ****** ***** *** 12.3.0 ********** Sep 23 2024 18:35:50 arm
  [ap] dump_assert_info: Assertion failed panic: at file: armv8-m/arm_busfault.c:113 task: testdev process: testdev 0x2c86ca75

This commit build is ok

jlaitine · 2024-09-24T05:03:03Z

This PR seems problematic also for the performance; what is the reason for example for saving the FPU registers every time? Before this PR we had well working and performant arm64 in both flat and kernel builds (and extensively using FPU as well).

Or why to execute system calls with interrupts disabled? That is clearly wrong.

Can you please explain what is the problem this PR is trying to solve? Just to break everything for fun?

Sorry to be negative, but I don't see any benefits in breaking things like this, please explain!

jlaitine · 2024-09-24T05:14:38Z

FYI: I managed to get things running in our own branches again by reverting everything in this PR except for e38f2b2

arch/arm64/src/common/arm64_vector_table.S

xiaoxiang781216 · 2024-09-24T07:32:27Z

let's revert this patch first and refine the change later.

jlaitine · 2024-09-24T08:06:42Z

The idea in "arm64: refine the fatal handler" is good; I'd suggest making a separate PR of that one, on top of the existing implementation.

xiaoxiang781216 · 2024-09-24T14:15:33Z

@GUIDINGLI will resend the patch again.

GUIDINGLI · 2024-09-26T16:56:54Z

@jlaitine @pussuw
#13652

pussuw · 2024-09-26T17:04:54Z

@jlaitine @pussuw #13652

I'll take a look tomorrow, thanks

pussuw · 2024-09-26T17:37:35Z

arch/arm64/src/common/arm64_vectors.S

-#if CONFIG_ARCH_ARM64_EXCEPTION_LEVEL == 3
-    mrs    x0, esr_el3
+    /* Switch to IRQ stack and save current sp on it. */
+#ifdef CONFIG_SMP


@GUIDINGLI I assume this is the code that SMP needs ? I can adapt this part so that kernel mode works like before. I'll try to provide a patch tomorrow, or on Monday.

pussuw · 2024-09-26T17:38:28Z

arch/arm64/src/common/arm64_arch.h

@@ -299,6 +299,9 @@ struct regs_context
  uint64_t  spsr;
  uint64_t  sp_el0;
  uint64_t  exe_depth;
+#ifdef CONFIG_ARCH_FPU


Is the content of this patch needed by SMP too ?

The removable of FPU trap reason:

The FPU trap logic is not stable in some SMP situation (HERE haven't the detail analysis)

Make the logic Complex and different to read.

The most important reason: it has almost no benefit for the performance, because the compiler will optimize the code with float/double instructions when use the option -O3 (always be). Even the printf/syslog will use use the FPU, so the FPU is used too often and almost each context switch.
Or, you must use -mgeneral-regs-only to limit the FPU registers usage, that will harmful for the speed.

How about use the FPU registers in the IRQ/SVC ISR, maybe there are hidden bugs ?

lipengfei28 force-pushed the arm64_irq1 branch 4 times, most recently from 1e1c326 to 2b07421 Compare September 18, 2024 07:04

acassis approved these changes Sep 18, 2024

View reviewed changes

xiaoxiang781216 reviewed Sep 18, 2024

View reviewed changes

arch/arm64/src/common/arm64_syscall.c Outdated Show resolved Hide resolved

arch/arm64/src/common/arm64_syscall.c Outdated Show resolved Hide resolved

lipengfei28 force-pushed the arm64_irq1 branch 3 times, most recently from 2539ba1 to 5d4b00a Compare September 20, 2024 02:03

github-actions bot added the Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces. label Sep 20, 2024

lipengfei28 force-pushed the arm64_irq1 branch 4 times, most recently from 8f7fb07 to 851d7ff Compare September 23, 2024 02:23

github-actions bot added the Arch: arm64 Issues related to ARM64 (64-bit) architecture label Sep 23, 2024

qinwei2004 and others added 2 commits September 23, 2024 16:07

arm64: save FPU regs every time

43b49c2

Signed-off-by: ligd <[email protected]>

lipengfei28 force-pushed the arm64_irq1 branch from 851d7ff to a94e570 Compare September 23, 2024 08:13

acassis reviewed Sep 23, 2024

View reviewed changes

arch/arm64/src/common/arm64_arch.h Outdated Show resolved Hide resolved

qinwei2004 and others added 3 commits September 23, 2024 21:17

arm64: add arm64_current_el to obtain current EL

37ff503

Summary Add a macro to obtain current execute level Signed-off-by: qinwei1 <[email protected]>

arm64: simply the vectors

671c1f9

Signed-off-by: ligd <[email protected]>

Kernel build: enter exception save sp_sl0,exit exception restroe sp_el0

848afc2

Signed-off-by: lipengfei28 <[email protected]>

lipengfei28 force-pushed the arm64_irq1 branch from a94e570 to 848afc2 Compare September 23, 2024 13:17

xiaoxiang781216 approved these changes Sep 23, 2024

View reviewed changes

xiaoxiang781216 merged commit 52a4fb6 into apache:master Sep 23, 2024
29 checks passed

pussuw reviewed Sep 23, 2024

View reviewed changes

jlaitine reviewed Sep 24, 2024

View reviewed changes

arch/arm64/src/common/arm64_vector_table.S Show resolved Hide resolved

xiaoxiang781216 mentioned this pull request Sep 24, 2024

Revert "Arm64 irq1" #13590

Merged

GUIDINGLI mentioned this pull request Sep 26, 2024

ARM64 SMP IRQ stack bug modify request #13652

Closed

pussuw reviewed Sep 26, 2024

View reviewed changes

pussuw mentioned this pull request Sep 27, 2024

arm64/SMP: Fix FPU and exception handling for ARM64 #13695

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Arm64 irq1 #13520

Arm64 irq1 #13520

lipengfei28 commented Sep 18, 2024

lipengfei28 commented Sep 18, 2024

github-actions bot commented Sep 23, 2024

github-actions bot commented Sep 23, 2024

pussuw Sep 23, 2024 •

edited

Loading

pussuw Sep 23, 2024 •

edited

Loading

pussuw Sep 23, 2024

xiaoxiang781216 Sep 24, 2024

jlaitine commented Sep 23, 2024

lipengfei28 commented Sep 24, 2024

jlaitine commented Sep 24, 2024

jlaitine commented Sep 24, 2024

xiaoxiang781216 commented Sep 24, 2024

jlaitine commented Sep 24, 2024

xiaoxiang781216 commented Sep 24, 2024

GUIDINGLI commented Sep 26, 2024

pussuw commented Sep 26, 2024

pussuw Sep 26, 2024

GUIDINGLI Sep 27, 2024

pussuw Sep 26, 2024

GUIDINGLI Sep 27, 2024

Arm64 irq1 #13520

Arm64 irq1 #13520

Conversation

lipengfei28 commented Sep 18, 2024

Summary

Impact

Testing

lipengfei28 commented Sep 18, 2024

github-actions bot commented Sep 23, 2024

github-actions bot commented Sep 23, 2024

pussuw Sep 23, 2024 • edited Loading

Choose a reason for hiding this comment

pussuw Sep 23, 2024 • edited Loading

Choose a reason for hiding this comment

pussuw Sep 23, 2024

Choose a reason for hiding this comment

xiaoxiang781216 Sep 24, 2024

Choose a reason for hiding this comment

jlaitine commented Sep 23, 2024

lipengfei28 commented Sep 24, 2024

jlaitine commented Sep 24, 2024

jlaitine commented Sep 24, 2024

xiaoxiang781216 commented Sep 24, 2024

jlaitine commented Sep 24, 2024

xiaoxiang781216 commented Sep 24, 2024

GUIDINGLI commented Sep 26, 2024

pussuw commented Sep 26, 2024

pussuw Sep 26, 2024

Choose a reason for hiding this comment

GUIDINGLI Sep 27, 2024

Choose a reason for hiding this comment

pussuw Sep 26, 2024

Choose a reason for hiding this comment

GUIDINGLI Sep 27, 2024

Choose a reason for hiding this comment

pussuw Sep 23, 2024 •

edited

Loading

pussuw Sep 23, 2024 •

edited

Loading