Change in coreboot[master]: cpu/x86/smm: Edit - improve clarity - coreboot-gerrit

List overview All Threads
Download

newer

Change in coreboot[master]: cpu/x86/smm: Edit - improve clarity

older

Change in coreboot[master]:...

Name of user not set (Code Review)

10 Feb 2020 10 Feb '20

10:50 p.m.

cedarhouse1@comcast.net has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38821 )

Change subject: cpu/x86/smm: Edit - improve clarity ......................................................................

cpu/x86/smm: Edit - improve clarity

Removed blank line to maintain the relation between the previous comment and the remainder of the block.

Signed-off-by: Eugene D. Myers edmyers@tycho.nsa.gov Change-Id: Ib9754c6723ecd5e4895898490fc7228e1c3839d0 --- M src/cpu/x86/smm/smm_module_loader.c 1 file changed, 0 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/38821/1

diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index 81020a4..66a40c4 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -396,7 +396,6 @@ /* Does the required amount of memory exceed the SMRAM region size? */ total_size = total_stack_size + handler_size; total_size += fxsave_size + SMM_DEFAULT_SIZE; - // account for the bios resource list if (CONFIG(STM)) total_size += CONFIG_BIOS_RESOURCE_LIST_SIZE;

Attachments:

attachment.htm (text/html — 2.4 KB)

Show replies by date

Paul Menzel (Code Review)

10 Feb 10 Feb

11:32 p.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38821 )

Change subject: cpu/x86/smm: Edit - improve clarity ......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/38821/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/38821/1//COMMIT_MSG@7 PS1, Line 7: cpu/x86/smm: Edit - improve clarity This is an unusual description. Why not:

...

cpu/x86/smm: Remove blank line in code

https://review.coreboot.org/c/coreboot/+/38821/1//COMMIT_MSG@9 PS1, Line 9: Removed Remove

-- To view, visit https://review.coreboot.org/c/coreboot/+/38821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9754c6723ecd5e4895898490fc7228e1c3839d0 Gerrit-Change-Number: 38821 Gerrit-PatchSet: 1 Gerrit-Owner: cedarhouse1@comcast.net Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: cedarhouse1@comcast.net Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 10 Feb 2020 23:32:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Name of user not set (Code Review)

11:39 p.m.

cedarhouse1@comcast.net has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38821 )

Change subject: cpu/x86/smm: Edit - improve clarity ......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/38821/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/38821/1//COMMIT_MSG@7 PS1, Line 7: cpu/x86/smm: Edit - improve clarity

...

This is an unusual description. Why not: […]

Done

https://review.coreboot.org/c/coreboot/+/38821/1//COMMIT_MSG@9 PS1, Line 9: Removed

...

Remove

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/38821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9754c6723ecd5e4895898490fc7228e1c3839d0 Gerrit-Change-Number: 38821 Gerrit-PatchSet: 1 Gerrit-Owner: cedarhouse1@comcast.net Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: cedarhouse1@comcast.net Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 10 Feb 2020 23:39:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Name of user not set (Code Review)

11:40 p.m.

New subject: Change in coreboot[master]: cpu/x86/smm: Remove blank line in code

Hello build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38821

to look at the new patch set (#2).

Change subject: cpu/x86/smm: Remove blank line in code ......................................................................

cpu/x86/smm: Remove blank line in code

Remove blank line to maintain the relation between the previous comment and the remainder of the block.

Signed-off-by: Eugene D. Myers edmyers@tycho.nsa.gov Change-Id: Ib9754c6723ecd5e4895898490fc7228e1c3839d0 --- M src/cpu/x86/smm/smm_module_loader.c 1 file changed, 0 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/38821/2

Angel Pons (Code Review)

12 Feb 12 Feb

9:51 p.m.

New subject: Change in coreboot[master]: cpu/x86/smm: Remove blank line in code

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38821 )

Change subject: cpu/x86/smm: Remove blank line in code ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38821/2/src/cpu/x86/smm/smm_module_... File src/cpu/x86/smm/smm_module_loader.c:

