Re: [Xen-devel] [PATCH 3/4] xen/link: Fold .data.read_mostly into .data

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

On 19/06/2019 22:28, Julien Grall wrote:
> On 6/19/19 9:11 PM, Andrew Cooper wrote:
>> .data.read_mostly only needs separating from adjacent data by one
>> cache line
>> to be effective, and placing it adjacent to .data.page_aligned
>> fulfills this
>> requirement.
>>
>> For ARM, .ex_table appears to be a vestigial remnant.  Nothing in the
>> resulting build ever inspects or acts on the contents of the table. 
>> The arm32
>> build does however have some assembly routines which fill .ex_table.
>>
>> Drop all of ARM's .ex_table infrastructure, to reduce the size of the
>> compiled
>> binary, and avoid giving the illusion of exception handling working.
>
> I am not in favor of this change. assembler.h is meant to be a
> verbatim copy of the Linux counterpart.

What alternative do you propose then, because having infrastructure that
gives the illusion of exception recovery working is a far worse option
than deviating from Linux.

>
> [...]
>
>> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
>> index 2b44e5d..3dc5117 100644
>> --- a/xen/arch/arm/xen.lds.S
>> +++ b/xen/arch/arm/xen.lds.S
>> @@ -94,27 +94,13 @@ SECTIONS
>>     _erodata = .;                /* End of read-only data */
>>       .data : {                    /* Data */
>> +       *(.data.read_mostly)
>
> Before, .data.read_mostly was SMP_CACHE_BYTES aligned. Now, it seems
> there are no alignment.
>
> This may end up to have _erodata and .data.read_mostly to be part of
> the same page. As Arm enforce read-only, you may crash on access to
> .data.read_mostly.
>
> So I think you have
> . = ALIGN(PAGE_SIZE)
> *(.data.read_mostly)
> .align(SMP_CACHE_BYTES).

These both need to be PAGE_SIZE, or .data.page_aligned can end up having
problems.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel