Wierd code in Entry.S

Wierd code in Entry.S

[Artem Alimarine]

[-- Attachment #1: Type: text/plain, Size: 968 bytes --]

Hi guys,

I am new to PARISC and to this forum. I have a small question. There is 
an instruction in entry.S that I do not understand. It is in the the 
macro make_insert_tlb

Kernel 2.6.26.2:

 537        /* Enforce uncacheable pages.
 538         * This should ONLY be use for MMIO on PA 2.0 machines.
 539         * Memory/DMA is cache coherent on all PA2.0 machines we 
support
 540         * (that means T-class is NOT supported) and the memory 
controllers
 541         * on most of those machines only handles cache transactions.
 542         */
 543        extrd,u,*=      \pte,_PAGE_NO_CACHE_BIT+32,1,%r0
 544        depi            1,12,1,\prot


The DEPI instruction on a 64-bit machine sets bit 44=32+12,
whereas we use the value as the argument to IDTLBT, which expects bit 12 
to be used instead. Does it mean that the U-bit is never set and the 
authorization id gets corrupted???

Is it a bug or my misunderstanding of the code???

Best regards,
Artem

[-- Attachment #2: artem_alimarine.vcf --]
[-- Type: text/x-vcard, Size: 283 bytes --]

begin:vcard
fn:dr. Artem Alimarine
n:Alimarine;Artem
org:STROMASYS SA
adr:;;De Zaale 11;Eindhoven;;5612AJ;The Netherlands
email;internet:artem.alimarine@stromasys.com
title:Software Architect
tel;work:+31-40-2390863
tel;fax:+31-40-2390800
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: Wierd code in Entry.S

[Grant Grundler]

On Thu, Jul 09, 2009 at 03:11:19PM +0200, Artem Alimarine wrote:
> Hi guys,
>
> I am new to PARISC and to this forum. I have a small question. There is  
> an instruction in entry.S that I do not understand. It is in the the  
> macro make_insert_tlb
>
> Kernel 2.6.26.2:
>
> 537        /* Enforce uncacheable pages.
> 538         * This should ONLY be use for MMIO on PA 2.0 machines.
> 539         * Memory/DMA is cache coherent on all PA2.0 machines we support
> 540         * (that means T-class is NOT supported) and the memory controllers
> 541         * on most of those machines only handles cache transactions.
> 542         */
> 543        extrd,u,*=      \pte,_PAGE_NO_CACHE_BIT+32,1,%r0
> 544        depi            1,12,1,\prot
>
>

The "*=" in line 543 will determine if "depi" instruction (line 544)
gets executed or not. You'll need the "PA-RISC 2.0 Architecture":
    http://www.parisc-linux.org/documentation/index.html
    http://ftp.parisc-linux.org/docs/arch/parisc2.0.pdf

And read page 7-47, 7-48, and Table D-14.


> The DEPI instruction on a 64-bit machine sets bit 44=32+12,
> whereas we use the value as the argument to IDTLBT, which expects bit 12  
> to be used instead.
>
> Does it mean that the U-bit is never set and the  
> authorization id gets corrupted???

U-bit will get set only if _PAGE_NO_CACHE_BIT+32 is also set.

The bit enumeration is *reverse* with MSb being 0 for all ASM instructions
and all references in the PA2.0 Arch manual.

Because this is a 64 bit build, "+32" is needed to refer to the lower half
of the double word (word == 32 bits).

> Is it a bug or my misunderstanding of the code???

It looks correct to me. "12" here always seems to refer to the U-bit as
defined in the PA2.0 Arch manual.

hth,
grant

Re: Wierd code in Entry.S

[John David Anglin]

> On Thu, Jul 09, 2009 at 03:11:19PM +0200, Artem Alimarine wrote:
> > Hi guys,
> >
> > I am new to PARISC and to this forum. I have a small question. There is  
> > an instruction in entry.S that I do not understand. It is in the the  
> > macro make_insert_tlb
> >
> > Kernel 2.6.26.2:
> >
> > 537        /* Enforce uncacheable pages.
> > 538         * This should ONLY be use for MMIO on PA 2.0 machines.
> > 539         * Memory/DMA is cache coherent on all PA2.0 machines we support
> > 540         * (that means T-class is NOT supported) and the memory controllers
> > 541         * on most of those machines only handles cache transactions.
> > 542         */
> > 543        extrd,u,*=      \pte,_PAGE_NO_CACHE_BIT+32,1,%r0
> > 544        depi            1,12,1,\prot
> >
> >
> 
> The "*=" in line 543 will determine if "depi" instruction (line 544)
> gets executed or not. You'll need the "PA-RISC 2.0 Architecture":
>     http://www.parisc-linux.org/documentation/index.html
>     http://ftp.parisc-linux.org/docs/arch/parisc2.0.pdf
> 
> And read page 7-47, 7-48, and Table D-14.
> 
> 
> > The DEPI instruction on a 64-bit machine sets bit 44=32+12,
> > whereas we use the value as the argument to IDTLBT, which expects bit 12  
> > to be used instead.
> >
> > Does it mean that the U-bit is never set and the  
> > authorization id gets corrupted???
> 
> U-bit will get set only if _PAGE_NO_CACHE_BIT+32 is also set.
> 
> The bit enumeration is *reverse* with MSb being 0 for all ASM instructions
> and all references in the PA2.0 Arch manual.
> 
> Because this is a 64 bit build, "+32" is needed to refer to the lower half
> of the double word (word == 32 bits).
> 
> > Is it a bug or my misunderstanding of the code???
> 
> It looks correct to me. "12" here always seems to refer to the U-bit as
> defined in the PA2.0 Arch manual.

To me, it looks like the instruction should be a depdi.  See the preceding
deposit of PAGE_USER.  According to the IDTLBT description, the U bit is
bit 12 in r2, not bit 44.

rp3440 boots with the depdi change.  Building gcc...

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

Re: Wierd code in Entry.S

[Artem Alimarine]

[-- Attachment #1: Type: text/plain, Size: 2415 bytes --]

Hi Grant,

My point is that IDTLBT (see page 7-65 in the PA-RISC 2.0w spec) expects 
the U-bit in 12th bit of the 64-bit DWORD, thus, in the upper part 
instead of the lower part (see the code at dtlb_miss_20w). As you said, 
the bit is set in the lower part. I still suspect that the wrong bit is 
set on a 64-bit machine.

The confusion comes from the fact that the The PA1.1 instruction IDTLBP 
expects  U-bit in the bit 12 of the WORD, whereas PA2.0 IDTLBT expects 
it in bit 12 of the DWORD.

Best regards,
Artem

Grant Grundler wrote:
> On Thu, Jul 09, 2009 at 03:11:19PM +0200, Artem Alimarine wrote:
>   
>> Hi guys,
>>
>> I am new to PARISC and to this forum. I have a small question. There is  
>> an instruction in entry.S that I do not understand. It is in the the  
>> macro make_insert_tlb
>>
>> Kernel 2.6.26.2:
>>
>> 537        /* Enforce uncacheable pages.
>> 538         * This should ONLY be use for MMIO on PA 2.0 machines.
>> 539         * Memory/DMA is cache coherent on all PA2.0 machines we support
>> 540         * (that means T-class is NOT supported) and the memory controllers
>> 541         * on most of those machines only handles cache transactions.
>> 542         */
>> 543        extrd,u,*=      \pte,_PAGE_NO_CACHE_BIT+32,1,%r0
>> 544        depi            1,12,1,\prot
>>
>>
>>     
>
> The "*=" in line 543 will determine if "depi" instruction (line 544)
> gets executed or not. You'll need the "PA-RISC 2.0 Architecture":
>     http://www.parisc-linux.org/documentation/index.html
>     http://ftp.parisc-linux.org/docs/arch/parisc2.0.pdf
>
> And read page 7-47, 7-48, and Table D-14.
>
>
>   
>> The DEPI instruction on a 64-bit machine sets bit 44=32+12,
>> whereas we use the value as the argument to IDTLBT, which expects bit 12  
>> to be used instead.
>>
>> Does it mean that the U-bit is never set and the  
>> authorization id gets corrupted???
>>     
>
> U-bit will get set only if _PAGE_NO_CACHE_BIT+32 is also set.
>
> The bit enumeration is *reverse* with MSb being 0 for all ASM instructions
> and all references in the PA2.0 Arch manual.
>
> Because this is a 64 bit build, "+32" is needed to refer to the lower half
> of the double word (word == 32 bits).
>
>   
>> Is it a bug or my misunderstanding of the code???
>>     
>
> It looks correct to me. "12" here always seems to refer to the U-bit as
> defined in the PA2.0 Arch manual.
>
> hth,
> grant
>
>   


[-- Attachment #2: artem_alimarine.vcf --]
[-- Type: text/x-vcard, Size: 283 bytes --]

begin:vcard
fn:dr. Artem Alimarine
n:Alimarine;Artem
org:STROMASYS SA
adr:;;De Zaale 11;Eindhoven;;5612AJ;The Netherlands
email;internet:artem.alimarine@stromasys.com
title:Software Architect
tel;work:+31-40-2390863
tel;fax:+31-40-2390800
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: Wierd code in Entry.S

[Grant Grundler]

On Thu, Jul 09, 2009 at 08:15:04PM -0400, John David Anglin wrote:
...
> > > The DEPI instruction on a 64-bit machine sets bit 44=32+12,
> > > whereas we use the value as the argument to IDTLBT, which expects bit 12  
> > > to be used instead.
> > >
> > > Does it mean that the U-bit is never set and the  
> > > authorization id gets corrupted???
> > 
> > U-bit will get set only if _PAGE_NO_CACHE_BIT+32 is also set.
...
> To me, it looks like the instruction should be a depdi.  See the preceding
> deposit of PAGE_USER.  According to the IDTLBT description, the U bit is
> bit 12 in r2, not bit 44.

Yes - I see it now too. Excellent catch.

But I'm still wondering what the effect of this bug will be.
The first order (not setting U-bit) should only affect ZX1 (pa8800/pa8900)
machines. Those have uncacheable IO space between 2GB-4GB physical address.
My guess is the machines should HPMC since the CPU would attempt to access
those ranges as a cacheline read/write instead of sub-cacheline transactions.

It's not clear from the arch manual what happens if bit 44 is set in R2.
I didn't look far enough to see where the auth ID gets corrupted.

thanks,
grant

> 
> rp3440 boots with the depdi change.  Building gcc...
> 
> Dave
> -- 
> J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
> National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: Wierd code in Entry.S

[James Bottomley]

On Thu, 2009-07-09 at 20:15 -0400, John David Anglin wrote:
> > On Thu, Jul 09, 2009 at 03:11:19PM +0200, Artem Alimarine wrote:
> > > Hi guys,
> > >
> > > I am new to PARISC and to this forum. I have a small question. There is  
> > > an instruction in entry.S that I do not understand. It is in the the  
> > > macro make_insert_tlb
> > >
> > > Kernel 2.6.26.2:
> > >
> > > 537        /* Enforce uncacheable pages.
> > > 538         * This should ONLY be use for MMIO on PA 2.0 machines.
> > > 539         * Memory/DMA is cache coherent on all PA2.0 machines we support
> > > 540         * (that means T-class is NOT supported) and the memory controllers
> > > 541         * on most of those machines only handles cache transactions.
> > > 542         */
> > > 543        extrd,u,*=      \pte,_PAGE_NO_CACHE_BIT+32,1,%r0
> > > 544        depi            1,12,1,\prot
> > >
> > >
> > 
> > The "*=" in line 543 will determine if "depi" instruction (line 544)
> > gets executed or not. You'll need the "PA-RISC 2.0 Architecture":
> >     http://www.parisc-linux.org/documentation/index.html
> >     http://ftp.parisc-linux.org/docs/arch/parisc2.0.pdf
> > 
> > And read page 7-47, 7-48, and Table D-14.
> > 
> > 
> > > The DEPI instruction on a 64-bit machine sets bit 44=32+12,
> > > whereas we use the value as the argument to IDTLBT, which expects bit 12  
> > > to be used instead.
> > >
> > > Does it mean that the U-bit is never set and the  
> > > authorization id gets corrupted???
> > 
> > U-bit will get set only if _PAGE_NO_CACHE_BIT+32 is also set.
> > 
> > The bit enumeration is *reverse* with MSb being 0 for all ASM instructions
> > and all references in the PA2.0 Arch manual.
> > 
> > Because this is a 64 bit build, "+32" is needed to refer to the lower half
> > of the double word (word == 32 bits).
> > 
> > > Is it a bug or my misunderstanding of the code???
> > 
> > It looks correct to me. "12" here always seems to refer to the U-bit as
> > defined in the PA2.0 Arch manual.
> 
> To me, it looks like the instruction should be a depdi.  See the preceding
> deposit of PAGE_USER.  According to the IDTLBT description, the U bit is
> bit 12 in r2, not bit 44.
> 
> rp3440 boots with the depdi change.  Building gcc...

Yes, looks like we've got a set of cockups in this file.  Apparently gas
is evaluating depi as the macro DEPI, which seems to be correct for
every other case in the file I've looked at except this one.

I suppose the whole file needs fixing ... preferably with a macro whose
name isn't an instruction mnemonic.

James

Re: Wierd code in Entry.S

[John David Anglin]

> But I'm still wondering what the effect of this bug will be.
> The first order (not setting U-bit) should only affect ZX1 (pa8800/pa8900)
> machines. Those have uncacheable IO space between 2GB-4GB physical address.
> My guess is the machines should HPMC since the CPU would attempt to access
> those ranges as a cacheline read/write instead of sub-cacheline transactions.

I think the depdi change may fix the random memory corruption that
I have been complaining about.  My rp3440 has got through a full build
of GCC with 2.6.30.1.  Previous two attempts failed with segmentation
faults in the dynamic loader, one of which I reported on.  It will
take a few more builds to be sure.  Your comment would explain why I
don't see this on c3750.  Could this affect PA8700?

I installed the change on my rp3440, c3750 and gsyprf11 last night.
At some point, I need to test the change with an SMP build on the rp3440.

There is one other issue that I see on the rp3440 which I don't see
on the c3750 or gsyprf11.  I get occassional testsuite timeouts during
compilation on compilations that shouldn't timeout.  I had always
thought this to be a tcl/expect issue, but now I think this is likely
a kernel issue.  I need to change the timeout value in the testsuite
to something big so I can see what's happening.

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

Re: Wierd code in Entry.S

[Artem Alimarine]

[-- Attachment #1: Type: text/plain, Size: 1143 bytes --]

Hi Grant,

Bit 44 falls into the access_id area. So, access id get corrupted. As 
far as I understand we should get interrupt 18 (access check trap) on 
such TLB  entries. However we do not. The bug can go unnoticed when the 
access id is smaller than 31 bits. The PA2.0 manual says in the IDTLBT 
description: If smaller than 31-bit access IDs are implemented, only the 
appropriate number of the
rightmost bits of GR[r]{32..62} are stored in the TLB. Obviously, bit 44 
is not among the stored bits.

Actually, I have no idea on how many bits are used by the hardware. 
Myself I have rp2470.

Best regards,
Artem

> But I'm still wondering what the effect of this bug will be.
> The first order (not setting U-bit) should only affect ZX1 (pa8800/pa8900)
> machines. Those have uncacheable IO space between 2GB-4GB physical address.
> My guess is the machines should HPMC since the CPU would attempt to access
> those ranges as a cacheline read/write instead of sub-cacheline transactions.
>
> It's not clear from the arch manual what happens if bit 44 is set in R2.
> I didn't look far enough to see where the auth ID gets corrupted.
>
>   

[-- Attachment #2: artem_alimarine.vcf --]
[-- Type: text/x-vcard, Size: 283 bytes --]

begin:vcard
fn:dr. Artem Alimarine
n:Alimarine;Artem
org:STROMASYS SA
adr:;;De Zaale 11;Eindhoven;;5612AJ;The Netherlands
email;internet:artem.alimarine@stromasys.com
title:Software Architect
tel;work:+31-40-2390863
tel;fax:+31-40-2390800
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: Wierd code in Entry.S

[James Bottomley]

On Fri, 2009-07-10 at 11:55 -0400, John David Anglin wrote:
> > But I'm still wondering what the effect of this bug will be.
> > The first order (not setting U-bit) should only affect ZX1 (pa8800/pa8900)
> > machines. Those have uncacheable IO space between 2GB-4GB physical address.
> > My guess is the machines should HPMC since the CPU would attempt to access
> > those ranges as a cacheline read/write instead of sub-cacheline transactions.
> 
> I think the depdi change may fix the random memory corruption that
> I have been complaining about.  My rp3440 has got through a full build
> of GCC with 2.6.30.1.  Previous two attempts failed with segmentation
> faults in the dynamic loader, one of which I reported on.  It will
> take a few more builds to be sure.  Your comment would explain why I
> don't see this on c3750.  Could this affect PA8700?

In theory it would affect every box running a 64 bit kernel.  We
actually set PAGE_NO_CACHE on ioremaps(), so it's spreading out from the
PCI device space.

> I installed the change on my rp3440, c3750 and gsyprf11 last night.
> At some point, I need to test the change with an SMP build on the rp3440.

rp3440 is a pa88/8900 system?  We have ion in the test ring at
cupertino, it's a 4 CPU pa8900 ... I've been keeping it up to date with
debian testing.

> There is one other issue that I see on the rp3440 which I don't see
> on the c3750 or gsyprf11.  I get occassional testsuite timeouts during
> compilation on compilations that shouldn't timeout.  I had always
> thought this to be a tcl/expect issue, but now I think this is likely
> a kernel issue.  I need to change the timeout value in the testsuite
> to something big so I can see what's happening.

It sounds a bit like another caching issue ... perhaps a cached value
for a spinlock or futex making the thread thing the lock is held against
it when it isn't?

James

Re: Wierd code in Entry.S

[Artem Alimarine]

[-- Attachment #1: Type: text/plain, Size: 575 bytes --]

Hi,
> Yes, looks like we've got a set of cockups in this file.  Apparently gas
> is evaluating depi as the macro DEPI, which seems to be correct for
> every other case in the file I've looked at except this one
What I saw is that in the end DEPDI opcode with bit 44 is generated for 
than instruction.

The macro make_insert_tlb is used with PA2.0 and thus with IDTLBT and 
make_insert_tlb_11 with PA1.1 and thus with IDTLBP. It would probably be 
enough to change DEPI into DEPDI in the 2.0 variant. I do not know 
whether there are more similar cases.

Best regards,
Artem

[-- Attachment #2: artem_alimarine.vcf --]
[-- Type: text/x-vcard, Size: 283 bytes --]

begin:vcard
fn:dr. Artem Alimarine
n:Alimarine;Artem
org:STROMASYS SA
adr:;;De Zaale 11;Eindhoven;;5612AJ;The Netherlands
email;internet:artem.alimarine@stromasys.com
title:Software Architect
tel;work:+31-40-2390863
tel;fax:+31-40-2390800
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: Wierd code in Entry.S

[John David Anglin]

> > I installed the change on my rp3440, c3750 and gsyprf11 last night.
> > At some point, I need to test the change with an SMP build on the rp3440.
> 
> rp3440 is a pa88/8900 system?

Yes, it's a four cpu pa8800.  It was ior.

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

Re: Wierd code in Entry.S

[John David Anglin]

> Yes, looks like we've got a set of cockups in this file.  Apparently gas
> is evaluating depi as the macro DEPI, which seems to be correct for
> every other case in the file I've looked at except this one.

Yikes!  Macro and instruction mneumonics are case independent.  The
same issue applies to the EXTR and DEP macros.  It appears these macros
were intended to provide consistent PA 1.x and PA 2.0 mneumonics.
However, depwi isn't technically a valid PA 1.x instruction although
gas may not care.  This could present a problem for PA 1.x builds if
gas treats .level strictly.

The macro names really should be changed.  Looking at the code, I didn't
realize that depi was a macro...

> I suppose the whole file needs fixing ... preferably with a macro whose
> name isn't an instruction mnemonic.

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

Re: Wierd code in Entry.S

[John David Anglin]

> The macro names really should be changed.  Looking at the code, I didn't
> realize that depi was a macro...

It looks to me like these macros can be deleted.  For example, the PA 1.x
depi mneumonic is equivalent to the PA 2.0 depwi mneumonic.  See Table
J-2 in PA 2.0 arch.  There's no need for the 64-bit versions.  The PA 1.x
mneumonics are still valid in PA 2.0 assembler.  However, I would suggest
that the PA 2.0 mneumonics be used in PA 2.0 specific code.

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

Re: Wierd code in Entry.S

[John David Anglin]

> It looks to me like these macros can be deleted.  For example, the PA 1.x
> depi mneumonic is equivalent to the PA 2.0 depwi mneumonic.  See Table
> J-2 in PA 2.0 arch.  There's no need for the 64-bit versions.  The PA 1.x
> mneumonics are still valid in PA 2.0 assembler.  However, I would suggest
> that the PA 2.0 mneumonics be used in PA 2.0 specific code.

The following still boots (64-bit UP kernel).

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

diff --git a/arch/parisc/kernel/entry.S b/arch/parisc/kernel/entry.S
index ae3e70c..896791a 100644
--- a/arch/parisc/kernel/entry.S
+++ b/arch/parisc/kernel/entry.S
@@ -364,32 +364,6 @@
 	.align		32
 	.endm
 
-	/* The following are simple 32 vs 64 bit instruction
-	 * abstractions for the macros */
-	.macro		EXTR	reg1,start,length,reg2
-#ifdef CONFIG_64BIT
-	extrd,u		\reg1,32+(\start),\length,\reg2
-#else
-	extrw,u		\reg1,\start,\length,\reg2
-#endif
-	.endm
-
-	.macro		DEP	reg1,start,length,reg2
-#ifdef CONFIG_64BIT
-	depd		\reg1,32+(\start),\length,\reg2
-#else
-	depw		\reg1,\start,\length,\reg2
-#endif
-	.endm
-
-	.macro		DEPI	val,start,length,reg
-#ifdef CONFIG_64BIT
-	depdi		\val,32+(\start),\length,\reg
-#else
-	depwi		\val,\start,\length,\reg
-#endif
-	.endm
-
 	/* In LP64, the space contains part of the upper 32 bits of the
 	 * fault.  We have to extract this and place it in the va,
 	 * zeroing the corresponding bits in the space register */
@@ -442,19 +416,19 @@
 	 */
 	.macro		L2_ptep	pmd,pte,index,va,fault
 #if PT_NLEVELS == 3
-	EXTR		\va,31-ASM_PMD_SHIFT,ASM_BITS_PER_PMD,\index
+	extru		\va,31-ASM_PMD_SHIFT,ASM_BITS_PER_PMD,\index
 #else
-	EXTR		\va,31-ASM_PGDIR_SHIFT,ASM_BITS_PER_PGD,\index
+	extru		\va,31-ASM_PGDIR_SHIFT,ASM_BITS_PER_PGD,\index
 #endif
-	DEP             %r0,31,PAGE_SHIFT,\pmd  /* clear offset */
+	dep             %r0,31,PAGE_SHIFT,\pmd  /* clear offset */
 	copy		%r0,\pte
 	ldw,s		\index(\pmd),\pmd
 	bb,>=,n		\pmd,_PxD_PRESENT_BIT,\fault
-	DEP		%r0,31,PxD_FLAG_SHIFT,\pmd /* clear flags */
+	dep		%r0,31,PxD_FLAG_SHIFT,\pmd /* clear flags */
 	copy		\pmd,%r9
 	SHLREG		%r9,PxD_VALUE_SHIFT,\pmd
-	EXTR		\va,31-PAGE_SHIFT,ASM_BITS_PER_PTE,\index
-	DEP		%r0,31,PAGE_SHIFT,\pmd  /* clear offset */
+	extru		\va,31-PAGE_SHIFT,ASM_BITS_PER_PTE,\index
+	dep		%r0,31,PAGE_SHIFT,\pmd  /* clear offset */
 	shladd		\index,BITS_PER_PTE_ENTRY,\pmd,\pmd
 	LDREG		%r0(\pmd),\pte		/* pmd is now pte */
 	bb,>=,n		\pte,_PAGE_PRESENT_BIT,\fault
@@ -553,7 +527,7 @@
 	 * on most of those machines only handles cache transactions.
 	 */
 	extrd,u,*=	\pte,_PAGE_NO_CACHE_BIT+32,1,%r0
-	depi		1,12,1,\prot
+	depdi		1,12,1,\prot
 
 	/* Drop prot bits and convert to page addr for iitlbt and idtlbt */
 	convert_for_tlb_insert20 \pte
@@ -605,7 +579,7 @@
 	depdi		0,31,32,\tmp
 #endif
 	copy		\va,\tmp1
-	DEPI		0,31,23,\tmp1
+	depi		0,31,23,\tmp1
 	cmpb,COND(<>),n	\tmp,\tmp1,\fault
 	ldi		(_PAGE_DIRTY|_PAGE_WRITE|_PAGE_READ),\prot
 	depd,z		\prot,8,7,\prot

Re: Wierd code in Entry.S

[John David Anglin]

> > It looks to me like these macros can be deleted.  For example, the PA 1.x
> > depi mneumonic is equivalent to the PA 2.0 depwi mneumonic.  See Table
> > J-2 in PA 2.0 arch.  There's no need for the 64-bit versions.  The PA 1.x
> > mneumonics are still valid in PA 2.0 assembler.  However, I would suggest
> > that the PA 2.0 mneumonics be used in PA 2.0 specific code.

Sorry, the macros can't be removed.  The PA 1.x mneumonics result in undefined
behavior in the most sign-significant 32 bits.  The should just be renamed.

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

Re: Wierd code in Entry.S

[Kyle McMartin]

On Sun, Jul 12, 2009 at 03:40:34PM -0400, John David Anglin wrote:
> > > It looks to me like these macros can be deleted.  For example, the PA 1.x
> > > depi mneumonic is equivalent to the PA 2.0 depwi mneumonic.  See Table
> > > J-2 in PA 2.0 arch.  There's no need for the 64-bit versions.  The PA 1.x
> > > mneumonics are still valid in PA 2.0 assembler.  However, I would suggest
> > > that the PA 2.0 mneumonics be used in PA 2.0 specific code.
> 
> Sorry, the macros can't be removed.  The PA 1.x mneumonics result in undefined
> behavior in the most sign-significant 32 bits.  The should just be renamed.
> 

Can someone distill all these mails into a patch? It will directly apply
to upstream and all the current stable releases, so a single patch
should suffice.

I'd do it myself, but I'm too busy writing a paper this week. :/

regards, Kyle

Re: Wierd code in Entry.S

[John David Anglin]

> Can someone distill all these mails into a patch? It will directly apply
> to upstream and all the current stable releases, so a single patch
> should suffice.
> 
> I'd do it myself, but I'm too busy writing a paper this week. :/

The most important change is attached.  Bit 12 needs to be set in the
protection register, not bit 44.  The change has had three days testing
on my rp3440 (PA8800) without any unusual segmentation faults.  Also,
tested on a couple of PA8700 machines.  This fixes the problem noticed
by Artem Alimarine.

At the moment, I don't have a change for the macros.  The DEP and DEPI
macros conflict with PA 1.x mneumonics.  So, they should be renamed.
I also think the 32-bit instructions in the macros should use PA 1.x
rather than 2.0 mneumonics.  The 1.x mneumonics should be ok for both
PA 1.x and 2.0.  It is possible that building for a PA 1.x machine might
not work with the current macros.  Maybe James has something for the
macros.

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

diff --git a/arch/parisc/kernel/entry.S b/arch/parisc/kernel/entry.S
index ae3e70c..e552e54 100644
--- a/arch/parisc/kernel/entry.S
+++ b/arch/parisc/kernel/entry.S
@@ -553,7 +553,7 @@
 	 * on most of those machines only handles cache transactions.
 	 */
 	extrd,u,*=	\pte,_PAGE_NO_CACHE_BIT+32,1,%r0
-	depi		1,12,1,\prot
+	depdi		1,12,1,\prot
 
 	/* Drop prot bits and convert to page addr for iitlbt and idtlbt */
 	convert_for_tlb_insert20 \pte

Re: Wierd code in Entry.S

[Kyle McMartin]

On Sun, Jul 12, 2009 at 09:44:37PM -0400, John David Anglin wrote:
> > Can someone distill all these mails into a patch? It will directly apply
> > to upstream and all the current stable releases, so a single patch
> > should suffice.
> > 
> > I'd do it myself, but I'm too busy writing a paper this week. :/
> 
> The most important change is attached.  Bit 12 needs to be set in the
> protection register, not bit 44.  The change has had three days testing
> on my rp3440 (PA8800) without any unusual segmentation faults.  Also,
> tested on a couple of PA8700 machines.  This fixes the problem noticed
> by Artem Alimarine.
> 
> At the moment, I don't have a change for the macros.  The DEP and DEPI
> macros conflict with PA 1.x mneumonics.  So, they should be renamed.
> I also think the 32-bit instructions in the macros should use PA 1.x
> rather than 2.0 mneumonics.  The 1.x mneumonics should be ok for both
> PA 1.x and 2.0.  It is possible that building for a PA 1.x machine might
> not work with the current macros.  Maybe James has something for the
> macros.
> 

Great. May I append a Signed-off-by: John David Anglin
<dave@hiauly1.hia.nrc.ca>?

cheers, Kyle

Re: Wierd code in Entry.S

[John David Anglin]

> Great. May I append a Signed-off-by: John David Anglin
> <dave@hiauly1.hia.nrc.ca>?

Yes, but change the email to <dave.anglin@nrc-cnrc.gc.ca>.

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

Re: Wierd code in Entry.S

[Grant Grundler]

On Fri, Jul 10, 2009 at 11:18:21AM -0500, James Bottomley wrote:
> On Fri, 2009-07-10 at 11:55 -0400, John David Anglin wrote:
...
> > Your comment would explain why I
> > don't see this on c3750.  Could this affect PA8700?
> 
> In theory it would affect every box running a 64 bit kernel.

Yes, regarding corrupting bit-44 but not U-bit.

IIRC, only pa880 and pa8900 pay attention to the U-bit.
I thought all the previous CPUs ignored U-bit.

> We
> actually set PAGE_NO_CACHE on ioremaps(), so it's spreading out from the
> PCI device space.

Yes - but only ZX1 chipset (e.g. rp3440 and C8000) uses IO space that
is outside of F-space. F-space is hardwired to be uncachable by the CPU.

hth,
grant

Re: Wierd code in Entry.S

[Grant Grundler]

On Fri, Jul 10, 2009 at 06:11:35PM +0200, Artem Alimarine wrote:
> Hi Grant,
>
> Bit 44 falls into the access_id area. So, access id get corrupted. As  
> far as I understand we should get interrupt 18 (access check trap) on  
> such TLB  entries. However we do not. The bug can go unnoticed when the  
> access id is smaller than 31 bits. The PA2.0 manual says in the IDTLBT  
> description: If smaller than 31-bit access IDs are implemented, only the  
> appropriate number of the
> rightmost bits of GR[r]{32..62} are stored in the TLB. Obviously, bit 44  
> is not among the stored bits.

Ok - thanks for looking that up. CPU specific ERS would contain the
details that describe access ID implementations.

> Actually, I have no idea on how many bits are used by the hardware.  
> Myself I have rp2470.

Yeah, that's Astro/Elroy and not ZX1. So should not be affected
by the U-bit being mis-set.

thanks,
grant

Re: Wierd code in Entry.S

[James Bottomley]

On Wed, 2009-07-15 at 00:38 -0600, Grant Grundler wrote:
> On Fri, Jul 10, 2009 at 11:18:21AM -0500, James Bottomley wrote:
> > On Fri, 2009-07-10 at 11:55 -0400, John David Anglin wrote:
> ...
> > > Your comment would explain why I
> > > don't see this on c3750.  Could this affect PA8700?
> > 
> > In theory it would affect every box running a 64 bit kernel.
> 
> Yes, regarding corrupting bit-44 but not U-bit.
> 
> IIRC, only pa880 and pa8900 pay attention to the U-bit.
> I thought all the previous CPUs ignored U-bit.

Um, no, almost all pa chips use it ... it's the way we make coherent
memory on the very old 710 and similar systems: use an ordinary kmalloc
and turn off caching in the map.  The 715 class systems with the oldest
pa chips don't respect the U bit ... these are the ones we have to pull
the driver cache flushing tricks on to get them working.

We also use the U bit for ioremaps.

> > We
> > actually set PAGE_NO_CACHE on ioremaps(), so it's spreading out from the
> > PCI device space.
> 
> Yes - but only ZX1 chipset (e.g. rp3440 and C8000) uses IO space that
> is outside of F-space. F-space is hardwired to be uncachable by the CPU.

OK, so we don't really use the concept of F-Space in the linux kernel
virtual memory map on parisc linux.  What we do is map the whole of
memory into the virtual address space, but none of the I/O space.  the
readX/writeX macros actually go via absolute accesses.  Because the
kernel virtual addresses are offset mapped from the absolute addresses,
we do get a hole in the virtual map corresponding to F-Space (simply
because there's no accessible memory there) but we're actually able to
fill that hole later with virtual mappings if we choose.  The net result
is that I/O devices aren't mapped into the kernel at all until we call
ioremap.  If a device uses readX/writeX only, it never actually gets an
appearance in our virtual space because we simply use absolute accesses
to get the data to and from the device.  If it really needs a remapped
area, we use ioremap, then it does appear in our virtual map, and we set
the U bit on the mapping.

James

Re: Wierd code in Entry.S

[Matthew Wilcox]

On Wed, Jul 15, 2009 at 08:41:30AM -0400, James Bottomley wrote:
> On Wed, 2009-07-15 at 00:38 -0600, Grant Grundler wrote:
> > IIRC, only pa880 and pa8900 pay attention to the U-bit.
> > I thought all the previous CPUs ignored U-bit.
> 
> Um, no, almost all pa chips use it ... it's the way we make coherent
> memory on the very old 710 and similar systems: use an ordinary kmalloc
> and turn off caching in the map.  The 715 class systems with the oldest
> pa chips don't respect the U bit ... these are the ones we have to pull
> the driver cache flushing tricks on to get them working.

Even the 715-class systems respect the U bit.  The problem is that
the memory controller in use on those platforms cannot cope with
sub-cacheline-size accesses to memory.  So if we were to set the U bit
on memory, we would crash.

> > Yes - but only ZX1 chipset (e.g. rp3440 and C8000) uses IO space that
> > is outside of F-space. F-space is hardwired to be uncachable by the CPU.
> 
> OK, so we don't really use the concept of F-Space in the linux kernel
> virtual memory map on parisc linux.  What we do is map the whole of
> memory into the virtual address space, but none of the I/O space.  the
> readX/writeX macros actually go via absolute accesses.  Because the

Uhm, no.  The gsc_read/writeX macros go via absolute accesses.
__raw_read/writeX go via ioremapped addresses, as do the more common
readX/writeX.

> kernel virtual addresses are offset mapped from the absolute addresses,
> we do get a hole in the virtual map corresponding to F-Space (simply
> because there's no accessible memory there) but we're actually able to
> fill that hole later with virtual mappings if we choose.  The net result
> is that I/O devices aren't mapped into the kernel at all until we call
> ioremap.  If a device uses readX/writeX only, it never actually gets an
> appearance in our virtual space because we simply use absolute accesses
> to get the data to and from the device.  If it really needs a remapped
> area, we use ioremap, then it does appear in our virtual map, and we set
> the U bit on the mapping.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: Wierd code in Entry.S

[Grant Grundler]

On Wed, Jul 15, 2009 at 08:41:30AM -0400, James Bottomley wrote:
> On Wed, 2009-07-15 at 00:38 -0600, Grant Grundler wrote:
> > On Fri, Jul 10, 2009 at 11:18:21AM -0500, James Bottomley wrote:
> > > On Fri, 2009-07-10 at 11:55 -0400, John David Anglin wrote:
> > ...
> > > > Your comment would explain why I
> > > > don't see this on c3750.  Could this affect PA8700?
> > > 
> > > In theory it would affect every box running a 64 bit kernel.
> > 
> > Yes, regarding corrupting bit-44 but not U-bit.
> > 
> > IIRC, only pa880 and pa8900 pay attention to the U-bit.
> > I thought all the previous CPUs ignored U-bit.
> 
> Um, no, almost all pa chips use it ... it's the way we make coherent
> memory on the very old 710 and similar systems: use an ordinary kmalloc
> and turn off caching in the map.  The 715 class systems with the oldest
> pa chips don't respect the U bit ... these are the ones we have to pull
> the driver cache flushing tricks on to get them working.

Sorry - I was thinking only in the context of F-space but didn't make
that distinction clear.  I wasn't thinking about about the use of U-bit
for DMA coherence. 

thanks,
grant

> We also use the U bit for ioremaps.
> 
> > > We
> > > actually set PAGE_NO_CACHE on ioremaps(), so it's spreading out from the
> > > PCI device space.
> > 
> > Yes - but only ZX1 chipset (e.g. rp3440 and C8000) uses IO space that
> > is outside of F-space. F-space is hardwired to be uncachable by the CPU.
> 
> OK, so we don't really use the concept of F-Space in the linux kernel
> virtual memory map on parisc linux.  What we do is map the whole of
> memory into the virtual address space, but none of the I/O space.  the
> readX/writeX macros actually go via absolute accesses.  Because the
> kernel virtual addresses are offset mapped from the absolute addresses,
> we do get a hole in the virtual map corresponding to F-Space (simply
> because there's no accessible memory there) but we're actually able to
> fill that hole later with virtual mappings if we choose.  The net result
> is that I/O devices aren't mapped into the kernel at all until we call
> ioremap.  If a device uses readX/writeX only, it never actually gets an
> appearance in our virtual space because we simply use absolute accesses
> to get the data to and from the device.  If it really needs a remapped
> area, we use ioremap, then it does appear in our virtual map, and we set
> the U bit on the mapping.
> 
> James
>