[U-Boot] [UBOOT PATCH v5 1/3] x86: Generate a valid ACPI table
Saket Sinha
saket.sinha89 at gmail.com
Fri Aug 21 06:33:09 CEST 2015
Hi Bin,
Please find my response inline -
On Tue, Aug 18, 2015 at 12:36 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Saket,
>
> On Tue, Aug 18, 2015 at 3:29 AM, Saket Sinha <saket.sinha89 at gmail.com> wrote:
>> Implement write_acpi_table() to create a minimal working ACPI table.
>> This includes writing FACS, XSDT, RSDP, FADT, MCFG, MADT, DSDT & SSDT
>> ACPI table entries.
>>
>> Use a Kconfig option GENERATE_ACPI_TABLE to tell U-Boot whether we need
>> actually write the APCI table just like we did for PIRQ routing, MP table
>> and SFI tables. With ACPI table existence, linux kernel gets control of
>> power management, thermal management, configuration management and
>> monitoring in hardware.
>>
>
> Nice write-up!
>
>> Signed-off-by: Saket Sinha <saket.sinha89 at gmail.com>
>> ---
>>
>> arch/x86/Kconfig | 9 +
>> arch/x86/include/asm/acpi_table.h | 390 ++++++++++++++++++++++++++++++++++
>> arch/x86/lib/Makefile | 1 +
>> arch/x86/lib/acpi_table.c | 433 ++++++++++++++++++++++++++++++++++++++
>> arch/x86/lib/tables.c | 5 +
>> scripts/Makefile.lib | 11 +
>> 6 files changed, 849 insertions(+)
>> create mode 100644 arch/x86/include/asm/acpi_table.h
>> create mode 100644 arch/x86/lib/acpi_table.c
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 01ed760..ae881a1 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -344,6 +344,15 @@ config GENERATE_MP_TABLE
>> multiprocessing as well as symmetric I/O interrupt handling with
>> the local APIC and I/O APIC.
>>
>> +config GENERATE_ACPI_TABLE
>> + bool "Generate an ACPI (Advanced Configuration and Power Interface) table"
>> + default n
>> + help
>> + The Advanced Configuration and Power Interface (ACPI) specification
>> + provides an open standard for device configuration and management
>> + by the operating system. It defines platform-independent interfaces
>> + for configuration and power management monitoring.
>> +
>> endmenu
>>
>> config MAX_PIRQ_LINKS
>> diff --git a/arch/x86/include/asm/acpi_table.h b/arch/x86/include/asm/acpi_table.h
>> new file mode 100644
>> index 0000000..a813a0a
>> --- /dev/null
>> +++ b/arch/x86/include/asm/acpi_table.h
>> @@ -0,0 +1,390 @@
>> +/*
>> + * Based on acpi.c from coreboot
>> + *
>> + * Copyright (C) 2015, Saket Sinha <saket.sinha89 at gmail.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <malloc.h>
>> +#include <asm/post.h>
>> +#include <linux/string.h>
>> +
>> +#define RSDP_SIG "RSD PTR " /* RSDT pointer signature */
>> +#define ACPI_TABLE_CREATOR "UBOOT " /* Must be exactly 8 bytes long! */
>> +#define OEM_ID "UBOOT " /* Must be exactly 6 bytes long! */
>> +#define ASLC "INTL" /* Must be exactly 4 bytes long! */
>> +
>
> Nits: could we use tab to between macro names, value and comments?
Addressed in patchset series v6.
>
>> +#define OEM_REVISION 42
>> +#define ASL_COMPILER_REVISION 42
>> +
>
> ditto
>
Addressed in patchset series v6.
>> +#define APM_CNT 0xb2
>> +#define APM_CNT_CST_CONTROL 0x85
>> +#define APM_CNT_PST_CONTROL 0x80
>> +#define APM_CNT_ACPI_DISABLE 0x1e
>> +#define APM_CNT_ACPI_ENABLE 0xe1
>> +#define APM_CNT_MBI_UPDATE 0xeb
>> +#define APM_CNT_GNVS_UPDATE 0xea
>> +#define APM_CNT_FINALIZE 0xcb
>> +#define APM_CNT_LEGACY 0xcc
>> +#define APM_ST 0xb3
>> +
>
> ditto, and can you add some comments on what these are?
>
Addressed in patchset series v6.
>> +#define MP_IRQ_POLARITY_DEFAULT 0x0
>> +#define MP_IRQ_POLARITY_HIGH 0x1
>> +#define MP_IRQ_POLARITY_LOW 0x3
>> +#define MP_IRQ_POLARITY_MASK 0x3
>> +#define MP_IRQ_TRIGGER_DEFAULT 0x0
>> +#define MP_IRQ_TRIGGER_EDGE 0x4
>> +#define MP_IRQ_TRIGGER_LEVEL 0xc
>> +#define MP_IRQ_TRIGGER_MASK 0xc
>> +
>
> ditto, and can you add some comments on what these are?
>
Addressed in patchset series v6.
>> +#define ACTL 0x00
>> +#define SCIS_MASK 0x07
>> +#define SCIS_IRQ9 0x00
>> +#define SCIS_IRQ10 0x01
>> +#define SCIS_IRQ11 0x02
>> +#define SCIS_IRQ20 0x04
>> +#define SCIS_IRQ21 0x05
>> +#define SCIS_IRQ22 0x06
>> +#define SCIS_IRQ23 0x07
>> +
>
> ditto, and can you add some comments on what these are?
>
Addressed in patchset series v6.
>> +enum acpi_bus_type {
>> + PIC = 0,
>> + APIC = 2,
>> + ETHIGH = 5
>> +};
>> +
>
> I remember I pointed this out in previous version that the name of
> PIC, APIC is too generic.
>
Addressed in patchset series v6.
>> +#define ACPI_REV_ACPI_1_0 1
>> +#define ACPI_REV_ACPI_2_0 1
>> +#define ACPI_REV_ACPI_3_0 2
>> +#define ACPI_REV_ACPI_4_0 3
>> +#define ACPI_REV_ACPI_5_0 5
>> +
>> +#define ACPI_RSDP_REV_ACPI_1_0 0
>> +#define ACPI_RSDP_REV_ACPI_2_0 2
>> +
>> +typedef struct acpi_gen_regaddr {
>> + u8 space_id; /* Address space ID */
>> + u8 bit_width; /* Register size in bits */
>> + u8 bit_offset; /* Register bit offset */
>> + union {
>> + /* Reserved in ACPI 2.0 - 2.0b */
>> + u8 resv;
>> + /* Access size in ACPI 2.0c/3.0/4.0/5.0 */
>> + u8 access_size;
>> + };
>> + u32 addrl; /* Register address, low 32 bits */
>> + u32 addrh; /* Register address, high 32 bits */
>> +} acpi_addr_t;
>> +
>> +
>> +/* RSDP (Root System Description Pointer)
>> +Note: ACPI 1.0 didn't have length, xsdt_address, and ext_checksum. */
>
> Please use multi-line comment format. Please fix this globally (I
> think I pointed this out in previous review)
>
Addressed in patchset series v6.
>> +struct acpi_rsdp {
>> + char signature[8]; /* RSDP signature */
>> + u8 checksum; /* Checksum of the first 20 bytes */
>> + char oem_id[6]; /* OEM ID */
>> + u8 revision; /* 0 for ACPI 1.0, 2 for ACPI 2.0/3.0/4.0 */
>> + u32 rsdt_address; /* Physical address of RSDT (32 bits) */
>> + u32 length; /* Total RSDP length (incl. extended part) */
>> + u64 xsdt_address; /* Physical address of XSDT (64 bits) */
>> + u8 ext_checksum; /* Checksum of the whole table */
>> + u8 reserved[3];
>> +};
>> +
>> +enum acpi_address_space_type {
>> + ACPI_ADDRESS_SPACE_MEMORY = 0, /* System memory */
>> + ACPI_ADDRESS_SPACE_IO = 1, /* System I/O */
>> + ACPI_ADDRESS_SPACE_PCI = 2, /* PCI config space */
>> + ACPI_ADDRESS_SPACE_EC = 3, /* Embedded controller */
>> + ACPI_ADDRESS_SPACE_SMBUS = 4, /* SMBus */
>> + ACPI_ADDRESS_SPACE_PCC = 0x0a, /* Platform Comm. Channel */
>> + ACPI_ADDRESS_SPACE_FIXED = 0x7f /* Functional fixed hardware */
>> +};
>> +
>> +#define ACPI_FFIXEDHW_VENDOR_INTEL 1 /* Intel */
>> +#define ACPI_FFIXEDHW_CLASS_HLT 0 /* C1 Halt */
>> +#define ACPI_FFIXEDHW_CLASS_IO_HLT 1 /* C1 I/O then Halt */
>> +#define ACPI_FFIXEDHW_CLASS_MWAIT 2 /* MWAIT Native C-state */
>> +#define ACPI_FFIXEDHW_FLAG_HW_COORD 1 /* Hardware Coordination bit */
>> +#define ACPI_FFIXEDHW_FLAG_BM_STS 2 /* BM_STS avoidance bit */
>> +
>> +/* Access size definitions for Generic address structure */
>> +enum acpi_address_space_size {
>> + ACPI_ACCESS_SIZE_UNDEFINED = 0, /* Undefined (legacy reasons) */
>> + ACPI_ACCESS_SIZE_BYTE_ACCESS = 1,
>> + ACPI_ACCESS_SIZE_WORD_ACCESS = 2,
>> + ACPI_ACCESS_SIZE_DWORD_ACCESS = 3,
>> + ACPI_ACCESS_SIZE_QWORD_ACCESS = 4
>> +};
>> +
>> +/* Generic ACPI header, provided by (almost) all tables */
>> +typedef struct acpi_table_header {
>> + char signature[4]; /* ACPI signature (4 ASCII characters) */
>> + u32 length; /* Table length in bytes (incl. header) */
>> + u8 revision; /* Table version (not ACPI version!) */
>> + volatile u8 checksum; /* To make sum of entire table == 0 */
>> + char oem_id[6]; /* OEM identification */
>> + char oem_table_id[8]; /* OEM table identification */
>> + u32 oem_revision; /* OEM revision number */
>> + char asl_compiler_id[4]; /* ASL compiler vendor ID */
>> + u32 asl_compiler_revision; /* ASL compiler revision number */
>> +} acpi_header_t;
>> +
>> +/* A maximum number of 32 ACPI tables ought to be enough for now. */
>
> Please remove the ending .
>
Addressed in patchset series v6.
>> +#define MAX_ACPI_TABLES 32
>> +
>> +/* RSDT (Root System Description Table) */
>> +struct acpi_rsdt {
>> + struct acpi_table_header header;
>> + u32 entry[MAX_ACPI_TABLES];
>> +};
>> +
>> +/* XSDT (Extended System Description Table) */
>> +struct acpi_xsdt {
>> + struct acpi_table_header header;
>> + u64 entry[MAX_ACPI_TABLES];
>> +};
>> +
>> +/* MCFG (PCI Express MMIO config space BAR description table) */
>> +struct acpi_mcfg {
>> + struct acpi_table_header header;
>> + u8 reserved[8];
>> +};
>> +
>> +struct acpi_mcfg_mmconfig {
>> + u32 base_address;
>> + u32 base_reserved;
>> + u16 pci_segment_group_number;
>> + u8 start_bus_number;
>> + u8 end_bus_number;
>> + u8 reserved[4];
>> +};
>> +
>> +/* MADT (Multiple APIC Description Table) */
>> +struct acpi_madt {
>> + struct acpi_table_header header;
>> + u32 lapic_addr; /* Local APIC address */
>> + u32 flags; /* Multiple APIC flags */
>> +} acpi_madt_t;
>> +
>> +enum dev_scope_type {
>> + SCOPE_PCI_ENDPOINT = 1,
>> + SCOPE_PCI_SUB = 2,
>> + SCOPE_IOAPIC = 3,
>> + SCOPE_MSI_HPET = 4
>> +};
>> +
>> +typedef struct dev_scope {
>> + u8 type;
>> + u8 length;
>> + u8 reserved[2];
>> + u8 enumeration;
>> + u8 start_bus;
>> + struct {
>> + u8 dev;
>> + u8 fn;
>> + } path[0];
>> +} __packed dev_scope_t;
>> +
>> +/* MADT: APIC Structure Type*/
>> +enum acpi_apic_types {
>> + LOCALAPIC = 0, /* Processor local APIC */
>> + IOAPIC = 1, /* I/O APIC */
>> + IRQSOURCEOVERRIDE = 2, /* Interrupt source override */
>> + NMITYPE = 3, /* NMI source */
>> + LOCALNMITYPE = 4, /* Local APIC NMI */
>> + LAPICADDRESSOVERRIDE = 5, /* Local APIC address override */
>> + IOSAPIC = 6, /* I/O SAPIC */
>> + LOCALSAPIC = 7, /* Local SAPIC */
>> + PLATFORMIRQSOURCES = 8, /* Platform interrupt sources */
>> + LOCALX2SAPIC = 9, /* Processor local x2APIC */
>> + LOCALX2APICNMI = 10, /* Local x2APIC NMI */
>> +};
>> +
>> +/* MADT: Processor Local APIC Structure */
>> +struct acpi_madt_lapic {
>> + u8 type; /* Type (0) */
>> + u8 length; /* Length in bytes (8) */
>> + u8 processor_id; /* ACPI processor ID */
>> + u8 apic_id; /* Local APIC ID */
>> + u32 flags; /* Local APIC flags */
>> +};
>> +
>> +#define LOCAL_APIC_FLAG_ENABLED (1 << 0)
>> +/* bits 1-31: reserved */
>> +#define PCAT_COMPAT (1 << 0)
>> +/* bits 1-31: reserved */
>> +
>> +/* MADT: Local APIC NMI Structure */
>> +struct acpi_madt_lapic_nmi {
>> + u8 type; /* Type (4) */
>> + u8 length; /* Length in bytes (6) */
>> + u8 processor_id; /* ACPI processor ID */
>> + u16 flags; /* MPS INTI flags */
>> + u8 lint; /* Local APIC LINT# */
>> +};
>> +
>> +/* MADT: I/O APIC Structure */
>> +struct acpi_madt_ioapic {
>> + u8 type; /* Type (1) */
>> + u8 length; /* Length in bytes (12) */
>> + u8 ioapic_id; /* I/O APIC ID */
>> + u8 reserved;
>> + u32 ioapic_addr; /* I/O APIC address */
>> + u32 gsi_base; /* Global system interrupt base */
>> +};
>> +
>> +/* MADT: Interrupt Source Override Structure */
>> +struct acpi_madt_irqoverride {
>> + u8 type; /* Type (2) */
>> + u8 length; /* Length in bytes (10) */
>> + u8 bus; /* ISA (0) */
>> + u8 source; /* Bus-relative int. source (IRQ) */
>> + u32 gsirq; /* Global system interrupt */
>> + u16 flags; /* MPS INTI flags */
>> +};
>> +
>> +/* FADT (Fixed ACPI Description Table) */
>> +struct __packed acpi_fadt {
>> + struct acpi_table_header header;
>> + u32 firmware_ctrl;
>> + u32 dsdt;
>> + u8 model;
>> + u8 preferred_pm_profile;
>> + u16 sci_int;
>> + u32 smi_cmd;
>> + u8 acpi_enable;
>> + u8 acpi_disable;
>> + u8 s4bios_req;
>> + u8 pstate_cnt;
>> + u32 pm1a_evt_blk;
>> + u32 pm1b_evt_blk;
>> + u32 pm1a_cnt_blk;
>> + u32 pm1b_cnt_blk;
>> + u32 pm2_cnt_blk;
>> + u32 pm_tmr_blk;
>> + u32 gpe0_blk;
>> + u32 gpe1_blk;
>> + u8 pm1_evt_len;
>> + u8 pm1_cnt_len;
>> + u8 pm2_cnt_len;
>> + u8 pm_tmr_len;
>> + u8 gpe0_blk_len;
>> + u8 gpe1_blk_len;
>> + u8 gpe1_base;
>> + u8 cst_cnt;
>> + u16 p_lvl2_lat;
>> + u16 p_lvl3_lat;
>> + u16 flush_size;
>> + u16 flush_stride;
>> + u8 duty_offset;
>> + u8 duty_width;
>> + u8 day_alrm;
>> + u8 mon_alrm;
>> + u8 century;
>> + u16 iapc_boot_arch;
>> + u8 res2;
>> + u32 flags;
>> + struct acpi_gen_regaddr reset_reg;
>> + u8 reset_value;
>> + u8 res3;
>> + u8 res4;
>> + u8 res5;
>> + u32 x_firmware_ctl_l;
>> + u32 x_firmware_ctl_h;
>> + u32 x_dsdt_l;
>> + u32 x_dsdt_h;
>> + struct acpi_gen_regaddr x_pm1a_evt_blk;
>> + struct acpi_gen_regaddr x_pm1b_evt_blk;
>> + struct acpi_gen_regaddr x_pm1a_cnt_blk;
>> + struct acpi_gen_regaddr x_pm1b_cnt_blk;
>> + struct acpi_gen_regaddr x_pm2_cnt_blk;
>> + struct acpi_gen_regaddr x_pm_tmr_blk;
>> + struct acpi_gen_regaddr x_gpe0_blk;
>> + struct acpi_gen_regaddr x_gpe1_blk;
>> +};
>> +
>> +/* Flags for p_lvl2_lat and p_lvl3_lat */
>> +#define ACPI_FADT_C2_NOT_SUPPORTED 101
>> +#define ACPI_FADT_C3_NOT_SUPPORTED 1001
>> +
>> +/* FADT Feature Flags */
>> +#define ACPI_FADT_WBINVD (1 << 0)
>> +#define ACPI_FADT_WBINVD_FLUSH (1 << 1)
>> +#define ACPI_FADT_C1_SUPPORTED (1 << 2)
>> +#define ACPI_FADT_C2_MP_SUPPORTED (1 << 3)
>> +#define ACPI_FADT_POWER_BUTTON (1 << 4)
>> +#define ACPI_FADT_SLEEP_BUTTON (1 << 5)
>> +#define ACPI_FADT_FIXED_RTC (1 << 6)
>> +#define ACPI_FADT_S4_RTC_WAKE (1 << 7)
>> +#define ACPI_FADT_32BIT_TIMER (1 << 8)
>> +#define ACPI_FADT_DOCKING_SUPPORTED (1 << 9)
>> +#define ACPI_FADT_RESET_REGISTER (1 << 10)
>> +#define ACPI_FADT_SEALED_CASE (1 << 11)
>> +#define ACPI_FADT_HEADLESS (1 << 12)
>> +#define ACPI_FADT_SLEEP_TYPE (1 << 13)
>> +#define ACPI_FADT_PCI_EXPRESS_WAKE (1 << 14)
>> +#define ACPI_FADT_PLATFORM_CLOCK (1 << 15)
>> +#define ACPI_FADT_S4_RTC_VALID (1 << 16)
>> +#define ACPI_FADT_REMOTE_POWER_ON (1 << 17)
>> +#define ACPI_FADT_APIC_CLUSTER (1 << 18)
>> +#define ACPI_FADT_APIC_PHYSICAL (1 << 19)
>> +/* Bits 20-31: reserved ACPI 3.0 & 4.0 */
>> +#define ACPI_FADT_HW_REDUCED_ACPI (1 << 20)
>> +#define ACPI_FADT_LOW_PWR_IDLE_S0 (1 << 21)
>> +/* bits 22-31: reserved ACPI 5.0 */
>> +
>
> Nits: could we use tab to between macro names, value and comments?
>
Addressed in patchset series v6.
>> +/* FADT Boot Architecture Flags */
>> +#define ACPI_FADT_LEGACY_DEVICES (1 << 0)
>> +#define ACPI_FADT_8042 (1 << 1)
>> +#define ACPI_FADT_VGA_NOT_PRESENT (1 << 2)
>> +#define ACPI_FADT_MSI_NOT_SUPPORTED (1 << 3)
>> +#define ACPI_FADT_NO_PCIE_ASPM_CONTROL (1 << 4)
>> +#define ACPI_FADT_LEGACY_FREE 0x00 /* No legacy devices (including 8042) */
>> +
>
> ditto.
>
Addressed in patchset series v6.
>> +/* FADT Preferred Power Management Profile */
>> +#define PM_UNSPECIFIED 0
>> +#define PM_DESKTOP 1
>> +#define PM_MOBILE 2
>> +#define PM_WORKSTATION 3
>> +#define PM_ENTERPRISE_SERVER 4
>> +#define PM_SOHO_SERVER 5
>> +#define PM_APPLIANCE_PC 6
>> +#define PM_PERFORMANCE_SERVER 7
>> +#define PM_TABLET 8, /* ACPI 5.0 */
>> +
>
> ditto.
>
Addressed in patchset series v6.
>> +/* FACS (Firmware ACPI Control Structure) */
>> +struct acpi_facs {
>> + char signature[4]; /* "FACS" */
>> + u32 length; /* Length in bytes (>= 64) */
>> + u32 hardware_signature; /* Hardware signature */
>> + u32 firmware_waking_vector; /* Firmware waking vector */
>> + u32 global_lock; /* Global lock */
>> + u32 flags; /* FACS flags */
>> + u32 x_firmware_waking_vector_l; /* X FW waking vector, low */
>> + u32 x_firmware_waking_vector_h; /* X FW waking vector, high */
>> + u8 version; /* ACPI 4.0: 2 */
>> + u8 resv[31]; /* FIXME: 4.0: ospm_flags */
>> +};
>> +
>> +/* FACS flags */
>> +#define ACPI_FACS_S4BIOS_F (1 << 0)
>> +#define ACPI_FACS_64BIT_WAKE_F (1 << 1)
>> +/* Bits 31..2: reserved */
>> +
>> +/* These can be used by the target port. */
>> +
>> +unsigned long acpi_create_madt_lapics(unsigned long current);
>> +int acpi_create_madt_ioapic(struct acpi_madt_ioapic *ioapic, u8 id, u32 addr,
>
> Can we always use u32 for the parameters? like u32 id
>
These will make me change all the structure and recheck that nothing
breaks since these values are getting assigned to the table structures
directly.
I do not think this makes any sense to have 4 bytes for something that
can be fitted in 1 byte.
>> + u32 gsi_base);
>> +int acpi_create_madt_irqoverride(struct acpi_madt_irqoverride *irqoverride,
>> + u8 bus, u8 source, u32 gsirq, u16 flags);
>
> Ditto, for bus, source and flags
>
Addressed in patchset series v6.
>> +unsigned long acpi_fill_madt(unsigned long current);
>> +void acpi_create_fadt(struct acpi_fadt *fadt, struct acpi_facs *facs,
>> + void *dsdt);
>
> I believe checkpatch.pl will report indention warning here.
>
Addressed in patchset series v6.
>> +int acpi_create_madt_lapic_nmi(struct acpi_madt_lapic_nmi *lapic_nmi, u8 cpu,
>> + u16 flags, u8 lint);
>
> Ditto, for cpu, flags and lint.
>
I do not think this makes any sense to have 4 bytes for something that
can be fitted in 1 byte.
>> +unsigned long write_acpi_tables(unsigned long start);
>> +
>> diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
>> index dcfe9ee..6ecd6db 100644
>> --- a/arch/x86/lib/Makefile
>> +++ b/arch/x86/lib/Makefile
>> @@ -30,6 +30,7 @@ obj-y += physmem.o
>> obj-$(CONFIG_X86_RAMTEST) += ramtest.o
>> obj-y += sfi.o
>> obj-y += string.o
>> +obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi_table.o
>> obj-y += tables.o
>> obj-$(CONFIG_SYS_X86_TSC_TIMER) += tsc_timer.o
>> obj-$(CONFIG_CMD_ZBOOT) += zimage.o
>> diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
>> new file mode 100644
>> index 0000000..bd880ea
>> --- /dev/null
>> +++ b/arch/x86/lib/acpi_table.c
>> @@ -0,0 +1,433 @@
>> +/*
>> + * Based on acpi.c from coreboot
>> + *
>> + * Copyright (C) 2015, Saket Sinha <saket.sinha89 at gmail.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <cpu.h>
>> +#include <dm.h>
>> +#include <dm/uclass-internal.h>
>> +#include <dm/lists.h>
>> +#include <asm/acpi_table.h>
>> +#include <asm/cpu.h>
>> +#include <asm/ioapic.h>
>> +#include <asm/lapic.h>
>> +#include <asm/tables.h>
>> +#include <asm/pci.h>
>> +
>> +
>> +extern const unsigned char AmlCode[];
>> +
>
> Please add a comment block here to explain why we have to use
> CamelCase, in case someone later wants to improve but will fail.
>
Addressed in patchset series v6.
>> +/**
>> +* Add an ACPI table to the RSDT (and XSDT) structure, recalculate length
>> +* and checksum.
>> +*/
>> +static void acpi_add_table(struct acpi_rsdp *rsdp, void *table)
>> +{
>> + int i, entries_num;
>> + struct acpi_rsdt *rsdt;
>> + struct acpi_xsdt *xsdt = NULL;
>> +
>> + /* The RSDT is mandatory while the XSDT is not */
>> + rsdt = (struct acpi_rsdt *)rsdp->rsdt_address;
>> +
>> + if (rsdp->xsdt_address)
>> + xsdt = (struct acpi_xsdt *)((u32)rsdp->xsdt_address);
>> +
>> + /* This should always be MAX_ACPI_TABLES */
>> + entries_num = ARRAY_SIZE(rsdt->entry);
>> +
>> + for (i = 0; i < entries_num; i++) {
>> + if (rsdt->entry[i] == 0)
>> + break;
>> + }
>> +
>> + if (i >= entries_num) {
>> + debug("ACPI: Error: too many tables.\n");
>> + return;
>> + }
>> +
>> + /* Add table to the RSDT */
>> + rsdt->entry[i] = (u32)table;
>> +
>> + /* Fix RSDT length or the kernel will assume invalid entries */
>> + rsdt->header.length = sizeof(acpi_header_t) + (sizeof(u32) * (i + 1));
>> +
>> + /* Re-calculate checksum. */
>
> Nits: please remove the ending .
>
Addressed in patchset series v6.
>> + rsdt->header.checksum = 0;
>> + rsdt->header.checksum = table_compute_checksum((u8 *)rsdt,
>> + rsdt->header.length);
>> +
>> + /*
>> + * And now the same thing for the XSDT. We use the same index as for
>> + * now we want the XSDT and RSDT to always be in sync in uboot
>
> Should be U-Boot
>
Addressed in patchset series v6.
>> + */
>> + if (xsdt) {
>> + /* Add table to the XSDT */
>> + xsdt->entry[i] = (u64)(u32)table;
>> +
>> + /* Fix XSDT length */
>> + xsdt->header.length = sizeof(acpi_header_t) +
>> + (sizeof(u64) * (i + 1));
>
> Can we adjust the indention here, to align with sizeof above?
>
Addressed in patchset series v6.
>> +
>> + /* Re-calculate checksum */
>> + xsdt->header.checksum = 0;
>> + xsdt->header.checksum = table_compute_checksum((u8 *)xsdt,
>> + xsdt->header.length);
>> + }
>> +}
>> +
>> +static int acpi_create_madt_lapic(struct acpi_madt_lapic *lapic,
>> + u8 cpu, u8 apic)
>
> I believe chckpatch.pl will report indention warnings. Please fix this globally.
>
Addressed in patchset series v6.
>> +{
>> + lapic->type = LOCALAPIC; /* Local APIC structure */
>> + lapic->length = sizeof(struct acpi_madt_lapic);
>> + lapic->flags = LOCAL_APIC_FLAG_ENABLED; /* Processor/LAPIC enabled */
>> + lapic->processor_id = cpu;
>> + lapic->apic_id = apic;
>> +
>> + return lapic->length;
>> +}
>> +
>> +unsigned long acpi_create_madt_lapics(unsigned long current)
>> +{
>> + struct udevice *dev;
>> +
>> + for (uclass_find_first_device(UCLASS_CPU, &dev);
>> + dev;
>> + uclass_find_next_device(&dev)) {
>> + struct cpu_platdata *plat = dev_get_parent_platdata(dev);
>> +
>> + current += acpi_create_madt_lapic
>> + ((struct acpi_madt_lapic *)current,
>> + plat->cpu_id, plat->cpu_id);
>> + }
>> + return current;
>> +}
>> +
>> +int acpi_create_madt_ioapic(struct acpi_madt_ioapic *ioapic, u8 id, u32 addr,
>> + u32 gsi_base)
>> +{
>> + ioapic->type = IOAPIC;
>> + ioapic->length = sizeof(struct acpi_madt_ioapic);
>> + ioapic->reserved = 0x00;
>> + ioapic->gsi_base = gsi_base;
>> + ioapic->ioapic_id = id;
>> + ioapic->ioapic_addr = addr;
>> +
>> + return ioapic->length;
>> +}
>> +
>> +int acpi_create_madt_irqoverride(struct acpi_madt_irqoverride *irqoverride,
>> + u8 bus, u8 source, u32 gsirq, u16 flags)
>> +{
>> + irqoverride->type = IRQSOURCEOVERRIDE;
>> + irqoverride->length = sizeof(struct acpi_madt_irqoverride);
>> + irqoverride->bus = bus;
>> + irqoverride->source = source;
>> + irqoverride->gsirq = gsirq;
>> + irqoverride->flags = flags;
>> +
>> + return irqoverride->length;
>> +}
>> +
>> +int acpi_create_madt_lapic_nmi(struct acpi_madt_lapic_nmi *lapic_nmi,
>> + u8 cpu, u16 flags, u8 lint)
>> +{
>> + lapic_nmi->type = LOCALNMITYPE;
>> + lapic_nmi->length = sizeof(struct acpi_madt_lapic_nmi);
>> + lapic_nmi->flags = flags;
>> + lapic_nmi->processor_id = cpu;
>> + lapic_nmi->lint = lint;
>> +
>> + return lapic_nmi->length;
>> +}
>> +
>> +static void fill_header(acpi_header_t *header, char *signature, int length)
>> +{
>> + memcpy(header->signature, signature, length);
>> + memcpy(header->oem_id, OEM_ID, 6);
>> + memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);
>> + memcpy(header->asl_compiler_id, ASLC, 4);
>> +}
>> +
>> +static void acpi_create_madt(struct acpi_madt *madt)
>> +{
>> + acpi_header_t *header = &(madt->header);
>> + unsigned long current = (unsigned long)madt + sizeof(struct acpi_madt);
>> +
>> + memset((void *)madt, 0, sizeof(struct acpi_madt));
>> +
>> + /* Fill out header fields. */
>
> Nits: please remove the ending .
>
Addressed in patchset series v6.
>> + fill_header(header, "APIC", 4);
>> + header->length = sizeof(struct acpi_madt);
>> +
>> + /* ACPI 1.0/2.0: 1, ACPI 3.0: 2, ACPI 4.0: 3 */
>> + header->revision = ACPI_REV_ACPI_2_0;
>> +
>> + madt->lapic_addr = LAPIC_DEFAULT_BASE;
>> + madt->flags = PCAT_COMPAT;
>> +
>> + current = acpi_fill_madt(current);
>> +
>> + /* (Re)calculate length and checksum */
>> + header->length = current - (unsigned long)madt;
>> +
>> + header->checksum = table_compute_checksum((void *)madt, header->length);
>> +}
>> +
>> +static int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig,
>> + u32 base, u16 seg_nr, u8 start, u8 end)
>> +{
>> + memset(mmconfig, 0, sizeof(*mmconfig));
>> + mmconfig->base_address = base;
>> + mmconfig->base_reserved = 0;
>> + mmconfig->pci_segment_group_number = seg_nr;
>> + mmconfig->start_bus_number = start;
>> + mmconfig->end_bus_number = end;
>> +
>> + return sizeof(struct acpi_mcfg_mmconfig);
>> +}
>> +
>> +static unsigned long acpi_fill_mcfg(unsigned long current)
>> +{
>> + current += acpi_create_mcfg_mmconfig
>> + ((struct acpi_mcfg_mmconfig *)current,
>
> Please remove the multiple spaces here.
>
Addressed in patchset series v6.
>> + CONFIG_PCIE_ECAM_BASE, 0x0, 0x0, 255);
>> +
>> + return current;
>> +}
>> +
>> +/* MCFG is defined in the PCI Firmware Specification 3.0 */
>> +static void acpi_create_mcfg(struct acpi_mcfg *mcfg)
>> +{
>> + acpi_header_t *header = &(mcfg->header);
>> + unsigned long current = (unsigned long)mcfg + sizeof(struct acpi_mcfg);
>> +
>> + memset((void *)mcfg, 0, sizeof(struct acpi_mcfg));
>> +
>> + /* Fill out header fields */
>> + fill_header(header, "MCFG", 4);
>> + header->length = sizeof(struct acpi_mcfg);
>> +
>> + /* ACPI 1.0/2.0: 1, ACPI 3.0: 2, ACPI 4.0: 3 */
>> + header->revision = ACPI_REV_ACPI_2_0;
>> +
>> + current = acpi_fill_mcfg(current);
>> +
>> + /* (Re)calculate length and checksum */
>> + header->length = current - (unsigned long)mcfg;
>> + header->checksum = table_compute_checksum((void *)mcfg, header->length);
>> +}
>> +
>> +static void acpi_create_facs(struct acpi_facs *facs)
>> +{
>> + memset((void *)facs, 0, sizeof(struct acpi_facs));
>> +
>> + memcpy(facs->signature, "FACS", 4);
>> + facs->length = sizeof(struct acpi_facs);
>> + facs->hardware_signature = 0;
>> + facs->firmware_waking_vector = 0;
>> + facs->global_lock = 0;
>> + facs->flags = 0;
>> + facs->x_firmware_waking_vector_l = 0;
>> + facs->x_firmware_waking_vector_h = 0;
>> + facs->version = 1; /* ACPI 1.0: 0, ACPI 2.0/3.0: 1, ACPI 4.0: 2 */
>> +}
>> +
>> +static void acpi_write_rsdt(struct acpi_rsdt *rsdt)
>> +{
>> + acpi_header_t *header = &(rsdt->header);
>> +
>> + /* Fill out header fields */
>> + fill_header(header, "RSDT", 4);
>> + header->length = sizeof(struct acpi_rsdt);
>> +
>> + /* ACPI 1.0/2.0: 1, ACPI 3.0: 2, ACPI 4.0: 3 */
>> + header->revision = ACPI_REV_ACPI_2_0;
>> +
>> + /* Entries are filled in later, we come with an empty set */
>> +
>> + /* Fix checksum */
>> + header->checksum = table_compute_checksum((void *)rsdt,
>> + sizeof(struct acpi_rsdt));
>> +}
>> +
>> +static void acpi_write_xsdt(struct acpi_xsdt *xsdt)
>> +{
>> + acpi_header_t *header = &(xsdt->header);
>> +
>> + /* Fill out header fields */
>> + fill_header(header, "XSDT", 4);
>> + header->length = sizeof(struct acpi_xsdt);
>> +
>> + /* ACPI 1.0/2.0: 1, ACPI 3.0: 2, ACPI 4.0: 3 */
>> + header->revision = ACPI_REV_ACPI_2_0;
>> +
>> + /* Entries are filled in later, we come with an empty set */
>> +
>> + /* Fix checksum */
>> + header->checksum = table_compute_checksum((void *)xsdt,
>> + sizeof(struct acpi_xsdt));
>> +}
>> +
>> +static void acpi_write_rsdp(struct acpi_rsdp *rsdp, struct acpi_rsdt *rsdt,
>> + struct acpi_xsdt *xsdt)
>> +{
>> + memset(rsdp, 0, sizeof(struct acpi_rsdp));
>> +
>> + memcpy(rsdp->signature, RSDP_SIG, 8);
>> + memcpy(rsdp->oem_id, OEM_ID, 6);
>> +
>> + rsdp->length = sizeof(struct acpi_rsdp);
>> + rsdp->rsdt_address = (u32)rsdt;
>> +
>> + /*
>> + * Revision: ACPI 1.0: 0, ACPI 2.0/3.0/4.0: 2
>> + *
>> + * Some OSes expect an XSDT to be present for RSD PTR revisions >= 2.
>> + * If we don't have an ACPI XSDT, force ACPI 1.0 (and thus RSD PTR
>> + * revision 0)
>> + */
>> + if (xsdt == NULL) {
>> + rsdp->revision = ACPI_RSDP_REV_ACPI_1_0;
>> + } else {
>> + rsdp->xsdt_address = (u64)(u32)xsdt;
>> + rsdp->revision = ACPI_RSDP_REV_ACPI_2_0;
>> + }
>> +
>> + /* Calculate checksums */
>> + rsdp->checksum = table_compute_checksum((void *)rsdp, 20);
>> + rsdp->ext_checksum = table_compute_checksum((void *)rsdp,
>> + sizeof(struct acpi_rsdp));
>> +}
>> +
>> +static void acpi_create_ssdt_generator(acpi_header_t *ssdt,
>> + const char *oem_table_id)
>> +{
>> + unsigned long current = (unsigned long)ssdt + sizeof(acpi_header_t);
>> +
>> + memset((void *)ssdt, 0, sizeof(acpi_header_t));
>> +
>> + memcpy(&ssdt->signature, "SSDT", 4);
>> + /* Access size in ACPI 2.0c/3.0/4.0/5.0 */
>> + ssdt->revision = ACPI_REV_ACPI_3_0;
>> + memcpy(&ssdt->oem_id, OEM_ID, 6);
>> + memcpy(&ssdt->oem_table_id, oem_table_id, 8);
>> + ssdt->oem_revision = OEM_REVISION;
>> + memcpy(&ssdt->asl_compiler_id, ASLC, 4);
>> + ssdt->asl_compiler_revision = ASL_COMPILER_REVISION;
>> + ssdt->length = sizeof(acpi_header_t);
>> +
>> + /* (Re)calculate length and checksum */
>> + ssdt->length = current - (unsigned long)ssdt;
>> + ssdt->checksum = table_compute_checksum((void *)ssdt, ssdt->length);
>> +}
>> +
>> +unsigned long write_acpi_tables(unsigned long start)
>> +{
>> + unsigned long current;
>> + struct acpi_rsdp *rsdp;
>> + struct acpi_rsdt *rsdt;
>> + struct acpi_xsdt *xsdt;
>> + struct acpi_facs *facs;
>> + acpi_header_t *dsdt;
>> + struct acpi_fadt *fadt;
>> + struct acpi_mcfg *mcfg;
>> + struct acpi_madt *madt;
>> + acpi_header_t *ssdt;
>> +
>> + current = start;
>> +
>> + /* Align ACPI tables to 16byte */
>> + current = (ALIGN(current, 16));
>
> Please remove the unnecessary outer () and fix this globally in this routine.
>
Addressed in patchset series v6.
>> +
>> + debug("ACPI: Writing ACPI tables at %lx.\n", start);
>> +
>> + /* We need at least an RSDP and an RSDT Table */
>> + rsdp = (struct acpi_rsdp *)current;
>> + current += sizeof(struct acpi_rsdp);
>> + current = (ALIGN(current, 16));
>> + rsdt = (struct acpi_rsdt *)current;
>> + current += sizeof(struct acpi_rsdt);
>> + current = (ALIGN(current, 16));
>> + xsdt = (struct acpi_xsdt *)current;
>> + current += sizeof(struct acpi_xsdt);
>> + current = (ALIGN(current, 16));
>> +
>> + /* clear all table memory */
>> + memset((void *)start, 0, current - start);
>> +
>> + acpi_write_rsdp(rsdp, rsdt, xsdt);
>> + acpi_write_rsdt(rsdt);
>> + acpi_write_xsdt(xsdt);
>> +
>> + debug("ACPI: * FACS\n");
>> + facs = (struct acpi_facs *)current;
>> + current += sizeof(struct acpi_facs);
>> + current = (ALIGN(current, 16));
>> +
>> + acpi_create_facs(facs);
>> +
>> + debug("ACPI: * DSDT\n");
>> + dsdt = (acpi_header_t *)current;
>> + memcpy(dsdt, &AmlCode, sizeof(acpi_header_t));
>> + if (dsdt->length >= sizeof(acpi_header_t)) {
>> + current += sizeof(acpi_header_t);
>> + memcpy((char *)current,
>> + (char *)&AmlCode + sizeof(acpi_header_t),
>> + dsdt->length - sizeof(acpi_header_t));
>> + current += dsdt->length - sizeof(acpi_header_t);
>> +
>> + /* (Re)calculate length and checksum */
>> + dsdt->length = current - (unsigned long)dsdt;
>> + dsdt->checksum = 0;
>> + dsdt->checksum = table_compute_checksum((void *)dsdt,
>> + dsdt->length);
>> + }
>> + current = (ALIGN(current, 16));
>> +
>> + debug("ACPI: * FADT\n");
>> + fadt = (struct acpi_fadt *)current;
>> + current += sizeof(struct acpi_fadt);
>> + current = (ALIGN(current, 16));
>> + acpi_create_fadt(fadt, facs, dsdt);
>> + acpi_add_table(rsdp, fadt);
>> +
>> + debug("ACPI: * MCFG\n");
>> + mcfg = (struct acpi_mcfg *)current;
>> + acpi_create_mcfg(mcfg);
>> + if (mcfg->header.length > sizeof(struct acpi_mcfg)) {
>> + current += mcfg->header.length;
>> + current = (ALIGN(current, 16));
>> + acpi_add_table(rsdp, mcfg);
>> + }
>> +
>> + debug("ACPI: * MADT\n");
>> + madt = (struct acpi_madt *)current;
>> + acpi_create_madt(madt);
>> + if (madt->header.length > sizeof(struct acpi_madt)) {
>> + current += madt->header.length;
>> + acpi_add_table(rsdp, madt);
>> + }
>> + current = (ALIGN(current, 16));
>> +
>> + debug("ACPI: * SSDT\n");
>> + ssdt = (acpi_header_t *)current;
>> + acpi_create_ssdt_generator(ssdt, ACPI_TABLE_CREATOR);
>> + if (ssdt->length > sizeof(acpi_header_t)) {
>> + current += ssdt->length;
>> + acpi_add_table(rsdp, ssdt);
>> + current = (ALIGN(current, 16));
>> + }
>> +
>> +
>
> Nits: please remove one blank linke.
>
Addressed in patchset series v6.
>> + debug("current = %lx\n", current);
>> +
>> + debug("ACPI: done.\n");
>
> Nits: please add one blank line before return.
>
Addressed in patchset series v6.
>> + return current;
>> +}
>> +
>> diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
>> index 75ffbc1..f15b2e2 100644
>> --- a/arch/x86/lib/tables.c
>> +++ b/arch/x86/lib/tables.c
>> @@ -8,6 +8,7 @@
>> #include <asm/sfi.h>
>> #include <asm/mpspec.h>
>> #include <asm/tables.h>
>> +#include <asm/acpi_table.h>
>>
>> u8 table_compute_checksum(void *v, int len)
>> {
>> @@ -51,4 +52,8 @@ void write_tables(void)
>> rom_table_end = write_mp_table(rom_table_end);
>> rom_table_end = ALIGN(rom_table_end, 1024);
>> #endif
>> +#ifdef CONFIG_GENERATE_ACPI_TABLE
>> + rom_table_end = write_acpi_tables(rom_table_end);
>> + rom_table_end = ALIGN(rom_table_end, 1024);
>> +#endif
>> }
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index 1c949fc..ed30bf5 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -297,6 +297,17 @@ $(obj)/%.dtb: $(src)/%.dts FORCE
>>
>> dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
>>
>> +# ACPI
>> +# ---------------------------------------------------------------------------
>> +quiet_cmd_acpi_c_asl= ASL $@
>> +cmd_acpi_c_asl= \
>> + $(CPP) -x assembler-with-cpp -P -o $<.tmp $<; \
>> + iasl -p $< -tc -va $<.tmp; \
>> + mv $(patsubst %.asl,%.hex,$<) $@
>> +
>> +$(obj)/%.c: $(src)/%.asl
>> + $(call cmd,acpi_c_asl)
>> +
>> # Bzip2
>> # ---------------------------------------------------------------------------
>>
>> --
>
> Regards,
> Bin
Regards,
Saket Sinha
More information about the U-Boot
mailing list