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

[Laszlo Ersek <lersek@redhat.com>]

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.

>> +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.

Thanks
Laszlo