Messages in this thread | | | Date | Wed, 5 Jan 2022 16:12:08 +0000 | From | Yazen Ghannam <> | Subject | Re: [PATCH v2 1/2] EDAC/amd64: Check register values from all UMCs |
| |
On Thu, Dec 30, 2021 at 12:36:44PM +0100, Borislav Petkov wrote: > On Thu, Dec 16, 2021 at 04:08:18PM +0000, Yazen Ghannam wrote: > > No, that's a good question. And actually the assumption is incorrect. It is > > allowed to have different DIMM types in a system though all DIMMs on a single > > UMC must match. > > Oh fun, really?! > > So a single system can have DDR4 *and* DDR5 on the same board?! >
Well, I don't know about that specifically. There are some restrictions, but you could have UDIMMs and RDIMMs of the same generation, at least.
> So then that > > pvt->dram_type > > is insufficient to store the DIMM type for a pvt. If you have multiple > UMCs on a pvt and all have different type DIMMs, then you need the > relevant DIMM type when you dump it in sysfs... > > Which then means, you need ->dram_type to be per UMC... > > And also, I'm assuming the hw already enforces that DIMMs on a single > UMC must match - it simply won't boot if they don't so you don't have to > enforce that, at least. > > > Do you recommend a follow up patch or should this one be reworked? > > This one is insufficient, I'm afraid. > > One way to address this is, you could use pvt->umc at the places where > dram_type is used and assign directly to the dimm->mtype thing. But then > you'd need a way to map each struct dimm_info *dimm to the UMC so that > you can determine the correct DIMM type. >
I did send a patch that did something like this. https://lkml.kernel.org/r/20211228200615.412999-2-yazen.ghannam@amd.com
Though this got a build warning report, so I need to follow up on that.
> Which would make pvt->dram_type redundant and can be removed. >
I kept this so as to not break legacy systems. But I'll look at it again. I think you may be right.
Thanks, Yazen
| |