Re: [edk2-devel] [PATCH] OvmfPkg/XenBusDxe: Close XenIoProtocol openned by childs
On Sun, Jun 30, 2019 at 12:56:57PM +0200, Laszlo Ersek wrote: > (5) Side remark (no need to do anything about it in the scope of this > patch): I think the DisconnectController() call in > XenBusDxeDriverBindingStop() is superfluous. That kind of disconnection > is not the job of EFI_DRIVER_BINDING_PROTOCOL.Stop(). That sounds good and works fine without the call, I'll send a separate patch. -- Anthony PERARD -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43114): https://edk2.groups.io/g/devel/message/43114 Mute This Topic: https://groups.io/mt/32243460/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] OvmfPkg/XenBusDxe: Close XenIoProtocol openned by childs
Hi Anthony, the patch is good; please post a v2 with the following minor improvements: On 06/28/19 18:16, Anthony PERARD wrote: > In XenBusDxe, the XenBusAddDevice() opens the gXenIoProtocolGuid on > behalf of child controllers. It is never closed and prevent from (1) s/prevent/prevents us/ > uninstalling the protocol. > > Close it were we stop all the childs in XenBusDxe->Stop(). (2) s/were/where/ (3) s/childs/children/ -- applies to the subject line as well > > Signed-off-by: Anthony PERARD > --- > OvmfPkg/XenBusDxe/XenBusDxe.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.c b/OvmfPkg/XenBusDxe/XenBusDxe.c > index 0e63707f50..fac6f3a09d 100644 > --- a/OvmfPkg/XenBusDxe/XenBusDxe.c > +++ b/OvmfPkg/XenBusDxe/XenBusDxe.c > @@ -453,6 +453,11 @@ XenBusDxeDriverBindingStop ( >continue; > } > > +Status = gBS->CloseProtocol (Dev->ControllerHandle, , > + Dev->This->DriverBindingHandle, > + ChildData->Handle); > +ASSERT_EFI_ERROR (Status); > + (4) The indentation of function call arguments is inconsistent in this driver. Still, if it's not a lot of trouble, please correct the indentation here. Please pick one of the two below: (4a) Status = gBS->CloseProtocol ( Dev->ControllerHandle, , Dev->This->DriverBindingHandle, ChildData->Handle ); (4b) Status = gBS->CloseProtocol (Dev->ControllerHandle, , Dev->This->DriverBindingHandle, ChildData->Handle); With those updates, please add, to v2: Reviewed-by: Laszlo Ersek (5) Side remark (no need to do anything about it in the scope of this patch): I think the DisconnectController() call in XenBusDxeDriverBindingStop() is superfluous. That kind of disconnection is not the job of EFI_DRIVER_BINDING_PROTOCOL.Stop(). Thanks Laszlo > Status = gBS->UninstallMultipleProtocolInterfaces ( > ChildData->Handle, > , ChildData->DevicePath, > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43049): https://edk2.groups.io/g/devel/message/43049 Mute This Topic: https://groups.io/mt/32243460/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] OvmfPkg/XenBusDxe: Close XenIoProtocol openned by childs
In XenBusDxe, the XenBusAddDevice() opens the gXenIoProtocolGuid on behalf of child controllers. It is never closed and prevent from uninstalling the protocol. Close it were we stop all the childs in XenBusDxe->Stop(). Signed-off-by: Anthony PERARD --- OvmfPkg/XenBusDxe/XenBusDxe.c | 5 + 1 file changed, 5 insertions(+) diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.c b/OvmfPkg/XenBusDxe/XenBusDxe.c index 0e63707f50..fac6f3a09d 100644 --- a/OvmfPkg/XenBusDxe/XenBusDxe.c +++ b/OvmfPkg/XenBusDxe/XenBusDxe.c @@ -453,6 +453,11 @@ XenBusDxeDriverBindingStop ( continue; } +Status = gBS->CloseProtocol (Dev->ControllerHandle, , + Dev->This->DriverBindingHandle, + ChildData->Handle); +ASSERT_EFI_ERROR (Status); + Status = gBS->UninstallMultipleProtocolInterfaces ( ChildData->Handle, , ChildData->DevicePath, -- Anthony PERARD -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43031): https://edk2.groups.io/g/devel/message/43031 Mute This Topic: https://groups.io/mt/32243460/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-