You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "W-M-R (via GitHub)" <gi...@apache.org> on 2025/01/06 12:19:05 UTC
[PR] arm64/mte: Allows thread to automatically control the switch of mte [nuttx]
W-M-R opened a new pull request, #15437:
URL: https://github.com/apache/nuttx/pull/15437
*Note: Please adhere to [Contributing Guidelines](https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md).*
## Summary
arm64/mte: Allows thread to automatically control the switch of mte
## Impact
*Update this section, where applicable, on how change affects users,
build process, hardware, documentation, security, compatibility, etc.*
## Testing
*Update this section with details on how did you verify the change,
what Host was used for build (OS, CPU, compiler, ..), what Target was
used for verification (arch, board:config, ..), etc. Providing build
and runtime logs from before and after change is highly appreciated.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arm64: Allows thread to independent control the switch of sctlr [nuttx]
Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #15437:
URL: https://github.com/apache/nuttx/pull/15437#issuecomment-2573031195
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
This PR does **not** fully meet the NuttX requirements. While it provides a summary of *what* the change does, it lacks crucial information.
Here's a breakdown of what's missing:
**Summary:**
* **Missing "Why":** The summary explains the *how* but not the *why*. Why is per-thread MTE control necessary? What problem does it solve? Is there a bug being fixed? Is this a performance optimization?
* **Missing Scope:** What specific files/modules are affected? Saying "the method is the same as..." isn't descriptive enough. Name the specific functions or files modified.
* **Missing Issue References:** Are there any related NuttX or NuttX-apps issues that prompted this change? If so, link them.
**Impact:**
* **Completely Empty:** This section is critical. The maintainer needs to understand the ramifications of merging this change. Address *all* the points (user impact, build impact, hardware impact, documentation, security, compatibility). Even if the answer is "NO", explicitly state it. For example: "Impact on user: NO"
* **Security implications are especially important here** given that this relates to MTE (Memory Tagging Extension), a security feature. What are the potential security benefits or drawbacks of this change?
**Testing:**
* **Completely Empty:** This section is also crucial. Without evidence of testing, the PR is unlikely to be merged. Provide specific details:
* **Build Host:** e.g., "Linux (Ubuntu 22.04), x86_64, GCC 11.2.0"
* **Target:** e.g., "qemu-rv64:nsh"
* **Logs:** Include *actual* logs showing the behavior before and after the change. Demonstrate that the change works as intended and doesn't introduce regressions. If the logs are too long, consider providing a link to a gist or pastebin.
**In short, the PR needs significant improvements before it can be considered for merging.** It needs to be more thorough and provide the necessary information for reviewers to evaluate the change effectively. Follow the template more closely and fill in all the required sections.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arm64: Allows thread to independent control the switch of sctlr [nuttx]
Posted by "W-M-R (via GitHub)" <gi...@apache.org>.
W-M-R commented on PR #15437:
URL: https://github.com/apache/nuttx/pull/15437#issuecomment-2581888324
> > > This approach causes the code size of "nuttx/arch/arm64/src/common/arm64_vector_table.S： arm64_enter_exception“ to be larger than the 128 bytes required by the vector table. Can this problem be solved by changing the macro to a function? ! I passed the test by turning off "CONFIG_ARCH_FPU"
> >
> >
> > let's try to reduce the code size first.
>
> @W-M-R some optimization can be done:
>
> 1. Skip save and restore x0 since it's always zero
> 2. Remove the duplication loading tpidrro_el0 at line 92
> 3. Remove line 93 by using add with immediate value:
> https://developer.arm.com/documentation/ddi0596/2021-03/Base-Instructions/ADDS--immediate---Add--immediate---setting-flags-#:~:text=Add%20(immediate)%2C%20setting%20flags,the%20alias%20CMN%20(immediate).
I didn't find any information about the X0 register being always 0 for arm64? I only saw that X0 is used to pass the first parameter, https://developer.arm.com/documentation/102374/0102/Procedure-Call-Standard, not sure about the status of X0 under arm64 vector?
But fortunately, after optimizing according to 2 and 3 instructions, the size of the vector table meets the requirements again
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arm64: Allows thread to independent control the switch of sctlr [nuttx]
Posted by "zouboan (via GitHub)" <gi...@apache.org>.
zouboan commented on PR #15437:
URL: https://github.com/apache/nuttx/pull/15437#issuecomment-2630251657
#15747
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arm64: Allows thread to independent control the switch of sctlr [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #15437:
URL: https://github.com/apache/nuttx/pull/15437#issuecomment-2581919934
> > > > This approach causes the code size of "nuttx/arch/arm64/src/common/arm64_vector_table.S： arm64_enter_exception“ to be larger than the 128 bytes required by the vector table. Can this problem be solved by changing the macro to a function? ! I passed the test by turning off "CONFIG_ARCH_FPU"
> > >
> > >
> > > let's try to reduce the code size first.
> >
> >
> > @W-M-R some optimization can be done:
> >
> > 1. Skip save and restore x0 since it's always zero
> > 2. Remove the duplication loading tpidrro_el0 at line 92
> > 3. Remove line 93 by using add with immediate value:
> > https://developer.arm.com/documentation/ddi0596/2021-03/Base-Instructions/ADDS--immediate---Add--immediate---setting-flags-#:~:text=Add%20(immediate)%2C%20setting%20flags,the%20alias%20CMN%20(immediate).
>
> I didn't find any information about the X0 register being always 0 for arm64? I only saw that X0 is used to pass the first parameter, https://developer.arm.com/documentation/102374/0102/Procedure-Call-Standard, not sure about the status of X0 under arm64 vector? But fortunately, after optimizing according to 2 and 3 instructions, the size of the vector table meets the requirements again
Ok, my memory is wrong, the zero register is X31, not X0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arm64: Allows thread to independent control the switch of sctlr [nuttx]
Posted by "W-M-R (via GitHub)" <gi...@apache.org>.
W-M-R commented on PR #15437:
URL: https://github.com/apache/nuttx/pull/15437#issuecomment-2573070939
This approach causes the code size of "nuttx/arch/arm64/src/common/arm64_vector_table.S" to be larger than the 128 bytes required by the vector table. Can this problem be solved by changing the macro to a function?
I passed the test by turning off "CONFIG_ARCH_FPU"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arm64: Allows thread to automatically control the switch of sctlr [nuttx]
Posted by "W-M-R (via GitHub)" <gi...@apache.org>.
W-M-R commented on PR #15437:
URL: https://github.com/apache/nuttx/pull/15437#issuecomment-2573009275
This approach requires dependencies:
This macro needs to be changed to a function implementation. The vector table requires 128 bytes to align. Using the macro and adding new code will make the code segment exceed 128 bytes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arm64: Allows thread to independent control the switch of sctlr [nuttx]
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on code in PR #15437:
URL: https://github.com/apache/nuttx/pull/15437#discussion_r1904252020
##########
arch/arm64/src/common/arm64_vector_table.S:
##########
@@ -80,6 +80,9 @@
#endif
stp \xreg0, \xreg1, [sp, #8 * REG_ELR]
+ mrs \xreg0, sctlr_el1
+ str \xreg0, [sp, #8 * REG_SCTLR_EL1]
Review Comment:
```suggestion
str \xreg0, [sp, #8 * REG_SCTLR_EL1]
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arm64: Allows thread to independent control the switch of sctlr [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #15437:
URL: https://github.com/apache/nuttx/pull/15437
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arm64: Allows thread to independent control the switch of sctlr [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #15437:
URL: https://github.com/apache/nuttx/pull/15437#issuecomment-2575823580
> > This approach causes the code size of "nuttx/arch/arm64/src/common/arm64_vector_table.S： arm64_enter_exception“ to be larger than the 128 bytes required by the vector table. Can this problem be solved by changing the macro to a function? ! I passed the test by turning off "CONFIG_ARCH_FPU"
>
> let's try to reduce the code size first.
some optimization can be done:
1. Skip save and restore x0 since it's always zero
2. Remove the duplication loading tpidrro_el0 at line 92
3. Remove line 93 by using add with immediate value:
https://developer.arm.com/documentation/ddi0596/2021-03/Base-Instructions/ADDS--immediate---Add--immediate---setting-flags-#:~:text=Add%20(immediate)%2C%20setting%20flags,the%20alias%20CMN%20(immediate).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arm64: Allows thread to independent control the switch of sctlr [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #15437:
URL: https://github.com/apache/nuttx/pull/15437#issuecomment-2573285842
> This approach causes the code size of "nuttx/arch/arm64/src/common/arm64_vector_table.S： arm64_enter_exception“ to be larger than the 128 bytes required by the vector table. Can this problem be solved by changing the macro to a function? ! I passed the test by turning off "CONFIG_ARCH_FPU"
let's try to reduce the code size first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org