On Thu, Jun 15, 2017 at 11:41:12AM +0200, Borislav Petkov wrote: > On Wed, Jun 14, 2017 at 03:40:28PM -0500, Tom Lendacky wrote: > > > WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst > > > #134: FILE: drivers/iommu/amd_iommu.c:866: > > > +static void build_completion_wait(struct iommu_cmd *cmd, volatile u64 *sem) > > > > > > > The semaphore area is written to by the device so the use of volatile is > > appropriate in this case. > > 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. 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. Regards, Joerg
