On Fri, 27 Sep 2024 07:16:56 -0700
Dave Jiang <dave.jiang@xxxxxxxxx> 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.
What does the I/O hole correspond to in the system?
>
> Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
> ---
> drivers/acpi/numa/hmat.c | 136 +++++++++++++++++++++++++++++++++++++++
> include/linux/acpi.h | 14 ++++
> 2 files changed, 150 insertions(+)
>
> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
> index d299f8d7af8c..834314582f4c 100644
> --- a/drivers/acpi/numa/hmat.c
> +++ b/drivers/acpi/numa/hmat.c
> @@ -152,6 +152,142 @@ 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 *alias_res = NULL;
> + 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) {
> + alias_res = res;
> + break;
> + }
> + prev = res;
> + }
> + if (!alias_res)
if (!res) and you can just use res instead of alias_res for the following
as you exit the loop with it set to the right value.
> + return -EINVAL;
> +
> + /* No memory hole */
> + if (alias_res == prev)
> + return 0;
> +
> + /* If address is within the current resource, no need to deal with memory hole */
> + if (address >= alias_res->start)
> + return 0;
> +
> + *hole = (struct range) {
> + .start = prev->end + 1,
> + .end = alias_res->start - 1,
> + };
Ordering assumption should be avoided in such a generic
sounding function. Can the hole be first?
or rename the function to include preceding_hole or something like that.
> +
> + return 0;
> +}
> +
> +int hmat_extended_linear_cache_alias_xlat(u64 address, u64 *alias, int nid)
> +{
> + unsigned int pxm = node_to_pxm(nid);
> + struct memory_target *target;
> + struct range iohole;
> + int rc;
> +
> + target = find_mem_target(pxm);
> + if (!target)
> + return -EINVAL;
> +
> + rc = alias_address_find_iohole(target, address, *alias, &iohole);
> + if (rc)
> + return rc;
> +
> + if (!range_len(&iohole))
> + return 0;
> +
Maybe reformat like this and add comments on each condition.
if (address >= iohole.start)
return 0;
if (*alias <= iohole.start)
return 0;
*alias += range_len(&iohole);
return 0;
> + if (address < iohole.start) {
> + if (*alias > iohole.start) {
> + *alias = *alias + range_len(&iohole);
> + return 0;
> + }
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(hmat_extended_linear_cache_alias_xlat, CXL);
> +
> +static int target_address_find_iohole(struct memory_target *target,
> + u64 address, u64 alias,
> + struct range *hole)
> +{
> + struct resource *addr_res = NULL;
> + struct resource *res, *next;
> +
> + *hole = (struct range) {
> + .start = 0,
> + .end = -1,
> + };
> +
> + /* First find the resource that the address is in */
> + for (res = target->memregions.child; res; res = res->sibling) {
> + if (address >= res->start && address <= res->end) {
> + addr_res = res;
Could just use res as it's scope is outside the loop.
> + break;
> + }
> + }
> + if (!addr_res)
> + return -EINVAL;
> +
> + next = res->sibling;
> + /* No memory hole after the region */
> + if (!next)
> + return 0;
> +
> + /* If alias is within the current resource, no need to deal with memory hole */
> + if (alias <= addr_res->end)
> + return 0;
> +
> + *hole = (struct range) {
> + .start = addr_res->end + 1,
> + .end = next->start - 1,
> + };
> +
> + return 0;
> +}
> +
> +int hmat_extended_linear_cache_address_xlat(u64 *address, u64 alias, int nid)
> +{
> + unsigned int pxm = node_to_pxm(nid);
> + struct memory_target *target;
> + struct range iohole;
> + int rc;
> +
> + target = find_mem_target(pxm);
> + if (!target)
> + return -EINVAL;
> +
> + rc = target_address_find_iohole(target, *address, alias, &iohole);
> + if (rc)
> + return rc;
> +
> + if (!range_len(&iohole))
> + return 0;
> +
Similar to above, maybe break into multiple reasons to exit early.
> + if (alias > iohole.end) {
> + if (*address < iohole.end) {
> + *address = *address - range_len(&iohole);
> + return 0;
> + }
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(hmat_extended_linear_cache_address_xlat, CXL);
>
Jonathan
