[gPXE] Patch to gPXE-1.0.1 UNDI code for Interrupt Vs Polling

[Silver, Geoffrey]

Awesome, thanks!

--
Geoffrey Silver
Global Infrastructure
CITADEL


-----Original Message-----
From: Marty Connor [mailto:mdc at etherboot.org] 
Sent: Tuesday, November 30, 2010 12:40 PM
To: Silver, Geoffrey
Cc: 'Marty Connor'; 'Muralidhar.Appalla at emulex.com'; 'gpxe at etherboot.org'; 'etherboot-developers at lists.sourceforge.net'
Subject: Re: [gPXE] Patch to gPXE-1.0.1 UNDI code for Interrupt Vs Polling

Cool, thanks.  We'll get this in the gPXE-1.0.2 queue.  Thanks for your testing and confirmation.

Regards,

/ Marty /

On 11/30/10 1:27 PM, Silver, Geoffrey wrote:
> Marty,
>
> I can confirm that the original patch is reversed.  I applied the following, and it resolves my gPXE failures for both HP G7 servers using the Emulex (ServerEngines) NICs as well as Xsigo HCAs.  Presumably this affects other polling NICs as well.
>
>
> --- gpxe.orig/src/arch/i386/drivers/net//undinet.c     2010-11-16 07:47:29.000000000 -0800
> +++ gpxe.new/src/arch/i386/drivers/net//undinet.c       2010-11-16 07:53:07.000000000 -0800
> @@ -49,6 +49,8 @@
>          int isr_processing;
>          /** Bug workarounds */
>          int hacks;
> +       /** Flag to indicate device supports IRQ or not */
> +       int irq_supported;
>   };
>
>   /**
> @@ -252,7 +254,7 @@
>
>          if ( ! undinic->isr_processing ) {
>                  /* Do nothing unless ISR has been triggered */
> -               if ( ! undinet_isr_triggered() ) {
> +               if ( undinic->irq_supported&&  ! 
> + undinet_isr_triggered() ) {
>                          /* Allow interrupt to occur */
>                          __asm__ __volatile__ ( REAL_CODE ( "sti\n\t"
>                                                             "nop\n\t"
> @@ -357,10 +359,12 @@
>          struct s_PXENV_UNDI_OPEN undi_open;
>          int rc;
>
> -       /* Hook interrupt service routine and enable interrupt */
> -       undinet_hook_isr ( undinic->irq );
> -       enable_irq ( undinic->irq );
> -       send_eoi ( undinic->irq );
> +       /* Hook interrupt service routine and enable interrupt if supported */
> +        if (undinic->irq_supported) {
> +          undinet_hook_isr ( undinic->irq );
> +          enable_irq ( undinic->irq );
> +          send_eoi ( undinic->irq );
> +        }
>
>          /* Set station address.  Required for some PXE stacks; will
>           * spuriously fail on others.  Ignore failures.  We only ever 
> @@ -425,9 +429,11 @@
>          pxeparent_call ( undinet_entry, PXENV_UNDI_CLOSE,
>                           &undi_close, sizeof ( undi_close ) );
>
> -       /* Disable interrupt and unhook ISR */
> -       disable_irq ( undinic->irq );
> -       undinet_unhook_isr ( undinic->irq );
> +       /* Disable interrupt and unhook ISR if supported */
> +        if (undinic->irq_supported) {
> +          disable_irq ( undinic->irq );
> +          undinet_unhook_isr ( undinic->irq );
> +        }
>
>          DBGC ( undinic, "UNDINIC %p closed\n", undinic );
>   }
> @@ -548,6 +554,7 @@
>                  undinic->hacks |= UNDI_HACK_EB54;
>          }
>
> +        undinic->irq_supported = undi_iface.ServiceFlags&  
> + SUPPORTED_IRQ ? 1 : 0;
>          /* Mark as link up; we don't handle link state */
>          netdev_link_up ( netdev );
>
> --
> Geoffrey Silver
> Global Infrastructure
> CITADEL
>
> -----Original Message-----
> From: gpxe-bounces at etherboot.org [mailto:gpxe-bounces at etherboot.org] 
> On Behalf Of Marty Connor
> Sent: Saturday, November 20, 2010 2:55 PM
> To: Muralidhar.Appalla at emulex.com
> Cc: gpxe at etherboot.org; etherboot-developers at lists.sourceforge.net
> Subject: Re: [gPXE] Patch to gPXE-1.0.1 UNDI code for Interrupt Vs 
> Polling
>
> Hi Muralidhar,
>
> Your patch looks interesting, but I think you may have inverted the diff (new->old instead of old->new).  Can you verify this?
>
> Thanks,
>
> / Marty /
>
> On 11/18/10 3:03 PM, Muralidhar.Appalla at emulex.com wrote:
>> Hi,
>>
>> Problem Description: We have noticed that gPXE UNDI code is always 
>> expecting interrupt(s) to be generated by the underlying NIC HW to 
>> process the receive path in undinet_poll() routine. This is causing 
>> problem on some of the PCI NIC controllers where though the NIC 
>> device supports IRQ and able to generate interrupts,  during PXE UNDI 
>> code some devices do not enable interrupts on their device and run in 
>> polling mode. Those devices expect upper layer UNDI drivers to poll 
>> for the receive packets instead of waiting for interrupt from the 
>> device. But in undinet_poll() routine the code checks for ISR 
>> triggered or not if not it just returns. [(please check "if 
>> (undinet_isr_triggered)" statement ) in undinet_poll()]
>>
>> Solution Description: I have attached a patch here to take care of 
>> this so that gPXE UNDI code will work on both types of devices 
>> (interrupt&  polling supported devices). The patch is to check 
>> whether the device supports IRQ or not in "ServiceFlags" returned by 
>> the API call PXENV_UNDI_GET_IFACE_INFO in undinet_probe() routine. If 
>> the underlying device supports IRQ  then enable the newly introduced 
>> flag "irq_supported" in undi_nic structure. Later in  undinet_open()&
>> undinet_close() routines check this flag (irq_supported) to decide to 
>> hook the interrupts or to unhook the interrupts. Also in
>> undinet_poll() routine check whether interrupt got generated or not, 
>> only if the device supports interrupts otherwise get the packet from 
>> underlying UNDI NIC drivers by calling PXENV_UNDI_ISR using FuncFlag 
>> PXEENV_UNDI_ISR_GET_NEXT.
>>
>> I am attaching here the patch for review and acceptance.
>>
>> Muralidhar Appalla
>> Engineer Principal
>> 2560 N.First Street
>> Sanjose,CA
>> Emulex Corporation
>> (408)678-3472 (work)
>
> _______________________________________________
> gPXE mailing list
> gPXE at etherboot.org
> http://etherboot.org/mailman/listinfo/gpxe
>
>
> CONFIDENTIALITY AND SECURITY NOTICE
>
> The contents of this message and any attachments may be confidential and proprietary and also may be covered by the Electronic Communications Privacy Act. This message is not intended to be used by, and should not be relied upon in any way, by any third party.  If you are not an intended recipient, please inform the sender of the transmission error and delete this message immediately without reading, disseminating, distributing or copying the contents. Citadel makes no assurances that this e-mail and any attachments are free of viruses and other harmful code.