Ray,

I did not name the protocol this way because EmbeddedPkg already describes a 
protocol with a similar name that is PLATFORM_BOOT_MANAGER_PROTOCOL. If you 
think we can still go ahead with new protocol named 
EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL, I can make the necessary changes.

I agree with your suggestion about unifying both functions into a single one as 
well.

Thanks
Ashish

-----Original Message-----
From: Ni, Ray <ray...@intel.com> 
Sent: Tuesday, December 17, 2019 5:55 PM
To: Ashish Singhal <ashishsin...@nvidia.com>; devel@edk2.groups.io; Wang, Jian 
J <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com>; Gao, Zhichao 
<zhichao....@intel.com>
Subject: RE: [PATCH v3] MdeModulePkg: Add Platform Boot Options Protocol

External email: Use caution opening links or attachments


Ashish,
I prefer EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL and could have two fields for 
this protocol for now:
Revision and RefreshAllBootOption (IN Options, IN OptionCount, OUT 
UpdatedOptions, OUT UpdatedOptionCount) Usually EDKII puts Count in second and 
buffer in first.

The reason of using EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL as the new protocol 
name is in future we could increase the revision and put more platform hook API 
in this protocol.

The reason of combining two APIs to one RefreshAllBootOption() is when I 
checked the code change below, I see no need to separate them.

What do you think?

Thanks,
Ray

>    //
> +  // Locate Platform Boot Options Protocol  //  PlatformBootOptions = 
> + NULL;  Status = gBS->LocateProtocol 
> + (&gEdkiiPlatformBootOptionsProtocolGuid,
> +                                NULL,
> +                                (VOID **)&PlatformBootOptions);  if 
> + (!EFI_ERROR (Status)) {
> +    //
> +    // If found, call platform specific overrides to auto enumerated
> +    // boot options.
> +    //
> +    Status = PlatformBootOptions->OverridePlatformBootOptions ((CONST 
> UINTN)BootOptionCount,
> +                                                               (CONST 
> EFI_BOOT_MANAGER_LOAD_OPTION *)BootOptions,
> +                                                               
> &UpdatedBootOptionCount,
> +                                                               
> &UpdatedBootOptions);
> +    if (!EFI_ERROR (Status)) {
> +      EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> +      BootOptions = UpdatedBootOptions;
> +      BootOptionCount = UpdatedBootOptionCount;
> +    }
> +
> +    //
> +    // Call platform specific override to remove invalid boot options from NV
> +    //
> +    Status = PlatformBootOptions->RemoveInvalidPlatformNvBootOptions ((CONST 
> UINTN)NvBootOptionCount,
> +                                                                      (CONST 
> EFI_BOOT_MANAGER_LOAD_OPTION *)NvBootOptions,
> +                                                                      
> &UpdatedBootOptionCount,
> +                                                                      
> &UpdatedBootOptions);
> +    if (!EFI_ERROR (Status)) {
> +      EfiBootManagerFreeLoadOptions (NvBootOptions, NvBootOptionCount);
> +      NvBootOptions = UpdatedBootOptions;
> +      NvBootOptionCount = UpdatedBootOptionCount;
> +    }
> +  }
> +

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may 
contain
confidential information.  Any unauthorized review, use, disclosure or 
distribution
is prohibited.  If you are not the intended recipient, please contact the 
sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#52326): https://edk2.groups.io/g/devel/message/52326
Mute This Topic: https://groups.io/mt/68770160/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to