[PATCH v2 00/10] mini-os: add missing PVH features

[PATCH v2 00/10] mini-os: add missing PVH features

[Juergen Gross]

Mini-OS in PVH mode is missing some features, especially in the areas
of ballooning and grant tables.

With this series I am able to run Xenstore stubdom in PVH mode.

Changes in V2:
- multiple comments addressed

Juergen Gross (10):
  mini-os: split e820 map handling into new source file
  mini-os: sort and sanitize e820 memory map
  mini-os: don't assume contiguous RAM when initializing in PVH mode
  mini-os: respect memory map when ballooning up
  mini-os: don't repeat definition available via header file
  mini-os: add memory map service functions
  mini-os: move x86 specific gnttab coding into arch/x86/gnttab.c
  mini-os: add proper pvh grant table handling
  mini-os: prepare grantmap entry interface for use by PVH mode
  mini-os: modify grant mappings to work in PVH mode

 Makefile           |   1 +
 arch/arm/mm.c      |  11 +-
 arch/x86/balloon.c |   4 +-
 arch/x86/gnttab.c  | 109 +++++++++++++
 arch/x86/mm.c      | 121 +--------------
 arch/x86/setup.c   |   8 +-
 balloon.c          |  33 ++--
 e820.c             | 376 +++++++++++++++++++++++++++++++++++++++++++++
 gntmap.c           | 125 +++++++++------
 include/balloon.h  |   5 +-
 include/e820.h     |  11 ++
 include/gntmap.h   |   1 +
 mm.c               |   7 +-
 13 files changed, 615 insertions(+), 197 deletions(-)
 create mode 100644 arch/x86/gnttab.c
 create mode 100644 e820.c

-- 
2.26.2

[PATCH v2 01/10] mini-os: split e820 map handling into new source file

[Juergen Gross]

Introduce e820.c containing all the E820 memory map handling.

No functional change.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 Makefile       |   1 +
 arch/arm/mm.c  |   8 ----
 arch/x86/mm.c  |  70 +----------------------------
 e820.c         | 119 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/e820.h |   6 +++
 5 files changed, 128 insertions(+), 76 deletions(-)
 create mode 100644 e820.c

diff --git a/Makefile b/Makefile
index 4b76b55..06b60fc 100644
--- a/Makefile
+++ b/Makefile
@@ -41,6 +41,7 @@ src-$(CONFIG_TPMFRONT) += tpmfront.c
 src-$(CONFIG_TPM_TIS) += tpm_tis.c
 src-$(CONFIG_TPMBACK) += tpmback.c
 src-y += daytime.c
+src-y += e820.c
 src-y += events.c
 src-$(CONFIG_FBFRONT) += fbfront.c
 src-y += gntmap.c
diff --git a/arch/arm/mm.c b/arch/arm/mm.c
index f806c9f..9068166 100644
--- a/arch/arm/mm.c
+++ b/arch/arm/mm.c
@@ -7,14 +7,6 @@
 #include <lib.h>
 
 uint32_t physical_address_offset;
-struct e820entry e820_map[1] = {
-    {
-        .addr = 0,
-        .size = ULONG_MAX - 1,
-        .type = E820_RAM
-    }
-};
-unsigned e820_entries = 1;
 
 unsigned long allocate_ondemand(unsigned long n, unsigned long alignment)
 {
diff --git a/arch/x86/mm.c b/arch/x86/mm.c
index 8ba14a5..8df93da 100644
--- a/arch/x86/mm.c
+++ b/arch/x86/mm.c
@@ -64,15 +64,6 @@ extern char stack[];
 extern void page_walk(unsigned long va);
 
 #ifdef CONFIG_PARAVIRT
-struct e820entry e820_map[1] = {
-    {
-        .addr = 0,
-        .size = ULONG_MAX - 1,
-        .type = E820_RAM
-    }
-};
-unsigned e820_entries = 1;
-
 void arch_mm_preinit(void *p)
 {
     start_info_t *si = p;
@@ -112,25 +103,11 @@ desc_ptr idt_ptr =
     .base = (unsigned long)&idt,
 };
 
-struct e820entry e820_map[E820_MAX];
-unsigned e820_entries;
-
-static char *e820_types[E820_TYPES] = {
-    [E820_RAM]      = "RAM",
-    [E820_RESERVED] = "Reserved",
-    [E820_ACPI]     = "ACPI",
-    [E820_NVS]      = "NVS",
-    [E820_UNUSABLE] = "Unusable",
-    [E820_PMEM]     = "PMEM"
-};
-
 void arch_mm_preinit(void *p)
 {
     long ret;
     domid_t domid = DOMID_SELF;
-    struct xen_memory_map memmap;
-    int i;
-    unsigned long pfn, max = 0;
+    unsigned long max;
 
     pt_base = page_table_base;
     first_free_pfn = PFN_UP(to_phys(&_end));
@@ -142,53 +119,10 @@ void arch_mm_preinit(void *p)
     }
     last_free_pfn = ret;
 
-    memmap.nr_entries = E820_MAX;
-    set_xen_guest_handle(memmap.buffer, e820_map);
-    ret = HYPERVISOR_memory_op(XENMEM_memory_map, &memmap);
-    if ( ret < 0 )
-    {
-        xprintk("could not get memory map\n");
-        do_exit();
-    }
-    e820_entries = memmap.nr_entries;
-
-    for ( i = 0; i < e820_entries; i++ )
-    {
-        if ( e820_map[i].type != E820_RAM )
-            continue;
-        pfn = (e820_map[i].addr + e820_map[i].size) >> PAGE_SHIFT;
-        if ( pfn > max )
-            max = pfn;
-    }
-
+    max = e820_get_maxpfn();
     if ( max < last_free_pfn )
         last_free_pfn = max;
 }
-
-void arch_print_memmap(void)
-{
-    int i;
-    unsigned long from, to;
-    char *type;
-    char buf[12];
-
-    printk("Memory map:\n");
-    for ( i = 0; i < e820_entries; i++ )
-    {
-        if ( e820_map[i].type >= E820_TYPES || !e820_types[e820_map[i].type] )
-        {
-            snprintf(buf, sizeof(buf), "%8x", e820_map[i].type);
-            type = buf;
-        }
-        else
-        {
-            type = e820_types[e820_map[i].type];
-        }
-        from = e820_map[i].addr;
-        to = from + e820_map[i].size - 1;
-        printk("%012lx-%012lx: %s\n", from, to, type);
-    }
-}
 #endif
 
 /*
diff --git a/e820.c b/e820.c
new file mode 100644
index 0000000..2165280
--- /dev/null
+++ b/e820.c
@@ -0,0 +1,119 @@
+/* -*-  Mode:C; c-basic-offset:4; tab-width:4 -*-
+ *
+ * (C) 2021 - Juergen Gross, SUSE Software Solutions Germany GmbH
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include <mini-os/types.h>
+#include <mini-os/lib.h>
+#include <mini-os/console.h>
+#include <mini-os/os.h>
+#include <mini-os/posix/limits.h>
+#include <mini-os/e820.h>
+#include <xen/memory.h>
+
+#ifdef CONFIG_E820_TRIVIAL
+struct e820entry e820_map[1] = {
+    {
+        .addr = 0,
+        .size = ULONG_MAX - 1,
+        .type = E820_RAM
+    }
+};
+
+unsigned e820_entries = 1;
+
+static void e820_get_memmap(void)
+{
+}
+
+#else
+struct e820entry e820_map[E820_MAX];
+unsigned e820_entries;
+
+static char *e820_types[E820_TYPES] = {
+    [E820_RAM]      = "RAM",
+    [E820_RESERVED] = "Reserved",
+    [E820_ACPI]     = "ACPI",
+    [E820_NVS]      = "NVS",
+    [E820_UNUSABLE] = "Unusable",
+    [E820_PMEM]     = "PMEM"
+};
+
+static void e820_get_memmap(void)
+{
+    long ret;
+    struct xen_memory_map memmap;
+
+    memmap.nr_entries = E820_MAX;
+    set_xen_guest_handle(memmap.buffer, e820_map);
+    ret = HYPERVISOR_memory_op(XENMEM_memory_map, &memmap);
+    if ( ret < 0 )
+    {
+        xprintk("could not get memory map\n");
+        do_exit();
+    }
+    e820_entries = memmap.nr_entries;
+}
+
+void arch_print_memmap(void)
+{
+    int i;
+    unsigned long from, to;
+    char *type;
+    char buf[12];
+
+    printk("Memory map:\n");
+    for ( i = 0; i < e820_entries; i++ )
+    {
+        if ( e820_map[i].type >= E820_TYPES || !e820_types[e820_map[i].type] )
+        {
+            snprintf(buf, sizeof(buf), "%8x", e820_map[i].type);
+            type = buf;
+        }
+        else
+        {
+            type = e820_types[e820_map[i].type];
+        }
+        from = e820_map[i].addr;
+        to = from + e820_map[i].size - 1;
+        printk("%012lx-%012lx: %s\n", from, to, type);
+    }
+}
+#endif
+
+unsigned long e820_get_maxpfn(void)
+{
+    int i;
+    unsigned long pfn, max = 0;
+
+    e820_get_memmap();
+
+    for ( i = 0; i < e820_entries; i++ )
+    {
+        if ( e820_map[i].type != E820_RAM )
+            continue;
+        pfn = (e820_map[i].addr + e820_map[i].size) >> PAGE_SHIFT;
+        if ( pfn > max )
+            max = pfn;
+    }
+
+    return max;
+}
diff --git a/include/e820.h b/include/e820.h
index 920551c..af2129f 100644
--- a/include/e820.h
+++ b/include/e820.h
@@ -24,6 +24,10 @@
 #ifndef __E820_HEADER
 #define __E820_HEADER
 
+#if defined(__arm__) || defined(__aarch64__) || defined(CONFIG_PARAVIRT)
+#define CONFIG_E820_TRIVIAL
+#endif
+
 /* PC BIOS standard E820 types and structure. */
 #define E820_RAM          1
 #define E820_RESERVED     2
