[PATCH v2 for 2.6.32] SFI - Simple Firmware Interface support

[PATCH v2 for 2.6.32] SFI - Simple Firmware Interface support

[Len Brown]

Here is version 2 of the SFI patch series for Linux 2.6.32.
This series is based on 2.6.31-rc2, and is available in git:

git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-sfi-2.6.git sfi-test

We've tried to respond to all feedback on the v1 series.
In particular, thanks to Ingo Molnar and Andi Kleen for their thorough review.

We followed Andi's advice and deleted the data structure
to track tables, and instead we access all table headers in-place.
Indeed, this shrunk the memory footprint a bit.

We also now detect and optimize for the common case
where all SFI tables fit on a single page.

We added some small bits from the upcoming draft of SFI 0.7,
which should be available on http://simplefirmware.org shortly.
Of note, we can assert that the SYST shall not cross a 4K boundary.

Thanks,
-Len Brown & Tang-Feng, Intel Open Source Technology Center

---
 Documentation/kernel-parameters.txt |    5 +
 MAINTAINERS                         |   12 +
 arch/x86/Kconfig                    |    4 +-
 arch/x86/include/asm/io_apic.h      |    3 +-
 arch/x86/kernel/Makefile            |    1 +
 arch/x86/kernel/acpi/boot.c         |   22 --
 arch/x86/kernel/apic/io_apic.c      |   28 ++-
 arch/x86/kernel/e820.c              |    5 +
 arch/x86/kernel/setup.c             |    3 +
 arch/x86/kernel/sfi.c               |  284 +++++++++++++++++++++++++
 arch/x86/pci/mmconfig-shared.c      |    5 +-
 drivers/Makefile                    |    1 +
 drivers/acpi/tables.c               |    3 +
 drivers/sfi/Kconfig                 |   16 ++
 drivers/sfi/Makefile                |    3 +
 drivers/sfi/sfi_acpi.c              |  151 ++++++++++++++
 drivers/sfi/sfi_core.c              |  387 +++++++++++++++++++++++++++++++++++
 drivers/sfi/sfi_core.h              |   44 ++++
 include/linux/sfi.h                 |  198 ++++++++++++++++++
 include/linux/sfi_acpi.h            |   60 ++++++
 init/main.c                         |    2 +
 21 files changed, 1204 insertions(+), 33 deletions(-)
 create mode 100644 arch/x86/kernel/sfi.c
 create mode 100644 drivers/sfi/Kconfig
 create mode 100644 drivers/sfi/Makefile
 create mode 100644 drivers/sfi/sfi_acpi.c
 create mode 100644 drivers/sfi/sfi_core.c
 create mode 100644 drivers/sfi/sfi_core.h
 create mode 100644 include/linux/sfi.h
 create mode 100644 include/linux/sfi_acpi.h

commits:

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

Len Brown (2):
      SFI: Simple Firmware Interface - new MAINTAINERS entry
      ACPI: check acpi_disabled in acpi_table_parse()

with this log:

commit 7d8801c4e11f26a68fca70798dc40d5f405b4be2
Author: Feng Tang <feng.tang@intel.com>
Date:   Tue Jul 7 23:34:39 2009 -0400

    SFI: add boot-time initialization hooks
    
    There are two SFI boot-time hooks:
    
    sfi_init() maps the SYST using early_ioremap() and validates all the tables.
    
    sfi_init_late() re-maps SYST with ioremap(), and initializes
    the ACPI extension, if present.
    
    Signed-off-by: Feng Tang <feng.tang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 24fe646fcbbd9049850de4ac57cf6a67846b38c4
Author: Feng Tang <feng.tang@intel.com>
Date:   Tue Jul 7 23:30:32 2009 -0400

    SFI, PCI: Hook MMCONFIG
    
    First check ACPI, and if that fails, ask SFI to find the MCFG.
    
    Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
    Signed-off-by: Feng Tang <feng.tang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 4e176b0f00143e2dfca4480402d3b27b3a0f835f
Author: Len Brown <len.brown@intel.com>
Date:   Tue Jul 7 23:22:58 2009 -0400

    ACPI: check acpi_disabled in acpi_table_parse()
    
    Allow consumers of the acpi_table_parse() API
    to gracefully handle the acpi_disabled=1 case
    without checking the flag themselves.
    
    Signed-off-by: Feng Tang <feng.tang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 5ab9dd34acc6209c5b6a3c754075e408e5298a2d
Author: Feng Tang <feng.tang@intel.com>
Date:   Tue Jul 7 23:15:22 2009 -0400

    SFI: Enable SFI to parse ACPI tables
    
    Extend SFI to access standard ACPI tables.
    (eg. the PCI MCFG) using sfi_acpi_table_parse().
    
    Note that this is _not_ a hybrid ACPI + SFI mode.
    The platform boots in either ACPI mode or SFI mode.
    
    SFI runs only with acpi_disabled=1, which can be set
    at build-time via CONFIG_ACPI=n, or at boot time by
    the failure to find ACPI platform support.
    
    So this extension simply allows SFI-platforms to
    re-use existing standard table formats that happen to
    be defined to live in ACPI envelopes.
    
    Signed-off-by: Feng Tang <feng.tang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 5bf6b3c7c08a76ea8dc52e9e07728c2958938952
Author: Feng Tang <feng.tang@intel.com>
Date:   Tue Jul 7 23:11:44 2009 -0400

    SFI, x86: hook e820() for memory map initialization
    
    Signed-off-by: Feng Tang <feng.tang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 4676b1fee4cae65c678754fbdecae626ac161b81
Author: Feng Tang <feng.tang@intel.com>
Date:   Tue Jul 7 23:09:19 2009 -0400

    SFI: add x86 support
    
    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 <feng.tang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit bf8ea7cda3ad3f287f1c9164d366a94eae07a4a5
Author: Feng Tang <feng.tang@intel.com>
Date:   Tue Jul 7 23:01:15 2009 -0400

    ACPI, x86: remove ACPI dependency on some IO-APIC routines
    
    Both ACPI and SFI x86 systems will use these io_apic routines:
    
    uniq_ioapic_id(u8 id);
    io_apic_get_unique_id(int ioapic, int apic_id);
    io_apic_get_version(int ioapic);
    io_apic_get_redir_entries(int ioapic);
    
    so move the 1st from acpi/boot.c to io_apic.c,
    and remove the #ifdef ACPI around the other three.
    
    Signed-off-by: Feng Tang <feng.tang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 10fb28adc204382cb3b1acc99eabbb369d378a0f
Author: Feng Tang <feng.tang@intel.com>
Date:   Tue Jul 7 22:54:22 2009 -0400

    SFI: add core support
    
    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 <feng.tang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 56f1291c60859b65ac62521e7fe4b82e73205ef6
Author: Feng Tang <feng.tang@intel.com>
Date:   Tue Jul 7 22:39:49 2009 -0400

    SFI: create include/linux/sfi.h
    
    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 <feng.tang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 57dac60d76c191e3bd72f186833fac01e4c5f8f1
Author: Feng Tang <feng.tang@intel.com>
Date:   Tue Jul 7 22:33:09 2009 -0400

    SFI: document boot param "sfi=off"
    
    "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 <feng.tang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit d5a8e3203627c313e8fdbf4fa9ac1cb1cdc6706c
Author: Feng Tang <feng.tang@intel.com>
Date:   Tue Jul 7 22:30:29 2009 -0400

    SFI, x86: add CONFIG_SFI
    
    analogous to ACPI, drivers/sfi/Kconfig is pulled in by arch/x86/Kconfig
    
    Signed-off-by: Feng Tang <feng.tang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

commit 8e4a93858bce74ed3080dd607aa471023f1a2737
Author: Len Brown <len.brown@intel.com>
Date:   Tue Jul 7 22:25:46 2009 -0400

    SFI: Simple Firmware Interface - new MAINTAINERS entry
    
    CONFIG_SFI=y shall enable the kernel to boot and run optimally
    on platforms that support the Simple Firmware Interface.
    
    Thanks to Jacob Pan for prototyping the initial Linux SFI support,
    and to Feng Tang for Linux bring-up and debug both in emulation
    and on Moorestown hardware.
    
    See http://simplefirmware.org for more information on SFI.
    
    Signed-off-by: Len Brown <len.brown@intel.com>

[PATCH 01/12] SFI: Simple Firmware Interface - new MAINTAINERS entry

[Len Brown]

From: Len Brown <len.brown@intel.com>

CONFIG_SFI=y shall enable the kernel to boot and run optimally
on platforms that support the Simple Firmware Interface.

Thanks to Jacob Pan for prototyping the initial Linux SFI support,
and to Feng Tang for Linux bring-up and debug both in emulation
and on Moorestown hardware.

See http://simplefirmware.org for more information on SFI.

Signed-off-by: Len Brown <len.brown@intel.com>
---
 MAINTAINERS |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 381190c..2c03d31 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5298,6 +5298,18 @@ L:	linux-pci@vger.kernel.org
 S:	Supported
 F:	drivers/pci/hotplug/shpchp*
 
+SIMPLE FIRMWARE INTERFACE (SFI)
+P:	Len Brown
+M:	lenb@kernel.org
+L:	sfi-devel@simplefirmware.org
+W:	http://simplefirmware.org/
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-sfi-2.6.git
+S:	Supported
+F:	arch/x86/kernel/*sfi*
+F:	drivers/sfi/
+F:	include/linux/sfi*.h
+
+
 SIMTEC EB110ATX (Chalice CATS)
 P:	Ben Dooks
 P:	Vincent Sanders
-- 
1.6.0.6

[PATCH 02/12] SFI, x86: add CONFIG_SFI

[Len Brown]

From: Feng Tang <feng.tang@intel.com>

analogous to ACPI, drivers/sfi/Kconfig is pulled in by arch/x86/Kconfig

Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 arch/x86/Kconfig    |    2 ++
 drivers/sfi/Kconfig |   16 ++++++++++++++++
 2 files changed, 18 insertions(+), 0 deletions(-)
 create mode 100644 drivers/sfi/Kconfig

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c07f722..8e864c0 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1683,6 +1683,8 @@ source "kernel/power/Kconfig"
 
 source "drivers/acpi/Kconfig"
 
+source "drivers/sfi/Kconfig"
+
 config X86_APM_BOOT
 	bool
 	default y
diff --git a/drivers/sfi/Kconfig b/drivers/sfi/Kconfig
new file mode 100644
index 0000000..b217f32
--- /dev/null
+++ b/drivers/sfi/Kconfig
@@ -0,0 +1,16 @@
+#
+# SFI Configuration
+#
+
+menuconfig SFI
+	bool "SFI (Simple Firmware Interface) Support"
+	depends on X86
+	default n
+	---help---
+	The Simple Firmware Interface (SFI) provides a lightweight method
+	for platform firmware to pass information to the Operating System
+	via static tables in memory.
+
+	For more information, see http://simplefirmware.org
+
+	Say 'Y' here to enable the kernel to boot on SFI-only platforms.
-- 
1.6.0.6

[PATCH 03/12] SFI: document boot param "sfi=off"

[Len Brown]

From: Feng Tang <feng.tang@intel.com>

"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 <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 Documentation/kernel-parameters.txt |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index d77fbd8..68337e6 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -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]
-- 
1.6.0.6

[PATCH 04/12] SFI: create include/linux/sfi.h

[Len Brown]

From: Feng Tang <feng.tang@intel.com>

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 <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 include/linux/sfi.h |  198 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 198 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/sfi.h

diff --git a/include/linux/sfi.h b/include/linux/sfi.h
new file mode 100644
index 0000000..f00f4da
--- /dev/null
+++ b/include/linux/sfi.h
@@ -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"
+#define SFI_SIG_CPUS		"CPUS"
+#define SFI_SIG_MTMR		"MTMR"
+#define SFI_SIG_MRTC		"MRTC"
+#define SFI_SIG_MMAP		"MMAP"
+#define SFI_SIG_APIC		"APIC"
+#define SFI_SIG_XSDT		"XSDT"
+#define SFI_SIG_WAKE		"WAKE"
+#define SFI_SIG_SPIB		"SPIB"
+#define SFI_SIG_I2CB		"I2CB"
+#define SFI_SIG_GPEM		"GPEM"
+
+#define SFI_ACPI_TABLE		(1 << 0)
+#define SFI_NORMAL_TABLE	(1 << 1)
+
+#define SFI_SIGNATURE_SIZE	4
+#define SFI_OEM_ID_SIZE		6
+#define SFI_OEM_TABLE_ID_SIZE	8
+
+#define SFI_SYST_SEARCH_BEGIN		0x000E0000
+#define SFI_SYST_SEARCH_END		0x000FFFFF
+
+#define SFI_GET_NUM_ENTRIES(ptable, entry_type) \
+	((ptable->header.length - sizeof(struct sfi_table_header)) / \
+	(sizeof(entry_type)))
+/*
+ * Table structures must be byte-packed to match the SFI specification,
+ * as they are provided by the BIOS.
+ */
+struct sfi_table_header {
+	char	signature[SFI_SIGNATURE_SIZE];
+	u32	length;
+	u8	revision;
+	u8	checksum;
+	char	oem_id[SFI_OEM_ID_SIZE];
+	char	oem_table_id[SFI_OEM_TABLE_ID_SIZE];
+} __packed;
+
+struct sfi_table_simple {
+	struct sfi_table_header		header;
+	u64				pentry[1];
+} __packed;
+
+/* comply with UEFI spec 2.1 */
+struct sfi_mem_entry {
+	u32	type;
+	u64	phy_start;
+	u64	vir_start;
+	u64	pages;
+	u64	attrib;
+} __packed;
+
+struct sfi_cpu_table_entry {
+	u32	apicid;
+} __packed;
+
+struct sfi_cstate_table_entry {
+	u32	hint;		/* MWAIT hint */
+	u32	latency;	/* latency in ms */
+} __packed;
+
+struct sfi_apic_table_entry {
+	u64	phy_addr;	/* phy base addr for APIC reg */
+} __packed;
+
+struct sfi_freq_table_entry {
+	u32	freq;
+	u32	latency;	/* transition latency in ms */
+	u32	ctrl_val;	/* value to write to PERF_CTL */
+} __packed;
+
+struct sfi_wake_table_entry {
+	u64 phy_addr;	/* pointer to where the wake vector locates */
+} __packed;
+
+struct sfi_timer_table_entry {
+	u64	phy_addr;	/* phy base addr for the timer */
+	u32	freq;		/* in HZ */
+	u32	irq;
+} __packed;
+
+struct sfi_rtc_table_entry {
+	u64	phy_addr;	/* phy base addr for the RTC */
+	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 {
+	u16	host_num;
+	u16	addr;		/* slave addr */
+	u16	irq_info;
+	char	name[16];
+	u8	dev_info[10];
+} __packed;
+
+struct sfi_gpe_table_entry {
+	u16	logical_id;	/* logical id */
+	u16	phy_id;		/* physical GPE id */
+} __packed;
+
+
+typedef int (*sfi_table_handler) (struct sfi_table_header *table);
+
+#ifdef	CONFIG_SFI
+extern int __init sfi_init_memory_map(void);
+extern void __init sfi_init(void);
+extern int __init sfi_platform_init(void);
+extern void __init sfi_init_late(void);
+
+int sfi_table_parse(char *signature, char *oem_id, char *oem_table_id,
+			uint flag, sfi_table_handler handler);
+
+extern int sfi_disabled;
+static inline void disable_sfi(void)
+{
+	sfi_disabled = 1;
+}
+
+#else /* !CONFIG_SFI */
+
+static inline int sfi_init_memory_map(void)
+{
+	return -1;
+}
+
+static inline void sfi_init(void)
+{
+	return;
+}
+
+static inline void sfi_init_late(void)
+{
+}
+
+#define sfi_disabled	0
+
+static inline int sfi_table_parse(char *signature, char *oem_id,
+					char *oem_table_id, unsigned int flags,
+					sfi_table_handler handler)
+{
+	return -1;
+}
+
+#endif /* CONFIG_SFI */
+
+#endif	/*_LINUX_SFI_H*/
-- 
1.6.0.6

[PATCH 05/12] SFI: add core support

[Len Brown]

From: Feng Tang <feng.tang@intel.com>

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 <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 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

diff --git a/drivers/Makefile b/drivers/Makefile
index bc4205d..ccfa259 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -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/
diff --git a/drivers/sfi/Makefile b/drivers/sfi/Makefile
new file mode 100644
index 0000000..f176470
--- /dev/null
+++ b/drivers/sfi/Makefile
@@ -0,0 +1 @@
+obj-y	+= sfi_core.o
diff --git a/drivers/sfi/sfi_core.c b/drivers/sfi/sfi_core.c
new file mode 100644
index 0000000..f9a9849
--- /dev/null
+++ b/drivers/sfi/sfi_core.c
@@ -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>
+
+#include <asm/pgtable.h>
+
+#include "sfi_core.h"
+
+#define ON_SAME_PAGE(addr1, addr2) \
+	(((unsigned long)(addr1) & PAGE_MASK) == \
+	((unsigned long)(addr2) & PAGE_MASK))
+#define TABLE_ON_PAGE(page, table, size) (ON_SAME_PAGE(page, table) && \
+				ON_SAME_PAGE(page, table + size))
+
+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.
+ */
+struct sfi_table_header *sfi_map_table(u64 pa)
+{
+	struct sfi_table_header *th;
+	u32 length;
+
+	if (!TABLE_ON_PAGE(syst_pa, pa, sizeof(struct sfi_table_header)))
+		th = sfi_map_memory(pa, sizeof(struct sfi_table_header));
+	else
+		th = (void *)syst_va - (syst_pa - pa);
+
+	 /* If table fits on same page as its header, we are done */
+	if (TABLE_ON_PAGE(th, th, th->length))
+		return th;
+
+	/* entire table does not fit on same page as SYST */
+	length = th->length;
+	if (!TABLE_ON_PAGE(syst_pa, pa, sizeof(struct sfi_table_header)))
+		sfi_unmap_memory(th, sizeof(struct sfi_table_header));
+
+	return sfi_map_memory(pa, length);
+}
+
+/*
+ * 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,
+						SFI_OEM_TABLE_ID_SIZE))
+			goto loop_continue;
+
+		return th;	/* success */
+loop_continue:
+		sfi_unmap_table(th);
+	}
+
+	return NULL;
+}
+
+void sfi_put_table(struct sfi_table_header *table)
+{
+	if (!ON_SAME_PAGE(((void *)table + table->length),
+		(void *)syst_va + syst_va->header.length)
+		&& !ON_SAME_PAGE(table, syst_va))
+		sfi_unmap_memory(table, table->length);
+}
+
+/* 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;
+
+	table = sfi_get_table(signature, oem_id, oem_table_id, flags);
+	if (!table)
+		return -EINVAL;
+
+	ret = handler(table);
+	sfi_put_table(table);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(sfi_table_parse);
+
+/*
+ * sfi_check_table(pa)
+ */
+int __init sfi_check_table(u64 pa)
+{
+	struct sfi_table_header *th;
+	int ret;
+
+	th = sfi_map_table(pa);
+	if (!th)
+		return -1;
+
+	sfi_print_table_header(pa, th);
+	ret = sfi_verify_table(th);
+
+	sfi_unmap_table(th);
+
+	return ret;
+}
+
+/*
+ * sfi_parse_syst()
+ * checksum all the tables in SYST and print their headers
+ *
+ * success: set syst_va, return 0
+ */
+static int __init sfi_parse_syst(void)
+{
+	int tbl_cnt, i;
+
+	syst_va = sfi_map_memory(syst_pa, sizeof(struct sfi_table_simple));
+	if (!syst_va)
+		return -1;
+
+	tbl_cnt = SFI_GET_NUM_ENTRIES(syst_va, u64);
+	for (i = 0; i < tbl_cnt; i++) {
+		if (sfi_check_table(syst_va->pentry[i]))
+			return -1;
+	}
+
+	return 0;
+}
+
+/*
+ * The OS finds the System Table by searching 16-byte boundaries between
+ * physical address 0x000E0000 and 0x000FFFFF. The OS shall search this region
+ * starting at the low address and shall stop searching when the 1st valid SFI
+ * System Table is found.
+ *
+ * success: set syst_pa, return 0
+ * fail: return -1
+ */
+static __init int sfi_find_syst(void)
+{
+	unsigned long offset, len;
+	void *start;
+
+	len = SFI_SYST_SEARCH_END - SFI_SYST_SEARCH_BEGIN;
+	start = sfi_map_memory(SFI_SYST_SEARCH_BEGIN, len);
+	if (!start)
+		return -1;
+
+	for (offset = 0; offset < len; offset += 16) {
+		struct sfi_table_header *syst_hdr;
+
+		syst_hdr = start + offset;
+		if (strncmp(syst_hdr->signature, SFI_SIG_SYST,
+				SFI_SIGNATURE_SIZE))
+			continue;
+
+		if (syst_hdr->length > PAGE_SIZE)
+			continue;
+
+		sfi_print_table_header(SFI_SYST_SEARCH_BEGIN + offset,
+					syst_hdr);
+
+		if (sfi_verify_table(syst_hdr))
+			continue;
+
+		/*
+		 * Enforce SFI spec mandate that SYST reside within a page.
+		 */
+		if (!ON_SAME_PAGE(syst_pa, syst_pa + syst_hdr->length)) {
+			pr_debug("SYST 0x%llx + 0x%x crosses page\n",
+					syst_pa, syst_hdr->length);
+			continue;
+		}
+
+		/* success */
+		syst_pa = SFI_SYST_SEARCH_BEGIN + offset;
+		sfi_unmap_memory(start, len);
+		return 0;
+	}
+
+	sfi_unmap_memory(start, len);
+	return -1;
+}
+
+void __init sfi_init(void)
+{
+	if (!acpi_disabled)
+		disable_sfi();
+
+	if (sfi_disabled)
+		return;
+
+	pr_info("Simple Firmware Interface v0.7 http://simplefirmware.org\n");
+
+	if (sfi_find_syst() || sfi_parse_syst())
+		disable_sfi();
+
+	return;
+}
+
+void __init sfi_init_late(void)
+{
+	int length;
+
+	if (sfi_disabled)
+		return;
+
+	length = syst_va->header.length;
+	sfi_unmap_memory(syst_va, sizeof(struct sfi_table_simple));
+
+	/* use ioremap now after it is ready */
+	sfi_use_ioremap = 1;
+	syst_va = sfi_map_memory(syst_pa, length);
+}
+
+static int __init sfi_parse_cmdline(char *arg)
+{
+	if (!arg)
+		return -EINVAL;
+
+	if (!strcmp(arg, "off"))
+		sfi_disabled = 1;
+
+	return 0;
+}
+
+early_param("sfi", sfi_parse_cmdline);
diff --git a/drivers/sfi/sfi_core.h b/drivers/sfi/sfi_core.h
new file mode 100644
index 0000000..cb599bf
--- /dev/null
+++ b/drivers/sfi/sfi_core.h
@@ -0,0 +1,44 @@
+/* sfi_core.h Simple Firmware Interface, internal header */
+
+/*
+ * 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.
+ */
+
+extern int __init sfi_acpi_init(void);
+extern int sfi_check_table(u64 paddr);
+extern void sfi_put_table(struct sfi_table_header *table);
+extern struct sfi_table_header *sfi_map_table(u64 pa);
+extern void sfi_unmap_table(struct sfi_table_header *th);
-- 
1.6.0.6

