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

[Stephen Boyd]

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.