linux-kernel - Re: [PATCH v18 00/19] EDAC: Scrub: introduce generic EDAC RAS control feature driver + CXL/ACPI-RAS2 drivers

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: <20250114150546.0a769a8f@foz.lan>
Date: Tue, 14 Jan 2025 15:06:27 +0100
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: <linux-acpi@...r.kernel.org>, <linux-mm@...ck.org>,
 <vishal.l.verma@...el.com>, <shiju.jose@...wei.com>,
 <linux-edac@...r.kernel.org>, <linux-cxl@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <bp@...en8.de>, <tony.luck@...el.com>,
 <rafael@...nel.org>, <lenb@...nel.org>, <mchehab@...nel.org>,
 <dan.j.williams@...el.com>, <dave@...olabs.net>, <dave.jiang@...el.com>,
 <alison.schofield@...el.com>, <ira.weiny@...el.com>, <david@...hat.com>,
 <Vilas.Sridharan@....com>, <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@...wei.com>, <prime.zeng@...ilicon.com>,
 <roberto.sassu@...wei.com>, <kangkang.shen@...urewei.com>,
 <wanghuiqiang@...wei.com>, <linuxarm@...wei.com>
Subject: Re: [PATCH v18 00/19] EDAC: Scrub: introduce generic EDAC RAS
 control feature driver + CXL/ACPI-RAS2 drivers

Em Mon, 13 Jan 2025 15:36:39 +0000
Jonathan Cameron <Jonathan.Cameron@...wei.com> escreveu:

> > >    
> > > 5. CXL features driver supporting ECS control feature.
> > > 6. ACPI RAS2 driver adds OS interface for RAS2 communication through
> > >    PCC mailbox and extracts ACPI RAS2 feature table (RAS2) and
> > >    create platform device for the RAS memory features, which binds
> > >    to the memory ACPI RAS2 driver.
> > > 7. Memory ACPI RAS2 driver gets the PCC subspace for communicating
> > >    with the ACPI compliant platform supports ACPI RAS2. Add callback
> > >    functions and registers with EDAC device to support user to
> > >    control the HW patrol scrubbers exposed to the kernel via the
> > >    ACPI RAS2 table.
> > > 8. Support for CXL maintenance mailbox command, which is used by
> > >    CXL device memory repair feature.   
> > > 9. CXL features driver supporting PPR control feature.
> > > 10. CXL features driver supporting memory sparing control feature.
> > >     Note: There are other PPR, memory sparing drivers to come.    
> > 
> > The text above should be inside Documentation, and not on patch 0.
> > 
> > A big description like that makes hard to review this series. It is
> > also easier to review the text after having it parsed by kernel doc
> > build specially for summary tables like the "Comparison of scrubbing 
> > features", which deserves ReST links processed by Sphinx to the 
> > corresponding definitions of the terms that are be compared there.  
> 
> Whilst I fully agree that having a huge cover letter makes for a burden
> for any reviewer coming to the series, this is here at specific request
> of reviewers. 

Ok, then. Yet, even for them it would be very hard to track what
changes from v19 to the next versions if you change something at 
patch 00.

> We can look at keeping more of it in documentation though
> it's a bit white paper like in comparison with what I'd normally expect
> to see in kernel documentation.

Personally, I like comprehensive documentation at the Kernel.

> >   
> > > Open Questions based on feedbacks from the community:
> > > 1. Leo: Standardize unit for scrub rate, for example ACPI RAS2 does not define
> > >    unit for the scrub rate. RAS2 clarification needed.     
> > 
> > I noticed the same when reviewing a patch series for rasdaemon. Ideally,
> > ACPI requires an errata defining what units are expected for scrub rate.  
> 
> There is a code first ACPI ECN that indeed adds units.  That is accepted
> for next ACPI specification release.
> 
> Seems the tianocore bugzilla is unhelpfully down for a migration
> but it should be id 1013 at bugzilla.tianocore.com
> 
> That adds a detailed description of what the scrub rate settings mean but
> we may well still have older platforms where the scaling is arbitrary.
> The units defined are sufficient to map to whatever presentation we like.
>
> > While ACPI doesn't define it, better to not add support for it - or be
> > conservative using a low granularity for it (like using minutes instead 
> > of hours).  
> 
> I don't mind changing this, though for systems we are aware of default scrub
> is typically once or twice in 24 hours.

Yes, I noticed that we're using seconds after reading other patches.
It sounds OK to me to keep it as-is. 

It is really unlikely that we would ever have scrubbing finishing in less
than a second.

Thanks,
Mauro