| Message ID | 1490099187-54849-1-git-send-email-chenhui.sun@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Leif, This patch is under IntelFrameworkModulePkg, So I sent it out indenpdently, expect your comments about the solution and the limitation. Thanks and Regards, Chenhui 在 2017/3/21 20:26, Chenhui Sun 写道: > Support the feature that BIOS get boot option from BMC and set > it to the first boot order. > > The feature works dependding on this patch and the OPP patch > "Hisilicon/D03/D05: get boot option from BMC" both be added. > > And it have a limitation, only set the boot order by type, can't by > the specfic devices. > example: there have 4 ethernet ports at D05 board, it can only be booted > from the ethernet port, but which port can not be defined. so it try from > the first port to the end. > > so is there any solution? expect your comments. > > Signed-off-by: Huang ming <huangming23@linaro.org> > Signed-off-by: Chenhui Sun <chenhui.sun@linaro.org> > --- > .../Universal/BdsDxe/BdsEntry.c | 44 ++++++++++++---------- > 1 file changed, 24 insertions(+), 20 deletions(-) > > diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c > index bf81de4..ee51055 100644 > --- a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c > +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c > @@ -36,7 +36,6 @@ EFI_BDS_ARCH_PROTOCOL gBds = { > BdsEntry > }; > > -UINT16 *mBootNext = NULL; > > /// > /// The read-only variables defined in UEFI Spec. > @@ -123,6 +122,9 @@ BdsBootDeviceSelect ( > BOOLEAN BootNextExist; > LIST_ENTRY *LinkBootNext; > EFI_EVENT ConnectConInEvent; > + UINTN BootNextSize; > + UINT16 *BootNext = NULL; > + UINT16 Index; > > // > // Got the latest boot option > @@ -154,7 +156,16 @@ BdsBootDeviceSelect ( > } > } > > - if (mBootNext != NULL) { > + // > + // Check if we have the boot next option > + // > + BootNext = BdsLibGetVariableAndSize ( > + L"BootNext", > + &gEfiGlobalVariableGuid, > + &BootNextSize > + ); > + > + if (BootNext != NULL) { > // > // Indicate we have the boot next variable, so this time > // boot will always have this boot option > @@ -179,17 +190,19 @@ BdsBootDeviceSelect ( > // > // Add the boot next boot option > // > - UnicodeSPrint (Buffer, sizeof (Buffer), L"Boot%04x", *mBootNext); > - BootOption = BdsLibVariableToOption (&BootLists, Buffer); > + for (Index = 0; Index < (BootNextSize/sizeof(UINT16)); Index++) { > + UnicodeSPrint (Buffer, sizeof (Buffer), L"Boot%04x", BootNext[Index]); > + BootOption = BdsLibVariableToOption (&BootLists, Buffer); > > - // > - // If fail to get boot option from variable, just return and do nothing. > - // > - if (BootOption == NULL) { > - return; > - } > + // > + // If fail to get boot option from variable, just return and do nothing. > + // > + if (BootOption == NULL) { > + return; > + } > > - BootOption->BootCurrent = *mBootNext; > + BootOption->BootCurrent = BootNext[Index]; > + } > } > // > // Parse the boot order to get boot option > @@ -528,7 +541,6 @@ BdsEntry ( > { > LIST_ENTRY DriverOptionList; > LIST_ENTRY BootOptionList; > - UINTN BootNextSize; > CHAR16 *FirmwareVendor; > EFI_STATUS Status; > UINT16 BootTimeOut; > @@ -638,14 +650,6 @@ BdsEntry ( > if (!IsListEmpty (&DriverOptionList)) { > BdsLibLoadDrivers (&DriverOptionList); > } > - // > - // Check if we have the boot next option > - // > - mBootNext = BdsLibGetVariableAndSize ( > - L"BootNext", > - &gEfiGlobalVariableGuid, > - &BootNextSize > - ); > > // > // Setup some platform policy here
On 21 March 2017 at 12:26, Chenhui Sun <chenhui.sun@linaro.org> wrote: > Support the feature that BIOS get boot option from BMC and set > it to the first boot order. > > The feature works dependding on this patch and the OPP patch > "Hisilicon/D03/D05: get boot option from BMC" both be added. > > And it have a limitation, only set the boot order by type, can't by > the specfic devices. > example: there have 4 ethernet ports at D05 board, it can only be booted > from the ethernet port, but which port can not be defined. so it try from > the first port to the end. > > so is there any solution? expect your comments. > > Signed-off-by: Huang ming <huangming23@linaro.org> > Signed-off-by: Chenhui Sun <chenhui.sun@linaro.org> I understand that this change is required to allow the boot order to be set via the BMC. But could you please explain why you need to make the changes below? Why do you have to change the way BootNext is handled? > --- > .../Universal/BdsDxe/BdsEntry.c | 44 ++++++++++++---------- > 1 file changed, 24 insertions(+), 20 deletions(-) > > diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c > index bf81de4..ee51055 100644 > --- a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c > +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c > @@ -36,7 +36,6 @@ EFI_BDS_ARCH_PROTOCOL gBds = { > BdsEntry > }; > > -UINT16 *mBootNext = NULL; > > /// > /// The read-only variables defined in UEFI Spec. > @@ -123,6 +122,9 @@ BdsBootDeviceSelect ( > BOOLEAN BootNextExist; > LIST_ENTRY *LinkBootNext; > EFI_EVENT ConnectConInEvent; > + UINTN BootNextSize; > + UINT16 *BootNext = NULL; > + UINT16 Index; > > // > // Got the latest boot option > @@ -154,7 +156,16 @@ BdsBootDeviceSelect ( > } > } > > - if (mBootNext != NULL) { > + // > + // Check if we have the boot next option > + // > + BootNext = BdsLibGetVariableAndSize ( > + L"BootNext", > + &gEfiGlobalVariableGuid, > + &BootNextSize > + ); > + > + if (BootNext != NULL) { > // > // Indicate we have the boot next variable, so this time > // boot will always have this boot option > @@ -179,17 +190,19 @@ BdsBootDeviceSelect ( > // > // Add the boot next boot option > // > - UnicodeSPrint (Buffer, sizeof (Buffer), L"Boot%04x", *mBootNext); > - BootOption = BdsLibVariableToOption (&BootLists, Buffer); > + for (Index = 0; Index < (BootNextSize/sizeof(UINT16)); Index++) { > + UnicodeSPrint (Buffer, sizeof (Buffer), L"Boot%04x", BootNext[Index]); > + BootOption = BdsLibVariableToOption (&BootLists, Buffer); > > - // > - // If fail to get boot option from variable, just return and do nothing. > - // > - if (BootOption == NULL) { > - return; > - } > + // > + // If fail to get boot option from variable, just return and do nothing. > + // > + if (BootOption == NULL) { > + return; > + } > > - BootOption->BootCurrent = *mBootNext; > + BootOption->BootCurrent = BootNext[Index]; > + } > } > // > // Parse the boot order to get boot option > @@ -528,7 +541,6 @@ BdsEntry ( > { > LIST_ENTRY DriverOptionList; > LIST_ENTRY BootOptionList; > - UINTN BootNextSize; > CHAR16 *FirmwareVendor; > EFI_STATUS Status; > UINT16 BootTimeOut; > @@ -638,14 +650,6 @@ BdsEntry ( > if (!IsListEmpty (&DriverOptionList)) { > BdsLibLoadDrivers (&DriverOptionList); > } > - // > - // Check if we have the boot next option > - // > - mBootNext = BdsLibGetVariableAndSize ( > - L"BootNext", > - &gEfiGlobalVariableGuid, > - &BootNextSize > - ); > > // > // Setup some platform policy here > -- > 1.9.1 >
Hi Ard, 在 2017/3/28 1:11, Ard Biesheuvel 写道: > On 21 March 2017 at 12:26, Chenhui Sun <chenhui.sun@linaro.org> wrote: >> Support the feature that BIOS get boot option from BMC and set >> it to the first boot order. >> >> The feature works dependding on this patch and the OPP patch >> "Hisilicon/D03/D05: get boot option from BMC" both be added. >> >> And it have a limitation, only set the boot order by type, can't by >> the specfic devices. >> example: there have 4 ethernet ports at D05 board, it can only be booted >> from the ethernet port, but which port can not be defined. so it try from >> the first port to the end. >> >> so is there any solution? expect your comments. >> >> Signed-off-by: Huang ming <huangming23@linaro.org> >> Signed-off-by: Chenhui Sun <chenhui.sun@linaro.org> > I understand that this change is required to allow the boot order to > be set via the BMC. > > But could you please explain why you need to make the changes below? > Why do you have to change the way BootNext is handled? Sorry for replying late. In BdsEntry function, geting the BootNext Variable is before the calling of PlatformBdsPolicyBehavior(), but the function ProductBdsPolicyAfterSetup that get boot option type from BMC is called in the PlatformBdsPolicyBehavior function. The function ProductBdsPolicyAfterSetup will update the BootNext variable. We think that the operation of getting the BootNext variable should be in BdsBootDeviceSelect function, just before using the BootNext variable, and the global variable mBootNext is no need. Regards, Chenhui >> --- >> .../Universal/BdsDxe/BdsEntry.c | 44 ++++++++++++---------- >> 1 file changed, 24 insertions(+), 20 deletions(-) >> >> diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c >> index bf81de4..ee51055 100644 >> --- a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c >> +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c >> @@ -36,7 +36,6 @@ EFI_BDS_ARCH_PROTOCOL gBds = { >> BdsEntry >> }; >> >> -UINT16 *mBootNext = NULL; >> >> /// >> /// The read-only variables defined in UEFI Spec. >> @@ -123,6 +122,9 @@ BdsBootDeviceSelect ( >> BOOLEAN BootNextExist; >> LIST_ENTRY *LinkBootNext; >> EFI_EVENT ConnectConInEvent; >> + UINTN BootNextSize; >> + UINT16 *BootNext = NULL; >> + UINT16 Index; >> >> // >> // Got the latest boot option >> @@ -154,7 +156,16 @@ BdsBootDeviceSelect ( >> } >> } >> >> - if (mBootNext != NULL) { >> + // >> + // Check if we have the boot next option >> + // >> + BootNext = BdsLibGetVariableAndSize ( >> + L"BootNext", >> + &gEfiGlobalVariableGuid, >> + &BootNextSize >> + ); >> + >> + if (BootNext != NULL) { >> // >> // Indicate we have the boot next variable, so this time >> // boot will always have this boot option >> @@ -179,17 +190,19 @@ BdsBootDeviceSelect ( >> // >> // Add the boot next boot option >> // >> - UnicodeSPrint (Buffer, sizeof (Buffer), L"Boot%04x", *mBootNext); >> - BootOption = BdsLibVariableToOption (&BootLists, Buffer); >> + for (Index = 0; Index < (BootNextSize/sizeof(UINT16)); Index++) { >> + UnicodeSPrint (Buffer, sizeof (Buffer), L"Boot%04x", BootNext[Index]); >> + BootOption = BdsLibVariableToOption (&BootLists, Buffer); >> >> - // >> - // If fail to get boot option from variable, just return and do nothing. >> - // >> - if (BootOption == NULL) { >> - return; >> - } >> + // >> + // If fail to get boot option from variable, just return and do nothing. >> + // >> + if (BootOption == NULL) { >> + return; >> + } >> >> - BootOption->BootCurrent = *mBootNext; >> + BootOption->BootCurrent = BootNext[Index]; >> + } >> } >> // >> // Parse the boot order to get boot option >> @@ -528,7 +541,6 @@ BdsEntry ( >> { >> LIST_ENTRY DriverOptionList; >> LIST_ENTRY BootOptionList; >> - UINTN BootNextSize; >> CHAR16 *FirmwareVendor; >> EFI_STATUS Status; >> UINT16 BootTimeOut; >> @@ -638,14 +650,6 @@ BdsEntry ( >> if (!IsListEmpty (&DriverOptionList)) { >> BdsLibLoadDrivers (&DriverOptionList); >> } >> - // >> - // Check if we have the boot next option >> - // >> - mBootNext = BdsLibGetVariableAndSize ( >> - L"BootNext", >> - &gEfiGlobalVariableGuid, >> - &BootNextSize >> - ); >> >> // >> // Setup some platform policy here >> -- >> 1.9.1 >>
diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c index bf81de4..ee51055 100644 --- a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c @@ -36,7 +36,6 @@ EFI_BDS_ARCH_PROTOCOL gBds = { BdsEntry }; -UINT16 *mBootNext = NULL; /// /// The read-only variables defined in UEFI Spec. @@ -123,6 +122,9 @@ BdsBootDeviceSelect ( BOOLEAN BootNextExist; LIST_ENTRY *LinkBootNext; EFI_EVENT ConnectConInEvent; + UINTN BootNextSize; + UINT16 *BootNext = NULL; + UINT16 Index; // // Got the latest boot option @@ -154,7 +156,16 @@ BdsBootDeviceSelect ( } } - if (mBootNext != NULL) { + // + // Check if we have the boot next option + // + BootNext = BdsLibGetVariableAndSize ( + L"BootNext", + &gEfiGlobalVariableGuid, + &BootNextSize + ); + + if (BootNext != NULL) { // // Indicate we have the boot next variable, so this time // boot will always have this boot option @@ -179,17 +190,19 @@ BdsBootDeviceSelect ( // // Add the boot next boot option // - UnicodeSPrint (Buffer, sizeof (Buffer), L"Boot%04x", *mBootNext); - BootOption = BdsLibVariableToOption (&BootLists, Buffer); + for (Index = 0; Index < (BootNextSize/sizeof(UINT16)); Index++) { + UnicodeSPrint (Buffer, sizeof (Buffer), L"Boot%04x", BootNext[Index]); + BootOption = BdsLibVariableToOption (&BootLists, Buffer); - // - // If fail to get boot option from variable, just return and do nothing. - // - if (BootOption == NULL) { - return; - } + // + // If fail to get boot option from variable, just return and do nothing. + // + if (BootOption == NULL) { + return; + } - BootOption->BootCurrent = *mBootNext; + BootOption->BootCurrent = BootNext[Index]; + } } // // Parse the boot order to get boot option @@ -528,7 +541,6 @@ BdsEntry ( { LIST_ENTRY DriverOptionList; LIST_ENTRY BootOptionList; - UINTN BootNextSize; CHAR16 *FirmwareVendor; EFI_STATUS Status; UINT16 BootTimeOut; @@ -638,14 +650,6 @@ BdsEntry ( if (!IsListEmpty (&DriverOptionList)) { BdsLibLoadDrivers (&DriverOptionList); } - // - // Check if we have the boot next option - // - mBootNext = BdsLibGetVariableAndSize ( - L"BootNext", - &gEfiGlobalVariableGuid, - &BootNextSize - ); // // Setup some platform policy here