Manuel Bouyer wrote:

>>Are you saying that there is a case where hardware caching would be 
>>desirable, but yet merging or reordering accesses would be bad?  I can't 
>>imagine this to be true, but maybe I'm not understanding what you are 
>>meaning here.
>>    
>>
>
>With a write-thru cache, I think you can achieve read caching without
>enabling write merging/reordering. Whenever this is desireable depends
>on the device you're talking to.
>For example, with ix(4) i think you could write to the buffer using PIO
>and read it using the memory-mapped ram.
>  
>
Seems like an unusual configuration to me.  But it does explain why we 
have separate degrees of freedom for cacheable and prefetchable.

It is interesting to me that we assume read/write will be treated the 
same in our mappings.  Should we allow for seperation of this?  Again, I 
can't think of any concrete examples where caching (as an example) loads 
vs. stores would be useful.  (I guess you could have buffer that has no 
side effects for read, but which you can write to -- think a xmit buffer 
that you wind up going and reexamining for some reason.  Seems a bit 
contrived though.)

>  
>
>>>>do.  For example, imagine two 8 bit registers side by side.  One of them
>>>>gets updated in an interrupt, and one gets updated in main line code. 
>>>>You can't make this work right easily from a software perspective, but
>>>>hardware can.
>>>>  
>>>>
>>>>        
>>>>
>>>But I don't think of a hardware were this would do desireable. This is
>>>typically the kind of situation where you want write order to be preserved,
>>>and probably synchronous in the interrupt case.
>>>
>>>
>>>      
>>>
>>You would want to ensure that the write completed in the interrupt case, 
>>but if another register write was beside it and had been written 
>>earlier, it might be benign to write them both concurrently in the 
>>interrupt case.
>>    
>>
>
>Maybe, but probably still ordered. 
>
>  
>
>
>
>  
>
>>>>If however, you can set it up so that bus_space_barrier knows whether
>>>>the memory space is cached or not (e.g. by way of the flags), and is a
>>>>noop for those mappings that are not cached, then this would probably be
>>>>very good.
>>>>        
>>>>
>>>bus_space_barrier already has the informations for this. If it's not using 
>>>it
>>>it's a bug.
>>>
>>>
>>>      
>>>
>>Okay.  Will you be fixing any implementation(s)? that suffer from this 
>>defect?
>>    
>>
>
>I probably don't have the knowledges to fix this, but I'll look at it anyway
>  
>
Okay, then I think we need to get more info, buy-in from those that do.  
I'm willing to fix at least evbmips.  (I'd offer to do other MIPS ports, 
but I only have evbmips hardware.)

>>I think there are a lot of cases where the driver just wants plain, 
>>unordered, uncached access to device registers.
>>    
>>
>
>I think you mean "ordered" here.
>  
>
Yes.  (I was actually thinking "un-reordered" when I typed that.  Stupid 
of me.)

>  
>
>>Accurately figuring out 
>>barriers beyond that is likely to be somewhat painful for a large 
>>variety of drivers.
>>    
>>
>
>Sure. But as most drivers are not using barrier at all right now, it's not
>a problem.
>  
>
Okay, but I still would like to see the convenience/compatibility macros 
defined.

>  
>
>>I think the documentation will need to be updated.
>>    
>>
>
>Yes, this was the point which started this thread: the documentation and
>implementation are not in sync :)
>  
>
Apart from the finer grained values, a driver conforming to the 
documentation is still "correct" (if underperforming) so its not as bad 
you suggest.

My main point here is that if we're going to have implicit barriers, we 
need to be very careful to document when explicit barriers must or may 
not be used.

>  
>
>>It would be nice if 
>>the name of the flag BUS_SPACE_MAP_PREFETCHABLE somehow reflected the 
>>fact that barriers were explicit vs. implicit.
>>    
>>
>
>Hum, I can't think of a better one. Do you have any suggestions?
>
How about "BUS_SPACE_MAP_STRICT_ORDER", or conversely, 
"BUS_SPACE_MAP_LAX_ORDER" (or something similar)?

Alternatively, "BUS_SPACE_MAP_BARRIER_EXPLICIT/IMPLICIT". I like this 
less, because I think you still want barriers if BUS_SPACE_CACHEABLE is 
used.

>
>  
>
>>Most device on MIPS plaforms are located in uncached memory space, so 
>>the assumption is *usually* correct.
>>
>>Yet the barrier operation is non-null.  One could argue reasonably that 
>>this is a bug in the mips implementation.  But until this bug is fixed, 
>>your proposed changes will have a bad effect on performance, at least on 
>>MIPS hardware.
>>    
>>
>
>So I guess the barrier should check if the bus_space handle used is
>mapped in cached or non-cached memory.
>  
>
Agreed, although there are challenges there -- owing to the fact that 
most of this bus_space code is in MIPS "MI" code, but the details of 
which addresses are cached or not is really MD.  (The fact that there 
are over half a dozen different MIPS "ports" doesn't help.)

>  
>
>>I wonder if other ports have similar issues.  I have not done a survey 
>>of them.  Have you?
>>    
>>
>
>Only i386 and sparc64. On i386 no barrier are needed, on sparc64
>none should be needed either, but I'm tracking down a bug in a driver
>which could in fact be caused by write happening out of order.
>  
>
I think it would be a good idea to look at more of them, then -- or at 
least solicit input from the port masters.

Also, I'd think a barrier may be needed on some high end sparc64 
hardware.  (I seem to recall that this was certainly true for Solaris 
drivers with DMA mapped consistent -- you still had to do a 
ddi_dma_sync().  I'm not sure whether PIO needed it or not.  The problem 
involved flushing some buffers in the crossbar on SF15K class systems.  
Not sure that NetBSD is ever run on that class of machine anyway, 
though.  Not many developers/users have several hundred K $ to run a 
NetBSD on it.)

As far as your particular bug, I think the first best fix is to add the 
barrier to the driver, without waiting for resolution of the larger 
issue here.  (Besides which, the change you propose is really too big to 
be considered as a backport, but I think a small barrier fix to a given 
driver would be reasonable to backport, if the need existed.)

>These have to be done port by port, and I don't know enouth details about
>most of them.
>
>  
>
>>>This I can't see why it would be slower than the current code. These
>>>devices needs read/read and write/write barrier on every register
>>>access.
>>>
>>>
>>>      
>>>
>>Concrete example: wi(4) on mips.  Currently these just "work" because 
>>the PCI space is mapped uncached.  But the barrier ops for that platform 
>>issues a cache flush.
>>
>>I guess a reasonable argument is that the platform code is busted 
>>because it should know whether it is uncached or not, and do the "right" 
>>thing.
>>    
>>
>
>Yes, clearly. This kind of detail should not be known to the Mi device
>drivers. If we strictly followed the bus_space man page, right now,
>wi(4) and the ide drivers should use barrier in the MI part of the
>code.
>  
>
Ok.  So can we define "fixing" the MIPS port(s) as a requirement for 
this change, please.  Otherwise performance on many platforms will be 
adversely impacted.  (At this point I'm not arguing that the current 
code is incorrect, only that it functions reasonably well.)

>  
>
>>But it doesn't, and the end result of your changes will 
>>seriously impact performance of these devices on (probably) all MIPS 
>>platforms.  I suspect that other ports may have similar issues, but I 
>>have not checked.
>>    
>>
>
>I didn't either. but bus_space read/write is already doing the right thing
>or most driver wouldn't work properly. I think the mips port is a special
>case, and I suspect it can be easily fixed.
>  
>
Btw, there is not just "one" MIPS port.  There are a number of them, 
including (off memory) evbmips, sbmips, pmax, cobalt, sgimips, mipsco, 
ews4800mips (or whatever it is called), playstation2.  There may be 
others.  Most of them suffer from this deficiency.  It might be a good 
idea to include port-mips in the discussion.

    -- Garrett