Skip to content

Conversation

@npitre
Copy link

@npitre npitre commented May 13, 2024

Looking at my own code I couldn't convince myself it was OK.
So I wrote a test and guess what? The code was broken... of course.

Here's the fix which required a slight rethinking of the page table
usage tracking. And more tests.

@zephyrbot zephyrbot added the area: ARM64 ARM (64-bit) Architecture label May 13, 2024
Existing code confused table usage and table reference counts together.
This obviously doesn't work. A table with one reference to it and one
populated PTE is not the same as a table with 2 references to it and
no PTe in use.

So split the two concepts and adjust the code accordingly. A page needs
to have its PTE usage count drop to zero before the last reference is
released. When both counts are 0 then the page is free.

Signed-off-by: Nicolas Pitre <[email protected]>
@npitre npitre force-pushed the arm64mmtest branch 2 times, most recently from 91d2f9c to 452be2a Compare May 14, 2024 15:56
This code was never formally tested before... and without the preceding
commit it obviously didn't work either.

Signed-off-by: Nicolas Pitre <[email protected]>
@npitre npitre requested a review from nashif June 12, 2024 18:06
@npitre npitre added this to the v3.7.0 milestone Jun 12, 2024
@nashif nashif merged commit 6cc9d7e into zephyrproject-rtos:main Jun 13, 2024
@nashif
Copy link
Member

nashif commented Jun 13, 2024

@npitre this f7e1164 is now causing a test to fail in main:

west build -p -b qemu_cortex_a53 tests/kernel/mem_protect/mem_map -T kernel.memory_protection.mem_map -t run

E: Current thread: 0x4003f380 (test_k_mem_unmap_phys_bare)
Caught system error -- reason 0
 PASS - test_k_mem_unmap_phys_bare in 0.001 seconds
===================================================================
TESTSUITE mem_map succeeded
Running TESTSUITE mem_map_api
===================================================================
START - test_k_mem_map_exhaustion
Free memory: 133640192
E: insufficient virtual address space (requested 12288)
Mapped 632 pages
Free memory now: 131051520
ASSERTION FAIL [is_table_desc(src_table[i], level)] @ WEST_TOPDIR/zephyr/arch/arm64/core/mmu.c:486
	can't have partial block pte here
E: ELR_ELn: 0x0000000040002ca0
E: ESR_ELn: 0x0000000056000002
E:   EC:  0x15 (Unknown)
E:   IL:  0x1
E:   ISS: 0x2
E: TPIDRRO: 0x01000000400401d8
E: x0:  0x00000000400401d8  x1:  0x00000000000001e6
E: x2:  0x0000000000000090  x3:  0xffffff80ffffffc8
E: x4:  0x0000000000000000  x5:  0x000000000000002d
E: x6:  0x0000000000000000  x7:  0x0000000000000004
E: x8:  0x0000000000000004  x9:  0x0000000040006204
E: x10: 0x0000000040015ec0  x11: 0x0000000000000000
E: x12: 0x0000000000000000  x13: 0x000000009600004f
E: x14: 0x0000000040088f00  x15: 0x000000004003fa80
E: x16: 0x0000000040007734  x17: 0x00000000400401d8
E: x18: 0x0000000000000040  lr:  0x0000000040005364
E: >>> ZEPHYR FATAL ERROR 4: Kernel panic on CPU 0
E: Current thread: 0x4003f380 (test_k_mem_map_exhaustion)
Caught system error -- reason 4
Unexpected fault during test
===================================================================
PROJECT EXECUTION FAILED

@nashif
Copy link
Member

nashif commented Jun 13, 2024

It is probably worth it adding qemu_cortex_a53 as an integration platform in the test.

@dcpleung
Copy link
Member

It seems the PTE is of value zero. So this is the case of table being referenced but PTE not in use (as described in the commit message for usage count change). If a test for invalid PTE is added before the assert to continue, the test passed. Though I am not sure if that is the correct fix.

@npitre
Copy link
Author

npitre commented Jun 13, 2024

Needed fix in #74269

@npitre npitre deleted the arm64mmtest branch June 20, 2024 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ARM64 ARM (64-bit) Architecture

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants