Re: [RFC PATCH 4/6] acpi/hmat: Add helper functions to provide extended linear cache translation

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Wed, 30 Oct 2024 15:53:20 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> On 10/17/24 9:33 AM, Jonathan Cameron wrote:
> > On Fri, 27 Sep 2024 07:16:56 -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.  
> > 
> > What does the I/O hole correspond to in the system?
> >   
> >>
> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> >> ---
> >>  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.
> >  
> Ok will do that
>  
> > 
> >   
> >> +		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?  
> 
> Do you mean if the address mapping starts out with an MMIO range and then memory range? I'm not sure if such an implementation exists in the x86 world. And if the hole is behind all the ranges, then it shouldn't impact the address calculations. 
> 
That was me not really understanding what the hole was.
Tony filled in that gap.

> > 
> > or rename the function to include preceding_hole or something like that.  
> >> +
> >> +	return 0;
> >> +}

>