On 6/21/2017 11:59 AM, Borislav Petkov wrote: > On Wed, Jun 21, 2017 at 05:37:22PM +0200, Joerg Roedel wrote: >>> Do you mean this is like the last exception case in that document above: >>> >>> " >>> - Pointers to data structures in coherent memory which might be modified >>> by I/O devices can, sometimes, legitimately be volatile. A ring buffer >>> used by a network adapter, where that adapter changes pointers to >>> indicate which descriptors have been processed, is an example of this >>> type of situation." >>> >>> ? >> >> So currently (without this patch) the build_completion_wait function >> does not take a volatile parameter, only wait_on_sem() does. >> >> Wait_on_sem() needs it because its purpose is to poll a memory location >> which is changed by the iommu-hardware when its done with command >> processing. > > Right, the reason above - memory modifiable by an IO device. You could > add a comment there explaining the need for the volatile. > >> But the 'volatile' in build_completion_wait() looks unnecessary, because >> the function does not poll the memory location. It only uses the >> pointer, converts it to a physical address and writes it to the command >> to be queued. > > Ok. Ok, so the (now) current version of the patch that doesn't change the function signature is the right way to go. Thanks, Tom > > Thanks. >