SFI - Simple Firmware Interface support

From: Feng Tang <[email protected]>

"sfi=off" is analogous to "acpi=off"

In practice, "sfi=off" isn't likely to be very useful, for
1. SFI is used only when ACPI is not available
2. Today's SFI systems are not legacy PC-compatible

ie. "sfi=off" on an ACPI-platform is a NO-OP,
and "sfi=off" on an SFI-platform will likely result in boot failure.

Signed-off-by Feng Tang <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 Documentation/kernel-parameters.txt |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

Documentation/kernel-parameters.txt +5 -0

index d77fbd8..68337e6 100644
@@ -92,6 +92,7 @@ parameter is applicable:
 	SECURITY Different security models are enabled.
 	SELINUX SELinux support is enabled.
 	SERIAL	Serial support is enabled.
+	SFI	Simple Firmware Interface
 	SH	SuperH architecture is enabled.
 	SMP	The kernel is an SMP kernel.
 	SPARC	Sparc architecture is enabled.

@@ -2162,6 +2163,10 @@ and is between 256 and 4096 characters. It is defined in the file
 			If enabled at boot time, /selinux/disable can be used
 			later to disable prior to initial policy load.
 
+	sfi=		[SFI,X86] Simple Firmware Interface
+			Format: { "off" }
+			off -- disable SFI
+
 	serialnumber	[BUGS=X86-32]
 
 	shapers=	[NET]

Quoted 8 lines

From: Feng Tang <[email protected]>

"sfi=off" is analogous to "acpi=off"

In practice, "sfi=off" isn't likely to be very useful, for
1. SFI is used only when ACPI is not available
2. Today's SFI systems are not legacy PC-compatible

Quoted 8 lines

ie. "sfi=off" on an ACPI-platform is a NO-OP,
and "sfi=off" on an SFI-platform will likely result in boot failure.

Signed-off-by Feng Tang <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 Documentation/kernel-parameters.txt |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

I think the documentation update should be part of the patch that changes the functionality, so the doc always matches the code.

From: Feng Tang <[email protected]>

include/linux/include/sfi.h defines everything that customers of SFI need to know in order to use the SFI suport in the kernel.

The primary API is sfi_table_parse(), where a driver or another part of the kernel can supply a handler to parse the named table.

sfi.h also includes the currently defined table signatures and table formats.

Signed-off-by Feng Tang <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 include/linux/sfi.h |  198 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 198 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/sfi.h

include/linux/sfi.h new file +46 -0

new file mode 100644
index 0000000..f00f4da
@@ -0,0 +1,198 @@
+/* sfi.h Simple Firmware Interface */
+
+/*
+ * Copyright (C) 2009, Intel Corp.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions, and the following disclaimer,
+ *    without modification.
+ * 2. Redistributions in binary form must reproduce at minimum a disclaimer
+ *    substantially similar to the "NO WARRANTY" disclaimer below
+ *    ("Disclaimer") and any redistribution must be conditioned upon
+ *    including a substantially similar Disclaimer requirement for further
+ *    binary redistribution.
+ * 3. Neither the names of the above-listed copyright holders nor the names
+ *    of any contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ *
+ * NO WARRANTY
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * HOLDERS OR CONTRIBUTORS BE LIABLE FOR SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING
+ * IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGES.
+ */
+
+#ifndef _LINUX_SFI_H
+#define _LINUX_SFI_H
+
+/* Table signatures reserved by the SFI specification */
+#define SFI_SIG_SYST		"SYST"
+#define SFI_SIG_FREQ		"FREQ"
+#define SFI_SIG_IDLE		"IDLE"

include/linux/sfi.h new file +12 -0

