Change in ...coreboot[master]: ACPI: Correct acpi signatures tables

List overview All Threads
Download

newer

older

Change in ...coreboot[master]:...

HAOUAS Elyes (Code Review)

23 Feb 2019 23 Feb '19

9:16 a.m.

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31592

Change subject: ACPI: Correct acpi signatures tables ......................................................................

ACPI: Correct acpi signatures tables

Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/arch/x86/include/arch/acpi.h 1 file changed, 10 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/31592/1

diff --git a/src/arch/x86/include/arch/acpi.h b/src/arch/x86/include/arch/acpi.h index a061a27..6f259a4 100644 --- a/src/arch/x86/include/arch/acpi.h +++ b/src/arch/x86/include/arch/acpi.h @@ -77,16 +77,18 @@ COREBOOT_ACPI_ID_MAX = 0xFFFF, /* BOOTFFFF */ };

-/* Table 5-30 DESCRIPTION_HEADER Signatures for tables defined by ACPI 6.2a - * Additional tables mssing in 5-30: MADT, RSDP, VFCT, NHLT +/* Table 5-29 and 5-30 DESCRIPTION_HEADER Signatures for tables + * defined/reserved by ACPI 6.3 */ enum acpi_tables { - APIC, BERT, BGRT, CPEP, DSDT, ECDT, EINJ, ERST, FACP, FADT, FACS, - FPDT, GTDT, HEST, MSCT, MPST, NFIT, OEMX, PCCT, PMTT, PSDT, RASF, - RSDT, SBST, SDEV, SLIT, SRAT, SSDT, XSDT, BOOT, CSRT, DBG2, DBGP, - DMAR, DPPT, DRTM, ETDT, HPET, IBFT, IORT, IVRS, LPIT, MCFG, MCHI, - MSDM, SDEI, SLIC, SPCR, SPMI, STAO, TCPA, TPM2, WAET, WDAT, WDRT, - WPBT, WSMT, XENV, MADT, RSDP, VFCT, NHLT + /* Table 5-29 Signatures for tables defined by ACPI */ + APIC, BERT, BGRT, CPEP, DSDT, ECDT, EINJ, ERST, FACP, FACS, FPDT, + GTDT, HEST, MSCT, MPST, NFIT, OEMX, PCCT, PMTT, PSDT, RASF, RSDT, + SBST, SDEV, SLIT, SRAT, SSDT, XSDT, + /* Table 5-30 Signatures for tables reserved by ACPI */ + BOOT, CDIT, CRAT, CSRT, DBG2, DBGP, DMAR, DPPT, DRTM, HPET, IBFT, + IORT, IVRS, LPIT, MCFG, MCHI, MSDM, SDEI, SLIC, SPCR, SPMI, STAO, + TCPA, TPM2, UEFI, WAET, WDAT, WDRT, WPBT, WSMT, XENV };

/* RSDP (Root System Description Pointer) */

Attachments:

attachment.htm (text/html — 4.1 KB)

Show replies by date

HAOUAS Elyes (Code Review)

23 Feb 23 Feb

9:19 a.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31592 )

Change subject: ACPI: Correct acpi signatures tables ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/31592/1/src/arch/x86/include/arch/acpi.h File src/arch/x86/include/arch/acpi.h:

https://review.coreboot.org/#/c/31592/1/src/arch/x86/include/arch/acpi.h@a87 PS1, Line 87: "superseded by “HPET” and is now obsolete"

-- To view, visit https://review.coreboot.org/c/coreboot/+/31592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Gerrit-Change-Number: 31592 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Comment-Date: Sat, 23 Feb 2019 09:19:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

10:47 a.m.

New subject: Change in ...coreboot[master]: ACPI: Correct acpi_tables

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31592 )

Change subject: ACPI: Correct acpi_tables ......................................................................

Set Ready For Review

-- To view, visit https://review.coreboot.org/c/coreboot/+/31592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Gerrit-Change-Number: 31592 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 23 Feb 2019 10:47:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

10:49 a.m.

New subject: Change in ...coreboot[master]: ACPI: Correct acpi_tables

Hello build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31592

to look at the new patch set (#3).

Change subject: ACPI: Correct acpi_tables ......................................................................

ACPI: Correct acpi_tables

Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/arch/x86/include/arch/acpi.h 1 file changed, 10 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/31592/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/31592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Gerrit-Change-Number: 31592 Gerrit-PatchSet: 3 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Nico Huber (Code Review)

11:35 a.m.

New subject: Change in ...coreboot[master]: ACPI: Correct acpi_tables

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31592 )

Change subject: ACPI: Correct acpi_tables ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/31592/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/#/c/31592/3//COMMIT_MSG@7 PS3, Line 7: ACPI: Correct acpi_tables This basically says nothing. "Correct" to what meaning?

First, we have to decide what should be in this enum. If we don't define it exactly, anybody could push a commit that "corrects" it for any reason.

My personal preference would be "what we use". That'd currently only be tables that get_acpi_table_revision() knows about.

Alternatively, we could make it everything that was ever specified as table by ACPI, plus proprietary tables that coreboot provides too. I don't like dropping obso- lete tables as that makes it even more confusing (and increases patch noise). I think this might be the ori- ginally intended definition.

The former would be much clearer and easier to main- tain, IMHO. I'm open for other definitions as well, as long as it is reasonably maintainable. And doesn't create more noise (e.g. gets fixed just for the sake of its useless definition).

-- To view, visit https://review.coreboot.org/c/coreboot/+/31592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Gerrit-Change-Number: 31592 Gerrit-PatchSet: 3 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 23 Feb 2019 11:35:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

12:29 p.m.

New subject: Change in ...coreboot[master]: ACPI: Enum only used ACPI tables

Hello Patrick Rudolph, build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31592

to look at the new patch set (#4).

Change subject: ACPI: Enum only used ACPI tables ......................................................................

ACPI: Enum only used ACPI tables

Not used defined ACPI tables are commented.

Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/arch/x86/include/arch/acpi.h 1 file changed, 18 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/31592/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/31592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Gerrit-Change-Number: 31592 Gerrit-PatchSet: 4 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

build bot (Jenkins) (Code Review)

12:30 p.m.

New subject: Change in ...coreboot[master]: ACPI: Enum only used ACPI tables

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31592 )

Change subject: ACPI: Enum only used ACPI tables ......................................................................

Patch Set 4:

(3 comments)

https://review.coreboot.org/#/c/31592/4/src/arch/x86/include/arch/acpi.h File src/arch/x86/include/arch/acpi.h:

https://review.coreboot.org/#/c/31592/4/src/arch/x86/include/arch/acpi.h@87 PS4, Line 87: * BGRT, BOOT, CDIT, CPEP, CRAT, CSRT, DBGP, DPPT, DRTM, EINJ, ERST, FPDT, line over 80 characters

https://review.coreboot.org/#/c/31592/4/src/arch/x86/include/arch/acpi.h@88 PS4, Line 88: * GTDT, IBFT, IORT, LPIT, MCHI, MPST, MSCT, MSDM, NFIT, OEMX, PCCT, PMTT, line over 80 characters

https://review.coreboot.org/#/c/31592/4/src/arch/x86/include/arch/acpi.h@89 PS4, Line 89: * RASF, SBST, SDEI, SDEV, SLIC, SPCR, SPMI, STAO, UEFI, WAET, WDAT, WDRT, line over 80 characters

-- To view, visit https://review.coreboot.org/c/coreboot/+/31592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Gerrit-Change-Number: 31592 Gerrit-PatchSet: 4 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 23 Feb 2019 12:30:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Nico Huber (Code Review)

12:59 p.m.

New subject: Change in ...coreboot[master]: ACPI: Enum only used ACPI tables

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31592 )

Change subject: ACPI: Enum only used ACPI tables ......................................................................

Patch Set 4: Code-Review+1

(2 comments)

https://review.coreboot.org/#/c/31592/4/src/arch/x86/include/arch/acpi.h File src/arch/x86/include/arch/acpi.h:

https://review.coreboot.org/#/c/31592/4/src/arch/x86/include/arch/acpi.h@80 PS4, Line 80: /* ACPI Tables defined/reserved by ACPI 6.3 */ Drop/update this?

https://review.coreboot.org/#/c/31592/4/src/arch/x86/include/arch/acpi.h@95 PS4, Line 95: */ Maybe get rid of the commented table names? Otherwise these comments could get out of sync, or worse, some- body could try to keep them sync'ed.

-- To view, visit https://review.coreboot.org/c/coreboot/+/31592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Gerrit-Change-Number: 31592 Gerrit-PatchSet: 4 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 23 Feb 2019 12:59:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

1:07 p.m.

New subject: Change in ...coreboot[master]: ACPI: Enum only used ACPI tables

