Change in coreboot[master]: soc/amd/picasso: Halt if workbuf is absent after psp_verstage

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Martin Roth (Code Review)

25 Jun 2020 25 Jun '20

3:45 p.m.

Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42810 )

Change subject: soc/amd/picasso: Halt if workbuf is absent after psp_verstage ......................................................................

soc/amd/picasso: Halt if workbuf is absent after psp_verstage

Check for the workbuf in bootblock if psp_verstage is being used.

BUG=b:158124527 TEST=Build & boot Trembyle with psp_verstage

Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e --- M src/soc/amd/picasso/bootblock/bootblock.c 1 file changed, 17 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/42810/1

diff --git a/src/soc/amd/picasso/bootblock/bootblock.c b/src/soc/amd/picasso/bootblock/bootblock.c index a3935cc..f633767 100644 --- a/src/soc/amd/picasso/bootblock/bootblock.c +++ b/src/soc/amd/picasso/bootblock/bootblock.c @@ -14,6 +14,11 @@ #include <amdblocks/amd_pci_mmconf.h> #include <acpi/acpi.h>

+/* vboot includes directory may bot be in include path if vboot is not enabled */ +#if CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) +#include <2struct.h> +#endif + asmlinkage void bootblock_resume_entry(void);

/* PSP performs the memory training and setting up DRAM map prior to x86 cores @@ -123,5 +128,17 @@ u32 val = cpuid_eax(1); printk(BIOS_DEBUG, "Family_Model: %08x\n", val);

+#if CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) +#include <2struct.h> + unsigned int *workbuf_location = (unsigned int *)CONFIG_PSP_SHAREDMEM_BASE; + if (*workbuf_location != VB2_SHARED_DATA_MAGIC) { + printk(BIOS_ERR,"ERROR: VBOOT workbuf not valid.\n"); + + printk(BIOS_DEBUG,"Signature: %#08x\n",*workbuf_location); + + die("Halting.\n"); + } +#endif + fch_early_init(); }

Attachments:

attachment.htm (text/html — 4.1 KB)

Show replies by date

build bot (Jenkins) (Code Review)

25 Jun 25 Jun

4:28 p.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42810 )

Change subject: soc/amd/picasso: Halt if workbuf is absent after psp_verstage ......................................................................

Patch Set 1:

(4 comments)

https://review.coreboot.org/c/coreboot/+/42810/1/src/soc/amd/picasso/bootblo... File src/soc/amd/picasso/bootblock/bootblock.c:

https://review.coreboot.org/c/coreboot/+/42810/1/src/soc/amd/picasso/bootblo... PS1, Line 135: printk(BIOS_ERR,"ERROR: VBOOT workbuf not valid.\n"); space required after that ',' (ctx:VxV)

https://review.coreboot.org/c/coreboot/+/42810/1/src/soc/amd/picasso/bootblo... PS1, Line 137: printk(BIOS_DEBUG,"Signature: %#08x\n",*workbuf_location); space required after that ',' (ctx:VxV)

https://review.coreboot.org/c/coreboot/+/42810/1/src/soc/amd/picasso/bootblo... PS1, Line 137: printk(BIOS_DEBUG,"Signature: %#08x\n",*workbuf_location); space required after that ',' (ctx:VxO)

https://review.coreboot.org/c/coreboot/+/42810/1/src/soc/amd/picasso/bootblo... PS1, Line 137: printk(BIOS_DEBUG,"Signature: %#08x\n",*workbuf_location); space required before that '*' (ctx:OxV)

-- To view, visit https://review.coreboot.org/c/coreboot/+/42810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e Gerrit-Change-Number: 42810 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Roth martinroth@google.com Gerrit-CC: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 25 Jun 2020 16:28:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Martin Roth (Code Review)

5:16 p.m.

Hello build bot (Jenkins),

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

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

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

Change subject: soc/amd/picasso: Halt if workbuf is absent after psp_verstage ......................................................................

soc/amd/picasso: Halt if workbuf is absent after psp_verstage

Check for the workbuf in bootblock if psp_verstage is being used.

BUG=b:158124527 TEST=Build & boot Trembyle with psp_verstage

Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e --- M src/soc/amd/picasso/bootblock/bootblock.c 1 file changed, 17 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/42810/2

Raul Rangel (Code Review)

9:06 p.m.

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42810 )

Change subject: soc/amd/picasso: Halt if workbuf is absent after psp_verstage ......................................................................

Patch Set 2:

(3 comments)

https://review.coreboot.org/c/coreboot/+/42810/2/src/soc/amd/picasso/bootblo... File src/soc/amd/picasso/bootblock/bootblock.c:

https://review.coreboot.org/c/coreboot/+/42810/2/src/soc/amd/picasso/bootblo... PS2, Line 17: bot not

https://review.coreboot.org/c/coreboot/+/42810/2/src/soc/amd/picasso/bootblo... PS2, Line 132: #include <2struct.h> Why the include again?

https://review.coreboot.org/c/coreboot/+/42810/2/src/soc/amd/picasso/bootblo... PS2, Line 133: CONFIG_PSP_SHAREDMEM_BASE Can we use the symbol defined in the mamlayout.ld?

-- To view, visit https://review.coreboot.org/c/coreboot/+/42810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e Gerrit-Change-Number: 42810 Gerrit-PatchSet: 2 Gerrit-Owner: Martin Roth martinroth@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Raul Rangel rrangel@chromium.org Gerrit-Comment-Date: Thu, 25 Jun 2020 21:06:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Martin Roth (Code Review)

11:53 p.m.

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42810 )

Change subject: soc/amd/picasso: Halt if workbuf is absent after psp_verstage ......................................................................

Patch Set 2:

(3 comments)

https://review.coreboot.org/c/coreboot/+/42810/2/src/soc/amd/picasso/bootblo... File src/soc/amd/picasso/bootblock/bootblock.c:

https://review.coreboot.org/c/coreboot/+/42810/2/src/soc/amd/picasso/bootblo... PS2, Line 17: bot

...

not

Done

https://review.coreboot.org/c/coreboot/+/42810/2/src/soc/amd/picasso/bootblo... PS2, Line 132: #include <2struct.h>

...

Why the include again?

Just forgot to remove it when I moved it above. I initially tried putting it here so we only had one #if , but it blew up being included inside the function.

Removed.

https://review.coreboot.org/c/coreboot/+/42810/2/src/soc/amd/picasso/bootblo... PS2, Line 133: CONFIG_PSP_SHAREDMEM_BASE

...

Can we use the symbol defined in the mamlayout. […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/42810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e Gerrit-Change-Number: 42810 Gerrit-PatchSet: 2 Gerrit-Owner: Martin Roth martinroth@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Raul Rangel rrangel@chromium.org Gerrit-Comment-Date: Thu, 25 Jun 2020 23:53:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel rrangel@chromium.org Gerrit-MessageType: comment

Martin Roth (Code Review)

26 Jun 26 Jun

12:05 a.m.

Hello build bot (Jenkins),

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

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

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

Change subject: soc/amd/picasso: Halt if workbuf is absent after psp_verstage ......................................................................

soc/amd/picasso: Halt if workbuf is absent after psp_verstage

Check for the workbuf in bootblock if psp_verstage is being used.

BUG=b:158124527 TEST=Build & boot Trembyle with psp_verstage

Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e --- M src/soc/amd/picasso/bootblock/bootblock.c 1 file changed, 16 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/42810/3

build bot (Jenkins) (Code Review)

12:34 a.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42810 )

Change subject: soc/amd/picasso: Halt if workbuf is absent after psp_verstage ......................................................................

Patch Set 3:

(4 comments)

https://review.coreboot.org/c/coreboot/+/42810/3/src/soc/amd/picasso/bootblo... File src/soc/amd/picasso/bootblock/bootblock.c:

https://review.coreboot.org/c/coreboot/+/42810/3/src/soc/amd/picasso/bootblo... PS3, Line 134: printk(BIOS_ERR,"ERROR: VBOOT workbuf not valid.\n"); space required after that ',' (ctx:VxV)

https://review.coreboot.org/c/coreboot/+/42810/3/src/soc/amd/picasso/bootblo... PS3, Line 136: printk(BIOS_DEBUG,"Signature: %#08x\n",*(uint32_t *)_vboot2_work); space required after that ',' (ctx:VxV)

-- To view, visit https://review.coreboot.org/c/coreboot/+/42810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e Gerrit-Change-Number: 42810 Gerrit-PatchSet: 3 Gerrit-Owner: Martin Roth martinroth@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Raul Rangel rrangel@chromium.org Gerrit-Comment-Date: Fri, 26 Jun 2020 00:34:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Raul Rangel (Code Review)

4:20 p.m.

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42810 )

Change subject: soc/amd/picasso: Halt if workbuf is absent after psp_verstage ......................................................................

Patch Set 3: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/42810/3/src/soc/amd/picasso/bootblo... File src/soc/amd/picasso/bootblock/bootblock.c:

https://review.coreboot.org/c/coreboot/+/42810/3/src/soc/amd/picasso/bootblo... PS3, Line 134: printk(BIOS_ERR,"ERROR: VBOOT workbuf not valid.\n");

...

space required after that ',' (ctx:VxV)

clang-format please

-- To view, visit https://review.coreboot.org/c/coreboot/+/42810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e Gerrit-Change-Number: 42810 Gerrit-PatchSet: 3 Gerrit-Owner: Martin Roth martinroth@google.com Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Fri, 26 Jun 2020 16:20:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: comment

Martin Roth (Code Review)

8 Jul 8 Jul

7:49 p.m.

Hello build bot (Jenkins), Raul Rangel,

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

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

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

Change subject: soc/amd/picasso: Halt if workbuf is absent after psp_verstage ......................................................................

soc/amd/picasso: Halt if workbuf is absent after psp_verstage

Check for the workbuf in bootblock if psp_verstage is being used.

BUG=b:158124527 TEST=Build & boot Trembyle with psp_verstage

Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e --- M src/soc/amd/picasso/bootblock/bootblock.c 1 file changed, 16 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/42810/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/42810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e Gerrit-Change-Number: 42810 Gerrit-PatchSet: 4 Gerrit-Owner: Martin Roth martinroth@google.com Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Martin Roth (Code Review)

7:51 p.m.

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42810 )

Change subject: soc/amd/picasso: Halt if workbuf is absent after psp_verstage ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42810/3/src/soc/amd/picasso/bootblo... File src/soc/amd/picasso/bootblock/bootblock.c:

https://review.coreboot.org/c/coreboot/+/42810/3/src/soc/amd/picasso/bootblo... PS3, Line 134: printk(BIOS_ERR,"ERROR: VBOOT workbuf not valid.\n");

...

clang-format please

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/42810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e Gerrit-Change-Number: 42810 Gerrit-PatchSet: 4 Gerrit-Owner: Martin Roth martinroth@google.com Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 08 Jul 2020 19:51:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: build bot (Jenkins) no-reply@coreboot.org Comment-In-Reply-To: Raul Rangel rrangel@chromium.org Gerrit-MessageType: comment

Raul Rangel (Code Review)

7:59 p.m.

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42810 )

Change subject: soc/amd/picasso: Halt if workbuf is absent after psp_verstage ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/42810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e Gerrit-Change-Number: 42810 Gerrit-PatchSet: 4 Gerrit-Owner: Martin Roth martinroth@google.com Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 08 Jul 2020 19:59:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Martin Roth (Code Review)

8:46 p.m.

Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42810 )

Change subject: soc/amd/picasso: Halt if workbuf is absent after psp_verstage ......................................................................

soc/amd/picasso: Halt if workbuf is absent after psp_verstage

Check for the workbuf in bootblock if psp_verstage is being used.

BUG=b:158124527 TEST=Build & boot Trembyle with psp_verstage

Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e Reviewed-on: https://review.coreboot.org/c/coreboot/+/42810 Reviewed-by: Raul Rangel rrangel@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/amd/picasso/bootblock/bootblock.c 1 file changed, 16 insertions(+), 0 deletions(-)

Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved

diff --git a/src/soc/amd/picasso/bootblock/bootblock.c b/src/soc/amd/picasso/bootblock/bootblock.c index a3935cc..556fbad 100644 --- a/src/soc/amd/picasso/bootblock/bootblock.c +++ b/src/soc/amd/picasso/bootblock/bootblock.c @@ -13,6 +13,12 @@ #include <soc/i2c.h> #include <amdblocks/amd_pci_mmconf.h> #include <acpi/acpi.h> +#include <security/vboot/symbols.h> + +/* vboot includes directory may not be in include path if vboot is not enabled */ +#if CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) +#include <2struct.h> +#endif

asmlinkage void bootblock_resume_entry(void);

@@ -123,5 +129,15 @@ u32 val = cpuid_eax(1); printk(BIOS_DEBUG, "Family_Model: %08x\n", val);

+#if CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) + if (*(uint32_t *)_vboot2_work != VB2_SHARED_DATA_MAGIC) { + printk(BIOS_ERR, "ERROR: VBOOT workbuf not valid.\n"); + + printk(BIOS_DEBUG, "Signature: %#08x\n", *(uint32_t *)_vboot2_work); + + die("Halting.\n"); + } +#endif + fch_early_init(); }

-- To view, visit https://review.coreboot.org/c/coreboot/+/42810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ec8d2c953bce4c44cde5102d2765e0ab9b5875e Gerrit-Change-Number: 42810 Gerrit-PatchSet: 5 Gerrit-Owner: Martin Roth martinroth@google.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: merged

1409

days inactive

1422

days old

coreboot-gerrit@coreboot.org

11 comments

3 participants

tags (0)

participants (3)

build bot (Jenkins) (Code Review)
Martin Roth (Code Review)
Raul Rangel (Code Review)