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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andrew,

On 6/19/19 10:48 PM, Andrew Cooper wrote:
> 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.
I learnt the hard way before that trying to adapt a Linux file to Xen 
makes very difficult to keep track what's going on.
So my preference here is to just disable the section if they exists.

> > [...]
> >
> > > 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.
Good point, I missed the .data.page_aligned.

Cheers,

--
Julien Grall

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