Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1580711 > unrolled thread

Re: [PATCH v4 1/3] x86: Introduce a new constant KERNEL_MAPPING_SIZE

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v4 1/3] x86: Introduce a new constant KERNEL_MAPPING_SIZE Borislav Petkov <bp@suse.de> - 2017-02-14 18:40 +0100
    Re: [PATCH v4 1/3] x86: Introduce a new constant KERNEL_MAPPING_SIZE Baoquan He <bhe@redhat.com> - 2017-02-26 05:20 +0100

#1580711 — Re: [PATCH v4 1/3] x86: Introduce a new constant KERNEL_MAPPING_SIZE

On Thu, Feb 02, 2017 at 08:54:35PM +0800, Baoquan He wrote:
> In x86, KERNEL_IMAGE_SIZE is used to limit the size of kernel image in
> running space, but also represents the size of kernel image mapping area.
> This looks good when kernel virtual address is invariable inside 512M
> area and kernel image size is not bigger than 512M.
> 
> Along with the adding of kaslr, in x86_64 the area of kernel mapping is
> extended up another 512M. It becomes improper to let KERNEL_IMAGE_SIZE
> alone still play two roles now.
> 
> So introduce a new constant KERNEL_MAPPING_SIZE to represent the size of
> kernel mapping area. Let KERNEL_IMAGE_SIZE be as its name is saying. In
> x86_32 though kernel image size is the same as kernel mapping size, for
> generic handling in kaslr.c KERNEL_MAPPING_SIZE is also introduced.
> 
> In this patch, just add KERNEL_MAPPING_SIZE and replace KERNEL_IMAGE_SIZE
> with it in the relevant places. No functional change.
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  arch/x86/boot/compressed/kaslr.c        | 10 +++++-----
>  arch/x86/include/asm/page_32_types.h    |  6 ++++++
>  arch/x86/include/asm/page_64_types.h    | 12 +++++++++---
>  arch/x86/include/asm/pgtable_64_types.h |  2 +-
>  arch/x86/kernel/head64.c                |  4 ++--
>  arch/x86/kernel/head_64.S               |  2 +-
>  arch/x86/kernel/machine_kexec_64.c      |  2 +-
>  arch/x86/mm/init_64.c                   |  2 +-
>  arch/x86/mm/physaddr.c                  |  6 +++---
>  9 files changed, 29 insertions(+), 17 deletions(-)

...

> diff --git a/arch/x86/include/asm/page_64_types.h b/arch/x86/include/asm/page_64_types.h
> index 9215e05..24c9098 100644
> --- a/arch/x86/include/asm/page_64_types.h
> +++ b/arch/x86/include/asm/page_64_types.h
> @@ -50,16 +50,22 @@
>  #define __VIRTUAL_MASK_SHIFT	47
>  
>  /*
> - * Kernel image size is limited to 1GiB due to the fixmap living in the
> + * Kernel image size is limited to 512 MB. The kernel code+data+bss

This is not what it said there before. With your change you have:

- 0
.
.
.
- 512 - KERNEL_IMAGE_SIZE
.
.
.
- 1024 - KERNEL_MAPPING_SIZE

and KERNEL_IMAGE_SIZE is not limited to 512Mb but it is "Use 512Mib by
default". And we do enforce that in various places like in the linker
script assertions but there's some headroom open in the upper 512Mib if
needed.

KERNEL_MAPPING_SIZE OTOH is the one limited to 1G due to the fixmap L2
PGT...

> + * must not be bigger than that.
> + */
> +#define KERNEL_IMAGE_SIZE	(512 * 1024 * 1024)
> +
> +/*
> + * Kernel mapping size is limited to 1GiB due to the fixmap living in the
>   * next 1GiB (see level2_kernel_pgt in arch/x86/kernel/head_64.S). Use
>   * 512MiB by default, leaving 1.5GiB for modules once the page tables
>   * are fully set up. If kernel ASLR is configured, it can extend the
>   * kernel page table mapping, reducing the size of the modules area.
>   */
>  #if defined(CONFIG_RANDOMIZE_BASE)
> -#define KERNEL_IMAGE_SIZE	(1024 * 1024 * 1024)
> +#define KERNEL_MAPPING_SIZE	(1024 * 1024 * 1024)
>  #else
> -#define KERNEL_IMAGE_SIZE	(512 * 1024 * 1024)
> +#define KERNEL_MAPPING_SIZE	(512 * 1024 * 1024)
>  #endif

... and since you're adding that define now, fixup the comments in this
patch too, to explain what they mean.

Also, I'd like for the text to say that both defines are dependent in
the sense that IMAGE_SIZE <= MAPPING_SIZE so that people know what's
going on and which is which.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[toc] | [next] | [standalone]

#1588271

Hi Boris,

Thanks a lot for your comments, sorry so late to reply!

On 02/14/17 at 06:32pm, Borislav Petkov wrote:
> > diff --git a/arch/x86/include/asm/page_64_types.h b/arch/x86/include/asm/page_64_types.h
> > index 9215e05..24c9098 100644
> > --- a/arch/x86/include/asm/page_64_types.h
> > +++ b/arch/x86/include/asm/page_64_types.h
> > @@ -50,16 +50,22 @@
> >  #define __VIRTUAL_MASK_SHIFT	47
> >  
> >  /*
> > - * Kernel image size is limited to 1GiB due to the fixmap living in the
> > + * Kernel image size is limited to 512 MB. The kernel code+data+bss
> 
> This is not what it said there before. With your change you have:
> 
> - 0
> .
> .
> .
> - 512 - KERNEL_IMAGE_SIZE
> .
> .
> .
> - 1024 - KERNEL_MAPPING_SIZE
> 
> and KERNEL_IMAGE_SIZE is not limited to 512Mb but it is "Use 512Mib by
> default". And we do enforce that in various places like in the linker
> script assertions but there's some headroom open in the upper 512Mib if
> needed.

Before below commit merged, it said:

 * Kernel image size is limited to 512 MB (see level2_kernel_pgt in
 * arch/x86/kernel/head_64.S), and it is mapped here:
 */
#define KERNEL_IMAGE_SIZE      (512 * 1024 * 1024)

commit 6145cfe3 ("x86, kaslr: Raise the maximum virtual address to -1
GiB on x86_64")

So you can see KERNEL_IMAGE_SIZE is a constant value and not optional.
Then Kees posted above commit, The default mentioned in "Use 512Mib by
default" should be KERNEL_IMAGE_SIZE in the case that kaslr code not
compiled in, 512M, which is relative to the case that kaslr code
compiled in, 1G. In fact, it's meaning the kernel mapping size defaults
to 512M, and will change to 1G if CONFIG_RANDOMIZE_BASE is chosen.

Seems in kernel only linker script checks kernel image size as below:

. = ASSERT((_end - _text <= KERNEL_IMAGE_SIZE),                                                                                                  
           "kernel image bigger than KERNEL_IMAGE_SIZE");

In arch/x86/kernel/head64.c, it's more likely checking the kernel
mapping size, just because they share the same constant,
KERNEL_IMAGE_SIZE.

About headroom, in boot/compressed/kaslr.c kernel iamge size is
considered, including head room. Here I am not quite sure. Do you mean:
KERNEL_IMAGE_SIZE use 512Mib by default, only kerel image size is
limited to 512M which is (_end - _text) since linker script will check
it. While with headroom, it could be bigger than 512M if needed.

Am I right on understanding it?

> 
> KERNEL_MAPPING_SIZE OTOH is the one limited to 1G due to the fixmap L2
> PGT...
> 
> > + * must not be bigger than that.
> > + */
> > +#define KERNEL_IMAGE_SIZE	(512 * 1024 * 1024)
> > +
> > +/*
> > + * Kernel mapping size is limited to 1GiB due to the fixmap living in the
> >   * next 1GiB (see level2_kernel_pgt in arch/x86/kernel/head_64.S). Use
> >   * 512MiB by default, leaving 1.5GiB for modules once the page tables
> >   * are fully set up. If kernel ASLR is configured, it can extend the
> >   * kernel page table mapping, reducing the size of the modules area.
> >   */
> >  #if defined(CONFIG_RANDOMIZE_BASE)
> > -#define KERNEL_IMAGE_SIZE	(1024 * 1024 * 1024)
> > +#define KERNEL_MAPPING_SIZE	(1024 * 1024 * 1024)
> >  #else
> > -#define KERNEL_IMAGE_SIZE	(512 * 1024 * 1024)
> > +#define KERNEL_MAPPING_SIZE	(512 * 1024 * 1024)
> >  #endif
> 
> ... and since you're adding that define now, fixup the comments in this
> patch too, to explain what they mean.

Yes, agree, this makes it clearer. Will do.

> 
> Also, I'd like for the text to say that both defines are dependent in
> the sense that IMAGE_SIZE <= MAPPING_SIZE so that people know what's
> going on and which is which.

It makes sense, will do. 

Thanks for great suggestion!

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web