Current-Users archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: iwn, wpa_supplicant, and wpa2 enterprise



On Thu June 3 2010 11:21:28 Sverre Froyen wrote:
> On Thu June 3 2010 10:32:32 Pouya D. Tafti wrote:
> ...
> 
> > uvm_fault(0xffffffff80ab21e0, 0xffff80200767a000, 1) -> e
> > fatal page fault in supervisor mode
> > trap type 6 code 0 rip ffffffff803ae51a cs 8 rflags 10286 cr2
> > ffff80200767a1a0 cpl 6 rsp ffff80004e1428f0
> > kernel: page fault trap, code=0
> > 
> > Stopped in pid 110.1 (wpa_supplicant) at        netbsd:kern_free+0x2d:
> >  movzwl  0
> > 
> > (%rbx),%r12d
> > db {3}> t
> > kern_free() at netbsd:kern_free+0x2d
> > iwn_init()  at netbsd:iwn_init+0x368
> > iwn_ioctl() at netbsd:iwn_ioctl+0xe4
> > ifioctl() at netbsd:ifioctl()+0x1e4
> > soo_ioctl() at netbsd:soo_ioctl+0x2a5
> > sys_ioctl() at netbsd:sys_ioctl+0xe9
> > syscall() at netbsd:syscall+0xaa
> 
> There are some recent patches to the OpenBSD driver that might be relevant.
> In particular:
> 
> ---------------------------------------------------------------------------
> ----------- Revision 1.94: download - view: text, markup, annotated -
> select for diffs Wed May 5 19:47:43 2010 UTC (4 weeks ago) by damien
>  Branches: MAIN
>  Diff to: previous 1.93: preferred, coloured
>  Changes since revision 1.93: +9 -0 lines
> 
> Prevent a process from entering iwn_ioctl while another process is
> tsleep'ing (for example waiting for the firmware to become alive)
> in iwn_init.
> 
> I believe this might fix a crash reported by dhill@
> This is a temporary fix until I find something better that I will
> apply to my other drivers that can tsleep in if_init (wpi, run etc...)
> ---------------------------------------------------------------------------
> -----------
> 

I've attached a patch that updates the iwn driver with the latest OpenBSD 
changes. I suspect it will fix the reported uvm_fault and perhaps it will help 
with the fatal firmware error (the changes include updates to the firmware 
handling).

The new code does, however, prevent my laptop from booting cleanly.  Here is 
what I think might be happening:

"wpa_supplicant -B -i iwn0" starts and returns immediately (before the 
interface has associated).

"dhcpcd iwn0" starts and attempts to use the interface (which is still not 
ready). dhcpcd fails.

I see a "iwn0: cannot assign link-local address" message on the console, which 
could be from a dhcpcd triggered iwn_ioctl call that fails because of the rev 
1.94 change, thereby causing a dhcpcd failure.

If, this is correct, delaying the call to dhcpcd until the interface is 
associated would be a fix. Perhaps wpa_supplicant could sprout an option not to 
return until that is the case :-)

Please test the patch. I suggest that we hold off committing it until the boot 
issue is resolved.

Regards,
Sverre
Index: sys/dev/pci/if_iwn.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_iwn.c,v
retrieving revision 1.45
diff -u -p -r1.45 if_iwn.c
--- sys/dev/pci/if_iwn.c        12 May 2010 12:26:16 -0000      1.45
+++ sys/dev/pci/if_iwn.c        4 Jun 2010 01:06:33 -0000
@@ -1,5 +1,5 @@
 /*     $NetBSD: if_iwn.c,v 1.45 2010/05/12 12:26:16 christos Exp $     */
