[coreboot] [PATCH] Table code cleanup
mylesgw at gmail.com
Thu May 14 16:04:52 CEST 2009
> -----Original Message-----
> From: Patrick Georgi [mailto:patrick at georgi-clan.de]
> Sent: Thursday, May 14, 2009 2:00 AM
> To: Myles Watson
> Cc: coreboot
> Subject: Re: [coreboot] [PATCH] Table code cleanup
> Am 14.05.2009 00:01, schrieb Myles Watson:
> > tables.diff:
> > Add comments.
> > Remove ACPI-specific code.
> > Align low_table_end after gdt (so it matches high_table_end).
> > Correct "New low_table_end..." line in coreboot_table.c.
> > high_low.diff:
> > Factor out common code for writing tables.
> > both.diff:
> > If we need to have the tables written both times we can do something
> like this.
> > Signed-off-by: Myles Watson<mylesgw at gmail.com>
> > Thanks,
> > Myles
> + smp_write_floating_table_physaddr(low_table_end, mpc_start);
> crashes some linux versions that expect the MP table to directly follow
> the floating table, I think.
? I must have misunderstood. How can we put MP table in the high tables
area if the floating table has to be in low memory to be found, if they have
to be adjacent?
> Also your change seems to create _only_ a high ACPI table if
> HAVE_HIGH_TABLES is activated, which is less than optimal because ACPI
> tables there aren't found automatically but require the payload to
> create a low mem RSDP - which the code that you removed already does.
Yes. I think it should go into the ACPI code. I'll add it if you'll test.
> In my opinion we should keep the current behaviour:
> - high tables: create ACPI in high tables area
> - low tables: create ACPI in low tables area
> - high and low tables: create ACPI in high tables area and a pointer to
> the structure in low tables.
I wasn't unhappy with the behavior, just the maintainability. I think the
code flow was too complex.
> The original behaviour was to write ACPI twice. Unfortunately, we need
> ACPI and SMM to interact, and this requires the address of an ACPI
> object in the SMM handler.
> With two ACPI tables, that object exists twice, and we can't know for
> sure which one is used by the OS.
Thanks for the comments,
More information about the coreboot