@@ -45,4 +49,6 @@ struct __packed e820entry {
 extern struct e820entry e820_map[];
 extern unsigned e820_entries;
 
+unsigned long e820_get_maxpfn(void);
+
 #endif /*__E820_HEADER*/
-- 
2.26.2

[PATCH v2 02/10] mini-os: sort and sanitize e820 memory map

[Juergen Gross]

Do some processing of the E820 memory map obtained from the hypervisor:

- align the entries to page boundaries
- sort the entries by their start address
- merge adjacent entries of same type

This is relevant for PVH mode only.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- correct page boundary rounding
- handle overlaps after rounding (Samuel Thibault)
- improve sorting (Samuel Thibault)
---
 e820.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 186 insertions(+)

diff --git a/e820.c b/e820.c
index 2165280..1770158 100644
--- a/e820.c
+++ b/e820.c
@@ -57,6 +57,190 @@ static char *e820_types[E820_TYPES] = {
     [E820_PMEM]     = "PMEM"
 };
 
+/*
+ * E820 type based bitmask for deciding how to round entries to page
+ * boundaries: A set bit means the type relates to a resource managed by
+ * Mini-OS (e.g. RAM), so rounding needs to be done to only include pages
+ * completely of the related type (narrowing). All other types need to be
+ * rounded to include all pages with parts of that type (widening).
+ */
+#define E820_NARROW ((1U << E820_RAM) | (1U << E820_NVS) | (1 << E820_PMEM))
+
+/* Private type used to mark a range temporarily as reserved (lowest prio). */
+#define E820_TMP_RESERVED    0
+
+static void e820_remove_entry(int idx)
+{
+    int i;
+
+    e820_entries--;
+    for ( i = idx; i < e820_entries; i++ )
+        e820_map[i] = e820_map[i + 1];
+}
+
+static void e820_insert_entry_at(int idx, unsigned long addr,
+                                 unsigned long size, unsigned int type)
+{
+    int i;
+
+    if ( e820_entries == E820_MAX )
+    {
+        xprintk("E820 memory map overflow\n");
+        do_exit();
+    }
+
+    e820_entries++;
+    for ( i = e820_entries - 1; i > idx; i-- )
+        e820_map[i] = e820_map[i - 1];
+
+    e820_map[idx].addr = addr;
+    e820_map[idx].size = size;
+    e820_map[idx].type = type;
+}
+
+static void e820_insert_entry(unsigned long addr, unsigned long size,
+                              unsigned int type)
+{
+    int i;
+
+    for ( i = 0; i < e820_entries && addr > e820_map[i].addr; i++ );
+
+    e820_insert_entry_at(i, addr, size, type);
+}
+
+static void e820_swap_entries(int idx1, int idx2)
+{
+    struct e820entry entry;
+
+    entry = e820_map[idx1];
+    e820_map[idx1] = e820_map[idx2];
+    e820_map[idx2] = entry;
+}
+
+/*
+ * Do a memory map sanitizing sweep:
+ * - sort the entries by start address
+ * - remove overlaps of entries (higher type value wins)
+ * - merge adjacent entries of same type
+ */
+static void e820_process_entries(void)
+{
+    int i, j;
+    unsigned long end, start;
+    unsigned int type;
+
+    /* Sort entries. */
+    for ( i = 1; i < e820_entries; i++ )
+        for ( j = i; j > 0 && e820_map[j - 1].addr > e820_map[j].addr; j-- )
+            e820_swap_entries(j - 1, j);
+
+    /* Handle overlapping entries (higher type values win). */
+    for ( i = 1; i < e820_entries; i++ )
+    {
+        if ( e820_map[i - 1].addr + e820_map[i - 1].size <= e820_map[i].addr )
+            continue;
+        if ( e820_map[i - 1].addr < e820_map[i].addr )
+        {
+            e820_insert_entry_at(i - 1, e820_map[i - 1].addr,
+                                 e820_map[i].addr - e820_map[i - 1].addr,
+                                 e820_map[i - 1].type);
+            e820_map[i].addr += e820_map[i - 1].size;
+            e820_map[i].size -= e820_map[i - 1].size;
+            i++;
+        }
+        if ( e820_map[i - 1].type < e820_map[i].type )
+            e820_swap_entries(i - 1, i);
+        if ( e820_map[i - 1].size >= e820_map[i].size )
+        {
+            e820_remove_entry(i);
+            i--;
+        }
+        else
+        {
+            start = e820_map[i].addr + e820_map[i - 1].size;
+            end = e820_map[i].addr + e820_map[i].size;
+            type = e820_map[i].type;
+            e820_remove_entry(i);
+            e820_insert_entry(start, end - start, type);
+        }
+    }
+
+    /* Merge adjacent entries. */
+    for ( i = 0; i < e820_entries - 1; i++ )
+    {
+        if ( e820_map[i].type == e820_map[i + 1].type &&
+             e820_map[i].addr + e820_map[i].size >= e820_map[i + 1].addr )
+        {
+            if ( e820_map[i].addr + e820_map[i].size <
+                 e820_map[i + 1].addr + e820_map[i + 1].size )
+            {
+                e820_map[i].size = e820_map[i + 1].addr - e820_map[i].addr +
+                                   e820_map[i + 1].size;
+            }
+            e820_remove_entry(i + 1);
+            i--;
+        }
+    }
+}
+
+/*
+ * Transform memory map into a well sorted map without any overlaps.
+ * - sort map entries by start address
+ * - handle overlaps
+ * - merge adjacent entries of same type (possibly removing boundary in the
+ *   middle of a page)
+ * - trim entries to page boundaries (depending on type either expanding
+ *   the entry or narrowing it down)
+ * - repeat first 3 sanitizing steps
+ * - make remaining temporarily reserved entries permanently reserved
+ */
+static void e820_sanitize(void)
+{
+    int i;
+    unsigned long end, start;
+
+    /* Sanitize memory map in current form. */
+    e820_process_entries();
+
+    /* Adjust map entries to page boundaries. */
+    for ( i = 0; i < e820_entries; i++ )
+    {
+        start = e820_map[i].addr;
+        end = start + e820_map[i].size;
+        if ( (1U << e820_map[i].type) & E820_NARROW )
+        {
+            if ( start & (PAGE_SIZE - 1) )
+            {
+                start = round_pgup(start);
+                e820_insert_entry_at(i, start - PAGE_SIZE, PAGE_SIZE,
+                                     E820_TMP_RESERVED);
+                i++;
+            }
+            if ( end & (PAGE_SIZE - 1) )
+            {
+                end = round_pgdown(end);
+                e820_insert_entry_at(i, end, PAGE_SIZE, E820_TMP_RESERVED);
+                i++;
+            }
+        }
+        else
+        {
+            start = round_pgdown(start);
+            end = round_pgup(end);
+        }
+        e820_map[i].addr = start;
+        e820_map[i].size = end - start;
+    }
+
+    /* Sanitize memory map (again). */
+    e820_process_entries();
+
+    /* Make remaining temporarily reserved entries permanently reserved. */
+    for ( i = 0; i < e820_entries; i++ )
+        if ( e820_map[i].type == E820_TMP_RESERVED )
+            e820_map[i].type = E820_RESERVED;
+}
+
 static void e820_get_memmap(void)
 {
     long ret;
@@ -71,6 +255,8 @@ static void e820_get_memmap(void)
         do_exit();
     }
     e820_entries = memmap.nr_entries;
+
+    e820_sanitize();
 }
 
 void arch_print_memmap(void)
-- 
2.26.2

[PATCH v2 03/10] mini-os: don't assume contiguous RAM when initializing in PVH mode

[Juergen Gross]

Sizing the available memory should respect memory holes, so look at
the memory map when setting the boundary for the memory allocator.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- rename "max" to "start" (Samuel Thibault)
---
 arch/x86/mm.c  |  6 +-----
 e820.c         | 14 ++++++++------
 include/e820.h |  2 +-
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/arch/x86/mm.c b/arch/x86/mm.c
index 8df93da..3bf6170 100644
--- a/arch/x86/mm.c
+++ b/arch/x86/mm.c
@@ -107,7 +107,6 @@ void arch_mm_preinit(void *p)
 {
     long ret;
     domid_t domid = DOMID_SELF;
-    unsigned long max;
 
     pt_base = page_table_base;
     first_free_pfn = PFN_UP(to_phys(&_end));
@@ -117,11 +116,8 @@ void arch_mm_preinit(void *p)
         xprintk("could not get memory size\n");
         do_exit();
     }
-    last_free_pfn = ret;
 
-    max = e820_get_maxpfn();
-    if ( max < last_free_pfn )
-        last_free_pfn = max;
+    last_free_pfn = e820_get_maxpfn(ret);
 }
 #endif
 
diff --git a/e820.c b/e820.c
index 1770158..6d15cdf 100644
--- a/e820.c
+++ b/e820.c
@@ -285,10 +285,10 @@ void arch_print_memmap(void)
 }
 #endif
 
-unsigned long e820_get_maxpfn(void)
+unsigned long e820_get_maxpfn(unsigned long pages)
 {
     int i;
-    unsigned long pfn, max = 0;
+    unsigned long pfns, start = 0;
 
     e820_get_memmap();
 
@@ -296,10 +296,12 @@ unsigned long e820_get_maxpfn(void)
     {
         if ( e820_map[i].type != E820_RAM )
             continue;
-        pfn = (e820_map[i].addr + e820_map[i].size) >> PAGE_SHIFT;
-        if ( pfn > max )
-            max = pfn;
+        pfns = e820_map[i].size >> PAGE_SHIFT;
+        start = e820_map[i].addr >> PAGE_SHIFT;
+        if ( pages <= pfns )
+            return start + pages;
+        pages -= pfns;
     }
 
-    return max;
+    return start + pfns;
 }
diff --git a/include/e820.h b/include/e820.h
index af2129f..6a57f05 100644
--- a/include/e820.h
+++ b/include/e820.h
@@ -49,6 +49,6 @@ struct __packed e820entry {
 extern struct e820entry e820_map[];
 extern unsigned e820_entries;
 
-unsigned long e820_get_maxpfn(void);
+unsigned long e820_get_maxpfn(unsigned long pages);
 
 #endif /*__E820_HEADER*/
-- 
2.26.2

[PATCH v2 04/10] mini-os: respect memory map when ballooning up

[Juergen Gross]

Today Mini-OS won't look at the memory map when ballooning up. This can
result in problems for PVH domains with more than 4 GB of RAM, as
ballooning will happily run into the ACPI area.

Fix that by adding only pages being marked as RAM in the memory map and
by distinguishing between the current number of RAM pages and the first
unallocated page.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- rename and fix e820_get_max_pages() (Samuel Thibault)
---
 arch/arm/mm.c      |  3 +++
 arch/x86/balloon.c |  4 ++--
 arch/x86/mm.c      |  2 ++
 balloon.c          | 33 ++++++++++++++++++++++++---------
 e820.c             | 21 ++++++++++++++++++++-
 include/balloon.h  |  5 +++--
 include/e820.h     |  1 +
 mm.c               |  7 ++-----
 8 files changed, 57 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mm.c b/arch/arm/mm.c
index 9068166..11962f8 100644
--- a/arch/arm/mm.c
+++ b/arch/arm/mm.c
@@ -3,6 +3,7 @@
 #include <arch_mm.h>
 #include <mini-os/errno.h>
 #include <mini-os/hypervisor.h>
+#include <mini-os/balloon.h>
 #include <libfdt.h>
 #include <lib.h>
 
@@ -70,6 +71,8 @@ void arch_init_mm(unsigned long *start_pfn_p, unsigned long *max_pfn_p)
     }
     device_tree = new_device_tree;
     *max_pfn_p = to_phys(new_device_tree) >> PAGE_SHIFT;
+
+    balloon_set_nr_pages(*max_pfn_p, *max_pfn_p);
 }
 
 void arch_init_demand_mapping_area(void)
diff --git a/arch/x86/balloon.c b/arch/x86/balloon.c
index 10b440c..fe79644 100644
--- a/arch/x86/balloon.c
+++ b/arch/x86/balloon.c
@@ -61,10 +61,10 @@ void arch_remap_p2m(unsigned long max_pfn)
     p2m_invalidate(l2_list, L2_P2M_IDX(max_pfn - 1) + 1);
     p2m_invalidate(l1_list, L1_P2M_IDX(max_pfn - 1) + 1);
 
-    if ( p2m_pages(nr_max_pages) <= p2m_pages(max_pfn) )
+    if ( p2m_pages(nr_max_pfn) <= p2m_pages(max_pfn) )
         return;
 
-    new_p2m = alloc_virt_kernel(p2m_pages(nr_max_pages));
+    new_p2m = alloc_virt_kernel(p2m_pages(nr_max_pfn));
     for ( pfn = 0; pfn < max_pfn; pfn += P2M_ENTRIES )
     {
         map_frame_rw(new_p2m + PAGE_SIZE * (pfn / P2M_ENTRIES),
diff --git a/arch/x86/mm.c b/arch/x86/mm.c
index 3bf6170..c30d8bc 100644
--- a/arch/x86/mm.c
+++ b/arch/x86/mm.c
@@ -72,6 +72,7 @@ void arch_mm_preinit(void *p)
     pt_base = (pgentry_t *)si->pt_base;
     first_free_pfn = PFN_UP(to_phys(pt_base)) + si->nr_pt_frames;
     last_free_pfn = si->nr_pages;
+    balloon_set_nr_pages(last_free_pfn, last_free_pfn);
 }
 #else
 #include <mini-os/desc.h>
@@ -118,6 +119,7 @@ void arch_mm_preinit(void *p)
     }
 
     last_free_pfn = e820_get_maxpfn(ret);
+    balloon_set_nr_pages(ret, last_free_pfn);
 }
 #endif
 
diff --git a/balloon.c b/balloon.c
index 5676d3b..9dc77c5 100644
--- a/balloon.c
+++ b/balloon.c
@@ -23,14 +23,24 @@
 
 #include <mini-os/os.h>
 #include <mini-os/balloon.h>
+#include <mini-os/e820.h>
 #include <mini-os/errno.h>
 #include <mini-os/lib.h>
 #include <mini-os/paravirt.h>
 #include <xen/xen.h>
 #include <xen/memory.h>
 