-/*     $OpenBSD: if_iwn.c,v 1.88 2010/04/10 08:37:36 damien Exp $      */
+/*     $OpenBSD: if_iwn.c,v 1.96 2010/05/13 09:25:03 damien Exp $      */
 
 /*-
  * Copyright (c) 2007-2010 Damien Bergamini 
<damien.bergamini%free.fr@localhost>
@@ -32,7 +32,7 @@ __KERNEL_RCSID(0, "$NetBSD: if_iwn.c,v 1
 
 #include <sys/param.h>
 #include <sys/sockio.h>
-#include <sys/sysctl.h>
+#include <sys/proc.h>
 #include <sys/mbuf.h>
 #include <sys/kernel.h>
 #include <sys/socket.h>
@@ -96,6 +96,19 @@ static const pci_product_id_t iwn_device
        PCI_PRODUCT_INTEL_WIFI_LINK_6050_2X2_2,
        PCI_PRODUCT_INTEL_WIFI_LINK_6005_2X2_1,
        PCI_PRODUCT_INTEL_WIFI_LINK_6005_2X2_2,
+#ifdef notyet
+       /*
+        * XXX NetBSD: the 6005A replaces the two 6005, above
+        * (see OpenBSD rev 1.96).
+        */
+       PCI_PRODUCT_INTEL_WIFI_LINK_6005A_2X2_1,
+       PCI_PRODUCT_INTEL_WIFI_LINK_6005A_2X2_2,
+       PCI_PRODUCT_INTEL_WIFI_LINK_6005B_1X1_1,
+       PCI_PRODUCT_INTEL_WIFI_LINK_6005B_1X1_2,
+       PCI_PRODUCT_INTEL_WIFI_LINK_6005B_2X2_1,
+       PCI_PRODUCT_INTEL_WIFI_LINK_6005B_2X2_2,
+       PCI_PRODUCT_INTEL_WIFI_LINK_6005B_2X2_3
+#endif
 };
 
 /*
@@ -263,6 +276,10 @@ static int iwn4965_load_bootcode(struct 
 static int     iwn4965_load_firmware(struct iwn_softc *);
 static int     iwn5000_load_firmware_section(struct iwn_softc *, uint32_t,
                    const uint8_t *, int);
+static int     iwn_read_firmware_leg(struct iwn_softc *,
+                   struct iwn_fw_info *);
+static int     iwn_read_firmware_tlv(struct iwn_softc *,
+                   struct iwn_fw_info *, uint16_t);
 static int     iwn5000_load_firmware(struct iwn_softc *);
 static int     iwn_read_firmware(struct iwn_softc *);
 static int     iwn_clock_wait(struct iwn_softc *);
@@ -3144,6 +3161,16 @@ iwn_ioctl(struct ifnet *ifp, u_long cmd,
        int s, error = 0;
 
        s = splnet();
+       /*
+        * Prevent processes from entering this function while another
+        * process is tsleep'ing in it.
+        */
+       if (sc->sc_flags & IWN_FLAG_BUSY) {
+               splx(s);
+               return EBUSY;
+       }
+
+       sc->sc_flags |= IWN_FLAG_BUSY;
 
        switch (cmd) {
        case SIOCSIFADDR:
@@ -3190,6 +3217,8 @@ iwn_ioctl(struct ifnet *ifp, u_long cmd,
                        error = iwn_init(ifp);
                }
        }
+
+       sc->sc_flags &= ~IWN_FLAG_BUSY;
        splx(s);
        return error;
 }
@@ -4296,7 +4325,7 @@ iwn_scan(struct iwn_softc *sc, uint16_t 
        tx->lifetime = htole32(IWN_LIFETIME_INFINITE);
 
        if (flags & IEEE80211_CHAN_5GHZ) {
-               hdr->crc_threshold = htole16(1);
+               hdr->crc_threshold = 0xffff;
                /* Send probe requests at 6Mbps. */
                tx->plcp = iwn_rates[IWN_RIDX_OFDM6].plcp;
                rs = &ic->ic_sup_rates[IEEE80211_MODE_11A];
@@ -4334,6 +4363,10 @@ iwn_scan(struct iwn_softc *sc, uint16_t 
        frm = (uint8_t *)(wh + 1);
        frm = ieee80211_add_ssid(frm, NULL, 0);
        frm = ieee80211_add_rates(frm, rs);
+#ifndef IEEE80211_NO_HT
+       if (ic->ic_flags & IEEE80211_F_HTON)
+               frm = ieee80211_add_htcaps(frm, ic);
+#endif
        if (rs->rs_nrates > IEEE80211_RATE_SIZE)
                frm = ieee80211_add_xrates(frm, rs);
 
@@ -5242,20 +5275,155 @@ iwn5000_load_firmware(struct iwn_softc *
        return 0;
 }
 
+/*
+ * Extract text and data sections from a legacy firmware image.
+ */
+static int
+iwn_read_firmware_leg(struct iwn_softc *sc, struct iwn_fw_info *fw)
+{
+       const uint32_t *ptr;
+       size_t hdrlen = 24;
+       uint32_t rev;
+
+       ptr = (const uint32_t *)fw->data;
+       rev = le32toh(*ptr++);
+
+       /* Check firmware API version. */
+       if (IWN_FW_API(rev) <= 1) {
+               aprint_error_dev(sc->sc_dev,
+                   "bad firmware, need API version >=2\n");
+               return EINVAL;
+       }
+       if (IWN_FW_API(rev) >= 3) {
+               /* Skip build number (version 2 header). */
+               hdrlen += 4;
+               ptr++;
+       }
+       if (fw->size < hdrlen) {
+               aprint_error_dev(sc->sc_dev,
+                   "firmware too short: %zd bytes\n", fw->size);
+               return EINVAL;
+       }
+       fw->main.textsz = le32toh(*ptr++);
+       fw->main.datasz = le32toh(*ptr++);
+       fw->init.textsz = le32toh(*ptr++);
+       fw->init.datasz = le32toh(*ptr++);
+       fw->boot.textsz = le32toh(*ptr++);
+
+       /* Check that all firmware sections fit. */
+       if (fw->size < hdrlen + fw->main.textsz + fw->main.datasz +
+           fw->init.textsz + fw->init.datasz + fw->boot.textsz) {
+               aprint_error_dev(sc->sc_dev,
+                   "firmware too short: %zd bytes\n", fw->size);
+               return EINVAL;
+       }
+
+       /* Get pointers to firmware sections. */
+       fw->main.text = (const uint8_t *)ptr;
+       fw->main.data = fw->main.text + fw->main.textsz;
+       fw->init.text = fw->main.data + fw->main.datasz;
+       fw->init.data = fw->init.text + fw->init.textsz;
+       fw->boot.text = fw->init.data + fw->init.datasz;
+       return 0;
+}
+
+/*
+ * Extract text and data sections from a TLV firmware image.
+ */
+static int
+iwn_read_firmware_tlv(struct iwn_softc *sc, struct iwn_fw_info *fw,
+    uint16_t alt)
+{
+       const struct iwn_fw_tlv_hdr *hdr;
+       const struct iwn_fw_tlv *tlv;
+       const uint8_t *ptr, *end;
+       uint64_t altmask;
+       uint32_t len;
+
+       if (fw->size < sizeof (*hdr)) {
+               aprint_error_dev(sc->sc_dev,
+                   "firmware too short: %zd bytes\n", fw->size);
+               return EINVAL;
+       }
+       hdr = (const struct iwn_fw_tlv_hdr *)fw->data;
+       if (hdr->signature != htole32(IWN_FW_SIGNATURE)) {
+               aprint_error_dev(sc->sc_dev,
+                   "bad firmware signature 0x%08x\n", le32toh(hdr->signature));
+               return EINVAL;
+       }
+       DPRINTF(("FW: \"%.64s\", build 0x%x\n", hdr->descr,
+           le32toh(hdr->build)));
+
+       /*
+        * Select the closest supported alternative that is less than
+        * or equal to the specified one.
+        */
+       altmask = le64toh(hdr->altmask);
+       while (alt > 0 && !(altmask & (1ULL << alt)))
+               alt--;  /* Downgrade. */
+       DPRINTF(("using alternative %d\n", alt));
+
+       ptr = (const uint8_t *)(hdr + 1);
+       end = (const uint8_t *)(fw->data + fw->size);
+
+       /* Parse type-length-value fields. */
+       while (ptr + sizeof (*tlv) <= end) {
+               tlv = (const struct iwn_fw_tlv *)ptr;
+               len = le32toh(tlv->len);
+
+               ptr += sizeof (*tlv);
+               if (ptr + len > end) {
+                       aprint_error_dev(sc->sc_dev,
+                           "firmware too short: %zd bytes\n", fw->size);
+                       return EINVAL;
+               }
+               /* Skip other alternatives. */
+               if (tlv->alt != 0 && tlv->alt != htole16(alt))
+                       goto next;
+
+               switch (le16toh(tlv->type)) {
+               case IWN_FW_TLV_MAIN_TEXT:
+                       fw->main.text = ptr;
+                       fw->main.textsz = len;
+                       break;
+               case IWN_FW_TLV_MAIN_DATA:
+                       fw->main.data = ptr;
+                       fw->main.datasz = len;
+                       break;
+               case IWN_FW_TLV_INIT_TEXT:
+                       fw->init.text = ptr;
+                       fw->init.textsz = len;
+                       break;
+               case IWN_FW_TLV_INIT_DATA:
+                       fw->init.data = ptr;
+                       fw->init.datasz = len;
+                       break;
+               case IWN_FW_TLV_BOOT_TEXT:
+                       fw->boot.text = ptr;
+                       fw->boot.textsz = len;
+                       break;
+               default:
+                       DPRINTF(("TLV type %d not handled\n",
+                           le16toh(tlv->type)));
+                       break;
+               }
+ next:         /* TLV fields are 32-bit aligned. */
+               ptr += (len + 3) & ~3;
+       }
+       return 0;
+}
+
 static int
 iwn_read_firmware(struct iwn_softc *sc)
 {
        const struct iwn_hal *hal = sc->sc_hal;
        struct iwn_fw_info *fw = &sc->fw;
        firmware_handle_t fwh;
-       const uint32_t *ptr;
-       uint32_t rev;
-       size_t size;
        int error;
 
        /* Initialize for error returns */
        fw->data = NULL;
-       fw->datasz = 0;
+       fw->size = 0;
 
        /* Open firmware image. */
        if ((error = firmware_open("if_iwn", sc->fwname, &fwh)) != 0) {
@@ -5263,53 +5431,42 @@ iwn_read_firmware(struct iwn_softc *sc)
                    "could not get firmware handle %s\n", sc->fwname);
                return error;
        }
-       size = firmware_get_size(fwh);
-       if (size < 28) {
+       fw->size = firmware_get_size(fwh);
+       if (fw->size < sizeof (uint32_t)) {
                aprint_error_dev(sc->sc_dev,
-                   "truncated firmware header: %zu bytes\n", size);
+                   "firmware too short: %zd bytes\n", fw->size);
                firmware_close(fwh);
                return EINVAL;
        }
 
        /* Read the firmware. */
-       fw->data = firmware_malloc(size);
+       fw->data = firmware_malloc(fw->size);
        if (fw->data == NULL) {
                aprint_error_dev(sc->sc_dev,
                    "not enough memory to stock firmware %s\n", sc->fwname);
                firmware_close(fwh);
                return ENOMEM;
        }
-       error = firmware_read(fwh, 0, fw->data, size);
+       error = firmware_read(fwh, 0, fw->data, fw->size);
        firmware_close(fwh);
-       fw->datasz = size;
        if (error != 0) {
                aprint_error_dev(sc->sc_dev,
                    "could not read firmware %s\n", sc->fwname);
                goto out;
        }
 
-       /* Process firmware header. */
-       ptr = (const uint32_t *)fw->data;
-       rev = le32toh(*ptr++);
-       /* Check firmware API version. */
-       if (IWN_FW_API(rev) <= 1) {
+       /* Retrieve text and data sections. */
+       if (*(const uint32_t *)fw->data != 0)   /* Legacy image. */
+               error = iwn_read_firmware_leg(sc, fw);
+       else
+               error = iwn_read_firmware_tlv(sc, fw, 1);
+       if (error != 0) {
                aprint_error_dev(sc->sc_dev,
-                   "bad firmware, need API version >=2\n");
+                   "could not read firmware sections\n");
                goto out;
        }
-       if (IWN_FW_API(rev) >= 3) {
-               /* Skip build number (version 2 header). */
-               size -= 4;
-               ptr++;
-       }
-       fw->main.textsz = le32toh(*ptr++);
-       fw->main.datasz = le32toh(*ptr++);
-       fw->init.textsz = le32toh(*ptr++);
-       fw->init.datasz = le32toh(*ptr++);
-       fw->boot.textsz = le32toh(*ptr++);
-       size -= 24;
 
-       /* Sanity-check firmware header. */
+       /* Make sure text and data sections fit in hardware memory. */
        if (fw->main.textsz > hal->fw_text_maxsz ||
            fw->main.datasz > hal->fw_data_maxsz ||
            fw->init.textsz > hal->fw_text_maxsz ||
@@ -5317,30 +5474,16 @@ iwn_read_firmware(struct iwn_softc *sc)
            fw->boot.textsz > IWN_FW_BOOT_TEXT_MAXSZ ||
            (fw->boot.textsz & 3) != 0) {
                aprint_error_dev(sc->sc_dev,
-                   "invalid firmware header\n");
-               goto out;
-       }
-
-       /* Check that all firmware sections fit. */
-       if (fw->main.textsz + fw->main.datasz + fw->init.textsz +
-           fw->init.datasz + fw->boot.textsz > size) {
-               aprint_error_dev(sc->sc_dev,
-                   "firmware file too short: %zu bytes\n", size);
+                   "firmware sections too large\n");
                goto out;
        }
 
-       /* Get pointers to firmware sections. */
-       fw->main.text = (const uint8_t *)ptr;
-       fw->main.data = fw->main.text + fw->main.textsz;
-       fw->init.text = fw->main.data + fw->main.datasz;
-       fw->init.data = fw->init.text + fw->init.textsz;
-       fw->boot.text = fw->init.data + fw->init.datasz;
-
+       /* We can proceed with loading the firmware. */
        return 0;
 out:
-       firmware_free(fw->data, fw->datasz);
+       firmware_free(fw->data, fw->size);
        fw->data = NULL;
-       fw->datasz = 0;
+       fw->size = 0;
        return error ? error : EINVAL;
 }
 
@@ -5733,11 +5876,11 @@ iwn_init(struct ifnet *ifp)
        sc->sc_flags &= ~IWN_FLAG_USE_ICT;
 
        /* Initialize hardware and upload firmware. */
-       KASSERT(sc->fw.data != NULL && sc->fw.datasz > 0);
+       KASSERT(sc->fw.data != NULL && sc->fw.size > 0);
        error = iwn_hw_init(sc);
-       firmware_free(sc->fw.data, sc->fw.datasz);
+       firmware_free(sc->fw.data, sc->fw.size);
        sc->fw.data = NULL;
-       sc->fw.datasz = 0;
+       sc->fw.size = 0;
        if (error != 0) {
                aprint_error_dev(sc->sc_dev,
                    "could not initialize hardware\n");
Index: sys/dev/pci/if_iwnreg.h
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_iwnreg.h,v
retrieving revision 1.7
diff -u -p -r1.7 if_iwnreg.h
--- sys/dev/pci/if_iwnreg.h     16 Apr 2010 01:40:41 -0000      1.7
+++ sys/dev/pci/if_iwnreg.h     4 Jun 2010 01:06:33 -0000
@@ -1,5 +1,5 @@
 /*     $NetBSD: if_iwnreg.h,v 1.7 2010/04/16 01:40:41 christos Exp $   */
-/*     $OpenBSD: if_iwnreg.h,v 1.38 2010/04/10 08:37:36 damien Exp $   */
+/*     $OpenBSD: if_iwnreg.h,v 1.40 2010/05/05 19:41:57 damien Exp $   */
 
 /*-
  * Copyright (c) 2007, 2008
@@ -1253,6 +1253,34 @@ struct iwn_fw_dump {
        uint32_t        time[2];
 } __packed;
 
+/* TLV firmware header. */
+struct iwn_fw_tlv_hdr {
+       uint32_t        zero;   /* Always 0, to differentiate from legacy. */
+       uint32_t        signature;
+#define IWN_FW_SIGNATURE       0x0a4c5749      /* "IWL\n" */
+
+       uint8_t         descr[64];
+       uint32_t        rev;
+#define IWN_FW_API(x)  (((x) >> 8) & 0xff)
+
+       uint32_t        build;
+       uint64_t        altmask;
+} __packed;
+
+/* TLV header. */
+struct iwn_fw_tlv {
+       uint16_t        type;
+#define IWN_FW_TLV_MAIN_TEXT           1
+#define IWN_FW_TLV_MAIN_DATA           2
+#define IWN_FW_TLV_INIT_TEXT           3
+#define IWN_FW_TLV_INIT_DATA           4
+#define IWN_FW_TLV_BOOT_TEXT           5
+#define IWN_FW_TLV_PBREQ_MAXLEN                6
+
+       uint16_t        alt;
+       uint32_t        len;
+} __packed;
+
 #define IWN4965_FW_TEXT_MAXSZ  ( 96 * 1024)
 #define IWN4965_FW_DATA_MAXSZ  ( 40 * 1024)
 #define IWN5000_FW_TEXT_MAXSZ  (256 * 1024)
@@ -1261,8 +1289,6 @@ struct iwn_fw_dump {
 #define IWN4965_FWSZ           (IWN4965_FW_TEXT_MAXSZ + IWN4965_FW_DATA_MAXSZ)
 #define IWN5000_FWSZ           IWN5000_FW_TEXT_MAXSZ
 
-#define IWN_FW_API(x)  (((x) >> 8) & 0xff)
-
 /*
  * Offsets into EEPROM.
  */
Index: sys/dev/pci/if_iwnvar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_iwnvar.h,v
retrieving revision 1.10
diff -u -p -r1.10 if_iwnvar.h
--- sys/dev/pci/if_iwnvar.h     23 Apr 2010 20:56:20 -0000      1.10
+++ sys/dev/pci/if_iwnvar.h     4 Jun 2010 01:06:33 -0000
@@ -1,5 +1,5 @@
 /*     $NetBSD: if_iwnvar.h,v 1.10 2010/04/23 20:56:20 christos Exp $  */
-/*     $OpenBSD: if_iwnvar.h,v 1.17 2010/02/17 18:23:00 damien Exp $   */
+/*     $OpenBSD: if_iwnvar.h,v 1.19 2010/05/05 19:47:43 damien Exp $   */
 
 /*-
  * Copyright (c) 2007, 2008
@@ -168,7 +168,7 @@ struct iwn_fw_part {
 
 struct iwn_fw_info {
        u_char                  *data;
-       uint32_t                datasz;
+       size_t                  size;
        struct iwn_fw_part      init;
        struct iwn_fw_part      main;
        struct iwn_fw_part      boot;
@@ -226,6 +226,7 @@ struct iwn_softc {
 #define IWN_FLAG_CALIB_DONE    (1 << 2)
 #define IWN_FLAG_USE_ICT       (1 << 3)
 #define IWN_FLAG_INTERNAL_PA   (1 << 4)
+#define IWN_FLAG_BUSY          (1 << 5)
 /* Added for NetBSD */
 #define IWN_FLAG_SCANNING      (1 << 8)
 


Home | Main Index | Thread Index | Old Index