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

[Marc Zyngier <marc.zyngier@arm.com>]

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...