linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ia64: SN specific version of dma_get_required_mask()
@ 2008-11-17 16:24 John Keller
  2008-11-17 16:39 ` Bernhard Walle
  2008-11-19  4:07 ` FUJITA Tomonori
  0 siblings, 2 replies; 12+ messages in thread
From: John Keller @ 2008-11-17 16:24 UTC (permalink / raw)
  To: linux-ia64; +Cc: linux-kernel, John Keller

Create a platform specific version of dma_get_required_mask()
for ia64 SN Altix. All SN Altix platforms support 64 bit DMA
addressing regardless of the size of system memory.
Create a ia64 machvec for dma_get_required_mask, with the
SN version unconditionally returning DMA_64BIT_MASK.


Signed-off-by: John Keller <jpk@sgi.com>
---

 Documentation/DMA-API.txt            |    9 ++++-----
 arch/ia64/include/asm/dma-mapping.h  |    3 +++
 arch/ia64/include/asm/machvec.h      |    7 +++++++
 arch/ia64/include/asm/machvec_init.h |    1 +
 arch/ia64/include/asm/machvec_sn2.h  |    2 ++
 arch/ia64/pci/pci.c                  |   21 +++++++++++++++++++++
 arch/ia64/sn/pci/pci_dma.c           |    6 ++++++
 include/linux/dma-mapping.h          |    2 ++
 8 files changed, 46 insertions(+), 5 deletions(-)


Index: linux-2.6/arch/ia64/include/asm/machvec_sn2.h
===================================================================
--- linux-2.6.orig/arch/ia64/include/asm/machvec_sn2.h	2008-11-17 08:38:00.763580614 -0600
+++ linux-2.6/arch/ia64/include/asm/machvec_sn2.h	2008-11-17 08:39:50.473272192 -0600
@@ -67,6 +67,7 @@ extern ia64_mv_dma_sync_single_for_devic
 extern ia64_mv_dma_sync_sg_for_device	sn_dma_sync_sg_for_device;
 extern ia64_mv_dma_mapping_error	sn_dma_mapping_error;
 extern ia64_mv_dma_supported		sn_dma_supported;
+extern ia64_mv_dma_get_required_mask	sn_dma_get_required_mask;
 extern ia64_mv_migrate_t		sn_migrate;
 extern ia64_mv_kernel_launch_event_t	sn_kernel_launch_event;
 extern ia64_mv_setup_msi_irq_t		sn_setup_msi_irq;
@@ -123,6 +124,7 @@ extern ia64_mv_pci_fixup_bus_t		sn_pci_f
 #define platform_dma_sync_sg_for_device	sn_dma_sync_sg_for_device
 #define platform_dma_mapping_error		sn_dma_mapping_error
 #define platform_dma_supported		sn_dma_supported
+#define platform_dma_get_required_mask	sn_dma_get_required_mask
 #define platform_migrate		sn_migrate
 #define platform_kernel_launch_event    sn_kernel_launch_event
 #ifdef CONFIG_PCI_MSI
Index: linux-2.6/Documentation/DMA-API.txt
===================================================================
--- linux-2.6.orig/Documentation/DMA-API.txt	2008-11-17 08:37:57.963231137 -0600
+++ linux-2.6/Documentation/DMA-API.txt	2008-11-17 08:39:50.481273191 -0600
@@ -170,16 +170,15 @@ Returns: 0 if successful and a negative 
 u64
 dma_get_required_mask(struct device *dev)
 
-After setting the mask with dma_set_mask(), this API returns the
-actual mask (within that already set) that the platform actually
-requires to operate efficiently.  Usually this means the returned mask
+This API returns the mask that the platform requires to
+operate efficiently.  Usually this means the returned mask
 is the minimum required to cover all of memory.  Examining the
 required mask gives drivers with variable descriptor sizes the
 opportunity to use smaller descriptors as necessary.
 
 Requesting the required mask does not alter the current mask.  If you
-wish to take advantage of it, you should issue another dma_set_mask()
-call to lower the mask again.
+wish to take advantage of it, you should issue a dma_set_mask()
+call to set the mask to the value returned.
 
 
 Part Id - Streaming DMA mappings
Index: linux-2.6/arch/ia64/include/asm/dma-mapping.h
===================================================================
--- linux-2.6.orig/arch/ia64/include/asm/dma-mapping.h	2008-11-17 08:38:00.755579616 -0600
+++ linux-2.6/arch/ia64/include/asm/dma-mapping.h	2008-11-17 08:39:50.497275188 -0600
@@ -103,6 +103,9 @@ static inline void dma_unmap_sg(struct d
 #define dma_unmap_page(dev, dma_addr, size, dir)			\
 	dma_unmap_single(dev, dma_addr, size, dir)
 
+#define ARCH_HAS_DMA_GET_REQUIRED_MASK
+#define dma_get_required_mask	platform_dma_get_required_mask
+
 /*
  * Rest of this file is part of the "Advanced DMA API".  Use at your own risk.
  * See Documentation/DMA-API.txt for details.
Index: linux-2.6/arch/ia64/include/asm/machvec.h
===================================================================
--- linux-2.6.orig/arch/ia64/include/asm/machvec.h	2008-11-17 08:38:00.763580614 -0600
+++ linux-2.6/arch/ia64/include/asm/machvec.h	2008-11-17 08:39:50.517277684 -0600
@@ -62,6 +62,7 @@ typedef dma_addr_t ia64_mv_dma_map_singl
 typedef void ia64_mv_dma_unmap_single_attrs (struct device *, dma_addr_t, size_t, int, struct dma_attrs *);
 typedef int ia64_mv_dma_map_sg_attrs (struct device *, struct scatterlist *, int, int, struct dma_attrs *);
 typedef void ia64_mv_dma_unmap_sg_attrs (struct device *, struct scatterlist *, int, int, struct dma_attrs *);
+typedef u64 ia64_mv_dma_get_required_mask (struct device *);
 
 /*
  * WARNING: The legacy I/O space is _architected_.  Platforms are
@@ -159,6 +160,7 @@ extern void machvec_tlb_migrate_finish (
 #  define platform_dma_sync_sg_for_device ia64_mv.dma_sync_sg_for_device
 #  define platform_dma_mapping_error		ia64_mv.dma_mapping_error
 #  define platform_dma_supported	ia64_mv.dma_supported
+#  define platform_dma_get_required_mask ia64_mv.dma_get_required_mask
 #  define platform_irq_to_vector	ia64_mv.irq_to_vector
 #  define platform_local_vector_to_irq	ia64_mv.local_vector_to_irq
 #  define platform_pci_get_legacy_mem	ia64_mv.pci_get_legacy_mem
@@ -213,6 +215,7 @@ struct ia64_machine_vector {
 	ia64_mv_dma_sync_sg_for_device *dma_sync_sg_for_device;
 	ia64_mv_dma_mapping_error *dma_mapping_error;
 	ia64_mv_dma_supported *dma_supported;
+	ia64_mv_dma_get_required_mask *dma_get_required_mask;
 	ia64_mv_irq_to_vector *irq_to_vector;
 	ia64_mv_local_vector_to_irq *local_vector_to_irq;
 	ia64_mv_pci_get_legacy_mem_t *pci_get_legacy_mem;
@@ -263,6 +266,7 @@ struct ia64_machine_vector {
 	platform_dma_sync_sg_for_device,	\
 	platform_dma_mapping_error,			\
 	platform_dma_supported,			\
+	platform_dma_get_required_mask,		\
 	platform_irq_to_vector,			\
 	platform_local_vector_to_irq,		\
 	platform_pci_get_legacy_mem,		\
@@ -366,6 +370,9 @@ extern void machvec_init_from_cmdline(co
 #ifndef platform_dma_supported
 # define  platform_dma_supported	swiotlb_dma_supported
 #endif
+#ifndef platform_dma_get_required_mask
+# define  platform_dma_get_required_mask	ia64_dma_get_required_mask
+#endif
 #ifndef platform_irq_to_vector
 # define platform_irq_to_vector		__ia64_irq_to_vector
 #endif
Index: linux-2.6/arch/ia64/sn/pci/pci_dma.c
===================================================================
--- linux-2.6.orig/arch/ia64/sn/pci/pci_dma.c	2008-11-17 08:38:00.899597589 -0600
+++ linux-2.6/arch/ia64/sn/pci/pci_dma.c	2008-11-17 08:39:50.537280180 -0600
@@ -356,6 +356,12 @@ int sn_dma_mapping_error(struct device *
 }
 EXPORT_SYMBOL(sn_dma_mapping_error);
 
+u64 sn_dma_get_required_mask(struct device *dev)
+{
+	return DMA_64BIT_MASK;
+}
+EXPORT_SYMBOL_GPL(sn_dma_get_required_mask);
+
 char *sn_pci_get_legacy_mem(struct pci_bus *bus)
 {
 	if (!SN_PCIBUS_BUSSOFT(bus))
Index: linux-2.6/arch/ia64/pci/pci.c
===================================================================
--- linux-2.6.orig/arch/ia64/pci/pci.c	2008-11-17 08:38:00.871594094 -0600
+++ linux-2.6/arch/ia64/pci/pci.c	2008-11-17 08:39:50.553282177 -0600
@@ -19,6 +19,7 @@
 #include <linux/ioport.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/bootmem.h>
 
 #include <asm/machvec.h>
 #include <asm/page.h>
@@ -748,6 +749,26 @@ static void __init set_pci_cacheline_siz
 	pci_cache_line_size = (1 << cci.pcci_line_size) / 4;
 }
 
+u64 ia64_dma_get_required_mask(struct device *dev)
+{
+	u32 low_totalram = ((max_pfn - 1) << PAGE_SHIFT);
+	u32 high_totalram = ((max_pfn - 1) >> (32 - PAGE_SHIFT));
+	u64 mask;
+
+	if (!high_totalram) {
+		/* convert to mask just covering totalram */
+		low_totalram = (1 << (fls(low_totalram) - 1));
+		low_totalram += low_totalram - 1;
+		mask = low_totalram;
+	} else {
+		high_totalram = (1 << (fls(high_totalram) - 1));
+		high_totalram += high_totalram - 1;
+		mask = (((u64)high_totalram) << 32) + 0xffffffff;
+	}
+	return mask;
+}
+EXPORT_SYMBOL_GPL(ia64_dma_get_required_mask);
+
 static int __init pcibios_init(void)
 {
 	set_pci_cacheline_size();
Index: linux-2.6/include/linux/dma-mapping.h
===================================================================
--- linux-2.6.orig/include/linux/dma-mapping.h	2008-11-17 08:38:07.740451313 -0600
+++ linux-2.6/include/linux/dma-mapping.h	2008-11-17 08:39:50.573284674 -0600
@@ -70,7 +70,9 @@ static inline u64 dma_get_mask(struct de
 	return DMA_32BIT_MASK;
 }
 
+#ifndef CONFIG_IA64
 extern u64 dma_get_required_mask(struct device *dev);
+#endif
 
 static inline unsigned int dma_get_max_seg_size(struct device *dev)
 {
Index: linux-2.6/arch/ia64/include/asm/machvec_init.h
===================================================================
--- linux-2.6.orig/arch/ia64/include/asm/machvec_init.h	2008-11-17 08:38:00.763580614 -0600
+++ linux-2.6/arch/ia64/include/asm/machvec_init.h	2008-11-17 08:39:50.589286671 -0600
@@ -3,6 +3,7 @@
 
 extern ia64_mv_send_ipi_t ia64_send_ipi;
 extern ia64_mv_global_tlb_purge_t ia64_global_tlb_purge;
+extern ia64_mv_dma_get_required_mask ia64_dma_get_required_mask;
 extern ia64_mv_irq_to_vector __ia64_irq_to_vector;
 extern ia64_mv_local_vector_to_irq __ia64_local_vector_to_irq;
 extern ia64_mv_pci_get_legacy_mem_t ia64_pci_get_legacy_mem;

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ia64: SN specific version of dma_get_required_mask()
  2008-11-17 16:24 [PATCH] ia64: SN specific version of dma_get_required_mask() John Keller
@ 2008-11-17 16:39 ` Bernhard Walle
  2008-11-18 14:08   ` John Keller
  2008-11-19  4:07 ` FUJITA Tomonori
  1 sibling, 1 reply; 12+ messages in thread
From: Bernhard Walle @ 2008-11-17 16:39 UTC (permalink / raw)
  To: John Keller; +Cc: linux-ia64, linux-kernel, John Keller

* John Keller [2008-11-17 10:24]:
>
> Create a platform specific version of dma_get_required_mask()
> for ia64 SN Altix. All SN Altix platforms support 64 bit DMA
> addressing regardless of the size of system memory.
> Create a ia64 machvec for dma_get_required_mask, with the
> SN version unconditionally returning DMA_64BIT_MASK.

Maybe we should then switch the ia64_platform_is("sn2")
in check_crashkernel_memory() to dma_get_required_mask() ==
DMA_64BIT_MASK because that's basically what the check is about.

BTW: What about UV?


Regards,
Bernhard
-- 
Bernhard Walle, SUSE Linux Products GmbH, Architecture Development

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ia64: SN specific version of dma_get_required_mask()
  2008-11-17 16:39 ` Bernhard Walle
@ 2008-11-18 14:08   ` John Keller
  2008-11-18 15:35     ` Luck, Tony
  2008-11-23 13:37     ` Bernhard Walle
  0 siblings, 2 replies; 12+ messages in thread
From: John Keller @ 2008-11-18 14:08 UTC (permalink / raw)
  To: Bernhard Walle; +Cc: linux-ia64, linux-kernel

> 
> * John Keller [2008-11-17 10:24]:
> >
> > Create a platform specific version of dma_get_required_mask()
> > for ia64 SN Altix. All SN Altix platforms support 64 bit DMA
> > addressing regardless of the size of system memory.
> > Create a ia64 machvec for dma_get_required_mask, with the
> > SN version unconditionally returning DMA_64BIT_MASK.
> 
> Maybe we should then switch the ia64_platform_is("sn2")
> in check_crashkernel_memory() to dma_get_required_mask() ==
> DMA_64BIT_MASK because that's basically what the check is about.
> 
> BTW: What about UV?
> 
> 
> Regards,
> Bernhard
> -- 
> Bernhard Walle, SUSE Linux Products GmbH, Architecture Development
> 


This patch addresses a problem on SN Altix systems with < 4GB, where
device drivers using the dma_get_required_mask() API would be told
to use 32 bit DMA, when 64 bit is more efficient.

How exactly the use of dma_get_required_mask() relates to the crash
kernel code you refer to is unclear to me.

If, for all platforms, the crash kernel code could use the mask returned
from dma_get_required_mask() to do its check, then switching the code
might be OK. But, if that's not possible for some platforms, then I'd
wonder if dma_get_required_mask() is being used in the wrong context in
this case.

For UV systems, the default/generic dma_get_required_mask() will be used,
returning a value based on the size of system memory.

John

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH] ia64: SN specific version of dma_get_required_mask()
  2008-11-18 14:08   ` John Keller
@ 2008-11-18 15:35     ` Luck, Tony
  2008-11-18 15:53       ` John Keller
  2008-11-23 13:37     ` Bernhard Walle
  1 sibling, 1 reply; 12+ messages in thread
From: Luck, Tony @ 2008-11-18 15:35 UTC (permalink / raw)
  To: John Keller, Bernhard Walle; +Cc: linux-ia64, linux-kernel

> This patch addresses a problem on SN Altix systems with < 4GB, where
> device drivers using the dma_get_required_mask() API would be told
> to use 32 bit DMA, when 64 bit is more efficient.

Even if someone did configure an Altix with < 4GB (which seems a very
unlikely occurance) all of that 4G would be located above 4GB (lowest
physical address on Altix is something like 384 TB, isn't it?)

Did we really make some dma mask decisions based on the amount
of memory rather than its location?  If we do, then perhaps we
should fix this in a generic place, not in sn2 specific code.

-Tony

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ia64: SN specific version of dma_get_required_mask()
  2008-11-18 15:35     ` Luck, Tony
@ 2008-11-18 15:53       ` John Keller
  0 siblings, 0 replies; 12+ messages in thread
From: John Keller @ 2008-11-18 15:53 UTC (permalink / raw)
  To: Luck, Tony; +Cc: John Keller, Bernhard Walle, linux-ia64, linux-kernel

> 
> > This patch addresses a problem on SN Altix systems with < 4GB, where
> > device drivers using the dma_get_required_mask() API would be told
> > to use 32 bit DMA, when 64 bit is more efficient.
> 
> Even if someone did configure an Altix with < 4GB (which seems a very
> unlikely occurance) all of that 4G would be located above 4GB (lowest
> physical address on Altix is something like 384 TB, isn't it?)
> 
> Did we really make some dma mask decisions based on the amount
> of memory rather than its location?  If we do, then perhaps we
> should fix this in a generic place, not in sn2 specific code.
> 
> -Tony
> 

This is the generic routine for all archs and platforms...

drivers/base/platform.c


#ifndef ARCH_HAS_DMA_GET_REQUIRED_MASK
u64 dma_get_required_mask(struct device *dev)
{
        u32 low_totalram = ((max_pfn - 1) << PAGE_SHIFT);
        u32 high_totalram = ((max_pfn - 1) >> (32 - PAGE_SHIFT));
        u64 mask;

        if (!high_totalram) {
                /* convert to mask just covering totalram */
                low_totalram = (1 << (fls(low_totalram) - 1));
                low_totalram += low_totalram - 1;
                mask = low_totalram;
        } else {
                high_totalram = (1 << (fls(high_totalram) - 1));
                high_totalram += high_totalram - 1;
                mask = (((u64)high_totalram) << 32) + 0xffffffff;
        }
        return mask;
}
EXPORT_SYMBOL_GPL(dma_get_required_mask);
#endif


John
----

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ia64: SN specific version of dma_get_required_mask()
  2008-11-17 16:24 [PATCH] ia64: SN specific version of dma_get_required_mask() John Keller
  2008-11-17 16:39 ` Bernhard Walle
@ 2008-11-19  4:07 ` FUJITA Tomonori
  1 sibling, 0 replies; 12+ messages in thread
From: FUJITA Tomonori @ 2008-11-19  4:07 UTC (permalink / raw)
  To: jpk; +Cc: linux-ia64, linux-kernel

On Mon, 17 Nov 2008 10:24:54 -0600
John Keller <jpk@sgi.com> wrote:

> Create a platform specific version of dma_get_required_mask()
> for ia64 SN Altix. All SN Altix platforms support 64 bit DMA
> addressing regardless of the size of system memory.
> Create a ia64 machvec for dma_get_required_mask, with the
> SN version unconditionally returning DMA_64BIT_MASK.
> 
> 
> Signed-off-by: John Keller <jpk@sgi.com>
> ---
> 
>  Documentation/DMA-API.txt            |    9 ++++-----
>  arch/ia64/include/asm/dma-mapping.h  |    3 +++
>  arch/ia64/include/asm/machvec.h      |    7 +++++++
>  arch/ia64/include/asm/machvec_init.h |    1 +
>  arch/ia64/include/asm/machvec_sn2.h  |    2 ++
>  arch/ia64/pci/pci.c                  |   21 +++++++++++++++++++++
>  arch/ia64/sn/pci/pci_dma.c           |    6 ++++++
>  include/linux/dma-mapping.h          |    2 ++
>  8 files changed, 46 insertions(+), 5 deletions(-)

(snip)

>  static int __init pcibios_init(void)
>  {
>  	set_pci_cacheline_size();
> Index: linux-2.6/include/linux/dma-mapping.h
> ===================================================================
> --- linux-2.6.orig/include/linux/dma-mapping.h	2008-11-17 08:38:07.740451313 -0600
> +++ linux-2.6/include/linux/dma-mapping.h	2008-11-17 08:39:50.573284674 -0600
> @@ -70,7 +70,9 @@ static inline u64 dma_get_mask(struct de
>  	return DMA_32BIT_MASK;
>  }
>  
> +#ifndef CONFIG_IA64
>  extern u64 dma_get_required_mask(struct device *dev);
> +#endif

I think that adding CONFIG_IA64 to include/linux/dma-mapping.h is
wrong. I also think that you don't need to ifndef this extern.

If you need this trick with only CONFIG_IA64_SGI_SN2, how about
something like this? It's simple and we can avoid duplicate the
generic dma_get_required_mask in arch/ia64/pci/pci.c


diff --git a/arch/ia64/include/asm/dma-mapping.h b/arch/ia64/include/asm/dma-mapping.h
index bbab7e2..4ffbd18 100644
--- a/arch/ia64/include/asm/dma-mapping.h
+++ b/arch/ia64/include/asm/dma-mapping.h
@@ -144,6 +144,13 @@ static inline struct dma_mapping_ops *get_dma_ops(struct device *dev)
 	return dma_ops;
 }
 
-
+#ifdef CONFIG_IA64_SGI_SN2
+#define ARCH_HAS_DMA_GET_REQUIRED_MASK
+static inline u64 dma_get_required_mask(struct device *dev)
+{
+	return DMA_64BIT_MASK;
+}
+EXPORT_SYMBOL_GPL(dma_get_required_mask);
+#endif
 
 #endif /* _ASM_IA64_DMA_MAPPING_H */

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] ia64: SN specific version of dma_get_required_mask()
  2008-11-18 14:08   ` John Keller
  2008-11-18 15:35     ` Luck, Tony
@ 2008-11-23 13:37     ` Bernhard Walle
  2008-11-25 20:50       ` John Keller
  1 sibling, 1 reply; 12+ messages in thread
From: Bernhard Walle @ 2008-11-23 13:37 UTC (permalink / raw)
  To: John Keller; +Cc: linux-ia64, linux-kernel

Hi,

[Sorry for the late reply and for not following the whole thread, I'm
just busy.]

* John Keller [2008-11-18 08:08]:
> 
> This patch addresses a problem on SN Altix systems with < 4GB, where
> device drivers using the dma_get_required_mask() API would be told
> to use 32 bit DMA, when 64 bit is more efficient.
> 
> How exactly the use of dma_get_required_mask() relates to the crash
> kernel code you refer to is unclear to me.

I'm not sure myself. The crashkernel reservation code on IA64 (for
other architectures I don't know any machines that have basically their
whole memory except a small amount which is used for booting mapped
above 4 GiB physical address space) needs to check if it's okay to
use memory for the crashkernel that is *all* above 4 GiB.

This is only possible if a hardware IO/MMU is present (and working
correctly in the kdump case which isn't the case on HP IA64) and SWIOTBL
is not used because SWIOTBL needs some memory below that 4 GiB margin.

Now I thought that there's a relationship between "memory above 4 GiB
can be used for DMA" and the return value of dma_get_required_mask().
My assumption was:

  (dma_get_required_mask() & 0xffffffff00000000ull) > 0
  -> memory above 4 GiB can be used for DMA and so the
     crashkernel memory can reside above 4 GiB

  (dma_get_required_mask() & 0xffffffff00000000ull) == 0
  -> memory above 4 GiB can not be used for DMA and so the
     crashkernel memory can not all reside above 4 GiB

Is that wrong?

> If, for all platforms, the crash kernel code could use the mask returned
> from dma_get_required_mask() to do its check, then switching the code
> might be OK. But, if that's not possible for some platforms, then I'd
> wonder if dma_get_required_mask() is being used in the wrong context in
> this case.

The crashkernel reservation code is different for every platform, so it
does not matter. However, in theory I think the check would return
correct results.


Regards,
Bernhard
-- 
Bernhard Walle, SUSE LINUX Products GmbH, Architecture Development

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ia64: SN specific version of dma_get_required_mask()
  2008-11-23 13:37     ` Bernhard Walle
@ 2008-11-25 20:50       ` John Keller
  0 siblings, 0 replies; 12+ messages in thread
From: John Keller @ 2008-11-25 20:50 UTC (permalink / raw)
  To: Bernhard Walle; +Cc: linux-ia64, linux-kernel

> 
> Hi,
> 
> [Sorry for the late reply and for not following the whole thread, I'm
> just busy.]
> 
> * John Keller [2008-11-18 08:08]:
> > 
> > This patch addresses a problem on SN Altix systems with < 4GB, where
> > device drivers using the dma_get_required_mask() API would be told
> > to use 32 bit DMA, when 64 bit is more efficient.
> > 
> > How exactly the use of dma_get_required_mask() relates to the crash
> > kernel code you refer to is unclear to me.
> 
> I'm not sure myself. The crashkernel reservation code on IA64 (for
> other architectures I don't know any machines that have basically their
> whole memory except a small amount which is used for booting mapped
> above 4 GiB physical address space) needs to check if it's okay to
> use memory for the crashkernel that is *all* above 4 GiB.
> 
> This is only possible if a hardware IO/MMU is present (and working
> correctly in the kdump case which isn't the case on HP IA64) and SWIOTBL
> is not used because SWIOTBL needs some memory below that 4 GiB margin.
> 
> Now I thought that there's a relationship between "memory above 4 GiB
> can be used for DMA" and the return value of dma_get_required_mask().
> My assumption was:
> 
>   (dma_get_required_mask() & 0xffffffff00000000ull) > 0
>   -> memory above 4 GiB can be used for DMA and so the
>      crashkernel memory can reside above 4 GiB
> 
>   (dma_get_required_mask() & 0xffffffff00000000ull) == 0
>   -> memory above 4 GiB can not be used for DMA and so the
>      crashkernel memory can not all reside above 4 GiB
> 
> Is that wrong?

In the case of SN Altix, the return value (with my patch) will always
be 0xffffffffffffffffull. So, your check would work. And just as the
current code's ia64_platform_is("sn2") check would always return TRUE
on any SN2 system, so would your proposed use of dma_get_required_mask().

However, for SN2, memory size or location cannot be inferred from the
return value, as it has no affect on the returned value.
As Documentation/DMA-API.txt says:
    "This API returns the mask that the platform requires to
    operate efficiently."

And for SN2, this is always a 64 bit mask, irregardless of memory size,
location, etc.


> 
> > If, for all platforms, the crash kernel code could use the mask returned
> > from dma_get_required_mask() to do its check, then switching the code
> > might be OK. But, if that's not possible for some platforms, then I'd
> > wonder if dma_get_required_mask() is being used in the wrong context in
> > this case.
> 
> The crashkernel reservation code is different for every platform, so it
> does not matter. However, in theory I think the check would return
> correct results.

OK, I must not be understanding something. check_crashkernel_memory()
appears to be coded to handle 3 or more platforms (sn2, uv, and others).


> 
> 
> Regards,
> Bernhard
> -- 
> Bernhard Walle, SUSE LINUX Products GmbH, Architecture Development
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ia64: SN specific version of dma_get_required_mask()
  2008-11-20 22:57   ` John Keller
@ 2008-11-21  5:11     ` FUJITA Tomonori
  0 siblings, 0 replies; 12+ messages in thread
From: FUJITA Tomonori @ 2008-11-21  5:11 UTC (permalink / raw)
  To: jpk; +Cc: fujita.tomonori, linux-ia64, linux-kernel

On Thu, 20 Nov 2008 16:57:41 -0600 (CST)
John Keller <jpk@sgi.com> wrote:

> > 
> > On Wed, 19 Nov 2008 10:12:23 -0600 (CST)
> > John Keller <jpk@sgi.com> wrote:
> > 
> > > On Tue, 18 Nov 2008
> > > Fujita Tomonori wrote:
> > > 
> > > > On Mon, 17 Nov 2008 10:24:54 -0600
> > > > John Keller <jpk@sgi.com> wrote:
> > > 
> > > > > Create a platform specific version of dma_get_required_mask()
> > > > > for ia64 SN Altix. All SN Altix platforms support 64 bit DMA
> > > > > addressing regardless of the size of system memory.
> > > > > Create a ia64 machvec for dma_get_required_mask, with the
> > > > > SN version unconditionally returning DMA_64BIT_MASK.
> > > > > 
> > > > > 
> > > > > Signed-off-by: John Keller <jpk@sgi.com>
> > > > > ---
> > > > > 
> > > > >  Documentation/DMA-API.txt            |    9 ++++-----
> > > > >  arch/ia64/include/asm/dma-mapping.h  |    3 +++
> > > > >  arch/ia64/include/asm/machvec.h      |    7 +++++++
> > > > >  arch/ia64/include/asm/machvec_init.h |    1 +
> > > > >  arch/ia64/include/asm/machvec_sn2.h  |    2 ++
> > > > >  arch/ia64/pci/pci.c                  |   21 +++++++++++++++++++++
> > > > >  arch/ia64/sn/pci/pci_dma.c           |    6 ++++++
> > > > >  include/linux/dma-mapping.h          |    2 ++
> > > > >  8 files changed, 46 insertions(+), 5 deletions(-)
> > > > 
> > > > (snip)
> > > > 
> > > > >  static int __init pcibios_init(void)
> > > > >  {
> > > > >  	set_pci_cacheline_size();
> > > > > Index: linux-2.6/include/linux/dma-mapping.h
> > > > > ===================================================================
> > > > > --- linux-2.6.orig/include/linux/dma-mapping.h	2008-11-17 08:38:07.740451313 -0600
> > > > > +++ linux-2.6/include/linux/dma-mapping.h	2008-11-17 08:39:50.573284674 -0600
> > > > > @@ -70,7 +70,9 @@ static inline u64 dma_get_mask(struct de
> > > > >  	return DMA_32BIT_MASK;
> > > > >  }
> > > > >  
> > > > > +#ifndef CONFIG_IA64
> > > > >  extern u64 dma_get_required_mask(struct device *dev);
> > > > > +#endif
> > > > 
> > > > I think that adding CONFIG_IA64 to include/linux/dma-mapping.h is
> > > > wrong. I also think that you don't need to ifndef this extern.
> > > 
> > > Without this change ia64 would not build for me.
> > 
> > Odd. It works for me. Can you send your .config?
> 
> It built with CONFIG_IA64_SGI_SN2, but not CONFIG_IA64_GENERIC.
> The error was related to the machvec magic, where the extern was
> transformed into:
> 
> extern u64 ia64_mv.dma_get_required_mask(struct device *dev);
> 
> and the error was:
> 
> include/linux/dma-mapping.h:73: error: expected '=', ',' ';', 'asm' or '__attribute__' before '.' token
> 
> I'll be resubmitting the patch with this fixed and the #ifndef removed.

Ok, if you don't add IA64 specific stuff to
include/linux/dma-mapping.h, I think that there is no complaint from
non IA64 developers.


> > > > If you need this trick with only CONFIG_IA64_SGI_SN2, how about
> > > > something like this? It's simple and we can avoid duplicate the
> > > > generic dma_get_required_mask in arch/ia64/pci/pci.c
> > > 
> > > Actually I need it for CONFIG_IA64_GENERIC and CONFIG_IA64_SGI_SN2.
> > 
> > Hmm, but your patch puts sn_dma_get_required_mask in
> > arch/ia64/sn/pci/pci_dma.c. arch/ia64/sn/pci/pci_dma.c is compiled
> > only with CONFIG_IA64_SGI_SN2?
> 
> No, arch/ia64/sn/pci/pci_dma.c is complied with CONFIG_IA64_GENERIC
> as well, and if the kernel is booted on a SN2 platform, the
> sn_machvec becomes the default.

Ah, I see. Thanks,

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ia64: SN specific version of dma_get_required_mask()
  2008-11-20  4:49 ` FUJITA Tomonori
@ 2008-11-20 22:57   ` John Keller
  2008-11-21  5:11     ` FUJITA Tomonori
  0 siblings, 1 reply; 12+ messages in thread
From: John Keller @ 2008-11-20 22:57 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: linux-ia64, linux-kernel

> 
> On Wed, 19 Nov 2008 10:12:23 -0600 (CST)
> John Keller <jpk@sgi.com> wrote:
> 
> > On Tue, 18 Nov 2008
> > Fujita Tomonori wrote:
> > 
> > > On Mon, 17 Nov 2008 10:24:54 -0600
> > > John Keller <jpk@sgi.com> wrote:
> > 
> > > > Create a platform specific version of dma_get_required_mask()
> > > > for ia64 SN Altix. All SN Altix platforms support 64 bit DMA
> > > > addressing regardless of the size of system memory.
> > > > Create a ia64 machvec for dma_get_required_mask, with the
> > > > SN version unconditionally returning DMA_64BIT_MASK.
> > > > 
> > > > 
> > > > Signed-off-by: John Keller <jpk@sgi.com>
> > > > ---
> > > > 
> > > >  Documentation/DMA-API.txt            |    9 ++++-----
> > > >  arch/ia64/include/asm/dma-mapping.h  |    3 +++
> > > >  arch/ia64/include/asm/machvec.h      |    7 +++++++
> > > >  arch/ia64/include/asm/machvec_init.h |    1 +
> > > >  arch/ia64/include/asm/machvec_sn2.h  |    2 ++
> > > >  arch/ia64/pci/pci.c                  |   21 +++++++++++++++++++++
> > > >  arch/ia64/sn/pci/pci_dma.c           |    6 ++++++
> > > >  include/linux/dma-mapping.h          |    2 ++
> > > >  8 files changed, 46 insertions(+), 5 deletions(-)
> > > 
> > > (snip)
> > > 
> > > >  static int __init pcibios_init(void)
> > > >  {
> > > >  	set_pci_cacheline_size();
> > > > Index: linux-2.6/include/linux/dma-mapping.h
> > > > ===================================================================
> > > > --- linux-2.6.orig/include/linux/dma-mapping.h	2008-11-17 08:38:07.740451313 -0600
> > > > +++ linux-2.6/include/linux/dma-mapping.h	2008-11-17 08:39:50.573284674 -0600
> > > > @@ -70,7 +70,9 @@ static inline u64 dma_get_mask(struct de
> > > >  	return DMA_32BIT_MASK;
> > > >  }
> > > >  
> > > > +#ifndef CONFIG_IA64
> > > >  extern u64 dma_get_required_mask(struct device *dev);
> > > > +#endif
> > > 
> > > I think that adding CONFIG_IA64 to include/linux/dma-mapping.h is
> > > wrong. I also think that you don't need to ifndef this extern.
> > 
> > Without this change ia64 would not build for me.
> 
> Odd. It works for me. Can you send your .config?

It built with CONFIG_IA64_SGI_SN2, but not CONFIG_IA64_GENERIC.
The error was related to the machvec magic, where the extern was
transformed into:

extern u64 ia64_mv.dma_get_required_mask(struct device *dev);

and the error was:

include/linux/dma-mapping.h:73: error: expected '=', ',' ';', 'asm' or '__attribute__' before '.' token

I'll be resubmitting the patch with this fixed and the #ifndef removed.



> 
> 
> > > If you need this trick with only CONFIG_IA64_SGI_SN2, how about
> > > something like this? It's simple and we can avoid duplicate the
> > > generic dma_get_required_mask in arch/ia64/pci/pci.c
> > 
> > Actually I need it for CONFIG_IA64_GENERIC and CONFIG_IA64_SGI_SN2.
> 
> Hmm, but your patch puts sn_dma_get_required_mask in
> arch/ia64/sn/pci/pci_dma.c. arch/ia64/sn/pci/pci_dma.c is compiled
> only with CONFIG_IA64_SGI_SN2?

No, arch/ia64/sn/pci/pci_dma.c is complied with CONFIG_IA64_GENERIC
as well, and if the kernel is booted on a SN2 platform, the
sn_machvec becomes the default.


> 
> 
> > But, even so, unfortunately your suggestion doesn't build.
> > 
> > I see numerous errors such as these:
> > 
> > kernel/fork.o: In function `__crc_dma_get_required_mask':
> > fork.c:(*ABS*+0xb0339303): multiple definition of `__crc_dma_get_required_mask'
> > kernel/exit.o: In function `__crc_dma_get_required_mask':
> > exit.c:(*ABS*+0x4a7c4115): multiple definiti  CC      fs/sysfs/group.o
> > on of `__crc_dma_get_required_mask'
> 
> Odd, it works for me. Looks that #define
> ARCH_HAS_DMA_GET_REQUIRED_MASK doesn't work. I'll try this with your
> .config.

Your suggested fix would work for CONFIG_IA64_SGI_SN2, but
we need it to work for CONFIG_IA64_GENERIC kernels as well, which
is what I was building when seeing these errors.



> 
> Thanks,
> 
> 
> > > 
> > > 
> > > diff --git a/arch/ia64/include/asm/dma-mapping.h b/arch/ia64/include/asm/dma-mapping.h
> > > index bbab7e2..4ffbd18 100644
> > > --- a/arch/ia64/include/asm/dma-mapping.h
> > > +++ b/arch/ia64/include/asm/dma-mapping.h
> > > @@ -144,6 +144,13 @@ static inline struct dma_mapping_ops *get_dma_ops(struct device *dev)
> > > 	return dma_ops;
> > > }
> > > 
> > > -
> > > +#ifdef CONFIG_IA64_SGI_SN2
> > > +#define ARCH_HAS_DMA_GET_REQUIRED_MASK
> > > +static inline u64 dma_get_required_mask(struct device *dev)
> > > +{
> > > +	return DMA_64BIT_MASK;
> > > +}
> > > +EXPORT_SYMBOL_GPL(dma_get_required_mask);
> > > +#endif
> > >  
> > > #endif /* _ASM_IA64_DMA_MAPPING_H */
> > --
> > 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/
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ia64: SN specific version of dma_get_required_mask()
  2008-11-19 16:12 John Keller
@ 2008-11-20  4:49 ` FUJITA Tomonori
  2008-11-20 22:57   ` John Keller
  0 siblings, 1 reply; 12+ messages in thread
From: FUJITA Tomonori @ 2008-11-20  4:49 UTC (permalink / raw)
  To: jpk; +Cc: fujita.tomonori, linux-ia64, linux-kernel

On Wed, 19 Nov 2008 10:12:23 -0600 (CST)
John Keller <jpk@sgi.com> wrote:

> On Tue, 18 Nov 2008
> Fujita Tomonori wrote:
> 
> > On Mon, 17 Nov 2008 10:24:54 -0600
> > John Keller <jpk@sgi.com> wrote:
> 
> > > Create a platform specific version of dma_get_required_mask()
> > > for ia64 SN Altix. All SN Altix platforms support 64 bit DMA
> > > addressing regardless of the size of system memory.
> > > Create a ia64 machvec for dma_get_required_mask, with the
> > > SN version unconditionally returning DMA_64BIT_MASK.
> > > 
> > > 
> > > Signed-off-by: John Keller <jpk@sgi.com>
> > > ---
> > > 
> > >  Documentation/DMA-API.txt            |    9 ++++-----
> > >  arch/ia64/include/asm/dma-mapping.h  |    3 +++
> > >  arch/ia64/include/asm/machvec.h      |    7 +++++++
> > >  arch/ia64/include/asm/machvec_init.h |    1 +
> > >  arch/ia64/include/asm/machvec_sn2.h  |    2 ++
> > >  arch/ia64/pci/pci.c                  |   21 +++++++++++++++++++++
> > >  arch/ia64/sn/pci/pci_dma.c           |    6 ++++++
> > >  include/linux/dma-mapping.h          |    2 ++
> > >  8 files changed, 46 insertions(+), 5 deletions(-)
> > 
> > (snip)
> > 
> > >  static int __init pcibios_init(void)
> > >  {
> > >  	set_pci_cacheline_size();
> > > Index: linux-2.6/include/linux/dma-mapping.h
> > > ===================================================================
> > > --- linux-2.6.orig/include/linux/dma-mapping.h	2008-11-17 08:38:07.740451313 -0600
> > > +++ linux-2.6/include/linux/dma-mapping.h	2008-11-17 08:39:50.573284674 -0600
> > > @@ -70,7 +70,9 @@ static inline u64 dma_get_mask(struct de
> > >  	return DMA_32BIT_MASK;
> > >  }
> > >  
> > > +#ifndef CONFIG_IA64
> > >  extern u64 dma_get_required_mask(struct device *dev);
> > > +#endif
> > 
> > I think that adding CONFIG_IA64 to include/linux/dma-mapping.h is
> > wrong. I also think that you don't need to ifndef this extern.
> 
> Without this change ia64 would not build for me.

Odd. It works for me. Can you send your .config?


> > If you need this trick with only CONFIG_IA64_SGI_SN2, how about
> > something like this? It's simple and we can avoid duplicate the
> > generic dma_get_required_mask in arch/ia64/pci/pci.c
> 
> Actually I need it for CONFIG_IA64_GENERIC and CONFIG_IA64_SGI_SN2.

Hmm, but your patch puts sn_dma_get_required_mask in
arch/ia64/sn/pci/pci_dma.c. arch/ia64/sn/pci/pci_dma.c is compiled
only with CONFIG_IA64_SGI_SN2?


> But, even so, unfortunately your suggestion doesn't build.
> 
> I see numerous errors such as these:
> 
> kernel/fork.o: In function `__crc_dma_get_required_mask':
> fork.c:(*ABS*+0xb0339303): multiple definition of `__crc_dma_get_required_mask'
> kernel/exit.o: In function `__crc_dma_get_required_mask':
> exit.c:(*ABS*+0x4a7c4115): multiple definiti  CC      fs/sysfs/group.o
> on of `__crc_dma_get_required_mask'

Odd, it works for me. Looks that #define
ARCH_HAS_DMA_GET_REQUIRED_MASK doesn't work. I'll try this with your
.config.

Thanks,


> > 
> > 
> > diff --git a/arch/ia64/include/asm/dma-mapping.h b/arch/ia64/include/asm/dma-mapping.h
> > index bbab7e2..4ffbd18 100644
> > --- a/arch/ia64/include/asm/dma-mapping.h
> > +++ b/arch/ia64/include/asm/dma-mapping.h
> > @@ -144,6 +144,13 @@ static inline struct dma_mapping_ops *get_dma_ops(struct device *dev)
> > 	return dma_ops;
> > }
> > 
> > -
> > +#ifdef CONFIG_IA64_SGI_SN2
> > +#define ARCH_HAS_DMA_GET_REQUIRED_MASK
> > +static inline u64 dma_get_required_mask(struct device *dev)
> > +{
> > +	return DMA_64BIT_MASK;
> > +}
> > +EXPORT_SYMBOL_GPL(dma_get_required_mask);
> > +#endif
> >  
> > #endif /* _ASM_IA64_DMA_MAPPING_H */
> --
> 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/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ia64: SN specific version of dma_get_required_mask()
@ 2008-11-19 16:12 John Keller
  2008-11-20  4:49 ` FUJITA Tomonori
  0 siblings, 1 reply; 12+ messages in thread
From: John Keller @ 2008-11-19 16:12 UTC (permalink / raw)
  To: fujita.tomonori; +Cc: linux-ia64, linux-kernel

On Tue, 18 Nov 2008
Fujita Tomonori wrote:

> On Mon, 17 Nov 2008 10:24:54 -0600
> John Keller <jpk@sgi.com> wrote:

> > Create a platform specific version of dma_get_required_mask()
> > for ia64 SN Altix. All SN Altix platforms support 64 bit DMA
> > addressing regardless of the size of system memory.
> > Create a ia64 machvec for dma_get_required_mask, with the
> > SN version unconditionally returning DMA_64BIT_MASK.
> > 
> > 
> > Signed-off-by: John Keller <jpk@sgi.com>
> > ---
> > 
> >  Documentation/DMA-API.txt            |    9 ++++-----
> >  arch/ia64/include/asm/dma-mapping.h  |    3 +++
> >  arch/ia64/include/asm/machvec.h      |    7 +++++++
> >  arch/ia64/include/asm/machvec_init.h |    1 +
> >  arch/ia64/include/asm/machvec_sn2.h  |    2 ++
> >  arch/ia64/pci/pci.c                  |   21 +++++++++++++++++++++
> >  arch/ia64/sn/pci/pci_dma.c           |    6 ++++++
> >  include/linux/dma-mapping.h          |    2 ++
> >  8 files changed, 46 insertions(+), 5 deletions(-)
> 
> (snip)
> 
> >  static int __init pcibios_init(void)
> >  {
> >  	set_pci_cacheline_size();
> > Index: linux-2.6/include/linux/dma-mapping.h
> > ===================================================================
> > --- linux-2.6.orig/include/linux/dma-mapping.h	2008-11-17 08:38:07.740451313 -0600
> > +++ linux-2.6/include/linux/dma-mapping.h	2008-11-17 08:39:50.573284674 -0600
> > @@ -70,7 +70,9 @@ static inline u64 dma_get_mask(struct de
> >  	return DMA_32BIT_MASK;
> >  }
> >  
> > +#ifndef CONFIG_IA64
> >  extern u64 dma_get_required_mask(struct device *dev);
> > +#endif
> 
> I think that adding CONFIG_IA64 to include/linux/dma-mapping.h is
> wrong. I also think that you don't need to ifndef this extern.

Without this change ia64 would not build for me.


> 
> If you need this trick with only CONFIG_IA64_SGI_SN2, how about
> something like this? It's simple and we can avoid duplicate the
> generic dma_get_required_mask in arch/ia64/pci/pci.c

Actually I need it for CONFIG_IA64_GENERIC and CONFIG_IA64_SGI_SN2.
But, even so, unfortunately your suggestion doesn't build.

I see numerous errors such as these:

kernel/fork.o: In function `__crc_dma_get_required_mask':
fork.c:(*ABS*+0xb0339303): multiple definition of `__crc_dma_get_required_mask'
kernel/exit.o: In function `__crc_dma_get_required_mask':
exit.c:(*ABS*+0x4a7c4115): multiple definiti  CC      fs/sysfs/group.o
on of `__crc_dma_get_required_mask'



> 
> 
> diff --git a/arch/ia64/include/asm/dma-mapping.h b/arch/ia64/include/asm/dma-mapping.h
> index bbab7e2..4ffbd18 100644
> --- a/arch/ia64/include/asm/dma-mapping.h
> +++ b/arch/ia64/include/asm/dma-mapping.h
> @@ -144,6 +144,13 @@ static inline struct dma_mapping_ops *get_dma_ops(struct device *dev)
> 	return dma_ops;
> }
> 
> -
> +#ifdef CONFIG_IA64_SGI_SN2
> +#define ARCH_HAS_DMA_GET_REQUIRED_MASK
> +static inline u64 dma_get_required_mask(struct device *dev)
> +{
> +	return DMA_64BIT_MASK;
> +}
> +EXPORT_SYMBOL_GPL(dma_get_required_mask);
> +#endif
>  
> #endif /* _ASM_IA64_DMA_MAPPING_H */

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2008-11-25 20:50 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-11-17 16:24 [PATCH] ia64: SN specific version of dma_get_required_mask() John Keller
2008-11-17 16:39 ` Bernhard Walle
2008-11-18 14:08   ` John Keller
2008-11-18 15:35     ` Luck, Tony
2008-11-18 15:53       ` John Keller
2008-11-23 13:37     ` Bernhard Walle
2008-11-25 20:50       ` John Keller
2008-11-19  4:07 ` FUJITA Tomonori
2008-11-19 16:12 John Keller
2008-11-20  4:49 ` FUJITA Tomonori
2008-11-20 22:57   ` John Keller
2008-11-21  5:11     ` FUJITA Tomonori

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).