linux-kernel - Re: [PATCH 2/4] efi/libstub: detect panel-id

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKv+Gu9pH9=AnNee7B-z0sP3rGJ-0Qnpziyx445M30KWC=Vq+w@mail.gmail.com>
Date:   Tue, 2 Jul 2019 23:53:16 +0200
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     Rob Clark <robdclark@...il.com>
Cc:     dri-devel <dri-devel@...ts.freedesktop.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        freedreno <freedreno@...ts.freedesktop.org>,
        aarch64-laptops@...ts.linaro.org,
        Rob Clark <robdclark@...omium.org>,
        Ingo Molnar <mingo@...nel.org>, Will Deacon <will@...nel.org>,
        Leif Lindholm <leif.lindholm@...aro.org>,
        Steve Capper <steve.capper@....com>,
        Lukas Wunner <lukas@...ner.de>,
        Julien Thierry <julien.thierry@....com>,
        linux-efi <linux-efi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/4] efi/libstub: detect panel-id

On Tue, 2 Jul 2019 at 23:02, Rob Clark <robdclark@...il.com> wrote:
>
> On Tue, Jul 2, 2019 at 1:35 PM Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote:
> >
> > On Tue, 2 Jul 2019 at 22:26, Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote:
> > >
> > > On Sun, 30 Jun 2019 at 22:36, Rob Clark <robdclark@...il.com> wrote:
> > > >
> > > > From: Rob Clark <robdclark@...omium.org>
> > > >
> > > > On snapdragon aarch64 laptops, a 'UEFIDisplayInfo' variable is provided
> > > > to communicate some information about the display.  Crutially it has the
> > > > panel-id, so the appropriate panel driver can be selected.  Read this
> > > > out and stash in /chosen/panel-id so that display driver can use it to
> > > > pick the appropriate panel.
> > > >
> > > > Signed-off-by: Rob Clark <robdclark@...omium.org>
> > >
> > > Hi Rob,
> > >
> > > I understand why you are doing this, but this *really* belongs elsewhere.
> > >
> > > So we are dealing with a platform that violates the UEFI spec, since
> > > it does not bother to implement variable services at runtime (because
> > > MS let the vendor get away with this).
> > >
> >
> > To clarify, the above remark applies to populating the DT from the OS
> > rather than from the firmware.
>
> yeah, it isn't pretty, but there *are* some other similar cases where
> efi-stub is populating DT.. (like update_fdt_memmap() and
> kaslr-seed)..
>

True, but those don't describe the hardware.

> it would be kinda nice to have an early-quirks mechanism where this
> could fit, but I thought that might be equally unpopular ;-)
>

Very :-)

> >
> > > First of all, to pass data between the EFI stub and the OS proper, we
> > > should use a configuration table rather than a DT property, since the
> > > former is ACPI/DT agnostic. Also, I'd like the consumer of the data to
> > > actually interpret it, i.e., just dump the whole opaque thing into a
> > > config table in the stub, and do the parsing in the OS proper.
> > >
> > > However, I am not thrilled at adding code to the stub that
> > > unconditionally looks for some variable with some magic name on all
> > > ARM/arm64 EFI systems, so this will need to live under a Kconfig
> > > option that depends on ARM64 (and does not default to y)
>
> I defn can add this under kconfig.. is it ok if that option is
> select'd by ARCH_QCOM?
>

I guess some mobile SOC/snapdragon symbol would be more appropriate,
but given that qcom left the server business, I guess it hardly
matters, so default y if ARM64 && ARCH_QCOM is fine with me

> (Just trying to minimize the things that can go wrong and the "I get a
> blank screen at boot" bug reports I get ;-))
>

Sure

> > ... but saving variables at boot time for consumption at runtime is
> > something that we will likely see more of in the future.
>
> I think this will be nice, but it also doesn't address the need for a
> quirk to get this into /chosen..  I guess we *could* use a shim or
> something that runs before the kernel to do this.  But that just seems
> like a logistical/support nightmare.  There is one kernel, and there
> are N distro's, so debugging a users "I don't get a screen at boot"
> problem because their distro missed some shim patch really just
> doesn't seem like a headache I want to have.
>

I'd argue that this does not belong in /chosen in the first place,
i.e., it doesn't belong in the DT at all if the OS can access the
config table (and therefore the variable) directly.