Yet another ccio fix idea?

Yet another ccio fix idea?

[Joel Soete]

Hello Grant,

Always tracking this ccio-dma bug, I figure out this stuff could be helpfull:
--- a/drivers/parisc/ccio-dma.c	2008-03-09 12:55:00.000000000 +0000
+++ b/drivers/parisc/ccio-dma.c	2008-03-09 12:55:21.000000000 +0000
@@ -802,7 +802,7 @@
  			 * Hopefully someone figures out how to patch (NOP) the
  			 * FDC/SYNC out at boot time.
  			 */
-			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr[7]));
+			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr));

What's your opinion?

Tx,
	r.

Re: Yet another ccio fix idea?

[Grant Grundler]

On Sun, Mar 09, 2008 at 01:55:15PM +0000, Joel Soete wrote:
> Hello Grant,
>
> Always tracking this ccio-dma bug, I figure out this stuff could be 
> helpfull:
> --- a/drivers/parisc/ccio-dma.c	2008-03-09 12:55:00.000000000 +0000
> +++ b/drivers/parisc/ccio-dma.c	2008-03-09 12:55:21.000000000 +0000
> @@ -802,7 +802,7 @@
>  			 * Hopefully someone figures out how to patch (NOP) the
>  			 * FDC/SYNC out at boot time.
>  			 */
> -			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr[7]));
> +			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr));
>
> What's your opinion?

I think you are right. That's what sba_iommu.c is using on line 560.

Have you tested this to see if it solves your problem?

cheers,
grant

Re: Yet another ccio fix idea?

[Grant Grundler]

On Sun, Mar 09, 2008 at 01:55:15PM +0000, Joel Soete wrote:
> Hello Grant,
>
> Always tracking this ccio-dma bug, I figure out this stuff could be 
> helpfull:
> --- a/drivers/parisc/ccio-dma.c	2008-03-09 12:55:00.000000000 +0000
> +++ b/drivers/parisc/ccio-dma.c	2008-03-09 12:55:21.000000000 +0000
> @@ -802,7 +802,7 @@
>  			 * Hopefully someone figures out how to patch (NOP) the
>  			 * FDC/SYNC out at boot time.
>  			 */
> -			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr[7]));
> +			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr));

BTW, once you've tested/demostrated this fixes the DMA problems on C-xxx,
please resend with "Signed-off-by:" line.

thanks,
grant

>
> What's your opinion?
>
> Tx,
> 	r.

Re: Yet another ccio fix idea?

[James Bottomley]

On Sun, 2008-03-09 at 13:55 +0000, Joel Soete wrote:
> Hello Grant,
> 
> Always tracking this ccio-dma bug, I figure out this stuff could be helpfull:
> --- a/drivers/parisc/ccio-dma.c	2008-03-09 12:55:00.000000000 +0000
> +++ b/drivers/parisc/ccio-dma.c	2008-03-09 12:55:21.000000000 +0000
> @@ -802,7 +802,7 @@
>   			 * Hopefully someone figures out how to patch (NOP) the
>   			 * FDC/SYNC out at boot time.
>   			 */
> -			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr[7]));
> +			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr));

That's the wrong fix.  What it's trying to do is flush the write made
above this to pdir_ptr[7].  Therefore there should be an '&' in front,
so

asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[7]))

Flushing pdir_ptr may or may not work depending on the cache line width
(pdr_ptr[7] is 28 bytes away, so if the line size is 16 it will fail;
although I think almost every machine with a ccio has a 32 byte or
higher cache line width).

James

Re: Yet another ccio fix idea?

[John David Anglin]

> asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[7]))
> 
> Flushing pdir_ptr may or may not work depending on the cache line width
> (pdr_ptr[7] is 28 bytes away, so if the line size is 16 it will fail;
> although I think almost every machine with a ccio has a 32 byte or
> higher cache line width).

Actually, pdr_ptr is a char *, so this is 7 bytes away.  However,
I think you are correct in saying that pdr_ptr[7] needs to be flushed.

