Kevin O'Connor wrote:
Hi Stefan,
Can I CC the coreboot mailing list on this? Other people may run into similar issues.
CCed.
On Thu, Feb 19, 2009 at 08:45:28PM +0100, Stefan Reinauer wrote:
And here is our e820 map as seen by SeaBIOS:
e820 map has 5 items: 0: 0000000000000000 - 000000000009fc00 = 1 1: 000000000009fc00 - 00000000000a0000 = 2 2: 00000000000f0000 - 0000000000100000 = 2 3: 0000000000100000 - 000000003f7f0000 = 1 4: 000000003f7f0000 - 0000000040000000 = 2 final bios_table_addr: 0x000f9f80 (used 18%)
Noticably 0x0 - 0x9fc00 are "usable" despite coreboot table being at 0x500. BAD
One must not reserve any memory between 0x0-0x9fc00 in the e820 - otherwise all sorts of programs (eg, grub) get terribly confused.
Noticably 0xa0000 - 0xeffff are completely missing in the list
It needs to be that way - otherwise Windows gets confused.
Ah interesting! Maybe we should put that knowledge on some Wiki page?
Noticably the PCIe window 0xf0000000 - f4000000 is not in the list as reserved
Hrmm. I've never seen that reserved before. Typically, the e820 is designed to map memory - if its not memory (eg, mmio space) then I don't think it should be in the e820.
Today's systems put a lot of stuff in the e820. ACPI tables, some parts of MMIO space, etc.
The resource above, the MMCFG windows, particularly needs to be in the e820 table because otherwise Linux will refuse to treat PCIe devices as PCIe and will just treat them as PCI instead, notifying the user that the BIOS is buggy because the MMCFG area is only in ACPI but not in e820.
Noticably there is one shared entry for the "high tables" space 3f7f0000
- 0x3f800000 and vga memory 0x3f800000-0x40000000
SeaBIOS will auto-merge like areas. There should be no harm in this.
There is none.
Now, which one is the culprit? Any of them?
I don't see an issue. The AMI bios log you provided showed a similar map.
One thing that is missing is the rom at itself 00000000ffb00000 - 0000000100000000. It would be nice if coreboot added this to the coreboot memory table. (SeaBIOS can't really add it, because SeaBIOS wont know how big the rom chip is.)
Yes,.. coreboot should export quite some more stuff. I will try to look into this for the PCIe MMCFG stuff,.. I'll try to add the ROM, too. Though the ROM size might not be known to coreboot. ROM_SIZE might be insufficient because it lacks the option rom space in v2. We could probe for the ROM but I'm not sure we want that. Maybe we have to solve option roms differently in v2.
Stefan
On Sat, Feb 21, 2009 at 01:57:11AM +0100, Stefan Reinauer wrote:
Kevin O'Connor wrote:
On Thu, Feb 19, 2009 at 08:45:28PM +0100, Stefan Reinauer wrote:
Noticably the PCIe window 0xf0000000 - f4000000 is not in the list as reserved
Hrmm. I've never seen that reserved before. Typically, the e820 is designed to map memory - if its not memory (eg, mmio space) then I don't think it should be in the e820.
Today's systems put a lot of stuff in the e820. ACPI tables, some parts of MMIO space, etc.
SeaBIOS basically just copies the coreboot memory information to the e820 map. If one defines it in the coreboot table, it'll show up in the e820 map. So, I guess the best thing would be to add this support to coreboot.
One thing that is missing is the rom at itself 00000000ffb00000 - 0000000100000000. It would be nice if coreboot added this to the coreboot memory table. (SeaBIOS can't really add it, because SeaBIOS wont know how big the rom chip is.)
Yes,.. coreboot should export quite some more stuff. I will try to look into this for the PCIe MMCFG stuff,.. I'll try to add the ROM, too.
Thanks.
-Kevin
Kevin O'Connor wrote:
If one defines it in the coreboot table, it'll show up in the e820 map. So, I guess the best thing would be to add this support to coreboot.
Tread carefully here. e820 gets a memory map, but coreboot tables are much more generic. Please don't start adding memory entries to the coreboot table!
Make sure to use new entries in the coreboot structures and let SeaBIOS translate them.
//Peter
Peter Stuge wrote:
Kevin O'Connor wrote:
If one defines it in the coreboot table, it'll show up in the e820 map. So, I guess the best thing would be to add this support to coreboot.
Tread carefully here. e820 gets a memory map, but coreboot tables are much more generic. Please don't start adding memory entries to the coreboot table!
I don't think I get your point.
What's wrong about adding memory map specific entries to the memory map part of the coreboot table?
Stefan Reinauer wrote:
Tread carefully here. e820 gets a memory map, but coreboot tables are much more generic. Please don't start adding memory entries to the coreboot table!
I don't think I get your point.
Hah! No wonder. I did not make any sense at all there!
What's wrong about adding memory map specific entries to the memory map part of the coreboot table?
As long as they are indeed memory map entries it's certainly the right place. But if some information is or should be available elsewhere then we should think twice before adding it to the memory map.
//Peter
On Sat, Feb 21, 2009 at 06:01:39PM +0100, Peter Stuge wrote:
Kevin O'Connor wrote:
If one defines it in the coreboot table, it'll show up in the e820 map. So, I guess the best thing would be to add this support to coreboot.
Tread carefully here. e820 gets a memory map, but coreboot tables are much more generic. Please don't start adding memory entries to the coreboot table!
Make sure to use new entries in the coreboot structures and let SeaBIOS translate them.
Huh, it's already in coreboot.
From src/include/boot/coreboot_tables.h:
struct lb_memory_range { struct lb_uint64 start; struct lb_uint64 size; uint32_t type; #define LB_MEM_RAM 1 /* Memory anyone can use */ #define LB_MEM_RESERVED 2 /* Don't use this memory region */ #define LB_MEM_TABLE 16 /* Ram configuration tables are kept in */ };
And the e820 looks like:
#define E820_RAM 1 #define E820_RESERVED 2 #define E820_ACPI 3 #define E820_NVS 4 #define E820_UNUSABLE 5
struct e820entry { u64 start; u64 size; u32 type; };
So, SeaBIOS just copies the info over (with special handling for LB_MEM_TABLE).
-Kevin
-
Kevin O'Connor -
Peter Stuge -
Stefan Reinauer