Change in coreboot[master]: soc/intel/{apl, cnl, skl}: Add sanity check of SA_PCIEX_LENGTH_MIB
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40337 )
Change subject: soc/intel/{apl, cnl, skl}: Add sanity check of SA_PCIEX_LENGTH_MIB ......................................................................
soc/intel/{apl, cnl, skl}: Add sanity check of SA_PCIEX_LENGTH_MIB
Not all IA SoC has provision for higher PCIEXBAR length hence added required check inside SoC to avoid wrong length programming.
TEST=Select PCIEX_LENGTH_4096MB config in Cannonlake Kconfig file and build causes compilation error.
Change-Id: I0dd67868df373c6ab5c724a21371bcc206968036 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/apollolake/systemagent.c M src/soc/intel/cannonlake/systemagent.c M src/soc/intel/skylake/systemagent.c 3 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/40337/1
diff --git a/src/soc/intel/apollolake/systemagent.c b/src/soc/intel/apollolake/systemagent.c index f564e8f..27686ee 100644 --- a/src/soc/intel/apollolake/systemagent.c +++ b/src/soc/intel/apollolake/systemagent.c @@ -23,6 +23,10 @@ #include <soc/iomap.h> #include <soc/systemagent.h>
+#if (CONFIG_SA_PCIEX_LENGTH_MIB > 0x100) /* Max length is 256MB */ +#error "Invalid SA_PCIEX_LENGTH_MIB selection!" +#endif + /* * SoC implementation * diff --git a/src/soc/intel/cannonlake/systemagent.c b/src/soc/intel/cannonlake/systemagent.c index 54b16e2..9f56890 100644 --- a/src/soc/intel/cannonlake/systemagent.c +++ b/src/soc/intel/cannonlake/systemagent.c @@ -12,6 +12,10 @@ #include <soc/systemagent.h> #include "chip.h"
+#if (CONFIG_SA_PCIEX_LENGTH_MIB > 0x100) /* Max length is 256MB */ +#error "Invalid SA_PCIEX_LENGTH_MIB selection!" +#endif + /* * SoC implementation * diff --git a/src/soc/intel/skylake/systemagent.c b/src/soc/intel/skylake/systemagent.c index 10d79d3..22a764a 100644 --- a/src/soc/intel/skylake/systemagent.c +++ b/src/soc/intel/skylake/systemagent.c @@ -14,6 +14,10 @@ #include <soc/systemagent.h> #include "chip.h"
+#if (CONFIG_SA_PCIEX_LENGTH_MIB > 0x100) /* Max length is 256MB */ +#error "Invalid SA_PCIEX_LENGTH_MIB selection!" +#endif + bool soc_is_vtd_capable(void) { struct device *const root_dev = pcidev_path_on_root(SA_DEVFN_ROOT);
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40337 )
Change subject: soc/intel/{apl, cnl, skl}: Add sanity check of SA_PCIEX_LENGTH_MIB ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40337/1/src/soc/intel/skylake/syste... File src/soc/intel/skylake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/40337/1/src/soc/intel/skylake/syste... PS1, Line 17: #if (CONFIG_SA_PCIEX_LENGTH_MIB > 0x100) /* Max length is 256MB */ : #error "Invalid SA_PCIEX_LENGTH_MIB selection!" : #endif The code from src/soc/intel is used not only for SoC. What about the Xeon E3-1230 v6 lga-1151 processor? Is this limitation also necessary for all Sky/Kaby Lake CPUs?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40337 )
Change subject: soc/intel/{apl, cnl, skl}: Add sanity check of SA_PCIEX_LENGTH_MIB ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40337/1/src/soc/intel/skylake/syste... File src/soc/intel/skylake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/40337/1/src/soc/intel/skylake/syste... PS1, Line 17: #if (CONFIG_SA_PCIEX_LENGTH_MIB > 0x100) /* Max length is 256MB */ : #error "Invalid SA_PCIEX_LENGTH_MIB selection!" : #endif
The code from src/soc/intel is used not only for SoC. […]
Yes, looking at EDS, it appears that maximum PCIEXBAR length support for SKL/KBL is 256MB.
With CB:40235 we are adding new soc capability without any additional guard at common layer hence thinking guard at soc layer might ensure wrong selection by users.
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40337 )
Change subject: soc/intel/{apl, cnl, skl}: Add sanity check of SA_PCIEX_LENGTH_MIB ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/40337/1/src/soc/intel/skylake/syste... File src/soc/intel/skylake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/40337/1/src/soc/intel/skylake/syste... PS1, Line 17: #if (CONFIG_SA_PCIEX_LENGTH_MIB > 0x100) /* Max length is 256MB */ : #error "Invalid SA_PCIEX_LENGTH_MIB selection!" : #endif
Yes, looking at EDS, it appears that maximum PCIEXBAR length support for SKL/KBL is 256MB. […]
Yes you are right. It seemed to me that xeon e3 might be different from other skl/kbl cpus. Thanks
Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40337 )
Change subject: soc/intel/{apl, cnl, skl}: Add sanity check of SA_PCIEX_LENGTH_MIB ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/40337/4/src/soc/intel/apollolake/sy... File src/soc/intel/apollolake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/40337/4/src/soc/intel/apollolake/sy... PS4, Line 25: : #if (CONFIG_SA_PCIEX_LENGTH_MIB > 0x100) /* Max length is 256MB */ : #error "Invalid SA_PCIEX_LENGTH_MIB selection!" : #endif May be we can put this check in soc/systemagent.h ?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40337 )
Change subject: soc/intel/{apl, cnl, skl}: Add sanity check of SA_PCIEX_LENGTH_MIB ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40337/4/src/soc/intel/apollolake/sy... File src/soc/intel/apollolake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/40337/4/src/soc/intel/apollolake/sy... PS4, Line 25: : #if (CONFIG_SA_PCIEX_LENGTH_MIB > 0x100) /* Max length is 256MB */ : #error "Invalid SA_PCIEX_LENGTH_MIB selection!" : #endif
May be we can put this check in soc/systemagent. […]
Is there any specific different of keeping this inside .h file ? its due to #if you are asking ?
Subrata Banik has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/40337 )
Change subject: soc/intel/{apl, cnl, skl}: Add sanity check of SA_PCIEX_LENGTH_MIB ......................................................................
Abandoned
-
Maxim Polyakov (Code Review) -
Pratikkumar V Prajapati (Code Review)
-
Subrata Banik (Code Review)