The same issue would appear to apply to the other flush in the file.

The cache line alignment of pdir's isn't immediately apparent to
me.  If the pdir was defined as a two word array, it might span
two lines.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: Yet another ccio fix idea?

[Grant Grundler]

On Mon, Mar 10, 2008 at 02:59:47PM -0500, James Bottomley wrote:
> On Sun, 2008-03-09 at 13:55 +0000, Joel Soete wrote:
> > Hello Grant,
> > 
> > Always tracking this ccio-dma bug, I figure out this stuff could be helpfull:
> > --- a/drivers/parisc/ccio-dma.c	2008-03-09 12:55:00.000000000 +0000
> > +++ b/drivers/parisc/ccio-dma.c	2008-03-09 12:55:21.000000000 +0000
> > @@ -802,7 +802,7 @@
> >   			 * Hopefully someone figures out how to patch (NOP) the
> >   			 * FDC/SYNC out at boot time.
> >   			 */
> > -			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr[7]));
> > +			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr));
> 
> That's the wrong fix.  What it's trying to do is flush the write made
> above this to pdir_ptr[7].  Therefore there should be an '&' in front,
> so
> 
> asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[7]))

In this case, it's certainly the same cacheline.
I can guarantee pdir_ptr is 8 byte aligned.

> Flushing pdir_ptr may or may not work depending on the cache line width
> (pdr_ptr[7] is 28 bytes away, so if the line size is 16 it will fail;
> although I think almost every machine with a ccio has a 32 byte or
> higher cache line width).

Yes (32-byte cachelines at least) and jda already pointed out pdir_ptr
is a char*.

thanks,
grant

> 
> James
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Yet another ccio fix idea?

[rubisher]

> On Sun, Mar 09, 2008 at 01:55:15PM +0000, Joel Soete wrote:
> > Hello Grant,
> >
> > Always tracking this ccio-dma bug, I figure out this stuff could be 
> > helpfull:
> > --- a/drivers/parisc/ccio-dma.c	2008-03-09 12:55:00.000000000 +0000
> > +++ b/drivers/parisc/ccio-dma.c	2008-03-09 12:55:21.000000000 +0000
> > @@ -802,7 +802,7 @@
> >  			 * Hopefully someone figures out how to patch (NOP) the
> >  			 * FDC/SYNC out at boot time.
> >  			 */
> > -			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr[7]));
> > +			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr));
> 
> BTW, once you've tested/demostrated this fixes the DMA problems on C-xxx,
> please resend with "Signed-off-by:" line.
> 
I would very to say: "Yes it works" like make me falsely guess this test on a
d380 with an disk connected to ncr53c720 hba:
# CNT=0; while true; do nice -n -5 tar -xslpf linux-2.6-trace.tar; nice -n -5
rm -rf linux-2.6-trace; CNT=$(($CNT + 1)); echo "CNT=$CNT"; date; done

it run several hours without issue, till console shows me again a failure in
the fs:
#CNT=1
#Mon Mar 10 12:33:54 CET 2008
#CNT=2
#Mon Mar 10 12:35:14 CET 2008

#CNT=190
#Mon Mar 10 16:32:29 CET 2008
#CNT=191
#Mon Mar 10 16:33:44 CET 2008

But again same kind of error:
#EXT3-fs error (device sdb9): ext3_readdir: bad entry in directory #99187:
rec_len % 4 != 0 - offset=0, inode=1953393
#EXT3-fs warning (device sdb9): empty_dir: bad directory (dir #99187) - no `.'
or `..'          

Note that the 53c710 of the external drive didn't shows much better results:
#scsi3: (3:0) phase mismatch at 01e8, phase IO CD MSG BSY REQ MSG IN         
                                       
#scsi3: Bus Reset detected, executing command 1fe1f2c0, slot 1fe388a4, dsp
003781e8[01e8]                                            
# failing command because of reset, slot 1fe388a4, cmnd 1fe1f2c0

just during a dist-upgrade.

Sorry,
    r.

> thanks,
> grant
> 
> >
> > What's your opinion?
> >
> > Tx,
> > 	r.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
---
Scarlet One, ADSL 6 Mbps + Telephone, from EUR 29,95...
http://www.scarlet.be/

Re: Yet another ccio fix idea?

[John David Anglin]

> > On Sun, Mar 09, 2008 at 01:55:15PM +0000, Joel Soete wrote:
> > > Hello Grant,
> > >
> > > Always tracking this ccio-dma bug, I figure out this stuff could be 
> > > helpfull:
> > > --- a/drivers/parisc/ccio-dma.c	2008-03-09 12:55:00.000000000 +0000
> > > +++ b/drivers/parisc/ccio-dma.c	2008-03-09 12:55:21.000000000 +0000
> > > @@ -802,7 +802,7 @@
> > >  			 * Hopefully someone figures out how to patch (NOP) the
> > >  			 * FDC/SYNC out at boot time.
> > >  			 */
> > > -			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr[7]));
> > > +			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr));
> > 
> > BTW, once you've tested/demostrated this fixes the DMA problems on C-xxx,
> > please resend with "Signed-off-by:" line.

The existing code is clearly wrong.  pdir_ptr[7] is not a pointer and
the previous statement sets it to zero to clear the valid bit.  So, don't
drop this line of investigation.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: Yet another ccio fix idea?

[Grant Grundler]

On Mon, Mar 10, 2008 at 01:26:43PM -0400, John David Anglin wrote:
> > > On Sun, Mar 09, 2008 at 01:55:15PM +0000, Joel Soete wrote:
> > > > Hello Grant,
> > > >
> > > > Always tracking this ccio-dma bug, I figure out this stuff could be 
> > > > helpfull:
> > > > --- a/drivers/parisc/ccio-dma.c	2008-03-09 12:55:00.000000000 +0000
> > > > +++ b/drivers/parisc/ccio-dma.c	2008-03-09 12:55:21.000000000 +0000
> > > > @@ -802,7 +802,7 @@
> > > >  			 * Hopefully someone figures out how to patch (NOP) the
> > > >  			 * FDC/SYNC out at boot time.
> > > >  			 */
> > > > -			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr[7]));
> > > > +			asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr));
> > > 
> > > BTW, once you've tested/demostrated this fixes the DMA problems on C-xxx,
> > > please resend with "Signed-off-by:" line.
> 
> The existing code is clearly wrong.  pdir_ptr[7] is not a pointer and
> the previous statement sets it to zero to clear the valid bit.  So, don't
> drop this line of investigation.

Dave is right. Please submit the proper patch (with Signed-off-by)
and I'll Ack it.

I was hoping testing this would "just work".

thanks,
grant

> 
> Dave
> -- 
> J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
> National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: Yet another ccio fix idea?

[rubisher]

> > asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[7]))
> > 
> > Flushing pdir_ptr may or may not work depending on the cache line width
> > (pdr_ptr[7] is 28 bytes away, so if the line size is 16 it will fail;
> > although I think almost every machine with a ccio has a 32 byte or
> > higher cache line width).
> 
> Actually, pdr_ptr is a char *, so this is 7 bytes away.  However,
> I think you are correct in saying that pdr_ptr[7] needs to be flushed.
> 
> The same issue would appear to apply to the other flush in the file.
> 
> The cache line alignment of pdir's isn't immediately apparent to
> me.  If the pdir was defined as a two word array, it might span
> two lines.
> 
> Dave
> -- 
> J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
> National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
mmm follwing stuff shouldn't so be enough:
Index: b/drivers/parisc/ccio-dma.c
===================================================================
--- a/drivers/parisc/ccio-dma.c 2008-03-12 08:49:20.000000000 +0000
+++ b/drivers/parisc/ccio-dma.c 2008-03-12 08:50:54.000000000 +0000
@@ -624,7 +624,8 @@
        ** the real mode coherence index generation of U2, the PDIR entry
        ** must be flushed to memory to retain coherence."
        */
-       asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr));
+       asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[1]));
+       asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[0]));
        asm volatile("sync");
 }

@@ -695,7 +696,7 @@
                ** Hopefully someone figures out how to patch (NOP) the
                ** FDC/SYNC out at boot time.
                */
-               asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr[7]));
+               asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[7]));

                iovp     += IOVP_SIZE;
                byte_cnt -= IOVP_SIZE;
=== <> ===

Specialy for the first hunk where pdir_ptr is a u64 *:
ccio_io_pdir_entry(u64 *pdir_ptr, space_t sid, unsigned long vba,
                   unsigned long hints)

into which with temporary pa var (defined as an unsigned long)
        register unsigned long pa;

*pdir_ptr is changed like
        ((u32 *)pdir_ptr)[1] = (u32) pa;
[snip]
        ((u32 *)pdir_ptr)[0] = (u32) pa;

What would be the best way to works?

Tx,
    r.

PS: in this doc feb96a6.pdf (easy to find) it was said:
"[snip]
Entries in the PA 7200 and PA 8000 caches are stored in lines
of 32 bytes.
[snip]

Because one-word writes occur in the I/O system,
for registers, semaphores, or short DMA writes it was necessary that the I/O
adapter implement a one-line-deep cache to
buffer cache lines, so that these one-word writes could be executed by
performing a coherent read private transaction on
the Runway bus, obtaining the most recent copy of the cache line, modifying it
locally in cache, and finally writing the
modified line back to main memory. For the I/O adapter to support a cache on
the Runway bus, it has to have the ability to
compare processor-generated virtual address transactions with the address
contained in its cache to ensure that the
processors always receive the most up-to-date data.
[s
[snip]"

2 thinks:
    - it seems to confirm that cache line is well 32bytes wide ;-)

    - for 'one word writes...' how should it be implemented?
      It look like the sba_iommu driver:
      "READ_REG(ioc-ioc_ha_IOC_PCOM); /* flush purges */
      but without more detailed docs on U2/UTurn ccio, I don't know how to
      implement matter, sorry.

 

---
Scarlet One, ADSL 6 Mbps + Telephone, from EUR 29,95...
http://www.scarlet.be/

Re: Yet another ccio fix idea?

[James Bottomley]

On Wed, 2008-03-12 at 10:16 +0100, rubisher wrote:
> -       asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr));
> +       asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[1]));
> +       asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[0]));
>         asm volatile("sync");

Actually, no, I'm afraid this is wrong.

The code is a complete dogs breakfast, since pdir_ptr is defined as u64
* in ccio_pdir_entry() and char * in ccio_mark_invalid.

What Dave was pointing out is that this structure is 8 bytes long and
may span two cache lines on the word boundary.  Assuming that's the
case, this needs to flush the first and second word, not the first and
second u64 entry as your code would (the second flush is actually
flushing the next pdir entry).

However, since it's contained in a u64 pointer, it must reasonably be on
an 8 byte boundary (or be triggering an unaligned poiner error) and
therefore shouldn't span multiple cache lines anyway.

James

Re: Yet another ccio fix idea?

[John David Anglin]

> However, since it's contained in a u64 pointer, it must reasonably be on
> an 8 byte boundary (or be triggering an unaligned poiner error) and
> therefore shouldn't span multiple cache lines anyway.

It's not a hard rule that u64 objects be aligned on an 8 byte
boundary as the PA 1.x architecture doesn't have 64-bit integer
loads and stores.  A u64 object has to be loaded with two
32-bit loads and lives in a pair of 32-bit registers.  As a 
result, an unaligned u64 object will only trigger an unaligned
pointer error if it is not aligned on a 4 byte boundary.

However in GCC, we force u64 alignment (except in packed structs)
to a 8 byte boundary.  This allows casting pointers between long
longs and doubles.  It also simplifies the GCC backend as otherwise
special treatment would be needed for long longs.  There's been some
discussion of this for x86 recently.

In this situation, the alignment of pdir's depends on how they
were originally declared.  If declared as long long, GCC will give
them 8 byte alignment, so the two words should always be in the
same cache line.  On the otherhand, if the pdir is declared as
a pair of 32-bit words, the pdir might span two cache lines.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: Yet another ccio fix idea?

[John David Anglin]

> In this situation, the alignment of pdir's depends on how they
> were originally declared.  If declared as long long, GCC will give
> them 8 byte alignment, so the two words should always be in the
> same cache line.  On the otherhand, if the pdir is declared as
> a pair of 32-bit words, the pdir might span two cache lines.

It looks to me like pdirs should be 8 byte aligned and thus always
should lie in a cache line.

The following insn appears to put hints in the wrong place on
a 64-bit kernel since `pa' is then a 64-bit type:

        asm volatile("depw  %1,31,12,%0" : "+r" (pa) : "r" (hints));

The depw will operate on the left word.  Looks like it should be

        asm volatile("depw  %1,31,12,%R0" : "+r" (pa) : "r" (hints));

when __LP64__ is defined.  Think the same is true for the following
depw as well.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: Yet another ccio fix idea?

[Grant Grundler]

On Wed, Mar 12, 2008 at 07:47:22AM -0400, James Bottomley wrote:
> On Wed, 2008-03-12 at 10:16 +0100, rubisher wrote:
> > -       asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr));
> > +       asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[1]));
> > +       asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[0]));
> >         asm volatile("sync");
> 
> Actually, no, I'm afraid this is wrong.
> 
> The code is a complete dogs breakfast, since pdir_ptr is defined as u64
> * in ccio_pdir_entry() and char * in ccio_mark_invalid.

*ugh* sorry about that.
And pdir_ptr cast to "u32 *" in both uses where it's dereferenced.


> What Dave was pointing out is that this structure is 8 bytes long and
> may span two cache lines on the word boundary.  Assuming that's the
> case, this needs to flush the first and second word, not the first and
> second u64 entry as your code would (the second flush is actually
> flushing the next pdir entry).
> 
> However, since it's contained in a u64 pointer, it must reasonably be on
> an 8 byte boundary (or be triggering an unaligned poiner error) and
> therefore shouldn't span multiple cache lines anyway.

pdir_ptr will always point at a u64 * and will always be 8 byte aligned.
The allocation is done in ccio_ioc_init():
        ioc->pdir_base = (u64 *)__get_free_pages(GFP_KERNEL,
                                                 get_order(ioc->pdir_size));

hth,
grant

Re: Yet another ccio fix idea?

[Grant Grundler]

On Wed, Mar 12, 2008 at 10:16:29AM +0100, rubisher wrote:
...
> mmm follwing stuff shouldn't so be enough:
> Index: b/drivers/parisc/ccio-dma.c
> ===================================================================
> --- a/drivers/parisc/ccio-dma.c 2008-03-12 08:49:20.000000000 +0000
> +++ b/drivers/parisc/ccio-dma.c 2008-03-12 08:50:54.000000000 +0000
> @@ -624,7 +624,8 @@
>         ** the real mode coherence index generation of U2, the PDIR entry
>         ** must be flushed to memory to retain coherence."
>         */
> -       asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr));
> +       asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[1]));
> +       asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[0]));

NACK

>         asm volatile("sync");
>  }
> 
> @@ -695,7 +696,7 @@
>                 ** Hopefully someone figures out how to patch (NOP) the
>                 ** FDC/SYNC out at boot time.
>                 */
> -               asm volatile("fdc %%r0(%0)" : : "r" (pdir_ptr[7]));
> +               asm volatile("fdc %%r0(%0)" : : "r" (&pdir_ptr[7]));

NACK - keep it simple with the original proposed patch.

> PS: in this doc feb96a6.pdf (easy to find) it was said:
> "[snip]
> Entries in the PA 7200 and PA 8000 caches are stored in lines
> of 32 bytes.
> [snip]
> 
> Because one-word writes occur in the I/O system, for registers,
> semaphores, or short DMA writes it was necessary that the I/O
> adapter implement a one-line-deep cache to buffer cache lines,
> so that these one-word writes could be executed by performing
> a coherent read private transaction on the Runway bus, obtaining
> the most recent copy of the cache line, modifying it locally in
> cache, and finally writing the modified line back to main memory.
...

> 2 thinks:
>     - it seems to confirm that cache line is well 32bytes wide ;-)
> 
>     - for 'one word writes...' how should it be implemented?

"one word writes" refers to DMA writes, not CPU writes.

>       It look like the sba_iommu driver:
>       "READ_REG(ioc-ioc_ha_IOC_PCOM); /* flush purges */
>       but without more detailed docs on U2/UTurn ccio, I don't know how to
>       implement matter, sorry.

The IOMMU driver programs the IO Pdir with the results of "lci" instruction
and this should be all the IOMMU needs to coherently read/modify/write
the cacheline that the "one word" DMA write is targeting. In short,
it's already implemented or I'd think we'd see much worse problems
on ccio platforms.

hth,
grant

> 
> 
> 
> ---
> Scarlet One, ADSL 6 Mbps + Telephone, from EUR 29,95...
> http://www.scarlet.be/

Re: Yet another ccio fix idea?

[rubisher]

Grant Grundler wrote:
[snip]
>> PS: in this doc feb96a6.pdf (easy to find) it was said:
>> "[snip]
>> Entries in the PA 7200 and PA 8000 caches are stored in lines
>> of 32 bytes.
>> [snip]
>>
>> Because one-word writes occur in the I/O system, for registers,
>> semaphores, or short DMA writes it was necessary that the I/O
>> adapter implement a one-line-deep cache to buffer cache lines,
>> so that these one-word writes could be executed by performing
>> a coherent read private transaction on the Runway bus, obtaining
>> the most recent copy of the cache line, modifying it locally in
>> cache, and finally writing the modified line back to main memory.
> ...
> 
>> 2 thinks:
>>     - it seems to confirm that cache line is well 32bytes wide ;-)
>>
>>     - for 'one word writes...' how should it be implemented?
> 
> "one word writes" refers to DMA writes, not CPU writes.
> 
(tx it's not always easy to follow the master and the slave in this story)

>>       It look like the sba_iommu driver:
>>       "READ_REG(ioc-ioc_ha_IOC_PCOM); /* flush purges */
>>       but without more detailed docs on U2/UTurn ccio, I don't know how to
>>       implement matter, sorry.
> 
> The IOMMU driver programs the IO Pdir with the results of "lci" instruction
> and this should be all the IOMMU needs to coherently read/modify/write
> the cacheline that the "one word" DMA write is targeting. In short,
> it's already implemented or I'd think we'd see much worse problems
> on ccio platforms.
> 
> hth,
> grant
> 
Ok.

Just one thought because in the backport I did on ccio of your job on sba (which seems to works for me as well as previous 
stuff, may be a bit better for system low in ram like my c110), it was one of the matter related to io coherency which I 
didn't reach to backport?

Tx again,
	r.

Re: Yet another ccio fix idea?

[rubisher]

> > In this situation, the alignment of pdir's depends on how they
> > were originally declared.  If declared as long long, GCC will give
> > them 8 byte alignment, so the two words should always be in the
> > same cache line.  On the otherhand, if the pdir is declared as
> > a pair of 32-bit words, the pdir might span two cache lines.
> 
> It looks to me like pdirs should be 8 byte aligned and thus always
> should lie in a cache line.
> 
> The following insn appears to put hints in the wrong place on
> a 64-bit kernel since `pa' is then a 64-bit type:
> 
>         asm volatile("depw  %1,31,12,%0" : "+r" (pa) : "r" (hints));
> 
> The depw will operate on the left word.  Looks like it should be
> 
>         asm volatile("depw  %1,31,12,%R0" : "+r" (pa) : "r" (hints));
> 
As far as I never reach to boot a 64bit kernel on this box, I would like to
test but am I facing to a gcc bug or did I again miss something:
{standard input}: Assembler messages:
{standard input}:97: Error: Field out of range [0..31] (68).
{standard input}:97: Error: Invalid operands
{standard input}:1199: Error: Field out of range [0..31] (68).
{standard input}:1199: Error: Invalid operands
make[3]: *** [drivers/parisc/ccio-dma.o] Error 1

(The original compile fine???)

> when __LP64__ is defined.  Think the same is true for the following
> depw as well.
> 
Ok I will check in deep.

Tx,
    r.
> Dave
> -- 
> J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
> National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
---
Scarlet One, ADSL 6 Mbps + Telephone, from EUR 29,95...
http://www.scarlet.be/

Re: Yet another ccio fix idea?

[John David Anglin]

> > The depw will operate on the left word.  Looks like it should be
> > 
> >         asm volatile("depw  %1,31,12,%R0" : "+r" (pa) : "r" (hints));

Sorry, the `R' to access the right half of pa is needed when generating
32-bit code.  The original form might be ok when generating 64-bit
code but I'd have to check the arch.  With 64-bit code, you might need
to use depd.  The other approach might be to change pa to u32.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: Yet another ccio fix idea?

[rubisher]

John David Anglin wrote:
>>> The depw will operate on the left word.  Looks like it should be
>>>
>>>         asm volatile("depw  %1,31,12,%R0" : "+r" (pa) : "r" (hints));
> 
> Sorry, the `R' to access the right half of pa is needed when generating
> 32-bit code.  The original form might be ok when generating 64-bit
> code but I'd have to check the arch.  With 64-bit code, you might need
> to use depd.  The other approach might be to change pa to u32.
> 
> Dave
Ok if I have some time next week I will check.

Tx,
	r.

Re: Yet another ccio fix idea?

[rubisher]

> > > In this situation, the alignment of pdir's depends on how they
> > > were originally declared.  If declared as long long, GCC will give
> > > them 8 byte alignment, so the two words should always be in the
> > > same cache line.  On the otherhand, if the pdir is declared as
> > > a pair of 32-bit words, the pdir might span two cache lines.
> > 
> > It looks to me like pdirs should be 8 byte aligned and thus always
> > should lie in a cache line.
> > 
> > The following insn appears to put hints in the wrong place on
> > a 64-bit kernel since `pa' is then a 64-bit type:
> > 
> >         asm volatile("depw  %1,31,12,%0" : "+r" (pa) : "r" (hints));
> > 
> > The depw will operate on the left word.  Looks like it should be
> > 
> >         asm volatile("depw  %1,31,12,%R0" : "+r" (pa) : "r" (hints));
> > 
> As far as I never reach to boot a 64bit kernel on this box, I would like to
> test but am I facing to a gcc bug or did I again miss something:
> {standard input}: Assembler messages:
> {standard input}:97: Error: Field out of range [0..31] (68).
> {standard input}:97: Error: Invalid operands
> {standard input}:1199: Error: Field out of range [0..31] (68).
> {standard input}:1199: Error: Invalid operands
> make[3]: *** [drivers/parisc/ccio-dma.o] Error 1
> 
oops I need to better check why does it want to use fr:
     96 #APP
     97         depw  %r23,31,12,%fr4
     98 #NO_APP

   1198 #APP
   1199         depw  %r5,31,12,%fr4
   1200 #NO_APP

I will try to understand that next week.

r.
> (The original compile fine???)
> 
> > when __LP64__ is defined.  Think the same is true for the following
> > depw as well.
> > 
> Ok I will check in deep.
> 
> Tx,
>     r.
> > Dave
> > -- 
> > J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
> > National Research Council of Canada              (613) 990-0752 (FAX:
952-6602)
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> ---
> Scarlet One, ADSL 6 Mbps + Telephone, from EUR 29,95...
> http://www.scarlet.be/
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
---
Scarlet One, ADSL 6 Mbps + Telephone, from EUR 29,95...
http://www.scarlet.be/