-unsigned long nr_max_pages;
-unsigned long nr_mem_pages;
+unsigned long nr_max_pfn;
+
+static unsigned long nr_max_pages;
+static unsigned long nr_mem_pfn;
+static unsigned long nr_mem_pages;
+
+void balloon_set_nr_pages(unsigned long pages, unsigned long pfn)
+{
+    nr_mem_pages = pages;
+    nr_mem_pfn = pfn;
+}
 
 void get_max_pages(void)
 {
@@ -46,16 +56,18 @@ void get_max_pages(void)
 
     nr_max_pages = ret;
     printk("Maximum memory size: %ld pages\n", nr_max_pages);
+
+    nr_max_pfn = e820_get_maxpfn(nr_max_pages);
 }
 
 void mm_alloc_bitmap_remap(void)
 {
     unsigned long i, new_bitmap;
 
-    if ( mm_alloc_bitmap_size >= ((nr_max_pages + 1) >> 3) )
+    if ( mm_alloc_bitmap_size >= ((nr_max_pfn + 1) >> 3) )
         return;
 
-    new_bitmap = alloc_virt_kernel(PFN_UP((nr_max_pages + 1) >> 3));
+    new_bitmap = alloc_virt_kernel(PFN_UP((nr_max_pfn + 1) >> 3));
     for ( i = 0; i < mm_alloc_bitmap_size; i += PAGE_SIZE )
     {
         map_frame_rw(new_bitmap + i,
@@ -70,7 +82,7 @@ static unsigned long balloon_frames[N_BALLOON_FRAMES];
 
 int balloon_up(unsigned long n_pages)
 {
-    unsigned long page, pfn;
+    unsigned long page, pfn, start_pfn;
     int rc;
     struct xen_memory_reservation reservation = {
         .domid        = DOMID_SELF
@@ -81,8 +93,11 @@ int balloon_up(unsigned long n_pages)
     if ( n_pages > N_BALLOON_FRAMES )
         n_pages = N_BALLOON_FRAMES;
 
+    start_pfn = e820_get_maxpfn(nr_mem_pages + 1) - 1;
+    n_pages = e820_get_max_contig_pages(start_pfn, n_pages);
+
     /* Resize alloc_bitmap if necessary. */
-    while ( mm_alloc_bitmap_size * 8 < nr_mem_pages + n_pages )
+    while ( mm_alloc_bitmap_size * 8 < start_pfn + n_pages )
     {
         page = alloc_page();
         if ( !page )
@@ -99,14 +114,14 @@ int balloon_up(unsigned long n_pages)
         mm_alloc_bitmap_size += PAGE_SIZE;
     }
 
-    rc = arch_expand_p2m(nr_mem_pages + n_pages);
+    rc = arch_expand_p2m(start_pfn + n_pages);
     if ( rc )
         return rc;
 
     /* Get new memory from hypervisor. */
     for ( pfn = 0; pfn < n_pages; pfn++ )
     {
-        balloon_frames[pfn] = nr_mem_pages + pfn;
+        balloon_frames[pfn] = start_pfn + pfn;
     }
     set_xen_guest_handle(reservation.extent_start, balloon_frames);
     reservation.nr_extents = n_pages;
@@ -116,7 +131,7 @@ int balloon_up(unsigned long n_pages)
 
     for ( pfn = 0; pfn < rc; pfn++ )
     {
-        arch_pfn_add(nr_mem_pages + pfn, balloon_frames[pfn]);
+        arch_pfn_add(start_pfn + pfn, balloon_frames[pfn]);
         free_page(pfn_to_virt(nr_mem_pages + pfn));
     }
 
diff --git a/e820.c b/e820.c
index 6d15cdf..659f71c 100644
--- a/e820.c
+++ b/e820.c
@@ -290,7 +290,8 @@ unsigned long e820_get_maxpfn(unsigned long pages)
     int i;
     unsigned long pfns, start = 0;
 
-    e820_get_memmap();
+    if ( !e820_entries )
+        e820_get_memmap();
 
     for ( i = 0; i < e820_entries; i++ )
     {
@@ -305,3 +306,21 @@ unsigned long e820_get_maxpfn(unsigned long pages)
 
     return start + pfns;
 }
+
+unsigned long e820_get_max_contig_pages(unsigned long pfn, unsigned long pages)
+{
+    int i;
+    unsigned long end;
+
+    for ( i = 0; i < e820_entries && e820_map[i].addr < (pfn << PAGE_SHIFT);
+          i++ )
+    {
+        end = (e820_map[i].addr + e820_map[i].size) >> PAGE_SHIFT;
+        if ( e820_map[i].type != E820_RAM || end <= pfn )
+            continue;
+
+        return ((end - pfn) > pages) ? pages : end - pfn;
+    }
+
+    return 0;
+}
diff --git a/include/balloon.h b/include/balloon.h
index 6cfec4f..8f7c8bd 100644
--- a/include/balloon.h
+++ b/include/balloon.h
@@ -32,11 +32,11 @@
  */
 #define BALLOON_EMERGENCY_PAGES   64
 
-extern unsigned long nr_max_pages;
-extern unsigned long nr_mem_pages;
+extern unsigned long nr_max_pfn;
 
 void get_max_pages(void);
 int balloon_up(unsigned long n_pages);
+void balloon_set_nr_pages(unsigned long pages, unsigned long pfn);
 
 void mm_alloc_bitmap_remap(void);
 void arch_pfn_add(unsigned long pfn, unsigned long mfn);
@@ -50,6 +50,7 @@ static inline int chk_free_pages(unsigned long needed)
 {
     return needed <= nr_free_pages;
 }
+static inline balloon_set_nr_pages(unsigned long pages, unsigned long pfn) { }
 
 #endif /* CONFIG_BALLOON */
 #endif /* _BALLOON_H_ */
diff --git a/include/e820.h b/include/e820.h
index 6a57f05..8d4d371 100644
--- a/include/e820.h
+++ b/include/e820.h
@@ -50,5 +50,6 @@ extern struct e820entry e820_map[];
 extern unsigned e820_entries;
 
 unsigned long e820_get_maxpfn(unsigned long pages);
+unsigned long e820_get_max_contig_pages(unsigned long pfn, unsigned long pages);
 
 #endif /*__E820_HEADER*/
diff --git a/mm.c b/mm.c
index 932ceeb..6493bdd 100644
--- a/mm.c
+++ b/mm.c
@@ -396,8 +396,9 @@ void init_mm(void)
 
     printk("MM: Init\n");
 
-    get_max_pages();
     arch_init_mm(&start_pfn, &max_pfn);
+    get_max_pages();
+
     /*
      * now we can initialise the page allocator
      */
@@ -407,10 +408,6 @@ void init_mm(void)
     arch_init_p2m(max_pfn);
     
     arch_init_demand_mapping_area();
-
-#ifdef CONFIG_BALLOON
-    nr_mem_pages = max_pfn;
-#endif
 }
 
 void fini_mm(void)
-- 
2.26.2

[PATCH v2 05/10] mini-os: don't repeat definition available via header file

[Juergen Gross]

arch/x86/setup.c is repeating the definition of __pte() instead using
the appropriate header. Fix that.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 arch/x86/setup.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/x86/setup.c b/arch/x86/setup.c
index 1ec68d3..b27bbed 100644
--- a/arch/x86/setup.c
+++ b/arch/x86/setup.c
@@ -29,6 +29,7 @@
 #include <mini-os/os.h>
 #include <mini-os/lib.h> /* for printk, memcpy */
 #include <mini-os/kernel.h>
+#include <mini-os/hypervisor.h>
 #include <xen/xen.h>
 #include <xen/arch-x86/cpuid.h>
 #include <xen/arch-x86/hvm/start_info.h>
@@ -61,13 +62,6 @@ char stack[2*STACK_SIZE];
 
 extern char shared_info[PAGE_SIZE];
 
-#if defined(__x86_64__)
-#define __pte(x) ((pte_t) { (x) } )
-#else
-#define __pte(x) ({ unsigned long long _x = (x);        \
-    ((pte_t) {(unsigned long)(_x), (unsigned long)(_x>>32)}); })
-#endif
-
 static inline void fpu_init(void) {
 	asm volatile("fninit");
 }
-- 
2.26.2

[PATCH v2 06/10] mini-os: add memory map service functions

[Juergen Gross]

Add two functions for adding reserved areas to the memory map and
for removing them again.

Those will be needed for proper grant table/mapping support in PVH
mode.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- fix e820_put_reserved_pfns() (Samuel Thibault)
---
 e820.c         | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/e820.h |  4 ++++
 2 files changed, 54 insertions(+)

diff --git a/e820.c b/e820.c
index 659f71c..25e2f9d 100644
--- a/e820.c
+++ b/e820.c
@@ -283,6 +283,56 @@ void arch_print_memmap(void)
         printk("%012lx-%012lx: %s\n", from, to, type);
     }
 }
