Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33487
Change subject: acpi: Add SPMI table ......................................................................
acpi: Add SPMI table
Add the SPMI table as defined in the IPMI spec v2.
Change-Id: Idff5134ce4c124f7e76acb0080da404b0c0dfffe Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/arch/x86/acpi.c M src/arch/x86/include/arch/acpi.h 2 files changed, 93 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/33487/1
diff --git a/src/arch/x86/acpi.c b/src/arch/x86/acpi.c index bf9813c..e9ba1ec 100644 --- a/src/arch/x86/acpi.c +++ b/src/arch/x86/acpi.c @@ -772,6 +772,55 @@ header->checksum = acpi_checksum((void *)vfct, header->length); }
+void acpi_create_ipmi(struct device *device, + struct acpi_spmi *spmi, + const u16 ipmi_revision, + const acpi_addr_t *addr, + const enum acpi_ipmi_interface_type type, + const s8 gpe_interrupt, + const u32 apic_interrupt) +{ + acpi_header_t *header = &(spmi->header); + memset((void *)spmi, 0, sizeof(struct acpi_spmi)); + + /* Fill out header fields. */ + memcpy(header->signature, "SPMI", 4); + memcpy(header->oem_id, OEM_ID, 6); + memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8); + memcpy(header->asl_compiler_id, ASLC, 4); + + header->asl_compiler_revision = asl_revision; + header->length = sizeof(struct acpi_spmi); + header->revision = get_acpi_table_revision(SPMI); + + spmi->reserved = 1; + + if (device->path.type == DEVICE_PATH_PCI) { + spmi->pci_device_flag = 1; + spmi->pci_bus = device->bus->secondary; + spmi->pci_device = device->path.pci.devfn >> 3; + spmi->pci_function = device->path.pci.devfn & 0x7; + } + + spmi->base_address = *addr; + spmi->specification_revision = ipmi_revision; + + spmi->interface_type = type; + + if (gpe_interrupt >= 0 && gpe_interrupt < 32) { + spmi->gpe = gpe_interrupt; + spmi->interrupt_type |= 1; + } + if (apic_interrupt > 0) { + spmi->global_system_interrupt = apic_interrupt; + spmi->interrupt_type |= 2; + } + + /* (Re)calculate length and checksum. */ + header->length = sizeof(struct acpi_spmi); + header->checksum = acpi_checksum((void *)spmi, header->length); +} + void acpi_create_ivrs(acpi_ivrs_t *ivrs, unsigned long (*acpi_fill_ivrs)(acpi_ivrs_t *ivrs_struct, unsigned long current)) @@ -1490,6 +1539,8 @@ return 1; case SLIT: /* ACPI 2.0 upto 6.3: 1 */ return 1; + case SPMI: /* IMPI 2.0 */ + return 5; case HPET: /* Currently 1. Table added in ACPI 2.0. */ return 1; case VFCT: /* ACPI 2.0/3.0/4.0: 1 */ diff --git a/src/arch/x86/include/arch/acpi.h b/src/arch/x86/include/arch/acpi.h index dbf46a9..a864071 100644 --- a/src/arch/x86/include/arch/acpi.h +++ b/src/arch/x86/include/arch/acpi.h @@ -82,7 +82,7 @@ BERT, DBG2, DMAR, DSDT, FACS, FADT, HEST, HPET, IVRS, MADT, MCFG, RSDP, RSDT, SLIT, SRAT, SSDT, TCPA, TPM2, XSDT, ECDT, /* Additional proprietary tables used by coreboot */ - VFCT, NHLT + VFCT, NHLT, SPMI };
/* RSDP (Root System Description Pointer) */ @@ -782,6 +782,39 @@ UPC_TYPE_HUB };
+enum acpi_ipmi_interface_type { + IPMI_INTERFACE_RESERVED, + IPMI_INTERFACE_KCS, + IPMI_INTERFACE_SMIC, + IPMI_INTERFACE_BT, + IPMI_INTERFACE_SSIF, +}; + +/* ACPI IPMI 2.0 */ +struct acpi_spmi { + struct acpi_table_header header; + u8 interface_type; + u8 reserved; + u16 specification_revision; + u8 interrupt_type; + u8 gpe; + u8 reserved2; + u8 pci_device_flag; + + u32 global_system_interrupt; + acpi_addr_t base_address; + union { + struct { + u8 pci_segment_group; + u8 pci_bus; + u8 pci_device; + u8 pci_function; + }; + u8 uid[4]; + }; + u8 reserved3; +} __packed ; + unsigned long fw_cfg_acpi_tables(unsigned long start);
/* These are implemented by the target port or north/southbridge. */ @@ -834,6 +867,14 @@ struct acpi_vfct *vfct_struct, unsigned long current));
+void acpi_create_ipmi(struct device *device, + struct acpi_spmi *spmi, + const u16 ipmi_revision, + const acpi_addr_t *addr, + const enum acpi_ipmi_interface_type type, + const s8 gpe_interrupt, + const u32 apic_interrupt); + void acpi_create_ivrs(acpi_ivrs_t *ivrs, unsigned long (*acpi_fill_ivrs)(acpi_ivrs_t *ivrs_struct, unsigned long current));
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33487 )
Change subject: acpi: Add SPMI table ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/33487/1/src/arch/x86/include/arch/acpi.h File src/arch/x86/include/arch/acpi.h:
https://review.coreboot.org/#/c/33487/1/src/arch/x86/include/arch/acpi.h@816 PS1, Line 816: } __packed ; space prohibited before semicolon
Hello HAOUAS Elyes, Christian Walter, Philipp Deppenwiese, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33487
to look at the new patch set (#2).
Change subject: acpi: Add SPMI table ......................................................................
acpi: Add SPMI table
Add the SPMI table as defined in the IPMI spec v2.
Change-Id: Idff5134ce4c124f7e76acb0080da404b0c0dfffe Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/arch/x86/acpi.c M src/arch/x86/include/arch/acpi.h 2 files changed, 93 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/33487/2
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33487 )
Change subject: acpi: Add SPMI table ......................................................................
Patch Set 2: Code-Review+1
(2 comments)
https://review.coreboot.org/#/c/33487/2/src/arch/x86/acpi.c File src/arch/x86/acpi.c:
https://review.coreboot.org/#/c/33487/2/src/arch/x86/acpi.c@793 PS2, Line 793: header->length = sizeof(struct acpi_spmi); is it needed to do this twice? I don;t see why the value should change between this line and line 820
https://review.coreboot.org/#/c/33487/2/src/arch/x86/include/arch/acpi.h File src/arch/x86/include/arch/acpi.h:
https://review.coreboot.org/#/c/33487/2/src/arch/x86/include/arch/acpi.h@786 PS2, Line 786: IPMI_INTERFACE_RESERVED, maybe add an explicit = 0 here, so that it is more obvious that this will get used as raw values
Hello Felix Held, HAOUAS Elyes, Christian Walter, Philipp Deppenwiese, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33487
to look at the new patch set (#3).
Change subject: acpi: Add SPMI table ......................................................................
acpi: Add SPMI table
Add the SPMI table as defined in the IPMI spec v2.
Change-Id: Idff5134ce4c124f7e76acb0080da404b0c0dfffe Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/arch/x86/acpi.c M src/arch/x86/include/arch/acpi.h 2 files changed, 92 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/33487/3
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33487 )
Change subject: acpi: Add SPMI table ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/#/c/33487/2/src/arch/x86/acpi.c File src/arch/x86/acpi.c:
https://review.coreboot.org/#/c/33487/2/src/arch/x86/acpi.c@793 PS2, Line 793: header->length = sizeof(struct acpi_spmi);
is it needed to do this twice? I don;t see why the value should change between this line and line 82 […]
Done
https://review.coreboot.org/#/c/33487/2/src/arch/x86/include/arch/acpi.h File src/arch/x86/include/arch/acpi.h:
https://review.coreboot.org/#/c/33487/2/src/arch/x86/include/arch/acpi.h@786 PS2, Line 786: IPMI_INTERFACE_RESERVED,
maybe add an explicit = 0 here, so that it is more obvious that this will get used as raw values
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33487 )
Change subject: acpi: Add SPMI table ......................................................................
Patch Set 3: Code-Review+2
Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33487 )
Change subject: acpi: Add SPMI table ......................................................................
Patch Set 3:
(1 comment)
Mention SPMI spec link https://www.intel.com/content/dam/www/public/us/en/documents/product-briefs/... will be better.
https://review.coreboot.org/#/c/33487/3/src/arch/x86/acpi.c File src/arch/x86/acpi.c:
https://review.coreboot.org/#/c/33487/3/src/arch/x86/acpi.c@812 PS3, Line 812: spmi->interrupt_type |= 1 Instead of |=, just use "=" directly? Instead of using value 1 and 2 directly, may be replace with 1 <<0 and 1<<1 or some meaningful definition in acpi.h to make it more clear?
enum acpi_ipmi_interrupt_type { IPMI_INTERRUPT_GPE = 1, IPMI_INTERRUPT_APIC = 2 };
Hello Felix Held, HAOUAS Elyes, Christian Walter, Philipp Deppenwiese, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33487
to look at the new patch set (#4).
Change subject: acpi: Add SPMI table ......................................................................
acpi: Add SPMI table
Add the SPMI table as defined in the IPMI spec v2.
Change-Id: Idff5134ce4c124f7e76acb0080da404b0c0dfffe Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/arch/x86/acpi.c M src/arch/x86/include/arch/acpi.h 2 files changed, 100 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/33487/4
Hello Felix Held, HAOUAS Elyes, Christian Walter, Philipp Deppenwiese, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33487
to look at the new patch set (#6).
Change subject: acpi: Add SPMI table ......................................................................
acpi: Add SPMI table
Add the SPMI table as defined in the IPMI spec v2: https://www.intel.com/content/dam/www/public/us/en/documents/product-briefs/...
Tested on Wedge100s.
Change-Id: Idff5134ce4c124f7e76acb0080da404b0c0dfffe Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/arch/x86/acpi.c M src/arch/x86/include/arch/acpi.h 2 files changed, 100 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/33487/6
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33487 )
Change subject: acpi: Add SPMI table ......................................................................
Patch Set 6:
(1 comment)
Patch Set 3:
(1 comment)
Mention SPMI spec link https://www.intel.com/content/dam/www/public/us/en/documents/product-briefs/... will be better.
Done.
https://review.coreboot.org/#/c/33487/3/src/arch/x86/acpi.c File src/arch/x86/acpi.c:
https://review.coreboot.org/#/c/33487/3/src/arch/x86/acpi.c@812 PS3, Line 812: spmi->interrupt_type |= 1
Instead of |=, just use "=" directly? Instead of using value 1 and 2 directly, may be replace with 1 […]
added defines for interrupts and PCI device flag
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33487 )
Change subject: acpi: Add SPMI table ......................................................................
Patch Set 6: Code-Review+2
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33487 )
Change subject: acpi: Add SPMI table ......................................................................
Patch Set 6: Code-Review+1
Patrick Rudolph has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33487 )
Change subject: acpi: Add SPMI table ......................................................................
acpi: Add SPMI table
Add the SPMI table as defined in the IPMI spec v2: https://www.intel.com/content/dam/www/public/us/en/documents/product-briefs/...
Tested on Wedge100s.
Change-Id: Idff5134ce4c124f7e76acb0080da404b0c0dfffe Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/33487 Reviewed-by: Felix Held felix-coreboot@felixheld.de Reviewed-by: HAOUAS Elyes ehaouas@noos.fr Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/arch/x86/acpi.c M src/arch/x86/include/arch/acpi.h 2 files changed, 100 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved HAOUAS Elyes: Looks good to me, but someone else must approve
diff --git a/src/arch/x86/acpi.c b/src/arch/x86/acpi.c index bf9813c..1b8aaae 100644 --- a/src/arch/x86/acpi.c +++ b/src/arch/x86/acpi.c @@ -772,6 +772,57 @@ header->checksum = acpi_checksum((void *)vfct, header->length); }
+void acpi_create_ipmi(struct device *device, + struct acpi_spmi *spmi, + const u16 ipmi_revision, + const acpi_addr_t *addr, + const enum acpi_ipmi_interface_type type, + const s8 gpe_interrupt, + const u32 apic_interrupt, + const u32 uid) +{ + acpi_header_t *header = &(spmi->header); + memset((void *)spmi, 0, sizeof(struct acpi_spmi)); + + /* Fill out header fields. */ + memcpy(header->signature, "SPMI", 4); + memcpy(header->oem_id, OEM_ID, 6); + memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8); + memcpy(header->asl_compiler_id, ASLC, 4); + + header->asl_compiler_revision = asl_revision; + header->length = sizeof(struct acpi_spmi); + header->revision = get_acpi_table_revision(SPMI); + + spmi->reserved = 1; + + if (device->path.type == DEVICE_PATH_PCI) { + spmi->pci_device_flag = ACPI_IPMI_PCI_DEVICE_FLAG; + spmi->pci_bus = device->bus->secondary; + spmi->pci_device = device->path.pci.devfn >> 3; + spmi->pci_function = device->path.pci.devfn & 0x7; + } else if (type != IPMI_INTERFACE_SSIF) { + memcpy(spmi->uid, &uid, sizeof(spmi->uid)); + } + + spmi->base_address = *addr; + spmi->specification_revision = ipmi_revision; + + spmi->interface_type = type; + + if (gpe_interrupt >= 0 && gpe_interrupt < 32) { + spmi->gpe = gpe_interrupt; + spmi->interrupt_type |= ACPI_IPMI_INT_TYPE_SCI; + } + if (apic_interrupt > 0) { + spmi->global_system_interrupt = apic_interrupt; + spmi->interrupt_type |= ACPI_IPMI_INT_TYPE_APIC; + } + + /* Calculate checksum. */ + header->checksum = acpi_checksum((void *)spmi, header->length); +} + void acpi_create_ivrs(acpi_ivrs_t *ivrs, unsigned long (*acpi_fill_ivrs)(acpi_ivrs_t *ivrs_struct, unsigned long current)) @@ -1490,6 +1541,8 @@ return 1; case SLIT: /* ACPI 2.0 upto 6.3: 1 */ return 1; + case SPMI: /* IMPI 2.0 */ + return 5; case HPET: /* Currently 1. Table added in ACPI 2.0. */ return 1; case VFCT: /* ACPI 2.0/3.0/4.0: 1 */ diff --git a/src/arch/x86/include/arch/acpi.h b/src/arch/x86/include/arch/acpi.h index dbf46a9..e48b2da 100644 --- a/src/arch/x86/include/arch/acpi.h +++ b/src/arch/x86/include/arch/acpi.h @@ -82,7 +82,7 @@ BERT, DBG2, DMAR, DSDT, FACS, FADT, HEST, HPET, IVRS, MADT, MCFG, RSDP, RSDT, SLIT, SRAT, SSDT, TCPA, TPM2, XSDT, ECDT, /* Additional proprietary tables used by coreboot */ - VFCT, NHLT + VFCT, NHLT, SPMI };
/* RSDP (Root System Description Pointer) */ @@ -782,6 +782,43 @@ UPC_TYPE_HUB };
+enum acpi_ipmi_interface_type { + IPMI_INTERFACE_RESERVED = 0, + IPMI_INTERFACE_KCS, + IPMI_INTERFACE_SMIC, + IPMI_INTERFACE_BT, + IPMI_INTERFACE_SSIF, +}; + +#define ACPI_IPMI_PCI_DEVICE_FLAG (1 << 0) +#define ACPI_IPMI_INT_TYPE_SCI (1 << 0) +#define ACPI_IPMI_INT_TYPE_APIC (1 << 1) + +/* ACPI IPMI 2.0 */ +struct acpi_spmi { + struct acpi_table_header header; + u8 interface_type; + u8 reserved; + u16 specification_revision; + u8 interrupt_type; + u8 gpe; + u8 reserved2; + u8 pci_device_flag; + + u32 global_system_interrupt; + acpi_addr_t base_address; + union { + struct { + u8 pci_segment_group; + u8 pci_bus; + u8 pci_device; + u8 pci_function; + }; + u8 uid[4]; + }; + u8 reserved3; +} __packed; + unsigned long fw_cfg_acpi_tables(unsigned long start);
/* These are implemented by the target port or north/southbridge. */ @@ -834,6 +871,15 @@ struct acpi_vfct *vfct_struct, unsigned long current));
+void acpi_create_ipmi(struct device *device, + struct acpi_spmi *spmi, + const u16 ipmi_revision, + const acpi_addr_t *addr, + const enum acpi_ipmi_interface_type type, + const s8 gpe_interrupt, + const u32 apic_interrupt, + const u32 uid); + void acpi_create_ivrs(acpi_ivrs_t *ivrs, unsigned long (*acpi_fill_ivrs)(acpi_ivrs_t *ivrs_struct, unsigned long current));
-
build bot (Jenkins) (Code Review) -
Felix Held (Code Review)
-
HAOUAS Elyes (Code Review)
-
Lijian Zhao (Code Review)
-
Patrick Rudolph (Code Review)