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

Message-ID: <152908522534.16708.13094007571453293776@swboyd.mtv.corp.google.com>
Date:   Fri, 15 Jun 2018 10:53:45 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Marc Zyngier <marc.zyngier@....com>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        jason@...edaemon.net, sudeep.holla@....com, tglx@...utronix.de
Cc:     linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        rnayak@...eaurora.org, bjorn.andersson@...aro.org,
        nicolas.dechesne@...aro.org
Subject: Re: [RFC PATCH] irqchip/gic-v3: Add quirk for msm8996 secured registers

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.