Marc Jones has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45845 )
Change subject: soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt() ......................................................................
soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt()
Prepare for common APCI code. Make acpi_fill_fadt() match intel/commom/block/acpi/acpi.c function. Use soc_acpi_fill_fadt() to set cpx fadt->flags.
Change-Id: I9c04dd478aa5e0f1467e63d06da094128edd9650 Signed-off-by: Marc Jones marcjones@sysproconsulting.com --- M src/soc/intel/xeon_sp/cpx/acpi.c M src/soc/intel/xeon_sp/cpx/soc_acpi.c 2 files changed, 25 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/45845/1
diff --git a/src/soc/intel/xeon_sp/cpx/acpi.c b/src/soc/intel/xeon_sp/cpx/acpi.c index b9e261e..93cd8ed 100644 --- a/src/soc/intel/xeon_sp/cpx/acpi.c +++ b/src/soc/intel/xeon_sp/cpx/acpi.c @@ -129,13 +129,18 @@
void acpi_fill_fadt(acpi_fadt_t *fadt) { - const uint16_t pmbase = ACPI_BASE_ADDRESS;
fadt->header.revision = get_acpi_table_revision(FADT);
fadt->sci_int = acpi_sci_irq();
+ if (permanent_smi_handler()) { + fadt->smi_cmd = APM_CNT; + fadt->acpi_enable = APM_CNT_ACPI_ENABLE; + fadt->acpi_disable = APM_CNT_ACPI_DISABLE; + } + fadt->pm1a_evt_blk = pmbase + PM1_STS; fadt->pm1a_cnt_blk = pmbase + PM1_CNT;
@@ -150,26 +155,29 @@ fadt->duty_offset = 1; fadt->day_alrm = 0xd;
- fadt->flags |= ACPI_FADT_WBINVD | ACPI_FADT_C1_SUPPORTED | ACPI_FADT_C2_MP_SUPPORTED | + fadt->flags |= ACPI_FADT_WBINVD | ACPI_FADT_C1_SUPPORTED | + ACPI_FADT_C2_MP_SUPPORTED | ACPI_FADT_SLEEP_BUTTON | + ACPI_FADT_SEALED_CASE | ACPI_FADT_S4_RTC_WAKE | ACPI_FADT_PLATFORM_CLOCK;
fadt->x_pm1a_evt_blk.space_id = ACPI_ADDRESS_SPACE_IO; fadt->x_pm1a_evt_blk.bit_width = fadt->pm1_evt_len * 8; fadt->x_pm1a_evt_blk.addrl = pmbase + PM1_STS; + fadt->x_pm1a_evt_blk.access_size = ACPI_ACCESS_SIZE_WORD_ACCESS;
fadt->x_pm1a_cnt_blk.space_id = ACPI_ADDRESS_SPACE_IO; fadt->x_pm1a_cnt_blk.bit_width = fadt->pm1_cnt_len * 8; fadt->x_pm1a_cnt_blk.addrl = pmbase + PM1_CNT; + fadt->x_pm1a_cnt_blk.access_size = ACPI_ACCESS_SIZE_WORD_ACCESS;
- if (permanent_smi_handler()) { - fadt->smi_cmd = APM_CNT; - fadt->acpi_enable = APM_CNT_ACPI_ENABLE; - fadt->acpi_disable = APM_CNT_ACPI_DISABLE; - } - - /* General-Purpose Event Registers */ + /* + * Windows 10 requires x_gpe0_blk to be set starting with FADT revision 5. + * The bit_width field intentionally overflows here. + * The OSPM can instead use the values in `fadt->gpe0_blk{,_len}`, which + * seems to work fine on Linux 5.0 and Windows 10. + */ fadt->x_gpe0_blk.space_id = ACPI_ADDRESS_SPACE_IO; - fadt->x_gpe0_blk.bit_width = 64; /* EventStatus + EventEnable */ + fadt->x_gpe0_blk.bit_width = fadt->gpe0_blk_len * 8; fadt->x_gpe0_blk.bit_offset = 0; fadt->x_gpe0_blk.access_size = ACPI_ACCESS_SIZE_BYTE_ACCESS; fadt->x_gpe0_blk.addrl = fadt->gpe0_blk; diff --git a/src/soc/intel/xeon_sp/cpx/soc_acpi.c b/src/soc/intel/xeon_sp/cpx/soc_acpi.c index 6bf38f5..1aa1192 100644 --- a/src/soc/intel/xeon_sp/cpx/soc_acpi.c +++ b/src/soc/intel/xeon_sp/cpx/soc_acpi.c @@ -52,6 +52,13 @@ return pci_read_config32(dev, PMC_ACPI_CNT); }
+void soc_fill_fadt(acpi_fadt_t *fadt) +{ + /* Clear flags set by common/block/acpi/acpi.c acpi_fill_fadt() */ + fadt->flags &= ~(ACPI_FADT_SLEEP_BUTTON | ACPI_FADT_SEALED_CASE | + ACPI_FADT_S4_RTC_WAKE); +} + /* * Currently called in southbridge_inject_dsdt(). Change to soc_southbridge_inject_dsdt() * with a call from the common/function or find another way to call this at the correct place
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45845 )
Change subject: soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt() ......................................................................
Patch Set 3: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45845 )
Change subject: soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt() ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45845/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45845/4//COMMIT_MSG@9 PS4, Line 9: APCI ACPI
Hello build bot (Jenkins), Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45845
to look at the new patch set (#5).
Change subject: soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt() ......................................................................
soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt()
Prepare for common ACPI code. Make acpi_fill_fadt() match intel/commom/block/acpi/acpi.c function. Use soc_acpi_fill_fadt() to set cpx fadt->flags.
Change-Id: I9c04dd478aa5e0f1467e63d06da094128edd9650 Signed-off-by: Marc Jones marcjones@sysproconsulting.com --- M src/soc/intel/xeon_sp/cpx/acpi.c M src/soc/intel/xeon_sp/cpx/soc_acpi.c 2 files changed, 25 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/45845/5
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45845 )
Change subject: soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt() ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45845/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45845/4//COMMIT_MSG@9 PS4, Line 9: APCI
ACPI
Done
Jay Talbott has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45845 )
Change subject: soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt() ......................................................................
Patch Set 8: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45845 )
Change subject: soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt() ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45845/8/src/soc/intel/xeon_sp/cpx/a... File src/soc/intel/xeon_sp/cpx/acpi.c:
https://review.coreboot.org/c/coreboot/+/45845/8/src/soc/intel/xeon_sp/cpx/a... PS8, Line 161: ACPI_FADT_SEALED_CASE I don't think we want to set this
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45845 )
Change subject: soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt() ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45845/8/src/soc/intel/xeon_sp/cpx/a... File src/soc/intel/xeon_sp/cpx/acpi.c:
https://review.coreboot.org/c/coreboot/+/45845/8/src/soc/intel/xeon_sp/cpx/a... PS8, Line 161: ACPI_FADT_SEALED_CASE
I don't think we want to set this
This is set to match intel/commom/block/acpi/acpi.c. Use soc_acpi_fill_fadt() to set cpx fadt->flags for the platform. We are getting close to removing this file.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45845 )
Change subject: soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt() ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45845/8/src/soc/intel/xeon_sp/cpx/a... File src/soc/intel/xeon_sp/cpx/acpi.c:
https://review.coreboot.org/c/coreboot/+/45845/8/src/soc/intel/xeon_sp/cpx/a... PS8, Line 161: ACPI_FADT_SEALED_CASE
This is set to match intel/commom/block/acpi/acpi.c. Use soc_acpi_fill_fadt() […]
Ack
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45845 )
Change subject: soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt() ......................................................................
Patch Set 10: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/45845/10/src/soc/intel/xeon_sp/cpx/... File src/soc/intel/xeon_sp/cpx/acpi.c:
https://review.coreboot.org/c/coreboot/+/45845/10/src/soc/intel/xeon_sp/cpx/... PS10, Line 176: The bit_width field intentionally overflows here. That's a bold claim, but what is the intention?
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45845 )
Change subject: soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt() ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45845/10/src/soc/intel/xeon_sp/cpx/... File src/soc/intel/xeon_sp/cpx/acpi.c:
https://review.coreboot.org/c/coreboot/+/45845/10/src/soc/intel/xeon_sp/cpx/... PS10, Line 176: The bit_width field intentionally overflows here.
That's a bold claim, but what is the intention?
It is from CB:39805
Marc Jones has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45845 )
Change subject: soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt() ......................................................................
soc/intel/xeon_sp/cpx: Add soc_acpi_fill_fadt()
Prepare for common ACPI code. Make acpi_fill_fadt() match intel/commom/block/acpi/acpi.c function. Use soc_acpi_fill_fadt() to set cpx fadt->flags.
Change-Id: I9c04dd478aa5e0f1467e63d06da094128edd9650 Signed-off-by: Marc Jones marcjones@sysproconsulting.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/45845 Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org Reviewed-by: Jay Talbott JayTalbott@sysproconsulting.com Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/xeon_sp/cpx/acpi.c M src/soc/intel/xeon_sp/cpx/soc_acpi.c 2 files changed, 25 insertions(+), 10 deletions(-)
Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve Jay Talbott: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/xeon_sp/cpx/acpi.c b/src/soc/intel/xeon_sp/cpx/acpi.c index a1ebc4a..8d3e86a 100644 --- a/src/soc/intel/xeon_sp/cpx/acpi.c +++ b/src/soc/intel/xeon_sp/cpx/acpi.c @@ -130,13 +130,18 @@
void acpi_fill_fadt(acpi_fadt_t *fadt) { - const uint16_t pmbase = ACPI_BASE_ADDRESS;
fadt->header.revision = get_acpi_table_revision(FADT);
fadt->sci_int = acpi_sci_irq();
+ if (permanent_smi_handler()) { + fadt->smi_cmd = APM_CNT; + fadt->acpi_enable = APM_CNT_ACPI_ENABLE; + fadt->acpi_disable = APM_CNT_ACPI_DISABLE; + } + fadt->pm1a_evt_blk = pmbase + PM1_STS; fadt->pm1a_cnt_blk = pmbase + PM1_CNT;
@@ -151,26 +156,29 @@ fadt->duty_offset = 1; fadt->day_alrm = 0xd;
- fadt->flags |= ACPI_FADT_WBINVD | ACPI_FADT_C1_SUPPORTED | ACPI_FADT_C2_MP_SUPPORTED | + fadt->flags |= ACPI_FADT_WBINVD | ACPI_FADT_C1_SUPPORTED | + ACPI_FADT_C2_MP_SUPPORTED | ACPI_FADT_SLEEP_BUTTON | + ACPI_FADT_SEALED_CASE | ACPI_FADT_S4_RTC_WAKE | ACPI_FADT_PLATFORM_CLOCK;
fadt->x_pm1a_evt_blk.space_id = ACPI_ADDRESS_SPACE_IO; fadt->x_pm1a_evt_blk.bit_width = fadt->pm1_evt_len * 8; fadt->x_pm1a_evt_blk.addrl = pmbase + PM1_STS; + fadt->x_pm1a_evt_blk.access_size = ACPI_ACCESS_SIZE_WORD_ACCESS;
fadt->x_pm1a_cnt_blk.space_id = ACPI_ADDRESS_SPACE_IO; fadt->x_pm1a_cnt_blk.bit_width = fadt->pm1_cnt_len * 8; fadt->x_pm1a_cnt_blk.addrl = pmbase + PM1_CNT; + fadt->x_pm1a_cnt_blk.access_size = ACPI_ACCESS_SIZE_WORD_ACCESS;
- if (permanent_smi_handler()) { - fadt->smi_cmd = APM_CNT; - fadt->acpi_enable = APM_CNT_ACPI_ENABLE; - fadt->acpi_disable = APM_CNT_ACPI_DISABLE; - } - - /* General-Purpose Event Registers */ + /* + * Windows 10 requires x_gpe0_blk to be set starting with FADT revision 5. + * The bit_width field intentionally overflows here. + * The OSPM can instead use the values in `fadt->gpe0_blk{,_len}`, which + * seems to work fine on Linux 5.0 and Windows 10. + */ fadt->x_gpe0_blk.space_id = ACPI_ADDRESS_SPACE_IO; - fadt->x_gpe0_blk.bit_width = 64; /* EventStatus + EventEnable */ + fadt->x_gpe0_blk.bit_width = fadt->gpe0_blk_len * 8; fadt->x_gpe0_blk.bit_offset = 0; fadt->x_gpe0_blk.access_size = ACPI_ACCESS_SIZE_BYTE_ACCESS; fadt->x_gpe0_blk.addrl = fadt->gpe0_blk; diff --git a/src/soc/intel/xeon_sp/cpx/soc_acpi.c b/src/soc/intel/xeon_sp/cpx/soc_acpi.c index db1d3d2..d0b82e5 100644 --- a/src/soc/intel/xeon_sp/cpx/soc_acpi.c +++ b/src/soc/intel/xeon_sp/cpx/soc_acpi.c @@ -52,6 +52,13 @@ return pci_read_config32(dev, PMC_ACPI_CNT); }
+void soc_fill_fadt(acpi_fadt_t *fadt) +{ + /* Clear flags set by common/block/acpi/acpi.c acpi_fill_fadt() */ + fadt->flags &= ~(ACPI_FADT_SLEEP_BUTTON | ACPI_FADT_SEALED_CASE | + ACPI_FADT_S4_RTC_WAKE); +} + /* * Currently called in southbridge_inject_dsdt(). Change to soc_southbridge_inject_dsdt() * with a call from the common/function or find another way to call this at the correct place
-
Angel Pons (Code Review) -
Jay Talbott (Code Review)
-
Marc Jones (Code Review)
-
Stefan Reinauer (Code Review)