[RFC] Support different block names with same EDAC device

[RFC] Support different block names with same EDAC device

[Hawa, Hanna]

Hi,

Amazon Alpine SoCs have different HW units that have an ability to 
report on correctable/uncorrectable errors. We want to add support for 
this HWs using dedicated EDAC driver based on edac device subsystem.
In edac_device_ctl_info() function one of the parameters is 
“edac_block_name”, to create nr_blocks kobjects based on that single 
block name.
Our target to create multiple block names with the same edac device, e.g.:
/sys/devices/system/edac/MY_HW_UNIT/error_name_a/
/sys/devices/system/edac/MY_HW_UNIT/error_name_b/
/sys/devices/system/edac/MY_HW_UNIT/error_name_c/
/sys/devices/system/edac/MY_HW_UNIT/error_name_a/

Can the edac device infrastructure support different block names? Shall 
we add support for such feature?

Thanks,
Hanna

Re: [RFC] Support different block names with same EDAC device

[Borislav Petkov]

On Mon, May 11, 2020 at 03:59:38PM +0300, Hawa, Hanna wrote:
> Can the edac device infrastructure support different block names? Shall we
> add support for such feature?

Does the "EDAC Blocks" section here:

Documentation/driver-api/edac.rst

answer your question?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [RFC] Support different block names with same EDAC device

[Hawa, Hanna]

On 5/11/2020 4:55 PM, Borislav Petkov wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> On Mon, May 11, 2020 at 03:59:38PM +0300, Hawa, Hanna wrote:
>> Can the edac device infrastructure support different block names? Shall we
>> add support for such feature?
> 
> Does the "EDAC Blocks" section here:
> 
> Documentation/driver-api/edac.rst
> 
> answer your question?

Hi Boris,

Thanks for your quick answer, according to the section you point to, the 
block names shall be different e.g. L1-cache/L2-cache. But when 
initializing the sysfs block names, the block index appends the 
block-name [1].

I'm trying to create different block names e.g. hw_block_a, hw_block_b.

[1]: 
https://elixir.bootlin.com/linux/v5.7-rc5/source/drivers/edac/edac_device.c#L161

> 
> --
> Regards/Gruss,
>      Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette
> 

Re: [RFC] Support different block names with same EDAC device

[Borislav Petkov]

On Mon, May 11, 2020 at 05:21:14PM +0300, Hawa, Hanna wrote:
> I'm trying to create different block names e.g. hw_block_a, hw_block_b.

Please give an actual example of the names you're trying to add and how
the hierarchy you think should look like.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [RFC] Support different block names with same EDAC device

[Hawa, Hanna]

On 5/11/2020 5:26 PM, Borislav Petkov wrote:
> On Mon, May 11, 2020 at 05:21:14PM +0300, Hawa, Hanna wrote:
>> I'm trying to create different block names e.g. hw_block_a, hw_block_b.
> 
> Please give an actual example of the names you're trying to add and how
> the hierarchy you think should look like.

For example PCIe controller interface, that can report on different 
errors, e.g. axi read parity error, axi write parity error, completion 
timeout, write overflow error, and parity error.

Then I think the hierarchy should look:

pcie/pcie0/..
           /axi_write_parity_error/ce_count
                                  /ue_count
           /axi_read_parity_error/ce_count
                                 /ue_count
           /completion_timeout/ce_count
                              /ue_count
           /write_overflow_error/ce_count
                                /ue_count
           /parity_error/ce_count
                        /ue_count

same for pcie1/pcie2

Thanks,
Hanna

> 
> --
> Regards/Gruss,
>      Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette
> 

Re: [RFC] Support different block names with same EDAC device

[Borislav Petkov]

On Tue, May 12, 2020 at 11:47:06AM +0300, Hawa, Hanna wrote:
> pcie/pcie0/..
>           /axi_write_parity_error/ce_count
>                                  /ue_count
>           /axi_read_parity_error/ce_count
>                                 /ue_count
>           /completion_timeout/ce_count
>                              /ue_count
>           /write_overflow_error/ce_count
>                                /ue_count
>           /parity_error/ce_count
>                        /ue_count
> 
> same for pcie1/pcie2

The "...pci" layout on my box looks like this:

$ tree /sys/devices/system/edac/pci/
/sys/devices/system/edac/pci/
├── check_pci_errors
├── edac_pci_log_npe
├── edac_pci_log_pe
├── edac_pci_panic_on_pe
├── pci0
│   ├── device -> ../../../../pci0000:00/0000:00:18.0
│   ├── npe_count
│   └── pe_count
├── pci_nonparity_count
└── pci_parity_count

so what's wrong with having "pcie0, pcie1, pcie2, ..." for those
different functional units?

I guess you could add a "name" node so that you have:

pcie
|-> pcie0
    |-> name:	axi_write_parity_error
    |-> ce_count
    |-> ue_count

and so on so that tools can know what those FUs are.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [RFC] Support different block names with same EDAC device

[Hawa, Hanna]

On 5/12/2020 1:35 PM, Borislav Petkov wrote:
> The "...pci" layout on my box looks like this:
> 
> $ tree/sys/devices/system/edac/pci/
> /sys/devices/system/edac/pci/
> ├── check_pci_errors
> ├── edac_pci_log_npe
> ├── edac_pci_log_pe
> ├── edac_pci_panic_on_pe
> ├── pci0
> │   ├── device -> ../../../../pci0000:00/0000:00:18.0
> │   ├── npe_count
> │   └── pe_count
> ├── pci_nonparity_count
> └── pci_parity_count
> 
> so what's wrong with having "pcie0, pcie1, pcie2, ..." for those
> different functional units?

The pci layout on EDAC supports counters for parity/non parity errors. 
We are searching for something more general. To add more error for PCIe 
layout, to have counters, and to control panic flag per error.

> 
> I guess you could add a "name" node so that you have:
> 
> pcie
> |-> pcie0
>      |-> name:   axi_write_parity_error
>      |-> ce_count
>      |-> ue_count
> 
> and so on so that tools can know what those FUs are.

Didn't get your suggestion, where can I add name?

Thanks,
Hanna

Re: [RFC] Support different block names with same EDAC device

[Borislav Petkov]

On Tue, May 12, 2020 at 10:27:07PM +0300, Hawa, Hanna wrote:
> The pci layout on EDAC supports counters for parity/non parity errors. We
> are searching for something more general. To add more error for PCIe layout,
> to have counters, and to control panic flag per error.

Looking at edac_device_alloc_ctl_info(), reading the doc section I
pointed you at earlier and then maybe extending that function - if
needed - is what I'd do. After having looked how the others call it.

But you cannot break the existing drivers - adding new
functionality/sysfs nodes etc should be fine, of course.

> Didn't get your suggestion, where can I add name?

See above.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette