Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1666607

Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with memory encryption

From Borislav Petkov <bp@alien8.de>
Newsgroups linux.kernel
Subject Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with memory encryption
Date 2017-06-15 11:50 +0200
Message-ID <tSzzA-MV-19@gated-at.bofh.it> (permalink)
References <tPRjk-44i-19@gated-at.bofh.it> <tPVwC-6GZ-15@gated-at.bofh.it> <tSkAx-8cx-7@gated-at.bofh.it> <tSzzA-MV-21@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with  memory encryption Borislav Petkov <bp@alien8.de> - 2017-06-14 19:50 +0200
  Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with  memory encryption Borislav Petkov <bp@alien8.de> - 2017-06-15 11:50 +0200
    Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with  memory encryption Borislav Petkov <bp@alien8.de> - 2017-06-15 17:40 +0200
      Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with memory encryption Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> - 2017-06-15 22:20 +0200
      Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with  memory encryption Borislav Petkov <bp@alien8.de> - 2017-06-19 19:20 +0200

csiph-web