Change in coreboot[master]: x86/lapic: Set EXTINT on BSP only

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Felix Held (Code Review)

16 Jun 2020 16 Jun '20

12:47 p.m.

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

x86/lapic: Set EXTINT on BSP only

aac79e0b8f4777f8a912ccdfc483755b7a4da52c changed the LAPIC delivery mode for both the BSP and the APs. The change on the APs caused a regression that massively slows down the boot process. Configuring the AP's LAPIC delivery mode to fixed mode fixes this regression.

Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/cpu/x86/lapic/lapic.c 1 file changed, 2 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/42434/1

diff --git a/src/cpu/x86/lapic/lapic.c b/src/cpu/x86/lapic/lapic.c index 653c3b2..2e04464 100644 --- a/src/cpu/x86/lapic/lapic.c +++ b/src/cpu/x86/lapic/lapic.c @@ -31,6 +31,8 @@ (LAPIC_LVT_REMOTE_IRR | LAPIC_SEND_PENDING); if (boot_cpu()) lvt0_val = SET_LAPIC_DELIVERY_MODE(lvt0_val, LAPIC_MODE_EXINT); + else + lvt0_val = SET_LAPIC_DELIVERY_MODE(lvt0_val, LAPIC_MODE_FIXED); lapic_write_around(LAPIC_LVT0, lvt0_val);

lapic_write_around(LAPIC_LVT1,

Attachments:

attachment.htm (text/html — 2.7 KB)

Show replies by date

Angel Pons (Code Review)

16 Jun 16 Jun

12:59 p.m.

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

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42434/1/src/cpu/x86/lapic/lapic.c File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/1/src/cpu/x86/lapic/lapic.c@2... PS1, Line 27: lvt0_val dead assignment

Angel Pons (Code Review)

1:02 p.m.

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

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42434/1/src/cpu/x86/lapic/lapic.c File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/1/src/cpu/x86/lapic/lapic.c@2... PS1, Line 27: lvt0_val

...

dead assignment

someone please slap me

Felix Held (Code Review)

1:02 p.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42434/1/src/cpu/x86/lapic/lapic.c File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/1/src/cpu/x86/lapic/lapic.c@2... PS1, Line 27: lvt0_val

...

dead assignment

not a dead assignment. lvt0_val is a parameter in SET_LAPIC_DELIVERY_MODE

Aaron Durbin (Code Review)

1:05 p.m.

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 1: Code-Review+2

I thought the !boot_cpu() case was handled in the previous patch.

Felix Held (Code Review)

1:06 p.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 1:

...

Patch Set 1: Code-Review+2

I thought the !boot_cpu() case was handled in the previous patch.

it was until it was suggested to not mask bits in the non-BSP case

Raul Rangel (Code Review)

1:10 p.m.

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

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 1: Code-Review+2

Angel Pons (Code Review)

1:18 p.m.

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

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 1: Code-Review-1

Let's not rush things. This is used by pretty much all x86 platforms. Has anyone tested them?

Angel Pons (Code Review)

1:24 p.m.

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

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42434/1/src/cpu/x86/lapic/lapic.c File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/1/src/cpu/x86/lapic/lapic.c@2... PS1, Line 27: lvt0_val

...

not a dead assignment. […]

That macro has been unused for decades, btw. Probably because it's completely opaque. I would prefer using |=

Felix Held (Code Review)

1:27 p.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 1: Code-Review-2

...

Let's not rush things. This is used by pretty much all x86 platforms. Has anyone tested them?

ok, then let's just merge the revert https://review.coreboot.org/c/coreboot/+/42166 for now and sort out the proper fix later. also hard to say what the exact issue is here; I'm currently about 4 layers deep into regressions

Felix Held (Code Review)

1:30 p.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42434/1/src/cpu/x86/lapic/lapic.c File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/1/src/cpu/x86/lapic/lapic.c@2... PS1, Line 27: lvt0_val

...

That macro has been unused for decades, btw. Probably because it's completely opaque. […]

i preferred this version https://review.coreboot.org/c/coreboot/+/42219/6/src/cpu/x86/lapic/lapic.c

Angel Pons (Code Review)

1:48 p.m.

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

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 1:

(1 comment)

...

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42434/1/src/cpu/x86/lapic/lapic.c File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/1/src/cpu/x86/lapic/lapic.c@2... PS1, Line 27: lvt0_val

...

i preferred this version https://review.coreboot.org/c/coreboot/+/42219/6/src/cpu/x86/lapic/lapic. […]

Same

Felix Held (Code Review)

2:03 p.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 1:

while this patch improves the boot time a bit, a complete revert still results in a faster boot (SeaBIOS booting kubuntu from a USB stick)

Bao Zheng (Code Review)

18 Jun 18 Jun

2:33 a.m.

Bao Zheng has uploaded a new patch set (#2) to the change originally created by Felix Held. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

x86/lapic: Set EXTINT on BSP only

Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/cpu/x86/lapic/lapic.c 1 file changed, 2 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/42434/2

Bao Zheng (Code Review)

2:38 a.m.

Bao Zheng has uploaded a new patch set (#3) to the change originally created by Felix Held. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

x86/lapic: Set EXTINT on BSP only

Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/cpu/x86/lapic/lapic.c 1 file changed, 12 insertions(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/42434/3

Bao Zheng (Code Review)

2:48 a.m.

Bao Zheng has uploaded a new patch set (#4) to the change originally created by Felix Held. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

x86/lapic: Set EXTINT on BSP only

Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/cpu/x86/lapic/lapic.c 1 file changed, 13 insertions(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/42434/4

Angel Pons (Code Review)

4:44 a.m.

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

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/c/coreboot/+/42434/4/src/cpu/x86/lapic/lapic.c File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/4/src/cpu/x86/lapic/lapic.c@9 PS4, Line 9: uint32_t unsigned long

https://review.coreboot.org/c/coreboot/+/42434/4/src/cpu/x86/lapic/lapic.c@1... PS4, Line 13: uint32_t unsigned long

Angel Pons (Code Review)

4:56 a.m.

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

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/c/coreboot/+/42434/4/src/cpu/x86/lapic/lapic.c File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/4/src/cpu/x86/lapic/lapic.c@9 PS4, Line 9: uint32_t

...

unsigned long

actually, the lapic functions are wrong

https://review.coreboot.org/c/coreboot/+/42434/4/src/cpu/x86/lapic/lapic.c@1... PS4, Line 13: uint32_t

...

unsigned long

actually, the lapic functions are wrong

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 18 Jun 2020 11:56:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Felix Held (Code Review)

6:17 a.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 4: -Code-Review

since the +2s are gone, i'll remove my -2 that I added so this didn't get fast-path merged instead of the revert.

Angel Pons (Code Review)

6:31 a.m.

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

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 4:

...

Patch Set 4: -Code-Review

since the +2s are gone, i'll remove my -2 that I added so this didn't get fast-path merged instead of the revert.

Sounds good

Angel Pons (Code Review)

6:31 a.m.

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

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42434/4//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/42434/4//COMMIT_MSG@9 PS4, Line 9: aac79e0b8f4777f8a912ccdfc483755b7a4da52c changed the LAPIC delivery mode This would need to be updated, I guess? CB:42219 was reverted

Felix Held (Code Review)

6:45 a.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 4:

https://review.coreboot.org/c/coreboot/+/36777 is an APIC-related cleanup patch; might be good to have a look at that one

Paul Menzel (Code Review)

13 Jul 13 Jul

12:16 a.m.

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

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42434/4//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/42434/4//COMMIT_MSG@9 PS4, Line 9: aac79e0b8f4777f8a912ccdfc483755b7a4da52c Please use *commit aac79e0b (x86/lapic: Set EXTINT on BSP only)* in the future. Then Gerrit marks it up as a link.

Michał Żygowski (Code Review)

25 Aug 25 Aug

4:48 a.m.

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 4:

Felix, I got an suggestion to have LINT0 set as ExtINT on BSP, but all APs should mask ExtINT then (not set to fixed delivery). It should be the correct solution. Credits to Andrew Cooper.

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 25 Aug 2020 11:48:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Felix Held (Code Review)

4:39 p.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 4:

...

Patch Set 4:

Felix, I got an suggestion to have LINT0 set as ExtINT on BSP, but all APs should mask ExtINT then (not set to fixed delivery). It should be the correct solution. Credits to Andrew Cooper.

Masking vs. masking and then setting it to fixed delivery mode should do the same though, since LAPIC_DELIVERY_MODE_FIXED is defined as 0 << 8. Or am i missing something here?

When applying the patch I still get some additional delay from seabios starting to the first screen of the live Linux system on the USB stick where i can optionally change the kernel parameters on Mandolin; haven't tested on my x230 yet.

I'm not sure if this really is a coreboot issue or if the issue is on the seabios side or even a bootloader issue. I don't see an obvious problem with the coreboot patch though, but this part of the x86 arch is definitely not something I'm too familiar with.

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 25 Aug 2020 23:39:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Michał Żygowski (Code Review)

27 Aug 27 Aug

2:40 a.m.

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 4:

...

Patch Set 4:

...
Patch Set 4:

Felix, I got an suggestion to have LINT0 set as ExtINT on BSP, but all APs should mask ExtINT then (not set to fixed delivery). It should be the correct solution. Credits to Andrew Cooper.

Masking vs. masking and then setting it to fixed delivery mode should do the same though, since LAPIC_DELIVERY_MODE_FIXED is defined as 0 << 8. Or am i missing something here?

When applying the patch I still get some additional delay from seabios starting to the first screen of the live Linux system on the USB stick where i can optionally change the kernel parameters on Mandolin; haven't tested on my x230 yet.

I'm not sure if this really is a coreboot issue or if the issue is on the seabios side or even a bootloader issue. I don't see an obvious problem with the coreboot patch though, but this part of the x86 arch is definitely not something I'm too familiar with.

LINT0 should be set as masked (bit 16) on APs. The current patchset has lvt0_mask with LAPIC_LVT_MASKED (1<<16) set, but when used with lapic_write_around, it is negated. It results in bit 16 being cleared on both BSP and APs. That should be the issue:

if (boot_cpu()) lvt0_val |= LAPIC_DELIVERY_MODE_EXTINT; else - lvt0_val |= LAPIC_DELIVERY_MODE_FIXED; + lvt0_val |= LAPIC_DELIVERY_MODE_FIXED | LAPIC_LVT_MASKED;

I will test on few platforms here and there to see whether it changes anything. What is the scale of the delay between SeaBIOS and Linux screen?

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 27 Aug 2020 09:40:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Michał Żygowski (Code Review)

3:25 a.m.

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 4:

...

I will test on few platforms here and there to see whether it changes anything. What is the scale of the delay between SeaBIOS and Linux screen?

I took a quick test on Protectli FW6. With the change proposed by me above, the time between SeaBIOS screen and Linux login screen is shorter (even by few seconds). Let me know what your results are. Also this issue is somehow related the the Xen booting problem we had on apu2: https://github.com/pcengines/apu2-documentation/issues/109 I will run some statistical tests whether it fixes the problem. Hopefully this will be an useful input.

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 27 Aug 2020 10:25:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Felix Held (Code Review)

9 Sep 9 Sep

7:29 a.m.

New subject: Change in coreboot[master]: [WIP] x86/lapic: Set EXTINT on BSP only

Hello build bot (Jenkins), Bao Zheng, Raul Rangel, Nico Huber, Furquan Shaikh, Zheng Bao, Angel Pons, Aaron Durbin,

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

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

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

Change subject: [WIP] x86/lapic: Set EXTINT on BSP only ......................................................................

[WIP] x86/lapic: Set EXTINT on BSP only

Still causes a massive slowdown during boot. Might be that syslinux on the ubuntu 18.04 live system USB stick just expects a different lapic configuration than coreboot does with this patch applied.

Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/cpu/x86/lapic/lapic.c 1 file changed, 13 insertions(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/42434/5

Michał Żygowski (Code Review)

10 Sep 10 Sep

2:54 a.m.

New subject: Change in coreboot[master]: [WIP] x86/lapic: Set EXTINT on BSP only

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: [WIP] x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 5: Code-Review+1

In this configuration, the IRQ errors in (xl) dmesg in Linux and/or Xen disappeared. Boot speed looks still the same, so it should be the right solution IMO. But let's wait for others to comment on it.

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 10 Sep 2020 09:54:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Felix Held (Code Review)

11:24 a.m.

New subject: Change in coreboot[master]: [WIP] x86/lapic: Set EXTINT on BSP only

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: [WIP] x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 5:

just tested on my x230; applying the current patch didn't introduce a regression there. The boot configuration there is coreboot -> seabios -> grub2 in MBR -> some probably not too recent fedora linux

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 10 Sep 2020 18:24:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Nico Huber (Code Review)

13 Sep 13 Sep

7:56 a.m.

New subject: Change in coreboot[master]: [WIP] x86/lapic: Set EXTINT on BSP only

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

Change subject: [WIP] x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 5:

(6 comments)

Now that I looked deeper into it, the MultiProcessor Specification looks good, with quite nice pictures. One just needs to know which to look at (Virtual Wire Mode via Local APIC, I believe).

I still come to the same conclusion, though. We simply have to mask the interrupts on the APs (or leave their registers alone). Did anybody ever try if that fixes your issues?

https://review.coreboot.org/c/coreboot/+/42434/5//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/42434/5//COMMIT_MSG@11 PS5, Line 11: configuration than coreboot does with this patch applied. Is this still up-to-date? I don't see how the current version could cause such. Also don't see why syslinux would expect anything wrt. to LAPICs.

https://review.coreboot.org/c/coreboot/+/42434/5/src/cpu/x86/lapic/lapic.c File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/5/src/cpu/x86/lapic/lapic.c@7 PS5, Line 7: void do_lapic_init(void) What we (try to) do here is configuring `Virtual Wire Mode via Local APIC` as found in section 3.6.2.2 `MultiProcessor Specification 1.4`. I think that deserves a comment so that we don't need to read the whole spec everytime we want to understand it.

We could also mention what assumptions are made based on the spec:

* NMI is connected to LINT1 of all APICs. * INTR of the legacy master PIC is connected as edge-triggered to LINT0 of all APICs. * LINT0 needs to be configured as EXTINT because of the interrupt acknowlegde needs of the legacy PICs. * Interrupts on APs need to be disabled.

https://review.coreboot.org/c/coreboot/+/42434/5/src/cpu/x86/lapic/lapic.c@9 PS5, Line 9: lvt0_mask It's the same for LVT1, should we use it there too?

https://review.coreboot.org/c/coreboot/+/42434/5/src/cpu/x86/lapic/lapic.c@1... PS5, Line 13: uint32_t lvt0_val = LAPIC_LVT_REMOTE_IRR | LAPIC_SEND_PENDING; Maybe comment that this is the mask and value (well minus EXTINT but see below) used by `Example A-1 of the MultiProcessor Specification`.

https://review.coreboot.org/c/coreboot/+/42434/5/src/cpu/x86/lapic/lapic.c@3... PS5, Line 34: lvt0_val |= LAPIC_DELIVERY_MODE_FIXED | LAPIC_LVT_MASKED; I was much surprised when I learned what EXTINT means. It's about the way interrupts are acknowledged for the legacy PICs. I don't see how this part of the hardware is wired can differ for the APs.

Now it's hard to argue what values to set for disabled interrupts. But as we mimic the BSP configuration in all the other bits, why not these too? A mere

if (!boot_cpu()) lvt0_val |= LAPIC_LVT_MASKED;

would be more clear, IMHO.

Not setting anything on the APs at all might also work but I couldn't find any specification of the reset defaults for these registers.

https://review.coreboot.org/c/coreboot/+/42434/5/src/cpu/x86/lapic/lapic.c@4... PS5, Line 45: ); The spec (B.1) says

"The APs are in a halted condition with interrupts disabled. This means that the AP’s local APICs are passively monitoring the APIC bus and will react only to INIT or STARTUP interprocessor interrupts (IPIs)."

I think we should mask this one on APs too.

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 13 Sep 2020 14:56:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Paul Menzel (Code Review)

18 May 18 May

11:35 p.m.

New subject: Change in coreboot[master]: [WIP] x86/lapic: Set EXTINT on BSP only

Attention is currently required from: Felix Held. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: [WIP] x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 5:

(1 comment)

Patchset:

PS5: I just saw this again on the Asus F2A85-M PRO with SeaBIOS. So, adding the comment to gently bump the change-set. ;-)

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Wed, 19 May 2021 06:35:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

20 Jun 20 Jun

10:55 a.m.

New subject: Change in coreboot[master]: [WIP] x86/lapic: Set EXTINT on BSP only

Attention is currently required from: Nico Huber, Felix Held. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: [WIP] x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 5:

(3 comments)

Patchset:

PS5: Felix, I'd like to reuse Change-Id from this with CB:55701.

File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/comment/148dfab5_032bf5e2 PS5, Line 29: | LAPIC_SPIV_ENABLE); Sets spurious interrupt to vector 0? And LVT0 for AP used vector 0 too?

10.9 SPURIOUS INTERRUPT

Do not program an LVT or IOAPIC RTE with a spurious vector even if you set the mask bit. A spurious vector ISR does not do an EOI. If for some reason an interrupt is generated by an LVT or RTE entry, the bit in the in-service register will be left set for the spurious vector. This will mask all interrupts at the same or lower priority

https://review.coreboot.org/c/coreboot/+/42434/comment/7537f320_ec347774 PS5, Line 34: lvt0_val |= LAPIC_DELIVERY_MODE_FIXED | LAPIC_LVT_MASKED;

...

I was much surprised when I learned what EXTINT means. It's about the […]

10.5.2 Valid Interrupt Vectors

When an interrupt vector in the range of 0 to 15 is sent or received through the local APIC, the APIC indicates an illegal vector in its Error Status Register (see Section 10.5.3, “Error Handling”)

When an illegal vector value (0 to 15) is written to an LVT entry and the delivery mode is Fixed (bits 8-11 equal 0), the APIC may signal an illegal vector error, without regard to whether the mask bit is set or whether an interrupt is actually seen on the input.

Figure 10-8. Local Vector Table (LVT) Value After Reset: 0001 0000H

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Sun, 20 Jun 2021 17:55:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Felix Held (Code Review)

21 Jun 21 Jun

6:16 a.m.

New subject: Change in coreboot[master]: [WIP] x86/lapic: Set EXTINT on BSP only

Attention is currently required from: Nico Huber, Kyösti Mälkki. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: [WIP] x86/lapic: Set EXTINT on BSP only ......................................................................

Patch Set 5:

(1 comment)

Patchset:

PS5:

...

Felix, I'd like to reuse Change-Id from this with CB:55701.

feel free to take over this change-id

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Comment-Date: Mon, 21 Jun 2021 13:16:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

23 Jun 23 Jun

9:45 p.m.

New subject: Change in coreboot[master]: [WIP] cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Nico Huber, Kyösti Mälkki. Kyösti Mälkki has uploaded a new patch set (#6) to the change originally created by Felix Held. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: [WIP] cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

[WIP] cpu/x86/lapic: Only deliver ExtINT to BSP

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/42434/6

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 6 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: newpatchset

build bot (Jenkins) (Code Review)

10:26 p.m.

New subject: Change in coreboot[master]: [WIP] cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Nico Huber, Kyösti Mälkki. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: [WIP] cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 6:

(1 comment)

File src/cpu/x86/lapic/lapic.c:

Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-122642): https://review.coreboot.org/c/coreboot/+/42434/comment/5d9b6fbb_fe004c6c PS6, Line 62: lapic_update32(LAPIC_LVT0, ~mask, LAPIC_LVT_MASKED | LAPIC_DELIVERY_MODE_EXTINT); line over 96 characters

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 6 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Comment-Date: Thu, 24 Jun 2021 05:26:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

build bot (Jenkins) (Code Review)

25 Jun 25 Jun

8:17 a.m.

New subject: Change in coreboot[master]: [WIP] cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Nico Huber, Kyösti Mälkki. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: [WIP] cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 7:

(1 comment)

File src/cpu/x86/lapic/lapic.c:

Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-122746): https://review.coreboot.org/c/coreboot/+/42434/comment/965bf3b1_a1df225f PS7, Line 63: lapic_update32(LAPIC_LVT0, ~mask, LAPIC_LVT_MASKED | LAPIC_DELIVERY_MODE_EXTINT); line over 96 characters

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 7 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Comment-Date: Fri, 25 Jun 2021 15:17:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

build bot (Jenkins) (Code Review)

29 Jun 29 Jun

11:59 p.m.

New subject: Change in coreboot[master]: [WIP] cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Nico Huber, Kyösti Mälkki. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: [WIP] cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 8:

(1 comment)

File src/cpu/x86/lapic/lapic.c:

Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-123070): https://review.coreboot.org/c/coreboot/+/42434/comment/02e2fbe2_a26ceaf0 PS8, Line 63: lapic_update32(LAPIC_LVT0, ~mask, LAPIC_LVT_MASKED | LAPIC_DELIVERY_MODE_EXTINT); line over 96 characters

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 8 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Comment-Date: Wed, 30 Jun 2021 06:59:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

14 Aug 14 Aug

2:20 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Nico Huber, Kyösti Mälkki. Kyösti Mälkki has uploaded a new patch set (#9) to the change originally created by Felix Held. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

cpu/x86/lapic: Only deliver ExtINT to BSP

ExtINT is related to external PIC mode i8259 interrupts, they should be delivered to one CPU (BSP) only.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/42434/9

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 9 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: newpatchset

Angel Pons (Code Review)

13 Oct 13 Oct

3:26 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Nico Huber, Paul Menzel, Kyösti Mälkki, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 11: Code-Review+1

(3 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/42434/comment/8c5200dd_091892af PS4, Line 9: aac79e0b8f4777f8a912ccdfc483755b7a4da52c changed the LAPIC delivery mode

...

This would need to be updated, I guess? CB:42219 was reverted

Done

https://review.coreboot.org/c/coreboot/+/42434/comment/3c6c2d59_7bce9aa2 PS4, Line 9: aac79e0b8f4777f8a912ccdfc483755b7a4da52c

...

Please use *commit aac79e0b (x86/lapic: Set EXTINT on BSP only)* in the future. […]

No longer applies

Commit Message:

https://review.coreboot.org/c/coreboot/+/42434/comment/bedb00cc_a2b3e6bf PS5, Line 11: configuration than coreboot does with this patch applied.

...

Is this still up-to-date? I don't see how the current version could […]

Anything else to do? The commit message was updated.

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 11 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Wed, 13 Oct 2021 10:26:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Paul Menzel paulepanter@mailbox.org Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

14 Oct 14 Oct

3:53 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Nico Huber, Paul Menzel, Felix Held. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 11:

(5 comments)

File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/comment/83a0fe67_d42528f6 PS5, Line 9: lvt0_mask

...

It's the same for LVT1, should we use it there too?

Done

https://review.coreboot.org/c/coreboot/+/42434/comment/2d489455_9e781fb6 PS5, Line 13: uint32_t lvt0_val = LAPIC_LVT_REMOTE_IRR | LAPIC_SEND_PENDING;

...

Maybe comment that this is the mask and value (well minus EXTINT but see below) used by […]

Ack

https://review.coreboot.org/c/coreboot/+/42434/comment/8b1e9b43_ea9293bb PS5, Line 29: | LAPIC_SPIV_ENABLE);

...

Sets spurious interrupt to vector 0? And LVT0 for AP used vector 0 too? […]

Ack

https://review.coreboot.org/c/coreboot/+/42434/comment/b0f33051_6e4fbacb PS5, Line 34: lvt0_val |= LAPIC_DELIVERY_MODE_FIXED | LAPIC_LVT_MASKED;

...

10.5.2 Valid Interrupt Vectors […]

Ack

https://review.coreboot.org/c/coreboot/+/42434/comment/073601d5_35b02060 PS5, Line 45: );

...

The spec (B.1) says […]

I cannot figure out what "this one" here referred to.

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 11 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Thu, 14 Oct 2021 10:53:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: comment

Nico Huber (Code Review)

4:37 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Paul Menzel, Angel Pons, Kyösti Mälkki, Felix Held. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 12:

(2 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/42434/comment/bae10c1c_61a843f1 PS5, Line 11: configuration than coreboot does with this patch applied.

...

Anything else to do? The commit message was updated.

Ack

File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/comment/b9e4671d_ac882709 PS5, Line 45: );

...

I cannot figure out what "this one" here referred to.

LVT1

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 12 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Angel Pons th3fanbus@gmail.com Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Thu, 14 Oct 2021 11:37:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: comment

Nico Huber (Code Review)

4:50 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Paul Menzel, Angel Pons, Kyösti Mälkki, Felix Held. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 12:

(1 comment)

File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/comment/d5652b10_f0b8fb77 PS5, Line 45: );

...

LVT1

Turning it into a question: If we don't want LVT0 on APs, why do we want LVT1 (NMIs) on APs?

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 12 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Angel Pons th3fanbus@gmail.com Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Thu, 14 Oct 2021 11:50:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

15 Oct 15 Oct

3:04 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Nico Huber, Paul Menzel, Angel Pons, Felix Held. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 13:

(1 comment)

File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/comment/4c5f2b78_966be864 PS5, Line 45: );

...

Turning it into a question: If we don't want LVT0 on APs, why do we want […]

Regarding spec B.1, PARALLEL_MP will send some IPIs and only some CPU models return to wait for INIT/STARTUP IPI state. So APs can enter NMI handler. I would have assumed that any NMI signal connected to LINT1 pins would be shared between CPUs, so delivering it to APs too would sounds like correct thing to do. It's closer to the state prior to enabling the LAPIC, while there is still possibility NMI handler is not installed at all.

For ExtINT SDK says that it should be routed to one CPU only. I think legacy PIC i8259 controller gets confused otherwise.

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 13 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Angel Pons th3fanbus@gmail.com Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Fri, 15 Oct 2021 10:04:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: comment

Nico Huber (Code Review)

3:17 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Paul Menzel, Angel Pons, Kyösti Mälkki, Felix Held. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 13:

(1 comment)

File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/comment/4c30bd86_98e66666 PS5, Line 45: );

...

Regarding spec B. […]

Ah, so do we install an NMI handler on APs? I would have thought it's that simple: if we have a handler, enable it; if not, mask LVT1. But I'm really an amateur in this area.

I think you are right that the LVT0 situation is quite different. ExtINT is for one APIC only, no matter the handler. So marking this as resolved.

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 13 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Angel Pons th3fanbus@gmail.com Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Fri, 15 Oct 2021 10:17:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: comment

Angel Pons (Code Review)

3:20 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Paul Menzel, Kyösti Mälkki, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 13: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 13 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Fri, 15 Oct 2021 10:20:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons (Code Review)

3:21 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Nico Huber, Paul Menzel, Kyösti Mälkki, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 13:

(1 comment)

File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/comment/f1f96685_68e8a22b PS5, Line 7: void do_lapic_init(void)

...

What we (try to) do here is configuring `Virtual Wire Mode via Local APIC` as found […]

Anything to do?

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 13 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Fri, 15 Oct 2021 10:21:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Nico Huber (Code Review)

3:27 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Paul Menzel, Kyösti Mälkki, Felix Held. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 13:

(1 comment)

File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/comment/65a9076d_8b0f1ee5 PS5, Line 7: void do_lapic_init(void)

...

Anything to do?

I guess not as this comment was rebased out of context ;) We could still add a comment about it, but given that this is a whole patch train now, it should be a separate commit.

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 13 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Fri, 15 Oct 2021 10:27:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Nico Huber (Code Review)

3:33 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Paul Menzel, Kyösti Mälkki, Felix Held. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 13: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 13 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Fri, 15 Oct 2021 10:33:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

5:48 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Paul Menzel, Felix Held. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 13:

(2 comments)

Patchset:

PS13: Felix, does this take care of CB:42219 error messages without added delays?

File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/comment/4025b07a_4b79cfca PS5, Line 45: );

...

Ah, so do we install an NMI handler on APs? I would have thought it's […]

Do we install _any_ interrupt handlers in coreboot proper?

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 13 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Fri, 15 Oct 2021 12:48:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: comment

Angel Pons (Code Review)

5:57 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Paul Menzel, Kyösti Mälkki, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 13:

(1 comment)

File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/comment/a611cd6c_e831e2ab PS5, Line 45: );

...

Do we install _any_ interrupt handlers in coreboot proper?

I don't think we do. Moreover, the `nmi` CMOS option is broken since the CMOS code clobbers the NMI enable bit. I know it doesn't disable all NMI sources, but controls many of them.

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 13 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Fri, 15 Oct 2021 12:57:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: comment

Nico Huber (Code Review)

6:09 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Paul Menzel, Kyösti Mälkki, Felix Held. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 13:

(1 comment)

File src/cpu/x86/lapic/lapic.c:

https://review.coreboot.org/c/coreboot/+/42434/comment/8e3eabd9_ef000fa4 PS5, Line 45: );

...

I don't think we do. […]

Well, if that counts, there is an exception handler somewhere I'm sure (I only look at it about every 3 years).

IIRC, the CMOS option was about the state we hand over to the OS and was likely only added because of a very specific use case (of us, secunet).

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 13 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Attention: Felix Held felix-coreboot@felixheld.de Gerrit-Comment-Date: Fri, 15 Oct 2021 13:09:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: comment

Felix Held (Code Review)

16 Oct 16 Oct

11:13 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Paul Menzel, Kyösti Mälkki. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 13:

(1 comment)

Patchset:

PS13:

...

Felix, does this take care of CB:42219 error messages without added delays?

I'll run a test on Mandolin and also verify that it won't cause a regression on ivy bridge like one of the early attempts to fix this did. fyi: since i get tons of notification emails from gerrit, i don't regularly look at those any more, so pinging me on irc might be a good idea when i should quickly react/test/review

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 13 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Comment-Date: Sat, 16 Oct 2021 18:13:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: comment

Felix Held (Code Review)

7:44 p.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Attention is currently required from: Paul Menzel, Kyösti Mälkki. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 14: Code-Review+2

(2 comments)

Patchset:

PS13:

...

I'll run a test on Mandolin and also verify that it won't cause a regression on ivy bridge like one […]

I've verified that this fixes the spurious IRQs on the APs on amd/mandolin using picasso and doesn't introduce any delays on lenovo/x230 using ivy bridge

Patchset:

PS14: thanks for fixing this!

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 14 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Comment-Date: Sun, 17 Oct 2021 02:44:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Comment-In-Reply-To: Felix Held felix-coreboot@felixheld.de Gerrit-MessageType: comment

Felix Held (Code Review)

