Maulik V Vaghela has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: rtc/post.c: Fix compilation issue for post.c ......................................................................
rtc/post.c: Fix compilation issue for post.c
cmos_post_code linking will provide an error if some code is using the function in smm stage. Adding post.c file in smm stage would fix this issue
Change-Id: Iffdcccaad48e7ad96e068d07046630fbe4297e65 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/drivers/pc80/rtc/Makefile.inc 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/38370/1
diff --git a/src/drivers/pc80/rtc/Makefile.inc b/src/drivers/pc80/rtc/Makefile.inc index 4938d78..fa622c0 100644 --- a/src/drivers/pc80/rtc/Makefile.inc +++ b/src/drivers/pc80/rtc/Makefile.inc @@ -12,6 +12,7 @@ smm-$(CONFIG_USE_OPTION_TABLE) += option.c
all-$(CONFIG_CMOS_POST) += post.c +smm-$(CONFIG_CMOS_POST) += post.c
ifeq ($(CONFIG_USE_OPTION_TABLE),y) cbfs-files-$(CONFIG_HAVE_CMOS_DEFAULT) += cmos.default
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: rtc/post.c: Fix compilation issue for post.c ......................................................................
Patch Set 1:
Please add test configuration for the feature.
CMOS uses a split IO transaction, I doubt if there is a backup/restore implemented for the related IO index address, so it's not really safe to call cmos_post_code from SMM context, I would prefer adding !ENV_SMM at the call-site.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: rtc/post.c: Fix compilation issue for post.c ......................................................................
Patch Set 1:
@Maulik, i guess your problem statement is you are trying to use postcodes inside SMM mode and you saw compiler issue. Is that right ?
Although i don't know if we have any restriction of postcode usage in SMM mode but somewhere its worth to capture this and as Kyosti has mentioned if you don't want SMM use postcode then guard with preprocessor macros
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: rtc/post.c: Fix compilation issue for post.c ......................................................................
Patch Set 1:
Patch Set 1:
@Maulik, i guess your problem statement is you are trying to use postcodes inside SMM mode and you saw compiler issue. Is that right ?
Although i don't know if we have any restriction of postcode usage in SMM mode but somewhere its worth to capture this and as Kyosti has mentioned if you don't want SMM use postcode then guard with preprocessor macros
Yes Subrata...I see compilation issues Yes, I think I can update the patch to capture this information and restrict the usage of cmos_post_code inside SMM
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: rtc/post.c: Fix compilation issue for post.c ......................................................................
Patch Set 1:
Patch Set 1:
Please add test configuration for the feature.
CMOS uses a split IO transaction, I doubt if there is a backup/restore implemented for the related IO index address, so it's not really safe to call cmos_post_code from SMM context, I would prefer adding !ENV_SMM at the call-site.
Hey Kyosti,
In that case, Let me update this patch and capture this information so that it is taken care going forward.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: rtc/post.c: Fix compilation issue for post.c ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
@Maulik, i guess your problem statement is you are trying to use postcodes inside SMM mode and you saw compiler issue. Is that right ?
Although i don't know if we have any restriction of postcode usage in SMM mode but somewhere its worth to capture this and as Kyosti has mentioned if you don't want SMM use postcode then guard with preprocessor macros
Yes Subrata...I see compilation issues Yes, I think I can update the patch to capture this information and restrict the usage of cmos_post_code inside SMM
cool
Hello Kyösti Mälkki, Subrata Banik, Aamir Bohra, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38370
to look at the new patch set (#2).
Change subject: rtc/post.c: Fix compilation issue for post.c ......................................................................
rtc/post.c: Fix compilation issue for post.c
cmos_post_code linking will provide an error if some code is using the function in smm stage.
Also as per patch discussion, CMOS uses a split IO transaction and it's not really safe to call cmos_post_code from SMM context. Thus guarding caller function to resolve the issue
Change-Id: Iffdcccaad48e7ad96e068d07046630fbe4297e65 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/console/post.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/38370/2
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: rtc/post.c: Fix compilation issue for post.c ......................................................................
Patch Set 2:
Patch Set 1:
Please add test configuration for the feature.
CMOS uses a split IO transaction, I doubt if there is a backup/restore implemented for the related IO index address, so it's not really safe to call cmos_post_code from SMM context, I would prefer adding !ENV_SMM at the call-site.
I have updated patchset with suggestion
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: rtc/post.c: Fix compilation issue for post.c ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/38370/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38370/2//COMMIT_MSG@7 PS2, Line 7: rtc/post.c: Fix compilation issue for post.c Too generic. Maybe:
rtc/post: Hide cmos_post_code() in SMM
https://review.coreboot.org/c/coreboot/+/38370/2//COMMIT_MSG@10 PS2, Line 10: function in smm stage. What is the error?
https://review.coreboot.org/c/coreboot/+/38370/2//COMMIT_MSG@14 PS2, Line 14: caller function to resolve the issue It looks like this is the only remaining change?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: rtc/post.c: Fix compilation issue for post.c ......................................................................
Patch Set 2: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: rtc/post.c: Fix compilation issue for post.c ......................................................................
Patch Set 2:
CB:38187 got merged so this needs rebase on master now.
Hello Kyösti Mälkki, Subrata Banik, Aamir Bohra, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38370
to look at the new patch set (#3).
Change subject: rtc/post.c: Fix compilation issue for post.c ......................................................................
rtc/post.c: Fix compilation issue for post.c
cmos_post_code linking will provide an error if some code is using the function in smm stage.
Also as per patch discussion, CMOS uses a split IO transaction and it's not really safe to call cmos_post_code from SMM context. Thus guarding caller function to resolve the issue
Change-Id: Iffdcccaad48e7ad96e068d07046630fbe4297e65 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/arch/x86/post.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/38370/3
Hello Kyösti Mälkki, Subrata Banik, Aamir Bohra, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38370
to look at the new patch set (#4).
Change subject: arch/x86/post.c: Hide cmos_post_code from SMM context ......................................................................
arch/x86/post.c: Hide cmos_post_code from SMM context
Code in SMM segment using cmos_post_code will give compiler error since cmos_post_code function is not getting compiled during SMM stage.
Also as per patch discussion, CMOS uses a split IO transaction and it's not really safe to call cmos_post_code from SMM context. Thus we'll hide the call for SMM context.
Change-Id: Iffdcccaad48e7ad96e068d07046630fbe4297e65 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/arch/x86/post.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/38370/4
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: arch/x86/post.c: Hide cmos_post_code from SMM context ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/38370/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38370/2//COMMIT_MSG@7 PS2, Line 7: rtc/post.c: Fix compilation issue for post.c
Too generic. Maybe: […]
Ack
https://review.coreboot.org/c/coreboot/+/38370/2//COMMIT_MSG@10 PS2, Line 10: function in smm stage.
What is the error?
Done
https://review.coreboot.org/c/coreboot/+/38370/2//COMMIT_MSG@14 PS2, Line 14: caller function to resolve the issue
It looks like this is the only remaining change?
Yes
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: arch/x86/post.c: Hide cmos_post_code from SMM context ......................................................................
Patch Set 4: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: arch/x86/post.c: Hide cmos_post_code from SMM context ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38370/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38370/2//COMMIT_MSG@14 PS2, Line 14: caller function to resolve the issue
Yes
Done
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38370 )
Change subject: arch/x86/post.c: Hide cmos_post_code from SMM context ......................................................................
arch/x86/post.c: Hide cmos_post_code from SMM context
Code in SMM segment using cmos_post_code will give compiler error since cmos_post_code function is not getting compiled during SMM stage.
Also as per patch discussion, CMOS uses a split IO transaction and it's not really safe to call cmos_post_code from SMM context. Thus we'll hide the call for SMM context.
Change-Id: Iffdcccaad48e7ad96e068d07046630fbe4297e65 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/38370 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/arch/x86/post.c 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved
diff --git a/src/arch/x86/post.c b/src/arch/x86/post.c index 0a20bab..b9cd26b 100644 --- a/src/arch/x86/post.c +++ b/src/arch/x86/post.c @@ -21,6 +21,6 @@ if (CONFIG(POST_IO)) outb(value, CONFIG_POST_IO_PORT);
- if (CONFIG(CMOS_POST)) + if (CONFIG(CMOS_POST) && !ENV_SMM) cmos_post_code(value); }
-
Kyösti Mälkki (Code Review) -
Maulik V Vaghela (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Subrata Banik (Code Review)