All of lore.kernel.org
 help / color / mirror / Atom feed
From: Charlie Jenkins <charlie@rivosinc.com>
To: Conor Dooley <conor@kernel.org>
Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	paul.walmsley@sifive.com, palmer@rivosinc.com,
	aou@eecs.berkeley.edu, anup@brainfault.org,
	konstantin@linuxfoundation.org, linux-doc@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-mm@kvack.org,
	mick@ics.forth.gr, jrtc27@jrtc27.com, rdunlap@infradead.org,
	alexghiti@rivosinc.com
Subject: Re: [PATCH v7 1/4] RISC-V: mm: Restrict address space for sv39,sv48,sv57
Date: Thu, 27 Jul 2023 17:29:16 -0400	[thread overview]
Message-ID: <ZMLhrFfoerPIxRL7@ghost> (raw)
In-Reply-To: <20230727-unruffled-joyride-410fc348ce7b@spud>

On Thu, Jul 27, 2023 at 01:33:41PM +0100, Conor Dooley wrote:
> Hey Charlie,
> 
> On Wed, Jul 26, 2023 at 09:45:55AM -0700, Charlie Jenkins wrote:
> > Make sv48 the default address space for mmap as some applications
> > currently depend on this assumption. A hint address passed to mmap will
> > cause the largest address space that fits entirely into the hint to be
> > used. If the hint is less than or equal to 1<<38, an sv39 address will
> > be used. An exception is that if the hint address is 0, then a sv48
> > address will be used. After an address space is completely full, the next
> > smallest address space will be used.
> > 
> > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > ---
> >  arch/riscv/include/asm/elf.h       |  2 +-
> >  arch/riscv/include/asm/pgtable.h   | 13 ++++++++-
> >  arch/riscv/include/asm/processor.h | 47 +++++++++++++++++++++++++-----
> >  3 files changed, 53 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
> > index c24280774caf..5d3368d5585c 100644
> > --- a/arch/riscv/include/asm/elf.h
> > +++ b/arch/riscv/include/asm/elf.h
> > @@ -49,7 +49,7 @@ extern bool compat_elf_check_arch(Elf32_Ehdr *hdr);
> >   * the loader.  We need to make sure that it is out of the way of the program
> >   * that it will "exec", and that there is sufficient room for the brk.
> >   */
> > -#define ELF_ET_DYN_BASE		((TASK_SIZE / 3) * 2)
> > +#define ELF_ET_DYN_BASE		((DEFAULT_MAP_WINDOW / 3) * 2)
> >  
> >  #ifdef CONFIG_64BIT
> >  #ifdef CONFIG_COMPAT
> > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> > index 75970ee2bda2..530f6a171a2b 100644
> > --- a/arch/riscv/include/asm/pgtable.h
> > +++ b/arch/riscv/include/asm/pgtable.h
> > @@ -63,12 +63,23 @@
> >   * position vmemmap directly below the VMALLOC region.
> >   */
> >  #ifdef CONFIG_64BIT
> > +#define VA_BITS_SV39 39
> > +#define VA_BITS_SV48 48
> > +#define VA_BITS_SV57 57
> > +
> > +#define VA_USER_SV39 (UL(1) << (VA_BITS_SV39 - 1))
> > +#define VA_USER_SV48 (UL(1) << (VA_BITS_SV48 - 1))
> > +#define VA_USER_SV57 (UL(1) << (VA_BITS_SV57 - 1))
> > +
> >  #define VA_BITS		(pgtable_l5_enabled ? \
> > -				57 : (pgtable_l4_enabled ? 48 : 39))
> > +				VA_BITS_SV57 : (pgtable_l4_enabled ? VA_BITS_SV48 : VA_BITS_SV39))
> >  #else
> >  #define VA_BITS		32
> >  #endif
> 
> Please, at the very least, build test things for rv32 if you are going
> to change things in mm:
> io_uring/io_uring.c:3457:20: error: use of undeclared identifier 'VA_BITS_SV39'
> io_uring/io_uring.c:3457:20: error: use of undeclared identifier 'VA_BITS_SV39'
> io_uring/io_uring.c:3457:20: error: use of undeclared identifier 'VA_BITS_SV39'
> io_uring/io_uring.c:3457:20: error: use of undeclared identifier 'VA_BITS_SV39'
> io_uring/io_uring.c:3457:20: error: use of undeclared identifier 'VA_BITS_SV39'
> io_uring/io_uring.c:3457:20: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/util.c:441:19: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/util.c:441:19: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/util.c:441:19: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/util.c:441:19: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/util.c:441:19: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/util.c:441:19: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/mmap.c:1770:20: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/mmap.c:1770:20: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/mmap.c:1770:20: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/mmap.c:1770:20: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/mmap.c:1770:20: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/mmap.c:1770:20: error: use of undeclared identifier 'VA_BITS_SV39'
> 
> Thanks,
> Conor.
Thanks for that catch, I sent out a new patch to fix that up. There were
also some problems with 32-bit compat support that I resolved.


WARNING: multiple messages have this Message-ID (diff)
From: Charlie Jenkins <charlie@rivosinc.com>
To: Conor Dooley <conor@kernel.org>
Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	paul.walmsley@sifive.com, palmer@rivosinc.com,
	aou@eecs.berkeley.edu, anup@brainfault.org,
	konstantin@linuxfoundation.org, linux-doc@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-mm@kvack.org,
	mick@ics.forth.gr, jrtc27@jrtc27.com, rdunlap@infradead.org,
	alexghiti@rivosinc.com
Subject: Re: [PATCH v7 1/4] RISC-V: mm: Restrict address space for sv39,sv48,sv57
Date: Thu, 27 Jul 2023 17:29:16 -0400	[thread overview]
Message-ID: <ZMLhrFfoerPIxRL7@ghost> (raw)
In-Reply-To: <20230727-unruffled-joyride-410fc348ce7b@spud>

On Thu, Jul 27, 2023 at 01:33:41PM +0100, Conor Dooley wrote:
> Hey Charlie,
> 
> On Wed, Jul 26, 2023 at 09:45:55AM -0700, Charlie Jenkins wrote:
> > Make sv48 the default address space for mmap as some applications
> > currently depend on this assumption. A hint address passed to mmap will
> > cause the largest address space that fits entirely into the hint to be
> > used. If the hint is less than or equal to 1<<38, an sv39 address will
> > be used. An exception is that if the hint address is 0, then a sv48
> > address will be used. After an address space is completely full, the next
> > smallest address space will be used.
> > 
> > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > ---
> >  arch/riscv/include/asm/elf.h       |  2 +-
> >  arch/riscv/include/asm/pgtable.h   | 13 ++++++++-
> >  arch/riscv/include/asm/processor.h | 47 +++++++++++++++++++++++++-----
> >  3 files changed, 53 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
> > index c24280774caf..5d3368d5585c 100644
> > --- a/arch/riscv/include/asm/elf.h
> > +++ b/arch/riscv/include/asm/elf.h
> > @@ -49,7 +49,7 @@ extern bool compat_elf_check_arch(Elf32_Ehdr *hdr);
> >   * the loader.  We need to make sure that it is out of the way of the program
> >   * that it will "exec", and that there is sufficient room for the brk.
> >   */
> > -#define ELF_ET_DYN_BASE		((TASK_SIZE / 3) * 2)
> > +#define ELF_ET_DYN_BASE		((DEFAULT_MAP_WINDOW / 3) * 2)
> >  
> >  #ifdef CONFIG_64BIT
> >  #ifdef CONFIG_COMPAT
> > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> > index 75970ee2bda2..530f6a171a2b 100644
> > --- a/arch/riscv/include/asm/pgtable.h
> > +++ b/arch/riscv/include/asm/pgtable.h
> > @@ -63,12 +63,23 @@
> >   * position vmemmap directly below the VMALLOC region.
> >   */
> >  #ifdef CONFIG_64BIT
> > +#define VA_BITS_SV39 39
> > +#define VA_BITS_SV48 48
> > +#define VA_BITS_SV57 57
> > +
> > +#define VA_USER_SV39 (UL(1) << (VA_BITS_SV39 - 1))
> > +#define VA_USER_SV48 (UL(1) << (VA_BITS_SV48 - 1))
> > +#define VA_USER_SV57 (UL(1) << (VA_BITS_SV57 - 1))
> > +
> >  #define VA_BITS		(pgtable_l5_enabled ? \
> > -				57 : (pgtable_l4_enabled ? 48 : 39))
> > +				VA_BITS_SV57 : (pgtable_l4_enabled ? VA_BITS_SV48 : VA_BITS_SV39))
> >  #else
> >  #define VA_BITS		32
> >  #endif
> 
> Please, at the very least, build test things for rv32 if you are going
> to change things in mm:
> io_uring/io_uring.c:3457:20: error: use of undeclared identifier 'VA_BITS_SV39'
> io_uring/io_uring.c:3457:20: error: use of undeclared identifier 'VA_BITS_SV39'
> io_uring/io_uring.c:3457:20: error: use of undeclared identifier 'VA_BITS_SV39'
> io_uring/io_uring.c:3457:20: error: use of undeclared identifier 'VA_BITS_SV39'
> io_uring/io_uring.c:3457:20: error: use of undeclared identifier 'VA_BITS_SV39'
> io_uring/io_uring.c:3457:20: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/util.c:441:19: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/util.c:441:19: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/util.c:441:19: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/util.c:441:19: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/util.c:441:19: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/util.c:441:19: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/mmap.c:1770:20: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/mmap.c:1770:20: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/mmap.c:1770:20: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/mmap.c:1770:20: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/mmap.c:1770:20: error: use of undeclared identifier 'VA_BITS_SV39'
> mm/mmap.c:1770:20: error: use of undeclared identifier 'VA_BITS_SV39'
> 
> Thanks,
> Conor.
Thanks for that catch, I sent out a new patch to fix that up. There were
also some problems with 32-bit compat support that I resolved.


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2023-07-27 21:29 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-26 16:45 [PATCH v7 0/4] RISC-V: mm: Make SV48 the default address space Charlie Jenkins
2023-07-26 16:45 ` Charlie Jenkins
2023-07-26 16:45 ` [PATCH v7 1/4] RISC-V: mm: Restrict address space for sv39,sv48,sv57 Charlie Jenkins
2023-07-26 16:45   ` Charlie Jenkins
2023-07-27 12:33   ` Conor Dooley
2023-07-27 12:33     ` Conor Dooley
2023-07-27 21:29     ` Charlie Jenkins [this message]
2023-07-27 21:29       ` Charlie Jenkins
2023-07-26 16:45 ` [PATCH v7 2/4] RISC-V: mm: Add tests for RISC-V mm Charlie Jenkins
2023-07-26 16:45   ` Charlie Jenkins
2023-07-26 16:45 ` [PATCH v7 3/4] RISC-V: mm: Update pgtable comment documentation Charlie Jenkins
2023-07-26 16:45   ` Charlie Jenkins
2023-07-26 16:45 ` [PATCH v7 4/4] RISC-V: mm: Document mmap changes Charlie Jenkins
2023-07-26 16:45   ` Charlie Jenkins

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZMLhrFfoerPIxRL7@ghost \
    --to=charlie@rivosinc.com \
    --cc=alexghiti@rivosinc.com \
    --cc=anup@brainfault.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=conor@kernel.org \
    --cc=jrtc27@jrtc27.com \
    --cc=konstantin@linuxfoundation.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=mick@ics.forth.gr \
    --cc=palmer@rivosinc.com \
    --cc=paul.walmsley@sifive.com \
    --cc=rdunlap@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.