RE: [RFC PATCH v7 00/12] memory: scrub: introduce subsystem + CXL/ACPI-RAS2 drivers

Shiju Jose
2024-04-03T13:52:06Z

Hi Daniel,

Thanks for the feedbacks.

>-----Original Message-----
>From: Daniel Ferguson <danielf@os.amperecomputing.com>
>Sent: 28 March 2024 23:40
>To: Shiju Jose <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; Jonathan Cameron <jonathan.cameron@huawei.com>;
>dave.jiang@intel.com; alison.schofield@intel.com; vishal.l.verma@intel.com;
>ira.weiny@intel.com
>Cc: linux-edac@vger.kernel.org; linux-kernel@vger.kernel.org;
>david@redhat.com; Vilas.Sridharan@amd.com; leo.duran@amd.com;
>Yazen.Ghannam@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 <tanxiaofei@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>;
>kangkang.shen@futurewei.com; wanghuiqiang <wanghuiqiang@huawei.com>;
>Linuxarm <linuxarm@huawei.com>; wbs@os.amperecomputing.com
>Subject: Re: [RFC PATCH v7 00/12] memory: scrub: introduce subsystem +
>CXL/ACPI-RAS2 drivers
>
>> RAS2 HW based memory patrol scrub needs RAS2 PCC interfaces and ACPI
>> RAS2 driver for communication b/w kernel and firmware.
>> ACPI RAS2 Driver adds platform device, for each memory feature, which
>> binds to the RAS2 memory driver.
>> Memory RAS2 driver registers with the memory scrub subsystem to expose
>> the RAS2 scrub controls to the user.
>
>Hi Shiju,
>Thanks for this work. This has been very useful for us, as we've been using it to
>test a RAS2 implementation here at Ampere Computing.
>
>In general, the pieces implementing RAS2 are useful and functional. We did,
>however, encounter a few places where we had to make some bug fixes.
>I'll reply to specific patches with changes I can recommend. Also, there are a
>few implementation choices that I will have questions about.
>
>Here is an outline containing the generalized comments I plan on making
>within/near relevant patches:
>1) Found a bug in ras2.c in how the pcc_desc_list pointer is incremented.
This bug was fixed in the local v8 code.

>2) Executing a RAS2 command seems unnecessary for feature detection.
>specifically, in the functions ras2_is_patrol_scrub_support() and
>ras2_get_patrol_scrub_params()
I will check and modify in the next version.

>3) Consider adding more error detection/reporting in ras2_check_pcc_chan()
Sure.

>4) Concerns with multiple scrub devices attempting to map the same PCC
>channel.
I will reply in the patch.
>
>-Daniel
>
Thanks,
Shiju