On Tue, Mar 15, 2016 at 09:41:25AM +0800, Fu Siyuan wrote: > The HTTP Boot driver have some UEFI driver model problems which will make the > code ASSERT when it's disconnected. > First, the driver opens the HttpSb protocol BY_CHILD without BY_DRIVER > attribute. > So the driver binding stop won't be called when HTTP driver is disconnected, > and > a child handle is left which made HTTP driver binding stop function goes into > error. > This patch remove this unnecessary OpenProtocol and only unload the Hii from > when > both the IP4 and IP6 stack have been stopped completely. > The second issue is the HTTP boot driver always use the driver's image handle > as > it's driver binding handle, it's not correct. HTTP Boot driver provides 2 > separate > driver binding protocols for IP4 and IP6 stack, so it has 2 driver binding > handles. > This patch fix the code to use correct driver binding handle when create/open > a HTTP child handle. > This patch works for me.
Tested-by: Gary Lin <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Fu Siyuan <[email protected]> > Cc: Gary Lin <[email protected]> > CC: Laszlo Ersek <[email protected]> > Cc: Wu Jiaxin <[email protected]> > Cc: Zhang Lubo <[email protected]> > --- > NetworkPkg/HttpBootDxe/HttpBootClient.c | 5 ++++- > NetworkPkg/HttpBootDxe/HttpBootConfig.c | 36 > +++++++++----------------------- > NetworkPkg/HttpBootDxe/HttpBootConfig.h | 1 + > NetworkPkg/HttpBootDxe/HttpBootDxe.c | 18 ++++++++-------- > NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +- > NetworkPkg/HttpBootDxe/HttpBootSupport.c | 8 +++---- > 6 files changed, 29 insertions(+), 41 deletions(-) > > diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c > b/NetworkPkg/HttpBootDxe/HttpBootClient.c > index aae4527..0c47293 100644 > --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c > +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c > @@ -447,6 +447,7 @@ HttpBootCreateHttpIo ( > { > HTTP_IO_CONFIG_DATA ConfigData; > EFI_STATUS Status; > + EFI_HANDLE ImageHandle; > > ASSERT (Private != NULL); > > @@ -456,14 +457,16 @@ HttpBootCreateHttpIo ( > ConfigData.Config4.RequestTimeOut = HTTP_BOOT_REQUEST_TIMEOUT; > IP4_COPY_ADDRESS (&ConfigData.Config4.LocalIp, &Private->StationIp.v4); > IP4_COPY_ADDRESS (&ConfigData.Config4.SubnetMask, > &Private->SubnetMask.v4); > + ImageHandle = Private->Ip4Nic->ImageHandle; > } else { > ConfigData.Config6.HttpVersion = HttpVersion11; > ConfigData.Config6.RequestTimeOut = HTTP_BOOT_REQUEST_TIMEOUT; > IP6_COPY_ADDRESS (&ConfigData.Config6.LocalIp, &Private->StationIp.v6); > + ImageHandle = Private->Ip6Nic->ImageHandle; > } > > Status = HttpIoCreateIo ( > - Private->Image, > + ImageHandle, > Private->Controller, > Private->UsingIpv6 ? IP_VERSION_6 : IP_VERSION_4, > &ConfigData, > diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.c > b/NetworkPkg/HttpBootDxe/HttpBootConfig.c > index 5971923..0c1ff43 100644 > --- a/NetworkPkg/HttpBootDxe/HttpBootConfig.c > +++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.c > @@ -553,7 +553,6 @@ HttpBootConfigFormInit ( > EFI_STATUS Status; > HTTP_BOOT_FORM_CALLBACK_INFO *CallbackInfo; > VENDOR_DEVICE_PATH VendorDeviceNode; > - EFI_SERVICE_BINDING_PROTOCOL *HttpSb; > CHAR16 *MacString; > CHAR16 *OldMenuString; > CHAR16 MenuString[128]; > @@ -600,20 +599,6 @@ HttpBootConfigFormInit ( > &CallbackInfo->ConfigAccess, > NULL > ); > - if (!EFI_ERROR (Status)) { > - // > - // Open the Parent Handle for the child > - // > - Status = gBS->OpenProtocol ( > - Private->Controller, > - &gEfiHttpServiceBindingProtocolGuid, > - (VOID **) &HttpSb, > - Private->Image, > - CallbackInfo->ChildHandle, > - EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER > - ); > - } > - > if (EFI_ERROR (Status)) { > goto Error; > } > @@ -636,7 +621,7 @@ HttpBootConfigFormInit ( > // > // Append MAC string in the menu help string > // > - Status = NetLibGetMacString (Private->Controller, Private->Image, > &MacString); > + Status = NetLibGetMacString (Private->Controller, NULL, &MacString); > if (!EFI_ERROR (Status)) { > OldMenuString = HiiGetString ( > CallbackInfo->RegisteredHandle, > @@ -654,6 +639,7 @@ HttpBootConfigFormInit ( > FreePool (MacString); > FreePool (OldMenuString); > > + CallbackInfo->Initilized = TRUE; > return EFI_SUCCESS; > } > > @@ -666,6 +652,7 @@ Error: > /** > Unload the configuration form, this includes: delete all the configuration > entries, uninstall the form callback protocol, and free the resources used. > + The form will only be unload completely when both IP4 and IP6 stack are > stopped. > > @param[in] Private Pointer to the driver private data. > > @@ -680,18 +667,15 @@ HttpBootConfigFormUnload ( > { > HTTP_BOOT_FORM_CALLBACK_INFO *CallbackInfo; > > - CallbackInfo = &Private->CallbackInfo; > - if (CallbackInfo->ChildHandle != NULL) { > + if (Private->Ip4Nic != NULL || Private->Ip6Nic != NULL) { > // > - // Close the child handle > + // Only unload the configuration form when both IP4 and IP6 stack are > stopped. > // > - gBS->CloseProtocol ( > - Private->Controller, > - &gEfiHttpServiceBindingProtocolGuid, > - Private->Image, > - CallbackInfo->ChildHandle > - ); > - > + return EFI_SUCCESS; > + } > + > + CallbackInfo = &Private->CallbackInfo; > + if (CallbackInfo->ChildHandle != NULL) { > // > // Uninstall EFI_HII_CONFIG_ACCESS_PROTOCOL > // > diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.h > b/NetworkPkg/HttpBootDxe/HttpBootConfig.h > index a2afd18..e610fe8 100644 > --- a/NetworkPkg/HttpBootDxe/HttpBootConfig.h > +++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.h > @@ -63,6 +63,7 @@ HttpBootConfigFormInit ( > /** > Unload the configuration form, this includes: delete all the configuration > entries, uninstall the form callback protocol, and free the resources used. > + The form will only be unload completely when both IP4 and IP6 stack are > stopped. > > @param[in] Private Pointer to the driver private data. > > diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.c > b/NetworkPkg/HttpBootDxe/HttpBootDxe.c > index 6a3033d..642e0fe 100644 > --- a/NetworkPkg/HttpBootDxe/HttpBootDxe.c > +++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.c > @@ -321,7 +321,7 @@ HttpBootIp4DxeDriverBindingStart ( > ); > > if (!EFI_ERROR (Status)) { > - Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); > + Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); > } else { > // > // Initialize the private data structure. > @@ -332,7 +332,6 @@ HttpBootIp4DxeDriverBindingStart ( > } > Private->Signature = HTTP_BOOT_PRIVATE_DATA_SIGNATURE; > Private->Controller = ControllerHandle; > - Private->Image = This->ImageHandle; > InitializeListHead (&Private->CacheList); > // > // Get the NII interface if it exists, it's not required. > @@ -399,8 +398,9 @@ HttpBootIp4DxeDriverBindingStart ( > if (Private->Ip4Nic == NULL) { > return EFI_OUT_OF_RESOURCES; > } > - Private->Ip4Nic->Private = Private; > - Private->Ip4Nic->Signature = HTTP_BOOT_VIRTUAL_NIC_SIGNATURE; > + Private->Ip4Nic->Private = Private; > + Private->Ip4Nic->ImageHandle = This->DriverBindingHandle; > + Private->Ip4Nic->Signature = HTTP_BOOT_VIRTUAL_NIC_SIGNATURE; > > // > // Create DHCP4 child instance. > @@ -793,7 +793,7 @@ HttpBootIp6DxeDriverBindingStart ( > ); > > if (!EFI_ERROR (Status)) { > - Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); > + Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); > } else { > // > // Initialize the private data structure. > @@ -804,7 +804,6 @@ HttpBootIp6DxeDriverBindingStart ( > } > Private->Signature = HTTP_BOOT_PRIVATE_DATA_SIGNATURE; > Private->Controller = ControllerHandle; > - Private->Image = This->ImageHandle; > InitializeListHead (&Private->CacheList); > // > // Get the NII interface if it exists, it's not required. > @@ -871,9 +870,10 @@ HttpBootIp6DxeDriverBindingStart ( > if (Private->Ip6Nic == NULL) { > return EFI_OUT_OF_RESOURCES; > } > - Private->Ip6Nic->Private = Private; > - Private->Ip6Nic->Signature = HTTP_BOOT_VIRTUAL_NIC_SIGNATURE; > - > + Private->Ip6Nic->Private = Private; > + Private->Ip6Nic->ImageHandle = This->DriverBindingHandle; > + Private->Ip6Nic->Signature = HTTP_BOOT_VIRTUAL_NIC_SIGNATURE; > + > // > // Create Dhcp6 child and open Dhcp6 protocol > Status = NetLibCreateServiceChild ( > diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.h > b/NetworkPkg/HttpBootDxe/HttpBootDxe.h > index 7cb4b2c..b3e2576 100644 > --- a/NetworkPkg/HttpBootDxe/HttpBootDxe.h > +++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.h > @@ -102,6 +102,7 @@ typedef union { > struct _HTTP_BOOT_VIRTUAL_NIC { > UINT32 Signature; > EFI_HANDLE Controller; > + EFI_HANDLE ImageHandle; > EFI_LOAD_FILE_PROTOCOL LoadFile; > EFI_DEVICE_PATH_PROTOCOL *DevicePath; > HTTP_BOOT_PRIVATE_DATA *Private; > @@ -118,7 +119,6 @@ struct _HTTP_BOOT_VIRTUAL_NIC { > struct _HTTP_BOOT_PRIVATE_DATA { > UINT32 Signature; > EFI_HANDLE Controller; > - EFI_HANDLE Image; > > HTTP_BOOT_VIRTUAL_NIC *Ip4Nic; > HTTP_BOOT_VIRTUAL_NIC *Ip6Nic; > diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.c > b/NetworkPkg/HttpBootDxe/HttpBootSupport.c > index e678379..6330d5b 100644 > --- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c > +++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c > @@ -372,7 +372,7 @@ HttpBootDns ( > // > Status = NetLibCreateServiceChild ( > Private->Controller, > - Private->Image, > + Private->Ip6Nic->ImageHandle, > &gEfiDns6ServiceBindingProtocolGuid, > &Dns6Handle > ); > @@ -384,7 +384,7 @@ HttpBootDns ( > Dns6Handle, > &gEfiDns6ProtocolGuid, > (VOID **) &Dns6, > - Private->Image, > + Private->Ip6Nic->ImageHandle, > Private->Controller, > EFI_OPEN_PROTOCOL_BY_DRIVER > ); > @@ -474,7 +474,7 @@ Exit: > gBS->CloseProtocol ( > Dns6Handle, > &gEfiDns6ProtocolGuid, > - Private->Image, > + Private->Ip6Nic->ImageHandle, > Private->Controller > ); > } > @@ -482,7 +482,7 @@ Exit: > if (Dns6Handle != NULL) { > NetLibDestroyServiceChild ( > Private->Controller, > - Private->Image, > + Private->Ip6Nic->ImageHandle, > &gEfiDns6ServiceBindingProtocolGuid, > Dns6Handle > ); > -- > 2.7.0.windows.2 > > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
