From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bin Meng Date: Tue, 28 Apr 2015 14:51:05 +0800 Subject: [U-Boot] [PATCH 07/20] x86: Add support for the Simple Firmware Interface (SFI) In-Reply-To: <1430174911-27538-8-git-send-email-sjg@chromium.org> References: <1430174911-27538-1-git-send-email-sjg@chromium.org> <1430174911-27538-8-git-send-email-sjg@chromium.org> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, On Tue, Apr 28, 2015 at 6:48 AM, Simon Glass wrote: > This provides a way of passing information to Linux without requiring the > full ACPI horror. Provide a rudimentary implementation sufficient to be > recognised and parsed by Linux. > > Signed-off-by: Simon Glass > --- It's great to see we are adding more configuration tables support in U-Boot. > arch/x86/Kconfig | 28 +++++++++ > arch/x86/lib/Makefile | 1 + > arch/x86/lib/sfi.c | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++ > arch/x86/lib/zimage.c | 7 +++ > include/linux/sfi.h | 139 ++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 346 insertions(+) > create mode 100644 arch/x86/lib/sfi.c > create mode 100644 include/linux/sfi.h > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index aaceaef..30a08ec 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -499,6 +499,34 @@ config PCIE_ECAM_BASE > assigned to PCI devices - i.e. the memory and prefetch regions, as > passed to pci_set_region(). > > +config SFI Can we rename this to "GENERATE_SFI_TABLE" under menu "System tables" in arch/x86/Kconfig? See more comments below. > + bool "SFI (Simple Firmware Interface) Support" > + ---help--- I think just 'help', like other options. > + The Simple Firmware Interface (SFI) provides a lightweight method > + for platform firmware to pass information to the operating system > + via static tables in memory. Kernel SFI support is required to > + boot on SFI-only platforms. If you have ACPI tables then these are > + used instead. The indention is wrong for this paragraph. > + For more information, see http://simplefirmware.org > + > + Say 'Y' here to enable the kernel to boot properly on SFI-only > + platforms. I guess this is from Linux kernel, so could be removed for U-Boot? > +config SFI_BASE We can just drop this config option, and let U-Boot calculate its address in write_tables(). See more comments below. > + hex "SFI base address (0xe0000 to 0xff000)" > + default 0xe0000 > + depends on SFI > + help > + The OS searches addresses in the range 0xe00000 to 0xffff0 for the > + Simple Firmware Interface (SFI) header. Use this option to determine > + where the table will be placed. It must be a multiple of 16 bytes and > + the header part (which U-Boot places at the end) must not cross a 4KB > + boundary. A 4KB-aligned address is recommended for these reasons. > + > + U-Boot writes this table in sfi_write_tables() just before booting > + the OS. > + > config BOOTSTAGE > default y > > diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile > index 0178fe1..c55b9be 100644 > --- a/arch/x86/lib/Makefile > +++ b/arch/x86/lib/Makefile > @@ -26,6 +26,7 @@ obj-y += pirq_routing.o > obj-y += relocate.o > obj-y += physmem.o > obj-$(CONFIG_X86_RAMTEST) += ramtest.o > +obj-$(CONFIG_SFI) += sfi.o I think it is safe to say: obj-y += sfi.o > obj-y += string.o > obj-y += tables.o > obj-$(CONFIG_SYS_X86_TSC_TIMER) += tsc_timer.o > diff --git a/arch/x86/lib/sfi.c b/arch/x86/lib/sfi.c > new file mode 100644 > index 0000000..060651b > --- /dev/null > +++ b/arch/x86/lib/sfi.c > @@ -0,0 +1,171 @@ > +/* > + * Copyright (c) 2015 Google, Inc > + * Written by Simon Glass > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +/* > + * Intel Simple Firmware Interface (SFI) > + * > + * Yet another way to pass information to the Linux kernel. > + * > + * See https://simplefirmware.org/ for details > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +struct table_info { > + u32 base; > + int ptr; > + u32 entry_start; > + u64 table[16]; I assume 16 is SFI_TABLE_MAX_ENTRIES? > + int count; > +}; > + > +void *get_entry_start(struct table_info *tab) Should this function be static? > +{ > + if (tab->count == ARRAY_SIZE(tab->table)) Then we can just write: tab->count == SFI_TABLE_MAX_ENTRIES > + return NULL; > + tab->entry_start = tab->base + tab->ptr; > + tab->table[tab->count] = tab->entry_start; > + tab->entry_start += sizeof(struct sfi_table_header); > + > + return (void *)tab->entry_start; > +} > + > +static void finish_table(struct table_info *tab, const char *sig, void *entry) > +{ > + struct sfi_table_header *hdr; > + uint8_t *p; > + int sum; > + > + hdr = (struct sfi_table_header *)(tab->base + tab->ptr); > + strcpy(hdr->sig, sig); > + hdr->len = sizeof(*hdr) + ((ulong)entry - tab->entry_start); > + hdr->rev = 1; > + strncpy(hdr->oem_id, "U-Boot", SFI_OEM_ID_SIZE); > + strncpy(hdr->oem_table_id, "Table v1", SFI_OEM_TABLE_ID_SIZE); > + hdr->csum = 0; > + for (sum = 0, p = (uint8_t *)hdr; (void *)p < entry; p++) > + sum += *p; > + hdr->csum = 256 - (sum & 255); The above can be replaced to: hdr->csum = 0; hdr->csum = table_compute_checksum(hdr, hdr->len); > + tab->ptr += hdr->len; > + tab->ptr = ALIGN(tab->ptr, 16); > + tab->count++; > +} > + > +static int sfi_write_system_header(struct table_info *tab) > +{ > + u64 *entry = get_entry_start(tab); > + int i; > + > + if (!entry) > + return -ENOSPC; Need a blank line here. > + for (i = 0; i < tab->count; i++) > + *entry++ = tab->table[i]; > + finish_table(tab, SFI_SIG_SYST, entry); > + > + return 0; > +} > + > +static int sfi_write_cpus(struct table_info *tab) > +{ > + struct sfi_cpu_table_entry *entry = get_entry_start(tab); > + struct udevice *dev; > + int count = 0; > + > + if (!entry) > + return -ENOSPC; Need a blank line here. > + for (uclass_find_first_device(UCLASS_CPU, &dev); UCLASS_CPU is introduced in #8 of this series. Please reorder the patches. > + dev; > + uclass_find_next_device(&dev)) { > + struct cpu_platdata *plat = dev_get_parent_platdata(dev); > + > + if (!device_active(dev)) > + continue; > + entry->apic_id = plat->cpu_id; > + entry++; > + count++; > + } > + > + /* Omit the table if there is only one CPU */ > + if (count > 1) > + finish_table(tab, SFI_SIG_CPUS, entry); > + > + return 0; > +} > + > +static int sfi_write_apic(struct table_info *tab) > +{ > + struct sfi_apic_table_entry *entry = get_entry_start(tab); > + > + if (!entry) > + return -ENOSPC; > + > + entry->phys_addr = IO_APIC_ADDR; > + entry++; > + finish_table(tab, SFI_SIG_APIC, entry); > + > + return 0; > +} > + > +static int sfi_write_rtc(struct table_info *tab) > +{ > + struct sfi_rtc_table_entry *entry = get_entry_start(tab); > + > + if (!entry) > + return -ENOSPC; > + > + entry->phys_addr = CONFIG_SYS_ISA_IO_BASE_ADDRESS + 0x70; > + entry->irq = 0; /* Should be 8? */ > + entry++; > + finish_table(tab, SFI_SIG_MRTC, entry); > + > + return 0; > +} Is this RTC table a must-have that without it Linux cannot boot? If not, I think we can leave it unimplemented. > +static int sfi_write_xsdt(struct table_info *tab) > +{ > + struct sfi_xsdt_header *entry = get_entry_start(tab); > + > + if (!entry) > + return -ENOSPC; > + > + entry->oem_revision = 1; > + entry->creator_id = 1; > + entry->creator_revision = 1; > + entry++; > + finish_table(tab, SFI_SIG_XSDT, entry); > + > + return 0; > +} > + > +int sfi_write_tables(void) > +{ > + struct table_info table; > + > + table.base = CONFIG_SFI_BASE; > + table.ptr = 0; > + table.count = 0; > + sfi_write_cpus(&table); > + sfi_write_apic(&table); > + sfi_write_rtc(&table); > + > + /* > + * The SFI specification marks the XSDT table as option, but Linux 4.0 > + * crashes on start-up when it is not provided. > + */ > + sfi_write_xsdt(&table); > + > + /* Finally, write out the system header which points to the others */ > + sfi_write_system_header(&table); > + > + return 0; > +} > diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c > index c3f8a73..d2c68f6 100644 > --- a/arch/x86/lib/zimage.c > +++ b/arch/x86/lib/zimage.c > @@ -24,6 +24,7 @@ > #include > #endif > #include > +#include > > DECLARE_GLOBAL_DATA_PTR; > > @@ -232,9 +233,15 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, > { > struct setup_header *hdr = &setup_base->hdr; > int bootproto = get_boot_protocol(hdr); > + int ret; > > setup_base->e820_entries = install_e820_map( > ARRAY_SIZE(setup_base->e820_map), setup_base->e820_map); > + ret = sfi_write_tables(); > + if (ret && ret != -ENOSYS) { > + printf("Failed to write SFI tables: err=%d\n", ret); > + return ret; > + } Can we move this to arch/x86/lib/tables.c which is the central place to write x86 configuration tables? Like this: void write_tables(void) { u32 __maybe_unused rom_table_end = ROM_TABLE_ADDR; #ifdef CONFIG_GENERATE_PIRQ_TABLE rom_table_end = write_pirq_routing_table(rom_table_end); rom_table_end = ALIGN(rom_table_end, 1024); #endif #ifdef CONFIG_GENERATE_SFI_TABLE rom_table_end = write_sfi_table(rom_table_end); rom_table_end = ALIGN(rom_table_end, 1024); #endif } > if (bootproto == 0x0100) { > setup_base->screen_info.cl_magic = COMMAND_LINE_MAGIC; > diff --git a/include/linux/sfi.h b/include/linux/sfi.h > new file mode 100644 > index 0000000..4094fc7 > --- /dev/null > +++ b/include/linux/sfi.h I believe it is better to put this file to arch/x86/include/asm/ as this is x86-specific tables (like the apic stuff). > @@ -0,0 +1,139 @@ > +/* > + * Copyright(c) 2009 Intel Corporation. All rights reserved. > + * > + * SPDX-License-Identifier: GPL-2.0+ BSD-3-Clause > + */ > + > +#ifndef _LINUX_SFI_H > +#define _LINUX_SFI_H > + > +#include > +#include > + > +/* Table signatures reserved by the SFI specification */ > +#define SFI_SIG_SYST "SYST" > +#define SFI_SIG_FREQ "FREQ" > +#define SFI_SIG_IDLE "IDLE" The SFI spec v0.8.2 deleted the "IDEL" table. > +#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_DEVS "DEVS" > +#define SFI_SIG_GPIO "GPIO" > + > +#define SFI_SIGNATURE_SIZE 4 > +#define SFI_OEM_ID_SIZE 6 > +#define SFI_OEM_TABLE_ID_SIZE 8 > + > +#define SFI_NAME_LEN 16 > + > +#define SFI_SYST_SEARCH_BEGIN 0x000e0000 > +#define SFI_SYST_SEARCH_END 0x000fffff I think we can remove these two macros. > +#define SFI_GET_NUM_ENTRIES(ptable, entry_type) \ > + ((ptable->header.len - sizeof(struct sfi_table_header)) / \ > + (sizeof(entry_type))) > + > +/* Table structures must be byte-packed to match the SFI specification */ > +struct sfi_table_header { > + char sig[SFI_SIGNATURE_SIZE]; > + u32 len; > + u8 rev; > + u8 csum; > + char oem_id[SFI_OEM_ID_SIZE]; > + char oem_table_id[SFI_OEM_TABLE_ID_SIZE]; > +} __packed; IIRC, the checkpatch.pl will complain if __packed is put after the structure defines. If that's true, please fix it globally. > +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 phys_start; > + u64 virt_start; > + u64 pages; > + u64 attrib; > +} __packed; > + > +struct sfi_cpu_table_entry { > + u32 apic_id; > +} __packed; > + > +struct sfi_cstate_table_entry { > + u32 hint; /* MWAIT hint */ > + u32 latency; /* latency in ms */ > +} __packed; > + > +struct sfi_apic_table_entry { > + u64 phys_addr; /* phy base addr for APIC reg */ > +} __packed; > + > +struct sfi_freq_table_entry { > + u32 freq_mhz; /* in MHZ */ > + u32 latency; /* transition latency in ms */ > + u32 ctrl_val; /* value to write to PERF_CTL */ > +} __packed; > + > +struct sfi_wake_table_entry { > + u64 phys_addr; /* pointer to where the wake vector locates */ > +} __packed; > + > +struct sfi_timer_table_entry { > + u64 phys_addr; /* phy base addr for the timer */ > + u32 freq_hz; /* in HZ */ > + u32 irq; > +} __packed; > + > +struct sfi_rtc_table_entry { > + u64 phys_addr; /* phy base addr for the RTC */ > + u32 irq; > +} __packed; > + > +struct sfi_device_table_entry { > + u8 type; /* bus type, I2C, SPI or ...*/ > +#define SFI_DEV_TYPE_SPI 0 > +#define SFI_DEV_TYPE_I2C 1 > +#define SFI_DEV_TYPE_UART 2 > +#define SFI_DEV_TYPE_HSI 3 > +#define SFI_DEV_TYPE_IPC 4 There is one more type added in SFI spec v0.8.2 which is SD. And please move these defines out of the structure. > + u8 host_num; /* attached to host 0, 1...*/ > + u16 addr; > + u8 irq; > + u32 max_freq; > + char name[SFI_NAME_LEN]; > +} __packed; > + > +struct sfi_gpio_table_entry { > + char controller_name[SFI_NAME_LEN]; > + u16 pin_no; > + char pin_name[SFI_NAME_LEN]; > +} __packed; > + > +struct sfi_xsdt_header { > + uint32_t oem_revision; > + uint32_t creator_id; > + uint32_t creator_revision; > +}; > + > +typedef int (*sfi_table_handler) (struct sfi_table_header *table); > + > +#ifdef CONFIG_SFI There is no need to wrap this with #ifdefs. > +/** > + * sfi_write_tables() - Write Simple Firmware Interface tables > + */ > +int sfi_write_tables(void); > + > +#else > + > +static inline int sfi_write_tables(void) { return -ENOSYS; } > + > +#endif > + > +#endif /*_LINUX_SFI_H */ > -- Regards, Bin