Re: [Xen-devel] [PATCH v4 12/16] bug/x86/arm: Align bug_frames sections.

[Julien Grall <julien.grall@xxxxxxx>]

On 19/09/2016 16:34, Julien Grall wrote:
> Hi Konrad,
>
> On 16/09/2016 18:38, Konrad Rzeszutek Wilk wrote:
> > Most of the WARN_ON or BUG_ON sections are properly aligned on
> > x86. However on ARM and on x86 assembler the macros don't include
> > any aligment information - hence they end up being the default
> s/aligment/alignment/
>
> > byte granularity.
> >
> > On ARM32 it is paramount that the aligment is word-size (4)
> Ditto
>
> > otherwise if one tries to use (uint32_t*) access (such
> > as livepatch ELF relocations) we get a Data Abort.
> >
> > Enforcing bug_frames to have the proper aligment across all
> Ditto
>
> > architectures and in both C and x86 makes them all the same.
> >
> > Furthermore on x86 the bloat-o-meter detects that with this
> > change:
> >
> > add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
> > function                                     old     new   delta
> >
> > On ARM32:
> > add/remove: 1/0 grow/shrink: 0/1 up/down: 384/-288 (96)
> > function                                     old     new   delta
> > gnttab_unpopulate_status_frames                -     384    +384
> > do_grant_table_op                          10808   10520    -288
> >
> > And ARM64:
> > add/remove: 1/2 grow/shrink: 0/1 up/down: 4164/-4236 (-72)
> > function                                     old     new   delta
> > gnttab_map_grant_ref                           -    4164   +4164
> > do_grant_table_op                           9892    9836     -56
> > grant_map_exists                             300       -    -300
> > __gnttab_map_grant_ref                      3880       -   -3880
> >
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
I forgot to mention that with those NITs fixed:

Reviewed-by: Julien Grall <julien.grall@xxxxxxx>

> > ---
> > Cc: Julien Grall <julien.grall@xxxxxxx>
> > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >
> > v3: First submission. Replaces the "livepatch/elf: Adjust section
> > aligment to word"
> >     patch.
> > v4: Remove the .ALIGN(4) in xen.lds.S for x86 (the only place needing
> >     that change).
> >     Update the commit description with correct x86 results
> >     Remove the . = ALIGN(4); in linker filer on x86 [the only file
> > needing the change]
> > ---
> >  xen/arch/x86/xen.lds.S    | 1 -
> >  xen/include/asm-arm/bug.h | 1 +
> >  xen/include/asm-x86/bug.h | 1 +
> >  3 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> > index d903c31..7676de9 100644
> > --- a/xen/arch/x86/xen.lds.S
> > +++ b/xen/arch/x86/xen.lds.S
> > @@ -79,7 +79,6 @@ SECTIONS
> >    .rodata : {
> >         _srodata = .;
> >         /* Bug frames table */
> > -       . = ALIGN(4);
> >         __start_bug_frames = .;
> >         *(.bug_frames.0)
> >         __stop_bug_frames_0 = .;
> > diff --git a/xen/include/asm-arm/bug.h b/xen/include/asm-arm/bug.h
> > index 68353e1..773d63e 100644
> > --- a/xen/include/asm-arm/bug.h
> > +++ b/xen/include/asm-arm/bug.h
> > @@ -52,6 +52,7 @@ struct bug_frame {
> >
> > ".popsection\n"                                                    \
> >           ".pushsection .bug_frames." __stringify(type) ", \"a\",
> > %progbits\n"\
> >
> > "4:\n"                                                             \
> > +         ".align
> > 4\n"                                                       \
> >           ".long (1b -
> > 4b)\n"                                                \
> >           ".long (2b -
> > 4b)\n"                                                \
> >           ".long (3b -
> > 4b)\n"                                                \
> > diff --git a/xen/include/asm-x86/bug.h b/xen/include/asm-x86/bug.h
> > index c5d2d4c..9bb4a19 100644
> > --- a/xen/include/asm-x86/bug.h
> > +++ b/xen/include/asm-x86/bug.h
> > @@ -98,6 +98,7 @@ extern const struct bug_frame __start_bug_frames[],
> >      .popsection
> >
> >      .pushsection .bug_frames.\type, "a", @progbits
> > +        .p2align 2
> >          .L\@bf:
> >          .long (.L\@ud - .L\@bf) + \
> >                 ((\line >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)
> Regards,
--
Julien Grall

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