[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/3] mini-os: fix number of pages for PVH



On 18.06.22 17:56, Samuel Thibault wrote:
Juergen Gross, le sam. 18 juin 2022 16:07:07 +0200, a ecrit:
On 18.06.22 14:13, Samuel Thibault wrote:
Hello,

Juergen Gross, le sam. 18 juin 2022 12:48:16 +0200, a ecrit:
@@ -124,7 +126,7 @@ void arch_mm_preinit(void *p)
           do_exit();
       }
-    last_free_pfn = e820_get_maxpfn(ret);
+    last_free_pfn = e820_get_maxpfn(ret - e820_initial_reserved_pfns);

Mmm, but the reserved pfn could be in the middle of the e820 address
space.

That doesn't matter.

e820_get_maxpfn(n) will just return the pfn of the n-th RAM pfn it is
finding in the E820 map.

Yes, but subtracting at this point looks a bit hacky to me.

It seems to me that it'd be better to make e820_get_maxpfn count by
itself the reserved pages (but never return its pfn of course), rather
than having to make e820_sanitize look at the reserved pages, store
it somewhere, and hope that other code will remember to subtract that
before calling e820_get_maxpfn.

I mean something like:

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++ )
     {
         pfns = e820_map[i].size >> PAGE_SHIFT;

        if ( e820_map[i].type == E820_RESERVED )
        {
            /* This counts in the memory reservation, but is not usable */
             pages -= pfns;
            continue;
        }
         if ( e820_map[i].type != E820_RAM )
             continue;

         start = e820_map[i].addr >> PAGE_SHIFT;
         if ( pages <= pfns )
             return start + pages;
         pages -= pfns;
     }

     return start + pfns;
}

This would lead to wrong values of nr_mem_pages. I think the best solution
would be to have functions returning the number of available and max RAM
pages to e820.c. This would address your valid concern, while not leading
to wrong values at the callers side.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.