Here is the output of discussion with Yinghai Lu. This patchset fixes
GART isses due to the commit 75f1cdf1dda92cae037ec848ae63690d91913eac
(x86: Handle HW IOMMU initialization failure gracefully).
I split Yinghai's v4 patch to make the changes clearer (and fixed the
comment a bit) however there is no functional change:
http://marc.info/?l=linux-kernel&m=125965342214387&w=2
The first patch is a sligtly modified version of my patch to move back
swiotlb initialization after IOMMU detection:
http://marc.info/?l=linux-kernel&m=125907069703595&w=2
I put the detailed descriptions to both.
The commit 75f1cdf1dda92cae037ec848ae63690d91913eac moved SWIOTLB
initialization before IOMMU detection since the majority of IOMMU
drivers need SWIOTLB even if we find them. And the code is cleaner
if we initialize SWIOTLB earlier.
The problem is that we initialize SWIOTLB after dma32_free_bootmem so
we wrongly steal memory area allocated for GART with broken BIOS
earlier.
We could initialize SWIOTLB before dma32_free_bootmem however it means
that we temporarily allocate more memory during booting. It could
break systems that don't have lots of memory. So unfortunately it
seems that we need to allocate SWIOTLB after IOMMU detection.
With this patch, the x86 IOMMU initialization sequence are:
1. We set swiotlb to 1 in the case of (max_pfn > MAX_DMA32_PFN &&
!no_iommu). If swiotlb usage is forced by the boot option, we go to
the step 3 and finish (we don't try to detect IOMMUs).
2. We call the detection functions of all the IOMMUs. The detection
function sets x86_init.iommu.iommu_init to the IOMMU initialization
function (so we can avoid calling the initialization functions of
all the IOMMUs needlessly).
3. We initialize swiotlb (and set dma_ops to swiotlb_dma_ops) if
swiotlb is set to 1.
4. If the IOMMU initialization function doesn't need swiotlb (e.g. the
initialization is sucessful) then sets swiotlb to zero.
5. If we find that swiotlb is set to zero, we free swiotlb
resource.
Signed-off-by: FUJITA Tomonori <[email protected]>
---
arch/x86/include/asm/swiotlb.h | 8 ++++++--
arch/x86/kernel/pci-dma.c | 6 ++++--
arch/x86/kernel/pci-swiotlb.c | 11 +++++++----
3 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/swiotlb.h b/arch/x86/include/asm/swiotlb.h
index 87ffcb1..8085277 100644
--- a/arch/x86/include/asm/swiotlb.h
+++ b/arch/x86/include/asm/swiotlb.h
@@ -5,13 +5,17 @@
#ifdef CONFIG_SWIOTLB
extern int swiotlb;
-extern int pci_swiotlb_init(void);
+extern int __init pci_swiotlb_detect(void);
+extern void __init 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) {}
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index afcc58b..75e14e2 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -124,8 +124,8 @@ 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 +135,8 @@ 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,
diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index e3c0a66..7d2829d 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -43,12 +43,12 @@ static struct dma_map_ops swiotlb_dma_ops = {
};
/*
- * pci_swiotlb_init - initialize swiotlb if necessary
+ * pci_swiotlb_detect - set swiotlb to 1 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;
}
--
1.5.6.5
From: Yinghai Lu <[email protected]>
This fixes the following breakage of the commit
75f1cdf1dda92cae037ec848ae63690d91913eac:
- GART systems that don't AGP with broken BIOS and more than 4GB
memory are forced to use swiotlb. They can allocate aperture by hand
and use GART.
- GART systems without GAP must disable GART on shutdown.
- swiotlb usage is forced by the boot option, gart_iommu_hole_init()
is not called. so we disable GART early_gart_iommu_check().
Signed-off-by: Yinghai Lu <[email protected]>
Signed-off-by: FUJITA Tomonori <[email protected]>
---
arch/x86/kernel/aperture_64.c | 11 ++++++-----
arch/x86/kernel/pci-gart_64.c | 3 ++-
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
index e0dfb68..3704997 100644
--- a/arch/x86/kernel/aperture_64.c
+++ b/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 ||
diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index e6a0d40..56c0e73 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/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++) {
--
1.5.6.5
On Tue, 8 Dec 2009 20:04:57 +0900
FUJITA Tomonori <[email protected]> wrote:
> Here is the output of discussion with Yinghai Lu. This patchset fixes
> GART isses due to the commit 75f1cdf1dda92cae037ec848ae63690d91913eac
> (x86: Handle HW IOMMU initialization failure gracefully).
>
> I split Yinghai's v4 patch to make the changes clearer (and fixed the
> comment a bit) however there is no functional change:
>
> http://marc.info/?l=linux-kernel&m=125965342214387&w=2
>
> The first patch is a sligtly modified version of my patch to move back
> swiotlb initialization after IOMMU detection:
>
> http://marc.info/?l=linux-kernel&m=125907069703595&w=2
>
> I put the detailed descriptions to both.
Sorry, the description in the first patch is wrong.
I'll retry tomorrow. Please discard this.