Re: [PATCH] [edk2-platforms]:Enabling Secure boot feature support on hikey platfrom

[Kalyan Nagabhirava <kalyankumar.nagabhirava@linaro.org>]

HI Leif,
we didn't implemented enabling USB host and  Secure boot support  on Hikey
, we just took the code from openplatfrompkg (hikey branch)
 , we have implemented secureboot and DRI -disaster recovery image (HTTP
image download) application
and tested on HIkey platform , so for that purpose we are trying to
upstream the hikey code.

but hikey platform code  looks in bad shape (as per ard and your comments)
,so  we are planning to upstream
only our application code which is independent of platform.

Regards,
kalyan.


On 27 November 2017 at 22:27, Leif Lindholm <leif.lindholm@linaro.org>
wrote:

> On Mon, Nov 27, 2017 at 02:02:32PM +0100, Laszlo Ersek wrote:
> > On 11/26/17 16:22, Leif Lindholm wrote:
> > > (Adding Laszlo to cc based on a single comment I make below.)
> > >
> > > On Tue, Nov 21, 2017 at 04:23:36PM +0530, kalyan-nagabhirava wrote:
> >
> > >>  [Guids.common]
> > >>    gHiKeyTokenSpaceGuid          =  { 0x91148425, 0xcdd2, 0x4830, {
> 0x8b, 0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }
> > >> +  gHwTokenSpaceGuid             =  { 0x99999999, 0x74c5, 0x4043, {
> 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
> > >
> > > This very much looks like a not properly generated GUID.
> > > GUIDs must always be generated using an RFC4122-compliant algorithm.
> > > I generally recommend using
> > > https://www.guidgenerator.com/online-guid-generator.aspx.
> >
> > I just run "uuidgen" in a terminal window.
>
> Yeah, I just prefer pointing to someone that does not require
> installing anything, or requires specific operating systems.
>
> > >> +EFI_STATUS
> > >> +EFIAPI
> > >> +FvbSetAttributes(
> > >> +  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL  *This,
> > >> +  IN OUT    EFI_FVB_ATTRIBUTES_2                 *Attributes
> > >> +  )
> > >> +{
> > >> +  DEBUG ((DEBUG_BLKIO, "FvbSetAttributes(0x%X) is not
> supported\n",*Attributes));
> > >> +  return EFI_UNSUPPORTED;
> > >
> > > As per my (very) recent comment to Marcin, I do not believe returning
> > > EFI_UNSUPPORTED is a valid thing to do here. Which to me suggests the
> > > implementation of FvbGetAttributes is also incorrect.
> > >
> > > Laszlo - what's your take on this in conjunction with PI 1.6 section
> > > 3.4.2? OvmfPkg does something very similar in
> > > EmuVariableFvbRuntimeDxe/Fvb.c.
> >
> > I guess you are right. The particular OvmfPkg code that you mention is
> > likely also spec-breaking.
> >
> > FWIW, in the OVMF flash driver that actually uses pflash, namely
> >
> >   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
> >
> > the FvbSetVolumeAttributes() function appears both appropriate for the
> > spec and generic enough to copy elsewhere.
>
> Yes, that looks good, thanks!
>
> Marcin, Kalyan - please have a look at that implementation for
> inspiration.
>
> /
>     Leif
>



-- 
regards,
kalyan.