Re: [PATCH] powerpc/4xx/ocm: fix compiler error
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: > > Le 22/12/2018 à 18:16, Segher Boessenkool a écrit : > > >On Sat, Dec 22, 2018 at 02:08:02PM +0100, christophe leroy wrote: > > >> > > >>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
Re: [PATCH] powerpc/4xx/ocm: fix compiler error
Le 23/12/2018 à 10:31, Segher Boessenkool a écrit : On Sun, Dec 23, 2018 at 09:29:44AM +0100, Gabriel Paubert wrote: • 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
Re: [PATCH] powerpc/4xx/ocm: fix compiler error
On Sun, Dec 23, 2018 at 09:29:44AM +0100, Gabriel Paubert wrote: > • 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). Segher
Re: [PATCH] powerpc/4xx/ocm: fix compiler error
On Sat, Dec 22, 2018 at 08:37:28PM +0100, christophe leroy wrote: > Le 22/12/2018 à 18:16, Segher Boessenkool a écrit : > >On Sat, Dec 22, 2018 at 02:08:02PM +0100, christophe leroy wrote: > >> > >>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
Re: [PATCH] powerpc/4xx/ocm: fix compiler error
Le 22/12/2018 à 18:16, Segher Boessenkool a écrit : On Sat, Dec 22, 2018 at 02:08:02PM +0100, christophe leroy wrote: 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
Re: [PATCH] powerpc/4xx/ocm: fix compiler error
On Sat, Dec 22, 2018 at 02:08:02PM +0100, christophe leroy wrote: > > Usually, Guarded implies no exec (at least on 6xx and 8xx). Huh? What do you mean here? Segher
Re: [PATCH] powerpc/4xx/ocm: fix compiler error
On Saturday, December 22, 2018 2:08:02 PM CET christophe leroy wrote:
>
> Le 22/12/2018 à 12:27, Christian Lamparter a écrit :
> > On Saturday, December 22, 2018 11:59:04 AM CET christophe leroy wrote:
> >>
> >> 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
> >>
> >> 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.
> >
> > 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 ’})
> > _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.
>
> >^
> > arch/powerpc/platforms/4xx/ocm.c:197:17: error: invalid operands to binary
> > | (have ‘int’ and ‘pgprot_t’ {aka ‘struct ’})
> > _PAGE_EXEC | PAGE_KERNEL);
>
> That's PAGE_KERNEL_X
>
> >^
> >
> > 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
> >
> > Christian
> >>> ---
> >>>arch/powerpc/platforms/4xx/ocm.c | 6 --
> >>>1 file changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/platforms/4xx/ocm.c
> >>> b/arch/powerpc/platforms/4xx/ocm.c
> >>> index 561b09de69bf..04ff69ddac09 100644
> >>> --- a/arch/powerpc/platforms/4xx/ocm.c
> >>> +++ b/arch/powerpc/platforms/4xx/ocm.c
> >>> @@ -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)
>
> >>>
> >>> if (!ocm->nc.virt) {
> >>> printk(KERN_ERR
> >>> @@ -194,7 +195,8 @@ static void __init ocm_init_node(int count, struct
> >>> device_node *node)
> >>>
> >>> if (ocm->c.memtotal) {
> >>> ocm->c.virt = __ioremap(ocm->c.phys, ocm->c.memtotal,
> >>> - _PAGE_EXEC | PAGE_KERNEL);
> >>> + _PAGE_EXEC | _PAGE_BASE |
> >>> + _PAGE_KERNEL_RW);
>
> What about _PAGE_BASE | _PAGE_KERNEL_RWX instead ?
>
> Or pgprot_val(PAGE_KERNEL_RWX) (that's my preference)
I'll test if any of these make a difference and sent a separate patch.
Christian
Re: [PATCH] powerpc/4xx/ocm: fix compiler error
Le 22/12/2018 à 12:27, Christian Lamparter a écrit :
On Saturday, December 22, 2018 11:59:04 AM CET christophe leroy wrote:
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
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 ’})
_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 ’})
_PAGE_EXEC | PAGE_KERNEL);
That's PAGE_KERNEL_X
^
I think pgprot_val(PAGE_KERNEL[_NCG]) could be an option too.
Yes I may have a preference for that.
Christian
---
arch/powerpc/platforms/4xx/ocm.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
index 561b09de69bf..04ff69ddac09 100644
--- a/arch/powerpc/platforms/4xx/ocm.c
+++ b/arch/powerpc/platforms/4xx/ocm.c
@@ -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)
if (!ocm->nc.virt) {
printk(KERN_ERR
@@ -194,7 +195,8 @@ static void __init ocm_init_node(int count, struct
device_node *node)
if (ocm->c.memtotal) {
ocm->c.virt = __ioremap(ocm->c.phys, ocm->c.memtotal,
- _PAGE_EXEC | PAGE_KERNEL);
+ _PAGE_EXEC | _PAGE_BASE |
+ _PAGE_KERNEL_RW);
What about _PAGE_BASE | _PAGE_KERNEL_RWX instead ?
Or pgprot_val(PAGE_KERNEL_RWX) (that's my preference)
Christophe
if (!ocm->c.virt) {
printk(KERN_ERR
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel
antivirus Avast.
https://www.avast.com/antivirus
Re: [PATCH] powerpc/4xx/ocm: fix compiler error
On Saturday, December 22, 2018 11:59:04 AM CET christophe leroy wrote:
>
> 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
>
> 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 ’})
_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 ’})
_PAGE_EXEC | PAGE_KERNEL);
^
I think pgprot_val(PAGE_KERNEL[_NCG]) could be an option too.
Christian
> > ---
> > arch/powerpc/platforms/4xx/ocm.c | 6 --
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/4xx/ocm.c
> > b/arch/powerpc/platforms/4xx/ocm.c
> > index 561b09de69bf..04ff69ddac09 100644
> > --- a/arch/powerpc/platforms/4xx/ocm.c
> > +++ b/arch/powerpc/platforms/4xx/ocm.c
> > @@ -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);
> >
> > if (!ocm->nc.virt) {
> > printk(KERN_ERR
> > @@ -194,7 +195,8 @@ static void __init ocm_init_node(int count, struct
> > device_node *node)
> >
> > if (ocm->c.memtotal) {
> > ocm->c.virt = __ioremap(ocm->c.phys, ocm->c.memtotal,
> > - _PAGE_EXEC | PAGE_KERNEL);
> > + _PAGE_EXEC | _PAGE_BASE |
> > + _PAGE_KERNEL_RW);
> >
> > if (!ocm->c.virt) {
> > printk(KERN_ERR
> >
>
Re: [PATCH] powerpc/4xx/ocm: fix compiler error
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
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
---
arch/powerpc/platforms/4xx/ocm.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
index 561b09de69bf..04ff69ddac09 100644
--- a/arch/powerpc/platforms/4xx/ocm.c
+++ b/arch/powerpc/platforms/4xx/ocm.c
@@ -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);
if (!ocm->nc.virt) {
printk(KERN_ERR
@@ -194,7 +195,8 @@ static void __init ocm_init_node(int count, struct
device_node *node)
if (ocm->c.memtotal) {
ocm->c.virt = __ioremap(ocm->c.phys, ocm->c.memtotal,
- _PAGE_EXEC | PAGE_KERNEL);
+ _PAGE_EXEC | _PAGE_BASE |
+ _PAGE_KERNEL_RW);
if (!ocm->c.virt) {
printk(KERN_ERR
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel
antivirus Avast.
https://www.avast.com/antivirus
[PATCH] powerpc/4xx/ocm: fix compiler error
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
---
arch/powerpc/platforms/4xx/ocm.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
index 561b09de69bf..04ff69ddac09 100644
--- a/arch/powerpc/platforms/4xx/ocm.c
+++ b/arch/powerpc/platforms/4xx/ocm.c
@@ -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);
if (!ocm->nc.virt) {
printk(KERN_ERR
@@ -194,7 +195,8 @@ static void __init ocm_init_node(int count, struct
device_node *node)
if (ocm->c.memtotal) {
ocm->c.virt = __ioremap(ocm->c.phys, ocm->c.memtotal,
- _PAGE_EXEC | PAGE_KERNEL);
+ _PAGE_EXEC | _PAGE_BASE |
+ _PAGE_KERNEL_RW);
if (!ocm->c.virt) {
printk(KERN_ERR
--
2.20.1
