From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1aZIpW-0005UL-32 for mharc-grub-devel@gnu.org; Fri, 26 Feb 2016 08:53:14 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43015) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aZIpR-0005Oz-6V for grub-devel@gnu.org; Fri, 26 Feb 2016 08:53:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aZIpP-0006g9-KN for grub-devel@gnu.org; Fri, 26 Feb 2016 08:53:09 -0500 Received: from mail-wm0-x231.google.com ([2a00:1450:400c:c09::231]:38261) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aZIpP-0006g5-A7 for grub-devel@gnu.org; Fri, 26 Feb 2016 08:53:07 -0500 Received: by mail-wm0-x231.google.com with SMTP id a4so70846228wme.1 for ; Fri, 26 Feb 2016 05:53:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=jIZ5BIurCC1sQ97rtMmB3Zffr+xYS/NQaPLb02reYG8=; b=s8neGnT3YsrT6Q05vkZrI0Y9QW/o/niEz/ez5jvC4kK/Ibdh804d8dr+aG26ZBRqmF hS35pSR1O1fUOl8qsVvJa1fJlSQs20Tz4sSV+ky22ys0/ZdbP3ggtlM5E+mTiCsM541T 20klLGbQDZefpFvrvrq0f0ju/H+i9znoyKkx24uCGpvtw+YHkEey2PgTEXxJnssk8lHc QNl/5PznoFn8fHdgfzNIH8XIzO7GGOgh1I49VrTgwYzFzqTX646zTiTtgsTVYVrBytLy cUWS/8vn/gt7xqxLffnH5mLcgaK4sWMjp6Eic3pWLYfBxJsusHMG9iqWjXxbcV1uKFkd g09g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=jIZ5BIurCC1sQ97rtMmB3Zffr+xYS/NQaPLb02reYG8=; b=JjjFj8CqkQebQ3aQUdH5jZorsOZKFKvEcmMyANmlAAPGH8W7fAeE0VcG4ZxOdDd5DU bt9rKszBDtcrOTUtbOePZMuzNBWfQh9DcvHAO7JretO+5sdw6ZlKHZWI87M9ZSQq0ww5 xKZs3zpBA5RX4eT0ty7zhXIiYyEdi/WlQHskuh2squkDevaIy5BSejmO/CyvjMyw5BAh N5bI9EvEoaxC+4+PvjiDFUkIcbRucmWYjFl7AfY9n5OohMF+Osy84MswzLk1WqZwIu+N As96aiP6/TPhHX0X8KRW2p3QYo+F2CJefawmUJq1IP/37kKAmqdNYGCpttSSFGaN60l+ 6miw== X-Gm-Message-State: AD7BkJL8639SwGWKUTYBk1+Z+L5X/Is4PkMHb7UIO+mb+x7GRvS2nipFjWfNPV5OxR3YZhbNR0APBjht01t0rA== X-Received: by 10.28.111.91 with SMTP id k88mr3142989wmc.86.1456494786525; Fri, 26 Feb 2016 05:53:06 -0800 (PST) MIME-Version: 1.0 References: <1455898714-25127-1-git-send-email-leif.lindholm@linaro.org> <1455898714-25127-2-git-send-email-leif.lindholm@linaro.org> <56CABF64.7020003@gmail.com> <20160222140255.GO1159@bivouac.eciton.net> <56CB3DB9.4050102@gmail.com> <20160224115921.GU1159@bivouac.eciton.net> <20160224135738.GV1159@bivouac.eciton.net> <56CDEB27.6060606@gmail.com> <56CDFEB2.5010104@gmail.com> In-Reply-To: From: "Vladimir 'phcoder' Serbinenko" Date: Fri, 26 Feb 2016 13:52:56 +0000 Message-ID: Subject: Re: [PATCH 1/2] disk: Add support for device-specific malloc function To: The development of GNU GRUB Content-Type: multipart/alternative; boundary=001a1146978e5e4058052cac9e7a X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:400c:c09::231 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Feb 2016 13:53:11 -0000 --001a1146978e5e4058052cac9e7a Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Le ven. 26 f=C3=A9vr. 2016 14:48, Andrei Borzenkov a =C3=A9crit : > On Fri, Feb 26, 2016 at 4:27 PM, Vladimir 'phcoder' Serbinenko > wrote: > > Andrei, your patch is pretty complicated and would be a subject to > putting > > into next branch because of its breakage potential. Issue at hand affec= ts > > x86 as well. Can we split the solution from radical rework of disk.c wi= th > > optimisations? > > > > Then we need to add additional small read to for unaligned beginning > of user supplied buffer in main agglomeration loop. It is doable I > think. > It's not. If you try, then the remaining read is not sector-aligned. I like Leif's approach: fix small reads and do copying if necessary. This fixes most of problems and lets us fix others whenever we feel to. It also allows us to easier use similar approach for DMA drivers. I'm thinking of having some code to reuse user-supplied buffer for DMA when possible > > > Le mer. 24 f=C3=A9vr. 2016 20:04, Andrei Borzenkov a > =C3=A9crit > > : > >> > >> 24.02.2016 20:40, Andrei Borzenkov =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > >> > 24.02.2016 16:57, Leif Lindholm =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > >> >> On Wed, Feb 24, 2016 at 03:09:13PM +0300, Andrei Borzenkov wrote: > >> >>>>> Could you test attached patch with your alignment fixes on top. > This > >> >>>>> implements my idea of using shared buffers. Seems to work in nai= ve > >> >>>>> testing. > >> >>>> > >> >>>> Testing this with a grub-mkstandalone image, I get: > >> >>>> > >> >>>> kern/dl.c:556: flushing 0x10f1 bytes at 0x9ffb5ac20 > >> >>>> kern/dl.c:649: module name: tar > >> >>>> kern/dl.c:650: init function: 0x9ffb5b220 > >> >>>> kern/disk.c:217: Opening `memdisk'... > >> >>>> kern/fs.c:56: Detecting tarfs... > >> >>>> > >> >>>> And then spectacular crash in UEFI due to an EL2 translation faul= t. > >> >>> > >> >>> To be sure - is it with my patch alone or with your patches? If so= me > >> >>> more patches are used - could you send exact diff to trunk to avoi= d > >> >>> misunderstanding? > >> >> > >> >> Double checked with only your patch on top of > >> >> 1b782e902e69516f82158203674d4951a40c82d4 (previously running with > >> >> _only_ my alignment fixup in efidisk.c). Same behaviour. > >> > > >> > I cannot reproduce it on x86_64 (also with mm-debug enabled) and I d= o > >> > not know how to load standalone image on ppc; is it possible to use > QEMU > >> > to run ARM64 (I assume you have it)? If not what are options to test > it? > >> > > >> > Anyway, there was one problem I fixed later (although I did not get > any > >> > issue before as well), I attach updated version. Thank you for > testing! > >> > > >> > >> I still cannot reproduce it with either patch version using current GI= T > >> QEMU + binary QEMU_EFI.fd from > >> > >> > http://snapshots.linaro.org/components/kernel/leg-virt-tianocore-edk2-ups= tream/554/QEMU-AARCH64/RELEASE_GCC49/QEMU_EFI.fd > ; > >> I run it as > >> > >> aarch64-softmmu/qemu-system-aarch64 -m 1024 -cpu cortex-a57 -M virt > >> -bios ~/vm/QEMU_EFI.fd -cdrom /tmp/grub.iso -serial stdio > >> > >> Where grub.iso is built > >> > >> pkgdatadir=3D$PWD ./grub-mkstandalone -d grub-core -O arm64-efi -o > >> /tmp/grub.efi > >> pkgdatadir=3D$PWD ./grub-mkrescue -d grub-core -o /tmp/grub.iso > >> /grub.efi=3D/tmp/grub.efi > >> > >> and I do > >> > >> chainloader /grub.efi > >> boot > >> > >> after that. > >> > >> _______________________________________________ > >> Grub-devel mailing list > >> Grub-devel@gnu.org > >> https://lists.gnu.org/mailman/listinfo/grub-devel > > > > > > _______________________________________________ > > Grub-devel mailing list > > Grub-devel@gnu.org > > https://lists.gnu.org/mailman/listinfo/grub-devel > > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel > --001a1146978e5e4058052cac9e7a Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

Le ven. 26 f=C3=A9vr. 2= 016 14:48, Andrei Borzenkov <arvi= djaar@gmail.com> a =C3=A9crit=C2=A0:
On Fri, Feb 26, 2016 at 4:27 PM, Vladimir 'phcoder' Serbin= enko
<phcoder@gmail.co= m> wrote:
> Andrei, your patch is pretty complicated and would be a subject to put= ting
> into next branch because of its breakage potential. Issue at hand affe= cts
> x86 as well. Can we split the solution from radical rework of disk.c w= ith
> optimisations?
>

Then we need to add additional small read to for unaligned beginning
of user supplied buffer in main agglomeration loop. It is doable I
think.
It's not. If you try, then the remain= ing read is not sector-aligned. I like Leif's approach: fix small reads= and do copying if necessary. This fixes most of problems and lets us fix o= thers whenever we feel to. It also allows us to easier use similar approach= for DMA drivers. I'm thinking of having some code to reuse user-suppli= ed buffer for DMA when possible

> Le mer. 24 f=C3=A9vr. 2016 20:04, Andrei Borzenkov <arvidjaar@gmail.com> a =C3= =A9crit
> :
>>
>> 24.02.2016 20:40, Andrei Borzenkov =D0=BF=D0=B8=D1=88=D0=B5=D1=82:=
>> > 24.02.2016 16:57, Leif Lindholm =D0=BF=D0=B8=D1=88=D0=B5=D1= =82:
>> >> On Wed, Feb 24, 2016 at 03:09:13PM +0300, Andrei Borzenko= v wrote:
>> >>>>> Could you test attached patch with your align= ment fixes on top. This
>> >>>>> implements my idea of using shared buffers. S= eems to work in naive
>> >>>>> testing.
>> >>>>
>> >>>> Testing this with a grub-mkstandalone image, I ge= t:
>> >>>>
>> >>>> kern/dl.c:556: flushing 0x10f1 bytes at 0x9ffb5ac= 20
>> >>>> kern/dl.c:649: module name: tar
>> >>>> kern/dl.c:650: init function: 0x9ffb5b220
>> >>>> kern/disk.c:217: Opening `memdisk'...
>> >>>> kern/fs.c:56: Detecting tarfs...
>> >>>>
>> >>>> And then spectacular crash in UEFI due to an EL2 = translation fault.
>> >>>
>> >>> To be sure - is it with my patch alone or with your p= atches? If some
>> >>> more patches are used - could you send exact diff to = trunk to avoid
>> >>> misunderstanding?
>> >>
>> >> Double checked with only your patch on top of
>> >> 1b782e902e69516f82158203674d4951a40c82d4 (previously runn= ing with
>> >> _only_ my alignment fixup in efidisk.c). Same behaviour.<= br> >> >
>> > I cannot reproduce it on x86_64 (also with mm-debug enabled) = and I do
>> > not know how to load standalone image on ppc; is it possible = to use QEMU
>> > to run ARM64 (I assume you have it)? If not what are options = to test it?
>> >
>> > Anyway, there was one problem I fixed later (although I did n= ot get any
>> > issue before as well), I attach updated version. Thank you fo= r testing!
>> >
>>
>> I still cannot reproduce it with either patch version using curren= t GIT
>> QEMU + binary QEMU_EFI.fd from
>>
>> http://snapshots.linaro.org/components/kernel= /leg-virt-tianocore-edk2-upstream/554/QEMU-AARCH64/RELEASE_GCC49/QEMU_EFI.f= d;
>> I run it as
>>
>> aarch64-softmmu/qemu-system-aarch64=C2=A0 -m 1024 -cpu cortex-a57 = -M virt
>> -bios ~/vm/QEMU_EFI.fd -cdrom /tmp/grub.iso -serial stdio
>>
>> Where grub.iso is built
>>
>> pkgdatadir=3D$PWD ./grub-mkstandalone -d grub-core -O arm64-efi -o=
>> /tmp/grub.efi
>> pkgdatadir=3D$PWD ./grub-mkrescue -d grub-core -o /tmp/grub.iso >> /grub.efi=3D/tmp/grub.efi
>>
>> and I do
>>
>> chainloader /grub.efi
>> boot
>>
>> after that.
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel= @gnu.org
>> https://lists.gnu.org/mailman/listinfo/gr= ub-devel
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu= .org
> https://lists.gnu.org/mailman/listinfo/grub-de= vel
>

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org<= /a>
https://lists.gnu.org/mailman/listinfo/grub-devel
--001a1146978e5e4058052cac9e7a--