[U-Boot] [U-BOOT PATCH v3 2/4] x86: Added support for ACPI table generation at the generic layer.
Bin Meng
bmeng.cn at gmail.com
Thu Aug 13 12:01:35 CEST 2015
Hi Saket,
On Thu, Aug 13, 2015 at 11:01 AM, Saket Sinha <saket.sinha89 at gmail.com> wrote:
Please see my comments in your [1/4] patch regarding to patch title
and commit message.
> Signed-off-by: Saket Sinha <saket.sinha89 at gmail.com>
> ---
>
> arch/x86/include/asm/acpi_table.h | 387 +++++++++++++++++++++++++++++++++++
> arch/x86/lib/Makefile | 1 +
> arch/x86/lib/acpi_table.c | 413 ++++++++++++++++++++++++++++++++++++++
> arch/x86/lib/tables.c | 5 +
> scripts/Makefile.lib | 11 +
> 5 files changed, 817 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/include/asm/acpi_table.h b/arch/x86/include/asm/acpi_table.h
> new file mode 100644
> index 0000000..7b52e7d
> --- /dev/null
> +++ b/arch/x86/include/asm/acpi_table.h
> @@ -0,0 +1,387 @@
Please add a comment header to give others some hints on where this
file is ported from.
> +#include <linux/string.h>
> +#include <malloc.h>
> +#include <common.h>
> +#include <asm/post.h>
We should avoid including header files in header file.
> +
> +#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 "UBOO" /* Must be exactly 4 bytes long! */
As I mentioned in my review comments to the v2 series, please change
ASLC to "INTL" to indicate that we are using Intel ASL compiler to
compile ASL files.
> +
> +#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_STS 0xb3
> +
> +
We only allow one blank line between code blocks.
> +#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
> +
> +#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
Please remove the space between # and define.
> +
> +#define ILB_BASE_ADDRESS 0xfed08000
> +#define ILB_BASE_SIZE 0x400
> +
These two macros look chip-specific. Please remove them.
> +enum bus_type {
bus_type is too generic, maybe acpi_bus_type?
> + PIC = 0,
> + APIC = 2,
> + ETHIGH = 5
> +};
> +
> +
> +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 {
> + u8 resv; /* Reserved in ACPI 2.0 - 2.0b */
> + u8 access_size; /* Access size in ACPI 2.0c/3.0/4.0/5.0 */
> + };
The indention is not aligned to others.
> + u32 addrl; /* Register address, low 32 bits */
> + u32 addrh; /* Register address, high 32 bits */
> +} __attribute__ ((packed)) acpi_addr_t;
I believe this structure is naturally aligned. Please double check.
> +
> +
> +/* RSDP (Root System Description Pointer) */
> +struct __packed 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];
> +};
I believe this structure is naturally aligned. Please double check.
> +/* Note: ACPI 1.0 didn't have length, xsdt_address, and ext_checksum. */
This comment should be moved on top of this structure.
> +
> +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 */
Please use lower case 0xa.
> + 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 */
Please do not use two spaces after define.
> +/* 0x80-0xbf: Reserved */
> +/* 0xc0-0xff: OEM defined */
> +
What are these two for?
> +/* 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!) */
> + 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 */
> +} __attribute__ ((packed)) acpi_header_t;
I believe this structure is naturally aligned. Please double check.
> +
> +/* A maximum number of 32 ACPI tables ought to be enough for now. */
> +#define MAX_ACPI_TABLES 32
> +
> +/* RSDT (Root System Description Table) */
> +struct __packed acpi_rsdt {
> + struct acpi_table_header header;
> + u32 entry[MAX_ACPI_TABLES];
> +};
> +
> +/* XSDT (Extended System Description Table) */
> +struct __packed acpi_xsdt {
> + struct acpi_table_header header;
> + u64 entry[MAX_ACPI_TABLES];
> +};
> +
> +/* MCFG (PCI Express MMIO config space BAR description table) */
> +struct __packed acpi_mcfg {
> + struct acpi_table_header header;
> + u8 reserved[8];
> +};
> +
> +struct __packed 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 __packed acpi_madt {
> + struct acpi_table_header header;
> + u32 lapic_addr; /* Local APIC address */
> + u32 flags; /* Multiple APIC flags */
> +} __attribute__ ((packed)) acpi_madt_t;
> +
Please check all the 5 tables above. I believe all of them are
naturally aligned.
> +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;
> + } __attribute__((packed)) path[0];
> +} __attribute__ ((packed)) dev_scope_t;
Please check __packed is really necessary.
> +
> +/* MADT: APIC Structure Types */
> +/* TODO: Convert to ALLCAPS. */
Please use multi-line comment format.
> +enum acpi_apic_types {
> + LocalApic = 0, /* Processor local APIC */
> + IOApic = 1, /* I/O APIC */
> + IRQSourceOverride = 2, /* Interrupt source override */
> + NMIType = 3, /* NMI source */
> + LocalApicNMI = 4, /* Local APIC NMI */
> + LApicAddressOverride = 5, /* Local APIC address override */
> + IOSApic = 6, /* I/O SAPIC */
> + LocalSApic = 7, /* Local SAPIC */
> + PlatformIRQSources = 8, /* Platform interrupt sources */
> + Localx2Apic = 9, /* Processor local x2APIC */
> + Localx2ApicNMI = 10, /* Local x2APIC NMI */
Please do not use CamelCase.
> + /* 0x0b-0x7f: Reserved */
> + /* 0x80-0xff: Reserved for OEM use */
> +};
> +
> +/* MADT: Processor Local APIC Structure */
> +struct __packed 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 */
> +};
No need for __packed.
> +
> +/* MADT: Local APIC NMI Structure */
> +struct __packed 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 __packed 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 */
> +};
No need for __packed.
> +
> +/* MADT: Interrupt Source Override Structure */
> +struct __packed 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;
> +};
> +
> +/* FADT TABLE Revision values */
> +#define ACPI_FADT_REV_ACPI_1_0 1
> +#define ACPI_FADT_REV_ACPI_2_0 3
> +#define ACPI_FADT_REV_ACPI_3_0 4
> +#define ACPI_FADT_REV_ACPI_4_0 4
> +#define ACPI_FADT_REV_ACPI_5_0 5
Please use enum.
> +
> +/* 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 */
> +
> +/* FADT Boot Architecture Flags */
> +enum acpi_fadt_boot_flags {
> + ACPI_FADT_LEGACY_DEVICES = (1 << 0),
> + ACPI_FADT_8042 = (1 << 1),
> + ACPI_FADT_VGA_NOT_PRESENT = (1 << 2),
> + ACPI_FADT_MSI_NOT_SUPPORTED = (1 << 3),
> + ACPI_FADT_NO_PCIE_ASPM_CONTROL = (1 << 4),
> + ACPI_FADT_LEGACY_FREE = 0x00 /* No legacy devices (including 8042) */
> +};
To keep consistency, either move these to plain defines, or convert
"FADT Feature Flags" above to enum.
Please add one blank line here.
> +/* FADT Preferred Power Management Profile */
> +enum acpi_preferred_pm_profiles {
> + PM_UNSPECIFIED = 0,
> + PM_DESKTOP = 1,
> + PM_MOBILE = 2,
> + PM_WORKSTATION = 3,
> + PM_ENTERPRISE_SERVER = 4,
> + PM_SOHO_SERVER = 5,
> + PM_APPLIANCE_PC = 6,
> + PM_PERFORMANCE_SERVER = 7,
> + PM_TABLET = 8, /* ACPI 5.0 */
> +};
> +
> +/* FACS (Firmware ACPI Control Structure) */
> +struct __packed 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. */
> +
> +void acpi_add_table(struct acpi_rsdp *rsdp, void *table);
> +unsigned long write_acpi_tables(unsigned long start);
> +int acpi_create_madt_lapic(struct acpi_madt_lapic *lapic, u8 cpu, u8 apic);
> +unsigned long acpi_create_madt_lapics(unsigned long current);
> +int acpi_create_madt_ioapic(struct acpi_madt_ioapic *ioapic, u8 id, u32 addr, u32 gsi_base);
> +int acpi_create_madt_irqoverride(struct acpi_madt_irqoverride *irqoverride, u8 bus, u8 source, u32 gsirq, u16 flags);
> +unsigned long acpi_fill_mcfg(unsigned long current);
> +unsigned long acpi_fill_madt(unsigned long current);
> +void acpi_create_fadt(struct acpi_fadt *fadt, struct acpi_facs *facs, void *dsdt);
> +int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base, u16 seg_nr, u8 start, u8 end);
> +int acpi_create_madt_lapic_nmi(struct acpi_madt_lapic_nmi *lapic_nmi, u8 cpu, u16 flags, u8 lint);
Blank line please.
> +static inline uint32_t read32(const void *addr)
> +{
> + return *(volatile uint32_t *)addr;
> +}
What is this?
> +
> 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..2e20317
> --- /dev/null
> +++ b/arch/x86/lib/acpi_table.c
> @@ -0,0 +1,413 @@
> +/*
> + * Copyright (C) 2015, Saket Sinha <saket.sinha89 at gmail.com>
> + *
Please add comments to give others some hints on where this file is ported from.
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#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>
> +#include <cpu.h>
> +#include <dm.h>
> +#include <dm/uclass-internal.h>
> +#include <dm/lists.h>
> +
> +extern const unsigned char AmlCode[];
Can we avoid using CamelCase here?
> +
> +/**
> +* Add an ACPI table to the RSDT (and XSDT) structure, recalculate length
> +* and checksum.
> +*/
> +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... */
> + rsdt = (struct acpi_rsdt *)rsdp->rsdt_address;
> +
> + /* ...while the XSDT is not. */
Can we merge this comment to /* The RSDT is mandatory... */?
> + if (rsdp->xsdt_address)
> + xsdt = (struct acpi_xsdt *)((u32)rsdp->xsdt_address);
> +
> + /* This should always be MAX_ACPI_TABLES. */
Nits: please remove the ending period. Please fix this globally in all files.
> + 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: Could not add ACPI table, "
> + "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. */
> + rsdt->header.checksum = 0; /* Hope this won't get optimized away */
This is suspicious. I guess it might be optimized. Can you double
check to create some way that guarantees it won't be optimized?
> + 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 coreboot.
coreboot -> U-Boot
> + */
> + 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));
> +
> + /* Re-calculate checksum. */
> + xsdt->header.checksum = 0;
And this might be optimized too.
> + xsdt->header.checksum = table_compute_checksum((u8 *)xsdt,
> + xsdt->header.length);
> + }
> +}
> +
> +int acpi_create_madt_lapic(struct acpi_madt_lapic *lapic, u8 cpu, u8 apic)
> +{
> + lapic->type = 0; /* Local APIC structure */
Please use enum for the type. I mentioned this in the v2 patch.
> + lapic->length = sizeof(struct acpi_madt_lapic);
> + lapic->flags = (1 << 0); /* Processor/LAPIC enabled */
Please define this flag as a macro.
> + 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)
The indention is not correct.
> +{
> + ioapic->type = 1; /* I/O APIC structure */
Please use enum for the type.
> + 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 = 2; /* Interrupt source override */
Please use enum for the type.
> + 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)
The indention is not correct.
> +{
> + lapic_nmi->type = 4; /* Local APIC NMI structure */
Please use enum for the type.
> + 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 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. */
> + memcpy(header->signature, "APIC", 4);
> + memcpy(header->oem_id, OEM_ID, 6);
> + memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);
> + memcpy(header->asl_compiler_id, ASLC, 4);
Please create a function to fill out the common headers. Looks the
only variable here is the table signature.
> +
> + header->length = sizeof(struct acpi_madt);
> + header->revision = 1; /* ACPI 1.0/2.0: 1, ACPI 3.0: 2, ACPI 4.0: 3 */
Please use enum for the revision.
> +
> + madt->lapic_addr = LAPIC_DEFAULT_BASE;
> + madt->flags = 0x1; /* PCAT_COMPAT */
Please define this flag as a macro.
> +
> + 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);
> +}
> +
> +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);
> +}
> +
> +/* 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. */
> + memcpy(header->signature, "MCFG", 4);
> + memcpy(header->oem_id, OEM_ID, 6);
> + memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);
> + memcpy(header->asl_compiler_id, ASLC, 4);
Please create a function to fill out the common headers.
> +
> + header->length = sizeof(struct acpi_mcfg);
> + header->revision = 1;
Please use enum for the revision.
> +
> + 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 */
Please use enum for the revision.
> +}
> +
> +static void acpi_write_rsdt(struct acpi_rsdt *rsdt)
> +{
> + acpi_header_t *header = &(rsdt->header);
> +
> + /* Fill out header fields. */
> + memcpy(header->signature, "RSDT", 4);
> + memcpy(header->oem_id, OEM_ID, 6);
> + memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);
> + memcpy(header->asl_compiler_id, ASLC, 4);
Please create a function to fill out the common headers.
> +
> + header->length = sizeof(struct acpi_rsdt);
> + header->revision = 1; /* ACPI 1.0/2.0/3.0/4.0: 1 */
Please use enum for the revision.
> +
> + /* 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. */
> + memcpy(header->signature, "XSDT", 4);
> + memcpy(header->oem_id, OEM_ID, 6);
> + memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);
> + memcpy(header->asl_compiler_id, ASLC, 4);
Please create a function to fill out the common headers.
> +
> + header->length = sizeof(struct acpi_xsdt);
> + header->revision = 1; /* ACPI 1.0: N/A, 2.0/3.0/4.0: 1 */
Please use enum for the revision.
> +
> + /* 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 = 0;
> + } else {
> + rsdp->xsdt_address = (u64)(u32)xsdt;
> + rsdp->revision = 2;
> + }
> +
> + /* 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);
> + ssdt->revision = 2; /* ACPI 1.0/2.0: ?, ACPI 3.0/4.0: 2 */
Please use enum for the revision.
> + memcpy(&ssdt->oem_id, OEM_ID, 6);
> + memcpy(&ssdt->oem_table_id, oem_table_id, 8);
> + ssdt->oem_revision = 42;
Please use defines for the revision.
> + memcpy(&ssdt->asl_compiler_id, ASLC, 4);
> + ssdt->asl_compiler_revision = 42;
Please use defines for the 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);
> +}
> +
> +
> +#define ALIGN_CURRENT current = (ALIGN(current, 16))
> +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 */
> + ALIGN_CURRENT;
> +
> + 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);
> + ALIGN_CURRENT;
> + rsdt = (struct acpi_rsdt *) current;
> + current += sizeof(struct acpi_rsdt);
> + ALIGN_CURRENT;
> + xsdt = (struct acpi_xsdt *) current;
> + current += sizeof(struct acpi_xsdt);
> + ALIGN_CURRENT;
> +
> + /* 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);
> + ALIGN_CURRENT;
> +
> + 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);
> + }
> + ALIGN_CURRENT;
> +
> + debug("ACPI: * FADT\n");
> + fadt = (struct acpi_fadt *) current;
> + current += sizeof(struct acpi_fadt);
> + ALIGN_CURRENT;
> + 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;
> + ALIGN_CURRENT;
> + 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);
> + }
> + ALIGN_CURRENT;
> +
> + 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);
> + ALIGN_CURRENT;
> + }
> +
> +
Please remove the blank line.
> + debug("current = %lx\n", current);
> +
> + debug("ACPI: done.\n");
> + 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..c259683 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 $<; \
Why do we use $(CPP) here?
Can we also suppress the IASL output (see below) to the shell when
building U-Boot without 'V=1'?
Intel ACPI Component Architecture
ASL Optimizing Compiler version 20140828-64 [Sep 18 2014]
Copyright (c) 2000 - 2014 Intel Corporation
ASL Input: arch/x86/cpu/qemu/dsdt.asl.tmp - 442 lines, 25925
bytes, 342 keywords
AML Output: arch/x86/cpu/qemu/dsdt.aml - 6952 bytes, 226 named
objects, 116 executable opcodes
Hex Dump: arch/x86/cpu/qemu/dsdt.hex - 65515 bytes
> + iasl -tc -p $< -tc $<.tmp; \
There are two -tc here. Duplicated?
> + mv $(patsubst %.asl,%.hex,$<) $@
> +
> +$(obj)/%.c: $(src)/%.asl
> + $(call cmd,acpi_c_asl)
> +
> # Bzip2
> # ---------------------------------------------------------------------------
>
> --
Regards,
Bin
More information about the U-Boot
mailing list