linux-kernel - Re: [PATCH 9/9] x86: Detect whether we should use Xen SWIOTLB.

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: <20100728095157I.fujita.tomonori@lab.ntt.co.jp>
Date:	Wed, 28 Jul 2010 09:52:34 +0900
From:	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
To:	hpa@...or.com
Cc:	fujita.tomonori@....ntt.co.jp, konrad.wilk@...cle.com,
	jeremy@...p.org, Ian.Campbell@...rix.com, albert_herranz@...oo.es,
	x86@...nel.org, jbarnes@...tuousgeek.org,
	linux-kernel@...r.kernel.org, iommu@...ts.linux-foundation.org,
	tglx@...utronix.de
Subject: Re: [PATCH 9/9] x86: Detect whether we should use Xen SWIOTLB.

On Tue, 27 Jul 2010 17:19:41 -0700
"H. Peter Anvin" <hpa@...or.com> wrote:

> On 07/27/2010 04:36 PM, FUJITA Tomonori wrote:
> >>>
> >>> Is there any way we can abstract this out a bit more instead of crapping
> >>> on generic code?
> > 
> > I don't like this change much too, however I think that this is the
> > most simple and straightforward.
> > 
> > Basically, Xen's swiotlb works like a new IOMMU implementation so we
> > need to initialize it like other IOMMU implementations (call the
> > detect and init functions in order).
> > 
> 
> Even mentioning "xen" in generic code should be considered a bug.  I
> think we *do* need to driverize the iommu stuff, and yes, Xen's swiotlb
> should just be handled like one in the list.
> 
> > 
> >> I was toying with something like this:
> >>
> >> diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> >> index 9f07cfc..e0cd388 100644
> >> --- a/arch/x86/kernel/pci-dma.c
> >> +++ b/arch/x86/kernel/pci-dma.c
> >> @@ -45,6 +45,25 @@ int iommu_detected __read_mostly = 0;
> >>   */
> >>  int iommu_pass_through __read_mostly;
> >>  
> >> +initcall_t __swiotlb_initcall_detect[] =
> >> +	{pci_xen_swiotlb_detect,
> >> +	 pci_swiotlb_detect,
> >> +	NULL};
> >> +
> >> +initcall_t __swiotlb_initcall_init[] = {
> >> +	pci_xen_swiotlb_init,
> >> +	pci_swiotlb_init,
> >> +	NULL};
> >> +
> >> +
> >> +initcall_t __iommu_initcall_detect[] = {
> >> +	gart_iommu_hole_init,
> >> +	detect_calgary,
> >> +	detect_intel_iommu,
> >> +	/* needs to be called after gart_iommu_hole_init */
> >> +	amd_iommu_detect,
> >> +	NULL};
> > 
> > I really don't think that this makes the code better. I prefer the
> > current simple (dumb) code.
> > 
> 
> The special handling of swiotlb here really looks wrong, but otherwise I
> think it's the right idea.
> 
> > btw, this comment is wrong. We check if we are forced to use SWIOTLB
> > by kernel command line here.
> > 
> > Even if SWIOTLB works, we see if hardware IOMMU is available. SWIOTLB
> > is a last resort. We prefer hardware IOMMU.
> 
> Any reason to not just handle swiotlb like any of the other iommus, at
> the bottom of the list?

we need to check if swiotlb usage is forced by the command line since:

- we skip hardware IOMMU initialization if so.


We also need swiotlb initialization after all hardware IOMMU
initialization since:

- if all hardware IOMMU initialization fails, we might need to
initialize swiotlb.

- even if hardware IOMMU initialization is successful, we might need
to initialize swiotlb (even if a system has hardware IOMMU, some
devices are not connected to hardware IOMMU).

- swiotlb initialization must be after GART initialization. We reserve
some DMA32 memory for broken bios with GART. The order must be freeing
the memory, initializing GART, then initializing swiotlb.
Initializing swiotlb before GART steals the reserved memory. It breaks
GART.
--
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/