mbox series

[linux-next,0/2] ACPI: Add support for ACPI RAS2 feature table

Message ID 20250228122752.2062-1-shiju.jose@huawei.com
Headers show
Series ACPI: Add support for ACPI RAS2 feature table | expand

Message

Shiju Jose Feb. 28, 2025, 12:27 p.m. UTC 
From: Shiju Jose <shiju.jose@huawei.com>

Add support for ACPI RAS2 feature table (RAS2) defined in the ACPI 6.5
specification, section 5.2.21 and RAS2 HW based memory scrubbing feature.

ACPI RAS2 patches were part of the EDAC series [1].

1. https://lore.kernel.org/linux-cxl/20250212143654.1893-1-shiju.jose@huawei.com/ 

Shiju Jose (2):
  ACPI:RAS2: Add ACPI RAS2 driver
  ras: mem: Add memory ACPI RAS2 driver

 Documentation/edac/scrub.rst |  73 ++++++
 drivers/acpi/Kconfig         |  11 +
 drivers/acpi/Makefile        |   1 +
 drivers/acpi/ras2.c          | 417 +++++++++++++++++++++++++++++++++++
 drivers/ras/Kconfig          |  11 +
 drivers/ras/Makefile         |   1 +
 drivers/ras/acpi_ras2.c      | 383 ++++++++++++++++++++++++++++++++
 include/acpi/ras2_acpi.h     |  47 ++++
 8 files changed, 944 insertions(+)
 create mode 100755 drivers/acpi/ras2.c
 create mode 100644 drivers/ras/acpi_ras2.c
 create mode 100644 include/acpi/ras2_acpi.h

Comments

Jonathan Cameron March 3, 2025, 9:35 a.m. UTC | #1 
On Fri, 28 Feb 2025 12:27:48 +0000
<shiju.jose@huawei.com> wrote:

> From: Shiju Jose <shiju.jose@huawei.com>
> 
> Add support for ACPI RAS2 feature table (RAS2) defined in the ACPI 6.5
> specification, section 5.2.21 and RAS2 HW based memory scrubbing feature.
> 
> ACPI RAS2 patches were part of the EDAC series [1].

Whilst linux-next now contains the EDAC patches, we shouldn't base
a feature submission on it.  This should be the same as you
did for the CXL tree with a statement that it depends on 

https://web.git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git/log/?h=edac-cxl

which is the immutable tag / branch Borislav provided.

I doubt there is anything else hitting this code so
shouldn't be any need to rebase (I could be wrong though!)

Assuming everyone is happy with this series, who is going to pick
it up?

Borislav via ras.git, or Rafael via acpi.git?  I don't really
have any preference other than making sure it doesn't fall down
the cracks!

Jonathan

> 
> 1. https://lore.kernel.org/linux-cxl/20250212143654.1893-1-shiju.jose@huawei.com/ 
> 
> Shiju Jose (2):
>   ACPI:RAS2: Add ACPI RAS2 driver
>   ras: mem: Add memory ACPI RAS2 driver
> 
>  Documentation/edac/scrub.rst |  73 ++++++
>  drivers/acpi/Kconfig         |  11 +
>  drivers/acpi/Makefile        |   1 +
>  drivers/acpi/ras2.c          | 417 +++++++++++++++++++++++++++++++++++
>  drivers/ras/Kconfig          |  11 +
>  drivers/ras/Makefile         |   1 +
>  drivers/ras/acpi_ras2.c      | 383 ++++++++++++++++++++++++++++++++
>  include/acpi/ras2_acpi.h     |  47 ++++
>  8 files changed, 944 insertions(+)
>  create mode 100755 drivers/acpi/ras2.c
>  create mode 100644 drivers/ras/acpi_ras2.c
>  create mode 100644 include/acpi/ras2_acpi.h
>
Shiju Jose March 3, 2025, 10:21 a.m. UTC | #2 
>-----Original Message-----
>From: Jonathan Cameron <jonathan.cameron@huawei.com>
>Sent: 03 March 2025 09:36
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: linux-edac@vger.kernel.org; linux-acpi@vger.kernel.org; rafael@kernel.org;
>bp@alien8.de; tony.luck@intel.com; lenb@kernel.org; mchehab@kernel.org;
>linux-mm@kvack.org; linux-kernel@vger.kernel.org; linux-cxl@vger.kernel.org;
>j.williams@intel.com; dave@stgolabs.net; dave.jiang@intel.com;
>alison.schofield@intel.com; vishal.l.verma@intel.com; ira.weiny@intel.com;
>david@redhat.com; Vilas.Sridharan@amd.com; leo.duran@amd.com;
>Yazen.Ghannam@amd.com; rientjes@google.com; jiaqiyan@google.com;
>Jon.Grimm@amd.com; dave.hansen@linux.intel.com;
>naoya.horiguchi@nec.com; james.morse@arm.com; jthoughton@google.com;
>somasundaram.a@hpe.com; erdemaktas@google.com; pgonda@google.com;
>duenwen@google.com; gthelen@google.com;
>wschwartz@amperecomputing.com; dferguson@amperecomputing.com;
>wbs@os.amperecomputing.com; nifan.cxl@gmail.com; tanxiaofei
><tanxiaofei@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>; Roberto
>Sassu <roberto.sassu@huawei.com>; kangkang.shen@futurewei.com;
>wanghuiqiang <wanghuiqiang@huawei.com>; Linuxarm
><linuxarm@huawei.com>
>Subject: Re: [PATCH linux-next 0/2] ACPI: Add support for ACPI RAS2 feature
>table
>
>On Fri, 28 Feb 2025 12:27:48 +0000
><shiju.jose@huawei.com> wrote:
>
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> Add support for ACPI RAS2 feature table (RAS2) defined in the ACPI 6.5
>> specification, section 5.2.21 and RAS2 HW based memory scrubbing feature.
>>
>> ACPI RAS2 patches were part of the EDAC series [1].
>
>Whilst linux-next now contains the EDAC patches, we shouldn't base a feature
>submission on it.  This should be the same as you did for the CXL tree with a
>statement that it depends on
>
>https://web.git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git/log/?h=edac-cxl
>
>which is the immutable tag / branch Borislav provided.

Hi Jonathan,

These RAS2 patches are applied cleanly, built and tested fine in the 
immutable ras.git: 'edac-cxl' branch Borislav provided. 
(https://web.git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git/log/?h=edac-cxl).

Thanks,
Shiju
>
>I doubt there is anything else hitting this code so shouldn't be any need to rebase
>(I could be wrong though!)
>
>Assuming everyone is happy with this series, who is going to pick it up?
>
>Borislav via ras.git, or Rafael via acpi.git?  I don't really have any preference
>other than making sure it doesn't fall down the cracks!
>
>Jonathan
>
>>
>> 1.
>> https://lore.kernel.org/linux-cxl/20250212143654.1893-1-shiju.jose@hua
>> wei.com/
>>
>> Shiju Jose (2):
>>   ACPI:RAS2: Add ACPI RAS2 driver
>>   ras: mem: Add memory ACPI RAS2 driver
>>
>>  Documentation/edac/scrub.rst |  73 ++++++
>>  drivers/acpi/Kconfig         |  11 +
>>  drivers/acpi/Makefile        |   1 +
>>  drivers/acpi/ras2.c          | 417 +++++++++++++++++++++++++++++++++++
>>  drivers/ras/Kconfig          |  11 +
>>  drivers/ras/Makefile         |   1 +
>>  drivers/ras/acpi_ras2.c      | 383 ++++++++++++++++++++++++++++++++
>>  include/acpi/ras2_acpi.h     |  47 ++++
>>  8 files changed, 944 insertions(+)
>>  create mode 100755 drivers/acpi/ras2.c  create mode 100644
>> drivers/ras/acpi_ras2.c  create mode 100644 include/acpi/ras2_acpi.h
>>
Borislav Petkov March 3, 2025, 10:35 a.m. UTC | #3 
On Mon, Mar 03, 2025 at 05:35:38PM +0800, Jonathan Cameron wrote:
> Borislav via ras.git, or Rafael via acpi.git?  I don't really
> have any preference other than making sure it doesn't fall down
> the cracks!

It's probably easier if I take it.

However, just from a cursory look, it would need some scrubbing. There's stuff
like:

+               ps_sm->params.requested_address_range[0] = 0;
+               ps_sm->params.requested_address_range[1] = 0;
+               ps_sm->params.scrub_params_in &= ~RAS2_PATROL_SCRUB_SCHRS_IN_MASK;
+               ps_sm->params.scrub_params_in |= FIELD_PREP(RAS2_PATROL_SCRUB_SCHRS_IN_MASK,
+                                                           ras2_ctx->scrub_cycle_hrs);
+               ps_sm->params.patrol_scrub_command = RAS2_START_PATROL_SCRUBBER;


which definitely needs shortening. There's no need for a wholly written out
"requested_address_range". I know variables should have meaningfull names but
writing fiction shouldn't be either.

+static int ras2_acpi_parse_table(struct acpi_table_header *pAcpiTable)

Yuck, CamelCase?!

And I'm pretty sure if I start looking more, I'll find more funky stuff.

HTH.