Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42864 )
Change subject: sb/intel: Factor out soc_fill_gnvs() ......................................................................
sb/intel: Factor out soc_fill_gnvs()
Name the common part of GNVS initialisation as soc_fill_gnvs(). It is also moved before the call to acpi_create_gnvs(), which followup will rename to mainbord_fill_gnvs() to reflect that implementation is under mb/.
Change-Id: Ic4cf1548b65a86212d6e45d460fcd23bb8036365 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/acpi/gnvs.c M src/include/acpi/acpi_gnvs.h M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/i82801gx/lpc.c M src/southbridge/intel/i82801ix/lpc.c M src/southbridge/intel/i82801jx/lpc.c M src/southbridge/intel/ibexpeak/lpc.c M src/southbridge/intel/lynxpoint/lpc.c 8 files changed, 58 insertions(+), 81 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/42864/1
diff --git a/src/acpi/gnvs.c b/src/acpi/gnvs.c index 40c9745..23b69a1 100644 --- a/src/acpi/gnvs.c +++ b/src/acpi/gnvs.c @@ -29,6 +29,8 @@ *gnvs_cbmc = (uintptr_t)cbmem_find(CBMEM_ID_CONSOLE); }
+__weak void soc_fill_gnvs(struct global_nvs *gnvs) { } + void *gnvs_get_or_create(void) { size_t gnvs_size; diff --git a/src/include/acpi/acpi_gnvs.h b/src/include/acpi/acpi_gnvs.h index 6173fa1..d827a07 100644 --- a/src/include/acpi/acpi_gnvs.h +++ b/src/include/acpi/acpi_gnvs.h @@ -33,4 +33,6 @@ void acpi_create_gnvs(struct global_nvs *gnvs); void acpi_init_gnvs(struct global_nvs *gnvs);
+void soc_fill_gnvs(struct global_nvs *gnvs); + #endif diff --git a/src/southbridge/intel/bd82x6x/lpc.c b/src/southbridge/intel/bd82x6x/lpc.c index 3f142db..d5077de 100644 --- a/src/southbridge/intel/bd82x6x/lpc.c +++ b/src/southbridge/intel/bd82x6x/lpc.c @@ -15,7 +15,6 @@ #include <acpi/acpi_gnvs.h> #include <acpi/acpigen.h> #include <cpu/x86/smm.h> -#include <cbmem.h> #include <string.h> #include "chip.h" #include "pch.h" @@ -648,25 +647,23 @@ pch_enable(dev); }
+void soc_fill_gnvs(struct global_nvs *gnvs) +{ + gnvs->apic = 1; + gnvs->mpen = 1; /* Enable Multi Processing */ + gnvs->pcnt = dev_count_cpu(); +} + void southbridge_inject_dsdt(const struct device *dev) { struct global_nvs *gnvs = acpi_get_gnvs(); + if (!gnvs) + return;
- if (gnvs) { + soc_fill_gnvs(gnvs); + acpi_create_gnvs(gnvs);
- acpi_create_gnvs(gnvs); - - gnvs->apic = 1; - gnvs->mpen = 1; /* Enable Multi Processing */ - gnvs->pcnt = dev_count_cpu(); - - - - /* Add it to DSDT. */ - acpigen_write_scope("\"); - acpigen_write_name_dword("NVSA", (u32) gnvs); - acpigen_pop_len(); - } + acpi_inject_nvsa(); }
void acpi_fill_fadt(acpi_fadt_t *fadt) diff --git a/src/southbridge/intel/i82801gx/lpc.c b/src/southbridge/intel/i82801gx/lpc.c index 129e9b1..a733b6a 100644 --- a/src/southbridge/intel/i82801gx/lpc.c +++ b/src/southbridge/intel/i82801gx/lpc.c @@ -16,7 +16,6 @@ #include <cpu/x86/smm.h> #include <acpi/acpigen.h> #include <arch/smp/mpspec.h> -#include <cbmem.h> #include <string.h> #include <southbridge/intel/common/acpi_pirq_gen.h> #include <southbridge/intel/common/pmbase.h> @@ -610,23 +609,21 @@ outb(POST_OS_BOOT, 0x80); }
+void soc_fill_gnvs(struct global_nvs *gnvs) +{ + gnvs->apic = 1; + gnvs->mpen = 1; /* Enable Multi Processing */ +} + void southbridge_inject_dsdt(const struct device *dev) { struct global_nvs *gnvs = acpi_get_gnvs(); + if (!gnvs) + return;
- if (gnvs) { - - gnvs->apic = 1; - gnvs->mpen = 1; /* Enable Multi Processing */ - - acpi_create_gnvs(gnvs); - - - /* Add it to SSDT. */ - acpigen_write_scope("\"); - acpigen_write_name_dword("NVSA", (u32) gnvs); - acpigen_pop_len(); - } + soc_fill_gnvs(gnvs); + acpi_create_gnvs(gnvs); + acpi_inject_nvsa(); }
static const char *lpc_acpi_name(const struct device *dev) diff --git a/src/southbridge/intel/i82801ix/lpc.c b/src/southbridge/intel/i82801ix/lpc.c index bbbdd71..efad617 100644 --- a/src/southbridge/intel/i82801ix/lpc.c +++ b/src/southbridge/intel/i82801ix/lpc.c @@ -15,7 +15,6 @@ #include <acpi/acpi_gnvs.h> #include <cpu/x86/smm.h> #include <acpi/acpigen.h> -#include <cbmem.h> #include <string.h> #include "chip.h" #include "i82801ix.h" @@ -458,20 +457,10 @@
void southbridge_inject_dsdt(const struct device *dev) { - struct global_nvs *gnvs = acpi_get_gnvs(); - - if (gnvs) { - acpi_create_gnvs(gnvs); - - - /* Add it to SSDT. */ - acpigen_write_scope("\"); - acpigen_write_name_dword("NVSA", (uintptr_t)gnvs); - acpigen_pop_len(); - } + acpi_create_gnvs(gnvs); + acpi_inject_nvsa(); }
- static const char *lpc_acpi_name(const struct device *dev) { return "LPCB"; diff --git a/src/southbridge/intel/i82801jx/lpc.c b/src/southbridge/intel/i82801jx/lpc.c index 645ebd6..a371ba1 100644 --- a/src/southbridge/intel/i82801jx/lpc.c +++ b/src/southbridge/intel/i82801jx/lpc.c @@ -16,7 +16,6 @@ #include <cpu/x86/smm.h> #include <acpi/acpigen.h> #include <arch/smp/mpspec.h> -#include <cbmem.h> #include <string.h> #include "chip.h" #include "i82801jx.h" @@ -618,16 +617,11 @@ void southbridge_inject_dsdt(const struct device *dev) { struct global_nvs *gnvs = acpi_get_gnvs(); + if (!gnvs) + return;
- if (gnvs) { - acpi_create_gnvs(gnvs); - - - /* Add it to SSDT. */ - acpigen_write_scope("\"); - acpigen_write_name_dword("NVSA", (u32) gnvs); - acpigen_pop_len(); - } + acpi_create_gnvs(gnvs); + acpi_inject_nvsa(); }
static const char *lpc_acpi_name(const struct device *dev) diff --git a/src/southbridge/intel/ibexpeak/lpc.c b/src/southbridge/intel/ibexpeak/lpc.c index 6e5c025..723fd0d 100644 --- a/src/southbridge/intel/ibexpeak/lpc.c +++ b/src/southbridge/intel/ibexpeak/lpc.c @@ -16,7 +16,6 @@ #include <acpi/acpi_gnvs.h> #include <elog.h> #include <acpi/acpigen.h> -#include <cbmem.h> #include <string.h> #include <cpu/x86/smm.h> #include "chip.h" @@ -553,24 +552,23 @@ pch_enable(dev); }
+void soc_fill_gnvs(struct global_nvs *gnvs) +{ + gnvs->apic = 1; + gnvs->mpen = 1; /* Enable Multi Processing */ + gnvs->pcnt = dev_count_cpu(); +} + void southbridge_inject_dsdt(const struct device *dev) { struct global_nvs *gnvs = acpi_get_gnvs(); + if (!gnvs) + return;
- if (gnvs) { + soc_fill_gnvs(gnvs); + acpi_create_gnvs(gnvs);
- acpi_create_gnvs(gnvs); - - gnvs->apic = 1; - gnvs->mpen = 1; /* Enable Multi Processing */ - gnvs->pcnt = dev_count_cpu(); - - - /* Add it to SSDT. */ - acpigen_write_scope("\"); - acpigen_write_name_dword("NVSA", (u32) gnvs); - acpigen_pop_len(); - } + acpi_inject_nvsa(); }
void acpi_fill_fadt(acpi_fadt_t *fadt) diff --git a/src/southbridge/intel/lynxpoint/lpc.c b/src/southbridge/intel/lynxpoint/lpc.c index 21cdac7..ca7300d 100644 --- a/src/southbridge/intel/lynxpoint/lpc.c +++ b/src/southbridge/intel/lynxpoint/lpc.c @@ -13,7 +13,6 @@ #include <acpi/acpi.h> #include <acpi/acpi_gnvs.h> #include <cpu/x86/smm.h> -#include <cbmem.h> #include <string.h> #include "chip.h" #include "nvs.h" @@ -689,24 +688,23 @@ pch_enable(dev); }
+void soc_fill_gnvs(struct global_nvs *gnvs) +{ + gnvs->apic = 1; + gnvs->mpen = 1; /* Enable Multi Processing */ + gnvs->pcnt = dev_count_cpu(); +} + void southbridge_inject_dsdt(const struct device *dev) { - struct global_nvs *gnvs; + struct global_nvs *gnvs = acpi_get_gnvs(); + if (!gnvs) + return;
- gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS); + soc_fill_gnvs(gnvs); + acpi_create_gnvs(gnvs);
- if (gnvs) { - acpi_create_gnvs(gnvs); - - gnvs->apic = 1; - gnvs->mpen = 1; /* Enable Multi Processing */ - gnvs->pcnt = dev_count_cpu(); - - /* Add it to DSDT. */ - acpigen_write_scope("\"); - acpigen_write_name_dword("NVSA", (u32) gnvs); - acpigen_pop_len(); - } + acpi_inject_nvsa(); }
void acpi_fill_fadt(acpi_fadt_t *fadt)
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42864
to look at the new patch set (#2).
Change subject: sb/intel: Factor out soc_fill_gnvs() ......................................................................
sb/intel: Factor out soc_fill_gnvs()
Name the common part of GNVS initialisation as soc_fill_gnvs(). It is also moved before the call to acpi_create_gnvs(), which followup will rename to mainbord_fill_gnvs() to reflect that implementation is under mb/.
Change-Id: Ic4cf1548b65a86212d6e45d460fcd23bb8036365 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/acpi/gnvs.c M src/include/acpi/acpi_gnvs.h M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/i82801gx/lpc.c M src/southbridge/intel/i82801ix/lpc.c M src/southbridge/intel/i82801jx/lpc.c M src/southbridge/intel/ibexpeak/lpc.c M src/southbridge/intel/lynxpoint/lpc.c 8 files changed, 58 insertions(+), 81 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/42864/2
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42864
to look at the new patch set (#3).
Change subject: sb/intel: Factor out soc_fill_gnvs() ......................................................................
sb/intel: Factor out soc_fill_gnvs()
Name the common part of GNVS initialisation as soc_fill_gnvs(). It is also moved before the call to acpi_create_gnvs(), which followup will rename to mainbord_fill_gnvs() to reflect that implementation is under mb/.
Change-Id: Ic4cf1548b65a86212d6e45d460fcd23bb8036365 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/acpi/gnvs.c M src/include/acpi/acpi_gnvs.h M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/i82801gx/lpc.c M src/southbridge/intel/i82801ix/lpc.c M src/southbridge/intel/i82801jx/lpc.c M src/southbridge/intel/ibexpeak/lpc.c M src/southbridge/intel/lynxpoint/lpc.c 8 files changed, 60 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/42864/3
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42864 )
Change subject: sb/intel: Factor out soc_fill_gnvs() ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42864/3/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/lpc.c:
https://review.coreboot.org/c/coreboot/+/42864/3/src/southbridge/intel/bd82x... PS3, Line 664: acpi_create_gnvs(gnvs); Implementation of acpi_create_gnvs() is under mb/ so the changed order here would seem preferable.
I was about to initialise GNVS prior to SMM module loader, but that dev_count_cpu() call has to be done after MP init.
Only the fields of GNVS that smihandler has references to, need to be initialised prior to first SMI trigger (after relocation). Perhaps there are none, I did not check yet.
Kyösti Mälkki has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/42864 )
Change subject: sb/intel: Factor out soc_fill_gnvs() ......................................................................
Abandoned
-
Kyösti Mälkki (Code Review)