arch:unicore32:mm: add devmem_is_allowed() to support STRICT_DEVMEM

PATCH Superseded Chen Gang 3 participants 11 messages Apr 7, 2014 – Apr 16, 2014

Share & tools

Chen Gang

author

2014-04-07 11:28 UTC #0 lore raw

PATCH 1/1 arch:unicore32:mm: add devmem_is_allowed() to support STRICT_DEVMEM

unicore32 supports STRICT_DEVMEM, so it needs devmem_is_allowed(), like some of other architectures have done (e.g. arm, powerpc, x86 ...).

The related error with allmodconfig:

CC drivers/char/mem.o
drivers/char/mem.c: In function ‘range_is_allowed’:
drivers/char/mem.c:69: error: implicit declaration of function ‘devmem_is_allowed’ make[2]: *** [drivers/char/mem.o] Error 1
make[1]: *** [drivers/char] Error 2
make: *** [drivers] Error 2

Signed-off-by Chen Gang <[email protected]>

---
 arch/unicore32/include/asm/page.h |    4 ++++
 arch/unicore32/mm/init.c          |   18 ++++++++++++++++++
 2 files changed, 22 insertions(+)

arch/unicore32/include/asm/page.h +4 -0

index 594b322..231cb89 100644
@@ -68,6 +68,10 @@ typedef struct page *pgtable_t;
 
 extern int pfn_valid(unsigned long);
 
+#ifdef CONFIG_STRICT_DEVMEM
+extern int devmem_is_allowed(unsigned long pfn);
+#endif /* CONFIG_STRICT_DEVMEM */
+
 #include <asm/memory.h>
 
 #endif /* !__ASSEMBLY__ */

arch/unicore32/mm/init.c +18 -0

index be2bde9..3bc3a59 100644
@@ -449,3 +449,21 @@ static int __init keepinitrd_setup(char *__unused)
 
 __setup("keepinitrd", keepinitrd_setup);
 #endif
+
+#ifdef CONFIG_STRICT_DEVMEM
+/*
+ * devmem_is_allowed() checks to see if /dev/mem access to a certain
+ * address is valid. The argument is a physical page number.
+ * We mimic x86 here by disallowing access to system RAM as well as
+ * device-exclusive MMIO regions. This effectively disable read()/write()
+ * on /dev/mem.
+ */
+int devmem_is_allowed(unsigned long pfn)
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	return 0;
+}
+#endif /* CONFIG_STRICT_DEVMEM */

Signature · 1 line

1.7.9.5

管雪涛

Acked-by Xuetao Guan on v1 1/1

2014-04-08 03:20 UTC #1 lore raw

I'd like to put the code into asm/io.h, and make it static.

Quoted 7 lines

arch/unicore32/mm/init.c +7 -0

+#ifdef CONFIG_STRICT_DEVMEM
+/*
+ * devmem_is_allowed() checks to see if /dev/mem access to a certain
+ * address is valid. The argument is a physical page number.
+ * We mimic x86 here by disallowing access to system RAM as well as
+ * device-exclusive MMIO regions. This effectively disable read()/write()
+ * on /dev/mem.

arch/unicore32/mm/init.c static int __init keepinitrd_setup(char *__unused) · line 469

Quoted 10 lines

arch/unicore32/mm/init.c +10 -0

+ */
+static int devmem_is_allowed(unsigned long pfn)
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	return 0;
+}
+#endif /* CONFIG_STRICT_DEVMEM */

Thanks,

Acked-by Xuetao Guan <[email protected]>

Quoted 62 lines · ----- Chen Gang

unicore32 supports STRICT_DEVMEM, so it needs devmem_is_allowed(), like some of other architectures have done (e.g. arm, powerpc, x86 ...).
The related error with allmodconfig:
CC drivers/char/mem.o
drivers/char/mem.c: In function ‘range_is_allowed’:
drivers/char/mem.c:69: error: implicit declaration of function ‘devmem_is_allowed’ make[2]: *** [drivers/char/mem.o] Error 1
make[1]: *** [drivers/char] Error 2
make: *** [drivers] Error 2
Signed-off-by Chen Gang <[email protected]>
---
 arch/unicore32/include/asm/page.h |    4 ++++
 arch/unicore32/mm/init.c          |   18 ++++++++++++++++++
 2 files changed, 22 insertions(+)
