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

[Juergen Gross <lists@xxxxxxxxx>]

On 03/11/17 16:04, Volodymyr Babchuk wrote:
> 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.

No, this is correct. It should catch 32 bit overflow on X86_32, where
ULONG_MAX is 0xffffffff while e820_map[index].addr is a 64 bit quantity.


Juergen

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