| Message ID | 20230412075134.21240-4-binbin.wu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: Add test cases for LAM | expand |
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@linux.intel.com> Reviewed-by: Chao Gao <chao.gao@intel.com> 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)? would you mind adding a comment in patch 2 to explain why the virtual addresses are kernel mode addresses? >+ 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? >+ >+ 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 >
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@linux.intel.com> > Reviewed-by: Chao Gao <chao.gao@intel.com> > > 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 >>
diff --git a/lib/x86/processor.h b/lib/x86/processor.h index 4bb8cd7..a181e0b 100644 --- a/lib/x86/processor.h +++ b/lib/x86/processor.h @@ -56,7 +56,9 @@ #define X86_CR3_PCID_MASK GENMASK(11, 0) #define X86_CR3_LAM_U57_BIT (61) +#define X86_CR3_LAM_U57 BIT_ULL(X86_CR3_LAM_U57_BIT) #define X86_CR3_LAM_U48_BIT (62) +#define X86_CR3_LAM_U48 BIT_ULL(X86_CR3_LAM_U48_BIT) #define X86_CR4_VME_BIT (0) #define X86_CR4_VME BIT(X86_CR4_VME_BIT) diff --git a/x86/lam.c b/x86/lam.c index 63c3fde..50bcdf5 100644 --- a/x86/lam.c +++ b/x86/lam.c @@ -18,9 +18,11 @@ #include "vm.h" #include "asm/io.h" #include "ioram.h" +#include "usermode.h" #define LAM57_MASK GENMASK_ULL(62, 57) #define LAM48_MASK GENMASK_ULL(62, 48) +#define CR3_LAM_BITS_MASK (X86_CR3_LAM_U48 | X86_CR3_LAM_U57) #define FLAGS_LAM_ACTIVE BIT_ULL(0) #define FLAGS_LA57 BIT_ULL(1) @@ -41,6 +43,16 @@ static inline bool lam_sup_active(void) return !!(read_cr4() & X86_CR4_LAM_SUP); } +static inline bool lam_u48_active(void) +{ + return (read_cr3() & CR3_LAM_BITS_MASK) == X86_CR3_LAM_U48; +} + +static inline bool lam_u57_active(void) +{ + return !!(read_cr3() & X86_CR3_LAM_U57); +} + /* According to LAM mode, set metadata in high bits */ static inline u64 set_metadata(u64 src, u64 metadata_mask) { @@ -92,6 +104,7 @@ static void do_mov(void *mem) static u64 test_ptr(u64 arg1, u64 arg2, u64 arg3, u64 arg4) { bool lam_active = !!(arg1 & FLAGS_LAM_ACTIVE); + bool la_57 = !!(arg1 & FLAGS_LA57); u64 lam_mask = arg2; u64 *ptr = (u64 *)arg3; bool is_mmio = !!arg4; @@ -105,6 +118,17 @@ static u64 test_ptr(u64 arg1, u64 arg2, u64 arg3, u64 arg4) report(fault != lam_active,"Test tagged addr (%s)", is_mmio ? "MMIO" : "Memory"); + /* + * This test case is only triggered when LAM_U57 is active and 4-level + * paging is used. For the case, bit[56:47] aren't all 0 triggers #GP. + */ + if (lam_active && (lam_mask == LAM57_MASK) && !la_57) { + ptr = (u64 *)set_metadata((u64)ptr, LAM48_MASK); + fault = test_for_exception(GP_VECTOR, do_mov, ptr); + report(fault, "Test non-LAM-canonical addr (%s)", + is_mmio ? "MMIO" : "Memory"); + } + return 0; } @@ -221,6 +245,52 @@ static void test_lam_sup(bool has_lam, bool fep_available) test_invlpg(lam_mask, vaddr, true); } +static void test_lam_user_mode(bool has_lam, u64 lam_mask, u64 mem, u64 mmio) +{ + unsigned r; + bool raised_vector; + unsigned long cr3 = read_cr3() & ~CR3_LAM_BITS_MASK; + u64 flags = 0; + + if (is_la57()) + flags |= FLAGS_LA57; + + if (has_lam) { + if (lam_mask == LAM48_MASK) { + r = write_cr3_safe(cr3 | X86_CR3_LAM_U48); + report((r == 0) && lam_u48_active(), "Set LAM_U48"); + } else { + r = write_cr3_safe(cr3 | X86_CR3_LAM_U57); + report((r == 0) && lam_u57_active(), "Set LAM_U57"); + } + } + if (lam_u48_active() || lam_u57_active()) + flags |= FLAGS_LAM_ACTIVE; + + run_in_user((usermode_func)test_ptr, GP_VECTOR, flags, lam_mask, mem, + false, &raised_vector); + run_in_user((usermode_func)test_ptr, GP_VECTOR, flags, lam_mask, mmio, + true, &raised_vector); +} + +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. + */ + paddr = virt_to_phys(alloc_page()); + install_page((void *)cr3, paddr, (void *)paddr); + install_page((void *)cr3, IORAM_BASE_PHYS, (void *)IORAM_BASE_PHYS); + + 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(); }
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@linux.intel.com> --- lib/x86/processor.h | 2 ++ x86/lam.c | 71 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+)