https://review.coreboot.org/c/coreboot/+/38821/2/src/cpu/x86/smm/smm_module_... PS2, Line 399: // account for the bios resource list If you ask me, the blank line actually helps differentiating the blocks of code. IMHO, the lone C++ style comment that starts in lowercase is more of an eyesore. Oh, and 'bios' in lowercase.

-- To view, visit https://review.coreboot.org/c/coreboot/+/38821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9754c6723ecd5e4895898490fc7228e1c3839d0 Gerrit-Change-Number: 38821 Gerrit-PatchSet: 2 Gerrit-Owner: cedarhouse1@comcast.net Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: cedarhouse1@comcast.net Gerrit-CC: Angel Pons th3fanbus@gmail.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 12 Feb 2020 21:51:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Name of user not set (Code Review)

10:16 p.m.

New subject: Change in coreboot[master]: cpu/x86/smm: Remove blank line in code

cedarhouse1@comcast.net has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38821 )

Change subject: cpu/x86/smm: Remove blank line in code ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38821/2/src/cpu/x86/smm/smm_module_... File src/cpu/x86/smm/smm_module_loader.c:

https://review.coreboot.org/c/coreboot/+/38821/2/src/cpu/x86/smm/smm_module_... PS2, Line 399: // account for the bios resource list

...

If you ask me, the blank line actually helps differentiating the blocks of code. […]

The eyesore I can fix.

The reviewer conflict I'll leave to the process.

-- To view, visit https://review.coreboot.org/c/coreboot/+/38821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9754c6723ecd5e4895898490fc7228e1c3839d0 Gerrit-Change-Number: 38821 Gerrit-PatchSet: 2 Gerrit-Owner: cedarhouse1@comcast.net Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: cedarhouse1@comcast.net Gerrit-CC: Angel Pons th3fanbus@gmail.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 12 Feb 2020 22:16:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Angel Pons (Code Review)

11:06 p.m.

New subject: Change in coreboot[master]: cpu/x86/smm: Remove blank line in code

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38821 )

Change subject: cpu/x86/smm: Remove blank line in code ......................................................................

Patch Set 2: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/38821/2/src/cpu/x86/smm/smm_module_... File src/cpu/x86/smm/smm_module_loader.c:

https://review.coreboot.org/c/coreboot/+/38821/2/src/cpu/x86/smm/smm_module_... PS2, Line 399: // account for the bios resource list

...

The eyesore I can fix. […]

Ack. Not worth bikeshedding about such things, the compiler doesn't squish comments nor whitespace into machine code anyway.

-- To view, visit https://review.coreboot.org/c/coreboot/+/38821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9754c6723ecd5e4895898490fc7228e1c3839d0 Gerrit-Change-Number: 38821 Gerrit-PatchSet: 2 Gerrit-Owner: cedarhouse1@comcast.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: cedarhouse1@comcast.net Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 12 Feb 2020 23:06:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: cedarhouse1@comcast.net Gerrit-MessageType: comment

Name of user not set (Code Review)

11:08 p.m.

New subject: Change in coreboot[master]: cpu/x86/smm: Remove blank line in code

cedarhouse1@comcast.net has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38821 )

Change subject: cpu/x86/smm: Remove blank line in code ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38821/2/src/cpu/x86/smm/smm_module_... File src/cpu/x86/smm/smm_module_loader.c:

https://review.coreboot.org/c/coreboot/+/38821/2/src/cpu/x86/smm/smm_module_... PS2, Line 399: // account for the bios resource list

...

Ack. […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/38821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9754c6723ecd5e4895898490fc7228e1c3839d0 Gerrit-Change-Number: 38821 Gerrit-PatchSet: 2 Gerrit-Owner: cedarhouse1@comcast.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: cedarhouse1@comcast.net Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 12 Feb 2020 23:08:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: cedarhouse1@comcast.net Gerrit-MessageType: comment

Nico Huber (Code Review)

13 Feb 13 Feb

9:16 a.m.

New subject: Change in coreboot[master]: cpu/x86/smm: Remove blank line in code

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38821 )

