RE: [PATCH v2 3/3] ras: mem: Add memory ACPI RAS2 driver

[Shiju Jose <shiju.jose@huawei.com>]

>-----Original Message-----
>From: Shiju Jose
>Sent: 10 March 2025 11:12
>To: 'Daniel Ferguson' <danielf@os.amperecomputing.com>; linux-
>edac@vger.kernel.org; linux-acpi@vger.kernel.org; bp@alien8.de;
>tony.luck@intel.com; rafael@kernel.org; lenb@kernel.org;
>mchehab@kernel.org; leo.duran@amd.com; Yazen.Ghannam@amd.com
>Cc: linux-cxl@vger.kernel.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;
>david@redhat.com; Vilas.Sridharan@amd.com; linux-mm@kvack.org; linux-
>kernel@vger.kernel.org; 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 v2 3/3] ras: mem: Add memory ACPI RAS2 driver
>
>>-----Original Message-----
>>From: Daniel Ferguson <danielf@os.amperecomputing.com>
>>Sent: 07 March 2025 21:52
>>To: Shiju Jose <shiju.jose@huawei.com>; linux-edac@vger.kernel.org;
>>linux- acpi@vger.kernel.org; bp@alien8.de; tony.luck@intel.com;
>>rafael@kernel.org; lenb@kernel.org; mchehab@kernel.org;
>>leo.duran@amd.com; Yazen.Ghannam@amd.com
>>Cc: linux-cxl@vger.kernel.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; david@redhat.com;
>>Vilas.Sridharan@amd.com; linux-mm@kvack.org; linux-
>>kernel@vger.kernel.org; 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 v2 3/3] ras: mem: Add memory ACPI RAS2 driver
>>
>>
>>> +static int ras2_hw_scrub_read_size(struct device *dev, void
>>> +*drv_data, u64 *size) {
>>> +	struct ras2_mem_ctx *ras2_ctx = drv_data;
>>> +	int ret;
>>> +
>>> +	if (ras2_ctx->bg_scrub)
>>> +		return -EBUSY;
>>> +
>>> +	ret = ras2_update_patrol_scrub_params_cache(ras2_ctx);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	*size = ras2_ctx->size;
>>> +
>>> +	return 0;
>>> +}
>>
>>Calling ras2_update_patrol_scrub_params_cache here is problematic.
>>
>>Imagine:
>>  echo 0x1000 > size
>>  cat size
>>  echo 0x2000000000 > addr
>>
>>What happens here? What happens is the scrub range is not what you
>>expect it to be.  Once you cat size, you reset the size from what you initially set
>it to.
>>I don't think that is what anyone will expect. It certainly caused us
>>to stumble while testing.
>
>This is an expected behavior and this extra call was added here when changed
>using attribute 'addr' to start the on-demand  scrub operation instead of
>previous separate attribute ' enable_on_demand' to start the on-demand scrub
>operation, according to Borislav's suggestion in v13.
>
> Please see the following comment in the ras2_hw_scrub_read_addr() fnction,
>"Userspace will get the status of the demand scrubbing through the address
>range read from the firmware. When the demand scrubbing is finished
>firmware must reset actual address range to 0. Otherwise userspace assumes
>demand scrubbing is in progress."
>
>Here sysfs attributes 'addr' and 'size' is reading the field: Actual Address Range
>of Table 5.87: Parameter Block Structure for PATROL_SCRUB, written by the
>firmware.
>
>In my opinion, reading back the address range size set in the sysfs before
>actually writing the address range to the firmware and starting the on-demand
>scrub operation doesn't hold much significance?

After further discussion, I will add a fix for this case to return the 'size' which the user set in the sysfs
until the scrubbing is started.

Thanks,
Shiju 
>