[PATCH] x86: always define DECLARE_PCI_UNMAP* macros

[PATCH] x86: always define DECLARE_PCI_UNMAP* macros

[Joerg Roedel]

Currently these macros evaluate to a no-op except the kernel is compiled
with GART or Calgary support. But we also need these macros when we have
SWIOTLB, VT-d or AMD IOMMU in the kernel. Since we always compile at
least with SWIOTLB we can define these macros always.
This patch is also for stable backport for the same reason the SWIOTLB
default selection patch is.

Cc: stable@kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/pci_64.h |   14 --------------
 1 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/pci_64.h b/arch/x86/include/asm/pci_64.h
index 5b28995..d02d936 100644
--- a/arch/x86/include/asm/pci_64.h
+++ b/arch/x86/include/asm/pci_64.h
@@ -34,8 +34,6 @@ extern void pci_iommu_alloc(void);
  */
 #define PCI_DMA_BUS_IS_PHYS (dma_ops->is_phys)
 
-#if defined(CONFIG_GART_IOMMU) || defined(CONFIG_CALGARY_IOMMU)
-
 #define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME)	\
 	dma_addr_t ADDR_NAME;
 #define DECLARE_PCI_UNMAP_LEN(LEN_NAME)		\
@@ -49,18 +47,6 @@ extern void pci_iommu_alloc(void);
 #define pci_unmap_len_set(PTR, LEN_NAME, VAL)		\
 	(((PTR)->LEN_NAME) = (VAL))
 
-#else
-/* No IOMMU */
-
-#define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME)
-#define DECLARE_PCI_UNMAP_LEN(LEN_NAME)
-#define pci_unmap_addr(PTR, ADDR_NAME)		(0)
-#define pci_unmap_addr_set(PTR, ADDR_NAME, VAL)	do { } while (0)
-#define pci_unmap_len(PTR, LEN_NAME)		(0)
-#define pci_unmap_len_set(PTR, LEN_NAME, VAL)	do { } while (0)
-
-#endif
-
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_X86_PCI_64_H */
-- 
1.5.6.4

Re: [PATCH] x86: always define DECLARE_PCI_UNMAP* macros

[Ingo Molnar]

* Joerg Roedel <joerg.roedel@amd.com> wrote:

> Currently these macros evaluate to a no-op except the kernel is 
> compiled with GART or Calgary support. But we also need these macros 
> when we have SWIOTLB, VT-d or AMD IOMMU in the kernel. Since we 
> always compile at least with SWIOTLB we can define these macros 
> always. This patch is also for stable backport for the same reason 
> the SWIOTLB default selection patch is.
> 
> Cc: stable@kernel.org
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  arch/x86/include/asm/pci_64.h |   14 --------------
>  1 files changed, 0 insertions(+), 14 deletions(-)

applied to tip/x86/urgent (for v2.6.28), as this is a DMA mapping 
bugfix for IOMMUs. If the AMD IOMMU is turned on in the .config but 
CONFIG_GART_IOMMU is off (a rare but possible combo), we'd leak on 
unmap and crash the box quickly.

Jesse, is that path of upstream merge fine with you?

	Ingo

Re: [PATCH] x86: always define DECLARE_PCI_UNMAP* macros

[Jesse Barnes]

On Thursday, November 27, 2008 3:45 am Ingo Molnar wrote:
> * Joerg Roedel <joerg.roedel@amd.com> wrote:
> > Currently these macros evaluate to a no-op except the kernel is
> > compiled with GART or Calgary support. But we also need these macros
> > when we have SWIOTLB, VT-d or AMD IOMMU in the kernel. Since we
> > always compile at least with SWIOTLB we can define these macros
> > always. This patch is also for stable backport for the same reason
> > the SWIOTLB default selection patch is.
> >
> > Cc: stable@kernel.org
> > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > ---
> >  arch/x86/include/asm/pci_64.h |   14 --------------
> >  1 files changed, 0 insertions(+), 14 deletions(-)
>
> applied to tip/x86/urgent (for v2.6.28), as this is a DMA mapping
> bugfix for IOMMUs. If the AMD IOMMU is turned on in the .config but
> CONFIG_GART_IOMMU is off (a rare but possible combo), we'd leak on
> unmap and crash the box quickly.
>
> Jesse, is that path of upstream merge fine with you?

Yeah, that's fine with me.  OTOH removing config options generally seems like 
a good idea too, so maybe this combination should just be disallowed.  Or is 
there a good reason for this type of config?

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] x86: always define DECLARE_PCI_UNMAP* macros

[Ingo Molnar]

* Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Thursday, November 27, 2008 3:45 am Ingo Molnar wrote:
> > * Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > Currently these macros evaluate to a no-op except the kernel is
> > > compiled with GART or Calgary support. But we also need these macros
> > > when we have SWIOTLB, VT-d or AMD IOMMU in the kernel. Since we
> > > always compile at least with SWIOTLB we can define these macros
> > > always. This patch is also for stable backport for the same reason
> > > the SWIOTLB default selection patch is.
> > >
> > > Cc: stable@kernel.org
> > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > ---
> > >  arch/x86/include/asm/pci_64.h |   14 --------------
> > >  1 files changed, 0 insertions(+), 14 deletions(-)
> >
> > applied to tip/x86/urgent (for v2.6.28), as this is a DMA mapping 
> > bugfix for IOMMUs. If the AMD IOMMU is turned on in the .config but 
> > CONFIG_GART_IOMMU is off (a rare but possible combo), we'd leak on 
> > unmap and crash the box quickly.
> >
> > Jesse, is that path of upstream merge fine with you?
> 
> Yeah, that's fine with me.  OTOH removing config options generally 
> seems like a good idea too, so maybe this combination should just be 
> disallowed.  Or is there a good reason for this type of config?

well, it's an IOMMU driver, so it's a kernel sizing/configuration thing, 
in line with how we generally allow drivers - with no particular 
dependency between them.

	Ingo