mbox series

[0/4] arm64: Cleanup ioremap() and support ioremap_prot()

Message ID 20220427121413.168468-1-wangkefeng.wang@huawei.com (mailing list archive)
Headers show
Series arm64: Cleanup ioremap() and support ioremap_prot() | expand

Message

Kefeng Wang April 27, 2022, 12:14 p.m. UTC
Let's arm64 use GENERIC_IOREMAP to cleanup code, and
support ioremap_prot()/HAVE_IOREMAP_PROT, which could
enable generic_access_phys().

Kefeng Wang (4):
  mm: ioremap: Setup phys_addr of struct vm_struct
  mm: ioremap: Add arch_ioremap/iounmap_check()
  arm64: mm: Convert to GENERIC_IOREMAP
  arm64: Add HAVE_IOREMAP_PROT support

 .../features/vm/ioremap_prot/arch-support.txt |  2 +-
 arch/arm64/Kconfig                            |  2 +
 arch/arm64/include/asm/io.h                   | 14 +--
 arch/arm64/include/asm/pgtable.h              | 10 +++
 arch/arm64/kernel/acpi.c                      |  2 +-
 arch/arm64/mm/hugetlbpage.c                   | 10 ---
 arch/arm64/mm/ioremap.c                       | 86 +++----------------
 include/asm-generic/io.h                      |  3 +
 mm/ioremap.c                                  | 21 ++++-
 9 files changed, 56 insertions(+), 94 deletions(-)

Comments

Will Deacon April 28, 2022, 10:46 a.m. UTC | #1
On Wed, Apr 27, 2022 at 08:14:09PM +0800, Kefeng Wang wrote:
> Let's arm64 use GENERIC_IOREMAP to cleanup code, and
> support ioremap_prot()/HAVE_IOREMAP_PROT, which could
> enable generic_access_phys().
> 
> Kefeng Wang (4):
>   mm: ioremap: Setup phys_addr of struct vm_struct
>   mm: ioremap: Add arch_ioremap/iounmap_check()
>   arm64: mm: Convert to GENERIC_IOREMAP
>   arm64: Add HAVE_IOREMAP_PROT support
> 
>  .../features/vm/ioremap_prot/arch-support.txt |  2 +-
>  arch/arm64/Kconfig                            |  2 +
>  arch/arm64/include/asm/io.h                   | 14 +--
>  arch/arm64/include/asm/pgtable.h              | 10 +++
>  arch/arm64/kernel/acpi.c                      |  2 +-
>  arch/arm64/mm/hugetlbpage.c                   | 10 ---
>  arch/arm64/mm/ioremap.c                       | 86 +++----------------
>  include/asm-generic/io.h                      |  3 +
>  mm/ioremap.c                                  | 21 ++++-
>  9 files changed, 56 insertions(+), 94 deletions(-)

That's not a massively compelling diffstat for a cleanup, in all honesty.
I looked at generic_access_phys() to try to figure out why we would want
that on arm64, but it seems like it's related to mmap() of devices in
userspace. Bearing in mind that CONFIG_STRICT_DEVMEM=y by default, please
can you justify why this is something worth doing?

Will
Kefeng Wang April 28, 2022, 11:48 a.m. UTC | #2
On 2022/4/28 18:46, Will Deacon wrote:
> On Wed, Apr 27, 2022 at 08:14:09PM +0800, Kefeng Wang wrote:
>> Let's arm64 use GENERIC_IOREMAP to cleanup code, and
>> support ioremap_prot()/HAVE_IOREMAP_PROT, which could
>> enable generic_access_phys().
>>
>> Kefeng Wang (4):
>>    mm: ioremap: Setup phys_addr of struct vm_struct
>>    mm: ioremap: Add arch_ioremap/iounmap_check()
>>    arm64: mm: Convert to GENERIC_IOREMAP
>>    arm64: Add HAVE_IOREMAP_PROT support
>>
>>   .../features/vm/ioremap_prot/arch-support.txt |  2 +-
>>   arch/arm64/Kconfig                            |  2 +
>>   arch/arm64/include/asm/io.h                   | 14 +--
>>   arch/arm64/include/asm/pgtable.h              | 10 +++
>>   arch/arm64/kernel/acpi.c                      |  2 +-
>>   arch/arm64/mm/hugetlbpage.c                   | 10 ---
>>   arch/arm64/mm/ioremap.c                       | 86 +++----------------
>>   include/asm-generic/io.h                      |  3 +
>>   mm/ioremap.c                                  | 21 ++++-
>>   9 files changed, 56 insertions(+), 94 deletions(-)
> That's not a massively compelling diffstat for a cleanup, in all honesty.
> I looked at generic_access_phys() to try to figure out why we would want
> that on arm64, but it seems like it's related to mmap() of devices in
> userspace. Bearing in mind that CONFIG_STRICT_DEVMEM=y by default, please
> can you justify why this is something worth doing?

The geneirc_access_phys was introduced by

   7ae8ed5053a3 use generic_access_phys for /dev/mem mappings
   28b2ee20c7cb access_process_vm device memory infrastructure

and add into uio.c  by

   7294151d0592 "uio: provide vm access to UIO_MEM_PHYS maps"

It could let user to debug(eg, gdb or ptrace) app via access_process_vm().

also some upstream drivers and out-of-tree modules could use it too
   drivers/fpga/dfl-afu-main.c:    .access = generic_access_phys,
   drivers/pci/mmap.c:     .access = generic_access_phys,

When see the ioremap_prot from generic ioremap, for better debug, I think

we could enabled it on arm64 too.

> Will
> .
Christoph Hellwig April 28, 2022, 3:48 p.m. UTC | #3
On Thu, Apr 28, 2022 at 11:46:12AM +0100, Will Deacon wrote:
> That's not a massively compelling diffstat for a cleanup, in all honesty.
> I looked at generic_access_phys() to try to figure out why we would want
> that on arm64, but it seems like it's related to mmap() of devices in
> userspace. Bearing in mind that CONFIG_STRICT_DEVMEM=y by default, please
> can you justify why this is something worth doing?

While I don't care much about ioremap_prot I'd really love to eventually
convert everyone to the common ioremap code as there really is no
point in duplicating it in the architectures.