From: Ard Biesheuvel <firstname.lastname@example.org> To: Sasha Levin <email@example.com>, Marc Zyngier <firstname.lastname@example.org> Cc: Linux Kernel Mailing List <email@example.com>, stable <firstname.lastname@example.org>, Jeremy Linton <email@example.com>, linux-efi <firstname.lastname@example.org> Subject: Re: [PATCH AUTOSEL 4.19 133/191] efi: honour memory reservations passed via a linux specific config table Date: Sun, 10 Nov 2019 16:26:15 +0000 Message-ID: <CAKv+Gu_uGKKQrkvHXhoKCY6dqEWka6qVpjXBit5Ggjbk6+_c7A@mail.gmail.com> (raw) In-Reply-To: <20191110155655.GO4787@sasha-vm> (adding Marc, the GIC maintainer) On Sun, 10 Nov 2019 at 15:57, Sasha Levin <email@example.com> wrote: > > On Sun, Nov 10, 2019 at 02:16:57PM +0000, Ard Biesheuvel wrote: > >On Sun, 10 Nov 2019 at 13:27, Sasha Levin <firstname.lastname@example.org> wrote: > >> > >> On Sun, Nov 10, 2019 at 08:33:47AM +0100, Ard Biesheuvel wrote: > >> >On Sun, 10 Nov 2019 at 03:44, Sasha Levin <email@example.com> wrote: > >> >> > >> >> From: Ard Biesheuvel <firstname.lastname@example.org> > >> >> > >> >> [ Upstream commit 71e0940d52e107748b270213a01d3b1546657d74 ] > >> >> > >> >> In order to allow the OS to reserve memory persistently across a > >> >> kexec, introduce a Linux-specific UEFI configuration table that > >> >> points to the head of a linked list in memory, allowing each kernel > >> >> to add list items describing memory regions that the next kernel > >> >> should treat as reserved. > >> >> > >> >> This is useful, e.g., for GICv3 based ARM systems that cannot disable > >> >> DMA access to the LPI tables, forcing them to reuse the same memory > >> >> region again after a kexec reboot. > >> >> > >> >> Tested-by: Jeremy Linton <email@example.com> > >> >> Signed-off-by: Ard Biesheuvel <firstname.lastname@example.org> > >> >> Signed-off-by: Sasha Levin <email@example.com> > >> > > >> >NAK > >> > > >> >This doesn't belong in -stable, and I'd be interested in understanding > >> >how this got autoselected, and how I can prevent this from happening > >> >again in the future. > >> > >> It was selected because it's part of a fix for a real issue reported by > >> users: > >> > > > >For my understanding, are you saying your AI is reading launchpad bug > >reports etc? Because it is marked AUTOSEL. > > Not quite. This review set was me feeding all the patches Ubuntu has on > top of stable trees into AUTOSEL, and sending out the output for review. > I doesn't look into launchpad bug reports on it's own, but in my > experience one can find a bug report for mostly everything AUTOSEL > considers to be a bug. > So the assumption is that taking an arbitrary subset of what Ubuntu backported (and tested extensively), and letting that subset be chosen by a bot is a process that improves the quality of stable trees? I'm rather skeptical of that tbh. > >> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1806766 > >> > > > >That pages mentions > > > >""" > >2 upstream patch series are required to fix this: > > https://<email address hidden>/msg10328.html > >Which provides an EFI facility consumed by: > > https://lkml.org/lkml/2018/9/21/1066 > >There were also some follow-on fixes to deal with ARM-specific > >problems associated with this usage: > > https://www.spinics.net/lists/arm-kernel/msg685751.html > >""" > > > >and without the other patches, we only add bugs and don't fix any. > > > >> Besides ubuntu, it is also carried by: > >> > >> SUSE: https://www.suse.com/support/update/announcement/2019/suse-su-20191530-1/ > >> CentOS: https://koji.mbox.centos.org/koji/buildinfo?buildID=4558 > >> > >> As a way to resolve the reported bug. > >> > > > >Backporting a feature/fix like this requires careful consideration of > >the patches involved, and doing actual testing on hardware. > > > >> Any reason this *shouldn't* be in stable? > > > >Yes. By itself, it causes crashes at early boot and does not actually > >solve the problem. > > Sure, let's work on gathering all the needed patches then and testing it > out. > No, let's not. This is a feature that was introduced to address a shortcoming in some hardware that makes kexec/kdump problematic on them. If you want kexec/kdump on that hardware, use a newer kernel. > >> I'm aware that there might be > >> dependencies that are not obvious to me, but the solution here is to > >> take those dependencies as well rather than ignore the process > >> completely. > >> > > > >This is not a bugfix. kexec/kdump never worked correctly on the > >hardware involved, and backporting a feature like that goes way beyond > >what I am willing to accept for stable backports affecting the EFI > >subsystem. > > I'm a bit confused. The bug report starts with: > > [Impact] > kdump support isn't usable on HiSilicon D05 systems. This > previously worked in bionic. > > So it seems like it did use to work, but not anymore? > I have no idea what Ubuntu shipped in the previous kernel, but labelling this as a software regression is dubious at least, and wholly inaccurate for upstream. > Either way, I understand that you want to keep the stable tree > conservative, but keep in mind that the flip side of not taking fixes > that users ask for means that distros end up having to carry them > anyway, which means that they don't get the review and testing they > need. > I'd say it is the opposite. At least the distros test their backports on actual hardware. Taking any part of this set without testing it by doing kexec/kdump on an affected ARM system, and regression testing it on the hardware that got broken by it (with hundreds of cores IIRC) is totally irresponsible, and I don't have the time or the hardware to do the testing. > We can argue all we want around whether it's a fix or not, but if most > distros carry it then I think our argument is moot. > If someone cares enough to backport these as a coherent set, with boot tests on the affected hardware etc, then I am not going to object.
next prev parent reply index Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <firstname.lastname@example.org> 2019-11-10 2:39 ` Sasha Levin 2019-11-10 7:33 ` Ard Biesheuvel 2019-11-10 13:27 ` Sasha Levin 2019-11-10 14:16 ` Ard Biesheuvel 2019-11-10 15:56 ` Sasha Levin 2019-11-10 16:26 ` Ard Biesheuvel [this message] 2019-11-10 18:08 ` Marc Zyngier 2019-11-10 2:39 ` [PATCH AUTOSEL 4.19 134/191] efi: Make efi_rts_work accessible to efi page fault handler Sasha Levin 2019-11-10 7:35 ` Ard Biesheuvel 2019-11-10 2:39 ` [PATCH AUTOSEL 4.19 135/191] efi/x86: Handle page faults occurring while running EFI runtime services Sasha Levin 2019-11-10 7:40 ` Ard Biesheuvel 2019-11-10 2:40 ` [PATCH AUTOSEL 4.19 191/191] x86/efi: fix a -Wtype-limits compilation warning Sasha Levin
Reply instructions: You may reply publically to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAKv+Gu_uGKKQrkvHXhoKCY6dqEWka6qVpjXBit5Ggjbk6+_c7A@mail.gmail.com \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Linux-EFI Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-efi/0 linux-efi/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-efi linux-efi/ https://lore.kernel.org/linux-efi \ firstname.lastname@example.org public-inbox-index linux-efi Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-efi AGPL code for this site: git clone https://public-inbox.org/public-inbox.git