Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arm64/mte: Add support for arm64 mte #14978

Merged
merged 3 commits into from
Dec 2, 2024
Merged

arm64/mte: Add support for arm64 mte #14978

merged 3 commits into from
Dec 2, 2024

Conversation

W-M-R
Copy link
Contributor

@W-M-R W-M-R commented Nov 29, 2024

Note: Please adhere to Contributing Guidelines.

Summary

arm64/mte: Add support for arm64 mte
arm64/qemu: Add arm64 mte defconfig support
arm64/qemu: Add support for arm64 qemu's maximum feature cpu

Impact

This feature can only be run on qemu for now, see "Testing" for details
For details, please refer to the kernel's introduction to this at "https://docs.kernel.org/arch/arm64/memory-tagging-extension.html" and Android's introduction to this at "https://source.android.com/docs/security/test/memory-safety/arm-mte"
Of course, there is also the following detailed principle introduction
https://developer.arm.com/-/media/Arm%20Developer%20Community/PDF/Arm_Memory_Tagging_Extension_Whitepaper.pdf
The modification of this patch is only to merge the simplest MTE function support. In the future, the MTE function will be integrated into the kernel to a greater extent, for example, hardware MTE Kasan will be supported in the future.

Testing

Please compile my newly added arm64/qemu:mte.
Then run it with the following command

qemu-system-aarch64 -cpu max -nographic -smp 4 \
        -machine virt,virtualization=on,gic-version=3,mte=on -chardev stdio,id=con,mux=on,  -serial chardev:con \
        -mon chardev=con,mode=readline  -kernel ./nuttx/nuttx

Add the following app to test the mte function:

/****************************************************************************
 * Included Files
 ****************************************************************************/

#include <nuttx/config.h>
#include <nuttx/compiler.h>

#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>

/****************************************************************************
 * Private Data
 ****************************************************************************/

static aligned_data(16) char buffer[65536];

/****************************************************************************
 * Public Functions
 ****************************************************************************/

static void __attribute__((noinline)) tagset(void *p, size_t size)
{
  size_t i;
  for (i = 0; i < size; i += 16) {
    asm("stg %0, [%0]" : : "r"(p + i));
  }
}

static  void __attribute__((noinline)) tagcheck(void *p, size_t size)
{
  size_t i;
  void *c;

  for (i = 0; i < size; i += 16) 
    {
      asm("ldg %0, [%1]" : "=r"(c) : "r"(p + i), "0"(p));
      assert(c == p);
    }
}

int main(int argc, FAR char *argv[])
{
  char *p0 = (uint64_t)buffer;
  long excl = 1;
  char *p1 = 0;
  size_t size = 65536;

  /* Tag the pointer. */

  asm("irg %0,%1,%2" : "=r"(p1) : "r"(p0), "r"(excl));

  printf("p0: %llx\n", (unsigned long long)p0);
  printf("p1: %llx\n", (unsigned long long)p1);

  tagset(p1, 65536);
  tagcheck(p1, 65536);

  for (int i = 0; i < size / sizeof(int); i++)
    {
      p0[i] = i;
      printf("p0[%d]: %d\n", i, p0[i]);
    }

    return 0;
}

It will run with an error and enter the tag synchronization detection exception:

➜  NX git:(master) ✗ qemu-system-aarch64 -cpu max -nographic -smp 4 \
        -machine virt,virtualization=on,gic-version=3,mte=on -chardev stdio,id=con,mux=on,  -serial chardev:con \
        -mon chardev=con,mode=readline  -kernel ./nuttx/nuttx
- Ready to Boot Primary CPU
- Boot from EL2
- Boot from EL1
- Boot to C runtime for OS Initialize

NuttShell (NSH)
nsh> 
nsh> 
nsh> hello
p0: 403d4d50
p1: 3000000403d4d50
default_fatal_handler: (IFSC/DFSC) for Data/Instruction aborts: synchronous tag check fault
arm64_exception_handler: CurrentEL: MODE_EL1
arm64_exception_handler: ESR_ELn: 0x96000051
arm64_exception_handler: FAR_ELn: 0x403d4d50
arm64_exception_handler: ELR_ELn: 0x402a6aa8
print_ec_cause: DABT (current EL)
print_ec_cause: Data Abort taken without a change in Exception level
dump_assert_info: Current Version: NuttX  0.0.0 757d27880e Nov 29 2024 11:14:09 arm64
dump_assert_info: Assertion failed panic: at file: common/arm64_fatal.c:571 task: hello process: hello 0x402a6a44
up_dump_register: stack = 0x403f37f0
up_dump_register: x0:   0x403bbc98          x1:   0x0
up_dump_register: x2:   0x0                 x3:   0x3000000403e4d40
up_dump_register: x4:   0x0                 x5:   0x1
up_dump_register: x6:   0x1                 x7:   0x0
up_dump_register: x8:   0x1                 x9:   0x1
up_dump_register: x10:  0x0                 x11:  0x0
up_dump_register: x12:  0x0                 x13:  0x0
up_dump_register: x14:  0x0                 x15:  0x0
up_dump_register: x16:  0x0                 x17:  0x0
up_dump_register: x18:  0x0                 x19:  0x0
up_dump_register: x20:  0x403d4d50          x21:  0x403bbc98
up_dump_register: x22:  0x0                 x23:  0x0
up_dump_register: x24:  0x0                 x25:  0x0
up_dump_register: x26:  0x0                 x27:  0x0
up_dump_register: x28:  0x0                 x29:  0x403f3b20
up_dump_register: x30:  0x402a6a9c        
up_dump_register: 
up_dump_register: STATUS Registers:
up_dump_register: SPSR:      0x60000005        
up_dump_register: ELR:       0x402a6aa8        
up_dump_register: SP_EL0:    0x403f3870        
up_dump_register: SP_ELX:    0x403f3b20        
up_dump_register: EXE_DEPTH: 0xfffffffffffffffd
dump_tasks:    PID GROUP PRI POLICY   TYPE    NPX STATE   EVENT      SIGMASK          STACKBASE  STACKSIZE      USED   FILLED    COMMAND
dump_tasks:   ----   --- --- -------- ------- --- ------- ---------- ---------------- 0x403e9000      4096       320     7.8%    irq
dump_task:       0     0   0 FIFO     Kthread -   Ready              0000000000000000 0x403ea010      8176      1488    18.1%    Idle_Task
dump_task:       1     0 192 RR       Kthread -   Waiting Semaphore  0000000000000000 0x403ec8e0      8112      1008    12.4%    hpwork 0x403c64d8 0x403c6520
dump_task:       2     2 100 RR       Task    -   Waiting Semaphore  0000000000000000 0x403eef70      8144      2160    26.5%    nsh_main
dump_task:       3     3 100 RR       Task    -   Running            0000000000000000 0x403f1bd0      8144      2512    30.8%    hello

Of course, if you use the labeled p1 pointer to access, this error will not be reported.

@github-actions github-actions bot added Arch: arm64 Issues related to ARM64 (64-bit) architecture Board: arm64 Size: M The size of the change in this PR is medium labels Nov 29, 2024
@nuttxpr
Copy link

nuttxpr commented Nov 29, 2024

[Experimental Bot, please feedback here]

This PR description does not fully meet the NuttX requirements, although it's closer than many. Here's a breakdown:

Strengths:

  • Clear Summary of Functionality: The summary explains what MTE is and what this PR adds. The links to external documentation are helpful.
  • Impact Section Addresses Key Points: It identifies the limited hardware support (qemu only) and acknowledges future expansion of the feature.
  • Testing Section Provides Specific Instructions: The instructions for running qemu with MTE enabled and the test application are good. The output showing the expected failure (tag check exception) is also helpful.

Weaknesses:

  • Summary Lacks Conciseness and Focus: While the explanation of MTE is good context, the summary needs to be more concise. Focus on what changed in the code, not a general description of MTE. Mention the specific files modified (which is currently in the impact section).
  • Impact Section Lacks "YES/NO" Responses: It describes the impact but omits the explicit YES/NO answers requested by the template. This makes it harder to quickly assess the scope of the changes.
  • Testing Lacks "Before" Logs: The template specifically requests "Testing logs before change." While there's output after the change that shows the expected failure, there are no logs showing behavior before the change. This makes it difficult to assess the actual impact of the PR. What should happen on a system without MTE support? Did the system gracefully ignore MTE instructions or crash differently?
  • Missing Build Host Information: The Testing section is missing details about the build host environment.

Recommendations for Improvement:

  1. Rewrite the Summary: Example: "This PR adds initial support for the ARMv8.5 Memory Tagging Extension (MTE) to the NuttX kernel, specifically for the QEMU arm64 platform. Changes were made to arch/arm64/src/armv8-m/ and the QEMU configuration. This includes a new mte defconfig and modifications to support QEMU's 'max' CPU."
  2. Add YES/NO Responses to Impact: Explicitly answer each question with YES/NO, followed by a brief explanation if YES. For instance:
    • Impact on user: YES (Users can now enable MTE support when running NuttX on QEMU with a compatible CPU model.)
    • Impact on build: YES (A new defconfig mte has been added for arm64/qemu.)
    • ...and so on for all impact categories.
  3. Provide "Before" Test Logs: Show the output of the test application (or a similar relevant test) on a system without MTE support compiled in.
  4. Include Build Host Details: Specify the build host OS, CPU architecture, and compiler version used for testing.

By addressing these weaknesses, the PR description will be much more complete and comply with the NuttX requirements. This will help reviewers quickly understand and evaluate the changes.

@W-M-R W-M-R force-pushed the hwasan branch 8 times, most recently from ee801dc to 1cf29fb Compare November 29, 2024 06:35
@acassis
Copy link
Contributor

acassis commented Nov 30, 2024

@W-M-R it is important to have Documentation/ about MTE. Also the board profile should be included at https://nuttx.apache.org/docs/latest/platforms/arm64/qemu/boards/qemu-armv8a/index.html (it needs to be converted to RST format)

@github-actions github-actions bot added the Area: Documentation Improvements or additions to documentation label Dec 2, 2024
For details, please refer to the kernel's introduction to this at "https://docs.kernel.org/arch/arm64/memory-tagging-extension.html" and Android's introduction to this at "https://source.android.com/docs/security/test/memory-safety/arm-mte"

Of course, there is also the following detailed principle introduction
https://developer.arm.com/-/media/Arm%20Developer%20Community/PDF/Arm_Memory_Tagging_Extension_Whitepaper.pdf

The modification of this patch is only to merge the simplest MTE function support. In the future, the MTE function will be integrated into the kernel to a greater extent, for example, hardware MTE Kasan will be supported in the future.

Signed-off-by: wangmingrong1 <[email protected]>
@acassis acassis merged commit 9555f9f into apache:master Dec 2, 2024
13 checks passed
* Private Functions
****************************************************************************/

static int arm64_mte_is_support(void)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int->bool

@@ -159,6 +159,11 @@
#define TCR_KASAN_SW_FLAGS 0
#endif

#ifdef CONFIG_ARM64_MTE
#define TCR_MTE_FLAGS (TCR_TCMA1 | TCR_TBI0 | TCR_TBI1 | TCR_ASID_8)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add space from line 135 to line 164

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: arm64 Issues related to ARM64 (64-bit) architecture Area: Documentation Improvements or additions to documentation Board: arm64 Size: M The size of the change in this PR is medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants