[PATCH -tip] x86: fix iommu=soft boot option

[PATCH -tip] x86: fix iommu=soft boot option

[FUJITA Tomonori]

"x86: Handle HW IOMMU initialization failure gracefully" patchset
handled this option properly however somehow I broke it during cleanup
after that. Sorry.

=
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH -tip] x86: fix iommu=soft boot option

iommu=soft boot option forces the kernel to use swiotlb.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 arch/x86/kernel/pci-swiotlb.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index e36e71d..e3c0a66 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -50,6 +50,8 @@ static struct dma_map_ops swiotlb_dma_ops = {
  */
 int __init pci_swiotlb_init(void)
 {
+	int use_swiotlb = swiotlb | swiotlb_force;
+
 	/* don't initialize swiotlb if iommu=off (no_iommu=1) */
 #ifdef CONFIG_X86_64
 	if (!no_iommu && max_pfn > MAX_DMA32_PFN)
@@ -63,5 +65,5 @@ int __init pci_swiotlb_init(void)
 		dma_ops = &swiotlb_dma_ops;
 	}
 
-	return swiotlb_force;
+	return use_swiotlb;
 }
-- 
1.5.6.5

Re: [PATCH -tip] x86: fix iommu=soft boot option

[Yinghai Lu]

FUJITA Tomonori wrote:
> "x86: Handle HW IOMMU initialization failure gracefully" patchset
> handled this option properly however somehow I broke it during cleanup
> after that. Sorry.
> 
> =
> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Subject: [PATCH -tip] x86: fix iommu=soft boot option
> 
> iommu=soft boot option forces the kernel to use swiotlb.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> ---
>  arch/x86/kernel/pci-swiotlb.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
> index e36e71d..e3c0a66 100644
> --- a/arch/x86/kernel/pci-swiotlb.c
> +++ b/arch/x86/kernel/pci-swiotlb.c
> @@ -50,6 +50,8 @@ static struct dma_map_ops swiotlb_dma_ops = {
>   */
>  int __init pci_swiotlb_init(void)
>  {
> +	int use_swiotlb = swiotlb | swiotlb_force;
> +
>  	/* don't initialize swiotlb if iommu=off (no_iommu=1) */
>  #ifdef CONFIG_X86_64
>  	if (!no_iommu && max_pfn > MAX_DMA32_PFN)
> @@ -63,5 +65,5 @@ int __init pci_swiotlb_init(void)
>  		dma_ops = &swiotlb_dma_ops;
>  	}
>  
> -	return swiotlb_force;
> +	return use_swiotlb;
>  }

before your cleanup patchset:
for AMD 64bit, MEM > 4g, no AGP, iommu=soft
1. if BIOS have correct gart setting, Kernel will use gart
2. if BIOS does not have correct gart setting, Kernel will use swiotlb

for AMD 64bit, MEM > 4g, no AGP, no "iommu=soft"
1. if BIOS have correct gart setting, Kernel will use gart
2. if BIOS does not have correct gart setting, Kernel will allocate some RAM, and set range in AMD NB, and use gart iommu

YH

Re: [PATCH -tip] x86: fix iommu=soft boot option

[FUJITA Tomonori]

On Tue, 24 Nov 2009 15:55:03 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> FUJITA Tomonori wrote:
> > "x86: Handle HW IOMMU initialization failure gracefully" patchset
> > handled this option properly however somehow I broke it during cleanup
> > after that. Sorry.
> > 
> > =
> > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > Subject: [PATCH -tip] x86: fix iommu=soft boot option
> > 
> > iommu=soft boot option forces the kernel to use swiotlb.
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > ---
> >  arch/x86/kernel/pci-swiotlb.c |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
> > index e36e71d..e3c0a66 100644
> > --- a/arch/x86/kernel/pci-swiotlb.c
> > +++ b/arch/x86/kernel/pci-swiotlb.c
> > @@ -50,6 +50,8 @@ static struct dma_map_ops swiotlb_dma_ops = {
> >   */
> >  int __init pci_swiotlb_init(void)
> >  {
> > +	int use_swiotlb = swiotlb | swiotlb_force;
> > +
> >  	/* don't initialize swiotlb if iommu=off (no_iommu=1) */
> >  #ifdef CONFIG_X86_64
> >  	if (!no_iommu && max_pfn > MAX_DMA32_PFN)
> > @@ -63,5 +65,5 @@ int __init pci_swiotlb_init(void)
> >  		dma_ops = &swiotlb_dma_ops;
> >  	}
> >  
> > -	return swiotlb_force;
> > +	return use_swiotlb;
> >  }
> 
> before your cleanup patchset:
> for AMD 64bit, MEM > 4g, no AGP, iommu=soft
> 1. if BIOS have correct gart setting, Kernel will use gart

I know but it's GART problem.

'iommu=soft' means that the kernel must use swiotlb. All the IOMMU
drivers except for GART works properly.

Documentation/x86/x86_64/boot-options.txt says:

  General iommu options:
    off                Don't initialize and use any kind of IOMMU.
    noforce            Don't force hardware IOMMU usage when it is not needed.
                       (default).
    force              Force the use of the hardware IOMMU even when it is
                       not actually needed (e.g. because < 3 GB memory).
    soft               Use software bounce buffering (SWIOTLB) (default for
                       Intel machines). This can be used to prevent the usage
                       of an available hardware IOMMU.


> 2. if BIOS does not have correct gart setting, Kernel will use swiotlb
> 
> for AMD 64bit, MEM > 4g, no AGP, no "iommu=soft"
> 1. if BIOS have correct gart setting, Kernel will use gart
> 2. if BIOS does not have correct gart setting, Kernel will allocate some RAM, and set range in AMD NB, and use gart iommu
> 
> YH
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH -tip] x86: fix iommu=soft boot option

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> FUJITA Tomonori wrote:
> > "x86: Handle HW IOMMU initialization failure gracefully" patchset
> > handled this option properly however somehow I broke it during cleanup
> > after that. Sorry.
> > 
> > =
> > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > Subject: [PATCH -tip] x86: fix iommu=soft boot option
> > 
> > iommu=soft boot option forces the kernel to use swiotlb.
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > ---
> >  arch/x86/kernel/pci-swiotlb.c |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
> > index e36e71d..e3c0a66 100644
> > --- a/arch/x86/kernel/pci-swiotlb.c
> > +++ b/arch/x86/kernel/pci-swiotlb.c
> > @@ -50,6 +50,8 @@ static struct dma_map_ops swiotlb_dma_ops = {
> >   */
> >  int __init pci_swiotlb_init(void)
> >  {
> > +	int use_swiotlb = swiotlb | swiotlb_force;
> > +
> >  	/* don't initialize swiotlb if iommu=off (no_iommu=1) */
> >  #ifdef CONFIG_X86_64
> >  	if (!no_iommu && max_pfn > MAX_DMA32_PFN)
> > @@ -63,5 +65,5 @@ int __init pci_swiotlb_init(void)
> >  		dma_ops = &swiotlb_dma_ops;
> >  	}
> >  
> > -	return swiotlb_force;
> > +	return use_swiotlb;
> >  }
> 
> before your cleanup patchset:
> for AMD 64bit, MEM > 4g, no AGP, iommu=soft
> 1. if BIOS have correct gart setting, Kernel will use gart
> 2. if BIOS does not have correct gart setting, Kernel will use swiotlb
> 
> for AMD 64bit, MEM > 4g, no AGP, no "iommu=soft"
> 1. if BIOS have correct gart setting, Kernel will use gart
> 2. if BIOS does not have correct gart setting, Kernel will allocate some RAM, and set range in AMD NB, and use gart iommu

So the question is, how many people relied on the previous behavior of 
'iommu=soft' not really falling back to the swiotlb code but preventing 
the quirk from running.

Are you aware of specific versions of distributions adding iommu=soft 
and relying on that? Should we be careful about changing the previous 
behavior?

	Ingo

Re: [PATCH -tip] x86: fix iommu=soft boot option

[Yinghai Lu]

Ingo Molnar wrote:
> * Yinghai Lu <yinghai@kernel.org> wrote:
> 
>> FUJITA Tomonori wrote:
>>> "x86: Handle HW IOMMU initialization failure gracefully" patchset
>>> handled this option properly however somehow I broke it during cleanup
>>> after that. Sorry.
>>>
>>> =
>>> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
>>> Subject: [PATCH -tip] x86: fix iommu=soft boot option
>>>
>>> iommu=soft boot option forces the kernel to use swiotlb.
>>>
>>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
>>> ---
>>>  arch/x86/kernel/pci-swiotlb.c |    4 +++-
>>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
>>> index e36e71d..e3c0a66 100644
>>> --- a/arch/x86/kernel/pci-swiotlb.c
>>> +++ b/arch/x86/kernel/pci-swiotlb.c
>>> @@ -50,6 +50,8 @@ static struct dma_map_ops swiotlb_dma_ops = {
>>>   */
>>>  int __init pci_swiotlb_init(void)
>>>  {
>>> +	int use_swiotlb = swiotlb | swiotlb_force;
>>> +
>>>  	/* don't initialize swiotlb if iommu=off (no_iommu=1) */
>>>  #ifdef CONFIG_X86_64
>>>  	if (!no_iommu && max_pfn > MAX_DMA32_PFN)
>>> @@ -63,5 +65,5 @@ int __init pci_swiotlb_init(void)
>>>  		dma_ops = &swiotlb_dma_ops;
>>>  	}
>>>  
>>> -	return swiotlb_force;
>>> +	return use_swiotlb;
>>>  }
>> before your cleanup patchset:
>> for AMD 64bit, MEM > 4g, no AGP, iommu=soft
>> 1. if BIOS have correct gart setting, Kernel will use gart
>> 2. if BIOS does not have correct gart setting, Kernel will use swiotlb
>>
>> for AMD 64bit, MEM > 4g, no AGP, no "iommu=soft"
>> 1. if BIOS have correct gart setting, Kernel will use gart
>> 2. if BIOS does not have correct gart setting, Kernel will allocate some RAM, and set range in AMD NB, and use gart iommu
> 
> So the question is, how many people relied on the previous behavior of 
> 'iommu=soft' not really falling back to the swiotlb code but preventing 
> the quirk from running.
> 
> Are you aware of specific versions of distributions adding iommu=soft 
> and relying on that? Should we be careful about changing the previous 
> behavior?

only remember that SLES 9 SP3 (?) at some point has problem with AMD 10h ( quad core version)
when memory > 4 g (with USB controller ?)
because the gart code only check K8 device id, and didn't check 10h device id. so gart iommu is not used.
and happenly swiotlb code has problem with that kernel version.

thinking we should keep old behavior, until some day we can remove them all.

YH

Re: [PATCH -tip] x86: fix iommu=soft boot option

[FUJITA Tomonori]

On Wed, 25 Nov 2009 00:54:34 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> only remember that SLES 9 SP3 (?) at some point has problem with AMD 10h ( quad core version)
> when memory > 4 g (with USB controller ?)
> because the gart code only check K8 device id, and didn't check 10h device id. so gart iommu is not used.
> and happenly swiotlb code has problem with that kernel version.
> 
> thinking we should keep old behavior, until some day we can remove them all.

Why? We are talking about changing 2.6.33 behavior. swiotlb in 2.6.33
should be the safe option.

Re: [PATCH -tip] x86: fix iommu=soft boot option

[Ingo Molnar]

* FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> On Wed, 25 Nov 2009 00:54:34 -0800
> Yinghai Lu <yinghai@kernel.org> wrote:
> 
> > only remember that SLES 9 SP3 (?) at some point has problem with AMD 
> > 10h ( quad core version) when memory > 4 g (with USB controller ?) 
> > because the gart code only check K8 device id, and didn't check 10h 
> > device id. so gart iommu is not used. and happenly swiotlb code has 
> > problem with that kernel version.
> > 
> > thinking we should keep old behavior, until some day we can remove 
> > them all.
> 
> Why? We are talking about changing 2.6.33 behavior. swiotlb in 2.6.33 
> should be the safe option.

If that behavior was relied on for suspected (or real) bugs in the 
swiotlb code then i agree that we should do this change. (and fix any 
bugs if they still occur.)

	Ingo

Re: [PATCH -tip] x86: fix iommu=soft boot option

[Yinghai Lu]

Ingo Molnar wrote:
> * FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
> 
>> On Wed, 25 Nov 2009 00:54:34 -0800
>> Yinghai Lu <yinghai@kernel.org> wrote:
>>
>>> only remember that SLES 9 SP3 (?) at some point has problem with AMD 
>>> 10h ( quad core version) when memory > 4 g (with USB controller ?) 
>>> because the gart code only check K8 device id, and didn't check 10h 
>>> device id. so gart iommu is not used. and happenly swiotlb code has 
>>> problem with that kernel version.
>>>
>>> thinking we should keep old behavior, until some day we can remove 
>>> them all.
>> Why? We are talking about changing 2.6.33 behavior. swiotlb in 2.6.33 
>> should be the safe option.
> 
> If that behavior was relied on for suspected (or real) bugs in the 
> swiotlb code then i agree that we should do this change. (and fix any 
> bugs if they still occur.)

after look at gart_iommu_hole_init() closely,

should be

for AMD 64bit, MEM > 4g, no AGP, iommu=soft
1. if BIOS have correct gart setting, Kernel will use swiotlb
2. if BIOS does not have correct gart setting, Kernel will use swiotlb

and for the all the cases, the codes after that 
/* Fix up the north bridges */
...

will disable the translation...

so even swiotlb=soft is used, gart_iommu_hole_init() still need to be called. to make sure aperture is disabled somehow.

YH

Re: [PATCH -tip] x86: fix iommu=soft boot option

[Joerg Roedel]

On Tue, Nov 24, 2009 at 03:55:03PM -0800, Yinghai Lu wrote:
> FUJITA Tomonori wrote:
> > "x86: Handle HW IOMMU initialization failure gracefully" patchset
> > handled this option properly however somehow I broke it during cleanup
> > after that. Sorry.
> > 
> > =
> > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > Subject: [PATCH -tip] x86: fix iommu=soft boot option
> > 
> > iommu=soft boot option forces the kernel to use swiotlb.
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > ---
> >  arch/x86/kernel/pci-swiotlb.c |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
> > index e36e71d..e3c0a66 100644
> > --- a/arch/x86/kernel/pci-swiotlb.c
> > +++ b/arch/x86/kernel/pci-swiotlb.c
> > @@ -50,6 +50,8 @@ static struct dma_map_ops swiotlb_dma_ops = {
> >   */
> >  int __init pci_swiotlb_init(void)
> >  {
> > +	int use_swiotlb = swiotlb | swiotlb_force;
> > +
> >  	/* don't initialize swiotlb if iommu=off (no_iommu=1) */
> >  #ifdef CONFIG_X86_64
> >  	if (!no_iommu && max_pfn > MAX_DMA32_PFN)
> > @@ -63,5 +65,5 @@ int __init pci_swiotlb_init(void)
> >  		dma_ops = &swiotlb_dma_ops;
> >  	}
> >  
> > -	return swiotlb_force;
> > +	return use_swiotlb;
> >  }
> 
> before your cleanup patchset:
> for AMD 64bit, MEM > 4g, no AGP, iommu=soft
> 1. if BIOS have correct gart setting, Kernel will use gart
> 2. if BIOS does not have correct gart setting, Kernel will use swiotlb
> 
> for AMD 64bit, MEM > 4g, no AGP, no "iommu=soft"
> 1. if BIOS have correct gart setting, Kernel will use gart
> 2. if BIOS does not have correct gart setting, Kernel will allocate some RAM, and set range in AMD NB, and use gart iommu

The swiotlb is used as a dma-api backend always when 'iommu=soft' is specified,
at least the code I read without the cleanup patchset. The cleanup patchset
broke that in some places. Now I need to pass swiotlb=force to achieve the same
for AMD IOMMU. The above patch fixes this.
But that iommu=soft will prevent the kernel from fixing up broken gart settings
is new to me. Where have you seen this behavior?

	Joerg

Re: [PATCH -tip] x86: fix iommu=soft boot option

[FUJITA Tomonori]

On Wed, 25 Nov 2009 01:45:10 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> Ingo Molnar wrote:
> > * FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
> > 
> >> On Wed, 25 Nov 2009 00:54:34 -0800
> >> Yinghai Lu <yinghai@kernel.org> wrote:
> >>
> >>> only remember that SLES 9 SP3 (?) at some point has problem with AMD 
> >>> 10h ( quad core version) when memory > 4 g (with USB controller ?) 
> >>> because the gart code only check K8 device id, and didn't check 10h 
> >>> device id. so gart iommu is not used. and happenly swiotlb code has 
> >>> problem with that kernel version.
> >>>
> >>> thinking we should keep old behavior, until some day we can remove 
> >>> them all.
> >> Why? We are talking about changing 2.6.33 behavior. swiotlb in 2.6.33 
> >> should be the safe option.
> > 
> > If that behavior was relied on for suspected (or real) bugs in the 
> > swiotlb code then i agree that we should do this change. (and fix any 
> > bugs if they still occur.)
> 
> after look at gart_iommu_hole_init() closely,
> 
> should be
> 
> for AMD 64bit, MEM > 4g, no AGP, iommu=soft
> 1. if BIOS have correct gart setting, Kernel will use swiotlb
> 2. if BIOS does not have correct gart setting, Kernel will use swiotlb
> 
> and for the all the cases, the codes after that 
> /* Fix up the north bridges */
> ...
> 
> will disable the translation...

What code are you talking about? GART translation is disabled at boot
time (if not, we are dead because some GART hardware are broken so we
need to fix things before enabling them).


> so even swiotlb=soft is used, gart_iommu_hole_init() still need to
> be called. to make sure aperture is disabled somehow.

I don't think so.

[tip:core/iommu] x86: Fix iommu=soft boot option

[tip-bot for FUJITA Tomonori]

Commit-ID:  273bee27fa9f79d94b78c83506016f2e41e78983
Gitweb:     http://git.kernel.org/tip/273bee27fa9f79d94b78c83506016f2e41e78983
Author:     FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
AuthorDate: Wed, 25 Nov 2009 08:46:28 +0900
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 25 Nov 2009 10:12:51 +0100

x86: Fix iommu=soft boot option

iommu=soft boot option forces the kernel to use swiotlb.

( This has the side-effect of enabling the swiotlb over the
  GART if this boot option is provided. This is the desired
  behavior of the swiotlb boot option and works like that
  for all other hw-IOMMU drivers. )

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: yinghai@kernel.org
LKML-Reference: <20091125084611O.fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/pci-swiotlb.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index e36e71d..e3c0a66 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -50,6 +50,8 @@ static struct dma_map_ops swiotlb_dma_ops = {
  */
 int __init pci_swiotlb_init(void)
 {
+	int use_swiotlb = swiotlb | swiotlb_force;
+
 	/* don't initialize swiotlb if iommu=off (no_iommu=1) */
 #ifdef CONFIG_X86_64
 	if (!no_iommu && max_pfn > MAX_DMA32_PFN)
@@ -63,5 +65,5 @@ int __init pci_swiotlb_init(void)
 		dma_ops = &swiotlb_dma_ops;
 	}
 
-	return swiotlb_force;
+	return use_swiotlb;
 }

Re: [PATCH -tip] x86: fix iommu=soft boot option

[Yinghai Lu]

FUJITA Tomonori wrote:
> On Wed, 25 Nov 2009 01:45:10 -0800
> Yinghai Lu <yinghai@kernel.org> wrote:
> 
>> Ingo Molnar wrote:
>>> * FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
>>>
>>>> On Wed, 25 Nov 2009 00:54:34 -0800
>>>> Yinghai Lu <yinghai@kernel.org> wrote:
>>>>
>>>>> only remember that SLES 9 SP3 (?) at some point has problem with AMD 
>>>>> 10h ( quad core version) when memory > 4 g (with USB controller ?) 
>>>>> because the gart code only check K8 device id, and didn't check 10h 
>>>>> device id. so gart iommu is not used. and happenly swiotlb code has 
>>>>> problem with that kernel version.
>>>>>
>>>>> thinking we should keep old behavior, until some day we can remove 
>>>>> them all.
>>>> Why? We are talking about changing 2.6.33 behavior. swiotlb in 2.6.33 
>>>> should be the safe option.
>>> If that behavior was relied on for suspected (or real) bugs in the 
>>> swiotlb code then i agree that we should do this change. (and fix any 
>>> bugs if they still occur.)
>> after look at gart_iommu_hole_init() closely,
>>
>> should be
>>
>> for AMD 64bit, MEM > 4g, no AGP, iommu=soft
>> 1. if BIOS have correct gart setting, Kernel will use swiotlb
>> 2. if BIOS does not have correct gart setting, Kernel will use swiotlb
>>
>> and for the all the cases, the codes after that 
>> /* Fix up the north bridges */
>> ...
>>
>> will disable the translation...
> 
> What code are you talking about? GART translation is disabled at boot
> time (if not, we are dead because some GART hardware are broken so we
> need to fix things before enabling them).
> 
> 
>> so even swiotlb=soft is used, gart_iommu_hole_init() still need to
>> be called. to make sure aperture is disabled somehow.
> 
> I don't think so.

in aperture_64.c::gart_iommu_hole_init()
        printk(KERN_INFO  "Checking aperture...\n");

        if (!fallback_aper_force)
                agp_aper_base = search_agp_bridge(&agp_aper_order, &valid_agp);

        fix = 0;
        node = 0;
        for (i = 0; i < ARRAY_SIZE(bus_dev_ranges); i++) {
                int bus;
                int dev_base, dev_limit;

                bus = bus_dev_ranges[i].bus;
                dev_base = bus_dev_ranges[i].dev_base;
                dev_limit = bus_dev_ranges[i].dev_limit;

                for (slot = dev_base; slot < dev_limit; slot++) {

                        if (!early_is_k8_nb(read_pci_config(bus, slot, 3, 0x00)))
                                continue;

                        iommu_detected = 1;
                        gart_iommu_aperture = 1;

                        aper_order = (read_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL) >> 1) & 7;
                        aper_size = (32 * 1024 * 1024) << aper_order;
                        aper_base = read_pci_config(bus, slot, 3, AMD64_GARTAPERTUREBASE) & 0x7fff;
                        aper_base <<= 25;

                        printk(KERN_INFO "Node %d: aperture @ %Lx size %u MB\n",
                                        node, aper_base, aper_size >> 20);
                        node++;

                        if (!aperture_valid(aper_base, aper_size, 64<<20)) {
                                if (valid_agp && agp_aper_base &&
                                    agp_aper_base == aper_base &&
                                    agp_aper_order == aper_order) {
                                        /* the same between two setting from NB and agp */
                                        if (!no_iommu &&
                                            max_pfn > MAX_DMA32_PFN &&
                                            !printed_gart_size_msg) {
                                                printk(KERN_ERR "you are using iommu with agp, but GART size is less than 64M\n");
                                                printk(KERN_ERR "please increase GART size in your BIOS setup\n");
                                                printk(KERN_ERR "if BIOS doesn't have that option, contact your HW vendor!\n");
                                                printed_gart_size_msg = 1;
                                        }
                                } else {
POINT A:
                                        fix = 1;
                                        goto out;
                                }
                        }

                        if ((last_aper_order && aper_order != last_aper_order) ||
                            (last_aper_base && aper_base != last_aper_base)) {
                                fix = 1;
                                goto out;
                        }
                        last_aper_order = aper_order;
                        last_aper_base = aper_base;
                }
        }

out:
        if (!fix && !fallback_aper_force) {
                if (last_aper_base) {
                        unsigned long n = (32 * 1024 * 1024) << last_aper_order;

                        insert_aperture_resource((u32)last_aper_base, n);
                }
                return;
        }

        if (!fallback_aper_force) {
                aper_alloc = agp_aper_base;
                aper_order = agp_aper_order;
        }

        if (aper_alloc) {
                /* Got the aperture from the AGP bridge */
        } else if (swiotlb && !valid_agp) {
POINT: B
                /* Do nothing */
        } else if ((!no_iommu && max_pfn > MAX_DMA32_PFN) ||
                   force_iommu ||
                   valid_agp ||
                   fallback_aper_force) {
POINT: C
                printk(KERN_INFO
                        "Your BIOS doesn't leave a aperture memory hole\n");
                printk(KERN_INFO
                        "Please enable the IOMMU option in the BIOS setup\n");
                printk(KERN_INFO
                        "This costs you %d MB of RAM\n",
                                32 << fallback_aper_order);

                aper_order = fallback_aper_order;
                aper_alloc = allocate_aperture();
                if (!aper_alloc) {
                        /*
                         * Could disable AGP and IOMMU here, but it's
                         * probably not worth it. But the later users
                         * cannot deal with bad apertures and turning
                         * on the aperture over memory causes very
                         * strange problems, so it's better to panic
                         * early.
                         */
                        panic("Not enough memory for aperture");
                }
        } else {
                return;
        }

POINT X:

        /* Fix up the north bridges */
        for (i = 0; i < ARRAY_SIZE(bus_dev_ranges); i++) {
                int bus;
                int dev_base, dev_limit;

                bus = bus_dev_ranges[i].bus;
                dev_base = bus_dev_ranges[i].dev_base;
                dev_limit = bus_dev_ranges[i].dev_limit;
                for (slot = dev_base; slot < dev_limit; slot++) {
                        if (!early_is_k8_nb(read_pci_config(bus, slot, 3, 0x00)))
                                continue;

                        /* Don't enable translation yet. That is done later.
                           Assume this BIOS didn't initialise the GART so
                           just overwrite all previous bits */
                        write_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL, aper_order << 1);
                        write_pci_config(bus, slot, 3, AMD64_GARTAPERTUREBASE, aper_alloc >> 25);
                }
        }

when AMD64 mem > 4g, no AGP, BIOS set all gart iommu on all nodes size to 32M, and enable bit are set.
1. iommu=soft, will go through POINT A and POINT B
2. no "iommu=soft", will go through POINT A and POINT C
and all will reach POINT X to make sure ENABLE bit is not set.

please check

[PATCH] x86: fix gart iommu using for amd 64 bit system -v3

after
|commit 75f1cdf1dda92cae037ec848ae63690d91913eac
|Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
|Date:   Tue Nov 10 19:46:20 2009 +0900
|
|    x86: Handle HW IOMMU initialization failure gracefully
|    
|    If HW IOMMU initialization fails (Intel VT-d often does this,
|    typically due to BIOS bugs), we fall back to nommu. It doesn't
|    work for the majority since nowadays we have more than 4GB
|    memory so we must use swiotlb instead of nommu.
...

amd 64 systems that
1. do not have  AGP
2. do not have IOMMU
3. mem > 4g
4. BIOS do not allocate  correct gart in NB.
will leave them to use SWIOTLB forcely.

also in pci_iommu_alloc()
pci_swiotlb_init is stealing the preallocate range that is for
gart_iommu_hole workaround.

so restore the sequence...

will get back
[    0.000000] Your BIOS doesn't leave a aperture memory hole
[    0.000000] Please enable the IOMMU option in the BIOS setup
[    0.000000] This costs you 64 MB of RAM
[    0.000000] Mapping aperture over 65536 KB of RAM @ 20000000
...
[   12.702822] calling  pci_iommu_init+0x0/0x31 @ 1
[   12.708728] PCI-DMA: Disabling AGP.
[   12.712812] PCI-DMA: aperture base @ 20000000 size 65536 KB
[   12.718384] PCI-DMA: using GART IOMMU.
[   12.722139] PCI-DMA: Reserving 64MB of IOMMU area in the AGP aperture
[   12.736944] initcall pci_iommu_init+0x0/0x31 returned 0 after 28802 usecs

-v2: call shutdown when no agp is there
-v3: add check_store to restore state for swiotlb
     separate pci_swiotlb_detect and pci_swiotlb_init from FUJITA

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/include/asm/swiotlb.h |    8 ++++++--
 arch/x86/kernel/aperture_64.c  |   17 ++++++++++++++---
 arch/x86/kernel/pci-dma.c      |   11 +++++++++--
 arch/x86/kernel/pci-gart_64.c  |    3 ++-
 arch/x86/kernel/pci-swiotlb.c  |   11 +++++++----
 5 files changed, 38 insertions(+), 12 deletions(-)

Index: linux-2.6/arch/x86/kernel/aperture_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/aperture_64.c
+++ linux-2.6/arch/x86/kernel/aperture_64.c
@@ -375,6 +375,9 @@ void __init gart_iommu_hole_init(void)
 	u64 aper_base, last_aper_base = 0;
 	int fix, slot, valid_agp = 0;
 	int i, node;
+	int iommu_detected_old = iommu_detected;
+	int gart_iommu_aperture_old = gart_iommu_aperture;
+	int (*iommu_init_old)(void) = x86_init.iommu.iommu_init;
 
 	if (gart_iommu_aperture_disabled || !fix_aperture ||
 	    !early_pci_allowed())
@@ -448,7 +451,7 @@ out:
 
 			insert_aperture_resource((u32)last_aper_base, n);
 		}
-		return;
+		goto check_restore;
 	}
 
 	if (!fallback_aper_force) {
@@ -458,7 +461,7 @@ out:
 
 	if (aper_alloc) {
 		/* Got the aperture from the AGP bridge */
-	} else if (!valid_agp) {
+	} else if (swiotlb && !valid_agp) {
 		/* Do nothing */
 	} else if ((!no_iommu && max_pfn > MAX_DMA32_PFN) ||
 		   force_iommu ||
@@ -486,7 +489,7 @@ out:
 			panic("Not enough memory for aperture");
 		}
 	} else {
-		return;
+		goto check_restore;
 	}
 
 	/* Fix up the north bridges */
@@ -510,4 +513,12 @@ out:
 	}
 
 	set_up_gart_resume(aper_order, aper_alloc);
+
+check_restore:
+	if (swiotlb) {
+		/* restore the setting */
+		iommu_detected = iommu_detected_old;
+		gart_iommu_aperture = gart_iommu_aperture_old;
+		x86_init.iommu.iommu_init = iommu_init_old;
+	}
 }
Index: linux-2.6/arch/x86/kernel/pci-dma.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/pci-dma.c
+++ linux-2.6/arch/x86/kernel/pci-dma.c
@@ -120,21 +120,28 @@ static void __init dma32_free_bootmem(vo
 
 void __init pci_iommu_alloc(void)
 {
+	int ret;
+
 #ifdef CONFIG_X86_64
 	/* free the range so iommu could get some range less than 4G */
 	dma32_free_bootmem();
 #endif
-	if (pci_swiotlb_init())
-		return;
+	ret = pci_swiotlb_detect();
 
 	gart_iommu_hole_init();
 
+	if (ret)
+		goto out;
+
 	detect_calgary();
 
 	detect_intel_iommu();
 
 	/* needs to be called after gart_iommu_hole_init */
 	amd_iommu_detect();
+
+out:
+	pci_swiotlb_init();
 }
 
 void *dma_generic_alloc_coherent(struct device *dev, size_t size,
Index: linux-2.6/arch/x86/kernel/pci-gart_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/pci-gart_64.c
+++ linux-2.6/arch/x86/kernel/pci-gart_64.c
@@ -710,7 +710,8 @@ static void gart_iommu_shutdown(void)
 	struct pci_dev *dev;
 	int i;
 
-	if (no_agp)
+	/* don't shutdown it if there is AGP installed */
+	if (!no_agp)
 		return;
 
 	for (i = 0; i < num_k8_northbridges; i++) {
Index: linux-2.6/arch/x86/include/asm/swiotlb.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/swiotlb.h
+++ linux-2.6/arch/x86/include/asm/swiotlb.h
@@ -5,13 +5,17 @@
 
 #ifdef CONFIG_SWIOTLB
 extern int swiotlb;
-extern int pci_swiotlb_init(void);
+int pci_swiotlb_detect(void);
+void pci_swiotlb_init(void);
 #else
 #define swiotlb 0
-static inline int pci_swiotlb_init(void)
+static inline int pci_swiotlb_detect(void)
 {
 	return 0;
 }
+static inline void pci_swiotlb_init(void)
+{
+}
 #endif
 
 static inline void dma_mark_clean(void *addr, size_t size) {}
Index: linux-2.6/arch/x86/kernel/pci-swiotlb.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/pci-swiotlb.c
+++ linux-2.6/arch/x86/kernel/pci-swiotlb.c
@@ -43,12 +43,12 @@ static struct dma_map_ops swiotlb_dma_op
 };
 
 /*
- * pci_swiotlb_init - initialize swiotlb if necessary
+ * pci_swiotlb_detect - initialize swiotlb if necessary
  *
  * This returns non-zero if we are forced to use swiotlb (by the boot
  * option).
  */
-int __init pci_swiotlb_init(void)
+int __init pci_swiotlb_detect(void)
 {
 	int use_swiotlb = swiotlb | swiotlb_force;
 
@@ -60,10 +60,13 @@ int __init pci_swiotlb_init(void)
 	if (swiotlb_force)
 		swiotlb = 1;
 
+	return use_swiotlb;
+}
+
+void __init pci_swiotlb_init(void)
+{
 	if (swiotlb) {
 		swiotlb_init(0);
 		dma_ops = &swiotlb_dma_ops;
 	}
-
-	return use_swiotlb;
 }

Re: [PATCH -tip] x86: fix iommu=soft boot option

[FUJITA Tomonori]

On Wed, 25 Nov 2009 14:33:27 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> in aperture_64.c::gart_iommu_hole_init()
>         printk(KERN_INFO  "Checking aperture...\n");
> 
>         if (!fallback_aper_force)
>                 agp_aper_base = search_agp_bridge(&agp_aper_order, &valid_agp);
> 
>         fix = 0;
>         node = 0;
>         for (i = 0; i < ARRAY_SIZE(bus_dev_ranges); i++) {
>                 int bus;
>                 int dev_base, dev_limit;
> 
>                 bus = bus_dev_ranges[i].bus;
>                 dev_base = bus_dev_ranges[i].dev_base;
>                 dev_limit = bus_dev_ranges[i].dev_limit;
> 
>                 for (slot = dev_base; slot < dev_limit; slot++) {
> 
>                         if (!early_is_k8_nb(read_pci_config(bus, slot, 3, 0x00)))
>                                 continue;
> 
>                         iommu_detected = 1;
>                         gart_iommu_aperture = 1;
> 
>                         aper_order = (read_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL) >> 1) & 7;
>                         aper_size = (32 * 1024 * 1024) << aper_order;
>                         aper_base = read_pci_config(bus, slot, 3, AMD64_GARTAPERTUREBASE) & 0x7fff;
>                         aper_base <<= 25;
> 
>                         printk(KERN_INFO "Node %d: aperture @ %Lx size %u MB\n",
>                                         node, aper_base, aper_size >> 20);
>                         node++;
> 
>                         if (!aperture_valid(aper_base, aper_size, 64<<20)) {
>                                 if (valid_agp && agp_aper_base &&
>                                     agp_aper_base == aper_base &&
>                                     agp_aper_order == aper_order) {
>                                         /* the same between two setting from NB and agp */
>                                         if (!no_iommu &&
>                                             max_pfn > MAX_DMA32_PFN &&
>                                             !printed_gart_size_msg) {
>                                                 printk(KERN_ERR "you are using iommu with agp, but GART size is less than 64M\n");
>                                                 printk(KERN_ERR "please increase GART size in your BIOS setup\n");
>                                                 printk(KERN_ERR "if BIOS doesn't have that option, contact your HW vendor!\n");
>                                                 printed_gart_size_msg = 1;
>                                         }
>                                 } else {
> POINT A:
>                                         fix = 1;
>                                         goto out;
>                                 }
>                         }
> 
>                         if ((last_aper_order && aper_order != last_aper_order) ||
>                             (last_aper_base && aper_base != last_aper_base)) {
>                                 fix = 1;
>                                 goto out;
>                         }
>                         last_aper_order = aper_order;
>                         last_aper_base = aper_base;
>                 }
>         }
> 
> out:
>         if (!fix && !fallback_aper_force) {
>                 if (last_aper_base) {
>                         unsigned long n = (32 * 1024 * 1024) << last_aper_order;
> 
>                         insert_aperture_resource((u32)last_aper_base, n);
>                 }
>                 return;
>         }
> 
>         if (!fallback_aper_force) {
>                 aper_alloc = agp_aper_base;
>                 aper_order = agp_aper_order;
>         }
> 
>         if (aper_alloc) {
>                 /* Got the aperture from the AGP bridge */
>         } else if (swiotlb && !valid_agp) {
> POINT: B
>                 /* Do nothing */
>         } else if ((!no_iommu && max_pfn > MAX_DMA32_PFN) ||
>                    force_iommu ||
>                    valid_agp ||
>                    fallback_aper_force) {
> POINT: C
>                 printk(KERN_INFO
>                         "Your BIOS doesn't leave a aperture memory hole\n");
>                 printk(KERN_INFO
>                         "Please enable the IOMMU option in the BIOS setup\n");
>                 printk(KERN_INFO
>                         "This costs you %d MB of RAM\n",
>                                 32 << fallback_aper_order);
> 
>                 aper_order = fallback_aper_order;
>                 aper_alloc = allocate_aperture();
>                 if (!aper_alloc) {
>                         /*
>                          * Could disable AGP and IOMMU here, but it's
>                          * probably not worth it. But the later users
>                          * cannot deal with bad apertures and turning
>                          * on the aperture over memory causes very
>                          * strange problems, so it's better to panic
>                          * early.
>                          */
>                         panic("Not enough memory for aperture");
>                 }
>         } else {
>                 return;
>         }
> 
> POINT X:
> 
>         /* Fix up the north bridges */
>         for (i = 0; i < ARRAY_SIZE(bus_dev_ranges); i++) {
>                 int bus;
>                 int dev_base, dev_limit;
> 
>                 bus = bus_dev_ranges[i].bus;
>                 dev_base = bus_dev_ranges[i].dev_base;
>                 dev_limit = bus_dev_ranges[i].dev_limit;
>                 for (slot = dev_base; slot < dev_limit; slot++) {
>                         if (!early_is_k8_nb(read_pci_config(bus, slot, 3, 0x00)))
>                                 continue;
> 
>                         /* Don't enable translation yet. That is done later.
>                            Assume this BIOS didn't initialise the GART so
>                            just overwrite all previous bits */
>                         write_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL, aper_order << 1);
>                         write_pci_config(bus, slot, 3, AMD64_GARTAPERTUREBASE, aper_alloc >> 25);
>                 }
>         }
> 
> when AMD64 mem > 4g, no AGP, BIOS set all gart iommu on all nodes size to 32M, and enable bit are set.