7:45 p.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42434 )

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

cpu/x86/lapic: Only deliver ExtINT to BSP

ExtINT is related to external PIC mode i8259 interrupts, they should be delivered to one CPU (BSP) only.

Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Signed-off-by: Felix Held felix-coreboot@felixheld.de Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42434 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Nico Huber nico.h@gmx.de --- M src/cpu/x86/lapic/lapic.c 1 file changed, 6 insertions(+), 1 deletion(-)

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

diff --git a/src/cpu/x86/lapic/lapic.c b/src/cpu/x86/lapic/lapic.c index 58e4524..b4d3c4d 100644 --- a/src/cpu/x86/lapic/lapic.c +++ b/src/cpu/x86/lapic/lapic.c @@ -5,6 +5,7 @@ #include <cpu/x86/lapic_def.h> #include <cpu/x86/msr.h> #include <console/console.h> +#include <smp/node.h> #include <stdint.h>

void enable_lapic(void) @@ -55,7 +56,11 @@ uint32_t mask = LAPIC_LVT_MASKED | LAPIC_LVT_LEVEL_TRIGGER | LAPIC_INPUT_POLARITY | LAPIC_DELIVERY_MODE_MASK;

- lapic_update32(LAPIC_LVT0, ~mask, LAPIC_DELIVERY_MODE_EXTINT); + if (boot_cpu()) + lapic_update32(LAPIC_LVT0, ~mask, LAPIC_DELIVERY_MODE_EXTINT); + else + lapic_update32(LAPIC_LVT0, ~mask, LAPIC_LVT_MASKED | + LAPIC_DELIVERY_MODE_EXTINT);

