Date:   Thu, 16 Dec 2021 16:08:18 +0000
From:   Yazen Ghannam <yazen.ghannam@amd.com>
To:     Borislav Petkov <bp@alien8.de>
Cc:     linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
        mchehab@kernel.org, tony.luck@intel.com, james.morse@arm.com,
        rric@kernel.org, Smita.KoralahalliChannabasappa@amd.com,
        william.roche@oracle.com
Subject: Re: [PATCH v2 1/2] EDAC/amd64: Check register values from all UMCs
Message-ID: <YbtkcppejpW8gHqY@yaz-ubuntu>
References: <20211215155309.2711917-1-yazen.ghannam@amd.com>
 <20211215155309.2711917-2-yazen.ghannam@amd.com>
 <YbotciKVDsH1Fl1H@zn.tnic>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YbotciKVDsH1Fl1H@zn.tnic>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?mBrt5vZz5/0UhNk5mAQCSCjqOPpedGH3tUEMaQOgzwVzcuabRps0FRBh5XY2?=
 =?us-ascii?Q?I3pTpPHmPTMlAJkXAlxtwKQ0zgUdVqEU+K1dep05ILVGLFZE2331tEF2OTAP?=
 =?us-ascii?Q?79XRd67e7cXevTRw1ycJ0SVx4RZEPq0n4SMHBqpscusk8zACUhkaubbr165n?=
 =?us-ascii?Q?muO+0wVAPkCHvZCKiUlTFBiCidK7Lr8iDD/oZX9bolD6i5EliphYi3DU7adg?=
 =?us-ascii?Q?2eVBSvOjz61uiCfUX4j4xOrgvF4w5qtVL9rwsWzVvhyH57mgXDatRowobFvA?=
 =?us-ascii?Q?R9LliWBsOUNNJTV7d4aI3A2iU+yrvIcHn7BHN+v7nKZZ7XX1qA/crf06NsZY?=
 =?us-ascii?Q?Th+nIUFCCQsdN2j5Sq9zrwvvPH5fFIy/1Zl1PYrOjzzsy/EM7ag1Bk5vzOVx?=
 =?us-ascii?Q?9nuzbqaUgUn5pdLWmfCZuhym9CjvktBg9sfej/y8jgvGczjRQ/MM7pggYrlV?=
 =?us-ascii?Q?X1NzpyfZBk+mS9iBraq1sGKODLraBOi9CMI8ERxBxrZ+1GyviNunqRxf3e5Y?=
 =?us-ascii?Q?MjP86xpTqvyPu+eRnURETCw4PkKsL352Lja/dXqD+0RjOyIiVGCvwco90pjj?=
 =?us-ascii?Q?9KFhhfm6KhTsLaBrwUKWGarUiAdGSuFO1HlDD5AZwqW4GEGdjLT6StFJlwr1?=
 =?us-ascii?Q?dnz4LzOq4UYOv/hiLZ+ItVIpVj9kkfIaSoMmgE8scAnNug2OYu0EqmYYKFuU?=
 =?us-ascii?Q?zzveEXCn0Nz98QgBoJji3y94Adb5iQ9ysnDCy45DajkeeceoiyAsFS+hYc2R?=
 =?us-ascii?Q?zofZ0RArpvxz9APPkqp1bxF16LX//LAHnHzRnXmKK57MU+HESA5keD8QrATf?=
 =?us-ascii?Q?fwboY67mIgU8H40yVuZPYLN+yYtpFnmSLxg7+zjdt5nDoXGOsmpkCC+BnU+q?=
 =?us-ascii?Q?4ovsA9B6EoD0OeIpFf2EzCXcldDIhTabNb4MJBeWz+3cHemHSgjt6zPhEWJc?=
 =?us-ascii?Q?mixHYiyVFLxVc/EZv9mvx4qz5aj0Lo6nchNPNvYwwZ0Z42KSNMkQitetaZHO?=
 =?us-ascii?Q?2Ev/n4CRBfZegULxm9fx1IuFe7zlTbwUReAd+prXkQftQ+SbAf8X/CK1xUEu?=
 =?us-ascii?Q?Ft+B3qJFQ3AylkDbbyIeP26DLTm3RNJMPKEHBrtYPmbl28WYoi4cDscLuuWd?=
 =?us-ascii?Q?uPw+TbOZjAbqxxQpz3KqbGRYNM574flxm4CQRI/Kl6IBKdJ7xOX2Ado4Wu+g?=
 =?us-ascii?Q?NZuujPQcJsJbeTBMmo3+pqYm8xxO+ukfVTKevpxAyD/qSLVFjXvH1+rexPit?=
 =?us-ascii?Q?G66lrmUTbZRvw93WK+NF01klodmDT2R4tAE7j6mRIxX4CQ0hw57V3KM86xXe?=
 =?us-ascii?Q?AZnexvdLrzSUZBdw0nwjYgr43vHxdBJ/X4f9GKgBHJ2H+vyrj/bfHicoY39i?=
 =?us-ascii?Q?e2fF/l2g1dXPA96qWLYlqbbIz0Gpu4XgPOe/LrrXLsCUdZ7SpNgjDBsU7g0h?=
 =?us-ascii?Q?rAAdiGD65hGkTcXsirwtVNXZLPjTrW2wrdWdYbq+6SBTlm0AygrtzEzQSRFZ?=
 =?us-ascii?Q?ia6e+Va/pd/m/C/cfVDwmNTPLTmuDW2l4Nho75PZmFecxS4gPuT7JO4lhDOc?=
 =?us-ascii?Q?D91t+9WXAsGJ1a1cQdBzS5AmeGY0D1kw8RztI1jFL3ZKhRAvUNVpuo0QO9wS?=
 =?us-ascii?Q?rkpd7R3tIxyC70g/FqC66Q4=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1e9e3c30-51b3-4f16-292e-08d9c0ae4b5f
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3108.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Dec 2021 16:08:27.3349
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: cqsc2Cx2ZGPx7F2692/o3xj+WLlS7q6jweND92Z+uJnFraV2QUHLJniY6qc2TbXM2maFU8oYIhRMtTe35kZj+Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR1201MB2480
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Dec 15, 2021 at 07:01:22PM +0100, Borislav Petkov wrote:
> On Wed, Dec 15, 2021 at 03:53:08PM +0000, Yazen Ghannam wrote:
> > -		if ((pvt->umc[0].dimm_cfg | pvt->umc[1].dimm_cfg) & BIT(5))
> > +		u32 umc_cfg = 0, dimm_cfg = 0, i = 0;
> > +
> > +		for_each_umc(i) {
> > +			umc_cfg  |= pvt->umc[i].umc_cfg;
> > +			dimm_cfg |= pvt->umc[i].dimm_cfg;
> > +		}
> > +
> > +		if (dimm_cfg & BIT(5))
> >  			pvt->dram_type = MEM_LRDDR4;
> > -		else if ((pvt->umc[0].dimm_cfg | pvt->umc[1].dimm_cfg) & BIT(4))
> > +		else if (dimm_cfg & BIT(4))
> 
> You're working here under the assumption that bit 4 and 5 will have the
> same value on all those UMCs.
> 
> You're probably going to say that that is how the BIOS is programming
> them so they should be all the same and any other configuration is
> invalid but lemme still ask about it explicitly.
> 
> And if so, this would probably need a comment above it which I can add
> when applying...
> 
> Hmm?
>

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.

Do you recommend a follow up patch or should this one be reworked?

Thanks,
Yazen