I have such machine (with sane BIOS).

But if BIOS is broken, early_gart_iommu_check() disables GART_EN bit
(bits 0)?

> 1. iommu=soft, will go through POINT A and POINT B

Not always. if aperture_valid() is true, it doesn't go POINT A. My
GART machine doesn't go.


> 2. no "iommu=soft", will go through POINT A and POINT C

As I said, it's not true about POINT A.


> and all will reach POINT X to make sure ENABLE bit is not set.

POINT X doesn't look make sure ENABLE bit is not set. It just fixes up
(sets) the address and its size.

And with 2.6.31, if I use iommu=soft, my GART machine uses swiotlb but
it still keeps GART_EN bit enabled.

So for what does 'iommu=soft' go to POINT B and POINT X? That is, why
we can't skip them with 'iommu=soft'?


> please check
> 
> [PATCH] x86: fix gart iommu using for amd 64 bit system -v3
> 
> after
> |commit 75f1cdf1dda92cae037ec848ae63690d91913eac
> |Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> |Date:   Tue Nov 10 19:46:20 2009 +0900
> |
> |    x86: Handle HW IOMMU initialization failure gracefully
> |    
> |    If HW IOMMU initialization fails (Intel VT-d often does this,
> |    typically due to BIOS bugs), we fall back to nommu. It doesn't
> |    work for the majority since nowadays we have more than 4GB
> |    memory so we must use swiotlb instead of nommu.
> ...
> 
> amd 64 systems that
> 1. do not have  AGP
> 2. do not have IOMMU
> 3. mem > 4g
> 4. BIOS do not allocate  correct gart in NB.
> will leave them to use SWIOTLB forcely.

What should such system use in this case?


> also in pci_iommu_alloc()
> pci_swiotlb_init is stealing the preallocate range that is for
> gart_iommu_hole workaround.
> 
> so restore the sequence...
> 
> will get back
> [    0.000000] Your BIOS doesn't leave a aperture memory hole
> [    0.000000] Please enable the IOMMU option in the BIOS setup
> [    0.000000] This costs you 64 MB of RAM
> [    0.000000] Mapping aperture over 65536 KB of RAM @ 20000000
> ...
> [   12.702822] calling  pci_iommu_init+0x0/0x31 @ 1
> [   12.708728] PCI-DMA: Disabling AGP.
> [   12.712812] PCI-DMA: aperture base @ 20000000 size 65536 KB
> [   12.718384] PCI-DMA: using GART IOMMU.
> [   12.722139] PCI-DMA: Reserving 64MB of IOMMU area in the AGP aperture
> [   12.736944] initcall pci_iommu_init+0x0/0x31 returned 0 after 28802 usecs
> 
> -v2: call shutdown when no agp is there
> -v3: add check_store to restore state for swiotlb
>      separate pci_swiotlb_detect and pci_swiotlb_init from FUJITA
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  arch/x86/include/asm/swiotlb.h |    8 ++++++--
>  arch/x86/kernel/aperture_64.c  |   17 ++++++++++++++---
>  arch/x86/kernel/pci-dma.c      |   11 +++++++++--
>  arch/x86/kernel/pci-gart_64.c  |    3 ++-
>  arch/x86/kernel/pci-swiotlb.c  |   11 +++++++----
>  5 files changed, 38 insertions(+), 12 deletions(-)
> 
> Index: linux-2.6/arch/x86/kernel/aperture_64.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/aperture_64.c
> +++ linux-2.6/arch/x86/kernel/aperture_64.c
> @@ -375,6 +375,9 @@ void __init gart_iommu_hole_init(void)
>  	u64 aper_base, last_aper_base = 0;
>  	int fix, slot, valid_agp = 0;
>  	int i, node;
> +	int iommu_detected_old = iommu_detected;
> +	int gart_iommu_aperture_old = gart_iommu_aperture;
> +	int (*iommu_init_old)(void) = x86_init.iommu.iommu_init;

This 'setting first and restoring later' code is hacky. It's better to
set only when we necessary.


>  	if (gart_iommu_aperture_disabled || !fix_aperture ||
>  	    !early_pci_allowed())
> @@ -448,7 +451,7 @@ out:
>  
>  			insert_aperture_resource((u32)last_aper_base, n);
>  		}
> -		return;
> +		goto check_restore;
>  	}
>  
>  	if (!fallback_aper_force) {
> @@ -458,7 +461,7 @@ out:
>  
>  	if (aper_alloc) {
>  		/* Got the aperture from the AGP bridge */
> -	} else if (!valid_agp) {
> +	} else if (swiotlb && !valid_agp) {
>  		/* Do nothing */
>  	} else if ((!no_iommu && max_pfn > MAX_DMA32_PFN) ||
>  		   force_iommu ||
> @@ -486,7 +489,7 @@ out:
>  			panic("Not enough memory for aperture");
>  		}
>  	} else {
> -		return;
> +		goto check_restore;
>  	}
>  
>  	/* Fix up the north bridges */
> @@ -510,4 +513,12 @@ out:
>  	}
>  
>  	set_up_gart_resume(aper_order, aper_alloc);
> +
> +check_restore:
> +	if (swiotlb) {
> +		/* restore the setting */
> +		iommu_detected = iommu_detected_old;
> +		gart_iommu_aperture = gart_iommu_aperture_old;
> +		x86_init.iommu.iommu_init = iommu_init_old;
> +	}
>  }
> Index: linux-2.6/arch/x86/kernel/pci-dma.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/pci-dma.c
> +++ linux-2.6/arch/x86/kernel/pci-dma.c
> @@ -120,21 +120,28 @@ static void __init dma32_free_bootmem(vo
>  
>  void __init pci_iommu_alloc(void)
>  {
> +	int ret;
> +
>  #ifdef CONFIG_X86_64
>  	/* free the range so iommu could get some range less than 4G */
>  	dma32_free_bootmem();
>  #endif
> -	if (pci_swiotlb_init())
> -		return;
> +	ret = pci_swiotlb_detect();
>  
>  	gart_iommu_hole_init();
>  
> +	if (ret)
> +		goto out;
> +
>  	detect_calgary();
>  
>  	detect_intel_iommu();
>  
>  	/* needs to be called after gart_iommu_hole_init */
>  	amd_iommu_detect();
> +
> +out:
> +	pci_swiotlb_init();
>  }
>  
>  void *dma_generic_alloc_coherent(struct device *dev, size_t size,
> Index: linux-2.6/arch/x86/kernel/pci-gart_64.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/pci-gart_64.c
> +++ linux-2.6/arch/x86/kernel/pci-gart_64.c
> @@ -710,7 +710,8 @@ static void gart_iommu_shutdown(void)
>  	struct pci_dev *dev;
>  	int i;
>  
> -	if (no_agp)
> +	/* don't shutdown it if there is AGP installed */
> +	if (!no_agp)
>  		return;
>  
>  	for (i = 0; i < num_k8_northbridges; i++) {
> Index: linux-2.6/arch/x86/include/asm/swiotlb.h
> ===================================================================
> --- linux-2.6.orig/arch/x86/include/asm/swiotlb.h
> +++ linux-2.6/arch/x86/include/asm/swiotlb.h
> @@ -5,13 +5,17 @@
>  
>  #ifdef CONFIG_SWIOTLB
>  extern int swiotlb;
> -extern int pci_swiotlb_init(void);
> +int pci_swiotlb_detect(void);
> +void pci_swiotlb_init(void);
>  #else
>  #define swiotlb 0
> -static inline int pci_swiotlb_init(void)
> +static inline int pci_swiotlb_detect(void)
>  {
>  	return 0;
>  }
> +static inline void pci_swiotlb_init(void)
> +{
> +}
>  #endif
>  
>  static inline void dma_mark_clean(void *addr, size_t size) {}
> Index: linux-2.6/arch/x86/kernel/pci-swiotlb.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/pci-swiotlb.c
> +++ linux-2.6/arch/x86/kernel/pci-swiotlb.c
> @@ -43,12 +43,12 @@ static struct dma_map_ops swiotlb_dma_op
>  };
>  
>  /*
> - * pci_swiotlb_init - initialize swiotlb if necessary
> + * pci_swiotlb_detect - initialize swiotlb if necessary
>   *
>   * This returns non-zero if we are forced to use swiotlb (by the boot
>   * option).
>   */
> -int __init pci_swiotlb_init(void)
> +int __init pci_swiotlb_detect(void)
>  {
>  	int use_swiotlb = swiotlb | swiotlb_force;
>  
> @@ -60,10 +60,13 @@ int __init pci_swiotlb_init(void)
>  	if (swiotlb_force)
>  		swiotlb = 1;
>  
> +	return use_swiotlb;
> +}
> +
> +void __init pci_swiotlb_init(void)
> +{
>  	if (swiotlb) {
>  		swiotlb_init(0);
>  		dma_ops = &swiotlb_dma_ops;
>  	}
> -
> -	return use_swiotlb;
>  }
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH -tip] x86: fix iommu=soft boot option

[Yinghai Lu]

FUJITA Tomonori wrote:
> On Wed, 25 Nov 2009 14:33:27 -0800
> Yinghai Lu <yinghai@kernel.org> wrote:
> 
>> in aperture_64.c::gart_iommu_hole_init()
>>         printk(KERN_INFO  "Checking aperture...\n");
>>
>>         if (!fallback_aper_force)
>>                 agp_aper_base = search_agp_bridge(&agp_aper_order, &valid_agp);
>>
>>         fix = 0;
>>         node = 0;
>>         for (i = 0; i < ARRAY_SIZE(bus_dev_ranges); i++) {
>>                 int bus;
>>                 int dev_base, dev_limit;
>>
>>                 bus = bus_dev_ranges[i].bus;
>>                 dev_base = bus_dev_ranges[i].dev_base;
>>                 dev_limit = bus_dev_ranges[i].dev_limit;
>>
>>                 for (slot = dev_base; slot < dev_limit; slot++) {
>>
>>                         if (!early_is_k8_nb(read_pci_config(bus, slot, 3, 0x00)))
>>                                 continue;
>>
>>                         iommu_detected = 1;
>>                         gart_iommu_aperture = 1;
>>
>>                         aper_order = (read_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL) >> 1) & 7;
>>                         aper_size = (32 * 1024 * 1024) << aper_order;
>>                         aper_base = read_pci_config(bus, slot, 3, AMD64_GARTAPERTUREBASE) & 0x7fff;
>>                         aper_base <<= 25;
>>
>>                         printk(KERN_INFO "Node %d: aperture @ %Lx size %u MB\n",
>>                                         node, aper_base, aper_size >> 20);
>>                         node++;
>>
>>                         if (!aperture_valid(aper_base, aper_size, 64<<20)) {
>>                                 if (valid_agp && agp_aper_base &&
>>                                     agp_aper_base == aper_base &&
>>                                     agp_aper_order == aper_order) {
>>                                         /* the same between two setting from NB and agp */
>>                                         if (!no_iommu &&
>>                                             max_pfn > MAX_DMA32_PFN &&
>>                                             !printed_gart_size_msg) {
>>                                                 printk(KERN_ERR "you are using iommu with agp, but GART size is less than 64M\n");
>>                                                 printk(KERN_ERR "please increase GART size in your BIOS setup\n");
>>                                                 printk(KERN_ERR "if BIOS doesn't have that option, contact your HW vendor!\n");
>>                                                 printed_gart_size_msg = 1;
>>                                         }
>>                                 } else {
>> POINT A:
>>                                         fix = 1;
>>                                         goto out;
>>                                 }
>>                         }
>>
>>                         if ((last_aper_order && aper_order != last_aper_order) ||
>>                             (last_aper_base && aper_base != last_aper_base)) {
>>                                 fix = 1;
>>                                 goto out;
>>                         }
>>                         last_aper_order = aper_order;
>>                         last_aper_base = aper_base;
>>                 }
>>         }
>>
>> out:
>>         if (!fix && !fallback_aper_force) {
>>                 if (last_aper_base) {
>>                         unsigned long n = (32 * 1024 * 1024) << last_aper_order;
>>
>>                         insert_aperture_resource((u32)last_aper_base, n);
>>                 }
>>                 return;
>>         }
>>
>>         if (!fallback_aper_force) {
>>                 aper_alloc = agp_aper_base;
>>                 aper_order = agp_aper_order;
>>         }
>>
>>         if (aper_alloc) {
>>                 /* Got the aperture from the AGP bridge */
>>         } else if (swiotlb && !valid_agp) {
>> POINT: B
>>                 /* Do nothing */
>>         } else if ((!no_iommu && max_pfn > MAX_DMA32_PFN) ||
>>                    force_iommu ||
>>                    valid_agp ||
>>                    fallback_aper_force) {
>> POINT: C
>>                 printk(KERN_INFO
>>                         "Your BIOS doesn't leave a aperture memory hole\n");
>>                 printk(KERN_INFO
>>                         "Please enable the IOMMU option in the BIOS setup\n");
>>                 printk(KERN_INFO
>>                         "This costs you %d MB of RAM\n",
>>                                 32 << fallback_aper_order);
>>
>>                 aper_order = fallback_aper_order;
>>                 aper_alloc = allocate_aperture();
>>                 if (!aper_alloc) {
>>                         /*
>>                          * Could disable AGP and IOMMU here, but it's
>>                          * probably not worth it. But the later users
>>                          * cannot deal with bad apertures and turning
>>                          * on the aperture over memory causes very
>>                          * strange problems, so it's better to panic
>>                          * early.
>>                          */
>>                         panic("Not enough memory for aperture");
>>                 }
>>         } else {
>>                 return;
>>         }
>>
>> POINT X:
>>
>>         /* Fix up the north bridges */
>>         for (i = 0; i < ARRAY_SIZE(bus_dev_ranges); i++) {
>>                 int bus;
>>                 int dev_base, dev_limit;
>>
>>                 bus = bus_dev_ranges[i].bus;
>>                 dev_base = bus_dev_ranges[i].dev_base;
>>                 dev_limit = bus_dev_ranges[i].dev_limit;
>>                 for (slot = dev_base; slot < dev_limit; slot++) {
>>                         if (!early_is_k8_nb(read_pci_config(bus, slot, 3, 0x00)))
>>                                 continue;
>>
>>                         /* Don't enable translation yet. That is done later.
>>                            Assume this BIOS didn't initialise the GART so
>>                            just overwrite all previous bits */
>>                         write_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL, aper_order << 1);
>>                         write_pci_config(bus, slot, 3, AMD64_GARTAPERTUREBASE, aper_alloc >> 25);
>>                 }
>>         }
>>
>> when AMD64 mem > 4g, no AGP, BIOS set all gart iommu on all nodes size to 32M, and enable bit are set.
> 
> I have such machine (with sane BIOS).
> 
> But if BIOS is broken, early_gart_iommu_check() disables GART_EN bit
> (bits 0)?

not for 32M small size.

that function only clear that bit when different nodes have different setting.

> 
>> 1. iommu=soft, will go through POINT A and POINT B
> 
> Not always. if aperture_valid() is true, it doesn't go POINT A. My
> GART machine doesn't go.

maybe your bios set GART iommu set to 64M?

> 
> 
>> 2. no "iommu=soft", will go through POINT A and POINT C
> 
> As I said, it's not true about POINT A.

maybe your bios set GART iommu set to 64M?

> 
> 
>> and all will reach POINT X to make sure ENABLE bit is not set.
> 
> POINT X doesn't look make sure ENABLE bit is not set. It just fixes up
> (sets) the address and its size.

                        write_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL, aper_order << 1);

that bit is that bit 0 of AMD64_GARTAPERTURECTL reg.

YH

Re: [PATCH -tip] x86: fix iommu=soft boot option

[FUJITA Tomonori]

On Thu, 26 Nov 2009 23:45:33 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> >> when AMD64 mem > 4g, no AGP, BIOS set all gart iommu on all nodes size to 32M, and enable bit are set.
> > 
> > I have such machine (with sane BIOS).
> > 
> > But if BIOS is broken, early_gart_iommu_check() disables GART_EN bit
> > (bits 0)?
> 
> not for 32M small size.
> 
> that function only clear that bit when different nodes have different setting.
> 
> > 
> >> 1. iommu=soft, will go through POINT A and POINT B
> > 
> > Not always. if aperture_valid() is true, it doesn't go POINT A. My
> > GART machine doesn't go.
> 
> maybe your bios set GART iommu set to 64M?

Yeah, my machine has sane BIOS.


> >> 2. no "iommu=soft", will go through POINT A and POINT C
> > 
> > As I said, it's not true about POINT A.
> 
> maybe your bios set GART iommu set to 64M?
> 
> > 
> > 
> >> and all will reach POINT X to make sure ENABLE bit is not set.
> > 
> > POINT X doesn't look make sure ENABLE bit is not set. It just fixes up
> > (sets) the address and its size.
> 
>                         write_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL, aper_order << 1);
> 
> that bit is that bit 0 of AMD64_GARTAPERTURECTL reg.

Oops, thanks.

But as I wrote in the previous mail, can you tell me why we need this?
With 2.6.31 my machine uses swiotlb with GART_EN bit enabled.

My another question is that why can't early_gart_iommu_check()
_always_ disable GART_EN bit?

Re: [PATCH -tip] x86: fix iommu=soft boot option

[Yinghai Lu]

FUJITA Tomonori wrote:
> On Thu, 26 Nov 2009 23:45:33 -0800
> Yinghai Lu <yinghai@kernel.org> wrote:
> 
>>>> when AMD64 mem > 4g, no AGP, BIOS set all gart iommu on all nodes size to 32M, and enable bit are set.
>>> I have such machine (with sane BIOS).
>>>
>>> But if BIOS is broken, early_gart_iommu_check() disables GART_EN bit
>>> (bits 0)?
>> not for 32M small size.
>>
>> that function only clear that bit when different nodes have different setting.
>>
>>>> 1. iommu=soft, will go through POINT A and POINT B
>>> Not always. if aperture_valid() is true, it doesn't go POINT A. My
>>> GART machine doesn't go.
>> maybe your bios set GART iommu set to 64M?
> 
> Yeah, my machine has sane BIOS.
> 
> 
>>>> 2. no "iommu=soft", will go through POINT A and POINT C
>>> As I said, it's not true about POINT A.
>> maybe your bios set GART iommu set to 64M?
>>
>>>
>>>> and all will reach POINT X to make sure ENABLE bit is not set.
>>> POINT X doesn't look make sure ENABLE bit is not set. It just fixes up
>>> (sets) the address and its size.
>>                         write_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL, aper_order << 1);
>>
>> that bit is that bit 0 of AMD64_GARTAPERTURECTL reg.
> 
> Oops, thanks.
> 
> But as I wrote in the previous mail, can you tell me why we need this?
> With 2.6.31 my machine uses swiotlb with GART_EN bit enabled.
> 
> My another question is that why can't early_gart_iommu_check()
> _always_ disable GART_EN bit?

please check

[PATCH] x86: fix gart iommu using for amd 64 bit system -v4

after
|commit 75f1cdf1dda92cae037ec848ae63690d91913eac
|Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
|Date:   Tue Nov 10 19:46:20 2009 +0900
|
|    x86: Handle HW IOMMU initialization failure gracefully
|    
|    If HW IOMMU initialization fails (Intel VT-d often does this,
|    typically due to BIOS bugs), we fall back to nommu. It doesn't
|    work for the majority since nowadays we have more than 4GB
|    memory so we must use swiotlb instead of nommu.
...

amd 64 systems that
1. do not have  AGP
2. do not have IOMMU
3. mem > 4g
4. BIOS do not allocate  correct gart in NB.
will leave them to use SWIOTLB forcely.

also in pci_iommu_alloc()
pci_swiotlb_init is stealing the preallocate range that is for
gart_iommu_hole workaround.

so restore the sequence...

will get back
[    0.000000] Your BIOS doesn't leave a aperture memory hole
[    0.000000] Please enable the IOMMU option in the BIOS setup
[    0.000000] This costs you 64 MB of RAM
[    0.000000] Mapping aperture over 65536 KB of RAM @ 20000000
...
[   12.702822] calling  pci_iommu_init+0x0/0x31 @ 1
[   12.708728] PCI-DMA: Disabling AGP.
[   12.712812] PCI-DMA: aperture base @ 20000000 size 65536 KB
[   12.718384] PCI-DMA: using GART IOMMU.
[   12.722139] PCI-DMA: Reserving 64MB of IOMMU area in the AGP aperture
[   12.736944] initcall pci_iommu_init+0x0/0x31 returned 0 after 28802 usecs

-v2: call shutdown when no agp is there
-v3: add check_store to restore state for swiotlb
     separate pci_swiotlb_detect and pci_swiotlb_init from FUJITA
-v4: disable translating in early_gart_iommu_check according to FUJITA

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/include/asm/swiotlb.h |    8 ++++++--
 arch/x86/kernel/aperture_64.c  |   11 ++++++-----
 arch/x86/kernel/pci-dma.c      |    8 ++++++--
 arch/x86/kernel/pci-gart_64.c  |    3 ++-
 arch/x86/kernel/pci-swiotlb.c  |   11 +++++++----
 5 files changed, 27 insertions(+), 14 deletions(-)

Index: linux-2.6/arch/x86/kernel/aperture_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/aperture_64.c
+++ linux-2.6/arch/x86/kernel/aperture_64.c
@@ -280,7 +280,8 @@ void __init early_gart_iommu_check(void)
 	 * or BIOS forget to put that in reserved.
 	 * try to update e820 to make that region as reserved.
 	 */
-	int i, fix, slot;
+	u32 agp_aper_base = 0, agp_aper_order = 0;
+	int i, fix, slot, valid_agp = 0;
 	u32 ctl;
 	u32 aper_size = 0, aper_order = 0, last_aper_order = 0;
 	u64 aper_base = 0, last_aper_base = 0;
@@ -290,6 +291,8 @@ void __init early_gart_iommu_check(void)
 		return;
 
 	/* This is mostly duplicate of iommu_hole_init */
+	agp_aper_base = search_agp_bridge(&agp_aper_order, &valid_agp);
+
 	fix = 0;
 	for (i = 0; i < ARRAY_SIZE(bus_dev_ranges); i++) {
 		int bus;
@@ -342,10 +345,10 @@ void __init early_gart_iommu_check(void)
 		}
 	}
 
-	if (!fix)
+	if (valid_agp)
 		return;
 
-	/* different nodes have different setting, disable them all at first*/
+	/* disable them all at first */
 	for (i = 0; i < ARRAY_SIZE(bus_dev_ranges); i++) {
 		int bus;
 		int dev_base, dev_limit;
@@ -458,8 +461,6 @@ out:
 
 	if (aper_alloc) {
 		/* Got the aperture from the AGP bridge */
-	} else if (!valid_agp) {
-		/* Do nothing */
 	} else if ((!no_iommu && max_pfn > MAX_DMA32_PFN) ||
 		   force_iommu ||
 		   valid_agp ||
Index: linux-2.6/arch/x86/kernel/pci-dma.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/pci-dma.c
+++ linux-2.6/arch/x86/kernel/pci-dma.c
@@ -124,8 +124,9 @@ void __init pci_iommu_alloc(void)
 	/* free the range so iommu could get some range less than 4G */
 	dma32_free_bootmem();
 #endif
-	if (pci_swiotlb_init())
-		return;
+
+	if (pci_swiotlb_detect())
+		goto out;
 
 	gart_iommu_hole_init();
 
@@ -135,6 +136,9 @@ void __init pci_iommu_alloc(void)
 
 	/* needs to be called after gart_iommu_hole_init */
 	amd_iommu_detect();
+
+out:
+	pci_swiotlb_init();
 }
 
 void *dma_generic_alloc_coherent(struct device *dev, size_t size,
Index: linux-2.6/arch/x86/kernel/pci-gart_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/pci-gart_64.c
+++ linux-2.6/arch/x86/kernel/pci-gart_64.c
@@ -710,7 +710,8 @@ static void gart_iommu_shutdown(void)
 	struct pci_dev *dev;
 	int i;
 
-	if (no_agp)
+	/* don't shutdown it if there is AGP installed */
+	if (!no_agp)
 		return;
 
 	for (i = 0; i < num_k8_northbridges; i++) {
Index: linux-2.6/arch/x86/include/asm/swiotlb.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/swiotlb.h
+++ linux-2.6/arch/x86/include/asm/swiotlb.h
@@ -5,13 +5,17 @@
 
 #ifdef CONFIG_SWIOTLB
 extern int swiotlb;
-extern int pci_swiotlb_init(void);
+int pci_swiotlb_detect(void);
+void pci_swiotlb_init(void);
 #else
 #define swiotlb 0
-static inline int pci_swiotlb_init(void)
+static inline int pci_swiotlb_detect(void)
 {
 	return 0;
 }
+static inline void pci_swiotlb_init(void)
+{
+}
 #endif
 
 static inline void dma_mark_clean(void *addr, size_t size) {}
Index: linux-2.6/arch/x86/kernel/pci-swiotlb.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/pci-swiotlb.c
+++ linux-2.6/arch/x86/kernel/pci-swiotlb.c
@@ -43,12 +43,12 @@ static struct dma_map_ops swiotlb_dma_op
 };
 
 /*
- * pci_swiotlb_init - initialize swiotlb if necessary
+ * pci_swiotlb_detect - initialize swiotlb if necessary
  *
  * This returns non-zero if we are forced to use swiotlb (by the boot
  * option).
  */
-int __init pci_swiotlb_init(void)
+int __init pci_swiotlb_detect(void)
 {
 	int use_swiotlb = swiotlb | swiotlb_force;
 
@@ -60,10 +60,13 @@ int __init pci_swiotlb_init(void)
 	if (swiotlb_force)
 		swiotlb = 1;
 
+	return use_swiotlb;
+}
+
+void __init pci_swiotlb_init(void)
+{
 	if (swiotlb) {
 		swiotlb_init(0);
 		dma_ops = &swiotlb_dma_ops;
 	}
-
-	return use_swiotlb;
 }

Re: [PATCH -tip] x86: fix iommu=soft boot option

[FUJITA Tomonori]

On Mon, 30 Nov 2009 23:42:00 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> FUJITA Tomonori wrote:
> > On Thu, 26 Nov 2009 23:45:33 -0800
> > Yinghai Lu <yinghai@kernel.org> wrote:
> > 
> >>>> when AMD64 mem > 4g, no AGP, BIOS set all gart iommu on all nodes size to 32M, and enable bit are set.
> >>> I have such machine (with sane BIOS).
> >>>
> >>> But if BIOS is broken, early_gart_iommu_check() disables GART_EN bit
> >>> (bits 0)?
> >> not for 32M small size.
> >>
> >> that function only clear that bit when different nodes have different setting.
> >>
> >>>> 1. iommu=soft, will go through POINT A and POINT B
> >>> Not always. if aperture_valid() is true, it doesn't go POINT A. My
> >>> GART machine doesn't go.
> >> maybe your bios set GART iommu set to 64M?
> > 
> > Yeah, my machine has sane BIOS.
> > 
> > 
> >>>> 2. no "iommu=soft", will go through POINT A and POINT C
> >>> As I said, it's not true about POINT A.
> >> maybe your bios set GART iommu set to 64M?
> >>
> >>>
> >>>> and all will reach POINT X to make sure ENABLE bit is not set.
> >>> POINT X doesn't look make sure ENABLE bit is not set. It just fixes up
> >>> (sets) the address and its size.
> >>                         write_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL, aper_order << 1);
> >>
> >> that bit is that bit 0 of AMD64_GARTAPERTURECTL reg.
> > 
> > Oops, thanks.
> > 
> > But as I wrote in the previous mail, can you tell me why we need this?
> > With 2.6.31 my machine uses swiotlb with GART_EN bit enabled.
> > 
> > My another question is that why can't early_gart_iommu_check()
> > _always_ disable GART_EN bit?
> 
> please check

Thanks.

As I said, it looks cleaner if early_gart_iommu_check() disables
GART_EN bit. So this patch looks better but I still have some
questions.


> [PATCH] x86: fix gart iommu using for amd 64 bit system -v4
> 
> after
> |commit 75f1cdf1dda92cae037ec848ae63690d91913eac
> |Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> |Date:   Tue Nov 10 19:46:20 2009 +0900
> |
> |    x86: Handle HW IOMMU initialization failure gracefully
> |    
> |    If HW IOMMU initialization fails (Intel VT-d often does this,
> |    typically due to BIOS bugs), we fall back to nommu. It doesn't
> |    work for the majority since nowadays we have more than 4GB
> |    memory so we must use swiotlb instead of nommu.
> ...
> 
> amd 64 systems that
> 1. do not have  AGP
> 2. do not have IOMMU
> 3. mem > 4g
> 4. BIOS do not allocate  correct gart in NB.
> will leave them to use SWIOTLB forcely.

As I asked earlier, can you tell me what dma ops such system is
supposed to use?


> also in pci_iommu_alloc()
> pci_swiotlb_init is stealing the preallocate range that is for
> gart_iommu_hole workaround.
> 
> so restore the sequence...
> 
> will get back
> [    0.000000] Your BIOS doesn't leave a aperture memory hole
> [    0.000000] Please enable the IOMMU option in the BIOS setup
> [    0.000000] This costs you 64 MB of RAM
> [    0.000000] Mapping aperture over 65536 KB of RAM @ 20000000
> ...
> [   12.702822] calling  pci_iommu_init+0x0/0x31 @ 1
> [   12.708728] PCI-DMA: Disabling AGP.
> [   12.712812] PCI-DMA: aperture base @ 20000000 size 65536 KB
> [   12.718384] PCI-DMA: using GART IOMMU.
> [   12.722139] PCI-DMA: Reserving 64MB of IOMMU area in the AGP aperture
> [   12.736944] initcall pci_iommu_init+0x0/0x31 returned 0 after 28802 usecs
> 
> -v2: call shutdown when no agp is there

Is this change related with my above patchset?


> -v3: add check_store to restore state for swiotlb
>      separate pci_swiotlb_detect and pci_swiotlb_init from FUJITA
> -v4: disable translating in early_gart_iommu_check according to FUJITA

Why can't we always disable translation in early_gart_iommu_check? We
really need search_agp_bridge() in early_gart_iommu_check()?

Re: [PATCH -tip] x86: fix iommu=soft boot option

[Yinghai Lu]

FUJITA Tomonori wrote:
> On Mon, 30 Nov 2009 23:42:00 -0800
> Yinghai Lu <yinghai@kernel.org> wrote:
> 
>> FUJITA Tomonori wrote:
>>> On Thu, 26 Nov 2009 23:45:33 -0800
>>> Yinghai Lu <yinghai@kernel.org> wrote:
>>>
>>>>>> when AMD64 mem > 4g, no AGP, BIOS set all gart iommu on all nodes size to 32M, and enable bit are set.
>>>>> I have such machine (with sane BIOS).
>>>>>
>>>>> But if BIOS is broken, early_gart_iommu_check() disables GART_EN bit
>>>>> (bits 0)?
>>>> not for 32M small size.
>>>>
>>>> that function only clear that bit when different nodes have different setting.
>>>>
>>>>>> 1. iommu=soft, will go through POINT A and POINT B
>>>>> Not always. if aperture_valid() is true, it doesn't go POINT A. My
>>>>> GART machine doesn't go.
>>>> maybe your bios set GART iommu set to 64M?
>>> Yeah, my machine has sane BIOS.
>>>
>>>
>>>>>> 2. no "iommu=soft", will go through POINT A and POINT C
>>>>> As I said, it's not true about POINT A.
>>>> maybe your bios set GART iommu set to 64M?
>>>>
>>>>>> and all will reach POINT X to make sure ENABLE bit is not set.
>>>>> POINT X doesn't look make sure ENABLE bit is not set. It just fixes up
>>>>> (sets) the address and its size.
>>>>                         write_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL, aper_order << 1);
>>>>
>>>> that bit is that bit 0 of AMD64_GARTAPERTURECTL reg.
>>> Oops, thanks.
>>>
>>> But as I wrote in the previous mail, can you tell me why we need this?
>>> With 2.6.31 my machine uses swiotlb with GART_EN bit enabled.
>>>
>>> My another question is that why can't early_gart_iommu_check()
>>> _always_ disable GART_EN bit?
>> please check
> 
> Thanks.
> 
> As I said, it looks cleaner if early_gart_iommu_check() disables
> GART_EN bit. So this patch looks better but I still have some
> questions.
> 
> 
>> [PATCH] x86: fix gart iommu using for amd 64 bit system -v4
>>
>> after
>> |commit 75f1cdf1dda92cae037ec848ae63690d91913eac
>> |Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
>> |Date:   Tue Nov 10 19:46:20 2009 +0900
>> |
>> |    x86: Handle HW IOMMU initialization failure gracefully
>> |    
>> |    If HW IOMMU initialization fails (Intel VT-d often does this,
>> |    typically due to BIOS bugs), we fall back to nommu. It doesn't
>> |    work for the majority since nowadays we have more than 4GB
>> |    memory so we must use swiotlb instead of nommu.
>> ...
>>
>> amd 64 systems that
>> 1. do not have  AGP
>> 2. do not have IOMMU
>> 3. mem > 4g
>> 4. BIOS do not allocate  correct gart in NB.
>> will leave them to use SWIOTLB forcely.
> 
> As I asked earlier, can you tell me what dma ops such system is
> supposed to use?

gart_dma_ops
 
> 
>> also in pci_iommu_alloc()
>> pci_swiotlb_init is stealing the preallocate range that is for
>> gart_iommu_hole workaround.
>>
>> so restore the sequence...
>>
>> will get back
>> [    0.000000] Your BIOS doesn't leave a aperture memory hole
>> [    0.000000] Please enable the IOMMU option in the BIOS setup
>> [    0.000000] This costs you 64 MB of RAM
>> [    0.000000] Mapping aperture over 65536 KB of RAM @ 20000000
>> ...
>> [   12.702822] calling  pci_iommu_init+0x0/0x31 @ 1
>> [   12.708728] PCI-DMA: Disabling AGP.
>> [   12.712812] PCI-DMA: aperture base @ 20000000 size 65536 KB
>> [   12.718384] PCI-DMA: using GART IOMMU.
>> [   12.722139] PCI-DMA: Reserving 64MB of IOMMU area in the AGP aperture
>> [   12.736944] initcall pci_iommu_init+0x0/0x31 returned 0 after 28802 usecs
>>
>> -v2: call shutdown when no agp is there
> 
> Is this change related with my above patchset?

looks like one bug that is exposed by
commit 338bac527ed0e35b4cb50390972f15d3cbce92ca
Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Date:   Tue Oct 27 16:34:44 2009 +0900

    x86: Use x86_platform for iommu_shutdown
    
    This patch cleans up pci_iommu_shutdown() a bit to use
    x86_platform (similar to how IA64 initializes an IOMMU driver).
    
    This adds iommu_shutdown() to x86_platform to avoid calling
    every IOMMUs' shutdown functions in pci_iommu_shutdown() in
    order. The IOMMU shutdown functions are platform specific (we
    don't have multiple different IOMMU hardware) so the current way
    is pointless.
    
    An IOMMU driver sets x86_platform.iommu_shutdown to the shutdown
    function if necessary.
    
    Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
    Cc: joerg.roedel@amd.com
    LKML-Reference: <20091027163358F.fujita.tomonori@lab.ntt.co.jp>
    Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index a7f1b64..a9bcdf7 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -39,6 +39,7 @@
 #include <asm/swiotlb.h>
 #include <asm/dma.h>
 #include <asm/k8.h>
+#include <asm/x86_init.h>
 
 static unsigned long iommu_bus_base;   /* GART remapping area (physical) */
 static unsigned long iommu_size;       /* size of remapping area bytes */
@@ -688,12 +689,12 @@ static struct dma_map_ops gart_dma_ops = {
        .free_coherent                  = gart_free_coherent,
 };
 
-void gart_iommu_shutdown(void)
+static void gart_iommu_shutdown(void)
 {
        struct pci_dev *dev;
        int i;
 
-       if (no_agp && (dma_ops != &gart_dma_ops))
+       if (no_agp)
                return;
 
        for (i = 0; i < num_k8_northbridges; i++) {
@@ -838,6 +839,7 @@ void __init gart_iommu_init(void)
 
        flush_gart();
        dma_ops = &gart_dma_ops;
+       x86_platform.iommu_shutdown = gart_iommu_shutdown;
 }
 
 void __init gart_parse_options(char *p)

on system without agp, the gart should be shutdown during system shutdown. so make next kexeced kernel happy.

> 
> 
>> -v3: add check_store to restore state for swiotlb
>>      separate pci_swiotlb_detect and pci_swiotlb_init from FUJITA
>> -v4: disable translating in early_gart_iommu_check according to FUJITA
> 
> Why can't we always disable translation in early_gart_iommu_check? We
> really need search_agp_bridge() in early_gart_iommu_check()?

just don't want to messed them up.

YH

Re: [PATCH -tip] x86: fix iommu=soft boot option

[FUJITA Tomonori]

On Tue, 01 Dec 2009 22:57:37 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> FUJITA Tomonori wrote:
> > On Mon, 30 Nov 2009 23:42:00 -0800
> > Yinghai Lu <yinghai@kernel.org> wrote:
> > 
> >> FUJITA Tomonori wrote:
> >>> On Thu, 26 Nov 2009 23:45:33 -0800
> >>> Yinghai Lu <yinghai@kernel.org> wrote:
> >>>
> >>>>>> when AMD64 mem > 4g, no AGP, BIOS set all gart iommu on all nodes size to 32M, and enable bit are set.
> >>>>> I have such machine (with sane BIOS).
> >>>>>
> >>>>> But if BIOS is broken, early_gart_iommu_check() disables GART_EN bit
> >>>>> (bits 0)?
> >>>> not for 32M small size.
> >>>>
> >>>> that function only clear that bit when different nodes have different setting.
> >>>>
> >>>>>> 1. iommu=soft, will go through POINT A and POINT B
> >>>>> Not always. if aperture_valid() is true, it doesn't go POINT A. My
> >>>>> GART machine doesn't go.
> >>>> maybe your bios set GART iommu set to 64M?
> >>> Yeah, my machine has sane BIOS.
> >>>
> >>>
> >>>>>> 2. no "iommu=soft", will go through POINT A and POINT C
> >>>>> As I said, it's not true about POINT A.
> >>>> maybe your bios set GART iommu set to 64M?
> >>>>
> >>>>>> and all will reach POINT X to make sure ENABLE bit is not set.
> >>>>> POINT X doesn't look make sure ENABLE bit is not set. It just fixes up
> >>>>> (sets) the address and its size.
> >>>>                         write_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL, aper_order << 1);
> >>>>
> >>>> that bit is that bit 0 of AMD64_GARTAPERTURECTL reg.
> >>> Oops, thanks.
> >>>
> >>> But as I wrote in the previous mail, can you tell me why we need this?
> >>> With 2.6.31 my machine uses swiotlb with GART_EN bit enabled.
> >>>
> >>> My another question is that why can't early_gart_iommu_check()
> >>> _always_ disable GART_EN bit?
> >> please check
> > 
> > Thanks.
> > 
> > As I said, it looks cleaner if early_gart_iommu_check() disables
> > GART_EN bit. So this patch looks better but I still have some
> > questions.
> > 
> > 
> >> [PATCH] x86: fix gart iommu using for amd 64 bit system -v4
> >>
> >> after
> >> |commit 75f1cdf1dda92cae037ec848ae63690d91913eac
> >> |Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> >> |Date:   Tue Nov 10 19:46:20 2009 +0900
> >> |
> >> |    x86: Handle HW IOMMU initialization failure gracefully
> >> |    
> >> |    If HW IOMMU initialization fails (Intel VT-d often does this,
> >> |    typically due to BIOS bugs), we fall back to nommu. It doesn't
> >> |    work for the majority since nowadays we have more than 4GB
> >> |    memory so we must use swiotlb instead of nommu.
> >> ...
> >>
> >> amd 64 systems that
> >> 1. do not have  AGP
> >> 2. do not have IOMMU
> >> 3. mem > 4g
> >> 4. BIOS do not allocate  correct gart in NB.
> >> will leave them to use SWIOTLB forcely.
> > 
> > As I asked earlier, can you tell me what dma ops such system is
> > supposed to use?
> 
> gart_dma_ops

How does gart_dma_ops work on systems without IOMMU?


> >> -v2: call shutdown when no agp is there
> > 
> > Is this change related with my above patchset?
> 
> looks like one bug that is exposed by
> commit 338bac527ed0e35b4cb50390972f15d3cbce92ca
> Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Date:   Tue Oct 27 16:34:44 2009 +0900
> 
>     x86: Use x86_platform for iommu_shutdown
>     
>     This patch cleans up pci_iommu_shutdown() a bit to use
>     x86_platform (similar to how IA64 initializes an IOMMU driver).
>     
>     This adds iommu_shutdown() to x86_platform to avoid calling
>     every IOMMUs' shutdown functions in pci_iommu_shutdown() in
>     order. The IOMMU shutdown functions are platform specific (we
>     don't have multiple different IOMMU hardware) so the current way
>     is pointless.
>     
>     An IOMMU driver sets x86_platform.iommu_shutdown to the shutdown
>     function if necessary.
>     
>     Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
>     Cc: joerg.roedel@amd.com
>     LKML-Reference: <20091027163358F.fujita.tomonori@lab.ntt.co.jp>
>     Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 
> diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
> index a7f1b64..a9bcdf7 100644
> --- a/arch/x86/kernel/pci-gart_64.c
> +++ b/arch/x86/kernel/pci-gart_64.c
> @@ -39,6 +39,7 @@
>  #include <asm/swiotlb.h>
>  #include <asm/dma.h>
>  #include <asm/k8.h>
> +#include <asm/x86_init.h>
>  
>  static unsigned long iommu_bus_base;   /* GART remapping area (physical) */
>  static unsigned long iommu_size;       /* size of remapping area bytes */
> @@ -688,12 +689,12 @@ static struct dma_map_ops gart_dma_ops = {
>         .free_coherent                  = gart_free_coherent,
>  };
>  
> -void gart_iommu_shutdown(void)
> +static void gart_iommu_shutdown(void)
>  {
>         struct pci_dev *dev;
>         int i;
>  
> -       if (no_agp && (dma_ops != &gart_dma_ops))
> +       if (no_agp)
>                 return;
>  
>         for (i = 0; i < num_k8_northbridges; i++) {
> @@ -838,6 +839,7 @@ void __init gart_iommu_init(void)
>  
>         flush_gart();
>         dma_ops = &gart_dma_ops;
> +       x86_platform.iommu_shutdown = gart_iommu_shutdown;
>  }
>  
>  void __init gart_parse_options(char *p)
> 
> on system without agp, the gart should be shutdown during system shutdown. so make next kexeced kernel happy.

Ah, I see. Sorry about this mess-up.


> >> -v3: add check_store to restore state for swiotlb
> >>      separate pci_swiotlb_detect and pci_swiotlb_init from FUJITA
> >> -v4: disable translating in early_gart_iommu_check according to FUJITA
> > 
> > Why can't we always disable translation in early_gart_iommu_check? We
> > really need search_agp_bridge() in early_gart_iommu_check()?
> 
> just don't want to messed them up.

Sorry, I can't follow you. What does 'always disabling' mess up?

Re: [PATCH -tip] x86: fix iommu=soft boot option

[Yinghai Lu]

FUJITA Tomonori wrote:
> On Tue, 01 Dec 2009 22:57:37 -0800
> Yinghai Lu <yinghai@kernel.org> wrote:
> 
>> FUJITA Tomonori wrote:
>>> On Mon, 30 Nov 2009 23:42:00 -0800
>>> Yinghai Lu <yinghai@kernel.org> wrote:
>>>
>>>> FUJITA Tomonori wrote:
>>>>> On Thu, 26 Nov 2009 23:45:33 -0800
>>>>> Yinghai Lu <yinghai@kernel.org> wrote:
>>>>>
>>>>>>>> when AMD64 mem > 4g, no AGP, BIOS set all gart iommu on all nodes size to 32M, and enable bit are set.
>>>>>>> I have such machine (with sane BIOS).
>>>>>>>
>>>>>>> But if BIOS is broken, early_gart_iommu_check() disables GART_EN bit
>>>>>>> (bits 0)?
>>>>>> not for 32M small size.
>>>>>>
>>>>>> that function only clear that bit when different nodes have different setting.
>>>>>>
>>>>>>>> 1. iommu=soft, will go through POINT A and POINT B
>>>>>>> Not always. if aperture_valid() is true, it doesn't go POINT A. My
>>>>>>> GART machine doesn't go.
>>>>>> maybe your bios set GART iommu set to 64M?
>>>>> Yeah, my machine has sane BIOS.
>>>>>
>>>>>
>>>>>>>> 2. no "iommu=soft", will go through POINT A and POINT C
>>>>>>> As I said, it's not true about POINT A.
>>>>>> maybe your bios set GART iommu set to 64M?
>>>>>>
>>>>>>>> and all will reach POINT X to make sure ENABLE bit is not set.
>>>>>>> POINT X doesn't look make sure ENABLE bit is not set. It just fixes up
>>>>>>> (sets) the address and its size.
>>>>>>                         write_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL, aper_order << 1);
>>>>>>
>>>>>> that bit is that bit 0 of AMD64_GARTAPERTURECTL reg.
>>>>> Oops, thanks.
>>>>>
>>>>> But as I wrote in the previous mail, can you tell me why we need this?
>>>>> With 2.6.31 my machine uses swiotlb with GART_EN bit enabled.
>>>>>
>>>>> My another question is that why can't early_gart_iommu_check()
>>>>> _always_ disable GART_EN bit?
>>>> please check
>>> Thanks.
>>>
>>> As I said, it looks cleaner if early_gart_iommu_check() disables
>>> GART_EN bit. So this patch looks better but I still have some
>>> questions.
>>>
>>>
>>>> [PATCH] x86: fix gart iommu using for amd 64 bit system -v4
>>>>
>>>> after
>>>> |commit 75f1cdf1dda92cae037ec848ae63690d91913eac
>>>> |Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
>>>> |Date:   Tue Nov 10 19:46:20 2009 +0900
>>>> |
>>>> |    x86: Handle HW IOMMU initialization failure gracefully
>>>> |    
>>>> |    If HW IOMMU initialization fails (Intel VT-d often does this,
>>>> |    typically due to BIOS bugs), we fall back to nommu. It doesn't
>>>> |    work for the majority since nowadays we have more than 4GB
>>>> |    memory so we must use swiotlb instead of nommu.
>>>> ...
>>>>
>>>> amd 64 systems that
>>>> 1. do not have  AGP
>>>> 2. do not have IOMMU
>>>> 3. mem > 4g
>>>> 4. BIOS do not allocate  correct gart in NB.
>>>> will leave them to use SWIOTLB forcely.
>>> As I asked earlier, can you tell me what dma ops such system is
>>> supposed to use?
>> gart_dma_ops
> 
> How does gart_dma_ops work on systems without IOMMU?

?

> 
> 
>>>> -v2: call shutdown when no agp is there
>>> Is this change related with my above patchset?
>> looks like one bug that is exposed by
>> commit 338bac527ed0e35b4cb50390972f15d3cbce92ca
>> Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
>> Date:   Tue Oct 27 16:34:44 2009 +0900
>>
>>     x86: Use x86_platform for iommu_shutdown
>>     
>>     This patch cleans up pci_iommu_shutdown() a bit to use
>>     x86_platform (similar to how IA64 initializes an IOMMU driver).
>>     
>>     This adds iommu_shutdown() to x86_platform to avoid calling
>>     every IOMMUs' shutdown functions in pci_iommu_shutdown() in
>>     order. The IOMMU shutdown functions are platform specific (we
>>     don't have multiple different IOMMU hardware) so the current way
>>     is pointless.
>>     
>>     An IOMMU driver sets x86_platform.iommu_shutdown to the shutdown
>>     function if necessary.
>>     
>>     Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
>>     Cc: joerg.roedel@amd.com
>>     LKML-Reference: <20091027163358F.fujita.tomonori@lab.ntt.co.jp>
>>     Signed-off-by: Ingo Molnar <mingo@elte.hu>
>>
>> diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
>> index a7f1b64..a9bcdf7 100644
>> --- a/arch/x86/kernel/pci-gart_64.c
>> +++ b/arch/x86/kernel/pci-gart_64.c
>> @@ -39,6 +39,7 @@
>>  #include <asm/swiotlb.h>
>>  #include <asm/dma.h>
>>  #include <asm/k8.h>
>> +#include <asm/x86_init.h>
>>  
>>  static unsigned long iommu_bus_base;   /* GART remapping area (physical) */
>>  static unsigned long iommu_size;       /* size of remapping area bytes */
>> @@ -688,12 +689,12 @@ static struct dma_map_ops gart_dma_ops = {
>>         .free_coherent                  = gart_free_coherent,
>>  };
>>  
>> -void gart_iommu_shutdown(void)
>> +static void gart_iommu_shutdown(void)
>>  {
>>         struct pci_dev *dev;
>>         int i;
>>  
>> -       if (no_agp && (dma_ops != &gart_dma_ops))
>> +       if (no_agp)
>>                 return;
>>  
>>         for (i = 0; i < num_k8_northbridges; i++) {
>> @@ -838,6 +839,7 @@ void __init gart_iommu_init(void)
>>  
>>         flush_gart();
>>         dma_ops = &gart_dma_ops;
>> +       x86_platform.iommu_shutdown = gart_iommu_shutdown;
>>  }
>>  
>>  void __init gart_parse_options(char *p)
>>
>> on system without agp, the gart should be shutdown during system shutdown. so make next kexeced kernel happy.
> 
> Ah, I see. Sorry about this mess-up.
> 
> 
>>>> -v3: add check_store to restore state for swiotlb
>>>>      separate pci_swiotlb_detect and pci_swiotlb_init from FUJITA
>>>> -v4: disable translating in early_gart_iommu_check according to FUJITA
>>> Why can't we always disable translation in early_gart_iommu_check? We
>>> really need search_agp_bridge() in early_gart_iommu_check()?
>> just don't want to messed them up.
> 
> Sorry, I can't follow you. What does 'always disabling' mess up?

the AMD K8 system with AGP etc...those systems are some kind of 5 years old.

and don't have those kind of system to verify...

YH

Re: [PATCH -tip] x86: fix iommu=soft boot option

[FUJITA Tomonori]

On Tue, 01 Dec 2009 23:55:28 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> >>>> amd 64 systems that
> >>>> 1. do not have  AGP
> >>>> 2. do not have IOMMU
> >>>> 3. mem > 4g
> >>>> 4. BIOS do not allocate  correct gart in NB.
> >>>> will leave them to use SWIOTLB forcely.
> >>> As I asked earlier, can you tell me what dma ops such system is
> >>> supposed to use?
> >> gart_dma_ops
> > 
> > How does gart_dma_ops work on systems without IOMMU?
> 
> ?

If a system doesn't have IOMMU, what does gart_dma_ops do?


> >>>> -v4: disable translating in early_gart_iommu_check according to FUJITA
> >>> Why can't we always disable translation in early_gart_iommu_check? We
> >>> really need search_agp_bridge() in early_gart_iommu_check()?
> >> just don't want to messed them up.
> > 
> > Sorry, I can't follow you. What does 'always disabling' mess up?
> 
> the AMD K8 system with AGP etc...those systems are some kind of 5 years old.
> 
> and don't have those kind of system to verify...

Ok.

Re: [PATCH -tip] x86: fix iommu=soft boot option

[FUJITA Tomonori]

On Wed, 2 Dec 2009 17:07:24 +0900
FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> On Tue, 01 Dec 2009 23:55:28 -0800
> Yinghai Lu <yinghai@kernel.org> wrote:
> 
> > >>>> amd 64 systems that
> > >>>> 1. do not have  AGP
> > >>>> 2. do not have IOMMU
> > >>>> 3. mem > 4g
> > >>>> 4. BIOS do not allocate  correct gart in NB.
> > >>>> will leave them to use SWIOTLB forcely.
> > >>> As I asked earlier, can you tell me what dma ops such system is
> > >>> supposed to use?
> > >> gart_dma_ops
> > > 
> > > How does gart_dma_ops work on systems without IOMMU?
> > 
> > ?
> 
> If a system doesn't have IOMMU, what does gart_dma_ops do?

Ping?

I don't understand the above but the patch is fine about fixing two
things:

- swiotlb wrongly steals the preallocate memory for broken gart.

- a system without agp should disable gart on shutdown.

Re: [PATCH -tip] x86: fix iommu=soft boot option

[Yinghai Lu]

FUJITA Tomonori wrote:
> On Wed, 2 Dec 2009 17:07:24 +0900
> FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
> 
>> On Tue, 01 Dec 2009 23:55:28 -0800
>> Yinghai Lu <yinghai@kernel.org> wrote:
>>
>>>>>>> amd 64 systems that
>>>>>>> 1. do not have  AGP
>>>>>>> 2. do not have IOMMU
>>>>>>> 3. mem > 4g
>>>>>>> 4. BIOS do not allocate  correct gart in NB.
>>>>>>> will leave them to use SWIOTLB forcely.
>>>>>> As I asked earlier, can you tell me what dma ops such system is
>>>>>> supposed to use?
>>>>> gart_dma_ops
>>>> How does gart_dma_ops work on systems without IOMMU?
>>> ?
>> If a system doesn't have IOMMU, what does gart_dma_ops do?
> 
> Ping?

gart can do hw translation...

YH