Patch: Make ACPI subsystem provide CEDT table

Patch: Make ACPI subsystem provide CEDT table

[Robert Kiraly]

This is a simple ACPI patch. I'm submitting a revised version to 
linux-acpi per Rafael Wysocki and linux-cxl per Dan Williams.

The patch has been moved to the message body and a Signed-off-by tag has 
been added. Thunderbird has added CRs so "patch -p1 -l" is needed. To 
address other points:

Q. Dan Williams said: Yes, although I don't see much incremental benefit 
to developers publishing a root only raw table... So, not opposed to the 
patch, but I want to direct you to the sysfs representation of the CEDT 
and CFMWS in /sys/bus/cxl/devices... Platform firmware CXL details are 
modeled as "CXL root" objects in the /sys/bus/cxl device model.

A. The patch is being submitted in connection with work on a CXL bridge. 
The developers would like to express appreciation for the "sysfs" point. 
They find it useful to be able to do initialization through the table 
but will look at the "sysfs" approach.

Q. Rafael J. Wysocki said: I'm not sure why the #ifdef is needed.

A. I reviewed cxl-next and decided to add the #ifdef to ensure that the 
code would compile regardless of the direction that the codebase takes.

==== start of patch ====
Subject: Make ACPI subsystem provide CEDT table
Signed-off-by: Robert Kiraly <me@boldcoder.com>

This is a simple patch to the ACPI subsystem. The patch adds ACPI
_SIG_CEDT to table_sigs[] in  "drivers/acpi/tables.c".

Presently,  CXL code isn't able to make use of the  CEDT table at
the initrd stage. Subsequent to the change, the CEDT table can be
used.

If  a developer  plans to use the CEDT table to  program  the CXL
ports,  this change is needed.  Otherwise,  it isn't  expected to
make a difference.

I certify  compliance with  Developer’s Certificate of Origin 1.1
as listed in:
https://www.kernel.org/doc/html/v4.10/process/submitting-patches.
html

--- linux-5.16.old/drivers/acpi/tables.c
+++ linux-5.16/drivers/acpi/tables.c
@@ -500,6 +500,9 @@
  	ACPI_SIG_WDDT, ACPI_SIG_WDRT, ACPI_SIG_DSDT, ACPI_SIG_FADT,
  	ACPI_SIG_PSDT, ACPI_SIG_RSDT, ACPI_SIG_XSDT, ACPI_SIG_SSDT,
  	ACPI_SIG_IORT, ACPI_SIG_NFIT, ACPI_SIG_HMAT, ACPI_SIG_PPTT,
+#ifdef ACPI_SIG_CEDT
+       ACPI_SIG_CEDT,
+#endif
  	ACPI_SIG_NHLT };

  #define ACPI_HEADER_SIZE sizeof(struct acpi_table_header)
==== end of patch ====

Re: Patch: Make ACPI subsystem provide CEDT table

[Rafael J. Wysocki]

On Tue, Jan 25, 2022 at 7:30 AM Robert Kiraly <me@boldcoder.com> wrote:
>
> This is a simple ACPI patch. I'm submitting a revised version to
> linux-acpi per Rafael Wysocki and linux-cxl per Dan Williams.
>
> The patch has been moved to the message body and a Signed-off-by tag has
> been added. Thunderbird has added CRs so "patch -p1 -l" is needed. To
> address other points:
>
> Q. Dan Williams said: Yes, although I don't see much incremental benefit
> to developers publishing a root only raw table... So, not opposed to the
> patch, but I want to direct you to the sysfs representation of the CEDT
> and CFMWS in /sys/bus/cxl/devices... Platform firmware CXL details are
> modeled as "CXL root" objects in the /sys/bus/cxl device model.
>
> A. The patch is being submitted in connection with work on a CXL bridge.
> The developers would like to express appreciation for the "sysfs" point.
> They find it useful to be able to do initialization through the table
> but will look at the "sysfs" approach.
>
> Q. Rafael J. Wysocki said: I'm not sure why the #ifdef is needed.
>
> A. I reviewed cxl-next and decided to add the #ifdef to ensure that the
> code would compile regardless of the direction that the codebase takes.