+
+unsigned long e820_get_reserved_pfns(int pages)
+{
+    int i;
+    unsigned long last = 0, needed = (long)pages << PAGE_SHIFT;
+
+    for ( i = 0; i < e820_entries && e820_map[i].addr < last + needed; i++ )
+        last = e820_map[i].addr + e820_map[i].size;
+
+    if ( i == 0 || e820_map[i - 1].type != E820_RESERVED )
+        e820_insert_entry_at(i, last, needed, E820_RESERVED);
+    else
+        e820_map[i - 1].size += needed;
+
+    return last >> PAGE_SHIFT;
+}
+
+void e820_put_reserved_pfns(unsigned long start_pfn, int pages)
+{
+    int i;
+    unsigned long addr = start_pfn << PAGE_SHIFT;
+    unsigned long size = (long)pages << PAGE_SHIFT;
+
+    for ( i = 0;
+          i < e820_entries && addr >= e820_map[i].addr + e820_map[i].size;
+          i++ );
+
+    BUG_ON(i == e820_entries || e820_map[i].type != E820_RESERVED ||
+           addr + size > e820_map[i].addr + e820_map[i].size);
+
+    if ( addr == e820_map[i].addr )
+    {
+        e820_map[i].addr += size;
+        e820_map[i].size -= size;
+        if ( e820_map[i].size == 0 )
+            e820_remove_entry(i);
+        return;
+    }
+
+    if ( addr + size == e820_map[i].addr + e820_map[i].size )
+    {
+        e820_map[i].size -= size;
+        return;
+    }
+
+    e820_insert_entry_at(i + 1, addr + size,
+                         e820_map[i].addr + e820_map[i].size - addr - size,
+                         E820_RESERVED);
+    e820_map[i].size = addr - e820_map[i].addr;
+}
 #endif
 
 unsigned long e820_get_maxpfn(unsigned long pages)
diff --git a/include/e820.h b/include/e820.h
index 8d4d371..aaf2f2c 100644
--- a/include/e820.h
+++ b/include/e820.h
@@ -51,5 +51,9 @@ extern unsigned e820_entries;
 
 unsigned long e820_get_maxpfn(unsigned long pages);
 unsigned long e820_get_max_contig_pages(unsigned long pfn, unsigned long pages);
+#ifndef CONFIG_E820_TRIVIAL
+unsigned long e820_get_reserved_pfns(int pages);
+void e820_put_reserved_pfns(unsigned long start_pfn, int pages);
+#endif
 
 #endif /*__E820_HEADER*/
-- 
2.26.2

[PATCH v2 07/10] mini-os: move x86 specific gnttab coding into arch/x86/gnttab.c

[Juergen Gross]

Having grant table code in arch/x86/mm.c seems wrong. Move it to the
new file arch/x86/gnttab.c, especially as the amount of code is
expected to grow further.

While doing that replace  type casts to pte_t with the more appropriate
__pte() macro.

No functional change.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 arch/x86/gnttab.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/mm.c     | 47 ----------------------------
 2 files changed, 78 insertions(+), 47 deletions(-)
 create mode 100644 arch/x86/gnttab.c

diff --git a/arch/x86/gnttab.c b/arch/x86/gnttab.c
new file mode 100644
index 0000000..56e59d7
--- /dev/null
+++ b/arch/x86/gnttab.c
@@ -0,0 +1,78 @@
+/* -*-  Mode:C; c-basic-offset:4; tab-width:4 -*-
+ *
+ * (C) 2021 - Juergen Gross, SUSE Software Solutions Germany GmbH
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include <mini-os/os.h>
+#include <mini-os/hypervisor.h>
+#include <mini-os/gnttab.h>
+#include <mini-os/mm.h>
+#include <mini-os/types.h>
+
+grant_entry_v1_t *arch_init_gnttab(int nr_grant_frames)
+{
+    struct gnttab_setup_table setup;
+    unsigned long frames[nr_grant_frames];
+
+    setup.dom = DOMID_SELF;
+    setup.nr_frames = nr_grant_frames;
+    set_xen_guest_handle(setup.frame_list, frames);
+
+    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
+    return map_frames(frames, nr_grant_frames);
+}
+
+void arch_suspend_gnttab(grant_entry_v1_t *gnttab_table, int nr_grant_frames)
+{
+#ifdef CONFIG_PARAVIRT
+    int i;
+
+    for ( i = 0; i < nr_grant_frames; i++ )
+    {
+        HYPERVISOR_update_va_mapping((unsigned long)gnttab_table + PAGE_SIZE * i,
+                __pte(0x0 << PAGE_SHIFT), UVMF_INVLPG);
+    }
+#endif
+    return;
+}
+
+void arch_resume_gnttab(grant_entry_v1_t *gnttab_table, int nr_grant_frames)
+{
+    struct gnttab_setup_table setup;
+    unsigned long frames[nr_grant_frames];
+#ifdef CONFIG_PARAVIRT
+    int i;
+#endif
+
+    setup.dom = DOMID_SELF;
+    setup.nr_frames = nr_grant_frames;
+    set_xen_guest_handle(setup.frame_list, frames);
+
+    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
+
+#ifdef CONFIG_PARAVIRT
+    for ( i = 0; i < nr_grant_frames; i++ )
+    {
+        HYPERVISOR_update_va_mapping((unsigned long)gnttab_table + PAGE_SIZE * i,
+                __pte((frames[i] << PAGE_SHIFT) | L1_PROT), UVMF_INVLPG);
+    }
+#endif
+}
diff --git a/arch/x86/mm.c b/arch/x86/mm.c
index c30d8bc..220c0b4 100644
--- a/arch/x86/mm.c
+++ b/arch/x86/mm.c
@@ -837,53 +837,6 @@ void arch_init_mm(unsigned long* start_pfn_p, unsigned long* max_pfn_p)
 #endif
 }
 
-grant_entry_v1_t *arch_init_gnttab(int nr_grant_frames)
-{
-    struct gnttab_setup_table setup;
-    unsigned long frames[nr_grant_frames];
-
-    setup.dom = DOMID_SELF;
-    setup.nr_frames = nr_grant_frames;
-    set_xen_guest_handle(setup.frame_list, frames);
-
-    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
-    return map_frames(frames, nr_grant_frames);
-}
-
-void arch_suspend_gnttab(grant_entry_v1_t *gnttab_table, int nr_grant_frames)
-{
-#ifdef CONFIG_PARAVIRT
-    int i;
-
-    for (i = 0; i < nr_grant_frames; i++) {
-        HYPERVISOR_update_va_mapping((unsigned long)(((char *)gnttab_table) + PAGE_SIZE * i),
-                (pte_t){0x0<<PAGE_SHIFT}, UVMF_INVLPG);
-    }
-#endif
-    return;
-}
-
-void arch_resume_gnttab(grant_entry_v1_t *gnttab_table, int nr_grant_frames)
-{
-    struct gnttab_setup_table setup;
-    unsigned long frames[nr_grant_frames];
-#ifdef CONFIG_PARAVIRT
-    int i;
-#endif
-    setup.dom = DOMID_SELF;
-    setup.nr_frames = nr_grant_frames;
-    set_xen_guest_handle(setup.frame_list, frames);
-
-    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
-
-#ifdef CONFIG_PARAVIRT
-    for (i = 0; i < nr_grant_frames; i++) {
-        HYPERVISOR_update_va_mapping((unsigned long)(((char *)gnttab_table) + PAGE_SIZE * i),
-                (pte_t){(frames[i] << PAGE_SHIFT) | L1_PROT}, UVMF_INVLPG);
-    }
-#endif
-}
-
 unsigned long alloc_virt_kernel(unsigned n_pages)
 {
     unsigned long addr;
-- 
2.26.2

[PATCH v2 08/10] mini-os: add proper pvh grant table handling

[Juergen Gross]

Grant table initialization for PVH requires some additional actions
compared to PV mode. Add those.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 arch/x86/gnttab.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/x86/gnttab.c b/arch/x86/gnttab.c
index 56e59d7..281c207 100644
--- a/arch/x86/gnttab.c
+++ b/arch/x86/gnttab.c
@@ -22,11 +22,15 @@
  */
 
 #include <mini-os/os.h>
