From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mx.groups.io with SMTP id smtpd.web09.4822.1576630506247772092 for ; Tue, 17 Dec 2019 16:55:06 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.88, mailfrom: ray.ni@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Dec 2019 16:55:05 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,327,1571727600"; d="scan'208";a="365589961" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga004.jf.intel.com with ESMTP; 17 Dec 2019 16:55:05 -0800 Received: from fmsmsx126.amr.corp.intel.com (10.18.125.43) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 17 Dec 2019 16:55:05 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX126.amr.corp.intel.com (10.18.125.43) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 17 Dec 2019 16:55:05 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.90]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.222]) with mapi id 14.03.0439.000; Wed, 18 Dec 2019 08:55:03 +0800 From: "Ni, Ray" To: Ashish Singhal , "devel@edk2.groups.io" , "Wang, Jian J" , "Wu, Hao A" , "Gao, Zhichao" Subject: Re: [PATCH v3] MdeModulePkg: Add Platform Boot Options Protocol Thread-Topic: [PATCH v3] MdeModulePkg: Add Platform Boot Options Protocol Thread-Index: AQHVtRmuGeyhmgP4/EyI0qINRGSpRqe/Ct1A Date: Wed, 18 Dec 2019 00:55:03 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C3A114B@SHSMSX104.ccr.corp.intel.com> References: <6f24dc6e1495eded1c77fa35aff40b574a75d7be.1576614938.git.ashishsingha@nvidia.com> In-Reply-To: <6f24dc6e1495eded1c77fa35aff40b574a75d7be.1576614938.git.ashishsingha@nvidia.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYTA0YWU4OGEtYzMyYi00Njk3LWFiZTktOTRiYzM3MWM3ZDRmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiZ0JNaytkWEZcL01tdDRDOTNVVXVyTnN1QklTMUJLd2R6UmQ1Q0g4dmxrOW1qWGtNZnVGOElKVVgyQTBtZXVLVzgifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 UpdatedO= ptions, OUT UpdatedOptionCount) Usually EDKII puts Count in second and buffer in first. The reason of using EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL as the new protoco= l name is in future we could increase the revision and put more platform hook API in this proto= col. The reason of combining two APIs to one RefreshAllBootOption() is when I ch= ecked the code change below, I see no need to separate them. What do you think? Thanks, Ray > // > + // Locate Platform Boot Options Protocol > + // > + PlatformBootOptions =3D NULL; > + Status =3D gBS->LocateProtocol (&gEdkiiPlatformBootOptionsProtocolGuid= , > + NULL, > + (VOID **)&PlatformBootOptions); > + if (!EFI_ERROR (Status)) { > + // > + // If found, call platform specific overrides to auto enumerated > + // boot options. > + // > + Status =3D PlatformBootOptions->OverridePlatformBootOptions ((CONST = UINTN)BootOptionCount, > + (CONST EF= I_BOOT_MANAGER_LOAD_OPTION *)BootOptions, > + &UpdatedB= ootOptionCount, > + &UpdatedB= ootOptions); > + if (!EFI_ERROR (Status)) { > + EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount); > + BootOptions =3D UpdatedBootOptions; > + BootOptionCount =3D UpdatedBootOptionCount; > + } > + > + // > + // Call platform specific override to remove invalid boot options fr= om NV > + // > + Status =3D PlatformBootOptions->RemoveInvalidPlatformNvBootOptions (= (CONST UINTN)NvBootOptionCount, > + (C= ONST EFI_BOOT_MANAGER_LOAD_OPTION *)NvBootOptions, > + &U= pdatedBootOptionCount, > + &U= pdatedBootOptions); > + if (!EFI_ERROR (Status)) { > + EfiBootManagerFreeLoadOptions (NvBootOptions, NvBootOptionCount); > + NvBootOptions =3D UpdatedBootOptions; > + NvBootOptionCount =3D UpdatedBootOptionCount; > + } > + } > +