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(+)
diff --git a/arch/unicore32/include/asm/page.h b/arch/unicore32/include/asm/page.h
index 594b322..231cb89 100644
--- a/arch/unicore32/include/asm/page.h
+++ b/arch/unicore32/include/asm/page.h
@@ -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__ */
diff --git a/arch/unicore32/mm/init.c b/arch/unicore32/mm/init.c
index be2bde9..3bc3a59 100644
--- a/arch/unicore32/mm/init.c
+++ b/arch/unicore32/mm/init.c
@@ -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 */
--
1.7.9.5
I'd like to put the code into asm/io.h, and make it static.
> +#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)
> +{
> + 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]>
----- Chen Gang <[email protected]> 写道：
> 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(+)
>
> diff --git a/arch/unicore32/include/asm/page.h b/arch/unicore32/include/asm/page.h
> index 594b322..231cb89 100644
> --- a/arch/unicore32/include/asm/page.h
> +++ b/arch/unicore32/include/asm/page.h
> @@ -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__ */
> diff --git a/arch/unicore32/mm/init.c b/arch/unicore32/mm/init.c
> index be2bde9..3bc3a59 100644
> --- a/arch/unicore32/mm/init.c
> +++ b/arch/unicore32/mm/init.c
> @@ -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 */
> --
> 1.7.9.5
On 04/08/2014 11:20 AM, 管雪涛 wrote:
> 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.
>> +#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)"?
>> +{
>> + 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]>
>
[...]
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
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.
On 04/08/2014 12:54 PM, Chen Gang wrote:
>
> On 04/08/2014 11:20 AM, 管雪涛 wrote:
>> 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.
>
>>> +#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)"?
>
>>> +{
>>> + 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]>
>>
>
> [...]
>
> Thanks.
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
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(+)
diff --git a/arch/unicore32/include/asm/io.h b/arch/unicore32/include/asm/io.h
index 39decb6..839c2ea 100644
--- a/arch/unicore32/include/asm/io.h
+++ b/arch/unicore32/include/asm/io.h
@@ -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__ */
--
1.7.9.5
Oh, sorry, it is my careless, it even can not pass compiling, I
will/should send patch v3.
Thanks.
On 04/15/2014 08:28 AM, Chen Gang wrote:
> 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(+)
>
> diff --git a/arch/unicore32/include/asm/io.h b/arch/unicore32/include/asm/io.h
> index 39decb6..839c2ea 100644
> --- a/arch/unicore32/include/asm/io.h
> +++ b/arch/unicore32/include/asm/io.h
> @@ -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__ */
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
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(+)
diff --git a/arch/unicore32/include/asm/io.h b/arch/unicore32/include/asm/io.h
index 39decb6..ae327e4 100644
--- a/arch/unicore32/include/asm/io.h
+++ b/arch/unicore32/include/asm/io.h
@@ -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__ */
--
1.7.9.5
Acked-by: Xuetao Guan <[email protected]>
----- Chen Gang <[email protected]> 写道：
> 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(+)
>
> diff --git a/arch/unicore32/include/asm/io.h b/arch/unicore32/include/asm/io.h
> index 39decb6..ae327e4 100644
> --- a/arch/unicore32/include/asm/io.h
> +++ b/arch/unicore32/include/asm/io.h
> @@ -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__ */
> --
> 1.7.9.5
>
On 04/16/2014 01:15 PM, 管雪涛 wrote:
> Acked-by: Xuetao Guan <[email protected]>
>
OK, thanks.
> ----- Chen Gang <[email protected]> 写道：
>> 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(+)
>>
>> diff --git a/arch/unicore32/include/asm/io.h b/arch/unicore32/include/asm/io.h
>> index 39decb6..ae327e4 100644
>> --- a/arch/unicore32/include/asm/io.h
>> +++ b/arch/unicore32/include/asm/io.h
>> @@ -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__ */
>> --
>> 1.7.9.5
>>
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
On Tue 15-04-14 09:21:30, Chen Gang wrote:
[...]
> diff --git a/arch/unicore32/include/asm/io.h b/arch/unicore32/include/asm/io.h
> index 39decb6..ae327e4 100644
> --- a/arch/unicore32/include/asm/io.h
> +++ b/arch/unicore32/include/asm/io.h
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.
> @@ -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__ */
> --
> 1.7.9.5
>
--
Michal Hocko
SUSE Labs
On 04/16/2014 05:05 PM, Michal Hocko wrote:
> On Tue 15-04-14 09:21:30, Chen Gang wrote:
> [...]
>> diff --git a/arch/unicore32/include/asm/io.h b/arch/unicore32/include/asm/io.h
>> index 39decb6..ae327e4 100644
>> --- a/arch/unicore32/include/asm/io.h
>> +++ b/arch/unicore32/include/asm/io.h
>
> 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.
> 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.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed