Hi,
I tested the sym-2 driver on ppc64 and found that hcb_p can be > 1 page
but __sym_malloc fails for allocations over 1 page. This means we
die in sym_attach.
With this patch the sym-2 works on ppc64. BTW so far it looks solid :)
Anton
diff -urN 2.4.15-pre4/drivers/scsi/sym53c8xx_2/sym_glue.h linuxppc_2_4_devel_work/drivers/scsi/sym53c8xx_2/sym_glue.h
--- 2.4.15-pre4/drivers/scsi/sym53c8xx_2/sym_glue.h Thu Nov 15 13:38:02 2001
+++ linuxppc_2_4_devel_work/drivers/scsi/sym53c8xx_2/sym_glue.h Tue Nov 13 18:03:07 2001
@@ -526,7 +526,7 @@
* couple of things related to the memory allocator.
*/
typedef u_long m_addr_t; /* Enough bits to represent any address */
-#define SYM_MEM_PAGE_ORDER 0 /* 1 PAGE maximum */
+#define SYM_MEM_PAGE_ORDER 1 /* 2 PAGE maximum */
#define SYM_MEM_CLUSTER_SHIFT (PAGE_SHIFT+SYM_MEM_PAGE_ORDER)
#ifdef MODULE
#define SYM_MEM_FREE_UNUSED /* Free unused pages immediately */
From: Anton Blanchard <[email protected]>
Date: Thu, 15 Nov 2001 15:36:54 +1100
I tested the sym-2 driver on ppc64 and found that hcb_p can be > 1 page
but __sym_malloc fails for allocations over 1 page. This means we
die in sym_attach.
Are you using 4K pages on ppc64? :-(
Franks a lot,
David S. Miller
[email protected]
From: Anton Blanchard <[email protected]>
Date: Thu, 15 Nov 2001 22:35:26 +1100
> Are you using 4K pages on ppc64? :-(
Unfortunately so. We will definitely be looking to decouple hardware and
software page sizes (like sparc64 is doing) once things stabilise, a
4KB page size is pretty small for a 64 bit arch.
Using an 8K page size should really be transparent to
any sane ELF userland, why not just do it? Is there
some hardcoded dependency in the ppc ELF stuff or is
it just a "some of our kernel code still assumes PAGE_SIZE
= 4K"?
> Are you using 4K pages on ppc64? :-(
Unfortunately so. We will definitely be looking to decouple hardware and
software page sizes (like sparc64 is doing) once things stabilise, a
4KB page size is pretty small for a 64 bit arch.
Anton
> Using an 8K page size should really be transparent to
> any sane ELF userland, why not just do it? Is there
> some hardcoded dependency in the ppc ELF stuff or is
> it just a "some of our kernel code still assumes PAGE_SIZE
> = 4K"?
Its just that the kernel code assumes 4k PAGE_SIZE. Fixing this is high
on my list but there are still more basic things to finish first :)
Anton
On Thu, 15 Nov 2001, Anton Blanchard wrote:
>
> Hi,
>
> I tested the sym-2 driver on ppc64 and found that hcb_p can be > 1 page
> but __sym_malloc fails for allocations over 1 page. This means we
> die in sym_attach.
The driver should not need more than 4096 bytes for a single allocation.
If the ppc64 page size is smaller, your patch is ok, otherwise something
may have to be fixed, likely in the driver. I cannot access to kernel
source immediately but I will check what kind of page size ppc64 is using
asap.
> With this patch the sym-2 works on ppc64. BTW so far it looks solid :)
Great!
Thanks for your report.
Regards,
G?rard.
>
> Anton
>
> diff -urN 2.4.15-pre4/drivers/scsi/sym53c8xx_2/sym_glue.h linuxppc_2_4_devel_work/drivers/scsi/sym53c8xx_2/sym_glue.h
> --- 2.4.15-pre4/drivers/scsi/sym53c8xx_2/sym_glue.h Thu Nov 15 13:38:02 2001
> +++ linuxppc_2_4_devel_work/drivers/scsi/sym53c8xx_2/sym_glue.h Tue Nov 13 18:03:07 2001
> @@ -526,7 +526,7 @@
> * couple of things related to the memory allocator.
> */
> typedef u_long m_addr_t; /* Enough bits to represent any address */
> -#define SYM_MEM_PAGE_ORDER 0 /* 1 PAGE maximum */
> +#define SYM_MEM_PAGE_ORDER 1 /* 2 PAGE maximum */
> #define SYM_MEM_CLUSTER_SHIFT (PAGE_SHIFT+SYM_MEM_PAGE_ORDER)
> #ifdef MODULE
> #define SYM_MEM_FREE_UNUSED /* Free unused pages immediately */
>
>
From: G?rard Roudier <[email protected]>
Date: Thu, 15 Nov 2001 17:27:38 +0100 (CET)
The driver should not need more than 4096 bytes for a single allocation.
If platform is 64-bit and PAGE_SIZE < 8K, yes it will.
And ppc64 fits this criteria.
?:.?˛???m???ka??b???zwm??b????˛???m?b??????z_???^n?r???z???h?????&???z??z?ޗ?+??+zf???h???~????i?????????z_???j:+v???)ߣ?m?S?y? ?杶????i?????? ????i
On Thu, 15 Nov 2001, David S. Miller wrote:
> From: G?rard Roudier <[email protected]>
> Date: Thu, 15 Nov 2001 17:27:38 +0100 (CET)
>
> The driver should not need more than 4096 bytes for a single allocation.
>
> If platform is 64-bit and PAGE_SIZE < 8K, yes it will.
> And ppc64 fits this criteria.
Btw, I didn't see any ppc64 stuff in linux kernel. As a result this
platform is still in the unsupported status. :-)
Indeed a 4K page on a 64 bit machine looks very suboptimal. Note that it
is also way too small for 32 bits machines with hundreds of megabytes of
memory. I am wondering about the reasons that made us keep with so small a
page size. As you know earlier BSDs used 2K on Vaxen that as you also know
had (have?) a physical PAGE_SIZE of 512 bytes.
About the sym-2 driver, it may well be some pointers that make it need
more than 4K allocation on 64 bit machines.
I will try to make it fit a page size max allocation even on such weird
configuration, since I do consider as high^H^H^H^Hslightly broken any
piece of software that requires more that 1 PAGE of physically contiguous
allocation. :-) :-) :-)
To be serious, the right fix is to have some logical page be some power of
two of the physical page when the physical page is too small. Can we hope
Linux-2.5 to allow this?
G?rard.
On Thu, 15 Nov 2001, G?rard Roudier wrote:
>
> To be serious, the right fix is to have some logical page be some power of
> two of the physical page when the physical page is too small. Can we hope
> Linux-2.5 to allow this?
It's certainly doable. I have an i386 patch against 2.4.7 which did that,
MMUPAGE_SIZE 4kB distinguished from PAGE_SIZE 4kB, 8kB, 16kB or 32kB
(but 64kB truncates to 0 in unsigned short b_size, doesn't work so well!);
while still presenting the 4kB EXEC_PAGESIZE to userspace.
It's a bit tedious working through each kernel update, to decide which
PAGEs should be MMUPAGEs etc, and I didn't see an immediate reward of
a huge leap in performance, so I haven't kept it up to date since then.
Hugh
On Thu, 15 Nov 2001, G?rard Roudier wrote:
> On Thu, 15 Nov 2001, Anton Blanchard wrote:
>
> >
> > Hi,
> >
> > I tested the sym-2 driver on ppc64 and found that hcb_p can be > 1 page
> > but __sym_malloc fails for allocations over 1 page. This means we
> > die in sym_attach.
>
> The driver should not need more than 4096 bytes for a single allocation.
> If the ppc64 page size is smaller, your patch is ok, otherwise something
> may have to be fixed, likely in the driver. I cannot access to kernel
> source immediately but I will check what kind of page size ppc64 is using
> asap.
Could you revert your change and give my patch below a try. Btw, you will
be in sync with my current sources. Booting with sym53c8xx=debug:1 will
let the driver print all memory allocations to the syslog. You may send me
the drivers messages related to these allocations for information.
G?rard.
PS: I do have tried the patch on a IA32 machine under linux-2.4.13.
> > With this patch the sym-2 works on ppc64. BTW so far it looks solid :)
>
> Great!
>
> Thanks for your report.
>
> Regards,
> G?rard.
> >
> > Anton
> >
> > diff -urN 2.4.15-pre4/drivers/scsi/sym53c8xx_2/sym_glue.h linuxppc_2_4_devel_work/drivers/scsi/sym53c8xx_2/sym_glue.h
> > --- 2.4.15-pre4/drivers/scsi/sym53c8xx_2/sym_glue.h Thu Nov 15 13:38:02 2001
> > +++ linuxppc_2_4_devel_work/drivers/scsi/sym53c8xx_2/sym_glue.h Tue Nov 13 18:03:07 2001
> > @@ -526,7 +526,7 @@
> > * couple of things related to the memory allocator.
> > */
> > typedef u_long m_addr_t; /* Enough bits to represent any address */
> > -#define SYM_MEM_PAGE_ORDER 0 /* 1 PAGE maximum */
> > +#define SYM_MEM_PAGE_ORDER 1 /* 2 PAGE maximum */
> > #define SYM_MEM_CLUSTER_SHIFT (PAGE_SHIFT+SYM_MEM_PAGE_ORDER)
> > #ifdef MODULE
> > #define SYM_MEM_FREE_UNUSED /* Free unused pages immediately */
diff -u ../sym-2-orig/sym_glue.h ./sym_glue.h
--- ../sym-2-orig/sym_glue.h Thu Nov 15 22:53:34 2001
+++ ./sym_glue.h Thu Nov 15 23:18:58 2001
@@ -77,7 +77,6 @@
#include <linux/errno.h>
#include <linux/pci.h>
#include <linux/string.h>
-#include <linux/malloc.h>
#include <linux/mm.h>
#include <linux/ioport.h>
#include <linux/time.h>
Only in .: sym_glue.o
diff -u ../sym-2-orig/sym_hipd.c ./sym_hipd.c
--- ../sym-2-orig/sym_hipd.c Thu Nov 15 22:53:28 2001
+++ ./sym_hipd.c Thu Nov 15 23:16:03 2001
@@ -4691,6 +4691,7 @@
OUTL_DSP (SCRIPTA_BA (np, clrack));
return;
out_stuck:
+ return;
}
/*
@@ -5226,6 +5227,7 @@
return;
fail:
+ return;
}
/*
@@ -5788,6 +5790,13 @@
goto attach_failed;
/*
+ * Allocate the array of lists of CCBs hashed by DSA.
+ */
+ np->ccbh = sym_calloc(sizeof(ccb_p *)*CCB_HASH_SIZE, "CCBH");
+ if (!np->ccbh)
+ goto attach_failed;
+
+ /*
* Initialyze the CCB free and busy queues.
*/
sym_que_init(&np->free_ccbq);
@@ -5978,6 +5987,8 @@
sym_mfree_dma(cp, sizeof(*cp), "CCB");
}
}
+ if (np->ccbh)
+ sym_mfree(np->ccbh, sizeof(ccb_p *)*CCB_HASH_SIZE, "CCBH");
if (np->badluntbl)
sym_mfree_dma(np->badluntbl, 256,"BADLUNTBL");
diff -u ../sym-2-orig/sym_hipd.h ./sym_hipd.h
--- ../sym-2-orig/sym_hipd.h Thu Nov 15 22:53:34 2001
+++ ./sym_hipd.h Thu Nov 15 22:54:31 2001
@@ -1068,7 +1068,8 @@
/*
* CCB lists and queue.
*/
- ccb_p ccbh[CCB_HASH_SIZE]; /* CCB hashed by DSA value */
+ ccb_p *ccbh; /* CCBs hashed by DSA value */
+ /* CCB_HASH_SIZE lists of CCBs */
SYM_QUEHEAD free_ccbq; /* Queue of available CCBs */
SYM_QUEHEAD busy_ccbq; /* Queue of busy CCBs */
diff -u ../sym-2-orig/sym_nvram.c ./sym_nvram.c
--- ../sym-2-orig/sym_nvram.c Thu Nov 15 22:53:28 2001
+++ ./sym_nvram.c Thu Nov 15 22:54:25 2001
@@ -505,10 +505,10 @@
return retv;
}
-#undef SET_BIT 0
-#undef CLR_BIT 1
-#undef SET_CLK 2
-#undef CLR_CLK 3
+#undef SET_BIT
+#undef CLR_BIT
+#undef SET_CLK
+#undef CLR_CLK
/*
* Try reading Symbios NVRAM.
From: G?rard Roudier <[email protected]>
Date: Thu, 15 Nov 2001 20:46:34 +0100 (CET)
diff -u ../sym-2-orig/sym_glue.h ./sym_glue.h
--- ../sym-2-orig/sym_glue.h Thu Nov 15 22:53:34 2001
+++ ./sym_glue.h Thu Nov 15 23:18:58 2001
@@ -77,7 +77,6 @@
#include <linux/errno.h>
#include <linux/pci.h>
#include <linux/string.h>
-#include <linux/malloc.h>
#include <linux/mm.h>
#include <linux/ioport.h>
#include <linux/time.h>
Hmmm, why not add linux/slab.h? It exists in every Linux kernel tree
your driver would ever be compiled under.
?:.?˛???m???ka??b???zwm??b????˛???m?b??????z_???^n?r???z???h?????&???z??z?ޗ?+??+zf???h???~????i?????????z_???j:+v???)ߣ?m?S?y? ?杶????i?????? ????i
I promise to fix my mailer when responding to non-ascii
postings, sigh :(
On Thu, 15 Nov 2001, Hugh Dickins wrote:
> On Thu, 15 Nov 2001, G?rard Roudier wrote:
> >
> > To be serious, the right fix is to have some logical page be some power of
> > two of the physical page when the physical page is too small. Can we hope
> > Linux-2.5 to allow this?
>
> It's certainly doable. I have an i386 patch against 2.4.7 which did that,
> MMUPAGE_SIZE 4kB distinguished from PAGE_SIZE 4kB, 8kB, 16kB or 32kB
> (but 64kB truncates to 0 in unsigned short b_size, doesn't work so well!);
> while still presenting the 4kB EXEC_PAGESIZE to userspace.
>
> It's a bit tedious working through each kernel update, to decide which
> PAGEs should be MMUPAGEs etc, and I didn't see an immediate reward of
> a huge leap in performance, so I haven't kept it up to date since then.
Think about portable _software_ as large parts of the kernel that have to
cope with very different _hardware_ allocation granularities depending on
targetted platforms. This led and still leads to useless complexity in
many places. Apart this that will require to modify the kernel code in
order to gain full advantage of a new larger page size, you will get some
not negligible immediate reward on IA32, as for example fork() to be
really unlikely to fail on loaded machines, etc..., etc...
G?rard.
On Thu, 15 Nov 2001, David S. Miller wrote:
> From: G?rard Roudier <[email protected]>
> Date: Thu, 15 Nov 2001 20:46:34 +0100 (CET)
>
> diff -u ../sym-2-orig/sym_glue.h ./sym_glue.h
> --- ../sym-2-orig/sym_glue.h Thu Nov 15 22:53:34 2001
> +++ ./sym_glue.h Thu Nov 15 23:18:58 2001
> @@ -77,7 +77,6 @@
> #include <linux/errno.h>
> #include <linux/pci.h>
> #include <linux/string.h>
> -#include <linux/malloc.h>
> #include <linux/mm.h>
> #include <linux/ioport.h>
> #include <linux/time.h>
>
> Hmmm, why not add linux/slab.h? It exists in every Linux kernel tree
> your driver would ever be compiled under.
Just because the driver has its own allocator as you know and thus does
not need this header (it didn't need malloc.h too).
G?rard.
Hi,
> Could you revert your change and give my patch below a try. Btw, you will
> be in sync with my current sources. Booting with sym53c8xx=debug:1 will
> let the driver print all memory allocations to the syslog. You may send me
> the drivers messages related to these allocations for information.
Thanks, it boots OK now. Do you still want a debug log?
BTW on ppc64 we can have io port addresses > 32 bits so this change is
required.
Anton
diff -urN linuxppc_2_4_devel/drivers/scsi/sym53c8xx_2/sym_glue.h linuxppc_2_4_devel_work/drivers/scsi/sym53c8xx_2/sym_glue.h
--- linuxppc_2_4_devel/drivers/scsi/sym53c8xx_2/sym_glue.h Mon Nov 12 11:46:42 2001
+++ linuxppc_2_4_devel_work/drivers/scsi/sym53c8xx_2/sym_glue.h Tue Nov 20 16:35:14 2001
@@ -463,7 +462,7 @@
vm_offset_t mmio_va; /* MMIO kernel virtual address */
vm_offset_t ram_va; /* RAM kernel virtual address */
- u32 io_port; /* IO port address */
+ u_long io_port; /* IO port address */
u_short io_ws; /* IO window size */
int irq; /* IRQ number */
Hello,
On Tue, 20 Nov 2001, Anton Blanchard wrote:
> Hi,
>
> > Could you revert your change and give my patch below a try. Btw, you will
> > be in sync with my current sources. Booting with sym53c8xx=debug:1 will
> > let the driver print all memory allocations to the syslog. You may send me
> > the drivers messages related to these allocations for information.
>
> Thanks, it boots OK now. Do you still want a debug log?
I can guess the result.
> BTW on ppc64 we can have io port addresses > 32 bits so this change is
> required.
Linux/ppc64 looks strange invention to me. As you know IO base addresses
are limited to 32 bit in PCI. And, btw, 32 bits seems to work just fine
here as PPC is defined from the driver as using normal IO. But, IIRC, the
strange Linux/PPC invention only supports MMIO. :-)
If you want to play with _explicit_ MMIO, you just have to remove a couple
of line from sym53c8xx.h. Here they are:
/*
* Use normal IO if configured. Forced for alpha and powerpc.
* Powerpc fails copying to on-chip RAM using memcpy_toio().
* Forced to MMIO for sparc.
*/
#if defined(__alpha__)
#define SYM_CONF_IOMAPPED
#elif defined(__powerpc__)
- #define SYM_CONF_IOMAPPED
- #define SYM_OPT_NO_BUS_MEMORY_MAPPING
#elif defined(__sparc__)
#undef SYM_CONF_IOMAPPED
#elif defined(CONFIG_SCSI_SYM53C8XX_IOMAPPED)
#define SYM_CONF_IOMAPPED
#endif
Btw, I cannot guess the result here. You may want to really let me know
this time. :)
I cannot apply your patch as it is, since I want the driver to distinguish
between kernel fake cookies associated with base addresses and actual
values of those registers. This is needed, since some of these values must
be known from SCSI SCRIPTS and thus must fit the _reality_ and not any
kernel developpers' dream, could be the greatest ones:).
Thanks, anyway, for reporting this problem.
Regards,
G?rard.
> diff -urN linuxppc_2_4_devel/drivers/scsi/sym53c8xx_2/sym_glue.h linuxppc_2_4_devel_work/drivers/scsi/sym53c8xx_2/sym_glue.h
> --- linuxppc_2_4_devel/drivers/scsi/sym53c8xx_2/sym_glue.h Mon Nov 12 11:46:42 2001
> +++ linuxppc_2_4_devel_work/drivers/scsi/sym53c8xx_2/sym_glue.h Tue Nov 20 16:35:14 2001
> @@ -463,7 +462,7 @@
>
> vm_offset_t mmio_va; /* MMIO kernel virtual address */
> vm_offset_t ram_va; /* RAM kernel virtual address */
> - u32 io_port; /* IO port address */
> + u_long io_port; /* IO port address */
> u_short io_ws; /* IO window size */
> int irq; /* IRQ number */
Hi,
> Linux/ppc64 looks strange invention to me. As you know IO base addresses
> are limited to 32 bit in PCI. And, btw, 32 bits seems to work just fine
> here as PPC is defined from the driver as using normal IO. But, IIRC, the
> strange Linux/PPC invention only supports MMIO. :-)
Since all PCI IO is memory mapped on ppc64, IO addresses end up > 32 bits.
Until recently we used to have an IO offset that we added to all accesses
which kept the driver visible IO addresses < 32 bits. (This is still the
case with ppc32)
The change was made to support error handling, the 64 bit token has the
pci bus,dev,fn embedded in it so that the low level IO routines can do
error recovery if in{b,w,l} fails.
I didnt make these changes and it would seem we can link IO address <->
pci bus,dev,fn in other ways, if it turns out many drivers cannot use u64
for IO ports then we will have to investigate them.
> If you want to play with _explicit_ MMIO, you just have to remove a couple
> of line from sym53c8xx.h. Here they are:
Yes MMIO works fine. Is there a reason we force PCI IO on __powerpc__?
Anton
From: Anton Blanchard <[email protected]>
Date: Wed, 21 Nov 2001 13:19:01 +1100
I didnt make these changes and it would seem we can link IO address <->
pci bus,dev,fn in other ways, if it turns out many drivers cannot use u64
for IO ports then we will have to investigate them.
Any driver which cannot handle PCI I/O resources being an unsigned
long is completely broken.
They are 64-bits on Sparc64 already, and this is the type of
PCI resource values.
On Wed, 21 Nov 2001, Anton Blanchard wrote:
> Hi,
>
> > Linux/ppc64 looks strange invention to me. As you know IO base addresses
> > are limited to 32 bit in PCI. And, btw, 32 bits seems to work just fine
> > here as PPC is defined from the driver as using normal IO. But, IIRC, the
> > strange Linux/PPC invention only supports MMIO. :-)
>
> Since all PCI IO is memory mapped on ppc64, IO addresses end up > 32 bits.
> Until recently we used to have an IO offset that we added to all accesses
> which kept the driver visible IO addresses < 32 bits. (This is still the
> case with ppc32)
>
> The change was made to support error handling, the 64 bit token has the
> pci bus,dev,fn embedded in it so that the low level IO routines can do
> error recovery if in{b,w,l} fails.
When it happens to implement a driver that involves both C code and some
firmware that works on the PCI-chip, using opaque handles as for C is not
enough. If you think your driver only with those abstractions, you will
get a bag of bits that does not work.
In such situations C abstractions are just dreams of some ideal world that
does not exists and that will never do. Having in mind and in some places
of the code the _reality_ is required if you want your stuff to have
chance to work.
> I didnt make these changes and it would seem we can link IO address <->
> pci bus,dev,fn in other ways, if it turns out many drivers cannot use u64
> for IO ports then we will have to investigate them.
The sym driver can do all what I wrote about regarding DMA addressing,
given expected kernel interfaces and appropriate hardware.
> > If you want to play with _explicit_ MMIO, you just have to remove a couple
> > of line from sym53c8xx.h. Here they are:
>
> Yes MMIO works fine. Is there a reason we force PCI IO on __powerpc__?
My opinion, may-be just 0.02 euros:
At the time of early PPC port, only a few courageous PCI device drivers
wanted to default to PCI-recommended MMIO. There were a least 1 driver
that wanted so, but not may more :). At this early time, the MMIO kernel
interface in ppc port was not behaving as expected and MMIO wasn't usable.
This lasted years. I would be very glad if MMIO related kernel interface
is now fine, but allow me to still doubt a bit about.
If Linux/ppc team can ensure that Linux/ppc is now ok for MMIO, then you
can send the corresponding patch, that default PPC to MMIO as seen by the
driver, to kernel maintainer(s) with my _full_ approbation.
Regards,
G?rard.
Gerard Roudier writes:
> If Linux/ppc team can ensure that Linux/ppc is now ok for MMIO, then you
> can send the corresponding patch, that default PPC to MMIO as seen by the
> driver, to kernel maintainer(s) with my _full_ approbation.
Yes, Linux/PPC now does MMIO just fine.
Paul.