linux-kernel - Re: [RFC PATCH] irqchip/gic-v3: Add quirk for msm8996 secured registers

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] [day] [month] [year] [list]

Message-ID: <20180626202839.GI1860@tuxbook-pro>
Date:   Tue, 26 Jun 2018 13:28:39 -0700
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Stephen Boyd <sboyd@...nel.org>
Cc:     Marc Zyngier <marc.zyngier@....com>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        jason@...edaemon.net, sudeep.holla@....com, tglx@...utronix.de,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        rnayak@...eaurora.org, nicolas.dechesne@...aro.org
Subject: Re: [RFC PATCH] irqchip/gic-v3: Add quirk for msm8996 secured
 registers

On Fri 15 Jun 10:53 PDT 2018, Stephen Boyd wrote:

> Quoting Marc Zyngier (2018-06-15 01:16:02)
> > On 14/06/18 21:33, Stephen Boyd wrote:
> > > Quoting Srinivas Kandagatla (2018-06-14 10:54:43)
> > >>>
> > >>>> +{
> > >>>> +    struct gic_chip_data *d = data;
> > >>>> +
> > >>>> +    d->flags |= GICV3_FLAGS_WORKAROUND_IW_GICR_WAKER;
> > >>>> +
> > >>>> +    return true;
> > >>>> +}
> > >>>> +
> > >>>> +static const struct gic_quirk gicv3_quirks[] = {
> > >>>> +    {
> > >>>> +            .desc   = "GICV3: Qualcomm MSM8996 WAKER IW",
> > >>>
> > >>> Please the erratum number in the message. It should read something like:
> > >>>
> > >>>               "GICv3: Qualcomm erratum BIGNUMBERHERE"
> > >>>
> > >>>> +            .iidr   = 0x00001070,   /* MSM8996 */
> > >>>> +            .mask   = 0x0000ffff,
> > >>>
> > >>> Please match the full GICD_IIDR register, not just the implementer and
> > >>> the revision. Unless you expect all the QC systems to have the same
> > >>> behaviour?
> > >> There seems to be more than one SoC that has this issue,  I will dig up 
> > >> more info before sending next version.
> > >>
> > > 
> > > It depends on the firmware and if that firmware decides to block or
> > > allow access to this register space. I don't see how it can be quirked
> > > based on the IIDR at all because there could be different firmware on
> > > the board that doesn't block access to the register. Can a DT property
> > > work?
> > 
> > Are you saying that the IIDR doesn't isn't unique per implementation of
> > the firmware (which, as far as the kernel is concerned in this case,
> > implements the GIC)? That would be another erratum. If we did change the
> > behaviour of the vGIC in KVM, we'd certainly change the IIDR value! This
> > is the exact same case.
> 
> I don't know for certain. All I know is that we can't assume all QC
> systems have a firmware that brings the whole system down when you read
> the WAKER register. Hopefully Srini can find out if reads to IIDR are
> being trapped by the hypervisor and emulated as something different.
> 

I took another look at the internal documentation related to this change
and concluded that it was introduced for the "lead device" on the 8996
platform. As such I expect that all publicly available 8996 devices have
this implementation.

Looking through some relevant platforms it seems like it's only 8996
that that sports a GICv3 with an IIDR of 0x1070 (e.g. 8994 has a QGICv2
with the same IIDR...), so the quirk seems reasonable in scope.

Regards,
Bjorn