May 2018 - coreboot-gerrit - mail.coreboot.org

Change in coreboot[master]: Documentation/Intel: Add MultiProcessorInit documentation
by Subrata Banik (Code Review) May 1, 2018

May 1, 2018

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/25921 ) Change subject: Documentation/Intel: Add MultiProcessorInit documentation ...................................................................... Patch Set 3: (5 comments) https://review.coreboot.org/#/c/25921/3/Documentation/Intel/MultiProcessorI… File Documentation/Intel/MultiProcessorInit/MultiProcessorInit.md: https://review.coreboot.org/#/c/25921/3/Documentation/Intel/MultiProcessorI… PS3, Line 7: for Intel 9th Gen (Cannon Lake) and beyond CPUs > This is why you are doing it, but it seems to me that the result […] i have made this clear why we need to create this API interface in this section. Although i agree this is very generic implementation and dependent on mp_init.c so going forward we might feel a feed to make it generic, today we can't do because it has some EDK2 header dependencies which is only available for intel UDK2017 platform (CNL onwards). btw, why you might need to have 2 document, i'm not going to explain how mp_init works on x86 platform, those are already in SDM, this document is to only talk about why we are creating EDK2 interface in coreboot and how intel thinks this can help our partners design. https://review.coreboot.org/#/c/25921/3/Documentation/Intel/MultiProcessorI… PS3, Line 25: closed source in nature > It might be just me, but to me "closed source in nature" sounds like […] i don;t understand what you mean by can't be trusted ? https://review.coreboot.org/#/c/25921/3/Documentation/Intel/MultiProcessorI… PS3, Line 29: get-rid of such close source > If I understand it correctly (please correct me if not), it only […] Yes, you are right https://review.coreboot.org/#/c/25921/3/Documentation/Intel/MultiProcessorI… PS3, Line 35: It’s also possible for coreboot to extend its own : support model and create a sets of APIs which later can be used by FSP to run CPU feature : programming using coreboot published APIs > I don't understand. coreboot already has such an API […] i guess you have missed problem statement, you are referring to proposal and try to understand why i have made this proposal. its clearly mentioned in problem statement why we even need this proposal https://review.coreboot.org/#/c/25921/3/Documentation/Intel/MultiProcessorI… PS3, Line 68: 1. coreboot was using SkipMpInit=1 which will skip entire FSP CPU feature programming. > This is decided per board and not for coreboot in general, AFAICS. not sure what you mean by coreboot? this is a UPD which is intel fsp specific. not sure how this can be in general ? -- To view, visit https://review.coreboot.org/25921 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5b6096ef31d8a523c00cbad39ab9d4884e735fde Gerrit-Change-Number: 25921 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Comment-Date: Tue, 01 May 2018 11:43:09 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: Documentation/Intel: Add MultiProcessorInit documentation
by Subrata Banik (Code Review) May 1, 2018

May 1, 2018

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/25921 ) Change subject: Documentation/Intel: Add MultiProcessorInit documentation ...................................................................... Patch Set 3: > (5 comments) > > Thanks for writing this up. The relation to the `SkipMpInit` wasn't > clear to me before (I'm not 100% sure yet, but much closer). > > Please make it clear if you intend to force usage of this interface > or if it is optional (for `SkipMpInit=1`). Nothing is force here, its optional if you want to avoid FSP to use those interface which coreboot has published, then just set SkipMpInit=1, FSP won't call this APIs to run those feature programming over AP. -- To view, visit https://review.coreboot.org/25921 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5b6096ef31d8a523c00cbad39ab9d4884e735fde Gerrit-Change-Number: 25921 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Comment-Date: Tue, 01 May 2018 11:35:42 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in coreboot[master]: Documentation/Intel: Add MultiProcessorInit documentation
by Nico Huber (Code Review) May 1, 2018

May 1, 2018

Nico Huber has posted comments on this change. ( https://review.coreboot.org/25921 ) Change subject: Documentation/Intel: Add MultiProcessorInit documentation ...................................................................... Patch Set 3: (5 comments) Thanks for writing this up. The relation to the `SkipMpInit` wasn't clear to me before (I'm not 100% sure yet, but much closer). Please make it clear if you intend to force usage of this interface or if it is optional (for `SkipMpInit=1`). https://review.coreboot.org/#/c/25921/3/Documentation/Intel/MultiProcessorI… File Documentation/Intel/MultiProcessorInit/MultiProcessorInit.md: https://review.coreboot.org/#/c/25921/3/Documentation/Intel/MultiProcessorI… PS3, Line 7: for Intel 9th Gen (Cannon Lake) and beyond CPUs This is why you are doing it, but it seems to me that the result is 100% generic. Both, coreboot's MP API and EFI_MP_SERVICES_PPI, are SoC and vendor independent. Hence I wouldn't expect this in Documentation/Intel/ and the implementation in cpu/x86/ along- side `mp_init.c`. Maybe split this in two documents, one that states that coreboot implements EFI_MP_SERVICES_PPI APIs and why (foreign code linked with coreboot may rely on it) and one that explains how it's working together with FSP? https://review.coreboot.org/#/c/25921/3/Documentation/Intel/MultiProcessorI… PS3, Line 25: closed source in nature It might be just me, but to me "closed source in nature" sounds like "could not even be trusted if published". https://review.coreboot.org/#/c/25921/3/Documentation/Intel/MultiProcessorI… PS3, Line 29: get-rid of such close source If I understand it correctly (please correct me if not), it only reduces closed-source overhead in case of `SkipMpInit=0`, right? `SkipMpInit=1` will still work and run the least closed-source code? https://review.coreboot.org/#/c/25921/3/Documentation/Intel/MultiProcessorI… PS3, Line 35: It’s also possible for coreboot to extend its own : support model and create a sets of APIs which later can be used by FSP to run CPU feature : programming using coreboot published APIs I don't understand. coreboot already has such an API (include/cpu/x86/mp.h). If it could be used by FSP later, why can't it be used now? https://review.coreboot.org/#/c/25921/3/Documentation/Intel/MultiProcessorI… PS3, Line 68: 1. coreboot was using SkipMpInit=1 which will skip entire FSP CPU feature programming. This is decided per board and not for coreboot in general, AFAICS. -- To view, visit https://review.coreboot.org/25921 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5b6096ef31d8a523c00cbad39ab9d4884e735fde Gerrit-Change-Number: 25921 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Comment-Date: Tue, 01 May 2018 11:26:51 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: arch/x86: Print CPU Index and APIC ID in case of exception
by build bot (Jenkins) (Code Review) May 1, 2018

May 1, 2018

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/25709 ) Change subject: arch/x86: Print CPU Index and APIC ID in case of exception ...................................................................... Patch Set 14: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-gerrit/71077/ : SUCCESS https://qa.coreboot.org/job/coreboot-checkpatch/25252/ : SUCCESS -- To view, visit https://review.coreboot.org/25709 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d3776c9259747197a5f2410032f9b03786407fb Gerrit-Change-Number: 25709 Gerrit-PatchSet: 14 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Tue, 01 May 2018 09:03:10 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in coreboot[master]: arch/x86: Print CPU Index and APIC ID in case of exception
by build bot (Jenkins) (Code Review) May 1, 2018

