[v3,3/4] mini-os: fix number of pages for PVH

Commit Message

Jürgen Groß June 21, 2022, 7:23 a.m. UTC

When getting the current allocation from Xen, this value includes the
pages allocated in the MMIO area. Fix the highest available RAM page
by subtracting the size of that area.

This requires to read the E820 map before needing this value. Add two
functions returning the current and the maximum number of RAM pages
taking this correction into account.

At the same time add the LAPIC page to the memory map in order to
avoid reusing that PFN for internal purposes.

Signed-off-by: Juergen Gross <[e-mail]>
Reviewed-by: Samuel Thibault <[e-mail]>
---
V2:
- make e820_initial_reserved_pfns static (Samuel Thibault)
- add e820_get_current_pages() and e820_get_max_pages()
V3:
- change return type of e820_get_current_pages() and e820_get_max_pages()
  to unsigned long (Samuel Thibault)
---
 arch/x86/mm.c         | 17 +++++--------
 balloon.c             | 16 +++---------
 e820.c                | 58 +++++++++++++++++++++++++++++++++++++------
 include/e820.h        |  2 ++
 include/x86/arch_mm.h |  2 ++
 5 files changed, 65 insertions(+), 30 deletions(-)

Comments

Andrew Cooper June 21, 2022, 8:43 a.m. UTC | #1

On 21/06/2022 08:23, Juergen Gross wrote:
> diff --git a/e820.c b/e820.c
> index ad91e00b..49b16878 100644
> --- a/e820.c
> +++ b/e820.c
> @@ -29,6 +29,38 @@
>  #include <mini-os/e820.h>
>  #include <xen/memory.h>
>  
> +static unsigned long e820_initial_reserved_pfns;
> +
> +unsigned long e820_get_current_pages(void)
> +{
> +    domid_t domid = DOMID_SELF;
> +    long ret;
> +
> +    ret = HYPERVISOR_memory_op(XENMEM_current_reservation, &domid);
> +    if ( ret < 0 )
> +    {
> +        xprintk("could not get memory size\n");

%ld ret

Also, xprintk() vs ...

> +        do_exit();
> +    }
> +
> +    return ret - e820_initial_reserved_pfns;
> +}
> +
> +unsigned long e820_get_max_pages(void)
> +{
> +    domid_t domid = DOMID_SELF;
> +    long ret;
> +
> +    ret = HYPERVISOR_memory_op(XENMEM_maximum_reservation, &domid);
> +    if ( ret < 0 )
> +    {
> +        printk("Could not get maximum pfn\n");

... printk()?

Shouldn't they both be printk()?  Can fix both issues on commit.

~Andrew

Jürgen Groß June 21, 2022, 8:52 a.m. UTC | #2

On 21.06.22 10:43, Andrew Cooper wrote:
> On 21/06/2022 08:23, Juergen Gross wrote:
>> diff --git a/e820.c b/e820.c
>> index ad91e00b..49b16878 100644
>> --- a/e820.c
>> +++ b/e820.c
>> @@ -29,6 +29,38 @@
>>   #include <mini-os/e820.h>
>>   #include <xen/memory.h>
>>   
>> +static unsigned long e820_initial_reserved_pfns;
>> +
>> +unsigned long e820_get_current_pages(void)
>> +{
>> +    domid_t domid = DOMID_SELF;
>> +    long ret;
>> +
>> +    ret = HYPERVISOR_memory_op(XENMEM_current_reservation, &domid);
>> +    if ( ret < 0 )
>> +    {
>> +        xprintk("could not get memory size\n");
> 
> %ld ret
> 
> Also, xprintk() vs ...
> 
>> +        do_exit();
>> +    }
>> +
>> +    return ret - e820_initial_reserved_pfns;
>> +}
>> +
>> +unsigned long e820_get_max_pages(void)
>> +{
>> +    domid_t domid = DOMID_SELF;
>> +    long ret;
>> +
>> +    ret = HYPERVISOR_memory_op(XENMEM_maximum_reservation, &domid);
>> +    if ( ret < 0 )
>> +    {
>> +        printk("Could not get maximum pfn\n");
> 
> ... printk()?
> 
> Shouldn't they both be printk()?  Can fix both issues on commit.

e820_get_current_pages() is being called before console initialization,
so it should really use xprintk().

Adding the returned error might be interesting, though (even if an error
is very unlikely).

Note that the error message printing has just been moved from the previous
use case into the new function.


Juergen

Patch

diff --git a/arch/x86/mm.c b/arch/x86/mm.c
index 41fcee67..cfc978f6 100644
--- a/arch/x86/mm.c
+++ b/arch/x86/mm.c
@@ -107,25 +107,20 @@  desc_ptr idt_ptr =
 
 void arch_mm_preinit(void *p)
 {
-    long ret;
-    domid_t domid = DOMID_SELF;
+    unsigned int pages;
     struct hvm_start_info *hsi = p;
 
     if ( hsi->version >= 1 && hsi->memmap_entries > 0 )
         e820_init_memmap((struct hvm_memmap_table_entry *)(unsigned long)
                          hsi->memmap_paddr, hsi->memmap_entries);
+    else
+        e820_init_memmap(NULL, 0);
 
     pt_base = page_table_base;
     first_free_pfn = PFN_UP(to_phys(&_end));
-    ret = HYPERVISOR_memory_op(XENMEM_current_reservation, &domid);
-    if ( ret < 0 )
-    {
-        xprintk("could not get memory size\n");
-        do_exit();
-    }
-
-    last_free_pfn = e820_get_maxpfn(ret);
-    balloon_set_nr_pages(ret, last_free_pfn);
+    pages = e820_get_current_pages();
+    last_free_pfn = e820_get_maxpfn(pages);
+    balloon_set_nr_pages(pages, last_free_pfn);
 }
 #endif
 
diff --git a/balloon.c b/balloon.c
index 9dc77c54..6ad07644 100644
--- a/balloon.c
+++ b/balloon.c
@@ -44,20 +44,12 @@  void balloon_set_nr_pages(unsigned long pages, unsigned long pfn)
 
 void get_max_pages(void)
 {
-    long ret;
-    domid_t domid = DOMID_SELF;
-
-    ret = HYPERVISOR_memory_op(XENMEM_maximum_reservation, &domid);
-    if ( ret < 0 )
+    nr_max_pages = e820_get_max_pages();
+    if ( nr_max_pages )
     {
-        printk("Could not get maximum pfn\n");
-        return;
+        printk("Maximum memory size: %ld pages\n", nr_max_pages);
+        nr_max_pfn = e820_get_maxpfn(nr_max_pages);
     }
-
-    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)
diff --git a/e820.c b/e820.c
index ad91e00b..49b16878 100644
--- a/e820.c
+++ b/e820.c
@@ -29,6 +29,38 @@ 
 #include <mini-os/e820.h>
 #include <xen/memory.h>
 
+static unsigned long e820_initial_reserved_pfns;
+
+unsigned long e820_get_current_pages(void)
+{
+    domid_t domid = DOMID_SELF;
+    long ret;
+
+    ret = HYPERVISOR_memory_op(XENMEM_current_reservation, &domid);
+    if ( ret < 0 )
+    {
+        xprintk("could not get memory size\n");
+        do_exit();
+    }
+
+    return ret - e820_initial_reserved_pfns;
+}
+
+unsigned long e820_get_max_pages(void)
+{
+    domid_t domid = DOMID_SELF;
+    long ret;
+
+    ret = HYPERVISOR_memory_op(XENMEM_maximum_reservation, &domid);
+    if ( ret < 0 )
+    {
+        printk("Could not get maximum pfn\n");
+        return 0;
+    }
+
+    return ret - e820_initial_reserved_pfns;
+}
+
 #ifdef CONFIG_E820_TRIVIAL
 struct e820entry e820_map[1] = {
     {
@@ -40,10 +72,6 @@  struct e820entry e820_map[1] = {
 
 unsigned e820_entries = 1;
 
-static void e820_get_memmap(void)
-{
-}
-
 #else
 struct e820entry e820_map[E820_MAX];
 unsigned e820_entries;
@@ -199,6 +227,7 @@  static void e820_sanitize(void)
 {
     int i;
     unsigned long end, start;
+    bool found_lapic = false;
 
     /* Sanitize memory map in current form. */
     e820_process_entries();
@@ -238,8 +267,20 @@  static void e820_sanitize(void)
 
     /* 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;
+        if ( e820_map[i].type == E820_RESERVED )
+        {
+            e820_initial_reserved_pfns += e820_map[i].size / PAGE_SIZE;
+            if ( e820_map[i].addr <= LAPIC_ADDRESS &&
+                 e820_map[i].addr + e820_map[i].size > LAPIC_ADDRESS )
+                found_lapic = true;
+        }
+    }
+
+    if ( !found_lapic )
+        e820_insert_entry(LAPIC_ADDRESS, PAGE_SIZE, E820_RESERVED);
 }
 
 static void e820_get_memmap(void)
@@ -264,6 +305,12 @@  void e820_init_memmap(struct hvm_memmap_table_entry *entry, unsigned int num)
 {
     unsigned int i;
 
+    if ( !entry )
+    {
+        e820_get_memmap();
+        return;
+    }
+
     BUILD_BUG_ON(XEN_HVM_MEMMAP_TYPE_RAM != E820_RAM);
     BUILD_BUG_ON(XEN_HVM_MEMMAP_TYPE_RESERVED != E820_RESERVED);
     BUILD_BUG_ON(XEN_HVM_MEMMAP_TYPE_ACPI != E820_ACPI);
@@ -365,9 +412,6 @@  unsigned long e820_get_maxpfn(unsigned long pages)
     int i;
     unsigned long pfns = 0, start = 0;
 
-    if ( !e820_entries )
-        e820_get_memmap();
-
     for ( i = 0; i < e820_entries; i++ )
     {
         if ( e820_map[i].type != E820_RAM )
diff --git a/include/e820.h b/include/e820.h
index 5438a7c8..ffa15aa9 100644
--- a/include/e820.h
+++ b/include/e820.h
@@ -52,6 +52,8 @@  struct __packed e820entry {
 extern struct e820entry e820_map[];
 extern unsigned e820_entries;
 
+unsigned long e820_get_current_pages(void);
+unsigned long e820_get_max_pages(void);
 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
diff --git a/include/x86/arch_mm.h b/include/x86/arch_mm.h
index ffbec5a8..a1b975dc 100644
--- a/include/x86/arch_mm.h
+++ b/include/x86/arch_mm.h
@@ -207,6 +207,8 @@  typedef unsigned long pgentry_t;
 /* to align the pointer to the (next) page boundary */
 #define PAGE_ALIGN(addr)        (((addr)+PAGE_SIZE-1)&PAGE_MASK)
 
+#define LAPIC_ADDRESS	CONST(0xfee00000)
+
 #ifndef __ASSEMBLY__
 /* Definitions for machine and pseudophysical addresses. */
 #ifdef __i386__