Le 22/12/2018 à 11:09, Christian Lamparter a écrit :
This patch fixes a recent regression in ocm:
ocm.c: In function ‘ocm_init_node’:
ocm.c:182:18: error: invalid operands to binary |
ocm.c:197:17: error: invalid operands to binary |
Fixes: 56f3c1413f5c ("powerpc/mm: properly set PAGE_KERNEL flags in ioremap()")
Signed-off-by: Christian Lamparter <redacted>
What's the problem here ? Is PAGE_KERNEL_NCG undefined ? If that's the
case, wouldn't it be better for fix that by including asm/pgtable.h ?
Christophe
From: Christian Lamparter <hidden> Date: 2018-12-22 11:29:56
On Saturday, December 22, 2018 11:59:04 AM CET christophe leroy wrote:
Le 22/12/2018 à 11:09, Christian Lamparter a écrit :
quoted
This patch fixes a recent regression in ocm:
ocm.c: In function ‘ocm_init_node’:
ocm.c:182:18: error: invalid operands to binary |
ocm.c:197:17: error: invalid operands to binary |
Fixes: 56f3c1413f5c ("powerpc/mm: properly set PAGE_KERNEL flags in ioremap()")
Signed-off-by: Christian Lamparter <redacted>
What's the problem here ? Is PAGE_KERNEL_NCG undefined ? If that's the
case, wouldn't it be better for fix that by including asm/pgtable.h ?
PAGE_KERNEL[_NCG] type is a struct of pgprot_t, whereas _PAGE_EXEC is
considered an int.
arch/powerpc/platforms/4xx/ocm.c: In function ‘ocm_init_node’:
arch/powerpc/platforms/4xx/ocm.c:182:18: error: invalid operands to binary | (have ‘int’ and ‘pgprot_t’ {aka ‘struct <anonymous>’})
_PAGE_EXEC | PAGE_KERNEL_NCG);
^
arch/powerpc/platforms/4xx/ocm.c:197:17: error: invalid operands to binary | (have ‘int’ and ‘pgprot_t’ {aka ‘struct <anonymous>’})
_PAGE_EXEC | PAGE_KERNEL);
^
I think pgprot_val(PAGE_KERNEL[_NCG]) could be an option too.
Christian
Le 22/12/2018 à 12:27, Christian Lamparter a écrit :
On Saturday, December 22, 2018 11:59:04 AM CET christophe leroy wrote:
quoted
Le 22/12/2018 à 11:09, Christian Lamparter a écrit :
quoted
This patch fixes a recent regression in ocm:
ocm.c: In function ‘ocm_init_node’:
ocm.c:182:18: error: invalid operands to binary |
ocm.c:197:17: error: invalid operands to binary |
Fixes: 56f3c1413f5c ("powerpc/mm: properly set PAGE_KERNEL flags in ioremap()")
Signed-off-by: Christian Lamparter <redacted>
What's the problem here ? Is PAGE_KERNEL_NCG undefined ? If that's the
case, wouldn't it be better for fix that by including asm/pgtable.h ?
PAGE_KERNEL[_NCG] type is a struct of pgprot_t, whereas _PAGE_EXEC is
considered an int.
Oops, I missed that.
Could you put the entire error message in the commit log ?
arch/powerpc/platforms/4xx/ocm.c: In function ‘ocm_init_node’:
arch/powerpc/platforms/4xx/ocm.c:182:18: error: invalid operands to binary | (have ‘int’ and ‘pgprot_t’ {aka ‘struct <anonymous>’})
_PAGE_EXEC | PAGE_KERNEL_NCG);
Usually, Guarded implies no exec (at least on 6xx and 8xx). Does the 4xx
accept guarded exec ?
^
arch/powerpc/platforms/4xx/ocm.c:197:17: error: invalid operands to binary | (have ‘int’ and ‘pgprot_t’ {aka ‘struct <anonymous>’})
_PAGE_EXEC | PAGE_KERNEL);
That's PAGE_KERNEL_X
^
I think pgprot_val(PAGE_KERNEL[_NCG]) could be an option too.
@@ -179,7 +179,8 @@ static void __init ocm_init_node(int count, struct device_node *node)/* ioremap the non-cached region */if(ocm->nc.memtotal){ocm->nc.virt=__ioremap(ocm->nc.phys,ocm->nc.memtotal,-_PAGE_EXEC|PAGE_KERNEL_NCG);+_PAGE_EXEC|_PAGE_BASE_NC|+_PAGE_KERNEL_RW|_PAGE_NO_CACHE|_PAGE_GUARDED);
Could be _PAGE_BASE_NC | _PAGE_KERNEL_RWX | _PAGE_NO_CACHE | _PAGE_GUARDED
Or pgprot_val(PAGE_KERNEL_NCG) | _PAGE_EXEC
Or pgprot_val(pgprot_noncached(PAGE_KERNEL_RWX)) (that's my preference)
From: Christian Lamparter <hidden> Date: 2018-12-22 14:35:19
On Saturday, December 22, 2018 2:08:02 PM CET christophe leroy wrote:
Le 22/12/2018 à 12:27, Christian Lamparter a écrit :
quoted
On Saturday, December 22, 2018 11:59:04 AM CET christophe leroy wrote:
quoted
Le 22/12/2018 à 11:09, Christian Lamparter a écrit :
quoted
This patch fixes a recent regression in ocm:
ocm.c: In function ‘ocm_init_node’:
ocm.c:182:18: error: invalid operands to binary |
ocm.c:197:17: error: invalid operands to binary |
Fixes: 56f3c1413f5c ("powerpc/mm: properly set PAGE_KERNEL flags in ioremap()")
Signed-off-by: Christian Lamparter <redacted>
What's the problem here ? Is PAGE_KERNEL_NCG undefined ? If that's the
case, wouldn't it be better for fix that by including asm/pgtable.h ?
PAGE_KERNEL[_NCG] type is a struct of pgprot_t, whereas _PAGE_EXEC is
considered an int.
Oops, I missed that.
Could you put the entire error message in the commit log ?
Will do in v2... which I'm going to sent out shortly. I mainly wanted to
make the most "trivial fix" since it probably needs to be backported to
4.20-stable at this late in the rc phase.
quoted
arch/powerpc/platforms/4xx/ocm.c: In function ‘ocm_init_node’:
arch/powerpc/platforms/4xx/ocm.c:182:18: error: invalid operands to binary | (have ‘int’ and ‘pgprot_t’ {aka ‘struct <anonymous>’})
_PAGE_EXEC | PAGE_KERNEL_NCG);
Usually, Guarded implies no exec (at least on 6xx and 8xx). Does the 4xx
accept guarded exec ?
Oh, I simply copied over the individual _PAGE_* flags that the PAGE_KERNEL_NCG
macro set. The OCM is usually a small (32 KiB) DMA descriptor storage, I don't
think anyone would want to execute code in there.
quoted
^
arch/powerpc/platforms/4xx/ocm.c:197:17: error: invalid operands to binary | (have ‘int’ and ‘pgprot_t’ {aka ‘struct <anonymous>’})
_PAGE_EXEC | PAGE_KERNEL);
That's PAGE_KERNEL_X
quoted
^
I think pgprot_val(PAGE_KERNEL[_NCG]) could be an option too.
Yes I may have a preference for that.
K, I liked having all _PAGE_*. But I'm fine either way.
see you for v2. :-D
@@ -179,7 +179,8 @@ static void __init ocm_init_node(int count, struct device_node *node)/* ioremap the non-cached region */if(ocm->nc.memtotal){ocm->nc.virt=__ioremap(ocm->nc.phys,ocm->nc.memtotal,-_PAGE_EXEC|PAGE_KERNEL_NCG);+_PAGE_EXEC|_PAGE_BASE_NC|+_PAGE_KERNEL_RW|_PAGE_NO_CACHE|_PAGE_GUARDED);
Could be _PAGE_BASE_NC | _PAGE_KERNEL_RWX | _PAGE_NO_CACHE | _PAGE_GUARDED
Or pgprot_val(PAGE_KERNEL_NCG) | _PAGE_EXEC
Or pgprot_val(pgprot_noncached(PAGE_KERNEL_RWX)) (that's my preference)
Le 22/12/2018 à 18:16, Segher Boessenkool a écrit :
On Sat, Dec 22, 2018 at 02:08:02PM +0100, christophe leroy wrote:
quoted
Usually, Guarded implies no exec (at least on 6xx and 8xx).
Huh? What do you mean here?
From the 885 Reference Manual:
Address translation: the EA is translated by using the MMU’s TLB
mechanism. Instructions are not fetched from no-execute or guarded
memory and data accesses are not executed speculatively to or from the
guarded memory.
6.1.3.4 Instruction TLB Error Exception (0x01300)
This type of exception occurs as a result of one of the following
conditions if MSR[IR] = 1:
- The EA cannot be translated.
- The fetch access violates memory protection
- The fetch access is to guarded memory
From e300core reference manual:
Translation Exception Conditions:
Exception condition: Instruction fetch from guarded memory
with MSR[IR] = 1 ==> ISI interrupt SRR1[3] = 1
Christophe
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
On Sat, Dec 22, 2018 at 08:37:28PM +0100, christophe leroy wrote:
Le 22/12/2018 à 18:16, Segher Boessenkool a écrit :
quoted
On Sat, Dec 22, 2018 at 02:08:02PM +0100, christophe leroy wrote:
quoted
Usually, Guarded implies no exec (at least on 6xx and 8xx).
Huh? What do you mean here?
From the 885 Reference Manual:
Address translation: the EA is translated by using the MMU’s TLB
mechanism. Instructions are not fetched from no-execute or guarded
memory and data accesses are not executed speculatively to or from the
guarded memory.
6.1.3.4 Instruction TLB Error Exception (0x01300)
This type of exception occurs as a result of one of the following
conditions if MSR[IR] = 1:
- The EA cannot be translated.
- The fetch access violates memory protection
- The fetch access is to guarded memory
From e300core reference manual:
Translation Exception Conditions:
Exception condition: Instruction fetch from guarded memory
with MSR[IR] = 1 ==> ISI interrupt SRR1[3] = 1
Right, but you said 6xx as well, i.e. pure PowerPC.
If for example IR=0 you cannot have N=1, but you do have G=1. There is
no case where G=1 implies N=1 afaik, or where fetch is prohibited some
other way (causes an ISI, say).
Segher
Le 23/12/2018 à 10:31, Segher Boessenkool a écrit :
On Sun, Dec 23, 2018 at 09:29:44AM +0100, Gabriel Paubert wrote:
quoted
• The fetch access is to guarded storage and MSR[IR] = 1.
That is architected, yes. You get an ISI if IR=1 and (N=1 or G=1).
If IR=0, mappings don't apply. So in 4xx/ocm, this mapping with
_PAGE_EXEC and _PAGE_GUARDED at the same time seems pointless.
A standard ioremap() should do it (ie without exec), or a non guarded
exec mapping if execution is expected for this area.
Christophe
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
From: Gabriel Paubert <hidden> Date: 2018-12-26 07:10:28
On Sat, Dec 22, 2018 at 05:04:51PM -0600, Segher Boessenkool wrote:
On Sat, Dec 22, 2018 at 08:37:28PM +0100, christophe leroy wrote:
quoted
Le 22/12/2018 à 18:16, Segher Boessenkool a écrit :
quoted
On Sat, Dec 22, 2018 at 02:08:02PM +0100, christophe leroy wrote:
quoted
Usually, Guarded implies no exec (at least on 6xx and 8xx).
Huh? What do you mean here?
From the 885 Reference Manual:
Address translation: the EA is translated by using the MMU’s TLB
mechanism. Instructions are not fetched from no-execute or guarded
memory and data accesses are not executed speculatively to or from the
guarded memory.
6.1.3.4 Instruction TLB Error Exception (0x01300)
This type of exception occurs as a result of one of the following
conditions if MSR[IR] = 1:
- The EA cannot be translated.
- The fetch access violates memory protection
- The fetch access is to guarded memory
From e300core reference manual:
Translation Exception Conditions:
Exception condition: Instruction fetch from guarded memory
with MSR[IR] = 1 ==> ISI interrupt SRR1[3] = 1
Right, but you said 6xx as well, i.e. pure PowerPC.
If for example IR=0 you cannot have N=1, but you do have G=1. There is
no case where G=1 implies N=1 afaik, or where fetch is prohibited some
other way (causes an ISI, say).
From the 750fx user's manual, similar wording can be found for other
processors (except 601), including the ones that use software TLB load
like 603/603e (the test for guarded memory is in the sample code for
the tlb miss handler):
"
An ISI exception occurs when no higher priority exception exists and an
attempt to fetch the next instruction fails. This exception is implemented
as it is defined by the PowerPC architecture (OEA), and is taken for the
following conditions:
• The effective address cannot be translated.
• The fetch access is to a no-execute segment (SR[N] = 1).
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
• The fetch access is to guarded storage and MSR[IR] = 1.
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
• The fetch access is to a segment for which SR[T] is set.
• The fetch access violates memory protection.
When an ISI exception is taken, instruction fetching resumes at offset
0x00400 from the physical base indicated by MSR[IP].
"
Gabriel