Re: multiboot on EFI

[phcoder <phcoder@gmail.com>]

Robert Millan wrote:
> Would it be hard to split the patch and make it more granular?  I see it
> implements base mmap / lsmmap support on efi, then ports the *BSD loaders
> and the Multiboot loader too, and the uppermem facility.
The only reason why it's not splitted is that it's totally "preview". 
When it'll be more ready I'll split it
> If everybody's fine with it, I'd like to suggest adding stuff to pkglib_MODULES
> in the same place as its corresponding variables.  I've done this already a few
> times, and I think it makes the build system a bit more maintainable.  What do
> you all think about this?

I also agree with this but I temporarily kept this in 
architecture-specific file because of some minor problems with 
multiboot2. I'll fix this too

> 
>> diff --git a/include/grub/i386/pc/memory.h b/include/grub/i386/pc/memory.h
>> index 08e92a9..e69ff77 100644
>> --- a/include/grub/i386/pc/memory.h
>> +++ b/include/grub/i386/pc/memory.h
>> @@ -92,6 +92,8 @@ struct grub_machine_mmap_entry
>>    grub_uint64_t len;
>>  #define GRUB_MACHINE_MEMORY_AVAILABLE	1
>>  #define GRUB_MACHINE_MEMORY_RESERVED	2
>> +#define GRUB_MACHINE_MEMORY_ACPI	3
>> +#define GRUB_MACHINE_MEMORY_NVS 	4
>>    grub_uint32_t type;
>>  } __attribute__((packed));
> 
> Do we need specific knowledge of these two on i386-pc ?
> 
This one is because some loaders just copy e820 map types and I don't 
want to modify what OS gets on i386-pc
>>  /* The minimum and maximum heap size for GRUB itself.  */
>>  #define MIN_HEAP_SIZE	0x100000
>> -#define MAX_HEAP_SIZE	(16 * 0x100000)
>> +#define MAX_HEAP_SIZE	(1600 * 0x100000)
> 
> Is 1600 MB what we want, or to remove the limit?
> 
I would suggest to remove the limit altogether
>> +  /* Bubble-sort the memory map */
>> +  while (done)
>> +    {
>> +      done = 0;
>> +      for (i = 0; i < count - 1; i++)
>> +	if (regions[i].start > regions[i + 1].start)
>> +	  {
>> +	    done = 1;
>> +	    t = regions[i];
>> +	    regions[i] = regions[i + 1];
>> +	    regions[i + 1] = t;
>> +	  }
>> +    }
> 
> Do we need the memory map to be sorted?  AFAIK loadees can cope with unsorted
> maps fine;  is there an exception?
> 
I prefer to sort. Even as just a precaution. Actually even sorted EFI 
map may break a lot of OS because it usually has more entries (the 
runtime code isn't guaranteed to be contiguous and if it isn't it 
results in mmap having a lot of entries) and sometimes the first N 
kilobytes are defined as unusable (it's the case with qemu-tianocore) 
which under current definition means that low_memory=0
>> +#ifdef GRUB_MACHINE_PCBIOS
>> +  grub_stop_floppy ();
>> +#endif
> 
> grub_stop_floppy() doesn't do any BIOS-specific stuff.  Wouldn't __i386__
> be more appropiate?
> 
I've already moved it to machine_fini just because my computer died I 
couldn't send the new patch

-- 

Regards
Vladimir 'phcoder' Serbinenko