[PATCH] efi_loader: Set EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT again in efi_net_receive()

[Masami Hiramatsu]

Hi Heinrich,

This is obscure in the specification (I can not see any detailed
description about EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT).
If EFI_SIMPLE_NETWORK.GetStatus() returns only the hardware interrupt
state, it is an useless interface, because it doesn't sync to the
status of the packet buffer and wait_for_event() result.
If EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT is set only if the rx
interrupts (this is not the rx interrupt in U-Boot anyway...) is
coming, the efi selftest code must not use the net->get_status() for
checking the packet is received, or call net->receive() until the
packet buffer is empty(EFI_NOT_READY) (and also, it has to ensure the
packet buffer is empty before calling wait_for_event()).

So, I think the correct test code will be something like this;

submit_dhcp_discover()
for (;;) {
  wait_for_event(net)
  net->get_status() // and check receive interrupt
  while (net->receive() != EFI_NOT_READY) {
     // check dhcp reply
  }
}

Thank you,

2021年9月15日(水) 0:45 Heinrich Schuchardt <xypron.glpk at gmx.de>:

>
> On 9/14/21 4:06 AM, Masami Hiramatsu wrote:
> > From: Kazuhiko Sakamoto <sakamoto.kazuhiko at socionext.com>
> >
> > Since 'this->int_status' is cleared by efi_net_get_status(), if user
>
> Is it correct to clear int_status unconditionally in efi_net_get_status()?
>
> Shouldn't two consecutive calls to EFI_SIMPLE_NETWORK.GetStatus() return
> the same result?
>
> Best regards
>
> Heinrich
>
> > does wait_for_event(wait_for_packet) and efi_net_get_status() loop
> > and there are several received packets on the buffer, the second
> > efi_net_get_status() does not return EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT
> > even if the 'wait_for_packet' is ready.
> >
> > This happens on "efiboot selftest" with noisy network.
> > (The network device can receive both of other packets and dhcp discover
> >   packet in a short time.)
> >
> > Set EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT again when we found any
> > packet still on the receive buffer.
> >
> > Signed-off-by: Kazuhiko Sakamoto <sakamoto.kazuhiko at socionext.com>
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>
> > ---
> >   lib/efi_loader/efi_net.c |   13 +++++++++++--
> >   1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
> > index 69276b275d..9ca1e0c354 100644
> > --- a/lib/efi_loader/efi_net.c
> > +++ b/lib/efi_loader/efi_net.c
> > @@ -640,10 +640,19 @@ static efi_status_t EFIAPI efi_net_receive
> >       *buffer_size = receive_lengths[rx_packet_idx];
> >       rx_packet_idx = (rx_packet_idx + 1) % ETH_PACKETS_BATCH_RECV;
> >       rx_packet_num--;
> > -     if (rx_packet_num)
> > +     if (rx_packet_num) {
> > +             /*
> > +              * Since 'this->int_status' is cleared by efi_net_get_status(),
> > +              * if user does wait_for_event(wait_for_packet) and
> > +              * efi_net_get_status() again, EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT
> > +              * is not set even if 'wait_for_packet' is ready.
> > +              * Set EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT again here.
> > +              */
> > +             this->int_status |= EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
> >               wait_for_packet->is_signaled = true;
> > -     else
> > +     } else {
> >               this->int_status &= ~EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
> > +     }
> >   out:
> >       return EFI_EXIT(ret);
> >   }
> >
>


--
Masami Hiramatsu