+#include <mini-os/console.h>
+#include <mini-os/e820.h>
 #include <mini-os/hypervisor.h>
 #include <mini-os/gnttab.h>
 #include <mini-os/mm.h>
 #include <mini-os/types.h>
+#include <xen/memory.h>
 
+#ifdef CONFIG_PARAVIRT
 grant_entry_v1_t *arch_init_gnttab(int nr_grant_frames)
 {
     struct gnttab_setup_table setup;
@@ -39,6 +43,33 @@ grant_entry_v1_t *arch_init_gnttab(int nr_grant_frames)
     HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
     return map_frames(frames, nr_grant_frames);
 }
+#else
+grant_entry_v1_t *arch_init_gnttab(int nr_grant_frames)
+{
+    int i, rc;
+    struct xen_add_to_physmap xatp;
+    unsigned long pfn;
+    unsigned long frames[nr_grant_frames];
+
+    pfn = e820_get_reserved_pfns(nr_grant_frames);
+    for ( i = 0; i < nr_grant_frames; i++ )
+    {
+        xatp.domid = DOMID_SELF;
+        xatp.idx = i;
+        xatp.space = XENMAPSPACE_grant_table;
+        xatp.gpfn = pfn + i;
+        rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);
+        if ( rc )
+        {
+            xprintk("could not init grant table\n");
+            do_exit();
+        }
+        frames[i] = pfn + i;
+    }
+
+    return map_frames(frames, nr_grant_frames);
+}
+#endif
 
 void arch_suspend_gnttab(grant_entry_v1_t *gnttab_table, int nr_grant_frames)
 {
-- 
2.26.2

[PATCH v2 09/10] mini-os: prepare grantmap entry interface for use by PVH mode

[Juergen Gross]

Instead of passing the pointer of a grantmap entry to the
_gntmap_[un]map_grant_ref() sub-functions use the map pointer and the
entry index instead. This will be needed for PVH mode usage.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 gntmap.c | 48 +++++++++++++++++++++++-------------------------
 1 file changed, 23 insertions(+), 25 deletions(-)

diff --git a/gntmap.c b/gntmap.c
index f6ab3ad..7ae8fe6 100644
--- a/gntmap.c
+++ b/gntmap.c
@@ -55,36 +55,34 @@ struct gntmap_entry {
 };
 
 static inline int
-gntmap_entry_used(struct gntmap_entry *entry)
+gntmap_entry_used(struct gntmap *map, int idx)
 {
-    return entry->host_addr != 0;
+    return map->entries[idx].host_addr != 0;
 }
 
-static struct gntmap_entry*
-gntmap_find_free_entry(struct gntmap *map)
+static int gntmap_find_free_entry(struct gntmap *map)
 {
     int i;
 
     for (i = 0; i < map->nentries; i++) {
-        if (!gntmap_entry_used(&map->entries[i]))
-            return &map->entries[i];
+        if (!gntmap_entry_used(map, i))
+            return i;
     }
 
     DEBUG("(map=%p): all %d entries full",
            map, map->nentries);
-    return NULL;
+    return -1;
 }
 
-static struct gntmap_entry*
-gntmap_find_entry(struct gntmap *map, unsigned long addr)
+static int gntmap_find_entry(struct gntmap *map, unsigned long addr)
 {
     int i;
 
     for (i = 0; i < map->nentries; i++) {
         if (map->entries[i].host_addr == addr)
-            return &map->entries[i];
+            return i;
     }
-    return NULL;
+    return -1;
 }
 
 int
@@ -105,12 +103,13 @@ gntmap_set_max_grants(struct gntmap *map, int count)
 }
 
 static int
-_gntmap_map_grant_ref(struct gntmap_entry *entry, 
+_gntmap_map_grant_ref(struct gntmap *map, int idx,
                       unsigned long host_addr,
                       uint32_t domid,
                       uint32_t ref,
                       int writable)
 {
+    struct gntmap_entry *entry = map->entries + idx;
     struct gnttab_map_grant_ref op;
     int rc;
 
@@ -135,8 +134,9 @@ _gntmap_map_grant_ref(struct gntmap_entry *entry,
 }
 
 static int
-_gntmap_unmap_grant_ref(struct gntmap_entry *entry)
+_gntmap_unmap_grant_ref(struct gntmap *map, int idx)
 {
+    struct gntmap_entry *entry = map->entries + idx;
     struct gnttab_unmap_grant_ref op;
     int rc;
 
@@ -160,19 +160,19 @@ int
 gntmap_munmap(struct gntmap *map, unsigned long start_address, int count)
 {
     int i, rc;
-    struct gntmap_entry *ent;
+    int idx;
 
     DEBUG("(map=%p, start_address=%lx, count=%d)",
            map, start_address, count);
 
     for (i = 0; i < count; i++) {
-        ent = gntmap_find_entry(map, start_address + PAGE_SIZE * i);
-        if (ent == NULL) {
+        idx = gntmap_find_entry(map, start_address + PAGE_SIZE * i);
+        if (idx < 0) {
             printk("gntmap: tried to munmap unknown page\n");
             return -EINVAL;
         }
 
-        rc = _gntmap_unmap_grant_ref(ent);
+        rc = _gntmap_unmap_grant_ref(map, idx);
         if (rc != 0)
             return rc;
     }
@@ -189,7 +189,7 @@ gntmap_map_grant_refs(struct gntmap *map,
                       int writable)
 {
     unsigned long addr;
-    struct gntmap_entry *ent;
+    int idx;
     int i;
 
     DEBUG("(map=%p, count=%" PRIu32 ", "
@@ -206,9 +206,9 @@ gntmap_map_grant_refs(struct gntmap *map,
         return NULL;
 
     for (i = 0; i < count; i++) {
-        ent = gntmap_find_free_entry(map);
-        if (ent == NULL ||
-            _gntmap_map_grant_ref(ent,
+        idx = gntmap_find_free_entry(map);
+        if (idx < 0 ||
+            _gntmap_map_grant_ref(map, idx,
                                   addr + PAGE_SIZE * i,
                                   domids[i * domids_stride],
                                   refs[i],
@@ -233,15 +233,13 @@ gntmap_init(struct gntmap *map)
 void
 gntmap_fini(struct gntmap *map)
 {
-    struct gntmap_entry *ent;
     int i;
 
     DEBUG("(map=%p)", map);
 
     for (i = 0; i < map->nentries; i++) {
-        ent = &map->entries[i];
-        if (gntmap_entry_used(ent))
-            (void) _gntmap_unmap_grant_ref(ent);
+        if (gntmap_entry_used(map, i))
+            (void) _gntmap_unmap_grant_ref(map, i);
     }
 
     xfree(map->entries);
-- 
2.26.2

[PATCH v2 10/10] mini-os: modify grant mappings to work in PVH mode

[Juergen Gross]

For being able to use the grant mapping interface in PVH mode some
changes are required, as the guest needs to specify a physical address
in the hypercall interface.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 gntmap.c         | 81 ++++++++++++++++++++++++++++++++++--------------
 include/gntmap.h |  1 +
 2 files changed, 59 insertions(+), 23 deletions(-)

diff --git a/gntmap.c b/gntmap.c
index 7ae8fe6..126b04f 100644
--- a/gntmap.c
+++ b/gntmap.c
@@ -32,6 +32,7 @@
 
 #include <mini-os/os.h>
 #include <mini-os/lib.h>
+#include <mini-os/e820.h>
 #include <mini-os/xmalloc.h>
 #include <errno.h>
 #include <xen/grant_table.h>
@@ -97,11 +98,42 @@ gntmap_set_max_grants(struct gntmap *map, int count)
     if (map->entries == NULL)
         return -ENOMEM;
 
+#ifndef CONFIG_PARAVIRT
+    map->start_pfn = e820_get_reserved_pfns(count);
+#endif
+
     memset(map->entries, 0, sizeof(struct gntmap_entry) * count);
     map->nentries = count;
     return 0;
 }
 
+static int
+_gntmap_unmap_grant_ref(struct gntmap *map, int idx)
+{
+    struct gntmap_entry *entry = map->entries + idx;
+    struct gnttab_unmap_grant_ref op;
+    int rc;
+
+#ifdef CONFIG_PARAVIRT
+    op.host_addr    = (uint64_t) entry->host_addr;
+#else
+    op.host_addr    = (uint64_t)(map->start_pfn + idx) << PAGE_SHIFT;
+#endif
+    op.dev_bus_addr = 0;
+    op.handle       = entry->handle;
+
+    rc = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1);
+    if (rc != 0 || op.status != GNTST_okay) {
+        printk("GNTTABOP_unmap_grant_ref failed: "
+               "returned %d, status %" PRId16 "\n",
+               rc, op.status);
+        return rc != 0 ? rc : op.status;
+    }
+
+    entry->host_addr = 0;
+    return 0;
+}
+
 static int
 _gntmap_map_grant_ref(struct gntmap *map, int idx,
                       unsigned long host_addr,
@@ -112,10 +144,17 @@ _gntmap_map_grant_ref(struct gntmap *map, int idx,
     struct gntmap_entry *entry = map->entries + idx;
     struct gnttab_map_grant_ref op;
     int rc;
+#ifndef CONFIG_PARAVIRT
+    unsigned long pfn = map->start_pfn + idx;
+#endif
 
     op.ref = (grant_ref_t) ref;
     op.dom = (domid_t) domid;
+#ifdef CONFIG_PARAVIRT
     op.host_addr = (uint64_t) host_addr;
+#else
+    op.host_addr = (uint64_t)pfn << PAGE_SHIFT; 
+#endif
     op.flags = GNTMAP_host_map;
     if (!writable)
         op.flags |= GNTMAP_readonly;
@@ -128,31 +167,18 @@ _gntmap_map_grant_ref(struct gntmap *map, int idx,
         return rc != 0 ? rc : op.status;
     }
 
-    entry->host_addr = host_addr;
-    entry->handle = op.handle;
-    return 0;
-}
-
-static int
-_gntmap_unmap_grant_ref(struct gntmap *map, int idx)
-{
-    struct gntmap_entry *entry = map->entries + idx;
-    struct gnttab_unmap_grant_ref op;
-    int rc;
-
-    op.host_addr    = (uint64_t) entry->host_addr;
-    op.dev_bus_addr = 0;
-    op.handle       = entry->handle;
-
-    rc = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1);
-    if (rc != 0 || op.status != GNTST_okay) {
-        printk("GNTTABOP_unmap_grant_ref failed: "
-               "returned %d, status %" PRId16 "\n",
-               rc, op.status);
-        return rc != 0 ? rc : op.status;
+#ifndef CONFIG_PARAVIRT
+    rc = do_map_frames(host_addr, &pfn, 1, 0, 0, DOMID_SELF, NULL,
+                       writable ? L1_PROT : L1_PROT_RO);
+    if ( rc )
+    {
+        _gntmap_unmap_grant_ref(map, idx);
+        return rc;
     }
+#endif
 
-    entry->host_addr = 0;
+    entry->host_addr = host_addr;
+    entry->handle = op.handle;
     return 0;
 }
 
@@ -165,6 +191,10 @@ gntmap_munmap(struct gntmap *map, unsigned long start_address, int count)
     DEBUG("(map=%p, start_address=%lx, count=%d)",
            map, start_address, count);
 
+#ifndef CONFIG_PARAVIRT
+    unmap_frames(start_address, count);
+#endif
+
     for (i = 0; i < count; i++) {
         idx = gntmap_find_entry(map, start_address + PAGE_SIZE * i);
         if (idx < 0) {
@@ -242,6 +272,11 @@ gntmap_fini(struct gntmap *map)
             (void) _gntmap_unmap_grant_ref(map, i);
     }
 
+#ifndef CONFIG_PARAVIRT
+    e820_put_reserved_pfns(map->start_pfn, map->nentries);
+    map->start_pfn = 0;
+#endif
+
     xfree(map->entries);
     map->entries = NULL;
     map->nentries = 0;
diff --git a/include/gntmap.h b/include/gntmap.h
index fde53f3..d3d7e88 100644
--- a/include/gntmap.h
+++ b/include/gntmap.h
@@ -10,6 +10,7 @@
 struct gntmap {
     int nentries;
     struct gntmap_entry *entries;
+    unsigned long start_pfn;
 };
 
 int
-- 
2.26.2

Re: [PATCH v2 02/10] mini-os: sort and sanitize e820 memory map

[Samuel Thibault]

Juergen Gross, le lun. 20 déc. 2021 17:07:08 +0100, a ecrit:
> +static void e820_sanitize(void)
> +{
> +    int i;
> +    unsigned long end, start;
> +
> +    /* Sanitize memory map in current form. */
> +    e820_process_entries();
> +
> +    /* Adjust map entries to page boundaries. */
> +    for ( i = 0; i < e820_entries; i++ )
> +    {
> +        start = e820_map[i].addr;
> +        end = start + e820_map[i].size;
> +        if ( (1U << e820_map[i].type) & E820_NARROW )
> +        {
> +            if ( start & (PAGE_SIZE - 1) )
> +            {
> +                start = round_pgup(start);
> +                e820_insert_entry_at(i, start - PAGE_SIZE, PAGE_SIZE,
> +                                     E820_TMP_RESERVED);
> +                i++;
> +            }
> +            if ( end & (PAGE_SIZE - 1) )
> +            {
> +                end = round_pgdown(end);
> +                e820_insert_entry_at(i, end, PAGE_SIZE, E820_TMP_RESERVED);

Rather i+1 so it's most probably already sorted?

Apart from that,

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Samuel

Re: [PATCH v2 03/10] mini-os: don't assume contiguous RAM when initializing in PVH mode

[Samuel Thibault]

Juergen Gross, le lun. 20 déc. 2021 17:07:09 +0100, a ecrit:
> Sizing the available memory should respect memory holes, so look at
> the memory map when setting the boundary for the memory allocator.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
> V2:
> - rename "max" to "start" (Samuel Thibault)
> ---
>  arch/x86/mm.c  |  6 +-----
>  e820.c         | 14 ++++++++------
>  include/e820.h |  2 +-
>  3 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/mm.c b/arch/x86/mm.c
> index 8df93da..3bf6170 100644
> --- a/arch/x86/mm.c
> +++ b/arch/x86/mm.c
> @@ -107,7 +107,6 @@ void arch_mm_preinit(void *p)
>  {
>      long ret;
>      domid_t domid = DOMID_SELF;
> -    unsigned long max;
>  
>      pt_base = page_table_base;
>      first_free_pfn = PFN_UP(to_phys(&_end));
> @@ -117,11 +116,8 @@ void arch_mm_preinit(void *p)
>          xprintk("could not get memory size\n");
>          do_exit();
>      }
> -    last_free_pfn = ret;
>  
> -    max = e820_get_maxpfn();
> -    if ( max < last_free_pfn )
> -        last_free_pfn = max;
> +    last_free_pfn = e820_get_maxpfn(ret);
>  }
>  #endif
>  
> diff --git a/e820.c b/e820.c
> index 1770158..6d15cdf 100644
> --- a/e820.c
> +++ b/e820.c
> @@ -285,10 +285,10 @@ void arch_print_memmap(void)
>  }
>  #endif
>  
> -unsigned long e820_get_maxpfn(void)
> +unsigned long e820_get_maxpfn(unsigned long pages)
>  {
>      int i;
> -    unsigned long pfn, max = 0;
> +    unsigned long pfns, start = 0;
>  
>      e820_get_memmap();
>  
> @@ -296,10 +296,12 @@ unsigned long e820_get_maxpfn(void)
>      {
>          if ( e820_map[i].type != E820_RAM )
>              continue;
> -        pfn = (e820_map[i].addr + e820_map[i].size) >> PAGE_SHIFT;
> -        if ( pfn > max )
> -            max = pfn;
> +        pfns = e820_map[i].size >> PAGE_SHIFT;
> +        start = e820_map[i].addr >> PAGE_SHIFT;
> +        if ( pages <= pfns )
> +            return start + pages;
> +        pages -= pfns;
>      }
>  
> -    return max;
> +    return start + pfns;
>  }
> diff --git a/include/e820.h b/include/e820.h
> index af2129f..6a57f05 100644
> --- a/include/e820.h
> +++ b/include/e820.h
> @@ -49,6 +49,6 @@ struct __packed e820entry {
>  extern struct e820entry e820_map[];
>  extern unsigned e820_entries;
>  
> -unsigned long e820_get_maxpfn(void);
> +unsigned long e820_get_maxpfn(unsigned long pages);
>  
>  #endif /*__E820_HEADER*/
> -- 
> 2.26.2
> 

-- 
Samuel
Now I know someone out there is going to claim, "Well then, UNIX is intuitive,
because you only need to learn 5000 commands, and then everything else follows
from that! Har har har!"
(Andy Bates in comp.os.linux.misc, on "intuitive interfaces", slightly
defending Macs.)

Re: [PATCH v2 04/10] mini-os: respect memory map when ballooning up

[Samuel Thibault]

Juergen Gross, le lun. 20 déc. 2021 17:07:10 +0100, a ecrit:
> +unsigned long e820_get_max_contig_pages(unsigned long pfn, unsigned long pages)
> +{
> +    int i;
> +    unsigned long end;
> +
> +    for ( i = 0; i < e820_entries && e820_map[i].addr < (pfn << PAGE_SHIFT);

Shouldn't that be addr+size? Otherwise if pfn is in the middle of an
e820 entry, we will miss picking up that.

> +          i++ )
> +    {
> +        end = (e820_map[i].addr + e820_map[i].size) >> PAGE_SHIFT;
> +        if ( e820_map[i].type != E820_RAM || end <= pfn )
> +            continue;
> +
> +        return ((end - pfn) > pages) ? pages : end - pfn;
> +    }
> +
> +    return 0;
> +}

Re: [PATCH v2 06/10] mini-os: add memory map service functions

[Samuel Thibault]

Juergen Gross, le lun. 20 déc. 2021 17:07:12 +0100, a ecrit:
> Add two functions for adding reserved areas to the memory map and
> for removing them again.
> 
> Those will be needed for proper grant table/mapping support in PVH
> mode.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
> V2:
> - fix e820_put_reserved_pfns() (Samuel Thibault)
> ---
>  e820.c         | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/e820.h |  4 ++++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/e820.c b/e820.c
> index 659f71c..25e2f9d 100644
> --- a/e820.c
> +++ b/e820.c
> @@ -283,6 +283,56 @@ void arch_print_memmap(void)
>          printk("%012lx-%012lx: %s\n", from, to, type);
>      }
>  }
> +
> +unsigned long e820_get_reserved_pfns(int pages)
> +{
> +    int i;
> +    unsigned long last = 0, needed = (long)pages << PAGE_SHIFT;
> +
> +    for ( i = 0; i < e820_entries && e820_map[i].addr < last + needed; i++ )
> +        last = e820_map[i].addr + e820_map[i].size;
> +
> +    if ( i == 0 || e820_map[i - 1].type != E820_RESERVED )
> +        e820_insert_entry_at(i, last, needed, E820_RESERVED);
> +    else
> +        e820_map[i - 1].size += needed;
> +
> +    return last >> PAGE_SHIFT;
> +}
> +
> +void e820_put_reserved_pfns(unsigned long start_pfn, int pages)
> +{
> +    int i;
> +    unsigned long addr = start_pfn << PAGE_SHIFT;
> +    unsigned long size = (long)pages << PAGE_SHIFT;
> +
> +    for ( i = 0;
> +          i < e820_entries && addr >= e820_map[i].addr + e820_map[i].size;
> +          i++ );
> +
> +    BUG_ON(i == e820_entries || e820_map[i].type != E820_RESERVED ||
> +           addr + size > e820_map[i].addr + e820_map[i].size);
> +
> +    if ( addr == e820_map[i].addr )
> +    {
> +        e820_map[i].addr += size;
> +        e820_map[i].size -= size;
> +        if ( e820_map[i].size == 0 )
> +            e820_remove_entry(i);
> +        return;
> +    }
> +
> +    if ( addr + size == e820_map[i].addr + e820_map[i].size )
> +    {
> +        e820_map[i].size -= size;
> +        return;
> +    }
> +
> +    e820_insert_entry_at(i + 1, addr + size,
> +                         e820_map[i].addr + e820_map[i].size - addr - size,
> +                         E820_RESERVED);
> +    e820_map[i].size = addr - e820_map[i].addr;
> +}
>  #endif
>  
>  unsigned long e820_get_maxpfn(unsigned long pages)
> diff --git a/include/e820.h b/include/e820.h
> index 8d4d371..aaf2f2c 100644
> --- a/include/e820.h
> +++ b/include/e820.h
> @@ -51,5 +51,9 @@ extern unsigned e820_entries;
>  
>  unsigned long e820_get_maxpfn(unsigned long pages);
>  unsigned long e820_get_max_contig_pages(unsigned long pfn, unsigned long pages);
> +#ifndef CONFIG_E820_TRIVIAL
> +unsigned long e820_get_reserved_pfns(int pages);
> +void e820_put_reserved_pfns(unsigned long start_pfn, int pages);
> +#endif
>  
>  #endif /*__E820_HEADER*/
> -- 
> 2.26.2
> 

-- 
Samuel
<A> sauf que le firewall bloque tout sauf internet
 -+- ben ouais, il bloque ipx/spx ! -+-

Re: [PATCH v2 02/10] mini-os: sort and sanitize e820 memory map

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 1257 bytes --]

On 21.12.21 00:17, Samuel Thibault wrote:
> Juergen Gross, le lun. 20 déc. 2021 17:07:08 +0100, a ecrit:
>> +static void e820_sanitize(void)
>> +{
>> +    int i;
>> +    unsigned long end, start;
>> +
>> +    /* Sanitize memory map in current form. */
>> +    e820_process_entries();
>> +
>> +    /* Adjust map entries to page boundaries. */
>> +    for ( i = 0; i < e820_entries; i++ )
>> +    {
>> +        start = e820_map[i].addr;
>> +        end = start + e820_map[i].size;
>> +        if ( (1U << e820_map[i].type) & E820_NARROW )
>> +        {
>> +            if ( start & (PAGE_SIZE - 1) )
>> +            {
>> +                start = round_pgup(start);
>> +                e820_insert_entry_at(i, start - PAGE_SIZE, PAGE_SIZE,
>> +                                     E820_TMP_RESERVED);
>> +                i++;
>> +            }
>> +            if ( end & (PAGE_SIZE - 1) )
>> +            {
>> +                end = round_pgdown(end);
>> +                e820_insert_entry_at(i, end, PAGE_SIZE, E820_TMP_RESERVED);
> 
> Rather i+1 so it's most probably already sorted?

Ah, yes, good catch.

> 
> Apart from that,
> 
> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Thanks,


Juergen


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v2 04/10] mini-os: respect memory map when ballooning up

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 616 bytes --]

On 21.12.21 00:22, Samuel Thibault wrote:
> Juergen Gross, le lun. 20 déc. 2021 17:07:10 +0100, a ecrit:
>> +unsigned long e820_get_max_contig_pages(unsigned long pfn, unsigned long pages)
>> +{
>> +    int i;
>> +    unsigned long end;
>> +
>> +    for ( i = 0; i < e820_entries && e820_map[i].addr < (pfn << PAGE_SHIFT);
> 
> Shouldn't that be addr+size? Otherwise if pfn is in the middle of an
> e820 entry, we will miss picking up that.

No, we want to check all map entries starting below or at the given pfn.
The test should be "e820_map[i].addr <= (pfn << PAGE_SHIFT)", of course.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v2 04/10] mini-os: respect memory map when ballooning up

[Samuel Thibault]

Juergen Gross, le mar. 21 déc. 2021 07:16:49 +0100, a ecrit:
> On 21.12.21 00:22, Samuel Thibault wrote:
> > Juergen Gross, le lun. 20 déc. 2021 17:07:10 +0100, a ecrit:
> > > +unsigned long e820_get_max_contig_pages(unsigned long pfn, unsigned long pages)
> > > +{
> > > +    int i;
> > > +    unsigned long end;
> > > +
> > > +    for ( i = 0; i < e820_entries && e820_map[i].addr < (pfn << PAGE_SHIFT);
> > 
> > Shouldn't that be addr+size? Otherwise if pfn is in the middle of an
> > e820 entry, we will miss picking up that.
> 
> No, we want to check all map entries starting below or at the given pfn.
> The test should be "e820_map[i].addr <= (pfn << PAGE_SHIFT)", of course.

Ah, yes, due to the "<" I mistook it for a loop that skips the entries
before the targetted pfn :)

With <=, 

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Samuel