[2/3] efi_selftest: Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT

Message ID 163178241612.65790.3191991741031171590.stgit@localhost
State Accepted
Commit 9845b924369cc71457a21e78b5f9e6f7af43532d
Headers show
Series
  • efi_selftest: Update SIMPLE_NETWORK_PROTOCOL selftest
Related show

Commit Message

Masami Hiramatsu Sept. 16, 2021, 8:53 a.m.
Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT in packet
receiving loop. This depends on the implementation and not
related to whether the packet can be received or not.

Whether the received packets are available or not is ensured
by wait_for_packet, and that is already done in the loop.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>

---
 lib/efi_selftest/efi_selftest_snp.c |   11 -----------
 1 file changed, 11 deletions(-)

Comments

Heinrich Schuchardt Sept. 16, 2021, 9:30 a.m. | #1
On 9/16/21 10:53 AM, Masami Hiramatsu wrote:
> Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT in packet

> receiving loop. This depends on the implementation and not

> related to whether the packet can be received or not.


Shouldn't EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT always be set if a
package needs to be processed?

Best regards

Heinrich

>

> Whether the received packets are available or not is ensured

> by wait_for_packet, and that is already done in the loop.

>

> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>

> ---

>   lib/efi_selftest/efi_selftest_snp.c |   11 -----------

>   1 file changed, 11 deletions(-)

>

> diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c

> index cb0db7eea2..c5366c872c 100644

> --- a/lib/efi_selftest/efi_selftest_snp.c

> +++ b/lib/efi_selftest/efi_selftest_snp.c

> @@ -340,8 +340,6 @@ static int execute(void)

>   	events[0] = timer;

>   	events[1] = net->wait_for_packet;

>   	for (;;) {

> -		u32 int_status;

> -

>   		/*

>   		 * Wait for packet to be received or timer event.

>   		 */

> @@ -367,15 +365,6 @@ static int execute(void)

>   		 * Receive packet

>   		 */

>   		buffer_size = sizeof(buffer);

> -		ret = net->get_status(net, &int_status, NULL);

> -		if (ret != EFI_SUCCESS) {

> -			efi_st_error("Failed to get status");

> -			return EFI_ST_FAILURE;

> -		}

> -		if (!(int_status & EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT)) {

> -			efi_st_error("RX interrupt not set");

> -			return EFI_ST_FAILURE;

> -		}

>   		ret = net->receive(net, NULL, &buffer_size, &buffer,

>   				   &srcaddr, &destaddr, NULL);

>   		if (ret != EFI_SUCCESS) {

>
Masami Hiramatsu Sept. 16, 2021, 9:57 a.m. | #2
Hi Heinrich,

2021年9月16日(木) 18:30 Heinrich Schuchardt <xypron.glpk@gmx.de>:
>

>

>

> On 9/16/21 10:53 AM, Masami Hiramatsu wrote:

> > Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT in packet

> > receiving loop. This depends on the implementation and not

> > related to whether the packet can be received or not.

>

> Shouldn't EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT always be set if a

> package needs to be processed?


No, that is not promised as far as I can read the specification.
It is just a bit flag of the device (Spec says "the interrupt status will be
read from the device."). Not ensure the received packet buffer status.

However, the WaitForPacket ensures the packet is received.
"Event used with EFI_BOOT_SERVICES.WaitForEvent() to wait
for a packet to be received."

So I think it is enough to check the WaitForPacket.

Thank you,


>

> Best regards

>

> Heinrich

>

> >

> > Whether the received packets are available or not is ensured

> > by wait_for_packet, and that is already done in the loop.

> >

> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>

> > ---

> >   lib/efi_selftest/efi_selftest_snp.c |   11 -----------

> >   1 file changed, 11 deletions(-)

> >

> > diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c

> > index cb0db7eea2..c5366c872c 100644

> > --- a/lib/efi_selftest/efi_selftest_snp.c

> > +++ b/lib/efi_selftest/efi_selftest_snp.c

> > @@ -340,8 +340,6 @@ static int execute(void)

> >       events[0] = timer;

> >       events[1] = net->wait_for_packet;

> >       for (;;) {

> > -             u32 int_status;

> > -

> >               /*

> >                * Wait for packet to be received or timer event.

> >                */

> > @@ -367,15 +365,6 @@ static int execute(void)

> >                * Receive packet

> >                */

> >               buffer_size = sizeof(buffer);

> > -             ret = net->get_status(net, &int_status, NULL);

> > -             if (ret != EFI_SUCCESS) {

> > -                     efi_st_error("Failed to get status");

> > -                     return EFI_ST_FAILURE;

> > -             }

> > -             if (!(int_status & EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT)) {

> > -                     efi_st_error("RX interrupt not set");

> > -                     return EFI_ST_FAILURE;

> > -             }

> >               ret = net->receive(net, NULL, &buffer_size, &buffer,

> >                                  &srcaddr, &destaddr, NULL);

> >               if (ret != EFI_SUCCESS) {

> >




--
Masami Hiramatsu

Patch

diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c
index cb0db7eea2..c5366c872c 100644
--- a/lib/efi_selftest/efi_selftest_snp.c
+++ b/lib/efi_selftest/efi_selftest_snp.c
@@ -340,8 +340,6 @@  static int execute(void)
 	events[0] = timer;
 	events[1] = net->wait_for_packet;
 	for (;;) {
-		u32 int_status;
-
 		/*
 		 * Wait for packet to be received or timer event.
 		 */
@@ -367,15 +365,6 @@  static int execute(void)
 		 * Receive packet
 		 */
 		buffer_size = sizeof(buffer);
-		ret = net->get_status(net, &int_status, NULL);
-		if (ret != EFI_SUCCESS) {
-			efi_st_error("Failed to get status");
-			return EFI_ST_FAILURE;
-		}
-		if (!(int_status & EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT)) {
-			efi_st_error("RX interrupt not set");
-			return EFI_ST_FAILURE;
-		}
 		ret = net->receive(net, NULL, &buffer_size, &buffer,
 				   &srcaddr, &destaddr, NULL);
 		if (ret != EFI_SUCCESS) {