Author: brooks Date: Mon Apr 9 22:59:10 2018 New Revision: 332343 URL: https://svnweb.freebsd.org/changeset/base/332343
Log: Refactor PCIOCGETCONF for improved readability. The code now has a single, consistant flow for all three ioctl variants. ifdefs and for pre-FreeBSD-7 compatability are moved to functions and macros. So the flow is alwasy the same, we impose the cost of allocating, copying to, updating from, and freeing a copy of struct pci_conf_io on all paths. This change will allow PCIOCGETCONF32 support currently in sys/compat/freebsd32/freebsd32_ioctl.c to be moved here. Reviewed by: kib, jhb Obtained from: CheriBSD Sponsored by: DARPA, AFRL Differential Revision: https://reviews.freebsd.org/D14978 Modified: head/sys/dev/pci/pci_user.c Modified: head/sys/dev/pci/pci_user.c ============================================================================== --- head/sys/dev/pci/pci_user.c Mon Apr 9 22:23:45 2018 (r332342) +++ head/sys/dev/pci/pci_user.c Mon Apr 9 22:59:10 2018 (r332343) @@ -65,8 +65,6 @@ __FBSDID("$FreeBSD$"); static d_open_t pci_open; static d_close_t pci_close; -static int pci_conf_match(struct pci_match_conf *matches, int num_matches, - struct pci_conf *match_buf); static d_ioctl_t pci_ioctl; struct cdevsw pcicdev = { @@ -106,7 +104,7 @@ pci_close(struct cdev *dev, int flag, int devtype, str * This function returns 1 on failure, 0 on success. */ static int -pci_conf_match(struct pci_match_conf *matches, int num_matches, +pci_conf_match_native(struct pci_match_conf *matches, int num_matches, struct pci_conf *match_buf) { int i; @@ -275,9 +273,6 @@ struct pci_conf_io32 { #define PCIOCREAD_OLD _IOWR('p', 2, struct pci_io_old) #define PCIOCWRITE_OLD _IOWR('p', 3, struct pci_io_old) -static int pci_conf_match_old(struct pci_match_conf_old *matches, - int num_matches, struct pci_conf *match_buf); - static int pci_conf_match_old(struct pci_match_conf_old *matches, int num_matches, struct pci_conf *match_buf) @@ -405,9 +400,46 @@ pci_conf_match_old32(struct pci_match_conf_old32 *matc return (1); } #endif /* COMPAT_FREEBSD32 */ -#endif /* PRE7_COMPAT */ +#endif /* !PRE7_COMPAT */ +union pci_conf_union { + struct pci_conf pc; +#ifdef PRE7_COMPAT + struct pci_conf_old pco; +#ifdef COMPAT_FREEBSD32 + struct pci_conf_old32 pco32; +#endif +#endif +}; + static int +pci_conf_match(u_long cmd, struct pci_match_conf *matches, int num_matches, + struct pci_conf *match_buf) +{ + + switch (cmd) { + case PCIOCGETCONF: + return (pci_conf_match_native( + (struct pci_match_conf *)matches, num_matches, match_buf)); +#ifdef PRE7_COMPAT + case PCIOCGETCONF_OLD: + return (pci_conf_match_old( + (struct pci_match_conf_old *)matches, num_matches, + match_buf)); +#ifdef COMPAT_FREEBSD32 + case PCIOCGETCONF_OLD32: + return (pci_conf_match_old32( + (struct pci_match_conf_old32 *)matches, num_matches, + match_buf)); +#endif +#endif + default: + /* programmer error */ + return (0); + } +} + +static int pci_list_vpd(device_t dev, struct pci_list_vpd_io *lvio) { struct pci_vpd_element vpd_element, *vpd_user; @@ -490,11 +522,184 @@ pci_list_vpd(device_t dev, struct pci_list_vpd_io *lvi return (0); } +static size_t +pci_match_conf_size(u_long cmd) +{ + + switch (cmd) { + case PCIOCGETCONF: + return (sizeof(struct pci_match_conf)); +#ifdef PRE7_COMPAT + case PCIOCGETCONF_OLD: + return (sizeof(struct pci_match_conf_old)); +#ifdef COMPAT_FREEBSD32 + case PCIOCGETCONF_OLD32: + return (sizeof(struct pci_match_conf_old32)); +#endif +#endif + default: + /* programmer error */ + return (0); + } +} + +static size_t +pci_conf_size(u_long cmd) +{ + + switch (cmd) { + case PCIOCGETCONF: + return (sizeof(struct pci_conf)); +#ifdef PRE7_COMPAT + case PCIOCGETCONF_OLD: + return (sizeof(struct pci_conf_old)); +#ifdef COMPAT_FREEBSD32 + case PCIOCGETCONF_OLD32: + return (sizeof(struct pci_conf_old32)); +#endif +#endif + default: + /* programmer error */ + return (0); + } +} + +static void +pci_conf_io_init(struct pci_conf_io *cio, caddr_t data, u_long cmd) +{ +#if defined(PRE7_COMPAT) && defined(COMPAT_FREEBSD32) + struct pci_conf_io32 *cio32; +#endif + + switch (cmd) { + case PCIOCGETCONF: +#ifdef PRE7_COMPAT + case PCIOCGETCONF_OLD: +#endif + *cio = *(struct pci_conf_io *)data; + return; + +#if defined(PRE7_COMPAT) && defined(COMPAT_FREEBSD32) + case PCIOCGETCONF_OLD32: + cio32 = (struct pci_conf_io32 *)data; + cio->pat_buf_len = cio32->pat_buf_len; + cio->num_patterns = cio32->num_patterns; + cio->patterns = (void *)(uintptr_t)cio32->patterns; + cio->match_buf_len = cio32->match_buf_len; + cio->num_matches = cio32->num_matches; + cio->matches = (void *)(uintptr_t)cio32->matches; + cio->offset = cio32->offset; + cio->generation = cio32->generation; + cio->status = cio32->status; + return; +#endif + + default: + /* programmer error */ + return; + } +} + +static void +pci_conf_io_update_data(const struct pci_conf_io *cio, caddr_t data, + u_long cmd) +{ + struct pci_conf_io *d_cio; +#if defined(PRE7_COMPAT) && defined(COMPAT_FREEBSD32) + struct pci_conf_io32 *cio32; +#endif + + switch (cmd) { + case PCIOCGETCONF: +#ifdef PRE7_COMPAT + case PCIOCGETCONF_OLD: +#endif + d_cio = (struct pci_conf_io *)data; + d_cio->status = cio->status; + d_cio->generation = cio->generation; + d_cio->offset = cio->offset; + d_cio->num_matches = cio->num_matches; + return; + +#if defined(PRE7_COMPAT) && defined(COMPAT_FREEBSD32) + case PCIOCGETCONF_OLD32: + cio32 = (struct pci_conf_io32 *)data; + + cio32->status = cio->status; + cio32->generation = cio->generation; + cio32->offset = cio->offset; + cio32->num_matches = cio->num_matches; + return; +#endif + + default: + /* programmer error */ + return; + } +} + +static void +pci_conf_for_copyout(const struct pci_conf *pcp, union pci_conf_union *pcup, + u_long cmd) +{ + + memset(pcup, 0, sizeof(*pcup)); + + switch (cmd) { + case PCIOCGETCONF: + pcup->pc = *pcp; + return; + +#ifdef PRE7_COMPAT +#ifdef COMPAT_FREEBSD32 + case PCIOCGETCONF_OLD32: + pcup->pco32.pc_sel.pc_bus = pcp->pc_sel.pc_bus; + pcup->pco32.pc_sel.pc_dev = pcp->pc_sel.pc_dev; + pcup->pco32.pc_sel.pc_func = pcp->pc_sel.pc_func; + pcup->pco32.pc_hdr = pcp->pc_hdr; + pcup->pco32.pc_subvendor = pcp->pc_subvendor; + pcup->pco32.pc_subdevice = pcp->pc_subdevice; + pcup->pco32.pc_vendor = pcp->pc_vendor; + pcup->pco32.pc_device = pcp->pc_device; + pcup->pco32.pc_class = pcp->pc_class; + pcup->pco32.pc_subclass = pcp->pc_subclass; + pcup->pco32.pc_progif = pcp->pc_progif; + pcup->pco32.pc_revid = pcp->pc_revid; + strlcpy(pcup->pco32.pd_name, pcp->pd_name, + sizeof(pcup->pco32.pd_name)); + pcup->pco32.pd_unit = (uint32_t)pcp->pd_unit; + return; + +#endif /* COMPAT_FREEBSD32 */ + case PCIOCGETCONF_OLD: + pcup->pco.pc_sel.pc_bus = pcp->pc_sel.pc_bus; + pcup->pco.pc_sel.pc_dev = pcp->pc_sel.pc_dev; + pcup->pco.pc_sel.pc_func = pcp->pc_sel.pc_func; + pcup->pco.pc_hdr = pcp->pc_hdr; + pcup->pco.pc_subvendor = pcp->pc_subvendor; + pcup->pco.pc_subdevice = pcp->pc_subdevice; + pcup->pco.pc_vendor = pcp->pc_vendor; + pcup->pco.pc_device = pcp->pc_device; + pcup->pco.pc_class = pcp->pc_class; + pcup->pco.pc_subclass = pcp->pc_subclass; + pcup->pco.pc_progif = pcp->pc_progif; + pcup->pco.pc_revid = pcp->pc_revid; + strlcpy(pcup->pco.pd_name, pcp->pd_name, + sizeof(pcup->pco.pd_name)); + pcup->pco.pd_unit = pcp->pd_unit; + return; +#endif /* PRE7_COMPAT */ + + default: + /* programmer error */ + return; + } +} + static int pci_ioctl(struct cdev *dev, u_long cmd, caddr_t data, int flag, struct thread *td) { device_t pcidev; - void *confdata; const char *name; struct devlist *devlist_head; struct pci_conf_io *cio = NULL; @@ -504,31 +709,25 @@ pci_ioctl(struct cdev *dev, u_long cmd, caddr_t data, struct pci_list_vpd_io *lvio; struct pci_match_conf *pattern_buf; struct pci_map *pm; - size_t confsz, iolen, pbufsz; + size_t confsz, iolen; int error, ionum, i, num_patterns; + union pci_conf_union pcu; #ifdef PRE7_COMPAT -#ifdef COMPAT_FREEBSD32 - struct pci_conf_io32 *cio32 = NULL; - struct pci_conf_old32 conf_old32; - struct pci_match_conf_old32 *pattern_buf_old32 = NULL; -#endif - struct pci_conf_old conf_old; struct pci_io iodata; struct pci_io_old *io_old; - struct pci_match_conf_old *pattern_buf_old = NULL; io_old = NULL; #endif if (!(flag & FWRITE)) { switch (cmd) { + case PCIOCGETCONF: #ifdef PRE7_COMPAT + case PCIOCGETCONF_OLD: #ifdef COMPAT_FREEBSD32 case PCIOCGETCONF_OLD32: #endif - case PCIOCGETCONF_OLD: #endif - case PCIOCGETCONF: case PCIOCGETBAR: case PCIOCLISTVPD: break; @@ -537,39 +736,18 @@ pci_ioctl(struct cdev *dev, u_long cmd, caddr_t data, } } - switch (cmd) { -#ifdef PRE7_COMPAT -#ifdef COMPAT_FREEBSD32 - case PCIOCGETCONF_OLD32: - cio32 = (struct pci_conf_io32 *)data; - cio = malloc(sizeof(struct pci_conf_io), M_TEMP, M_WAITOK); - cio->pat_buf_len = cio32->pat_buf_len; - cio->num_patterns = cio32->num_patterns; - cio->patterns = (void *)(uintptr_t)cio32->patterns; - cio->match_buf_len = cio32->match_buf_len; - cio->num_matches = cio32->num_matches; - cio->matches = (void *)(uintptr_t)cio32->matches; - cio->offset = cio32->offset; - cio->generation = cio32->generation; - cio->status = cio32->status; - cio32->num_matches = 0; - break; -#endif - case PCIOCGETCONF_OLD: -#endif - case PCIOCGETCONF: - cio = (struct pci_conf_io *)data; - } switch (cmd) { + case PCIOCGETCONF: #ifdef PRE7_COMPAT + case PCIOCGETCONF_OLD: #ifdef COMPAT_FREEBSD32 case PCIOCGETCONF_OLD32: #endif - case PCIOCGETCONF_OLD: #endif - case PCIOCGETCONF: - + cio = malloc(sizeof(struct pci_conf_io), M_TEMP, + M_WAITOK | M_ZERO); + pci_conf_io_init(cio, data, cmd); pattern_buf = NULL; num_patterns = 0; dinfo = NULL; @@ -608,17 +786,7 @@ pci_ioctl(struct cdev *dev, u_long cmd, caddr_t data, * multiple of sizeof(struct pci_conf) in case the user * didn't specify a multiple of that size. */ -#ifdef PRE7_COMPAT -#ifdef COMPAT_FREEBSD32 - if (cmd == PCIOCGETCONF_OLD32) - confsz = sizeof(struct pci_conf_old32); - else -#endif - if (cmd == PCIOCGETCONF_OLD) - confsz = sizeof(struct pci_conf_old); - else -#endif - confsz = sizeof(struct pci_conf); + confsz = pci_conf_size(cmd); iolen = min(cio->match_buf_len - (cio->match_buf_len % confsz), pci_numdevs * confsz); @@ -647,18 +815,8 @@ pci_ioctl(struct cdev *dev, u_long cmd, caddr_t data, * it's far more likely to just catch folks that * updated their kernel but not their userland. */ -#ifdef PRE7_COMPAT -#ifdef COMPAT_FREEBSD32 - if (cmd == PCIOCGETCONF_OLD32) - pbufsz = sizeof(struct pci_match_conf_old32); - else -#endif - if (cmd == PCIOCGETCONF_OLD) - pbufsz = sizeof(struct pci_match_conf_old); - else -#endif - pbufsz = sizeof(struct pci_match_conf); - if (cio->num_patterns * pbufsz != cio->pat_buf_len) { + if (cio->num_patterns * pci_match_conf_size(cmd) != + cio->pat_buf_len) { /* The user made a mistake, return an error. */ cio->status = PCI_GETCONF_ERROR; error = EINVAL; @@ -668,28 +826,10 @@ pci_ioctl(struct cdev *dev, u_long cmd, caddr_t data, /* * Allocate a buffer to hold the patterns. */ -#ifdef PRE7_COMPAT -#ifdef COMPAT_FREEBSD32 - if (cmd == PCIOCGETCONF_OLD32) { - pattern_buf_old32 = malloc(cio->pat_buf_len, - M_TEMP, M_WAITOK); - error = copyin(cio->patterns, - pattern_buf_old32, cio->pat_buf_len); - } else -#endif /* COMPAT_FREEBSD32 */ - if (cmd == PCIOCGETCONF_OLD) { - pattern_buf_old = malloc(cio->pat_buf_len, - M_TEMP, M_WAITOK); - error = copyin(cio->patterns, - pattern_buf_old, cio->pat_buf_len); - } else -#endif /* PRE7_COMPAT */ - { - pattern_buf = malloc(cio->pat_buf_len, M_TEMP, - M_WAITOK); - error = copyin(cio->patterns, pattern_buf, - cio->pat_buf_len); - } + pattern_buf = malloc(cio->pat_buf_len, M_TEMP, + M_WAITOK); + error = copyin(cio->patterns, pattern_buf, + cio->pat_buf_len); if (error != 0) { error = EINVAL; goto getconfexit; @@ -732,27 +872,9 @@ pci_ioctl(struct cdev *dev, u_long cmd, caddr_t data, dinfo->conf.pd_unit = 0; } -#ifdef PRE7_COMPAT - if ( -#ifdef COMPAT_FREEBSD32 - (cmd == PCIOCGETCONF_OLD32 && - (pattern_buf_old32 == NULL || - pci_conf_match_old32(pattern_buf_old32, - num_patterns, &dinfo->conf) == 0)) || -#endif - (cmd == PCIOCGETCONF_OLD && - (pattern_buf_old == NULL || - pci_conf_match_old(pattern_buf_old, num_patterns, - &dinfo->conf) == 0)) || - (cmd == PCIOCGETCONF && - (pattern_buf == NULL || - pci_conf_match(pattern_buf, num_patterns, - &dinfo->conf) == 0))) { -#else if (pattern_buf == NULL || - pci_conf_match(pattern_buf, num_patterns, + pci_conf_match(cmd, pattern_buf, num_patterns, &dinfo->conf) == 0) { -#endif /* * If we've filled up the user's buffer, * break out at this point. Since we've @@ -766,79 +888,8 @@ pci_ioctl(struct cdev *dev, u_long cmd, caddr_t data, break; } -#ifdef PRE7_COMPAT -#ifdef COMPAT_FREEBSD32 - if (cmd == PCIOCGETCONF_OLD32) { - memset(&conf_old32, 0, - sizeof(conf_old32)); - conf_old32.pc_sel.pc_bus = - dinfo->conf.pc_sel.pc_bus; - conf_old32.pc_sel.pc_dev = - dinfo->conf.pc_sel.pc_dev; - conf_old32.pc_sel.pc_func = - dinfo->conf.pc_sel.pc_func; - conf_old32.pc_hdr = dinfo->conf.pc_hdr; - conf_old32.pc_subvendor = - dinfo->conf.pc_subvendor; - conf_old32.pc_subdevice = - dinfo->conf.pc_subdevice; - conf_old32.pc_vendor = - dinfo->conf.pc_vendor; - conf_old32.pc_device = - dinfo->conf.pc_device; - conf_old32.pc_class = - dinfo->conf.pc_class; - conf_old32.pc_subclass = - dinfo->conf.pc_subclass; - conf_old32.pc_progif = - dinfo->conf.pc_progif; - conf_old32.pc_revid = - dinfo->conf.pc_revid; - strncpy(conf_old32.pd_name, - dinfo->conf.pd_name, - sizeof(conf_old32.pd_name)); - conf_old32.pd_name[PCI_MAXNAMELEN] = 0; - conf_old32.pd_unit = - (uint32_t)dinfo->conf.pd_unit; - confdata = &conf_old32; - } else -#endif /* COMPAT_FREEBSD32 */ - if (cmd == PCIOCGETCONF_OLD) { - memset(&conf_old, 0, sizeof(conf_old)); - conf_old.pc_sel.pc_bus = - dinfo->conf.pc_sel.pc_bus; - conf_old.pc_sel.pc_dev = - dinfo->conf.pc_sel.pc_dev; - conf_old.pc_sel.pc_func = - dinfo->conf.pc_sel.pc_func; - conf_old.pc_hdr = dinfo->conf.pc_hdr; - conf_old.pc_subvendor = - dinfo->conf.pc_subvendor; - conf_old.pc_subdevice = - dinfo->conf.pc_subdevice; - conf_old.pc_vendor = - dinfo->conf.pc_vendor; - conf_old.pc_device = - dinfo->conf.pc_device; - conf_old.pc_class = - dinfo->conf.pc_class; - conf_old.pc_subclass = - dinfo->conf.pc_subclass; - conf_old.pc_progif = - dinfo->conf.pc_progif; - conf_old.pc_revid = - dinfo->conf.pc_revid; - strncpy(conf_old.pd_name, - dinfo->conf.pd_name, - sizeof(conf_old.pd_name)); - conf_old.pd_name[PCI_MAXNAMELEN] = 0; - conf_old.pd_unit = - dinfo->conf.pd_unit; - confdata = &conf_old; - } else -#endif /* PRE7_COMPAT */ - confdata = &dinfo->conf; - error = copyout(confdata, + pci_conf_for_copyout(&dinfo->conf, &pcu, cmd); + error = copyout(&pcu, (caddr_t)cio->matches + confsz * cio->num_matches, confsz); if (error) @@ -871,23 +922,9 @@ pci_ioctl(struct cdev *dev, u_long cmd, caddr_t data, cio->status = PCI_GETCONF_MORE_DEVS; getconfexit: -#ifdef PRE7_COMPAT -#ifdef COMPAT_FREEBSD32 - if (cmd == PCIOCGETCONF_OLD32) { - cio32->status = cio->status; - cio32->generation = cio->generation; - cio32->offset = cio->offset; - cio32->num_matches = cio->num_matches; - free(cio, M_TEMP); - } - if (pattern_buf_old32 != NULL) - free(pattern_buf_old32, M_TEMP); -#endif - if (pattern_buf_old != NULL) - free(pattern_buf_old, M_TEMP); -#endif - if (pattern_buf != NULL) - free(pattern_buf, M_TEMP); + pci_conf_io_update_data(cio, data, cmd); + free(cio, M_TEMP); + free(pattern_buf, M_TEMP); break; _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
