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

Shiju Jose posted 2 patches 11 months, 1 week ago
Only 0 patches received!
There is a newer version of this series
RE: [PATCH linux-next 0/2] ACPI: Add support for ACPI RAS2 feature table
Posted by Shiju Jose 11 months, 1 week ago
>-----Original Message-----
>From: Borislav Petkov <bp@alien8.de>
>Sent: 03 March 2025 10:35
>To: Jonathan Cameron <jonathan.cameron@huawei.com>
>Cc: Shiju Jose <shiju.jose@huawei.com>; linux-edac@vger.kernel.org; linux-
>acpi@vger.kernel.org; rafael@kernel.org; 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
>
[...]
>
>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.

Hi Borislav,

Some of these variables, for e.g. requested_address_range are not defined 
in this patch, but in the 'include/acpi/actbl2.h'.
My understanding is that those changes required to upstream first via
https://github.com/acpica/acpica ?
>
>+static int ras2_acpi_parse_table(struct acpi_table_header *pAcpiTable)
>
>Yuck, CamelCase?!
Fixed.

>
>And I'm pretty sure if I start looking more, I'll find more funky stuff.
Will check and fix.
>
>HTH.
>
>--
>Regards/Gruss,
>    Boris.
>
>https://people.kernel.org/tglx/notes-about-netiquette

Thanks,
Shiju
Re: [PATCH linux-next 0/2] ACPI: Add support for ACPI RAS2 feature table
Posted by Borislav Petkov 11 months, 1 week ago
On Tue, Mar 04, 2025 at 06:19:58PM +0000, Shiju Jose wrote:
> Some of these variables, for e.g. requested_address_range are not defined 
> in this patch, but in the 'include/acpi/actbl2.h'.
> My understanding is that those changes required to upstream first via
> https://github.com/acpica/acpica ?

Are you sure?

...
 * Additional ACPI Tables (2)
 *
 * These tables are not consumed directly by the ACPICA subsystem, but are
 * included here to support device drivers and the AML disassembler.
 ...

In any case, if this goes through me, I will have to review it first as it
looks funky.

Your call guys.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
RE: [PATCH linux-next 0/2] ACPI: Add support for ACPI RAS2 feature table
Posted by Luck, Tony 11 months, 1 week ago
> > Some of these variables, for e.g. requested_address_range are not defined
> > in this patch, but in the 'include/acpi/actbl2.h'.
> > My understanding is that those changes required to upstream first via
> > https://github.com/acpica/acpica ?
>
> Are you sure?
>
> ...
>  * Additional ACPI Tables (2)
>  *
>  * These tables are not consumed directly by the ACPICA subsystem, but are
>  * included here to support device drivers and the AML disassembler.
>  ...
>
> In any case, if this goes through me, I will have to review it first as it
> looks funky.

That requested_address_range field has been in the
acpi_rasf_patrol_scrub_parameter structure since 2018

Got moved/copied into the acpi_ras2_patrol_scrub_parameter structure
in 2023.

$ git blame include/acpi/actbl2.h | grep requested_address_range
e62f8227851da (Erik Kaneda                2018-02-15 13:09:26 -0800 2722)       u64 requested_address_range[2];
2e94dc1189804 (Shiju Jose                 2023-09-27 17:41:52 +0100 2829)       u64 requested_address_range[2];

-Tony