2008-11-17 16:25:22

by John Keller

[permalink] [raw]
Subject: [PATCH] ia64: SN specific version of dma_get_required_mask()

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 <[email protected]>
---

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;


2008-11-17 16:39:41

by Bernhard Walle

[permalink] [raw]
Subject: Re: [PATCH] ia64: SN specific version of dma_get_required_mask()

* 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

2008-11-18 14:08:25

by John Keller

[permalink] [raw]
Subject: Re: [PATCH] ia64: SN specific version of dma_get_required_mask()

>
> * 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

2008-11-18 15:35:34

by Tony Luck

[permalink] [raw]
Subject: RE: [PATCH] ia64: SN specific version of dma_get_required_mask()

> 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

2008-11-18 16:01:18

by John Keller

[permalink] [raw]
Subject: Re: [PATCH] ia64: SN specific version of dma_get_required_mask()

>
> > 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
----

2008-11-19 04:08:15

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH] ia64: SN specific version of dma_get_required_mask()

On Mon, 17 Nov 2008 10:24:54 -0600
John Keller <[email protected]> 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 <[email protected]>
> ---
>
> 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 */

2008-11-19 16:12:36

by John Keller

[permalink] [raw]
Subject: Re: [PATCH] ia64: SN specific version of dma_get_required_mask()

On Tue, 18 Nov 2008
Fujita Tomonori wrote:

> On Mon, 17 Nov 2008 10:24:54 -0600
> John Keller <[email protected]> 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 <[email protected]>
> > ---
> >
> > 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 */

2008-11-20 04:54:51

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH] ia64: SN specific version of dma_get_required_mask()

On Wed, 19 Nov 2008 10:12:23 -0600 (CST)
John Keller <[email protected]> wrote:

> On Tue, 18 Nov 2008
> Fujita Tomonori wrote:
>
> > On Mon, 17 Nov 2008 10:24:54 -0600
> > John Keller <[email protected]> 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 <[email protected]>
> > > ---
> > >
> > > 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2008-11-20 22:57:55

by John Keller

[permalink] [raw]
Subject: Re: [PATCH] ia64: SN specific version of dma_get_required_mask()

>
> On Wed, 19 Nov 2008 10:12:23 -0600 (CST)
> John Keller <[email protected]> wrote:
>
> > On Tue, 18 Nov 2008
> > Fujita Tomonori wrote:
> >
> > > On Mon, 17 Nov 2008 10:24:54 -0600
> > > John Keller <[email protected]> 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 <[email protected]>
> > > > ---
> > > >
> > > > 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 [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>

2008-11-21 05:12:06

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH] ia64: SN specific version of dma_get_required_mask()

On Thu, 20 Nov 2008 16:57:41 -0600 (CST)
John Keller <[email protected]> wrote:

> >
> > On Wed, 19 Nov 2008 10:12:23 -0600 (CST)
> > John Keller <[email protected]> wrote:
> >
> > > On Tue, 18 Nov 2008
> > > Fujita Tomonori wrote:
> > >
> > > > On Mon, 17 Nov 2008 10:24:54 -0600
> > > > John Keller <[email protected]> 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 <[email protected]>
> > > > > ---
> > > > >
> > > > > 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,

2008-11-23 13:37:35

by Bernhard Walle

[permalink] [raw]
Subject: Re: [PATCH] ia64: SN specific version of dma_get_required_mask()

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

2008-11-25 20:50:20

by John Keller

[permalink] [raw]
Subject: Re: [PATCH] ia64: SN specific version of dma_get_required_mask()

>
> 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
>