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: <61229c8f-7de0-a798-5af4-ba5e0ea78001@arm.com>
Date:   Fri, 15 Jun 2018 09:16:02 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     Stephen Boyd <sboyd@...nel.org>,
        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

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.

Whoever thought this was a good idea shouldn't be allowed near a
compiler. They clearly are clueless.

As for a DT property, I can't see how this allows us to move forward.
Firmware could get updated at any point, and the DT would be just as wrong.

	M.
-- 
Jazz is not dead. It just smells funny...