[v5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

[edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

Posted by Star Zeng 6 years, 9 months ago

From: Amit Kumar <amit.ak@samsung.com>

Change since v4: Revise the patch based on V4 sent by Amit Kumar
1) Only return the corresponding protocol interface in *Interface
if the return status is EFI_SUCCESS or EFI_ALREADY_STARTED.
2) Interface is returned unmodified for all error conditions except
EFI_UNSUPPORTED and EFI_ALREADY_STARTED, NULL will be returned in
*Interface when EFI_UNSUPPORTED and Attributes is not
EFI_OPEN_PROTOCOL_TEST_PROTOCOL, the protocol interface will be
returned in *Interface when EFI_ALREADY_STARTED.

Change since v3:
1) Fixed issue when Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL
and Inteface = NULL case. [Reported by:star.zeng at intel.com]

Change Since v2:
1) Modified to use EFI_ERROR to get status code

Change since v1:
1) Fixed typo protocal to protocol
2) Fixed coding style

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Amit Kumar <amit.ak@samsung.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Gabriel Somlo <gsomlo@gmail.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Amit Kumar <amit.ak@samsung.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Core/Dxe/Hand/Handle.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c b/MdeModulePkg/Core/Dxe/Hand/Handle.c
index 59b89148c8f0..3862a3876f4a 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
@@ -1006,12 +1006,8 @@ CoreOpenProtocol (
   //
   // Check for invalid Interface
   //
-  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
-    if (Interface == NULL) {
-      return EFI_INVALID_PARAMETER;
-    } else {
-      *Interface = NULL;
-    }
+  if ((Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) && (Interface == NULL)) {
+    return EFI_INVALID_PARAMETER;
   }
 
   //
@@ -1078,12 +1074,6 @@ CoreOpenProtocol (
     goto Done;
   }
 
-  //
-  // This is the protocol interface entry for this protocol
-  //
-  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
-    *Interface = Prot->Interface;
-  }
   Status = EFI_SUCCESS;
 
   ByDriver        = FALSE;
@@ -1177,8 +1167,28 @@ CoreOpenProtocol (
   }
 
 Done:
+
+  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
+    //
+    // Keep Interface unmodified in case of any Error
+    // except EFI_ALREADY_STARTED and EFI_UNSUPPORTED.
+    //
+    if (!EFI_ERROR (Status) || Status == EFI_ALREADY_STARTED) {
+      //
+      // EFI_ALREADY_STARTED is not an error for bus driver.
+      // Return the corresponding protocol interface. 
+      //
+      *Interface = Prot->Interface;
+    } else if (Status == EFI_UNSUPPORTED) {
+      //
+      // Return NULL Interface if Unsupported Protocol.
+      //
+      *Interface = NULL;
+    }
+  }
+
   //
-  // Done. Release the database lock are return
+  // Done. Release the database lock and return
   //
   CoreReleaseProtocolLock ();
   return Status;
-- 
2.7.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

Posted by Amit Kumar 6 years, 9 months ago

 Spec + Pach Looks fine to me.
 
--------- Original Message ---------
Sender : Star Zeng <star.zeng@intel.com>
Date   : 2017-06-28 18:52 (GMT+5:30)
Title  : [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol
To : edk2-devel@lists.01.org
CC : Amit Kumar<amit.ak@samsung.com>, null<lersek@redhat.com>, null<michael.d.kinney@intel.com>, null<liming.gao@intel.com>, null<gsomlo@gmail.com>, null<star.zeng@intel.com>
 
From: Amit Kumar <amit.ak@samsung.com>
 
Change since v4: Revise the patch based on V4 sent by Amit Kumar
1) Only return the corresponding protocol interface in *Interface
if the return status is EFI_SUCCESS or EFI_ALREADY_STARTED.
2) Interface is returned unmodified for all error conditions except
EFI_UNSUPPORTED and EFI_ALREADY_STARTED, NULL will be returned in
*Interface when EFI_UNSUPPORTED and Attributes is not
EFI_OPEN_PROTOCOL_TEST_PROTOCOL, the protocol interface will be
returned in *Interface when EFI_ALREADY_STARTED.
 
Change since v3:
1) Fixed issue when Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL
and Inteface = NULL case. [Reported by:star.zeng at intel.com]
 
Change Since v2:
1) Modified to use EFI_ERROR to get status code
 
Change since v1:
1) Fixed typo protocal to protocol
2) Fixed coding style
 
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Amit Kumar <amit.ak@samsung.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Gabriel Somlo <gsomlo@gmail.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Amit Kumar <amit.ak@samsung.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Core/Dxe/Hand/Handle.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)
 
diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c b/MdeModulePkg/Core/Dxe/Hand/Handle.c
index 59b89148c8f0..3862a3876f4a 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
@@ -1006,12 +1006,8 @@ CoreOpenProtocol (
   //
   // Check for invalid Interface
   //
-  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
-    if (Interface == NULL) {
-      return EFI_INVALID_PARAMETER;
-    } else {
-      *Interface = NULL;
-    }
+  if ((Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) && (Interface == NULL)) {
+    return EFI_INVALID_PARAMETER;
   }
 
   //
@@ -1078,12 +1074,6 @@ CoreOpenProtocol (
     goto Done;
   }
 
-  //
-  // This is the protocol interface entry for this protocol
-  //
-  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
-    *Interface = Prot->Interface;
-  }
   Status = EFI_SUCCESS;
 
   ByDriver        = FALSE;
@@ -1177,8 +1167,28 @@ CoreOpenProtocol (
   }
 
 Done:
+
+  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
+    //
+    // Keep Interface unmodified in case of any Error
+    // except EFI_ALREADY_STARTED and EFI_UNSUPPORTED.
+    //
+    if (!EFI_ERROR (Status) || Status == EFI_ALREADY_STARTED) {
+      //
+      // EFI_ALREADY_STARTED is not an error for bus driver.
+      // Return the corresponding protocol interface. 
+      //
+      *Interface = Prot->Interface;
+    } else if (Status == EFI_UNSUPPORTED) {
+      //
+      // Return NULL Interface if Unsupported Protocol.
+      //
+      *Interface = NULL;
+    }
+  }
+
   //
-  // Done. Release the database lock are return
+  // Done. Release the database lock and return
   //
   CoreReleaseProtocolLock ();
   return Status;
-- 
2.7.0.windows.1
 
 
 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

Posted by Gabriel L. Somlo 6 years, 9 months ago

On Wed, Jun 28, 2017 at 09:22:50PM +0800, Star Zeng wrote:
> From: Amit Kumar <amit.ak@samsung.com>
> 
> Change since v4: Revise the patch based on V4 sent by Amit Kumar
> 1) Only return the corresponding protocol interface in *Interface
> if the return status is EFI_SUCCESS or EFI_ALREADY_STARTED.
> 2) Interface is returned unmodified for all error conditions except
> EFI_UNSUPPORTED and EFI_ALREADY_STARTED, NULL will be returned in
> *Interface when EFI_UNSUPPORTED and Attributes is not
> EFI_OPEN_PROTOCOL_TEST_PROTOCOL, the protocol interface will be
> returned in *Interface when EFI_ALREADY_STARTED.

Tested-by: Gabriel Somlo <gsomlo@gmail.com>

With this patch applied, OVMF works fine for me now.

Thanks much,
--Gabriel

> Change since v3:
> 1) Fixed issue when Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL
> and Inteface = NULL case. [Reported by:star.zeng at intel.com]
> 
> Change Since v2:
> 1) Modified to use EFI_ERROR to get status code
> 
> Change since v1:
> 1) Fixed typo protocal to protocol
> 2) Fixed coding style
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Amit Kumar <amit.ak@samsung.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Gabriel Somlo <gsomlo@gmail.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Amit Kumar <amit.ak@samsung.com>
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Core/Dxe/Hand/Handle.c | 36 +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> index 59b89148c8f0..3862a3876f4a 100644
> --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
> +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> @@ -1006,12 +1006,8 @@ CoreOpenProtocol (
>    //
>    // Check for invalid Interface
>    //
> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> -    if (Interface == NULL) {
> -      return EFI_INVALID_PARAMETER;
> -    } else {
> -      *Interface = NULL;
> -    }
> +  if ((Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) && (Interface == NULL)) {
> +    return EFI_INVALID_PARAMETER;
>    }
>  
>    //
> @@ -1078,12 +1074,6 @@ CoreOpenProtocol (
>      goto Done;
>    }
>  
> -  //
> -  // This is the protocol interface entry for this protocol
> -  //
> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> -    *Interface = Prot->Interface;
> -  }
>    Status = EFI_SUCCESS;
>  
>    ByDriver        = FALSE;
> @@ -1177,8 +1167,28 @@ CoreOpenProtocol (
>    }
>  
>  Done:
> +
> +  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> +    //
> +    // Keep Interface unmodified in case of any Error
> +    // except EFI_ALREADY_STARTED and EFI_UNSUPPORTED.
> +    //
> +    if (!EFI_ERROR (Status) || Status == EFI_ALREADY_STARTED) {
> +      //
> +      // EFI_ALREADY_STARTED is not an error for bus driver.
> +      // Return the corresponding protocol interface. 
> +      //
> +      *Interface = Prot->Interface;
> +    } else if (Status == EFI_UNSUPPORTED) {
> +      //
> +      // Return NULL Interface if Unsupported Protocol.
> +      //
> +      *Interface = NULL;
> +    }
> +  }
> +
>    //
> -  // Done. Release the database lock are return
> +  // Done. Release the database lock and return
>    //
>    CoreReleaseProtocolLock ();
>    return Status;
> -- 
> 2.7.0.windows.1
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

Posted by Laszlo Ersek 6 years, 9 months ago

On 06/28/17 15:22, Star Zeng wrote:
> From: Amit Kumar <amit.ak@samsung.com>
> 
> Change since v4: Revise the patch based on V4 sent by Amit Kumar
> 1) Only return the corresponding protocol interface in *Interface
> if the return status is EFI_SUCCESS or EFI_ALREADY_STARTED.
> 2) Interface is returned unmodified for all error conditions except
> EFI_UNSUPPORTED and EFI_ALREADY_STARTED, NULL will be returned in
> *Interface when EFI_UNSUPPORTED and Attributes is not
> EFI_OPEN_PROTOCOL_TEST_PROTOCOL, the protocol interface will be
> returned in *Interface when EFI_ALREADY_STARTED.
> 
> Change since v3:
> 1) Fixed issue when Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL
> and Inteface = NULL case. [Reported by:star.zeng at intel.com]
> 
> Change Since v2:
> 1) Modified to use EFI_ERROR to get status code
> 
> Change since v1:
> 1) Fixed typo protocal to protocol
> 2) Fixed coding style
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Amit Kumar <amit.ak@samsung.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Gabriel Somlo <gsomlo@gmail.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Amit Kumar <amit.ak@samsung.com>
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Core/Dxe/Hand/Handle.c | 36 +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

> diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> index 59b89148c8f0..3862a3876f4a 100644
> --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
> +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> @@ -1006,12 +1006,8 @@ CoreOpenProtocol (
>    //
>    // Check for invalid Interface
>    //
> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> -    if (Interface == NULL) {
> -      return EFI_INVALID_PARAMETER;
> -    } else {
> -      *Interface = NULL;
> -    }
> +  if ((Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) && (Interface == NULL)) {
> +    return EFI_INVALID_PARAMETER;
>    }
>  
>    //
> @@ -1078,12 +1074,6 @@ CoreOpenProtocol (
>      goto Done;
>    }
>  
> -  //
> -  // This is the protocol interface entry for this protocol
> -  //
> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> -    *Interface = Prot->Interface;
> -  }
>    Status = EFI_SUCCESS;
>  
>    ByDriver        = FALSE;
> @@ -1177,8 +1167,28 @@ CoreOpenProtocol (
>    }
>  
>  Done:
> +
> +  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> +    //
> +    // Keep Interface unmodified in case of any Error
> +    // except EFI_ALREADY_STARTED and EFI_UNSUPPORTED.
> +    //
> +    if (!EFI_ERROR (Status) || Status == EFI_ALREADY_STARTED) {
> +      //
> +      // EFI_ALREADY_STARTED is not an error for bus driver.
> +      // Return the corresponding protocol interface. 
> +      //
> +      *Interface = Prot->Interface;
> +    } else if (Status == EFI_UNSUPPORTED) {
> +      //
> +      // Return NULL Interface if Unsupported Protocol.
> +      //
> +      *Interface = NULL;
> +    }
> +  }
> +
>    //
> -  // Done. Release the database lock are return
> +  // Done. Release the database lock and return
>    //
>    CoreReleaseProtocolLock ();
>    return Status;
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

Posted by Amit kumar 6 years, 9 months ago

Try this; just a quick thing



EFI_STATUS
EFIAPI
UefiMain (
  IN EFI_HANDLE        ImageHandle,
  IN EFI_SYSTEM_TABLE  *SystemTable
  )
{
    EFI_STATUS Status;

ConvertHandleIndexToHandle((UINTN)Intermediate);


gBS->DisconnectController (

       ConvertHandleIndexToHandle (175),

       ConvertHandleIndexToHandle (176),
       NULL);

return (Status); }


________________________________
From: edk2-devel <edk2-devel-bounces@lists.01.org> on behalf of Laszlo Ersek <lersek@redhat.com>
Sent: Wednesday, June 28, 2017 7:43:45 PM
To: Star Zeng; edk2-devel@lists.01.org
Cc: Michael D Kinney; Gabriel Somlo; Liming Gao
Subject: Re: [edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

On 06/28/17 15:22, Star Zeng wrote:
> From: Amit Kumar <amit.ak@samsung.com>
>
> Change since v4: Revise the patch based on V4 sent by Amit Kumar
> 1) Only return the corresponding protocol interface in *Interface
> if the return status is EFI_SUCCESS or EFI_ALREADY_STARTED.
> 2) Interface is returned unmodified for all error conditions except
> EFI_UNSUPPORTED and EFI_ALREADY_STARTED, NULL will be returned in
> *Interface when EFI_UNSUPPORTED and Attributes is not
> EFI_OPEN_PROTOCOL_TEST_PROTOCOL, the protocol interface will be
> returned in *Interface when EFI_ALREADY_STARTED.
>
> Change since v3:
> 1) Fixed issue when Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL
> and Inteface = NULL case. [Reported by:star.zeng at intel.com]
>
> Change Since v2:
> 1) Modified to use EFI_ERROR to get status code
>
> Change since v1:
> 1) Fixed typo protocal to protocol
> 2) Fixed coding style
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Amit Kumar <amit.ak@samsung.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Gabriel Somlo <gsomlo@gmail.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Amit Kumar <amit.ak@samsung.com>
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Core/Dxe/Hand/Handle.c | 36 +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

> diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> index 59b89148c8f0..3862a3876f4a 100644
> --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
> +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> @@ -1006,12 +1006,8 @@ CoreOpenProtocol (
>    //
>    // Check for invalid Interface
>    //
> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> -    if (Interface == NULL) {
> -      return EFI_INVALID_PARAMETER;
> -    } else {
> -      *Interface = NULL;
> -    }
> +  if ((Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) && (Interface == NULL)) {
> +    return EFI_INVALID_PARAMETER;
>    }
>
>    //
> @@ -1078,12 +1074,6 @@ CoreOpenProtocol (
>      goto Done;
>    }
>
> -  //
> -  // This is the protocol interface entry for this protocol
> -  //
> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> -    *Interface = Prot->Interface;
> -  }
>    Status = EFI_SUCCESS;
>
>    ByDriver        = FALSE;
> @@ -1177,8 +1167,28 @@ CoreOpenProtocol (
>    }
>
>  Done:
> +
> +  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> +    //
> +    // Keep Interface unmodified in case of any Error
> +    // except EFI_ALREADY_STARTED and EFI_UNSUPPORTED.
> +    //
> +    if (!EFI_ERROR (Status) || Status == EFI_ALREADY_STARTED) {
> +      //
> +      // EFI_ALREADY_STARTED is not an error for bus driver.
> +      // Return the corresponding protocol interface.
> +      //
> +      *Interface = Prot->Interface;
> +    } else if (Status == EFI_UNSUPPORTED) {
> +      //
> +      // Return NULL Interface if Unsupported Protocol.
> +      //
> +      *Interface = NULL;
> +    }
> +  }
> +
>    //
> -  // Done. Release the database lock are return
> +  // Done. Release the database lock and return
>    //
>    CoreReleaseProtocolLock ();
>    return Status;
>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

Posted by Laszlo Ersek 6 years, 9 months ago

On 06/28/17 20:08, Amit kumar wrote:
> Try this; just a quick thing
> 
> 
> 
> EFI_STATUS
> EFIAPI
> UefiMain (
>   IN EFI_HANDLE        ImageHandle,
>   IN EFI_SYSTEM_TABLE  *SystemTable
>   )
> {
>     EFI_STATUS Status;
> 
> ConvertHandleIndexToHandle((UINTN)Intermediate);
> 
> 
> gBS->DisconnectController (
> 
>        ConvertHandleIndexToHandle (175),
> 
>        ConvertHandleIndexToHandle (176),
>        NULL);
> 
> return (Status); }

Sorry, what is this program supposed to do, and what are the (likely
undesirable) symptoms that you are witnessing (... presumably as a
consequence of the v5 patch below)?

Thanks
Laszlo

> 
> 
> ________________________________
> From: edk2-devel <edk2-devel-bounces@lists.01.org> on behalf of Laszlo Ersek <lersek@redhat.com>
> Sent: Wednesday, June 28, 2017 7:43:45 PM
> To: Star Zeng; edk2-devel@lists.01.org
> Cc: Michael D Kinney; Gabriel Somlo; Liming Gao
> Subject: Re: [edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol
> 
> On 06/28/17 15:22, Star Zeng wrote:
>> From: Amit Kumar <amit.ak@samsung.com>
>>
>> Change since v4: Revise the patch based on V4 sent by Amit Kumar
>> 1) Only return the corresponding protocol interface in *Interface
>> if the return status is EFI_SUCCESS or EFI_ALREADY_STARTED.
>> 2) Interface is returned unmodified for all error conditions except
>> EFI_UNSUPPORTED and EFI_ALREADY_STARTED, NULL will be returned in
>> *Interface when EFI_UNSUPPORTED and Attributes is not
>> EFI_OPEN_PROTOCOL_TEST_PROTOCOL, the protocol interface will be
>> returned in *Interface when EFI_ALREADY_STARTED.
>>
>> Change since v3:
>> 1) Fixed issue when Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL
>> and Inteface = NULL case. [Reported by:star.zeng at intel.com]
>>
>> Change Since v2:
>> 1) Modified to use EFI_ERROR to get status code
>>
>> Change since v1:
>> 1) Fixed typo protocal to protocol
>> 2) Fixed coding style
>>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Amit Kumar <amit.ak@samsung.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Gabriel Somlo <gsomlo@gmail.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Amit Kumar <amit.ak@samsung.com>
>> Signed-off-by: Star Zeng <star.zeng@intel.com>
>> ---
>>  MdeModulePkg/Core/Dxe/Hand/Handle.c | 36 +++++++++++++++++++++++-------------
>>  1 file changed, 23 insertions(+), 13 deletions(-)
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Tested-by: Laszlo Ersek <lersek@redhat.com>
> 
> Thanks!
> Laszlo
> 
>> diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c b/MdeModulePkg/Core/Dxe/Hand/Handle.c
>> index 59b89148c8f0..3862a3876f4a 100644
>> --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
>> +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
>> @@ -1006,12 +1006,8 @@ CoreOpenProtocol (
>>    //
>>    // Check for invalid Interface
>>    //
>> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
>> -    if (Interface == NULL) {
>> -      return EFI_INVALID_PARAMETER;
>> -    } else {
>> -      *Interface = NULL;
>> -    }
>> +  if ((Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) && (Interface == NULL)) {
>> +    return EFI_INVALID_PARAMETER;
>>    }
>>
>>    //
>> @@ -1078,12 +1074,6 @@ CoreOpenProtocol (
>>      goto Done;
>>    }
>>
>> -  //
>> -  // This is the protocol interface entry for this protocol
>> -  //
>> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
>> -    *Interface = Prot->Interface;
>> -  }
>>    Status = EFI_SUCCESS;
>>
>>    ByDriver        = FALSE;
>> @@ -1177,8 +1167,28 @@ CoreOpenProtocol (
>>    }
>>
>>  Done:
>> +
>> +  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
>> +    //
>> +    // Keep Interface unmodified in case of any Error
>> +    // except EFI_ALREADY_STARTED and EFI_UNSUPPORTED.
>> +    //
>> +    if (!EFI_ERROR (Status) || Status == EFI_ALREADY_STARTED) {
>> +      //
>> +      // EFI_ALREADY_STARTED is not an error for bus driver.
>> +      // Return the corresponding protocol interface.
>> +      //
>> +      *Interface = Prot->Interface;
>> +    } else if (Status == EFI_UNSUPPORTED) {
>> +      //
>> +      // Return NULL Interface if Unsupported Protocol.
>> +      //
>> +      *Interface = NULL;
>> +    }
>> +  }
>> +
>>    //
>> -  // Done. Release the database lock are return
>> +  // Done. Release the database lock and return
>>    //
>>    CoreReleaseProtocolLock ();
>>    return Status;
>>
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

Posted by Zeng, Star 6 years, 7 months ago

Thanks all the Tested-by and Reviewed-by.

The new UEFI spec 2.7a has been published at http://www.uefi.org/sites/default/files/resources/UEFI%20Spec%202_7_A%20Sept%206.pdf
which includes the spec change "1815 OpenProtocol() / EFI_ALREADY_STARTED should output existent Interface" that is required for this patch.

If no more comments, I will go to push this patch. :)


Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Wednesday, June 28, 2017 10:14 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Amit Kumar <amit.ak@samsung.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Gabriel Somlo <gsomlo@gmail.com>
Subject: Re: [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

On 06/28/17 15:22, Star Zeng wrote:
> From: Amit Kumar <amit.ak@samsung.com>
> 
> Change since v4: Revise the patch based on V4 sent by Amit Kumar
> 1) Only return the corresponding protocol interface in *Interface if 
> the return status is EFI_SUCCESS or EFI_ALREADY_STARTED.
> 2) Interface is returned unmodified for all error conditions except 
> EFI_UNSUPPORTED and EFI_ALREADY_STARTED, NULL will be returned in 
> *Interface when EFI_UNSUPPORTED and Attributes is not 
> EFI_OPEN_PROTOCOL_TEST_PROTOCOL, the protocol interface will be 
> returned in *Interface when EFI_ALREADY_STARTED.
> 
> Change since v3:
> 1) Fixed issue when Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL and 
> Inteface = NULL case. [Reported by:star.zeng at intel.com]
> 
> Change Since v2:
> 1) Modified to use EFI_ERROR to get status code
> 
> Change since v1:
> 1) Fixed typo protocal to protocol
> 2) Fixed coding style
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Amit Kumar <amit.ak@samsung.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Gabriel Somlo <gsomlo@gmail.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Amit Kumar <amit.ak@samsung.com>
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Core/Dxe/Hand/Handle.c | 36 
> +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

> diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c 
> b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> index 59b89148c8f0..3862a3876f4a 100644
> --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
> +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> @@ -1006,12 +1006,8 @@ CoreOpenProtocol (
>    //
>    // Check for invalid Interface
>    //
> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> -    if (Interface == NULL) {
> -      return EFI_INVALID_PARAMETER;
> -    } else {
> -      *Interface = NULL;
> -    }
> +  if ((Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) && (Interface == NULL)) {
> +    return EFI_INVALID_PARAMETER;
>    }
>  
>    //
> @@ -1078,12 +1074,6 @@ CoreOpenProtocol (
>      goto Done;
>    }
>  
> -  //
> -  // This is the protocol interface entry for this protocol
> -  //
> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> -    *Interface = Prot->Interface;
> -  }
>    Status = EFI_SUCCESS;
>  
>    ByDriver        = FALSE;
> @@ -1177,8 +1167,28 @@ CoreOpenProtocol (
>    }
>  
>  Done:
> +
> +  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> +    //
> +    // Keep Interface unmodified in case of any Error
> +    // except EFI_ALREADY_STARTED and EFI_UNSUPPORTED.
> +    //
> +    if (!EFI_ERROR (Status) || Status == EFI_ALREADY_STARTED) {
> +      //
> +      // EFI_ALREADY_STARTED is not an error for bus driver.
> +      // Return the corresponding protocol interface. 
> +      //
> +      *Interface = Prot->Interface;
> +    } else if (Status == EFI_UNSUPPORTED) {
> +      //
> +      // Return NULL Interface if Unsupported Protocol.
> +      //
> +      *Interface = NULL;
> +    }
> +  }
> +
>    //
> -  // Done. Release the database lock are return
> +  // Done. Release the database lock and return
>    //
>    CoreReleaseProtocolLock ();
>    return Status;
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

Posted by Zeng, Star 6 years, 7 months ago

Thanks all.
The patch has been pushed at 89f7f2cdf0266619976cb53b45b5de1aba2f8fac.

Star
-----Original Message-----
From: Zeng, Star 
Sent: Tuesday, September 19, 2017 3:14 PM
To: Laszlo Ersek <lersek@redhat.com>; edk2-devel@lists.01.org
Cc: Amit Kumar <amit.ak@samsung.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Gabriel Somlo <gsomlo@gmail.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

Thanks all the Tested-by and Reviewed-by.

The new UEFI spec 2.7a has been published at http://www.uefi.org/sites/default/files/resources/UEFI%20Spec%202_7_A%20Sept%206.pdf
which includes the spec change "1815 OpenProtocol() / EFI_ALREADY_STARTED should output existent Interface" that is required for this patch.

If no more comments, I will go to push this patch. :)


Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Wednesday, June 28, 2017 10:14 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Amit Kumar <amit.ak@samsung.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Gabriel Somlo <gsomlo@gmail.com>
Subject: Re: [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

On 06/28/17 15:22, Star Zeng wrote:
> From: Amit Kumar <amit.ak@samsung.com>
> 
> Change since v4: Revise the patch based on V4 sent by Amit Kumar
> 1) Only return the corresponding protocol interface in *Interface if 
> the return status is EFI_SUCCESS or EFI_ALREADY_STARTED.
> 2) Interface is returned unmodified for all error conditions except 
> EFI_UNSUPPORTED and EFI_ALREADY_STARTED, NULL will be returned in 
> *Interface when EFI_UNSUPPORTED and Attributes is not 
> EFI_OPEN_PROTOCOL_TEST_PROTOCOL, the protocol interface will be 
> returned in *Interface when EFI_ALREADY_STARTED.
> 
> Change since v3:
> 1) Fixed issue when Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL and 
> Inteface = NULL case. [Reported by:star.zeng at intel.com]
> 
> Change Since v2:
> 1) Modified to use EFI_ERROR to get status code
> 
> Change since v1:
> 1) Fixed typo protocal to protocol
> 2) Fixed coding style
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Amit Kumar <amit.ak@samsung.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Gabriel Somlo <gsomlo@gmail.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Amit Kumar <amit.ak@samsung.com>
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Core/Dxe/Hand/Handle.c | 36
> +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

> diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c
> b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> index 59b89148c8f0..3862a3876f4a 100644
> --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
> +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> @@ -1006,12 +1006,8 @@ CoreOpenProtocol (
>    //
>    // Check for invalid Interface
>    //
> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> -    if (Interface == NULL) {
> -      return EFI_INVALID_PARAMETER;
> -    } else {
> -      *Interface = NULL;
> -    }
> +  if ((Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) && (Interface == NULL)) {
> +    return EFI_INVALID_PARAMETER;
>    }
>  
>    //
> @@ -1078,12 +1074,6 @@ CoreOpenProtocol (
>      goto Done;
>    }
>  
> -  //
> -  // This is the protocol interface entry for this protocol
> -  //
> -  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> -    *Interface = Prot->Interface;
> -  }
>    Status = EFI_SUCCESS;
>  
>    ByDriver        = FALSE;
> @@ -1177,8 +1167,28 @@ CoreOpenProtocol (
>    }
>  
>  Done:
> +
> +  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> +    //
> +    // Keep Interface unmodified in case of any Error
> +    // except EFI_ALREADY_STARTED and EFI_UNSUPPORTED.
> +    //
> +    if (!EFI_ERROR (Status) || Status == EFI_ALREADY_STARTED) {
> +      //
> +      // EFI_ALREADY_STARTED is not an error for bus driver.
> +      // Return the corresponding protocol interface. 
> +      //
> +      *Interface = Prot->Interface;
> +    } else if (Status == EFI_UNSUPPORTED) {
> +      //
> +      // Return NULL Interface if Unsupported Protocol.
> +      //
> +      *Interface = NULL;
> +    }
> +  }
> +
>    //
> -  // Done. Release the database lock are return
> +  // Done. Release the database lock and return
>    //
>    CoreReleaseProtocolLock ();
>    return Status;
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

Posted by Amit kumar 6 years, 9 months ago

sorry, wrong thread

________________________________
From: edk2-devel <edk2-devel-bounces@lists.01.org> on behalf of Star Zeng <star.zeng@intel.com>
Sent: Wednesday, June 28, 2017 6:52:50 PM
To: edk2-devel@lists.01.org
Cc: Liming Gao; Gabriel Somlo; Michael D Kinney; Laszlo Ersek; Star Zeng
Subject: [edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

From: Amit Kumar <amit.ak@samsung.com>

Change since v4: Revise the patch based on V4 sent by Amit Kumar
1) Only return the corresponding protocol interface in *Interface
if the return status is EFI_SUCCESS or EFI_ALREADY_STARTED.
2) Interface is returned unmodified for all error conditions except
EFI_UNSUPPORTED and EFI_ALREADY_STARTED, NULL will be returned in
*Interface when EFI_UNSUPPORTED and Attributes is not
EFI_OPEN_PROTOCOL_TEST_PROTOCOL, the protocol interface will be
returned in *Interface when EFI_ALREADY_STARTED.

Change since v3:
1) Fixed issue when Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL
and Inteface = NULL case. [Reported by:star.zeng at intel.com]

Change Since v2:
1) Modified to use EFI_ERROR to get status code

Change since v1:
1) Fixed typo protocal to protocol
2) Fixed coding style

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Amit Kumar <amit.ak@samsung.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Gabriel Somlo <gsomlo@gmail.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Amit Kumar <amit.ak@samsung.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Core/Dxe/Hand/Handle.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c b/MdeModulePkg/Core/Dxe/Hand/Handle.c
index 59b89148c8f0..3862a3876f4a 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
@@ -1006,12 +1006,8 @@ CoreOpenProtocol (
   //
   // Check for invalid Interface
   //
-  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
-    if (Interface == NULL) {
-      return EFI_INVALID_PARAMETER;
-    } else {
-      *Interface = NULL;
-    }
+  if ((Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) && (Interface == NULL)) {
+    return EFI_INVALID_PARAMETER;
   }

   //
@@ -1078,12 +1074,6 @@ CoreOpenProtocol (
     goto Done;
   }

-  //
-  // This is the protocol interface entry for this protocol
-  //
-  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
-    *Interface = Prot->Interface;
-  }
   Status = EFI_SUCCESS;

   ByDriver        = FALSE;
@@ -1177,8 +1167,28 @@ CoreOpenProtocol (
   }

 Done:
+
+  if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
+    //
+    // Keep Interface unmodified in case of any Error
+    // except EFI_ALREADY_STARTED and EFI_UNSUPPORTED.
+    //
+    if (!EFI_ERROR (Status) || Status == EFI_ALREADY_STARTED) {
+      //
+      // EFI_ALREADY_STARTED is not an error for bus driver.
+      // Return the corresponding protocol interface.
+      //
+      *Interface = Prot->Interface;
+    } else if (Status == EFI_UNSUPPORTED) {
+      //
+      // Return NULL Interface if Unsupported Protocol.
+      //
+      *Interface = NULL;
+    }
+  }
+
   //
-  // Done. Release the database lock are return
+  // Done. Release the database lock and return
   //
   CoreReleaseProtocolLock ();
   return Status;
--
2.7.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH V5] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

Posted by Laszlo Ersek 6 years, 9 months ago

On 06/28/17 20:10, Amit kumar wrote:
> sorry, wrong thread

Ah, OK. Ignore my questions then.

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel