Re: [RFC PATCH v7 12/12] memory: RAS2: Add memory RAS2 driver

From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Yazen Ghannam <yazen.ghannam@amd.com>
Cc: <shiju.jose@huawei.com>, <linux-cxl@vger.kernel.org>,
	<linux-acpi@vger.kernel.org>, <linux-mm@kvack.org>,
	<dan.j.williams@intel.com>, <dave@stgolabs.net>,
	<dave.jiang@intel.com>, <alison.schofield@intel.com>,
	<vishal.l.verma@intel.com>, <ira.weiny@intel.com>,
	<linux-edac@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<david@redhat.com>, <Vilas.Sridharan@amd.com>,
	<leo.duran@amd.com>, <rientjes@google.com>, <jiaqiyan@google.com>,
	<tony.luck@intel.com>, <Jon.Grimm@amd.com>,
	<dave.hansen@linux.intel.com>, <rafael@kernel.org>,
	<lenb@kernel.org>, <naoya.horiguchi@nec.com>,
	<james.morse@arm.com>, <jthoughton@google.com>,
	<somasundaram.a@hpe.com>, <erdemaktas@google.com>,
	<pgonda@google.com>, <duenwen@google.com>,
	<mike.malvestuto@intel.com>, <gthelen@google.com>,
	<wschwartz@amperecomputing.com>, <dferguson@amperecomputing.com>,
	<tanxiaofei@huawei.com>, <prime.zeng@hisilicon.com>,
	<kangkang.shen@futurewei.com>, <wanghuiqiang@huawei.com>,
	<linuxarm@huawei.com>
Subject: Re: [RFC PATCH v7 12/12] memory: RAS2: Add memory RAS2 driver
Date: Tue, 2 Apr 2024 11:17:09 +0100	[thread overview]
Message-ID: <20240402111709.0000653a@Huawei.com> (raw)
In-Reply-To: <71d41ec9-17ed-442f-8d68-a6cb9bcbb4b0@amd.com>

On Thu, 28 Mar 2024 11:23:38 -0400
Yazen Ghannam <yazen.ghannam@amd.com> wrote:

> On 2/23/2024 9:37 AM, shiju.jose@huawei.com wrote:
> > From: Shiju Jose <shiju.jose@huawei.com>
> > 
> > Memory RAS2 driver binds to the platform device add by the ACPI RAS2
> > driver.
> > Driver registers the PCC channel for communicating with the ACPI compliant
> > platform that contains RAS2 command support in the hardware.
> > 
> > Add interface functions to support configuring the parameters of HW patrol
> > scrubs in the system, which exposed to the kernel via the RAS2 and PCC,
> > using the RAS2 commands.
> > 
> > Add support for RAS2 platform devices to register with scrub subsystem
> > driver. This enables user to configure the parameters of HW patrol scrubs,
> > which exposed to the kernel via the RAS2 table, through the scrub sysfs
> > attributes.
> > 
> > Open Question:
> > Sysfs scrub control attribute "enable_background_scrub" is added for RAS2,
> > based on the feedback from Bill Schwartz <wschwartz@amperecomputing.com
> > on v4 to enable/disable the background_scrubbing in the platform as defined in the
> > “Configure Scrub Parameters [INPUT]“ field  in RAS2 Table 5.87: Parameter Block
> > Structure for PATROL_SCRUB.
> > Is it a right approach to support "enable_background_scrub" in the sysfs
> > scrub control?
> > 
> > Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> > ---
> >  drivers/memory/Kconfig       |  14 ++
> >  drivers/memory/Makefile      |   2 +
> >  drivers/memory/ras2.c        | 364 +++++++++++++++++++++++++++++++++++
> >  drivers/memory/ras2_common.c | 282 +++++++++++++++++++++++++++
> >  include/memory/ras2.h        |  88 +++++++++
> >  5 files changed, 750 insertions(+)
> >  create mode 100644 drivers/memory/ras2.c
> >  create mode 100644 drivers/memory/ras2_common.c
> >  create mode 100755 include/memory/ras2.h
> >   
> 
> Sorry if this was already covered, but why not put the common scrub and ras2
> changes under /drivers/ras/?

Hi Yazen

Location / naming etc is definitely an open question.
Given the OCP RAS API has repeatedly come up in discussions (and that
has a much wider scope), a RAS focused location may well make sense + some
renaming to avoid being memory scrub specific.

Jonathan

> 
> Thanks,
> Yazen