[PATCH 0/2] Add STRICT_DEVMEM support on RISC-V

[PATCH 0/2] Add STRICT_DEVMEM support on RISC-V

[Zong Li]

This patchset include two parts, the first one is that register System
RAM as iomem resources, it is needed for page_is_ram API which checks
the specified address whether registered as System RAM in iomem_resource
list. The second patch is that add devmem_is_allowed to support
STRICT_DEVMEM, and it would invoke page_is_ram to check the addresses.

Zong Li (2):
  riscv: Register System RAM as iomem resources
  riscv: Support CONFIG_STRICT_DEVMEM

 arch/riscv/Kconfig          |  1 +
 arch/riscv/include/asm/io.h |  2 ++
 arch/riscv/mm/init.c        | 41 +++++++++++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+)

-- 
2.27.0

[PATCH 1/2] riscv: Register System RAM as iomem resources

[Zong Li]

Add System RAM to /proc/iomem, various tools expect it such as kdump.
It is also needed for page_is_ram API which checks the specified address
whether registered as System RAM in iomem_resource list.

Signed-off-by: Zong Li <zong.li@sifive.com>
---
 arch/riscv/mm/init.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index f4adb3684f3d..bbe816e03b2f 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -517,6 +517,27 @@ void mark_rodata_ro(void)
 }
 #endif
 
+void __init resource_init(void)
+{
+	struct memblock_region *region;
+
+	for_each_memblock(memory, region) {
+		struct resource *res;
+
+		res = memblock_alloc(sizeof(struct resource), SMP_CACHE_BYTES);
+		if (!res)
+			panic("%s: Failed to allocate %zu bytes\n", __func__,
+			      sizeof(struct resource));
+
+		res->name = "System RAM";
+		res->start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
+		res->end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 1;
+		res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+
+		request_resource(&iomem_resource, res);
+	}
+}
+
 void __init paging_init(void)
 {
 	setup_vm_final();
@@ -524,6 +545,7 @@ void __init paging_init(void)
 	sparse_init();
 	setup_zero_page();
 	zone_sizes_init();
+	resource_init();
 }
 
 #ifdef CONFIG_SPARSEMEM_VMEMMAP
-- 
2.27.0

[PATCH 2/2] riscv: Support CONFIG_STRICT_DEVMEM

[Zong Li]

Implement the 'devmem_is_allowed()' interface for RISC-V, like some of
other architectures have done. It will be called from range_is_allowed()
when userpsace attempts to access /dev/mem.

Access to exclusive IOMEM and kernel RAM is denied unless
CONFIG_STRICT_DEVMEM is set to 'n'.

Test it by devmem, the result as follows:

 - CONFIG_STRICT_DEVMEM=y
	$ devmem 0x10010000
	0x00000000
	$ devmem 0x80200000
	0x0000106F

 - CONFIG_STRICT_DEVMEM is not set
	$ devmem 0x10010000
	devmem: mmap: Operation not permitted
	$ devmem 0x80200000
	devmem: mmap: Operation not permitted

Signed-off-by: Zong Li <zong.li@sifive.com>
---
 arch/riscv/Kconfig          |  1 +
 arch/riscv/include/asm/io.h |  2 ++
 arch/riscv/mm/init.c        | 19 +++++++++++++++++++
 3 files changed, 22 insertions(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 128192e14ff2..ffd7841ede4c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -16,6 +16,7 @@ config RISCV
 	select ARCH_HAS_BINFMT_FLAT
 	select ARCH_HAS_DEBUG_VIRTUAL if MMU
 	select ARCH_HAS_DEBUG_WX
+	select ARCH_HAS_DEVMEM_IS_ALLOWED
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_HAS_GIGANTIC_PAGE
 	select ARCH_HAS_MMIOWB
diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
index 3835c3295dc5..04ac65ab93ce 100644
--- a/arch/riscv/include/asm/io.h
+++ b/arch/riscv/include/asm/io.h
@@ -147,4 +147,6 @@ __io_writes_outs(outs, u64, q, __io_pbr(), __io_paw())
 
 #include <asm-generic/io.h>
 
+extern int devmem_is_allowed(unsigned long pfn);
+
 #endif /* _ASM_RISCV_IO_H */
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index bbe816e03b2f..5e7e61519acc 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -517,6 +517,25 @@ void mark_rodata_ro(void)
 }
 #endif
 
+#ifdef CONFIG_STRICT_DEVMEM
+#include <linux/ioport.h>
+/*
+ * devmem_is_allowed() checks to see if /dev/mem access to a certain address
+ * is valid. The argument is a physical page number.
+ *
+ * Disallow 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
+
 void __init resource_init(void)
 {
 	struct memblock_region *region;
-- 
2.27.0

Re: [PATCH 1/2] riscv: Register System RAM as iomem resources

[Nick Kossifidis]

Στις 2020-06-16 10:45, Zong Li έγραψε:
> Add System RAM to /proc/iomem, various tools expect it such as kdump.
> It is also needed for page_is_ram API which checks the specified 
> address
> whether registered as System RAM in iomem_resource list.
> 
> Signed-off-by: Zong Li <zong.li@sifive.com>
> ---
>  arch/riscv/mm/init.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index f4adb3684f3d..bbe816e03b2f 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -517,6 +517,27 @@ void mark_rodata_ro(void)
>  }
>  #endif
> 
> +void __init resource_init(void)
> +{
> +	struct memblock_region *region;
> +
> +	for_each_memblock(memory, region) {
> +		struct resource *res;
> +
> +		res = memblock_alloc(sizeof(struct resource), SMP_CACHE_BYTES);
> +		if (!res)
> +			panic("%s: Failed to allocate %zu bytes\n", __func__,
> +			      sizeof(struct resource));
> +
> +		res->name = "System RAM";
> +		res->start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
> +		res->end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 
> 1;
> +		res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> +
> +		request_resource(&iomem_resource, res);
> +	}
> +}
> +
>  void __init paging_init(void)
>  {
>  	setup_vm_final();
> @@ -524,6 +545,7 @@ void __init paging_init(void)
>  	sparse_init();
>  	setup_zero_page();
>  	zone_sizes_init();
> +	resource_init();
>  }
> 
>  #ifdef CONFIG_SPARSEMEM_VMEMMAP


I already have a patch for registering System RAM as an iomem resource 
on my kexec/kdump series. Since I don't care about System RAM regions 
being accurately exposed to userspace (I parse the current device tree 
instead) I just use memblock_start_of_DRAM/end_of_DRAM. This approach 
from arm64 codebase is better since it also handles the case of sparse 
memory regions but in order to be useful for kdump we need to add the 
various segments of the kernel image as child nodes to their respective 
region for kexec-tools. I'll re-spin my patchset anyway so I'll extend 
it to better handle System RAM regions.

Re: [PATCH 2/2] riscv: Support CONFIG_STRICT_DEVMEM

[Nick Kossifidis]

Στις 2020-06-16 10:45, Zong Li έγραψε:
> Implement the 'devmem_is_allowed()' interface for RISC-V, like some of
> other architectures have done. It will be called from 
> range_is_allowed()
> when userpsace attempts to access /dev/mem.
> 
> Access to exclusive IOMEM and kernel RAM is denied unless
> CONFIG_STRICT_DEVMEM is set to 'n'.
> 
> Test it by devmem, the result as follows:
> 
>  - CONFIG_STRICT_DEVMEM=y
> 	$ devmem 0x10010000
> 	0x00000000
> 	$ devmem 0x80200000
> 	0x0000106F
> 
>  - CONFIG_STRICT_DEVMEM is not set
> 	$ devmem 0x10010000
> 	devmem: mmap: Operation not permitted
> 	$ devmem 0x80200000
> 	devmem: mmap: Operation not permitted
> 
> Signed-off-by: Zong Li <zong.li@sifive.com>
> ---
>  arch/riscv/Kconfig          |  1 +
>  arch/riscv/include/asm/io.h |  2 ++
>  arch/riscv/mm/init.c        | 19 +++++++++++++++++++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 128192e14ff2..ffd7841ede4c 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -16,6 +16,7 @@ config RISCV
>  	select ARCH_HAS_BINFMT_FLAT
>  	select ARCH_HAS_DEBUG_VIRTUAL if MMU
>  	select ARCH_HAS_DEBUG_WX
> +	select ARCH_HAS_DEVMEM_IS_ALLOWED
>  	select ARCH_HAS_GCOV_PROFILE_ALL
>  	select ARCH_HAS_GIGANTIC_PAGE
>  	select ARCH_HAS_MMIOWB
> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
> index 3835c3295dc5..04ac65ab93ce 100644
> --- a/arch/riscv/include/asm/io.h
> +++ b/arch/riscv/include/asm/io.h
> @@ -147,4 +147,6 @@ __io_writes_outs(outs, u64, q, __io_pbr(), 
> __io_paw())
> 
>  #include <asm-generic/io.h>
> 
> +extern int devmem_is_allowed(unsigned long pfn);
> +
>  #endif /* _ASM_RISCV_IO_H */
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index bbe816e03b2f..5e7e61519acc 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -517,6 +517,25 @@ void mark_rodata_ro(void)
>  }
>  #endif
> 
> +#ifdef CONFIG_STRICT_DEVMEM
> +#include <linux/ioport.h>
> +/*
> + * devmem_is_allowed() checks to see if /dev/mem access to a certain 
> address
> + * is valid. The argument is a physical page number.
> + *
> + * Disallow 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
> +
>  void __init resource_init(void)
>  {
>  	struct memblock_region *region;

This shouldn't be part of /mm/init.c, it has nothing to do with memory 
initialization, I suggest we move it to another file like mmap.c on 
arm/arm64. Also before using iomem_is_exclusive we should probably also 
mark any reserved regions with the no-map attribute as busy|exclusive, 
reserved-memory regions are not necessarily part of the main memory so 
the page_is_ram check may pass and iomem_is_exclusive won't do any good.

Re: [PATCH 1/2] riscv: Register System RAM as iomem resources

[Zong Li]

On Tue, Jun 16, 2020 at 7:52 PM Nick Kossifidis <mick@ics.forth.gr> wrote:
>
> Στις 2020-06-16 10:45, Zong Li έγραψε:
> > Add System RAM to /proc/iomem, various tools expect it such as kdump.
> > It is also needed for page_is_ram API which checks the specified
> > address
> > whether registered as System RAM in iomem_resource list.
> >
> > Signed-off-by: Zong Li <zong.li@sifive.com>
> > ---
> >  arch/riscv/mm/init.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index f4adb3684f3d..bbe816e03b2f 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -517,6 +517,27 @@ void mark_rodata_ro(void)
> >  }
> >  #endif
> >
> > +void __init resource_init(void)
> > +{
> > +     struct memblock_region *region;
> > +
> > +     for_each_memblock(memory, region) {
> > +             struct resource *res;
> > +
> > +             res = memblock_alloc(sizeof(struct resource), SMP_CACHE_BYTES);
> > +             if (!res)
> > +                     panic("%s: Failed to allocate %zu bytes\n", __func__,
> > +                           sizeof(struct resource));
> > +
> > +             res->name = "System RAM";
> > +             res->start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
> > +             res->end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) -
> > 1;
> > +             res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> > +
> > +             request_resource(&iomem_resource, res);
> > +     }
> > +}
> > +
> >  void __init paging_init(void)
> >  {
> >       setup_vm_final();
> > @@ -524,6 +545,7 @@ void __init paging_init(void)
> >       sparse_init();
> >       setup_zero_page();
> >       zone_sizes_init();
> > +     resource_init();
> >  }
> >
> >  #ifdef CONFIG_SPARSEMEM_VMEMMAP
>
>
> I already have a patch for registering System RAM as an iomem resource
> on my kexec/kdump series. Since I don't care about System RAM regions
> being accurately exposed to userspace (I parse the current device tree
> instead) I just use memblock_start_of_DRAM/end_of_DRAM. This approach
> from arm64 codebase is better since it also handles the case of sparse
> memory regions but in order to be useful for kdump we need to add the
> various segments of the kernel image as child nodes to their respective
> region for kexec-tools. I'll re-spin my patchset anyway so I'll extend
> it to better handle System RAM regions.

OK, great, I would remove this patch here and only reserve the second
patch in the next version.

Re: [PATCH 2/2] riscv: Support CONFIG_STRICT_DEVMEM

[Zong Li]

On Tue, Jun 16, 2020 at 8:27 PM Nick Kossifidis <mick@ics.forth.gr> wrote:
>
> Στις 2020-06-16 10:45, Zong Li έγραψε:
> > Implement the 'devmem_is_allowed()' interface for RISC-V, like some of
> > other architectures have done. It will be called from
> > range_is_allowed()
> > when userpsace attempts to access /dev/mem.
> >
> > Access to exclusive IOMEM and kernel RAM is denied unless
> > CONFIG_STRICT_DEVMEM is set to 'n'.
> >
> > Test it by devmem, the result as follows:
> >
> >  - CONFIG_STRICT_DEVMEM=y
> >       $ devmem 0x10010000
> >       0x00000000
> >       $ devmem 0x80200000
> >       0x0000106F
> >
> >  - CONFIG_STRICT_DEVMEM is not set
> >       $ devmem 0x10010000
> >       devmem: mmap: Operation not permitted
> >       $ devmem 0x80200000
> >       devmem: mmap: Operation not permitted
> >
> > Signed-off-by: Zong Li <zong.li@sifive.com>
> > ---
> >  arch/riscv/Kconfig          |  1 +
> >  arch/riscv/include/asm/io.h |  2 ++
> >  arch/riscv/mm/init.c        | 19 +++++++++++++++++++
> >  3 files changed, 22 insertions(+)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 128192e14ff2..ffd7841ede4c 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -16,6 +16,7 @@ config RISCV
> >       select ARCH_HAS_BINFMT_FLAT
> >       select ARCH_HAS_DEBUG_VIRTUAL if MMU
> >       select ARCH_HAS_DEBUG_WX
> > +     select ARCH_HAS_DEVMEM_IS_ALLOWED
> >       select ARCH_HAS_GCOV_PROFILE_ALL
> >       select ARCH_HAS_GIGANTIC_PAGE
> >       select ARCH_HAS_MMIOWB
> > diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
> > index 3835c3295dc5..04ac65ab93ce 100644
> > --- a/arch/riscv/include/asm/io.h
> > +++ b/arch/riscv/include/asm/io.h
> > @@ -147,4 +147,6 @@ __io_writes_outs(outs, u64, q, __io_pbr(),
> > __io_paw())
> >
> >  #include <asm-generic/io.h>
> >
> > +extern int devmem_is_allowed(unsigned long pfn);
> > +
> >  #endif /* _ASM_RISCV_IO_H */
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index bbe816e03b2f..5e7e61519acc 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -517,6 +517,25 @@ void mark_rodata_ro(void)
> >  }
> >  #endif
> >
> > +#ifdef CONFIG_STRICT_DEVMEM
> > +#include <linux/ioport.h>
> > +/*
> > + * devmem_is_allowed() checks to see if /dev/mem access to a certain
> > address
> > + * is valid. The argument is a physical page number.
> > + *
> > + * Disallow 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
> > +
> >  void __init resource_init(void)
> >  {
> >       struct memblock_region *region;
>
> This shouldn't be part of /mm/init.c, it has nothing to do with memory
> initialization, I suggest we move it to another file like mmap.c on

Let me move it, thanks.

> arm/arm64. Also before using iomem_is_exclusive we should probably also
> mark any reserved regions with the no-map attribute as busy|exclusive,
> reserved-memory regions are not necessarily part of the main memory so
> the page_is_ram check may pass and iomem_is_exclusive won't do any good.

What do you think if we mark the reserved region in
kdump_resource_init, and change the kdump_resource_init to a more
generic name for initializing resources?

Re: [PATCH 2/2] riscv: Support CONFIG_STRICT_DEVMEM

[Nick Kossifidis]

Στις 2020-06-17 04:56, Zong Li έγραψε:
> On Tue, Jun 16, 2020 at 8:27 PM Nick Kossifidis <mick@ics.forth.gr> 
> wrote:
>> 
>> Στις 2020-06-16 10:45, Zong Li έγραψε:
>> > Implement the 'devmem_is_allowed()' interface for RISC-V, like some of
>> > other architectures have done. It will be called from
>> > range_is_allowed()
>> > when userpsace attempts to access /dev/mem.
>> >
>> > Access to exclusive IOMEM and kernel RAM is denied unless
>> > CONFIG_STRICT_DEVMEM is set to 'n'.
>> >
>> > Test it by devmem, the result as follows:
>> >
>> >  - CONFIG_STRICT_DEVMEM=y
>> >       $ devmem 0x10010000
>> >       0x00000000
>> >       $ devmem 0x80200000
>> >       0x0000106F
>> >
>> >  - CONFIG_STRICT_DEVMEM is not set
>> >       $ devmem 0x10010000
>> >       devmem: mmap: Operation not permitted
>> >       $ devmem 0x80200000
>> >       devmem: mmap: Operation not permitted
>> >
>> > Signed-off-by: Zong Li <zong.li@sifive.com>
>> > ---
>> >  arch/riscv/Kconfig          |  1 +
>> >  arch/riscv/include/asm/io.h |  2 ++
>> >  arch/riscv/mm/init.c        | 19 +++++++++++++++++++
>> >  3 files changed, 22 insertions(+)
>> >
>> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> > index 128192e14ff2..ffd7841ede4c 100644
>> > --- a/arch/riscv/Kconfig
>> > +++ b/arch/riscv/Kconfig
>> > @@ -16,6 +16,7 @@ config RISCV
>> >       select ARCH_HAS_BINFMT_FLAT
>> >       select ARCH_HAS_DEBUG_VIRTUAL if MMU
>> >       select ARCH_HAS_DEBUG_WX
>> > +     select ARCH_HAS_DEVMEM_IS_ALLOWED
>> >       select ARCH_HAS_GCOV_PROFILE_ALL
>> >       select ARCH_HAS_GIGANTIC_PAGE
>> >       select ARCH_HAS_MMIOWB
>> > diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
>> > index 3835c3295dc5..04ac65ab93ce 100644
>> > --- a/arch/riscv/include/asm/io.h
>> > +++ b/arch/riscv/include/asm/io.h
>> > @@ -147,4 +147,6 @@ __io_writes_outs(outs, u64, q, __io_pbr(),
>> > __io_paw())
>> >
>> >  #include <asm-generic/io.h>
>> >
>> > +extern int devmem_is_allowed(unsigned long pfn);
>> > +
>> >  #endif /* _ASM_RISCV_IO_H */
>> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> > index bbe816e03b2f..5e7e61519acc 100644
>> > --- a/arch/riscv/mm/init.c
>> > +++ b/arch/riscv/mm/init.c
>> > @@ -517,6 +517,25 @@ void mark_rodata_ro(void)
>> >  }
>> >  #endif
>> >
>> > +#ifdef CONFIG_STRICT_DEVMEM
>> > +#include <linux/ioport.h>
>> > +/*
>> > + * devmem_is_allowed() checks to see if /dev/mem access to a certain
>> > address
>> > + * is valid. The argument is a physical page number.
>> > + *
>> > + * Disallow 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
>> > +
>> >  void __init resource_init(void)
>> >  {
>> >       struct memblock_region *region;
>> 
>> This shouldn't be part of /mm/init.c, it has nothing to do with memory
>> initialization, I suggest we move it to another file like mmap.c on
> 
> Let me move it, thanks.
> 
>> arm/arm64. Also before using iomem_is_exclusive we should probably 
>> also
>> mark any reserved regions with the no-map attribute as busy|exclusive,
>> reserved-memory regions are not necessarily part of the main memory so
>> the page_is_ram check may pass and iomem_is_exclusive won't do any 
>> good.
> 
> What do you think if we mark the reserved region in
> kdump_resource_init, and change the kdump_resource_init to a more
> generic name for initializing resources?

Sounds good to me, I'll work on this within the week. Do you agree with 
marking the no-map reserved-memory regions as exclusive ?

Re: [PATCH 2/2] riscv: Support CONFIG_STRICT_DEVMEM

[Zong Li]

On Wed, Jun 17, 2020 at 1:28 PM Nick Kossifidis <mick@ics.forth.gr> wrote:
>
> Στις 2020-06-17 04:56, Zong Li έγραψε:
> > On Tue, Jun 16, 2020 at 8:27 PM Nick Kossifidis <mick@ics.forth.gr>
> > wrote:
> >>
> >> Στις 2020-06-16 10:45, Zong Li έγραψε:
> >> > Implement the 'devmem_is_allowed()' interface for RISC-V, like some of
> >> > other architectures have done. It will be called from
> >> > range_is_allowed()
> >> > when userpsace attempts to access /dev/mem.
> >> >
> >> > Access to exclusive IOMEM and kernel RAM is denied unless
> >> > CONFIG_STRICT_DEVMEM is set to 'n'.
> >> >
> >> > Test it by devmem, the result as follows:
> >> >
> >> >  - CONFIG_STRICT_DEVMEM=y
> >> >       $ devmem 0x10010000
> >> >       0x00000000
> >> >       $ devmem 0x80200000
> >> >       0x0000106F
> >> >
> >> >  - CONFIG_STRICT_DEVMEM is not set
> >> >       $ devmem 0x10010000
> >> >       devmem: mmap: Operation not permitted
> >> >       $ devmem 0x80200000
> >> >       devmem: mmap: Operation not permitted
> >> >
> >> > Signed-off-by: Zong Li <zong.li@sifive.com>
> >> > ---
> >> >  arch/riscv/Kconfig          |  1 +
> >> >  arch/riscv/include/asm/io.h |  2 ++
> >> >  arch/riscv/mm/init.c        | 19 +++++++++++++++++++
> >> >  3 files changed, 22 insertions(+)
> >> >
> >> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> >> > index 128192e14ff2..ffd7841ede4c 100644
> >> > --- a/arch/riscv/Kconfig
> >> > +++ b/arch/riscv/Kconfig
> >> > @@ -16,6 +16,7 @@ config RISCV
> >> >       select ARCH_HAS_BINFMT_FLAT
> >> >       select ARCH_HAS_DEBUG_VIRTUAL if MMU
> >> >       select ARCH_HAS_DEBUG_WX
> >> > +     select ARCH_HAS_DEVMEM_IS_ALLOWED
> >> >       select ARCH_HAS_GCOV_PROFILE_ALL
> >> >       select ARCH_HAS_GIGANTIC_PAGE
> >> >       select ARCH_HAS_MMIOWB
> >> > diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
> >> > index 3835c3295dc5..04ac65ab93ce 100644
> >> > --- a/arch/riscv/include/asm/io.h
> >> > +++ b/arch/riscv/include/asm/io.h
> >> > @@ -147,4 +147,6 @@ __io_writes_outs(outs, u64, q, __io_pbr(),
> >> > __io_paw())
> >> >
> >> >  #include <asm-generic/io.h>
> >> >
> >> > +extern int devmem_is_allowed(unsigned long pfn);
> >> > +
> >> >  #endif /* _ASM_RISCV_IO_H */
> >> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> >> > index bbe816e03b2f..5e7e61519acc 100644
> >> > --- a/arch/riscv/mm/init.c
> >> > +++ b/arch/riscv/mm/init.c
> >> > @@ -517,6 +517,25 @@ void mark_rodata_ro(void)
> >> >  }
> >> >  #endif
> >> >
> >> > +#ifdef CONFIG_STRICT_DEVMEM
> >> > +#include <linux/ioport.h>
> >> > +/*
> >> > + * devmem_is_allowed() checks to see if /dev/mem access to a certain
> >> > address
> >> > + * is valid. The argument is a physical page number.
> >> > + *
> >> > + * Disallow 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
> >> > +
> >> >  void __init resource_init(void)
> >> >  {
> >> >       struct memblock_region *region;
> >>
> >> This shouldn't be part of /mm/init.c, it has nothing to do with memory
> >> initialization, I suggest we move it to another file like mmap.c on
> >
> > Let me move it, thanks.
> >
> >> arm/arm64. Also before using iomem_is_exclusive we should probably
> >> also
> >> mark any reserved regions with the no-map attribute as busy|exclusive,
> >> reserved-memory regions are not necessarily part of the main memory so
> >> the page_is_ram check may pass and iomem_is_exclusive won't do any
> >> good.
> >
> > What do you think if we mark the reserved region in
> > kdump_resource_init, and change the kdump_resource_init to a more
> > generic name for initializing resources?
>
> Sounds good to me, I'll work on this within the week. Do you agree with
> marking the no-map reserved-memory regions as exclusive ?

It's OK to me, It seems to me that exclusive is more suitable than
busy when initialization.

Re: [PATCH 1/2] riscv: Register System RAM as iomem resources

[Palmer Dabbelt]

On Tue, 16 Jun 2020 00:45:46 PDT (-0700), zong.li@sifive.com wrote:
> Add System RAM to /proc/iomem, various tools expect it such as kdump.
> It is also needed for page_is_ram API which checks the specified address
> whether registered as System RAM in iomem_resource list.
>
> Signed-off-by: Zong Li <zong.li@sifive.com>
> ---
>  arch/riscv/mm/init.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index f4adb3684f3d..bbe816e03b2f 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -517,6 +517,27 @@ void mark_rodata_ro(void)
>  }
>  #endif
>
> +void __init resource_init(void)
> +{
> +	struct memblock_region *region;
> +
> +	for_each_memblock(memory, region) {
> +		struct resource *res;
> +
> +		res = memblock_alloc(sizeof(struct resource), SMP_CACHE_BYTES);
> +		if (!res)
> +			panic("%s: Failed to allocate %zu bytes\n", __func__,
> +			      sizeof(struct resource));
> +
> +		res->name = "System RAM";
> +		res->start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
> +		res->end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 1;
> +		res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;

Looks like everyone else is checking MEMBLOCK_NOMAP before registering memory
regions.  I've added that and put this on for-next.  Thanks!

commit 11dc632bf515874c84887727614e8044452f1f28
gpg: Signature made Thu 09 Jul 2020 11:24:08 AM PDT
gpg:                using RSA key 2B3C3747446843B24A943A7A2E1319F35FBB1889
gpg:                issuer "palmer@dabbelt.com"
gpg: Good signature from "Palmer Dabbelt <palmer@dabbelt.com>" [ultimate]
gpg:                 aka "Palmer Dabbelt <palmerdabbelt@google.com>" [ultimate]
Author: Zong Li <zong.li@sifive.com>
Date:   Tue Jun 16 15:45:46 2020 +0800

    riscv: Register System RAM as iomem resources
    
    Add System RAM to /proc/iomem, various tools expect it such as kdump.
    It is also needed for page_is_ram API which checks the specified address
    whether registered as System RAM in iomem_resource list.
    
    Signed-off-by: Zong Li <zong.li@sifive.com>
    [Palmer: check MEMBLOCK_NOMAP]
    Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index f4adb3684f3d..8b78fd23713e 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -517,6 +517,32 @@ void mark_rodata_ro(void)
 }
 #endif
 
+void __init resource_init(void)
+{
+	struct memblock_region *region;
+
+	for_each_memblock(memory, region) {
+		struct resource *res;
+
+		res = memblock_alloc(sizeof(struct resource), SMP_CACHE_BYTES);
+		if (!res)
+			panic("%s: Failed to allocate %zu bytes\n", __func__,
+			      sizeof(struct resource));
+
+		if (memblock_is_nomap(region) {
+			res->name = "reserved";
+			res->flags = IORESOURCE_MEM;
+		} else {
+			res->name = "System RAM";
+			res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+		}
+		res->start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
+		res->end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 1;
+
+		request_resource(&iomem_resource, res);
+	}
+}
+
 void __init paging_init(void)
 {
 	setup_vm_final();
@@ -524,6 +550,7 @@ void __init paging_init(void)
 	sparse_init();
 	setup_zero_page();
 	zone_sizes_init();
+	resource_init();
 }
 
 #ifdef CONFIG_SPARSEMEM_VMEMMAP


> +
> +		request_resource(&iomem_resource, res);
> +	}
> +}
> +
>  void __init paging_init(void)
>  {
>  	setup_vm_final();
> @@ -524,6 +545,7 @@ void __init paging_init(void)
>  	sparse_init();
>  	setup_zero_page();
>  	zone_sizes_init();
> +	resource_init();
>  }
>
>  #ifdef CONFIG_SPARSEMEM_VMEMMAP

Re: [PATCH 2/2] riscv: Support CONFIG_STRICT_DEVMEM

[Palmer Dabbelt]

On Tue, 16 Jun 2020 00:45:47 PDT (-0700), zong.li@sifive.com wrote:
> Implement the 'devmem_is_allowed()' interface for RISC-V, like some of
> other architectures have done. It will be called from range_is_allowed()
> when userpsace attempts to access /dev/mem.

In fact, it's exactly the same (down to a few words of the comment) to the
others that I checked.  I'm going to put a generic version in lib/ instead,
I've sent out the patches.

> Access to exclusive IOMEM and kernel RAM is denied unless
> CONFIG_STRICT_DEVMEM is set to 'n'.
>
> Test it by devmem, the result as follows:
>
>  - CONFIG_STRICT_DEVMEM=y
> 	$ devmem 0x10010000
> 	0x00000000
> 	$ devmem 0x80200000
> 	0x0000106F
>
>  - CONFIG_STRICT_DEVMEM is not set
> 	$ devmem 0x10010000
> 	devmem: mmap: Operation not permitted
> 	$ devmem 0x80200000
> 	devmem: mmap: Operation not permitted
>
> Signed-off-by: Zong Li <zong.li@sifive.com>
> ---
>  arch/riscv/Kconfig          |  1 +
>  arch/riscv/include/asm/io.h |  2 ++
>  arch/riscv/mm/init.c        | 19 +++++++++++++++++++
>  3 files changed, 22 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 128192e14ff2..ffd7841ede4c 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -16,6 +16,7 @@ config RISCV
>  	select ARCH_HAS_BINFMT_FLAT
>  	select ARCH_HAS_DEBUG_VIRTUAL if MMU
>  	select ARCH_HAS_DEBUG_WX
> +	select ARCH_HAS_DEVMEM_IS_ALLOWED
>  	select ARCH_HAS_GCOV_PROFILE_ALL
>  	select ARCH_HAS_GIGANTIC_PAGE
>  	select ARCH_HAS_MMIOWB
> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
> index 3835c3295dc5..04ac65ab93ce 100644
> --- a/arch/riscv/include/asm/io.h
> +++ b/arch/riscv/include/asm/io.h
> @@ -147,4 +147,6 @@ __io_writes_outs(outs, u64, q, __io_pbr(), __io_paw())
>
>  #include <asm-generic/io.h>
>
> +extern int devmem_is_allowed(unsigned long pfn);
> +
>  #endif /* _ASM_RISCV_IO_H */
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index bbe816e03b2f..5e7e61519acc 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -517,6 +517,25 @@ void mark_rodata_ro(void)
>  }
>  #endif
>
> +#ifdef CONFIG_STRICT_DEVMEM
> +#include <linux/ioport.h>
> +/*
> + * devmem_is_allowed() checks to see if /dev/mem access to a certain address
> + * is valid. The argument is a physical page number.
> + *
> + * Disallow 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
> +
>  void __init resource_init(void)
>  {
>  	struct memblock_region *region;

Re: [PATCH 1/2] riscv: Register System RAM as iomem resources

[Nick Kossifidis]

Στις 2020-07-09 21:27, Palmer Dabbelt έγραψε:
> On Tue, 16 Jun 2020 00:45:46 PDT (-0700), zong.li@sifive.com wrote:
>> Add System RAM to /proc/iomem, various tools expect it such as kdump.
>> It is also needed for page_is_ram API which checks the specified 
>> address
>> whether registered as System RAM in iomem_resource list.
>> 
>> Signed-off-by: Zong Li <zong.li@sifive.com>
>> ---
>>  arch/riscv/mm/init.c | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>> 
>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> index f4adb3684f3d..bbe816e03b2f 100644
>> --- a/arch/riscv/mm/init.c
>> +++ b/arch/riscv/mm/init.c
>> @@ -517,6 +517,27 @@ void mark_rodata_ro(void)
>>  }
>>  #endif
>> 
>> +void __init resource_init(void)
>> +{
>> +	struct memblock_region *region;
>> +
>> +	for_each_memblock(memory, region) {
>> +		struct resource *res;
>> +
>> +		res = memblock_alloc(sizeof(struct resource), SMP_CACHE_BYTES);
>> +		if (!res)
>> +			panic("%s: Failed to allocate %zu bytes\n", __func__,
>> +			      sizeof(struct resource));
>> +
>> +		res->name = "System RAM";
>> +		res->start = 
>> __pfn_to_phys(memblock_region_memory_base_pfn(region));
>> +		res->end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 
>> 1;
>> +		res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> 
> Looks like everyone else is checking MEMBLOCK_NOMAP before registering 
> memory
> regions.  I've added that and put this on for-next.  Thanks!
> 
> commit 11dc632bf515874c84887727614e8044452f1f28
> gpg: Signature made Thu 09 Jul 2020 11:24:08 AM PDT
> gpg:                using RSA key 
> 2B3C3747446843B24A943A7A2E1319F35FBB1889
> gpg:                issuer "palmer@dabbelt.com"
> gpg: Good signature from "Palmer Dabbelt <palmer@dabbelt.com>" 
> [ultimate]
> gpg:                 aka "Palmer Dabbelt <palmerdabbelt@google.com>" 
> [ultimate]
> Author: Zong Li <zong.li@sifive.com>
> Date:   Tue Jun 16 15:45:46 2020 +0800
> 
>    riscv: Register System RAM as iomem resources
>       Add System RAM to /proc/iomem, various tools expect it such as 
> kdump.
>    It is also needed for page_is_ram API which checks the specified 
> address
>    whether registered as System RAM in iomem_resource list.
>       Signed-off-by: Zong Li <zong.li@sifive.com>
>    [Palmer: check MEMBLOCK_NOMAP]
>    Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
> 
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index f4adb3684f3d..8b78fd23713e 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -517,6 +517,32 @@ void mark_rodata_ro(void)
> }
> #endif
> 
> +void __init resource_init(void)
> +{
> +	struct memblock_region *region;
> +
> +	for_each_memblock(memory, region) {
> +		struct resource *res;
> +
> +		res = memblock_alloc(sizeof(struct resource), SMP_CACHE_BYTES);
> +		if (!res)
> +			panic("%s: Failed to allocate %zu bytes\n", __func__,
> +			      sizeof(struct resource));
> +
> +		if (memblock_is_nomap(region) {
> +			res->name = "reserved";
> +			res->flags = IORESOURCE_MEM;
> +		} else {
> +			res->name = "System RAM";
> +			res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> +		}
> +		res->start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
> +		res->end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 
> 1;
> +
> +		request_resource(&iomem_resource, res);
> +	}
> +}
> +
> void __init paging_init(void)
> {
> 	setup_vm_final();
> @@ -524,6 +550,7 @@ void __init paging_init(void)
> 	sparse_init();
> 	setup_zero_page();
> 	zone_sizes_init();
> +	resource_init();
> }
> 
> #ifdef CONFIG_SPARSEMEM_VMEMMAP
> 
> 
>> +
>> +		request_resource(&iomem_resource, res);
>> +	}
>> +}
>> +
>>  void __init paging_init(void)
>>  {
>>  	setup_vm_final();
>> @@ -524,6 +545,7 @@ void __init paging_init(void)
>>  	sparse_init();
>>  	setup_zero_page();
>>  	zone_sizes_init();
>> +	resource_init();
>>  }
>> 
>>  #ifdef CONFIG_SPARSEMEM_VMEMMAP
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Zong Li sent a newer version of this series without this patch, since 
I'm handling this on the kexec/kdump series as well where other sections 
needed for kdump are also registered.

Re: [PATCH 2/2] riscv: Support CONFIG_STRICT_DEVMEM

[Zong Li]

On Fri, Jul 10, 2020 at 4:08 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Tue, 16 Jun 2020 00:45:47 PDT (-0700), zong.li@sifive.com wrote:
> > Implement the 'devmem_is_allowed()' interface for RISC-V, like some of
> > other architectures have done. It will be called from range_is_allowed()
> > when userpsace attempts to access /dev/mem.
>
> In fact, it's exactly the same (down to a few words of the comment) to the
> others that I checked.  I'm going to put a generic version in lib/ instead,
> I've sent out the patches.
>

OK, no problem, thanks for improving it.

> > Access to exclusive IOMEM and kernel RAM is denied unless
> > CONFIG_STRICT_DEVMEM is set to 'n'.
> >
> > Test it by devmem, the result as follows:
> >
> >  - CONFIG_STRICT_DEVMEM=y
> >       $ devmem 0x10010000
> >       0x00000000
> >       $ devmem 0x80200000
> >       0x0000106F
> >
> >  - CONFIG_STRICT_DEVMEM is not set
> >       $ devmem 0x10010000
> >       devmem: mmap: Operation not permitted
> >       $ devmem 0x80200000
> >       devmem: mmap: Operation not permitted
> >
> > Signed-off-by: Zong Li <zong.li@sifive.com>
> > ---
> >  arch/riscv/Kconfig          |  1 +
> >  arch/riscv/include/asm/io.h |  2 ++
> >  arch/riscv/mm/init.c        | 19 +++++++++++++++++++
> >  3 files changed, 22 insertions(+)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 128192e14ff2..ffd7841ede4c 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -16,6 +16,7 @@ config RISCV
> >       select ARCH_HAS_BINFMT_FLAT
> >       select ARCH_HAS_DEBUG_VIRTUAL if MMU
> >       select ARCH_HAS_DEBUG_WX
> > +     select ARCH_HAS_DEVMEM_IS_ALLOWED
> >       select ARCH_HAS_GCOV_PROFILE_ALL
> >       select ARCH_HAS_GIGANTIC_PAGE
> >       select ARCH_HAS_MMIOWB
> > diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
> > index 3835c3295dc5..04ac65ab93ce 100644
> > --- a/arch/riscv/include/asm/io.h
> > +++ b/arch/riscv/include/asm/io.h
> > @@ -147,4 +147,6 @@ __io_writes_outs(outs, u64, q, __io_pbr(), __io_paw())
> >
> >  #include <asm-generic/io.h>
> >
> > +extern int devmem_is_allowed(unsigned long pfn);
> > +
> >  #endif /* _ASM_RISCV_IO_H */
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index bbe816e03b2f..5e7e61519acc 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -517,6 +517,25 @@ void mark_rodata_ro(void)
> >  }
> >  #endif
> >
> > +#ifdef CONFIG_STRICT_DEVMEM
> > +#include <linux/ioport.h>
> > +/*
> > + * devmem_is_allowed() checks to see if /dev/mem access to a certain address
> > + * is valid. The argument is a physical page number.
> > + *
> > + * Disallow 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
> > +
> >  void __init resource_init(void)
> >  {
> >       struct memblock_region *region;