Re: [PATCH v13 15/18] EDAC: Add memory repair control feature

[Jonathan Cameron]

On Mon, 14 Oct 2024 17:39:12 +0100
Shiju Jose <shiju.jose@xxxxxxxxxx> wrote:

> >-----Original Message-----
> >From: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>
> >Sent: 14 October 2024 17:23
> >To: Shiju Jose <shiju.jose@xxxxxxxxxx>
> >Cc: linux-edac@xxxxxxxxxxxxxxx; linux-cxl@xxxxxxxxxxxxxxx; linux-
> >acpi@xxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> >bp@xxxxxxxxx; tony.luck@xxxxxxxxx; rafael@xxxxxxxxxx; lenb@xxxxxxxxxx;
> >mchehab@xxxxxxxxxx; dan.j.williams@xxxxxxxxx; dave@xxxxxxxxxxxx;
> >dave.jiang@xxxxxxxxx; alison.schofield@xxxxxxxxx; vishal.l.verma@xxxxxxxxx;
> >ira.weiny@xxxxxxxxx; david@xxxxxxxxxx; Vilas.Sridharan@xxxxxxx;
> >leo.duran@xxxxxxx; Yazen.Ghannam@xxxxxxx; rientjes@xxxxxxxxxx;
> >jiaqiyan@xxxxxxxxxx; Jon.Grimm@xxxxxxx; dave.hansen@xxxxxxxxxxxxxxx;
> >naoya.horiguchi@xxxxxxx; james.morse@xxxxxxx; jthoughton@xxxxxxxxxx;
> >somasundaram.a@xxxxxxx; erdemaktas@xxxxxxxxxx; pgonda@xxxxxxxxxx;
> >duenwen@xxxxxxxxxx; gthelen@xxxxxxxxxx;
> >wschwartz@xxxxxxxxxxxxxxxxxxx; dferguson@xxxxxxxxxxxxxxxxxxx;
> >wbs@xxxxxxxxxxxxxxxxxxxxxx; nifan.cxl@xxxxxxxxx; tanxiaofei
> ><tanxiaofei@xxxxxxxxxx>; Zengtao (B) <prime.zeng@xxxxxxxxxxxxx>; Roberto
> >Sassu <roberto.sassu@xxxxxxxxxx>; kangkang.shen@xxxxxxxxxxxxx;
> >wanghuiqiang <wanghuiqiang@xxxxxxxxxx>; Linuxarm
> ><linuxarm@xxxxxxxxxx>
> >Subject: Re: [PATCH v13 15/18] EDAC: Add memory repair control feature
> >
> >On Wed, 9 Oct 2024 13:41:16 +0100
> ><shiju.jose@xxxxxxxxxx> wrote:
> >  
> >> From: Shiju Jose <shiju.jose@xxxxxxxxxx>
> >>
> >> Add generic EDAC memory repair control, eg. PPR(Post Package Repair),
> >> memory sparing etc, control driver in order to control memory repairs
> >> in the system. Supports sPPR(soft PPR), hPPR(hard PPR), soft/hard
> >> memory sparing, memory sparing at cacheline/row/bank/rank granularity etc.
> >> Device with memory repair features registers with EDAC device driver,
> >> which retrieves memory repair descriptor from EDAC memory repair
> >> driver and exposes the sysfs repair control attributes to userspace in
> >> /sys/bus/edac/devices/<dev-name>/mem_repairX/.
> >>
> >> The common memory repair control interface abstracts the control of an
> >> arbitrary memory repair functionality to a common set of functions.
> >> The sysfs memory repair attribute nodes would be present only if the
> >> client driver has implemented the corresponding attribute callback
> >> function and passed in ops to the EDAC device driver during registration.
> >>
> >> Signed-off-by: Shiju Jose <shiju.jose@xxxxxxxxxx>  
> [...]
> >  
> >> +
> >> +What:		/sys/bus/edac/devices/<dev-name>/mem_repairX/hpa
> >> +Date:		Oct 2024
> >> +KernelVersion:	6.12
> >> +Contact:	linux-edac@xxxxxxxxxxxxxxx
> >> +Description:
> >> +		(WO) Set HPA (Host Physical Address) for memory repair.  
> >
> >Can we not just read back what was written?  Seems like userspace might expect
> >that?  
> I am fine to add read back.
> I did not add read back  for controls because there was no such requirement from the client driver and
> also tried to reduce the number of callbacks in the initial version. 

I think we can for now at least just cache in the core code.
If we have future implementations where more validation is possible
then we can add optional callbacks at that stage.

Jonathan

> 
> Thanks,
> Shiju