Re: [Minios-devel] [PATCH 13/40] mini-os: remove the e820 from common code

[Volodymyr Babchuk <vlad.babchuk@xxxxxxxxx>]

Hi Huang,

On 3 November 2017 at 05:11, Huang Shijie <shijie.huang@xxxxxxx> wrote:
> The e820 is x86 specific. This patch adds a new
> function arch_check_mem_block() and a mem_blocks.
>
> Different archs implements the mem_blocks and arch_check_mem_block.
> By this way, we remove the e820 code from the common code.
>
> Change-Id: I6cfa5bcb12128f55b910f72f592e5b43ebd31dd4
> Jira: ENTOS-247
> Signed-off-by: Huang Shijie <shijie.huang@xxxxxxx>
> ---
>  arch/arm/mm.c | 18 ++++++++++--------
>  arch/x86/mm.c | 17 +++++++++++++++--
>  include/mm.h  |  3 +++
>  mm.c          |  8 ++------
>  4 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/mm.c b/arch/arm/mm.c
> index 3d88d3b..f600672 100644
> --- a/arch/arm/mm.c
> +++ b/arch/arm/mm.c
> @@ -3,18 +3,20 @@
>  #include <arch_mm.h>
>  #include <mini-os/errno.h>
>  #include <mini-os/hypervisor.h>
> +#include <mini-os/posix/limits.h>
>  #include <libfdt.h>
>  #include <lib.h>
>
>  paddr_t physical_address_offset;
> -struct e820entry e820_map[1] = {
> -    {
> -        .addr = 0,
> -        .size = ULONG_MAX - 1,
> -        .type = E820_RAM
> -    }
> -};
> -unsigned e820_entries = 1;
> +
> +unsigned mem_blocks = 1;
> +
> +int arch_check_mem_block(int index, unsigned long *r_min, unsigned long 
> *r_max)
> +{
> +    *r_min = 0;
> +    *r_max = ULONG_MAX - 1;
> +    return 0;
> +}
>
>  unsigned long allocate_ondemand(unsigned long n, unsigned long alignment)
>  {
> diff --git a/arch/x86/mm.c b/arch/x86/mm.c
> index 05ad029..ba1bfc5 100644
> --- a/arch/x86/mm.c
> +++ b/arch/x86/mm.c
> @@ -71,7 +71,7 @@ struct e820entry e820_map[1] = {
>          .type = E820_RAM
>      }
>  };
> -unsigned e820_entries = 1;
> +unsigned mem_blocks = 1;
>
>  void arch_mm_preinit(void *p)
>  {
> @@ -113,7 +113,8 @@ desc_ptr idt_ptr =
>  };
>
>  struct e820entry e820_map[E820_MAX];
> -unsigned e820_entries;
> +unsigned mem_blocks;
> +#define e820_entries mem_blocks;
>
>  static char *e820_types[E820_TYPES] = {
>      [E820_RAM]      = "RAM",
> @@ -191,6 +192,18 @@ void arch_print_memmap(void)
>  }
>  #endif
>
> +int arch_check_mem_block(int index, unsigned long *r_min, unsigned long 
> *r_max)
> +{
> +    if (e820_map[index].type != E820_RAM)
> +        return 1;
> +    if (e820_map[index].addr + e820_map[index].size >= ULONG_MAX)
Hmm, does this check makes sense? You need to cast left part to 128-bit integer.
I see that you just move this code from another place. But for me it
looks wrong.

> +        BUG();
> +
> +    *r_min = e820_map[index].addr;
> +    *r_max = *r_min + e820_map[index].size;
> +    return 0;
> +}
> +
>  /*
>   * Make pt_pfn a new 'level' page table frame and hook it into the page
>   * table at offset in previous level MFN (pref_l_mfn). pt_pfn is a guest
> diff --git a/include/mm.h b/include/mm.h
> index 4fc364f..08e434d 100644
> --- a/include/mm.h
> +++ b/include/mm.h
> @@ -87,4 +87,7 @@ extern unsigned long heap, brk, heap_mapped, heap_end;
>  int free_physical_pages(xen_pfn_t *mfns, int n);
>  void fini_mm(void);
>
> +extern unsigned int mem_blocks;
> +extern int arch_check_mem_block(int index, unsigned long *r_min,
> +                                unsigned long *r_max);
>  #endif /* _MM_H_ */
> diff --git a/mm.c b/mm.c
> index 4880b5e..74565d1 100644
> --- a/mm.c
> +++ b/mm.c
> @@ -175,15 +175,11 @@ static void init_page_allocator(unsigned long min, 
> unsigned long max)
>      /* All allocated by default. */
>      memset(mm_alloc_bitmap, ~0, mm_alloc_bitmap_size);
>
> -    for ( m = 0; m < e820_entries; m++ )
> +    for ( m = 0; m < mem_blocks; m++ )
>      {
> -        if ( e820_map[m].type != E820_RAM )
> +        if (arch_check_mem_block(m, &r_min, &r_max))
>              continue;
> -        if ( e820_map[m].addr + e820_map[m].size >= ULONG_MAX )
> -            BUG();
>
> -        r_min = e820_map[m].addr;
> -        r_max = r_min + e820_map[m].size;
>          if ( r_max <= min || r_min >= max )
>              continue;
>          if ( r_min < min )
> --
> 2.7.4
>



-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babchuk@xxxxxxxxx

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/minios-devel