ACPI_SIG_CEDT is defined in 5.17-rc1 AFAICS, so the #ifdef is redundant.

> ==== start of patch ====
> Subject: Make ACPI subsystem provide CEDT table
> Signed-off-by: Robert Kiraly <me@boldcoder.com>
>
> This is a simple patch to the ACPI subsystem. The patch adds ACPI
> _SIG_CEDT to table_sigs[] in  "drivers/acpi/tables.c".
>
> Presently,  CXL code isn't able to make use of the  CEDT table at
> the initrd stage. Subsequent to the change, the CEDT table can be
> used.
>
> If  a developer  plans to use the CEDT table to  program  the CXL
> ports,  this change is needed.  Otherwise,  it isn't  expected to
> make a difference.
>
> I certify  compliance with  Developer’s Certificate of Origin 1.1
> as listed in:
> https://www.kernel.org/doc/html/v4.10/process/submitting-patches.
> html
>
> --- linux-5.16.old/drivers/acpi/tables.c
> +++ linux-5.16/drivers/acpi/tables.c
> @@ -500,6 +500,9 @@
>         ACPI_SIG_WDDT, ACPI_SIG_WDRT, ACPI_SIG_DSDT, ACPI_SIG_FADT,
>         ACPI_SIG_PSDT, ACPI_SIG_RSDT, ACPI_SIG_XSDT, ACPI_SIG_SSDT,
>         ACPI_SIG_IORT, ACPI_SIG_NFIT, ACPI_SIG_HMAT, ACPI_SIG_PPTT,
> +#ifdef ACPI_SIG_CEDT
> +       ACPI_SIG_CEDT,
> +#endif
>         ACPI_SIG_NHLT };
>
>   #define ACPI_HEADER_SIZE sizeof(struct acpi_table_header)
> ==== end of patch ====
>
>

Re: Patch: Make ACPI subsystem provide CEDT table

[Robert Kiraly]

On 01/25/2022 06:49AM, Rafael J. Wysocki wrote:
 >
 > ACPI_SIG_CEDT is defined in 5.17-rc1 AFAICS, so the #ifdef is redundant.

Noted. Thank you. Should I resubmit without the #ifdef or if the patch 
is accepted can it be removed by upstream?

Re: Patch: Make ACPI subsystem provide CEDT table

[Rafael J. Wysocki]

On Wed, Jan 26, 2022 at 4:12 AM Robert Kiraly <me@boldcoder.com> wrote:
>
> On 01/25/2022 06:49AM, Rafael J. Wysocki wrote:
>  >
>  > ACPI_SIG_CEDT is defined in 5.17-rc1 AFAICS, so the #ifdef is redundant.
>
> Noted. Thank you. Should I resubmit without the #ifdef or if the patch
> is accepted can it be removed by upstream?

Please resubmit it without the #ifdef.

Patch: Make ACPI subsystem provide CEDT table

[Robert Kiraly]

This is a simple ACPI patch. I'm submitting a revised version to 
linux-acpi per Rafael Wysocki and linux-cxl per Dan Williams.

Thunderbird has added CRs so "patch -p1 -l" is needed.

==== start of patch ====
Subject: Make ACPI subsystem provide CEDT table
Signed-off-by: Robert Kiraly <me@boldcoder.com>

This is a simple patch to the ACPI subsystem. The patch adds ACPI
_SIG_CEDT to table_sigs[] in  "drivers/acpi/tables.c".

Presently,  CXL code isn't able to make use of the  CEDT table at
the initrd stage. Subsequent to the change, the CEDT table can be
used.

If  a developer  plans to use the CEDT table to  program  the CXL
ports,  this change is needed.  Otherwise,  it isn't  expected to
make a difference.

I certify  compliance with  Developer’s Certificate of Origin 1.1
as listed in:
https://www.kernel.org/doc/html/v4.10/process/submitting-patches.
html

--- linux-5.16.old/drivers/acpi/tables.c
+++ linux-5.16/drivers/acpi/tables.c
@@ -500,7 +500,7 @@
  	ACPI_SIG_WDDT, ACPI_SIG_WDRT, ACPI_SIG_DSDT, ACPI_SIG_FADT,
  	ACPI_SIG_PSDT, ACPI_SIG_RSDT, ACPI_SIG_XSDT, ACPI_SIG_SSDT,
  	ACPI_SIG_IORT, ACPI_SIG_NFIT, ACPI_SIG_HMAT, ACPI_SIG_PPTT,
