Re: [PATCH] ARM: add a private asm/unaligned.h

[Ard Biesheuvel]

On 1 November 2017 at 18:00, Russell King - ARM Linux
<linux@xxxxxxxxxxxxxxx> wrote:
> On Wed, Nov 01, 2017 at 03:57:36PM +0000, Ard Biesheuvel wrote:
>> On 31 October 2017 at 13:22, Gregory CLEMENT
>> <gregory.clement@xxxxxxxxxxxxxxxxxx> wrote:
>> > Hi Ard,
>> >
>> >  On mar., oct. 31 2017, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote:
>> >
>> >> On 31 October 2017 at 12:47, Russell King - ARM Linux
>> >> <linux@xxxxxxxxxxxxxxx> wrote:
>> >>> On Mon, Oct 30, 2017 at 04:38:17PM +0000, Russell King - ARM Linux wrote:
>> >>>> On Mon, Oct 30, 2017 at 05:24:34PM +0100, Gregory CLEMENT wrote:
>> >>>> > Hi Russell King,
>> >>>> >
>> >>>> > Here you will find all the objects included the vmlinux:
>> >>>> >
>> >>>> > http://free-electrons.com/~gregory/pub/compressed.tgz
>> >>>>
>> >>>> Thanks.  Unfortunately, nothing stands out, but I do see a difference
>> >>>> between the output of your linker from mine.
>> >>>>
>> >>>> Yours:
>> >>>>
>> >>>> Idx Name          Size      VMA       LMA       File off  Algn
>> >>>>   0 .text         00005ef8  00000000  00000000  00010000  2**5
>> >>>>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>> >>>>
>> >>>> Mine:
>> >>>>
>> >>>> Idx Name          Size      VMA       LMA       File off  Algn
>> >>>>   0 .text         00005f00  00000000  00000000  00010000  2**5
>> >>>>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>> >>>>
>> >>>> That has the effect of moving the addresses of the following
>> >>>> sections in your vmlinux down by 8 bytes.  However, I don't think
>> >>>> that's the cause of this - but it does hint at something being
>> >>>> different in binutils in the way sections are processed in the
>> >>>> linker.
>> >>>>
>> >>>> Please add to your linker script after the assignment of _edata:
>> >>>>
>> >>>>   .image_end (NOLOAD) : {
>> >>>>     _edata_foo = .;
>> >>>>   }
>> >>>>
>> >>>> relink the decompressor, and see what value _edata_foo ends up with
>> >>>> compared to _edata?  They should be the same, but I suspect using
>> >>>> your linker, they will be different.
>> >>>>
>> >>>> Also try adding
>> >>>>     BYTE(0);
>> >>>>
>> >>>> after the _edata_foo assignment as a separate test, and see whether
>> >>>> that makes any difference - with that you should end up with the
>> >>>> .image_end section in the output image.
>> >>>
>> >>> Gregory sent me has new url... for _both_ changes, which gives me:
>> >
>> > If needed I can provide this url.
>> >
>> >>>
>> >>> $ arm-linux-nm vmlinux |grep _edata
>> >>> 00491160 D _edata
>> >>> 00491160 D _edata_foo
>> >>>
>> >>> So there's no reason that ASSERT() should be failing!  However, as I
>> >>> don't have the intermediate step, I can't say whether the addition
>> >>> of the BYTE() affected it in some way - sorry, but I asked for _both_
>> >>> to be tested above because I wanted to speed up the process, and
>> >>> clearly that's backfired.
>> >>>
>> >>> Given how close we potentially are to 4.14, I don't think we're going
>> >>> to get to the bottom of this to make 4.14.  I'd want to get this
>> >>> sorted by Wednesday so linux-next (which is resuming this evening)
>> >>> can grab a copy of my tree with it in, and we have another day to
>> >>> sort out any remaining issues, but I'm basically out of time to do
>> >>> anything further with this as of now.
>> >>
>> >>> So, 4.14 will likely be released without any of this being fixed.
>> >>>
>> >>
>> >> IIUC, the current issue is limited to the ASSERT() itself, which is
>> >> there to prevent future regressions, while the other two patches deal
>> >> with severe and difficult to diagnose known issues.
>> >
>> > I confirm that whithout the last commit (adding the ASSERT()) in the
>> > fixes branch it worked well.
>> >
>> >>
>> >> So why can't we apply those two patches as fixes, and revisit the
>> >> patch that helps us prevent this from regressing in the future for
>> >> v4.15?
>> >
>> > I also agree with this.
>> >
>>
>> Russell,
>>
>> Please drop the EFI patch from your tree. I will forward it myself.
>
> Really, no, I'm not going to.  I've enough to do than chase around
> playing political games about who should send what patches.  You
> asked me to merge it, and I will merge it.
>

Fine, then merge it. I am not the one who is playing games here, I
just want to get stuff fixed. I don't understand why you are dragging
your feet like this.