Date: Wed, 03 May 2006 19:04:15 -0700 From: Craig Leres <leres@ee.lbl.gov> To: freebsd-gnats-submit@FreeBSD.org Cc: brad@openbsd.org Subject: kern/96743: [sk] [patch] broken 32-bit register operations Message-ID: <200605040204.k4424FP9016553@fun.ee.lbl.gov> Resent-Message-ID: <200605040210.k442AJDW015482@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 96743 >Category: kern >Synopsis: [sk] [patch] broken 32-bit register operations >Confidential: no >Severity: non-critical >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu May 04 02:10:19 GMT 2006 >Closed-Date: >Last-Modified: >Originator: Craig Leres >Release: FreeBSD 6.1-BETA4 i386 >Organization: Lawrence Berkeley National Laboratory >Environment: fox 15 % uname -a FreeBSD fox.ee.lbl.gov 6.1-BETA4 FreeBSD 6.1-BETA4 #29: Wed May 3 17:45:44 PDT 2006 leres@fox.ee.lbl.gov:/usr/src/6.1-BETA4/sys/i386/compile/LBLSMP i386 SysKonnect SK-9843 gige adapter >Description: While attempting to apply a local patch that implements receive hardware timestamps (nice for research and intrusion detection applications) with the syskonnect SK-9843, I noticed that some 32-bit register operations are broken. Most of the registers, including XM_MODE and XM_TSTAMP_READ, must be accessed 16-bits at a time. One step in enabling timestamps is to set the XM_MODE_TIMESTAMP bit in the XM_MODE register but it was not possible to access the high bytes of the register. I developed my timestamp patch with 4.8-RELEASE using version 1.8.2.1 of if_skreg.h which defines SK_XM_READ_4() and SK_XM_WRITE_4() macros that use sk_win_read_2() and sk_win_read_2() to read and write 16 bits at a time, as required by the hardware. Version 1.8.2.2 of the include file change the macros to use sk_win_read_4() and sk_win_write_4() which only return the low order 16-bits. This problem probably wasn't detected before now because the current driver only seems to ever access bits in the low bytes of the registers (e.g. XM_MODE_RX_PROMISC in XM_MODE). >How-To-Repeat: This patch helps demonstrate the problem: ============================================================================== *** if_sk.c.virgin Wed May 3 17:56:30 2006 --- if_sk.c Wed May 3 17:56:37 2006 *************** *** 2532,2537 **** --- 2532,2540 ---- /* Save the XMAC II revision */ sc_if->sk_xmac_rev = XM_XMAC_REV(SK_XM_READ_4(sc_if, XM_DEVID)); + printf("sk%d: XM_DEVID 0x%x (0x%x/0x%x)\n", + sc_if->sk_unit, SK_XM_READ_4(sc_if, XM_DEVID), + SK_XM_READ_2(sc_if, XM_DEVID_HI), SK_XM_READ_2(sc_if, XM_DEVID_LO)); /* * Perform additional initialization for external PHYs, ============================================================================== After applying the patch and building and booting the kernel, the driver reports: skc0: <SysKonnect Gigabit Ethernet (V1.0)> port 0x3000-0x30ff mem 0xdd300000-0xd d303fff irq 48 at device 1.0 on pci3 skc0: SysKonnect SK-NET Gigabit Ethernet Adapter SK-9843 SX rev. (0x0) sk0: <XaQti Corp. XMAC II> on skc0 sk0: Ethernet address: 00:00:5a:9a:80:18 sk0: XM_DEVID 0xae20 (0xe0/0xae20) Notice that SK_XM_READ_4() returned the same value as the low bytes SK_XM_READ_2() and that the high bytes have 0xe0, not zero. >Fix: The appended patch (vs. 1.29.2.1 of if_skreg.h) re-enables the 2-byte versions if the SK_XM_READ_4() and SK_XM_WRITE_4() macros. It also protects the SK_XM_WRITE_4() macro. After booting the new kernel, the debugging printout shows the complete device id value: sk0: XM_DEVID 0xe0ae20 (0xe0/0xae20) It looks like this patch would also apply to the -current version (1.35). ============================================================================== *** if_skreg.h.virgin Wed May 3 18:28:19 2006 --- if_skreg.h Wed May 3 18:29:56 2006 *************** *** 1150,1156 **** #define SK_XMAC_REG(sc, reg) (((reg) * 2) + SK_XMAC1_BASE + \ (((sc)->sk_port) * (SK_XMAC2_BASE - SK_XMAC1_BASE))) ! #if 0 #define SK_XM_READ_4(sc, reg) \ ((sk_win_read_2(sc->sk_softc, \ SK_XMAC_REG(sc, reg)) & 0xFFFF) | \ --- 1150,1160 ---- #define SK_XMAC_REG(sc, reg) (((reg) * 2) + SK_XMAC1_BASE + \ (((sc)->sk_port) * (SK_XMAC2_BASE - SK_XMAC1_BASE))) ! /* ! * For at least some registers (e.g. XM_MODE) with at least some ! * boards (e.g. SK-9843) you can only read/write 16 bits at a time. ! */ ! #if 1 #define SK_XM_READ_4(sc, reg) \ ((sk_win_read_2(sc->sk_softc, \ SK_XMAC_REG(sc, reg)) & 0xFFFF) | \ *************** *** 1158,1167 **** SK_XMAC_REG(sc, reg + 2)) & 0xFFFF) << 16)) #define SK_XM_WRITE_4(sc, reg, val) \ ! sk_win_write_2(sc->sk_softc, SK_XMAC_REG(sc, reg), \ ! ((val) & 0xFFFF)); \ ! sk_win_write_2(sc->sk_softc, SK_XMAC_REG(sc, reg + 2), \ ! ((val) >> 16) & 0xFFFF) #else #define SK_XM_READ_4(sc, reg) \ sk_win_read_4(sc->sk_softc, SK_XMAC_REG(sc, reg)) --- 1162,1173 ---- SK_XMAC_REG(sc, reg + 2)) & 0xFFFF) << 16)) #define SK_XM_WRITE_4(sc, reg, val) \ ! do { \ ! sk_win_write_2(sc->sk_softc, SK_XMAC_REG(sc, reg), \ ! ((val) & 0xFFFF)); \ ! sk_win_write_2(sc->sk_softc, SK_XMAC_REG(sc, reg + 2), \ ! ((val) >> 16) & 0xFFFF); \ ! } while (0) #else #define SK_XM_READ_4(sc, reg) \ sk_win_read_4(sc->sk_softc, SK_XMAC_REG(sc, reg)) >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200605040204.k4424FP9016553>