Re: [PATCH v3 1/6] x86/resctrl: Parse ACPI ERDT table and map RMDD domains by L3 cache ID

[Thomas Gleixner <tglx@kernel.org>]

On Sat, Jun 06 2026 at 10:32, Chen Yu wrote:
> +
> +static inline struct acpi_subtbl_hdr_16 *rmdd_subtbl(struct acpi_erdt_rmdd *rmdd)
> +{
> +	return (void *)rmdd + sizeof(*rmdd);
> +}
> +
> +static inline struct acpi_subtbl_hdr_16 *next_subtbl(struct acpi_subtbl_hdr_16 *subtbl)
> +{
> +	return (void *)subtbl + subtbl->length;
> +}

Second thoughts on this. Hit send too fast.

> +static inline bool subtbl_valid(struct acpi_erdt_rmdd *rmdd, struct acpi_subtbl_hdr_16 *subtbl)
> +{
> +	void *rmdd_end = (void *)rmdd + rmdd->header.length;
> +
> +	if (subtbl->length < sizeof(*subtbl))
> +		return false;
> +
> +	if ((void *)subtbl + sizeof(*subtbl) > rmdd_end)
> +		return false;
> +
> +	if ((void *)subtbl + subtbl->length > rmdd_end)
> +		return false;

These conditions are confusing. This basically allows subtbl->length to
be larger than sizeof(tbl) as long as it's within the limits.

If that's intentional then the second condition is pointless because
according to the first condition

          length >= sizeof(tbl)

so

          tbl + length <= end

is catching both. No?

> +	return true;

> +static __init int enumerate_erdt_table(struct acpi_table_header *table_hdr)
> +{
> +	struct acpi_table_erdt *erdt = (struct acpi_table_erdt *)table_hdr;
> +	struct acpi_subtbl_hdr_16 *subtbl;
> +	void *table_end;
> +
> +	if (erdt->header.revision != ERDT_VALID_VERSION) {
> +		pr_info("Unknown ERDT table revision %d\n", erdt->header.revision);
> +		return -EINVAL;
> +	}
> +
> +	if (erdt->header.length < sizeof(*erdt)) {
> +		pr_info(FW_BUG "ERDT: Invalid table length %u\n", erdt->header.length);
> +		return -EINVAL;
> +	}
> +
> +	subtbl = (void *)erdt + sizeof(struct acpi_table_erdt);
> +	table_end = (void *)erdt + erdt->header.length;
> +
> +	while ((void *)subtbl + sizeof(*subtbl) <= table_end) {
> +		if (subtbl->length < sizeof(*subtbl) ||
> +		    (void *)subtbl + subtbl->length > table_end) {
> +			pr_info("ERDT: Invalid subtable length\n");
> +			goto cleanup;
> +		}

So this is yet another version of the above, just slighty different with
the same strange conditions.

If you make subtbl_valid():

static inline bool subtbl_valid(void *end, struct acpi_subtbl_hdr_16 *subtbl)
   
and calculate the end at the call sites you can reuse that function.

> +
> +		if (subtbl->type == ACPI_ERDT_TYPE_RMDD)
> +			if (!parse_rmdd_entry(subtbl))
> +				goto cleanup;
> +
> +		subtbl = (void *)subtbl + subtbl->length;

 open coded variant of next_subtbl()