-	ACPI_SIG_NHLT };
+	ACPI_SIG_CEDT, ACPI_SIG_NHLT };

  #define ACPI_HEADER_SIZE sizeof(struct acpi_table_header)

==== end of patch ====

Re: Patch: Make ACPI subsystem provide CEDT table

[Rafael J. Wysocki]

On Thu, Jan 27, 2022 at 11:15 AM Robert Kiraly <me@boldcoder.com> wrote:
>
> This is a simple ACPI patch. I'm submitting a revised version to
> linux-acpi per Rafael Wysocki and linux-cxl per Dan Williams.
>
> Thunderbird has added CRs so "patch -p1 -l" is needed.
>
> ==== start of patch ====
> Subject: Make ACPI subsystem provide CEDT table
> Signed-off-by: Robert Kiraly <me@boldcoder.com>
>
> This is a simple patch to the ACPI subsystem. The patch adds ACPI
> _SIG_CEDT to table_sigs[] in  "drivers/acpi/tables.c".
>
> Presently,  CXL code isn't able to make use of the  CEDT table at
> the initrd stage. Subsequent to the change, the CEDT table can be
> used.
>
> If  a developer  plans to use the CEDT table to  program  the CXL
> ports,  this change is needed.  Otherwise,  it isn't  expected to
> make a difference.
>
> I certify  compliance with  Developer’s Certificate of Origin 1.1
> as listed in:
> https://www.kernel.org/doc/html/v4.10/process/submitting-patches.
> html
>
> --- linux-5.16.old/drivers/acpi/tables.c
> +++ linux-5.16/drivers/acpi/tables.c
> @@ -500,7 +500,7 @@
>         ACPI_SIG_WDDT, ACPI_SIG_WDRT, ACPI_SIG_DSDT, ACPI_SIG_FADT,
>         ACPI_SIG_PSDT, ACPI_SIG_RSDT, ACPI_SIG_XSDT, ACPI_SIG_SSDT,
>         ACPI_SIG_IORT, ACPI_SIG_NFIT, ACPI_SIG_HMAT, ACPI_SIG_PPTT,
> -       ACPI_SIG_NHLT };
> +       ACPI_SIG_CEDT, ACPI_SIG_NHLT };
>
>   #define ACPI_HEADER_SIZE sizeof(struct acpi_table_header)
>
> ==== end of patch ====

I've queued up this change for 5.18, but I needed to rebase the patch
and I changed its subject and changelog.

Thanks!

Re: Patch: Make ACPI subsystem provide CEDT table

[Robert Kiraly]

On 02/04/2022 10:45AM, Rafael J. Wysocki wrote:
 >
 > I've queued up this change for 5.18, but I needed to rebase the patch
 > and I changed its subject and changelog.
 >
 > Thanks!

Thank you for your time and patience in discussion of issues.

Re: Patch: Make ACPI subsystem provide CEDT table

[Dan Williams]

[ add linux-cxl ]


On Thu, Jan 20, 2022 at 10:49 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Jan 13, 2022 at 12:16 AM Robert Kiraly <me@boldcoder.com> wrote:
> >
> > This is a simple ACPI patch. I'm submitting a revised version to
> > linux-acpi as opposed to linux-pm at Rafael Wysocki's suggestion.
> >
> > The patch adds ACPI_SIG_CEDT to table_sigs[] in "drivers/acpi/tables.c".
>
> This can be done, but please send a patch in the message body (not as
> an attachment) and provide a Signed-off-by tag for it.

Yes, although I don't see much incremental benefit to developers
publishing a root only raw table given it needs to be parsed along
with all the other cxl information in the system. So, not opposed to
the patch, but I want to direct you to the sysfs representation of the
CEDT and CFMWS in /sys/bus/cxl/devices is what the cxl tool and libcxl
library [1] will be using instead of the raw table. Platform firmware
CXL details are modeled as "CXL root" objects in the /sys/bus/cxl
device model.

[1]: https://github.com/pmem/ndctl/tree/main/cxl