grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] efinet: check for broken firmware


From: Vladimir 'phcoder' Serbinenko
Subject: Re: [PATCH] efinet: check for broken firmware
Date: Fri, 13 Nov 2015 15:38:48 +0100

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" <address@hidden> a écrit :
On 11/13/2015 08:10 AM, Andrei Borzenkov wrote:
On Fri, Nov 13, 2015 at 1:07 AM, Josef Bacik <address@hidden> 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 <address@hidden>
---
  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
address@hidden
https://lists.gnu.org/mailman/listinfo/grub-devel

reply via email to

[Prev in Thread] Current Thread [Next in Thread]