On Wed, Sep 08, 2021 at 08:41:46PM +0200, Borislav Petkov wrote:

On Wed, Sep 01, 2021 at 06:42:26PM +0000, Yazen Ghannam wrote:

quoted

err.channel still needs to be used in error_address_to_page_and_offset()
below.

I think you mean __log_ecc_error().

Yep, you're right.
 

quoted

This is a good idea. But we have a global *fam_type, so this should be moved
into struct amd64_pvt, if possible. Then each node can have its own fam_type.

per_family_init() does assign stuff to pvt members so yes, we're saying
the same thing, practically.

quoted

Fair point. I like the idea of having unique names though. Is this possible
with the current EDAC framework? Or is it not worth it?

We don't have unique names for the CPU nodes:

[   25.637486] EDAC MC0: Giving out device to module amd64_edac controller F17h_M30h: DEV 0000:00:18.3 (INTERRUPT)
[   25.799554] EDAC MC1: Giving out device to module amd64_edac controller F17h_M30h: DEV 0000:00:19.3 (INTERRUPT)

why does it matter to have unique names for the accelerators?

If you wanna differentiate them, you can dump the PCI devs like above.

Just to make it clear - I'm not against it per-se - I'd just need a
stronger justification for doing this than just "I like the idea".

There isn't a strong reason at the moment. I think it may be one less hurdle
for users to go through when identifying a device. But, as you said, there are
other ways to differentiate devices.

Thanks,
Yazen