From owner-freebsd-arch Sat Dec 15 17:33:32 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mail.gmx.net (mail.gmx.net [213.165.64.20]) by hub.freebsd.org (Postfix) with SMTP id 6BB0E37B417 for ; Sat, 15 Dec 2001 17:33:15 -0800 (PST) Received: (qmail 10863 invoked by uid 0); 16 Dec 2001 01:33:13 -0000 Received: from p5086edec.dip.t-dialin.net (HELO forge.local) (80.134.237.236) by mail.gmx.net (mp020-rz3) with SMTP; 16 Dec 2001 01:33:13 -0000 Received: from tmm by forge.local with local (Exim 3.30 #1) id 16FQB8-0001yQ-00; Sun, 16 Dec 2001 02:33:22 +0100 Date: Sun, 16 Dec 2001 02:33:21 +0100 From: Thomas Moestl To: Mike Smith Cc: arch@FreeBSD.org Subject: Re: Please review: changes to MI bus code for sparc64 Message-ID: <20011216023321.C458@crow.dom2ip.de> Mail-Followup-To: Mike Smith , arch@FreeBSD.org References: <200112140028.fBE0Sol04630@mass.dis.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <200112140028.fBE0Sol04630@mass.dis.org>; from msmith@freebsd.org on Thu, Dec 13, 2001 at 04:28:50PM -0800 Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Thu, 2001/12/13 at 16:28:50 -0800, Mike Smith wrote: > > > The PCI_BROKEN_INTPIN/PCI_INTLINE_0_BAD seem to be the same thing; > > > they should be protected by a single name (probably PCI_BROKEN_INTPIN) > > > in the #ifdef in pci.c; it should be "all or nothing" on a single > > > value. As it is, you must define one if you define the other, but > > > not vice versa, and the effect seems to be linked, anyway, so you > > > might as well use a single protection mechanism. > > > > It is not uncommon that i386 BIOSes to set the intline register to 0 > > when it should really be 0xff (to indicate an unrouted interrupt). So, > > I figured that it might be useful to make this an extra option. > > No. Fix the i386-specific config space accessor to convert 0 to 255. > > If you haven't seen the theme here yet; here it is. The MD layers should > correct for platform-specific aberrations in the PCI implementation where > possible. > > Adding compile-time options to MI code which indirectly relate exclusively > to MD PCI issues is just the Wrong Thing to Do. OK, I've attached a new diff. Changes are: - remove PCI_INTLINE_0_BAD - move the code for PCI_BROKEN_INTPIN to MD code, and use a quirk table to identify the devices that need this fixup (I don't really like this change, as I think it is not totally unlikely that the device in question may also be used for other architectures; it is not really specific to sparc64) - revert the changes associated to pci_pci.c, at the cost of duplicating some of the device hierarchy assumptions in the apb driver code - add a diff I had previously forgotten: move the PCI_ENABLE_IO_MODES from conf/options.i386 to conf/options This should remove all the changes you did not like, except the addition to the resource manager. I do not see a good alternative to this last change, and I think I did not fully understand what you did not like about it (taking into consideration the task I am using it for); I'm of course open for further discussion. I would still like to commit this new diff around the 21st, if there are no further objections and I can make it. - thomas --- freebsd/sys/isa/isa_common.c Sat Sep 8 19:46:52 2001 +++ sparc64/sys/isa/isa_common.c Wed Oct 10 00:59:24 2001 @@ -92,7 +92,7 @@ isa_probe(device_t dev) { device_set_desc(dev, "ISA bus"); - isa_init(); /* Allow machdep code to initialise */ + isa_init(dev); /* Allow machdep code to initialise */ return 0; } @@ -634,37 +634,6 @@ } static int -isa_print_resources(struct resource_list *rl, const char *name, int type, - int count, const char *format) -{ - struct resource_list_entry *rle; - int printed; - int i, retval = 0;; - - printed = 0; - for (i = 0; i < count; i++) { - rle = resource_list_find(rl, type, i); - if (rle) { - if (printed == 0) - retval += printf(" %s ", name); - else if (printed > 0) - retval += printf(","); - printed++; - retval += printf(format, rle->start); - if (rle->count > 1) { - retval += printf("-"); - retval += printf(format, - rle->start + rle->count - 1); - } - } else if (i > 3) { - /* check the first few regardless */ - break; - } - } - return retval; -} - -static int isa_print_all_resources(device_t dev) { struct isa_device *idev = DEVTOISA(dev); @@ -674,14 +643,10 @@ if (SLIST_FIRST(rl) || device_get_flags(dev)) retval += printf(" at"); - retval += isa_print_resources(rl, "port", SYS_RES_IOPORT, - ISA_NPORT, "%#lx"); - retval += isa_print_resources(rl, "iomem", SYS_RES_MEMORY, - ISA_NMEM, "%#lx"); - retval += isa_print_resources(rl, "irq", SYS_RES_IRQ, - ISA_NIRQ, "%ld"); - retval += isa_print_resources(rl, "drq", SYS_RES_DRQ, - ISA_NDRQ, "%ld"); + retval += resource_list_print_type(rl, "port", SYS_RES_IOPORT, "%#lx"); + retval += resource_list_print_type(rl, "iomem", SYS_RES_MEMORY, "%#lx"); + retval += resource_list_print_type(rl, "irq", SYS_RES_IRQ, "%ld"); + retval += resource_list_print_type(rl, "drq", SYS_RES_DRQ, "%ld"); if (device_get_flags(dev)) retval += printf(" flags %#x", device_get_flags(dev)); --- freebsd/sys/isa/isa_common.h Sat Sep 8 19:46:52 2001 +++ sparc64/sys/isa/isa_common.h Wed Oct 10 00:59:27 2001 @@ -65,7 +65,7 @@ /* * These functions are architecture dependant. */ -extern void isa_init(void); +extern void isa_init(device_t dev); extern struct resource *isa_alloc_resource(device_t bus, device_t child, int type, int *rid, u_long start, u_long end, --- freebsd/sys/dev/pci/pci.c Sun Nov 4 01:39:41 2001 +++ sparc64/sys/dev/pci/pci.c Sat Dec 15 18:14:45 2001 @@ -78,9 +78,6 @@ device_t dev); static void pci_add_children(device_t dev, int busno); static int pci_probe(device_t dev); -static int pci_print_resources(struct resource_list *rl, - const char *name, int type, - const char *format); static int pci_print_child(device_t dev, device_t child); static void pci_probe_nomatch(device_t dev, device_t child); static int pci_describe_parse_line(char **ptr, int *vendor, @@ -831,34 +828,6 @@ } static int -pci_print_resources(struct resource_list *rl, const char *name, int type, - const char *format) -{ - struct resource_list_entry *rle; - int printed, retval; - - printed = 0; - retval = 0; - /* Yes, this is kinda cheating */ - SLIST_FOREACH(rle, rl, link) { - if (rle->type == type) { - if (printed == 0) - retval += printf(" %s ", name); - else if (printed > 0) - retval += printf(","); - printed++; - retval += printf(format, rle->start); - if (rle->count > 1) { - retval += printf("-"); - retval += printf(format, rle->start + - rle->count - 1); - } - } - } - return retval; -} - -static int pci_print_child(device_t dev, device_t child) { struct pci_devinfo *dinfo; @@ -872,9 +841,9 @@ retval += bus_print_child_header(dev, child); - retval += pci_print_resources(rl, "port", SYS_RES_IOPORT, "%#lx"); - retval += pci_print_resources(rl, "mem", SYS_RES_MEMORY, "%#lx"); - retval += pci_print_resources(rl, "irq", SYS_RES_IRQ, "%ld"); + retval += resource_list_print_type(rl, "port", SYS_RES_IOPORT, "%#lx"); + retval += resource_list_print_type(rl, "mem", SYS_RES_MEMORY, "%#lx"); + retval += resource_list_print_type(rl, "irq", SYS_RES_IRQ, "%ld"); if (device_get_flags(dev)) retval += printf(" flags %#x", device_get_flags(dev)); --- freebsd/sys/dev/pci/pcivar.h Sun Aug 5 19:55:43 2001 +++ sparc64/sys/dev/pci/pcivar.h Wed Oct 10 00:59:22 2001 @@ -182,20 +182,8 @@ /* * Simplified accessors for pci devices */ -#define PCI_ACCESSOR(A, B, T) \ - \ -static __inline T pci_get_ ## A(device_t dev) \ -{ \ - uintptr_t v; \ - BUS_READ_IVAR(device_get_parent(dev), dev, PCI_IVAR_ ## B, &v); \ - return (T) v; \ -} \ - \ -static __inline void pci_set_ ## A(device_t dev, T t) \ -{ \ - uintptr_t v = (uintptr_t) t; \ - BUS_WRITE_IVAR(device_get_parent(dev), dev, PCI_IVAR_ ## B, v); \ -} +#define PCI_ACCESSOR(var, ivar, type) \ + __BUS_ACCESSOR(pci, var, PCI, ivar, type) PCI_ACCESSOR(subvendor, SUBVENDOR, u_int16_t) PCI_ACCESSOR(subdevice, SUBDEVICE, u_int16_t) --- freebsd/sys/sys/bus.h Sun Nov 4 01:40:09 2001 +++ sparc64/sys/sys/bus.h Sun Nov 4 01:14:54 2001 @@ -180,6 +180,14 @@ int type, int rid, struct resource *res); /* + * Print all resources of a specified type, for use in bus_print_child. + * The name is printed if at least one resource of the given type is available. + * The format ist used to print resource start and end. + */ +int resource_list_print_type(struct resource_list *rl, + const char *name, int type, + const char *format); +/* * The root bus, to which all top-level busses are attached. */ extern device_t root_bus; @@ -410,6 +418,26 @@ }; \ DECLARE_MODULE(name##_##busname, name##_##busname##_mod, \ SI_SUB_DRIVERS, SI_ORDER_MIDDLE) + +/* + * Generic ivar accessor generation macros for bus drivers + */ +#define __BUS_ACCESSOR(varp, var, ivarp, ivar, type) \ + \ +static __inline type varp ## _get_ ## var(device_t dev) \ +{ \ + uintptr_t v; \ + BUS_READ_IVAR(device_get_parent(dev), dev, \ + ivarp ## _IVAR_ ## ivar, &v); \ + return ((type) v); \ +} \ + \ +static __inline void varp ## _set_ ## var(device_t dev, type t) \ +{ \ + uintptr_t v = (uintptr_t) t; \ + BUS_WRITE_IVAR(device_get_parent(dev), dev, \ + ivarp ## _IVAR_ ## ivar, v); \ +} #endif /* _KERNEL */ --- freebsd/sys/sys/rman.h Sat Sep 8 19:53:09 2001 +++ sparc64/sys/sys/rman.h Thu Nov 22 23:54:27 2001 @@ -126,6 +126,9 @@ struct resource *rman_reserve_resource(struct rman *rm, u_long start, u_long end, u_long count, u_int flags, struct device *dev); +struct resource *rman_reserve_resource_bound(struct rman *rm, u_long start, + u_long end, u_long count, u_long bound, + u_int flags, struct device *dev); uint32_t rman_make_alignment_flags(uint32_t size); #define rman_get_start(r) ((r)->r_start) --- freebsd/sys/kern/subr_bus.c Mon Dec 10 20:44:39 2001 +++ sparc64/sys/kern/subr_bus.c Thu Dec 13 04:06:25 2001 @@ -1207,8 +1207,8 @@ if (isdefault) { start = rle->start; - count = max(count, rle->count); - end = max(rle->end, start + count - 1); + count = ulmax(count, rle->count); + end = ulmax(rle->end, start + count - 1); } rle->res = BUS_ALLOC_RESOURCE(device_get_parent(bus), child, @@ -1253,6 +1253,34 @@ rle->res = NULL; return (0); +} + +int +resource_list_print_type(struct resource_list *rl, const char *name, int type, + const char *format) +{ + struct resource_list_entry *rle; + int printed, retval; + + printed = 0; + retval = 0; + /* Yes, this is kinda cheating */ + SLIST_FOREACH(rle, rl, link) { + if (rle->type == type) { + if (printed == 0) + retval += printf(" %s ", name); + else + retval += printf(","); + printed++; + retval += printf(format, rle->start); + if (rle->count > 1) { + retval += printf("-"); + retval += printf(format, rle->start + + rle->count - 1); + } + } + } + return retval; } /* --- freebsd/sys/kern/subr_rman.c Sun Nov 25 14:51:37 2001 +++ sparc64/sys/kern/subr_rman.c Thu Nov 22 23:54:25 2001 @@ -175,12 +175,13 @@ } struct resource * -rman_reserve_resource(struct rman *rm, u_long start, u_long end, u_long count, - u_int flags, struct device *dev) +rman_reserve_resource_bound(struct rman *rm, u_long start, u_long end, + u_long count, u_long bound, u_int flags, + struct device *dev) { u_int want_activate; struct resource *r, *s, *rv; - u_long rstart, rend; + u_long rstart, rend, amask, bmask; rv = 0; @@ -202,6 +203,9 @@ goto out; } + amask = (1ul << RF_ALIGNMENT(flags)) - 1; + /* If bound is 0, bmask will also be 0 */ + bmask = ~(bound - 1); /* * First try to find an acceptable totally-unshared region. */ @@ -215,10 +219,19 @@ DPRINTF(("region is allocated\n")); continue; } - rstart = max(s->r_start, start); - rstart = (rstart + ((1ul << RF_ALIGNMENT(flags))) - 1) & - ~((1ul << RF_ALIGNMENT(flags)) - 1); - rend = min(s->r_end, max(rstart + count, end)); + rstart = ulmax(s->r_start, start); + /* + * Try to find a region by adjusting to boundary and alignment + * until both conditions are satisfied. This is not an optimal + * algorithm, but in most cases it isn't really bad, either. + */ + do { + rstart = (rstart + amask) & ~amask; + if (((rstart ^ (rstart + count)) & bmask) != 0) + rstart += bound - (rstart & ~bmask); + } while ((rstart & amask) != 0 && rstart < end && + rstart < s->r_end); + rend = ulmin(s->r_end, ulmax(rstart + count, end)); DPRINTF(("truncated region: [%#lx, %#lx]; size %#lx (requested %#lx)\n", rstart, rend, (rend - rstart + 1), count)); @@ -313,10 +326,12 @@ break; if ((s->r_flags & flags) != flags) continue; - rstart = max(s->r_start, start); - rend = min(s->r_end, max(start + count, end)); + rstart = ulmax(s->r_start, start); + rend = ulmin(s->r_end, ulmax(start + count, end)); if (s->r_start >= start && s->r_end <= end - && (s->r_end - s->r_start + 1) == count) { + && (s->r_end - s->r_start + 1) == count && + (s->r_start & amask) == 0 && + ((s->r_start ^ s->r_end) & bmask) == 0) { rv = malloc(sizeof *rv, M_RMAN, M_NOWAIT | M_ZERO); if (rv == 0) goto out; @@ -368,6 +383,15 @@ return (rv); } +struct resource * +rman_reserve_resource(struct rman *rm, u_long start, u_long end, u_long count, + u_int flags, struct device *dev) +{ + + return (rman_reserve_resource_bound(rm, start, end, count, 0, flags, + dev)); +} + static int int_rman_activate_resource(struct rman *rm, struct resource *r, struct resource **whohas) @@ -575,7 +599,7 @@ * Find the hightest bit set, and add one if more than one bit * set. We're effectively computing the ceil(log2(size)) here. */ - for (i = 32; i > 0; i--) + for (i = 31; i > 0; i--) if ((1 << i) & size) break; if (~(1 << i) & size) --- freebsd/sys/i386/isa/isa.c Fri Oct 12 16:18:20 2001 +++ sparc64/sys/i386/isa/isa.c Thu Dec 13 17:53:29 2001 @@ -68,7 +68,7 @@ #include void -isa_init(void) +isa_init(device_t dev) { } --- freebsd/sys/ia64/isa/isa.c Sun Aug 5 20:05:14 2001 +++ sparc64/sys/ia64/isa/isa.c Thu Dec 13 17:53:29 2001 @@ -69,7 +69,7 @@ #include void -isa_init(void) +isa_init(device_t dev) { } --- freebsd/sys/alpha/isa/isa.c Sun Aug 5 19:43:26 2001 +++ sparc64/sys/alpha/isa/isa.c Thu Dec 13 17:53:29 2001 @@ -94,7 +94,7 @@ } void -isa_init(void) +isa_init(device_t dev) { isa_init_intr(); } --- freebsd/sys/conf/options.i386 Fri Dec 14 23:26:25 2001 +++ sparc64/sys/conf/options.i386 Sat Dec 15 18:14:45 2001 @@ -107,8 +107,6 @@ PSM_RESETAFTERSUSPEND opt_psm.h PSM_DEBUG opt_psm.h -PCI_ENABLE_IO_MODES opt_pci.h - PCIC_RESUME_RESET opt_pcic.h ATKBD_DFLT_KEYMAP opt_atkbd.h --- freebsd/sys/conf/options Sun Nov 25 14:51:22 2001 +++ sparc64/sys/conf/options Sat Dec 15 18:14:45 2001 @@ -416,6 +419,7 @@ # PCI related options PCI_QUIET opt_pci.h +PCI_ENABLE_IO_MODES opt_pci.h # NFS options NFS_MINATTRTIMO opt_nfs.h To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message