On Wed, Jun 14, 2017 at 03:40:28PM -0500, Tom Lendacky wrote: > I was trying to keep all the logic for it here in the SME related files > rather than put it in the iommu code itself. But it is easy enough to > move if you think it's worth it. Yes please - the less needlessly global symbols, the better. > > Also, you said in another mail on this subthread that c->microcode > > is not yet set. Are you saying, that the iommu init gunk runs before > > init_amd(), where we do set c->microcode? > > > > If so, we can move the setting to early_init_amd() or so. > > I'll look into that. And I don't think c->microcode is not set by the time we init the iommu because, AFAICT, we do the latter in pci_iommu_init() and that's a rootfs_initcall() which happens later then the CPU init stuff. > I'll look into simplifying the checks. Something like this maybe? if (rev >= 0x1205) return true; if (rev <= 0x11ff && rev >= 0x1126) return true; return false; > > 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." ? If so, it did work fine until now, without the volatile. Why is it needed now, all of a sudden? -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.