Re: [PATCH] minios: don't rely on specific page table allocation scheme

[Wei Liu <wei.liu2@citrix.com>]

On Thu, Nov 19, 2015 at 05:05:16PM +0100, Juergen Gross wrote:
> Today mini-os is making assumptions how the page tables it is started
> with are being allocated. Especially it is using the number of page
> table frames to calculate which is the first unmapped pfn.
> 
> Instead of relying on page table number assumptions just look into the
> page tables to find the first pfn not already mapped.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

I ran both pv-grub-x86_{64,32} up to the point when grub menu was
shown -- That's beyond the point of the original crash.

I also ran mini-os 32 and 64 bit with this patch on toolstack without
large p2m series. They worked.

> ---
>  arch/x86/mm.c         | 47 ++++++++++++++++++++++++++++++++++++-----------
>  include/x86/arch_mm.h |  7 -------
>  2 files changed, 36 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/x86/mm.c b/arch/x86/mm.c
> index 9c6d1b8..5d7c006 100644
> --- a/arch/x86/mm.c
> +++ b/arch/x86/mm.c
> @@ -200,8 +200,8 @@ static void build_pagetable(unsigned long *start_pfn, unsigned long *max_pfn)
>      int count = 0;
>      int rc;
>  
> -    pfn_to_map = 
> -        (start_info.nr_pt_frames - NOT_L1_FRAMES) * L1_PAGETABLE_ENTRIES;
> +    pfn_to_map = (*start_pfn + L1_PAGETABLE_ENTRIES - 1) &
> +                 ~(L1_PAGETABLE_ENTRIES - 1);
>  
>      if ( *max_pfn >= virt_to_pfn(HYPERVISOR_VIRT_START) )
>      {
> @@ -229,9 +229,15 @@ static void build_pagetable(unsigned long *start_pfn, unsigned long *max_pfn)
>  #if defined(__x86_64__)
>          offset = l4_table_offset(start_address);
>          /* Need new L3 pt frame */
> -        if ( !(start_address & L3_MASK) )
> -            if ( need_pt_frame(start_address, L3_FRAME) ) 
> -                new_pt_frame(&pt_pfn, pt_mfn, offset, L3_FRAME);
> +        if ( !(tab[offset] & _PAGE_PRESENT) )
> +        {
> +            if ( !need_pt_frame(start_address, L3_FRAME) )
> +            {
> +                printk("ERROR: build_pagetable(): L3 frame not present\n");
> +                do_exit();
> +            }
> +            new_pt_frame(&pt_pfn, pt_mfn, offset, L3_FRAME);

I'm not sure about this hunk and other similar hunks. Shouldn't this be

               if ( need_pt_frame(start_address, L3_FRAME) )
	           new_pt_frame(&pt_pfn, pt_mfn, offset, L3_FRAME);

That is, just like before.


Wei.