Commit c543053a authored by Charles Jacobsen's avatar Charles Jacobsen Committed by Vikram Narayanan

test-v2: RAM map tests pass.

Couple duplicate memory interval tree inserts/deletes were leading
to some use-after-frees/page faults. Cleaned that up. Added some
resource tree debug code along the way.

Also, caught something subtle (noted in code). I didn't consider
the following scenario: Heap tries to allocate pages; the allocator
notices it needs to bring in more fresh pages, and notifies the
heap (via a callback); the heap allocs the fresh pages (from the
microkernel), maps them, and inserts those pages into the memory interval
tree; the memory interval tree kmallocs a tree node; kmalloc
calls back into the heap to grow a slab cache.

That last bit could be a potential problem (recursive call back
into the heap before we finish the original call). Lucky for me,
I designed the heap/allocator so that (1) the pages from the first
call are already marked as in use (not on a free list); (2) the
fresh pages are mapped first *before* inserting the corresponding
cptr into the memory interval tree.

The Linux kernel deals with these same recursive issues (they
resolve them using special GFP_ flags so that you don't get
recursion). In my case, the recursion is risky, but works.
parent 73de8d7f
......@@ -353,6 +353,13 @@ int lcd_arch_ept_gpa_to_hpa(struct lcd_arch *lcd, gpa_t ga, hpa_t *ha_out);
* a.
*/
int lcd_arch_set_pc(struct lcd_arch *lcd_arch, gva_t a);
/**
* Read LCD's %rip
*/
static inline u64 lcd_arch_get_pc(struct lcd_arch *lcd)
{
return lcd->regs[LCD_ARCH_REGS_RIP];
}
/**
* Set the lcd's stack pointer to the guest virtual address
* a.
......
......@@ -2852,6 +2852,9 @@ static void __noclone vmx_enter(struct lcd_arch *lcd_arch)
clear_non_root();
lcd_arch->regs[LCD_ARCH_REGS_RIP] = vmcs_readl(GUEST_RIP);
lcd_arch->regs[LCD_ARCH_REGS_RSP] = vmcs_readl(GUEST_RSP);
if (unlikely(lcd_arch->fail)) {
/*
* See Intel SDM V3 30.4 for error codes
......
......@@ -105,6 +105,32 @@ void lcd_resource_tree_remove(struct lcd_resource_tree *t,
interval_tree_remove(&n->it_node, &t->root);
}
void lcd_resource_tree_dump(struct lcd_resource_tree *t)
{
struct lcd_resource_node *cursor;
LIBLCD_MSG(" DUMPING RESOURCE TREE 0x%p:", t);
cursor = lcd_resource_tree_first(t);
while (cursor) {
printk(" node:\n");
printk(" start: 0x%lx\n",
lcd_resource_node_start(cursor));
printk(" last: 0x%lx\n",
lcd_resource_node_last(cursor));
printk(" size: 0x%lx\n",
lcd_resource_node_size(cursor));
printk(" cptr: 0x%lx\n",
cptr_val(lcd_resource_node_cptr(cursor)));
printk(" flags: 0x%x\n",
lcd_resource_node_flags(cursor));
cursor = lcd_resource_tree_next(cursor);
}
LIBLCD_MSG(" END RESOURCE TREE DUMP");
}
/* EXPORTS -------------------------------------------------- */
EXPORT_SYMBOL(lcd_resource_tree_init);
......@@ -113,3 +139,4 @@ EXPORT_SYMBOL(lcd_resource_tree_search);
EXPORT_SYMBOL(lcd_resource_tree_first);
EXPORT_SYMBOL(lcd_resource_tree_next);
EXPORT_SYMBOL(lcd_resource_tree_remove);
EXPORT_SYMBOL(lcd_resource_tree_dump);
......@@ -51,6 +51,10 @@ int __liblcd_mem_itree_insert(gpa_t start, unsigned long size,
* @n is no longer a valid pointer after this call.
*/
void __liblcd_mem_itree_delete(struct lcd_resource_node *n);
/**
* __liblcd_mem_itree_dump -- Print memory itree to console, for debugging
*/
void __liblcd_mem_itree_dump(void);
/*
* HEAP --------------------------------------------------
......
......@@ -166,4 +166,10 @@ lcd_resource_tree_next(struct lcd_resource_node *n);
void lcd_resource_tree_remove(struct lcd_resource_tree *t,
struct lcd_resource_node *n);
/**
* lcd_resource_tree_dump -- Print resource tree to console, for debugging
* @t: tree to dump
*/
void lcd_resource_tree_dump(struct lcd_resource_tree *t);
#endif /* LCD_DOMAINS_RESOURCE_TREE_H */
......@@ -126,10 +126,6 @@ static void __do_one_heap_free(struct lcd_resource_node *n)
* Unmap from guest physical
*/
_lcd_munmap(pages, base);
/*
* Remove from resource tree
*/
__liblcd_mem_itree_delete(n);
/*
* Free pages from host
*/
......@@ -489,7 +485,7 @@ static int setup_struct_page_array(void)
heap_page_array = (void *)gva_val(heap_page_block_to_addr(pb));
memset(heap_page_array,
0,
(1 << (order + PAGE_SHIFT)));
(1UL << (order + PAGE_SHIFT)));
return 0;
......
......@@ -44,6 +44,16 @@ static struct lcd_resource_node *alloc_itree_node(void)
struct lcd_resource_node *n;
if (likely(mem_itree_booted)) {
/*
* XXX: Be careful here. The heap may be calling into
* here when it is growing itself (alloc'ing fresh
* pages, and inserting the cptr into the resource tree).
* This call to kzalloc may recursively lead *back into*
* the heap to grow a slab cache for kmalloc. This
* recursion is OK because of how the heap maps the
* new pages (it maps them first *before* inserting them
* into the mem itree). See ram_map.c:_lcd_mmap.
*/
n = kzalloc(sizeof(struct lcd_resource_node), GFP_KERNEL);
if (!n) {
LIBLCD_ERR("kmalloc failed");
......@@ -89,6 +99,11 @@ void __liblcd_mem_itree_delete(struct lcd_resource_node *n)
free_itree_node(n);
}
void __liblcd_mem_itree_dump(void)
{
lcd_resource_tree_dump(&itree);
}
int lcd_phys_to_resource_node(gpa_t paddr, struct lcd_resource_node **n_out)
{
struct lcd_resource_node *n;
......
......@@ -23,6 +23,16 @@ int _lcd_mmap(cptr_t mo, unsigned int order, gpa_t base)
{
int ret;
/*
* BEWARE: This code is a bit fragile. You must do the actual
* map *before* inserting into the memory interval tree. This
* is because the mem itree code uses kmalloc (after we've
* booted). Scenario: the heap is calling this function to map
* fresh pages; mem itree is going to call kmalloc before
* this function returns; kmalloc may need to grow the slab
* cache, which leads into the heap again; but if we've alloc'd
* and mapped the pages, it should all be OK. Just a bit
* of scary and risky recursion.
*
* Do low level syscall to map memory object
*/
ret = lcd_syscall_mmap(mo, base);
......@@ -90,6 +100,7 @@ ram_alloc_map_metadata_memory(const struct lcd_page_allocator_cbs *cbs,
ret = -ENOMEM;
goto fail1;
}
return 0;
fail1:
......@@ -151,25 +162,11 @@ static int do_map_into_phys(cptr_t pages, unsigned int order, gpa_t *base_out)
LIBLCD_ERR("failed to map in guest physical");
goto fail2;
}
/*
* Insert address range into tree so we can do address -> cptr
* translation.
*/
ret = __liblcd_mem_itree_insert(
addr,
(1UL << (order + PAGE_SHIFT)),
pages);
if (ret) {
LIBLCD_ERR("failed to insert in mem itree");
goto fail3;
}
*base_out = addr;
return 0;
fail3:
_lcd_munmap(pages, addr);
fail2:
lcd_page_allocator_free(ram_map_allocator, pb, order);
fail1:
......@@ -199,6 +196,11 @@ static void do_unmap_from_phys(gpa_t base, unsigned int order)
int ret;
struct lcd_resource_node *n;
cptr_t pages;
/*
* Adjust order so that it's >= LCD_RAM_MAP_MIN_ORDER
*/
if (order < LCD_RAM_MAP_MIN_ORDER)
order = LCD_RAM_MAP_MIN_ORDER;
/*
* Resolve address to resource node
*/
......@@ -208,10 +210,6 @@ static void do_unmap_from_phys(gpa_t base, unsigned int order)
return;
}
pages = n->cptr;
/*
* Remove from tree
*/
__liblcd_mem_itree_delete(n);
/*
* Free address block from RAM region
*/
......
......@@ -255,14 +255,15 @@ static int ram_map_tests(void)
unsigned int alloc_order[10] = { 5, 9, 3, 2, 8, 7, 1, 0, 4, 6 };
gva_t gvas[10];
unsigned int i, j, k, n;
unsigned int order;
int ret;
unsigned char *ptr;
/*
* Low level allocs
*/
for (i = 0; i < 10; i++) {
ret = _lcd_alloc_pages(&pages[alloc_order[i]],
0, orders[alloc_order[i]]);
ret = _lcd_alloc_pages(0, orders[alloc_order[i]],
&pages[alloc_order[i]]);
if (ret) {
LIBLCD_ERR("low level alloc failed");
goto fail1;
......@@ -294,8 +295,9 @@ static int ram_map_tests(void)
*/
for (j = 0; j < 10; j++) {
ptr = (void *)gva_val(gvas[alloc_order[j]]);
for (n = 0; n < 10; n++) {
if (ptr[n] != orders[alloc_order[j]]) {
order = orders[alloc_order[j]];
for (n = 0; n < (1UL << (PAGE_SHIFT + order)); n++) {
if (ptr[n] != order) {
LIBLCD_ERR("bad byte at idx 0x%lx for order %d: expected %d, but found %d",
n, order, order, ptr[n]);
}
......@@ -364,7 +366,7 @@ static int __noreturn __init liblcd_test_lcd_init(void)
}
LIBLCD_MSG("kmem cache tests passed!");
ret = ram_map_test();
ret = ram_map_tests();
if (ret) {
LIBLCD_ERR("ram map test failed!");
goto out;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment