* [PATCH edk2-platforms v2] Silicon/Broadcom/BcmGenetDxe: implement media state adapter info protocol
@ 2020-06-17 18:15 Ard Biesheuvel
2020-06-17 18:22 ` Samer El-Haj-Mahmoud
0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2020-06-17 18:15 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Pete Batard, Andrei Warkentin,
Samer El-Haj-Mahmoud
NetLibDetectMedia () in DxeNetLib is used as a fallback on implementations
of the SNP protocol that do not also carry an implementation of the adapter
info protocol to provide media state information, and it does all kinds of
terrible things to the network interface (stopping and restarting multiple
times, reprogramming the multicast filters etc etc) to workaround some
alleged UNDI shortcoming.
Although our GENET code should be bullet proof and therefore able to take
this kind of abuse, it is better to avoid it, and provide an implementation
of the adapter info protocol that returns the media state directly, without
the need to mistreat the SNP layer.
Cc: Pete Batard <pete@akeo.ie>
Cc: Andrei Warkentin (awarkentin@vmware.com) <awarkentin@vmware.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
v2:
- use the correct accessor to retrieve the GENET_PRIVATE_DATA pointer
- call into the PHY routines to get the state, to ensure it is not stale
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf | 3 +
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h | 5 +
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c | 106 ++++++++++++++++++++
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c | 12 ++-
4 files changed, 121 insertions(+), 5 deletions(-)
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
index 28f3e66ebaf0..89dee9f10c83 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
@@ -19,6 +19,7 @@ [Defines]
UNLOAD_IMAGE = GenetUnload
[Sources]
+ AdapterInfo.c
BcmGenetDxe.h
ComponentName.c
DriverBinding.c
@@ -49,10 +50,12 @@ [LibraryClasses]
[Protocols]
gBcmGenetPlatformDeviceProtocolGuid ## TO_START
+ gEfiAdapterInformationProtocolGuid ## BY_START
gEfiDevicePathProtocolGuid ## BY_START
gEfiSimpleNetworkProtocolGuid ## BY_START
[Guids]
+ gEfiAdapterInfoMediaStateGuid
gEfiEventExitBootServicesGuid
[FixedPcd]
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
index 0af9d5209cf2..1a117b25f87f 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
@@ -14,6 +14,7 @@
#include <Uefi.h>
#include <Library/UefiLib.h>
#include <Protocol/BcmGenetPlatformDevice.h>
+#include <Protocol/AdapterInformation.h>
#include <Protocol/ComponentName.h>
#include <Protocol/ComponentName2.h>
#include <Protocol/SimpleNetwork.h>
@@ -209,6 +210,8 @@ typedef struct {
EFI_SIMPLE_NETWORK_PROTOCOL Snp;
EFI_SIMPLE_NETWORK_MODE SnpMode;
+ EFI_ADAPTER_INFORMATION_PROTOCOL Aip;
+
BCM_GENET_PLATFORM_DEVICE_PROTOCOL *Dev;
GENERIC_PHY_PRIVATE_DATA Phy;
@@ -234,9 +237,11 @@ extern EFI_COMPONENT_NAME_PROTOCOL gGenetComponentName;
extern EFI_COMPONENT_NAME2_PROTOCOL gGenetComponentName2;
extern CONST EFI_SIMPLE_NETWORK_PROTOCOL gGenetSimpleNetworkTemplate;
+extern CONST EFI_ADAPTER_INFORMATION_PROTOCOL gGenetAdapterInfoTemplate;
#define GENET_DRIVER_SIGNATURE SIGNATURE_32('G', 'N', 'E', 'T')
#define GENET_PRIVATE_DATA_FROM_SNP_THIS(a) CR(a, GENET_PRIVATE_DATA, Snp, GENET_DRIVER_SIGNATURE)
+#define GENET_PRIVATE_DATA_FROM_AIP_THIS(a) CR(a, GENET_PRIVATE_DATA, Aip, GENET_DRIVER_SIGNATURE)
#define GENET_RX_BUFFER(g, idx) ((UINT8 *)(UINTN)(g)->RxBuffer + GENET_MAX_PACKET_SIZE * (idx))
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
new file mode 100644
index 000000000000..4b42b2bba67a
--- /dev/null
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
@@ -0,0 +1,106 @@
+/** @file
+
+ Copyright (c) 2020 Arm, Limited. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+
+#include "BcmGenetDxe.h"
+
+STATIC
+EFI_STATUS
+EFIAPI
+GenetAipGetInformation (
+ IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
+ IN EFI_GUID *InformationType,
+ OUT VOID **InformationBlock,
+ OUT UINTN *InformationBlockSize
+ )
+{
+ EFI_ADAPTER_INFO_MEDIA_STATE *AdapterInfo;
+ GENET_PRIVATE_DATA *Genet;
+
+ if (This == NULL || InformationBlock == NULL ||
+ InformationBlockSize == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (!CompareGuid (InformationType, &gEfiAdapterInfoMediaStateGuid)) {
+ return EFI_UNSUPPORTED;
+ }
+
+ AdapterInfo = AllocateZeroPool (sizeof (EFI_ADAPTER_INFO_MEDIA_STATE));
+ if (AdapterInfo == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ *InformationBlock = AdapterInfo;
+ *InformationBlockSize = sizeof (EFI_ADAPTER_INFO_MEDIA_STATE);
+
+ Genet = GENET_PRIVATE_DATA_FROM_AIP_THIS (This);
+ AdapterInfo->MediaState = GenericPhyUpdateConfig (&Genet->Phy);
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+GenetAipSetInformation (
+ IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
+ IN EFI_GUID *InformationType,
+ IN VOID *InformationBlock,
+ IN UINTN InformationBlockSize
+ )
+{
+ if (This == NULL || InformationBlock == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (CompareGuid (InformationType, &gEfiAdapterInfoMediaStateGuid)) {
+ return EFI_WRITE_PROTECTED;
+ }
+
+ return EFI_UNSUPPORTED;
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+GenetAipGetSupportedTypes (
+ IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
+ OUT EFI_GUID **InfoTypesBuffer,
+ OUT UINTN *InfoTypesBufferCount
+ )
+{
+ EFI_GUID *Guid;
+
+ if (This == NULL || InfoTypesBuffer == NULL ||
+ InfoTypesBufferCount == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Guid = AllocatePool (sizeof *Guid);
+ if (Guid == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ CopyGuid (Guid, &gEfiAdapterInfoMediaStateGuid);
+
+ *InfoTypesBuffer = Guid;
+ *InfoTypesBufferCount = 1;
+
+ return EFI_SUCCESS;
+}
+
+CONST EFI_ADAPTER_INFORMATION_PROTOCOL gGenetAdapterInfoTemplate = {
+ GenetAipGetInformation,
+ GenetAipSetInformation,
+ GenetAipGetSupportedTypes,
+};
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
index 7f93c68cd608..f9aa006dc799 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
@@ -163,6 +163,7 @@ GenetDriverBindingStart (
EfiInitializeLock (&Genet->Lock, TPL_CALLBACK);
CopyMem (&Genet->Snp, &gGenetSimpleNetworkTemplate, sizeof Genet->Snp);
+ CopyMem (&Genet->Aip, &gGenetAdapterInfoTemplate, sizeof Genet->Aip);
Genet->Snp.Mode = &Genet->SnpMode;
Genet->SnpMode.State = EfiSimpleNetworkStopped;
@@ -203,7 +204,8 @@ GenetDriverBindingStart (
}
Status = gBS->InstallMultipleProtocolInterfaces (&ControllerHandle,
- &gEfiSimpleNetworkProtocolGuid, &Genet->Snp,
+ &gEfiSimpleNetworkProtocolGuid, &Genet->Snp,
+ &gEfiAdapterInformationProtocolGuid, &Genet->Aip,
NULL);
if (EFI_ERROR (Status)) {
@@ -273,10 +275,10 @@ GenetDriverBindingStop (
ASSERT (Genet->ControllerHandle == ControllerHandle);
- Status = gBS->UninstallProtocolInterface (ControllerHandle,
- &gEfiSimpleNetworkProtocolGuid,
- &Genet->Snp
- );
+ Status = gBS->UninstallMultipleProtocolInterfaces (ControllerHandle,
+ &gEfiSimpleNetworkProtocolGuid, &Genet->Snp,
+ &gEfiAdapterInformationProtocolGuid, &Genet->Aip,
+ NULL);
if (EFI_ERROR (Status)) {
return Status;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH edk2-platforms v2] Silicon/Broadcom/BcmGenetDxe: implement media state adapter info protocol
2020-06-17 18:15 [PATCH edk2-platforms v2] Silicon/Broadcom/BcmGenetDxe: implement media state adapter info protocol Ard Biesheuvel
@ 2020-06-17 18:22 ` Samer El-Haj-Mahmoud
2020-06-18 10:34 ` Ard Biesheuvel
0 siblings, 1 reply; 3+ messages in thread
From: Samer El-Haj-Mahmoud @ 2020-06-17 18:22 UTC (permalink / raw)
To: Ard Biesheuvel, devel@edk2.groups.io
Cc: Ard Biesheuvel, Pete Batard,
Andrei Warkentin (awarkentin@vmware.com), Samer El-Haj-Mahmoud
Tested on RPi4 w/ 4GB
Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Sent: Wednesday, June 17, 2020 2:15 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Pete Batard
> <pete@akeo.ie>; Andrei Warkentin (awarkentin@vmware.com)
> <awarkentin@vmware.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
> Mahmoud@arm.com>
> Subject: [PATCH edk2-platforms v2] Silicon/Broadcom/BcmGenetDxe:
> implement media state adapter info protocol
>
> NetLibDetectMedia () in DxeNetLib is used as a fallback on implementations
> of the SNP protocol that do not also carry an implementation of the adapter
> info protocol to provide media state information, and it does all kinds of
> terrible things to the network interface (stopping and restarting multiple
> times, reprogramming the multicast filters etc etc) to workaround some
> alleged UNDI shortcoming.
>
> Although our GENET code should be bullet proof and therefore able to take
> this kind of abuse, it is better to avoid it, and provide an implementation
> of the adapter info protocol that returns the media state directly, without
> the need to mistreat the SNP layer.
>
>
> Cc: Pete Batard <pete@akeo.ie>
>
> Cc: Andrei Warkentin (awarkentin@vmware.com)
> <awarkentin@vmware.com>
>
> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>
> v2:
>
> - use the correct accessor to retrieve the GENET_PRIVATE_DATA pointer
>
> - call into the PHY routines to get the state, to ensure it is not stale
>
>
> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf | 3 +
> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h | 5 +
> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c | 106
> ++++++++++++++++++++
> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c | 12 ++-
> 4 files changed, 121 insertions(+), 5 deletions(-)
>
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
> b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
> index 28f3e66ebaf0..89dee9f10c83 100644
> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
> @@ -19,6 +19,7 @@ [Defines]
> UNLOAD_IMAGE = GenetUnload
>
>
>
> [Sources]
>
> + AdapterInfo.c
>
> BcmGenetDxe.h
>
> ComponentName.c
>
> DriverBinding.c
>
> @@ -49,10 +50,12 @@ [LibraryClasses]
>
>
> [Protocols]
>
> gBcmGenetPlatformDeviceProtocolGuid ## TO_START
>
> + gEfiAdapterInformationProtocolGuid ## BY_START
>
> gEfiDevicePathProtocolGuid ## BY_START
>
> gEfiSimpleNetworkProtocolGuid ## BY_START
>
>
>
> [Guids]
>
> + gEfiAdapterInfoMediaStateGuid
>
> gEfiEventExitBootServicesGuid
>
>
>
> [FixedPcd]
>
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
> b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
> index 0af9d5209cf2..1a117b25f87f 100644
> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
> @@ -14,6 +14,7 @@
> #include <Uefi.h>
>
> #include <Library/UefiLib.h>
>
> #include <Protocol/BcmGenetPlatformDevice.h>
>
> +#include <Protocol/AdapterInformation.h>
>
> #include <Protocol/ComponentName.h>
>
> #include <Protocol/ComponentName2.h>
>
> #include <Protocol/SimpleNetwork.h>
>
> @@ -209,6 +210,8 @@ typedef struct {
> EFI_SIMPLE_NETWORK_PROTOCOL Snp;
>
> EFI_SIMPLE_NETWORK_MODE SnpMode;
>
>
>
> + EFI_ADAPTER_INFORMATION_PROTOCOL Aip;
>
> +
>
> BCM_GENET_PLATFORM_DEVICE_PROTOCOL *Dev;
>
>
>
> GENERIC_PHY_PRIVATE_DATA Phy;
>
> @@ -234,9 +237,11 @@ extern EFI_COMPONENT_NAME_PROTOCOL
> gGenetComponentName;
> extern EFI_COMPONENT_NAME2_PROTOCOL
> gGenetComponentName2;
>
>
>
> extern CONST EFI_SIMPLE_NETWORK_PROTOCOL
> gGenetSimpleNetworkTemplate;
>
> +extern CONST EFI_ADAPTER_INFORMATION_PROTOCOL
> gGenetAdapterInfoTemplate;
>
>
>
> #define GENET_DRIVER_SIGNATURE SIGNATURE_32('G', 'N', 'E', 'T')
>
> #define GENET_PRIVATE_DATA_FROM_SNP_THIS(a) CR(a,
> GENET_PRIVATE_DATA, Snp, GENET_DRIVER_SIGNATURE)
>
> +#define GENET_PRIVATE_DATA_FROM_AIP_THIS(a) CR(a,
> GENET_PRIVATE_DATA, Aip, GENET_DRIVER_SIGNATURE)
>
>
>
> #define GENET_RX_BUFFER(g, idx) ((UINT8 *)(UINTN)(g)->RxBuffer +
> GENET_MAX_PACKET_SIZE * (idx))
>
>
>
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
> b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
> new file mode 100644
> index 000000000000..4b42b2bba67a
> --- /dev/null
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
> @@ -0,0 +1,106 @@
> +/** @file
>
> +
>
> + Copyright (c) 2020 Arm, Limited. All rights reserved.
>
> +
>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +
>
> +**/
>
> +
>
> +#include <Uefi.h>
>
> +#include <Library/BaseMemoryLib.h>
>
> +#include <Library/DebugLib.h>
>
> +#include <Library/MemoryAllocationLib.h>
>
> +
>
> +#include "BcmGenetDxe.h"
>
> +
>
> +STATIC
>
> +EFI_STATUS
>
> +EFIAPI
>
> +GenetAipGetInformation (
>
> + IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
>
> + IN EFI_GUID *InformationType,
>
> + OUT VOID **InformationBlock,
>
> + OUT UINTN *InformationBlockSize
>
> + )
>
> +{
>
> + EFI_ADAPTER_INFO_MEDIA_STATE *AdapterInfo;
>
> + GENET_PRIVATE_DATA *Genet;
>
> +
>
> + if (This == NULL || InformationBlock == NULL ||
>
> + InformationBlockSize == NULL) {
>
> + return EFI_INVALID_PARAMETER;
>
> + }
>
> +
>
> + if (!CompareGuid (InformationType, &gEfiAdapterInfoMediaStateGuid)) {
>
> + return EFI_UNSUPPORTED;
>
> + }
>
> +
>
> + AdapterInfo = AllocateZeroPool (sizeof
> (EFI_ADAPTER_INFO_MEDIA_STATE));
>
> + if (AdapterInfo == NULL) {
>
> + return EFI_OUT_OF_RESOURCES;
>
> + }
>
> +
>
> + *InformationBlock = AdapterInfo;
>
> + *InformationBlockSize = sizeof (EFI_ADAPTER_INFO_MEDIA_STATE);
>
> +
>
> + Genet = GENET_PRIVATE_DATA_FROM_AIP_THIS (This);
>
> + AdapterInfo->MediaState = GenericPhyUpdateConfig (&Genet->Phy);
>
> +
>
> + return EFI_SUCCESS;
>
> +}
>
> +
>
> +STATIC
>
> +EFI_STATUS
>
> +EFIAPI
>
> +GenetAipSetInformation (
>
> + IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
>
> + IN EFI_GUID *InformationType,
>
> + IN VOID *InformationBlock,
>
> + IN UINTN InformationBlockSize
>
> + )
>
> +{
>
> + if (This == NULL || InformationBlock == NULL) {
>
> + return EFI_INVALID_PARAMETER;
>
> + }
>
> +
>
> + if (CompareGuid (InformationType, &gEfiAdapterInfoMediaStateGuid)) {
>
> + return EFI_WRITE_PROTECTED;
>
> + }
>
> +
>
> + return EFI_UNSUPPORTED;
>
> +}
>
> +
>
> +STATIC
>
> +EFI_STATUS
>
> +EFIAPI
>
> +GenetAipGetSupportedTypes (
>
> + IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
>
> + OUT EFI_GUID **InfoTypesBuffer,
>
> + OUT UINTN *InfoTypesBufferCount
>
> + )
>
> +{
>
> + EFI_GUID *Guid;
>
> +
>
> + if (This == NULL || InfoTypesBuffer == NULL ||
>
> + InfoTypesBufferCount == NULL) {
>
> + return EFI_INVALID_PARAMETER;
>
> + }
>
> +
>
> + Guid = AllocatePool (sizeof *Guid);
>
> + if (Guid == NULL) {
>
> + return EFI_OUT_OF_RESOURCES;
>
> + }
>
> +
>
> + CopyGuid (Guid, &gEfiAdapterInfoMediaStateGuid);
>
> +
>
> + *InfoTypesBuffer = Guid;
>
> + *InfoTypesBufferCount = 1;
>
> +
>
> + return EFI_SUCCESS;
>
> +}
>
> +
>
> +CONST EFI_ADAPTER_INFORMATION_PROTOCOL
> gGenetAdapterInfoTemplate = {
>
> + GenetAipGetInformation,
>
> + GenetAipSetInformation,
>
> + GenetAipGetSupportedTypes,
>
> +};
>
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
> b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
> index 7f93c68cd608..f9aa006dc799 100644
> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
> @@ -163,6 +163,7 @@ GenetDriverBindingStart (
>
>
> EfiInitializeLock (&Genet->Lock, TPL_CALLBACK);
>
> CopyMem (&Genet->Snp, &gGenetSimpleNetworkTemplate, sizeof Genet-
> >Snp);
>
> + CopyMem (&Genet->Aip, &gGenetAdapterInfoTemplate, sizeof Genet-
> >Aip);
>
>
>
> Genet->Snp.Mode = &Genet->SnpMode;
>
> Genet->SnpMode.State = EfiSimpleNetworkStopped;
>
> @@ -203,7 +204,8 @@ GenetDriverBindingStart (
> }
>
>
>
> Status = gBS->InstallMultipleProtocolInterfaces (&ControllerHandle,
>
> - &gEfiSimpleNetworkProtocolGuid, &Genet->Snp,
>
> + &gEfiSimpleNetworkProtocolGuid, &Genet->Snp,
>
> + &gEfiAdapterInformationProtocolGuid, &Genet->Aip,
>
> NULL);
>
>
>
> if (EFI_ERROR (Status)) {
>
> @@ -273,10 +275,10 @@ GenetDriverBindingStop (
>
>
> ASSERT (Genet->ControllerHandle == ControllerHandle);
>
>
>
> - Status = gBS->UninstallProtocolInterface (ControllerHandle,
>
> - &gEfiSimpleNetworkProtocolGuid,
>
> - &Genet->Snp
>
> - );
>
> + Status = gBS->UninstallMultipleProtocolInterfaces (ControllerHandle,
>
> + &gEfiSimpleNetworkProtocolGuid, &Genet->Snp,
>
> + &gEfiAdapterInformationProtocolGuid, &Genet->Aip,
>
> + NULL);
>
> if (EFI_ERROR (Status)) {
>
> return Status;
>
> }
>
> --
> 2.27.0
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH edk2-platforms v2] Silicon/Broadcom/BcmGenetDxe: implement media state adapter info protocol
2020-06-17 18:22 ` Samer El-Haj-Mahmoud
@ 2020-06-18 10:34 ` Ard Biesheuvel
0 siblings, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2020-06-18 10:34 UTC (permalink / raw)
To: Samer El-Haj-Mahmoud, devel@edk2.groups.io
Cc: Pete Batard, Andrei Warkentin (awarkentin@vmware.com)
On 6/17/20 8:22 PM, Samer El-Haj-Mahmoud wrote:
> Tested on RPi4 w/ 4GB
>
> Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
>
Thanks
Pushed as 743f501b66db712d486347ddca781ffc705c1766
>> -----Original Message-----
>> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Sent: Wednesday, June 17, 2020 2:15 PM
>> To: devel@edk2.groups.io
>> Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Pete Batard
>> <pete@akeo.ie>; Andrei Warkentin (awarkentin@vmware.com)
>> <awarkentin@vmware.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
>> Mahmoud@arm.com>
>> Subject: [PATCH edk2-platforms v2] Silicon/Broadcom/BcmGenetDxe:
>> implement media state adapter info protocol
>>
>> NetLibDetectMedia () in DxeNetLib is used as a fallback on implementations
>> of the SNP protocol that do not also carry an implementation of the adapter
>> info protocol to provide media state information, and it does all kinds of
>> terrible things to the network interface (stopping and restarting multiple
>> times, reprogramming the multicast filters etc etc) to workaround some
>> alleged UNDI shortcoming.
>>
>> Although our GENET code should be bullet proof and therefore able to take
>> this kind of abuse, it is better to avoid it, and provide an implementation
>> of the adapter info protocol that returns the media state directly, without
>> the need to mistreat the SNP layer.
>>
>>
>> Cc: Pete Batard <pete@akeo.ie>
>>
>> Cc: Andrei Warkentin (awarkentin@vmware.com)
>> <awarkentin@vmware.com>
>>
>> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> ---
>>
>> v2:
>>
>> - use the correct accessor to retrieve the GENET_PRIVATE_DATA pointer
>>
>> - call into the PHY routines to get the state, to ensure it is not stale
>>
>>
>> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf | 3 +
>> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h | 5 +
>> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c | 106
>> ++++++++++++++++++++
>> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c | 12 ++-
>> 4 files changed, 121 insertions(+), 5 deletions(-)
>>
>> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
>> b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
>> index 28f3e66ebaf0..89dee9f10c83 100644
>> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
>> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
>> @@ -19,6 +19,7 @@ [Defines]
>> UNLOAD_IMAGE = GenetUnload
>>
>>
>>
>> [Sources]
>>
>> + AdapterInfo.c
>>
>> BcmGenetDxe.h
>>
>> ComponentName.c
>>
>> DriverBinding.c
>>
>> @@ -49,10 +50,12 @@ [LibraryClasses]
>>
>>
>> [Protocols]
>>
>> gBcmGenetPlatformDeviceProtocolGuid ## TO_START
>>
>> + gEfiAdapterInformationProtocolGuid ## BY_START
>>
>> gEfiDevicePathProtocolGuid ## BY_START
>>
>> gEfiSimpleNetworkProtocolGuid ## BY_START
>>
>>
>>
>> [Guids]
>>
>> + gEfiAdapterInfoMediaStateGuid
>>
>> gEfiEventExitBootServicesGuid
>>
>>
>>
>> [FixedPcd]
>>
>> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
>> b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
>> index 0af9d5209cf2..1a117b25f87f 100644
>> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
>> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
>> @@ -14,6 +14,7 @@
>> #include <Uefi.h>
>>
>> #include <Library/UefiLib.h>
>>
>> #include <Protocol/BcmGenetPlatformDevice.h>
>>
>> +#include <Protocol/AdapterInformation.h>
>>
>> #include <Protocol/ComponentName.h>
>>
>> #include <Protocol/ComponentName2.h>
>>
>> #include <Protocol/SimpleNetwork.h>
>>
>> @@ -209,6 +210,8 @@ typedef struct {
>> EFI_SIMPLE_NETWORK_PROTOCOL Snp;
>>
>> EFI_SIMPLE_NETWORK_MODE SnpMode;
>>
>>
>>
>> + EFI_ADAPTER_INFORMATION_PROTOCOL Aip;
>>
>> +
>>
>> BCM_GENET_PLATFORM_DEVICE_PROTOCOL *Dev;
>>
>>
>>
>> GENERIC_PHY_PRIVATE_DATA Phy;
>>
>> @@ -234,9 +237,11 @@ extern EFI_COMPONENT_NAME_PROTOCOL
>> gGenetComponentName;
>> extern EFI_COMPONENT_NAME2_PROTOCOL
>> gGenetComponentName2;
>>
>>
>>
>> extern CONST EFI_SIMPLE_NETWORK_PROTOCOL
>> gGenetSimpleNetworkTemplate;
>>
>> +extern CONST EFI_ADAPTER_INFORMATION_PROTOCOL
>> gGenetAdapterInfoTemplate;
>>
>>
>>
>> #define GENET_DRIVER_SIGNATURE SIGNATURE_32('G', 'N', 'E', 'T')
>>
>> #define GENET_PRIVATE_DATA_FROM_SNP_THIS(a) CR(a,
>> GENET_PRIVATE_DATA, Snp, GENET_DRIVER_SIGNATURE)
>>
>> +#define GENET_PRIVATE_DATA_FROM_AIP_THIS(a) CR(a,
>> GENET_PRIVATE_DATA, Aip, GENET_DRIVER_SIGNATURE)
>>
>>
>>
>> #define GENET_RX_BUFFER(g, idx) ((UINT8 *)(UINTN)(g)->RxBuffer +
>> GENET_MAX_PACKET_SIZE * (idx))
>>
>>
>>
>> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
>> b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
>> new file mode 100644
>> index 000000000000..4b42b2bba67a
>> --- /dev/null
>> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
>> @@ -0,0 +1,106 @@
>> +/** @file
>>
>> +
>>
>> + Copyright (c) 2020 Arm, Limited. All rights reserved.
>>
>> +
>>
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>>
>> +
>>
>> +**/
>>
>> +
>>
>> +#include <Uefi.h>
>>
>> +#include <Library/BaseMemoryLib.h>
>>
>> +#include <Library/DebugLib.h>
>>
>> +#include <Library/MemoryAllocationLib.h>
>>
>> +
>>
>> +#include "BcmGenetDxe.h"
>>
>> +
>>
>> +STATIC
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +GenetAipGetInformation (
>>
>> + IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
>>
>> + IN EFI_GUID *InformationType,
>>
>> + OUT VOID **InformationBlock,
>>
>> + OUT UINTN *InformationBlockSize
>>
>> + )
>>
>> +{
>>
>> + EFI_ADAPTER_INFO_MEDIA_STATE *AdapterInfo;
>>
>> + GENET_PRIVATE_DATA *Genet;
>>
>> +
>>
>> + if (This == NULL || InformationBlock == NULL ||
>>
>> + InformationBlockSize == NULL) {
>>
>> + return EFI_INVALID_PARAMETER;
>>
>> + }
>>
>> +
>>
>> + if (!CompareGuid (InformationType, &gEfiAdapterInfoMediaStateGuid)) {
>>
>> + return EFI_UNSUPPORTED;
>>
>> + }
>>
>> +
>>
>> + AdapterInfo = AllocateZeroPool (sizeof
>> (EFI_ADAPTER_INFO_MEDIA_STATE));
>>
>> + if (AdapterInfo == NULL) {
>>
>> + return EFI_OUT_OF_RESOURCES;
>>
>> + }
>>
>> +
>>
>> + *InformationBlock = AdapterInfo;
>>
>> + *InformationBlockSize = sizeof (EFI_ADAPTER_INFO_MEDIA_STATE);
>>
>> +
>>
>> + Genet = GENET_PRIVATE_DATA_FROM_AIP_THIS (This);
>>
>> + AdapterInfo->MediaState = GenericPhyUpdateConfig (&Genet->Phy);
>>
>> +
>>
>> + return EFI_SUCCESS;
>>
>> +}
>>
>> +
>>
>> +STATIC
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +GenetAipSetInformation (
>>
>> + IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
>>
>> + IN EFI_GUID *InformationType,
>>
>> + IN VOID *InformationBlock,
>>
>> + IN UINTN InformationBlockSize
>>
>> + )
>>
>> +{
>>
>> + if (This == NULL || InformationBlock == NULL) {
>>
>> + return EFI_INVALID_PARAMETER;
>>
>> + }
>>
>> +
>>
>> + if (CompareGuid (InformationType, &gEfiAdapterInfoMediaStateGuid)) {
>>
>> + return EFI_WRITE_PROTECTED;
>>
>> + }
>>
>> +
>>
>> + return EFI_UNSUPPORTED;
>>
>> +}
>>
>> +
>>
>> +STATIC
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +GenetAipGetSupportedTypes (
>>
>> + IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
>>
>> + OUT EFI_GUID **InfoTypesBuffer,
>>
>> + OUT UINTN *InfoTypesBufferCount
>>
>> + )
>>
>> +{
>>
>> + EFI_GUID *Guid;
>>
>> +
>>
>> + if (This == NULL || InfoTypesBuffer == NULL ||
>>
>> + InfoTypesBufferCount == NULL) {
>>
>> + return EFI_INVALID_PARAMETER;
>>
>> + }
>>
>> +
>>
>> + Guid = AllocatePool (sizeof *Guid);
>>
>> + if (Guid == NULL) {
>>
>> + return EFI_OUT_OF_RESOURCES;
>>
>> + }
>>
>> +
>>
>> + CopyGuid (Guid, &gEfiAdapterInfoMediaStateGuid);
>>
>> +
>>
>> + *InfoTypesBuffer = Guid;
>>
>> + *InfoTypesBufferCount = 1;
>>
>> +
>>
>> + return EFI_SUCCESS;
>>
>> +}
>>
>> +
>>
>> +CONST EFI_ADAPTER_INFORMATION_PROTOCOL
>> gGenetAdapterInfoTemplate = {
>>
>> + GenetAipGetInformation,
>>
>> + GenetAipSetInformation,
>>
>> + GenetAipGetSupportedTypes,
>>
>> +};
>>
>> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
>> b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
>> index 7f93c68cd608..f9aa006dc799 100644
>> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
>> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
>> @@ -163,6 +163,7 @@ GenetDriverBindingStart (
>>
>>
>> EfiInitializeLock (&Genet->Lock, TPL_CALLBACK);
>>
>> CopyMem (&Genet->Snp, &gGenetSimpleNetworkTemplate, sizeof Genet-
>>> Snp);
>>
>> + CopyMem (&Genet->Aip, &gGenetAdapterInfoTemplate, sizeof Genet-
>>> Aip);
>>
>>
>>
>> Genet->Snp.Mode = &Genet->SnpMode;
>>
>> Genet->SnpMode.State = EfiSimpleNetworkStopped;
>>
>> @@ -203,7 +204,8 @@ GenetDriverBindingStart (
>> }
>>
>>
>>
>> Status = gBS->InstallMultipleProtocolInterfaces (&ControllerHandle,
>>
>> - &gEfiSimpleNetworkProtocolGuid, &Genet->Snp,
>>
>> + &gEfiSimpleNetworkProtocolGuid, &Genet->Snp,
>>
>> + &gEfiAdapterInformationProtocolGuid, &Genet->Aip,
>>
>> NULL);
>>
>>
>>
>> if (EFI_ERROR (Status)) {
>>
>> @@ -273,10 +275,10 @@ GenetDriverBindingStop (
>>
>>
>> ASSERT (Genet->ControllerHandle == ControllerHandle);
>>
>>
>>
>> - Status = gBS->UninstallProtocolInterface (ControllerHandle,
>>
>> - &gEfiSimpleNetworkProtocolGuid,
>>
>> - &Genet->Snp
>>
>> - );
>>
>> + Status = gBS->UninstallMultipleProtocolInterfaces (ControllerHandle,
>>
>> + &gEfiSimpleNetworkProtocolGuid, &Genet->Snp,
>>
>> + &gEfiAdapterInformationProtocolGuid, &Genet->Aip,
>>
>> + NULL);
>>
>> if (EFI_ERROR (Status)) {
>>
>> return Status;
>>
>> }
>>
>> --
>> 2.27.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-06-18 10:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-17 18:15 [PATCH edk2-platforms v2] Silicon/Broadcom/BcmGenetDxe: implement media state adapter info protocol Ard Biesheuvel
2020-06-17 18:22 ` Samer El-Haj-Mahmoud
2020-06-18 10:34 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox