From owner-svn-src-all@FreeBSD.ORG Thu Oct 15 04:51:51 2009 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6CD16106566C; Thu, 15 Oct 2009 04:51:51 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id 1E3358FC0A; Thu, 15 Oct 2009 04:51:51 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.3/8.14.1) with ESMTP id n9F4iMwK034091; Wed, 14 Oct 2009 22:44:22 -0600 (MDT) (envelope-from imp@bsdimp.com) Date: Wed, 14 Oct 2009 22:44:34 -0600 (MDT) Message-Id: <20091014.224434.-345402487.imp@bsdimp.com> To: jhb@freebsd.org From: "M. Warner Losh" In-Reply-To: <200910141738.43910.jhb@freebsd.org> References: <200910141645.26010.jhb@freebsd.org> <6B860A3C-C1F0-42A0-8ECB-3D41DA698EE2@mac.com> <200910141738.43910.jhb@freebsd.org> X-Mailer: Mew version 5.2 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, xcllnt@mac.com, src-committers@freebsd.org Subject: Re: svn commit: r197969 - head/sys/conf X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Oct 2009 04:51:51 -0000 In message: <200910141738.43910.jhb@freebsd.org> John Baldwin writes: : On Wednesday 14 October 2009 5:20:38 pm Marcel Moolenaar wrote: : > : > On Oct 14, 2009, at 1:45 PM, John Baldwin wrote: : > : > > On Wednesday 14 October 2009 2:35:16 pm Marcel Moolenaar wrote: : > >> On Oct 14, 2009, at 10:39 AM, Warner Losh wrote: : > >>> : > >>> I can't be more clear than this. You keep ignoring me, and it is : > >>> very : > >>> frustrating. : > >> : > >> I'm not ignoring you. I'm still talking. You simply haven't convinced : > >> me. While it's possible (likely?) that I don't understand the issues, : > >> all you've achieved so far is that I'm more convinced that limiting : > >> orm(4) to i386 and amd64 is the right thing, because the alternative : > >> is not at all appealing. : > >> : > >>> The problem is that the : > >>> powerpc and itanium isa modules allow memory ranges that shouldn't : > >>> be : > >>> allowed. That's the platform specific code that needs to be fixed. : > >> : > >> isa_set_resource() is MI code and it happily adds whatever resources : > >> a driver wants. The only chance MD code has is to fail the : > >> allocation, : > >> but since the whole ISA code bypasses the newbus hierarchy, there's : > >> no way we know in the isa MD code what is valid and what isn't unless : > >> we add kluges to platform code. They aren't kludges. They are codifying the restrictions the platform has for that bus. : > >> If you want to fix it for real, does that mean fix it for real or : > >> does that mean add kluges to platform code? We add proper code to the platform code. The problem exists if you accidentally configure aha. It will probe memory. : > >> Shouldn't we have ISA bridges obtain the set of valid resources : > >> from their parent in the newbus hierarchy? We likely should have the ISA bridge create and give out a bus_space that maps the ISA bus addresses to wherever they are mapped on the host. : > > Hmm, can we even know that? PCI-ISA bridges in x86 at least don't : > > have any : > > I/O limit registers like PCI-PCI bridges, instead they are : > > subtractively : > > decoded, i.e. they "eat" any memory request that no one else claims. Correct. They always assume the default x86 mapping of the ISA bus space to physical memory. : > The key here being requests that reach the PCI-ISA bridge. It's entirely : > platform specific which I/O memory addresses generated by the CPU gets : > decoded and forwarded in such a way that it's visible to the PCI-ISA : > bridge. This is what needs to be obtained from the parent in the newbus : > hierarchy. Rather than hardcoding [0xC0000 .. 0x100000> as the ISA : > option : > ROM memory range, it should be something like [isa_mem_base+0xC0000 .. : > isa_mem_base + 0x100000> or even [isa_rom_base .. isa_rom_base + : > 0x40000>. Hard coding 0xc0000, etc is proper here because that's not a physical address. It is an ISA bus address. This address needs to be mapped to the host memory segment at the bus_space level so that the drivers we have in the tree will work on those architectures where the mapping isn't the identity like on x86. The drivers should have no knowledge this is happening if they are using bus_space routines. That's what they exist for. : That might certainly be a reasonable IVAR for isab(4) to provide to isa(4): a : ROM base. However, orm(4) should be enabled for all ISA bridges assuming : that is fixed. OTOH, the way many bus drivers I've seen handle this so far : is to change the base address of SYS_RES_MEMORY objects as they pass through : the relevant bridge so that the actual memory address is properly adjusted : when it gets to the equivalent of the nexus. This is how many of the : Foo->PCI bridges in arm that I've looked at work, and I think this is more : inline with Warner's original patch which is to allow the various : platform-specific ISA bridge drivers reject memory ranges they do not decode : and provide any needed adjustments to ranges they do decode to transform them : into suitable resources for their parent. I think that they are done at the wrong level if they are done there. They should be done in the bus_space layer so that the driver can get the bus addresses to maybe program into pci hardware. : Then orm(4) would just see it's : attempts to do bus_alloc_resource() fail and end up DTRT. Yes and no. bus_alloc_resource would do the right thing. If the bridge driver knew that it had a private mapping, it would create an rman that had the memory ranges (in ISA memory space) that it decoded. It would then carve out allocates from this rman for any child devices that asked for it. It would return a bus_space handle that could be used by the driver and bus_space_read* to get the right thing from the memory. For x86, you couldn't implement this because the bus_space assumes one global space. On arm and mips, for example, their bus_space implementations are such that at any level you can do things like byte ordering hacks or address translations like this. On those platforms, the bus_space functions become calls into a function pointer table that allows for this kind of mapping. : Given that, I do : think this logic belongs in the ISA bridge drivers rather than in individual : ISA drivers. We don't make ISA peripheral drivers add an 'isa_mem_base' or : equivalent to their I/O resources, and in terms of I/O resources, orm(4) is : just a special blackhole device (much like the ACPI or PnPBIOS system : resource devices, or the ram0 or apic0 devices on x86). Keep in mind that 0xc0000 is a bus space address on the ISA bus, not a physical memory address. On x86 it just happens to be mapped 1:1 to physical memory. On other systems this isn't the case. On those systems, the bus_space that is returned for this resource should do the translation. that's what it was invented for. For example, the TS-7200 board maps the 8-bit memory reads to one address range, 16-bit memory reads to another, 8-bit port access to a third and 16-bit port access to a forth. Let's call these M8, M16, I8 and I16. So a bus_space_read_1 of a memory rid will return *(uint8_t *)(M8 + offset). A bus_space_read_2 of the same offset will return *(uint16_t *)(M16 + offset), etc. So when orm runs on this board, its bus_space stuff will read M8 + 0xc0000, etc. No changs to the driver are necessary to accomplish this. All the fixes are done in the bus_space that's used to access the isa bus on this board. This is the basis of my saying that the MD code for the machine in question must either filter out the requests for the memory range on the ISA bus that isn't decoded. Or it must provide the translation from the ISA BUS address passed to bus_space_read_1 to the actual physical/virtual address that the bus is mapped in on that machine. If you think about it, this needs to be the case. Many ISA cards need to have the address on the bus programmed into them for various things. If you had to have every platform and subbus codified for them in the driver, we'd quickly have a big mess in every driver since they would all have to know how to map the physical addresses. This is also the source of my frustration with the thread, I think. I don't think that Marcel understood this world view and assumed that no mapping was needed and therefore orm was this horribly x86 specific code. Since I didn't understand this until today, I reacted badly and lost my temper, for which I'd like to apologize. I should have stopped and explained this view better. Warner P.S. The code to implement this board is only in perforce right now (and may be broken). You can see how NetBSD accomplishes this in their tree.