svn commit: r197969 - head/sys/conf

[M. Warner Losh]

In message: <200910141738.43910.jhb at freebsd.org>
            John Baldwin <jhb at freebsd.org> 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.