[PATCH 06/12] ACPI, x86: remove ACPI dependency on some IO-APIC routines

[Len Brown]

From: Feng Tang <feng.tang@intel.com>

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

uniq_ioapic_id(u8 id);
io_apic_get_unique_id(int ioapic, int apic_id);
io_apic_get_version(int ioapic);
io_apic_get_redir_entries(int ioapic);

so move the 1st from acpi/boot.c to io_apic.c,
and remove the #ifdef ACPI around the other three.

Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 arch/x86/include/asm/io_apic.h |    3 +--
 arch/x86/kernel/acpi/boot.c    |   22 ----------------------
 arch/x86/kernel/apic/io_apic.c |   28 +++++++++++++++++++++-------
 3 files changed, 22 insertions(+), 31 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index daf866e..1a097b9 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -150,11 +150,10 @@ extern int timer_through_8259;
 #define io_apic_assign_pci_irqs \
 	(mp_irq_entries && !skip_ioapic_setup && io_apic_irqs)
 
-#ifdef CONFIG_ACPI
+extern u8 uniq_ioapic_id(u8 id);
 extern int io_apic_get_unique_id(int ioapic, int apic_id);
 extern int io_apic_get_version(int ioapic);
 extern int io_apic_get_redir_entries(int ioapic);
-#endif /* CONFIG_ACPI */
 
 struct io_apic_irq_attr;
 extern int io_apic_set_pci_routing(struct device *dev, int irq,
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 6b8ca3a..a7f9e89 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -863,28 +863,6 @@ int mp_find_ioapic_pin(int ioapic, int gsi)
 	return gsi - mp_ioapic_routing[ioapic].gsi_base;
 }
 
-static u8 __init uniq_ioapic_id(u8 id)
-{
-#ifdef CONFIG_X86_32
-	if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
-	    !APIC_XAPIC(apic_version[boot_cpu_physical_apicid]))
-		return io_apic_get_unique_id(nr_ioapics, id);
-	else
-		return id;
-#else
-	int i;
-	DECLARE_BITMAP(used, 256);
-	bitmap_zero(used, 256);
-	for (i = 0; i < nr_ioapics; i++) {
-		struct mpc_ioapic *ia = &mp_ioapics[i];
-		__set_bit(ia->apicid, used);
-	}
-	if (!test_bit(id, used))
-		return id;
-	return find_first_zero_bit(used, 256);
-#endif
-}
-
 static int bad_ioapic(unsigned long address)
 {
 	if (nr_ioapics >= MAX_IO_APICS) {
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 4d0216f..5884e60 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3944,11 +3944,27 @@ int io_apic_set_pci_routing(struct device *dev, int irq,
 	return __io_apic_set_pci_routing(dev, irq, irq_attr);
 }
 
-/* --------------------------------------------------------------------------
-                          ACPI-based IOAPIC Configuration
-   -------------------------------------------------------------------------- */
-
-#ifdef CONFIG_ACPI
+u8 __init uniq_ioapic_id(u8 id)
+{
+#ifdef CONFIG_X86_32
+	if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
+	    !APIC_XAPIC(apic_version[boot_cpu_physical_apicid]))
+		return io_apic_get_unique_id(nr_ioapics, id);
+	else
+		return id;
+#else
+	int i;
+	DECLARE_BITMAP(used, 256);
+	bitmap_zero(used, 256);
+	for (i = 0; i < nr_ioapics; i++) {
+		struct mpc_ioapic *ia = &mp_ioapics[i];
+		__set_bit(ia->apicid, used);
+	}
+	if (!test_bit(id, used))
+		return id;
+	return find_first_zero_bit(used, 256);
+#endif
+}
 
 #ifdef CONFIG_X86_32
 int __init io_apic_get_unique_id(int ioapic, int apic_id)
@@ -4057,8 +4073,6 @@ int acpi_get_override_irq(int bus_irq, int *trigger, int *polarity)
 	return 0;
 }
 
-#endif /* CONFIG_ACPI */
-
 /*
  * This function currently is only a helper for the i386 smp boot process where
  * we need to reprogram the ioredtbls to cater for the cpus which have come online
-- 
1.6.0.6

[PATCH 07/12] SFI: add x86 support

[Len Brown]

From: Feng Tang <feng.tang@intel.com>

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 <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 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

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 6c327b8..e430123 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -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
diff --git a/arch/x86/kernel/sfi.c b/arch/x86/kernel/sfi.c
new file mode 100644
index 0000000..a96ea0f
--- /dev/null
+++ b/arch/x86/kernel/sfi.c
@@ -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;
+	char *pchar = (char *)SFI_SYST_SEARCH_BEGIN;
+
+	/* SFI spec defines the SYST starts at a 16-byte boundary */
+	for (i = 0; SFI_SYST_SEARCH_BEGIN + i < SFI_SYST_SEARCH_END; i += 16) {
+		if (!strncmp(SFI_SIG_SYST, pchar, SFI_SIGNATURE_SIZE))
+			return (struct sfi_table_simple *)
+					(SFI_SYST_SEARCH_BEGIN + i);
+
+		pchar += 16;
+	}
+	return NULL;
+}
+
+/*
+ * 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)
+{
+	struct sfi_table_simple *syst, *mmapt;
+	struct sfi_mem_entry *mentry;
+	unsigned long long start, end, size;
+	int i, num, type, tbl_cnt;
+	u64 *pentry;
+
+	if (sfi_disabled)
+		return -1;
+
+	/* first search the syst table */
+	syst = sfi_early_find_syst();
+	if (!syst)
+		return -1;
+
+	tbl_cnt = (syst->header.length - sizeof(struct sfi_table_header)) /
+			sizeof(u64);
+	pentry = syst->pentry;
+
+	/* walk through the syst to search the mmap table */
+	mmapt = NULL;
+	for (i = 0; i < tbl_cnt; i++) {
+		if (!strncmp(SFI_SIG_MMAP, (char *)(u32)*pentry, 4)) {
+			mmapt = (struct sfi_table_simple *)(u32)*pentry;
+			break;
+		}
+		pentry++;
+	}
+	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;
+	for (i = 0; i < num; i++) {
+		start = mentry->phy_start;
+		size = mentry->pages << PAGE_SHIFT;
+		end = start + size;
+
+		if (start > end)
+			return -1;
+
+		pr_debug("start = 0x%08x end = 0x%08x type = %d\n",
+			(u32)start, (u32)end, mentry->type);
+
+		/* 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++;
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_X86_LOCAL_APIC
+void __init mp_sfi_register_lapic_address(unsigned long address)
+{
+	mp_lapic_addr = address;
+	set_fixmap_nocache(FIX_APIC_BASE, mp_lapic_addr);
+
+	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++) {
+		mp_sfi_register_lapic(pentry->apicid);
+		pentry++;
+	}
+
+	smp_found_config = 1;
+	return 0;
+}
+#endif /* CONFIG_X86_LOCAL_APIC */
+
+#ifdef	CONFIG_X86_IO_APIC
+void __init mp_sfi_register_ioapic(u8 id, u32 paddr)
+{
+	int idx = 0;
+	int tmpid;
+	static u32 gsi_base;
+
+	if (nr_ioapics >= MAX_IO_APICS) {
+		pr_err("ERROR: Max # of I/O APICs (%d) exceeded "
+			"(found %d)\n", MAX_IO_APICS, nr_ioapics);
+		panic("Recompile kernel with bigger MAX_IO_APICS!\n");
+	}
+	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);
+	tmpid = uniq_ioapic_id(id);
+	if (tmpid == -1)
+		return;
+
+	mp_ioapics[idx].apicid = tmpid;
+#ifdef CONFIG_X86_32
+	mp_ioapics[idx].apicver = io_apic_get_version(idx);
+#else
+	mp_ioapics[idx].apicver = 0;
+#endif
+
+	/*
+	 * 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 */
+
+/*
+ * sfi_platform_init(): register lapics & io-apics
+ */
+int __init sfi_platform_init(void)
+{
+#ifdef CONFIG_X86_LOCAL_APIC
+	mp_sfi_register_lapic_address(sfi_lapic_addr);
+	sfi_table_parse(SFI_SIG_CPUS, NULL, NULL, 0, sfi_parse_cpus);
+#endif
+#ifdef CONFIG_X86_IO_APIC
+	sfi_table_parse(SFI_SIG_APIC, NULL, NULL, 0, sfi_parse_ioapic);
+#endif
+	return 0;
+}
diff --git a/drivers/sfi/sfi_core.c b/drivers/sfi/sfi_core.c
index f9a9849..8939215 100644
--- a/drivers/sfi/sfi_core.c
+++ b/drivers/sfi/sfi_core.c
@@ -350,7 +350,7 @@ void __init sfi_init(void)
 
 	pr_info("Simple Firmware Interface v0.7 http://simplefirmware.org\n");
 
-	if (sfi_find_syst() || sfi_parse_syst())
+	if (sfi_find_syst() || sfi_parse_syst() || sfi_platform_init())
 		disable_sfi();
 
 	return;
-- 
1.6.0.6

[PATCH 08/12] SFI, x86: hook e820() for memory map initialization

[Len Brown]

From: Feng Tang <feng.tang@intel.com>

Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 arch/x86/kernel/e820.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index c4ca89d..e399d0e 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -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) {
-- 
1.6.0.6

[PATCH 09/12] SFI: Enable SFI to parse ACPI tables

[Len Brown]

From: Feng Tang <feng.tang@intel.com>

Extend SFI to access standard ACPI tables.
(eg. the PCI MCFG) using sfi_acpi_table_parse().

Note that this is _not_ a hybrid ACPI + SFI mode.
The platform boots in either ACPI mode or SFI mode.

SFI runs only with acpi_disabled=1, which can be set
at build-time via CONFIG_ACPI=n, or at boot time by
the failure to find ACPI platform support.

So this extension simply allows SFI-platforms to
re-use existing standard table formats that happen to
be defined to live in ACPI envelopes.

Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/sfi/Makefile     |    2 +
 drivers/sfi/sfi_acpi.c   |  151 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/sfi/sfi_core.c   |    2 +
 include/linux/sfi_acpi.h |   60 ++++++++++++++++++
 4 files changed, 215 insertions(+), 0 deletions(-)
 create mode 100644 drivers/sfi/sfi_acpi.c
 create mode 100644 include/linux/sfi_acpi.h

diff --git a/drivers/sfi/Makefile b/drivers/sfi/Makefile
index f176470..2343732 100644
--- a/drivers/sfi/Makefile
+++ b/drivers/sfi/Makefile
@@ -1 +1,3 @@
+obj-y	+= sfi_acpi.o
 obj-y	+= sfi_core.o
+
diff --git a/drivers/sfi/sfi_acpi.c b/drivers/sfi/sfi_acpi.c
new file mode 100644
index 0000000..0cc2332
--- /dev/null
+++ b/drivers/sfi/sfi_acpi.c
@@ -0,0 +1,151 @@
+/* sfi_acpi.c Simple Firmware Interface - ACPI extensions */
+
+/*
+ * 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/kernel.h>
+#include <acpi/acpi.h>
+#include <linux/sfi.h>
+
+#include "sfi_core.h"
+
+/*
+ * SFI can access ACPI-defined tables via an optional ACPI XSDT.
+ *
+ * This allows re-use, and avoids re-definition, of standard tables.
+ * For example, the "MCFG" table is defined by PCI, reserved by ACPI,
+ * and is expected to be present many SFI-only systems.
+ */
+
+static struct acpi_table_xsdt *xsdt_va;
+
+#define XSDT_GET_NUM_ENTRIES(ptable, entry_type) \
+	((ptable->header.length - sizeof(struct acpi_table_header)) / \
+	(sizeof(entry_type)))
+
+/*
+ * sfi_acpi_parse_xsdt()
+ *
+ * Parse the ACPI XSDT for later access by sfi_acpi_table_parse().
+ */
+static int __init sfi_acpi_parse_xsdt(struct sfi_table_header *table)
+{
+	int tbl_cnt, i;
+	xsdt_va = (struct acpi_table_xsdt *)table;
+
+	tbl_cnt = XSDT_GET_NUM_ENTRIES(xsdt_va, u64);
+
+	for (i = 0; i < tbl_cnt; i++) {
+		if (sfi_check_table(xsdt_va->table_offset_entry[i])) {
+			disable_sfi();
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
+int __init sfi_acpi_init(void)
+{
+	sfi_table_parse(SFI_SIG_XSDT, NULL, NULL, 0, sfi_acpi_parse_xsdt);
+	return 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;
+
+		if (oem_table_id && strncmp(th->oem_table_id, oem_table_id,
+						SFI_OEM_TABLE_ID_SIZE))
+			goto loop_continue;
+
+		return th;      /* success */
+loop_continue:
+		sfi_unmap_table((struct sfi_table_header *)th);
+	}
+
+	return NULL;
+}
+
+static void sfi_acpi_put_table(struct acpi_table_header *table)
+{
+	sfi_put_table((struct sfi_table_header *)table);
+}
+
+/*
+ * sfi_acpi_table_parse()
+ *
+ * find specified table in XSDT, run handler on it and return its return value
+ */
+int sfi_acpi_table_parse(char *signature, char *oem_id, char *oem_table_id,
+		 unsigned int flags, int(*handler)(struct acpi_table_header *))
+{
+	int ret = 0;
+	struct acpi_table_header *table = NULL;
+
+	if (sfi_disabled)
+		return -1;
+
+	table = sfi_acpi_get_table(signature, oem_id, oem_table_id, flags);
+	if (!table)
+		return -EINVAL;
+
+	ret = handler(table);
+	sfi_acpi_put_table(table);
+	return ret;
+}
diff --git a/drivers/sfi/sfi_core.c b/drivers/sfi/sfi_core.c
index 8939215..f411a02 100644
--- a/drivers/sfi/sfi_core.c
+++ b/drivers/sfi/sfi_core.c
@@ -369,6 +369,8 @@ void __init sfi_init_late(void)
 	/* use ioremap now after it is ready */
 	sfi_use_ioremap = 1;
 	syst_va = sfi_map_memory(syst_pa, length);
+
+	sfi_acpi_init();
 }
 
 static int __init sfi_parse_cmdline(char *arg)
diff --git a/include/linux/sfi_acpi.h b/include/linux/sfi_acpi.h
new file mode 100644
index 0000000..d19a00a
--- /dev/null
+++ b/include/linux/sfi_acpi.h
@@ -0,0 +1,60 @@
+/* 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_ACPI_H
+#define _LINUX_SFI_ACPI_H
+
+#ifdef	CONFIG_SFI
+#include <acpi/acpi.h>		/* struct acpi_table_header */
+
+int sfi_acpi_table_parse(char *signature, char *oem_id, char* oem_table_id,
+	uint flag, int (*handler)(struct acpi_table_header *));
+
+#else /* !CONFIG_SFI */
+
+static inline int sfi_acpi_table_parse(char *signature, char *oem_id,
+	char *oem_table_id, unsigned int flags,
+	int (*handler)(struct acpi_table_header *))
+{
+	return -1;
+}
+
+#endif	/* CONFIG_SFI */
+
+#endif	/*_LINUX_SFI_ACPI_H*/
-- 
1.6.0.6

[PATCH 10/12] ACPI: check acpi_disabled in acpi_table_parse()

[Len Brown]

From: Len Brown <len.brown@intel.com>

Allow consumers of the acpi_table_parse() API
to gracefully handle the acpi_disabled=1 case
without checking the flag themselves.

Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/tables.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 646d39c..921746f 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -277,6 +277,9 @@ int __init acpi_table_parse(char *id, acpi_table_handler handler)
 	struct acpi_table_header *table = NULL;
 	acpi_size tbl_size;
 
+	if (acpi_disabled)
+		return 1;
+
 	if (!handler)
 		return -EINVAL;
 
-- 
1.6.0.6

[PATCH 11/12] SFI, PCI: Hook MMCONFIG

[Len Brown]

From: Feng Tang <feng.tang@intel.com>

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

Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 arch/x86/Kconfig               |    2 +-
 arch/x86/pci/mmconfig-shared.c |    5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 8e864c0..8e20a0e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1880,7 +1880,7 @@ config PCI_DIRECT
 
 config PCI_MMCONFIG
 	def_bool y
-	depends on X86_32 && PCI && ACPI && (PCI_GOMMCONFIG || PCI_GOANY)
+	depends on X86_32 && PCI && (ACPI || SFI) && (PCI_GOMMCONFIG || PCI_GOANY)
 
 config PCI_OLPC
 	def_bool y
diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index 712443e..373780b 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -13,6 +13,7 @@
 #include <linux/pci.h>
 #include <linux/init.h>
 #include <linux/acpi.h>
+#include <linux/sfi_acpi.h>
 #include <linux/bitmap.h>
 #include <linux/sort.h>
 #include <asm/e820.h>
@@ -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);
 
 	pci_mmcfg_reject_broken(early);
 
-- 
1.6.0.6

[PATCH 12/12] SFI: add boot-time initialization hooks

[Len Brown]

From: Feng Tang <feng.tang@intel.com>

There are two SFI boot-time hooks:

sfi_init() maps the SYST using early_ioremap() and validates all the tables.

sfi_init_late() re-maps SYST with ioremap(), and initializes
the ACPI extension, if present.

Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 arch/x86/kernel/setup.c |    3 +++
 init/main.c             |    2 ++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index de2cab1..94ad296 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -27,6 +27,7 @@
 #include <linux/screen_info.h>
 #include <linux/ioport.h>
 #include <linux/acpi.h>
+#include <linux/sfi.h>
 #include <linux/apm_bios.h>
 #include <linux/initrd.h>
 #include <linux/bootmem.h>
@@ -977,6 +978,8 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	acpi_boot_init();
 
+	sfi_init();
+
 #if defined(CONFIG_X86_MPPARSE) || defined(CONFIG_X86_VISWS)
 	/*
 	 * get boot-time SMP configuration:
diff --git a/init/main.c b/init/main.c
index 2c5ade7..d32a1ff 100644
--- a/init/main.c
+++ b/init/main.c
@@ -68,6 +68,7 @@
 #include <linux/async.h>
 #include <linux/kmemcheck.h>
 #include <linux/kmemtrace.h>
+#include <linux/sfi.h>
 #include <trace/boot.h>
 
 #include <asm/io.h>
@@ -712,6 +713,7 @@ asmlinkage void __init start_kernel(void)
 	check_bugs();
 
 	acpi_early_init(); /* before LAPIC and SMP init */
+	sfi_init_late();
 
 	ftrace_init();
 
-- 
1.6.0.6

Re: [PATCH 08/12] SFI, x86: hook e820() for memory map initialization

[H. Peter Anvin]

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


Len Brown wrote:
> From: Feng Tang <feng.tang@intel.com>
> 
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
>  arch/x86/kernel/e820.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index c4ca89d..e399d0e 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -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) {

Re: [PATCH 08/12] SFI, x86: hook e820() for memory map initialization

[Feng Tang]

On Thu, 9 Jul 2009 05:37:08 +0800
"H. Peter Anvin" <hpa@zytor.com> wrote:

> 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

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

> 
> 
> Len Brown wrote:
> > From: Feng Tang <feng.tang@intel.com>
> > 
> > Signed-off-by: Feng Tang <feng.tang@intel.com>
> > Signed-off-by: Len Brown <len.brown@intel.com>
> > ---
> >  arch/x86/kernel/e820.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> > index c4ca89d..e399d0e 100644
> > --- a/arch/x86/kernel/e820.c
> > +++ b/arch/x86/kernel/e820.c
> > @@ -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) {
> 

Re: [PATCH 08/12] SFI, x86: hook e820() for memory map initialization

[H. Peter Anvin]

Feng Tang wrote:
> 
> 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
> 

What I'm concerned about is that we will want to be able to use the
range allocator, which relies on the e820 memory map, extremely early
during boot.  In the EFI case, we allow (*optionally*) to fill in
additional information from the EFI map after initial boot, but we still
require memory ranges in the initial map (they can, however, be a subset
of the available memory.)

I'm really concerned about saying "well, it works now" and tying
ourselves into an interface that we cannot support in the long term
(read: we'll be stuck with it.)

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH v2 for 2.6.32] SFI - Simple Firmware Interface support

[Ingo Molnar]

* Len Brown <lenb@kernel.org> wrote:

> 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 ]

>       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:

>       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. )

Thanks,

	Ingo

Re: [PATCH 02/12] SFI, x86: add CONFIG_SFI

[Ingo Molnar]

* Len Brown <lenb@kernel.org> wrote:

> +++ b/drivers/sfi/Kconfig
> @@ -0,0 +1,16 @@
> +#
> +# SFI Configuration
> +#
> +
> +menuconfig SFI
> +	bool "SFI (Simple Firmware Interface) Support"
> +	depends on X86

Instead of 'depends on X86' it's cleaner to add a HAVE_ARCH_SFI 
Kconfig variable to arch/x86/Kconfig, and turn the above into:

	depends on HAVE_ARCH_SFI

That will make it easier to enable it on other architectures such as 
IA64, should the need arise - and it's cleaner in any case.

> +	default n

Kconfig uses 'default n' by default, so this line can be left out.

> +	---help---
> +	The Simple Firmware Interface (SFI) provides a lightweight method
> +	for platform firmware to pass information to the Operating System
> +	via static tables in memory.
> +
> +	For more information, see http://simplefirmware.org
> +
> +	Say 'Y' here to enable the kernel to boot on SFI-only platforms.

Nitpick: s/Operating System/Linux kernel

(and even if we want to formulate it in a general way, it should be 
'operating system')

	Ingo

Re: [PATCH 11/12] SFI, PCI: Hook MMCONFIG

[Ingo Molnar]

* Len Brown <lenb@kernel.org> wrote:

> From: Feng Tang <feng.tang@intel.com>
> 
> First check ACPI, and if that fails, ask SFI to find the MCFG.
> 
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
>  arch/x86/Kconfig               |    2 +-
>  arch/x86/pci/mmconfig-shared.c |    5 ++++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 8e864c0..8e20a0e 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1880,7 +1880,7 @@ config PCI_DIRECT
>  
>  config PCI_MMCONFIG
>  	def_bool y
> -	depends on X86_32 && PCI && ACPI && (PCI_GOMMCONFIG || PCI_GOANY)
> +	depends on X86_32 && PCI && (ACPI || SFI) && (PCI_GOMMCONFIG || PCI_GOANY)

Please also update the 64-bit side (there's no reason to do this on 
32-bit only - even though your sample hw is probably 32-bit only) - 
and while at it, also unify the currently separate 32-bit and 64-bit 
PCI_MMCONFIG definitions.

> @@ -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

Re: [PATCH 09/12] SFI: Enable SFI to parse ACPI tables

[Ingo Molnar]

* Len Brown <lenb@kernel.org> wrote:

> From: Feng Tang <feng.tang@intel.com>

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

> +static struct acpi_table_xsdt *xsdt_va;

Should be __read_mostly.

> +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;
> +
> +		if (oem_table_id && strncmp(th->oem_table_id, oem_table_id,
> +						SFI_OEM_TABLE_ID_SIZE))
> +			goto loop_continue;
> +
> +		return th;      /* success */
> +loop_continue:
> +		sfi_unmap_table((struct sfi_table_header *)th);
> +	}
> +
> +	return NULL;

That loop looks weird and non-standard.

The body should be factored out into a check_table(th) helper 
function, and the loop can thus be written as a very straightforward 
one:

	th = NULL;

	for (i = 0; i < tbl_cnt; i++) {
		pa = xsdt_va->table_offset_entry[i];

		th = check_table(pa, signature, oem_id, oem_table_id))
		if (IS_ERR(th))
			return NULL;
		if (th)
			return th;
	}

	return th;

where check_table() returns PTR_ERR(-EINVAL) on mapping failure, 
NULL on mismatch and a table header on match.

Also, a general comment for the whole series: the various type casts 
between ACPI and SFI table headers look ugly and are a bit fragile. 
It would be better to define explicit type conversion helper inlines 
between ACPI an SFI types - and those would thus be type-safe. 
(right now it's easy to typo a variable name and pass in something 
that is neither an ACPI nor an SFI header.)

So we should have two primitives:

	acpi_to_sfi_table(table)
	sfi_to_acpi_table(table)

> +static void sfi_acpi_put_table(struct acpi_table_header *table)
> +{
> +	sfi_put_table((struct sfi_table_header *)table);
> +}

the above could thus be written as:

static void sfi_acpi_put_table(struct acpi_table_header *table)
{
	sfi_put_table(acpi_to_sfi_table(table));
}

> +/*
> + * sfi_acpi_table_parse()
> + *
> + * find specified table in XSDT, run handler on it and return its return value
> + */
> +int sfi_acpi_table_parse(char *signature, char *oem_id, char *oem_table_id,
> +		 unsigned int flags, int(*handler)(struct acpi_table_header *))
> +{

i'd suggest to define a helper structure for the 'table key' fields 
above. They get passed around repetitively. Also, a 
acpi_handler_fn_t helper would be useful as well. That way the above 
prototype could be written as:

int sfi_acpi_table_parse(struct acpi_table_key key, unsigned int flags,
                         acpi_handler_fn_t *handler);

which is a lot more readable and 'key' could be passed straight to 
the sfi parser.

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

> +#ifdef	CONFIG_SFI

Small nit: we dont generally put tabs into ifdefs, unless strongly 
warranted (which this one does not seem to be).

	Ingo

Re: [PATCH 07/12] SFI: add x86 support

[Ingo Molnar]

* Len Brown <lenb@kernel.org> wrote:

> From: Feng Tang <feng.tang@intel.com>
> 
> 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 <feng.tang@intel.com>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
>  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
> 
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 6c327b8..e430123 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -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
> diff --git a/arch/x86/kernel/sfi.c b/arch/x86/kernel/sfi.c
> new file mode 100644
> index 0000000..a96ea0f
> --- /dev/null
> +++ b/arch/x86/kernel/sfi.c
> @@ -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;
> +	char *pchar = (char *)SFI_SYST_SEARCH_BEGIN;
> +
> +	/* SFI spec defines the SYST starts at a 16-byte boundary */
> +	for (i = 0; SFI_SYST_SEARCH_BEGIN + i < SFI_SYST_SEARCH_END; i += 16) {
> +		if (!strncmp(SFI_SIG_SYST, pchar, SFI_SIGNATURE_SIZE))
> +			return (struct sfi_table_simple *)
> +					(SFI_SYST_SEARCH_BEGIN + i);
> +
> +		pchar += 16;
> +	}

Hm, this loop seems a bit confused about how it wants to iterate 
over a range of addresses.

SFI_SYST_SEARCH_BEGIN and END should probably have a void * basic 
type, which would allow a _much_ cleaner loop like this:

static __init struct sfi_table_simple *sfi_early_find_syst(void)
{
	/* SFI spec defines the SYST starts at a 16-byte boundary */
	for (p = SFI_SYST_SEARCH_BEGIN; p < SFI_SYST_SEARCH_END, p += 16) {
		if (!strncmp(SFI_SIG_SYST, p, SFI_SIGNATURE_SIZE))
			return p;
	}
	return NULL;
}

And this is a general observation for the other patches as well: if 
you anywhere do a C type cast please double check whether you really 
need to do it. In 90% of the cases it's a sign of some badness, in 
99% of the cases it's a source of fragility, and in 99.9% of the 
cases it can be avoided by restructuring the code.

I saw a handful of other cases where it's really avoidable.

> +/*
> + * 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)
> +{
> +	struct sfi_table_simple *syst, *mmapt;
> +	struct sfi_mem_entry *mentry;
> +	unsigned long long start, end, size;
> +	int i, num, type, tbl_cnt;
> +	u64 *pentry;
> +
> +	if (sfi_disabled)
> +		return -1;
> +
> +	/* first search the syst table */

Nit: s/syst/SYST in the comment text - to stay in line with how we 
refer to these firmware tables.

> +	syst = sfi_early_find_syst();
> +	if (!syst)
> +		return -1;
> +
> +	tbl_cnt = (syst->header.length - sizeof(struct sfi_table_header)) /
> +			sizeof(u64);

We have a helper for this, SFI_GET_NUM_ENTRIES(), please use it as 
SFI_GET_NUM_ENTRIES(syst, u64) or so.

> +	pentry = syst->pentry;
> +
> +	/* walk through the syst to search the mmap table */
> +	mmapt = NULL;
> +	for (i = 0; i < tbl_cnt; i++) {
> +		if (!strncmp(SFI_SIG_MMAP, (char *)(u32)*pentry, 4)) {
> +			mmapt = (struct sfi_table_simple *)(u32)*pentry;
> +			break;
> +		}
> +		pentry++;
> +	}

This loop does a double type cast - which already shows that we do 
something weird here. Firstly, this will trigger a warning on 64-bit 
builds. Secondly, it's cast twice - to different target types.

Thirdly, the double cast itself discards the high 32 bits of the 
value. This is pretty bad as it allows the firmware to pass in some 
bogus value there and we could build a dependency on it.

> +	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;
> +	for (i = 0; i < num; i++) {
> +		start = mentry->phy_start;
> +		size = mentry->pages << PAGE_SHIFT;
> +		end = start + size;
> +
> +		if (start > end)
> +			return -1;
> +
> +		pr_debug("start = 0x%08x end = 0x%08x type = %d\n",
> +			(u32)start, (u32)end, mentry->type);

DEBUG is not defined at the top of the file so this is dead code.

> +		/* 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++;
> +	}
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_X86_LOCAL_APIC
> +void __init mp_sfi_register_lapic_address(unsigned long address)
> +{
> +	mp_lapic_addr = address;
> +	set_fixmap_nocache(FIX_APIC_BASE, mp_lapic_addr);

Please put newlines after local variable definition blocks.

> +
> +	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++) {
> +		mp_sfi_register_lapic(pentry->apicid);
> +		pentry++;
> +	}
> +
> +	smp_found_config = 1;
> +	return 0;
> +}
> +#endif /* CONFIG_X86_LOCAL_APIC */
> +
> +#ifdef	CONFIG_X86_IO_APIC

(stray tab.)

> +void __init mp_sfi_register_ioapic(u8 id, u32 paddr)
> +{
> +	int idx = 0;
> +	int tmpid;
> +	static u32 gsi_base;

I think this static variable should be outside of this function - so 
that it's apparent at a glance that it persists.

> +
> +	if (nr_ioapics >= MAX_IO_APICS) {
> +		pr_err("ERROR: Max # of I/O APICs (%d) exceeded "
> +			"(found %d)\n", MAX_IO_APICS, nr_ioapics);
> +		panic("Recompile kernel with bigger MAX_IO_APICS!\n");

We should fail more gracefully here i think - print a warning and 
continue. The system might not boot ... or it might, with a few 
devices not operational.

> +	}
> +	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);
> +	tmpid = uniq_ioapic_id(id);
> +	if (tmpid == -1)
> +		return;
> +
> +	mp_ioapics[idx].apicid = tmpid;
> +#ifdef CONFIG_X86_32
> +	mp_ioapics[idx].apicver = io_apic_get_version(idx);
> +#else
> +	mp_ioapics[idx].apicver = 0;
> +#endif

Why is this define needed? io_apic_get_version() exists on 64-bit as 
well.

> +
> +	/*
> +	 * 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 */
> +
> +/*
> + * sfi_platform_init(): register lapics & io-apics
> + */
> +int __init sfi_platform_init(void)
> +{
> +#ifdef CONFIG_X86_LOCAL_APIC
> +	mp_sfi_register_lapic_address(sfi_lapic_addr);
> +	sfi_table_parse(SFI_SIG_CPUS, NULL, NULL, 0, sfi_parse_cpus);
> +#endif
> +#ifdef CONFIG_X86_IO_APIC
> +	sfi_table_parse(SFI_SIG_APIC, NULL, NULL, 0, sfi_parse_ioapic);

General comment: sfi_table_parse() has these pointless NULL, NULL, 0 
portions 90% of the uses i've seen. They are the 'any match' key for 
table parsing.

Combined with the struct table_key suggestion i made in the other 
reply, we could have this define:

#define SFI_ANY_KEY { .signature = NULL, .oem_id = NULL, .oem_table_id = 0 }

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

Re: [PATCH 07/12] SFI: add x86 support

[Feng Tang]

On Fri, 10 Jul 2009 14:37:26 +0800
Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Len Brown <lenb@kernel.org> wrote:
> 
> > From: Feng Tang <feng.tang@intel.com>
> >
> > 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 <feng.tang@intel.com>
> > Signed-off-by: Len Brown <len.brown@intel.com>
> > ---
> >  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
> >
> > diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> > index 6c327b8..e430123 100644
> > --- a/arch/x86/kernel/Makefile
> > +++ b/arch/x86/kernel/Makefile
> > @@ -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

Thanks again for the great comments to this v2 series, will address them soon.

Thanks,
Feng

Re: [PATCH 04/12] SFI: create include/linux/sfi.h

[Ingo Molnar]

* Len Brown <lenb@kernel.org> wrote:

> From: Feng Tang <feng.tang@intel.com>
> 
> 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 <feng.tang@intel.com>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
>  include/linux/sfi.h |  198 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 198 insertions(+), 0 deletions(-)
>  create mode 100644 include/linux/sfi.h
> 
> diff --git a/include/linux/sfi.h b/include/linux/sfi.h
> new file mode 100644
> index 0000000..f00f4da
> --- /dev/null
> +++ b/include/linux/sfi.h
> @@ -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"
> +#define SFI_SIG_CPUS		"CPUS"
> +#define SFI_SIG_MTMR		"MTMR"
> +#define SFI_SIG_MRTC		"MRTC"
> +#define SFI_SIG_MMAP		"MMAP"
> +#define SFI_SIG_APIC		"APIC"
> +#define SFI_SIG_XSDT		"XSDT"
> +#define SFI_SIG_WAKE		"WAKE"
> +#define SFI_SIG_SPIB		"SPIB"
> +#define SFI_SIG_I2CB		"I2CB"
> +#define SFI_SIG_GPEM		"GPEM"

Sidenote: these strings, if used in different places, might be 
instantiated in a duplicated way, bloating the kernel a tiny bit. It 
might be better to do an intermediate const array and return 
elements of them.

> +
> +#define SFI_ACPI_TABLE		(1 << 0)
> +#define SFI_NORMAL_TABLE	(1 << 1)
> +
> +#define SFI_SIGNATURE_SIZE	4
> +#define SFI_OEM_ID_SIZE		6
> +#define SFI_OEM_TABLE_ID_SIZE	8
> +
> +#define SFI_SYST_SEARCH_BEGIN		0x000E0000
> +#define SFI_SYST_SEARCH_END		0x000FFFFF

(see my suggestion about the proper type of these in my other mail)

> +
> +#define SFI_GET_NUM_ENTRIES(ptable, entry_type) \
> +	((ptable->header.length - sizeof(struct sfi_table_header)) / \
> +	(sizeof(entry_type)))
> +/*
> + * Table structures must be byte-packed to match the SFI specification,
> + * as they are provided by the BIOS.
> + */
> +struct sfi_table_header {
> +	char	signature[SFI_SIGNATURE_SIZE];
> +	u32	length;
> +	u8	revision;
> +	u8	checksum;

These could be abbreviated with common terms:

 s/signature/sig
 s/length/len
 s/revision/rev
 s/checksum/csum

> +	char	oem_id[SFI_OEM_ID_SIZE];
> +	char	oem_table_id[SFI_OEM_TABLE_ID_SIZE];
> +} __packed;
> +
> +struct sfi_table_simple {
> +	struct sfi_table_header		header;
> +	u64				pentry[1];
> +} __packed;
> +
> +/* comply with UEFI spec 2.1 */
> +struct sfi_mem_entry {
> +	u32	type;
> +	u64	phy_start;

s/phy_start/phys_start

> +	u64	vir_start;

s/vir_start/virt_start

> +	u64	pages;
> +	u64	attrib;
> +} __packed;
> +
> +struct sfi_cpu_table_entry {
> +	u32	apicid;

s/apicid/apic_id

(i realize that apicid is still the commonly used one in arch/x86, 
but that's not a reason to continue that bad practice with new 
code.)

> +} __packed;
> +
> +struct sfi_cstate_table_entry {
> +	u32	hint;		/* MWAIT hint */
> +	u32	latency;	/* latency in ms */
> +} __packed;
> +
> +struct sfi_apic_table_entry {
> +	u64	phy_addr;	/* phy base addr for APIC reg */

s/phy_addr/phys_addr

> +} __packed;
> +
> +struct sfi_freq_table_entry {
> +	u32	freq;

is this in Hz or in kHz? (should be reflected in the name: say 
freq_khz)

I suspect it's HZ here - but that's not unambiguous - often cpufreq 
settings are in khz.

> +	u32	latency;	/* transition latency in ms */
> +	u32	ctrl_val;	/* value to write to PERF_CTL */

PERF_CTL is an u64 MSR.

> +} __packed;
> +
> +struct sfi_wake_table_entry {
> +	u64 phy_addr;	/* pointer to where the wake vector locates */

s/phy_addr/phys_addr

> +} __packed;
> +
> +struct sfi_timer_table_entry {
> +	u64	phy_addr;	/* phy base addr for the timer */

s/phy_addr/phys_addr

> +	u32	freq;		/* in HZ */

So we limit frequency to ~4 GHz?

> +	u32	irq;
> +} __packed;
> +
> +struct sfi_rtc_table_entry {
> +	u64	phy_addr;	/* phy base addr for the RTC */

s/phy_addr/phys_addr

> +	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 {
> +	u16	host_num;
> +	u16	addr;		/* slave addr */
> +	u16	irq_info;
> +	char	name[16];
> +	u8	dev_info[10];
> +} __packed;
> +
> +struct sfi_gpe_table_entry {
> +	u16	logical_id;	/* logical id */
> +	u16	phy_id;		/* physical GPE id */

s/phy_id/phys_id

> +} __packed;
> +
> +
> +typedef int (*sfi_table_handler) (struct sfi_table_header *table);
> +
> +#ifdef	CONFIG_SFI

(stray tab.)

> +extern int __init sfi_init_memory_map(void);
> +extern void __init sfi_init(void);
> +extern int __init sfi_platform_init(void);
> +extern void __init sfi_init_late(void);
> +
> +int sfi_table_parse(char *signature, char *oem_id, char *oem_table_id,
> +			uint flag, sfi_table_handler handler);

some prototypes come with externs ... some without. Please use one 
variant.

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

no need for that return.

Thanks,

	Ingo

Re: [PATCH 06/12] ACPI, x86: remove ACPI dependency on some IO-APIC routines

[Ingo Molnar]

* Len Brown <lenb@kernel.org> wrote:

> From: Feng Tang <feng.tang@intel.com>
> 
> Both ACPI and SFI x86 systems will use these io_apic routines:
> 
> uniq_ioapic_id(u8 id);

While touching this code please also fix the misnomer:

 s/uniq_ioapic_id/unique_ioapic_id

Thanks,

	Ingo

Re: [PATCH 11/12] SFI, PCI: Hook MMCONFIG

[Feng Tang]

On Fri, 10 Jul 2009 13:52:29 +0800
Ingo Molnar <mingo@elte.hu> wrote:

> 
> > @@ -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.
>

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?

Thanks,
Feng 
 
> 	Ingo

Re: [PATCH 05/12] SFI: add core support

[Ingo Molnar]

* Len Brown <lenb@kernel.org> wrote:

> From: Feng Tang <feng.tang@intel.com>
> 
> 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 <feng.tang@intel.com>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
>  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
> 
> diff --git a/drivers/Makefile b/drivers/Makefile
> index bc4205d..ccfa259 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -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/
> diff --git a/drivers/sfi/Makefile b/drivers/sfi/Makefile
> new file mode 100644
> index 0000000..f176470
> --- /dev/null
> +++ b/drivers/sfi/Makefile
> @@ -0,0 +1 @@
> +obj-y	+= sfi_core.o
> diff --git a/drivers/sfi/sfi_core.c b/drivers/sfi/sfi_core.c
> new file mode 100644
> index 0000000..f9a9849
> --- /dev/null
> +++ b/drivers/sfi/sfi_core.c
> @@ -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>
> +
> +#include <asm/pgtable.h>
> +
> +#include "sfi_core.h"
> +
> +#define ON_SAME_PAGE(addr1, addr2) \
> +	(((unsigned long)(addr1) & PAGE_MASK) == \
> +	((unsigned long)(addr2) & PAGE_MASK))
> +#define TABLE_ON_PAGE(page, table, size) (ON_SAME_PAGE(page, table) && \
> +				ON_SAME_PAGE(page, table + size))

these two should be written in C, not CPP.

> +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.
> + */
> +struct sfi_table_header *sfi_map_table(u64 pa)
> +{
> +	struct sfi_table_header *th;
> +	u32 length;
> +
> +	if (!TABLE_ON_PAGE(syst_pa, pa, sizeof(struct sfi_table_header)))
> +		th = sfi_map_memory(pa, sizeof(struct sfi_table_header));
> +	else
> +		th = (void *)syst_va - (syst_pa - pa);

The '- (syst_pa - pa)' arithmetics is correct but rather unreadable 
- it should be something like:

		th = (void *)syst_va + (pa - syst_pa);

To show that we return an offset into the SYST table - instead of 
the double negation.

> +
> +	 /* If table fits on same page as its header, we are done */
> +	if (TABLE_ON_PAGE(th, th, th->length))
> +		return th;
> +
> +	/* entire table does not fit on same page as SYST */

Nit: please capitalize comments consistently, by starting them with 
a capital letter.

> +	length = th->length;
> +	if (!TABLE_ON_PAGE(syst_pa, pa, sizeof(struct sfi_table_header)))
> +		sfi_unmap_memory(th, sizeof(struct sfi_table_header));
> +
> +	return sfi_map_memory(pa, length);

tricky.

> +}
> +
> +/*
> + * 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,
> +						SFI_OEM_TABLE_ID_SIZE))
> +			goto loop_continue;
> +
> +		return th;	/* success */
> +loop_continue:
> +		sfi_unmap_table(th);
> +	}
> +
> +	return NULL;
> +}

This loop should be cleaned up in the same fashion as i suggested 
for sfi_acpi_get_table().

> +
> +void sfi_put_table(struct sfi_table_header *table)
> +{
> +	if (!ON_SAME_PAGE(((void *)table + table->length),
> +		(void *)syst_va + syst_va->header.length)
> +		&& !ON_SAME_PAGE(table, syst_va))
> +		sfi_unmap_memory(table, table->length);

syst_va should probably be a void * to begin with - that avoids the 
second cast.

I'm quite uneasy about the many open-coded ON_SAME_PAGE() 
conditions. That logic should perhaps be in sfi_map/unmap_memory() 
instead? (with an additional 'finegrained_len' argument to it or 
so.)

> +}
> +
> +/* 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;
> +
> +	table = sfi_get_table(signature, oem_id, oem_table_id, flags);
> +	if (!table)
> +		return -EINVAL;
> +
> +	ret = handler(table);
> +	sfi_put_table(table);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(sfi_table_parse);

the error conditions could be written cleaner and shorter as:

int sfi_table_parse(char *signature, char *oem_id, char *oem_table_id,
			unsigned int flags, sfi_table_handler handler)
{
	struct sfi_table_header *table = NULL;
	int ret = -EINVAL;

	if (sfi_disabled || !handler || !signature)
		goto err;

	table = sfi_get_table(signature, oem_id, oem_table_id, flags);
	if (!table)
		goto err;

	ret = handler(table);
	sfi_put_table(table);
err:
	return ret;
}

Thanks,

	Ingo

Re: [PATCH 11/12] SFI, PCI: Hook MMCONFIG

[Ingo Molnar]

* Feng Tang <feng.tang@intel.com> wrote:

> On Fri, 10 Jul 2009 13:52:29 +0800
> Ingo Molnar <mingo@elte.hu> wrote:
> 
> > 
> > > @@ -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.
> 
> 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.

They are of the same family and there's reuse in terms of table 
parsing code, etc. Do you have some nice name that covers both? I 
didnt find any good one beyond the x86_table_*() namespace.

> 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?

it all depends on the patches ... and the APIC enumeration code 
definitely needs cleanups so if you can do it that would be welcome.

	Ingo

Re: [PATCH v2 for 2.6.32] SFI - Simple Firmware Interface support

[Len Brown]

> 
> > 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 ]
> 
> >       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:
> 
> >       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. )

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

cheers,
-Len

Re: [PATCH v2 for 2.6.32] SFI - Simple Firmware Interface support

[Ingo Molnar]

* Len Brown <lenb@kernel.org> wrote:

> 
> > 
> > > 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 ]
> > 
> > >       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:
> > 
> > >       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. )
> 
> Ingo,
> While I really do appreciate your attention to detail,
> a little time off might be good for you:-)

No objections on my part ;-)

	Ingo

Re: [PATCH 03/12] SFI: document boot param "sfi=off"

[Bjorn Helgaas]

On Tuesday 07 July 2009 10:13:49 pm Len Brown wrote:
> From: Feng Tang <feng.tang@intel.com>
> 
> "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 <feng.tang@intel.com>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
>  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.

Bjorn

> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index d77fbd8..68337e6 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -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]

Re: [PATCH 03/12] SFI: document boot param "sfi=off"

[Len Brown]

> On Tuesday 07 July 2009 10:13:49 pm Len Brown wrote:
> > From: Feng Tang <feng.tang@intel.com>
> > 
> > "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 <feng.tang@intel.com>
> > Signed-off-by: Len Brown <len.brown@intel.com>
> > ---
> >  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.

True.
I was somewhat lazy when i generated this patch series
from as single patch, and split mainly in groups of files.

Re: sfi=off
I'm actually going to delete this one --
perhaps adding it later when/if it does something useful.

thanks,
-Len Brown, Intel Open Source Technology Center