Hello Patrick Rudolph, build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31592

to look at the new patch set (#5).

Change subject: ACPI: Enum only used ACPI tables ......................................................................

ACPI: Enum only used ACPI tables

Not used defined ACPI tables are commented.

Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/arch/x86/include/arch/acpi.h 1 file changed, 5 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/31592/5

-- To view, visit https://review.coreboot.org/c/coreboot/+/31592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Gerrit-Change-Number: 31592 Gerrit-PatchSet: 5 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Nico Huber (Code Review)

2:41 p.m.

New subject: Change in ...coreboot[master]: ACPI: Enum only used ACPI tables

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31592 )

Change subject: ACPI: Enum only used ACPI tables ......................................................................

Patch Set 5: Code-Review+2

(1 comment)

https://review.coreboot.org/#/c/31592/5//COMMIT_MSG Commit Message:

https://review.coreboot.org/#/c/31592/5//COMMIT_MSG@9 PS5, Line 9: Not used defined ACPI tables are commented. Update, please.

Maybe mention a reasoning, e.g. enum contained redundant names and wasn't exhaustive anyway.

-- To view, visit https://review.coreboot.org/c/coreboot/+/31592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Gerrit-Change-Number: 31592 Gerrit-PatchSet: 5 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 23 Feb 2019 14:41:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

4:43 p.m.

New subject: Change in ...coreboot[master]: ACPI: Enum only used ACPI tables

Hello Patrick Rudolph, build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31592

to look at the new patch set (#6).

Change subject: ACPI: Enum only used ACPI tables ......................................................................

ACPI: Enum only used ACPI tables

enum contained redundant names and wasn't exhaustive anyway.

Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/arch/x86/include/arch/acpi.h 1 file changed, 5 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/31592/6

-- To view, visit https://review.coreboot.org/c/coreboot/+/31592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Gerrit-Change-Number: 31592 Gerrit-PatchSet: 6 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Patrick Georgi (Code Review)

25 Feb 25 Feb

11:20 a.m.

New subject: Change in ...coreboot[master]: ACPI: Enum only used ACPI tables

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31592 )

Change subject: ACPI: Enum only used ACPI tables ......................................................................

ACPI: Enum only used ACPI tables

enum contained redundant names and wasn't exhaustive anyway.

Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Signed-off-by: Elyes HAOUAS ehaouas@noos.fr Reviewed-on: https://review.coreboot.org/c/31592 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/arch/x86/include/arch/acpi.h 1 file changed, 5 insertions(+), 9 deletions(-)

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved

diff --git a/src/arch/x86/include/arch/acpi.h b/src/arch/x86/include/arch/acpi.h index a061a27..cc0f1a9 100644 --- a/src/arch/x86/include/arch/acpi.h +++ b/src/arch/x86/include/arch/acpi.h @@ -77,16 +77,12 @@ COREBOOT_ACPI_ID_MAX = 0xFFFF, /* BOOTFFFF */ };

-/* Table 5-30 DESCRIPTION_HEADER Signatures for tables defined by ACPI 6.2a - * Additional tables mssing in 5-30: MADT, RSDP, VFCT, NHLT - */ enum acpi_tables { - APIC, BERT, BGRT, CPEP, DSDT, ECDT, EINJ, ERST, FACP, FADT, FACS, - FPDT, GTDT, HEST, MSCT, MPST, NFIT, OEMX, PCCT, PMTT, PSDT, RASF, - RSDT, SBST, SDEV, SLIT, SRAT, SSDT, XSDT, BOOT, CSRT, DBG2, DBGP, - DMAR, DPPT, DRTM, ETDT, HPET, IBFT, IORT, IVRS, LPIT, MCFG, MCHI, - MSDM, SDEI, SLIC, SPCR, SPMI, STAO, TCPA, TPM2, WAET, WDAT, WDRT, - WPBT, WSMT, XENV, MADT, RSDP, VFCT, NHLT + /* Tables defined by ACPI and used by coreboot */ + 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 };

/* RSDP (Root System Description Pointer) */

-- To view, visit https://review.coreboot.org/c/coreboot/+/31592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d74ff61c555c5953932efbd7edccfd3157cb5be Gerrit-Change-Number: 31592 Gerrit-PatchSet: 7 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: merged

2104

days inactive

2106

days old

coreboot-gerrit@coreboot.org

11 comments

4 participants

tags (0)

participants (4)

build bot (Jenkins) (Code Review)
HAOUAS Elyes (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)