[PATCH] efinet: check for broken firmware

[PATCH] efinet: check for broken firmware

[Josef Bacik]

The firmware bug I've been tracking down has been too extensive to work around
effectively.  Basically once we switch to EXCLUSIVE everything is completely
horked.  I can add a multicast filter but it's timing sensitive, so it has to be
done at least 10 seconds after initialization for it to take place, and we have
to continue to enable PROMISCUOUS_MULTICAST because otherwise we no longer get
unicast traffic.  I discovered however that if we do not make the EXCLUSIVE
switch over everything works fine.  So instead add a function that checks for
this broken firmware and uses GET_PROTOCOL instead of EXCLUSIVE.  This also
keeps the original protocol we use when scanning the cards and leaves the
initialization stuff for ->open.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 grub-core/net/drivers/efi/efinet.c | 99 ++++++++++++++++++--------------------
 1 file changed, 46 insertions(+), 53 deletions(-)

diff --git a/grub-core/net/drivers/efi/efinet.c b/grub-core/net/drivers/efi/efinet.c
index c8f80a1..5a207fd 100644
--- a/grub-core/net/drivers/efi/efinet.c
+++ b/grub-core/net/drivers/efi/efinet.c
@@ -156,59 +156,40 @@ get_card_packet (struct grub_net_card *dev)
 static grub_err_t
 open_card (struct grub_net_card *dev)
 {
-  grub_efi_simple_network_t *net;
+  grub_efi_simple_network_t *net = dev->efi_net;
 
-  /* Try to reopen SNP exlusively to close any active MNP protocol instance
-     that may compete for packet polling
-   */
-  net = grub_efi_open_protocol (dev->efi_handle, &net_io_guid,
-				GRUB_EFI_OPEN_PROTOCOL_BY_EXCLUSIVE);
-  if (net)
+  if (net->mode->state == GRUB_EFI_NETWORK_STOPPED)
+    return grub_error (GRUB_ERR_NET_NO_CARD, "%s: card stopped",
+		       dev->name);
+
+  if (net->mode->state == GRUB_EFI_NETWORK_STARTED
+      && efi_call_3 (net->initialize, net, 0, 0) != GRUB_EFI_SUCCESS)
+	  return grub_error (GRUB_ERR_NET_NO_CARD, "%s: net initialize failed",
+			     dev->name);
+
+  /* Enable hardware receive filters if driver declares support for it.
+     We need unicast and broadcast and additionaly all nodes and
+     solicited multicast for IPv6. Solicited multicast is per-IPv6
+     address and we currently do not have API to do it so simply
+     try to enable receive of all multicast packets or evertyhing in
+     the worst case (i386 PXE driver always enables promiscuous too).
+
+     This does trust firmware to do what it claims to do.
+    */
+  if (net->mode->receive_filter_mask)
     {
-      if (net->mode->state == GRUB_EFI_NETWORK_STOPPED
-	  && efi_call_1 (net->start, net) != GRUB_EFI_SUCCESS)
-	return grub_error (GRUB_ERR_NET_NO_CARD, "%s: net start failed",
-			   dev->name);
-
-      if (net->mode->state == GRUB_EFI_NETWORK_STOPPED)
-	return grub_error (GRUB_ERR_NET_NO_CARD, "%s: card stopped",
-			   dev->name);
-
-      if (net->mode->state == GRUB_EFI_NETWORK_STARTED
-	  && efi_call_3 (net->initialize, net, 0, 0) != GRUB_EFI_SUCCESS)
-	return grub_error (GRUB_ERR_NET_NO_CARD, "%s: net initialize failed",
-			   dev->name);
-
-      /* Enable hardware receive filters if driver declares support for it.
-	 We need unicast and broadcast and additionaly all nodes and
-	 solicited multicast for IPv6. Solicited multicast is per-IPv6
-	 address and we currently do not have API to do it so simply
-	 try to enable receive of all multicast packets or evertyhing in
-	 the worst case (i386 PXE driver always enables promiscuous too).
-
-	 This does trust firmware to do what it claims to do.
-       */
-      if (net->mode->receive_filter_mask)
-	{
-	  grub_uint32_t filters = GRUB_EFI_SIMPLE_NETWORK_RECEIVE_UNICAST   |
-				  GRUB_EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST |
-				  GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST;
-
-	  filters &= net->mode->receive_filter_mask;
-	  if (!(filters & GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST))
-	    filters |= (net->mode->receive_filter_mask &
-			GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS);
+      grub_uint32_t filters = GRUB_EFI_SIMPLE_NETWORK_RECEIVE_UNICAST   |
+	      GRUB_EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST |
+	      GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST;
 
-	  efi_call_6 (net->receive_filters, net, filters, 0, 0, 0, NULL);
-	}
+      filters &= net->mode->receive_filter_mask;
+      if (!(filters & GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST))
+	filters |= (net->mode->receive_filter_mask &
+		    GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS);
 
-      efi_call_4 (grub_efi_system_table->boot_services->close_protocol,
-		  dev->efi_net, &net_io_guid,
-		  grub_efi_image_handle, dev->efi_handle);
-      dev->efi_net = net;
+      efi_call_6 (net->receive_filters, net, filters, 0, 0, 0, NULL);
     }
 
-  /* If it failed we just try to run as best as we can */
   return GRUB_ERR_NONE;
 }
 
@@ -278,12 +259,26 @@ grub_efinet_is_mac_device (struct grub_net_card *card)
   return 0;
 }
 
