From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A949C23BD17 for ; Mon, 8 Jun 2026 08:59:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780909200; cv=none; b=rWj4FwrIYbsG+/ZnTDwDJaPqHBQLeCPNlERzRxTzKu7jrewkNDd4qxwFllFTs8dC7pbn87S0blt3dWUOepUg1gxYxCZh0vP98KzTGTavwXgthT/PgEV7h956GsVsa9TeWaWVfqVKpEgvsjik2tah1F6hctjaf0HRfM3q2NyOrIs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780909200; c=relaxed/simple; bh=Eaiom6nnIk9PnU1RZs+J9rq/V6BbDNWH9zrDC2JyZ0o=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=HXrI2NHt4MMPeNwU7Q/DRyPhD/+zpaZSA67tUUW8OiTTGWIonfxm26zFj1VMr5mzt4Sr5SK3Xqa5LeBs/z1YTeXbM4UGc/RclcuVU9MRlrw3gzmZKFRNNkqQL/cIhtUWOTmknMMcj39XkfQY3NY8XJdXta6vD8Yds6nfdprIDNU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=TzYuscQy; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="TzYuscQy" Received: by smtp.kernel.org (Postfix) with ESMTPSA id C10BB1F00893; Mon, 8 Jun 2026 08:59:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1780909199; bh=PBLQbP+CWRuJKW7HBlC8tZ57eijqufT4Zc+S0l9nD7M=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=TzYuscQyJmU8EjJnfqvA3nMkTHvLqxQjzzmmbYsLyfGAqnVo/+HItYUF/TKOf+RCQ O0peLIPQPgtAOTHny6rR8j8lejS3rpJbPzvp6mO+OAfDz4exWT86gsfq6adbdHT2UD kyuOKxHzYioONnwFI7h8967vKqvIgepDSd1g0xQw7/M+I3RirY0ZLT+7BDsVR9NrGs tTkSVp7bVBzwv6sPsy/0x9wJa/YBWyG5vwA9mfeCRBxQAKPpUWMKiHczlb/YK6Wm0G D/HNodZ7AGONN4kXQdL9pwbI53w0cdzdfdJ7qGjg42f2ULeBEG+VGgPlSfGG28YMQD mX3DbCAls+SFg== From: Thomas Gleixner To: Chen Yu , tony.luck@intel.com, reinette.chatre@intel.com Cc: x86@kernel.org, linux-kernel@vger.kernel.org, bp@alien8.de, mingo@redhat.com, dave.hansen@linux.intel.com, hpa@zytor.com, dave.martin@arm.com, james.morse@arm.com, fenghuay@nvidia.com, babu.moger@amd.com, anil.keshavamurthy@broadcom.com Subject: Re: [PATCH v3 1/6] x86/resctrl: Parse ACPI ERDT table and map RMDD domains by L3 cache ID In-Reply-To: <53eeaead2d02cfa53204fe3101df3c0a09796ff9.1780710620.git.yu.c.chen@intel.com> References: <53eeaead2d02cfa53204fe3101df3c0a09796ff9.1780710620.git.yu.c.chen@intel.com> Date: Mon, 08 Jun 2026 10:59:56 +0200 Message-ID: <87qzmh4dbn.ffs@fw13> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain 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()