[coreboot-gerrit] Change in coreboot[master]: Documentation/Intel: Add MultiProcessorInit documentation

[Nico Huber (Code Review)]

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

Change subject: Documentation/Intel: Add MultiProcessorInit documentation
......................................................................


Patch Set 3:

>> (4 comments)
> 
> I agree with you on most this comments that you have put, but my
> only point is that, if i tries to put this implementation of
> creating EDK2 interface inside Coreboot to do certain thinks might
> not justify the purpose of adding those "foreign" interface as your
> words. Today coreboot code is so independent in nature so that we
> don;t need to talk help of such EDK2 interface to do certain
> things, if we really want to increase mp_init.c capability we could
> have done the same using native c why UEFI?
> 
> UEFI is because this implementation only targeted for Intel fsp 9th
> gen + designs where we might want to do few things using coreboot
> callback from fsp, hence i could see a direct dependencies between
> this implementation proposal and supporting FSP model to justify
> that callback using SkipMpInit=0. if i make this code into common
> x86 place as well, no one will use this except 9th gen + socs from
> intel with proper FSP support.
> 
> whats the point of keeping certain code into common when there are
> no user of that code expect intel native FSP support?

One point would be visibility. If anybody not familiar with the
soc/intel/ code would look at our MP interfaces, he might miss
the fact that a generic implementation of your EFI interface
already exists, might even reimplement it one way or another and
coreboot would decompose a little more.

That's just a general thought. I agree that this interface might
be too FSP specific (from a coreboot point of view).

> 
> Hope i could able to explain my concern, i have written this
> document so that i can keep it anywhere but myself not very
> convinced about why other users will interested to make use of EFI
> code interface inside x86/coreboot when mp_init.c is capable of
> doing everything without those APIs.

You are right. I wouldn't expect it to be used inside coreboot,
I was implying other potential blob use cases.


-- 
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 at intel.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki at gmail.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik at intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-CC: Nico Huber <nico.h at gmx.de>
Gerrit-CC: Patrick Georgi <pgeorgi at google.com>
Gerrit-Comment-Date: Tue, 01 May 2018 13:10:39 +0000
Gerrit-HasComments: No
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180501/d87fe457/attachment.html>