+static grub_efi_uint32_t
+grub_snp_attributes (void)
+{
+  /* Some firmware will completely screw up the transition to exclusive SNP,
+     doing things like not honoring receive filters or taking multiple seconds
+     to make the switch over.  So don't bother using exclusive in this case.
+   */
+  if (!grub_memcmp(grub_efi_system_table->firmware_vendor, "A", 1) &&
+      grub_efi_system_table->firmware_revision == (grub_efi_uint32_t)262798)
+    return GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL;
+  return GRUB_EFI_OPEN_PROTOCOL_BY_EXCLUSIVE;
+}
+
 static void
 grub_efinet_findcards (void)
 {
   grub_efi_uintn_t num_handles;
   grub_efi_handle_t *handles;
   grub_efi_handle_t *handle;
+  grub_efi_uint32_t attributes;
   int i = 0;
 
   /* Find handles which support the disk io interface.  */
@@ -291,6 +286,9 @@ grub_efinet_findcards (void)
 				    0, &num_handles);
   if (! handles)
     return;
+
+  attributes = grub_snp_attributes();
+
   for (handle = handles; num_handles--; handle++)
     {
       grub_efi_simple_network_t *net;
@@ -319,8 +317,7 @@ grub_efinet_findcards (void)
 	  && GRUB_EFI_DEVICE_PATH_SUBTYPE (parent) == GRUB_EFI_MAC_ADDRESS_DEVICE_PATH_SUBTYPE)
 	continue;
 
-      net = grub_efi_open_protocol (*handle, &net_io_guid,
-				    GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+      net = grub_efi_open_protocol (*handle, &net_io_guid, attributes);
       if (! net)
 	/* This should not happen... Why?  */
 	continue;
@@ -332,10 +329,6 @@ grub_efinet_findcards (void)
       if (net->mode->state == GRUB_EFI_NETWORK_STOPPED)
 	continue;
 
-      if (net->mode->state == GRUB_EFI_NETWORK_STARTED
-	  && efi_call_3 (net->initialize, net, 0, 0) != GRUB_EFI_SUCCESS)
-	continue;
-
       card = grub_zalloc (sizeof (struct grub_net_card));
       if (!card)
 	{
-- 
1.8.1

Re: [PATCH] efinet: check for broken firmware

[Andrei Borzenkov]

On Fri, Nov 13, 2015 at 1:07 AM, Josef Bacik <jbacik@fb.com> wrote:
> The firmware bug I've been tracking down has been too extensive to work around
> effectively.  Basically once we switch to EXCLUSIVE everything is completely
> horked.  I can add a multicast filter but it's timing sensitive, so it has to be
> done at least 10 seconds after initialization for it to take place, and we have
> to continue to enable PROMISCUOUS_MULTICAST because otherwise we no longer get
> unicast traffic.  I discovered however that if we do not make the EXCLUSIVE
> switch over everything works fine.  So instead add a function that checks for

Does your firmware use MNP at all?

> this broken firmware and uses GET_PROTOCOL instead of EXCLUSIVE.  This also
> keeps the original protocol we use when scanning the cards and leaves the
> initialization stuff for ->open.  Thanks,
>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
> ---
>  grub-core/net/drivers/efi/efinet.c | 99 ++++++++++++++++++--------------------
>  1 file changed, 46 insertions(+), 53 deletions(-)
>
> diff --git a/grub-core/net/drivers/efi/efinet.c b/grub-core/net/drivers/efi/efinet.c
> index c8f80a1..5a207fd 100644
> --- a/grub-core/net/drivers/efi/efinet.c
> +++ b/grub-core/net/drivers/efi/efinet.c
> @@ -156,59 +156,40 @@ get_card_packet (struct grub_net_card *dev)
>  static grub_err_t
>  open_card (struct grub_net_card *dev)
>  {
> -  grub_efi_simple_network_t *net;
> +  grub_efi_simple_network_t *net = dev->efi_net;
>
> -  /* Try to reopen SNP exlusively to close any active MNP protocol instance
> -     that may compete for packet polling
> -   */
> -  net = grub_efi_open_protocol (dev->efi_handle, &net_io_guid,
> -                               GRUB_EFI_OPEN_PROTOCOL_BY_EXCLUSIVE);
> -  if (net)
> +  if (net->mode->state == GRUB_EFI_NETWORK_STOPPED)
> +    return grub_error (GRUB_ERR_NET_NO_CARD, "%s: card stopped",
> +                      dev->name);
> +
> +  if (net->mode->state == GRUB_EFI_NETWORK_STARTED
> +      && efi_call_3 (net->initialize, net, 0, 0) != GRUB_EFI_SUCCESS)
> +         return grub_error (GRUB_ERR_NET_NO_CARD, "%s: net initialize failed",
> +                            dev->name);
> +
> +  /* Enable hardware receive filters if driver declares support for it.
> +     We need unicast and broadcast and additionaly all nodes and
> +     solicited multicast for IPv6. Solicited multicast is per-IPv6
> +     address and we currently do not have API to do it so simply
> +     try to enable receive of all multicast packets or evertyhing in
> +     the worst case (i386 PXE driver always enables promiscuous too).
> +
> +     This does trust firmware to do what it claims to do.
> +    */
> +  if (net->mode->receive_filter_mask)
>      {
> -      if (net->mode->state == GRUB_EFI_NETWORK_STOPPED
> -         && efi_call_1 (net->start, net) != GRUB_EFI_SUCCESS)
> -       return grub_error (GRUB_ERR_NET_NO_CARD, "%s: net start failed",
> -                          dev->name);
> -
> -      if (net->mode->state == GRUB_EFI_NETWORK_STOPPED)
> -       return grub_error (GRUB_ERR_NET_NO_CARD, "%s: card stopped",
> -                          dev->name);
> -
> -      if (net->mode->state == GRUB_EFI_NETWORK_STARTED
> -         && efi_call_3 (net->initialize, net, 0, 0) != GRUB_EFI_SUCCESS)
> -       return grub_error (GRUB_ERR_NET_NO_CARD, "%s: net initialize failed",
> -                          dev->name);
> -
> -      /* Enable hardware receive filters if driver declares support for it.
> -        We need unicast and broadcast and additionaly all nodes and
> -        solicited multicast for IPv6. Solicited multicast is per-IPv6
> -        address and we currently do not have API to do it so simply
> -        try to enable receive of all multicast packets or evertyhing in
> -        the worst case (i386 PXE driver always enables promiscuous too).
> -
> -        This does trust firmware to do what it claims to do.
> -       */
> -      if (net->mode->receive_filter_mask)
> -       {
> -         grub_uint32_t filters = GRUB_EFI_SIMPLE_NETWORK_RECEIVE_UNICAST   |
> -                                 GRUB_EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST |
> -                                 GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST;
> -
> -         filters &= net->mode->receive_filter_mask;
> -         if (!(filters & GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST))
> -           filters |= (net->mode->receive_filter_mask &
> -                       GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS);
> +      grub_uint32_t filters = GRUB_EFI_SIMPLE_NETWORK_RECEIVE_UNICAST   |
> +             GRUB_EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST |
> +             GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST;
>
> -         efi_call_6 (net->receive_filters, net, filters, 0, 0, 0, NULL);
> -       }
> +      filters &= net->mode->receive_filter_mask;
> +      if (!(filters & GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST))
> +       filters |= (net->mode->receive_filter_mask &
> +                   GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS);
>
> -      efi_call_4 (grub_efi_system_table->boot_services->close_protocol,
> -                 dev->efi_net, &net_io_guid,
> -                 grub_efi_image_handle, dev->efi_handle);
> -      dev->efi_net = net;
> +      efi_call_6 (net->receive_filters, net, filters, 0, 0, 0, NULL);
>      }
>
> -  /* If it failed we just try to run as best as we can */
>    return GRUB_ERR_NONE;
>  }
>
> @@ -278,12 +259,26 @@ grub_efinet_is_mac_device (struct grub_net_card *card)
>    return 0;
>  }
>
> +static grub_efi_uint32_t
> +grub_snp_attributes (void)

Attributes? I'd say

use_snp ? GRUB_EFI_OPEN_PROTOCOL_BY_EXCLUSIVE :
GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL

looks more readable.

> +{
> +  /* Some firmware will completely screw up the transition to exclusive SNP,
> +     doing things like not honoring receive filters or taking multiple seconds
> +     to make the switch over.  So don't bother using exclusive in this case.
> +   */
> +  if (!grub_memcmp(grub_efi_system_table->firmware_vendor, "A", 1) &&
> +      grub_efi_system_table->firmware_revision == (grub_efi_uint32_t)262798)

Well, I'm not thrilled by this check ... at least we need to compare
full firmware_vendor then.

> +    return GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL;
> +  return GRUB_EFI_OPEN_PROTOCOL_BY_EXCLUSIVE;
> +}
> +
>  static void
>  grub_efinet_findcards (void)
>  {
>    grub_efi_uintn_t num_handles;
>    grub_efi_handle_t *handles;
>    grub_efi_handle_t *handle;
> +  grub_efi_uint32_t attributes;
>    int i = 0;
>
>    /* Find handles which support the disk io interface.  */
> @@ -291,6 +286,9 @@ grub_efinet_findcards (void)
>                                     0, &num_handles);
>    if (! handles)
>      return;
> +
> +  attributes = grub_snp_attributes();
> +
>    for (handle = handles; num_handles--; handle++)
>      {
>        grub_efi_simple_network_t *net;
> @@ -319,8 +317,7 @@ grub_efinet_findcards (void)
>           && GRUB_EFI_DEVICE_PATH_SUBTYPE (parent) == GRUB_EFI_MAC_ADDRESS_DEVICE_PATH_SUBTYPE)
>         continue;
>
> -      net = grub_efi_open_protocol (*handle, &net_io_guid,
> -                                   GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> +      net = grub_efi_open_protocol (*handle, &net_io_guid, attributes);

No, we cannot open exclusively here, it will destroy autocnfiguration
information we need later. You need to add conditional in open_card.

>        if (! net)
>         /* This should not happen... Why?  */
>         continue;
> @@ -332,10 +329,6 @@ grub_efinet_findcards (void)
>        if (net->mode->state == GRUB_EFI_NETWORK_STOPPED)
>         continue;
>
> -      if (net->mode->state == GRUB_EFI_NETWORK_STARTED
> -         && efi_call_3 (net->initialize, net, 0, 0) != GRUB_EFI_SUCCESS)
> -       continue;
> -
>        card = grub_zalloc (sizeof (struct grub_net_card));
>        if (!card)
>         {
> --
> 1.8.1
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] efinet: check for broken firmware

[Josef Bacik]

On 11/13/2015 08:10 AM, Andrei Borzenkov wrote:
> On Fri, Nov 13, 2015 at 1:07 AM, Josef Bacik <jbacik@fb.com> wrote:
>> The firmware bug I've been tracking down has been too extensive to work around
>> effectively.  Basically once we switch to EXCLUSIVE everything is completely
>> horked.  I can add a multicast filter but it's timing sensitive, so it has to be
>> done at least 10 seconds after initialization for it to take place, and we have
>> to continue to enable PROMISCUOUS_MULTICAST because otherwise we no longer get
>> unicast traffic.  I discovered however that if we do not make the EXCLUSIVE
>> switch over everything works fine.  So instead add a function that checks for
>
> Does your firmware use MNP at all?
>

I was going to look into that today, with as much problems as SNP has 
been giving us I'm wondering if MNP works better.  I'm going to rig up a 
barebones MNP driver and see if it behaves better.  The question is how 
to make us use MNP vs. SNP when it is supported, or to force us to go 
back to SNP if MNP is fuuuuucked up.

>> this broken firmware and uses GET_PROTOCOL instead of EXCLUSIVE.  This also
>> keeps the original protocol we use when scanning the cards and leaves the
>> initialization stuff for ->open.  Thanks,
>>
>> Signed-off-by: Josef Bacik <jbacik@fb.com>
>> ---
>>   grub-core/net/drivers/efi/efinet.c | 99 ++++++++++++++++++--------------------
>>   1 file changed, 46 insertions(+), 53 deletions(-)
>>
>> diff --git a/grub-core/net/drivers/efi/efinet.c b/grub-core/net/drivers/efi/efinet.c
>> index c8f80a1..5a207fd 100644
>> --- a/grub-core/net/drivers/efi/efinet.c
>> +++ b/grub-core/net/drivers/efi/efinet.c
>> @@ -156,59 +156,40 @@ get_card_packet (struct grub_net_card *dev)
>>   static grub_err_t
>>   open_card (struct grub_net_card *dev)
>>   {
>> -  grub_efi_simple_network_t *net;
>> +  grub_efi_simple_network_t *net = dev->efi_net;
>>
>> -  /* Try to reopen SNP exlusively to close any active MNP protocol instance
>> -     that may compete for packet polling
>> -   */
>> -  net = grub_efi_open_protocol (dev->efi_handle, &net_io_guid,
>> -                               GRUB_EFI_OPEN_PROTOCOL_BY_EXCLUSIVE);
>> -  if (net)
>> +  if (net->mode->state == GRUB_EFI_NETWORK_STOPPED)
>> +    return grub_error (GRUB_ERR_NET_NO_CARD, "%s: card stopped",
>> +                      dev->name);
>> +
>> +  if (net->mode->state == GRUB_EFI_NETWORK_STARTED
>> +      && efi_call_3 (net->initialize, net, 0, 0) != GRUB_EFI_SUCCESS)
>> +         return grub_error (GRUB_ERR_NET_NO_CARD, "%s: net initialize failed",
>> +                            dev->name);
>> +
>> +  /* Enable hardware receive filters if driver declares support for it.
>> +     We need unicast and broadcast and additionaly all nodes and
>> +     solicited multicast for IPv6. Solicited multicast is per-IPv6
>> +     address and we currently do not have API to do it so simply
>> +     try to enable receive of all multicast packets or evertyhing in
>> +     the worst case (i386 PXE driver always enables promiscuous too).
>> +
>> +     This does trust firmware to do what it claims to do.
>> +    */
>> +  if (net->mode->receive_filter_mask)
>>       {
>> -      if (net->mode->state == GRUB_EFI_NETWORK_STOPPED
>> -         && efi_call_1 (net->start, net) != GRUB_EFI_SUCCESS)
>> -       return grub_error (GRUB_ERR_NET_NO_CARD, "%s: net start failed",
>> -                          dev->name);
>> -
>> -      if (net->mode->state == GRUB_EFI_NETWORK_STOPPED)
>> -       return grub_error (GRUB_ERR_NET_NO_CARD, "%s: card stopped",
>> -                          dev->name);
>> -
>> -      if (net->mode->state == GRUB_EFI_NETWORK_STARTED
>> -         && efi_call_3 (net->initialize, net, 0, 0) != GRUB_EFI_SUCCESS)
>> -       return grub_error (GRUB_ERR_NET_NO_CARD, "%s: net initialize failed",
>> -                          dev->name);
>> -
>> -      /* Enable hardware receive filters if driver declares support for it.
>> -        We need unicast and broadcast and additionaly all nodes and
>> -        solicited multicast for IPv6. Solicited multicast is per-IPv6
>> -        address and we currently do not have API to do it so simply
>> -        try to enable receive of all multicast packets or evertyhing in
>> -        the worst case (i386 PXE driver always enables promiscuous too).
>> -
>> -        This does trust firmware to do what it claims to do.
>> -       */
>> -      if (net->mode->receive_filter_mask)
>> -       {
>> -         grub_uint32_t filters = GRUB_EFI_SIMPLE_NETWORK_RECEIVE_UNICAST   |
>> -                                 GRUB_EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST |
>> -                                 GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST;
>> -
>> -         filters &= net->mode->receive_filter_mask;
>> -         if (!(filters & GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST))
>> -           filters |= (net->mode->receive_filter_mask &
>> -                       GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS);
>> +      grub_uint32_t filters = GRUB_EFI_SIMPLE_NETWORK_RECEIVE_UNICAST   |
>> +             GRUB_EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST |
>> +             GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST;
>>
>> -         efi_call_6 (net->receive_filters, net, filters, 0, 0, 0, NULL);
>> -       }
>> +      filters &= net->mode->receive_filter_mask;
>> +      if (!(filters & GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST))
>> +       filters |= (net->mode->receive_filter_mask &
>> +                   GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS);
>>
>> -      efi_call_4 (grub_efi_system_table->boot_services->close_protocol,
>> -                 dev->efi_net, &net_io_guid,
>> -                 grub_efi_image_handle, dev->efi_handle);
>> -      dev->efi_net = net;
>> +      efi_call_6 (net->receive_filters, net, filters, 0, 0, 0, NULL);
>>       }
>>
>> -  /* If it failed we just try to run as best as we can */
>>     return GRUB_ERR_NONE;
>>   }
>>
>> @@ -278,12 +259,26 @@ grub_efinet_is_mac_device (struct grub_net_card *card)
>>     return 0;
>>   }
>>
>> +static grub_efi_uint32_t
>> +grub_snp_attributes (void)
>
> Attributes? I'd say
>
> use_snp ? GRUB_EFI_OPEN_PROTOCOL_BY_EXCLUSIVE :
> GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL
>
> looks more readable.
>
>> +{
>> +  /* Some firmware will completely screw up the transition to exclusive SNP,
>> +     doing things like not honoring receive filters or taking multiple seconds
>> +     to make the switch over.  So don't bother using exclusive in this case.
>> +   */
>> +  if (!grub_memcmp(grub_efi_system_table->firmware_vendor, "A", 1) &&
>> +      grub_efi_system_table->firmware_revision == (grub_efi_uint32_t)262798)
>
> Well, I'm not thrilled by this check ... at least we need to compare
> full firmware_vendor then.
>

That is the full firmware_vendor for our firmware.  I suppose I should 
also do strlen(firmware_vendor) == 1 && memcmp() to make sure it doesn't 
just match something that starts with A, I can fix that up.

>> +    return GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL;
>> +  return GRUB_EFI_OPEN_PROTOCOL_BY_EXCLUSIVE;
>> +}
>> +
>>   static void
>>   grub_efinet_findcards (void)
>>   {
>>     grub_efi_uintn_t num_handles;
>>     grub_efi_handle_t *handles;
>>     grub_efi_handle_t *handle;
>> +  grub_efi_uint32_t attributes;
>>     int i = 0;
>>
>>     /* Find handles which support the disk io interface.  */
>> @@ -291,6 +286,9 @@ grub_efinet_findcards (void)
>>                                      0, &num_handles);
>>     if (! handles)
>>       return;
>> +
>> +  attributes = grub_snp_attributes();
>> +
>>     for (handle = handles; num_handles--; handle++)
>>       {
>>         grub_efi_simple_network_t *net;
>> @@ -319,8 +317,7 @@ grub_efinet_findcards (void)
>>            && GRUB_EFI_DEVICE_PATH_SUBTYPE (parent) == GRUB_EFI_MAC_ADDRESS_DEVICE_PATH_SUBTYPE)
>>          continue;
>>
>> -      net = grub_efi_open_protocol (*handle, &net_io_guid,
>> -                                   GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>> +      net = grub_efi_open_protocol (*handle, &net_io_guid, attributes);
>
> No, we cannot open exclusively here, it will destroy autocnfiguration
> information we need later. You need to add conditional in open_card.

The autoconfig stuff still works later for me but I can change it back. 
  Thanks,

Josef

Re: [PATCH] efinet: check for broken firmware

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 9153 bytes --]

Please try the patch currently used in Solaris flavour of GRUB. I think of
upstreaming their mnp driver
Le 13 nov. 2015 3:30 PM, "Josef Bacik" <jbacik@fb.com> a écrit :

> On 11/13/2015 08:10 AM, Andrei Borzenkov wrote:
>
>> On Fri, Nov 13, 2015 at 1:07 AM, Josef Bacik <jbacik@fb.com> wrote:
>>
>>> The firmware bug I've been tracking down has been too extensive to work
>>> around
>>> effectively.  Basically once we switch to EXCLUSIVE everything is
>>> completely
>>> horked.  I can add a multicast filter but it's timing sensitive, so it
>>> has to be
>>> done at least 10 seconds after initialization for it to take place, and
>>> we have
>>> to continue to enable PROMISCUOUS_MULTICAST because otherwise we no
>>> longer get
>>> unicast traffic.  I discovered however that if we do not make the
>>> EXCLUSIVE
>>> switch over everything works fine.  So instead add a function that
>>> checks for
>>>
>>
>> Does your firmware use MNP at all?
>>
>>
> I was going to look into that today, with as much problems as SNP has been
> giving us I'm wondering if MNP works better.  I'm going to rig up a
> barebones MNP driver and see if it behaves better.  The question is how to
> make us use MNP vs. SNP when it is supported, or to force us to go back to
> SNP if MNP is fuuuuucked up.
>
> this broken firmware and uses GET_PROTOCOL instead of EXCLUSIVE.  This also
>>> keeps the original protocol we use when scanning the cards and leaves the
>>> initialization stuff for ->open.  Thanks,
>>>
>>> Signed-off-by: Josef Bacik <jbacik@fb.com>
>>> ---
>>>   grub-core/net/drivers/efi/efinet.c | 99
>>> ++++++++++++++++++--------------------
>>>   1 file changed, 46 insertions(+), 53 deletions(-)
>>>
>>> diff --git a/grub-core/net/drivers/efi/efinet.c
>>> b/grub-core/net/drivers/efi/efinet.c
>>> index c8f80a1..5a207fd 100644
>>> --- a/grub-core/net/drivers/efi/efinet.c
>>> +++ b/grub-core/net/drivers/efi/efinet.c
>>> @@ -156,59 +156,40 @@ get_card_packet (struct grub_net_card *dev)
>>>   static grub_err_t
>>>   open_card (struct grub_net_card *dev)
>>>   {
>>> -  grub_efi_simple_network_t *net;
>>> +  grub_efi_simple_network_t *net = dev->efi_net;
>>>
>>> -  /* Try to reopen SNP exlusively to close any active MNP protocol
>>> instance
>>> -     that may compete for packet polling
>>> -   */
>>> -  net = grub_efi_open_protocol (dev->efi_handle, &net_io_guid,
>>> -                               GRUB_EFI_OPEN_PROTOCOL_BY_EXCLUSIVE);
>>> -  if (net)
>>> +  if (net->mode->state == GRUB_EFI_NETWORK_STOPPED)
>>> +    return grub_error (GRUB_ERR_NET_NO_CARD, "%s: card stopped",
>>> +                      dev->name);
>>> +
>>> +  if (net->mode->state == GRUB_EFI_NETWORK_STARTED
>>> +      && efi_call_3 (net->initialize, net, 0, 0) != GRUB_EFI_SUCCESS)
>>> +         return grub_error (GRUB_ERR_NET_NO_CARD, "%s: net initialize
>>> failed",
>>> +                            dev->name);
>>> +
>>> +  /* Enable hardware receive filters if driver declares support for it.
>>> +     We need unicast and broadcast and additionaly all nodes and
>>> +     solicited multicast for IPv6. Solicited multicast is per-IPv6
>>> +     address and we currently do not have API to do it so simply
>>> +     try to enable receive of all multicast packets or evertyhing in
>>> +     the worst case (i386 PXE driver always enables promiscuous too).
>>> +
>>> +     This does trust firmware to do what it claims to do.
>>> +    */
>>> +  if (net->mode->receive_filter_mask)
>>>       {
>>> -      if (net->mode->state == GRUB_EFI_NETWORK_STOPPED
>>> -         && efi_call_1 (net->start, net) != GRUB_EFI_SUCCESS)
>>> -       return grub_error (GRUB_ERR_NET_NO_CARD, "%s: net start failed",
>>> -                          dev->name);
>>> -
>>> -      if (net->mode->state == GRUB_EFI_NETWORK_STOPPED)
>>> -       return grub_error (GRUB_ERR_NET_NO_CARD, "%s: card stopped",
>>> -                          dev->name);
>>> -
>>> -      if (net->mode->state == GRUB_EFI_NETWORK_STARTED
>>> -         && efi_call_3 (net->initialize, net, 0, 0) != GRUB_EFI_SUCCESS)
>>> -       return grub_error (GRUB_ERR_NET_NO_CARD, "%s: net initialize
>>> failed",
>>> -                          dev->name);
>>> -
>>> -      /* Enable hardware receive filters if driver declares support for
>>> it.
>>> -        We need unicast and broadcast and additionaly all nodes and
>>> -        solicited multicast for IPv6. Solicited multicast is per-IPv6
>>> -        address and we currently do not have API to do it so simply
>>> -        try to enable receive of all multicast packets or evertyhing in
>>> -        the worst case (i386 PXE driver always enables promiscuous too).
>>> -
>>> -        This does trust firmware to do what it claims to do.
>>> -       */
>>> -      if (net->mode->receive_filter_mask)
>>> -       {
>>> -         grub_uint32_t filters =
>>> GRUB_EFI_SIMPLE_NETWORK_RECEIVE_UNICAST   |
>>> -
>>>  GRUB_EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST |
>>> -
>>>  GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST;
>>> -
>>> -         filters &= net->mode->receive_filter_mask;
>>> -         if (!(filters &
>>> GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST))
>>> -           filters |= (net->mode->receive_filter_mask &
>>> -                       GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS);
>>> +      grub_uint32_t filters = GRUB_EFI_SIMPLE_NETWORK_RECEIVE_UNICAST
>>>  |
>>> +             GRUB_EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST |
>>> +             GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST;
>>>
>>> -         efi_call_6 (net->receive_filters, net, filters, 0, 0, 0, NULL);
>>> -       }
>>> +      filters &= net->mode->receive_filter_mask;
>>> +      if (!(filters &
>>> GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST))
>>> +       filters |= (net->mode->receive_filter_mask &
>>> +                   GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS);
>>>
>>> -      efi_call_4 (grub_efi_system_table->boot_services->close_protocol,
>>> -                 dev->efi_net, &net_io_guid,
>>> -                 grub_efi_image_handle, dev->efi_handle);
>>> -      dev->efi_net = net;
>>> +      efi_call_6 (net->receive_filters, net, filters, 0, 0, 0, NULL);
>>>       }
>>>
>>> -  /* If it failed we just try to run as best as we can */
>>>     return GRUB_ERR_NONE;
>>>   }
>>>
>>> @@ -278,12 +259,26 @@ grub_efinet_is_mac_device (struct grub_net_card
>>> *card)
>>>     return 0;
>>>   }
>>>
>>> +static grub_efi_uint32_t
>>> +grub_snp_attributes (void)
>>>
>>
>> Attributes? I'd say
>>
>> use_snp ? GRUB_EFI_OPEN_PROTOCOL_BY_EXCLUSIVE :
>> GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL
>>
>> looks more readable.
>>
>> +{
>>> +  /* Some firmware will completely screw up the transition to exclusive
>>> SNP,
>>> +     doing things like not honoring receive filters or taking multiple
>>> seconds
>>> +     to make the switch over.  So don't bother using exclusive in this
>>> case.
>>> +   */
>>> +  if (!grub_memcmp(grub_efi_system_table->firmware_vendor, "A", 1) &&
>>> +      grub_efi_system_table->firmware_revision ==
>>> (grub_efi_uint32_t)262798)
>>>
>>
>> Well, I'm not thrilled by this check ... at least we need to compare
>> full firmware_vendor then.
>>
>>
> That is the full firmware_vendor for our firmware.  I suppose I should
> also do strlen(firmware_vendor) == 1 && memcmp() to make sure it doesn't
> just match something that starts with A, I can fix that up.
>
> +    return GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL;
>>> +  return GRUB_EFI_OPEN_PROTOCOL_BY_EXCLUSIVE;
>>> +}
>>> +
>>>   static void
>>>   grub_efinet_findcards (void)
>>>   {
>>>     grub_efi_uintn_t num_handles;
>>>     grub_efi_handle_t *handles;
>>>     grub_efi_handle_t *handle;
>>> +  grub_efi_uint32_t attributes;
>>>     int i = 0;
>>>
>>>     /* Find handles which support the disk io interface.  */
>>> @@ -291,6 +286,9 @@ grub_efinet_findcards (void)
>>>                                      0, &num_handles);
>>>     if (! handles)
>>>       return;
>>> +
>>> +  attributes = grub_snp_attributes();
>>> +
>>>     for (handle = handles; num_handles--; handle++)
>>>       {
>>>         grub_efi_simple_network_t *net;
>>> @@ -319,8 +317,7 @@ grub_efinet_findcards (void)
>>>            && GRUB_EFI_DEVICE_PATH_SUBTYPE (parent) ==
>>> GRUB_EFI_MAC_ADDRESS_DEVICE_PATH_SUBTYPE)
>>>          continue;
>>>
>>> -      net = grub_efi_open_protocol (*handle, &net_io_guid,
>>> -                                   GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>>> +      net = grub_efi_open_protocol (*handle, &net_io_guid, attributes);
>>>
>>
>> No, we cannot open exclusively here, it will destroy autocnfiguration
>> information we need later. You need to add conditional in open_card.
>>
>
> The autoconfig stuff still works later for me but I can change it back.
> Thanks,
>
> Josef
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 11213 bytes --]

Re: [PATCH] efinet: check for broken firmware

[Andrei Borzenkov]

On Fri, Nov 13, 2015 at 5:30 PM, Josef Bacik <jbacik@fb.com> wrote:
>>> @@ -291,6 +286,9 @@ grub_efinet_findcards (void)
>>>                                      0, &num_handles);
>>>     if (! handles)
>>>       return;
>>> +
>>> +  attributes = grub_snp_attributes();
>>> +
>>>     for (handle = handles; num_handles--; handle++)
>>>       {
>>>         grub_efi_simple_network_t *net;
>>> @@ -319,8 +317,7 @@ grub_efinet_findcards (void)
>>>            && GRUB_EFI_DEVICE_PATH_SUBTYPE (parent) ==
>>> GRUB_EFI_MAC_ADDRESS_DEVICE_PATH_SUBTYPE)
>>>          continue;
>>>
>>> -      net = grub_efi_open_protocol (*handle, &net_io_guid,
>>> -                                   GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>>> +      net = grub_efi_open_protocol (*handle, &net_io_guid, attributes);
>>
>>
>> No, we cannot open exclusively here, it will destroy autocnfiguration
>> information we need later. You need to add conditional in open_card.
>
>
> The autoconfig stuff still works later for me but I can change it back.

Thaty would mean your firmware probably is SNP based and not MNP
based. But it will definitely break autoconfig for others - was there,
done that :)

Re: [PATCH] efinet: check for broken firmware

[Andrei Borzenkov]

On Fri, Nov 13, 2015 at 5:38 PM, Vladimir 'phcoder' Serbinenko
<phcoder@gmail.com> wrote:
> Please try the patch currently used in Solaris flavour of GRUB. I think of
> upstreaming their mnp driver


where can we get it?

Re: [PATCH] efinet: check for broken firmware

[Josef Bacik]

On 11/13/2015 09:38 AM, Andrei Borzenkov wrote:
> On Fri, Nov 13, 2015 at 5:30 PM, Josef Bacik <jbacik@fb.com> wrote:
>>>> @@ -291,6 +286,9 @@ grub_efinet_findcards (void)
>>>>                                       0, &num_handles);
>>>>      if (! handles)
>>>>        return;
>>>> +
>>>> +  attributes = grub_snp_attributes();
>>>> +
>>>>      for (handle = handles; num_handles--; handle++)
>>>>        {
>>>>          grub_efi_simple_network_t *net;
>>>> @@ -319,8 +317,7 @@ grub_efinet_findcards (void)
>>>>             && GRUB_EFI_DEVICE_PATH_SUBTYPE (parent) ==
>>>> GRUB_EFI_MAC_ADDRESS_DEVICE_PATH_SUBTYPE)
>>>>           continue;
>>>>
>>>> -      net = grub_efi_open_protocol (*handle, &net_io_guid,
>>>> -                                   GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>>>> +      net = grub_efi_open_protocol (*handle, &net_io_guid, attributes);
>>>
>>>
>>> No, we cannot open exclusively here, it will destroy autocnfiguration
>>> information we need later. You need to add conditional in open_card.
>>
>>
>> The autoconfig stuff still works later for me but I can change it back.
>
> Thaty would mean your firmware probably is SNP based and not MNP
> based. But it will definitely break autoconfig for others - was there,
> done that :)
>

Fair enough, looks like this firmware is 2.3.1 so theoretically has MNP, 
I'll try and track down this solaris mnp driver and see if I can get it 
to work.  Thanks,

Josef

Re: [PATCH] efinet: check for broken firmware

[Josef Bacik]

On 11/13/2015 09:39 AM, Andrei Borzenkov wrote:
> On Fri, Nov 13, 2015 at 5:38 PM, Vladimir 'phcoder' Serbinenko
> <phcoder@gmail.com> wrote:
>> Please try the patch currently used in Solaris flavour of GRUB. I think of
>> upstreaming their mnp driver
>
>
> where can we get it?
>

It is in part2 of the solaris source code here

http://www.oracle.com/technetwork/opensource/systems-solaris-1562786.html

You're welcome, I just saved you a 1.9 gig download of part 1 ;),

Josef

Re: [PATCH] efinet: check for broken firmware

[Andrei Borzenkov]

13.11.2015 18:52, Josef Bacik пишет:
> On 11/13/2015 09:39 AM, Andrei Borzenkov wrote:
>> On Fri, Nov 13, 2015 at 5:38 PM, Vladimir 'phcoder' Serbinenko
>> <phcoder@gmail.com> wrote:
>>> Please try the patch currently used in Solaris flavour of GRUB. I
>>> think of
>>> upstreaming their mnp driver
>>
>>
>> where can we get it?
>>
>
> It is in part2 of the solaris source code here
>
> http://www.oracle.com/technetwork/opensource/systems-solaris-1562786.html

Ah, I was not aware of this. Thanks!

>
> You're welcome, I just saved you a 1.9 gig download of part 1 ;),
>

Sure, much appreciated.

Re: [PATCH] efinet: check for broken firmware

[Josef Bacik]

On 11/13/2015 09:38 AM, Vladimir 'phcoder' Serbinenko wrote:
> Please try the patch currently used in Solaris flavour of GRUB. I think
> of upstreaming their mnp driver

This driver doesn't work at all for me.  I may have done a bad job 
porting it, or the firmware is even crappier with MNP, either way it's 
completely unusable.  I'll fix up this patch and send it along.  Thanks,

Josef

Re: [PATCH] efinet: check for broken firmware

[Andrei Borzenkov]

13.11.2015 22:34, Josef Bacik пишет:
> On 11/13/2015 09:38 AM, Vladimir 'phcoder' Serbinenko wrote:
>> Please try the patch currently used in Solaris flavour of GRUB. I think
>> of upstreaming their mnp driver
>
> This driver doesn't work at all for me.  I may have done a bad job
> porting it, or the firmware is even crappier with MNP, either way it's
> completely unusable.  I'll fix up this patch and send it along.  Thanks,
>

So that I understand. You have a system where a) there is MNP Binding 
Protocol on network device and b) this does not work? Or your system 
does not offer MNP Binding Protocol for device?

Re: [PATCH] efinet: check for broken firmware

[Josef Bacik]

> On Nov 13, 2015, at 10:19 PM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 
> 13.11.2015 22:34, Josef Bacik пишет:
>>> On 11/13/2015 09:38 AM, Vladimir 'phcoder' Serbinenko wrote:
>>> Please try the patch currently used in Solaris flavour of GRUB. I think
>>> of upstreaming their mnp driver
>> 
>> This driver doesn't work at all for me.  I may have done a bad job
>> porting it, or the firmware is even crappier with MNP, either way it's
>> completely unusable.  I'll fix up this patch and send it along.  Thanks,
> 
> So that I understand. You have a system where a) there is MNP Binding Protocol on network device and b) this does not work? Or your system does not offer MNP Binding Protocol for device?

There is a MNP binding and the driver does not work. It sets everything up right but doesn't transmit or receive properly.

I figured out why my original approach wasn't working and why it works if I don't switch to exclusive.  I can now switch to exclusive and have things still working, just fixing up the patch now, I'll finish it on Monday and send it out.  Thanks,

Josef

Re: [PATCH] efinet: check for broken firmware

[Andrei Borzenkov]

14.11.2015 07:08, Josef Bacik пишет:
>
>> On Nov 13, 2015, at 10:19 PM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>>
>> 13.11.2015 22:34, Josef Bacik пишет:
>>>> On 11/13/2015 09:38 AM, Vladimir 'phcoder' Serbinenko wrote:
>>>> Please try the patch currently used in Solaris flavour of GRUB. I think
>>>> of upstreaming their mnp driver
>>>
>>> This driver doesn't work at all for me.  I may have done a bad job
>>> porting it, or the firmware is even crappier with MNP, either way it's
>>> completely unusable.  I'll fix up this patch and send it along.  Thanks,
>>
>> So that I understand. You have a system where a) there is MNP Binding Protocol on network device and b) this does not work? Or your system does not offer MNP Binding Protocol for device?
>
> There is a MNP binding and the driver does not work. It sets everything up right but doesn't transmit or receive properly.
>

That's bad. It means even if we prefer MNP we still need to implement 
quirks to avoid it on known broken platforms.

> I figured out why my original approach wasn't working and why it works if I don't switch to exclusive.  I can now switch to exclusive and have things still working, just fixing up the patch now, I'll finish it on Monday and send it out.  Thanks,
>
> Josef
>

Re: [PATCH] efinet: check for broken firmware

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

Le 14 nov. 2015 5:08 AM, "Josef Bacik" <jbacik@fb.com> a écrit :

>

>

> > On Nov 13, 2015, at 10:19 PM, Andrei Borzenkov <arvidjaar@gmail.com>
wrote:

> >

> > 13.11.2015 22:34, Josef Bacik пишет:

> >>> On 11/13/2015 09:38 AM, Vladimir 'phcoder' Serbinenko wrote:

> >>> Please try the patch currently used in Solaris flavour of GRUB. I
think

> >>> of upstreaming their mnp driver

> >>

> >> This driver doesn't work at all for me.  I may have done a bad job

> >> porting it, or the firmware is even crappier with MNP, either way it's

> >> completely unusable.  I'll fix up this patch and send it along.
Thanks,

> >

> > So that I understand. You have a system where a) there is MNP Binding
Protocol on network device and b) this does not work? Or your system does
not offer MNP Binding Protocol for device?

>

> There is a MNP binding and the driver does not work. It sets everything
up right but doesn't transmit or receive properly.

>

Are you sure that the driver itself works? Could you have introduced bugs
when adapting it?

> I figured out why my original approach wasn't working and why it works if
I don't switch to exclusive.  I can now switch to exclusive and have things
still working, just fixing up the patch now, I'll finish it on Monday and
send it out.  Thanks,

>

> Josef

[-- Attachment #2: Type: text/html, Size: 1696 bytes --]

Re: [PATCH] efinet: check for broken firmware

[Josef Bacik]

On 11/14/2015 09:13 AM, Vladimir 'phcoder' Serbinenko wrote:
> Le 14 nov. 2015 5:08 AM, "Josef Bacik" <jbacik@fb.com
> <mailto:jbacik@fb.com>> a écrit :
>
>  >
>
>  >
>
>  > > On Nov 13, 2015, at 10:19 PM, Andrei Borzenkov <arvidjaar@gmail.com
> <mailto:arvidjaar@gmail.com>> wrote:
>
>  > >
>
>  > > 13.11.2015 22:34, Josef Bacik пишет:
>
>  > >>> On 11/13/2015 09:38 AM, Vladimir 'phcoder' Serbinenko wrote:
>
>  > >>> Please try the patch currently used in Solaris flavour of GRUB. I
> think
>
>  > >>> of upstreaming their mnp driver
>
>  > >>
>
>  > >> This driver doesn't work at all for me.  I may have done a bad job
>
>  > >> porting it, or the firmware is even crappier with MNP, either way it's
>
>  > >> completely unusable.  I'll fix up this patch and send it along.
> Thanks,
>
>  > >
>
>  > > So that I understand. You have a system where a) there is MNP
> Binding Protocol on network device and b) this does not work? Or your
> system does not offer MNP Binding Protocol for device?
>
>  >
>
>  > There is a MNP binding and the driver does not work. It sets
> everything up right but doesn't transmit or receive properly.
>
>  >
>
> Are you sure that the driver itself works? Could you have introduced
> bugs when adapting it?
>

I could have, the only thing I had to do was add the definitions for the 
efi api's that we are missing, so the trigger stuff basically.  It was 
just copy and pasting, it's not like I changed any functionality, I 
didn't touch the driver at all.  Their grub2 version seems to be a 
little older.  If somebody gets a driver into the git tree then I'll 
happily test it, but this port definitely didn't work and I have little 
interest in trying to adapt somebody else's code and trying to debug it. 
  Thanks,

Josef