Change in coreboot[master]: sec/intel/txt: Bail if var MTRRs cannot snugly cache the BIOS ACM
Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46422 )
Change subject: sec/intel/txt: Bail if var MTRRs cannot snugly cache the BIOS ACM ......................................................................
sec/intel/txt: Bail if var MTRRs cannot snugly cache the BIOS ACM
When caching the BIOS ACM, one must cache less than a page (4 KiB) of unused memory past the end of the BIOS ACM. Failure to do so will result in a lovely TXT reset with Class Code 5, Major Error Code 2. How boring.
The current approach uses a single variable MTRR to cache the whole BIOS ACM. Before fighting with the variable MTRRs in assembly code, ensure that enough variable MTRRs exist to cache the BIOS ACM's size. Since the code checks that the ACM base is aligned to its size, each `one` bit in the ACM size will require one variable MTRR to properly cache the ACM.
One of the several BIOS ACMs for Haswell has a size of 101504 bytes. This is 0x18c80 in hexadecimal, and 0001 1000 1100 1000 0000 in binary.
After aligning up the BIOS ACM size to a page boundary, the resulting size is 0x19000 in hexadecimal, and 0001 1001 0000 0000 0000 in binary.
To successfully invoke said ACM, its base must be a multiple of 0x20000 and three variable MTRRs must be used to cache the ACM. The MTRR ranges must be contiguous and cover 0x10000, 0x8000, 0x1000 bytes, in order.
The assembly code is updated in a follow-up, and relies on these checks.
Change-Id: I480dc3e4a9e4a59fbb73d571fd62b0257abc65b3 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/security/intel/txt/common.c 1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/46422/1
diff --git a/src/security/intel/txt/common.c b/src/security/intel/txt/common.c index e73defb..d5c794d 100644 --- a/src/security/intel/txt/common.c +++ b/src/security/intel/txt/common.c @@ -6,6 +6,7 @@ #include <cpu/x86/cr.h> #include <cpu/x86/lapic.h> #include <cpu/x86/mp.h> +#include <cpu/x86/mtrr.h> #include <lib.h> #include <smp/node.h> #include <string.h> @@ -266,6 +267,17 @@ return -1; }
+ /* + * When setting up the MTRRs to cache the BIOS ACM, one must cache less + * than a page (4 KiB) of unused memory after the BIOS ACM. Failure to + * do so will cause a TXT reset with Class Code 5, Major Error Code 2. + */ + if (popcnt(ALIGN_UP(acm_len, 4096)) > get_var_mtrr_count()) { + printk(BIOS_ERR, "TEE-TXT: Not enough MTRRs to cache this BIOS ACM's size.\n"); + rdev_munmap(&acm, acm_data); + return -1; + } + if (CONFIG(INTEL_TXT_LOGGING)) txt_dump_acm_info(acm_data);
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46422 )
Change subject: sec/intel/txt: Bail if var MTRRs cannot snugly cache the BIOS ACM ......................................................................
Patch Set 1: Code-Review+2
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46422 )
Change subject: sec/intel/txt: Bail if var MTRRs cannot snugly cache the BIOS ACM ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/46422/1/src/security/intel/txt/comm... File src/security/intel/txt/common.c:
https://review.coreboot.org/c/coreboot/+/46422/1/src/security/intel/txt/comm... PS1, Line 273: * do so will cause a TXT reset with Class Code 5, Major Error Code 2. No minor? Can you add the chipset generation you used as these error codes tend to change between the chipset generations.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46422 )
Change subject: sec/intel/txt: Bail if var MTRRs cannot snugly cache the BIOS ACM ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46422/1/src/security/intel/txt/comm... File src/security/intel/txt/common.c:
https://review.coreboot.org/c/coreboot/+/46422/1/src/security/intel/txt/comm... PS1, Line 273: * do so will cause a TXT reset with Class Code 5, Major Error Code 2.
No minor? Can you add the chipset generation you used as these error codes tend to change between th […]
I think no minor is defined for this error, and yes I'll mention this happens on Haswell
Hello Philipp Deppenwiese, build bot (Jenkins), Patrick Rudolph, Christian Walter, Arthur Heymans,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46422
to look at the new patch set (#2).
Change subject: sec/intel/txt: Bail if var MTRRs cannot snugly cache the BIOS ACM ......................................................................
sec/intel/txt: Bail if var MTRRs cannot snugly cache the BIOS ACM
When caching the BIOS ACM, one must cache less than a page (4 KiB) of unused memory past the end of the BIOS ACM. Failure to do so on Haswell will result in a lovely TXT reset with Class Code 5, Major Error Code 2.
The current approach uses a single variable MTRR to cache the whole BIOS ACM. Before fighting with the variable MTRRs in assembly code, ensure that enough variable MTRRs exist to cache the BIOS ACM's size. Since the code checks that the ACM base is aligned to its size, each `one` bit in the ACM size will require one variable MTRR to properly cache the ACM.
One of the several BIOS ACMs for Haswell has a size of 101504 bytes. This is 0x18c80 in hexadecimal, and 0001 1000 1100 1000 0000 in binary.
After aligning up the BIOS ACM size to a page boundary, the resulting size is 0x19000 in hexadecimal, and 0001 1001 0000 0000 0000 in binary.
To successfully invoke said ACM, its base must be a multiple of 0x20000 and three variable MTRRs must be used to cache the ACM. The MTRR ranges must be contiguous and cover 0x10000, 0x8000, 0x1000 bytes, in order.
The assembly code is updated in a follow-up, and relies on these checks.
Change-Id: I480dc3e4a9e4a59fbb73d571fd62b0257abc65b3 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/security/intel/txt/common.c 1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/46422/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46422 )
Change subject: sec/intel/txt: Bail if var MTRRs cannot snugly cache the BIOS ACM ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46422/1/src/security/intel/txt/comm... File src/security/intel/txt/common.c:
https://review.coreboot.org/c/coreboot/+/46422/1/src/security/intel/txt/comm... PS1, Line 273: * do so will cause a TXT reset with Class Code 5, Major Error Code 2.
I think no minor is defined for this error, and yes I'll mention this happens on Haswell
Done
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46422 )
Change subject: sec/intel/txt: Bail if var MTRRs cannot snugly cache the BIOS ACM ......................................................................
Patch Set 3: Code-Review+2
Philipp Deppenwiese has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46422 )
Change subject: sec/intel/txt: Bail if var MTRRs cannot snugly cache the BIOS ACM ......................................................................
sec/intel/txt: Bail if var MTRRs cannot snugly cache the BIOS ACM
When caching the BIOS ACM, one must cache less than a page (4 KiB) of unused memory past the end of the BIOS ACM. Failure to do so on Haswell will result in a lovely TXT reset with Class Code 5, Major Error Code 2.
The current approach uses a single variable MTRR to cache the whole BIOS ACM. Before fighting with the variable MTRRs in assembly code, ensure that enough variable MTRRs exist to cache the BIOS ACM's size. Since the code checks that the ACM base is aligned to its size, each `one` bit in the ACM size will require one variable MTRR to properly cache the ACM.
One of the several BIOS ACMs for Haswell has a size of 101504 bytes. This is 0x18c80 in hexadecimal, and 0001 1000 1100 1000 0000 in binary.
After aligning up the BIOS ACM size to a page boundary, the resulting size is 0x19000 in hexadecimal, and 0001 1001 0000 0000 0000 in binary.
To successfully invoke said ACM, its base must be a multiple of 0x20000 and three variable MTRRs must be used to cache the ACM. The MTRR ranges must be contiguous and cover 0x10000, 0x8000, 0x1000 bytes, in order.
The assembly code is updated in a follow-up, and relies on these checks.
Change-Id: I480dc3e4a9e4a59fbb73d571fd62b0257abc65b3 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/46422 Reviewed-by: Arthur Heymans arthur@aheymans.xyz Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/security/intel/txt/common.c 1 file changed, 12 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved
diff --git a/src/security/intel/txt/common.c b/src/security/intel/txt/common.c index e73defb..5f8a976 100644 --- a/src/security/intel/txt/common.c +++ b/src/security/intel/txt/common.c @@ -6,6 +6,7 @@ #include <cpu/x86/cr.h> #include <cpu/x86/lapic.h> #include <cpu/x86/mp.h> +#include <cpu/x86/mtrr.h> #include <lib.h> #include <smp/node.h> #include <string.h> @@ -266,6 +267,17 @@ return -1; }
+ /* + * When setting up the MTRRs to cache the BIOS ACM, one must cache less than + * a page (4 KiB) of unused memory after the BIOS ACM. On Haswell, failure + * to do so will cause a TXT reset with Class Code 5, Major Error Code 2. + */ + if (popcnt(ALIGN_UP(acm_len, 4096)) > get_var_mtrr_count()) { + printk(BIOS_ERR, "TEE-TXT: Not enough MTRRs to cache this BIOS ACM's size.\n"); + rdev_munmap(&acm, acm_data); + return -1; + } + if (CONFIG(INTEL_TXT_LOGGING)) txt_dump_acm_info(acm_data);
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Christian Walter (Code Review)
-
Philipp Deppenwiese (Code Review)