From 375fa0aa41b95aca6b081230c5f17725b9dcc893 Mon Sep 17 00:00:00 2001 From: jhb Date: Mon, 11 Sep 2006 20:52:41 +0000 Subject: Minor overhaul of SMBus support: - Change smbus_callback() to pass a void * rather than caddr_t. - Change smbus_bread() to pass a pointer to the count and have it be an in/out parameter. The input is the size of the buffer (same as before), but on return it will contain the actual amount of data read back from the bus. Note that this value may be larger than the input value. It is up to the caller to treat this as an error if desired. - Change the SMB_BREAD ioctl to write out the updated struct smbcmd which will contain the actual number of bytes read in the 'count' field. To preserve the previous ABI, the old ioctl value is mapped to SMB_OLD_BREAD which doesn't copy the updated smbcmd back out to userland. I doubt anyone actually used the old BREAD anyway as it was rediculous to do a bulk-read but not tell the using program how much data was actually read. - Make the smbus driver and devclass public in the smbus module and push all the DRIVER_MODULE()'s for attaching the smbus driver to various foosmb drivers out into the foosmb modules. This makes all the foosmb logic centralized and allows new foosmb modules to be self-contained w/o having to hack smbus.c everytime a new smbus driver is added. - Add a new SMB_EINVAL error bit and use it in place of EINVAL to return an error for bad arguments (such as invalid counts for bread and bwrite). - Map SMB bus error bits to EIO in smbus_error(). - Make the smbus driver call bus_generic_probe() and require child drivers such as smb(4) to create device_t's via identify routines. Previously, smbus just created one anonymous device during attach, and if you had multiple drivers that could attach it was just random chance as to which driver got to probe for the sole device_t first. - Add a mutex to the smbus(4) softc and use it in place of dummy splhigh() to protect the 'owner' field and perform necessary synchronization for smbus_request_bus() and smbus_release_bus(). - Change the bread() and bwrite() methods of alpm(4), amdpm(4), and viapm(4) to only perform a single transaction and not try to use a loop of multiple transactions for a large request. The framing and commands to use for a large transaction depend on the upper-layer protocol (such as SSIF for IPMI over SMBus) from what I can tell, and the smb(4) driver never allowed bulk read/writes of more than 32-bytes anyway. The other smb drivers only performed single transactions. - Fix buffer overflows in the bread() methods of ichsmb(4), alpm(4), amdpm(4), amdsmb(4), intpm(4), and nfsmb(4). - Use SMB_xxx errors in viapm(4). - Destroy ichsmb(4)'s mutex after bus_generic_detach() to avoid problems from child devices making smb upcalls that would use the mutex during their detach methods. MFC after: 1 week Reviewed by: jmg (mostly) --- sys/pci/alpm.c | 87 ++++++++++++++++++++++------------------------ sys/pci/amdpm.c | 95 ++++++++++++++++++++++++-------------------------- sys/pci/amdsmb.c | 29 ++++++++++------ sys/pci/intpm.c | 30 +++++++++------- sys/pci/nfsmb.c | 29 ++++++++++------ sys/pci/viapm.c | 103 ++++++++++++++++++++++++++----------------------------- 6 files changed, 188 insertions(+), 185 deletions(-) (limited to 'sys/pci') diff --git a/sys/pci/alpm.c b/sys/pci/alpm.c index e6f8149..d90d919 100644 --- a/sys/pci/alpm.c +++ b/sys/pci/alpm.c @@ -259,7 +259,7 @@ alpm_detach(device_t dev) } static int -alpm_callback(device_t dev, int index, caddr_t *data) +alpm_callback(device_t dev, int index, void *data) { int error = 0; @@ -525,82 +525,76 @@ static int alpm_bwrite(device_t dev, u_char slave, char cmd, u_char count, char *buf) { struct alpm_softc *sc = (struct alpm_softc *)device_get_softc(dev); - u_char remain, len, i; - int error = SMB_ENOERR; + u_char i; + int error; + if (count < 1 || count > 32) + return (SMB_EINVAL); alpm_clear(sc); if(!alpm_idle(sc)) return (SMB_EBUSY); - remain = count; - while (remain) { - len = min(remain, 32); - - ALPM_SMBOUTB(sc, SMBHADDR, slave & ~LSB); + ALPM_SMBOUTB(sc, SMBHADDR, slave & ~LSB); - /* set the cmd and reset the - * 32-byte long internal buffer */ - ALPM_SMBOUTB(sc, SMBCMD, SMBWRBLOCK | SMB_BLK_CLR); - - ALPM_SMBOUTB(sc, SMBHDATA, len); + /* set the cmd and reset the + * 32-byte long internal buffer */ + ALPM_SMBOUTB(sc, SMBCMD, SMBWRBLOCK | SMB_BLK_CLR); - /* fill the 32-byte internal buffer */ - for (i=0; i 32) + return (SMB_EINVAL); alpm_clear(sc); if (!alpm_idle(sc)) return (SMB_EBUSY); - remain = count; - while (remain) { - ALPM_SMBOUTB(sc, SMBHADDR, slave | LSB); + ALPM_SMBOUTB(sc, SMBHADDR, slave | LSB); - /* set the cmd and reset the - * 32-byte long internal buffer */ - ALPM_SMBOUTB(sc, SMBCMD, SMBWRBLOCK | SMB_BLK_CLR); + /* set the cmd and reset the + * 32-byte long internal buffer */ + ALPM_SMBOUTB(sc, SMBCMD, SMBWRBLOCK | SMB_BLK_CLR); - ALPM_SMBOUTB(sc, SMBHCMD, cmd); - ALPM_SMBOUTB(sc, SMBSTART, 0xff); + ALPM_SMBOUTB(sc, SMBHCMD, cmd); + ALPM_SMBOUTB(sc, SMBSTART, 0xff); - if ((error = alpm_wait(sc)) != SMB_ENOERR) + if ((error = alpm_wait(sc)) != SMB_ENOERR) goto error; - len = ALPM_SMBINB(sc, SMBHDATA); + len = ALPM_SMBINB(sc, SMBHDATA); - /* read the 32-byte internal buffer */ - for (i=0; i 32) + return (SMB_EINVAL); amdpm_clear(sc); if(!amdpm_idle(sc)) return (SMB_EBUSY); - remain = count; - while (remain) { - len = min(remain, 32); - - AMDPM_SMBOUTW(sc, AMDSMB_HSTADDR, slave & ~LSB); + AMDPM_SMBOUTW(sc, AMDSMB_HSTADDR, slave & ~LSB); - /* - * Do we have to reset the internal 32-byte buffer? - * Can't see how to do this from the data sheet. - */ - - AMDPM_SMBOUTW(sc, AMDSMB_HSTDATA, len); - - /* Fill the 32-byte internal buffer */ - for (i=0; i 32) + return (SMB_EINVAL); amdpm_clear(sc); if (!amdpm_idle(sc)) return (SMB_EBUSY); - remain = count; - while (remain) { - AMDPM_SMBOUTW(sc, AMDSMB_HSTADDR, slave | LSB); + AMDPM_SMBOUTW(sc, AMDSMB_HSTADDR, slave | LSB); - AMDPM_SMBOUTB(sc, AMDSMB_HSTCMD, cmd); + AMDPM_SMBOUTB(sc, AMDSMB_HSTCMD, cmd); - l = AMDPM_SMBINW(sc, AMDSMB_GLOBAL_ENABLE); - AMDPM_SMBOUTW(sc, AMDSMB_GLOBAL_ENABLE, (l & 0xfff8) | AMDSMB_GE_CYC_BLOCK | AMDSMB_GE_HOST_STC); + l = AMDPM_SMBINW(sc, AMDSMB_GLOBAL_ENABLE); + AMDPM_SMBOUTW(sc, AMDSMB_GLOBAL_ENABLE, + (l & 0xfff8) | AMDSMB_GE_CYC_BLOCK | AMDSMB_GE_HOST_STC); - if ((error = amdpm_wait(sc)) != SMB_ENOERR) - goto error; - - len = AMDPM_SMBINW(sc, AMDSMB_HSTDATA); + if ((error = amdpm_wait(sc)) != SMB_ENOERR) + goto error; - /* Read the 32-byte internal buffer */ - for (i=0; i 32) + return (SMB_EINVAL); amdsmb_ec_write(sc, SMB_CMD, cmd); - amdsmb_ec_write(sc, SMB_BCNT, len); - for (i = 0; i < len; i++) + amdsmb_ec_write(sc, SMB_BCNT, count); + for (i = 0; i < count; i++) amdsmb_ec_write(sc, SMB_DATA + i, buf[i]); amdsmb_ec_write(sc, SMB_ADDR, slave); amdsmb_ec_write(sc, SMB_PRTCL, SMB_PRTCL_WRITE | SMB_PRTCL_BLOCK_DATA); @@ -464,25 +465,30 @@ amdsmb_bwrite(device_t dev, u_char slave, char cmd, u_char count, char *buf) } static int -amdsmb_bread(device_t dev, u_char slave, char cmd, u_char count, char *buf) +amdsmb_bread(device_t dev, u_char slave, char cmd, u_char *count, char *buf) { struct amdsmb_softc *sc = (struct amdsmb_softc *)device_get_softc(dev); - u_char len, i; + u_char data, len, i; int error; + if (*count < 1 || *count > 32) + return (SMB_EINVAL); amdsmb_ec_write(sc, SMB_CMD, cmd); amdsmb_ec_write(sc, SMB_ADDR, slave); amdsmb_ec_write(sc, SMB_PRTCL, SMB_PRTCL_READ | SMB_PRTCL_BLOCK_DATA); if ((error = amdsmb_wait(sc)) == SMB_ENOERR) { amdsmb_ec_read(sc, SMB_BCNT, &len); - len = min(len, 32); - for (i = 0; i < len; i++) - amdsmb_ec_read(sc, SMB_DATA + i, buf + i); + for (i = 0; i < len; i++) { + amdsmb_ec_read(sc, SMB_DATA + i, &data); + if (i < *count) + buf[i] = data; + } + *count = len; } AMDSMB_DEBUG(printf("amdsmb: READBLK to 0x%x, count=0x%x, cmd=0x%x, " - "error=0x%x", slave, count, cmd, error)); + "error=0x%x", slave, *count, cmd, error)); return (error); } @@ -517,6 +523,7 @@ static driver_t amdsmb_driver = { }; DRIVER_MODULE(amdsmb, pci, amdsmb_driver, amdsmb_devclass, 0, 0); +DRIVER_MODULE(smbus, amdsmb, smbus_driver, smbus_devclass, 0, 0); MODULE_DEPEND(amdsmb, pci, 1, 1, 1); MODULE_DEPEND(amdsmb, smbus, SMBUS_MINVER, SMBUS_PREFVER, SMBUS_MAXVER); diff --git a/sys/pci/intpm.c b/sys/pci/intpm.c index 1e454bb..3bf1c2c 100644 --- a/sys/pci/intpm.c +++ b/sys/pci/intpm.c @@ -71,7 +71,7 @@ static int intsmb_attach(device_t); static int intsmb_intr(device_t dev); static int intsmb_slvintr(device_t dev); static void intsmb_alrintr(device_t dev); -static int intsmb_callback(device_t dev, int index, caddr_t data); +static int intsmb_callback(device_t dev, int index, void *data); static int intsmb_quick(device_t dev, u_char slave, int how); static int intsmb_sendb(device_t dev, u_char slave, char byte); static int intsmb_recvb(device_t dev, u_char slave, char *byte); @@ -81,7 +81,7 @@ static int intsmb_readb(device_t dev, u_char slave, char cmd, char *byte); static int intsmb_readw(device_t dev, u_char slave, char cmd, short *word); static int intsmb_pcall(device_t dev, u_char slave, char cmd, short sdata, short *rdata); static int intsmb_bwrite(device_t dev, u_char slave, char cmd, u_char count, char *buf); -static int intsmb_bread(device_t dev, u_char slave, char cmd, u_char count, char *buf); +static int intsmb_bread(device_t dev, u_char slave, char cmd, u_char *count, char *buf); static void intsmb_start(device_t dev,u_char cmd,int nointr); static int intsmb_stop(device_t dev); static int intsmb_stop_poll(device_t dev); @@ -175,7 +175,7 @@ intsmb_attach(device_t dev) } static int -intsmb_callback(device_t dev, int index, caddr_t data) +intsmb_callback(device_t dev, int index, void *data) { int error = 0; intrmask_t s; @@ -585,7 +585,7 @@ intsmb_bwrite(device_t dev, u_char slave, char cmd, u_char count, char *buf) struct intsmb_softc *sc = (struct intsmb_softc *)device_get_softc(dev); error=intsmb_free(dev); if(count>SMBBLOCKTRANS_MAX||count==0) - error=EINVAL; + error=SMB_EINVAL; if(!error){ /*Reset internal array index*/ bus_space_read_1(sc->st,sc->sh,PIIX4_SMBHSTCNT); @@ -603,32 +603,35 @@ intsmb_bwrite(device_t dev, u_char slave, char cmd, u_char count, char *buf) } static int -intsmb_bread(device_t dev, u_char slave, char cmd, u_char count, char *buf) +intsmb_bread(device_t dev, u_char slave, char cmd, u_char *count, char *buf) { int error,i; + u_char data, nread; struct intsmb_softc *sc = (struct intsmb_softc *)device_get_softc(dev); error=intsmb_free(dev); - if(count>SMBBLOCKTRANS_MAX||count==0) - error=EINVAL; + if(*count>SMBBLOCKTRANS_MAX||*count==0) + error=SMB_EINVAL; if(!error){ /*Reset internal array index*/ bus_space_read_1(sc->st,sc->sh,PIIX4_SMBHSTCNT); bus_space_write_1(sc->st,sc->sh,PIIX4_SMBHSTADD,slave|LSB); bus_space_write_1(sc->st,sc->sh,PIIX4_SMBHSTCMD,cmd); - bus_space_write_1(sc->st,sc->sh,PIIX4_SMBHSTDAT0,count); + bus_space_write_1(sc->st,sc->sh,PIIX4_SMBHSTDAT0,*count); intsmb_start(dev,PIIX4_SMBHSTCNT_PROT_BLOCK,0); error=intsmb_stop(dev); if(!error){ - bzero(buf,count);/*Is it needed?*/ - count= bus_space_read_1(sc->st,sc->sh, + nread= bus_space_read_1(sc->st,sc->sh, PIIX4_SMBHSTDAT0); - if(count!=0&&count<=SMBBLOCKTRANS_MAX){ - for(i=0;ist, + if(nread!=0&&nread<=SMBBLOCKTRANS_MAX){ + for(i=0;ist, sc->sh, PIIX4_SMBBLKDAT); + if (i < *count) + buf[i] = data; } + *count = nread; } else{ error=EIO; @@ -739,6 +742,7 @@ intpm_probe(device_t dev) } } DRIVER_MODULE(intpm, pci , intpm_pci_driver, intpm_devclass, 0, 0); +DRIVER_MODULE(smbus, intsmb, smbus_driver, smbus_devclass, 0, 0); MODULE_DEPEND(intpm, smbus, SMBUS_MINVER, SMBUS_PREFVER, SMBUS_MAXVER); MODULE_VERSION(intpm, 1); diff --git a/sys/pci/nfsmb.c b/sys/pci/nfsmb.c index 5cd1b08..1511121 100644 --- a/sys/pci/nfsmb.c +++ b/sys/pci/nfsmb.c @@ -252,7 +252,7 @@ nfsmb_detach(device_t dev) } static int -nfsmb_callback(device_t dev, int index, caddr_t *data) +nfsmb_callback(device_t dev, int index, void *data) { int error = 0; @@ -456,13 +456,14 @@ static int nfsmb_bwrite(device_t dev, u_char slave, char cmd, u_char count, char *buf) { struct nfsmb_softc *sc = (struct nfsmb_softc *)device_get_softc(dev); - u_char len, i; + u_char i; int error; - len = min(count, 32); + if (count < 1 || count > 32) + return (SMB_EINVAL); NFSMB_SMBOUTB(sc, SMB_CMD, cmd); - NFSMB_SMBOUTB(sc, SMB_BCNT, len); - for (i = 0; i < len; i++) + NFSMB_SMBOUTB(sc, SMB_BCNT, count); + for (i = 0; i < count; i++) NFSMB_SMBOUTB(sc, SMB_DATA + i, buf[i]); NFSMB_SMBOUTB(sc, SMB_ADDR, slave); NFSMB_SMBOUTB(sc, SMB_PRTCL, SMB_PRTCL_WRITE | SMB_PRTCL_BLOCK_DATA); @@ -475,24 +476,29 @@ nfsmb_bwrite(device_t dev, u_char slave, char cmd, u_char count, char *buf) } static int -nfsmb_bread(device_t dev, u_char slave, char cmd, u_char count, char *buf) +nfsmb_bread(device_t dev, u_char slave, char cmd, u_char *count, char *buf) { struct nfsmb_softc *sc = (struct nfsmb_softc *)device_get_softc(dev); - u_char len, i; + u_char data, len, i; int error; + if (*count < 1 || *count > 32) + return (SMB_EINVAL); NFSMB_SMBOUTB(sc, SMB_CMD, cmd); NFSMB_SMBOUTB(sc, SMB_ADDR, slave); NFSMB_SMBOUTB(sc, SMB_PRTCL, SMB_PRTCL_READ | SMB_PRTCL_BLOCK_DATA); if ((error = nfsmb_wait(sc)) == SMB_ENOERR) { len = NFSMB_SMBINB(sc, SMB_BCNT); - len = min(len, 32); - for (i = 0; i < len; i++) - buf[i] = NFSMB_SMBINB(sc, SMB_DATA + i); + for (i = 0; i < len; i++) { + data = NFSMB_SMBINB(sc, SMB_DATA + i); + if (i < *count) + buf[i] = data; + } + *count = len; } - NFSMB_DEBUG(printf("nfsmb: READBLK to 0x%x, count=0x%x, cmd=0x%x, error=0x%x", slave, count, cmd, error)); + NFSMB_DEBUG(printf("nfsmb: READBLK to 0x%x, count=0x%x, cmd=0x%x, error=0x%x", slave, *count, cmd, error)); return (error); } @@ -555,6 +561,7 @@ static driver_t nfsmbsub_driver = { DRIVER_MODULE(nfsmb, pci, nfsmb_driver, nfsmb_devclass, 0, 0); DRIVER_MODULE(nfsmb, nfsmb, nfsmbsub_driver, nfsmb_devclass, 0, 0); +DRIVER_MODULE(smbus, nfsmb, smbus_driver, smbus_devclass, 0, 0); MODULE_DEPEND(nfsmb, pci, 1, 1, 1); MODULE_DEPEND(nfsmb, smbus, SMBUS_MINVER, SMBUS_PREFVER, SMBUS_MAXVER); diff --git a/sys/pci/viapm.c b/sys/pci/viapm.c index 0bddb22..a26703e 100644 --- a/sys/pci/viapm.c +++ b/sys/pci/viapm.c @@ -640,7 +640,7 @@ viasmb_quick(device_t dev, u_char slave, int how) viapm_clear(viapm); if (viapm_busy(viapm)) - return (EBUSY); + return (SMB_EBUSY); switch (how) { case SMB_QWRITE: @@ -670,7 +670,7 @@ viasmb_sendb(device_t dev, u_char slave, char byte) viapm_clear(viapm); if (viapm_busy(viapm)) - return (EBUSY); + return (SMB_EBUSY); VIAPM_OUTB(SMBHADDR, slave & ~ LSB); VIAPM_OUTB(SMBHCMD, byte); @@ -692,7 +692,7 @@ viasmb_recvb(device_t dev, u_char slave, char *byte) viapm_clear(viapm); if (viapm_busy(viapm)) - return (EBUSY); + return (SMB_EBUSY); VIAPM_OUTB(SMBHADDR, slave | LSB); @@ -714,7 +714,7 @@ viasmb_writeb(device_t dev, u_char slave, char cmd, char byte) viapm_clear(viapm); if (viapm_busy(viapm)) - return (EBUSY); + return (SMB_EBUSY); VIAPM_OUTB(SMBHADDR, slave & ~ LSB); VIAPM_OUTB(SMBHCMD, cmd); @@ -737,7 +737,7 @@ viasmb_readb(device_t dev, u_char slave, char cmd, char *byte) viapm_clear(viapm); if (viapm_busy(viapm)) - return (EBUSY); + return (SMB_EBUSY); VIAPM_OUTB(SMBHADDR, slave | LSB); VIAPM_OUTB(SMBHCMD, cmd); @@ -760,7 +760,7 @@ viasmb_writew(device_t dev, u_char slave, char cmd, short word) viapm_clear(viapm); if (viapm_busy(viapm)) - return (EBUSY); + return (SMB_EBUSY); VIAPM_OUTB(SMBHADDR, slave & ~ LSB); VIAPM_OUTB(SMBHCMD, cmd); @@ -785,7 +785,7 @@ viasmb_readw(device_t dev, u_char slave, char cmd, short *word) viapm_clear(viapm); if (viapm_busy(viapm)) - return (EBUSY); + return (SMB_EBUSY); VIAPM_OUTB(SMBHADDR, slave | LSB); VIAPM_OUTB(SMBHCMD, cmd); @@ -808,37 +808,31 @@ static int viasmb_bwrite(device_t dev, u_char slave, char cmd, u_char count, char *buf) { struct viapm_softc *viapm = (struct viapm_softc *)device_get_softc(dev); - u_char remain, len, i; - int error = SMB_ENOERR; + u_char i; + int error; + + if (count < 1 || count > 32) + return (SMB_EINVAL); viapm_clear(viapm); if (viapm_busy(viapm)) - return (EBUSY); - - remain = count; - while (remain) { - len = min(remain, 32); - - VIAPM_OUTB(SMBHADDR, slave & ~LSB); - VIAPM_OUTB(SMBHCMD, cmd); - VIAPM_OUTB(SMBHDATA0, len); - i = VIAPM_INB(SMBHCTRL); - - /* fill the 32-byte internal buffer */ - for (i=0; i 32) + return (SMB_EINVAL); viapm_clear(viapm); if (viapm_busy(viapm)) - return (EBUSY); - - remain = count; - while (remain) { - VIAPM_OUTB(SMBHADDR, slave | LSB); - VIAPM_OUTB(SMBHCMD, cmd); - VIAPM_OUTB(SMBHCTRL, SMBHCTRL_START | SMBHCTRL_BLOCK); - - if ((error = viapm_wait(viapm)) != SMB_ENOERR) - goto error; + return (SMB_EBUSY); - len = VIAPM_INB(SMBHDATA0); - i = VIAPM_INB(SMBHCTRL); /* reset counter */ + VIAPM_OUTB(SMBHADDR, slave | LSB); + VIAPM_OUTB(SMBHCMD, cmd); + VIAPM_OUTB(SMBHCTRL, SMBHCTRL_START | SMBHCTRL_BLOCK); - len = min(len, remain); + if ((error = viapm_wait(viapm)) != SMB_ENOERR) + goto error; - /* read the 32-byte internal buffer */ - for (i=0; i