Re: [PATCH V2] efinet: handle get_status() on buggy firmware properly

[Andrei Borzenkov]

В Thu,  6 Aug 2015 10:49:46 -0700
Josef Bacik <address@hidden> пишет:

> The EFI spec indicates that get_status() should return the address of the 
> buffer
> we passed into transmit to indicate the the buffer was transmitted.  However 
> we
> have boxes where the firmware returns some arbitrary address instead, which
> makes grub think that we've not sent anything.  So since we have the SNP stuff
> opened in exclusive mode just assume any non-NULL txbuf means that our 
> transmit
> occurred properly.  This makes grub able to do its networking stuff properly 
> on
> our broken firmware.  Thanks,
> 

Committed. Let's see if someone screams.

> cc: Peter Jones <address@hidden>
> Signed-off-by: Josef Bacik <address@hidden>
> ---
> V1->V2:
> -updated the changelog to be more clear about what we are fixing.
> -added a comment to the code to indicate why it is safe to only check for
>  non-NULL txbufs and why we need to do it.
> 
>  grub-core/net/drivers/efi/efinet.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/grub-core/net/drivers/efi/efinet.c 
> b/grub-core/net/drivers/efi/efinet.c
> index f27a117..692d5ad 100644
> --- a/grub-core/net/drivers/efi/efinet.c
> +++ b/grub-core/net/drivers/efi/efinet.c
> @@ -47,19 +47,19 @@ send_card_buffer (struct grub_net_card *dev,
>       if (st != GRUB_EFI_SUCCESS)
>         return grub_error (GRUB_ERR_IO,
>                            N_("couldn't send network packet"));
> -     if (txbuf == dev->txbuf)
> +     /*
> +        Some buggy firmware could return an arbitrary address instead of the
> +        txbuf address we trasmitted, so just check that txbuf is non NULL
> +        for success.  This is ok because we open the SNP protocol in
> +        exclusive mode so we know we're the only ones transmitting on this
> +        box and since we only transmit one packet at a time we know our
> +        transmit was successfull.
> +      */
> +     if (txbuf)
>         {
>           dev->txbusy = 0;
>           break;
>         }
> -     if (txbuf)
> -       {
> -         st = efi_call_7 (net->transmit, net, 0, dev->last_pkt_size,
> -                          dev->txbuf, NULL, NULL, NULL);
> -         if (st != GRUB_EFI_SUCCESS)
> -           return grub_error (GRUB_ERR_IO,
> -                              N_("couldn't send network packet"));
> -       }
>       if (limit_time < grub_get_time_ms ())
>         return grub_error (GRUB_ERR_TIMEOUT,
>                            N_("couldn't send network packet"));
> @@ -84,8 +84,9 @@ send_card_buffer (struct grub_net_card *dev,
>       we run in the GRUB_ERR_TIMEOUT case above.
>       Perhaps a timeout in the FW has discarded the recycle buffer.
>     */
> +  txbuf = NULL;
>    st = efi_call_3 (net->get_status, net, 0, &txbuf);
> -  dev->txbusy = !(st == GRUB_EFI_SUCCESS && txbuf == dev->txbuf);
> +  dev->txbusy = !(st == GRUB_EFI_SUCCESS && txbuf);
>  
>    return GRUB_ERR_NONE;
>  }