linux-kernel - RE: [PATCH v2 7/8] cxl/memfeature: Add CXL memory device soft PPR control feature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <481fb71e2867469d963e74d89a0228be@huawei.com>
Date: Fri, 28 Mar 2025 13:05:20 +0000
From: Shiju Jose <shiju.jose@...wei.com>
To: Borislav Petkov <bp@...en8.de>
CC: "linux-cxl@...r.kernel.org" <linux-cxl@...r.kernel.org>,
	"dan.j.williams@...el.com" <dan.j.williams@...el.com>, "dave@...olabs.net"
	<dave@...olabs.net>, Jonathan Cameron <jonathan.cameron@...wei.com>,
	"dave.jiang@...el.com" <dave.jiang@...el.com>, "alison.schofield@...el.com"
	<alison.schofield@...el.com>, "vishal.l.verma@...el.com"
	<vishal.l.verma@...el.com>, "ira.weiny@...el.com" <ira.weiny@...el.com>,
	"david@...hat.com" <david@...hat.com>, "Vilas.Sridharan@....com"
	<Vilas.Sridharan@....com>, "linux-edac@...r.kernel.org"
	<linux-edac@...r.kernel.org>, "linux-acpi@...r.kernel.org"
	<linux-acpi@...r.kernel.org>, "linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"tony.luck@...el.com" <tony.luck@...el.com>, "rafael@...nel.org"
	<rafael@...nel.org>, "lenb@...nel.org" <lenb@...nel.org>,
	"mchehab@...nel.org" <mchehab@...nel.org>, "leo.duran@....com"
	<leo.duran@....com>, "Yazen.Ghannam@....com" <Yazen.Ghannam@....com>,
	"rientjes@...gle.com" <rientjes@...gle.com>, "jiaqiyan@...gle.com"
	<jiaqiyan@...gle.com>, "Jon.Grimm@....com" <Jon.Grimm@....com>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"naoya.horiguchi@....com" <naoya.horiguchi@....com>, "james.morse@....com"
	<james.morse@....com>, "jthoughton@...gle.com" <jthoughton@...gle.com>,
	"somasundaram.a@....com" <somasundaram.a@....com>, "erdemaktas@...gle.com"
	<erdemaktas@...gle.com>, "pgonda@...gle.com" <pgonda@...gle.com>,
	"duenwen@...gle.com" <duenwen@...gle.com>, "gthelen@...gle.com"
	<gthelen@...gle.com>, "wschwartz@...erecomputing.com"
	<wschwartz@...erecomputing.com>, "dferguson@...erecomputing.com"
	<dferguson@...erecomputing.com>, "wbs@...amperecomputing.com"
	<wbs@...amperecomputing.com>, "nifan.cxl@...il.com" <nifan.cxl@...il.com>,
	tanxiaofei <tanxiaofei@...wei.com>, "Zengtao (B)" <prime.zeng@...ilicon.com>,
	Roberto Sassu <roberto.sassu@...wei.com>, "kangkang.shen@...urewei.com"
	<kangkang.shen@...urewei.com>, wanghuiqiang <wanghuiqiang@...wei.com>,
	Linuxarm <linuxarm@...wei.com>
Subject: RE: [PATCH v2 7/8] cxl/memfeature: Add CXL memory device soft PPR
 control feature

>-----Original Message-----
>From: Borislav Petkov <bp@...en8.de>
>Sent: 27 March 2025 17:09
>To: Shiju Jose <shiju.jose@...wei.com>
>Cc: linux-cxl@...r.kernel.org; dan.j.williams@...el.com; dave@...olabs.net;
>Jonathan Cameron <jonathan.cameron@...wei.com>; dave.jiang@...el.com;
>alison.schofield@...el.com; vishal.l.verma@...el.com; ira.weiny@...el.com;
>david@...hat.com; Vilas.Sridharan@....com; linux-edac@...r.kernel.org;
>linux-acpi@...r.kernel.org; linux-mm@...ck.org; linux-kernel@...r.kernel.org;
>tony.luck@...el.com; rafael@...nel.org; lenb@...nel.org;
>mchehab@...nel.org; leo.duran@....com; Yazen.Ghannam@....com;
>rientjes@...gle.com; jiaqiyan@...gle.com; Jon.Grimm@....com;
>dave.hansen@...ux.intel.com; naoya.horiguchi@....com;
>james.morse@....com; jthoughton@...gle.com; somasundaram.a@....com;
>erdemaktas@...gle.com; pgonda@...gle.com; duenwen@...gle.com;
>gthelen@...gle.com; wschwartz@...erecomputing.com;
>dferguson@...erecomputing.com; wbs@...amperecomputing.com;
>nifan.cxl@...il.com; tanxiaofei <tanxiaofei@...wei.com>; Zengtao (B)
><prime.zeng@...ilicon.com>; Roberto Sassu <roberto.sassu@...wei.com>;
>kangkang.shen@...urewei.com; wanghuiqiang <wanghuiqiang@...wei.com>;
>Linuxarm <linuxarm@...wei.com>
>Subject: Re: [PATCH v2 7/8] cxl/memfeature: Add CXL memory device soft PPR
>control feature
>
>On Thu, Mar 27, 2025 at 06:01:56PM +0100, Borislav Petkov wrote:
>> On Thu, Mar 20, 2025 at 06:04:44PM +0000, shiju.jose@...wei.com wrote:
>> > diff --git a/drivers/edac/mem_repair.c b/drivers/edac/mem_repair.c
>> > index 3b1a845457b0..bf7e01a8b4dd 100755
>> > --- a/drivers/edac/mem_repair.c
>> > +++ b/drivers/edac/mem_repair.c
>> > @@ -45,6 +45,11 @@ struct edac_mem_repair_context {
>> >  	struct attribute_group group;
>> >  };
>> >
>> > +const char * const edac_repair_type[] = {
>> > +	[EDAC_PPR] = "ppr",
>> > +};
>> > +EXPORT_SYMBOL_GPL(edac_repair_type);
>>
>> Why is this thing exported instead of adding a getter function and
>> having all its users pass in proper defines as arguments?
>>
>> And "EDAC_PPR" is not a proper define - it doesn't tell me what it is.
>>
>> It should be more likely a
>>
>> EDAC_REPAIR_PPR,
>> EDAC_REPAIR_ROW_SPARING,
>> EDAC_REPAIR_BANK_SPARING,
>>
>> and so on.

Hi Borislav,

Will change.

>
>Looking at this more:
>
>+static int cxl_ppr_get_repair_type(struct device *dev, void *drv_data,
>+				   const char **repair_type)
>+{
>+	*repair_type = edac_repair_type[EDAC_PPR];
>+
>+	return 0;
>+}
>
>Can this be any more silly?
>
>An ops member which copies a string pointer into some argument. What for?
>
>If those strings are for userspace, why don't you simply return *numbers* and
>let userspace convert them into strings?
Yes. The EDAC memory repair interface defined as return 'numbers' for userspace until 
v18 of the EDAC series.  Changed to return 'string' as Mauro wanted.
Please see discussion here.
https://lore.kernel.org/all/20250114152617.14eb41b5@foz.lan/

>
Thanks,
Shiju