On 4/21/2023 1:06 PM, Chao Gao wrote:
On Wed, Apr 12, 2023 at 03:51:33PM +0800, Binbin Wu wrote:
This unit test covers:
1. CR3 LAM bits toggles.
2. Memory/MMIO access with user mode address containing LAM metadata.
Signed-off-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
Reviewed-by: Chao Gao <chao.gao@xxxxxxxxx>
two nits below:
---
+static void test_lam_user(bool has_lam)
+{
+ phys_addr_t paddr;
+ unsigned long cr3 = read_cr3();
+
+ /*
+ * The physical address width is within 36 bits, so that using identical
+ * mapping, the linear address will be considered as user mode address
+ * from the view of LAM.
+ */
Why 36 bits (i.e., 64G)?
KUT memory allocator supports 4 Memory Areas:
AREA_NORMAL, AREA_HIGH, AREA_LOW and AREA_LOWEST.
AREA_NORMAL has the maximum address width, which is 36 bits.
How about change the comment to this:
+ /*
+ * The physical address width supported by KUT memory allocator is within 36 bits,
+ * so that using identical mapping, the linear address will be considered as user
+ * mode address from the view of LAM.
+ */
would you mind adding a comment in patch 2 to explain why the virtual
addresses are kernel mode addresses?
OK. Will add the following comments:
/*
* KUT initializes vfree_top to 0 for X86_64, and each virtual address
* allocation decreases the size from vfree_top. It's guaranteed that
* the return value of alloc_vpage() is considered as kernel mode
* address and canonical since only a small mount virtual address range
* is allocated in this test.
*/
+ paddr = virt_to_phys(alloc_page());
+ install_page((void *)cr3, paddr, (void *)paddr);
+ install_page((void *)cr3, IORAM_BASE_PHYS, (void *)IORAM_BASE_PHYS);
are the two lines necessary?
The two lines are not necessary.
Check setup_mmu(), it already sets up the identical mapping for avialble
physcial memory/MMIO.
Will remove them.
+
+ test_lam_user_mode(has_lam, LAM48_MASK, paddr, IORAM_BASE_PHYS);
+ test_lam_user_mode(has_lam, LAM57_MASK, paddr, IORAM_BASE_PHYS);
+}
+
int main(int ac, char **av)
{
bool has_lam;
@@ -239,6 +309,7 @@ int main(int ac, char **av)
"use kvm.force_emulation_prefix=1 to enable\n");
test_lam_sup(has_lam, fep_available);
+ test_lam_user(has_lam);
return report_summary();
}
--
2.25.1