lapic_update32(LAPIC_LVT1, ~mask, LAPIC_DELIVERY_MODE_NMI); }

Angel Pons (Code Review)

17 Oct 17 Oct

2:49 a.m.

New subject: Change in coreboot[master]: cpu/x86/lapic: Only deliver ExtINT to BSP

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

Change subject: cpu/x86/lapic: Only deliver ExtINT to BSP ......................................................................

Patch Set 15:

(1 comment)

Patchset:

PS13:

...

I've verified that this fixes the spurious IRQs on the APs on amd/mandolin using picasso and doesn't […]

I forgot to mention I boot-tested this on Haswell (Asrock B85M Pro4) and didn't see any regressions, but I didn't try booting an OS (I just checked if tianocore still works).

-- To view, visit https://review.coreboot.org/c/coreboot/+/42434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba Gerrit-Change-Number: 42434 Gerrit-PatchSet: 15 Gerrit-Owner: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Bao Zheng fishbaozi@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Zheng Bao zheng.bao@amd.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jason Glenesk jason.glenesk@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Comment-Date: Sun, 17 Oct 2021 09:49:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Comment-In-Reply-To: Felix Held felix-coreboot@felixheld.de Gerrit-MessageType: comment

1127

days inactive

1615

days old

coreboot-gerrit@coreboot.org

55 comments

10 participants

tags (0)

participants (10)

Aaron Durbin (Code Review)
Angel Pons (Code Review)
Bao Zheng (Code Review)
build bot (Jenkins) (Code Review)
Felix Held (Code Review)
Kyösti Mälkki (Code Review)
Michał Żygowski (Code Review)
Nico Huber (Code Review)
Paul Menzel (Code Review)
Raul Rangel (Code Review)