Kevin O'Connor wrote:
--- src/mainboard/kontron/986lcd-m/mainboard.c (revision 3886) +++ src/mainboard/kontron/986lcd-m/mainboard.c (working copy) @@ -21,8 +21,31 @@
#include <device/device.h> +#include <console/console.h> +#include <boot/tables.h> #include "chip.h"
+/* in arch/i386/boot/tables.c */ +extern uint64_t high_tables_base, high_tables_size;
+/* in northbridge/intel/i945/northbridge.c */ +extern uint64_t uma_memory_base, uma_memory_size;
+int add_mainboard_resources(struct lb_memory *mem) +{ +#if HAVE_HIGH_TABLES == 1
- printk_debug("Adding high table area\n");
- lb_add_memory_range(mem, LB_MEM_TABLE,
high_tables_base, high_tables_size);+#endif
Isn't there a way we can add this support without having to change every platform?
We probably could, by parsing the device tree and finding the memory resource.
But then again, this only makes sense on those platforms where we have ACPI which is a small hand full.
Also, even without the high tables code, most boards will need a locat "add_mainboard_resources" function for their PCIe BAR or their UMA graphics (or both)
--- src/arch/i386/boot/tables.c (revision 3886) +++ src/arch/i386/boot/tables.c (working copy)
[...]
/* Write ACPI tables */ /* write them in the rom area because DSDT can be large (8K on epia-m) which * pushes coreboot table out of first 4K if set up in low table area */ +#if HAVE_LOW_TABLES == 1 rom_table_end = write_acpi_tables(rom_table_end); rom_table_end = (rom_table_end+1023) & ~1023;
+#endif +#if HAVE_HIGH_TABLES == 1
- if (high_tables_base) {
high_table_end = write_acpi_tables(high_table_end);high_table_end = (high_table_end+1023) & ~1023;- }
+#endif
I'm a little concerned about generating the data twice. That could confuse debugging efforts (the OS might see one version while the user investigates another). The data wont be identical because acpi has pointers in the data structures.
So am I, as you probably saw from my "HAVE_LOW_TABLES" guards. It's possible to disable the "low tables" and that would generally be a good idea, if all payloads would start copying tables around. But
- only SeaBIOS does this (so far?) (and seabios users can help it with the HAVE_LOW_TABLES=0 switch - SeaBIOS will wipe away the low tables in FSEG upon load - The "high tables" are never searched by the OS in case the low tables are found
So it's a trade off, and basically it's a smart thing to disable the tables that the payload or kernel will not really use.
I like the idea of always keeping support for tables in 0xf0000 - maybe we could just copy the 36 byte ACPI RSDP struct to low memory when HIGH_TABLES are enabled. (And similarly, the 12 byte "mptable floating structure".)
So are you suggesting to write 48 bytes to the fseg both in coreboot and in seabios, and have the "rest" in the "high tables area"? That would work.
But what about DMI. How do we handle DMI? It needs to go to the FSEG and it doesn't have pointers afaik.
-#if HAVE_MP_TABLE==1 +#if HAVE_MP_TABLE == 1 /* Don't write anything in the traditional x86 BIOS data segment, * for example the linux kernel smp need to use 0x467 to pass reset vector * or use 0x40e/0x413 for EBDA finding...
I think the above "#if HAVE_MP_TABLE" branch doesn't make any sense. Currently, coreboot tries to put the mptable in the first 1KiB of ram. If it can't fit, it will put it into the 0xf0000 segment. This complexity doesn't make any sense, because there is no reason why we can't always put the mptable in the 0xf0000 segment.
That is, in the HAVE_LOW_TABLES case, we can use:
rom_table_end = write_smp_table(rom_table_end); rom_table_end = (rom_table_end+1023) & ~1023;
and completely remove the HAVE_MP_TABLE case.
I think YhLu added that for some side case... Yinghai.. mind to share the insight?
Stefan