arch/unicore32/include/asm/page.h +4 -0
index 594b322..231cb89 100644
@@ -68,6 +68,10 @@ typedef struct page *pgtable_t;
 
 extern int pfn_valid(unsigned long);
 
+#ifdef CONFIG_STRICT_DEVMEM
+extern int devmem_is_allowed(unsigned long pfn);
+#endif /* CONFIG_STRICT_DEVMEM */
+
 #include <asm/memory.h>
 
 #endif /* !__ASSEMBLY__ */
arch/unicore32/mm/init.c +18 -0
index be2bde9..3bc3a59 100644
@@ -449,3 +449,21 @@ static int __init keepinitrd_setup(char *__unused)
 
 __setup("keepinitrd", keepinitrd_setup);
 #endif
+
+#ifdef CONFIG_STRICT_DEVMEM
+/*
+ * devmem_is_allowed() checks to see if /dev/mem access to a certain
+ * address is valid. The argument is a physical page number.
+ * We mimic x86 here by disallowing access to system RAM as well as
+ * device-exclusive MMIO regions. This effectively disable read()/write()
+ * on /dev/mem.
+ */
+int devmem_is_allowed(unsigned long pfn)
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	return 0;
+}
+#endif /* CONFIG_STRICT_DEVMEM */
Signature · 1 line
1.7.9.5

Chen Gang

author

2014-04-08 04:54 UTC #2 lore raw

Quoted 2 lines · 管雪涛

I'd like to put the code into asm/io.h, and make it static.

It sounds OK to me, but I don't know why the other architectures (e.g. arm, powerpc, x86) put them into ".c".

iomem_is_exclusive() and page_is_ram() are all extern functions, so for me, devmem_is_allowed() is shot enough to be as inline function.

Quoted 9 lines

arch/unicore32/mm/init.c static int __init keepinitrd_setup(char *__unused)

Quoted 9 lines

arch/unicore32/mm/init.c +9 -0

+#ifdef CONFIG_STRICT_DEVMEM
+/*
+ * devmem_is_allowed() checks to see if /dev/mem access to a certain
+ * address is valid. The argument is a physical page number.
+ * We mimic x86 here by disallowing access to system RAM as well as
+ * device-exclusive MMIO regions. This effectively disable read()/write()
+ * on /dev/mem.
+ */
+static int devmem_is_allowed(unsigned long pfn)

How about "static inline int devmem_is_allowed(unsigned long pfn)"?

Quoted 11 lines

arch/unicore32/mm/init.c static int __init keepinitrd_setup(char *__unused) · line 469
Quoted 8 lines
arch/unicore32/mm/init.c +8 -0
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	return 0;
+}
+#endif /* CONFIG_STRICT_DEVMEM */
Thanks,

Quoted 2 lines

Acked-by Xuetao Guan <[email protected]>

[...]

Thanks.

Signature · 3 lines

Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Chen Gang

author

2014-04-15 00:14 UTC #3 lore raw

We got no-reply almost a week, so we can assume "can put the code into "asm/io.h" as static inline function".

I will/should send patch v2 for it.

Thanks.

Quoted 41 lines · Chen Gang

Quoted 2 lines · 管雪涛
I'd like to put the code into asm/io.h, and make it static.
It sounds OK to me, but I don't know why the other architectures (e.g. arm, powerpc, x86) put them into ".c".
iomem_is_exclusive() and page_is_ram() are all extern functions, so for me, devmem_is_allowed() is shot enough to be as inline function.
Quoted 9 lines
arch/unicore32/mm/init.c static int __init keepinitrd_setup(char *__unused)
Quoted 9 lines
arch/unicore32/mm/init.c +9 -0
+#ifdef CONFIG_STRICT_DEVMEM
+/*
+ * devmem_is_allowed() checks to see if /dev/mem access to a certain
+ * address is valid. The argument is a physical page number.
+ * We mimic x86 here by disallowing access to system RAM as well as
+ * device-exclusive MMIO regions. This effectively disable read()/write()
+ * on /dev/mem.
+ */
+static int devmem_is_allowed(unsigned long pfn)
How about "static inline int devmem_is_allowed(unsigned long pfn)"?
Quoted 11 lines
arch/unicore32/mm/init.c static int __init keepinitrd_setup(char *__unused) · line 469
Quoted 8 lines
arch/unicore32/mm/init.c +8 -0
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	return 0;
+}
+#endif /* CONFIG_STRICT_DEVMEM */
Thanks,
Quoted 2 lines
Acked-by Xuetao Guan <[email protected]>
[...]
Thanks.

Signature · 3 lines

Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Chen Gang

author

2014-04-15 00:29 UTC #4 lore raw

PATCH v2 1/1 arch:unicore32:mm: add devmem_is_allowed() to support STRICT_DEVMEM

unicore32 supports STRICT_DEVMEM, so it needs devmem_is_allowed(), like some of other architectures have done (e.g. arm, powerpc, x86 ...).

The related error with allmodconfig:

Signed-off-by Chen Gang <[email protected]>

---
 arch/unicore32/include/asm/io.h |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

arch/unicore32/include/asm/io.h +18 -0

index 39decb6..839c2ea 100644
@@ -44,5 +44,23 @@ extern void __uc32_iounmap(volatile void __iomem *addr);
 #define PIO_MASK		(unsigned int)(IO_SPACE_LIMIT)
 #define PIO_RESERVED		(PIO_OFFSET + PIO_MASK + 1)
 
+#ifdef CONFIG_STRICT_DEVMEM
+/*
+ * devmem_is_allowed() checks to see if /dev/mem access to a certain
+ * address is valid. The argument is a physical page number.
+ * We mimic x86 here by disallowing access to system RAM as well as
+ * device-exclusive MMIO regions. This effectively disable read()/write()
+ * on /dev/mem.
+ */
+static inline int devmem_is_allowed(unsigned long pfn)
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	return 0;
+}
+#endif /* CONFIG_STRICT_DEVMEM */
+
 #endif	/* __KERNEL__ */
 #endif	/* __UNICORE_IO_H__ */

Signature · 1 line

1.7.9.5

Chen Gang

author

2014-04-15 00:42 UTC #5 lore raw

Oh, sorry, it is my careless, it even can not pass compiling, I will/should send patch v3.

Thanks.

Quoted 47 lines · Chen Gang

unicore32 supports STRICT_DEVMEM, so it needs devmem_is_allowed(), like some of other architectures have done (e.g. arm, powerpc, x86 ...).
The related error with allmodconfig:
CC drivers/char/mem.o
drivers/char/mem.c: In function ‘range_is_allowed’:
drivers/char/mem.c:69: error: implicit declaration of function ‘devmem_is_allowed’ make[2]: *** [drivers/char/mem.o] Error 1
make[1]: *** [drivers/char] Error 2
make: *** [drivers] Error 2
Signed-off-by Chen Gang <[email protected]>
---
 arch/unicore32/include/asm/io.h |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
arch/unicore32/include/asm/io.h +18 -0
index 39decb6..839c2ea 100644
@@ -44,5 +44,23 @@ extern void __uc32_iounmap(volatile void __iomem *addr);
 #define PIO_MASK		(unsigned int)(IO_SPACE_LIMIT)
 #define PIO_RESERVED		(PIO_OFFSET + PIO_MASK + 1)
 
+#ifdef CONFIG_STRICT_DEVMEM
+/*
+ * devmem_is_allowed() checks to see if /dev/mem access to a certain
+ * address is valid. The argument is a physical page number.
+ * We mimic x86 here by disallowing access to system RAM as well as
+ * device-exclusive MMIO regions. This effectively disable read()/write()
+ * on /dev/mem.
+ */
+static inline int devmem_is_allowed(unsigned long pfn)
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	return 0;
+}
+#endif /* CONFIG_STRICT_DEVMEM */
+
 #endif	/* __KERNEL__ */
 #endif	/* __UNICORE_IO_H__ */

Signature · 3 lines

Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Chen Gang

author

2014-04-15 01:21 UTC #6 lore raw

PATCH v3 1/1 arch:unicore32:mm: add devmem_is_allowed() to support STRICT_DEVMEM

unicore32 supports STRICT_DEVMEM, so it needs devmem_is_allowed(), like some of other architectures have done (e.g. arm, powerpc, x86 ...).

The related error with allmodconfig:

Signed-off-by Chen Gang <[email protected]>

---
 arch/unicore32/include/asm/io.h |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

arch/unicore32/include/asm/io.h +23 -0

index 39decb6..ae327e4 100644
@@ -44,5 +44,28 @@ extern void __uc32_iounmap(volatile void __iomem *addr);
 #define PIO_MASK		(unsigned int)(IO_SPACE_LIMIT)
 #define PIO_RESERVED		(PIO_OFFSET + PIO_MASK + 1)
 
+#ifdef CONFIG_STRICT_DEVMEM
+
+#include <linux/ioport.h>
+#include <linux/mm.h>
+
+/*
+ * devmem_is_allowed() checks to see if /dev/mem access to a certain
+ * address is valid. The argument is a physical page number.
+ * We mimic x86 here by disallowing access to system RAM as well as
+ * device-exclusive MMIO regions. This effectively disable read()/write()
+ * on /dev/mem.
+ */
+static inline int devmem_is_allowed(unsigned long pfn)
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	return 0;
+}
+
+#endif /* CONFIG_STRICT_DEVMEM */
+
 #endif	/* __KERNEL__ */
 #endif	/* __UNICORE_IO_H__ */

Signature · 1 line

1.7.9.5

管雪涛

Acked-by Xuetao Guan on v3 1/1

2014-04-16 05:15 UTC #7 lore raw

Acked-by Xuetao Guan <[email protected]>

Quoted 54 lines · ----- Chen Gang

unicore32 supports STRICT_DEVMEM, so it needs devmem_is_allowed(), like some of other architectures have done (e.g. arm, powerpc, x86 ...).
The related error with allmodconfig:
CC drivers/char/mem.o
drivers/char/mem.c: In function ‘range_is_allowed’:
drivers/char/mem.c:69: error: implicit declaration of function ‘devmem_is_allowed’ make[2]: *** [drivers/char/mem.o] Error 1
make[1]: *** [drivers/char] Error 2
make: *** [drivers] Error 2
Signed-off-by Chen Gang <[email protected]>
---
 arch/unicore32/include/asm/io.h |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
arch/unicore32/include/asm/io.h +23 -0
index 39decb6..ae327e4 100644
@@ -44,5 +44,28 @@ extern void __uc32_iounmap(volatile void __iomem *addr);
 #define PIO_MASK		(unsigned int)(IO_SPACE_LIMIT)
 #define PIO_RESERVED		(PIO_OFFSET + PIO_MASK + 1)
 
+#ifdef CONFIG_STRICT_DEVMEM
+
+#include <linux/ioport.h>
+#include <linux/mm.h>
+
+/*
+ * devmem_is_allowed() checks to see if /dev/mem access to a certain
+ * address is valid. The argument is a physical page number.
+ * We mimic x86 here by disallowing access to system RAM as well as
+ * device-exclusive MMIO regions. This effectively disable read()/write()
+ * on /dev/mem.
+ */
+static inline int devmem_is_allowed(unsigned long pfn)
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	return 0;
+}
+
+#endif /* CONFIG_STRICT_DEVMEM */
+
 #endif	/* __KERNEL__ */
 #endif	/* __UNICORE_IO_H__ */
Signature · 1 line
1.7.9.5

Chen Gang

author

2014-04-16 05:43 UTC #8 lore raw

Quoted 2 lines · 管雪涛

Acked-by Xuetao Guan <[email protected]>

OK, thanks.

Quoted 56 lines

Quoted 54 lines · ----- Chen Gang
unicore32 supports STRICT_DEVMEM, so it needs devmem_is_allowed(), like some of other architectures have done (e.g. arm, powerpc, x86 ...).
The related error with allmodconfig:
CC drivers/char/mem.o
drivers/char/mem.c: In function ‘range_is_allowed’:
drivers/char/mem.c:69: error: implicit declaration of function ‘devmem_is_allowed’ make[2]: *** [drivers/char/mem.o] Error 1
make[1]: *** [drivers/char] Error 2
make: *** [drivers] Error 2
Signed-off-by Chen Gang <[email protected]>
---
 arch/unicore32/include/asm/io.h |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
arch/unicore32/include/asm/io.h +23 -0
index 39decb6..ae327e4 100644
@@ -44,5 +44,28 @@ extern void __uc32_iounmap(volatile void __iomem *addr);
 #define PIO_MASK		(unsigned int)(IO_SPACE_LIMIT)
 #define PIO_RESERVED		(PIO_OFFSET + PIO_MASK + 1)
 
+#ifdef CONFIG_STRICT_DEVMEM
+
+#include <linux/ioport.h>
+#include <linux/mm.h>
+
+/*
+ * devmem_is_allowed() checks to see if /dev/mem access to a certain
+ * address is valid. The argument is a physical page number.
+ * We mimic x86 here by disallowing access to system RAM as well as
+ * device-exclusive MMIO regions. This effectively disable read()/write()
+ * on /dev/mem.
+ */
+static inline int devmem_is_allowed(unsigned long pfn)
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	return 0;
+}
+
+#endif /* CONFIG_STRICT_DEVMEM */
+
 #endif	/* __KERNEL__ */
 #endif	/* __UNICORE_IO_H__ */
Signature · 1 line
1.7.9.5

Signature · 3 lines

Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Michal Hocko

2014-04-16 09:05 UTC #9 lore raw

In reply to Chen Gang

On Tue 15-04-14 09:21:30, Chen Gang wrote:
[...]

arch/unicore32/include/asm/io.h

Quoted 4 lines

arch/unicore32/include/asm/io.h +0 -0
index 39decb6..ae327e4 100644

There is already quite a mess where the function is defined. We have it in mmap.c, init.c, mem.c and s390 defines it in page.h. Is there any good reason to add yet another place and pull in additional header files dependencies?
Why not follow x86 here? Or even better git rid of the code duplication and provide generic implementation which different arches can reuse and extend? arm{64}, unicore32 seem to be identical. Powepc and x86 have an additional test to the core one used by the above arches. Only tile seems to be be doing something completely different.

Quoted 32 lines

arch/unicore32/include/asm/io.h +23 -0

@@ -44,5 +44,28 @@ extern void __uc32_iounmap(volatile void __iomem *addr);
 #define PIO_MASK		(unsigned int)(IO_SPACE_LIMIT)
 #define PIO_RESERVED		(PIO_OFFSET + PIO_MASK + 1)
 
+#ifdef CONFIG_STRICT_DEVMEM
+
+#include <linux/ioport.h>
+#include <linux/mm.h>
+
+/*
+ * devmem_is_allowed() checks to see if /dev/mem access to a certain
+ * address is valid. The argument is a physical page number.
+ * We mimic x86 here by disallowing access to system RAM as well as
+ * device-exclusive MMIO regions. This effectively disable read()/write()
+ * on /dev/mem.
+ */
+static inline int devmem_is_allowed(unsigned long pfn)
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	return 0;
+}
+
+#endif /* CONFIG_STRICT_DEVMEM */
+
 #endif	/* __KERNEL__ */
 #endif	/* __UNICORE_IO_H__ */

Signature · 1 line

1.7.9.5

Signature · 2 lines

Michal Hocko
SUSE Labs

Chen Gang

author

2014-04-16 10:11 UTC #10 lore raw

Quoted 11 lines · Michal Hocko

On Tue 15-04-14 09:21:30, Chen Gang wrote:
[...]
arch/unicore32/include/asm/io.h
Quoted 4 lines
arch/unicore32/include/asm/io.h +0 -0
index 39decb6..ae327e4 100644
There is already quite a mess where the function is defined. We have it in mmap.c, init.c, mem.c and s390 defines it in page.h. Is there any good reason to add yet another place and pull in additional header files dependencies?

It is short enough to be as static inline function. It also can bypass choosing which ".c" file contents the implementation (arm{64} declare it in "io.h").

So recommend devmem_is_allowed() in arm{64} are also "static inline".

Hmm... is it possible to move the static inline implementation from "io.h" to "page.h", since all other archs declare or implement in "page.h" related header file.

Quoted 6 lines

Why not follow x86 here? Or even better git rid of the code duplication and provide generic implementation which different arches can reuse and extend? arm{64}, unicore32 seem to be identical. Powepc and x86 have an additional test to the core one used by the above arches. Only tile seems to be be doing something completely different.

For me, need not put it in 'asm-generic':

- 9/29 archs need it, so at present, it is not generic enough.

- most of them are different, which hard to find generic one.

- frv and m32r same, but different with others.

- arm{64} and unicore32 same, but different with others.

- powerpc, s390, tile, and x86 are different with each other.

If we are sure most new archs will support devmem_is_allowed(), and at least, 40% are the same, we can continue to consider about whether put it in 'asm-generic' or not.

Altogether, for me, let the related declaration/implementation in "page.h" or related header file will be fine, at least present, it is not need to put it in 'asm-generic'.

[...]

Thanks.

Signature · 3 lines

Chen Gang

Open, share, and attitude like air, water, and life which God blessed