* [PATCH 02/12] SFI, x86: add CONFIG_SFI
2009-07-08 4:13 ` [PATCH 01/12] SFI: Simple Firmware Interface - new MAINTAINERS entry Len Brown
@ 2009-07-08 4:13 ` Len Brown
2009-07-10 5:23 ` Ingo Molnar
2009-07-08 4:13 ` [PATCH 03/12] SFI: document boot param "sfi=off" Len Brown
` (9 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Len Brown @ 2009-07-08 4:13 UTC (permalink / raw)
To: x86, sfi-devel, linux-kernel, linux-acpi; +Cc: Feng Tang, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 02/12] SFI, x86: add CONFIG_SFI
2009-07-08 4:13 ` [PATCH 02/12] SFI, x86: add CONFIG_SFI Len Brown
@ 2009-07-10 5:23 ` Ingo Molnar
0 siblings, 0 replies; 31+ messages in thread
From: Ingo Molnar @ 2009-07-10 5:23 UTC (permalink / raw)
To: Len Brown; +Cc: x86, sfi-devel, linux-kernel, linux-acpi, Feng Tang, Len Brown
* 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 03/12] SFI: document boot param "sfi=off"
2009-07-08 4:13 ` [PATCH 01/12] SFI: Simple Firmware Interface - new MAINTAINERS entry Len Brown
2009-07-08 4:13 ` [PATCH 02/12] SFI, x86: add CONFIG_SFI Len Brown
@ 2009-07-08 4:13 ` Len Brown
2009-07-28 19:24 ` Bjorn Helgaas
2009-07-08 4:13 ` [PATCH 04/12] SFI: create include/linux/sfi.h Len Brown
` (8 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Len Brown @ 2009-07-08 4:13 UTC (permalink / raw)
To: x86, sfi-devel, linux-kernel, linux-acpi; +Cc: Feng Tang, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 03/12] SFI: document boot param "sfi=off"
2009-07-08 4:13 ` [PATCH 03/12] SFI: document boot param "sfi=off" Len Brown
@ 2009-07-28 19:24 ` Bjorn Helgaas
2009-07-28 19:52 ` Len Brown
0 siblings, 1 reply; 31+ messages in thread
From: Bjorn Helgaas @ 2009-07-28 19:24 UTC (permalink / raw)
To: Len Brown; +Cc: x86, sfi-devel, linux-kernel, linux-acpi, Feng Tang, 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.
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]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 03/12] SFI: document boot param "sfi=off"
2009-07-28 19:24 ` Bjorn Helgaas
@ 2009-07-28 19:52 ` Len Brown
0 siblings, 0 replies; 31+ messages in thread
From: Len Brown @ 2009-07-28 19:52 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: x86, sfi-devel, Linux Kernel Mailing List, linux-acpi, Feng Tang
> 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 04/12] SFI: create include/linux/sfi.h
2009-07-08 4:13 ` [PATCH 01/12] SFI: Simple Firmware Interface - new MAINTAINERS entry Len Brown
2009-07-08 4:13 ` [PATCH 02/12] SFI, x86: add CONFIG_SFI Len Brown
2009-07-08 4:13 ` [PATCH 03/12] SFI: document boot param "sfi=off" Len Brown
@ 2009-07-08 4:13 ` Len Brown
2009-07-10 6:48 ` Ingo Molnar
2009-07-08 4:13 ` [PATCH 05/12] SFI: add core support Len Brown
` (7 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Len Brown @ 2009-07-08 4:13 UTC (permalink / raw)
To: x86, sfi-devel, linux-kernel, linux-acpi; +Cc: Feng Tang, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 04/12] SFI: create include/linux/sfi.h
2009-07-08 4:13 ` [PATCH 04/12] SFI: create include/linux/sfi.h Len Brown
@ 2009-07-10 6:48 ` Ingo Molnar
0 siblings, 0 replies; 31+ messages in thread
From: Ingo Molnar @ 2009-07-10 6:48 UTC (permalink / raw)
To: Len Brown; +Cc: x86, sfi-devel, linux-kernel, linux-acpi, Feng Tang, Len Brown
* 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 05/12] SFI: add core support
2009-07-08 4:13 ` [PATCH 01/12] SFI: Simple Firmware Interface - new MAINTAINERS entry Len Brown
` (2 preceding siblings ...)
2009-07-08 4:13 ` [PATCH 04/12] SFI: create include/linux/sfi.h Len Brown
@ 2009-07-08 4:13 ` Len Brown
2009-07-10 7:40 ` Ingo Molnar
2009-07-08 4:13 ` [PATCH 06/12] ACPI, x86: remove ACPI dependency on some IO-APIC routines Len Brown
` (6 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Len Brown @ 2009-07-08 4:13 UTC (permalink / raw)
To: x86, sfi-devel, linux-kernel, linux-acpi; +Cc: Feng Tang, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 05/12] SFI: add core support
2009-07-08 4:13 ` [PATCH 05/12] SFI: add core support Len Brown
@ 2009-07-10 7:40 ` Ingo Molnar
0 siblings, 0 replies; 31+ messages in thread
From: Ingo Molnar @ 2009-07-10 7:40 UTC (permalink / raw)
To: Len Brown; +Cc: x86, sfi-devel, linux-kernel, linux-acpi, Feng Tang, Len Brown
* 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 06/12] ACPI, x86: remove ACPI dependency on some IO-APIC routines
2009-07-08 4:13 ` [PATCH 01/12] SFI: Simple Firmware Interface - new MAINTAINERS entry Len Brown
` (3 preceding siblings ...)
2009-07-08 4:13 ` [PATCH 05/12] SFI: add core support Len Brown
@ 2009-07-08 4:13 ` Len Brown
2009-07-10 6:51 ` Ingo Molnar
2009-07-08 4:13 ` [PATCH 07/12] SFI: add x86 support Len Brown
` (5 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Len Brown @ 2009-07-08 4:13 UTC (permalink / raw)
To: x86, sfi-devel, linux-kernel, linux-acpi; +Cc: Feng Tang, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 07/12] SFI: add x86 support
2009-07-08 4:13 ` [PATCH 01/12] SFI: Simple Firmware Interface - new MAINTAINERS entry Len Brown
` (4 preceding siblings ...)
2009-07-08 4:13 ` [PATCH 06/12] ACPI, x86: remove ACPI dependency on some IO-APIC routines Len Brown
@ 2009-07-08 4:13 ` Len Brown
2009-07-10 6:37 ` Ingo Molnar
2009-07-08 4:13 ` [PATCH 08/12] SFI, x86: hook e820() for memory map initialization Len Brown
` (4 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Len Brown @ 2009-07-08 4:13 UTC (permalink / raw)
To: x86, sfi-devel, linux-kernel, linux-acpi; +Cc: Feng Tang, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 07/12] SFI: add x86 support
2009-07-08 4:13 ` [PATCH 07/12] SFI: add x86 support Len Brown
@ 2009-07-10 6:37 ` Ingo Molnar
2009-07-10 6:48 ` Feng Tang
0 siblings, 1 reply; 31+ messages in thread
From: Ingo Molnar @ 2009-07-10 6:37 UTC (permalink / raw)
To: Len Brown; +Cc: x86, sfi-devel, linux-kernel, linux-acpi, Feng Tang, Len Brown
* 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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 07/12] SFI: add x86 support
2009-07-10 6:37 ` Ingo Molnar
@ 2009-07-10 6:48 ` Feng Tang
0 siblings, 0 replies; 31+ messages in thread
From: Feng Tang @ 2009-07-10 6:48 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Len Brown, x86, sfi-devel, linux-kernel, linux-acpi
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 08/12] SFI, x86: hook e820() for memory map initialization
2009-07-08 4:13 ` [PATCH 01/12] SFI: Simple Firmware Interface - new MAINTAINERS entry Len Brown
` (5 preceding siblings ...)
2009-07-08 4:13 ` [PATCH 07/12] SFI: add x86 support Len Brown
@ 2009-07-08 4:13 ` Len Brown
2009-07-08 21:37 ` H. Peter Anvin
2009-07-08 4:13 ` [PATCH 09/12] SFI: Enable SFI to parse ACPI tables Len Brown
` (3 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Len Brown @ 2009-07-08 4:13 UTC (permalink / raw)
To: x86, sfi-devel, linux-kernel, linux-acpi; +Cc: Feng Tang, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 08/12] SFI, x86: hook e820() for memory map initialization
2009-07-08 4:13 ` [PATCH 08/12] SFI, x86: hook e820() for memory map initialization Len Brown
@ 2009-07-08 21:37 ` H. Peter Anvin
2009-07-09 1:11 ` Feng Tang
0 siblings, 1 reply; 31+ messages in thread
From: H. Peter Anvin @ 2009-07-08 21:37 UTC (permalink / raw)
To: Len Brown; +Cc: x86, sfi-devel, linux-kernel, linux-acpi, Feng Tang, Len Brown
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) {
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 08/12] SFI, x86: hook e820() for memory map initialization
2009-07-08 21:37 ` H. Peter Anvin
@ 2009-07-09 1:11 ` Feng Tang
2009-07-09 3:57 ` H. Peter Anvin
0 siblings, 1 reply; 31+ messages in thread
From: Feng Tang @ 2009-07-09 1:11 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Len Brown, x86, sfi-devel, linux-kernel, linux-acpi, Brown, Len
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) {
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 08/12] SFI, x86: hook e820() for memory map initialization
2009-07-09 1:11 ` Feng Tang
@ 2009-07-09 3:57 ` H. Peter Anvin
0 siblings, 0 replies; 31+ messages in thread
From: H. Peter Anvin @ 2009-07-09 3:57 UTC (permalink / raw)
To: Feng Tang; +Cc: Len Brown, x86, sfi-devel, linux-kernel, linux-acpi, Brown, Len
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 09/12] SFI: Enable SFI to parse ACPI tables
2009-07-08 4:13 ` [PATCH 01/12] SFI: Simple Firmware Interface - new MAINTAINERS entry Len Brown
` (6 preceding siblings ...)
2009-07-08 4:13 ` [PATCH 08/12] SFI, x86: hook e820() for memory map initialization Len Brown
@ 2009-07-08 4:13 ` Len Brown
2009-07-10 6:10 ` Ingo Molnar
2009-07-08 4:13 ` [PATCH 10/12] ACPI: check acpi_disabled in acpi_table_parse() Len Brown
` (2 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Len Brown @ 2009-07-08 4:13 UTC (permalink / raw)
To: x86, sfi-devel, linux-kernel, linux-acpi; +Cc: Feng Tang, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 09/12] SFI: Enable SFI to parse ACPI tables
2009-07-08 4:13 ` [PATCH 09/12] SFI: Enable SFI to parse ACPI tables Len Brown
@ 2009-07-10 6:10 ` Ingo Molnar
0 siblings, 0 replies; 31+ messages in thread
From: Ingo Molnar @ 2009-07-10 6:10 UTC (permalink / raw)
To: Len Brown; +Cc: x86, sfi-devel, linux-kernel, linux-acpi, Feng Tang, Len Brown
* 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 10/12] ACPI: check acpi_disabled in acpi_table_parse()
2009-07-08 4:13 ` [PATCH 01/12] SFI: Simple Firmware Interface - new MAINTAINERS entry Len Brown
` (7 preceding siblings ...)
2009-07-08 4:13 ` [PATCH 09/12] SFI: Enable SFI to parse ACPI tables Len Brown
@ 2009-07-08 4:13 ` Len Brown
2009-07-08 4:13 ` [PATCH 11/12] SFI, PCI: Hook MMCONFIG Len Brown
2009-07-08 4:13 ` [PATCH 12/12] SFI: add boot-time initialization hooks Len Brown
10 siblings, 0 replies; 31+ messages in thread
From: Len Brown @ 2009-07-08 4:13 UTC (permalink / raw)
To: x86, sfi-devel, linux-kernel, linux-acpi; +Cc: Len Brown, Feng Tang
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 11/12] SFI, PCI: Hook MMCONFIG
2009-07-08 4:13 ` [PATCH 01/12] SFI: Simple Firmware Interface - new MAINTAINERS entry Len Brown
` (8 preceding siblings ...)
2009-07-08 4:13 ` [PATCH 10/12] ACPI: check acpi_disabled in acpi_table_parse() Len Brown
@ 2009-07-08 4:13 ` Len Brown
2009-07-10 5:52 ` Ingo Molnar
2009-07-08 4:13 ` [PATCH 12/12] SFI: add boot-time initialization hooks Len Brown
10 siblings, 1 reply; 31+ messages in thread
From: Len Brown @ 2009-07-08 4:13 UTC (permalink / raw)
To: x86, sfi-devel, linux-kernel, linux-acpi
Cc: Feng Tang, Jesse Barnes, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 11/12] SFI, PCI: Hook MMCONFIG
2009-07-08 4:13 ` [PATCH 11/12] SFI, PCI: Hook MMCONFIG Len Brown
@ 2009-07-10 5:52 ` Ingo Molnar
2009-07-10 7:17 ` Feng Tang
0 siblings, 1 reply; 31+ messages in thread
From: Ingo Molnar @ 2009-07-10 5:52 UTC (permalink / raw)
To: Len Brown
Cc: x86, sfi-devel, linux-kernel, linux-acpi, Feng Tang,
Jesse Barnes, Len Brown
* 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 11/12] SFI, PCI: Hook MMCONFIG
2009-07-10 5:52 ` Ingo Molnar
@ 2009-07-10 7:17 ` Feng Tang
2009-07-10 11:14 ` Ingo Molnar
0 siblings, 1 reply; 31+ messages in thread
From: Feng Tang @ 2009-07-10 7:17 UTC (permalink / raw)
To: Ingo Molnar
Cc: Len Brown, x86, sfi-devel, linux-kernel, linux-acpi, Jesse Barnes
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 11/12] SFI, PCI: Hook MMCONFIG
2009-07-10 7:17 ` Feng Tang
@ 2009-07-10 11:14 ` Ingo Molnar
0 siblings, 0 replies; 31+ messages in thread
From: Ingo Molnar @ 2009-07-10 11:14 UTC (permalink / raw)
To: Feng Tang
Cc: Len Brown, x86, sfi-devel, linux-kernel, linux-acpi, Jesse Barnes
* 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 12/12] SFI: add boot-time initialization hooks
2009-07-08 4:13 ` [PATCH 01/12] SFI: Simple Firmware Interface - new MAINTAINERS entry Len Brown
` (9 preceding siblings ...)
2009-07-08 4:13 ` [PATCH 11/12] SFI, PCI: Hook MMCONFIG Len Brown
@ 2009-07-08 4:13 ` Len Brown
10 siblings, 0 replies; 31+ messages in thread
From: Len Brown @ 2009-07-08 4:13 UTC (permalink / raw)
To: x86, sfi-devel, linux-kernel, linux-acpi; +Cc: Feng Tang, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread