Hi Heinrich,

2021年9月15日(水) 10:31 Masami Hiramatsu masami.hiramatsu@linaro.org:

Hi Heinrich,

This is obscure in the specification (I can not see any detailed description about EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT).

OK, I checked the UEFI specification v2.9 again. The main purpose of EFI_SIMPLE_NETWORK.GetStatus() seems to update the EFI_SIMPLE_NETWORK_MODE::MediaPresent, in other words, it checks link status. And InterruptStatus of EFI_SIMPLE_NETWORK.GetStatus() means "the currently active interrupts", not "packet to be received". Thus, I think it should be tested for checking the link status instead of checking receive packets in DHCP. You also can see the EFI_SIMPLE_NETWORK_PROTOCOL::WaitForPacket ensures that you can receive the packet (The spec says "Event used with EFI_BOOT_SERVICES.WaitForEvent() to wait for a packet to be received." ) So, it is enough to use the WaitForPacket in the packet receiving loop, no need to use GetStatus(). Then, correct test should be something like this.

---- net->get_status(); if (!net->mode.MediaPresent) { error(no link up!) return; }

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

For your information, as far as I can see, the interrupt bit is cleared or not depends on the platform driver implementation in EDK2 too. In many cases (e.g. virtio-net), they do not clear the original bits, in another case, no interrupt bit supported (IntrruptStatus always be 0). But I couldn't find the code which really cleared the interrupt bit... So I think there is a difference between UEFI specification and reference implementation (EDK2).

Thank you,

Thank you,

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

...

On 9/14/21 4:06 AM, Masami Hiramatsu wrote:

...

From: Kazuhiko Sakamoto sakamoto.kazuhiko@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@socionext.com Signed-off-by: Masami Hiramatsu masami.hiramatsu@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

-- Masami Hiramatsu