Change subject: cpu/x86/smm: Remove blank line in code ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38821/2/src/cpu/x86/smm/smm_module_... File src/cpu/x86/smm/smm_module_loader.c:

https://review.coreboot.org/c/coreboot/+/38821/2/src/cpu/x86/smm/smm_module_... PS2, Line 399: // account for the bios resource list

...

Done

My original thought was to remove both blank lines and possibly the intermediate comment, becasue the comment in line 396 refers to line 403. Doesn't matter much, the new code will likely be removed here midterm. No need to bikeshed cosmetics in this case.

-- To view, visit https://review.coreboot.org/c/coreboot/+/38821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9754c6723ecd5e4895898490fc7228e1c3839d0 Gerrit-Change-Number: 38821 Gerrit-PatchSet: 2 Gerrit-Owner: cedarhouse1@comcast.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: cedarhouse1@comcast.net Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 13 Feb 2020 09:16:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: cedarhouse1@comcast.net Gerrit-MessageType: comment

Name of user not set (Code Review)

5:10 p.m.

New subject: Change in coreboot[master]: cpu/x86/smm: Remove blank line in code

cedarhouse1@comcast.net has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38821 )

Change subject: cpu/x86/smm: Remove blank line in code ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38821/2/src/cpu/x86/smm/smm_module_... File src/cpu/x86/smm/smm_module_loader.c:

https://review.coreboot.org/c/coreboot/+/38821/2/src/cpu/x86/smm/smm_module_... PS2, Line 399: // account for the bios resource list

...

My original thought was to remove both blank lines and possibly the […]

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/38821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9754c6723ecd5e4895898490fc7228e1c3839d0 Gerrit-Change-Number: 38821 Gerrit-PatchSet: 2 Gerrit-Owner: cedarhouse1@comcast.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: cedarhouse1@comcast.net Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 13 Feb 2020 17:10:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: cedarhouse1@comcast.net Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Patrick Georgi (Code Review)

17 Feb 17 Feb

3:35 p.m.

New subject: Change in coreboot[master]: cpu/x86/smm: Remove blank line in code

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38821 )

Change subject: cpu/x86/smm: Remove blank line in code ......................................................................

cpu/x86/smm: Remove blank line in code

Remove blank line to maintain the relation between the previous comment and the remainder of the block.

Signed-off-by: Eugene D. Myers edmyers@tycho.nsa.gov Change-Id: Ib9754c6723ecd5e4895898490fc7228e1c3839d0 Reviewed-on: https://review.coreboot.org/c/coreboot/+/38821 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/cpu/x86/smm/smm_module_loader.c 1 file changed, 0 insertions(+), 1 deletion(-)

Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved

-- To view, visit https://review.coreboot.org/c/coreboot/+/38821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9754c6723ecd5e4895898490fc7228e1c3839d0 Gerrit-Change-Number: 38821 Gerrit-PatchSet: 3 Gerrit-Owner: cedarhouse1@comcast.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: cedarhouse1@comcast.net Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

9elements QA (Code Review)

3:43 p.m.

New subject: Change in coreboot[master]: cpu/x86/smm: Remove blank line in code

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38821 )

Change subject: cpu/x86/smm: Remove blank line in code ......................................................................

Patch Set 3:

Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/640 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/639 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/638

Please note: This test is under development and might not be accurate at all!

-- To view, visit https://review.coreboot.org/c/coreboot/+/38821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9754c6723ecd5e4895898490fc7228e1c3839d0 Gerrit-Change-Number: 38821 Gerrit-PatchSet: 3 Gerrit-Owner: cedarhouse1@comcast.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: cedarhouse1@comcast.net Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 17 Feb 2020 15:43:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1580

days inactive

1587

days old

coreboot-gerrit@coreboot.org

11 comments

6 participants

tags (0)

participants (6)

9elements QA (Code Review)
Angel Pons (Code Review)
Name of user not set (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)
Paul Menzel (Code Review)