Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 9 Mar 2022 21:42:21 +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
Subject: Re: [PATCH v4 01/24] EDAC/amd64: Define Data Fabric operations
Message-ID: <YikfPWACGyWPIxKP@yaz-ubuntu>
References: <20220127204115.384161-1-yazen.ghannam@amd.com>
 <20220127204115.384161-2-yazen.ghannam@amd.com>
 <Yga0s+9cqtfB3Yxu@zn.tnic>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yga0s+9cqtfB3Yxu@zn.tnic>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?zmLFaTSzId11VBot2J3W9OQw9glFlI+JMHwv3/64G/my/0a+TSoGwG2TN28Z?=
 =?us-ascii?Q?XKKU+vnLYWTXk9TKiDIUc+OMwXk5UOLjsvhpUZjsa2iZZU4AmNtDnfzqVHq3?=
 =?us-ascii?Q?owahu4yOwNLHO2lZy+/Ii7rviO3PKvFP1w2xhHpNZ+LDCD017/HgOfYjYiW2?=
 =?us-ascii?Q?hVqDsmyP1fcOnoO8mrLwOB8vXCVpjC4bCG8wIJhOEVZKDqKU4XlJspfvZtN1?=
 =?us-ascii?Q?TuwOHsx37W/y3W9Wp0Gxf+YGZPx6O1JiQcOEHmjUYCcXw8+nnOKfg06iF2qP?=
 =?us-ascii?Q?UHqx79uIMjLzayvrrXRqO9POFU7EVMxeM2KETg60WqvR5yPd0L2vl56kcHXn?=
 =?us-ascii?Q?z+faK2gIgJxmaaZmN/7aEZr5aNtbHbPKHQ1pC2vNzqkd6+ooiUkhwIIfEcm5?=
 =?us-ascii?Q?11osdy6WnnfW8/jfvRYRDa3WUwU1jtMIpNtsnPuyVtdibr4vNMVMainIQrIu?=
 =?us-ascii?Q?k+IEeGtPFXEJfaL+5D8CtmKjGvSuH88v9lzuRaD4GLeQsUhNhUrZtv5WFBLM?=
 =?us-ascii?Q?seQVNVY+K1odhStfPO/WbRJ3jQ/J6rJSHIaTuInlsam7NHdWMeT6Gvu1QhAv?=
 =?us-ascii?Q?QenNAehWhXGQslG44M8y0fXjC0CqNlPrKnDyE6r1olam+iCnpWKmsvQabNRb?=
 =?us-ascii?Q?Wf/SEA0KmbffI5SzuW4BeuwGwdTap18zV+PWU05FXstGFI8QIvs6ad9F5/mT?=
 =?us-ascii?Q?XeXssL4eSdS9awFAzOFrQR29KhHAwcfIDYhYOOCFRQ+04lGUyVDS3sqNGqa9?=
 =?us-ascii?Q?7UDdEsVwW02ou9TlnCMs4J8B3rhIE1YUudtYoX1WLj7P2QFwAQ4oTHN0o2kJ?=
 =?us-ascii?Q?c2HW2OPaFuQehk8qvL2M4NEKD++wOJe9VsGr994VaSXCrogLSlQU+t4fhAeD?=
 =?us-ascii?Q?HFZsDAMdo6BZJEvAzzBVWw2HuYOjO/aWu1INuTCyOC/bWZ8MuCM4nSaaujED?=
 =?us-ascii?Q?FY5JxQbk1gVBNB997bqTK2g96WVl9a91/DA28MdZk2qJnRtPrTXEi3cIhlsL?=
 =?us-ascii?Q?mNqhQOTxf5R7hpWenjKQpi+jSzUFRpV6swNNu6sF0W4ZYMDwMad0XBe5U8aL?=
 =?us-ascii?Q?wjkZg34jY8LjzgLeqrAOAKWY/XhtrATLkvRb8Te95RkS1O/GDVt159YKKAa/?=
 =?us-ascii?Q?GBDGZbAKCJ9ou0vHMKUXuBoOp+GTD0AYzM4gNlvGGEIBfmWx+ACCSL2FWhJf?=
 =?us-ascii?Q?eizVQCnz5TB9i0lRDO5CHablh6wcIzjK9oYN3iPMvnV6JBrFZw+FjLE3Tncw?=
 =?us-ascii?Q?BTA9+dlnau0c+DHzzaY8LZllR5XuEF/XzfppmNXdiorN5dhSMK5FF4RtJ2Fw?=
 =?us-ascii?Q?xUwMqlaRkfxpG+vDrf2J8JdqWuDVwFpa89/WR5zKv6h8PLV8UyfrYBXHTZGy?=
 =?us-ascii?Q?E9iaos+zNO1YLFcWFteFqyVGjic941tVThOZuiEUTj+3gZE0p/1joYkN6/Gt?=
 =?us-ascii?Q?2YUmjZGUc7aR2DEjK4bzTw8NY5E73t11?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d1625643-b72d-4751-d15c-08da0215b640
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3108.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Mar 2022 21:42:30.3022
 (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: cQGI5AR1BrVUILRI9hv7HYdugZjxDJ1+VxiK4BIe1Y85/Iz9PQQofsr+y0D6Z+NOFLNWfyiuIiiTOp7aSUd8OQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR1201MB0241
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Feb 11, 2022 at 08:10:43PM +0100, Borislav Petkov wrote:
> > +/*
> > + * Glossary of acronyms used in address translation for Zen-based systems
> > + *
> > + * DF          =       Data Fabric
> 
> Yeah, "DF: Data Fabric" is probably easier to parse, without that weird
> spacing and equals sign.
> 

Okay, will change.

> > + */
> > +
> >  /* Protect the PCI config register pairs used for DF indirect access. */
> >  static DEFINE_MUTEX(df_indirect_mutex);
> >  
> > @@ -1058,6 +1064,14 @@ struct addr_ctx {
> >  	u8 inst_id;
> >  };
> >  
> > +struct data_fabric_ops {
> > +};
> 
> I know that this is not the only example but we have struct definitions
> interspersed with functions in the .c file while former should be all in
> the header. It is a lot cleaner to have definitions and inline functions
> in the header and the actual functionality in the C file but I leave it
> up to you to decide what you prefer.
>

Makes sense. I'll make the change.
 
> > +
> > +struct data_fabric_ops df2_ops = {
> > +};
> > +
> > +struct data_fabric_ops *df_ops;
> > +
> >  static int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr)
> >  {
> >  	u64 dram_base_addr, dram_limit_addr, dram_hole_base;
> > @@ -1072,6 +1086,11 @@ static int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr
> >  
> >  	struct addr_ctx ctx;
> >  
> > +	if (!df_ops) {
> > +		pr_debug("Data Fabric Operations not set");
> 
> That probably wants to be a WARN_ON_ONCE() so that it is loud and
> prominent when it happens...
>

I'd like to keep this a debug message because address translation updates will
almost certainly lag general EDAC enablement. For example, Family 19h Model
10h is enabled now, but df_ops won't be set. The translation code will return
early here, and the EDAC message will say that the tranlsation failed. But
this isn't a bug that needs a WARNing. Rather it's just that the feature isn't
enabled yet for new systems.

Thanks,
Yazen