Re: [PATCH v4] EDAC: Add ARM64 EDAC

[Brijesh Singh]

Hi,

>> I have looked at possibility of pushing correctable error logging in the
>> firmware; but given current hardware limitation it seems like OS is the best
>> place to implement it. Let me summaries the issues we are running into:
>>
>> * Correctable errors does not generate any interrupt:
>>   If we have to implement error parsing inside the firmware then work need
>>   to be split between OS and firmware. Maybe OS can call SMC instruction to 
>>   dial into firmware and then firmware can check error syndrome registers; 
>>   if it finds correctable error then build HEST table. This method will introduce
>>   performance issue because it require OS executing SMC every 100ms or so to just
>>   poll for correctable error. If you have any other recommendation then please share it.
> 
> I agree that this is a problem, and is an unfortunate hardware
> limitation.
> 
> I am still wary of making use of IMPLEMENTATION DEFINED features like
> this in the kernel.
>
I do see the reason behind shy away from IMPLEMENTATION DEFINED features
but to support L1/L2 correctable error feature we have limited choices:
- use of IMPLEMENTATION DEFINED register in the kernel
   or
- go with SMC kind of method explained above which causes performance hit
  (which also means platform specific driver)
  or
- don't implement it ;)

> 
> You won't have the HEST and DT information at the same time, given that
> at runtime the kernel uses one of ACPI or DT.
> 
> This also doesn't define the affinity of interrupts (i.e. which
> cluster/CPU does each interrupt get generated by), which is the more
> important part.
> 

It seems like we might need to describe
- cluster topology
- which cpu have the feature
- interrupt affinity
e.g
edac {
    cluster0 {
        core0 { cpu = <&cpu_0>; }
	core1 { cpu = <&cpu_1>; }
    }

    cluster1 {
	 core0 { cpu = <&cpu_2>; }
	 core1 { cpu = <&cpu_3>; }
    }

    ..............
    ..............
    num-interrupts = 2;
    interrupts  = <0, 92, 0> <0, 94, 0> ;
    interrupt-affinity = <&cluster0> <&cluster1>;    
}

Is it possible to avoid all these DT entries and do something at runtime ?
e.g we know for sure that fatal interrupts are generate per cluster.

> What about correctable errors? What if someone wants to poll that in FW
> (no matter how horrible that may be for performance)? What if a future
> CPU revision adds an optional interrupt for that?
> 
If FW handles the correctable error then DT should not contain the edac entry
and driver will not be probed. This also raises question on how we are going
to load the driver in ACPI case ?


Thanks
Brijesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/