May 1, 2018

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/25709 ) Change subject: arch/x86: Print CPU Index and APIC ID in case of exception ...................................................................... Patch Set 13: Build Unstable https://qa.coreboot.org/job/coreboot-gerrit/71076/ : UNSTABLE https://qa.coreboot.org/job/coreboot-checkpatch/25251/ : SUCCESS -- To view, visit https://review.coreboot.org/25709 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d3776c9259747197a5f2410032f9b03786407fb Gerrit-Change-Number: 25709 Gerrit-PatchSet: 13 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Tue, 01 May 2018 08:01:29 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in coreboot[master]: arch/x86: Make cpu_index() available for bootblock
by build bot (Jenkins) (Code Review) May 1, 2018

May 1, 2018

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/25964 ) Change subject: arch/x86: Make cpu_index() available for bootblock ...................................................................... Patch Set 3: Verified-1 Build Unstable https://qa.coreboot.org/job/coreboot-gerrit/71075/ : UNSTABLE https://qa.coreboot.org/job/coreboot-checkpatch/25250/ : SUCCESS -- To view, visit https://review.coreboot.org/25964 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02b2387eb601abe2d64ed20ebf8a98572befd26c Gerrit-Change-Number: 25964 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Tue, 01 May 2018 07:46:36 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in coreboot[master]: arch/x86: Make cpu_index() available for bootblock
by build bot (Jenkins) (Code Review) May 1, 2018

May 1, 2018

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/25964 ) Change subject: arch/x86: Make cpu_index() available for bootblock ...................................................................... Patch Set 2: Build Unstable https://qa.coreboot.org/job/coreboot-gerrit/71073/ : UNSTABLE -- To view, visit https://review.coreboot.org/25964 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02b2387eb601abe2d64ed20ebf8a98572befd26c Gerrit-Change-Number: 25964 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 01 May 2018 04:59:46 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in coreboot[master]: arch/x86: Print CPU Index and APIC ID in case of exception
by build bot (Jenkins) (Code Review) May 1, 2018

May 1, 2018

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/25709 ) Change subject: arch/x86: Print CPU Index and APIC ID in case of exception ...................................................................... Patch Set 12: Build Unstable https://qa.coreboot.org/job/coreboot-gerrit/71074/ : UNSTABLE https://qa.coreboot.org/job/coreboot-checkpatch/25249/ : SUCCESS -- To view, visit https://review.coreboot.org/25709 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d3776c9259747197a5f2410032f9b03786407fb Gerrit-Change-Number: 25709 Gerrit-PatchSet: 12 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 01 May 2018 04:52:33 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in coreboot[master]: arch/x86: Make cpu_index() available for bootblock
by build bot (Jenkins) (Code Review) May 1, 2018

May 1, 2018

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/25964 ) Change subject: arch/x86: Make cpu_index() available for bootblock ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/coreboot-gerrit/71072/ : ABORTED https://qa.coreboot.org/job/coreboot-checkpatch/25248/ : SUCCESS -- To view, visit https://review.coreboot.org/25964 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02b2387eb601abe2d64ed20ebf8a98572befd26c Gerrit-Change-Number: 25964 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 01 May 2018 04:43:22 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in coreboot[master]: arch/x86: Print CPU Index and APIC ID in case of exception
by build bot (Jenkins) (Code Review) May 1, 2018

May 1, 2018

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/25709 ) Change subject: arch/x86: Print CPU Index and APIC ID in case of exception ...................................................................... Patch Set 11: Build Failed https://qa.coreboot.org/job/coreboot-gerrit/71071/ : ABORTED https://qa.coreboot.org/job/coreboot-checkpatch/25247/ : SUCCESS -- To view, visit https://review.coreboot.org/25709 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d3776c9259747197a5f2410032f9b03786407fb Gerrit-Change-Number: 25709 Gerrit-PatchSet: 11 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 01 May 2018 04:43:21 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0