Messages in this thread | | | Date | Tue, 23 Jun 2009 16:32:46 +0800 | From | Feng Tang <> | Subject | Re: [PATCH 3/8] SFI: core support |
| |
On Tue, 23 Jun 2009 15:56:43 +0800 Ingo Molnar <mingo@elte.hu> wrote:
> > > > +#ifdef CONFIG_X86_LOCAL_APIC > > +static u64 sfi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE; > > +#endif > > if SFI adds a 'depends on X86_LOCAL_APIC' the ugly #ifdef can be > dropped.
When Len designed the SFI spec, he considered the possibility of being used by multiple archs, so we chose not to add a x86 dependency, though adding these #ifdef does make code ugly :P
> > +{ > > + unsigned long i; > > ... like here. > > > + char *pchar = (char *)SFI_SYST_SEARCH_BEGIN; > > + > > + for (i = 0; SFI_SYST_SEARCH_BEGIN + i < SFI_SYST_SEARCH_END; > > i += 16, pchar += 16) { > > What does the magic constant '16' mean here?
My bad not puting clear comments here, the SFI spec defines SYST table starts at a 16-byte boundary
> > + > > +static int __init sfi_parse_apic(struct sfi_table_header *table) > > +{ > > + struct sfi_table_simple *sb; > > + struct sfi_apic_table_entry *pentry; > > + int i, num; > > + > > + BUG_ON(!table); > > Same as comment above - is this case anticipated? If yes, is a crash > the best answer?
Yes, usually table won't be NULL
> > + > > +#define SFI_ACPI_TABLE 1 > > In general, nice stuff - basically SFI is cleanly implemented ACPI > tables without any of the run-code-in-acpi-tables complications, > right?
Thanks for the comments, I really got inspired :). The expectation for SFI is to be able to run cleanly with CONFIG_ACPI=n, and it works fine on some intel platform.
Thanks, Feng
> > Ingo
| |