+	u32	irq;
+} __packed;
+
+struct sfi_spi_table_entry {
+	u16	host_num;	/* attached to host 0, 1...*/
+	u16	cs;		/* chip select */
+	u16	irq_info;
+	char	name[16];
+	u8	dev_info[10];
+} __packed;
+
+struct sfi_i2c_table_entry {

include/linux/sfi.h new file +8 -0

+
+extern int sfi_disabled;
+static inline void disable_sfi(void)
+{
+	sfi_disabled = 1;
+}
+
+#else /* !CONFIG_SFI */

From: Feng Tang <[email protected]>

drivers/sfi/sfi_core.c contains the generic SFI implementation. It has a private header, sfi_core.h, for its own use and the private use of future files in drivers/sfi/

Signed-off-by Feng Tang <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 drivers/Makefile       |    1 +
 drivers/sfi/Makefile   |    1 +
 drivers/sfi/sfi_core.c |  385 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/sfi/sfi_core.h |   44 ++++++
 4 files changed, 431 insertions(+), 0 deletions(-)
 create mode 100644 drivers/sfi/Makefile
 create mode 100644 drivers/sfi/sfi_core.c
 create mode 100644 drivers/sfi/sfi_core.h

drivers/Makefile +1 -0

index bc4205d..ccfa259 100644
@@ -11,6 +11,7 @@ obj-$(CONFIG_PARISC)		+= parisc/
 obj-$(CONFIG_RAPIDIO)		+= rapidio/
 obj-y				+= video/
 obj-$(CONFIG_ACPI)		+= acpi/
+obj-$(CONFIG_SFI)		+= sfi/
 # PnP must come after ACPI since it will eventually need to check if acpi
 # was used and do nothing if so
 obj-$(CONFIG_PNP)		+= pnp/

drivers/sfi/Makefile new file +1 -0

new file mode 100644
index 0000000..f176470
@@ -0,0 +1 @@
+obj-y	+= sfi_core.o

drivers/sfi/sfi_core.c new file +53 -0

new file mode 100644
index 0000000..f9a9849
@@ -0,0 +1,385 @@
+/* sfi_core.c Simple Firmware Interface - core internals */
+
+/*
+ * Copyright (C) 2009, Intel Corp.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions, and the following disclaimer,
+ *    without modification.
+ * 2. Redistributions in binary form must reproduce at minimum a disclaimer
+ *    substantially similar to the "NO WARRANTY" disclaimer below
+ *    ("Disclaimer") and any redistribution must be conditioned upon
+ *    including a substantially similar Disclaimer requirement for further
+ *    binary redistribution.
+ * 3. Neither the names of the above-listed copyright holders nor the names
+ *    of any contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ *
+ * NO WARRANTY
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * HOLDERS OR CONTRIBUTORS BE LIABLE FOR SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING
+ * IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGES.
+ */
+
+#define KMSG_COMPONENT "SFI"
+#define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
+
+#include <linux/bootmem.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/errno.h>
+#include <linux/types.h>
+#include <linux/acpi.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/smp.h>
+#include <linux/sfi.h>

drivers/sfi/sfi_core.c new file +79 -0

+int sfi_disabled __read_mostly;
+EXPORT_SYMBOL(sfi_disabled);
+
+static u64 syst_pa __read_mostly;
+static struct sfi_table_simple *syst_va __read_mostly;
+
+/*
+ * FW creates and saves the SFI tables in memory. When these tables get
+ * used, they may need to be mapped to virtual address space, and the mapping
+ * can happen before or after the ioremap() is ready, so a flag is needed
+ * to indicating this
+ */
+static u32 sfi_use_ioremap __read_mostly;
+
+static void __iomem *sfi_map_memory(u64 phys, u32 size)
+{
+	if (!phys || !size)
+		return NULL;
+
+	if (sfi_use_ioremap)
+		return ioremap(phys, size);
+	else
+		return early_ioremap(phys, size);
+}
+
+static void sfi_unmap_memory(void __iomem *virt, u32 size)
+{
+	if (!virt || !size)
+		return;
+
+	if (sfi_use_ioremap)
+		iounmap(virt);
+	else
+		early_iounmap(virt, size);
+}
+
+static void sfi_print_table_header(unsigned long long pa,
+				struct sfi_table_header *header)
+{
+	pr_info("%4.4s %llX, %04X (v%d %6.6s %8.8s)\n",
+		header->signature, pa,
+		header->length, header->revision, header->oem_id,
+		header->oem_table_id);
+}
+
+/*
+ * sfi_verify_table()
+ * sanity check table lengh, calculate checksum
+ */
+static __init int sfi_verify_table(struct sfi_table_header *table)
+{
+
+	u8 checksum = 0;
+	u8 *puchar = (u8 *)table;
+	u32 length = table->length;
+
+	/* Sanity check table length against arbitrary 1MB limit */
+	if (length > 0x100000) {
+		pr_err("Invalid table length 0x%x\n", length);
+		return -1;
+	}
+
+	while (length--)
+		checksum += *puchar++;
+
+	if (checksum) {
+		pr_err("Checksum %2.2X should be %2.2X\n",
+			table->checksum, table->checksum - checksum);
+		return -1;
+	}
+	return 0;
+}
+
+/*
+ * sfi_map_table()
+ *
+ * Return address of mapped table
+ * Check for common case that we can re-use mapping to SYST,
+ * which requires syst_pa, syst_va to be initialized.

drivers/sfi/sfi_core.c new file +41 -0

+}
+
+/*
+ * sfi_unmap_table()
+ *
+ * undoes effect of sfi_map_table() by unmapping table
+ * if it did not completely fit on same page as SYST.
+ */
+void sfi_unmap_table(struct sfi_table_header *th)
+{
+	if (!TABLE_ON_PAGE(syst_va, th, th->length))
+		sfi_unmap_memory(th, TABLE_ON_PAGE(th, th, th->length) ?
+					sizeof(*th) : th->length);
+}
+
+/*
+ * sfi_get_table()
+ *
+ * Search SYST for the specified table.
+ * return the mapped table
+ */
+static struct sfi_table_header *sfi_get_table(char *signature, char *oem_id,
+		char *oem_table_id, unsigned int flags)
+{
+	struct sfi_table_header *th;
+	u32 tbl_cnt, i;
+
+	tbl_cnt = SFI_GET_NUM_ENTRIES(syst_va, u64);
+
+	for (i = 0; i < tbl_cnt; i++) {
+		th = sfi_map_table(syst_va->pentry[i]);
+		if (!th)
+			return NULL;
+
+		if (strncmp(th->signature, signature, SFI_SIGNATURE_SIZE))
+			goto loop_continue;
+
+		if (oem_id && strncmp(th->oem_id, oem_id, SFI_OEM_ID_SIZE))
+			goto loop_continue;
+
+		if (oem_table_id && strncmp(th->oem_table_id, oem_table_id,

drivers/sfi/sfi_core.c new file +11 -0

+}
+
+/* find table with signature, run handler on it */
+int sfi_table_parse(char *signature, char *oem_id, char *oem_table_id,
+			unsigned int flags, sfi_table_handler handler)
+{
+	int ret = 0;
+	struct sfi_table_header *table = NULL;
+
+	if (sfi_disabled || !handler || !signature)
+		return -EINVAL;

From: Feng Tang <[email protected]>

Both ACPI and SFI x86 systems will use these io_apic routines:

uniq_ioapic_id(u8 id);

From: Feng Tang <[email protected]>

arch/x86/kernel/sfi.c serves the dual-purpose of supporting the SFI core with arch specific code, as well as a home for the arch-specific code that uses SFI.

Signed-off-by Feng Tang <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 arch/x86/kernel/Makefile |    1 +
 arch/x86/kernel/sfi.c    |  284 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/sfi/sfi_core.c   |    2 +-
 3 files changed, 286 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/kernel/sfi.c

arch/x86/kernel/Makefile +1 -0

index 6c327b8..e430123 100644
@@ -53,6 +53,7 @@ obj-y				+= step.o
 obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
 obj-y				+= cpu/
 obj-y				+= acpi/
+obj-$(CONFIG_SFI)		+= sfi.o
 obj-y				+= reboot.o
 obj-$(CONFIG_MCA)		+= mca_32.o
 obj-$(CONFIG_X86_MSR)		+= msr.o

arch/x86/kernel/sfi.c new file +58 -0

new file mode 100644
index 0000000..a96ea0f
@@ -0,0 +1,284 @@
+/*
+ *  sfi.c - SFI Boot Support (refer acpi/boot.c)
+ *
+ *  Copyright (C) 2008-2009	Intel Corporation
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#define KMSG_COMPONENT "SFI"
+#define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
+
+#include <linux/bootmem.h>
+#include <linux/cpumask.h>
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/init.h>
+#include <linux/efi.h>
+#include <linux/irq.h>
+#include <linux/sfi.h>
+#include <linux/io.h>
+
+#include <asm/io_apic.h>
+#include <asm/pgtable.h>
+#include <asm/mpspec.h>
+#include <asm/setup.h>
+#include <asm/apic.h>
+#include <asm/e820.h>
+
+#ifdef CONFIG_X86_LOCAL_APIC
+static unsigned long sfi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE;
+#endif
+
+#ifdef CONFIG_X86_IO_APIC
+static struct mp_ioapic_routing {
+	int	gsi_base;
+	int	gsi_end;
+} mp_ioapic_routing[MAX_IO_APICS];
+#endif
+
+static __init struct sfi_table_simple *sfi_early_find_syst(void)
+{
+	unsigned long i;

arch/x86/kernel/sfi.c new file +6 -0

+/*
+ * called in a early boot phase before the paging table is created,
+ * setup a mmap table in e820 format
+ */
+int __init sfi_init_memory_map(void)
+{

arch/x86/kernel/sfi.c new file +8 -0

+	if (!mmapt) {
+		pr_warning("could not find a valid memory map table\n");
+		return -1;
+	}
+
+	/* refer copy_e820_memory() */
+	num = SFI_GET_NUM_ENTRIES(mmapt, struct sfi_mem_entry);
+	mentry = (struct sfi_mem_entry *)mmapt->pentry;

arch/x86/kernel/sfi.c new file +14 -0

+		/* translate SFI mmap type to E820 map type */
+		switch (mentry->type) {
+		case EFI_CONVENTIONAL_MEMORY:
+			type = E820_RAM;
+			break;
+		case EFI_UNUSABLE_MEMORY:
+			type = E820_UNUSABLE;
+			break;
+		default:
+			type = E820_RESERVED;
+		}
+
+		e820_add_region(start, size, type);
+		mentry++;

arch/x86/kernel/sfi.c new file +37 -0

+
+	if (boot_cpu_physical_apicid == -1U)
+		boot_cpu_physical_apicid = read_apic_id();
+
+	pr_debug("Boot CPU = %d\n", boot_cpu_physical_apicid);
+}
+
+/* All CPUs enumerated by SFI must be present and enabled */
+void __cpuinit mp_sfi_register_lapic(u8 id)
+{
+	int boot_cpu = 0;
+
+	if (MAX_APICS - id <= 0) {
+		pr_warning("Processor #%d invalid (max %d)\n",
+			id, MAX_APICS);
+		return;
+	}
+
+	if (id == boot_cpu_physical_apicid)
+		boot_cpu = 1;
+	pr_info("registering lapic[%d]\n", id);
+
+	generic_processor_info(id, GET_APIC_VERSION(apic_read(APIC_LVR)));
+}
+
+static int __init sfi_parse_cpus(struct sfi_table_header *table)
+{
+	struct sfi_table_simple *sb;
+	struct sfi_cpu_table_entry *pentry;
+	int i;
+	int cpu_num;
+
+	sb = (struct sfi_table_simple *)table;
+	cpu_num = SFI_GET_NUM_ENTRIES(sb, struct sfi_cpu_table_entry);
+	pentry = (struct sfi_cpu_table_entry *)sb->pentry;
+
+	for (i = 0; i < cpu_num; i++) {

arch/x86/kernel/sfi.c new file +14 -0

+	}
+	if (!paddr) {
+		pr_warning("WARNING: Bogus (zero) I/O APIC address"
+			" found in MADT table, skipping!\n");
+		return;
+	}
+
+	idx = nr_ioapics;
+
+	mp_ioapics[idx].type = MP_IOAPIC;
+	mp_ioapics[idx].flags = MPC_APIC_USABLE;
+	mp_ioapics[idx].apicaddr = paddr;
+
+	set_fixmap_nocache(FIX_IO_APIC_BASE_0 + idx, paddr);

arch/x86/kernel/sfi.c new file +43 -0

+
+	/*
+	 * Build basic GSI lookup table to facilitate gsi->io_apic lookups
+	 * and to prevent reprogramming of IOAPIC pins (PCI GSIs).
+	 */
+	mp_ioapic_routing[idx].gsi_base = gsi_base;
+	mp_ioapic_routing[idx].gsi_end = gsi_base +
+		io_apic_get_redir_entries(idx);
+	gsi_base = mp_ioapic_routing[idx].gsi_end + 1;
+
+	pr_info("IOAPIC[%d]: apic_id %d, version %d, address 0x%x, "
+		"GSI %d-%d\n",
+		idx, mp_ioapics[idx].apicid,
+		mp_ioapics[idx].apicver, (u32)mp_ioapics[idx].apicaddr,
+		mp_ioapic_routing[idx].gsi_base,
+		mp_ioapic_routing[idx].gsi_end);
+
+	nr_ioapics++;
+}
+
+static int __init sfi_parse_ioapic(struct sfi_table_header *table)
+{
+	struct sfi_table_simple *sb;
+	struct sfi_apic_table_entry *pentry;
+	int i, num;
+
+	sb = (struct sfi_table_simple *)table;
+	num = SFI_GET_NUM_ENTRIES(sb, struct sfi_apic_table_entry);
+	pentry = (struct sfi_apic_table_entry *)sb->pentry;
+
+	for (i = 0; i < num; i++) {
+		mp_sfi_register_ioapic(i, pentry->phy_addr);
+		pentry++;
+	}
+
+	WARN(pic_mode, KERN_WARNING
+		"SFI: pic_mod shouldn't be 1 when IOAPIC table is present\n");
+	pic_mode = 0;
+	return 0;
+}
+#endif /* CONFIG_X86_IO_APIC */
+
+/*

Quoted 21 lines · Len Brown

From: Feng Tang <[email protected]>

arch/x86/kernel/sfi.c serves the dual-purpose of supporting the SFI core with arch specific code, as well as a home for the arch-specific code that uses SFI.

Signed-off-by Feng Tang <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 arch/x86/kernel/Makefile |    1 +
 arch/x86/kernel/sfi.c    |  284

++++++++++++++++++++++++++++++++++++++++++++++
drivers/sfi/sfi_core.c | 2 +- 3 files changed, 286
insertions(+), 1 deletions(-) create mode 100644
arch/x86/kernel/sfi.c

arch/x86/kernel/Makefile +0 -0

index 6c327b8..e430123 100644

arch/x86/kernel/Makefile obj-y += step.o · line 59

Quoted 7 lines

arch/x86/kernel/Makefile +1 -0

@@ -53,6 +53,7 @@ obj-y                               += step.o
 obj-$(CONFIG_STACKTRACE)     += stacktrace.o
 obj-y                                += cpu/
 obj-y                                += acpi/
+obj-$(CONFIG_SFI)            += sfi.o
 obj-y                                += reboot.o
 obj-$(CONFIG_MCA)            += mca_32.o
 obj-$(CONFIG_X86_MSR)                += msr.o

and write those 'any match' lines as:

        sfi_table_parse(SFI_SIG_CPUS, SFI_ANY_KEY, sfi_parse_cpus);

which is _far_ more readable and more extensible as well. (it is trivial to extend such a design with a new key field - while with the current open-coded structure it's far more invasive to do such a change.)

Thanks,

Ingo

From: Feng Tang <[email protected]>

Signed-off-by Feng Tang <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 arch/x86/kernel/e820.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

arch/x86/kernel/e820.c +5 -0

index c4ca89d..e399d0e 100644
@@ -20,6 +20,7 @@
 #include <linux/pfn.h>
 #include <linux/suspend.h>
 #include <linux/firmware-map.h>
+#include <linux/sfi.h>
 
 #include <asm/pgtable.h>
 #include <asm/page.h>

@@ -1437,6 +1438,10 @@ char *__init default_machine_specific_memory_setup(void)
 	  < 0) {
 		u64 mem_size;
 
+		/* if SFI mmap table exists, use SFI to setup e820 mmap */
+		if (!sfi_init_memory_map())
+			return "SFI";
+
 		/* compare results from other methods and take the greater */
 		if (boot_params.alt_mem_k
 		    < boot_params.screen_info.ext_mem_k) {

This bothers me... we keep saying that memory map initialization belongs to the boot loader, and yet we keep doing the opposite. This isn't just an arbitrary difference, either: it is pretty essential to being able to use the early range allocator safely.

-hpa

Quoted 32 lines · Len Brown

From: Feng Tang <[email protected]>

Signed-off-by Feng Tang <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 arch/x86/kernel/e820.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

arch/x86/kernel/e820.c +2 -0

index c4ca89d..e399d0e 100644
@@ -20,6 +20,7 @@
 #include <linux/pfn.h>
 #include <linux/suspend.h>
 #include <linux/firmware-map.h>
+#include <linux/sfi.h>
 
 #include <asm/pgtable.h>
 #include <asm/page.h>

@@ -1437,6 +1438,10 @@ char *__init
 default_machine_specific_memory_setup(void) < 0) {
 		u64 mem_size;
 
+		/* if SFI mmap table exists, use SFI to setup e820

mmap */

arch/x86/kernel/e820.c +3 -0

+		if (!sfi_init_memory_map())
+			return "SFI";
+
 		/* compare results from other methods and take the

greater */ if (boot_params.alt_mem_k
< boot_params.screen_info.ext_mem_k) {

Hi hpa,

I understand your concern, and I've added that code into our boot loader for Moosrestown which sets up a e820 memory table in boot parameters by parsing SFI table.

The reason we still keep this piece of code is to be compatible with old version boot loaders which may not know SFI info yet. And sfi_init_memory_map() only get called when kernel can't find an e820 table in boot parameters.

Anyway, I think we can remove the code if it really breaks the rule.

Thanks,
Feng

From: Feng Tang <[email protected]>

+++ b/drivers/sfi/sfi_acpi.c

+static struct acpi_table_xsdt *xsdt_va;

drivers/sfi/sfi_acpi.c new file +22 -0

+static struct acpi_table_header *sfi_acpi_get_table(char *signature,
+			char *oem_id, char *oem_table_id, unsigned int flags)
+{
+	struct acpi_table_header *th;
+	u32 tbl_cnt, i;
+	u64 pa;
+
+	tbl_cnt = XSDT_GET_NUM_ENTRIES(xsdt_va, u64);
+
+	for (i = 0; i < tbl_cnt; i++) {
+		pa = xsdt_va->table_offset_entry[i];
+
+		th = (struct acpi_table_header *)sfi_map_table(pa);
+		if (!th)
+			return NULL;
+
+		if (strncmp(th->signature, signature, SFI_SIGNATURE_SIZE))
+			goto loop_continue;
+
+		if (oem_id && strncmp(th->oem_id, oem_id, SFI_OEM_ID_SIZE))
+			goto loop_continue;
+

+++ b/include/linux/sfi_acpi.h

+#ifdef CONFIG_SFI

From: Feng Tang <[email protected]>

First check ACPI, and if that fails, ask SFI to find the MCFG.

Cc Jesse Barnes <[email protected]>

Signed-off-by Feng Tang <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 arch/x86/Kconfig               |    2 +-
 arch/x86/pci/mmconfig-shared.c |    5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

arch/x86/Kconfig +0 -0

index 8e864c0..8e20a0e 100644

arch/x86/pci/mmconfig-shared.c static void __init __pci_mmcfg_init(int early) · line 614

Quoted 11 lines

arch/x86/pci/mmconfig-shared.c +3 -1

@@ -606,7 +607,9 @@ static void __init __pci_mmcfg_init(int early)
 	}
 
 	if (!known_bridge)
-		acpi_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg);
+		if (acpi_table_parse(ACPI_SIG_MCFG,
 pci_parse_mcfg))
+			sfi_acpi_table_parse(ACPI_SIG_MCFG, NULL,
 NULL, 0,
+				pci_parse_mcfg);

Please introduce one common/generic helper:

		x86_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg);

and do the fallback in that helper. We generally want to try ACPI first, SFI second. That helper makes it easier to add such fallback in other places as well, and will de-uglify the above code as well.

Ingo

Quoted 16 lines · On Fri, 10 Jul 2009 13:52:29 +0800 Ingo Molnar <[email protected]> wrote:

arch/x86/pci/mmconfig-shared.c static void __init __pci_mmcfg_init(int early) · line 614

Quoted 11 lines

arch/x86/pci/mmconfig-shared.c +3 -1

@@ -606,7 +607,9 @@ static void __init __pci_mmcfg_init(int early)
 	}
 
 	if (!known_bridge)
-		acpi_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg);
+		if (acpi_table_parse(ACPI_SIG_MCFG,
 pci_parse_mcfg))
+			sfi_acpi_table_parse(ACPI_SIG_MCFG, NULL,
 NULL, 0,
+				pci_parse_mcfg);

Please introduce one common/generic helper:

		x86_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg);

Quoted 4 lines

and do the fallback in that helper. We generally want to try ACPI first, SFI second. That helper makes it easier to add such fallback in other places as well, and will de-uglify the above code as well.

Should we have a new acpi_sfi.c or .h to contain all these helper functions? I think it is not appropriate to put it to either ACPI or SFI code.

Also, ACPI and SFI code under arch/x86/kernel have lots of similar code in cpu/io-apic parsing, we thought about extracting these sharable codes out and move them to apic.c/io_apic.c, but don't know if this will uglify current apic/ioapic code? how do you think about it?

Feng Tang (10):
SFI, x86: add CONFIG_SFI
SFI: document boot param "sfi=off"
SFI: create include/linux/sfi.h
SFI: add core support
ACPI, x86: remove ACPI dependency on some IO-APIC routines
SFI: add x86 support
SFI, x86: hook e820() for memory map initialization
SFI: Enable SFI to parse ACPI tables
SFI, PCI: Hook MMCONFIG
SFI: add boot-time initialization hooks

      SFI, x86:  add CONFIG_SFI
      SFI:       document boot param "sfi=off"
      SFI:       create include/linux/sfi.h
      SFI:       add core support

ACPI, x86: remove ACPI dependency on some IO-APIC routines
SFI: add x86 support
SFI, x86: hook e820() for memory map initialization
SFI: Enable SFI to parse ACPI tables
SFI, PCI: Hook MMCONFIG
SFI: add boot-time initialization hooks

      SFI, x86:  Add CONFIG_SFI
      SFI:       Document boot param "sfi=off"
      SFI:       Create include/linux/sfi.h
      SFI:       Add core support

ACPI, x86: Remove ACPI dependency on some IO-APIC routines
SFI: Add x86 support
SFI, x86: Hook e820() for memory map initialization
SFI: Enable SFI to parse ACPI tables
SFI, PCI: Hook MMCONFIG
SFI: Add boot-time initialization hooks

Quoted 13 lines

Feng Tang (10):
SFI, x86: add CONFIG_SFI
SFI: document boot param "sfi=off"
SFI: create include/linux/sfi.h
SFI: add core support
ACPI, x86: remove ACPI dependency on some IO-APIC routines
SFI: add x86 support
SFI, x86: hook e820() for memory map initialization
SFI: Enable SFI to parse ACPI tables
SFI, PCI: Hook MMCONFIG
SFI: add boot-time initialization hooks

Now that commit quality is all the rage, let me point out a (trivial) detail that could be improved: patch title capitalization.

In particular for commits touching arch/x86/ we try to standardize on capitalizing the word after the 'x86: ' tag. But in the above titles the capitalization is mixed-case:

[aligned vertically for easier readability ]

Quoted 10 lines

      SFI, x86:  add CONFIG_SFI
      SFI:       document boot param "sfi=off"
      SFI:       create include/linux/sfi.h
      SFI:       add core support

ACPI, x86: remove ACPI dependency on some IO-APIC routines
SFI: add x86 support
SFI, x86: hook e820() for memory map initialization
SFI: Enable SFI to parse ACPI tables
SFI, PCI: Hook MMCONFIG
SFI: add boot-time initialization hooks

So it would be more consistent to have:

Quoted 10 lines

      SFI, x86:  Add CONFIG_SFI
      SFI:       Document boot param "sfi=off"
      SFI:       Create include/linux/sfi.h
      SFI:       Add core support

ACPI, x86: Remove ACPI dependency on some IO-APIC routines
SFI: Add x86 support
SFI, x86: Hook e820() for memory map initialization
SFI: Enable SFI to parse ACPI tables
SFI, PCI: Hook MMCONFIG
SFI: Add boot-time initialization hooks

( Capitalizing like that has the added (minor) benefit of making it
slightly easier to read these titles - the 'real' natural language sentence portion of the title begins with a capital letter. )

Quoted 47 lines

Quoted 13 lines

Feng Tang (10):
SFI, x86: add CONFIG_SFI
SFI: document boot param "sfi=off"
SFI: create include/linux/sfi.h
SFI: add core support
ACPI, x86: remove ACPI dependency on some IO-APIC routines
SFI: add x86 support
SFI, x86: hook e820() for memory map initialization
SFI: Enable SFI to parse ACPI tables
SFI, PCI: Hook MMCONFIG
SFI: add boot-time initialization hooks

Now that commit quality is all the rage, let me point out a (trivial) detail that could be improved: patch title capitalization.

In particular for commits touching arch/x86/ we try to standardize on capitalizing the word after the 'x86: ' tag. But in the above titles the capitalization is mixed-case:

[aligned vertically for easier readability ]

Quoted 10 lines

      SFI, x86:  add CONFIG_SFI
      SFI:       document boot param "sfi=off"
      SFI:       create include/linux/sfi.h
      SFI:       add core support

ACPI, x86: remove ACPI dependency on some IO-APIC routines
SFI: add x86 support
SFI, x86: hook e820() for memory map initialization
SFI: Enable SFI to parse ACPI tables
SFI, PCI: Hook MMCONFIG
SFI: add boot-time initialization hooks

So it would be more consistent to have:

Quoted 10 lines

      SFI, x86:  Add CONFIG_SFI
      SFI:       Document boot param "sfi=off"
      SFI:       Create include/linux/sfi.h
      SFI:       Add core support

ACPI, x86: Remove ACPI dependency on some IO-APIC routines
SFI: Add x86 support
SFI, x86: Hook e820() for memory map initialization
SFI: Enable SFI to parse ACPI tables
SFI, PCI: Hook MMCONFIG
SFI: Add boot-time initialization hooks

Quoted 3 lines

( Capitalizing like that has the added (minor) benefit of making it
slightly easier to read these titles - the 'real' natural language sentence portion of the title begins with a capital letter. )

Ingo,
While I really do appreciate your attention to detail, a little time off might be good for you:-)