linux-kernel - Re: [PATCH 2/5] iommu/amd: Implement DOMAIN_ATTR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120127110120.GL19255@amd.com>
Date:	Fri, 27 Jan 2012 12:01:20 +0100
From:	Joerg Roedel <joerg.roedel@....com>
To:	Scott Wood <scottwood@...escale.com>
CC:	Joerg Roedel <joro@...tes.org>,
	Sethi Varun-B16395 <B16395@...escale.com>,
	"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
	Ohad Ben-Cohen <ohad@...ery.com>,
	Tony Lindgren <tony@...mide.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Wood Scott-B07421 <B07421@...escale.com>,
	David Brown <davidb@...eaurora.org>,
	David Woodhouse <dwmw2@...radead.org>
Subject: Re: [PATCH 2/5] iommu/amd: Implement DOMAIN_ATTR_GEOMETRY attribute

On Thu, Jan 26, 2012 at 02:02:26PM -0600, Scott Wood wrote:
> On 01/26/2012 01:44 PM, Joerg Roedel wrote:

> >>> Another reason why it must be in the generic struct is the intended
> >>> generic dma-ops layer on-top. This code can decide on this flag wheter a
> >>> address needs to be remapped at all.
> >>
> >> So the DMA API would just read this, not write it?

Yes, the dma-ops code needs this information to decide whether remapping
is required at all and where the remap window is.

> > The whole geometry thing is only implemented on the read side. There is
> > no implementation in domain_set_attr for it. So the geometry
> > information is read-only by default.
> 
> We will need to be able to set this, for some vfio+kvm uses.

That's fine. Just implement a handler for it in the driver-specific
set_attr callback then. 

> >> Still no reason why it couldn't be a separate attribute.  Then if you
> >> get a failure trying to write it, it's more obvious why.
> > 
> > This would mean iommu specific hacks, which are not necessary in this
> > case.
> 
> Why would making it a separate generic attribute require iommu specific
> hacks?

Because the dma-ops code needs something like

	iommu_domain_get_attr(domain, GART_ATTR_FORCE_APERTURE, data);

to check it. I call this a hack because the dma-ops code asks if it runs
on a specific hardware. This is not necessary here.

> 
> >>> Setting this flag wrong does not create unintended identity mappings.
> >>
> >> Failing to set it means that DMA can go through that is not limited to
> >> explicitly created mappings.  In some contexts (e.g. vfio) this is a
> >> security hole.
> > 
> > No, when the hardware does not allow this, then software can't enforce
> > it.
> 
> OK, it looked like an option that could be enabled or disabled --
> because I didn't realize you were considering geometry as read-only.

It is indeed read-only for most iommus and the dma-ops code only needs
to read this information. And it is necessary for the iommu-api user to
get this information when we want to support iommus that have a limited
remapping-window for each domain.

> So how would a vfio user set up the iommu so a kvm guest sees iova ==
> guest physical, for at least its main memory, if the default aperture
> doesn't cover it?
> 
> We also will gain performance by being able to set custom geometry,
> because we will not be as hard on the IOMMU cache if we can use fewer,
> larger subwindows.

No problem if you also implement the write-side for your implementation.

> > Which hardware capabilities besides the geometry do you mean?
> 
> Well, we have things like stash target (automatic cache prefetch after
> DMA) configuration, but in this case I was thinking about restrictions
> on what kind of aperture you can set, and what sort of mappings you can
> create with the result.

The stash target is a perfect fit for a PAMU specific domain attribute.

> On current Freescale PAMUs, the aperture must be a size-aligned
> power-of-two region, which is carved into up to 256 equal-size
> subwindows.  A mapping can be any power of 2 up to the subwindow size,
> but its iova must be the subwindow start address.
> 
> We cannot just say "oh look, an aperture from here to there, we can
> create all the mappings we want within that space", at least not with an
> aperture over 1 MiB (256 contiguous 4k subwindows).

Yes, we talked about that already. Probably we should talk about code to
make progress here. Do you have anything ready to post?


	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/