Re: [Xen-devel] [PATCH] xen/x86: Use 2M superpages for text/data/bss mappings

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.02.16 at 19:03, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -921,13 +921,51 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>              /* The only data mappings to be relocated are in the Xen area. */
>              pl2e = __va(__pa(l2_xenmap));
>              *pl2e++ = l2e_from_pfn(xen_phys_start >> PAGE_SHIFT,
> -                                   PAGE_HYPERVISOR_RWX | _PAGE_PSE);
> +                                   PAGE_HYPERVISOR_RX | _PAGE_PSE);
>              for ( i = 1; i < L2_PAGETABLE_ENTRIES; i++, pl2e++ )
>              {
> +                unsigned int flags;
> +
>                  if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
>                      continue;
> -                *pl2e = l2e_from_intpte(l2e_get_intpte(*pl2e) +
> -                                        xen_phys_start);
> +
> +                if ( /*
> +                      * Should be:
> +                      *
> +                      * i >= l2_table_offset((unsigned 
> long)&__2M_text_start) &&
> +                      *
> +                      * but the EFI build can't manage the relocation.  It
> +                      * evaluates to 0, so just use the upper bound.
> +                      */
> +                     i < l2_table_offset((unsigned long)&__2M_text_end) )

I'll need some more detail about this, not the least because
excusing what looks like a hack with EFI, under which we won't
ever get here, is suspicious.

> +                {
> +                    flags = PAGE_HYPERVISOR_RX | _PAGE_PSE;
> +                }
> +                else if ( i >= l2_table_offset((unsigned 
> long)&__2M_rodata_start) &&
> +                          i <  l2_table_offset((unsigned 
> long)&__2M_rodata_end) )
> +                {
> +                    flags = PAGE_HYPERVISOR_RO | _PAGE_PSE;
> +                }
> +                else if ( (i >= l2_table_offset((unsigned 
> long)&__2M_data_start) &&
> +                           i <  l2_table_offset((unsigned 
> long)&__2M_data_end)) ||
> +                          (i >= l2_table_offset((unsigned 
> long)&__2M_bss_start) &&
> +                           i <  l2_table_offset((unsigned 
> long)&__2M_bss_end)) )

This is odd - why can't .data and .bss share a (multiple of) 2M
region, at once presumably getting the whole image down to 10M
again?

> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -38,6 +38,9 @@ SECTIONS
>    . = __XEN_VIRT_START;
>    __image_base__ = .;
>  #endif
> +
> +  __2M_text_start = .;         /* Start of 2M superpages, mapped RX. */

Is the reason for aforementioned build problem perhaps the fact
that this label (and the others too) lives outside of any section?

> --- a/xen/include/xen/kernel.h
> +++ b/xen/include/xen/kernel.h
> @@ -65,6 +65,12 @@
>       1;                                      \
>  })
>  
> +extern unsigned long __2M_text_start[], __2M_text_end[];
> +extern unsigned long __2M_rodata_start[], __2M_rodata_end[];
> +extern unsigned long __2M_data_start[], __2M_data_end[];
> +extern unsigned long __2M_init_start[], __2M_init_end[];
> +extern unsigned long __2M_bss_start[], __2M_bss_end[];

I'd really like to see at least the ones which are reference to r/o
sections marked const.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel