[edk2-platforms PATCH 0/8] Marvell Pp2Dxe fixes

[edk2-platforms PATCH 0/8] Marvell Pp2Dxe fixes

[Marcin Wojtas]

Hi,

ES ACS v1.0 test suite (https://github.com/ARM-software/arm-systemready)
unveiled a number of issues in Pp2Dxe SNP callbacks.
This patchset fixes all of them and allow for
successful SCT tests run.

For convenience, the patches are exposed on a public branch:
https://github.com/semihalf-wojtas-marcin/edk2-platforms/commits/pp2dxe-upstream-r20220314

Any comments or remarks would be welcome.

Best regards,
Marcin

Marcin Wojtas (8):
  Marvell/Drivers: Pp2Dxe: Fix Pp2SnpShutdown
  Marvell/Drivers: Pp2Dxe: Fix Pp2SnpReceiveFilters
  Marvell/Drivers: Pp2Dxe: Fix Pp2SnpStart & Pp2SnpStop
  Marvell/Drivers: Pp2Dxe: Implement Pp2SnpIpToMac
  Marvell/Drivers: Pp2Dxe: Fix Pp2SnpGetStatus
  Marvell/Drivers: Pp2Dxe: Fix Pp2SnpTransmit
  Marvell/Drivers: Pp2Dxe: Fix Pp2SnpReset
  Marvell/Drivers: Pp2Dxe: Fix Pp2SnpReceive

 Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 333 ++++++++++++++++++--
 1 file changed, 299 insertions(+), 34 deletions(-)

-- 
2.29.0

[edk2-platforms PATCH 1/8] Marvell/Drivers: Pp2Dxe: Fix Pp2SnpShutdown

[Marcin Wojtas]

The SnpShutdown callback requires updating the instance state
and also disabling hardware interfaces. For that purpose
reuse Pp2Halt routine.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 50 +++++++++++++++-----
 1 file changed, 38 insertions(+), 12 deletions(-)

diff --git a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
index 0872f17889..30e091f807 100644
--- a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
+++ b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
@@ -633,14 +633,12 @@ Pp2SnpReset (
   return EFI_SUCCESS;
 }
 
+STATIC
 VOID
-EFIAPI
 Pp2DxeHalt (
-  IN EFI_EVENT Event,
-  IN VOID *Context
+  IN PP2DXE_CONTEXT *Pp2Context
   )
 {
-  PP2DXE_CONTEXT *Pp2Context = Context;
   PP2DXE_PORT *Port = &Pp2Context->Port;
   MVPP2_SHARED *Mvpp2Shared = Pp2Context->Port.Priv;
   INTN Index;
@@ -661,30 +659,58 @@ Pp2DxeHalt (
   MvGop110PortDisable(Port);
 }
 
+VOID
+EFIAPI
+Pp2DxeHaltEvent (
+  IN EFI_EVENT Event,
+  IN VOID *Context
+  )
+{
+  PP2DXE_CONTEXT *Pp2Context = Context;
+
+  Pp2DxeHalt (Pp2Context);
+}
+
 EFI_STATUS
 EFIAPI
 Pp2SnpShutdown (
   IN EFI_SIMPLE_NETWORK_PROTOCOL *This
   )
 {
+  PP2DXE_CONTEXT *Pp2Context;
   EFI_TPL SavedTpl;
+
+  /* Check Snp Instance. */
+  if (This == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   SavedTpl = gBS->RaiseTPL (TPL_CALLBACK);
-  PP2DXE_CONTEXT *Pp2Context = INSTANCE_FROM_SNP(This);
-  UINT32 State = This->Mode->State;
 
-  if (State != EfiSimpleNetworkInitialized) {
-    switch (State) {
+  Pp2Context = INSTANCE_FROM_SNP (This);
+
+  /* Check whether the driver was started and initialized. */
+  if (This->Mode->State != EfiSimpleNetworkInitialized) {
+    switch (This->Mode->State) {
     case EfiSimpleNetworkStopped:
-      DEBUG((DEBUG_WARN, "Pp2Dxe%d: not started\n", Pp2Context->Instance));
+      DEBUG ((DEBUG_WARN, "Pp2Dxe%d: not started\n", Pp2Context->Instance));
       ReturnUnlock (SavedTpl, EFI_NOT_STARTED);
     case EfiSimpleNetworkStarted:
-    /* Fall through */
+      DEBUG ((DEBUG_WARN, "Pp2Dxe%d: not initialized\n", Pp2Context->Instance));
+      ReturnUnlock (SavedTpl, EFI_DEVICE_ERROR);
     default:
-      DEBUG((DEBUG_ERROR, "Pp2Dxe%d: wrong state\n", Pp2Context->Instance));
+      DEBUG ((DEBUG_WARN,
+        "Pp2Dxe%d: wrong state: %u\n",
+        Pp2Context->Instance,
+        This->Mode->State));
       ReturnUnlock (SavedTpl, EFI_DEVICE_ERROR);
     }
   }
 
+  Pp2DxeHalt (Pp2Context);
+
+  This->Mode->State = EfiSimpleNetworkStarted;
+
   ReturnUnlock (SavedTpl, EFI_SUCCESS);
 }
 
@@ -1458,7 +1484,7 @@ Pp2DxeInitialiseController (
     Status = gBS->CreateEvent (
                  EVT_SIGNAL_EXIT_BOOT_SERVICES,
                  TPL_NOTIFY,
-                 Pp2DxeHalt,
+                 Pp2DxeHaltEvent,
                  Pp2Context,
                  &Pp2Context->EfiExitBootServicesEvent
                );
-- 
2.29.0

[edk2-platforms PATCH 2/8] Marvell/Drivers: Pp2Dxe: Fix Pp2SnpReceiveFilters

[Marcin Wojtas]

Until now the Pp2SnpReceiveFilters callback unconditionally
returned EFI_SUCCESS. Allow proper handling of the input
values on the protocol level. Keep the HW registers intact.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 62 +++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
index 30e091f807..3e09fafc4c 100644
--- a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
+++ b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
@@ -725,7 +725,67 @@ Pp2SnpReceiveFilters (
   IN EFI_MAC_ADDRESS             *MCastFilter OPTIONAL
   )
 {
-  return EFI_SUCCESS;
+  PP2DXE_CONTEXT *Pp2Context;
+  EFI_TPL SavedTpl;
+  UINTN Count;
+
+  /* Check Snp Instance. */
+  if (This == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  SavedTpl = gBS->RaiseTPL (TPL_CALLBACK);
+
+  Pp2Context = INSTANCE_FROM_SNP(This);
+
+  /* Check whether the driver was started and initialized. */
+  if (This->Mode->State != EfiSimpleNetworkInitialized) {
+    switch (This->Mode->State) {
+    case EfiSimpleNetworkStopped:
+      DEBUG ((DEBUG_WARN, "Pp2Dxe%d: not started\n", Pp2Context->Instance));
+      ReturnUnlock (SavedTpl, EFI_NOT_STARTED);
+    case EfiSimpleNetworkStarted:
+      DEBUG ((DEBUG_WARN, "Pp2Dxe%d: not initialized\n", Pp2Context->Instance));
+      ReturnUnlock (SavedTpl, EFI_DEVICE_ERROR);
+    default:
+      DEBUG ((DEBUG_WARN,
+        "Pp2Dxe%d: wrong state: %u\n",
+        Pp2Context->Instance,
+        This->Mode->State));
+      ReturnUnlock (SavedTpl, EFI_DEVICE_ERROR);
+    }
+  }
+
+  if (((Enable | Disable) & ~This->Mode->ReceiveFilterMask) != 0) {
+    ReturnUnlock (SavedTpl, EFI_INVALID_PARAMETER);
+  }
+
+  if (!ResetMCastFilter &&
+      (Disable & EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST) == 0 &&
+      (Enable & EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST) != 0) {
+    if (MCastFilterCnt == 0 ||
+        MCastFilterCnt > This->Mode->MaxMCastFilterCount ||
+        MCastFilter == NULL) {
+      ReturnUnlock (SavedTpl, EFI_INVALID_PARAMETER);
+    }
+
+    for (Count = 0; Count < MCastFilterCnt; Count++) {
+      if ((MCastFilter[Count].Addr[0] & 1) == 0) {
+        ReturnUnlock (SavedTpl, EFI_INVALID_PARAMETER);
+      }
+      CopyMem (&This->Mode->MCastFilter[Count],
+        &MCastFilter[Count],
+        sizeof (EFI_MAC_ADDRESS));
+    }
+    This->Mode->MCastFilterCount = MCastFilterCnt;
+  } else if (ResetMCastFilter) {
+    This->Mode->MCastFilterCount = 0;
+  }
+
+  This->Mode->ReceiveFilterSetting |= Enable;
+  This->Mode->ReceiveFilterSetting &= ~Disable;
+
+  ReturnUnlock (SavedTpl, EFI_SUCCESS);
 }
 
 EFI_STATUS
-- 
2.29.0

[edk2-platforms PATCH 3/8] Marvell/Drivers: Pp2Dxe: Fix Pp2SnpStart & Pp2SnpStop

[Marcin Wojtas]

Add sanity check if the SNP instance pointer is not NULL
in SnpStart/Stop callbacks.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 23 +++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
index 3e09fafc4c..5f487c4dc6 100644
--- a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
+++ b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
@@ -575,11 +575,19 @@ Pp2SnpStart (
   )
 {
   PP2DXE_CONTEXT *Pp2Context;
-  UINT32 State = This->Mode->State;
   EFI_TPL SavedTpl;
+  UINT32 State;
+
+  /* Check Snp Instance. */
+  if (This == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
 
   SavedTpl = gBS->RaiseTPL (TPL_CALLBACK);
+
   Pp2Context = INSTANCE_FROM_SNP(This);
+  State = This->Mode->State;
 
   if (State != EfiSimpleNetworkStopped) {
     switch (State) {
@@ -604,9 +612,18 @@ Pp2SnpStop (
   )
 {
   EFI_TPL SavedTpl;
+  PP2DXE_CONTEXT *Pp2Context;
+  UINT32 State;
+
+  // Check Snp Instance
+  if (This == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   SavedTpl = gBS->RaiseTPL (TPL_CALLBACK);
-  PP2DXE_CONTEXT *Pp2Context = INSTANCE_FROM_SNP(This);
-  UINT32 State = This->Mode->State;
+
+  Pp2Context = INSTANCE_FROM_SNP(This);
+  State = This->Mode->State;
 
   if (State != EfiSimpleNetworkStarted && State != EfiSimpleNetworkInitialized) {
     switch (State) {
-- 
2.29.0

[edk2-platforms PATCH 4/8] Marvell/Drivers: Pp2Dxe: Implement Pp2SnpIpToMac

[Marcin Wojtas]

The SNP MCastIpToMac callback was unsupported in Pp2Dxe driver.
Implement it.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 81 +++++++++++++++++++-
 1 file changed, 77 insertions(+), 4 deletions(-)

diff --git a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
index 5f487c4dc6..91cd573b87 100644
--- a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
+++ b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
@@ -889,12 +889,85 @@ EFI_STATUS
 EFIAPI
 Pp2SnpIpToMac (
   IN EFI_SIMPLE_NETWORK_PROTOCOL *This,
-  IN BOOLEAN                     IPv6,
-  IN EFI_IP_ADDRESS              *IP,
-  OUT EFI_MAC_ADDRESS            *MAC
+  IN BOOLEAN                     IsIpv6,
+  IN EFI_IP_ADDRESS              *Ip,
+  OUT EFI_MAC_ADDRESS            *McastMac
   )
 {
-  return EFI_UNSUPPORTED;
+  PP2DXE_CONTEXT *Pp2Context;
+  EFI_TPL SavedTpl;
+
+  /* Check Snp Instance. */
+  if (This == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  SavedTpl = gBS->RaiseTPL (TPL_CALLBACK);
+
+  Pp2Context = INSTANCE_FROM_SNP (This);
+
+  /* Check that driver was started and initialised. */
+  if (This->Mode->State != EfiSimpleNetworkInitialized) {
+    switch (This->Mode->State) {
+    case EfiSimpleNetworkStopped:
+      DEBUG ((DEBUG_WARN, "Pp2Dxe%d: not started\n", Pp2Context->Instance));
+      ReturnUnlock (SavedTpl, EFI_NOT_STARTED);
+    case EfiSimpleNetworkStarted:
+      DEBUG ((DEBUG_WARN, "Pp2Dxe%d: not initialized\n", Pp2Context->Instance));
+      ReturnUnlock (SavedTpl, EFI_DEVICE_ERROR);
+    default:
+      DEBUG ((DEBUG_WARN,
+        "Pp2Dxe%d: wrong state: %u\n",
+        Pp2Context->Instance,
+        This->Mode->State));
+      ReturnUnlock (SavedTpl, EFI_DEVICE_ERROR);
+    }
+  }
+
+  /* Check parameters. */
+  if ((McastMac == NULL) || (Ip == NULL)) {
+    ReturnUnlock (SavedTpl, EFI_INVALID_PARAMETER);
+  }
+
+  /* Make sure MAC address is empty. */
+  ZeroMem (McastMac, sizeof(EFI_MAC_ADDRESS));
+
+  /* If we need ipv4 address. */
+  if (!IsIpv6) {
+    /*
+     * Most significant 25 bits of a multicast HW address are set.
+     * 01-00-5E is the IPv4 Ethernet Multicast Address (see RFC 1112).
+     */
+    McastMac->Addr[0] = 0x01;
+    McastMac->Addr[1] = 0x00;
+    McastMac->Addr[2] = 0x5E;
+
+    /*
+     * Lower 23 bits from ipv4 address
+     * Clear the most significant bit (25th bit of MAC must be 0).
+     */
+    McastMac->Addr[3] = Ip->v4.Addr[1] & 0x7F;
+    McastMac->Addr[4] = Ip->v4.Addr[2];
+    McastMac->Addr[5] = Ip->v4.Addr[3];
+  } else {
+    /*
+     * Most significant 16 bits of multicast v6 HW address are set
+     * 33-33 is the IPv6 Ethernet Multicast Address (see RFC 2464).
+     */
+    McastMac->Addr[0] = 0x33;
+    McastMac->Addr[1] = 0x33;
+
+    /* Lower four octets are taken from ipv6 address. */
+    McastMac->Addr[2] = Ip->v6.Addr[8];
+    McastMac->Addr[3] = Ip->v6.Addr[9];
+    McastMac->Addr[4] = Ip->v6.Addr[10];
+    McastMac->Addr[5] = Ip->v6.Addr[11];
+  }
+
+  /* Restore TPL and return. */
+  gBS->RestoreTPL (SavedTpl);
+
+  return EFI_SUCCESS;
 }
 
 EFI_STATUS
-- 
2.29.0

[edk2-platforms PATCH 5/8] Marvell/Drivers: Pp2Dxe: Fix Pp2SnpGetStatus

[Marcin Wojtas]

This patch adds missing parameter's and SNP instance
status checks in SnpGetStatus callback.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 31 ++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
index 91cd573b87..8a4c4545c8 100644
--- a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
+++ b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
@@ -991,16 +991,43 @@ Pp2SnpGetStatus (
   OUT VOID                       **TxBuf OPTIONAL
   )
 {
-  PP2DXE_CONTEXT *Pp2Context = INSTANCE_FROM_SNP(Snp);
-  PP2DXE_PORT *Port = &Pp2Context->Port;
+  PP2DXE_CONTEXT *Pp2Context;
+  PP2DXE_PORT *Port;
   BOOLEAN LinkUp;
   EFI_TPL SavedTpl;
 
+  /* Check Snp Instance. */
+  if (Snp == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   SavedTpl = gBS->RaiseTPL (TPL_CALLBACK);
 
+  Pp2Context = INSTANCE_FROM_SNP (Snp);
+
+  /* Check whether the driver was started and initialized. */
+  if (Snp->Mode->State != EfiSimpleNetworkInitialized) {
+    switch (Snp->Mode->State) {
+    case EfiSimpleNetworkStopped:
+      DEBUG ((DEBUG_WARN, "Pp2Dxe%d: not started\n", Pp2Context->Instance));
+      ReturnUnlock (SavedTpl, EFI_NOT_STARTED);
+    case EfiSimpleNetworkStarted:
+      DEBUG ((DEBUG_WARN, "Pp2Dxe%d: not initialized\n", Pp2Context->Instance));
+      ReturnUnlock (SavedTpl, EFI_DEVICE_ERROR);
+    default:
+      DEBUG ((DEBUG_WARN,
+        "Pp2Dxe%d: wrong state: %u\n",
+        Pp2Context->Instance,
+        Snp->Mode->State));
+      ReturnUnlock (SavedTpl, EFI_DEVICE_ERROR);
+    }
+  }
+
   if (!Pp2Context->Initialized)
     ReturnUnlock(SavedTpl, EFI_NOT_READY);
 
+  Port = &Pp2Context->Port;
+
   LinkUp = Port->AlwaysUp ? TRUE : MvGop110PortIsLinkUp(Port);
 
   if (LinkUp != Snp->Mode->MediaPresent) {
-- 
2.29.0

[edk2-platforms PATCH 6/8] Marvell/Drivers: Pp2Dxe: Fix Pp2SnpTransmit

[Marcin Wojtas]

Error checking for invalid input parameters was too
hard. Replace ASSERT with returning error value.
Moreover set EtherType only when we are sure it
won't be dereferencing NULL pointer.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
index 8a4c4545c8..deb3f34625 100644
--- a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
+++ b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
@@ -1074,9 +1074,15 @@ Pp2SnpTransmit (
   }
 
   if (HeaderSize != 0) {
-    ASSERT (HeaderSize == This->Mode->MediaHeaderSize);
-    ASSERT (EtherTypePtr != NULL);
-    ASSERT (DestAddr != NULL);
+    if (HeaderSize != This->Mode->MediaHeaderSize ||
+        EtherTypePtr == NULL ||
+        DestAddr == NULL) {
+      return EFI_INVALID_PARAMETER;
+    }
+  }
+
+  if (BufferSize < This->Mode->MediaHeaderSize) {
+      return EFI_BUFFER_TOO_SMALL;
   }
 
   SavedTpl = gBS->RaiseTPL (TPL_CALLBACK);
@@ -1100,8 +1106,6 @@ Pp2SnpTransmit (
     ReturnUnlock(SavedTpl, EFI_NOT_READY);
   }
 
-  EtherType = HTONS (*EtherTypePtr);
-
   /* Fetch next descriptor */
   TxDesc = Mvpp2TxqNextDescGet(AggrTxq);
 
@@ -1118,6 +1122,8 @@ Pp2SnpTransmit (
     else
       CopyMem(DataPtr + NET_ETHER_ADDR_LEN, &This->Mode->CurrentAddress, NET_ETHER_ADDR_LEN);
 
+    EtherType = HTONS (*EtherTypePtr);
+
     CopyMem(DataPtr + NET_ETHER_ADDR_LEN * 2, &EtherType, 2);
   }
 
-- 
2.29.0

[edk2-platforms PATCH 7/8] Marvell/Drivers: Pp2Dxe: Fix Pp2SnpReset

[Marcin Wojtas]

This patch adds missing parameter's and SNP instance
status checks.

igned-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 27 ++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
index deb3f34625..841a1c8f84 100644
--- a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
+++ b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
@@ -647,6 +647,33 @@ Pp2SnpReset (
   IN BOOLEAN                     ExtendedVerification
   )
 {
+  PP2DXE_CONTEXT *Pp2Context;
+
+  /* Check This Instance. */
+  if (This == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Pp2Context = INSTANCE_FROM_SNP (This);
+
+  /* Check that driver was started and initialized. */
+  if (This->Mode->State != EfiSimpleNetworkInitialized) {
+    switch (This->Mode->State) {
+    case EfiSimpleNetworkStopped:
+      DEBUG ((DEBUG_WARN, "Pp2Dxe%d: not started\n", Pp2Context->Instance));
+      return EFI_NOT_STARTED;
+    case EfiSimpleNetworkStarted:
+      DEBUG ((DEBUG_WARN, "Pp2Dxe%d: not initialized\n", Pp2Context->Instance));
+      return EFI_DEVICE_ERROR;
+    default:
+      DEBUG ((DEBUG_WARN,
+        "Pp2Dxe%d: wrong state: %u\n",
+        Pp2Context->Instance,
+        This->Mode->State));
+      return EFI_DEVICE_ERROR;
+    }
+  }
+
   return EFI_SUCCESS;
 }
 
-- 
2.29.0

[edk2-platforms PATCH 8/8] Marvell/Drivers: Pp2Dxe: Fix Pp2SnpReceive

[Marcin Wojtas]

This patch adds missing parameter's and SNP instance
status checks in SnpReceive callback. Additionally,
the local variables declarations are cleaned-up.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 43 ++++++++++++++++----
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
index 841a1c8f84..5e463ac932 100644
--- a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
+++ b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
@@ -1214,23 +1214,50 @@ Pp2SnpReceive (
   )
 {
   INTN ReceivedPackets;
-  PP2DXE_CONTEXT *Pp2Context = INSTANCE_FROM_SNP(This);
-  PP2DXE_PORT *Port = &Pp2Context->Port;
-  MVPP2_SHARED *Mvpp2Shared = Pp2Context->Port.Priv;
+  PP2DXE_CONTEXT *Pp2Context;
+  PP2DXE_PORT *Port;
   UINTN PhysAddr, VirtAddr;
-  EFI_STATUS Status = EFI_SUCCESS;
+  EFI_STATUS Status;
   EFI_TPL SavedTpl;
   UINT32 StatusReg;
   INTN PoolId;
   UINTN PktLength;
   UINT8 *DataPtr;
   MVPP2_RX_DESC *RxDesc;
-  MVPP2_RX_QUEUE *Rxq = &Port->Rxqs[0];
+  MVPP2_RX_QUEUE *Rxq;
+
+  /* Check input parameters. */
+  if (This == NULL || Buffer == NULL || BufferSize == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  SavedTpl = gBS->RaiseTPL (TPL_CALLBACK);
+
+  Pp2Context = INSTANCE_FROM_SNP (This);
 
+  /* Check whether the driver was started and initialized. */
+  if (This->Mode->State != EfiSimpleNetworkInitialized) {
+    switch (This->Mode->State) {
+    case EfiSimpleNetworkStopped:
+      DEBUG ((DEBUG_WARN, "Pp2Dxe%d: not started\n", Pp2Context->Instance));
+      ReturnUnlock (SavedTpl, EFI_NOT_STARTED);
+    case EfiSimpleNetworkStarted:
+      DEBUG ((DEBUG_WARN, "Pp2Dxe%d: not initialized\n", Pp2Context->Instance));
+      ReturnUnlock (SavedTpl, EFI_DEVICE_ERROR);
+    default:
+      DEBUG ((DEBUG_WARN,
+        "Pp2Dxe%d: wrong state: %u\n",
+        Pp2Context->Instance,
+        This->Mode->State));
+      ReturnUnlock (SavedTpl, EFI_DEVICE_ERROR);
+    }
+  }
+
+  Port = &Pp2Context->Port;
   ASSERT (Port != NULL);
+  Rxq = &Port->Rxqs[0];
   ASSERT (Rxq != NULL);
 
-  SavedTpl = gBS->RaiseTPL (TPL_CALLBACK);
   ReceivedPackets = Mvpp2RxqReceived(Port, Rxq->Id);
 
   if (ReceivedPackets == 0) {
@@ -1285,10 +1312,12 @@ Pp2SnpReceive (
     *EtherType = NTOHS (*(UINT16 *)(&DataPtr[12]));
   }
 
+  Status = EFI_SUCCESS;
+
 drop:
   /* Refill: pass packet back to BM */
   PoolId = (StatusReg & MVPP2_RXD_BM_POOL_ID_MASK) >> MVPP2_RXD_BM_POOL_ID_OFFS;
-  Mvpp2BmPoolPut(Mvpp2Shared, PoolId, PhysAddr, VirtAddr);
+  Mvpp2BmPoolPut (Pp2Context->Port.Priv, PoolId, PhysAddr, VirtAddr);
 
   /* Update counters with 1 packet received and 1 packet refilled */
   Mvpp2RxqStatusUpdate(Port, Rxq->Id, 1, 1);
-- 
2.29.0

Re: [edk2-platforms PATCH 0/8] Marvell Pp2Dxe fixes

[Marcin Wojtas]

Hi,


pon., 14 mar 2022 o 16:38 Marcin Wojtas <mw@semihalf.com> napisał(a):
>
> Hi,
>
> ES ACS v1.0 test suite (https://github.com/ARM-software/arm-systemready)
> unveiled a number of issues in Pp2Dxe SNP callbacks.
> This patchset fixes all of them and allow for
> successful SCT tests run.
>
> For convenience, the patches are exposed on a public branch:
> https://github.com/semihalf-wojtas-marcin/edk2-platforms/commits/pp2dxe-upstream-r20220314
>
> Any comments or remarks would be welcome.
>
> Best regards,
> Marcin
>
> Marcin Wojtas (8):
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpShutdown
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpReceiveFilters
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpStart & Pp2SnpStop
>   Marvell/Drivers: Pp2Dxe: Implement Pp2SnpIpToMac
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpGetStatus
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpTransmit
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpReset
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpReceive
>
>  Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 333 ++++++++++++++++++--
>  1 file changed, 299 insertions(+), 34 deletions(-)
>

Do you have any feedback about the patchset?

Best regards,
Marcin

Re: [edk2-platforms PATCH 0/8] Marvell Pp2Dxe fixes

[Sunny Wang]

Sorry for the delay and thanks for fixing the issues, Marcin.

The patch series look good to me.  Also, we have tested the patches on a CN9130 based system, and it works fine. The patches fix the SCT failures below, and UEFI network function works fine.
    - PlatformSpecificElements: [FAILED]
    - Shutdown_Func: [FAILED]
    - Start_Conf: [FAILED]
    - Start_Func: [FAILE]
    - Stop_Conf: [FAILED]

Reviewed-by: Sunny Wang <sunny.wang@arm.com>

Best Regards,
Sunny
-----Original Message-----
From: Marcin Wojtas <mw@semihalf.com>
Sent: 06 April 2022 21:02
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Grzegorz Jaszczyk <jaz@semihalf.com>; Grzegorz Bernacki <gjb@semihalf.com>; upstream@semihalf.com; Sunny Wang <Sunny.Wang@arm.com>
Subject: Re: [edk2-platforms PATCH 0/8] Marvell Pp2Dxe fixes

Hi,


pon., 14 mar 2022 o 16:38 Marcin Wojtas <mw@semihalf.com> napisał(a):
>
> Hi,
>
> ES ACS v1.0 test suite (https://github.com/ARM-software/arm-systemready)
> unveiled a number of issues in Pp2Dxe SNP callbacks.
> This patchset fixes all of them and allow for
> successful SCT tests run.
>
> For convenience, the patches are exposed on a public branch:
> https://github.com/semihalf-wojtas-marcin/edk2-platforms/commits/pp2dxe-upstream-r20220314
>
> Any comments or remarks would be welcome.
>
> Best regards,
> Marcin
>
> Marcin Wojtas (8):
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpShutdown
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpReceiveFilters
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpStart & Pp2SnpStop
>   Marvell/Drivers: Pp2Dxe: Implement Pp2SnpIpToMac
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpGetStatus
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpTransmit
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpReset
>   Marvell/Drivers: Pp2Dxe: Fix Pp2SnpReceive
>
>  Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 333 ++++++++++++++++++--
>  1 file changed, 299 insertions(+), 34 deletions(-)
>

Do you have any feedback about the patchset?

Best regards,
Marcin
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.

Re: [edk2-platforms PATCH 0/8] Marvell Pp2Dxe fixes

[Ard Biesheuvel]

On Thu, 7 Apr 2022 at 12:52, Sunny Wang <Sunny.Wang@arm.com> wrote:
>
> Sorry for the delay and thanks for fixing the issues, Marcin.
>
> The patch series look good to me.  Also, we have tested the patches on a CN9130 based system, and it works fine. The patches fix the SCT failures below, and UEFI network function works fine.
>     - PlatformSpecificElements: [FAILED]
>     - Shutdown_Func: [FAILED]
>     - Start_Conf: [FAILED]
>     - Start_Func: [FAILE]
>     - Stop_Conf: [FAILED]
>
> Reviewed-by: Sunny Wang <sunny.wang@arm.com>
>

Pushed as 170f455d1b1b..fe223fb30f74

Thanks all,