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

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 22/02/16 09:55, Jan Beulich wrote:
>
>>>> +                {
>>>> +                    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?
>> .init is between .data and .bss, to allow .bss to be the final section
>> and not included in the result of mkelf32
> But I don't think it needs to remain there? Should be possible to be
> put between .text and .rodata, or between .rodata and .data ...

Actually yes - shifting .init to between .rodata and .data should work fine.

>
>>>> --- 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?
>> I am not sure.  It is only this symbol which is a problem.  All others
>> are fine.
>>
>> I actually intended this to be an RFC patch, to see if anyone had
>> suggestions.
> Since you now imply this to be at the image base, I don't see
> why using e.g. __XEN_VIRT_START (in its place, or via #define)
> wouldn't be as good an option.

I don't understand what you mean here.

If you are suggesting #define __2M_text_start __XEN_VIRT_START then I
don't see how that is going to help with the relocation.

~Andrew