xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* mini-os: arm: grant mapping
@ 2014-09-22 11:01 Dave Scott
  2014-09-22 11:09 ` Ian Campbell
  2014-09-22 11:25 ` Samuel Thibault
  0 siblings, 2 replies; 5+ messages in thread
From: Dave Scott @ 2014-09-22 11:01 UTC (permalink / raw)
  To: xen-devel@lists.xenproject.org
  Cc: Samuel Thibault, Thomas Leonard, Stefano Stabellini,
	Anil Madhavapeddy

Hi,

I’ve been playing with grant mapping on arm mini-os. Specifically I’ve made vchan ‘clients' (which act as backends, mapping frontend aka ‘server’ pages) work with some extremely hacky patches and I’m curious to know what you think and especially, how it should really be done.

Current arch/arm/mm.c has a missing populate_ondemand function:

unsigned long allocate_ondemand(unsigned long n, unsigned long alignment)
{
    // FIXME
    BUG();
} 

I notice the arch/x86/mm.c version looks for runs of free frames in the page table. I notice that gntmap_munmap doesn’t have any corresponding call to free so I assume the act of unmapping the grant causes Xen to mark the pages as free for you — is this correct? Would a similar trick work on arm or would we have to manage the memory explicitly ourselves?

I also noticed that the x86 version initialises the “demand mapping area” from “max_pfn” in mm.c. I’m a bit suspicious about this on arm since the grant table is being mapped in somewhere pre-ordained in the device tree (If I’m reading it correctly) and there could be an overlap (if we’re unlucky).

Anyway, as an experiment I hade a trivial memory allocator for a small number of pages which tracks used/free (and which doesn’t even support freeing memory):

diff --git a/extras/mini-os/arch/arm/mm.c b/extras/mini-os/arch/arm/mm.c
index 813a34e..86dd43c 100644
--- a/extras/mini-os/arch/arm/mm.c
+++ b/extras/mini-os/arch/arm/mm.c
@@ -9,12 +9,6 @@ void arm_map_extra_stack_section(int);
 
 uint32_t physical_address_offset;
 
-unsigned long allocate_ondemand(unsigned long n, unsigned long alignment)
-{
-    // FIXME
-    BUG();
-}
-
 void arch_init_mm(unsigned long *start_pfn_p, unsigned long *max_pfn_p)
 {
     int memory;
@@ -86,8 +80,59 @@ void arch_init_p2m(unsigned long max_pfn)
 {
 }
 
+static unsigned long demand_map_area_start;
+#define DEMAND_MAP_PAGES 160 /* 640 KiB is enough for anyone */
+
+// FIXME: the x86 backend doesn't track free/in-use explicitly
+// because it can read the pagetable. Can we do that too?
+
+// FIXME: because of the above there isn't a way to mark a page
+// as free, so we leak.
+
+/* One whole byte to signal free (true) or in-use (false) */
+static char demand_map_area_free[DEMAND_MAP_PAGES];
+
 void arch_init_demand_mapping_area(unsigned long cur_pfn)
 {
+    int i;
+    cur_pfn++;
+    printk("Next pfn = 0x%lx (== %p VA)\n", cur_pfn, pfn_to_virt(cur_pfn));
+
+    demand_map_area_start = (unsigned long) pfn_to_virt(cur_pfn);
+    cur_pfn += DEMAND_MAP_PAGES;
+    printk("Demand map memory at 0x%lx-0x%p.\n", 
+           demand_map_area_start, pfn_to_virt(cur_pfn));
+    for (i = 0; i < DEMAND_MAP_PAGES; i++ ) {
+        demand_map_area_free[i] = 1;
+    }
+}
+
+unsigned long allocate_ondemand(unsigned long n, unsigned long alignment)
+{
+    int i, j, found;
+    unsigned long addr;
+    if (alignment != 1) {
+        printk("allocate_ondemand: we only support alignment = 1 (was: %ld)\n", alignment);
+        BUG();
+    }
+    for (i = 0, j = 0; i < DEMAND_MAP_PAGES - n; i += j+1 ) {
+        /* Is the contiguous section free? */
+       found = 0;
+        for (j = 0; j < n; j++ )
+          if (!demand_map_area_free[i+j]) {
+              found = 1;
+             break;
+         }
+       if (found) continue;
+        /* It's free, so allocate it. */
+       for (j = 0; j < n; j++ )
+          demand_map_area_free[i+j] = 0;
+        addr = demand_map_area_start + i * PAGE_SIZE;
+        printk("allocate_ondemand(%ld, %ld) returning %lx\n", n, alignment, addr);
+        return addr;
+    }
+    printk("allocate_ondemand: demand map area is full.\n");
+    BUG();
 }
 
 /* Get Xen's suggested physical page assignments for the grant table. */

With this it still didn’t work: the grant map hypercall succeeded but the data just wasn’t there. I had a read of the hypercall doc which describes the address as

 *  2. If GNTMAP_host_map is specified then a mapping will be added at
 *     either a host virtual address in the current address space, or at
 *     a PTE at the specified machine address.

I’m not totally sure what a ‘host virtual address’ refers to but I notice the address we’re using in the call is the regular virtual address (the one we can simply deref and read the data). As an experiment I wrapped it via the to_phys macro and suddenly it worked:

diff --git a/extras/mini-os/gntmap.c b/extras/mini-os/gntmap.c
index f6ab3ad..328d953 100644
--- a/extras/mini-os/gntmap.c
+++ b/extras/mini-os/gntmap.c
@@ -205,11 +205,12 @@ gntmap_map_grant_refs(struct gntmap *map,
     if (addr == 0)
         return NULL;
 
+    // FIXME: to_phys below is probably wrong on x86
     for (i = 0; i < count; i++) {
         ent = gntmap_find_free_entry(map);
         if (ent == NULL ||
             _gntmap_map_grant_ref(ent,
-                                  addr + PAGE_SIZE * i,
+                                  to_phys(addr + PAGE_SIZE * i),
                                   domids[i * domids_stride],
                                   refs[i],
                                   writable) != 0) {


where in arch/arm/include/arch_mm.h:

#define to_phys(x)                 (((paddr_t)(x)+physical_address_offset) & 0xffffffff)

Of course this might be the wrong thing to do— it might break x86 where:

#define VIRT_START                 ((unsigned long)&_text)

#define to_phys(x)                 ((unsigned long)(x)-VIRT_START)

I’ve not had a chance to see whether this breaks x86. Certainly x86 worked before my patch.

Anyway I’d appreciate any feedback you may have :-) Apart from this (and a workaround-able problem involving timers) minios on arm is working nicely for me — thanks for that!

Cheers,
Dave

^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-09-22 11:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-22 11:01 mini-os: arm: grant mapping Dave Scott
2014-09-22 11:09 ` Ian Campbell
2014-09-22 11:31   ` Samuel Thibault
2014-09-22 11:34     ` Ian Campbell
2014-09-22 11:25 ` Samuel Thibault

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).