Re: [PATCH v4 04/23] cxl: Add common helpers for cdat parsing

[Dan Williams <dan.j.williams@intel.com>]

Dave Jiang wrote:
> 
> 
> On 4/24/23 3:33 PM, Dan Williams wrote:
> > Dave Jiang wrote:
> >> Add helper functions to parse the CDAT table and provide a callback to
> >> parse the sub-table. Helpers are provided for DSMAS and DSLBIS sub-table
> >> parsing. The code is patterned after the ACPI table parsing helpers.
> > 
> > It seems a shame that CDAT is so ACPI-like, but can't reuse the ACPI
> > table parsing infrastructure. Can this not be achieved by modifying some
> > of the helpers helpers in drivers/acpi/tables.c to take a passed in
> > @table_header?
> 
> Rafael,
> Do you have any issues with adding some endieness support in 
> drivers/acpi/tables.c in order to support CDAT parsing by BE hosts? To 
> start off with something like below?

Some additional background, recall that CDAT is an ACPI-like data
structure that lives on endpoint CXL devices to describe the access
characteristics of the device's memory similar to SRAT+HMAT for
host-memory. Unlike ACPI that is guaranteed to be deployed on a
little-endian host-cpu, a big-endian host might also encounter CXL
endpoints.

This reuse ends up at ~50 lines and duplication ends up at ~100 lines.
Not a huge win, but a win nonetheless.

> 
> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> index 7b4680da57d7..e63e2daf151d 100644
> --- a/drivers/acpi/tables.c
> +++ b/drivers/acpi/tables.c
> @@ -287,6 +287,12 @@ acpi_get_subtable_type(char *id)
>          return ACPI_SUBTABLE_COMMON;
>   }
> 
> +static unsigned long __init_or_acpilib
> +acpi_table_get_length(struct acpi_table_header *hdr)
> +{
> +       return le32_to_cpu((__force __le32)hdr->length);
> +}
> +
>   static __init_or_acpilib bool has_handler(struct acpi_subtable_proc *proc)
>   {
>          return proc->handler || proc->handler_arg;
> @@ -337,7 +343,8 @@ static int __init_or_acpilib acpi_parse_entries_array(
>          int errs = 0;
>          int i;
> 
> -       table_end = (unsigned long)table_header + table_header->length;
> +       table_end = (unsigned long)table_header +
> +                   acpi_table_get_length(table_header);
> 
>          /* Parse all entries looking for a match. */
>