Re: [RFC PATCH v2 3/5] acpi/hmat: Add helper functions to provide extended linear cache translation

Jonathan Cameron
2024-11-27T10:23:34Z

On Tue, 12 Nov 2024 15:12:35 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Add helper functions to help do address translation for either the address
> of the extended linear cache or its alias address. The translation function
> attempt to detect an I/O hole in the proximity domain and adjusts the
> address if the hole impacts the aliasing of the address. The range of the
> I/O hole is retrieved by walking through the associated memory target
> resources.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
Trivial comment inline. I'm far from expert on requirements here but it
seems to match your description.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> v2:
> - Drop extra variable and use 'res' from the loop. (Jonathan)
> - Break up multiple if statements into single blocks and add comments. (Jonathan)
> ---
> drivers/acpi/numa/hmat.c | 148 +++++++++++++++++++++++++++++++++++++++
> include/linux/acpi.h | 14 ++++
> 2 files changed, 162 insertions(+)
>
> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
> index 92b818b72ecc..6c686d3c7266 100644
> --- a/drivers/acpi/numa/hmat.c
> +++ b/drivers/acpi/numa/hmat.c
> @@ -152,6 +152,154 @@ int hmat_get_extended_linear_cache_size(struct resource *backing_res, int nid,
> }
> EXPORT_SYMBOL_NS_GPL(hmat_get_extended_linear_cache_size, CXL);
>
> +static int alias_address_find_iohole(struct memory_target *target,
> + u64 address, u64 alias, struct range *hole)
> +{
> + struct resource *res, *prev;
> +
> + *hole = (struct range) {
> + .start = 0,
> + .end = -1,
> + };
> +
> + /* First find the resource that the address is in */
> + prev = target->memregions.child;
> + for (res = target->memregions.child; res; res = res->sibling) {
> + if (alias >= res->start && alias <= res->end)
> + break;
> + prev = res;
> + }
> + if (!res)
> + return -EINVAL;
> +
> + /* No memory hole */
> + if (res == prev)
> + return 0;
> +
> + /* If address is within the current resource, no need to deal with memory hole */

Rather long line that could be easily broken.

> + if (address >= res->start)
> + return 0;
> +
> + *hole = (struct range) {
> + .start = prev->end + 1,
> + .end = res->start - 1,
> + };
> +
> + return 0;
> +}